From 0a9f7161d22a91fd47d3b357c0f0efd04cf0b4ec Mon Sep 17 00:00:00 2001 From: how2how2how2-arch Date: Thu, 29 Oct 2026 03:10:59 +0800 Subject: [PATCH] feat: let validate check a bundle against a digest from outside it --- docs/cli.md | 50 ++++++++++++++ src/stepfork/cli/validate.py | 65 +++++++++++++++++- src/stepfork/trace/integrity.py | 7 +- tests/unit/test_cli_validate.py | 112 ++++++++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 2 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 52fbc93..bdb7d70 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -60,6 +60,7 @@ Options: |---|---| | `--partial` | Allow incomplete in-progress traces, skipping `run_start` checks. | | `--verify-integrity` | Verify per-event payload hashes and bundle integrity metadata. | +| `--expect-digest` | Require the bundle's byte digest to equal this SHA-256, as recorded in a receipt by `stepfork run`. | | `--help` | Show command help and exit. | Strict mode (default) requires exactly one `run_start` and an error event for @@ -83,6 +84,52 @@ stepfork validate failure.sftrace --verify-integrity Validation passed. ``` +### Checking a bundle you were given + +`--verify-integrity` checks each payload file against the `integrity.json` the +bundle carries. That catches a file which was corrupted or edited by itself. It +cannot catch a bundle whose record was rewritten with the payload it covers: +whoever edits `events.jsonl` can recompute the three SHA-256 values in +`integrity.json`, and the bundle then verifies clean. A file the record does not +name is invisible to it too. + +A digest from outside the bundle closes that gap. `stepfork run --receipt` +records one -- SHA-256 over every file in the bundle, taken in sorted +relative-path order -- so a bundle and its receipt can be checked against each +other after the fact: + +```bash +stepfork run tests/test_regression.py --receipt receipts/notify.json +stepfork validate notify.sftrace --expect-digest "$(jq -r .bundle_digest receipts/notify.json)" +``` + +```text +Bundle digest: MATCH 2a37f0c94f1db9a2... +``` + +Any file added, removed or changed in the bundle moves the digest, `integrity.json` +included. A mismatch prints both digests and exits `1`: + +```text +Bundle digest: NO MATCH + expected 2a37f0c94f1db9a2... + actual 7c1e55b3a08d4421... + + These are not the bytes that digest was computed over. A matching + integrity.json does not rule this out: it records hashes the bundle itself + carries, so it can be rewritten along with the files it covers. + +Bundle digest check failed. +``` + +The digest is compared in lower case after trimming whitespace, so a value +copied out of a JSON receipt needs no cleanup. + +What this does not prove: that the bundle is honest, only that it is the one the +digest names. A digest that arrives inside the same archive as the bundle is +worth nothing -- it has to travel by a route the bundle's author cannot edit, +such as a CI log or the receipt file itself. + Common errors: - `Validation failed: issues.` with a code per issue (for example @@ -91,6 +138,9 @@ Common errors: `invalid_event`, `unsupported_schema`). Exit `2`. - `Bundle has no integrity.json metadata.` (`UNVERIFIED (legacy bundle)`). Exit `3` with `--verify-integrity`. +- `Bundle digest check failed.` with the expected and actual digests. Exit `1`. +- `--expect-digest is not a SHA-256 hex digest: ''`. Exit `2` -- nothing + was compared, so this is not a verdict about the bundle. --- diff --git a/src/stepfork/cli/validate.py b/src/stepfork/cli/validate.py index 54b9f53..c7f7576 100644 --- a/src/stepfork/cli/validate.py +++ b/src/stepfork/cli/validate.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hmac from pathlib import Path from typing import Annotated @@ -9,8 +10,13 @@ from rich.console import Console from rich.table import Table +from stepfork.export.receipt import bundle_digest from stepfork.trace import TraceStorageError, ValidationResult, validate_bundle -from stepfork.trace.integrity import IntegrityStatus, verify_bundle_integrity +from stepfork.trace.integrity import ( + IntegrityStatus, + is_sha256_digest, + verify_bundle_integrity, +) from stepfork.trace.storage import load_trace console = Console() @@ -51,6 +57,16 @@ def validate_command( help="Verify per-event payload hashes and bundle integrity metadata.", ), ] = False, + expect_digest: Annotated[ + str | None, + typer.Option( + "--expect-digest", + help=( + "Require the bundle's byte digest to equal this SHA-256, as " + "recorded in a receipt by `stepfork run`." + ), + ), + ] = None, ) -> None: """Validate a `.sftrace` bundle.""" strict = not partial @@ -63,6 +79,8 @@ def validate_command( _print_header(path, strict=strict) _print_metadata(path, strict=strict) + if expect_digest is not None: + _enforce_expected_digest(path, expect_digest) if verify_integrity and structure_result.valid: _print_integrity(path) @@ -158,3 +176,48 @@ def _has_legacy_integrity_issue(result: ValidationResult) -> bool: issue.code == "integrity_unverified" and issue.location == "integrity.json" for issue in result.issues ) + + +def _enforce_expected_digest(path: Path, expected: str) -> None: + """Require the bundle's byte digest to equal an externally supplied one. + + The digest names every file in the bundle, ``integrity.json`` included, so + it answers a question the integrity record cannot: whether this is the + bundle the digest was computed over, rather than a bundle that carries an + integrity record. Whoever edits a payload can rewrite the record that + covers it, which leaves ``--verify-integrity`` reporting a clean bundle. + """ + requested = expected.strip().lower() + if not is_sha256_digest(requested): + console.print( + f"[red]✗[/red] --expect-digest is not a SHA-256 hex digest: {expected!r}" + ) + console.print( + " Expected 64 hexadecimal characters. Nothing was compared, so " + "this says nothing about the bundle." + ) + raise typer.Exit(2) + + if not path.is_dir(): + # Structure validation reports a missing or unreadable bundle and can + # say which file is absent; there is nothing to digest yet. + return + + actual = bundle_digest(path) + if hmac.compare_digest(actual, requested): + console.print(f"Bundle digest: [green]MATCH[/green] {actual}") + console.print() + return + + console.print("Bundle digest: [red]NO MATCH[/red]") + console.print(f" expected {requested}") + console.print(f" actual {actual}") + console.print() + console.print( + " These are not the bytes that digest was computed over. A matching " + "integrity.json does not rule this out: it records hashes the bundle " + "itself carries, so it can be rewritten along with the files it covers." + ) + console.print() + console.print("[red]Bundle digest check failed.[/red]") + raise typer.Exit(1) diff --git a/src/stepfork/trace/integrity.py b/src/stepfork/trace/integrity.py index cab3e9b..0556e37 100644 --- a/src/stepfork/trace/integrity.py +++ b/src/stepfork/trace/integrity.py @@ -58,6 +58,11 @@ class IntegrityResult(BaseModel): verified_files: list[str] = Field(default_factory=list) +def is_sha256_digest(value: str) -> bool: + """Whether ``value`` is a lowercase SHA-256 hex digest.""" + return _SHA256_RE.fullmatch(value) is not None + + def write_bundle_integrity(path: Path) -> IntegrityRecord: """Write integrity metadata for the finalized bundle payload files.""" record = IntegrityRecord( @@ -172,7 +177,7 @@ def _validate_record(record: IntegrityRecord) -> list[IntegrityIssue]: file=filename, ) ) - elif _SHA256_RE.fullmatch(digest) is None: + elif not is_sha256_digest(digest): issues.append( IntegrityIssue( code="integrity_metadata_invalid", diff --git a/tests/unit/test_cli_validate.py b/tests/unit/test_cli_validate.py index e769c48..95bd91e 100644 --- a/tests/unit/test_cli_validate.py +++ b/tests/unit/test_cli_validate.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import json import shutil from pathlib import Path @@ -10,10 +11,14 @@ from stepfork import ToolCall, Trace from stepfork.cli.main import app +from stepfork.export.receipt import bundle_digest from stepfork.trace import RunEnd, RunStart, RunStatus, ToolResult +from stepfork.trace.integrity import IntegrityStatus, verify_bundle_integrity runner = CliRunner() +INTEGRITY_COVERED = ("manifest.json", "events.jsonl", "redactions.json") + def complete_trace() -> Trace: trace = Trace(agent_name="demo-agent") @@ -130,4 +135,111 @@ def test_validate_help(monkeypatch: pytest.MonkeyPatch) -> None: assert result.exit_code == 0 assert "--partial" in result.stdout + assert "--expect-digest" in result.stdout assert ".sftrace" in result.stdout + + +def rewrite_integrity_record(path: Path) -> None: + """Recompute ``integrity.json`` over the bundle's current payload bytes. + + This is what an edit looks like once the record is rewritten with it: the + three payload files agree with ``integrity.json`` again, so the record on + its own has nothing left to disagree with. + """ + files = { + name: hashlib.sha256((path / name).read_bytes()).hexdigest() + for name in INTEGRITY_COVERED + } + record = {"schema_version": "0.1", "algorithm": "sha256", "files": files} + (path / "integrity.json").write_text( + json.dumps(record, indent=2) + "\n", encoding="utf-8", newline="" + ) + + +def test_validate_expect_digest_matches(tmp_path: Path) -> None: + path = complete_trace().save( + tmp_path / "valid.sftrace", + status=RunStatus.COMPLETED, + ) + digest = bundle_digest(path) + + result = runner.invoke(app, ["validate", str(path), "--expect-digest", digest]) + + assert result.exit_code == 0 + assert "MATCH" in result.stdout + assert "Validation passed" in result.stdout + + +def test_validate_expect_digest_ignores_case_and_surrounding_space( + tmp_path: Path, +) -> None: + path = complete_trace().save( + tmp_path / "valid.sftrace", + status=RunStatus.COMPLETED, + ) + digest = bundle_digest(path) + + result = runner.invoke( + app, ["validate", str(path), "--expect-digest", f" {digest.upper()}\n"] + ) + + assert result.exit_code == 0 + assert "MATCH" in result.stdout + + +def test_validate_expect_digest_catches_an_added_file(tmp_path: Path) -> None: + path = complete_trace().save( + tmp_path / "valid.sftrace", + status=RunStatus.COMPLETED, + ) + recorded = bundle_digest(path) + (path / "agent-notes.txt").write_text("added after the receipt was written\n") + + # The integrity record does not cover the added file, which is exactly why + # it cannot answer this question. + assert verify_bundle_integrity(path).status is IntegrityStatus.VERIFIED + + result = runner.invoke(app, ["validate", str(path), "--expect-digest", recorded]) + + assert result.exit_code == 1 + assert "NO MATCH" in result.stdout + assert "Bundle digest check failed" in result.stdout + + +def test_validate_expect_digest_catches_a_rewritten_integrity_record( + tmp_path: Path, +) -> None: + path = complete_trace().save( + tmp_path / "tampered.sftrace", + status=RunStatus.COMPLETED, + ) + recorded = bundle_digest(path) + + lines = (path / "events.jsonl").read_text().splitlines() + event = json.loads(lines[1]) + event["input"]["query"] = "Pokhara flights" + lines[1] = json.dumps(event) + (path / "events.jsonl").write_text("\n".join(lines) + "\n") + rewrite_integrity_record(path) + + # Rewriting the record with the payload leaves the record self-consistent. + assert verify_bundle_integrity(path).status is IntegrityStatus.VERIFIED + + result = runner.invoke(app, ["validate", str(path), "--expect-digest", recorded]) + + assert result.exit_code == 1 + assert "NO MATCH" in result.stdout + + +def test_validate_expect_digest_rejects_a_malformed_digest(tmp_path: Path) -> None: + path = complete_trace().save( + tmp_path / "valid.sftrace", + status=RunStatus.COMPLETED, + ) + + result = runner.invoke( + app, ["validate", str(path), "--expect-digest", "not-a-digest"] + ) + + assert result.exit_code == 2 + assert "not a SHA-256 hex digest" in result.stdout