diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index c0881066c..68061d61a 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. The same goes for npm purls when an installed pnpm tree records its virtual store outside the project (`enableGlobalVirtualStore`, or a `virtualStoreDir` that climbs out): transitive deps there are invisible to the crawler. A pnpm `modulesDir` inside the project is crawled. | `(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: diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index 0d1e79a19..b57d475fb 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -40,7 +40,8 @@ use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; -use socket_patch_core::crawlers::npm_crawler::find_store_peer_variant_copies; +#[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. @@ -109,13 +113,38 @@ pub(crate) async fn hosted_consumed_copies( let mut paths = all.remove(purl).unwrap_or_default(); // The installed-tree lookup already resolves importer-tree // aliases, so most of the walk's finds are in `paths` already. - for alias in aliases.remove(purl).unwrap_or_default() { - if !paths.contains(&alias) { - paths.push(alias); + let extra: Vec = aliases + .remove(purl) + .unwrap_or_default() + .into_iter() + .filter(|alias| !paths.contains(alias)) + .collect(); + 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); + } + } } - } - if npm.contains(&purl) { - paths = with_store_variants(paths).await; + } 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(), @@ -321,28 +350,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 @@ -604,6 +611,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( @@ -857,7 +1066,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"), @@ -866,7 +1075,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 15eb29225..cb9c9f134 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-cli/tests/apply/in_process_npm_multicopy.rs b/crates/socket-patch-cli/tests/apply/in_process_npm_multicopy.rs index 203e60e5a..2e97747d9 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 @@ -398,6 +398,86 @@ fn apply_and_rollback_reach_both_transitive_only_vlt_store_copies() { 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"), + ); + } +} + /// #626: a `node_modules/` link to first-party source (an npm /// workspace member, which a `file:` directory dependency lays out the /// same way) that shares a patched package's `name@version` is the user's diff --git a/crates/socket-patch-cli/tests/e2e_vex.rs b/crates/socket-patch-cli/tests/e2e_vex.rs index 8d53bdb10..7a735d568 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}"); + } +} + /// Regression (#356): an npm alias install (`node_modules/lp` holding the /// real `dup-pkg@1.0.0`) is an installed copy of `pkg:npm/dup-pkg@1.0.0`. /// `vex` used to verify only `node_modules/dup-pkg`, so it attested the diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 8a1f3823f..d6eb7637b 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -1273,9 +1273,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, @@ -1393,7 +1397,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) }); @@ -1436,11 +1450,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)), + ); + } } } } @@ -1589,7 +1616,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. @@ -1720,7 +1747,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 @@ -1735,23 +1762,35 @@ 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 { - 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); + /// 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()), + _ => 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) } // ------------------------------------------------------------------ @@ -2646,7 +2685,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, @@ -2711,6 +2750,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(); @@ -2813,6 +2863,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 @@ -2868,6 +2928,34 @@ 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(); + // `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_reusing(path, &mut scanned).await { + let canonical = tokio::fs::canonicalize(©).await.unwrap_or(copy.clone()); + if seen.insert(canonical) { + out.push(copy); + } + } + } + out +} + // --------------------------------------------------------------------------- // Utility // --------------------------------------------------------------------------- @@ -2956,6 +3044,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 @@ -4203,8 +4305,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() @@ -4212,8 +4321,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" ); } @@ -4409,6 +4517,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 226b9b474..cef842932 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -2336,6 +2336,165 @@ 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}"); + } + } +} + +/// 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