mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore: add a separate reject-policy flag for skill approvals (#14271)
## Summary - add `skill_approval` to `RejectConfig` and the app-server v2 `AskForApproval::Reject` payload so skill-script prompts can be configured independently from sandbox and rule-based prompts - update Unix shell escalation to reject prompts based on the actual decision source, keeping prefix rules tied to `rules`, unmatched command fallbacks tied to `sandbox_approval`, and skill scripts tied to `skill_approval` - regenerate the affected protocol/config schemas and expand unit/integration coverage for the new flag and skill approval behavior
This commit is contained in:
committed by
Michael Bolin
Unverified
parent
83b22bb612
commit
c1a424691f
@@ -66,6 +66,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -4944,6 +4944,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": [
|
||||
|
||||
@@ -6911,6 +6911,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": [
|
||||
@@ -9433,6 +9438,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -740,6 +740,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -157,6 +157,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -33,6 +33,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -33,6 +33,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -29,6 +29,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -33,6 +33,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -33,6 +33,10 @@
|
||||
},
|
||||
"sandbox_approval": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"skill_approval": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -206,6 +206,8 @@ pub enum AskForApproval {
|
||||
sandbox_approval: bool,
|
||||
rules: bool,
|
||||
#[serde(default)]
|
||||
skill_approval: bool,
|
||||
#[serde(default)]
|
||||
request_permissions: bool,
|
||||
mcp_elicitations: bool,
|
||||
},
|
||||
@@ -221,11 +223,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,
|
||||
}),
|
||||
@@ -243,6 +247,7 @@ impl From<CoreAskForApproval> 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,
|
||||
},
|
||||
@@ -6159,6 +6164,7 @@ mod tests {
|
||||
let v2_policy = AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
};
|
||||
@@ -6169,6 +6175,7 @@ mod tests {
|
||||
CoreAskForApproval::Reject(CoreRejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
})
|
||||
@@ -6179,7 +6186,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::<AskForApproval>(serde_json::json!({
|
||||
"reject": {
|
||||
"sandbox_approval": true,
|
||||
@@ -6194,6 +6201,7 @@ mod tests {
|
||||
AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}
|
||||
@@ -6206,6 +6214,7 @@ mod tests {
|
||||
&AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
},
|
||||
@@ -6228,6 +6237,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
@@ -6255,6 +6265,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
@@ -6305,6 +6316,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
@@ -6340,6 +6352,7 @@ mod tests {
|
||||
allowed_approval_policies: Some(vec![AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
}]),
|
||||
@@ -6362,6 +6375,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
@@ -6383,6 +6397,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
@@ -6404,6 +6419,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
@@ -6426,6 +6442,7 @@ mod tests {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
|
||||
@@ -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": [
|
||||
|
||||
@@ -2308,6 +2308,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,
|
||||
},
|
||||
@@ -2382,6 +2383,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,
|
||||
},
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
|
||||
@@ -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,
|
||||
}))
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<PermissionProfile>) -> 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!(
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -481,6 +481,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!(
|
||||
@@ -488,6 +489,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."
|
||||
|
||||
@@ -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::<RejectConfig>(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,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user