From be93406a2d12d208f48f2d8aa827e84f9a68c906 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 08:52:52 +0000 Subject: [PATCH 1/3] Start fix for #823 (production suites) Assisted-by: Claude Code:claude-opus-5-5 From e362b7e3e2f19a19b940e9bfc60c9b957cf592d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:07:33 +0000 Subject: [PATCH 2/3] CI: hermetic spawns in production e2e suites The hosted and vendored production suites started socket-patch with a bare Command::new and their own SOCKET_* sweeps. The vex legs forced neither the update check nor telemetry off, and the hosted run let an activated venv reach the unscoped scan. All four spawns now go through the shared hermetic builder with the venv scrubbed. Their private hostile seeds (API token, API and proxy URLs, offline) move into the shared HOSTILE_SEEDS. The builder already removed those names, so no other suite's child sees a different environment. With these two files migrated, PENDING_RAW_SPAWNS is empty and goes away: the ratchet now rejects any bare binary spawn in the test tree. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/common/hermetic.rs | 7 +++ .../tests/e2e_hosted_production.rs | 41 +++++------------- .../tests/e2e_vendored_production.rs | 43 +++++-------------- .../tests/spawn_env_hygiene.rs | 34 +++------------ 4 files changed, 33 insertions(+), 92 deletions(-) diff --git a/crates/socket-patch-cli/tests/common/hermetic.rs b/crates/socket-patch-cli/tests/common/hermetic.rs index 62e3e8246..167ecae5e 100644 --- a/crates/socket-patch-cli/tests/common/hermetic.rs +++ b/crates/socket-patch-cli/tests/common/hermetic.rs @@ -47,6 +47,13 @@ pub const HOSTILE_SEEDS: &[(&str, &str)] = &[ ("SOCKET_VERBOSE", "true"), ("SOCKET_UPDATE_BASE_URL", "http://127.0.0.1:1"), ("SOCKET_UPDATE_STATE_DIR", "/nonexistent"), + // An ambient token or API/proxy URL would move a child off the public + // proxy, or point the production suites at staging while every + // assertion still passed. + ("SOCKET_API_TOKEN", "hostile-seed-must-be-scrubbed"), + ("SOCKET_API_URL", "http://127.0.0.1:1/hostile"), + ("SOCKET_PROXY_URL", "http://127.0.0.1:1/hostile"), + ("SOCKET_OFFLINE", "true"), ]; /// The opt-outs [`command`] forces on in every child: the config layer diff --git a/crates/socket-patch-cli/tests/e2e_hosted_production.rs b/crates/socket-patch-cli/tests/e2e_hosted_production.rs index 6357650a2..ecc3b88ed 100644 --- a/crates/socket-patch-cli/tests/e2e_hosted_production.rs +++ b/crates/socket-patch-cli/tests/e2e_hosted_production.rs @@ -86,13 +86,11 @@ mod rollback_json; #[path = "common/mod.rs"] mod common; -use common::binary; +use common::{binary, hermetic}; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; -use socket_patch_cli::args::{GLOBAL_ARG_ENV_VARS, LOCAL_ARG_ENV_VARS}; - #[path = "npm_e2e_common/manifestless.rs"] mod npm_e2e_common; #[path = "vex_e2e_common/mod.rs"] @@ -302,25 +300,16 @@ fn has_command(cmd: &str) -> bool { /// `SOCKET_PROXY_URL` or `SOCKET_API_URL` would silently point the run at /// staging — and every assertion below would still pass, proving nothing about /// production. An ambient `SOCKET_API_TOKEN` would move the run off the free -/// public proxy that this suite exists to cover. The hostile seeds below are -/// removed by the same loop that removes the real ones, so if the scrub is -/// ever dropped the seeds turn the suite red immediately instead of letting a -/// developer's ambient shell decide what got tested. +/// public proxy that this suite exists to cover. The shared builder +/// (`hermetic::binary_command`) seeds each of those with a hostile value and +/// removes it, so if the scrub is ever dropped the seeds turn the suite red +/// immediately instead of letting a developer's ambient shell decide what got +/// tested. An activated venv is scrubbed too: the hosted scan discovers +/// packages unscoped under the cwd. fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); - cmd.args(args) - .current_dir(cwd) - .env("SOCKET_GLOBAL", "true") - .env("SOCKET_GLOBAL_PREFIX", "/nonexistent") - .env("SOCKET_DRY_RUN", "true") - .env("SOCKET_SAVE_ONLY", "true") - .env("SOCKET_OFFLINE", "true") - .env("SOCKET_API_TOKEN", "hostile-seed-must-be-scrubbed") - .env("SOCKET_PROXY_URL", "http://127.0.0.1:1/hostile") - .env("SOCKET_MANIFEST_PATH", "/nonexistent/manifest.json"); - for var in GLOBAL_ARG_ENV_VARS.iter().chain(LOCAL_ARG_ENV_VARS) { - cmd.env_remove(var); - } + let mut cmd = hermetic::binary_command(); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.args(args).current_dir(cwd); let out: Output = cmd.output().expect("failed to execute socket-patch binary"); ( out.status.code().unwrap_or(-1), @@ -1736,15 +1725,7 @@ fn bun_manifestless_vex_production( let vex = |extra: &[&str]| -> (Option, serde_json::Value, Option) { let out_path = checkout.join("out.vex.json"); let _ = std::fs::remove_file(&out_path); - let mut cmd = Command::new(binary()); - for (key, _) in std::env::vars_os() { - if key.to_string_lossy().starts_with("SOCKET_") { - cmd.env_remove(key); - } - } - let out = cmd - .env("SOCKET_TELEMETRY_DISABLED", "1") - .env("SOCKET_NO_CONFIG", "1") + let out = hermetic::binary_command() .env("SOCKET_NO_API_TOKEN", "1") .current_dir(checkout) .args(["vex", "--json", "--output"]) diff --git a/crates/socket-patch-cli/tests/e2e_vendored_production.rs b/crates/socket-patch-cli/tests/e2e_vendored_production.rs index 25d865281..d69941c86 100644 --- a/crates/socket-patch-cli/tests/e2e_vendored_production.rs +++ b/crates/socket-patch-cli/tests/e2e_vendored_production.rs @@ -102,7 +102,7 @@ #[path = "common/mod.rs"] mod common; -use common::binary; +use common::{binary, hermetic}; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; @@ -323,30 +323,15 @@ fn has_corepack_pm(pm: &str) -> bool { /// The scrub is load-bearing. An ambient `SOCKET_PROXY_URL`/`SOCKET_API_URL` /// would silently point the run at staging; an ambient `SOCKET_API_TOKEN` (or a /// socket-cli login on disk) would move it off the free public proxy this suite -/// exists to cover. Every ambient `SOCKET_*` is removed (except the config -/// kill-switch), hostile seeds are planted and then removed on the adjacent -/// line so a dropped scrub reddens the suite instead of leaking a developer's -/// shell, and `SOCKET_NO_CONFIG=true` blocks the socket-cli config.json. +/// exists to cover. The shared builder (`hermetic::binary_command`) removes +/// every ambient `SOCKET_*`, seeds those three with hostile values and removes +/// them (so a dropped scrub reddens the suite instead of leaking a developer's +/// shell), and forces `SOCKET_NO_CONFIG=1` to block the socket-cli +/// config.json. An activated venv is scrubbed too. fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let mut cmd = Command::new(binary()); - cmd.args(args) - .current_dir(cwd) - // Seed hostile values, then remove them (adjacent, so deleting a remove - // line leaves the seed and breaks the suite rather than passing vacuously). - .env("SOCKET_API_TOKEN", "hostile-seed-must-be-scrubbed") - .env("SOCKET_PROXY_URL", "http://127.0.0.1:1/hostile") - .env("SOCKET_API_URL", "http://127.0.0.1:1/hostile") - .env_remove("SOCKET_API_TOKEN") - .env_remove("SOCKET_PROXY_URL") - .env_remove("SOCKET_API_URL"); - for (k, _) in std::env::vars_os() { - let key = k.to_string_lossy(); - if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { - cmd.env_remove(&k); - } - } - cmd.env_remove("VIRTUAL_ENV"); - cmd.env("SOCKET_NO_CONFIG", "true"); + let mut cmd = hermetic::binary_command(); + hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); + cmd.args(args).current_dir(cwd); let out: Output = cmd.output().expect("failed to execute socket-patch binary"); ( out.status.code().unwrap_or(-1), @@ -1688,15 +1673,7 @@ fn bun_manifestless_vex_production( let vex = |extra: &[&str]| -> (Option, serde_json::Value, Option) { let out_path = checkout.join("out.vex.json"); let _ = std::fs::remove_file(&out_path); - let mut cmd = Command::new(binary()); - for (key, _) in std::env::vars_os() { - if key.to_string_lossy().starts_with("SOCKET_") { - cmd.env_remove(key); - } - } - let out = cmd - .env("SOCKET_TELEMETRY_DISABLED", "1") - .env("SOCKET_NO_CONFIG", "1") + let out = hermetic::binary_command() .env("SOCKET_NO_API_TOKEN", "1") .current_dir(checkout) .args(["vex", "--json", "--output"]) diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index 8a051dae0..79b6a2022 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -8,11 +8,8 @@ //! `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 private `scrub_socket_env` copy, and -//! no new file that spawns the binary with a bare `Command::new`. The -//! `PENDING_RAW_SPAWNS` list below only shrinks — move a file onto -//! `hermetic::command` / `common::run*` and delete its entry. (The copy -//! check matches that one name; files that still sweep `SOCKET_*` by hand -//! under another name are the bare spawns that list tracks.) +//! no file that spawns the binary with a bare `Command::new`; every spawn +//! goes through `hermetic::command` / `common::run*`. #[path = "common/hermetic.rs"] mod hermetic; @@ -25,11 +22,6 @@ use std::process::Command; use hermetic::Extra; use serial_test::serial; -/// 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] = &["e2e_hosted_production.rs", "e2e_vendored_production.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 { @@ -559,27 +551,11 @@ fn no_new_bare_binary_spawns() { .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(), + found.is_empty(), "these test files spawn the binary with a bare Command::new: \ - {unexpected:?}. Use `hermetic::command(&bin)` / \ + {found:?}. Use `hermetic::command(&bin)` / \ `hermetic::binary_command()` or `common::run*` (tests/common/) so the \ - child gets the hermetic SOCKET_* environment. Do not add them to \ - PENDING_RAW_SPAWNS." - ); - 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: {stale:?}. Delete them \ - from PENDING_RAW_SPAWNS in \ - crates/socket-patch-cli/tests/spawn_env_hygiene.rs (another PR may \ - have migrated them; rebase and drop the entries)." + child gets the hermetic SOCKET_* environment." ); } From 4b81320b38b2c877aeb46984cc444e091011fed1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:12:25 +0000 Subject: [PATCH 3/3] CI: hermetic spawns in two missed fixtures The shared vendor fixture and the cli_parse_scan JSON-shape test bound the binary path to a local before spawning it, so the ratchet's detector never saw them. Both swept SOCKET_* by hand and kept an ambient SOCKET_NO_CONFIG=0, which let a developer's socket login reach the child. They now spawn through the shared hermetic builder. The ratchet's doc now says which spellings it can see and that PTY spawns keep their own scrubs, and a stale comment about how the vendored pypi leg discovers its venv is corrected. Refs #823 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/cli_parse_scan.rs | 23 +++++++++---------- .../tests/e2e_vendored_production.rs | 3 ++- .../tests/spawn_env_hygiene.rs | 7 ++++-- .../tests/vendor_ecosystem_fixtures/mod.rs | 14 ++++------- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/crates/socket-patch-cli/tests/cli_parse_scan.rs b/crates/socket-patch-cli/tests/cli_parse_scan.rs index 3a146aa57..ab0ace2eb 100644 --- a/crates/socket-patch-cli/tests/cli_parse_scan.rs +++ b/crates/socket-patch-cli/tests/cli_parse_scan.rs @@ -9,6 +9,9 @@ //! endpoint at run time (500 on the authenticated API, 100 on the public //! proxy), so an explicit value must stay distinguishable from none. +#[path = "common/hermetic.rs"] +mod hermetic; + use clap::Parser; use socket_patch_cli::commands::scan::{resolve_mode_flags, ScanArgs, ScanMode}; use socket_patch_cli::{Cli, Commands}; @@ -438,22 +441,18 @@ fn scan_json_empty_cwd_emits_updates_key() { // empty-scan JSON contract *exactly*, so a regression that drops/renames // a key, flips a default count, or leaks an unexpected `gc`/`apply`/`vex` // sub-object onto the hosted default path fails loudly. - let bin = env!("CARGO_BIN_EXE_socket-patch"); let tmp = tempfile::tempdir().expect("tempdir"); // Its own repo root, so no socket.yml above the temp dir leaks in. std::fs::create_dir(tmp.path().join(".git")).expect(".git"); - let mut cmd = std::process::Command::new(bin); + // Spawn with the hermetic environment, which strips *every* ambient + // SOCKET_* override. It is not enough to drop the API creds: an ambient + // `SOCKET_VEX` would fold a `vex` object into the output, + // `SOCKET_OFFLINE`/`SOCKET_GLOBAL` would steer the crawl, and + // `SOCKET_JSON=false` would suppress JSON entirely — any of which would + // either spuriously fail the exact-shape lock or, worse, change the + // branch under test. + let mut cmd = hermetic::binary_command(); cmd.args(["scan", "--json", "--cwd"]).arg(tmp.path()); - // Strip *every* SOCKET_* override the child would otherwise inherit. - // It is not enough to drop the API creds: an ambient `SOCKET_VEX` would - // fold a `vex` object into the output, `SOCKET_OFFLINE`/`SOCKET_GLOBAL` - // would steer the crawl, and `SOCKET_JSON=false` would suppress JSON - // entirely — any of which would either spuriously fail the exact-shape - // lock or, worse, change the branch under test. Clear them all so the - // subprocess sees only the CLI args we pass. - for k in SCAN_ENV_VARS { - cmd.env_remove(k); - } let out = cmd.output().expect("spawn socket-patch"); assert_eq!( diff --git a/crates/socket-patch-cli/tests/e2e_vendored_production.rs b/crates/socket-patch-cli/tests/e2e_vendored_production.rs index d69941c86..6a1f6e76b 100644 --- a/crates/socket-patch-cli/tests/e2e_vendored_production.rs +++ b/crates/socket-patch-cli/tests/e2e_vendored_production.rs @@ -1854,7 +1854,8 @@ fn pypi_requirements_txt_vendored_install_proof() { ); let reqs_before = std::fs::read(proj.join("requirements.txt")).unwrap(); - // Discovery reads the venv via VIRTUAL_ENV. + // Discovery finds the `.venv` under the cwd (`run_socket` scrubs + // VIRTUAL_ENV). let env_json = scan_vendored(&proj, &["--ecosystems", "pypi"]); assert_vendor_applied(&env_json, "urllib3@1.26.18", LEG); assert_download_uuid(&env_json, PYPI_UUIDS, LEG); diff --git a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs index 79b6a2022..906f6cbf7 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -8,8 +8,11 @@ //! `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 private `scrub_socket_env` copy, and -//! no file that spawns the binary with a bare `Command::new`; every spawn -//! goes through `hermetic::command` / `common::run*`. +//! no file that spawns the binary with a bare `Command::new`; spawns go +//! through `hermetic::command` / `common::run*`. The detector matches the +//! spellings in `has_raw_spawn`, not a path bound to a local first, and +//! doesn't see `portable_pty::CommandBuilder` spawns, which keep their +//! own scrubs. #[path = "common/hermetic.rs"] mod hermetic; diff --git a/crates/socket-patch-cli/tests/vendor_ecosystem_fixtures/mod.rs b/crates/socket-patch-cli/tests/vendor_ecosystem_fixtures/mod.rs index 0e4819770..93a18b0b0 100644 --- a/crates/socket-patch-cli/tests/vendor_ecosystem_fixtures/mod.rs +++ b/crates/socket-patch-cli/tests/vendor_ecosystem_fixtures/mod.rs @@ -13,7 +13,6 @@ mod prebuilt_common; use std::path::{Path, PathBuf}; -use std::process::Command; use sha2::Digest as _; @@ -95,7 +94,8 @@ impl Fixture { } /// Run the built binary in the project with the fixture's environment, - /// every ambient `SOCKET_*` scrubbed and `extra_env` on top. Returns + /// the hermetic `SOCKET_*` environment (`hermetic::command`) and + /// `extra_env` on top. Returns /// the exit code, stdout and stderr. pub fn run(&self, args: &[&str], extra_env: &[(&str, &str)]) -> (i32, String, String) { self.run_bin(env!("CARGO_BIN_EXE_socket-patch"), args, extra_env) @@ -109,7 +109,7 @@ impl Fixture { args: &[&str], extra_env: &[(&str, &str)], ) -> (i32, String, String) { - let mut cmd = Command::new(bin); + let mut cmd = crate::common::hermetic::command(Path::new(bin)); let args: Vec<_> = args .iter() .copied() @@ -118,16 +118,10 @@ impl Fixture { }) .collect(); cmd.args(&args).current_dir(&self.root); - for (key, _) in std::env::vars() { - if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { - cmd.env_remove(key); - } - } for var in ["VIRTUAL_ENV", "CONDA_PREFIX", "BUNDLE_PATH", "GEM_HOME"] { cmd.env_remove(var); } - cmd.env("SOCKET_TELEMETRY_DISABLED", "1") - .env("HOME", self.store.join("home")) + cmd.env("HOME", self.store.join("home")) .env("CARGO_HOME", self.store.join("cargo-home")) .env("GOMODCACHE", self.store.join("modcache")) .env("MAVEN_REPO_LOCAL", self.store.join("m2"))