Keep Go unit tests off the host's Go env

crates/socket-patch-core/src/test_env.rs

@@ -4,6 +4,22 @@
 //! must also be `#[serial_test::serial]`.
 
 use std::ffi::{OsStr, OsString};
+use std::sync::{Mutex, MutexGuard, PoisonError};
+
+/// The keys live guards hold, once per guard entry.
+static GUARDED: Mutex<Vec<OsString>> = Mutex::new(Vec::new());
+
+fn guarded() -> MutexGuard<'static, Vec<OsString>> {
+    GUARDED.lock().unwrap_or_else(PoisonError::into_inner)
+}
+
+/// `key`'s value while an [`EnvGuard`] holds it, else `None` whatever the
+/// host set. For lookups that must not see the developer's environment
+/// (`utils::go_env` in the unit tests).
+pub(crate) fn guarded_var(key: &str) -> Option<String> {
+    let held = guarded().iter().any(|k| k == key);
+    held.then(|| std::env::var(key).ok()).flatten()
+}
 
 /// Sets or removes env vars and puts the previous values back on drop, a
 /// panicking assertion included. Previous values are kept as `OsString`,
@@ -33,6 +49,7 @@ impl EnvGuard {
         let mut guard = Self { saved: Vec::new() };
         for (key, value) in pairs {
             let key = key.as_ref();
+            guarded().push(key.to_os_string());
             guard
                 .saved
                 .push((key.to_os_string(), std::env::var_os(key)));
@@ -53,6 +70,10 @@ impl Drop for EnvGuard {
                 Some(v) => std::env::set_var(&key, v),
                 None => std::env::remove_var(&key),
             }
+            let mut held = guarded();
+            if let Some(i) = held.iter().rposition(|k| *k == key) {
+                held.swap_remove(i);
+            }
         }
     }
 }

crates/socket-patch-core/src/test_env_tests.rs

@@ -47,3 +47,22 @@ fn keeps_a_non_utf8_previous_value() {
     assert_eq!(std::env::var_os(KEY).as_deref(), Some(raw));
     std::env::remove_var(KEY);
 }
+
+#[test]
+#[serial_test::serial]
+fn guarded_var_sees_only_vars_a_live_guard_holds() {
+    std::env::set_var(KEY, "host");
+    assert_eq!(crate::test_env::guarded_var(KEY), None, "host value hidden");
+    {
+        let _outer = EnvGuard::set(KEY, "a");
+        assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a"));
+        {
+            let _inner = EnvGuard::unset(KEY);
+            assert_eq!(crate::test_env::guarded_var(KEY), None);
+        }
+        // The outer guard still holds it once the inner one drops.
+        assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a"));
+    }
+    assert_eq!(crate::test_env::guarded_var(KEY), None, "released on drop");
+    std::env::remove_var(KEY);
+}

crates/socket-patch-core/src/utils/go_env.rs

@@ -24,7 +24,22 @@ use std::path::{Path, PathBuf};
 /// caller applies go's default). An empty environment variable falls
 /// through to the file, as go's own `cfg.Getenv` does.
 pub(crate) fn go_env(key: &str) -> Option<String> {
-    go_env_with(key, |k| std::env::var(k).ok())
+    go_env_with(key, process_env)
+}
+
+/// The process environment [`go_env`] reads.
+#[cfg(not(test))]
+fn process_env(key: &str) -> Option<String> {
+    std::env::var(key).ok()
+}
+
+/// In the unit tests, only the vars a test holds with an
+/// [`EnvGuard`](crate::test_env::EnvGuard). A developer's `GOWORK=off`,
+/// `GOFLAGS=-modfile=...` or `go env -w` file (found through `HOME`) would
+/// otherwise change what every Go fixture resolves.
+#[cfg(test)]
+fn process_env(key: &str) -> Option<String> {
+    crate::test_env::guarded_var(key)
 }
 
 /// [`go_env`] over an injected environment lookup.

crates/socket-patch-core/src/vendor/registry_fetch.rs

@@ -1546,24 +1546,17 @@ mod tests {
         // GOPROXY is a comma- OR pipe-separated list (go help goproxy); a
         // pipe-separated value must yield the first usable proxy, not a
         // `https://a|b`-shaped base that builds an unparseable URL.
-        let saved_socket = std::env::var("SOCKET_GOPROXY").ok();
-        let saved = std::env::var("GOPROXY").ok();
-        std::env::remove_var("SOCKET_GOPROXY");
-        std::env::set_var(
-            "GOPROXY",
-            "https://athens.example|https://proxy.golang.org|direct",
-        );
+        let _env = crate::test_env::EnvGuard::many([
+            ("SOCKET_GOPROXY", None),
+            (
+                "GOPROXY",
+                Some("https://athens.example|https://proxy.golang.org|direct"),
+            ),
+        ]);
         let piped = goproxy_base(MODULE);
-        std::env::set_var("GOPROXY", "https://mirror.example/,direct");
+        let _mixed_env =
+            crate::test_env::EnvGuard::set("GOPROXY", "https://mirror.example/,direct");
         let mixed = goproxy_base(MODULE);
-        match saved {
-            Some(v) => std::env::set_var("GOPROXY", v),
-            None => std::env::remove_var("GOPROXY"),
-        }
-        match saved_socket {
-            Some(v) => std::env::set_var("SOCKET_GOPROXY", v),
-            None => std::env::remove_var("SOCKET_GOPROXY"),
-        }
         assert_eq!(piped.as_deref(), Ok("https://athens.example"));
         assert_eq!(mixed.as_deref(), Ok("https://mirror.example"));
     }
@@ -1671,17 +1664,14 @@ mod tests {
             "GONOPROXY",
             "SOCKET_GOPROXY",
         ];
-        let saved: Vec<_> = keys.iter().map(|k| (*k, std::env::var(k).ok())).collect();
+        let _env = crate::test_env::EnvGuard::many(keys.map(|k| (k, None::<&str>)));
         let dir = tempfile::tempdir().unwrap();
         let file = dir.path().join("env");
         std::fs::write(
             &file,
             "GOPROXY=http://127.0.0.1:18702\nGOPRIVATE=example.com\n",
         )
         .unwrap();
-        for k in &keys[1..] {
-            std::env::remove_var(k);
-        }
         std::env::set_var("GOENV", &file);
         let private = goproxy_base("example.com/upstream");
         let public = goproxy_base("golang.org/x/text");
@@ -1690,12 +1680,6 @@ mod tests {
         std::env::set_var("GOENV", "off");
         std::env::remove_var("GOPROXY");
         let off = goproxy_base("example.com/upstream");
-        for (k, v) in saved {
-            match v {
-                Some(v) => std::env::set_var(k, v),
-                None => std::env::remove_var(k),
-            }
-        }
         let err = private.unwrap_err();
         assert!(err.contains("matches GOPRIVATE"), "{err}");
         assert_eq!(public.as_deref(), Ok("http://127.0.0.1:18702"));
@@ -1707,11 +1691,12 @@ mod tests {
     #[serial_test::serial]
     fn goproxy_base_env_precedence() {
         const MODULE: &str = "example.com/m";
-        let saved_socket = std::env::var("SOCKET_GOPROXY").ok();
-        let saved = std::env::var("GOPROXY").ok();
         // No `go env -w` file: the developer's own must not leak in.
-        let saved_goenv = std::env::var("GOENV").ok();
-        std::env::set_var("GOENV", "off");
+        let _env = crate::test_env::EnvGuard::many([
+            ("SOCKET_GOPROXY", None),
+            ("GOPROXY", None),
+            ("GOENV", Some("off")),
+        ]);
 
         // SOCKET_GOPROXY wins over GOPROXY (trailing slash trimmed).
         std::env::set_var("SOCKET_GOPROXY", "https://socket.example/");
@@ -1729,19 +1714,6 @@ mod tests {
         // default proxy.
         std::env::set_var("GOPROXY", "direct,off");
         let no_proxy = goproxy_base(MODULE);
-
-        match saved_socket {
-            Some(v) => std::env::set_var("SOCKET_GOPROXY", v),
-            None => std::env::remove_var("SOCKET_GOPROXY"),
-        }
-        match saved {
-            Some(v) => std::env::set_var("GOPROXY", v),
-            None => std::env::remove_var("GOPROXY"),
-        }
-        match saved_goenv {
-            Some(v) => std::env::set_var("GOENV", v),
-            None => std::env::remove_var("GOENV"),
-        }
         assert_eq!(socket_wins.as_deref(), Ok("https://socket.example"));
         assert_eq!(
             empty_falls_through.as_deref(),