GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix npm VEX attesting a patch the twin lock lacks (#798) by mikolalysenko · Pull Request #799 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #798

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


          Refuse npm VEX when the twin lock lacks the pkg

3a47046

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

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

Uh oh!

There was an error while loading. Please reload this page.


          Make the dual-lock read test use agreeing twins

bfdc21e

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) marked this pull request as ready for review

October 4, 2026 19:12

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          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

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 4, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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) commented Oct 4, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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


          Merge origin/main into agent/fix-npm-twin-lock-missing-entry

33edf82

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 <[email protected]>

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Copy link

Copy Markdown

Collaborator Author

bugbot run

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 5, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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


          Fix vex alias tests broken by store-copy merge

87db844

#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) commented Oct 5, 2026

Copy link

Copy Markdown

Collaborator Author

[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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay Singla (Tanmay182003) reviewed

Oct 5, 2026

View reviewed changes

Comment thread

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

Uh oh!

There was an error while loading. Please reload this page.


          Contest a twin npm lock that lacks the wired version

375f576

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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01TtZrsd52E6vxhvF9hpLVSw

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 5, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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) commented Oct 5, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) merged commit 48085ce into main

Oct 5, 2026

462 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-twin-lock-missing-entry branch

October 5, 2026 17:27

This was referenced Oct 6, 2026

npm 12 never reads npm-shrinkwrap.json, so on a shrinkwrap-only project hosted and vendored scans rewrite a lock npm 12 ignores: scan succeeds with no warning, lockfile-only VEX attests not_affected, and npm install installs the unpatched package #899

Closed

vendor --check fails a vendored package whose lock is contested by a sibling package-lock.json with "no lockfile or config references .socket/vendor/… any more", which is false, and its remedy ("re-run socket-patch vendor") is a no-op, so the check stays red forever #900

Closed

Perf regression: npm/hosted wall +15% (2463257a..9c43dfc9) #993

Closed

Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request

Oct 7, 2026


          Fix npm hosted scan re-walking lockfiles it no longer needs (#993)

3ded39d

The npm/hosted bench drifted +15% wall / +12% CPU over a week of main
(2463257..9c43dfc). The new npm lock passes from #345, #491, #646 and
#799 each added a walk of the 3000-entry package-lock.json, on top of
two costs that scale badly with every extra walk:

- Every hosted scan ran a full second lockfile discovery after the
  rewrite (`hosted_state_from_lockfiles`) only to classify a hosted vs
  vendored takeover, even with no vendored ledger to overlap, and then
  a THIRD one inside `classify_overlap_takeover_with` when there was
  one. The classifier now discovers once, and not at all when the
  vendored ledger has no entries (the hosted common case).
- `Discovery::resolved_elsewhere` deduped with `Vec::contains` on every
  call, so recording a lock's registry entries was quadratic in its
  size. `finalize` already sorts and dedups the list and every earlier
  reader only asks whether some entry matches, so the per-call check
  is gone.

Also trimmed per-entry allocations on the new passes: the npm extractor
builds each entry's purl once (it was built for `mention` and again for
`unwired`), `resolved_is_non_registry` no longer formats a prefix per
entry, and `npm_spec_is_registry` no longer lowercases every edge spec.

Instructions retired for `scan --json --dry-run` on the bench's npm
fixture (median of 9, macOS): base 2463257 2.647G, branch head 2.755G
(+4.1%), this commit 2.353G (-11.1% vs base, -14.6% vs head). Scan
output is byte-identical to before.

Regression test: `takeover_classifier_discovers_at_most_once` counts
discoveries (test-only counter in `discover_wiring`); new unit tests pin
the rewritten registry/tarball predicates.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

+1 more reviewer

github-advanced-security[bot]

github-advanced-security[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.