From 805b37589ce72c42f6349cb73186eb5deb9ab06f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:00:19 +0000 Subject: [PATCH 1/5] Start refactor for #823 Assisted-by: Claude Code:claude-opus-5-5 From 37b5b4e47a4933a12539ed328795401c73ad7bdf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:08:55 +0000 Subject: [PATCH 2/5] Spawn CLI test children via one hermetic builder Test children inherited ambient SOCKET_* settings through 15 private scrub_socket_env copies and 10 unscrubbed spawners, so a developer's shell (SOCKET_DRY_RUN, SOCKET_OFFLINE, ...) could silently change what a suite exercises. common/hermetic.rs now holds the one builder: hermetic::command seeds and scrubs SOCKET_* and forces SOCKET_NO_CONFIG and SOCKET_NO_UPDATE_CHECK; scrub_extra adds the opt-in yarn, pnpm and venv sweeps. run_bin_with_env is built on it. This moves the 8 copies and 8 unscrubbed spawners that no open fix PR touches onto the builder and deletes those copies. spawn_env_hygiene tests the builder's contract and ratchets the remaining copies and bare binary spawns. Test-only; no production change. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/cli/api_client_errors_e2e.rs | 19 +- .../tests/cli_parse_remove.rs | 5 +- .../socket-patch-cli/tests/common/hermetic.rs | 149 ++++++ crates/socket-patch-cli/tests/common/mod.rs | 65 +-- .../tests/e2e_redirect_pnpm_build.rs | 34 +- .../tests/e2e_redirect_rush_sim.rs | 29 +- .../tests/e2e_vendor_pnpm_build.rs | 36 +- .../tests/e2e_vendor_yarn_classic_dev_flow.rs | 23 +- .../e2e_yarn_legacy_cachekey_refusal_build.rs | 27 +- .../tests/ecosystem_dispatch_e2e.rs | 66 +-- .../tests/get/get_batch_paths_e2e.rs | 26 +- .../tests/mode_migration_npm.rs | 26 +- .../tests/repair/repair_vendor_e2e.rs | 3 +- .../tests/scan/scan_sync_e2e.rs | 9 +- .../socket-patch-cli/tests/self_update_e2e.rs | 2 +- .../tests/spawn_env_hygiene.rs | 503 ++++++++++++++++++ .../tests/update/covgap_update_swap.rs | 2 +- 17 files changed, 735 insertions(+), 289 deletions(-) create mode 100644 crates/socket-patch-cli/tests/common/hermetic.rs create mode 100644 crates/socket-patch-cli/tests/spawn_env_hygiene.rs diff --git a/crates/socket-patch-cli/tests/cli/api_client_errors_e2e.rs b/crates/socket-patch-cli/tests/cli/api_client_errors_e2e.rs index ef9d2f75a..9415fb024 100644 --- a/crates/socket-patch-cli/tests/cli/api_client_errors_e2e.rs +++ b/crates/socket-patch-cli/tests/cli/api_client_errors_e2e.rs @@ -6,7 +6,6 @@ //! failure into a fake success fails loudly. use std::path::{Path, PathBuf}; -use std::process::Command; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -116,7 +115,7 @@ async fn get_uuid_with_401_falls_back_to_proxy() { .await; let tmp = tempfile::tempdir().unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", UUID, @@ -199,7 +198,7 @@ async fn get_uuid_with_500_reports_error() { .await; let tmp = tempfile::tempdir().unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", UUID, @@ -239,7 +238,7 @@ async fn get_uuid_with_malformed_json_reports_parse_error() { .await; let tmp = tempfile::tempdir().unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", UUID, @@ -281,7 +280,7 @@ async fn scan_with_400_bad_request_reports_failure() { write_root(tmp.path()); write_npm_package(tmp.path(), "foo"); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", @@ -323,7 +322,7 @@ async fn scan_with_400_bad_request_reports_failure() { async fn get_with_unreachable_api_url_reports_error() { let tmp = tempfile::tempdir().unwrap(); // Port 1 is reserved and reliably refuses connections. - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", UUID, @@ -354,7 +353,7 @@ async fn scan_with_unreachable_api_url_reports_failure() { write_root(tmp.path()); write_npm_package(tmp.path(), "bar"); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", @@ -397,7 +396,7 @@ async fn get_by_cve_with_500_reports_error() { .await; let tmp = tempfile::tempdir().unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", cve, @@ -434,7 +433,7 @@ async fn get_by_ghsa_with_404_reports_not_found() { .await; let tmp = tempfile::tempdir().unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "get", ghsa, @@ -510,7 +509,7 @@ async fn repair_with_blob_404_marks_failure_in_summary() { ) .unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "repair", "--json", diff --git a/crates/socket-patch-cli/tests/cli_parse_remove.rs b/crates/socket-patch-cli/tests/cli_parse_remove.rs index 3a73819e6..ee4db08b8 100644 --- a/crates/socket-patch-cli/tests/cli_parse_remove.rs +++ b/crates/socket-patch-cli/tests/cli_parse_remove.rs @@ -12,6 +12,9 @@ use socket_patch_cli::commands::remove::{run, RemoveArgs}; use socket_patch_cli::{Cli, Commands}; use std::path::PathBuf; +#[path = "common/hermetic.rs"] +mod hermetic; + fn parse_remove(extra: &[&str]) -> RemoveArgs { let mut argv = vec!["socket-patch", "remove"]; argv.extend_from_slice(extra); @@ -335,7 +338,7 @@ fn record_json(uuid: &str) -> String { /// Run the compiled `socket-patch remove` binary against `cwd`, fully offline /// and with telemetry disabled so the test never touches the network. fn run_remove_binary(cwd: &std::path::Path, extra: &[&str]) -> std::process::Output { - std::process::Command::new(env!("CARGO_BIN_EXE_socket-patch")) + hermetic::binary_command() .arg("remove") .arg("--cwd") .arg(cwd) diff --git a/crates/socket-patch-cli/tests/common/hermetic.rs b/crates/socket-patch-cli/tests/common/hermetic.rs new file mode 100644 index 000000000..31822f99c --- /dev/null +++ b/crates/socket-patch-cli/tests/common/hermetic.rs @@ -0,0 +1,149 @@ +//! The one hermetic environment for every child process the CLI tests spawn. +//! +//! The binary binds a wide `SOCKET_*` env surface (SOCKET_CWD, +//! SOCKET_DRY_RUN, SOCKET_STRICT, SOCKET_GLOBAL, SOCKET_MANIFEST_PATH, ...). +//! An ambient value silently changes what a test exercises: +//! `SOCKET_DRY_RUN=true` turns every real apply into a no-op, +//! `SOCKET_GLOBAL_PREFIX` flips commands into global mode (aiming mutations +//! at the host's *real* global caches), and the output-mode trio +//! (`SOCKET_JSON` / `SOCKET_SILENT` / `SOCKET_VERBOSE`) flips which printer a +//! test's assertions run against. +//! +//! [`command`] (and [`binary_command`] for the built binary) is the only way +//! a test spawns `socket-patch`; `common::run_bin_with_env` is built on it. +//! Package-manager children the same tests spawn get [`scrub_socket_vars`] +//! plus whichever [`Extra`] scrubs their tool needs. +//! +//! Files that don't need the rest of `common` pull this in on its own with +//! `#[path = "common/hermetic.rs"] mod hermetic;`. +//! +//! ## Ordering +//! +//! `Command`'s env operations are keyed by variable name and the last call +//! for a given name wins. So scrub first (the sweeps iterate the *parent* +//! environment and would otherwise remove values set earlier), then +//! `cache_env::isolate`, then any env the individual test needs. + +#![allow(dead_code)] + +use std::path::Path; +use std::process::Command; + +/// The highest-risk `SOCKET_*` vars. [`command`] seeds each with a hostile +/// value and then removes it: `env_remove` clears the seed too, so the child +/// never sees it, but if a scrub line is ever dropped the seed (rather than a +/// developer's ambient shell, which the suites can't rely on) turns the tests +/// red immediately. +const HOSTILE_SEEDS: &[(&str, &str)] = &[ + ("SOCKET_GLOBAL", "true"), + ("SOCKET_GLOBAL_PREFIX", "/nonexistent"), + ("SOCKET_DRY_RUN", "true"), + ("SOCKET_MANIFEST_PATH", "/nonexistent/manifest.json"), + ("SOCKET_JSON", "true"), + ("SOCKET_SILENT", "true"), + ("SOCKET_VERBOSE", "true"), + ("SOCKET_UPDATE_BASE_URL", "http://127.0.0.1:1"), + ("SOCKET_UPDATE_STATE_DIR", "/nonexistent"), +]; + +/// A `Command` for `bin` with the hermetic `SOCKET_*` environment: the +/// hostile seeds scrubbed, every other ambient `SOCKET_*` removed (removing +/// `SOCKET_API_TOKEN` also forces the public proxy), and the two opt-outs +/// forced on. Callers add args, cwd and their own env afterwards; caller env +/// lands last, so explicit injections survive the scrub. +pub fn command(bin: &Path) -> Command { + let mut cmd = Command::new(bin); + for (k, v) in HOSTILE_SEEDS { + cmd.env(k, v); + } + for (k, _) in HOSTILE_SEEDS { + cmd.env_remove(k); + } + cmd.env_remove("SOCKET_API_TOKEN"); + scrub_socket_vars(&mut cmd); + // Belt-and-braces on top of the `.cargo/config.toml` `[env]` default: + // a developer's real `socket login` (the socket-cli config.json token + // fallback) must never authenticate a test child — it would flip every + // "no token → public proxy" assertion onto the authed path. + cmd.env("SOCKET_NO_CONFIG", "1"); + // Same posture for the passive update notifier: no test child may ever + // fetch release metadata from real GitHub. The stderr-TTY guard covers + // piped children, but the PTY suites hand the binary a real terminal — + // this force-set is the layer that holds there. Notifier tests opt back + // in via caller env (which lands last). + cmd.env("SOCKET_NO_UPDATE_CHECK", "1"); + cmd +} + +/// [`command`] for the `socket-patch` binary cargo built for this test run. +pub fn binary_command() -> Command { + command(Path::new(env!("CARGO_BIN_EXE_socket-patch"))) +} + +/// Remove every ambient `SOCKET_*` var from `cmd`, except the telemetry +/// opt-outs (an opted-out developer stays opted out), `SOCKET_NO_CONFIG` +/// and `SOCKET_NO_UPDATE_CHECK`. [`command`] runs this; package-manager +/// children call it directly. +pub fn scrub_socket_vars(cmd: &mut Command) { + for (key, _) in std::env::vars_os() { + let name = key.to_string_lossy(); + if name.starts_with("SOCKET_") + && !name.contains("TELEMETRY") + && name != "SOCKET_NO_CONFIG" + && name != "SOCKET_NO_UPDATE_CHECK" + { + cmd.env_remove(&key); + } + } +} + +/// Opt-in scrubs for the ambient config of the tools a suite drives. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Extra { + /// `VIRTUAL_ENV`: an activated venv redirects Python discovery. + Venv, + /// Every `YARN_*` var. Yarn berry lets any `.yarnrc.yml` setting be + /// overridden by env, so an ambient `YARN_NODE_LINKER=pnp` would build a + /// PnP tree and void the node_modules probes. Seeded with that value and + /// then scrubbed. + Yarn, + /// Every `PNPM_*` and `npm_config_*` var (any case). pnpm lets any + /// `.npmrc` setting be overridden by an `npm_config_*` env var (env + /// outranks the project npmrc), so an ambient + /// `npm_config_node_linker=pnp` makes pnpm emit a `.pnp.cjs` that + /// `vendor` refuses. Seeded with that value and then scrubbed. + Pnpm, +} + +/// Apply each of `extras` to `cmd` (see [`Extra`]). Run it before +/// `cache_env::isolate`, which sets some of the same names. +pub fn scrub_extra(cmd: &mut Command, extras: &[Extra]) { + for extra in extras { + match extra { + Extra::Venv => { + cmd.env_remove("VIRTUAL_ENV"); + } + Extra::Yarn => { + cmd.env("YARN_NODE_LINKER", "pnp"); + for (key, _) in std::env::vars_os() { + if key.to_string_lossy().starts_with("YARN_") { + cmd.env_remove(&key); + } + } + cmd.env_remove("YARN_NODE_LINKER"); + } + Extra::Pnpm => { + cmd.env("npm_config_node_linker", "pnp"); + for (key, _) in std::env::vars_os() { + let name = key.to_string_lossy(); + if name.starts_with("PNPM_") + || name.to_ascii_lowercase().starts_with("npm_config_") + { + cmd.env_remove(&key); + } + } + cmd.env_remove("npm_config_node_linker"); + } + } + } +} diff --git a/crates/socket-patch-cli/tests/common/mod.rs b/crates/socket-patch-cli/tests/common/mod.rs index 7199ee0e6..4cbc43a86 100644 --- a/crates/socket-patch-cli/tests/common/mod.rs +++ b/crates/socket-patch-cli/tests/common/mod.rs @@ -27,6 +27,13 @@ use sha2::{Digest, Sha256}; /// `#[path = "common/cache_env.rs"] mod cache_env;`. pub mod cache_env; +/// The hermetic `SOCKET_*` environment every test child is spawned with. +/// Files that don't need the rest of this module pull it in on its own with +/// `#[path = "common/hermetic.rs"] mod hermetic;`. +pub mod hermetic; + +pub use hermetic::command as hermetic_command; + // ── Binary discovery + invocation ───────────────────────────────────── /// Absolute path to the built `socket-patch` binary that cargo @@ -77,64 +84,8 @@ pub fn run_bin_with_env( args: &[&str], env: &[(&str, &str)], ) -> (i32, String, String) { - let mut cmd = Command::new(bin); + let mut cmd = hermetic_command(bin); cmd.args(args).current_dir(cwd); - // The binary binds a wide `SOCKET_*` env surface (SOCKET_CWD, - // SOCKET_DRY_RUN, SOCKET_STRICT, SOCKET_GLOBAL, SOCKET_MANIFEST_PATH, - // ...). An ambient value silently changes what these tests exercise — - // SOCKET_DRY_RUN=true turns every real apply into a no-op, - // SOCKET_GLOBAL_PREFIX flips commands into global mode (aiming - // mutations at the host's *real* global caches), and the output-mode - // trio (SOCKET_JSON / SOCKET_SILENT / SOCKET_VERBOSE) silently flips - // which printer a test's assertions run against. The highest-risk - // vars are seeded with hostile values and then scrubbed — `env_remove` - // clears the seed too, so the child never sees it, but if a scrub line - // is ever dropped the seed (rather than a developer's ambient shell, - // which this suite can't rely on) turns the tests red immediately. - cmd.env("SOCKET_GLOBAL", "true") - .env("SOCKET_GLOBAL_PREFIX", "/nonexistent") - .env("SOCKET_DRY_RUN", "true") - .env("SOCKET_MANIFEST_PATH", "/nonexistent/manifest.json") - .env("SOCKET_JSON", "true") - .env("SOCKET_SILENT", "true") - .env("SOCKET_VERBOSE", "true") - .env("SOCKET_UPDATE_BASE_URL", "http://127.0.0.1:1") - .env("SOCKET_UPDATE_STATE_DIR", "/nonexistent") - .env_remove("SOCKET_GLOBAL") - .env_remove("SOCKET_GLOBAL_PREFIX") - .env_remove("SOCKET_DRY_RUN") - .env_remove("SOCKET_MANIFEST_PATH") - .env_remove("SOCKET_JSON") - .env_remove("SOCKET_SILENT") - .env_remove("SOCKET_VERBOSE") - .env_remove("SOCKET_UPDATE_BASE_URL") - .env_remove("SOCKET_UPDATE_STATE_DIR") - .env_remove("SOCKET_API_TOKEN"); - // Prefix-scrub whatever else the ambient shell carries; removing - // SOCKET_API_TOKEN also forces the public proxy (free-tier). - // Telemetry opt-outs are deliberately kept so an opted-out dev - // stays opted out. - for (key, _) in std::env::vars_os() { - let name = key.to_string_lossy(); - if name.starts_with("SOCKET_") - && !name.contains("TELEMETRY") - && name != "SOCKET_NO_CONFIG" - && name != "SOCKET_NO_UPDATE_CHECK" - { - cmd.env_remove(&key); - } - } - // Belt-and-braces on top of the `.cargo/config.toml` `[env]` default: - // a developer's real `socket login` (the socket-cli config.json token - // fallback) must never authenticate a test child — it would flip every - // "no token → public proxy" assertion onto the authed path. - cmd.env("SOCKET_NO_CONFIG", "1"); - // Same posture for the passive update notifier: no test child may ever - // fetch release metadata from real GitHub. The stderr-TTY guard covers - // piped children, but the PTY suites hand the binary a real terminal — - // this force-set is the layer that holds there. Notifier tests opt back - // in via caller env (which lands last). - cmd.env("SOCKET_NO_UPDATE_CHECK", "1"); // Caller-supplied env lands last so explicit injections (runtime // gates, discovery roots) survive the scrub. for (k, v) in env { diff --git a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs index ecd2686f0..4d65bbf62 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs @@ -72,6 +72,8 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; use vex_e2e_common::{ @@ -149,31 +151,6 @@ fn has_corepack_pm(pm: &str) -> bool { ok } -/// Remove ambient `SOCKET_*` / `PNPM_*` / `npm_config_*` vars. -/// -/// Seed-then-scrub (mirrors e2e_vendor_pnpm_build.rs): pnpm lets EVERY -/// `.npmrc` setting be overridden by an `npm_config_*` env var (env outranks -/// the project npmrc), so an ambient `npm_config_node_linker=pnp` alone can -/// turn a capstone red. The explicit env_remove below clears the seed too, -/// but if the prefix scrub is ever dropped the seed (rather than a -/// developer's ambient shell, which this suite can't rely on) turns the test -/// red immediately. -fn scrub_socket_env(cmd: &mut Command) { - cmd.env("npm_config_node_linker", "pnp"); - for (k, _) in std::env::vars_os() { - let key = k.to_string_lossy(); - if (key.starts_with("SOCKET_") - || key.starts_with("PNPM_") - || key.to_ascii_lowercase().starts_with("npm_config_")) - && key != "SOCKET_NO_CONFIG" - { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("npm_config_node_linker"); -} - fn corepack(cwd: &Path, pm: &str, args: &[&str]) -> Output { let mut cmd = pnpm_command(pm); let legacy = pm @@ -207,7 +184,8 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str]) -> Output { "--fetch-retry-maxtimeout=500", ]); } - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Pnpm]); // After the scrub: it strips ambient `PNPM_*` / `npm_config_*`, which // would otherwise take the sandbox values back out again. cache_env::isolate(&mut cmd); @@ -221,9 +199,9 @@ fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { /// [`run_socket`] with extra env applied after the scrub. fn run_socket_env(cwd: &Path, args: &[&str], env: &[(String, String)]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args(args).current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Pnpm]); for (k, v) in env { cmd.env(k, v); } diff --git a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs b/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs index 85ed473e0..594492eef 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs @@ -33,6 +33,8 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; use vex_e2e_common::{ @@ -90,21 +92,13 @@ fn has_command(cmd: &str) -> bool { .unwrap_or(false) } -fn scrub_socket_env(cmd: &mut Command) { - for (k, _) in std::env::vars_os() { - if k.to_string_lossy().starts_with("SOCKET_") && k.to_string_lossy() != "SOCKET_NO_CONFIG" { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("npm_config_store_dir"); - cmd.env_remove("PNPM_HOME"); -} - fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output { let mut cmd = Command::new("corepack"); cmd.arg(pm).args(args).current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.env_remove("npm_config_store_dir") + .env_remove("PNPM_HOME"); // After the scrub: it strips ambient `PNPM_HOME` / `npm_config_store_dir`, // which would otherwise take the sandbox values back out again. cache_env::isolate(&mut cmd); @@ -116,9 +110,11 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> } fn run_socket(cwd: &Path, args: &[&str]) -> Output { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args(args).current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.env_remove("npm_config_store_dir") + .env_remove("PNPM_HOME"); cmd.output().expect("failed to run socket-patch binary") } @@ -644,7 +640,10 @@ async fn rush_hosted_real_rush_update_install() { full.extend_from_slice(args); let mut cmd = Command::new("npm"); cmd.args(&full).current_dir(root); - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.env_remove("npm_config_store_dir") + .env_remove("PNPM_HOME"); cache_env::isolate(&mut cmd); for (k, _) in std::env::vars_os() { if k.to_string_lossy().starts_with("RUSH_") { diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index ebd939ab1..9f18fb746 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs @@ -74,6 +74,8 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; use vex_e2e_common::{ @@ -191,44 +193,18 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str]) -> Output { cmd.args(&args) .current_dir(cwd) .env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0"); - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Pnpm]); // After the scrub: it strips ambient `PNPM_*` and `npm_config_*`, which // would otherwise take the sandbox values back out again. cache_env::isolate(&mut cmd); cmd.output().expect("failed to run corepack") } -/// Remove ambient `SOCKET_*` / `PNPM_*` / `npm_config_*` vars (the -/// `--store-dir` flag is always passed explicitly). -/// -/// Seed-then-scrub (mirrors e2e_redirect_yarn_berry_build.rs): pnpm lets -/// EVERY `.npmrc` setting be overridden by an `npm_config_*` env var (env -/// outranks the project npmrc), so an ambient `npm_config_node_linker=pnp` -/// was verified to turn the capstone red — pnpm emits a `.pnp.cjs` and -/// `vendor` refuses the project as unsupported Plug'n'Play. The explicit -/// env_remove below clears the seed too, but if the prefix scrub is ever -/// dropped the seed (rather than a developer's ambient shell, which this -/// suite can't rely on) turns the test red immediately. -fn scrub_socket_env(cmd: &mut Command) { - cmd.env("npm_config_node_linker", "pnp"); - for (k, _) in std::env::vars_os() { - let key = k.to_string_lossy(); - if (key.starts_with("SOCKET_") - || key.starts_with("PNPM_") - || key.to_ascii_lowercase().starts_with("npm_config_")) - && key != "SOCKET_NO_CONFIG" - { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("npm_config_node_linker"); -} - fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Pnpm]); let _fixture = prebuilt_common::prepare_command(&mut cmd, cwd, args, &[]); let out = cmd.output().expect("failed to run socket-patch binary"); ( diff --git a/crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_dev_flow.rs b/crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_dev_flow.rs index 631bba379..48f846196 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_dev_flow.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_dev_flow.rs @@ -66,6 +66,8 @@ const CVE: &str = "CVE-2024-99999"; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; #[path = "common/yarn_classic_vex.rs"] @@ -125,7 +127,8 @@ fn binary() -> PathBuf { fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output { let mut cmd = Command::new("corepack"); cmd.arg(pm).args(args).current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); cache_env::isolate(&mut cmd); cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0"); for (k, v) in extra_env { @@ -134,24 +137,12 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> cmd.output().expect("failed to run corepack") } -/// Remove every ambient `SOCKET_*` var (so a developer's `SOCKET_DRY_RUN=1` -/// etc. can't flip behavior) and the PM cache var the harness controls. -fn scrub_socket_env(cmd: &mut Command) { - for (k, _) in std::env::vars_os() { - let k = k.to_string_lossy(); - if k.starts_with("SOCKET_") { - cmd.env_remove(k.as_ref()); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("YARN_CACHE_FOLDER"); -} - /// Run the socket-patch binary with a scrubbed environment. fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.env_remove("YARN_CACHE_FOLDER"); let _fixture = prebuilt_common::prepare_command(&mut cmd, cwd, args, &[]); let out = cmd.output().expect("failed to run socket-patch binary"); ( diff --git a/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs b/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs index df8525dd7..d43f7e2d0 100644 --- a/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs +++ b/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs @@ -50,6 +50,8 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; #[path = "yarn_berry_common/mod.rs"] @@ -108,30 +110,13 @@ fn has_corepack_pm(pm: &str) -> bool { .unwrap_or(false) } -fn scrub_socket_env(cmd: &mut Command) { - // Seed-then-scrub (mirrors e2e_redirect_yarn_berry_build.rs): yarn berry - // lets EVERY `.yarnrc.yml` setting be overridden by a `YARN_*` env var, - // so an ambient `YARN_NODE_LINKER=pnp` would build a PnP tree and - // node_modules/left-pad would never exist. The env_remove below clears - // the seed too, but if the scrub is ever dropped the seed turns these - // tests red immediately rather than relying on a developer's shell. - cmd.env("YARN_NODE_LINKER", "pnp"); - for (k, _) in std::env::vars_os() { - let key = k.to_string_lossy(); - if (key.starts_with("SOCKET_") || key.starts_with("YARN_")) && key != "SOCKET_NO_CONFIG" { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("YARN_NODE_LINKER"); -} - fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output { let mut cmd = yarn_berry_common::corepack_command(); cmd.arg(pm).args(args).current_dir(cwd); // Scrub FIRST (it removes YARN_* / SOCKET_* from the inherited env), then // set the hermetic flags so they survive (Command: last env call wins). - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Yarn]); cache_env::isolate(&mut cmd); yarn_berry_common::pin_berry_ci_defaults(&mut cmd, pm); cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0") @@ -143,9 +128,9 @@ fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> } fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args(args).current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Yarn]); let out = cmd.output().expect("failed to run socket-patch binary"); ( out.status.code().unwrap_or(-1), diff --git a/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs b/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs index a10666cea..ff8e086b0 100644 --- a/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs +++ b/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs @@ -39,6 +39,9 @@ use std::process::Command; use serde_json::Value; use sha2::{Digest, Sha256}; +#[path = "common/hermetic.rs"] +mod hermetic; + const ORIGINAL: &[u8] = b"original\n"; const PATCHED: &[u8] = b"patched\n"; @@ -63,40 +66,6 @@ fn write_root_package_json(root: &Path) { .unwrap(); } -/// Hermeticity scrub for the apply/rollback helpers. The binary binds a wide -/// `SOCKET_*` env surface; an ambient value silently changes the branch under -/// test — `SOCKET_DRY_RUN=true` turns every rollback into a no-op -/// (`rolledBack: 0`, bytes never restored) and `SOCKET_MANIFEST_PATH` points -/// apply at a manifest that isn't there (`noManifest`, exit 0). Both verified -/// red against unscrubbed helpers. Seed-then-scrub: hostile values for the -/// vars that break these tests are set first, then the whole prefix is -/// removed — if the scrub ever stops running, the seeds turn every test in -/// this file red immediately. Telemetry opt-outs are deliberately kept so an -/// opted-out dev stays opted out (`--offline` already disables telemetry). -fn scrub_socket_env(cmd: &mut Command) { - const HOSTILE_SEEDS: &[(&str, &str)] = &[ - ("SOCKET_DRY_RUN", "true"), - ("SOCKET_GLOBAL", "true"), - ("SOCKET_GLOBAL_PREFIX", "/nonexistent"), - ("SOCKET_MANIFEST_PATH", "/nonexistent/manifest.json"), - ]; - for (k, v) in HOSTILE_SEEDS { - cmd.env(k, v); - } - // Explicit removes cover the seeds (they are not in the parent env); - // the vars_os() sweep covers whatever the ambient shell/CI exported. - for (k, _) in HOSTILE_SEEDS { - cmd.env_remove(k); - } - for (key, _) in std::env::vars_os() { - let name = key.to_string_lossy(); - if name.starts_with("SOCKET_") && !name.contains("TELEMETRY") && name != "SOCKET_NO_CONFIG" - { - cmd.env_remove(&key); - } - } -} - /// Write a minimal manifest with one (file-less) patch for the given PURL. fn write_manifest(root: &Path, purl: &str) { let socket = root.join(".socket"); @@ -122,7 +91,7 @@ fn write_manifest(root: &Path, purl: &str) { /// Run `socket-patch apply --offline --json --ecosystems ` and return /// the exit code + parsed envelope. fn run_apply_for_ecosystem(cwd: &Path, ecosystem: &str) -> (i32, Value) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args([ "apply", "--offline", @@ -132,7 +101,6 @@ fn run_apply_for_ecosystem(cwd: &Path, ecosystem: &str) -> (i32, Value) { "--silent", ]) .current_dir(cwd); - scrub_socket_env(&mut cmd); let out = cmd.output().expect("run socket-patch"); let stdout = String::from_utf8_lossy(&out.stdout).to_string(); let env: Value = serde_json::from_str(stdout.trim()) @@ -460,7 +428,7 @@ fn run_rollback( global: bool, envs: &[(String, String)], ) -> (i32, Value) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args([ "rollback", "--offline", @@ -473,10 +441,9 @@ fn run_rollback( cmd.arg("--global"); } cmd.current_dir(cwd); - // Scrub BEFORE seeding fixture envs, so a fixture-supplied - // SOCKET_-prefixed var survives the prefix sweep (last env call - // per key wins). - scrub_socket_env(&mut cmd); + // The hermetic command scrubbed BEFORE the fixture envs land, so a + // fixture-supplied SOCKET_-prefixed var survives the prefix sweep + // (last env call per key wins). for (k, v) in envs { cmd.env(k, v); } @@ -831,27 +798,12 @@ fn rollback_dispatch_branch_composer() { // the prefix verbatim as a node_modules root, so `paths` is never empty. // --------------------------------------------------------------------------- -use socket_patch_cli::args::GLOBAL_ARG_ENV_VARS; - /// Run the binary with a scrubbed SOCKET_* environment so ambient /// developer/CI configuration (tokens, silent/json toggles, vex modes) /// can't change the branch under test. fn run_scrubbed(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.args(args).current_dir(cwd); - for var in GLOBAL_ARG_ENV_VARS { - cmd.env_remove(var); - } - for var in [ - "SOCKET_VEX", - "SOCKET_VEX_OUTPUT", - "SOCKET_VEX_PRODUCT", - "SOCKET_VEX_NO_VERIFY", - "SOCKET_VEX_DOC_ID", - "SOCKET_VEX_COMPACT", - ] { - cmd.env_remove(var); - } cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); let out = cmd.output().expect("run socket-patch"); ( diff --git a/crates/socket-patch-cli/tests/get/get_batch_paths_e2e.rs b/crates/socket-patch-cli/tests/get/get_batch_paths_e2e.rs index 80ea9514c..0c0883851 100644 --- a/crates/socket-patch-cli/tests/get/get_batch_paths_e2e.rs +++ b/crates/socket-patch-cli/tests/get/get_batch_paths_e2e.rs @@ -11,7 +11,6 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::process::Command; use wiremock::matchers::{method, path, path_regex}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -24,28 +23,6 @@ const ORG_SLUG: &str = "test-org"; const UUID_A: &str = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; const UUID_B: &str = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; -/// Scrub the binary's entire `SOCKET_*` env surface (keeping telemetry -/// opt-outs, so an opted-out dev stays opted out) before spawning. These -/// subprocess tests assert an EXACT envelope, so any `#[arg(env=…)]` -/// fallback leaking in from the ambient shell (CI, a dev's `.envrc`, …) -/// can silently redirect the command to a different path (offline mode, a -/// real api-url, …) — or, for value-parsed args like `SOCKET_LOCK_TIMEOUT` -/// (u64) and `SOCKET_VENDOR_SOURCE` (enum), turn every invocation into an -/// exit-2 usage error before `get` even runs. A fixed allowlist here -/// rotted as flags were added (it predated `SOCKET_LOCK_TIMEOUT`, -/// `SOCKET_STRICT`, `SOCKET_VENDOR_*`, `SOCKET_PATCH_SERVER_URL` — -/// ambient `SOCKET_LOCK_TIMEOUT=bogus` failed 6 of these 7 tests); the -/// prefix scrub can't rot. -fn scrub_socket_env(cmd: &mut Command) { - for (key, _) in std::env::vars_os() { - let name = key.to_string_lossy(); - if name.starts_with("SOCKET_") && !name.contains("TELEMETRY") && name != "SOCKET_NO_CONFIG" - { - cmd.env_remove(&key); - } - } -} - /// Run `socket-patch get ` with `--json --save-only --yes` /// against `api_url` (authenticated mode). Returns (code, stdout, stderr). fn run_get_auth( @@ -68,9 +45,8 @@ fn run_get_auth( ORG_SLUG, ]; args.extend_from_slice(extra); - let mut cmd = Command::new(binary()); + let mut cmd = crate::common::hermetic_command(&binary()); cmd.args(&args).current_dir(cwd); - scrub_socket_env(&mut cmd); let out = cmd.output().expect("run socket-patch"); ( out.status.code().unwrap_or(-1), diff --git a/crates/socket-patch-cli/tests/mode_migration_npm.rs b/crates/socket-patch-cli/tests/mode_migration_npm.rs index 7fb28b5f0..5ee80a051 100644 --- a/crates/socket-patch-cli/tests/mode_migration_npm.rs +++ b/crates/socket-patch-cli/tests/mode_migration_npm.rs @@ -34,6 +34,8 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; #[path = "common/cache_env.rs"] mod cache_env; +#[path = "common/hermetic.rs"] +mod hermetic; // yarn legs: release selection (classic) + the manifest-less VEX matrices. #[path = "vex_e2e_common/mod.rs"] mod vex_e2e_common; @@ -81,28 +83,12 @@ fn has_corepack_pm(pm: &str) -> bool { .unwrap_or(false) } -/// Remove ambient `SOCKET_*` (except the hermetic `SOCKET_NO_CONFIG`) and -/// every `YARN_*` var. Seed-then-scrub for `YARN_NODE_LINKER` (mirrors -/// `e2e_redirect_yarn_berry_build.rs`): berry lets any yarnrc setting be -/// overridden by env, so an ambient `YARN_NODE_LINKER=pnp` would silently -/// build a PnP tree and void the node_modules probes. -fn scrub_socket_env(cmd: &mut Command) { - cmd.env("YARN_NODE_LINKER", "pnp"); - for (k, _) in std::env::vars_os() { - let key = k.to_string_lossy(); - if (key.starts_with("SOCKET_") || key.starts_with("YARN_")) && key != "SOCKET_NO_CONFIG" { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env_remove("YARN_NODE_LINKER"); -} - fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output { let mut cmd = Command::new("corepack"); cmd.arg(pm).args(args).current_dir(cwd); // Scrub FIRST, then the hermetic flags, then per-call env (last wins). - scrub_socket_env(&mut cmd); + hermetic::scrub_socket_vars(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Yarn]); cache_env::isolate(&mut cmd); cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0") // No global mirror/cache: the fresh-checkout legs must not be able to @@ -120,9 +106,9 @@ fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { /// [`run_socket`] with extra env applied after the scrub. fn run_socket_env(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); + let mut cmd = hermetic::command(&binary()); cmd.current_dir(cwd); - scrub_socket_env(&mut cmd); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv, hermetic::Extra::Yarn]); for (k, v) in env { cmd.env(k, v); } diff --git a/crates/socket-patch-cli/tests/repair/repair_vendor_e2e.rs b/crates/socket-patch-cli/tests/repair/repair_vendor_e2e.rs index a3effd3e5..88fd159f6 100644 --- a/crates/socket-patch-cli/tests/repair/repair_vendor_e2e.rs +++ b/crates/socket-patch-cli/tests/repair/repair_vendor_e2e.rs @@ -10,7 +10,6 @@ //! byte-for-byte on real `bundle lock` output (bundler 4.0.15). use std::path::{Path, PathBuf}; -use std::process::Command; use sha2::{Digest, Sha256}; use wiremock::matchers::{method, path}; @@ -198,7 +197,7 @@ fn run_cli(root: &Path, mock_uri: &str, argv: &[&str]) -> (i32, String, String) "--org", ORG_SLUG, ]); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(&full) .current_dir(root) .env("SOCKET_TELEMETRY_DISABLED", "1") diff --git a/crates/socket-patch-cli/tests/scan/scan_sync_e2e.rs b/crates/socket-patch-cli/tests/scan/scan_sync_e2e.rs index b5065ebef..697637386 100644 --- a/crates/socket-patch-cli/tests/scan/scan_sync_e2e.rs +++ b/crates/socket-patch-cli/tests/scan/scan_sync_e2e.rs @@ -5,7 +5,6 @@ //! file fixture. use std::path::{Path, PathBuf}; -use std::process::Command; use sha2::{Digest, Sha256}; use wiremock::matchers::{method, path}; @@ -125,7 +124,7 @@ async fn scan_sync_against_clean_project_adds_and_applies_patch() { write_root(tmp.path()); write_npm_package(tmp.path(), "sync-target", "1.0.0", before); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", @@ -358,7 +357,7 @@ async fn scan_apply_with_existing_blob_uses_local_cache() { std::fs::create_dir_all(&blobs).unwrap(); std::fs::write(blobs.join(&after_hash), after).unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", @@ -481,7 +480,7 @@ async fn scan_apply_with_no_patches_emits_empty_apply_object() { write_root(tmp.path()); write_npm_package(tmp.path(), "empty-target", "1.0.0", b"x"); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", @@ -625,7 +624,7 @@ async fn scan_apply_skips_vendored_purl_without_downloading() { ) .unwrap(); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "scan", "--json", diff --git a/crates/socket-patch-cli/tests/self_update_e2e.rs b/crates/socket-patch-cli/tests/self_update_e2e.rs index 61d40e8a4..1a771819a 100644 --- a/crates/socket-patch-cli/tests/self_update_e2e.rs +++ b/crates/socket-patch-cli/tests/self_update_e2e.rs @@ -96,7 +96,7 @@ async fn update_force_swaps_binary_end_to_end() { } // The new binary runs. - let out = std::process::Command::new(&install.bin) + let out = common::hermetic_command(&install.bin) .arg("--version") .output() .expect("spawn updated binary"); diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs new file mode 100644 index 000000000..eded27b9f --- /dev/null +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -0,0 +1,503 @@ +//! Every CLI test child is spawned with the one hermetic environment in +//! `common/hermetic.rs` (#823). +//! +//! Two halves: +//! +//! * the shared builder's contract, checked on the inputs where the per-file +//! `scrub_socket_env` copies it replaced used to differ (whether +//! `SOCKET_NO_CONFIG` survived, whether `SOCKET_NO_UPDATE_CHECK` was +//! forced, which package-manager vars were swept, case of `npm_config_*`); +//! * a ratchet over the test tree: no new private `scrub_socket_env`, and no +//! new file that spawns the binary with a bare `Command::new`. Both lists +//! below only shrink — move a file onto `hermetic::command` / +//! `common::run*` and delete its entry. + +#[path = "common/hermetic.rs"] +mod hermetic; + +use std::collections::BTreeMap; +use std::ffi::OsString; +use std::path::Path; +use std::process::Command; + +use hermetic::Extra; +use serial_test::serial; + +/// Files that still carry a private `scrub_socket_env`. Each one is changed +/// by an open fix PR; migrate it once that lands. +const PENDING_SCRUB_COPIES: &[&str] = &[ + "e2e_redirect_yarn_berry_build.rs", + "e2e_redirect_yarn_classic_build.rs", + "e2e_vendor_yarn_berry_build.rs", + "e2e_vendor_yarn_classic_build.rs", + "e2e_yarn4_pnpm_linker_build.rs", + "e2e_yarn4_workspaces_build.rs", + "scan/covgap_ecosystem_dispatch.rs", +]; + +/// Files that still spawn the binary with a bare `Command::new`, so their +/// children see whatever `SOCKET_*` the ambient shell exports unless the file +/// scrubs by hand. +const PENDING_RAW_SPAWNS: &[&str] = &[ + "apply/apply_invariants.rs", + "apply/apply_network.rs", + "apply/cli_gem_variant_mismatch_policy.rs", + "apply/covgap_commands_apply.rs", + "apply/in_process_gem_config_warning.rs", + "apply/in_process_gem_fallback_home.rs", + "apply/in_process_gem_multicopy.rs", + "apply/in_process_npm_multicopy.rs", + "apply/in_process_variant_apply_failure.rs", + "cli/cli_dry_run_paths_e2e.rs", + "cli/covgap_api_client.rs", + "cli/covgap_commands_list.rs", + "cli/telemetry_e2e.rs", + "cli_apply_silent.rs", + "cli_argv_non_utf8.rs", + "cli_config_fallback.rs", + "cli_get_silent.rs", + "cli_global_args.rs", + "cli_parse_list.rs", + "cli_remove_silent.rs", + "cli_scan_silent.rs", + "cli_sigpipe.rs", + "coverage_fix_apply_silent_mute_exit.rs", + "coverage_fix_scan_hosted_dryrun_vendored.rs", + "coverage_fix_vendor_silent_mute_exit.rs", + "covgap_commands_rollback.rs", + "covgap_commands_scan_mod.rs", + "covgap_commands_vendor.rs", + "covgap_commands_vex.rs", + "covgap_utils_socket_cli_config.rs", + "diff_created_file_e2e.rs", + "e2e_cargo.rs", + "e2e_composer.rs", + "e2e_composer_version_identity.rs", + "e2e_embedded_vex.rs", + "e2e_gem.rs", + "e2e_golang.rs", + "e2e_golang_build.rs", + "e2e_golang_workspace_build.rs", + "e2e_hosted_production.rs", + "e2e_maven.rs", + "e2e_npm.rs", + "e2e_nuget.rs", + "e2e_nuget_dotnet_build.rs", + "e2e_pypi.rs", + "e2e_pypi_multi_copy.rs", + "e2e_redirect_bun_build.rs", + "e2e_redirect_cargo_build.rs", + "e2e_redirect_cargo_shapes.rs", + "e2e_redirect_composer_build.rs", + "e2e_redirect_gem_build.rs", + "e2e_redirect_maven_build.rs", + "e2e_redirect_npm_build.rs", + "e2e_redirect_yarn_berry_build.rs", + "e2e_redirect_yarn_classic_build.rs", + "e2e_scan.rs", + "e2e_socket_yml_policy.rs", + "e2e_vendor_bun_build.rs", + "e2e_vendor_cargo_build.rs", + "e2e_vendor_composer_build.rs", + "e2e_vendor_composer_crlf.rs", + "e2e_vendor_gem_build.rs", + "e2e_vendor_golang_build.rs", + "e2e_vendor_jvm_build.rs", + "e2e_vendor_maven_build.rs", + "e2e_vendor_npm_build.rs", + "e2e_vendor_pypi_build.rs", + "e2e_vendor_yarn_berry_build.rs", + "e2e_vendor_yarn_classic_build.rs", + "e2e_vendored_production.rs", + "e2e_vex.rs", + "e2e_vex_build/deno.rs", + "e2e_vex_build/hatch.rs", + "e2e_vex_build/poetry.rs", + "e2e_vex_lockfile/bun.rs", + "e2e_vex_lockfile/cargo.rs", + "e2e_vex_lockfile/golang.rs", + "e2e_vex_lockfile/maven.rs", + "e2e_vex_lockfile/npm.rs", + "e2e_vex_lockfile/nuget.rs", + "e2e_vex_lockfile/pnpm.rs", + "e2e_vex_lockfile/poetry.rs", + "e2e_vex_lockfile/uv.rs", + "e2e_vex_lockfile/yarn.rs", + "e2e_vex_redirect.rs", + "e2e_vex_vendor.rs", + "e2e_yarn4_pnpm_linker_build.rs", + "e2e_yarn4_workspaces_build.rs", + "get/get_edge_cases_e2e.rs", + "get/global_packages_e2e.rs", + "hosted_memory_common/mod.rs", + "hosted_memory_engine.rs", + "hosted_superseding_pypi.rs", + "in_process_redirect.rs", + "in_process_redirect_pdm.rs", + "in_process_redirect_pnpm.rs", + "in_process_redirect_poetry.rs", + "in_process_rollback_hosted.rs", + "in_process_rollback_vendored.rs", + "in_process_vendor.rs", + "in_process_vendor_bun_takeover.rs", + "in_process_vendor_npm_v1_takeover.rs", + "mode_migration_bun.rs", + "mode_migration_cargo.rs", + "mode_migration_pypi.rs", + "remove/remove_network.rs", + "remove_rollback_api_overrides.rs", + "repair/coverage_fix_repair_vendor_predelete.rs", + "repair/covgap_commands_repair.rs", + "repair/covgap_commands_repair_vendor.rs", + "repair/repair_invariants.rs", + "rollback/cli_rollback_silent.rs", + "rollback/rollback_duality_invariants.rs", + "rollback/rollback_invariants.rs", + "rollback/rollback_multicopy_blob_gate.rs", + "scan/coverage_fix_scan_discovery_corrupt_ledger.rs", + "scan/covgap_commands_fetch_stage.rs", + "scan/covgap_commands_scan_vendor_flow.rs", + "scan/covgap_ecosystem_dispatch.rs", + "scan/hosted_management_refusals.rs", + "scan/scan_batch_sizing_e2e.rs", + "scan/scan_ecosystems_scope_e2e.rs", + "scan/scan_invariants.rs", + "scan/scan_ordered_concurrency_e2e.rs", + "scan/scan_paths_e2e.rs", + "scan/scan_vendor_step_error_e2e.rs", + "scan_api_retry_e2e.rs", + "scan_pnpm_relocated_store_cwd_e2e.rs", + "scan_requirements_lock_only.rs", + "scan_rollout_e2e.rs", + "scan_vendor_e2e.rs", + "scan_vendor_requirements_unwired.rs", + "vendor/e2e_golang_redirect.rs", + "vendor/in_process_vendor_bun.rs", + "vendor/vendor_gem_lockfile_only_e2e.rs", + "vendor/vendor_rerun_no_network_e2e.rs", + "vendor_crash_safety_e2e.rs", + "vendor_eject.rs", + "vendor_eject_bun_lockb.rs", + "vendor_eject_fresh_checkout.rs", + "vendor_jvm_cli.rs", + "vendor_partial_staging_e2e.rs", + "vex_e2e_common/uv.rs", + "vex_pdm_hatch_common/mod.rs", + "vex_pipenv_pip_common/mod.rs", + "vex_terminal_output.rs", + "vlt_e2e_common/mod.rs", + "vlt_hosted_common/mod.rs", + "vlt_vendor_common/mod.rs", +]; + +/// The environment `cmd`'s child would see: the parent environment with +/// `cmd`'s explicit sets and removals applied. +fn effective_env(cmd: &Command) -> BTreeMap { + let mut env: BTreeMap = std::env::vars_os().collect(); + for (k, v) in cmd.get_envs() { + match v { + Some(v) => { + env.insert(k.to_os_string(), v.to_os_string()); + } + None => { + env.remove(k); + } + } + } + env +} + +fn get<'a>(env: &'a BTreeMap, key: &str) -> Option<&'a str> { + env.get(&OsString::from(key)).and_then(|v| v.to_str()) +} + +/// Set `vars` in this process for the duration of `f`, then restore them. +/// Callers are `#[serial]`. +fn with_ambient(vars: &[(&str, &str)], f: impl FnOnce()) { + let saved: Vec<(String, Option)> = vars + .iter() + .map(|(k, _)| (k.to_string(), std::env::var_os(k))) + .collect(); + for (k, v) in vars { + std::env::set_var(k, v); + } + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)); + for (k, v) in saved { + match v { + Some(v) => std::env::set_var(&k, v), + None => std::env::remove_var(&k), + } + } + if let Err(e) = result { + std::panic::resume_unwind(e); + } +} + +#[test] +#[serial] +fn command_scrubs_ambient_socket_vars_and_forces_the_opt_outs() { + with_ambient( + &[ + ("SOCKET_DRY_RUN", "true"), + ("SOCKET_OFFLINE", "true"), + ("SOCKET_ECOSYSTEMS", "pypi"), + ("SOCKET_API_TOKEN", "ambient-token"), + ("SOCKET_LOCK_TIMEOUT", "bogus"), + ("SOCKET_VEX_OUTPUT", "/tmp/x"), + // One copy kept these, one removed them; the shared builder + // forces both on. + ("SOCKET_NO_CONFIG", "0"), + ("SOCKET_NO_UPDATE_CHECK", "0"), + ("SOCKET_TELEMETRY_DISABLED", "1"), + ], + || { + let env = effective_env(&hermetic::command(Path::new("socket-patch"))); + for gone in [ + "SOCKET_DRY_RUN", + "SOCKET_OFFLINE", + "SOCKET_ECOSYSTEMS", + "SOCKET_API_TOKEN", + "SOCKET_LOCK_TIMEOUT", + "SOCKET_VEX_OUTPUT", + ] { + assert_eq!(get(&env, gone), None, "{gone} must be scrubbed"); + } + assert_eq!(get(&env, "SOCKET_NO_CONFIG"), Some("1")); + assert_eq!(get(&env, "SOCKET_NO_UPDATE_CHECK"), Some("1")); + assert_eq!( + get(&env, "SOCKET_TELEMETRY_DISABLED"), + Some("1"), + "telemetry opt-outs survive" + ); + }, + ); +} + +#[test] +#[serial] +fn command_never_leaks_its_hostile_seeds() { + let env = effective_env(&hermetic::command(Path::new("socket-patch"))); + for key in [ + "SOCKET_GLOBAL", + "SOCKET_GLOBAL_PREFIX", + "SOCKET_DRY_RUN", + "SOCKET_MANIFEST_PATH", + "SOCKET_JSON", + "SOCKET_SILENT", + "SOCKET_VERBOSE", + "SOCKET_UPDATE_BASE_URL", + "SOCKET_UPDATE_STATE_DIR", + ] { + assert_eq!(get(&env, key), None, "{key} seed must be scrubbed"); + } +} + +#[test] +#[serial] +fn caller_env_set_after_command_survives_the_scrub() { + with_ambient(&[("SOCKET_API_URL", "http://ambient.invalid")], || { + let mut cmd = hermetic::command(Path::new("socket-patch")); + cmd.env("SOCKET_API_URL", "http://127.0.0.1:9") + .env("SOCKET_NO_UPDATE_CHECK", "0"); + let env = effective_env(&cmd); + assert_eq!(get(&env, "SOCKET_API_URL"), Some("http://127.0.0.1:9")); + assert_eq!(get(&env, "SOCKET_NO_UPDATE_CHECK"), Some("0")); + }); +} + +#[test] +#[serial] +fn scrub_socket_vars_is_the_package_manager_half() { + with_ambient( + &[ + ("SOCKET_DRY_RUN", "true"), + ("SOCKET_NO_CONFIG", "1"), + ("SOCKET_TELEMETRY_DISABLED", "1"), + ("YARN_ENABLE_GLOBAL_CACHE", "true"), + ], + || { + let mut cmd = Command::new("yarn"); + hermetic::scrub_socket_vars(&mut cmd); + let env = effective_env(&cmd); + assert_eq!(get(&env, "SOCKET_DRY_RUN"), None); + assert_eq!(get(&env, "SOCKET_NO_CONFIG"), Some("1")); + assert_eq!(get(&env, "SOCKET_TELEMETRY_DISABLED"), Some("1")); + assert_eq!( + get(&env, "YARN_ENABLE_GLOBAL_CACHE"), + Some("true"), + "package-manager vars are opt-in extras" + ); + }, + ); +} + +#[test] +#[serial] +fn extra_scrubs_sweep_each_tools_ambient_config() { + with_ambient( + &[ + ("VIRTUAL_ENV", "/ambient/venv"), + ("YARN_NODE_LINKER", "pnp"), + ("YARN_CACHE_FOLDER", "/ambient/yarn"), + ("PNPM_HOME", "/ambient/pnpm"), + ("npm_config_node_linker", "pnp"), + ("NPM_CONFIG_STORE_DIR", "/ambient/store"), + ], + || { + let mut venv = Command::new("tool"); + hermetic::scrub_extra(&mut venv, &[Extra::Venv]); + let env = effective_env(&venv); + assert_eq!(get(&env, "VIRTUAL_ENV"), None); + assert_eq!(get(&env, "YARN_NODE_LINKER"), Some("pnp")); + + let mut yarn = Command::new("yarn"); + hermetic::scrub_extra(&mut yarn, &[Extra::Yarn]); + let env = effective_env(&yarn); + assert_eq!(get(&env, "YARN_NODE_LINKER"), None); + assert_eq!(get(&env, "YARN_CACHE_FOLDER"), None); + assert_eq!(get(&env, "PNPM_HOME"), Some("/ambient/pnpm")); + + let mut pnpm = Command::new("pnpm"); + hermetic::scrub_extra(&mut pnpm, &[Extra::Pnpm]); + let env = effective_env(&pnpm); + assert_eq!(get(&env, "PNPM_HOME"), None); + assert_eq!(get(&env, "npm_config_node_linker"), None); + assert_eq!(get(&env, "NPM_CONFIG_STORE_DIR"), None, "any case"); + assert_eq!(get(&env, "VIRTUAL_ENV"), Some("/ambient/venv")); + }, + ); +} + +#[test] +#[serial] +fn extra_seeds_are_scrubbed_whatever_the_ambient_value() { + let mut cmd = Command::new("tool"); + hermetic::scrub_extra(&mut cmd, &[Extra::Yarn, Extra::Pnpm]); + let env = effective_env(&cmd); + assert_eq!(get(&env, "YARN_NODE_LINKER"), None); + assert_eq!(get(&env, "npm_config_node_linker"), None); +} + +/// Every `.rs` file under `tests/`, as `/`-separated paths relative to it. +fn test_sources() -> Vec<(String, String)> { + fn walk(dir: &Path, root: &Path, out: &mut Vec<(String, String)>) { + let mut entries: Vec<_> = std::fs::read_dir(dir) + .unwrap_or_else(|e| panic!("read {}: {e}", dir.display())) + .map(|e| e.unwrap().path()) + .collect(); + entries.sort(); + for path in entries { + if path.is_dir() { + walk(&path, root, out); + } else if path.extension().is_some_and(|e| e == "rs") { + let rel = path + .strip_prefix(root) + .unwrap() + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect::>() + .join("/"); + let text = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + out.push((rel, text)); + } + } + } + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests"); + let mut out = Vec::new(); + walk(&root, &root, &mut out); + out +} + +/// Whether `text` spawns the `socket-patch` binary with a bare +/// `Command::new`: `binary()` (any path prefix), the `CARGO_BIN_EXE_*` +/// literal, `socket_bin()` or a `BINARY` const. +fn has_raw_spawn(text: &str) -> bool { + text.split("Command::new(").skip(1).any(|rest| { + let arg = rest.trim_start_matches(|c: char| c.is_alphanumeric() || c == '_' || c == ':'); + let head = &rest[..rest.len() - arg.len()]; + let name = head.rsplit("::").next().unwrap_or(head); + (name == "binary" || name == "socket_bin") && arg.starts_with("()") + || head == "BINARY" && arg.starts_with(')') + || rest.starts_with("env!(\"CARGO_BIN_EXE_socket-patch\")") + }) +} + +#[test] +fn raw_spawn_detector_matches_the_spellings_in_the_tree() { + for spawn in [ + "Command::new(binary())", + "std::process::Command::new(common::binary())", + "Command::new(vex_e2e_common::binary())", + "Command::new(env!(\"CARGO_BIN_EXE_socket-patch\"))", + "Command::new(socket_bin())", + "Command::new(BINARY)", + ] { + assert!(has_raw_spawn(spawn), "{spawn}"); + } + for ok in [ + "hermetic::command(&binary())", + "Command::new(\"npm\")", + "Command::new(bin)", + "Command::new(binary_path)", + "Command::new(BINARY_DIR)", + ] { + assert!(!has_raw_spawn(ok), "{ok}"); + } +} + +#[test] +fn no_new_private_scrub_socket_env_copies() { + let found: Vec = test_sources() + .into_iter() + .filter(|(rel, text)| { + rel != "spawn_env_hygiene.rs" && text.contains("fn scrub_socket_env(") + }) + .map(|(rel, _)| rel) + .collect(); + let unexpected: Vec<&String> = found + .iter() + .filter(|f| !PENDING_SCRUB_COPIES.contains(&f.as_str())) + .collect(); + assert!( + unexpected.is_empty(), + "spawn through `common/hermetic.rs` (hermetic::command + scrub_extra) \ + instead of a private scrub_socket_env: {unexpected:?}" + ); + let stale: Vec<&&str> = PENDING_SCRUB_COPIES + .iter() + .filter(|f| !found.iter().any(|g| g == *f)) + .collect(); + assert!( + stale.is_empty(), + "these files no longer carry a scrub_socket_env copy; drop them from \ + PENDING_SCRUB_COPIES: {stale:?}" + ); +} + +#[test] +fn no_new_bare_binary_spawns() { + let found: Vec = test_sources() + .into_iter() + .filter(|(rel, text)| rel != "spawn_env_hygiene.rs" && has_raw_spawn(text)) + .map(|(rel, _)| rel) + .collect(); + let unexpected: Vec<&String> = found + .iter() + .filter(|f| !PENDING_RAW_SPAWNS.contains(&f.as_str())) + .collect(); + assert!( + unexpected.is_empty(), + "spawn the binary through `hermetic::command` / `common::run*`, not a \ + bare Command::new: {unexpected:?}" + ); + let stale: Vec<&&str> = PENDING_RAW_SPAWNS + .iter() + .filter(|f| !found.iter().any(|g| g == *f)) + .collect(); + assert!( + stale.is_empty(), + "these files no longer spawn the binary bare; drop them from \ + PENDING_RAW_SPAWNS: {stale:?}" + ); +} diff --git a/crates/socket-patch-cli/tests/update/covgap_update_swap.rs b/crates/socket-patch-cli/tests/update/covgap_update_swap.rs index bb9c4f8b6..cefbedf94 100644 --- a/crates/socket-patch-cli/tests/update/covgap_update_swap.rs +++ b/crates/socket-patch-cli/tests/update/covgap_update_swap.rs @@ -107,7 +107,7 @@ async fn update_without_resolvable_state_dir_proceeds_unlocked() { "unlocked swap must still be a rename, not an overwrite" ); } - let out = std::process::Command::new(&install.bin) + let out = crate::common::hermetic_command(&install.bin) .arg("--version") .output() .expect("spawn updated binary"); From 5ad0465c86d054ae4e6cb36807824de4defdb1b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:21:05 +0000 Subject: [PATCH 3/5] Drop imports the hermetic move left unused Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_yarn_legacy_cachekey_refusal_build.rs | 2 +- crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs b/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs index d43f7e2d0..5ec77dd37 100644 --- a/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs +++ b/crates/socket-patch-cli/tests/e2e_yarn_legacy_cachekey_refusal_build.rs @@ -41,7 +41,7 @@ //! install cannot reach the registry; every assertion after that is HARD. use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Output, Stdio}; use base64::Engine as _; use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; diff --git a/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs b/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs index ff8e086b0..84d93c194 100644 --- a/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs +++ b/crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs @@ -34,7 +34,6 @@ //! assertions fail loudly. use std::path::{Path, PathBuf}; -use std::process::Command; use serde_json::Value; use sha2::{Digest, Sha256}; From 4384385292a52f04e08844987750a3d164da562f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:25:30 +0000 Subject: [PATCH 4/5] Spawn cli_dry_run_paths through the hermetic builder Ambient SOCKET_DRY_RUN failed the real-apply leg of apply_dry_run_with_real_patch_verifies_without_mutating; the cli target now gives the same result with or without it. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/cli/cli_dry_run_paths_e2e.rs | 41 +++++-------------- .../tests/spawn_env_hygiene.rs | 1 - 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/crates/socket-patch-cli/tests/cli/cli_dry_run_paths_e2e.rs b/crates/socket-patch-cli/tests/cli/cli_dry_run_paths_e2e.rs index b5b91d44d..2b7d23c0e 100644 --- a/crates/socket-patch-cli/tests/cli/cli_dry_run_paths_e2e.rs +++ b/crates/socket-patch-cli/tests/cli/cli_dry_run_paths_e2e.rs @@ -4,7 +4,6 @@ //! dry-run flag-propagation branches each command's `run` has. use std::path::{Path, PathBuf}; -use std::process::Command; use sha2::{Digest, Sha256}; @@ -95,11 +94,9 @@ fn make_applicable_npm_patch(root: &Path) { fn apply_dry_run_empty_manifest_emits_dry_run_envelope() { let tmp = tempfile::tempdir().expect("tempdir"); make_socket_with_empty_manifest(tmp.path()); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["apply", "--json", "--dry-run"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -165,11 +162,9 @@ fn apply_dry_run_with_real_patch_verifies_without_mutating() { ); // ---- DRY RUN ---- - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["apply", "--json", "--dry-run", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply --dry-run"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -249,11 +244,9 @@ fn apply_dry_run_with_real_patch_verifies_without_mutating() { // This guarantees the dry-run assertions above are non-vacuous: the // patch really is applicable, so "nothing changed" under --dry-run is a // meaningful result rather than an artifact of an inapplicable fixture. - let out2 = Command::new(binary()) + let out2 = crate::common::hermetic_command(&binary()) .args(["apply", "--json", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply (real)"); let stdout2 = String::from_utf8_lossy(&out2.stdout); @@ -335,11 +328,9 @@ fn apply_dry_run_human_count_excludes_vendored() { // Prove the fixture is non-vacuous first: in JSON mode the vendored // entry must classify as skipped/vendored (if the vendor ledger were // unreadable it would fail open and this test would assert nothing). - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["apply", "--json", "--dry-run", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply --json --dry-run"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -366,11 +357,9 @@ fn apply_dry_run_human_count_excludes_vendored() { // The human summary must agree with that classification: only the // genuinely applicable package counts as patchable. - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["apply", "--dry-run", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply --dry-run"); assert_eq!(out.status.code(), Some(0)); @@ -387,11 +376,9 @@ fn apply_dry_run_human_count_excludes_vendored() { fn repair_dry_run_offline_emits_dry_run_envelope() { let tmp = tempfile::tempdir().expect("tempdir"); make_socket_with_empty_manifest(tmp.path()); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["repair", "--json", "--dry-run", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run repair"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -415,11 +402,9 @@ fn repair_dry_run_offline_emits_dry_run_envelope() { fn rollback_with_empty_manifest_emits_envelope() { let tmp = tempfile::tempdir().expect("tempdir"); make_socket_with_empty_manifest(tmp.path()); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["rollback", "--json", "--offline"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run rollback"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -449,7 +434,7 @@ fn rollback_with_empty_manifest_emits_envelope() { fn remove_with_no_socket_dir_emits_manifest_not_found() { let tmp = tempfile::tempdir().expect("tempdir"); // NO .socket/ directory at all. - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args([ "remove", "11111111-1111-4111-8111-111111111111", @@ -458,8 +443,6 @@ fn remove_with_no_socket_dir_emits_manifest_not_found() { "--skip-rollback", ]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run remove"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -483,11 +466,9 @@ fn remove_with_no_socket_dir_emits_manifest_not_found() { fn list_with_empty_manifest_emits_empty_envelope() { let tmp = tempfile::tempdir().expect("tempdir"); make_socket_with_empty_manifest(tmp.path()); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["list", "--json"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run list"); let stdout = String::from_utf8_lossy(&out.stdout); @@ -511,11 +492,9 @@ fn list_with_empty_manifest_emits_empty_envelope() { #[test] fn apply_silent_no_manifest_produces_no_output() { let tmp = tempfile::tempdir().expect("tempdir"); - let out = Command::new(binary()) + let out = crate::common::hermetic_command(&binary()) .args(["apply", "--silent"]) .current_dir(tmp.path()) - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_CLI_API_TOKEN") .output() .expect("run apply"); assert_eq!(out.status.code(), Some(0)); diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index eded27b9f..057700a3b 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -48,7 +48,6 @@ const PENDING_RAW_SPAWNS: &[&str] = &[ "apply/in_process_gem_multicopy.rs", "apply/in_process_npm_multicopy.rs", "apply/in_process_variant_apply_failure.rs", - "cli/cli_dry_run_paths_e2e.rs", "cli/covgap_api_client.rs", "cli/covgap_commands_list.rs", "cli/telemetry_e2e.rs", From efa5cdea8ccd780112d888517f8bbec05d63a378 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:32:04 +0000 Subject: [PATCH 5/5] Port #851 vex alias test fix from main breakage main @ 4646693 (#605) broke two vex_consumed alias tests; the coverage job fails on every PR. Same change as #851, so it no-ops once that lands. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/vex_consumed.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vex_consumed.rs b/crates/socket-patch-cli/src/commands/vex_consumed.rs index b57d475fb..cb0c68023 100644 --- a/crates/socket-patch-cli/src/commands/vex_consumed.rs +++ b/crates/socket-patch-cli/src/commands/vex_consumed.rs @@ -715,8 +715,11 @@ mod tests { None, ) .await; - assert_eq!(installed_again, installed); - let (paths, calls) = tracked_npm_hosted(&common, &installed_again).await; + // Since #605 the name-keyed resolver probes bundled trees itself, so + // it already returns the aliases and the nested store's peers. Feed + // the earlier, alias-free set to keep exercising alias expansion; + // the resolver's own set is checked against the same result below. + let (paths, calls) = tracked_npm_hosted(&common, &installed).await; assert_eq!(calls.len(), 1); let mut inputs = calls[0].clone(); inputs.sort(); @@ -738,6 +741,9 @@ mod tests { .len(), paths.len() ); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed_again).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)] @@ -768,14 +774,19 @@ mod tests { None, ) .await; - assert!(installed.is_empty(), "{installed:?}"); - let (mut paths, calls) = tracked_npm_hosted(&common, &installed).await; + // Since #605 the name-keyed resolver reaches the alias and its + // sibling peers on its own. An alias-only set (what an alias-blind + // resolver returns) must still expand to the same copies. + let (mut paths, calls) = tracked_npm_hosted(&common, &HashMap::new()).await; assert_eq!(calls, vec![vec![alias.clone()]]); let mut expected = peers; expected.push(alias); paths.sort(); expected.sort(); assert_eq!(paths, expected); + let (mut resolved, _) = tracked_npm_hosted(&common, &installed).await; + resolved.sort(); + assert_eq!(resolved, expected, "the resolver's own copy set"); } #[cfg(unix)]