fix(anchor): one transactional writer for identity.json, + witness-key delegation preimage - #128
fix(anchor): one transactional writer for identity.json, + witness-key delegation preimage#128c-1k wants to merge 12 commits into
Conversation
Stage 1 of the Rekor witness-key work (spec §5.2). Fixes three defects that already existed and that the witness-key mint would have depended on. 1. Unserialized read-modify-write. bumpAnchorHighWater and recordRotatedIdentity each did read -> spread -> write with nothing serializing them, so a write that started from a stale read could overwrite a newer lastAnchorSeq. That rolls back the durable high-water, which the anchoring-monotonicity invariant exists to make impossible: re-minting an occupied position in an append-only external store is permanent, unrewritable fork evidence. The same race silently dropped keyHistory entries, stranding records whose signing key nothing could name. Every mutation now goes through updateAnchorIdentity, which takes the emitter's advisory lock, re-reads UNDER the lock, merges, and writes. heldLock is an explicit parameter rather than inferred from the in-process lock set: inferring it would let a caller that forgot to lock ride on an unrelated component's lock, and that failure is invisible. 2. Shared temp filename. identity.json.tmp-<pid> collided between two writers in one process, so one could rename a file the other was still writing and publish a torn identity that still parses as JSON. Now carries random bytes. 3. No directory fsync. The bytes were fsync'd but the rename was not, so "persisted before we act on it" was false across a crash. POSIX requires syncing the containing directory; best-effort by platform. Merge semantics are union-and-monotonic, never last-writer-wins: max lastAnchorSeq, union of key histories by keyId, and a refusal rather than a winner on a vaultId conflict, since a wrong vaultId re-homes every signed record in the vault. A no-op mutation writes nothing, so the emission path does not burn an fsync pair on every anchor. Also adds AnchorIdentity.witnessKeyHistory and the WitnessKeyEntry type (delegation signature, delegating root epoch, monotonic delegationIndex, and the anchorSeq range that makes revocation expressible), and extracts the AC-6.2 refusal into refuseKeyInsideVault so the anchor key and the witness key are governed by one copy of the rule rather than two that can drift. No behaviour change to publishing. Spec: docs/superpowers/specs/2026-08-17-rekor-witness-key-design.md Signed-off-by: Cam <cam@camwhiteus.com>
…o NOT prove Five tests for the single transactional writer, each mutation-verified rather than assumed. Removing the lock acquisition fails two of them; that is the guard these exist for. The finding worth recording is the negative one. Two of the five survived deleting mergeIdentity's monotonic max AND deleting its keyHistory union outright — the suite stayed green with both guards gone. The reason is structural, not an oversight in the tests: updateAnchorIdentity hands mutate() a copy re-read UNDER the lock, so no public caller can produce a stale proposal, and the merge is a no-op on every path that currently exists. So the live protection against a rolled-back lastAnchorSeq is the re-read, not the merge. The merge stays as defense in depth for a future caller that computes a proposal from an earlier capture (Stage 2's witness-key mint is the obvious candidate) and because the failure is unrecoverable — a lowered high-water re-mints an occupied position in an append-only store. But it is now documented at mergeIdentity as currently unreachable, and the two tests are retitled to claim only what they pin: that a rotation does not DROP the high-water or the superseded key. A guard nobody can reach is not evidence, and a test that passes with the guard deleted is not evidence for that guard. The resume test signs a real record with the vault key. An unsigned one was rejected by resume's own validation before reaching the lock, which would have made the test pass for a reason unrelated to what it claims to check. Full suite green: 4372 passed, 14 skipped (the documented openclaw contract skips, proven in the openclaw-contract CI job). Typecheck and biome clean. Signed-off-by: Cam <cam@camwhiteus.com>
Stage 2a of the Rekor witness-key work (spec §3.3). Adds the exact bytes a
witness-key delegation is signed over, in BOTH copies of anchor-verify.ts,
because the verifier recomputes this preimage to answer "is this witness key
root-delegated?" and a verifier importing core's copy would be checking the code
it exists to check independently.
The delegation is what closes the throwaway-key attack: mint a one-off P-256
key, submit an authentic payload hash to Rekor under it, present the valid
receipt, delete witnessKeyHistory — every other check passes and the canonical
index an auditor would enumerate is empty.
Every variable-length field carries a u32be BYTE-length prefix. Without them the
fields concatenate ambiguously and two different delegations share one preimage,
so one root signature authorizes both. That is reachable rather than theoretical:
vaultId is only ever validated as a non-empty string, never as a UUID, so it is
attacker-chosen. Byte lengths, not code units, so another language's
implementation derives identical bytes instead of diverging silently on
multibyte input.
The open-ended range sentinel is 2^53-1, not the 2^64-1 the u64 encoding
suggests. 2^64-1 is not representable as a JS number and this codebase validates
every parsed integer with Number.isSafeInteger, so that sentinel would fail its
own validation and could not survive the JSON round-trip between what was signed
and what is stored. The FIELD is still encoded u64be for cross-implementation
clarity; the VALUE stays safe.
Returns null rather than throwing on anything it cannot encode exactly. This runs
on untrusted input on the verification side, where throwing out of a checking
function is itself the defect.
Six tests, and the mutation testing is what makes them worth anything: dropping
the length prefixes fails the ambiguity case. Three further mutants were
DISCARDED as invalid — they broke module loading, so all six tests failed with
"witnessDelegationPreimage is not a function", which looks like maximum
sensitivity and proves nothing. A mutant that stops the code compiling tests the
harness, not the guard.
packages/verify LOC tripwire raised 9900 -> 10200, in the test (the authority)
and AGENTS.md together, with the accounting the guard's own comment requires:
~100 mirrored lines, node:buffer only, nothing vendored, and assertions 4 and 5
(imports are node:*/relative, dependencies is {}) re-verified and still passing.
Signed-off-by: Cam <cam@camwhiteus.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ee65614db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A vault that had NEVER been witnessed verified byte-identically to one that
had, and reported ANCHORED_VERIFIED while doing it. verifySuppliedRekorReceipts
returns null when no receipts are supplied, so rekorFailed stayed false and the
verdict was untouched. Nothing anywhere said "this evidence was never present".
That is how this repo's Rekor sink stayed structurally non-functional for six
months without a single verdict noticing, and it is the worst defect class
available in a product whose proposition is verifiable receipts: a verifier
reporting a guarantee it does not have. It is also independent of whether the
transparency-log path ever works, which is why it lands first.
Adds anchoring.witnessLog, ALWAYS present — unlike the optional rekor block. An
absent field is how the absence stayed invisible; a state that must be rendered
is how it stops being.
THE FOLD IS OVER ANCHORS, NOT RECEIPTS. Folding over receipts lets an anchor
with no receipt contribute nothing and disappear, so nine covered anchors beside
one unwitnessed one read as fully verified. Every anchor contributes exactly one
outcome and WITNESS_VERIFIED requires all of them covered. An empty fold is
UNKNOWN rather than VERIFIED, because zero anchors makes "all anchors covered"
trivially true — the same vacuous truth one level up. A failing receipt outranks
a passing one for the same anchor: a forgery beside a good receipt is evidence
of an attempt, not noise to discard.
exitCodeForAnchored gains requireWitness, opt-in exactly like requireAnchor and
requireExternalAnchor, so default exit codes are unchanged and a vault that
verified clean yesterday still does. It fails CLOSED when witnessLog is absent
entirely: an older result shape must not satisfy a witness requirement by
omission.
Reachability is stated in the code rather than implied: only WITNESS_VERIFIED,
WITNESS_UNKNOWN and WITNESS_INVALID can currently be produced. ABSENT, UNPROVEN
and PARTIAL need the emission-time sink declaration that distinguishes "should
have been witnessed" from "never claimed to be", which is not built yet. They
are declared because they are the spec's lattice, and marked unimplemented so
nobody reads the type as the feature.
Mirrored into packages/verify — the differential suite caught the omission
immediately, which is the parity contract working: core and the standalone
verifier must produce identical verdicts, and a verifier that under-reports the
witness state is exactly the divergence that contract exists to prevent.
One self-inflicted find worth recording: the AC-2.2 import guard flagged
"index.ts: was never meant to be witnessed" — my own doc comment contained
`from "` and the scanner is a text matcher, not a parser. AGENTS.md warns about
this for the file-diff rule ("do not introduce one") and the same hazard applies
here. Reworded rather than exempted.
Full suite 4388 passed, 0 failed. Typecheck clean.
Signed-off-by: Cam <cam@camwhiteus.com>
Added: G5 — report the absence of witnessing (commit 4)Scope note: this branch now carries a fourth commit that is a feature, not a bug fix. Prioritised over the remaining Rekor wire work after the assessment below; happy to split it into its own PR if reviewers prefer the bug fixes isolated. The defectA vault that had never been witnessed verified byte-identically to one that had, and reported That is how the Rekor sink stayed non-functional without a single verdict noticing, and it is independent of whether the transparency-log path ever works — which is why it lands ahead of the wire fix. What changed
The fold is over anchors, not receipts. Folding over receipts lets an anchor with no receipt contribute nothing and disappear, so nine covered anchors beside one unwitnessed one read as fully verified. Every anchor contributes exactly one outcome. An empty fold is
Reachability, stated rather than impliedOnly VerificationMutation-verified, four valid mutants:
Full suite 4388 passed, 0 failed. Typecheck and biome clean. The differential suite caught the |
Codex Review (local CLI, max effort) — REQUEST CHANGESCertifying round, first Codex read of these 4 commits. Max effort was a deliberate per-round choice, not a default. Review gate: request changes. I found four high-severity and three medium-severity defects. Findings
Explicit category checks
Focused Vitest could not start because the read-only environment blocked Vite’s Findings accepted; all seven are legitimate. Fixes to follow on this branch. |
All verified independently before fixing, not taken on the reviewer's word. F1 (high) — the delegation preimage was NOT injective over accepted vaultIds. "\uD800" and "\uD801" are distinct, non-empty JavaScript strings that both encode to U+FFFD, so Buffer.from produced IDENTICAL preimage bytes and one root signature would have authorized two different delegations. Measured: both give efbfbd. vaultId is only ever validated as non-empty, never as a UUID, so it is attacker-chosen. Every string field now has to survive a UTF-8 round-trip — the exact property required, since length prefixes protect nothing if the bytes they measure no longer distinguish their inputs. A well-formed astral pair still encodes. F2 (high) — a partial anchor history passed --require-witness. anchorSeq is 1-based and contiguous, so a vault whose newest anchor is 10 asserts that 1..9 existed; folding over only the records in hand returned WITNESS_VERIFIED with nine anchors unaccounted for. This is the same vacuous truth the fold was written to prevent, one level up: there the missing thing was an anchor with no receipt, here an anchor with no record. Implied-but-absent positions now count as unknown, and `anchors` reports the IMPLIED history length so the three counts always sum to it and a gap cannot hide in the difference. partial-history stays legitimately accepted for anchorState — it is simply not a witness claim. F3 (high) — resumeAnchorMirror MUTATED THE MIRROR AND THEN THREW. It appended to anchors.jsonl and only afterwards bumped the high-water, which is where the lock was taken. A resume racing a live emission left the caller with a refusal and the state already changed, so a concurrent emitter holding the tail it read a moment earlier could mint the same anchorSeq: permanent, unrewritable fork evidence. The lock is now taken before anything is touched, which also closes the check-then-act between "mirror is behind" and the append. My own test asserted only the throw and missed this entirely; it now asserts the mirror bytes and the high-water are unchanged. F5 (medium) — the directory-fsync catch swallowed EVERY error as "platform unsupported", including EIO, ENOSPC and EDQUOT, so a real durability failure returned success and callers acted on an update a crash could undo. Narrowed to the errnos that actually mean "cannot open a directory for fsync"; everything else propagates. My own comment claimed the narrow behaviour the code did not have — prose ahead of code, in a commit whose message called that class out. F7 (medium) — witnessDelegationPreimage's "returns null, never throws" contract was false. 2**32 IS a safe integer, so it passed validation and then writeUInt32BE threw ERR_OUT_OF_RANGE on the untrusted verification path. Bounded to u32. Regression tests for each; F2 also pins that a contiguous fully-covered history is still VERIFIED, so the guard cannot make the good case unreachable. Not fixed here: F4 (same-PID stale lock reclaim via an uncanonicalized registry path) is PRE-EXISTING in tryAcquireAnchorLock and wider than this branch, and F6 (neither CLI parses --require-witness or renders witnessLog) is real and is the human-facing half of G5. Both are tracked and addressed separately rather than silently folded in. Full suite 4394 passed, 0 failed. Typecheck and biome clean. Signed-off-by: Cam <cam@camwhiteus.com>
Fixes for the certifying round — 5 of 7 addressedEvery finding was verified independently before fixing, not taken on the reviewer's word. Measured confirmations: lone surrogates both encode to
F2 deserves calling out: it is the same vacuous truth this fold was written to prevent, one level up. There the missing thing was an anchor with no receipt; here it was an anchor with no record. A vault whose newest anchor is seq 10 asserts 1..9 existed, and folding over only the records in hand returned F3 also exposed a gap in my own test. It asserted only the throw, so a refusal that had already appended to the mirror passed as correct. It now asserts the mirror bytes and the high-water are unchanged. F5 was prose ahead of code in a commit whose message calls that class out — the comment claimed "platform does not support directory fsync" while the catch swallowed everything. Deliberately not folded in
VerificationMutation-verified, sane failure counts (a 100% failure rate would indicate a broken module, not a sensitive suite):
Full suite 4394 passed, 0 failed. Typecheck and biome clean. |
The human-facing half of G5, and the half that actually matters to an operator.
The library has reported the witness state since the first commit, but neither
shipped CLI rendered it or parsed a gate for it — so `usertrust verify` still
printed "VERIFIED (externally anchored)" and exited 0 on a vault whose
transparency-log leg had never run, with nothing on screen saying so. The silent
success survived in the one place a person actually looks, which is why the
earlier claim that G5 "closes" this was overstated.
Both CLIs now:
- ALWAYS print a witness line on the anchored path, including — especially —
when nothing was witnessed. Absence is the case the line exists for.
- accept --require-witness, opt-in exactly like --require-anchor and
--require-external-anchor, so default exit codes are unchanged.
- count --require-witness as an anchor-mode trigger, so asking for the gate is
enough to get the anchored path rather than silently doing nothing.
The core CLI's --json `success` is no longer `result.valid` alone. A witness gate
can exit 1 while the chain itself is valid, so a consumer reading the body rather
than $? was told the run succeeded while the process failed — a second silent
success, one layer out from the one this commit removes.
Three tests, each mutation-verified: the witness line appears on an anchored
vault with no receipts; --require-witness exits 1 on that same vault while the
DEFAULT still exits 0 (the additive property); and --json does not report
success:true when the gate fails.
One note on the first test, because it nearly passed for the wrong reason: it
originally used a fixture property that does not exist, so `--anchors` got no
value, anchor mode never engaged, and the assertion failed for a reason
unrelated to the code under test. Fixed to `storeFile` — a test that reaches a
different path than the one it names proves nothing about that path.
Full suite 4397 passed, 0 failed. Typecheck and biome clean.
Signed-off-by: Cam <cam@camwhiteus.com>
F6 fixed — the witness state is now visible to an operatorThis was the half that actually mattered. The library had reported the witness state since the first commit, but neither shipped CLI rendered it or parsed a gate for it — so Both CLIs now:
The core CLI's Verification
Full suite 4397 passed, 0 failed. Typecheck and biome clean. Worth flagging honestly: the first version of the witness-line test used a fixture property that does not exist, so Remaining from the certifying roundF4 only (same-PID stale lock reclaim via an uncanonicalized registry path). Pre-existing in 6 of 7 findings addressed. |
R1 (high) — A FLAG WAS BEING CONSUMED AS ANOTHER FLAG'S VALUE. Both CLI parsers took the next token unconditionally, so `--vault-id --require-witness` ate the gate as the vault id: the run then verified UNANCHORED, printed no witness line, and exited 0. A strict flag silently disarmed by an adjacent flag is the worst shape available for a CI gate — the pipeline stays green while checking nothing, and this was in code added to REMOVE a silent success. Both parsers now refuse a value beginning with "-" and support the `--flag=value` escape, mirroring `requireValue` in cli/budget.ts (AGENTS.md names that pattern and the reason for the escape: refusing dash-leading values without one makes legitimate ids unpassable). R2 (medium) — MY OWN F5 FIX INTRODUCED THIS. The high-water bump sits between the mirror append and trackPublish, so once the identity fsync stopped being swallowed, a genuine I/O failure there skipped publication entirely. The self-heal only re-appends orphans AHEAD of the mirror tail, so a record that reached the mirror but never reached a sink was invisible to both paths: `anchor now` returned no-new-events and the CLI exited 0 saying nothing needed anchoring, while the record sat undelivered forever. Making one silent failure loud created a quieter one downstream. Fixed by DRAINING the outbox before declaring nothing to do — the outbox is the delivery intent, so anything still in it needs delivering — rather than re-ordering the durability sequence, which is pinned for its own reasons. R3 (medium) — F5 WAS STILL WRONG. Suppressing EACCES and EPERM globally kept real POSIX failures silent: the reviewer reproduced a directory whose write and rename succeed while opening it for fsync returns EACCES, and identity init still reported success with the rename not durable. Now platform-gated — suppressed only on win32, which cannot open a directory this way at all — so every POSIX error propagates. R4 (medium) — F6 WAS INCOMPLETE. The standalone `--tx` branch printed an affirmative receipt and left through the gate with nothing on screen about the transparency log; under --require-witness it exited 1 beside a receipt that still read as a pass, so the operator saw the affirmation and not the reason. The renderer is now a single function called on every path that reports an anchored verdict. Round-2 verification of the earlier fixes, for the record: F1 fixed (all 2,048 lone surrogates rejected in every string field, both implementations), F2 fixed (exhaustive histories through seq 14 found no unaccounted WITNESS_VERIFIED), F3 fixed (lock precedes mutation; every post-acquisition throw releases via finally), F7 fixed. F4 confirmed PRE-EXISTING and byte-identical to origin/master — the branch broadens the state the lock protects but does not increase its severity. Full suite 4399 passed, 0 failed. Typecheck and biome clean. Signed-off-by: Cam <cam@camwhiteus.com>
The R3 fix shipped UNTESTED and I only found that by mutating it: removing the platform gate left all 253 anchoring tests green. A fix with no test that fails without it is not verified, whatever the diff looks like. Uses the reviewer's own reproduction rather than a mock: mode 0o300 gives a directory that is writable and renameable but cannot be OPENED for read, so the directory fsync fails with EACCES exactly as it would in the field. Suppressing EACCES globally — the first cut of the F5 fix — meant the write returned success with the rename not durable. The assertion is deliberately precise about what the throw means, because my first version of it was wrong. The rename SUCCEEDS and the new identity IS on disk; only the fsync fails. This is not a rollback. What the throw buys is that nobody is told the write was durable when it was not — a crash at that point could still restore the previous identity and high-water, and the caller now learns that instead of proceeding on a false guarantee. Asserting the old keyId survived would have been asserting a rollback that does not happen. Mutation-verified: removing the platform gate now fails this test and only this test. Signed-off-by: Cam <cam@camwhiteus.com>
Codex Review round 2 (local CLI, max effort) — REQUEST CHANGESVerified by locating the verdict block, not by exit status: the first attempt died at exactly 10 minutes with 610KB of output and no verdict, so it did not count and was re-run detached. REQUEST CHANGES — 4 findings
Fix verification:
Clean checks:
All four fixed in
|
…nd-2 fixes Round 3 found 5. Three are in code round 2 added, which is the part worth recording: each round of fixes has introduced its own defects, and this one includes a REGRESSION of documented working behaviour. F3 (medium) — THE REGRESSION, and the worst of the five. Closing the `--vault-id --require-witness` hole by rejecting every leading-dash value also rejected a bare "-", which MEANS STDIN, is documented in packages/verify/README.md, and is special-cased by readArtifact. `--anchor -`, `--bundle -` and `--rekor-receipts -` all stopped working. A security fix that silently deletes a working documented feature is not a fix. Now rejects FLAGS, not dashes. I would have caught this by reading the README for the flags I was changing; I read the attack instead. F1 (high) — my R2 drain fixed the delivery attempt and left the REPORTING lying. `emit()` still returned "no-new-events", which the CLI EXEMPTS from a non-zero exit, so a permanent sink failure printed "Nothing to anchor" and exited 0 with records still pending. Now returns a distinct `outbox-pending (N undelivered; retrying)` reason so the exemption cannot swallow it. Fixing the mechanism and leaving the report is the same silent success one layer out. F2 (medium) — my R2 drain double-published. publishRecord ALREADY drains the whole backlog oldest-first; queuing one trackPublish per pending record made each queued call re-drain everything, so [1,2] delivered as [1,2,2] and every failing cycle grew the work queue. Now one call on the oldest record. F4 (medium) — my R3 platform gate was still too broad. win32 alone suppressed EIO, ENOSPC, ENOENT and EDQUOT under cover of "unsupported operation". Platform AND errno allowlist now. F5 (low) — `--pubkey=` was accepted as an empty value and handed "" to readFileSync, producing an uncaught stack trace. An empty inline value is a typo, not an empty path. Tests pin the regression and the arity case — the two that would have caught F3 and F5 before review. Full suite 4402 passed, 0 failed. Round-3 clean checks, recorded because they are load-bearing: embedded "=" values, repeated flags, index advancement and existing `--` behaviour unchanged; every successful anchored `--tx` path prints the witness line (not-found and invalid exit 2/1 before it, which is correct); F1/F2/F3/F7 from round 1 remain intact. Signed-off-by: Cam <cam@camwhiteus.com>
Round 4 was killed before a verdict, but reproduced two more defects IN THE DRAIN I added in round 3: a corrupt outbox entry still produced exit-0 "no-new-events" (pendingOutboxRecords silently drops unparseable files, so the drain saw nothing pending), and a SUCCESSFUL retry could still exit 1 because the reason string was computed before the publish settled. That is two consecutive rounds of defects in one mechanism, which is a signal about the mechanism rather than about the last patch. So the drain is gone, replaced by a one-line reordering that addresses the actual cause. The stranding existed because bumpAnchorHighWater sits between the mirror append and trackPublish and CAN THROW — once a real directory-fsync failure stopped being swallowed, a throw there skipped publication and left a minted record in the outbox with nothing to retry it. Scheduling publication BEFORE the bump makes the stranding impossible rather than detectable afterwards, and the throw still propagates. The durability order is untouched: trackPublish writes nothing durable, it only queues a network publish, so outbox -> mirror -> high-water still holds. Verified publishRecord has no dependency on the high-water. Both drain defects disappear with the drain, rather than needing a third fix in the same sub-area. Pinned STRUCTURALLY, deliberately: the behavioural test needs a real fsync failure mid-emission, and the drain that tried to cover this behaviourally is exactly what produced two rounds of defects. Ordering is the whole fix, so ordering is what the test asserts — including that both still sit after the mirror append. Mutation-verified: swapping the order back fails it. Signed-off-by: Cam <cam@camwhiteus.com>
…rotated vault A P1 I introduced, found on the GitHub connector review surface — a surface I never checked across four local Codex rounds. Recording that, because the round count I had been reporting as evidence of rigor never included it. THE DEFECT. Making identity.json's writer transactional turned a previously unconditional write into a non-blocking one that throws when contended. Rotation cannot survive that: `emitter.rotate()` has ALREADY appended the cross-signed successor to the mirror and RELEASED its lock before recordRotatedIdentity runs. Lose that race and the mirror names the successor while identity.json still names the predecessor — the old signer is then rejected by the epoch guard, and re-running rotation cannot repair it because the successor is already minted. The comment at the CLI call site says identity.json MUST advance in lockstep; my change is what broke exactly that. THE FIX. A `mustSucceed` caller waits rather than refusing on first contention, bounded at 15 x 100ms, and if it still cannot acquire it throws a message NAMING THE REPAIR rather than leaving an operator with a desynced vault and no next step. Every other caller is unchanged and still refuses immediately. TWO THINGS THE TESTS NOW COVER THAT THEY DID NOT. The first cut used 25 x 200ms, which made the test that exercises it run 4987ms against vitest's 5000ms timeout — 13ms of margin. A guard whose own test flakes under load is not a guard you can rely on. 1.5s is still generous for an emission; an emission that has not cleared by then is pathological, which is what the repair message is for. And the existing test only proved the FAILING direction. A guard shown only to refuse can be one that never goes green — two lanes shipped exactly that defect tonight. Added the positive case: rotation completes normally when nothing holds the lock, and does not burn the retry budget getting there. Full suite green. Typecheck and biome clean. Signed-off-by: Cam <cam@camwhiteus.com>
Mutation showed the previous pair proved nothing about retrying: reverting to refuse-on-first-contention, and removing the sleep entirely, both left every test green. They pinned the repair MESSAGE and the uncontended path and never the wait itself. This one holds the lock as a file owned by pid 1 (alive, foreign, so every attempt is refused) and has a detached shell remove the FILE mid-wait, so a later attempt's O_EXCL create succeeds. Asserts both that rotation completed and that it waited >150ms rather than walking straight in. The first version used a short-lived child and went stale on its pid. It failed for a reason unrelated to the code: this thread's wait is synchronous, so Node never handles SIGCHLD while blocked, the exited child stays a ZOMBIE, and kill(pid, 0) keeps reporting it alive. The harness was wrong, not the guard. Signed-off-by: Cam <cam@camwhiteus.com>
Connector review threads — all four addressed, resolvingI missed this surface entirely for four local Codex rounds. These threads are invisible to both
On the u32 thread not being marked outdatedIt anchors to line 401, the The rotation P1 was mine, and it was a second-order effect of an earlier fixMaking Mutation-verified after three failed attempts at testing it honestly: the first budget put its own test 13ms inside vitest's 5s timeout; the next pair of tests survived both mutants (pinning the error message and the uncontended path, never the waiting); and the first real retry test failed for a reason unrelated to the code — a synchronous wait blocks Node's Resolving these four. Reopen any that I've read wrong. |
Merge bar-check — does not clear. Two of three gates pass; the head is ungated.Run at ✅ 1. Job-level CI at head — 7/7✅ 2. Connector review threads — 4 total, 0 unresolvedAll four resolved on 2026-08-22 with per-thread fix commits and verification. ❌ 3. Gated SHA == head — it is not, and the gap is four commitsThe last Codex verdict posted to any surface on this PR is round 2, 2026-08-19T22:47Z, whose fixes were
Three of those four change anchoring/locking behaviour, and one deletes a mechanism. None of them has a posted review. Round 3's verdict does not exist on this PR. That is this repo's own documented failure mode, from
So round 3's findings cannot be read, and its disposition cannot be checked. The 2026-08-22 comment is thorough, but it addresses the connector threads — a different surface — and does not stand in for a CLI verdict at head. This is also the worst place to carry an unreviewed gap: every one of those four commits is remediation of an earlier finding, and What clears itOne max-effort round at Separately: the delegation preimage cites a spec that cannot be retrieved
That spec has never been committed to any branch of this repo — So a reader of the public repo cannot obtain the normative definition of the bytes this function produces. That is the same defect class as #139 item 2 (an unretrievable normative input), now present in code rather than spec. Suggestion, for whoever gates this: the three |
Three pre-existing defects in the anchoring identity path, plus the first piece of the Rekor witness-key work. All three bugs are live on
mastertoday.The fixes (commit 1)
identity.jsonhad no single writer.bumpAnchorHighWaterandrecordRotatedIdentityeach did an unserialized read → spread → write:lastAnchorSeq. That is the anchoring-monotonicity invariant — re-minting an occupied position in an append-only external store is permanent, unrewritable fork evidence. The same race could silently dropkeyHistoryentries, stranding records whose signing key nothing could name.identity.json.tmp-<pid>, shared between two writers in one process, so one could rename a file the other was still writing and publish a torn identity that still parses as JSON.fsync'd but the containing directory was not, so "persisted before we act on it" was false across a crash.Every mutation now goes through
updateAnchorIdentity: take the emitter's advisory lock, re-read under the lock, merge, write atomically.heldLockis an explicit parameter rather than inferred — inferring it would let a caller that forgot to lock ride on an unrelated component's lock, and that failure is invisible.Behaviour change worth flagging in review:
usertrust anchor resumenow fails with "locked by an in-flight emission" if an emission is running, where it previously wrote regardless. Fail-closed, but operator-facing.What the tests do and do not prove (commit 2)
Five tests, each mutation-verified. Removing the lock fails two of them.
Recorded honestly in the code: two of the five survive deleting
mergeIdentity's monotonic max and its key-history union outright. That is structural, not an oversight —mutate()is handed a copy re-read under the lock, so no current caller can produce a stale proposal and the merge is a no-op on every existing path. The live protection is the re-read, not the merge. The merge stays as defence in depth for a caller that computes a proposal from an earlier capture, and is documented at the function as currently unreachable. A guard nobody can reach is not evidence.Delegation preimage (commit 3)
The bytes a witness-key delegation is signed over, mirrored byte-identically into
packages/verifybecause the verifier must recompute it without importing core.Every variable-length field carries a
u32bebyte-length prefix. Without them two different delegations share one preimage, so one root signature authorizes both — reachable rather than theoretical, sincevaultIdis only ever validated as a non-empty string, never as a UUID.The open-ended sentinel is
2^53-1, not the2^64-1the u64 encoding suggests:2^64-1is not representable as a JS number and would fail this codebase's ownNumber.isSafeIntegervalidation, so what was signed and what is stored would differ.packages/verifyLOC tripwire raised 9900 → 10200 in the test (the authority) andAGENTS.mdtogether, with the accounting its comment requires. Assertions 4 and 5 — imports arenode:*/relative,dependenciesis{}— re-verified and still passing.Verification
npm run typecheckcleanbiome checkclean🤖 Generated with Claude Code