From ab7eea0ecc7b38f71cdec3cf8e2ca6ef0711b7c1 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Fri, 1 May 2026 10:22:17 -0700 Subject: [PATCH] Resolve Windows sandbox command paths before spawn --- codex-rs/core/src/shell.rs | 27 +++ codex-rs/windows-sandbox-rs/src/conpty/mod.rs | 11 +- codex-rs/windows-sandbox-rs/src/lib.rs | 3 + codex-rs/windows-sandbox-rs/src/process.rs | 21 +- .../src/program_resolution.rs | 218 ++++++++++++++++++ 5 files changed, 272 insertions(+), 8 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/program_resolution.rs diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 48c760d667..895b9254d8 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,5 +1,9 @@ use crate::shell_detect::detect_shell_type; use crate::shell_snapshot::ShellSnapshot; +#[cfg(windows)] +use codex_shell_command::powershell::try_find_powershell_executable_blocking; +#[cfg(windows)] +use codex_shell_command::powershell::try_find_pwsh_executable_blocking; use serde::Deserialize; use serde::Serialize; use std::path::PathBuf; @@ -250,6 +254,29 @@ const POWERSHELL_FALLBACK_PATHS: &[&str] = const POWERSHELL_FALLBACK_PATHS: &[&str] = &[]; fn get_powershell_shell(path: Option<&PathBuf>) -> Option { + #[cfg(windows)] + let shell_path = path + .and_then(file_exists) + .or_else(|| try_find_pwsh_executable_blocking().map(|path| path.into_path_buf())) + .or_else(|| try_find_powershell_executable_blocking().map(|path| path.into_path_buf())) + .or_else(|| { + get_shell_path( + ShellType::PowerShell, + /* provided_path */ None, + "pwsh", + PWSH_FALLBACK_PATHS, + ) + }) + .or_else(|| { + get_shell_path( + ShellType::PowerShell, + /* provided_path */ None, + "powershell", + POWERSHELL_FALLBACK_PATHS, + ) + }); + + #[cfg(not(windows))] let shell_path = get_shell_path(ShellType::PowerShell, path, "pwsh", PWSH_FALLBACK_PATHS) .or_else(|| { get_shell_path( diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index cd839952b5..09e5275a3e 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -8,6 +8,7 @@ use crate::desktop::LaunchDesktop; use crate::proc_thread_attr::ProcThreadAttributeList; +use crate::program_resolution::resolve_spawn_command; use crate::winutil::format_last_error; use crate::winutil::quote_windows_arg; use crate::winutil::to_wide; @@ -98,12 +99,15 @@ pub fn spawn_conpty_process_as_user( use_private_desktop: bool, logs_base_dir: Option<&Path>, ) -> Result<(PROCESS_INFORMATION, ConptyInstance)> { - let cmdline_str = argv + let resolved_command = resolve_spawn_command(argv, cwd, env_map); + let spawn_argv = resolved_command.argv; + let cmdline_str = spawn_argv .iter() .map(|arg| quote_windows_arg(arg)) .collect::>() .join(" "); let mut cmdline: Vec = to_wide(&cmdline_str); + let application_name = resolved_command.application_name.map(to_wide); let env_block = make_env_block(env_map); let mut si: STARTUPINFOEXW = unsafe { std::mem::zeroed() }; si.StartupInfo.cb = std::mem::size_of::() as u32; @@ -131,7 +135,10 @@ pub fn spawn_conpty_process_as_user( let ok = unsafe { CreateProcessAsUserW( h_token, - std::ptr::null(), + application_name + .as_ref() + .map(|path| path.as_ptr()) + .unwrap_or(std::ptr::null()), cmdline.as_mut_ptr(), std::ptr::null_mut(), std::ptr::null_mut(), diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 522f8926d5..0af666751b 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -33,6 +33,9 @@ windows_modules!( workspace_acl ); +#[cfg(any(target_os = "windows", test))] +mod program_resolution; + #[cfg(target_os = "windows")] #[path = "conpty/mod.rs"] mod conpty; diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 1571bb8ccf..53fac177df 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -1,30 +1,31 @@ use crate::desktop::LaunchDesktop; use crate::logging; use crate::proc_thread_attr::ProcThreadAttributeList; +use crate::program_resolution::resolve_spawn_command; use crate::winutil::argv_to_command_line; use crate::winutil::format_last_error; use crate::winutil::to_wide; -use anyhow::anyhow; use anyhow::Result; +use anyhow::anyhow; use std::collections::HashMap; use std::ffi::c_void; use std::path::Path; use std::ptr; -use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::CloseHandle; -use windows_sys::Win32::Foundation::SetHandleInformation; +use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT; use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::SetHandleInformation; use windows_sys::Win32::Storage::FileSystem::ReadFile; use windows_sys::Win32::System::Console::GetStdHandle; use windows_sys::Win32::System::Console::STD_ERROR_HANDLE; use windows_sys::Win32::System::Console::STD_INPUT_HANDLE; use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE; use windows_sys::Win32::System::Pipes::CreatePipe; -use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT; use windows_sys::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; +use windows_sys::Win32::System::Threading::CreateProcessAsUserW; use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; use windows_sys::Win32::System::Threading::STARTF_USESTDHANDLES; use windows_sys::Win32::System::Threading::STARTUPINFOEXW; @@ -84,6 +85,8 @@ pub unsafe fn create_process_as_user( stdio: Option<(HANDLE, HANDLE, HANDLE)>, use_private_desktop: bool, ) -> Result { + let resolved_command = resolve_spawn_command(argv, cwd, env_map); + let application_name = resolved_command.application_name.map(to_wide); let cmdline_str = argv_to_command_line(argv); let mut cmdline: Vec = to_wide(&cmdline_str); let env_block = make_env_block(env_map); @@ -122,7 +125,10 @@ pub unsafe fn create_process_as_user( let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; let ok = CreateProcessAsUserW( h_token, - std::ptr::null(), + application_name + .as_ref() + .map(|path| path.as_ptr()) + .unwrap_or(std::ptr::null()), cmdline.as_mut_ptr(), std::ptr::null_mut(), std::ptr::null_mut(), @@ -163,7 +169,10 @@ pub unsafe fn create_process_as_user( let creation_flags = CREATE_UNICODE_ENVIRONMENT; let ok = CreateProcessAsUserW( h_token, - std::ptr::null(), + application_name + .as_ref() + .map(|path| path.as_ptr()) + .unwrap_or(std::ptr::null()), cmdline.as_mut_ptr(), std::ptr::null_mut(), std::ptr::null_mut(), diff --git a/codex-rs/windows-sandbox-rs/src/program_resolution.rs b/codex-rs/windows-sandbox-rs/src/program_resolution.rs new file mode 100644 index 0000000000..2bcc13158c --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/program_resolution.rs @@ -0,0 +1,218 @@ +use std::collections::HashMap; +use std::env; +use std::path::Path; +use std::path::PathBuf; + +pub(crate) struct ResolvedSpawnCommand { + pub(crate) argv: Vec, + pub(crate) application_name: Option, +} + +pub(crate) fn resolve_spawn_command( + argv: &[String], + cwd: &Path, + env_map: &HashMap, +) -> ResolvedSpawnCommand { + let Some(program) = argv.first() else { + return ResolvedSpawnCommand { + argv: Vec::new(), + application_name: None, + }; + }; + + let Some(resolved_program) = resolve_program_path(program, cwd, env_map) else { + return ResolvedSpawnCommand { + argv: argv.to_vec(), + application_name: None, + }; + }; + + let resolved_str = resolved_program.to_string_lossy().to_string(); + let mut resolved_argv = argv.to_vec(); + resolved_argv[0] = resolved_str.clone(); + ResolvedSpawnCommand { + argv: resolved_argv, + application_name: Some(resolved_str), + } +} + +fn resolve_program_path( + program: &str, + cwd: &Path, + env_map: &HashMap, +) -> Option { + let program_path = Path::new(program); + if has_path_qualifier(program_path) { + return resolve_explicit_program_path(program_path, cwd, env_map); + } + + if is_cmd_program(program) { + let comspec = env_map + .get("ComSpec") + .cloned() + .or_else(|| env::var("ComSpec").ok()); + if let Some(comspec) = comspec { + let comspec_path = PathBuf::from(comspec); + if comspec_path.is_file() && !is_windowsapps_path(&comspec_path) { + return Some(comspec_path); + } + } + } + + search_path_for_program(program_path, env_map) +} + +fn resolve_explicit_program_path( + program_path: &Path, + cwd: &Path, + env_map: &HashMap, +) -> Option { + let candidate = if program_path.is_absolute() { + program_path.to_path_buf() + } else { + cwd.join(program_path) + }; + if candidate.is_file() && !is_windowsapps_path(&candidate) { + return Some(candidate); + } + if is_windowsapps_path(&candidate) { + let file_name = program_path.file_name()?.to_string_lossy().to_string(); + return resolve_program_path(&file_name, cwd, env_map); + } + None +} + +fn search_path_for_program( + program_path: &Path, + env_map: &HashMap, +) -> Option { + let search_path = env_map + .get("PATH") + .cloned() + .or_else(|| env::var("PATH").ok())?; + let path_exts = path_extensions(env_map); + let has_extension = program_path.extension().is_some(); + + for dir in env::split_paths(&search_path) { + let direct_candidate = dir.join(program_path); + if direct_candidate.is_file() && !is_windowsapps_path(&direct_candidate) { + return Some(direct_candidate); + } + + if has_extension { + continue; + } + + for ext in &path_exts { + let candidate = dir.join(program_path).with_extension(ext); + if candidate.is_file() && !is_windowsapps_path(&candidate) { + return Some(candidate); + } + } + } + + None +} + +fn path_extensions(env_map: &HashMap) -> Vec { + env_map + .get("PATHEXT") + .cloned() + .or_else(|| env::var("PATHEXT").ok()) + .unwrap_or_else(|| ".COM;.EXE;.BAT;.CMD".to_string()) + .split(';') + .filter(|ext| !ext.is_empty()) + .map(|ext| ext.trim_start_matches('.').to_string()) + .collect() +} + +fn has_path_qualifier(path: &Path) -> bool { + path.is_absolute() || path.parent().is_some() +} + +fn is_cmd_program(program: &str) -> bool { + program.eq_ignore_ascii_case("cmd") || program.eq_ignore_ascii_case("cmd.exe") +} + +fn is_windowsapps_path(path: &Path) -> bool { + let normalized = path + .to_string_lossy() + .replace('/', "\\") + .to_ascii_lowercase(); + normalized.contains("\\program files\\windowsapps\\") + || normalized.contains("\\appdata\\local\\microsoft\\windowsapps\\") +} + +#[cfg(test)] +mod tests { + use super::is_windowsapps_path; + use super::resolve_spawn_command; + use pretty_assertions::assert_eq; + use std::collections::HashMap; + use std::env; + use std::fs; + use tempfile::TempDir; + + #[test] + fn windowsapps_path_detection_handles_forward_slashes() { + assert!(is_windowsapps_path(std::path::Path::new( + "C:/Users/alice/AppData/Local/Microsoft/WindowsApps/pwsh.exe" + ))); + assert!(is_windowsapps_path(std::path::Path::new( + "C:/Program Files/WindowsApps/Foo/tool.exe" + ))); + assert!(!is_windowsapps_path(std::path::Path::new( + "C:/Windows/System32/cmd.exe" + ))); + } + + #[test] + fn resolve_spawn_command_skips_windowsapps_aliases() { + let temp = TempDir::new().expect("tempdir"); + let alias_dir = temp.path().join("AppData/Local/Microsoft/WindowsApps"); + let real_dir = temp.path().join("real-bin"); + fs::create_dir_all(&alias_dir).expect("create alias dir"); + fs::create_dir_all(&real_dir).expect("create real dir"); + fs::write(alias_dir.join("tool.cmd"), "@echo off\r\n").expect("write alias"); + fs::write(real_dir.join("tool.cmd"), "@echo off\r\n").expect("write real"); + + let mut env_map = HashMap::new(); + let path = env::join_paths([alias_dir.as_path(), real_dir.as_path()]) + .expect("join path") + .to_string_lossy() + .to_string(); + env_map.insert("PATH".to_string(), path); + env_map.insert("PATHEXT".to_string(), ".EXE;.CMD".to_string()); + + let resolved = resolve_spawn_command( + &["tool".to_string(), "/c".to_string()], + temp.path(), + &env_map, + ); + assert_eq!( + resolved.argv[0], + real_dir.join("tool.cmd").to_string_lossy().to_string() + ); + assert_eq!(resolved.application_name, Some(resolved.argv[0].clone())); + } + + #[test] + fn resolve_spawn_command_uses_comspec_for_cmd() { + let temp = TempDir::new().expect("tempdir"); + let system32 = temp.path().join("Windows/System32"); + fs::create_dir_all(&system32).expect("create system32"); + let cmd = system32.join("cmd.exe"); + fs::write(&cmd, "stub").expect("write cmd"); + + let mut env_map = HashMap::new(); + env_map.insert("ComSpec".to_string(), cmd.to_string_lossy().to_string()); + + let resolved = resolve_spawn_command( + &["cmd".to_string(), "/c".to_string()], + temp.path(), + &env_map, + ); + assert_eq!(resolved.argv[0], cmd.to_string_lossy().to_string()); + assert_eq!(resolved.application_name, Some(resolved.argv[0].clone())); + } +}