Skip to content

fix(vendor): verify the extracted tree after a service-download, not just the archive bytes#122

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
fix/vendor-service-copy-verify
Jul 10, 2026
Merged

fix(vendor): verify the extracted tree after a service-download, not just the archive bytes#122
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
fix/vendor-service-copy-verify

Conversation

@mikolalysenko

Copy link
Copy Markdown
Collaborator

Summary

The extract-into-dir service-download paths (composer, cargo, golang, gem) verified only the downloaded archive's sha512 SRI, then reported success synthesized from record.files alone — they never checked that the patched files actually landed at their recorded paths on disk. This adds a post-extraction disk verification so a wrong internal layout fails closed (fallback under auto, hard-fail under --vendor-source=service) instead of shipping an empty/misplaced copy with vendor exiting 0.

The bug

extract_zip(..., strip_first=true) (and the golang/cargo/gem equivalents) assume the archive has exactly one wrapper directory. A prebuilt dist zip whose layout differs — an extra wrapper (outer/pkg-<sha>/src/… → single strip leaves pkg-<sha>/src/…) or a root-level src/… (single strip over-strips to LoggerInterface.php) — extracts "successfully" with every file at the wrong path. The Used arm then maps to already_patched_result(purl, &copy_dir, &record.files), which builds a success result from the record keys without reading copy_dir. Net: vendor --json exits 0, envelope status:success/failed:0, and the copy dir is missing its patched files. Archive-bytes SRI verification (fetch_verified_archive) can't catch this — a self-consistent archive with the wrong internal shape passes it.

This is the root fail-open behind depscan's flaky 94_socket-patch-vendor-composer e2e (run 29040958337): the CLI reported success and the test tripped one line later on a bare ENOENT.

Fix

After extraction, each backend calls the existing shared copy_matches_after_hashes(copy_dir, &record.files) and, on mismatch, routes through its own miss closure:

  • composer / cargo: verify before the success return; remove the bad copy.
  • golang: verify before wiring the consumer's go.mod (so nothing points at a bad copy).
  • gem: verify after the stub gemspec is written (the stub isn't in record.files, so it isn't part of the check).

auto → warn vendor_prebuilt_layout_mismatch + fall back to the local build; --vendor-source=service → hard-fail (no silent wrong copy). npm/pypi are intentionally untouched — they persist a single integrity-verified packed artifact (.tgz/.whl), not an extracted tree, so there's no layout-strip step to get wrong.

Tests

Two new composer_lock regression tests build a double-wrapper zip that the single-level strip misplaces:

  • service_wrong_layout_service_mode_hard_fails: refuses (copy absent at the recorded path) instead of synthesizing success.
  • service_wrong_layout_auto_falls_back_to_build: falls back to a correct local build, with the vendor_prebuilt_layout_mismatch warning recorded.

Both were green-exit-empty-copy before the fix. cargo test -p socket-patch-core --lib vendor:: (546 tests) + clippy pass.

Pairs with

depscan PR fix/composer-vendor-e2e-flake, which independently hardens the e2e assertion (a zero-event vendor no longer passes as success) and captures the envelope + on-disk tree on failure, plus makes the patches/view endpoint fail closed on an unreadable blob.

…just the archive bytes

The extract-into-dir service paths (composer, cargo, golang, gem)
verified only the downloaded archive's sha512 SRI, then synthesized a
success ApplyResult from `record.files` alone — never checking that the
patched files actually landed at their recorded paths on disk. A
self-consistent archive whose internal layout differs from the assumed
single wrapper dir (an extra `pkg-<sha>/` level, or a root-level `src/…`
that the single-level strip over-strips) extracts "successfully" with
every file at the WRONG path, and `vendor` exits 0 with a green envelope
over an effectively empty copy dir. That is the exit-0-empty-copy shape
behind the flaky composer e2e (depscan run 29040958337): the assertion
downstream only tripped one line later on a bare ENOENT, with the
envelope and on-disk tree unrecoverable.

Each backend now runs copy_matches_after_hashes(copy_dir,
&record.files) immediately after extraction (composer/cargo before the
success return; golang before wiring the consumer go.mod; gem after the
stub gemspec is written). On mismatch it routes through the backend's
existing `miss` closure: `auto` falls back to the local build (loud
warning `vendor_prebuilt_layout_mismatch`), `--vendor-source=service`
hard-fails rather than shipping a wrong copy. npm/pypi are unaffected —
they persist a single integrity-verified packed artifact (.tgz/.whl),
not an extracted tree, so they have no layout-strip step.

Regression tests (composer_lock): a double-wrapper zip that the
single-level strip misplaces now (a) hard-fails under service mode with
the copy absent at its recorded path, and (b) falls back to a correct
local build under auto. Both were green-exit-empty-copy before this fix.

Assisted-by: Claude Code:claude-fable-5

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'm not an expert in rust, so I might have overlooked important details

@mikolalysenko Mikola Lysenko (mikolalysenko) merged commit 0f1696c into main Jul 10, 2026
54 checks passed
@mikolalysenko Mikola Lysenko (mikolalysenko) deleted the fix/vendor-service-copy-verify branch July 10, 2026 14:35
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.

2 participants