From 7c7e2675010df55565b547ed101aaea60f9acfe4 Mon Sep 17 00:00:00 2001 From: Jack Mousseau Date: Thu, 12 Mar 2026 21:13:17 -0700 Subject: [PATCH] Simplify permissions available in request permissions tool (#14529) --- .../app-server/src/bespoke_event_handling.rs | 202 ++++++------------ codex-rs/core/src/codex.rs | 9 +- codex-rs/core/src/codex_delegate_tests.rs | 10 +- codex-rs/core/src/codex_tests.rs | 19 +- .../src/tools/handlers/request_permissions.rs | 5 +- codex-rs/core/src/tools/spec.rs | 63 ++---- codex-rs/core/src/tools/spec_tests.rs | 20 +- .../core/tests/suite/request_permissions.rs | 51 ++--- .../tests/suite/request_permissions_tool.rs | 18 +- .../on_request_rule_request_permission.md | 6 +- codex-rs/protocol/src/request_permissions.rs | 40 +++- codex-rs/tui/src/app.rs | 2 +- .../tui/src/bottom_pane/approval_overlay.rs | 16 +- codex-rs/tui/src/bottom_pane/mod.rs | 2 +- 14 files changed, 203 insertions(+), 260 deletions(-) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index c8255e4dd..3853a661e 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -123,6 +123,7 @@ use codex_protocol::protocol::ReviewOutputEvent; use codex_protocol::protocol::TokenCountEvent; use codex_protocol::protocol::TurnDiffEvent; use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse as CoreRequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; @@ -699,7 +700,7 @@ pub(crate) async fn apply_bespoke_event_handling( turn_id: request.turn_id.clone(), item_id: request.call_id.clone(), reason: request.reason, - permissions: request.permissions.into(), + permissions: CorePermissionProfile::from(request.permissions).into(), }; let (pending_request_id, rx) = outgoing .send_request(ServerRequestPayload::PermissionsRequestApproval(params)) @@ -2227,7 +2228,7 @@ fn mcp_server_elicitation_response_from_client_result( async fn on_request_permissions_response( call_id: String, - requested_permissions: CorePermissionProfile, + requested_permissions: CoreRequestPermissionProfile, pending_request_id: RequestId, receiver: oneshot::Receiver, conversation: Arc, @@ -2255,7 +2256,7 @@ async fn on_request_permissions_response( } fn request_permissions_response_from_client_result( - requested_permissions: CorePermissionProfile, + requested_permissions: CoreRequestPermissionProfile, response: std::result::Result, ) -> Option { let value = match response { @@ -2287,9 +2288,10 @@ fn request_permissions_response_from_client_result( }); Some(CoreRequestPermissionsResponse { permissions: intersect_permission_profiles( - requested_permissions, + requested_permissions.into(), response.permissions.into(), - ), + ) + .into(), scope: response.scope.to_core(), }) } @@ -2646,10 +2648,8 @@ mod tests { use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::TurnPlanStepStatus; use codex_protocol::mcp::CallToolResult; - use codex_protocol::models::MacOsAutomationPermission; - use codex_protocol::models::MacOsContactsPermission; - use codex_protocol::models::MacOsPreferencesPermission; - use codex_protocol::models::MacOsSeatbeltProfileExtensions; + use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; + use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions; use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::protocol::CollabResumeBeginEvent; @@ -2660,6 +2660,7 @@ mod tests { use codex_protocol::protocol::RateLimitWindow; use codex_protocol::protocol::TokenUsage; use codex_protocol::protocol::TokenUsageInfo; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use rmcp::model::Content; use serde_json::Value as JsonValue; @@ -2721,7 +2722,7 @@ mod tests { }; let response = request_permissions_response_from_client_result( - CorePermissionProfile::default(), + CoreRequestPermissionProfile::default(), Ok(Err(error)), ); @@ -2729,156 +2730,91 @@ mod tests { } #[test] - fn request_permissions_response_accepts_partial_macos_grants() { - let requested_permissions = CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::ReadWrite, - macos_automation: MacOsAutomationPermission::BundleIds(vec![ - "com.apple.Notes".to_string(), - "com.apple.Reminders".to_string(), - ]), - macos_launch_services: true, - macos_accessibility: true, - macos_calendar: true, - macos_reminders: true, - macos_contacts: MacOsContactsPermission::ReadWrite, + fn request_permissions_response_accepts_partial_network_and_file_system_grants() { + let input_path = if cfg!(target_os = "windows") { + r"C:\tmp\input" + } else { + "/tmp/input" + }; + let output_path = if cfg!(target_os = "windows") { + r"C:\tmp\output" + } else { + "/tmp/output" + }; + let ignored_path = if cfg!(target_os = "windows") { + r"C:\tmp\ignored" + } else { + "/tmp/ignored" + }; + let absolute_path = |path: &str| { + AbsolutePathBuf::try_from(std::path::PathBuf::from(path)).expect("absolute path") + }; + let requested_permissions = CoreRequestPermissionProfile { + network: Some(CoreNetworkPermissions { + enabled: Some(true), + }), + file_system: Some(CoreFileSystemPermissions { + read: Some(vec![absolute_path(input_path)]), + write: Some(vec![absolute_path(output_path)]), }), - ..Default::default() }; let cases = vec![ - (serde_json::json!({}), CorePermissionProfile::default()), ( - serde_json::json!({ - "preferences": "read_only", - }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::ReadOnly, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: false, - macos_accessibility: false, - macos_calendar: false, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::None, - }), - ..Default::default() - }, + serde_json::json!({}), + CoreRequestPermissionProfile::default(), ), ( serde_json::json!({ - "automations": { - "bundle_ids": ["com.apple.Notes"], + "network": { + "enabled": true, }, }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::BundleIds(vec![ - "com.apple.Notes".to_string(), - ]), - macos_launch_services: false, - macos_accessibility: false, - macos_calendar: false, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::None, + CoreRequestPermissionProfile { + network: Some(CoreNetworkPermissions { + enabled: Some(true), }), - ..Default::default() + ..CoreRequestPermissionProfile::default() }, ), ( serde_json::json!({ - "launchServices": true, + "fileSystem": { + "write": [output_path], + }, }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: true, - macos_accessibility: false, - macos_calendar: false, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::None, + CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions { + read: None, + write: Some(vec![absolute_path(output_path)]), }), - ..Default::default() + ..CoreRequestPermissionProfile::default() }, ), ( serde_json::json!({ - "accessibility": true, + "fileSystem": { + "read": [input_path], + "write": [output_path, ignored_path], + }, + "macos": { + "calendar": true, + }, }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: false, - macos_accessibility: true, - macos_calendar: false, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::None, + CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions { + read: Some(vec![absolute_path(input_path)]), + write: Some(vec![absolute_path(output_path)]), }), - ..Default::default() - }, - ), - ( - serde_json::json!({ - "calendar": true, - }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: false, - macos_accessibility: false, - macos_calendar: true, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::None, - }), - ..Default::default() - }, - ), - ( - serde_json::json!({ - "reminders": true, - }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: false, - macos_accessibility: false, - macos_calendar: false, - macos_reminders: true, - macos_contacts: MacOsContactsPermission::None, - }), - ..Default::default() - }, - ), - ( - serde_json::json!({ - "contacts": "read_only", - }), - CorePermissionProfile { - macos: Some(MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::None, - macos_automation: MacOsAutomationPermission::None, - macos_launch_services: false, - macos_accessibility: false, - macos_calendar: false, - macos_reminders: false, - macos_contacts: MacOsContactsPermission::ReadOnly, - }), - ..Default::default() + ..CoreRequestPermissionProfile::default() }, ), ]; - for (granted_macos, expected_permissions) in cases { + for (granted_permissions, expected_permissions) in cases { let response = request_permissions_response_from_client_result( requested_permissions.clone(), Ok(Ok(serde_json::json!({ - "permissions": { - "macos": granted_macos, - }, + "permissions": granted_permissions, }))), ) .expect("response should be accepted"); @@ -2896,7 +2832,7 @@ mod tests { #[test] fn request_permissions_response_preserves_session_scope() { let response = request_permissions_response_from_client_result( - CorePermissionProfile::default(), + CoreRequestPermissionProfile::default(), Ok(Ok(serde_json::json!({ "scope": "session", "permissions": {}, @@ -2907,7 +2843,7 @@ mod tests { assert_eq!( response, CoreRequestPermissionsResponse { - permissions: CorePermissionProfile::default(), + permissions: CoreRequestPermissionProfile::default(), scope: CorePermissionGrantScope::Session, } ); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 4e5d5be55..1c0f6e1f2 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -106,6 +106,7 @@ use codex_protocol::protocol::TurnContextNetworkItem; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::W3cTraceContext; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsArgs; use codex_protocol::request_permissions::RequestPermissionsEvent; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -2908,7 +2909,7 @@ impl Session { match turn_context.approval_policy.value() { AskForApproval::Never => { return Some(RequestPermissionsResponse { - permissions: PermissionProfile::default(), + permissions: RequestPermissionProfile::default(), scope: PermissionGrantScope::Turn, }); } @@ -2916,7 +2917,7 @@ impl Session { if !granular_config.allows_request_permissions() => { return Some(RequestPermissionsResponse { - permissions: PermissionProfile::default(), + permissions: RequestPermissionProfile::default(), scope: PermissionGrantScope::Turn, }); } @@ -3102,7 +3103,7 @@ impl Session { if entry.is_some() && !response.permissions.is_empty() { match response.scope { PermissionGrantScope::Turn => { - ts.record_granted_permissions(response.permissions.clone()); + ts.record_granted_permissions(response.permissions.clone().into()); } PermissionGrantScope::Session => { granted_for_session = Some(response.permissions.clone()); @@ -3116,7 +3117,7 @@ impl Session { }; if let Some(permissions) = granted_for_session { let mut state = self.state.lock().await; - state.record_granted_permissions(permissions); + state.record_granted_permissions(permissions.into()); } match entry { Some(tx_response) => { diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 3d4c49c9d..e2b7657e8 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -1,13 +1,13 @@ use super::*; use async_channel::bounded; use codex_protocol::models::NetworkPermissions; -use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentStatus; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RawResponseItemEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnAbortedEvent; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsEvent; use codex_protocol::request_permissions::RequestPermissionsResponse; use pretty_assertions::assert_eq; @@ -150,11 +150,11 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { let call_id = "tool-call-1".to_string(); let expected_response = RequestPermissionsResponse { - permissions: PermissionProfile { + permissions: RequestPermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), }), - ..PermissionProfile::default() + ..RequestPermissionProfile::default() }, scope: PermissionGrantScope::Turn, }; @@ -175,11 +175,11 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { call_id: request_call_id, turn_id: "child-turn-1".to_string(), reason: Some("need access".to_string()), - permissions: PermissionProfile { + permissions: RequestPermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), }), - ..PermissionProfile::default() + ..RequestPermissionProfile::default() }, }, &cancel_token, diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 5e397d69a..0d3c69bb7 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -25,6 +25,7 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use tracing::Span; use crate::protocol::CompactedItem; @@ -2216,11 +2217,11 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() { .notify_request_permissions_response( "missing", codex_protocol::request_permissions::RequestPermissionsResponse { - permissions: codex_protocol::models::PermissionProfile { + permissions: RequestPermissionProfile { network: Some(codex_protocol::models::NetworkPermissions { enabled: Some(true), }), - ..Default::default() + ..RequestPermissionProfile::default() }, scope: PermissionGrantScope::Turn, }, @@ -2252,11 +2253,11 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() let turn_context = Arc::new(turn_context); let call_id = "call-1".to_string(); let expected_response = codex_protocol::request_permissions::RequestPermissionsResponse { - permissions: codex_protocol::models::PermissionProfile { + permissions: RequestPermissionProfile { network: Some(codex_protocol::models::NetworkPermissions { enabled: Some(true), }), - ..Default::default() + ..RequestPermissionProfile::default() }, scope: PermissionGrantScope::Turn, }; @@ -2272,11 +2273,11 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() call_id, codex_protocol::request_permissions::RequestPermissionsArgs { reason: Some("need network".to_string()), - permissions: codex_protocol::models::PermissionProfile { + permissions: RequestPermissionProfile { network: Some(codex_protocol::models::NetworkPermissions { enabled: Some(true), }), - ..Default::default() + ..RequestPermissionProfile::default() }, }, ) @@ -2332,11 +2333,11 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req call_id, codex_protocol::request_permissions::RequestPermissionsArgs { reason: Some("need network".to_string()), - permissions: codex_protocol::models::PermissionProfile { + permissions: RequestPermissionProfile { network: Some(codex_protocol::models::NetworkPermissions { enabled: Some(true), }), - ..Default::default() + ..RequestPermissionProfile::default() }, }, ) @@ -2346,7 +2347,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req response, Some( codex_protocol::request_permissions::RequestPermissionsResponse { - permissions: codex_protocol::models::PermissionProfile::default(), + permissions: RequestPermissionProfile::default(), scope: PermissionGrantScope::Turn, } ) diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index f14a8bd71..2ba41ed9a 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -11,7 +11,7 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; pub(crate) fn request_permissions_tool_description() -> String { - "Request additional permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope." + "Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope." .to_string() } @@ -45,7 +45,8 @@ impl ToolHandler for RequestPermissionsHandler { let mut args: RequestPermissionsArgs = parse_arguments_with_base_path(&arguments, turn.cwd.as_path())?; - args.permissions = normalize_additional_permissions(args.permissions) + args.permissions = normalize_additional_permissions(args.permissions.into()) + .map(codex_protocol::request_permissions::RequestPermissionProfile::from) .map_err(FunctionCallError::RespondToModel)?; if args.permissions.is_empty() { return Err(FunctionCallError::RespondToModel( diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 6e679bbfd..a931940fe 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -494,44 +494,21 @@ fn create_file_system_permissions_schema() -> JsonSchema { } } -fn create_macos_permissions_schema() -> JsonSchema { - JsonSchema::Object { - properties: BTreeMap::from([ - ( - "preferences".to_string(), - JsonSchema::String { - description: Some( - "macOS preferences access. Supported values: `none`, `read_only`, or `read_write`." - .to_string(), - ), - }, - ), - ( - "automations".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("macOS automation access as app bundle identifiers.".to_string()), - }, - ), - ( - "accessibility".to_string(), - JsonSchema::Boolean { - description: Some("Whether to request macOS accessibility access.".to_string()), - }, - ), - ( - "calendar".to_string(), - JsonSchema::Boolean { - description: Some("Whether to request macOS calendar access.".to_string()), - }, - ), - ]), - required: None, - additional_properties: Some(false.into()), - } -} - -fn create_permissions_schema() -> JsonSchema { +fn create_additional_permissions_schema() -> JsonSchema { + JsonSchema::Object { + properties: BTreeMap::from([ + ("network".to_string(), create_network_permissions_schema()), + ( + "file_system".to_string(), + create_file_system_permissions_schema(), + ), + ]), + required: None, + additional_properties: Some(false.into()), + } +} + +fn create_request_permissions_schema() -> JsonSchema { JsonSchema::Object { properties: BTreeMap::from([ ("network".to_string(), create_network_permissions_schema()), @@ -539,7 +516,6 @@ fn create_permissions_schema() -> JsonSchema { "file_system".to_string(), create_file_system_permissions_schema(), ), - ("macos".to_string(), create_macos_permissions_schema()), ]), required: None, additional_properties: Some(false.into()), @@ -555,7 +531,7 @@ fn create_approval_parameters( JsonSchema::String { description: Some( 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\"." + "Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem or network 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\"." } @@ -592,7 +568,7 @@ fn create_approval_parameters( if exec_permission_approvals_enabled { properties.insert( "additional_permissions".to_string(), - create_permissions_schema(), + create_additional_permissions_schema(), ); } @@ -1455,7 +1431,10 @@ fn create_request_permissions_tool() -> ToolSpec { ), }, ); - properties.insert("permissions".to_string(), create_permissions_schema()); + properties.insert( + "permissions".to_string(), + create_request_permissions_schema(), + ); ToolSpec::Function(ResponsesApiTool { name: "request_permissions".to_string(), diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 30147ffc9..e56abbd29 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -2205,7 +2205,7 @@ fn shell_tool_with_request_permission_includes_additional_permissions() { panic!("expected sandbox_permissions description"); }; assert!(description.contains("with_additional_permissions")); - assert!(description.contains("macOS permissions")); + assert!(description.contains("filesystem or network permissions")); let Some(JsonSchema::Object { properties: additional_properties, @@ -2216,7 +2216,7 @@ fn shell_tool_with_request_permission_includes_additional_permissions() { }; assert!(additional_properties.contains_key("network")); assert!(additional_properties.contains_key("file_system")); - assert!(additional_properties.contains_key("macos")); + assert!(!additional_properties.contains_key("macos")); } #[test] @@ -2240,7 +2240,7 @@ fn request_permissions_tool_includes_full_permission_schema() { assert_eq!(additional_properties, &Some(false.into())); assert!(permission_properties.contains_key("network")); assert!(permission_properties.contains_key("file_system")); - assert!(permission_properties.contains_key("macos")); + assert!(!permission_properties.contains_key("macos")); let Some(JsonSchema::Object { properties: network_properties, @@ -2264,20 +2264,6 @@ fn request_permissions_tool_includes_full_permission_schema() { assert_eq!(additional_properties, &Some(false.into())); assert!(file_system_properties.contains_key("read")); assert!(file_system_properties.contains_key("write")); - - let Some(JsonSchema::Object { - properties: macos_properties, - additional_properties, - .. - }) = permission_properties.get("macos") - else { - panic!("expected macos object"); - }; - assert_eq!(additional_properties, &Some(false.into())); - assert!(macos_properties.contains_key("preferences")); - assert!(macos_properties.contains_key("automations")); - assert!(macos_properties.contains_key("accessibility")); - assert!(macos_properties.contains_key("calendar")); } #[test] diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 2e5c6c264..9373a938a 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -14,6 +14,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -85,10 +86,10 @@ fn parse_result(item: &Value) -> CommandResult { } } -fn shell_event_with_request_permissions( +fn shell_event_with_request_permissions( call_id: &str, command: &str, - additional_permissions: &PermissionProfile, + additional_permissions: &S, ) -> Result { let args = json!({ "command": command, @@ -103,7 +104,7 @@ fn shell_event_with_request_permissions( fn request_permissions_tool_event( call_id: &str, reason: &str, - permissions: &PermissionProfile, + permissions: &RequestPermissionProfile, ) -> Result { let args = json!({ "reason": reason, @@ -131,10 +132,10 @@ fn exec_command_event(call_id: &str, command: &str) -> Result { Ok(ev_function_call(call_id, "exec_command", &args_str)) } -fn exec_command_event_with_request_permissions( +fn exec_command_event_with_request_permissions( call_id: &str, command: &str, - additional_permissions: &PermissionProfile, + additional_permissions: &S, ) -> Result { let args = json!({ "cmd": command, @@ -259,7 +260,7 @@ async fn wait_for_exec_approval_or_completion( async fn expect_request_permissions_event( test: &TestCodex, expected_call_id: &str, -) -> PermissionProfile { +) -> RequestPermissionProfile { let event = wait_for_event(&test.codex, |event| { matches!( event, @@ -288,23 +289,23 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { } } -fn requested_directory_write_permissions(path: &Path) -> PermissionProfile { - PermissionProfile { +fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { + RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![absolute_path(path)]), }), - ..Default::default() + ..RequestPermissionProfile::default() } } -fn normalized_directory_write_permissions(path: &Path) -> Result { - Ok(PermissionProfile { +fn normalized_directory_write_permissions(path: &Path) -> Result { + Ok(RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), }), - ..Default::default() + ..RequestPermissionProfile::default() }) } @@ -477,7 +478,7 @@ async fn request_permissions_tool_is_auto_denied_when_granular_request_permissio assert_eq!( result, RequestPermissionsResponse { - permissions: PermissionProfile::default(), + permissions: RequestPermissionProfile::default(), scope: PermissionGrantScope::Turn, } ); @@ -820,21 +821,21 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "printf {:?} > {:?} && cat {:?}", "outside-cwd-ok", outside_write, outside_write ); - let requested_permissions = PermissionProfile { + let requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![absolute_path(outside_dir.path())]), }), - ..Default::default() + ..RequestPermissionProfile::default() }; - let normalized_requested_permissions = PermissionProfile { + let normalized_requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), }), - ..Default::default() + ..RequestPermissionProfile::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -861,7 +862,7 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> let approval = expect_exec_approval(&test, &command).await; assert_eq!( approval.additional_permissions, - Some(normalized_requested_permissions) + Some(normalized_requested_permissions.into()) ); test.codex .submit(Op::ExecApproval { @@ -1024,14 +1025,14 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul "printf {:?} > {:?} && cat {:?}", "sticky-grant-ok", outside_write, outside_write ); - let requested_permissions = PermissionProfile { + let requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![absolute_path(outside_dir.path())]), }), ..Default::default() }; - let normalized_requested_permissions = PermissionProfile { + let normalized_requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![AbsolutePathBuf::try_from( @@ -1092,7 +1093,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul if let Some(approval) = wait_for_exec_approval_or_completion(&test).await { assert_eq!( approval.additional_permissions, - Some(normalized_requested_permissions.clone()) + Some(normalized_requested_permissions.clone().into()) ); test.codex .submit(Op::ExecApproval { @@ -1488,7 +1489,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() "partial-grant-ok", second_write, second_write ); - let requested_permissions = PermissionProfile { + let requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![ @@ -1496,9 +1497,9 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() absolute_path(second_dir.path()), ]), }), - ..Default::default() + ..RequestPermissionProfile::default() }; - let normalized_requested_permissions = PermissionProfile { + let normalized_requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![ @@ -1506,7 +1507,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), }), - ..Default::default() + ..RequestPermissionProfile::default() }; let granted_permissions = normalized_directory_write_permissions(first_dir.path())?; let second_dir_permissions = requested_directory_write_permissions(second_dir.path()); diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index 9aff62d94..8a092f69f 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -5,13 +5,13 @@ use anyhow::Result; use codex_core::config::Constrained; use codex_core::features::Feature; use codex_protocol::models::FileSystemPermissions; -use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -42,7 +42,7 @@ fn absolute_path(path: &Path) -> AbsolutePathBuf { fn request_permissions_tool_event( call_id: &str, reason: &str, - permissions: &PermissionProfile, + permissions: &RequestPermissionProfile, ) -> Result { let args = json!({ "reason": reason, @@ -79,23 +79,23 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { } } -fn requested_directory_write_permissions(path: &Path) -> PermissionProfile { - PermissionProfile { +fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { + RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![absolute_path(path)]), }), - ..Default::default() + ..RequestPermissionProfile::default() } } -fn normalized_directory_write_permissions(path: &Path) -> Result { - Ok(PermissionProfile { +fn normalized_directory_write_permissions(path: &Path) -> Result { + Ok(RequestPermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), }), - ..Default::default() + ..RequestPermissionProfile::default() }) } @@ -160,7 +160,7 @@ async fn submit_turn( async fn expect_request_permissions_event( test: &TestCodex, expected_call_id: &str, -) -> PermissionProfile { +) -> RequestPermissionProfile { let event = wait_for_event(&test.codex, |event| { matches!( event, diff --git a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md index 68a342bf3..16d4101cc 100644 --- a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md +++ b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md @@ -11,10 +11,8 @@ When you need extra sandboxed permissions for one command, use: - `network.enabled`: set to `true` to enable network access - `file_system.read`: list of paths that need read access - `file_system.write`: list of paths that need write access - - `macos.preferences`: `readonly` or `readwrite` - - `macos.automations`: list of bundle IDs that need Apple Events access - - `macos.accessibility`: set to `true` to allow accessibility APIs - - `macos.calendar`: set to `true` to allow Calendar access + +When using the `request_permissions` tool directly, only request `network` and `file_system` permissions. This keeps execution inside the current sandbox policy, while adding only the requested permissions for that command, unless an exec-policy allow rule applies and authorizes running the command outside the sandbox. diff --git a/codex-rs/protocol/src/request_permissions.rs b/codex-rs/protocol/src/request_permissions.rs index fc66e9b3d..db5396c5e 100644 --- a/codex-rs/protocol/src/request_permissions.rs +++ b/codex-rs/protocol/src/request_permissions.rs @@ -1,3 +1,5 @@ +use crate::models::FileSystemPermissions; +use crate::models::NetworkPermissions; use crate::models::PermissionProfile; use schemars::JsonSchema; use serde::Deserialize; @@ -12,16 +14,48 @@ pub enum PermissionGrantScope { Session, } +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(deny_unknown_fields)] +pub struct RequestPermissionProfile { + pub network: Option, + pub file_system: Option, +} + +impl RequestPermissionProfile { + pub fn is_empty(&self) -> bool { + self.network.is_none() && self.file_system.is_none() + } +} + +impl From for PermissionProfile { + fn from(value: RequestPermissionProfile) -> Self { + Self { + network: value.network, + file_system: value.file_system, + macos: None, + } + } +} + +impl From for RequestPermissionProfile { + fn from(value: PermissionProfile) -> Self { + Self { + network: value.network, + file_system: value.file_system, + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] pub struct RequestPermissionsArgs { #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, - pub permissions: PermissionProfile, + pub permissions: RequestPermissionProfile, } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] pub struct RequestPermissionsResponse { - pub permissions: PermissionProfile, + pub permissions: RequestPermissionProfile, #[serde(default)] pub scope: PermissionGrantScope, } @@ -36,5 +70,5 @@ pub struct RequestPermissionsEvent { pub turn_id: String, #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, - pub permissions: PermissionProfile, + pub permissions: RequestPermissionProfile, } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 87b2dc795..a244965d5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3306,7 +3306,7 @@ impl App { lines.push(Line::from("")); } if let Some(rule_line) = - crate::bottom_pane::format_additional_permissions_rule(&permissions) + crate::bottom_pane::format_requested_permissions_rule(&permissions) { lines.push(Line::from(vec![ "Permission rule: ".into(), diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 2420fb323..a0ddbc350 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -30,6 +30,7 @@ use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -60,7 +61,7 @@ pub(crate) enum ApprovalRequest { thread_label: Option, call_id: String, reason: Option, - permissions: PermissionProfile, + permissions: RequestPermissionProfile, }, ApplyPatch { thread_id: ThreadId, @@ -272,7 +273,7 @@ impl ApprovalOverlay { fn handle_permissions_decision( &self, call_id: &str, - permissions: &PermissionProfile, + permissions: &RequestPermissionProfile, decision: ReviewDecision, ) { let Some(request) = self.current_request.as_ref() else { @@ -567,7 +568,7 @@ fn build_header(request: &ApprovalRequest) -> Box { header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()])); header.push(Line::from("")); } - if let Some(rule_line) = format_additional_permissions_rule(permissions) { + if let Some(rule_line) = format_requested_permissions_rule(permissions) { header.push(Line::from(vec![ "Permission rule: ".into(), rule_line.cyan(), @@ -821,6 +822,12 @@ pub(crate) fn format_additional_permissions_rule( } } +pub(crate) fn format_requested_permissions_rule( + permissions: &RequestPermissionProfile, +) -> Option { + format_additional_permissions_rule(&permissions.clone().into()) +} + fn patch_options() -> Vec { vec![ ApprovalOption { @@ -957,7 +964,7 @@ mod tests { thread_label: None, call_id: "test".to_string(), reason: Some("need workspace access".to_string()), - permissions: PermissionProfile { + permissions: RequestPermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), }), @@ -965,7 +972,6 @@ mod tests { read: Some(vec![absolute_path("/tmp/readme.txt")]), write: Some(vec![absolute_path("/tmp/out.txt")]), }), - ..Default::default() }, } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 2d6600241..cee2fc086 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -53,7 +53,7 @@ pub(crate) use app_link_view::AppLinkView; pub(crate) use app_link_view::AppLinkViewParams; pub(crate) use approval_overlay::ApprovalOverlay; pub(crate) use approval_overlay::ApprovalRequest; -pub(crate) use approval_overlay::format_additional_permissions_rule; +pub(crate) use approval_overlay::format_requested_permissions_rule; pub(crate) use mcp_server_elicitation::McpServerElicitationFormRequest; pub(crate) use mcp_server_elicitation::McpServerElicitationOverlay; pub(crate) use request_user_input::RequestUserInputOverlay;