From bc03f2f7c312397292067141537b89475b1a6b04 Mon Sep 17 00:00:00 2001 From: henleda Date: Mon, 27 Jul 2026 12:37:43 -0500 Subject: [PATCH 1/2] feat(sign): optional detached minisign signature over the evidence bundle (J1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit manifest.json already SHA-256s every member, so tampering with a member is detectable from inside the bundle. What it could not do is bind the bundle to whoever produced it — `generated_by` and `generated_on` are unauthenticated strings anyone can edit. `sign.py` signs the manifest, which covers every member transitively through their digests. Shells out to the `minisign` binary rather than adding a crypto dependency: export.py stays stdlib-only (which J2's acceptance also asks for), and a reviewer verifies with the same one-line command whether or not they have this toolchain. Optional at every level, because an evidence bundle that refuses to be produced for want of a signing key is worse than an unsigned one. No key configured, no binary on PATH, an unreadable key, a signer that exits non-zero, a signer that hangs — each logs one line and exports unsigned. No passphrase handling, by design: the key must be unencrypted (`minisign -G -W`). A signing passphrase is a credential this tool has no business holding, and the constraint is documented rather than discovered at a prompt. It signs the EXACT manifest bytes that ship — signing a different serialization would make every verification fail, or pass against something the reviewer never received. Pinned by a test that captures what the signer was handed and compares it to the member in the zip. The bundle carries its own limits: a caveat, true whether or not a signature is present, says the signature attests who exported the bundle rather than that the log inside is truthful, and that the public key must come from out of band — a key shipped inside a bundle proves nothing about it. 10 new tests, all offline against a stub signer on PATH. 278 pass, coverage 73%. Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 2 + ROADMAP.md | 19 ++++-- docs/AUDIT.md | 3 + docs/USAGE.md | 27 +++++++++ src/vpcopilot/export.py | 37 ++++++++---- src/vpcopilot/sign.py | 79 +++++++++++++++++++++++++ tests/test_sign.py | 126 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 279 insertions(+), 14 deletions(-) create mode 100644 src/vpcopilot/sign.py create mode 100644 tests/test_sign.py diff --git a/.env.example b/.env.example index 40567da..7e5fa4d 100644 --- a/.env.example +++ b/.env.example @@ -25,6 +25,8 @@ GITHUB_TOKEN= # repo-scoped PAT for opening PRs (or use `gh auth login`) # VPCOPILOT_ACTOR=jane.doe # audit-log attribution — default: the OS user; set it in CI # VPCOPILOT_SIM_THRESHOLD=0.01 # would-block rate that flags a policy in `vpcopilot simulate` # VPCOPILOT_SIM_LOGS=traffic.jsonl # traffic sample the refiner also checks each candidate against +# VPCOPILOT_MINISIGN_KEY=~/.minisign/vpcopilot.key # sign exported bundles (unencrypted key: minisign -G -W) +# VPCOPILOT_MINISIGN_BIN= # path to minisign, if not on PATH # --- Validation auth (auth-protected targets: let the probe log in so it can demonstrate the exploit) --- # VPCOPILOT_PROBE_USER= # username the validation probe logs in with (cookie or token app) # VPCOPILOT_PROBE_PASS= # its password diff --git a/ROADMAP.md b/ROADMAP.md index 8d3e787..49c505e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -415,7 +415,10 @@ The export is internally consistent: `manifest.json` SHA-256s every member. Noth `generated_on` (`export.py:165`), but as unauthenticated strings — and nothing outside the machine sees the trail. J1–J4 are the open `BACKLOG.md` evidence entries, scheduled; **J5 is new**. -- [ ] **J1** Sign the evidence bundle. (S, P1) +- [x] **J1** Sign the evidence bundle. (S, P1) — **DONE:** `sign.py` shells out to `minisign` and + drops `manifest.json.minisig` beside the manifest in every bundle (each run's manifest in an + `--all` bundle). No new dependency, so `export.py` stays stdlib-only. Optional at every level: no + key, no binary, an unreadable key or a failing signer each log one line and export unsigned. A detached signature beside the manifest, making a bundle attributable after it leaves the machine. **DECIDED 2026-07-27: minisign.** One small dependency, one keypair, a detached `manifest.json.minisig`, and verification is a single command a reviewer runs without this @@ -423,9 +426,17 @@ sees the trail. J1–J4 are the open `BACKLOG.md` evidence entries, scheduled; * format) is the more defensible answer but is **not S** — raise it as its own item if wanted. The signature attests **who exported this bundle**, not that the audit log it contains is truthful; `docs/USAGE.md` has to say exactly that. - - Acceptance: signing is optional and its absence never fails an export; the signature - covers the manifest digest; `docs/USAGE.md` states plainly what the signature does and - does not attest. + - Acceptance: signing is optional and its absence never fails an export ✅; the signature + covers the manifest digest ✅ (it signs the exact manifest bytes that ship, and the manifest + SHA-256s every member, so coverage is transitive); `docs/USAGE.md` states plainly what the + signature does and does not attest ✅. + - **No passphrase handling, by design.** The key must be unencrypted (`minisign -G -W`). A signing + passphrase is a credential this tool has no business holding; requiring a key managed by + whatever already manages your secrets is the honest trade, and it is documented rather than + discovered at the prompt. + - **The signature is not self-verifying from inside the bundle.** The reviewer needs the public + key out of band — a key shipped alongside the thing it signs proves nothing. Stated in the + manifest's own `caveats`, so it travels with the bundle. - [ ] **J2** `vpcopilot export --verify`. (S, P1) Independent of J1. Re-read a bundle, recompute every member digest against the manifest, check the signature diff --git a/docs/AUDIT.md b/docs/AUDIT.md index 3f1dfa2..ea5cae6 100644 --- a/docs/AUDIT.md +++ b/docs/AUDIT.md @@ -46,6 +46,9 @@ zip can never imply more coverage than it has: ### Honest limits +- A bundle can be **signed** (`VPCOPILOT_MINISIGN_KEY`, see `docs/USAGE.md`), which binds it to + the holder of a key. That is a statement about *who exported it*, not about whether the log is + true — everything below still applies to a signed bundle. - The log is written by the same process that makes the change, to a local file. It is **not tamper-evident** — anyone who can write the out dir can edit `audit.log`. The manifest's SHA-256s prove a bundle was not altered *after export*; they say nothing about the authenticity of the log diff --git a/docs/USAGE.md b/docs/USAGE.md index 9aa53e4..bb0f20a 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -97,6 +97,33 @@ nothing here touches XC or GitHub. Dry runs are not in it: nothing changed, so nothing is logged. The bundle is evidence for a human reviewer, not a compliance certification. Full reference: **[AUDIT.md](AUDIT.md)**. +**Signing a bundle (optional).** Point `VPCOPILOT_MINISIGN_KEY` at an *unencrypted* minisign secret +key and every export gains `manifest.json.minisig` beside the manifest: + +```sh +minisign -G -W -s ~/.minisign/vpcopilot.key # -W = no passphrase; this tool never holds one +export VPCOPILOT_MINISIGN_KEY=~/.minisign/vpcopilot.key +vpcopilot export --out out +``` + +A reviewer verifies with your **public** key: + +```sh +unzip -o audit-bundle.zip manifest.json manifest.json.minisig +minisign -V -p vpcopilot.pub -m manifest.json +``` + +What that signature **does** attest: this manifest was signed by the holder of that key, and — since +the manifest SHA-256s every member — that no file in the bundle changed after it was signed. + +What it **does not** attest: that the audit log inside is truthful. The log is written by the same +process that made the changes, to a local file; a signature proves who exported it, not that what it +says happened. And get the public key **out of band** — a key shipped inside a bundle proves nothing +about that bundle. + +Signing is optional at every level. No key, no `minisign` on PATH, or a signer that fails all mean +an unsigned bundle and a successful export — never a failed one. + Every scan also drops a self-contained `out/report.html` (no server, no external assets). In the console it's on **② Review** and **⚙ Setup** — **Open HTML report ↗** for a new tab, **Download** for a timestamped copy. Both rebuild it from the current run dir on every open, so you always get diff --git a/src/vpcopilot/export.py b/src/vpcopilot/export.py index 9841e5f..bdb8cca 100644 --- a/src/vpcopilot/export.py +++ b/src/vpcopilot/export.py @@ -20,7 +20,14 @@ import zipfile from pathlib import Path +from typing import Callable + from . import __version__, audit, ledger, runmeta +from .sign import sign_bytes + + +def _quiet(_msg: str) -> None: + """Default log sink — the export is a library call first, a command second.""" # What each action DID, for a reviewer scanning a column rather than reading action names. CATEGORY = { @@ -175,6 +182,10 @@ def build_manifest(out_dir: str = "out", *, members: dict | None = None) -> dict "apply; a CLI-driven run has none.", "Entries written by older builds may lack finding_id / namespace / actor — those cells are " "blank rather than inferred.", + "If manifest.json.minisig is present it signs THIS manifest, which SHA-256s every " + "member — so it attests who exported the bundle, not that the audit log inside is " + "truthful. Verify it with the signer's public key obtained OUT OF BAND; a key shipped " + "inside a bundle proves nothing about that bundle.", "simulation.json, when present, describes ONE recorded sample replayed through a spare " "load balancer — not production traffic in general. Its records are redacted at ingest " "(see `redacted`); read the window and record count with the rate.", @@ -183,23 +194,23 @@ def build_manifest(out_dir: str = "out", *, members: dict | None = None) -> dict } -def write_bundle(out_dir: str = "out", output: str | None = None) -> str: +def write_bundle(out_dir: str = "out", output: str | None = None, *, log: Callable = _quiet) -> str: """Zip the run's evidence to `output` (default `/audit-bundle.zip`) and return the path.""" path = Path(output) if output else Path(out_dir) / "audit-bundle.zip" path.parent.mkdir(parents=True, exist_ok=True) - path.write_bytes(bundle_bytes(out_dir)) + path.write_bytes(bundle_bytes(out_dir, log=log)) return str(path) -def bundle_bytes(out_dir: str = "out", *, prefix: str = "") -> bytes: +def bundle_bytes(out_dir: str = "out", *, prefix: str = "", log: Callable = _quiet) -> bytes: """The evidence bundle as bytes, so the console can stream it without touching disk.""" buf = io.BytesIO() with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as z: - _add_run(z, out_dir, prefix) + _add_run(z, out_dir, prefix, log=log) return buf.getvalue() -def _add_run(z: zipfile.ZipFile, out_dir: str, prefix: str = "") -> dict: +def _add_run(z: zipfile.ZipFile, out_dir: str, prefix: str = "", *, log: Callable = _quiet) -> dict: """Write one run into an open archive under `prefix`, returning its manifest (so a multi-run bundle can index what it contains).""" out = Path(out_dir) @@ -223,7 +234,13 @@ def add(name: str, data: bytes) -> None: add(f"{sub}/{p.name}", p.read_bytes()) manifest = build_manifest(out_dir, members=members) - z.writestr(f"{prefix}manifest.json", json.dumps(manifest, indent=2)) + # Sign the EXACT bytes that ship: signing a different serialization would make every + # verification fail, or pass against something the reviewer never received. + blob = json.dumps(manifest, indent=2).encode() + z.writestr(f"{prefix}manifest.json", blob) + sig = sign_bytes(blob, log=log, comment=f"vpcopilot evidence bundle · {out_dir}") + if sig: + z.writestr(f"{prefix}manifest.json.minisig", sig) return manifest @@ -238,7 +255,7 @@ def find_runs(root: str = ".") -> list[str]: return [str(p) for p in cands if (p / "audit.log").exists() or (p / "findings.json").exists()] -def bundle_all_bytes(root: str = ".") -> bytes: +def bundle_all_bytes(root: str = ".", *, log: Callable = _quiet) -> bytes: """Every run on disk in one archive, each under its own folder, with a top-level index. For the case an auditor asks for "everything", not one engagement.""" runs = find_runs(root) @@ -251,7 +268,7 @@ def bundle_all_bytes(root: str = ".") -> bytes: except ValueError: rel = Path(r) folder = str(rel).replace("/", "-").replace("\\", "-").strip("-") or "run" - m = _add_run(z, r, prefix=f"{folder}/") + m = _add_run(z, r, prefix=f"{folder}/", log=log) index.append({"out_dir": str(r), "folder": folder, "events": m["events"], "run_id": (m.get("run") or {}).get("run_id", "")}) z.writestr("index.json", json.dumps({ @@ -261,8 +278,8 @@ def bundle_all_bytes(root: str = ".") -> bytes: return buf.getvalue() -def write_bundle_all(root: str = ".", output: str | None = None) -> str: +def write_bundle_all(root: str = ".", output: str | None = None, *, log: Callable = _quiet) -> str: path = Path(output) if output else Path(root) / "audit-bundle-all.zip" path.parent.mkdir(parents=True, exist_ok=True) - path.write_bytes(bundle_all_bytes(root)) + path.write_bytes(bundle_all_bytes(root, log=log)) return str(path) diff --git a/src/vpcopilot/sign.py b/src/vpcopilot/sign.py new file mode 100644 index 0000000..bc38978 --- /dev/null +++ b/src/vpcopilot/sign.py @@ -0,0 +1,79 @@ +"""J1 — a detached minisign signature over an evidence bundle's manifest. + +`manifest.json` already SHA-256s every member, so tampering with a *member* is detectable from +inside the bundle. What it cannot do is bind the bundle to whoever produced it: `generated_by` and +`generated_on` are unauthenticated strings anyone can edit. A detached signature over the manifest +closes that, and covers every member transitively through their digests. + +**Signing is optional at every level.** No key configured, no `minisign` on PATH, an unreadable key, +a signer that exits non-zero — each means no signature and a successful export. An evidence bundle +that refuses to be produced because a signing key is missing is worse than an unsigned one. + +**No new dependency.** This shells out to the `minisign` binary rather than pulling in a crypto +library, which keeps `export.py` stdlib-only and means a reviewer verifies with the same one-line +command whether or not they have this toolchain installed. + +**No passphrase handling.** The key must be an unencrypted minisign key (`minisign -G -W`). A +signing passphrase is a credential this tool has no business holding; requiring an unencrypted key +managed by whatever already manages your secrets is the honest trade, and it is documented rather +than discovered.""" +from __future__ import annotations + +import os +import shutil +import subprocess +import tempfile +from pathlib import Path +from typing import Callable + +KEY_ENV = "VPCOPILOT_MINISIGN_KEY" +BIN_ENV = "VPCOPILOT_MINISIGN_BIN" + + +def configured() -> bool: + """Whether the operator asked for signing at all — distinct from whether it will succeed.""" + return bool((os.environ.get(KEY_ENV) or "").strip()) + + +def _binary() -> str | None: + explicit = (os.environ.get(BIN_ENV) or "").strip() + if explicit: + return explicit if Path(explicit).is_file() and os.access(explicit, os.X_OK) else None + return shutil.which("minisign") + + +def sign_bytes(data: bytes, *, log: Callable = print, comment: str = "") -> bytes | None: + """Detached signature over `data`, or None when signing is unconfigured or unavailable. + + Every failure path returns None and logs one line. Callers must treat None as "unsigned", + never as an error.""" + if not configured(): + return None + key = os.environ[KEY_ENV].strip() + if not Path(key).is_file(): + log(f" ⚠ signing skipped — no minisign key at {key} (bundle exported unsigned)") + return None + binary = _binary() + if not binary: + log(" ⚠ signing skipped — `minisign` not found on PATH " + f"(set {BIN_ENV}, or install it; bundle exported unsigned)") + return None + + with tempfile.TemporaryDirectory() as td: + src, sig = Path(td) / "manifest.json", Path(td) / "manifest.json.minisig" + src.write_bytes(data) + cmd = [binary, "-S", "-s", key, "-m", str(src), "-x", str(sig)] + if comment: + cmd += ["-t", comment] + try: + r = subprocess.run(cmd, capture_output=True, text=True, timeout=30) + except Exception as e: # noqa: BLE001 — a broken signer must never fail an export + log(f" ⚠ signing skipped — could not run minisign: {e} (bundle exported unsigned)") + return None + if r.returncode != 0 or not sig.exists(): + detail = (r.stderr or r.stdout or "").strip().splitlines() + why = detail[-1] if detail else f"exit {r.returncode}" + log(f" ⚠ signing skipped — minisign failed: {why} (bundle exported unsigned)") + return None + log(" signed manifest.json (detached minisign signature)") + return sig.read_bytes() diff --git a/tests/test_sign.py b/tests/test_sign.py new file mode 100644 index 0000000..d32e6cc --- /dev/null +++ b/tests/test_sign.py @@ -0,0 +1,126 @@ +"""J1 — a detached minisign signature over the bundle manifest. + +Signing is OPTIONAL at every level: no key configured, no binary on PATH, or a signer that fails +all mean no signature and a successful export. An evidence bundle that refuses to be produced +because a signing key is missing is worse than an unsigned one.""" +import io +import os +import stat +import zipfile + +from vpcopilot import audit, export, sign + + +def _run(tmp_path): + out = tmp_path / "out-run" + audit.record(str(out), "apply_waf", finding_id="f-1", lb="lab", namespace="ns") + return out + + +def _stub_signer(tmp_path, *, body="untrusted comment: x\nRWSIG\ntrusted comment: y\nSIGBYTES\n", code=0): + """A fake `minisign` on PATH: writes a signature file at -x and exits `code`.""" + p = tmp_path / "bin" / "minisign" + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text( + "#!/bin/sh\n" + "while [ $# -gt 0 ]; do case $1 in -x) shift; OUT=$1;; -m) shift; IN=$1;; esac; shift; done\n" + f"printf %s '{body}' > \"$OUT\"\n" + f"exit {code}\n") + p.chmod(p.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + return str(p.parent) + + +# ---- optional at every level ---- +def test_no_key_configured_means_no_signature_and_a_clean_export(tmp_path, monkeypatch): + monkeypatch.delenv("VPCOPILOT_MINISIGN_KEY", raising=False) + z = zipfile.ZipFile(io.BytesIO(export.bundle_bytes(str(_run(tmp_path))))) + assert "manifest.json" in z.namelist() + assert not [n for n in z.namelist() if n.endswith(".minisig")] + + +def test_a_configured_key_with_no_binary_warns_but_still_exports(tmp_path, monkeypatch): + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("VPCOPILOT_MINISIGN_BIN", str(tmp_path / "definitely-not-here")) + logs = [] + data = export.bundle_bytes(str(_run(tmp_path)), log=logs.append) + z = zipfile.ZipFile(io.BytesIO(data)) + assert "manifest.json" in z.namelist() + assert not [n for n in z.namelist() if n.endswith(".minisig")] + assert any("sign" in m.lower() for m in logs) # said so, rather than failing silently + + +def test_a_missing_key_file_does_not_fail_the_export(tmp_path, monkeypatch): + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "nope.key")) + monkeypatch.setenv("PATH", _stub_signer(tmp_path) + os.pathsep + os.environ["PATH"]) + z = zipfile.ZipFile(io.BytesIO(export.bundle_bytes(str(_run(tmp_path)), log=lambda m: None))) + assert not [n for n in z.namelist() if n.endswith(".minisig")] + + +def test_a_signer_that_fails_does_not_fail_the_export(tmp_path, monkeypatch): + (tmp_path / "key").write_text("untrusted comment: minisign secret key\nRWRTY\n") + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("PATH", _stub_signer(tmp_path, code=3) + os.pathsep + os.environ["PATH"]) + logs = [] + z = zipfile.ZipFile(io.BytesIO(export.bundle_bytes(str(_run(tmp_path)), log=logs.append))) + assert "manifest.json" in z.namelist() + assert not [n for n in z.namelist() if n.endswith(".minisig")] + assert any("sign" in m.lower() for m in logs) + + +# ---- the happy path ---- +def test_the_signature_ships_beside_the_manifest(tmp_path, monkeypatch): + (tmp_path / "key").write_text("untrusted comment: minisign secret key\nRWRTY\n") + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("PATH", _stub_signer(tmp_path) + os.pathsep + os.environ["PATH"]) + z = zipfile.ZipFile(io.BytesIO(export.bundle_bytes(str(_run(tmp_path)), log=lambda m: None))) + assert "manifest.json.minisig" in z.namelist() + assert b"RWSIG" in z.read("manifest.json.minisig") + + +def test_the_signature_covers_exactly_the_manifest_that_shipped(tmp_path, monkeypatch): + """Signing a different byte string than the one in the bundle would make every verification + fail — or, worse, pass against something the reviewer never received.""" + (tmp_path / "key").write_text("k") + signed = tmp_path / "signed-input" + p = tmp_path / "bin" / "minisign" + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("#!/bin/sh\n" + "while [ $# -gt 0 ]; do case $1 in -x) shift; OUT=$1;; -m) shift; IN=$1;; esac; shift; done\n" + f"cp \"$IN\" '{signed}'\n" + "printf 'RWSIG\\n' > \"$OUT\"\n") + p.chmod(0o755) + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("PATH", str(p.parent) + os.pathsep + os.environ["PATH"]) + z = zipfile.ZipFile(io.BytesIO(export.bundle_bytes(str(_run(tmp_path)), log=lambda m: None))) + assert signed.read_bytes() == z.read("manifest.json") + + +def test_every_run_in_a_multi_run_bundle_is_signed(tmp_path, monkeypatch): + for n in ("out-a", "out-b"): + audit.record(str(tmp_path / n), "retire", finding_id="f-1", lb="lab") + (tmp_path / "key").write_text("k") + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("PATH", _stub_signer(tmp_path) + os.pathsep + os.environ["PATH"]) + z = zipfile.ZipFile(io.BytesIO(export.bundle_all_bytes(str(tmp_path), log=lambda m: None))) + sigs = sorted(n for n in z.namelist() if n.endswith(".minisig")) + assert sigs == ["out-a/manifest.json.minisig", "out-b/manifest.json.minisig"] + + +# ---- what the bundle says about it ---- +def test_the_manifest_states_what_a_signature_does_and_does_not_attest(tmp_path): + caveats = " ".join(export.build_manifest(str(_run(tmp_path)))["caveats"]).lower() + assert "minisig" in caveats + assert "out of band" in caveats # a key shipped inside proves nothing + assert "who exported" in caveats or "exported" in caveats + + +def test_sign_manifest_returns_none_when_unconfigured(tmp_path, monkeypatch): + monkeypatch.delenv("VPCOPILOT_MINISIGN_KEY", raising=False) + assert sign.sign_bytes(b"x", log=lambda m: None) is None + + +def test_sign_is_reported_as_configured_or_not(tmp_path, monkeypatch): + monkeypatch.delenv("VPCOPILOT_MINISIGN_KEY", raising=False) + assert sign.configured() is False + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "k")) + assert sign.configured() is True From f4401f870cc0e9d337a31a9628d93cfeed9fc231 Mon Sep 17 00:00:00 2001 From: henleda Date: Mon, 27 Jul 2026 12:42:57 -0500 Subject: [PATCH 2/2] fix(sign): carry the run id in the signed comment, not the exporter's path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-trip against the real minisign 0.12, which the stub-based tests could not do: keygen -G -W -> export -> minisign -V "Signature and comment signature verified" tampered manifest "Signature verification failed" tampered member (audit.log) caught by the manifest's own SHA-256, signature intact — the chain is what makes that digest trustworthy wrong public key fails on key-id mismatch The CLI flag contract I had assumed (-S -s -m -x -t ) matches the real binary exactly, so the shell-out needed no change. What the real run did surface: the trusted comment — which is SIGNED and travels with the bundle — embedded the exporter's absolute filesystem path. Unnecessary disclosure for an artifact whose whole purpose is to leave the machine. It now carries the tool version and run_id, the join key that already identifies the run, with a test pinning that no local path appears in it. 279 pass. Co-Authored-By: Claude Opus 5 (1M context) --- ROADMAP.md | 7 +++++++ src/vpcopilot/export.py | 6 +++++- tests/test_sign.py | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 49c505e..56c9f44 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -434,6 +434,13 @@ sees the trail. J1–J4 are the open `BACKLOG.md` evidence entries, scheduled; * passphrase is a credential this tool has no business holding; requiring a key managed by whatever already manages your secrets is the honest trade, and it is documented rather than discovered at the prompt. + - **Round-trip verified against minisign 0.12**, not just a stub: keygen → export → verify + ("Signature and comment signature verified"); a tampered manifest fails; a tampered *member* + is caught by the manifest's own SHA-256 while the signature stays valid — the chain is what + makes that digest trustworthy; a wrong public key fails on key-id mismatch. + - **The real run found a leak a stub could not.** The signed trusted comment embedded the + exporter's absolute filesystem path. It now carries the tool version and `run_id` — the join + key that already identifies the run — and a test pins that no local path appears in it. - **The signature is not self-verifying from inside the bundle.** The reviewer needs the public key out of band — a key shipped alongside the thing it signs proves nothing. Stated in the manifest's own `caveats`, so it travels with the bundle. diff --git a/src/vpcopilot/export.py b/src/vpcopilot/export.py index bdb8cca..8eaf38e 100644 --- a/src/vpcopilot/export.py +++ b/src/vpcopilot/export.py @@ -238,7 +238,11 @@ def add(name: str, data: bytes) -> None: # verification fail, or pass against something the reviewer never received. blob = json.dumps(manifest, indent=2).encode() z.writestr(f"{prefix}manifest.json", blob) - sig = sign_bytes(blob, log=log, comment=f"vpcopilot evidence bundle · {out_dir}") + # The trusted comment is SIGNED and travels with the bundle, so it carries the run identity — + # not the exporter's filesystem path, which a real round-trip showed it was leaking. + run_id = (manifest.get("run") or {}).get("run_id") or "unknown-run" + sig = sign_bytes(blob, log=log, + comment=f"vpcopilot {__version__} evidence bundle · run {run_id}") if sig: z.writestr(f"{prefix}manifest.json.minisig", sig) return manifest diff --git a/tests/test_sign.py b/tests/test_sign.py index d32e6cc..f1c3f0c 100644 --- a/tests/test_sign.py +++ b/tests/test_sign.py @@ -124,3 +124,25 @@ def test_sign_is_reported_as_configured_or_not(tmp_path, monkeypatch): assert sign.configured() is False monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "k")) assert sign.configured() is True + + +def test_the_signed_comment_carries_the_run_id_not_a_local_path(tmp_path, monkeypatch): + """The trusted comment is signed and travels with the bundle. A real round-trip showed it was + embedding the exporter's absolute filesystem path — unnecessary disclosure for an artifact + whose whole purpose is to leave the machine.""" + captured = tmp_path / "args" + p = tmp_path / "bin" / "minisign" + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("#!/bin/sh\n" + "while [ $# -gt 0 ]; do case $1 in -x) shift; OUT=$1;; -t) shift; T=$1;; esac; shift; done\n" + f"printf %s \"$T\" > '{captured}'\n" + "printf 'RWSIG\\n' > \"$OUT\"\n") + p.chmod(0o755) + (tmp_path / "key").write_text("k") + monkeypatch.setenv("VPCOPILOT_MINISIGN_KEY", str(tmp_path / "key")) + monkeypatch.setenv("PATH", str(p.parent) + os.pathsep + os.environ["PATH"]) + out = tmp_path / "out-run" + audit.record(str(out), "retire", finding_id="f-1", lb="lab") + export.bundle_bytes(str(out), log=lambda m: None) + comment = captured.read_text() + assert "run " in comment and str(tmp_path) not in comment