From 576dd3670dedbfe64463851a4da065bf66f4fa5d Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Wed, 14 Oct 2026 15:34:20 +0200 Subject: [PATCH] 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 --- ...ries-when-resolving-windows-programs.patch | 90 +++++++++++++++++++ vendor/portable-pty.patches.md | 45 ++++++++++ vendor/portable-pty/src/cmdbuilder.rs | 50 ++++++++++- 3 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 vendor/patches/portable-pty/0004-skip-directories-when-resolving-windows-programs.patch diff --git a/vendor/patches/portable-pty/0004-skip-directories-when-resolving-windows-programs.patch b/vendor/patches/portable-pty/0004-skip-directories-when-resolving-windows-programs.patch new file mode 100644 index 000000000..5297a81c5 --- /dev/null +++ b/vendor/patches/portable-pty/0004-skip-directories-when-resolving-windows-programs.patch @@ -0,0 +1,90 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: herdr maintainers +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: 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() { diff --git a/vendor/portable-pty.patches.md b/vendor/portable-pty.patches.md index 14eae8694..09a0bfc9e 100644 --- a/vendor/portable-pty.patches.md +++ b/vendor/portable-pty.patches.md @@ -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 +``` diff --git a/vendor/portable-pty/src/cmdbuilder.rs b/vendor/portable-pty/src/cmdbuilder.rs index e4a9edc78..81ebd7237 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: 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() {