[Mikola Lysenko (mikolalysenko)]

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

Fixes #1965

Summary

When the root package.json overrides a registry range with a remote tarball URL, git or path spec, npm installs that spec and ignores the lock's resolved. Hosted and vendored scans now treat that copy like any other non-registry install. Hosted skips it with redirect_npm_non_registry_entry_skipped, vendored refuses it (vendor_lock_entry_not_rewritable), and VEX lockfile discovery attests nothing for it. Before this change, both modes rewired the copy and exited 0, npm ci then installed the unpatched upstream bytes, and vendored vex / vendor --check and the hosted in-run --vex still attested the patch.

Root cause

npm_non_registry_entries (crates/socket-patch-core/src/vendor/npm_origin.rs) skipped every edge whose declared spec is a registry spec before looking at overrides. It only handled the opposite direction (#490: a git/URL spec that an override turns into a registry one). A remote-tarball override also gets past the resolved check, because its resolved is an ordinary https://…tgz. The hosted rewriter, the vendored lock writer and VEX discovery all read this one function, so every mode was affected.

Fix

• NpmOverrides::non_registry_replacement: for a registry-spec edge, returns the non-registry spec an override makes npm install. When the winning rule is decided exactly, its value counts. When it isn't decided (a target selector other than the spec, a range selector on an ancestor, or equally close rules that disagree), any rule for the package with a non-registry value counts. That follows the module's existing rule for unclear cases: leave the copy unpatched and reported, never attest it. dependent_chain now starts a dependent inside a workspace member with that member (named by its name field, else its root-level link entry, else its folder with any @scope/ parent), because npm reaches members through the root's link edge and applies member-scoped rules to them (this also makes the npm hosted and vendored modes refuse a registry-installed package when its dependent's git spec is replaced by an overrides entry (regression from #345) #490 direction recognize member-scoped registry overrides). A fast path returns early when no rule for the package names a non-registry spec, so locks with unrelated overrides don't pay a per-edge chain walk.
• npm_override_only_entries: lists the entries flagged only because of such an override. VEX discovery keeps a Socket pin at one of those entries shadowed, not foreign, so rollback / remove can still unwind a pin that a release before this fix wrote there. Without this, those projects would be left with a hosted pin nothing unwinds.
• No wrapper changes (npm/, pypi/, gem/ only dispatch to the binary).

Tests (red → green)

Every new test fails with the fix neutralized (non_registry_replacement forced to None) and passes with it:

Test	Covers
vendor::npm_origin::tests::issue_1965_an_override_of_a_registry_edge_to_a_url_is_non_registry	top-level, scoped (foo, foo@1.0.0), "." object, $name, dep@spec selector; URL/git/file:/path/npm: alias-to-URL; fresh, hosted-pinned and vendored-wired resolved
…issue_1965_an_unclear_override_that_may_swap_in_a_url_keeps_the_edge	range selectors, contested rules, unresolvable $ref beside a URL rule
…issue_1965_a_url_override_scoped_to_a_workspace_member_is_non_registry	a URL rule keyed by a workspace member's name (app, app@2.0.0) reaches the member's deps; a non-matching version or another member doesn't (Bugbot finding on 70d9aff)
…issue_1965_a_workspace_member_is_named_as_npm_names_it	a scoped member without name keeps its @scope/ folder; only the root-level link names a member, not a nested alias link (second Bugbot finding, on 2df1c47)
…issue_1965_a_url_override_of_a_nested_registry_edge_names_its_copy	only the nested overridden copy is flagged, not a hoisted twin
…issue_1965_registry_and_no_op_overrides_of_a_registry_edge_stay_registry	control (passes before and after): registry, *, empty, non-applying and shadowed rules
patch::redirect::tests::issue_1965_a_registry_edge_overridden_to_a_url_is_unredirected	hosted rewriter: lock untouched, warning names the override, patch kept out of the in-run VEX; registry-override control still redirects
vendor::npm_lock::tests::issue_1965_a_registry_edge_overridden_to_a_url_is_refused	vendored writer refuses and writes nothing; registry-override control vendors
vex::discover::npm::tests::issue_1965_a_registry_edge_overridden_to_a_url_is_not_attested	hosted and vendored pins: not attested, and still shadowed for rollback (verified red with the shadow exclusion removed)
e2e_redirect_npm_build::npm_redirect_skips_a_registry_edge_overridden_to_a_url (ignored e2e)	real npm 10.9.9, the issue's foo + URL-override fixture: hosted scan skips loudly, lock byte-identical, nothing attested not_affected. Red before the fix (scan reported applied)

Per-issue checklist

• npm hosted and vendored scans pin a dependency that overrides forces to a remote tarball URL; npm ci installs it unpatched, and vendored vex / vendor --check still pass #1965 hosted: patch::redirect::tests::issue_1965_…, e2e_redirect_npm_build::npm_redirect_skips_a_registry_edge_overridden_to_a_url
• npm hosted and vendored scans pin a dependency that overrides forces to a remote tarball URL; npm ci installs it unpatched, and vendored vex / vendor --check still pass #1965 vendored: vendor::npm_lock::tests::issue_1965_…
• npm hosted and vendored scans pin a dependency that overrides forces to a remote tarball URL; npm ci installs it unpatched, and vendored vex / vendor --check still pass #1965 vex / vendor --check attestation: vex::discover::npm::tests::issue_1965_…
• npm 8 (lockfileVersion 2) / 10 / 12 (allow-remote=all): the logic reads packages only (lockfileVersion ≥ 2) and doesn't depend on the npm version. The e2e runs on whatever npm the CI matrix leg pins.

Local verification

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• cargo test --workspace --all-features --no-fail-fast: everything passes except 11 tests that fail for environmental reasons, all in code this PR doesn't touch. The sandbox runs as root, so 9 chmod/unremovable write-failure tests can't make writes fail (in_process_redirect ×3, repair ×2, core copy_tree / vlt_heal / pypi_poetry / pypi_requirements). The other 2 (e2e_bun_lockb) fetch blobs from the live patches-api.socket.dev, which the sandbox can't reach (see Fix bun.lockb nightly e2e reaching the live blob API #1905). CI runs as non-root.
• SOCKET_PATCH_NPM_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_redirect_npm_build -- --ignored npm_redirect_skips_a_registry_edge_overridden_to_a_url: ok (npm 10.9.9)

---

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 review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

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

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: fa843ede4f3ab84c19f72176979069deafc5e563
• CI: 31/31 green, 19 path-skipped; no main-wide failures.
• Bugbot: clean on fa843ed. Its two earlier findings (on 70d9aff and 2df1c47: member-scoped override rules and naming workspace members) were fixed in later commits, and both threads are resolved.
• Mergeable, no CHANGELOG.md change.
• For reviewers: the "unclear override" rule in NpmOverrides::non_registry_replacement errs toward leaving a copy unpatched. Also check that the npm_override_only_entries shadowed-pin path keeps rollback/remove able to unwind pins written by earlier releases.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review: one gap to settle before this merges (agent:needs-human)

The fix still misses overrides scoped to a logical ancestor that isn't on the physical lock path. In that case the copy is treated as registry, so it gets rewired and attested while npm ci installs the override's URL. That is the #1965 failure again, in the unsafe direction. It is not a regression from main.

Where: npm_origin.rs:137-158 (non_registry_replacement). search builds its chain with dependent_chain, which walks physical node_modules/ nesting. When no rule on that chain matches, the result is best == None, unclear == false. That counts as "decided", so the function returns None, even though any_non_registry_rule found a URL rule for the package. npm applies a scoped override to the package's whole logical subtree, at any depth, through dependency edges, and hoisting doesn't change that.

Repro: root → a → b → left-pad, with b and left-pad hoisted to node_modules/b and node_modules/left-pad, and "overrides": {"a": {"left-pad": "https://…/lp.tgz"}}. npm installs left-pad from the URL, but npm_non_registry_entries returns {}. A scratch unit test on this head confirmed it. The same gap applies to a workspace member's scoped rule when the dependency that pulls the package in is hoisted to the root.

Question for a human: which behavior do we want?

1. Conservative: when a non-registry rule for the package exists under a scope and no physical-chain rule decides the edge, count the copy as non-registry, as the doc comment at :132-135 already describes. This flips the existing assertion that {"other": {"left-pad": URL}} stays registry (npm_origin.rs:1388). Unrelated copies would be left unpatched and reported, never wrongly attested.
2. Exact: compute logical ancestry from the lock's dependency edges, not physical nesting. This is more work, but it doesn't over-report.
3. Accept the gap for this PR and track it in a follow-up issue.

Everything else checked out:

• issue_1965_* tests pass, and they would fail on main.
• CHANGELOG.md is untouched.
• CI is 31/31 green (19 path-skipped), and Bugbot is clean on fa843ed.

I'm not posting a review brief or arming auto-merge until this is settled.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed fa843ede4f3ab84c19f72176979069deafc5e563. Checked URL override detection, workspace-dependent names, conservative ambiguous rules and unwind ownership. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.