CI: hermetic env for the PTY test spawns (#823)

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

@@ -19,7 +19,7 @@ use crate::common::binary;
 use std::path::Path;
 use std::time::Duration;
 
-use portable_pty::{native_pty_system, CommandBuilder, PtySize};
+use portable_pty::{native_pty_system, PtySize};
 use wiremock::matchers::{method, path};
 use wiremock::{Mock, MockServer, ResponseTemplate};
 
@@ -104,58 +104,24 @@ fn run_in_pty_inner(
         })
         .expect("openpty");
 
+    // The hermetic environment (`tests/common/hermetic.rs`): an ambient
+    // SOCKET_YES=true would skip the confirm prompt the bare-Enter test
+    // drives, and SOCKET_JSON=true flips select_one onto its
+    // JsonModeNeedsExplicit branch before the dialoguer menu renders. The
+    // `sh` wrapper `exec`s the binary, which inherits the same environment.
     let mut cmd = if sigint == Sigint::Ignored {
-        let mut cmd = CommandBuilder::new("/bin/sh");
+        let mut cmd = common::hermetic::pty_command("/bin/sh");
         cmd.arg("-c");
         cmd.arg("trap '' INT; exec \"$0\" \"$@\"");
         cmd.arg(binary());
         cmd
     } else {
-        CommandBuilder::new(binary())
+        common::hermetic::pty_binary_command()
     };
     for a in args {
         cmd.arg(a);
     }
     cmd.cwd(cwd);
-    // The binary binds a wide `SOCKET_*` env surface. An ambient value
-    // silently reroutes what these tests exercise — SOCKET_YES=true skips
-    // the very confirm prompt the bare-Enter test exists to drive, and
-    // SOCKET_JSON=true flips select_one onto its JsonModeNeedsExplicit
-    // branch before the dialoguer menu ever renders. 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_YES", "true");
-    cmd.env("SOCKET_JSON", "true");
-    cmd.env("SOCKET_DRY_RUN", "true");
-    cmd.env("SOCKET_SILENT", "true");
-    cmd.env_remove("SOCKET_YES");
-    cmd.env_remove("SOCKET_JSON");
-    cmd.env_remove("SOCKET_DRY_RUN");
-    cmd.env_remove("SOCKET_SILENT");
-    // Prefix-scrub whatever else the ambient shell carries (SOCKET_CWD,
-    // SOCKET_MANIFEST_PATH, SOCKET_API_TOKEN, SOCKET_PROXY_URL — the get
-    // tests pin their API endpoint by flag and must not be rerouted).
-    // 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);
-        }
-    }
-    // A developer's real `socket login` (the socket-cli config.json token
-    // fallback) must never authenticate a test child.
-    cmd.env("SOCKET_NO_CONFIG", "1");
-    // This suite hands children a REAL terminal, so the update notifier's
-    // stderr-TTY guard does not protect it. Force the kill-switch so no PTY
-    // child ever fetches release metadata mid-prompt.
-    cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
     // `dialoguer`/`console` key handling needs a terminal type; pin one so
     // an unset ambient TERM (bare CI shells) can't skew rendering.
     cmd.env("TERM", "xterm-256color");

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

@@ -7,12 +7,10 @@
 
 #![cfg(unix)]
 
-use crate::common::binary;
-
 use std::path::Path;
 use std::time::Duration;
 
-use portable_pty::{native_pty_system, CommandBuilder, PtySize};
+use portable_pty::{native_pty_system, PtySize};
 
 /// Spawn the socket-patch binary inside a PTY, send `input`, and
 /// collect all output until the child exits. Returns `(exit_code,
@@ -52,49 +50,17 @@ fn run_in_pty_bytes(args: &[&str], cwd: &Path, input: &[u8], timeout: Duration)
         })
         .expect("openpty");
 
-    let mut cmd = CommandBuilder::new(binary());
+    // The hermetic environment (`tests/common/hermetic.rs`): an ambient
+    // SOCKET_YES=true would skip the very confirm prompts this file exists
+    // to drive, and SOCKET_SILENT=true would suppress the output the
+    // oracles match. It also forces the update notifier off: this suite
+    // hands the child a REAL terminal, so the stderr-TTY guard does not
+    // protect it.
+    let mut cmd = crate::common::hermetic::pty_binary_command();
     for a in args {
         cmd.arg(a);
     }
     cmd.cwd(cwd);
-    // The binary binds a wide `SOCKET_*` env surface (SOCKET_YES,
-    // SOCKET_JSON, SOCKET_DRY_RUN, SOCKET_SILENT, SOCKET_MANIFEST_PATH,
-    // ...). An ambient value silently reroutes what these tests exercise —
-    // SOCKET_YES=true skips the very confirm prompts this file exists to
-    // drive, and SOCKET_SILENT=true suppresses the output the oracles
-    // match. 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_YES", "true");
-    cmd.env("SOCKET_JSON", "true");
-    cmd.env("SOCKET_DRY_RUN", "true");
-    cmd.env("SOCKET_SILENT", "true");
-    cmd.env_remove("SOCKET_YES");
-    cmd.env_remove("SOCKET_JSON");
-    cmd.env_remove("SOCKET_DRY_RUN");
-    cmd.env_remove("SOCKET_SILENT");
-    // Prefix-scrub whatever else the ambient shell carries (SOCKET_CWD,
-    // SOCKET_MANIFEST_PATH, SOCKET_API_TOKEN — removing the token also
-    // forces the public proxy). 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);
-        }
-    }
-    // This suite is the one place test children get a REAL terminal, so the
-    // update notifier's stderr-TTY guard does not protect it. Force the
-    // kill-switch (mirroring the `.cargo/config.toml` `[env]` default the
-    // prefix scrub would otherwise strip) so no PTY child ever fetches
-    // release metadata mid-prompt.
-    cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
 
     let mut child = pair
         .slave

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

@@ -11,6 +11,8 @@
 //!
 //! [`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.
+//! PTY suites use [`pty_command`] / [`pty_binary_command`], which carry the
+//! same environment onto a `portable_pty::CommandBuilder`.
 //! Package-manager children the same tests spawn get [`scrub_socket_vars`]
 //! plus whichever [`Extra`] scrubs their tool needs.
 //!
@@ -40,6 +42,8 @@ pub const HOSTILE_SEEDS: &[(&str, &str)] = &[
     ("SOCKET_GLOBAL_PREFIX", "/nonexistent"),
     ("SOCKET_DRY_RUN", "true"),
     ("SOCKET_SAVE_ONLY", "true"),
+    // Skips every confirm prompt the PTY suites exist to drive.
+    ("SOCKET_YES", "true"),
     ("SOCKET_ECOSYSTEMS", "cargo"),
     ("SOCKET_MANIFEST_PATH", "/nonexistent/manifest.json"),
     ("SOCKET_JSON", "true"),
@@ -131,23 +135,64 @@ pub fn tool_command(name: &str) -> Command {
     )
 }
 
+/// [`command`] for a PTY child: the same hermetic environment on a
+/// `portable_pty::CommandBuilder` for `program`. The PTY suites hand the
+/// binary a real terminal, so they can't use `std::process::Command`;
+/// before this they each hand-rolled a scrub, and the copies drifted (one
+/// stripped the opt-outs, several didn't force them, none removed the
+/// flag-bound vars by name). `program` is usually the binary, or a shell
+/// that `exec`s it, which inherits the same environment.
+///
+/// The builder's environment is replaced outright with what [`command`]'s
+/// child would see. The copy here and [`command`]'s own sweep each read the
+/// process environment, so a var a parallel in-process test clears between
+/// the two reads would survive the plan; the copy is swept once more with
+/// the same rule. Callers add args, cwd and env
+/// afterwards, as with [`command`].
+pub fn pty_command(program: impl AsRef<std::ffi::OsStr>) -> portable_pty::CommandBuilder {
+    let program = program.as_ref();
+    let mut env: std::collections::BTreeMap<_, _> = std::env::vars_os().collect();
+    for (key, value) in command(Path::new(program)).get_envs() {
+        match value {
+            Some(value) => env.insert(key.to_owned(), value.to_owned()),
+            None => env.remove(key),
+        };
+    }
+    env.retain(|key, _| !is_scrubbed_socket_var(key));
+    let mut pty = portable_pty::CommandBuilder::new(program);
+    pty.env_clear();
+    for (key, value) in env {
+        pty.env(key, value);
+    }
+    pty
+}
+
+/// [`pty_command`] for the `socket-patch` binary cargo built for this run.
+pub fn pty_binary_command() -> portable_pty::CommandBuilder {
+    pty_command(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"
-        {
+        if is_scrubbed_socket_var(&key) {
             cmd.env_remove(&key);
         }
     }
 }
 
+/// Whether [`scrub_socket_vars`] removes the var `key`.
+fn is_scrubbed_socket_var(key: &std::ffi::OsStr) -> bool {
+    let name = key.to_string_lossy();
+    name.starts_with("SOCKET_")
+        && !name.contains("TELEMETRY")
+        && name != "SOCKET_NO_CONFIG"
+        && name != "SOCKET_NO_UPDATE_CHECK"
+}
+
 /// The in-process half, for test binaries that only parse argv with clap:
 /// clap reads every env-bound `SOCKET_*` flag at parse time, so an ambient
 /// value (or a `.cargo/config.toml` `[env]` default such as

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

@@ -2377,7 +2377,7 @@ fn manifest_write_failure_warns_and_exits_one() {
 #[cfg(unix)]
 mod interactive {
     use super::*;
-    use portable_pty::{native_pty_system, CommandBuilder, PtySize};
+    use portable_pty::{native_pty_system, PtySize};
     use std::time::Duration;
 
     /// Spawn the binary inside a PTY, send `input`, collect all output —
@@ -2393,32 +2393,13 @@ mod interactive {
             })
             .expect("openpty");
 
-        let mut cmd = CommandBuilder::new(common::binary());
+        // The hermetic environment: SOCKET_YES would skip the very prompt
+        // under test.
+        let mut cmd = common::hermetic::pty_binary_command();
         for a in args {
             cmd.arg(a);
         }
         cmd.cwd(cwd);
-        // Seed-then-scrub the highest-risk vars (SOCKET_YES would skip the
-        // very prompt under test), then prefix-scrub the rest.
-        cmd.env("SOCKET_YES", "true");
-        cmd.env("SOCKET_JSON", "true");
-        cmd.env("SOCKET_DRY_RUN", "true");
-        cmd.env("SOCKET_SILENT", "true");
-        cmd.env_remove("SOCKET_YES");
-        cmd.env_remove("SOCKET_JSON");
-        cmd.env_remove("SOCKET_DRY_RUN");
-        cmd.env_remove("SOCKET_SILENT");
-        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);
-            }
-        }
-        cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
 
         let mut child = pair
             .slave

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

@@ -1581,7 +1581,7 @@ mod pty {
     use super::*;
     use std::time::Duration;
 
-    use portable_pty::{native_pty_system, CommandBuilder, PtySize};
+    use portable_pty::{native_pty_system, PtySize};
 
     /// Spawn the binary in a PTY, send `input`, and collect all output
     /// until exit (the `interactive_prompts_e2e.rs` harness pattern:
@@ -1611,37 +1611,16 @@ mod pty {
             })
             .expect("openpty");
 
-        let mut cmd = CommandBuilder::new(binary());
+        // The hermetic environment (`tests/common/hermetic.rs`), which also
+        // forces the update notifier off: this child gets a REAL terminal,
+        // so the stderr-TTY guard does not protect it.
+        let mut cmd = common::hermetic::pty_binary_command();
         for a in args {
             cmd.arg(a);
         }
         cmd.cwd(cwd);
         // These tests assert VT rendering, even when the parent has TERM=dumb.
         cmd.env("TERM", "xterm-256color");
-        // Scrub the flag-bound SOCKET_* surface so ambient configuration
-        // cannot reroute the run; keep telemetry disabled and the
-        // update notifier off — this child gets a REAL terminal, so the
-        // stderr-TTY guard does not protect it.
-        for (key, _) in std::env::vars_os() {
-            let name = key.to_string_lossy();
-            if name.starts_with("SOCKET_") {
-                cmd.env_remove(&key);
-            }
-        }
-        // In-process tests in this binary `std::env::set_var` these via
-        // `apply_env_toggles`; one set by a parallel test between the scan
-        // above 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);
-        }
-        cmd.env("SOCKET_TELEMETRY_DISABLED", "1");
-        cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
         for (k, v) in env {
             cmd.env(k, v);
         }

crates/socket-patch-cli/tests/remove/covgap_commands_remove.rs

@@ -13,8 +13,6 @@
 //! remove_invariants.rs / remove_duality_invariants.rs /
 //! interactive_prompts_e2e.rs.
 
-use crate::common::binary;
-
 use std::path::{Path, PathBuf};
 
 use crate::common;
@@ -1892,7 +1890,7 @@ fn remove_rollback_infrastructure_error_surfaces_rollback_failed() {
 #[cfg(unix)]
 mod pty {
     use super::*;
-    use portable_pty::{native_pty_system, CommandBuilder, PtySize};
+    use portable_pty::{native_pty_system, PtySize};
     use std::time::Duration;
 
     /// Spawn the binary inside a PTY, send `input`, collect all output
@@ -1908,26 +1906,13 @@ mod pty {
             })
             .expect("openpty");
 
-        let mut cmd = CommandBuilder::new(binary());
+        // The hermetic environment: SOCKET_YES would skip the very prompt
+        // this test drives.
+        let mut cmd = common::hermetic::pty_binary_command();
         for a in args {
             cmd.arg(a);
         }
         cmd.cwd(cwd);
-        // Scrub the ambient SOCKET_* surface (SOCKET_YES would skip the
-        // very prompt this test drives); keep telemetry opt-outs.
-        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);
-            }
-        }
-        cmd.env("SOCKET_NO_CONFIG", "1");
-        cmd.env("SOCKET_NO_UPDATE_CHECK", "1");
-        cmd.env("SOCKET_TELEMETRY_DISABLED", "1");
 
         let mut child = pair.slave.spawn_command(cmd).expect("spawn in PTY");
         drop(pair.slave);