From 8f29f995a88194c5970f802fdf1299a8fb2cf021 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 12 Oct 2026 04:50:32 +0000
Subject: [PATCH] Fix env race in ambient scrub self-tests

The cache_env self-tests ambient_{bun,vlt}_scrub_removes_only_matching_
parent_vars read the process env twice: once inside the scrub and once
to check that every matching parent var was removed. common/cache_env.rs
is compiled into ~120 test binaries, and in-process tests in the same
binary mirror --offline into the env (apply_env_toggles sets
SOCKET_OFFLINE), so a var appearing between the two reads failed the
self-test with "SOCKET_OFFLINE ... was not scrubbed" / "kept".

Snapshot the parent env once and pass that snapshot to both the scrub
and the assertions. The public scrubs behave exactly as before.

Locally in_process_edge_cases failed 8/60 runs before and 0/200 after.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018js6KeL55GXJTLwgF3bQWV
---
 .../tests/common/cache_env.rs                 | 35 +++++++++++++------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/crates/socket-patch-cli/tests/common/cache_env.rs b/crates/socket-patch-cli/tests/common/cache_env.rs
index 8ba1f5879..d01fd7e4b 100644
--- a/crates/socket-patch-cli/tests/common/cache_env.rs
+++ b/crates/socket-patch-cli/tests/common/cache_env.rs
@@ -59,6 +59,7 @@
 
 #![allow(dead_code)]
 
+use std::ffi::OsString;
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -285,8 +286,22 @@ pub fn is_ambient_bun_var(name: &str) -> bool {
 /// rule in the module docs: the scrub iterates the parent env and would
 /// otherwise remove what those seed).
 pub fn scrub_ambient_bun_env(cmd: &mut Command) {
-    for (k, _) in std::env::vars_os() {
-        if is_ambient_bun_var(&k.to_string_lossy()) {
+    scrub_parent_vars(cmd, parent_var_names(), is_ambient_bun_var);
+}
+
+/// The names of the PARENT environment, read once.
+fn parent_var_names() -> Vec<OsString> {
+    std::env::vars_os().map(|(k, _)| k).collect()
+}
+
+/// Remove every name in `parent` that `ambient` matches from `cmd`. The
+/// scrubs and their self-tests share it so a self-test can check the
+/// result against the same snapshot of the parent env the scrub saw:
+/// in-process tests in the same binary mirror flags into the env
+/// (`--offline` sets `SOCKET_OFFLINE`), so two separate reads race.
+fn scrub_parent_vars(cmd: &mut Command, parent: Vec<OsString>, ambient: fn(&str) -> bool) {
+    for k in parent {
+        if ambient(&k.to_string_lossy()) {
             cmd.env_remove(&k);
         }
     }
@@ -316,11 +331,7 @@ pub fn is_ambient_vlt_var(name: &str) -> bool {
 /// Remove every [`is_ambient_vlt_var`] variable of the PARENT environment
 /// from `cmd` (call it before [`isolate`], see the module docs).
 pub fn scrub_ambient_vlt_env(cmd: &mut Command) {
-    for (k, _) in std::env::vars_os() {
-        if is_ambient_vlt_var(&k.to_string_lossy()) {
-            cmd.env_remove(&k);
-        }
-    }
+    scrub_parent_vars(cmd, parent_var_names(), is_ambient_vlt_var);
 }
 
 // ── Self-tests ────────────────────────────────────────────────────────
@@ -373,8 +384,9 @@ mod cache_env_selftests {
     /// nothing else is touched, and no matching parent variable is missed.
     #[test]
     fn ambient_bun_scrub_removes_only_matching_parent_vars() {
+        let parent = parent_var_names();
         let mut cmd = Command::new("true");
-        scrub_ambient_bun_env(&mut cmd);
+        scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_bun_var);
         let removed: Vec<String> = cmd
             .get_envs()
             .filter(|(_, value)| value.is_none())
@@ -386,7 +398,7 @@ mod cache_env_selftests {
                 "{name} was removed but is not an ambient bun var"
             );
         }
-        for (name, _) in std::env::vars_os() {
+        for name in &parent {
             let name = name.to_string_lossy();
             if is_ambient_bun_var(&name) {
                 assert!(
@@ -435,8 +447,9 @@ mod cache_env_selftests {
 
     #[test]
     fn ambient_vlt_scrub_removes_only_matching_parent_vars() {
+        let parent = parent_var_names();
         let mut cmd = Command::new("true");
-        scrub_ambient_vlt_env(&mut cmd);
+        scrub_parent_vars(&mut cmd, parent.clone(), is_ambient_vlt_var);
         let removed: Vec<String> = cmd
             .get_envs()
             .filter(|(_, value)| value.is_none())
@@ -445,7 +458,7 @@ mod cache_env_selftests {
         for name in &removed {
             assert!(is_ambient_vlt_var(name), "{name} was removed");
         }
-        for (name, _) in std::env::vars_os() {
+        for name in &parent {
             let name = name.to_string_lossy();
             if is_ambient_vlt_var(&name) {
                 assert!(removed.iter().any(|r| *r == name), "{name} kept");