Fix npm rewiring git/URL/file lock entries (#326)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #326

Summary

scan --mode hosted and vendor no longer rewire an npm lock entry that npm installs from a git, remote-tarball (URL) or file: spec, and vex no longer attests one. Before this change both modes rewrote that entry's resolved/integrity and reported the package patched, and vex attested not_affected. Meanwhile npm ci fetched the git checkout or URL again and installed the original bytes.

Changes:

• vendor/npm_origin.rs (new): npm_non_registry_entries(lock) returns the packages keys npm installs from a non-registry source, each with a reason. An entry counts as non-registry in two cases:

  • An inbound dependency spec that resolves to it is not a registry spec. npm_spec_is_registry mirrors npm-package-arg: versions, ranges, dist-tags and npm: aliases count as registry; git, GitHub shorthand, URLs, paths and tarball names don't. The edges come from every entry's dependencies/optionalDependencies/devDependencies/peerDependencies, including the root and workspace members, resolved in node's lookup order.
  • Its own resolved is git+…/github:…/… or a file: outside .socket/vendor/.

  The spec check is what catches a URL spec whose resolved looks exactly like a registry tarball. It also catches an entry that has already been rewired: our own wiring only rewrites resolved, never the dependent's spec.

• Hosted (patch/redirect/mod.rs): such an entry is skipped with redirect_npm_non_registry_entry_skipped, a stays-UNPATCHED warning that names the entry and the spec. It still counts as a match, so redirect_npm_entry_not_found stays quiet.

• Vendored (vendor/npm_lock.rs): such an entry is skipped with vendor_non_registry_entry_skipped. When no rewritable copy is left, the vendor refuses with vendor_lock_entry_not_rewritable (the existing bundled/link refusal, with its wording widened) and writes nothing.

• v2 legacy mirror (both modes): in a lockfileVersion 2 lock, each legacy dependencies node maps to the packages key it mirrors (legacy_packages_key). A node whose twin is non-registry is left alone, even when it stores the plain version.

• VEX (vex/discover/npm.rs): each lock drops every ref whose name@version has a non-registry copy in that lock, diagnosed as patched_ref_unattributable. That copy also counts as resolved elsewhere, so it contests the same version in the sibling npm lock and in other lock types. This covers locks that were rewired before this fix, and a registry copy that is wired while a git copy of the same version stays unpatched next to it.

• Docs: a CHANGELOG Fixed entry, and the CLI_CONTRACT npm row.

Root cause

The npm lock rewriters pick entries by name and version only: rewrite_one_npm_lock and rewrite_npm_v2_deps for hosted, scan_lock_matches and rewrite_legacy_tree for vendored. Only link/inBundle entries are excluded. npm installs a git, URL or file: dependency from the dependent's spec and ignores the lock's resolved, so the rewrite had no effect at install time. VEX discovery trusted the rewired resolved in the same way.

Not affected: lockfileVersion 1. There, a git/URL/file: entry's version is the spec itself, so it never matches a patch's name@version.

Test evidence

#326 case	Regression test	Red on main	Green
hosted: git, URL-tarball, file: tarball (direct)	patch::redirect::tests::npm_non_registry_entries_are_skipped_with_loud_warning	✅ fails	✅
hosted: nested git copy skipped, hoisted registry copy still rewired	npm_nested_git_copy_is_skipped_and_registry_copy_rewired	✅ fails	✅
hosted, v2: legacy mirror of the git copy untouched, registry copy's mirror rewired	npm_v2_legacy_mirror_of_a_non_registry_entry_is_not_rewired	✅ fails (guard disabled)	✅
vendored: git, URL, file: (refuses, writes nothing)	vendor::npm_lock::tests::non_registry_only_instances_refuse_and_write_nothing	✅ fails	✅
vendored: nested git copy skipped	nested_git_instance_is_skipped_with_warning	✅ fails	✅
vendored, v2: legacy mirror of the git copy untouched	v2_legacy_mirror_of_a_git_instance_is_not_rewritten	✅ fails (guard disabled)	✅
VEX: hosted- and vendored-wired entry with a git/URL/file: spec, nested git copy contests the wired copy, plus a registry control	vex::discover::npm::tests::entries_npm_installs_from_a_non_registry_spec_are_not_attested	✅ fails	✅
real npm 10 / 12: URL-tarball dependency, vendor then vex	e2e_vendor_npm_build::npm_vendor_refuses_a_remote_tarball_dependency	✅ fails (applied: 1)	✅
classifier	vendor::npm_origin::tests::* (9 tests: spec classification, node lookup, workspaces, aliases)	new	✅

Red was shown by stashing the three core source files and re-running the new tests against main's implementation. For the two v2-mirror tests, it was shown by disabling the new guard.

• cargo test -p socket-patch-core --all-features --lib -- npm vex::discover redirect: 1494 passed and 1 failed. The failure is vlt_heal::tests::an_unremovable_hidden_lock_keeps_every_store_entry, which fails on the unchanged branch too (the sandbox runs as root).
• SOCKET_PATCH_NPM_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_vendor_npm_build --test e2e_redirect_npm_build -- --include-ignored (real npm 10.9.7, Node 22): 14 + 15 passed. The new e2e also passes against npm 12.1.0.
  • npm 12 refuses URL specs by default (EALLOWREMOTE). That made the new e2e fail its fixture install in CI's install-proof (12.x) legs, so it now passes --allow-remote=all on npm ≥ 12.
• 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 (on d53ef13): 10,407 passed and 22 failed. The 22 are the same sandbox-only set reported on Fix #258: state the real Maven Trusted Checksums floor #322, Fix Poetry venv discovery to match Poetry (#327, #329) #330 and Fix npm VEX attesting packages with an unpatched bundled copy (#325) #337: 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 npm or VEX code.
• CI on c46271c is green. Three jobs on earlier commits hit registry timeouts and passed on a re-run or on the next push: Composer/Windows (Packagist), macOS vlt 1.2.0 install-proof (npm registry) and macOS Poetry 1.8.5 (PyPI).
• CodeQL flagged two new test assertions for "cleartext logging" because they printed the patch URL with its uuid. Fixed in f07b94a, and the new diagnostic no longer prints the uuid either.
• The npm, PyPI and gem wrappers only dispatch, so they need no change.
• There's no real-npm hosted e2e for this. The hosted capstone fixture hard-codes a registry install, so the hosted path is covered by the rewriter unit tests above. The in-run scan --vex goes through the same discovery gate.

Notes

• This touches vex/discover/npm.rs next to Fix npm VEX attesting packages with an unpatched bundled copy (#325) #337, which adds a same-shaped gate for bundled copies. The two edit different functions (extract_package_lock here, push_uncontested there), so whichever lands second should need at most a trivial merge.
• Known limit: when a root overrides entry forces a URL spec onto a registry-range edge, the lock alone doesn't show it. The resolved check still catches forced git and file: sources.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LycgFGuki2BqZ25VNhwxJ5

---

Note

Medium Risk
Changes npm lock rewrite, vendoring, and VEX attestation gates for a common dependency pattern; incorrect classification could skip legitimate registry patches or still over-attest, but behavior is covered by extensive tests and fail-closed warnings.

Overview
Fixes false “patched” reporting for npm deps installed from git, URLs, or file: specs (#326). npm honors the dependent’s spec, not the lock entry’s resolved, so rewiring those entries did not change what npm ci installed while hosted/vendored flows and VEX still claimed success.

A shared npm_non_registry_entries classifier (new vendor/npm_origin.rs) drives consistent behavior: hosted scan skips with redirect_npm_non_registry_entry_skipped, vendor skips or refuses with vendor_non_registry_entry_skipped / vendor_lock_entry_not_rewritable, and VEX discovery drops attestations for affected name@version pairs (patched_ref_unattributable). Lockfile v2 legacy dependencies mirrors of skipped entries are left untouched. Docs and e2e/unit tests cover git, URL tarball, file:, and mixed registry/non-registry trees.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] native (macos-latest, 1.8.5) failed on f07b94a. I don't think this PR caused it:

• The direct hosted case errored before any assertion ran: poetry install hit ConnectionError … Cannot install urllib3 while setting up the fixture.
• The crlf hosted case failed on the four install-dependent checks (poetryInstallExit0, installedBytesPatched, warmInstallReplacesUpstream, freshCloneInstallsPatch). The same job's vex and relock checks passed.

This PR only changes npm lock handling and npm VEX discovery; no PyPI or Poetry code is touched. I've re-run the failed jobs of run 36755620097 once. If it fails again, I'll treat it as a real failure and dig in.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] install-proof (macos-latest, 1.2.0) failed on f07b94a in vlt_pinned_matrix_vendored_autocrlf_checkout. I don't think this PR caused it:

• vlt ci failed with TypeError: fetch failed, caused by ConnectTimeoutError … attempted addresses: 172.66.3.58:443, 162.159.143.62:443, timeout: 10000ms. The registry never answered.
• The other 31 tests in that binary passed, as did the rest of the job's vlt legs.

This PR doesn't change any vlt code. I can't re-run the job yet because workflow run 36755620093 still has jobs in progress (the API returns 403). I'll re-run it once when the run finishes, and treat a second failure as real.

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] composer 2.2.30 / php 8.3 / windows-latest failed on c46271c in composer_vendor_keeps_files_mirror_filters_would_drop. I don't think this PR caused it:

• The fixture's composer update failed with curl error 28 while downloading https://repo.packagist.org/packages.json: Connection timed out after 10001 milliseconds. The test's own message suggests the same cause: registry unreachable?.
• The job's other composer e2e tests passed.

This PR only changes npm lock handling and npm VEX; no Composer code is touched. I can't re-run it yet because run 36758922826 still has jobs in progress (403). I'll re-run it once when the run finishes, and treat a second failure as real.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Ready for review on 8e361c2: mergeable, 0 commits behind main. CI: 488/488 completed checks green (6 workflow skips, no failures). Bugbot reviewed 8e361c2 and found no new issues; no unresolved review threads. Reviewer should look at npm_spec_is_registry in vendor/npm_origin.rs (mirrors npm-package-arg classification).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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

[Mikola Lysenko (mikolalysenko)]

[agent] Ready for review on f1e0108.

• Merged main twice to clear conflicts:
  • CHANGELOG: kept both sides' Fixed entries.
  • vex/discover/npm.rs: kept both npm VEX attests not_affected while a bundled (inBundle) copy of the same package@version stays unpatched #325's bundled-copy contest (from main) and this PR's drop_non_registry_installs. Both now run in extract_package_lock, and both sets of tests are present.
• CI: 483/483 check runs green (6 skipped, 1 neutral). Locally, cargo test -p socket-patch-core --lib vex::discover::npm passes (38/38), as do the non_registry and bundled filters (19/19).
• Bugbot reviewed f1e0108 and found no issues. The earlier Medium finding (v2 legacy mirror) was fixed in c46271c.
• One commit behind main; merges without conflicts.
• For the reviewer: the merged extract_package_lock in crates/socket-patch-core/src/vex/discover/npm.rs.

Slack announcement not sent this run: the Slack send tool isn't available in the agent session. The next run will retry.

---

Generated by Claude Code