From 72a4510b43bf8c1f01969c51567aac02912190c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:24:42 +0000 Subject: [PATCH 1/6] Start fix for #601, #603 Assisted-by: Claude Code:claude-opus-5-5 From 1e73ecccb702a8816a10dbd286c5b26b009c2f25 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:37:10 +0000 Subject: [PATCH 2/6] Add tests for store copies apply and vex miss A copy of a package bundled inside another package's pnpm/vlt store entry is left unpatched when the package is also installed normally (#601), and agent-mode vex never hashes store peer-variant copies such as a Deno _1 copy (#603). These tests fail on main. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/apply/in_process_npm_multicopy.rs | 80 ++++++++++++ crates/socket-patch-cli/tests/e2e_vex.rs | 122 ++++++++++++++++++ .../tests/crawler_npm_e2e.rs | 54 ++++++++ 3 files changed, 256 insertions(+) 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 fd0ce2390..8be7ad913 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 @@ -397,3 +397,83 @@ fn apply_and_rollback_reach_both_transitive_only_vlt_store_copies() { assert_eq!(v["alreadyOriginal"], 1, "envelope={v}"); assert_vlt_copies([&primary, &twin], false, "after rollback"); } + +/// #601: a copy bundled inside ANOTHER package's vlt or pnpm store entry +/// (`.vlt/~npm~bundler@1.0.0/node_modules/bundler/node_modules/dupvuln`) +/// is what that package loads, so apply must patch it even when the same +/// `name@version` is also installed normally, and rollback must restore +/// it. Before the fix only the normal copy was patched. +#[cfg(unix)] +#[test] +fn apply_and_rollback_reach_a_bundled_copy_beside_a_normal_install() { + for (store, normal_id, host_id) in [ + (".vlt", "~npm~dupvuln@1.0.0", "~npm~bundler@1.0.0"), + (".pnpm", "dupvuln@1.0.0", "bundler@1.0.0"), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let name = "dupvuln"; + 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": "bundled-root", "version": "0.0.0" }"#, + ) + .unwrap(); + let nm = root.join("node_modules"); + let store_dir = nm.join(store); + let normal = write_copy( + &store_dir.join(normal_id).join("node_modules").join(name), + name, + "1.0.0", + original, + ); + let host = store_dir.join(host_id).join("node_modules").join("bundler"); + write_copy( + &host, + "bundler", + "1.0.0", + b"module.exports = require('dupvuln');\n", + ); + let bundled = write_copy( + &host.join("node_modules").join(name), + name, + "1.0.0", + original, + ); + std::os::unix::fs::symlink( + store_dir.join(normal_id).join("node_modules").join(name), + nm.join(name), + ) + .unwrap(); + std::os::unix::fs::symlink(&host, nm.join("bundler")).unwrap(); + stage_manifest_and_blob( + root, + "pkg:npm/dupvuln@1.0.0", + &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, "{store}: apply must succeed; envelope={v}"); + assert_eq!(v["status"], "success", "{store}: envelope={v}"); + assert_vlt_copies([&normal, &bundled], true, &format!("{store} after apply")); + + let (code, v) = run_rollback(root); + assert_eq!(code, 0, "{store}: rollback must succeed; envelope={v}"); + assert_vlt_copies( + [&normal, &bundled], + false, + &format!("{store} after rollback"), + ); + } +} diff --git a/crates/socket-patch-cli/tests/e2e_vex.rs b/crates/socket-patch-cli/tests/e2e_vex.rs index ffe748bbf..2195805f7 100644 --- a/crates/socket-patch-cli/tests/e2e_vex.rs +++ b/crates/socket-patch-cli/tests/e2e_vex.rs @@ -991,6 +991,128 @@ fn verify_mode_requires_every_installed_copy_patched() { assert_eq!(stmts[0]["status"], "not_affected"); } +/// Regressions #603 and #601: the every-copy rule covers store copies +/// too. `apply` patches a package's other store copies (a Deno `_1` copy +/// index, a pnpm peer variant) and a copy bundled inside another +/// package's store entry, so `vex` must hash each of them: one pristine +/// copy omits the purl, and all patched attests it. Each layout has the +/// primary copy linked from the importer, as `deno install`, pnpm and vlt +/// write it. +#[cfg(unix)] +#[test] +fn verify_mode_requires_every_store_copy_patched() { + let patched: &[u8] = b"patched store index"; + let pristine: &[u8] = b"pristine store index"; + let after_hash = compute_git_sha256_from_bytes(patched); + let before_hash = compute_git_sha256_from_bytes(pristine); + + // (label, the importer-linked copy, the other copy) — both relative to + // `node_modules`. + let layouts = [ + ( + "deno copy index (#603)", + ".deno/store-pkg@1.0.0/node_modules/store-pkg", + ".deno/store-pkg@1.0.0_1/node_modules/store-pkg", + ), + ( + "pnpm peer variant (#603)", + ".pnpm/store-pkg@1.0.0(react@17.0.2)/node_modules/store-pkg", + ".pnpm/store-pkg@1.0.0(react@18.2.0)/node_modules/store-pkg", + ), + ( + "vlt bundled copy (#601)", + ".vlt/~npm~store-pkg@1.0.0/node_modules/store-pkg", + ".vlt/~npm~bundler@1.0.0/node_modules/bundler/node_modules/store-pkg", + ), + ( + "pnpm bundled copy (#601)", + ".pnpm/store-pkg@1.0.0/node_modules/store-pkg", + ".pnpm/bundler@1.0.0/node_modules/bundler/node_modules/store-pkg", + ), + ]; + + let run = |primary: &str, other: &str, other_bytes: &[u8]| { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + let nm = cwd.join("node_modules"); + for (rel, bytes) in [(primary, patched), (other, other_bytes)] { + let dir = nm.join(rel); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("package.json"), + r#"{"name":"store-pkg","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(dir.join("index.js"), bytes).unwrap(); + } + // A bundled copy's host package, so its store entry is real. + if let Some(host) = other.split_once("/node_modules/bundler/") { + let host = nm.join(host.0).join("node_modules/bundler"); + std::fs::write( + host.join("package.json"), + r#"{"name":"bundler","version":"1.0.0"}"#, + ) + .unwrap(); + std::os::unix::fs::symlink(&host, nm.join("bundler")).unwrap(); + } + std::os::unix::fs::symlink(nm.join(primary), nm.join("store-pkg")).unwrap(); + + let mut manifest = PatchManifest::new(); + manifest.patches.insert( + "pkg:npm/store-pkg@1.0.0".to_string(), + make_record( + "55555555-5555-4555-8555-555555555555", + "package/index.js", + before_hash.as_str(), + after_hash.as_str(), + "GHSA-store", + &["CVE-STORE"], + ), + ); + write_manifest(cwd, &manifest); + let out = cli() + .args([ + "vex", + "--cwd", + cwd.to_str().unwrap(), + "--product", + "pkg:npm/test-app@1.0.0", + ]) + .output() + .expect("invoke vex"); + ( + out.status.success(), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) + }; + + for (label, primary, other) in layouts { + let (ok, stdout, stderr) = run(primary, other, pristine); + assert!( + !ok && !stdout.contains("GHSA-store"), + "{label}: an unpatched store copy must keep the purl out of the \ + VEX doc.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert!( + stderr.contains("Warning: omitting pkg:npm/store-pkg@1.0.0 from VEX") + && stderr.contains("(not_applied)"), + "{label}: the omission must carry not_applied. got: {stderr}" + ); + + // Control: every copy patched → attested. + let (ok, stdout, stderr) = run(primary, other, patched); + assert!( + ok, + "{label}: all copies patched must attest. stderr:\n{stderr}" + ); + let doc: Value = serde_json::from_str(&stdout).unwrap(); + let stmts = doc["statements"].as_array().unwrap(); + assert_eq!(stmts.len(), 1, "{label}: doc:\n{stdout}"); + assert_eq!(stmts[0]["status"], "not_affected", "{label}"); + } +} + #[test] fn verify_mode_all_failed_exits_non_zero() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 226b9b474..c49b3ad12 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -2336,6 +2336,60 @@ async fn find_by_purls_resolves_bundled_only_target_via_fallback_pass() { ); } +/// #601: a bundled copy inside ANOTHER package's store entry +/// (`.vlt/~npm~bundler@1.0.0/node_modules/bundler/node_modules/left-pad`) +/// is a physical copy the host loads, so it must be returned even when the +/// same `name@version` is also installed normally. Pass 1's store filter +/// drops the host's entry once the normal copy matched, and the unfiltered +/// pass 2 only re-probes targets with no copy at all, so apply patched only +/// the normal copy and vex attested it. Covers every pnpm-shaped store and +/// vlt's, with the normal copy reached through an importer link and as a +/// transitive-only store copy. +#[cfg(unix)] +#[tokio::test] +#[serial_test::parallel] +async fn find_by_purls_returns_bundled_copy_of_an_already_found_target() { + for (store_name, normal_entry, host_entry) in [ + (".pnpm", "left-pad@1.3.0", "bundler@1.0.0"), + (".vlt", "~npm~left-pad@1.3.0", "~npm~bundler@1.0.0"), + (".bun", "left-pad@1.3.0", "bundler@1.0.0"), + (".deno", "left-pad@1.3.0", "bundler@1.0.0"), + ] { + for importer_link in [true, false] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let nm = root.join("node_modules"); + let store = nm.join(store_name); + + let normal_nm = store.join(normal_entry).join("node_modules"); + stage_npm_pkg(&normal_nm, "left-pad", "1.3.0").await; + let host_nm = store.join(host_entry).join("node_modules"); + stage_npm_pkg(&host_nm, "bundler", "1.0.0").await; + let bundled_nm = host_nm.join("bundler").join("node_modules"); + stage_npm_pkg(&bundled_nm, "left-pad", "1.3.0").await; + std::os::unix::fs::symlink(host_nm.join("bundler"), nm.join("bundler")).unwrap(); + let normal = if importer_link { + std::os::unix::fs::symlink(normal_nm.join("left-pad"), nm.join("left-pad")) + .unwrap(); + nm.join("left-pad") + } else { + normal_nm.join("left-pad") + }; + + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + let result = NpmCrawler + .find_by_purls(&nm, std::slice::from_ref(&purl)) + .await + .unwrap(); + let mut got: Vec<_> = result[&purl].iter().map(|p| p.path.clone()).collect(); + got.sort(); + let mut want = vec![normal, bundled_nm.join("left-pad")]; + want.sort(); + assert_eq!(got, want, "{store_name} importer_link={importer_link}"); + } + } +} + // ── vlt store (.vlt), staged from captured real layouts ──────── /// The eras with a captured `vlt install` layout under From 7b4ba772296f6c1ac788c63632b4ed88001a18e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:37:10 +0000 Subject: [PATCH 3/6] Patch and verify every npm store copy Agent-mode apply and rollback now reach a copy bundled inside another package's pnpm, vlt, Bun or Deno store entry even when the same name@version is installed normally: the resolver probes a skipped store entry's bundled tree (one stat per entry). Agent-mode vex now checks the store peer-variant copies apply patches, so one unpatched copy omits the purl instead of producing a false not_affected. Fixes #601, #603 Assisted-by: Claude Code:claude-opus-5-5 --- CHANGELOG.md | 8 ++ .../src/commands/vex_consumed.rs | 30 +----- .../src/ecosystem_dispatch.rs | 18 +++- .../src/crawlers/npm_crawler.rs | 98 ++++++++++++++++--- 4 files changed, 108 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3572a298c..b964fde22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,14 @@ limits, and required install commands. ### Fixed +- Agent-mode `apply` and `rollback` now reach a copy of a package that is + bundled inside another package's pnpm, vlt, Bun or Deno store entry, even + when the same `name@version` is also installed normally. Before, only the + normal copy was patched, and `vex` attested the patch while the bundling + package loaded the unpatched copy (#601). Agent-mode `vex` now also checks + every store peer-variant copy that `apply` patches (a Deno `_1` copy, a pnpm + `(peer)` variant, a vlt `~peer` extra), so one unpatched copy omits the purl + instead of being attested (#603). - Global mode (`-g`) finds npm, yarn, pnpm, bun, RubyGems and Composer on Windows, where they install as `.cmd` / `.bat` shims, instead of reporting an empty scan. The yarn and npm-family global lookups no longer run from the diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index 22e5fd751..fc69b6738 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -40,7 +40,7 @@ use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; -use socket_patch_core::crawlers::npm_crawler::find_store_peer_variant_copies; +use socket_patch_core::crawlers::npm_crawler::with_store_peer_variant_copies; use socket_patch_core::crawlers::{ CargoCrawler, CrawlerOptions, Ecosystem, GoCrawler, MavenCrawler, NpmCrawler, }; @@ -109,7 +109,7 @@ pub(crate) async fn hosted_consumed_copies( let mut paths = all.remove(purl).unwrap_or_default(); paths.extend(aliases.remove(purl).unwrap_or_default()); if npm.contains(&purl) { - paths = with_store_variants(paths).await; + paths = with_store_peer_variant_copies(paths).await; } out.insert( purl.clone(), @@ -315,28 +315,6 @@ async fn npm_identity_fallback_reusing( } } -/// `paths` plus every store variant of each (a pnpm peer suffix, a vlt peer -/// or modifier extra, a vlt registry-alias instance of the same -/// `name@version`): the crawler resolves a store copy only for a package -/// with no importer copy, leaving the variants to apply's fan-out, but each -/// variant is what some dependent loads. -async fn with_store_variants(paths: Vec) -> Vec { - let mut seen: std::collections::HashSet = std::collections::HashSet::new(); - for path in &paths { - seen.insert(tokio::fs::canonicalize(path).await.unwrap_or(path.clone())); - } - let mut out = paths.clone(); - for path in &paths { - for copy in find_store_peer_variant_copies(path).await { - let canonical = tokio::fs::canonicalize(©).await.unwrap_or(copy.clone()); - if seen.insert(canonical) { - out.push(copy); - } - } - } - out -} - // ── golang ─────────────────────────────────────────────────────────────── /// Under `replace M v => patch.socket.dev/gopatch/ ` the build @@ -851,7 +829,7 @@ mod tests { nm.join("left-pad"), ) .unwrap(); - let mut got = with_store_variants(vec![nm.join("left-pad")]).await; + let mut got = with_store_peer_variant_copies(vec![nm.join("left-pad")]).await; got.sort(); let mut want = vec![ nm.join("left-pad"), @@ -860,7 +838,7 @@ mod tests { ]; want.sort(); assert_eq!(got, want); - assert!(with_store_variants(Vec::new()).await.is_empty()); + assert!(with_store_peer_variant_copies(Vec::new()).await.is_empty()); } /// vlt twin of the `.pnpm` case: every importer entry is a link into diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 89b9d3a7c..0d710dbe3 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use crate::args::GlobalArgs; +use socket_patch_core::crawlers::npm_crawler::with_store_peer_variant_copies; use socket_patch_core::crawlers::walk_pool; use socket_patch_core::crawlers::CargoCrawler; use socket_patch_core::crawlers::ComposerCrawler; @@ -593,8 +594,8 @@ pub(crate) fn npm_paths_by_identity_in( /// patch would silently resolve as `package_not_found`. The rollback /// variant fans each base path back out to every qualified manifest PURL /// — the same mapping the manifest was written with (`get` uses the same -/// resolver). `vex` hashes the first copy of a manifest purl and every copy -/// of a hosted one from this one lookup. +/// resolver). `vex` hashes every copy of a manifest purl and of a hosted +/// one from this one lookup; npm copies include their store variants. /// /// With `prior`, the npm `node_modules` roots come /// from it (a crawl of the same options earlier in this process, over @@ -612,14 +613,23 @@ pub async fn find_manifest_package_copies_reusing( 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)); - dispatch_find( + let mut copies = dispatch_find( &partitioned, &crawler_options, quiet, merge_qualified, npm_roots, ) - .await + .await; + // `apply` also writes every store variant of each npm copy (a pnpm peer + // suffix, a Deno `_N` copy index, a vlt peer extra), so "every copy" + // includes them (#603). + for (purl, paths) in copies.iter_mut() { + if purl.starts_with("pkg:npm/") { + *paths = with_store_peer_variant_copies(std::mem::take(paths)).await; + } + } + copies } /// Box the future `make` returns, constructing it inside this (non-async) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index c72e4ca45..361f62273 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -1165,9 +1165,13 @@ impl NpmCrawler { /// pnpm's and vlt's store peer-variant copies are deliberately NOT /// enumerated here for a copy already found in an importer tree (a /// symlinked direct dep): those are handled by the apply engine's - /// [`find_store_peer_variant_copies`] fan-out. A transitive-only package + /// [`find_store_peer_variant_copies`] fan-out, and a caller that checks + /// every copy without applying (`vex`) adds them with + /// [`with_store_peer_variant_copies`]. A transitive-only package /// that lives ONLY in the store is still resolved (its store copies are - /// probed because no importer-tree copy was found). + /// probed because no importer-tree copy was found). A copy BUNDLED + /// inside another package's store entry is always returned, found or + /// not elsewhere: no fan-out reaches it (#601). pub async fn find_by_purls( &self, node_modules_path: &Path, @@ -1328,11 +1332,24 @@ impl NpmCrawler { for nested in visit.nested { match nested { NestedNodeModules::Dir(dir) => next_level.push((dir, false)), - NestedNodeModules::StoreEntries(entries) => next_level.extend( - Self::pending_store_entries(entries, filter) + NestedNodeModules::StoreEntries(entries) => { + let (probed, skipped): (Vec, Vec) = entries .into_iter() - .map(|dir| (dir, true)), - ), + .partition(|entry| Self::store_entry_may_hold(entry, filter)); + next_level.extend(probed.into_iter().map(|e| (e.node_modules, true))); + // A skipped entry's own package can still + // carry a BUNDLED copy of a target that was + // already found elsewhere (#601): Node loads + // that copy for the host, so apply and vex + // need it too. Only the bundled tree is + // walked, and only where one exists. + next_level.extend( + par_map(skipped, Self::skipped_entry_bundled_tree) + .into_iter() + .flatten() + .map(|dir| (dir, false)), + ); + } } } } @@ -1565,17 +1582,41 @@ impl NpmCrawler { entries: Vec, pending_names: Option<&HashSet<&str>>, ) -> Vec { - let mut out = Vec::new(); - for entry in entries { - if let (Some(filter), Some((entry_pkg, _version))) = (pending_names, &entry.advertised) - { - if !filter.contains(entry_pkg.as_str()) { - continue; - } - } - out.push(entry.node_modules); + entries + .into_iter() + .filter(|entry| Self::store_entry_may_hold(entry, pending_names)) + .map(|entry| entry.node_modules) + .collect() + } + + /// Whether [`Self::pending_store_entries`] keeps `entry`: no filter, an + /// undecodable name, or an advertised package that is still pending. + fn store_entry_may_hold(entry: &StoreEntry, pending_names: Option<&HashSet<&str>>) -> bool { + match (pending_names, &entry.advertised) { + (Some(filter), Some((entry_pkg, _version))) => filter.contains(entry_pkg.as_str()), + _ => true, + } + } + + /// The bundled-dependency tree of a store entry the pending-name filter + /// skipped: `/node_modules//node_modules`, the same + /// dir an unfiltered visit of the entry would enqueue. The entry's own + /// package is the one its name advertises, a real dir there (pnpm, vlt, + /// Bun, Deno) or a link to the entry's `package` dir (Yarn 4). One stat + /// for an entry without bundled dependencies, which is nearly all of + /// them. + fn skipped_entry_bundled_tree(entry: StoreEntry) -> Option { + let (own_name, _version) = entry.advertised?; + if !own_name.split('/').all(is_safe_npm_component) { + return None; } - out + let own = if is_real_package_dir_sync(&entry.node_modules, &own_name) { + entry.node_modules.join(&own_name) + } else { + store_entry_own_package_sync(&entry.node_modules, &own_name)? + }; + let nested = own.join("node_modules"); + is_dir_sync(&nested).then_some(nested) } // ------------------------------------------------------------------ @@ -2692,6 +2733,31 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { copies } +/// `paths` plus every store variant of each (a pnpm peer suffix, a Deno +/// copy index, a vlt peer or modifier extra, a vlt registry-alias instance +/// of the same `name@version`), deduped by canonical path. This is the +/// copy set `apply` writes: [`NpmCrawler::find_by_purls`] resolves a store +/// copy only for a package with no importer copy and leaves the variants +/// to apply's [`find_store_peer_variant_copies`] fan-out, but each variant +/// is what some dependent loads, so a check of "every installed copy" +/// (`vex`) must see them too. +pub async fn with_store_peer_variant_copies(paths: Vec) -> Vec { + let mut seen: HashSet = HashSet::new(); + for path in &paths { + seen.insert(tokio::fs::canonicalize(path).await.unwrap_or(path.clone())); + } + let mut out = paths.clone(); + for path in &paths { + for copy in find_store_peer_variant_copies(path).await { + let canonical = tokio::fs::canonicalize(©).await.unwrap_or(copy.clone()); + if seen.insert(canonical) { + out.push(copy); + } + } + } + out +} + // --------------------------------------------------------------------------- // Utility // --------------------------------------------------------------------------- From 767f0476543124428b69ba3896d650858ca349cf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:38:59 +0000 Subject: [PATCH 4/6] Drop the now-unused store entry filter helper Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 361f62273..a2f941159 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -1430,7 +1430,7 @@ impl NpmCrawler { /// project. The one exception is pnpm's virtual store (see below), /// whose entries are returned whole: which of them get enqueued is /// decided by the caller's pending-name filter - /// ([`Self::pending_store_entries`]) at replay time. + /// ([`Self::store_entry_may_hold`]) at replay time. /// /// Entries are examined in parallel; their contributions keep listing /// order. @@ -1561,7 +1561,7 @@ impl NpmCrawler { } } - /// The virtual-store entries that can still hold a pending target. + /// Whether a virtual-store entry can still hold a pending target. /// A manifest routinely lists packages that simply aren't installed /// here, and probing every entry of a large monorepo store for them /// would add a readdir+stat storm to every apply/rollback run. The @@ -1576,21 +1576,9 @@ impl NpmCrawler { /// only advertises the entry's OWN package, so a target present solely /// as a bundled dependency INSIDE another package's entry hides behind /// a non-matching name — `find_by_purls`' pass-2 fallback probes every - /// entry for exactly those. Both enumerators only yield entries whose - /// `node_modules` exists, so no re-stat here. - fn pending_store_entries( - entries: Vec, - pending_names: Option<&HashSet<&str>>, - ) -> Vec { - entries - .into_iter() - .filter(|entry| Self::store_entry_may_hold(entry, pending_names)) - .map(|entry| entry.node_modules) - .collect() - } - - /// Whether [`Self::pending_store_entries`] keeps `entry`: no filter, an - /// undecodable name, or an advertised package that is still pending. + /// entry for exactly those. (An entry skipped here still has its own + /// package's bundled tree walked, see + /// [`Self::skipped_entry_bundled_tree`].) fn store_entry_may_hold(entry: &StoreEntry, pending_names: Option<&HashSet<&str>>) -> bool { match (pending_names, &entry.advertised) { (Some(filter), Some((entry_pkg, _version))) => filter.contains(entry_pkg.as_str()), @@ -3989,8 +3977,15 @@ mod tests { ] }; let pending: HashSet<&str> = ["foo", "@s/p"].into_iter().collect(); + let kept = |entries: Vec| -> Vec { + entries + .into_iter() + .filter(|e| NpmCrawler::store_entry_may_hold(e, Some(&pending))) + .map(|e| e.node_modules) + .collect() + }; assert_eq!( - NpmCrawler::pending_store_entries(StoreEntry::vlt(entries()), Some(&pending)), + kept(StoreEntry::vlt(entries())), vec![PathBuf::from("a"), PathBuf::from("b"), PathBuf::from("c")] ); let as_pnpm = entries() @@ -3998,8 +3993,7 @@ mod tests { .map(|(n, p)| (n.into_string().unwrap(), p)) .collect(); assert!( - !NpmCrawler::pending_store_entries(StoreEntry::pnpm(as_pnpm), Some(&pending)) - .contains(&PathBuf::from("a")), + !kept(StoreEntry::pnpm(as_pnpm)).contains(&PathBuf::from("a")), "the pnpm decoder misreads the legacy name" ); } From a513449c43d3f82f1f7da8fa0a92e15a6dafe7f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 20:39:40 +0000 Subject: [PATCH 5/6] Document store copies in the vex every-copy rule Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 69fb09df9..660a7c0ea 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -387,7 +387,7 @@ Recognition rules that hold for every ecosystem: |---|---|---| | Vendored: a lockfile/config wires a `.socket/vendor` artifact, or a live vendor ledger entry | The **committed artifact** is hashed against the record's `afterHash`. The ledger entry is used when it names the wired artifact (it carries the dir-artifact inventory); otherwise an entry is synthesized from the reference. A present installed tree with different bytes only warns `vendored_tree_out_of_sync`. | `(vendored)` | | Hosted: a discovered patch-host reference (or a live pre-v5 redirect-ledger record) | The installed copies the build **consumes** through the hosted wiring are hash-verified when any exist: the Go replacement module, never the pristine `M@v` in the module cache; the Socket-registry cargo source dir; maven's suffixed version. Installed evidence wins: `hash_mismatch` / `not_applied` are omitted. With **nothing installed**, a discovered reference whose lock pins the artifact (or whose format's rewriter never writes a pin) attests from that pin, which is the same evidence as in-run `scan --mode hosted --vex`. A pre-v5 ledger-only record, or a reference whose required pin is missing, stays `package_not_found`. So do purls that `--ecosystems` kept out of the crawl, because "not installed" has to mean the crawler looked. | `(redirected)` | -| Agent: a manifest record with no live hosted/vendored wiring | The installed tree, unchanged. **Every** installed copy the crawler finds for the purl (npm nests duplicates of one `name@version`) must hash to the patched bytes, as `apply` patches every copy. One unpatched copy omits the purl with that copy's tag (`not_applied` / `hash_mismatch`). | none | +| Agent: a manifest record with no live hosted/vendored wiring | The installed tree, unchanged. **Every** installed copy the crawler finds for the purl (npm nests duplicates of one `name@version`; pnpm, vlt, Bun and Deno stores add peer-variant copies and copies bundled inside other packages) must hash to the patched bytes, as `apply` patches every copy. One unpatched copy omits the purl with that copy's tag (`not_applied` / `hash_mismatch`). | none | **Liveness gates.** These gates run before hashing, and `--no-verify` / `--vex-no-verify` skips only the hashing, never the gates: From b92456b37893a09fad16991058ba6aa0d5238bf5 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 2 Oct 2026 18:50:22 -0400 Subject: [PATCH 6/6] Fix npm store traversal cycles and repeated variant scans --- .../src/commands/vex_consumed.rs | 238 +++++++++++++++++- .../src/crawlers/npm_crawler.rs | 151 ++++++++++- .../tests/crawler_npm_e2e.rs | 105 ++++++++ 3 files changed, 488 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index fc69b6738..9cc4d7a5a 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -40,6 +40,7 @@ use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; +#[cfg(not(test))] use socket_patch_core::crawlers::npm_crawler::with_store_peer_variant_copies; use socket_patch_core::crawlers::{ CargoCrawler, CrawlerOptions, Ecosystem, GoCrawler, MavenCrawler, NpmCrawler, @@ -50,6 +51,8 @@ use socket_patch_core::vendor::go_mod_edit::{ }; use socket_patch_core::vendor::lock_inventory::LockIntegrity; use socket_patch_core::vex::HostedCopies; +#[cfg(test)] +use tests::recording_store_variants as with_store_peer_variant_copies; use crate::args::GlobalArgs; use crate::commands::vex_sources::HostedWiring; @@ -61,8 +64,9 @@ use crate::ecosystem_dispatch::{ /// module docs), under the same crawler options and `--ecosystems` scope as /// the installed-tree lookup. `installed` is that lookup's every-copy /// result ([`crate::ecosystem_dispatch::find_manifest_package_copies_reusing`] over -/// the record view, which holds every hosted purl): the shared-location -/// ecosystems read it instead of crawling the tree a second time. `prior` +/// the record view, which holds every hosted purl), including npm store +/// variants. The shared-location ecosystems read it instead of crawling +/// the tree a second time. `prior` /// (embedded hosted `scan --vex` only) is scan's npm crawl of the same /// tree: the alias walk takes its `node_modules` roots and the identity /// fallback its packages instead of walking the tree again. @@ -107,9 +111,33 @@ pub(crate) async fn hosted_consumed_copies( let npm: Vec<&String> = shared.get(&Ecosystem::Npm).into_iter().flatten().collect(); for purl in shared.values().flatten() { let mut paths = all.remove(purl).unwrap_or_default(); - paths.extend(aliases.remove(purl).unwrap_or_default()); - if npm.contains(&purl) { + let extra = aliases.remove(purl).unwrap_or_default(); + if npm.contains(&purl) && installed.get(purl).is_some_and(|p| !p.is_empty()) { + // The installed lookup already expanded these copies. + // Expanding its N variants again scans the store N times. + // Only aliases are new; expand them before merging so a + // different alias/store can still contribute more copies. + if !extra.is_empty() { + let added = with_store_peer_variant_copies(extra).await; + let mut seen = std::collections::HashSet::new(); + for path in &paths { + seen.insert(tokio::fs::canonicalize(path).await.unwrap_or(path.clone())); + } + for path in added { + let canonical = + tokio::fs::canonicalize(&path).await.unwrap_or(path.clone()); + if seen.insert(canonical) { + paths.push(path); + } + } + } + } else if npm.contains(&purl) { + // No installed copies: the identity fallback and aliases + // have not had their store variants enumerated yet. + paths.extend(extra); paths = with_store_peer_variant_copies(paths).await; + } else { + paths.extend(extra); } out.insert( purl.clone(), @@ -576,6 +604,208 @@ async fn maven_copies(options: &CrawlerOptions, purl: &str, wiring: &HostedWirin mod tests { use super::*; + tokio::task_local! { + // Observe real expansion work only in the regression's own task; + // concurrent tests keep calling the production helper normally. + static VARIANT_INPUTS: std::cell::RefCell>>; + } + + pub(super) async fn recording_store_variants(paths: Vec) -> Vec { + let _ = VARIANT_INPUTS.try_with(|calls| calls.borrow_mut().push(paths.clone())); + socket_patch_core::crawlers::npm_crawler::with_store_peer_variant_copies(paths).await + } + + #[cfg(unix)] + async fn tracked_npm_hosted( + common: &GlobalArgs, + installed: &HashMap>, + ) -> (Vec, Vec>) { + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + let hosted = BTreeMap::from([( + purl.clone(), + HostedWiring { + uuid: "11111111-1111-4111-8111-111111111111".to_string(), + refs: Vec::new(), + }, + )]); + VARIANT_INPUTS + .scope(std::cell::RefCell::new(Vec::new()), async { + let mut found = hosted_consumed_copies(common, &hosted, installed, None).await; + let paths = found.remove(&purl).unwrap().paths; + let calls = VARIANT_INPUTS.with(|inputs| inputs.borrow().clone()); + (paths, calls) + }) + .await + } + + #[cfg(unix)] + fn peer_copies(store: &Path, count: usize) -> Vec { + (0..count) + .map(|i| { + let path = store.join(format!( + "left-pad@1.3.0(peer@1.0.{i})/node_modules/left-pad" + )); + pkg(&path, "left-pad", "1.3.0"); + path + }) + .collect() + } + + #[cfg(unix)] + #[tokio::test] + async fn hosted_reuses_expanded_npm_copies_and_merges_alias_variants() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().canonicalize().unwrap().join("node_modules"); + let peers = peer_copies(&nm.join(".pnpm"), 8); + std::os::unix::fs::symlink(&peers[0], nm.join("left-pad")).unwrap(); + let common = GlobalArgs { + cwd: tmp.path().canonicalize().unwrap(), + ecosystems: Some(vec!["npm".to_string()]), + ..GlobalArgs::default() + }; + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + let installed = crate::ecosystem_dispatch::find_manifest_package_copies_reusing( + std::slice::from_ref(&purl), + &common, + true, + None, + ) + .await; + assert_eq!(installed[&purl].len(), peers.len()); + let (paths, calls) = tracked_npm_hosted(&common, &installed).await; + assert_eq!(paths, installed[&purl]); + assert!( + calls.is_empty(), + "already-expanded copies were rescanned: {calls:?}" + ); + + // A real alias is absent from the name-keyed installed set. Its + // store variants overlap that set canonically, including the + // importer link's physical copy; keep the alias once and preserve + // the original importer-first path choices. + let alias = nm.join("lp"); + pkg(&alias, "left-pad", "1.3.0"); + let (paths, calls) = tracked_npm_hosted(&common, &installed).await; + assert_eq!(calls, vec![vec![alias.clone()]]); + let mut expected = installed[&purl].clone(); + expected.push(alias); + assert_eq!(paths, expected); + + // An alias beneath a real nested host can reach another store. + // The installed root copy makes the name-keyed resolver skip + // those peers, so alias expansion must still add them even when + // installed copies are already present. + let host = nm.join("host"); + pkg(&host, "host", "1.0.0"); + let host_nm = host.join("node_modules"); + let nested_peers = peer_copies(&host_nm.join(".pnpm"), 2); + let nested_alias = host_nm.join("lp"); + pkg(&nested_alias, "left-pad", "1.3.0"); + let installed_again = crate::ecosystem_dispatch::find_manifest_package_copies_reusing( + std::slice::from_ref(&purl), + &common, + true, + None, + ) + .await; + assert_eq!(installed_again, installed); + let (paths, calls) = tracked_npm_hosted(&common, &installed_again).await; + assert_eq!(calls.len(), 1); + let mut inputs = calls[0].clone(); + inputs.sort(); + let mut aliases = vec![nm.join("lp"), nested_alias.clone()]; + aliases.sort(); + assert_eq!(inputs, aliases); + assert_eq!(&paths[..installed[&purl].len()], installed[&purl]); + expected.push(nested_alias); + expected.extend(nested_peers); + let mut actual = paths.clone(); + actual.sort(); + expected.sort(); + assert_eq!(actual, expected); + assert_eq!( + paths + .iter() + .map(|path| path.canonicalize().unwrap()) + .collect::>() + .len(), + paths.len() + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn hosted_expands_alias_only_copies() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp + .path() + .canonicalize() + .unwrap() + .join("node_modules/.pnpm"); + let peers = peer_copies(&store, 2); + // Run within a store package whose nested dependency is an alias. + // The sibling peer copies are outside its project-root search. + let root = store.join("host@1.0.0/node_modules/host"); + let alias = root.join("node_modules/lp"); + pkg(&alias, "left-pad", "1.3.0"); + let common = GlobalArgs { + cwd: root, + ecosystems: Some(vec!["npm".to_string()]), + ..GlobalArgs::default() + }; + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + let installed = crate::ecosystem_dispatch::find_manifest_package_copies_reusing( + std::slice::from_ref(&purl), + &common, + true, + None, + ) + .await; + assert!(installed.is_empty(), "{installed:?}"); + let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await; + assert_eq!(calls, vec![vec![alias.clone()]]); + let mut expected = peers; + expected.push(alias); + paths.sort(); + expected.sort(); + assert_eq!(paths, expected); + } + + #[cfg(unix)] + #[tokio::test] + async fn hosted_expands_identity_fallback_with_empty_installed_entry() { + let tmp = tempfile::tempdir().unwrap(); + let peers = peer_copies( + &tmp.path() + .canonicalize() + .unwrap() + .join("external/node_modules/.pnpm"), + 2, + ); + let root = tmp.path().canonicalize().unwrap().join("project"); + let alias = root.join("node_modules/lp"); + std::fs::create_dir_all(alias.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&peers[0], &alias).unwrap(); + let common = GlobalArgs { + cwd: root, + ecosystems: Some(vec!["npm".to_string()]), + ..GlobalArgs::default() + }; + let purl = "pkg:npm/left-pad@1.3.0".to_string(); + assert!( + npm_alias_copies(&common.crawler_options(), std::slice::from_ref(&purl)) + .await + .is_empty() + ); + 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()]; + paths.sort(); + expected.sort(); + assert_eq!(paths, expected); + } + fn pkg(dir: &Path, name: &str, version: &str) { std::fs::create_dir_all(dir).unwrap(); std::fs::write( diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index a2f941159..7d33acbf3 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -1289,7 +1289,17 @@ impl NpmCrawler { // Each dir is tagged `true` when it is a pnpm or vlt store entry's // `node_modules`. let mut level: Vec<(PathBuf, bool)> = vec![(node_modules_path.to_path_buf(), false)]; + let mut visited = HashSet::new(); while !level.is_empty() { + // A bundled node_modules may link back to an ancestor (or to + // another already-visited tree). Keep the first/root-first + // spelling without traversing the same physical tree again. + // Store and importer visits have different link policies, so + // retain both modes; each resolution pass gets its own set. + level.retain(|(path, store_entry)| { + let canonical = std::fs::canonicalize(path).unwrap_or_else(|_| path.clone()); + visited.insert((canonical, *store_entry)) + }); let visits: Vec = par_map(level, |(nm_path, store_entry)| { Self::visit_resolver_dir(nm_path, store_entry, &pending) }); @@ -2499,7 +2509,7 @@ impl Default for NpmCrawler { // --------------------------------------------------------------------------- /// Which store layout a candidate store directory uses. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] enum StoreLayout { Pnpm, Bun, @@ -2564,6 +2574,17 @@ impl StoreLayout { /// which breaks content-store hardlinks per copy — CoW safety holds for /// every copy independently. pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { + find_store_peer_variant_copies_reusing(pkg_path, &mut HashSet::new()).await +} + +/// Discover candidate stores for every input path, but enumerate a +/// physical store only once per layout and package identity in one +/// aggregate expansion. Alias ancestry can expose additional stores even +/// when the input's canonical package was already seen. +async fn find_store_peer_variant_copies_reusing( + pkg_path: &Path, + scanned: &mut HashSet<(PathBuf, StoreLayout, String, String)>, +) -> Vec { // 1. Candidate stores from both ancestor chains (cheap stats only — // no file reads until a store is actually found). let canonical_pkg = tokio::fs::canonicalize(pkg_path).await.ok(); @@ -2666,6 +2687,16 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { let mut copies: Vec = Vec::new(); let mut seen_copies: HashSet = HashSet::new(); for (layout, store) in stores { + let canonical_store = tokio::fs::canonicalize(&store) + .await + .unwrap_or_else(|_| store.clone()); + if !scanned.insert((canonical_store, layout, full_name.clone(), version.clone())) { + continue; + } + #[cfg(test)] + let _ = tests::VARIANT_STORE_SCANS.try_with(|scans| { + scans.borrow_mut().push(store.clone()); + }); let entries = match layout { StoreLayout::Pnpm | StoreLayout::Bun | StoreLayout::Deno => { NpmCrawler::list_pnpm_shaped_store_entries(&store, layout).await @@ -2735,8 +2766,11 @@ pub async fn with_store_peer_variant_copies(paths: Vec) -> Vec seen.insert(tokio::fs::canonicalize(path).await.unwrap_or(path.clone())); } let mut out = paths.clone(); + // `out` already holds every primary, including the primary excluded + // by a store's first scan. Sharing scan state therefore loses no copy. + let mut scanned = HashSet::new(); for path in &paths { - for copy in find_store_peer_variant_copies(path).await { + for copy in find_store_peer_variant_copies_reusing(path, &mut scanned).await { let canonical = tokio::fs::canonicalize(©).await.unwrap_or(copy.clone()); if seen.insert(canonical) { out.push(copy); @@ -2834,6 +2868,20 @@ fn is_safe_npm_component(component: &str) -> bool { mod tests { use super::*; + tokio::task_local! { + pub(super) static VARIANT_STORE_SCANS: std::cell::RefCell>; + } + + async fn tracked_store_expansion(paths: Vec) -> (Vec, Vec) { + VARIANT_STORE_SCANS + .scope(std::cell::RefCell::new(Vec::new()), async { + let expanded = with_store_peer_variant_copies(paths).await; + let scans = VARIANT_STORE_SCANS.with(|scans| scans.borrow().clone()); + (expanded, scans) + }) + .await + } + fn listing_of(names: &[&str], complete: bool) -> Listing { Listing { entries: names @@ -4189,6 +4237,105 @@ mod tests { } } + #[tokio::test] + async fn test_store_expansion_scans_transitive_peer_store_once() { + let tmp = tempfile::tempdir().unwrap(); + let nm = tmp.path().canonicalize().unwrap().join("node_modules"); + let store = nm.join(".pnpm"); + let peers: Vec<_> = (0..8) + .map(|i| store.join(format!("foo@1.0.0(peer@1.0.{i})/node_modules/foo"))) + .collect(); + for path in &peers { + write_pkg(path, "foo", "1.0.0"); + } + // Without an importer link the resolver already returns every + // peer. Agent VEX passes this entire set to variant expansion. + let purl = "pkg:npm/foo@1.0.0".to_string(); + let found = NpmCrawler::new() + .find_by_purls(&nm, std::slice::from_ref(&purl)) + .await + .unwrap(); + let paths: Vec<_> = found[&purl].iter().map(|pkg| pkg.path.clone()).collect(); + assert_eq!(paths.len(), peers.len()); + let (expanded, scans) = tracked_store_expansion(paths.clone()).await; + assert_eq!( + expanded, paths, + "all original copies and their order survive" + ); + assert_eq!( + scans.len(), + 1, + "one enumeration per store/identity: {scans:?}" + ); + } + + #[tokio::test] + async fn test_store_expansion_keeps_distinct_identities_and_alias_stores() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let store = root.join("main/node_modules/.pnpm"); + let mut inputs = Vec::new(); + let mut expected = HashSet::new(); + for (name, version) in [("foo", "1.0.0"), ("foo", "2.0.0"), ("bar", "1.0.0")] { + for i in 0..2 { + let path = store.join(format!( + "{name}@{version}(peer@1.0.{i})/node_modules/{name}" + )); + write_pkg(&path, name, version); + expected.insert(path.canonicalize().unwrap()); + if i == 0 { + inputs.push(path); + } + } + } + let alias_nm = root.join("alias/node_modules"); + let alias_store = alias_nm.join(".pnpm"); + for i in 0..2 { + let path = alias_store.join(format!("foo@1.0.0(peer@1.0.{i})/node_modules/foo")); + write_pkg(&path, "foo", "1.0.0"); + expected.insert(path.canonicalize().unwrap()); + } + let alias = alias_nm.join("foo"); + link_dir(&inputs[0], &alias); + inputs.push(alias); + // Another lexical route to the same primary AND alias store. + // Candidate discovery must run, but this physical store/identity + // has already been scanned through the preceding alias. + let linked_nm = root.join("linked/node_modules"); + std::fs::create_dir_all(&linked_nm).unwrap(); + link_dir(&alias_store, &linked_nm.join(".pnpm")); + let linked_alias = linked_nm.join("foo"); + link_dir(&inputs[0], &linked_alias); + inputs.push(linked_alias); + + let (expanded, scans) = tracked_store_expansion(inputs.clone()).await; + assert_eq!(&expanded[..inputs.len()], inputs); + assert_eq!( + expanded.len(), + expected.len() + 2, + "retain initial alias paths" + ); + assert_eq!( + expanded + .iter() + .map(|p| p.canonicalize().unwrap()) + .collect::>(), + expected + ); + let scans: Vec<_> = scans.iter().map(|p| p.canonicalize().unwrap()).collect(); + assert_eq!(scans.iter().filter(|p| **p == store).count(), 3); + assert_eq!(scans.iter().filter(|p| **p == alias_store).count(), 1); + + // Standalone apply/rollback discovery gets a fresh scan and still + // excludes only its own primary copy. + let copies = find_store_peer_variant_copies(&inputs[0]).await; + assert_eq!(copies.len(), 1); + assert_ne!( + copies[0].canonicalize().unwrap(), + inputs[0].canonicalize().unwrap() + ); + } + /// D19: a vendored copy's `.socket/vendor/npm///node_modules` /// is never a crawl root (hidden dirs are skipped), so the only /// inventory entry is the importer link that points at it. diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index c49b3ad12..cef842932 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -2390,6 +2390,111 @@ async fn find_by_purls_returns_bundled_copy_of_an_already_found_target() { } } +/// Skipped store entries can link their bundled tree back to the importer. +/// Bound this regression in a child process: a broken walk must fail the +/// test instead of leaving a blocking-pool traversal running indefinitely. +#[cfg(unix)] +#[test] +fn find_by_purls_bounds_bundled_store_cycles_and_keeps_linked_copies() { + const CHILD: &str = "SOCKET_TEST_BUNDLED_STORE_CYCLE_CHILD"; + if std::env::var_os(CHILD).is_none() { + let mut child = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "find_by_purls_bounds_bundled_store_cycles_and_keeps_linked_copies", + "--nocapture", + ]) + .env(CHILD, "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + let timed_out = loop { + if child.try_wait().unwrap().is_some() { + break false; + } + if std::time::Instant::now() >= deadline { + child.kill().unwrap(); + break true; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + }; + let output = child.wait_with_output().unwrap(); + assert!( + !timed_out && output.status.success(), + "cycle resolution timed_out={timed_out}: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + return; + } + tokio::runtime::Runtime::new().unwrap().block_on(async { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let nm = root.join("node_modules"); + stage_npm_pkg(&nm, "target", "1.0.0").await; + for host in ["host-a", "host-b"] { + let entry_nm = nm.join(format!(".pnpm/{host}@1.0.0/node_modules")); + stage_npm_pkg(&entry_nm, host, "1.0.0").await; + std::os::unix::fs::symlink(&nm, entry_nm.join(host).join("node_modules")).unwrap(); + } + // This link reaches another physical copy, rather than an ancestor; + // cycle protection must not discard legitimate linked nested trees. + let linked = root.join("linked-bundle"); + stage_npm_pkg(&linked, "target", "1.0.0").await; + let entry_nm = nm.join(".pnpm/host-c@1.0.0/node_modules"); + stage_npm_pkg(&entry_nm, "host-c", "1.0.0").await; + std::os::unix::fs::symlink(&linked, entry_nm.join("host-c/node_modules")).unwrap(); + let purl = "pkg:npm/target@1.0.0".to_string(); + let found = NpmCrawler + .find_by_purls(&nm, std::slice::from_ref(&purl)) + .await + .unwrap(); + let copies = &found[&purl]; + assert_eq!(copies.len(), 2); + assert_eq!(copies[0].path, nm.join("target")); + assert_eq!( + std::fs::canonicalize(&copies[1].path).unwrap(), + linked.join("target") + ); + assert_ne!( + std::fs::canonicalize(&copies[0].path).unwrap(), + std::fs::canonicalize(&copies[1].path).unwrap() + ); + }); +} + +/// The same physical directory can be a store entry and an ordinary +/// nested tree. The latter may resolve a dependency link that the former +/// intentionally skips; deduplication must preserve both visit policies. +#[cfg(unix)] +#[tokio::test] +async fn find_by_purls_preserves_importer_mode_after_store_visit() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let nm = root.join("node_modules"); + let physical = root.join("physical"); + stage_npm_pkg(&physical, "only-linked", "1.0.0").await; + let opaque_nm = nm.join(".pnpm/opaque/node_modules"); + std::fs::create_dir_all(&opaque_nm).unwrap(); + std::os::unix::fs::symlink(physical.join("only-linked"), opaque_nm.join("only-linked")) + .unwrap(); + let host_nm = nm.join(".pnpm/host@1.0.0/node_modules"); + stage_npm_pkg(&host_nm, "host", "1.0.0").await; + std::os::unix::fs::symlink(&opaque_nm, host_nm.join("host/node_modules")).unwrap(); + let purl = "pkg:npm/only-linked@1.0.0".to_string(); + let found = NpmCrawler + .find_by_purls(&nm, std::slice::from_ref(&purl)) + .await + .unwrap(); + assert_eq!(found[&purl].len(), 1); + assert_eq!( + std::fs::canonicalize(&found[&purl][0].path).unwrap(), + physical.join("only-linked") + ); +} + // ── vlt store (.vlt), staged from captured real layouts ──────── /// The eras with a captured `vlt install` layout under