Close the in-process env race in the builder

Some test binaries set flag-bound SOCKET_* vars in their own process
(apply_env_toggles, env save/restore helpers). The hermetic builder only
removed names present when it swept the parent environment, so a var
set by a parallel test before the spawn still reached the child, and
suites patched that with their own, already diverging, removal lists.

hermetic::command now removes every flag-bound var by name, so the
per-file lists go away, and SOCKET_CWD joins the hostile seeds. A new
contract test sets vars after the builder runs and checks none leak.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5

Author: claude

crates/socket-patch-cli/tests/cli/covgap_commands_list.rs

@@ -64,7 +64,7 @@ fn write_empty_manifest(root: &Path) {
 /// fixture and the config layer re-enabled. `debug` adds the `--debug`
 /// flag under test.
 fn list_cmd(project: &Path, data_dir: &Path, debug: bool) -> Command {
-    let mut cmd = common::hermetic::binary_command();
+    let mut cmd = common::hermetic_command(&common::binary());
     cmd.arg("list");
     if debug {
         cmd.arg("--debug");

crates/socket-patch-cli/tests/common/hermetic.rs

@@ -35,6 +35,7 @@ use std::process::Command;
 /// developer's ambient shell, which the suites can't rely on) turns the tests
 /// red immediately.
 const HOSTILE_SEEDS: &[(&str, &str)] = &[
+    ("SOCKET_CWD", "/nonexistent"),
     ("SOCKET_GLOBAL", "true"),
     ("SOCKET_GLOBAL_PREFIX", "/nonexistent"),
     ("SOCKET_DRY_RUN", "true"),
@@ -48,11 +49,12 @@ const HOSTILE_SEEDS: &[(&str, &str)] = &[
 ];
 
 /// 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 three opt-outs
-/// (config layer, update notifier, telemetry) forced on. Callers add args,
-/// cwd and their own env afterwards; caller env lands last, so explicit
-/// injections survive the scrub.
+/// hostile seeds scrubbed, every flag-bound `SOCKET_*` and every other
+/// ambient `SOCKET_*` removed (removing `SOCKET_API_TOKEN` also forces the
+/// public proxy), and the three opt-outs (config layer, update notifier,
+/// telemetry) 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 {
@@ -61,6 +63,16 @@ pub fn command(bin: &Path) -> Command {
     for (k, _) in HOSTILE_SEEDS {
         cmd.env_remove(k);
     }
+    // The flag-bound vars are removed by name, not only when the sweep below
+    // finds them: in-process tests in the same binary set some of them
+    // (`apply_env_toggles`, env save/restore helpers), and one set between
+    // this sweep and the spawn would otherwise reach the child.
+    for k in socket_patch_cli::args::GLOBAL_ARG_ENV_VARS
+        .iter()
+        .chain(socket_patch_cli::args::LOCAL_ARG_ENV_VARS)
+    {
+        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:

crates/socket-patch-cli/tests/covgap_commands_rollback.rs

@@ -248,7 +248,8 @@ fn readonly_dir_enforced(dir: &Path) -> bool {
 
 /// Spawn the binary WITHOUT waiting (the lock-choreography tests mutate
 /// the fixture while the child is blocked on the apply lock), with the
-/// same hermetic environment as `common::run`.
+/// same hermetic `SOCKET_*` environment as `common::run` (not its JVM
+/// isolation; no test here spawns a JVM fixture this way).
 fn spawn_scrubbed(cwd: &Path, args: &[&str]) -> std::process::Child {
     let mut cmd = common::hermetic_command(&common::binary());
     cmd.args(args).current_dir(cwd);

crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs

@@ -47,8 +47,6 @@ use std::path::Path;
 use wiremock::matchers::{method, path};
 use wiremock::{Mock, MockServer, ResponseTemplate};
 
-use socket_patch_cli::args::{GLOBAL_ARG_ENV_VARS, LOCAL_ARG_ENV_VARS};
-
 const ORG_SLUG: &str = "test-org";
 const UUID: &str = "11111111-1111-4111-8111-111111111111";
 const OLD_UUID: &str = "99999999-9999-4999-8999-999999999999";
@@ -78,13 +76,6 @@ fn write_npm_package(root: &Path, name: &str, version: &str, content: &[u8]) {
 fn run_scan(cwd: &Path, args: &[&str]) -> (i32, String, String) {
     let mut cmd = hermetic_command(&binary());
     cmd.arg("scan").args(args).current_dir(cwd);
-    // The `mode_fold` tests below clear and restore the flag-bound vars
-    // in-process; one restored by a parallel test between the builder's
-    // scrub and the spawn would be inherited, so remove the whole
-    // flag-bound surface unconditionally too.
-    for var in GLOBAL_ARG_ENV_VARS.iter().chain(LOCAL_ARG_ENV_VARS.iter()) {
-        cmd.env_remove(var);
-    }
     // The unconditional python crawl consults VIRTUAL_ENV first, so an
     // activated host virtualenv would make "empty project" scans non-empty.
     cmd.env_remove("VIRTUAL_ENV");

crates/socket-patch-cli/tests/covgap_commands_vendor.rs

@@ -205,18 +205,6 @@ fn run_cli(cwd: &Path, args: &[&str], extra_env: &[(&str, &str)]) -> (i32, Strin
         }
     }
     cmd.args(&filtered).current_dir(cwd);
-    // In-process tests in this binary `std::env::set_var` these via
-    // `apply_env_toggles`; one set by a parallel test between the
-    // builder's scrub and the spawn would be inherited, so remove them
-    // unconditionally (see in_process_vendor.rs `run_cli`).
-    for key in [
-        "SOCKET_OFFLINE",
-        "SOCKET_DEBUG",
-        "SOCKET_API_URL",
-        "SOCKET_PROXY_URL",
-    ] {
-        cmd.env_remove(key);
-    }
     for (k, v) in extra_env {
         cmd.env(k, v);
     }
@@ -763,8 +751,6 @@ fn npm_only_human_vendor_does_not_read_composer_lock() {
         .env("SOCKET_VENDOR_URL", &fixture.uri)
         .args(["vendor", "--cwd", fx.root().to_str().unwrap()])
         .current_dir(fx.root())
-        .env_remove("SOCKET_OFFLINE")
-        .env_remove("SOCKET_API_URL")
         .stdin(std::process::Stdio::null())
         .stdout(std::process::Stdio::piped())
         .stderr(std::process::Stdio::piped())

crates/socket-patch-cli/tests/spawn_env_hygiene.rs

@@ -342,6 +342,7 @@ fn workflow_env_copies_carry_every_opt_out() {
 fn command_never_leaks_its_hostile_seeds() {
     let env = effective_env(&hermetic::command(Path::new("socket-patch")));
     for key in [
+        "SOCKET_CWD",
         "SOCKET_GLOBAL",
         "SOCKET_GLOBAL_PREFIX",
         "SOCKET_DRY_RUN",
@@ -357,6 +358,34 @@ fn command_never_leaks_its_hostile_seeds() {
     }
 }
 
+/// In-process tests set flag-bound vars in the test binary's own
+/// environment (`apply_env_toggles`, save/restore helpers). One set after
+/// the builder's sweep but before the spawn must still not reach the child.
+#[test]
+#[serial]
+fn command_drops_flag_bound_vars_set_after_it_is_built() {
+    let cmd = hermetic::command(Path::new("socket-patch"));
+    with_ambient(
+        &[
+            ("SOCKET_OFFLINE", "true"),
+            ("SOCKET_DEBUG", "1"),
+            ("SOCKET_PROXY_URL", "http://ambient.invalid"),
+            ("SOCKET_SKIP_ROLLBACK", "true"),
+        ],
+        || {
+            let env = effective_env(&cmd);
+            for key in [
+                "SOCKET_OFFLINE",
+                "SOCKET_DEBUG",
+                "SOCKET_PROXY_URL",
+                "SOCKET_SKIP_ROLLBACK",
+            ] {
+                assert_eq!(get(&env, key), None, "{key} set after build leaked");
+            }
+        },
+    );
+}
+
 #[test]
 #[serial]
 fn caller_env_set_after_command_survives_the_scrub() {