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:


6e6338aa87/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.
This commit is contained in:
Michael Bolin
2025-12-12 13:06:49 -08:00
committed by GitHub
parent 26d0d822a2
commit 9009490357
4 changed files with 501 additions and 123 deletions

View File

@@ -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

View File

@@ -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<Vec<Vec<String>>> {
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<Vec<Vec<String>>> {
fn parse_powershell_invocation(executable: &str, args: &[String]) -> Option<Vec<Vec<String>>> {
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<Vec<Vec<String>>> {
// 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<Vec<Vec<String>>> {
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<Vec<Vec<String>>> {
// 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<Vec<Vec<String>>> {
/// Tokenizes an inline PowerShell script and delegates to the command splitter.
/// Examples of when this is called: pwsh.exe -Command '<script>' or pwsh.exe -Command:<script>
fn parse_powershell_script(script: &str) -> Option<Vec<Vec<String>>> {
let tokens = shlex_split(script)?;
split_into_commands(tokens)
}
/// Splits tokens into pipeline segments while ensuring no unsafe separators slip through.
/// e.g. Get-ChildItem | Measure-Object -> [['Get-ChildItem'], ['Measure-Object']]
fn split_into_commands(tokens: Vec<String>) -> Option<Vec<Vec<String>>> {
if tokens.is_empty() {
// Examples rejected here: "pwsh -Command ''" and "powershell -Command \"\"".
return None;
fn parse_powershell_script(executable: &str, script: &str) -> Option<Vec<Vec<String>>> {
if let PowershellParseOutcome::Commands(commands) =
parse_with_powershell_ast(executable, script)
{
Some(commands)
} else {
None
}
let mut commands = Vec::new();
let mut current = Vec::new();
for token in tokens.into_iter() {
match token.as_str() {
"|" | "||" | "&&" | ";" => {
if current.is_empty() {
// Examples rejected here: "pwsh -Command '| Get-ChildItem'" and "pwsh -Command '; dir'".
return None;
}
commands.push(current);
current = Vec::new();
}
// Reject if any token embeds separators, redirection, or call operator characters.
_ if token.contains(['|', ';', '>', '<', '&']) || token.contains("$(") => {
// Examples rejected here: "pwsh -Command 'dir|select'" and "pwsh -Command 'echo hi > out.txt'".
return None;
}
_ => current.push(token),
}
}
if current.is_empty() {
// Examples rejected here: "pwsh -Command 'dir |'" and "pwsh -Command 'Get-ChildItem ;'".
return None;
}
commands.push(current);
Some(commands)
}
/// Returns true when the executable name is one of the supported PowerShell binaries.
@@ -144,6 +121,105 @@ fn is_powershell_executable(exe: &str) -> bool {
)
}
/// Attempts to parse PowerShell using the real PowerShell parser, returning every pipeline element
/// as a flat argv vector when possible. If parsing fails or the AST includes unsupported constructs,
/// we conservatively reject the command instead of trying to split it manually.
fn parse_with_powershell_ast(executable: &str, script: &str) -> PowershellParseOutcome {
let encoded_script = encode_powershell_base64(script);
let encoded_parser_script = encoded_parser_script();
match Command::new(executable)
.args([
"-NoLogo",
"-NoProfile",
"-NonInteractive",
"-EncodedCommand",
encoded_parser_script,
])
.env("CODEX_POWERSHELL_PAYLOAD", &encoded_script)
.output()
{
Ok(output) if output.status.success() => {
if let Ok(result) =
serde_json::from_slice::<PowershellParserOutput>(output.stdout.as_slice())
{
result.into_outcome()
} else {
PowershellParseOutcome::Failed
}
}
_ => PowershellParseOutcome::Failed,
}
}
fn encode_powershell_base64(script: &str) -> String {
let mut utf16 = Vec::with_capacity(script.len() * 2);
for unit in script.encode_utf16() {
utf16.extend_from_slice(&unit.to_le_bytes());
}
BASE64_STANDARD.encode(utf16)
}
fn encoded_parser_script() -> &'static str {
static ENCODED: LazyLock<String> =
LazyLock::new(|| encode_powershell_base64(POWERSHELL_PARSER_SCRIPT));
&ENCODED
}
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct PowershellParserOutput {
status: String,
commands: Option<Vec<Vec<String>>>,
}
impl PowershellParserOutput {
fn into_outcome(self) -> PowershellParseOutcome {
match self.status.as_str() {
"ok" => self
.commands
.filter(|commands| {
!commands.is_empty()
&& commands
.iter()
.all(|cmd| !cmd.is_empty() && cmd.iter().all(|word| !word.is_empty()))
})
.map(PowershellParseOutcome::Commands)
.unwrap_or(PowershellParseOutcome::Unsupported),
"unsupported" => PowershellParseOutcome::Unsupported,
_ => PowershellParseOutcome::Failed,
}
}
}
enum PowershellParseOutcome {
Commands(Vec<Vec<String>>),
Unsupported,
Failed,
}
fn join_arguments_as_script(args: &[String]) -> String {
let mut words = Vec::with_capacity(args.len());
if let Some((first, rest)) = args.split_first() {
words.push(first.clone());
for arg in rest {
words.push(quote_argument(arg));
}
}
words.join(" ")
}
fn quote_argument(arg: &str) -> String {
if arg.is_empty() {
return "''".to_string();
}
if arg.chars().all(|ch| !ch.is_whitespace()) {
return arg.to_string();
}
format!("'{}'", arg.replace('\'', "''"))
}
/// Validates that a parsed PowerShell command stays within our read-only safelist.
/// Everything before this is parsing, and rejecting things that make us feel uncomfortable.
fn is_safe_powershell_command(words: &[String]) -> bool {
@@ -176,17 +252,6 @@ fn is_safe_powershell_command(words: &[String]) -> bool {
}
}
// Block PowerShell call operator or any redirection explicitly.
if words.iter().any(|w| {
matches!(
w.as_str(),
"&" | ">" | ">>" | "1>" | "2>" | "2>&1" | "*>" | "<" | "<<"
)
}) {
// Examples rejected here: "pwsh -Command '& Remove-Item foo'" and "pwsh -Command 'Get-Content foo > bar'".
return false;
}
let command = words[0]
.trim_matches(|c| c == '(' || c == ')')
.trim_start_matches('-')
@@ -279,9 +344,10 @@ fn is_safe_git_command(words: &[String]) -> bool {
false
}
#[cfg(test)]
#[cfg(all(test, windows))]
mod tests {
use super::is_safe_command_windows;
use super::*;
use crate::powershell::try_find_pwsh_executable_blocking;
use std::string::ToString;
/// Converts a slice of string literals into owned `String`s for the tests.
@@ -312,12 +378,14 @@ mod tests {
])));
// pwsh parity
assert!(is_safe_command_windows(&vec_str(&[
"pwsh.exe",
"-NoProfile",
"-Command",
"Get-ChildItem",
])));
if let Some(pwsh) = try_find_pwsh_executable_blocking() {
assert!(is_safe_command_windows(&[
pwsh.as_path().to_str().unwrap().into(),
"-NoProfile".to_string(),
"-Command".to_string(),
"Get-ChildItem".to_string(),
]));
}
}
#[test]
@@ -327,12 +395,14 @@ mod tests {
return;
}
assert!(is_safe_command_windows(&vec_str(&[
r"C:\Program Files\PowerShell\7\pwsh.exe",
"-NoProfile",
"-Command",
"Get-ChildItem -Path .",
])));
if let Some(pwsh) = try_find_pwsh_executable_blocking() {
assert!(is_safe_command_windows(&[
pwsh.as_path().to_str().unwrap().into(),
"-NoProfile".to_string(),
"-Command".to_string(),
"Get-ChildItem -Path .".to_string(),
]));
}
assert!(is_safe_command_windows(&vec_str(&[
r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe",
@@ -343,47 +413,53 @@ mod tests {
#[test]
fn allows_read_only_pipelines_and_git_usage() {
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-NoLogo",
"-NoProfile",
"-Command",
"rg --files-with-matches foo | Measure-Object | Select-Object -ExpandProperty Count",
])));
let Some(pwsh) = try_find_pwsh_executable_blocking() else {
return;
};
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-NoLogo",
"-NoProfile",
"-Command",
"Get-Content foo.rs | Select-Object -Skip 200",
])));
let pwsh: String = pwsh.as_path().to_str().unwrap().into();
assert!(is_safe_command_windows(&[
pwsh.clone(),
"-NoLogo".to_string(),
"-NoProfile".to_string(),
"-Command".to_string(),
"rg --files-with-matches foo | Measure-Object | Select-Object -ExpandProperty Count"
.to_string()
]));
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-NoLogo",
"-NoProfile",
"-Command",
"git -c core.pager=cat show HEAD:foo.rs",
])));
assert!(is_safe_command_windows(&[
pwsh.clone(),
"-NoLogo".to_string(),
"-NoProfile".to_string(),
"-Command".to_string(),
"Get-Content foo.rs | Select-Object -Skip 200".to_string()
]));
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-Command",
"-git cat-file -p HEAD:foo.rs",
])));
assert!(is_safe_command_windows(&[
pwsh.clone(),
"-NoLogo".to_string(),
"-NoProfile".to_string(),
"-Command".to_string(),
"git -c core.pager=cat show HEAD:foo.rs".to_string()
]));
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-Command",
"(Get-Content foo.rs -Raw)",
])));
assert!(is_safe_command_windows(&[
pwsh.clone(),
"-Command".to_string(),
"-git cat-file -p HEAD:foo.rs".to_string()
]));
assert!(is_safe_command_windows(&vec_str(&[
"pwsh",
"-Command",
"Get-Item foo.rs | Select-Object Length",
])));
assert!(is_safe_command_windows(&[
pwsh.clone(),
"-Command".to_string(),
"(Get-Content foo.rs -Raw)".to_string()
]));
assert!(is_safe_command_windows(&[
pwsh,
"-Command".to_string(),
"Get-Item foo.rs | Select-Object Length".to_string()
]));
}
#[test]
@@ -455,5 +531,93 @@ mod tests {
"-Command",
"Get-Content (New-Item bar.txt)",
])));
// Unsafe @ expansion.
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"ls @(calc.exe)"
])));
// Unsupported constructs that the AST parser refuses (no fallback to manual splitting).
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"ls && pwd"
])));
// Sub-expressions are rejected even if they contain otherwise safe commands.
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"Write-Output $(Get-Content foo)"
])));
// Empty words from the parser (e.g. '') are rejected.
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"''"
])));
}
#[test]
fn accepts_constant_expression_arguments() {
assert!(is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"Get-Content 'foo bar'"
])));
assert!(is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"Get-Content \"foo bar\""
])));
}
#[test]
fn rejects_dynamic_arguments() {
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"Get-Content $foo"
])));
assert!(!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-Command",
"Write-Output \"foo $bar\""
])));
}
#[test]
fn uses_invoked_powershell_variant_for_parsing() {
if !cfg!(windows) {
return;
}
let chain = "pwd && ls";
assert!(
!is_safe_command_windows(&vec_str(&[
"powershell.exe",
"-NoProfile",
"-Command",
chain,
])),
"`{chain}` is not recognized by powershell.exe"
);
if let Some(pwsh) = try_find_pwsh_executable_blocking() {
assert!(
is_safe_command_windows(&[
pwsh.as_path().to_str().unwrap().into(),
"-NoProfile".to_string(),
"-Command".to_string(),
chain.to_string(),
]),
"`{chain}` should be considered safe to pwsh.exe"
);
}
}
}

View File

@@ -1,6 +1,6 @@
use std::path::PathBuf;
#[cfg(windows)]
#[cfg(any(windows, test))]
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::shell::ShellType;
@@ -57,7 +57,7 @@ pub(crate) fn try_find_powershellish_executable_blocking() -> Option<AbsolutePat
}
/// This function attempts to find a powershell.exe executable on the system.
#[cfg(windows)]
#[cfg(any(windows, test))]
pub(crate) fn try_find_powershell_executable_blocking() -> Option<AbsolutePathBuf> {
try_find_powershellish_executable_in_path(&["powershell.exe"])
}
@@ -72,7 +72,7 @@ pub(crate) fn try_find_powershell_executable_blocking() -> Option<AbsolutePathBu
/// pwsh.exe must be installed separately by the user. And even when the user
/// has installed pwsh.exe, it may not be available in the system PATH, in which
/// case we attempt to locate it via other means.
#[cfg(windows)]
#[cfg(any(windows, test))]
pub(crate) fn try_find_pwsh_executable_blocking() -> Option<AbsolutePathBuf> {
if let Some(ps_home) = std::process::Command::new("cmd")
.args(["/C", "pwsh", "-NoProfile", "-Command", "$PSHOME"])
@@ -99,7 +99,7 @@ pub(crate) fn try_find_pwsh_executable_blocking() -> Option<AbsolutePathBuf> {
try_find_powershellish_executable_in_path(&["pwsh.exe"])
}
#[cfg(windows)]
#[cfg(any(windows, test))]
fn try_find_powershellish_executable_in_path(candidates: &[&str]) -> Option<AbsolutePathBuf> {
for candidate in candidates {
let Ok(resolved_path) = which::which(candidate) else {
@@ -120,7 +120,7 @@ fn try_find_powershellish_executable_in_path(candidates: &[&str]) -> Option<Abso
None
}
#[cfg(windows)]
#[cfg(any(windows, test))]
fn is_powershellish_executable_available(powershell_or_pwsh_exe: &std::path::Path) -> bool {
// This test works for both powershell.exe and pwsh.exe.
std::process::Command::new(powershell_or_pwsh_exe)

View File

@@ -303,6 +303,8 @@ mod tests {
use crate::codex::make_session_and_context;
use crate::exec_env::create_env;
use crate::is_safe_command::is_known_safe_command;
use crate::powershell::try_find_powershell_executable_blocking;
use crate::powershell::try_find_pwsh_executable_blocking;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::shell::ShellType;
@@ -328,12 +330,23 @@ mod tests {
};
assert_safe(&zsh_shell, "ls -la");
let powershell = Shell {
shell_type: ShellType::PowerShell,
shell_path: PathBuf::from("pwsh.exe"),
shell_snapshot: None,
};
assert_safe(&powershell, "ls -Name");
if let Some(path) = try_find_powershell_executable_blocking() {
let powershell = Shell {
shell_type: ShellType::PowerShell,
shell_path: path.to_path_buf(),
shell_snapshot: None,
};
assert_safe(&powershell, "ls -Name");
}
if let Some(path) = try_find_pwsh_executable_blocking() {
let pwsh = Shell {
shell_type: ShellType::PowerShell,
shell_path: path.to_path_buf(),
shell_snapshot: None,
};
assert_safe(&pwsh, "ls -Name");
}
}
fn assert_safe(shell: &Shell, command: &str) {