fix: gate key-material and pairing handlers to extension-page senders (v4.8.2)

@@ -1,199 +0,0 @@1-# Peach Browser Extension — Security & Reliability Audit2-3-**Version:** 4.4.6  4-**Audit date:** 2026-07-30  5-**Scope:** Full source audit (read-only), delta from v4.0.0  6-**Auditor:** Codex agent (GPT-5.6)  7-**Prior audit:** v4.0.0  8-9----10-11-## Executive Summary12-13-Peach v4.4.6 is **fit for production**. No critical or high-severity vulnerabilities were identified. The codebase demonstrates mature security engineering: a Rust-compiled Noise protocol core, per-entry authenticated encryption with key separation, constant-time comparisons at every authenticated site, Ed25519-signed device manifests with freshness windows, and a well-bounded sync receive path. The changes since v4.0.0 (recovery hardening, pairing/adoption, SAS verification, decentralized mesh convergence) are implemented safely and follow the established security patterns.14-15----16-17-## Findings by Threat Level18-19-### CRITICAL20-21-None identified.22-23-### HIGH24-25-None identified.26-27-### MEDIUM28-29-#### M-1: Session key persisted in `chrome.storage.session` (accepted risk)30-31-- **Location:** `src/background/vault.ts:1390–1410`32-- **Detail:** The AES-256 derived key is stored hex-encoded in `chrome.storage.session` to survive MV3 service-worker restarts. This storage is memory-backed and cleared on browser close, but a same-session XSS in an extension page (not a web page) could theoretically read it.33-- **Mitigation in place:** Extension pages have a strict CSP (`script-src 'self' 'wasm-unsafe-eval'; object-src 'none'`), no `eval`, no remote scripts. The content script (which runs in page context) cannot access `chrome.storage.session`. Destructive operations are gated to extension-origin senders.34-- **Verdict:** Accepted MV3 tradeoff. The alternative (re-prompt on every SW restart) is a worse UX with no meaningful security gain given the CSP posture.35-36-#### M-2: Tombstone accumulation without bound37-38-- **Location:** `src/background/sync-vault-adapter.ts:115–130`39-- **Detail:** Every deleted entry leaves a `{id, deletedAt}` tombstone that persists indefinitely and is sent in every sync session. A vault with tens of thousands of deletions over years will grow its sync manifest linearly.40-- **Mitigation in place:** `SYNC_LIMITS.MAX_TOMBSTONE_COUNT` (100,000) caps a single sync payload. Trash-retention purge eventually splices the entry array. Records are tiny (id + timestamp).41-- **Verdict:** Reliability concern for extreme long-term use, not a security issue. Documented in-code as a known limitation with a future pruning path.42-43-#### M-3: Recovery words encode 128 bits of a 256-bit root key44-45-- **Location:** `src/shared/recovery-words.ts:25–30`, `src/background/vault.ts:300–310`46-- **Detail:** The 12-word BIP39 phrase encodes only `rootKey[0..16]`. The recovery KEK is derived from a prefix of the plaintext it wraps. Effective recovery security is 128-bit, not 256-bit.47-- **Mitigation in place:** Documented extensively in-code (PCH-01-005). 128-bit security is the standard for BIP39 12-word phrases and is computationally infeasible to brute-force. The GCM ciphertext leaks nothing without the words.48-- **Verdict:** Intentional design, well-documented, standard security level for mnemonic recovery.49-50-### LOW51-52-#### L-1: `console.log`/`console.warn` statements in production code (76 instances)53-54-- **Location:** Various (`src/background/`, `src/shared/`)55-- **Detail:** 76 console statements remain in non-test source. None log secrets (keys, passwords, entry content) — verified by inspection. They log sync stage names, error messages, and diagnostic counters.56-- **Verdict:** No information leakage risk. Cosmetic/hygiene issue only.57-58-#### L-2: No lint script configured59-60-- **Location:** `package.json`61-- **Detail:** The project has no `lint` script. Type checking (`tsc --noEmit`) and the full test suite (3,638 tests) pass, but there is no static analysis for common JS/TS pitfalls (unused vars, implicit any, etc.).62-- **Verdict:** Reliability hygiene gap, not a security issue. The strict TypeScript config and comprehensive test suite compensate substantially.63-64-#### L-3: `unlockWithKey` accepts a raw hex key without Argon2id65-66-- **Location:** `src/background/vault.ts:1466`67-- **Detail:** Used by Android biometric unlock where the key was derived natively. The browser extension never calls this path from user input — it is only reachable via the Capacitor/native bridge.68-- **Verdict:** Correct separation of concerns. The native side owns the KDF for biometric unlock.69-70-#### L-4: HIBP breach check uses SHA-1 (dictated by API)71-72-- **Location:** `src/background/vault.ts:230–250`73-- **Detail:** The Have I Been Pwned k-anonymity API requires SHA-1 prefixes. This is not a Peach design choice. Response padding is requested (`Add-Padding: true`).74-- **Verdict:** No action possible; the API dictates the hash. The k-anonymity model (5-char prefix only) means the full password hash never leaves the device.75-76-### INFORMATIONAL77-78-#### I-1: Duress slot size ceiling (256 KiB)79-80-- **Location:** `src/background/vault.ts:70–75`81-- **Detail:** Both slots are padded to a fixed 256 KiB ceiling when duress is configured. A vault exceeding this would re-expose the duress bit via ciphertext length divergence. Documented as an extreme edge case.82-83-#### I-2: Subnet probe retired84-85-- **Location:** `src/background/sync-runner.ts:1–15`86-- **Detail:** LAN peer-to-peer sync is retired (`docs/sync-lan-retirement.md`). The subnet probe code remains in `sync-transport.ts` but is unreachable from the production runner. The relay is the sole transport.87-88-#### I-3: E2E passkey bypass compile-time flag89-90-- **Location:** `src/background/messages.ts:95`, `src/background/passkey.ts:25`91-- **Detail:** `__PEACH_E2E_PASSKEYS__` is folded to `false` by Vite's `define` in every production build. Verified in the bundled output by the build script. The gate is fully intact for users.92-93----94-95-## Security Architecture Assessment96-97-### Cryptography ✅98-99-| Component | Implementation | Assessment |100-|-----------|---------------|------------|101-| KDF | Argon2id (m=64MiB, t=3, p=4) | Exceeds OWASP password-manager tier |102-| Vault encryption | AES-256-GCM, random 96-bit nonce, 128-bit tag, AAD domain separation | Correct |103-| Per-entry sync encryption | HKDF-SHA256 per entry ID, AES-GCM with entry-bound AAD | Prevents slot-swap attacks |104-| Key zeroing | `zeroKey()` on all ephemeral/derived keys after use | Consistent |105-| Self-test | PBKDF2+AES-GCM KAT on every unlock | Catches broken WebCrypto |106-| Cross-platform KAT | Argon2id vector coordinated with Android BouncyCastle | Prevents silent param drift |107-108-### Protocol Security ✅109-110-| Component | Implementation | Assessment |111-|-----------|---------------|------------|112-| Initial pairing | Noise_NK_25519_ChaChaPoly_BLAKE2s (Rust `snow` → WASM) | Correct pattern choice |113-| Reconnect | Noise_KK with both static keys, no NK fallback | Prevents downgrade |114-| Post-handshake auth | HMAC-SHA256 bound to handshake hash + DH + domain separation | Prevents session hijack |115-| SAS | 8-digit from handshake hash (64 PRF bits, negligible modulo bias) | Correct |116-| Device manifest | Ed25519 signed, canonical JSON, ±freshness window, version-pinned | Correct |117-| Revocation | Terminal, key-matched (not just id-matched), self-revocation filtered | Prevents re-admission |118-119-### Input Validation ✅120-121-- Sync frame size: 24 MiB hard cap (MV-3)122-- Entry count: 100,000 per payload123-- Entry plaintext: 512 KiB per entry124-- Tombstone count: 100,000 per payload125-- Timestamp clamping: ±5 minutes forward skew (HV-1)126-- Pairing invitation: 128-bit bearer token, base64url validated127-- Adoption frames: strict type guards on every field128-- Relay URLs: `wss:` only, no credentials in URL129-130-### XSS / Injection Surface ✅131-132-- Content script `innerHTML`: all assignments are static SVG strings or typed numbers — no page-derived data reaches markup133-- Note editor: allow-list sanitizer (12 tags, all attributes stripped)134-- Popup: SolidJS (auto-escaping by default), no `dangerouslySetInnerHTML`135-- CSP: `script-src 'self' 'wasm-unsafe-eval'; object-src 'none'`136-- Message handler: sender validation, destructive ops gated to extension origin137-138-### Reliability ✅139-140-- 164 test files, 3,638 passing tests + 40 todo141-- Typecheck passes (`tsc --noEmit`)142-- Chrome/Firefox/Mobile production builds pass143-- MV3 static-import guard (build fails on runtime `import()` in worker graph)144-- Clipboard clear: eviction-proof (chrome.alarms + offscreen document + retry)145-- Auto-lock: alarm-based + idle detection, recovery-confirm exemption146-- Sync timeout: 30s per session, stage-level diagnostics147-- Mesh listener: bounded retry with exponential backoff, abort on lock148-149----150-151-## Delta Assessment (v4.0.0 → v4.4.6)152-153-| Change Area | Versions | Safety Verdict |154-|-------------|----------|----------------|155-| Recovery flow hardening (try/finally, canvas ref fix) | 4.0.x | ✅ Safe |156-| Onboarding reveal toggle + copy | 4.0.x | ✅ Safe |157-| Playwright smoke tests (recovery, popup lifecycle) | 4.0.x | ✅ Safe |158-| Pairing/adoption visual QA (QR, SAS, success states) | 4.4.4–4.4.5 | ✅ Safe (CSS/layout only) |159-| CSS cascade fix (compact selector specificity) | 4.4.5 | ✅ Safe |160-| Decentralized mesh convergence (deviceManifestVersion: 3) | 4.4.6 | ✅ Safe |161-| Vouched trust source promotion (direct/vouched → equal authority) | 4.4.6 | ✅ Safe |162-| SYNC_DEVICE_MANIFEST on equal entry versions | 4.4.6 | ✅ Safe |163-164-The mesh convergence change (v4.4.6) is the highest-risk delta. It was implemented correctly:165-- Capability-gated: peers that omit `deviceManifestVersion` retain the legacy fast path166-- Authority model: any admitted, Noise-authenticated peer with a valid signing key may propagate; unknown/revoked keys remain blocked167-- Revocation ordering: absorbed before vouching in the same manifest to prevent race168-- Self-revocation filtered: a peer cannot revoke the receiver's own device169-170----171-172-## Production Worthiness173-174-**Verdict: APPROVED FOR PRODUCTION**175-176-The codebase demonstrates:177-- Defense-in-depth at every trust boundary (transport + envelope encryption)178-- Constant-time comparisons at all authenticated sites179-- Strict input validation with bounded resource consumption180-- Comprehensive test coverage (3,638 tests) with adversarial and KAT suites181-- Clean separation between the content-script attack surface and the privileged background182-- Documented security model with inline rationale at every non-obvious decision183-- No hardcoded secrets, no eval, no remote code loading184-- MV3 compliance with static-import enforcement185-186-No blocking issues. The MEDIUM findings are accepted design tradeoffs with appropriate mitigations. The LOW findings are hygiene items that do not affect security posture.187-188----189-190-## Recommendations (non-blocking)191-192-1. Add a lint script (ESLint or Biome) to CI for ongoing hygiene.193-2. Consider pruning tombstones older than N months once all peers confirm receipt.194-3. Reduce console statements to structured debug logging behind a compile-time flag.195-4. Document the duress 256 KiB ceiling in user-facing materials for extreme vault sizes.196-197----198-199-*End of audit.*