mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
execpolicy: unwrap PowerShell -Command wrappers on Windows (#20336)
## Why On Windows, Codex runs shell commands through a top-level `powershell.exe -NoProfile -Command ...` wrapper. `execpolicy` was matching that wrapper instead of the inner command, so prefix rules like `["git", "push"]` did not fire for PowerShell-wrapped commands even though the same normalization already happens for `bash -lc` on Unix. This change makes the Windows shell wrapper transparent to rule matching while preserving the existing Windows unmatched-command safelist and dangerous-command heuristics. ## What changed - add `parse_powershell_command_plain_commands()` in `shell-command/src/powershell.rs` to unwrap the top-level PowerShell `-Command` body with `extract_powershell_command()` and parse it with the existing PowerShell AST parser - update `core/src/exec_policy.rs` so `commands_for_exec_policy()` treats top-level PowerShell wrappers like `bash -lc` and evaluates rules against the parsed inner commands - carry a small `ExecPolicyCommandOrigin` through unmatched-command evaluation and expose `is_safe_powershell_words()` / `is_dangerous_powershell_words()` so Windows safelist and dangerous-command checks still work after unwrap - add Windows-focused tests for wrapped PowerShell prompt/allow matches, wrapper parsing, and unmatched safe/dangerous inner commands, and re-enable the end-to-end `execpolicy_blocks_shell_invocation` test on Windows ## Testing - `cargo test -p codex-shell-command`
This commit is contained in:
@@ -28,6 +28,21 @@ pub fn command_might_be_dangerous(command: &[String]) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns whether already-tokenized PowerShell words should be treated as
|
||||
/// dangerous by the Windows unmatched-command heuristics.
|
||||
pub fn is_dangerous_powershell_words(command: &[String]) -> bool {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
windows_dangerous_commands::is_dangerous_powershell_words(command)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let _ = command;
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
fn is_git_global_option_with_value(arg: &str) -> bool {
|
||||
matches!(
|
||||
arg,
|
||||
@@ -190,4 +205,15 @@ mod tests {
|
||||
assert!(git_global_option_requires_prompt("-C"));
|
||||
assert!(git_global_option_requires_prompt("-C/path/to/repo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn direct_powershell_words_reuse_windows_dangerous_detection() {
|
||||
let command = vec_str(&["Remove-Item", "test", "-Force"]);
|
||||
|
||||
if cfg!(windows) {
|
||||
assert!(is_dangerous_powershell_words(&command));
|
||||
} else {
|
||||
assert!(!is_dangerous_powershell_words(&command));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,8 @@ use crate::command_safety::is_dangerous_command::executable_name_lookup_key;
|
||||
use crate::command_safety::is_dangerous_command::find_git_subcommand;
|
||||
use crate::command_safety::is_dangerous_command::git_global_option_requires_prompt;
|
||||
use crate::command_safety::windows_safe_commands::is_safe_command_windows;
|
||||
#[cfg(windows)]
|
||||
use crate::command_safety::windows_safe_commands::is_safe_powershell_words as is_safe_powershell_words_windows;
|
||||
|
||||
pub fn is_known_safe_command(command: &[String]) -> bool {
|
||||
let command: Vec<String> = command
|
||||
@@ -44,6 +46,21 @@ pub fn is_known_safe_command(command: &[String]) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns whether already-tokenized PowerShell words are read-only enough to
|
||||
/// be auto-approved by the Windows safelist.
|
||||
pub fn is_safe_powershell_words(command: &[String]) -> bool {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
is_safe_powershell_words_windows(command)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let _ = command;
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
fn is_safe_to_call_with_exec(command: &[String]) -> bool {
|
||||
let Some(cmd0) = command.first().map(String::as_str) else {
|
||||
return false;
|
||||
@@ -638,4 +655,15 @@ mod tests {
|
||||
"> redirection should be rejected"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn direct_powershell_words_use_windows_safelist() {
|
||||
let command = vec_str(&["Get-Content", "Cargo.toml"]);
|
||||
|
||||
if cfg!(windows) {
|
||||
assert!(is_safe_powershell_words(&command));
|
||||
} else {
|
||||
assert!(!is_safe_powershell_words(&command));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,3 +3,4 @@ mod powershell_parser;
|
||||
pub mod is_dangerous_command;
|
||||
pub mod is_safe_command;
|
||||
pub(crate) mod windows_safe_commands;
|
||||
pub(crate) use powershell_parser::try_parse_powershell_ast_commands;
|
||||
|
||||
@@ -34,6 +34,16 @@ pub(super) fn parse_with_powershell_ast(executable: &str, script: &str) -> Power
|
||||
parse_with_cached_process(&mut parser_processes, executable, script)
|
||||
}
|
||||
|
||||
pub(crate) fn try_parse_powershell_ast_commands(
|
||||
executable: &str,
|
||||
script: &str,
|
||||
) -> Option<Vec<Vec<String>>> {
|
||||
match parse_with_powershell_ast(executable, script) {
|
||||
PowershellParseOutcome::Commands(commands) => Some(commands),
|
||||
PowershellParseOutcome::Unsupported | PowershellParseOutcome::Failed => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(super) enum PowershellParseOutcome {
|
||||
Commands(Vec<Vec<String>>),
|
||||
|
||||
@@ -34,12 +34,15 @@ fn is_dangerous_powershell(command: &[String]) -> bool {
|
||||
return false;
|
||||
};
|
||||
|
||||
let tokens_lc: Vec<String> = parsed
|
||||
.tokens
|
||||
is_dangerous_powershell_words(&parsed.tokens)
|
||||
}
|
||||
|
||||
pub(crate) fn is_dangerous_powershell_words(words: &[String]) -> bool {
|
||||
let tokens_lc: Vec<String> = words
|
||||
.iter()
|
||||
.map(|t| t.trim_matches('\'').trim_matches('"').to_ascii_lowercase())
|
||||
.collect();
|
||||
let has_url = args_have_url(&parsed.tokens);
|
||||
let has_url = args_have_url(words);
|
||||
|
||||
if has_url
|
||||
&& tokens_lc.iter().any(|t| {
|
||||
@@ -83,11 +86,7 @@ fn is_dangerous_powershell(command: &[String]) -> bool {
|
||||
}
|
||||
|
||||
// Check for force delete operations (e.g., Remove-Item -Force)
|
||||
if has_force_delete_cmdlet(&tokens_lc) {
|
||||
return true;
|
||||
}
|
||||
|
||||
false
|
||||
has_force_delete_cmdlet(&tokens_lc)
|
||||
}
|
||||
|
||||
fn is_dangerous_cmd(command: &[String]) -> bool {
|
||||
|
||||
@@ -9,7 +9,7 @@ pub fn is_safe_command_windows(command: &[String]) -> bool {
|
||||
if let Some(commands) = try_parse_powershell_command_sequence(command) {
|
||||
commands
|
||||
.iter()
|
||||
.all(|cmd| is_safe_powershell_command(cmd.as_slice()))
|
||||
.all(|cmd| is_safe_powershell_words(cmd.as_slice()))
|
||||
} else {
|
||||
// Only PowerShell invocations are allowed on Windows for now; anything else is unsafe.
|
||||
false
|
||||
@@ -142,7 +142,7 @@ fn quote_argument(arg: &str) -> String {
|
||||
|
||||
/// 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 {
|
||||
pub(crate) fn is_safe_powershell_words(words: &[String]) -> bool {
|
||||
if words.is_empty() {
|
||||
// Examples rejected here: "pwsh -Command ''" and "pwsh -Command \"\"".
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user