diff --git a/flake.nix b/flake.nix index e3e5ea7..532e30c 100644 --- a/flake.nix +++ b/flake.nix @@ -44,10 +44,6 @@ # an OS sandbox inside the Nix build sandbox, which is unavailable there. cargoTestFlags = [ "--lib" ]; - # Several unit tests mutate HOME and fail when run in parallel - # (reproducible with plain `cargo test --lib` on a many-core machine). - dontUseCargoParallelTests = true; - meta = { description = "Credential broker for AI agents — tools get your secrets, the agent never does"; homepage = "https://github.com/ModernPath/airlock"; diff --git a/src/client.rs b/src/client.rs index 7d6515a..6f9d12f 100644 --- a/src/client.rs +++ b/src/client.rs @@ -424,12 +424,12 @@ mod tests { // ── Helpers ────────────────────────────────────────────────────────── - use std::sync::{Mutex, MutexGuard}; + use std::sync::MutexGuard; - /// Global mutex serializing tests that modify env vars. - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - - /// RAII guard for temporary environment variable overrides. + /// RAII guard for temporary environment variable overrides. Holds + /// [`crate::test_support::ENV_MUTEX`] — the crate-wide lock — so these + /// tests serialize against every other test that touches the process + /// environment, not just the ones in this module. struct TempEnvVar { key: String, prev: Option, @@ -438,8 +438,13 @@ mod tests { impl TempEnvVar { fn new(key: &str, value: &str) -> Self { - let lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + let lock = crate::test_support::ENV_MUTEX + .lock() + .unwrap_or_else(|e| e.into_inner()); let prev = std::env::var(key).ok(); + // SAFETY: we hold the crate-wide ENV_MUTEX, so no other test + // thread anywhere in the suite is reading or writing env vars + // concurrently. unsafe { std::env::set_var(key, value) }; Self { key: key.to_string(), diff --git a/src/exec.rs b/src/exec.rs index 2336cde..baf14e5 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -587,6 +587,15 @@ mod tests { use std::time::Duration; use super::*; + use crate::test_support::ENV_MUTEX; + + /// Hold the crate-wide env lock. The tests below compare `build_env()`'s + /// snapshot of the environment against a second `std::env::var` read of the + /// same variable; without the lock a concurrent test mutating `HOME` or + /// `LANG` can change the answer between the two reads. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { + ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()) + } // ── Binary resolution ──────────────────────────────────────────────────── @@ -704,6 +713,8 @@ mod tests { /// The output map contains `PATH` with the value from the daemon's environment. #[test] fn build_env_contains_path_from_daemon_env() { + let _guard = env_lock(); + // PATH is almost certainly set in any test environment; guard anyway. let Ok(expected_path) = std::env::var("PATH") else { // PATH is not set — nothing to verify. @@ -722,6 +733,8 @@ mod tests { /// in the daemon's environment. #[test] fn build_env_contains_essential_vars_when_present_in_daemon_env() { + let _guard = env_lock(); + let env = build_env(&[]); for var in &["HOME", "TERM", "LANG", "USER"] { @@ -752,6 +765,8 @@ mod tests { /// 2. Every key is either a declared secret or an essential variable. #[test] fn build_env_has_exactly_secrets_plus_present_essential_vars() { + let _guard = env_lock(); + let secrets = vec![ ("SECRET_ALPHA".to_string(), "value_a".to_string()), ("SECRET_BETA".to_string(), "value_b".to_string()), @@ -837,6 +852,8 @@ mod tests { /// not appear. #[test] fn build_env_omits_absent_essential_vars_without_error() { + let _guard = env_lock(); + let env = build_env(&[]); match std::env::var("TERM") { diff --git a/src/secrets.rs b/src/secrets.rs index 3e2859c..e075ea8 100644 --- a/src/secrets.rs +++ b/src/secrets.rs @@ -379,22 +379,18 @@ mod tests { use super::*; use std::collections::HashMap; use std::path::PathBuf; - use std::sync::{Mutex, MutexGuard}; + use std::sync::MutexGuard; use std::time::Duration; use crate::config::{Config, SecretSpec}; // ── Helpers ────────────────────────────────────────────────────────── - /// Global mutex that serializes all tests that modify environment variables. - /// - /// Environment variables are process-global state. Without serialization, - /// concurrent tests that modify env vars race against each other, producing - /// flaky failures. - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - /// RAII guard that sets environment variables for the duration of a test - /// and restores them when dropped. Also holds the [`ENV_MUTEX`] lock. + /// and restores them when dropped. Holds + /// [`crate::test_support::ENV_MUTEX`] — the crate-wide lock — so these + /// tests serialize against every other test that touches the process + /// environment, not just the ones in this module. struct EnvGuard { vars: Vec<(String, Option)>, _lock: MutexGuard<'static, ()>, @@ -404,14 +400,17 @@ mod tests { /// Set the given environment variables, saving their previous values /// for restoration on drop. fn new(vars: &[(&str, &str)]) -> Self { - let lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + let lock = crate::test_support::ENV_MUTEX + .lock() + .unwrap_or_else(|e| e.into_inner()); let mut saved = Vec::with_capacity(vars.len()); for (key, value) in vars { let prev = std::env::var(*key).ok(); saved.push((key.to_string(), prev)); - // SAFETY: We hold ENV_MUTEX, so no other test thread is reading - // or writing env vars concurrently within this test module. + // SAFETY: we hold the crate-wide ENV_MUTEX, so no other test + // thread anywhere in the suite is reading or writing env vars + // concurrently. unsafe { std::env::set_var(*key, *value) }; } @@ -424,7 +423,9 @@ mod tests { /// Acquire the env mutex without setting any variables. Useful when /// we need to set and clear in a specific order within the test body. fn lock_only() -> Self { - let lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + let lock = crate::test_support::ENV_MUTEX + .lock() + .unwrap_or_else(|e| e.into_inner()); Self { vars: Vec::new(), _lock: lock,