diff --git a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs index 6d5383721..f51c3685b 100644 --- a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs +++ b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs @@ -1118,6 +1118,144 @@ async fn yarn_berry_node_modules_linker_apply_patches_file() { assert_patched(&ms_index, &patched, &before_hash, &after_hash); } +// --------------------------------------------------------------------------- +// npm workspace members named like build dirs (hand-built npm layout) +// --------------------------------------------------------------------------- + +/// The layout `npm install` writes for the #1627 repro: the root pins +/// `ms@2.1.3` and the members `packages/app` and `packages/build` (or +/// `dist`, ...) each need `ms@2.0.0`, which npm can't hoist and installs +/// into each member's own `node_modules`. Returns the two member copies' +/// `index.js`, the patched bytes and the record hashes. +fn stage_skip_dir_member_workspace( + root: &Path, + member: &str, +) -> (PathBuf, PathBuf, Vec, String, String) { + std::fs::write( + root.join("package.json"), + r#"{"name":"root","version":"1.0.0","private":true,"workspaces":["packages/*"],"dependencies":{"ms":"2.1.3"}}"#, + ) + .unwrap(); + let stage_ms = |nm: PathBuf, version: &str| { + let dir = nm.join("ms"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!(r#"{{"name":"ms","version":"{version}"}}"#), + ) + .unwrap(); + std::fs::write( + dir.join("index.js"), + format!("module.exports = '{version}';\n"), + ) + .unwrap(); + dir.join("index.js") + }; + stage_ms(root.join("node_modules"), "2.1.3"); + let mut copies = Vec::new(); + for name in ["app", member] { + let dir = root.join("packages").join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!(r#"{{"name":"{name}","version":"1.0.0","dependencies":{{"ms":"2.0.0"}}}}"#), + ) + .unwrap(); + copies.push(stage_ms(dir.join("node_modules"), "2.0.0")); + } + let original = std::fs::read(&copies[0]).unwrap(); + let before_hash = git_sha256(&original); + let mut patched = original; + patched.extend_from_slice(b"// socket-patched\n"); + let after_hash = git_sha256(&patched); + let socket = root.join(".socket"); + write_manifest(&socket, "pkg:npm/ms@2.0.0", &before_hash, &after_hash); + // A vulnerability, so `vex` has a statement to make. + let manifest = std::fs::read_to_string(socket.join("manifest.json")).unwrap(); + let manifest = manifest.replacen( + r#""vulnerabilities": {}"#, + r#""vulnerabilities": { "GHSA-skip-dir-member": { "cves": ["CVE-2026-1627"], + "summary": "s", "severity": "high", "description": "d" } }"#, + 1, + ); + std::fs::write(socket.join("manifest.json"), manifest).unwrap(); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write(socket.join("blobs").join(&after_hash), &patched).unwrap(); + let member_copy = copies.pop().unwrap(); + ( + copies.pop().unwrap(), + member_copy, + patched, + before_hash, + after_hash, + ) +} + +/// #1627: agent-mode apply patches the nested copy of a declared workspace +/// member whose directory is named `build`, `dist`, `vendor`, `tmp`, `temp` +/// or `coverage`, not just the other members' copies. +#[tokio::test] +#[serial] +async fn apply_patches_workspace_member_named_like_a_build_dir() { + for member in ["build", "dist", "vendor", "tmp", "temp", "coverage"] { + let tmp = tempfile::tempdir().unwrap(); + let (app_copy, member_copy, patched, before_hash, after_hash) = + stage_skip_dir_member_workspace(tmp.path(), member); + + let code = apply_run(default_apply(tmp.path())).await; + assert_eq!(code, 0, "apply (member packages/{member})"); + assert_patched(&app_copy, &patched, &before_hash, &after_hash); + assert_patched(&member_copy, &patched, &before_hash, &after_hash); + } +} + +/// #1627: with only the other member's copy patched, `apply --check` +/// reports the `packages/build` copy as drift (exit 1), and `vex` attests +/// nothing as `not_affected` until that copy is patched too. +#[tokio::test] +#[serial] +async fn apply_check_and_vex_see_unpatched_member_named_like_a_build_dir() { + let tmp = tempfile::tempdir().unwrap(); + let (app_copy, member_copy, patched, _, _) = + stage_skip_dir_member_workspace(tmp.path(), "build"); + std::fs::write(&app_copy, &patched).unwrap(); + assert_ne!(std::fs::read(&member_copy).unwrap(), patched); + + let mut args = default_apply(tmp.path()); + args.check = true; + let code = apply_run(args).await; + assert_eq!( + code, 1, + "apply --check must report the unpatched member copy" + ); + + let vex_path = tmp.path().join("vex.json"); + let vex = || async { + let _ = std::fs::remove_file(&vex_path); + let code = socket_patch_cli::commands::vex::run(socket_patch_cli::commands::vex::VexArgs { + common: default_apply(tmp.path()).common, + output: Some(vex_path.clone()), + product: Some("pkg:npm/root@1.0.0".to_string()), + no_verify: false, + doc_id: None, + compact: false, + }) + .await; + (code, std::fs::read_to_string(&vex_path).unwrap_or_default()) + }; + let (code, doc) = vex().await; + assert!( + !doc.contains("not_affected"), + "VEX must not attest not_affected while packages/build's copy is unpatched \ + (exit {code}):\n{doc}" + ); + // The control: with both copies patched, the same run attests it. + std::fs::write(&member_copy, &patched).unwrap(); + let (code, doc) = vex().await; + assert_eq!(code, 0, "{doc}"); + assert!(doc.contains("not_affected"), "{doc}"); +} + // --------------------------------------------------------------------------- // Rush pnpm symlink farm (hand-built, no real installer) // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index c21756c8e..4069ea988 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -65,6 +65,147 @@ pub(super) fn configured_install_roots(start_path: &Path) -> Vec { roots } +/// The `node_modules` dirs of declared workspace members that the +/// workspace walk prunes by name (#1627): members whose path crosses a +/// [`SKIP_DIRS`] name (`packages/build`, `vendor/lib`). npm, yarn and Bun +/// install a member's unhoistable deps into `/node_modules` from +/// the root lock, so such a copy is as live as any other member's. The +/// workspace roots are `start_path` and the project of every `walked` +/// root (`/node_modules`), so a workspace nested below the scan +/// root counts too. Sorted and deduplicated. +pub(super) fn skipped_workspace_member_roots( + start_path: &Path, + walked: &[PathBuf], +) -> Vec { + let mut projects = vec![start_path]; + projects.extend( + walked + .iter() + .filter(|nm| nm.file_name().is_some_and(|n| n == "node_modules")) + .filter_map(|nm| nm.parent()), + ); + let mut roots: Vec = projects + .into_iter() + .collect::>() + .into_iter() + .flat_map(skipped_members_of) + .collect(); + roots.sort(); + roots.dedup(); + roots +} + +/// [`skipped_workspace_member_roots`] for the one workspace root `root`. A +/// member is a dir the root `package.json` `workspaces` patterns list in +/// any package manager's reading ([`WorkspacesDialect::ALL`]: an extra root +/// can only keep a copy, never drop one) that holds a `package.json`. Each +/// pattern is walked from its literal prefix to the depth its segments +/// allow (any depth for `**`, within [`WORKSPACE_GLOB_DIR_BUDGET`] dirs per +/// pattern), with the walk's own rules: real dirs only (never a symlink), +/// never a hidden dir, a `node_modules` or a tagged cache dir below the +/// root, literal prefix included. Names are +/// read from the listings, so the [`SKIP_DIRS`] test sees each dir as the +/// walk does even when a pattern spells it in another case. +fn skipped_members_of(root: &Path) -> Vec { + use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; + + let Some(patterns) = crate::utils::fs::read_regular_to_string_sync(&root.join("package.json")) + .ok() + .and_then(|text| crate::hosted::governing_root::workspace_patterns(&text)) + else { + return Vec::new(); + }; + let walkable = |entry: &ListedEntry| { + entry.file_type.is_some_and(|ft| ft.is_dir()) + && entry.name_str != "node_modules" + && !entry.name_str.starts_with('.') + }; + let mut roots = Vec::new(); + // A leading `!` is a negation in some readings and part of an extglob + // or a literal name in others; the dirs it can reach are walked either + // way, and `workspaces_include_as` decides. + for pattern in &patterns { + let segments: Vec<&str> = pattern + .trim_start_matches('!') + .split(['/', '\\']) + .filter(|s| !s.is_empty() && *s != ".") + .collect(); + let is_glob = |s: &str| s.contains(['*', '?', '[', ']', '{', '}', '(', ')', '!', '+', '@']); + let literal = segments.iter().take_while(|s| !is_glob(s)).count(); + // The literal prefix, each component as its parent lists it (an + // exact name first, else the ASCII-case-insensitive match a + // case-insensitive filesystem resolves). A prefix that climbs out + // (`..`) or is not walkable names nothing here. + let mut dir = root.to_path_buf(); + let mut rel: Vec = Vec::new(); + let mut reachable = true; + for segment in &segments[..literal] { + let entries: Vec = list_dir_sync(&dir) + .entries + .into_iter() + .filter(|e| walkable(e)) + .collect(); + let found = entries.iter().find(|e| e.name_str == *segment).or_else(|| { + entries + .iter() + .find(|e| e.name_str.eq_ignore_ascii_case(segment)) + }); + let Some(entry) = found else { + reachable = false; + break; + }; + dir.push(&entry.name); + rel.push(entry.name_str.clone()); + if is_tagged_cache_dir(&dir, &list_dir_sync(&dir)) { + reachable = false; + break; + } + } + if !reachable { + continue; + } + let rest = &segments[literal..]; + let max_depth = if rest.contains(&"**") { + usize::MAX + } else { + rest.len() + }; + let mut budget = WORKSPACE_GLOB_DIR_BUDGET; + let mut stack = vec![(dir, rel, 0usize)]; + while let Some((dir, rel, depth)) = stack.pop() { + let listing = list_dir_sync(&dir); + // The workspace root itself is exempt, as the walk always + // crawls the scan root (and never reaches a nested project + // inside a tagged dir). + if !rel.is_empty() && is_tagged_cache_dir(&dir, &listing) { + continue; + } + if rel.iter().any(|seg| SKIP_DIRS.contains(&seg.as_str())) + && WorkspacesDialect::ALL + .iter() + .any(|&dialect| workspaces_include_as(&patterns, &rel, dialect)) + && dir.join("package.json").is_file() + && has_node_modules_dir(&dir, &listing) + { + roots.push(dir.join("node_modules")); + } + if depth >= max_depth { + continue; + } + for entry in listing.entries.into_iter().filter(|e| walkable(e)) { + let Some(left) = budget.checked_sub(1) else { + break; + }; + budget = left; + let mut child_rel = rel.clone(); + child_rel.push(entry.name_str.clone()); + stack.push((dir.join(&entry.name), child_rel, depth + 1)); + } + } + } + roots +} + /// Whether the installed pnpm tree of the project at `project` keeps its /// virtual store where the crawler does not look: a `.modules.yaml` in /// `node_modules` or a pnpm modules dir @@ -2892,6 +3033,9 @@ impl NpmCrawler { // Recursively search for workspace node_modules results.extend(Self::find_workspace_node_modules(start_path, listing)); + // Plus the declared members the walk prunes by name (#1627). + let members = skipped_workspace_member_roots(start_path, &results); + results.extend(members); merge_configured_install_roots(results, configured_install_roots(start_path)) } diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs index 2d26df0b9..5663ba351 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs @@ -10,8 +10,9 @@ use std::path::{Path, PathBuf}; use super::{ build_npm_purl, configured_install_roots, is_legacy_pnpm_store_dir_name, is_safe_npm_component, - merge_configured_install_roots, parse_package_name, read_package_json, NpmCrawler, StoreEntry, - Target, NESTED_STORE_MAX_DEPTH, NESTED_STORE_MAX_DIRS, SKIP_DIRS, VLT_STORE_NAME, + merge_configured_install_roots, parse_package_name, read_package_json, + skipped_workspace_member_roots, NpmCrawler, StoreEntry, Target, NESTED_STORE_MAX_DEPTH, + NESTED_STORE_MAX_DIRS, SKIP_DIRS, VLT_STORE_NAME, }; use crate::crawlers::types::{CrawledPackage, CrawlerOptions}; use crate::utils::fs::is_dir; @@ -493,9 +494,11 @@ impl LegacyNpmCrawler { // Recursively search for workspace node_modules Self::find_workspace_node_modules(start_path, &mut results).await; - - // The package-manager-configured roots are shared with the parent + // The declared members the walk prunes by name (#1627) and the + // package-manager-configured roots are shared with the parent // module (not part of the walk this oracle checks). + let members = skipped_workspace_member_roots(start_path, &results); + results.extend(members); merge_configured_install_roots(results, configured_install_roots(start_path)) } diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index e612a7de2..467d50c69 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -1006,7 +1006,7 @@ fn vlt_workspace_patterns(vlt_json: &str) -> Option>> { /// yarn, Bun) or the object form's `packages` array (yarn classic's /// `nohoist` shape, Bun's catalogs shape). `None` when the field is absent /// or the manifest does not parse. -fn workspace_patterns(package_json: &str) -> Option> { +pub(crate) fn workspace_patterns(package_json: &str) -> Option> { let text = strip_bom(package_json); let doc: serde_json::Value = serde_json::from_str(text).ok()?; let field = doc.get("workspaces")?; diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 6b733b5a8..dda4d2ca4 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -848,6 +848,187 @@ async fn crawl_all_recurses_into_workspace_packages() { ); } +/// Stage a workspace member at `rel` (its `package.json` plus a nested +/// dependency npm could not hoist, named after the member so `crawl_all`, +/// which reports each name@version once, lists every copy). +async fn stage_member(root: &Path, rel: &str) -> PathBuf { + let dir = root.join(rel); + tokio::fs::create_dir_all(&dir).await.unwrap(); + let name = rel.replace('/', "-"); + tokio::fs::write( + dir.join("package.json"), + format!(r#"{{"name":"{name}","version":"1.0.0","dependencies":{{"dep-{name}":"2.0.0"}}}}"#), + ) + .await + .unwrap(); + let dep = format!("dep-{name}"); + stage_npm_pkg(&dir.join("node_modules"), &dep, "2.0.0").await; + dir.join("node_modules").join(dep) +} + +/// #1627: a declared `workspaces` member whose directory carries a +/// SKIP_DIRS name (`build`, `dist`, `vendor`, ...) is still crawled: npm, +/// yarn and Bun install its unhoistable deps into `/node_modules` +/// from the root lock. Each member layout the report lists is covered, +/// plus a literal pattern, a brace pattern and the `{packages: [...]}` +/// field shape. +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_finds_declared_members_with_skip_dir_names() { + for (workspaces, members) in [ + ( + r#"["packages/*"]"#, + &["packages/build", "packages/dist", "packages/lib"][..], + ), + (r#"["vendor/*"]"#, &["vendor/lib"][..]), + (r#"["tools/build"]"#, &["tools/build"][..]), + (r#"["{tools,apps}/*"]"#, &["tools/coverage", "apps/tmp"][..]), + ( + r#"{"packages":["packages/**"]}"#, + &["packages/temp/inner"][..], + ), + // yarn berry reads a leading `!(` as an extglob group, not a negation. + (r#"["!(docs)/*"]"#, &["tools/build"][..]), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + tokio::fs::write( + root.join("package.json"), + format!(r#"{{"name":"root","private":true,"workspaces":{workspaces}}}"#), + ) + .await + .unwrap(); + stage_npm_pkg(&root.join("node_modules"), "ms", "2.1.3").await; + let mut expected = vec![root.join("node_modules").join("ms")]; + for member in members { + expected.push(stage_member(root, member).await); + } + + let result = NpmCrawler.crawl_all(&options_at(root)).await; + let mut got: Vec = result.iter().map(|p| p.path.clone()).collect(); + got.sort(); + expected.sort(); + assert_eq!(got, expected, "workspaces {workspaces}"); + } +} + +/// #1627: the members of a workspace nested below the scan root (found by +/// the walk through its own `node_modules`) count too. +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_finds_skip_dir_members_of_a_nested_workspace() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let web = root.join("web"); + tokio::fs::create_dir_all(&web).await.unwrap(); + tokio::fs::write( + web.join("package.json"), + r#"{"name":"web","private":true,"workspaces":["packages/*"]}"#, + ) + .await + .unwrap(); + stage_npm_pkg(&web.join("node_modules"), "ms", "2.1.3").await; + let member = stage_member(root, "web/packages/dist").await; + + let result = NpmCrawler.crawl_all(&options_at(root)).await; + let mut got: Vec = result.iter().map(|p| p.path.clone()).collect(); + got.sort(); + assert_eq!(got, vec![web.join("node_modules").join("ms"), member]); +} + +/// #1627: the member walk keeps the walk's other rules. A member inside a +/// tagged cache dir stays pruned (through a glob or a literal pattern), and a pattern that spells a member dir +/// in another case than the disk (`packages/build` for `packages/Build`, +/// which the walk does not prune) lists its copy once. +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_member_walk_keeps_the_walk_rules() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + tokio::fs::write( + root.join("package.json"), + r#"{"name":"root","private":true,"workspaces":["cache/*/*","cache/build/lit","packages/build"]}"#, + ) + .await + .unwrap(); + stage_member(root, "cache/build/pkg").await; + stage_member(root, "cache/build/lit").await; + tokio::fs::write( + root.join("cache").join("CACHEDIR.TAG"), + "Signature: 8a477f597d28d172789f06886806bc55\n", + ) + .await + .unwrap(); + let member = stage_member(root, "packages/Build").await; + + let result = NpmCrawler.crawl_all(&options_at(root)).await; + let got: Vec = result.iter().map(|p| p.path.clone()).collect(); + assert_eq!(got, vec![member]); +} + +/// #1627: a tagged scan root is still crawled (as the walk crawls it), so +/// its skip-named members are found through a pattern that starts with a +/// glob too. +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_finds_members_of_a_tagged_scan_root() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + tokio::fs::write( + root.join("package.json"), + r#"{"name":"root","private":true,"workspaces":["{apps,packages}/*"]}"#, + ) + .await + .unwrap(); + tokio::fs::write( + root.join("CACHEDIR.TAG"), + "Signature: 8a477f597d28d172789f06886806bc55\n", + ) + .await + .unwrap(); + let member = stage_member(root, "apps/build").await; + + let result = NpmCrawler.crawl_all(&options_at(root)).await; + let got: Vec = result.iter().map(|p| p.path.clone()).collect(); + assert_eq!(got, vec![member]); +} + +/// #1627 control: the name-based prune still holds for a `build` / `dist` +/// dir no `workspaces` pattern declares, for a declared dir without a +/// `package.json` (no package manager calls it a member). +#[tokio::test] +#[serial_test::parallel] +async fn crawl_all_still_skips_undeclared_skip_dirs() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + tokio::fs::write( + root.join("package.json"), + r#"{"name":"root","private":true,"workspaces":["packages/*","build/*"]}"#, + ) + .await + .unwrap(); + // Undeclared: `dist` at the root and `packages/x/build` below a member. + stage_npm_pkg( + &root.join("dist").join("node_modules"), + "not-root-dist", + "1.0", + ) + .await; + let member = stage_member(root, "packages/x").await; + stage_npm_pkg( + &root.join("packages/x/build/node_modules"), + "not-member-build", + "1.0", + ) + .await; + // Declared by `build/*` but no package.json. + stage_npm_pkg(&root.join("build/out/node_modules"), "not-a-member", "1.0").await; + + let result = NpmCrawler.crawl_all(&options_at(root)).await; + let got: Vec = result.iter().map(|p| p.path.clone()).collect(); + assert_eq!(got, vec![member]); +} + #[tokio::test] #[serial_test::parallel] async fn crawl_all_skips_hidden_and_skip_dirs() { diff --git a/docs/ecosystems.md b/docs/ecosystems.md index c35f6d056..0fd6dc691 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -431,7 +431,12 @@ projects), at any depth. The walk does not descend into symlinked directories, hidden directories (`.git`, `.cache`, ...), `node_modules` itself (packages are read from it, not searched for workspaces), or directories named `dist`, `build`, `coverage`, `tmp`, `temp`, `__pycache__` -or `vendor`. It also prunes every directory that carries a +or `vendor`, except that a workspace member the root `package.json` +`workspaces` field declares (a directory holding a `package.json` that any +of npm, yarn or Bun reads the patterns as listing, such as `packages/build` +or `vendor/lib`) still has its own `node_modules` crawled. npm, yarn and +Bun install a member's unhoistable dependencies there from the root lock. +It also prunes every directory that carries a [Cache Directory Tagging](https://bford.info/cachedir/) `CACHEDIR.TAG` file beginning with the standard signature — a cargo `target/`, and caches of other tools that follow the convention: neither that directory's own