From 619bc15dcc4558ab732e2a4eb8a011276dc252fe Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 29 Apr 2026 23:04:22 -0700 Subject: [PATCH] tui: stop referencing SandboxMode --- .../app-server-protocol/src/protocol/v2.rs | 97 +++++++++++ codex-rs/tui/src/app_server_session.rs | 153 ++++-------------- codex-rs/tui/src/lib.rs | 26 +-- 3 files changed, 147 insertions(+), 129 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 2c1f2abc30..98b01f01b7 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -413,6 +413,32 @@ impl From for SandboxMode { } } +/// Projects a runtime permission profile into the legacy thread `sandbox` +/// override field. This is intentionally lossy and should only be used when a +/// client cannot send the newer permissions selection API. +pub fn thread_sandbox_override_for_permission_profile( + permission_profile: &codex_protocol::models::PermissionProfile, + cwd: &std::path::Path, +) -> Option { + match permission_profile { + codex_protocol::models::PermissionProfile::Disabled => Some(SandboxMode::DangerFullAccess), + codex_protocol::models::PermissionProfile::External { .. } => None, + codex_protocol::models::PermissionProfile::Managed { .. } => { + let file_system_policy = permission_profile.file_system_sandbox_policy(); + if file_system_policy.has_full_disk_write_access() { + permission_profile + .network_sandbox_policy() + .is_enabled() + .then_some(SandboxMode::DangerFullAccess) + } else if file_system_policy.can_write_path_with_cwd(cwd, cwd) { + Some(SandboxMode::WorkspaceWrite) + } else { + Some(SandboxMode::ReadOnly) + } + } + } +} + v2_enum_from_core!( pub enum ReviewDelivery from codex_protocol::protocol::ReviewDelivery { Inline, Detached @@ -7993,6 +8019,8 @@ mod tests { use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::items::WebSearchItem; + use codex_protocol::models::ManagedFileSystemPermissions as CoreManagedFileSystemPermissionsForTest; + use codex_protocol::models::PermissionProfile as CorePermissionProfileForTest; use codex_protocol::models::WebSearchAction as CoreWebSearchAction; use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess; use codex_protocol::user_input::UserInput as CoreUserInput; @@ -8041,6 +8069,75 @@ mod tests { } } + #[test] + fn thread_sandbox_override_preserves_legacy_remote_projection() { + let cwd = test_path_buf("/workspace/project").abs(); + let extra_root = test_path_buf("/workspace/cache").abs(); + let non_cwd_write_profile = CorePermissionProfileForTest::Managed { + file_system: CoreManagedFileSystemPermissionsForTest::Restricted { + entries: vec![ + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: CoreFileSystemSpecialPath::Root, + }, + access: CoreFileSystemAccessMode::Read, + }, + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Path { path: extra_root }, + access: CoreFileSystemAccessMode::Write, + }, + ], + glob_scan_max_depth: None, + }, + network: CoreNetworkSandboxPolicy::Restricted, + }; + let cwd_write_profile = CorePermissionProfileForTest::Managed { + file_system: CoreManagedFileSystemPermissionsForTest::Restricted { + entries: vec![ + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: CoreFileSystemSpecialPath::Root, + }, + access: CoreFileSystemAccessMode::Read, + }, + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: CoreFileSystemSpecialPath::ProjectRoots { subpath: None }, + }, + access: CoreFileSystemAccessMode::Write, + }, + ], + glob_scan_max_depth: None, + }, + network: CoreNetworkSandboxPolicy::Restricted, + }; + + assert_eq!( + thread_sandbox_override_for_permission_profile( + &CorePermissionProfileForTest::Disabled, + cwd.as_path() + ), + Some(SandboxMode::DangerFullAccess) + ); + assert_eq!( + thread_sandbox_override_for_permission_profile( + &CorePermissionProfileForTest::External { + network: CoreNetworkSandboxPolicy::Restricted, + }, + cwd.as_path(), + ), + None + ); + assert_eq!( + thread_sandbox_override_for_permission_profile(&non_cwd_write_profile, cwd.as_path()), + Some(SandboxMode::ReadOnly) + ); + assert_eq!( + thread_sandbox_override_for_permission_profile(&cwd_write_profile, cwd.as_path()), + Some(SandboxMode::WorkspaceWrite) + ); + } + #[test] fn thread_list_params_accepts_single_cwd() { let params = serde_json::from_value::(json!({ diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 263838e7b1..a33c97fd37 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1103,31 +1103,6 @@ fn config_request_overrides_from_config( }) } -fn sandbox_mode_from_permission_profile( - permission_profile: &PermissionProfile, - cwd: &std::path::Path, -) -> Option { - match permission_profile { - PermissionProfile::Disabled => { - Some(codex_app_server_protocol::SandboxMode::DangerFullAccess) - } - PermissionProfile::External { .. } => None, - PermissionProfile::Managed { .. } => { - let file_system_policy = permission_profile.file_system_sandbox_policy(); - if file_system_policy.has_full_disk_write_access() { - 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) { - Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite) - } else { - Some(codex_app_server_protocol::SandboxMode::ReadOnly) - } - } - } -} - fn permissions_selection_from_active_profile( active: ActivePermissionProfile, ) -> PermissionProfileSelectionParams { @@ -1195,15 +1170,14 @@ fn thread_start_params_from_config( session_start_source: Option, ) -> ThreadStartParams { let permissions = permissions_selection_from_config(config, thread_params_mode); - let sandbox = permissions - .is_none() - .then(|| { - sandbox_mode_from_permission_profile( - &config.permissions.permission_profile(), - config.cwd.as_path(), - ) - }) - .flatten(); + let sandbox = if permissions.is_none() { + codex_app_server_protocol::thread_sandbox_override_for_permission_profile( + &config.permissions.permission_profile(), + config.cwd.as_path(), + ) + } else { + None + }; ThreadStartParams { model: config.model.clone(), model_provider: thread_params_mode.model_provider_from_config(config), @@ -1227,15 +1201,14 @@ fn thread_resume_params_from_config( remote_cwd_override: Option<&std::path::Path>, ) -> ThreadResumeParams { let permissions = permissions_selection_from_config(&config, thread_params_mode); - let sandbox = permissions - .is_none() - .then(|| { - sandbox_mode_from_permission_profile( - &config.permissions.permission_profile(), - config.cwd.as_path(), - ) - }) - .flatten(); + let sandbox = if permissions.is_none() { + codex_app_server_protocol::thread_sandbox_override_for_permission_profile( + &config.permissions.permission_profile(), + config.cwd.as_path(), + ) + } else { + None + }; ThreadResumeParams { thread_id: thread_id.to_string(), model: config.model.clone(), @@ -1258,15 +1231,14 @@ fn thread_fork_params_from_config( remote_cwd_override: Option<&std::path::Path>, ) -> ThreadForkParams { let permissions = permissions_selection_from_config(&config, thread_params_mode); - let sandbox = permissions - .is_none() - .then(|| { - sandbox_mode_from_permission_profile( - &config.permissions.permission_profile(), - config.cwd.as_path(), - ) - }) - .flatten(); + let sandbox = if permissions.is_none() { + codex_app_server_protocol::thread_sandbox_override_for_permission_profile( + &config.permissions.permission_profile(), + config.cwd.as_path(), + ) + } else { + None + }; ThreadForkParams { thread_id: thread_id.to_string(), model: config.model.clone(), @@ -1714,10 +1686,11 @@ mod tests { let temp_dir = tempfile::tempdir().expect("tempdir"); let config = build_config(&temp_dir).await; let thread_id = ThreadId::new(); - let expected_sandbox = sandbox_mode_from_permission_profile( - &config.permissions.permission_profile(), - config.cwd.as_path(), - ); + let expected_sandbox = + codex_app_server_protocol::thread_sandbox_override_for_permission_profile( + &config.permissions.permission_profile(), + config.cwd.as_path(), + ); let start = thread_start_params_from_config( &config, @@ -1752,75 +1725,17 @@ mod tests { assert_eq!(fork.permissions, None); } - #[test] - fn sandbox_mode_does_not_project_non_cwd_write_roots_for_remote_sessions() { - let cwd = test_path_buf("/workspace/project").abs(); - let extra_root = test_path_buf("/workspace/cache").abs(); - let permission_profile = PermissionProfile::Managed { - file_system: ManagedFileSystemPermissions::Restricted { - entries: vec![ - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Path { path: extra_root }, - access: FileSystemAccessMode::Write, - }, - ], - glob_scan_max_depth: None, - }, - network: NetworkSandboxPolicy::Restricted, - }; - - assert_eq!( - sandbox_mode_from_permission_profile(&permission_profile, cwd.as_path()), - Some(codex_app_server_protocol::SandboxMode::ReadOnly) - ); - } - - #[test] - fn sandbox_mode_projects_cwd_write_for_remote_sessions() { - let cwd = test_path_buf("/workspace/project").abs(); - let permission_profile = PermissionProfile::Managed { - file_system: ManagedFileSystemPermissions::Restricted { - entries: vec![ - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::ProjectRoots { subpath: None }, - }, - access: FileSystemAccessMode::Write, - }, - ], - glob_scan_max_depth: None, - }, - network: NetworkSandboxPolicy::Restricted, - }; - - assert_eq!( - sandbox_mode_from_permission_profile(&permission_profile, cwd.as_path()), - Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite) - ); - } - #[tokio::test] async fn thread_lifecycle_params_forward_explicit_remote_cwd_override_for_remote_sessions() { let temp_dir = tempfile::tempdir().expect("tempdir"); let config = build_config(&temp_dir).await; let thread_id = ThreadId::new(); let remote_cwd = PathBuf::from("repo/on/server"); - let expected_sandbox = sandbox_mode_from_permission_profile( - &config.permissions.permission_profile(), - config.cwd.as_path(), - ); + let expected_sandbox = + codex_app_server_protocol::thread_sandbox_override_for_permission_profile( + &config.permissions.permission_profile(), + config.cwd.as_path(), + ); let start = thread_start_params_from_config( &config, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 39b4294b4a..505f97aec5 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -44,8 +44,8 @@ use codex_login::default_client::set_default_client_residency_requirement; use codex_login::enforce_login_restrictions; use codex_protocol::ThreadId; use codex_protocol::config_types::AltScreenMode; -use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; @@ -697,16 +697,15 @@ pub async fn run_main( .cwd .clone() .filter(|_| matches!(app_server_target, AppServerTarget::Remote { .. })); - let (sandbox_mode, approval_policy) = if cli.dangerously_bypass_approvals_and_sandbox { - ( - Some(SandboxMode::DangerFullAccess), - Some(AskForApproval::Never), - ) + let approval_policy = if cli.dangerously_bypass_approvals_and_sandbox { + Some(AskForApproval::Never) } else { - ( - cli.sandbox_mode.map(Into::::into), - cli.approval_policy.map(Into::into), - ) + cli.approval_policy.map(Into::into) + }; + let sandbox_mode = if cli.dangerously_bypass_approvals_and_sandbox { + None + } else { + cli.sandbox_mode.map(Into::into) }; // Map the legacy --search flag to the canonical web_search mode. @@ -789,6 +788,12 @@ pub async fn run_main( tracing::warn!(error = %err, "failed to run personality migration"); } + let permission_profile = if cli.dangerously_bypass_approvals_and_sandbox { + Some(PermissionProfile::Disabled) + } else { + None + }; + let chatgpt_base_url = config_toml .chatgpt_base_url .clone() @@ -843,6 +848,7 @@ pub async fn run_main( model, approval_policy, sandbox_mode, + permission_profile, cwd: if matches!(app_server_target, AppServerTarget::Remote { .. }) { None } else {