mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Rename reject approval policy to granular (#14516)
This commit is contained in:
committed by
GitHub
Unverified
parent
d32820ab07
commit
b7dba72dbd
@@ -2912,8 +2912,8 @@ impl Session {
|
||||
scope: PermissionGrantScope::Turn,
|
||||
});
|
||||
}
|
||||
AskForApproval::Reject(reject_config)
|
||||
if reject_config.rejects_request_permissions() =>
|
||||
AskForApproval::Granular(granular_config)
|
||||
if !granular_config.allows_request_permissions() =>
|
||||
{
|
||||
return Some(RequestPermissionsResponse {
|
||||
permissions: PermissionProfile::default(),
|
||||
@@ -2923,7 +2923,7 @@ impl Session {
|
||||
AskForApproval::OnFailure
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::UnlessTrusted
|
||||
| AskForApproval::Reject(_) => {}
|
||||
| AskForApproval::Granular(_) => {}
|
||||
}
|
||||
|
||||
let (tx_response, rx_response) = oneshot::channel();
|
||||
@@ -3381,7 +3381,9 @@ impl Session {
|
||||
turn_context.approval_policy.value(),
|
||||
self.services.exec_policy.current().as_ref(),
|
||||
&turn_context.cwd,
|
||||
turn_context.features.enabled(Feature::RequestPermissions),
|
||||
turn_context
|
||||
.features
|
||||
.enabled(Feature::ExecPermissionApprovals),
|
||||
)
|
||||
.into_text(),
|
||||
);
|
||||
@@ -5565,7 +5567,7 @@ pub(crate) async fn run_turn(
|
||||
AskForApproval::UnlessTrusted
|
||||
| AskForApproval::OnFailure
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::Reject(_) => "default",
|
||||
| AskForApproval::Granular(_) => "default",
|
||||
}
|
||||
.to_string();
|
||||
let session_start_request = codex_hooks::SessionStartRequest {
|
||||
@@ -5714,7 +5716,7 @@ pub(crate) async fn run_turn(
|
||||
AskForApproval::UnlessTrusted
|
||||
| AskForApproval::OnFailure
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::Reject(_) => "default",
|
||||
| AskForApproval::Granular(_) => "default",
|
||||
}
|
||||
.to_string();
|
||||
let stop_request = codex_hooks::StopRequest {
|
||||
|
||||
@@ -2231,18 +2231,18 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn request_permissions_emits_event_when_reject_policy_allows_requests() {
|
||||
async fn request_permissions_emits_event_when_granular_policy_allows_requests() {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single turn context ref")
|
||||
.approval_policy
|
||||
.set(crate::protocol::AskForApproval::Reject(
|
||||
crate::protocol::RejectConfig {
|
||||
.set(crate::protocol::AskForApproval::Granular(
|
||||
crate::protocol::GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
},
|
||||
))
|
||||
@@ -2306,19 +2306,19 @@ async fn request_permissions_emits_event_when_reject_policy_allows_requests() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn request_permissions_is_auto_denied_when_reject_policy_blocks_tool_requests() {
|
||||
async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_requests() {
|
||||
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
|
||||
*session.active_turn.lock().await = Some(ActiveTurn::default());
|
||||
Arc::get_mut(&mut turn_context)
|
||||
.expect("single turn context ref")
|
||||
.approval_policy
|
||||
.set(crate::protocol::AskForApproval::Reject(
|
||||
crate::protocol::RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
.set(crate::protocol::AskForApproval::Granular(
|
||||
crate::protocol::GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
},
|
||||
))
|
||||
.expect("test setup should allow updating approval policy");
|
||||
@@ -2355,7 +2355,7 @@ async fn request_permissions_is_auto_denied_when_reject_policy_blocks_tool_reque
|
||||
tokio::time::timeout(StdDuration::from_millis(100), rx.recv())
|
||||
.await
|
||||
.is_err(),
|
||||
"request_permissions should not emit an event when reject.request_permissions is set"
|
||||
"request_permissions should not emit an event when granular.request_permissions is false"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -75,7 +75,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
||||
.expect("test setup should allow enabling guardian approvals");
|
||||
session
|
||||
.features
|
||||
.enable(Feature::RequestPermissions)
|
||||
.enable(Feature::ExecPermissionApprovals)
|
||||
.expect("test setup should allow enabling request permissions");
|
||||
turn_context_raw
|
||||
.sandbox_policy
|
||||
@@ -191,7 +191,7 @@ async fn guardian_allows_unified_exec_additional_permissions_requests_past_polic
|
||||
.expect("test setup should allow enabling guardian approvals");
|
||||
session
|
||||
.features
|
||||
.enable(Feature::RequestPermissions)
|
||||
.enable(Feature::ExecPermissionApprovals)
|
||||
.expect("test setup should allow enabling request permissions");
|
||||
let session = Arc::new(session);
|
||||
let turn_context = Arc::new(turn_context_raw);
|
||||
|
||||
@@ -45,7 +45,7 @@ fn build_permissions_update_item(
|
||||
next.approval_policy.value(),
|
||||
exec_policy,
|
||||
&next.cwd,
|
||||
next.features.enabled(Feature::RequestPermissions),
|
||||
next.features.enabled(Feature::ExecPermissionApprovals),
|
||||
))
|
||||
}
|
||||
|
||||
|
||||
@@ -38,9 +38,9 @@ use shlex::try_join as shlex_try_join;
|
||||
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";
|
||||
"approval required by policy, but AskForApproval::Granular.sandbox_approval is false";
|
||||
const REJECT_RULES_APPROVAL_REASON: &str =
|
||||
"approval required by policy rule, but AskForApproval::Reject.rules is set";
|
||||
"approval required by policy rule, but AskForApproval::Granular.rules is false";
|
||||
const RULES_DIR_NAME: &str = "rules";
|
||||
const RULE_EXTENSION: &str = "rules";
|
||||
const DEFAULT_POLICY_FILE: &str = "default.rules";
|
||||
@@ -104,7 +104,7 @@ fn is_policy_match(rule_match: &RuleMatch) -> bool {
|
||||
/// current prompt to the user.
|
||||
///
|
||||
/// `prompt_is_rule` distinguishes policy-rule prompts from sandbox/escalation
|
||||
/// prompts so `Reject.rules` and `Reject.sandbox_approval` are honored
|
||||
/// prompts so granular `rules` and `sandbox_approval` settings are honored
|
||||
/// independently. When both are present, policy-rule prompts take precedence.
|
||||
pub(crate) fn prompt_is_rejected_by_policy(
|
||||
approval_policy: AskForApproval,
|
||||
@@ -115,14 +115,14 @@ pub(crate) fn prompt_is_rejected_by_policy(
|
||||
AskForApproval::OnFailure => None,
|
||||
AskForApproval::OnRequest => None,
|
||||
AskForApproval::UnlessTrusted => None,
|
||||
AskForApproval::Reject(reject_config) => {
|
||||
AskForApproval::Granular(granular_config) => {
|
||||
if prompt_is_rule {
|
||||
if reject_config.rejects_rules_approval() {
|
||||
if !granular_config.allows_rules_approval() {
|
||||
Some(REJECT_RULES_APPROVAL_REASON)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else if reject_config.rejects_sandbox_approval() {
|
||||
} else if !granular_config.allows_sandbox_approval() {
|
||||
Some(REJECT_SANDBOX_APPROVAL_REASON)
|
||||
} else {
|
||||
None
|
||||
@@ -519,7 +519,7 @@ pub fn render_decision_for_unmatched_command(
|
||||
AskForApproval::OnFailure
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::UnlessTrusted
|
||||
| AskForApproval::Reject(_) => Decision::Prompt,
|
||||
| AskForApproval::Granular(_) => Decision::Prompt,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -554,7 +554,7 @@ pub fn render_decision_for_unmatched_command(
|
||||
}
|
||||
}
|
||||
}
|
||||
AskForApproval::Reject(_) => match file_system_sandbox_policy.kind {
|
||||
AskForApproval::Granular(_) => match file_system_sandbox_policy.kind {
|
||||
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
|
||||
// Mirror on-request behavior for unmatched commands; prompt-vs-reject is handled
|
||||
// by `prompt_is_rejected_by_policy`.
|
||||
|
||||
@@ -9,7 +9,7 @@ use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -766,18 +766,18 @@ async fn exec_approval_requirement_respects_approval_policy() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unmatched_reject_policy_still_prompts_for_restricted_sandbox_escalation() {
|
||||
fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() {
|
||||
let command = vec!["madeup-cmd".to_string()];
|
||||
|
||||
assert_eq!(
|
||||
Decision::Prompt,
|
||||
render_decision_for_unmatched_command(
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
&read_only_file_system_sandbox_policy(),
|
||||
@@ -807,19 +807,19 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_sandbox_rejection_enabled()
|
||||
async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_granular_sandbox_is_disabled()
|
||||
{
|
||||
let command = vec!["madeup-cmd".to_string()];
|
||||
|
||||
let requirement = ExecPolicyManager::default()
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
@@ -853,12 +853,12 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::Reject(RejectConfig {
|
||||
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
@@ -874,7 +874,7 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mixed_rule_and_sandbox_prompt_rejects_when_rules_rejection_enabled() {
|
||||
async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled() {
|
||||
let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt")"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
@@ -890,12 +890,12 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_rules_rejection_enabled() {
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
approval_policy: AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
sandbox_policy: &SandboxPolicy::new_read_only_policy(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
|
||||
@@ -95,8 +95,8 @@ pub enum Feature {
|
||||
ShellZshFork,
|
||||
/// Include the freeform apply_patch tool.
|
||||
ApplyPatchFreeform,
|
||||
/// Allow requesting additional filesystem permissions while staying sandboxed.
|
||||
RequestPermissions,
|
||||
/// Allow exec tools to request additional permissions while staying sandboxed.
|
||||
ExecPermissionApprovals,
|
||||
/// Enable Claude-style lifecycle hooks loaded from hooks.json files.
|
||||
CodexHooks,
|
||||
/// Expose the built-in request_permissions tool.
|
||||
@@ -626,8 +626,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::RequestPermissions,
|
||||
key: "request_permissions",
|
||||
id: Feature::ExecPermissionApprovals,
|
||||
key: "exec_permission_approvals",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
|
||||
@@ -29,6 +29,10 @@ const ALIASES: &[Alias] = &[
|
||||
legacy_key: "include_apply_patch_tool",
|
||||
feature: Feature::ApplyPatchFreeform,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "request_permissions",
|
||||
feature: Feature::ExecPermissionApprovals,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "web_search",
|
||||
feature: Feature::WebSearchRequest,
|
||||
|
||||
@@ -80,8 +80,11 @@ fn guardian_approval_is_experimental_and_user_toggleable() {
|
||||
|
||||
#[test]
|
||||
fn request_permissions_is_under_development() {
|
||||
assert_eq!(Feature::RequestPermissions.stage(), Stage::UnderDevelopment);
|
||||
assert_eq!(Feature::RequestPermissions.default_enabled(), false);
|
||||
assert_eq!(
|
||||
Feature::ExecPermissionApprovals.stage(),
|
||||
Stage::UnderDevelopment
|
||||
);
|
||||
assert_eq!(Feature::ExecPermissionApprovals.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -254,7 +254,7 @@ fn elicitation_is_rejected_by_policy(approval_policy: AskForApproval) -> bool {
|
||||
AskForApproval::OnFailure => false,
|
||||
AskForApproval::OnRequest => false,
|
||||
AskForApproval::UnlessTrusted => false,
|
||||
AskForApproval::Reject(reject_config) => reject_config.rejects_mcp_elicitations(),
|
||||
AskForApproval::Granular(granular_config) => !granular_config.allows_mcp_elicitations(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::*;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::McpAuthStatus;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use rmcp::model::JsonObject;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
@@ -61,7 +61,7 @@ fn create_codex_apps_tools_cache_context(
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn elicitation_reject_policy_defaults_to_prompting() {
|
||||
fn elicitation_granular_policy_defaults_to_prompting() {
|
||||
assert!(!elicitation_is_rejected_by_policy(
|
||||
AskForApproval::OnFailure
|
||||
));
|
||||
@@ -71,27 +71,27 @@ fn elicitation_reject_policy_defaults_to_prompting() {
|
||||
assert!(!elicitation_is_rejected_by_policy(
|
||||
AskForApproval::UnlessTrusted
|
||||
));
|
||||
assert!(!elicitation_is_rejected_by_policy(AskForApproval::Reject(
|
||||
RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
assert!(elicitation_is_rejected_by_policy(AskForApproval::Granular(
|
||||
GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn elicitation_reject_policy_respects_never_and_reject_config() {
|
||||
fn elicitation_granular_policy_respects_never_and_config() {
|
||||
assert!(elicitation_is_rejected_by_policy(AskForApproval::Never));
|
||||
assert!(elicitation_is_rejected_by_policy(AskForApproval::Reject(
|
||||
RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
assert!(elicitation_is_rejected_by_policy(AskForApproval::Granular(
|
||||
GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ pub fn assess_patch_safety(
|
||||
AskForApproval::OnFailure
|
||||
| AskForApproval::Never
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::Reject(_) => {
|
||||
| AskForApproval::Granular(_) => {
|
||||
// Continue to see if this can be auto-approved.
|
||||
}
|
||||
// TODO(ragona): I'm not sure this is actually correct? I believe in this case
|
||||
@@ -56,7 +56,7 @@ pub fn assess_patch_safety(
|
||||
let rejects_sandbox_approval = matches!(policy, AskForApproval::Never)
|
||||
|| matches!(
|
||||
policy,
|
||||
AskForApproval::Reject(reject_config) if reject_config.sandbox_approval
|
||||
AskForApproval::Granular(granular_config) if !granular_config.sandbox_approval
|
||||
);
|
||||
|
||||
// Even though the patch appears to be constrained to writable paths, it is
|
||||
|
||||
@@ -3,7 +3,7 @@ use codex_protocol::protocol::FileSystemAccessMode;
|
||||
use codex_protocol::protocol::FileSystemPath;
|
||||
use codex_protocol::protocol::FileSystemSandboxEntry;
|
||||
use codex_protocol::protocol::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
@@ -87,7 +87,7 @@ fn external_sandbox_auto_approves_in_on_request() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_with_all_flags_false_matches_on_request_for_out_of_root_patch() {
|
||||
fn granular_with_all_flags_true_matches_on_request_for_out_of_root_patch() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let parent = cwd.parent().unwrap().to_path_buf();
|
||||
@@ -115,12 +115,12 @@ fn reject_with_all_flags_false_matches_on_request_for_out_of_root_patch() {
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_outside,
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&policy_workspace_only,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
@@ -132,7 +132,7 @@ fn reject_with_all_flags_false_matches_on_request_for_out_of_root_patch() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_sandbox_approval_rejects_out_of_root_patch() {
|
||||
fn granular_sandbox_approval_false_rejects_out_of_root_patch() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cwd = tmp.path().to_path_buf();
|
||||
let parent = cwd.parent().unwrap().to_path_buf();
|
||||
@@ -149,12 +149,12 @@ fn reject_sandbox_approval_rejects_out_of_root_patch() {
|
||||
assert_eq!(
|
||||
assess_patch_safety(
|
||||
&add_outside,
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&policy_workspace_only,
|
||||
&FileSystemSandboxPolicy::from(&policy_workspace_only),
|
||||
|
||||
@@ -118,7 +118,7 @@ pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
&& (uses_additional_permissions || additional_permissions.is_some())
|
||||
{
|
||||
return Err(
|
||||
"additional permissions are disabled; enable `features.request_permissions` before using `with_additional_permissions`"
|
||||
"additional permissions are disabled; enable `features.exec_permission_approvals` before using `with_additional_permissions`"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
@@ -239,7 +239,7 @@ mod tests {
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
@@ -266,18 +266,18 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_inline_feature()
|
||||
fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_exec_permission_approvals_feature()
|
||||
{
|
||||
let cwd = tempdir().expect("tempdir");
|
||||
|
||||
let normalized = normalize_and_validate_additional_permissions(
|
||||
false,
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
Some(network_permissions()),
|
||||
@@ -290,7 +290,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fresh_additional_permissions_still_require_request_permissions_feature() {
|
||||
fn fresh_additional_permissions_still_require_exec_permission_approvals_feature() {
|
||||
let cwd = tempdir().expect("tempdir");
|
||||
|
||||
let err = normalize_and_validate_additional_permissions(
|
||||
@@ -305,7 +305,7 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
err,
|
||||
"additional permissions are disabled; enable `features.request_permissions` before using `with_additional_permissions`"
|
||||
"additional permissions are disabled; enable `features.exec_permission_approvals` before using `with_additional_permissions`"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -336,7 +336,8 @@ impl ShellHandler {
|
||||
}
|
||||
}
|
||||
|
||||
let request_permission_enabled = session.features().enabled(Feature::RequestPermissions);
|
||||
let exec_permission_approvals_enabled =
|
||||
session.features().enabled(Feature::ExecPermissionApprovals);
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session.as_ref(),
|
||||
@@ -344,7 +345,7 @@ impl ShellHandler {
|
||||
additional_permissions,
|
||||
)
|
||||
.await;
|
||||
let additional_permissions_allowed = request_permission_enabled
|
||||
let additional_permissions_allowed = exec_permission_approvals_enabled
|
||||
|| (session.features().enabled(Feature::RequestPermissionsTool)
|
||||
&& effective_additional_permissions.permissions_preapproved);
|
||||
let normalized_additional_permissions = implicit_granted_permissions(
|
||||
|
||||
@@ -170,8 +170,8 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
..
|
||||
} = args;
|
||||
|
||||
let request_permission_enabled =
|
||||
session.features().enabled(Feature::RequestPermissions);
|
||||
let exec_permission_approvals_enabled =
|
||||
session.features().enabled(Feature::ExecPermissionApprovals);
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
context.session.as_ref(),
|
||||
@@ -179,7 +179,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
additional_permissions,
|
||||
)
|
||||
.await;
|
||||
let additional_permissions_allowed = request_permission_enabled
|
||||
let additional_permissions_allowed = exec_permission_approvals_enabled
|
||||
|| (session.features().enabled(Feature::RequestPermissionsTool)
|
||||
&& effective_additional_permissions.permissions_preapproved);
|
||||
|
||||
|
||||
@@ -166,7 +166,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
|
||||
match policy {
|
||||
AskForApproval::Never => false,
|
||||
AskForApproval::Reject(reject_config) => !reject_config.rejects_sandbox_approval(),
|
||||
AskForApproval::Granular(granular_config) => granular_config.allows_sandbox_approval(),
|
||||
AskForApproval::OnFailure => true,
|
||||
AskForApproval::OnRequest => true,
|
||||
AskForApproval::UnlessTrusted => true,
|
||||
|
||||
@@ -1,28 +1,28 @@
|
||||
use super::*;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[test]
|
||||
fn wants_no_sandbox_approval_reject_respects_sandbox_flag() {
|
||||
fn wants_no_sandbox_approval_granular_respects_sandbox_flag() {
|
||||
let runtime = ApplyPatchRuntime::new();
|
||||
assert!(runtime.wants_no_sandbox_approval(AskForApproval::OnRequest));
|
||||
assert!(
|
||||
!runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
!runtime.wants_no_sandbox_approval(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}))
|
||||
);
|
||||
assert!(
|
||||
runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
runtime.wants_no_sandbox_approval(AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -65,11 +65,11 @@ pub(crate) struct PreparedUnifiedExecZshFork {
|
||||
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";
|
||||
"approval required by policy, but AskForApproval::Granular.sandbox_approval is false";
|
||||
const REJECT_RULES_APPROVAL_REASON: &str =
|
||||
"approval required by policy rule, but AskForApproval::Reject.rules is set";
|
||||
"approval required by policy rule, but AskForApproval::Granular.rules is false";
|
||||
const REJECT_SKILL_APPROVAL_REASON: &str =
|
||||
"approval required by skill, but AskForApproval::Reject.skill_approval is set";
|
||||
"approval required by skill, but AskForApproval::Granular.skill_approval is false";
|
||||
|
||||
fn approval_sandbox_permissions(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
@@ -358,18 +358,18 @@ fn execve_prompt_is_rejected_by_policy(
|
||||
) -> 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() =>
|
||||
(AskForApproval::Granular(granular_config), DecisionSource::SkillScript { .. })
|
||||
if !granular_config.allows_skill_approval() =>
|
||||
{
|
||||
Some(REJECT_SKILL_APPROVAL_REASON)
|
||||
}
|
||||
(AskForApproval::Reject(reject_config), DecisionSource::PrefixRule)
|
||||
if reject_config.rejects_rules_approval() =>
|
||||
(AskForApproval::Granular(granular_config), DecisionSource::PrefixRule)
|
||||
if !granular_config.allows_rules_approval() =>
|
||||
{
|
||||
Some(REJECT_RULES_APPROVAL_REASON)
|
||||
}
|
||||
(AskForApproval::Reject(reject_config), DecisionSource::UnmatchedCommandFallback)
|
||||
if reject_config.rejects_sandbox_approval() =>
|
||||
(AskForApproval::Granular(granular_config), DecisionSource::UnmatchedCommandFallback)
|
||||
if !granular_config.allows_sandbox_approval() =>
|
||||
{
|
||||
Some(REJECT_SANDBOX_APPROVAL_REASON)
|
||||
}
|
||||
|
||||
@@ -16,8 +16,8 @@ use crate::config::Permissions;
|
||||
use crate::config::types::ShellEnvironmentPolicy;
|
||||
use crate::exec::SandboxType;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::GranularApprovalConfig;
|
||||
use crate::protocol::ReadOnlyAccess;
|
||||
use crate::protocol::RejectConfig;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
@@ -105,12 +105,12 @@ fn execve_prompt_rejection_uses_skill_approval_for_skill_scripts() {
|
||||
|
||||
assert_eq!(
|
||||
super::execve_prompt_is_rejected_by_policy(
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&decision_source,
|
||||
),
|
||||
@@ -118,16 +118,16 @@ fn execve_prompt_rejection_uses_skill_approval_for_skill_scripts() {
|
||||
);
|
||||
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,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&decision_source,
|
||||
),
|
||||
Some("approval required by skill, but AskForApproval::Reject.skill_approval is set"),
|
||||
Some("approval required by skill, but AskForApproval::Granular.skill_approval is false"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -135,16 +135,16 @@ fn execve_prompt_rejection_uses_skill_approval_for_skill_scripts() {
|
||||
fn execve_prompt_rejection_keeps_prefix_rules_on_rules_flag() {
|
||||
assert_eq!(
|
||||
super::execve_prompt_is_rejected_by_policy(
|
||||
AskForApproval::Reject(RejectConfig {
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
rules: false,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&super::DecisionSource::PrefixRule,
|
||||
),
|
||||
Some("approval required by policy rule, but AskForApproval::Reject.rules is set"),
|
||||
Some("approval required by policy rule, but AskForApproval::Granular.rules is false"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -152,16 +152,16 @@ fn execve_prompt_rejection_keeps_prefix_rules_on_rules_flag() {
|
||||
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,
|
||||
AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
&super::DecisionSource::UnmatchedCommandFallback,
|
||||
),
|
||||
Some("approval required by policy, but AskForApproval::Reject.sandbox_approval is set"),
|
||||
Some("approval required by policy, but AskForApproval::Granular.sandbox_approval is false"),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -161,8 +161,8 @@ impl ExecApprovalRequirement {
|
||||
|
||||
/// - Never, OnFailure: do not ask
|
||||
/// - OnRequest: ask unless filesystem access is unrestricted
|
||||
/// - Reject: ask unless filesystem access is unrestricted, but auto-reject
|
||||
/// when `sandbox_approval` rejection is enabled.
|
||||
/// - Granular: ask unless filesystem access is unrestricted, but auto-reject
|
||||
/// when granular sandbox approval is disabled.
|
||||
/// - UnlessTrusted: always ask
|
||||
pub(crate) fn default_exec_approval_requirement(
|
||||
policy: AskForApproval,
|
||||
@@ -170,7 +170,7 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
) -> ExecApprovalRequirement {
|
||||
let needs_approval = match policy {
|
||||
AskForApproval::Never | AskForApproval::OnFailure => false,
|
||||
AskForApproval::OnRequest | AskForApproval::Reject(_) => {
|
||||
AskForApproval::OnRequest | AskForApproval::Granular(_) => {
|
||||
matches!(
|
||||
file_system_sandbox_policy.kind,
|
||||
FileSystemSandboxKind::Restricted
|
||||
@@ -182,11 +182,12 @@ pub(crate) fn default_exec_approval_requirement(
|
||||
if needs_approval
|
||||
&& matches!(
|
||||
policy,
|
||||
AskForApproval::Reject(reject_config) if reject_config.rejects_sandbox_approval()
|
||||
AskForApproval::Granular(granular_config)
|
||||
if !granular_config.allows_sandbox_approval()
|
||||
)
|
||||
{
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
reason: "approval policy rejected sandbox approval prompt".to_string(),
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
} else if needs_approval {
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
@@ -268,7 +269,7 @@ pub(crate) trait Approvable<Req> {
|
||||
AskForApproval::UnlessTrusted => true,
|
||||
AskForApproval::Never => false,
|
||||
AskForApproval::OnRequest => false,
|
||||
AskForApproval::Reject(reject_config) => !reject_config.sandbox_approval,
|
||||
AskForApproval::Granular(granular_config) => granular_config.sandbox_approval,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use super::*;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use codex_protocol::protocol::RejectConfig;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
@@ -37,13 +37,13 @@ fn restricted_sandbox_requires_exec_approval_on_request() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_exec_approval_requirement_rejects_sandbox_prompt_when_configured() {
|
||||
let policy = AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disables_it() {
|
||||
let policy = AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: true,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
@@ -53,19 +53,19 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_configured() {
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::Forbidden {
|
||||
reason: "approval policy rejected sandbox approval prompt".to_string(),
|
||||
reason: "approval policy disallowed sandbox approval prompt".to_string(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_exec_approval_requirement_keeps_prompt_when_sandbox_rejection_is_disabled() {
|
||||
let policy = AskForApproval::Reject(RejectConfig {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
skill_approval: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_approval() {
|
||||
let policy = AskForApproval::Granular(GranularApprovalConfig {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
skill_approval: true,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
@@ -118,7 +118,7 @@ pub(crate) struct ToolsConfig {
|
||||
pub agent_roles: BTreeMap<String, AgentRoleConfig>,
|
||||
pub search_tool: bool,
|
||||
pub tool_suggest: bool,
|
||||
pub request_permission_enabled: bool,
|
||||
pub exec_permission_approvals_enabled: bool,
|
||||
pub request_permissions_tool_enabled: bool,
|
||||
pub code_mode_enabled: bool,
|
||||
pub js_repl_enabled: bool,
|
||||
@@ -184,7 +184,7 @@ impl ToolsConfig {
|
||||
features.enabled(Feature::Artifact) && codex_artifacts::can_manage_artifact_runtime();
|
||||
let include_image_gen_tool =
|
||||
features.enabled(Feature::ImageGeneration) && supports_image_generation(model_info);
|
||||
let request_permission_enabled = features.enabled(Feature::RequestPermissions);
|
||||
let exec_permission_approvals_enabled = features.enabled(Feature::ExecPermissionApprovals);
|
||||
let request_permissions_tool_enabled = features.enabled(Feature::RequestPermissionsTool);
|
||||
let shell_command_backend =
|
||||
if features.enabled(Feature::ShellTool) && features.enabled(Feature::ShellZshFork) {
|
||||
@@ -255,7 +255,7 @@ impl ToolsConfig {
|
||||
agent_roles: BTreeMap::new(),
|
||||
search_tool: include_search_tool,
|
||||
tool_suggest: include_tool_suggest,
|
||||
request_permission_enabled,
|
||||
exec_permission_approvals_enabled,
|
||||
request_permissions_tool_enabled,
|
||||
code_mode_enabled: include_code_mode,
|
||||
js_repl_enabled: include_js_repl,
|
||||
@@ -441,13 +441,15 @@ fn create_permissions_schema() -> JsonSchema {
|
||||
}
|
||||
}
|
||||
|
||||
fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<String, JsonSchema> {
|
||||
fn create_approval_parameters(
|
||||
exec_permission_approvals_enabled: bool,
|
||||
) -> BTreeMap<String, JsonSchema> {
|
||||
let mut properties = BTreeMap::from([
|
||||
(
|
||||
"sandbox_permissions".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
if request_permission_enabled {
|
||||
if exec_permission_approvals_enabled {
|
||||
"Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem, network, or macOS permissions (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
|
||||
} else {
|
||||
"Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
|
||||
@@ -482,7 +484,7 @@ fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<Stri
|
||||
)
|
||||
]);
|
||||
|
||||
if request_permission_enabled {
|
||||
if exec_permission_approvals_enabled {
|
||||
properties.insert(
|
||||
"additional_permissions".to_string(),
|
||||
create_permissions_schema(),
|
||||
@@ -492,7 +494,10 @@ fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap<Stri
|
||||
properties
|
||||
}
|
||||
|
||||
fn create_exec_command_tool(allow_login_shell: bool, request_permission_enabled: bool) -> ToolSpec {
|
||||
fn create_exec_command_tool(
|
||||
allow_login_shell: bool,
|
||||
exec_permission_approvals_enabled: bool,
|
||||
) -> ToolSpec {
|
||||
let mut properties = BTreeMap::from([
|
||||
(
|
||||
"cmd".to_string(),
|
||||
@@ -552,7 +557,9 @@ fn create_exec_command_tool(allow_login_shell: bool, request_permission_enabled:
|
||||
},
|
||||
);
|
||||
}
|
||||
properties.extend(create_approval_parameters(request_permission_enabled));
|
||||
properties.extend(create_approval_parameters(
|
||||
exec_permission_approvals_enabled,
|
||||
));
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "exec_command".to_string(),
|
||||
@@ -669,7 +676,7 @@ fn create_exec_wait_tool() -> ToolSpec {
|
||||
})
|
||||
}
|
||||
|
||||
fn create_shell_tool(request_permission_enabled: bool) -> ToolSpec {
|
||||
fn create_shell_tool(exec_permission_approvals_enabled: bool) -> ToolSpec {
|
||||
let mut properties = BTreeMap::from([
|
||||
(
|
||||
"command".to_string(),
|
||||
@@ -691,7 +698,9 @@ fn create_shell_tool(request_permission_enabled: bool) -> ToolSpec {
|
||||
},
|
||||
),
|
||||
]);
|
||||
properties.extend(create_approval_parameters(request_permission_enabled));
|
||||
properties.extend(create_approval_parameters(
|
||||
exec_permission_approvals_enabled,
|
||||
));
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"].
|
||||
@@ -726,7 +735,7 @@ Examples of valid command strings:
|
||||
|
||||
fn create_shell_command_tool(
|
||||
allow_login_shell: bool,
|
||||
request_permission_enabled: bool,
|
||||
exec_permission_approvals_enabled: bool,
|
||||
) -> ToolSpec {
|
||||
let mut properties = BTreeMap::from([
|
||||
(
|
||||
@@ -761,7 +770,9 @@ fn create_shell_command_tool(
|
||||
},
|
||||
);
|
||||
}
|
||||
properties.extend(create_approval_parameters(request_permission_enabled));
|
||||
properties.extend(create_approval_parameters(
|
||||
exec_permission_approvals_enabled,
|
||||
));
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
r#"Runs a Powershell command (Windows) and returns its output.
|
||||
@@ -2359,7 +2370,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
let js_repl_handler = Arc::new(JsReplHandler);
|
||||
let js_repl_reset_handler = Arc::new(JsReplResetHandler);
|
||||
let artifacts_handler = Arc::new(ArtifactsHandler);
|
||||
let request_permission_enabled = config.request_permission_enabled;
|
||||
let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled;
|
||||
|
||||
if config.code_mode_enabled {
|
||||
let nested_config = config.for_code_mode_nested_tools();
|
||||
@@ -2399,7 +2410,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
ConfigShellToolType::Default => {
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_shell_tool(request_permission_enabled),
|
||||
create_shell_tool(exec_permission_approvals_enabled),
|
||||
true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
@@ -2415,7 +2426,10 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
ConfigShellToolType::UnifiedExec => {
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_exec_command_tool(config.allow_login_shell, request_permission_enabled),
|
||||
create_exec_command_tool(
|
||||
config.allow_login_shell,
|
||||
exec_permission_approvals_enabled,
|
||||
),
|
||||
true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
@@ -2434,7 +2448,10 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
ConfigShellToolType::ShellCommand => {
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_shell_command_tool(config.allow_login_shell, request_permission_enabled),
|
||||
create_shell_command_tool(
|
||||
config.allow_login_shell,
|
||||
exec_permission_approvals_enabled,
|
||||
),
|
||||
true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
|
||||
@@ -461,7 +461,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
}
|
||||
|
||||
if config.request_permission_enabled {
|
||||
if config.exec_permission_approvals_enabled {
|
||||
let spec = create_request_permissions_tool();
|
||||
expected.insert(tool_name(&spec).to_string(), spec);
|
||||
}
|
||||
@@ -744,7 +744,7 @@ fn request_permissions_tool_is_independent_from_additional_permissions() {
|
||||
let config = test_config();
|
||||
let model_info = ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::RequestPermissions);
|
||||
features.enable(Feature::ExecPermissionApprovals);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
|
||||
Reference in New Issue
Block a user