diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 7ed3b2976..3fc4ed0bd 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -72,13 +72,15 @@ impl App { "Failed to carry forward approval policy override: {err}" )); } - if let Some(policy) = self.runtime_sandbox_policy_override.as_ref() - && let Err(err) = config.permissions.sandbox_policy.set(policy.clone()) - { - tracing::warn!(%err, "failed to carry forward sandbox policy override"); - self.chat_widget.add_error_message(format!( - "Failed to carry forward sandbox policy override: {err}" - )); + if let Some(policy) = self.runtime_sandbox_policy_override.as_ref() { + if let Err(err) = config.permissions.sandbox_policy.set(policy.clone()) { + tracing::warn!(%err, "failed to carry forward sandbox policy override"); + self.chat_widget.add_error_message(format!( + "Failed to carry forward sandbox policy override: {err}" + )); + } else { + sync_runtime_permissions_from_legacy_sandbox_policy(config); + } } } @@ -117,6 +119,7 @@ impl App { .add_error_message(format!("{user_message_prefix}: {err}")); return false; } + sync_runtime_permissions_from_legacy_sandbox_policy(config); true } @@ -306,6 +309,10 @@ impl App { self.chat_widget .add_error_message(format!("Failed to enable Auto-review: {err}")); } + if sandbox_policy_override.is_some() { + self.runtime_sandbox_policy_override = + Some(self.config.permissions.sandbox_policy.get().clone()); + } if approval_policy_override.is_some() || approvals_reviewer_override.is_some() @@ -536,6 +543,17 @@ impl App { } } +fn sync_runtime_permissions_from_legacy_sandbox_policy(config: &mut Config) { + let sandbox_policy = config.permissions.sandbox_policy.get(); + config.permissions.file_system_sandbox_policy = + codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy( + sandbox_policy, + &config.cwd, + ); + config.permissions.network_sandbox_policy = + codex_protocol::permissions::NetworkSandboxPolicy::from(sandbox_policy); +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index bb6f8dbe6..b2141977c 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1643,7 +1643,10 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result< auto_review.approvals_reviewer ); assert_eq!(app.runtime_approval_policy_override, None); - assert_eq!(app.runtime_sandbox_policy_override, None); + assert_eq!( + app.runtime_sandbox_policy_override, + Some(auto_review.sandbox_policy.clone()) + ); assert_eq!( op_rx.try_recv(), Ok(Op::OverrideTurnContext { diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 241695cc2..406e9ea7d 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -571,6 +571,14 @@ impl App { } } if should_start_turn { + let permission_profile = if !app_server.is_remote() + && self.runtime_sandbox_policy_override.is_some() + && !matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. }) + { + Some(self.config.permissions.permission_profile()) + } else { + None + }; app_server .turn_start( thread_id, @@ -580,6 +588,7 @@ impl App { approvals_reviewer .unwrap_or(self.chat_widget.config_ref().approvals_reviewer), sandbox_policy.clone(), + permission_profile, model.to_string(), effort, *summary, diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index d33df0f68..302bcb5a6 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -3,6 +3,7 @@ use crate::app_server_session::ThreadSessionState; use crate::read_session_model; use codex_app_server_protocol::Thread; use codex_protocol::ThreadId; +use codex_protocol::protocol::SandboxPolicy; impl App { pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) { @@ -13,10 +14,22 @@ impl App { let approval_policy = self.config.permissions.approval_policy.value(); let approvals_reviewer = self.config.approvals_reviewer; let sandbox_policy = self.config.permissions.sandbox_policy.get().clone(); + let permission_profile = if matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. }) + { + None + } else { + Some( + self.chat_widget + .config_ref() + .permissions + .permission_profile(), + ) + }; let update_session = |session: &mut ThreadSessionState| { session.approval_policy = approval_policy; session.approvals_reviewer = approvals_reviewer; session.sandbox_policy = sandbox_policy.clone(); + session.permission_profile = permission_profile.clone(); }; if self.primary_thread_id == Some(active_thread_id) @@ -91,7 +104,14 @@ mod tests { use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use codex_config::types::ApprovalsReviewer; + use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; + use codex_protocol::protocol::FileSystemAccessMode; + use codex_protocol::protocol::FileSystemPath; + use codex_protocol::protocol::FileSystemSandboxEntry; + use codex_protocol::protocol::FileSystemSandboxPolicy; + use codex_protocol::protocol::FileSystemSpecialPath; + use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -157,8 +177,17 @@ mod tests { app.config.permissions.approval_policy = codex_config::Constrained::allow_any(AskForApproval::OnRequest); app.config.approvals_reviewer = ApprovalsReviewer::AutoReview; + let expected_sandbox_policy = SandboxPolicy::new_workspace_write_policy(); + let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy( + &expected_sandbox_policy, + &main_session.cwd, + ); + app.chat_widget.handle_thread_session(main_session.clone()); + app.chat_widget + .set_sandbox_policy(expected_sandbox_policy.clone()) + .expect("set widget sandbox policy"); app.config.permissions.sandbox_policy = - codex_config::Constrained::allow_any(SandboxPolicy::new_workspace_write_policy()); + codex_config::Constrained::allow_any(expected_sandbox_policy.clone()); app.sync_active_thread_permission_settings_to_cached_session() .await; @@ -166,7 +195,8 @@ mod tests { let expected_main_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::AutoReview, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + sandbox_policy: expected_sandbox_policy, + permission_profile: Some(expected_permission_profile), ..main_session }; assert_eq!( @@ -196,4 +226,67 @@ mod tests { .clone(); assert_eq!(side_store_session, Some(side_session)); } + + #[tokio::test] + async fn permission_settings_sync_preserves_active_profile_only_rules() { + let mut app = make_test_app().await; + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000403").expect("valid thread"); + let profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/.env".to_string(), + }, + access: FileSystemAccessMode::None, + }, + ]), + NetworkSandboxPolicy::Restricted, + ); + let session = ThreadSessionState { + permission_profile: Some(profile.clone()), + ..test_thread_session(thread_id, test_path_buf("/tmp/main")) + }; + + app.primary_thread_id = Some(thread_id); + app.active_thread_id = Some(thread_id); + app.primary_session_configured = Some(session.clone()); + app.thread_event_channels.insert( + thread_id, + ThreadEventChannel::new_with_session(/*capacity*/ 4, session.clone(), Vec::new()), + ); + app.chat_widget.handle_thread_session(session.clone()); + app.config.permissions.approval_policy = + codex_config::Constrained::allow_any(AskForApproval::OnRequest); + + app.sync_active_thread_permission_settings_to_cached_session() + .await; + + let expected_session = ThreadSessionState { + approval_policy: AskForApproval::OnRequest, + permission_profile: Some(profile), + ..session + }; + assert_eq!( + app.primary_session_configured, + Some(expected_session.clone()) + ); + + let store_session = app + .thread_event_channels + .get(&thread_id) + .expect("thread channel") + .store + .lock() + .await + .session + .clone(); + assert_eq!(store_session, Some(expected_session)); + } } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index b1f1e01c9..8992d97d6 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -526,6 +526,7 @@ impl AppServerSession { approval_policy: AskForApproval, approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, sandbox_policy: SandboxPolicy, + permission_profile: Option, model: String, effort: Option, summary: Option, @@ -535,13 +536,11 @@ impl AppServerSession { output_schema: Option, ) -> Result { let request_id = self.next_request_id(); - let sandbox_policy = if self.is_remote() - || matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. }) - { - Some(sandbox_policy.into()) - } else { - None - }; + let (sandbox_policy, permission_profile) = turn_start_permission_overrides( + self.thread_params_mode(), + sandbox_policy, + permission_profile, + ); self.client .request_typed(ClientRequest::TurnStart { request_id, @@ -553,11 +552,8 @@ impl AppServerSession { cwd: Some(cwd), approval_policy: Some(approval_policy.into()), approvals_reviewer: Some(approvals_reviewer.into()), - // Embedded sessions already installed their full profile - // at thread start/resume/fork. Avoid sending a lossy - // legacy projection until user turns carry profiles. sandbox_policy, - permission_profile: None, + permission_profile, model: Some(model), service_tier, effort, @@ -1046,6 +1042,26 @@ fn sandbox_mode_from_policy( } } +fn turn_start_permission_overrides( + mode: ThreadParamsMode, + sandbox_policy: SandboxPolicy, + permission_profile: Option, +) -> ( + Option, + Option, +) { + let is_external_sandbox = matches!(&sandbox_policy, SandboxPolicy::ExternalSandbox { .. }); + match (mode, is_external_sandbox, permission_profile) { + (ThreadParamsMode::Embedded, false, Some(permission_profile)) => { + (None, Some(permission_profile.into())) + } + (ThreadParamsMode::Embedded, false, None) => (None, None), + (ThreadParamsMode::Embedded, true, _) | (ThreadParamsMode::Remote, _, _) => { + (Some(sandbox_policy.into()), None) + } + } +} + fn permission_profile_override_from_config( config: &Config, thread_params_mode: ThreadParamsMode, @@ -1530,6 +1546,55 @@ mod tests { assert_eq!(fork.permission_profile, None); } + #[test] + fn turn_start_permission_overrides_send_profiles_only_for_embedded_runtime_overrides() { + let cwd = test_path_buf("/tmp/project"); + let workspace_write = SandboxPolicy::new_workspace_write_policy(); + let workspace_write_profile = + PermissionProfile::from_legacy_sandbox_policy(&workspace_write, &cwd); + + let (sandbox, profile) = turn_start_permission_overrides( + ThreadParamsMode::Embedded, + workspace_write.clone(), + Some(workspace_write_profile.clone()), + ); + assert_eq!(sandbox, None); + assert_eq!(profile, Some(workspace_write_profile.into())); + + let (sandbox, profile) = turn_start_permission_overrides( + ThreadParamsMode::Embedded, + workspace_write.clone(), + /*permission_profile*/ None, + ); + assert_eq!(sandbox, None); + assert_eq!(profile, None); + + let (sandbox, profile) = turn_start_permission_overrides( + ThreadParamsMode::Remote, + workspace_write.clone(), + Some(PermissionProfile::from_legacy_sandbox_policy( + &workspace_write, + &cwd, + )), + ); + assert_eq!(sandbox, Some(workspace_write.into())); + assert_eq!(profile, None); + + let external_sandbox = SandboxPolicy::ExternalSandbox { + network_access: codex_protocol::protocol::NetworkAccess::Restricted, + }; + let (sandbox, profile) = turn_start_permission_overrides( + ThreadParamsMode::Embedded, + external_sandbox.clone(), + Some(PermissionProfile::from_legacy_sandbox_policy( + &external_sandbox, + &cwd, + )), + ); + assert_eq!(sandbox, Some(external_sandbox.into())); + assert_eq!(profile, None); + } + #[tokio::test] async fn thread_fork_params_forward_instruction_overrides() { let temp_dir = tempfile::tempdir().expect("tempdir"); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c6804b19c..927305c4b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2119,6 +2119,20 @@ impl ChatWidget { display: SessionConfiguredDisplay, fork_parent_title: Option, ) { + let (file_system_sandbox_policy, network_sandbox_policy) = match event + .permission_profile + .as_ref() + { + Some(permission_profile) => permission_profile.to_runtime_permissions(), + None => ( + codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &event.sandbox_policy, + &event.cwd, + ), + codex_protocol::permissions::NetworkSandboxPolicy::from(&event.sandbox_policy), + ), + }; + self.last_agent_markdown = None; self.agent_turn_markdowns.clear(); self.visible_user_turn_count = 0; @@ -2156,6 +2170,8 @@ impl ChatWidget { self.config.permissions.sandbox_policy = Constrained::allow_only(event.sandbox_policy.clone()); } + self.config.permissions.file_system_sandbox_policy = file_system_sandbox_policy; + self.config.permissions.network_sandbox_policy = network_sandbox_policy; self.config.approvals_reviewer = event.approvals_reviewer; self.status_line_project_root_name_cache = None; let forked_from_id = event.forked_from_id; @@ -9764,6 +9780,14 @@ impl ChatWidget { #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn set_sandbox_policy(&mut self, policy: SandboxPolicy) -> ConstraintResult<()> { self.config.permissions.sandbox_policy.set(policy)?; + let sandbox_policy = self.config.permissions.sandbox_policy.get(); + self.config.permissions.file_system_sandbox_policy = + codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy( + sandbox_policy, + &self.config.cwd, + ); + self.config.permissions.network_sandbox_policy = + codex_protocol::permissions::NetworkSandboxPolicy::from(sandbox_policy); Ok(()) } diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 3e2f5b4d7..5f089ebbf 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -1,4 +1,12 @@ use super::*; +use codex_protocol::protocol::FileSystemAccessMode; +use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSandboxEntry; +use codex_protocol::protocol::FileSystemSandboxKind; +use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::FileSystemSpecialPath; +use codex_protocol::protocol::NetworkAccess; +use codex_protocol::protocol::NetworkSandboxPolicy; use pretty_assertions::assert_eq; #[tokio::test] @@ -252,6 +260,25 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { let expected_sandbox = SandboxPolicy::new_read_only_policy(); let expected_cwd = test_path_buf("/home/user/sub-agent").abs(); + let expected_file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/.secret".to_string(), + }, + access: FileSystemAccessMode::None, + }, + ]); + let expected_permission_profile = + codex_protocol::models::PermissionProfile::from_runtime_permissions( + &expected_file_system_policy, + NetworkSandboxPolicy::Restricted, + ); let configured = codex_protocol::protocol::SessionConfiguredEvent { session_id: ThreadId::new(), forked_from_id: None, @@ -262,7 +289,7 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: expected_sandbox.clone(), - permission_profile: None, + permission_profile: Some(expected_permission_profile.clone()), cwd: expected_cwd.clone(), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -285,7 +312,72 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { chat.config_ref().permissions.sandbox_policy.get(), &expected_sandbox ); + assert_eq!( + chat.config_ref().permissions.permission_profile(), + expected_permission_profile + ); assert_eq!(&chat.config_ref().cwd, &expected_cwd); + + let updated_sandbox = SandboxPolicy::new_workspace_write_policy(); + chat.set_sandbox_policy(updated_sandbox.clone()) + .expect("set sandbox policy"); + assert_eq!( + chat.config_ref().permissions.permission_profile(), + codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy( + &updated_sandbox, + &expected_cwd + ), + "local sandbox changes should replace SessionConfigured profile-derived runtime permissions" + ); +} + +#[tokio::test] +async fn session_configured_external_sandbox_keeps_external_runtime_policy() { + let (mut chat, _rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; + + let expected_sandbox = SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Restricted, + }; + let configured = codex_protocol::protocol::SessionConfiguredEvent { + session_id: ThreadId::new(), + forked_from_id: None, + thread_name: None, + model: "test-model".to_string(), + model_provider_id: "test-provider".to_string(), + service_tier: None, + approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, + sandbox_policy: expected_sandbox.clone(), + permission_profile: None, + cwd: test_path_buf("/home/user/external").abs(), + reasoning_effort: Some(ReasoningEffortConfig::default()), + history_log_id: 0, + history_entry_count: 0, + initial_messages: None, + network_proxy: None, + rollout_path: None, + }; + + chat.handle_codex_event(Event { + id: "session-configured".into(), + msg: EventMsg::SessionConfigured(configured), + }); + + assert_eq!( + chat.config_ref().permissions.sandbox_policy.get(), + &expected_sandbox + ); + assert_eq!( + chat.config_ref() + .permissions + .file_system_sandbox_policy + .kind, + FileSystemSandboxKind::ExternalSandbox, + ); + assert_eq!( + chat.config_ref().permissions.network_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); } #[tokio::test]