diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 810c5cb1d..f5b79084a 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -18,6 +18,7 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; +use crate::tools::handlers::apply_granted_turn_permissions; use crate::tools::handlers::parse_arguments; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::registry::ToolHandler; @@ -30,7 +31,10 @@ use crate::tools::spec::JsonSchema; use async_trait::async_trait; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; +use std::collections::BTreeSet; use std::sync::Arc; pub struct ApplyPatchHandler; @@ -61,6 +65,31 @@ fn to_abs_path(cwd: &Path, path: &Path) -> Option { AbsolutePathBuf::resolve_path_against_base(path, cwd).ok() } +fn write_permissions_for_paths(file_paths: &[AbsolutePathBuf]) -> Option { + let write_paths = file_paths + .iter() + .map(|path| { + path.parent() + .unwrap_or_else(|| path.clone()) + .into_path_buf() + }) + .collect::>() + .into_iter() + .map(AbsolutePathBuf::from_absolute_path) + .collect::, _>>() + .ok()?; + + let permissions = (!write_paths.is_empty()).then_some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(write_paths), + }), + ..Default::default() + })?; + + crate::sandboxing::normalize_additional_permissions(permissions).ok() +} + #[async_trait] impl ToolHandler for ApplyPatchHandler { fn kind(&self) -> ToolKind { @@ -119,6 +148,12 @@ impl ToolHandler for ApplyPatchHandler { InternalApplyPatchInvocation::DelegateToExec(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); let file_paths = file_paths_for_action(&apply.action); + let effective_additional_permissions = apply_granted_turn_permissions( + session.as_ref(), + crate::sandboxing::SandboxPermissions::UseDefault, + write_permissions_for_paths(&file_paths), + ) + .await; let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); let event_ctx = ToolEventCtx::new( @@ -134,6 +169,12 @@ impl ToolHandler for ApplyPatchHandler { file_paths, changes, exec_approval_requirement: apply.exec_approval_requirement, + sandbox_permissions: effective_additional_permissions + .sandbox_permissions, + additional_permissions: effective_additional_permissions + .additional_permissions, + permissions_preapproved: effective_additional_permissions + .permissions_preapproved, timeout_ms: None, codex_exe: turn.codex_linux_sandbox_exe.clone(), }; @@ -222,6 +263,12 @@ pub(crate) async fn intercept_apply_patch( InternalApplyPatchInvocation::DelegateToExec(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); let approval_keys = file_paths_for_action(&apply.action); + let effective_additional_permissions = apply_granted_turn_permissions( + session.as_ref(), + crate::sandboxing::SandboxPermissions::UseDefault, + write_permissions_for_paths(&approval_keys), + ) + .await; let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); let event_ctx = ToolEventCtx::new( session.as_ref(), @@ -236,6 +283,11 @@ pub(crate) async fn intercept_apply_patch( file_paths: approval_keys, changes, exec_approval_requirement: apply.exec_approval_requirement, + sandbox_permissions: effective_additional_permissions.sandbox_permissions, + additional_permissions: effective_additional_permissions + .additional_permissions, + permissions_preapproved: effective_additional_permissions + .permissions_preapproved, timeout_ms, codex_exe: turn.codex_linux_sandbox_exe.clone(), }; diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 516374179..8b14dd1af 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -23,6 +23,7 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::ReviewDecision; @@ -37,6 +38,9 @@ pub struct ApplyPatchRequest { pub file_paths: Vec, pub changes: std::collections::HashMap, pub exec_approval_requirement: ExecApprovalRequirement, + pub sandbox_permissions: SandboxPermissions, + pub additional_permissions: Option, + pub permissions_preapproved: bool, pub timeout_ms: Option, pub codex_exe: Option, } @@ -87,8 +91,8 @@ impl ApplyPatchRuntime { expiration: req.timeout_ms.into(), // Run apply_patch with a minimal environment for determinism and to avoid leaks. env: HashMap::new(), - sandbox_permissions: SandboxPermissions::UseDefault, - additional_permissions: None, + sandbox_permissions: req.sandbox_permissions, + additional_permissions: req.additional_permissions.clone(), justification: None, }) } @@ -134,6 +138,9 @@ impl Approvable for ApplyPatchRuntime { let action = ApplyPatchRuntime::build_guardian_review_request(req); return review_approval_request(session, turn, action, retry_reason).await; } + if req.permissions_preapproved && retry_reason.is_none() { + return ReviewDecision::Approved; + } if let Some(reason) = retry_reason { let rx_approve = session .request_patch_approval(turn, call_id, changes.clone(), Some(reason), None) @@ -244,6 +251,9 @@ mod tests { reason: None, proposed_execpolicy_amendment: None, }, + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + permissions_preapproved: false, timeout_ms: None, codex_exe: None, }; diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index e6c3c4487..8f99a9a0f 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -15,6 +15,7 @@ use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -60,6 +61,14 @@ fn exec_command_event(call_id: &str, command: &str) -> Result { Ok(ev_function_call(call_id, "exec_command", &args_str)) } +fn build_add_file_patch(patch_path: &Path, content: &str) -> String { + format!( + "*** Begin Patch\n*** Add File: {}\n+{}\n*** End Patch\n", + patch_path.display(), + content + ) +} + fn workspace_write_excluding_tmp() -> SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots: vec![], @@ -291,3 +300,121 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s Ok(()) } + +#[tokio::test(flavor = "current_thread")] +#[cfg(target_os = "macos")] +async fn approved_folder_write_request_permissions_unblocks_later_apply_patch_without_prompt() +-> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::OnRequest; + let sandbox_policy = workspace_write_excluding_tmp(); + let sandbox_policy_for_config = sandbox_policy.clone(); + + let mut builder = test_codex().with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config + .features + .enable(Feature::RequestPermissions) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::RequestPermissionsTool) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + let requested_dir = tempfile::tempdir()?; + let requested_file = requested_dir.path().join("allowed-patch.txt"); + let requested_permissions = requested_directory_write_permissions(requested_dir.path()); + let normalized_requested_permissions = + normalized_directory_write_permissions(requested_dir.path())?; + let patch = build_add_file_patch(&requested_file, "patched-via-request-permissions"); + + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-request-permissions-patch-1"), + request_permissions_tool_event( + "permissions-call", + "Allow patching outside the workspace", + &requested_permissions, + )?, + ev_completed("resp-request-permissions-patch-1"), + ]), + sse(vec![ + ev_response_created("resp-request-permissions-patch-2"), + ev_apply_patch_function_call("apply-patch-call", &patch), + ev_completed("resp-request-permissions-patch-2"), + ]), + sse(vec![ + ev_response_created("resp-request-permissions-patch-3"), + ev_assistant_message("msg-request-permissions-patch-1", "done"), + ev_completed("resp-request-permissions-patch-3"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "patch outside the workspace", + approval_policy, + sandbox_policy, + ) + .await?; + + let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await; + assert_eq!( + granted_permissions, + normalized_requested_permissions.clone() + ); + test.codex + .submit(Op::RequestPermissionsResponse { + id: "permissions-call".to_string(), + response: RequestPermissionsResponse { + permissions: normalized_requested_permissions, + scope: PermissionGrantScope::Turn, + }, + }) + .await?; + + let event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + match event { + EventMsg::TurnComplete(_) => {} + EventMsg::ApplyPatchApprovalRequest(approval) => { + panic!( + "unexpected apply_patch approval request after granted permissions: {:?}", + approval.call_id + ) + } + other => panic!("unexpected event: {other:?}"), + } + + let patch_output = responses + .function_call_output_text("apply-patch-call") + .map(|output| json!({ "output": output })) + .unwrap_or_else(|| panic!("expected apply-patch-call output")); + let (exit_code, stdout) = parse_result(&patch_output); + assert!(exit_code.is_none() || exit_code == Some(0)); + assert!( + stdout.contains("Success."), + "unexpected patch output: {stdout}" + ); + assert_eq!( + fs::read_to_string(&requested_file)?, + "patched-via-request-permissions\n" + ); + + Ok(()) +}