diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index c05a459049..7cfed72f8a 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -50,6 +50,7 @@ pub(crate) async fn apply_patch( action, auto_approved: !user_explicitly_approved, exec_approval_requirement: ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, @@ -62,6 +63,7 @@ pub(crate) async fn apply_patch( action, auto_approved: false, exec_approval_requirement: ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, }, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 54ad8058d0..5f32f96682 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -267,6 +267,7 @@ impl ExecPolicyManager { &exec_policy_fallback, &match_options, ); + let execpolicy_matched = evaluation.matched_rules.iter().any(is_policy_match); let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule( prefix_rule.as_ref(), @@ -279,6 +280,7 @@ impl ExecPolicyManager { match evaluation.decision { Decision::Forbidden => ExecApprovalRequirement::Forbidden { + execpolicy_matched, reason: derive_forbidden_reason(command, &evaluation), }, Decision::Prompt => { @@ -287,9 +289,11 @@ impl ExecPolicyManager { }); match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) { Some(reason) => ExecApprovalRequirement::Forbidden { + execpolicy_matched, reason: reason.to_string(), }, None => ExecApprovalRequirement::NeedsApproval { + execpolicy_matched, reason: derive_prompt_reason(command, &evaluation), proposed_execpolicy_amendment: requested_amendment.or_else(|| { if auto_amendment_allowed { @@ -304,6 +308,7 @@ impl ExecPolicyManager { } } Decision::Allow => ExecApprovalRequirement::Skip { + execpolicy_matched, // Bypass sandbox only when every parsed command segment is // explicitly allowed by execpolicy. bypass_sandbox: commands.iter().all(|command| { diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index fe4560a781..a6d5afcf60 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -636,6 +636,7 @@ async fn evaluates_bash_lc_inner_commands() { prefix_rule: None, }, ExecApprovalRequirement::Forbidden { + execpolicy_matched: true, reason: "`bash -lc 'rm -rf /some/important/folder'` rejected: policy forbids commands starting with `rm`".to_string(), }, ) @@ -718,6 +719,7 @@ async fn evaluates_heredoc_script_against_prefix_rules() { prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: true, bypass_sandbox: true, proposed_execpolicy_amendment: None, }, @@ -742,6 +744,7 @@ async fn omits_auto_amendment_for_heredoc_fallback_prompts() { prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, }, @@ -770,6 +773,7 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_mat ]), }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, }, @@ -803,6 +807,7 @@ prefix_rule( prefix_rule: None, }, ExecApprovalRequirement::Forbidden { + execpolicy_matched: true, reason: "`rm -rf /some/important/folder` rejected: destructive command".to_string(), }, ) @@ -822,6 +827,7 @@ async fn exec_approval_requirement_prefers_execpolicy_match() { prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: true, reason: Some("`rm` requires approval by policy".to_string()), proposed_execpolicy_amendment: None, }, @@ -850,6 +856,7 @@ prefix_rule(pattern=["git"], decision="allow") prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: true, bypass_sandbox: true, proposed_execpolicy_amendment: None, }, @@ -884,6 +891,7 @@ prefix_rule(pattern=["git"], decision="prompt") prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ disallowed_git_path, @@ -911,6 +919,7 @@ async fn requested_prefix_rule_can_approve_absolute_path_commands() { prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "cargo".to_string(), @@ -934,6 +943,7 @@ async fn exec_approval_requirement_respects_approval_policy() { prefix_rule: None, }, ExecApprovalRequirement::Forbidden { + execpolicy_matched: true, reason: PROMPT_CONFLICT_REASON.to_string(), }, ) @@ -998,6 +1008,7 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "touch".to_string(), @@ -1028,6 +1039,7 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra prefix_rule: None, }, ExecApprovalRequirement::Forbidden { + execpolicy_matched: false, reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(), }, ) @@ -1105,6 +1117,7 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled( assert_eq!( requirement, ExecApprovalRequirement::Forbidden { + execpolicy_matched: true, reason: REJECT_RULES_APPROVAL_REASON.to_string(), } ); @@ -1129,6 +1142,7 @@ async fn exec_approval_requirement_falls_back_to_heuristics() { assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)) } @@ -1154,6 +1168,7 @@ async fn empty_bash_lc_script_falls_back_to_original_command() { assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), } @@ -1183,6 +1198,7 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() { assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), } @@ -1212,6 +1228,7 @@ async fn request_rule_uses_prefix_rule() { assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "cargo".to_string(), @@ -1244,6 +1261,7 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands( assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "rm".to_string(), @@ -1280,6 +1298,7 @@ async fn heuristics_apply_when_other_commands_match_policy() { }) .await, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: true, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "orange".to_string() @@ -1354,6 +1373,7 @@ async fn proposed_execpolicy_amendment_is_present_for_single_command_without_pol prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), }, @@ -1374,6 +1394,7 @@ async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() { prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: true, reason: Some("`rm` requires approval by policy".to_string()), proposed_execpolicy_amendment: None, }, @@ -1398,6 +1419,7 @@ async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() { prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "cargo".to_string(), @@ -1428,6 +1450,7 @@ async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scri prefix_rule: None, }, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: true, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "apple".to_string(), @@ -1452,6 +1475,7 @@ async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() { prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), }, @@ -1472,6 +1496,7 @@ async fn proposed_execpolicy_amendment_is_suppressed_when_policy_matches_allow() prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: true, bypass_sandbox: true, proposed_execpolicy_amendment: None, }, @@ -1503,6 +1528,7 @@ prefix_rule(pattern=["cat"], decision="allow") prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: true, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, @@ -1535,6 +1561,7 @@ prefix_rule(pattern=["bash"], decision="allow") prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: true, bypass_sandbox: true, proposed_execpolicy_amendment: None, }, @@ -1740,6 +1767,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { that no sandbox is present, so anything that is not "provably safe" should require approval."#, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: expected_amendment.clone(), }, @@ -1748,6 +1776,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { ( "On non-Windows, rely on the read-only sandbox to prevent harm.", ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: expected_amendment.clone(), }, @@ -1772,6 +1801,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { let dangerous_command = vec_str(&["rm", "-rf", "/important/data"]); assert_eq!( ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[ "rm", @@ -1797,6 +1827,7 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() { // AskForApproval::Never. assert_eq!( ExecApprovalRequirement::Forbidden { + execpolicy_matched: false, reason: "`rm -rf /important/data` rejected: blocked by policy".to_string(), }, policy @@ -1830,6 +1861,7 @@ async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() { prefix_rule: None, }, ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: Some(ExecPolicyAmendment { command: vec_str(&["rm", "-rf", "/tmp/nonexistent"]), @@ -1855,6 +1887,7 @@ async fn dangerous_command_forbidden_in_external_sandbox_when_policy_matches() { prefix_rule: None, }, ExecApprovalRequirement::Forbidden { + execpolicy_matched: true, reason: "approval required by policy, but AskForApproval is set to Never".to_string(), }, ) diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index c1b960bc8d..4d9013335e 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -124,6 +124,7 @@ impl ToolOrchestrator { let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| { default_exec_approval_requirement(approval_policy, &turn_ctx.file_system_sandbox_policy) }); + let execpolicy_matched = requirement.execpolicy_matched(); match requirement { ExecApprovalRequirement::Skip { .. } => { otel.tool_decision( @@ -131,9 +132,10 @@ impl ToolOrchestrator { otel_ci, &ReviewDecision::Approved, ToolDecisionSource::Config, + Some(execpolicy_matched), ); } - ExecApprovalRequirement::Forbidden { reason } => { + ExecApprovalRequirement::Forbidden { reason, .. } => { return Err(ToolError::Rejected(reason)); } ExecApprovalRequirement::NeedsApproval { reason, .. } => { @@ -154,6 +156,7 @@ impl ToolOrchestrator { tool_ctx, use_guardian, &otel, + Some(execpolicy_matched), ) .await?; @@ -387,6 +390,7 @@ impl ToolOrchestrator { tool_ctx: &ToolCtx, use_guardian: bool, otel: &codex_otel::SessionTelemetry, + execpolicy_matched: Option, ) -> Result where T: ToolRuntime, @@ -407,6 +411,7 @@ impl ToolOrchestrator { &tool_ctx.call_id, &decision, ToolDecisionSource::Config, + execpolicy_matched, ); return Ok(decision); } @@ -417,6 +422,7 @@ impl ToolOrchestrator { &tool_ctx.call_id, &decision, ToolDecisionSource::Config, + execpolicy_matched, ); return Err(ToolError::Rejected(message)); } @@ -435,6 +441,7 @@ impl ToolOrchestrator { &tool_ctx.call_id, &decision, otel_source, + execpolicy_matched, ); Ok(decision) } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 7f3641c03c..21e38745aa 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -58,6 +58,7 @@ fn guardian_review_request_includes_patch_context() { }, )]), exec_approval_requirement: ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, }, @@ -95,6 +96,7 @@ fn file_system_sandbox_context_uses_active_attempt() { file_paths: vec![path.clone()], changes: HashMap::new(), exec_approval_requirement: ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, @@ -151,6 +153,7 @@ fn file_system_sandbox_context_omits_legacy_equivalent_policy() { file_paths: vec![path.clone()], changes: HashMap::new(), exec_approval_requirement: ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, @@ -192,6 +195,7 @@ fn no_sandbox_attempt_has_no_file_system_context() { file_paths: vec![path.clone()], changes: HashMap::new(), exec_approval_requirement: ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 87bef4617c..a8f0d188a0 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -143,6 +143,8 @@ pub(crate) struct PermissionRequestPayload { pub(crate) enum ExecApprovalRequirement { /// No approval required for this tool call. Skip { + /// Whether at least one concrete execpolicy rule matched this command. + execpolicy_matched: bool, /// The first attempt should skip sandboxing (e.g., when explicitly /// greenlit by policy). bypass_sandbox: bool, @@ -152,13 +154,19 @@ pub(crate) enum ExecApprovalRequirement { }, /// Approval required for this tool call. NeedsApproval { + /// Whether at least one concrete execpolicy rule matched this command. + execpolicy_matched: bool, reason: Option, /// Proposed execpolicy amendment to skip future approvals for similar commands /// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined. proposed_execpolicy_amendment: Option, }, /// Execution forbidden for this tool call. - Forbidden { reason: String }, + Forbidden { + /// Whether at least one concrete execpolicy rule matched this command. + execpolicy_matched: bool, + reason: String, + }, } impl ExecApprovalRequirement { @@ -175,6 +183,20 @@ impl ExecApprovalRequirement { _ => None, } } + + pub fn execpolicy_matched(&self) -> bool { + match self { + Self::Skip { + execpolicy_matched, .. + } + | Self::NeedsApproval { + execpolicy_matched, .. + } + | Self::Forbidden { + execpolicy_matched, .. + } => *execpolicy_matched, + } + } } /// - Never, OnFailure: do not ask @@ -205,15 +227,18 @@ pub(crate) fn default_exec_approval_requirement( ) { ExecApprovalRequirement::Forbidden { + execpolicy_matched: false, reason: "approval policy disallowed sandbox approval prompt".to_string(), } } else if needs_approval { ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, } } else { ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, } diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 4a4dac3e81..caa99c7917 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -15,6 +15,7 @@ fn external_sandbox_skips_exec_approval_on_request() { &FileSystemSandboxPolicy::from(&sandbox_policy), ), ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, } @@ -30,6 +31,7 @@ fn restricted_sandbox_requires_exec_approval_on_request() { &FileSystemSandboxPolicy::from(&sandbox_policy) ), ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, } @@ -53,6 +55,7 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl assert_eq!( requirement, ExecApprovalRequirement::Forbidden { + execpolicy_matched: false, reason: "approval policy disallowed sandbox approval prompt".to_string(), } ); @@ -75,6 +78,7 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a assert_eq!( requirement, ExecApprovalRequirement::NeedsApproval { + execpolicy_matched: false, reason: None, proposed_execpolicy_amendment: None, } @@ -87,6 +91,7 @@ fn additional_permissions_allow_bypass_sandbox_first_attempt_when_execpolicy_ski sandbox_override_for_first_attempt( SandboxPermissions::WithAdditionalPermissions, &ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: true, proposed_execpolicy_amendment: None, }, @@ -101,6 +106,7 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() { sandbox_override_for_first_attempt( SandboxPermissions::RequireEscalated, &ExecApprovalRequirement::Skip { + execpolicy_matched: false, bypass_sandbox: false, proposed_execpolicy_amendment: None, }, diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index f93945e78f..4d9f5933d2 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -1040,10 +1040,12 @@ fn tool_decision_assertion<'a>( call_id: &'a str, expected_decision: &'a str, expected_source: &'a str, + expected_execpolicy_matched: &'a str, ) -> impl Fn(&[&str]) -> Result<(), String> + 'a { let call_id = call_id.to_string(); let expected_decision = expected_decision.to_string(); let expected_source = expected_source.to_string(); + let expected_execpolicy_matched = expected_execpolicy_matched.to_string(); move |lines: &[&str]| { let line = lines @@ -1063,6 +1065,11 @@ fn tool_decision_assertion<'a>( if !lower.contains(&format!("source={expected_source}")) { return Err(format!("unexpected source for {expected_source}")); } + if !lower.contains(&format!("execpolicy_matched={expected_execpolicy_matched}")) { + return Err(format!( + "unexpected execpolicy_matched for {call_id}: expected {expected_execpolicy_matched}" + )); + } Ok(()) } @@ -1122,6 +1129,7 @@ async fn handle_container_exec_autoapprove_from_config_records_tool_decision() { "auto_config_call", "approved", "config", + "false", )); } @@ -1189,6 +1197,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() { "user_approved_call", "approved", "user", + "false", )); } @@ -1256,6 +1265,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() "user_approved_session_call", "approvedforsession", "user", + "false", )); } @@ -1323,6 +1333,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { "sandbox_retry_call", "approved", "user", + "false", )); } @@ -1390,6 +1401,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() { "user_denied_call", "denied", "user", + "false", )); } @@ -1457,6 +1469,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() "sandbox_session_call", "approvedforsession", "user", + "false", )); } @@ -1525,5 +1538,6 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { "sandbox_deny_call", "denied", "user", + "false", )); } diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index 14ae1cb3e5..e7c351e003 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -867,6 +867,7 @@ impl SessionTelemetry { call_id: &str, decision: &ReviewDecision, source: ToolDecisionSource, + execpolicy_matched: Option, ) { log_event!( self, @@ -875,6 +876,9 @@ impl SessionTelemetry { call_id = %call_id, decision = %decision.clone().to_string().to_lowercase(), source = %source.to_string(), + execpolicy_matched = %execpolicy_matched + .map(|matched| matched.to_string()) + .unwrap_or_default(), ); }