diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 36a4b3c8da..73439df097 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -250,13 +250,17 @@ impl ExecPolicyManager { prefix_rule, } = req; let exec_policy = self.current(); - let (commands, used_complex_parsing) = commands_for_exec_policy(original_command); #[cfg(windows)] let powershell_commands = try_parse_powershell_command_sequence( original_command, PowershellCommandSequenceParseMode::ExecPolicy, ) .filter(|commands| !commands.is_empty()); + let (commands, used_complex_parsing) = if let Some(commands) = powershell_commands.clone() { + (commands, false) + } else { + commands_for_exec_policy(original_command) + }; // Keep heredoc prefix parsing for rule evaluation so existing // allow/prompt/forbidden rules still apply, but avoid auto-derived // amendments when only the heredoc fallback parser matched. @@ -291,14 +295,30 @@ impl ExecPolicyManager { #[cfg(windows)] if powershell_commands.is_some() { - for rule_match in &mut evaluation.matched_rules { - if let RuleMatch::HeuristicsRuleMatch { - command: matched_command, - .. - } = rule_match - { - *matched_command = original_command.to_vec(); + let outer_policy_matches = exec_policy.matches_for_command_with_options( + original_command, + /*heuristics_fallback*/ None, + &match_options, + ); + if outer_policy_matches.is_empty() { + for rule_match in &mut evaluation.matched_rules { + if let RuleMatch::HeuristicsRuleMatch { + command: matched_command, + .. + } = rule_match + { + *matched_command = original_command.to_vec(); + } } + } else { + evaluation.matched_rules.retain(is_policy_match); + evaluation.matched_rules.extend(outer_policy_matches); + evaluation.decision = evaluation + .matched_rules + .iter() + .map(RuleMatch::decision) + .max() + .expect("invariant failed: matched_rules must be non-empty"); } } @@ -338,19 +358,10 @@ impl ExecPolicyManager { } } Decision::Allow => ExecApprovalRequirement::Skip { - // Bypass sandbox only when every parsed command segment is - // explicitly allowed by execpolicy. - bypass_sandbox: commands.iter().all(|command| { - exec_policy - .matches_for_command_with_options( - command, - /*heuristics_fallback*/ None, - &match_options, - ) - .iter() - .any(|rule_match| { - is_policy_match(rule_match) && rule_match.decision() == Decision::Allow - }) + // Bypass sandbox only when the allow decision came entirely + // from explicit execpolicy allow rules. + bypass_sandbox: evaluation.matched_rules.iter().all(|rule_match| { + is_policy_match(rule_match) && rule_match.decision() == Decision::Allow }), proposed_execpolicy_amendment: if auto_amendment_allowed { try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules) @@ -736,15 +747,6 @@ fn commands_for_exec_policy(command: &[String]) -> (Vec>, bool) { return (commands, false); } - #[cfg(windows)] - if let Some(commands) = try_parse_powershell_command_sequence( - command, - PowershellCommandSequenceParseMode::ExecPolicy, - ) && !commands.is_empty() - { - return (commands, false); - } - if let Some(single_command) = parse_shell_lc_single_command_prefix(command) { return (vec![single_command], true); } diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 4f808de892..990e729b4a 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -698,6 +698,18 @@ async fn evaluates_powershell_wrapped_inner_commands_against_prefix_rules() { ]), vec_str(&["Remove-Item"]), ), + ( + r#"prefix_rule(pattern=["git", "push"], decision="forbidden")"#.to_string(), + vec_str(&[ + "powershell.exe", + "-Version", + "5.1", + "-NoExit", + "-Command", + "git push origin main", + ]), + vec_str(&["git", "push"]), + ), ]; for (policy_src, command, matched_prefix) in cases { @@ -724,6 +736,33 @@ async fn evaluates_powershell_wrapped_inner_commands_against_prefix_rules() { } } +#[cfg(windows)] +#[tokio::test] +async fn powershell_wrapper_rules_still_apply_when_inner_commands_are_parsed() { + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: Some( + concat!( + r#"prefix_rule(pattern=["powershell.exe"], decision="forbidden")"#, + "\n", + r#"prefix_rule(pattern=["Get-Content"], decision="allow")"#, + ) + .to_string(), + ), + command: vec_str(&["powershell.exe", "-Command", "Get-Content Cargo.toml"]), + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_read_only_policy(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }, + ExecApprovalRequirement::Forbidden { + reason: "`powershell.exe -Command 'Get-Content Cargo.toml'` rejected: policy forbids commands starting with `powershell.exe`".to_string(), + }, + ) + .await; +} + #[cfg(windows)] #[tokio::test] async fn unmatched_powershell_wrappers_keep_outer_command_heuristics() { diff --git a/codex-rs/shell-command/src/powershell.rs b/codex-rs/shell-command/src/powershell.rs index 7179dac6c1..acbddb8279 100644 --- a/codex-rs/shell-command/src/powershell.rs +++ b/codex-rs/shell-command/src/powershell.rs @@ -91,7 +91,8 @@ pub fn try_parse_powershell_command_sequence( ) -> Option>> { let (executable, args) = command.split_first()?; if is_powershell_executable(executable) { - parse_powershell_invocation(executable, args, mode) + let parser_executable = trusted_powershell_parser_executable(executable)?; + parse_powershell_invocation(&parser_executable, args, mode) } else { None } @@ -149,8 +150,13 @@ fn parse_powershell_invocation( { return None; } - _ if lower.starts_with('-') => { - return None; + _ if looks_like_powershell_flag(&lower) => { + if mode == PowershellCommandSequenceParseMode::SafeCommand { + return None; + } + + idx += powershell_wrapper_flag_length(args, idx); + continue; } _ => { if mode == PowershellCommandSequenceParseMode::ExecPolicy { @@ -173,6 +179,22 @@ pub(crate) fn parse_powershell_script_to_commands( try_parse_powershell_ast_commands(executable, script) } +fn trusted_powershell_parser_executable(exe: &str) -> Option { + let executable_name = std::path::Path::new(exe) + .file_name() + .and_then(|osstr| osstr.to_str()) + .unwrap_or(exe) + .to_ascii_lowercase(); + + let parser_executable = match executable_name.as_str() { + "powershell" | "powershell.exe" => try_find_powershell_executable_blocking()?, + "pwsh" | "pwsh.exe" => try_find_pwsh_executable_blocking()?, + _ => return None, + }; + + Some(parser_executable.as_path().to_string_lossy().into_owned()) +} + pub(crate) fn is_powershell_executable(exe: &str) -> bool { let executable_name = std::path::Path::new(exe) .file_name() @@ -209,6 +231,10 @@ fn quote_argument(arg: &str) -> String { format!("'{}'", arg.replace('\'', "''")) } +fn looks_like_powershell_flag(lower: &str) -> bool { + lower.starts_with('-') || lower.starts_with('/') +} + fn is_powershell_no_arg_parse_flag(lower: &str) -> bool { POWERSHELL_NO_ARG_PARSE_FLAGS.contains(&lower) } @@ -251,6 +277,18 @@ fn split_flag_inline_value(lower: &str) -> Option<(&str, &str)> { lower.split_once(':') } +fn powershell_wrapper_flag_length(args: &[String], idx: usize) -> usize { + let Some(next_arg) = args.get(idx + 1) else { + return 1; + }; + + if looks_like_powershell_flag(&next_arg.to_ascii_lowercase()) { + 1 + } else { + 2 + } +} + /// This function attempts to find a powershell.exe executable on the system. pub fn try_find_powershell_executable_blocking() -> Option { try_find_powershellish_executable_in_path(&["powershell.exe"]) @@ -321,7 +359,10 @@ fn is_powershellish_executable_available(powershell_or_pwsh_exe: &std::path::Pat #[cfg(test)] mod tests { + use super::PowershellCommandSequenceParseMode; use super::extract_powershell_command; + use super::try_parse_powershell_command_sequence; + use pretty_assertions::assert_eq; #[test] fn extracts_basic_powershell_command() { @@ -369,4 +410,49 @@ mod tests { let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); assert_eq!(script, "Get-ChildItem | Select-String foo"); } + + #[cfg(windows)] + #[test] + fn exec_policy_parsing_ignores_ordinary_wrapper_flags() { + let command = vec![ + "powershell.exe".to_string(), + "-Version".to_string(), + "5.1".to_string(), + "-NoExit".to_string(), + "-Command".to_string(), + "Get-Content 'foo bar'".to_string(), + ]; + + assert_eq!( + try_parse_powershell_command_sequence( + &command, + PowershellCommandSequenceParseMode::ExecPolicy, + ), + Some(vec![ + vec!["Get-Content".to_string(), "foo bar".to_string(),] + ]), + ); + } + + #[cfg(windows)] + #[test] + fn uses_trusted_system_powershell_for_ast_parsing() { + let command = vec![ + r"C:\repo\powershell.exe".to_string(), + "-NoExit".to_string(), + "-Command".to_string(), + "Get-Content Cargo.toml".to_string(), + ]; + + assert_eq!( + try_parse_powershell_command_sequence( + &command, + PowershellCommandSequenceParseMode::ExecPolicy, + ), + Some(vec![vec![ + "Get-Content".to_string(), + "Cargo.toml".to_string(), + ]]), + ); + } }