fix:bundle import can overwrite the audit log

[jonathanchang31]

Summary

This PR fixes issue #166 by making bundle import reject non-committable or unsafe files before they can be written into .vouch/.

Previously, a crafted bundle containing audit.log.jsonl could pass vouch import-check and overwrite the append-only audit log when imported with --on-conflict overwrite.

The fix enforces the documented bundle safety contract:

• reject audit.log.jsonl
• reject state.db
• reject state.db-*
• reject proposed/*
• reject unsafe manifest safety flags such as has_audit_log: true

Related Issue

Fixes: #166

Change Type

• Bug fix
• Security hardening
• Regression test
• New feature
• Breaking change
• Documentation update

Real Behavior Proof

Before the fix, this malicious bundle was accepted:

vouch import-check audit-overwrite.tar.gz

{
  "bundle_id": "audit-overwrite-demo",
  "conflicts": ["audit.log.jsonl"],
  "issues": [],
  "ok": true
}

And overwrite import replaced the audit log:

vouch import-apply audit-overwrite.tar.gz --on-conflict overwrite

{
  "written": ["audit.log.jsonl"],
  "on_conflict": "overwrite"
}

After the fix, the same bundle is rejected:

vouch import-check audit-overwrite.tar.gz

{
  "bundle_id": "audit-overwrite-demo",
  "issues": [
    "manifest safety flag has_audit_log=true includes forbidden audit.log.jsonl",
    "forbidden path in bundle: 'audit.log.jsonl'"
  ],
  "ok": false
}

And import refuses to write:

vouch import-apply audit-overwrite.tar.gz --on-conflict overwrite

Error: refusing to import: manifest safety flag has_audit_log=true includes forbidden audit.log.jsonl

The original audit log remains intact.

Checklist

• Reproduced the original issue locally
• Confirmed import-check previously accepted the malicious bundle
• Confirmed import-apply --on-conflict overwrite previously overwrote audit.log.jsonl
• Implemented import safety validation
• Added regression tests
• Verified the malicious bundle is now rejected
• Verified audit log is not modified after rejection
• Ran targeted bundle tests
• Ran full test suite
• Ran lint
• Built package successfully

Summary by CodeRabbit

• Security

  • Strengthened bundle import validation with stricter safety checks.
  • Bundles with forbidden artifacts or unavailable components are now rejected during import.
  • Import validation failures are detected earlier in the import process.

• Tests

  • Added comprehensive test coverage for bundle import security validation.

[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: dd2b720e-4758-4e1f-868a-e717d22933bf

📥 Commits

Reviewing files that changed from the base of the PR and between 3beb821 and eefc7c5.

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

This PR hardens bundle import security by adding manifest-level and path-level validation. It defines forbidden artifacts (audit.log.jsonl, state.db*, proposed/*) and validates bundle manifest safety flags, rejecting imports that claim unavailable components or contain unsafe member paths during both import_check and import_apply.

Changes

Bundle import safety enforcement

Layer / File(s)	Summary
Import-time path and manifest safety validation functions src/vouch/bundle.py	Introduces IMPORT_ROOT_FILES constant and forbidden safety flags mapping. Adds _import_path_issue() to block tar members outside export directories and reject specific forbidden paths (audit.log.jsonl, state.db*, proposed/*). Adds _manifest_safety_issues() to validate manifest safety object and report flags set to true for forbidden content.
Validation integration into import flow src/vouch/bundle.py	Updates _safe_member_path() to use _import_path_issue() for rejecting unsafe tar member paths. Extends import_check() to accumulate _manifest_safety_issues() results, causing import to fail if manifest claims unavailable components.
Test infrastructure and safety-focused import tests tests/test_bundle.py	Updates _write_malicious_bundle() helper to accept optional safety keyword parameter for per-test manifest safety control. Adds parametrized tests verifying import_check rejection of non-committable tar paths and bundles with unsafe manifest safety flags, and confirming import_apply rejection and audit log preservation.

Possibly related PRs

• vouchdev/vouch#34: Extends import_check/import_apply with additional validation gates, focusing on Pydantic schema and content validation.
• vouchdev/vouch#24: Updates import-side path safety logic in _safe_member_path and adds rejection behavior for unsafe/escapable tar member paths.
• vouchdev/vouch#85: Updates import_check to reject bundles based on manifest.json contents, detecting missing paths in the tarball.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A bundle comes to vouch's door,
But we check the contents before—
No audit log theft, no state DB swap,
Just safe exports, no sneaky flop! ✨

🚥 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 directly addresses the main bug fix: preventing bundle import from overwriting the audit log, which is the primary objective of this PR.
Linked Issues check	✅ Passed	The PR implements all required objectives from #166: rejects forbidden paths (audit.log.jsonl, state.db, state.db-, proposed/) and unsafe manifest safety flags during both import_check and import_apply.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to addressing issue #166: adding import-specific path validation rules and manifest safety checks to prevent audit log overwriting.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[jonathanchang31]

@plind-junior Could you plz review my PR?

[plind-junior]

Review

Summary: Adds _import_path_issue and _manifest_safety_issues to enforce the bundle safety contract that was documented but never enforced. The path-level check is wired into _safe_member_path, giving write-time defence-in-depth identical to the existing TOCTOU protection for hash mismatches. The manifest safety flag check fires early in import_check (before the file loop), and import_apply re-runs the full import_check at its start, so forbidden bundles are rejected before any write.

What works

• bundle.py:146–180 — _import_path_issue correctly layers the existing _unsafe_name_reason checks, then adds the per-path forbidden list and the EXPORT_SUBDIRS allowlist. Paths like state.db.backup or audit.log.jsonl.bak are caught by the subdir allowlist even though they don't match any explicit forbidden prefix.
• bundle.py:183–192 — _manifest_safety_issues rejects bundles that declare has_audit_log/has_state_db/has_proposed=true at the manifest level, closing the gap where a crafted manifest could signal intent to include forbidden content even if the per-path check hadn't fired yet.
• bundle.py:225–227 — replacing _unsafe_name_reason with _import_path_issue in _safe_member_path means the forbidden-path check fires at write time inside import_apply, not just during import_check. This is the same defence-in-depth pattern already used for the per-member hash re-verify (bundle.py:338).
• bundle.py:284 — issues.extend(_manifest_safety_issues(manifest)) is placed before the file iteration loop, so a manifest with a forbidden safety flag fails fast and the error message is clear.
• tests/test_bundle.py:162–224 — the two parametrized tests cover all four forbidden path patterns and all three safety flag permutations, and each test independently verifies both import_check rejection and import_apply raise, which matches the two-step contract.

Suggestions

• [non-blocking] tests/test_bundle.py:113 — The safety or {default} idiom is a mild footgun: an empty dict {} is falsy, so _write_malicious_bundle(path, name, payload, safety={}) would silently fall back to the all-false default instead of writing an empty safety object. Prefer safety if safety is not None else {default} for a test helper that is also called in existing hash-mismatch tests. No test currently passes safety={}, so this doesn't affect correctness today.

• [non-blocking] bundle.py:149–165 — FORBIDDEN_SAFETY_FLAGS and the explicit forbidden-path branches in _import_path_issue are manually kept in sync (both list audit.log.jsonl, state.db, proposed/). A future contributor adding a new forbidden artifact would need to update both places. A brief comment cross-referencing the two, or deriving the check from the constant, would prevent silent divergence.

Verdict

approve — The fix correctly closes the reported vulnerability. Both import_check and import_apply now reject forbidden paths and unsafe manifest flags, write-time protection is in place for path checks (matching the existing TOCTOU defence for hash mismatches), and the regression tests are thorough. The two suggestions above are cosmetic; neither affects correctness or security.

[plind-junior]

PR didn't target test branch will be auto closed by an agent

[jonathanchang31]

Replacement PR targeting test: #183\n\nThe branch has been rebased onto upstream/test, pushed, and revalidated. GitHub would not allow changing the base of this closed PR, so I opened the replacement PR instead.

[jonathanchang31]

@plind-junior Could you plz reopen my PR #167 and draft? Thanks!