From 7e29db4a4fcdfe93c853ffd6a75e0a7f1b0eee63 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:23:30 +0000 Subject: [PATCH 1/8] Start fix for #1965 Assisted-by: Claude Code:claude-opus-5-5 From b55c0e0e52e1ca34c876a9374254d112f2d07082 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:29:01 +0000 Subject: [PATCH 2/8] Test npm overrides that swap in a URL spec A registry range that the root package.json overrides with a remote tarball URL, git or path spec is installed by npm from that spec, not from the lock's resolved URL. These tests show the copy is not yet treated as a non-registry install, so hosted and vendored scans rewire it and VEX attests a patch that npm ci never installs (#1965). Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/npm_origin.rs | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index 717fdd1ec..b43f0f92d 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -1141,4 +1141,162 @@ mod tests { assert_eq!(split_selector("@s/pad"), ("@s/pad", None)); assert_eq!(split_selector("@s/pad@^2"), ("@s/pad", Some("^2"))); } + + /// The #1965 lock: `foo` (a `file:` tarball) depends on left-pad by a + /// registry range, and the lock records a registry-looking `resolved` + /// for it whatever the root overrides it to. + fn overridden_range_lock(left_pad_resolved: &str) -> Value { + lock(json!({ + "": { "dependencies": { "foo": "file:foo-1.0.0.tgz" } }, + "node_modules/foo": { + "version": "1.0.0", + "resolved": "file:foo-1.0.0.tgz", + "dependencies": { "left-pad": "^1.3.0" } + }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": left_pad_resolved, + "integrity": "sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==" + } + })) + } + + fn range_manifest(overrides: Value) -> NpmOverrides { + NpmOverrides::from_manifest(&json!({ + "name": "app", + "dependencies": { + "foo": "file:foo-1.0.0.tgz", + "url-pad": REGISTRY_TGZ, + "reg-pad": "1.3.0" + }, + "overrides": overrides + })) + } + + #[test] + fn issue_1965_an_override_of_a_registry_edge_to_a_url_is_non_registry() { + // Fresh lock, after a hosted redirect, and after a vendored rewire: + // npm installs the override spec, never the lock's `resolved`. + for resolved in [ + REGISTRY_TGZ, + "https://patch.socket.dev/npm/left-pad/-/left-pad-1.3.0.tgz", + "file:.socket/vendor/npm/0123/left-pad-1.3.0.tgz", + ] { + let lock = overridden_range_lock(resolved); + for overrides in [ + json!({ "left-pad": REGISTRY_TGZ }), + json!({ "left-pad": "http://example.test/left-pad-1.3.0.tgz" }), + json!({ "left-pad": { ".": REGISTRY_TGZ } }), + json!({ "left-pad": "$url-pad" }), + json!({ "foo": { "left-pad": REGISTRY_TGZ } }), + json!({ "foo@1.0.0": { "left-pad": REGISTRY_TGZ } }), + json!({ "left-pad@^1.3.0": REGISTRY_TGZ }), + json!({ "left-pad": "github:someone/left-pad" }), + json!({ "left-pad": "git+https://example.test/left-pad.git" }), + json!({ "left-pad": "file:../left-pad" }), + json!({ "left-pad": "./vendor/left-pad-1.3.0.tgz" }), + json!({ "left-pad": "npm:other@https://example.test/x.tgz" }), + ] { + let found = npm_non_registry_entries(&lock, &range_manifest(overrides.clone())); + let reason = found.get("node_modules/left-pad"); + assert!(reason.is_some(), "{overrides} / {resolved}: {found:?}"); + assert!( + reason.unwrap().contains("override"), + "{overrides}: {reason:?}" + ); + } + } + } + + #[test] + fn issue_1965_registry_and_no_op_overrides_of_a_registry_edge_stay_registry() { + let lock = overridden_range_lock(REGISTRY_TGZ); + for overrides in [ + json!({}), + json!({ "left-pad": "1.3.0" }), + json!({ "left-pad": "^1.3.0" }), + json!({ "left-pad": "latest" }), + json!({ "left-pad": "npm:left-pad@1.3.0" }), + json!({ "left-pad": "$reg-pad" }), + json!({ "left-pad": "*" }), + json!({ "left-pad": "" }), + json!({ "left-pad": { "nested": REGISTRY_TGZ } }), + // Rules that don't apply to the edge. + json!({ "right-pad": REGISTRY_TGZ }), + json!({ "other": { "left-pad": REGISTRY_TGZ } }), + json!({ "foo@2.0.0": { "left-pad": REGISTRY_TGZ } }), + // A closer no-op shadows a farther URL rule, as in npm. + json!({ "left-pad": REGISTRY_TGZ, "foo": { "left-pad": "*" } }), + json!({ "left-pad": REGISTRY_TGZ, "foo": { "left-pad": "1.3.0" } }), + // A selector for a different spec, with only registry values + // anywhere for the package. + json!({ "left-pad@1.2.0": "1.3.0" }), + ] { + let found = npm_non_registry_entries(&lock, &range_manifest(overrides.clone())); + assert!( + !found.contains_key("node_modules/left-pad"), + "{overrides}: {found:?}" + ); + // The `file:` dependent itself always counts. + assert!(found.contains_key("node_modules/foo"), "{found:?}"); + } + } + + #[test] + fn issue_1965_an_unclear_override_that_may_swap_in_a_url_keeps_the_edge() { + // npm's choice isn't modelled exactly here, and one candidate is a + // URL, so the copy is left unpatched and reported, never attested. + let lock = overridden_range_lock(REGISTRY_TGZ); + for overrides in [ + // A target selector other than the edge's own spec. + json!({ "left-pad@^1": REGISTRY_TGZ }), + json!({ "left-pad@>=1.0.0": "github:someone/left-pad" }), + // A range selector on the dependent around a URL rule. + json!({ "foo@^1": { "left-pad": REGISTRY_TGZ } }), + // Two equally close rules that disagree. + json!({ + "foo": { "left-pad": "1.3.0" }, + "foo@1.0.0": { "left-pad": REGISTRY_TGZ } + }), + // An unresolvable `$` reference beside a URL rule. + json!({ "left-pad": "$missing", "left-pad@^1": REGISTRY_TGZ }), + ] { + let found = npm_non_registry_entries(&lock, &range_manifest(overrides.clone())); + assert!( + found.contains_key("node_modules/left-pad"), + "{overrides}: {found:?}" + ); + } + } + + #[test] + fn issue_1965_a_url_override_of_a_nested_registry_edge_names_its_copy() { + // The overridden copy is nested under its dependent; only that copy + // is marked, not a hoisted registry copy of the same package. + let lock = lock(json!({ + "": { "dependencies": { "a": "^1.0.0", "left-pad": "^1.3.0" } }, + "node_modules/a": { + "version": "1.0.0", + "resolved": REGISTRY_TGZ, + "dependencies": { "left-pad": "^1.0.0" } + }, + "node_modules/a/node_modules/left-pad": { + "version": "1.3.0", + "resolved": REGISTRY_TGZ + }, + "node_modules/left-pad": { "version": "1.3.0", "resolved": REGISTRY_TGZ } + })); + let found = npm_non_registry_entries( + &lock, + &NpmOverrides::from_manifest(&json!({ + "dependencies": { "a": "^1.0.0", "left-pad": "^1.3.0" }, + "overrides": { "a": { "left-pad": REGISTRY_TGZ } } + })), + ); + assert!( + found.contains_key("node_modules/a/node_modules/left-pad"), + "{found:?}" + ); + assert!(!found.contains_key("node_modules/left-pad"), "{found:?}"); + } } From cb2c02189365419fcd639b6e3450321d502e6a27 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:31:10 +0000 Subject: [PATCH 3/8] Skip npm copies an override sends to a URL 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 URL. Hosted and vendored scans now treat such a copy like any other non-registry install: hosted skips it with redirect_npm_non_registry_entry_skipped, vendored refuses it, and VEX no longer attests a patch that npm ci never installs. When the winning override can't be decided exactly, any rule for the package with a non-registry value keeps the copy unpatched and reported, matching the module's cautious rule for unclear cases. Fixes #1965 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/npm_origin.rs | 133 ++++++++++++++---- 1 file changed, 106 insertions(+), 27 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index b43f0f92d..a8e9bf376 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -118,19 +118,75 @@ impl NpmOverrides { dep_name: &str, spec: &str, ) -> Option { + let search = self.search(packages, from, dep_name, spec)?; + if search.unclear { + return None; + } + let best = search.best.filter(|b| !b.contested)?; + effective(best.value) + } + + /// For an edge whose own `spec` is a registry spec: the non-registry + /// spec (url, git, path) an override may make npm install instead, or + /// `None` when npm installs a registry release (#1965). + /// + /// This leans the other way from [`Self::replacement`]: when the + /// winning rule can't be decided exactly, any rule for `dep_name` with + /// a non-registry value counts, so the copy is left unpatched and + /// reported rather than rewired and attested. + fn non_registry_replacement( + &self, + packages: &Map, + from: &str, + dep_name: &str, + spec: &str, + ) -> Option { + let search = self.search(packages, from, dep_name, spec)?; + let decided = match &search.best { + Some(best) => !search.unclear && !best.contested, + None => !search.unclear, + }; + if decided { + return search + .best + .and_then(|best| effective(best.value)) + .filter(|value| !npm_spec_is_registry(value)); + } + self.any_non_registry_rule(&self.rules, dep_name) + } + + /// The rules found for the edge, or `None` when there are no rules. + fn search( + &self, + packages: &Map, + from: &str, + dep_name: &str, + spec: &str, + ) -> Option { if self.rules.is_empty() { return None; } let chain = dependent_chain(packages, from); let mut search = RuleSearch::default(); self.visit(&self.rules, &chain, 0, 0, dep_name, spec, &mut search); - if search.unclear { - return None; - } - let best = search.best.filter(|b| !b.contested)?; - // npm ignores a `*` (or empty) replacement: the raw spec stays. - let value = best.value.trim(); - (!value.is_empty() && value != "*").then_some(best.value) + Some(search) + } + + /// The first non-registry value of a rule keyed by `dep_name`, at any + /// depth of `rules`, whatever its selectors. + fn any_non_registry_rule(&self, rules: &Map, dep_name: &str) -> Option { + rules.iter().find_map(|(key, value)| { + let own = (key != "." && split_selector(key).0 == dep_name) + .then(|| self.rule_value(value)) + .flatten() + .and_then(effective) + .filter(|value| !npm_spec_is_registry(value)); + own.or_else(|| { + value + .as_object() + .and_then(|children| self.any_non_registry_rule(children, dep_name)) + }) + }) } /// Search `rules` (nested `depth` levels deep; the enclosing rules @@ -224,6 +280,13 @@ impl NpmOverrides { } } +/// The spec a rule's value makes npm install: `None` for a `*` (or empty) +/// value, which npm ignores, leaving the dependent's own spec in effect. +fn effective(value: String) -> Option { + let trimmed = value.trim(); + (!trimmed.is_empty() && trimmed != "*").then_some(value) +} + /// What [`NpmOverrides::visit`] has found for one edge. #[derive(Debug, Default)] struct RuleSearch { @@ -382,30 +445,46 @@ pub(crate) fn npm_non_registry_entries( let Some(spec) = spec.as_str() else { continue; }; - if npm_spec_is_registry(spec) { - continue; - } - // An override that swaps the spec for a registry one makes - // npm install the registry release instead (#490). - if overrides - .replacement(packages, from, dep_name, spec) - .is_some_and(|replacement| npm_spec_is_registry(&replacement)) - { - continue; - } - let Some(target) = resolve_edge(packages, from, dep_name) else { - continue; + let dependent = || { + if from.is_empty() { + "the project".to_string() + } else { + format!("`{from}`") + } }; - let dependent = if from.is_empty() { - "the project".to_string() + let reason = if npm_spec_is_registry(spec) { + // An override that swaps a registry spec for a url, git + // or path one makes npm install from that spec, though + // the lock's `resolved` still looks like the registry's + // (#1965). + let Some(replacement) = + overrides.non_registry_replacement(packages, from, dep_name, spec) + else { + continue; + }; + format!( + "{} depends on it as {spec:?}, which the root package.json's \ + overrides replace with {replacement:?}, and npm installs from that spec", + dependent() + ) } else { - format!("`{from}`") - }; - out.entry(target).or_insert_with(|| { + // An override that swaps the spec for a registry one + // makes npm install the registry release instead (#490). + if overrides + .replacement(packages, from, dep_name, spec) + .is_some_and(|replacement| npm_spec_is_registry(&replacement)) + { + continue; + } format!( - "{dependent} depends on it as {spec:?}, which npm installs from that spec" + "{} depends on it as {spec:?}, which npm installs from that spec", + dependent() ) - }); + }; + let Some(target) = resolve_edge(packages, from, dep_name) else { + continue; + }; + out.entry(target).or_insert(reason); } } } From 3e024542d58a522cdd38d44f48576f898a694563 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:37:26 +0000 Subject: [PATCH 4/8] Keep URL-overridden npm copies unwindable Test the hosted rewriter, the vendored lock writer and VEX lockfile discovery against a registry range that the root overrides send to a remote tarball URL: hosted skips it loudly, vendored refuses it, and VEX attests nothing (#1965). A pin an older release wrote at such a copy is still the rewriters' own output, so VEX discovery keeps it shadowed and rollback and remove can still unwind it. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/mod.rs | 81 +++++++++++++++++++ .../socket-patch-core/src/vendor/npm_lock.rs | 50 ++++++++++++ .../src/vendor/npm_origin.rs | 52 ++++++++++-- .../socket-patch-core/src/vex/discover/npm.rs | 81 ++++++++++++++++++- 4 files changed, 256 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ccb8bb226..7f8072b72 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -20871,6 +20871,87 @@ mod tests { assert!(!r.files.contains_key("package.json")); } + /// #1965: `foo` depends on left-pad by a registry range and the root + /// `overrides` swap it for a remote tarball url, so npm installs that + /// url whatever the lock's (registry-looking) `resolved` says. The + /// rewriter must skip the entry loudly and keep it out of the VEX. + #[test] + fn issue_1965_a_registry_edge_overridden_to_a_url_is_unredirected() { + let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"; + let lock = json!({ + "name": "app", + "lockfileVersion": 3, + "packages": { + "": { "name": "app", "version": "0.0.0", "dependencies": { "foo": "file:foo-1.0.0.tgz" } }, + "node_modules/foo": { + "version": "1.0.0", + "resolved": "file:foo-1.0.0.tgz", + "dependencies": { "left-pad": "^1.3.0" } + }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": url, + "integrity": "sha512-UPSTREAM==" + } + } + }); + let overrides = vec![npm_override( + "left-pad", + "1.3.0", + "http://patch.test/lp.tgz", + "sha512-PATCHED==", + )]; + let files_with = |manifest_overrides: Value| { + let mut files = BTreeMap::new(); + files.insert( + "package-lock.json".to_string(), + serde_json::to_string_pretty(&lock).unwrap(), + ); + files.insert( + "package.json".to_string(), + json!({ + "name": "app", + "dependencies": { "foo": "file:foo-1.0.0.tgz" }, + "overrides": manifest_overrides + }) + .to_string(), + ); + files + }; + for manifest_overrides in [ + json!({ "left-pad": url }), + json!({ "foo": { "left-pad": url } }), + json!({ "left-pad": "github:someone/left-pad" }), + ] { + let r = rewrite_registry_redirect(&files_with(manifest_overrides.clone()), &overrides); + assert!(r.files.is_empty(), "{manifest_overrides}: {:?}", r.edits); + let skipped = r + .warnings + .iter() + .find(|w| w.code == "redirect_npm_non_registry_entry_skipped") + .unwrap_or_else(|| panic!("{manifest_overrides}: {:?}", r.warnings)); + assert!( + skipped.detail.contains("overrides"), + "the warning names the override: {}", + skipped.detail + ); + assert!( + r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid), + "{manifest_overrides}: the skipped copy keeps the patch out of the in-run VEX" + ); + } + // Control: a registry override (or none) still redirects. + for manifest_overrides in [json!({}), json!({ "left-pad": "1.3.0" })] { + let r = rewrite_registry_redirect(&files_with(manifest_overrides.clone()), &overrides); + let rewritten: Value = serde_json::from_str(&r.files["package-lock.json"]).unwrap(); + assert_eq!( + rewritten["packages"]["node_modules/left-pad"]["resolved"], + "http://patch.test/lp.tgz", + "{manifest_overrides}" + ); + } + } + #[test] fn npm_non_registry_entries_are_skipped_with_loud_warning() { let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"; diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 3d670a66b..bc83ffbfa 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -2696,6 +2696,56 @@ mod tests { ); } + /// #1965: a registry range the root `overrides` swap for a remote + /// tarball url is installed from that url by `npm ci`, so vendoring + /// the copy would report `applied` over unpatched bytes. With no other + /// copy the vendor refuses and writes nothing. + #[tokio::test] + async fn issue_1965_a_registry_edge_overridden_to_a_url_is_refused() { + let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"; + let lock = json!({ + "name": "fixture", + "version": "1.0.0", + "lockfileVersion": 3, + "packages": { + "": { "name": "fixture", "version": "1.0.0", + "dependencies": { "foo": "file:foo-1.0.0.tgz" } }, + "node_modules/foo": { + "version": "1.0.0", + "resolved": "file:foo-1.0.0.tgz", + "dependencies": { "left-pad": "^1.3.0" } + }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": url, + "integrity": "sha512-orig==" + } + } + }); + let fx = fixture_with("left-pad", "1.3.0", lock.clone()).await; + tokio::fs::write( + fx.root().join("package.json"), + format!( + r#"{{"name":"fixture","version":"1.0.0","dependencies":{{"foo":"file:foo-1.0.0.tgz"}},"overrides":{{"left-pad":"{url}"}}}}"# + ), + ) + .await + .unwrap(); + expect_refused(fx.vendor(false).await, "vendor_lock_entry_not_rewritable"); + assert_eq!(fx.read_lock().await, lock, "nothing is written"); + + // Control: a registry override is vendored. + tokio::fs::write( + fx.root().join("package.json"), + r#"{"name":"fixture","version":"1.0.0","dependencies":{"foo":"file:foo-1.0.0.tgz"},"overrides":{"left-pad":"1.3.0"}}"#, + ) + .await + .unwrap(); + let (result, entry, _warnings) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + assert!(entry.is_some()); + } + #[tokio::test] async fn non_registry_only_instances_refuse_and_write_nothing() { let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"; diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index a8e9bf376..d905000d1 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -422,9 +422,50 @@ pub(crate) fn npm_non_registry_entries( lock: &Value, overrides: &NpmOverrides, ) -> BTreeMap { - let mut out = BTreeMap::new(); + non_registry_entries(lock, overrides) + .into_iter() + .map(|(key, found)| (key, found.reason)) + .collect() +} + +/// The [`npm_non_registry_entries`] keys whose only evidence is an +/// `overrides` rule that swaps a registry spec for a url, git or path one +/// (#1965). Their `resolved` is a registry-style tarball url, so a Socket +/// rewriter from before #1965 may have wired them: unlike the other +/// non-registry entries, a Socket ref there is the rewriters' own output, +/// which rollback and remove must still unwind. +pub(crate) fn npm_override_only_entries( + lock: &Value, + overrides: &NpmOverrides, +) -> std::collections::BTreeSet { + non_registry_entries(lock, overrides) + .into_iter() + .filter(|(_, found)| found.override_only) + .map(|(key, _)| key) + .collect() +} + +/// Why one `packages` entry is a non-registry install. +struct NonRegistry { + /// The first reason found (for the skip warnings). + reason: String, + /// Every piece of evidence is a registry spec that `overrides` swap + /// for a non-registry one. + override_only: bool, +} + +fn non_registry_entries(lock: &Value, overrides: &NpmOverrides) -> BTreeMap { + let mut out: BTreeMap = BTreeMap::new(); + let mut note = |key: String, reason: String, override_only: bool| { + out.entry(key) + .and_modify(|found| found.override_only &= override_only) + .or_insert(NonRegistry { + reason, + override_only, + }); + }; let Some(packages) = lock.get("packages").and_then(Value::as_object) else { - return out; + return BTreeMap::new(); }; for (key, entry) in packages { if !key.contains("node_modules/") { @@ -432,7 +473,7 @@ pub(crate) fn npm_non_registry_entries( } if let Some(resolved) = entry.get("resolved").and_then(Value::as_str) { if resolved_is_non_registry(resolved) { - out.insert(key.clone(), format!("it resolves to {resolved:?}")); + note(key.clone(), format!("it resolves to {resolved:?}"), false); } } } @@ -452,7 +493,8 @@ pub(crate) fn npm_non_registry_entries( format!("`{from}`") } }; - let reason = if npm_spec_is_registry(spec) { + let override_only = npm_spec_is_registry(spec); + let reason = if override_only { // An override that swaps a registry spec for a url, git // or path one makes npm install from that spec, though // the lock's `resolved` still looks like the registry's @@ -484,7 +526,7 @@ pub(crate) fn npm_non_registry_entries( let Some(target) = resolve_edge(packages, from, dep_name) else { continue; }; - out.entry(target).or_insert(reason); + note(target, reason, override_only); } } } diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 3193d7f80..b91ac41be 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -70,7 +70,8 @@ use crate::vendor::lock_inventory::{ NpmLockNode, }; use crate::vendor::npm_origin::{ - npm_link_source_identity, npm_non_registry_entries, npm_shrinkwrapped_entries, NpmOverrides, + npm_link_source_identity, npm_non_registry_entries, npm_override_only_entries, + npm_shrinkwrapped_entries, NpmOverrides, }; pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { @@ -436,8 +437,16 @@ fn drop_non_registry_installs( out: &mut Discovery, ) { let non_registry = npm_non_registry_entries(doc, overrides); - // Locations a wiring of which is not the rewriters' own output. - let foreign: BTreeSet = non_registry.keys().cloned().collect(); + // Locations a wiring of which is not the rewriters' own output. A copy + // flagged only because `overrides` swap its registry spec for a url + // (#1965) has a registry-style `resolved` that a pre-#1965 run may + // have wired, so a ref there is still ours to unwind. + let override_only = npm_override_only_entries(doc, overrides); + let foreign: BTreeSet = non_registry + .keys() + .filter(|key| !override_only.contains(*key)) + .cloned() + .collect(); // (lock key, why that copy installs from elsewhere) let elsewhere: Vec<(String, String)> = non_registry .into_iter() @@ -2435,6 +2444,72 @@ mod tests { } } + /// #1965: a registry range the root `overrides` swap for a remote + /// tarball url is installed from that url, so a copy a pre-#1965 run + /// wired is not attested. The wiring is still the rewriters' own + /// output, so it stays shadowed for rollback / remove to unwind. + #[tokio::test] + async fn issue_1965_a_registry_edge_overridden_to_a_url_is_not_attested() { + let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let vendored = format!("file:.socket/vendor/npm/{UUID_B}/left-pad-1.3.0.tgz"); + let tarball = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"; + for (wiring, resolved) in [("hosted", &hosted), ("vendored", &vendored)] { + for (overrides, attested) in [ + (None, true), + (Some(r#"{"left-pad":"1.3.0"}"#.to_string()), true), + (Some(format!(r#"{{"left-pad":"{tarball}"}}"#)), false), + ( + Some(format!(r#"{{"foo":{{"left-pad":"{tarball}"}}}}"#)), + false, + ), + ] { + let p = Project::new(); + p.write( + "package-lock.json", + lock_with_packages(serde_json::json!({ + "": { "name": "app", "version": "1.0.0", + "dependencies": { "foo": "file:foo-1.0.0.tgz" } }, + "node_modules/foo": { + "version": "1.0.0", + "resolved": "file:foo-1.0.0.tgz", + "dependencies": { "left-pad": "^1.3.0" } + }, + "node_modules/left-pad": { + "version": "1.3.0", "resolved": resolved, "integrity": SRI + }, + })), + ); + if let Some(overrides) = &overrides { + p.write( + "package.json", + format!( + r#"{{"name":"app","dependencies":{{"foo":"file:foo-1.0.0.tgz"}},"overrides":{overrides}}}"# + ), + ); + } + let out = run(&p).await; + assert_eq!( + out.refs.len(), + usize::from(attested), + "{wiring} / {overrides:?}: {:#?}", + out.diagnostics + ); + if !attested { + assert!( + diag_codes(&out).contains(&DIAG_REF_UNATTRIBUTABLE), + "{wiring} / {overrides:?}: {:?}", + diag_codes(&out) + ); + assert_eq!( + out.shadowed.len(), + 1, + "{wiring} / {overrides:?}: the old wiring stays unwindable" + ); + } + } + } + } + /// #326: a Socket-wired entry npm installs from a git / url / `file:` /// spec (a lock rewired before the rewriters refused these, or by /// hand) wires nothing, and neither does a wired registry copy while a From 2c768cc6ff68b70a6451dd66981d04deb63d033a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:45:42 +0000 Subject: [PATCH 5/8] E2E: hosted npm skips a URL-overridden copy Real npm installs a local foo that depends on left-pad ^1.3.0 while the root overrides send left-pad to a remote tarball URL. The hosted scan now skips that copy with redirect_npm_non_registry_entry_skipped, leaves the lock byte-identical and attests nothing (#1965). Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_npm_build.rs | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs index 9529ae9f5..bae0de526 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs @@ -1416,3 +1416,217 @@ async fn npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link() { ); } } + +/// #1965: a local `foo` depends on left-pad by a registry range, and the +/// project's `overrides` replace it with a remote tarball url. npm records +/// that copy with a registry-looking `resolved`, but installs it from the +/// override spec, so a hosted pin would never install. The hosted scan +/// must skip it loudly (`redirect_npm_non_registry_entry_skipped`), leave +/// the lock byte-identical, and attest nothing in the in-run VEX. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"] +async fn npm_redirect_skips_a_registry_edge_overridden_to_a_url() { + let suite = "e2e_redirect_npm_build (overridden-to-url)"; + if npm_e2e_common::npm_major().is_none() { + npm_e2e_common::skip(suite, "`npm` not installed"); + return; + } + if !npm_supports_overrides() { + println!("N/A {suite}: this npm predates `overrides` (npm 8.3)"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + let foo = tmp.path().join("foo"); + let cache = tmp.path().join("npm-cache"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::create_dir_all(&foo).unwrap(); + std::fs::write( + foo.join("package.json"), + format!( + r#"{{"name":"foo","version":"1.0.0","dependencies":{{"{DEP}":"^{DEP_VERSION}"}}}}"# + ), + ) + .unwrap(); + std::fs::write( + foo.join("index.js"), + format!("module.exports = require('{DEP}');\n"), + ) + .unwrap(); + let packed = npm_e2e_common::npm(&foo, &["pack", "--cache", cache.to_str().unwrap()]); + assert!( + packed.status.success(), + "npm pack foo: {}", + npm_e2e_common::output_text(&packed) + ); + std::fs::rename(foo.join("foo-1.0.0.tgz"), proj.join("foo-1.0.0.tgz")).unwrap(); + let tarball = format!("https://registry.npmjs.org/{DEP}/-/{DEP}-{DEP_VERSION}.tgz"); + std::fs::write( + proj.join("package.json"), + format!( + r#"{{"name":"app","version":"1.0.0","private":true,"dependencies":{{"foo":"file:foo-1.0.0.tgz"}},"overrides":{{"{DEP}":"{tarball}"}}}}"# + ), + ) + .unwrap(); + // npm >= 12 refuses a url override unless remote installs are allowed. + std::fs::write(proj.join(".npmrc"), "allow-remote=all\n").unwrap(); + let out = npm_e2e_common::npm( + &proj, + &[ + "install", + "--no-audit", + "--no-fund", + "--cache", + cache.to_str().unwrap(), + ], + ); + if !out.status.success() { + npm_e2e_common::skip( + suite, + &format!( + "`npm install` of the overrides fixture failed (registry unreachable?):\n{}", + npm_e2e_common::output_text(&out) + ), + ); + return; + } + // The #1965 premise: the copy's `resolved` looks like a registry entry, + // and `foo`'s entry keeps its registry range. + let lock_before = std::fs::read(proj.join("package-lock.json")).unwrap(); + let lock: serde_json::Value = serde_json::from_slice(&lock_before).unwrap(); + assert_eq!( + lock["packages"][format!("node_modules/{DEP}")]["resolved"], + tarball.as_str(), + "{lock}" + ); + assert_eq!( + lock["packages"]["node_modules/foo"]["dependencies"][DEP], + format!("^{DEP_VERSION}"), + "{lock}" + ); + + let server = MockServer::start().await; + let hosted_url = format!( + "{}/patch/npm/{DEP}/{DEP_VERSION}/{TOKEN}/{UUID}/{DEP}-{DEP_VERSION}.tgz", + server.uri() + ); + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": [{ + "purl": PURL, + "patches": [{ + "uuid": UUID, "purl": PURL, "tier": "free", + "cveIds": [], "ghsaIds": [], "severity": "high", + "title": "overridden-to-url fixture" + }] + }], + "canAccessPaidPatches": false, + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex(format!( + "^/v0/orgs/{ORG}/patches/by-package/.+$" + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "patches": [{ + "uuid": UUID, "purl": PURL, + "publishedAt": "2026-01-01T00:00:00Z", + "description": "x", "license": "MIT", "tier": "free", + "vulnerabilities": {} + }], + "canAccessPaidPatches": false, + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/package"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "results": { + UUID: { + "status": "granted", + "url": hosted_url, + "purl": PURL, + "artifacts": [{ + "kind": "tarball", + "url": hosted_url, + "integrity": { "sha512": "sha512-UEFUQ0hFRA==" } + }], + "registryOverride": null + } + } + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(format!("/v0/orgs/{ORG}/patches/view/{UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "uuid": UUID, + "purl": PURL, + "publishedAt": "2026-01-01T00:00:00Z", + "files": {}, + "vulnerabilities": { + GHSA: { + "cves": ["CVE-2026-1111"], + "summary": "overridden-to-url vuln", + "severity": "high", + "description": "d" + } + }, + "description": "x", "license": "MIT", "tier": "free" + }))) + .mount(&server) + .await; + + let uri = server.uri(); + let argv = [ + "scan", + "--mode=hosted", + "--json", + "--yes", + "--cwd", + proj.to_str().unwrap(), + "--api-url", + &uri, + "--org", + ORG, + "--api-token", + "fake", + "--vex", + "out.vex.json", + "--vex-product", + PRODUCT, + ]; + let (_code, stdout, stderr) = run_socket(&proj, &argv); + let env: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("not JSON ({e}):\n{stdout}\nstderr:\n{stderr}")); + let skipped = env["warnings"] + .as_array() + .and_then(|w| { + w.iter() + .find(|w| w["code"] == "redirect_npm_non_registry_entry_skipped") + }) + .unwrap_or_else(|| panic!("the overridden copy must be skipped loudly: {env}")); + assert!( + skipped["detail"] + .as_str() + .is_some_and(|d| d.contains("overrides")), + "the skip names the override: {skipped}" + ); + assert_eq!( + std::fs::read(proj.join("package-lock.json")).unwrap(), + lock_before, + "the lock is left byte-identical: {env}" + ); + let vex_path = proj.join("out.vex.json"); + if vex_path.exists() { + let vex: serde_json::Value = + serde_json::from_slice(&std::fs::read(&vex_path).unwrap()).unwrap(); + let not_affected = vex["statements"].as_array().is_some_and(|s| { + s.iter() + .any(|s| s["status"].as_str() == Some("not_affected")) + }); + assert!(!not_affected, "nothing is attested not_affected: {vex}"); + } +} From 70d9aff4078e362806a728040b1aaed9e08b47ae Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 02:55:06 +0000 Subject: [PATCH 6/8] Look up npm URL overrides only when one exists Settle registry edges whose package has no non-registry override rule before walking the dependent's chain, so large locks with an unrelated overrides block don't pay a per-edge search. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vendor/npm_origin.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index d905000d1..e8af8869f 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -141,6 +141,10 @@ impl NpmOverrides { dep_name: &str, spec: &str, ) -> Option { + // Nearly every edge is a registry edge, so settle the common case + // (no rule for the package can name a non-registry spec) without + // walking the dependent's chain. + let candidate = self.any_non_registry_rule(&self.rules, dep_name)?; let search = self.search(packages, from, dep_name, spec)?; let decided = match &search.best { Some(best) => !search.unclear && !best.contested, @@ -152,7 +156,7 @@ impl NpmOverrides { .and_then(|best| effective(best.value)) .filter(|value| !npm_spec_is_registry(value)); } - self.any_non_registry_rule(&self.rules, dep_name) + Some(candidate) } /// The rules found for the edge, or `None` when there are no rules. From 2df1c470dde7e02d624794183092f7699349bb29 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 03:29:40 +0000 Subject: [PATCH 7/8] Match npm overrides scoped to a workspace member npm reaches a workspace member through the root's link edge, so an override keyed by the member's name applies to its dependencies. The override lookup now starts a dependent's ancestor chain with its workspace member, so a URL override scoped to a member skips that member's copy instead of rewiring and attesting it. Found in review. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/npm_origin.rs | 91 ++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index e8af8869f..1bbd21c6d 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -370,12 +370,20 @@ fn split_selector(key: &str) -> (&str, Option<&str>) { /// The packages on the path from the project root to the lock key `from`, /// outermost first, as `(name, version)`: each `node_modules/` -/// segment's entry (its `name` field for an alias, else the segment). The -/// root and workspace members contribute nothing. +/// segment's entry (its `name` field for an alias, else the segment). A +/// dependent inside a workspace member starts with that member, which npm +/// reaches through the root's link edge, so a rule keyed by the member's +/// name scopes to its dependencies (#1965). The root contributes nothing. fn dependent_chain(packages: &Map, from: &str) -> Vec<(String, Option)> { let mut chain = Vec::new(); let mut rest = from; let mut prefix = String::new(); + if !from.is_empty() && !from.starts_with("node_modules/") { + let member = from.split("/node_modules/").next().unwrap_or(from); + chain.push(workspace_member(packages, member)); + prefix.push_str(member); + rest = &from[member.len()..]; + } while let Some(ix) = rest.find("node_modules/") { let after = &rest[ix + "node_modules/".len()..]; // A package segment runs to the next nested `node_modules/`. @@ -399,6 +407,33 @@ fn dependent_chain(packages: &Map, from: &str) -> Vec<(String, Op chain } +/// The `(name, version)` of the workspace member at lock key `member`: its +/// `name` field, else the name of the root link entry that points at it +/// (npm omits `name` when it matches), else its folder name. +fn workspace_member(packages: &Map, member: &str) -> (String, Option) { + let entry = packages.get(member); + let version = entry + .and_then(|e| e.get("version")) + .and_then(Value::as_str) + .map(str::to_string); + let name = entry + .and_then(|e| e.get("name")) + .and_then(Value::as_str) + .map(str::to_string) + .or_else(|| { + packages.iter().find_map(|(key, link)| { + let linked = link.get("link").and_then(Value::as_bool) == Some(true) + && link.get("resolved").and_then(Value::as_str) == Some(member); + linked + .then(|| key.rsplit_once("node_modules/").map(|(_, name)| name)) + .flatten() + .map(str::to_string) + }) + }) + .unwrap_or_else(|| member.rsplit('/').next().unwrap_or(member).to_string()); + (name, version) +} + /// The dependency maps whose specs npm resolves against `packages` entries. const EDGE_FIELDS: [&str; 4] = [ "dependencies", @@ -1394,6 +1429,58 @@ mod tests { } } + #[test] + fn issue_1965_a_url_override_scoped_to_a_workspace_member_is_non_registry() { + // The Bugbot probe: npm reaches a workspace member through the + // root's link edge, so a rule keyed by the member's name scopes to + // its dependencies, hoisted or nested. Members are found by their + // `name` field, their link entry's key, or their folder name. + let lock_with = |member: Value| { + lock(json!({ + "": { "name": "root", "workspaces": ["packages/*"] }, + "packages/app": member, + "node_modules/app": { "resolved": "packages/app", "link": true }, + "packages/app/node_modules/left-pad": { + "version": "1.3.0", + "resolved": REGISTRY_TGZ + }, + "node_modules/other": { + "version": "1.0.0", + "resolved": REGISTRY_TGZ, + "dependencies": { "left-pad": "^1.0.0" } + }, + "node_modules/left-pad": { "version": "1.3.0", "resolved": REGISTRY_TGZ } + })) + }; + let deps = json!({ "left-pad": "^1.3.0" }); + for member in [ + json!({ "name": "app", "version": "2.0.0", "dependencies": deps }), + json!({ "version": "2.0.0", "dependencies": deps }), + ] { + let lock = lock_with(member.clone()); + for (scope, flagged) in [ + ("app", true), + ("app@2.0.0", true), + ("app@3.0.0", false), + ("other-member", false), + ] { + let found = npm_non_registry_entries( + &lock, + &NpmOverrides::from_manifest(&json!({ + "overrides": { scope: { "left-pad": REGISTRY_TGZ } } + })), + ); + assert_eq!( + found.contains_key("packages/app/node_modules/left-pad"), + flagged, + "{scope} / {member}: {found:?}" + ); + // `other`'s hoisted copy is outside the member's subtree. + assert!(!found.contains_key("node_modules/left-pad"), "{found:?}"); + } + } + } + #[test] fn issue_1965_a_url_override_of_a_nested_registry_edge_names_its_copy() { // The overridden copy is nested under its dependent; only that copy From fa843ede4f3ab84c19f72176979069deafc5e563 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 03:45:40 +0000 Subject: [PATCH 8/8] Name npm workspace members as npm does A member without a name field keeps its @scope/ folder parent, and only the root-level link entry names a member: a nested link to the same folder may be an alias. Member-scoped overrides now match the member's real name. Found in review. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/npm_origin.rs | 84 +++++++++++++++---- 1 file changed, 69 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index 1bbd21c6d..058d43d85 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -408,8 +408,9 @@ fn dependent_chain(packages: &Map, from: &str) -> Vec<(String, Op } /// The `(name, version)` of the workspace member at lock key `member`: its -/// `name` field, else the name of the root link entry that points at it -/// (npm omits `name` when it matches), else its folder name. +/// `name` field, else the name of the root-level link entry that points at +/// it (npm omits `name` when it matches; a nested link may be an alias), +/// else its folder name, keeping an `@scope/` parent as npm does. fn workspace_member(packages: &Map, member: &str) -> (String, Option) { let entry = packages.get(member); let version = entry @@ -422,15 +423,14 @@ fn workspace_member(packages: &Map, member: &str) -> (String, Opt .map(str::to_string) .or_else(|| { packages.iter().find_map(|(key, link)| { + let name = key.strip_prefix("node_modules/")?; + let root_level = !name.contains("/node_modules/"); let linked = link.get("link").and_then(Value::as_bool) == Some(true) && link.get("resolved").and_then(Value::as_str) == Some(member); - linked - .then(|| key.rsplit_once("node_modules/").map(|(_, name)| name)) - .flatten() - .map(str::to_string) + (root_level && linked).then(|| name.to_string()) }) }) - .unwrap_or_else(|| member.rsplit('/').next().unwrap_or(member).to_string()); + .unwrap_or_else(|| name_from_folder(member)); (name, version) } @@ -583,18 +583,22 @@ pub(crate) fn npm_link_source_identity(lock: &Value, link: &Value) -> Option<(St let version = source.get("version")?.as_str()?; let name = match source.get("name").and_then(Value::as_str) { Some(name) => name.to_string(), - None => { - let mut segments = resolved.rsplit('/'); - let base = segments.next().unwrap_or(resolved); - match segments.next() { - Some(scope) if scope.starts_with('@') => format!("{scope}/{base}"), - _ => base.to_string(), - } - } + None => name_from_folder(resolved), }; Some((name, version.to_string())) } +/// A package's name from its folder path, as npm's `nameFromFolder` reads +/// it: the last segment, keeping an `@scope/` parent. +fn name_from_folder(path: &str) -> String { + let mut segments = path.rsplit('/'); + let base = segments.next().unwrap_or(path); + match segments.next() { + Some(scope) if scope.starts_with('@') => format!("{scope}/{base}"), + _ => base.to_string(), + } +} + /// Every `packages` key installed beneath a package that ships its own /// `npm-shrinkwrap.json` (the lock marks that ancestor `"hasShrinkwrap": /// true`, e.g. `firebase-tools`, `netlify-cli`), mapped to the outermost @@ -1481,6 +1485,56 @@ mod tests { } } + #[test] + fn issue_1965_a_workspace_member_is_named_as_npm_names_it() { + // The second Bugbot probe: a scoped member without a `name` field + // keeps its `@scope/` folder, and the root workspace link names it + // even when a nested alias link to the same folder sorts first. + let left_pad = json!({ "version": "1.3.0", "resolved": REGISTRY_TGZ }); + let deps = json!({ "left-pad": "^1.3.0" }); + let scoped = lock(json!({ + "": { "workspaces": ["packages/*"] }, + "packages/@s/app": { "version": "1.0.0", "dependencies": deps }, + "packages/@s/app/node_modules/left-pad": left_pad, + })); + let aliased = lock(json!({ + "": { "workspaces": ["packages/*"] }, + "node_modules/a/node_modules/alias": { "resolved": "packages/app", "link": true }, + "node_modules/app": { "resolved": "packages/app", "link": true }, + "packages/app": { "version": "1.0.0", "dependencies": deps }, + "packages/app/node_modules/left-pad": left_pad, + })); + for (lock, key, scope, flagged) in [ + ( + &scoped, + "packages/@s/app/node_modules/left-pad", + "@s/app", + true, + ), + ( + &scoped, + "packages/@s/app/node_modules/left-pad", + "app", + false, + ), + (&aliased, "packages/app/node_modules/left-pad", "app", true), + ( + &aliased, + "packages/app/node_modules/left-pad", + "alias", + false, + ), + ] { + let found = npm_non_registry_entries( + lock, + &NpmOverrides::from_manifest(&json!({ + "overrides": { scope: { "left-pad": REGISTRY_TGZ } } + })), + ); + assert_eq!(found.contains_key(key), flagged, "{scope}: {found:?}"); + } + } + #[test] fn issue_1965_a_url_override_of_a_nested_registry_edge_names_its_copy() { // The overridden copy is nested under its dependent; only that copy