From 0a60e4d34b0d01bd35cacb5494e5039bdee08ba7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 10:25:45 -0400 Subject: [PATCH 1/8] Fold every lexical path normalizer into utils::relpath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twelve hand-rolled `.`/`..` folding loops (fs::normalize_lexically, the pnpm crawler's private copy, cargo_workspace::normalize_rel, gradle::resolve_rel, sbt normalize_dir, pypi normalize_rel_path, cargo_manifest::normalize_socket_path, cargo_config::path_is_socket_owned, composer normalize_config_vendor_dir, the yarnrc modules-folder resolver, vendor/jvm/gradle resolve_dir and maven_reactor normalize) each had its own rules. They now share one segment stack in utils::relpath: resolve_rel (fail closed past a floor), normalize_rel_keeping_escapes (keep the escape so a containment check can name it), normalize_lexically and normalize_lexically_keeping_escapes for Paths. Each format keeps only its own admission check (what spellings it refuses up front). Behavior fix: the pnpm crawler's copy let a second leading `..` pop the first (PathBuf::pop removes a `..` segment), so `../../x` collapsed to `x`; the shared keep-escapes normalizer keeps every leading parent. Audit: architecture audit §3.B (path normalizers). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/crawlers/composer_crawler.rs | 38 +-- .../src/crawlers/npm_crawler.rs | 38 +-- .../src/crawlers/ruby_crawler.rs | 5 +- .../src/formats/gem/manifest.rs | 2 +- .../src/formats/sbt/build.rs | 13 +- crates/socket-patch-core/src/gradle/mod.rs | 15 +- .../src/utils/cargo_workspace.rs | 22 +- crates/socket-patch-core/src/utils/fs.rs | 37 --- crates/socket-patch-core/src/utils/mod.rs | 1 + crates/socket-patch-core/src/utils/relpath.rs | 232 ++++++++++++++++++ .../src/vendor/cargo_config.rs | 15 +- .../src/vendor/cargo_manifest.rs | 9 +- .../src/vendor/jvm/gradle.rs | 12 +- .../src/vendor/jvm/maven_reactor.rs | 12 +- .../src/vendor/pypi_requirements.rs | 51 +--- 15 files changed, 267 insertions(+), 235 deletions(-) create mode 100644 crates/socket-patch-core/src/utils/relpath.rs diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index 606b0ef1c..9a67ea99a 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -5,8 +5,9 @@ use std::path::{Path, PathBuf}; use super::types::{CrawledPackage, CrawlerOptions}; use crate::patch::path_safety; use crate::utils::composer_version::composer_versions_equivalent; -use crate::utils::fs::{is_dir, is_dir_sync, is_file, normalize_lexically, run_blocking}; +use crate::utils::fs::{is_dir, is_dir_sync, is_file, run_blocking}; use crate::utils::process::{CommandRunner, GlobalProbeRunner}; +use crate::utils::relpath::normalize_lexically; #[cfg(test)] mod oracle; @@ -530,17 +531,7 @@ fn normalize_config_vendor_dir(raw: &str) -> Option { if raw.starts_with(['/', '\\']) { return None; } - let mut segments: Vec<&str> = Vec::new(); - for segment in raw.split(['/', '\\']) { - match segment { - "" | "." => {} - ".." => { - segments.pop()?; - } - other => segments.push(other), - } - } - (!segments.is_empty()).then(|| segments.join("/")) + crate::utils::relpath::resolve_rel("", raw, 0).filter(|segments| !segments.is_empty()) } /// Read `config.vendor-dir` from a composer.json on disk. Read with @@ -589,8 +580,7 @@ async fn resolve_project_root(vendor_path: &Path) -> PathBuf { } // `normalize_lexically` (resolve `.`/`..` without touching the filesystem) -// lives in `crate::utils::fs` — shared with the ruby crawler's -// config-sourced `BUNDLE_PATH` containment guard. +// lives in `crate::utils::relpath` with every other lexical normalizer. /// Resolve an installed.json `install-path` against the vendor tree. /// @@ -1597,26 +1587,6 @@ mod tests { )); } - #[test] - fn test_normalize_lexically() { - let n = |p: &str| normalize_lexically(Path::new(p)); - // `.` drops out, `..` pops the previous segment. - assert_eq!( - n("/a/b/composer/../monolog/monolog").unwrap(), - PathBuf::from("/a/b/monolog/monolog") - ); - assert_eq!( - n("/a/b/composer/./installers").unwrap(), - PathBuf::from("/a/b/composer/installers") - ); - assert_eq!(n("/a/b/c/../../../web/x").unwrap(), PathBuf::from("/web/x")); - // Popping above the path's own root fails closed. - assert_eq!(n("/a/../.."), None); - assert_eq!(n("../x"), None); - // Relative paths stay relative. - assert_eq!(n("a/b/../c").unwrap(), PathBuf::from("a/c")); - } - #[tokio::test] async fn test_resolve_install_path_rejects_absolute_escape_from_relative_root() { // The CLI defaults to `--cwd .`, so local discovery hands the diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 8acd18f16..eea4e995a 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -156,8 +156,8 @@ pub fn pnpm_store_outside_project(project: &Path) -> bool { let Some(recorded) = parse_modules_yaml_virtual_store_dir(&text) else { return false; }; - let importer = normalize_lexically(project); - let store = normalize_lexically(&nm.join(recorded)); + let importer = crate::utils::relpath::normalize_lexically_keeping_escapes(project); + let store = crate::utils::relpath::normalize_lexically_keeping_escapes(&nm.join(recorded)); store_below_importer(&importer, &store).is_none() }) } @@ -245,16 +245,8 @@ fn resolve_modules_folder(project_in_rc_dir: &[String], raw: &str) -> Option = Vec::new(); - for segment in raw.split(['/', '\\']) { - match segment { - "" | "." => {} - ".." => { - segments.pop()?; - } - other => segments.push(other), - } - } + let resolved = crate::utils::relpath::resolve_rel("", raw, 0)?; + let segments: Vec<&str> = resolved.split('/').filter(|s| !s.is_empty()).collect(); let inside = segments.get(project_in_rc_dir.len()..)?; let at_project = segments.iter().zip(project_in_rc_dir).all(|(s, p)| s == p); if !at_project || inside.is_empty() { @@ -613,24 +605,6 @@ fn parse_modules_yaml_virtual_store_dir(text: &str) -> Option { (!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 -} - /// `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, @@ -683,8 +657,8 @@ fn store_below_importer(importer: &Path, store: &Path) -> Option { 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)); + let importer = crate::utils::relpath::normalize_lexically_keeping_escapes(nm.parent()?); + let store = crate::utils::relpath::normalize_lexically_keeping_escapes(&nm.join(recorded)); 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 diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 61db2695f..a58c8c3cd 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -4,10 +4,9 @@ use std::path::{Path, PathBuf}; use super::types::{CrawledPackage, CrawlerOptions}; use crate::patch::path_safety; -use crate::utils::fs::{ - entry_is_dir, home_dir, is_dir, is_file, list_dir_entries, normalize_lexically, run_blocking, -}; +use crate::utils::fs::{entry_is_dir, home_dir, is_dir, is_file, list_dir_entries, run_blocking}; use crate::utils::process::{CommandRunner, SystemCommandRunner}; +use crate::utils::relpath::normalize_lexically; use crate::vendor::lock_inventory::{DiskSnapshot, ProjectView}; /// Ruby/RubyGems ecosystem crawler for discovering gems in Bundler vendor diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 7e86635a2..1cfa65736 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -37,7 +37,7 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; use crate::crawlers::ruby_crawler::bundle_config_setting_including_empty; -use crate::utils::fs::normalize_lexically; +use crate::utils::relpath::normalize_lexically; /// Where a configured `BUNDLE_GEMFILE` came from. #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/socket-patch-core/src/formats/sbt/build.rs b/crates/socket-patch-core/src/formats/sbt/build.rs index 7bb93086e..4505a3150 100644 --- a/crates/socket-patch-core/src/formats/sbt/build.rs +++ b/crates/socket-patch-core/src/formats/sbt/build.rs @@ -290,18 +290,11 @@ fn normalize_dir(raw: &str) -> Option { if raw.starts_with('/') || raw.contains('\\') || raw.contains(':') { return None; } - let segments: Vec<&str> = raw - .split('/') - .filter(|s| !s.is_empty() && *s != ".") - .collect(); - if segments.contains(&"..") { + if raw.split('/').any(|s| s == "..") { return None; } - Some(if segments.is_empty() { - ".".to_string() - } else { - segments.join("/") - }) + let dir = crate::utils::relpath::resolve_rel("", raw, 0)?; + Some(if dir.is_empty() { ".".to_string() } else { dir }) } /// The projects the build defines, id → root-relative base directory (`.` diff --git a/crates/socket-patch-core/src/gradle/mod.rs b/crates/socket-patch-core/src/gradle/mod.rs index ec07fb6cb..32add2875 100644 --- a/crates/socket-patch-core/src/gradle/mod.rs +++ b/crates/socket-patch-core/src/gradle/mod.rs @@ -108,21 +108,8 @@ pub(crate) fn resolve_rel(floor: &str, base: &str, p: &str) -> Option { { return None; } - let mut parts: Vec<&str> = base.split('/').filter(|s| !s.is_empty()).collect(); let floor_len = floor.split('/').filter(|s| !s.is_empty()).count(); - for seg in p.split('/') { - match seg { - "" | "." => {} - ".." => { - if parts.len() <= floor_len { - return None; - } - parts.pop(); - } - s => parts.push(s), - } - } - Some(parts.join("/")) + crate::utils::relpath::resolve_rel(base, p, floor_len) } /// 1-based line number of byte offset `at` in `text`. diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index 38749c36a..f711fa32d 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -12,7 +12,7 @@ //! Cargo.lock dependents check refuses a crate one of them depends on. use std::collections::BTreeSet; -use std::path::{Component, Path}; +use std::path::Path; use std::sync::Arc; use toml_edit::{DocumentMut, Item, Table}; @@ -323,26 +323,10 @@ fn path_dependencies(doc: &DocumentMut) -> Vec { /// `base/rel` lexically normalized to a repo-relative slash path; `None` /// when it is absolute or climbs out of the root. pub(crate) fn normalize_rel(base: &str, rel: &str) -> Option { - let rel = rel.replace('\\', "/"); - if rel.starts_with('/') || Path::new(&rel).is_absolute() { + if crate::utils::relpath::is_anchored(rel) { return None; } - let mut parts: Vec = base - .split('/') - .filter(|s| !s.is_empty()) - .map(str::to_string) - .collect(); - for component in Path::new(&rel).components() { - match component { - Component::Normal(seg) => parts.push(seg.to_str()?.to_string()), - Component::CurDir => {} - Component::ParentDir => { - parts.pop()?; - } - Component::RootDir | Component::Prefix(_) => return None, - } - } - Some(parts.join("/")) + crate::utils::relpath::resolve_rel(base, rel, 0) } /// Expand a cargo `members` / `exclude` glob (`*`, `?`, `**`) to the diff --git a/crates/socket-patch-core/src/utils/fs.rs b/crates/socket-patch-core/src/utils/fs.rs index 4f9e67581..50bdf967b 100644 --- a/crates/socket-patch-core/src/utils/fs.rs +++ b/crates/socket-patch-core/src/utils/fs.rs @@ -394,43 +394,6 @@ pub(crate) fn home_dir() -> PathBuf { PathBuf::from(home) } -/// Resolve `.`/`..` without touching the filesystem, so a path can be -/// containment-checked BEFORE it is opened (a canonicalizing check would -/// have to stat the very path being validated, and would fail on -/// not-yet-existing directories). Returns `None` when `..` pops above the -/// path's own root — nothing legitimate does that, so it fails closed. -/// -/// Symlinks are not resolved: a symlink INSIDE the project pointing out -/// of it is a pre-existing trust decision of the project's own tree, the -/// same assumption the rest of the crawler layer makes. -/// -/// Shared by the composer crawler's `install-path` containment guard and -/// the ruby crawler's config-sourced `BUNDLE_PATH` containment guard. -pub(crate) fn normalize_lexically(path: &Path) -> Option { - use std::path::Component; - - let mut out = PathBuf::new(); - let mut depth = 0usize; - for component in path.components() { - match component { - Component::Prefix(_) | Component::RootDir => out.push(component.as_os_str()), - Component::CurDir => {} - Component::ParentDir => { - if depth == 0 { - return None; - } - out.pop(); - depth -= 1; - } - Component::Normal(segment) => { - out.push(segment); - depth += 1; - } - } - } - Some(out) -} - /// Atomically commit `content` to `path` via stage + fsync + rename. /// /// The single shared implementation of the hardened-writer pattern used for diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index efcb02f9c..b7d9c0cfe 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -20,6 +20,7 @@ pub mod process; pub mod purl; pub mod python_lock; pub mod python_script; +pub(crate) mod relpath; pub(crate) mod requirements; pub(crate) mod serde; pub mod socket_cli_config; diff --git a/crates/socket-patch-core/src/utils/relpath.rs b/crates/socket-patch-core/src/utils/relpath.rs new file mode 100644 index 000000000..e83fcb06d --- /dev/null +++ b/crates/socket-patch-core/src/utils/relpath.rs @@ -0,0 +1,232 @@ +//! Lexical path normalization: the one place that resolves `.` and `..` +//! segments without touching the filesystem. +//! +//! Every normalizer in the crate folds segments through the same +//! [`Segments`] stack; they differ only in what a caller does with a `..` +//! that climbs above the floor (fail closed, or keep it so a later +//! containment check can report the escape) and in which spellings a +//! format refuses up front (absolute paths, backslashes, drive letters), +//! which stays with each format's own admission check. +//! +//! Symlinks are never resolved: a symlink inside a project pointing out +//! of it is a pre-existing trust decision of the project's own tree. + +use std::ffi::OsStr; +use std::path::{Component, Path, PathBuf}; + +/// A stack of plain path segments with a floor: a `..` pops one segment +/// while the stack is above the floor and otherwise counts as an escape. +struct Segments { + parts: Vec, + floor: usize, + escapes: usize, +} + +impl Segments { + fn new(parts: Vec, floor: usize) -> Self { + Segments { + parts, + floor, + escapes: 0, + } + } + + fn parent(&mut self) { + if self.parts.len() > self.floor { + self.parts.pop(); + } else { + self.escapes += 1; + } + } +} + +/// `rel` folded onto `base`, both split on either separator; empty and +/// `.` segments drop out. +fn fold_str<'a>(base: &'a str, rel: &'a str, floor: usize) -> Segments<&'a str> { + let plain = |s: &&str| !s.is_empty() && *s != "."; + let mut segments = Segments::new(base.split(['/', '\\']).filter(plain).collect(), floor); + for seg in rel.split(['/', '\\']) { + match seg { + "" | "." => {} + ".." => segments.parent(), + other => segments.parts.push(other), + } + } + segments +} + +/// The relative path `rel` (either separator) resolved against `base`, a +/// `/`-separated relative directory (`""` for the root), as a `/`-joined +/// string (`""` for the root itself). `None` when a `..` would pop below +/// the first `floor` segments of `base`; a `floor` of `0` means "never +/// above the root `base` is relative to". +/// +/// Absolute and drive-anchored spellings are NOT detected here: a caller +/// whose format can carry them refuses them first (each format has its +/// own rules — see [`is_anchored`] for the common one). +pub(crate) fn resolve_rel(base: &str, rel: &str, floor: usize) -> Option { + let segments = fold_str(base, rel, floor); + (segments.escapes == 0).then(|| segments.parts.join("/")) +} + +/// The relative or `/`-rooted path `path` (either separator) normalized +/// lexically, keeping what [`resolve_rel`] would refuse visible instead: +/// an escape above the root keeps one leading `../` per climbed level and +/// a rooted path keeps its leading `/` (`a/../../x` → `../x`, +/// `deps\..\dev.txt` → `dev.txt`), so a later containment check can tell +/// an out-of-root path from an in-root one and report it by name. +pub(crate) fn normalize_rel_keeping_escapes(path: &str) -> String { + let rooted = path.starts_with(['/', '\\']); + let segments = fold_str("", path, 0); + let mut out = String::new(); + if rooted { + out.push('/'); + } + for _ in 0..segments.escapes { + out.push_str("../"); + } + out.push_str(&segments.parts.join("/")); + out +} + +/// Whether `rel` is anchored rather than relative: a leading `/` or `\`, +/// or (on Windows) a drive or UNC prefix, including the drive-relative +/// `C:foo`. +pub(crate) fn is_anchored(rel: &str) -> bool { + rel.starts_with(['/', '\\']) + || matches!( + Path::new(rel).components().next(), + Some(Component::Prefix(_) | Component::RootDir) + ) +} + +/// `path`'s anchor (prefix and root components, as spelled) and its +/// remaining segments folded with a floor of `0`. +fn fold_path(path: &Path) -> (PathBuf, Segments<&OsStr>) { + let mut anchor = PathBuf::new(); + let mut segments = Segments::new(Vec::new(), 0); + for component in path.components() { + match component { + Component::Prefix(_) | Component::RootDir => anchor.push(component.as_os_str()), + Component::CurDir => {} + Component::ParentDir => segments.parent(), + Component::Normal(segment) => segments.parts.push(segment), + } + } + (anchor, segments) +} + +/// Resolve `.`/`..` in `path` without touching the filesystem, so a path +/// can be containment-checked BEFORE it is opened (a canonicalizing check +/// would have to stat the very path being validated, and would fail on +/// not-yet-existing directories). Returns `None` when `..` pops above the +/// path's own root (or, for a relative path, above its first segment): +/// nothing legitimate does that, so it fails closed. +pub(crate) fn normalize_lexically(path: &Path) -> Option { + let (mut out, segments) = fold_path(path); + if segments.escapes > 0 { + return None; + } + out.extend(segments.parts); + Some(out) +} + +/// [`normalize_lexically`] that never fails: a relative path keeps one +/// leading `..` per level it climbs above its start (`../../x` stays +/// `../../x`), and a `..` at a filesystem root stays at the root, as the +/// OS resolves it. For joining a recorded relative location onto a base +/// that may itself be relative (`--cwd ../app`), before a containment +/// check compares the two spellings. +pub(crate) fn normalize_lexically_keeping_escapes(path: &Path) -> PathBuf { + let (mut out, segments) = fold_path(path); + if out.as_os_str().is_empty() { + for _ in 0..segments.escapes { + out.push(".."); + } + } + out.extend(segments.parts); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_rel_folds_against_base_with_a_floor() { + assert_eq!(resolve_rel("", "a/./b/../c", 0).as_deref(), Some("a/c")); + assert_eq!( + resolve_rel("crates/x", "../y", 0).as_deref(), + Some("crates/y") + ); + assert_eq!( + resolve_rel("crates/x", "..\\..\\z", 0).as_deref(), + Some("z") + ); + assert_eq!(resolve_rel("crates/x", "../../..", 0), None); + assert_eq!(resolve_rel("", "..", 0), None); + assert_eq!(resolve_rel("a", "..", 0).as_deref(), Some("")); + // The floor keeps the first `floor` base segments. + assert_eq!(resolve_rel("sub/dir", "../x", 1).as_deref(), Some("sub/x")); + assert_eq!(resolve_rel("sub/dir", "../../x", 1), None); + // Empty segments drop out of both sides. + assert_eq!(resolve_rel("a//b/", ".//c", 0).as_deref(), Some("a/b/c")); + } + + #[test] + fn keeping_escapes_reports_out_of_root_paths() { + let n = normalize_rel_keeping_escapes; + assert_eq!(n("deps/../dev.txt"), "dev.txt"); + assert_eq!(n("a/b/../../c"), "c"); + assert_eq!(n("deps/../../x"), "../x"); + assert_eq!(n("./a//b/./c"), "a/b/c"); + assert_eq!(n("../x"), "../x"); + assert_eq!(n("../../x"), "../../x"); + assert_eq!(n("/abs/../x"), "/x"); + assert_eq!(n("deps\\..\\dev.txt"), "dev.txt"); + } + + #[test] + fn anchored_spellings() { + assert!(is_anchored("/x")); + assert!(is_anchored("\\x")); + assert!(!is_anchored("x/y")); + assert!(!is_anchored("../x")); + assert!(!is_anchored("")); + #[cfg(windows)] + { + assert!(is_anchored("C:\\x")); + assert!(is_anchored("C:x")); + } + } + + #[test] + fn normalize_lexically_fails_closed_on_escape() { + let n = |p: &str| normalize_lexically(Path::new(p)); + assert_eq!( + n("/a/b/composer/../monolog/monolog"), + Some(PathBuf::from("/a/b/monolog/monolog")) + ); + assert_eq!(n("/a/b/c/../../../web/x"), Some(PathBuf::from("/web/x"))); + assert_eq!(n("/a/../.."), None); + assert_eq!(n("../x"), None); + assert_eq!(n("a/b/../c"), Some(PathBuf::from("a/c"))); + assert_eq!(n(""), Some(PathBuf::new())); + } + + /// Regression: the pnpm crawler's former private copy let a second + /// leading `..` pop the first (`PathBuf::pop` removes a `..` segment + /// like any other), so `../../x` collapsed to `x` and a store two + /// levels up compared as a child of the importer's spelling. + #[test] + fn keeping_escapes_keeps_every_leading_parent() { + let n = |p: &str| normalize_lexically_keeping_escapes(Path::new(p)); + assert_eq!(n("../../x"), PathBuf::from("../../x")); + assert_eq!(n("../a/../../x"), PathBuf::from("../../x")); + assert_eq!(n("a/../../x"), PathBuf::from("../x")); + assert_eq!(n("a/./b/.."), PathBuf::from("a")); + assert_eq!(n("/.."), PathBuf::from("/")); + assert_eq!(n("/a/../../b"), PathBuf::from("/b")); + assert_eq!(n(""), PathBuf::new()); + } +} diff --git a/crates/socket-patch-core/src/vendor/cargo_config.rs b/crates/socket-patch-core/src/vendor/cargo_config.rs index 6740038f7..9d5747835 100644 --- a/crates/socket-patch-core/src/vendor/cargo_config.rs +++ b/crates/socket-patch-core/src/vendor/cargo_config.rs @@ -424,20 +424,19 @@ async fn edit_config( /// `/abs/.socket/vendor/cargo/…`, `sub/.socket/vendor/cargo/…`) is /// user-authored and must never be rewritten or removed. pub(crate) fn path_is_socket_owned(path: &str) -> bool { - let norm = path.replace('\\', "/"); - if norm.starts_with('/') { + if path.starts_with(['/', '\\']) { return false; // absolute (also covers //unc-style prefixes) } - if norm.as_bytes().get(1) == Some(&b':') { + if path.as_bytes().get(1) == Some(&b':') { return false; // Windows drive-letter absolute (C:/…) } - let segments: Vec<&str> = norm - .split('/') - .filter(|s| !s.is_empty() && *s != ".") - .collect(); - if segments.contains(&"..") { + if path.split(['/', '\\']).any(|s| s == "..") { return false; } + let Some(norm) = crate::utils::relpath::resolve_rel("", path, 0) else { + return false; + }; + let segments: Vec<&str> = norm.split('/').collect(); let prefix: Vec<&str> = CARGO_VENDOR_DIR.split('/').collect(); segments.len() > prefix.len() && segments[..prefix.len()] == prefix[..] } diff --git a/crates/socket-patch-core/src/vendor/cargo_manifest.rs b/crates/socket-patch-core/src/vendor/cargo_manifest.rs index 8b35f5fa9..89b5db19a 100644 --- a/crates/socket-patch-core/src/vendor/cargo_manifest.rs +++ b/crates/socket-patch-core/src/vendor/cargo_manifest.rs @@ -154,12 +154,9 @@ pub fn normalize_socket_path(path: &str) -> Option { if !path_is_socket_owned(path) { return None; } - let norm = path.replace('\\', "/"); - let segments: Vec<&str> = norm - .split('/') - .filter(|s| !s.is_empty() && *s != ".") - .collect(); - Some(segments.join("/")) + // `path_is_socket_owned` admits no `..`, so this only drops `.` and + // empty segments and unifies the separators. + crate::utils::relpath::resolve_rel("", path, 0) } /// Is `path` a Socket-owned copy of exactly `name@version`: under diff --git a/crates/socket-patch-core/src/vendor/jvm/gradle.rs b/crates/socket-patch-core/src/vendor/jvm/gradle.rs index 31a678cb5..ea3df8415 100644 --- a/crates/socket-patch-core/src/vendor/jvm/gradle.rs +++ b/crates/socket-patch-core/src/vendor/jvm/gradle.rs @@ -1669,17 +1669,7 @@ fn resolve_dir(base: &str, p: &str) -> Option { if p.starts_with('/') || p.contains('\\') || p.contains(':') { return None; } - let mut parts: Vec<&str> = base.split('/').filter(|s| !s.is_empty()).collect(); - for seg in p.split('/') { - match seg { - "" | "." => {} - ".." => { - parts.pop()?; - } - s => parts.push(s), - } - } - Some(parts.join("/")) + crate::utils::relpath::resolve_rel(base, p, 0) } // ── in-block pluginManagement entry ────────────────────────────────────── diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 4d311b2ee..b35abe136 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -1202,17 +1202,7 @@ fn normalize(base_dir: &str, rel: &str) -> Option { if rel.starts_with('/') || rel.starts_with('\\') || rel.contains(':') { return None; } - let mut segments: Vec<&str> = base_dir.split('/').filter(|s| !s.is_empty()).collect(); - for segment in rel.split(['/', '\\']) { - match segment { - "" | "." => {} - ".." => { - segments.pop()?; - } - s => segments.push(s), - } - } - Some(segments.join("/")) + crate::utils::relpath::resolve_rel(base_dir, rel, 0) } fn is_range(version: &str) -> bool { diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 7733f6154..b77a67560 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -986,7 +986,7 @@ pub(crate) fn requirements_includes(rel: &str, content: &str) -> Vec { } else { format!("{include_dir}/{target}") }; - normalize_rel_path(&joined) + crate::utils::relpath::normalize_rel_keeping_escapes(&joined) }) .collect() } @@ -1028,38 +1028,6 @@ fn include_target_with(text: &str, env: impl Fn(&str) -> Option) -> Opti target.filter(|t| !t.is_empty()) } -/// Lexically normalize a relative path (`a/../b` → `b`); escapes above the -/// root keep their `../` prefix and absolute paths keep their leading `/`, -/// so the caller can spot out-of-root includes. -fn normalize_rel_path(path: &str) -> String { - let mut stack: Vec<&str> = Vec::new(); - let mut leading_parents = 0usize; - let normalized = path.replace('\\', "/"); - let absolute = normalized.starts_with('/'); - for comp in normalized.split('/') { - match comp { - "" | "." => {} - ".." => { - if stack.is_empty() { - leading_parents += 1; - } else { - stack.pop(); - } - } - other => stack.push(other), - } - } - let mut out = String::new(); - if absolute { - out.push('/'); - } - for _ in 0..leading_parents { - out.push_str("../"); - } - out.push_str(&stack.join("/")); - out -} - // The logical-line lexer lives in `utils::requirements` (shared with the // lockfile inventory and lockfile discovery). @@ -2376,7 +2344,7 @@ mod tests { /// A `-r` inside a non-root file resolves against the INCLUDING file's /// directory (the module's documented contract): `deps/a.txt` reaches /// `b.txt` (sibling → `deps/b.txt`) and `../c.txt` (back at the root — - /// the interior `..` pop of `normalize_rel_path`). The pin in deps/b.txt + /// the interior `..` pop of `normalize_rel_keeping_escapes`). The pin in deps/b.txt /// is rewritten in place; nothing else is touched and no transitive /// duplicate is appended, proving BOTH nested includes were walked. #[tokio::test] @@ -2567,21 +2535,6 @@ mod tests { /// Lexical normalization: interior `..` pops the stack (which decides /// editable-vs-refuse for nested includes); escapes keep their `../` /// prefix; absolute paths keep their leading `/`. - #[test] - fn normalize_rel_path_unit_matrix() { - assert_eq!(normalize_rel_path("deps/../dev.txt"), "dev.txt"); - assert_eq!(normalize_rel_path("a/b/../../c"), "c"); - assert_eq!( - normalize_rel_path("deps/../../x"), - "../x", - "pop, then the second `..` escapes" - ); - assert_eq!(normalize_rel_path("./a//b/./c"), "a/b/c"); - assert_eq!(normalize_rel_path("../x"), "../x"); - assert_eq!(normalize_rel_path("/abs/../x"), "/x"); - assert_eq!(normalize_rel_path("deps\\..\\dev.txt"), "dev.txt"); - } - /// Lines that do not start with a PEP 508 name are not requirements — /// in particular this module's OWN vendor-line shape must be invisible /// to the pin search, or an already-wired path line would misparse as a From 2771e37ad4af0391c88e1579f37d65913aa5a75b Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 10:26:01 -0400 Subject: [PATCH 2/8] Never resolve home-relative probes against the working directory utils::fs::home_dir fell back to the literal relative path `~` when HOME and USERPROFILE were unset, so every `home_dir().join(...)` probe (cargo registry and config chain, nuget, deno, ruby, python user site-packages, pipx/uv/pdm/conda/pyenv roots) resolved against the process cwd: `./~/.cargo/config.toml` inside the scanned project was read as the user's cargo config. It now returns Option holding only an absolute HOME/USERPROFILE, and every caller probes nothing without one. The coursier/ivy `process_home` wrapper (the one caller that already filtered for absolute), the go crawler's and the ruby crawler's private HOME/USERPROFILE readers and update::channel's copy now route through the same resolver. Audit: B66 (fs.rs home_dir relative fallback), C19 (home resolvers). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/crawlers/cargo_crawler.rs | 18 +-- .../src/crawlers/coursier_cache.rs | 9 +- .../src/crawlers/deno_crawler.rs | 39 +++--- .../src/crawlers/go_crawler.rs | 11 +- .../src/crawlers/ivy_cache.rs | 6 +- .../src/crawlers/nuget_crawler.rs | 27 +++-- .../src/crawlers/python_crawler.rs | 114 +++++++++--------- .../src/crawlers/ruby_crawler.rs | 28 ++--- crates/socket-patch-core/src/telemetry.rs | 11 +- .../socket-patch-core/src/update/channel.rs | 2 +- crates/socket-patch-core/src/utils/fs.rs | 95 +++++++++------ crates/socket-patch-core/src/vendor/cargo.rs | 2 +- .../src/vendor/cargo_config.rs | 26 ++-- 13 files changed, 209 insertions(+), 179 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs index 35cf18d64..af787b772 100644 --- a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs @@ -270,7 +270,9 @@ impl CargoCrawler { /// Each subdirectory corresponds to a registry index /// (e.g. `index.crates.io-6f17d22bba15001f/`). async fn get_registry_src_paths() -> Vec { - let cargo_home = Self::cargo_home(); + let Some(cargo_home) = Self::cargo_home() else { + return Vec::new(); + }; let registry_src = cargo_home.join("registry").join("src"); let mut paths = Vec::new(); @@ -346,14 +348,14 @@ impl CargoCrawler { Some((name.to_string(), version.to_string())) } - /// Get `CARGO_HOME`, defaulting to `$HOME/.cargo`. An empty value means - /// unset (the env_non_empty convention) — `PathBuf::from("")` would - /// otherwise resolve `registry/src` against the CWD and silently crawl - /// nothing. - fn cargo_home() -> PathBuf { + /// Get `CARGO_HOME`, defaulting to `$HOME/.cargo` (`None` with no + /// home). An empty value means unset (the env_non_empty convention) — + /// `PathBuf::from("")` would otherwise resolve `registry/src` against + /// the CWD and silently crawl nothing. + fn cargo_home() -> Option { match std::env::var("CARGO_HOME") { - Ok(v) if !v.trim().is_empty() => PathBuf::from(v), - _ => crate::utils::fs::home_dir().join(".cargo"), + Ok(v) if !v.trim().is_empty() => Some(PathBuf::from(v)), + _ => crate::utils::fs::home_dir().map(|home| home.join(".cargo")), } } } diff --git a/crates/socket-patch-core/src/crawlers/coursier_cache.rs b/crates/socket-patch-core/src/crawlers/coursier_cache.rs index ed37591ab..999aff180 100644 --- a/crates/socket-patch-core/src/crawlers/coursier_cache.rs +++ b/crates/socket-patch-core/src/crawlers/coursier_cache.rs @@ -227,7 +227,7 @@ pub fn is_coursier_cache_dir(p: &Path) -> bool { /// `.sbtopts`) is named in the `SOCKET_DEBUG` log. pub fn process_cache_dirs(cwd: &Path) -> Vec { let env = |name: &str| std::env::var(name).ok().filter(|v| !v.is_empty()); - let home = process_home(); + let home = crate::utils::fs::home_dir(); coursier_cache_dirs(TargetOs::host(), &env, home.as_deref(), cwd) .into_iter() .map(|(dir, source)| { @@ -237,13 +237,6 @@ pub fn process_cache_dirs(cwd: &Path) -> Vec { .collect() } -/// This process's home directory, only when absolute: the shared -/// `home_dir()` fallback (`~`) would resolve every default location -/// against the process's working directory. -pub(crate) fn process_home() -> Option { - Some(crate::utils::fs::home_dir()).filter(|h| h.is_absolute()) -} - /// Debug-log a cache location and the source that named it; a location a /// repository file chose is called out as such. pub(crate) fn log_source(what: &str, dir: &Path, source: &str) { diff --git a/crates/socket-patch-core/src/crawlers/deno_crawler.rs b/crates/socket-patch-core/src/crawlers/deno_crawler.rs index 33d367e2b..2881f000a 100644 --- a/crates/socket-patch-core/src/crawlers/deno_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/deno_crawler.rs @@ -72,7 +72,9 @@ impl DenoCrawler { if !options.global && !is_deno_project(&options.cwd).await { return Ok(Vec::new()); } - let cache = deno_dir().join("npm").join("jsr.io"); + let Some(cache) = deno_dir().map(|dir| dir.join("npm").join("jsr.io")) else { + return Ok(Vec::new()); + }; if is_dir(&cache).await { Ok(vec![cache]) } else { @@ -235,41 +237,41 @@ async fn is_deno_project(cwd: &Path) -> bool { /// * Linux/other Unix: `$XDG_CACHE_HOME/deno`, else `$HOME/.cache/deno`. /// * Windows: `%LOCALAPPDATA%\deno` (falling back to `~\.cache\deno` /// if LOCALAPPDATA isn't set). -fn deno_dir() -> PathBuf { +fn deno_dir() -> Option { if let Ok(d) = std::env::var("DENO_DIR") { if !d.is_empty() { - return PathBuf::from(d); + return Some(PathBuf::from(d)); } } - default_cache_root().join("deno") + Some(default_cache_root()?.join("deno")) } /// Per-platform system cache root that Deno appends `deno` to. #[cfg(target_os = "macos")] -fn default_cache_root() -> PathBuf { - home_dir().join("Library").join("Caches") +fn default_cache_root() -> Option { + Some(home_dir()?.join("Library").join("Caches")) } /// Per-platform system cache root that Deno appends `deno` to. #[cfg(windows)] -fn default_cache_root() -> PathBuf { +fn default_cache_root() -> Option { if let Ok(local) = std::env::var("LOCALAPPDATA") { if !local.is_empty() { - return PathBuf::from(local); + return Some(PathBuf::from(local)); } } - home_dir().join(".cache") + Some(home_dir()?.join(".cache")) } /// Per-platform system cache root that Deno appends `deno` to. #[cfg(all(not(target_os = "macos"), not(windows)))] -fn default_cache_root() -> PathBuf { +fn default_cache_root() -> Option { if let Ok(xdg) = std::env::var("XDG_CACHE_HOME") { if !xdg.is_empty() { - return PathBuf::from(xdg); + return Some(PathBuf::from(xdg)); } } - home_dir().join(".cache") + Some(home_dir()?.join(".cache")) } #[cfg(test)] @@ -628,7 +630,7 @@ mod tests { #[serial_test::serial] fn deno_dir_honors_explicit_env() { let _g = EnvGuard::set("DENO_DIR", "/tmp/custom-deno"); - assert_eq!(deno_dir(), PathBuf::from("/tmp/custom-deno")); + assert_eq!(deno_dir(), Some(PathBuf::from("/tmp/custom-deno"))); } #[test] @@ -637,7 +639,7 @@ mod tests { // Empty DENO_DIR must NOT resolve to PathBuf::from("") — it falls // through to the platform default, which always ends in `deno`. let _g = EnvGuard::set("DENO_DIR", ""); - let dir = deno_dir(); + let dir = deno_dir().expect("a home directory"); assert_ne!(dir, PathBuf::from("")); assert!(dir.ends_with("deno"), "got {dir:?}"); } @@ -647,7 +649,7 @@ mod tests { #[serial_test::serial] fn deno_dir_uses_library_caches_on_macos() { let _g = EnvGuard::unset("DENO_DIR"); - let dir = deno_dir(); + let dir = deno_dir().expect("a home directory"); // Regression: macOS must NOT use ~/.cache/deno. assert!( dir.ends_with("Library/Caches/deno"), @@ -662,7 +664,10 @@ mod tests { fn deno_dir_honors_xdg_cache_home_on_linux() { let _d = EnvGuard::unset("DENO_DIR"); let _x = EnvGuard::set("XDG_CACHE_HOME", "/tmp/xdg-cache"); - assert_eq!(deno_dir(), PathBuf::from("/tmp/xdg-cache").join("deno")); + assert_eq!( + deno_dir(), + Some(PathBuf::from("/tmp/xdg-cache").join("deno")) + ); } #[cfg(all(not(target_os = "macos"), not(windows)))] @@ -671,7 +676,7 @@ mod tests { fn deno_dir_falls_back_to_dot_cache_on_linux() { let _d = EnvGuard::unset("DENO_DIR"); let _x = EnvGuard::unset("XDG_CACHE_HOME"); - let dir = deno_dir(); + let dir = deno_dir().expect("a home directory"); assert!(dir.ends_with(".cache/deno"), "got {dir:?}"); } } diff --git a/crates/socket-patch-core/src/crawlers/go_crawler.rs b/crates/socket-patch-core/src/crawlers/go_crawler.rs index 7a11ac8d4..f2956654d 100644 --- a/crates/socket-patch-core/src/crawlers/go_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/go_crawler.rs @@ -179,11 +179,12 @@ impl GoCrawler { // GOMODCACHE and GOPATH guards above: honoring `""` would yield the // RELATIVE path `go/pkg/mod`, pointing the crawl at a directory // inside the user's project instead of a real module cache. - let home = std::env::var("HOME") - .ok() - .filter(|h| !h.is_empty()) - .or_else(|| std::env::var("USERPROFILE").ok().filter(|h| !h.is_empty()))?; - Some(PathBuf::from(home).join("go").join("pkg").join("mod")) + Some( + crate::utils::fs::home_dir()? + .join("go") + .join("pkg") + .join("mod"), + ) } /// The unit tests' entry point to [`parse_versioned_dir`] (the walk diff --git a/crates/socket-patch-core/src/crawlers/ivy_cache.rs b/crates/socket-patch-core/src/crawlers/ivy_cache.rs index c1fd42e8c..0231cbffa 100644 --- a/crates/socket-patch-core/src/crawlers/ivy_cache.rs +++ b/crates/socket-patch-core/src/crawlers/ivy_cache.rs @@ -18,9 +18,7 @@ use std::collections::{HashMap, HashSet}; use std::io::Read as _; use std::path::{Path, PathBuf}; -use super::coursier_cache::{ - existing_dedup, jvm_option_values, log_source, process_home, TargetOs, -}; +use super::coursier_cache::{existing_dedup, jvm_option_values, log_source, TargetOs}; use super::maven_crawler::{is_safe_maven_coordinate, parse_pom_group_artifact_version}; use super::types::CrawledPackage; use crate::utils::fs::{open_regular_file_sync, read_regular_to_bytes_sync}; @@ -76,7 +74,7 @@ fn ivy_cache_dirs_with_source( /// home). pub fn process_cache_dirs(cwd: &Path) -> Vec { let env = |name: &str| std::env::var(name).ok().filter(|v| !v.is_empty()); - let home = process_home(); + let home = crate::utils::fs::home_dir(); ivy_cache_dirs_with_source(TargetOs::host(), &env, home.as_deref(), cwd) .into_iter() .map(|(dir, source)| { diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index 4e2fcd1b2..34191ab7f 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -42,9 +42,10 @@ impl NuGetCrawler { if let Some(ref custom) = options.global_prefix { return Ok(vec![custom.clone()]); } - let home = nuget_home(); - if is_dir(&home).await { - return Ok(vec![home]); + if let Some(home) = nuget_home() { + if is_dir(&home).await { + return Ok(vec![home]); + } } return Ok(Vec::new()); } @@ -72,9 +73,10 @@ impl NuGetCrawler { } // 2. Fall back to the global cache. - let home = nuget_home(); - if is_dir(&home).await && seen.insert(home.clone()) { - paths.push(home); + if let Some(home) = nuget_home() { + if is_dir(&home).await && seen.insert(home.clone()) { + paths.push(home); + } } // 3. Check obj/ dirs for project.assets.json @@ -388,18 +390,19 @@ fn is_safe_nuget_coordinate(name: &str, version: &str) -> bool { /// Get the NuGet global packages folder. /// -/// Checks `NUGET_PACKAGES` env var, falls back to `~/.nuget/packages/`. -fn nuget_home() -> PathBuf { +/// Checks `NUGET_PACKAGES` env var, falls back to `~/.nuget/packages/` +/// (`None` with no home directory). +fn nuget_home() -> Option { // NuGet itself treats an empty NUGET_PACKAGES as unset and falls back // to the default folder; honoring "" here would make global discovery // probe `is_dir("")` and silently scan nothing. if let Ok(custom) = std::env::var("NUGET_PACKAGES") { if !custom.is_empty() { - return PathBuf::from(custom); + return Some(PathBuf::from(custom)); } } - crate::utils::fs::home_dir().join(".nuget").join("packages") + crate::utils::fs::home_dir().map(|home| home.join(".nuget").join("packages")) } /// Check if the cwd contains any .NET project indicators. @@ -1027,7 +1030,7 @@ mod tests { let custom = "/tmp/test-nuget-packages"; std::env::set_var("NUGET_PACKAGES", custom); let home = nuget_home(); - assert_eq!(home, PathBuf::from(custom)); + assert_eq!(home, Some(PathBuf::from(custom))); std::env::remove_var("NUGET_PACKAGES"); } @@ -1041,7 +1044,7 @@ mod tests { async fn test_nuget_home_empty_env_var_falls_back_to_default() { let prev = std::env::var("NUGET_PACKAGES").ok(); std::env::set_var("NUGET_PACKAGES", ""); - let home = nuget_home(); + let home = nuget_home().expect("a home directory"); match prev { Some(v) => std::env::set_var("NUGET_PACKAGES", v), None => std::env::remove_var("NUGET_PACKAGES"), diff --git a/crates/socket-patch-core/src/crawlers/python_crawler.rs b/crates/socket-patch-core/src/crawlers/python_crawler.rs index 062dcdc9d..1d0b7b956 100644 --- a/crates/socket-patch-core/src/crawlers/python_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/python_crawler.rs @@ -2443,8 +2443,10 @@ pub async fn get_global_python_site_packages() -> Vec { ) .await; // pip --user on Unix - let user_local = home_dir.join(".local"); - scan_well_known(&user_local, "site-packages", &mut seen, &mut results).await; + if let Some(home_dir) = &home_dir { + let user_local = home_dir.join(".local"); + scan_well_known(&user_local, "site-packages", &mut seen, &mut results).await; + } } // macOS-specific @@ -2482,13 +2484,15 @@ pub async fn get_global_python_site_packages() -> Vec { // interpreter first on PATH — so on a stock Mac with both Apple's // python3 and a Homebrew/pyenv python3, user installs under every // other interpreter were invisible. - let user_fw_matches = find_python_dirs( - &home_dir.join("Library").join("Python"), - &["*", "lib", "python", "site-packages"], - ) - .await; - for m in user_fw_matches { - add_path(m, &mut seen, &mut results); + if let Some(home_dir) = &home_dir { + let user_fw_matches = find_python_dirs( + &home_dir.join("Library").join("Python"), + &["*", "lib", "python", "site-packages"], + ) + .await; + for m in user_fw_matches { + add_path(m, &mut seen, &mut results); + } } } @@ -2536,25 +2540,31 @@ pub async fn get_global_python_site_packages() -> Vec { #[cfg(not(windows))] { let pyenv_root = std::env::var("PYENV_ROOT") + .ok() .map(PathBuf::from) - .unwrap_or_else(|_| home_dir.join(".pyenv")); - let pyenv_versions = pyenv_root.join("versions"); - let pyenv_matches = - find_python_dirs(&pyenv_versions, &["*", "lib", "python3.*", "site-packages"]).await; - for m in pyenv_matches { - add_path(m, &mut seen, &mut results); + .or_else(|| home_dir.as_ref().map(|h| h.join(".pyenv"))); + if let Some(pyenv_root) = pyenv_root { + let pyenv_versions = pyenv_root.join("versions"); + let pyenv_matches = + find_python_dirs(&pyenv_versions, &["*", "lib", "python3.*", "site-packages"]) + .await; + for m in pyenv_matches { + add_path(m, &mut seen, &mut results); + } } } // Conda - let anaconda = home_dir.join("anaconda3"); - scan_well_known(&anaconda, "site-packages", &mut seen, &mut results).await; - let miniconda = home_dir.join("miniconda3"); - scan_well_known(&miniconda, "site-packages", &mut seen, &mut results).await; + if let Some(home_dir) = &home_dir { + let anaconda = home_dir.join("anaconda3"); + scan_well_known(&anaconda, "site-packages", &mut seen, &mut results).await; + let miniconda = home_dir.join("miniconda3"); + scan_well_known(&miniconda, "site-packages", &mut seen, &mut results).await; + } // uv tool envs (`uv tool install`): one venv per tool under every // root uv may use (see `uv_dir_candidates`). - for tools in uv_dir_candidates(&home_dir, "UV_TOOL_DIR", "tools") { + for tools in uv_dir_candidates(home_dir.as_deref(), "UV_TOOL_DIR", "tools") { for m in find_child_env_site_packages(&tools).await { add_path(m, &mut seen, &mut results); } @@ -2565,7 +2575,7 @@ pub async fn get_global_python_site_packages() -> Vec { // scanned, not just the one pipx would pick today: an app installed // under an older default is still a real install, and `seen` dedups // overlaps (e.g. PIPX_HOME set to the default). - for pipx_home in pipx_home_candidates(&home_dir) { + for pipx_home in pipx_home_candidates(home_dir.as_deref()) { let venvs = pipx_home.join("venvs"); #[cfg(not(windows))] let mut matches = @@ -2612,7 +2622,7 @@ pub async fn get_global_python_site_packages() -> Vec { // can install packages directly into the managed interpreter (e.g. via // `uv pip install --system --python `), and globally // discovered crawls should surface those. - for python in uv_dir_candidates(&home_dir, "UV_PYTHON_INSTALL_DIR", "python") { + for python in uv_dir_candidates(home_dir.as_deref(), "UV_PYTHON_INSTALL_DIR", "python") { for m in find_child_env_site_packages(&python).await { add_path(m, &mut seen, &mut results); } @@ -2620,7 +2630,7 @@ pub async fn get_global_python_site_packages() -> Vec { // PDM's global project (`pdm add -g`) and PDM-managed interpreters // (`pdm python install`). - for m in pdm_global_site_packages(&home_dir).await { + for m in pdm_global_site_packages(home_dir.as_deref()).await { add_path(m, &mut seen, &mut results); } @@ -2683,7 +2693,7 @@ fn absolute_env_dir(var: &str) -> Option { /// `%LOCALAPPDATA%\uv`, which earlier socket-patch releases scanned, on /// Windows. Callers skip the ones that don't exist. #[cfg_attr(windows, allow(unused_variables))] -fn uv_dir_candidates(home_dir: &Path, override_var: &str, bucket: &str) -> Vec { +fn uv_dir_candidates(home_dir: Option<&Path>, override_var: &str, bucket: &str) -> Vec { let mut dirs = Vec::new(); if let Some(dir) = std::env::var_os(override_var).filter(|v| !v.is_empty()) { let dir = PathBuf::from(dir); @@ -2694,22 +2704,15 @@ fn uv_dir_candidates(home_dir: &Path, override_var: &str, bucket: &str) -> Vec

Option) -> Vec/pdm/config.toml`, and from the site config /// ([`pdm_site_config_dirs`]) PDM layers under it. Every candidate is /// collected, and the ones that don't exist yield nothing. -async fn pdm_global_site_packages(home_dir: &Path) -> Vec { +async fn pdm_global_site_packages(home_dir: Option<&Path>) -> Vec { pdm_global_site_packages_with(home_dir, &|name: &str| std::env::var(name).ok()).await } /// [`pdm_global_site_packages`] with the environment read through `env`. async fn pdm_global_site_packages_with( - home_dir: &Path, + home_dir: Option<&Path>, env: &impl Fn(&str) -> Option, ) -> Vec { - let config_dirs = - pdm_dir_candidates(Some(home_dir), env, "XDG_CONFIG_HOME", Path::new(".config")); + let config_dirs = pdm_dir_candidates(home_dir, env, "XDG_CONFIG_HOME", Path::new(".config")); let data_dirs = pdm_dir_candidates( - Some(home_dir), + home_dir, env, "XDG_DATA_HOME", &Path::new(".local").join("share"), @@ -2932,12 +2934,12 @@ async fn pdm_global_site_packages_with( /// `%USERPROFILE%\pipx` on Windows (with `%LOCALAPPDATA%\pipx\pipx` as its /// platformdirs fallback). All of them are returned; callers skip the ones /// that don't exist. -fn pipx_home_candidates(home_dir: &Path) -> Vec { +fn pipx_home_candidates(home_dir: Option<&Path>) -> Vec { let mut homes = Vec::new(); if let Some(pipx_home) = std::env::var_os("PIPX_HOME").filter(|v| !v.is_empty()) { homes.push(PathBuf::from(pipx_home)); } - homes.push(home_dir.join(".local").join("pipx")); + homes.extend(home_dir.map(|h| h.join(".local").join("pipx"))); #[cfg(all(not(target_os = "macos"), not(windows)))] { // platformdirs ignores a relative XDG_DATA_HOME, per the XDG spec. @@ -2947,18 +2949,13 @@ fn pipx_home_candidates(home_dir: &Path) -> Vec { { homes.push(xdg.join("pipx")); } - homes.push(home_dir.join(".local").join("share").join("pipx")); + homes.extend(home_dir.map(|h| h.join(".local").join("share").join("pipx"))); } #[cfg(target_os = "macos")] - homes.push( - home_dir - .join("Library") - .join("Application Support") - .join("pipx"), - ); + homes.extend(home_dir.map(|h| h.join("Library").join("Application Support").join("pipx"))); #[cfg(windows)] { - homes.push(home_dir.join("pipx")); + homes.extend(home_dir.map(|h| h.join("pipx"))); if let Ok(local) = std::env::var("LOCALAPPDATA") { homes.push(PathBuf::from(local).join("pipx").join("pipx")); } @@ -4119,7 +4116,7 @@ mod tests { ), ) .unwrap(); - let found = pdm_global_site_packages_with(&home, &env_of(&pairs)).await; + let found = pdm_global_site_packages_with(Some(&home), &env_of(&pairs)).await; assert!(found.contains(&project_site), "{found:?}"); assert!(found.contains(&managed_site), "{found:?}"); } @@ -6626,15 +6623,14 @@ G= #[test] #[serial_test::serial] fn test_home_dir_detection() { - // Verify the shared fallback chain (HOME -> USERPROFILE -> "~") - // yields a real path, not the "~" sentinel, on any CI or dev machine. + // Verify the shared chain (HOME -> USERPROFILE, absolute only) + // yields a real path on any CI or dev machine. // `serial`: other tests in this binary mutate HOME (go_crawler's // gomodcache fallback chain, utils::fs's empty-HOME regression) — - // reading home_dir() while one of them holds HOME="" would see the - // "~" sentinel and fail spuriously. - let home = crate::utils::fs::home_dir(); - assert_ne!(home, PathBuf::from("~"), "expected a real home directory"); - assert!(!home.as_os_str().is_empty()); + // reading home_dir() while one of them holds HOME="" would see no + // home and fail spuriously. + let home = crate::utils::fs::home_dir().expect("expected a real home directory"); + assert!(home.is_absolute()); } /// Global discovery must honor `PYENV_ROOT`: a pyenv install tree at diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index a58c8c3cd..524bd21d8 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -731,14 +731,16 @@ impl RubyCrawler { let mut paths = Self::gem_env_gems_dirs().await; let mut seen: HashSet = paths.iter().cloned().collect(); - // Fallback well-known paths - let home = home_dir(); - - let fallback_globs = [ - home.join(".gem").join("ruby"), - home.join(".rbenv").join("versions"), - home.join(".rvm").join("gems"), - ]; + // Fallback well-known paths (none without a home directory) + let fallback_globs: Vec = home_dir() + .map(|home| { + vec![ + home.join(".gem").join("ruby"), + home.join(".rbenv").join("versions"), + home.join(".rvm").join("gems"), + ] + }) + .unwrap_or_default(); for base in &fallback_globs { for entry in list_dir_entries(base).await { @@ -1107,13 +1109,11 @@ pub fn config_path_ignored_warning(value: &str) -> (&'static str, String) { ) } -/// The ambient home directory as an env value (`HOME`, else Windows' -/// `USERPROFILE`), `None` when unset or empty — the `~`-expansion base for -/// ambient runs; tests inject theirs through the `_with_env` seams. +/// The ambient home directory ([`home_dir`]) as an env value — the +/// `~`-expansion base for ambient runs; tests inject theirs through the +/// `_with_env` seams. fn ambient_home() -> Option { - std::env::var_os("HOME") - .filter(|v| !v.is_empty()) - .or_else(|| std::env::var_os("USERPROFILE").filter(|v| !v.is_empty())) + home_dir().map(PathBuf::into_os_string) } /// Pure parser for `gem env ` stdout. Returns the trimmed path diff --git a/crates/socket-patch-core/src/telemetry.rs b/crates/socket-patch-core/src/telemetry.rs index d49aaa5c6..88b69f0a0 100644 --- a/crates/socket-patch-core/src/telemetry.rs +++ b/crates/socket-patch-core/src/telemetry.rs @@ -164,12 +164,13 @@ fn build_telemetry_context(command: &str) -> PatchTelemetryContext { /// Replaces the user's home directory path with `~` to avoid leaking /// sensitive file system information. pub fn sanitize_error_message(message: &str) -> String { - let home = home_dir(); + let Some(home) = home_dir() else { + return message.to_string(); + }; let home = home.to_string_lossy(); - // `home_dir()` falls back to a literal `"~"` when no home is set, and - // replacing `"~"` with `"~"` is a no-op. A set-but-empty HOME must be - // skipped explicitly — replacing `""` would splice `~` between every byte. - // Trailing separators are trimmed so a `HOME=/home/user/` redaction keeps + // `home_dir()` is `None` with no (absolute) home set, so a set-but-empty + // HOME never reaches here — replacing `""` would splice `~` between + // every byte. Trailing separators are trimmed so a `HOME=/home/user/` redaction keeps // the separator (`~/.cache`, not `~.cache`); a home that trims to nothing // (`HOME=/`, common for unmapped-UID containers) is a filesystem root with // no user-identifying prefix to redact — replacing it would splice `~` diff --git a/crates/socket-patch-core/src/update/channel.rs b/crates/socket-patch-core/src/update/channel.rs index 0b5e332aa..ad2988a9e 100644 --- a/crates/socket-patch-core/src/update/channel.rs +++ b/crates/socket-patch-core/src/update/channel.rs @@ -59,7 +59,7 @@ impl ChannelEnv { ChannelEnv { cargo_home: path_var("CARGO_HOME"), xdg_cache_home: path_var("XDG_CACHE_HOME"), - home: path_var("HOME").or_else(|| path_var("USERPROFILE")), + home: crate::utils::fs::home_dir(), local_app_data: path_var("LOCALAPPDATA"), } } diff --git a/crates/socket-patch-core/src/utils/fs.rs b/crates/socket-patch-core/src/utils/fs.rs index 50bdf967b..212cd6f3d 100644 --- a/crates/socket-patch-core/src/utils/fs.rs +++ b/crates/socket-patch-core/src/utils/fs.rs @@ -374,24 +374,22 @@ pub(crate) async fn entry_file_type(entry: &DirEntry) -> Option PathBuf { - let home = std::env::var("HOME") - .ok() - .filter(|h| !h.is_empty()) - .or_else(|| std::env::var("USERPROFILE").ok().filter(|h| !h.is_empty())) - .unwrap_or_else(|| "~".to_string()); - PathBuf::from(home) +/// The user's home directory: `HOME`, then `USERPROFILE` (Windows), each +/// only when set, non-empty and ABSOLUTE; otherwise `None`. A relative or +/// empty value (stripped CI/container/sudo environments, `env -i`) would +/// turn every `home_dir().join(…)` probe into a CWD-relative path, +/// pointing the crawlers at directories inside the user's project as if +/// they were the per-user package roots (`~/.cargo`, `~/.m2`, `~/.nuget`, +/// …), so a caller with no home probes nothing there. +/// +/// The one home resolver for the crawlers' well-known per-user roots, +/// telemetry's home-dir redaction and the repository walk's home stop. +pub(crate) fn home_dir() -> Option { + ["HOME", "USERPROFILE"] + .into_iter() + .filter_map(std::env::var_os) + .map(PathBuf::from) + .find(|home| home.is_absolute()) } /// Atomically commit `content` to `path` via stage + fsync + rename. @@ -1064,20 +1062,18 @@ mod tests { ); } - /// Regression: a set-but-empty `HOME` (stripped CI/container/sudo - /// environments) must be treated as unset, exactly like the documented - /// no-home fallback. Honoring `""` made `home_dir()` return an empty - /// `PathBuf`, so every `home_dir().join(".cargo")`-style probe became a - /// CWD-relative path and the crawlers scanned directories inside the - /// user's project as if they were the per-user package roots. - #[test] - #[serial_test::serial] - fn home_dir_treats_empty_home_as_unset() { - let prev_home = std::env::var("HOME").ok(); - let prev_profile = std::env::var("USERPROFILE").ok(); - std::env::set_var("HOME", ""); - std::env::set_var("USERPROFILE", ""); - let home = home_dir(); + /// Run `f` with `HOME`/`USERPROFILE` set to `home`/`profile` + /// (`None` = unset), restoring both. + fn with_home_env(home: Option<&str>, profile: Option<&str>, f: impl FnOnce() -> T) -> T { + let prev_home = std::env::var_os("HOME"); + let prev_profile = std::env::var_os("USERPROFILE"); + let set = |name: &str, v: Option<&str>| match v { + Some(v) => std::env::set_var(name, v), + None => std::env::remove_var(name), + }; + set("HOME", home); + set("USERPROFILE", profile); + let out = f(); match prev_home { Some(v) => std::env::set_var("HOME", v), None => std::env::remove_var("HOME"), @@ -1086,10 +1082,39 @@ mod tests { Some(v) => std::env::set_var("USERPROFILE", v), None => std::env::remove_var("USERPROFILE"), } + out + } + + /// Regression: a set-but-empty `HOME` (stripped CI/container/sudo + /// environments) must be treated as unset. Honoring `""` made + /// `home_dir()` return an empty `PathBuf`, so every + /// `home_dir().join(".cargo")`-style probe became a CWD-relative path + /// and the crawlers scanned directories inside the user's project as + /// if they were the per-user package roots. + #[test] + #[serial_test::serial] + fn home_dir_treats_empty_home_as_unset() { + assert_eq!(with_home_env(Some(""), Some(""), home_dir), None); + } + + /// B66: with no usable home the resolver used to fall back to the + /// literal RELATIVE path `~`, which every caller joined onto and so + /// probed `./~/.cargo`, `./~/.nuget/packages`, … under the process + /// working directory. A relative value is no home either. + #[test] + #[serial_test::serial] + fn home_dir_never_returns_a_relative_path() { + assert_eq!(with_home_env(None, None, home_dir), None); + assert_eq!(with_home_env(Some("rel/home"), None, home_dir), None); + let abs = std::env::temp_dir(); + let abs = abs.to_str().unwrap(); + assert_eq!( + with_home_env(Some("~"), Some(abs), home_dir), + Some(PathBuf::from(abs)) + ); assert_eq!( - home, - PathBuf::from("~"), - "empty HOME/USERPROFILE must fall back to the harmless `~` sentinel" + with_home_env(Some(abs), Some("elsewhere"), home_dir), + Some(PathBuf::from(abs)) ); } diff --git a/crates/socket-patch-core/src/vendor/cargo.rs b/crates/socket-patch-core/src/vendor/cargo.rs index 86d4311e6..4a933b2c0 100644 --- a/crates/socket-patch-core/src/vendor/cargo.rs +++ b/crates/socket-patch-core/src/vendor/cargo.rs @@ -5584,7 +5584,7 @@ mod tests { tokio::fs::create_dir_all(&d).await.unwrap(); tokio::fs::write(d.join("config.toml"), body).await.unwrap(); } - let chain = cargo_config::read_config_chain_with(&root, &home).await; + let chain = cargo_config::read_config_chain_with(&root, Some(&home)).await; let got: Vec<(String, bool, bool, PathBuf)> = chain .iter() .map(|c| { diff --git a/crates/socket-patch-core/src/vendor/cargo_config.rs b/crates/socket-patch-core/src/vendor/cargo_config.rs index 9d5747835..bfb08097c 100644 --- a/crates/socket-patch-core/src/vendor/cargo_config.rs +++ b/crates/socket-patch-core/src/vendor/cargo_config.rs @@ -153,17 +153,21 @@ pub struct ChainConfig { /// every ancestor directory's, and `$CARGO_HOME`'s (default `~/.cargo`), in /// that order (cargo lets a config item replace the manifest item with the /// same key, whatever its version). Read-only and fail-soft: missing / -/// unreadable / malformed files contribute nothing. +/// unreadable / malformed files contribute nothing, and with no +/// `$CARGO_HOME` and no home directory there is no home config. pub async fn read_config_chain(project_root: &Path) -> Vec { let cargo_home = match std::env::var("CARGO_HOME") { - Ok(v) if !v.trim().is_empty() => PathBuf::from(v), - _ => crate::utils::fs::home_dir().join(".cargo"), + Ok(v) if !v.trim().is_empty() => Some(PathBuf::from(v)), + _ => crate::utils::fs::home_dir().map(|home| home.join(".cargo")), }; - read_config_chain_with(project_root, &cargo_home).await + read_config_chain_with(project_root, cargo_home.as_deref()).await } /// [`read_config_chain`] with an explicit `$CARGO_HOME`. -pub async fn read_config_chain_with(project_root: &Path, cargo_home: &Path) -> Vec { +pub async fn read_config_chain_with( + project_root: &Path, + cargo_home: Option<&Path>, +) -> Vec { let root = fs::canonicalize(project_root) .await .unwrap_or_else(|_| project_root.to_path_buf()); @@ -171,11 +175,13 @@ pub async fn read_config_chain_with(project_root: &Path, cargo_home: &Path) -> V .ancestors() .map(|dir| (dir.join(".cargo"), dir.to_path_buf(), dir == root)) .collect(); - let home_base = cargo_home - .parent() - .map(Path::to_path_buf) - .unwrap_or_else(|| cargo_home.to_path_buf()); - dirs.push((cargo_home.to_path_buf(), home_base, false)); + if let Some(cargo_home) = cargo_home { + let home_base = cargo_home + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| cargo_home.to_path_buf()); + dirs.push((cargo_home.to_path_buf(), home_base, false)); + } let mut seen: Vec = Vec::new(); let mut out = Vec::new(); for (cargo_dir, base, project) in dirs { From e96a11ce3f15310ef4c93c424801cdcbad2c2754 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 10:26:06 -0400 Subject: [PATCH 3/8] Read project files FIFO-safe in get and the cargo/maven probes get's pnpm PnP hosted narrowing read pnpm-lock.yaml with a plain read_to_string, so a FIFO at that path wedged `get` in open(2); the hosted flow it claims to match already reads it through read_regular_to_string_sync. The cargo workspace manifest reader and the maven checksum-sidecar reader checked `lstat` and then opened with a blocking read, a hand-rolled copy of the regular-file guard with a check-then-open race; they keep the lstat (symlinks are still not followed) but now read through the non-blocking regular-file helpers. Audit: B74. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/get.rs | 12 +++++++++--- crates/socket-patch-core/src/patch/sidecars/maven.rs | 9 ++++++++- .../socket-patch-core/src/utils/cargo_workspace.rs | 5 ++++- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index a41f95802..e61a5d4b5 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1522,10 +1522,16 @@ async fn filter_to_installed_purls( // mark the installed version — but the pnpm-lock.yaml the hosted // rewriter will edit is right there. Read its raw text once and gate the // keep-branch below on version membership, so a large advisory fan-out - // doesn't request grants for every version ever patched (raw - // `read_to_string` matches the hosted flow's own candidate-file reads). + // doesn't request grants for every version ever patched. Read FIFO-safe, + // like the hosted flow's own candidate-file reads: a FIFO planted at + // `pnpm-lock.yaml` must not wedge `get` in open(2). let pnpm_pnp_lock_text: Option = (pnp_pnpm && mode == super::scan::ScanMode::Hosted) - .then(|| std::fs::read_to_string(common.cwd.join("pnpm-lock.yaml")).ok()) + .then(|| { + socket_patch_core::utils::fs::read_regular_to_string_sync( + &common.cwd.join("pnpm-lock.yaml"), + ) + .ok() + }) .flatten(); let pnpm_pnp_lock = pnpm_pnp_lock_text.as_deref().map(PnpmLock::parse); diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index 8798bfce6..d9a17c0ba 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -77,11 +77,18 @@ fn first_token(text: &str) -> Option<(usize, usize)> { /// `dir/.` as text when it is a regular, small, UTF-8 file. fn read_sidecar(path: &Path) -> Option { + // A symlinked sidecar is not followed; the read itself is the + // non-blocking regular-file one, so a FIFO swapped in after the + // `lstat` fails fast instead of wedging in open(2). let meta = std::fs::symlink_metadata(path).ok()?; if !meta.is_file() || meta.len() > MAX_SIDECAR_BYTES { return None; } - String::from_utf8(std::fs::read(path).ok()?).ok() + let bytes = crate::utils::fs::read_regular_to_bytes_sync(path).ok()?; + if bytes.len() as u64 > MAX_SIDECAR_BYTES { + return None; + } + String::from_utf8(bytes).ok() } /// The checksum files beside each of `leaves` (patch-file keys, `package/` diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index f711fa32d..46bd4385e 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -193,10 +193,13 @@ impl ManifestFacts { static FACTS_MEMO: ParseMemo = ParseMemo::new(); fn read_manifest(path: &Path) -> Option> { + // A symlinked manifest is not followed; the read itself is the + // non-blocking regular-file one, so a FIFO swapped in after the + // `lstat` fails fast instead of wedging in open(2). if !std::fs::symlink_metadata(path).is_ok_and(|m| m.is_file()) { return None; } - let text = std::fs::read_to_string(path).ok()?; + let text = crate::utils::fs::read_regular_to_string_sync(path).ok()?; FACTS_MEMO .parse(text.as_bytes(), || { text.parse::() From 613a09b46c65a8734948c53664c897095fe83bfd Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 10:46:00 -0400 Subject: [PATCH 4/8] Use one repository-root walk for socket.yml, VEX and the JVM root check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ancestor walks decided "the repository this directory belongs to" with different rules. policy::find_repo_root_with_warnings honored `.git` files, the home-directory stop, GIT_CEILING_DIRECTORIES and owner trust; the VEX product detector only looked for `

/.git/config` and walked to `/`; the JVM not_build_root check stopped at any `.git` but crossed the home directory and the ceilings. utils::repo_root now owns the walk (search_dirs / ancestor_search_dirs / find_git_repo) and the owner-trust rule, and resolves a checkout's git config through a `gitdir:` file and a worktree's `commondir`. All three callers route through it; the copies in policy and vex/product.rs are deleted. Behavior fixes for VEX product detection (B22): - a submodule names its own origin instead of the superproject's; - a linked worktree reads the shared repository's remotes instead of falling back to a manifest (or nothing); - a dotfiles repository at $HOME is no longer every project's product, and GIT_CEILING_DIRECTORIES and an untrusted owner are honored; - the config is read with the FIFO-safe reader. The JVM build-root search keeps walking past a checkout at the project itself (a submodule can be a module of the build above it) but now stops at the home directory and the ceilings like every other lookup. Audit: B22, §3.B (repo-root walks). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/vex.rs | 4 +- crates/socket-patch-core/src/policy/mod.rs | 94 +---- crates/socket-patch-core/src/policy/tests.rs | 10 - crates/socket-patch-core/src/utils/mod.rs | 1 + .../socket-patch-core/src/utils/repo_root.rs | 331 ++++++++++++++++++ .../src/vendor/maven_repo.rs | 17 +- crates/socket-patch-core/src/vex/product.rs | 141 +++++--- 7 files changed, 456 insertions(+), 142 deletions(-) create mode 100644 crates/socket-patch-core/src/utils/repo_root.rs diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 01402de52..5dc282117 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -51,7 +51,9 @@ pub struct VexArgs { /// /// Auto-detection tries, in order: /// 1. the git `origin` remote: pkg:github// for github.com - /// (likewise gitlab.com and bitbucket.org), the raw URL otherwise + /// (likewise gitlab.com and bitbucket.org), the raw URL otherwise. + /// The nearest checkout counts (a submodule or worktree names + /// itself); a repository at the home directory only when run there /// 2. package.json: pkg:npm/@ /// 3. pyproject.toml: pkg:pypi/@ /// 4. Cargo.toml: pkg:cargo/@ diff --git a/crates/socket-patch-core/src/policy/mod.rs b/crates/socket-patch-core/src/policy/mod.rs index 940b288a5..6d7a312eb 100644 --- a/crates/socket-patch-core/src/policy/mod.rs +++ b/crates/socket-patch-core/src/policy/mod.rs @@ -801,87 +801,27 @@ fn canonical_pypi_purl(purl: String) -> String { format!("pkg:pypi/{}{version}", canonicalize_pypi_name(name)) } -fn home_dir() -> Option { - let var = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; - std::env::var_os(var) - .filter(|v| !v.is_empty()) - .map(PathBuf::from) - .map(|p| std::fs::canonicalize(&p).unwrap_or(p)) -} - -fn ceiling_dirs() -> Vec { - std::env::var_os("GIT_CEILING_DIRECTORIES") - .map(|v| { - std::env::split_paths(&v) - .filter(|p| !p.as_os_str().is_empty()) - .map(|p| std::fs::canonicalize(&p).unwrap_or(p)) - .collect() - }) - .unwrap_or_default() -} - -#[cfg(unix)] -fn trusted_owner(meta: &std::fs::Metadata) -> bool { - use std::os::unix::fs::MetadataExt; - let sudo_uid = std::env::var("SUDO_UID").ok().and_then(|v| v.trim().parse::().ok()); - // SAFETY: geteuid has no preconditions and cannot fail. - owner_trusted(meta.uid(), unsafe { libc::geteuid() }, sudo_uid) -} - -/// `.git` is trusted when it belongs to the invoking user, to root, or -/// (under sudo) to the user sudo ran for. Root trusts every owner: a root -/// process is exposed to the whole filesystem anyway, and CI containers -/// commonly run as root over a checkout owned by another uid, where -/// distrust would silently drop the repo's policy (which only narrows). -#[cfg(unix)] -fn owner_trusted(owner: u32, euid: u32, sudo_uid: Option) -> bool { - euid == 0 || owner == euid || owner == 0 || sudo_uid == Some(owner) -} - -#[cfg(not(unix))] -fn trusted_owner(_meta: &std::fs::Metadata) -> bool { - true -} - -/// The repo root for `cwd` (4.5) with the lookup's warnings: the nearest -/// ancestor (inclusive) holding a `.git` directory or file, not walking -/// past `GIT_CEILING_DIRECTORIES` or into the home directory, and (Unix) -/// only when `.git` belongs to a trusted owner ([`owner_trusted`]). -/// Otherwise `cwd`. +/// The repo root for `cwd` (4.5) with the lookup's warnings: the checkout +/// [`crate::utils::repo_root::find_git_repo`] finds (nearest `.git` +/// directory or file, not past `GIT_CEILING_DIRECTORIES` or into the home +/// directory), when its `.git` belongs to a trusted owner. Otherwise +/// `cwd`. pub fn find_repo_root_with_warnings(cwd: &Path) -> (PathBuf, Vec) { let cwd = std::fs::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf()); - let ceilings = ceiling_dirs(); - let home = home_dir(); let mut warnings = Vec::new(); - let mut dir: &Path = &cwd; - loop { - if dir != cwd && home.as_deref() == Some(dir) { - break; - } - // `metadata` follows a `.git` symlink, as git does. - if let Ok(meta) = std::fs::metadata(dir.join(".git")) { - if meta.is_dir() || meta.is_file() { - if trusted_owner(&meta) { - return (dir.to_path_buf(), warnings); - } - warnings.push(PolicyWarning { - code: SOCKET_YML_REPO_UNTRUSTED, - detail: format!( - "{} is owned by another user; using {} as the repository root", - dir.join(".git").display(), - cwd.display() - ), - }); - break; - } - } - let Some(parent) = dir.parent() else { break }; - if ceilings.iter().any(|c| c == parent) { - break; - } - dir = parent; + match crate::utils::repo_root::find_git_repo(&cwd) { + Some(repo) if repo.trusted => return (repo.root, warnings), + Some(repo) => warnings.push(PolicyWarning { + code: SOCKET_YML_REPO_UNTRUSTED, + detail: format!( + "{} is owned by another user; using {} as the repository root", + repo.dot_git().display(), + cwd.display() + ), + }), + None => {} } - (cwd.clone(), warnings) + (cwd, warnings) } /// [`find_repo_root_with_warnings`] without the warnings. diff --git a/crates/socket-patch-core/src/policy/tests.rs b/crates/socket-patch-core/src/policy/tests.rs index 5a6aa87cd..56b59e96f 100644 --- a/crates/socket-patch-core/src/policy/tests.rs +++ b/crates/socket-patch-core/src/policy/tests.rs @@ -655,16 +655,6 @@ mod disk { assert_eq!(find_repo_root(&cwd), base); } - #[cfg(unix)] - #[test] - fn owner_rule() { - assert!(owner_trusted(1000, 1000, None)); - assert!(owner_trusted(0, 1000, None)); - assert!(!owner_trusted(1001, 1000, None)); - assert!(owner_trusted(1001, 1000, Some(1001)), "sudo's invoking user"); - assert!(owner_trusted(1001, 0, None), "root trusts every owner"); - } - #[cfg(unix)] #[test] fn symlinked_git_marks_the_repo_root() { diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index b7d9c0cfe..c19ef8060 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -21,6 +21,7 @@ pub mod purl; pub mod python_lock; pub mod python_script; pub(crate) mod relpath; +pub mod repo_root; pub(crate) mod requirements; pub(crate) mod serde; pub mod socket_cli_config; diff --git a/crates/socket-patch-core/src/utils/repo_root.rs b/crates/socket-patch-core/src/utils/repo_root.rs new file mode 100644 index 000000000..be6bcaccb --- /dev/null +++ b/crates/socket-patch-core/src/utils/repo_root.rs @@ -0,0 +1,331 @@ +//! The one walk from a directory up to its enclosing git checkout. +//! +//! Every feature that needs "the repository this directory belongs to" +//! (`socket.yml` lookup, VEX product detection, the JVM build-root check) +//! goes through [`search_dirs`] / [`find_git_repo`], so they agree on the +//! rules git itself uses: +//! +//! - the nearest ancestor (inclusive) holding `.git` wins — a directory, +//! or a FILE as in linked worktrees and submodules (`gitdir: …`), or a +//! symlink to either; +//! - the walk never enters the home directory unless it starts there, so a +//! dotfiles repository at `~` never claims every project below it; +//! - the walk never moves into a `GIT_CEILING_DIRECTORIES` entry. + +use std::path::{Path, PathBuf}; + +/// The checkout [`find_git_repo`] found. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitRepo { + /// The working-tree root: the directory holding `.git`. + pub root: PathBuf, + /// Whether `.git` belongs to a trusted owner ([`owner_trusted`]). An + /// untrusted checkout is still the boundary of the walk, but nothing + /// in it should be read as the repository's own configuration (git + /// refuses it the same way, `safe.directory`). + pub trusted: bool, +} + +impl GitRepo { + /// The `.git` entry at the root. + pub fn dot_git(&self) -> PathBuf { + self.root.join(".git") + } + + /// The checkout's git `config` file: `.git/config` for a plain + /// checkout; for a `.git` FILE (`gitdir: `, relative to the + /// root), the config of that git dir — or, for a linked worktree whose + /// git dir names a `commondir`, the shared repository's config, which + /// is where `git worktree add` keeps the remotes. `None` when it + /// cannot be resolved to a regular file. + pub fn config_path(&self) -> Option { + let dot_git = self.dot_git(); + let git_dir = if dot_git.is_dir() { + dot_git + } else { + let text = crate::utils::fs::read_regular_to_string_sync(&dot_git).ok()?; + let pointer = text + .lines() + .find_map(|line| line.trim().strip_prefix("gitdir:"))? + .trim(); + if pointer.is_empty() { + return None; + } + self.root.join(pointer) + }; + let common = match crate::utils::fs::read_regular_to_string_sync(&git_dir.join("commondir")) + { + Ok(text) if !text.trim().is_empty() => git_dir.join(text.trim()), + _ => git_dir, + }; + let config = common.join("config"); + std::fs::metadata(&config) + .is_ok_and(|m| m.is_file()) + .then_some(config) + } +} + +/// `start` (canonicalized when it exists) and its ancestors, nearest +/// first, as far as a repository lookup from `start` may look: through the +/// first directory holding `.git` (inclusive), never into the home +/// directory unless `start` is it, and never into a +/// `GIT_CEILING_DIRECTORIES` entry. +pub fn search_dirs(start: &Path) -> Vec { + search_dirs_with(start, home().as_deref(), &ceiling_dirs(), true) +} + +/// The ancestors of `start` (canonicalized when it exists) that +/// [`search_dirs`] would visit if a checkout AT `start` did not end the +/// walk: for a project that may itself be a checkout (a submodule) and +/// still belong to a build above it. Nearest first; `start` itself is not +/// included. +pub fn ancestor_search_dirs(start: &Path) -> Vec { + let mut dirs = search_dirs_with(start, home().as_deref(), &ceiling_dirs(), false); + if !dirs.is_empty() { + dirs.remove(0); + } + dirs +} + +/// The home directory, canonicalized so it compares with canonical walks. +fn home() -> Option { + crate::utils::fs::home_dir().map(|p| std::fs::canonicalize(&p).unwrap_or(p)) +} + +/// [`search_dirs`] with the home directory and the ceilings injected; +/// `start_git_ends` false walks on past a checkout at `start` itself. +fn search_dirs_with( + start: &Path, + home: Option<&Path>, + ceilings: &[PathBuf], + start_git_ends: bool, +) -> Vec { + let start = std::fs::canonicalize(start).unwrap_or_else(|_| start.to_path_buf()); + let mut dirs = Vec::new(); + let mut dir: &Path = &start; + loop { + if dir != start && home == Some(dir) { + break; + } + dirs.push(dir.to_path_buf()); + if (start_git_ends || dir != start) && dot_git_metadata(dir).is_some() { + break; + } + let Some(parent) = dir.parent() else { break }; + if ceilings.iter().any(|c| c == parent) { + break; + } + dir = parent; + } + dirs +} + +/// The git checkout enclosing `start` (see the module rules), or `None`. +pub fn find_git_repo(start: &Path) -> Option { + repo_at_end_of(search_dirs(start)) +} + +/// The checkout at the last directory of a [`search_dirs`] walk, if the +/// walk ended on one. +fn repo_at_end_of(mut dirs: Vec) -> Option { + let root = dirs.pop()?; + let meta = dot_git_metadata(&root)?; + Some(GitRepo { + root, + trusted: trusted_owner(&meta), + }) +} + +/// `/.git`'s metadata when it is a directory or a regular file; +/// `metadata` follows a `.git` symlink, as git does. +fn dot_git_metadata(dir: &Path) -> Option { + std::fs::metadata(dir.join(".git")) + .ok() + .filter(|meta| meta.is_dir() || meta.is_file()) +} + +fn ceiling_dirs() -> Vec { + std::env::var_os("GIT_CEILING_DIRECTORIES") + .map(|v| { + std::env::split_paths(&v) + .filter(|p| !p.as_os_str().is_empty()) + .map(|p| std::fs::canonicalize(&p).unwrap_or(p)) + .collect() + }) + .unwrap_or_default() +} + +#[cfg(unix)] +fn trusted_owner(meta: &std::fs::Metadata) -> bool { + use std::os::unix::fs::MetadataExt; + let sudo_uid = std::env::var("SUDO_UID") + .ok() + .and_then(|v| v.trim().parse::().ok()); + // SAFETY: geteuid has no preconditions and cannot fail. + owner_trusted(meta.uid(), unsafe { libc::geteuid() }, sudo_uid) +} + +/// `.git` is trusted when it belongs to the invoking user, to root, or +/// (under sudo) to the user sudo ran for. Root trusts every owner: a root +/// process is exposed to the whole filesystem anyway, and CI containers +/// commonly run as root over a checkout owned by another uid, where +/// distrust would silently drop the repository's own settings. +#[cfg(unix)] +fn owner_trusted(owner: u32, euid: u32, sudo_uid: Option) -> bool { + euid == 0 || owner == euid || owner == 0 || sudo_uid == Some(owner) +} + +#[cfg(not(unix))] +fn trusted_owner(_meta: &std::fs::Metadata) -> bool { + true +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn find(start: &Path, home: Option<&Path>, ceilings: &[PathBuf]) -> Option { + repo_at_end_of(search_dirs_with(start, home, ceilings, true)).map(|r| r.root) + } + + #[test] + fn nearest_git_dir_or_file_wins() { + let tmp = tempfile::tempdir().unwrap(); + let base = fs::canonicalize(tmp.path()).unwrap(); + let outer = base.join("outer"); + fs::create_dir_all(outer.join(".git")).unwrap(); + let sub = outer.join("sub"); + fs::create_dir_all(sub.join("deep")).unwrap(); + fs::write(sub.join(".git"), "gitdir: ../.git/modules/sub\n").unwrap(); + assert_eq!(find(&sub.join("deep"), None, &[]), Some(sub.clone())); + assert_eq!( + search_dirs_with(&sub.join("deep"), None, &[], true), + vec![sub.join("deep"), sub.clone()] + ); + assert_eq!(find(&outer, None, &[]), Some(outer.clone())); + // No checkout at all: the walk reaches the filesystem root. + let plain = base.join("plain"); + fs::create_dir_all(&plain).unwrap(); + let dirs = search_dirs_with(&plain, Some(&base), &[], true); + assert_eq!(dirs, vec![plain.clone()]); + assert_eq!(find(&plain, Some(&base), &[]), None); + } + + /// B22: a dotfiles repository at `$HOME` must not become the + /// repository of every project below it. + #[test] + fn walk_never_enters_home_unless_it_starts_there() { + let tmp = tempfile::tempdir().unwrap(); + let home = fs::canonicalize(tmp.path()).unwrap(); + fs::create_dir_all(home.join(".git")).unwrap(); + let project = home.join("code/app"); + fs::create_dir_all(&project).unwrap(); + assert_eq!(find(&project, Some(&home), &[]), None); + assert_eq!( + search_dirs_with(&project, Some(&home), &[], true), + vec![project.clone(), home.join("code")] + ); + assert_eq!(find(&home, Some(&home), &[]), Some(home.clone())); + } + + #[test] + fn ancestor_walk_passes_a_checkout_at_the_start() { + let tmp = tempfile::tempdir().unwrap(); + let base = fs::canonicalize(tmp.path()).unwrap(); + fs::create_dir_all(base.join("outer/.git")).unwrap(); + let sub = base.join("outer/sub"); + fs::create_dir_all(sub.join(".git")).unwrap(); + let mut dirs = search_dirs_with(&sub, None, &[], false); + dirs.remove(0); + assert_eq!(dirs, vec![base.join("outer")]); + // Home still bounds it: a project directly under home never + // reads home itself. + let mut dirs = search_dirs_with(&sub, Some(&base.join("outer")), &[], false); + dirs.remove(0); + assert!(dirs.is_empty(), "{dirs:?}"); + } + + #[test] + fn walk_stops_at_ceiling_dirs() { + let tmp = tempfile::tempdir().unwrap(); + let base = fs::canonicalize(tmp.path()).unwrap(); + fs::create_dir_all(base.join(".git")).unwrap(); + let cwd = base.join("ceiling/cwd"); + fs::create_dir_all(&cwd).unwrap(); + assert_eq!(find(&cwd, None, &[base.join("ceiling")]), None); + assert_eq!(find(&cwd, None, &[]), Some(base)); + } + + #[test] + fn config_of_a_plain_checkout() { + let tmp = tempfile::tempdir().unwrap(); + let root = fs::canonicalize(tmp.path()).unwrap(); + fs::create_dir_all(root.join(".git")).unwrap(); + fs::write(root.join(".git/config"), "").unwrap(); + let repo = GitRepo { + root: root.clone(), + trusted: true, + }; + assert_eq!(repo.config_path(), Some(root.join(".git/config"))); + } + + #[test] + fn config_of_a_submodule_follows_its_gitdir() { + let tmp = tempfile::tempdir().unwrap(); + let root = fs::canonicalize(tmp.path()).unwrap(); + let modules = root.join(".git/modules/sub"); + fs::create_dir_all(&modules).unwrap(); + fs::write(modules.join("config"), "").unwrap(); + let sub = root.join("sub"); + fs::create_dir_all(&sub).unwrap(); + fs::write(sub.join(".git"), "gitdir: ../.git/modules/sub\n").unwrap(); + let repo = GitRepo { + root: sub, + trusted: true, + }; + assert_eq!( + repo.config_path(), + Some(root.join("sub/../.git/modules/sub/config")) + ); + } + + #[test] + fn config_of_a_linked_worktree_is_the_common_dirs() { + let tmp = tempfile::tempdir().unwrap(); + let base = fs::canonicalize(tmp.path()).unwrap(); + let main_git = base.join("main/.git"); + let wt_git = main_git.join("worktrees/wt"); + fs::create_dir_all(&wt_git).unwrap(); + fs::write(main_git.join("config"), "").unwrap(); + fs::write(wt_git.join("commondir"), "../..\n").unwrap(); + let wt = base.join("wt"); + fs::create_dir_all(&wt).unwrap(); + fs::write(wt.join(".git"), format!("gitdir: {}\n", wt_git.display())).unwrap(); + let repo = GitRepo { + root: wt, + trusted: true, + }; + let config = repo.config_path().unwrap(); + assert_eq!( + fs::canonicalize(config).unwrap(), + fs::canonicalize(main_git.join("config")).unwrap() + ); + // A dangling pointer resolves to nothing. + fs::write(repo.dot_git(), "gitdir: /nonexistent/x\n").unwrap(); + assert_eq!(repo.config_path(), None); + } + + #[cfg(unix)] + #[test] + fn owner_rule() { + assert!(owner_trusted(1000, 1000, None)); + assert!(owner_trusted(0, 1000, None)); + assert!(!owner_trusted(1001, 1000, None)); + assert!( + owner_trusted(1001, 1000, Some(1001)), + "sudo's invoking user" + ); + assert!(owner_trusted(1001, 0, None), "root trusts every owner"); + } +} diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 85cc676e1..2d5740f3f 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -729,7 +729,11 @@ async fn legacy_mixed_root(project_root: &Path) -> bool { /// Maven reactor, a project of a Gradle build or an sbt subproject of a /// build rooted above it: vendoring /// there would wire a build nobody runs and leave the real one unpatched -/// (#428). Ancestors are searched up to the enclosing git checkout. +/// (#428). Ancestors are searched up to the enclosing git checkout (a +/// checkout at `project_root` itself does not stop the search: a submodule +/// can still be a module of the build above it), with the repository +/// lookup's own bounds: never into the home directory or a +/// `GIT_CEILING_DIRECTORIES` entry ([`crate::utils::repo_root`]). pub(super) fn not_build_root(project_root: &Path) -> Option { let project = super::jvm::apply::ProjectReader::new(project_root); let own_settings = ["settings.gradle", "settings.gradle.kts"] @@ -738,9 +742,13 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { let own_build = ["build.gradle", "build.gradle.kts"] .iter() .any(|f| project_root.join(f).is_file()); - for ancestor in project_root.ancestors().skip(1) { + let canonical_root = + std::fs::canonicalize(project_root).unwrap_or_else(|_| project_root.to_path_buf()); + let ancestors = crate::utils::repo_root::ancestor_search_dirs(&canonical_root); + for ancestor in &ancestors { + let ancestor = ancestor.as_path(); let reader = super::jvm::apply::ProjectReader::new(ancestor); - let rel = project_root + let rel = canonical_root .strip_prefix(ancestor) .ok() .map(|p| p.to_string_lossy().replace('\\', "/")); @@ -780,9 +788,6 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { )); } } - if ancestor.join(".git").exists() { - break; - } } None } diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index 3273b6e6e..bcf1f0132 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -407,46 +407,40 @@ fn scan_toml_section(content: &str, section: &str) -> Option<(String, String)> { Some((name, version)) } -/// Walk up from `start` looking for a `.git/config` (the working tree -/// or any of its ancestors). When found, parse the -/// `[remote "origin"] url = ...` line and convert that URL to a PURL. +/// The `[remote "origin"] url = ...` of the git checkout enclosing +/// `start`, converted to a PURL. +/// +/// The checkout is the one every repository lookup uses +/// ([`crate::utils::repo_root::find_git_repo`]): the NEAREST `.git` +/// directory or file, so a submodule or linked worktree names itself (its +/// config is followed through the `gitdir:` pointer, and a worktree's +/// through `commondir`), never past `GIT_CEILING_DIRECTORIES`, and never a +/// repository at the home directory unless `start` is it — a dotfiles +/// repo at `~` is not every project's product. A checkout owned by an +/// untrusted user is not read (git refuses it too). /// /// Returns `None` when: -/// * `cwd` is not inside a git working tree, -/// * `.git/config` has no `[remote "origin"]` section, or +/// * `start` is not inside such a checkout, or its config is unreadable, +/// * the config has no `[remote "origin"]` section, or /// * the URL is empty / parsing failed catastrophically. (Otherwise /// even unrecognized hosts fall through to the raw-URL case.) /// -/// Worktrees (`.git` as a file pointing at a real git dir elsewhere) -/// are deliberately NOT followed — they're rare and the package- -/// manifest fallback handles them correctly. Submodules likewise: -/// only the outermost `.git/config` wins. +/// The package-manifest fallback then names the product. async fn detect_git_remote(start: &Path) -> Option { - let git_config_path = find_git_config(start).await?; - let content = tokio::fs::read_to_string(&git_config_path).await.ok()?; + let start = start.to_path_buf(); + let git_config_path = crate::utils::fs::run_blocking(move || { + crate::utils::repo_root::find_git_repo(&start) + .filter(|repo| repo.trusted)? + .config_path() + }) + .await?; + let content = crate::utils::fs::read_regular_to_string(&git_config_path) + .await + .ok()?; let url = scan_remote_origin_url(&content)?; Some(remote_url_to_purl(&url)) } -/// Walk ancestors looking for `/.git/config` as a regular file. -/// Returns the path to it, or `None` if we exhaust the chain. -async fn find_git_config(start: &Path) -> Option { - let mut cursor = match tokio::fs::canonicalize(start).await { - Ok(p) => p, - Err(_) => start.to_path_buf(), - }; - loop { - let candidate = cursor.join(".git").join("config"); - if crate::utils::fs::is_file(&candidate).await { - return Some(candidate); - } - match cursor.parent() { - Some(p) => cursor = p.to_path_buf(), - None => return None, - } - } -} - /// Read the `url = ...` line out of the `[remote "origin"]` section of /// a git config file. Returns the trimmed URL, or `None`. fn scan_remote_origin_url(content: &str) -> Option { @@ -1255,31 +1249,82 @@ mod tests { assert!(r.warnings.is_empty()); } - /// `find_git_config` returns None for a path that genuinely has - /// no `.git/config` on any ancestor. Tempdir on `/var/folders` (macOS) - /// or `/tmp` (linux) gives us a tree that escapes the user's home. + /// No checkout above a tempdir: `/var/folders` (macOS) and `/tmp` + /// (Linux) live outside any git repository. #[tokio::test] - async fn find_git_config_returns_none_when_no_repo_ancestor() { - // Walk up from the tempdir — none of its ancestors should - // contain `.git/config`. This depends on the test runner's - // tempdir living outside any git repo; both macOS - // /var/folders and Linux /tmp satisfy that. + async fn detect_git_remote_returns_none_when_no_repo_ancestor() { let dir = tempfile::tempdir().unwrap(); - let r = find_git_config(dir.path()).await; - assert!(r.is_none(), "unexpected .git/config above {dir:?}: {r:?}"); + let r = detect_git_remote(dir.path()).await; + assert!(r.is_none(), "unexpected checkout above {dir:?}: {r:?}"); } - /// `find_git_config` handles a non-existent start path via the - /// `canonicalize → Err` arm and still walks ancestors of the - /// raw input. Returns None when no config is found. + /// A start path that does not exist walks its raw ancestors and finds + /// nothing. #[tokio::test] - async fn find_git_config_handles_non_existent_start_path() { + async fn detect_git_remote_handles_non_existent_start_path() { let dir = tempfile::tempdir().unwrap(); let nonexistent = dir.path().join("does/not/exist"); - // No I/O panic; the fallback `start.to_path_buf()` arm of - // the `canonicalize` match runs. - let r = find_git_config(&nonexistent).await; - assert!(r.is_none()); + assert!(detect_git_remote(&nonexistent).await.is_none()); + } + + /// B22: inside a submodule (`.git` is a `gitdir:` FILE), the product is + /// the submodule's own origin, not the superproject's. + #[tokio::test] + async fn detect_in_submodule_names_the_submodule() { + let root = tempfile::tempdir().unwrap(); + let root = root.path(); + let modules = root.join(".git/modules/sub"); + std::fs::create_dir_all(&modules).unwrap(); + std::fs::write( + root.join(".git/config"), + "[remote \"origin\"]\n\turl = git@github.com:acme/superproject.git\n", + ) + .unwrap(); + std::fs::write( + modules.join("config"), + "[remote \"origin\"]\n\turl = git@github.com:acme/child.git\n", + ) + .unwrap(); + let sub = root.join("sub"); + std::fs::create_dir_all(&sub).unwrap(); + std::fs::write(sub.join(".git"), "gitdir: ../.git/modules/sub\n").unwrap(); + std::fs::write( + sub.join("package.json"), + r#"{"name":"child-app","version":"2.0.0"}"#, + ) + .unwrap(); + + let r = detect_product(&sub).await; + assert_eq!(r.purl.as_deref(), Some("pkg:github/acme/child")); + + // A submodule whose git dir is gone is not silently attributed to + // the superproject: the manifest names it. + std::fs::remove_dir_all(&modules).unwrap(); + let r = detect_product(&sub).await; + assert_eq!(r.purl.as_deref(), Some("pkg:npm/child-app@2.0.0")); + } + + /// B22: a linked worktree (`git worktree add`) reads the shared + /// repository's remotes through `commondir`. + #[tokio::test] + async fn detect_in_linked_worktree_uses_the_common_config() { + let base = tempfile::tempdir().unwrap(); + let base = base.path(); + let main_git = base.join("main/.git"); + let wt_git = main_git.join("worktrees/wt"); + std::fs::create_dir_all(&wt_git).unwrap(); + std::fs::write( + main_git.join("config"), + "[remote \"origin\"]\n\turl = https://github.com/acme/app.git\n", + ) + .unwrap(); + std::fs::write(wt_git.join("commondir"), "../..\n").unwrap(); + let wt = base.join("wt"); + std::fs::create_dir_all(wt.join("src")).unwrap(); + std::fs::write(wt.join(".git"), format!("gitdir: {}\n", wt_git.display())).unwrap(); + + let r = detect_product(&wt.join("src")).await; + assert_eq!(r.purl.as_deref(), Some("pkg:github/acme/app")); } /// `package.json` where `name` is a number, not a string → None. From d22ea0fe532729093005be0d29d241b2cdfa8ca4 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:21:43 -0400 Subject: [PATCH 5/8] Never default the Maven repo or Python config probes to a relative home m2_repo_path_with fell back to the literal `~` (and accepted a relative HOME), so with no usable home the Maven local repository resolved to the CWD-relative ./~/.m2/repository inside the scanned project (B66). It now returns None and JvmEnv::m2_repo is optional; every caller skips the Maven repo when there is none. The Python crawler's injected-env HOME readers (pdm/poetry config, cache and installer dirs, expand_home, pipenv's home) took an empty or relative HOME as-is. They now share utils::fs::home_from_env, the same rooted-and-non-empty rule as home_dir. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/apply.rs | 8 +- .../socket-patch-cli/src/commands/scan/mod.rs | 13 +-- .../src/ecosystem_dispatch.rs | 8 +- .../src/crawlers/jvm_cache.rs | 5 +- .../src/crawlers/maven_crawler.rs | 89 +++++++++++++------ .../src/crawlers/python_crawler.rs | 68 ++++++++++---- crates/socket-patch-core/src/utils/fs.rs | 32 +++++-- .../tests/crawler_gradle_e2e.rs | 6 +- 8 files changed, 166 insertions(+), 63 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index d3a23819f..055d62b1c 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -2687,7 +2687,13 @@ async fn apply_maven_base(m: &MavenBase<'_>) -> MavenApplied { let m2_copies: Vec = copies .consumed .iter() - .filter(|c| c.starts_with(&m.scope.env.m2_repo)) + .filter(|c| { + m.scope + .env + .m2_repo + .as_ref() + .is_some_and(|m2| c.starts_with(m2)) + }) .map(|p| p.display().to_string()) .collect(); if matches!( diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index 874f40aa7..f88c2c942 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -1267,19 +1267,20 @@ async fn gradle_scan( .collect(); candidates.sort(); candidates.dedup(); - let only_m2: Vec = if candidates.is_empty() { - Vec::new() - } else { + let m2_repo = env.m2_repo.as_ref().filter(|_| !candidates.is_empty()); + let only_m2: Vec = if let Some(m2_repo) = m2_repo { let mut found: Vec = socket_patch_core::crawlers::MavenCrawler - .find_by_purls(&env.m2_repo, &candidates) + .find_by_purls(m2_repo, &candidates) .await .unwrap_or_default() .into_keys() .collect(); found.sort(); found + } else { + Vec::new() }; - if !only_m2.is_empty() { + if let Some(m2_repo) = m2_repo.filter(|_| !only_m2.is_empty()) { const SHOWN: usize = 5; let mut list = only_m2[..only_m2.len().min(SHOWN)].join(", "); if only_m2.len() > SHOWN { @@ -1291,7 +1292,7 @@ async fn gradle_scan( "this Gradle build declares no mavenLocal(), so it does not resolve \ from the Maven local repository ({}); {} found only there {} not \ scanned: {list}", - env.m2_repo.display(), + m2_repo.display(), plural(only_m2.len(), "module", "modules"), if only_m2.len() == 1 { "is" } else { "are" }, ), diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index e2620eaa6..938270666 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -747,7 +747,13 @@ impl JvmScope { for path in paths { if self.is_read_only(path) { out.read_only.push(path.clone()); - } else if path.starts_with(&self.env.m2_repo) && !self.m2_consumed() { + } else if self + .env + .m2_repo + .as_ref() + .is_some_and(|m2| path.starts_with(m2)) + && !self.m2_consumed() + { out.m2_ignored.push(path.clone()); } else { out.consumed.push(path.clone()); diff --git a/crates/socket-patch-core/src/crawlers/jvm_cache.rs b/crates/socket-patch-core/src/crawlers/jvm_cache.rs index d7276f290..da7cd117d 100644 --- a/crates/socket-patch-core/src/crawlers/jvm_cache.rs +++ b/crates/socket-patch-core/src/crawlers/jvm_cache.rs @@ -221,8 +221,9 @@ pub fn all_local_roots_with(cwd: &Path, env: &super::maven_crawler::JvmEnv) -> V } let m2 = env .m2_repo - .is_dir() - .then(|| JvmCacheRoot::new(env.m2_repo.clone(), JvmCacheLayout::Maven2)); + .as_ref() + .filter(|repo| repo.is_dir()) + .map(|repo| JvmCacheRoot::new(repo.clone(), JvmCacheLayout::Maven2)); if super::gradle_cache::has_gradle_marker(cwd) { gradle.extend(m2); gradle diff --git a/crates/socket-patch-core/src/crawlers/maven_crawler.rs b/crates/socket-patch-core/src/crawlers/maven_crawler.rs index 8a507a57d..7b6796f67 100644 --- a/crates/socket-patch-core/src/crawlers/maven_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/maven_crawler.rs @@ -566,8 +566,9 @@ fn coursier_repo_root(path: PathBuf) -> JvmCacheRoot { /// environment; [`JvmEnv::resolve`] an explicit one (tests). #[derive(Debug, Clone, PartialEq, Eq)] pub struct JvmEnv { - /// The Maven local repository (see [`MavenCrawler::get_maven_repo_paths`]). - pub m2_repo: PathBuf, + /// The Maven local repository (see [`MavenCrawler::get_maven_repo_paths`]); + /// `None` when nothing names one and there is no usable home. + pub m2_repo: Option, /// The Gradle user home; `None` when none can be resolved. pub gradle: Option, } @@ -628,22 +629,26 @@ pub fn is_ro_root(path: &Path) -> bool { /// The Maven local repository `env` names: `$MAVEN_REPO_LOCAL`, else /// `$M2_HOME/repository`, else `/.m2/repository` with `` = -/// `$HOME`, `$USERPROFILE`, `home_dir`, or `~`. A set-but-empty variable -/// counts as unset (see [`MavenCrawler::m2_repo_path`]). -pub fn m2_repo_path_with(env: &dyn Env, home_dir: Option<&Path>) -> PathBuf { +/// `$HOME`, `$USERPROFILE` or `home_dir`, the first that is set and rooted +/// ([`crate::utils::fs::home_from_env`]). A set-but-empty variable counts +/// as unset (see [`MavenCrawler::m2_repo_path`]). `None` when there is no +/// usable home: a relative or literal `~` would resolve against the working +/// directory and treat a `~/.m2` inside the scanned project as the user's +/// local repository (B66). +pub fn m2_repo_path_with(env: &dyn Env, home_dir: Option<&Path>) -> Option { let set = |k: &str| env.var(k).filter(|v| !v.is_empty()); if let Some(repo_local) = set("MAVEN_REPO_LOCAL") { - return PathBuf::from(repo_local); + return Some(PathBuf::from(repo_local)); } if let Some(m2_home) = set("M2_HOME") { - return PathBuf::from(m2_home).join("repository"); - } - let home = set("HOME") - .or_else(|| set("USERPROFILE")) - .map(PathBuf::from) - .or_else(|| home_dir.map(Path::to_path_buf)) - .unwrap_or_else(|| PathBuf::from("~")); - home.join(".m2").join("repository") + return Some(PathBuf::from(m2_home).join("repository")); + } + let home = crate::utils::fs::home_from_env(|k| set(k).map(Into::into)).or_else(|| { + home_dir + .filter(|h| crate::utils::fs::is_usable_home(h)) + .map(Path::to_path_buf) + })?; + Some(home.join(".m2").join("repository")) } /// A `--global-prefix` as the cache root it names: a Gradle user home @@ -864,11 +869,10 @@ impl MavenCrawler { let (cwd, env) = (options.cwd.clone(), env.clone()); run_walk(move || m2_gate(&cwd, &env)).await != M2Gate::Ignored }; - if m2 && is_dir(&env.m2_repo).await { - roots.push(JvmCacheRoot::new( - env.m2_repo.clone(), - JvmCacheLayout::Maven2, - )); + if let Some(m2_repo) = env.m2_repo.as_ref().filter(|_| m2) { + if is_dir(m2_repo).await { + roots.push(JvmCacheRoot::new(m2_repo.clone(), JvmCacheLayout::Maven2)); + } } // sbt / Mill / scala-cli: Coursier's per-repository roots, the Ivy // caches, then the caches local sbt evidence points into (the @@ -950,8 +954,10 @@ impl MavenCrawler { let jvm = options.global_prefix.is_none() && (options.global || jvm_cache::is_jvm_project(&options.cwd).await); let mut paths = Vec::new(); - if jvm && is_dir(&env.m2_repo).await { - paths.push(env.m2_repo.clone()); + if let Some(m2_repo) = env.m2_repo.as_ref().filter(|_| jvm) { + if is_dir(m2_repo).await { + paths.push(m2_repo.clone()); + } } // One physical cache once, however it is spelled (a root reached // through a symlinked home names the `~/.m2` pushed above): the @@ -1119,7 +1125,7 @@ impl MavenCrawler { /// /// Same rule as `nuget_home()`, `deno_dir()`, `go_crawler`'s /// `get_gomodcache`, and `utils::fs::home_dir`. - fn m2_repo_path() -> PathBuf { + fn m2_repo_path() -> Option { m2_repo_path_with(&gradle_cache::ProcessEnv, None) } @@ -2454,6 +2460,38 @@ mod tests { } } + /// B66: with no usable home the default used to be the literal + /// RELATIVE `~/.m2/repository`, which resolves against the working + /// directory: a `~/.m2` inside the scanned project was crawled (and + /// patched in place) as the user's local repository. A relative `HOME` + /// is no home either; an absolute `USERPROFILE` or injected home still + /// names one. + #[test] + fn m2_repo_path_with_no_usable_home_names_no_repository() { + let env = |pairs: &[(&str, &str)]| -> HashMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + }; + assert_eq!(m2_repo_path_with(&env(&[]), None), None); + assert_eq!(m2_repo_path_with(&env(&[("HOME", "")]), None), None); + assert_eq!(m2_repo_path_with(&env(&[("HOME", "~")]), None), None); + assert_eq!( + m2_repo_path_with(&env(&[("HOME", "rel/home")]), Some(Path::new("also-rel"))), + None + ); + let abs = std::env::temp_dir(); + let abs_s = abs.to_str().unwrap(); + let want = Some(abs.join(".m2").join("repository")); + assert_eq!( + m2_repo_path_with(&env(&[("HOME", "rel"), ("USERPROFILE", abs_s)]), None), + want + ); + assert_eq!(m2_repo_path_with(&env(&[("HOME", "~")]), Some(&abs)), want); + assert_eq!(m2_repo_path_with(&env(&[("HOME", abs_s)]), None), want); + } + #[test] #[serial_test::serial] fn m2_repo_path_treats_empty_maven_repo_local_as_unset() { @@ -2471,7 +2509,7 @@ mod tests { let repo = MavenCrawler::m2_repo_path(); assert_eq!( repo, - m2_home.path().join("repository"), + Some(m2_home.path().join("repository")), "empty MAVEN_REPO_LOCAL must fall through to the M2_HOME arm, got {repo:?}" ); } @@ -2491,11 +2529,12 @@ mod tests { let repo = MavenCrawler::m2_repo_path(); assert_ne!( repo, - PathBuf::from("repository"), + Some(PathBuf::from("repository")), "empty M2_HOME must not yield a CWD-relative repo path" ); assert!( - repo.ends_with(".m2/repository"), + repo.as_deref() + .is_none_or(|r| r.ends_with(".m2/repository")), "empty M2_HOME must fall through to the ~/.m2/repository default, got {repo:?}" ); } diff --git a/crates/socket-patch-core/src/crawlers/python_crawler.rs b/crates/socket-patch-core/src/crawlers/python_crawler.rs index 1d0b7b956..3aa189c3f 100644 --- a/crates/socket-patch-core/src/crawlers/python_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/python_crawler.rs @@ -498,9 +498,7 @@ async fn pdm_project_setting( table: &str, key: &str, ) -> Option { - let home = var("HOME") - .or_else(|| var("USERPROFILE")) - .map(PathBuf::from); + let home = env_home(var); let files = [cwd.join(".pdm.toml"), cwd.join("pdm.toml")] .into_iter() .chain(pdm_user_config_files(home.as_deref(), var)); @@ -1316,9 +1314,7 @@ fn poetry_user_config_path(var: &impl Fn(&str) -> Option) -> Option Option) -> Option Option) -> Option { - let home = var("HOME") - .or_else(|| var("USERPROFILE")) - .map(PathBuf::from); + let home = env_home(var); if cfg!(windows) { Some( var("LOCALAPPDATA") @@ -1685,9 +1679,7 @@ fn poetry_installer_data_dir(var: &impl Fn(&str) -> Option) -> Option Option) -> Option Option) -> PathBuf { if let Some(rest) = raw.strip_prefix("~/").or_else(|| raw.strip_prefix("~\\")) { - if let Some(home) = var("HOME").or_else(|| var("USERPROFILE")) { - return PathBuf::from(home).join(rest); + if let Some(home) = env_home(var) { + return home.join(rest); } } if raw == "~" { - if let Some(home) = var("HOME").or_else(|| var("USERPROFILE")) { - return PathBuf::from(home); + if let Some(home) = env_home(var) { + return home; } } PathBuf::from(raw) } +/// `HOME`, then `USERPROFILE`, from an injected environment, under the +/// shared rule ([`crate::utils::fs::home_from_env`]): an empty or relative +/// value is no home, so the pdm/poetry probes never resolve against the +/// working directory. +fn env_home(var: &impl Fn(&str) -> Option) -> Option { + crate::utils::fs::home_from_env(|k| var(k).map(Into::into)) +} + /// `site-packages` of every virtualenv Poetry created for the project at /// `cwd` under its `virtualenvs.path` (`--py`; one per /// interpreter minor the user ran `poetry env use` with). Empty for @@ -2070,8 +2070,12 @@ fn pipenv_home_dir(var: &impl Fn(&str) -> Option) -> Option { }) .or_else(|| var("HOME").and_then(non_empty)) .map(PathBuf::from) + .filter(|h| crate::utils::fs::is_usable_home(h)) } else { - var("HOME").and_then(non_empty).map(PathBuf::from) + var("HOME") + .and_then(non_empty) + .map(PathBuf::from) + .filter(|h| crate::utils::fs::is_usable_home(h)) } } @@ -3675,7 +3679,11 @@ mod tests { fake_venv(&tmp.path().join("uv-env"), "venv"); let uv_env = env_of(&[( "UV_PROJECT_ENVIRONMENT", - tmp.path().join("uv-env").join("venv").to_string_lossy().into_owned(), + tmp.path() + .join("uv-env") + .join("venv") + .to_string_lossy() + .into_owned(), )]); assert_eq!( find_local_venv_site_packages_with(&project, &uv_env).await, @@ -5881,6 +5889,30 @@ G= ); } + /// B66: an empty or relative injected `HOME` is no home. It used to be + /// joined as-is, so the pdm/poetry config, cache and installer probes + /// read `./.config/pypoetry/…` (and the like) under the working + /// directory. + #[test] + fn injected_home_must_be_rooted() { + fn only_home(home: &'static str) -> impl Fn(&str) -> Option { + move |k: &str| (k == "HOME").then(|| home.to_string()) + } + for bad in ["", "rel", "~"] { + let var = only_home(bad); + assert_eq!(env_home(&var), None, "HOME={bad:?}"); + assert_eq!(poetry_user_config_path(&var), None, "HOME={bad:?}"); + assert_eq!(poetry_default_cache_dir(&var), None, "HOME={bad:?}"); + assert_eq!(poetry_installer_data_dir(&var), None, "HOME={bad:?}"); + assert_eq!(pipenv_home_dir(&var), None, "HOME={bad:?}"); + assert_eq!(expand_home("~/x", &var), PathBuf::from("~/x")); + } + let abs = std::env::temp_dir(); + let var = |k: &str| (k == "HOME").then(|| abs.to_string_lossy().into_owned()); + assert_eq!(env_home(&var), Some(abs.clone())); + assert_eq!(expand_home("~/x", &var), abs.join("x")); + } + #[test] fn poetry_data_dir_recursively_resolves_only_referenced_settings() { let cwd = Path::new("/project"); diff --git a/crates/socket-patch-core/src/utils/fs.rs b/crates/socket-patch-core/src/utils/fs.rs index 212cd6f3d..75d9f265e 100644 --- a/crates/socket-patch-core/src/utils/fs.rs +++ b/crates/socket-patch-core/src/utils/fs.rs @@ -375,21 +375,39 @@ pub(crate) async fn entry_file_type(entry: &DirEntry) -> Option Option { + home_from_env(|k| std::env::var_os(k)) +} + +/// [`home_dir`]'s rule over an injected environment lookup. +pub(crate) fn home_from_env(var: impl Fn(&str) -> Option) -> Option { ["HOME", "USERPROFILE"] .into_iter() - .filter_map(std::env::var_os) + .filter_map(var) .map(PathBuf::from) - .find(|home| home.is_absolute()) + .find(|home| is_usable_home(home)) +} + +/// Whether `home` may anchor per-user probes: not empty and not +/// CWD-relative. +pub(crate) fn is_usable_home(home: &Path) -> bool { + home.has_root() } /// Atomically commit `content` to `path` via stage + fsync + rename. diff --git a/crates/socket-patch-core/tests/crawler_gradle_e2e.rs b/crates/socket-patch-core/tests/crawler_gradle_e2e.rs index 6699dc880..5f6732c69 100644 --- a/crates/socket-patch-core/tests/crawler_gradle_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_gradle_e2e.rs @@ -607,7 +607,7 @@ fn home_precedence() { let m = Machine::new(); let base = m.jvm_env(); assert_eq!(base.gradle.as_ref().unwrap().files21, m.gradle_files21()); - assert_eq!(base.m2_repo, m.m2()); + assert_eq!(base.m2_repo, Some(m.m2())); let mut env = m.env.clone(); env.insert("GRADLE_USER_HOME".into(), "/guh".into()); @@ -635,12 +635,12 @@ fn home_precedence() { env.insert("M2_HOME".into(), "/m2home".into()); assert_eq!( JvmEnv::resolve(&env, Os::Unix, None).m2_repo, - PathBuf::from("/mrl") + Some(PathBuf::from("/mrl")) ); env.remove("MAVEN_REPO_LOCAL"); assert_eq!( JvmEnv::resolve(&env, Os::Unix, None).m2_repo, - PathBuf::from("/m2home/repository") + Some(PathBuf::from("/m2home/repository")) ); } From 06cad49b38f4b2eba9dcd69cc28c393567fc74dc Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:21:44 -0400 Subject: [PATCH 6/8] Say why VEX skipped a checkout; keep git's home precedence for the walk VEX product detection now adds a warning when the nearest checkout is owned by another user, or when the walk stopped below a repository at the home directory, instead of silently falling back to the manifest. The repository walk's home stop reads HOME on Unix and USERPROFILE on Windows again (the old policy rule, which is git's), rather than the crawler home resolver's HOME-then-USERPROFILE order, so an MSYS/Cygwin HOME does not move the socket.yml stop. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../socket-patch-core/src/utils/repo_root.rs | 38 +++++++- crates/socket-patch-core/src/vex/product.rs | 88 ++++++++++++++++--- 2 files changed, 113 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/utils/repo_root.rs b/crates/socket-patch-core/src/utils/repo_root.rs index be6bcaccb..65e0b418b 100644 --- a/crates/socket-patch-core/src/utils/repo_root.rs +++ b/crates/socket-patch-core/src/utils/repo_root.rs @@ -87,9 +87,16 @@ pub fn ancestor_search_dirs(start: &Path) -> Vec { dirs } -/// The home directory, canonicalized so it compares with canonical walks. -fn home() -> Option { - crate::utils::fs::home_dir().map(|p| std::fs::canonicalize(&p).unwrap_or(p)) +/// The home directory as git's own home rule reads it (`HOME` on Unix, +/// `USERPROFILE` on Windows: an MSYS/Cygwin `HOME` does not move the +/// stop), when set and rooted, canonicalized so it compares with canonical +/// walks. +pub(crate) fn home() -> Option { + let var = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; + std::env::var_os(var) + .map(PathBuf::from) + .filter(|p| crate::utils::fs::is_usable_home(p)) + .map(|p| std::fs::canonicalize(&p).unwrap_or(p)) } /// [`search_dirs`] with the home directory and the ceilings injected; @@ -125,6 +132,24 @@ pub fn find_git_repo(start: &Path) -> Option { repo_at_end_of(search_dirs(start)) } +/// The checkout AT the home directory that a lookup from `start` stopped +/// short of (the home rule), if there is one: for callers that should say +/// why a repository the user may expect was not used. +pub fn home_repo_not_entered(start: &Path) -> Option { + home_repo_beyond(&search_dirs(start), home().as_deref()) +} + +/// The home directory when it holds `.git` and is the directory just above +/// a `dirs` walk that ended without a checkout. +fn home_repo_beyond(dirs: &[PathBuf], home: Option<&Path>) -> Option { + let last = dirs.last()?; + if dot_git_metadata(last).is_some() { + return None; + } + let parent = last.parent()?; + (home == Some(parent) && dot_git_metadata(parent).is_some()).then(|| parent.to_path_buf()) +} + /// The checkout at the last directory of a [`search_dirs`] walk, if the /// walk ended on one. fn repo_at_end_of(mut dirs: Vec) -> Option { @@ -227,6 +252,13 @@ mod tests { vec![project.clone(), home.join("code")] ); assert_eq!(find(&home, Some(&home), &[]), Some(home.clone())); + // The skipped home checkout is reported, so callers can say why. + let walk = search_dirs_with(&project, Some(&home), &[], true); + assert_eq!(home_repo_beyond(&walk, Some(&home)), Some(home.clone())); + assert_eq!(home_repo_beyond(&walk, None), None); + fs::create_dir_all(project.join(".git")).unwrap(); + let walk = search_dirs_with(&project, Some(&home), &[], true); + assert_eq!(home_repo_beyond(&walk, Some(&home)), None); } #[test] diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index bcf1f0132..d097d5726 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -61,7 +61,7 @@ pub async fn detect_product(cwd: &Path) -> DetectResult { let mut result = DetectResult::default(); // 1. git remote origin (highest priority — canonical when present). - if let Some(purl) = detect_git_remote(cwd).await { + if let Some(purl) = detect_git_remote(cwd, &mut result.warnings).await { result.purl = Some(purl); return result; } @@ -417,7 +417,9 @@ fn scan_toml_section(content: &str, section: &str) -> Option<(String, String)> { /// through `commondir`), never past `GIT_CEILING_DIRECTORIES`, and never a /// repository at the home directory unless `start` is it — a dotfiles /// repo at `~` is not every project's product. A checkout owned by an -/// untrusted user is not read (git refuses it too). +/// untrusted user is not read (git refuses it too). Both of those skips +/// push a note onto `warnings`, so the fallback to the manifest is never +/// silent. /// /// Returns `None` when: /// * `start` is not inside such a checkout, or its config is unreadable, @@ -426,14 +428,36 @@ fn scan_toml_section(content: &str, section: &str) -> Option<(String, String)> { /// even unrecognized hosts fall through to the raw-URL case.) /// /// The package-manifest fallback then names the product. -async fn detect_git_remote(start: &Path) -> Option { +async fn detect_git_remote(start: &Path, warnings: &mut Vec) -> Option { let start = start.to_path_buf(); - let git_config_path = crate::utils::fs::run_blocking(move || { - crate::utils::repo_root::find_git_repo(&start) - .filter(|repo| repo.trusted)? - .config_path() + let (git_config_path, warning) = crate::utils::fs::run_blocking(move || { + use crate::utils::repo_root; + match repo_root::find_git_repo(&start) { + Some(repo) if repo.trusted => (repo.config_path(), None), + Some(repo) => ( + None, + Some(format!( + "git checkout {} is owned by another user; not reading its remote \ + origin for the top-level product (git refuses it too, see \ + safe.directory)", + repo.root.display() + )), + ), + None => ( + None, + repo_root::home_repo_not_entered(&start).map(|home| { + format!( + "not using the git repository at the home directory {} for the \ + top-level product; pass --product to name it", + home.display() + ) + }), + ), + } }) - .await?; + .await; + warnings.extend(warning); + let git_config_path = git_config_path?; let content = crate::utils::fs::read_regular_to_string(&git_config_path) .await .ok()?; @@ -1254,7 +1278,7 @@ mod tests { #[tokio::test] async fn detect_git_remote_returns_none_when_no_repo_ancestor() { let dir = tempfile::tempdir().unwrap(); - let r = detect_git_remote(dir.path()).await; + let r = detect_git_remote(dir.path(), &mut Vec::new()).await; assert!(r.is_none(), "unexpected checkout above {dir:?}: {r:?}"); } @@ -1264,7 +1288,7 @@ mod tests { async fn detect_git_remote_handles_non_existent_start_path() { let dir = tempfile::tempdir().unwrap(); let nonexistent = dir.path().join("does/not/exist"); - assert!(detect_git_remote(&nonexistent).await.is_none()); + assert!(detect_git_remote(&nonexistent, &mut Vec::new()).await.is_none()); } /// B22: inside a submodule (`.git` is a `gitdir:` FILE), the product is @@ -1342,6 +1366,50 @@ mod tests { assert!(r.purl.is_none()); } + /// B22: a dotfiles repository at `$HOME` is not the product of a project + /// below it, and the fallback to the manifest says why. + #[tokio::test] + #[serial_test::serial] + async fn detect_below_a_home_repository_warns_and_uses_the_manifest() { + let tmp = tempfile::tempdir().unwrap(); + let home = std::fs::canonicalize(tmp.path()).unwrap(); + std::fs::create_dir_all(home.join(".git")).unwrap(); + std::fs::write( + home.join(".git/config"), + "[remote \"origin\"]\n\turl = git@github.com:me/dotfiles.git\n", + ) + .unwrap(); + let project = home.join("app"); + std::fs::create_dir_all(&project).unwrap(); + std::fs::write( + project.join("package.json"), + r#"{"name":"app","version":"1.0.0"}"#, + ) + .unwrap(); + + let var = if cfg!(windows) { "USERPROFILE" } else { "HOME" }; + let prev = std::env::var_os(var); + std::env::set_var(var, &home); + let below = detect_product(&project).await; + let at_home = detect_product(&home).await; + match prev { + Some(v) => std::env::set_var(var, v), + None => std::env::remove_var(var), + } + + assert_eq!(below.purl.as_deref(), Some("pkg:npm/app@1.0.0")); + assert_eq!(below.warnings.len(), 1, "{:?}", below.warnings); + assert!( + below.warnings[0].contains("home directory") + && below.warnings[0].contains(&home.display().to_string()), + "{:?}", + below.warnings + ); + // Run from the home directory itself, the repository is the product. + assert_eq!(at_home.purl.as_deref(), Some("pkg:github/me/dotfiles")); + assert!(at_home.warnings.is_empty()); + } + /// `package.json` where `version` is a number → None. #[tokio::test] async fn package_json_with_non_string_version_returns_none() { From bd9ea3eaf2a1c12e796bb725aa6d3ea48cb8b82c Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:21:44 -0400 Subject: [PATCH 7/8] Name the JVM build root the way the caller spelled the project not_build_root walks canonical parents, so its refusal printed /private/var/... on macOS and \\?\C:\... on Windows. It now shows the caller's own lexical ancestor when that names the same directory, else the canonical path without the verbatim prefix. A test covers a relative project root, which the old lexical ancestors() walk could not climb. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/vendor/maven_repo.rs | 69 +++++++++++++++++-- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 2d5740f3f..16ea21ee1 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -733,7 +733,11 @@ async fn legacy_mixed_root(project_root: &Path) -> bool { /// checkout at `project_root` itself does not stop the search: a submodule /// can still be a module of the build above it), with the repository /// lookup's own bounds: never into the home directory or a -/// `GIT_CEILING_DIRECTORIES` entry ([`crate::utils::repo_root`]). +/// `GIT_CEILING_DIRECTORIES` entry ([`crate::utils::repo_root`]). Like git, +/// the walk climbs the PHYSICAL parents (`project_root` canonicalized), so +/// a relative root such as `.` still has ancestors; the refusal names the +/// ancestor in the caller's own spelling of `project_root` whenever that +/// spelling reaches it ([`shown_ancestor`]). pub(super) fn not_build_root(project_root: &Path) -> Option { let project = super::jvm::apply::ProjectReader::new(project_root); let own_settings = ["settings.gradle", "settings.gradle.kts"] @@ -753,19 +757,20 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { .ok() .map(|p| p.to_string_lossy().replace('\\', "/")); let Some(rel) = rel else { break }; + let shown = || shown_ancestor(project_root, ancestor, &canonical_root); if super::jvm::maven_reactor::contains_module( &|p| reader.read(p), &format!("{rel}/pom.xml"), ) { return Some(format!( "reason: not_build_root: run vendor from reactor root {}", - ancestor.display() + shown().display() )); } if super::jvm::sbt::nested_in_build(&|p: &str| project.read(p), &|p: &str| reader.read(p)) { return Some(format!( "reason: not_build_root: run vendor from sbt build root {}", - ancestor.display() + shown().display() )); } let read_text = |p: &str| crate::gradle::dsl::decode(&reader.read(p)?); @@ -784,7 +789,7 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { if owner.is_some() || undecodable || (own_build && !own_settings) { return Some(format!( "reason: not_build_root: run vendor from Gradle root {}", - ancestor.display() + shown().display() )); } } @@ -792,6 +797,26 @@ pub(super) fn not_build_root(project_root: &Path) -> Option { None } +/// `ancestor` (a physical ancestor of `canonical_root`, the canonical form +/// of `project_root`) as the caller spelled it: `project_root`'s own +/// lexical ancestor the same number of levels up when that names the same +/// directory, else the canonical path without Windows' verbatim `\\?\` +/// prefix (a `.` root, or a spelling through a symlink). +fn shown_ancestor(project_root: &Path, ancestor: &Path, canonical_root: &Path) -> PathBuf { + let levels = canonical_root + .strip_prefix(ancestor) + .map_or(0, |rel| rel.components().count()); + project_root + .ancestors() + .nth(levels) + .filter(|logical| !logical.as_os_str().is_empty()) + .filter(|logical| std::fs::canonicalize(logical).is_ok_and(|c| c == ancestor)) + .map(Path::to_path_buf) + .unwrap_or_else(|| { + crate::utils::pnpm_workspace::without_verbatim_prefix(ancestor.to_path_buf()) + }) +} + /// Where an upstream file of a vendored GAV is looked for locally: the /// crawler's version directory first (read directly), then every copy /// [`locate_artifact`] finds over the cache that directory belongs to and @@ -5322,6 +5347,42 @@ mod tests { ); } + /// The build-root walk climbs physical parents, so a RELATIVE project + /// root still finds the build above it (the old lexical + /// `project_root.ancestors()` saw no real ancestor of `.`), and the + /// refusal names that root the way the caller spelled the project: + /// not the canonical `/private/var/…` (macOS) or `\\?\C:\…` (Windows). + #[cfg(unix)] + #[test] + fn not_build_root_walks_a_relative_root_and_names_the_callers_spelling() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join(".git")).unwrap(); + std::fs::write(root.join("settings.gradle"), "include 'tools:gen'\n").unwrap(); + std::fs::create_dir_all(root.join("tools/gen")).unwrap(); + std::fs::write(root.join("tools/gen/build.gradle"), "").unwrap(); + let want = |shown: &Path| { + Some(format!( + "reason: not_build_root: run vendor from Gradle root {}", + shown.display() + )) + }; + assert_eq!(not_build_root(&root.join("tools/gen")), want(root)); + + // The same project reached by a path relative to the working + // directory: up to `/`, then down. + let cwd = std::env::current_dir().unwrap(); + let mut rel_root = PathBuf::new(); + for c in cwd.components() { + if matches!(c, std::path::Component::Normal(_)) { + rel_root.push(".."); + } + } + rel_root.push(root.strip_prefix("/").unwrap()); + assert!(rel_root.is_relative()); + assert_eq!(not_build_root(&rel_root.join("tools/gen")), want(&rel_root)); + } + fn fixture_record() -> PatchRecord { PatchRecord { uuid: UUID.to_string(), From 3723b445e09497601026de1b223cc55c25c766bf Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 7 Oct 2026 12:21:44 -0400 Subject: [PATCH 8/8] Add FIFO guard tests for get, cargo_workspace and maven sidecars Also drop the doc comment orphaned by the normalizer test removal in pypi_requirements. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-cli/src/commands/get.rs | 75 +++++++++++++++++++ .../src/patch/sidecars/maven.rs | 11 +++ .../src/utils/cargo_workspace.rs | 13 ++++ .../src/vendor/pypi_requirements.rs | 3 - 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index e61a5d4b5..e9c25390f 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -6302,6 +6302,81 @@ mod tests { assert_eq!(out.skip_records[0]["errorCode"], "package_not_installed"); } + /// B74: a FIFO planted at `pnpm-lock.yaml` of a pnpm-PnP project must + /// not wedge hosted `get` in open(2). A FIFO present from the start is + /// already refused by the lock inventory (so this guards the whole + /// hosted filter path, and passes on the old code too); the raw read + /// behind the pnpm-PnP keep-gate is now FIFO-safe as well, which closes + /// the window where a FIFO is swapped in after the inventory read. A + /// watchdog thread + /// opens the FIFO's write end (non-blocking) after a grace period, which + /// releases a reader stuck in open(2): the test then FAILS instead of + /// hanging the suite. + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn filter_to_installed_purls_pnpm_pnp_hosted_fifo_lock_does_not_wedge() { + use std::os::unix::fs::OpenOptionsExt; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Arc; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join(".pnp.cjs"), b"// pnp loader\n").unwrap(); + let lock = tmp.path().join("pnpm-lock.yaml"); + let c = std::ffi::CString::new(lock.to_str().unwrap()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o600) }, 0); + std::fs::create_dir_all(tmp.path().join("node_modules")).unwrap(); + std::fs::write(tmp.path().join("node_modules/.modules.yaml"), b"").unwrap(); + + let done = Arc::new(AtomicBool::new(false)); + let rescued = Arc::new(AtomicBool::new(false)); + let watchdog = { + let (done, rescued, lock) = (done.clone(), rescued.clone(), lock.clone()); + std::thread::spawn(move || { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !done.load(Ordering::SeqCst) && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(50)); + } + // Keep releasing until the body returns: each open lets one + // blocked reader through to EOF. + while !done.load(Ordering::SeqCst) { + if std::fs::OpenOptions::new() + .write(true) + .custom_flags(libc::O_NONBLOCK) + .open(&lock) + .is_ok() + { + rescued.store(true, Ordering::SeqCst); + } + std::thread::sleep(std::time::Duration::from_millis(50)); + } + }) + }; + + let common = crate::args::GlobalArgs { + cwd: tmp.path().to_path_buf(), + ..Default::default() + }; + let accessible = vec![mk_patch( + "88888888-8888-4888-8888-888888888888", + "pkg:npm/covgap-judged@1.0.0", + "free", + "2024-01-01", + )]; + let out = filter_to_installed_purls( + &accessible, + &common, + crate::commands::scan::ScanMode::Hosted, + ) + .await; + done.store(true, Ordering::SeqCst); + watchdog.join().unwrap(); + assert!( + !rescued.load(Ordering::SeqCst), + "a FIFO pnpm-lock.yaml wedged get in open(2)" + ); + assert!(out.kept.is_empty(), "{:?}", out.kept); + } + /// pnpm-PnP + hosted: a purl the lock probe CANNOT judge (no `@version` /// coordinate to look for) must keep the layout-refusal code — the same /// no-judgment fallback as an unreadable lock — never a false diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index d9a17c0ba..e53f71dcb 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -375,6 +375,17 @@ fn md5(input: &[u8]) -> [u8; 16] { mod tests { use super::*; + /// B74: a FIFO sidecar reads as absent and returns at once. + #[cfg(unix)] + #[test] + fn read_sidecar_rejects_a_fifo_without_blocking() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("lib-1.0.jar.sha1"); + let c = std::ffi::CString::new(path.to_str().unwrap()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o600) }, 0); + assert_eq!(read_sidecar(&path), None); + } + #[test] fn md5_matches_rfc1321_vectors() { assert_eq!(hex::encode(md5(b"")), "d41d8cd98f00b204e9800998ecf8427e"); diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index 46bd4385e..c52e6662a 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -400,6 +400,19 @@ fn wildcard_match(pattern: &[u8], name: &[u8]) -> bool { mod tests { use super::*; + /// B74: a FIFO at `Cargo.toml` reads as "no manifest" and returns at + /// once (the lstat rejects it; the read behind it is the non-blocking + /// regular-file one, so a FIFO swapped in after the lstat fails fast too). + #[cfg(unix)] + #[test] + fn read_manifest_rejects_a_fifo_without_blocking() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("Cargo.toml"); + let c = std::ffi::CString::new(path.to_str().unwrap()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o600) }, 0); + assert!(read_manifest(&path).is_none()); + } + fn write(root: &Path, rel: &str, content: &str) { let path = root.join(rel); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index b77a67560..a524ac965 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -2532,9 +2532,6 @@ mod tests { // ── pure-function matrices ─────────────────────────────────────────── - /// Lexical normalization: interior `..` pops the stack (which decides - /// editable-vs-refuse for nested includes); escapes keep their `../` - /// prefix; absolute paths keep their leading `/`. /// Lines that do not start with a PEP 508 name are not requirements — /// in particular this module's OWN vendor-line shape must be invisible /// to the pin search, or an already-wired path line would misparse as a