diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 2b19acbdc..6a92f6945 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -104,7 +104,7 @@ fn is_policy_match(rule_match: &RuleMatch) -> bool { /// `prompt_is_rule` distinguishes policy-rule prompts from sandbox/escalation /// prompts so `Reject.rules` and `Reject.sandbox_approval` are honored /// independently. When both are present, policy-rule prompts take precedence. -fn prompt_is_rejected_by_policy( +pub(crate) fn prompt_is_rejected_by_policy( approval_policy: AskForApproval, prompt_is_rule: bool, ) -> Option<&'static str> { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 99d7c1476..c8b16a1ad 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -5,6 +5,7 @@ use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; use crate::exec::is_likely_sandbox_denied; +use crate::exec_policy::prompt_is_rejected_by_policy; use crate::features::Feature; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; @@ -28,7 +29,6 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::NetworkPolicyRuleAction; -use codex_protocol::protocol::RejectConfig; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_shell_command::bash::parse_shell_lc_plain_commands; @@ -451,11 +451,12 @@ impl CoreShellActionProvider { EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } Decision::Prompt => { - if matches!( + if prompt_is_rejected_by_policy( self.approval_policy, - AskForApproval::Never - | AskForApproval::Reject(RejectConfig { rules: true, .. }) - ) { + matches!(decision_source, DecisionSource::PrefixRule), + ) + .is_some() + { EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } else { match self diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index ec25d838a..27341ec24 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -8,6 +8,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::Op; +use codex_protocol::protocol::RejectConfig; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; @@ -265,6 +266,172 @@ permissions: Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_false_still_prompts() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("zsh-fork reject false skill prompt test")? else { + return Ok(()); + }; + + let approval_policy = AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: true, + mcp_elicitations: false, + }); + let server = start_mock_server().await; + let tool_call_id = "zsh-fork-skill-reject-false"; + let test = build_zsh_fork_test( + &server, + runtime, + approval_policy, + SandboxPolicy::new_workspace_write_policy(), + |home| { + write_skill_with_shell_script(home, "mbolin-test-skill", "hello-mbolin.sh").unwrap(); + write_skill_metadata( + home, + "mbolin-test-skill", + r#" +permissions: + file_system: + write: + - "./output" +"#, + ) + .unwrap(); + }, + ) + .await?; + + let (script_path_str, command) = skill_script_command(&test, "hello-mbolin.sh")?; + let arguments = shell_command_arguments(&command)?; + let mocks = + mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command") + .await; + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + approval_policy, + SandboxPolicy::new_workspace_write_policy(), + ) + .await?; + + let maybe_approval = wait_for_exec_approval_request(&test).await; + let approval = match maybe_approval { + Some(approval) => approval, + None => { + let call_output = mocks + .completion + .single_request() + .function_call_output(tool_call_id); + panic!( + "expected exec approval request before completion; function_call_output={call_output:?}" + ); + } + }; + assert_eq!(approval.call_id, tool_call_id); + assert_eq!(approval.command, vec![script_path_str]); + + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Denied, + }) + .await?; + + wait_for_turn_complete(&test).await; + + let call_output = mocks + .completion + .single_request() + .function_call_output(tool_call_id); + let output = call_output["output"].as_str().unwrap_or_default(); + assert!( + output.contains("Execution denied: User denied execution"), + "expected rejection marker in function_call_output: {output:?}" + ); + + Ok(()) +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_true_skips_prompt() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("zsh-fork reject true skill prompt test")? else { + return Ok(()); + }; + + let approval_policy = AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + }); + let server = start_mock_server().await; + let tool_call_id = "zsh-fork-skill-reject-true"; + let test = build_zsh_fork_test( + &server, + runtime, + approval_policy, + SandboxPolicy::new_workspace_write_policy(), + |home| { + write_skill_with_shell_script(home, "mbolin-test-skill", "hello-mbolin.sh").unwrap(); + write_skill_metadata( + home, + "mbolin-test-skill", + r#" +permissions: + file_system: + write: + - "./output" +"#, + ) + .unwrap(); + }, + ) + .await?; + + let (_, command) = skill_script_command(&test, "hello-mbolin.sh")?; + let arguments = shell_command_arguments(&command)?; + let mocks = + mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command") + .await; + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + approval_policy, + SandboxPolicy::new_workspace_write_policy(), + ) + .await?; + + let approval = wait_for_exec_approval_request(&test).await; + assert!( + approval.is_none(), + "expected reject sandbox approval policy to skip exec approval" + ); + + wait_for_turn_complete(&test).await; + + let call_output = mocks + .completion + .single_request() + .function_call_output(tool_call_id); + let output = call_output["output"].as_str().unwrap_or_default(); + assert!( + output.contains("Execution denied: Execution forbidden by policy"), + "expected policy rejection marker in function_call_output: {output:?}" + ); + + Ok(()) +} + /// Permissionless skills should inherit the turn sandbox without prompting. #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]