From ef771ccc19bbf461365e33250a73d07f4ec500b8 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 1 Jun 2026 14:52:31 -0700 Subject: [PATCH] core: stop threading SandboxPolicy through exec Migrate the exec-side Windows sandbox override plumbing to pass PermissionProfile plus the split filesystem/network policies instead of accepting a legacy SandboxPolicy. This keeps the remaining compatibility projection local to the writable-root comparison that still needs it, and removes ExecRequest::compatibility_sandbox_policy without touching the broader config, protocol, app-server, telemetry, or session surfaces from #25450. --- codex-rs/core/src/exec.rs | 62 ++++++++--------- codex-rs/core/src/exec_tests.rs | 101 +++++++++++++++++++++++----- codex-rs/core/src/sandboxing/mod.rs | 11 --- codex-rs/core/src/spawn.rs | 2 +- 4 files changed, 115 insertions(+), 61 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index e050552246..620e15eff1 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -38,12 +38,12 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandOutputDeltaEvent; use codex_protocol::protocol::ExecOutputStream; -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use codex_utils_pty::process_group::kill_child_process_group; @@ -419,11 +419,10 @@ pub fn build_exec_request( exec_req.windows_sandbox_level, exec_req.network.is_some(), ); - let sandbox_policy = exec_req.compatibility_sandbox_policy(); exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend { resolve_windows_elevated_filesystem_overrides( exec_req.sandbox, - &sandbox_policy, + &exec_req.permission_profile, &exec_req.file_system_sandbox_policy, exec_req.network_sandbox_policy, sandbox_cwd, @@ -432,7 +431,7 @@ pub fn build_exec_request( } else { resolve_windows_restricted_token_filesystem_overrides( exec_req.sandbox, - &sandbox_policy, + &exec_req.permission_profile, &exec_req.file_system_sandbox_policy, exec_req.network_sandbox_policy, sandbox_cwd, @@ -1006,21 +1005,17 @@ async fn exec( #[cfg_attr(not(target_os = "windows"), allow(dead_code))] fn should_use_windows_restricted_token_sandbox( sandbox: SandboxType, - sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, ) -> bool { sandbox == SandboxType::WindowsRestrictedToken && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted - && !matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } - ) + && !file_system_sandbox_policy.has_full_disk_write_access() } #[cfg_attr(not(test), allow(dead_code))] pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( sandbox: SandboxType, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &AbsolutePathBuf, @@ -1029,7 +1024,7 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( if windows_sandbox_level == WindowsSandboxLevel::Elevated { resolve_windows_elevated_filesystem_overrides( sandbox, - sandbox_policy, + permission_profile, file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd, @@ -1039,7 +1034,7 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( } else { resolve_windows_restricted_token_filesystem_overrides( sandbox, - sandbox_policy, + permission_profile, file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd, @@ -1051,7 +1046,7 @@ pub(crate) fn unsupported_windows_restricted_token_sandbox_reason( pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( sandbox: SandboxType, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &AbsolutePathBuf, @@ -1066,22 +1061,15 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( let needs_direct_runtime_enforcement = file_system_sandbox_policy .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd); - if should_use_windows_restricted_token_sandbox( - sandbox, - sandbox_policy, - file_system_sandbox_policy, - ) && !needs_direct_runtime_enforcement + if should_use_windows_restricted_token_sandbox(sandbox, file_system_sandbox_policy) + && !needs_direct_runtime_enforcement { return Ok(None); } - if !should_use_windows_restricted_token_sandbox( - sandbox, - sandbox_policy, - file_system_sandbox_policy, - ) { + if !should_use_windows_restricted_token_sandbox(sandbox, file_system_sandbox_policy) { return Err(format!( - "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, legacy_policy={sandbox_policy:?}; refusing to run unsandboxed", + "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, permission_profile={permission_profile:?}; refusing to run unsandboxed", file_system_sandbox_policy.kind, )); } @@ -1108,7 +1096,13 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( ); } - let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + let legacy_projection = compatibility_sandbox_policy_for_permission_profile( + permission_profile, + file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd.as_path(), + ); + let legacy_writable_roots = legacy_projection.get_writable_roots_with_cwd(sandbox_policy_cwd); let split_writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); let legacy_root_paths: BTreeSet = legacy_writable_roots @@ -1204,7 +1198,7 @@ fn windows_policy_has_root_read_access( pub(crate) fn resolve_windows_elevated_filesystem_overrides( sandbox: SandboxType, - sandbox_policy: &SandboxPolicy, + permission_profile: &PermissionProfile, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, sandbox_policy_cwd: &AbsolutePathBuf, @@ -1214,13 +1208,9 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( return Ok(None); } - if !should_use_windows_restricted_token_sandbox( - sandbox, - sandbox_policy, - file_system_sandbox_policy, - ) { + if !should_use_windows_restricted_token_sandbox(sandbox, file_system_sandbox_policy) { return Err(format!( - "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, legacy_policy={sandbox_policy:?}; refusing to run unsandboxed", + "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, permission_profile={permission_profile:?}; refusing to run unsandboxed", file_system_sandbox_policy.kind, )); } @@ -1242,7 +1232,13 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( let needs_direct_runtime_enforcement = file_system_sandbox_policy .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd); let normalize_path = |path: PathBuf| dunce::canonicalize(&path).unwrap_or(path); - let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + let legacy_projection = compatibility_sandbox_policy_for_permission_profile( + permission_profile, + file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd.as_path(), + ); + let legacy_writable_roots = legacy_projection.get_writable_roots_with_cwd(sandbox_policy_cwd); let legacy_root_paths: BTreeSet = legacy_writable_roots .iter() .map(|root| normalize_path(root.root.to_path_buf())) diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 1c71d02651..c0433cbaec 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -1,6 +1,8 @@ use super::*; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::SandboxEnforcement; +use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxType; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -26,6 +28,17 @@ fn make_exec_output( } } +fn permission_profile_for_runtime_permissions( + policy: &SandboxPolicy, + file_system_policy: &FileSystemSandboxPolicy, +) -> PermissionProfile { + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(policy), + file_system_policy, + NetworkSandboxPolicy::from(policy), + ) +} + #[test] fn sandbox_detection_requires_keywords() { let output = make_exec_output(/*exit_code*/ 1, "", "", ""); @@ -387,7 +400,6 @@ fn windows_restricted_token_skips_external_sandbox_policies() { assert_eq!( should_use_windows_restricted_token_sandbox( SandboxType::WindowsRestrictedToken, - &policy, &file_system_policy, ), false @@ -402,7 +414,6 @@ fn windows_restricted_token_runs_for_legacy_restricted_policies() { assert_eq!( should_use_windows_restricted_token_sandbox( SandboxType::WindowsRestrictedToken, - &policy, &file_system_policy, ), true @@ -431,33 +442,69 @@ fn windows_restricted_token_rejects_network_only_restrictions() { network_access: codex_protocol::protocol::NetworkAccess::Restricted, }; let file_system_policy = FileSystemSandboxPolicy::unrestricted(); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); let sandbox_policy_cwd = AbsolutePathBuf::current_dir().expect("cwd"); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &sandbox_policy_cwd, WindowsSandboxLevel::RestrictedToken, ), Some( - "windows sandbox backend cannot enforce file_system=Unrestricted, network=Restricted, legacy_policy=ExternalSandbox { network_access: Restricted }; refusing to run unsandboxed".to_string() + "windows sandbox backend cannot enforce file_system=Unrestricted, network=Restricted, permission_profile=Managed { file_system: Unrestricted, network: Restricted }; refusing to run unsandboxed".to_string() ) ); } +#[test] +fn windows_restricted_token_rejects_full_write_split_profiles() { + let policy = SandboxPolicy::ExternalSandbox { + network_access: codex_protocol::protocol::NetworkAccess::Restricted, + }; + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + codex_protocol::permissions::FileSystemSandboxEntry { + path: codex_protocol::permissions::FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Root, + }, + access: codex_protocol::permissions::FileSystemAccessMode::Write, + }, + ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); + let sandbox_policy_cwd = AbsolutePathBuf::current_dir().expect("cwd"); + + assert_eq!( + unsupported_windows_restricted_token_sandbox_reason( + SandboxType::WindowsRestrictedToken, + &permission_profile, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + &sandbox_policy_cwd, + WindowsSandboxLevel::RestrictedToken, + ), + Some(format!( + "windows sandbox backend cannot enforce file_system=Restricted, network=Restricted, permission_profile={permission_profile:?}; refusing to run unsandboxed", + )) + ); +} + #[test] fn windows_restricted_token_allows_legacy_restricted_policies() { let policy = SandboxPolicy::new_read_only_policy(); let file_system_policy = FileSystemSandboxPolicy::from(&policy); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); let sandbox_policy_cwd = AbsolutePathBuf::current_dir().expect("cwd"); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &sandbox_policy_cwd, @@ -476,12 +523,14 @@ fn windows_restricted_token_allows_legacy_workspace_write_policies() { exclude_slash_tmp: true, }; let file_system_policy = FileSystemSandboxPolicy::from(&policy); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); let sandbox_policy_cwd = AbsolutePathBuf::current_dir().expect("cwd"); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &sandbox_policy_cwd, @@ -508,11 +557,13 @@ fn windows_elevated_allows_split_restricted_read_policies() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -550,11 +601,13 @@ fn windows_restricted_token_rejects_split_only_filesystem_policies() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -593,11 +646,13 @@ fn windows_restricted_token_rejects_root_write_read_only_carveouts() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -644,6 +699,8 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); // The legacy workspace-write root already protects top-level `.codex`, so // the restricted-token overlay only needs the extra read-only docs carveout. @@ -652,7 +709,7 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { assert_eq!( resolve_windows_restricted_token_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &cwd, @@ -702,11 +759,13 @@ fn windows_restricted_token_rejects_unreadable_split_carveouts() { access: codex_protocol::permissions::FileSystemAccessMode::Deny, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( resolve_windows_restricted_token_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &cwd, @@ -737,11 +796,13 @@ fn windows_elevated_supports_split_restricted_read_roots() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( resolve_windows_elevated_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -792,11 +853,13 @@ fn windows_elevated_supports_split_write_read_carveouts() { access: codex_protocol::permissions::FileSystemAccessMode::Read, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( resolve_windows_elevated_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -850,11 +913,13 @@ fn windows_elevated_supports_unreadable_split_carveouts() { access: codex_protocol::permissions::FileSystemAccessMode::Deny, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( resolve_windows_elevated_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -912,11 +977,13 @@ fn windows_elevated_supports_unreadable_globs() { access: codex_protocol::permissions::FileSystemAccessMode::Deny, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( resolve_windows_elevated_filesystem_overrides( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), @@ -977,11 +1044,13 @@ fn windows_elevated_rejects_reopened_writable_descendants() { access: codex_protocol::permissions::FileSystemAccessMode::Write, }, ]); + let permission_profile = + permission_profile_for_runtime_permissions(&policy, &file_system_policy); assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, - &policy, + &permission_profile, &file_system_policy, NetworkSandboxPolicy::Restricted, &temp_dir.path().abs(), diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index f853ea3ba6..8a2a2849fd 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -22,10 +22,8 @@ use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; -use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxType; -use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; @@ -102,15 +100,6 @@ impl ExecRequest { } } - pub(crate) fn compatibility_sandbox_policy(&self) -> SandboxPolicy { - compatibility_sandbox_policy_for_permission_profile( - &self.permission_profile, - &self.file_system_sandbox_policy, - self.network_sandbox_policy, - self.windows_sandbox_policy_cwd.as_path(), - ) - } - pub(crate) fn from_sandbox_exec_request( request: SandboxExecRequest, options: ExecOptions, diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index a2c4ebe597..a23a1d749e 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -30,7 +30,7 @@ pub enum StdioPolicy { Inherit, } -/// Spawns the appropriate child process for the ExecParams and SandboxPolicy, +/// Spawns the appropriate child process for the exec params and sandbox settings, /// ensuring the args and environment variables used to create the `Command` /// (and `Child`) honor the configuration. ///