From 4be1ed7e8d4a32acb891e59559fc2ae72aec59ee Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 16 Apr 2026 17:12:38 -0700 Subject: [PATCH] sandboxing: intersect permission profiles semantically --- .../app-server/src/bespoke_event_handling.rs | 125 ++++++++++++++-- .../core/src/tools/handlers/apply_patch.rs | 1 + .../src/tools/handlers/apply_patch_tests.rs | 7 +- 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 + .../src/tools/handlers/unified_exec_tests.rs | 8 +- .../src/tools/runtimes/apply_patch_tests.rs | 8 +- .../runtimes/shell/unix_escalation_tests.rs | 8 +- .../core/tests/suite/request_permissions.rs | 134 +++++++++--------- .../tests/suite/request_permissions_tool.rs | 16 +-- codex-rs/sandboxing/src/policy_transforms.rs | 89 +++++++++++- .../sandboxing/src/policy_transforms_tests.rs | 72 +++++++++- 13 files changed, 365 insertions(+), 108 deletions(-) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 5d19bde70c..e11d5d9c11 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -2566,9 +2566,12 @@ async fn on_request_permissions_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 cwd = conversation.config_snapshot().await.cwd; + let Some(response) = request_permissions_response_from_client_result( + requested_permissions, + response, + cwd.as_path(), + ) else { return; }; @@ -2586,6 +2589,7 @@ async fn on_request_permissions_response( 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, @@ -2618,6 +2622,7 @@ fn request_permissions_response_from_client_result( permissions: intersect_permission_profiles( requested_permissions.into(), response.permissions.into(), + cwd, ) .into(), scope: response.scope.to_core(), @@ -2992,6 +2997,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; @@ -3676,6 +3685,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); @@ -3705,10 +3715,10 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), }; let cases = vec![ ( @@ -3735,10 +3745,10 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: None, - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + None, + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), @@ -3753,21 +3763,23 @@ mod tests { }, }), CoreRequestPermissionProfile { - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![absolute_path(input_path)]), - write: Some(vec![absolute_path(output_path)]), - }), + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(input_path)]), + Some(vec![absolute_path(output_path)]), + )), ..CoreRequestPermissionProfile::default() }, ), ]; + 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"); @@ -3789,6 +3801,7 @@ mod tests { "scope": "session", "permissions": {}, }))), + std::env::current_dir().expect("current dir").as_path(), ) .expect("response should be accepted"); @@ -3801,6 +3814,88 @@ 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, + }], + }), + ..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( + None, + Some(vec![child]), + )), + ..Default::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( + 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/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 59b464ea2f..248eeed637 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -112,6 +112,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/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 86e05fb8a1..e874dd5070 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -85,7 +85,12 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs(); assert_eq!( - permissions.and_then(|profile| profile.file_system.and_then(|fs| fs.write)), + permissions.and_then(|profile| { + profile.file_system.and_then(|fs| { + let (_, write) = fs.legacy_read_write_roots()?; + write + }) + }), Some(vec![expected_outside]) ); } diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index f436921e13..0791cb54e3 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 9aa700cd44..be095c0e8d 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -419,6 +419,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 99c7e4a195..02001d369a 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -227,6 +227,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/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index b641814516..0e9e6f1222 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -188,10 +188,10 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() - assert_eq!( args.additional_permissions, Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![expected_write.abs()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![expected_write.abs()]), + )), ..Default::default() }) ); diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 0ba3e131af..f838a43a7a 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -82,10 +82,10 @@ fn file_system_sandbox_context_uses_active_attempt() { .abs(); let additional_permissions = PermissionProfile { network: None, - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(Vec::new()), + )), }; let req = ApplyPatchRequest { action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()), diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index f11050f72b..855759f9bf 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -250,12 +250,12 @@ fn map_exec_result_preserves_stdout_and_stderr() { #[test] fn shell_request_escalation_execution_is_explicit() { let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![ AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), ]), - }), + )), ..Default::default() }; let sandbox_policy = SandboxPolicy::WorkspaceWrite { diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 2cfd1cf6f7..4b37ff4101 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -293,20 +293,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } @@ -343,10 +343,10 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let call_id = "request_permissions_skip_approval"; let command = "touch requested-dir/requested-but-unused.txt"; let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?; @@ -521,10 +521,10 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul let call_id = "request_permissions_relative_workdir"; let command = "touch relative-write.txt"; let expected_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![absolute_path(&nested_dir_canonical)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![absolute_path(&nested_dir_canonical)]), + )), ..Default::default() }; let event = shell_event_with_raw_request_permissions( @@ -624,10 +624,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd "cwd-widened", unrequested_write, unrequested_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -725,10 +725,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp "tmp-widened", tmp_write, tmp_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(&requested_write)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(&requested_write)]), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -824,19 +824,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "outside-cwd-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..RequestPermissionProfile::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -926,19 +926,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul "should-not-write", outside_write, outside_write ); let requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -1028,19 +1028,19 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul "sticky-grant-ok", outside_write, outside_write ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(outside_dir.path())]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(outside_dir.path())]), + )), ..Default::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from( + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from( outside_dir.path().canonicalize()?, )?]), - }), + )), ..Default::default() }; let responses = mount_sse_sequence( @@ -1492,35 +1492,35 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() ); let requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ absolute_path(first_dir.path()), absolute_path(second_dir.path()), ]), - }), + )), ..RequestPermissionProfile::default() }; let normalized_requested_permissions = RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..RequestPermissionProfile::default() }; let granted_permissions = normalized_directory_write_permissions(first_dir.path())?; let second_dir_permissions = requested_directory_write_permissions(second_dir.path()); let merged_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?, AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?, ]), - }), + )), ..Default::default() }; @@ -1584,16 +1584,20 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let approval_file_system = approval_permissions .file_system .unwrap_or_else(|| panic!("expected filesystem permissions")); - assert!(approval_file_system.read.as_ref().is_none_or(Vec::is_empty)); + let (approval_reads, approval_writes) = approval_file_system + .legacy_read_write_roots() + .unwrap_or_else(|| panic!("expected legacy-compatible permissions")); + assert!(approval_reads.as_ref().is_none_or(Vec::is_empty)); - let mut approval_writes = approval_file_system.write.unwrap_or_default(); + let mut approval_writes = approval_writes.unwrap_or_default(); approval_writes.sort_by_key(|path| path.display().to_string()); - let mut expected_writes = merged_permissions + let (_, expected_writes) = merged_permissions .file_system .unwrap_or_else(|| panic!("expected merged filesystem permissions")) - .write - .unwrap_or_default(); + .legacy_read_write_roots() + .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()); assert_eq!(approval_writes, expected_writes); diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index 14506f4a41..0578441e99 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -81,20 +81,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy { fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile { RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![absolute_path(path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![absolute_path(path)]), + )), ..RequestPermissionProfile::default() } } fn normalized_directory_write_permissions(path: &Path) -> Result { Ok(RequestPermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]), + )), ..RequestPermissionProfile::default() }) } diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index bcd02acd55..ede9b5a147 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -1,10 +1,12 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; 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::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; @@ -12,6 +14,8 @@ use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; use std::collections::HashSet; +use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Eq)] pub struct EffectiveSandboxPermissions { @@ -125,15 +129,26 @@ 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 = requested_file_system + let requested_policy = + FileSystemSandboxPolicy::restricted(requested_file_system.entries.clone()); + let entries = granted + .file_system + .unwrap_or_default() .entries .into_iter() - .filter(|entry| granted_file_system.entries.contains(entry)) + .filter(|entry| { + granted_file_system_entry_within_request( + &requested_file_system, + &requested_policy, + entry, + cwd, + ) + }) .collect(); FileSystemPermissions { entries } }) @@ -158,6 +173,74 @@ pub fn intersect_permission_profiles( } } +fn granted_file_system_entry_within_request( + requested: &FileSystemPermissions, + requested_policy: &FileSystemSandboxPolicy, + 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) { + 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 access_covers(requested: FileSystemAccessMode, granted: FileSystemAccessMode) -> bool { + match granted { + FileSystemAccessMode::Read => requested.can_read(), + FileSystemAccessMode::Write => requested.can_write(), + FileSystemAccessMode::None => false, + } +} + +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 3e16462ec8..b951d7a943 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -189,7 +189,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 ); } @@ -210,7 +210,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() ); } @@ -231,7 +231,73 @@ 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, + }], + }), + ..Default::default() + }; + let granted = PermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![child.clone()]), + )), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted.clone(), cwd.as_path()), + granted + ); +} + +#[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( + 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, + }], + }), + ..Default::default() + }; + + assert_eq!( + intersect_permission_profiles(requested, granted, cwd.as_path()), PermissionProfile::default() ); }