From 870728ce2bbfcefad4cf086c076873bb54fd23b9 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:24:19 -0600 Subject: [PATCH 1/3] Skip unsafe Rush subspace locks in the lock inventory Lockfile discovery lists a Rush monorepo's subspace pnpm locks with `rush_lock_rels`, which keeps only real directories whose names pass `is_safe_single_segment` (no `\`, `:`, traversal) and are UTF-8, and reads them through the `ProjectView`. The lock inventory listed the same locks separately: on disk and Snapshot views it ran its own `read_dir` with no name check and read each lock by absolute path, so it inventoried packages from subspace locks discovery deliberately skips (e.g. a subspace named `a:b`), and under a Snapshot view it bypassed the snapshot cache, overlay and read tracking, so the two passes could see different bytes in one run. The Memory branch listed children without the name check too. `inventory_rush_pnpm_locks_in` now takes its paths from `rush_lock_rels(view)` and reads each through the view with `inventory_pnpm_lock_rel_in`, for every view kind. The `rush.json` gate is unchanged (disk/Snapshot: `metadata` on disk; Memory: `contains`). The disk-only `inventory_rush_pnpm_locks` and the duplicated Memory listing are gone, and `inventory_pnpm_lock_at`, now used only by the FIFO test, is `#[cfg(test)]`. Tested: new `rush_skips_unsafe_subspace_names`, the inventory twin of discovery's `pnpm_rush_without_subspaces_and_unsafe_subspace_names` (Unix only, as `:` can't be created on Windows); it fails on the old code and passes now. The existing Rush pins (`rush_monorepo_inventories_common_and_subspace_locks`, `rush_json_without_any_lock_yields_none`, `root_pnpm_lock_wins_over_rush_fallback`), discovery's Rush tests and the vex-discover golden corpus (incl. `nested-rush-lock`) still pass. Co-Authored-By: Claude Opus 5.5 --- .../src/vendor/lock_inventory/pnpm.rs | 84 ++++++------------- .../src/vendor/lock_inventory/tests.rs | 35 ++++++++ 2 files changed, 60 insertions(+), 59 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs index 9efe7dfd7..f7320557b 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs @@ -2,12 +2,9 @@ //! Rush's pnpm locks: Rush's lock enumeration and the registry view over the //! format's model ([`crate::formats::pnpm::PnpmLock`]). -use std::path::Path; - use crate::constants::npm_family::{PNPM_LOCK, RUSH_COMMON_LOCK_REL, RUSH_SUBSPACES_DIR}; use crate::formats::pnpm::PnpmLock; use crate::patch::path_safety::is_safe_single_segment; -use crate::utils::fs::read_regular_to_string; use super::view::ProjectView; use super::LockfileEntry; @@ -43,7 +40,7 @@ pub(crate) async fn rush_lock_rels(view: ProjectView<'_>) -> Vec { // ── registry view ── #[cfg(test)] -pub(super) async fn inventory_pnpm_lock(root: &Path) -> Option> { +pub(super) async fn inventory_pnpm_lock(root: &std::path::Path) -> Option> { inventory_pnpm_lock_in(&ProjectView::Disk(root)).await } @@ -51,7 +48,10 @@ pub(super) async fn inventory_pnpm_lock_in(view: &ProjectView<'_>) -> Option, rel: &str, @@ -60,11 +60,15 @@ pub(super) async fn inventory_pnpm_lock_rel_in( pnpm_lock_text_inventory(&text) } -/// Inventory a specific `pnpm-lock.yaml` (path given explicitly so the Rush -/// fallback can point it at `common/config/rush/…` and subspace locks): -/// [`PnpmLock::entries`], `None` when the lock has no `packages:` section. -pub(super) async fn inventory_pnpm_lock_at(lock_path: &Path) -> Option> { - let text = read_regular_to_string(lock_path).await.ok()?; +/// Inventory a `pnpm-lock.yaml` at an absolute path (the FIFO test's +/// handle on a lock outside the root-relative names). +#[cfg(test)] +pub(super) async fn inventory_pnpm_lock_at( + lock_path: &std::path::Path, +) -> Option> { + let text = crate::utils::fs::read_regular_to_string(lock_path) + .await + .ok()?; pnpm_lock_text_inventory(&text) } @@ -104,66 +108,28 @@ fn pnpm_lock_text_inventory(text: &str) -> Option> { /// /// Only called (via [`inventory_npm_lock`]) when there is NO root lock but /// `rush.json` is present, so it never shadows a plain pnpm project. The -/// subspace directory is read sorted for deterministic output. Missing -/// files/dirs are skipped fail-soft; the caller drops the whole result when -/// it comes back empty. +/// locks are the ones lockfile discovery reads ([`rush_lock_rels`]: sorted, +/// real dirs with traversal-safe UTF-8 names), each read through the view. +/// Missing files/dirs are skipped fail-soft; the caller drops the whole +/// result when it comes back empty. pub(super) async fn inventory_rush_pnpm_locks_in(view: &ProjectView<'_>) -> Vec { - let project = match view { + let is_rush = match view { ProjectView::Disk(_) | ProjectView::Snapshot(_) => { let project_root = view.disk_root().expect("a disk view has a root"); - return inventory_rush_pnpm_locks(project_root).await; + tokio::fs::metadata(project_root.join("rush.json")) + .await + .is_ok() } - ProjectView::Memory(project) => *project, + ProjectView::Memory(project) => project.contains("rush.json"), }; - if !project.contains("rush.json") { + if !is_rush { return Vec::new(); } let mut out = Vec::new(); - if let Some(entries) = inventory_pnpm_lock_rel_in(view, RUSH_COMMON_LOCK_REL).await { - out.extend(entries); - } - for (name, is_dir) in project.children(RUSH_SUBSPACES_DIR) { - if !is_dir { - continue; - } - let rel = format!("{RUSH_SUBSPACES_DIR}/{name}/{PNPM_LOCK}"); + for rel in rush_lock_rels(*view).await { if let Some(entries) = inventory_pnpm_lock_rel_in(view, &rel).await { out.extend(entries); } } out } - -async fn inventory_rush_pnpm_locks(project_root: &Path) -> Vec { - if tokio::fs::metadata(project_root.join("rush.json")) - .await - .is_err() - { - return Vec::new(); - } - let mut out = Vec::new(); - - // The single source-of-truth lock. - let common_lock = project_root.join(RUSH_COMMON_LOCK_REL); - if let Some(entries) = inventory_pnpm_lock_at(&common_lock).await { - out.extend(entries); - } - - // Per-subspace locks, sorted for determinism. - let subspaces_dir = project_root.join(RUSH_SUBSPACES_DIR); - if let Ok(mut read_dir) = tokio::fs::read_dir(&subspaces_dir).await { - let mut subspace_dirs: Vec = Vec::new(); - while let Ok(Some(entry)) = read_dir.next_entry().await { - if entry.file_type().await.is_ok_and(|t| t.is_dir()) { - subspace_dirs.push(entry.path()); - } - } - subspace_dirs.sort(); - for dir in subspace_dirs { - if let Some(entries) = inventory_pnpm_lock_at(&dir.join(PNPM_LOCK)).await { - out.extend(entries); - } - } - } - out -} diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index bd14058d0..4556cf71c 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -785,6 +785,41 @@ packages: ); } +/// The inventory twin of discovery's +/// `pnpm_rush_without_subspaces_and_unsafe_subspace_names`: a subspace dir +/// whose name is not a safe path segment is never read, so the inventory +/// lists no package that discovery skips. +#[tokio::test] +async fn rush_skips_unsafe_subspace_names() { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; + write_nested(tmp.path(), "common/config/rush/pnpm-lock.yaml", PNPM_LOCK).await; + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); + + // `:` is not a legal Windows path character: Unix only. + if cfg!(unix) { + write_nested( + tmp.path(), + "common/config/subspaces/a:b/pnpm-lock.yaml", + "lockfileVersion: '9.0' + +packages: + + only-in-unsafe@1.0.0: + resolution: {integrity: sha512-unsafe==} +", + ) + .await; + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); + assert!( + !entries.iter().any(|e| e.name == "only-in-unsafe"), + "an unsafe subspace name must not be read: {entries:?}" + ); + } +} + // ── yarn classic ────────────────────────────────────────────────────── const YARN_CLASSIC: &str = "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. From f6f32f2670e0886b8feebd874b0b79bd4b821ca7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:10:29 -0600 Subject: [PATCH 2/3] Share one Rush gate and refuse symlinked Rush lock dirs Review follow-ups on the Rush subspace listing fix. The inventory's rush.json gate went around the view on Disk and Snapshot views (`disk_root()` + `tokio::fs::metadata`). That marked a tracked snapshot's recording unusable, kept rush.json out of the ReadSet and ignored overlays. It also followed links, where discovery uses lstat (`exists_no_follow`), so a dangling rush.json symlink or an implied in-memory `rush.json/` directory got different answers from the two. The member-lock reader used a third check (`is_file`). Now one `is_rush(view)` (`exists_no_follow("rush.json")`) gates discovery, the Rush lock reader and the member-lock reader. `rush_lock_rels` only checked that the subspace leaf was a real directory. A symlinked `common`, `common/config`, `common/config/rush` or `common/config/subspaces` was followed, so another repository's locks could be read. Any symlinked component now leaves its locks out. The `#[cfg(test)]` `inventory_pnpm_lock_at` helper is gone. The FIFO test reads `shrinkwrap.yaml` through the production `inventory_pnpm_lock_rel_in(&ProjectView::Disk(..), ..)`. The Rush fallback's docs now give the condition it actually runs under: any non-PnP flavor-probe refusal that no earlier fallback answered. Tests, each failing before this change: - rush_gate_sees_the_snapshot_overlay - rush_gate_keeps_the_recording_reusable - rush_gate_counts_a_dangling_symlink (Unix) - rush_gate_counts_a_memory_implied_dir - rush_member_locks_gate_matches_discovery - rush_refuses_symlinked_lock_dirs (Unix) rush_skips_unsafe_subspace_names now also runs through Snapshot and Memory views. The Memory case runs on every platform and covers both `a:b` and `a\b`. Co-Authored-By: Claude Opus 5.5 --- .../src/vendor/lock_inventory/mod.rs | 2 +- .../src/vendor/lock_inventory/npm_family.rs | 6 +- .../src/vendor/lock_inventory/pnpm.rs | 99 +++++---- .../src/vendor/lock_inventory/tests.rs | 202 ++++++++++++++++-- .../socket-patch-core/src/vex/discover/npm.rs | 4 +- 5 files changed, 245 insertions(+), 68 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs b/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs index 3d4bc58fb..6d41e3a73 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs @@ -95,7 +95,7 @@ use self::{ gem::gem_remotes, npm::inventory_package_lock, npm_family::finalize_npm, - pnpm::{inventory_pnpm_lock, inventory_pnpm_lock_at}, + pnpm::{inventory_pnpm_lock, inventory_pnpm_lock_rel_in}, pypi::{is_public_pypi_url, python_lock_inventory, socket_reference_coords}, recover::pure_wheel_from_uv_unit, vlt::inventory_vlt, diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs index 208934cf1..3b3edc459 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/npm_family.rs @@ -146,9 +146,9 @@ pub(super) async fn inventory_npm_lock_raw_in( } // Rush monorepos have no root package.json/lock pair; their // single pnpm source-of-truth lives under common/config/rush/. - // The flavor probe (root-relative) can't see it, so fall back - // explicitly when the root lock is absent but rush.json is - // present. + // The flavor probe (root-relative) can't see it, so every + // refusal that got this far falls back to Rush's locks, which + // read nothing outside a Rush monorepo (no rush.json). let rush = inventory_rush_pnpm_locks_in(view).await; return Ok((!rush.is_empty()).then(|| (NpmLockFlavor::Pnpm, guard_npm(rush)))); } diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs index f7320557b..fadbe0fb2 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs @@ -11,32 +11,57 @@ use super::LockfileEntry; // ── file selection ── +/// Whether the project is a Rush monorepo: a `rush.json` at the root, +/// probed through the view without following links (a dangling link or, +/// in memory, an implied directory counts). The one gate lockfile +/// discovery and every lock-inventory reader share, so they agree on which +/// locks a Rush project installs from. +pub(crate) async fn is_rush(view: ProjectView<'_>) -> bool { + view.exists_no_follow("rush.json").await +} + /// A Rush monorepo's pnpm locks, root-relative, in lockfile discovery's -/// order: the single source-of-truth lock -/// ([`RUSH_COMMON_LOCK_REL`]), then every subspace's -/// `common/config/subspaces//pnpm-lock.yaml`, sorted by name — real -/// directories only (a symlinked subspace dir could point the read outside -/// the project) with traversal-safe UTF-8 names. Stat / list only; whether -/// the project IS a Rush monorepo (`rush.json`) is the caller's check. +/// order: the single source-of-truth lock ([`RUSH_COMMON_LOCK_REL`]), then +/// every subspace's `common/config/subspaces//pnpm-lock.yaml`, sorted +/// by name. Every directory on the way is a real one, never a symlink that +/// could point the read at another repository: `common`, `common/config` +/// and `common/config/rush` for the common lock (else it is left out); +/// `common`, `common/config`, `common/config/subspaces` and each subspace +/// dir for the subspace locks (else they are). Subspace names must be +/// traversal-safe UTF-8 segments. Stat / list only; whether the project IS +/// a Rush monorepo is the caller's [`is_rush`] check. pub(crate) async fn rush_lock_rels(view: ProjectView<'_>) -> Vec { - let mut names: Vec = view - .list_dir(RUSH_SUBSPACES_DIR) - .await - .unwrap_or_default() - .into_iter() - .filter(|entry| entry.is_dir && is_safe_single_segment(&entry.name)) - .map(|entry| entry.name) - .collect(); - names.sort(); - let mut rels = vec![RUSH_COMMON_LOCK_REL.to_string()]; + let mut rels = Vec::new(); + if !has_symlinked_dir(view, RUSH_COMMON_LOCK_DIR) { + rels.push(RUSH_COMMON_LOCK_REL.to_string()); + } + if has_symlinked_dir(view, RUSH_SUBSPACES_DIR) { + return rels; + } + // The listing is sorted by name; `is_dir` does not follow links. rels.extend( - names + view.list_dir(RUSH_SUBSPACES_DIR) + .await + .unwrap_or_default() .into_iter() - .map(|name| format!("{RUSH_SUBSPACES_DIR}/{name}/{PNPM_LOCK}")), + .filter(|entry| entry.is_dir && is_safe_single_segment(&entry.name)) + .map(|entry| format!("{RUSH_SUBSPACES_DIR}/{}/{PNPM_LOCK}", entry.name)), ); rels } +/// The directory holding [`RUSH_COMMON_LOCK_REL`]. +const RUSH_COMMON_LOCK_DIR: &str = "common/config/rush"; + +/// Whether root-relative directory `dir` or any directory above it (below +/// the root) is a symlink. +fn has_symlinked_dir(view: ProjectView<'_>, dir: &str) -> bool { + dir.match_indices('/') + .map(|(at, _)| &dir[..at]) + .chain([dir]) + .any(|rel| view.is_symlink(rel)) +} + // ── registry view ── #[cfg(test)] @@ -60,18 +85,6 @@ pub(super) async fn inventory_pnpm_lock_rel_in( pnpm_lock_text_inventory(&text) } -/// Inventory a `pnpm-lock.yaml` at an absolute path (the FIFO test's -/// handle on a lock outside the root-relative names). -#[cfg(test)] -pub(super) async fn inventory_pnpm_lock_at( - lock_path: &std::path::Path, -) -> Option> { - let text = crate::utils::fs::read_regular_to_string(lock_path) - .await - .ok()?; - pnpm_lock_text_inventory(&text) -} - /// Inventory the workspace members' own locks pnpm installs from under /// `sharedWorkspaceLockfile: false` (#492), the same ones hosted mode pins /// and discovery reads ([`member_locks`]): empty for a shared lock, an @@ -79,7 +92,7 @@ pub(super) async fn inventory_pnpm_lock_at( /// instead). A member lock that cannot be read adds nothing. pub(super) async fn inventory_pnpm_member_locks_in(view: &ProjectView<'_>) -> Vec { use crate::utils::pnpm_workspace::{member_locks, MemberLocks}; - if view.is_file("rush.json") { + if is_rush(*view).await { return Vec::new(); } let MemberLocks::PerMember(keys) = member_locks(view).await else { @@ -106,23 +119,19 @@ fn pnpm_lock_text_inventory(text: &str) -> Option> { /// `common/config/subspaces//pnpm-lock.yaml`. `rush install` copies /// the source lock into common/temp and runs pnpm there. /// -/// Only called (via [`inventory_npm_lock`]) when there is NO root lock but -/// `rush.json` is present, so it never shadows a plain pnpm project. The -/// locks are the ones lockfile discovery reads ([`rush_lock_rels`]: sorted, -/// real dirs with traversal-safe UTF-8 names), each read through the view. +/// The last fallback of [`super::npm_family::inventory_npm_lock_raw_in`]: +/// reached whenever the npm-family flavor probe refuses the project (any +/// refusal but the PnP ones, which propagate) and no earlier fallback (the +/// out-of-family root pnpm lock, a live sibling lock, pnpm <= 2's +/// `shrinkwrap.yaml`, the members' own locks) returned entries — typically +/// no root lock at all, where the probe refuses with +/// `vendor_rush_unsupported` or `vendor_lockfile_missing`. Outside a Rush +/// monorepo ([`is_rush`]) it reads nothing. The locks are the ones lockfile +/// discovery reads ([`rush_lock_rels`]), each read through the view. /// Missing files/dirs are skipped fail-soft; the caller drops the whole /// result when it comes back empty. pub(super) async fn inventory_rush_pnpm_locks_in(view: &ProjectView<'_>) -> Vec { - let is_rush = match view { - ProjectView::Disk(_) | ProjectView::Snapshot(_) => { - let project_root = view.disk_root().expect("a disk view has a root"); - tokio::fs::metadata(project_root.join("rush.json")) - .await - .is_ok() - } - ProjectView::Memory(project) => project.contains("rush.json"), - }; - if !is_rush { + if !is_rush(*view).await { return Vec::new(); } let mut out = Vec::new(); diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 4556cf71c..ed2b13040 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -791,35 +791,203 @@ packages: /// lists no package that discovery skips. #[tokio::test] async fn rush_skips_unsafe_subspace_names() { - let tmp = tempfile::tempdir().unwrap(); - write(tmp.path(), "rush.json", r#"{"rushVersion":"5.0.0"}"#).await; - write_nested(tmp.path(), "common/config/rush/pnpm-lock.yaml", PNPM_LOCK).await; - let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); - assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); + // Memory: every platform, both unsafe spellings. + let mut project = MemoryProject::new(); + project.insert_text("rush.json", RUSH_JSON); + project.insert_text(RUSH_COMMON_LOCK, PNPM_LOCK); + project.insert_text("common/config/subspaces/a:b/pnpm-lock.yaml", OUTSIDE_LOCK); + project.insert_text("common/config/subspaces/a\\b/pnpm-lock.yaml", OUTSIDE_LOCK); + let names = npm_inventory_names(&ProjectView::Memory(&project)).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); + assert!(!names.contains(&"only-outside".to_string()), "{names:?}"); + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", RUSH_JSON).await; + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; // `:` is not a legal Windows path character: Unix only. if cfg!(unix) { write_nested( tmp.path(), "common/config/subspaces/a:b/pnpm-lock.yaml", - "lockfileVersion: '9.0' - -packages: - - only-in-unsafe@1.0.0: - resolution: {integrity: sha512-unsafe==} -", + OUTSIDE_LOCK, ) .await; - let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); - assert_eq!(entry(&entries, "left-pad").version, "1.3.0"); + } + let snapshot = DiskSnapshot::new(tmp.path()); + for view in [ + ProjectView::Disk(tmp.path()), + ProjectView::Snapshot(&snapshot), + ] { + let names = npm_inventory_names(&view).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); assert!( - !entries.iter().any(|e| e.name == "only-in-unsafe"), - "an unsafe subspace name must not be read: {entries:?}" + !names.contains(&"only-outside".to_string()), + "an unsafe subspace name must not be read: {names:?}" ); } } +const RUSH_JSON: &str = r#"{"rushVersion":"5.0.0"}"#; +const RUSH_COMMON_LOCK: &str = "common/config/rush/pnpm-lock.yaml"; +/// A lock whose only package marks a read the inventory must not make. +const OUTSIDE_LOCK: &str = "lockfileVersion: '9.0' + +packages: + + only-outside@1.0.0: + resolution: {integrity: sha512-outside==} +"; + +/// The sorted package names the npm-family inventory lists through `view` +/// (empty when it lists none). +async fn npm_inventory_names(view: &ProjectView<'_>) -> Vec { + let entries = super::npm_family::inventory_npm_lock_in(view) + .await + .unwrap() + .map(|(_, entries)| entries) + .unwrap_or_default(); + let mut names: Vec = entries.into_iter().map(|e| e.name).collect(); + names.sort(); + names +} + +// The inventory's `rush.json` gate is discovery's (`exists_no_follow` +// through the view), pinned by the four `rush_gate_*` tests below. + +/// A rush.json only in a Snapshot's overlay counts. +#[tokio::test] +async fn rush_gate_sees_the_snapshot_overlay() { + let tmp = tempfile::tempdir().unwrap(); + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; + let snapshot = DiskSnapshot::new(tmp.path()); + snapshot.overlay("rush.json", RUSH_JSON.as_bytes()); + let names = npm_inventory_names(&ProjectView::Snapshot(&snapshot)).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); +} + +/// The gate probes through the view, so a tracked snapshot's recording +/// stays reusable and covers `rush.json`. +#[tokio::test] +async fn rush_gate_keeps_the_recording_reusable() { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", RUSH_JSON).await; + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; + let tracked = DiskSnapshot::tracked(tmp.path()); + tracked.begin_recording(); + let names = npm_inventory_names(&ProjectView::Snapshot(&tracked)).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); + let reads = tracked + .end_recording() + .expect("the rush gate must not read around the view"); + assert!(!reads.is_empty()); +} + +/// A dangling rush.json symlink is Rush to discovery's lstat, so it is to +/// the inventory too. +#[cfg(unix)] +#[tokio::test] +async fn rush_gate_counts_a_dangling_symlink() { + let tmp = tempfile::tempdir().unwrap(); + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; + std::os::unix::fs::symlink("missing.json", tmp.path().join("rush.json")).unwrap(); + let names = npm_inventory_names(&ProjectView::Disk(tmp.path())).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); +} + +/// An implied in-memory directory named rush.json counts as discovery +/// counts it. +#[tokio::test] +async fn rush_gate_counts_a_memory_implied_dir() { + let mut project = MemoryProject::new(); + project.insert_present("rush.json/placeholder"); + project.insert_text(RUSH_COMMON_LOCK, PNPM_LOCK); + let names = npm_inventory_names(&ProjectView::Memory(&project)).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); +} + +/// The member-lock reader skips a Rush monorepo on the same gate as +/// discovery (which reads Rush's locks there, never the members'). +#[tokio::test] +async fn rush_member_locks_gate_matches_discovery() { + let mut project = MemoryProject::new(); + project.insert_text("pnpm-workspace.yaml", "packages:\n - packages/*\n"); + project.insert_text(".npmrc", "shared-workspace-lockfile=false\n"); + project.insert_text("packages/a/package.json", "{}"); + project.insert_text("packages/a/pnpm-lock.yaml", OUTSIDE_LOCK); + let view = ProjectView::Memory(&project); + assert!( + !super::pnpm::inventory_pnpm_member_locks_in(&view) + .await + .is_empty(), + "the fixture must list a member lock without rush.json" + ); + project.insert_present("rush.json/placeholder"); + let view = ProjectView::Memory(&project); + let members = super::pnpm::inventory_pnpm_member_locks_in(&view).await; + assert!(members.is_empty(), "{members:?}"); +} + +/// Rush's lock dirs are read only through real directories: a symlinked +/// `common`, `common/config`, `common/config/rush` or +/// `common/config/subspaces` could point the read at another repository. +#[cfg(unix)] +#[tokio::test] +async fn rush_refuses_symlinked_lock_dirs() { + use std::os::unix::fs::symlink; + let outside = tempfile::tempdir().unwrap(); + write_nested(outside.path(), "rush/pnpm-lock.yaml", OUTSIDE_LOCK).await; + write_nested(outside.path(), "subspaces/x/pnpm-lock.yaml", OUTSIDE_LOCK).await; + write_nested( + outside.path(), + "common/config/rush/pnpm-lock.yaml", + OUTSIDE_LOCK, + ) + .await; + let rels = |root: &Path| { + let root = root.to_path_buf(); + async move { super::pnpm::rush_lock_rels(ProjectView::Disk(&root)).await } + }; + + // A symlinked subspaces dir: the real common lock alone. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", RUSH_JSON).await; + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; + symlink( + outside.path().join("subspaces"), + tmp.path().join("common/config/subspaces"), + ) + .unwrap(); + assert_eq!(rels(tmp.path()).await, vec![RUSH_COMMON_LOCK.to_string()]); + let names = npm_inventory_names(&ProjectView::Disk(tmp.path())).await; + assert!(names.contains(&"left-pad".to_string()), "{names:?}"); + assert!(!names.contains(&"only-outside".to_string()), "{names:?}"); + + // A symlinked common/config/rush: no common lock. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", RUSH_JSON).await; + tokio::fs::create_dir_all(tmp.path().join("common/config")) + .await + .unwrap(); + symlink( + outside.path().join("rush"), + tmp.path().join("common/config/rush"), + ) + .unwrap(); + assert!(rels(tmp.path()).await.is_empty()); + assert!(npm_inventory_names(&ProjectView::Disk(tmp.path())) + .await + .is_empty()); + + // A symlinked `common`: nothing under it. + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "rush.json", RUSH_JSON).await; + symlink(outside.path().join("common"), tmp.path().join("common")).unwrap(); + assert!(rels(tmp.path()).await.is_empty()); + assert!(npm_inventory_names(&ProjectView::Disk(tmp.path())) + .await + .is_empty()); +} + // ── yarn classic ────────────────────────────────────────────────────── const YARN_CLASSIC: &str = "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. @@ -2272,7 +2440,7 @@ async fn fifo_lockfiles_fail_fast_instead_of_wedging() { inventory_yarn_berry(&root).await, inventory_bun(&root).await, inventory_vlt(&root).await, - inventory_pnpm_lock_at(&root.join("shrinkwrap.yaml")).await, + inventory_pnpm_lock_rel_in(&ProjectView::Disk(&root), "shrinkwrap.yaml").await, gem_remotes(&root).await, ) }; diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index 3193d7f80..c7562d185 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -64,7 +64,7 @@ use crate::formats::pnpm::{ }; use crate::formats::text::strip_bom; use crate::utils::digest::is_sri_pin; -use crate::vendor::lock_inventory::pnpm::rush_lock_rels; +use crate::vendor::lock_inventory::pnpm::{is_rush, rush_lock_rels}; use crate::vendor::lock_inventory::{ npm_lock_bundled_nodes, npm_lock_legacy_mirror_nodes, npm_lock_located_nodes, LockIntegrity, NpmLockNode, @@ -688,7 +688,7 @@ async fn extract_pnpm(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { // pnpm's `lockfile` setting off (#1074): pnpm reads neither the root // lock nor a member's, so their hosted pins keep no lockfile basis. // Rush runs pnpm with a workspace and `.npmrc` of its own. - let rush = ctx.exists("rush.json").await; + let rush = is_rush(ctx.view).await; let disabled = if rush { None } else { From c0fe5b513cd07ed9a64f09cd28097e623f0fc9c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:05:02 +0000 Subject: [PATCH 3/3] Share the Rush gate and lock list with hosted mode Second review round. Hosted mode had its own Rush gate (`rush.json` is a regular file) and its own subspace listing (no name check, symlinked `common/...` dirs followed), so it pinned, and wrote back, locks that discovery and the inventory refuse, and the readers' gate (`exists_no_follow`) disagreed with it on a dangling `rush.json` link or a directory of that name. Modes are ground truth, so the shared `is_rush` gate is now hosted's: `view.is_file("rush.json")` (on disk a link to a regular file counts; a dangling link, a directory, or an implied in-memory directory does not, since Rush cannot load any of them). Hosted mode, the vendor flavor probe, discovery and both inventory readers use it, and hosted pins exactly the locks `rush_lock_rels` lists. Behavior changes: - discovery / inventory: a dangling `rush.json` link or a `rush.json` directory is no longer a Rush monorepo (no Rush lock read; a plain pnpm workspace's member locks are read again); - hosted: a subspace whose name is not a safe path segment, and every lock under a symlinked `common`, `common/config`, `common/config/rush` or `common/config/subspaces`, is no longer pinned; - vendor probe: a `rush.json` directory no longer refuses with `vendor_rush_unsupported`. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-core/src/hosted/engine.rs | 83 +++++++++++++------ .../src/vendor/lock_inventory/pnpm.rs | 14 ++-- .../src/vendor/lock_inventory/tests.rs | 37 +++++++-- .../src/vendor/npm_flavor.rs | 13 ++- .../socket-patch-core/src/vex/discover/npm.rs | 22 +++++ 5 files changed, 130 insertions(+), 39 deletions(-) diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 09901663b..7968a1da7 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -24,9 +24,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use serde::{Deserialize, Serialize}; use crate::api::types::PackageVendorResult; -use crate::constants::npm_family::{ - RUSH_COMMON_LOCK_REL, RUSH_SUBSPACES_DIR, VLT_HIDDEN_LOCK_REL, VLT_LOCK, -}; +use crate::constants::npm_family::{VLT_HIDDEN_LOCK_REL, VLT_LOCK}; use crate::formats::cargo::source_replacement::CratesIoSource; use crate::formats::governing_locks::PYPI_REQUIREMENTS; use crate::patch::redirect::npmrc::{ @@ -43,6 +41,7 @@ use crate::patch::redirect::{ use crate::utils::pnpm_workspace::governing_workspace_file; use crate::utils::purl::purl_parts; use crate::utils::redact::url_host; +use crate::vendor::lock_inventory::pnpm::{is_rush, rush_lock_rels}; use crate::vendor::lock_inventory::{bun_text_lock_drives, MemoryEntry, ProjectView}; use super::guidance::{ @@ -437,17 +436,6 @@ impl CandidateFiles { } } -/// Whether the project is a Rush monorepo (disk: `rush.json` is a file). -fn rush_repo(view: &ProjectView<'_>) -> bool { - match view { - ProjectView::Disk(_) | ProjectView::Snapshot(_) => { - let cwd = view.disk_root().expect("a disk view has a root"); - cwd.join("rush.json").is_file() - } - ProjectView::Memory(project) => project.contains("rush.json"), - } -} - /// Whether `rel` is a [`PRESENCE_ONLY`](crate::formats::registry::PRESENCE_ONLY) /// row the in-memory host lists without usable content (a symbolic link, /// an oversize or presence-only entry). Its planners only ask whether it @@ -900,17 +888,14 @@ pub async fn read_candidate_files( // common/config/subspaces//. Added under their repo-relative // keys — the pnpm rewriter is basename-generalized, so nested keys are // rewritten in place, and the write-back is path-generic. - if rush_repo(view) { - if out.read(view, unreadable, RUSH_COMMON_LOCK_REL).await { - out.rush_lock_keys.push(RUSH_COMMON_LOCK_REL.to_string()); - } - // Sorted by name: deterministic output. - if let Ok(entries) = view.list_dir(RUSH_SUBSPACES_DIR).await { - for entry in entries.into_iter().filter(|e| e.is_dir) { - let key = format!("{RUSH_SUBSPACES_DIR}/{}/pnpm-lock.yaml", entry.name); - if out.read(view, unreadable, &key).await { - out.rush_lock_keys.push(key); - } + // The gate and the lock list are the ones lockfile discovery and the + // lock inventory use: sorted by name, real directories only (a + // symlinked `common/...` dir could point the write at another + // repository), traversal-safe subspace names. + if is_rush(*view).await { + for key in rush_lock_rels(*view).await { + if out.read(view, unreadable, &key).await { + out.rush_lock_keys.push(key); } } } else if candidates.iter().any(|c| c.dep.ecosystem == "npm") { @@ -3670,6 +3655,7 @@ pub fn warning(code: &str, detail: impl Into) -> RewriteWarning { #[cfg(test)] mod tests { use super::*; + use crate::constants::npm_family::RUSH_COMMON_LOCK_REL; use crate::vendor::lock_inventory::MemoryProject; /// In memory as on disk, a go.work go cannot parse reaches the @@ -4652,6 +4638,53 @@ snapshots: assert!(detail.contains(PNPM_TRUST_RUSH_MIXED_NOTE), "{detail}"); } + /// Hosted pins the Rush locks lockfile discovery reads + /// (`rush_lock_rels`): a subspace dir whose name is not a safe path + /// segment is skipped, as is every lock under a symlinked + /// `common/config/subspaces` (it could point the write at another + /// repository). + #[tokio::test] + async fn rush_locks_are_the_ones_discovery_reads() { + let mut rush = MemoryProject::new(); + rush.insert_text("rush.json", r#"{ "rushVersion": "5.100.0" }"#); + rush.insert_text(RUSH_COMMON_LOCK_REL, LEFT_PAD_V9_LOCK); + rush.insert_text( + "common/config/subspaces/ok/pnpm-lock.yaml", + LEFT_PAD_V9_LOCK, + ); + rush.insert_text( + "common/config/subspaces/a:b/pnpm-lock.yaml", + LEFT_PAD_V9_LOCK, + ); + rush.insert_text( + "common/config/subspaces/a\\b/pnpm-lock.yaml", + LEFT_PAD_V9_LOCK, + ); + let (read, _) = npm_rewrite(&ProjectView::Memory(&rush), &BTreeSet::new()).await; + assert_eq!( + read.rush_lock_keys, + [ + RUSH_COMMON_LOCK_REL, + "common/config/subspaces/ok/pnpm-lock.yaml" + ] + ); + + #[cfg(unix)] + { + let outside = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(outside.path().join("x")).unwrap(); + std::fs::write(outside.path().join("x/pnpm-lock.yaml"), LEFT_PAD_V9_LOCK).unwrap(); + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("rush.json"), "{}").unwrap(); + std::fs::create_dir_all(tmp.path().join("common/config/rush")).unwrap(); + std::fs::write(tmp.path().join(RUSH_COMMON_LOCK_REL), LEFT_PAD_V9_LOCK).unwrap(); + std::os::unix::fs::symlink(outside.path(), tmp.path().join("common/config/subspaces")) + .unwrap(); + let (read, _) = npm_rewrite(&ProjectView::Disk(tmp.path()), &BTreeSet::new()).await; + assert_eq!(read.rush_lock_keys, [RUSH_COMMON_LOCK_REL]); + } + } + /// #713 HEAL-ON-RERUN: a Rush lock an earlier run already redirected /// splices nothing on a re-run, yet its `rush install` still needs the /// Rush remedy — the re-run re-issues it (and still writes nothing). diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs index fadbe0fb2..2479f21ab 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pnpm.rs @@ -11,13 +11,15 @@ use super::LockfileEntry; // ── file selection ── -/// Whether the project is a Rush monorepo: a `rush.json` at the root, -/// probed through the view without following links (a dangling link or, -/// in memory, an implied directory counts). The one gate lockfile -/// discovery and every lock-inventory reader share, so they agree on which -/// locks a Rush project installs from. +/// Whether the project is a Rush monorepo: a regular `rush.json` file at +/// the root, probed through the view (on disk a link to a file counts; a +/// dangling link, a directory or, in memory, an implied directory does +/// not, since Rush cannot load any of them). The one gate hosted mode, +/// the vendor flavor probe, lockfile discovery and every lock-inventory +/// reader share, so they agree on which locks a Rush project installs +/// from. Hosted mode's gate is the ground truth it was taken from. pub(crate) async fn is_rush(view: ProjectView<'_>) -> bool { - view.exists_no_follow("rush.json").await + view.is_file("rush.json") } /// A Rush monorepo's pnpm locks, root-relative, in lockfile discovery's diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index ed2b13040..5670f18f7 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -882,26 +882,41 @@ async fn rush_gate_keeps_the_recording_reusable() { assert!(!reads.is_empty()); } -/// A dangling rush.json symlink is Rush to discovery's lstat, so it is to -/// the inventory too. +/// A dangling rush.json symlink is not Rush (Rush cannot load it): hosted +/// mode's gate, which discovery and the inventory share. #[cfg(unix)] #[tokio::test] -async fn rush_gate_counts_a_dangling_symlink() { +async fn rush_gate_ignores_a_dangling_symlink() { let tmp = tempfile::tempdir().unwrap(); write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; std::os::unix::fs::symlink("missing.json", tmp.path().join("rush.json")).unwrap(); + assert!(!super::pnpm::is_rush(ProjectView::Disk(tmp.path())).await); let names = npm_inventory_names(&ProjectView::Disk(tmp.path())).await; - assert!(names.contains(&"left-pad".to_string()), "{names:?}"); + assert!(!names.contains(&"left-pad".to_string()), "{names:?}"); } -/// An implied in-memory directory named rush.json counts as discovery -/// counts it. +/// An implied in-memory directory named rush.json is not Rush either; a +/// link to a regular rush.json is. #[tokio::test] -async fn rush_gate_counts_a_memory_implied_dir() { +async fn rush_gate_ignores_a_memory_implied_dir() { let mut project = MemoryProject::new(); project.insert_present("rush.json/placeholder"); project.insert_text(RUSH_COMMON_LOCK, PNPM_LOCK); + assert!(!super::pnpm::is_rush(ProjectView::Memory(&project)).await); let names = npm_inventory_names(&ProjectView::Memory(&project)).await; + assert!(!names.contains(&"left-pad".to_string()), "{names:?}"); +} + +/// On disk a symlink to a regular rush.json counts, as hosted's +/// `is_file` gate always counted it. +#[cfg(unix)] +#[tokio::test] +async fn rush_gate_counts_a_link_to_a_file() { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "real-rush.json", RUSH_JSON).await; + write_nested(tmp.path(), RUSH_COMMON_LOCK, PNPM_LOCK).await; + std::os::unix::fs::symlink("real-rush.json", tmp.path().join("rush.json")).unwrap(); + let names = npm_inventory_names(&ProjectView::Disk(tmp.path())).await; assert!(names.contains(&"left-pad".to_string()), "{names:?}"); } @@ -921,8 +936,16 @@ async fn rush_member_locks_gate_matches_discovery() { .is_empty(), "the fixture must list a member lock without rush.json" ); + // An implied directory named rush.json is not Rush: members still count. project.insert_present("rush.json/placeholder"); let view = ProjectView::Memory(&project); + assert!(!super::pnpm::inventory_pnpm_member_locks_in(&view) + .await + .is_empty()); + let mut project = project.clone(); + project.remove("rush.json/placeholder"); + project.insert_text("rush.json", RUSH_JSON); + let view = ProjectView::Memory(&project); let members = super::pnpm::inventory_pnpm_member_locks_in(&view).await; assert!(members.is_empty(), "{members:?}"); } diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 06355d051..496bfaa8c 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -228,7 +228,7 @@ pub(crate) async fn detect_npm_lock_flavor_in( // survive Rush's generated-workspace install (installs run // from common/temp). Point the user at hosted mode, which // edits the lock in place. - if view.exists("rush.json").await { + if crate::vendor::lock_inventory::pnpm::is_rush(*view).await { return Err(( "vendor_rush_unsupported", format!( @@ -1771,6 +1771,17 @@ mod tests { ); } + /// The probe's Rush gate is the shared one: a directory named + /// `rush.json` (or a dangling link) is not a Rush monorepo, so the + /// refusal is the missing-lock one. + #[tokio::test] + async fn a_rush_json_directory_is_not_rush() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::create_dir(tmp.path().join("rush.json")).unwrap(); + let (code, _) = detect_npm_lock_flavor(tmp.path()).await.unwrap_err(); + assert_ne!(code, "vendor_rush_unsupported"); + } + #[tokio::test] async fn rush_check_fires_only_when_nothing_else_matched() { // A repo with rush.json AND a recognized root lock is a normal npm diff --git a/crates/socket-patch-core/src/vex/discover/npm.rs b/crates/socket-patch-core/src/vex/discover/npm.rs index c7562d185..ad5574de7 100644 --- a/crates/socket-patch-core/src/vex/discover/npm.rs +++ b/crates/socket-patch-core/src/vex/discover/npm.rs @@ -3942,6 +3942,28 @@ packages: ); } + /// Discovery's Rush gate is hosted mode's: a dangling `rush.json` link + /// is not a Rush monorepo (Rush cannot load it, and hosted never pins + /// Rush's locks there), so the common lock is not attested. + #[cfg(unix)] + #[tokio::test] + async fn pnpm_rush_gate_ignores_a_dangling_rush_json() { + let common = hosted_rewrite( + "pnpm-lock.yaml", + "lockfileVersion: '9.0'\n\npackages:\n\n ms@1.3.0:\n resolution: {integrity: sha512-UP==}\n", + &[pnpm_override("ms", UUID_A, None)], + ); + let p = Project::new(); + p.write("common/config/rush/pnpm-lock.yaml", &common); + std::os::unix::fs::symlink("missing.json", p.root().join("rush.json")).unwrap(); + let out = run(&p).await; + assert_refs(&out, &[]); + std::fs::remove_file(p.root().join("rush.json")).unwrap(); + p.write("rush.json", "{}"); + let out = run(&p).await; + assert_refs(&out, &[("pkg:npm/ms@1.3.0", UUID_A, WiringMode::Hosted)]); + } + /// Rush without subspaces reads the common lock alone; a subspace dir /// whose name is not a safe path segment is never read. #[tokio::test]