From c18f1689a9dbd68619a116601d584cd1c37b041a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 25 Apr 2025 15:58:44 -0700 Subject: [PATCH] fix: small fixes so Codex compiles on Windows (#673) Small fixes required: * `ExitStatusExt` differs because UNIX expects exit code to be `i32` whereas Windows does `u32` * Marking a file "executable only by owner" is a bit more involved on Windows. We just do something approximate for now (and add a TODO) to get things compiling. I created this PR on my personal Windows machine and `cargo test` and `cargo clippy` succeed. Once this is in, I'll rebase https://github.com/openai/codex/pull/665 on top so Windows stays fixed! --- codex-rs/core/src/exec.rs | 4 +-- codex-rs/execpolicy/src/execv_checker.rs | 35 ++++++++++++++++++------ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index fd48558f..aa414e62 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -322,7 +322,7 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { } #[cfg(windows)] -fn synthetic_exit_status(code: u32) -> ExitStatus { +fn synthetic_exit_status(code: i32) -> ExitStatus { use std::os::windows::process::ExitStatusExt; - std::process::ExitStatus::from_raw(code) + std::process::ExitStatus::from_raw(code.try_into().unwrap()) } diff --git a/codex-rs/execpolicy/src/execv_checker.rs b/codex-rs/execpolicy/src/execv_checker.rs index 787fbce1..242ea6d1 100644 --- a/codex-rs/execpolicy/src/execv_checker.rs +++ b/codex-rs/execpolicy/src/execv_checker.rs @@ -13,7 +13,6 @@ use crate::Policy; use crate::Result; use crate::ValidExec; use path_absolutize::*; -use std::os::unix::fs::PermissionsExt; macro_rules! check_file_in_folders { ($file:expr, $folders:expr, $error:ident) => { @@ -120,9 +119,20 @@ fn is_executable_file(path: &str) -> bool { let file_path = Path::new(path); if let Ok(metadata) = std::fs::metadata(file_path) { - let permissions = metadata.permissions(); - // Check if the file is executable (by checking the executable bit for the owner) - return metadata.is_file() && (permissions.mode() & 0o111 != 0); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let permissions = metadata.permissions(); + + // Check if the file is executable (by checking the executable bit for the owner) + return metadata.is_file() && (permissions.mode() & 0o111 != 0); + } + + #[cfg(windows)] + { + // TODO(mbolin): Check against PATHEXT environment variable. + return metadata.is_file(); + } } false @@ -157,10 +167,19 @@ system_path=[{fake_cp:?}] // Create an executable file that can be used with the system_path arg. let fake_cp = temp_dir.path().join("cp"); - let fake_cp_file = std::fs::File::create(&fake_cp).unwrap(); - let mut permissions = fake_cp_file.metadata().unwrap().permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&fake_cp, permissions).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let fake_cp_file = std::fs::File::create(&fake_cp).unwrap(); + let mut permissions = fake_cp_file.metadata().unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&fake_cp, permissions).unwrap(); + } + #[cfg(windows)] + { + std::fs::File::create(&fake_cp).unwrap(); + } // Create root_path and reference to files under the root. let root_path = temp_dir.path().to_path_buf();