docs: carve browser-held key custody out of the all-crypto-in-Rust rule - #1270
Merged
Conversation
Rule 4 said without qualification that TypeScript has no crypto of its own, and main now contradicts it: #1242 shipped a non-extractable AES-GCM wrapping key for the Core Kit store, and ADR 0009 needs a device identity key that signs an approval exchange before the vault key exists. Neither case can move to the engine. A WASM implementation materializes key bytes in linear memory, which is the property non-extractability denies, and before start(secret) there is no session to derive from. Bounds the exception rather than widening it: local state only, no KDF edge, no wire format, no KAT, and the key never leaves WebCrypto. Entire-Checkpoint: eedb31e3ced7
WalkthroughThe crypto policy now permits two narrowly scoped WebCrypto exceptions for non-extractable browser-held local-state keys. Protocol-related cryptography remains implemented in Rust. ChangesCrypto policy
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 33: Update the WebCrypto exception in AGENTS.md to remove the device
identity key and ADR 0009 reference, leaving only the Core Kit store’s wrapping
key as the permitted browser-held key. Preserve the requirements that the key
protects local state only, derives nothing, touches no wire format or KAT, and
never leaves WebCrypto.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fd245f1-22c4-479e-a9af-9b8e2fcdb0b2
📒 Files selected for processing (1)
AGENTS.md
| 2. **Never** log sensitive keys or seeds | ||
| 3. **Never** send unencrypted keys to the server — the server is zero-knowledge and NEVER sees plaintext or unencrypted keys | ||
| 4. **All crypto lives in `crates/core`** — TypeScript has no codec or crypto of its own; never implement crypto in TS | ||
| 4. **All crypto lives in `crates/core`** — TypeScript has no codec or crypto of its own; never implement crypto in TS. **One exception: browser-held key custody via WebCrypto**, for a key that must be non-extractable, or must exist before the engine has a session. The engine cannot serve either case — a WASM implementation necessarily materializes key bytes in linear memory, which is the property non-extractability exists to deny, and before `start(secret)` there is no session to derive from. Conditions, all of them: the key protects local state only, it derives nothing in the KDF catalog, it touches no wire format and no KAT, and it never leaves WebCrypto. Two live instances — the Core Kit store's wrapping key (`apps/web/src/auth/sealedStore.ts`), and the device identity key that signs a device-approval exchange before reconstruction (ADR 0009). Anything protocol-shaped is still Rust, without exception. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 \
'ADR 0009|device[-_ ]identity|device[-_ ]approval|approval request|CryptoKey|subtle\.(sign|verify)|start\(secret\)' .Repository: FSM1/cipher-box
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate ADR and context files ---'
git ls-files | grep -Ei '(^|/)(ADR|adr|CONTEXT|context)(/|\.|$)|0009|device.*approval|approval.*device' | head -200
printf '%s\n' '--- repository instructions ---'
cat -n AGENTS.md | sed -n '1,80p'
printf '%s\n' '--- exact approval/device references ---'
rg -n -i -C 6 \
'ADR[ _-]*0*009|device[-_ ]identity|device[-_ ]approval|approval request|approve(device|ment)?|reconstruction|reconstruct' \
--glob '!**/*.md' --glob '!AGENTS.md' --glob '!docs/**' .
printf '%s\n' '--- relevant documentation references ---'
rg -n -i -C 8 \
'ADR[ _-]*0*009|device[-_ ]identity|device[-_ ]approval|approval request|reconstruction|reconstruct' \
AGENTS.md CONTEXT.md blueprint docs 2>/dev/null | head -500Repository: FSM1/cipher-box
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all tracked files containing approval-related terms ---'
rg -l -i \
'device[-_ ]approval|approval request|device identity|reconstruct|reconstruction|ADR[ _-]*0*009' \
--glob '!**/node_modules/**' --glob '!**/dist/**' . | sort
printf '%s\n' '--- WebCrypto signing and verification call sites ---'
rg -n -C 12 \
'crypto\.subtle\.(sign|verify)|subtle\.(sign|verify)|generateKey\(' \
apps packages crates --glob '!**/*.md' 2>/dev/null | head -1000
printf '%s\n' '--- API/client paths related to device approval ---'
rg -n -i -C 10 \
'approval|device' \
apps/api packages apps/web crates --glob '!**/*.md' 2>/dev/null | head -1200Repository: FSM1/cipher-box
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- root and ADR directory inventory ---'
find . -maxdepth 3 -type f \( -iname 'CONTEXT.md' -o -iname 'AGENTS.md' -o -iname '*adr*' -o -path '*/adr/*' \) -print | sort
printf '%s\n' '--- tracked paths with 0009 or approval in name ---'
git ls-files | grep -Ei '0009|approval|device' | head -300
printf '%s\n' '--- protocol ownership statements ---'
rg -n -i -C 8 \
'protocol|wire format|wire data|WebCrypto|browser-held|device approval|challenge-signature|signature' \
blueprint/core.md blueprint/engine.md blueprint/web-client.md AGENTS.md 2>/dev/null | head -1000Repository: FSM1/cipher-box
Length of output: 441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- v2 ownership and login boundaries ---'
rg -n -i -C 12 \
'WebCrypto|browser-held|device approval|protocol|wire|signature|key custody|host seam|pre-engine' \
blueprint/core.md blueprint/engine.md blueprint/web-client.md CONTEXT.md | head -1200
printf '%s\n' '--- local authentication and device-approval design text ---'
rg -n -i -C 12 \
'device approval|device-approval|approval request|identity key|sign|reconstruct|cross-device' \
docs/AUTHENTICATION_ARCHITECTURE.md docs/ARCHITECTURE.md designs/DESIGN.md CHANGELOG.md | head -1000
printf '%s\n' '--- external frozen-corpus ADR 0009 availability ---'
for url in \
'https://api.github.com/repos/FSM1/cipher-box-next/contents/docs/adr/0009*' \
'https://api.github.com/repos/FSM1/cipher-box-next/git/trees/main?recursive=1' \
'https://raw.githubusercontent.com/FSM1/cipher-box-next/main/docs/adr/0009-device-approval.md' \
'https://raw.githubusercontent.com/FSM1/cipher-box-next/main/docs/adr/0009.md'
do
echo "URL: $url"
curl -L --fail --silent --show-error "$url" 2>/dev/null | head -80 || true
doneRepository: FSM1/cipher-box
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact normative statements ---'
sed -n '1,120p' blueprint/core.md
sed -n '1,180p' blueprint/engine.md
sed -n '185,215p' blueprint/web-client.md
printf '%s\n' '--- external corpus tree entries related to ADR 0009 ---'
curl -L --fail --silent --show-error \
'https://api.github.com/repos/FSM1/cipher-box-next/git/trees/main?recursive=1' |
python3 -c '
import json, sys
data=json.load(sys.stdin)
for item in data.get("tree", []):
path=item.get("path", "")
if "adr" in path.lower() or "approval" in path.lower() or "device" in path.lower():
print(path)
' | grep -Ei '0009|adr|approval|device' | head -300Repository: FSM1/cipher-box
Length of output: 26438
Remove the device identity key from the WebCrypto exception.
The v2 blueprint assigns device approval to Core Kit and derives the identity key in the engine from the login secret. A WebCrypto key that signs a device-approval exchange would violate the local-state-only and no-wire-format conditions. Keep this exception limited to the Core Kit wrapping key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 33, Update the WebCrypto exception in AGENTS.md to remove
the device identity key and ADR 0009 reference, leaving only the Core Kit
store’s wrapping key as the permitted browser-held key. Preserve the
requirements that the key protects local state only, derives nothing, touches no
wire format or KAT, and never leaves WebCrypto.
Source: Path instructions
Matches how blueprint/core.md and blueprint/engine.md already cite ADRs. Entire-Checkpoint: 89dd578ce0c3
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
1 participant
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.
Rule 4 said, without qualification, that TypeScript has no crypto of its own.
mainnow contradicts it, and a second case is already scoped.Why the rule has to give
#1242 shipped it.
apps/web/src/auth/sealedStore.tsmints a non-extractable AES-GCM key via WebCrypto and wraps the Core Kit store under it. As things stand, the next reviewer — human or bot — reads that file as a rule-4 violation.ADR 0009 needs it again, harder. The device identity key must sign an approval request before the vault key exists. v1's mistake is instructive: it wrapped the device key under HKDF of the vault private key, which made the key unavailable exactly when it was needed, so the approval request fell back to an unsigned throwaway identifier — and that is why v1's exchange ended up bound to self-reported strings an attacker controls.
Why neither case can move to
crates/coreThey are not "TypeScript is more convenient" cases.
facade.start(secret)there is no session and no derived key, so there is nothing for the engine to protect a pre-reconstruction key with.The exception is bounded, not open
All four conditions, together: the key protects local state only, it derives nothing in the KDF catalog, it touches no wire format and no KAT, and it never leaves WebCrypto. Anything protocol-shaped stays in Rust without exception.
The rule now names its two live instances, so a third use has to argue for itself rather than cite precedent.
Documentation only, no code surface.
markdownlint-cli2andprettier --checkclean.Note
Document WebCrypto exception to the all-crypto-in-Rust rule in AGENTS.md
Expands Critical Security Rule 4 in AGENTS.md to carve out a narrow exception for browser-held key custody via WebCrypto. The exception applies only when a non-extractable key is required or a key must exist before the engine session starts, and imposes strict conditions: the key protects only local state, derives nothing in the KDF catalog, touches no wire format or KAT, and never leaves WebCrypto. Two concrete instances are cited: the Core Kit store's wrapping key at sealedStore.ts and the device identity key used for device-approval before reconstruction (per ADR 0009). All protocol-shaped crypto remains in Rust.
Macroscope summarized 6519afc.
Summary by CodeRabbit