Prevent Windows test PATH leaks into User PATH

CHANGELOG.md

@@ -18,6 +18,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Windows User PATH pollution from acceptance tests**: `PathRestoreGuard` sets
+  a test-only flag that makes `persist_path_dir` / Unix `persist_user_path`
+  skip durable User PATH, `~/.local/bin/nu`, and shell-profile writes while the
+  guard is held (it does not rewrite the Windows User PATH registry on drop, so
+  concurrent external PATH edits are preserved). `numan setup nu` / `use` also
+  refuse to persist paths under the system temp folder (fail closed if the temp
+  root cannot be canonicalized), so tempfile fixtures like `Temp\.tmp*\off`
+  cannot land on PATH again. If you already have those leftovers, clean User
+  PATH (PowerShell):
+
+  ```powershell
+  $userPath = [Environment]::GetEnvironmentVariable('Path', 'User')
+  $cleaned = ($userPath -split ';' | Where-Object {
+    $_ -and ($_ -notmatch '(?i)\\Temp\\.tmp.*\\(off|existing-nu)$')
+  }) -join ';'
+  [Environment]::SetEnvironmentVariable('Path', $cleaned, 'User')
+  ```
+
+  Then open a new terminal and confirm with `$env:Path -split ';'`.
+
 - **`numan setup nu`**: official Nushell 0.114.x release archives exceed the old
   256 MiB extract cap (~279 MiB uncompressed on linux-gnu). Bootstrap now
   extracts only the `nu` binary (skipping bundled plugins) and raises the

src/nu/bootstrap.rs

@@ -435,6 +435,21 @@ pub fn persist_user_path(binary: &Path) -> Result<()> {
     }
     #[cfg(unix)]
     {
+        // Same test harness skip as `persist_path_dir_*`: PathRestoreGuard sets
+        // this so ignored acceptance tests cannot leave a dangling
+        // `~/.local/bin/nu` or shell-profile export after a tempfile fixture.
+        if std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH").is_some() {
+            return Ok(());
+        }
+        // Match Windows `persist_path_dir` temp refuse: never durable-link a
+        // tempfile-rooted binary into `~/.local/bin/nu`.
+        if path_is_under_temp_dir(binary) {
+            bail!(
+                "Refusing to add temporary directory '{}' to the user PATH. \
+                 Install or register a stable Nushell location instead.",
+                binary.display()
+            );
+        }
         persist_user_path_unix(binary)?;
         ensure_local_bin_on_path()?;
         Ok(())
@@ -569,7 +584,21 @@ pub fn register_existing_nu(binary: &Path, options: &NuSetupOptions) -> Result<P
 
 #[cfg(windows)]
 fn persist_path_dir_windows(dir: &Path) -> Result<()> {
+    // Test harness sets this while PathRestoreGuard is held so ignored
+    // acceptance tests cannot permanently pollute the developer User PATH.
+    if std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH").is_some() {
+        return Ok(());
+    }
     let dir = normalize_path_entry(dir);
+    // Refuse tempfile roots: test fixtures and one-off extracts must not land
+    // on the durable User PATH (seen as Temp\.tmp*\off / existing-nu leaks).
+    if path_is_under_temp_dir(&dir) {
+        bail!(
+            "Refusing to add temporary directory '{}' to the user PATH. \
+             Install or register a stable Nushell location instead.",
+            dir.display()
+        );
+    }
     let dir_str = dir
         .to_str()
         .with_context(|| format!("PATH entry '{}' is not valid UTF-8", dir.display()))?;
@@ -586,6 +615,28 @@ fn persist_path_dir_windows(dir: &Path) -> Result<()> {
     Ok(())
 }
 
+fn path_is_under_temp_dir(dir: &Path) -> bool {
+    path_is_under_temp_dir_with(dir, &std::env::temp_dir())
+}
+
+/// Returns true when `dir` sits under `temp_raw`, failing closed if either
+/// path cannot be canonicalized (lexical `starts_with` fallback).
+fn path_is_under_temp_dir_with(dir: &Path, temp_raw: &Path) -> bool {
+    let Ok(temp) = temp_raw.canonicalize() else {
+        // Fail closed: an unresolvable temp root must still refuse lexical
+        // children (same fallback as an uncanonicalizable `dir`).
+        return match dir.canonicalize() {
+            Ok(d) => d.starts_with(temp_raw),
+            Err(_) => dir.starts_with(temp_raw),
+        };
+    };
+    let Ok(dir) = dir.canonicalize() else {
+        // If the dir vanished, still treat literal temp prefixes as unsafe.
+        return dir.starts_with(temp_raw);
+    };
+    dir.starts_with(&temp)
+}
+
 #[cfg(unix)]
 fn shell_escape_for_double_quotes(value: &str) -> String {
     let mut escaped = String::with_capacity(value.len());
@@ -603,6 +654,16 @@ fn shell_escape_for_double_quotes(value: &str) -> String {
 
 #[cfg(unix)]
 fn persist_path_dir_unix(dir: &Path) -> Result<()> {
+    if std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH").is_some() {
+        return Ok(());
+    }
+    if path_is_under_temp_dir(dir) {
+        bail!(
+            "Refusing to add temporary directory '{}' to the user PATH. \
+             Install or register a stable Nushell location instead.",
+            dir.display()
+        );
+    }
     let dir_str = dir
         .to_str()
         .with_context(|| format!("PATH entry '{}' is not valid UTF-8", dir.display()))?;
@@ -1099,6 +1160,98 @@ mod tests {
         );
     }
 
+    #[test]
+    fn persist_path_dir_refuses_temp_directories() {
+        use crate::util::test_paths::PathRestoreGuard;
+        // Hold the PATH mutex so concurrent tests do not race on the env flag.
+        let _guard = PathRestoreGuard::new();
+        let dir = TempDir::new().unwrap();
+        let nested = dir.path().join("off");
+        std::fs::create_dir_all(&nested).unwrap();
+        // Clear the test harness skip flag so we exercise the production refuse.
+        std::env::remove_var("NUMAN_TEST_NO_PERSIST_USER_PATH");
+        let err = persist_path_dir(&nested).expect_err("temp dirs must not be persisted");
+        let msg = format!("{err:#}");
+        assert!(
+            msg.contains("temporary directory") || msg.contains("Refusing"),
+            "unexpected error: {msg}"
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn persist_user_path_honors_test_no_persist_flag() {
+        use crate::util::test_paths::PathRestoreGuard;
+        let _guard = PathRestoreGuard::new();
+        let home = TempDir::new().unwrap();
+        let original_home = std::env::var_os("HOME");
+        std::env::set_var("HOME", home.path());
+
+        let binary = home.path().join("fixture-nu");
+        std::fs::write(&binary, b"fake").unwrap();
+        persist_user_path(&binary).expect("flag must no-op durable Unix PATH writes");
+
+        assert!(
+            !home.path().join(".local").join("bin").join("nu").exists(),
+            "must not create ~/.local/bin/nu while PathRestoreGuard is held"
+        );
+        // No shell-profile export either.
+        for name in [".zshrc", ".bashrc", ".profile"] {
+            assert!(
+                !home.path().join(name).exists(),
+                "must not create {name} while PathRestoreGuard is held"
+            );
+        }
+
+        match original_home {
+            Some(h) => std::env::set_var("HOME", h),
+            None => std::env::remove_var("HOME"),
+        }
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn persist_user_path_refuses_temp_binaries_without_flag() {
+        use crate::util::test_paths::PathRestoreGuard;
+        let _guard = PathRestoreGuard::new();
+        std::env::remove_var("NUMAN_TEST_NO_PERSIST_USER_PATH");
+        let dir = TempDir::new().unwrap();
+        let binary = dir.path().join("nu");
+        std::fs::write(&binary, b"fake").unwrap();
+        let err = persist_user_path(&binary).expect_err("temp binaries must not be persisted");
+        let msg = format!("{err:#}");
+        assert!(
+            msg.contains("temporary directory") || msg.contains("Refusing"),
+            "unexpected error: {msg}"
+        );
+    }
+
+    #[test]
+    fn path_is_under_temp_dir_fails_closed_when_temp_uncanonicalizable() {
+        // A temp root that does not exist cannot be canonicalized; the helper
+        // must still refuse lexical children (fail closed), not return false.
+        let missing_temp =
+            std::env::temp_dir().join(format!("numan-missing-temp-root-{}", std::process::id()));
+        assert!(
+            !missing_temp.exists(),
+            "precondition: missing temp root must not exist"
+        );
+        let nested = missing_temp.join("off");
+        assert!(
+            path_is_under_temp_dir_with(&nested, &missing_temp),
+            "lexical child of an uncanonicalizable temp root must be refused"
+        );
+        let outside = PathBuf::from(if cfg!(windows) {
+            r"C:\Windows\System32"
+        } else {
+            "/usr/bin"
+        });
+        assert!(
+            !path_is_under_temp_dir_with(&outside, &missing_temp),
+            "unrelated paths must not match an uncanonicalizable temp root"
+        );
+    }
+
     #[test]
     fn nu_release_size_cap_exceeds_known_official_archive() {
         // Nu 0.114.1 x86_64-unknown-linux-gnu was ~279 MiB uncompressed and

src/util/test_paths.rs

@@ -4,6 +4,14 @@
 //! any PATH mutation with [`PathRestoreGuard`] so concurrent tests cannot race
 //! on the process-global environment and so a developer shell is not left with
 //! a poisoned PATH after the test binary exits.
+//!
+//! On Windows (and Unix), the guard sets a test-only
+//! `NUMAN_TEST_NO_PERSIST_USER_PATH` flag so production
+//! [`crate::nu::bootstrap::persist_path_dir`] / `persist_user_path` skip durable
+//! User PATH, `~/.local/bin/nu`, and shell-profile writes while the guard is
+//! held. The guard does **not** rewrite the Windows User PATH registry value on
+//! drop: a full snapshot restore would overwrite legitimate PATH changes made
+//! by the user or another process during the test.
 
 use std::ffi::OsString;
 use std::sync::{Mutex, MutexGuard};
@@ -16,6 +24,9 @@ static PATH_MUTEX: Mutex<()> = Mutex::new(());
 /// restores it on drop. Acquires a shared process-wide mutex so callers
 /// never need a separate `Mutex` for PATH serialization.
 ///
+/// Also sets `NUMAN_TEST_NO_PERSIST_USER_PATH` for the guard lifetime so durable
+/// PATH persistence is suppressed (and restored to its prior value on drop).
+///
 /// Use this around any test that mutates PATH so real-Nu runs from a
 /// developer terminal are not poisoned by the test process, and so
 /// parallel ignored acceptance tests cannot overwrite each other's PATH.
@@ -29,10 +40,12 @@ static PATH_MUTEX: Mutex<()> = Mutex::new(());
 /// ```text
 /// let _path_guard = PathRestoreGuard::new();
 /// // mutate PATH...
-/// // drop restores original PATH (or unsets it if it was unset)
+/// // drop restores the original process PATH
 /// ```
 pub struct PathRestoreGuard {
     original: Option<OsString>,
+    /// Pre-existing `NUMAN_TEST_NO_PERSIST_USER_PATH` value (or `None` if absent).
+    previous_no_persist: Option<OsString>,
     _lock: MutexGuard<'static, ()>,
 }
 
@@ -41,8 +54,17 @@ impl PathRestoreGuard {
         let lock = PATH_MUTEX
             .lock()
             .unwrap_or_else(|poisoned| poisoned.into_inner());
+        // Capture then set so Drop can restore a pre-existing value (or remove
+        // only when the variable was originally absent).
+        let previous_no_persist = std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH");
+        // Block durable PATH writes for the duration of the guard.
+        // Production `persist_path_dir` / `persist_user_path` check this;
+        // without it, ignored acceptance tests permanently pollute developer
+        // User PATH / shell profiles with tempfile fixture dirs.
+        std::env::set_var("NUMAN_TEST_NO_PERSIST_USER_PATH", "1");
         Self {
             original: std::env::var_os("PATH"),
+            previous_no_persist,
             _lock: lock,
         }
     }
@@ -54,6 +76,10 @@ impl Drop for PathRestoreGuard {
             Some(path) => std::env::set_var("PATH", path),
             None => std::env::remove_var("PATH"),
         }
+        match self.previous_no_persist.as_ref() {
+            Some(prev) => std::env::set_var("NUMAN_TEST_NO_PERSIST_USER_PATH", prev),
+            None => std::env::remove_var("NUMAN_TEST_NO_PERSIST_USER_PATH"),
+        }
     }
 }
 
@@ -62,3 +88,65 @@ impl Default for PathRestoreGuard {
         Self::new()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Like [`PathRestoreGuard::new`], but installs `previous` under the PATH
+    /// mutex before capturing so parallel tests cannot race the set/capture window.
+    fn guard_with_previous_no_persist(previous: Option<&str>) -> PathRestoreGuard {
+        let lock = PATH_MUTEX
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        match previous {
+            Some(v) => std::env::set_var("NUMAN_TEST_NO_PERSIST_USER_PATH", v),
+            None => std::env::remove_var("NUMAN_TEST_NO_PERSIST_USER_PATH"),
+        }
+        let previous_no_persist = std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH");
+        std::env::set_var("NUMAN_TEST_NO_PERSIST_USER_PATH", "1");
+        PathRestoreGuard {
+            original: std::env::var_os("PATH"),
+            previous_no_persist,
+            _lock: lock,
+        }
+    }
+
+    #[test]
+    fn path_restore_guard_preserves_no_persist_flag_across_drop() {
+        {
+            let guard = guard_with_previous_no_persist(Some("preexisting"));
+            assert_eq!(
+                std::env::var("NUMAN_TEST_NO_PERSIST_USER_PATH").as_deref(),
+                Ok("1")
+            );
+            drop(guard);
+            // Re-acquire so parallel PathRestoreGuard users cannot flip the flag
+            // between Drop restore and our assertion.
+            let _lock = PATH_MUTEX
+                .lock()
+                .unwrap_or_else(|poisoned| poisoned.into_inner());
+            assert_eq!(
+                std::env::var("NUMAN_TEST_NO_PERSIST_USER_PATH").as_deref(),
+                Ok("preexisting")
+            );
+            std::env::remove_var("NUMAN_TEST_NO_PERSIST_USER_PATH");
+        }
+
+        {
+            let guard = guard_with_previous_no_persist(None);
+            assert_eq!(
+                std::env::var("NUMAN_TEST_NO_PERSIST_USER_PATH").as_deref(),
+                Ok("1")
+            );
+            drop(guard);
+            let _lock = PATH_MUTEX
+                .lock()
+                .unwrap_or_else(|poisoned| poisoned.into_inner());
+            assert!(
+                std::env::var_os("NUMAN_TEST_NO_PERSIST_USER_PATH").is_none(),
+                "flag must be removed when it was originally absent"
+            );
+        }
+    }
+}