From df52676d946421ff7c33de61b20f5d140bfd9c95 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:23:47 +0000 Subject: [PATCH 1/4] Start fix for #439, #686 Assisted-by: Claude Code:claude-opus-5-5 From 0e87f4210e311012fb1a7c6b7af401763fe9b526 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:38:38 +0000 Subject: [PATCH 2/4] Follow Composer's vendor-dir config cascade scan -g and agent scans missed Composer installs whose vendor-dir was set anywhere but the project's composer.json: the global project's composer.json, the user-level $COMPOSER_HOME/config.json (what `composer config -g vendor-dir` writes), or COMPOSER_VENDOR_DIR for `composer global`. They reported "No packages found" and exit 0 while the installed copies stayed unpatched. One resolver now mirrors Composer's Config::get('vendor-dir'): env, project composer.json, user config.json, then vendor, with Platform::expandPath's ~/, $VAR and %VAR% expansion. The global branch, local discovery and the reinstall-hint label all use it. Values from the user's own environment and Composer home are honoured as given; the scanned project's composer.json keeps the existing relative-subpath gate, and a ~/ or $VAR value there is now refused instead of resolving to a literal /~/... directory. Fixes #439. Refs #686. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/composer_hints.rs | 14 +- .../src/crawlers/composer_crawler.rs | 291 ++++++++++++++---- .../tests/crawler_composer_e2e.rs | 233 ++++++++++++++ 3 files changed, 474 insertions(+), 64 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/composer_hints.rs b/crates/socket-patch-cli/src/commands/composer_hints.rs index 32012cda7..247d74519 100644 --- a/crates/socket-patch-cli/src/commands/composer_hints.rs +++ b/crates/socket-patch-cli/src/commands/composer_hints.rs @@ -9,7 +9,8 @@ //! Composer compares. Composer 1 never reinstalls a changed package. In both //! cases `//` must be removed first, where //! `` is the directory Composer installs into -//! (`COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`): a hint +//! (`COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else the user-level +//! `$COMPOSER_HOME/config.json` `vendor-dir`, else `vendor`): a hint //! naming `vendor/` in a project that relocated it sends the user to delete //! a path that does not exist, and `composer install` then leaves the //! package unpatched (#658). @@ -21,9 +22,9 @@ use socket_patch_core::patch::redirect::FileEdit; /// The project's Composer vendor directory as the hints print it: relative /// to `cwd` with `/` separators when it lies inside the project (`vendor`, -/// `lib`, `deps/php`), else the absolute path `COMPOSER_VENDOR_DIR` named. -/// A `config.vendor-dir` the crawler refuses (absolute, or climbing out of -/// the project) prints as the `` placeholder rather than a +/// `lib`, `deps/php`), else the absolute path `COMPOSER_VENDOR_DIR` or the +/// user-level config named. A `config.vendor-dir` the crawler refuses +/// (absolute, home-relative, or climbing out of the project) prints as the `` placeholder rather than a /// directory that would be wrong. pub(crate) fn vendor_dir_label(cwd: &Path) -> String { let Some(dir) = @@ -304,5 +305,10 @@ mod tests { label(Some(r#"{"config":{"vendor-dir":"../outside"}}"#)), "" ); + // Composer would expand `~/`: the literal `/~/deps` is wrong. + assert_eq!( + label(Some(r#"{"config":{"vendor-dir":"~/deps"}}"#)), + "" + ); } } diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index 9629233ff..751a1111f 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::ffi::OsString; use std::path::{Path, PathBuf}; use super::types::{CrawledPackage, CrawlerOptions}; @@ -38,12 +39,14 @@ impl ComposerCrawler { /// Get vendor paths based on options. /// - /// In global mode, checks `$COMPOSER_HOME/vendor/` (env var, command - /// fallback, or platform defaults). + /// In global mode, checks the global project's vendor directory under + /// `$COMPOSER_HOME` (env var, command fallback, or platform defaults), + /// resolved through the same `vendor-dir` cascade as a local project. /// /// In local mode, checks the project's vendor directory /// (`COMPOSER_VENDOR_DIR` / composer.json `config.vendor-dir` / - /// `vendor`, see [`resolve_local_vendor_dir`]) but only if the + /// `$COMPOSER_HOME/config.json` / `vendor`, see + /// [`resolve_vendor_dir`]) but only if the /// directory contains `composer/installed.json` and the cwd looks like /// a PHP project (`composer.json` or `composer.lock` present). pub async fn get_vendor_paths( @@ -56,9 +59,11 @@ impl ComposerCrawler { } let mut paths = Vec::new(); if let Some(composer_home) = get_composer_home().await { - let vendor_dir = composer_home.join("vendor"); - if is_dir(&vendor_dir).await { - paths.push(vendor_dir); + let vendor_dir = run_blocking(move || global_vendor_dir_sync(&composer_home)).await; + if let Some(vendor_dir) = vendor_dir { + if is_dir(&vendor_dir).await { + paths.push(vendor_dir); + } } } return Ok(paths); @@ -464,15 +469,12 @@ pub(crate) fn normalize_version(version: &str) -> &str { /// filesystem can't widen the write boundary. const PROJECT_ROOT_SEARCH_DEPTH: usize = 3; -/// Read `config.vendor-dir` out of a composer.json body, mirroring the +/// Read `config.vendor-dir` out of a composer.json (or the user-level +/// `config.json`, same `{"config": {...}}` shape) body, mirroring the /// slice of Composer's `Config::get('vendor-dir')` that matters here: /// trailing separators are trimmed (`"vendor/"` is legal) and an empty -/// value counts as unset. -/// -/// Composer additionally expands `$HOME`/`~`/`%VAR%` placeholders -/// (`Platform::expandPath`); that is deliberately NOT implemented — an -/// unexpanded value simply resolves to a directory that does not exist, -/// so discovery reports nothing rather than guessing at a path. +/// value counts as unset. Placeholder expansion is the caller's call, see +/// [`resolve_vendor_dir`]. fn parse_config_vendor_dir(composer_json: &str) -> Option { let doc: serde_json::Value = serde_json::from_str(composer_json).ok()?; let raw = doc.get("config")?.get("vendor-dir")?.as_str()?; @@ -480,64 +482,172 @@ fn parse_config_vendor_dir(composer_json: &str) -> Option { (!trimmed.is_empty()).then(|| trimmed.to_string()) } -/// Resolve the vendor directory of a local project the way Composer does: -/// `COMPOSER_VENDOR_DIR` wins, else composer.json `config.vendor-dir` -/// (relative to the manifest directory), else `vendor`. -/// -/// Composer relocates the WHOLE vendor tree — `composer/installed.json` -/// included — so assuming `/vendor` makes every installed package -/// invisible: scan reports them as lockfile-only ("not yet installed") -/// and apply resolves them as `package_not_found`. +/// Expand the placeholders Composer expands in every `*-dir` setting +/// (`Platform::expandPath`): a leading `~/` (or `~\`) becomes the user's +/// home, and a leading `$VAR` or `%VAR%` becomes that environment +/// variable. Anything else is returned as is. /// -/// Returns `None` when composer.json configures a value that is not a -/// plain relative subpath. That value comes from the project being -/// scanned and names the directory apply later WRITES patch content into, -/// so `../../elsewhere` or `/etc` is refused outright rather than -/// silently downgraded to `vendor/` (which would patch an unrelated -/// tree). Absolute paths are legal in Composer but are refused for the -/// same reason; a project using one discovers nothing, exactly as today. -/// `COMPOSER_VENDOR_DIR` is not gated — it comes from the invoking -/// environment, the same trust level as `CARGO_HOME` / `NUGET_PACKAGES` / -/// `MAVEN_REPO_LOCAL`, which are all honored verbatim. -async fn resolve_local_vendor_dir(cwd: &Path) -> Option { - let manifest = crate::utils::fs::read_regular_to_string(&cwd.join("composer.json")) - .await - .ok(); - vendor_dir_from(cwd, manifest.as_deref()) -} - -/// The blocking twin of [`resolve_local_vendor_dir`], for the human -/// next-step hints that name the installed package directory a user has -/// to remove before `composer install` (#658): those are formatted on a -/// synchronous path after the run, and must name the directory Composer -/// actually installed into, not a hardcoded `vendor/`. -pub fn resolve_local_vendor_dir_sync(cwd: &Path) -> Option { - let manifest = crate::utils::fs::read_regular_to_string_sync(&cwd.join("composer.json")).ok(); - vendor_dir_from(cwd, manifest.as_deref()) +/// `None` when the placeholder names nothing usable (no home, an unset or +/// empty variable). Composer would substitute `""` there and resolve a +/// root-relative path, which is never the tree a real install used. +fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> Option { + if let Some(rest) = raw.strip_prefix('~') { + if rest.starts_with(['/', '\\']) { + let home = crate::utils::fs::home_from_env(var)?; + return Some(format!("{}{rest}", home.to_string_lossy())); + } + return Some(raw.to_string()); + } + let is_word = |c: char| c.is_ascii_alphanumeric() || c == '_'; + let (name, rest) = if let Some(after) = raw.strip_prefix('$') { + let end = after.find(|c| !is_word(c)).unwrap_or(after.len()); + (&after[..end], &after[end..]) + } else if let Some(after) = raw.strip_prefix('%') { + let end = after.find(|c| !is_word(c)).unwrap_or(after.len()); + match after[end..].strip_prefix('%') { + Some(rest) => (&after[..end], rest), + None => return Some(raw.to_string()), + } + } else { + return Some(raw.to_string()); + }; + if name.is_empty() { + return Some(raw.to_string()); + } + // Composer treats `HOME` as an alias for `USERPROFILE` on Windows. + let value = match name { + "HOME" => crate::utils::fs::home_from_env(var).map(PathBuf::into_os_string), + _ => var(name).filter(|v| !v.is_empty()), + }?; + Some(format!("{}{rest}", value.to_string_lossy())) } -/// The shared resolution of the two entry points above, given the -/// composer.json text (`None` when it is missing or unreadable). -fn vendor_dir_from(cwd: &Path, composer_json: Option<&str>) -> Option { +/// Resolve Composer's `vendor-dir` the way `Config::get` does, highest +/// precedence first: +/// +/// 1. `COMPOSER_VENDOR_DIR`; +/// 2. the project's composer.json `config.vendor-dir` (`manifest`); +/// 3. the user-level `$COMPOSER_HOME/config.json` `config.vendor-dir` +/// (`user_config`, what `composer config -g vendor-dir` writes); +/// 4. `vendor`. +/// +/// Every value is expanded (`~/`, `$VAR`, `%VAR%`) and a relative one +/// resolves against `base`: the project directory, or `$COMPOSER_HOME` +/// for the global project. The environment and the Composer home belong +/// to the invoking user, the same trust level as `CARGO_HOME` / +/// `NUGET_PACKAGES` / `MAVEN_REPO_LOCAL`, so they are honoured as given. +/// +/// A composer.json that is NOT `manifest_trusted` (the scanned project's) +/// names the directory apply later WRITES patch content into, so it keeps +/// the stricter rule: only a plain relative subpath of the project, no +/// expansion. `../../elsewhere`, `/etc` or `~/deps` is refused (`None`) rather than +/// silently downgraded to `vendor/`, which would patch an unrelated tree. +/// Composer accepts absolute and home-relative values there; refusing +/// them means such a project discovers nothing. +fn resolve_vendor_dir( + base: &Path, + manifest: Option<&str>, + manifest_trusted: bool, + user_config: Option<&str>, + var: &impl Fn(&str) -> Option, +) -> Option { + let honoured = |raw: &str| expand_composer_path(raw, var).map(|value| base.join(value)); // A set-but-empty value counts as unset (twin of the MAVEN_REPO_LOCAL // and NUGET_PACKAGES rules): honoring `""` would resolve the vendor - // tree to the project root itself. - if let Some(from_env) = std::env::var("COMPOSER_VENDOR_DIR") - .ok() + // tree to the project root itself. `join` substitutes an absolute + // value for the base, matching Composer's relative-to-the-base-dir + // resolution. + if let Some(from_env) = var("COMPOSER_VENDOR_DIR") + .and_then(|v| v.into_string().ok()) .map(|v| v.trim_end_matches(['/', '\\']).to_string()) .filter(|v| !v.is_empty()) { - // `join` substitutes an absolute value for the base, matching - // Composer's own relative-to-the-manifest-dir resolution. - return Some(cwd.join(from_env)); + return honoured(&from_env); } - - match composer_json.and_then(parse_config_vendor_dir) { - Some(configured) => normalize_config_vendor_dir(&configured) + if let Some(configured) = manifest.and_then(parse_config_vendor_dir) { + if manifest_trusted { + return honoured(&configured); + } + // A placeholder Composer would expand (`~/x`, `$HOME/x`) names a + // directory outside the project, never the literal `/~/x`. + if expand_composer_path(&configured, var).as_deref() != Some(configured.as_str()) { + return None; + } + return normalize_config_vendor_dir(&configured) .filter(|normalized| path_safety::is_safe_multi_segment(normalized)) - .map(|normalized| cwd.join(normalized)), - None => Some(cwd.join("vendor")), + .map(|normalized| base.join(normalized)); + } + match user_config.and_then(parse_config_vendor_dir) { + Some(configured) => honoured(&configured), + None => Some(base.join("vendor")), + } +} + +/// `$COMPOSER_HOME` for LOCAL discovery, without running `composer`: the +/// env var, else the first platform default that exists (the fallbacks +/// [`get_composer_home`] ends with). Local scans read the user-level +/// `config.json` from here and must not spawn a subprocess to find it. +fn local_composer_home(var: &impl Fn(&str) -> Option) -> Option { + if let Some(home) = var("COMPOSER_HOME").map(PathBuf::from) { + if is_dir_sync(&home) { + return Some(home); + } } + composer_home_candidates(var, cfg!(windows)) + .into_iter() + .find(|candidate| is_dir_sync(candidate)) +} + +/// The vendor directory of the global project at `composer_home`: its +/// composer.json and config.json both live in the user's own Composer +/// home, so every layer is honoured as Composer honours it (#439). +fn global_vendor_dir_sync(composer_home: &Path) -> Option { + let read = + |name: &str| crate::utils::fs::read_regular_to_string_sync(&composer_home.join(name)).ok(); + let manifest = read("composer.json"); + let user_config = read("config.json"); + resolve_vendor_dir( + composer_home, + manifest.as_deref(), + true, + user_config.as_deref(), + &|name| std::env::var_os(name), + ) +} + +/// Resolve the vendor directory of a local project the way Composer does +/// (see [`resolve_vendor_dir`] for the cascade and the rules). +/// +/// Composer relocates the WHOLE vendor tree — `composer/installed.json` +/// included — so assuming `/vendor` makes every installed package +/// invisible: scan reports them as lockfile-only ("not yet installed") +/// and apply resolves them as `package_not_found`. +/// +/// Returns `None` when the project's composer.json configures a value the +/// crawler refuses (absolute, home-relative, or climbing out of the +/// project), or a user-level value whose placeholder names nothing. +async fn resolve_local_vendor_dir(cwd: &Path) -> Option { + let cwd = cwd.to_path_buf(); + run_blocking(move || resolve_local_vendor_dir_sync(&cwd)).await +} + +/// The blocking form of [`resolve_local_vendor_dir`], also used by the +/// human next-step hints that name the installed package directory a user +/// has to remove before `composer install` (#658): those are formatted on +/// a synchronous path after the run, and must name the directory Composer +/// actually installed into, not a hardcoded `vendor/`. +pub fn resolve_local_vendor_dir_sync(cwd: &Path) -> Option { + let var = |name: &str| std::env::var_os(name); + let read = |path: &Path| crate::utils::fs::read_regular_to_string_sync(path).ok(); + let manifest = read(&cwd.join("composer.json")); + let user_config = local_composer_home(&var).and_then(|home| read(&home.join("config.json"))); + resolve_vendor_dir( + cwd, + manifest.as_deref(), + false, + user_config.as_deref(), + &var, + ) } /// Reduce a `config.vendor-dir` value to plain `a/b` segments before the @@ -1565,6 +1675,67 @@ mod tests { assert_eq!(parse_config_vendor_dir("{ not json"), None); } + /// Composer's `Platform::expandPath`: only a LEADING `~/`, `$VAR` or + /// `%VAR%` expands; a placeholder naming nothing resolves nothing + /// rather than a root-relative path. + #[test] + fn test_expand_composer_path() { + let env = |name: &str| match name { + "HOME" => Some(OsString::from("/home/u")), + "DEPS" => Some(OsString::from("/srv/deps")), + "EMPTY" => Some(OsString::new()), + _ => None, + }; + let x = |raw: &str| expand_composer_path(raw, &env); + assert_eq!(x("~/shared").as_deref(), Some("/home/u/shared")); + assert_eq!(x("~\\shared").as_deref(), Some("/home/u\\shared")); + assert_eq!(x("$HOME/shared").as_deref(), Some("/home/u/shared")); + assert_eq!(x("$DEPS/php").as_deref(), Some("/srv/deps/php")); + assert_eq!(x("%DEPS%/php").as_deref(), Some("/srv/deps/php")); + // Not placeholders: left as is. + assert_eq!(x("lib/deps").as_deref(), Some("lib/deps")); + assert_eq!(x("~user/x").as_deref(), Some("~user/x")); + assert_eq!(x("lib/$DEPS").as_deref(), Some("lib/$DEPS")); + assert_eq!(x("%DEPS/x").as_deref(), Some("%DEPS/x")); + assert_eq!(x("$/x").as_deref(), Some("$/x")); + // Placeholders naming nothing. + assert_eq!(x("$UNSET/x"), None); + assert_eq!(x("$EMPTY/x"), None); + assert_eq!(expand_composer_path("~/x", &|_: &str| None), None); + } + + /// The cascade: env, then the project's composer.json, then the + /// user-level config.json, then `vendor`; only the scanned project's + /// composer.json keeps the relative-subpath gate (#439, #686). + #[test] + fn test_resolve_vendor_dir_cascade() { + let base = Path::new("/proj"); + let no_env = |name: &str| (name == "HOME").then(|| OsString::from("/home/u")); + let lib = Some(r#"{"config":{"vendor-dir":"lib"}}"#); + let abs = Some(r#"{"config":{"vendor-dir":"/abs/deps"}}"#); + let home = Some(r#"{"config":{"vendor-dir":"~/deps"}}"#); + let r = + |manifest, trusted, user| resolve_vendor_dir(base, manifest, trusted, user, &no_env); + + assert_eq!(r(None, false, None), Some(base.join("vendor"))); + assert_eq!(r(None, false, lib), Some(base.join("lib"))); + assert_eq!( + r(home, false, lib), + None, + "untrusted ~/ is refused, not skipped" + ); + assert_eq!(r(abs, false, lib), None, "untrusted absolute is refused"); + assert_eq!(r(abs, true, lib), Some(PathBuf::from("/abs/deps"))); + assert_eq!(r(home, true, None), Some(PathBuf::from("/home/u/deps"))); + assert_eq!(r(None, false, home), Some(PathBuf::from("/home/u/deps"))); + + let env = |name: &str| (name == "COMPOSER_VENDOR_DIR").then(|| OsString::from("third/")); + assert_eq!( + resolve_vendor_dir(base, abs, false, lib, &env), + Some(base.join("third")) + ); + } + #[test] fn test_normalize_config_vendor_dir() { let n = normalize_config_vendor_dir; diff --git a/crates/socket-patch-core/tests/crawler_composer_e2e.rs b/crates/socket-patch-core/tests/crawler_composer_e2e.rs index 9ca86bbf0..1bb13c615 100644 --- a/crates/socket-patch-core/tests/crawler_composer_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_composer_e2e.rs @@ -1341,3 +1341,236 @@ async fn uppercase_v_version_normalizes_identically_in_crawl_and_lock_inventory( package double-counts as installed + lockfile-only" ); } + +// ── vendor-dir config cascade (#439, #686) ──────────────────────────── + +/// Sets environment variables for one test and restores their previous +/// values on drop, so a failing assert can't leak them into later tests. +struct EnvVars(Vec<(&'static str, Option)>); + +impl EnvVars { + fn set(vars: &[(&'static str, Option<&std::ffi::OsStr>)]) -> Self { + let saved = vars + .iter() + .map(|(name, _)| (*name, std::env::var_os(name))) + .collect(); + for (name, value) in vars { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + EnvVars(saved) + } +} + +impl Drop for EnvVars { + fn drop(&mut self) { + for (name, value) in &self.0 { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + } +} + +fn global_opts(cwd: &Path) -> CrawlerOptions { + CrawlerOptions { + cwd: cwd.to_path_buf(), + global: true, + global_prefix: None, + } +} + +/// #439: `composer global config vendor-dir deps` writes the setting into +/// the global project's `$COMPOSER_HOME/composer.json`, and `composer +/// global require` then installs under `$COMPOSER_HOME/deps`. `scan -g` +/// must crawl that tree, not the hardcoded `$COMPOSER_HOME/vendor`. +#[tokio::test] +#[serial_test::serial] +async fn global_vendor_dir_from_global_composer_json() { + let home = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(home.path(), "deps", Some("deps")).await; + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", None), + ]); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&global_opts(home.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor.clone()]); + let packages = crawler.crawl_all(&global_opts(home.path())).await; + assert_eq!(packages.len(), 1, "got {packages:?}"); + assert_eq!(packages[0].path, vendor.join("monolog").join("monolog")); +} + +/// #439: the user-level `$COMPOSER_HOME/config.json` applies to the global +/// project too (Composer loads it under every project's composer.json). +#[tokio::test] +#[serial_test::serial] +async fn global_vendor_dir_from_user_config_json() { + let home = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(home.path(), "gdeps", None).await; + tokio::fs::write( + home.path().join("config.json"), + br#"{"config":{"vendor-dir":"gdeps"}}"#, + ) + .await + .unwrap(); + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", None), + ]); + + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(home.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor]); +} + +/// #439: `COMPOSER_VENDOR_DIR=gdeps composer global require …` installs +/// under `$COMPOSER_HOME/gdeps`: Composer resolves the relative value +/// against the global project's directory, the Composer home — not the +/// directory `scan -g` happens to run from. +#[tokio::test] +#[serial_test::serial] +async fn global_vendor_dir_env_resolves_against_composer_home() { + let home = tempfile::tempdir().unwrap(); + let elsewhere = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(home.path(), "gdeps", None).await; + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", Some(std::ffi::OsStr::new("gdeps"))), + ]); + + let paths = ComposerCrawler + .get_vendor_paths(&global_opts(elsewhere.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor]); +} + +/// #439: `composer config -g vendor-dir lib` writes the user-level +/// `$COMPOSER_HOME/config.json`, and every project's `composer install` +/// then installs into `/lib`. Local discovery must read that +/// layer under the project's own composer.json. +#[tokio::test] +#[serial_test::serial] +async fn local_vendor_dir_from_user_config_json() { + let home = tempfile::tempdir().unwrap(); + tokio::fs::write( + home.path().join("config.json"), + br#"{"config":{"vendor-dir":"lib"}}"#, + ) + .await + .unwrap(); + let proj = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(proj.path(), "lib", None).await; + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", None), + ]); + + let crawler = ComposerCrawler; + let paths = crawler + .get_vendor_paths(&options_at(proj.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor.clone()]); + let packages = crawler.crawl_all(&options_at(proj.path())).await; + assert_eq!(packages.len(), 1, "got {packages:?}"); + assert_eq!(packages[0].path, vendor.join("monolog").join("monolog")); +} + +/// The project's own `config.vendor-dir` outranks the user-level layer, +/// as Composer merges the project config over `config.json`. +#[tokio::test] +#[serial_test::serial] +async fn project_vendor_dir_outranks_user_config_json() { + let home = tempfile::tempdir().unwrap(); + tokio::fs::write( + home.path().join("config.json"), + br#"{"config":{"vendor-dir":"lib"}}"#, + ) + .await + .unwrap(); + let proj = tempfile::tempdir().unwrap(); + let vendor = stage_relocated_project(proj.path(), "deps", Some("deps")).await; + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", None), + ]); + + let paths = ComposerCrawler + .get_vendor_paths(&options_at(proj.path())) + .await + .unwrap(); + assert_eq!(paths, vec![vendor]); +} + +/// #686 (the part this fix covers): Composer expands a leading `~/` and +/// `$VAR` in every `*-dir` setting (`Platform::expandPath`). A value from +/// the invoking user's own `config.json` or environment is honoured after +/// that expansion, absolute results included. +#[tokio::test] +#[serial_test::serial] +async fn user_level_vendor_dir_expands_home_and_env_placeholders() { + let home = tempfile::tempdir().unwrap(); + let user_home = tempfile::tempdir().unwrap(); + let shared = stage_relocated_project(user_home.path(), "shared-deps", None).await; + let proj = tempfile::tempdir().unwrap(); + tokio::fs::write(proj.path().join("composer.json"), b"{}") + .await + .unwrap(); + + for value in [ + "~/shared-deps", + "$HOME/shared-deps", + "$SP_DEPS_ROOT/shared-deps", + ] { + tokio::fs::write( + home.path().join("config.json"), + format!(r#"{{"config":{{"vendor-dir":"{value}"}}}}"#), + ) + .await + .unwrap(); + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ("COMPOSER_VENDOR_DIR", None), + ("HOME", Some(user_home.path().as_os_str())), + ("SP_DEPS_ROOT", Some(user_home.path().as_os_str())), + ]); + let paths = ComposerCrawler + .get_vendor_paths(&options_at(proj.path())) + .await + .unwrap(); + assert_eq!( + paths, + vec![shared.clone()], + "config.json vendor-dir {value}" + ); + } + + // The env var gets the same expansion. + let _env = EnvVars::set(&[ + ("COMPOSER_HOME", Some(home.path().as_os_str())), + ( + "COMPOSER_VENDOR_DIR", + Some(std::ffi::OsStr::new("~/shared-deps")), + ), + ("HOME", Some(user_home.path().as_os_str())), + ]); + tokio::fs::remove_file(home.path().join("config.json")) + .await + .unwrap(); + let paths = ComposerCrawler + .get_vendor_paths(&options_at(proj.path())) + .await + .unwrap(); + assert_eq!(paths, vec![shared]); +} From 91fbb0f8432e3bee25fa6776631ea0fbf4afaefb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 01:58:23 +0000 Subject: [PATCH 3/4] Rewrap vendor-dir resolver doc comment Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/crawlers/composer_crawler.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index 751a1111f..aa8fefa7f 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -540,10 +540,10 @@ fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> O /// A composer.json that is NOT `manifest_trusted` (the scanned project's) /// names the directory apply later WRITES patch content into, so it keeps /// the stricter rule: only a plain relative subpath of the project, no -/// expansion. `../../elsewhere`, `/etc` or `~/deps` is refused (`None`) rather than -/// silently downgraded to `vendor/`, which would patch an unrelated tree. -/// Composer accepts absolute and home-relative values there; refusing -/// them means such a project discovers nothing. +/// expansion. `../../elsewhere`, `/etc` or `~/deps` is refused (`None`) +/// rather than silently downgraded to `vendor/`, which would patch an +/// unrelated tree. Composer accepts absolute and home-relative values +/// there; refusing them means such a project discovers nothing. fn resolve_vendor_dir( base: &Path, manifest: Option<&str>, From fed9863dd52c24c1e827b658e27ce2aca87aa2ad Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 02:12:56 +0000 Subject: [PATCH 4/4] Honour a set COMPOSER_HOME and read lazily Address review findings on the vendor-dir resolver: - A set COMPOSER_HOME is used exactly as Composer uses it, so a missing directory no longer falls back to ~/.composer's config.json, and a relative value resolves against the project. - The user-level config.json is read only when neither the env nor the project's composer.json decided the vendor dir. - Global and local discovery share one platform-default probe. - ~/ and $VAR expansion keep non-UTF-8 bytes. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/composer_hints.rs | 4 +- .../src/crawlers/composer_crawler.rs | 117 +++++++++++------- 2 files changed, 75 insertions(+), 46 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/composer_hints.rs b/crates/socket-patch-cli/src/commands/composer_hints.rs index 247d74519..d4637c423 100644 --- a/crates/socket-patch-cli/src/commands/composer_hints.rs +++ b/crates/socket-patch-cli/src/commands/composer_hints.rs @@ -24,8 +24,8 @@ use socket_patch_core::patch::redirect::FileEdit; /// to `cwd` with `/` separators when it lies inside the project (`vendor`, /// `lib`, `deps/php`), else the absolute path `COMPOSER_VENDOR_DIR` or the /// user-level config named. A `config.vendor-dir` the crawler refuses -/// (absolute, home-relative, or climbing out of the project) prints as the `` placeholder rather than a -/// directory that would be wrong. +/// (absolute, home-relative, or climbing out of the project) prints as the +/// `` placeholder rather than a directory that would be wrong. pub(crate) fn vendor_dir_label(cwd: &Path) -> String { let Some(dir) = socket_patch_core::crawlers::composer_crawler::resolve_local_vendor_dir_sync(cwd) diff --git a/crates/socket-patch-core/src/crawlers/composer_crawler.rs b/crates/socket-patch-core/src/crawlers/composer_crawler.rs index aa8fefa7f..4e9e2784c 100644 --- a/crates/socket-patch-core/src/crawlers/composer_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/composer_crawler.rs @@ -389,14 +389,15 @@ async fn get_composer_home() -> Option { } // Platform defaults (see `composer_home_candidates`). - let var = |name: &str| std::env::var_os(name); - for candidate in composer_home_candidates(&var, cfg!(windows)) { - if is_dir(&candidate).await { - return Some(candidate); - } - } + run_blocking(|| default_composer_home(&|name: &str| std::env::var_os(name))).await +} - None +/// The first of [`composer_home_candidates`] that exists: the platform +/// default home both global and local discovery fall back to. +fn default_composer_home(var: &impl Fn(&str) -> Option) -> Option { + composer_home_candidates(var, cfg!(windows)) + .into_iter() + .find(|candidate| is_dir_sync(candidate)) } /// The directories Composer itself uses as its home when `COMPOSER_HOME` @@ -490,13 +491,19 @@ fn parse_config_vendor_dir(composer_json: &str) -> Option { /// `None` when the placeholder names nothing usable (no home, an unset or /// empty variable). Composer would substitute `""` there and resolve a /// root-relative path, which is never the tree a real install used. -fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> Option { +fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> Option { + // Built from `OsString`s so a non-UTF-8 home or variable keeps its bytes. + let join = |head: OsString, rest: &str| { + let mut out = head; + out.push(rest); + out + }; if let Some(rest) = raw.strip_prefix('~') { if rest.starts_with(['/', '\\']) { let home = crate::utils::fs::home_from_env(var)?; - return Some(format!("{}{rest}", home.to_string_lossy())); + return Some(join(home.into_os_string(), rest)); } - return Some(raw.to_string()); + return Some(raw.into()); } let is_word = |c: char| c.is_ascii_alphanumeric() || c == '_'; let (name, rest) = if let Some(after) = raw.strip_prefix('$') { @@ -506,20 +513,20 @@ fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> O let end = after.find(|c| !is_word(c)).unwrap_or(after.len()); match after[end..].strip_prefix('%') { Some(rest) => (&after[..end], rest), - None => return Some(raw.to_string()), + None => return Some(raw.into()), } } else { - return Some(raw.to_string()); + return Some(raw.into()); }; if name.is_empty() { - return Some(raw.to_string()); + return Some(raw.into()); } // Composer treats `HOME` as an alias for `USERPROFILE` on Windows. let value = match name { "HOME" => crate::utils::fs::home_from_env(var).map(PathBuf::into_os_string), _ => var(name).filter(|v| !v.is_empty()), }?; - Some(format!("{}{rest}", value.to_string_lossy())) + Some(join(value, rest)) } /// Resolve Composer's `vendor-dir` the way `Config::get` does, highest @@ -528,7 +535,8 @@ fn expand_composer_path(raw: &str, var: &impl Fn(&str) -> Option) -> O /// 1. `COMPOSER_VENDOR_DIR`; /// 2. the project's composer.json `config.vendor-dir` (`manifest`); /// 3. the user-level `$COMPOSER_HOME/config.json` `config.vendor-dir` -/// (`user_config`, what `composer config -g vendor-dir` writes); +/// (`user_config` reads its text; what `composer config -g vendor-dir` +/// writes); /// 4. `vendor`. /// /// Every value is expanded (`~/`, `$VAR`, `%VAR%`) and a relative one @@ -548,7 +556,7 @@ fn resolve_vendor_dir( base: &Path, manifest: Option<&str>, manifest_trusted: bool, - user_config: Option<&str>, + user_config: impl FnOnce() -> Option, var: &impl Fn(&str) -> Option, ) -> Option { let honoured = |raw: &str| expand_composer_path(raw, var).map(|value| base.join(value)); @@ -570,32 +578,31 @@ fn resolve_vendor_dir( } // A placeholder Composer would expand (`~/x`, `$HOME/x`) names a // directory outside the project, never the literal `/~/x`. - if expand_composer_path(&configured, var).as_deref() != Some(configured.as_str()) { + if expand_composer_path(&configured, var) != Some(OsString::from(&configured)) { return None; } return normalize_config_vendor_dir(&configured) .filter(|normalized| path_safety::is_safe_multi_segment(normalized)) .map(|normalized| base.join(normalized)); } - match user_config.and_then(parse_config_vendor_dir) { + // Read only when nothing above decided: most projects never need it. + match user_config().as_deref().and_then(parse_config_vendor_dir) { Some(configured) => honoured(&configured), None => Some(base.join("vendor")), } } -/// `$COMPOSER_HOME` for LOCAL discovery, without running `composer`: the -/// env var, else the first platform default that exists (the fallbacks -/// [`get_composer_home`] ends with). Local scans read the user-level -/// `config.json` from here and must not spawn a subprocess to find it. -fn local_composer_home(var: &impl Fn(&str) -> Option) -> Option { - if let Some(home) = var("COMPOSER_HOME").map(PathBuf::from) { - if is_dir_sync(&home) { - return Some(home); - } +/// `$COMPOSER_HOME` for LOCAL discovery of the project at `cwd`, without +/// running `composer`: the env var exactly as Composer uses it (a relative +/// value against the directory composer runs in, the project; a missing +/// directory just holds no config), else [`default_composer_home`]. Local +/// scans read the user-level `config.json` from here and must not spawn a +/// subprocess to find it. +fn local_composer_home(cwd: &Path, var: &impl Fn(&str) -> Option) -> Option { + match var("COMPOSER_HOME").filter(|home| !home.is_empty()) { + Some(home) => Some(cwd.join(home)), + None => default_composer_home(var), } - composer_home_candidates(var, cfg!(windows)) - .into_iter() - .find(|candidate| is_dir_sync(candidate)) } /// The vendor directory of the global project at `composer_home`: its @@ -605,12 +612,11 @@ fn global_vendor_dir_sync(composer_home: &Path) -> Option { let read = |name: &str| crate::utils::fs::read_regular_to_string_sync(&composer_home.join(name)).ok(); let manifest = read("composer.json"); - let user_config = read("config.json"); resolve_vendor_dir( composer_home, manifest.as_deref(), true, - user_config.as_deref(), + || read("config.json"), &|name| std::env::var_os(name), ) } @@ -640,14 +646,9 @@ pub fn resolve_local_vendor_dir_sync(cwd: &Path) -> Option { let var = |name: &str| std::env::var_os(name); let read = |path: &Path| crate::utils::fs::read_regular_to_string_sync(path).ok(); let manifest = read(&cwd.join("composer.json")); - let user_config = local_composer_home(&var).and_then(|home| read(&home.join("config.json"))); - resolve_vendor_dir( - cwd, - manifest.as_deref(), - false, - user_config.as_deref(), - &var, - ) + let user_config = + || local_composer_home(cwd, &var).and_then(|home| read(&home.join("config.json"))); + resolve_vendor_dir(cwd, manifest.as_deref(), false, user_config, &var) } /// Reduce a `config.vendor-dir` value to plain `a/b` segments before the @@ -1686,7 +1687,7 @@ mod tests { "EMPTY" => Some(OsString::new()), _ => None, }; - let x = |raw: &str| expand_composer_path(raw, &env); + let x = |raw: &str| expand_composer_path(raw, &env).map(|v| v.into_string().unwrap()); assert_eq!(x("~/shared").as_deref(), Some("/home/u/shared")); assert_eq!(x("~\\shared").as_deref(), Some("/home/u\\shared")); assert_eq!(x("$HOME/shared").as_deref(), Some("/home/u/shared")); @@ -1714,8 +1715,15 @@ mod tests { let lib = Some(r#"{"config":{"vendor-dir":"lib"}}"#); let abs = Some(r#"{"config":{"vendor-dir":"/abs/deps"}}"#); let home = Some(r#"{"config":{"vendor-dir":"~/deps"}}"#); - let r = - |manifest, trusted, user| resolve_vendor_dir(base, manifest, trusted, user, &no_env); + let r = |manifest, trusted, user: Option<&str>| { + resolve_vendor_dir( + base, + manifest, + trusted, + || user.map(str::to_string), + &no_env, + ) + }; assert_eq!(r(None, false, None), Some(base.join("vendor"))); assert_eq!(r(None, false, lib), Some(base.join("lib"))); @@ -1731,9 +1739,30 @@ mod tests { let env = |name: &str| (name == "COMPOSER_VENDOR_DIR").then(|| OsString::from("third/")); assert_eq!( - resolve_vendor_dir(base, abs, false, lib, &env), + resolve_vendor_dir(base, abs, false, || panic!("env decided"), &env), Some(base.join("third")) ); + // The user config is only read when nothing above decided. + assert_eq!( + resolve_vendor_dir(base, lib, false, || panic!("manifest decided"), &no_env), + Some(base.join("lib")) + ); + } + + /// A set `COMPOSER_HOME` is used exactly as Composer uses it, even when + /// the directory is missing (no fallback to another home's config), and + /// a relative value resolves against the project. + #[test] + fn test_local_composer_home_honours_set_env() { + let cwd = Path::new("/proj"); + let env = |value: &'static str| { + move |name: &str| (name == "COMPOSER_HOME").then(|| OsString::from(value)) + }; + assert_eq!( + local_composer_home(cwd, &env("/missing/composer-home")), + Some(PathBuf::from("/missing/composer-home")) + ); + assert_eq!(local_composer_home(cwd, &env("ch")), Some(cwd.join("ch"))); } #[test]