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]