From 9009490357fc3843c14cab1c3e0132acc70487ed Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 12 Dec 2025 13:06:49 -0800 Subject: [PATCH] fix: use PowerShell to parse PowerShell (#7607) Previous to this PR, we used a hand-rolled PowerShell parser in `windows_safe_commands.rs` to take a `&str` of PowerShell script see if it is equivalent to a list of `execvp(3)` invocations, and if so, we then test each using `is_safe_powershell_command()` to determine if the overall command is safe: https://github.com/openai/codex/blob/6e6338aa876bb4258abe25b02ac6417b8ea9dff0/codex-rs/core/src/command_safety/windows_safe_commands.rs#L89-L98 Unfortunately, our PowerShell parser did not recognize `@(...)` as a special construct, so it was treated as an ordinary token. This meant that the following would erroneously be considered "safe:" ```powershell ls @(calc.exe) ``` The fix introduced in this PR is to do something comparable what we do for Bash/Zsh, which is to use a "proper" parser to derive the list of `execvp(3)` calls. For Bash/Zsh, we rely on https://crates.io/crates/tree-sitter-bash, but there does not appear to be a crate of comparable quality for parsing PowerShell statically (https://github.com/airbus-cert/tree-sitter-powershell/ is the best thing I found). Instead, in this PR, we use a PowerShell script to parse the input PowerShell program to produce the AST. --- .../src/command_safety/powershell_parser.ps1 | 201 +++++++++ .../command_safety/windows_safe_commands.rs | 388 +++++++++++++----- codex-rs/core/src/powershell.rs | 10 +- codex-rs/core/src/tools/handlers/shell.rs | 25 +- 4 files changed, 501 insertions(+), 123 deletions(-) create mode 100644 codex-rs/core/src/command_safety/powershell_parser.ps1 diff --git a/codex-rs/core/src/command_safety/powershell_parser.ps1 b/codex-rs/core/src/command_safety/powershell_parser.ps1 new file mode 100644 index 000000000..af71cb7f3 --- /dev/null +++ b/codex-rs/core/src/command_safety/powershell_parser.ps1 @@ -0,0 +1,201 @@ +$ErrorActionPreference = 'Stop' + +$payload = $env:CODEX_POWERSHELL_PAYLOAD +if ([string]::IsNullOrEmpty($payload)) { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +try { + $source = + [System.Text.Encoding]::Unicode.GetString( + [System.Convert]::FromBase64String($payload) + ) +} catch { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +$tokens = $null +$errors = $null + +$ast = $null +try { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + $source, + [ref]$tokens, + [ref]$errors + ) +} catch { + Write-Output '{"status":"parse_failed"}' + exit 0 +} + +if ($errors.Count -gt 0) { + Write-Output '{"status":"parse_errors"}' + exit 0 +} + +function Convert-CommandElement { + param($element) + + if ($element -is [System.Management.Automation.Language.StringConstantExpressionAst]) { + return @($element.Value) + } + + if ($element -is [System.Management.Automation.Language.ExpandableStringExpressionAst]) { + if ($element.NestedExpressions.Count -gt 0) { + return $null + } + return @($element.Value) + } + + if ($element -is [System.Management.Automation.Language.ConstantExpressionAst]) { + return @($element.Value.ToString()) + } + + if ($element -is [System.Management.Automation.Language.CommandParameterAst]) { + if ($element.Argument -eq $null) { + return @('-' + $element.ParameterName) + } + + if ($element.Argument -is [System.Management.Automation.Language.StringConstantExpressionAst]) { + return @('-' + $element.ParameterName, $element.Argument.Value) + } + + if ($element.Argument -is [System.Management.Automation.Language.ConstantExpressionAst]) { + return @('-' + $element.ParameterName, $element.Argument.Value.ToString()) + } + + return $null + } + + return $null +} + +function Convert-PipelineElement { + param($element) + + if ($element -is [System.Management.Automation.Language.CommandAst]) { + if ($element.Redirections.Count -gt 0) { + return $null + } + + if ( + $element.InvocationOperator -ne $null -and + $element.InvocationOperator -ne [System.Management.Automation.Language.TokenKind]::Unknown + ) { + return $null + } + + $parts = @() + foreach ($commandElement in $element.CommandElements) { + $converted = Convert-CommandElement $commandElement + if ($converted -eq $null) { + return $null + } + $parts += $converted + } + return $parts + } + + if ($element -is [System.Management.Automation.Language.CommandExpressionAst]) { + if ($element.Redirections.Count -gt 0) { + return $null + } + + if ($element.Expression -is [System.Management.Automation.Language.ParenExpressionAst]) { + $innerPipeline = $element.Expression.Pipeline + if ($innerPipeline -and $innerPipeline.PipelineElements.Count -eq 1) { + return Convert-PipelineElement $innerPipeline.PipelineElements[0] + } + } + + return $null + } + + return $null +} + +function Add-CommandsFromPipelineAst { + param($pipeline, $commands) + + if ($pipeline.PipelineElements.Count -eq 0) { + return $false + } + + foreach ($element in $pipeline.PipelineElements) { + $words = Convert-PipelineElement $element + if ($words -eq $null -or $words.Count -eq 0) { + return $false + } + $null = $commands.Add($words) + } + + return $true +} + +function Add-CommandsFromPipelineChain { + param($chain, $commands) + + if (-not (Add-CommandsFromPipelineBase $chain.LhsPipelineChain $commands)) { + return $false + } + + if (-not (Add-CommandsFromPipelineAst $chain.RhsPipeline $commands)) { + return $false + } + + return $true +} + +function Add-CommandsFromPipelineBase { + param($pipeline, $commands) + + if ($pipeline -is [System.Management.Automation.Language.PipelineAst]) { + return Add-CommandsFromPipelineAst $pipeline $commands + } + + if ($pipeline -is [System.Management.Automation.Language.PipelineChainAst]) { + return Add-CommandsFromPipelineChain $pipeline $commands + } + + return $false +} + +$commands = [System.Collections.ArrayList]::new() + +foreach ($statement in $ast.EndBlock.Statements) { + if (-not (Add-CommandsFromPipelineBase $statement $commands)) { + $commands = $null + break + } +} + +if ($commands -ne $null) { + $normalized = [System.Collections.ArrayList]::new() + foreach ($cmd in $commands) { + if ($cmd -is [string]) { + $null = $normalized.Add(@($cmd)) + continue + } + + if ($cmd -is [System.Array] -or $cmd -is [System.Collections.IEnumerable]) { + $null = $normalized.Add(@($cmd)) + continue + } + + $normalized = $null + break + } + + $commands = $normalized +} + +$result = if ($commands -eq $null) { + @{ status = 'unsupported' } +} else { + @{ status = 'ok'; commands = $commands } +} + +,$result | ConvertTo-Json -Depth 3 diff --git a/codex-rs/core/src/command_safety/windows_safe_commands.rs b/codex-rs/core/src/command_safety/windows_safe_commands.rs index a1d3b297f..ac479a4d2 100644 --- a/codex-rs/core/src/command_safety/windows_safe_commands.rs +++ b/codex-rs/core/src/command_safety/windows_safe_commands.rs @@ -1,30 +1,38 @@ -use shlex::split as shlex_split; +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use serde::Deserialize; use std::path::Path; +use std::process::Command; +use std::sync::LazyLock; + +const POWERSHELL_PARSER_SCRIPT: &str = include_str!("powershell_parser.ps1"); /// 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) { - return commands + commands .iter() - .all(|cmd| is_safe_powershell_command(cmd.as_slice())); + .all(|cmd| is_safe_powershell_command(cmd.as_slice())) + } else { + // Only PowerShell invocations are allowed on Windows for now; anything else is unsafe. + false } - // Only PowerShell invocations are allowed on Windows for now; anything else is unsafe. - false } /// 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) { - return None; + if is_powershell_executable(exe) { + parse_powershell_invocation(exe, rest) + } else { + None } - parse_powershell_invocation(rest) } /// Parses a PowerShell invocation into discrete command vectors, rejecting unsafe patterns. -fn parse_powershell_invocation(args: &[String]) -> Option>> { +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; @@ -42,7 +50,7 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { // Examples rejected here: "pwsh -Command foo bar" and "powershell -c ls extra". return None; } - return parse_powershell_script(script); + return parse_powershell_script(executable, script); } _ if lower.starts_with("-command:") || lower.starts_with("/command:") => { if idx + 1 != args.len() { @@ -51,7 +59,7 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { return None; } let script = arg.split_once(':')?.1; - return parse_powershell_script(script); + return parse_powershell_script(executable, script); } // Benign, no-arg flags we tolerate. @@ -77,7 +85,8 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { // This happens if powershell is invoked without -Command, e.g. // ["pwsh", "-NoLogo", "git", "-c", "core.pager=cat", "status"] _ => { - return split_into_commands(args[idx..].to_vec()); + let script = join_arguments_as_script(&args[idx..]); + return parse_powershell_script(executable, &script); } } } @@ -88,46 +97,14 @@ fn parse_powershell_invocation(args: &[String]) -> Option>> { /// Tokenizes an inline PowerShell script and delegates to the command splitter. /// Examples of when this is called: pwsh.exe -Command '