Skip to content

Commit 05d75c6

Browse files
authored
fix: tighten uv version parser and platform-gate SYSTEMROOT in test client (#411)
## Summary Addresses two unresolved Copilot review comments from merged PR #410. ## Changes - **uv version parser:** Major and minor components are now explicitly validated as purely numeric. Pre-release suffix (e.g., `0a4`, `0rc1`) is only allowed on the patch component (3rd+). Previously `3.12abc` passed validation because the minor was the "last" component and only needed to start with a digit. Added test case for this scenario. - **SYSTEMROOT in test client:** Platform-gated with `#[cfg(windows)]` and only set when `std::env::var("SYSTEMROOT")` succeeds. Previously used `unwrap_or_default()` which silently set it to empty string on non-Windows or when missing. Follow-up to #410
1 parent 3a7b3fa commit 05d75c6

File tree

2 files changed

+36
-16
lines changed

2 files changed

+36
-16
lines changed

crates/pet-uv/src/lib.rs

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -350,21 +350,33 @@ fn parse_version_from_uv_dir_name(dir_name: &str) -> Option<String> {
350350
return None;
351351
}
352352
// Verify at minimum X.Y format (e.g., "3.12" or "3.12.3").
353-
// Components before the last must be purely numeric.
354-
// The last component may have a pre-release suffix (e.g., "0a4", "0rc1").
353+
// Major and minor must always be purely numeric.
354+
// Only the patch component (3rd+) may have a pre-release suffix (e.g., "0a4", "0rc1").
355355
let components: Vec<&str> = version.split('.').collect();
356356
if components.len() < 2 {
357357
return None;
358358
}
359-
// All components except the last must be purely numeric.
360-
let all_but_last = &components[..components.len() - 1];
361-
if !all_but_last
362-
.iter()
363-
.all(|c| !c.is_empty() && c.chars().all(|ch| ch.is_ascii_digit()))
364-
{
359+
360+
let is_numeric = |c: &str| !c.is_empty() && c.chars().all(|ch| ch.is_ascii_digit());
361+
362+
// Major and minor must be purely numeric.
363+
if !is_numeric(components[0]) || !is_numeric(components[1]) {
364+
return None;
365+
}
366+
367+
// For X.Y versions (no patch), we're done.
368+
if components.len() == 2 {
369+
return Some(version.to_string());
370+
}
371+
372+
// Any components between minor and the last must be purely numeric.
373+
let middle = &components[2..components.len() - 1];
374+
if !middle.iter().all(|c| is_numeric(c)) {
365375
return None;
366376
}
367-
// The last component must start with a digit (allows pre-release suffix like "0a4").
377+
378+
// The last component (patch or beyond) must start with a digit
379+
// (allows pre-release suffix like "0a4").
368380
let last = components.last()?;
369381
if last.is_empty() || !last.starts_with(|ch: char| ch.is_ascii_digit()) {
370382
return None;
@@ -1141,6 +1153,11 @@ exclude = ["packages/legacy"]"#;
11411153
parse_version_from_uv_dir_name("cpython-3.12abc.1-linux"),
11421154
None
11431155
);
1156+
// 2-component version with non-numeric minor must be rejected.
1157+
assert_eq!(
1158+
parse_version_from_uv_dir_name("cpython-3.12abc-linux-x86_64-gnu"),
1159+
None
1160+
);
11441161
}
11451162

11461163
#[test]

crates/pet/tests/jsonrpc_client.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,20 +95,23 @@ pub struct PetJsonRpcClient {
9595

9696
impl PetJsonRpcClient {
9797
pub fn spawn() -> Result<Self, String> {
98-
let mut process = Command::new(env!("CARGO_BIN_EXE_pet"))
99-
.arg("server")
98+
let mut cmd = Command::new(env!("CARGO_BIN_EXE_pet"));
99+
cmd.arg("server")
100100
.stdin(Stdio::piped())
101101
.stdout(Stdio::piped())
102102
.stderr(Stdio::piped())
103103
// Clear all inherited env vars to prevent host-specific tool
104104
// configuration from leaking into the test environment, then
105105
// restore only the minimum required for the OS to function.
106106
.env_clear()
107-
.env("PATH", "")
108-
.env(
109-
"SYSTEMROOT",
110-
std::env::var("SYSTEMROOT").unwrap_or_default(),
111-
)
107+
.env("PATH", "");
108+
// On Windows, SYSTEMROOT is required for basic OS functionality
109+
// (crypto, networking, etc.). Only set it when present.
110+
#[cfg(windows)]
111+
if let Ok(val) = std::env::var("SYSTEMROOT") {
112+
cmd.env("SYSTEMROOT", val);
113+
}
114+
let mut process = cmd
112115
.spawn()
113116
.map_err(|e| format!("Failed to spawn pet server: {e}"))?;
114117

0 commit comments

Comments
 (0)