test: stop process-global test state from failing unrelated tests

src/env.rs

@@ -8,7 +8,7 @@
 use std::ffi::{OsStr, OsString};
 
 #[cfg(any(test, feature = "test-support"))]
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 #[cfg(any(test, feature = "test-support"))]
 use std::sync::{OnceLock, RwLock};
 
@@ -79,14 +79,90 @@ pub fn remove_test_var<K: AsRef<OsStr>>(key: K) {
         .insert(key.as_ref().to_os_string(), None);
 }
 
-/// Resolve an executable only when tests explicitly override `PATH`.
+// Program names that must resolve as missing, regardless of what is really on
+// `PATH`. See [`mask_test_programs`].
+#[cfg(any(test, feature = "test-support"))]
+static MASKED_PROGRAMS: OnceLock<RwLock<HashSet<OsString>>> = OnceLock::new();
+
+/// Directory component that cannot exist, so a masked program spawns with the
+/// same `ErrorKind::NotFound` a genuinely absent binary would produce. Any path
+/// with more than one component bypasses the platform's `PATH` search entirely.
+#[cfg(any(test, feature = "test-support"))]
+const MASKED_PROGRAM_DIR: &str = "cortex-test-masked-program-does-not-exist";
+
+/// Make specific bare program names resolve as missing until the returned guard
+/// is dropped.
+///
+/// This exists so a test asserting "this command is not installed" does not have
+/// to replace `PATH` to say so. The override map is process-global, so replacing
+/// `PATH` with a fixture directory takes every *other* test's subprocess spawns
+/// down with it — they stop finding `sh`, `git`, and everything else — and under
+/// plain `cargo test` those tests run concurrently. Masking states the actual
+/// intent instead, and scopes the blast radius to the named programs.
+///
+/// The mask is still process-global: only mask programs whose spawning tests are
+/// serialized against this one.
+#[cfg(any(test, feature = "test-support"))]
+#[doc(hidden)]
+#[must_use = "the mask is lifted when the guard is dropped"]
+pub fn mask_test_programs<I, S>(names: I) -> MaskedPrograms
+where
+    I: IntoIterator<Item = S>,
+    S: AsRef<OsStr>,
+{
+    let names: Vec<OsString> = names
+        .into_iter()
+        .map(|name| name.as_ref().to_os_string())
+        .collect();
+    let mut masked = MASKED_PROGRAMS
+        .get_or_init(|| RwLock::new(HashSet::new()))
+        .write()
+        .expect("masked program set lock poisoned");
+    for name in &names {
+        masked.insert(name.clone());
+    }
+    drop(masked);
+    MaskedPrograms(names)
+}
+
+/// Guard returned by [`mask_test_programs`]; lifts the mask on drop.
+#[cfg(any(test, feature = "test-support"))]
+#[doc(hidden)]
+pub struct MaskedPrograms(Vec<OsString>);
+
+#[cfg(any(test, feature = "test-support"))]
+impl Drop for MaskedPrograms {
+    fn drop(&mut self) {
+        if let Some(masked) = MASKED_PROGRAMS.get() {
+            let mut masked = masked.write().expect("masked program set lock poisoned");
+            for name in &self.0 {
+                masked.remove(name);
+            }
+        }
+    }
+}
+
+#[cfg(any(test, feature = "test-support"))]
+fn program_is_masked(program: &OsStr) -> bool {
+    MASKED_PROGRAMS.get().is_some_and(|masked| {
+        masked
+            .read()
+            .expect("masked program set lock poisoned")
+            .contains(program)
+    })
+}
+
+/// Resolve an executable when tests mask it or explicitly override `PATH`.
 /// Normal test-support builds keep the platform's native command lookup semantics.
 #[cfg(any(test, feature = "test-support"))]
 fn resolve_test_program(program: &OsStr) -> Option<std::path::PathBuf> {
     let program_path = std::path::Path::new(program);
     if program_path.components().count() != 1 {
         return None;
     }
+    if program_is_masked(program) {
+        return Some(std::path::Path::new(MASKED_PROGRAM_DIR).join(program));
+    }
     let search_path = test_override(OsStr::new("PATH"))??;
     std::env::split_paths(&search_path)
         .map(|dir| dir.join(program))
@@ -180,6 +256,66 @@ mod tests {
         assert!(var_os(KEY).is_none());
     }
 
+    /// Masking must make a program that *is* on `PATH` resolve to something that
+    /// cannot be spawned, and must lift cleanly — otherwise it becomes the same
+    /// process-global leak that replacing `PATH` was.
+    #[cfg(unix)]
+    #[test]
+    fn masking_hides_a_resolvable_program_and_lifts_on_drop() {
+        // Restore the effective PATH: `remove_test_var` masks the key outright,
+        // which would leave every later test in this binary with no PATH at all.
+        struct PathGuard(Option<OsString>);
+        impl Drop for PathGuard {
+            fn drop(&mut self) {
+                match self.0.take() {
+                    Some(previous) => set_test_var("PATH", previous),
+                    None => remove_test_var("PATH"),
+                }
+            }
+        }
+
+        // A name nothing else in this binary spawns, so the process-global mask
+        // cannot disturb a concurrently running test.
+        const PROGRAM: &str = "cortex-test-mask-probe";
+
+        let dir = tempfile::tempdir().unwrap();
+        let binary = dir.path().join(PROGRAM);
+        std::fs::write(&binary, "#!/bin/sh\nexit 0\n").unwrap();
+        use std::os::unix::fs::PermissionsExt;
+        std::fs::set_permissions(&binary, std::fs::Permissions::from_mode(0o755)).unwrap();
+
+        // Prepend, never replace: a bare fixture PATH breaks unrelated spawns.
+        let mut search_path = vec![dir.path().to_path_buf()];
+        if let Some(existing) = var_os("PATH") {
+            search_path.extend(std::env::split_paths(&existing));
+        }
+        let _path = PathGuard(var_os("PATH"));
+        set_test_var("PATH", std::env::join_paths(search_path).unwrap());
+
+        assert_eq!(
+            resolve_test_program(OsStr::new(PROGRAM)).as_deref(),
+            Some(binary.as_path()),
+            "the fixture directory should resolve the program before masking"
+        );
+
+        {
+            let _masked = mask_test_programs([PROGRAM]);
+            let resolved = resolve_test_program(OsStr::new(PROGRAM))
+                .expect("a masked program still resolves, to a path that cannot exist");
+            assert!(
+                !resolved.exists(),
+                "masking must resolve to an absent path so the spawn fails with \
+                 NotFound, exactly as an uninstalled binary does"
+            );
+        }
+
+        assert_eq!(
+            resolve_test_program(OsStr::new(PROGRAM)).as_deref(),
+            Some(binary.as_path()),
+            "dropping the guard must lift the mask"
+        );
+    }
+
     #[test]
     fn unrelated_keys_do_not_interfere() {
         const LEFT: &str = "CORTEX_TEST_ENV_OVERLAY_LEFT";

src/heartbeat_agent_tests.rs

@@ -720,6 +720,7 @@ fn transcript_forward_env_does_not_gate_local_sessions_watch_service() {
 /// dead or permission-denied daemon — a CI container, for instance — reported a
 /// probe error instead. Both are the same fact about the host.
 #[tokio::test]
+#[serial]
 async fn container_probe_reports_unreachable_when_docker_ps_fails() {
     let dir = tempfile::tempdir().unwrap();
     let fake_docker = dir.path().join("docker");
@@ -733,7 +734,17 @@ async fn container_probe_reports_unreachable_when_docker_ps_fails() {
         use std::os::unix::fs::PermissionsExt;
         std::fs::set_permissions(&fake_docker, std::fs::Permissions::from_mode(0o755)).unwrap();
     }
-    crate::env::set_test_var("PATH", dir.path().to_str().unwrap());
+    // `PATH` overrides live in a process-global map that every subprocess spawn
+    // in this test binary resolves against, so this has to both prepend rather
+    // than replace and restore on drop. A bare, unrestored fixture directory
+    // outlives the test and leaves every later `sh`, `git`, or `docker` spawn
+    // in the binary resolving against a deleted temp dir. `cargo nextest` hides
+    // that behind one process per test; plain `cargo test` does not.
+    let mut search_path = vec![dir.path().to_path_buf()];
+    if let Some(existing) = crate::env::var_os("PATH") {
+        search_path.extend(std::env::split_paths(&existing));
+    }
+    let _path = EnvGuard::set("PATH", std::env::join_paths(search_path).unwrap());
 
     let output = LinuxContainerProbe.collect().await.expect(
         "a failing `docker ps` is an unreachable runtime, not a probe error — \

src/inventory/device_tests.rs

@@ -117,7 +117,20 @@ async fn collect_warns_when_optional_device_commands_are_missing() {
     let bin_dir = dir.path().join("bin");
     std::fs::create_dir_all(&bin_dir).unwrap();
     executable_file(&bin_dir.join("hostname"), "#!/bin/sh\nprintf '\\n'\n");
-    let _path_guard = EnvGuard::set("PATH", bin_dir.as_os_str());
+    // Mask the optional commands instead of replacing `PATH` with `bin_dir`.
+    // `PATH` overrides are process-global, so wiping it here also stopped every
+    // concurrently-running test in this binary from finding `sh`, `git`, and
+    // friends. Masking says what this test actually means — these four commands
+    // are not installed — and leaves every other program resolving normally.
+    // Safe to mask process-wide because `inventory::device` is the only caller
+    // of them and its tests are `#[serial]` with each other.
+    let _masked = crate::env::mask_test_programs(["uname", "ip", "ss", "df"]);
+    let path = format!(
+        "{}:{}",
+        bin_dir.display(),
+        crate::env::var("PATH").unwrap_or_default()
+    );
+    let _path_guard = EnvGuard::set("PATH", path);
 
     let output = collect(std::time::Duration::from_millis(50)).await;