From 7a7c188682c3f3aae6c7efabf3acd1a9c7dfd3e6 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 3 Sep 2026 16:10:18 +0000 Subject: [PATCH] Preserve target-native paths in command approvals (#42577) ## What changed - Pass the selected executor's `PathUri` through user command approval requests instead of converting the working directory to a host-native path. - Make native-path permission helpers explicitly local and require executor-aware permission transforms to receive a `FileSystemSandboxPolicyContext`. - Cover approved remote commands to verify that the approval request reports the selected remote working directory and execution resumes after approval. GitOrigin-RevId: 0aa3042f2d7594af6c7d0abf31d7f99cf67d5556 --- .../request_processors/thread_processor.rs | 2 +- codex-rs/core/src/config/config_tests.rs | 60 ++--- codex-rs/core/src/config/mod.rs | 9 +- codex-rs/core/src/config/permissions_tests.rs | 12 +- codex-rs/core/src/session/tests.rs | 4 +- codex-rs/core/src/tools/approvals.rs | 11 +- codex-rs/core/src/tools/handlers/mod.rs | 10 +- codex-rs/core/tests/suite/cloud_config.rs | 2 +- codex-rs/core/tests/suite/remote_env.rs | 47 +++- codex-rs/exec-server/src/fs_sandbox.rs | 26 ++- codex-rs/exec/src/lib.rs | 2 +- .../src/permission_profile_intersection.rs | 13 +- .../permission_profile_intersection_tests.rs | 22 +- codex-rs/protocol/src/permissions.rs | 214 ++++++------------ codex-rs/sandboxing/src/manager.rs | 6 +- codex-rs/sandboxing/src/policy_transforms.rs | 69 ------ .../sandboxing/src/policy_transforms_tests.rs | 88 +++++-- codex-rs/sandboxing/src/seatbelt.rs | 2 +- codex-rs/sandboxing/src/windows.rs | 10 +- codex-rs/tui/src/additional_dirs.rs | 2 +- codex-rs/tui/src/app_server_session.rs | 2 +- .../tui/src/chatwidget/permission_popups.rs | 2 +- .../src/chatwidget/windows_sandbox_prompts.rs | 2 +- codex-rs/tui/src/external_editor.rs | 5 +- codex-rs/tui/src/external_editor_tests.rs | 28 +-- codex-rs/tui/src/inline_visualization.rs | 4 +- .../src/deny_read_resolver.rs | 8 +- .../src/deny_read_resolver_access_tests.rs | 4 +- .../src/deny_read_walker.rs | 4 +- .../src/resolved_permissions.rs | 2 +- 30 files changed, 304 insertions(+), 368 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 2549be0d60..fe4bec3135 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -1329,7 +1329,7 @@ impl ThreadRequestProcessor { codex_protocol::models::PermissionProfile::Managed { .. } => { effective_permission_profile .file_system_sandbox_policy() - .can_write_path_with_cwd(config.cwd.as_path(), config.cwd.as_path()) + .can_write_local_path_with_cwd(config.cwd.as_path(), config.cwd.as_path()) } }; diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index b1bb7ecddf..83d1ec5e79 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2383,7 +2383,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: !config .permissions .file_system_sandbox_policy() - .can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()) + .can_write_local_path_with_cwd(&cwd.path().join(".git"), cwd.path()) ); assert_eq!( config.permissions.network_sandbox_policy(), @@ -2828,7 +2828,7 @@ async fn permission_profile_override_keeps_memories_root_out_of_legacy_projectio !config .permissions .file_system_sandbox_policy() - .can_write_path_with_cwd(memories_root.as_path(), cwd.path()) + .can_write_local_path_with_cwd(memories_root.as_path(), cwd.path()) ); assert_eq!( &config.legacy_sandbox_policy(), @@ -3064,11 +3064,11 @@ async fn default_permissions_can_select_builtin_profile_without_permissions_tabl Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE) ); assert!( - policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected :workspace to allow writing the project root, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()), + !policy.can_write_local_path_with_cwd(&cwd.path().join(".git"), cwd.path()), "expected :workspace to protect project metadata, policy: {policy:?}" ); Ok(()) @@ -3097,7 +3097,7 @@ async fn default_permissions_read_only_keeps_add_dir_read_only() -> std::io::Res let policy = config.permissions.file_system_sandbox_policy(); assert!( - !policy.can_write_path_with_cwd(extra_root.as_path(), cwd.path()), + !policy.can_write_local_path_with_cwd(extra_root.as_path(), cwd.path()), "expected :read-only to stay read-only for runtime workspace roots, policy: {policy:?}" ); assert_eq!( @@ -3186,15 +3186,15 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( let policy = config.permissions.file_system_sandbox_policy(); for root in [cwd_abs, runtime_root_abs, profile_root_abs.clone()] { assert!( - policy.can_write_path_with_cwd(root.as_path(), cwd.as_path()), + policy.can_write_local_path_with_cwd(root.as_path(), cwd.as_path()), "expected workspace root to be writable, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&root.join(".git"), cwd.as_path()), + !policy.can_write_local_path_with_cwd(&root.join(".git"), cwd.as_path()), "expected .git carveout under {root:?}, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&root.join(".codex"), cwd.as_path()), + !policy.can_write_local_path_with_cwd(&root.join(".codex"), cwd.as_path()), "expected .codex carveout under {root:?}, policy: {policy:?}" ); } @@ -3293,11 +3293,11 @@ async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io:: let policy = config.permissions.file_system_sandbox_policy(); assert!( - policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected profile extending :workspace to keep project-root writes, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()), + !policy.can_write_local_path_with_cwd(&cwd.path().join(".git"), cwd.path()), "expected profile extending :workspace to keep metadata carveouts, policy: {policy:?}" ); assert!( @@ -3388,11 +3388,11 @@ async fn default_permissions_profile_can_extend_builtin_read_only() -> std::io:: let policy = config.permissions.file_system_sandbox_policy(); assert!( - policy.can_read_path_with_cwd(cwd.path(), cwd.path()), + policy.can_read_local_path_with_cwd(cwd.path(), cwd.path()), "expected profile extending :read-only to keep read access, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + !policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected profile extending :read-only to stay non-writable, policy: {policy:?}" ); assert_eq!( @@ -3448,16 +3448,16 @@ async fn empty_config_defaults_to_builtin_profile_for_trusted_project() -> std:: ); if cfg!(target_os = "windows") { assert!( - !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + !policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected trusted project fallback to stay read-only without Windows sandbox support, policy: {policy:?}" ); } else { assert!( - policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected trusted project fallback to use :workspace, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&cwd.path().join(".codex"), cwd.path()), + !policy.can_write_local_path_with_cwd(&cwd.path().join(".codex"), cwd.path()), "expected :workspace metadata carveouts, policy: {policy:?}" ); } @@ -3502,21 +3502,21 @@ async fn empty_config_defaults_to_builtin_profile_for_untrusted_project() -> std }) ); assert!( - policy.can_read_path_with_cwd(cwd.path(), cwd.path()), + policy.can_read_local_path_with_cwd(cwd.path(), cwd.path()), "expected untrusted project fallback to allow reads, policy: {policy:?}" ); if cfg!(target_os = "windows") { assert!( - !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + !policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected untrusted project fallback to stay read-only without Windows sandbox support, policy: {policy:?}" ); } else { assert!( - policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected untrusted project fallback to use :workspace, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(&cwd.path().join(".codex"), cwd.path()), + !policy.can_write_local_path_with_cwd(&cwd.path().join(".codex"), cwd.path()), "expected :workspace metadata carveouts, policy: {policy:?}" ); } @@ -3562,7 +3562,7 @@ async fn implicit_builtin_workspace_profile_preserves_sandbox_workspace_write_se let policy = config.permissions.file_system_sandbox_policy(); assert!( - policy.can_write_path_with_cwd(extra_root.as_path(), cwd.path()), + policy.can_write_local_path_with_cwd(extra_root.as_path(), cwd.path()), "expected implicit :workspace to preserve sandbox_workspace_write.writable_roots, policy: {policy:?}" ); assert_eq!( @@ -3629,12 +3629,12 @@ async fn implicit_builtin_workspace_profile_preserves_add_dir_metadata_carveouts let policy = config.permissions.file_system_sandbox_policy(); let extra_root = extra_root.path().abs(); assert!( - policy.can_write_path_with_cwd(extra_root.as_path(), cwd.path()), + policy.can_write_local_path_with_cwd(extra_root.as_path(), cwd.path()), "expected implicit :workspace to preserve additional writable roots, policy: {policy:?}" ); for subpath in [".git", ".agents", ".codex"] { assert!( - !policy.can_write_path_with_cwd(&extra_root.join(subpath), cwd.path()), + !policy.can_write_local_path_with_cwd(&extra_root.join(subpath), cwd.path()), "expected implicit :workspace to preserve legacy metadata carveout for {subpath}, \ policy: {policy:?}" ); @@ -3660,11 +3660,11 @@ async fn empty_config_defaults_to_builtin_read_only_without_trust_decision() -> let policy = config.permissions.file_system_sandbox_policy(); assert!( - policy.can_read_path_with_cwd(cwd.path(), cwd.path()), + policy.can_read_local_path_with_cwd(cwd.path(), cwd.path()), "expected :read-only to allow reads, policy: {policy:?}" ); assert!( - !policy.can_write_path_with_cwd(cwd.path(), cwd.path()), + !policy.can_write_local_path_with_cwd(cwd.path(), cwd.path()), "expected :read-only to deny writes, policy: {policy:?}" ); Ok(()) @@ -3842,7 +3842,7 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() config .permissions .file_system_sandbox_policy() - .can_write_path_with_cwd(external_write_path.as_path(), cwd.path()) + .can_write_local_path_with_cwd(external_write_path.as_path(), cwd.path()) ); assert_eq!( &config.legacy_sandbox_policy(), @@ -6035,8 +6035,12 @@ async fn memory_tool_makes_memories_root_readable_without_creating_or_widening_w memories_root.display() ); let file_system_policy = config.permissions.file_system_sandbox_policy(); - assert!(file_system_policy.can_read_path_with_cwd(memories_root_abs.as_path(), cwd.path())); - assert!(!file_system_policy.can_write_path_with_cwd(memories_root_abs.as_path(), cwd.path())); + assert!( + file_system_policy.can_read_local_path_with_cwd(memories_root_abs.as_path(), cwd.path()) + ); + assert!( + !file_system_policy.can_write_local_path_with_cwd(memories_root_abs.as_path(), cwd.path()) + ); if cfg!(target_os = "windows") { match &config.legacy_sandbox_policy() { @@ -10975,7 +10979,7 @@ async fn permission_profile_override_preserves_split_write_roots() -> std::io::R config .permissions .file_system_sandbox_policy() - .can_write_path_with_cwd(outside_root.as_path(), config.cwd.as_path()) + .can_write_local_path_with_cwd(outside_root.as_path(), config.cwd.as_path()) ); assert!(matches!( &config.legacy_sandbox_policy(), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 748ef9b2a0..0ae1b06cb1 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -4091,17 +4091,14 @@ impl Config { }) = filesystem_requirements.as_ref() && let Some(managed_file_system_policy) = managed_deny_read_policy.as_ref() { - let _initial_matcher = - ReadDenyMatcher::try_new(managed_file_system_policy, resolved_cwd.as_path()) + let managed_deny_matcher = + ReadDenyMatcher::try_new_for_local_paths(managed_file_system_policy, resolved_cwd.as_path()) .map_err(std::io::Error::other)?; let managed_file_system_policy = Arc::clone(managed_file_system_policy); - let permission_cwd = resolved_cwd.clone(); let requirement_source = requirement_source.clone(); constrained_permission_profile .value .add_validator(move |permission_profile| { - let managed_deny_matcher = - ReadDenyMatcher::new(&managed_file_system_policy, permission_cwd.as_path()); let file_system_policy = permission_profile.file_system_sandbox_policy(); let missing_required_deny = managed_file_system_policy .entries @@ -4118,7 +4115,7 @@ impl Config { let path = path.to_abs_path().ok()?; managed_deny_matcher .as_ref() - .is_some_and(|matcher| matcher.is_read_denied(path.as_path())) + .is_some_and(|matcher| matcher.is_local_path_read_denied(path.as_path())) .then_some(path) }); if missing_required_deny || violating_root.is_some() { diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index c87f5fea32..abc185ba18 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -96,15 +96,15 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu let policy = config.permissions.file_system_sandbox_policy(); assert!( - policy.can_read_path_with_cwd(expected_zsh.as_path(), &cwd), + policy.can_read_local_path_with_cwd(expected_zsh.as_path(), &cwd), "expected zsh helper path to be readable, policy: {policy:?}" ); assert!( - policy.can_read_path_with_cwd(expected_allowed_arg0_dir.as_path(), &cwd), + policy.can_read_local_path_with_cwd(expected_allowed_arg0_dir.as_path(), &cwd), "expected active arg0 helper dir to be readable, policy: {policy:?}" ); assert!( - !policy.can_read_path_with_cwd(expected_sibling_arg0_dir.as_path(), &cwd), + !policy.can_read_local_path_with_cwd(expected_sibling_arg0_dir.as_path(), &cwd), "expected sibling arg0 helper dir to remain unreadable, policy: {policy:?}" ); @@ -493,7 +493,7 @@ docs = "read" let (read_deny_policy, _) = compile_permission_profile(&permissions, "read_deny", &mut startup_warnings)?; assert_eq!( - read_deny_policy.resolve_access_with_cwd(cwd.path(), cwd.path()), + read_deny_policy.resolve_access_for_local_path_with_cwd(cwd.path(), cwd.path()), FileSystemAccessMode::Deny ); @@ -501,7 +501,7 @@ docs = "read" compile_permission_profile(&permissions, "write_deny", &mut startup_warnings)?; assert!(!write_deny_policy.has_full_disk_write_access()); assert_eq!( - write_deny_policy.resolve_access_with_cwd(cwd.path(), cwd.path()), + write_deny_policy.resolve_access_for_local_path_with_cwd(cwd.path(), cwd.path()), FileSystemAccessMode::Deny ); @@ -509,7 +509,7 @@ docs = "read" compile_permission_profile(&permissions, "write_read", &mut startup_warnings)?; assert!(!write_read_policy.has_full_disk_write_access()); assert_eq!( - write_read_policy.resolve_access_with_cwd(&docs, cwd.path()), + write_read_policy.resolve_access_for_local_path_with_cwd(&docs, cwd.path()), FileSystemAccessMode::Read ); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ffac44c011..1d30b0c219 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5894,13 +5894,13 @@ async fn session_configuration_apply_preserves_absolute_cwd_write_root_on_cwd_up assert!( updated .file_system_sandbox_policy(&[]) - .can_write_path_with_cwd(original_cwd.as_path(), updated.cwd().as_path()), + .can_write_local_path_with_cwd(original_cwd.as_path(), updated.cwd().as_path()), "absolute grant to the old cwd must remain writable" ); assert!( !updated .file_system_sandbox_policy(&[]) - .can_write_path_with_cwd(next_cwd.as_path(), updated.cwd().as_path()), + .can_write_local_path_with_cwd(next_cwd.as_path(), updated.cwd().as_path()), "cwd-only update must not reinterpret an absolute old-cwd grant as :workspace_roots" ); } diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 5da0a46c74..13b5f5d870 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -708,15 +708,6 @@ impl Session { proposed_execpolicy_amendment, .. } => { - let cwd = match guardian_cwd(environment_id, cwd.clone()) { - Ok(cwd) => cwd, - Err(err) => { - tracing::error!(%err, "failed to resolve approval command cwd"); - return ReviewDecision::denied(format!( - "failed to resolve approval command cwd: {err}" - )); - } - }; let tool_name = "unified_exec"; let reason = ctx .retry_reason @@ -743,7 +734,7 @@ impl Session { /*approval_id*/ None, Some(environment_id.clone()), command.clone(), - cwd.into(), + cwd.clone(), reason, ctx.network_approval_context.clone(), proposed_execpolicy_amendment.clone(), diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 4ce3be4a24..ce09df5b33 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -396,7 +396,7 @@ mod tests { use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; - use codex_sandboxing::policy_transforms::intersect_permission_profiles; + use codex_sandboxing::policy_transforms::intersect_permission_profiles_with_context; use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -538,21 +538,21 @@ mod tests { }), ..Default::default() }; - let stored_grant = intersect_permission_profiles( + let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI"); + let stored_grant = intersect_permission_profiles_with_context( requested_permissions.clone(), requested_permissions.clone(), - cwd.path(), + &local_context(&cwd_uri), ); let effective_permissions = merge_permission_profiles(Some(&requested_permissions), Some(&stored_grant)) .expect("merged permissions"); - let cwd = PathUri::from_host_native_path(cwd.path()).expect("cwd URI"); assert_eq!( preapproved_permission_profile( &effective_permissions, &stored_grant, - &local_context(&cwd), + &local_context(&cwd_uri), ), Some(stored_grant) ); diff --git a/codex-rs/core/tests/suite/cloud_config.rs b/codex-rs/core/tests/suite/cloud_config.rs index a8d49a69ab..60bcc49617 100644 --- a/codex-rs/core/tests/suite/cloud_config.rs +++ b/codex-rs/core/tests/suite/cloud_config.rs @@ -121,7 +121,7 @@ async fn managed_deny_read_requirements_follow_thread_permission_updates() -> Re !snapshot .permission_profile .file_system_sandbox_policy() - .can_read_path_with_cwd(nested_root.as_path(), snapshot.cwd().as_path()), + .can_read_local_path_with_cwd(nested_root.as_path(), snapshot.cwd().as_path()), "a live permission change must preserve managed deny-read rules" ); diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 532bad26f9..ab764061a0 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -383,7 +383,7 @@ async fn remote_test_env_exposes_target_shell_and_exec_guidance_to_model() -> Re } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { +async fn approved_remote_shell_runs_in_remote_cwd() -> Result<()> { const CALL_ID: &str = "remote-explicit-shell"; skip_if_no_remote_env!(Ok(())); @@ -406,9 +406,15 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { "shell": shell, "login": false, "yield_time_ms": 10_000, + "sandbox_permissions": SandboxPermissions::RequireEscalated, + "justification": "Test target-native command approval cwd.", }))?; - let mut builder = test_codex(); + let mut builder = test_codex().with_config(|config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.approvals_reviewer = ApprovalsReviewer::User; + }); let test = builder.build_with_auto_env(&server).await?; + let selection = test.executor_environment().selection().clone(); let response_mock = mount_sse_sequence( &server, vec![ @@ -426,16 +432,39 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { ) .await; - test.submit_turn_with_environments( + submit_turn_with_approval_and_environments( + &test, "run the remote shell in the remote cwd", - Some(vec![TurnEnvironmentSelection { - environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: test.executor_environment().selection().cwd.clone(), - workspace_roots: vec![test.executor_environment().selection().cwd.clone()], - config: EnvironmentConfigState::FromThread, - }]), + vec![selection.clone()], + AskForApproval::OnRequest, ) .await?; + + let event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + let EventMsg::ExecApprovalRequest(approval) = event else { + panic!("expected remote command approval before completion: {event:?}"); + }; + assert_eq!( + approval.cwd.to_inferred_path_uri().as_ref(), + Some(&selection.cwd) + ); + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; let request = response_mock .last_request() .context("model should receive the command output")?; diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index af0a4b2c9b..98cf5dfb4c 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -233,7 +233,7 @@ fn add_helper_runtime_permissions( } for helper_read_root in helper_read_roots { - if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) { + if file_system_policy.can_read_local_path_with_cwd(helper_read_root.as_path(), cwd) { continue; } @@ -571,8 +571,8 @@ mod tests { cwd.as_path(), ); - assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path())); - assert!(policy.can_write_path_with_cwd(writable.as_path(), cwd.as_path())); + assert!(policy.can_read_local_path_with_cwd(readable.as_path(), cwd.as_path())); + assert!(policy.can_write_local_path_with_cwd(writable.as_path(), cwd.as_path())); } #[test] @@ -800,10 +800,13 @@ mod tests { ); assert!( - policy.can_read_path_with_cwd(runtime_paths.codex_self_exe.as_path(), cwd.as_path()) + policy.can_read_local_path_with_cwd( + runtime_paths.codex_self_exe.as_path(), + cwd.as_path(), + ) ); - assert!(!policy.can_read_path_with_cwd(parent.as_path(), cwd.as_path())); - assert!(!policy.can_read_path_with_cwd(sibling.as_path(), cwd.as_path())); + assert!(!policy.can_read_local_path_with_cwd(parent.as_path(), cwd.as_path())); + assert!(!policy.can_read_local_path_with_cwd(sibling.as_path(), cwd.as_path())); } #[test] @@ -831,11 +834,14 @@ mod tests { ); assert!( - policy.can_read_path_with_cwd(runtime_paths.codex_self_exe.as_path(), cwd.as_path()) + policy.can_read_local_path_with_cwd( + runtime_paths.codex_self_exe.as_path(), + cwd.as_path(), + ) ); - assert!(policy.can_read_path_with_cwd(alias.as_path(), cwd.as_path())); - assert!(!policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path())); - assert!(!policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path())); + assert!(policy.can_read_local_path_with_cwd(alias.as_path(), cwd.as_path())); + assert!(!policy.can_read_local_path_with_cwd(codex_parent.as_path(), cwd.as_path())); + assert!(!policy.can_read_local_path_with_cwd(alias_parent.as_path(), cwd.as_path())); } fn restricted_policy(entries: Vec) -> FileSystemSandboxPolicy { diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index d643975084..aa03a05a9b 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1263,7 +1263,7 @@ fn sandbox_mode_from_permission_profile( .network_sandbox_policy() .is_enabled() .then_some(codex_app_server_protocol::SandboxMode::DangerFullAccess) - } else if file_system_policy.can_write_path_with_cwd(cwd, cwd) { + } else if file_system_policy.can_write_local_path_with_cwd(cwd, cwd) { Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite) } else { Some(codex_app_server_protocol::SandboxMode::ReadOnly) diff --git a/codex-rs/protocol/src/permission_profile_intersection.rs b/codex-rs/protocol/src/permission_profile_intersection.rs index 5ee6a1578a..9f29709bb0 100644 --- a/codex-rs/protocol/src/permission_profile_intersection.rs +++ b/codex-rs/protocol/src/permission_profile_intersection.rs @@ -143,9 +143,9 @@ pub fn intersect_effective_permission_profiles( policy.entries.retain(|entry| entry.path != tmpdir); } - let authority_denies = ReadDenyMatcher::try_new(&authority_policy, cwd) + let authority_denies = ReadDenyMatcher::try_new_for_local_paths(&authority_policy, cwd) .map_err(PermissionIntersectionError::UnsupportedPath)?; - let requested_denies = ReadDenyMatcher::try_new(&requested_policy, cwd) + let requested_denies = ReadDenyMatcher::try_new_for_local_paths(&requested_policy, cwd) .map_err(PermissionIntersectionError::UnsupportedPath)?; let sources = [ (&authority_policy, authority_denies.as_ref()), @@ -214,7 +214,8 @@ pub fn intersect_effective_permission_profiles( denies.sort_by_cached_key(|entry| format!("{:?}", entry.path)); let mut intersection = FileSystemSandboxPolicy::restricted(denies); intersection.glob_scan_max_depth = glob_depth.flatten(); - let intersection_denies = ReadDenyMatcher::new(&intersection, cwd); + let intersection_denies = ReadDenyMatcher::try_new_for_local_paths(&intersection, cwd) + .map_err(PermissionIntersectionError::UnsupportedPath)?; for path in paths { let [left, right] = @@ -395,11 +396,11 @@ fn effective_access( path: &Path, cwd: &Path, ) -> FileSystemAccessMode { - if denies.is_some_and(|matcher| matcher.is_read_denied(path)) { + if denies.is_some_and(|matcher| matcher.is_local_path_read_denied(path)) { FileSystemAccessMode::Deny - } else if policy.can_write_path_with_cwd(path, cwd) { + } else if policy.can_write_local_path_with_cwd(path, cwd) { FileSystemAccessMode::Write - } else if policy.can_read_path_with_cwd(path, cwd) { + } else if policy.can_read_local_path_with_cwd(path, cwd) { FileSystemAccessMode::Read } else { FileSystemAccessMode::Deny diff --git a/codex-rs/protocol/src/permission_profile_intersection_tests.rs b/codex-rs/protocol/src/permission_profile_intersection_tests.rs index 6245c79713..0b14f14c56 100644 --- a/codex-rs/protocol/src/permission_profile_intersection_tests.rs +++ b/codex-rs/protocol/src/permission_profile_intersection_tests.rs @@ -117,14 +117,15 @@ fn effective_workspace_intersection_preserves_network_metadata_and_temp() { assert_eq!( [&root, &project] - .map(|path| policy.resolve_access_with_cwd(path.as_path(), root.as_path())), + .map(|path| policy + .resolve_access_for_local_path_with_cwd(path.as_path(), root.as_path())), [Read, Write] ); assert_eq!(result.network_sandbox_policy(), Restricted); assert!(policy.entries.contains(&special(Tmpdir, Write))); for name in [".git", ".agents", ".codex"] { let protected = project.join(name); - assert!(!policy.can_write_path_with_cwd(protected.as_path(), root.as_path())); + assert!(!policy.can_write_local_path_with_cwd(protected.as_path(), root.as_path())); assert!(policy.entries.contains(&skipped(protected.into(), Read))); } assert!(policy.entries.contains(&skipped(gitdir.into(), Read))); @@ -177,10 +178,13 @@ fn exact_denies_nested_read_carveouts_and_reopened_writes_are_preserved() { let right = PermissionProfile::from_runtime_permissions(&right_policy, Restricted); let result = intersection(&left, &right, &root); let policy = result.file_system_sandbox_policy(); - let denies = ReadDenyMatcher::new(&policy, root.as_path()).expect("merged denies"); + let denies = ReadDenyMatcher::try_new_for_local_paths(&policy, root.as_path()) + .expect("valid merged deny globs") + .expect("merged denies"); assert_eq!( [&root, &shared, &editable] - .map(|path| policy.resolve_access_with_cwd(path.as_path(), root.as_path())), + .map(|path| policy + .resolve_access_for_local_path_with_cwd(path.as_path(), root.as_path())), [Write, Read, Write] ); assert_eq!( @@ -190,7 +194,7 @@ fn exact_denies_nested_read_carveouts_and_reopened_writes_are_preserved() { &root.join("credentials.env"), &root.join("credentials.token"), ] - .map(|path| denies.is_read_denied(path.as_path())), + .map(|path| denies.is_local_path_read_denied(path.as_path())), [true, true, true, true] ); assert_eq!(policy.glob_scan_max_depth, Some(4)); @@ -317,7 +321,7 @@ fn canonical_grants_cannot_escape_through_readable_or_writable_symlinks() { let parent = rooted(&root, Write, []); let child = rooted(&escaped, Write, []); let policy = intersection(&parent, &child, &root).file_system_sandbox_policy(); - assert!(!policy.can_write_path_with_cwd(outside.as_path(), root.as_path())); + assert!(!policy.can_write_local_path_with_cwd(outside.as_path(), root.as_path())); let parent = managed(vec![entry(root.as_path(), Read)]); let child = managed(vec![entry(escaped.as_path(), Read)]); @@ -329,7 +333,7 @@ fn canonical_grants_cannot_escape_through_readable_or_writable_symlinks() { let parent = rooted(&root, Write, []); let child = rooted(&internal_alias, Write, []); let policy = intersection(&parent, &child, &root).file_system_sandbox_policy(); - assert!(policy.can_write_path_with_cwd(inside.as_path(), root.as_path())); + assert!(policy.can_write_local_path_with_cwd(inside.as_path(), root.as_path())); assert!( !policy .entries @@ -355,7 +359,7 @@ fn macos_system_path_aliases_share_the_same_physical_permissions() { let right = managed(vec![entry(canonical.as_path(), access)]); let policy = intersection(&left, &right, &canonical).file_system_sandbox_policy(); assert_eq!( - policy.resolve_access_with_cwd(canonical.as_path(), canonical.as_path()), + policy.resolve_access_for_local_path_with_cwd(canonical.as_path(), canonical.as_path()), access ); } @@ -365,5 +369,5 @@ fn macos_system_path_aliases_share_the_same_physical_permissions() { &canonical, ) .file_system_sandbox_policy(); - assert!(policy.can_write_path_with_cwd(canonical.as_path(), canonical.as_path())); + assert!(policy.can_write_local_path_with_cwd(canonical.as_path(), canonical.as_path())); } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 2e02b851cd..d5c6c4cada 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -325,28 +325,12 @@ impl ReadDenyMatcher { .flatten() } - /// Builds a matcher from exact deny-read roots and deny-read glob entries. - /// - /// Returns `None` when the policy has no deny-read restrictions, so callers - /// can skip read-deny checks without allocating matcher state. The `cwd` - /// resolves cwd-relative policy paths and special paths before matching. - pub fn new(file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path) -> Option { - match Self::build( - file_system_sandbox_policy, - cwd, - InvalidDenyReadGlobBehavior::FailClosed, - ) { - Ok(matcher) => matcher, - Err(_) => unreachable!("fail-closed glob handling does not return errors"), - } - } - - /// Builds a matcher for callers that must reject malformed glob patterns. + /// Builds a local-path matcher for callers that must reject malformed glob patterns. /// /// Runtime read checks intentionally fail closed on malformed deny patterns. /// Host-side expansion work should use this constructor instead so a typo /// cannot broaden the set of paths it mutates before execution starts. - pub fn try_new( + pub fn try_new_for_local_paths( file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, ) -> Result, String> { @@ -386,8 +370,8 @@ impl ReadDenyMatcher { })) } - /// Returns whether `path` is denied by the policy used to build this matcher. - pub fn is_read_denied(&self, path: &Path) -> bool { + /// Returns whether a local native `path` is denied by this matcher. + pub fn is_local_path_read_denied(&self, path: &Path) -> bool { let Some(cwd) = self.native_cwd.as_ref() else { return true; }; @@ -421,8 +405,12 @@ impl ReadDenyMatcher { /// parent and a non-symlink directory entry. Symlinks and Windows junctions /// must be resolved separately. Do not reuse these locations across walks: /// a later operation must observe newly created files and changed links. - pub fn is_read_denied_with_canonical_path(&self, path: &Path, canonical_path: &Path) -> bool { - self.is_read_denied(path) || self.is_read_denied(canonical_path) + pub fn is_local_path_read_denied_with_canonical_path( + &self, + path: &Path, + canonical_path: &Path, + ) -> bool { + self.is_local_path_read_denied(path) || self.is_local_path_read_denied(canonical_path) } } @@ -641,7 +629,14 @@ impl FileSystemSandboxPolicy { self.entries.retain(|entry| !entry.skips_missing_path()); } - pub fn has_explicit_non_write_entry_for_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { + /// Native-path compatibility adapter for local executor boundaries. + /// Selected-executor orchestrators must keep paths as [`PathUri`] and use + /// context-aware policy APIs instead. + pub fn has_explicit_non_write_entry_for_local_path_with_cwd( + &self, + path: &Path, + cwd: &Path, + ) -> bool { let Some(path) = resolve_candidate_path(path, cwd) else { return false; }; @@ -930,18 +925,28 @@ impl FileSystemSandboxPolicy { }) } - pub fn resolve_access_with_cwd(&self, path: &Path, cwd: &Path) -> FileSystemAccessMode { + /// Native-path compatibility adapter for local executor boundaries. + /// Selected-executor orchestrators must use [`Self::resolve_access`] with + /// a [`PathUri`] and [`FileSystemSandboxPolicyContext`] instead. + pub fn resolve_access_for_local_path_with_cwd( + &self, + path: &Path, + cwd: &Path, + ) -> FileSystemAccessMode { with_local_policy_context(path, cwd, |path, context| { self.resolve_access(path, context) }) .unwrap_or(FileSystemAccessMode::Deny) } - pub fn can_read_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { - self.resolve_access_with_cwd(path, cwd).can_read() + /// Native-path compatibility adapter for local executor boundaries. + pub fn can_read_local_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { + self.resolve_access_for_local_path_with_cwd(path, cwd) + .can_read() } - pub fn can_write_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { + /// Native-path compatibility adapter for local executor boundaries. + pub fn can_write_local_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { with_local_policy_context(path, cwd, |path, context| { self.can_write_path(path, context) }) @@ -1166,38 +1171,6 @@ impl FileSystemSandboxPolicy { .collect() } - /// Replaces symbolic `:workspace_roots` entries with absolute paths resolved - /// against `cwd`. - /// - /// Use this when a durable permission profile must survive a cwd-only - /// update without rebinding its project-root authority to the new cwd. - pub fn materialize_project_roots_with_cwd(mut self, cwd: &Path) -> Self { - let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok(); - for entry in &mut self.entries { - match &entry.path { - FileSystemPath::Special { - value: FileSystemSpecialPath::ProjectRoots { .. }, - } => { - if let Some(path) = resolve_file_system_path(&entry.path, cwd.as_ref()) { - entry.path = path.into(); - } - } - FileSystemPath::GlobPattern { pattern } => { - if let (Some(cwd), Some(subpath)) = - (cwd.as_ref(), parse_project_roots_glob_pattern(pattern)) - { - entry.path = FileSystemPath::GlobPattern { - pattern: resolve_project_roots_glob_pattern(subpath, cwd), - }; - } - } - FileSystemPath::Special { value: _ } => {} - FileSystemPath::Path { .. } => {} - } - } - self - } - /// Replaces symbolic `:workspace_roots` entries with concrete entries for /// each workspace root. pub fn materialize_project_roots_with_workspace_roots( @@ -1344,7 +1317,7 @@ impl FileSystemSandboxPolicy { } for path in additional_readable_roots { - if self.can_read_path_with_cwd(path.as_path(), cwd) { + if self.can_read_local_path_with_cwd(path.as_path(), cwd) { continue; } @@ -1363,7 +1336,7 @@ impl FileSystemSandboxPolicy { additional_writable_roots: &[AbsolutePathBuf], ) -> Self { for path in additional_writable_roots { - if self.can_write_path_with_cwd(path.as_path(), cwd) { + if self.can_write_local_path_with_cwd(path.as_path(), cwd) { continue; } @@ -1452,7 +1425,7 @@ impl FileSystemSandboxPolicy { self.resolved_entries_with_cwd(cwd) .into_iter() .filter(|entry| entry.access.can_read()) - .filter(|entry| self.can_read_path_with_cwd(entry.path.as_path(), cwd)) + .filter(|entry| self.can_read_local_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path) .collect(), /*normalize_effective_paths*/ true, @@ -1465,18 +1438,6 @@ impl FileSystemSandboxPolicy { self.get_writable_roots_with_cwd_impl(cwd, WritableRootPathResolution::Effective) } - /// Returns whether any effective writable root exists without materializing its carveouts. - pub fn has_writable_roots_with_cwd(&self, cwd: &Path) -> bool { - !self.has_full_disk_write_access() - && self - .resolved_entries_with_cwd(cwd) - .into_iter() - .any(|entry| { - entry.access.can_write() - && self.can_write_path_with_cwd(entry.path.as_path(), cwd) - }) - } - /// Reports configured writable roots for diagnostics without inspecting the filesystem. /// /// Unlike runtime root resolution, this includes configured roots even if they @@ -1531,7 +1492,7 @@ impl FileSystemSandboxPolicy { let writable_entries: Vec = resolved_entries .iter() .filter(|entry| entry.access.can_write()) - .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) + .filter(|entry| self.can_write_local_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path.clone()) .collect(); @@ -1578,7 +1539,7 @@ impl FileSystemSandboxPolicy { resolved_entries .iter() .filter(|entry| !entry.access.can_write()) - .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) + .filter(|entry| !self.can_write_local_path_with_cwd(entry.path.as_path(), cwd)) .filter_map(|entry| { let effective_path = path_resolution.resolve(entry.path.clone()); // Preserve the literal in-root path whenever the @@ -1654,7 +1615,7 @@ impl FileSystemSandboxPolicy { self.resolved_entries_with_cwd(cwd) .iter() .filter(|entry| entry.access == FileSystemAccessMode::Deny) - .filter(|entry| !self.can_read_path_with_cwd(entry.path.as_path(), cwd)) + .filter(|entry| !self.can_read_local_path_with_cwd(entry.path.as_path(), cwd)) // Restricted policies already deny reads outside explicit allow roots, // so materializing the filesystem root here would erase narrower // readable carveouts when downstream sandboxes apply deny masks last. @@ -2408,10 +2369,9 @@ fn protected_metadata_names_for_writable_root( .map(|raw_root| raw_root.join(*metadata_name)), ); - if metadata_paths - .iter() - .all(|metadata_path| !policy.can_write_path_with_cwd(metadata_path.as_path(), cwd)) - { + if metadata_paths.iter().all(|metadata_path| { + !policy.can_write_local_path_with_cwd(metadata_path.as_path(), cwd) + }) { protected_names.push((*metadata_name).to_string()); } } @@ -2584,10 +2544,6 @@ mod tests { ]; for policy in policies { - assert_eq!( - policy.has_writable_roots_with_cwd(cwd.path()), - !policy.get_writable_roots_with_cwd(cwd.path()).is_empty() - ); assert_eq!( policy.has_configured_writable_roots_with_cwd(cwd.path()), !policy.get_writable_roots_with_cwd(cwd.path()).is_empty() @@ -2632,11 +2588,11 @@ mod tests { ]); assert!( - !policy.can_read_path_with_cwd(Path::new(path), cwd.path()), + !policy.can_read_local_path_with_cwd(Path::new(path), cwd.path()), "deny should apply to {path}" ); assert!( - policy.can_read_path_with_cwd(&cwd.path().join("ordinary"), cwd.path()), + policy.can_read_local_path_with_cwd(&cwd.path().join("ordinary"), cwd.path()), "opaque deny for {path} should not poison ordinary paths" ); } @@ -3044,7 +3000,7 @@ mod tests { /*exclude_tmpdir_env_var*/ true, /*exclude_slash_tmp*/ false, ) - .can_write_path_with_cwd(slash_tmp.as_path(), cwd.path()) + .can_write_local_path_with_cwd(slash_tmp.as_path(), cwd.path()) ); } @@ -3277,12 +3233,10 @@ mod tests { .contains(&explicit_dot_codex), "explicit .codex rule should win over the default protected carveout" ); - assert!( - policy.can_write_path_with_cwd( - explicit_dot_codex.join("config.toml").as_path(), - cwd.path() - ) - ); + assert!(policy.can_write_local_path_with_cwd( + explicit_dot_codex.join("config.toml").as_path(), + cwd.path() + )); } #[test] @@ -3299,9 +3253,9 @@ mod tests { missing_path_behavior: None, }]); - assert!(!file_system_policy.can_write_path_with_cwd(&dot_git_config, cwd.path())); - assert!(!file_system_policy.can_write_path_with_cwd(&dot_agents_config, cwd.path())); - assert!(!file_system_policy.can_write_path_with_cwd(&dot_codex_config, cwd.path())); + assert!(!file_system_policy.can_write_local_path_with_cwd(&dot_git_config, cwd.path())); + assert!(!file_system_policy.can_write_local_path_with_cwd(&dot_agents_config, cwd.path())); + assert!(!file_system_policy.can_write_local_path_with_cwd(&dot_codex_config, cwd.path())); let writable_roots = file_system_policy.get_writable_roots_with_cwd(cwd.path()); assert_eq!(writable_roots.len(), 1); @@ -3385,18 +3339,17 @@ mod tests { Some(".git") ); assert!( - file_system_policy.can_write_path_with_cwd(Path::new("src/main.rs"), relative_cwd,) + file_system_policy + .can_write_local_path_with_cwd(Path::new("src/main.rs"), relative_cwd,) ); assert!( !file_system_policy - .can_write_path_with_cwd(Path::new(".codex/config.toml"), relative_cwd,) - ); - assert!( - !file_system_policy.can_write_path_with_cwd( - Path::new(".agents/skills/example/SKILL.md"), - relative_cwd, - ) + .can_write_local_path_with_cwd(Path::new(".codex/config.toml"), relative_cwd,) ); + assert!(!file_system_policy.can_write_local_path_with_cwd( + Path::new(".agents/skills/example/SKILL.md"), + relative_cwd, + )); } #[cfg(unix)] @@ -3777,7 +3730,7 @@ mod tests { } #[test] - fn resolve_access_with_cwd_uses_most_specific_entry() { + fn resolve_access_for_local_path_with_cwd_uses_most_specific_entry() { let cwd = TempDir::new().expect("tempdir"); let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()); let docs_private = AbsolutePathBuf::resolve_path_against_base("docs/private", cwd.path()); @@ -3813,19 +3766,20 @@ mod tests { ]); assert_eq!( - policy.resolve_access_with_cwd(cwd.path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(cwd.path(), cwd.path()), FileSystemAccessMode::Write ); assert_eq!( - policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(docs.as_path(), cwd.path()), FileSystemAccessMode::Read ); assert_eq!( - policy.resolve_access_with_cwd(docs_private.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(docs_private.as_path(), cwd.path()), FileSystemAccessMode::Deny ); assert_eq!( - policy.resolve_access_with_cwd(docs_private_public.as_path(), cwd.path()), + policy + .resolve_access_for_local_path_with_cwd(docs_private_public.as_path(), cwd.path()), FileSystemAccessMode::Write ); } @@ -3939,7 +3893,7 @@ mod tests { assert!(!policy.has_full_disk_write_access()); assert_eq!( - policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(docs.as_path(), cwd.path()), FileSystemAccessMode::Read ); assert!( @@ -3978,7 +3932,7 @@ mod tests { ]); assert_eq!( - policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(docs.as_path(), cwd.path()), FileSystemAccessMode::Read ); assert_eq!( @@ -4013,7 +3967,7 @@ mod tests { assert!(!policy.has_full_disk_write_access()); assert_eq!( - policy.resolve_access_with_cwd(root.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(root.as_path(), cwd.path()), FileSystemAccessMode::Deny ); } @@ -4044,7 +3998,7 @@ mod tests { assert!(policy.has_full_disk_write_access()); assert_eq!( - policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + policy.resolve_access_for_local_path_with_cwd(docs.as_path(), cwd.path()), FileSystemAccessMode::Write ); } @@ -4215,33 +4169,6 @@ mod tests { ); } - #[test] - fn materialize_project_roots_with_cwd_expands_symbolic_glob_entries() { - let cwd = TempDir::new().expect("tempdir"); - let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::GlobPattern { - pattern: project_roots_glob_pattern(Path::new("**/*.env")), - }, - access: FileSystemAccessMode::Deny, - missing_path_behavior: None, - }]); - - let actual = policy.materialize_project_roots_with_cwd(cwd.path()); - - assert_eq!( - actual, - FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::GlobPattern { - pattern: AbsolutePathBuf::resolve_path_against_base("**/*.env", cwd.path()) - .to_string_lossy() - .into_owned(), - }, - access: FileSystemAccessMode::Deny, - missing_path_behavior: None, - }]) - ); - } - #[test] fn with_additional_legacy_workspace_writable_roots_protects_metadata() { let temp_dir = TempDir::new().expect("tempdir"); @@ -4375,8 +4302,9 @@ mod tests { file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, ) -> bool { - ReadDenyMatcher::new(file_system_sandbox_policy, cwd) - .is_some_and(|matcher| matcher.is_read_denied(path)) + ReadDenyMatcher::try_new_for_local_paths(file_system_sandbox_policy, cwd) + .expect("valid deny-read globs") + .is_some_and(|matcher| matcher.is_local_path_read_denied(path)) } #[test] diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 1245622392..0439e1c63e 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -686,11 +686,13 @@ fn compatibility_workspace_write_policy( .and_then(|tmpdir| { AbsolutePathBuf::from_absolute_path(std::path::PathBuf::from(tmpdir)).ok() }) - .is_some_and(|tmpdir| file_system_policy.can_write_path_with_cwd(tmpdir.as_path(), cwd)); + .is_some_and(|tmpdir| { + file_system_policy.can_write_local_path_with_cwd(tmpdir.as_path(), cwd) + }); let slash_tmp = Path::new("/tmp"); let slash_tmp_writable = slash_tmp.is_absolute() && slash_tmp.is_dir() - && file_system_policy.can_write_path_with_cwd(slash_tmp, cwd); + && file_system_policy.can_write_local_path_with_cwd(slash_tmp, cwd); SandboxPolicy::WorkspaceWrite { writable_roots, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 927ccc06fc..fed7bd15b1 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -12,11 +12,9 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::permissions::ReadDenyMatcher; use codex_protocol::permissions::file_system_root; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use std::num::NonZeroUsize; -use std::path::Path; pub fn normalize_additional_permissions( additional_permissions: AdditionalPermissionProfile, @@ -81,18 +79,6 @@ pub fn normalize_additional_permissions_with_context( /// Resolves cwd-dependent permission entries without filtering their authority. /// /// Unlike intersection, this preserves narrower grants beneath denied paths. -pub fn materialize_additional_permissions( - additional_permissions: AdditionalPermissionProfile, - cwd: &Path, -) -> Result { - let cwd = AbsolutePathBuf::from_absolute_path(cwd) - .map_err(|err| format!("invalid permission cwd: {err}"))?; - let cwd = PathUri::from(cwd); - with_local_context_for_cwd(&cwd, |context| { - materialize_additional_permissions_with_context(additional_permissions, context) - }) -} - pub fn materialize_additional_permissions_with_context( mut additional_permissions: AdditionalPermissionProfile, context: &FileSystemSandboxPolicyContext<'_>, @@ -158,21 +144,6 @@ pub fn merge_permission_profiles( } } -pub fn intersect_permission_profiles( - requested: AdditionalPermissionProfile, - granted: AdditionalPermissionProfile, - cwd: &Path, -) -> AdditionalPermissionProfile { - AbsolutePathBuf::from_absolute_path(cwd) - .ok() - .map(PathUri::from) - .map_or_else(AdditionalPermissionProfile::default, |cwd| { - with_local_context_for_cwd(&cwd, |context| { - intersect_permission_profiles_with_context(requested, granted, context) - }) - }) -} - pub fn intersect_permission_profiles_with_context( requested: AdditionalPermissionProfile, granted: AdditionalPermissionProfile, @@ -248,46 +219,6 @@ pub fn intersect_permission_profiles_with_context( } } -fn with_local_context_for_cwd( - cwd: &PathUri, - evaluate: impl FnOnce(&FileSystemSandboxPolicyContext<'_>) -> T, -) -> T { - let user_home_dir = PathUri::from_host_native_path("~").ok(); - let local_cwd = std::env::current_dir().ok(); - let temporary_directory_env_vars: &[&str] = if cfg!(windows) { - &["TEMP", "TMP"] - } else { - &["TMPDIR"] - }; - let normalize_temp_path = |path: std::ffi::OsString| { - PathUri::from_host_native_path(&path).ok().or_else(|| { - if cfg!(unix) { - PathUri::from_host_native_path(local_cwd.as_ref()?.join(path)).ok() - } else { - None - } - }) - }; - let mut temporary_directories = Vec::new(); - for name in temporary_directory_env_vars { - if let Some(path) = std::env::var_os(name) - .filter(|path| !path.is_empty()) - .filter(|path| cfg!(unix) || Path::new(path).is_absolute()) - .and_then(&normalize_temp_path) - && !temporary_directories.contains(&path) - { - temporary_directories.push(path); - } - } - let context = FileSystemSandboxPolicyContext { - cwd, - workspace_roots: std::slice::from_ref(cwd), - user_home_dir: user_home_dir.as_ref(), - temporary_directories: Some(&temporary_directories), - }; - evaluate(&context) -} - fn merge_glob_scan_max_depth( left_entries: &[FileSystemSandboxEntry], left_depth: Option, diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index c84f0185dc..28d5828b9c 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -1,7 +1,5 @@ use super::effective_file_system_sandbox_policy; -use super::intersect_permission_profiles; use super::intersect_permission_profiles_with_context; -use super::materialize_additional_permissions; use super::materialize_additional_permissions_with_context; use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; @@ -23,7 +21,6 @@ use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use dunce::canonicalize; use pretty_assertions::assert_eq; -#[cfg(unix)] use std::path::Path; use tempfile::TempDir; @@ -32,6 +29,21 @@ fn symlink_dir(original: &Path, link: &Path) -> std::io::Result<()> { std::os::unix::fs::symlink(original, link) } +fn intersect_permission_profiles_with_local_cwd( + requested: PermissionProfile, + granted: PermissionProfile, + cwd: &Path, +) -> PermissionProfile { + let cwd = PathUri::from_host_native_path(cwd).expect("cwd URI"); + let context = FileSystemSandboxPolicyContext { + cwd: &cwd, + workspace_roots: std::slice::from_ref(&cwd), + user_home_dir: None, + temporary_directories: None, + }; + intersect_permission_profiles_with_context(requested, granted, &context) +} + #[test] fn full_access_restricted_policy_skips_platform_sandbox_when_network_is_enabled() { let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { @@ -303,6 +315,13 @@ fn materialize_additional_permissions_preserves_authority_and_constraints() { ) .expect("absolute temp dir"); let home = PathUri::from_host_native_path("~").expect("host home"); + let cwd_uri = PathUri::from(cwd.clone()); + let context = FileSystemSandboxPolicyContext { + cwd: &cwd_uri, + workspace_roots: std::slice::from_ref(&cwd_uri), + user_home_dir: Some(&home), + temporary_directories: None, + }; let project_path = |subpath: &str| FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(subpath.to_owned())), }; @@ -342,7 +361,7 @@ fn materialize_additional_permissions_preserves_authority_and_constraints() { ]); assert_eq!( - materialize_additional_permissions(requested, cwd.as_path()) + materialize_additional_permissions_with_context(requested, &context) .expect("materialized permissions"), expected ); @@ -409,7 +428,7 @@ fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() { let granted = requested.clone(); assert_eq!( - intersect_permission_profiles(requested.clone(), granted, temp_dir.path()), + intersect_permission_profiles_with_local_cwd(requested.clone(), granted, temp_dir.path()), requested ); } @@ -430,7 +449,11 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { }; assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()), + intersect_permission_profiles_with_local_cwd( + requested, + PermissionProfile::default(), + temp_dir.path() + ), PermissionProfile::default() ); } @@ -451,7 +474,11 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() { }; assert_eq!( - intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()), + intersect_permission_profiles_with_local_cwd( + requested, + PermissionProfile::default(), + temp_dir.path() + ), PermissionProfile::default() ); } @@ -491,7 +518,7 @@ fn intersect_permission_profiles_preserves_parent_relative_project_root_restrict }; assert_eq!( - intersect_permission_profiles(requested, granted, cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted, cwd.as_path()), PermissionProfile::default() ); } @@ -522,11 +549,11 @@ fn intersect_permission_profiles_rejects_symbolic_slash_tmp_grants() { }; assert_eq!( - intersect_permission_profiles(requested, granted.clone(), cwd.path()), + intersect_permission_profiles_with_local_cwd(requested, granted.clone(), cwd.path()), PermissionProfile::default() ); assert_eq!( - intersect_permission_profiles(granted.clone(), granted, cwd.path()), + intersect_permission_profiles_with_local_cwd(granted.clone(), granted, cwd.path()), PermissionProfile::default() ); } @@ -553,7 +580,7 @@ fn intersect_permission_profiles_preserves_deny_across_case_variant_grant() { }; assert_eq!( - intersect_permission_profiles( + intersect_permission_profiles_with_local_cwd( profile(vec![requested_write, requested_deny.clone()]), profile(vec![granted_write.clone()]), requested_root.as_path(), @@ -698,7 +725,7 @@ fn intersect_permission_profiles_preserves_opaque_child_deny() { }; assert_eq!( - intersect_permission_profiles( + intersect_permission_profiles_with_local_cwd( profile(vec![requested_write, requested_deny.clone()]), profile(vec![granted_write.clone()]), root.as_path(), @@ -737,7 +764,7 @@ fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd() }; assert_eq!( - intersect_permission_profiles(requested, granted.clone(), cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted.clone(), cwd.as_path()), granted ); } @@ -763,7 +790,7 @@ fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() { ..Default::default() }; - let intersected = intersect_permission_profiles( + let intersected = intersect_permission_profiles_with_local_cwd( cwd_write_permissions.clone(), cwd_write_permissions, request_cwd.as_path(), @@ -780,7 +807,7 @@ fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() { } ); assert_eq!( - intersect_permission_profiles( + intersect_permission_profiles_with_local_cwd( PermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( /*read*/ None, @@ -822,7 +849,11 @@ fn intersect_permission_profiles_deduplicates_materialized_grants() { }; assert_eq!( - intersect_permission_profiles(permissions.clone(), permissions, cwd.as_path()), + intersect_permission_profiles_with_local_cwd( + permissions.clone(), + permissions, + cwd.as_path() + ), PermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( /*read*/ None, @@ -862,7 +893,11 @@ fn intersect_permission_profiles_materializes_cwd_deny_entries() { }; assert_eq!( - intersect_permission_profiles(permissions.clone(), permissions, request_cwd.as_path()), + intersect_permission_profiles_with_local_cwd( + permissions.clone(), + permissions, + request_cwd.as_path() + ), PermissionProfile { file_system: Some(FileSystemPermissions { entries: vec![ @@ -924,7 +959,7 @@ fn intersect_permission_profiles_drops_deny_entries_without_filesystem_grants() }; assert_eq!( - intersect_permission_profiles(requested, granted.clone(), cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted.clone(), cwd.as_path()), granted ); } @@ -968,7 +1003,7 @@ fn intersect_permission_profiles_rejects_concrete_grants_matched_by_requested_de }; assert_eq!( - intersect_permission_profiles(requested, granted, cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted, cwd.as_path()), PermissionProfile::default() ); } @@ -1002,8 +1037,11 @@ fn intersect_permission_profiles_materializes_relative_deny_globs_for_reuse() { ..Default::default() }; - let intersected = - intersect_permission_profiles(permissions.clone(), permissions, request_cwd.as_path()); + let intersected = intersect_permission_profiles_with_local_cwd( + permissions.clone(), + permissions, + request_cwd.as_path(), + ); assert_eq!( intersected, @@ -1029,7 +1067,7 @@ fn intersect_permission_profiles_materializes_relative_deny_globs_for_reuse() { } ); assert_eq!( - intersect_permission_profiles( + intersect_permission_profiles_with_local_cwd( PermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( /*read*/ None, @@ -1074,7 +1112,7 @@ fn intersect_permission_profiles_drops_broader_cwd_grant_for_requested_child_pat }; assert_eq!( - intersect_permission_profiles(requested, granted, cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted, cwd.as_path()), PermissionProfile::default() ); } @@ -1112,7 +1150,7 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() { }; assert_eq!( - intersect_permission_profiles(requested, granted, cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted, cwd.as_path()), PermissionProfile { file_system: Some(FileSystemPermissions { entries: vec![ @@ -1170,7 +1208,7 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() { }; assert_eq!( - intersect_permission_profiles(requested, granted, cwd.as_path()), + intersect_permission_profiles_with_local_cwd(requested, granted, cwd.as_path()), PermissionProfile { file_system: Some(FileSystemPermissions { entries: vec![ diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index ab60605015..3548264a97 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -607,7 +607,7 @@ fn protected_metadata_names_for_writable_root( continue; } let path = writable_root.root.join(*name); - if !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd) { + if !file_system_sandbox_policy.can_write_local_path_with_cwd(path.as_path(), cwd) { names.push((*name).to_string()); } } diff --git a/codex-rs/sandboxing/src/windows.rs b/codex-rs/sandboxing/src/windows.rs index 2c35407c94..b69168ca98 100644 --- a/codex-rs/sandboxing/src/windows.rs +++ b/codex-rs/sandboxing/src/windows.rs @@ -254,8 +254,10 @@ pub fn resolve_windows_elevated_filesystem_overrides( .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd); let has_explicit_write_carveouts = split_writable_roots.iter().any(|writable_root| { writable_root.read_only_subpaths.iter().any(|path| { - file_system_sandbox_policy - .has_explicit_non_write_entry_for_path_with_cwd(path.as_path(), sandbox_policy_cwd) + file_system_sandbox_policy.has_explicit_non_write_entry_for_local_path_with_cwd( + path.as_path(), + sandbox_policy_cwd, + ) }) }); let normalize_path = |path: PathBuf| dunce::canonicalize(&path).unwrap_or(path); @@ -322,7 +324,7 @@ pub fn resolve_windows_elevated_filesystem_overrides( }) }); let explicitly_configured = file_system_sandbox_policy - .has_explicit_non_write_entry_for_path_with_cwd( + .has_explicit_non_write_entry_for_local_path_with_cwd( read_only_subpath.as_path(), sandbox_policy_cwd, ); @@ -370,7 +372,7 @@ fn windows_policy_has_root_read_access( let Some(root) = cwd.as_path().ancestors().last() else { return false; }; - file_system_sandbox_policy.can_read_path_with_cwd(root, cwd.as_path()) + file_system_sandbox_policy.can_read_local_path_with_cwd(root, cwd.as_path()) } fn permission_profile_display_name(permission_profile: &PermissionProfile) -> &'static str { diff --git a/codex-rs/tui/src/additional_dirs.rs b/codex-rs/tui/src/additional_dirs.rs index 29cff6d3a8..c1317de12a 100644 --- a/codex-rs/tui/src/additional_dirs.rs +++ b/codex-rs/tui/src/additional_dirs.rs @@ -25,7 +25,7 @@ pub fn add_dir_warning_message( return None; } - if file_system_policy.can_write_path_with_cwd(cwd, cwd) { + if file_system_policy.can_write_local_path_with_cwd(cwd, cwd) { return None; } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index c1b89c3a3d..f68dddb88e 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1830,7 +1830,7 @@ fn sandbox_mode_from_permission_profile( .network_sandbox_policy() .is_enabled() .then_some(codex_app_server_protocol::SandboxMode::DangerFullAccess) - } else if file_system_policy.can_write_path_with_cwd(cwd, cwd) { + } else if file_system_policy.can_write_local_path_with_cwd(cwd, cwd) { Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite) } else { Some(codex_app_server_protocol::SandboxMode::ReadOnly) diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 09ab54a351..53525f7c6e 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -407,7 +407,7 @@ impl ChatWidget { matches!( current_permission_profile, PermissionProfile::Managed { .. } - ) && file_system_policy.can_write_path_with_cwd(cwd, cwd) + ) && file_system_policy.can_write_local_path_with_cwd(cwd, cwd) && !file_system_policy.has_full_disk_write_access() } _ => current_permission_profile == &preset.permission_profile, diff --git a/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs b/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs index 77b459ad3d..724f7a4f3c 100644 --- a/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs +++ b/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs @@ -91,7 +91,7 @@ impl ChatWidget { "Full Access mode" } else if profile .file_system_sandbox_policy() - .can_write_path_with_cwd(self.config.cwd.as_path(), self.config.cwd.as_path()) + .can_write_local_path_with_cwd(self.config.cwd.as_path(), self.config.cwd.as_path()) { "Agent mode" } else { diff --git a/codex-rs/tui/src/external_editor.rs b/codex-rs/tui/src/external_editor.rs index 8639e5d15e..3c4de6cb74 100644 --- a/codex-rs/tui/src/external_editor.rs +++ b/codex-rs/tui/src/external_editor.rs @@ -126,8 +126,9 @@ pub(super) fn editor_directory( let Some(parent) = directory.parent() else { return true; }; - let is_writable = file_system_policy.can_write_path_with_cwd(directory, cwd) - || file_system_policy.can_write_path_with_cwd(parent, cwd) + let is_writable = file_system_policy + .can_write_local_path_with_cwd(directory, cwd) + || file_system_policy.can_write_local_path_with_cwd(parent, cwd) || writable_roots.iter().any(|root| { root.is_path_writable(directory) || root.root.as_path().starts_with(directory) diff --git a/codex-rs/tui/src/external_editor_tests.rs b/codex-rs/tui/src/external_editor_tests.rs index 575221dcf2..a9ed2b7bc2 100644 --- a/codex-rs/tui/src/external_editor_tests.rs +++ b/codex-rs/tui/src/external_editor_tests.rs @@ -135,8 +135,8 @@ fn editor_directory_rejects_writable_codex_home_alias() { symlink(&paths.codex_home, &aliased_home).expect("create Codex home symlink"); let policy = workspace_write_policy(&[]); - assert!(policy.can_write_path_with_cwd(&aliased_home.join("editor"), &paths.cwd)); - assert!(!policy.can_write_path_with_cwd(&paths.codex_home.join("editor"), &paths.cwd)); + assert!(policy.can_write_local_path_with_cwd(&aliased_home.join("editor"), &paths.cwd)); + assert!(!policy.can_write_local_path_with_cwd(&paths.codex_home.join("editor"), &paths.cwd)); assert!(editor_directory(&[&aliased_home], &policy, &paths.cwd).is_err()); } @@ -156,8 +156,8 @@ fn editor_directory_rejects_writable_codex_home_alias_target() { symlink(&paths.codex_home, &aliased_home).expect("create Codex home symlink"); let policy = workspace_write_policy(&[&paths.codex_home]); - assert!(!policy.can_write_path_with_cwd(&aliased_home.join("editor"), &paths.cwd)); - assert!(policy.can_write_path_with_cwd(&paths.codex_home.join("editor"), &paths.cwd)); + assert!(!policy.can_write_local_path_with_cwd(&aliased_home.join("editor"), &paths.cwd)); + assert!(policy.can_write_local_path_with_cwd(&paths.codex_home.join("editor"), &paths.cwd)); assert!(editor_directory(&[&aliased_home], &policy, &paths.cwd).is_err()); } @@ -177,9 +177,9 @@ fn editor_directory_uses_protected_workspace_fallback_with_default_temporary_gra ); assert!(!workspace_codex_home.exists()); - assert!(policy.can_write_path_with_cwd(&codex_home, &cwd)); - assert!(!policy.can_write_path_with_cwd(&workspace_codex_home, &cwd)); - assert!(!policy.can_write_path_with_cwd(&workspace_codex_home.join("editor"), &cwd)); + assert!(policy.can_write_local_path_with_cwd(&codex_home, &cwd)); + assert!(!policy.can_write_local_path_with_cwd(&workspace_codex_home, &cwd)); + assert!(!policy.can_write_local_path_with_cwd(&workspace_codex_home.join("editor"), &cwd)); let directory = editor_directory(&[&codex_home, &workspace_codex_home], &policy, &cwd) .expect("use protected workspace metadata directory"); @@ -210,8 +210,8 @@ fn editor_directory_rejects_explicitly_writable_workspace_fallback() { /*exclude_slash_tmp*/ false, ); - assert!(policy.can_write_path_with_cwd(&codex_home, &cwd)); - assert!(policy.can_write_path_with_cwd(&workspace_codex_home, &cwd)); + assert!(policy.can_write_local_path_with_cwd(&codex_home, &cwd)); + assert!(policy.can_write_local_path_with_cwd(&workspace_codex_home, &cwd)); assert!( editor_directory(&[&codex_home, &workspace_codex_home], &policy, &cwd).is_err(), "explicitly writable metadata must not be used for editor buffers" @@ -229,8 +229,8 @@ fn editor_directory_rejects_workspace_fallback_symlink_to_writable_target() { symlink(&paths.codex_home, &workspace_codex_home).expect("create workspace metadata symlink"); let policy = workspace_write_policy(&[&paths.codex_home]); - assert!(!policy.can_write_path_with_cwd(&workspace_codex_home, &paths.cwd)); - assert!(policy.can_write_path_with_cwd(&paths.codex_home, &paths.cwd)); + assert!(!policy.can_write_local_path_with_cwd(&workspace_codex_home, &paths.cwd)); + assert!(policy.can_write_local_path_with_cwd(&paths.codex_home, &paths.cwd)); assert!( editor_directory( &[&paths.codex_home, &workspace_codex_home], @@ -347,9 +347,9 @@ async fn editor_process_uses_protected_workspace_fallback_with_default_temporary ]; assert!(!workspace_codex_home.exists()); - assert!(policy.can_write_path_with_cwd(&codex_home, &cwd)); - assert!(policy.can_write_path_with_cwd(&default_codex_home, &cwd)); - assert!(!policy.can_write_path_with_cwd(&workspace_codex_home, &cwd)); + assert!(policy.can_write_local_path_with_cwd(&codex_home, &cwd)); + assert!(policy.can_write_local_path_with_cwd(&default_codex_home, &cwd)); + assert!(!policy.can_write_local_path_with_cwd(&workspace_codex_home, &cwd)); let content = run_editor("seed", &editor_command, &codex_home, &policy, &cwd) .await diff --git a/codex-rs/tui/src/inline_visualization.rs b/codex-rs/tui/src/inline_visualization.rs index 657f867bab..8bf4f77d5d 100644 --- a/codex-rs/tui/src/inline_visualization.rs +++ b/codex-rs/tui/src/inline_visualization.rs @@ -64,9 +64,9 @@ impl InlineVisualizationContext { context.viewer_dir.parent()?.parent()?.to_path_buf(), ]; for viewer_cache in viewer_caches { - if file_system_policy.can_write_path_with_cwd(&viewer_cache, config.cwd.as_path()) + if file_system_policy.can_write_local_path_with_cwd(&viewer_cache, config.cwd.as_path()) || file_system_policy - .can_write_path_with_cwd(viewer_cache.parent()?, config.cwd.as_path()) + .can_write_local_path_with_cwd(viewer_cache.parent()?, config.cwd.as_path()) || writable_roots.iter().any(|root| { root.is_path_writable(&viewer_cache) || root.root.as_path().starts_with(&viewer_cache) diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs index e5f8a2099e..29604df91b 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs @@ -56,7 +56,8 @@ pub fn resolve_windows_deny_read_paths( }) .collect(), ); - let Some(matcher) = ReadDenyMatcher::try_new(&glob_policy, cwd.as_path())? else { + let Some(matcher) = ReadDenyMatcher::try_new_for_local_paths(&glob_policy, cwd.as_path())? + else { return Ok(paths); }; @@ -72,7 +73,7 @@ pub fn resolve_windows_deny_read_paths( let directory_scan_mode = if let Some(file_paths) = ripgrep_files(&scan_plan)? { for path in file_paths { - if matcher.is_read_denied(&path) { + if matcher.is_local_path_read_denied(&path) { push_absolute_path(&mut paths, &mut seen, path)?; } } @@ -131,7 +132,8 @@ fn ripgrep_files(scan_plan: &GlobScanPlan) -> Result>, Strin // Ripgrep uses exit 2 for traversal errors, including protected // Windows directories. Its output may be incomplete. The caller // must enumerate again using the matcher-backed walker; policy - // syntax has already been validated by ReadDenyMatcher::try_new. + // syntax has already been validated by + // ReadDenyMatcher::try_new_for_local_paths. return Ok(None); } diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs index 7a40902001..4e5b7c1776 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver_access_tests.rs @@ -219,11 +219,11 @@ fn native_walk_preserves_canonical_matches_through_directory_links_and_cycles() access: FileSystemAccessMode::Deny, missing_path_behavior: None, }]); - let matcher = ReadDenyMatcher::try_new(&policy, cwd.as_path()) + let matcher = ReadDenyMatcher::try_new_for_local_paths(&policy, cwd.as_path()) .expect("valid matcher") .expect("deny restriction"); assert!( - matcher.is_read_denied_with_canonical_path( + matcher.is_local_path_read_denied_with_canonical_path( &alias.join("secret.env"), &target .canonicalize() diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs b/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs index cb3e911ae0..dd1b6e3cca 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_walker.rs @@ -69,8 +69,8 @@ impl GlobWalker<'_> { let denied = match canonical { Some(canonical) => self .matcher - .is_read_denied_with_canonical_path(path, canonical), - None => self.matcher.is_read_denied(path), + .is_local_path_read_denied_with_canonical_path(path, canonical), + None => self.matcher.is_local_path_read_denied(path), }; if denied { push_absolute_path(self.paths, self.seen_paths, path.to_path_buf())?; diff --git a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs index 0e28b9d3dc..9834370ec0 100644 --- a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs +++ b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs @@ -114,7 +114,7 @@ impl ResolvedWindowsSandboxPermissions { }) && cwd .ancestors() .last() - .is_some_and(|root| self.file_system.can_read_path_with_cwd(root, cwd)) + .is_some_and(|root| self.file_system.can_read_local_path_with_cwd(root, cwd)) } pub(crate) fn include_platform_defaults(&self) -> bool {