feat(ledger): sign validator verdicts with offline-verifiable receipts - #857
feat(ledger): sign validator verdicts with offline-verifiable receipts#857tomjwxf wants to merge 3 commits into
Conversation
Closes the-open-engine#464. Validator verdicts are the one ledger record a third party may be asked to rely on. They are stored unsigned, so anyone who can write the database can change the answer afterwards and nothing shows it. This signs each verdict when recorded and chains it to its predecessor, so editing, deleting or reordering is detectable from the exported receipts and a published keyring alone. Built to the three properties requested in the issue triage rather than the original proposal: - Provider independent. Signing happens where verdicts are recorded, so it behaves the same whether the backend is Claude Code, Codex, Gemini or a local model. No dependency on any vendor hook mechanism. - Explicit key trust and rotation. Keyring with validity windows and revocation. Key ids derive from the public key so a kid cannot be reassigned. Revocation is not retroactive, since the alternative voids honest history every time an operator rotates; revoked_at exists for the compromise case. - Offline verification. Receipts plus a keyring, no cluster, no network. Ed25519 via node:crypto. No new dependencies. Purely additive: no existing file is modified. The store attaches to the ledger's existing topic:VALIDATION_RESULT event, so a deployment that never constructs one behaves exactly as today, with no table and no signing. Signature validity and sequence integrity are reported separately. A deleted verdict leaves every remaining signature genuine, so signature checking alone reports success while the chain reports ALTERED. The decision is read from content.data.approved, the same field the existing state reducer uses, and must be a real boolean. Truthy stand-ins cause a refusal rather than a signed guess, and findUnsignedVerdicts surfaces any verdict that went unsigned. 39 new tests, most of them adversarial: tampering, wrong-key signing under a claimed kid, out-of-window signing, revocation ordering, deletion, reordering, and cross-domain signature replay.
| return { | ||
| type: 'zeroshot.verdict_chain_verification.v1', | ||
| receipts_checked: receipts.length, | ||
| signatures_valid: signaturesValid, | ||
| signatures_invalid: receipts.length - signaturesValid, | ||
| chain_intact: !chainBroken, |
There was a problem hiding this comment.
Suffix truncation still verifies
When the final receipt or the entire receipt set is deleted before export, verification checks only the remaining internal links and has no trusted terminal hash or expected count, causing the shortened evidence to be reported as VERIFIED. How this was verified: The verifier treats an empty or suffix-truncated receipt array as valid whenever every supplied signature and predecessor link passes.
| `), | ||
| listForCluster: this.db.prepare(` | ||
| SELECT receipt_json FROM verdict_receipts | ||
| WHERE cluster_id = ? | ||
| ORDER BY timestamp ASC, rowid ASC |
There was a problem hiding this comment.
Timestamp sorting breaks chain order
When recordVerdict(message) receives an older timestamp after a newer receipt has already been stored, it chains to the latest stored receipt but list() later sorts both receipts by timestamp, causing genuine untampered evidence to be reported as Sequence ALTERED.
Greptile SummaryThis PR adds offline-verifiable, Ed25519-signed validator verdict receipts.
Confidence Score: 4/5The PR is not yet safe to merge because receipt suffix deletion can still verify successfully and timestamp-based ordering can reject honest chains. The verifier has no trusted count or terminal commitment, so removing the newest receipts—including all receipts—still yields a valid result; separately, the store orders receipts by message timestamp rather than insertion order, allowing a directly recorded older-timestamp verdict to make genuine evidence appear altered. Files Needing Attention: src/verdict-receipts.js, src/verdict-receipt-store.js
|
| Filename | Overview |
|---|---|
| src/verdict-receipts.js | Adds receipt canonicalization, Ed25519 signing, key trust-window checks, chain verification, and offline bundle construction; terminal completeness remains unresolved. |
| src/verdict-receipt-store.js | Adds opt-in SQLite persistence and ledger-event signing; receipt ordering still relies on timestamps rather than signing order. |
| scripts/verify-verdict-receipts.js | Adds a standalone verifier with independent-keyring support and human-readable or JSON reports. |
| tests/unit/verdict-receipts.test.js | Adds adversarial coverage for canonicalization, signatures, key lifecycle, chain mutation, payload validation, and export behavior. |
| tests/unit/verdict-receipt-store.test.js | Adds integration coverage for ledger attachment, refusal behavior, tampering detection, unsigned verdict discovery, and offline export. |
| docs/signed-validator-verdicts.md | Documents trust semantics, key custody, attachment modes, verification usage, and stated limitations. |
Sequence Diagram
sequenceDiagram
participant L as Ledger
participant S as VerdictReceiptStore
participant DB as Receipt database
participant E as Exporter
participant V as Offline verifier
L->>S: VALIDATION_RESULT
S->>DB: Read predecessor receipt hash
S->>S: Build and sign verdict payload
S->>DB: Store signed receipt
E->>DB: List cluster receipts
E-->>V: Bundle plus independent keyring
V->>V: Verify signatures and predecessor links
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/signed-val..." | Re-trigger Greptile
tomdps
left a comment
There was a problem hiding this comment.
Thanks for reworking this around the earlier feedback and for being explicit about the trust limits. The implementation and adversarial tests are thoughtful, but I can't approve this contract yet.
Three things need to change before this can merge:
- The offline verifier needs a trusted completeness anchor (for example a signed manifest/checkpoint containing the expected count and terminal receipt hash). Internal predecessor links cannot detect suffix deletion or an empty bundle, so the current result can say VERIFIED after the newest receipts are removed.
- Chain order must come from durable append/signing order, not message timestamps. An older timestamp recorded later currently makes honest evidence look altered.
- This needs to be integrated at the authoritative verdict persistence boundary. As submitted, the store is opt-in and no product path constructs it, while event attachment leaves a non-atomic verdict/receipt gap. That is useful library code, but not a delivered Zeroshot guarantee.
I'm leaving #464 open because the capability is still worth pursuing. Please resolve the trust/completeness contract there first if the atomic persistence change makes this substantially larger; I would rather review the right design than ask you to patch around the current shape.
|
All three findings are correct and I am not going to argue any of them. I reproduced the two concrete ones against this branch before replying: The suffix case is the one that matters. I led this PR with a middle deletion being caught and documented that a chain cannot prove you were given the beginning, but I never considered the end, which is the easier attack and the likelier one. Your third point is the one I had backwards. I optimised for touching no existing files and presented that as a strength. Zero files modified also means no product path uses it, which makes it library code rather than a guarantee Zeroshot delivers. Marking this draft rather than patching around the current shape. Design proposal in #464 covering the completeness anchor, durable ordering, and the persistence boundary, with the anchor question first since that is the part your review left open and the part that decides whether the rest is worth having. |
Closes #464.
Thanks for the triage on 29 July. It named three things an implementation would need, and this is built to those rather than to my original proposal in the issue, which leaned on Claude Code hooks and on a package I maintain. Neither is a dependency here.
The problem
Validator verdicts are the one ledger record a third party may later be asked to rely on: was this change independently validated before it shipped? They are stored unsigned in
messages, so anyone who can write the database can change the answer afterwards and nothing in the ledger shows it.What this does
Signs each verdict when it is recorded and chains it to its predecessor, so editing, deleting or reordering is detectable by someone holding only the exported receipts and a published keyring.
Provider independent. Signing happens where verdicts are recorded, so it behaves identically whether the backend is Claude Code, Codex, Gemini or a local model. No vendor hook mechanism, no external service.
Explicit key trust and rotation. Keys live in a keyring with validity windows and revocation. A verifier resolves the key a receipt names and checks the receipt falls inside that key's window. Two decisions worth arguing with:
kidcannot be reassigned and always checks against the key it names.revoked_atexists for the compromise case, where you do want everything from that moment onward to fail.Offline verification. Receipts plus a keyring. No cluster, no network, no service.
No new dependencies. Ed25519 via
node:crypto.Purely additive
No existing file is modified.
git statuson this branch shows six new files and nothing else.The store attaches to the ledger's existing
topic:VALIDATION_RESULTevent, so a deployment that never constructs one behaves exactly as today: no table, no signing, no cost. A test asserts that the receipts table is not created unless a store exists.The case this exists for
A deleted verdict leaves every remaining signature genuine, so signature checking alone reports success:
Signature validity and sequence integrity are reported separately on purpose. They are different failures calling for different responses, and collapsing them into one boolean would hide the distinction that makes the chain worth having. One break is reported once rather than cascading into every later receipt.
Refusing rather than guessing
The decision is read from
content.data.approved, the same fieldapplyValidationResultalready treats as the verdict, and it must be a real boolean."yes",1,"true"and a missing field all cause a refusal.An unsigned verdict is a visible gap, and
findUnsignedVerdicts()surfaces it. A confidently signed wrong one is worse, because the signature would verify perfectly over the wrong answer forever.Refusals go to an
onErrorcallback rather than throwing, since this runs inside the ledger'semitand a throw would surface as the failure of an unrelated append.Known limitation, stated rather than hidden
Event attachment writes the receipt immediately after the verdict row, not inside the same transaction. A process killed in that gap leaves a verdict with no receipt. That is why
findUnsignedVerdicts()exists.recordVerdict(message)is available for callers that want the write under their own control.What a verified receipt does not establish
Returned in every verification report rather than left to a reader to infer:
Testing
39 new tests, most of them adversarial: payload tampering, signing with a second key while claiming a trusted
kid, signing outside a validity window, revocation ordering in both directions, deletion, reordering, cross domain signature replay, and truthy stand ins forapproved.Verified locally against Node 22: 39 passing, plus 68 passing alongside the existing ledger suite. eslint clean, prettier clean, conventional commit per commitlint.
Disclosure
I opened #464, I maintain protect-mcp, and I author the Acta signed receipts Internet-Draft whose format it follows. This implementation depends on none of them, and the source and tests reference none of them. protect-mcp appears once in
docs/signed-validator-verdicts.md, as the fourth of four key custody options after file or environment, cloud KMS, and HSM, with that relationship stated in the doc. If you would rather it were not mentioned at all, deleting one bullet and one paragraph removes it with no effect on the implementation.Happy to split this, rename anything, or move the store under
src/agent/if that fits the layout better.