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/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..6a1f6e76b 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"]) @@ -1877,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 8a051dae0..906f6cbf7 100644 --- a/crates/socket-patch-cli/tests/spawn_env_hygiene.rs +++ b/crates/socket-patch-cli/tests/spawn_env_hygiene.rs @@ -8,11 +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 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`; 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; @@ -25,11 +25,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 +554,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." ); } 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"))