From 27f22b54aef4d7e5eb6c564e969c961c74605461 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Mon, 22 Jun 2026 20:09:20 -0700 Subject: [PATCH] [codex] Reject unlowered PowerShell AST regions (#24092) ## Why On Windows, Codex uses a PowerShell safe-command classifier to decide whether a command is read-only enough to run without additional approval. The classifier lowers `EndBlock.Statements` into argv-like command words and checks those words against a safelist. PowerShell can execute code stored elsewhere in the AST. Parameter defaults, named blocks, `using` preambles, and top-level `trap` handlers are not represented in the lowered statement list. Ignoring those regions can make a side-effecting script look like a read-only command. ## What Fail closed whenever a PowerShell script contains executable AST content that the current lowering does not represent. ## How - Return `unsupported` for parameter, dynamic-parameter, begin, process, and clean blocks. - Return `unsupported` for `using module` and `using assembly` preambles. - Return `unsupported` for non-empty `EndBlock.Traps` collections. - Preserve compatibility with Windows PowerShell 5.1 by looking up `CleanBlock` dynamically. - Treat `unsupported` as a failure to prove that the command is safe, routing it through the normal approval path. - Add parser-level and end-to-end regressions for parameter blocks, named blocks, using statements, and trap handlers. This does not make these PowerShell forms invalid or prevent them from running. It prevents automatic safe-command approval when the classifier cannot account for all executable behavior. ## Testing - `just test -p codex-shell-command` - Windows CI exercises the parser and end-to-end safe-command regressions against a real PowerShell installation. --------- Co-authored-by: viyatb-oai --- .../src/command_safety/powershell_parser.ps1 | 15 +++++ .../src/command_safety/powershell_parser.rs | 58 +++++++++++++++++++ .../command_safety/windows_safe_commands.rs | 40 +++++++++++++ 3 files changed, 113 insertions(+) diff --git a/codex-rs/shell-command/src/command_safety/powershell_parser.ps1 b/codex-rs/shell-command/src/command_safety/powershell_parser.ps1 index de696c88b..92291e7c9 100644 --- a/codex-rs/shell-command/src/command_safety/powershell_parser.ps1 +++ b/codex-rs/shell-command/src/command_safety/powershell_parser.ps1 @@ -42,6 +42,21 @@ function Invoke-ParseRequest { return @{ id = $RequestId; status = 'parse_errors' } } + # Top-level AST regions and collections outside the end-block statement list + # can execute code that the command lowering below does not inspect. + $cleanBlock = $ast.PSObject.Properties['CleanBlock'] + if ( + $ast.ParamBlock -ne $null -or + $ast.DynamicParamBlock -ne $null -or + $ast.BeginBlock -ne $null -or + $ast.ProcessBlock -ne $null -or + ($cleanBlock -ne $null -and $cleanBlock.Value -ne $null) -or + $ast.UsingStatements.Count -gt 0 -or + $ast.EndBlock.Traps.Count -gt 0 + ) { + return @{ id = $RequestId; status = 'unsupported' } + } + # PowerShell's stop-parsing marker hands the remaining source text to native # commands with runtime argument handling that does not match the AST shape we # flatten below. Keep that form out of the argv-like lowering path entirely. 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 ddcbdf572..00b18af6f 100644 --- a/codex-rs/shell-command/src/command_safety/powershell_parser.rs +++ b/codex-rs/shell-command/src/command_safety/powershell_parser.rs @@ -310,4 +310,62 @@ mod tests { .unwrap(); assert_eq!(parsed, PowershellParseOutcome::Unsupported); } + + #[test] + fn parser_process_rejects_param_blocks() { + let Some(powershell) = try_find_powershell_executable_blocking() else { + return; + }; + let powershell = powershell.as_path().to_str().unwrap(); + let mut parser = PowershellParserProcess::spawn(powershell).unwrap(); + + let parsed = parser + .parse("param([string]$path = (Get-Location)) Write-Output test") + .unwrap(); + assert_eq!(parsed, PowershellParseOutcome::Unsupported); + } + + #[test] + fn parser_process_rejects_named_blocks() { + let Some(powershell) = try_find_powershell_executable_blocking() else { + return; + }; + let powershell = powershell.as_path().to_str().unwrap(); + let mut parser = PowershellParserProcess::spawn(powershell).unwrap(); + + let parsed = parser + .parse("begin { Set-Content codex_poc.txt pwned } end { Get-Content Cargo.toml }") + .unwrap(); + assert_eq!(parsed, PowershellParseOutcome::Unsupported); + } + + #[test] + fn parser_process_rejects_using_statements() { + let Some(powershell) = try_find_powershell_executable_blocking() else { + return; + }; + let powershell = powershell.as_path().to_str().unwrap(); + let mut parser = PowershellParserProcess::spawn(powershell).unwrap(); + + let parsed = parser + .parse("using module ./codex_poc.psm1\nGet-Content Cargo.toml") + .unwrap(); + assert_eq!(parsed, PowershellParseOutcome::Unsupported); + } + + #[test] + fn parser_process_rejects_trap_blocks() { + let Some(powershell) = try_find_powershell_executable_blocking() else { + return; + }; + let powershell = powershell.as_path().to_str().unwrap(); + let mut parser = PowershellParserProcess::spawn(powershell).unwrap(); + + let parsed = parser + .parse( + "trap { Set-Content codex_poc.txt pwned; continue } Get-Content missing -ErrorAction Stop", + ) + .unwrap(); + assert_eq!(parsed, PowershellParseOutcome::Unsupported); + } } 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 a8dcef6de..df2025253 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 @@ -412,6 +412,46 @@ mod tests { ]))); } + #[test] + fn rejects_powershell_param_blocks() { + assert!(!is_safe_command_windows(&vec_str(&[ + "powershell.exe", + "-NoProfile", + "-Command", + "param([string]$path = (Get-Location)) Write-Output test", + ]))); + } + + #[test] + fn rejects_powershell_named_blocks() { + assert!(!is_safe_command_windows(&vec_str(&[ + "powershell.exe", + "-NoProfile", + "-Command", + "begin { Set-Content codex_poc.txt pwned } end { Get-Content Cargo.toml }", + ]))); + } + + #[test] + fn rejects_powershell_using_statements() { + assert!(!is_safe_command_windows(&vec_str(&[ + "powershell.exe", + "-NoProfile", + "-Command", + "using module ./codex_poc.psm1\nGet-Content Cargo.toml", + ]))); + } + + #[test] + fn rejects_powershell_trap_blocks() { + assert!(!is_safe_command_windows(&vec_str(&[ + "powershell.exe", + "-NoProfile", + "-Command", + "trap { Set-Content codex_poc.txt pwned; continue } Get-Content missing -ErrorAction Stop", + ]))); + } + #[test] fn rejects_powershell_commands_with_side_effects() { assert!(!is_safe_command_windows(&vec_str(&[