mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-10-29 08:52:04 +00:00
7b5730910ea2ee5146f7fb121dded10cf61c8f4e
98
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  thePR0M3TH3ANandClaude Opus 5
|
7b5730910e |
Write the oversized request with backpressure, so the refusal can be read
The daemon fix landed but the test still failed on macOS and Windows, and the remaining fault was the client. It blasted a megabyte in a single write, so the moment the daemon capped the line and hung up, the write raised EPIPE — and Node destroys a socket on error, discarding the refusal that was already inbound. Ignoring the error was not enough: by then the socket was gone. It now writes in 64KB chunks and honours the return value of `write`, waiting for `drain` rather than pushing into a full buffer, and stops as soon as a reply arrives. That is what a real client does, and it leaves room for `data` to fire between chunks. Worth stating plainly: the daemon was right the whole way through this. It capped the line and refused, on every platform. Two layers of test artefact — treating a client-side EPIPE as fatal, then writing without backpressure — made correct behaviour look broken on the two platforms nobody had run. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
73ac93a9f1 |
Deliver the oversize refusal before hanging up, and pin a POSIX-only test
Two left from the platform run, one real.
THE DAEMON HUNG UP WITHOUT SAYING WHY, ON macOS ONLY
The line-length cap did `socket.write(refusal)` and then `socket.destroy()`.
destroy() tears the socket down immediately and discards whatever is still
buffered — on Linux the write usually reaches the kernel first and the peer
sees "request too large", on macOS it does not and the peer sees an
unexplained close. The refusal itself worked on both; only the reason was
lost, and only on one platform, which is why nothing noticed until this suite
ran there.
Now `end(payload, cb)` and destroy from the callback, so the bytes are
flushed before teardown, plus a `refused` flag so a peer still pushing the
rest of its megabyte is ignored rather than re-buffered.
The test that caught it was itself half the problem: it treated its own EPIPE
as fatal, when a client's broken pipe is the expected consequence of a server
correctly hanging up mid-write. Fixed in the previous commit; this one fixes
what that then exposed.
AND ONE TEST ASSERTING POSIX BEHAVIOUR WITHOUT SAYING SO
`honours backslash escapes outside single quotes` called parseCommandSpec with
no platform flag, so it read process.platform and asserted POSIX semantics on
Windows — where backslash is a path separator and deliberately is not an
escape. It now passes `{ windows: false }`, because that is the branch it is
about.
Windows went 15 failures to 1 to 0; macOS 2 to 1 to 0.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
e2e9aa851e |
Fix what three platforms found: two shipped bugs, four unportable tests
The platform workflow's first run produced seventeen failures on Windows and macOS, where the JavaScript suite had never executed. Two were real defects in shipped code. A CONTAINMENT CHECK THAT REFUSED EVERYTHING ON WINDOWS `resolveWithinProfile` compared `target.startsWith(root + "/")` with a hardcoded forward slash. On Windows `resolve()` returns `C:\profile\file.txt` and the comparison asked for `C:\profile/`, so it matched nothing: document export and import refused every path, including legitimate ones. It failed CLOSED, so nothing leaked — the feature simply did not work. Now uses `relative()`, which needs no separator of our choosing. A COMMAND PARSER THAT ATE WINDOWS PATHS `parseCommandSpec` treated backslash as a POSIX escape unconditionally, so `--exec "C:\Program Files\tool.exe"` was spawned as the command `C:Program` with an argument `Filestool.exe`. Every Windows path handed to --exec or --stdin-to was mangled before the spawn. This is the SECOND instance of this exact bug today — the Python TUI palette had it too — which is what a whole class looks like rather than a one-off. `windows` is an explicit parameter, not a `process.platform` read, so both branches are tested from either platform. That parameter is the actual fix for the class: the reason these survived is that the Windows branch could only ever run on Windows. THE REST WERE TESTS, NOT CODE - `/bin/true` does not exist on macOS (it is /usr/bin/true), so a spawn failed with ENOENT and the refusal looked like the daemon's crash-safety fix having regressed. Now uses process.execPath, which exists everywhere by definition. - The oversized-request test treated its own EPIPE as fatal. The daemon caps the line, answers and hangs up; on macOS that hangup lands while the client is still pushing the remaining megabyte. Correct server behaviour was failing the test on one platform and passing on the other. - Three sink tests wrote `#!/bin/sh` files. Shebangs and /bin/sh do not exist on Windows. Now Node scripts run by the interpreter already running the tests. - Eight `0600` assertions and two clipboard tests are guarded by documented constants in test/helpers/platform.ts. Windows has no POSIX mode bits and the runner has no clipboard. The skips state a gap rather than closing it, and TODO.md now records that the Windows permissions question grew from two Python tests to ten across both languages — including the vault index and the semantic index, which hold secrets. With TypeScript becoming the only implementation that is a shipping question, not a test-suite one. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
4edf50e5de |
Attest the bundle on three platforms; give agents two gates with meanings
Adds ts-platform.yml and finishes the local-CI split. THE DIVISION OF LABOUR Properties of the CODE — cross-implementation agreement, fixture drift, differential fuzzing — are asked once, on Linux, because asking them three times measures nothing extra. ts-platform asks the questions where the platform IS the variable: does the bundle build, lint, test, run and install on a machine that is not the one we develop on. That is not theoretical. Every CI failure today that survived local checking was a platform difference: MSYS2 shadowing `python` on PATH, pwsh discarding a non-final exit code, portalocker needing pywin32, shlex eating the backslashes out of Windows paths in the TUI. None are visible from Linux, and all of them would have reached users' machines rather than ours. With TypeScript becoming the only implementation, the bundle working on Windows stops being a nice-to-have. The workflow sets `shell: bash` for every step, including Windows, for the reason found today: under pwsh only the last command in a multi-line block decides whether a step failed. ts-parity's inline bash smoke test is replaced by the Node one, so a single smoke test now runs locally, in parity, and on all three platforms — and it checks that revealing an entry twice gives the same secret, rather than that some output appeared. REPRODUCIBLE BUILDS, AS AN EXPERIMENT A second job compares the bundle's SHA-256 across the three platforms, marked continue-on-error. Two builds on one machine are already byte-identical, so the cross-platform question is real rather than a fantasy. If the three agree that is an invariant worth promoting to required; if they do not, the interesting part is why, and neither answer is worth blocking a merge over before we know which. TWO GATES WITH MEANINGS AGENTS.md now says an agent may not report a task complete or push until ci_prepush passes, and may not call a branch merge-ready until ci_parity passes — plus the two rules that make those gates trustworthy: invoke tools through the repository rather than off PATH, and never add a check that cannot fail. Both rules are written with the evidence from today attached, because a rule with its reason is one people keep. Verified by running ci_parity end to end before this commit: 689 JavaScript tests, 1132 Python tests, cross-implementation 41/41, fixtures clean, three fuzz seeds with no divergences across 1050 compared cases each. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
72b3e52f50 |
Delete 38 assertions that were never necessary, and the bug one was hiding
I expected the 35 remaining findings to need a VaultIndex remodel. They did not. `vaultIndexSchema` and every entry schema are `.loose()`, which already gives both types an index signature — so `index["_system"]`, `entry["label"]` and assignment to `Record<string, unknown>` all compile with no cast at all. I checked before rewriting anything, and the answer was that 38 of the 44 were simply redundant, copied from each other rather than needed. That is the linter's real value here: it did not find a design flaw, it made me test an assumption I would otherwise have acted on. The remodel would have been work done to satisfy a belief that was false. Removing them let the compiler see three things the casts had been hiding: - `nextTotpIndex` filtered on `type === "totp" || kind === "totp"`, and TS reported the second half as unreachable. It is: withKind() populates kind from type at parse time and the union is discriminated on kind, so the fallback could never fire. Now filters on the discriminant alone. - `ensureIndex0Payload` always ran normalizeIndex0 and then returned `Dict`, throwing the guarantee away, so all three callers asserted it back with `as unknown as Index0`. It now returns an `Index0Payload` that carries what the function established, built in that shape rather than asserted into it, and the type propagates through compactIndex0Payload and rebuildCanonicalViewsPayload. - `sign()` in recoveryDrills declared a DrillRecord parameter it never used as one — it HMACs a canonical serialization — which forced the verifier to assert a shape it had not established while checking a signature. Also `rebuildIndex0Checkpoints` typed to the one field it reads, and console.ts asking `"isTTY" in this.out` rather than asserting a shape onto a WritableStream. One assertion remains, in entrySchema, and it is structural rather than evidential: z.preprocess types its output from an inner union deliberately wider than Entry, and no zod spelling expresses "parses to a superset, hands back the narrow type". Disabled inline with that written down. 44 -> 0. The lint step is now in ci_prepush.sh, and the gate runs green in 44.5 seconds. 689 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
e9d49d10bc |
Stop 13 jobs firing on a README edit, and delete an SBOM check that never ran
Two GitHub-side changes, both about not wasting runs. PATH FILTERS. tests.yml (nine jobs across three OSes and three Pythons) and python-ci.yml (four) both triggered on every push and every pull request with no path restriction, so editing a markdown file spent thirteen jobs. They now trigger on the Python implementation and the files that pin its environment. ts-parity keeps triggering on src/** as well as js/**, deliberately: Python is the reference implementation the port is validated against, so a change there can break parity even when no TypeScript moved. THE SBOM STEP IS GONE. It invoked `@cyclonedx/cyclonedx-npm@latest --package-lock-only`, which is the NPM tool and reads package-lock.json — a file this pnpm workspace does not have and never will. It could not have worked on any run. It then swallowed the failure with `|| echo "non-fatal"` and uploaded with `if-no-files-found: warn`, two independent layers stopping it from going red, and its log said "No files were found with the provided path" every time. Removed rather than pinned. Pinning the version of a tool that cannot read this repository's lockfile fixes the wrong problem, and a security check that cannot fail is the same anti-pattern as the pwsh exit-code bug fixed earlier today — worse than absent, because it is trusted. `pnpm audit --audit-level moderate` stays and does fail on findings. A real bill of materials wants a pnpm-native generator, tracked in TODO.md. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
864229bad1 |
Add local CI: a preflight that refuses to lie, and a gate under a minute
Today cost roughly sixteen push-and-wait cycles discovering serially what one
local run could have surfaced at once. This is the local half of that, built
around one rule: a local gate is worth having only if it reaches the same
verdict CI would.
THE PREFLIGHT IS THE POINT
scripts/ci_preflight.py checks that this machine agrees with the lockfiles,
and refuses everything downstream when it does not. That is not theoretical
hygiene. Run right now it says:
black 26.1.0 != locked 24.10.0
pytest 9.0.2 != locked 8.4.1
coverage 7.10.1 != locked 7.10.2
which is exactly today's failure: `poetry run black --check .` passes here and
fails on GitHub, and an earlier commit titled "satisfy the black gate" had
already reformatted five files with the WRONG black, making the gate worse
while looking like a fix. A green local gate that disagrees with CI is not a
weak check, it is a harmful one, because people stop reading the real result.
It is stdlib-only and run by the system interpreter, so it works when the
environment it is checking is broken -- which is when it is needed. And it
only ever CHECKS. scripts/dev_sync.sh repairs. A gate that silently fixes
your environment hides the drift it exists to surface.
TOOLS COME FROM THE REPOSITORY, NEVER FROM PATH
Every invocation is `poetry run ...` or `pnpm -C js ...`. A bare tool name is
precisely how a local gate ends up disagreeing with CI, so the preflight
inspects those environments rather than PATH.
THE BUDGET IS THE DESIGN
Measured here: preflight 2.6s, lock check 0.6s, black 2.5s, typecheck 7.3s,
core vitest 24.0s, determinism subset 5.4s — about 44s. What that excludes
was measured too: the cli suite alone is 176.3s, three times the whole
budget, and full pytest is ~350s. Those go in ci_parity.sh, which is what you
run before calling a branch ready. A hook that takes minutes becomes
`git push --no-verify` inside a week, at which point it is a control everyone
believes in and nobody runs.
The cli suite is slow for a good reason — real daemons, real sockets, real
subprocesses instead of mocks — so the answer is to run it less often, not to
make it lie faster.
THE SMOKE TEST IS NOW NODE, NOT BASH
js/scripts/smoke-cli.mjs replaces an inline shell test that used mktemp,
export and `test -n`, and therefore could only run on Unix — leaving the one
artifact users actually execute never smoke-tested on Windows. Every Windows
failure today was a Unix assumption leaking. It also checks more than the
original: that the bundle runs with no node_modules in sight, that the secret
honours the requested length, and that revealing twice yields the same value,
because determinism is the product rather than an incidental property.
The lint step is written but commented out: `pnpm -C js lint` still exits
non-zero on 35 known assertions, and shipping a red check into the push gate
is how gates get routed around. It goes in when it goes green.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
74a5c23dfb |
Clear the linter's built-in findings, keeping the one that was deliberate
Six warnings from oxlint's default rules, five of them genuinely redundant:
`...(x ?? {})` in an object literal, where spreading undefined is already a
no-op, and a `Boolean()` wrapping an `if` condition.
The sixth is kept. `for (const held of [...this.held.keys()]) this.forget(held)`
looks like a pointless array copy, and oxlint is right that for..of accepts an
iterable directly — but forget() deletes from this.held, so the spread is
what makes the loop iterate a snapshot instead of the map it is mutating.
Disabled on that line with the reason written next to it.
That distinction is the whole reason to read lint output rather than apply
it: a rule that is correct in general was wrong about the one site here where
it mattered, and the site it was wrong about locks every held seed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
de43ace259 |
Type two functions to what they read, removing nine assertions
Nine of the gate's findings were not assertions at all in spirit — they were two function signatures narrower than their callers, paid for at every call site. `recomputeIndex0Stats` took a bare `Dict` while all five of its callers hold a real `Index0`, so each one laundered it through `as unknown as Dict`. It reads exactly three fields, so it now says so: a Pick over events, checkpoints and heads. Any Index0 satisfies it, a partial one still does, and the five casts are gone. `exportBackup` took `Record<string, unknown>` and is genuinely called two ways: with a live VaultIndex from three places in the CLI and API, and with a round-tripped backup payload from the tests. It treats either as an opaque JSON document to canonicalize and encrypt, so the signature now names both shapes instead of making the majority caller pretend to be the minority one. No type surgery and no behaviour change: the same values reach the same code, they just no longer have to lie about what they are on the way in. 44 -> 35. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
51855ddea6 |
Validate high-risk partition records before deriving secrets from them
The first real finding from the new lint gate, and the one that justified
turning it on.
`decryptPartition` proves a partition file is AUTHENTIC — it decrypts under
the partition key and Fernet carries an HMAC, so nobody without the factor
wrote it. It proves nothing about a record's SHAPE: it checks only that each
value is an object. Both hydration paths then asserted the result straight
into `Entry` and handed it to materializeSecret.
That was the one place in the vault where a record reached secret derivation
without passing the schema. Every entry arriving through the index goes
through parseVaultIndex; the partition path did not — and the partition is
where the ssh keys, pgp keys and seeds live. Python writes these files and
validates nothing, so a shape divergence between the implementations, a
renamed field or a kind added on one side, would not have raised anything
here. It would have derived from whatever fields it found and returned a
confidently wrong secret, which is worse than an error by a wide margin.
`parsePartitionRecord` validates per RECORD rather than per file, so one odd
record fails one entry instead of locking the user out of every secret in the
partition. The error names the entry and nothing else, because the record IS
the secret — asserted, along with the fact that a record of an unknown kind
is still carried through untouched per spec §8.2, which validation must not
quietly have turned into a refusal.
Typing the API's hydration path properly removed the round trip it used to
make — Entry to Record and back — so the fix subtracts assertions rather than
trading one for another.
Also clears the twelve dead imports the gate found. One of them was not dead:
the `cause` binding in decryptPartition is discarded on purpose, because the
reason a Fernet decrypt failed distinguishes "wrong key" from "corrupt blob"
and surfacing that hands a caller an oracle for probing the factor. It is now
`catch {}` with that written down, which says it better than an ignored name.
Chained assertions: 44 -> 42. Tests: 689 passing.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
ba4e92d454 |
Restore the lint gate after verifying it removes cleanly
Reverts the revert. The round trip was the point: the claim that this is easy to back out is now tested rather than asserted. `git revert` of the single gate commit followed by `pnpm install` left no oxlint.config.ts, no tools/ directory, no reference in package.json, and a workspace that typechecks and passes all 684 tests without it. Same instinct as killing a live mutation run earlier to prove its SIGTERM cleanup worked: an escape hatch nobody has opened is a guess. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3AN
|
7ca39df513 |
Revert "Add a lint gate for assertions that fabricate type evidence"
This reverts commit
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
1ff47caf15 |
Add a lint gate for assertions that fabricate type evidence
Vendors two rules from github.com/dmmulroy/anti-slop into js/tools/oxlint/anti-slop and wires oxlint into the workspace. This is the first JS linter in the repo. TWO of the project's fifteen rules are enabled. The rest were measured against this codebase and rejected, with reasons recorded in oxlint.config.ts, because "why is this off" is the question a future reader will have. Briefly: no-conditional-empty-object-spread has 77 hits that are all correct, since that idiom is REQUIRED by exactOptionalPropertyTypes; no-unsafe-dictionary-type and no-runtime-typeof have 176 hits that are overwhelmingly the agent's untrusted socket protocol, where a precise type would be false confidence and the per-field typeof checks ARE the boundary validation both rules ask for; three more have zero hits because the suite already uses real dependency seams rather than module mocks. The two that are on catch the same move: an assertion that manufactures certainty the code never established. That is the failure mode a vault cannot afford, because a wrong belief about a decrypted record's shape is a wrong belief about someone's secrets. They find 44 sites, and no-widen-then-assert finds none. Deliberately NOT wired into CI yet: a gate that lands red is a gate people learn to ignore. The findings get triaged and fixed first, then the CI step goes in green. Kept deliberately revertible and separate from any change it motivates: nothing under packages/*/src imports it, it is dev-only, and the shipped CLI bundles with no externals so it cannot reach the published artifact — verified, the rebuilt bundle contains no reference to it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
bc6ee2288e |
Declare tsx where the fuzz runner actually lives
The differential-fuzz CI job failed with "tsx: not found" — the TypeScript side of the fuzzer never ran at all, so the job was reporting on Python output alone. `tsx` was a devDependency of @seedpass/cli only, while the fuzz runner lives in packages/core and is invoked with cwd there. It worked locally because core's node_modules/.bin had a stale tsx from an earlier install, and could never work from a clean `pnpm install --frozen-lockfile`. Precisely the class of bug that only CI can see, on the first run after wiring these checks up. Declared in @seedpass/core rather than papering over it with a path or a --filter, so the invocation is correct by construction. Verified by removing core's node_modules, reinstalling frozen, and running CI's own seed 1 end to end: 1050 cases, no divergences. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
32f76a2f20 |
Cover every owner-op client method's denial, completing the agent sweep
Extends the denied-request test to a table over all nine owner-op methods
rather than the three that happened to have surviving mutants, so a method
added later is covered the day it appears. Each one's dropped check produces
a different false success:
shutdown() -> void, reading as "the daemon stopped" while it is still
running and still holding seeds. The test now pings the
socket afterwards to prove it did not.
status() -> [], reading as "no profiles are held"
tokenIssue() -> the STRING "undefined" handed back as a token
tokenRevoke()-> void, "token revoked", while the token stays live
tokenList() -> undefined
put() -> NaN
agent.ts now sweeps to zero survivors across all 86 mutants, up from 39 under
the old vocabulary rule. Every mutation target is clean: 234 mutants, none
surviving.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
a9e31215ea |
Throw on a denied agent request instead of returning a quiet zero
Every owner-op client method checks `ok` and throws, and none of those checks
was tested. Removing one does not produce an error — it produces whatever the
failed response coerces to, and for each of these that is a value which reads
as a legitimate outcome:
lock() -> 0, printed as "0 profiles locked" while the seeds are
still resident and the user has been told the vault is
locked
highRiskLock() -> false, indistinguishable from "there was nothing to
lock"
highRiskStatus() -> { unlocked: false, expires_at: NaN }
A refusal that looks like a successful no-op is worse than an error, because
nobody retries it. Forced by pointing SEEDPASS_AGENT_CAP at the wrong
capability, which is also the shape of the real failure: a caller that has
lost owner authority.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
d188005b10 |
Refuse unrecognised token scopes and malformed sink commands over the wire
Both checks fail closed if removed, and both fail closed SILENTLY, which is the reason they exist. An unknown scope grants nothing downstream — tokenAllows asks for read, use or reveal by name — so a token issued with `["admin"]` simply never authorises anything. The operator gets a token back, believes they issued it, and finds out when the job using it is denied. The likely cases are near-misses: "Read", "reveal " with a trailing space, "write" by analogy with other systems. Naming the bad scope in the refusal is the whole value. The sink `command` shape check is the same shape of argument. Every malformed command already fails closed, because the allowlist is checked with parseCommandSpec against the SAME array that is later spawned — so the check and the spawn cannot disagree. But failing closed there means a TypeError or a nonsense command word, and this daemon's stated posture is that every caller is untrusted. Refuse the shape while it is still describable. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
7a6b0a5f01 |
Refuse to replace a live agent; clean a stale socket
AgentDaemon.start() guards both cases and neither was tested. Without the live check the second daemon deletes the socket and listens in its place — the first keeps running, holding unlocked seeds, with an expiry sweep nobody can reach and an audit log nobody is writing to. Silently orphaning a process that holds seeds is worse than failing to start, which is what makes it a refusal rather than a convenience. The other half matters too: a crashed agent leaves its socket behind, and treating that as "already running" would make the agent unstartable until someone deleted a file by hand. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
f487a001a2 |
Test job-profile revocation, completing the routes.ts sweep
The job-profile surface had no API test at all. The DELETE is the one that matters: without its existence check it answers 200 "revoked" for a job that never existed, so an operator revoking an agent's access is TOLD it worked while nothing happened. For a revocation that is the worst available answer. Revoking twice is asserted too, since a second success is how the false positive shows itself. routes.ts now sweeps to zero survivors across all 85 mutants under the refusal-based selection, up from 30 mutants under the old vocabulary rule. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
46638e4b86 |
Test relay deletion by index, which had no test at all
DELETE /api/v1/relays/:idx guards the index twice and neither guard was exercised. An off-by-one here does not fail loudly — it removes the WRONG relay, so a user dropping one they distrust keeps it and loses one they wanted. Only asserting the exact resulting list can see that; a length check passes either way. Covers index 0 (a valid index, not a missing one), the last index versus one past it (the `>=` boundary), and a malformed index answering 400 rather than 404 — the request is wrong, not pointing at something absent. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
910693e952 |
Refuse an empty vault-import body as a 400, and cover a non-backup body
Without the explicit length check, parseBackupWrapper's JSON failure on an empty body escapes as an internal error — the server reporting a fault for a request the caller simply sent empty. Also pins that a body which is not a backup at all answers 4xx rather than 5xx, since the two failures take different paths and only one of them was exercised. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
5aa6d04248 |
Refuse a config write with no value, and 404 a document import with no file
Two more from the routes sweep, both cases where removing the check produced a wrong answer rather than no answer. `PUT /api/v1/config/:key` with a body carrying no `value` was only refused by the explicit check. Boolean(undefined) is false, so for every boolean key — offline_mode, secret_mode_enabled — an empty body would have quietly SET the setting to false instead of refusing. The test now confirms a refused write leaves the existing value alone, which is the property that actually matters. `POST /api/v1/entry/document/import` from a path checks the file exists; without it readFile's ENOENT surfaces as 500, "the server broke" for a filename the caller got wrong. Also asserts the traversal guard applies to the import side and not only to export, which nothing covered. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
d8292db325 |
Cover 404 on every entry-scoped route, not just the two that had tests
PUT, archive, unarchive and GET links each check that the entry exists and none of them was tested, so all four checks could be removed with the suite green — the underlying op would then run against a missing entry and answer 400 or 500, telling a client to fix its request or retry when neither would help. The secret route had the same gap. Enumerated from the route table rather than listed by hand, so a new entry-scoped route is covered the day it is added, which is exactly when its existence check is most likely to be forgotten. DELETE links validates its `target` before looking the entry up. That ordering is right — validate the request before touching state — so the test supplies a valid target rather than treating it as a finding. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
2d27b5aaa9 |
Assert optInt's three edges and the path guard's positive case
Both checks could only be observed failing, so both could have been quietly inverted. `resolveWithinProfile` is `target !== root && !target.startsWith(root + "/")`. Turn that `&&` into `||` and every path except the profile root itself is refused — which the traversal cases could not see, because they expect a refusal either way. A check that refuses everything is not a working check, so the test now writes to a legitimate path inside the profile and confirms the file lands there. `optInt` guards length, period, digits, index, delay and the history target, and each of its three clauses was unasserted: a non-integer (1.5), a negative, and — the one that matters for the boundary — zero, which is NOT negative and must stay accepted. `n < 0` quietly becoming `n <= 0` would reject a clipboard delay of 0 and a derivation index of 0, both real choices. Records one thing found on the way and deliberately not fixed here: a nested export path answers 500 rather than a refusal naming the missing directory. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
bd17ad5eae |
Pin the API's body-shape refusal and the partition gate's expiry case
Two more from the routes sweep. `bodyObject` rejects arrays explicitly because typeof [] is "object" — the array check is doing real work, not restating the typeof. Without it an array body reached the handlers, where `body["kind"]` is undefined and the caller was told a field was missing, sending them after the wrong problem. The partition gate's test covers the case that actually matters. With no tag at all, dropping the gate fails anyway because readPartition cannot decrypt with null — so the mutation survived. But an EXPIRED session still holds its tag, and without the gate readPartition succeeds with the stale key and returns the secret: the second factor lapsing silently. No test read a partition entry through the API at all before this. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
5315d8fae9 |
Fix: migrating a seed entry produced an index that would not parse
Found while writing a test for a mutation survivor on the API's partition gate. A high-risk STUB — what the index keeps after `agent high-risk migrate` moves an entry's substance into the encrypted partition file — was being validated against the full schema of the kind it stands in for. It is a different shape on purpose: kind, index, label, archived and the partition pointers, so an index read without the factor discloses that a high-risk entry exists and nothing about it. `seedEntrySchema` requires `word_count`, which a stub does not carry. So: - `agent high-risk migrate` on any vault holding a seed entry built an index saveVault refused to write — correctly, that check exists so a malformed index never reaches disk — but only AFTER writePartition had copied the secret into the partition file. The migration failed while leaving a copy of the secrets behind. - Worse, Python builds the byte-identical stub (high_risk_partition_store.py, migrate_high_risk_entries) and validates nothing on save, so it writes one happily. A vault Python had migrated could not be OPENED by this implementation at all. That is a cross-implementation data-compatibility break, not a TypeScript-only bug. Fixed by modelling the stub as its own shape in the entry union rather than by adding `word_count` to the stub, which would have made the two implementations write different bytes and diverged sync's canonical hashing. It is not a validation bypass: a stub holds no secret material, the `partition: "high_risk"` literal is the discriminator, and an entry claiming a known kind without it still faces that kind's own schema — asserted. Only `ssh` was ever exercised, which is exactly why this survived: ssh's schema has no required field a stub omits, and pgp's extras carry defaults. The new tests cover all five high-risk kinds, so a future kind with a required field breaks a test rather than a vault. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
4184de9b9e |
Close four more gaps the widened mutation rule found
Sweeping the smaller targets under the refusal-based selection. All four are cases where two code paths both refuse, so only the reason or the boundary was observable — and nothing observed either. - Approval RESOURCE SCOPING was tested only on its failing side. The check is `record !== "*" && record !== requested`; for a scoped record and a different resource both halves are true, so `&&` and `||` agree and the mutation was invisible. The two cases where they differ are exactly the ones that make scoping useful rather than merely restrictive: a scoped approval must WORK for the resource it names, and a wildcard must work for any resource. Under `||` both stop working entirely. - The request body cap's boundary. A limit of N bytes has to mean N bytes are allowed; `>` accepts exactly N and `>=` rejects it, and the existing test sent 500 bytes at a 64-byte cap, which cannot tell them apart. The cap could have gone off-by-one and surfaced only as requests failing at a size the operator configured as fine. - readEnvelope's existsSync guard. Without it, ENOENT is caught by the same handler as malformed JSON, so a profile that never configured a factor is told its partition envelope is invalid — "restore a backup" instead of "set one up". - sinks.ts and server.ts swept clean under the new rule with no further changes. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
9c9544b725 |
Mutate guards by what they DO, not just what they are about
The generator selected lines by an auth vocabulary — token, scope, expire, password. src/configFile.ts, whose validators are the API's only defence against a nonsense value being written permanently into an encrypted file, generated ZERO mutants: `if (n <= 0) throw new ConfigValueError(...)` is a refusal in every meaningful sense and contains not one word that list knows. The NOT MEASURED reporting added earlier is what surfaced it, on its first run against a new target. A guard whose body refuses is now selected on that basis, vocabulary or not. That takes the corpus from 132 mutants to 234 and it is the right trade: a refusal is the security surface, definitionally. It immediately found three gaps in the config validators written in the previous commit: - The `||` chain rejecting non-numbers was only tested with a string. Number(true) is 1 and Number(null) is 0, so `inactivity_timeout: true` became a one-second timeout and `clipboard_clear_delay: null` became zero — a setting silently changed to something nobody asked for. - `n < 0` versus `n <= 0` on the non-negative fields: zero was never tested, and it is a legitimate value that must not be conflated with negative. - The `kdf_iterations` floor had no test at all. Zero iterations is not a slow KDF, it is no KDF, in the config of a vault whose master password protects everything else. configFile.ts is registered as a mutation target so it stays measured. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
ae3889bfca |
Stop the config API accepting any key and any value
Adversarial pass, first finding. `PUT /api/v1/config/:key` took the key from the URL and the value from the body with no validation beyond blocking two credential verifiers. Two ways that goes wrong, both silent: A typo is stored forever. `inactivity_timout` was accepted, written into the encrypted config, and answered ok — while the setting it was meant to change never moved. Nothing downstream reads an unknown key, so this failure can only ever be silent; the caller has no way to find out. A wrong type is stored verbatim. `inactivity_timeout: "soon"` turns every later comparison into NaN, which compares false, so the vault simply never times out. That is a security setting failing open because a string reached a numeric field. There is now an allowlist of 23 keys, each with a normalizer that rejects values it cannot make sense of: negative delays, a zero inactivity timeout (indistinguishable from a broken vault), an empty relay list (not a configuration, an outage), a string where an array belongs. Unknown keys get Python's 400 "Unknown key"; the credential verifiers move from 400 to 403, matching Python, since the key is real and the refusal is about authority. The allowlist is deliberately a SUPERSET of Python's, which cannot set the password-policy keys this implementation honours. Copying Python exactly would have deleted working behaviour to reproduce a limitation. Recorded in TODO.md as a divergence to settle rather than left as an accident. Separately: the shared API test fixture was inheriting the default 100 requests/minute budget and had grown past it, so unrelated tests failed with 429 depending on how many requests ran before them. Rate limiting is tested on its own servers with explicit budgets, so the fixture no longer enforces one. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
e1ea9c2b48 |
Teach the mutation harness two blind spots it had, and close what they found
The harness only mutated block-form guards (`if (cond) {`), so every guard
written as a one-liner was invisible to it — which turned out to be most of
the load-bearing ones. src/highRisk.ts generated ZERO mutants and printed as
a clean sweep. Three changes to the tool:
- Mutate single-statement guards too (`if (cond) return ...;`). This alone
surfaced 27 previously unreachable mutants, including the entire token
validation chain: revoked, expired, exhausted, scope-not-granted.
- Report a target that generates no mutants as NOT MEASURED. Zero killed and
zero survived prints exactly like a clean sweep, and it is the one result
that must never be read as a pass.
- Add `kind`, `mnemonic` and `seed` to the security hints. A token's `kinds`
list is half of what it may reach, and leaving the word out hid the shape
check that stops a string `kinds` becoming a substring match.
Also adds `--kind` so a new mutation rule can be swept across every target
without re-judging the mutants already settled.
What the new mutants found, all of it previously unasserted:
- A token's `kinds` restriction was never tested. This file's own header
claimed kinds and labels were both covered; only labels were. Removing the
check let a token scoped to one kind read every other kind's secrets.
- `token-issue` would mint a token for a profile the agent is not holding.
The seed being resident is the only proof the caller had authority over
that vault.
- `token-issue`'s array shape checks for `scopes` and `kinds`. A string
`kinds` turns tokenMaySee's exact match into String.includes.
- An empty high-risk factor. Refused in both implementations, tested in
neither; it would wrap the partition key under nothing while
`factorConfigured` still answered true.
- POST /api/v1/fingerprint/select accepting a profile that does not exist —
the caller's string becomes ctx.fingerprint, which later requests join onto
a path. AppDir repeats the check, so the guard's real value is answering
404 instead of an unhandled 500.
- The API's requiresPassword flag is redundant for every route today, so
disabling it changed nothing. Its purpose is the route that does not exist
yet, whose author sets the flag and forgets the call — so the test now
registers exactly that route and confirms the server layer stops it before
the handler runs.
- sinks.ts drops SEEDPASS_-prefixed variables from a child's environment.
Nothing on the allowlist starts with SEEDPASS_, so the backstop cannot fire
against the real list. sinkEnv now takes the allowlist as a defaulted
parameter purely so that future-edit case can be exercised.
All six targets now sweep to zero survivors.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
0f59602622 |
Close every routes.ts mutation gap: seven unasserted API behaviours
routes.ts now reaches zero mutation survivors (30 mutants). The suite covered what the API refuses very thoroughly and what it REPORTS almost not at all, which is where all seven gaps were. The one that could destroy data: DELETE /api/v1/fingerprint/:fingerprint guards against deleting the profile the server is currently serving. Removing that guard entirely left the suite green -- the fixture only ever had one profile, so the guard was never exercisable. The test now seeds a second profile and asserts the pairing: the served one is refused, another is deleted. A refuse-everything implementation cannot pass it. The rest report vault state that nothing checked: - GET /api/v1/fingerprint's `current` flag. Inverted, every profile EXCEPT the served one is flagged, which a profile switcher shows straight to the user. Asserted as a pairing, since "at least one is current" survives the inversion. - GET /api/v1/vault/status's `locked`. Inverted, it reports locked while unlocked; the test also confirms the flag tracks something real by checking a locked vault refuses secrets. - GET /api/v1/high-risk/status, which had no test at all: the exact-expiry boundary, the tag-AND-unexpired conjunction (a stale expiry with no tag read as open), and the expires_at null decode. Built on its own context so the clock can be frozen. - POST /api/v1/entry with an empty totp `secret` must DERIVE one, not import the empty string as a key. An entry that looks like a second factor and is keyed on nothing. - GET /api/v1/totp/export must export a stored secret as stored and derive past a blank one. Inverted, every exported secret is still well-formed base32 -- so shape assertions pass while the codes are for another account. The fixture gains an entry with a present-but-empty secret, the shape a legacy or Python-written index can hold and the API cannot create. - POST /api/v1/high-risk/unlock answers 409 not_configured rather than falling through to 401 factor_invalid, which tells someone who never set a factor that the one they just chose is wrong. Two fixture counts were hardcoded literals; they now derive from the fixture, so adding an entry cannot silently weaken "no secrets in the listing" into "no secrets in an empty listing". Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
62e577a6e1 |
Collapse the duplicated high-risk expiry rule; close agent mutation gaps
Mutation testing carried agent.ts to zero survivors (37 mutants, all killed or non-compiling). Four gaps, in descending order of how much they mattered: The high-risk session's expiry was decided in two places -- the sweep in expire() and a second comparison inside highRiskTag(). Breaking EITHER left the whole suite green, because each covered for the other. Worse, the copy in highRiskTag() was unreachable: handle() sweeps before it dispatches, so no request could ever arrive with an expired session still in the map. It read like a check and enforced nothing. highRiskTag() now sweeps instead of re-testing expiresAt, so the rule has one home and a test can hold it to account. The remaining three were missing assertions, not missing checks: - `put`'s `!fingerprint || !mnemonic` guard could become `&&` unnoticed. Both fields are re-validated downstream, so nothing is permitted that was not before -- but a blank mnemonic then reports "fingerprint does not match the supplied seed", sending whoever reads it after the wrong field. Tests now pin the message. - Same shape on high-risk-unlock's fingerprint format guard: disabling it entirely still refused, via "profile not unlocked" -- a lie about a string that could never be a profile. It is also the guard between a caller-supplied string and the audit-log path join, so it should be the one that answers. - highRiskStatus's client-side `expires_at === null` decode was never asserted; inverted, it reports 0 for a locked profile and null for a live one. That number is how a caller knows what is left of the second factor. Also hoists FROZEN/TAG/daemonAt to module scope so the new tests share the injectable clock. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
17b94d3e26 |
fix(agent): an empty exec allowlist now fails closed; test the allowlist at all
Mutation slice 2 scored 0% before this, and the reason was blunt: **the exec allowlist had no test whatsoever.** The clipboard restriction could be inverted, weakened, or deleted outright and the suite stayed green. That control matters because the clipboard is readable by every process in the session. A token restricted to `cat` that may still reach the clipboard is not restricted to anything — the holder simply copies the secret out, and the allowlist looks enforced while containing nothing. Now tested: a command-restricted token is refused the clipboard, a token with no allowlist is not (so the rule is conditional rather than a blanket ban passing as one), an allowlisted command is permitted, and one outside the list is refused without the sink ever running. **A fail-open found while triaging one of the survivors.** An empty `exec_allowlist` was DROPPED at issue time, which made the token unrestricted. Automation that computes an allowlist and arrives at zero entries means "permit nothing" and was silently handed "permit everything" — a fail-open in the one field whose entire purpose is restriction. It is now refused at issue time with an error saying which of the two readings the caller probably wanted. Reachable only over the raw socket protocol, since commander requires at least one value. **An audit-accuracy gap.** A denial and a missing entry deliberately return the SAME message, so the response cannot be used to enumerate ids — the audit log is where the difference is supposed to be recorded, and nothing read it. The two reasons could be swapped with every test passing. Testing it needed two corrections of my own: the CLI resolves references locally so the request never reaches the daemon, and the shared token in that file is revoked by an earlier test, so both calls were refused at the token check instead. It also needed a stronger assertion than the one I first wrote — checking that both reasons merely APPEAR passes when they are swapped, which is exactly the mutation. It now asserts the pairing: which id got which reason. Three survivors are recorded as genuinely unkillable with reasons, rather than chased: the two `length > 0` comparisons cannot differ from `>= 0` now that an empty allowlist cannot be stored, and the capability early-return is redundant with the length check below it. agent slices 1-2 (mutants 1-16): 100% of viable mutants caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
12b945ca7e |
test(agent): pin token expiry at its exact boundary; give the daemon a clock
Mutation testing found `token.expires_at <= now` could become `<` with every test still passing. The suite checked well before and well after expiry, which cannot tell the two apart — so a token was free to be usable for one more request than it claimed, which is the wrong direction for a credential. The daemon now takes an injectable clock, so the boundary can be asserted exactly rather than raced against wall time: at T-1 the token works, at exactly T it is refused. Every expiry decision AND every expiry assignment goes through that clock — a token issued against real time while checked against test time would never expire, which would have made the test meaningless. The TOTP timestamp-skew window uses it too, since a clock that moved one but not the other is a difference no production path should have either. Also pins the owner capability: an empty or absent capability is refused, and so is one of the right length but wrong content — the latter proving the constant-time comparison is what rejects it rather than the length check. One survivor is now recorded as genuinely unkillable rather than chased. `!presented || !this.capability` cannot change behaviour when flipped to `&&`, because the length comparison below rejects an empty string either way. The harness learned a `mutation-equivalent:` marker that must carry a REASON, so future survivor lists mean "look at this" instead of "this again" — and the judgement stays re-checkable rather than merely trusted. The harness also gained `--offset`, so a long target is measured in foreground-sized slices. That is a safety property, not a convenience: this script writes broken security code into the working tree, and it must never run unattended alongside commits again. agent slice 1 (mutants 1-6): 100% of viable mutants caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
c2a6a11472 |
fix: revert an inverted security check I committed by accident, and make it impossible
Commit
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
cf18ab8091 |
test: close the gaps mutation testing found in approvals, sinks, and a flaky test
Approvals scored 58%. Five survivors, all genuine:
- `ttlSeconds` and `uses` validation was unguarded by any test. An approval
issued with ttl 0 is expired the instant it exists; one with uses 0 can
never be consumed. Either would look like a granted authorization to
whoever asked for it and silently authorize nothing — the confusing
failure rather than the safe one.
- The expiry boundary (`<=` vs `<`) was unpinned. It now asserts that an
approval expiring exactly now is refused, and that one millisecond earlier
it still works, so the boundary is where it claims to be rather than
approximately there. Python uses `<=`, so this is parity as well as the
safer direction.
Approvals now 100%.
Sinks scored 67%. The survivor turned `&&` into `||` in the command-spec
parser, which makes a multi-element spec whose FIRST element contains
whitespace get tokenized — silently discarding every element after it. Losing
an argument changes which command runs, and dropping a `--dry-run` or a target
path is exactly the kind of silent change that matters for a sink being handed
a secret. Sinks now 100%.
Separately, the full suite caught a flaky test of my own: the recovery-split
tamper check rewrote the payload's tail and, depending on the payload's
padding, sometimes reproduced the original string exactly — asserting nothing.
It only failed when a randomized share happened to end the wrong way. Now it
flips a character and asserts the result actually differs before expecting the
throw, plus a second case that tampers at four positions rather than trusting
one. Five consecutive runs are clean.
The agent's mapping was also wrong: agent.ts holds the high-risk session, whose
tests live in highRisk.test.ts, so leaving that suite out scored the agent at
38% and blamed the code for a gap that was really a gap in the harness.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
c12f5de213 |
fix(api): password-gate live TOTP codes; add mutation testing for the un-oracled surfaces
Differential fuzzing can only reach code both implementations have. The session agent, the API's HTTP layer, high-risk session handling and the sinks exist only in TypeScript — and that is where the security properties live. For those the question "would the suite notice if this broke?" has no oracle, so it has to be answered by breaking things deliberately. scripts/mutation_test.py does that: it flips comparison and boolean operators and forces guards to always-allow, but only on lines that decide whether something is permitted, expired, within a limit, or authentic. Mutating a log line would produce a survivor that means nothing. Every mutant is typechecked first, so a mutant that does not compile is reported as invalid rather than counted as caught, and the original file is restored even on interrupt. The first run on the API server found two survivors, and they were different in kind: **A real gap.** Disabling the rate-limit check entirely left every test passing — nothing exercised it. Now tested: the limit refuses the request past its budget, and unauthenticated requests consume budget too, since otherwise the limit is bypassed by simply not sending a token. **An equivalent mutant, which pointed at a real hole anyway.** The route-level `requiresPassword` flag turned out to be fully redundant with the `requirePassword` call in each handler, so disabling it changed nothing. The redundancy is worth keeping, but nothing ENFORCED the pairing — so a future route could set one and forget the other with the suite silent either way. Two structural tests now pin it. Writing the second of those tests immediately found a security regression I had introduced: **`GET /api/v1/totp` returned live TOTP codes on a bearer token alone.** Python gates that endpoint on the master password (`_require_password` in api.py) and this port did not, so a leaked token yielded working second factors for every TOTP entry in the vault. Now gated, and tested for. Server mutation score: 88% of viable mutants caught, the single survivor being the equivalent one above. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
991c71189e |
test: differential fuzzing against Python, as the substitute for outside review
The cutover plan's last open gate is an independent security review, and there
is no third party to hand the branch to. Self-review has a known failure mode:
you cannot see what you did not think of, which is why two earlier rounds each
found criticals that reading had missed. Me reviewing this session's code has
exactly that defect.
The substitute for another pair of eyes is not more reading. It is an oracle
that is not the author's judgement — and this project has the best one
available: two implementations required to agree. Feed both the same randomly
generated input and any disagreement is a bug in one of them, established
without anyone knowing in advance which answer is right or what to look for.
scripts/differential_fuzz.py covers the surfaces where a disagreement would be
silent and expensive: canonical JSON and hashing (underpins everything else),
password derivation with random policies, the sync CRDT (a mismatch means two
clients converge to DIFFERENT vaults), index0, recovery shares, the semantic
index, and entry hashing. Results are compared as canonical JSON so the
comparison cannot itself paper over an ordering difference. ~10,000 cases
across four seeds currently agree.
IT FOUND A REAL DEFECT, and then found a defect in itself.
canonical.ts asserted that the int/float ambiguity "does not arise" because
SeedPass data never carries float-typed whole numbers. That is true of data
SeedPass writes and false of data it carries: unknown-kind records, hand-edited
vaults, third-party tools. Python distinguishes int from float; JavaScript does
not, so `JSON.parse("0.0")` and `JSON.parse("0")` are the same value and TS
emits "0" where CPython emits "0.0". Entry hashes break same-timestamp merge
ties, so two clients could converge to different vaults — silently. No amount
of formatting fixes it: the information is destroyed at parse time.
The comment is now accurate rather than reassuring, and `findAmbiguousNumbers`
detects the condition on raw JSON text, which is the only moment the
distinction still exists. A second, separate limitation is documented in the
same place: values outside the safe-integer range are refused where Python
encodes them, which is the SAFE direction, because JSON.parse has already
rounded such a value and any hash of it would be a hash of the wrong number.
Then mutation testing was turned on the fuzzer itself, and it failed: two
deliberate off-by-ones in the tombstone-retention and checkpoint-subject caps
went undetected, because random small payloads never reach a bound of 2048 or
64. Two generator bugs behind it — tombstone keys drawn from 31 distinct
values, so 2050 writes landed in 31 slots; and event subjects that peaked at
63 distinct against a cap of 64, one short. Both fixed by generating well past
each bound with margin, and both mutations are now caught. Bounded code is
only tested at its bound.
A fuzzer that cannot fail is worthless, so that is verified too: inverting the
merge tie-break is caught in 20 of 120 cases.
Known number-representation divergences are classified separately rather than
filtered, and the classifier is deliberately narrow — "known limitation" is
exactly the category that quietly grows to hide real bugs.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
c6cb85e840 |
feat(vault): port index0/atlas, and keep it current on every mutation
index0 is the per-vault activity ledger under `_system.index0`: an append-only event stream per writer, daily checkpoints rolling those events up, and derived canonical views (children_of, counts_by_kind, recent_activity). It was the last piece carried through verbatim rather than computed — which was safe for round-tripping but meant a TypeScript client never UPDATED it. A vault written by this port went stale the moment it was touched, and a Python client reading it afterwards saw an activity log missing everything the TS client had done. Worse, `nostr restore` kept the local block verbatim, so a restore silently discarded the remote's history. Hash parity is the entire contract here: event ids, integrity hashes, checkpoint summaries and view hashes are SHA-256 over Python's canonical JSON, and the merge picks winners by comparing those hashes — one byte of encoding disagreement means the two implementations permanently disagree about which record wins. Everything routes through the existing canonicalJson (Python's ensure_ascii escaping, code-point key ordering, CPython number formatting) rather than JSON.stringify, and the tests compare whole structures via canonical JSON rather than toEqual, so they pass only if the bytes that get hashed are identical. Verified against Python fixtures covering the append chain, compaction, manifest metadata, views and a two-replica merge — including non-ASCII labels and tags, a non-numeric entry id, two writers across two days, and a managed-account scope path. Two Python quirks are replicated deliberately rather than "fixed", because diverging would change which events exist rather than merely how they hash: `str(None)` is "None" (so a JSON null in a required field is a non-empty string there), and a stored integrity_hash is preserved even when it disagrees — this normalizes, it does not verify, and silently rewriting a mismatched hash would hide the tampering the hash exists to reveal. Events are emitted by DIFFING the index at the single mutation funnel each surface goes through, rather than from each call site as Python does. That is deliberate: a call-site emitter is something a new code path can forget, and a silently incomplete ledger is worse than an absent one because it reads as "nothing happened". The event vocabulary matches Python's, so a stream from either implementation reads the same. Emission is best-effort — the atlas is derived state, and it must never fail a committed vault write. mergeIndexPayloads now defaults to merging index0 with the same deterministic rules; "preserve-current" and "reject" remain for callers that want them, and `nostr restore` no longer discards the remote's history. Also made gen_high_risk deterministic. Fernet embeds a random IV and the current time, so that fixture regenerated differently every run and would have failed the fixture-drift CI job each time — it now pins both, with a note that production must never do this. Verified end to end: the TS binary performed four mutations, and Python read the resulting atlas, normalized it without dropping a record, and compacted it into checkpoints and views. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
7f095cba86 |
feat(agent): port job profiles and recovery split; fix Shamir's sub-threshold secrecy
Recovery split is Shamir over GF(257), one polynomial per secret byte, shares
encoded as sprec1:<label>:<threshold>:<total>:<index>:<digest>:<payload>.
Verified interoperable in both directions on real shares, including multi-byte
UTF-8 and payloads containing colons: Python recovers TS-produced shares and
TS recovers Python-produced ones. Confirmed end to end by splitting a live
parent seed 3-of-5 with the TS binary and recovering it in Python.
**Python's coefficients were derived from the secret, and that is now fixed.**
`_coef` HMAC'd the secret to produce every polynomial coefficient, which made
the entire share set a deterministic function of it. Shamir's defining
property is that any sub-threshold set of shares is information-theoretically
independent of the secret; deriving coefficients from the secret destroys it,
because one share becomes an offline verifier — guess a secret, re-run the
split, compare. Demonstrated recovering "hunter2" from one share of five at
threshold three, before changing anything. It also meant re-splitting produced
byte-identical shares, so a leaked share could never be rotated out without
changing the secret.
The determinism was load-bearing in the wrong way: coefficients were
recomputed inside the per-share loop, so they HAD to be reproducible or the
points would not lie on one polynomial. Fixed by hoisting them out and drawing
from `secrets`. The share token format is unchanged and `recover_secret` is
untouched — recovery is Lagrange interpolation over (x, y) pairs and never
recomputes coefficients — so old shares keep recovering and both versions
interoperate. Zero interop cost for a real security property.
TypeScript never had the defect and its test suite pins it: reintroducing
secret-derived coefficients turns three tests red, including the guessing
attack. Coefficients use rejection sampling because 257 does not divide 2^16,
and a modulo draw would bias them.
Job profiles port straight across, with policy_stamp as the security-relevant
field: sha256 of the canonical agent_policy.json at creation, so a policy
tightened afterwards shows as a mismatch instead of leaving the job running
under rules nobody reviewed it against. The canonical form matches Python's
`sort_keys=True, separators=(",",":"), ensure_ascii=True` — including the
ensure_ascii escaping, which is easy to miss and would otherwise make a
non-ASCII policy hash differently in the two implementations. Verified
byte-identical stamps for both an empty policy and one with non-ASCII values.
An unreadable policy hashes to its own distinct value rather than to the empty
policy, so jobs read as stale rather than silently current.
Recovery drills use the same HMAC-chained log design as the audit log, with
tests that an edited record, a removed record and an unsigned appended record
are all detected.
Every Python API endpoint now has a TypeScript equivalent: UNPORTED_PREFIXES
is empty. The 501 mechanism stays, because the honest answer for a dropped
subsystem is 501 naming the feature rather than 404.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
0c48c1f9dc |
feat(agent): port high-risk partitions and approval gates
High-risk partitions move ssh/pgp/seed/nostr/managed_account records out of
the vault index into a separately encrypted file, so those kinds need a SECOND
factor beyond the master password. The index keeps a stub carrying kind, label
and archived — enough to list, nothing more.
Both file formats are byte-compatible with Python and verified bidirectionally:
TS unwraps a Python-written key envelope and reads a Python-written partition,
and Python unwraps a TS-written envelope and reads a TS-written partition. That
matters more than usual here — the entries are SSH keys, PGP keys and seeds, so
a partition only one implementation can open is lost secrets, not an
inconvenience. This required a Fernet encryptor in core, which had been
decrypt-only by design; the header now says why that exception exists and that
no other Fernet payload should ever be written.
**One deliberate divergence, and it is the reason this is not a straight port.**
Python records an unlock session in agent_high_risk_unlock.json, and that record
contains the partition_key_tag — the value the partition file's encryption key
is derived from. So for the life of any Python unlock session the partition is
decryptable from disk alone, with no factor, by anything that can read the app
directory; both files live in the same directory, so "can read one" implies "can
read the other". That voids the guarantee the partition exists to provide.
Verified empirically before writing any of this.
Here the tag lives only in the session agent's memory, beside the parent seeds
it already holds, with the same TTL discipline — and in the API process's memory
for the API. Nothing derived from the factor reaches disk, asserted by a test
that scans every file in the app directory for the tag, and confirmed on a live
install. No file format or interop changes; only where the live session lives.
Other properties the tests pin:
- a locked partition refuses reveal rather than deriving from the stub, which
would return a plausible wrong secret
- locking the vault drops the high-risk grant, which is scoped to it
- the grant expires on its own clock, typically much shorter than the seed's
- unlock is owner-gated: a scoped token cannot reach it
- the tag is absent from the audit log, which records only that an unlock
happened
- the factor comes from the environment, never argv
- factor-set refuses to replace an existing factor, since a new partition key
would make an existing partition permanently unreadable
- migrate writes the partition BEFORE the index, so a crash between them
cannot leave stubs pointing at records that were never stored
Approval gates port straight across: one-shot (or N-use) authorizations for
export / reveal_parent_seed / private_key_retrieval, consumed under the store
lock so two callers cannot both spend the last use, with every failure mode
getting its own reason string matching Python's. A corrupt store reads as empty,
which fails closed.
API: /api/v1/high-risk/{status,unlock,lock} are real rather than 501. Unlock
needs the bearer token, the master password header AND the factor header; a
wrong factor gets one indistinguishable reason so probing cannot tell failures
apart.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
885cbe5665 |
fix(semantic): delete indexes written before secrets stopped being indexed
Fixing the writer does not rewrite files already on disk, so a profile that built an index before today still has one holding stored secrets in plaintext. Reporting a version string in a status field and expecting the user to notice is not a remedy for that. Both implementations now check the manifest's model_id on every read — status, search, and the API equivalents — and delete records + manifest when it predates the fix. Reading a stale index would also mean serving results derived from the secrets it should never have held, so search purges before it reads rather than after. Deleting is safe: the index is a derived cache, rebuilt from the vault in milliseconds, and the user is told to rebuild. A manifest that cannot be parsed is deliberately NOT treated as stale. "Cannot tell" must not mean "delete it", or every corrupt-manifest read would throw away a working index; that case is tested on both sides. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
fdbdb12596 |
docs: record the API, CLI, QR and semantic ports, and the index secret leak
Also rewrites capabilities().not_yet_ported so each remaining entry says WHY it is not ported, since automation branches on that list: PGP RSA is a hard constraint (seeded prime search is not reproducible, so a port would derive a different key from the same seed), index0 is carried verbatim but not recomputed, and the agent authorization subsystems and Textual TUIs are choices. Flags one thing that needs a decision: existing semantic index files written before today still contain plaintext secrets, and the fix does not rewrite them. The model id distinguishes them, but relying on a user to read a status field is weak. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
39c4e45376 |
feat(semantic): port the retrieval index — and stop it writing secrets to disk
The "semantic (vector search)" entry in not_yet_ported was a misnomer. It is
not vector search and needs no model: MODEL_ID is "seedpass-token-overlap-v1",
and it scores by Jaccard overlap over tokenized metadata. That is why it could
be ported at all — there is nothing to embed.
Porting it surfaced two defects in the Python implementation.
**Stored secrets were written to a plaintext, 0664 file.** `_extract_text`
appended a key_value entry's `value` — the stored secret itself — and
`build()` wrote it to semantic_index/records.json in the clear, along with a
tokenized copy that leaked it just as thoroughly. The file was created at the
process umask, so on a default install it was group- and world-readable, next
to a vault that is encrypted and 0600. Anything that could read the directory
— a backup, a sync client, another user on a shared machine — got the secret
without the master password.
Fixed in Python (index the key NAME, never the value; chmod 0600 on both
files) and never done in TypeScript. The rule is worth keeping simple: nobody
searches for a secret they do not already know, so indexing one is pure
downside. MODEL_ID is bumped to v2 so an index built by the old code is
distinguishable — a v1 index on disk still contains the secrets and should be
rebuilt.
**Entry 0 was silently unsearchable.** The check was
`int(entry.get("id", 0) or 0) <= 0`, conflating "no id" with "id 0" — and
entry ids start at 0, so the first entry any user ever created was absent from
every search result with nothing to indicate why. Found by writing a test that
queried for it. Fixed in both implementations: skip an id that is absent,
unparseable or negative, which is what the check was reaching for.
The TypeScript port is wired into the CLI (`semantic build/status/search`),
the API (the /api/v1/semantic routes are now real rather than 501), and
writes through atomicWrite for a genuine 0600 inode. Fixtures come from
scripts/generate_ts_port_fixtures.py, which asserts the leaked value is absent
from its own output, so the fixture-drift job proves provenance the way it
does for every other fixture.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
51f1358da4 |
feat(tui): render real QR codes; verified module-for-module against the reference encoder
The TUI had a QR action that answered "QR rendering is not available in this
build". The stated reason was that a QR library is not worth a dependency in a
process holding unlocked seeds — which is true, and is why this implements the
encoder instead: byte mode, versions 1-40, all four error-correction levels,
zero dependencies.
A wrong QR code is worse than no QR code. It scans cleanly, yields the wrong
secret, and the user finds out when their 2FA stops working — so this is
verified rather than eyeballed:
- The block-structure table was extracted programmatically from qrcode's
RS_BLOCK_TABLE rather than transcribed. 160 rows of three-number triples
are exactly the kind of data a human copies wrong once and never notices.
- qr.json holds reference matrices from that library, and the tests compare
EVERY module — 10 cases spanning all four EC levels and versions 1 to 17,
including past version 7 where version-information blocks start being
written.
- Generation moved into scripts/generate_ts_port_fixtures.py, so the
fixture-drift CI job proves the committed fixture came from the committed
generator, like every other fixture. Regenerating produced byte-identical
output for all pre-existing fixtures.
Two things are pinned so the comparison is between implementations rather than
between configurations. Byte mode is forced, because the reference
auto-selects alphanumeric for an all-uppercase string — a different encoding
of the same text, not a different answer. And the mask is pinned, because the
two disagree about mask SELECTION: the spec scores the finished symbol, while
the reference scores one with its format modules blanked and its dark module
light (makeImpl(test=True, ...)), which is not a legal symbol. All eight masks
produce valid scannable codes, so the selection rule is checked as a property
instead of reproducing a known quirk to make an equality assertion pass. At a
pinned mask the matrices are identical, which is what establishes that the
data encoding, Reed-Solomon, interleaving, placement, format and version
information are all correct.
In the TUI, TOTP entries encode their otpauth:// URI so an authenticator can
consume them; everything else encodes its own value. Secret Mode suppresses
the code entirely — a QR code is the secret in a form a camera reads from
across a room, so the setting that exists to keep secrets off the screen
applies here more than anywhere — and an entry too large to encode says so
rather than looking unavailable.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
46ff51f3d6 |
feat(cli): close the remaining command-surface gaps with the Python CLI
Six commands existed in Python (and several in the TS TUI or API) but had no
TypeScript CLI equivalent:
vault stats — counts by kind, archived, relays; asserts no
secret reaches stdout
vault change-password — re-wraps the parent seed, honouring the
profile's configured kdf_iterations the way the
TUI does. Passwords come from the environment,
never argv: a password on the command line is
in the process list and the shell history.
vault reveal-parent-seed — same egress rule as `fingerprint create`: this
is the one secret that unlocks everything, so
it refuses to print into a pipe (a log, a
transcript, an agent's context) and writes a
0600 file it will not clobber.
entry export-totp — every 2FA secret, written through atomicWrite
for a genuine 0600 inode. Secrets and otpauth
URIs, not codes: a code expires in seconds and
cannot seed an authenticator, so an export of
codes looks correct and is useless. Same bug
that the API tests caught.
config toggle-secret-mode — `config set` could already do this, but only if
config toggle-offline the caller knew the key and that the value is a
JSON boolean. These read and flip.
api stop — POSTs /shutdown; token from SEEDPASS_API_TOKEN
rather than argv, like every other credential.
`api` becomes a group with `start` as the default subcommand, so `api` alone
keeps working.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
348245e14a |
feat(api): port the HTTP API surface to TypeScript
Closes the open "does the api get a TypeScript port at all" decision by
porting the subset that is not built on unported subsystems: entries (list,
search, read, create, modify, archive, links, document import/export), an
explicit secret route, config, profiles, TOTP, vault lock/unlock/export/import,
change-password, relays, nostr pubkey, stats, notifications, derivation check,
and shutdown.
Built on node:http with no framework. The CLI ships as a single audited bundle
with an empty production dependency list, and adding a web framework to a
process that holds unlocked parent seeds would be the largest supply-chain
change in the project.
This surface binds a TCP port while holding an unlocked seed, which is
strictly more exposed than the agent's 0600 unix socket, so it is shaped by
what it refuses:
- Loopback only. A non-loopback bind requires --allow-remote; an accidental
--host 0.0.0.0 on a seed-holding process is not a mistake that should be
recoverable after the fact.
- Bearer token on every route, printed once, never written to disk,
compared in constant time.
- Routes that produce plaintext additionally require the master password in
a header, so a token lifted from a script or a process list cannot read
the vault on its own.
- Path parameters resolve inside the profile directory or 400.
- Body size capped while the request is still arriving.
- Failed unlocks get their own much tighter budget, and exhausting it
returns 429 even for the correct password, so the port is not a password
oracle and lockout is not distinguishable from a wrong guess.
- No CORS headers unless the origin was explicitly allowlisted.
- password_hash / pin_hash are never returned and cannot be set.
Every handler goes through the CLI's own vault plumbing rather than
reimplementing it. That is the point: an API deriving secrets its own way
would be a second implementation to keep in parity, and the first place the
two would drift is which secret a given entry produces. Verified end to end
against a real running server: Python, the TS CLI and the TS API all derive
g'xH9E?3n3VS8EOmX8 for the same entry.
Routes belonging to deliberately unported subsystems (high-risk partitions,
agent job profiles, agent recovery split, semantic index) answer 501 naming
the feature rather than 404 — a 404 reads as a typo and would send an
integrator hunting for a spelling mistake that is not there.
One bug the tests caught before it shipped: the 2FA export returned
materializeSecret's value for deterministic TOTP entries, which is the current
six-digit CODE, not the base32 secret. An authenticator cannot be seeded from
a code, so the export would have looked correct and been useless. It now
re-derives the secret the way the TUI's export does, and returns the otpauth
URI alongside it.
28 API tests, most of them about what is refused rather than what works.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
4b64d26273 |
docs: all seven audit findings closed; record the deferred L-1 v2 design
Four fixed outright (M-1, M-2, L-4, L-7, L-8), one mitigated with the derivation change deliberately deferred (L-1), two surfaced rather than changed because their resolution is frozen for parity (L-2, L-3), and two resolved as documented-behaviour with the reasoning written down (L-5, L-6). The L-1 v2 design is now written up in TODO.md rather than left as a one-line recommendation: keep v1 frozen, add derivation_version alongside the existing gen_version pattern, domain-separate with an HMAC step rather than new BIP-85 app numbers, land both implementations together with cross-impl fixtures, and make the upgrade opt-in per entry. Sequenced after the outside review on purpose. Also recorded: three defects found while fixing, none of which the audit had reached — Python's publish_delta manifest republish, the fetch-limit downgrade path, and merge_index_payloads mutating its argument. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN |
||
|
thePR0M3TH3ANandClaude Opus 5
|
276aca65b6 |
feat(sync): report what a merge resolves silently; stop it editing its inputs (L-2, L-3)
The deterministic merge is frozen for parity -- both implementations must
reach the same vault from the same inputs -- so how it resolves is not up for
negotiation. What was missing is that nobody is told a resolution happened.
L-2: the allocation watermark is per-replica, so two devices working offline
can both allocate id N. The merge then resolves that id by timestamp then
hash, and the loser is replaced with no trace. An entry the user created is
simply absent afterwards. Worse than a plain overwrite, because an id is a
permanent BIP-85 derivation coordinate: the survivor derives from the same
coordinate the discarded entry did (see the L-1 detector).
L-3: past TOMBSTONE_RETENTION_CAP the oldest deletions are forgotten, and
merging a stale replica or old snapshot then resurrects those entries. A
documented protocol limit -- but a user cannot know the cap has actually begun
trimming their history unless something says so.
Both implementations gain an optional MergeReport collector: same-id conflicts
(with the kept and discarded entries named) and a count of evicted tombstones.
`nostr restore` reports both in its summary and on stderr, covering delta
replay as well as the final local merge.
Only genuine divergences are reported. Two replicas holding the same entry
with different timestamps is an ordinary edit and is ignored; flagging it
would bury the case that matters in noise and train the user past the warning.
The collector is write-only and cannot change the merged result -- asserted in
both implementations, because if observing could alter the outcome it would be
a parity divergence rather than an observation.
Two defects found while writing the tests:
- Python's merge MUTATED its `current` argument. `dict(current)` shares the
nested entries dict, so the merge wrote its result straight into the
caller's payload: after merging, "current" had already become the merged
value. Anything comparing before-and-after, or merging one payload twice,
silently saw the wrong input. Now deep-copied, matching TypeScript's
structuredClone. Found because the "reported whichever side wins" test
failed -- the forward merge had corrupted the input for the backward one.
- test_merge_replay_same_payload_is_idempotent was passing only because of
that aliasing: `once` and `twice` were one object. With inputs no longer
mutated, replaying the same payload is shown to normalize entry SHAPE --
the equal-timestamp path unions field sets and writes explicit nulls for
absent optional fields. TypeScript produces byte-identical output for the
same input, so this is shared designed behaviour rather than a Python
regression, and it converges on the second pass. The test now asserts what
is actually true (content unchanged, shape normalized, settles at two) and
records why the old assertion held.
Mutation-verified: removing the recording turns the reporting tests red.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|
||
|
thePR0M3TH3ANandClaude Opus 5
|
75e370d098 |
feat(vault): detect shared BIP-85 derivation coordinates; bind Python imports too (L-1, L-8)
L-1 — three entry kinds derive from BIP-85 app 32 at the entry's index:
ssh@N -> 32 bytes of m/83696968'/32'/N'
pgp@N -> 32 bytes of m/83696968'/32'/N' (the SAME 32 bytes)
password@N -> 64 bytes of the same, then PBKDF2
So ssh@N and pgp@N are byte-for-byte the same Ed25519 private key, and
password@N takes that key as its entropy prefix. The password stays safe (256
unknown bits remain, PBKDF2 is one-way) but this is not key separation, and
SSH keys are exactly what gets exported to a server or uploaded to a forge.
Deliberately NOT fixing the derivation itself yet. Domain-separating the three
uses is a versioned protocol change in both implementations, i.e. a new
derivation path -- and the project's own note is that "a derivation bug does
not throw, it quietly produces a secret that cannot be recovered". Adding one
immediately before the first outside review inverts the point of the review.
The v2 design is written up in TODO.md instead, ready to implement after it.
What ships is the audit's short-term recommendation, which needs no derivation
change: notice the state and say so. findDerivationCollisions /
find_derivation_collisions report cross-kind clashes at one app-32 index,
worst first, in both implementations with identical message text -- a user
moving between the two must not be told different things about one vault.
Wired into every path the state can arrive by: vault import (CLI and TUI, both
implementations), nostr restore's merge, `vault import --inspect` before
anything is written, and a new `util check-derivation` for auditing an
existing vault (exit 1 when it finds something, for scripts).
Neither implementation can create the state -- index == vault id, ids unique,
watermark prevents reuse -- so this is about imported, foreign, and merged
data, which is also why it must never be the thing that fails an import: it
reports, never refuses, and tolerates malformed input.
The lead test in each implementation does not exercise the detector at all.
It derives the keys and compares the bytes, proving the property the detector
reports. Without that, the rest only checks that a written-down rule is
applied consistently, and a derivation change could falsify the rule with
everything still green.
L-8 (Python) — while wiring the above it turned out Python's import_backup had
the same unbound-plaintext-import gap the audit found in TypeScript: the
wrapper carries a `fingerprint` and nothing compared it. Now fixed there too,
with a new ProfileMismatchError so the TUI can offer the override instead of
reporting a generic failure.
The comparison is against the fingerprint DERIVED FROM THE SEED, not the
profile directory's name. In production they are the same string, but the
question worth asking is "was this backup made from my seed?", and only the
seed answers it -- comparing directory names refused a legitimate restore of
the same seed into a freshly named profile, which test_account_roundtrip_
restore caught immediately. It is enforced only when the wrapper records
something that is actually a fingerprint: a backup carrying some other label
offers no evidence about its source seed, and refusing on no evidence blocks
restores without making anyone safer.
All guards mutation-verified on both sides.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN
|