diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 048a1818f46..bc2a846376a 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -66,6 +66,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 845c5eb4823..884c0c234c0 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -4920,6 +4920,11 @@ "sandbox_approval": { "description": "Reject approval prompts related to sandbox escalation.", "type": "boolean" + }, + "skill_approval": { + "default": false, + "description": "Reject approval prompts triggered by skill script execution.", + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index bc6f0c748ba..64b31f7797c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -6853,6 +6853,11 @@ "sandbox_approval": { "description": "Reject approval prompts related to sandbox escalation.", "type": "boolean" + }, + "skill_approval": { + "default": false, + "description": "Reject approval prompts triggered by skill script execution.", + "type": "boolean" } }, "required": [ @@ -9375,6 +9380,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index b67bb447a66..dd2736a5c8f 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -740,6 +740,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index a9c4d0b294c..fb832b42443 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -157,6 +157,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index 0eb33c2e12c..19d328f7505 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -29,6 +29,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json index 6d530e17fc9..113f1f657b0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json @@ -29,6 +29,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index 96772c6aae8..aa8017080e8 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -33,6 +33,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index c4d9dbc0c83..191ff80a992 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -29,6 +29,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index 013485bd122..3db3e5e96da 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -33,6 +33,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 69cde5a36af..630176f8c1e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -29,6 +29,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index 97193de56d2..eca31f4446d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -33,6 +33,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index 404a00209a4..2ea5881a236 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -33,6 +33,10 @@ }, "sandbox_approval": { "type": "boolean" + }, + "skill_approval": { + "default": false, + "type": "boolean" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts b/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts index 19b26481c70..67e5c261667 100644 --- a/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts +++ b/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts @@ -11,6 +11,10 @@ sandbox_approval: boolean, * Reject prompts triggered by execpolicy `prompt` rules. */ rules: boolean, +/** + * Reject approval prompts triggered by skill script execution. + */ +skill_approval: boolean, /** * Reject approval prompts related to built-in permission requests. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts index 46f5fa8c35a..55415eaea43 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AskForApproval = "untrusted" | "on-failure" | "on-request" | { "reject": { sandbox_approval: boolean, rules: boolean, request_permissions: boolean, mcp_elicitations: boolean, } } | "never"; +export type AskForApproval = "untrusted" | "on-failure" | "on-request" | { "reject": { sandbox_approval: boolean, rules: boolean, skill_approval: boolean, request_permissions: boolean, mcp_elicitations: boolean, } } | "never"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 1b7c0e7587b..846b6e74b98 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -205,6 +205,8 @@ pub enum AskForApproval { sandbox_approval: bool, rules: bool, #[serde(default)] + skill_approval: bool, + #[serde(default)] request_permissions: bool, mcp_elicitations: bool, }, @@ -220,11 +222,13 @@ impl AskForApproval { AskForApproval::Reject { sandbox_approval, rules, + skill_approval, request_permissions, mcp_elicitations, } => CoreAskForApproval::Reject(CoreRejectConfig { sandbox_approval, rules, + skill_approval, request_permissions, mcp_elicitations, }), @@ -242,6 +246,7 @@ impl From for AskForApproval { CoreAskForApproval::Reject(reject_config) => AskForApproval::Reject { sandbox_approval: reject_config.sandbox_approval, rules: reject_config.rules, + skill_approval: reject_config.skill_approval, request_permissions: reject_config.request_permissions, mcp_elicitations: reject_config.mcp_elicitations, }, @@ -6021,6 +6026,7 @@ mod tests { let v2_policy = AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }; @@ -6031,6 +6037,7 @@ mod tests { CoreAskForApproval::Reject(CoreRejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }) @@ -6041,7 +6048,7 @@ mod tests { } #[test] - fn ask_for_approval_reject_defaults_missing_request_permissions_to_false() { + fn ask_for_approval_reject_defaults_missing_optional_flags_to_false() { let decoded = serde_json::from_value::(serde_json::json!({ "reject": { "sandbox_approval": true, @@ -6056,6 +6063,7 @@ mod tests { AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, } @@ -6068,6 +6076,7 @@ mod tests { &AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }, @@ -6090,6 +6099,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }), @@ -6117,6 +6127,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }), @@ -6167,6 +6178,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }), @@ -6202,6 +6214,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Reject { sandbox_approval: true, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }]), @@ -6224,6 +6237,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }), @@ -6245,6 +6259,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }), @@ -6266,6 +6281,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }), @@ -6288,6 +6304,7 @@ mod tests { approval_policy: Some(AskForApproval::Reject { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }), diff --git a/codex-rs/app-server/tests/suite/v2/experimental_api.rs b/codex-rs/app-server/tests/suite/v2/experimental_api.rs index 1b07174fce2..aeb23814a47 100644 --- a/codex-rs/app-server/tests/suite/v2/experimental_api.rs +++ b/codex-rs/app-server/tests/suite/v2/experimental_api.rs @@ -183,6 +183,7 @@ async fn thread_start_reject_approval_policy_requires_experimental_api_capabilit approval_policy: Some(AskForApproval::Reject { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }), diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 573b35218a7..8ed6ad99850 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1344,6 +1344,11 @@ "sandbox_approval": { "description": "Reject approval prompts related to sandbox escalation.", "type": "boolean" + }, + "skill_approval": { + "default": false, + "description": "Reject approval prompts triggered by skill script execution.", + "type": "boolean" } }, "required": [ diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 7a17bdd98dd..225046d523f 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2307,6 +2307,7 @@ async fn request_permissions_emits_event_when_reject_policy_allows_requests() { crate::protocol::RejectConfig { sandbox_approval: true, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }, @@ -2381,6 +2382,7 @@ async fn request_permissions_returns_empty_grant_when_reject_policy_blocks_reque crate::protocol::RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, }, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 60edee65146..fc136fba093 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -1569,6 +1569,7 @@ prefix_rule(pattern=["git"], decision="prompt") AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), @@ -1591,6 +1592,7 @@ prefix_rule(pattern=["git"], decision="prompt") approval_policy: AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), @@ -1628,6 +1630,7 @@ prefix_rule(pattern=["git"], decision="prompt") approval_policy: AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), @@ -1663,6 +1666,7 @@ prefix_rule(pattern=["git"], decision="prompt") approval_policy: AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 713e16c8126..442e7e0c6e6 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -1738,6 +1738,7 @@ mod tests { RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, } @@ -1751,6 +1752,7 @@ mod tests { RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, } diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 7f39a7f6e6d..6d97e7cd61a 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -317,6 +317,7 @@ mod tests { AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), @@ -350,6 +351,7 @@ mod tests { AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }), diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index fd0168bf4d8..18a82bd948f 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -218,6 +218,7 @@ mod tests { !runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, })) @@ -226,6 +227,7 @@ mod tests { runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, })) 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 04732b00c70..35a4e332e90 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -5,7 +5,6 @@ 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::guardian::GuardianApprovalRequest; use crate::guardian::review_approval_request; @@ -63,6 +62,15 @@ pub(crate) struct PreparedUnifiedExecZshFork { pub(crate) escalation_session: EscalationSession, } +const PROMPT_CONFLICT_REASON: &str = + "approval required by policy, but AskForApproval is set to Never"; +const REJECT_SANDBOX_APPROVAL_REASON: &str = + "approval required by policy, but AskForApproval::Reject.sandbox_approval is set"; +const REJECT_RULES_APPROVAL_REASON: &str = + "approval required by policy rule, but AskForApproval::Reject.rules is set"; +const REJECT_SKILL_APPROVAL_REASON: &str = + "approval required by skill, but AskForApproval::Reject.skill_approval is set"; + pub(super) async fn try_run_zsh_fork( req: &ShellRequest, attempt: &SandboxAttempt<'_>, @@ -318,6 +326,31 @@ enum DecisionSource { UnmatchedCommandFallback, } +fn execve_prompt_is_rejected_by_policy( + approval_policy: AskForApproval, + decision_source: &DecisionSource, +) -> Option<&'static str> { + match (approval_policy, decision_source) { + (AskForApproval::Never, _) => Some(PROMPT_CONFLICT_REASON), + (AskForApproval::Reject(reject_config), DecisionSource::SkillScript { .. }) + if reject_config.rejects_skill_approval() => + { + Some(REJECT_SKILL_APPROVAL_REASON) + } + (AskForApproval::Reject(reject_config), DecisionSource::PrefixRule) + if reject_config.rejects_rules_approval() => + { + Some(REJECT_RULES_APPROVAL_REASON) + } + (AskForApproval::Reject(reject_config), DecisionSource::UnmatchedCommandFallback) + if reject_config.rejects_sandbox_approval() => + { + Some(REJECT_SANDBOX_APPROVAL_REASON) + } + _ => None, + } +} + impl CoreShellActionProvider { fn decision_driven_by_policy(matched_rules: &[RuleMatch], decision: Decision) -> bool { matched_rules.iter().any(|rule_match| { @@ -483,11 +516,8 @@ impl CoreShellActionProvider { EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } Decision::Prompt => { - if prompt_is_rejected_by_policy( - self.approval_policy, - matches!(decision_source, DecisionSource::PrefixRule), - ) - .is_some() + if execve_prompt_is_rejected_by_policy(self.approval_policy, &decision_source) + .is_some() { EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } else { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index af71bd5e4ea..e7c2145ba65 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -16,6 +16,7 @@ use crate::config::types::ShellEnvironmentPolicy; use crate::exec::SandboxType; use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; +use crate::protocol::RejectConfig; use crate::protocol::SandboxPolicy; use crate::sandboxing::SandboxPermissions; #[cfg(target_os = "macos")] @@ -80,6 +81,74 @@ fn test_skill_metadata(permission_profile: Option) -> SkillMe } } +#[test] +fn execve_prompt_rejection_uses_skill_approval_for_skill_scripts() { + let decision_source = super::DecisionSource::SkillScript { + skill: test_skill_metadata(None), + }; + + assert_eq!( + super::execve_prompt_is_rejected_by_policy( + AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: true, + skill_approval: false, + request_permissions: false, + mcp_elicitations: false, + }), + &decision_source, + ), + None, + ); + assert_eq!( + super::execve_prompt_is_rejected_by_policy( + AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: false, + skill_approval: true, + request_permissions: false, + mcp_elicitations: false, + }), + &decision_source, + ), + Some("approval required by skill, but AskForApproval::Reject.skill_approval is set"), + ); +} + +#[test] +fn execve_prompt_rejection_keeps_prefix_rules_on_rules_flag() { + assert_eq!( + super::execve_prompt_is_rejected_by_policy( + AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: true, + skill_approval: false, + request_permissions: false, + mcp_elicitations: false, + }), + &super::DecisionSource::PrefixRule, + ), + Some("approval required by policy rule, but AskForApproval::Reject.rules is set"), + ); +} + +#[test] +fn execve_prompt_rejection_keeps_unmatched_commands_on_sandbox_flag() { + assert_eq!( + super::execve_prompt_is_rejected_by_policy( + AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + skill_approval: false, + request_permissions: false, + mcp_elicitations: false, + }), + &super::DecisionSource::UnmatchedCommandFallback, + ), + Some("approval required by policy, but AskForApproval::Reject.sandbox_approval is set"), + ); +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 1a04f090eb3..fef4fa37378 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -398,6 +398,7 @@ mod tests { let policy = AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }); @@ -418,6 +419,7 @@ mod tests { let policy = AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }); diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 0c896aaed91..5abe2e8e987 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -288,6 +288,7 @@ async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_false_s let approval_policy = AskForApproval::Reject(RejectConfig { sandbox_approval: false, rules: true, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }); @@ -370,17 +371,20 @@ permissions: #[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() +async fn shell_zsh_fork_skill_script_reject_policy_with_sandbox_approval_true_still_prompts() -> Result<()> { skip_if_no_network!(Ok(())); - let Some(runtime) = zsh_fork_runtime("zsh-fork reject true skill prompt test")? else { + let Some(runtime) = + zsh_fork_runtime("zsh-fork reject sandbox approval true skill prompt test")? + else { return Ok(()); }; let approval_policy = AskForApproval::Reject(RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, }); @@ -422,10 +426,104 @@ permissions: ) .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); + + 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_skill_approval_true_skips_prompt() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("zsh-fork reject skill approval true skill prompt test")? + else { + return Ok(()); + }; + + let approval_policy = AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: false, + skill_approval: true, + request_permissions: false, + mcp_elicitations: false, + }); + let server = start_mock_server().await; + let tool_call_id = "zsh-fork-skill-reject-skill-approval-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" + "expected reject skill approval policy to skip exec approval" ); wait_for_turn_complete(&test).await; diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 28f6f5d60b8..af856786776 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -453,6 +453,7 @@ impl DeveloperInstructions { let on_request_instructions = on_request_instructions(); let sandbox_approval = reject_config.sandbox_approval; let rules = reject_config.rules; + let skill_approval = reject_config.skill_approval; let request_permissions = reject_config.request_permissions; let mcp_elicitations = reject_config.mcp_elicitations; format!( @@ -460,6 +461,7 @@ impl DeveloperInstructions { Approval policy is `reject`.\n\ - `sandbox_approval`: {sandbox_approval}\n\ - `rules`: {rules}\n\ + - `skill_approval`: {skill_approval}\n\ - `request_permissions`: {request_permissions}\n\ - `mcp_elicitations`: {mcp_elicitations}\n\ When a category is `true`, requests in that category are auto-rejected instead of prompting the user." diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 98ff8f7f55b..e76ae07643a 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -533,6 +533,9 @@ pub struct RejectConfig { pub sandbox_approval: bool, /// Reject prompts triggered by execpolicy `prompt` rules. pub rules: bool, + /// Reject approval prompts triggered by skill script execution. + #[serde(default)] + pub skill_approval: bool, /// Reject approval prompts related to built-in permission requests. #[serde(default)] pub request_permissions: bool, @@ -549,6 +552,10 @@ impl RejectConfig { self.rules } + pub const fn rejects_skill_approval(self) -> bool { + self.skill_approval + } + pub const fn rejects_request_permissions(self) -> bool { self.request_permissions } @@ -3457,6 +3464,7 @@ mod tests { RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, } @@ -3466,6 +3474,7 @@ mod tests { !RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, } @@ -3473,12 +3482,37 @@ mod tests { ); } + #[test] + fn reject_config_skill_approval_flag_is_field_driven() { + assert!( + RejectConfig { + sandbox_approval: false, + rules: false, + skill_approval: true, + request_permissions: false, + mcp_elicitations: false, + } + .rejects_skill_approval() + ); + assert!( + !RejectConfig { + sandbox_approval: false, + rules: false, + skill_approval: false, + request_permissions: false, + mcp_elicitations: false, + } + .rejects_skill_approval() + ); + } + #[test] fn reject_config_request_permissions_flag_is_field_driven() { assert!( RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: true, mcp_elicitations: false, } @@ -3488,6 +3522,7 @@ mod tests { !RejectConfig { sandbox_approval: false, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: false, } @@ -3496,7 +3531,7 @@ mod tests { } #[test] - fn reject_config_defaults_missing_request_permissions_to_false() { + fn reject_config_defaults_missing_optional_flags_to_false() { let decoded = serde_json::from_value::(serde_json::json!({ "sandbox_approval": true, "rules": false, @@ -3509,6 +3544,7 @@ mod tests { RejectConfig { sandbox_approval: true, rules: false, + skill_approval: false, request_permissions: false, mcp_elicitations: true, }