chore: harden uv parser, cache panics, test isolation, and temp dir cleanup (#410)

## Summary

Batch of four hardening and code quality improvements identified during
coverage gap analysis.

## Changes

- **#397 — uv version parser:** Validate that version components start
with digits, rejecting garbage like `cpython-3.abc.def-linux` while
still accepting pre-release versions like `3.14.0a4`. Added tests for
both rejection and acceptance cases.

- **#402 — LocatorCache `.unwrap()` → `.expect()`:** Replaced all 11
`.unwrap()` calls on RwLock operations with `.expect("locator cache lock
poisoned")` for better panic diagnostics.

- **#403 — tempfile::TempDir RAII:** Replaced manual temp dir management
in `pet-pixi` and `pet-windows-registry` tests with `tempfile::TempDir`.
Directories are now cleaned up automatically even on test panics.

- **#401 — JSONRPC test client isolation:** Replaced `.env("PATH", "")`
with `.env_clear()` to fully isolate the test server from host
environment variables. Only `PATH` (empty) and `SYSTEMROOT` are
restored.

Fixes #397, Fixes #402, Fixes #403, Fixes #401

Author: karthiknadig

Cargo.lock

@@ -29,27 +29,37 @@ impl<K: Eq + Hash, V: Clone> LocatorCache<K, V> {
 
     /// Returns a cloned value for the given key if it exists in the cache.
     pub fn get(&self, key: &K) -> Option<V> {
-        self.cache.read().unwrap().get(key).cloned()
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .get(key)
+            .cloned()
     }
 
     /// Checks if the cache contains the given key.
     pub fn contains_key(&self, key: &K) -> bool {
-        self.cache.read().unwrap().contains_key(key)
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .contains_key(key)
     }
 
     /// Inserts a key-value pair into the cache.
     ///
     /// Returns the previous value if the key was already present.
     pub fn insert(&self, key: K, value: V) -> Option<V> {
-        self.cache.write().unwrap().insert(key, value)
+        self.cache
+            .write()
+            .expect("locator cache lock poisoned")
+            .insert(key, value)
     }
 
     /// Inserts multiple key-value pairs into the cache atomically.
     ///
     /// This method acquires a single write lock for all insertions, which is more
     /// efficient than calling `insert` multiple times when inserting many entries.
     pub fn insert_many(&self, entries: impl IntoIterator<Item = (K, V)>) {
-        let mut cache = self.cache.write().unwrap();
+        let mut cache = self.cache.write().expect("locator cache lock poisoned");
         for (key, value) in entries {
             cache.insert(key, value);
         }
@@ -68,7 +78,7 @@ impl<K: Eq + Hash, V: Clone> LocatorCache<K, V> {
     {
         // First check with read lock
         {
-            let cache = self.cache.read().unwrap();
+            let cache = self.cache.read().expect("locator cache lock poisoned");
             if let Some(value) = cache.get(&key) {
                 return Some(value.clone());
             }
@@ -77,7 +87,7 @@ impl<K: Eq + Hash, V: Clone> LocatorCache<K, V> {
         // Compute the value (outside of any lock)
         if let Some(value) = f() {
             // Acquire write lock and insert
-            let mut cache = self.cache.write().unwrap();
+            let mut cache = self.cache.write().expect("locator cache lock poisoned");
             // Double-check in case another thread inserted while we were computing
             if let Some(existing) = cache.get(&key) {
                 return Some(existing.clone());
@@ -91,30 +101,47 @@ impl<K: Eq + Hash, V: Clone> LocatorCache<K, V> {
 
     /// Clears all entries from the cache.
     pub fn clear(&self) {
-        self.cache.write().unwrap().clear();
+        self.cache
+            .write()
+            .expect("locator cache lock poisoned")
+            .clear();
     }
 
     /// Returns all values in the cache as a vector.
     pub fn values(&self) -> Vec<V> {
-        self.cache.read().unwrap().values().cloned().collect()
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .values()
+            .cloned()
+            .collect()
     }
 
     /// Returns the number of entries in the cache.
     pub fn len(&self) -> usize {
-        self.cache.read().unwrap().len()
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .len()
     }
 
     /// Returns true if the cache is empty.
     pub fn is_empty(&self) -> bool {
-        self.cache.read().unwrap().is_empty()
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .is_empty()
     }
 
     /// Returns all entries in the cache as a HashMap.
     pub fn clone_map(&self) -> HashMap<K, V>
     where
         K: Clone,
     {
-        self.cache.read().unwrap().clone()
+        self.cache
+            .read()
+            .expect("locator cache lock poisoned")
+            .clone()
     }
 }
 

crates/pet-core/src/cache.rs

@@ -12,3 +12,6 @@ pet-conda = { path = "../pet-conda" }
 pet-core = { path = "../pet-core" }
 pet-python-utils = { path = "../pet-python-utils" }
 log = "0.4.21"
+
+[dev-dependencies]
+tempfile = "3.13"

crates/pet-pixi/Cargo.toml

@@ -89,24 +89,11 @@ impl Locator for Pixi {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::{
-        fs,
-        time::{SystemTime, UNIX_EPOCH},
-    };
-
-    fn create_test_dir(name: &str) -> PathBuf {
-        let unique = SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .unwrap()
-            .as_nanos();
-        let directory =
-            std::env::temp_dir().join(format!("pet-pixi-{name}-{}-{unique}", std::process::id()));
-        fs::create_dir_all(&directory).unwrap();
-        directory
-    }
+    use std::fs;
+    use tempfile::TempDir;
 
-    fn create_pixi_prefix() -> PathBuf {
-        let prefix = create_test_dir("prefix").join("pixi-env");
+    fn create_pixi_prefix(temp_dir: &TempDir) -> PathBuf {
+        let prefix = temp_dir.path().join("pixi-env");
         fs::create_dir_all(prefix.join("conda-meta")).unwrap();
         fs::write(prefix.join("conda-meta").join("pixi"), b"").unwrap();
         fs::create_dir_all(prefix.join(if cfg!(windows) { "Scripts" } else { "bin" })).unwrap();
@@ -126,17 +113,17 @@ mod tests {
 
     #[test]
     fn is_pixi_env_checks_for_pixi_marker_file() {
-        let prefix = create_pixi_prefix();
+        let temp_dir = TempDir::new().unwrap();
+        let prefix = create_pixi_prefix(&temp_dir);
 
         assert!(is_pixi_env(&prefix));
         assert!(!is_pixi_env(&prefix.join("conda-meta")));
-
-        fs::remove_dir_all(prefix.parent().unwrap()).unwrap();
     }
 
     #[test]
     fn try_from_identifies_pixi_env_from_explicit_prefix() {
-        let prefix = create_pixi_prefix();
+        let temp_dir = TempDir::new().unwrap();
+        let prefix = create_pixi_prefix(&temp_dir);
         let executable = prefix
             .join(if cfg!(windows) { "Scripts" } else { "bin" })
             .join(if cfg!(windows) {
@@ -163,7 +150,7 @@ mod tests {
                 .map(fs::canonicalize)
                 .transpose()
                 .unwrap(),
-            Some(fs::canonicalize(prefix.clone()).unwrap())
+            Some(fs::canonicalize(prefix).unwrap())
         );
         assert_eq!(
             pixi_env
@@ -174,13 +161,12 @@ mod tests {
                 .unwrap(),
             Some(fs::canonicalize(executable).unwrap())
         );
-
-        fs::remove_dir_all(prefix.parent().unwrap()).unwrap();
     }
 
     #[test]
     fn try_from_derives_pixi_prefix_from_nested_python_executable() {
-        let prefix = create_pixi_prefix();
+        let temp_dir = TempDir::new().unwrap();
+        let prefix = create_pixi_prefix(&temp_dir);
         let executable = prefix
             .join(if cfg!(windows) { "Scripts" } else { "bin" })
             .join(if cfg!(windows) {
@@ -202,22 +188,18 @@ mod tests {
                 .map(fs::canonicalize)
                 .transpose()
                 .unwrap(),
-            Some(fs::canonicalize(prefix.clone()).unwrap())
+            Some(fs::canonicalize(prefix).unwrap())
         );
-
-        fs::remove_dir_all(prefix.parent().unwrap()).unwrap();
     }
 
     #[test]
     fn try_from_rejects_non_pixi_environments() {
-        let prefix = create_test_dir("plain-prefix");
-        let executable = prefix.join("python");
+        let temp_dir = TempDir::new().unwrap();
+        let executable = temp_dir.path().join("python");
         fs::write(&executable, b"").unwrap();
         let locator = Pixi::new();
-        let env = PythonEnv::new(executable, Some(prefix.clone()), None);
+        let env = PythonEnv::new(executable, Some(temp_dir.path().to_path_buf()), None);
 
         assert!(locator.try_from(&env).is_none());
-
-        fs::remove_dir_all(prefix).unwrap();
     }
 }

crates/pet-pixi/src/lib.rs

@@ -344,12 +344,32 @@ fn parse_version_from_uv_dir_name(dir_name: &str) -> Option<String> {
     let mut parts = dir_name.splitn(3, '-');
     let _impl = parts.next()?;
     let version = parts.next()?;
-    // Verify at minimum X.Y format (e.g., "3.12" or "3.12.3")
-    if version.starts_with(|c: char| c.is_ascii_digit()) && version.split('.').count() >= 2 {
-        Some(version.to_string())
-    } else {
-        None
+    // Require the platform segment to exist (rejects bare "<impl>-<version>").
+    let platform = parts.next()?;
+    if platform.is_empty() {
+        return None;
+    }
+    // Verify at minimum X.Y format (e.g., "3.12" or "3.12.3").
+    // Components before the last must be purely numeric.
+    // The last component may have a pre-release suffix (e.g., "0a4", "0rc1").
+    let components: Vec<&str> = version.split('.').collect();
+    if components.len() < 2 {
+        return None;
+    }
+    // All components except the last must be purely numeric.
+    let all_but_last = &components[..components.len() - 1];
+    if !all_but_last
+        .iter()
+        .all(|c| !c.is_empty() && c.chars().all(|ch| ch.is_ascii_digit()))
+    {
+        return None;
+    }
+    // The last component must start with a digit (allows pre-release suffix like "0a4").
+    let last = components.last()?;
+    if last.is_empty() || !last.starts_with(|ch: char| ch.is_ascii_digit()) {
+        return None;
     }
+    Some(version.to_string())
 }
 
 /// Walks up from `project_path` looking for a workspace that this project belongs to.
@@ -1107,6 +1127,41 @@ exclude = ["packages/legacy"]"#;
         assert_eq!(parse_version_from_uv_dir_name("cpython-9-linux"), None);
     }
 
+    #[test]
+    fn test_parse_version_from_uv_dir_name_rejects_non_numeric_components() {
+        // Version components must start with a digit — "3.abc.def" should be rejected.
+        assert_eq!(
+            parse_version_from_uv_dir_name("cpython-3.abc.def-linux"),
+            None
+        );
+        // Empty component after dot.
+        assert_eq!(parse_version_from_uv_dir_name("cpython-3.12.-linux"), None);
+        // Non-numeric middle component must be rejected even if it starts with a digit.
+        assert_eq!(
+            parse_version_from_uv_dir_name("cpython-3.12abc.1-linux"),
+            None
+        );
+    }
+
+    #[test]
+    fn test_parse_version_from_uv_dir_name_rejects_missing_platform() {
+        // Bare "<impl>-<version>" without platform segment should be rejected.
+        assert_eq!(parse_version_from_uv_dir_name("cpython-3.12"), None);
+    }
+
+    #[test]
+    fn test_parse_version_from_uv_dir_name_accepts_prerelease() {
+        // Pre-release versions like "3.14.0a4" are valid uv install dirs.
+        assert_eq!(
+            parse_version_from_uv_dir_name("cpython-3.14.0a4-linux-x86_64-gnu"),
+            Some("3.14.0a4".to_string())
+        );
+        assert_eq!(
+            parse_version_from_uv_dir_name("cpython-3.13.0rc1-linux-x86_64-gnu"),
+            Some("3.13.0rc1".to_string())
+        );
+    }
+
     #[test]
     fn test_find_managed_python_installs_discovers_versions() {
         let temp_dir = TempDir::new().unwrap();

crates/pet-uv/src/lib.rs

@@ -20,3 +20,6 @@ pet-conda = { path = "../pet-conda" }
 log = "0.4.21"
 lazy_static = "1.4.0"
 regex = "1.10.4"
+
+[dev-dependencies]
+tempfile = "3.13"

crates/pet-windows-registry/Cargo.toml

@@ -149,21 +149,8 @@ mod tests {
     use std::{
         fs,
         path::{Path, PathBuf},
-        time::{SystemTime, UNIX_EPOCH},
     };
-
-    fn create_test_dir(name: &str) -> PathBuf {
-        let unique = SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .unwrap()
-            .as_nanos();
-        let directory = std::env::temp_dir().join(format!(
-            "pet-windows-registry-{name}-{}-{unique}",
-            std::process::id()
-        ));
-        fs::create_dir_all(&directory).unwrap();
-        directory
-    }
+    use tempfile::TempDir;
 
     fn create_virtualenv(prefix: &Path) -> PathBuf {
         let scripts_dir = prefix.join(if cfg!(windows) { "Scripts" } else { "bin" });
@@ -318,27 +305,25 @@ mod tests {
 
     #[test]
     fn test_try_from_rejects_virtualenv_before_registry_lookup() {
-        let prefix = create_test_dir("virtualenv");
+        let temp_dir = TempDir::new().unwrap();
+        let prefix = temp_dir.path().to_path_buf();
         let executable = create_virtualenv(&prefix);
-        let env = PythonEnv::new(executable, Some(prefix.clone()), None);
+        let env = PythonEnv::new(executable, Some(prefix), None);
         let locator = create_locator();
 
         assert!(locator.try_from(&env).is_none());
-
-        fs::remove_dir_all(prefix).unwrap();
     }
 
     #[test]
     fn test_try_from_rejects_conda_prefix_before_registry_lookup() {
-        let prefix = create_test_dir("conda-env");
+        let temp_dir = TempDir::new().unwrap();
+        let prefix = temp_dir.path().to_path_buf();
         fs::create_dir_all(prefix.join("conda-meta")).unwrap();
         let executable = prefix.join("python.exe");
         fs::write(&executable, b"").unwrap();
-        let env = PythonEnv::new(executable, Some(prefix.clone()), None);
+        let env = PythonEnv::new(executable, Some(prefix), None);
         let locator = create_locator();
 
         assert!(locator.try_from(&env).is_none());
-
-        fs::remove_dir_all(prefix).unwrap();
     }
 }

crates/pet-windows-registry/src/lib.rs

@@ -100,7 +100,15 @@ impl PetJsonRpcClient {
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .stderr(Stdio::piped())
+            // Clear all inherited env vars to prevent host-specific tool
+            // configuration from leaking into the test environment, then
+            // restore only the minimum required for the OS to function.
+            .env_clear()
             .env("PATH", "")
+            .env(
+                "SYSTEMROOT",
+                std::env::var("SYSTEMROOT").unwrap_or_default(),
+            )
             .spawn()
             .map_err(|e| format!("Failed to spawn pet server: {e}"))?;