From 42c80385cd6e2c8efc8b85c39bb9bde5edf01fc3 Mon Sep 17 00:00:00 2001 From: Adrian <145513011+adrian-openai@users.noreply.github.com> Date: Thu, 28 May 2026 20:00:35 -0700 Subject: [PATCH] [codex] Avoid PowerShell safety parsing off Windows (#24946) ## Summary This fixes BUGB-17567 by preventing non-Windows command safety classification from invoking the Windows PowerShell safelist/parser path. Previously, `is_known_safe_command` called the Windows PowerShell classifier on every platform. That classifier recognizes `pwsh`/`powershell` by basename and delegates script parsing to the PowerShell AST parser. The parser starts the supplied executable, so on macOS/Linux a repository-controlled `pwsh` path could execute during safety parsing before the normal sandboxed command execution path. The change gates the Windows PowerShell classifier and module behind `#[cfg(windows)]`. On macOS/Linux, PowerShell-looking commands are no longer auto-approved by the Windows classifier and instead fall through to the normal non-Windows safe-command logic. ## Validation - `/private/tmp/codex-tools/bin/just fmt` - `PATH=/private/tmp/codex-tools/bin:$PATH /private/tmp/codex-tools/bin/just test -p codex-shell-command` The focused test run passed 135 tests with 0 skipped and completed the crate bench-smoke step. ## Notes This PR is scoped to the BUGB-17567 macOS/Linux path. Windows still uses the PowerShell classifier; a separate hardening follow-up should ensure Windows safety parsing only executes a trusted PowerShell parser binary and does not spawn the command's `argv[0]` when that path may be repository-controlled. --- .../src/command_safety/is_safe_command.rs | 55 ++++++++++++++++++- .../shell-command/src/command_safety/mod.rs | 1 + 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/codex-rs/shell-command/src/command_safety/is_safe_command.rs b/codex-rs/shell-command/src/command_safety/is_safe_command.rs index b35144a12b..bca44139cd 100644 --- a/codex-rs/shell-command/src/command_safety/is_safe_command.rs +++ b/codex-rs/shell-command/src/command_safety/is_safe_command.rs @@ -4,6 +4,7 @@ use crate::command_safety::is_dangerous_command::executable_name_lookup_key; // may appear before it (e.g., `-C`, `-c`, `--git-dir`). // Implemented in `is_dangerous_command` and shared here. use crate::command_safety::is_dangerous_command::find_git_subcommand; +#[cfg(windows)] use crate::command_safety::windows_safe_commands::is_safe_command_windows; #[cfg(windows)] use crate::command_safety::windows_safe_commands::is_safe_powershell_words as is_safe_powershell_words_windows; @@ -20,8 +21,11 @@ pub fn is_known_safe_command(command: &[String]) -> bool { }) .collect(); - if is_safe_command_windows(&command) { - return true; + #[cfg(windows)] + { + if is_safe_command_windows(&command) { + return true; + } } if is_safe_to_call_with_exec(&command) { @@ -748,4 +752,51 @@ mod tests { assert!(!is_safe_powershell_words(&command)); } } + + #[cfg(unix)] + #[test] + fn non_windows_safe_classification_does_not_spawn_repo_powershell_path() { + use std::fs; + use std::io::Write; + use std::os::unix::fs::PermissionsExt; + use std::time::SystemTime; + use std::time::UNIX_EPOCH; + + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock should be after unix epoch") + .as_nanos(); + let temp_dir = std::env::temp_dir().join(format!( + "codex-safe-command-pwsh-test-{}-{unique}", + std::process::id() + )); + fs::create_dir(&temp_dir).expect("create temp dir for fake pwsh"); + let fake_pwsh = temp_dir.join("pwsh"); + let marker = temp_dir.join("marker"); + let quoted_marker = marker.to_string_lossy().replace('\'', "'\\''"); + + let mut script = fs::File::create(&fake_pwsh).expect("create fake pwsh"); + writeln!( + script, + "#!/bin/sh\nprintf spawned > '{quoted_marker}'\nexit 0" + ) + .expect("write fake pwsh"); + let mut permissions = fs::metadata(&fake_pwsh) + .expect("stat fake pwsh") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&fake_pwsh, permissions).expect("make fake pwsh executable"); + + assert!(!is_known_safe_command(&[ + fake_pwsh.to_string_lossy().into_owned(), + "-Command".to_string(), + "Get-ChildItem".to_string(), + ])); + assert!( + !marker.exists(), + "non-Windows safety classification must not spawn a PowerShell-looking path" + ); + + fs::remove_dir_all(temp_dir).expect("remove temp dir"); + } } diff --git a/codex-rs/shell-command/src/command_safety/mod.rs b/codex-rs/shell-command/src/command_safety/mod.rs index 2040364e5b..60bed2effc 100644 --- a/codex-rs/shell-command/src/command_safety/mod.rs +++ b/codex-rs/shell-command/src/command_safety/mod.rs @@ -2,5 +2,6 @@ mod powershell_parser; pub mod is_dangerous_command; pub mod is_safe_command; +#[cfg(windows)] pub(crate) mod windows_safe_commands; pub(crate) use powershell_parser::try_parse_powershell_ast_commands;