mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-27 07:52:03 +00:00
docs: record the L-4..L-8 remediation, and what was deliberately not changed
The audit report gains a remediation table and per-finding status; TODO gains the same detail where the rest of the work is tracked. Two entries are closed as "resolved" rather than "fixed", and say why: L-5's unanchored label_regex matches Python's re.search and anchoring it in one implementation only would silently narrow every token already issued, and L-6's argv-level containment needs a token-format change in both implementations. Recording the reasoning matters more than the checkbox — a future reader finding an unanchored regex should find the decision, not rediscover the finding. Also noted as still unported rather than silently skipped: restore_latest_backup, nostr_max_retries and nostr_retry_delay. M-1, L-1, L-2 and L-3 remain open; all four are protocol-level and need a matching change in Python. No GPT-family review has run yet, so the independent-review blocker is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
This commit is contained in:
1 parent
8653ee8006
commit
b209bb45c8
2 files changed
+75
-14
No files matched your search
@@ -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 `<profile>/backups/entries_db_backup_<ts>.json.enc`
|
||||
at 0600 and mirrors it to the configured second location as
|
||||
`<fingerprint>_<name>`, 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
|
||||
|
||||
|
||||
+29
-5
@@ -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.
|
||||
Reference in new issue
Block a user