replay: fail closed when the pack carries no usable binding

README.md

@@ -121,6 +121,9 @@ Initial CLI version: `v0.1.0`
 
 * `veip-verify replay <path>`
   Canonicalize the Evidence Pack and verify integrity bindings.
+  Fails when the pack carries no usable binding, since a run that checks no
+  binding detects no tampering. Pass `--allow-missing-binding` to accept such a
+  pack anyway; the PASS line then says `NOT VERIFIED` and names why.
 
 * `veip-verify schema`
   Print schema path and schema SHA256 fingerprint.

tests/test_binding_carrier.py

@@ -0,0 +1,87 @@
+"""Regression tests: a rewritten binding carrier must not read as a clean PASS.
+
+The schema types execution.outcome.result_ref as {"type": "string",
+"minLength": 1}, so a pack may carry any string there and still validate.
+Before this test existed, a pack could be tampered with and its carrier
+rewritten to a non-sha256 string, and `veip-verify replay pack.json` printed
+"PASS replay: no binding present (not required)" and exited 0.
+"""
+
+from __future__ import annotations
+
+import json
+import subprocess
+import sys
+from copy import deepcopy
+from pathlib import Path
+
+from veip_verifier_core.replay import verify_integrity_binding
+from veip_verifier_core.schema import validate_evidence_pack
+
+ROOT = Path(__file__).resolve().parents[1]
+FIXTURE = ROOT / "tests" / "fixture_pack.json"
+
+
+def run_cmd(*args: str) -> subprocess.CompletedProcess:
+    return subprocess.run(
+        [sys.executable, "-m", "veip_verifier_core.cli", *args],
+        text=True,
+        capture_output=True,
+        cwd=str(ROOT),
+    )
+
+
+def _tampered_with_rewritten_carrier(tmp_path: Path) -> Path:
+    """A schema-valid pack: one field changed, carrier overwritten with a
+    plausible opaque reference instead of the digest."""
+    pack = json.loads(FIXTURE.read_text(encoding="utf-8"))
+    pack["policy"]["policy_version"] = pack["policy"]["policy_version"] + "-tampered"
+    pack["execution"]["outcome"]["result_ref"] = "ref://opaque-store/result-1"
+
+    validate_evidence_pack(pack)          # still schema-valid — that is the point
+    p = tmp_path / "rewritten_carrier.json"
+    p.write_text(json.dumps(pack), encoding="utf-8")
+    return p
+
+
+def test_rewritten_carrier_fails_by_default(tmp_path: Path):
+    p = run_cmd("replay", str(_tampered_with_rewritten_carrier(tmp_path)))
+    assert p.returncode == 2, p.stdout + p.stderr
+    assert "not a sha256 digest" in p.stderr
+
+
+def test_rewritten_carrier_opt_out_does_not_claim_verification(tmp_path: Path):
+    p = run_cmd(
+        "replay",
+        str(_tampered_with_rewritten_carrier(tmp_path)),
+        "--allow-missing-binding",
+    )
+    assert p.returncode == 0, p.stdout + p.stderr
+    # PASS is allowed here, but it must not read as "this pack is untampered".
+    assert "NOT VERIFIED" in p.stdout
+
+
+def test_malformed_carrier_is_distinguished_from_an_absent_one(tmp_path: Path):
+    pack = json.loads(FIXTURE.read_text(encoding="utf-8"))
+    pack["execution"]["outcome"]["result_ref"] = "ref://opaque-store/result-1"
+    malformed = verify_integrity_binding(pack, require_binding=True)
+
+    absent = deepcopy(pack)
+    del absent["execution"]["outcome"]["result_ref"]
+    missing = verify_integrity_binding(absent, require_binding=True)
+
+    assert malformed.ok is False and missing.ok is False
+    assert malformed.reason != missing.reason
+    assert "not a sha256 digest" in malformed.reason
+    assert "missing binding" in missing.reason
+
+
+def test_intact_fixture_still_passes_under_the_stricter_default():
+    p = run_cmd("replay", str(FIXTURE))
+    assert p.returncode == 0, p.stdout + p.stderr
+    assert "binding matches" in p.stdout
+
+
+def test_require_binding_flag_is_still_accepted():
+    p = run_cmd("replay", str(FIXTURE), "--require-binding")
+    assert p.returncode == 0, p.stdout + p.stderr

veip_verifier_core/cli.py

@@ -133,7 +133,15 @@ def build_parser() -> argparse.ArgumentParser:
     sp_replay.add_argument(
         "--require-binding",
         action="store_true",
-        help="Fail if the binding carrier is missing (execution.outcome.result_ref sha256:<digest>).",
+        help="No-op: this is now the default. Accepted so existing scripts keep working.",
+    )
+    sp_replay.add_argument(
+        "--allow-missing-binding",
+        action="store_true",
+        help=(
+            "Do not fail when the pack carries no usable binding. PASS then means "
+            "'nothing was checked', not 'untampered' — see the printed reason."
+        ),
     )
 
     return p
@@ -149,7 +157,13 @@ def main(argv: Optional[list[str]] = None) -> int:
         return cmd_validate(args.path, quiet=bool(args.quiet))
 
     if args.command == "replay":
-        return cmd_replay(args.path, require_binding=bool(args.require_binding), quiet=bool(args.quiet))
+        # Fail closed: a verifier whose default run can silently skip tamper
+        # detection does not verify. Opt out explicitly with --allow-missing-binding.
+        return cmd_replay(
+            args.path,
+            require_binding=not bool(args.allow_missing_binding),
+            quiet=bool(args.quiet),
+        )
 
     eprint("ERROR: unknown command")
     return EXIT_ERROR

veip_verifier_core/replay.py

@@ -28,10 +28,15 @@ def sha256_hex(data: bytes) -> str:
     return hashlib.sha256(data).hexdigest()
 
 
-def _extract_binding_from_pack(evidence_pack: Dict[str, Any]) -> Optional[str]:
+def _raw_binding_carrier(evidence_pack: Dict[str, Any]) -> Optional[str]:
     """
-    Carrier for the binding digest (schema-allowed):
-      evidence_pack.execution.outcome.result_ref = "sha256:<64-hex>"
+    The carrier field exactly as the pack carries it, whatever its shape.
+    None only when the field is absent or is not a string.
+
+    Needed to tell two different situations apart, because the schema types
+    result_ref as {"type": "string", "minLength": 1} and so admits both:
+      - the carrier is absent          -> nothing claimed to bind
+      - the carrier is present but is not a sha256 digest
     """
     exe = evidence_pack.get("execution")
     if not isinstance(exe, dict):
@@ -40,7 +45,16 @@ def _extract_binding_from_pack(evidence_pack: Dict[str, Any]) -> Optional[str]:
     if not isinstance(out, dict):
         return None
     rr = out.get("result_ref")
-    if not isinstance(rr, str):
+    return rr if isinstance(rr, str) else None
+
+
+def _extract_binding_from_pack(evidence_pack: Dict[str, Any]) -> Optional[str]:
+    """
+    Carrier for the binding digest (schema-allowed):
+      evidence_pack.execution.outcome.result_ref = "sha256:<64-hex>"
+    """
+    rr = _raw_binding_carrier(evidence_pack)
+    if rr is None:
         return None
     if rr.startswith("sha256:") and len(rr) == len("sha256:") + 64:
         return rr.split("sha256:", 1)[1]
@@ -77,18 +91,30 @@ def compute_integrity_binding(evidence_pack: Dict[str, Any]) -> str:
 def verify_integrity_binding(evidence_pack: Dict[str, Any], require_binding: bool = False) -> IntegrityResult:
     """
     Verify that the pack's stored binding matches the computed binding.
-    If require_binding=True and no binding is present, verification FAILS.
+    If require_binding=True and no usable binding is present, verification FAILS.
+
+    Note for callers: with require_binding=False an ok=True result does NOT mean
+    the pack was found untampered — it means no binding was checked. The CLI now
+    passes require_binding=True by default for exactly this reason.
     """
     got = _extract_binding_from_pack(evidence_pack)
     if got is None:
-        if require_binding:
-            return IntegrityResult(
-                False,
-                "missing binding (execution.outcome.result_ref sha256:<digest>)",
-                expected=None,
-                got=None,
+        raw = _raw_binding_carrier(evidence_pack)
+        if raw is None:
+            detail = "missing binding (execution.outcome.result_ref sha256:<digest>)"
+        else:
+            # Say which of the two it is. Reporting a malformed carrier as simply
+            # "no binding present" hides the difference between a pack that never
+            # claimed a binding and a pack whose binding carrier was overwritten.
+            detail = (
+                "binding carrier is not a sha256 digest: "
+                f"execution.outcome.result_ref = {raw[:64]!r}"
             )
-        return IntegrityResult(True, "no binding present (not required)", expected=None, got=None)
+        if require_binding:
+            return IntegrityResult(False, detail, expected=None, got=None)
+        # ok=True here means only "not required", never "verified" — name that in
+        # the reason, because this string is what a reader sees next to PASS.
+        return IntegrityResult(True, f"NOT VERIFIED: {detail}", expected=None, got=None)
 
     expected = compute_integrity_binding(evidence_pack)
     if got == expected: