Fix npm VEX attesting a patch the twin lock lacks (#798) by mikolalysenko · Pull Request #799 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-04T18:29:21Z

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

Fixes #798

Summary

Lockfile-only vex no longer attests a patch when only one of npm-shrinkwrap.json / package-lock.json wires it and the other lock has no entry for the package. It now refuses with patched_ref_unattributable, the same way it already handled a twin lock with a registry entry.

Root cause

vex::discover::npm::push_uncontested counted the twin lock as contesting a wired package only when the twin had a non-Socket entry for the same name@version. Its doc comment said "The lock that does not mention the package at all contests nothing." That's wrong for npm. npm 12 installs from package-lock.json beside a committed shrinkwrap and re-resolves a missing entry from the registry, so the checkout installs unpatched bytes while VEX says not_affected. npm <= 11 does the same with a stale shrinkwrap.

Change

NpmLockRefs now records every package name each lock has any entry for: regular, bundled and non-registry entries, with aliases resolved to the real name.
push_uncontested: a ref is contested when another npm lock has no entry for the package's name. The diagnostic names the twin and says it has no entry for the package.
A twin holding the package only at another version still contests nothing. npm installs that version, not an unpatched copy of the wired one, so there's no false attestation.
docs/testing/npm-compatibility.md documents the rule.
The scan-side warnings the issue quotes (vendor_npm_sibling_lock_unwired, hosted "No package-lock.json entry") are unchanged. The issue's expected behaviour ("vex should refuse here as well") is met at the attestation boundary.

Why this issue: it's a p1 false VEX attestation (a correctness/security bug) with a small, self-contained fix in VEX discovery. It doesn't touch the files the many open gem/pypi agent PRs edit.

Test evidence

Issue	Test	Without fix	With fix
#798 (unit, both directions, hosted + vendored)	`vex::discover::npm::tests::a_sibling_npm_lock_without_the_package_contests_it`	FAILED	ok
#798 (CLI, `socket-patch vex` binary, hosted + vendored × stale package-lock / stale shrinkwrap)	`e2e_vex_lockfile npm::dual_lock_with_one_lock_missing_the_package_attests_nothing`	FAILED	ok
control: twin at another version stays attested	`vex::discover::npm::tests::a_sibling_npm_lock_with_another_version_contests_nothing`	ok	ok

both_locks_are_read_and_the_agreeing_mirror_adds_nothing used disjoint twins, which is exactly the #798 shape. It now uses agreeing twins and still proves both locks are read (4 refs) and that the v2 legacy mirror adds nothing.

Commands run locally (Linux, toolchain 1.93.1):

cargo clippy --workspace --all-features -- -D warnings: clean.
cargo fmt --all -- --check: the touched files are clean. main already has unrelated rustfmt diffs in other files, and CI has no fmt gate.
cargo test -p socket-patch-core --all-features: 5393 passed, 5 failed. One was the fixture above, now fixed: vex:: is 607/607. The other four (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_…) depend on read-only directories, which don't hold when the sandbox runs as root. They don't touch this code.
cargo test -p socket-patch-cli --all-features --test e2e_vex_lockfile: 287/287. --test e2e_vex_redirect 28/28, --test hosted_memory_engine 28/28, --test scan 104/104, --test vendor 81/81.
The full cargo test --workspace could not finish locally because the sandbox disk ran out linking ~240 test binaries. CI runs the full set.
Wrappers (npm/, pypi/, gem/) aren't affected: this is Rust core VEX discovery only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TtZrsd52E6vxhvF9hpLVSw

Note

Medium Risk
Changes npm dual-lock VEX discovery at the attestation boundary; wrong logic would still allow false not_affected claims, though coverage is extensive.

Overview
Lockfile-only vex no longer attests when one of npm-shrinkwrap.json / package-lock.json wires a Socket patch and the twin lock has no entry for that name@version (or only a different version). Those cases emit patched_ref_unattributable with messaging that npm may re-resolve from the registry depending on major, matching the existing “sibling resolves elsewhere” rule.

Discovery now tracks every name@version mentioned in each lock (NpmLockRefs.mentioned, populated from normal, bundled, and non-registry entries). push_uncontested treats a sibling that does not mentions the wired purl as contesting the ref. The same name@version at another path in the twin still agrees and attests.

Tests/docs: new unit cases for missing sibling entries, wrong-version siblings, and agreeing paths; e2e dual_lock_with_one_lock_missing_the_package_attests_nothing; npm-compatibility.md updated. vex_consumed hosted npm tests were adjusted so alias expansion is still exercised after the name-keyed resolver (#605) finds aliases on its own.

^{Reviewed by Cursor Bugbot for commit 375f576. Configure here.}

Generated by Claude Code

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

With both npm-shrinkwrap.json and package-lock.json committed, lockfile-only `vex` attested a patch that only one lock wired when the other lock had no entry for the package at all. npm 12 installs from package-lock.json and re-resolves a missing entry from the registry, so the checkout installed unpatched bytes while the VEX document said `not_affected`. A twin lock with no entry for the package now contests the wiring the same way a registry entry does (`patched_ref_unattributable`), in both directions and for hosted and vendored wiring. A twin that holds the package only at another version still contests nothing: npm installs that version, not unpatched bytes of the patched one. Fixes #798 Assisted-by: Claude Code:claude-opus-5-5

The test that proves both npm locks are read wired each package in only one lock. After #798 such a pair is contested (npm re-resolves the package missing from the other lock), so the fixture now has each lock wire both packages. It still proves both locks are read (4 refs) and that the v2 legacy mirror adds nothing. Refs #798 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-04T19:12:33Z

BugBot review

Generated by Claude Code

CodeQL flagged the new dual-lock test for printing the wired patch reference (which holds the patch uuid) in an assertion message. The message now names the wiring mode instead. Refs #798 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-04T19:14:40Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-04T19:26:33Z

[agent] CI status for the native compatibility suites:

PDM patch compatibility / native (ubuntu-latest, 2.25.9) failed on d604901 in the pep582 hosted cell. On 3a47046, native (ubuntu/macos, 2.22.4) failed in a different cell (multi-target hosted, rescanIdempotent). On bfdc21e, Bun patch compatibility / native (ubuntu-latest, 1.0.0) failed in transitive hosted (exitCodeContract), although Bun passed on 3a47046, which has the same production code.
I don't think this PR causes these. It only changes npm package-lock.json / npm-shrinkwrap.json VEX discovery (vex/discover/npm.rs), and those PDM and Bun projects have no npm lock. A different cell fails on each run. These harnesses call the live patches-api.socket.dev with retries (attempts 2), so a transient API failure fails whichever hosted cell hits it. I couldn't reproduce from the sandbox because the live API isn't reachable here.
No fix exists to port. I've re-run the failed PDM job once. If it fails again in the same cell, I'll treat it as real and dig in.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-04T20:10:17Z

Ready for review (burn-down agent).

Head: d604901661
CI: 461/461 check runs green on head
Bugbot: reviewed d604901661, no new issues; no unresolved review threads
Mergeable, no conflicts (blocked only on required human approval)

Generated by Claude Code

Conflict only in the NpmLockRefs doc comment (vex/discover/npm.rs): kept main's bundled-location wording plus this PR's package-name set; main's unwired→BTreeMap change and #588 same-lock check combine with the missing-entry contest unchanged. Co-Authored-By: Claude <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) · 2026-10-05T12:11:56Z

bugbot run

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

…ck-missing-entry

#605 taught the name-keyed npm resolver to probe bundled store trees, so it now finds aliased copies (node_modules/lp) and a nested host's store peers itself. Two vex_consumed tests from #738 assumed that set never held aliases, so main's CI went red after both merged. The tests now feed the alias-free set explicitly to keep covering alias expansion, and also check the resolver's own set reaches the same copies with no duplicates. No production code changes. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 40dac07)

Mikola Lysenko (mikolalysenko) · 2026-10-05T12:43:36Z

[agent] coverage and test-release failed on 33edf82: commands::vex_consumed::tests::hosted_expands_alias_only_copies and hosted_reuses_expanded_npm_copies_and_merges_alias_variants don't pass.

This PR didn't cause it. Current main (4646693, #605) fails both tests on its own, and they pass on 4646693^. CI ran them because it tests the PR merged into the latest main. The fix exists as #851 (40dac07, test-only).

I pushed 87db844: it merges the current main and cherry-picks 40dac07. Once main carries #851, the cherry-pick changes nothing. Locally on that head: socket-patch-cli --lib passes 840/840, core vex:: 610/610, e2e_vex_lockfile 287/287, and clippy is clean.

Generated by Claude Code

A twin lock that held the package only at another version still let the wired ref through. npm keeps that entry only while it satisfies package.json, and otherwise fetches the wired version unpatched from the registry, so the lock alone can't vouch for it. The twin now contests unless it has an entry for the same name@version at any path. Lock pairs the rewriters keep in sync share that set, so they are unaffected. Refs #798 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TtZrsd52E6vxhvF9hpLVSw

Mikola Lysenko (mikolalysenko) · 2026-10-05T13:18:20Z

BugBot review

Generated by Claude Code

cursor

✅ 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 375f576. Configure here.}

Mikola Lysenko (mikolalysenko) · 2026-10-05T15:26:47Z

Burn-down agent: labeled Ready for review.

Head: 375f5768a3738a1c16ad090925efa20637292be6
CI: 461/461 check runs green (success/skipped) on this head; mergeable clean
Bugbot: reviewed 375f576, no new issues; 2 earlier threads resolved
Reviewer focus: push_uncontested twin-lock-missing-entry refusal in vex/discover/npm.rs; already approved

Generated by Claude Code

Start fix for #798

7942656

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

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 4, 2026

npm VEX attests a patch that only npm-shrinkwrap.json wires when the package-lock.json beside it has no entry for the package, though npm 12 reads package-lock.json and installs the registry copy #798

Closed

github-advanced-security Bot found potential problems Oct 4, 2026

View reviewed changes

Comment thread crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs Fixed

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 4, 2026 19:12

Keep patch refs out of a vex test's messages

d604901

CodeQL flagged the new dual-lock test for printing the wired patch reference (which holds the patch uuid) in an assertion message. The message now names the wiring mode instead. Refs #798 Assisted-by: Claude Code:claude-opus-5-5

cursor Bot reviewed Oct 4, 2026

View reviewed changes

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

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 5, 2026

Fix uv vendored revert half-reverting after a relock (#806, #821) #822

Merged

4 tasks done

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

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Claude (claude) added 2 commits October 5, 2026 12:28

Merge remote-tracking branch 'origin/main' into agent/fix-npm-twin-lo…

56968d8

…ck-missing-entry

Tanmay Singla (Tanmay182003) reviewed Oct 5, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/vex/discover/npm.rs

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Tanmay Singla (Tanmay182003) approved these changes Oct 5, 2026

View reviewed changes

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

Mikola Lysenko (mikolalysenko) merged commit 48085ce into main Oct 5, 2026
462 checks passed

Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-twin-lock-missing-entry branch October 5, 2026 17:27

Navigation Menu

Repository navigation

Fix npm VEX attesting a patch the twin lock lacks (#798) - #799

Fix npm VEX attesting a patch the twin lock lacks (#798)#799
Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
agent/fix-npm-twin-lock-missing-entry

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Change

Test evidence

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading