From e8fef7981da26eddee7768b544fdb85d6aea2b56 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Wed, 13 May 2026 20:09:56 -0700 Subject: [PATCH] Refactor arbitrary command exec policy guard --- codex-rs/core/src/exec_policy.rs | 98 ++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 37 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 9f12ad4a1e..58c018c983 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -310,45 +310,15 @@ impl ExecPolicyManager { let match_options = MatchOptions { resolve_host_executables: true, }; - let mut evaluation = exec_policy.check_multiple_with_options( - commands.iter(), + let evaluation = apply_arbitrary_command_guard( + exec_policy.check_multiple_with_options( + commands.iter(), + &exec_policy_fallback, + &match_options, + ), + &commands, &exec_policy_fallback, - &match_options, ); - if evaluation.decision == Decision::Allow { - let mut arbitrary_command_matches = commands - .iter() - .filter_map(|command| { - if !command_can_execute_arbitrary_command(command) - || arbitrary_command_execution_is_explicitly_allowed( - command, - &evaluation.matched_rules, - ) - { - return None; - } - - let decision = exec_policy_fallback(command); - (decision != Decision::Allow).then(|| RuleMatch::HeuristicsRuleMatch { - command: command.clone(), - decision, - }) - }) - .collect::>(); - if !arbitrary_command_matches.is_empty() { - evaluation.decision = if arbitrary_command_matches - .iter() - .any(|rule_match| rule_match.decision() == Decision::Forbidden) - { - Decision::Forbidden - } else { - Decision::Prompt - }; - evaluation - .matched_rules - .append(&mut arbitrary_command_matches); - } - } let requested_amendment = if auto_amendment_allowed { derive_requested_execpolicy_amendment_from_prefix_rule( @@ -528,6 +498,60 @@ fn arbitrary_command_execution_is_explicitly_allowed( }) } +fn apply_arbitrary_command_guard( + evaluation: Evaluation, + commands: &[Vec], + exec_policy_fallback: &F, +) -> Evaluation +where + F: Fn(&[String]) -> Decision, +{ + if evaluation.decision != Decision::Allow { + return evaluation; + } + + let arbitrary_command_matches = commands + .iter() + .filter_map(|command| { + if !command_can_execute_arbitrary_command(command) + || arbitrary_command_execution_is_explicitly_allowed( + command, + &evaluation.matched_rules, + ) + { + return None; + } + + let decision = exec_policy_fallback(command); + (decision != Decision::Allow).then(|| RuleMatch::HeuristicsRuleMatch { + command: command.clone(), + decision, + }) + }) + .collect::>(); + + if arbitrary_command_matches.is_empty() { + return evaluation; + } + + let decision = if arbitrary_command_matches + .iter() + .any(|rule_match| rule_match.decision() == Decision::Forbidden) + { + Decision::Forbidden + } else { + Decision::Prompt + }; + let Evaluation { + mut matched_rules, .. + } = evaluation; + matched_rules.extend(arbitrary_command_matches); + Evaluation { + decision, + matched_rules, + } +} + impl Default for ExecPolicyManager { fn default() -> Self { Self::new(Arc::new(Policy::empty()))