From a699c879063d48321679954a4605d5f47ac14df7 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Thu, 9 Apr 2026 10:11:44 -0700 Subject: [PATCH] Unwrap PowerShell commands for exec policy --- codex-rs/core/src/exec_policy.rs | 7 + codex-rs/core/tests/suite/exec_policy.rs | 5 - .../shell-command/src/command_safety/mod.rs | 3 + .../src/command_safety/powershell_parser.rs | 4 +- .../command_safety/windows_safe_commands.rs | 130 +---------------- codex-rs/shell-command/src/powershell.rs | 137 ++++++++++++++++++ 6 files changed, 151 insertions(+), 135 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 54ad8058d0..c328c0b6e7 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -26,6 +26,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; use codex_shell_command::is_safe_command::is_known_safe_command; +use codex_shell_command::powershell::parse_powershell_command_sequence; use thiserror::Error; use tokio::fs; use tokio::sync::Semaphore; @@ -675,6 +676,12 @@ fn default_policy_path(codex_home: &Path) -> PathBuf { } fn commands_for_exec_policy(command: &[String]) -> (Vec>, bool) { + if let Some(commands) = parse_powershell_command_sequence(command) + && !commands.is_empty() + { + return (commands, false); + } + if let Some(commands) = parse_shell_lc_plain_commands(command) && !commands.is_empty() { diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 4fae6aafdf..4ab9f841f5 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -78,11 +78,6 @@ fn assert_no_matched_rules_invariant(output_item: &Value) { #[tokio::test] async fn execpolicy_blocks_shell_invocation() -> Result<()> { - // TODO execpolicy doesn't parse powershell commands yet - if cfg!(windows) { - return Ok(()); - } - let mut builder = test_codex().with_config(|config| { let policy_path = config.codex_home.join("rules").join("policy.rules"); fs::create_dir_all( diff --git a/codex-rs/shell-command/src/command_safety/mod.rs b/codex-rs/shell-command/src/command_safety/mod.rs index 31fa64f8af..3f3e6d3e27 100644 --- a/codex-rs/shell-command/src/command_safety/mod.rs +++ b/codex-rs/shell-command/src/command_safety/mod.rs @@ -3,3 +3,6 @@ mod powershell_parser; pub mod is_dangerous_command; pub mod is_safe_command; pub(crate) mod windows_safe_commands; + +pub(crate) use powershell_parser::PowershellParseOutcome; +pub(crate) use powershell_parser::parse_with_powershell_ast; diff --git a/codex-rs/shell-command/src/command_safety/powershell_parser.rs b/codex-rs/shell-command/src/command_safety/powershell_parser.rs index bf09e60be1..5188458953 100644 --- a/codex-rs/shell-command/src/command_safety/powershell_parser.rs +++ b/codex-rs/shell-command/src/command_safety/powershell_parser.rs @@ -24,7 +24,7 @@ const POWERSHELL_PARSER_SCRIPT: &str = include_str!("powershell_parser.ps1"); /// We keep the cache behind one mutex because each child process speaks a simple /// request/response protocol over a single stdin/stdout pair, so callers targeting the same /// executable must serialize access anyway. -pub(super) fn parse_with_powershell_ast(executable: &str, script: &str) -> PowershellParseOutcome { +pub(crate) fn parse_with_powershell_ast(executable: &str, script: &str) -> PowershellParseOutcome { static PARSER_PROCESSES: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); @@ -35,7 +35,7 @@ pub(super) fn parse_with_powershell_ast(executable: &str, script: &str) -> Power } #[derive(Debug, PartialEq, Eq)] -pub(super) enum PowershellParseOutcome { +pub(crate) enum PowershellParseOutcome { Commands(Vec>), Unsupported, Failed, diff --git a/codex-rs/shell-command/src/command_safety/windows_safe_commands.rs b/codex-rs/shell-command/src/command_safety/windows_safe_commands.rs index b6c5d863bd..6336227a42 100644 --- a/codex-rs/shell-command/src/command_safety/windows_safe_commands.rs +++ b/codex-rs/shell-command/src/command_safety/windows_safe_commands.rs @@ -1,12 +1,10 @@ use crate::command_safety::is_dangerous_command::git_global_option_requires_prompt; -use crate::command_safety::powershell_parser::PowershellParseOutcome; -use crate::command_safety::powershell_parser::parse_with_powershell_ast; -use std::path::Path; +use crate::powershell::parse_powershell_command_sequence; /// On Windows, we conservatively allow only clearly read-only PowerShell invocations /// that match a small safelist. Anything else (including direct CMD commands) is unsafe. pub fn is_safe_command_windows(command: &[String]) -> bool { - if let Some(commands) = try_parse_powershell_command_sequence(command) { + if let Some(commands) = parse_powershell_command_sequence(command) { commands .iter() .all(|cmd| is_safe_powershell_command(cmd.as_slice())) @@ -16,130 +14,6 @@ pub fn is_safe_command_windows(command: &[String]) -> bool { } } -/// Returns each command sequence if the invocation starts with a PowerShell binary. -/// For example, the tokens from `pwsh Get-ChildItem | Measure-Object` become two sequences. -fn try_parse_powershell_command_sequence(command: &[String]) -> Option>> { - let (exe, rest) = command.split_first()?; - if is_powershell_executable(exe) { - parse_powershell_invocation(exe, rest) - } else { - None - } -} - -/// Parses a PowerShell invocation into discrete command vectors, rejecting unsafe patterns. -fn parse_powershell_invocation(executable: &str, args: &[String]) -> Option>> { - if args.is_empty() { - // Examples rejected here: "pwsh" and "powershell.exe" with no additional arguments. - return None; - } - - let mut idx = 0; - while idx < args.len() { - let arg = &args[idx]; - let lower = arg.to_ascii_lowercase(); - match lower.as_str() { - "-command" | "/command" | "-c" => { - let script = args.get(idx + 1)?; - if idx + 2 != args.len() { - // Reject if there is more than one token representing the actual command. - // Examples rejected here: "pwsh -Command foo bar" and "powershell -c ls extra". - return None; - } - return parse_powershell_script(executable, script); - } - _ if lower.starts_with("-command:") || lower.starts_with("/command:") => { - if idx + 1 != args.len() { - // Reject if there are more tokens after the command itself. - // Examples rejected here: "pwsh -Command:dir C:\\" and "powershell /Command:dir C:\\" with trailing args. - return None; - } - let script = arg.split_once(':')?.1; - return parse_powershell_script(executable, script); - } - - // Benign, no-arg flags we tolerate. - "-nologo" | "-noprofile" | "-noninteractive" | "-mta" | "-sta" => { - idx += 1; - continue; - } - - // Explicitly forbidden/opaque or unnecessary for read-only operations. - "-encodedcommand" | "-ec" | "-file" | "/file" | "-windowstyle" | "-executionpolicy" - | "-workingdirectory" => { - // Examples rejected here: "pwsh -EncodedCommand ..." and "powershell -File script.ps1". - return None; - } - - // Unknown switch → bail conservatively. - _ if lower.starts_with('-') => { - // Examples rejected here: "pwsh -UnknownFlag" and "powershell -foo bar". - return None; - } - - // If we hit non-flag tokens, treat the remainder as a command sequence. - // This happens if powershell is invoked without -Command, e.g. - // ["pwsh", "-NoLogo", "git", "-c", "core.pager=cat", "status"] - _ => { - let script = join_arguments_as_script(&args[idx..]); - return parse_powershell_script(executable, &script); - } - } - } - - // Examples rejected here: "pwsh" and "powershell.exe -NoLogo" without a script. - None -} - -/// Tokenizes an inline PowerShell script and delegates to the command splitter. -/// Examples of when this is called: pwsh.exe -Command '