From 7942656ea7a4a91676498cd030db5851115de585 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 18:29:09 +0000 Subject: [PATCH 1/6] Start fix for #798 Assisted-by: Claude Code:claude-opus-5-5 From 3a470463aebb17ee3ce5dedc18c488502f463c08 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 18:41:28 +0000 Subject: [PATCH 2/6] Refuse npm VEX when the twin lock lacks the pkg 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 --- .../tests/e2e_vex_lockfile/npm.rs | 34 ++++ .../socket-patch-core/src/vex/discover/npm.rs | 145 +++++++++++++++++- docs/testing/npm-compatibility.md | 3 +- 3 files changed, 175 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs index f63340878..bb0898e99 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs @@ -361,6 +361,40 @@ fn dual_lock_with_one_lock_on_the_registry_attests_nothing() { } } +/// REGRESSION (#798): the same holds when the other lock has NO entry for +/// the package (a stale twin an npm <= 11 teammate left behind): npm +/// re-resolves the missing entry from the registry, so whichever npm major +/// reads that lock installs unpatched bytes. Nothing attests. +#[test] +fn dual_lock_with_one_lock_missing_the_package_attests_nothing() { + let api = api_for(UUID, PURL); + let stale = json!({ + "name": "app", "version": "1.0.0", "lockfileVersion": 3, "requires": true, + "packages": { "": { "name": "app", "version": "1.0.0" } }, + }); + for wired in [ + hosted_url("patch.socket.dev", UUID), + format!("file:{}", vendored_rel(UUID)), + ] { + for stale_file in ["package-lock.json", "npm-shrinkwrap.json"] { + let tmp = tempfile::tempdir().unwrap(); + let p = tmp.path(); + write_locks(p, Shape::Dual, &wired, PIN); + std::fs::write(p.join(stale_file), stale.to_string()).unwrap(); + write_artifact(p, UUID, PATCHED); + let out = run_vex(&binary(), p, &VexRun::online(&api)); + assert_ne!(out.code, Some(0), "{stale_file} {wired}:\n{out}"); + assert_absent(out.doc.as_ref(), PURL); + let text = out.stdout.clone() + &out.stderr; + assert!( + text.contains("patched_ref_unattributable") + && text.contains("has no entry for the package"), + "the contested wiring must be explained:\n{out}" + ); + } + } +} + /// Standalone `vex` over a manifest-less checkout honors the output /// conventions every `vex` run does: `-O -` prints the document to stdout /// (never a file named `-`) with the summary on stderr, and `--dry-run` diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 920932076..a56a527b9 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -72,13 +72,35 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { } /// What one parsed npm lock wires, plus the packages it resolves ELSEWHERE -/// (an entry whose `resolved` is not a Socket reference) and the packages it -/// installs BUNDLED (purl → the first bundled entry's lock location). +/// (an entry whose `resolved` is not a Socket reference), the packages it +/// installs BUNDLED (purl → the first bundled entry's lock location) and +/// every package NAME it has any entry for ([`purl_name`] keys). struct NpmLockRefs { file: &'static str, refs: Vec, unwired: BTreeSet, bundled: BTreeMap, + names: BTreeSet, +} + +impl NpmLockRefs { + /// Remember that this lock has an entry named `name` (any version). + fn mention(&mut self, name: &str) { + if let Some(purl) = npm_purl(name, "0") { + self.names.insert(purl_name(&purl).to_string()); + } + } + + /// Whether this lock has any entry for the package `purl` names. + fn mentions(&self, purl: &str) -> bool { + self.names.contains(purl_name(purl)) + } +} + +/// An npm purl without its `@version` (the version never holds an `@`; a +/// scope's `@` comes before it). +fn purl_name(purl: &str) -> &str { + purl.rsplit_once('@').map_or(purl, |(name, _)| name) } /// Push every ref no OTHER npm lock contests. npm <= 11 installs from @@ -89,9 +111,13 @@ struct NpmLockRefs { /// patched by some npm majors and unpatched by others — not decidable from /// the files, so it is diagnosed and not attested (the same call as the v2 /// legacy mirror: a lock section some npm reads must not attest bytes -/// another npm installs). The lock that does not mention the package at all -/// contests nothing. Two locks wiring DIFFERENT patches are both emitted -/// (the CLI's `wiring_conflict` gate). +/// another npm installs). A lock with NO entry for the package contests it +/// too (#798): npm re-resolves the missing entry from the registry (npm 12 +/// with a stale package-lock.json twin, npm <= 11 with a stale shrinkwrap). +/// A lock holding the package only at another version contests nothing: +/// it installs that version, not unpatched bytes of the wired one. Two +/// locks wiring DIFFERENT patches are both emitted (the CLI's +/// `wiring_conflict` gate). /// /// A bundled copy of the ref's `name@version` in either lock contests it /// too, the same lock included (#325): the rewired entry and the bundled @@ -140,6 +166,25 @@ fn push_uncontested(locks: Vec, out: &mut Discovery) { lock.file, r.purl, r.uuid, other.file, NPM_LOCKS[0], NPM_LOCKS[1], ), ); + } else if let Some(other) = locks + .iter() + .enumerate() + .find(|(j, other)| *j != i && !other.mentions(&r.purl)) + .map(|(_, other)| other) + { + out.diag( + DIAG_REF_UNATTRIBUTABLE, + lock.file, + format!( + "{}: {} is wired to Socket patch {} but {} has no entry for the \ + package, so npm re-resolves it from the registry when it installs \ + from that lock — npm <= 11 installs from {}, npm >= 12 from {}, so \ + whether the patched bytes install depends on the npm version; \ + rewire both locks (re-run `socket-patch vendor` / `scan --mode \ + hosted`) to attest it", + lock.file, r.purl, r.uuid, other.file, NPM_LOCKS[0], NPM_LOCKS[1], + ), + ); } else { out.push(r.clone()); } @@ -161,6 +206,7 @@ async fn extract_package_lock( refs: Vec::new(), unwired: BTreeSet::new(), bundled: BTreeMap::new(), + names: BTreeSet::new(), }; let doc: Value = match parse_json(file, &bytes) { Ok(doc) => doc, @@ -184,6 +230,7 @@ async fn extract_package_lock( // the same version, here ([`push_uncontested`]) and in any other lock // (the orchestrator). for (location, node) in npm_lock_bundled_nodes(&doc) { + read.mention(node.name); if let Some(purl) = node.version.and_then(|v| npm_purl(node.name, v)) { out.resolved_elsewhere(file, Some(purl.clone())); read.bundled.entry(purl).or_insert(location); @@ -232,6 +279,7 @@ fn drop_non_registry_installs( else { continue; }; + read.mention(name); out.resolved_elsewhere(file, Some(purl.clone())); read.unwired.insert(purl.clone()); unpatched.push((purl, key, reason)); @@ -264,6 +312,7 @@ fn entry_ref( out: &mut Discovery, ) { let name = node.name; + read.mention(name); let resolved = node.resolved; let Located { vendored, @@ -839,7 +888,7 @@ mod tests { /// The agreeing dual-lock state (both locks wired identically — what the /// hosted rewriter and the vendor backend now write) is a ref from each - /// lock; a lock that does not mention the package contests nothing. + /// lock. #[tokio::test] async fn agreeing_dual_npm_locks_both_wire_the_patch() { let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); @@ -858,6 +907,90 @@ mod tests { assert!(out.diagnostics.is_empty(), "{:?}", out.diagnostics); } + /// REGRESSION (#798): a twin npm lock with NO entry for the package + /// contests it too. npm 12 reads package-lock.json beside a committed + /// shrinkwrap and re-resolves the missing entry from the registry, so + /// the bytes it installs are unpatched, in either direction, hosted or + /// vendored. The uuids stay recognized (a ledger claim is dead too). + #[tokio::test] + async fn a_sibling_npm_lock_without_the_package_contests_it() { + 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}/minimist-1.2.5.tgz"); + let registry = |n: &str, v: &str| format!("https://registry.npmjs.org/{n}/-/{n}-{v}.tgz"); + let p = Project::new(); + // Shrinkwrap wires left-pad (hosted); package-lock has no left-pad. + // package-lock wires minimist (vendored); shrinkwrap has no minimist. + p.write( + "npm-shrinkwrap.json", + lock_with_packages(serde_json::json!({ + "node_modules/left-pad": { "version": "1.3.0", "resolved": hosted, "integrity": SRI }, + "node_modules/other": { "version": "1.0.0", "resolved": registry("other", "1.0.0"), "integrity": SRI }, + })), + ); + p.write( + "package-lock.json", + lock_with_packages(serde_json::json!({ + "node_modules/minimist": { "version": "1.2.5", "resolved": vendored, "integrity": SRI }, + })), + ); + let out = run(&p).await; + assert!( + out.refs.is_empty(), + "contested refs emitted: {:#?}", + out.refs + ); + let contested: Vec<&Diag> = out + .diagnostics + .iter() + .filter(|d| d.code == DIAG_REF_UNATTRIBUTABLE) + .collect(); + assert_eq!(contested.len(), 2, "{:#?}", out.diagnostics); + assert!(contested + .iter() + .any(|d| d.file == std::path::Path::new("npm-shrinkwrap.json") + && d.detail.contains("pkg:npm/left-pad@1.3.0") + && d.detail.contains("no entry") + && d.detail.contains("npm >= 12"))); + assert!(contested + .iter() + .any(|d| d.file == std::path::Path::new("package-lock.json") + && d.detail.contains("pkg:npm/minimist@1.2.5") + && d.detail.contains("no entry"))); + for (label, uuid) in [("UUID_A", UUID_A), ("UUID_B", UUID_B)] { + assert!( + out.recognized.iter().any(|r| r.uuid == uuid), + "{label} must stay recognized (authoritative, dead): {:#?}", + out.recognized + ); + } + } + + /// A twin lock that holds the package only at ANOTHER version installs + /// that version, not an unpatched copy of the wired one, so it contests + /// nothing (#798 is about a missing entry, not a different version). A + /// bundled or aliased entry of the name counts as mentioning it. + #[tokio::test] + async fn a_sibling_npm_lock_with_another_version_contests_nothing() { + let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); + let registry = |n: &str, v: &str| format!("https://registry.npmjs.org/{n}/-/{n}-{v}.tgz"); + let p = Project::new(); + p.write( + "npm-shrinkwrap.json", + lock_with_packages(serde_json::json!({ + "node_modules/left-pad": { "version": "1.3.0", "resolved": hosted, "integrity": SRI }, + })), + ); + p.write( + "package-lock.json", + lock_with_packages(serde_json::json!({ + "node_modules/left-pad": { "version": "1.2.0", "resolved": registry("left-pad", "1.2.0"), "integrity": SRI }, + })), + ); + let out = run(&p).await; + assert_eq!(out.refs.len(), 1, "{:#?}", out.diagnostics); + assert!(out.diagnostics.is_empty(), "{:?}", out.diagnostics); + } + /// REGRESSION: a v2 lock whose `packages` entry was reverted to the /// registry while its stale legacy `dependencies` mirror still carries /// the Socket url wires NOTHING — npm >= 7 installs from `packages`, so diff --git a/docs/testing/npm-compatibility.md b/docs/testing/npm-compatibility.md index e8c1a05fa..3da2b75e7 100644 --- a/docs/testing/npm-compatibility.md +++ b/docs/testing/npm-compatibility.md @@ -51,7 +51,8 @@ npm 12 notes: both present, npm 12 installs from package-lock.json while npm <= 11 installs from the shrinkwrap — so hosted and vendored rewrites wire BOTH, and manifest-less VEX refuses to attest a package one lock wires while the other - still resolves it from the registry (`patched_ref_unattributable`). + still resolves it from the registry or has no entry for it at all, which npm + re-resolves from the registry (`patched_ref_unattributable`). - `allow-file` defaults to `all`: vendored `file:` tarballs install unchanged. ## Suites From bfdc21e5a06d3e0bf4dd4ecd8b65af59c110a382 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 19:11:06 +0000 Subject: [PATCH 3/6] Make the dual-lock read test use agreeing twins 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 --- crates/socket-patch-core/src/vex/discover/npm.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index a56a527b9..f308b971d 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -778,8 +778,10 @@ mod tests { } /// Both npm locks are read (npm 12 installs from package-lock.json beside a - /// committed shrinkwrap); a v2 lock's legacy `dependencies` mirror is - /// not (its agreeing twin adds nothing). + /// committed shrinkwrap), each yielding its own refs; a v2 lock's legacy + /// `dependencies` mirror is not (its agreeing twin adds nothing). The + /// twins agree: a lock with no entry for a package would contest it + /// (#798). #[tokio::test] async fn both_locks_are_read_and_the_agreeing_mirror_adds_nothing() { let a = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); @@ -789,6 +791,7 @@ mod tests { "npm-shrinkwrap.json", lock_with_packages(serde_json::json!({ "node_modules/left-pad": { "version": "1.3.0", "resolved": a, "integrity": SRI }, + "node_modules/minimist": { "version": "1.2.5", "resolved": b, "integrity": SRI }, })), ); p.write( @@ -796,6 +799,7 @@ mod tests { serde_json::json!({ "lockfileVersion": 2, "packages": { + "node_modules/left-pad": { "version": "1.3.0", "resolved": a, "integrity": SRI }, "node_modules/minimist": { "version": "1.2.5", "resolved": b, "integrity": SRI }, }, "dependencies": { @@ -821,8 +825,9 @@ mod tests { ); assert_eq!( out.refs.len(), - 2, - "the v2 mirror and its nested legacy copy add nothing: {:#?}", + 4, + "one ref per package from each lock; the v2 mirror and its nested \ + legacy copy add nothing: {:#?}", out.refs ); } From d60490166118af195886a5b371b832a241cd8822 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 19:14:33 +0000 Subject: [PATCH 4/6] Keep patch refs out of a vex test's messages 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 --- crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs b/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs index bb0898e99..d072ef1af 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_lockfile/npm.rs @@ -372,9 +372,9 @@ fn dual_lock_with_one_lock_missing_the_package_attests_nothing() { "name": "app", "version": "1.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "app", "version": "1.0.0" } }, }); - for wired in [ - hosted_url("patch.socket.dev", UUID), - format!("file:{}", vendored_rel(UUID)), + for (mode, wired) in [ + ("hosted", hosted_url("patch.socket.dev", UUID)), + ("vendored", format!("file:{}", vendored_rel(UUID))), ] { for stale_file in ["package-lock.json", "npm-shrinkwrap.json"] { let tmp = tempfile::tempdir().unwrap(); @@ -383,7 +383,7 @@ fn dual_lock_with_one_lock_missing_the_package_attests_nothing() { std::fs::write(p.join(stale_file), stale.to_string()).unwrap(); write_artifact(p, UUID, PATCHED); let out = run_vex(&binary(), p, &VexRun::online(&api)); - assert_ne!(out.code, Some(0), "{stale_file} {wired}:\n{out}"); + assert_ne!(out.code, Some(0), "{mode}, stale {stale_file}:\n{out}"); assert_absent(out.doc.as_ref(), PURL); let text = out.stdout.clone() + &out.stderr; assert!( From 87db844e693bc3f06173798348c4801eb2553ef3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:21:57 +0000 Subject: [PATCH 5/6] Fix vex alias tests broken by store-copy merge #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 40dac077872880b6a9c22ae6bae4fcc988f617e9) --- .../src/commands/vex_consumed.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index b57d475fb..cb0c68023 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -715,8 +715,11 @@ mod tests { None, ) .await; - assert_eq!(installed_again, installed); - let (paths, calls) = tracked_npm_hosted(&common, &installed_again).await; + // Since #605 the name-keyed resolver probes bundled trees itself, so + // it already returns the aliases and the nested store's peers. Feed + // the earlier, alias-free set to keep exercising alias expansion; + // the resolver's own set is checked against the same result below. + let (paths, calls) = tracked_npm_hosted(&common, &installed).await; assert_eq!(calls.len(), 1); let mut inputs = calls[0].clone(); inputs.sort(); @@ -738,6 +741,9 @@ mod tests { .len(), paths.len() ); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed_again).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)] @@ -768,14 +774,19 @@ mod tests { None, ) .await; - assert!(installed.is_empty(), "{installed:?}"); - let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await; + // Since #605 the name-keyed resolver reaches the alias and its + // sibling peers on its own. An alias-only set (what an alias-blind + // resolver returns) must still expand to the same copies. + let (mut paths, calls) = tracked_npm_hosted(&common, &HashMap::new()).await; assert_eq!(calls, vec![vec![alias.clone()]]); let mut expected = peers; expected.push(alias); paths.sort(); expected.sort(); assert_eq!(paths, expected); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)] From 375f5768a3738a1c16ad090925efa20637292be6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 13:18:01 +0000 Subject: [PATCH 6/6] Contest a twin npm lock that lacks the wired version 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 Claude-Session: https://claude.ai/code/session_01TtZrsd52E6vxhvF9hpLVSw --- .../socket-patch-core/src/vex/discover/npm.rs | 115 +++++++++++++----- docs/testing/npm-compatibility.md | 6 +- 2 files changed, 87 insertions(+), 34 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 26e5ae198..d3769ab03 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -74,35 +74,30 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { /// What one parsed npm lock wires, plus the packages it resolves ELSEWHERE /// (an entry whose `resolved` is not a Socket reference), the packages it /// installs BUNDLED (each purl → the first such entry's lock location) and -/// every package NAME it has any entry for ([`purl_name`] keys). +/// every `name@version` it has any entry for, at any path. struct NpmLockRefs { file: &'static str, refs: Vec, unwired: BTreeMap, bundled: BTreeMap, - names: BTreeSet, + mentioned: BTreeSet, } impl NpmLockRefs { - /// Remember that this lock has an entry named `name` (any version). - fn mention(&mut self, name: &str) { - if let Some(purl) = npm_purl(name, "0") { - self.names.insert(purl_name(&purl).to_string()); + /// Remember that this lock has an entry for `name@version`. An entry + /// without a version vouches for no version. + fn mention(&mut self, name: &str, version: Option<&str>) { + if let Some(purl) = version.and_then(|v| npm_purl(name, v)) { + self.mentioned.insert(purl); } } - /// Whether this lock has any entry for the package `purl` names. + /// Whether this lock has an entry for exactly `purl`'s `name@version`. fn mentions(&self, purl: &str) -> bool { - self.names.contains(purl_name(purl)) + self.mentioned.contains(purl) } } -/// An npm purl without its `@version` (the version never holds an `@`; a -/// scope's `@` comes before it). -fn purl_name(purl: &str) -> &str { - purl.rsplit_once('@').map_or(purl, |(name, _)| name) -} - /// Push every ref no OTHER npm lock contests. npm <= 11 installs from /// npm-shrinkwrap.json when both exist; npm 12 auto-creates a /// package-lock.json beside it and installs from THAT (verified against real @@ -111,11 +106,14 @@ fn purl_name(purl: &str) -> &str { /// patched by some npm majors and unpatched by others — not decidable from /// the files, so it is diagnosed and not attested (the same call as the v2 /// legacy mirror: a lock section some npm reads must not attest bytes -/// another npm installs). A lock with NO entry for the package contests it -/// too (#798): npm re-resolves the missing entry from the registry (npm 12 -/// with a stale package-lock.json twin, npm <= 11 with a stale shrinkwrap). -/// A lock holding the package only at another version contests nothing: -/// it installs that version, not unpatched bytes of the wired one. Two +/// another npm installs). A lock with NO entry for the ref's +/// `name@version` contests it too (#798): npm re-resolves a missing entry +/// from the registry (npm 12 with a stale package-lock.json twin, npm <= 11 +/// with a stale shrinkwrap). An entry for another version holds only while +/// that version still satisfies `package.json`, which the lock alone cannot +/// tell, so a lock holding the package only at other versions (at any +/// path) contests it as well. Twins the rewriters keep in sync share their +/// `name@version` set and contest nothing. Two /// locks wiring DIFFERENT patches are both emitted (the CLI's /// `wiring_conflict` gate). /// @@ -196,8 +194,8 @@ fn push_uncontested(locks: Vec, out: &mut Discovery) { lock.file, format!( "{}: {} is wired to Socket patch {} but {} has no entry for the \ - package, so npm re-resolves it from the registry when it installs \ - from that lock — npm <= 11 installs from {}, npm >= 12 from {}, so \ + package at that version, so npm can re-resolve it from the registry \ + when it installs from that lock — npm <= 11 installs from {}, npm >= 12 from {}, so \ whether the patched bytes install depends on the npm version; \ rewire both locks (re-run `socket-patch vendor` / `scan --mode \ hosted`) to attest it", @@ -225,7 +223,7 @@ async fn extract_package_lock( refs: Vec::new(), unwired: BTreeMap::new(), bundled: BTreeMap::new(), - names: BTreeSet::new(), + mentioned: BTreeSet::new(), }; let doc: Value = match parse_json(file, &bytes) { Ok(doc) => doc, @@ -249,7 +247,7 @@ async fn extract_package_lock( // the same version, here ([`push_uncontested`]) and in any other lock // (the orchestrator). for (location, node) in npm_lock_bundled_nodes(&doc) { - read.mention(node.name); + read.mention(node.name, node.version); if let Some(purl) = node.version.and_then(|v| npm_purl(node.name, v)) { out.resolved_elsewhere(file, Some(purl.clone())); read.bundled.entry(purl).or_insert(location); @@ -298,7 +296,7 @@ fn drop_non_registry_installs( else { continue; }; - read.mention(name); + read.mentioned.insert(purl.clone()); out.resolved_elsewhere(file, Some(purl.clone())); read.unwired .entry(purl.clone()) @@ -334,7 +332,7 @@ fn entry_ref( out: &mut Discovery, ) { let name = node.name; - read.mention(name); + read.mention(name, node.version); let resolved = node.resolved; let Located { vendored, @@ -994,14 +992,64 @@ mod tests { } } - /// A twin lock that holds the package only at ANOTHER version installs - /// that version, not an unpatched copy of the wired one, so it contests - /// nothing (#798 is about a missing entry, not a different version). A - /// bundled or aliased entry of the name counts as mentioning it. + /// REGRESSION (#798 review): a sibling npm lock holding the wired + /// package only at ANOTHER version contests it as well. npm keeps that + /// entry only while it satisfies `package.json` (`^1.3.0` here rejects + /// 1.2.0), and otherwise fetches the wired version unpatched from the + /// registry. Covers a top-level and a nested wired copy. #[tokio::test] - async fn a_sibling_npm_lock_with_another_version_contests_nothing() { + async fn a_sibling_npm_lock_with_only_another_version_contests_it() { let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); let registry = |n: &str, v: &str| format!("https://registry.npmjs.org/{n}/-/{n}-{v}.tgz"); + for wired_at in [ + "node_modules/left-pad", + "node_modules/foo/node_modules/left-pad", + ] { + let p = Project::new(); + p.write( + "package.json", + serde_json::json!({ + "name": "app", "version": "1.0.0", + "dependencies": { "left-pad": "^1.3.0" }, + }) + .to_string(), + ); + p.write( + "npm-shrinkwrap.json", + lock_with_packages(serde_json::json!({ + wired_at: { "version": "1.3.0", "resolved": hosted, "integrity": SRI }, + })), + ); + p.write( + "package-lock.json", + lock_with_packages(serde_json::json!({ + "node_modules/left-pad": { "version": "1.2.0", "resolved": registry("left-pad", "1.2.0"), "integrity": SRI }, + })), + ); + let out = run(&p).await; + assert!(out.refs.is_empty(), "{wired_at}: {:#?}", out.refs); + assert_eq!( + diag_codes(&out), + vec![DIAG_REF_UNATTRIBUTABLE], + "{wired_at}: {:#?}", + out.diagnostics + ); + assert!( + out.diagnostics[0] + .detail + .contains("has no entry for the package at that version"), + "{:#?}", + out.diagnostics + ); + } + } + + /// The same `name@version` at a different path in the sibling lock + /// vouches for the wired one (the lock pair still agrees on what + /// installs). + #[tokio::test] + async fn a_sibling_npm_lock_with_the_version_at_another_path_contests_nothing() { + let hosted = hosted_url("npm", "left-pad", "1.3.0", UUID_A, "left-pad-1.3.0.tgz"); let p = Project::new(); p.write( "npm-shrinkwrap.json", @@ -1012,11 +1060,14 @@ mod tests { p.write( "package-lock.json", lock_with_packages(serde_json::json!({ - "node_modules/left-pad": { "version": "1.2.0", "resolved": registry("left-pad", "1.2.0"), "integrity": SRI }, + "node_modules/foo/node_modules/left-pad": { "version": "1.3.0", "resolved": hosted, "integrity": SRI }, })), ); let out = run(&p).await; - assert_eq!(out.refs.len(), 1, "{:#?}", out.diagnostics); + assert_refs( + &out, + &[("pkg:npm/left-pad@1.3.0", UUID_A, WiringMode::Hosted)], + ); assert!(out.diagnostics.is_empty(), "{:?}", out.diagnostics); } diff --git a/docs/testing/npm-compatibility.md b/docs/testing/npm-compatibility.md index 012a9d5a5..15bd4ce16 100644 --- a/docs/testing/npm-compatibility.md +++ b/docs/testing/npm-compatibility.md @@ -51,8 +51,10 @@ npm 12 notes: both present, npm 12 installs from package-lock.json while npm <= 11 installs from the shrinkwrap — so hosted and vendored rewrites wire BOTH, and manifest-less VEX refuses to attest a package one lock wires while the other - still resolves it from the registry or has no entry for it at all, which npm - re-resolves from the registry (`patched_ref_unattributable`). + still resolves it from the registry or has no entry for that `name@version` + (missing, or only another version that may no longer satisfy + `package.json`), which npm can re-resolve from the registry + (`patched_ref_unattributable`). - `allow-file` defaults to `all`: vendored `file:` tarballs install unchanged. ## Suites