test(signature): serialize registry-auth env tests to end cross-test env bleed (JEF-412)

engine/src/policies/signature/auth_tests.rs

@@ -1,12 +1,79 @@
 //! Tests for the shared per-image registry-auth resolver (JEF-339, JEF-352).
 //!
-//! The env-mutating cases rely on nextest's per-test process isolation, so each test owns a
-//! clean process env; the `unsafe { set_var }` blocks mirror the pattern the rest of the
-//! crate's env tests use.
+//! [`RegistryAuth::from_env`] reads three **process-global** env vars
+//! (`PROTECTOR_REGISTRY_USERNAME` / `PROTECTOR_REGISTRY_PASSWORD` /
+//! `PROTECTOR_REGISTRY_AUTH_FILE`). Rust's default test harness runs tests as parallel
+//! threads within one process, so a `set_var`/`remove_var` in one test is visible to any
+//! sibling that reads the same var mid-flight — a data race that made these cases flaky
+//! (JEF-412). Every case that touches those vars therefore goes through [`EnvGuard`], which
+//! serializes them on a shared lock and restores the prior values on drop (even on panic),
+//! so no test can observe another's env mutation.
+
+use std::sync::{Mutex, MutexGuard};
 
 use super::RegistryAuth;
 use sigstore::registry::Auth;
 
+/// The three process-global env vars [`RegistryAuth::from_env`] reads. [`EnvGuard`] snapshots
+/// and restores exactly these, and holds the lock for the whole set → read → assert window.
+const CRED_ENV_VARS: [&str; 3] = [
+    "PROTECTOR_REGISTRY_USERNAME",
+    "PROTECTOR_REGISTRY_PASSWORD",
+    "PROTECTOR_REGISTRY_AUTH_FILE",
+];
+
+/// Serializes every test that mutates the credential env vars. Without this, `cargo test` runs
+/// these cases as concurrent threads in one process and their `set_var`/`remove_var` calls race.
+static ENV_LOCK: Mutex<()> = Mutex::new(());
+
+/// RAII guard that gives a test exclusive, restore-on-drop access to the credential env vars.
+///
+/// On construction it takes [`ENV_LOCK`] (so only one env-mutating test runs at a time),
+/// snapshots the current value of each [`CRED_ENV_VARS`] entry, then clears them all so the test
+/// starts from a known-clean env. On drop it restores the snapshot — even if the test panics —
+/// so a failing assertion can never leak creds into a sibling.
+struct EnvGuard {
+    _lock: MutexGuard<'static, ()>,
+    saved: [(&'static str, Option<String>); 3],
+}
+
+impl EnvGuard {
+    /// Acquire the lock, snapshot the credential env vars, and clear them to a known-clean state.
+    fn acquire() -> Self {
+        // A prior test that panicked mid-window would poison the lock; recover the guard so one
+        // legitimate assertion failure doesn't cascade into spurious failures for later tests.
+        let lock = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
+        let saved = CRED_ENV_VARS.map(|k| (k, std::env::var(k).ok()));
+        // SAFETY: the lock guarantees no sibling test reads/writes these vars concurrently.
+        unsafe {
+            for (k, _) in &saved {
+                std::env::remove_var(k);
+            }
+        }
+        Self { _lock: lock, saved }
+    }
+
+    /// Set one credential env var within the guarded window.
+    fn set<V: AsRef<std::ffi::OsStr>>(&self, key: &str, value: V) {
+        // SAFETY: the guard holds the lock, so no sibling test races this mutation.
+        unsafe { std::env::set_var(key, value) };
+    }
+}
+
+impl Drop for EnvGuard {
+    fn drop(&mut self) {
+        // SAFETY: still holding the lock; restore each var to its pre-test value.
+        unsafe {
+            for (key, prior) in &self.saved {
+                match prior {
+                    Some(v) => std::env::set_var(key, v),
+                    None => std::env::remove_var(key),
+                }
+            }
+        }
+    }
+}
+
 /// Assert `auth` is `Basic(user, pass)`; panics with the actual variant otherwise.
 #[track_caller]
 fn assert_basic(auth: Auth, user: &str, pass: &str) {
@@ -32,16 +99,6 @@ fn write_config(tag: &str, contents: &str) -> std::path::PathBuf {
     path
 }
 
-/// Clear every credential env var so a test starts from a known-clean process env.
-/// SAFETY: nextest runs each test in its own process, so this mutation is isolated.
-fn clear_cred_env() {
-    unsafe {
-        std::env::remove_var("PROTECTOR_REGISTRY_USERNAME");
-        std::env::remove_var("PROTECTOR_REGISTRY_PASSWORD");
-        std::env::remove_var("PROTECTOR_REGISTRY_AUTH_FILE");
-    }
-}
-
 /// A multi-registry k8s `.dockerconfigjson`: ghcr.io (base64 `auth`), Docker Hub keyed by the
 /// legacy `https://index.docker.io/v1/` URL, and a private `host:port` registry.
 /// base64("thejefflarson:ghp_token")   = dGhlamVmZmxhcnNvbjpnaHBfdG9rZW4=
@@ -58,8 +115,8 @@ const MULTI_REGISTRY: &str = r#"{"auths":{
 #[test]
 fn resolves_per_image_across_multiple_registries() {
     let path = write_config("multi", MULTI_REGISTRY);
-    clear_cred_env();
-    unsafe { std::env::set_var("PROTECTOR_REGISTRY_AUTH_FILE", &path) };
+    let env = EnvGuard::acquire();
+    env.set("PROTECTOR_REGISTRY_AUTH_FILE", &path);
 
     let auth = RegistryAuth::from_env();
     assert_basic(
@@ -86,8 +143,8 @@ fn resolves_per_image_across_multiple_registries() {
 #[test]
 fn docker_io_variants_match_the_index_v1_config_key() {
     let path = write_config("dockerio", MULTI_REGISTRY);
-    clear_cred_env();
-    unsafe { std::env::set_var("PROTECTOR_REGISTRY_AUTH_FILE", &path) };
+    let env = EnvGuard::acquire();
+    env.set("PROTECTOR_REGISTRY_AUTH_FILE", &path);
 
     let auth = RegistryAuth::from_env();
     for image in [
@@ -112,8 +169,8 @@ fn ghcr_private_image_still_resolves_basic_from_the_auth_file() {
         "ghcr",
         r#"{"auths":{"ghcr.io":{"auth":"dGhlamVmZmxhcnNvbjpnaHBfdG9rZW4="}}}"#,
     );
-    clear_cred_env();
-    unsafe { std::env::set_var("PROTECTOR_REGISTRY_AUTH_FILE", &path) };
+    let env = EnvGuard::acquire();
+    env.set("PROTECTOR_REGISTRY_AUTH_FILE", &path);
 
     let auth = RegistryAuth::from_env();
     assert_basic(
@@ -132,8 +189,8 @@ fn explicit_username_password_wins_over_auth_field() {
         "explicit",
         r#"{"auths":{"ghcr.io":{"username":"bot","password":"pw","auth":"aWdub3JlZDppZ25vcmVk"}}}"#,
     );
-    clear_cred_env();
-    unsafe { std::env::set_var("PROTECTOR_REGISTRY_AUTH_FILE", &path) };
+    let env = EnvGuard::acquire();
+    env.set("PROTECTOR_REGISTRY_AUTH_FILE", &path);
 
     let auth = RegistryAuth::from_env();
     assert_basic(auth.for_image("ghcr.io/org/app"), "bot", "pw");
@@ -146,12 +203,10 @@ fn explicit_username_password_wins_over_auth_field() {
 #[test]
 fn env_override_applies_across_all_registries() {
     let path = write_config("envoverride", MULTI_REGISTRY);
-    clear_cred_env();
-    unsafe {
-        std::env::set_var("PROTECTOR_REGISTRY_USERNAME", "envuser");
-        std::env::set_var("PROTECTOR_REGISTRY_PASSWORD", "envpass");
-        std::env::set_var("PROTECTOR_REGISTRY_AUTH_FILE", &path);
-    }
+    let env = EnvGuard::acquire();
+    env.set("PROTECTOR_REGISTRY_USERNAME", "envuser");
+    env.set("PROTECTOR_REGISTRY_PASSWORD", "envpass");
+    env.set("PROTECTOR_REGISTRY_AUTH_FILE", &path);
 
     let auth = RegistryAuth::from_env();
     // ghcr.io has a file entry, yet the env override still wins…
@@ -165,7 +220,7 @@ fn env_override_applies_across_all_registries() {
 /// With no credentials of any kind, every image resolves to Anonymous — the safe default.
 #[test]
 fn anonymous_without_any_credentials() {
-    clear_cred_env();
+    let _env = EnvGuard::acquire();
     let auth = RegistryAuth::from_env();
     assert!(matches!(auth.for_image("ghcr.io/org/app"), Auth::Anonymous));
     assert!(matches!(
@@ -177,13 +232,11 @@ fn anonymous_without_any_credentials() {
 /// A missing/unparseable auth file never errors — it degrades to Anonymous per image.
 #[test]
 fn missing_auth_file_degrades_to_anonymous() {
-    clear_cred_env();
-    unsafe {
-        std::env::set_var(
-            "PROTECTOR_REGISTRY_AUTH_FILE",
-            "/nonexistent/protector/registry/config.json",
-        )
-    };
+    let env = EnvGuard::acquire();
+    env.set(
+        "PROTECTOR_REGISTRY_AUTH_FILE",
+        "/nonexistent/protector/registry/config.json",
+    );
     let auth = RegistryAuth::from_env();
     assert!(matches!(auth.for_image("ghcr.io/org/app"), Auth::Anonymous));
 }