From 790920a9ddf2f232aa7e430f69968b2042a30ec0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:24:05 +0000 Subject: [PATCH 1/4] Start fix for #1494 Assisted-by: Claude Code:claude-opus-5-5 From 0a81315b84ffdc51a9e092696623cf9a651e9dd0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:33:20 +0000 Subject: [PATCH 2/4] Patch npm alias links from a workspace member Run from inside a vlt, pnpm or npm-linked workspace member, apply reported a member's npm alias dependency ("lp": "npm:left-pad@1.3.0") as package_not_installed and exited 1, and rollback could not restore it, although scan had offered the patch. The member's node_modules is the only root walked there, and its alias entry is only a link into the root store, which the resolver never takes as a copy. Apply's and rollback's lookups now add the root links whose target is a requested package, the same pass path scoping got in #1470. Both commands already act on each physical copy once, so a run from the root still patches the store copy a single time. Fixes #1494 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/ecosystem_dispatch.rs | 105 ++++++++++++---- .../tests/apply/in_process_npm_multicopy.rs | 119 ++++++++++++++++++ 2 files changed, 202 insertions(+), 22 deletions(-) diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index d516c0341..878dcd25d 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -619,8 +619,9 @@ pub async fn find_all_packages_for_purls( // `merge_qualified`'s `push_path`. Cargo keeps every registry index // copy (`merge_cargo_copies`); the single-copy ecosystems keep true // first-wins via their own `merge_first_wins` wiring in - // `dispatch_find`. - dispatch_find(partitioned, options, silent, merge_variant_copies, None).await + // `dispatch_find`. A workspace member's npm alias link is a copy too + // (#1494); apply collapses it onto the store copy it names. + dispatch_find_with_npm_links(partitioned, options, silent, merge_variant_copies, None).await } /// Multi-copy variant of `find_packages_for_rollback` (qualified-aware @@ -634,41 +635,53 @@ pub async fn find_all_packages_for_rollback( } /// [`find_all_packages_for_rollback`], taking the npm `node_modules` roots -/// from `prior` as [`find_packages_for_rollback_reusing`] does. +/// from `prior` as [`find_packages_for_rollback_reusing`] does. A workspace +/// member's npm alias link is a copy too (#1494), as for apply. pub async fn find_all_packages_for_rollback_reusing( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - let npm_roots = prior - .filter(|p| p.taken_with(options)) - .map(|p| p.roots.as_slice()); - dispatch_find(partitioned, options, silent, merge_qualified, npm_roots).await + dispatch_find_with_npm_links(partitioned, options, silent, merge_qualified, prior).await } /// The install paths that reach each purl, for `scan`'s and `rollback`'s -/// path targets: [`find_all_packages_for_rollback_reusing`]'s physical -/// copies, plus each npm package link directly in a `node_modules` root -/// (scoped ones one level down) whose target's `package.json` names one -/// of the npm purls asked for. +/// path targets: [`find_all_packages_for_rollback_reusing`]'s copies, link +/// spellings included. The result is for path matching only: it can name +/// one physical copy more than once. +pub async fn find_package_paths_for_path_scope( + partitioned: &HashMap>, + options: &CrawlerOptions, + silent: bool, + prior: Option<&NpmCrawlSnapshot>, +) -> HashMap> { + dispatch_find_with_npm_links(partitioned, options, silent, merge_qualified, prior).await +} + +/// [`dispatch_find`]'s copies, plus each npm package link directly in a +/// `node_modules` root (scoped ones one level down) whose target's +/// `package.json` names one of the npm purls asked for, after the copies. /// /// The crawler records a plain-name link at its own path /// (`packages/a/node_modules/ms`), but never an alias link /// (`packages/a/node_modules/lp -> …/.pnpm/left-pad@1.3.0/node_modules/left-pad`): -/// links are not physical copies, and apply and VEX must see each copy -/// once (#856). Path scoping asks which paths install the package -/// instead, and pnpm's isolated linker or npm's `install-strategy=linked` -/// reach a workspace member's ALIAS dependency only through such a link -/// (#1266). A link is matched by its target's own identity, not against -/// the crawled copies, so a link to a store peer variant the crawl -/// skipped still counts, and the cost is one pass over the root links -/// however many purls are asked for. The result is for path matching -/// only: it can name one physical copy more than once. -pub async fn find_package_paths_for_path_scope( +/// links are not physical copies, and the resolver must see each copy +/// once (#856). Yet pnpm's isolated linker, vlt and npm's +/// `install-strategy=linked` reach a workspace member's ALIAS dependency +/// only through such a link. Path scoping needs it to select the member +/// (#1266), and a run from inside the member, whose `node_modules` is the +/// only root, has no other route to the copy at all (#1494). A link is +/// matched by its target's own identity, not against the crawled copies, +/// so a link to a store peer variant the crawl skipped still counts, and +/// the cost is one pass over the root links however many purls are asked +/// for. A link can name a copy the resolver also found: apply and +/// rollback act on each physical copy once ([`distinct_npm_copies`]). +async fn dispatch_find_with_npm_links( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, + variant_merge: MergeFn, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { let mut wanted: HashMap> = HashMap::new(); @@ -687,7 +700,7 @@ pub async fn find_package_paths_for_path_scope( partitioned, options, silent, - merge_qualified, + variant_merge, npm_roots.as_deref(), ) .await; @@ -2058,6 +2071,54 @@ mod tests { assert_eq!(single.get("pkg:npm/dup@1.0.0"), Some(&root_copy)); } + /// #1494: run from a workspace member, a member's alias link into the + /// root store (`packages/a/node_modules/lp`, vlt's layout here) is the + /// only route to the copy, so apply's and rollback's lookups must + /// return it; a link to another version must not count. From the root + /// the link rides after the store copy, which apply and rollback + /// collapse onto one physical dir. + #[cfg(unix)] + #[tokio::test] + async fn multi_copy_lookups_take_a_member_alias_link_into_the_root_store() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let store = root.join("node_modules/.vlt"); + let store_copy = write_npm_package(&store.join("~npm~left-pad@1.3.0"), "left-pad", "1.3.0"); + write_npm_package(&store.join("~npm~left-pad@1.2.0"), "left-pad", "1.2.0"); + let member_nm = root.join("packages/a/node_modules"); + std::fs::create_dir_all(&member_nm).unwrap(); + std::os::unix::fs::symlink( + "../../../node_modules/.vlt/~npm~left-pad@1.3.0/node_modules/left-pad", + member_nm.join("lp"), + ) + .unwrap(); + std::os::unix::fs::symlink( + "../../../node_modules/.vlt/~npm~left-pad@1.2.0/node_modules/left-pad", + member_nm.join("old"), + ) + .unwrap(); + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + let partitioned = partition_purls(std::slice::from_ref(&purl), None); + + let member = local_options(root.join("packages/a")); + let link = vec![member_nm.join("lp")]; + let applied = find_all_packages_for_purls(&partitioned, &member, true).await; + assert_eq!(applied.get(&purl), Some(&link), "{applied:?}"); + let rolled = find_all_packages_for_rollback(&partitioned, &member, true).await; + assert_eq!(rolled.get(&purl), Some(&link), "{rolled:?}"); + + let mut from_root = + find_all_packages_for_purls(&partitioned, &local_options(root.to_path_buf()), true) + .await; + assert_eq!( + from_root.get(&purl), + Some(&vec![store_copy.clone(), member_nm.join("lp")]), + "{from_root:?}" + ); + distinct_npm_copies(&mut from_root).await; + assert_eq!(from_root.get(&purl), Some(&vec![store_copy])); + } + /// Multi-copy for gem (mirrors the npm test above): bundler's scoped /// (`//gems`) and flat (`gems/`) store layouts coexist under /// one `vendor/bundle` root — a bundler-2 `--path` install beside a diff --git a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs index 6b6f02919..4d39676ab 100644 --- a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs +++ b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs @@ -811,6 +811,125 @@ fn apply_and_rollback_visit_a_pnpm_workspace_member_link_once() { ); } +/// #1494: run from inside a workspace member, a member's ALIAS dependency +/// (`"dv": "npm:dupvuln@1.0.0"`) under a root-store layout is only the link +/// `packages/a/node_modules/dv` into the root store, and the member's +/// `node_modules` is the only root the resolver walks. Apply used to call +/// it `package_not_installed` (exit 1) and rollback could not restore it, +/// though scan offered the patch through that link. From the root the same +/// copy is reached twice (store entry and member link) and must still be +/// visited once (#633). +#[cfg(unix)] +#[test] +fn apply_and_rollback_reach_a_member_alias_link_from_the_member() { + for (store, entry) in [ + (".vlt", "~npm~dupvuln@1.0.0"), + (".pnpm", "dupvuln@1.0.0"), + (".store", "dupvuln@1.0.0-0a1b2c3d"), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let name = "dupvuln"; + let purl = "pkg:npm/dupvuln@1.0.0"; + let original = b"module.exports = function(){ return 'VULNERABLE'; };\n"; + let mut patched = original.to_vec(); + patched.extend_from_slice(b"// SOCKET-PATCHED-MULTICOPY\n"); + std::fs::write( + root.join("package.json"), + r#"{ "name": "root", "version": "1.0.0", "private": true }"#, + ) + .unwrap(); + std::fs::write(root.join("vlt.json"), r#"{ "workspaces": ["packages/*"] }"#).unwrap(); + let store_copy = write_copy( + &root + .join("node_modules") + .join(store) + .join(entry) + .join("node_modules") + .join(name), + name, + "1.0.0", + original, + ); + let member = root.join("packages").join("a"); + std::fs::create_dir_all(member.join("node_modules")).unwrap(); + std::fs::write( + member.join("package.json"), + r#"{ "name": "a", "version": "1.0.0", "dependencies": { "dv": "npm:dupvuln@1.0.0" } }"#, + ) + .unwrap(); + std::os::unix::fs::symlink( + format!("../../../node_modules/{store}/{entry}/node_modules/{name}"), + member.join("node_modules").join("dv"), + ) + .unwrap(); + for socket_root in [root, member.as_path()] { + stage_manifest_and_blob( + socket_root, + purl, + &git_sha256(original), + &git_sha256(&patched), + &patched, + ); + std::fs::write( + socket_root + .join(".socket") + .join("blobs") + .join(git_sha256(original)), + original, + ) + .unwrap(); + } + + let (code, v) = run_apply(&member); + assert_eq!(code, 0, "{store}: member apply; envelope={v}"); + assert_eq!(std::fs::read(&store_copy).unwrap(), patched, "{store}"); + assert_eq!( + dupvuln_events(&v), + vec![("applied".to_string(), String::new())], + "{store}: member apply; envelope={v}" + ); + + let (code, v) = run_apply(&member); + assert_eq!(code, 0, "{store}: member re-apply; envelope={v}"); + assert_eq!( + dupvuln_events(&v), + vec![("skipped".to_string(), "already_patched".to_string())], + "{store}: member re-apply; envelope={v}" + ); + + let (code, v) = run_rollback_with(&member, &["--preserve-state".as_ref()]); + assert_eq!(code, 0, "{store}: member rollback; envelope={v}"); + assert_eq!(std::fs::read(&store_copy).unwrap(), original, "{store}"); + assert_eq!( + v["summary"]["rolledBack"], 1, + "{store}: member rollback; envelope={v}" + ); + + // From the root: the store entry and the member's link are one copy. + let (code, v) = run_apply(root); + assert_eq!(code, 0, "{store}: root apply; envelope={v}"); + assert_eq!(std::fs::read(&store_copy).unwrap(), patched, "{store}"); + assert_eq!( + dupvuln_events(&v), + vec![("applied".to_string(), String::new())], + "{store}: root apply; envelope={v}" + ); + let (code, v) = run_rollback(root); + assert_eq!(code, 0, "{store}: root rollback; envelope={v}"); + assert_eq!(std::fs::read(&store_copy).unwrap(), original, "{store}"); + assert_eq!( + v["summary"]["rolledBack"], 1, + "{store}: root rollback; envelope={v}" + ); + assert_eq!( + crate::rollback_json::already_original(&v), + 0, + "{store}: root rollback; envelope={v}" + ); + } +} + /// One pnpm 11+ isolated global install, `//node_modules`, with /// its own `.pnpm` holding a real `dupvuln@1.0.0`, linked at the top level /// (`direct`) or reached only through `wrapper` (transitive-only). From 6a3906f2994bdba44289c6240fc9159779b20b17 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:50:44 +0000 Subject: [PATCH 3/4] Skip alias links for already-found packages Apply, rollback, get and vendor now take a workspace member's alias link only for a package they found no other copy of. When a copy was found, apply already patches every store variant of it, so taking the link as well visited one variant twice and reported a phantom already_patched (or already-original on rollback). It also kept a first-party alias link beside a patched registry copy from turning a clean run into a refusal, and skips the link pass whenever every package resolves. Path-scoped scan and rollback still match every link. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/ecosystem_dispatch.rs | 127 +++++++++++++----- .../tests/apply/in_process_npm_multicopy.rs | 93 +++++++++++++ 2 files changed, 189 insertions(+), 31 deletions(-) diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 878dcd25d..8c115ba8a 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -619,9 +619,17 @@ pub async fn find_all_packages_for_purls( // `merge_qualified`'s `push_path`. Cargo keeps every registry index // copy (`merge_cargo_copies`); the single-copy ecosystems keep true // first-wins via their own `merge_first_wins` wiring in - // `dispatch_find`. A workspace member's npm alias link is a copy too - // (#1494); apply collapses it onto the store copy it names. - dispatch_find_with_npm_links(partitioned, options, silent, merge_variant_copies, None).await + // `dispatch_find`. An npm purl with no copy can still be reached + // through a workspace member's alias link (#1494). + dispatch_find_with_npm_links( + partitioned, + options, + silent, + merge_variant_copies, + None, + NpmLinks::ForUnresolved, + ) + .await } /// Multi-copy variant of `find_packages_for_rollback` (qualified-aware @@ -635,33 +643,66 @@ pub async fn find_all_packages_for_rollback( } /// [`find_all_packages_for_rollback`], taking the npm `node_modules` roots -/// from `prior` as [`find_packages_for_rollback_reusing`] does. A workspace -/// member's npm alias link is a copy too (#1494), as for apply. +/// from `prior` as [`find_packages_for_rollback_reusing`] does. An npm purl +/// with no copy can still be reached through a workspace member's alias +/// link (#1494), as for apply. pub async fn find_all_packages_for_rollback_reusing( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - dispatch_find_with_npm_links(partitioned, options, silent, merge_qualified, prior).await + dispatch_find_with_npm_links( + partitioned, + options, + silent, + merge_qualified, + prior, + NpmLinks::ForUnresolved, + ) + .await } /// The install paths that reach each purl, for `scan`'s and `rollback`'s -/// path targets: [`find_all_packages_for_rollback_reusing`]'s copies, link -/// spellings included. The result is for path matching only: it can name -/// one physical copy more than once. +/// path targets: [`find_all_packages_for_rollback_reusing`]'s copies plus +/// every matching npm root link, not only those of purls with no copy +/// (#1266). The result is for path matching only: it can name one +/// physical copy more than once. pub async fn find_package_paths_for_path_scope( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap> { - dispatch_find_with_npm_links(partitioned, options, silent, merge_qualified, prior).await + dispatch_find_with_npm_links( + partitioned, + options, + silent, + merge_qualified, + prior, + NpmLinks::All, + ) + .await +} + +/// Which npm root links [`dispatch_find_with_npm_links`] adds. +#[derive(Clone, Copy, PartialEq, Eq)] +enum NpmLinks { + /// Every link to a purl asked for: path scoping matches by install + /// path, so each spelling counts. + All, + /// Only links to purls the resolver found no copy of. The commands + /// that act per copy already reach every store peer variant of a + /// found copy through the apply engine's fan-out, so a link to one + /// would be a second visit (#633); and a link into first-party source + /// beside a found registry copy must not turn into a refusal. + ForUnresolved, } /// [`dispatch_find`]'s copies, plus each npm package link directly in a /// `node_modules` root (scoped ones one level down) whose target's -/// `package.json` names one of the npm purls asked for, after the copies. +/// `package.json` names one of the npm purls asked for (see [`NpmLinks`] +/// for which), after the copies. /// /// The crawler records a plain-name link at its own path /// (`packages/a/node_modules/ms`), but never an alias link @@ -675,14 +716,15 @@ pub async fn find_package_paths_for_path_scope( /// matched by its target's own identity, not against the crawled copies, /// so a link to a store peer variant the crawl skipped still counts, and /// the cost is one pass over the root links however many purls are asked -/// for. A link can name a copy the resolver also found: apply and -/// rollback act on each physical copy once ([`distinct_npm_copies`]). +/// for (none when every purl already has a copy and only the unresolved +/// ones' links are wanted). async fn dispatch_find_with_npm_links( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, variant_merge: MergeFn, prior: Option<&NpmCrawlSnapshot>, + links: NpmLinks, ) -> HashMap> { let mut wanted: HashMap> = HashMap::new(); for purl in partitioned.get(&Ecosystem::Npm).into_iter().flatten() { @@ -707,6 +749,16 @@ async fn dispatch_find_with_npm_links( let Some(roots) = npm_roots else { return found; }; + if links == NpmLinks::ForUnresolved { + let resolved = |purl: &String| found.get(purl).is_some_and(|paths| !paths.is_empty()); + wanted.retain(|_, purls| { + purls.retain(|purl| !resolved(purl)); + !purls.is_empty() + }); + if wanted.is_empty() { + return found; + } + } // A failed enumeration only loses the extra links; the copies stand. let links = tokio::task::spawn_blocking(move || npm_links_to(&wanted, &roots)) .await @@ -807,17 +859,25 @@ pub async fn find_packages_for_rollback( /// tree nothing has touched since) instead of walking the tree for them /// again. Only the root discovery is reused: each root is still searched /// by `find_by_purls`, so copy choice and order are unchanged. A snapshot -/// taken with other options is ignored. +/// taken with other options is ignored. An npm purl with no copy can still +/// be reached through a workspace member's alias link (#1494). pub async fn find_packages_for_rollback_reusing( partitioned: &HashMap>, options: &CrawlerOptions, silent: bool, prior: Option<&NpmCrawlSnapshot>, ) -> HashMap { - let npm_roots = prior - .filter(|p| p.taken_with(options)) - .map(|p| p.roots.as_slice()); - collapse_to_first(dispatch_find(partitioned, options, silent, merge_qualified, npm_roots).await) + collapse_to_first( + dispatch_find_with_npm_links( + partitioned, + options, + silent, + merge_qualified, + prior, + NpmLinks::ForUnresolved, + ) + .await, + ) } /// The npm half of one [`crawl_ecosystems_with_npm`] run: the packages @@ -2073,10 +2133,11 @@ mod tests { /// #1494: run from a workspace member, a member's alias link into the /// root store (`packages/a/node_modules/lp`, vlt's layout here) is the - /// only route to the copy, so apply's and rollback's lookups must - /// return it; a link to another version must not count. From the root - /// the link rides after the store copy, which apply and rollback - /// collapse onto one physical dir. + /// only route to the copy, so the copy lookups must return it; a link + /// to another version must not count. From the root the store copy is + /// found and the link is not added: the apply engine's store fan-out + /// already reaches a link's peer variant, and a second visit would be + /// #633's phantom `already_patched`. Path scoping still sees it. #[cfg(unix)] #[tokio::test] async fn multi_copy_lookups_take_a_member_alias_link_into_the_root_store() { @@ -2106,17 +2167,21 @@ mod tests { assert_eq!(applied.get(&purl), Some(&link), "{applied:?}"); let rolled = find_all_packages_for_rollback(&partitioned, &member, true).await; assert_eq!(rolled.get(&purl), Some(&link), "{rolled:?}"); - - let mut from_root = - find_all_packages_for_purls(&partitioned, &local_options(root.to_path_buf()), true) - .await; + let one = find_packages_for_rollback(&partitioned, &member, true).await; + assert_eq!(one.get(&purl), Some(&link[0]), "{one:?}"); + + let at_root = local_options(root.to_path_buf()); + let store_only = vec![store_copy.clone()]; + let applied = find_all_packages_for_purls(&partitioned, &at_root, true).await; + assert_eq!(applied.get(&purl), Some(&store_only), "{applied:?}"); + let rolled = find_all_packages_for_rollback(&partitioned, &at_root, true).await; + assert_eq!(rolled.get(&purl), Some(&store_only), "{rolled:?}"); + let scoped = find_package_paths_for_path_scope(&partitioned, &at_root, true, None).await; assert_eq!( - from_root.get(&purl), - Some(&vec![store_copy.clone(), member_nm.join("lp")]), - "{from_root:?}" + scoped.get(&purl), + Some(&vec![store_copy, member_nm.join("lp")]), + "{scoped:?}" ); - distinct_npm_copies(&mut from_root).await; - assert_eq!(from_root.get(&purl), Some(&vec![store_copy])); } /// Multi-copy for gem (mirrors the npm test above): bundler's scoped diff --git a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs index 4d39676ab..456a4476d 100644 --- a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs +++ b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs @@ -930,6 +930,99 @@ fn apply_and_rollback_reach_a_member_alias_link_from_the_member() { } } +/// #1494 / #633: the root's `dupvuln` link resolves to one pnpm peer +/// variant and member a's alias `dv` to another. Run from the root, the +/// apply engine's store fan-out from the root copy already patches the +/// member's variant, so the member's alias link must not add a second +/// visit (a phantom `already_patched` / already-original event). +#[cfg(unix)] +#[test] +fn apply_and_rollback_visit_a_member_alias_peer_variant_once() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let name = "dupvuln"; + let purl = "pkg:npm/dupvuln@1.0.0"; + let original = b"module.exports = function(){ return 'VULNERABLE'; };\n"; + let mut patched = original.to_vec(); + patched.extend_from_slice(b"// SOCKET-PATCHED-MULTICOPY\n"); + std::fs::write( + root.join("package.json"), + r#"{ "name": "root", "version": "1.0.0", "private": true, "dependencies": { "dupvuln": "1.0.0" } }"#, + ) + .unwrap(); + std::fs::write( + root.join("pnpm-workspace.yaml"), + "packages:\n - 'packages/*'\n", + ) + .unwrap(); + let variants = ["dupvuln@1.0.0_react@17.0.2", "dupvuln@1.0.0_react@18.2.0"].map(|v| { + write_copy( + &root + .join("node_modules/.pnpm") + .join(v) + .join("node_modules") + .join(name), + name, + "1.0.0", + original, + ) + }); + std::os::unix::fs::symlink( + ".pnpm/dupvuln@1.0.0_react@17.0.2/node_modules/dupvuln", + root.join("node_modules").join(name), + ) + .unwrap(); + let member = root.join("packages").join("a"); + std::fs::create_dir_all(member.join("node_modules")).unwrap(); + std::fs::write( + member.join("package.json"), + r#"{ "name": "a", "version": "1.0.0", "dependencies": { "dv": "npm:dupvuln@1.0.0" } }"#, + ) + .unwrap(); + std::os::unix::fs::symlink( + "../../../node_modules/.pnpm/dupvuln@1.0.0_react@18.2.0/node_modules/dupvuln", + member.join("node_modules").join("dv"), + ) + .unwrap(); + stage_manifest_and_blob( + root, + purl, + &git_sha256(original), + &git_sha256(&patched), + &patched, + ); + std::fs::write( + root.join(".socket") + .join("blobs") + .join(git_sha256(original)), + original, + ) + .unwrap(); + + let (code, v) = run_apply(root); + assert_eq!(code, 0, "apply; envelope={v}"); + for variant in &variants { + assert_eq!(std::fs::read(variant).unwrap(), patched, "{variant:?}"); + } + assert_eq!( + dupvuln_events(&v), + vec![("applied".to_string(), String::new())], + "apply; envelope={v}" + ); + + let (code, v) = run_rollback(root); + assert_eq!(code, 0, "rollback; envelope={v}"); + for variant in &variants { + assert_eq!(std::fs::read(variant).unwrap(), original, "{variant:?}"); + } + assert_eq!(v["summary"]["rolledBack"], 1, "rollback; envelope={v}"); + assert_eq!( + crate::rollback_json::already_original(&v), + 0, + "rollback; envelope={v}" + ); +} + /// One pnpm 11+ isolated global install, `//node_modules`, with /// its own `.pnpm` holding a real `dupvuln@1.0.0`, linked at the top level /// (`direct`) or reached only through `wrapper` (transitive-only). From 6104e76ac7f81ad8136960401fe9f9c5810e1bf5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 20:27:03 +0000 Subject: [PATCH 4/4] Let apply --check and vex see member alias links Run from a workspace member, apply now patches an alias dependency through the member's link into the root store, but apply --check and vex looked copies up without that link. The check reported the package as not installed and exited 0. They now use the same lookup as apply, which takes a root link only for a package with no other copy. Two vex_consumed tests assumed the resolver never returns a link. They now check that it returns the same copies the identity fallback finds. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0195pcxXZ2GXtByX1L8egMhJ --- .../src/commands/vex_consumed.rs | 27 ++++++++++++------- .../src/ecosystem_dispatch.rs | 9 ++++--- .../tests/apply/in_process_npm_multicopy.rs | 10 +++++++ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index b1cb2254e..94af3f22a 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -641,17 +641,21 @@ mod tests { ..GlobalArgs::default() }; let purl = "pkg:npm/left-pad@1.3.0".to_string(); - // The resolver never takes a link as a copy. - assert!(installed_copies(&common, &purl) + let mut expected = vec![alias.clone(), peers[1].clone()]; + expected.sort(); + // The resolver takes a root link only for a purl with no copy + // (#1494), and expands it to the same copies. + let mut resolved = installed_copies(&common, &purl) .await - .get(&purl) - .is_none_or(Vec::is_empty)); + .remove(&purl) + .unwrap(); + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); + // An empty installed entry still falls back to the alias identity. let installed = HashMap::from([(purl, Vec::new())]); let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await; - assert_eq!(calls, vec![vec![alias.clone()]]); - let mut expected = vec![alias, peers[1].clone()]; + assert_eq!(calls, vec![vec![alias]]); paths.sort(); - expected.sort(); assert_eq!(paths, expected); } @@ -746,9 +750,12 @@ mod tests { &purls[0], ) .await; - assert!( - resolved.get(&purls[0]).is_none_or(Vec::is_empty), - "the resolver skips symlinks: {resolved:?}" + // The crawler skips symlinks; the resolver takes the root link + // only because the purl has no other copy (#1494). + assert_eq!( + resolved.get(&purls[0]), + Some(&vec![root.join("node_modules/lp")]), + "{resolved:?}" ); let (_, _, _, snapshot) = diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 8c115ba8a..82595ae59 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -1050,6 +1050,9 @@ pub(crate) fn npm_paths_by_identity_in( /// [`find_packages_for_rollback_reusing`]. Only the root discovery is /// reused: each root is still searched by `find_by_purls`, so copy choice /// and order are unchanged. A snapshot taken with other options is ignored. +/// An npm purl with no copy can still be reached through a workspace +/// member's alias link (#1494), so `apply --check` and `vex` see the copy +/// `apply` patched. /// /// Read-only by contract (its one caller is `vex`), so it also searches the /// gem stores under a refused out-of-tree `.bundle/config` path @@ -1063,13 +1066,13 @@ pub async fn find_manifest_package_copies_reusing( ) -> HashMap> { let partitioned = partition_purls(purls, common.ecosystems.as_deref()); let crawler_options = common.crawler_options(); - let npm_roots = prior.and_then(|p| p.roots_for(&crawler_options)); - let mut copies = dispatch_find( + let mut copies = dispatch_find_with_npm_links( &partitioned, &crawler_options, quiet, merge_qualified, - npm_roots, + prior, + NpmLinks::ForUnresolved, ) .await; // `apply` also writes every store variant of each npm copy (a pnpm peer diff --git a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs index 456a4476d..1ab246a34 100644 --- a/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs +++ b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs @@ -890,6 +890,16 @@ fn apply_and_rollback_reach_a_member_alias_link_from_the_member() { "{store}: member apply; envelope={v}" ); + // `apply --check` (and `vex`, through the same lookup) must see the + // copy apply just patched, not report it missing. + let (code, v) = run_apply_with(&member, &["--check".as_ref()]); + assert_eq!(code, 0, "{store}: member check; envelope={v}"); + assert_eq!( + dupvuln_events(&v), + vec![("skipped".to_string(), "already_patched".to_string())], + "{store}: member check; envelope={v}" + ); + let (code, v) = run_apply(&member); assert_eq!(code, 0, "{store}: member re-apply; envelope={v}"); assert_eq!(