Skip to content

Fix npm VEX attesting packages with an unpatched bundled copy (#325) - #337

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-npm-vex-bundled-copies
Oct 1, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-npm-vex-bundled-copies

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #325

Summary

If a lock rewires name@version to a Socket patch and another package also bundles that same name@version, vex and scan --vex no longer attest it not_affected. The bundled copy is inBundle: true in v2/v3 locks or bundled: true in v1. npm unpacks it from the parent's tarball, so no rewire reaches it and it stays unpatched. The reference is now reported as patched_ref_unattributable, the warning names the bundled copy's lock location, and the reference isn't attested. That matches how the contract already treats "another lock resolves the same name@version from a non-Socket source".

Changes:

  • vendor/lock_inventory/npm.rs: the shared npm lock walk is split on the bundled flag. npm_lock_nodes is unchanged. The new npm_lock_bundled_nodes returns the bundled entries with their location: the packages key, or the >-joined v1 dependency chain.
  • vex/discover/npm.rs: each npm lock records its bundled purls, and push_uncontested drops any ref whose purl has a bundled copy in either lock of the npm pair. Bundled copies also count as resolved_elsewhere, so they contest the same version in other lock types too. The dropped uuid stays recognized, so a matching ledger record is dead as well (redirect_unwired / vendor_unwired), under --no-verify too.
  • Docs: a CHANGELOG Fixed entry, and the CLI_CONTRACT "Contested locks" rule.

Root cause

npm_lock_nodes skips bundled entries, which is correct for the rewriters. That meant VEX discovery never saw the unpatched bundled copy. contest_across_locks only contests refs from a different file, so the hoisted rewired entry was attested even though the same run warned *_bundled_instance_skipped ("that copy stays UNPATCHED").

Test evidence

Cell from #325 Regression test Red on main Green
hosted + vendored, one lock (v3) vex::discover::npm::tests::bundled_copy_of_the_same_version_contests_the_wired_entry (also checks that a bundled copy of a different version contests nothing) ✅ fails ✅
v1 bundled: true v1_bundled_copy_contests_the_wired_entry ✅ fails ✅
bundled copy in the sibling npm lock bundled_copy_in_the_sibling_npm_lock_contests_the_ref ✅ fails ✅
vendored vex, lock basis and after install (hoisted patched, bundled unpatched) e2e_vex_vendor::vendored_npm_patch_with_an_unpatched_bundled_copy_is_not_attested (plus a no-bundle control that still attests) ✅ fails (status: success, not_affected) ✅
hosted vex, lock basis / ledger (--no-verify) e2e_vex_redirect::hosted_npm_patch_with_an_unpatched_bundled_copy_is_not_attested (plus a control) ✅ fails ✅

The in-run scan --vex goes through the same generator (generate_vex_from_manifest_path, which calls discover_wiring), so it shares the gate. The hosted post-install cell already omitted the purl on main, and it still does.

Red was shown by stashing the two core source files and re-running the new tests against main's implementation.

  • cargo test -p socket-patch-core --all-features --lib -- vex::discover lock_inventory: 435 passed.
  • SOCKET_PATCH_NPM_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_redirect_npm_build -- --ignored (real npm 10, Node 22): 5 passed.
  • cargo test -p socket-patch-cli --all-features --test e2e_vendor_npm_build -- --include-ignored: 14 passed.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --all -- --check: the new code is rustfmt-clean. The remaining diffs are already on main, in files and hunks this PR doesn't touch.
  • cargo test --workspace --all-features --no-fail-fast: 10,397 passed and 22 failed. The 22 are the same sandbox-only set reported on Fix #258: state the real Maven Trusted Checksums floor #322 and Fix Poetry venv discovery to match Poetry (#327, #329) #330: write-failure and permission tests that fail because the sandbox runs as root, plus the peak-RSS stage_local_artifact_caps_oversized_artifact_before_buffering. None of them are in VEX or npm code.
  • The npm, PyPI and gem wrappers only dispatch, so they need no change.
  • CI on 2aa1f21: green. All checks pass. On attempt 1, test-release and test (windows-latest) failed in suites this PR doesn't touch (vendor_gem_lockfile_only_e2e and update_notifier_e2e; see the comment above). Both passed on their single rerun.

Follow-up (not needed for #325)

The vendored out-of-sync probe in vex/verify.rs still checks only the one package_paths copy for each purl. With this change, a bundled copy can no longer produce an attestation. The other duplicates of the same version are ordinary nested entries, and the vendor backend rewires all of those. So the single-copy probe only limits how complete the vendored_tree_out_of_sync warning is, not whether the attestation is correct.

Separately, vendor_gem_lockfile_only_e2e's dead_endpoint() releases its port before use, so tests running in parallel can race for it. The comment above proposes a fix for its own PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BAB2TPeepsb616tuH3Nd3S


Note

Medium Risk
Changes npm VEX discovery and attestation outcomes (fail-closed when bundled copies exist), which affects security attestations but aligns them with actual install bytes.

Overview
VEX and scan --vex no longer attest an npm name@version when the lock also installs an unpatched bundled copy of that same version (inBundle: true in v2/v3 locks, or v1 bundled: true). npm unpacks bundled deps from the parent tarball, so Socket rewires never reach that copy even when the hoisted entry is patched.

Discovery now indexes bundled lock entries via npm_lock_bundled_nodes and treats them like other contested installs: the Socket ref is dropped with patched_ref_unattributable, naming the bundled path, across hosted and vendored wiring and either npm lock in a shrinkwrap/package-lock pair. Bundled purls also register as resolved elsewhere, so they contest the same version in non-npm locks. Matching ledger records stay unwired (redirect_unwired / vendor equivalent) under --no-verify.

CHANGELOG and CLI_CONTRACT document the new contested-lock rule; unit and e2e tests cover v1/v3 locks, sibling locks, hosted redirect ledgers, and vendored lock-basis vs installed trees.

Reviewed by Cursor Bugbot for commit 00495f0. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A project can install the same name@version twice: the lock entry the
hosted or vendored rewriter points at Socket's patched tarball, and a
copy bundled inside another package (inBundle / bundled). npm unpacks
the bundled copy from its parent's tarball, so it stays unpatched, and
the rewriters already warn about that. VEX still attested the package
not_affected, because lockfile discovery skipped bundled entries
entirely.

Discovery now records each bundled entry's name@version. A Socket
reference with a bundled copy of the same version in either npm lock
is reported as patched_ref_unattributable, naming the bundled
location, and is not attested. The bundled copy also contests the
same package in other lockfiles. This covers the in-run scan --vex,
lock-basis and post-install vex runs in hosted and vendored mode.

Fixes #325

Assisted-by: Claude Code:claude-opus-5-5
Adds the CHANGELOG entry and the CLI contract rule: a bundled npm
copy of a patched name@version keeps that patch from being attested.

Refs #325

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review September 30, 2026 16:55
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Two jobs failed on 2aa1f21, both in suites this PR doesn't touch (the diff is limited to vex/discover/npm.rs, lock_inventory/npm.rs, two VEX e2e files and docs):

  • test-release: vendor_gem_lockfile_only_e2e::build_mode_refuses_a_lockfile_only_gem_before_downloading_it panicked at :226 ("build mode cannot use a fetched gem, so it must not download one"). It passes 5/5 locally, and test-release is green on main at the same base (f6b7fb9). Likely cause: a race in the test file itself. dead_endpoint() binds an ephemeral port and then releases it, and the file's five tests run in parallel. Another test's MockServer::start() can pick up that freed port, and this test's mock then sees the neighbour's API calls. Proposed patch, for a separate PR: keep the dead port reserved for the test's lifetime (return the TcpListener alongside the URL and drop it only after the run), or point --api-url at a mock that answers 500 to everything and assert on that instead.
  • test (windows-latest): the update_notifier_e2e target failed. The API returns only the last 5,000 log lines, which don't include the failing test, and the full log archive is blocked by this session's network proxy, so I couldn't read the assertion. The suite passes 39/39 locally on Linux, and the same job is green on main at f6b7fb9 and on Fix Poetry venv discovery to match Poetry (#327, #329) #330 today. Its timing-based cases (grace_budget_bounds_command_latency, the daily rate-limit tests) are the likely candidates on a slow Windows runner.

I've re-run the failed jobs once. If either fails again, I'll treat it as real and dig in.


Generated by Claude Code

Main's v5 consolidation (#277) rewrote the npm discover imports, the
lock_inventory re-exports, the CHANGELOG's Unreleased section and the
CLI contract, so the #325 fix no longer applied cleanly. Keep main's
structure and re-apply the fix on top: the bundled-node walk export and
import, the bundled-copy contest in push_uncontested, a condensed
Unreleased Fixed entry, and the bundled sentence on the Contested locks
rule.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GQoii5oP1pwcJh5mzzo1HU
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 00495f0. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review on 00495f0: mergeable, 0 commits behind main. CI: 458/458 completed checks green (6 workflow skips, no failures). Bugbot reviewed 00495f0 and found no new issues; no unresolved review threads. Reviewer should look at how bundled copies contest refs in vex/discover/npm.rs push_uncontested.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

npm VEX attests not_affected while a bundled (inBundle) copy of the same package@version stays unpatched

3 participants