From f8562bd47bcc6f2327590506890c0c9f1227e646 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 21 Apr 2026 10:23:01 -0700 Subject: [PATCH] sandboxing: intersect permission profiles semantically (#18275) ## Why Permission approval responses must not be able to grant more access than the tool requested. Moving this flow to `PermissionProfile` means the comparison must be profile-shaped instead of `SandboxPolicy`-shaped, and cwd-relative special paths such as `:cwd` and `:project_roots` must stay anchored to the turn that produced the request. ## What changed This implements semantic `PermissionProfile` intersection in `codex-sandboxing` for file-system and network permissions. The intersection accepts narrower path grants, rejects broader grants, preserves deny-read carve-outs and glob scan depth, and materializes cwd-dependent special-path grants to absolute paths before they can be recorded for reuse. The request-permissions response paths now use that intersection consistently. App-server captures the request turn cwd before waiting for the client response, includes that cwd in the v2 approval params, and core stores the requested profile plus cwd for direct TUI/client responses and Guardian decisions before recording turn- or session-scoped grants. The TUI app-server bridge now preserves the app-server request cwd when converting permission approval params into core events. ## Verification - `cargo test -p codex-sandboxing intersect_permission_profiles -- --nocapture` - `cargo test -p codex-app-server request_permissions_response -- --nocapture` - `cargo test -p codex-core request_permissions_response_materializes_session_cwd_grants_before_recording -- --nocapture` - `cargo check -p codex-tui --tests` - `cargo check --tests` - `cargo test -p codex-tui app_server_request_permissions_preserves_file_system_permissions` --- .../PermissionsRequestApprovalParams.json | 4 + .../schema/json/ServerRequest.json | 4 + .../codex_app_server_protocol.schemas.json | 4 + .../v2/PermissionsRequestApprovalParams.ts | 3 +- .../app-server-protocol/src/protocol/v2.rs | 4 + codex-rs/app-server/README.md | 3 +- .../app-server/src/bespoke_event_handling.rs | 201 ++++++++++-- .../tests/suite/v2/request_permissions.rs | 1 + codex-rs/core/src/codex_delegate.rs | 10 +- codex-rs/core/src/codex_delegate_tests.rs | 4 + codex-rs/core/src/session/mod.rs | 96 ++++-- codex-rs/core/src/session/tests.rs | 97 ++++++ codex-rs/core/src/state/mod.rs | 1 + codex-rs/core/src/state/turn.rs | 19 +- .../core/src/tools/handlers/apply_patch.rs | 1 + codex-rs/core/src/tools/handlers/mod.rs | 3 +- codex-rs/core/src/tools/handlers/shell.rs | 1 + .../core/src/tools/handlers/unified_exec.rs | 1 + .../core/tests/suite/request_permissions.rs | 6 +- codex-rs/protocol/src/request_permissions.rs | 4 + codex-rs/sandboxing/src/policy_transforms.rs | 206 ++++++++++++- .../sandboxing/src/policy_transforms_tests.rs | 288 +++++++++++++++++- codex-rs/tui/src/app/app_server_requests.rs | 1 + codex-rs/tui/src/app/tests.rs | 1 + codex-rs/tui/src/chatwidget.rs | 1 + .../src/chatwidget/tests/approval_requests.rs | 4 + 26 files changed, 897 insertions(+), 71 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index e389983bf..c2e26c553 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -295,6 +295,9 @@ } }, "properties": { + "cwd": { + "$ref": "#/definitions/AbsolutePathBuf" + }, "itemId": { "type": "string" }, @@ -315,6 +318,7 @@ } }, "required": [ + "cwd", "itemId", "permissions", "threadId", diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 6bd386b6a..016dbe341 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -1584,6 +1584,9 @@ }, "PermissionsRequestApprovalParams": { "properties": { + "cwd": { + "$ref": "#/definitions/AbsolutePathBuf" + }, "itemId": { "type": "string" }, @@ -1604,6 +1607,7 @@ } }, "required": [ + "cwd", "itemId", "permissions", "threadId", 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 630e8d493..d1518ce28 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 @@ -3432,6 +3432,9 @@ "PermissionsRequestApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "cwd": { + "$ref": "#/definitions/v2/AbsolutePathBuf" + }, "itemId": { "type": "string" }, @@ -3452,6 +3455,7 @@ } }, "required": [ + "cwd", "itemId", "permissions", "threadId", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts index efdefead7..308670a80 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts @@ -1,6 +1,7 @@ // 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. +import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { RequestPermissionProfile } from "./RequestPermissionProfile"; -export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, reason: string | null, permissions: RequestPermissionProfile, }; +export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, cwd: AbsolutePathBuf, reason: string | null, permissions: RequestPermissionProfile, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index a78db2e0e..b7162eb4d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -6826,6 +6826,7 @@ pub struct PermissionsRequestApprovalParams { pub thread_id: String, pub turn_id: String, pub item_id: String, + pub cwd: AbsolutePathBuf, pub reason: Option, pub permissions: RequestPermissionProfile, } @@ -7260,6 +7261,7 @@ mod tests { "threadId": "thr_123", "turnId": "turn_123", "itemId": "call_123", + "cwd": absolute_path_string("repo"), "reason": "Select a workspace root", "permissions": { "network": { @@ -7273,6 +7275,7 @@ mod tests { })) .expect("permissions request should deserialize"); + assert_eq!(params.cwd, absolute_path("repo")); assert_eq!( params.permissions, RequestPermissionProfile { @@ -7320,6 +7323,7 @@ mod tests { "threadId": "thr_123", "turnId": "turn_123", "itemId": "call_123", + "cwd": absolute_path_string("repo"), "reason": "Select a workspace root", "permissions": { "network": null, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 57cb6c173..86221ec80 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1148,7 +1148,7 @@ the client can offer session-scoped and/or persistent approval choices. ### Permission requests -The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. +The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. The `cwd` field identifies the directory used to resolve cwd-relative permissions such as `:cwd`, `:project_roots`, and relative deny globs. ```json { @@ -1158,6 +1158,7 @@ The built-in `request_permissions` tool sends an `item/permissions/requestApprov "threadId": "thr_123", "turnId": "turn_123", "itemId": "call_123", + "cwd": "/Users/me/project", "reason": "Select a workspace root", "permissions": { "fileSystem": { diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index d2b435ef9..a2ea77900 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::ThreadId; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem as CoreDynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse as CoreDynamicToolResponse; use codex_protocol::items::parse_hook_prompt_message; +use codex_protocol::models::PermissionProfile as CorePermissionProfile; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo; use codex_protocol::protocol::Event; @@ -908,27 +909,32 @@ pub(crate) async fn apply_bespoke_event_handling( .note_permission_requested(&conversation_id.to_string()) .await; let requested_permissions = request.permissions.clone(); + let request_cwd = match request.cwd.clone() { + Some(cwd) => cwd, + None => conversation.config_snapshot().await.cwd, + }; let params = PermissionsRequestApprovalParams { thread_id: conversation_id.to_string(), turn_id: request.turn_id.clone(), item_id: request.call_id.clone(), + cwd: request_cwd.clone(), reason: request.reason, permissions: request.permissions.into(), }; let (pending_request_id, rx) = outgoing .send_request(ServerRequestPayload::PermissionsRequestApproval(params)) .await; + let pending_response = PendingRequestPermissionsResponse { + call_id: request.call_id, + requested_permissions, + request_cwd, + pending_request_id, + receiver: rx, + request_permissions_guard: permission_guard, + }; tokio::spawn(async move { - on_request_permissions_response( - request.call_id, - requested_permissions, - pending_request_id, - rx, - conversation, - thread_state, - permission_guard, - ) - .await; + on_request_permissions_response(pending_response, conversation, thread_state) + .await; }); } else { error!( @@ -2590,20 +2596,26 @@ fn mcp_server_elicitation_response_from_client_result( } async fn on_request_permissions_response( - call_id: String, - requested_permissions: CoreRequestPermissionProfile, - pending_request_id: RequestId, - receiver: oneshot::Receiver, + pending_response: PendingRequestPermissionsResponse, conversation: Arc, thread_state: Arc>, - request_permissions_guard: ThreadWatchActiveGuard, ) { + let PendingRequestPermissionsResponse { + call_id, + requested_permissions, + request_cwd, + pending_request_id, + receiver, + request_permissions_guard, + } = pending_response; let response = receiver.await; resolve_server_request_on_thread_listener(&thread_state, pending_request_id).await; drop(request_permissions_guard); - let Some(response) = - request_permissions_response_from_client_result(requested_permissions, response) - else { + let Some(response) = request_permissions_response_from_client_result( + requested_permissions, + response, + request_cwd.as_path(), + ) else { return; }; @@ -2618,9 +2630,19 @@ async fn on_request_permissions_response( } } +struct PendingRequestPermissionsResponse { + call_id: String, + requested_permissions: CoreRequestPermissionProfile, + request_cwd: AbsolutePathBuf, + pending_request_id: RequestId, + receiver: oneshot::Receiver, + request_permissions_guard: ThreadWatchActiveGuard, +} + fn request_permissions_response_from_client_result( requested_permissions: CoreRequestPermissionProfile, response: std::result::Result, + cwd: &std::path::Path, ) -> Option { let value = match response { Ok(Ok(value)) => value, @@ -2649,12 +2671,14 @@ fn request_permissions_response_from_client_result( scope: codex_app_server_protocol::PermissionGrantScope::Turn, } }); + let granted_permissions: CorePermissionProfile = response.permissions.into(); + let permissions = if granted_permissions.is_empty() { + CoreRequestPermissionProfile::default() + } else { + intersect_permission_profiles(requested_permissions.into(), granted_permissions, cwd).into() + }; Some(CoreRequestPermissionsResponse { - permissions: intersect_permission_profiles( - requested_permissions.into(), - response.permissions.into(), - ) - .into(), + permissions, scope: response.scope.to_core(), }) } @@ -3027,6 +3051,10 @@ mod tests { use codex_protocol::mcp::CallToolResult; use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::protocol::CollabResumeBeginEvent; @@ -3711,6 +3739,7 @@ mod tests { let response = request_permissions_response_from_client_result( CoreRequestPermissionProfile::default(), Ok(Err(error)), + std::env::current_dir().expect("current dir").as_path(), ); assert_eq!(response, None); @@ -3797,12 +3826,14 @@ mod tests { ), ]; + let cwd = std::env::current_dir().expect("current dir"); for (granted_permissions, expected_permissions) in cases { let response = request_permissions_response_from_client_result( requested_permissions.clone(), Ok(Ok(serde_json::json!({ "permissions": granted_permissions, }))), + cwd.as_path(), ) .expect("response should be accepted"); @@ -3824,6 +3855,7 @@ mod tests { "scope": "session", "permissions": {}, }))), + std::env::current_dir().expect("current dir").as_path(), ) .expect("response should be accepted"); @@ -3836,6 +3868,129 @@ mod tests { ); } + #[test] + fn request_permissions_response_accepts_explicit_child_grant_for_requested_cwd_scope() { + let temp_dir = TempDir::new().expect("temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd"); + let child = cwd.join("child"); + let requested_permissions = CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + let response = request_permissions_response_from_client_result( + requested_permissions, + Ok(Ok(serde_json::json!({ + "permissions": { + "fileSystem": { + "write": [child], + }, + }, + }))), + cwd.as_path(), + ) + .expect("response should be accepted"); + + assert_eq!( + response.permissions, + CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + } + ); + } + + #[test] + fn request_permissions_response_rejects_child_grant_outside_requested_cwd_scope() { + let temp_dir = TempDir::new().expect("temp dir"); + let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd")) + .expect("absolute request cwd"); + let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd")) + .expect("absolute later cwd"); + let later_child = later_cwd.join("child"); + let requested_permissions = CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + let response = request_permissions_response_from_client_result( + requested_permissions, + Ok(Ok(serde_json::json!({ + "permissions": { + "fileSystem": { + "write": [later_child], + }, + }, + }))), + request_cwd.as_path(), + ) + .expect("response should be accepted"); + + assert_eq!( + response.permissions, + CoreRequestPermissionProfile::default() + ); + } + + #[test] + fn request_permissions_response_ignores_broader_cwd_grant_for_requested_child_path() { + let temp_dir = TempDir::new().expect("temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd"); + let child = cwd.join("child"); + let requested_permissions = CoreRequestPermissionProfile { + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + }; + + let response = request_permissions_response_from_client_result( + requested_permissions, + Ok(Ok(serde_json::json!({ + "permissions": { + "fileSystem": { + "entries": [{ + "path": { + "type": "special", + "value": { + "kind": "current_working_directory" + } + }, + "access": "write" + }], + }, + }, + }))), + cwd.as_path(), + ) + .expect("response should be accepted"); + + assert_eq!( + response.permissions, + CoreRequestPermissionProfile::default() + ); + } + #[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 295ac96a3..4f7c398f9 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -76,6 +76,7 @@ async fn request_permissions_round_trip() -> Result<()> { assert_eq!(params.thread_id, thread.id); assert_eq!(params.turn_id, turn.id); assert_eq!(params.item_id, "call1"); + assert!(params.cwd.as_path().is_absolute()); assert_eq!(params.reason, Some("Select a workspace root".to_string())); let requested_writes = params .permissions diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index be021fe6a..7247c601f 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -741,8 +741,14 @@ async fn handle_request_permissions( reason: event.reason, permissions: event.permissions, }; - let response_fut = - parent_session.request_permissions(parent_ctx, call_id.clone(), args, cancel_token.clone()); + let cwd = event.cwd.unwrap_or_else(|| parent_ctx.cwd.clone()); + let response_fut = parent_session.request_permissions_for_cwd( + parent_ctx, + call_id.clone(), + args, + cwd, + cancel_token.clone(), + ); let response = await_request_permissions_with_cancel(response_fut, parent_session, &call_id, cancel_token) .await; diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index d26460a25..3e685d92e 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -180,8 +180,10 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { }, scope: PermissionGrantScope::Turn, }; + let delegated_cwd = parent_ctx.cwd.join("delegated-cwd"); let cancel_token = CancellationToken::new(); let request_call_id = call_id.clone(); + let request_cwd = delegated_cwd.clone(); let handle = tokio::spawn({ let codex = Arc::clone(&codex); @@ -203,6 +205,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { }), ..RequestPermissionProfile::default() }, + cwd: Some(request_cwd), }, &cancel_token, ) @@ -218,6 +221,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { panic!("expected RequestPermissions event"); }; assert_eq!(request.call_id, call_id.clone()); + assert_eq!(request.cwd, Some(delegated_cwd)); parent_session .notify_request_permissions_response(&call_id, expected_response.clone()) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 568ca00e8..4b919800c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::fmt::Debug; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicU64; @@ -120,6 +121,7 @@ use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; use codex_rollout::RolloutConfig; use codex_rollout::state_db; +use codex_sandboxing::policy_transforms::intersect_permission_profiles; use codex_shell_command::parse_command::parse_command; use codex_terminal_detection::user_agent; use codex_thread_store::LocalThreadStore; @@ -275,6 +277,7 @@ use crate::skills_watcher::SkillsWatcher; use crate::skills_watcher::SkillsWatcherEvent; use crate::state::ActiveTurn; use crate::state::MailboxDeliveryPhase; +use crate::state::PendingRequestPermissions; use crate::state::SessionServices; use crate::state::SessionState; #[cfg(test)] @@ -1901,16 +1904,34 @@ impl Session { rx_approve } - #[expect( - clippy::await_holding_invalid_type, - reason = "active turn checks and turn state updates must remain atomic" - )] pub async fn request_permissions( self: &Arc, turn_context: &Arc, call_id: String, args: RequestPermissionsArgs, cancellation_token: CancellationToken, + ) -> Option { + self.request_permissions_for_cwd( + turn_context, + call_id, + args, + turn_context.cwd.clone(), + cancellation_token, + ) + .await + } + + #[expect( + clippy::await_holding_invalid_type, + reason = "active turn checks and turn state updates must remain atomic" + )] + pub(crate) async fn request_permissions_for_cwd( + self: &Arc, + turn_context: &Arc, + call_id: String, + args: RequestPermissionsArgs, + cwd: AbsolutePathBuf, + cancellation_token: CancellationToken, ) -> Option { match turn_context.as_ref().approval_policy.value() { AskForApproval::Never => { @@ -1933,8 +1954,9 @@ impl Session { | AskForApproval::Granular(_) => {} } + let requested_permissions = args.permissions; + if crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) { - let requested_permissions = args.permissions; let originating_turn_state = { let active = self.active_turn.lock().await; active.as_ref().map(|active| Arc::clone(&active.turn_state)) @@ -1964,19 +1986,19 @@ impl Session { let response = match decision { ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { RequestPermissionsResponse { - permissions: requested_permissions, + permissions: requested_permissions.clone(), scope: PermissionGrantScope::Turn, } } ReviewDecision::ApprovedForSession => RequestPermissionsResponse { - permissions: requested_permissions, + permissions: requested_permissions.clone(), scope: PermissionGrantScope::Session, }, ReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => match network_policy_amendment.action { NetworkPolicyRuleAction::Allow => RequestPermissionsResponse { - permissions: requested_permissions, + permissions: requested_permissions.clone(), scope: PermissionGrantScope::Turn, }, NetworkPolicyRuleAction::Deny => RequestPermissionsResponse { @@ -1991,6 +2013,11 @@ impl Session { } } }; + let response = Self::normalize_request_permissions_response( + requested_permissions, + response, + cwd.as_path(), + ); self.record_granted_request_permissions_for_turn( &response, originating_turn_state.as_ref(), @@ -2005,7 +2032,14 @@ impl Session { match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_request_permissions(call_id.clone(), tx_response) + ts.insert_pending_request_permissions( + call_id.clone(), + PendingRequestPermissions { + tx_response, + requested_permissions: requested_permissions.clone(), + cwd: cwd.clone(), + }, + ) } None => None, } @@ -2018,7 +2052,8 @@ impl Session { call_id: call_id.clone(), turn_id: turn_context.sub_id.clone(), reason: args.reason, - permissions: args.permissions, + permissions: requested_permissions, + cwd: Some(cwd), }); self.send_event(turn_context.as_ref(), event).await; tokio::select! { @@ -2121,16 +2156,19 @@ impl Session { None => (None, None), } }; - if entry.is_some() { - self.record_granted_request_permissions_for_turn( - &response, - originating_turn_state.as_ref(), - ) - .await; - } match entry { - Some(tx_response) => { - tx_response.send(response).ok(); + Some(entry) => { + let response = Self::normalize_request_permissions_response( + entry.requested_permissions, + response, + entry.cwd.as_path(), + ); + self.record_granted_request_permissions_for_turn( + &response, + originating_turn_state.as_ref(), + ) + .await; + entry.tx_response.send(response).ok(); } None => { warn!("No pending request_permissions found for call_id: {call_id}"); @@ -2138,6 +2176,26 @@ impl Session { } } + fn normalize_request_permissions_response( + requested_permissions: RequestPermissionProfile, + response: RequestPermissionsResponse, + cwd: &Path, + ) -> RequestPermissionsResponse { + if response.permissions.is_empty() { + return response; + } + + RequestPermissionsResponse { + permissions: intersect_permission_profiles( + requested_permissions.into(), + response.permissions.into(), + cwd, + ) + .into(), + scope: response.scope, + } + } + async fn record_granted_request_permissions_for_turn( &self, response: &RequestPermissionsResponse, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index aabc92cbe..f1a11bfcb 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -40,6 +40,7 @@ use codex_protocol::AgentPath; use codex_protocol::ThreadId; use codex_protocol::config_types::TrustLevel; use codex_protocol::exec_output::ExecToolCallOutput; +use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::permissions::FileSystemAccessMode; @@ -3535,6 +3536,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() panic!("expected request_permissions event"); }; assert_eq!(request.call_id, call_id); + assert_eq!(request.cwd, Some(turn_context.cwd.clone())); session .notify_request_permissions_response(&request.call_id, expected_response.clone()) @@ -3548,6 +3550,101 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() assert_eq!(response, Some(expected_response)); } +#[tokio::test] +async fn request_permissions_response_materializes_session_cwd_grants_before_recording() { + 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(AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval: true, + rules: true, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + })) + .expect("test setup should allow updating approval policy"); + + let session = Arc::new(session); + let turn_context = Arc::new(turn_context); + let call_id = "call-1".to_string(); + let requested_permissions = RequestPermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + let handle = tokio::spawn({ + let session = Arc::clone(&session); + let turn_context = Arc::clone(&turn_context); + let call_id = call_id.clone(); + let requested_permissions = requested_permissions.clone(); + async move { + session + .request_permissions( + &turn_context, + call_id, + codex_protocol::request_permissions::RequestPermissionsArgs { + reason: Some("need cwd write".to_string()), + permissions: requested_permissions, + }, + CancellationToken::new(), + ) + .await + } + }); + + let request_event = tokio::time::timeout(StdDuration::from_secs(1), rx.recv()) + .await + .expect("request_permissions event timed out") + .expect("request_permissions event missing"); + let EventMsg::RequestPermissions(request) = request_event.msg else { + panic!("expected request_permissions event"); + }; + let request_cwd = request.cwd.clone().expect("request cwd"); + + session + .notify_request_permissions_response( + &request.call_id, + codex_protocol::request_permissions::RequestPermissionsResponse { + permissions: request.permissions, + scope: PermissionGrantScope::Session, + }, + ) + .await; + + let expected_permissions = RequestPermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![request_cwd]), + )), + ..Default::default() + }; + let expected_response = codex_protocol::request_permissions::RequestPermissionsResponse { + permissions: expected_permissions.clone(), + scope: PermissionGrantScope::Session, + }; + + let response = tokio::time::timeout(StdDuration::from_secs(1), handle) + .await + .expect("request_permissions future timed out") + .expect("request_permissions join error"); + + assert_eq!(response, Some(expected_response)); + assert_eq!( + session.granted_session_permissions().await, + Some(expected_permissions.into()) + ); +} + #[tokio::test] 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; diff --git a/codex-rs/core/src/state/mod.rs b/codex-rs/core/src/state/mod.rs index f3ebc7225..13f3bf6c8 100644 --- a/codex-rs/core/src/state/mod.rs +++ b/codex-rs/core/src/state/mod.rs @@ -6,6 +6,7 @@ pub(crate) use service::SessionServices; pub(crate) use session::SessionState; pub(crate) use turn::ActiveTurn; pub(crate) use turn::MailboxDeliveryPhase; +pub(crate) use turn::PendingRequestPermissions; pub(crate) use turn::RunningTask; pub(crate) use turn::TaskKind; pub(crate) use turn::TurnState; diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index f1b7bec6b..903318c2a 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -11,9 +11,11 @@ use tokio_util::task::AbortOnDropHandle; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::models::ResponseInputItem; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; +use codex_utils_absolute_path::AbsolutePathBuf; use rmcp::model::RequestId; use tokio::sync::oneshot; @@ -97,7 +99,7 @@ impl ActiveTurn { #[derive(Default)] pub(crate) struct TurnState { pending_approvals: HashMap>, - pending_request_permissions: HashMap>, + pending_request_permissions: HashMap, pending_user_input: HashMap>, pending_elicitations: HashMap<(String, RequestId), oneshot::Sender>, pending_dynamic_tools: HashMap>, @@ -109,6 +111,12 @@ pub(crate) struct TurnState { pub(crate) token_usage_at_turn_start: TokenUsage, } +pub(crate) struct PendingRequestPermissions { + pub(crate) tx_response: oneshot::Sender, + pub(crate) requested_permissions: RequestPermissionProfile, + pub(crate) cwd: AbsolutePathBuf, +} + impl TurnState { pub(crate) fn insert_pending_approval( &mut self, @@ -137,15 +145,16 @@ impl TurnState { pub(crate) fn insert_pending_request_permissions( &mut self, key: String, - tx: oneshot::Sender, - ) -> Option> { - self.pending_request_permissions.insert(key, tx) + pending_request_permissions: PendingRequestPermissions, + ) -> Option { + self.pending_request_permissions + .insert(key, pending_request_permissions) } pub(crate) fn remove_pending_request_permissions( &mut self, key: &str, - ) -> Option> { + ) -> Option { self.pending_request_permissions.remove(key) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 9d40bbad0..033315a69 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -259,6 +259,7 @@ async fn effective_patch_permissions( ); let effective_additional_permissions = apply_granted_turn_permissions( session, + turn.cwd.as_path(), crate::sandboxing::SandboxPermissions::UseDefault, write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, &turn.cwd), ) diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 17c7c09be..0217cab33 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -169,6 +169,7 @@ pub(super) fn implicit_granted_permissions( pub(super) async fn apply_granted_turn_permissions( session: &Session, + cwd: &std::path::Path, sandbox_permissions: SandboxPermissions, additional_permissions: Option, ) -> EffectiveAdditionalPermissions { @@ -192,7 +193,7 @@ pub(super) async fn apply_granted_turn_permissions( ); let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) { (Some(effective_permissions), Some(granted_permissions)) => { - intersect_permission_profiles(effective_permissions.clone(), granted_permissions) + intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd) == *effective_permissions } _ => false, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b6bf4bc7a..877023894 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -424,6 +424,7 @@ impl ShellHandler { let requested_additional_permissions = additional_permissions.clone(); let effective_additional_permissions = apply_granted_turn_permissions( session.as_ref(), + turn.cwd.as_path(), exec_params.sandbox_permissions, additional_permissions, ) diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index ab5279d1b..fd04d42f2 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -230,6 +230,7 @@ impl ToolHandler for UnifiedExecHandler { let requested_additional_permissions = additional_permissions.clone(); let effective_additional_permissions = apply_granted_turn_permissions( context.session.as_ref(), + context.turn.cwd.as_path(), sandbox_permissions, additional_permissions, ) diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 39b0cfb44..614cd10b9 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -1586,17 +1586,17 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() .unwrap_or_else(|| panic!("expected filesystem permissions")); let (approval_reads, approval_writes) = approval_file_system .legacy_read_write_roots() - .unwrap_or_default(); + .unwrap_or_else(|| panic!("expected legacy-compatible permissions")); assert!(approval_reads.as_ref().is_none_or(Vec::is_empty)); let mut approval_writes = approval_writes.unwrap_or_default(); approval_writes.sort_by_key(|path| path.display().to_string()); - let (_expected_reads, expected_writes) = merged_permissions + let (_, expected_writes) = merged_permissions .file_system .unwrap_or_else(|| panic!("expected merged filesystem permissions")) .legacy_read_write_roots() - .unwrap_or_default(); + .unwrap_or_else(|| panic!("expected legacy-compatible permissions")); let mut expected_writes = expected_writes.unwrap_or_default(); expected_writes.sort_by_key(|path| path.display().to_string()); diff --git a/codex-rs/protocol/src/request_permissions.rs b/codex-rs/protocol/src/request_permissions.rs index 400fde4e2..473b8d973 100644 --- a/codex-rs/protocol/src/request_permissions.rs +++ b/codex-rs/protocol/src/request_permissions.rs @@ -1,6 +1,7 @@ use crate::models::FileSystemPermissions; use crate::models::NetworkPermissions; use crate::models::PermissionProfile; +use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -70,4 +71,7 @@ pub struct RequestPermissionsEvent { #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, pub permissions: RequestPermissionProfile, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub cwd: Option, } diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 2b8c01eaa..134ac84f1 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -6,7 +6,9 @@ use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::permissions::ReadDenyMatcher; use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; @@ -14,6 +16,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; use std::collections::HashSet; use std::num::NonZeroUsize; +use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Eq)] pub struct EffectiveSandboxPermissions { @@ -146,21 +150,47 @@ pub fn merge_permission_profiles( pub fn intersect_permission_profiles( requested: PermissionProfile, granted: PermissionProfile, + cwd: &Path, ) -> PermissionProfile { let file_system = requested .file_system .map(|requested_file_system| { let granted_file_system = granted.file_system.unwrap_or_default(); - let entries: Vec<_> = requested_file_system + let requested_policy = + FileSystemSandboxPolicy::restricted(requested_file_system.entries.clone()); + let requested_read_deny_matcher = ReadDenyMatcher::new(&requested_policy, cwd); + let accepted_entries: Vec<_> = granted_file_system .entries - .into_iter() - .filter(|entry| granted_file_system.entries.contains(entry)) + .iter() + .filter(|entry| { + granted_file_system_entry_within_request( + &requested_file_system, + &requested_policy, + requested_read_deny_matcher.as_ref(), + entry, + cwd, + ) + }) + .map(|entry| materialize_cwd_dependent_entry(entry, cwd)) .collect(); + let mut entries = accepted_entries.clone(); + let requested_retained_deny_entries = retain_constraining_deny_entries( + &requested_file_system.entries, + &accepted_entries, + cwd, + &mut entries, + ); + let granted_retained_deny_entries = retain_constraining_deny_entries( + &granted_file_system.entries, + &accepted_entries, + cwd, + &mut entries, + ); FileSystemPermissions { glob_scan_max_depth: merge_glob_scan_max_depth( - &entries, + &requested_retained_deny_entries, requested_file_system.glob_scan_max_depth.map(usize::from), - &entries, + &granted_retained_deny_entries, granted_file_system.glob_scan_max_depth.map(usize::from), ) .and_then(NonZeroUsize::new), @@ -230,6 +260,172 @@ enum GlobScanDepth { Unbounded, } +fn granted_file_system_entry_within_request( + requested: &FileSystemPermissions, + requested_policy: &FileSystemSandboxPolicy, + requested_read_deny_matcher: Option<&ReadDenyMatcher>, + granted_entry: &FileSystemSandboxEntry, + cwd: &Path, +) -> bool { + if !granted_entry.access.can_read() { + return false; + } + + if let Some(path) = resolve_permission_path(&granted_entry.path, cwd) { + if requested_read_deny_matcher.is_some_and(|matcher| matcher.is_read_denied(path.as_path())) + { + return false; + } + return access_covers( + requested_policy.resolve_access_with_cwd(path.as_path(), cwd), + granted_entry.access, + ); + } + + requested.entries.iter().any(|requested_entry| { + access_covers(requested_entry.access, granted_entry.access) + && requested_entry.path == granted_entry.path + }) +} + +fn retain_constraining_deny_entries( + source_entries: &[FileSystemSandboxEntry], + accepted_entries: &[FileSystemSandboxEntry], + cwd: &Path, + output_entries: &mut Vec, +) -> Vec { + let mut retained_entries = Vec::new(); + for entry in source_entries + .iter() + .filter(|entry| entry.access == FileSystemAccessMode::None) + { + if !deny_entry_constrains_accepted_grant(entry, accepted_entries, cwd) { + continue; + } + let entry = materialize_cwd_dependent_entry(entry, cwd); + if !output_entries.contains(&entry) { + output_entries.push(entry.clone()); + } + retained_entries.push(entry); + } + retained_entries +} + +fn deny_entry_constrains_accepted_grant( + deny_entry: &FileSystemSandboxEntry, + accepted_entries: &[FileSystemSandboxEntry], + cwd: &Path, +) -> bool { + accepted_entries + .iter() + .filter(|entry| entry.access.can_read()) + .any(|entry| { + let Some(grant_path) = resolve_permission_path(&entry.path, cwd) else { + return false; + }; + match &deny_entry.path { + FileSystemPath::GlobPattern { pattern } => glob_static_prefix_path(pattern, cwd) + .is_some_and(|prefix| paths_overlap(prefix.as_path(), grant_path.as_path())), + FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => { + resolve_permission_path(&deny_entry.path, cwd).is_some_and(|deny_path| { + paths_overlap(deny_path.as_path(), grant_path.as_path()) + }) + } + } + }) +} + +fn glob_static_prefix_path(pattern: &str, cwd: &Path) -> Option { + let resolved_pattern = AbsolutePathBuf::resolve_path_against_base(pattern, cwd); + let resolved_pattern = resolved_pattern.as_path().to_string_lossy(); + let prefix = match resolved_pattern.find(['*', '?', '[', ']']) { + Some(0) => return None, + Some(index) => { + let prefix = &resolved_pattern[..index]; + if prefix.ends_with(std::path::MAIN_SEPARATOR) + || prefix.ends_with('/') + || prefix.ends_with('\\') + { + Path::new(prefix) + } else { + Path::new(prefix).parent()? + } + } + None => Path::new(resolved_pattern.as_ref()), + }; + AbsolutePathBuf::from_absolute_path(prefix).ok() +} + +fn paths_overlap(left: &Path, right: &Path) -> bool { + left.starts_with(right) || right.starts_with(left) +} + +fn access_covers(requested: FileSystemAccessMode, granted: FileSystemAccessMode) -> bool { + match granted { + FileSystemAccessMode::Read => requested.can_read(), + FileSystemAccessMode::Write => requested.can_write(), + FileSystemAccessMode::None => false, + } +} + +fn materialize_cwd_dependent_entry( + entry: &FileSystemSandboxEntry, + cwd: &Path, +) -> FileSystemSandboxEntry { + match &entry.path { + FileSystemPath::Special { + value: + FileSystemSpecialPath::CurrentWorkingDirectory + | FileSystemSpecialPath::ProjectRoots { .. }, + } => resolve_permission_path(&entry.path, cwd) + .map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: entry.access, + }) + .unwrap_or_else(|| entry.clone()), + FileSystemPath::Path { .. } + | FileSystemPath::GlobPattern { .. } + | FileSystemPath::Special { .. } => entry.clone(), + } +} + +fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option { + match path { + FileSystemPath::Path { path } => Some(path.clone()), + FileSystemPath::GlobPattern { .. } => None, + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => { + let root = cwd.ancestors().last()?; + AbsolutePathBuf::from_absolute_path(root).ok() + } + FileSystemSpecialPath::CurrentWorkingDirectory => { + AbsolutePathBuf::from_absolute_path(cwd).ok() + } + FileSystemSpecialPath::ProjectRoots { subpath } => { + let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok()?; + Some(match subpath { + Some(subpath) => { + AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()) + } + None => cwd, + }) + } + FileSystemSpecialPath::Tmpdir => { + let tmpdir = std::env::var_os("TMPDIR")?; + if tmpdir.is_empty() { + None + } else { + AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok() + } + } + FileSystemSpecialPath::SlashTmp => AbsolutePathBuf::from_absolute_path("/tmp") + .ok() + .filter(|path| path.as_path().is_dir()), + FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => None, + }, + } +} + fn merge_permission_entries( base: &[FileSystemSandboxEntry], permissions: &[FileSystemSandboxEntry], diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index 8fcff965f..e34158bc9 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -244,7 +244,7 @@ fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() { let granted = requested.clone(); assert_eq!( - intersect_permission_profiles(requested.clone(), granted), + intersect_permission_profiles(requested.clone(), granted, temp_dir.path()), requested ); } @@ -265,7 +265,7 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { }; assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default()), + intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()), PermissionProfile::default() ); } @@ -286,13 +286,272 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() { }; assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default()), + intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()), + PermissionProfile::default() + ); +} + +#[test] +fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd() { + let temp_dir = TempDir::new().expect("create temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let child = cwd.join("child"); + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted.clone(), cwd.as_path()), + granted + ); +} + +#[test] +fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() { + let temp_dir = TempDir::new().expect("create temp dir"); + let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd")) + .expect("absolute request cwd"); + let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd")) + .expect("absolute later cwd"); + let cwd_write_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + let intersected = intersect_permission_profiles( + cwd_write_permissions.clone(), + cwd_write_permissions, + request_cwd.as_path(), + ); + + assert_eq!( + intersected, + PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![request_cwd]), + )), + ..Default::default() + } + ); + assert_eq!( + intersect_permission_profiles( + PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![later_cwd.join("child")]), + )), + ..Default::default() + }, + intersected, + later_cwd.as_path(), + ), + PermissionProfile::default() + ); +} + +#[test] +fn intersect_permission_profiles_materializes_cwd_deny_entries() { + let temp_dir = TempDir::new().expect("create temp dir"); + let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd")) + .expect("absolute request cwd"); + let permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::None, + }, + ], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(permissions.clone(), permissions, request_cwd.as_path()), + PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: request_cwd }, + access: FileSystemAccessMode::None, + }, + ], + glob_scan_max_depth: None, + }), + ..Default::default() + } + ); +} + +#[test] +fn intersect_permission_profiles_drops_deny_entries_without_filesystem_grants() { + let temp_dir = TempDir::new().expect("create temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let secret = cwd.join("secret"); + let requested = PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: secret }, + access: FileSystemAccessMode::None, + }, + ], + glob_scan_max_depth: None, + }), + }; + let granted = PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted.clone(), cwd.as_path()), + granted + ); +} + +#[test] +fn intersect_permission_profiles_rejects_concrete_grants_matched_by_requested_deny_globs() { + let temp_dir = TempDir::new().expect("create temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let env_file = cwd.join(".env"); + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: FileSystemAccessMode::None, + }, + ], + glob_scan_max_depth: std::num::NonZeroUsize::new(2), + }), + ..Default::default() + }; + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![env_file]), + )), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted, cwd.as_path()), + PermissionProfile::default() + ); +} + +#[test] +fn intersect_permission_profiles_drops_broader_cwd_grant_for_requested_child_path() { + let temp_dir = TempDir::new().expect("create temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let child = cwd.join("child"); + let requested = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![child]), + )), + ..Default::default() + }; + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }], + glob_scan_max_depth: None, + }), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted, cwd.as_path()), PermissionProfile::default() ); } #[test] fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() { + let cwd = std::env::current_dir().expect("current dir"); + let root_write = FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }; let deny_env_files = FileSystemSandboxEntry { path: FileSystemPath::GlobPattern { pattern: "**/*.env".to_string(), @@ -301,24 +560,24 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() { }; let requested = PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files.clone()], + entries: vec![root_write.clone(), deny_env_files.clone()], glob_scan_max_depth: std::num::NonZeroUsize::new(2), }), ..Default::default() }; let granted = PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files.clone()], + entries: vec![root_write.clone(), deny_env_files.clone()], glob_scan_max_depth: std::num::NonZeroUsize::new(4), }), ..Default::default() }; assert_eq!( - intersect_permission_profiles(requested, granted), + intersect_permission_profiles(requested, granted, cwd.as_path()), PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files], + entries: vec![root_write, deny_env_files], glob_scan_max_depth: std::num::NonZeroUsize::new(4), }), ..Default::default() @@ -328,6 +587,13 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() { #[test] fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() { + let cwd = std::env::current_dir().expect("current dir"); + let root_write = FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }; let deny_env_files = FileSystemSandboxEntry { path: FileSystemPath::GlobPattern { pattern: "**/*.env".to_string(), @@ -336,24 +602,24 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() { }; let requested = PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files.clone()], + entries: vec![root_write.clone(), deny_env_files.clone()], glob_scan_max_depth: std::num::NonZeroUsize::new(2), }), ..Default::default() }; let granted = PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files.clone()], + entries: vec![root_write.clone(), deny_env_files.clone()], glob_scan_max_depth: None, }), ..Default::default() }; assert_eq!( - intersect_permission_profiles(requested, granted), + intersect_permission_profiles(requested, granted, cwd.as_path()), PermissionProfile { file_system: Some(FileSystemPermissions { - entries: vec![deny_env_files], + entries: vec![root_write, deny_env_files], glob_scan_max_depth: None, }), ..Default::default() diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index 4e6356734..91701cd13 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -506,6 +506,7 @@ mod tests { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "perm-1".to_string(), + cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }), reason: None, permissions: serde_json::from_value(json!({ "network": { "enabled": null } diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index d34a49ec5..1749ee9f1 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2532,6 +2532,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( thread_id: thread_id.to_string(), turn_id: "turn-approval".to_string(), item_id: "call-approval".to_string(), + cwd: test_absolute_path("/tmp"), reason: Some("Need access to .git".to_string()), permissions: codex_app_server_protocol::RequestPermissionProfile { network: Some(AdditionalNetworkPermissions { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index d226fa3c8..be258fae4 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1673,6 +1673,7 @@ fn request_permissions_from_params( call_id: params.item_id, reason: params.reason, permissions: params.permissions.into(), + cwd: Some(params.cwd), } } diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 4e5bc7556..320dc8a9c 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -151,11 +151,14 @@ fn app_server_request_permissions_preserves_file_system_permissions() { .expect("absolute read path"); let write_path = AbsolutePathBuf::try_from(PathBuf::from(test_path_display("/tmp/write"))) .expect("absolute write path"); + let cwd = + AbsolutePathBuf::try_from(PathBuf::from(test_path_display("/tmp"))).expect("absolute cwd"); let request = request_permissions_from_params(AppServerPermissionsRequestApprovalParams { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), + cwd: cwd.clone(), reason: Some("Select a workspace root".to_string()), permissions: codex_app_server_protocol::RequestPermissionProfile { network: Some(AppServerAdditionalNetworkPermissions { @@ -182,6 +185,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() { )), } ); + assert_eq!(request.cwd, Some(cwd)); } #[tokio::test]