From 41a32d7f5cab78f19aa22c15015b2f048409db87 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 22 Apr 2026 16:05:42 -0700 Subject: [PATCH] clients: send permission profiles to app-server --- codex-rs/exec/src/lib.rs | 37 +++++++- codex-rs/exec/src/lib_tests.rs | 5 + codex-rs/tui/src/app/tests.rs | 78 +++++++++++++++ codex-rs/tui/src/app/thread_events.rs | 5 + codex-rs/tui/src/app/thread_session_state.rs | 5 +- codex-rs/tui/src/app_server_session.rs | 99 +++++++++++++++++++- 6 files changed, 220 insertions(+), 9 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 43c5e452507c..c8059ad5fed5 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -738,6 +738,10 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { items, output_schema, } => { + let permission_profile = permission_profile_override_from_config(&config); + let sandbox_policy = permission_profile + .is_none() + .then(|| default_sandbox_policy.clone().into()); let response: TurnStartResponse = send_request_with_response( &client, ClientRequest::TurnStart { @@ -750,8 +754,8 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { cwd: Some(default_cwd), approval_policy: Some(default_approval_policy.into()), approvals_reviewer: None, - sandbox_policy: Some(default_sandbox_policy.clone().into()), - permission_profile: None, + sandbox_policy, + permission_profile, model: None, service_tier: None, effort: default_effort, @@ -925,13 +929,19 @@ fn sandbox_mode_from_policy( } fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { + let permission_profile = permission_profile_override_from_config(config); + let sandbox = permission_profile + .is_none() + .then(|| sandbox_mode_from_policy(config.permissions.sandbox_policy.get())) + .flatten(); ThreadStartParams { model: config.model.clone(), model_provider: Some(config.model_provider_id.clone()), cwd: Some(config.cwd.to_string_lossy().to_string()), approval_policy: Some(config.permissions.approval_policy.value().into()), approvals_reviewer: approvals_reviewer_override_from_config(config), - sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()), + sandbox, + permission_profile, config: config_request_overrides_from_config(config), ephemeral: Some(config.ephemeral), ..ThreadStartParams::default() @@ -939,6 +949,11 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { } fn thread_resume_params_from_config(config: &Config, thread_id: String) -> ThreadResumeParams { + let permission_profile = permission_profile_override_from_config(config); + let sandbox = permission_profile + .is_none() + .then(|| sandbox_mode_from_policy(config.permissions.sandbox_policy.get())) + .flatten(); ThreadResumeParams { thread_id, model: config.model.clone(), @@ -946,12 +961,26 @@ fn thread_resume_params_from_config(config: &Config, thread_id: String) -> Threa cwd: Some(config.cwd.to_string_lossy().to_string()), approval_policy: Some(config.permissions.approval_policy.value().into()), approvals_reviewer: approvals_reviewer_override_from_config(config), - sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()), + sandbox, + permission_profile, config: config_request_overrides_from_config(config), ..ThreadResumeParams::default() } } +fn permission_profile_override_from_config( + config: &Config, +) -> Option { + if matches!( + config.permissions.sandbox_policy.get(), + SandboxPolicy::ExternalSandbox { .. } + ) { + None + } else { + Some(config.permissions.permission_profile().into()) + } +} + fn config_request_overrides_from_config(config: &Config) -> Option> { config .active_profile diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index 708cc9d0db77..50fe8552ba6b 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -361,6 +361,11 @@ async fn thread_start_params_include_review_policy_when_review_policy_is_manual_ params.approvals_reviewer, Some(codex_app_server_protocol::ApprovalsReviewer::User) ); + assert_eq!(params.sandbox, None); + assert_eq!( + params.permission_profile, + Some(config.permissions.permission_profile().into()) + ); } #[tokio::test] diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 87b7faa34b80..674e3204d877 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2215,6 +2215,10 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> { ThreadSessionState { approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/agent"), + )), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) }, @@ -2374,6 +2378,10 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()> ThreadSessionState { approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/agent"), + )), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) }, @@ -2596,6 +2604,10 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio ThreadSessionState { approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/agent"), + )), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) }, @@ -2649,6 +2661,10 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re let primary_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/main"), + )), ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) }; @@ -2760,6 +2776,10 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_ let primary_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/main"), + )), ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) }; @@ -2815,6 +2835,60 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_ Ok(()) } +/// `thread/read` is metadata/replay hydration and does not return an +/// authoritative runtime `PermissionProfile`, so it must not reuse the active +/// primary session profile after swapping in the read thread's cwd. +#[tokio::test] +async fn thread_read_session_state_does_not_reuse_primary_permission_profile() { + let mut app = make_test_app().await; + let main_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000401").expect("valid thread"); + let read_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000402").expect("valid thread"); + let primary_session = ThreadSessionState { + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + std::path::Path::new("/tmp/main"), + )), + ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) + }; + app.primary_session_configured = Some(primary_session); + + let thread = Thread { + id: read_thread_id.to_string(), + forked_from_id: None, + preview: "read thread".to_string(), + ephemeral: false, + model_provider: "read-provider".to_string(), + created_at: 1, + updated_at: 2, + status: codex_app_server_protocol::ThreadStatus::Idle, + path: None, + cwd: test_path_buf("/tmp/read").abs(), + cli_version: "0.0.0".to_string(), + source: codex_app_server_protocol::SessionSource::Unknown, + agent_nickname: None, + agent_role: None, + git_info: None, + name: Some("read thread".to_string()), + turns: Vec::new(), + }; + + let session = app + .session_state_for_thread_read(read_thread_id, &thread) + .await; + + assert_eq!(session.thread_id, read_thread_id); + assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/read").as_path()); + assert_eq!( + session.permission_profile, None, + "thread/read does not return an authoritative permission profile; reusing the primary \ + session profile would reinterpret cwd-bound entries against the read thread cwd" + ); +} + #[test] fn agent_picker_item_name_snapshot() { let thread_id = @@ -3675,6 +3749,10 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + cwd.as_path(), + )), cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index 89cef2332c75..10415c9f46ec 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -285,6 +285,7 @@ mod tests { use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnStartedNotification; use codex_config::types::ApprovalsReviewer; + use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use pretty_assertions::assert_eq; @@ -302,6 +303,10 @@ mod tests { approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), + permission_profile: Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + cwd.as_path(), + )), cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index a8fd34769e2b..49a715affc8d 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -10,6 +10,7 @@ impl App { thread_id: ThreadId, thread: &Thread, ) -> ThreadSessionState { + let sandbox_policy = self.config.permissions.sandbox_policy.get().clone(); let mut session = self .primary_session_configured .clone() @@ -23,7 +24,8 @@ impl App { service_tier: self.chat_widget.current_service_tier(), approval_policy: self.config.permissions.approval_policy.value(), approvals_reviewer: self.config.approvals_reviewer, - sandbox_policy: self.config.permissions.sandbox_policy.get().clone(), + sandbox_policy, + permission_profile: None, cwd: thread.cwd.clone(), instruction_source_paths: Vec::new(), reasoning_effort: self.chat_widget.current_reasoning_effort(), @@ -36,6 +38,7 @@ impl App { session.thread_name = thread.name.clone(); session.model_provider_id = thread.model_provider.clone(); session.cwd = thread.cwd.clone(); + session.permission_profile = None; session.instruction_source_paths = Vec::new(); session.rollout_path = thread.path.clone(); if let Some(model) = diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 8470c575ddc8..b1f1e01c9ac1 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -85,6 +85,7 @@ use codex_app_server_protocol::TurnSteerParams; use codex_app_server_protocol::TurnSteerResponse; use codex_otel::TelemetryAuthMode; use codex_protocol::ThreadId; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelAvailabilityNux; use codex_protocol::openai_models::ModelPreset; @@ -148,7 +149,13 @@ pub(crate) struct ThreadSessionState { pub(crate) service_tier: Option, pub(crate) approval_policy: AskForApproval, pub(crate) approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, + /// Legacy sandbox projection kept for compatibility. Use this only when + /// `permission_profile` is `None`. pub(crate) sandbox_policy: SandboxPolicy, + /// Canonical active permissions when available. Consumers should prefer + /// this over `sandbox_policy`; `None` means the session only has a legacy + /// sandbox projection or represents an external sandbox. + pub(crate) permission_profile: Option, pub(crate) cwd: AbsolutePathBuf, pub(crate) instruction_source_paths: Vec, pub(crate) reasoning_effort: Option, @@ -528,6 +535,13 @@ 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 + }; self.client .request_typed(ClientRequest::TurnStart { request_id, @@ -539,7 +553,10 @@ impl AppServerSession { cwd: Some(cwd), approval_policy: Some(approval_policy.into()), approvals_reviewer: Some(approvals_reviewer.into()), - sandbox_policy: Some(sandbox_policy.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, model: Some(model), service_tier, @@ -1029,19 +1046,43 @@ fn sandbox_mode_from_policy( } } +fn permission_profile_override_from_config( + config: &Config, + thread_params_mode: ThreadParamsMode, +) -> Option { + if matches!(thread_params_mode, ThreadParamsMode::Remote) { + return None; + } + + if matches!( + config.permissions.sandbox_policy.get(), + SandboxPolicy::ExternalSandbox { .. } + ) { + None + } else { + Some(config.permissions.permission_profile().into()) + } +} + fn thread_start_params_from_config( config: &Config, thread_params_mode: ThreadParamsMode, remote_cwd_override: Option<&std::path::Path>, session_start_source: Option, ) -> ThreadStartParams { + let permission_profile = permission_profile_override_from_config(config, thread_params_mode); + let sandbox = permission_profile + .is_none() + .then(|| sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone())) + .flatten(); ThreadStartParams { model: config.model.clone(), model_provider: thread_params_mode.model_provider_from_config(config), cwd: thread_cwd_from_config(config, thread_params_mode, remote_cwd_override), approval_policy: Some(config.permissions.approval_policy.value().into()), approvals_reviewer: approvals_reviewer_override_from_config(config), - sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone()), + sandbox, + permission_profile, config: config_request_overrides_from_config(config), ephemeral: Some(config.ephemeral), session_start_source, @@ -1056,6 +1097,11 @@ fn thread_resume_params_from_config( thread_params_mode: ThreadParamsMode, remote_cwd_override: Option<&std::path::Path>, ) -> ThreadResumeParams { + let permission_profile = permission_profile_override_from_config(&config, thread_params_mode); + let sandbox = permission_profile + .is_none() + .then(|| sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone())) + .flatten(); ThreadResumeParams { thread_id: thread_id.to_string(), model: config.model.clone(), @@ -1063,7 +1109,8 @@ fn thread_resume_params_from_config( cwd: thread_cwd_from_config(&config, thread_params_mode, remote_cwd_override), approval_policy: Some(config.permissions.approval_policy.value().into()), approvals_reviewer: approvals_reviewer_override_from_config(&config), - sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone()), + sandbox, + permission_profile, config: config_request_overrides_from_config(&config), persist_extended_history: true, ..ThreadResumeParams::default() @@ -1076,6 +1123,11 @@ fn thread_fork_params_from_config( thread_params_mode: ThreadParamsMode, remote_cwd_override: Option<&std::path::Path>, ) -> ThreadForkParams { + let permission_profile = permission_profile_override_from_config(&config, thread_params_mode); + let sandbox = permission_profile + .is_none() + .then(|| sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone())) + .flatten(); ThreadForkParams { thread_id: thread_id.to_string(), model: config.model.clone(), @@ -1083,7 +1135,8 @@ fn thread_fork_params_from_config( cwd: thread_cwd_from_config(&config, thread_params_mode, remote_cwd_override), approval_policy: Some(config.permissions.approval_policy.value().into()), approvals_reviewer: approvals_reviewer_override_from_config(&config), - sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get().clone()), + sandbox, + permission_profile, config: config_request_overrides_from_config(&config), base_instructions: config.base_instructions.clone(), developer_instructions: config.developer_instructions.clone(), @@ -1160,6 +1213,7 @@ async fn thread_session_state_from_thread_start_response( response.approval_policy.to_core(), response.approvals_reviewer.to_core(), response.sandbox.to_core(), + response.permission_profile.clone().map(Into::into), response.cwd.clone(), response.instruction_sources.clone(), response.reasoning_effort, @@ -1183,6 +1237,7 @@ async fn thread_session_state_from_thread_resume_response( response.approval_policy.to_core(), response.approvals_reviewer.to_core(), response.sandbox.to_core(), + response.permission_profile.clone().map(Into::into), response.cwd.clone(), response.instruction_sources.clone(), response.reasoning_effort, @@ -1206,6 +1261,7 @@ async fn thread_session_state_from_thread_fork_response( response.approval_policy.to_core(), response.approvals_reviewer.to_core(), response.sandbox.to_core(), + response.permission_profile.clone().map(Into::into), response.cwd.clone(), response.instruction_sources.clone(), response.reasoning_effort, @@ -1248,6 +1304,7 @@ async fn thread_session_state_from_thread_response( approval_policy: AskForApproval, approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, sandbox_policy: SandboxPolicy, + permission_profile: Option, cwd: AbsolutePathBuf, instruction_source_paths: Vec, reasoning_effort: Option, @@ -1274,6 +1331,7 @@ async fn thread_session_state_from_thread_response( approval_policy, approvals_reviewer, sandbox_policy, + permission_profile, cwd, instruction_source_paths, reasoning_effort, @@ -1366,6 +1424,11 @@ mod tests { ); assert_eq!(params.cwd, Some(config.cwd.to_string_lossy().to_string())); + assert_eq!(params.sandbox, None); + assert_eq!( + params.permission_profile, + Some(config.permissions.permission_profile().into()) + ); assert_eq!(params.model_provider, Some(config.model_provider_id)); } @@ -1389,6 +1452,8 @@ 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_policy(config.permissions.sandbox_policy.get().clone()); let start = thread_start_params_from_config( &config, @@ -1415,6 +1480,12 @@ mod tests { assert_eq!(start.model_provider, None); assert_eq!(resume.model_provider, None); assert_eq!(fork.model_provider, None); + assert_eq!(start.sandbox, expected_sandbox); + assert_eq!(resume.sandbox, expected_sandbox); + assert_eq!(fork.sandbox, expected_sandbox); + assert_eq!(start.permission_profile, None); + assert_eq!(resume.permission_profile, None); + assert_eq!(fork.permission_profile, None); } #[tokio::test] @@ -1423,6 +1494,8 @@ mod tests { 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_policy(config.permissions.sandbox_policy.get().clone()); let start = thread_start_params_from_config( &config, @@ -1449,6 +1522,12 @@ mod tests { assert_eq!(start.model_provider, None); assert_eq!(resume.model_provider, None); assert_eq!(fork.model_provider, None); + assert_eq!(start.sandbox, expected_sandbox); + assert_eq!(resume.sandbox, expected_sandbox); + assert_eq!(fork.sandbox, expected_sandbox); + assert_eq!(start.permission_profile, None); + assert_eq!(resume.permission_profile, None); + assert_eq!(fork.permission_profile, None); } #[tokio::test] @@ -1547,6 +1626,10 @@ mod tests { started.session.instruction_source_paths, response.instruction_sources ); + assert_eq!( + started.session.permission_profile, + response.permission_profile.clone().map(Into::into) + ); assert_eq!(started.turns.len(), 1); assert_eq!(started.turns[0], response.thread.turns[0]); } @@ -1575,6 +1658,10 @@ mod tests { AskForApproval::Never, codex_protocol::config_types::ApprovalsReviewer::User, SandboxPolicy::new_read_only_policy(), + Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + std::path::Path::new("/tmp/project"), + )), test_path_buf("/tmp/project").abs(), Vec::new(), /*reasoning_effort*/ None, @@ -1605,6 +1692,10 @@ mod tests { AskForApproval::Never, codex_protocol::config_types::ApprovalsReviewer::User, SandboxPolicy::new_read_only_policy(), + Some(PermissionProfile::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + std::path::Path::new("/tmp/project"), + )), test_path_buf("/tmp/project").abs(), Vec::new(), /*reasoning_effort*/ None,