From 24d7a1b45ec066278d9183ed577bfa15a2d0a26b Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Tue, 21 Jul 2026 10:11:22 -0600 Subject: [PATCH] fix(release): close audit gaps in advisory and proof validation validate_install_proof could raise ValueError on a retained proof with a non-integer bytes value instead of returning the standard failure list. That path became reachable for more callers once the semantic pass stopped being gated behind optional bindings, and in the CLI it surfaced through the generic exception renderer with repair: bash scripts/release.sh --candidate, which is actively wrong advice during --recover. It now fails closed with actionable repair text. --recover validated retained policy timestamps with a regex that accepts impossible values such as month 99, so recover could accept policy_run state the live rail could never produce. Retained validation now matches live PolicyRun strength across every policy timestamp field. Advisory freshness failures carried a copy-pasted repair string that told an operator nothing. They now name the real repair, and distinguish a stale fetch from stale content from a clock problem. The liveness gate imported three underscore-private helpers from the advisory rail. With two consumers these are API, so they are public names now, with no aliases left behind. Identity helpers still named their parameter db_root after identity moved to the snapshot. Rename those parameters to match the contract the module docstring states, with no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/check_release_advisory_liveness.py | 12 +++--- scripts/release_advisory_policy.py | 35 +++++++++------- scripts/release_candidate_driver.py | 36 ++++++++++++----- scripts/release_install_smoke.py | 47 ++++++++++++++++------ tests/test_release_candidate_driver.py | 19 +++++++++ tests/test_release_install_smoke.py | 43 ++++++++++++++++++++ 6 files changed, 148 insertions(+), 44 deletions(-) diff --git a/scripts/check_release_advisory_liveness.py b/scripts/check_release_advisory_liveness.py index d77a9ac86..234855689 100644 --- a/scripts/check_release_advisory_liveness.py +++ b/scripts/check_release_advisory_liveness.py @@ -24,9 +24,9 @@ from scripts.check_release_preflight import check_cargo_deny # noqa: E402 from scripts.check_rust_release_manifest import Failure # noqa: E402 from scripts.release_advisory_policy import ( # noqa: E402 ReleasePolicyError, - _advisory_check_argv, - _locate_advisory_snapshot, - _materialized_config_bytes, + advisory_check_argv, + locate_advisory_snapshot, + materialized_config_bytes, prepare_policy_run, ) @@ -193,7 +193,7 @@ def _commit_all(repo: Path, *, message: str, commit_date: str) -> None: def _write_config(config_path: Path, db_root: Path) -> None: config_path.write_bytes( - _materialized_config_bytes( + materialized_config_bytes( (ROOT / "core" / "deny.toml").read_bytes(), db_root=db_root, db_urls=(LOGICAL_DB_URL,), @@ -240,7 +240,7 @@ def _materialize( ], env=env, ) - snapshot = _locate_advisory_snapshot(db_root) + snapshot = locate_advisory_snapshot(db_root) return MaterializedDb( case_root=case_root, source_repo=source_repo, @@ -318,7 +318,7 @@ def _case_stale_snapshot_fails_binary(commit_date: str) -> None: db = _materialize("stale-binary", fixture_name="valid", commit_date=commit_date) _touch(db.snapshot / ".git" / "FETCH_HEAD", "20 days ago") result = _run( - _advisory_check_argv("cargo-deny", db.config_path, ROOT), + advisory_check_argv("cargo-deny", db.config_path, ROOT), check=False, ) # Layer (c): this is cargo-deny's own stale check, not the rail's Python check. diff --git a/scripts/release_advisory_policy.py b/scripts/release_advisory_policy.py index e148abb69..2fd230e01 100644 --- a/scripts/release_advisory_policy.py +++ b/scripts/release_advisory_policy.py @@ -60,6 +60,11 @@ SOURCE_ID_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*$") MAXIMUM_DB_FETCH_STALENESS_DELTA = timedelta(hours=24) MAXIMUM_DB_CONTENT_AGE_DELTA = timedelta(days=14) ARCHIVE_PREFIX = "advisory-db/" +ADVISORY_DB_FETCH_REPAIR = ( + "reacquire RELEASE_ADVISORY_DB_ROOT with cargo-deny --config " + "--manifest-path core/Cargo.toml fetch db twice" +) +ADVISORY_CLOCK_REPAIR = f"check the system clock, then {ADVISORY_DB_FETCH_REPAIR}" @dataclass(frozen=True) @@ -120,7 +125,7 @@ def _validate_policy_run_receipt(policy_run: PolicyRun) -> list[Failure]: "policy_checked_at", ): value = getattr(policy_run, key) - if not _is_normalized_utc_timestamp(value): + if not is_normalized_utc_timestamp(value): failures.append( _failure( f"policy_run.{key} is invalid", @@ -301,7 +306,7 @@ def _validate_source( return failures -def _materialized_config_bytes( +def materialized_config_bytes( base_bytes: bytes, *, db_root: Path, @@ -352,7 +357,7 @@ def _write_materialized_config( db_root: Path, db_urls: Sequence[str], ) -> Path: - materialized = _materialized_config_bytes( + materialized = materialized_config_bytes( (root / "core" / "deny.toml").read_bytes(), db_root=db_root, db_urls=db_urls, @@ -403,7 +408,7 @@ def _realpath(path: Path) -> Path: return path.resolve(strict=False) -def _locate_advisory_snapshot(db_root: Path) -> Path: +def locate_advisory_snapshot(db_root: Path) -> Path: try: entries = sorted(db_root.iterdir(), key=lambda path: path.name) except OSError as exc: @@ -543,11 +548,11 @@ def _strip_one_trailing_newline(value: str) -> str: return value[:-1] if value.endswith("\n") else value -def _git_db_commit(runner: Runner, db_root: Path) -> str: +def _git_db_commit(runner: Runner, snapshot: Path) -> str: value = _strip_one_trailing_newline( _run( runner, - ["git", "-C", str(db_root), "rev-parse", "--verify", "HEAD^{commit}"], + ["git", "-C", str(snapshot), "rev-parse", "--verify", "HEAD^{commit}"], ).stdout ) failures = validate_snapshot_identity( @@ -565,7 +570,7 @@ def _git_db_commit_timestamp(runner: Runner, snapshot: Path) -> datetime: return _parse_utc(value, label="advisory db commit timestamp") -def _advisory_check_argv(cargo_deny: str, config_path: Path, root: Path) -> list[str]: +def advisory_check_argv(cargo_deny: str, config_path: Path, root: Path) -> list[str]: return [ cargo_deny, "--config", @@ -612,12 +617,12 @@ def _assert_scanned_snapshot(stderr: str, snapshot: Path) -> None: ) -def _default_archive_hasher(db_root: Path) -> str: +def _default_archive_hasher(snapshot: Path) -> str: result = subprocess.run( [ "git", "-C", - str(db_root), + str(snapshot), "archive", "--format=tar", f"--prefix={ARCHIVE_PREFIX}", @@ -676,7 +681,7 @@ def _format_utc(value: datetime) -> str: ) -def _is_normalized_utc_timestamp(value: object) -> bool: +def is_normalized_utc_timestamp(value: object) -> bool: if not isinstance(value, str) or not RFC3339_UTC_RE.fullmatch(value): return False try: @@ -735,7 +740,7 @@ def _validate_acquisition_freshness( "advisory fetch time is in the future", expected="FETCH_HEAD mtime at or before policy check time", actual=advisory_acquired_at, - repair="python3 scripts/check_rust_release_manifest.py", + repair=ADVISORY_CLOCK_REPAIR, ) ) elif policy_utc - fetch_acquired > MAXIMUM_DB_FETCH_STALENESS_DELTA: @@ -747,7 +752,7 @@ def _validate_acquisition_freshness( f"{_duration_label(MAXIMUM_DB_FETCH_STALENESS_DELTA)}" ), actual=advisory_acquired_at, - repair="python3 scripts/check_rust_release_manifest.py", + repair=ADVISORY_DB_FETCH_REPAIR, ) ) if db_commit_time > policy_utc: @@ -756,7 +761,7 @@ def _validate_acquisition_freshness( "advisory db commit timestamp is in the future", expected="HEAD commit timestamp at or before policy check time", actual=db_commit_timestamp, - repair="python3 scripts/check_rust_release_manifest.py", + repair=ADVISORY_CLOCK_REPAIR, ) ) elif policy_utc - db_commit_time > MAXIMUM_DB_CONTENT_AGE_DELTA: @@ -816,7 +821,7 @@ def prepare_policy_run( db_urls=db_urls, ) - snapshot = _locate_advisory_snapshot(db_root) + snapshot = locate_advisory_snapshot(db_root) _assert_snapshot_git_top_level(runner, snapshot) _assert_snapshot_clean(runner, snapshot) db_commit = _git_db_commit(runner, snapshot) @@ -834,7 +839,7 @@ def prepare_policy_run( _validate_advisory_count(advisory_count) check_result = _run( runner, - _advisory_check_argv(cargo_deny, config_path, root), + advisory_check_argv(cargo_deny, config_path, root), cwd=root, ) _assert_scanned_snapshot(check_result.stderr, snapshot) diff --git a/scripts/release_candidate_driver.py b/scripts/release_candidate_driver.py index 9e23afca7..3ce2bb7ac 100644 --- a/scripts/release_candidate_driver.py +++ b/scripts/release_candidate_driver.py @@ -30,7 +30,6 @@ from scripts.check_release_preflight import ( from scripts.check_rust_release_manifest import ( LANES, NATIVE_TOOL_KEYS, - RFC3339_UTC_RE, SOURCE_COMMIT_RE, Failure, LaneEvidence, @@ -49,6 +48,7 @@ from scripts.check_wheel_contents import ( from scripts.record_macos_native_wheel import validate_macos_native_record from scripts.release_advisory_policy import ( PolicyRun, + is_normalized_utc_timestamp, prepare_policy_run, validate_snapshot_identity, ) @@ -63,6 +63,7 @@ from scripts.release_install_smoke import ( CANDIDATE, ENVROOT, PROOF_TARGETS, + RETAINED_PROOF_REPAIR, InstallProofError, candidate_file_entries, target_install_paths_from_ledger, @@ -1271,15 +1272,28 @@ def _validate_proof_binding( repair="bash scripts/release.sh --recover", ) ) - proof_entries = { - ( - str(entry.get("basename")), - int(entry.get("bytes", -1)), - str(entry.get("sha256")), + proof_entries: set[tuple[str, int, str]] = set() + for entry in proof.get("candidate_files", []): + if not isinstance(entry, Mapping): + continue + byte_count = entry.get("bytes", -1) + if type(byte_count) is not int or byte_count < 0: + failures.append( + _failure( + "install proof candidate file byte count is invalid", + expected="non-negative integer", + actual=repr(byte_count), + repair=RETAINED_PROOF_REPAIR, + ) + ) + continue + proof_entries.add( + ( + str(entry.get("basename")), + byte_count, + str(entry.get("sha256")), + ) ) - for entry in proof.get("candidate_files", []) - if isinstance(entry, Mapping) - } try: expected_install_paths = target_install_paths_from_ledger( ledger, @@ -2008,11 +2022,11 @@ def _validate_policy_payload(policy_run: Mapping[str, Any]) -> list[Failure]: "policy_checked_at", ): value = policy_run.get(key) - if not isinstance(value, str) or not RFC3339_UTC_RE.fullmatch(value): + if not is_normalized_utc_timestamp(value): failures.append( _failure( f"retained ledger {key} is invalid", - expected="RFC3339 UTC timestamp", + expected="RFC3339 UTC timestamp normalized with Z", actual=repr(value), repair="bash scripts/release.sh --recover", ) diff --git a/scripts/release_install_smoke.py b/scripts/release_install_smoke.py index 402e74474..1edca16af 100644 --- a/scripts/release_install_smoke.py +++ b/scripts/release_install_smoke.py @@ -55,6 +55,10 @@ TOP_LEVEL_KEYS = frozenset( PROOF_KIND = "solstone-native-install-proof" ENVROOT = "ENVROOT" CANDIDATE = "CANDIDATE" +RETAINED_PROOF_REPAIR = ( + "restore the retained install proof from unmodified release evidence; " + "--recover validates only and cannot repair mutated proof bytes" +) SCRUBBED_COMMAND_ENV: Mapping[str, str] = { "PIP_NO_INDEX": "1", "PYTHONNOUSERSITE": "1", @@ -1113,19 +1117,36 @@ def _validate_command_payload(label: str, value: Any) -> list[Failure]: return failures -def _proof_candidate_entries(proof: Mapping[str, Any]) -> set[tuple[str, int, str]]: +def _proof_candidate_entries( + proof: Mapping[str, Any], +) -> tuple[set[tuple[str, int, str]], list[Failure]]: entries = proof.get("candidate_files", []) if not isinstance(entries, list): - return set() - return { - ( - str(entry.get("basename")), - int(entry.get("bytes", -1)), - str(entry.get("sha256")), + return set(), [] + parsed: set[tuple[str, int, str]] = set() + failures: list[Failure] = [] + for entry in entries: + if not isinstance(entry, Mapping): + continue + byte_count = entry.get("bytes", -1) + if type(byte_count) is not int or byte_count < 0: + failures.append( + _failure( + "install proof candidate file byte count is invalid", + expected="non-negative integer", + actual=repr(byte_count), + repair=RETAINED_PROOF_REPAIR, + ) + ) + continue + parsed.add( + ( + str(entry.get("basename")), + byte_count, + str(entry.get("sha256")), + ) ) - for entry in entries - if isinstance(entry, Mapping) - } + return parsed, failures def _candidate_entries_for_paths(paths: Sequence[Path]) -> set[tuple[str, int, str]]: @@ -1153,7 +1174,9 @@ def _validate_proof_semantics( except InstallProofError as exc: return list(exc.failures) expected_entries = _candidate_entries_for_paths(install_paths) - if _proof_candidate_entries(proof) != expected_entries: + proof_entries, proof_entry_failures = _proof_candidate_entries(proof) + failures.extend(proof_entry_failures) + if proof_entries != expected_entries: failures.append( _failure( "install proof candidate inventory does not match target install set", @@ -1516,7 +1539,7 @@ def validate_install_proof( "install proof candidate file byte count is invalid", expected="non-negative integer", actual=repr(entry["bytes"]), - repair="python3 scripts/check_rust_release_manifest.py", + repair=RETAINED_PROOF_REPAIR, ) ) if not isinstance(entry["sha256"], str) or not SHA256_RE.fullmatch( diff --git a/tests/test_release_candidate_driver.py b/tests/test_release_candidate_driver.py index 9d02be74d..181de8e3f 100644 --- a/tests/test_release_candidate_driver.py +++ b/tests/test_release_candidate_driver.py @@ -660,6 +660,25 @@ def test_recovery_rejects_garbage_retained_advisory_identity( ) +def test_recovery_rejects_impossible_retained_policy_timestamp( + tmp_path: Path, +) -> None: + root = _repo(tmp_path) + report = driver.run_candidate(root, _env(), _services(root)) + ledger_path = report.evidence_dir / "ledger.json" + payload = json.loads(ledger_path.read_text(encoding="utf-8")) + payload["policy_run"]["db_commit_timestamp"] = "2026-99-19T12:00:00Z" + ledger_path.write_bytes(checker.canonical_json_bytes(payload)) + + with pytest.raises(driver.DriverError) as exc: + _recover(root) + + assert any( + failure.error == "retained ledger db_commit_timestamp is invalid" + for failure in exc.value.failures + ) + + def test_recovery_has_no_service_surface() -> None: parameters = set(inspect.signature(driver.run_recover).parameters) diff --git a/tests/test_release_install_smoke.py b/tests/test_release_install_smoke.py index acaee4fd0..4f9cb1523 100644 --- a/tests/test_release_install_smoke.py +++ b/tests/test_release_install_smoke.py @@ -582,6 +582,49 @@ def test_install_proof_validators_require_binding_arguments(tmp_path: Path) -> N smoke.validate_install_proof_bytes(smoke.canonical_json_bytes(proof)) +def test_install_proof_candidate_file_bad_bytes_returns_failure( + tmp_path: Path, +) -> None: + candidate, paths, ledger, install_paths = _linux_context(tmp_path) + proof = smoke.build_install_proof( + target="linux-x86_64-musl", + version="1.0.0", + source_commit=SOURCE_COMMIT, + core_lock_sha256=CORE_LOCK, + candidate_digest=ledger["candidate"]["candidate_digest"], + ledger_sha256=LEDGER_SHA, + candidate_dir=candidate, + candidate_paths=paths, + ledger_payload=ledger, + observation=_observation( + env_root=tmp_path / "env", + candidate_dir=candidate, + install_paths=install_paths, + macos=False, + ), + recorded_at=datetime(2026, 7, 20, 12, tzinfo=UTC), + ) + proof["candidate_files"][0]["bytes"] = "bad" + + failures = smoke.validate_install_proof( + proof, + target="linux-x86_64-musl", + version="1.0.0", + source_commit=SOURCE_COMMIT, + core_lock_sha256=CORE_LOCK, + candidate_digest=ledger["candidate"]["candidate_digest"], + ledger_sha256=LEDGER_SHA, + candidate_dir=candidate, + ledger_payload=ledger, + ) + + assert any( + failure.error == "install proof candidate file byte count is invalid" + and "restore the retained install proof" in failure.repair + for failure in failures + ) + + def _linux_context( tmp_path: Path, ) -> tuple[Path, list[Path], dict[str, Any], tuple[Path, ...]]: -- 2.51.2