diff --git a/src/vouch/bundle.py b/src/vouch/bundle.py index a0595464..fa3be0a4 100644 --- a/src/vouch/bundle.py +++ b/src/vouch/bundle.py @@ -35,8 +35,14 @@ SPEC_VERSION = "vouch-bundle-0.1" EXPORT_SUBDIRS = ( - "claims", "pages", "sources", "entities", "relations", - "evidence", "sessions", "decided", + "claims", + "pages", + "sources", + "entities", + "relations", + "evidence", + "sessions", + "decided", ) VALIDATORS: dict[str, Any] = { @@ -72,14 +78,16 @@ def build_manifest(kb_dir: Path) -> dict[str, Any]: files: list[dict[str, Any]] = [] for rel, abs_path in _iter_export_files(kb_dir): data = abs_path.read_bytes() - files.append({ - # tarfile member names use POSIX `/` on every platform; the - # manifest path must match so set lookups and the per-subdir - # counter below work on Windows too. - "path": rel.as_posix(), - "size": len(data), - "sha256": sha256_hex(data), - }) + files.append( + { + # tarfile member names use POSIX `/` on every platform; the + # manifest path must match so set lookups and the per-subdir + # counter below work on Windows too. + "path": rel.as_posix(), + "size": len(data), + "sha256": sha256_hex(data), + } + ) # Bundle id is the sha256 of the sorted per-file hashes — same inputs # always produce the same id, so duplicate exports are recognisable. h = hashlib.sha256() @@ -90,8 +98,7 @@ def build_manifest(kb_dir: Path) -> dict[str, Any]: "bundle_id": h.hexdigest(), "files": files, "counts": { - sub: sum(1 for f in files if f["path"].startswith(f"{sub}/")) - for sub in EXPORT_SUBDIRS + sub: sum(1 for f in files if f["path"].startswith(f"{sub}/")) for sub in EXPORT_SUBDIRS }, "safety": { "has_proposed": False, @@ -112,7 +119,9 @@ def export(kb_dir: Path, *, dest: Path, actor: str = "vouch-export") -> dict[str info.size = len(manifest_bytes) tar.addfile(info, io.BytesIO(manifest_bytes)) audit.log_event( - kb_dir, event="bundle.export", actor=actor, + kb_dir, + event="bundle.export", + actor=actor, object_ids=[manifest["bundle_id"]], data={"dest": str(dest), "files": len(manifest["files"])}, ) @@ -180,8 +189,10 @@ def export_check(bundle_path: Path) -> ExportCheckResult: except KeyError: issues.append(f"manifest lists missing file: {path}") return ExportCheckResult( - ok=not issues, bundle_id=bundle_id, - files_checked=files_checked, issues=issues, + ok=not issues, + bundle_id=bundle_id, + files_checked=files_checked, + issues=issues, ) @@ -243,9 +254,7 @@ def import_check(kb_dir: Path, bundle_path: Path) -> ImportCheckResult: try: mf_member = tar.getmember(MANIFEST_NAME) except KeyError: - return ImportCheckResult( - False, "", [], [], [], ["bundle missing manifest.json"] - ) + return ImportCheckResult(False, "", [], [], [], ["bundle missing manifest.json"]) manifest = json.loads(tar.extractfile(mf_member).read().decode()) # type: ignore[union-attr] bundle_id = manifest.get("bundle_id", "") recorded = {f["path"]: f for f in manifest["files"]} @@ -278,11 +287,19 @@ def import_check(kb_dir: Path, bundle_path: Path) -> ImportCheckResult: issues.append(f"hash mismatch: {member.name}") continue _validate_content(member.name, body, issues) + for path in manifest_paths: + try: + tar.getmember(path) + except KeyError: + issues.append(f"manifest lists missing file: {path}") return ImportCheckResult( - ok=not issues, bundle_id=bundle_id, - new_files=new_files, conflicts=conflicts, - identical=identical, issues=issues, + ok=not issues, + bundle_id=bundle_id, + new_files=new_files, + conflicts=conflicts, + identical=identical, + issues=issues, ) @@ -337,8 +354,7 @@ def import_apply( # the audit-truthfulness anti-pattern #74 was about. if sha256_hex(body) != expected_sha: raise RuntimeError( - f"refusing to import: hash mismatch at write time: " - f"{member.name}" + f"refusing to import: hash mismatch at write time: {member.name}" ) val_issues: list[str] = [] _validate_content(member.name, body, val_issues) @@ -355,7 +371,9 @@ def import_apply( "on_conflict": on_conflict, } audit.log_event( - kb_dir, event="bundle.import", actor=actor, + kb_dir, + event="bundle.import", + actor=actor, object_ids=[check.bundle_id], data={ "written": len(written), diff --git a/tests/test_bundle.py b/tests/test_bundle.py index a851204c..dbbbdefe 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -144,15 +144,67 @@ def test_import_apply_rejects_absolute_path(store: KBStore, tmp_path: Path) -> N assert not target.exists() +def test_import_check_rejects_manifest_listing_missing_file(store: KBStore, tmp_path: Path) -> None: + """Manifest entries without a matching tar member must be flagged.""" + bundle_path = tmp_path / "missing.tar.gz" + manifest = { + "spec": bundle.SPEC_VERSION, + "bundle_id": "deadbeef", + "files": [ + { + "path": "claims/c1.yaml", + "size": 16, + "sha256": hashlib.sha256(b"text: any\n").hexdigest(), + }, + ], + "counts": {}, + "safety": {"has_proposed": False, "has_state_db": False, "has_audit_log": False}, + } + with tarfile.open(bundle_path, "w:gz") as tar: + 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)) + + result = bundle.import_check(store.kb_dir, bundle_path) + assert not result.ok + assert any("missing file" in i for i in result.issues) + + +def test_import_apply_rejects_bundle_with_missing_manifest_file( + store: KBStore, tmp_path: Path +) -> None: + """import_apply must refuse a bundle whose manifest lists a file absent from the tarball.""" + bundle_path = tmp_path / "missing.tar.gz" + manifest = { + "spec": bundle.SPEC_VERSION, + "bundle_id": "deadbeef", + "files": [ + { + "path": "claims/c1.yaml", + "size": 16, + "sha256": hashlib.sha256(b"text: any\n").hexdigest(), + }, + ], + "counts": {}, + "safety": {"has_proposed": False, "has_state_db": False, "has_audit_log": False}, + } + with tarfile.open(bundle_path, "w:gz") as tar: + 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)) + + with pytest.raises(RuntimeError, match="missing file"): + bundle.import_apply(store.kb_dir, bundle_path) + + def test_import_check_flags_path_traversal(store: KBStore, tmp_path: Path) -> None: bundle_path = tmp_path / "evil.tar.gz" _write_malicious_bundle(bundle_path, "../../evil.txt", b"pwned") result = bundle.import_check(store.kb_dir, bundle_path) assert not result.ok - assert any( - "traversal" in i or "unsafe" in i or "absolute path" in i - for i in result.issues - ) + assert any("traversal" in i or "unsafe" in i or "absolute path" in i for i in result.issues) def _write_hash_mismatched_bundle( @@ -188,9 +240,7 @@ def _write_hash_mismatched_bundle( tar.addfile(mf_info, io.BytesIO(mf_bytes)) -def test_import_rejects_member_with_mismatched_sha256( - store: KBStore, tmp_path: Path -) -> None: +def test_import_rejects_member_with_mismatched_sha256(store: KBStore, tmp_path: Path) -> None: """Regression for #74: a tar member whose body does not hash to the sha256 the manifest claims is a documented integrity violation — export_check flags it, so import_check and import_apply must too.""" @@ -208,17 +258,13 @@ def test_import_rejects_member_with_mismatched_sha256( assert not (store.kb_dir / "claims" / "c1.yaml").exists() -def test_import_rejects_source_content_mismatch( - store: KBStore, tmp_path: Path -) -> None: +def test_import_rejects_source_content_mismatch(store: KBStore, tmp_path: Path) -> None: """`_validate_content` skips `sources/*/content` files, so the manifest sha256 is the only thing that can detect substituted source bytes.""" legitimate = b"original source bytes" tampered = b"attacker-controlled bytes" bundle_path = tmp_path / "tampered.tar.gz" - _write_hash_mismatched_bundle( - bundle_path, "sources/deadbeef/content", legitimate, tampered - ) + _write_hash_mismatched_bundle(bundle_path, "sources/deadbeef/content", legitimate, tampered) diff = bundle.import_check(store.kb_dir, bundle_path) assert not diff.ok @@ -242,16 +288,24 @@ def test_import_apply_raises_on_write_time_hash_mismatch( tampered = b"text: TAMPERED\n" bundle_path = tmp_path / "tampered.tar.gz" _write_hash_mismatched_bundle( - bundle_path, "claims/c1.yaml", legitimate, tampered, + bundle_path, + "claims/c1.yaml", + legitimate, + tampered, ) # Force the pre-write check to look clean so the apply path reaches # the write-time re-verify branch. monkeypatch.setattr( - bundle, "import_check", + bundle, + "import_check", lambda *_a, **_k: bundle.ImportCheckResult( - ok=True, bundle_id="deadbeef", - new_files=["claims/c1.yaml"], conflicts=[], identical=[], issues=[], + ok=True, + bundle_id="deadbeef", + new_files=["claims/c1.yaml"], + conflicts=[], + identical=[], + issues=[], ), ) @@ -263,9 +317,7 @@ def test_import_apply_raises_on_write_time_hash_mismatch( assert "bundle.import" not in audit_text, audit_text -def test_import_treats_missing_manifest_sha256_as_mismatch( - store: KBStore, tmp_path: Path -) -> None: +def test_import_treats_missing_manifest_sha256_as_mismatch(store: KBStore, tmp_path: Path) -> None: """Regression for #74 review feedback: a hand-crafted manifest entry without a `sha256` field used to raise a bare KeyError in import_check and import_apply. Treat the missing field as a hash mismatch so the @@ -278,7 +330,9 @@ def test_import_treats_missing_manifest_sha256_as_mismatch( "files": [{"path": "claims/c1.yaml", "size": len(payload)}], "counts": {}, "safety": { - "has_proposed": False, "has_state_db": False, "has_audit_log": False, + "has_proposed": False, + "has_state_db": False, + "has_audit_log": False, }, } with tarfile.open(bundle_path, "w:gz") as tar: @@ -317,14 +371,15 @@ def test_import_rejects_uncited_claim(store: KBStore, tmp_path: Path) -> None: manifest = { "spec": bundle.SPEC_VERSION, "bundle_id": "deadbeef", - "files": [{ - "path": "claims/bundle-uncited.yaml", - "size": len(uncited_yaml), - "sha256": hashlib.sha256(uncited_yaml).hexdigest(), - }], + "files": [ + { + "path": "claims/bundle-uncited.yaml", + "size": len(uncited_yaml), + "sha256": hashlib.sha256(uncited_yaml).hexdigest(), + } + ], "counts": {}, - "safety": {"has_proposed": False, "has_state_db": False, - "has_audit_log": False}, + "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/bundle-uncited.yaml") @@ -344,9 +399,7 @@ def test_import_rejects_uncited_claim(store: KBStore, tmp_path: Path) -> None: assert not (store.kb_dir / "claims" / "bundle-uncited.yaml").exists() -def test_import_check_passes_when_member_matches_manifest( - store: KBStore, tmp_path: Path -) -> None: +def test_import_check_passes_when_member_matches_manifest(store: KBStore, tmp_path: Path) -> None: """The hash check is positive too: a member that matches manifest sha256 should not be reported as `hash mismatch`.""" payload = b"text: original\n"