Skip to content

fix(bundle): reject import of bundles whose manifest lists files missing from tarball#85

Merged
plind-junior merged 2 commits into
vouchdev:testfrom
alpurkan17:fix/import-check-missing-manifest-files
May 28, 2026
Merged

fix(bundle): reject import of bundles whose manifest lists files missing from tarball#85
plind-junior merged 2 commits into
vouchdev:testfrom
alpurkan17:fix/import-check-missing-manifest-files

Conversation

@alpurkan17

@alpurkan17 alpurkan17 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Problem

bundle.import_check walks two loops in series: one over manifest
entries (to compute new_files / conflicts / identical), and one
over tar members (to schema-validate and verify the per-member sha256).
Neither loop flags the case where manifest.json lists a path that has
no corresponding tar member.

The symptom is the inverse of hash mismatch and is just as silent:

  • import_check returns ok=True for a bundle where manifest.json
    claims claims/c1.yaml exists, but the tarball contains no
    claims/c1.yaml member.
  • import_apply then iterates tar members, never reaches the
    manifest-listed-but-missing path, and writes nothing for it. The
    bundle.import audit event records a clean success — but the
    resulting KB is missing artifacts the manifest promised.

export_check already catches this (it walks recorded paths and checks
tar.getmember(path)). The same companion check was missing on the
import side — exactly the asymmetry that issue #74 was about, just for a
different attack/corruption shape (omission instead of substitution).

Fix

Add the missing-member pass to import_check in src/vouch/bundle.py,
mirroring the existing pattern in export_check:

for path in manifest_paths:
    try:
        tar.getmember(path)
    except KeyError:
        issues.append("manifest lists missing file: {path}")

import_apply then refuses to import because check.issues is
non-empty, raising RuntimeError("refusing to import: manifest lists missing file: ...").

Tests

  • test_import_check_rejects_manifest_listing_missing_file — builds a
    tarball with only manifest.json (no content members), asserts
    import_check.ok is False with a "missing file" issue.
  • test_import_apply_rejects_bundle_with_missing_manifest_file
    asserts import_apply raises RuntimeError matching "missing file".

Both pass. All existing bundle tests continue to pass.

Closes #80

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced bundle import/export workflows with stricter manifest and tarball alignment validation.
    • Improved detection and reporting of files listed in manifests that are missing from bundles.
    • Refined error messages and audit logging for bundle operations.
  • Tests

    • Added comprehensive test coverage for bundle validation scenarios involving missing files.

Review Change Stack

…ing from tarball (vouchdev#80)

import_check previously only verified that tar members listed in the
manifest had matching hashes, but never checked the reverse: whether
every manifest entry had a corresponding tar member. A bundle whose
manifest.json referenced claims/c1.yaml but whose tarball contained
only manifest.json would pass import_check with ok=True, and
import_apply would silently write nothing — no exception, no audit
event indicating data loss.

Add the missing-member pass (mirroring the existing check in
export_check) so that manifest entries without a matching tar member
produce a "manifest lists missing file" issue. import_apply then
refuses to import because check.issues is non-empty.
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f8b2287f-074e-4927-8d54-46097bce0fb8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR addresses issue #80 by adding manifest-missing-file validation to bundle.import_check. After verifying per-member file hashes, import_check now iterates over manifest paths and records issues when tar members are absent. The same codebase is reformatted for consistency: multiline dict/result constructors, keyword arguments, and test function signatures are expanded for readability without changing behavior.

Changes

Bundle Integrity Validation

Layer / File(s) Summary
Manifest validation - missing file detection
src/vouch/bundle.py, tests/test_bundle.py
import_check gains a validation loop that detects and reports manifest-listed files missing from the tarball. Two new regression tests verify that import_check rejects such bundles and import_apply raises RuntimeError.
Code formatting for clarity and consistency
src/vouch/bundle.py, tests/test_bundle.py
Multiline dict/result constructors, audit logging keyword args, error message formatting, test function signatures, and mapping literals are reformatted for readability. Behavior and field semantics are unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • vouchdev/vouch#80: The issue directly addresses the bug where import_check accepts bundles whose manifest lists files missing from the tarball—this PR implements the exact fix described.

Possibly related PRs

  • vouchdev/vouch#75: Both PRs enhance bundle integrity validation in import_check and import_apply by checking manifest consistency against tarball contents; this PR extends that work with missing-file detection.
  • vouchdev/vouch#34: Both PRs strengthen bundle validation in src/vouch/bundle.py by adding integrity checks that align manifest claims with actual tarball content before accepting or importing bundles.

Poem

🐰 A manifest whispers of files to find,
But some tar members left nothing behind—
Now import_check says "wait, where's that file?"
And refuses the bundle with a skeptical smile.
Integrity tightened, one loop at a time! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: rejecting bundle imports when manifest lists files missing from the tarball.
Linked Issues check ✅ Passed The PR fully implements the fix described in issue #80: adding manifest-to-tarball member validation in import_check and refusing import with appropriate error messages.
Out of Scope Changes check ✅ Passed All changes are directly related to the stated objective of adding missing-file detection to import_check; formatting adjustments are incidental.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@plind-junior

Copy link
Copy Markdown
Member

PR should target test branch

@alpurkan17
alpurkan17 changed the base branch from main to test May 26, 2026 14:33
@alpurkan17
alpurkan17 force-pushed the fix/import-check-missing-manifest-files branch from 7bb44c0 to f558fd0 Compare May 26, 2026 14:54
@plind-junior

Copy link
Copy Markdown
Member

Fix conflict

@plind-junior
plind-junior merged commit 696f028 into vouchdev:test May 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: import_check accepts bundles whose manifest lists files missing from the tarball (no issue raised)

2 participants