From d241dc598cb0bbadeefd5eab92c056a36b420624 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Mon, 9 Mar 2026 14:36:38 -0700 Subject: [PATCH] feat(core) Persist request_permission data across turns (#14009) ## Summary request_permissions flows should support persisting results for the session. Open Question: Still deciding if we need within-turn approvals - this adds complexity but I could see it being useful ## Testing - [x] Updated unit tests --------- Co-authored-by: Codex --- .../PermissionsRequestApprovalResponse.json | 15 + .../codex_app_server_protocol.schemas.json | 15 + .../typescript/v2/PermissionGrantScope.ts | 5 + .../v2/PermissionsRequestApprovalResponse.ts | 3 +- .../schema/typescript/v2/index.ts | 1 + .../app-server-protocol/src/protocol/v2.rs | 34 +- codex-rs/app-server/README.md | 3 +- .../app-server/src/bespoke_event_handling.rs | 27 ++ .../tests/suite/v2/request_permissions.rs | 2 + codex-rs/core/src/codex.rs | 20 +- codex-rs/core/src/codex_delegate.rs | 4 + codex-rs/core/src/codex_tests.rs | 2 + codex-rs/core/src/state/session.rs | 14 + codex-rs/core/src/tools/handlers/mod.rs | 7 +- .../src/tools/handlers/request_permissions.rs | 2 +- codex-rs/core/tests/suite/mod.rs | 2 + .../core/tests/suite/request_permissions.rs | 141 +++++++++ .../tests/suite/request_permissions_tool.rs | 293 ++++++++++++++++++ codex-rs/protocol/src/request_permissions.rs | 10 + .../tui/src/bottom_pane/approval_overlay.rs | 43 +++ ...__approval_overlay_permissions_prompt.snap | 3 +- 21 files changed, 638 insertions(+), 8 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/PermissionGrantScope.ts create mode 100644 codex-rs/core/tests/suite/request_permissions_tool.rs diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json index aca89004b..2637ed5dd 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json @@ -145,11 +145,26 @@ "read_write" ], "type": "string" + }, + "PermissionGrantScope": { + "enum": [ + "turn", + "session" + ], + "type": "string" } }, "properties": { "permissions": { "$ref": "#/definitions/GrantedPermissionProfile" + }, + "scope": { + "allOf": [ + { + "$ref": "#/definitions/PermissionGrantScope" + } + ], + "default": "turn" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 93b864561..fcc9cb1e9 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -6462,6 +6462,13 @@ } ] }, + "PermissionGrantScope": { + "enum": [ + "turn", + "session" + ], + "type": "string" + }, "PermissionProfile": { "properties": { "file_system": { @@ -6533,6 +6540,14 @@ "properties": { "permissions": { "$ref": "#/definitions/GrantedPermissionProfile" + }, + "scope": { + "allOf": [ + { + "$ref": "#/definitions/PermissionGrantScope" + } + ], + "default": "turn" } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionGrantScope.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionGrantScope.ts new file mode 100644 index 000000000..8ca127ebc --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionGrantScope.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type PermissionGrantScope = "turn" | "session"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalResponse.ts index b22cf3897..6561417b4 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalResponse.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalResponse.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { GrantedPermissionProfile } from "./GrantedPermissionProfile"; +import type { PermissionGrantScope } from "./PermissionGrantScope"; -export type PermissionsRequestApprovalResponse = { permissions: GrantedPermissionProfile, }; +export type PermissionsRequestApprovalResponse = { permissions: GrantedPermissionProfile, scope: PermissionGrantScope, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 37e54471b..50689c33c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -161,6 +161,7 @@ export type { NetworkRequirements } from "./NetworkRequirements"; export type { OverriddenMetadata } from "./OverriddenMetadata"; export type { PatchApplyStatus } from "./PatchApplyStatus"; export type { PatchChangeKind } from "./PatchChangeKind"; +export type { PermissionGrantScope } from "./PermissionGrantScope"; export type { PermissionsRequestApprovalParams } from "./PermissionsRequestApprovalParams"; export type { PermissionsRequestApprovalResponse } from "./PermissionsRequestApprovalResponse"; export type { PlanDeltaNotification } from "./PlanDeltaNotification"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 1ec397f18..756cad2e6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -68,6 +68,7 @@ use codex_protocol::protocol::SkillToolDependency as CoreSkillToolDependency; use codex_protocol::protocol::SubAgentSource as CoreSubAgentSource; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo; +use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope; use codex_protocol::user_input::ByteRange as CoreByteRange; use codex_protocol::user_input::TextElement as CoreTextElement; use codex_protocol::user_input::UserInput as CoreUserInput; @@ -83,12 +84,18 @@ use ts_rs::TS; // tends to use either snake_case or kebab-case. macro_rules! v2_enum_from_core { ( - pub enum $Name:ident from $Src:path { $( $Variant:ident ),+ $(,)? } + $(#[$enum_meta:meta])* + pub enum $Name:ident from $Src:path { + $( $(#[$variant_meta:meta])* $Variant:ident ),+ $(,)? + } ) => { #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] + $(#[$enum_meta])* #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] - pub enum $Name { $( $Variant ),+ } + pub enum $Name { + $( $(#[$variant_meta])* $Variant ),+ + } impl $Name { pub fn to_core(self) -> $Src { @@ -4983,11 +4990,22 @@ pub struct PermissionsRequestApprovalParams { pub permissions: AdditionalPermissionProfile, } +v2_enum_from_core!( + #[derive(Default)] + pub enum PermissionGrantScope from CorePermissionGrantScope { + #[default] + Turn, + Session + } +); + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct PermissionsRequestApprovalResponse { pub permissions: GrantedPermissionProfile, + #[serde(default)] + pub scope: PermissionGrantScope, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -5449,6 +5467,7 @@ mod tests { }), ..Default::default() }, + scope: PermissionGrantScope::Turn, }; assert_eq!( @@ -5459,10 +5478,21 @@ mod tests { "accessibility": true, }, }, + "scope": "turn", }) ); } + #[test] + fn permissions_request_approval_response_defaults_scope_to_turn() { + let response = serde_json::from_value::(json!({ + "permissions": {}, + })) + .expect("response should deserialize"); + + assert_eq!(response.scope, PermissionGrantScope::Turn); + } + #[test] fn command_exec_params_default_optional_streaming_flags() { let params = serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6913649e3..3452a8be2 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -901,12 +901,13 @@ The built-in `request_permissions` tool sends an `item/permissions/requestApprov } ``` -The client responds with `result.permissions`, which should be the granted subset of the requested permission profile: +The client responds with `result.permissions`, which should be the granted subset of the requested permission profile. It may also set `result.scope` to `"session"` to make the grant persist for later turns in the same session; omitted or `"turn"` keeps the existing turn-scoped behavior: ```json { "id": 61, "result": { + "scope": "session", "permissions": { "fileSystem": { "write": [ diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 38b03006b..5e34baef4 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -120,6 +120,7 @@ use codex_protocol::protocol::ReviewDecision; 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::RequestPermissionsResponse as CoreRequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; @@ -718,6 +719,7 @@ pub(crate) async fn apply_bespoke_event_handling( ); let empty = CoreRequestPermissionsResponse { permissions: Default::default(), + scope: CorePermissionGrantScope::Turn, }; if let Err(err) = conversation .submit(Op::RequestPermissionsResponse { @@ -2219,12 +2221,14 @@ fn request_permissions_response_from_client_result( error!("request failed with client error: {err:?}"); return Some(CoreRequestPermissionsResponse { permissions: Default::default(), + scope: CorePermissionGrantScope::Turn, }); } Err(err) => { error!("request failed: {err:?}"); return Some(CoreRequestPermissionsResponse { permissions: Default::default(), + scope: CorePermissionGrantScope::Turn, }); } }; @@ -2234,6 +2238,7 @@ fn request_permissions_response_from_client_result( error!("failed to deserialize PermissionsRequestApprovalResponse: {err}"); PermissionsRequestApprovalResponse { permissions: V2GrantedPermissionProfile::default(), + scope: codex_app_server_protocol::PermissionGrantScope::Turn, } }); Some(CoreRequestPermissionsResponse { @@ -2241,6 +2246,7 @@ fn request_permissions_response_from_client_result( requested_permissions, response.permissions.into(), ), + scope: response.scope.to_core(), }) } @@ -2766,11 +2772,32 @@ mod tests { response, CoreRequestPermissionsResponse { permissions: expected_permissions, + scope: CorePermissionGrantScope::Turn, } ); } } + #[test] + fn request_permissions_response_preserves_session_scope() { + let response = request_permissions_response_from_client_result( + CorePermissionProfile::default(), + Ok(Ok(serde_json::json!({ + "scope": "session", + "permissions": {}, + }))), + ) + .expect("response should be accepted"); + + assert_eq!( + response, + CoreRequestPermissionsResponse { + permissions: CorePermissionProfile::default(), + scope: CorePermissionGrantScope::Session, + } + ); + } + #[test] fn collab_resume_begin_maps_to_item_started_resume_agent() { let event = CollabResumeBeginEvent { diff --git a/codex-rs/app-server/tests/suite/v2/request_permissions.rs b/codex-rs/app-server/tests/suite/v2/request_permissions.rs index 94a7c7ecf..561a0fc74 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -6,6 +6,7 @@ use app_test_support::create_request_permissions_sse_response; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::PermissionGrantScope; use codex_app_server_protocol::PermissionsRequestApprovalResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ServerRequest; @@ -95,6 +96,7 @@ async fn request_permissions_round_trip() -> Result<()> { }), macos: None, }, + scope: PermissionGrantScope::Turn, })?, ) .await?; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7b6bf44c1..460eeaa56 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -103,6 +103,7 @@ use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnContextNetworkItem; use codex_protocol::protocol::TurnStartedEvent; +use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionsArgs; use codex_protocol::request_permissions::RequestPermissionsEvent; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -2996,6 +2997,7 @@ impl Session { call_id: &str, response: RequestPermissionsResponse, ) { + let mut granted_for_session = None; let entry = { let mut active = self.active_turn.lock().await; match active.as_mut() { @@ -3003,13 +3005,24 @@ impl Session { let mut ts = at.turn_state.lock().await; let entry = ts.remove_pending_request_permissions(call_id); if entry.is_some() && !response.permissions.is_empty() { - ts.record_granted_permissions(response.permissions.clone()); + match response.scope { + PermissionGrantScope::Turn => { + ts.record_granted_permissions(response.permissions.clone()); + } + PermissionGrantScope::Session => { + granted_for_session = Some(response.permissions.clone()); + } + } } entry } None => None, } }; + if let Some(permissions) = granted_for_session { + let mut state = self.state.lock().await; + state.record_granted_permissions(permissions); + } match entry { Some(tx_response) => { tx_response.send(response).ok(); @@ -3027,6 +3040,11 @@ impl Session { ts.granted_permissions() } + pub(crate) async fn granted_session_permissions(&self) -> Option { + let state = self.state.lock().await; + state.granted_permissions() + } + pub async fn notify_dynamic_tool_response(&self, call_id: &str, response: DynamicToolResponse) { let entry = { let mut active = self.active_turn.lock().await; diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 1595830a5..f353febd0 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -13,6 +13,7 @@ use codex_protocol::protocol::RequestUserInputEvent; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::Submission; +use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionsArgs; use codex_protocol::request_permissions::RequestPermissionsEvent; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -505,6 +506,7 @@ where _ = cancel_token.cancelled() => { let empty = RequestPermissionsResponse { permissions: Default::default(), + scope: PermissionGrantScope::Turn, }; parent_session .notify_request_permissions_response(call_id, empty.clone()) @@ -513,6 +515,7 @@ where } response = fut => response.unwrap_or_else(|| RequestPermissionsResponse { permissions: Default::default(), + scope: PermissionGrantScope::Turn, }), } } @@ -698,6 +701,7 @@ mod tests { }), ..PermissionProfile::default() }, + scope: PermissionGrantScope::Turn, }; let cancel_token = CancellationToken::new(); let request_call_id = call_id.clone(); diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index e31176733..d045d6753 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -17,6 +17,7 @@ use crate::tools::format_exec_output_str; use codex_protocol::ThreadId; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::request_permissions::PermissionGrantScope; use tracing::Span; use crate::protocol::CompactedItem; @@ -2156,6 +2157,7 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() { }), ..Default::default() }, + scope: PermissionGrantScope::Turn, }, ) .await; diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index 576db7cf3..cc4e12804 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -1,5 +1,6 @@ //! Session-wide mutable state. +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use std::collections::HashMap; use std::collections::HashSet; @@ -32,6 +33,7 @@ pub(crate) struct SessionState { pub(crate) startup_regular_task: Option>>, pub(crate) active_mcp_tool_selection: Option>, pub(crate) active_connector_selection: HashSet, + granted_permissions: Option, } impl SessionState { @@ -49,6 +51,7 @@ impl SessionState { startup_regular_task: None, active_mcp_tool_selection: None, active_connector_selection: HashSet::new(), + granted_permissions: None, } } @@ -218,6 +221,17 @@ impl SessionState { self.active_mcp_tool_selection = None; } + pub(crate) fn record_granted_permissions(&mut self, permissions: PermissionProfile) { + self.granted_permissions = crate::sandboxing::merge_permission_profiles( + self.granted_permissions.as_ref(), + Some(&permissions), + ); + } + + pub(crate) fn granted_permissions(&self) -> Option { + self.granted_permissions.clone() + } + // Adds connector IDs to the active set and returns the merged selection. pub(crate) fn merge_connector_selection(&mut self, connector_ids: I) -> HashSet where diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index a1a2a162e..c1452e3ff 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -172,7 +172,12 @@ pub(super) async fn apply_granted_turn_permissions( }; } - let granted_permissions = session.granted_turn_permissions().await; + let granted_session_permissions = session.granted_session_permissions().await; + let granted_turn_permissions = session.granted_turn_permissions().await; + let granted_permissions = merge_permission_profiles( + granted_session_permissions.as_ref(), + granted_turn_permissions.as_ref(), + ); let effective_permissions = merge_permission_profiles( additional_permissions.as_ref(), granted_permissions.as_ref(), diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index be9058e09..9e4235abf 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -12,7 +12,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." + "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." .to_string() } diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index e9867caa7..61dfbe03e 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -104,6 +104,8 @@ mod remote_models; mod request_compression; #[cfg(not(target_os = "windows"))] mod request_permissions; +#[cfg(not(target_os = "windows"))] +mod request_permissions_tool; mod request_user_input; mod resume; mod resume_warning; diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 01dea4217..bbe4d12a7 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -12,6 +12,7 @@ use codex_protocol::protocol::ExecApprovalRequestEvent; 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::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -993,6 +994,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul id: "permissions-call".to_string(), response: RequestPermissionsResponse { permissions: normalized_requested_permissions.clone(), + scope: PermissionGrantScope::Turn, }, }) .await?; @@ -1106,6 +1108,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req id: "permissions-call".to_string(), response: RequestPermissionsResponse { permissions: normalized_requested_permissions, + scope: PermissionGrantScope::Turn, }, }) .await?; @@ -1218,6 +1221,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu id: "permissions-call".to_string(), response: RequestPermissionsResponse { permissions: normalized_requested_permissions.clone(), + scope: PermissionGrantScope::Turn, }, }) .await?; @@ -1360,6 +1364,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() id: "permissions-call".to_string(), response: RequestPermissionsResponse { permissions: granted_permissions.clone(), + scope: PermissionGrantScope::Turn, }, }) .await?; @@ -1477,6 +1482,7 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { id: "permissions-call".to_string(), response: RequestPermissionsResponse { permissions: normalized_requested_permissions, + scope: PermissionGrantScope::Turn, }, }) .await?; @@ -1518,3 +1524,138 @@ async fn request_permissions_grants_do_not_carry_across_turns() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "current_thread")] +#[cfg(target_os = "macos")] +async fn request_permissions_session_grants_carry_across_turns() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::OnRequest; + let sandbox_policy = workspace_write_excluding_tmp(); + let sandbox_policy_for_config = sandbox_policy.clone(); + + let mut builder = test_codex().with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config + .features + .enable(Feature::RequestPermissions) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::RequestPermissionsTool) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + let outside_dir = tempfile::tempdir()?; + let outside_write = outside_dir.path().join("session-sticky-write.txt"); + let requested_permissions = requested_directory_write_permissions(outside_dir.path()); + let normalized_requested_permissions = + normalized_directory_write_permissions(outside_dir.path())?; + let command = format!( + "printf {:?} > {:?} && cat {:?}", + "session-sticky-ok", outside_write, outside_write + ); + + let _first_turn = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-session-turn-1"), + request_permissions_tool_event( + "permissions-call", + "Allow writing outside the workspace", + &requested_permissions, + )?, + ev_completed("resp-session-turn-1"), + ]), + sse(vec![ + ev_response_created("resp-session-turn-2"), + ev_assistant_message("msg-session-turn-1", "done"), + ev_completed("resp-session-turn-2"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "request session permissions for later use", + approval_policy, + sandbox_policy.clone(), + ) + .await?; + + let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await; + assert_eq!( + granted_permissions, + normalized_requested_permissions.clone() + ); + test.codex + .submit(Op::RequestPermissionsResponse { + id: "permissions-call".to_string(), + response: RequestPermissionsResponse { + permissions: normalized_requested_permissions, + scope: PermissionGrantScope::Session, + }, + }) + .await?; + wait_for_completion(&test).await; + + let second_turn = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-session-turn-3"), + exec_command_event("exec-call", &command)?, + ev_completed("resp-session-turn-3"), + ]), + sse(vec![ + ev_response_created("resp-session-turn-4"), + ev_assistant_message("msg-session-turn-2", "done"), + ev_completed("resp-session-turn-4"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "reuse session permissions in a later turn", + approval_policy, + sandbox_policy, + ) + .await?; + + let completion_event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + if let EventMsg::ExecApprovalRequest(approval) = completion_event { + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_completion(&test).await; + } + + let exec_output = second_turn + .function_call_output_text("exec-call") + .map(|output| json!({ "output": output })) + .unwrap_or_else(|| panic!("expected exec-call output")); + let result = parse_result(&exec_output); + assert_eq!(result.exit_code, Some(0)); + assert_eq!(result.stdout.trim(), "session-sticky-ok"); + assert_eq!(fs::read_to_string(&outside_write)?, "session-sticky-ok"); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs new file mode 100644 index 000000000..e6c3c4487 --- /dev/null +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -0,0 +1,293 @@ +#![allow(clippy::unwrap_used, clippy::expect_used)] +#![cfg(target_os = "macos")] + +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::RequestPermissionsResponse; +use codex_protocol::user_input::UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::skip_if_sandbox; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use regex_lite::Regex; +use serde_json::Value; +use serde_json::json; +use std::fs; +use std::path::Path; + +fn absolute_path(path: &Path) -> AbsolutePathBuf { + AbsolutePathBuf::try_from(path).expect("absolute path") +} + +fn request_permissions_tool_event( + call_id: &str, + reason: &str, + permissions: &PermissionProfile, +) -> Result { + let args = json!({ + "reason": reason, + "permissions": permissions, + }); + let args_str = serde_json::to_string(&args)?; + Ok(ev_function_call(call_id, "request_permissions", &args_str)) +} + +fn exec_command_event(call_id: &str, command: &str) -> Result { + let args = json!({ + "cmd": command, + "yield_time_ms": 1_000_u64, + }); + let args_str = serde_json::to_string(&args)?; + Ok(ev_function_call(call_id, "exec_command", &args_str)) +} + +fn workspace_write_excluding_tmp() -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + } +} + +fn requested_directory_write_permissions(path: &Path) -> PermissionProfile { + PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![absolute_path(path)]), + }), + ..Default::default() + } +} + +fn normalized_directory_write_permissions(path: &Path) -> Result { + Ok(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + }), + ..Default::default() + }) +} + +fn parse_result(item: &Value) -> (Option, String) { + let output_str = item + .get("output") + .and_then(Value::as_str) + .expect("shell output payload"); + match serde_json::from_str::(output_str) { + Ok(parsed) => { + let exit_code = parsed["metadata"]["exit_code"].as_i64(); + let stdout = parsed["output"].as_str().unwrap_or_default().to_string(); + (exit_code, stdout) + } + Err(_) => { + let structured = Regex::new(r"(?s)^Exit code:\s*(-?\d+).*?Output:\n(.*)$").unwrap(); + let regex = + Regex::new(r"(?s)^.*?Process exited with code (\d+)\n.*?Output:\n(.*)$").unwrap(); + if let Some(captures) = structured.captures(output_str) { + let exit_code = captures.get(1).unwrap().as_str().parse::().unwrap(); + let output = captures.get(2).unwrap().as_str(); + (Some(exit_code), output.to_string()) + } else if let Some(captures) = regex.captures(output_str) { + let exit_code = captures.get(1).unwrap().as_str().parse::().unwrap(); + let output = captures.get(2).unwrap().as_str(); + (Some(exit_code), output.to_string()) + } else { + (None, output_str.to_string()) + } + } + } +} + +async fn submit_turn( + test: &TestCodex, + prompt: &str, + approval_policy: AskForApproval, + sandbox_policy: SandboxPolicy, +) -> Result<()> { + let session_model = test.session_configured.model.clone(); + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: prompt.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd.path().to_path_buf(), + approval_policy, + sandbox_policy, + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + Ok(()) +} + +async fn expect_request_permissions_event( + test: &TestCodex, + expected_call_id: &str, +) -> PermissionProfile { + let event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + + match event { + EventMsg::RequestPermissions(request) => { + assert_eq!(request.call_id, expected_call_id); + request.permissions + } + EventMsg::TurnComplete(_) => panic!("expected request_permissions before completion"), + other => panic!("unexpected event: {other:?}"), + } +} + +#[tokio::test(flavor = "current_thread")] +#[cfg(target_os = "macos")] +async fn approved_folder_write_request_permissions_unblocks_later_exec_without_sandbox_args() +-> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::OnRequest; + let sandbox_policy = workspace_write_excluding_tmp(); + let sandbox_policy_for_config = sandbox_policy.clone(); + + let mut builder = test_codex().with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config + .features + .enable(Feature::RequestPermissions) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::RequestPermissionsTool) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + let requested_dir = tempfile::tempdir()?; + let requested_file = requested_dir.path().join("allowed-write.txt"); + let command = format!( + "printf {:?} > {:?} && cat {:?}", + "folder-grant-ok", requested_file, requested_file + ); + let requested_permissions = requested_directory_write_permissions(requested_dir.path()); + let normalized_requested_permissions = + normalized_directory_write_permissions(requested_dir.path())?; + + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-request-permissions-1"), + request_permissions_tool_event( + "permissions-call", + "Allow writing outside the workspace", + &requested_permissions, + )?, + ev_completed("resp-request-permissions-1"), + ]), + sse(vec![ + ev_response_created("resp-request-permissions-2"), + exec_command_event("exec-call", &command)?, + ev_completed("resp-request-permissions-2"), + ]), + sse(vec![ + ev_response_created("resp-request-permissions-3"), + ev_assistant_message("msg-request-permissions-1", "done"), + ev_completed("resp-request-permissions-3"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "write outside the workspace", + approval_policy, + sandbox_policy, + ) + .await?; + + let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await; + assert_eq!( + granted_permissions, + normalized_requested_permissions.clone() + ); + test.codex + .submit(Op::RequestPermissionsResponse { + id: "permissions-call".to_string(), + response: RequestPermissionsResponse { + permissions: normalized_requested_permissions, + scope: PermissionGrantScope::Turn, + }, + }) + .await?; + + let completion_event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + if let EventMsg::ExecApprovalRequest(approval) = completion_event { + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + } + + let exec_output = responses + .function_call_output_text("exec-call") + .map(|output| json!({ "output": output })) + .unwrap_or_else(|| panic!("expected exec-call output")); + let (exit_code, stdout) = parse_result(&exec_output); + assert!(exit_code.is_none() || exit_code == Some(0)); + assert!(stdout.contains("folder-grant-ok")); + assert!( + requested_file.exists(), + "touch command should create the file" + ); + assert_eq!(fs::read_to_string(&requested_file)?, "folder-grant-ok"); + + Ok(()) +} diff --git a/codex-rs/protocol/src/request_permissions.rs b/codex-rs/protocol/src/request_permissions.rs index feb43c30c..fc66e9b3d 100644 --- a/codex-rs/protocol/src/request_permissions.rs +++ b/codex-rs/protocol/src/request_permissions.rs @@ -4,6 +4,14 @@ use serde::Deserialize; use serde::Serialize; use ts_rs::TS; +#[derive(Debug, Clone, Copy, Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum PermissionGrantScope { + #[default] + Turn, + Session, +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] pub struct RequestPermissionsArgs { #[serde(skip_serializing_if = "Option::is_none")] @@ -14,6 +22,8 @@ pub struct RequestPermissionsArgs { #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] pub struct RequestPermissionsResponse { pub permissions: PermissionProfile, + #[serde(default)] + pub scope: PermissionGrantScope, } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 19f57b2f0..a13252939 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -28,6 +28,7 @@ use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; +use codex_protocol::request_permissions::PermissionGrantScope; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -282,9 +283,16 @@ impl ApprovalOverlay { ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::NetworkPolicyAmendment { .. } => Default::default(), }; + let scope = if matches!(decision, ReviewDecision::ApprovedForSession) { + PermissionGrantScope::Session + } else { + PermissionGrantScope::Turn + }; if request.thread_label().is_none() { let message = if granted_permissions.is_empty() { "You did not grant additional permissions" + } else if matches!(scope, PermissionGrantScope::Session) { + "You granted additional permissions for this session" } else { "You granted additional permissions" }; @@ -299,6 +307,7 @@ impl ApprovalOverlay { id: call_id.to_string(), response: codex_protocol::request_permissions::RequestPermissionsResponse { permissions: granted_permissions, + scope, }, }, }); @@ -831,6 +840,12 @@ fn permissions_options() -> Vec { display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], }, + ApprovalOption { + label: "Yes, grant these permissions for this session".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], + }, ApprovalOption { label: "No, continue without permissions".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Denied), @@ -1244,11 +1259,39 @@ mod tests { labels, vec![ "Yes, grant these permissions".to_string(), + "Yes, grant these permissions for this session".to_string(), "No, continue without permissions".to_string(), ] ); } + #[test] + fn permissions_session_shortcut_submits_session_scope() { + let (tx, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut view = + ApprovalOverlay::new(make_permissions_request(), tx, Features::with_defaults()); + + view.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE)); + + let mut saw_op = false; + while let Ok(ev) = rx.try_recv() { + if let AppEvent::SubmitThreadOp { + op: Op::RequestPermissionsResponse { response, .. }, + .. + } = ev + { + assert_eq!(response.scope, PermissionGrantScope::Session); + saw_op = true; + break; + } + } + assert!( + saw_op, + "expected permission approval decision to emit a session-scoped response" + ); + } + #[test] fn additional_permissions_prompt_shows_permission_rule_line() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_permissions_prompt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_permissions_prompt.snap index 9265c618a..e7a21c42c 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_permissions_prompt.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__approval_overlay__tests__approval_overlay_permissions_prompt.snap @@ -10,6 +10,7 @@ expression: "normalize_snapshot_paths(render_overlay_lines(&view, 120))" Permission rule: network; read `/tmp/readme.txt`; write `/tmp/out.txt` › 1. Yes, grant these permissions (y) - 2. No, continue without permissions (n) + 2. Yes, grant these permissions for this session (a) + 3. No, continue without permissions (n) Press enter to confirm or esc to cancel