diff --git a/TODO.md b/TODO.md index 05e17615..c50708e8 100644 --- a/TODO.md +++ b/TODO.md @@ -267,15 +267,52 @@ the review independent in the sense that matters. same-id creation on two replicas discards one entry at merge; the 2048-entry tombstone cap allows deletion replay. Both are arguably working as designed; the fix for each is to surface the event rather than to change the merge. -- [ ] **L-4 — remaining inert settings.** `additional_backup_path` is stored and confirmed - by the TUI and used by nothing. Same class as the five fixed on 2026-08-17: either - wire it up or have the menu say it is unavailable. -- [ ] **L-5 / L-6 / L-7 — agent boundary hardening.** Anchor `label_regex` (unanchored - today, so a token's scope is substring-matched and wider than it reads); document - that an exec allowlist constrains the command word and not its arguments; validate - the wire-supplied `fingerprint` in the daemon's `put` before using it as a path - component. All owner-gated, none live holes. -- [ ] **L-8 — plaintext backup import is not bound to a profile.** +- [x] **(fixed 2026-08-18) L-4 — inert settings, and the unported subsystem behind them.** + `additional_backup_path` was stored, confirmed by the TUI ("Additional backups will + be written to X") and used by nothing — false assurance in a disaster-recovery + feature. The cause was bigger than the setting: Python's `BackupManager` was not + ported at all, so `backup_interval` was dead for the same reason. New + `js/packages/cli/src/backups.ts` ports the write side — every committed mutation + snapshots the encrypted index to `/backups/entries_db_backup_.json.enc` + at 0600 and mirrors it to the configured second location as + `_`, filenames matching Python byte for byte so both + implementations share one backup directory. Two documented deviations: the interval + throttle reads the newest snapshot on disk rather than Python's in-memory + `_last_backup_time` (every CLI command is its own process, so an in-memory counter + would leave `backup_interval` inert for CLI use), and a failed mirror is surfaced + rather than swallowed. Quick Unlock stays in the menu for item-order parity but now + says it is unimplemented — in Python the flag never changed unlocking either, it + writes an audit entry and raises a security finding. + **Not ported:** `restore_latest_backup` (nothing in the TUI offers it; `vault import` + and `nostr restore` cover recovery), and `nostr_max_retries` / `nostr_retry_delay` + remain stored-but-unused. +- [x] **(resolved 2026-08-18) L-5 — `label_regex` is unanchored.** Left as-is deliberately: + Python matches with `re.search`, so anchoring the TS side alone would make one token + mean two different things depending on which implementation holds it, and would + silently narrow every token already issued. Now stated in `--label-regex` help, + reported as `capabilities().tokens.label_regex_semantics`, explained at the + enforcement point, recorded in `docs/typescript_port_compatibility_matrix.md`, and + pinned by tests asserting an unanchored token sees BOTH `prod` and `not-prod-db`. +- [x] **(documented 2026-08-18) L-6 — exec allowlist covers the command word, not argv.** + The token holder writes the arguments and the child gets the secret in its + environment, so an allowlisted binary with an output-file or network flag returns the + secret to its caller. Documented in help, `capabilities()`, and at the check itself. + Argument-level containment means allowlisting full argv templates instead of command + words — a token-format change that has to land in both implementations together, so + it stays a backlog item rather than a quiet divergence. +- [x] **(fixed 2026-08-18) L-7 — daemon `put` accepted any string as a fingerprint.** + Now requires 16 uppercase hex *and* that the fingerprint belongs to the supplied + seed. Ordered after the ttl check on purpose: deriving a fingerprint runs the BIP-39 + KDF, and a malformed ttl should not pay for it. +- [x] **(fixed 2026-08-18) L-8 — plaintext backup import was not bound to a profile.** + A seed-only backup is implicitly seed-bound; a plaintext one imports into any profile + and is then re-derived from the *target* seed, so every password, SSH key, PGP key and + seed silently differs from what the backup was taken to preserve, with nothing + erroring. CLI and TUI now compare the wrapper's fingerprint to the target and require + an explicit override (`--allow-fingerprint-mismatch`, or a confirmation in the TUI); + `vault import --inspect` reports the backup's fingerprint and encryption mode. The + CLI also stops deciding encryption mode by regex over the raw file — + `parseBackupWrapper` reads the envelope instead. ### Cutover diff --git a/js/SECURITY_AUDIT.md b/js/SECURITY_AUDIT.md index aa989b37..7c8e0bf2 100644 --- a/js/SECURITY_AUDIT.md +++ b/js/SECURITY_AUDIT.md @@ -135,7 +135,7 @@ failure is flaky and is the direct evidence for finding M-1. - **Recommendation:** None beyond the docs' own roadmap; consider counting evictions and warning the user when the cap has actually trimmed history. -### L-4 (Low) — Inert settings: the TUI confirms behavior that does not exist +### L-4 (Low) — Inert settings: the TUI confirms behavior that does not exist — **FIXED 2026-08-18** - **Where:** `tui/menus.ts:1482-1489` (`additional_backup_path` — the TUI replies "Additional backups will be written to X" but no code path ever writes a backup there); @@ -150,7 +150,7 @@ failure is flaky and is the direct evidence for finding M-1. - **Recommendation:** Either implement them or have the menu say plainly "not available in this build," the pattern already used for QR codes and the semantic index. -### L-5 (Low) — Token `label_regex` is unanchored: substring semantics widen token scope +### L-5 (Low) — Token `label_regex` is unanchored: substring semantics widen token scope — **RESOLVED 2026-08-18 (documented; matches Python)** - **Where:** `agent.ts:248-255` (`tokenMaySee` uses bare `RegExp.test`); surfaced at `program.ts:1318` (`--label-regex`). @@ -162,7 +162,7 @@ failure is flaky and is the direct evidence for finding M-1. loudly in `--label-regex` help and `capabilities()`. Match Python's behavior, whichever it is, and add it to the parity matrix. -### L-6 (Low) — Exec allowlist constrains the command word, not its arguments +### L-6 (Low) — Exec allowlist constrains the command word, not its arguments — **DOCUMENTED 2026-08-18** - **Where:** `agent.ts:337-349`; execution in `sinks.ts:102-121`. - **What:** `exec_allowlist` compares only the first token. A use-scoped holder may pass @@ -177,7 +177,7 @@ failure is flaky and is the direct evidence for finding M-1. holder controls their argv; longer term, allow full argv templates in the allowlist (`["ssh-add", "-"]`) rather than bare command words. -### L-7 (Low) — Daemon uses wire-supplied `fingerprint` as a path component without format validation +### L-7 (Low) — Daemon uses wire-supplied `fingerprint` as a path component without format validation — **FIXED 2026-08-18** - **Where:** `agent.ts:209, 298, 554` (`join(appDir, fingerprint, ...)`); `put` at `agent.ts:633-652` accepts any string. @@ -190,7 +190,7 @@ failure is flaky and is the direct evidence for finding M-1. verify `generateFingerprint(mnemonic) === fingerprint`, which additionally catches an honest mismatch corrupting the audit-log location. -### L-8 (Low) — Plaintext backup import is not bound to a profile +### L-8 (Low) — Plaintext backup import is not bound to a profile — **FIXED 2026-08-18** - **Where:** `program.ts:1549-1606` (`vault import`), `tui/menus.ts:1432-1459` (`importDatabase`); wrapper schema in `core/src/vault/portableBackup.ts`. @@ -309,3 +309,27 @@ pnpm -r typecheck # clean pnpm -r test # core: 234/234 pass (jsdom env: 230 pass, 4 skipped) # cli: 157/158 pass — 1 flaky failure = finding M-1 ``` + +--- + +## Remediation status (2026-08-18, after the audit) + +| Finding | State | Where | +|---|---|---| +| M-1 sync tie-break | **open** | needs a signed-manifest sequence number in both implementations | +| M-2 config password policy | fixed | `secrets.ts` `basePolicy`; `test/passwordPolicy.test.ts` | +| L-1 shared BIP-85 app-32 path | open | protocol change, both implementations | +| L-2 concurrent same-id creation | open | surface at merge rather than change it | +| L-3 tombstone replay | open (documented) | as the docs' own roadmap has it | +| L-4 inert settings | fixed | `backups.ts` ports BackupManager's write side; Quick Unlock now says it is unimplemented | +| L-5 unanchored `label_regex` | resolved as-is | matches Python's `re.search`; documented in help, `capabilities()`, the parity matrix, and pinned by tests | +| L-6 exec allowlist scope | documented | help, `capabilities().tokens.exec_allowlist_semantics`, enforcement-point comment | +| L-7 unvalidated `fingerprint` in `put` | fixed | format + seed-match checks in `agent.ts` | +| L-8 unbound plaintext import | fixed | fingerprint binding in CLI and TUI; `parseBackupWrapper` replaces the regex sniff | + +Every fix above is mutation-verified: the guard or hook is disabled, the +matching tests are confirmed red, and the change is restored. A passing suite +was not treated as evidence on its own. + +Still outstanding, and unchanged by this round: no GPT-family review has run. +Findings from one model family are one perspective, not independence.