From f948f077600aef3ea8b8ec7e190153be224ddffe Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 4 Jun 2026 21:59:25 -0700 Subject: [PATCH] core: remove SandboxPolicy from session settings updates --- .../src/request_processors/turn_processor.rs | 35 ++++++++++++--- codex-rs/core/src/codex_thread.rs | 3 -- codex-rs/core/src/session/handlers.rs | 20 ++++++++- codex-rs/core/src/session/session.rs | 43 ++++++------------- codex-rs/core/src/session/tests.rs | 4 +- 5 files changed, 63 insertions(+), 42 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 25beee3cdb..eef7f4121c 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -524,7 +524,10 @@ impl TurnRequestProcessor { // `thread/settings/update` only acknowledges that the update was queued. // Clients that send dependent partial updates should wait for // `thread/settings/updated` or combine the fields in one request. - let snapshot = if permissions.is_some() || runtime_workspace_roots_request.is_some() { + let snapshot = if sandbox_policy.is_some() + || permissions.is_some() + || runtime_workspace_roots_request.is_some() + { Some(thread.config_snapshot().await) } else { None @@ -564,7 +567,7 @@ impl TurnRequestProcessor { let approvals_reviewer = approvals_reviewer.map(codex_app_server_protocol::ApprovalsReviewer::to_core); let sandbox_policy = sandbox_policy.map(|policy| policy.to_core()); - let (permission_profile, active_permission_profile, profile_workspace_roots) = + let (permission_profile, active_permission_profile, profile_workspace_roots) = { if let Some(permissions) = permissions { let Some(snapshot) = snapshot.as_ref() else { return Err(internal_error(format!( @@ -611,9 +614,32 @@ impl TurnRequestProcessor { config.permissions.active_permission_profile(), Some(config.permissions.profile_workspace_roots().to_vec()), ) + } else if let Some(sandbox_policy) = sandbox_policy.as_ref() { + let Some(snapshot) = snapshot.as_ref() else { + return Err(internal_error(format!( + "{method} sandbox policy missing thread snapshot" + ))); + }; + let sandbox_cwd = cwd + .as_ref() + .map(|cwd| { + AbsolutePathBuf::resolve_path_against_base(cwd, snapshot.cwd.as_path()) + }) + .unwrap_or_else(|| snapshot.cwd.clone()); + ( + Some( + codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy_for_cwd( + sandbox_policy, + sandbox_cwd.as_path(), + ), + ), + None, + None, + ) } else { (None, None, None) - }; + } + }; let effort = effort.map(Some); if has_any_overrides { @@ -623,7 +649,6 @@ impl TurnRequestProcessor { workspace_roots: runtime_workspace_roots.clone(), approval_policy, approvals_reviewer, - sandbox_policy: sandbox_policy.clone(), permission_profile: permission_profile.clone(), active_permission_profile: active_permission_profile.clone(), profile_workspace_roots: profile_workspace_roots.clone(), @@ -647,7 +672,7 @@ impl TurnRequestProcessor { profile_workspace_roots, approval_policy, approvals_reviewer, - sandbox_policy, + sandbox_policy: None, permission_profile, active_permission_profile, windows_sandbox_level: None, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index ac28a58ec3..aeea06985b 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -131,7 +131,6 @@ pub struct CodexThreadSettingsOverrides { pub profile_workspace_roots: Option>, pub approval_policy: Option, pub approvals_reviewer: Option, - pub sandbox_policy: Option, pub permission_profile: Option, pub active_permission_profile: Option, pub windows_sandbox_level: Option, @@ -370,7 +369,6 @@ impl CodexThread { profile_workspace_roots, approval_policy, approvals_reviewer, - sandbox_policy, permission_profile, active_permission_profile, windows_sandbox_level, @@ -397,7 +395,6 @@ impl CodexThread { profile_workspace_roots, approval_policy, approvals_reviewer, - sandbox_policy, permission_profile, active_permission_profile, windows_sandbox_level, diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 7730a30ad7..57e2b8c2b5 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -26,6 +26,7 @@ use crate::tasks::UserShellCommandMode; use crate::tasks::UserShellCommandTask; use crate::tasks::execute_user_shell_command; use codex_protocol::models::ContentItem; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::CodexErrorInfo; @@ -138,6 +139,24 @@ async fn thread_settings_update( collaboration_mode, personality, } = thread_settings; + let (permission_profile, active_permission_profile) = + if let Some(permission_profile) = permission_profile { + (Some(permission_profile), active_permission_profile) + } else if let Some(sandbox_policy) = sandbox_policy { + let state = sess.state.lock().await; + let update_cwd = state + .session_configuration + .resolved_cwd_for_update(cwd.as_ref()); + ( + Some(PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &sandbox_policy, + update_cwd.as_path(), + )), + None, + ) + } else { + (None, active_permission_profile) + }; let collaboration_mode = match collaboration_mode { Some(collaboration_mode) => collaboration_mode, None => { @@ -156,7 +175,6 @@ async fn thread_settings_update( profile_workspace_roots, approval_policy, approvals_reviewer, - sandbox_policy, permission_profile, active_permission_profile, windows_sandbox_level, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 07d249613a..15c56182fe 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -248,19 +248,7 @@ impl SessionConfiguration { next_configuration.windows_sandbox_level = windows_sandbox_level; } - let absolute_cwd = updates - .cwd - .as_ref() - .map(|cwd| { - AbsolutePathBuf::relative_to_current_dir(normalize_for_native_workdir( - cwd.as_path(), - )) - .unwrap_or_else(|e| { - warn!("failed to normalize update cwd: {cwd:?}: {e}"); - self.cwd.clone() - }) - }) - .unwrap_or_else(|| self.cwd.clone()); + let absolute_cwd = self.resolved_cwd_for_update(updates.cwd.as_ref()); let cwd_changed = absolute_cwd.as_path() != self.cwd.as_path(); next_configuration.cwd = absolute_cwd; @@ -324,23 +312,6 @@ impl SessionConfiguration { )?; next_configuration.original_config_do_not_use = Arc::new(config); } - } else if let Some(sandbox_policy) = updates.sandbox_policy.clone() { - let file_system_sandbox_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( - &sandbox_policy, - &next_configuration.cwd, - ¤t_file_system_sandbox_policy, - ); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - next_configuration - .permission_profile_state - .set_legacy_permission_profile( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ), - )?; } else if cwd_changed && file_system_policy_matches_legacy && file_system_policy_has_rebindable_project_root_write @@ -373,6 +344,17 @@ impl SessionConfiguration { Ok(next_configuration) } + pub(crate) fn resolved_cwd_for_update(&self, cwd: Option<&PathBuf>) -> AbsolutePathBuf { + cwd.map(|cwd| { + AbsolutePathBuf::relative_to_current_dir(normalize_for_native_workdir(cwd.as_path())) + .unwrap_or_else(|e| { + warn!("failed to normalize update cwd: {cwd:?}: {e}"); + self.cwd.clone() + }) + }) + .unwrap_or_else(|| self.cwd.clone()) + } + fn set_permission_profile_projection( &mut self, permission_profile: PermissionProfile, @@ -417,7 +399,6 @@ pub(crate) struct SessionSettingsUpdate { pub(crate) profile_workspace_roots: Option>, pub(crate) approval_policy: Option, pub(crate) approvals_reviewer: Option, - pub(crate) sandbox_policy: Option, pub(crate) permission_profile: Option, pub(crate) active_permission_profile: Option, pub(crate) windows_sandbox_level: Option, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 1643574206..8fac1bf17d 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -917,9 +917,9 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow session .new_turn_with_sub_id( - "sandbox-policy-change".to_string(), + "permission-profile-change".to_string(), SessionSettingsUpdate { - sandbox_policy: Some(SandboxPolicy::DangerFullAccess), + permission_profile: Some(PermissionProfile::Disabled), ..Default::default() }, )