mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[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 <viyatb@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
7c22d376e5
commit
27f22b54ae
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(&[
|
||||
|
||||
Reference in New Issue
Block a user