From ce04c66e8c21a13a6fa2e153d9b80e0de7dc28a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:23:33 +0000 Subject: [PATCH 01/11] Start fix for #359, #362 Assisted-by: Claude Code:claude-opus-5-5 From 44b29c70eb4f306851fcc3cb06f49bf2a4e7befc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:39:31 +0000 Subject: [PATCH 02/11] Find packages in npm .store and pnpm virtualStoreDir With npm's install-strategy=linked, or a pnpm virtualStoreDir moved away from node_modules/.pnpm, transitive dependencies live only in a store the crawler never walked: it matched stores by fixed name and skipped every other hidden dir. apply, scan and vendor then reported those packages as not installed and left them unpatched. The crawler now walks npm's node_modules/.store (including scoped entries one level down) and the pnpm store that .modules.yaml records, in scan, apply's resolver and the peer-copy fan-out. A recorded store outside the project, such as pnpm's global virtual store shared by other projects, is still left alone. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 499 ++++++++++++++++++ 1 file changed, 499 insertions(+) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index e9094e9f1..e752c00c2 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -211,6 +211,123 @@ fn is_legacy_pnpm_store_dir_name(name: &str) -> bool { /// The `node_modules` child that is vlt's per-project package store. const VLT_STORE_NAME: &str = ".vlt"; +/// The `node_modules` child that is npm's `install-strategy=linked` store. +const NPM_LINKED_STORE_NAME: &str = ".store"; + +/// Length of the hash suffix npm's linked strategy appends to a store key: +/// the base64url of a 16-byte shake256 digest, unpadded (arborist's +/// `isolated-reifier.js` `getKey`). +const NPM_STORE_KEY_HASH_LEN: usize = 22; + +/// Decode an npm linked-store key (`@-`, scoped +/// `@scope/@-`) into the `(package_name, version)` it +/// advertises. The hash is base64url, so it may itself hold `-`/`_`: it +/// is cut by its fixed length, never by searching for a separator. +/// +/// `None` for anything else, e.g. the un-hashed `@` dir +/// npm extracts a shrinkwrapped dependency into, or a non-semver version. +/// Like the pnpm decoder the result is advisory: the package.json probe +/// stays the authority, and `None` means "unknowable", never "empty". +fn decode_npm_store_entry_name(entry_name: &str) -> Option<(String, String)> { + let cut = entry_name.len().checked_sub(NPM_STORE_KEY_HASH_LEN + 1)?; + let (key, hash) = (entry_name.get(..cut)?, entry_name.get(cut..)?); + let hash = hash.strip_prefix('-')?; + if !hash + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_') + { + return None; + } + let at = key.rfind('@')?; + if at == 0 || key[..at].ends_with('/') { + return None; + } + let version = &key[at + 1..]; + if !is_semver_triple(version) { + return None; + } + Some((key[..at].to_string(), version.to_string())) +} + +/// The `node_modules` child in which pnpm records its install state, +/// including where the virtual store lives. +const PNPM_MODULES_YAML: &str = ".modules.yaml"; + +/// The `virtualStoreDir` value of a `.modules.yaml`: JSON on pnpm 10+, +/// YAML before (a top-level `virtualStoreDir:` scalar, maybe quoted). +fn parse_modules_yaml_virtual_store_dir(text: &str) -> Option { + let text = crate::package_json::detect::strip_bom(text); + if let Ok(value) = serde_json::from_str::(text) { + return value + .get("virtualStoreDir")? + .as_str() + .filter(|s| !s.is_empty()) + .map(str::to_string); + } + let raw = text + .lines() + .find_map(|line| line.strip_prefix("virtualStoreDir:"))? + .trim(); + let value = if raw.starts_with('"') { + serde_json::from_str::(raw).ok()? + } else if let Some(inner) = raw.strip_prefix('\'').and_then(|r| r.strip_suffix('\'')) { + inner.replace("''", "'") + } else { + raw.to_string() + }; + (!value.is_empty()).then_some(value) +} + +/// `path` with `.` and `..` resolved lexically (no filesystem access), so +/// a recorded `../.vstore` joins to the same spelling the walks use. +fn normalize_lexically(path: &Path) -> PathBuf { + let mut out = PathBuf::new(); + for component in path.components() { + match component { + std::path::Component::CurDir => {} + std::path::Component::ParentDir => { + if !out.pop() { + out.push(component); + } + } + other => out.push(other), + } + } + out +} + +/// A pnpm virtual store that `node_modules/.modules.yaml` relocates away +/// from the default `node_modules/.pnpm` (pnpm's `virtualStoreDir` +/// setting, stored relative to `node_modules`, or absolute on old pnpm). +/// +/// Only a store INSIDE the importer (the directory holding `nm`) counts, +/// reached through real directories only. Anything else, notably pnpm's +/// global virtual store (`/v10/links`), is shared by other +/// projects on the machine: patching it would patch them too, so agent +/// mode leaves it alone. `None` also for the default location, which the +/// walks already handle by name. +fn relocated_pnpm_virtual_store_sync(nm: &Path) -> Option { + let text = crate::utils::fs::read_regular_to_string_sync(&nm.join(PNPM_MODULES_YAML)).ok()?; + let recorded = parse_modules_yaml_virtual_store_dir(&text)?; + let importer = normalize_lexically(nm.parent()?); + let store = normalize_lexically(&nm.join(recorded)); + if store == normalize_lexically(&nm.join(".pnpm")) || store == normalize_lexically(nm) { + return None; + } + let below = store.strip_prefix(&importer).ok()?; + if below.as_os_str().is_empty() { + return None; + } + let mut dir = importer; + for component in below.components() { + dir.push(component); + if !std::fs::symlink_metadata(&dir).is_ok_and(|m| m.is_dir()) { + return None; + } + } + Some(dir) +} + /// `(name, version)` a `.vlt/` entry name advertises: the vlt store /// decoder over the lossless name, `None` for git/file/remote/workspace /// ids and for anything undecodable (which stays probeable). The pnpm @@ -240,6 +357,16 @@ impl StoreEntry { .collect() } + fn npm(entries: Vec<(String, PathBuf)>) -> Vec { + entries + .into_iter() + .map(|(name, node_modules)| StoreEntry { + advertised: decode_npm_store_entry_name(&name), + node_modules, + }) + .collect() + } + fn vlt(entries: Vec<(OsString, PathBuf)>) -> Vec { entries .into_iter() @@ -1053,6 +1180,41 @@ impl NpmCrawler { let entries = Self::list_vlt_store_entries_sync(&nm_path.join(&entry.name)); return vec![NestedNodeModules::StoreEntries(StoreEntry::vlt(entries))]; } + // npm's `install-strategy=linked` store: the same transitive-only + // home, at `.store/@-/node_modules/`. + if name_str == NPM_LINKED_STORE_NAME { + if !entry.file_type.is_some_and(|ft| ft.is_dir()) { + return Vec::new(); + } + let entries = Self::list_npm_store_entries_sync(&nm_path.join(&entry.name), false) + .into_iter() + .map(|e| StoreEntry { + advertised: e.advertised, + node_modules: e.node_modules, + }) + .collect(); + return vec![NestedNodeModules::StoreEntries(entries)]; + } + // A pnpm virtual store relocated by `virtualStoreDir`, found + // through the `.modules.yaml` pnpm writes next to it. It may sit + // outside this `node_modules` or under a hidden name the skip + // below would swallow. + if name_str == PNPM_MODULES_YAML { + if !entry.file_type.is_some_and(|ft| ft.is_file()) { + return Vec::new(); + } + let Some(store) = relocated_pnpm_virtual_store_sync(nm_path) else { + return Vec::new(); + }; + let entries = Self::list_pnpm_store_entries_sync(&store, false) + .into_iter() + .map(|e| StoreEntry { + advertised: e.advertised, + node_modules: e.node_modules, + }) + .collect(); + return vec![NestedNodeModules::StoreEntries(entries)]; + } // pnpm <=3: the virtual store is a hidden `.` dir // (there is no `.pnpm` at all) with the same // transitive-only-deps property, so it gets the same probing. @@ -1343,6 +1505,8 @@ impl NpmCrawler { let listing = listing.unwrap_or_else(|| list_dir_sync(node_modules_path)); let mut pnpm_store: Option = None; let mut vlt_store: Option = None; + let mut npm_store: Option = None; + let mut relocated_pnpm_store: Option = None; let mut legacy_stores: Vec = Vec::new(); let mut children: Vec<(PathBuf, String, FileType)> = Vec::new(); @@ -1375,6 +1539,23 @@ impl NpmCrawler { continue; } + // npm's linked-strategy store, deferred for the same reason. + if !store_entry && name_str == NPM_LINKED_STORE_NAME { + if entry.file_type.is_some_and(|ft| ft.is_dir()) { + npm_store = Some(node_modules_path.join(&name_str)); + } + continue; + } + + // A pnpm virtual store relocated by `virtualStoreDir` (see + // `relocated_pnpm_virtual_store_sync`), deferred like `.pnpm`. + if !store_entry && name_str == PNPM_MODULES_YAML { + if entry.file_type.is_some_and(|ft| ft.is_file()) { + relocated_pnpm_store = relocated_pnpm_virtual_store_sync(node_modules_path); + } + continue; + } + // pnpm <=3 virtual store (a hidden `.` dir; // no `.pnpm` exists on those layouts): same // transitive-only-home property, same deferred scan so @@ -1435,10 +1616,18 @@ impl NpmCrawler { .collect(); events.extend(Self::gather_store_entries(entries)); } + if let Some(store_path) = relocated_pnpm_store { + let entries = Self::list_pnpm_store_entries_sync(&store_path, true); + events.extend(Self::gather_store_entries(entries)); + } if let Some(store_path) = vlt_store { let entries = Self::vlt_store_entry_dirs(&store_path); events.extend(Self::gather_store_entries(entries)); } + if let Some(store_path) = npm_store { + let entries = Self::list_npm_store_entries_sync(&store_path, true); + events.extend(Self::gather_store_entries(entries)); + } events } @@ -1720,6 +1909,67 @@ impl NpmCrawler { run_walk(move || Self::list_vlt_store_entries_sync(&store_path)).await } + /// Enumerate npm's linked store (`node_modules/.store`, written by + /// `install-strategy=linked`), yielding every REAL entry dir whose + /// `node_modules` is a real dir, named by its store key. A scoped + /// package's entry sits one level down (`.store/@scope/@-`), + /// so a real `@scope` dir is descended once and its entries are named + /// `@scope/@-`. Skipped: dot-names, a `node_modules` child, + /// files and links (a link is never a store entry). Entries are probed + /// in parallel and yielded in listing order; with `read_listings` each + /// entry's `node_modules` listing rides along, as for pnpm. + fn list_npm_store_entries_sync(store_path: &Path, read_listings: bool) -> Vec { + let is_candidate = |entry: &ListedEntry| { + !(entry.name_str.starts_with('.') || entry.name_str == "node_modules") + && entry.file_type.is_some_and(|ft| ft.is_dir()) + }; + let mut candidates: Vec<(String, PathBuf)> = Vec::new(); + for entry in list_dir_sync(store_path).entries { + if !is_candidate(&entry) { + continue; + } + let path = store_path.join(&entry.name); + if entry.name_str.starts_with('@') { + for scoped in list_dir_sync(&path).entries { + if is_candidate(&scoped) && !scoped.name_str.starts_with('@') { + let name = format!("{}/{}", entry.name_str, scoped.name_str); + candidates.push((name, path.join(&scoped.name))); + } + } + } else { + candidates.push((entry.name_str, path)); + } + } + par_map(candidates, |(name, entry_path)| { + let node_modules = entry_path.join("node_modules"); + if !std::fs::symlink_metadata(&node_modules).is_ok_and(|m| m.is_dir()) { + return None; + } + Some(StoreEntryDir { + advertised: decode_npm_store_entry_name(&name), + name, + listing: read_listings.then(|| list_dir_sync(&node_modules)), + node_modules, + }) + }) + .into_iter() + .flatten() + .collect() + } + + /// Async `(name, node_modules)` view of + /// [`Self::list_npm_store_entries_sync`]. + async fn list_npm_store_entries(store_path: &Path) -> Vec<(String, PathBuf)> { + let store_path = store_path.to_path_buf(); + run_walk(move || { + Self::list_npm_store_entries_sync(&store_path, false) + .into_iter() + .map(|entry| (entry.name, entry.node_modules)) + .collect() + }) + .await + } + /// Descend a *nested* virtual-store host dir, yielding /// `(name@version, /node_modules)` for each package home /// found. Covers the two pre-flat layouts (both confirmed against @@ -1866,6 +2116,7 @@ impl Default for NpmCrawler { enum StoreLayout { Pnpm, Vlt, + NpmLinked, } /// Find every OTHER physical copy of the package installed at `pkg_path` @@ -1933,10 +2184,20 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { stores.push((StoreLayout::Vlt, dir.to_path_buf())); } } + // `.store` is npm's only when it sits in a `node_modules`. + Some(NPM_LINKED_STORE_NAME) + if dir.parent().and_then(Path::file_name) + == Some(OsStr::new("node_modules")) => + { + if seen_stores.insert(dir.to_path_buf()) { + stores.push((StoreLayout::NpmLinked, dir.to_path_buf())); + } + } Some("node_modules") => { for (child, layout) in [ (".pnpm", StoreLayout::Pnpm), (VLT_STORE_NAME, StoreLayout::Vlt), + (NPM_LINKED_STORE_NAME, StoreLayout::NpmLinked), ] { let store = dir.join(child); if is_dir(&store).await && seen_stores.insert(store.clone()) { @@ -1949,6 +2210,28 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { cur = dir.parent(); } } + // A pnpm store relocated by `virtualStoreDir` has no fixed name; the + // importer's `node_modules/.modules.yaml` says where it is. Every + // ancestor is a candidate importer (a transitive copy's chain runs + // through the store, not through the importer's `node_modules`). + let importer_nms: Vec = chains + .into_iter() + .flatten() + .flat_map(|start| start.ancestors().skip(1)) + .map(|dir| dir.join("node_modules")) + .collect(); + let relocated = run_walk(move || { + importer_nms + .iter() + .filter_map(|nm| relocated_pnpm_virtual_store_sync(nm)) + .collect::>() + }) + .await; + for store in relocated { + if seen_stores.insert(store.clone()) { + stores.push((StoreLayout::Pnpm, store)); + } + } if stores.is_empty() { return Vec::new(); } @@ -1969,6 +2252,9 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { StoreEntry::pnpm(NpmCrawler::list_pnpm_store_entries(&store).await) } StoreLayout::Vlt => StoreEntry::vlt(NpmCrawler::list_vlt_store_entries(&store).await), + StoreLayout::NpmLinked => { + StoreEntry::npm(NpmCrawler::list_npm_store_entries(&store).await) + } }; for StoreEntry { advertised, @@ -3441,4 +3727,217 @@ mod tests { vec![("pkg:npm/left-pad@1.3.0".to_string(), nm.join("left-pad"))] ); } + + fn scan_paths(root: &Path) -> impl std::future::Future> { + let options = CrawlerOptions { + cwd: root.to_path_buf(), + global: false, + global_prefix: None, + }; + async move { + let mut scanned: Vec<(String, PathBuf)> = NpmCrawler::new() + .crawl_all(&options) + .await + .into_iter() + .map(|p| (p.purl, p.path)) + .collect(); + scanned.sort(); + scanned + } + } + + #[test] + fn test_decode_npm_store_entry_name() { + let hash = "Pqc5my552wJdjE6sp0MCIg"; + assert_eq!( + decode_npm_store_entry_name(&format!("is-number@6.0.0-{hash}")), + Some(("is-number".to_string(), "6.0.0".to_string())) + ); + // The hash is base64url, so it can hold `-` and `_` itself. + assert_eq!( + decode_npm_store_entry_name("escape-string-regexp@1.0.5-YUOzcg-PmWvuNTSNPuN4qw"), + Some(("escape-string-regexp".to_string(), "1.0.5".to_string())) + ); + assert_eq!( + decode_npm_store_entry_name(&format!("@babel/code-frame@7.0.0-beta.1-{hash}")), + Some(("@babel/code-frame".to_string(), "7.0.0-beta.1".to_string())) + ); + // A shrinkwrapped dependency's un-hashed `name@version` dir, a + // missing hash, and a non-semver version stay undecodable. + assert_eq!(decode_npm_store_entry_name("foo@1.0.0"), None); + assert_eq!(decode_npm_store_entry_name(&format!("foo-{hash}")), None); + assert_eq!( + decode_npm_store_entry_name(&format!("foo@abc-{hash}")), + None + ); + assert_eq!(decode_npm_store_entry_name(&format!("@1.0.0-{hash}")), None); + } + + /// #359: npm's `install-strategy=linked` keeps every package in + /// `node_modules/.store/@-/node_modules/` + /// (scoped: `.store/@scope/@…/node_modules/@scope/`). The + /// importer links direct deps only, so a transitive package is a real + /// dir ONLY inside the store. Scan, the resolver and the peer-variant + /// fan-out must all see it; dependency links inside an entry stay + /// edges. + #[tokio::test] + async fn test_npm_linked_store_transitive_packages_are_found() { + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let nm = root.join("node_modules"); + let store = nm.join(".store"); + + let odd_entry = store.join("is-odd@3.0.1-6I_Y0S8g8dpI-_3nzyUbcQ/node_modules"); + write_pkg(&odd_entry.join("is-odd"), "is-odd", "3.0.1"); + let number = store.join("is-number@6.0.0-Pqc5my552wJdjE6sp0MCIg/node_modules/is-number"); + write_pkg(&number, "is-number", "6.0.0"); + // Same name@version, different dependency graph: a second hash. + let number_twin = + store.join("is-number@6.0.0-AAAAAAAAAAAAAAAAAAAAAA/node_modules/is-number"); + write_pkg(&number_twin, "is-number", "6.0.0"); + link_dir(&number, &odd_entry.join("is-number")); + let frame = store + .join("@babel/code-frame@7.0.0-wERilBtYXgdUWVgsD7hGnw/node_modules/@babel/code-frame"); + write_pkg(&frame, "@babel/code-frame", "7.0.0"); + link_dir(&odd_entry.join("is-odd"), &nm.join("is-odd")); + + let scanned = scan_paths(&root).await; + let purls: Vec<&str> = scanned.iter().map(|(p, _)| p.as_str()).collect(); + assert_eq!( + purls, + vec![ + "pkg:npm/@babel/code-frame@7.0.0", + "pkg:npm/is-number@6.0.0", + "pkg:npm/is-odd@3.0.1", + ], + "{scanned:?}" + ); + assert!(scanned.contains(&("pkg:npm/is-odd@3.0.1".to_string(), nm.join("is-odd")))); + + let targets = [ + "pkg:npm/is-number@6.0.0".to_string(), + "pkg:npm/@babel/code-frame@7.0.0".to_string(), + ]; + let found = NpmCrawler::new() + .find_by_purls(&nm, &targets) + .await + .unwrap(); + let mut numbers: Vec = found["pkg:npm/is-number@6.0.0"] + .iter() + .map(|p| p.path.clone()) + .collect(); + numbers.sort(); + let mut want = vec![number.clone(), number_twin.clone()]; + want.sort(); + assert_eq!(numbers, want); + assert_eq!(found["pkg:npm/@babel/code-frame@7.0.0"][0].path, frame); + + assert_eq!( + find_store_peer_variant_copies(&number).await, + vec![number_twin.clone()] + ); + } + + /// #362: pnpm's `virtualStoreDir` moves the virtual store, and + /// `node_modules/.modules.yaml` records where (relative to + /// `node_modules`: JSON on pnpm 10+, YAML before). A relocated store + /// inside the project is walked like `.pnpm`, wherever it sits. + #[tokio::test] + async fn test_pnpm_relocated_virtual_store_dir_is_walked() { + for (modules_yaml, store_rel) in [ + ( + "{\n \"layoutVersion\": 5,\n \"virtualStoreDir\": \"../.vstore\"\n}", + ".vstore", + ), + // Older pnpm wrote an absolute path. + ( + "layoutVersion: 5\nvirtualStoreDir: \"/.abs-store\"\n", + ".abs-store", + ), + ( + "layoutVersion: 5\nvirtualStoreDir: '.custom'\n", + "node_modules/.custom", + ), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let nm = root.join("node_modules"); + let store = root.join(store_rel); + let odd_entry = store.join("is-odd@3.0.1/node_modules"); + write_pkg(&odd_entry.join("is-odd"), "is-odd", "3.0.1"); + let number = store.join("is-number@6.0.0/node_modules/is-number"); + write_pkg(&number, "is-number", "6.0.0"); + link_dir(&number, &odd_entry.join("is-number")); + let foo = store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); + let foo_twin = store.join("foo@1.0.0(react@18.2.0)/node_modules/foo"); + write_pkg(&foo, "foo", "1.0.0"); + write_pkg(&foo_twin, "foo", "1.0.0"); + std::fs::create_dir_all(nm.join(".pnpm")).unwrap(); + let modules_yaml = + modules_yaml.replace("", &root.display().to_string().replace('\\', "\\\\")); + std::fs::write(nm.join(".modules.yaml"), modules_yaml).unwrap(); + link_dir(&odd_entry.join("is-odd"), &nm.join("is-odd")); + + let scanned = scan_paths(&root).await; + assert!( + scanned.contains(&("pkg:npm/is-number@6.0.0".to_string(), number.clone())), + "{store_rel}: {scanned:?}" + ); + assert!(scanned.iter().any(|(p, _)| p == "pkg:npm/foo@1.0.0")); + + let found = NpmCrawler::new() + .find_by_purls(&nm, &["pkg:npm/is-number@6.0.0".to_string()]) + .await + .unwrap(); + assert_eq!( + found + .get("pkg:npm/is-number@6.0.0") + .map(|c| c.iter().map(|p| p.path.clone()).collect::>()), + Some(vec![number.clone()]), + "{store_rel}" + ); + + let mut variants = find_store_peer_variant_copies(&foo).await; + variants.sort(); + assert_eq!(variants, vec![foo_twin.clone()], "{store_rel}"); + } + } + + /// A `virtualStoreDir` outside the project (pnpm's global virtual + /// store, `/v10/links`, is shared by every project on the + /// machine) is NOT walked: agent mode must not patch a shared store + /// (#361). Nor is a link planted at the recorded path. + #[tokio::test] + async fn test_pnpm_virtual_store_dir_outside_project_is_ignored() { + let outside = tempfile::tempdir().unwrap(); + let shared: PathBuf = outside.path().components().collect(); + let number = shared.join("is-number@6.0.0/node_modules/is-number"); + write_pkg(&number, "is-number", "6.0.0"); + + let tmp = tempfile::tempdir().unwrap(); + let root: PathBuf = tmp.path().components().collect(); + let nm = root.join("node_modules"); + std::fs::create_dir_all(&nm).unwrap(); + let rel = format!("{}", shared.display()).replace('\\', "\\\\"); + std::fs::write( + nm.join(".modules.yaml"), + format!("{{\"virtualStoreDir\": \"{rel}\"}}"), + ) + .unwrap(); + assert!(scan_paths(&root).await.is_empty()); + + // A link inside the project pointing at the shared store. + std::fs::write( + nm.join(".modules.yaml"), + "{\"virtualStoreDir\": \"../.vstore\"}", + ) + .unwrap(); + link_dir(&shared, &root.join(".vstore")); + assert!(scan_paths(&root).await.is_empty()); + let found = NpmCrawler::new() + .find_by_purls(&nm, &["pkg:npm/is-number@6.0.0".to_string()]) + .await + .unwrap(); + assert!(found.is_empty(), "{found:?}"); + } } From 85b446bb0b34ac33bac5a192fafca8ee3d5605c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:45:25 +0000 Subject: [PATCH 03/11] Add real npm/pnpm e2e for relocated stores Covers #359 (npm install-strategy=linked, transitive dep only in node_modules/.store: apply, rollback and vendor) and #362 (pnpm virtualStoreDir at .vstore and node_modules/.custom: apply and rollback), each checking that Node loads the patched copy. Both fail on main with package_not_installed. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_vendor_npm_build.rs | 116 ++++++++++++++++++ .../tests/e2e_vendor_pnpm_build.rs | 82 +++++++++++++ 2 files changed, 198 insertions(+) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs index e9f6a0c60..4712ad093 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs @@ -1045,3 +1045,119 @@ fn npm6_installs_a_vendored_v2_lock_from_its_legacy_mirror() { &[VexVia::Apply, VexVia::Vendor], ); } + +/// The one real package dir npm's linked store holds for `name@version` +/// (`node_modules/.store/@-/node_modules/`). +fn linked_store_copy(proj: &Path, name: &str, version: &str) -> Option { + let prefix = format!("{name}@{version}-"); + std::fs::read_dir(proj.join("node_modules/.store")) + .ok()? + .flatten() + .find(|e| e.file_name().to_string_lossy().starts_with(&prefix)) + .map(|e| e.path().join("node_modules").join(name)) +} + +/// What Node loads for `dep` when `from` requires it. +fn node_loads(proj: &Path, from: &str, dep: &str) -> String { + let script = format!( + "const p=require('path');process.stdout.write(require('fs').readFileSync(\ + require.resolve('{dep}',{{paths:[p.dirname(require.resolve('{from}'))]}}),'utf8'))" + ); + let out = Command::new("node") + .args(["-e", &script]) + .current_dir(proj) + .output() + .expect("node runs"); + assert!( + out.status.success(), + "{}", + npm_e2e_common::output_text(&out) + ); + String::from_utf8_lossy(&out.stdout).into_owned() +} + +/// #359: with `install-strategy=linked` (npm 9+), a transitive package +/// is a real dir ONLY in `node_modules/.store`. `apply` must patch the +/// copy Node loads, `rollback` must restore it, and `vendor` must build +/// its tarball from it; before the fix all three reported +/// `package_not_installed`. +#[test] +fn npm_linked_strategy_transitive_package_is_patched_rolled_back_and_vendored() { + let suite = "e2e_vendor_npm_build (linked)"; + let Some(major) = npm_major_or_skip(suite) else { + return; + }; + if major < 9 { + println!("SKIP {suite}: npm {major} has no install-strategy=linked"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::write( + proj.join("package.json"), + r#"{"name":"linked-capstone","version":"0.0.0","private":true}"#, + ) + .unwrap(); + std::fs::write(proj.join(".npmrc"), "install-strategy=linked\n").unwrap(); + let cache = tmp.path().join("npm-cache"); + // is-odd@3.0.1 depends on is-number@6.0.0: transitive, so store-only. + if !npm_e2e_common::install_fixture(suite, &proj, &cache, "is-odd@3.0.1") { + return; + } + let copy = linked_store_copy(&proj, "is-number", "6.0.0") + .expect("npm's linked strategy put is-number in node_modules/.store"); + assert!(!proj.join("node_modules/is-number").exists()); + let index = copy.join("index.js"); + let orig = std::fs::read(&index).unwrap(); + let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); + stage_patch_with_vuln( + &proj, + "pkg:npm/is-number@6.0.0", + "package/index.js", + &orig, + &patched, + "GHSA-link-npm-real", + ); + // Rollback restores from the before-blob. + std::fs::write(proj.join(".socket/blobs").join(git_sha256(&orig)), &orig).unwrap(); + let cwd = proj.to_str().unwrap(); + + let (code, stdout, stderr) = run_socket(&proj, &["apply", "--json", "--offline", "--cwd", cwd]); + assert_eq!(code, 0, "apply failed.\n{stdout}\n{stderr}"); + let env = parse_envelope(&stdout); + assert_eq!(env["status"], "success", "{env}"); + assert_eq!(env["summary"]["applied"], 1, "{env}"); + assert_eq!(std::fs::read(&index).unwrap(), patched); + assert!(node_loads(&proj, "is-odd", "is-number").starts_with(MARKER)); + + let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--json", "--cwd", cwd]); + assert_eq!(code, 0, "rollback failed.\n{stdout}\n{stderr}"); + assert_eq!(std::fs::read(&index).unwrap(), orig); + + // Rollback drops the record from the manifest; stage it again. + stage_patch_with_vuln( + &proj, + "pkg:npm/is-number@6.0.0", + "package/index.js", + &orig, + &patched, + "GHSA-link-npm-real", + ); + let lock_before = std::fs::read(proj.join("package-lock.json")).unwrap(); + let (code, stdout, stderr) = + run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]); + assert_eq!(code, 0, "vendor failed.\n{stdout}\n{stderr}"); + assert_eq!(parse_envelope(&stdout)["status"], "success", "{stdout}"); + assert_ne!( + std::fs::read(proj.join("package-lock.json")).unwrap(), + lock_before, + "vendor must rewire the lock: {stdout}" + ); + let (code, stdout, stderr) = run_socket(&proj, &["vendor", "--revert", "--json", "--cwd", cwd]); + assert_eq!(code, 0, "vendor --revert failed.\n{stdout}\n{stderr}"); + assert_eq!( + std::fs::read(proj.join("package-lock.json")).unwrap(), + lock_before + ); +} diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index 46ae6eaab..d019eafb3 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs @@ -1791,3 +1791,85 @@ fn run_legacy_capstone(pm: &str, lock_head: &str) { assert!(!proj.join(".socket/vendor").exists()); eprintln!("REVERT OK ({pm})"); } + +/// #362: pnpm's `virtualStoreDir` moves the virtual store, and a +/// transitive dependency lives only there. Agent-mode `apply` must find +/// it through `node_modules/.modules.yaml` and patch the copy Node +/// loads, and `rollback` must restore it: both for a store next to +/// `node_modules` and for one inside it under a custom hidden name. +#[test] +fn pnpm_agent_apply_patches_a_transitive_dep_in_a_relocated_virtual_store() { + if !has_corepack_pm(PNPM_PRIMARY) { + println!("SKIP: `corepack {PNPM_PRIMARY}` unavailable"); + return; + } + for (setting, store_rel) in [ + (".vstore", ".vstore"), + ("node_modules/.custom", "node_modules/.custom"), + ] { + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::write( + proj.join("package.json"), + r#"{"name":"vsd","version":"0.0.0","private":true,"dependencies":{"is-odd":"3.0.1"}}"#, + ) + .unwrap(); + std::fs::write( + proj.join("pnpm-workspace.yaml"), + format!("virtualStoreDir: {setting}\n"), + ) + .unwrap(); + let store = tmp.path().join("pnpm-store"); + let install = corepack( + &proj, + PNPM_PRIMARY, + &["install", "--store-dir", store.to_str().unwrap()], + ); + if !install.status.success() { + assert!(!pnpm_required(), "fixture install failed: {install:?}"); + println!("SKIP: fixture `pnpm install` failed: {install:?}"); + return; + } + // is-odd@3.0.1 depends on is-number@6.0.0: transitive, store-only. + let copy = proj + .join(store_rel) + .join("is-number@6.0.0/node_modules/is-number"); + let index = copy.join("index.js"); + let orig = std::fs::read(&index) + .unwrap_or_else(|e| panic!("{setting}: pnpm put is-number at {}: {e}", copy.display())); + let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); + stage_patch( + &proj, + "pkg:npm/is-number@6.0.0", + "package/index.js", + &orig, + &patched, + ); + std::fs::write(proj.join(".socket/blobs").join(git_sha256(&orig)), &orig).unwrap(); + let cwd = proj.to_str().unwrap(); + + let (code, stdout, stderr) = + run_socket(&proj, &["apply", "--json", "--offline", "--cwd", cwd]); + assert_eq!(code, 0, "{setting}: apply failed.\n{stdout}\n{stderr}"); + let env = parse_envelope(&stdout); + assert_eq!(env["summary"]["applied"], 1, "{setting}: {env}"); + assert_eq!(std::fs::read(&index).unwrap(), patched, "{setting}"); + // Node loads that very copy. + let script = "const p=require('path');process.stdout.write(require('fs').readFileSync(\ + require.resolve('is-number',{paths:[p.dirname(require.resolve('is-odd'))]}),'utf8'))"; + let out = Command::new("node") + .args(["-e", script]) + .current_dir(&proj) + .output() + .expect("node runs"); + assert!( + String::from_utf8_lossy(&out.stdout).starts_with(MARKER), + "{setting}: {out:?}" + ); + + let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--json", "--cwd", cwd]); + assert_eq!(code, 0, "{setting}: rollback failed.\n{stdout}\n{stderr}"); + assert_eq!(std::fs::read(&index).unwrap(), orig, "{setting}"); + } +} From 6e84b6d38c40f800a3abdd193008dc3eda782e11 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:48:09 +0000 Subject: [PATCH 04/11] Document npm .store and pnpm virtualStoreDir walks Assisted-by: Claude Code:claude-opus-5-5 --- CHANGELOG.md | 11 +++++++++++ docs/ecosystems.md | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b85d8bf44..3283b13bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -914,6 +914,17 @@ into the new version's section — see docs/releasing.md. ### Fixed +- **Agent mode finds packages in npm's linked store and a relocated pnpm + virtual store.** With npm's `install-strategy=linked`, transitive + packages live only in `node_modules/.store`. With a pnpm + `virtualStoreDir` setting, they live wherever `node_modules/.modules.yaml` + says. The crawler only knew `node_modules/.pnpm` (and `.vlt`), so + `scan`, `apply`, `rollback` and `vendor` reported those packages + `package_not_installed` and left them unpatched (#359, #362). Both + stores are now walked. A recorded store outside the project, such as + pnpm's global virtual store (`enableGlobalVirtualStore`), is shared + with other projects and is still not patched in place. + - **Hosted nuget redirects survive a `` in `nuget.config`.** The Socket source (and, in an existing ``, its mapping) was inserted ahead of the section's ``, which NuGet diff --git a/docs/ecosystems.md b/docs/ecosystems.md index e5a49ecda..07ad87382 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -135,6 +135,17 @@ other tools that follow the convention: neither that directory's own crawled, even when it is tagged itself. A `CACHEDIR.TAG` that lacks the signature, is a directory or is a symlink prunes nothing. +Inside each `node_modules`, the package stores of isolated layouts are +walked too, since they are the only home of transitive dependencies: +pnpm's virtual store (`node_modules/.pnpm`, pnpm <= 3's +`node_modules/.registry.*`, or the directory a `virtualStoreDir` setting +moved it to, as recorded in `node_modules/.modules.yaml`), vlt's +`node_modules/.vlt`, and npm's `install-strategy=linked` store +`node_modules/.store`. A recorded virtual store outside the project, +notably pnpm's global virtual store (`enableGlobalVirtualStore`, under the +pnpm store directory), is not walked: other projects on the machine load +the same files, so patching it in place would patch them as well. + Every command that looks for installed npm copies walks these same trees, not only `scan`. A package installed only under a pruned directory is therefore "not installed" to `scan --prune` / `--sync`, which garbage-collect its From e86d87c7237251a2246416080e7be6726bee0787 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:50:32 +0000 Subject: [PATCH 05/11] Keep peer fan-out to stores the copy lives in The fan-out that patches every peer-variant copy looked up a relocated pnpm store from any ancestor directory. For a project nested inside another pnpm project, that could pick the outer project's store and patch copies this project never loads. A relocated store now counts only when the copy being patched sits inside it. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index e752c00c2..b1b00b929 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -2211,19 +2211,27 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { } } // A pnpm store relocated by `virtualStoreDir` has no fixed name; the - // importer's `node_modules/.modules.yaml` says where it is. Every - // ancestor is a candidate importer (a transitive copy's chain runs - // through the store, not through the importer's `node_modules`). - let importer_nms: Vec = chains + // importer's `node_modules/.modules.yaml` says where it is. Only a + // store that holds the primary itself counts (a transitive copy, or a + // direct dep's link target on the canonical chain): an enclosing + // project's `.modules.yaml` further up names a store this project + // does not use. + let candidates: Vec<(PathBuf, PathBuf)> = chains .into_iter() .flatten() - .flat_map(|start| start.ancestors().skip(1)) - .map(|dir| dir.join("node_modules")) + .flat_map(|start| { + start + .ancestors() + .skip(1) + .map(move |dir| (start.to_path_buf(), dir.join("node_modules"))) + }) .collect(); let relocated = run_walk(move || { - importer_nms + candidates .iter() - .filter_map(|nm| relocated_pnpm_virtual_store_sync(nm)) + .filter_map(|(start, nm)| { + relocated_pnpm_virtual_store_sync(nm).filter(|store| start.starts_with(store)) + }) .collect::>() }) .await; @@ -3900,9 +3908,45 @@ mod tests { let mut variants = find_store_peer_variant_copies(&foo).await; variants.sort(); assert_eq!(variants, vec![foo_twin.clone()], "{store_rel}"); + // From a direct dep's importer link into the store too. + link_dir(&foo, &nm.join("foo")); + assert_eq!( + find_store_peer_variant_copies(&nm.join("foo")).await, + vec![foo_twin.clone()], + "{store_rel}" + ); } } + /// The peer-variant fan-out only uses a relocated store that holds + /// the primary: an ENCLOSING project's `.modules.yaml` names a store + /// this project never loads from, and its copies are not ours to + /// patch. + #[tokio::test] + async fn test_enclosing_projects_relocated_store_is_not_a_peer_variant_source() { + let tmp = tempfile::tempdir().unwrap(); + let outer: PathBuf = tmp.path().components().collect(); + let outer_nm = outer.join("node_modules"); + std::fs::create_dir_all(&outer_nm).unwrap(); + std::fs::write( + outer_nm.join(".modules.yaml"), + "{\"virtualStoreDir\": \"../.vstore\"}", + ) + .unwrap(); + write_pkg( + &outer.join(".vstore/foo@1.0.0(react@18.2.0)/node_modules/foo"), + "foo", + "1.0.0", + ); + + let inner_store = outer.join("app/node_modules/.pnpm"); + let primary = inner_store.join("foo@1.0.0(react@17.0.2)/node_modules/foo"); + let twin = inner_store.join("foo@1.0.0(react@16.14.0)/node_modules/foo"); + write_pkg(&primary, "foo", "1.0.0"); + write_pkg(&twin, "foo", "1.0.0"); + assert_eq!(find_store_peer_variant_copies(&primary).await, vec![twin]); + } + /// A `virtualStoreDir` outside the project (pnpm's global virtual /// store, `/v10/links`, is shared by every project on the /// machine) is NOT walked: agent mode must not patch a shared store From 4e882518aa0a474707617e955f0e085faf9315a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 19:52:46 +0000 Subject: [PATCH 06/11] Skip the linked-store e2e below npm 9.4 npm 9.0-9.3 ignore install-strategy=linked and install the hoisted tree, so the npm 9.0.0 compatibility cell has no .store to test. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs index 4712ad093..878df3f9b 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs @@ -1076,7 +1076,7 @@ fn node_loads(proj: &Path, from: &str, dep: &str) -> String { String::from_utf8_lossy(&out.stdout).into_owned() } -/// #359: with `install-strategy=linked` (npm 9+), a transitive package +/// #359: with `install-strategy=linked` (npm 9.4+), a transitive package /// is a real dir ONLY in `node_modules/.store`. `apply` must patch the /// copy Node loads, `rollback` must restore it, and `vendor` must build /// its tarball from it; before the fix all three reported @@ -1087,8 +1087,13 @@ fn npm_linked_strategy_transitive_package_is_patched_rolled_back_and_vendored() let Some(major) = npm_major_or_skip(suite) else { return; }; - if major < 9 { - println!("SKIP {suite}: npm {major} has no install-strategy=linked"); + // `install-strategy=linked` arrived in npm 9.4.0 (measured: 9.0-9.3 + // ignore it and install the hoisted tree). + let minor: u32 = npm_e2e_common::npm_version() + .and_then(|v| v.split('.').nth(1)?.parse().ok()) + .unwrap_or(0); + if major < 9 || (major == 9 && minor < 4) { + println!("SKIP {suite}: npm {major}.{minor} has no install-strategy=linked"); return; } let tmp = tempfile::tempdir().unwrap(); From a6f8bf7e4f06956c1cd31b1a0cac0c8d235d0da7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:27:10 +0000 Subject: [PATCH 07/11] Keep the caller's path spelling for relocated pnpm stores On macOS the temp dir sits under the /var -> /private/var link, so the peer-variant fan-out from a direct dep's importer link only matched the relocated store on the canonical chain and reported the copy as /private/var/..., unlike the other store layouts. Containment is now also checked canonically, so the store is kept as the caller spelled it. The regression test reaches its project through a linked ancestor so this is covered on every platform. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6 --- .../src/crawlers/npm_crawler.rs | 21 ++++++++++++++++--- 1 file changed, 18 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 b1b00b929..0f76ebae0 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -2215,7 +2215,10 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { // store that holds the primary itself counts (a transitive copy, or a // direct dep's link target on the canonical chain): an enclosing // project's `.modules.yaml` further up names a store this project - // does not use. + // does not use. Containment is also checked canonically, so a store + // named through a linked ancestor (macOS `/var` → `/private/var`) + // is kept in the same spelling as the other layouts' stores. + let canonical_start = canonical_pkg.clone(); let candidates: Vec<(PathBuf, PathBuf)> = chains .into_iter() .flatten() @@ -2230,7 +2233,12 @@ pub async fn find_store_peer_variant_copies(pkg_path: &Path) -> Vec { candidates .iter() .filter_map(|(start, nm)| { - relocated_pnpm_virtual_store_sync(nm).filter(|store| start.starts_with(store)) + relocated_pnpm_virtual_store_sync(nm).filter(|store| { + start.starts_with(store) + || canonical_start.as_ref().is_some_and(|canon| { + std::fs::canonicalize(store).is_ok_and(|s| canon.starts_with(s)) + }) + }) }) .collect::>() }) @@ -3868,7 +3876,14 @@ mod tests { ), ] { let tmp = tempfile::tempdir().unwrap(); - let root: PathBuf = tmp.path().components().collect(); + let base: PathBuf = tmp.path().components().collect(); + // Reach the project through a linked ancestor, as macOS's + // `/var` → `/private/var` temp dirs do: reported copies keep + // the spelling the caller used. + let real = base.join("real"); + std::fs::create_dir_all(&real).unwrap(); + let root = base.join("linked"); + link_dir(&real, &root); let nm = root.join("node_modules"); let store = root.join(store_rel); let odd_entry = store.join("is-odd@3.0.1/node_modules"); From 9708b3d8041cf8d067319b364d5dc07edae3f84e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:39:43 +0000 Subject: [PATCH 08/11] Refuse relocated pnpm stores outside a relative importer With the CLI's default `--cwd .` the importer is the empty relative path, and `strip_prefix("")` accepts every path, so an absolute `virtualStoreDir` such as pnpm's global virtual store passed the in-project check and was crawled (and would be patched in place). The store must now sit strictly below the importer by plain child names: no root, prefix or `..` component. Reported by Cursor Bugbot on #365. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6 --- .../scan_pnpm_relocated_store_cwd_e2e.rs | 134 ++++++++++++++++++ .../src/crawlers/npm_crawler.rs | 55 ++++++- 2 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs diff --git a/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs b/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs new file mode 100644 index 000000000..f091fcef8 --- /dev/null +++ b/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs @@ -0,0 +1,134 @@ +//! `scan` with the default `--cwd .` must not walk a pnpm `virtualStoreDir` +//! outside the project. +//! +//! The default cwd makes the importer the empty relative path, which a +//! bare `strip_prefix` accepts as a prefix of every path: an absolute +//! recorded store (pnpm's global virtual store, `/v10/links`, +//! shared by every project on the machine) then passed the in-project +//! check and was crawled, so `apply` would patch the other projects too +//! (#361). A store inside the project is still walked from the same cwd. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG: &str = "test-org"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +fn write_pkg(dir: &Path, name: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!(r#"{{ "name": "{name}", "version": "1.0.0" }}"#), + ) + .unwrap(); +} + +/// A project under `tmp/proj` whose `.modules.yaml` records +/// `virtual_store_dir`, plus a pnpm-shaped store entry for `name` at +/// `store`. +fn stage(tmp: &Path, virtual_store_dir: &str, store: &Path, name: &str) -> PathBuf { + let proj = tmp.join("proj"); + std::fs::create_dir_all(proj.join("node_modules")).unwrap(); + std::fs::write( + proj.join("package.json"), + r#"{ "name": "cwd-root", "version": "0.0.0" }"#, + ) + .unwrap(); + std::fs::write( + proj.join("node_modules/.modules.yaml"), + serde_json::to_string(&serde_json::json!({ + "layoutVersion": 5, + "virtualStoreDir": virtual_store_dir, + })) + .unwrap(), + ) + .unwrap(); + write_pkg( + &store.join(format!("{name}@1.0.0/node_modules/{name}")), + name, + ); + proj +} + +/// `scan --json` from `cwd` with no `--cwd` flag, returning the batch +/// request bodies the crawl produced. +async fn scan_bodies(cwd: &Path) -> String { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": [], "canAccessPaidPatches": false, + }))) + .mount(&server) + .await; + let mut cmd = Command::new(binary()); + cmd.arg("scan").current_dir(cwd); + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("SOCKET_") + && key.to_string_lossy() != "SOCKET_NO_CONFIG" + { + cmd.env_remove(&key); + } + } + cmd.env_remove("VIRTUAL_ENV"); + cmd.env("CARGO_HOME", cwd.join(".cargo-home")); + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd + .args([ + "--json", + "-e", + "npm", + "--api-url", + &server.uri(), + "--api-token", + "fake-token-for-test", + "--org", + ORG, + ]) + .output() + .expect("run socket-patch"); + assert!( + out.status.success(), + "stdout={}; stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + server + .received_requests() + .await + .unwrap_or_default() + .iter() + .filter(|r| r.url.path().ends_with("/patches/batch")) + .map(|r| String::from_utf8_lossy(&r.body).into_owned()) + .collect::>() + .join("\n") +} + +#[tokio::test] +async fn default_cwd_scan_skips_an_absolute_store_outside_the_project() { + let tmp = tempfile::tempdir().unwrap(); + let global = tmp.path().join("pnpm-store/v10/links"); + let proj = stage( + tmp.path(), + &global.display().to_string(), + &global, + "shared-dep", + ); + let bodies = scan_bodies(&proj).await; + assert!(!bodies.contains("shared-dep"), "{bodies}"); +} + +#[tokio::test] +async fn default_cwd_scan_walks_a_store_inside_the_project() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("proj/.vstore"); + let proj = stage(tmp.path(), "../.vstore", &store, "inproj-dep"); + let bodies = scan_bodies(&proj).await; + assert!(bodies.contains("pkg:npm/inproj-dep@1.0.0"), "{bodies}"); +} diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 0f76ebae0..9ed19c876 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -296,6 +296,19 @@ fn normalize_lexically(path: &Path) -> PathBuf { out } +/// `store` relative to `importer`, when it names a directory strictly +/// below it by plain child names only. A bare `strip_prefix` is not +/// enough: the CLI's default `--cwd .` makes the importer the empty path, +/// which is a prefix of everything, including an absolute store (`/…`, +/// `C:\…`) or one that climbs out (`../…`). +fn path_below(importer: &Path, store: &Path) -> Option { + let below = store.strip_prefix(importer).ok()?; + let plain = below + .components() + .all(|c| matches!(c, std::path::Component::Normal(_))); + (plain && !below.as_os_str().is_empty()).then(|| below.to_path_buf()) +} + /// A pnpm virtual store that `node_modules/.modules.yaml` relocates away /// from the default `node_modules/.pnpm` (pnpm's `virtualStoreDir` /// setting, stored relative to `node_modules`, or absolute on old pnpm). @@ -314,10 +327,7 @@ fn relocated_pnpm_virtual_store_sync(nm: &Path) -> Option { if store == normalize_lexically(&nm.join(".pnpm")) || store == normalize_lexically(nm) { return None; } - let below = store.strip_prefix(&importer).ok()?; - if below.as_os_str().is_empty() { - return None; - } + let below = path_below(&importer, &store)?; let mut dir = importer; for component in below.components() { dir.push(component); @@ -3962,6 +3972,43 @@ mod tests { assert_eq!(find_store_peer_variant_copies(&primary).await, vec![twin]); } + /// A relocated store counts only when it sits strictly below the + /// importer by plain child names. With the CLI's default `--cwd .` the + /// importer is the empty path, which `strip_prefix` accepts as a + /// prefix of anything, absolute or climbing out. + #[test] + fn test_path_below_accepts_only_plain_children() { + let p = Path::new; + assert_eq!( + path_below(p(""), p(".vstore")), + Some(PathBuf::from(".vstore")) + ); + assert_eq!( + path_below(p(""), p("node_modules/.custom")), + Some(PathBuf::from("node_modules/.custom")) + ); + assert_eq!( + path_below(p("/proj"), p("/proj/.vstore")), + Some(PathBuf::from(".vstore")) + ); + for (importer, store) in [ + ("", "/home/u/.local/share/pnpm/store/v10/links"), + ("", "../other/.vstore"), + ("", ""), + ("/proj", "/proj"), + ("/proj", "/other/.vstore"), + ("../proj", "../other"), + ] { + assert_eq!( + path_below(p(importer), p(store)), + None, + "{importer:?} {store:?}" + ); + } + #[cfg(windows)] + assert_eq!(path_below(p(""), p(r"C:\pnpm\links")), None); + } + /// A `virtualStoreDir` outside the project (pnpm's global virtual /// store, `/v10/links`, is shared by every project on the /// machine) is NOT walked: agent mode must not patch a shared store From 0c511eca08f345ef25f337552639693cf60b95a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 21:22:58 +0000 Subject: [PATCH 09/11] Normalize separators before mklink in crawler tests The Windows test helper shells out to `cmd /C mklink /J`, which reads a `/` inside a path (`is-odd@3.0.1/node_modules/is-number`) as a switch, so the linked-store and relocated-store tests failed on Windows with "Invalid switch". Rebuild both paths from their components first so every separator is `\`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HcXxpY7X3MC84EEfA8jws6 --- crates/socket-patch-core/src/crawlers/npm_crawler.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 9ed19c876..46b672fd9 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -3600,10 +3600,14 @@ mod tests { std::os::unix::fs::symlink(target, link).unwrap(); #[cfg(windows)] { + // Rebuilt from components so every separator is `\`: `mklink` + // reads a `/` inside a path (`a@1/node_modules/b`) as a switch. + let link: PathBuf = link.components().collect(); + let target: PathBuf = target.components().collect(); let status = std::process::Command::new("cmd") .args(["/C", "mklink", "/J"]) - .arg(link) - .arg(target) + .arg(&link) + .arg(&target) .status() .unwrap(); assert!(status.success(), "mklink /J failed"); From 6c155c6aa09c3c92e826607f709a84560c3670c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 02:59:12 +0000 Subject: [PATCH 10/11] Walk absolute in-project pnpm stores from cwd . Old pnpm records virtualStoreDir as an absolute path. With the default --cwd . the importer is the empty path, which is never a prefix of an absolute store, so a store inside the project was skipped and its transitive packages stayed package_not_installed. An absolute store is now also compared with both sides canonicalized, which also covers a project reached through a linked ancestor. A store that resolves outside the project, such as pnpm's global virtual store, is still refused. Reported by Cursor Bugbot on #365. Assisted-by: Claude Code:claude-opus-5-5 --- .../scan_pnpm_relocated_store_cwd_e2e.rs | 20 ++++- .../src/crawlers/npm_crawler.rs | 87 ++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs b/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs index f091fcef8..7fe0b5739 100644 --- a/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs +++ b/crates/socket-patch-cli/tests/scan_pnpm_relocated_store_cwd_e2e.rs @@ -6,7 +6,8 @@ //! recorded store (pnpm's global virtual store, `/v10/links`, //! shared by every project on the machine) then passed the in-project //! check and was crawled, so `apply` would patch the other projects too -//! (#361). A store inside the project is still walked from the same cwd. +//! (#361). A store inside the project is still walked from the same cwd, +//! whether it is recorded relative or absolute. use std::path::{Path, PathBuf}; use std::process::Command; @@ -132,3 +133,20 @@ async fn default_cwd_scan_walks_a_store_inside_the_project() { let bodies = scan_bodies(&proj).await; assert!(bodies.contains("pkg:npm/inproj-dep@1.0.0"), "{bodies}"); } + +/// Old pnpm records `virtualStoreDir` as an absolute path. From the +/// default cwd the importer is the empty path, which is no lexical prefix +/// of an absolute store, but a store inside the project is still walked. +#[tokio::test] +async fn default_cwd_scan_walks_an_absolute_store_inside_the_project() { + let tmp = tempfile::tempdir().unwrap(); + let store = tmp.path().join("proj/.vstore"); + let proj = stage( + tmp.path(), + &store.display().to_string(), + &store, + "absolute-dep", + ); + let bodies = scan_bodies(&proj).await; + assert!(bodies.contains("pkg:npm/absolute-dep@1.0.0"), "{bodies}"); +} diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index b7e622faa..a54526b9f 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -309,6 +309,32 @@ fn path_below(importer: &Path, store: &Path) -> Option { (plain && !below.as_os_str().is_empty()).then(|| below.to_path_buf()) } +/// [`path_below`] for a recorded `virtualStoreDir`, which old pnpm writes +/// as an absolute path. A relative importer (the default `--cwd .` makes +/// it the empty path) is never a lexical prefix of an absolute store, and +/// an absolute one may be spelled through a link (macOS `/var` → +/// `/private/var`), so an absolute store is also compared with both sides +/// canonicalized. A store that resolves outside the importer, such as +/// pnpm's global virtual store or a link planted at the recorded path, +/// still gets `None`. +fn store_below_importer(importer: &Path, store: &Path) -> Option { + if let Some(below) = path_below(importer, store) { + return Some(below); + } + if !store.is_absolute() { + return None; + } + let importer = if importer.as_os_str().is_empty() { + Path::new(".") + } else { + importer + }; + path_below( + &std::fs::canonicalize(importer).ok()?, + &std::fs::canonicalize(store).ok()?, + ) +} + /// A pnpm virtual store that `node_modules/.modules.yaml` relocates away /// from the default `node_modules/.pnpm` (pnpm's `virtualStoreDir` /// setting, stored relative to `node_modules`, or absolute on old pnpm). @@ -327,7 +353,7 @@ fn relocated_pnpm_virtual_store_sync(nm: &Path) -> Option { if store == normalize_lexically(&nm.join(".pnpm")) || store == normalize_lexically(nm) { return None; } - let below = path_below(&importer, &store)?; + let below = store_below_importer(&importer, &store)?; let mut dir = importer; for component in below.components() { dir.push(component); @@ -4047,5 +4073,64 @@ mod tests { .await .unwrap(); assert!(found.is_empty(), "{found:?}"); + + // The same link recorded as an absolute path, the form old pnpm + // writes, is still refused. + let abs = format!("{}", root.join(".vstore").display()).replace('\\', "\\\\"); + std::fs::write( + nm.join(".modules.yaml"), + format!("{{\"virtualStoreDir\": \"{abs}\"}}"), + ) + .unwrap(); + assert!(scan_paths(&root).await.is_empty()); + } + + /// Old pnpm records `virtualStoreDir` as an absolute path. A store + /// inside the project must still be walked when the project is + /// reached through a different spelling than the recorded one (a + /// linked ancestor, like macOS's `/var` → `/private/var`), which no + /// lexical prefix check can match. + #[tokio::test] + async fn test_absolute_in_project_store_is_walked_through_any_spelling() { + let tmp = tempfile::tempdir().unwrap(); + let real: PathBuf = std::fs::canonicalize(tmp.path()) + .unwrap() + .join("real") + .components() + .collect(); + let store = real.join(".vstore"); + write_pkg( + &store.join("is-number@6.0.0/node_modules/is-number"), + "is-number", + "6.0.0", + ); + let nm = real.join("node_modules"); + std::fs::create_dir_all(&nm).unwrap(); + let abs = format!("{}", store.display()).replace('\\', "\\\\"); + std::fs::write( + nm.join(".modules.yaml"), + format!("{{\"virtualStoreDir\": \"{abs}\"}}"), + ) + .unwrap(); + + // Recorded and walked spellings agree. + let scanned = scan_paths(&real).await; + assert_eq!(scanned.len(), 1, "{scanned:?}"); + + // Walked through a linked ancestor: the recorded absolute path is + // no lexical prefix match, but both resolve to the same store. + let alias = tmp.path().join("alias"); + link_dir(&real, &alias); + let scanned = scan_paths(&alias).await; + assert_eq!(scanned.len(), 1, "{scanned:?}"); + assert!(scanned[0].1.starts_with(&alias), "{scanned:?}"); + let found = NpmCrawler::new() + .find_by_purls( + &alias.join("node_modules"), + &["pkg:npm/is-number@6.0.0".to_string()], + ) + .await + .unwrap(); + assert_eq!(found.len(), 1, "{found:?}"); } } From 80f4a71e864eb56cac566c135f303833a534e723 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 03:47:26 +0000 Subject: [PATCH 11/11] Skip an absolute default pnpm store by its tail pnpm's default store, node_modules/.pnpm, is walked by name. An old or Windows pnpm can record it in .modules.yaml as an absolute path, and from --cwd . (or through a linked ancestor) that spelling missed the lexical default-location check. The relocated-store reader then accepted it too, so every store copy was reported twice. The check now compares the importer-relative tail, so any spelling of the default store, or of node_modules itself, is skipped. Reported by Cursor Bugbot on #365. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index a54526b9f..80b31c3ec 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -350,10 +350,15 @@ fn relocated_pnpm_virtual_store_sync(nm: &Path) -> Option { let recorded = parse_modules_yaml_virtual_store_dir(&text)?; let importer = normalize_lexically(nm.parent()?); let store = normalize_lexically(&nm.join(recorded)); - if store == normalize_lexically(&nm.join(".pnpm")) || store == normalize_lexically(nm) { + let below = store_below_importer(&importer, &store)?; + // The default location (or `node_modules` itself), however it is + // spelled: compared on the importer-relative tail, so an absolute + // recording of `/node_modules/.pnpm` is not walked a second + // time beside the by-name `.pnpm` handling. + let nm_name = Path::new(nm.file_name()?); + if below == nm_name || below == nm_name.join(".pnpm") { return None; } - let below = store_below_importer(&importer, &store)?; let mut dir = importer; for component in below.components() { dir.push(component); @@ -4133,4 +4138,37 @@ mod tests { .unwrap(); assert_eq!(found.len(), 1, "{found:?}"); } + + /// An absolute recording of the DEFAULT store (`node_modules/.pnpm`), + /// or of `node_modules` itself, is not a relocated store, however the + /// project path is spelled: the by-name `.pnpm` handling already walks + /// it, and a second walk would report every store copy twice. + #[test] + fn test_absolute_default_store_is_not_a_relocated_store() { + let tmp = tempfile::tempdir().unwrap(); + let real: PathBuf = std::fs::canonicalize(tmp.path()) + .unwrap() + .join("real") + .components() + .collect(); + let real_nm = real.join("node_modules"); + std::fs::create_dir_all(real_nm.join(".pnpm")).unwrap(); + let alias = tmp.path().join("alias"); + link_dir(&real, &alias); + for recorded in [real_nm.join(".pnpm"), real_nm.clone()] { + let abs = format!("{}", recorded.display()).replace('\\', "\\\\"); + std::fs::write( + real_nm.join(".modules.yaml"), + format!("{{\"virtualStoreDir\": \"{abs}\"}}"), + ) + .unwrap(); + for nm in [real_nm.clone(), alias.join("node_modules")] { + assert_eq!( + relocated_pnpm_virtual_store_sync(&nm), + None, + "{recorded:?} via {nm:?}" + ); + } + } + } }