mirror of
https://github.com/herdrdev/herdr.git
synced 2026-11-06 00:06:18 +00:00
fix: skip directories when resolving Windows programs on PATH (#5269)
portable-pty's Windows search_path accepted any existing candidate, so a directory on PATH whose name matched the program shadowed the real executable and CreateProcessW failed with os error 5. Require a regular file for both the exact name and the PATHEXT candidates. refs #5267
This commit is contained in:
+90
@@ -0,0 +1,90 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: herdr maintainers <maintainers@herdr.dev>
|
||||
Date: Wed, 14 Oct 2026 00:00:00 +0000
|
||||
Subject: [PATCH] skip directories when resolving Windows programs
|
||||
|
||||
Windows PATH commonly contains directories whose names match a program, for
|
||||
example `C:\Windows\System32\PowerShell`. `search_path` returned such a
|
||||
directory because `Path::exists()` is true for directories, so
|
||||
`CreateProcessW` was asked to execute a directory and failed with os error 5.
|
||||
Require a regular file for both the exact name and the PATHEXT candidates so
|
||||
the real executable is found, matching CreateProcessW's own PATH search.
|
||||
|
||||
Herdr issue: https://github.com/herdrdev/herdr/issues/5267
|
||||
Vendored base: portable-pty 0.9.0
|
||||
---
|
||||
diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs
|
||||
index e4a9edc7..81ebd723 100644
|
||||
--- a/vendor/portable-pty/src/cmdbuilder.rs
|
||||
+++ b/vendor/portable-pty/src/cmdbuilder.rs
|
||||
@@ -636,7 +636,7 @@ impl CommandBuilder {
|
||||
for path in std::env::split_paths(&path) {
|
||||
// Check for exactly the user's string in this path dir
|
||||
let candidate = path.join(&exe);
|
||||
- if candidate.exists() {
|
||||
+ if candidate.is_file() {
|
||||
return candidate.into_os_string();
|
||||
}
|
||||
|
||||
@@ -648,7 +648,7 @@ impl CommandBuilder {
|
||||
// doesn't want that
|
||||
let ext = ext.to_str().expect("PATHEXT entries must be utf8");
|
||||
let path = path.join(&exe).with_extension(&ext[1..]);
|
||||
- if path.exists() {
|
||||
+ if path.is_file() {
|
||||
return path.into_os_string();
|
||||
}
|
||||
}
|
||||
@@ -831,6 +831,52 @@ fn is_cwd_relative_path<P: AsRef<Path>>(p: P) -> bool {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
+ #[cfg(windows)]
|
||||
+ #[test]
|
||||
+ fn windows_search_path_skips_directories() {
|
||||
+ use std::os::windows::ffi::OsStringExt;
|
||||
+
|
||||
+ let root = std::env::temp_dir().join(format!(
|
||||
+ "herdr-portable-pty-search-path-{}",
|
||||
+ std::process::id()
|
||||
+ ));
|
||||
+ let shadow_dir = root.join("shadow");
|
||||
+ let real_dir = root.join("real");
|
||||
+ std::fs::create_dir_all(shadow_dir.join("herdrprobe")).expect("create shadow directory");
|
||||
+ std::fs::create_dir_all(shadow_dir.join("herdrprobe.EXE"))
|
||||
+ .expect("create shadow extension directory");
|
||||
+ std::fs::create_dir_all(&real_dir).expect("create real directory");
|
||||
+ let source = std::path::PathBuf::from(std::env::var_os("SystemRoot").expect("SystemRoot"))
|
||||
+ .join("System32")
|
||||
+ .join("where.exe");
|
||||
+ let executable = real_dir.join("herdrprobe.exe");
|
||||
+ std::fs::copy(&source, &executable).expect("copy probe executable");
|
||||
+
|
||||
+ let mut cmd = CommandBuilder::new("herdrprobe");
|
||||
+ cmd.env(
|
||||
+ "PATH",
|
||||
+ std::env::join_paths([&shadow_dir, &real_dir]).expect("join PATH"),
|
||||
+ );
|
||||
+ cmd.env("PATHEXT", ".EXE");
|
||||
+
|
||||
+ let (exe, _) = cmd.cmdline().expect("command line");
|
||||
+ let nul = exe
|
||||
+ .iter()
|
||||
+ .position(|unit| *unit == 0)
|
||||
+ .expect("nul terminated executable");
|
||||
+ let resolved_path = std::path::PathBuf::from(OsString::from_wide(&exe[..nul]));
|
||||
+ let resolved = resolved_path.canonicalize();
|
||||
+ let expected = executable.canonicalize().expect("probe executable");
|
||||
+ let _ = std::fs::remove_dir_all(&root);
|
||||
+
|
||||
+ assert_eq!(
|
||||
+ resolved.expect("resolved executable"),
|
||||
+ expected,
|
||||
+ "search_path resolved {:?}",
|
||||
+ resolved_path
|
||||
+ );
|
||||
+ }
|
||||
+
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_cwd_relative() {
|
||||
Vendored
+45
@@ -129,3 +129,48 @@ On Windows, also run:
|
||||
```sh
|
||||
cargo test --manifest-path vendor/portable-pty/Cargo.toml windows_environment_rejects_malformed_entries
|
||||
```
|
||||
|
||||
## 0004 skip directories when resolving Windows programs
|
||||
|
||||
status: active
|
||||
|
||||
patch: `vendor/patches/portable-pty/0004-skip-directories-when-resolving-windows-programs.patch`
|
||||
|
||||
herdr issue: https://github.com/herdrdev/herdr/issues/5267
|
||||
|
||||
upstream discussion: none
|
||||
|
||||
upstream pr: none
|
||||
|
||||
vendored base: `portable-pty 0.9.0`
|
||||
|
||||
local files:
|
||||
|
||||
- `vendor/portable-pty/src/cmdbuilder.rs`
|
||||
|
||||
reason: Windows `PATH` commonly contains a directory whose name matches a
|
||||
program, such as `C:\Windows\System32\PowerShell`. `search_path` returned that
|
||||
directory because `Path::exists()` is true for directories, so Herdr asked
|
||||
`CreateProcessW` to execute a directory and the pane failed to open with
|
||||
`Access is denied` (os error 5). Requiring `is_file()` for both the exact name
|
||||
and the PATHEXT candidates skips shadowing directories and finds the real
|
||||
executable, matching CreateProcessW's own `PATH` search.
|
||||
|
||||
remove when: upstream `portable-pty` skips non-file `PATH` candidates, or Herdr
|
||||
replaces this launch path.
|
||||
|
||||
verification:
|
||||
|
||||
```sh
|
||||
python3 -m unittest scripts.test_vendor_portable_pty
|
||||
```
|
||||
|
||||
On Windows, run the regression test against the vendored crate. It lives inside
|
||||
the Herdr workspace but is only a `[patch]` provider, not a workspace member, so
|
||||
build a standalone copy and mark it as its own workspace root:
|
||||
|
||||
```sh
|
||||
cp -r vendor/portable-pty "$TMP/portable-pty"
|
||||
printf '\n[workspace]\n' >> "$TMP/portable-pty/Cargo.toml"
|
||||
cargo test --manifest-path "$TMP/portable-pty/Cargo.toml" windows_search_path_skips_directories
|
||||
```
|
||||
|
||||
+48
-2
@@ -636,7 +636,7 @@ impl CommandBuilder {
|
||||
for path in std::env::split_paths(&path) {
|
||||
// Check for exactly the user's string in this path dir
|
||||
let candidate = path.join(&exe);
|
||||
if candidate.exists() {
|
||||
if candidate.is_file() {
|
||||
return candidate.into_os_string();
|
||||
}
|
||||
|
||||
@@ -648,7 +648,7 @@ impl CommandBuilder {
|
||||
// doesn't want that
|
||||
let ext = ext.to_str().expect("PATHEXT entries must be utf8");
|
||||
let path = path.join(&exe).with_extension(&ext[1..]);
|
||||
if path.exists() {
|
||||
if path.is_file() {
|
||||
return path.into_os_string();
|
||||
}
|
||||
}
|
||||
@@ -831,6 +831,52 @@ fn is_cwd_relative_path<P: AsRef<Path>>(p: P) -> bool {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_search_path_skips_directories() {
|
||||
use std::os::windows::ffi::OsStringExt;
|
||||
|
||||
let root = std::env::temp_dir().join(format!(
|
||||
"herdr-portable-pty-search-path-{}",
|
||||
std::process::id()
|
||||
));
|
||||
let shadow_dir = root.join("shadow");
|
||||
let real_dir = root.join("real");
|
||||
std::fs::create_dir_all(shadow_dir.join("herdrprobe")).expect("create shadow directory");
|
||||
std::fs::create_dir_all(shadow_dir.join("herdrprobe.EXE"))
|
||||
.expect("create shadow extension directory");
|
||||
std::fs::create_dir_all(&real_dir).expect("create real directory");
|
||||
let source = std::path::PathBuf::from(std::env::var_os("SystemRoot").expect("SystemRoot"))
|
||||
.join("System32")
|
||||
.join("where.exe");
|
||||
let executable = real_dir.join("herdrprobe.exe");
|
||||
std::fs::copy(&source, &executable).expect("copy probe executable");
|
||||
|
||||
let mut cmd = CommandBuilder::new("herdrprobe");
|
||||
cmd.env(
|
||||
"PATH",
|
||||
std::env::join_paths([&shadow_dir, &real_dir]).expect("join PATH"),
|
||||
);
|
||||
cmd.env("PATHEXT", ".EXE");
|
||||
|
||||
let (exe, _) = cmd.cmdline().expect("command line");
|
||||
let nul = exe
|
||||
.iter()
|
||||
.position(|unit| *unit == 0)
|
||||
.expect("nul terminated executable");
|
||||
let resolved_path = std::path::PathBuf::from(OsString::from_wide(&exe[..nul]));
|
||||
let resolved = resolved_path.canonicalize();
|
||||
let expected = executable.canonicalize().expect("probe executable");
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
|
||||
assert_eq!(
|
||||
resolved.expect("resolved executable"),
|
||||
expected,
|
||||
"search_path resolved {:?}",
|
||||
resolved_path
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn test_cwd_relative() {
|
||||
|
||||
Reference in New Issue
Block a user