From d5b800e16043ecde475afb1cfc054a59fd6f047f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 12:23:42 +0000 Subject: [PATCH 1/5] Start fix for #1627 Assisted-by: Claude Code:claude-opus-5-5 From 35f1f1b39570912ed5de2daa478fdee59c29f463 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 12:47:11 +0000 Subject: [PATCH 2/5] Test workspace members named like build dirs A declared workspace member at packages/build, packages/dist, vendor/lib or tools/build is never crawled, so agent apply leaves its nested copy unpatched, apply --check calls it in sync and VEX attests not_affected (#1627). These tests fail on main. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_alternate_installers.rs | 114 ++++++++++++++++++ .../tests/crawler_npm_e2e.rs | 98 +++++++++++++++ 2 files changed, 212 insertions(+) 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..60b99afd0 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,120 @@ 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); + 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 its embedded +/// VEX attests nothing as `not_affected`. +#[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 vex_path = tmp.path().join("vex.json"); + let mut args = default_apply(tmp.path()); + args.check = true; + args.vex = socket_patch_cli::commands::vex::VexEmbedArgs { + vex: Some(vex_path.clone()), + vex_product: Some("pkg:npm/root@1.0.0".to_string()), + ..Default::default() + }; + let code = apply_run(args).await; + assert_eq!( + code, 1, + "apply --check must report the unpatched member copy" + ); + let vex = std::fs::read_to_string(&vex_path).unwrap_or_default(); + assert!( + !vex.contains("not_affected"), + "VEX must not attest not_affected while packages/build's copy is unpatched:\n{vex}" + ); +} + // --------------------------------------------------------------------------- // Rush pnpm symlink farm (hand-built, no real installer) // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 6b733b5a8..91e8755c8 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -848,6 +848,104 @@ 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"][..], + ), + ] { + 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 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() { From 7042dbab263214b2a2114e128f8c5a1132f102ee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 12:47:11 +0000 Subject: [PATCH 3/5] Crawl workspace members named like build dirs The npm-family crawler skips every directory named dist, build, coverage, tmp, temp, __pycache__ or vendor. It did so even when the root package.json workspaces field declares that directory as a member. npm, yarn and Bun install a member's unhoistable dependencies into its own node_modules from the root lock, so that copy was never patched. apply --check still passed, and VEX attested not_affected for a vulnerable copy that runs in production. The crawler now also crawls the node_modules of every declared member (a dir with a package.json that any package manager's workspaces reading lists) whose path crosses one of those names. Undeclared build output and cache dirs stay skipped. Fixes #1627 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 115 ++++++++++++++++++ .../src/crawlers/npm_crawler/oracle.rs | 14 ++- docs/ecosystems.md | 7 +- 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index c21756c8e..d386f49aa 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -65,6 +65,115 @@ pub(super) fn configured_install_roots(start_path: &Path) -> Vec { roots } +/// The `node_modules` dirs of the declared workspace members at +/// `start_path` 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. 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 positive pattern is walked +/// from its literal prefix, through real dirs only (never a symlink, +/// a hidden dir or a `node_modules`, as the walk itself), to the depth +/// its segments allow (any depth for `**`, within +/// [`WORKSPACE_GLOB_DIR_BUDGET`] dirs). Only existing directories are +/// returned, sorted. +pub(super) fn skipped_workspace_member_roots(start_path: &Path) -> Vec { + use crate::utils::workspace_globs::{workspaces_include_as, WorkspacesDialect}; + + let Some(patterns) = root_workspace_patterns(start_path) else { + return Vec::new(); + }; + let mut budget = WORKSPACE_GLOB_DIR_BUDGET; + let mut roots = HashSet::new(); + for pattern in patterns.iter().filter(|p| !p.starts_with('!')) { + let segments: Vec<&str> = pattern + .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(); + // A literal prefix that climbs out (`..`), or crosses a link or a + // dir the walk never enters for another reason, names nothing here. + let mut rel: Vec = Vec::new(); + let mut dir = start_path.to_path_buf(); + let mut reachable = true; + for segment in &segments[..literal] { + dir.push(segment); + rel.push(segment.to_string()); + if segment.starts_with('.') + || *segment == "node_modules" + || !std::fs::symlink_metadata(&dir).is_ok_and(|m| m.is_dir()) + { + reachable = false; + break; + } + } + if !reachable { + continue; + } + let rest = &segments[literal..]; + let max_depth = if rest.contains(&"**") { + usize::MAX + } else { + rest.len() + }; + let mut stack = vec![(dir, rel, 0usize)]; + while let Some((dir, rel, depth)) = stack.pop() { + 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() + && is_dir_sync(&dir.join("node_modules")) + { + roots.insert(dir.join("node_modules")); + } + if depth >= max_depth { + continue; + } + for entry in list_dir_sync(&dir).entries { + if !entry.file_type.is_some_and(|ft| ft.is_dir()) + || entry.name_str == "node_modules" + || entry.name_str.starts_with('.') + { + continue; + } + 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)); + } + } + } + let mut roots: Vec = roots.into_iter().collect(); + roots.sort(); + roots +} + +/// The root `package.json` `workspaces` patterns at `root` (the array or +/// the `{"packages": [...]}` shape); `None` when there are none or the +/// file cannot be read. +fn root_workspace_patterns(root: &Path) -> Option> { + let text = crate::utils::fs::read_regular_to_string_sync(&root.join("package.json")).ok()?; + let doc: serde_json::Value = serde_json::from_str(strip_bom(&text)).ok()?; + let list = match doc.get("workspaces")? { + serde_json::Value::Array(list) => list, + serde_json::Value::Object(map) => map.get("packages")?.as_array()?, + _ => return None, + }; + let patterns: Vec = list + .iter() + .filter_map(|v| v.as_str()) + .map(str::to_string) + .collect(); + (!patterns.is_empty()).then_some(patterns) +} + /// 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 +3001,12 @@ 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). + for root in skipped_workspace_member_roots(start_path) { + if !results.contains(&root) { + results.push(root); + } + } 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..3aba34aa6 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,14 @@ 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). + for root in skipped_workspace_member_roots(start_path) { + if !results.contains(&root) { + results.push(root); + } + } merge_configured_install_roots(results, configured_install_roots(start_path)) } 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 From 26978088dc0f26138e26403f3c7e2b330d6a4b6e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 13:01:01 +0000 Subject: [PATCH 4/5] Harden the skipped workspace member walk Review follow-ups on the #1627 fix: - Also read the workspaces of nested projects the walk finds, so a workspace below the scan root keeps its build/dist members. - Never descend into a tagged cache dir, and read each dir's name from its parent's listing, so a member spelled in another case on a case-insensitive disk is not listed twice. - Walk patterns that start with "!" too: yarn berry reads "!(..)" as an extglob that lists members. - Give each pattern its own dir budget, and reuse the existing workspaces field parser. - The CLI test now checks VEX with the standalone vex command, and the manifest carries a vulnerability so there is a statement to check. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_alternate_installers.rs | 46 ++++-- .../src/crawlers/npm_crawler.rs | 149 ++++++++++-------- .../src/crawlers/npm_crawler/oracle.rs | 7 +- .../src/hosted/governing_root.rs | 2 +- .../tests/crawler_npm_e2e.rs | 55 +++++++ 5 files changed, 178 insertions(+), 81 deletions(-) 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 60b99afd0..f51c3685b 100644 --- a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs +++ b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs @@ -1170,6 +1170,15 @@ fn stage_skip_dir_member_workspace( 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(); @@ -1201,8 +1210,8 @@ async fn apply_patches_workspace_member_named_like_a_build_dir() { } /// #1627: with only the other member's copy patched, `apply --check` -/// reports the `packages/build` copy as drift (exit 1) and its embedded -/// VEX attests nothing as `not_affected`. +/// 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() { @@ -1212,24 +1221,39 @@ async fn apply_check_and_vex_see_unpatched_member_named_like_a_build_dir() { std::fs::write(&app_copy, &patched).unwrap(); assert_ne!(std::fs::read(&member_copy).unwrap(), patched); - let vex_path = tmp.path().join("vex.json"); let mut args = default_apply(tmp.path()); args.check = true; - args.vex = socket_patch_cli::commands::vex::VexEmbedArgs { - vex: Some(vex_path.clone()), - vex_product: Some("pkg:npm/root@1.0.0".to_string()), - ..Default::default() - }; let code = apply_run(args).await; assert_eq!( code, 1, "apply --check must report the unpatched member copy" ); - let vex = std::fs::read_to_string(&vex_path).unwrap_or_default(); + + 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!( - !vex.contains("not_affected"), - "VEX must not attest not_affected while packages/build's copy is unpatched:\n{vex}" + !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}"); } // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index d386f49aa..8116c16b7 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -65,50 +65,96 @@ pub(super) fn configured_install_roots(start_path: &Path) -> Vec { roots } -/// The `node_modules` dirs of the declared workspace members at -/// `start_path` 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. 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 positive pattern is walked -/// from its literal prefix, through real dirs only (never a symlink, -/// a hidden dir or a `node_modules`, as the walk itself), to the depth -/// its segments allow (any depth for `**`, within -/// [`WORKSPACE_GLOB_DIR_BUDGET`] dirs). Only existing directories are -/// returned, sorted. -pub(super) fn skipped_workspace_member_roots(start_path: &Path) -> Vec { +/// 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. 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) = root_workspace_patterns(start_path) else { + 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 mut budget = WORKSPACE_GLOB_DIR_BUDGET; - let mut roots = HashSet::new(); - for pattern in patterns.iter().filter(|p| !p.starts_with('!')) { + 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(); - // A literal prefix that climbs out (`..`), or crosses a link or a - // dir the walk never enters for another reason, names nothing here. + // 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 dir = start_path.to_path_buf(); let mut reachable = true; for segment in &segments[..literal] { - dir.push(segment); - rel.push(segment.to_string()); - if segment.starts_with('.') - || *segment == "node_modules" - || !std::fs::symlink_metadata(&dir).is_ok_and(|m| m.is_dir()) - { + 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 !reachable { continue; @@ -119,27 +165,26 @@ pub(super) fn skipped_workspace_member_roots(start_path: &Path) -> Vec } 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); + if 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() - && is_dir_sync(&dir.join("node_modules")) + && has_node_modules_dir(&dir, &listing) { - roots.insert(dir.join("node_modules")); + roots.push(dir.join("node_modules")); } if depth >= max_depth { continue; } - for entry in list_dir_sync(&dir).entries { - if !entry.file_type.is_some_and(|ft| ft.is_dir()) - || entry.name_str == "node_modules" - || entry.name_str.starts_with('.') - { - continue; - } + for entry in listing.entries.into_iter().filter(|e| walkable(e)) { let Some(left) = budget.checked_sub(1) else { break; }; @@ -150,30 +195,9 @@ pub(super) fn skipped_workspace_member_roots(start_path: &Path) -> Vec } } } - let mut roots: Vec = roots.into_iter().collect(); - roots.sort(); roots } -/// The root `package.json` `workspaces` patterns at `root` (the array or -/// the `{"packages": [...]}` shape); `None` when there are none or the -/// file cannot be read. -fn root_workspace_patterns(root: &Path) -> Option> { - let text = crate::utils::fs::read_regular_to_string_sync(&root.join("package.json")).ok()?; - let doc: serde_json::Value = serde_json::from_str(strip_bom(&text)).ok()?; - let list = match doc.get("workspaces")? { - serde_json::Value::Array(list) => list, - serde_json::Value::Object(map) => map.get("packages")?.as_array()?, - _ => return None, - }; - let patterns: Vec = list - .iter() - .filter_map(|v| v.as_str()) - .map(str::to_string) - .collect(); - (!patterns.is_empty()).then_some(patterns) -} - /// 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 @@ -3002,11 +3026,8 @@ 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). - for root in skipped_workspace_member_roots(start_path) { - if !results.contains(&root) { - results.push(root); - } - } + 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 3aba34aa6..5663ba351 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs @@ -497,11 +497,8 @@ impl LegacyNpmCrawler { // 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). - for root in skipped_workspace_member_roots(start_path) { - if !results.contains(&root) { - results.push(root); - } - } + 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 91e8755c8..9bafdbb77 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -887,6 +887,8 @@ async fn crawl_all_finds_declared_members_with_skip_dir_names() { 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(); @@ -910,6 +912,59 @@ async fn crawl_all_finds_declared_members_with_skip_dir_names() { } } +/// #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, 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/*/*","packages/build"]}"#, + ) + .await + .unwrap(); + stage_member(root, "cache/build/pkg").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 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). From a8326e5abf510bf8f45d26b85093602a75a87f3c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 13:49:02 +0000 Subject: [PATCH 5/5] Prune tagged caches on literal member prefixes A workspace member reached through a literal pattern such as cache/build/pkg was crawled even when cache/ carries a CACHEDIR.TAG, so agent apply could rewrite files in a tool's cache. A tagged scan root instead dropped every member a glob-led pattern lists, although the walk always crawls the scan root. Now every dir below the workspace root is checked for the tag, literal prefix included, and the root itself is exempt. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 12 +++++-- .../tests/crawler_npm_e2e.rs | 32 +++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 8116c16b7..4069ea988 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -102,7 +102,8 @@ pub(super) fn skipped_workspace_member_roots( /// 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. Names are +/// 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 { @@ -155,6 +156,10 @@ fn skipped_members_of(root: &Path) -> Vec { }; 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; @@ -169,7 +174,10 @@ fn skipped_members_of(root: &Path) -> Vec { let mut stack = vec![(dir, rel, 0usize)]; while let Some((dir, rel, depth)) = stack.pop() { let listing = list_dir_sync(&dir); - if is_tagged_cache_dir(&dir, &listing) { + // 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())) diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 9bafdbb77..dda4d2ca4 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -937,7 +937,7 @@ async fn crawl_all_finds_skip_dir_members_of_a_nested_workspace() { } /// #1627: the member walk keeps the walk's other rules. A member inside a -/// tagged cache dir stays pruned, and a pattern that spells a member dir +/// 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] @@ -947,11 +947,12 @@ async fn crawl_all_member_walk_keeps_the_walk_rules() { let root = tmp.path(); tokio::fs::write( root.join("package.json"), - r#"{"name":"root","private":true,"workspaces":["cache/*/*","packages/build"]}"#, + 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", @@ -965,6 +966,33 @@ async fn crawl_all_member_walk_keeps_the_walk_rules() { 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).