From c29bc99949f90687171611bb845c4c289fa85dde Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 27 Aug 2026 15:47:50 +0000 Subject: [PATCH] Preserve restored permission profiles in TUI sessions (#41192) ## Why Restored app-server threads can carry permission profiles that cannot be represented without loss as legacy sandbox-mode overrides. Sending those profiles as per-turn overrides can discard their original permission details. ## What changed - Distinguish restored permission profiles from legacy per-turn sandbox overrides. - Preserve restored profiles while routing turns and changing to compatible working directories. - Reject `/cd` when a restored profile cannot be represented safely by a legacy sandbox mode. ## Testing - Cover retaining a restored read-only profile across `/cd` without a turn override. - Verify that lossy sandbox-mode projections are detected. GitOrigin-RevId: 6143dca4b83ab4f59bab05f6a59b4fe841c56192 --- codex-rs/tui/src/app.rs | 29 ++++++++ codex-rs/tui/src/app/agents_overview.rs | 10 +-- codex-rs/tui/src/app/agents_overview_tests.rs | 74 +++++++++++++++++++ codex-rs/tui/src/app/event_dispatch.rs | 2 +- codex-rs/tui/src/app/safety_buffering.rs | 2 +- .../app/tests/session_lifecycle_requests.rs | 18 ++++- codex-rs/tui/src/app/thread_routing.rs | 2 +- codex-rs/tui/src/app/working_directory.rs | 15 +++- codex-rs/tui/src/app_server_session.rs | 27 +++++++ 9 files changed, 167 insertions(+), 12 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 627b2f802c..6621bcab65 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -642,6 +642,13 @@ struct RuntimePermissionProfileOverride { permission_profile: PermissionProfile, active_permission_profile: Option, network: Option, + turn_override: RuntimePermissionProfileTurnOverride, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RuntimePermissionProfileTurnOverride { + Preserve, + LegacySandbox, } impl RuntimePermissionProfileOverride { @@ -650,8 +657,30 @@ impl RuntimePermissionProfileOverride { permission_profile: config.permissions.permission_profile().clone(), active_permission_profile: config.permissions.active_permission_profile(), network: config.permissions.network.clone(), + turn_override: RuntimePermissionProfileTurnOverride::LegacySandbox, } } + + fn from_restored_config(config: &Config) -> Self { + Self { + turn_override: RuntimePermissionProfileTurnOverride::Preserve, + ..Self::from_config(config) + } + } + + fn matches_config(&self, config: &Config) -> bool { + self.permission_profile == *config.permissions.permission_profile() + && self.active_permission_profile == config.permissions.active_permission_profile() + && self.network == config.permissions.network + } + + fn turn_permission_profile(&self) -> Option<&PermissionProfile> { + matches!( + self.turn_override, + RuntimePermissionProfileTurnOverride::LegacySandbox + ) + .then_some(&self.permission_profile) + } } fn active_turn_not_steerable_turn_error(error: &TypedRequestError) -> Option { diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 5d6b552c35..e30e8574c3 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -482,10 +482,10 @@ impl App { self.runtime_approval_policy_override = Some(self.config.permissions.approval_policy.value().into()); } - let destination_permissions = - RuntimePermissionProfileOverride::from_config(&self.config); - if destination_permissions != baseline_permissions { - self.runtime_permission_profile_override = Some(destination_permissions); + if !baseline_permissions.matches_config(&self.config) { + self.runtime_permission_profile_override = Some( + RuntimePermissionProfileOverride::from_restored_config(&self.config), + ); } if !self .backfill_loaded_subagent_threads(app_server) @@ -587,7 +587,7 @@ impl App { }; if let Some(profile) = self.runtime_permission_profile_override.as_ref() && profile.active_permission_profile.is_some() - && (RuntimePermissionProfileOverride::from_config(&config) != *profile + && (!profile.matches_config(&config) || config.permissions.profile_workspace_roots() != self.config.permissions.profile_workspace_roots()) { diff --git a/codex-rs/tui/src/app/agents_overview_tests.rs b/codex-rs/tui/src/app/agents_overview_tests.rs index 87b7210fe0..9b35bc270e 100644 --- a/codex-rs/tui/src/app/agents_overview_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_tests.rs @@ -19,6 +19,7 @@ use codex_app_server_protocol::ThreadUnsubscribeStatus; use codex_config::types::KeybindingSpec; use codex_config::types::KeybindingsSpec; use codex_config::types::TuiKeymap; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SubAgentSource; static OVERVIEW_TIMESTAMP: std::sync::LazyLock = @@ -511,3 +512,76 @@ async fn root_switch_preserves_idle_root_with_running_subagent() -> Result<()> { app_server.shutdown().await?; Ok(()) } + +#[tokio::test] +async fn restored_server_permission_profile_survives_cd_without_turn_override() -> Result<()> { + let mut app = make_test_app().await; + let destination = app.config.codex_home.join("destination"); + std::fs::create_dir(&destination)?; + crate::legacy_core::config::set_project_trust_level( + app.config.codex_home.as_path(), + &destination, + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?; + let mut app_server = + crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?; + let previous = app_server.start_thread(&app.config).await?; + app.enqueue_primary_thread_session(previous.session, previous.turns) + .await?; + let target = app_server.start_thread(&app.config).await?; + let target_thread_id = target.session.thread_id; + let mut tui = crate::tui::test_support::make_test_tui()?; + + app.select_agents_overview_thread(&mut tui, &mut app_server, target_thread_id) + .await?; + app.config + .permissions + .set_permission_profile(PermissionProfile::read_only())?; + app.chat_widget.set_permission_profile_with_active_profile( + PermissionProfile::read_only(), + /*active_permission_profile*/ None, + )?; + app.runtime_permission_profile_override = Some( + RuntimePermissionProfileOverride::from_restored_config(app.chat_widget.config_ref()), + ); + + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .effective_permission_profile(), + PermissionProfile::read_only() + ); + assert_eq!( + app.runtime_permission_profile_override + .as_ref() + .map(|profile| profile.turn_override), + Some(RuntimePermissionProfileTurnOverride::Preserve) + ); + assert_eq!( + App::turn_permissions_override_from_config( + app.chat_widget.config_ref(), + /*active_permission_profile*/ None, + app.runtime_permission_profile_override + .as_ref() + .and_then(RuntimePermissionProfileOverride::turn_permission_profile), + ), + TurnPermissionsOverride::Preserve + ); + + app.change_working_directory(&mut tui, &mut app_server, destination.clone()) + .await; + + assert_eq!(app.chat_widget.config_ref().cwd, destination); + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .effective_permission_profile(), + PermissionProfile::read_only() + ); + + app_server.shutdown().await?; + Ok(()) +} diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 9f148b5a83..84c22011bf 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1507,7 +1507,7 @@ impl App { config.permissions.active_permission_profile().as_ref(), self.runtime_permission_profile_override .as_ref() - .map(|profile| &profile.permission_profile), + .and_then(RuntimePermissionProfileOverride::turn_permission_profile), ); if turn_permissions_overrides(permissions_override, config.cwd.as_path()) .is_ok() diff --git a/codex-rs/tui/src/app/safety_buffering.rs b/codex-rs/tui/src/app/safety_buffering.rs index c9d0ee900b..132b8cbe62 100644 --- a/codex-rs/tui/src/app/safety_buffering.rs +++ b/codex-rs/tui/src/app/safety_buffering.rs @@ -71,7 +71,7 @@ impl App { active_permission_profile.as_ref(), self.runtime_permission_profile_override .as_ref() - .map(|profile| &profile.permission_profile), + .and_then(RuntimePermissionProfileOverride::turn_permission_profile), ); if let Err(err) = turn_permissions_overrides(permissions_override, cwd.as_path()) { self.chat_widget diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index fba09c5329..285b21c8aa 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -2638,6 +2638,11 @@ async fn changing_directory_preserves_project_trust_permissions_history_and_hook ("../trusted", "profile", "permission profile override"), ("../trusted", "reviewer", "reviewer"), ("../p", "named", "different settings"), + ( + "../trusted", + "restored", + "Permission profile cannot be preserved", + ), ("../p", "keymap", "open_transcript"), ("../unknown", "local", "This directory is not trusted"), ("../trusted", "main", "background terminals"), @@ -2659,8 +2664,17 @@ async fn changing_directory_preserves_project_trust_permissions_history_and_hook let mut profile = RuntimePermissionProfileOverride::from_config(&app.config); profile.active_permission_profile = (kind == "named").then(|| ActivePermissionProfile::new("dev")); + if kind == "restored" { + profile.permission_profile = PermissionProfile::workspace_write_with( + &[failed.clone().abs()], + codex_protocol::permissions::NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ); + profile.turn_override = RuntimePermissionProfileTurnOverride::Preserve; + } app.runtime_permission_profile_override = - matches!(kind, "profile" | "reviewer" | "named").then_some(profile); + matches!(kind, "profile" | "reviewer" | "named" | "restored").then_some(profile); app.app_server_target = crate::AppServerTarget::Embedded; if kind == "workspace" { let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?; @@ -2691,6 +2705,8 @@ async fn changing_directory_preserves_project_trust_permissions_history_and_hook let output = history().join(""); if kind == "mcp" { assert_snapshot!(output, @"■ MCP inventory is still loading."); + } else if kind == "restored" { + assert_snapshot!(output, @"■ Permission profile cannot be preserved by /cd."); } assert!(output.contains(expected), "{path}"); app.clear_committed_mcp_inventory_loading(); diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 8b5174f0d7..febc60b970 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -739,7 +739,7 @@ impl App { active_permission_profile.as_ref(), self.runtime_permission_profile_override .as_ref() - .map(|profile| &profile.permission_profile), + .and_then(RuntimePermissionProfileOverride::turn_permission_profile), ); let response = app_server .turn_start( diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index c2317b9714..c5d96291db 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -57,12 +57,22 @@ impl App { } if let Some(profile) = self.runtime_permission_profile_override.as_ref() && profile.active_permission_profile.is_some() - && (RuntimePermissionProfileOverride::from_config(&config) != *profile + && (!profile.matches_config(&config) || config.permissions.profile_workspace_roots() != self.config.permissions.profile_workspace_roots()) { return self.working_directory_error("Permission profile has different settings."); } + if let Some(profile) = self.runtime_permission_profile_override.as_ref() + && profile.turn_override == RuntimePermissionProfileTurnOverride::Preserve + && profile.active_permission_profile.is_none() + && !crate::app_server_session::permission_profile_is_safely_represented_by_sandbox_mode( + &profile.permission_profile, + cwd.as_path(), + ) + { + return self.working_directory_error("Permission profile cannot be preserved by /cd."); + } self.apply_runtime_policy_overrides(&mut config); if self.runtime_permission_profile_override.is_some() { let reviewer = self.config.approvals_reviewer; @@ -73,11 +83,10 @@ impl App { config.approvals_reviewer = reviewer; } let actual = config.permissions.approval_policy.value(); - let snapshot = RuntimePermissionProfileOverride::from_config(&config); let approval = self.runtime_approval_policy_override; let profile = self.runtime_permission_profile_override.as_ref(); if approval.is_some_and(|p| actual != p.to_core()) - || profile.is_some_and(|p| snapshot != *p) + || profile.is_some_and(|profile| !profile.matches_config(&config)) { return; } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index ae0a5977c7..c1c928e4aa 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1789,6 +1789,29 @@ fn sandbox_mode_from_permission_profile( } } +pub(crate) fn permission_profile_is_safely_represented_by_sandbox_mode( + permission_profile: &PermissionProfile, + cwd: &std::path::Path, +) -> bool { + let Some(sandbox_mode) = sandbox_mode_from_permission_profile(permission_profile, cwd) else { + return false; + }; + let projected_profile = match sandbox_mode { + codex_app_server_protocol::SandboxMode::ReadOnly => PermissionProfile::read_only(), + codex_app_server_protocol::SandboxMode::WorkspaceWrite => { + PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &codex_protocol::protocol::SandboxPolicy::new_workspace_write_policy(), + cwd, + ) + } + codex_app_server_protocol::SandboxMode::DangerFullAccess => PermissionProfile::Disabled, + }; + permission_profile.network_sandbox_policy() == projected_profile.network_sandbox_policy() + && permission_profile + .file_system_sandbox_policy() + .is_semantically_equivalent_to(&projected_profile.file_system_sandbox_policy(), cwd) +} + fn permission_profile_id_from_active_profile(active: ActivePermissionProfile) -> String { active.id } @@ -3001,6 +3024,10 @@ mod tests { sandbox_mode_from_permission_profile(&permission_profile, cwd.as_path()), Some(codex_app_server_protocol::SandboxMode::ReadOnly) ); + assert!(!permission_profile_is_safely_represented_by_sandbox_mode( + &permission_profile, + cwd.as_path(), + )); } #[test]