diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 2d6acefd5..8a2c7a751 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -273,6 +273,9 @@ pub(super) fn merge_configured_install_roots( /// the (untrusted) project, and a FIFO planted there would wedge a plain /// read forever. fn yarnrc_modules_folder(start_path: &Path) -> Option { + // Walk the absolute spelling: the default `--cwd .` has no lexical + // parents, so a parent `.yarnrc` would never be read (#1902). + let start_path = &crate::utils::relpath::absolute_lexically(start_path); let mut general: Option<(&Path, String)> = None; let mut install: Option<(&Path, String)> = None; for dir in start_path.ancestors() { @@ -1542,14 +1545,13 @@ fn relocated_yarn_pnpm_store_sync(nm: &Path) -> Option { if nm.file_name() != Some(OsStr::new("node_modules")) || !is_dir_sync(nm) { return None; } - let project = nm - .parent()? - .ancestors() - .find(|dir| dir.join("yarn.lock").is_file())?; - let store = super::pkg_managers::active_yarn_pnpm_store(project)?; - let (Ok(real_store), Ok(real_project)) = - (std::fs::canonicalize(store), std::fs::canonicalize(project)) - else { + let (abs_project, project) = crate::utils::relpath::ancestors_keeping_spelling(nm.parent()?) + .find(|(abs, _)| abs.join("yarn.lock").is_file())?; + let store = super::pkg_managers::active_yarn_pnpm_store(&abs_project)?; + let (Ok(real_store), Ok(real_project)) = ( + std::fs::canonicalize(store), + std::fs::canonicalize(&abs_project), + ) else { return None; }; let below = path_below(&real_project, &real_store)?; @@ -1558,7 +1560,7 @@ fn relocated_yarn_pnpm_store_sync(nm: &Path) -> Option { { return None; } - let mut dir = project.to_path_buf(); + let mut dir = project; for component in below.components() { dir.push(component); if !std::fs::symlink_metadata(&dir).is_ok_and(|m| m.is_dir()) { diff --git a/crates/socket-patch-core/src/crawlers/pnpm_layout.rs b/crates/socket-patch-core/src/crawlers/pnpm_layout.rs index de780ca4d..5fe2ff377 100644 --- a/crates/socket-patch-core/src/crawlers/pnpm_layout.rs +++ b/crates/socket-patch-core/src/crawlers/pnpm_layout.rs @@ -119,6 +119,9 @@ fn configured_modules_dirs_in(view: &ProjectView<'_>) -> Vec { /// [`crate::utils::fs::read_regular_to_string_sync`]: the files belong to /// the (untrusted) project. fn modules_dir_setting(project: &Path) -> Option { + // Walk the absolute spelling: the default `--cwd .` has no lexical + // parents, so a workspace root's settings would never be read (#1902). + let project = &crate::utils::relpath::absolute_lexically(project); let read = |path: PathBuf| crate::utils::fs::read_regular_to_string_sync(&path).ok(); project .ancestors() diff --git a/crates/socket-patch-core/src/utils/relpath.rs b/crates/socket-patch-core/src/utils/relpath.rs index e83fcb06d..da81e1257 100644 --- a/crates/socket-patch-core/src/utils/relpath.rs +++ b/crates/socket-patch-core/src/utils/relpath.rs @@ -148,8 +148,92 @@ pub(crate) fn normalize_lexically_keeping_escapes(path: &Path) -> PathBuf { out } +/// `path` made absolute against the working directory without touching +/// the filesystem (no symlink resolution), with `.`/`..` folded. The +/// empty path (the parent of a bare `node_modules`) is the working +/// directory, which `std::path::absolute` would reject. +/// +/// For upward config walks: a relative path's lexical ancestors stop at +/// the working directory, so under the default `--cwd .` a parent's +/// `.yarnrc` or `pnpm-workspace.yaml` would never be read (#1902). +pub(crate) fn absolute_lexically(path: &Path) -> PathBuf { + let path = if path.as_os_str().is_empty() { + Path::new(".") + } else { + path + }; + let abs = std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()); + normalize_lexically_keeping_escapes(&abs) +} + +/// The directories at and above `dir`, nearest first, each as its +/// absolute path ([`absolute_lexically`]) paired with a spelling in +/// `dir`'s own terms, for an upward walk whose finds are reported the +/// way `dir` is spelled. A relative `dir` keeps its lexical ancestors +/// (`./packages/a`, `./packages`, `.`), then spells each level above the +/// working directory with `..` (`./..`, `./../..`). So one directory +/// gets one spelling whichever descendant the walk starts from. An +/// absolute `dir` walks its own lexical ancestors. +pub(crate) fn ancestors_keeping_spelling(dir: &Path) -> impl Iterator { + let relative = !dir.is_absolute(); + let abs = if relative { + absolute_lexically(dir) + } else { + dir.to_path_buf() + }; + let abs_levels = + std::iter::successors(Some(abs), |p: &PathBuf| p.parent().map(Path::to_path_buf)); + let spelled = std::iter::successors(Some(dir.to_path_buf()), move |p: &PathBuf| { + if !relative { + return p.parent().map(Path::to_path_buf); + } + match p.components().next_back() { + Some(Component::Normal(_)) => p.parent().map(Path::to_path_buf), + _ => Some(p.join("..")), + } + }); + abs_levels.zip(spelled) +} + #[cfg(test)] mod tests { + #[test] + fn ancestors_keeping_spelling_climbs_past_the_working_directory() { + let cwd = absolute_lexically(Path::new(".")); + let walk = |dir: &str| -> Vec<(PathBuf, PathBuf)> { + ancestors_keeping_spelling(Path::new(dir)).take(4).collect() + }; + let up = |n: usize| cwd.ancestors().nth(n).unwrap().to_path_buf(); + let p = PathBuf::from; + // One directory, one spelling, from either start. + assert_eq!( + walk("./packages/a"), + vec![ + (cwd.join("packages/a"), p("./packages/a")), + (cwd.join("packages"), p("./packages")), + (up(0), p(".")), + (up(1), p("./..")), + ] + ); + assert_eq!(walk(".")[1], (up(1), p("./.."))); + // The parent of a bare `node_modules` is the empty path. + assert_eq!( + walk(""), + vec![ + (up(0), p("")), + (up(1), p("..")), + (up(2), p("../..")), + (up(3), p("../../..")), + ] + ); + assert_eq!(walk("../x")[1], (up(1), p(".."))); + assert_eq!(walk("../x")[2], (up(2), p("../.."))); + // An absolute start walks its own ancestors, ending at the root. + let abs: Vec<_> = ancestors_keeping_spelling(&cwd).collect(); + assert_eq!(abs.len(), cwd.ancestors().count()); + assert!(abs.iter().all(|(a, s)| a == s)); + } + use super::*; #[test] diff --git a/crates/socket-patch-core/tests/crawler_npm_e2e.rs b/crates/socket-patch-core/tests/crawler_npm_e2e.rs index 39079287c..f837f73e8 100644 --- a/crates/socket-patch-core/tests/crawler_npm_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_npm_e2e.rs @@ -28,6 +28,24 @@ fn options_at(root: &Path) -> CrawlerOptions { } } +/// Moves into `dir` and restores the working directory however the test +/// ends. Tests using it are `#[serial]`: the cwd is process-wide. +struct CwdGuard(PathBuf); + +impl CwdGuard { + fn enter(dir: &Path) -> Self { + let prev = std::env::current_dir().unwrap(); + std::env::set_current_dir(dir).unwrap(); + Self(prev) + } +} + +impl Drop for CwdGuard { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.0); + } +} + /// Stage a package inside node_modules. `name` may include a `@scope/` /// prefix. async fn stage_npm_pkg(node_modules: &Path, name: &str, version: &str) { @@ -5026,15 +5044,7 @@ async fn relative_deno_npm_dir_prefix_expands() { let registry = npm.join("registry.npmjs.org"); stage_deno_npm_pkg(®istry, "is-odd", "3.0.1", "3.0.1").await; - /// Restores the working directory however the test ends. - struct CwdGuard(PathBuf); - impl Drop for CwdGuard { - fn drop(&mut self) { - let _ = std::env::set_current_dir(&self.0); - } - } - let _guard = CwdGuard(std::env::current_dir().unwrap()); - std::env::set_current_dir(&npm).unwrap(); + let _guard = CwdGuard::enter(&npm); let opts = CrawlerOptions { cwd: npm.clone(), global: false, @@ -5044,3 +5054,133 @@ async fn relative_deno_npm_dir_prefix_expands() { assert_eq!(roots.len(), 1, "{roots:?}"); assert!(roots[0].ends_with("registry.npmjs.org"), "{roots:?}"); } + +fn sorted_purls(pkgs: &[socket_patch_core::crawlers::types::CrawledPackage]) -> Vec { + let mut purls: Vec = pkgs.iter().map(|p| p.purl.clone()).collect(); + purls.sort(); + purls.dedup(); + purls +} + +/// REGRESSION (#1902): under the default `--cwd .` the start path is the +/// relative `.`, whose lexical ancestors are only `.` and `""`, so a +/// parent `.yarnrc`'s `--modules-folder` was never read and the packages +/// installed there went unreported. Scanning with `.` must find what +/// scanning with the absolute path finds, spelled relative to `.`. +#[tokio::test] +#[serial_test::serial] +async fn parent_yarnrc_modules_folder_is_read_from_a_relative_cwd() { + let tmp = tempfile::tempdir().unwrap(); + let repo = std::fs::canonicalize(tmp.path()).unwrap(); + let project = repo.join("project"); + stage_npm_pkg(&project.join("deps"), "foo", "1.0.0").await; + std::fs::write(repo.join(".yarnrc"), "--modules-folder project/deps\n").unwrap(); + let want = vec!["pkg:npm/foo@1.0.0".to_string()]; + + // Control: the absolute spelling has always worked. + let crawler = NpmCrawler::new(); + assert_eq!( + sorted_purls(&crawler.crawl_all(&options_at(&project)).await), + want + ); + + let _cwd = CwdGuard::enter(&project); + let relative = options_at(Path::new(".")); + let roots = crawler.get_node_modules_paths(&relative).await.unwrap(); + assert_eq!(roots, vec![Path::new(".").join("deps")]); + let pkgs = crawler.crawl_all(&relative).await; + assert_eq!(sorted_purls(&pkgs), want); + assert_eq!(pkgs[0].path, Path::new(".").join("deps").join("foo")); +} + +/// REGRESSION (#1902): pnpm's `modulesDir` set in the workspace root's +/// `pnpm-workspace.yaml` applies to every member, but scanned from a +/// member under the default `--cwd .` the root file was never read. +#[tokio::test] +#[serial_test::serial] +async fn parent_pnpm_modules_dir_is_read_from_a_relative_member_cwd() { + let tmp = tempfile::tempdir().unwrap(); + let root = std::fs::canonicalize(tmp.path()).unwrap(); + std::fs::write( + root.join("pnpm-workspace.yaml"), + "packages:\n - packages/*\nmodulesDir: deps\n", + ) + .unwrap(); + let member = root.join("packages").join("a"); + stage_npm_pkg(&member.join("deps"), "left-pad", "1.3.0").await; + let want = vec!["pkg:npm/left-pad@1.3.0".to_string()]; + + // Control: the absolute spelling has always worked. + let crawler = NpmCrawler::new(); + assert_eq!( + sorted_purls(&crawler.crawl_all(&options_at(&member)).await), + want + ); + + let _cwd = CwdGuard::enter(&member); + let pkgs = crawler.crawl_all(&options_at(Path::new("."))).await; + assert_eq!(sorted_purls(&pkgs), want); + assert_eq!(pkgs[0].path, Path::new(".").join("deps").join("left-pad")); +} + +/// REGRESSION (#1902): scanned from a yarn workspace member under the +/// default `--cwd .`, the member's `node_modules` is the relative +/// `node_modules`, whose lexical ancestors never reach the root's +/// `yarn.lock`. So Yarn's pnpm-linker store, relocated by +/// `pnpmStoreFolder` and the only home of the transitive packages, was +/// never walked. It is found, spelled relative to the cwd. (Unix only: +/// the store's self-links are symlinks; the unit tests cover the Windows +/// junction layout.) +#[cfg(unix)] +#[tokio::test] +#[serial_test::serial] +async fn parent_yarn_pnpm_store_is_found_from_a_relative_member_cwd() { + let tmp = tempfile::tempdir().unwrap(); + let root = std::fs::canonicalize(tmp.path()).unwrap(); + std::fs::write(root.join("yarn.lock"), "").unwrap(); + std::fs::write( + root.join(".yarnrc.yml"), + "nodeLinker: pnpm\npnpmStoreFolder: store\n", + ) + .unwrap(); + let entry = root.join("store").join("is-number-npm-6.0.0-9f8e7d6c5b"); + let pkg = entry.join("package"); + tokio::fs::create_dir_all(&pkg).await.unwrap(); + tokio::fs::write( + pkg.join("package.json"), + r#"{"name":"is-number","version":"6.0.0"}"#, + ) + .await + .unwrap(); + // Yarn 4 links each entry's own package into the entry's + // `node_modules`, which is how the store walk recognizes an entry. + tokio::fs::create_dir_all(entry.join("node_modules")) + .await + .unwrap(); + std::os::unix::fs::symlink("../package", entry.join("node_modules/is-number")).unwrap(); + let member = root.join("packages").join("a"); + stage_npm_pkg(&member.join("node_modules"), "is-odd", "3.0.1").await; + let want = vec![ + "pkg:npm/is-number@6.0.0".to_string(), + "pkg:npm/is-odd@3.0.1".to_string(), + ]; + + // Control: the absolute spelling has always worked. + let crawler = NpmCrawler::new(); + assert_eq!( + sorted_purls(&crawler.crawl_all(&options_at(&member)).await), + want + ); + + let _cwd = CwdGuard::enter(&member); + let pkgs = crawler.crawl_all(&options_at(Path::new("."))).await; + assert_eq!(sorted_purls(&pkgs), want); + let number = pkgs.iter().find(|p| p.name == "is-number").unwrap(); + assert_eq!( + number.path, + Path::new("./../..") + .join("store") + .join("is-number-npm-6.0.0-9f8e7d6c5b") + .join("package") + ); +}