Skip to content

Phase4 - #5

Merged
MachineLearning-Nerd merged 5 commits into
mainfrom
phase4
Aug 26, 2026
Merged

MachineLearning-Nerd merged 5 commits into
mainfrom
phase4

Conversation

@RKOQWER

@RKOQWER RKOQWER commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

I761449 added 3 commits August 25, 2026 23:23
VerifiableReceipt bundles the release receipt with its candidate so the
verifier can recompute both hashes independently. verifyReceipt() re-runs
verifyRelease() from the contract library, which validates policy and
recomputes contractHash/outputHash from the parsed snapshot — any change
to purpose, audience, dimensions, rows, or group sizes fails closed.

Canary absence is checked over the serialized candidate text. When the
optional events array is present, checkEvents() verifies:
- at least one tool.approval_required before any tool.response
- no duplicate tool-call IDs in approval events
- canary absent from the serialized event stream

Every error path returns a typed VerifyOutcome; the function never throws.
Positive cases: well-formed receipt passes with and without events, events=null
treated as absent.

Negative cases — 20 enumerated scenarios:
- receipt_malformed: null/string/array/missing-fields/empty-fields/extra-keys
- contract_hash_mismatch / output_hash_mismatch: tampered hashes
- candidate_malformed: extra keys, null candidate
- candidate_denied: bad purpose
- canary_in_events: canary email and canary free_text in event stream
- approval_missing: no tool.approval_required in events
- release_before_approval: tool.response precedes approval
- duplicate_approval_event: same tool-call id in two approval events
- events_malformed: non-array and non-record-element events

Fail-closed: never throws on any hostile input shape; verifyReceipt always
returns a typed result with outcome != 'pass' for all malformed inputs.
cli.ts reads a VerifiableReceipt JSON from a file path argument or stdin,
runs verifyReceipt(), and exits 0 (PASS) or 1 (FAIL). Internal I/O errors
use exit 2. The output line format is:
  verify-receipt: PASS receipt=<id> query=<id>
  verify-receipt: FAIL outcome=<code> detail=<text>

scripts/ts-loader.mjs is a Node ESM loader hook that resolves relative .js
imports to .ts sources, required because the TypeScript project uses .js
extensions for ESM imports while the sources remain .ts files. This lets
Node 24's --experimental-strip-types run the CLI without a build step.

package.json adds a verify-receipt script:
  npm run verify-receipt -- receipt.json
  cat receipt.json | npm run verify-receipt
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add fail-closed release receipt verification and CLI

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Verify receipt hashes, release policy, canary containment, and optional approval events.
• Return typed fail-closed outcomes for malformed or inconsistent evidence.
• Add a build-free CLI and comprehensive positive and adversarial tests.
Diagram

graph TD
  Input["Receipt input"] --> CLI["CLI entry"] --> Verifier["Receipt verifier"] --> Snapshot["Safe snapshots"] --> Contract["Release contract"] --> Outcome["Typed outcome"]
  Verifier --> Canary["Canary scan"] --> Outcome
  Verifier --> Events["Event checks"] --> Outcome
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compile a dedicated CLI artifact
  • ➕ Uses standard Node module resolution
  • ➕ Avoids experimental loader hooks in production execution
  • ➕ Produces a stable distributable command
  • ➖ Adds build and artifact-management steps
  • ➖ Can drift from the source-only development workflow
2. Use a TypeScript runtime dependency
  • ➕ Removes the custom loader implementation
  • ➕ Provides established TypeScript ESM resolution behavior
  • ➖ Adds a runtime or development dependency for one command
  • ➖ Expands dependency and supply-chain surface

Recommendation: The current Node 24 strip-types approach is appropriate for a repository-local evidence command because it avoids a build step and new dependencies. If the CLI becomes externally distributed or operationally critical, prefer compiling a dedicated artifact to eliminate reliance on experimental loader behavior.

Files changed (6) +734 / -1

Enhancement (3) +342 / -0
cli.tsExpose receipt verification as an evidence CLI +72/-0

Expose receipt verification as an evidence CLI

• Reads JSON from a file or standard input, invokes the verifier, and prints concise PASS or FAIL evidence. Distinct exit codes separate successful verification, failed checks, and input I/O errors.

src/verify/cli.ts

receipt.tsDefine verifiable receipt and typed outcome contracts +57/-0

Define verifiable receipt and typed outcome contracts

• Introduces the receipt bundle, exhaustive verification outcomes, result union, exact-key lists, and TrueForge event constants shared by the verifier and tests.

src/verify/receipt.ts

verify.tsImplement fail-closed receipt and event verification +213/-0

Implement fail-closed receipt and event verification

• Strictly parses receipt evidence, reuses the release contract to validate policy and recompute hashes, scans for canaries, and checks optional approval-event ordering. Every path maps to a typed result instead of throwing.

src/verify/verify.ts

Tests (1) +372 / -0
verify.test.tsCover successful, malformed, tampered, and adversarial receipts +372/-0

Cover successful, malformed, tampered, and adversarial receipts

• Builds real receipts through vault handlers and tests hash mismatches, policy failures, malformed structures, canary leakage, approval ordering, duplicate approvals, and non-throwing behavior.

src/verify/verify.test.ts

Other (2) +20 / -1
package.jsonRegister the receipt verification command +2/-1

Register the receipt verification command

• Adds an npm script that runs the TypeScript receipt-verification CLI directly under Node 24 using type stripping and the custom loader.

package.json

ts-loader.mjsResolve source ESM imports to TypeScript files +18/-0

Resolve source ESM imports to TypeScript files

• Adds a Node loader hook that remaps relative '.js' imports to existing '.ts' sources, allowing the CLI to run without compiling the project.

scripts/ts-loader.mjs

@qodo-code-review

qodo-code-review Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Approvals authorize unrelated calls ✓ Resolved 🐞 Bug ⛨ Security
Description
Each tool.response is accepted when any approval-required event appears earlier, without matching
the response's tool_call_id to an approved call or confirming that it is the release_result
call. An unrelated tool approval can therefore make an unapproved release response pass
verification.
Code

src/verify/verify.ts[R130-133]

+  // Every tool.response must be preceded by at least one approval event.
+  for (const responsePos of responsePositions) {
+    const priorApproval = approvalPositions.some((approvalPos) => approvalPos < responsePos);
+    if (!priorApproval) {
Evidence
Approval IDs are gathered only for duplicate detection, while response ordering asks whether any
approval position is earlier and never reads the response call ID. The test fixture demonstrates
that responses carry tool_call_id, and the MCP server defines the relevant operation specifically
as release_result.

src/verify/verify.ts[82-105]
src/verify/verify.ts[122-139]
src/verify/verify.test.ts[101-109]
src/server/mcp.ts[94-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Ordering is checked globally rather than per tool call, allowing an unrelated approval to authorize a release response.

## Issue Context
Parse and validate call IDs and tool identity from approval-required, user-approval, and response events. Check the complete ordering chain specifically for the `release_result` invocation associated with the receipt.

## Fix Focus Areas
- src/verify/verify.ts[82-105]
- src/verify/verify.ts[122-139]
- src/server/mcp.ts[94-106]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Receipt metadata remains unbound ✓ Resolved 🐞 Bug ≡ Correctness
Description
verifyReceipt verifies only the two hashes, so arbitrary non-empty queryId, receiptId,
datasetVersion, and policyVersion values still produce pass. The command can therefore certify
a receipt that falsely identifies the query and versions that produced the candidate.
Code

src/verify/verify.ts[R163-165]

+    // Validate the candidate through the release contract and recompute hashes.
+    // verifyRelease calls validateRelease internally, then checks both hashes.
+    const verdict = verifyRelease(candidate, receipt.contractHash, receipt.outputHash);
Evidence
Receipt fields are only checked for non-empty string shape, while verifyRelease receives only the
hashes. The candidate schema contains the corresponding version fields and provenance query ID, and
the server creates receipt metadata from those exact candidate values.

src/verify/verify.ts[16-26]
src/verify/verify.ts[163-165]
src/verify/verify.ts[203-203]
src/contract/validate.ts[10-20]
src/server/handlers.ts[129-135]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The verifier checks receipt hashes but does not compare the receipt's query and version metadata with the verified candidate, allowing internally inconsistent receipts to pass.

## Issue Context
The candidate already carries `datasetVersion`, `policyVersion`, and `provenance.queryId`. `receiptId` cannot be authenticated from candidate data, so either explicitly treat it as an unverified display field or add trusted receipt provenance/signing rather than presenting it as verified.

## Fix Focus Areas
- src/verify/verify.ts[16-26]
- src/verify/verify.ts[163-165]
- src/verify/verify.ts[203-203]
- src/contract/validate.ts[10-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. User denial still passes ✓ Resolved 🐞 Bug ⛨ Security
Description
checkEvents collects user.tool_approval positions but never requires one or checks its
approval.status, so an approval-required event followed directly by execution—or an explicit
denial—passes. This certifies tool execution without evidence that the user allowed it.
Code

src/verify/verify.ts[R114-120]

+  // Find the position of any user.tool_approval (resume) event.
+  const resumePositions: number[] = [];
+  for (let index = 0; index < events.length; index++) {
+    if (sequence[index] === TF_EVENT_USER_TOOL_APPROVAL) {
+      resumePositions.push(index);
+    }
+  }
Evidence
The resume positions are computed and discarded, and success depends only on a prior
tool.approval_required. The verifier's own fixture shows the expected user-decision payload
includes both tool_call_id and approval.status: "allow".

src/verify/verify.ts[114-141]
src/verify/verify.test.ts[95-99]
src/verify/receipt.ts[42-45]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Event verification does not require a user approval event with an allow status before tool execution.

## Issue Context
Validate the expected `user.tool_approval` payload and require a matching `tool_call_id` with an allowed status between the approval-required event and response. Reject missing, denied, malformed, or out-of-order decisions.

## Fix Focus Areas
- src/verify/verify.ts[66-142]
- src/verify/receipt.ts[42-45]
- src/verify/verify.test.ts[95-99]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (1)
4. Filtered events corrupt positions ✓ Resolved 🐞 Bug ≡ Correctness
Description
sequence removes records without a string type, but later indexes that shortened array using
indexes from the original event list. Inserting an untyped record can make a pre-approval response
disappear from responsePositions, allowing an invalid ordering to pass, or can cause valid logs to
fail.
Code

src/verify/verify.ts[R78-80]

+  const sequence: string[] = events
+    .map((event) => (typeof event.type === "string" ? event.type : ""))
+    .filter((type) => type.length > 0);
Evidence
The parser permits any record, including one without type; sequence then filters such records
out. Approval positions use original event indexes while response positions are derived by indexing
the filtered sequence with those original indexes.

src/verify/verify.ts[53-63]
src/verify/verify.ts[77-80]
src/verify/verify.ts[82-90]
src/verify/verify.ts[122-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Filtering event types shifts indexes, but subsequent ordering checks treat them as indexes into the original event array.

## Issue Context
Inspect `events[index]?.type` directly in each loop, or retain `{type, originalIndex}` pairs without filtering away positional information. Add tests with untyped records before and between approval and response events.

## Fix Focus Areas
- src/verify/verify.ts[77-80]
- src/verify/verify.ts[114-139]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Stdin mode breaks Windows ✓ Resolved 🐞 Bug ≡ Correctness
Description
The documented stdin mode reads the Unix-only /dev/stdin path, so piped input fails with exit code
2 on Windows despite the package imposing no platform restriction. Node supports reading standard
input portably through file descriptor 0.
Code

src/verify/cli.ts[R30-36]

+function readInput(args: string[]): string {
+  const [filePath] = args;
+  if (filePath !== undefined && filePath !== "-") {
+    return readFileSync(filePath, "utf8");
+  }
+  // Read from stdin synchronously.
+  return readFileSync("/dev/stdin", "utf8");
Evidence
No-file and - modes unconditionally read /dev/stdin, and read errors become exit code 2. The
package requires Node 24 but declares no operating-system restriction.

src/verify/cli.ts[4-6]
src/verify/cli.ts[30-46]
package.json[6-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CLI hardcodes `/dev/stdin`, making its documented stdin mode unavailable on Windows.

## Issue Context
Use `readFileSync(0, "utf8")` for stdin and retain the existing file-path behavior and error mapping.

## Fix Focus Areas
- src/verify/cli.ts[30-36]
- src/verify/cli.ts[41-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/verify/verify.ts
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/cli.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The event-ordering check mixes filtered sequence indices with unfiltered events indices, which can misclassify a valid event stream as release_before_approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR ("Phase4") adds a new verify-receipt evidence command to the Need-to-Know project. It introduces a self-contained verifier that takes a bundle of a release receipt plus its candidate (and optional TrueForge session events), recomputes and checks the contract/output hashes, re-runs the release contract policy, scans for canary values, and validates approval-before-release event ordering — always returning a typed result and never throwing. A CLI entrypoint, a Node loader hook to run the TypeScript sources without a build step, and an npm script wire it up for command-line use.

Changes:

  • New src/verify/ module: receipt.ts (types/constants), verify.ts (core verifyReceipt logic), and cli.ts (stdin/file entrypoint with documented exit codes).
  • Comprehensive verify.test.ts covering positive cases, malformed receipts, hash mismatches, canary containment, event ordering, and fail-closed adversarial input.
  • Tooling: scripts/ts-loader.mjs loader hook and a verify-receipt npm script.
File summaries
File Description
src/verify/receipt.ts Defines VerifiableReceipt, VerifyOutcome/VerifyResult types, and receipt-key/event-type constants.
src/verify/verify.ts Core verifier: parsing, hash/policy verification, canary scan, and event-ordering checks.
src/verify/cli.ts CLI reading JSON from file/stdin and mapping outcomes to exit codes.
src/verify/verify.test.ts Test suite for the verifier across positive, negative, and adversarial inputs.
scripts/ts-loader.mjs Node loader hook remapping .js imports to .ts for the CLI.
package.json Adds the verify-receipt npm script.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 5
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.test.ts Outdated
Comment thread package.json Outdated

@MachineLearning-Nerd MachineLearning-Nerd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 87c3546 — pulled the branch, ran the suite (154/154), tsc and biome clean, and exercised the CLI end-to-end against a real vault release: PASS -> exit 0, tampered outputHash -> output_hash_mismatch, exit 1. Every finding below was reproduced against live code, not read off the diff; repro shapes are in the inline threads.

What's solid

  • The parse layer reuses snapshotRecord/snapshotArray, so the verifier gets Phase 2's single-read defenses (proto grafting, accessor re-reads, index aliases) for free — right call.
  • The never-throws contract holds, hash checks delegate to verifyRelease instead of reimplementing, and the CLI header is honest about what the tool does NOT prove (origin, delivery, session binding).

Findings (3 bugs + 1 comment)

  1. High — the user's approval is never checked: a transcript where the user denied and the tool ran anyway verifies as pass; resumePositions is dead code.
  2. High — every tool.response requires prior approval, so a realistic full session (describe/prepare responses before release's approval) false-fails with release_before_approval.
  3. Medium — filtered sequence misaligns indices with events: one type-less event false-fails a correctly ordered stream.
  4. Low — the "Build a VerifiableReceipt…" comment describes a function that doesn't exist.

Minor, no thread needed: --experimental-loader prints a deprecation warning on every CLI run (Node suggests the register() form via --import); invalid-JSON input reports on stderr while the header says failures print the outcome to stdout, and shares exit 1 with verification failures; and with no file argument and no piped stdin the CLI blocks silently on a TTY.

1 and 2 are worth fixing before merge — together they mean the event check currently passes the transcript it must reject and rejects the transcript it must pass. Both fixes are small and live in checkEvents.

Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts
The event check verified that the agent asked, never that the user
allowed: user.tool_approval positions were collected into a dead
variable and a transcript recording an explicit deny followed by the
tool running anyway verified as pass. Every gated tool.response must
now carry a user.tool_approval with status "allow" between its
approval request and its execution; anything else is
user_approval_missing, a new outcome.

The ordering rule also applied to every tool.response, so a real
session — describe_dataset and prepare_analysis answering long before
release_result asks — was rejected as release_before_approval, while
the comment claimed the check was scoped to release_result. The check
now applies only to responses whose tool_call_id was named in an
approval request; ungated responses pass through.

Positions are now real indices into the events array everywhere. The
filtered sequence view shifted response positions left past their
approval whenever a type-less event preceded them, false-failing a
correctly ordered stream.

Also drops a comment describing a builder function that was never
written. Regression tests cover all four: denied-then-ran, no grant at
all, full-session transcript, and a stream containing a heartbeat.

AI-assisted (Claude Code); reviewed and understood by the author.
The verifier bound the receipt to the candidate's content through the
two hashes but never to its identity: arbitrary queryId,
datasetVersion, and policyVersion still verified as pass, so the
command could certify a receipt naming the wrong query and versions.
After approval the three fields are now compared against the
candidate — provenance.queryId and the two version fields — and any
difference is receipt_metadata_mismatch, a new outcome. receiptId has
no candidate-side counterpart and is documented as an unverified
display field.

The canary_in_rows test asserted nothing: on its actual path (the
contract denies the email canary) the body returned before calling
verifyReceipt. Probing every carrier — a region value, both version
fields, provenance.queryId — shows the contract pins every string
field to a domain, constant, or pattern, so the canary scan is
currently unreachable through an approved candidate. The scan stays
as defence in depth against a future contract relaxation; the tests
now pin the denials standing in front of it for both canary values,
and say so.

Stdin now reads file descriptor 0 instead of the Unix-only
/dev/stdin, and the CLI registers its loader hook via --import
(module.register) instead of the deprecated --loader flag, which also
drops the deprecation warning from every run. Verified end to end:
file arg, piped stdin, and a tampered hash all behave per the header.

Found by Qodo and Copilot reviews on 87c3546.

AI-assisted (Claude Code); reviewed and understood by the author.
@MachineLearning-Nerd
MachineLearning-Nerd merged commit 3b78015 into main Aug 26, 2026
1 check passed
@MachineLearning-Nerd MachineLearning-Nerd mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants