From a5bb504a9059a8be1e77d78b9cd18b8b22e16ed3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:53:40 +0000 Subject: [PATCH 1/3] Start fix for #823 slice 13 Assisted-by: Claude Code:claude-opus-5-5 From dce11cca8821d5740b44081e7d7625d307640dad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 19:59:42 +0000 Subject: [PATCH 2/3] Spawn Cargo/Composer test CLIs hermetically The Cargo and Composer suites started socket-patch with a bare Command::new and their own partial SOCKET_* handling, which had drifted from tests/common/hermetic.rs. e2e_cargo and e2e_composer removed six named vars, so any other ambient SOCKET_* reached the child: with SOCKET_ECOSYSTEMS=npm in the shell, 5 e2e_cargo and 2 e2e_composer tests fail on main. None forced the config layer or the update notifier off. All three now start from hermetic::command, scrub an activated venv (Extra::Venv) and use the stand-in HOME from jvm_env::isolate_cli, since their scans are unscoped. The Composer suites also drop an ambient COMPOSER_VENDOR_DIR: it moves the vendor directory the crawler reads off the fixture's, and fails the agent-mode sync test in e2e_composer_version_identity on main. The files leave PENDING_RAW_SPAWNS (22 -> 19), so the ratchet keeps them hermetic. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/tests/e2e_cargo.rs | 27 +++++++++---------- crates/socket-patch-cli/tests/e2e_composer.rs | 27 ++++++++++--------- .../tests/e2e_composer_version_identity.rs | 21 ++++++++------- .../tests/spawn_env_hygiene.rs | 3 --- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_cargo.rs b/crates/socket-patch-cli/tests/e2e_cargo.rs index 58eb89672..c9b2b00ac 100644 --- a/crates/socket-patch-cli/tests/e2e_cargo.rs +++ b/crates/socket-patch-cli/tests/e2e_cargo.rs @@ -15,10 +15,10 @@ #[path = "common/mod.rs"] mod common; -use common::{binary, git_sha256}; +use common::{binary, git_sha256, hermetic}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::Output; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -46,27 +46,24 @@ async fn start_proxy() -> MockServer { /// Run the binary as a blocking subprocess (off the async runtime so the /// in-test proxy can service its requests concurrently), pinned to `proxy_url`. /// -/// `SOCKET_API_TOKEN` is stripped so the binary deterministically takes the -/// public-proxy path (an ambient token would flip it onto the authenticated -/// API, bypassing `--proxy-url`), and every other variable that could -/// redirect the API elsewhere or disable it is scrubbed so an ambient value -/// can't quietly change what the scan reports. +/// The child gets the hermetic `SOCKET_*` environment (no token, so the +/// binary deterministically takes the public-proxy path `--proxy-url` +/// pins). The scans are unscoped, so an activated venv is scrubbed and +/// `HOME` points at a stand-in (`jvm_env::isolate_cli`): the developer's +/// venv, `~/.m2` or user site-packages would add packages to the counts +/// these tests assert. `CARGO_HOME` is the fixture's own `.cargo`. async fn run(args: &[&str], cwd: &Path, proxy_url: &str) -> Output { let mut args: Vec = args.iter().map(|s| s.to_string()).collect(); args.extend(["--proxy-url".to_string(), proxy_url.to_string()]); let cwd = cwd.to_path_buf(); tokio::task::spawn_blocking(move || { let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); - Command::new(binary()) - .args(&arg_refs) + let mut cmd = hermetic::command(&binary()); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + common::jvm_env::isolate_cli(&mut cmd); + cmd.args(&arg_refs) .current_dir(&cwd) .env("CARGO_HOME", cwd.join(".cargo")) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") - .env_remove("SOCKET_API_URL") - .env_remove("SOCKET_OFFLINE") - .env_remove("SOCKET_PROXY_URL") - .env_remove("SOCKET_BATCH_SIZE") .output() .expect("Failed to run socket-patch binary") }) diff --git a/crates/socket-patch-cli/tests/e2e_composer.rs b/crates/socket-patch-cli/tests/e2e_composer.rs index a5ccc37f1..e04ae343a 100644 --- a/crates/socket-patch-cli/tests/e2e_composer.rs +++ b/crates/socket-patch-cli/tests/e2e_composer.rs @@ -14,9 +14,9 @@ #[path = "common/mod.rs"] mod common; -use common::binary; +use common::{binary, hermetic}; -use std::process::{Command, Output}; +use std::process::Output; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -43,24 +43,25 @@ async fn start_proxy() -> MockServer { /// Run the binary as a blocking subprocess (off the async runtime so the /// in-test proxy can service its requests concurrently), pinned to -/// `proxy_url`. `SOCKET_API_TOKEN` is stripped so the binary -/// deterministically takes the public-proxy path, and every other variable -/// that could redirect the API elsewhere or disable it is scrubbed. +/// `proxy_url`. The child gets the hermetic `SOCKET_*` environment (no +/// token, so the binary deterministically takes the public-proxy path +/// `--proxy-url` pins). The scans are unscoped, so an activated venv is +/// scrubbed and `HOME` points at a stand-in (`jvm_env::isolate_cli`): the +/// developer's venv, `~/.m2` or user site-packages would add packages to +/// the counts these tests assert. An ambient `COMPOSER_VENDOR_DIR` would +/// move the vendor directory the crawler reads off the fixture's. async fn run(args: &[&str], cwd: &std::path::Path, proxy_url: &str) -> Output { let mut args: Vec = args.iter().map(|s| s.to_string()).collect(); args.extend(["--proxy-url".to_string(), proxy_url.to_string()]); let cwd = cwd.to_path_buf(); tokio::task::spawn_blocking(move || { let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); - Command::new(binary()) - .args(&arg_refs) + let mut cmd = hermetic::command(&binary()); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + common::jvm_env::isolate_cli(&mut cmd); + cmd.args(&arg_refs) .current_dir(&cwd) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") - .env_remove("SOCKET_API_URL") - .env_remove("SOCKET_OFFLINE") - .env_remove("SOCKET_PROXY_URL") - .env_remove("SOCKET_BATCH_SIZE") + .env_remove("COMPOSER_VENDOR_DIR") .output() .expect("Failed to run socket-patch binary") }) diff --git a/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs b/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs index 0051d2bcd..4d802a362 100644 --- a/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs +++ b/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs @@ -9,6 +9,8 @@ //! `scan --redirect` repoints the lock entry. Each test runs the built binary //! against a wiremock API that serves only the padded spelling. +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "prebuilt_common/mod.rs"] mod prebuilt_common; @@ -180,17 +182,16 @@ async fn mount_api(server: &MockServer, prebuilt: bool) { .await; } -/// The built binary with every ambient `SOCKET_*` variable scrubbed and -/// telemetry off, so the assertions reflect only the argv. +/// The built binary in the hermetic `SOCKET_*` environment, so the +/// assertions reflect only the argv. The scans are unscoped, so an +/// activated venv is scrubbed and `HOME` points at a stand-in +/// (`jvm_env::isolate_cli`), and an ambient `COMPOSER_VENDOR_DIR` can't move +/// the vendor directory off the fixture's. fn cli() -> Command { - let mut cmd = Command::new(env!("CARGO_BIN_EXE_socket-patch")); - for (key, _) in std::env::vars_os() { - let name = key.to_string_lossy(); - if name.starts_with("SOCKET_") && name != "SOCKET_NO_CONFIG" { - cmd.env_remove(&key); - } - } - cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let mut cmd = hermetic::binary_command(); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + prebuilt_common::jvm_env::isolate_cli(&mut cmd); + cmd.env_remove("COMPOSER_VENDOR_DIR"); cmd } diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index 55f4bbb1b..928965453 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -30,9 +30,6 @@ use serial_test::serial; /// scrubs by hand. const PENDING_RAW_SPAWNS: &[&str] = &[ "apply/apply_network.rs", - "e2e_cargo.rs", - "e2e_composer.rs", - "e2e_composer_version_identity.rs", "e2e_gem.rs", "e2e_hosted_production.rs", "e2e_maven.rs", From cdb3669594937f01ebf666f364eae6de422be214 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 20:13:59 +0000 Subject: [PATCH 3/3] Keep user Composer config out of test CLIs From /code-review high: the Composer suites only dropped an ambient COMPOSER_VENDOR_DIR. The crawler also reads vendor-dir from the user-level config.json, which it finds through COMPOSER_HOME or an absolute XDG_CONFIG_HOME, so a developer whose Composer config sets vendor-dir still failed 2 e2e_composer tests and 1 e2e_composer_version_identity test. hermetic::Extra::Composer now removes every ambient COMPOSER_* var and pins COMPOSER_HOME to a missing directory, which outranks XDG_CONFIG_HOME and holds no config. Both Composer suites use it, and spawn_env_hygiene checks the scrub. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/common/hermetic.rs | 16 +++++++++ crates/socket-patch-cli/tests/e2e_composer.rs | 11 ++++--- .../tests/e2e_composer_version_identity.rs | 13 +++++--- .../tests/spawn_env_hygiene.rs | 33 +++++++++++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/tests/common/hermetic.rs b/crates/socket-patch-cli/tests/common/hermetic.rs index 8f5e78057..0d2eabf68 100644 --- a/crates/socket-patch-cli/tests/common/hermetic.rs +++ b/crates/socket-patch-cli/tests/common/hermetic.rs @@ -209,6 +209,14 @@ pub enum Extra { /// Every `POETRY_*` var: Poetry reads any config key from /// `POETRY_` (`POETRY_VIRTUALENVS_PATH`, repository URLs, ...). Poetry, + /// Every `COMPOSER_*` var, then `COMPOSER_HOME` pinned to a missing + /// directory. The crawler resolves a project's vendor directory from + /// `COMPOSER_VENDOR_DIR`, else the user-level `config.json` in the + /// Composer home, which it finds through `COMPOSER_HOME` or an + /// absolute `XDG_CONFIG_HOME`; any of them moves the crawl off the + /// fixture's `vendor/`. The pin outranks `XDG_CONFIG_HOME` and holds + /// no config. Run it before the suite sets its own `COMPOSER_*` values. + Composer, } /// Apply each of `extras` to `cmd` (see [`Extra`]). Run it before @@ -279,6 +287,14 @@ pub fn scrub_extra(cmd: &mut Command, extras: &[Extra]) { } } } + Extra::Composer => { + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("COMPOSER_") { + cmd.env_remove(&key); + } + } + cmd.env("COMPOSER_HOME", "/nonexistent/composer-home"); + } } } } diff --git a/crates/socket-patch-cli/tests/e2e_composer.rs b/crates/socket-patch-cli/tests/e2e_composer.rs index e04ae343a..cb378ffb2 100644 --- a/crates/socket-patch-cli/tests/e2e_composer.rs +++ b/crates/socket-patch-cli/tests/e2e_composer.rs @@ -48,8 +48,9 @@ async fn start_proxy() -> MockServer { /// `--proxy-url` pins). The scans are unscoped, so an activated venv is /// scrubbed and `HOME` points at a stand-in (`jvm_env::isolate_cli`): the /// developer's venv, `~/.m2` or user site-packages would add packages to -/// the counts these tests assert. An ambient `COMPOSER_VENDOR_DIR` would -/// move the vendor directory the crawler reads off the fixture's. +/// the counts these tests assert. `Extra::Composer` keeps an ambient +/// `COMPOSER_VENDOR_DIR` or user-level Composer config from moving the +/// vendor directory the crawler reads off the fixture's. async fn run(args: &[&str], cwd: &std::path::Path, proxy_url: &str) -> Output { let mut args: Vec = args.iter().map(|s| s.to_string()).collect(); args.extend(["--proxy-url".to_string(), proxy_url.to_string()]); @@ -57,11 +58,13 @@ async fn run(args: &[&str], cwd: &std::path::Path, proxy_url: &str) -> Output { tokio::task::spawn_blocking(move || { let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let mut cmd = hermetic::command(&binary()); - hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + hermetic::scrub_extra( + &mut cmd, + &[hermetic::Extra::Venv, hermetic::Extra::Composer], + ); common::jvm_env::isolate_cli(&mut cmd); cmd.args(&arg_refs) .current_dir(&cwd) - .env_remove("COMPOSER_VENDOR_DIR") .output() .expect("Failed to run socket-patch binary") }) diff --git a/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs b/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs index 4d802a362..8d1c31d81 100644 --- a/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs +++ b/crates/socket-patch-cli/tests/e2e_composer_version_identity.rs @@ -183,15 +183,18 @@ async fn mount_api(server: &MockServer, prebuilt: bool) { } /// The built binary in the hermetic `SOCKET_*` environment, so the -/// assertions reflect only the argv. The scans are unscoped, so an -/// activated venv is scrubbed and `HOME` points at a stand-in -/// (`jvm_env::isolate_cli`), and an ambient `COMPOSER_VENDOR_DIR` can't move +/// assertions reflect only the argv. Its scans and `vex` runs crawl every +/// ecosystem, so an activated venv is scrubbed and `HOME` points at a +/// stand-in (`jvm_env::isolate_cli`), and `Extra::Composer` keeps an +/// ambient `COMPOSER_VENDOR_DIR` or user-level Composer config from moving /// the vendor directory off the fixture's. fn cli() -> Command { let mut cmd = hermetic::binary_command(); - hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + hermetic::scrub_extra( + &mut cmd, + &[hermetic::Extra::Venv, hermetic::Extra::Composer], + ); prebuilt_common::jvm_env::isolate_cli(&mut cmd); - cmd.env_remove("COMPOSER_VENDOR_DIR"); cmd } diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index 928965453..e228b5424 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -454,6 +454,39 @@ fn bundle_extra_sweeps_every_bundler_override() { ); } +#[test] +#[serial] +fn composer_extra_sweeps_overrides_and_pins_an_empty_home() { + with_ambient( + &[ + ("COMPOSER_VENDOR_DIR", "third"), + ("COMPOSER_HOME", "/ambient/composer"), + ("COMPOSER_CACHE_DIR", "/ambient/composer-cache"), + ("XDG_CONFIG_HOME", "/ambient/config"), + ], + || { + let mut cmd = Command::new("socket-patch"); + hermetic::scrub_extra(&mut cmd, &[Extra::Composer]); + let env = effective_env(&cmd); + assert_eq!(get(&env, "COMPOSER_VENDOR_DIR"), None); + assert_eq!(get(&env, "COMPOSER_CACHE_DIR"), None); + // The pin outranks XDG_CONFIG_HOME in the Composer home lookup. + assert_eq!( + get(&env, "COMPOSER_HOME"), + Some("/nonexistent/composer-home") + ); + assert_eq!(get(&env, "XDG_CONFIG_HOME"), Some("/ambient/config")); + + // A suite's own COMPOSER_HOME, set after the scrub, survives. + cmd.env("COMPOSER_HOME", "/fixture/home"); + assert_eq!( + get(&effective_env(&cmd), "COMPOSER_HOME"), + Some("/fixture/home") + ); + }, + ); +} + #[test] #[serial] fn extra_seeds_are_scrubbed_whatever_the_ambient_value() {