From 25533ee9b2e73b506cda3d785150017a1dc7c1aa Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Mon, 15 Jun 2026 11:15:33 -0700 Subject: [PATCH] windows-sandbox: resolve PATH commands before spawn --- codex-rs/windows-sandbox-rs/Cargo.toml | 1 + codex-rs/windows-sandbox-rs/src/conpty/mod.rs | 4 +- codex-rs/windows-sandbox-rs/src/lib.rs | 2 + codex-rs/windows-sandbox-rs/src/process.rs | 4 +- .../src/program_resolution.rs | 81 +++++++++++++++++++ codex-rs/windows-sandbox-rs/src/setup.rs | 45 +++++++++++ 6 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/program_resolution.rs diff --git a/codex-rs/windows-sandbox-rs/Cargo.toml b/codex-rs/windows-sandbox-rs/Cargo.toml index 31df9ad390..f0c1d0a5f0 100644 --- a/codex-rs/windows-sandbox-rs/Cargo.toml +++ b/codex-rs/windows-sandbox-rs/Cargo.toml @@ -39,6 +39,7 @@ serde_json = "1.0" tempfile = "3" tokio = { workspace = true, features = ["sync", "rt"] } tracing-appender = { workspace = true } +which = { workspace = true } windows = { version = "0.58", features = [ "Win32_Foundation", "Win32_NetworkManagement_WindowsFirewall", diff --git a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs index cd839952b5..1a3f318f19 100644 --- a/codex-rs/windows-sandbox-rs/src/conpty/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/conpty/mod.rs @@ -7,6 +7,7 @@ //! Windows sandbox flows that need a PTY. use crate::desktop::LaunchDesktop; +use crate::program_resolution::resolve_bare_program_on_path; use crate::proc_thread_attr::ProcThreadAttributeList; use crate::winutil::format_last_error; use crate::winutil::quote_windows_arg; @@ -98,7 +99,8 @@ 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_argv = resolve_bare_program_on_path(argv, cwd, env_map); + let cmdline_str = resolved_argv .iter() .map(|arg| quote_windows_arg(arg)) .collect::>() diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 3bd2639868..59fbed4120 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -64,6 +64,8 @@ mod logging; #[cfg(target_os = "windows")] mod path_normalization; #[cfg(target_os = "windows")] +mod program_resolution; +#[cfg(target_os = "windows")] mod process; #[cfg(target_os = "windows")] mod resolved_permissions; diff --git a/codex-rs/windows-sandbox-rs/src/process.rs b/codex-rs/windows-sandbox-rs/src/process.rs index 0899d34c2f..e42b00f0ad 100644 --- a/codex-rs/windows-sandbox-rs/src/process.rs +++ b/codex-rs/windows-sandbox-rs/src/process.rs @@ -1,5 +1,6 @@ use crate::desktop::LaunchDesktop; use crate::logging; +use crate::program_resolution::resolve_bare_program_on_path; use crate::proc_thread_attr::ProcThreadAttributeList; use crate::winutil::argv_to_command_line; use crate::winutil::format_last_error; @@ -84,7 +85,8 @@ pub unsafe fn create_process_as_user( stdio: Option<(HANDLE, HANDLE, HANDLE)>, use_private_desktop: bool, ) -> Result { - let cmdline_str = argv_to_command_line(argv); + let resolved_argv = resolve_bare_program_on_path(argv, cwd, env_map); + let cmdline_str = argv_to_command_line(&resolved_argv); let mut cmdline: Vec = to_wide(&cmdline_str); let env_block = make_env_block(env_map); let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?; 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..351ca4ad6b --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/program_resolution.rs @@ -0,0 +1,81 @@ +use std::collections::HashMap; +use std::ffi::OsString; +use std::path::Path; + +pub(crate) fn resolve_bare_program_on_path( + argv: &[String], + cwd: &Path, + env_map: &HashMap, +) -> Vec { + let Some(program) = argv.first() else { + return Vec::new(); + }; + + if !is_bare_program_name(program) { + return argv.to_vec(); + } + + let search_path = env_map + .iter() + .find(|(key, _)| key.eq_ignore_ascii_case("PATH")) + .map(|(_, value)| OsString::from(value)); + + let Ok(resolved) = which::which_in(program, search_path.as_ref(), cwd) else { + return argv.to_vec(); + }; + + let mut resolved_argv = argv.to_vec(); + resolved_argv[0] = resolved.to_string_lossy().into_owned(); + resolved_argv +} + +fn is_bare_program_name(program: &str) -> bool { + !program.is_empty() && !program.contains(['\\', '/', ':']) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + #[test] + fn resolves_bare_programs_from_path() { + let temp = TempDir::new().expect("tempdir"); + let bin_dir = temp.path().join("bin"); + fs::create_dir_all(&bin_dir).expect("create bin dir"); + let exe_path = bin_dir.join("go.exe"); + fs::write(&exe_path, b"").expect("write executable stub"); + + let env_map = HashMap::from([( + "PATH".to_string(), + bin_dir.to_string_lossy().into_owned(), + )]); + let argv = vec!["go".to_string(), "version".to_string()]; + + let resolved = resolve_bare_program_on_path(&argv, temp.path(), &env_map); + + assert_eq!(resolved[0], exe_path.to_string_lossy()); + assert_eq!(resolved[1], "version"); + } + + #[test] + fn leaves_explicit_paths_unchanged() { + let env_map = HashMap::from([("PATH".to_string(), "C:\\tools".to_string())]); + let argv = vec![r"C:\tools\go.exe".to_string(), "version".to_string()]; + + let resolved = resolve_bare_program_on_path(&argv, Path::new("."), &env_map); + + assert_eq!(resolved, argv); + } + + #[test] + fn leaves_missing_programs_unchanged() { + let env_map = HashMap::from([("PATH".to_string(), "C:\\tools".to_string())]); + let argv = vec!["missing-tool".to_string(), "--help".to_string()]; + + let resolved = resolve_bare_program_on_path(&argv, Path::new("."), &env_map); + + assert_eq!(resolved, argv); + } +} diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 428b5ff6a1..8471e9f06f 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -370,6 +370,25 @@ fn canonical_existing(paths: &[PathBuf]) -> Vec { .collect() } +fn path_read_roots(env_map: &HashMap) -> Vec { + let path_value = env_map + .get("PATH") + .cloned() + .or_else(|| std::env::var("PATH").ok()) + .unwrap_or_default(); + + path_value + .split(';') + .filter_map(|entry| { + let trimmed = entry.trim().trim_matches('"'); + if trimmed.is_empty() { + return None; + } + Some(PathBuf::from(trimmed)) + }) + .collect() +} + fn profile_read_roots(user_profile: &Path) -> Vec { let entries = match std::fs::read_dir(user_profile) { Ok(entries) => entries, @@ -417,6 +436,7 @@ fn gather_full_read_roots_for_permissions( .into_iter() .map(|root| root.root), ); + roots.extend(path_read_roots(env_map)); canonical_existing(&roots) } @@ -444,6 +464,7 @@ pub(crate) fn gather_read_roots( ); } roots.extend(permissions.readable_roots_for_cwd(command_cwd)); + roots.extend(path_read_roots(env_map)); canonical_existing(&roots) } @@ -1593,6 +1614,30 @@ mod tests { assert!(roots.contains(&expected)); } + #[test] + fn gather_read_roots_include_existing_path_entries() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let command_cwd = tmp.path().join("workspace"); + let toolchain_bin = tmp.path().join("toolchain").join("bin"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&toolchain_bin).expect("create toolchain bin"); + let permission_profile = PermissionProfile::read_only(); + let workspace_roots = workspace_roots_for(command_cwd.as_path()); + let permissions = permissions_for(&permission_profile, workspace_roots.as_slice()); + let mut env_map = HashMap::new(); + env_map.insert( + "PATH".to_string(), + format!("{};{}", toolchain_bin.display(), tmp.path().join("missing").display()), + ); + + let roots = gather_read_roots(&command_cwd, &permissions, &env_map, &codex_home); + let expected_toolchain = + dunce::canonicalize(&toolchain_bin).expect("canonical toolchain bin"); + + assert!(roots.contains(&expected_toolchain)); + } + #[test] fn workspace_write_roots_remain_readable() { let tmp = TempDir::new().expect("tempdir");