From 0fa7f0acbd29c70ba69fcce158c90a80e62dbd3a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 01:23:48 +0000 Subject: [PATCH 1/5] Start fix for #493, #518 Assisted-by: Claude Code:claude-opus-5-5 From e1e963774c4cf0c317fa6f844ed859ee430e96e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 01:45:17 +0000 Subject: [PATCH 2/5] Crawl yarn modules-folder and Rush install roots The npm crawler found a project's installed packages only in dirs named node_modules, so it never looked where yarn classic installs with `--modules-folder` or where Rush installs (common/temp). Agent mode reported those packages as not installed and left them unpatched, and hosted vex treated the unpatched copy as absent and attested the patch from the lockfile pin alone. The crawler now also reads the nearest .yarnrc --modules-folder and adds common/temp/node_modules in a Rush repo (rush.json at the root). Fixes #493, #518 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vex_redirect.rs | 143 ++++++++ .../socket-patch-cli/tests/e2e_vex_vendor.rs | 58 ++++ .../tests/in_process_alternate_installers.rs | 116 +++++++ .../src/crawlers/npm_crawler.rs | 316 +++++++++++++++++- .../src/crawlers/npm_crawler/oracle.rs | 8 +- 5 files changed, 637 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vex_redirect.rs b/crates/socket-patch-cli/tests/e2e_vex_redirect.rs index 54b7e3adc..cfa235d27 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_redirect.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_redirect.rs @@ -2161,3 +2161,146 @@ fn vlt_redirect_ledger_is_judged_by_the_vlt_store_copy_while_the_lock_pins_it() ); } } + +/// The patch view for `name@version` (the [`left_pad_view`] shape). +fn npm_view(name: &str, version: &str, after_hash: &str) -> Value { + let mut view = left_pad_view(after_hash); + view["purl"] = Value::String(format!("pkg:npm/{name}@{version}")); + view +} + +/// REGRESSION (#493): yarn classic's `.yarnrc` `--modules-folder deps` +/// installs into `deps/`. The crawler never looked there, so the +/// unpatched installed copy read as "nothing installed" and the pinned +/// hosted lock attested `not_affected`. Installed evidence wins: the +/// copy is hash-checked and omitted. With nothing installed the lock +/// basis still attests. +#[test] +fn yarn_modules_folder_install_is_hash_verified_not_lockfile_attested() { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + let purl = "pkg:npm/left-pad@1.3.0"; + std::fs::write( + cwd.join("package.json"), + r#"{ "name": "app", "version": "1.0.0", "dependencies": { "left-pad": "1.3.0" } }"#, + ) + .unwrap(); + std::fs::write(cwd.join(".yarnrc"), "--modules-folder deps\n").unwrap(); + std::fs::write( + cwd.join("yarn.lock"), + format!( + "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ + # yarn lockfile v1\n\n\nleft-pad@1.3.0:\n version \"1.3.0\"\n \ + resolved \"{}\"\n integrity {SRI}\n", + hosted_npm_url("left-pad", "1.3.0", UUID) + ), + ) + .unwrap(); + let pkg = cwd.join("deps/left-pad"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + r#"{ "name": "left-pad", "version": "1.3.0" }"#, + ) + .unwrap(); + std::fs::write(pkg.join("index.js"), b"unpatched upstream bytes\n").unwrap(); + let patched = b"hosted patched index\n"; + let (_rt, server) = serve_patch_views(vec![( + UUID.to_string(), + left_pad_view(&compute_git_sha256_from_bytes(patched)), + )]); + + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_eq!( + code, + Some(1), + "the unpatched deps/ copy must not attest: {env}" + ); + assert_eq!(skipped_reason(&env, purl), "hash_mismatch", "{env}"); + + std::fs::write(pkg.join("index.js"), patched).unwrap(); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_eq!(code, Some(0), "a patched deps/ copy attests: {env}"); + + std::fs::remove_dir_all(cwd.join("deps")).unwrap(); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_eq!( + code, + Some(0), + "nothing installed: the lock basis attests: {env}" + ); +} + +/// REGRESSION (#518): Rush installs every package into +/// `common/temp/node_modules/.pnpm` and the projects' `node_modules` only +/// link their DIRECT deps. The crawler pruned `temp`, so an unpatched +/// transitive dep read as "nothing installed" and the hosted pin in +/// `common/config/rush/pnpm-lock.yaml` attested `not_affected`. +#[cfg(unix)] +#[test] +fn rush_common_temp_install_is_hash_verified_not_lockfile_attested() { + let tmp = tempfile::tempdir().unwrap(); + let cwd = tmp.path(); + let purl = "pkg:npm/is-number@7.0.0"; + std::fs::write(cwd.join("rush.json"), r#"{ "rushVersion": "5.180.0" }"#).unwrap(); + let lock_dir = cwd.join("common/config/rush"); + std::fs::create_dir_all(&lock_dir).unwrap(); + std::fs::write( + lock_dir.join("pnpm-lock.yaml"), + format!( + "lockfileVersion: '9.0'\n\nimporters:\n ../../apps/app:\n dependencies:\n \ + to-regex-range:\n specifier: 5.0.1\n version: 5.0.1\n\npackages:\n \ + is-number@7.0.0:\n resolution: {{integrity: {SRI}, tarball: {}}}\n \ + to-regex-range@5.0.1:\n resolution: {{integrity: sha512-UPSTREAMupstream==}}\n\n\ + snapshots:\n is-number@7.0.0: {{}}\n to-regex-range@5.0.1:\n dependencies:\n \ + is-number: 7.0.0\n", + hosted_npm_url("is-number", "7.0.0", UUID) + ), + ) + .unwrap(); + let store = cwd.join("common/temp/node_modules/.pnpm"); + let installed = store.join("is-number@7.0.0/node_modules/is-number"); + let direct = store.join("to-regex-range@5.0.1/node_modules/to-regex-range"); + for (dir, name, version) in [ + (&installed, "is-number", "7.0.0"), + (&direct, "to-regex-range", "5.0.1"), + ] { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!(r#"{{ "name": "{name}", "version": "{version}" }}"#), + ) + .unwrap(); + } + std::fs::write(installed.join("index.js"), b"unpatched upstream bytes\n").unwrap(); + std::os::unix::fs::symlink(&installed, direct.parent().unwrap().join("is-number")).unwrap(); + let app = cwd.join("apps/app"); + std::fs::create_dir_all(app.join("node_modules")).unwrap(); + std::fs::write( + app.join("package.json"), + r#"{ "name": "app", "version": "1.0.0", "dependencies": { "to-regex-range": "5.0.1" } }"#, + ) + .unwrap(); + std::os::unix::fs::symlink(&direct, app.join("node_modules/to-regex-range")).unwrap(); + let patched = b"hosted patched index\n"; + let (_rt, server) = serve_patch_views(vec![( + UUID.to_string(), + npm_view( + "is-number", + "7.0.0", + &compute_git_sha256_from_bytes(patched), + ), + )]); + + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_eq!( + code, + Some(1), + "the unpatched store copy must not attest: {env}" + ); + assert_eq!(skipped_reason(&env, purl), "hash_mismatch", "{env}"); + + std::fs::write(installed.join("index.js"), patched).unwrap(); + let (code, env) = vex_json(cwd, &["--proxy-url", &server.uri()]); + assert_eq!(code, Some(0), "a patched store copy attests: {env}"); +} diff --git a/crates/socket-patch-cli/tests/e2e_vex_vendor.rs b/crates/socket-patch-cli/tests/e2e_vex_vendor.rs index d33961cb4..253a6fb9c 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_vendor.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_vendor.rs @@ -2481,3 +2481,61 @@ fn vex_attests_a_vlt_vendored_dir_and_omits_a_tampered_one() { "a tampered dir is never attested" ); } + +/// REGRESSION (#493): the vendored out-of-sync disclosure for a yarn +/// classic project installed with `.yarnrc` `--modules-folder deps`. The +/// crawler never looked in `deps/`, so the drifted live tree went +/// unreported (the attestation itself stands, from the committed +/// artifact, as in [`vendored_live_tree_out_of_sync_warns_but_attests`]). +#[test] +fn vendored_modules_folder_tree_out_of_sync_warns() { + let tmp = tempfile::tempdir().expect("create tempdir"); + let cwd = tmp.path(); + let purl = "pkg:npm/lodash@4.17.21"; + let uuid = "0a0a0a0a-1111-4111-8111-0a0a0a0a0a0b"; + + let patched = b"patched npm bytes\n"; + let after_hash = compute_git_sha256_from_bytes(patched); + let rel = format!(".socket/vendor/npm/{uuid}/lodash-4.17.21.tgz"); + let sha256 = sha256_hex(&write_member_tgz( + &cwd.join(&rel), + "package/index.js", + patched, + )); + let record = make_record( + uuid, + "package/index.js", + &after_hash, + "GHSA-sync-bbbb", + &["CVE-2026-11"], + ); + let wiring = write_matrix_wiring(cwd, "npm", uuid, &rel); + let mut state = VendorState::new(); + state.entries.insert( + purl.to_string(), + detached_matrix_entry("npm", purl, uuid, &rel, sha256, record, wiring), + ); + std::fs::write( + cwd.join(".socket/vendor/state.json"), + serde_json::to_string_pretty(&state).expect("serialize vendor state"), + ) + .expect("write vendor state.json"); + + std::fs::write(cwd.join(".yarnrc"), "--modules-folder deps\n").unwrap(); + let installed = cwd.join("deps/lodash"); + std::fs::create_dir_all(&installed).unwrap(); + std::fs::write( + installed.join("package.json"), + r#"{"name":"lodash","version":"4.17.21"}"#, + ) + .unwrap(); + std::fs::write(installed.join("index.js"), b"original unpatched bytes\n").unwrap(); + + let (code, env) = vex_json(cwd, &[]); + assert_eq!(code, Some(0), "{env}"); + let w = env["warnings"] + .as_array() + .and_then(|ws| ws.iter().find(|w| w["code"] == "vendored_tree_out_of_sync")) + .unwrap_or_else(|| panic!("expected a vendored_tree_out_of_sync warning: {env}")); + assert!(w["detail"].as_str().unwrap().contains(purl), "{w}"); +} 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 a264cf335..e6654aa55 100644 --- a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs +++ b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs @@ -1060,3 +1060,119 @@ async fn rush_pnpm_symlink_farm_apply_patches_through_both_projects() { ); } } + +/// REGRESSION (#518): a Rush TRANSITIVE dependency lives only in the pnpm +/// store under `common/temp/node_modules/.pnpm` (no project links it), and +/// the crawler pruned `temp`, so agent-mode apply reported it +/// `package_not_installed` and left it unpatched. It is now found through +/// the `common/temp/node_modules` root and patched in place. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn rush_transitive_dep_in_common_temp_store_is_patched() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write(root.join("rush.json"), r#"{ "rushVersion": "5.180.0" }"#).unwrap(); + let store = root.join("common/temp/node_modules/.pnpm"); + let transitive = store.join("is-number@7.0.0/node_modules/is-number"); + let direct = store.join("to-regex-range@5.0.1/node_modules/to-regex-range"); + for (dir, name, version) in [ + (&transitive, "is-number", "7.0.0"), + (&direct, "to-regex-range", "5.0.1"), + ] { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!(r#"{{ "name": "{name}", "version": "{version}" }}"#), + ) + .unwrap(); + } + symlink(&transitive, direct.parent().unwrap().join("is-number")).unwrap(); + let app = root.join("apps/app"); + std::fs::create_dir_all(app.join("node_modules")).unwrap(); + std::fs::write( + app.join("package.json"), + r#"{ "name": "app", "version": "1.0.0", "dependencies": { "to-regex-range": "5.0.1" } }"#, + ) + .unwrap(); + symlink(&direct, app.join("node_modules/to-regex-range")).unwrap(); + + let original = b"module.exports = function isNumber() {}\n".to_vec(); + std::fs::write(transitive.join("index.js"), &original).unwrap(); + let before_hash = git_sha256(&original); + let mut patched = original.clone(); + patched.extend_from_slice(b"\n// SOCKET-PATCH-RUSH-TRANSITIVE-MARKER\n"); + let after_hash = git_sha256(&patched); + let socket = root.join(".socket"); + write_manifest( + &socket, + "pkg:npm/is-number@7.0.0", + &before_hash, + &after_hash, + ); + let blobs = socket.join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(&after_hash), &patched).unwrap(); + + let code = apply_run(default_apply(root)).await; + assert_eq!( + code, 0, + "apply must find the Rush transitive dep in common/temp" + ); + assert_patched( + &transitive.join("index.js"), + &patched, + &before_hash, + &after_hash, + ); + assert_eq!( + std::fs::read(direct.parent().unwrap().join("is-number/index.js")).unwrap(), + patched, + "the dependent's store link sees the patched bytes" + ); +} + +/// REGRESSION (#493): with `.yarnrc` `--modules-folder deps`, yarn classic +/// installs into `deps/` and there is no `node_modules`. Agent-mode apply +/// reported the package `package_not_installed` (exit 0 from `scan`, +/// unpatched); it is now patched at `deps/`. +#[tokio::test] +#[serial] +async fn yarn_modules_folder_install_is_patched() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write( + root.join("package.json"), + r#"{ "name": "app", "version": "1.0.0", "dependencies": { "left-pad": "1.3.0" } }"#, + ) + .unwrap(); + std::fs::write(root.join(".yarnrc"), "--modules-folder \"deps\"\n").unwrap(); + let pkg = root.join("deps/left-pad"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + r#"{ "name": "left-pad", "version": "1.3.0" }"#, + ) + .unwrap(); + let original = b"module.exports = leftPad;\n".to_vec(); + std::fs::write(pkg.join("index.js"), &original).unwrap(); + let before_hash = git_sha256(&original); + let mut patched = original.clone(); + patched.extend_from_slice(b"\n// SOCKET-PATCH-MODULES-FOLDER-MARKER\n"); + let after_hash = git_sha256(&patched); + let socket = root.join(".socket"); + write_manifest(&socket, "pkg:npm/left-pad@1.3.0", &before_hash, &after_hash); + let blobs = socket.join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(&after_hash), &patched).unwrap(); + + let code = apply_run(default_apply(root)).await; + assert_eq!( + code, 0, + "apply must find the package in the yarn modules folder" + ); + assert_patched(&pkg.join("index.js"), &patched, &before_hash, &after_hash); + assert!(!root.join("node_modules").exists()); +} diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 80b31c3ec..ab401e287 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -26,6 +26,117 @@ const SKIP_DIRS: &[&str] = &[ "vendor", ]; +// --------------------------------------------------------------------------- +// Helper: package-manager-configured install roots +// --------------------------------------------------------------------------- + +/// The `node_modules`-equivalent dirs a package manager is CONFIGURED to +/// install the project at `start_path` into, which the workspace walk +/// cannot find by name (it only collects dirs literally named +/// `node_modules`, and prunes `temp`): +/// - yarn classic's `--modules-folder ` from the nearest `.yarnrc` +/// (the project's or an ancestor's — yarn reads them all, nearest wins), +/// resolved against the project like yarn resolves it against its cwd; +/// - Rush's `common/temp/node_modules` when `rush.json` is at the root: +/// rush runs pnpm there, so every transitive dep lives in its `.pnpm` +/// store and the projects' own `node_modules` hold only links to their +/// direct deps. +/// +/// Only existing directories are returned. +pub(super) fn configured_install_roots(start_path: &Path) -> Vec { + let mut roots = Vec::new(); + if let Some(folder) = yarnrc_modules_folder(start_path) { + roots.push(start_path.join(folder)); + } + if start_path.join("rush.json").is_file() { + roots.push(start_path.join("common").join("temp").join("node_modules")); + } + roots.retain(|root| root.is_dir()); + roots +} + +/// Append the `configured` roots to the walk's `walked` roots. A walked +/// root inside a configured one is a package's nested `node_modules` the +/// walk mistook for a workspace (the walk descends into a modules folder +/// that is not named `node_modules`); it is dropped, since crawling the +/// configured root inventories it as a nested tree. A configured root the +/// walk already found is not repeated. +pub(super) fn merge_configured_install_roots( + mut walked: Vec, + configured: Vec, +) -> Vec { + if configured.is_empty() { + return walked; + } + walked.retain(|root| configured.iter().all(|c| root == c || !root.starts_with(c))); + for root in configured { + if !walked.contains(&root) { + walked.push(root); + } + } + walked +} + +/// The `--modules-folder` value from the nearest `.yarnrc` at or above +/// `start_path`, if any `.yarnrc` sets it. +fn yarnrc_modules_folder(start_path: &Path) -> Option { + start_path.ancestors().find_map(|dir| { + let rc = std::fs::read_to_string(dir.join(".yarnrc")).ok()?; + parse_yarnrc_modules_folder(&rc) + }) +} + +/// The `--modules-folder` (or command-scoped `--install.modules-folder`) +/// value of a yarn classic `.yarnrc`. The file is yarn's lockfile syntax: +/// one `key value` pair per line, either side optionally double-quoted, an +/// optional `:` after the key, `#` comment lines. The last setting wins. +fn parse_yarnrc_modules_folder(rc: &str) -> Option { + let mut found = None; + for line in rc.trim_start_matches('\u{feff}').lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + let Some((key, rest)) = split_yarnrc_token(line, true) else { + continue; + }; + if key != "--modules-folder" && key != "--install.modules-folder" { + continue; + } + let rest = rest.trim_start(); + let rest = rest.strip_prefix(':').unwrap_or(rest).trim_start(); + if let Some((value, _)) = split_yarnrc_token(rest, false) { + if !value.is_empty() { + found = Some(value); + } + } + } + found +} + +/// Split the leading token off `s`: a double-quoted string (with `\"` and +/// `\\` escapes) or a run of non-whitespace — for a key (`is_key`) also +/// ending at a `:`, which a value may contain (`C:\deps`). Returns the +/// unquoted token and the remainder. +fn split_yarnrc_token(s: &str, is_key: bool) -> Option<(String, &str)> { + if let Some(quoted) = s.strip_prefix('"') { + let mut value = String::new(); + let mut chars = quoted.char_indices(); + while let Some((i, c)) = chars.next() { + match c { + '\\' => value.push(chars.next()?.1), + '"' => return Some((value, "ed[i + 1..])), + c => value.push(c), + } + } + return None; + } + let end = s + .find(|c: char| c.is_whitespace() || (is_key && c == ':')) + .unwrap_or(s.len()); + (end > 0).then(|| (s[..end].to_string(), &s[end..])) +} + // --------------------------------------------------------------------------- // Helper: read and parse package.json // --------------------------------------------------------------------------- @@ -1427,7 +1538,7 @@ impl NpmCrawler { // Recursively search for workspace node_modules results.extend(Self::find_workspace_node_modules(start_path, listing)); - results + merge_configured_install_roots(results, configured_install_roots(start_path)) } /// Find `node_modules` in subdirectories (for monorepos / workspaces), @@ -4171,4 +4282,207 @@ mod tests { } } } + + /// `.yarnrc` `--modules-folder` parsing: bare and quoted keys and + /// values, an optional `:`, the command-scoped form, comments, a BOM, + /// CRLF, a Windows drive path, and last-setting-wins. + #[test] + fn test_parse_yarnrc_modules_folder() { + let parse = parse_yarnrc_modules_folder; + assert_eq!(parse("--modules-folder deps\n").as_deref(), Some("deps")); + assert_eq!( + parse("\"--modules-folder\" \"./my deps\"\n").as_deref(), + Some("./my deps") + ); + assert_eq!(parse("--modules-folder: lib\n").as_deref(), Some("lib")); + assert_eq!( + parse("--install.modules-folder vendor_modules").as_deref(), + Some("vendor_modules") + ); + assert_eq!( + parse("\u{feff}# comment\r\nyarn-offline-mirror \"./m\"\r\n--modules-folder deps\r\n") + .as_deref(), + Some("deps") + ); + assert_eq!( + parse("--modules-folder \"C:\\\\deps\"\n").as_deref(), + Some("C:\\deps") + ); + assert_eq!( + parse("--modules-folder C:\\deps\n").as_deref(), + Some("C:\\deps") + ); + assert_eq!( + parse("--modules-folder a\n--modules-folder b\n").as_deref(), + Some("b") + ); + // Not the key, a comment, or no value. + for rc in [ + "", + "# --modules-folder deps\n", + "--modules-folder-x deps\n", + "modules-folder deps\n", + "--modules-folder\n", + "--modules-folder \"unterminated\n", + ] { + assert_eq!(parse(rc), None, "{rc:?}"); + } + } + + fn local_options(cwd: &Path) -> CrawlerOptions { + CrawlerOptions { + cwd: cwd.to_path_buf(), + global: false, + global_prefix: None, + } + } + + /// REGRESSION (#493): yarn classic's `.yarnrc` `--modules-folder deps` + /// installs into `deps/`, which is a crawl root: its packages (and + /// their nested `node_modules`) are inventoried at their real paths. + /// The walk used to skip it, so agent mode reported them not installed + /// and hosted `vex` read the unpatched copy as absent. A nested + /// `deps//node_modules` is not reported as a workspace root. + #[tokio::test] + async fn test_yarnrc_modules_folder_is_a_crawl_root() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write(root.join("package.json"), r#"{"name":"app"}"#).unwrap(); + write_pkg(&root.join("deps/left-pad"), "left-pad", "1.3.0"); + write_pkg( + &root.join("deps/outer/node_modules/inner"), + "inner", + "2.0.0", + ); + write_pkg(&root.join("deps/outer"), "outer", "1.0.0"); + + let crawler = NpmCrawler::new(); + let options = local_options(root); + let names = |pkgs: &[CrawledPackage]| { + let mut v: Vec = pkgs.iter().map(|p| p.purl.clone()).collect(); + v.sort(); + v + }; + // Control: without the .yarnrc, deps/ is not an install root. + assert!(!names(&crawler.crawl_all(&options).await) + .contains(&"pkg:npm/left-pad@1.3.0".to_string())); + + std::fs::write(root.join(".yarnrc"), "--modules-folder deps\n").unwrap(); + let roots = crawler.get_node_modules_paths(&options).await.unwrap(); + assert_eq!(roots, vec![root.join("deps")]); + let pkgs = crawler.crawl_all(&options).await; + assert_eq!( + names(&pkgs), + vec![ + "pkg:npm/inner@2.0.0".to_string(), + "pkg:npm/left-pad@1.3.0".to_string(), + "pkg:npm/outer@1.0.0".to_string(), + ] + ); + let left_pad = pkgs.iter().find(|p| p.name == "left-pad").unwrap(); + assert_eq!(left_pad.path, root.join("deps/left-pad")); + let found = crawler + .find_by_purls(&root.join("deps"), &["pkg:npm/left-pad@1.3.0".to_string()]) + .await + .unwrap(); + assert_eq!(found.len(), 1); + + // An ancestor's .yarnrc applies too; the nearest one wins. + let member = root.join("packages/member"); + write_pkg(&member.join("lib/ms"), "ms", "2.1.3"); + std::fs::write(member.join("package.json"), r#"{"name":"member"}"#).unwrap(); + std::fs::write(root.join(".yarnrc"), "--modules-folder lib\n").unwrap(); + let roots = crawler + .get_node_modules_paths(&local_options(&member)) + .await + .unwrap(); + assert_eq!(roots, vec![member.join("lib")]); + std::fs::write(member.join(".yarnrc"), "--modules-folder deps\n").unwrap(); + let roots = crawler + .get_node_modules_paths(&local_options(&member)) + .await + .unwrap(); + assert!(roots.is_empty(), "member/deps does not exist: {roots:?}"); + } + + /// REGRESSION (#518): in a Rush repo pnpm installs into + /// `common/temp/node_modules` (the walk prunes `temp`), so a + /// TRANSITIVE dep lives only in its `.pnpm` store. It is a crawl root + /// when `rush.json` is present, and only then. + #[cfg(unix)] + #[tokio::test] + async fn test_rush_common_temp_is_a_crawl_root() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let temp_nm = root.join("common/temp/node_modules"); + let store = temp_nm.join(".pnpm"); + let direct = store.join("to-regex-range@5.0.1/node_modules/to-regex-range"); + write_pkg(&direct, "to-regex-range", "5.0.1"); + write_pkg( + &store.join("is-number@7.0.0/node_modules/is-number"), + "is-number", + "7.0.0", + ); + std::os::unix::fs::symlink( + store.join("is-number@7.0.0/node_modules/is-number"), + store.join("to-regex-range@5.0.1/node_modules/is-number"), + ) + .unwrap(); + let app_nm = root.join("apps/app/node_modules"); + std::fs::create_dir_all(&app_nm).unwrap(); + std::os::unix::fs::symlink(&direct, app_nm.join("to-regex-range")).unwrap(); + + let crawler = NpmCrawler::new(); + let options = local_options(root); + let has = |pkgs: &[CrawledPackage], purl: &str| pkgs.iter().any(|p| p.purl == purl); + // Control: not a Rush repo, common/temp stays pruned. + let pkgs = crawler.crawl_all(&options).await; + assert!(has(&pkgs, "pkg:npm/to-regex-range@5.0.1")); + assert!(!has(&pkgs, "pkg:npm/is-number@7.0.0")); + + std::fs::write(root.join("rush.json"), "{}").unwrap(); + let roots = crawler.get_node_modules_paths(&options).await.unwrap(); + assert_eq!(roots, vec![app_nm.clone(), temp_nm.clone()]); + let pkgs = crawler.crawl_all(&options).await; + assert!(has(&pkgs, "pkg:npm/to-regex-range@5.0.1"), "{pkgs:?}"); + let is_number: Vec<_> = pkgs.iter().filter(|p| p.name == "is-number").collect(); + assert_eq!(is_number.len(), 1, "{pkgs:?}"); + assert_eq!( + std::fs::canonicalize(&is_number[0].path).unwrap(), + std::fs::canonicalize(store.join("is-number@7.0.0/node_modules/is-number")).unwrap() + ); + let found = crawler + .find_by_purls(&temp_nm, &["pkg:npm/is-number@7.0.0".to_string()]) + .await + .unwrap(); + assert_eq!(found.len(), 1, "{found:?}"); + } + + /// The configured roots merge: a walked root inside one is dropped, + /// one the walk already found is not repeated, and order is kept. + #[test] + fn test_merge_configured_install_roots() { + let p = PathBuf::from; + assert_eq!( + merge_configured_install_roots(vec![p("/r/node_modules")], vec![]), + vec![p("/r/node_modules")] + ); + assert_eq!( + merge_configured_install_roots( + vec![ + p("/r/node_modules"), + p("/r/deps/a/node_modules"), + p("/r/deps-x/node_modules"), + p("/r/lib/node_modules"), + ], + vec![p("/r/deps"), p("/r/lib/node_modules")], + ), + vec![ + p("/r/node_modules"), + p("/r/deps-x/node_modules"), + p("/r/lib/node_modules"), + p("/r/deps"), + ] + ); + } } 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 d71a02127..95e04093e 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler/oracle.rs @@ -9,8 +9,8 @@ use std::ffi::OsString; use std::path::{Path, PathBuf}; use super::{ - build_npm_purl, is_legacy_pnpm_store_dir_name, - is_safe_npm_component, parse_package_name, read_package_json, NpmCrawler, StoreEntry, + 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, }; use crate::crawlers::types::{CrawledPackage, CrawlerOptions}; @@ -415,7 +415,9 @@ impl LegacyNpmCrawler { // Recursively search for workspace node_modules Self::find_workspace_node_modules(start_path, &mut results).await; - results + // The package-manager-configured roots are shared with the parent + // module (not part of the walk this oracle checks). + merge_configured_install_roots(results, configured_install_roots(start_path)) } /// Recursively find `node_modules` in subdirectories (for monorepos / workspaces). From 549f8c0527458e63f9ad2c0a923b5b2c6db90859 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 01:47:48 +0000 Subject: [PATCH 3/5] Add real yarn and Rush regression legs A real yarn classic install with --modules-folder is patched by agent apply, and on a real pnpm-installed Rush repo vex now refuses to attest when the installed copy under common/temp is unpatched. Refs #493, #518 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_rush_sim.rs | 50 +++++++++++++++ .../tests/in_process_alternate_installers.rs | 64 +++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs b/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs index 9a0c2a023..85ed473e0 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs @@ -460,6 +460,56 @@ async fn rush_hosted_scan_then_simulated_pnpm_install_lands_patched_bytes() { String::from_utf8_lossy(&installed[..installed.len().min(120)]) ); assert_rush_manifestless_vex(root, &server.uri(), &patched, rush_common_lock().as_bytes()); + assert_rush_stale_store_not_attested(root, &server.uri(), &patched, orig); +} + +/// REGRESSION (#518): the copy rush installs lives under +/// `common/temp/node_modules`, which the crawler used to prune (`temp`), so +/// an UNPATCHED installed copy read as "nothing installed" and the pinned +/// hosted lock attested it anyway. Revert the real pnpm-installed file to +/// the upstream bytes (a stale or tampered install): installed evidence +/// wins, and the patch is omitted with `hash_mismatch`. +fn assert_rush_stale_store_not_attested( + root: &Path, + patch_server: &str, + patched: &[u8], + upstream: &[u8], +) { + let installed = std::fs::canonicalize( + root.join("common/temp/node_modules") + .join(DEP) + .join("index.js"), + ) + .unwrap(); + // pnpm hardlinks from its store: replace the file rather than writing + // through the link. + std::fs::remove_file(&installed).unwrap(); + std::fs::write(&installed, upstream).unwrap(); + std::thread::scope(|s| { + s.spawn(|| { + let api = PatchApi::start(vec![( + UUID.to_string(), + patch_view( + UUID, + PURL, + &[("package/index.js", &git_sha256(patched))], + VULNS, + ), + )]); + let out = run_vex( + &binary(), + root, + &VexRun { + patch_server_url: Some(patch_server.to_string()), + ..VexRun::online(&api) + }, + ); + assert_eq!(out.code, Some(1), "rush, unpatched store copy: {out}"); + assert_not_attested(&out.envelope, PURL, "hash_mismatch"); + }) + .join() + .unwrap_or_else(|p| std::panic::resume_unwind(p)) + }); } /// Tamper twin: the hosted route serves DIFFERENT bytes than the pinned 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 e6654aa55..a0339e9f9 100644 --- a/crates/socket-patch-cli/tests/in_process_alternate_installers.rs +++ b/crates/socket-patch-cli/tests/in_process_alternate_installers.rs @@ -207,6 +207,70 @@ async fn yarn_install_then_apply_patches_file() { assert_patched(&ms_index, &patched, &before_hash, &after_hash); } +/// REGRESSION (#493), real yarn classic: `.yarnrc` `--modules-folder deps` +/// makes `yarn install` write `deps/` and no `node_modules`. Agent-mode +/// apply must find and patch the package there. Yarn berry ignores +/// `.yarnrc`, so the leg needs a 1.x `yarn` on PATH. +#[tokio::test] +#[serial] +async fn yarn_classic_modules_folder_install_then_apply_patches_file() { + let classic = pm_command("yarn", &["npm_config_", "YARN_"]) + .arg("--version") + .output() + .ok() + .filter(|o| o.status.success()) + .is_some_and(|o| String::from_utf8_lossy(&o.stdout).starts_with("1.")); + if !classic || !has("npm") { + println!("SKIP: yarn classic (1.x) or npm not on PATH"); + return; + } + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("package.json"), + r#"{ "name": "yarn-mf-test", "version": "0.0.0", "dependencies": { "ms": "2.1.3" } }"#, + ) + .unwrap(); + std::fs::write(tmp.path().join(".yarnrc"), "--modules-folder deps\n").unwrap(); + + let status = pm_command("yarn", &["npm_config_", "YARN_"]) + .args(["install", "--silent", "--no-progress"]) + .current_dir(tmp.path()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + .expect("yarn install"); + if !status.status.success() { + println!( + "SKIP: yarn install failed: {}", + String::from_utf8_lossy(&status.stderr) + ); + return; + } + let ms_index = tmp.path().join("deps/ms/index.js"); + assert!( + ms_index.exists(), + "yarn install succeeded but deps/ms/index.js is missing at {ms_index:?}" + ); + assert!(!tmp.path().join("node_modules").exists()); + + let original = std::fs::read(&ms_index).expect("read ms/index.js"); + let before_hash = git_sha256(&original); + let mut patched = original.clone(); + patched.extend_from_slice(b"\n// SOCKET-PATCH-YARN-MODULES-FOLDER-MARKER\n"); + let after_hash = git_sha256(&patched); + + let socket = tmp.path().join(".socket"); + write_manifest(&socket, "pkg:npm/ms@2.1.3", &before_hash, &after_hash); + let blobs = socket.join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(&after_hash), &patched).unwrap(); + + let code = apply_run(default_apply(tmp.path())).await; + assert_eq!(code, 0, "apply must patch the yarn modules-folder install"); + assert_patched(&ms_index, &patched, &before_hash, &after_hash); +} + // --------------------------------------------------------------------------- // pnpm install layout // --------------------------------------------------------------------------- From 113ae515a7471f43924af6cb83fa277c15d4e3b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 02:25:08 +0000 Subject: [PATCH 4/5] Keep yarn modules-folder inside the project The .yarnrc --modules-folder value comes from the project being scanned and names a tree apply writes patches into. An absolute or escaping value (/etc, ../elsewhere) is now ignored, like composer's vendor-dir, so the crawl and apply stay inside the project. The .yarnrc is also read with the regular-file guard, so a FIFO planted at that path can no longer hang scan, apply or vex. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 113 +++++++++++++++++- 1 file changed, 110 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index ab401e287..9fb219f3e 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -45,7 +45,10 @@ const SKIP_DIRS: &[&str] = &[ /// Only existing directories are returned. pub(super) fn configured_install_roots(start_path: &Path) -> Vec { let mut roots = Vec::new(); - if let Some(folder) = yarnrc_modules_folder(start_path) { + if let Some(folder) = yarnrc_modules_folder(start_path) + .as_deref() + .and_then(normalize_modules_folder) + { roots.push(start_path.join(folder)); } if start_path.join("rush.json").is_file() { @@ -78,14 +81,42 @@ pub(super) fn merge_configured_install_roots( } /// The `--modules-folder` value from the nearest `.yarnrc` at or above -/// `start_path`, if any `.yarnrc` sets it. +/// `start_path`, if any `.yarnrc` sets it. Read with +/// [`crate::utils::fs::read_regular_to_string_sync`]: the file belongs to +/// the (untrusted) project, and a FIFO planted there would wedge a plain +/// read forever. fn yarnrc_modules_folder(start_path: &Path) -> Option { start_path.ancestors().find_map(|dir| { - let rc = std::fs::read_to_string(dir.join(".yarnrc")).ok()?; + let rc = crate::utils::fs::read_regular_to_string_sync(&dir.join(".yarnrc")).ok()?; parse_yarnrc_modules_folder(&rc) }) } +/// Reduce a `--modules-folder` value to plain `a/b` segments under the +/// project, or `None`. The value comes from the project being scanned and +/// names a tree apply later WRITES patch content into, so (like composer's +/// `config.vendor-dir`) only a relative subpath is honored: `./deps` and +/// `lib/./deps` resolve, `..` is resolved lexically, and a value that is +/// absolute, drive-qualified, climbs above the project root or reduces to +/// it fails closed — the project then discovers nothing there, as before. +fn normalize_modules_folder(raw: &str) -> Option { + if raw.starts_with(['/', '\\']) { + return None; + } + let mut segments: Vec<&str> = Vec::new(); + for segment in raw.split(['/', '\\']) { + match segment { + "" | "." => {} + ".." => { + segments.pop()?; + } + other => segments.push(other), + } + } + let joined = segments.join("/"); + (!segments.is_empty() && path_safety::is_safe_multi_segment(&joined)).then_some(joined) +} + /// The `--modules-folder` (or command-scoped `--install.modules-folder`) /// value of a yarn classic `.yarnrc`. The file is yarn's lockfile syntax: /// one `key value` pair per line, either side optionally double-quoted, an @@ -4485,4 +4516,80 @@ mod tests { ] ); } + + /// `--modules-folder` names a patch WRITE target, so only a relative + /// subpath of the project is honored (the composer `vendor-dir` rule): + /// `.`/`..` resolve lexically, and an absolute, drive-qualified, + /// escaping or empty value is refused. + #[test] + fn test_normalize_modules_folder() { + let n = normalize_modules_folder; + assert_eq!(n("deps").as_deref(), Some("deps")); + assert_eq!(n("./deps/").as_deref(), Some("deps")); + assert_eq!(n("lib/./deps").as_deref(), Some("lib/deps")); + assert_eq!(n("lib\\deps").as_deref(), Some("lib/deps")); + assert_eq!(n("a/../deps").as_deref(), Some("deps")); + for raw in [ + "/abs/deps", + "\\abs", + "..", + "../outside", + "a/../..", + ".", + "./", + "", + "C:\\deps", + "C:deps", + ] { + assert_eq!(n(raw), None, "{raw:?}"); + } + } + + /// An escaping or absolute `--modules-folder` is not a crawl root + /// even when the directory exists: the crawl (and apply's writes) + /// stays inside the project. + #[tokio::test] + async fn test_escaping_modules_folder_is_not_a_crawl_root() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("project"); + let outside = tmp.path().join("outside"); + write_pkg(&outside.join("left-pad"), "left-pad", "1.3.0"); + std::fs::create_dir_all(&root).unwrap(); + let crawler = NpmCrawler::new(); + let abs = format!("{}", outside.display()).replace('\\', "\\\\"); + for rc in [ + "--modules-folder ../outside\n".to_string(), + format!("--modules-folder \"{abs}\"\n"), + ] { + std::fs::write(root.join(".yarnrc"), &rc).unwrap(); + let roots = crawler + .get_node_modules_paths(&local_options(&root)) + .await + .unwrap(); + assert!(roots.is_empty(), "{rc:?}: {roots:?}"); + assert!(crawler.crawl_all(&local_options(&root)).await.is_empty()); + } + } + + /// A FIFO planted at `.yarnrc` must not wedge the crawl: it is read + /// with the regular-file guard and ignored. + #[cfg(unix)] + #[test] + fn test_fifo_yarnrc_does_not_block_the_crawl() { + use std::os::unix::ffi::OsStrExt as _; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().to_path_buf(); + write_pkg(&root.join("node_modules/ms"), "ms", "2.1.3"); + let c_path = std::ffi::CString::new(root.join(".yarnrc").as_os_str().as_bytes()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c_path.as_ptr(), 0o644) }, 0); + let (tx, rx) = std::sync::mpsc::channel(); + let probe = root.clone(); + std::thread::spawn(move || { + let _ = tx.send(NpmCrawler::find_local_node_modules_dirs(&probe)); + }); + let roots = rx + .recv_timeout(std::time::Duration::from_secs(10)) + .expect("a FIFO .yarnrc must not block root discovery"); + assert_eq!(roots, vec![root.join("node_modules")]); + } } From ec95949973c26c6c218d075fcbab6dcde7e65c2d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:39:49 +0000 Subject: [PATCH 5/5] Resolve yarn modules-folder the way yarn 1.x does Two valid yarn classic setups still pointed the crawler at the wrong install root, leaving the real tree unpatched and letting vex fall back to lockfile-only evidence: - A modules folder set in an ancestor .yarnrc is relative to that file's directory, not the project. /repo/.yarnrc with "--modules-folder project/deps" installs /repo/project into /repo/project/deps; the crawler looked in project/project/deps. - --install.modules-folder wins over --modules-folder regardless of line order or which .yarnrc defines each, because yarn merges each key through the rc files separately and applies install-scoped args last. The crawler took whichever line came last. The resolved folder must still lie strictly inside the project. Verified against yarn 1.22.22 installs for all three layouts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012pqLNDF3U1KUabGfPsxoZ8 --- .../src/crawlers/npm_crawler.rs | 266 +++++++++++++++--- 1 file changed, 224 insertions(+), 42 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 9fb219f3e..861aa98df 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -34,9 +34,8 @@ const SKIP_DIRS: &[&str] = &[ /// install the project at `start_path` into, which the workspace walk /// cannot find by name (it only collects dirs literally named /// `node_modules`, and prunes `temp`): -/// - yarn classic's `--modules-folder ` from the nearest `.yarnrc` -/// (the project's or an ancestor's — yarn reads them all, nearest wins), -/// resolved against the project like yarn resolves it against its cwd; +/// - yarn classic's effective modules folder from the `.yarnrc` files at +/// or above the project (see [`yarnrc_modules_folder`]); /// - Rush's `common/temp/node_modules` when `rush.json` is at the root: /// rush runs pnpm there, so every transitive dep lives in its `.pnpm` /// store and the projects' own `node_modules` hold only links to their @@ -45,10 +44,7 @@ const SKIP_DIRS: &[&str] = &[ /// Only existing directories are returned. pub(super) fn configured_install_roots(start_path: &Path) -> Vec { let mut roots = Vec::new(); - if let Some(folder) = yarnrc_modules_folder(start_path) - .as_deref() - .and_then(normalize_modules_folder) - { + if let Some(folder) = yarnrc_modules_folder(start_path) { roots.push(start_path.join(folder)); } if start_path.join("rush.json").is_file() { @@ -80,26 +76,64 @@ pub(super) fn merge_configured_install_roots( walked } -/// The `--modules-folder` value from the nearest `.yarnrc` at or above -/// `start_path`, if any `.yarnrc` sets it. Read with -/// [`crate::utils::fs::read_regular_to_string_sync`]: the file belongs to +/// The modules folder yarn classic installs the project at `start_path` +/// into, as a path relative to the project, from the `.yarnrc` files at or +/// above it. Mirrors yarn 1.x's rc handling: +/// - each key merges through the rc hierarchy on its own, the nearest +/// `.yarnrc` defining it winning; +/// - a path value is resolved against the directory of the `.yarnrc` that +/// defines it, not the project (`/repo/.yarnrc` with +/// `--modules-folder project/deps` installs `/repo/project` into +/// `/repo/project/deps`); +/// - `--install.modules-folder` wins over `--modules-folder` wherever +/// either is defined, since yarn appends command-scoped args after the +/// general ones. +/// +/// The resolved folder must lie strictly inside the project (see +/// [`resolve_modules_folder`]), else `None`. Read with +/// [`crate::utils::fs::read_regular_to_string_sync`]: the files belong to /// the (untrusted) project, and a FIFO planted there would wedge a plain /// read forever. fn yarnrc_modules_folder(start_path: &Path) -> Option { - start_path.ancestors().find_map(|dir| { - let rc = crate::utils::fs::read_regular_to_string_sync(&dir.join(".yarnrc")).ok()?; - parse_yarnrc_modules_folder(&rc) - }) + let mut general: Option<(&Path, String)> = None; + let mut install: Option<(&Path, String)> = None; + for dir in start_path.ancestors() { + if general.is_some() && install.is_some() { + break; + } + let Ok(rc) = crate::utils::fs::read_regular_to_string_sync(&dir.join(".yarnrc")) else { + continue; + }; + let found = parse_yarnrc_modules_folder(&rc); + if general.is_none() { + general = found.general.map(|value| (dir, value)); + } + if install.is_none() { + install = found.install.map(|value| (dir, value)); + } + } + let (rc_dir, value) = install.or(general)?; + let project_in_rc_dir = start_path + .strip_prefix(rc_dir) + .ok()? + .components() + .map(|c| c.as_os_str().to_str().map(str::to_string)) + .collect::>>()?; + resolve_modules_folder(&project_in_rc_dir, &value) } -/// Reduce a `--modules-folder` value to plain `a/b` segments under the -/// project, or `None`. The value comes from the project being scanned and -/// names a tree apply later WRITES patch content into, so (like composer's -/// `config.vendor-dir`) only a relative subpath is honored: `./deps` and -/// `lib/./deps` resolve, `..` is resolved lexically, and a value that is -/// absolute, drive-qualified, climbs above the project root or reduces to -/// it fails closed — the project then discovers nothing there, as before. -fn normalize_modules_folder(raw: &str) -> Option { +/// Resolve a `.yarnrc` modules-folder `raw` value against the directory of +/// the `.yarnrc` that defines it, given the project's path below that +/// directory (`project_in_rc_dir`, empty when the `.yarnrc` is the +/// project's own), into plain `a/b` segments relative to the project, or +/// `None`. The value comes from the project being scanned and names a +/// tree apply later WRITES patch content into, so (like composer's +/// `config.vendor-dir`) only a relative value resolving strictly inside +/// the project is honored: `./deps` and `lib/./deps` resolve, `..` is +/// resolved lexically, and a value that is absolute, drive-qualified, or +/// resolves outside the project or to the project itself fails closed — +/// the project then discovers nothing there, as before. +fn resolve_modules_folder(project_in_rc_dir: &[String], raw: &str) -> Option { if raw.starts_with(['/', '\\']) { return None; } @@ -113,16 +147,32 @@ fn normalize_modules_folder(raw: &str) -> Option { other => segments.push(other), } } - let joined = segments.join("/"); - (!segments.is_empty() && path_safety::is_safe_multi_segment(&joined)).then_some(joined) + let inside = segments.get(project_in_rc_dir.len()..)?; + let at_project = segments.iter().zip(project_in_rc_dir).all(|(s, p)| s == p); + if !at_project || inside.is_empty() { + return None; + } + let joined = inside.join("/"); + path_safety::is_safe_multi_segment(&joined).then_some(joined) +} + +/// The modules-folder settings of one yarn classic `.yarnrc`, kept apart +/// because yarn merges and applies them separately. +#[derive(Debug, Default, PartialEq)] +struct YarnrcModulesFolder { + /// `--modules-folder`. + general: Option, + /// `--install.modules-folder`. + install: Option, } -/// The `--modules-folder` (or command-scoped `--install.modules-folder`) -/// value of a yarn classic `.yarnrc`. The file is yarn's lockfile syntax: -/// one `key value` pair per line, either side optionally double-quoted, an -/// optional `:` after the key, `#` comment lines. The last setting wins. -fn parse_yarnrc_modules_folder(rc: &str) -> Option { - let mut found = None; +/// The `--modules-folder` and command-scoped `--install.modules-folder` +/// values of a yarn classic `.yarnrc`. The file is yarn's lockfile +/// syntax: one `key value` pair per line, either side optionally +/// double-quoted, an optional `:` after the key, `#` comment lines. For +/// each key the last setting wins. +fn parse_yarnrc_modules_folder(rc: &str) -> YarnrcModulesFolder { + let mut found = YarnrcModulesFolder::default(); for line in rc.trim_start_matches('\u{feff}').lines() { let line = line.trim(); if line.is_empty() || line.starts_with('#') { @@ -131,14 +181,16 @@ fn parse_yarnrc_modules_folder(rc: &str) -> Option { let Some((key, rest)) = split_yarnrc_token(line, true) else { continue; }; - if key != "--modules-folder" && key != "--install.modules-folder" { - continue; - } + let slot = match key.as_str() { + "--modules-folder" => &mut found.general, + "--install.modules-folder" => &mut found.install, + _ => continue, + }; let rest = rest.trim_start(); let rest = rest.strip_prefix(':').unwrap_or(rest).trim_start(); if let Some((value, _)) = split_yarnrc_token(rest, false) { if !value.is_empty() { - found = Some(value); + *slot = Some(value); } } } @@ -4319,16 +4371,39 @@ mod tests { /// CRLF, a Windows drive path, and last-setting-wins. #[test] fn test_parse_yarnrc_modules_folder() { - let parse = parse_yarnrc_modules_folder; + let parse = |rc: &str| parse_yarnrc_modules_folder(rc).general; assert_eq!(parse("--modules-folder deps\n").as_deref(), Some("deps")); assert_eq!( parse("\"--modules-folder\" \"./my deps\"\n").as_deref(), Some("./my deps") ); assert_eq!(parse("--modules-folder: lib\n").as_deref(), Some("lib")); + // The command-scoped key is kept apart from the general one, in + // either line order: yarn merges and applies them separately. + for rc in [ + "--install.modules-folder specific\n--modules-folder general\n", + "--modules-folder general\n--install.modules-folder specific\n", + ] { + assert_eq!( + parse_yarnrc_modules_folder(rc), + YarnrcModulesFolder { + general: Some("general".into()), + install: Some("specific".into()), + }, + "{rc:?}" + ); + } assert_eq!( - parse("--install.modules-folder vendor_modules").as_deref(), - Some("vendor_modules") + parse_yarnrc_modules_folder("--install.modules-folder vendor_modules"), + YarnrcModulesFolder { + general: None, + install: Some("vendor_modules".into()), + } + ); + // Other command scopes are not the install's. + assert_eq!( + parse_yarnrc_modules_folder("--add.modules-folder x\n"), + YarnrcModulesFolder::default() ); assert_eq!( parse("\u{feff}# comment\r\nyarn-offline-mirror \"./m\"\r\n--modules-folder deps\r\n") @@ -4356,7 +4431,11 @@ mod tests { "--modules-folder\n", "--modules-folder \"unterminated\n", ] { - assert_eq!(parse(rc), None, "{rc:?}"); + assert_eq!( + parse_yarnrc_modules_folder(rc), + YarnrcModulesFolder::default(), + "{rc:?}" + ); } } @@ -4418,11 +4497,16 @@ mod tests { .unwrap(); assert_eq!(found.len(), 1); - // An ancestor's .yarnrc applies too; the nearest one wins. + // An ancestor's .yarnrc applies too (its value resolved against + // its own directory); the nearest one wins. let member = root.join("packages/member"); write_pkg(&member.join("lib/ms"), "ms", "2.1.3"); std::fs::write(member.join("package.json"), r#"{"name":"member"}"#).unwrap(); - std::fs::write(root.join(".yarnrc"), "--modules-folder lib\n").unwrap(); + std::fs::write( + root.join(".yarnrc"), + "--modules-folder packages/member/lib\n", + ) + .unwrap(); let roots = crawler .get_node_modules_paths(&local_options(&member)) .await @@ -4522,8 +4606,8 @@ mod tests { /// `.`/`..` resolve lexically, and an absolute, drive-qualified, /// escaping or empty value is refused. #[test] - fn test_normalize_modules_folder() { - let n = normalize_modules_folder; + fn test_resolve_modules_folder() { + let n = |raw: &str| resolve_modules_folder(&[], raw); assert_eq!(n("deps").as_deref(), Some("deps")); assert_eq!(n("./deps/").as_deref(), Some("deps")); assert_eq!(n("lib/./deps").as_deref(), Some("lib/deps")); @@ -4543,6 +4627,104 @@ mod tests { ] { assert_eq!(n(raw), None, "{raw:?}"); } + // Defined by an ancestor `.yarnrc`: resolved against that file's + // directory, then kept only when strictly inside the project. + let project = ["project".to_string()]; + let a = |raw: &str| resolve_modules_folder(&project, raw); + assert_eq!(a("project/deps").as_deref(), Some("deps")); + assert_eq!(a("./project/./lib\\deps").as_deref(), Some("lib/deps")); + assert_eq!(a("x/../project/deps").as_deref(), Some("deps")); + for raw in [ + "deps", + "project", + "project/..", + "../project/deps", + "projectx/deps", + "..", + ] { + assert_eq!(a(raw), None, "{raw:?}"); + } + } + + /// REVIEW (#520): a modules folder inherited from an ancestor + /// `.yarnrc` resolves against that file's directory, as yarn 1.x does: + /// `/repo/.yarnrc` `--modules-folder project/deps` installs + /// `/repo/project` into `/repo/project/deps`, not + /// `/repo/project/project/deps`. A value resolving outside the project + /// (here the sibling `/repo/deps`) is still refused. + #[tokio::test] + async fn test_inherited_yarnrc_modules_folder_resolves_against_its_dir() { + let tmp = tempfile::tempdir().unwrap(); + let repo = tmp.path(); + let project = repo.join("project"); + write_pkg(&project.join("deps/ms"), "ms", "2.1.3"); + write_pkg(&repo.join("deps/ms"), "ms", "2.1.3"); + let crawler = NpmCrawler::new(); + + std::fs::write(repo.join(".yarnrc"), "--modules-folder project/deps\n").unwrap(); + let roots = crawler + .get_node_modules_paths(&local_options(&project)) + .await + .unwrap(); + assert_eq!(roots, vec![project.join("deps")]); + + std::fs::write(repo.join(".yarnrc"), "--modules-folder deps\n").unwrap(); + let roots = crawler + .get_node_modules_paths(&local_options(&project)) + .await + .unwrap(); + assert!(roots.is_empty(), "{roots:?}"); + } + + /// REVIEW (#520): `--install.modules-folder` wins over + /// `--modules-folder` whatever their line order, and when they come + /// from different `.yarnrc` files (yarn merges each key through the + /// hierarchy on its own, then appends install-scoped args after the + /// general ones). + #[test] + fn test_install_scoped_modules_folder_takes_precedence() { + let tmp = tempfile::tempdir().unwrap(); + let repo = tmp.path(); + let project = repo.join("project"); + write_pkg(&project.join("specific/ms"), "ms", "2.1.3"); + write_pkg(&project.join("general/ms"), "ms", "2.1.3"); + let roots_for = |project_rc: Option<&str>, repo_rc: Option<&str>| { + for (dir, rc) in [(&project, project_rc), (&repo.to_path_buf(), repo_rc)] { + let path = dir.join(".yarnrc"); + match rc { + Some(rc) => std::fs::write(&path, rc).unwrap(), + None => { + let _ = std::fs::remove_file(&path); + } + } + } + NpmCrawler::find_local_node_modules_dirs(&project) + }; + let want = vec![project.join("specific")]; + for (project_rc, repo_rc) in [ + ( + Some("--install.modules-folder specific\n--modules-folder general\n"), + None, + ), + ( + Some("--modules-folder general\n--install.modules-folder specific\n"), + None, + ), + ( + Some("--modules-folder general\n"), + Some("--install.modules-folder project/specific\n"), + ), + ( + Some("--install.modules-folder specific\n"), + Some("--modules-folder project/general\n"), + ), + ] { + assert_eq!( + roots_for(project_rc, repo_rc), + want, + "{project_rc:?} / {repo_rc:?}" + ); + } } /// An escaping or absolute `--modules-folder` is not a crawl root