add peach-code-review.md — independent static review: 0 Critical, 0 High, 1 Low (UPDATE_ENTRY patch sanitizer gap), 9 Informational; full crypto/boundary/sync/codex coverage + CONTINUITY record

@@ -0,0 +1,513 @@1+# Peach Browser Extension — Independent Code Review2+**Reviewer:** Claude Fable 53+**Date:** 2026-06-124+**Codebase:** peach-browser (v4.0.0)5+**Method:** Complete static read of all security-relevant source files6+7+## Overview8+9+Peach is a local-first, zero-knowledge browser password manager (Manifest V3,10+Chrome + Firefox, plus a native Android port that is out of scope here). The11+vault is encrypted on-device with AES-256-GCM under an Argon2id-derived key; no12+plaintext ever leaves the machine. Beyond storage it implements browser13+autofill, WebAuthn/passkey provisioning, a Pro-gated LAN device-sync feature14+(audio "chirp" pairing → Noise-encrypted WebSocket), a paper backup system15+("Codex": Reed-Solomon-coded Aztec matrices rendered to PDF), and a duress/decoy16+two-slot vault.17+18+The dominant impression from reading the code is that it has been through19+serious, repeated security review and that the fixes were integrated20+thoughtfully rather than bolted on. Nearly every security-relevant decision21+carries an inline rationale tied to a specific finding ID (audit `HV-*`/`MV-*`/22+`LV-*`, technical-report `PCH-01-*`). Cryptographic primitives are standard and23+correctly parameterised. The extension's trust boundaries — content script vs.24+background, page origin vs. vault — are enforced with browser-attested signals25+rather than payload-supplied ones. Error handling consistently fails closed for26+secrets and fails gracefully for availability.27+28+The findings below are, with one exception, Informational or Low. I found no29+Critical or High issues. This is consistent with a codebase that has already30+absorbed two prior review rounds; what remains is mostly defense-in-depth31+hardening, documented residual risks, and a small number of asymmetries worth32+tightening.33+34+## Scope35+36+Read completely:37+38+- **Cryptographic layer:** `src/crypto/argon2.ts`, `src/crypto/zstd.ts`,39+  `src/crypto/totp.ts`, `src/crypto/peachScript.ts`, `src/crypto/peachPayload.ts`,40+  `src/background/aes.ts` (the canonical AES module — there is no41+  `src/crypto/aes.ts`; see note in Scope deviations), `src/background/self-test.ts`,42+  `src/shared/noise-xx.ts`, `src/shared/noise-js.ts` (the Noise NK43+  implementation — there is no `src/shared/noise-nk.ts`), `src/shared/sync-identity.ts`,44+  `src/shared/recovery-words.ts`.45+- **Vault core:** `src/background/vault.ts` (entire file).46+- **Extension boundary:** `src/background/messages.ts`, `src/background/entry.ts`,47+  `src/background/clipboard.ts`, `src/content/index.ts`, `src/content/detect.ts`,48+  `src/content/save.ts`, and the relevant interfaces of `src/content/inject.ts`.49+- **Sync + pairing:** `src/background/chirp-sync-coordinator.ts`,50+  `src/background/chirp-sync-vault-adapter.ts`, `src/shared/chirp-sync-transport.ts`,51+  `src/background/chirp-sync-runner.ts`, `src/background/paired-devices.ts`,52+  `src/shared/chirp-payload.ts`, `src/shared/chirp-handshake.ts`,53+  `src/shared/device-manifest.ts`, `src/shared/sync-limits.ts`.54+- **Backup + recovery:** `src/popup/codex/generate.ts`, `restore.ts`,55+  `pdf-import.ts`, `erasure.ts`, `src/shared/import.ts`, `src/shared/backup.ts`,56+  `src/shared/import-limits.ts` (the `erasure.ts` referenced in the brief as57+  `src/shared/erasure.ts` is at `src/popup/codex/erasure.ts`).58+- **Configuration:** `manifest.json`, `manifest.firefox.json`, `package.json`.59+60+**Scope deviations:** three brief paths do not exist as written. AES-GCM lives in61+`src/background/aes.ts` and inline in `peachPayload.ts` (a header comment in62+`aes.ts:4` records that a duplicate `src/crypto/aes.ts` was removed); the Noise NK63+pattern is in `noise-js.ts`; the Codex erasure module is in `src/popup/codex/`.64+All three were located and read.65+66+## Cryptographic Implementation67+68+**Argon2id (master-password KDF)** — `argon2.ts`, also inline in `peachPayload.ts`69+and as parameters in `codex-types`. m = 65536 KiB (64 MiB), t = 3, p = 4, 32-byte70+output, via `hash-wasm`. This meets the OWASP password-manager tier and exceeds71+it on parallelism. The password is NFC-normalised before derivation72+(`argon2.ts:32`) so the same passphrase typed under different Unicode73+normalisation forms (macOS NFD dead-keys vs. mobile NFC) derives the same key and74+stays cross-platform-compatible; a `deriveKeyRaw` legacy path75+(`argon2.ts:40-45`) exists solely to open vaults sealed before that fix. The salt76+is 16 bytes from `crypto.getRandomValues`, fresh at creation and on every password77+change (`argon2.ts:50-52`). Appropriate and correct.78+79+**AES-256-GCM (vault-at-rest and export envelope)** — `aes.ts`,80+`peachPayload.ts:997-1051`. 96-bit random nonce per encryption (`aes.ts:56`),81+128-bit tag (the maximum, explicitly pinned and commented as non-reducible at82+`aes.ts:51-55`). Keys are imported non-extractable, used once, then zeroed by83+callers (`vault.ts` zeroes `derivedKey`/`rootKey` on lock and after key swaps).84+The AAD carries a per-slot domain tag (`"slot_a_v1"`, `"slot_b_v1"`, `"pw_root_v1"`)85+with a legacy single-byte-AAD fallback on decrypt so pre-hardening vaults still86+open (`aes.ts:104-129`). The construction is textbook and the nonce-reuse hazard87+is correctly avoided with fresh randomness rather than a counter. Note that GCM's88+~2³² safe-message bound per key is irrelevant here — a vault is re-encrypted as a89+single message per write.90+91+**BLAKE3 (payload checksum, vault id, card fingerprints)** — via `hash-wasm`. In92+`peachPayload.ts` a 4-byte truncated BLAKE3 checksum sits *inside* the encrypted93+envelope (`blake3Truncated`, `1053-1061`) purely to fail fast on94+compressed-payload corruption before decompression — authenticity is GCM's job,95+not the checksum's, and the code says so (`peachPayload.ts:27-29`). The checksum96+comparison is constant-time (`1063-1068`). `computeVaultId` is97+`BLAKE3(masterKey || "peach-vault-id")` (`sync-identity.ts:68-75`).98+99+**zstd / pako (compression)** — `zstd.ts`, `generate.ts`. Compression is not a100+security primitive but is a decompression-bomb surface; it is bounded101+(see PCH-02-008 context and `import-limits.ts`).102+103+**X25519 + ChaCha20-Poly1305 + BLAKE2s (Noise XX / NK)** — `noise-xx.ts`,104+`noise-js.ts`. Both patterns (`Noise_XX_25519_ChaChaPoly_BLAKE2s` for mutual-auth105+WebSocket sync, `Noise_NK_…` for post-chirp pairing) derive the protocol-name106+init hash and the HKDF chain over BLAKE2s, matching the named pattern (a prior107+round had used SHA-256 here; `PCH-01-002` corrected it). Because `@stablelib`'s108+HKDF reuses one HMAC across output blocks and BLAKE2s's `restoreState` leaves the109+`finished` flag set, the code runs HKDF-Expand manually with a fresh HMAC-BLAKE2s110+per block (`noise-xx.ts:46-58`), documented as byte-identical to the Android111+BouncyCastle side. Transport keys are split correctly and direction-swapped on112+the responder; nonces are per-frame counters from 0. The XX initiator pins the113+responder's static key *before* transmitting its own static in message 3114+(`noise-xx.ts:78-133`, `PCH-01-003`), so a probed-but-wrong LAN host never learns115+the prober's long-term identity.116+117+**Ed25519 (device-manifest signing)** — `sync-identity.ts:89-161`,118+`device-manifest.ts`. Deterministically derived from the root key under a119+distinct HKDF salt (`"peach-sync-signing-v1"`) so the signing key is120+single-purpose and survives reinstall. Manifests are signed over a canonical121+JSON encoding (sorted keys, no insignificant whitespace, non-finite numbers122+rejected — `device-manifest.ts:183-204`) and verified with strict per-device123+field validation and a freshness window. The private key is imported as124+extractable, which the inline comment justifies and `PCH-01-022` documents: the125+32-byte seed is already in memory, so a JWK export to recover the public key adds126+no attack surface.127+128+**HKDF-SHA256 (sync identity, recovery KEK, per-entry sync keys)** —129+`sync-identity.ts`, `recovery-words.ts:130-155`, `chirp-sync-vault-adapter.ts:355-369`.130+Each use is domain-separated. The per-entry sync envelope key is131+`HKDF(rootKey, info = entryId + "\x00" + "sync-entry-v1")`; the null-byte132+separator makes the (id, suffix) boundary unambiguous (`PCH-01-005`), and the133+adapter rejects any decrypted envelope whose `entry.id` does not equal the map134+key it arrived under, so a ciphertext minted for one slot cannot be replayed into135+another (`chirp-sync-vault-adapter.ts:153-155`).136+137+**PBKDF2-SHA256 (legacy export, self-test)** — `backup.ts:146-173` derives the138+key for the *legacy* encrypted-JSON import format with PBKDF2-SHA256 at 250,000139+iterations. `self-test.ts` uses PBKDF2 at 1,000 iterations deliberately, only as140+a fast Known-Answer-Test vehicle on the same AES-GCM path (it pins the expected141+ciphertext, `PCH-01-020`, so a self-consistent-but-broken WebCrypto fails). See142+PCH-02-005.143+144+**BIP39 recovery words** — `recovery-words.ts`. A 12-word phrase encodes a145+128-bit prefix of the 256-bit root key plus a 4-bit SHA-256 checksum, exactly per146+BIP39. The code is explicit and repeated about the security model147+(`recovery-words.ts:20-26`, `116-120`, `vault.ts:282-296`): recovery is 128-bit,148+the words wrap the *full* root key via an HKDF-derived KEK from a *prefix* of that149+key, and a future 256-bit-recovery scheme would need independent entropy. This is150+a correct and unusually well-documented construction.151+152+**TOTP** — `totp.ts` delegates to the `otpauth` library; the entry's stored153+algorithm/digits/period are threaded through (`PCH-01-019`) with RFC defaults154+(SHA-1/6/30) when absent. The raw base32 seed never crosses to a content script155+(see Extension boundary); the background generates the current code and ships only156+that plus seconds-remaining.157+158+**PeachScript** — `peachScript.ts` is a trivial byte → Unicode-PUA codec159+(U+E000…U+E0FF) for the device-to-device export string. It is presentation, not160+cryptography — the payload it carries is already an AES-GCM envelope — and the161+file says so. Decode validates every code point is in range.162+163+## Code Review Findings164+165+### Finding PCH-02-001: Update-entry patch path bypasses the canary/tags/passkey sanitizers used on add and restore166+**Severity:** Low167+**Location:** `src/background/vault.ts:2317-2417` (`sanitizeEntryPatch`),168+`src/background/messages.ts:478-501` (`UPDATE_ENTRY` handler)169+**Description:** Entries entering the vault via add (`sanitizeEntryDraft`,170+`vault.ts:2256-2315`), restore, or sync (`sanitizeRestoredEntry`,171+`vault.ts:1603-1650`) are run through field whitelists that rebuild `passkey`172+and `canary` sub-objects field-by-field and bound `tags`173+(`sanitizePasskey`/`sanitizeCanary`/`sanitizeTags`). The `UPDATE_ENTRY` path does174+not: `sanitizeEntryPatch` copies `patch.canary` (`2375-2377`), `patch.tags`175+(`2386-2388`), and `patch.passkey` (`2379-2384`) through verbatim. `UPDATE_ENTRY`176+is also not in `DESTRUCTIVE_MESSAGE_TYPES` (`messages.ts:141-146`), so unlike177+`DESTROY_VAULT`/`RESTORE_*` it is not gated to extension-page senders — and it is178+legitimately sent from content-script context (the inline editor and the179+canary-ping bump at `content/index.ts:336-348` and `content/save.ts:57-61`), so180+it cannot simply be gated. The net effect is that a caller able to send an181+`UPDATE_ENTRY` for a known entry id could attach an arbitrary `passkey`182+sub-object, set unbounded `tags`, or replace a `canary` object on an entry that183+already has one without the field rebuild the other paths enforce.184+**Notes:** Exploitability is genuinely low: web pages cannot call185+`chrome.runtime.sendMessage` (no `externally_connectable`), so this requires186+compromising the content script itself, and Peach's injected UI mounts in *closed*187+shadow roots (`content/index.ts:164`, `inject.ts` `attachShadow({mode:"closed"})`188+throughout), which removes the obvious in-page foothold. Adding a *new* canary is189+separately Pro-gated (`messages.ts:491-498`) and the canary fire path re-validates190+the webhook with the full SSRF guard (`isAllowedCanaryWebhookUrl`,191+`messages.ts:1568-1600`). The cleanest fix is to route `sanitizeEntryPatch`'s192+`canary`/`tags`/`passkey` branches through the same sanitizers the other paths193+use; the change is local and does not alter the trust boundary.194+195+### Finding PCH-02-002: Noise XX message 1 omits the canonical empty-payload hash step196+**Severity:** Informational197+**Location:** `src/shared/noise-xx.ts:93-97` (initiator), `184-191` (responder)198+**Description:** In a canonical Noise handshake, every message — including the199+first `-> e` — ends by processing its payload through `EncryptAndHash`, which for200+an empty payload with no symmetric key yet still performs `MixHash(empty)`. The201+Peach XX implementation processes the ephemeral (`mixHash(h, e.publicKey)`) and202+sends it, but does not perform the empty-payload `MixHash` for message 1. The NK203+implementation *does* hash its message payloads (`noise-js.ts:90-91`, `112-116`),204+because by those points a key exists and a real AEAD tag is produced. The205+consequence is that Peach's XX transcript hash diverges by one `MixHash(empty)`206+from what a stock Noise library (e.g. `snow`) would compute, so a third-party207+Noise stack would not interoperate with Peach's XX handshake.208+**Notes:** This is not a confidentiality or authenticity weakness: both Peach209+peers (and the Android hand-port) compute `h` identically, the handshake hash210+still binds the full transcript that both sides observe, and the transport keys211+derive from the chaining key, not `h`. The only practical effect is interop with212+external implementations, which Peach does not rely on (the doc comment at213+`noise-js.ts:11-14` notes interop is held "by porting discipline, not by a shared214+library"). The header comment in `noise-js.ts:12` that "a stock `snow` stack215+would interoperate" holds for NK but is not strictly true for XX; worth a216+one-line correction so the claim is not over-read.217+218+### Finding PCH-02-003: Pairing authentication rests on the user-confirmed SAS and an unverifiable peer-side gate219+**Severity:** Informational220+**Location:** `src/shared/chirp-handshake.ts:125-216`221+**Description:** The post-chirp pairing handshake uses Noise NK, which222+authenticates only the *responder* (it proves the initiator encrypted to the223+responder's chirp-broadcast ephemeral). It does not cryptographically224+authenticate the initiator, and there is no proof-of-possession over the225+`long_term_public_key_hex` a peer advertises in its `PAIR_IDENTITY` frame. The226+compensating control is a 6-digit Short Authentication String derived from the227+shared Noise handshake hash (`derivePairingSAS`, `163-174`), which the extension228+pairing flow requires the user to confirm matches the peer's before any device is229+persisted (`gateOnSAS`, referenced as living in `src/popup/chirp-pair.ts`).230+**Notes:** This is called out thoroughly and honestly in the source itself231+(`chirp-handshake.ts:125-154`) — one of the better in-code threat statements in232+the codebase. Residual risk is the standard SAS bound: a MITM/relay survives with233+probability ~1e-6 per attempt *and only if the user does not actually compare the234+codes*. The open item the code names — that the Android/native peer must enforce235+the identical SAS gate before persisting — cannot be verified from this236+repository and is the more important dependency: if the app auto-confirms, the237+channel is only as strong as its weakest end. Recommend confirming the app-side238+gate against the Android source, and (as the comment suggests) considering a239+future proof-of-possession round via the already-present240+`performInitiatorAuth`/`performResponderAuth` so authentication becomes structural241+rather than dependent on user diligence.242+243+### Finding PCH-02-004: Crypto-adjacent dependencies are caret-ranged while core crypto deps are exact-pinned244+**Severity:** Informational245+**Location:** `package.json:31-52`246+**Description:** The core vault/crypto/transport dependencies are exact-pinned247+(no caret): `@stablelib/*` 2.0.1, `hash-wasm` 4.12.0, `@bokuweb/zstd-wasm` 0.0.27,248+`@subspace/reed-solomon-erasure.wasm` 0.2.5, `tldts` 7.0.28, `pdfjs-dist`249+5.7.284, `@zxcvbn-ts/*` (this is the `LV-9` exact-pin discipline). Several250+dependencies on or adjacent to a security path remain caret-ranged: `otpauth`251+`^9.5.0` (TOTP generation), `pako` `^2.1.0` (Codex decompression on the import252+side), `@zxing/library` `^0.23.0` and `bwip-js` `^4.10.1` (Aztec decode/encode),253+`pdf-lib` `^1.17.1`, `papaparse` `^5.5.3`.254+**Notes:** With a committed lockfile (`bun.lock`, `packageManager` pinned to255+`[email protected]`, frozen-lockfile CI per `LV-6`) the practical exposure is limited to256+fresh resolves, but extending the exact-pin discipline to `otpauth` and `pako` in257+particular would make the supply-chain posture uniform with the rest of the258+crypto surface.259+260+### Finding PCH-02-005: Legacy encrypted-JSON import uses PBKDF2-SHA256 rather than Argon2id261+**Severity:** Informational262+**Location:** `src/shared/backup.ts:146-173`263+**Description:** The legacy `.json` export/import envelope derives its key with264+PBKDF2-SHA256 at 250,000 iterations, whereas every current path uses Argon2id265+(m=64 MiB, t=3, p=4). PBKDF2 with no memory-hardness is materially weaker against266+GPU/ASIC attackers than Argon2id at equivalent wall-clock cost.267+**Notes:** Scope is bounded and the decision is reasonable: this format's UI268+export was removed in v2.5.0 (`backup.ts:5-12`), the helper is import-only269+(plus a test fixture generator), and current exports use PeachScript / the QR270+matrix encoder backed by Argon2id. No action is strictly required; flagged so the271+KDF asymmetry is visible to anyone who still relies on old `.json` backups.272+273+### Finding PCH-02-006: vault_id is a stable, LAN-broadcast, cross-device-linkable identifier274+**Severity:** Informational275+**Location:** `src/shared/sync-identity.ts:62-75`276+**Description:** `computeVaultId` returns `BLAKE3(masterKey || "peach-vault-id")`,277+a deterministic 64-hex value that, per its own comment, is carried in the mDNS TXT278+record and the first sync handshake (`SYNC_HELLO`). It is therefore a stable279+identifier visible to any device on the same LAN, and it is identical across all280+of a user's paired devices, making them linkable to a passive local observer for281+as long as the master key (root key) is unchanged.282+**Notes:** This is inherent to the discovery design — peers must recognise "same283+vault" before exchanging entries — and it is one-way (the master key is not284+recoverable from it). It is a minor privacy property worth stating, not a285+confidentiality flaw. "Rotate sync identity" (`vault.ts:2771-2814`) changes it by286+re-keying the root key.287+288+### Finding PCH-02-007: HIBP breach check omits the Add-Padding response-padding header289+**Severity:** Informational290+**Location:** `src/background/vault.ts:216-238`291+**Description:** `checkBreach` queries the Have I Been Pwned k-anonymity range292+API with the SHA-1 prefix but does not send `Add-Padding: true`. HIBP supports293+that header to pad the response to a fixed bucket count; without it, the number294+of suffixes returned for a given prefix is a (weak) side channel that a295+network-position observer could use to narrow which prefix was queried.296+**Notes:** Impact is small — only the 5-hex-char prefix is ever sent, never the297+password or full hash, and breach monitoring is an opt-out convenience feature.298+Adding the padding header is a one-line hardening that costs nothing.299+300+### Finding PCH-02-008: peachPayload BinaryReader performs no explicit bounds checking301+**Severity:** Informational302+**Location:** `src/crypto/peachPayload.ts:940-969` (`BinaryReader`),303+consumed in `decodeEntry` `638-813`304+**Description:** The binary entry decoder reads `u8`/`u16`/`u32`/length-prefixed305+strings without validating that the cursor stays within the buffer. Past-end306+reads yield `undefined`/`NaN` (for `u8`) or throw `RangeError` (DataView), and307+length-prefixed reads clamp via `subarray`. The decoded bytes are normally inside308+an AES-GCM-authenticated envelope, but the same decoder runs on imported `.peach`309+payloads where an attacker who also knows the passphrase controls the plaintext.310+**Notes:** This is a robustness observation, not a memory-safety issue — the311+runtime is JavaScript, so the worst case is a thrown exception (caught by the312+import UI) or a malformed entry that is later rejected by `sanitizeRestoredEntry`313+at the vault boundary. The declared section-length and cumulative-size caps314+(`peachPayload.ts:301-311`, `MV-4`) already bound total work before this point.315+Explicit cursor bounds checks would make the decoder's failure modes uniform and316+self-evidently safe.317+318+### Finding PCH-02-009: X25519 shared-secret computations do not reject low-order/zero outputs319+**Severity:** Informational320+**Location:** `src/shared/noise-xx.ts:108,119,140,196,204,231`,321+`src/shared/noise-js.ts:84,106,175,248,263,362`322+**Description:** All `sharedKey(secret, peerPublic)` calls use `@stablelib/x25519`323+with the default `rejectZeroSharedKey = false`, so a peer public key chosen as a324+low-order point (yielding an all-zero shared secret) is not rejected at the DH325+step.326+**Notes:** The practical impact is neutralised by the surrounding protocol: NK327+pre-mixes the known responder static key, the XX responder static is pinned328+against the paired-device registry before and after the handshake329+(`chirp-sync-transport.ts:130-154`), and pairing is additionally gated by the330+human SAS comparison. An attacker forcing a zero shared secret would still fail331+the static-key pin and/or the SAS. Passing `rejectZero = true` would make the DH332+contributory-by-construction and remove the need to reason about this at all.333+334+### Finding PCH-02-010: Clipboard auto-clear is unconditional and does not compare against the parked hash335+**Severity:** Informational336+**Location:** `src/background/clipboard.ts:98-117,124-178`337+**Description:** On copy, a SHA-256 of the copied value is parked in338+`storage.session` purely as a "a clear is owed" flag (`98-103`); the auto-clear339+routine (`clearClipboardNow`) then writes an empty string to the clipboard340+unconditionally when the timer/alarm fires, without reading the current clipboard341+to confirm it still holds the Peach-copied value. If the user copies something342+else within the clear window, that later value is wiped too.343+**Notes:** This is a deliberate trade-off in favour of the344+secret-must-not-persist invariant (the alarm path is eviction-proof, `HV-2`, and345+clearing an already-changed clipboard is harmless to confidentiality), and a346+content/popup-side write with a real user gesture is preferred when available347+(`alreadyWritten`). It is a minor UX surprise rather than a security issue; noted348+for completeness because the parked hash is computed but never used for a match349+check, which could read as dead bookkeeping to a future maintainer.350+351+## Positive Observations352+353+The codebase's strengths are specific and worth recording at the same resolution354+as the findings.355+356+- **Two-slot duress with a real plausible-deniability story.** Both vault slots357+  always exist; the decoy/noise slot is filled with CSPRNG bytes that are358+  indistinguishable at rest from a GCM ciphertext (`vault.ts:704-716`), and when359+  duress is configured both slots are padded to a fixed 256 KiB ceiling so their360+  lengths cannot reveal that a decoy exists (`vault.ts:62-72,1038,2213`). The361+  duress bit lives only inside slot A's encrypted payload — no external flag362+  (`vault.ts:44-45,1051-1053`).363+364+- **Constant-time unlock.** `attemptUnlockWithDerive` loads both slots, derives365+  both keys, and attempts both decrypts in parallel and unconditionally366+  (`vault.ts:1124-1154`), so wall-clock time does not distinguish a real-vault367+  unlock from a decoy unlock (`DUR-002`).368+369+- **Origin enforcement uses browser-attested signals only.**370+  `attestedSenderOrigin` reads `sender.origin`/`sender.url` (which a page cannot371+  forge, since pages reach the background only via the content-script relay) and372+  never payload-supplied origins (`messages.ts:217-231`). `GET_ENTRY_FOR_FILL`373+  re-validates the entry's domain set against the requesting tab's attested374+  origin and refuses cross-domain fills unless the user explicitly summoned them375+  (`messages.ts:698-727`, `MV-1`); `GET_ENTRY_FOR_EDIT` mirrors it (`PCH-01-006`).376+377+- **Secrets are minimised at the boundary.** The fill response strips the passkey378+  credential, the raw TOTP base32 seed, and the full password history; the379+  background generates the *current* TOTP code and sends only that plus380+  seconds-remaining (`messages.ts:728-760`). The TOTP toast deliberately does not381+  render the live code into page-readable DOM (`content/index.ts:374-379`).382+383+- **Destructive operations are gated to extension-page senders.**384+  `DESTROY_VAULT`/`CLEAR_VAULT_ENTRIES`/`RESTORE_VAULT_SNAPSHOT`/`RESTORE_FROM_CODEX`385+  require an extension-origin `sender.url`, so a compromised content script cannot386+  escalate to vault destruction (`messages.ts:267-278`, `PCH-01-018`).387+388+- **Transactional disk writes with rollback.** `addEntry`/`updateEntry`/389+  `deleteEntry` clone-and-restore in-memory state if the persist throws390+  (`vault.ts:1783-1790,1883-1888,1927-1933`), and `persistSlotA` restores the391+  prior slot A if a later write fails under quota pressure392+  (`vault.ts:2193-2198`). `changePassword` writes the new ciphertext, salt, and393+  re-wrapped root key in a single storage transaction and swaps the in-memory key394+  synchronously before any further await (`vault.ts:2849-2879`).395+396+- **Monotonic timestamps and clamped peer input.** Entry timestamps are forced397+  strictly increasing against wall-clock regressions (`vault.ts:585-594`), and398+  every peer-supplied `modifiedAt`/`deletedAt` is clamped to `now + 5 min` before399+  any comparison (`sync-limits.ts:13-25`, applied in400+  `vault.ts:2035-2040,2078` and at the adapter boundary), so a far-future remote401+  value cannot permanently pin against honest local edits (`HV-1`). Remote402+  password replacements and deletes of live entries raise durable, user-visible403+  alerts (`vault.ts:2052-2097`).404+405+- **Sync resource bounds are layered.** Oversized frames are refused at the406+  WebSocket `onmessage` before buffering (`chirp-sync-transport.ts:70-79`), again407+  before `JSON.parse` in the coordinator (`chirp-sync-coordinator.ts:283-285`),408+  and the adapter caps entry/tombstone counts and per-entry plaintext size before409+  and after decryption (`chirp-sync-vault-adapter.ts:133-152,167`, `MV-3`). The410+  subnet probe is rate-limited to once per 60 s per peer pubkey411+  (`chirp-sync-transport.ts:208-251`).412+413+- **Vouched devices require explicit confirmation.** A signed extension manifest414+  can introduce a never-seen device, but it is queued in a pending-vouched store415+  for user Allow/Reject rather than becoming a sync target on the signature alone416+  (`chirp-sync-vault-adapter.ts:270-287`, `MV-7`); a key change under a stable417+  device id raises a durable alert (`MV-8d`).418+419+- **Codex generation verifies itself end-to-end.** Generation gates each stage420+  (compression round-trip `generate.ts:341-350`, per-stripe erasure simulation421+  `359-410`, full reconstruction with the maximum tolerated chunk loss422+  `412-468`) and aborts with a typed `CodexError` on any mismatch — appropriate423+  for a disaster-recovery artifact where silent corruption is the worst outcome.424+425+- **Codex restore treats every scanned field as attacker-controlled.** The426+  stripe count `S`, within-stripe total `t`, chunk index `i`, and declared427+  fingerprint length are all validated/bounded before they can drive allocation428+  or pass a check (`restore.ts:128-173,354`,429+  `PCH-01-010/011/012/013`), and decompression is streamed with a hard output430+  ceiling (`generate.ts:320-339`, `MV-4`).431+432+- **PDF import is hardened in depth.** `pdfjs.getDocument` is called with433+  `isEvalSupported:false`, `disableFontFace`, `disableRange`, `disableStream`434+  (`pdf-import.ts:303-312`, `MV-9`, defending the CVE-2024-4367 font/eval class),435+  with file-size, page-count, and rendered-dimension caps436+  (`pdf-import.ts:119-126,238,273,327`, `PCH-01-014`).437+438+- **A real Known-Answer self-test runs on every unlock.** `self-test.ts` pins the439+  expected AES-GCM ciphertext for a fixed vector and compares constant-time440+  (`83-90`), so a WebCrypto that is broken-but-self-consistent (would pass a bare441+  `decrypt(encrypt(x))===x`) is caught and the vault re-locks442+  (`messages.ts:304-311`).443+444+- **Import data is sanitised at the vault boundary, not just at the parser.**445+  Every restore/sync path funnels through `sanitizeRestoredEntry` →446+  `normalizeEntry` field whitelist with field-by-field `passkey`/`canary` rebuilds447+  and bounded tags (`vault.ts:1603-1741`, `MV-6`), so parser leniency in448+  `import.ts`/`backup.ts` cannot smuggle unexpected fields into the vault. (The449+  one path that skips this is `UPDATE_ENTRY` — see PCH-02-001.)450+451+- **Honest, specific in-code threat documentation.** The pairing trust-model452+  comment (`chirp-handshake.ts:125-154`), the per-entry-key replay rationale453+  (`chirp-sync-vault-adapter.ts:9-21`), the recovery-words 128-bit-prefix model,454+  and the `removePairedDeviceById` "removal is insufficient, rotate instead" note455+  (`paired-devices.ts:72-95`, `MV-8`) all describe residual risk accurately rather456+  than overselling the controls.457+458+- **Minimal, justified manifest.** Permissions are459+  `storage`, `unlimitedStorage`, `activeTab`, `scripting`, `clipboardWrite`,460+  `alarms`, `idle`, `favicon`, `webAuthenticationProxy`, `offscreen`; the broad461+  `tabs` permission and the ggwave web-accessible-resource were both removed462+  (`MV-10`), `audioCapture` is `optional_permissions`, and the CSP is463+  `script-src 'self' 'wasm-unsafe-eval'; object-src 'none'` with no `unsafe-inline`464+  or `unsafe-eval` (`manifest.json:55-75`). Content-script UI is mounted in closed465+  shadow roots throughout, page-synthesised keydowns are ignored via `e.isTrusted`466+  (`PCH-01-007`), and no production content code uses `innerHTML`.467+468+## Summary Table469+470+| ID | Title | Severity | Location |471+|---|---|---|---|472+| PCH-02-001 | Update-entry patch path bypasses canary/tags/passkey sanitizers | Low | `vault.ts:2317-2417`, `messages.ts:478-501` |473+| PCH-02-002 | Noise XX message 1 omits canonical empty-payload hash step | Informational | `noise-xx.ts:93-97,184-191` |474+| PCH-02-003 | Pairing auth rests on user SAS + unverifiable peer-side gate | Informational | `chirp-handshake.ts:125-216` |475+| PCH-02-004 | Crypto-adjacent deps caret-ranged vs. exact-pinned core | Informational | `package.json:31-52` |476+| PCH-02-005 | Legacy JSON import uses PBKDF2 rather than Argon2id | Informational | `backup.ts:146-173` |477+| PCH-02-006 | vault_id is stable, LAN-broadcast, cross-device-linkable | Informational | `sync-identity.ts:62-75` |478+| PCH-02-007 | HIBP check omits Add-Padding header | Informational | `vault.ts:216-238` |479+| PCH-02-008 | peachPayload BinaryReader lacks explicit bounds checks | Informational | `peachPayload.ts:940-969` |480+| PCH-02-009 | X25519 DH does not reject low-order/zero outputs | Informational | `noise-xx.ts`, `noise-js.ts` (DH call sites) |481+| PCH-02-010 | Clipboard auto-clear unconditional; parked hash unused for match | Informational | `clipboard.ts:98-178` |482+483+## Conclusions484+485+Peach is a carefully engineered, security-conscious password manager whose486+cryptographic core is standard and correctly parameterised, and whose extension487+trust boundaries are enforced with browser-attested origins, closed shadow-DOM UI,488+sender-gated destructive operations, and consistent secret minimisation across the489+content-script boundary. The vault layer handles the hard cases — atomic490+key-rotation writes, in-memory rollback on failed persists, monotonic timestamps,491+constant-time duress unlock — with evident care, and the higher-risk feature492+surfaces (LAN sync, Codex PDF import) are bounded against malformed and malicious493+input at multiple layers.494+495+The review found no Critical or High issues. The single Low finding is a496+sanitizer asymmetry on the update path that is meaningfully mitigated by the497+extension's messaging model and is a local fix. The remaining items are498+Informational: documented residual risks (the pairing SAS dependency on an499+unverifiable peer-side gate is the most important to track, because its weakest500+link lives in a repository this review cannot see), small defense-in-depth501+hardening opportunities (reject zero DH outputs, add HIBP padding, bound the502+binary reader explicitly), and supply-chain/KDF consistency notes.503+504+A technically literate reader should come away with high confidence in the505+correctness of the encryption-at-rest and autofill-boundary implementations, and506+with a short, concrete list of hardening steps rather than any structural concern.507+The pervasive, accurate in-code documentation of *why* each control exists is508+itself a quality signal: it makes the security model auditable from the source509+alone, which is exactly what a transparency-focused publication of this review510+should want to demonstrate. The two cross-repository dependencies — the Android511+Noise hand-port and the Android SAS gate — are the parts of the trust model that512+this review could not close from the browser source, and they are the right place513+to direct any follow-up verification.