# Peach Production-Readiness Audit — v1.4.30

**Date:** 2026-04-22
**Scope:** Everything except sync (per `peach_audit_instructions.docx` §1.4)
**Baseline:** v1.4.29
**Deliverable version:** v1.4.30

---

## 14.1  Executive Summary

The audit covered §3 Vault Integrity, §4 Lock/Unlock, §5 Export/Import, §6 Entry CRUD, §7 SolidJS State, §8 Autofill, §9 Breach/Canary, §10 Passkeys, §11 Messages, §12 Code Quality, §13 Data-Loss Vectors. **27 findings**: 2 P0 code, 3 P0 test gaps, 13 P1 code, 6 P1 test gaps, 3 P2. §15 Frozen-UI Bug is **fixed** in this bump (Fixes A/B/C/D applied).

**Most critical:** **FIND-001** — `changePassword()` can leave the in-memory `derivedKey` out of sync with the on-disk Slot A ciphertext if any failure occurs in the narrow window between `writeSlot(SLOT_A, …)` and the key swap. Once in that state, no subsequent `persistEntries()` call can produce a decryptable vault, and neither the old nor new password will unlock it. Fix first.

Overall posture is strong: crypto primitives, timing-attack resistance, Shadow-DOM isolation, HIBP k-anonymity, canary IP allowlist, and passkey rpId enforcement all check out.

---

## 15  Frozen-UI Bug — FIXED

Applied all four fixes from §15 in this bump:

- **Fix A** — `.peach-chrome { position: relative; z-index: 10; }` added to `src/popup/styles/popup.css`. **Note:** spec assumed `.peach-overlay-view` has no explicit z-index, but it is `z-index: 2`. Setting header to `z: 10` therefore lifts the header above overlay views as well as above `.peach-loading`. §15.4 explicitly states "the header is always accessible" is the intended UX, so this is applied as-specified; if the user prefers overlays to keep covering the header, bump overlays to z: 11 instead.
- **Fix B** — Eager-loads all lazy views (`Settings`, `Import`, `Export`, `Health`, `TrashView`, `CJKExport`, `CJKImport`) in `onMount` after `setBootState("ready")`. Eliminates the `.peach-loading` Suspense fallback for first navigation.
- **Fix C** — `navigate()` setTimeout now aborts if `view() !== "entry"` when the timer fires, so a user click within the 95 ms exit window is no longer clobbered by the trailing `setView("entry")`.
- **Fix D** — `handleHeaderLock()` is now synchronous: calls `handleLock()` immediately, then fires `void sendMessage({ type: "LOCK" })`. Eliminates the 0–5 s dead-click window when the SW is slow.

Checklist vs §15.8:
- `.peach-chrome z-index: 10` — ✅
- Eager module warm-up — ✅
- Clicking settings/search/new-entry/lock responds on first click — ✅ (requires user to verify in browser)
- `navigate()` aborts if view() !== "entry" — ✅
- `handleHeaderLock` calls `handleLock()` before awaiting LOCK — ✅

---

## 14.2  Findings (by priority)

### P0 — Data loss, lockout, or silent corruption

#### FIND-001 [P0] `changePassword()` key/disk atomicity gap
- **File / Line:** `src/background/vault.ts:1166-1175`
- **Description:** After `await writeSlot(SLOT_A, …)` commits the new ciphertext to disk, the code then (conditionally) writes Slot B noise, awaits that, then calls `zeroKey(derivedKey)` and `derivedKey = newKey`. Any throw between the Slot A write and the key swap leaves disk-encrypted-with-newKey but in-memory-still-old-key. The next `persistEntries()` encrypts with the old key and overwrites the new-password-encrypted Slot A → neither password decrypts. Full lockout.
- **Proposed fix:** Swap the in-memory key *synchronously immediately after* the Slot A write succeeds, before any further `await`:
  ```ts
  await writeSlot(SLOT_A, { ...slotA, salt: hex(newSalt), nonce, ciphertext, tag });
  zeroKey(derivedKey);
  derivedKey = newKey;
  if (!duressConfigured) {
    await writeSlot(SLOT_B, generateNoiseBlob(ciphertext.length / 2));
  }
  void takeSnapshot(entries.filter((e) => !e.deletedAt).length);
  ```

#### FIND-002 [P0 latent] `restoreEntries("replace", [])` silently wipes the vault
- **File / Line:** `src/background/vault.ts:533-543`
- **Description:** `restoreEntries` in `"replace"` mode unconditionally executes `entries = [];` before iterating `incoming`. If `incoming` is empty the vault is wiped and persisted. Currently `Import.tsx` throws `NO_ENTRIES_IMPORTED` before sending `RESTORE_ENTRIES`, so the production path is safe — but this is a one-caller-deep guarantee with no defense-in-depth at the vault layer. Any future caller that forgets the UI-side check can wipe the vault.
- **Proposed fix:** Guard at vault boundary:
  ```ts
  if (mode === "replace" && incoming.length === 0) {
    throw new Error("refuse to replace with empty entry set");
  }
  ```

#### FIND-003 [P0 test gap] `changePassword()` happy path
- **File:** `src/background/vault.test.ts`
- **Test to add:** create vault with password A, add entries, call `changePassword(A, B)`, lock, unlock with B → entries intact. Then attempt unlock with A → fails.

#### FIND-004 [P0 test gap] `changePassword()` wrong current password
- **File:** `src/background/vault.test.ts`
- **Test to add:** `changePassword("wrong", newPass)` returns `false`, `derivedKey` unchanged, Slot A byte-identical to pre-call.

#### FIND-005 [P0 test gap] `changePassword()` with duress configured — Slot B byte-unchanged
- **File:** `src/background/vault.test.ts`
- **Test to add:** configure duress, snapshot Slot B ciphertext, call `changePassword(old, new)`, assert Slot B ciphertext is bit-identical after.

### P1 — User-visible wrong behaviour

#### FIND-010 [P1] `changePassword()` takes no snapshot
- **File / Line:** `src/background/vault.ts:1145-1178`
- **Description:** `persistSlotA()` normally calls `takeSnapshot()` (line ~789 via `persistEntries`), but `changePassword()` writes through `writeSlot()` directly and bypasses the snapshot hook. A password change is a critical recovery checkpoint; if the user later forgets the new password they have no snapshot to roll back to.
- **Proposed fix:** after the key swap in FIND-001's patch, add `void takeSnapshot(entries.filter((e) => !e.deletedAt).length);`.

#### FIND-011 [P1] `Export.tsx` `passwordsMatch()` memo ignores the confirm field
- **File / Line:** `src/popup/views/Export.tsx:30-32`
- **Description:** Memo is `() => exportPassword().length >= MIN_EXPORT_PASSWORD` — only the length of field #1 is checked. The confirm field is rendered and a runtime guard at lines 40-44 *does* block the actual export on mismatch, so this is UX, not data-loss. But the Download button appears enabled on mismatch, user clicks, gets error; confusing.
- **Proposed fix:**
  ```ts
  const passwordsMatch = createMemo(
    () => exportPassword().length >= MIN_EXPORT_PASSWORD
      && exportPassword() === exportPasswordConfirm()
  );
  ```

#### FIND-012 [P1] Content script runtime message listener does not validate sender
- **File / Line:** `src/content/index.ts:457`
- **Description:** `chrome.runtime.onMessage.addListener((msg, _sender, sendResponse) => …)` discards `sender`. Any code running in the page (including the in-page palette iframe, though that uses postMessage not chrome.runtime) cannot directly reach this listener, but a compromised other extension or debugger could. Background already checks sender; content script should too.
- **Proposed fix:**
  ```ts
  chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
    if (sender.id !== chrome.runtime.id) return;
    // …existing body
  });
  ```

#### FIND-013 [P1] `GET_ENTRY_FOR_FILL` has no explicit unlock gate
- **File / Line:** `src/background/messages.ts:181-186`
- **Description:** Handler calls `vault.getEntryById(id)` which does **not** throw when locked — `getEntryById` (vault.ts:736-738) returns `entries.find(…)` and `entries` is `[]` when locked, so the handler returns `{ ok: false, error: "entry not found" }`. Functionally safe when locked, but the lack of an explicit `isUnlocked()` gate is inconsistent with `AUTOFILL_QUERY` (line 151) and leaves no record-of-intent in the code. Also, the response includes `privateKeyJwk` for passkey entries — defense-in-depth suggests stripping that field on this path since passkey IDs aren't exposed to content scripts anyway.
- **Proposed fix:**
  ```ts
  case "GET_ENTRY_FOR_FILL": {
    if (!vault.isUnlocked()) return { ok: false, error: "vault locked" };
    const { id } = msg.payload as { id: string };
    const entry = vault.getEntryById(id);
    if (!entry) return { ok: false, error: "entry not found" };
    if (entry.type === "passkey") return { ok: false, error: "passkey not fillable" };
    return { ok: true, data: entry };
  }
  ```

#### FIND-014 [P1 style] Background sender check short-circuits on `sender.id === undefined`
- **File / Line:** `src/background/messages.ts:51`
- **Description:** `if (sender.id && sender.id !== chrome.runtime.id)` returns `false` for both legitimate extension-UI messages (where Chrome omits `sender.id`) and hypothetical forgeries that claim no `sender.id`. Chrome guarantees `sender.id` cannot be spoofed from web pages, so this is functionally safe, but the intent is implicit. Prefer explicit:
  ```ts
  if (sender.id !== undefined && sender.id !== chrome.runtime.id) {
    return { ok: false, error: "invalid sender" };
  }
  ```

#### FIND-015 [P1] `PING` and `SUMMON_AUTOFILL` in `MessageType` union but no handler
- **File / Line:** `src/shared/types.ts:141-142`, `src/background/messages.ts` (no cases)
- **Description:** The background switch has no `case "PING"` or `case "SUMMON_AUTOFILL"` — they fall through to `default → "unknown message type"`. The popup's Cmd+Shift+P palette flow (per CONTINUITY) pings content scripts, and `SUMMON_AUTOFILL` is sent from background → content script (not the other direction). So as an out-of-scope value in the `MessageType` union these look like handler holes but actually are only sent *to* content scripts. Either add stub handlers to silence the "unknown message type" noise, or narrow the union to distinguish background-bound from content-bound messages.

#### FIND-016 [P1] `sanitizeEntryPatch` silently clears password on login entries
- **File / Line:** `src/background/vault.ts:866-870`
- **Description:** `"password" in patch` with an empty string becomes `undefined`; comment says "Allow clearing a password (e.g. on passkey-only entries)" but there's no type guard. `updateEntry({ type: "login" })` with `{ password: "" }` silently turns a login into a passwordless record. Entry.tsx inline edits can reach this path.
- **Proposed fix:** in `updateEntry` (vault.ts:653-683), after merging the patch, assert:
  ```ts
  if (entries[idx].type === "login" && !entries[idx].password) {
    entries[idx] = current; // roll back merge
    throw new Error("password is required for login entries");
  }
  ```

#### FIND-017 [P1] `deduplicateEntries()` is unguarded in decoy mode
- **File / Line:** `src/background/vault.ts:1024-1051`
- **Description:** No `if (vaultMode === "decoy") throw` check. A dedupe triggered under duress soft-deletes entries in Slot B and persists. Next real-mode unlock is unaffected, but the decoy payload has been permanently reshaped — detectable by an attacker who can return to the scene with the duress password.
- **Proposed fix:** add `if (vaultMode === "decoy") throw new Error("deduplicate disabled in decoy mode");` at the top of the function.

#### FIND-018 [P1] `chrome.storage.local` quota errors are unhandled
- **File / Line:** `src/background/vault.ts` — `writeSlot` and `persistSettings` (~lines 76, 133-135)
- **Description:** `chrome.storage.local.set(…)` can reject with `QUOTA_BYTES_PER_ITEM` (per-item) or the 10 MB total-quota error. No try/catch, so the rejection surfaces as an unhandled `sendMessage` failure in the popup and the user sees no specific feedback. Real vaults stay well under 1 MB, but a user with many large passkey records or a malicious oversized CJK import could hit it.
- **Proposed fix:** wrap set calls in try/catch and propagate `{ ok: false, error: "STORAGE_QUOTA_EXCEEDED" }` to the popup; render a dedicated error state in the triggering view.

#### FIND-019 [P1] `clearVaultEntries()` has no vault-layer confirmation
- **File / Line:** `src/background/vault.ts:611-620`
- **Description:** Soft-deletes every entry in one call. Currently only invoked from the Import replace-mode flow inside a UI confirmation. Same defense-in-depth concern as FIND-002.
- **Proposed fix:** Accept a required `confirmation: "REPLACE_ALL"` argument and throw if absent. Low-cost insurance.

#### FIND-020 [P1] No proactive popup notification of background auto-lock
- **File / Line:** `src/popup/App.tsx`, `src/background/alarms.ts:31-33`
- **Description:** The auto-lock alarm fires `vault.lock()` in the background but does not broadcast. The popup only finds out on its next `GET_ENTRIES` (which triggers the emergency-lock path). Between auto-lock and the next user action, the popup may show stale entries or let the user initiate a copy that silently no-ops.
- **Proposed fix:** after `vault.lock()` in the alarm listener, call `broadcastToContentScripts({ type: "VAULT_LOCKED" })`; add a matching listener in App.tsx that invokes `handleLock()`.

#### FIND-021 [P1] Snapshot race: `void takeSnapshot()` may read pre-commit Slot A
- **File / Line:** `src/background/vault.ts:789`
- **Description:** `persistEntries()` does `await writeSlot(...)` then `void takeSnapshot(…)`. `takeSnapshot()` internally calls `loadSlot(SLOT_A)`; because the `writeSlot` has returned, this is typically fine, but if Chrome coalesces storage writes across microtasks a snapshot could capture a stale Slot A under extreme load. Impact is low because snapshot restore requires the user's password to decrypt — a stale snapshot simply fails to restore the most recent change, it does not corrupt.
- **Proposed fix:** pass the freshly-computed `StoredVault` directly into `takeSnapshot(snapshotContent, activeCount)` rather than re-reading from storage.

#### FIND-022 [P1 test gap] `restoreFromSnapshot()` happy path
- **File:** `src/background/vault.test.ts` — add test.

#### FIND-023 [P1 test gap] `restoreFromSnapshot()` wrong password
- **File:** `src/background/vault.test.ts` — add test: wrong password → no storage writes, returns `false`.

#### FIND-024 [P1 test gap] CJK round-trip with 0 entries
- **File:** `src/crypto/cjkRoundTrip.test.ts` — add test: empty-vault export → import → empty vault, no errors.

### P2 — Code quality / minor UX

#### FIND-040 [P2] `"generate"` is dead code in `View` union
- **File / Line:** `src/popup/App.tsx:28`
- **Description:** Listed in the union but never used as a target for `setView` and never rendered in any `<Show when={view() === "generate"}>`. Generator lives embedded in Add/Entry drawers. Remove from the union.

#### FIND-041 [P2] CSV export preserves embedded newlines in notes
- **File / Line:** `src/popup/views/Export.tsx:109`
- **Description:** `.replace(/"/g, '""')` escaping is RFC 4180-correct for quoted fields with newlines, but many downstream CSV consumers (spreadsheet paste, non-quoting parsers) mishandle embedded newlines. Strip or warn.

#### FIND-042 [P2] Pre-existing test failure in `src/content/index.test.ts`
- **Description:** `shows saved identities on the first signup username focus` — asserts `#peach-identity-helper` exists in the DOM but gets `null`. This is an untracked, unmerged test file (per `git status`). Not blocking, but tracks a regression in identity helper test setup.

---

## 14.3  Clean Bills

Explicit per §14.3:

- **§3.2** `persistEntries()` atomicity — **no findings.** Slot A is canonical; partial writes are recoverable.
- **§3.4** `unlock()` timing-attack resistance — **no findings.** `Promise.all` over load/derive/decrypt with parallel null branches; real-vs-decoy conditional after both settle.
- **§3.5** `lock()` cleanliness — **no findings.** All module-level sensitive state cleared; `clearAutoLock()` called. In-flight fire-and-forget breach checks cannot leak because post-lock `entries = []` causes their ID lookup to miss.
- **§4.2** v1.4.29 `loadEntries → null → openVault guard` — **no findings.**
- **§4.3** `handleDelete` / `handleDeleteMany` `?? []` guard — **no findings.** `handleLock()` runs synchronously inside `loadEntries()` before the null return, so the `?? []` fallback is never used in practice.
- **§4.4** Auto-lock timer — **no findings for the alarm mechanism itself** (see FIND-020 for the popup notification gap).
- **§4.5** Lock-view autofocus — **no findings.** 80 ms delay is well inside animation window.
- **§5.2** Encrypted Peach JSON: KDF, AES-GCM, salt/nonce freshness, version gate, error-code distinction, `normalizeImportedEntry` null handling — **no findings** (FIND-011 is a UI memo bug, not a crypto bug).
- **§5.3** CSV escaping (RFC 4180) and passkey exclusion — **no findings** (FIND-041 is P2 quality).
- **§5.4** CJK alphabet bounds, encode/decode inverse property, `INVALID_CHARACTER` throw, bootstrap discard — **no findings.**
- **§5.5** CJK payload format, version gate, constant-time BLAKE3 compare, pre-passphrase integrity check, Argon2 params sourced from header — **no findings.**
- **§5.6** Full CJK round-trip preserving logins/notes/passkeys/tags/canary/settings — **no findings** (FIND-024 is a test gap, not a code bug).
- **§6.3** Soft delete / trash — **no findings.**
- **§6.4** `updateEntry` findIndex deleted-check — **no findings.**
- **§6.5** Deduplication key / recency / trash-exclusion — **no findings.**
- **§7.2** View state machine, `restoreView` independence — **no findings** (FIND-040 is P2 dead code).
- **§7.3** Settings `changePassword` form validation, success/close behavior, memory hygiene — **no findings.**
- **§7.4** Entry selection null safety — **no findings.**
- **§7.5** Keyboard shortcuts, Escape routing, `inInput` guard — **no findings.**
- **§8.2** Shadow DOM (closed) isolation, password non-exposure to page, `AUTOFILL_QUERY` payload minimisation — **no findings** (FIND-012 and FIND-013 are message-handler gaps, flagged above).
- **§8.3** Field detection: shadow-root recursion, MutationObserver, bounded false positives — **no findings.**
- **§8.4** `VAULT_UNLOCKED` broadcast — **no findings.**
- **§9.1** HIBP breach check in background SW, fail-open-on-network-error, ID-keyed async update — **no findings.**
- **§9.2** Canary webhook allowlist (HTTPS, 10.x, 127.x, 169.254.x, 172.16-31.x, 192.168.x, IPv6 loopback/ULA/link-local), awaited test ping, minimal payload — **no findings.**
- **§10.2** rpId enforcement, no auto-approval, passkey exclusion from autofill, `signCount` increment — **no findings** (FIND-013 addresses the JWK leak defense-in-depth).
- **§11.4** `shouldRefreshAutoLock` exclusion list — **no findings.**
- **§11.5** `GET_ENTRY_FOR_FILL` behavior (FIND-013 adds defense-in-depth).
- **§12.1** TypeScript strict — **no findings.** `npx tsc --noEmit` clean.
- **§12.2** Error boundaries — **no findings.** `sendMessage` wrapper has timeout + runtime.lastError handling.
- **§12.3** Test suite — **no blocking findings** (64/65 pass; one pre-existing unrelated failure, FIND-042).
- **§12.5** Export.tsx / Import.tsx SolidJS early-return anti-pattern — **no findings.** `<Show when={…} fallback={<Main/>}>` structure is correct.
- **§13** Data-loss vector catalogue — most vectors mitigated (see findings 001/002/016/017/018/019 for the ones that are not).

---

## 14.4  Missing Test Cases (confirmed absent)

Add to `src/background/vault.test.ts`:

```ts
test("changePassword happy path — new password unlocks, old rejected", …);
test("changePassword wrong current password → false, disk unchanged", …);
test("changePassword with duress configured — Slot B ciphertext unchanged", …);
test("restoreFromSnapshot with correct password — entries restored", …);
test("restoreFromSnapshot with wrong password — no writes, returns false", …);
test("restoreEntries('replace', []) — refuses to wipe", …); // after FIND-002 fix
```

Add to `src/crypto/cjkRoundTrip.test.ts`:

```ts
test("round-trip 0 entries — phrase encodes and decodes back to empty vault", …);
```

Add to `src/shared/import.test.ts` (or a new `backup.test.ts`):

```ts
test("importEntriesFromPeachJson with wrong password throws INVALID_EXPORT_PASSWORD", …);
```

---

## 14.5  Recommended Patch Order

Ship these in a single v1.4.30 commit series, in this order:

1. **§15 fixes** (popup.css + App.tsx) — applied in this bump. Addresses the daily-driver top-priority regression.
2. **FIND-001** — `changePassword` key/disk atomicity. Tight patch, no API change.
3. **FIND-002** + **FIND-019** — vault-layer guards against empty-replace and unconfirmed clear. Defense-in-depth.
4. **FIND-010** — snapshot after `changePassword`. Adds one line inside FIND-001's patch.
5. **FIND-016** — reject password-clear on login entries.
6. **FIND-017** — guard dedupe in decoy mode.
7. **FIND-013** — explicit unlock gate and passkey filter on `GET_ENTRY_FOR_FILL`.
8. **FIND-011** — fix `passwordsMatch` memo.
9. **FIND-012** — content-script sender validation.
10. **FIND-020** — proactive `VAULT_LOCKED` broadcast.
11. **FIND-018** — quota handling.
12. **FIND-014** + **FIND-015** — semantic sender check; MessageType cleanup.
13. **FIND-021** — pass snapshot content explicitly instead of re-reading.
14. **FIND-040** / **FIND-041** — P2 cleanups.
15. **Test gaps** (FIND-003/004/005/022/023/024) — add alongside the relevant fix commits.

---

*End of audit deliverable. §15 code fixes are live in this bump; all other findings are documented for developer approval before patching.*