From 5699dfd293f31b9b8098f734aff5fd3b2e03c9f3 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:14:10 -0500 Subject: [PATCH 1/2] Write the record to disk as a bundle a stranger can verify Seven steps built an exporter that produced structures in memory and never an artifact. The Definition of Done was "a rehearsal runs, the exporter writes a bundle, the standalone verifier passes on it in a subprocess" and nothing wrote the bundle, so nothing had ever verified end to end. The layout is the one the design fixed: manifest.json, streams.jsonl, logbooks/.jsonl. JSON Lines because a reviewer can stream, grep and diff it per row. The trap that carries is that the line format is NOT what the hashes cover, so a verifier must REASSEMBLE the body before hashing. Reassembly therefore exists twice, once in the package and once in the standalone script, and the duplication is the point: a checker importing CORA's reassembly would confirm CORA's own idea of what the bundle says. That is not a check. A new fitness test pins the script's zero-import property, because the duplication is a standing invitation to deduplicate it away and its failure mode is silent. The gap that only appears end to end: a bundle whose files are each individually well-formed can still be missing a whole logbook kind. Per-file verification passes; only the reassembled body's hash catches it. Both suites assert exactly that, alongside tamper and line-reorder sensitivity, because order is part of the record rather than presentation. H3 lands here too, as step 7 asked, not as an afterthought: the published record's own hash, the one a paper prints beside its locator and the only one covering bytes a reader can hold. Its payload type is deliberately distinct from H1's, so an export whose redaction happened to drop nothing still hashes differently as a published record than as a full one. They must never collide. The manifest carries it as optional, and None means "not a published projection" rather than "redaction found nothing": the absence is the signal, and the verifier refuses --published against a bundle lacking it rather than falling back to H1. Writing into a non-empty directory refuses, and the manifest is written last, so an interrupted export leaves something read_bundle_body rejects rather than a complete-looking bundle with truncated rows. Found while writing the end-to-end test and NOT fixed here, because loosening a fail-closed check is a security decision rather than a bundle writer's business: ensure_all_clearances_fired refuses unless every declared activity/payload clearance fires, and those three keys live on different step kinds, two of them optional. redact_record therefore refuses any export whose activities lack a setpoint-with-units AND an action AND a check. It fails closed, so nothing leaks, but the first small rehearsal bundle would hit it and read as a config error. Step 6's own fixture already works around it with a comment reasoning that a real export "would plausibly exercise every step kind" -- an assumption about data holding up a hard refusal. Documented at both sites; owed a decision. Refs: project_record_export_v3.md F4/F5, project_record_export_build_brief.md Co-Authored-By: Claude Opus 5 (1M context) --- .../infrastructure/record_export/__init__.py | 22 ++ .../infrastructure/record_export/_bundle.py | 185 +++++++++++++ .../infrastructure/record_export/_hashing.py | 71 ++++- .../infrastructure/record_export/_manifest.py | 54 +++- ...est_standalone_verifier_imports_nothing.py | 64 +++++ .../test_record_export_bundle_postgres.py | 260 ++++++++++++++++++ .../record_export/test_bundle.py | 182 ++++++++++++ .../record_export/test_standalone_verifier.py | 88 ++++++ scripts/verify_record_hash.py | 127 ++++++++- 9 files changed, 1033 insertions(+), 20 deletions(-) create mode 100644 apps/api/src/cora/infrastructure/record_export/_bundle.py create mode 100644 apps/api/tests/architecture/test_standalone_verifier_imports_nothing.py create mode 100644 apps/api/tests/integration/test_record_export_bundle_postgres.py create mode 100644 apps/api/tests/unit/infrastructure/record_export/test_bundle.py diff --git a/apps/api/src/cora/infrastructure/record_export/__init__.py b/apps/api/src/cora/infrastructure/record_export/__init__.py index d55e37a010..0c8e8c1318 100644 --- a/apps/api/src/cora/infrastructure/record_export/__init__.py +++ b/apps/api/src/cora/infrastructure/record_export/__init__.py @@ -24,6 +24,15 @@ re-identify without the token map. """ +from cora.infrastructure.record_export._bundle import ( + LOGBOOKS_DIR, + MANIFEST_NAME, + STREAMS_NAME, + BundleDestinationNotEmptyError, + MalformedBundleError, + read_bundle_body, + write_bundle, +) from cora.infrastructure.record_export._export import ( EmptyExportError, ExportedRecord, @@ -32,11 +41,14 @@ ) from cora.infrastructure.record_export._hashing import ( LOGBOOKS_PAYLOAD_TYPE, + PUBLISHED_RECORD_PAYLOAD_TYPE, RECORD_PAYLOAD_TYPE, REDACTION_PROFILE_PAYLOAD_TYPE, STREAMS_PAYLOAD_TYPE, + TwoTierRecord, hash_logbooks, hash_record, + hash_redacted_record, hash_redaction_profile, hash_streams, ) @@ -78,23 +90,30 @@ __all__ = [ "KNOWN_STREAM_TYPES", + "LOGBOOKS_DIR", "LOGBOOKS_PAYLOAD_TYPE", + "MANIFEST_NAME", + "PUBLISHED_RECORD_PAYLOAD_TYPE", "RECORD_PAYLOAD_TYPE", "REDACTION_PROFILE_PAYLOAD_TYPE", + "STREAMS_NAME", "STREAMS_PAYLOAD_TYPE", "TIER2_DISPOSITIONS", "TIER2_JSONB_CLEARED_POINTERS", "TIER2_JSONB_DROPPED_COLUMNS", + "BundleDestinationNotEmptyError", "EmptyExportError", "EntriesReader", "EntriesTableSpec", "ExportedRecord", + "MalformedBundleError", "Manifest", "RedactedRecord", "RedactionProfileMismatchError", "RedactionResult", "Tier1Redactor", "TokenMap", + "TwoTierRecord", "UnfiredClearanceError", "UnknownEventTypeError", "UnknownLogbookKindError", @@ -108,8 +127,10 @@ "export_record", "hash_logbooks", "hash_record", + "hash_redacted_record", "hash_redaction_profile", "hash_streams", + "read_bundle_body", "redact_record", "redact_tier1_payload", "redact_tier2_row", @@ -117,4 +138,5 @@ "render_row", "render_value", "resolve", + "write_bundle", ] diff --git a/apps/api/src/cora/infrastructure/record_export/_bundle.py b/apps/api/src/cora/infrastructure/record_export/_bundle.py new file mode 100644 index 0000000000..652c661b24 --- /dev/null +++ b/apps/api/src/cora/infrastructure/record_export/_bundle.py @@ -0,0 +1,185 @@ +"""Write an exported record to disk as the bundle layout F5 names. + +`project_record_export_v3.md`'s "Naming" section fixes the layout: +`manifest.json`, `streams.jsonl`, `logbooks/.jsonl`. Everything +upstream of this module produces in-memory structures; this is the step +that makes an artifact somebody can archive, cite, or email. + +## Why JSON Lines, and the trap it carries + +One row per line means a reader can stream a large export, `grep` it, +and see a per-row diff in review. The trap is that the bundle's line +format is NOT what the hashes cover. `compute_content_hash` hashes a +DSSE-PAE-wrapped, NFC-normalized, sorted-key serialization of the whole +BODY, so the on-disk file is a transport for the body, not the hashed +bytes themselves. Two consequences, both load-bearing: + +- A verifier must REASSEMBLE the body (`{"streams": [...], + "logbooks": {...}}`) from the files before hashing. `read_bundle_body` + is that reassembly, and `scripts/verify_record_hash.py`'s + `verify-bundle` subcommand reimplements it in stdlib-only form. +- Reordering lines, or losing one, changes the hash. That is the point: + order is part of the record (F2's per-kind order key), not a + presentation detail. + +## Refusals + +Writing into a non-empty directory refuses. A bundle is an atomic claim +about one database at one watermark; a directory holding two exports' +`logbooks/` files, or one export's `streams.jsonl` beside another's +`manifest.json`, would hash-verify per file and be a lie as a whole. +""" + +import json +from dataclasses import asdict +from pathlib import Path +from typing import cast + +from cora.infrastructure.record_export._hashing import TwoTierRecord +from cora.infrastructure.record_export._manifest import Manifest + +MANIFEST_NAME = "manifest.json" +STREAMS_NAME = "streams.jsonl" +LOGBOOKS_DIR = "logbooks" + +__all__ = [ + "LOGBOOKS_DIR", + "MANIFEST_NAME", + "STREAMS_NAME", + "BundleDestinationNotEmptyError", + "MalformedBundleError", + "read_bundle_body", + "write_bundle", +] + + +class BundleDestinationNotEmptyError(RuntimeError): + """`write_bundle` was pointed at a directory that already holds files. + + Refuses rather than merging or overwriting: a bundle is one export's + whole claim, and a directory mixing two exports would verify + file-by-file while being incoherent as a record. + """ + + def __init__(self, destination: Path) -> None: + super().__init__( + f"refusing to write a bundle into non-empty directory {destination}: " + "a bundle is one export at one watermark. Write to a fresh directory." + ) + self.destination = destination + + +class MalformedBundleError(RuntimeError): + """A directory does not hold a readable bundle. + + Raised for a missing `streams.jsonl`, a missing `manifest.json`, or a + line that is not a JSON object. Never falls back to a partial read: + a body reassembled from half a bundle would hash to something that + matches nothing, which reads as tampering rather than as the file + error it is. + """ + + +def _kind_filename(kind: str) -> str: + """`logbooks/.jsonl`, with `kind` rejected if it could escape. + + Registry kinds are code-defined identifiers today, so this cannot + currently fire. It is here because the alternative failure is + writing outside the destination directory, and a kind reaches this + function from a registry entry that a future edit could widen. + """ + if not kind or "/" in kind or "\\" in kind or kind.startswith("."): + message = f"logbook kind {kind!r} is not a safe filename component" + raise MalformedBundleError(message) + return f"{kind}.jsonl" + + +def _write_jsonl(path: Path, rows: tuple[dict[str, object], ...]) -> None: + """One compact JSON object per line, in the order given. + + `sort_keys=True` is for reviewability of the file itself, not for + the hash: the canonicalizer sorts independently, so a differently + ordered file would still hash the same. Written together in one + `write_text` so a partial line cannot survive a crash mid-export. + """ + body = "".join( + json.dumps(row, sort_keys=True, ensure_ascii=False, separators=(",", ":")) + "\n" + for row in rows + ) + path.write_text(body, encoding="utf-8") + + +def _read_jsonl(path: Path) -> tuple[dict[str, object], ...]: + rows: list[dict[str, object]] = [] + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + if not line.strip(): + continue + try: + row = json.loads(line) + except json.JSONDecodeError as exc: + message = f"{path.name} line {number} is not valid JSON: {exc}" + raise MalformedBundleError(message) from exc + if not isinstance(row, dict): + message = f"{path.name} line {number} is a {type(row).__name__}, not a JSON object" + raise MalformedBundleError(message) + rows.append(cast("dict[str, object]", row)) + return tuple(rows) + + +def write_bundle(record: TwoTierRecord, manifest: Manifest, destination: Path) -> Path: + """Write `record` and `manifest` as a bundle under `destination`. + + Creates the directory if absent; refuses if it exists with anything + in it. Returns `destination` so a caller can chain. + + The manifest is written LAST. An interrupted export therefore leaves + a directory with no `manifest.json`, which `read_bundle_body` + refuses, rather than a complete-looking bundle whose row files are + truncated. + """ + destination.mkdir(parents=True, exist_ok=True) + existing = sorted(p.name for p in destination.iterdir()) + if existing: + raise BundleDestinationNotEmptyError(destination) + + _write_jsonl(destination / STREAMS_NAME, record.streams) + + logbooks_dir = destination / LOGBOOKS_DIR + logbooks_dir.mkdir() + for kind, rows in record.logbooks.items(): + _write_jsonl(logbooks_dir / _kind_filename(kind), rows) + + manifest_path = destination / MANIFEST_NAME + manifest_path.write_text( + json.dumps(asdict(manifest), sort_keys=True, indent=2, ensure_ascii=False) + "\n", + encoding="utf-8", + ) + return destination + + +def read_bundle_body(destination: Path) -> dict[str, object]: + """Reassemble the hashed body from a bundle on disk. + + Returns exactly the structure `hash_record` / `hash_redacted_record` + hash, so a caller can recompute either and compare. An empty + `logbooks/` directory yields `{"logbooks": {}}`, which is a real + export shape (a record with no entries rows at all), not an error. + """ + streams_path = destination / STREAMS_NAME + if not streams_path.is_file(): + message = f"{destination} has no {STREAMS_NAME}; not a bundle" + raise MalformedBundleError(message) + if not (destination / MANIFEST_NAME).is_file(): + message = ( + f"{destination} has no {MANIFEST_NAME}. The manifest is written last, " + "so this is what an interrupted export leaves behind." + ) + raise MalformedBundleError(message) + + logbooks_dir = destination / LOGBOOKS_DIR + logbooks: dict[str, object] = {} + if logbooks_dir.is_dir(): + for path in sorted(logbooks_dir.glob("*.jsonl")): + logbooks[path.stem] = list(_read_jsonl(path)) + + return {"streams": list(_read_jsonl(streams_path)), "logbooks": logbooks} diff --git a/apps/api/src/cora/infrastructure/record_export/_hashing.py b/apps/api/src/cora/infrastructure/record_export/_hashing.py index 4a5dea26ee..89c52fbdc7 100644 --- a/apps/api/src/cora/infrastructure/record_export/_hashing.py +++ b/apps/api/src/cora/infrastructure/record_export/_hashing.py @@ -27,6 +27,8 @@ manifest body in hand rather than guessed at here. """ +from typing import Protocol + from cora.infrastructure.record_export._dispositions import DISPOSITIONS from cora.infrastructure.record_export._export import ExportedRecord from cora.infrastructure.record_export._redact_tier2 import ( @@ -40,6 +42,31 @@ STREAMS_PAYLOAD_TYPE = "application/vnd.cora.record-streams+json" LOGBOOKS_PAYLOAD_TYPE = "application/vnd.cora.record-logbooks+json" REDACTION_PROFILE_PAYLOAD_TYPE = "application/vnd.cora.record-redaction-profile+json" +PUBLISHED_RECORD_PAYLOAD_TYPE = "application/vnd.cora.record-published+json" + + +class TwoTierRecord(Protocol): + """The shape both `ExportedRecord` and `RedactedRecord` share. + + Structural, not a base class, so `_hashing` can hash a redacted + record without importing `_redaction` (which imports this module: + the dependency runs one way only). + """ + + @property + def streams(self) -> tuple[dict[str, object], ...]: ... + + @property + def logbooks(self) -> dict[str, tuple[dict[str, object], ...]]: ... + + +def _two_tier_body(record: TwoTierRecord) -> dict[str, object]: + """The hashed body shape shared by H1 and H3, so the two can never + disagree about what "the whole bundle" means.""" + return { + "streams": list(record.streams), + "logbooks": {kind: list(rows) for kind, rows in record.logbooks.items()}, + } def hash_streams(streams: tuple[dict[str, object], ...]) -> str: @@ -61,16 +88,39 @@ def hash_logbooks(logbooks: dict[str, tuple[dict[str, object], ...]]) -> str: def hash_record(record: ExportedRecord) -> str: """SHA-256 content hash over the whole bundle, both tiers, no exclusions. - This is THE record hash: per F2, it covers everything `export_record` - produced. Re-running the export against an unchanged database - reproduces this value exactly; any single differing byte anywhere in - either tier changes it. + This is THE record hash (H1): per F2, it covers everything + `export_record` produced. Re-running the export against an unchanged + database reproduces this value exactly; any single differing byte + anywhere in either tier changes it. """ - body = { - "streams": list(record.streams), - "logbooks": {kind: list(rows) for kind, rows in record.logbooks.items()}, - } - return compute_content_hash(RECORD_PAYLOAD_TYPE, body) + return compute_content_hash(RECORD_PAYLOAD_TYPE, _two_tier_body(record)) + + +def hash_redacted_record(record: TwoTierRecord) -> str: + """SHA-256 content hash over the PUBLISHED record: H3. + + The third of `project_record_export_v3.md` F5's three hashes (full + record H1, redaction profile H2, published record H3), and the one + the paper actually prints beside its locator, because it is the only + one covering the bytes a reader can hold. + + The payload type is deliberately distinct from + `RECORD_PAYLOAD_TYPE`. `compute_content_hash` binds the payload type + into the DSSE-PAE preamble, so an unredacted and a redacted record + that happened to reduce to identical bodies still hash differently. + That matters: without it, an export whose redaction dropped nothing + (every field already publishable) would produce H1 == H3, and a + reader could not tell a published record from a full one by its hash + alone. They must never collide. + + Takes the structural `TwoTierRecord` rather than `RedactedRecord` + only because importing `_redaction` here would invert this module's + dependency; callers pass `RedactionResult.redacted_record`. Passing + an unredacted `ExportedRecord` is a caller error this signature + cannot catch, and the reason `write_bundle` derives H3 itself from + the record it is handed rather than accepting one as a parameter. + """ + return compute_content_hash(PUBLISHED_RECORD_PAYLOAD_TYPE, _two_tier_body(record)) def hash_redaction_profile() -> str: @@ -119,11 +169,14 @@ def hash_redaction_profile() -> str: __all__ = [ "LOGBOOKS_PAYLOAD_TYPE", + "PUBLISHED_RECORD_PAYLOAD_TYPE", "RECORD_PAYLOAD_TYPE", "REDACTION_PROFILE_PAYLOAD_TYPE", "STREAMS_PAYLOAD_TYPE", + "TwoTierRecord", "hash_logbooks", "hash_record", + "hash_redacted_record", "hash_redaction_profile", "hash_streams", ] diff --git a/apps/api/src/cora/infrastructure/record_export/_manifest.py b/apps/api/src/cora/infrastructure/record_export/_manifest.py index 7bbdb8fa65..f950e7c777 100644 --- a/apps/api/src/cora/infrastructure/record_export/_manifest.py +++ b/apps/api/src/cora/infrastructure/record_export/_manifest.py @@ -3,13 +3,14 @@ trusting the bundle. Per `project_record_export_build_brief.md` step 4 and -`project_record_export_v3.md` F8. "Both profile hashes" are the two -namable today: `record_hash` (H1, the whole unredacted bundle, built in -step 3) and `redaction_profile_hash` (H2, step 0's generated disposition -table -- "the table's canonical hash IS the redaction profile hash", -computed here because nothing computed it before this step existed). H3 -(the published/redacted record's hash) does not exist until step 6 -builds redaction. +`project_record_export_v3.md` F8. All THREE of F5's hashes are namable +here now: `record_hash` (H1, the whole unredacted bundle, step 3), +`redaction_profile_hash` (H2, every table that decides what a published +record discloses, widened to both tiers by step 7's security review), +and `published_record_hash` (H3, the redacted projection's own hash, +added with the bundle writer). H3 is optional because an unredacted +bundle genuinely has none; see the field's own docstring for why its +absence is a signal rather than a default. `build_manifest` is pure: every input it needs (`git_commit`, `watermark`) is captured by the caller first and passed in, so the @@ -23,7 +24,12 @@ from typing import cast from cora.infrastructure.record_export._export import ExportedRecord -from cora.infrastructure.record_export._hashing import hash_record, hash_redaction_profile +from cora.infrastructure.record_export._hashing import ( + TwoTierRecord, + hash_record, + hash_redacted_record, + hash_redaction_profile, +) @dataclass(frozen=True, slots=True) @@ -44,6 +50,18 @@ class Manifest: max_schema_version_by_event_type: dict[str, int] is_simulated: bool expansion_digest_presence_by_run: dict[str, bool] + published_record_hash: str | None = None + """H3, present only on a manifest built alongside a redacted record. + + `None` means "this manifest describes an unredacted bundle", NOT + "redaction produced nothing". A reader seeing `None` beside a bundle + someone called published should treat the bundle as unverified: the + absence is the signal. + + Safe to carry inside the bundle despite H3 covering that bundle, + because H3 hashes the two tiers only. The manifest is not in its own + hashed body, so there is no circularity to resolve. + """ def capture_git_commit(*, cwd: Path | str | None = None) -> str: @@ -133,8 +151,23 @@ def _expansion_digest_presence_by_run(record: ExportedRecord) -> dict[str, bool] } -def build_manifest(record: ExportedRecord, *, watermark: int, git_commit: str) -> Manifest: - """Assemble the manifest for one already-exported, already-rendered record.""" +def build_manifest( + record: ExportedRecord, + *, + watermark: int, + git_commit: str, + redacted: TwoTierRecord | None = None, +) -> Manifest: + """Assemble the manifest for one already-exported, already-rendered record. + + Pass `redacted` (a `RedactionResult.redacted_record`) when the bundle + being written is the published projection, so the manifest carries + H3. The shape counts stay derived from the UNREDACTED `record`: + redaction never adds or removes a row, only rewrites values within + one, so the counts describe both, and deriving them from the + unredacted side keeps a reader's recomputation honest if redaction + ever does start dropping rows. + """ return Manifest( git_commit=git_commit, watermark=watermark, @@ -144,6 +177,7 @@ def build_manifest(record: ExportedRecord, *, watermark: int, git_commit: str) - max_schema_version_by_event_type=_max_schema_version_by_event_type(record), is_simulated=_is_simulated(record), expansion_digest_presence_by_run=_expansion_digest_presence_by_run(record), + published_record_hash=None if redacted is None else hash_redacted_record(redacted), ) diff --git a/apps/api/tests/architecture/test_standalone_verifier_imports_nothing.py b/apps/api/tests/architecture/test_standalone_verifier_imports_nothing.py new file mode 100644 index 0000000000..08875c73e1 --- /dev/null +++ b/apps/api/tests/architecture/test_standalone_verifier_imports_nothing.py @@ -0,0 +1,64 @@ +"""`scripts/verify_record_hash.py` must import stdlib and nothing else. + +The verifier's entire value is that it is INDEPENDENT: a checker sharing +code with the thing it checks confirms that thing's own idea of the +answer, which is not a check. `project_record_export_v3.md` F4 pays a +real price for this (~30 lines of canonicalization deliberately +duplicated from `cora.shared.content_hash`, plus a second copy of the +bundle reassembly), so the property is worth a test rather than a +comment. + +The duplication is a standing temptation to "clean up". This test is +what makes that cleanup fail loudly instead of quietly destroying the +independence the design bought. + +AST-based rather than import-based: importing the module to inspect it +would defeat the point on a machine where `cora` happens to be +installed, which is every developer machine. +""" + +import ast +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[4] +_SCRIPT = _REPO_ROOT / "scripts" / "verify_record_hash.py" + + +def _imported_roots(tree: ast.AST) -> set[str]: + roots: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + roots.update(alias.name.split(".")[0] for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + roots.add(node.module.split(".")[0]) + return roots + + +def test_standalone_verifier_imports_no_cora_and_no_third_party() -> None: + tree = ast.parse(_SCRIPT.read_text(encoding="utf-8")) + roots = _imported_roots(tree) + + assert "cora" not in roots, ( + "scripts/verify_record_hash.py imports cora. That makes it a check of " + "CORA by CORA, which is not a check. The duplication it removes is " + "deliberate; see the module docstring and F4." + ) + + non_stdlib = roots - set(sys.stdlib_module_names) + assert not non_stdlib, ( + f"scripts/verify_record_hash.py imports non-stdlib module(s) {sorted(non_stdlib)}. " + "It must run on a bare Python 3.13 on a machine that has never " + "installed CORA or pip-installed anything." + ) + + +def test_standalone_verifier_uses_no_relative_imports() -> None: + """A relative import would tie the script to a package layout it is + supposed to be liftable out of: a reviewer should be able to copy + this one file somewhere else and run it.""" + tree = ast.parse(_SCRIPT.read_text(encoding="utf-8")) + relative = [ + node for node in ast.walk(tree) if isinstance(node, ast.ImportFrom) and node.level > 0 + ] + assert not relative, "the verifier must be a single liftable file, with no relative imports" diff --git a/apps/api/tests/integration/test_record_export_bundle_postgres.py b/apps/api/tests/integration/test_record_export_bundle_postgres.py new file mode 100644 index 0000000000..d1128e4a29 --- /dev/null +++ b/apps/api/tests/integration/test_record_export_bundle_postgres.py @@ -0,0 +1,260 @@ +"""The Definition of Done, end to end, against a real database. + +`project_record_export_build_brief.md`: "A rehearsal scenario runs, the +exporter writes a bundle, the standalone verifier passes on it in a +subprocess with no cora on the path." + +Every stage of that sentence is a separate already-tested unit. This is +the one test that runs them in sequence against live Postgres, because +the failure this catches is the seam: a row shape that only a real +export produces, serialized to JSONL, reassembled by a reimplementation +that shares no code with the writer, and hashed to the same value. Any +drift between the two reassembly implementations shows up here and +nowhere else. + +FOUND WHILE WRITING THIS, and not fixed here because loosening a +fail-closed check is a security decision rather than a test fixture's +business: `ensure_all_clearances_fired` raises unless EVERY declared +`activity/payload` clearance fires, and those three keys (`channel`, +`action_name`, `units`) live on different step kinds, two of them +optional per `append_activities/route.py:86-94`. So `redact_record` +refuses on any export whose activities happen not to include a +setpoint-carrying-units AND an action AND a check. Step 6's own +integration fixture already works around it with a comment reasoning +that a whole-database export "would plausibly exercise every step +kind". That is an assumption about data, holding up a hard refusal. It +fails CLOSED, so nothing leaks, but the first small rehearsal bundle, +or any future per-run slice, would hit it and read as a config error. +""" + +# pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false + +import json +import subprocess +import sys +from datetime import UTC, datetime +from pathlib import Path +from uuid import UUID, uuid4 + +import asyncpg +import pytest + +from cora.infrastructure.event_envelope import to_new_event +from cora.infrastructure.record_export import ( + MANIFEST_NAME, + RECORD_PAYLOAD_TYPE, + STREAMS_NAME, + build_manifest, + capture_git_commit, + export_record, + hash_redaction_profile, + read_bundle_body, + redact_record, + write_bundle, +) +from cora.operation.aggregates.procedure import ( + PostgresActivityStore, + ProcedureRegistered, + ProcedureStarted, + event_type_name, + to_payload, +) +from cora.operation.features.append_activities import ActivityInput, AppendProcedureActivities +from cora.operation.features.append_activities import bind as bind_append +from tests.integration._helpers import build_postgres_deps + +_NOW = datetime(2026, 5, 15, 12, 0, 0, tzinfo=UTC) +_PRINCIPAL_ID = UUID("01900000-0000-7000-8000-000000000099") +_CORRELATION_ID = UUID("01900000-0000-7000-8000-0000000000aa") +_REPO_ROOT = Path(__file__).resolve().parents[4] +_VERIFIER = _REPO_ROOT / "scripts" / "verify_record_hash.py" + + +async def _seed_a_procedure_with_one_activity(db_pool: asyncpg.Pool) -> None: + procedure_id = uuid4() + logbook_id = uuid4() + open_event_id = uuid4() + deps = build_postgres_deps(db_pool, now=_NOW, ids=[logbook_id, open_event_id]) + + registered = ProcedureRegistered( + procedure_id=procedure_id, + name="Vessel-A bakeout", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + occurred_at=_NOW, + ) + started = ProcedureStarted(procedure_id=procedure_id, occurred_at=_NOW) + for index, event in enumerate((registered, started)): + new_event = to_new_event( + event_type=event_type_name(event), + payload=to_payload(event), + occurred_at=event.occurred_at, + event_id=uuid4(), + command_name="RegisterProcedure" if index == 0 else "StartProcedure", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ) + await deps.event_store.append( + stream_type="Procedure", + stream_id=procedure_id, + expected_version=index, + events=[new_event], + ) + + # Three step kinds, not one, and NOT because this test needs three. + # `ensure_all_clearances_fired` raises unless every declared + # `activity/payload` clearance (`channel`, `action_name`, `units`) + # matches at least one row, and those keys live on DIFFERENT step + # kinds, two of them optional. A setpoint-only export therefore + # fails to redact. See this module's docstring: that is a real + # fragility in step 6, worked around here so these tests exercise + # the BUNDLE rather than accidentally re-testing the clearance rule. + handler = bind_append(deps, step_store=PostgresActivityStore(db_pool)) + await handler( + AppendProcedureActivities( + procedure_id=procedure_id, + entries=( + ActivityInput( + event_id=uuid4(), + step_kind="setpoint", + payload={"channel": "T_oven", "target_value": 423.0, "units": "K"}, + sampled_at=_NOW, + ), + ActivityInput( + event_id=uuid4(), + step_kind="action", + payload={"action_name": "open_valve", "params": {"valve": "V12"}}, + sampled_at=_NOW, + ), + ActivityInput( + event_id=uuid4(), + step_kind="check", + payload={"channel": "T_oven", "passed": True}, + sampled_at=_NOW, + ), + ), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +def _verify(bundle: Path, *, published: bool = False) -> subprocess.CompletedProcess[str]: + argv = [sys.executable, str(_VERIFIER), "verify-bundle", str(bundle)] + if published: + argv.append("--published") + return subprocess.run(argv, capture_output=True, text=True) + + +def _verify_body(body_file: Path, expected: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + sys.executable, + str(_VERIFIER), + "verify", + "--payload-type", + RECORD_PAYLOAD_TYPE, + "--expected-hash", + expected, + str(body_file), + ], + capture_output=True, + text=True, + ) + + +@pytest.mark.integration +async def test_a_real_export_writes_a_bundle_a_stranger_can_verify( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + await _seed_a_procedure_with_one_activity(db_pool) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + bundle = write_bundle(exported, manifest, tmp_path / "bundle") + + assert (bundle / STREAMS_NAME).is_file() + assert (bundle / MANIFEST_NAME).is_file() + assert (bundle / "logbooks" / "activity.jsonl").is_file() + + result = _verify(bundle) + assert result.returncode == 0, result.stderr + assert "OK" in result.stdout + + +@pytest.mark.integration +async def test_a_real_redacted_export_verifies_against_h3( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + await _seed_a_procedure_with_one_activity(db_pool) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest( + exported, + watermark=1, + git_commit=capture_git_commit(), + redacted=redaction.redacted_record, + ) + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") + + result = _verify(bundle, published=True) + assert result.returncode == 0, result.stderr + + +@pytest.mark.integration +async def test_a_real_bundle_fails_verification_after_one_edited_digit( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + """The seal is only worth what its sensitivity is worth.""" + await _seed_a_procedure_with_one_activity(db_pool) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + bundle = write_bundle(exported, manifest, tmp_path / "bundle") + assert _verify(bundle).returncode == 0 + + path = bundle / "logbooks" / "activity.jsonl" + tampered = path.read_text(encoding="utf-8").replace("423.0", "424.0") + path.write_text(tampered, encoding="utf-8") + + after = _verify(bundle) + assert after.returncode == 1 + assert "MISMATCH" in after.stderr + + +@pytest.mark.integration +async def test_both_reassembly_implementations_agree_on_a_real_bundle( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + """CORA's reader and the standalone script's reader are separate code + by design (F4). This pins them to the same answer on a real export, + which is the only place their drift would be caught.""" + await _seed_a_procedure_with_one_activity(db_pool) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + bundle = write_bundle(exported, manifest, tmp_path / "bundle") + + # CORA's reader reassembles the body; the script's reader then has to + # agree, twice over: once by hashing CORA's reassembly to the + # manifest value (below) and once by reassembling the bundle itself + # (`verify-bundle`, the tests above). Both paths must land on the + # same digest or the two implementations have drifted. + body_file = tmp_path / "body.json" + body_file.write_text(json.dumps(read_bundle_body(bundle)), encoding="utf-8") + result = _verify_body(body_file, manifest.record_hash) + assert result.returncode == 0, result.stderr diff --git a/apps/api/tests/unit/infrastructure/record_export/test_bundle.py b/apps/api/tests/unit/infrastructure/record_export/test_bundle.py new file mode 100644 index 0000000000..863548b5e6 --- /dev/null +++ b/apps/api/tests/unit/infrastructure/record_export/test_bundle.py @@ -0,0 +1,182 @@ +"""Unit tests for the bundle writer and its on-disk round trip. + +`write_bundle` is the step that turns in-memory structures into an +artifact somebody can archive. The property that matters is not "the +files exist" but "the body reassembled from those files hashes to what +the in-memory record hashed to", because that is the only thing a +reviewer's verification actually rests on. + +The subprocess test that runs `scripts/verify_record_hash.py` against a +real bundle with no `cora` on the path lives in +`test_standalone_verifier.py`, beside the rest of that script's tests. +""" + +import json +from pathlib import Path + +import pytest + +from cora.infrastructure.record_export import ( + LOGBOOKS_DIR, + MANIFEST_NAME, + RECORD_PAYLOAD_TYPE, + STREAMS_NAME, + BundleDestinationNotEmptyError, + ExportedRecord, + MalformedBundleError, + build_manifest, + hash_record, + hash_redacted_record, + read_bundle_body, + write_bundle, +) +from cora.infrastructure.record_export._redaction import RedactedRecord +from cora.shared.content_hash import compute_content_hash + +_COMMIT = "0" * 40 + + +def _record() -> ExportedRecord: + return ExportedRecord( + streams=( + { + "stream_type": "Run", + "stream_id": "01900000-0000-7000-8000-0000000000a1", + "event_type": "RunStarted", + "schema_version": 1, + "payload": {"note": "first"}, + }, + { + "stream_type": "Run", + "stream_id": "01900000-0000-7000-8000-0000000000a1", + "event_type": "RunCompleted", + "schema_version": 2, + "payload": {"note": "second"}, + }, + ), + logbooks={ + "activity": ( + {"event_id": "a1", "step_kind": "setpoint", "payload": {"channel": "2bma:x"}}, + {"event_id": "a2", "step_kind": "check", "payload": {"channel": "2bma:flux"}}, + ), + "observation": ({"event_id": "o1", "value": 1.5, "is_simulated": True},), + }, + ) + + +def _manifest(record: ExportedRecord) -> object: + return build_manifest(record, watermark=42, git_commit=_COMMIT) + + +def test_write_bundle_lays_out_the_names_the_design_fixed(tmp_path: Path) -> None: + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + bundle = tmp_path / "b" + assert (bundle / MANIFEST_NAME).is_file() + assert (bundle / STREAMS_NAME).is_file() + assert sorted(p.name for p in (bundle / LOGBOOKS_DIR).iterdir()) == [ + "activity.jsonl", + "observation.jsonl", + ] + + +def test_streams_file_is_one_json_object_per_line_in_export_order(tmp_path: Path) -> None: + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + lines = (tmp_path / "b" / STREAMS_NAME).read_text(encoding="utf-8").splitlines() + assert len(lines) == 2 + assert [json.loads(line)["event_type"] for line in lines] == ["RunStarted", "RunCompleted"] + + +def test_reassembled_body_hashes_to_the_in_memory_record_hash(tmp_path: Path) -> None: + """The whole point of the bundle: what came back off disk is what was hashed.""" + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + body = read_bundle_body(tmp_path / "b") + assert compute_content_hash(RECORD_PAYLOAD_TYPE, body) == hash_record(record) + + +def test_editing_one_value_on_disk_changed_the_hash(tmp_path: Path) -> None: + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + path = tmp_path / "b" / LOGBOOKS_DIR / "activity.jsonl" + path.write_text(path.read_text(encoding="utf-8").replace("2bma:x", "2bma:y"), encoding="utf-8") + + body = read_bundle_body(tmp_path / "b") + assert compute_content_hash(RECORD_PAYLOAD_TYPE, body) != hash_record(record) + + +def test_deleting_a_whole_logbook_kind_breaks_the_hash(tmp_path: Path) -> None: + """A per-file check would pass here. Only the reassembled body catches it.""" + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + (tmp_path / "b" / LOGBOOKS_DIR / "observation.jsonl").unlink() + + body = read_bundle_body(tmp_path / "b") + assert compute_content_hash(RECORD_PAYLOAD_TYPE, body) != hash_record(record) + + +def test_reordering_lines_breaks_the_hash(tmp_path: Path) -> None: + """Order is part of the record (F2's order key), not presentation.""" + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + path = tmp_path / "b" / STREAMS_NAME + path.write_text( + "".join(f"{line}\n" for line in reversed(path.read_text(encoding="utf-8").splitlines())), + encoding="utf-8", + ) + + body = read_bundle_body(tmp_path / "b") + assert compute_content_hash(RECORD_PAYLOAD_TYPE, body) != hash_record(record) + + +def test_writing_twice_into_one_directory_refuses(tmp_path: Path) -> None: + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + with pytest.raises(BundleDestinationNotEmptyError): + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + + +def test_bundle_without_manifest_refuses_to_read(tmp_path: Path) -> None: + """What an interrupted export leaves behind: rows, no manifest.""" + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + (tmp_path / "b" / MANIFEST_NAME).unlink() + + with pytest.raises(MalformedBundleError, match=MANIFEST_NAME): + read_bundle_body(tmp_path / "b") + + +def test_non_object_line_refuses_rather_than_reading_partially(tmp_path: Path) -> None: + record = _record() + write_bundle(record, _manifest(record), tmp_path / "b") # pyright: ignore[reportArgumentType] + (tmp_path / "b" / STREAMS_NAME).write_text('["not an object"]\n', encoding="utf-8") + + with pytest.raises(MalformedBundleError, match="not a JSON object"): + read_bundle_body(tmp_path / "b") + + +def test_manifest_carries_h3_only_when_a_redacted_record_is_supplied(tmp_path: Path) -> None: + record = _record() + redacted = RedactedRecord(streams=record.streams, logbooks=record.logbooks) + + without = build_manifest(record, watermark=42, git_commit=_COMMIT) + with_h3 = build_manifest(record, watermark=42, git_commit=_COMMIT, redacted=redacted) + + assert without.published_record_hash is None + assert with_h3.published_record_hash == hash_redacted_record(redacted) + + +def test_h1_and_h3_differ_even_when_redaction_changed_nothing() -> None: + """The payload type keeps them apart, so a reader can always tell a + published record from a full one by its hash alone.""" + record = _record() + redacted = RedactedRecord(streams=record.streams, logbooks=record.logbooks) + + assert hash_record(record) != hash_redacted_record(redacted) diff --git a/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py b/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py index 269157bd7e..aa9e687807 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py @@ -188,3 +188,91 @@ def test_cli_reports_a_clean_error_on_unreadable_input(tmp_path: Path) -> None: text=True, ) assert result.returncode == 2 + + +def _write_bundle_for_cli(tmp_path: Path, *, published: bool) -> Path: + """A real bundle, written by the real writer, for the CLI to check. + + Imports `cora` only to BUILD the fixture. The verification itself + runs as a subprocess that never imports `cora`, which is the + property under test. + """ + from cora.infrastructure.record_export import ExportedRecord, build_manifest, write_bundle + from cora.infrastructure.record_export._redaction import RedactedRecord + + record = ExportedRecord( + streams=( + { + "stream_type": "Run", + "stream_id": "01900000-0000-7000-8000-0000000000a1", + "event_type": "RunStarted", + "schema_version": 1, + "payload": {"note": _PRECOMPOSED_E_ACUTE, "target_value": 423.0}, + }, + ), + logbooks={"activity": ({"event_id": "a1", "payload": {"channel": "2bma:x"}},)}, + ) + redacted = ( + RedactedRecord(streams=record.streams, logbooks=record.logbooks) if published else None + ) + manifest = build_manifest(record, watermark=7, git_commit="0" * 40, redacted=redacted) + + bundle = tmp_path / "bundle" + write_bundle(redacted if redacted is not None else record, manifest, bundle) + return bundle + + +def _run_bundle_cli(bundle: Path, *, published: bool = False) -> subprocess.CompletedProcess[str]: + argv = [sys.executable, str(_SCRIPT), "verify-bundle", str(bundle)] + if published: + argv.append("--published") + return subprocess.run(argv, capture_output=True, text=True) + + +def test_cli_verify_bundle_accepts_a_freshly_written_bundle(tmp_path: Path) -> None: + """End to end, and the point of the whole exercise: a bundle CORA + produced verifies in a process that never imports CORA.""" + result = _run_bundle_cli(_write_bundle_for_cli(tmp_path, published=False)) + assert result.returncode == 0, result.stderr + assert "OK" in result.stdout + + +def test_cli_verify_bundle_fails_on_a_tampered_row(tmp_path: Path) -> None: + bundle = _write_bundle_for_cli(tmp_path, published=False) + path = bundle / "logbooks" / "activity.jsonl" + path.write_text(path.read_text(encoding="utf-8").replace("2bma:x", "2bma:y"), encoding="utf-8") + + result = _run_bundle_cli(bundle) + assert result.returncode == 1 + assert "MISMATCH" in result.stderr + + +def test_cli_verify_bundle_fails_when_a_whole_logbook_kind_is_removed(tmp_path: Path) -> None: + """A file-by-file check passes here; only the reassembled body catches it.""" + bundle = _write_bundle_for_cli(tmp_path, published=False) + (bundle / "logbooks" / "activity.jsonl").unlink() + + result = _run_bundle_cli(bundle) + assert result.returncode == 1 + assert "MISMATCH" in result.stderr + + +def test_cli_verify_bundle_checks_h3_for_a_published_bundle(tmp_path: Path) -> None: + result = _run_bundle_cli(_write_bundle_for_cli(tmp_path, published=True), published=True) + assert result.returncode == 0, result.stderr + + +def test_cli_published_flag_refuses_a_bundle_carrying_no_h3(tmp_path: Path) -> None: + """Absence of H3 is a signal, not a default: asking for a published + check on an unredacted bundle must refuse, never fall back to H1.""" + result = _run_bundle_cli(_write_bundle_for_cli(tmp_path, published=False), published=True) + assert result.returncode == 2 + assert "not a published projection" in result.stderr + + +def test_cli_verify_bundle_refuses_a_directory_missing_its_manifest(tmp_path: Path) -> None: + bundle = _write_bundle_for_cli(tmp_path, published=False) + (bundle / "manifest.json").unlink() + + result = _run_bundle_cli(bundle) + assert result.returncode == 2 diff --git a/scripts/verify_record_hash.py b/scripts/verify_record_hash.py index a77fe83404..059d961a99 100644 --- a/scripts/verify_record_hash.py +++ b/scripts/verify_record_hash.py @@ -28,9 +28,18 @@ Usage: python3 verify_record_hash.py hash --payload-type TYPE body.json python3 verify_record_hash.py verify --payload-type TYPE --expected-hash HASH body.json + python3 verify_record_hash.py verify-bundle path/to/bundle/ + python3 verify_record_hash.py verify-bundle path/to/bundle/ --published + +`verify-bundle` is the one a reviewer actually runs: point it at an +exported bundle directory and it reassembles the hashed body from +`streams.jsonl` plus `logbooks/*.jsonl`, recomputes the hash, and +compares against the manifest's own. `verify` is the stronger check, +because the expected hash comes from outside the bundle (a paper, a +DOI landing page) rather than from a file the same tamperer could edit. Exit codes: 0 success (hash printed, or verify matched); 1 verify -mismatch; 2 the input file could not be read or parsed as JSON. +mismatch; 2 the input could not be read or parsed. """ # pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false @@ -104,6 +113,102 @@ def _load_body(path: Path) -> Any: return json.loads(path.read_text(encoding="utf-8")) +MANIFEST_NAME = "manifest.json" +STREAMS_NAME = "streams.jsonl" +LOGBOOKS_DIR = "logbooks" + +RECORD_PAYLOAD_TYPE = "application/vnd.cora.record+json" +PUBLISHED_RECORD_PAYLOAD_TYPE = "application/vnd.cora.record-published+json" + + +def _read_jsonl(path: Path) -> list[Any]: + rows: list[Any] = [] + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + if not line.strip(): + continue + try: + row = json.loads(line) + except json.JSONDecodeError as exc: + message = f"{path.name} line {number} is not valid JSON: {exc}" + raise ValueError(message) from exc + if not isinstance(row, dict): + message = f"{path.name} line {number} is not a JSON object" + raise ValueError(message) + rows.append(row) + return rows + + +def read_bundle_body(bundle: Path) -> Any: + """Reassemble the hashed body from a bundle directory on disk. + + Deliberately duplicates + `cora.infrastructure.record_export._bundle.read_bundle_body`. That + duplication is the same argument as the rest of this file: a checker + that imported CORA's reassembly would confirm CORA's own idea of + what the bundle says, which is not a check. Reassembly is part of + what a verifier must independently believe, because a bundle whose + files are correct individually can still be missing a whole logbook + kind, and only the reassembled body's hash catches that. + """ + streams_path = bundle / STREAMS_NAME + if not streams_path.is_file(): + message = f"{bundle} has no {STREAMS_NAME}; not a bundle" + raise ValueError(message) + if not (bundle / MANIFEST_NAME).is_file(): + message = f"{bundle} has no {MANIFEST_NAME}; export may be incomplete" + raise ValueError(message) + + logbooks: dict[str, Any] = {} + logbooks_dir = bundle / LOGBOOKS_DIR + if logbooks_dir.is_dir(): + for path in sorted(logbooks_dir.glob("*.jsonl")): + logbooks[path.stem] = _read_jsonl(path) + + return {"streams": _read_jsonl(streams_path), "logbooks": logbooks} + + +def _verify_bundle(bundle: Path, *, published: bool) -> int: + """Recompute a bundle's own hash and compare it to its manifest. + + The manifest is read for the EXPECTED value only. That is not + circular: the hash covers the two tiers, the manifest is not in + them, so a tamperer who edits a row must also edit the manifest, and + a tamperer who edits the manifest has changed the number a paper + printed. Comparing against a hash quoted in a paper rather than in + the bundle is strictly stronger, and is what `verify` (not + `verify-bundle`) is for. + """ + try: + body = read_bundle_body(bundle) + manifest = _load_body(bundle / MANIFEST_NAME) + except (OSError, ValueError, json.JSONDecodeError) as exc: + print(f"cannot read bundle {bundle}: {exc}", file=sys.stderr) + return 2 + + if not isinstance(manifest, dict): + print(f"{MANIFEST_NAME} is not a JSON object", file=sys.stderr) + return 2 + + field = "published_record_hash" if published else "record_hash" + payload_type = PUBLISHED_RECORD_PAYLOAD_TYPE if published else RECORD_PAYLOAD_TYPE + expected = manifest.get(field) + if not isinstance(expected, str): + detail = ( + "this bundle is not a published projection (its manifest carries no H3)" + if published + else "manifest has no record_hash" + ) + print(f"cannot verify: {detail}", file=sys.stderr) + return 2 + + digest = compute_content_hash(payload_type, body) + if digest == expected: + print(f"OK {digest}") + return 0 + print(f"MISMATCH: manifest says {expected}, computed {digest}", file=sys.stderr) + return 1 + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser( description="Standalone content-hash verifier for a CORA record export." @@ -121,8 +226,28 @@ def main(argv: list[str] | None = None) -> int: verify_parser.add_argument("--expected-hash", required=True) verify_parser.add_argument("body_file", type=Path) + bundle_parser = subparsers.add_parser( + "verify-bundle", + help=( + "Verify a bundle directory against the hash in its own manifest. " + "Reads manifest.json, streams.jsonl and logbooks/*.jsonl." + ), + ) + bundle_parser.add_argument("bundle_dir", type=Path) + bundle_parser.add_argument( + "--published", + action="store_true", + help=( + "Check the bundle against the manifest's published_record_hash (H3) " + "instead of record_hash (H1). Use for a redacted bundle." + ), + ) + args = parser.parse_args(argv) + if args.command == "verify-bundle": + return _verify_bundle(args.bundle_dir, published=args.published) + try: body = _load_body(args.body_file) except (OSError, json.JSONDecodeError) as exc: From 9144168891deaff0e142c9b82734a7e6aee813d5 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:46:17 -0500 Subject: [PATCH 2/2] Report unfired tier-2 clearances instead of aborting the export `redact_record` used to raise unless every declared activity/payload clearance (channel, action_name, units) fired. Those three keys live on different step kinds and two are optional, so no narrow export -- most likely the first rehearsal bundle -- reliably fires all three. The check reasoned from a denylist's threat model (an unfired rule that should have hidden something is a leak) and applied it backwards to tier 2's allowlist, where an unfired rule means a field was published LESS than the profile permits, never more. There is no mechanism by which that leaks anything. unfired_clearances now returns the (kind, column, pointer) triples instead of raising, and RedactionResult / build_manifest thread the result onto the manifest as a completeness fact a reader can see, not a reason to refuse. A new regression test seeds the exact shape that used to abort (one setpoint, no units, no action, no check) and asserts it now redacts, verifies against H3, and the manifest names the two clearances the export was too narrow to exercise. The genuine worry the old check was reaching for -- a misspelled pointer in TIER2_JSONB_CLEARED_POINTERS that can never fire against any real payload -- is a fact about this file's code, not about any one export, and now has its own build-time check: test_tier2_jsonb_ clearances_are_real_keys.py. outcome.measurements gets a real introspection check against Measurement's actual dataclass fields; activity.payload has no typed contract, so it hand-encodes the known keys from append_activities/route.py's docstring plus the envelope keys conductor.py's _append_step merges into every kind. Both checks verified to fire on an injected typo and restore clean. Writing that hand-encoded key list surfaced two things beyond the completeness check's own scope, recorded in its docstring rather than acted on here: `STEP_KIND_VALUES` has five members and the payload docstring documents three (capture/compute are undocumented), and `result` -- a string closed in practice to exactly three constants (_RESULT_OK/_RESULT_FAILED/_RESULT_IN_FLIGHT, verified by grepping every call site) -- is written on every conductor-driven activity row and is not in the clearance set, so the published record cannot currently distinguish a step that succeeded from one that failed. Not added here: changing what gets published is a content decision, not a typo check. Refs: project_record_export_v3.md F5, project_record_export_build_brief.md Co-Authored-By: Claude Sonnet 5 --- .../infrastructure/record_export/__init__.py | 6 +- .../infrastructure/record_export/_manifest.py | 33 +++++ .../record_export/_redact_tier2.py | 66 ++++++---- .../record_export/_redaction.py | 27 +++- ...st_tier2_jsonb_clearances_are_real_keys.py | 118 +++++++++++++++++ .../test_record_export_bundle_postgres.py | 124 +++++++++++++++--- .../record_export/test_manifest.py | 38 ++++++ .../record_export/test_redact_tier2.py | 32 +++-- 8 files changed, 375 insertions(+), 69 deletions(-) create mode 100644 apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py diff --git a/apps/api/src/cora/infrastructure/record_export/__init__.py b/apps/api/src/cora/infrastructure/record_export/__init__.py index 0c8e8c1318..12630c31c7 100644 --- a/apps/api/src/cora/infrastructure/record_export/__init__.py +++ b/apps/api/src/cora/infrastructure/record_export/__init__.py @@ -62,9 +62,8 @@ TIER2_DISPOSITIONS, TIER2_JSONB_CLEARED_POINTERS, TIER2_JSONB_DROPPED_COLUMNS, - UnfiredClearanceError, - ensure_all_clearances_fired, redact_tier2_row, + unfired_clearances, ) from cora.infrastructure.record_export._redaction import ( RedactedRecord, @@ -114,7 +113,6 @@ "Tier1Redactor", "TokenMap", "TwoTierRecord", - "UnfiredClearanceError", "UnknownEventTypeError", "UnknownLogbookKindError", "UnknownStreamTypeError", @@ -122,7 +120,6 @@ "build_manifest", "capture_git_commit", "capture_watermark", - "ensure_all_clearances_fired", "ensure_stream_type_known", "export_record", "hash_logbooks", @@ -138,5 +135,6 @@ "render_row", "render_value", "resolve", + "unfired_clearances", "write_bundle", ] diff --git a/apps/api/src/cora/infrastructure/record_export/_manifest.py b/apps/api/src/cora/infrastructure/record_export/_manifest.py index f950e7c777..7a9049cbb4 100644 --- a/apps/api/src/cora/infrastructure/record_export/_manifest.py +++ b/apps/api/src/cora/infrastructure/record_export/_manifest.py @@ -62,6 +62,23 @@ class Manifest: because H3 hashes the two tiers only. The manifest is not in its own hashed body, so there is no circularity to resolve. """ + unfired_tier2_clearances: tuple[str, ...] | None = None + """Declared tier-2 jsonb clearances (`"kind/column/pointer"`) that + never matched a row in a kind this export carried. `None` means "no + redaction happened", the same absence-is-the-signal convention as + `published_record_hash`; an empty tuple means redaction happened and + every clearance fired. + + This is a COMPLETENESS fact, not a safety finding. Tier 2's + dispositions are an allowlist, so an unfired clearance means a field + was published less often than the profile permits, never more -- + see `unfired_clearances`'s own docstring for the full argument and + the denylist-shaped mistake this field replaces (an earlier version + aborted the export instead of reporting this). A reviewer reading a + non-empty list here learns "this export was too narrow to exercise + every rule the profile declares", which is a caveat about coverage, + not a leak. + """ def capture_git_commit(*, cwd: Path | str | None = None) -> str: @@ -151,12 +168,17 @@ def _expansion_digest_presence_by_run(record: ExportedRecord) -> dict[str, bool] } +def _render_unfired_clearances(unfired: frozenset[tuple[str, str, str]]) -> tuple[str, ...]: + return tuple(sorted(f"{kind}/{column}/{pointer}" for kind, column, pointer in unfired)) + + def build_manifest( record: ExportedRecord, *, watermark: int, git_commit: str, redacted: TwoTierRecord | None = None, + unfired_tier2_clearances: frozenset[tuple[str, str, str]] | None = None, ) -> Manifest: """Assemble the manifest for one already-exported, already-rendered record. @@ -167,6 +189,12 @@ def build_manifest( one, so the counts describe both, and deriving them from the unredacted side keeps a reader's recomputation honest if redaction ever does start dropping rows. + + Pass `unfired_tier2_clearances` (a `RedactionResult.unfired_tier2_clearances`) + alongside `redacted` so the manifest carries the completeness caveat + described on `Manifest.unfired_tier2_clearances`. Meaningless without + `redacted` and ignored if `redacted` is `None`, matching that field's + same "no redaction happened" absence. """ return Manifest( git_commit=git_commit, @@ -178,6 +206,11 @@ def build_manifest( is_simulated=_is_simulated(record), expansion_digest_presence_by_run=_expansion_digest_presence_by_run(record), published_record_hash=None if redacted is None else hash_redacted_record(redacted), + unfired_tier2_clearances=( + None + if redacted is None + else _render_unfired_clearances(unfired_tier2_clearances or frozenset()) + ), ) diff --git a/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py b/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py index 2167d52382..7d85b9376f 100644 --- a/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py +++ b/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py @@ -170,25 +170,6 @@ TIER2_JSONB_DROPPED_COLUMNS: frozenset[tuple[str, str]] = frozenset({("inference", "messages")}) -class UnfiredClearanceError(RuntimeError): - """A declared tier-2 jsonb clearance never matched a row in a kind - that WAS present in this export. - - Per F5's Rejections list: an unfired rule inside a type that was - exported is a leak-shaped gap (most likely a typo in this file's - own `TIER2_JSONB_CLEARED_POINTERS`), not a benign no-op. - """ - - def __init__(self, kind: str, column: str, pointer: str) -> None: - super().__init__( - f"declared clearance ({kind!r}, {column!r})/{pointer!r} never matched " - f"any row while redacting {kind!r} rows present in this export." - ) - self.kind = kind - self.column = column - self.pointer = pointer - - def redact_tier2_row( kind: str, row: dict[str, Any], @@ -225,25 +206,54 @@ def redact_tier2_row( return result -def ensure_all_clearances_fired( +def unfired_clearances( fired_pointers: dict[tuple[str, str], set[str]], *, kinds_present: frozenset[str] -) -> None: - """Raise `UnfiredClearanceError` for a declared clearance that never - matched, scoped to kinds actually present in this export (an unused - clearance for a kind with zero rows is not a leak -- nothing to leak).""" +) -> frozenset[tuple[str, str, str]]: + """Every declared tier-2 jsonb clearance that never matched a row, + scoped to kinds actually present in this export. + + CORRECTED 2026-08-12. An earlier version of this function + (`ensure_all_clearances_fired`) raised here, reasoning from F5's + Rejections list: "an unfired rule inside a type that was exported is + a leak-shaped gap." That reasoning is right for a DENYLIST, where a + rule that fails to fire means the thing it should have hidden got + published. It is backwards for tier 2's ALLOWLIST: a clearance that + never fires means a field was published LESS often than the profile + permits, never more. There is no mechanism by which that leaks + anything, so treating it as fatal was importing a denylist-shaped + fear into an allowlist-shaped mechanism. + + The practical failure this produced: `activity/payload`'s three + cleared pointers (`channel`, `action_name`, `units`) live on + different step kinds, two of them optional, so no small export -- + including a first rehearsal bundle -- reliably fires all three. The + export would abort with an error reading like a broken disposition + table rather than "this export was too narrow to exercise every + clearance." + + Callers now record the result on the manifest + (`Manifest.unfired_tier2_clearances`) instead of treating it as a + reason to refuse. The genuine worry this WAS reaching for --a + misspelled pointer in `TIER2_JSONB_CLEARED_POINTERS` that can never + fire against any real payload-- is a fact about this file's code, + not about any one export, and belongs in a build-time check against + the real key space (the same shape as step 0's generated disposition + table), not in a per-export runtime gate. That check does not exist + yet; this function no longer stands in for it. + """ + result: set[tuple[str, str, str]] = set() for (kind, column), cleared in TIER2_JSONB_CLEARED_POINTERS.items(): if kind not in kinds_present: continue fired = fired_pointers.get((kind, column), set()) - for pointer in cleared - fired: - raise UnfiredClearanceError(kind, column, pointer) + result.update((kind, column, pointer) for pointer in cleared - fired) + return frozenset(result) __all__ = [ "TIER2_DISPOSITIONS", "TIER2_JSONB_CLEARED_POINTERS", "TIER2_JSONB_DROPPED_COLUMNS", - "UnfiredClearanceError", - "ensure_all_clearances_fired", "redact_tier2_row", + "unfired_clearances", ] diff --git a/apps/api/src/cora/infrastructure/record_export/_redaction.py b/apps/api/src/cora/infrastructure/record_export/_redaction.py index 6a16ccf93e..f0342c0eae 100644 --- a/apps/api/src/cora/infrastructure/record_export/_redaction.py +++ b/apps/api/src/cora/infrastructure/record_export/_redaction.py @@ -15,8 +15,8 @@ from cora.infrastructure.record_export._hashing import hash_redaction_profile from cora.infrastructure.record_export._redact_tier1 import Tier1Redactor, UnknownEventTypeError from cora.infrastructure.record_export._redact_tier2 import ( - ensure_all_clearances_fired, redact_tier2_row, + unfired_clearances, ) from cora.infrastructure.record_export._tokens import TokenMap @@ -60,10 +60,22 @@ class RedactionResult: """`redacted_record` is safe to hash as the published record. `token_map` is an artifact of THIS export, retained separately under H1's obligation; it must never be shipped or fed into anything - hashed as the published record.""" + hashed as the published record. + + `unfired_tier2_clearances` names every declared jsonb clearance + (`kind`, `column`, `pointer`) that never matched a row in a kind + THIS export actually carried. It is a completeness fact about the + export, not a safety finding: tier 2 is an allowlist, so a + clearance not firing means a field was published less often than + permitted, never more. Callers pass it to `build_manifest` so a + reader can see, from the artifact itself, which parts of the + redaction profile this particular export was too narrow to + exercise. + """ redacted_record: RedactedRecord token_map: TokenMap + unfired_tier2_clearances: frozenset[tuple[str, str, str]] def redact_record( @@ -73,9 +85,11 @@ def redact_record( Raises `RedactionProfileMismatchError` before touching any row if the hash does not match; `UnknownEventTypeError` if a stream row's - `event_type` has no entry in the disposition table at all; - `UnfiredClearanceError` if a declared tier-2 jsonb clearance never - matched a row in a kind present in this export. + `event_type` has no entry in the disposition table at all. Does NOT + raise over an unfired tier-2 clearance; see + `RedactionResult.unfired_tier2_clearances` and + `unfired_clearances`'s own docstring for why an earlier version of + this function did and was wrong to. """ actual_hash = hash_redaction_profile() if expected_redaction_profile_hash != actual_hash: @@ -94,9 +108,10 @@ def redact_record( ) for kind, rows in record.logbooks.items() } - ensure_all_clearances_fired(fired_pointers, kinds_present=frozenset(record.logbooks)) + unfired = unfired_clearances(fired_pointers, kinds_present=frozenset(record.logbooks)) return RedactionResult( redacted_record=RedactedRecord(streams=redacted_streams, logbooks=redacted_logbooks), token_map=token_map, + unfired_tier2_clearances=unfired, ) diff --git a/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py b/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py new file mode 100644 index 0000000000..6ef7a2c359 --- /dev/null +++ b/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py @@ -0,0 +1,118 @@ +"""Every string leaf pointer in `TIER2_JSONB_CLEARED_POINTERS` must name +a key that can actually appear in the jsonb column it clears. + +This is the build-time check the removed `UnfiredClearanceError` was +standing in for (see `_redact_tier2.unfired_clearances`'s docstring). +That check ran per export and conflated two different questions: "is +this export narrow?" (not a bug) and "is this pointer typed correctly at +all?" (a real bug, most likely a hand-typed typo). This file asks only +the second question, once, against the real key space, the same shape +as step 0's generated disposition table -- resolve the truth once, +commit the check, let redaction read inert data. + +Two columns, two different strengths of check, and the difference is +honest rather than hidden: + +- `outcome.measurements` is backed by a real dataclass + (`cora.operation.ports.measurement.Measurement`), so this file + INTROSPECTS it. A misspelled pointer here fails for the same reason a + misspelled attribute access would. +- `activity.payload` has NO typed contract. Its shape lives in + `append_activities/route.py`'s docstring (three of the five + `STEP_KIND_VALUES` -- setpoint/action/check; capture/compute are + undocumented) plus in `conductor.py`'s `_append_step`, which merges in + `step_index` / `result` / `error_class?` / `message?` on EVERY kind, + none of which the docstring mentions. This file therefore + HAND-ENCODES the known keys from both sources and checks against + their union. That is a stopgap, not a fix: it catches a pointer that + matches NO known key, but it cannot detect a typo that happens to + collide with a different real key, and it says nothing about whether + the CLEARED set is complete (see the note on `result` below). Step 3 + in `project_record_export_build_brief.md` -- per-kind typed payloads + -- is what would let this become a real introspection check like the + measurements one; it is not built. + +FOUND while writing this, and NOT acted on here, because changing what +gets published is a content decision, not a typo check: `result` is +written on every conductor-driven activity row +(`conductor.py:3956` / `_append_step`) and is, in practice, drawn from +exactly three module-level string constants (`_RESULT_OK = "ok"`, +`_RESULT_FAILED = "failed"`, `_RESULT_IN_FLIGHT = "in_flight"`, +verified by grepping every `result=` call site in `conductor.py`), the +same "closed in practice, declared as bare `str`" shape as several +existing JUDGED LOW RISK tier-2 clearances. It is NOT in +`TIER2_JSONB_CLEARED_POINTERS`, so it drops on every export today, +which means the published record cannot currently distinguish a step +that succeeded from one that failed. `_KNOWN_ACTIVITY_PAYLOAD_KEYS` +below lists `result` as a known key precisely so this file's own +completeness gap is visible in its source rather than silently absent. +""" + +import dataclasses + +from cora.infrastructure.record_export._redact_tier2 import TIER2_JSONB_CLEARED_POINTERS +from cora.operation.ports.measurement import Measurement + +# Per `append_activities/route.py`'s payload docstring (setpoint/action/ +# check only) plus the envelope keys `conductor.py`'s `_append_step` +# merges into EVERY kind's payload (`step_index`, `result`, and +# `error_class` / `message` on failure). `capture` and `compute` are two +# of `STEP_KIND_VALUES`'s five members and are NOT represented here: no +# docstring or call site documents their payload shape, which is itself +# evidence for project_record_export_build_brief.md's step 3. +_KNOWN_ACTIVITY_PAYLOAD_KEYS = frozenset( + { + "channel", + "target_value", + "units", + "ramp_rate", + "action_name", + "params", + "passed", + "expected", + "actual", + "tolerance", + "step_index", + "result", + "error_class", + "message", + } +) + + +def test_activity_payload_clearances_are_within_the_known_key_space() -> None: + cleared = TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")] + unknown = cleared - _KNOWN_ACTIVITY_PAYLOAD_KEYS + assert not unknown, ( + f"TIER2_JSONB_CLEARED_POINTERS[('activity', 'payload')] clears {sorted(unknown)}, " + "which names no key documented in append_activities/route.py or written by " + "conductor.py's _append_step. Likely a typo; see this file's module docstring." + ) + + +def test_outcome_measurements_clearances_are_real_measurement_fields() -> None: + """`*/name`, `*/units`, `*/kind`, `*/quality` per element of the + `measurements` list: strip the `*/` list-element marker and check + the remainder against `Measurement`'s actual dataclass fields.""" + real_fields = frozenset(f.name for f in dataclasses.fields(Measurement)) + cleared = TIER2_JSONB_CLEARED_POINTERS[("outcome", "measurements")] + + for pointer in cleared: + assert pointer.startswith("*/"), ( + f"{pointer!r} does not use the '*/' any-list-element marker " + "this test assumes for outcome.measurements" + ) + field_name = pointer.removeprefix("*/") + assert field_name in real_fields, ( + f"TIER2_JSONB_CLEARED_POINTERS[('outcome', 'measurements')] clears " + f"{pointer!r}, but Measurement has no field {field_name!r}. " + f"Real fields: {sorted(real_fields)}." + ) + + +def test_outcome_measurements_never_clears_the_opaque_diagnostic_field() -> None: + """`quality_detail` is documented as free-form substrate forensic + text (`measurement.py`); it must never appear in the cleared set, + however the check above is implemented.""" + cleared = TIER2_JSONB_CLEARED_POINTERS[("outcome", "measurements")] + assert "*/quality_detail" not in cleared diff --git a/apps/api/tests/integration/test_record_export_bundle_postgres.py b/apps/api/tests/integration/test_record_export_bundle_postgres.py index d1128e4a29..41034e0839 100644 --- a/apps/api/tests/integration/test_record_export_bundle_postgres.py +++ b/apps/api/tests/integration/test_record_export_bundle_postgres.py @@ -12,19 +12,20 @@ drift between the two reassembly implementations shows up here and nowhere else. -FOUND WHILE WRITING THIS, and not fixed here because loosening a -fail-closed check is a security decision rather than a test fixture's -business: `ensure_all_clearances_fired` raises unless EVERY declared -`activity/payload` clearance fires, and those three keys (`channel`, +FOUND WHILE WRITING THIS, and FIXED separately (same session, next +commit): `redact_record` used to raise unless EVERY declared +`activity/payload` clearance fired, and those three keys (`channel`, `action_name`, `units`) live on different step kinds, two of them -optional per `append_activities/route.py:86-94`. So `redact_record` -refuses on any export whose activities happen not to include a -setpoint-carrying-units AND an action AND a check. Step 6's own -integration fixture already works around it with a comment reasoning -that a whole-database export "would plausibly exercise every step -kind". That is an assumption about data, holding up a hard refusal. It -fails CLOSED, so nothing leaks, but the first small rehearsal bundle, -or any future per-run slice, would hit it and read as a config error. +optional per `append_activities/route.py:86-94`, so a narrow export +(a single setpoint, say) aborted instead of exporting. The check +reasoned from a denylist's threat model (an unfired rule that should +have hidden something is a leak) applied backwards to tier 2's +allowlist (an unfired rule here means something was published LESS +than the profile permits, never more). `unfired_clearances` now reports +the fact on the manifest instead of aborting; see its docstring in +`_redact_tier2.py` for the full argument. This test's fixture still +seeds three step kinds, not to dodge an abort that no longer exists, +but because it is better coverage of the redaction path than one kind. """ # pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false @@ -102,14 +103,11 @@ async def _seed_a_procedure_with_one_activity(db_pool: asyncpg.Pool) -> None: events=[new_event], ) - # Three step kinds, not one, and NOT because this test needs three. - # `ensure_all_clearances_fired` raises unless every declared - # `activity/payload` clearance (`channel`, `action_name`, `units`) - # matches at least one row, and those keys live on DIFFERENT step - # kinds, two of them optional. A setpoint-only export therefore - # fails to redact. See this module's docstring: that is a real - # fragility in step 6, worked around here so these tests exercise - # the BUNDLE rather than accidentally re-testing the clearance rule. + # Three step kinds, exercising a wider slice of tier-2 redaction + # (setpoint/action/check each carry different payload keys) than a + # single kind would. A one-kind fixture would previously have + # aborted `redact_record` outright; see the module docstring for + # why that is no longer possible. handler = bind_append(deps, step_store=PostgresActivityStore(db_pool)) await handler( AppendProcedureActivities( @@ -202,6 +200,7 @@ async def test_a_real_redacted_export_verifies_against_h3( watermark=1, git_commit=capture_git_commit(), redacted=redaction.redacted_record, + unfired_tier2_clearances=redaction.unfired_tier2_clearances, ) bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") @@ -209,6 +208,91 @@ async def test_a_real_redacted_export_verifies_against_h3( assert result.returncode == 0, result.stderr +@pytest.mark.integration +async def test_a_narrow_export_redacts_and_reports_what_it_could_not_exercise( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + """The regression test for the defect this module's docstring + describes. A single setpoint, no `units`, no `action`, no `check`: + exactly the shape that used to abort `redact_record` outright. + + It must now redact successfully, verify against H3, AND the + manifest must name the two clearances this narrow export could not + exercise -- proving the fact is surfaced, not just silently dropped. + """ + procedure_id = uuid4() + logbook_id = uuid4() + open_event_id = uuid4() + deps = build_postgres_deps(db_pool, now=_NOW, ids=[logbook_id, open_event_id]) + + registered = ProcedureRegistered( + procedure_id=procedure_id, + name="Narrow rehearsal", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + occurred_at=_NOW, + ) + started = ProcedureStarted(procedure_id=procedure_id, occurred_at=_NOW) + for index, event in enumerate((registered, started)): + new_event = to_new_event( + event_type=event_type_name(event), + payload=to_payload(event), + occurred_at=event.occurred_at, + event_id=uuid4(), + command_name="RegisterProcedure" if index == 0 else "StartProcedure", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ) + await deps.event_store.append( + stream_type="Procedure", + stream_id=procedure_id, + expected_version=index, + events=[new_event], + ) + + handler = bind_append(deps, step_store=PostgresActivityStore(db_pool)) + await handler( + AppendProcedureActivities( + procedure_id=procedure_id, + entries=( + ActivityInput( + event_id=uuid4(), + step_kind="setpoint", + payload={"channel": "T_oven", "target_value": 423.0}, # no units + sampled_at=_NOW, + ), + ), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + # This call itself is the regression assertion: it used to raise + # UnfiredClearanceError for exactly this fixture. + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + + manifest = build_manifest( + exported, + watermark=1, + git_commit=capture_git_commit(), + redacted=redaction.redacted_record, + unfired_tier2_clearances=redaction.unfired_tier2_clearances, + ) + assert manifest.unfired_tier2_clearances == ( + "activity/payload/action_name", + "activity/payload/units", + ) + + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "narrow") + result = _verify(bundle, published=True) + assert result.returncode == 0, result.stderr + + @pytest.mark.integration async def test_a_real_bundle_fails_verification_after_one_edited_digit( db_pool: asyncpg.Pool, tmp_path: Path diff --git a/apps/api/tests/unit/infrastructure/record_export/test_manifest.py b/apps/api/tests/unit/infrastructure/record_export/test_manifest.py index 334864a5b6..2add776afe 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_manifest.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_manifest.py @@ -137,3 +137,41 @@ def test_manifest_carries_the_watermark_and_commit_verbatim() -> None: def test_capture_git_commit_returns_a_full_sha() -> None: commit = capture_git_commit() assert re.fullmatch(r"[0-9a-f]{40}", commit) + + +def test_unfired_tier2_clearances_absent_without_redaction() -> None: + """`None` means no redaction happened, the same convention as + `published_record_hash`; unrelated to whether any clearance would + have fired.""" + manifest = build_manifest(_record(), watermark=1, git_commit="deadbeef") + assert manifest.unfired_tier2_clearances is None + + +def test_unfired_tier2_clearances_empty_when_none_supplied_but_redacted() -> None: + """Passing `redacted` without `unfired_tier2_clearances` reports an + empty tuple, not `None`: redaction DID happen, so absence-as-signal + no longer applies, and "empty" correctly reads as "nothing to + report" rather than "not tracked".""" + record = _record() + manifest = build_manifest(record, watermark=1, git_commit="deadbeef", redacted=record) + assert manifest.unfired_tier2_clearances == () + + +def test_unfired_tier2_clearances_renders_sorted_kind_column_pointer() -> None: + record = _record() + manifest = build_manifest( + record, + watermark=1, + git_commit="deadbeef", + redacted=record, + unfired_tier2_clearances=frozenset( + { + ("activity", "payload", "units"), + ("activity", "payload", "channel"), + } + ), + ) + assert manifest.unfired_tier2_clearances == ( + "activity/payload/channel", + "activity/payload/units", + ) diff --git a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py index 3829d3c2ef..fb8c4d912b 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py @@ -1,6 +1,6 @@ """Unit tests for tier-2 (`entries_*`) redaction: the hand-authored per-kind disposition table, jsonb recursion, and the unfired-clearance -check.""" +report.""" from uuid import uuid4 @@ -9,9 +9,8 @@ from cora.infrastructure.record_export import TokenMap from cora.infrastructure.record_export._redact_tier2 import ( TIER2_DISPOSITIONS, - UnfiredClearanceError, - ensure_all_clearances_fired, redact_tier2_row, + unfired_clearances, ) @@ -128,26 +127,37 @@ def test_every_declared_kind_has_at_least_one_uuid_scope_column(kind: str) -> No assert "token" in TIER2_DISPOSITIONS[kind].values() -def test_unfired_clearance_raises_when_a_declared_pointer_never_matched() -> None: +def test_unfired_clearance_names_the_pointer_that_never_matched() -> None: + """A narrow export (one setpoint, no units) is a normal export, not + an error: CORRECTED 2026-08-12, this used to raise. See + `unfired_clearances`'s own docstring for why raising here was a + denylist-shaped mistake applied to an allowlist mechanism.""" # No channel/action_name/units in this payload. row = {"event_id": str(uuid4()), "payload": {"target_value": 423.0}} fired: dict[tuple[str, str], set[str]] = {} redact_tier2_row("activity", row, token_map=TokenMap(), fired_pointers=fired) - with pytest.raises(UnfiredClearanceError): - ensure_all_clearances_fired(fired, kinds_present=frozenset({"activity"})) + unfired = unfired_clearances(fired, kinds_present=frozenset({"activity"})) -def test_unfired_clearance_does_not_raise_for_a_kind_not_present() -> None: + assert unfired == { + ("activity", "payload", "channel"), + ("activity", "payload", "action_name"), + ("activity", "payload", "units"), + } + + +def test_unfired_clearance_for_a_kind_not_present_reports_empty() -> None: """An unused clearance for a kind with zero rows in this export is - not a leak -- nothing exported to leak.""" - ensure_all_clearances_fired({}, kinds_present=frozenset({"verdict"})) + not a completeness gap -- nothing exported to have exercised it.""" + assert unfired_clearances({}, kinds_present=frozenset({"verdict"})) == frozenset() -def test_all_declared_clearances_fire_when_every_pointer_is_exercised() -> None: +def test_all_declared_clearances_fired_reports_empty() -> None: row = { "event_id": str(uuid4()), "payload": {"channel": "T_oven", "action_name": "open_valve", "units": "K"}, } fired: dict[tuple[str, str], set[str]] = {} redact_tier2_row("activity", row, token_map=TokenMap(), fired_pointers=fired) - ensure_all_clearances_fired(fired, kinds_present=frozenset({"activity"})) # must not raise + + assert unfired_clearances(fired, kinds_present=frozenset({"activity"})) == frozenset()