Scrub color env in hermetic test children

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.
+//! `command` does not move `HOME`: only `run_bin_with_env` (and suites that
+//! call `jvm_env::isolate_cli` themselves) point the child at a stand-in.
 //! 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`]
@@ -31,34 +33,11 @@
 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.
-pub const HOSTILE_SEEDS: &[(&str, &str)] = &[
-    ("SOCKET_CWD", "/nonexistent"),
-    ("SOCKET_GLOBAL", "true"),
-    ("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"),
-    ("SOCKET_SILENT", "true"),
-    ("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 env the CLI's color and width policy reads (`ui::color_enabled`,
+/// `ui::width_of`) besides `TERM`. An ambient `CLICOLOR_FORCE=1` puts ANSI
+/// codes into every captured stdout. `TERM` is left alone: the PTY suites
+/// hand the binary a real terminal and pick their own.
+pub const COLOR_ENV: &[&str] = &["NO_COLOR", "CLICOLOR", "CLICOLOR_FORCE", "COLUMNS"];
 
 /// The opt-outs [`command`] forces on in every child: the config layer
 /// (a developer's real `socket login` must never authenticate a test
@@ -75,19 +54,15 @@ pub const FORCED_OPT_OUTS: &[(&str, &str)] = &[
     ("SOCKET_TELEMETRY_DISABLED", "1"),
 ];
 
-/// A `Command` for `bin` with the hermetic `SOCKET_*` environment: the
-/// 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.
+/// A `Command` for `bin` with the hermetic environment: every flag-bound
+/// and every other ambient `SOCKET_*` removed (removing `SOCKET_API_TOKEN`
+/// also forces the public proxy), [`COLOR_ENV`] removed, 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 {
-        cmd.env(k, v);
-    }
-    for (k, _) in HOSTILE_SEEDS {
+    for k in COLOR_ENV {
         cmd.env_remove(k);
     }
     // The flag-bound vars are removed by name, not only when the sweep below
@@ -101,6 +76,9 @@ pub fn command(bin: &Path) -> Command {
         cmd.env_remove(k);
     }
     cmd.env_remove("SOCKET_API_TOKEN");
+    // Not flag-bound; removed by name for the same reason.
+    cmd.env_remove("SOCKET_UPDATE_BASE_URL");
+    cmd.env_remove("SOCKET_UPDATE_STATE_DIR");
     // The `SOCKET_CLI_*` peer names too: the binary promotes each onto its
     // canonical `SOCKET_*` name, so a peer token set mid-run would put the
     // child on the authenticated path.
@@ -229,14 +207,13 @@ pub enum Extra {
     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.
+    /// PnP tree and void the node_modules probes.
     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.
+    /// `vendor` refuses.
     Pnpm,
     /// Yarn 1's ambient config: the cache folder in any spelling
     /// (`YARN_CACHE_FOLDER`, `yarn_cache_folder`, `npm_config_cache_folder`;
@@ -256,8 +233,8 @@ pub enum Extra {
     /// lib dir, which pin every child ruby to the outer Gemfile and bundler.
     /// `GEM_HOME`/`GEM_PATH` are kept on purpose: rvm/chruby
     /// export them and `bundle` itself resolves through them (see
-    /// `cache_env.rs`). Seeded with a bogus `BUNDLE_APP_CONFIG` and then
-    /// scrubbed; run it before the suite sets its own `BUNDLE_*` values.
+    /// `cache_env.rs`). Run it before the suite sets its own `BUNDLE_*`
+    /// values.
     Bundle,
     /// Every `PDM_*` var plus `UV_PROJECT_ENVIRONMENT`: PDM reads any
     /// setting from `PDM_<NAME>`, and its uv backend follows
@@ -309,16 +286,13 @@ pub fn scrub_extra(cmd: &mut Command, extras: &[Extra]) {
                 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_")
@@ -327,7 +301,6 @@ pub fn scrub_extra(cmd: &mut Command, extras: &[Extra]) {
                         cmd.env_remove(&key);
                     }
                 }
-                cmd.env_remove("npm_config_node_linker");
             }
             Extra::YarnClassic => {
                 cmd.env_remove("YARN_CACHE_FOLDER");
@@ -345,13 +318,11 @@ pub fn scrub_extra(cmd: &mut Command, extras: &[Extra]) {
                 cmd.env_remove("PREFIX");
             }
             Extra::Bundle => {
-                cmd.env("BUNDLE_APP_CONFIG", "/nonexistent/.bundle");
                 for (key, _) in std::env::vars_os() {
                     if key.to_string_lossy().starts_with("BUNDLE_") {
                         cmd.env_remove(&key);
                     }
                 }
-                cmd.env_remove("BUNDLE_APP_CONFIG");
                 cmd.env_remove("RUBYOPT");
                 cmd.env_remove("RUBYLIB");
             }

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

@@ -194,7 +194,7 @@ fn go_build_links_patch_via_replace_redirect() {
     // this suite fails deterministically if the leak returns. `GOTOOLCHAIN`
     // must lose to go_env's `local` pin (or every `go` command chases a
     // nonexistent toolchain through the file proxy). `SOCKET_DRY_RUN` needs
-    // no guard here: `hermetic::command` seeds and scrubs it itself.
+    // no guard here: `spawn_env_hygiene` pins `hermetic::command`'s scrub.
     std::env::set_var("GOTOOLCHAIN", "go1.99.99");
     let tmp = tempfile::tempdir().unwrap();
     let (consumer, modcache, proxy_url) = stage(tmp.path());

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

@@ -87,8 +87,8 @@
 //! No API token is used or needed — the suite deliberately runs against the
 //! **free public proxy** with `SOCKET_NO_CONFIG=true` so a developer's
 //! socket-cli login cannot move the run onto the org catalog. Every ambient
-//! `SOCKET_*` var is scrubbed and hostile seeds are planted so a dropped scrub
-//! turns the suite red immediately.
+//! `SOCKET_*` var is scrubbed; `spawn_env_hygiene` turns red if a scrub is
+//! dropped.
 //!
 //! # Running
 //!
@@ -324,9 +324,9 @@ fn has_corepack_pm(pm: &str) -> bool {
 /// 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. 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
+/// every ambient `SOCKET_*` (`spawn_env_hygiene` seeds those three in the
+/// test process and fails if one reaches the child), and forces
+/// `SOCKET_NO_CONFIG=1` to block the socket-cli
 /// config.json. The crawler's own ambient inputs are scrubbed too
 /// (`hermetic::CRAWLER_EXTRAS`: an activated venv, `BUNDLE_*`, `COMPOSER_*`),
 /// so an ambient BUNDLE_PATH can't move the gem leg's crawl off

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

@@ -239,13 +239,37 @@ fn workflow_env_copies_carry_every_opt_out() {
     );
 }
 
+/// Every flag-bound var but the forced opt-outs, plus the other high-risk
+/// `SOCKET_*` values and the color env, set in this process so a dropped
+/// scrub fails.
+fn hostile_ambient() -> Vec<(&'static str, &'static str)> {
+    let args = socket_patch_cli::args::GLOBAL_ARG_ENV_VARS
+        .iter()
+        .chain(socket_patch_cli::args::LOCAL_ARG_ENV_VARS)
+        .filter(|k| !hermetic::FORCED_OPT_OUTS.iter().any(|(f, _)| f == *k))
+        .map(|k| (*k, "true"));
+    let other = [
+        ("SOCKET_API_TOKEN", "ambient-token"),
+        ("SOCKET_UPDATE_BASE_URL", "http://127.0.0.1:1"),
+        ("SOCKET_UPDATE_STATE_DIR", "/nonexistent"),
+        ("NO_COLOR", "1"),
+        ("CLICOLOR", "1"),
+        ("CLICOLOR_FORCE", "1"),
+        ("COLUMNS", "20"),
+    ];
+    args.chain(other).collect()
+}
+
 #[test]
 #[serial]
-fn command_never_leaks_its_hostile_seeds() {
-    let env = effective_env(&hermetic::command(Path::new("socket-patch")));
-    for (key, _) in hermetic::HOSTILE_SEEDS {
-        assert_eq!(get(&env, key), None, "{key} seed must be scrubbed");
-    }
+fn command_scrubs_hostile_ambient_env() {
+    let hostile = hostile_ambient();
+    with_ambient(&hostile, || {
+        let env = effective_env(&hermetic::command(Path::new("socket-patch")));
+        for (key, _) in &hostile {
+            assert_eq!(get(&env, key), None, "ambient {key} must be scrubbed");
+        }
+    });
 }
 
 /// In-process tests set flag-bound vars in the test binary's own
@@ -334,14 +358,17 @@ fn pty_command_carries_the_same_env_as_command() {
 
 #[test]
 #[serial]
-fn pty_command_never_leaks_its_hostile_seeds_and_keeps_caller_env() {
-    let mut cmd = hermetic::pty_binary_command();
-    let env = pty_env(&cmd);
-    for (key, _) in hermetic::HOSTILE_SEEDS {
-        assert_eq!(env.get(*key), None, "{key} seed must be scrubbed");
-    }
-    cmd.env("SOCKET_NO_UPDATE_CHECK", "0");
-    assert_eq!(cmd.get_env("SOCKET_NO_UPDATE_CHECK"), Some("0".as_ref()));
+fn pty_command_scrubs_hostile_ambient_env_and_keeps_caller_env() {
+    let hostile = hostile_ambient();
+    with_ambient(&hostile, || {
+        let mut cmd = hermetic::pty_binary_command();
+        let env = pty_env(&cmd);
+        for (key, _) in &hostile {
+            assert_eq!(env.get(*key), None, "ambient {key} must be scrubbed");
+        }
+        cmd.env("SOCKET_NO_UPDATE_CHECK", "0");
+        assert_eq!(cmd.get_env("SOCKET_NO_UPDATE_CHECK"), Some("0".as_ref()));
+    });
 }
 
 #[test]
@@ -516,17 +543,6 @@ fn composer_extra_sweeps_overrides_and_pins_an_empty_home() {
     );
 }
 
-#[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, Extra::Bundle]);
-    let env = effective_env(&cmd);
-    assert_eq!(get(&env, "YARN_NODE_LINKER"), None);
-    assert_eq!(get(&env, "npm_config_node_linker"), None);
-    assert_eq!(get(&env, "BUNDLE_APP_CONFIG"), 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)>) {

crates/socket-patch-cli/tests/vendor/e2e_golang_redirect.rs

@@ -64,8 +64,8 @@ fn stage(root: &Path) -> (std::path::PathBuf, std::path::PathBuf) {
     (gomodcache, cache_dir)
 }
 
-/// Run the CLI binary through the shared hermetic runner. Its hostile seeds
-/// cover the vars verified to break this suite when inherited:
+/// Run the CLI binary through the shared hermetic runner. Its scrub
+/// covers the vars verified to break this suite when inherited:
 /// `SOCKET_GLOBAL` / `SOCKET_GLOBAL_PREFIX` take the redirect backend out of
 /// local scope (apply patches the fake module cache IN PLACE and `--check`
 /// exits 0 having checked nothing), `SOCKET_DRY_RUN` makes every apply a