docs: add comprehensive environment architecture for multi-env isolation#49
Merged
Conversation
Addresses the Web3Auth key isolation problem where all environments sharing the same project leads to IPNS sequence number conflicts. Documents four environments (local, CI, staging, production) with: - Environment-specific Ed25519 key derivation salt approach - Docker Compose configurations for offline/online IPFS modes - Web3Auth network switching (Devnet vs Mainnet) - Complete env var reference and implementation checklist
Extends ENVIRONMENTS.md with detailed TEE layer implications: - Local/CI: TEE disabled, IPNS can expire, user resets to clean slate - Staging: TEE active with Phala testnet, needs DHT cleanup strategy for orphaned records when Web3Auth testnet IDs become unstable - Production: Full TEE with monitoring, alerting thresholds, and incident runbooks for IPNS staleness detection Includes: - TEE environment matrix and configuration per environment - Staging cleanup job interface for orphaned IPNS records - Production monitoring requirements (staleness, error rates, latency) - Implementation checklist for TEE infrastructure rollout - Incident response runbook for IPNS staleness
Security review of ENVIRONMENTS.md found: - 2 Medium: HKDF parameter naming, random vs derived key migration - 2 Low: Cross-env data accessibility, mock service /reset endpoint - 3 Info: TEE epoch design, Web3Auth shared identity, JWT placeholder Overall assessment: APPROVED WITH CONCERNS Core cryptographic approach is sound, needs clarification before impl.
WalkthroughAdds two planning documents: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.planning/ENVIRONMENTS.md:
- Around line 203-209: Add a one-line safety note next to the "Test Setup
Pattern" snippet warning that the mock routing POST /reset endpoint must be
disabled or removed outside local development and CI (e.g., staging/production)
to prevent accidental state resets; place this sentence immediately after the
fetch('http://localhost:3001/reset', { method: 'POST' }) example so readers see
the safeguard when scanning the Test Setup Pattern and explicitly reference the
/reset endpoint.
- Around line 585-590: The markdown table showing alerting thresholds violates
MD058 (missing blank lines around the table); open .planning/ENVIRONMENTS.md and
add a blank line immediately before the table header line ("| Metric | Warning |
Critical |") and a blank line immediately after the final table row ("| Epoch
rotation failures | Any | >10% |") so the table is surrounded by blank lines and
the MD058 lint error is resolved.
- Around line 36-39: Update the bullet that states "Same user can
encrypt/decrypt files across environments (if CIDs are known)" to include a
short explicit warning about cross-environment data-access risks: note that CID
leakage can expose test or production data across environments, recommend using
separate test accounts/credentials for CI and production, and advise sanitizing
or isolating test data to avoid accidental access. Ensure the added text is
brief and placed directly after that bullet so readers immediately see the risk
and the recommendation.
- Around line 320-323: The fenced code block containing the environment
variables (the block with WEB3AUTH_TEST_EMAIL and WEB3AUTH_TEST_OTP) lacks a
language tag; update the opening triple-backtick to include a language (e.g.,
bash) so the block becomes ```bash to satisfy markdownlint and enable proper
syntax highlighting for that snippet.
- Around line 30-64: The current deriveIpnsKeypair snippet conflates HKDF salt
and info (ENVIRONMENT_SALTS and info), risking incorrect HKDF usage; change to
use a fixed HKDF salt constant (e.g. HKDF_SALT =
'CipherBox-IPNS-HKDF-SHA256-v1') and treat environment/folder as the HKDF
info/context (build info = `${ENVIRONMENT_SALTS[environment]}:${folderId}` or
rename ENVIRONMENT_SALTS to ENVIRONMENT_CONTEXT), supply the user's secp256k1
private key as IKM to HKDF-SHA256, and call HKDF with (IKM, salt=HKDF_SALT,
info=that context) so deriveIpnsKeypair, ENVIRONMENT_SALTS (or
ENVIRONMENT_CONTEXT), and info correctly reflect salt vs info roles.
In @.planning/security/REVIEW-2026-01-25-environments.md:
- Around line 279-283: The fenced code block containing the environment list
(the three-line block starting with "local/CI: cipherbox-dev (Sapphire Devnet) -
ephemeral testing") is missing a language tag; update the opening fence from ```
to ```text so markdownlint recognizes it (i.e., change the fence for that block
to use "text" as the language).
- Around line 45-46: The doc at
.planning/security/REVIEW-2026-01-25-environments.md currently embeds a local
absolute path (/home/user/cipher-box/packages/crypto/src/keys/derive.ts);
replace that with a repo-relative path (e.g., packages/crypto/src/keys/derive.ts
or ./packages/crypto/src/keys/derive.ts) so readers can locate the file in any
clone, and leave the rest of the sentence unchanged; update any other
occurrences of similar absolute paths in this document to repo-relative
equivalents.
- Clarify HKDF salt vs info usage to match RFC 5869 best practices - Add security warning about cross-environment CID leakage risks - Add safety note for /reset endpoint (local/CI only) - Add language tags to fenced code blocks (bash, text) - Add blank lines around alerting thresholds table (MD058) - Replace absolute paths with repo-relative paths Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 88.80% 88.94% +0.14%
==========================================
Files 34 34
Lines 1000 1058 +58
Branches 185 199 +14
==========================================
+ Hits 888 941 +53
- Misses 68 72 +4
- Partials 44 45 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.planning/ENVIRONMENTS.md:
- Around line 301-307: The doc shows an unused VITE_WEB3AUTH_NETWORK var that
conflicts with code which derives network from VITE_ENVIRONMENT; remove the
VITE_WEB3AUTH_NETWORK line from the production (and the other referenced)
snippet(s) and instead document the canonical pattern using VITE_ENVIRONMENT
(and show the correct VITE_WEB3AUTH_CLIENT_ID per environment) so readers won't
be confused by a stale/unused env var.
- Around line 403-408: Update the environment reference table entries so all URL
values include explicit schemes: for VITE_API_URL change "localhost:3000" to
"http://localhost:3000" (and CI localhost similarly) and change
"staging-api.cipherbox.io" and "api.cipherbox.io" to
"https://staging-api.cipherbox.io" and "https://api.cipherbox.io"; for
VITE_PINATA_GATEWAY_URL change "localhost:8080/ipfs" to
"http://localhost:8080/ipfs" and the Pinata host entries to
"https://gateway.pinata.cloud/ipfs"; leave non‑URL values like
VITE_WEB3AUTH_CLIENT_ID and VITE_ENVIRONMENT unchanged but ensure the table
format remains consistent with earlier examples.
In @.planning/security/REVIEW-2026-01-25-environments.md:
- Around line 30-93: Update the review findings that reference the old
HKDF/snippet to match the updated ENVIRONMENTS.md: replace the pre‑fix examples
with the current pattern (HKDF_SALT, ENVIRONMENT_PREFIXES, info usage) and point
reviewers to the updated deriveIpnsKeypair/deriveKey/ed25519KeypairFromSeed
approach, or mark each affected finding (including the ones covering lines
149-187 and 190-229) as "Addressed" with a date and link to the new
ENVIRONMENTS.md; ensure references to the /reset and cross-environment access
items reflect the new text so the review no longer contradicts the architecture
doc.
🧹 Nitpick comments (1)
.planning/ENVIRONMENTS.md (1)
47-72: Point implementers to the existing HKDF helper to avoid custom crypto.The snippet reads like a fresh HKDF implementation. A short note referencing the existing key-derivation helper will steer readers away from rolling their own crypto.
📝 Suggested doc note
- // ... derivation logic using HKDF(IKM=userSecp256k1PrivateKey, salt=HKDF_SALT, info=info) + // ... derivation logic using existing deriveKey helper (HKDF-SHA256) in packages/crypto/src/keys/derive.tsBased on learnings: “Use Web Crypto API or libsodium.js for implementing AES-256-GCM, HKDF-SHA256 key derivation … never implement custom cryptographic functions.”
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive environment architecture documentation to address IPNS sequence number conflicts caused by shared Web3Auth keys across development environments. The documentation proposes an environment-specific key derivation approach that maintains encryption key compatibility while isolating IPNS namespaces.
Changes:
- Documents a four-environment architecture (local, CI, staging, production) with environment-specific IPNS key derivation
- Provides Docker Compose configurations for offline (local/CI) and online (staging) IPFS modes
- Includes comprehensive environment variable references and TEE infrastructure specifications
- Adds a security review document that identifies several concerns with the proposed architecture
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.planning/ENVIRONMENTS.md |
Comprehensive environment architecture specification with Docker Compose configs, key derivation strategy, and implementation checklists |
.planning/security/REVIEW-2026-01-25-environments.md |
Security analysis identifying HKDF parameter confusion, architectural change implications, and other security considerations |
Comment on lines
+61
to
+64
| type Environment = keyof typeof ENVIRONMENT_CONTEXT; | ||
|
|
||
| export function deriveIpnsKeypair( | ||
| userSecp256k1PrivateKey: Uint8Array, |
There was a problem hiding this comment.
The HKDF implementation example contains confusing terminology that conflicts with both RFC 5869 and existing codebase patterns. The variable is named salt but is then used to construct the info parameter. This pattern appears throughout the document (e.g., line 160: "Environment salt: local", line 265: "Environment salt: staging").
In HKDF:
saltshould be a fixed value for domain separation (e.g.,CipherBox-IPNS-v1)infoshould contain variable context (e.g., environment and folderId)
The existing codebase in packages/crypto/src/keys/hierarchy.ts uses CIPHERBOX_SALT = new TextEncoder().encode('CipherBox-v1') as the fixed salt and passes context strings via the info parameter.
This terminology should be corrected throughout the document to match the pattern shown in the security review document (lines 52-83 in REVIEW-2026-01-25-environments.md) to avoid implementation errors.
| } as const; | ||
|
|
||
| type Environment = keyof typeof ENVIRONMENT_CONTEXT; | ||
|
|
There was a problem hiding this comment.
The inline comment "HKDF-SHA256 with environment-specific salt" is misleading and inconsistent with proper HKDF usage. The environment identifier should be part of the info parameter (context binding), not the salt parameter.
The salt should be a fixed application-level constant (like CipherBox-IPNS-v1), while the info parameter should contain the variable context (environment + folderId). This comment reinforces the incorrect mental model that could lead to improper implementation.
Comment on lines
+47
to
+52
| ```typescript | ||
| // packages/crypto/src/ipns.ts | ||
|
|
||
| // Fixed HKDF salt for domain separation (extract phase) | ||
| const HKDF_SALT = 'CipherBox-IPNS-v1'; | ||
|
|
There was a problem hiding this comment.
The constant name ENVIRONMENT_SALTS is misleading since these values should not be used as HKDF salts. Consider renaming to ENVIRONMENT_PREFIXES or ENVIRONMENT_CONTEXTS to accurately reflect that these are info/context strings, not cryptographic salts. This naming would better align with the existing codebase patterns and HKDF terminology from RFC 5869.
Comment on lines
+29
to
+36
| ### Recommended Approach: Environment Salt in Ed25519 Key Derivation | ||
|
|
||
| The IPNS keypair is derived from the user's vault key. By adding an environment-specific salt to this derivation, we get different IPNS keys per environment while keeping the same Web3Auth identity. | ||
|
|
||
| **Key Insight:** The problematic shared state is the **IPNS keypair**, not the user's encryption keypair. We can: | ||
|
|
||
| 1. Keep the same secp256k1 keypair from Web3Auth (same encryption keys) | ||
| 2. Add environment salt only to Ed25519 IPNS key derivation (different IPNS identities) |
There was a problem hiding this comment.
The section title "Environment Salt in Ed25519 Key Derivation" perpetuates the HKDF terminology confusion. The environment identifier is used as part of the HKDF info parameter (context), not the salt parameter. Consider revising to "Environment-Specific IPNS Key Derivation" or "Environment Context in IPNS Key Derivation" to avoid confusion about HKDF parameter usage.
Suggested change
| ### Recommended Approach: Environment Salt in Ed25519 Key Derivation | |
| The IPNS keypair is derived from the user's vault key. By adding an environment-specific salt to this derivation, we get different IPNS keys per environment while keeping the same Web3Auth identity. | |
| **Key Insight:** The problematic shared state is the **IPNS keypair**, not the user's encryption keypair. We can: | |
| 1. Keep the same secp256k1 keypair from Web3Auth (same encryption keys) | |
| 2. Add environment salt only to Ed25519 IPNS key derivation (different IPNS identities) | |
| ### Recommended Approach: Environment-Specific IPNS Key Derivation | |
| The IPNS keypair is derived from the user's vault key. By adding an environment-specific context (via the HKDF `info` parameter) to this derivation, we get different IPNS keys per environment while keeping the same Web3Auth identity. | |
| **Key Insight:** The problematic shared state is the **IPNS keypair**, not the user's encryption keypair. We can: | |
| 1. Keep the same secp256k1 keypair from Web3Auth (same encryption keys) | |
| 2. Add environment context only to Ed25519 IPNS key derivation (different IPNS identities) |
ENVIRONMENTS.md: - Rename section title to "Environment-Specific IPNS Key Derivation" - Replace "environment salt" terminology with "environment context" throughout - Remove unused VITE_WEB3AUTH_NETWORK env var (network derived from VITE_ENVIRONMENT) - Add URL schemes to environment variable reference table REVIEW-2026-01-25-environments.md: - Update findings #1, #3, #4 status to "Addressed" with dates Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.planning/ENVIRONMENTS.md:
- Around line 667-676: Add a new "TEE Key Encryption Architecture" section to
document the encryption and key management before implementing the
encryptedIpnsPrivateKey flow: describe the threat model, state the encryption
primitive must be ECIES over secp256k1, outline the encryption flow (IPNS
private key → ECIES(secp256k1) using current TEE public key → store
encryptedIpnsPrivateKey), specify how TeeService obtains/verifies the TEE public
key (e.g., fetched from Phala API with signature/consistency checks or
pinned/rotated trust root), explain the interaction with
TEE_KEY_EPOCH_ROTATION_WEEKS (how epochs are versioned, whether old ciphertexts
remain decryptable or require re-encryption, and rotation/retirement
procedures), and spell out forward secrecy/compromise mitigation steps for
rotated keys and replay/rollback protections so implementers know exactly how to
encrypt, distribute, rotate, and decrypt encryptedIpnsPrivateKey.
In @.planning/security/REVIEW-2026-01-25-environments.md:
- Around line 233-269: Add explicit TEE epoch rotation details: define
GRACE_PERIOD (recommend at least the republish cycle, e.g.,
TEE_REPUBLISH_INTERVAL_HOURS or preferably 1 week) and document exact
epoch-handling logic for the API: accept current epoch (normal processing),
accept previous epoch within GRACE_PERIOD and trigger re-encrypt-and-update of
the record, and reject older epochs with a clear error directing clients to
fetch the latest TEE public key; also state what happens when grace expires
(expired-epoch records must be considered invalid or require
re-ingestion/migration policy) and include these definitions near the TEE
section where TEE_REPUBLISH_INTERVAL_HOURS and TEE_KEY_EPOCH_ROTATION_WEEKS are
defined.
- Around line 338-411: Add a new "Private Key Security Tests" suite that
supplements the existing IPNS tests: use generateTestUserKey and
deriveIpnsKeypair to implement tests that (1) assert no logs contain private key
patterns ('privateKey','ecdsa','0x04') by spying/mocking the project logger, (2)
assert localStorage and sessionStorage do not contain keys or entries matching
'privateKey','ipns','ed25519' after deriveIpnsKeypair runs, (3) assert any
encryptedIpnsPrivateKey sent to the TEE endpoint is encrypted and was encrypted
with the TEE public key by mocking the TEE client/endpoint and inspecting
payloads, and (4) assert IPNS record acceptance path calls the IPNS signature
verification function (e.g., verifyIpnsSignature or the code path that validates
signatures) and rejects forged signatures; add necessary test helpers/mocks for
logger, storage, and TEE to make these assertions deterministic.
🧹 Nitpick comments (2)
.planning/ENVIRONMENTS.md (2)
165-165: Consider a more obviously invalid JWT secret placeholder.The current placeholder
local-dev-jwt-secret-change-in-productioncould be accidentally used in production if configuration files are copied without careful review.Per security review Finding
#7, consider using a format that is unmistakably invalid for production:-JWT_SECRET=local-dev-jwt-secret-change-in-production +JWT_SECRET=INSECURE_LOCAL_DEV_ONLY_DO_NOT_USE_IN_PRODUCTION_xxxxxxxxxxxxxxxxThis makes it immediately obvious in logs or error messages that a development secret is in use.
402-408: Optional: Consider wrapping URL values in inline code backticks.markdownlint reports bare URLs in the environment variable reference table (lines 405-406). While these are reference data in a table (not prose links), wrapping them in inline code backticks would:
- Silence the linter warnings
- Make it clearer these are literal configuration values
- Improve copy-paste accuracy
Example for line 405:
-| `VITE_API_URL` | http://localhost:3000 | http://localhost:3000 | https://staging-api.cipherbox.io | https://api.cipherbox.io | +| `VITE_API_URL` | `http://localhost:3000` | `http://localhost:3000` | `https://staging-api.cipherbox.io` | `https://api.cipherbox.io` |Apply similarly to lines 406-407.
- Add comprehensive TEE Key Encryption Architecture section: - Document threat model and security goals - Add encryption flow diagram (client -> API -> TEE) - Specify ECIES over secp256k1 as encryption primitive - Document TEE public key distribution with attestation - Define key epoch rotation scheme with 1-week grace period - Add epoch validation logic and error handling - Document forward secrecy considerations - Update security review findings: - Mark TEE epoch rotation finding (#5) as addressed - Add comprehensive private key security test cases: - Test that private keys are never logged - Test that keys are not stored in localStorage/sessionStorage - Test that only encrypted keys are sent to TEE - Test IPNS signature verification prevents forgery Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.planning/ENVIRONMENTS.md:
- Around line 692-719: The fenced ASCII diagram block in
.planning/ENVIRONMENTS.md is missing a language tag (causing MD040); update the
opening fence of the diagram (the triple backticks before the "User Client
CipherBox API TEE (Phala Cloud)" line) to include the
language identifier "text" (i.e., change ``` to ```text) so the diagram block is
tagged correctly.
Comment on lines
+692
to
+719
| ``` | ||
| User Client CipherBox API TEE (Phala Cloud) | ||
| | | | | ||
| | 1. Fetch TEE public key | | | ||
| |-------------------------------->| | | ||
| | | 2. Get current epoch key | | ||
| | |-------------------------------->| | ||
| | |<--------------------------------| | ||
| |<--------------------------------| {publicKey, epoch} | | ||
| | | | | ||
| | 3. ECIES encrypt IPNS key | | | ||
| | with TEE public key | | | ||
| | | | | ||
| | 4. POST /folders/:id/publish | | | ||
| | {encryptedIpnsPrivateKey, | | | ||
| | keyEpoch, ipnsValue, ...} | | | ||
| |-------------------------------->| | | ||
| | | 5. Store for republishing | | ||
| | | (cron every 3 hours) | | ||
| | | | | ||
| | | 6. Send encrypted key to TEE | | ||
| | |-------------------------------->| | ||
| | | | 7. Decrypt in hardware | ||
| | | | 8. Sign IPNS record | ||
| | | | 9. Discard key (never stored) | ||
| | |<--------------------------------| | ||
| | | {signedIpnsRecord} | | ||
| ``` |
There was a problem hiding this comment.
Add language tag to ASCII diagram.
The fenced code block containing the encryption flow diagram is missing a language specification, triggering a markdownlint warning (MD040). Please add text as the language identifier.
✍️ Proposed fix
-```
+```text
User Client CipherBox API TEE (Phala Cloud)
| | |
| 1. Fetch TEE public key | |🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 692-692: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @.planning/ENVIRONMENTS.md around lines 692 - 719, The fenced ASCII diagram
block in .planning/ENVIRONMENTS.md is missing a language tag (causing MD040);
update the opening fence of the diagram (the triple backticks before the "User
Client CipherBox API TEE (Phala Cloud)"
line) to include the language identifier "text" (i.e., change ``` to ```text) so
the diagram block is tagged correctly.
This was referenced Feb 9, 2026
5 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the Web3Auth key isolation problem where all environments
sharing the same project leads to IPNS sequence number conflicts.
Documents four environments (local, CI, staging, production) with:
Summary by CodeRabbit