fix(bundle): verify per-member sha256 against manifest on import

[galuis116]

Problem

bundle.import_check and bundle.import_apply never verify each
tar member's content against the per-file sha256 recorded in
manifest.json. The integrity guarantee that the README markets
(tar.gz + manifest.json with per-file sha256) is therefore only
enforced by export_check; the import side trusts whatever is in
the tarball as long as the path appears in the manifest and the
schema parses.

An attacker (or accidental disk corruption) who substitutes a tar
member's bytes without re-signing manifest.json can land
arbitrary content into a .vouch/ KB:

• The audit log records a successful bundle.import event with the
  legitimate bundle_id — no trace that the bytes ever differed
  from the manifest.
• For YAML/markdown artifacts the attacker only has to ship a
  payload that passes pydantic validation. Claim text, confidence,
  status, citations, page bodies, entity attributes, relation
  targets — all attacker-controlled in a single shipped bundle.
• For sources/<sha>/content files _validate_content skips
  validation entirely, so source bytes can be replaced with
  anything. Source.sha256 in meta.yaml remains the legitimate
  hash, so the resulting KB is internally inconsistent
  (vouch source verify would now fail), but the import itself
  succeeds.

This defeats the documented integrity story of vouch's portable
bundle format on every platform. The asymmetry:

• src/vouch/bundle.py:139 — export_check reads each member,
  hashes it, and reports hash mismatch: <name> on disagreement.
• src/vouch/bundle.py:231 — import_check reads each member only
  to feed _validate_content; never hashes, never compares to
  manifest.
• src/vouch/bundle.py:279 — import_apply reads each member and
  writes it after schema validation; the manifest sha256 is loaded
  into recorded[...]["sha256"] but only consulted to decide
  whether the destination file conflicts under
  on_conflict="skip". It is never compared to the bundle
  member.

Fix

Mirror the existing export_check hash logic into both import_check
and import_apply:

• import_check — for every tar member that appears in the manifest,
  compare sha256_hex(member_bytes) to manifest_entry["sha256"]
  and append hash mismatch: <name> on disagreement. This makes the
  ok field honest and is what callers of the diff API rely on.
• import_apply — re-verify the same hash immediately before
  dest.write_bytes(body) and skip on disagreement. This is defence
  in depth against a TOCTOU between import_check and the re-open
  inside import_apply. Because import_apply already calls
  import_check and raises on any issue, the practical effect is
  that tampered bundles raise RuntimeError("refusing to import: hash mismatch: ...") before writing.

No on-disk-layout, schema, or bundle-format change. Honest bundles
produced by vouch export remain byte-for-byte accepted; the
existing happy-path tests still pass on Linux/macOS.

Tests

Added three regression tests in tests/test_bundle.py:

• test_import_rejects_member_with_mismatched_sha256 — manifest
  declares one hash, tar member contains different bytes;
  import_check.ok is False, import_apply raises
  RuntimeError("hash mismatch: claims/c1.yaml"), and no file is
  written to the destination KB.
• test_import_rejects_source_content_mismatch — same attack
  against sources/<sha>/content, which _validate_content
  explicitly skips, so the sha256 check is the only protection.
• test_import_check_passes_when_member_matches_manifest — sanity:
  matching bytes do not surface as hash mismatch.

All three pass on Windows (Python 3.12.13). The full suite shows
the same pre-existing failures on Windows that exist without this
change (#72 path-separator bug on bundle export; O_NOFOLLOW use
in storage.py) — both untouched by this PR.

End-to-end demonstration with a malicious bundle constructed
independently of bundle.export:

$ python tamper_repro.py
declared sha256 (in manifest): a3ba9e30f4b31fd07ebd6ea8f9ab0797...
actual sha256   (in tar member): c3c0602bb429437edc56c43a3c54eacf...

export_check: ok=False
  - hash mismatch: claims/c1.yaml

import_check: ok=False
  - hash mismatch: claims/c1.yaml

import_apply refused: refusing to import: hash mismatch: claims/c1.yaml
FIX OK: tampered bundle rejected before any write.

Fixes #74

Summary by CodeRabbit

• Bug Fixes / Security
  • Bundle import now verifies each member’s sha256 against the manifest and re-checks at write time; tampered or mismatched members are rejected and not written, and the import is refused.
• Tests
  • Added comprehensive tests covering hash-mismatch, missing-hash, tampered content, and write-time verification scenarios.
• Documentation
  • Changelog updated to document the import-time integrity verification and rejection behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c54eb570-e625-4cd6-a8ff-e43d3850ed15

📥 Commits

Reviewing files that changed from the base of the PR and between ab5e942 and 160c430.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/vouch/bundle.py
• tests/test_bundle.py

✅ Files skipped from review due to trivial changes (1)

• CHANGELOG.md

---

📝 Walkthrough

Walkthrough

This PR adds per-member sha256 verification to bundle import operations. import_check now verifies each tar member's hash against the manifest before schema validation and reports mismatches as issues. import_apply re-verifies hashes at write time before persisting files to disk, preventing tampering attacks that could bypass the integrity check. Comprehensive test coverage validates hash mismatch detection and rejection behavior.

Changes

Bundle import integrity verification

Layer / File(s)	Summary
Manifest path derivation from recorded mapping src/vouch/bundle.py	manifest_paths is now derived from the pre-built recorded manifest mapping instead of directly from the manifest file structure, ensuring consistency.
Import check phase sha256 verification src/vouch/bundle.py	Each tar member is sha256-verified against the manifest's declared hash before schema validation; mismatches are recorded as issues and the member is skipped from further processing.
Import apply phase write-time sha256 verification src/vouch/bundle.py	Each extracted tar member's sha256 is re-verified immediately before writing to disk; a write-time mismatch raises RuntimeError and the member is not written.
Hash mismatch detection and rejection tests tests/test_bundle.py	Adds _write_hash_mismatched_bundle and tests asserting that import_check reports hash mismatches, import_apply raises on write-time mismatch, mismatched members are not created, missing manifest sha256 is treated as mismatch, and matching members pass.
CHANGELOG entry CHANGELOG.md	Documents the fix: import now validates per-member sha256 against manifest.json and re-checks at write time to reject tampered bundles before disk writes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• vouchdev/vouch#34: Both PRs modify src/vouch/bundle.py's import handling—PR #34 adds schema/content validation per member; this PR adds per-member sha256 re-verification that gates validation and write.

Poem

🐰 I hopped through tar and manifest bright,
Counting sha256s through the night,
Now mismatched bytes get bounced away,
So honest bundles safely stay. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding per-member sha256 verification during bundle import, which directly addresses the integrity gap described in the changeset.
Linked Issues check	✅ Passed	The implementation fully meets all coding requirements from issue #74: import_check now verifies each member's sha256 against the manifest and reports mismatches, import_apply re-verifies before writing and raises on failure, and tests comprehensively validate all hash-mismatch scenarios.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the integrity verification gap in bundle import. The PR adds hash verification logic, comprehensive tests, and documentation updates; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/vouch/bundle.py`:
- Around line 326-331: The code currently rechecks the manifest hash with
sha256_hex(body) vs recorded[member.name]["sha256"] and merely appends to
skipped and continues; change this to raise a RuntimeError immediately
(including context: member.name and both expected and actual hashes) to
hard-fail on write-time hash mismatch so import_apply cannot silently succeed
after a TOCTOU; update any surrounding logic in the import_apply/import_check
flow to not rely on skipped for this condition.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 913b5372-38ee-4d1b-a46c-23767f08e2fc

📥 Commits

Reviewing files that changed from the base of the PR and between c3accb6 and 53f9e8d.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/vouch/bundle.py
• tests/test_bundle.py

[plind-junior]

Nice fix — this is exactly the gap #74 was about, and you closed it the cleanest way possible by reusing the same hashing logic export_check already had and just mirroring it into the import path. The part I liked most is that your tests build the tampered bundle by hand instead of round-tripping through export, because that's what actually proves the check catches a real attack rather than a friendly bundle.

The core of it looks solid. Hashing each member in import_check at src/vouch/bundle.py:272 and surfacing a hash mismatch issue makes the ok flag honest again, and import_apply running the check first and bailing at src/vouch/bundle.py:298-300 means a tampered bundle is rejected before anything touches disk — your test asserting claims/c1.yaml never gets written confirms that. I especially liked the sources/*/content test, since that's the one path _validate_content skips, so the sha256 check is the only thing guarding it. And since all three tests fail on the parent commit, they're genuine regression guards rather than just green checkmarks.

The one thing I'd really like your take on is the write-time re-verify at src/vouch/bundle.py:329-331. The only way to reach that branch is if the bytes change between import_check and the re-open here — which basically means someone is actively tampering mid-import — but right now we just quietly add the member to skipped and still log a successful bundle.import event with the legitimate bundle_id. That's the exact "the audit log says everything's fine when it isn't" situation #74 was unhappy about, so I think this case should raise the way import_check does instead of skipping and reporting success. And even if you keep skipping it, folding it into skipped_conflicts at src/vouch/bundle.py:342 is a little misleading, because that bucket reads as "the destination already had different content," not "the bundle was tampered with."

Two smaller things while you're in there. At src/vouch/bundle.py:272 (and the same pattern at :320 and :329), recorded[member.name]["sha256"] will throw a raw KeyError if a manifest entry is missing its sha256, so a hand-crafted manifest could give you an ugly traceback instead of a clean rejection — .get("sha256") with missing-treated-as-mismatch would close that. And slightly out of scope, but a file that's listed in the manifest yet missing from the tarball just silently doesn't get imported with no issue raised (src/vouch/bundle.py:250-261); same integrity surface, probably worth a follow-up issue rather than something to tackle in this PR.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/vouch/bundle.py (1)

251-278: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard manifest sha256 access in import_check/import_apply

src/vouch/bundle.py indexes f["sha256"] / recorded[member.name]["sha256"] directly in import_check and import_apply (lines ~251-278, ~312-333). If a bundle’s manifest.json omits sha256, the import raises KeyError instead of cleanly reporting hash mismatch / skipping the member. Use f.get("sha256") (and similarly for recorded[...]) and treat missing/invalid hashes as integrity failures.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vouch/bundle.py` around lines 251 - 278, The import_check/import_apply
flow currently indexes f["sha256"] and recorded[member.name]["sha256"] directly
which raises KeyError if a manifest entry lacks a sha256; update the logic in
import_check/import_apply to use f.get("sha256") and
recorded[member.name].get("sha256") (or otherwise fetch via .get) and treat
missing or non-string/invalid hashes as integrity failures by appending an
issues entry like "hash mismatch: <path>" (or similar) and skipping the file;
ensure sha256_hex is only called when a valid hash string is present and
continue to call _validate_content for files that pass the hash check.

🧹 Nitpick comments (1)

tests/test_bundle.py (1)

158-189: ⚡ Quick win

Add malformed-manifest coverage for missing sha256 entries.

Current regression coverage is strong for mismatched bytes, but it doesn’t lock in behavior when a manifest file entry omits sha256 entirely. Add one test to ensure import rejects cleanly (instead of surfacing a raw KeyError) for that malformed-manifest case.

🧪 Suggested additional regression test

+def test_import_rejects_member_missing_sha256(store: KBStore, tmp_path: Path) -> None:
+    payload = b"text: original\n"
+    bundle_path = tmp_path / "missing-sha256.tar.gz"
+    manifest = {
+        "spec": bundle.SPEC_VERSION,
+        "bundle_id": "deadbeef",
+        "files": [{"path": "claims/c1.yaml", "size": len(payload)}],  # no sha256
+        "counts": {},
+        "safety": {"has_proposed": False, "has_state_db": False, "has_audit_log": False},
+    }
+    with tarfile.open(bundle_path, "w:gz") as tar:
+        info = tarfile.TarInfo("claims/c1.yaml")
+        info.size = len(payload)
+        tar.addfile(info, io.BytesIO(payload))
+        mf_bytes = json.dumps(manifest).encode()
+        mf_info = tarfile.TarInfo(bundle.MANIFEST_NAME)
+        mf_info.size = len(mf_bytes)
+        tar.addfile(mf_info, io.BytesIO(mf_bytes))
+
+    diff = bundle.import_check(store.kb_dir, bundle_path)
+    assert not diff.ok
+    assert any("hash mismatch" in i for i in diff.issues)
+    with pytest.raises(RuntimeError, match="hash mismatch"):
+        bundle.import_apply(store.kb_dir, bundle_path)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_bundle.py` around lines 158 - 189, Add a new test that builds a
tar.gz using the same pattern as _write_hash_mismatched_bundle but whose
manifest file entry for the member omits the "sha256" key; write the member
payload and a manifest (bundle.MANIFEST_NAME) whose file entry includes "path"
and "size" but no "sha256", then call the import routine (e.g.,
bundle.import_bundle or the same import helper used by other tests) and assert
it raises a clean validation error (e.g., a ValueError or the library's
bundle-validation exception) rather than surfacing a raw KeyError; implement the
helper alongside _write_hash_mismatched_bundle (or inline in the new test) and
add a test function that uses it and verifies the import fails with the expected
validation exception.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/vouch/bundle.py`:
- Around line 251-278: The import_check/import_apply flow currently indexes
f["sha256"] and recorded[member.name]["sha256"] directly which raises KeyError
if a manifest entry lacks a sha256; update the logic in
import_check/import_apply to use f.get("sha256") and
recorded[member.name].get("sha256") (or otherwise fetch via .get) and treat
missing or non-string/invalid hashes as integrity failures by appending an
issues entry like "hash mismatch: <path>" (or similar) and skipping the file;
ensure sha256_hex is only called when a valid hash string is present and
continue to call _validate_content for files that pass the hash check.

---

Nitpick comments:
In `@tests/test_bundle.py`:
- Around line 158-189: Add a new test that builds a tar.gz using the same
pattern as _write_hash_mismatched_bundle but whose manifest file entry for the
member omits the "sha256" key; write the member payload and a manifest
(bundle.MANIFEST_NAME) whose file entry includes "path" and "size" but no
"sha256", then call the import routine (e.g., bundle.import_bundle or the same
import helper used by other tests) and assert it raises a clean validation error
(e.g., a ValueError or the library's bundle-validation exception) rather than
surfacing a raw KeyError; implement the helper alongside
_write_hash_mismatched_bundle (or inline in the new test) and add a test
function that uses it and verifies the import fails with the expected validation
exception.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 33c98b9f-0245-4f24-bb16-54e98cf941a5

📥 Commits

Reviewing files that changed from the base of the PR and between 53f9e8d and ab5e942.

📒 Files selected for processing (2)

• src/vouch/bundle.py
• tests/test_bundle.py

[galuis116]

Thanks for the careful review — agreed on all three points.

Write-time mismatch now raises. import_apply at the
re-verify branch (formerly bundle.py:329-331) now raises
RuntimeError("refusing to import: hash mismatch at write time: <name>")
instead of appending to skipped and emitting a clean
bundle.import audit event. You're right that that was the exact
audit-truthfulness anti-pattern #74 was filed to fix — a tampered
member silently dropped while the audit log claims success is the
worst possible outcome here. Also avoids the misleading
skipped_conflicts bucketing you flagged.

Regression test test_import_apply_raises_on_write_time_hash_mismatch
in tests/test_bundle.py monkeypatches import_check to return a
clean result, then asserts both that import_apply raises and that
audit.log.jsonl contains no bundle.import event for the
attempted import.

.get("sha256") everywhere. All three sites in bundle.py
(the dest-diff comparator, the import_check member hash check,
and the import_apply write-time re-verify) now use
recorded[name].get("sha256"). A hand-crafted manifest missing
the field is reported as a clean hash mismatch (or surfaces as
refusing to import: hash mismatch at apply time) instead of a
bare KeyError. Regression test
test_import_treats_missing_manifest_sha256_as_mismatch covers it.

Missing-from-tarball gap → follow-up #80. Out of scope here
per your suggestion, but I wrote it up as a separate issue with the
suggested fix sketched out so it's easy to pick up:
#80.

make check locally: ruff clean, all bundle tests pass on Linux
semantics (1 pre-existing Windows-only CRLF failure in
test_export_import_round_trip is independent of this PR — same
on main).

[plind-junior]

LGTM! Thanks