diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 3574ee77fd..a44ffbe3ab 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -649,9 +649,6 @@ pub(crate) fn render_decision_for_unmatched_command( codex_shell_command::is_safe_command::is_safe_powershell_words(command) } }; - if is_known_safe && !used_complex_parsing { - return Decision::Allow; - } // On Windows, ReadOnly sandbox is not a real sandbox, so special-case it // here. @@ -662,6 +659,14 @@ pub(crate) fn render_decision_for_unmatched_command( sandbox_cwd, ); + if is_known_safe + && !used_complex_parsing + && (approval_policy == AskForApproval::UnlessTrusted + || environment_lacks_sandbox_protections) + { + return Decision::Allow; + } + // If the command is flagged as dangerous or we have no sandbox protection, // we should never allow it to run without approval. // diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index 96c32c4491..f15c0280b0 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -1144,6 +1144,29 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() { ); } +#[test] +fn known_safe_on_request_still_prompts_for_restricted_sandbox_escalation() { + let command = vec!["echo".to_string(), "hello".to_string()]; + + assert_eq!( + Decision::Prompt, + render_decision_for_unmatched_command( + &command, + UnmatchedCommandContext { + approval_policy: AskForApproval::OnRequest, + permission_profile: &permission_profile_from_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + ), + file_system_sandbox_policy: &workspace_write_file_system_sandbox_policy(), + sandbox_cwd: Path::new("/tmp"), + sandbox_permissions: SandboxPermissions::RequireEscalated, + used_complex_parsing: false, + command_origin: ExecPolicyCommandOrigin::Generic, + }, + ) + ); +} + #[test] fn managed_cwd_write_profile_is_not_read_only() { let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ @@ -1230,6 +1253,55 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und .await; } +#[tokio::test] +async fn exec_approval_requirement_prompts_for_known_safe_escalation_under_on_request() { + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: None, + command: vec!["echo".to_string(), "hello".to_string()], + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "echo".to_string(), + "hello".to_string(), + ])), + }, + ) + .await; +} + +#[tokio::test] +async fn exec_approval_requirement_rejects_known_safe_escalation_when_granular_sandbox_is_disabled() +{ + assert_exec_approval_requirement_for_command( + ExecApprovalRequirementScenario { + policy_src: None, + command: vec!["echo".to_string(), "hello".to_string()], + approval_policy: AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval: false, + rules: true, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }), + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }, + ExecApprovalRequirement::Forbidden { + reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(), + }, + ) + .await; +} + #[tokio::test] async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_granular_sandbox_is_disabled() { diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 10bbb852eb..6db6495933 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -15,6 +15,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::ExecPolicyAmendment; +use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; @@ -965,6 +966,46 @@ fn scenarios() -> Vec { output_contains: "rejected by user", }, }, + ScenarioSpec { + name: "known_safe_escalation_on_request_requires_approval", + approval_policy: OnRequest, + sandbox_policy: workspace_write(false), + action: ActionKind::RunCommand { + command: "echo known-safe-escalation", + }, + sandbox_permissions: SandboxPermissions::RequireEscalated, + features: vec![], + model_override: Some("gpt-5.2"), + outcome: Outcome::ExecApprovalWithAmendment { + decision: ReviewDecision::Denied, + expected_reason: None, + expected_execpolicy_amendment: Some(&["echo", "known-safe-escalation"]), + }, + expectation: Expectation::CommandFailure { + output_contains: "rejected by user", + }, + }, + ScenarioSpec { + name: "known_safe_escalation_granular_sandbox_disabled_rejects", + approval_policy: Granular(GranularApprovalConfig { + sandbox_approval: false, + rules: true, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }), + sandbox_policy: workspace_write(false), + action: ActionKind::RunCommand { + command: "echo known-safe-escalation-granular-disabled", + }, + sandbox_permissions: SandboxPermissions::RequireEscalated, + features: vec![], + model_override: Some("gpt-5.2"), + outcome: Outcome::Auto, + expectation: Expectation::CommandFailure { + output_contains: "you should not ask for escalated permissions", + }, + }, ScenarioSpec { name: "cat_heredoc_file_redirect_prefix_rule_requires_escalation_approval", approval_policy: OnRequest,