From b43e0189d7c15eba3ffa7255a5ecaee4f169d098 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 14 May 2026 18:27:54 -0700 Subject: [PATCH] permissions: resolve profile identity with constraints --- codex-rs/app-server/src/lib.rs | 2 +- .../command_exec_processor.rs | 8 +- codex-rs/cli/src/debug_sandbox.rs | 4 +- codex-rs/codex-mcp/src/connection_manager.rs | 9 +- codex-rs/core/src/config/config_tests.rs | 45 ++-- codex-rs/core/src/config/mod.rs | 169 ++++++++---- .../src/config/resolved_permission_profile.rs | 246 ++++++++++++++++++ codex-rs/core/src/guardian/review_session.rs | 2 +- codex-rs/core/src/guardian/tests.rs | 8 +- codex-rs/core/src/session/mod.rs | 18 +- codex-rs/core/src/session/session.rs | 102 +++++--- codex-rs/core/src/session/tests.rs | 117 +++++---- codex-rs/core/src/session/turn_context.rs | 8 +- .../src/tools/handlers/multi_agents_tests.rs | 17 +- codex-rs/exec/src/lib.rs | 2 +- codex-rs/thread-manager-sample/src/main.rs | 2 +- codex-rs/tui/src/app/config_persistence.rs | 2 +- codex-rs/tui/src/app/event_dispatch.rs | 2 +- codex-rs/tui/src/app/startup_prompts.rs | 2 +- codex-rs/tui/src/app/tests.rs | 9 +- codex-rs/tui/src/app/thread_session_state.rs | 5 +- .../tui/src/chatwidget/permission_popups.rs | 2 +- codex-rs/tui/src/chatwidget/session_flow.rs | 13 +- .../src/chatwidget/tests/history_replay.rs | 2 +- codex-rs/tui/src/status/tests.rs | 20 +- 25 files changed, 582 insertions(+), 234 deletions(-) create mode 100644 codex-rs/core/src/config/resolved_permission_profile.rs diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index f2ac6ca00afa..673dec6341d7 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -589,7 +589,7 @@ pub async fn run_main_with_transport_options( }); } if let Some(warning) = - codex_core::config::system_bwrap_warning(config.permissions.permission_profile().get()) + codex_core::config::system_bwrap_warning(config.permissions.permission_profile()) { config_warnings.push(ConfigWarningNotification { summary: warning, diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index d1781db5ff7c..2ae11363a5fc 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -164,7 +164,7 @@ impl CommandExecRequestProcessor { let started_network_proxy = match self.config.permissions.network.as_ref() { Some(spec) => match spec .start_proxy( - self.config.permissions.permission_profile().get(), + self.config.permissions.permission_profile(), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, @@ -243,8 +243,7 @@ impl CommandExecRequestProcessor { ); self.config .permissions - .permission_profile() - .can_set(&effective_permission_profile) + .can_set_permission_profile(&effective_permission_profile) .map_err(|err| invalid_request(format!("invalid permission profile: {err}")))?; effective_permission_profile } else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) { @@ -264,8 +263,7 @@ impl CommandExecRequestProcessor { ); self.config .permissions - .permission_profile() - .can_set(&permission_profile) + .can_set_permission_profile(&permission_profile) .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; permission_profile } else { diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index bdcf0a191f0f..bc9d6172f249 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -230,7 +230,7 @@ async fn run_command_under_sandbox( let network_proxy = match config.permissions.network.as_ref() { Some(spec) => Some( spec.start_proxy( - config.permissions.permission_profile().get(), + config.permissions.permission_profile(), /*policy_decider*/ None, /*blocked_request_observer*/ None, managed_network_requirements_enabled, @@ -965,7 +965,6 @@ mod tests { let actual = config .permissions .permission_profile() - .get() .file_system_sandbox_policy(); let expected = codex_protocol::models::PermissionProfile::workspace_write() .file_system_sandbox_policy(); @@ -1008,7 +1007,6 @@ mod tests { let actual = config .permissions .permission_profile() - .get() .file_system_sandbox_policy(); let expected = codex_protocol::models::PermissionProfile::workspace_write() .file_system_sandbox_policy(); diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index c5593a559822..e8dc36b9b2dd 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -80,6 +80,13 @@ impl McpConnectionManager { pub fn new_uninitialized( approval_policy: &Constrained, permission_profile: &Constrained, + ) -> Self { + Self::new_uninitialized_with_permission_profile(approval_policy, permission_profile.get()) + } + + pub fn new_uninitialized_with_permission_profile( + approval_policy: &Constrained, + permission_profile: &PermissionProfile, ) -> Self { Self { clients: HashMap::new(), @@ -87,7 +94,7 @@ impl McpConnectionManager { host_owned_codex_apps_enabled: false, elicitation_requests: ElicitationRequestManager::new( approval_policy.value(), - permission_profile.get().clone(), + permission_profile.clone(), /*reviewer*/ None, ), startup_cancellation_token: CancellationToken::new(), diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 2e077bae35b3..7700ea230ec8 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -105,6 +105,18 @@ use std::path::Path; use std::time::Duration; use tempfile::TempDir; +fn active_permission_profile_state( + permission_profile: PermissionProfile, + profile_id: impl Into, +) -> PermissionProfileState { + PermissionProfileState::from_constrained_active_profile( + Constrained::allow_any(permission_profile), + Some(ActivePermissionProfile::new(profile_id)), + Vec::new(), + ) + .expect("active permission profile state should be valid") +} + fn stdio_mcp(command: &str) -> McpServerConfig { McpServerConfig { transport: McpServerTransportConfig::Stdio { @@ -1893,8 +1905,9 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( ) .await?; + let profile_root_abs = profile_root.abs(); let policy = config.permissions.file_system_sandbox_policy(); - for root in [cwd.abs(), runtime_root.abs(), profile_root.abs()] { + for root in [cwd.abs(), runtime_root.abs(), profile_root_abs.clone()] { assert!( policy.can_write_path_with_cwd(root.as_path(), cwd.as_path()), "expected workspace root to be writable, policy: {policy:?}" @@ -1908,6 +1921,10 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( "expected .codex carveout under {root:?}, policy: {policy:?}" ); } + assert_eq!( + config.permissions.profile_workspace_roots(), + std::slice::from_ref(&profile_root_abs) + ); assert_eq!( config.permissions.active_permission_profile(), Some(ActivePermissionProfile::new("dev")) @@ -7541,12 +7558,10 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { model_provider: fixture.openai_provider.clone(), permissions: Permissions { approval_policy: Constrained::allow_any(AskForApproval::Never), - constrained_permissions_profile: Constrained::allow_any( - PermissionProfile::read_only() - ), - active_permission_profile: Some(ActivePermissionProfile::new( + permission_profile_state: active_permission_profile_state( + PermissionProfile::read_only(), BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), + ), workspace_roots: vec![fixture.cwd()], network: None, allow_login_shell: true, @@ -7993,10 +8008,10 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { model_provider: fixture.openai_custom_provider.clone(), permissions: Permissions { approval_policy: Constrained::allow_any(AskForApproval::UnlessTrusted), - constrained_permissions_profile: Constrained::allow_any(PermissionProfile::read_only()), - active_permission_profile: Some(ActivePermissionProfile::new( + permission_profile_state: active_permission_profile_state( + PermissionProfile::read_only(), BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), + ), workspace_roots: vec![fixture.cwd()], network: None, allow_login_shell: true, @@ -8157,10 +8172,10 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { model_provider: fixture.openai_provider.clone(), permissions: Permissions { approval_policy: Constrained::allow_any(AskForApproval::OnFailure), - constrained_permissions_profile: Constrained::allow_any(PermissionProfile::read_only()), - active_permission_profile: Some(ActivePermissionProfile::new( + permission_profile_state: active_permission_profile_state( + PermissionProfile::read_only(), BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), + ), workspace_roots: vec![fixture.cwd()], network: None, allow_login_shell: true, @@ -8306,10 +8321,10 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { model_provider: fixture.openai_provider.clone(), permissions: Permissions { approval_policy: Constrained::allow_any(AskForApproval::OnFailure), - constrained_permissions_profile: Constrained::allow_any(PermissionProfile::read_only()), - active_permission_profile: Some(ActivePermissionProfile::new( + permission_profile_state: active_permission_profile_state( + PermissionProfile::read_only(), BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), + ), workspace_roots: vec![fixture.cwd()], network: None, allow_login_shell: true, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 7472e5c5766c..d9f744155d7a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -134,6 +134,7 @@ mod managed_features; mod network_proxy_spec; mod otel; mod permissions; +mod resolved_permission_profile; #[cfg(test)] mod schema; pub use codex_config::ConfigLoadOptions; @@ -148,6 +149,7 @@ pub use managed_features::ManagedFeatures; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub(crate) use permissions::resolve_permission_profile; +pub(crate) use resolved_permission_profile::PermissionProfileState; const DEFAULT_IGNORE_LARGE_UNTRACKED_DIRS: i64 = 200; const DEFAULT_IGNORE_LARGE_UNTRACKED_FILES: i64 = 10 * 1024 * 1024; @@ -247,15 +249,11 @@ pub(crate) async fn test_config() -> Config { pub struct Permissions { /// Approval policy for executing commands. pub approval_policy: Constrained, - /// Canonical constrained permissions profile before runtime workspace-root - /// materialization has been applied. - constrained_permissions_profile: Constrained, - /// Named or implicit built-in profile selected by config, rather than an - /// ad-hoc override. - active_permission_profile: Option, + /// Constrained permission profile plus its selected profile identity, if + /// the profile came from a built-in or named config profile. + permission_profile_state: PermissionProfileState, /// Thread-scoped runtime workspace roots. Symbolic `:workspace_roots` - /// entries in `constrained_permissions_profile` are materialized against - /// these roots. + /// entries in the permission profile are materialized against these roots. workspace_roots: Vec, /// Effective network configuration applied to all spawned processes. pub network: Option, @@ -283,35 +281,103 @@ impl Permissions { pub fn from_approval_and_profile( approval_policy: Constrained, permission_profile: Constrained, - ) -> Self { - Self { + ) -> ConstraintResult { + Ok(Self { approval_policy, - constrained_permissions_profile: permission_profile, - active_permission_profile: None, + permission_profile_state: PermissionProfileState::from_constrained_legacy( + permission_profile, + )?, workspace_roots: Vec::new(), network: None, allow_login_shell: true, shell_environment_policy: ShellEnvironmentPolicy::default(), windows_sandbox_mode: None, windows_sandbox_private_desktop: true, - } + }) + } + + pub(crate) fn permission_profile_state(&self) -> &PermissionProfileState { + &self.permission_profile_state + } + + pub(crate) fn set_permission_profile_state( + &mut self, + permission_profile_state: PermissionProfileState, + ) { + self.permission_profile_state = permission_profile_state; + } + + /// Apply a permission profile snapshot emitted by core session state. + /// + /// This is a trusted-state bridge for consumers of `SessionConfigured`. + /// Config loading and app-server selection should resolve named profiles + /// through config instead of constructing this pair directly. + pub fn set_permission_profile_from_session_snapshot( + &mut self, + permission_profile: PermissionProfile, + active_permission_profile: Option, + ) -> ConstraintResult<()> { + self.set_permission_profile_from_session_snapshot_with_profile_workspace_roots( + permission_profile, + active_permission_profile, + Vec::new(), + ) } - /// Borrow the constrained canonical profile. This preserves the raw - /// symbolic `:workspace_roots` form for session/thread state. - pub fn permission_profile(&self) -> &Constrained { - &self.constrained_permissions_profile + pub fn set_permission_profile_from_session_snapshot_with_profile_workspace_roots( + &mut self, + permission_profile: PermissionProfile, + active_permission_profile: Option, + profile_workspace_roots: Vec, + ) -> ConstraintResult<()> { + self.permission_profile_state.set_active_permission_profile( + permission_profile, + active_permission_profile, + profile_workspace_roots, + ) } - /// Set the full constrained profile value and preserve the active profile - /// sidecar when the caller has already validated both together. - pub fn set_constrained_permission_profile_with_active_profile( + /// Replace the current permission constraints with a trusted session + /// snapshot. This is only for clients that must mirror core session state + /// after their local config constraints reject the snapshot. + pub fn replace_permission_profile_from_session_snapshot( &mut self, permission_profile: Constrained, active_permission_profile: Option, - ) { - self.constrained_permissions_profile = permission_profile; - self.active_permission_profile = active_permission_profile; + ) -> ConstraintResult<()> { + self.replace_permission_profile_from_session_snapshot_with_profile_workspace_roots( + permission_profile, + active_permission_profile, + Vec::new(), + ) + } + + pub fn replace_permission_profile_from_session_snapshot_with_profile_workspace_roots( + &mut self, + permission_profile: Constrained, + active_permission_profile: Option, + profile_workspace_roots: Vec, + ) -> ConstraintResult<()> { + self.permission_profile_state = PermissionProfileState::from_constrained_active_profile( + permission_profile, + active_permission_profile, + profile_workspace_roots, + )?; + Ok(()) + } + + /// Borrow the canonical profile before runtime workspace-root + /// materialization has been applied. + pub fn permission_profile(&self) -> &PermissionProfile { + self.permission_profile_state.permission_profile() + } + + pub fn can_set_permission_profile( + &self, + permission_profile: &PermissionProfile, + ) -> ConstraintResult<()> { + self.permission_profile_state + .can_set_legacy_permission_profile(permission_profile) } pub fn set_workspace_roots(&mut self, workspace_roots: Vec) { @@ -328,9 +394,12 @@ impl Permissions { &self.workspace_roots } + pub fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + self.permission_profile_state.profile_workspace_roots() + } + fn materialized_permission_profile(&self) -> PermissionProfile { - self.constrained_permissions_profile - .get() + self.permission_profile() .clone() .materialize_project_roots_with_workspace_roots(&self.workspace_roots) } @@ -343,7 +412,7 @@ impl Permissions { /// Named profile selected by config, if the current profile has one. pub fn active_permission_profile(&self) -> Option { - self.active_permission_profile.clone() + self.permission_profile_state.active_permission_profile() } /// Effective filesystem sandbox policy derived from the canonical profile. @@ -354,9 +423,7 @@ impl Permissions { /// Effective network sandbox policy derived from the canonical profile. pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy { - self.constrained_permissions_profile - .get() - .network_sandbox_policy() + self.permission_profile().network_sandbox_policy() } /// Legacy compatibility projection derived from the canonical profile. @@ -386,8 +453,8 @@ impl Permissions { &file_system_sandbox_policy, network_sandbox_policy, ); - self.constrained_permissions_profile - .can_set(&permission_profile) + self.permission_profile_state + .can_set_legacy_permission_profile(&permission_profile) } /// Set permissions from a legacy sandbox policy and keep every permission @@ -427,9 +494,8 @@ impl Permissions { ], }; - self.constrained_permissions_profile - .set(permission_profile)?; - self.active_permission_profile = None; + self.permission_profile_state + .set_legacy_permission_profile(permission_profile)?; Ok(()) } @@ -438,23 +504,8 @@ impl Permissions { &mut self, permission_profile: PermissionProfile, ) -> ConstraintResult<()> { - self.set_permission_profile_with_active_profile( - permission_profile, - /*active_permission_profile*/ None, - ) - } - - /// Set permissions from the canonical profile and record the named source - /// profile, if one is known. - pub fn set_permission_profile_with_active_profile( - &mut self, - permission_profile: PermissionProfile, - active_permission_profile: Option, - ) -> ConstraintResult<()> { - self.constrained_permissions_profile - .set(permission_profile)?; - self.active_permission_profile = active_permission_profile; - Ok(()) + self.permission_profile_state + .set_legacy_permission_profile(permission_profile) } } @@ -2532,6 +2583,7 @@ impl Config { permission_profile, file_system_sandbox_policy, mut active_permission_profile, + mut profile_workspace_roots, ) = if let Some(mut permission_profile) = permission_profile { let (mut file_system_sandbox_policy, network_sandbox_policy) = permission_profile.to_runtime_permissions(); @@ -2584,6 +2636,7 @@ impl Config { permission_profile, file_system_sandbox_policy, None, + Vec::new(), ) } else if profiles_are_active { let default_permissions = default_permissions.unwrap_or_else(|| { @@ -2674,6 +2727,7 @@ impl Config { permission_profile, file_system_sandbox_policy, active_permission_profile, + configured_workspace_roots, ) } else { let configured_network_proxy_config = NetworkProxyConfig::default(); @@ -2732,6 +2786,7 @@ impl Config { permission_profile, file_system_sandbox_policy, None, + Vec::new(), ) }; if enable_network_proxy && permission_profile.network_sandbox_policy().is_enabled() { @@ -3155,6 +3210,7 @@ impl Config { // The selected profile no longer describes the effective // permissions after requirements forced a fallback. active_permission_profile = None; + profile_workspace_roots.clear(); } apply_requirement_constrained_value( "web_search_mode", @@ -3225,6 +3281,12 @@ impl Config { .value .set(effective_permission_profile) .map_err(std::io::Error::from)?; + let permission_profile_state = PermissionProfileState::from_constrained_active_profile( + constrained_permission_profile.value, + active_permission_profile, + profile_workspace_roots, + ) + .map_err(std::io::Error::from)?; let otel = otel::resolve_config(cfg.otel.unwrap_or_default(), &mut startup_warnings); let config = Self { model, @@ -3240,8 +3302,7 @@ impl Config { startup_warnings, permissions: Permissions { approval_policy: constrained_approval_policy.value, - constrained_permissions_profile: constrained_permission_profile.value, - active_permission_profile, + permission_profile_state, workspace_roots, network, allow_login_shell, @@ -3526,7 +3587,7 @@ impl Config { pub fn managed_network_requirements_enabled(&self) -> bool { !matches!( - self.permissions.permission_profile().get(), + self.permissions.permission_profile(), PermissionProfile::Disabled ) && self .config_layer_stack diff --git a/codex-rs/core/src/config/resolved_permission_profile.rs b/codex-rs/core/src/config/resolved_permission_profile.rs new file mode 100644 index 000000000000..c7cf264dae60 --- /dev/null +++ b/codex-rs/core/src/config/resolved_permission_profile.rs @@ -0,0 +1,246 @@ +use codex_config::Constrained; +use codex_config::ConstraintResult; +use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; +use codex_protocol::models::PermissionProfile; +use codex_utils_absolute_path::AbsolutePathBuf; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum BuiltInPermissionProfileId { + ReadOnly, + Workspace, + DangerFullAccess, +} + +impl BuiltInPermissionProfileId { + fn from_str(id: &str) -> Option { + match id { + BUILT_IN_PERMISSION_PROFILE_READ_ONLY => Some(Self::ReadOnly), + BUILT_IN_PERMISSION_PROFILE_WORKSPACE => Some(Self::Workspace), + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS => Some(Self::DangerFullAccess), + _ => None, + } + } + + fn as_str(self) -> &'static str { + match self { + Self::ReadOnly => BUILT_IN_PERMISSION_PROFILE_READ_ONLY, + Self::Workspace => BUILT_IN_PERMISSION_PROFILE_WORKSPACE, + Self::DangerFullAccess => BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ResolvedPermissionProfile { + Legacy(LegacyPermissionProfile), + BuiltIn(BuiltInPermissionProfile), + Named(NamedPermissionProfile), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct LegacyPermissionProfile { + permission_profile: PermissionProfile, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct BuiltInPermissionProfile { + id: BuiltInPermissionProfileId, + extends: Option, + permission_profile: PermissionProfile, + profile_workspace_roots: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct NamedPermissionProfile { + id: String, + extends: Option, + permission_profile: PermissionProfile, + profile_workspace_roots: Vec, +} + +impl ResolvedPermissionProfile { + pub(crate) fn from_active_profile( + permission_profile: PermissionProfile, + active_permission_profile: Option, + profile_workspace_roots: Vec, + ) -> Self { + let Some(active_permission_profile) = active_permission_profile else { + return Self::legacy(permission_profile); + }; + + let ActivePermissionProfile { id, extends } = active_permission_profile; + if let Some(built_in_id) = BuiltInPermissionProfileId::from_str(&id) { + Self::BuiltIn(BuiltInPermissionProfile { + id: built_in_id, + extends, + permission_profile, + profile_workspace_roots, + }) + } else { + Self::Named(NamedPermissionProfile { + id, + extends, + permission_profile, + profile_workspace_roots, + }) + } + } + + pub(crate) fn legacy(permission_profile: PermissionProfile) -> Self { + Self::Legacy(LegacyPermissionProfile { permission_profile }) + } + + pub(crate) fn permission_profile(&self) -> &PermissionProfile { + match self { + Self::Legacy(profile) => &profile.permission_profile, + Self::BuiltIn(profile) => &profile.permission_profile, + Self::Named(profile) => &profile.permission_profile, + } + } + + fn with_permission_profile(&self, permission_profile: PermissionProfile) -> Self { + match self { + Self::Legacy(_) => Self::legacy(permission_profile), + Self::BuiltIn(profile) => Self::BuiltIn(BuiltInPermissionProfile { + id: profile.id, + extends: profile.extends.clone(), + permission_profile, + profile_workspace_roots: profile.profile_workspace_roots.clone(), + }), + Self::Named(profile) => Self::Named(NamedPermissionProfile { + id: profile.id.clone(), + extends: profile.extends.clone(), + permission_profile, + profile_workspace_roots: profile.profile_workspace_roots.clone(), + }), + } + } + + pub(crate) fn active_permission_profile(&self) -> Option { + match self { + Self::Legacy(_) => None, + Self::BuiltIn(profile) => Some(ActivePermissionProfile { + id: profile.id.as_str().to_string(), + extends: profile.extends.clone(), + }), + Self::Named(profile) => Some(ActivePermissionProfile { + id: profile.id.clone(), + extends: profile.extends.clone(), + }), + } + } + + pub(crate) fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + match self { + Self::Legacy(_) => &[], + Self::BuiltIn(profile) => &profile.profile_workspace_roots, + Self::Named(profile) => &profile.profile_workspace_roots, + } + } +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct PermissionProfileState { + resolved_permission_profile: Constrained, +} + +impl PermissionProfileState { + pub(crate) fn from_constrained_legacy( + constrained_permission_profile: Constrained, + ) -> ConstraintResult { + let resolved = + ResolvedPermissionProfile::legacy(constrained_permission_profile.get().clone()); + Self::from_constrained_resolved(constrained_permission_profile, resolved) + } + + pub(crate) fn from_constrained_active_profile( + constrained_permission_profile: Constrained, + active_permission_profile: Option, + profile_workspace_roots: Vec, + ) -> ConstraintResult { + let resolved = ResolvedPermissionProfile::from_active_profile( + constrained_permission_profile.get().clone(), + active_permission_profile, + profile_workspace_roots, + ); + Self::from_constrained_resolved(constrained_permission_profile, resolved) + } + + pub(crate) fn from_constrained_resolved( + constrained_permission_profile: Constrained, + resolved_permission_profile: ResolvedPermissionProfile, + ) -> ConstraintResult { + let permission_profile_constraint = constrained_permission_profile; + let resolved_permission_profile = Constrained::new( + resolved_permission_profile, + move |candidate: &ResolvedPermissionProfile| { + permission_profile_constraint.can_set(candidate.permission_profile()) + }, + )?; + Ok(Self { + resolved_permission_profile, + }) + } + + pub(crate) fn permission_profile(&self) -> &PermissionProfile { + self.resolved_permission_profile.get().permission_profile() + } + + pub(crate) fn clone_with_permission_profile( + &self, + permission_profile: PermissionProfile, + ) -> ConstraintResult { + let candidate = self + .resolved_permission_profile + .get() + .with_permission_profile(permission_profile); + let mut state = self.clone(); + state.resolved_permission_profile.set(candidate)?; + Ok(state) + } + + pub(crate) fn active_permission_profile(&self) -> Option { + self.resolved_permission_profile + .get() + .active_permission_profile() + } + + pub(crate) fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + self.resolved_permission_profile + .get() + .profile_workspace_roots() + } + + pub(crate) fn can_set_legacy_permission_profile( + &self, + permission_profile: &PermissionProfile, + ) -> ConstraintResult<()> { + let candidate = ResolvedPermissionProfile::legacy(permission_profile.clone()); + self.resolved_permission_profile.can_set(&candidate) + } + + pub(crate) fn set_legacy_permission_profile( + &mut self, + permission_profile: PermissionProfile, + ) -> ConstraintResult<()> { + self.resolved_permission_profile + .set(ResolvedPermissionProfile::legacy(permission_profile)) + } + + pub(crate) fn set_active_permission_profile( + &mut self, + permission_profile: PermissionProfile, + active_permission_profile: Option, + profile_workspace_roots: Vec, + ) -> ConstraintResult<()> { + let candidate = ResolvedPermissionProfile::from_active_profile( + permission_profile, + active_permission_profile, + profile_workspace_roots, + ); + self.resolved_permission_profile.set(candidate) + } +} diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 25b84420dccf..2f155489197a 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -918,7 +918,7 @@ pub(crate) fn build_guardian_review_session_config( guardian_config.permissions.network = Some(NetworkProxySpec::from_config_and_constraints( live_network_config, network_constraints, - guardian_config.permissions.permission_profile().get(), + guardian_config.permissions.permission_profile(), )?); } for feature in [ diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 18400fa23055..742b3c8ae496 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -2163,7 +2163,7 @@ async fn guardian_review_session_config_preserves_parent_network_proxy() { }), ..Default::default() }), - parent_config.permissions.permission_profile().get(), + parent_config.permissions.permission_profile(), ) .expect("network proxy spec"); parent_config.permissions.network = Some(network.clone()); @@ -2191,9 +2191,7 @@ async fn guardian_review_session_config_preserves_parent_network_proxy() { ); assert_eq!( guardian_config.permissions.permission_profile(), - &Constrained::allow_only(PermissionProfile::from_legacy_sandbox_policy( - &SandboxPolicy::new_read_only_policy(), - )) + &PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_read_only_policy()) ); } @@ -2230,7 +2228,7 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() { NetworkProxySpec::from_config_and_constraints( parent_network, /*requirements*/ None, - parent_config.permissions.permission_profile().get(), + parent_config.permissions.permission_profile(), ) .expect("parent network proxy spec"), ); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index eb909b220449..1938ae2a81c2 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -173,6 +173,7 @@ use crate::compact::collect_user_messages; use crate::config::Config; use crate::config::Constrained; use crate::config::ConstraintResult; +use crate::config::PermissionProfileState; use crate::config::StartedNetworkProxy; use crate::config::resolve_web_search_mode_for_turn; use crate::context_manager::ContextManager; @@ -617,8 +618,7 @@ impl Codex { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: session_permission_profile_from_config(&config)?, - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: session_permission_profile_state_from_config(&config)?, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -818,18 +818,18 @@ fn get_service_tier( .then_some(ServiceTier::Fast.request_value().to_string()) } -fn session_permission_profile_from_config( +fn session_permission_profile_state_from_config( config: &Config, -) -> CodexResult> { - let mut session_permission_profile = config.permissions.permission_profile().clone(); - session_permission_profile - .set(config.permissions.effective_permission_profile()) +) -> CodexResult { + config + .permissions + .permission_profile_state() + .clone_with_permission_profile(config.permissions.effective_permission_profile()) .map_err(|err| { CodexErr::Fatal(format!( "failed to materialize workspace roots for session permissions: {err}" )) - })?; - Ok(session_permission_profile) + }) } fn is_enterprise_default_service_tier_plan(plan_type: AccountPlanType) -> bool { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index a15fbedc0d43..5f140ee36d0a 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -63,11 +63,10 @@ pub(crate) struct SessionConfiguration { /// When to escalate for approval for execution pub(super) approval_policy: Constrained, pub(super) approvals_reviewer: ApprovalsReviewer, - /// Canonical permission profile for the session. - pub(super) permission_profile: Constrained, - /// Named or implicit built-in permissions profile selected from config, if - /// any. - pub(super) active_permission_profile: Option, + /// Permission profile state for the session. Keep the constrained profile + /// and selected profile id in sync by using the methods below instead of + /// mutating the fields independently. + pub(super) permission_profile_state: PermissionProfileState, pub(super) windows_sandbox_level: WindowsSandboxLevel, /// Absolute working directory that should be treated as the *root* of the @@ -103,12 +102,32 @@ impl SessionConfiguration { &self.codex_home } + pub(super) fn permission_profile_state(&self) -> &PermissionProfileState { + &self.permission_profile_state + } + pub(super) fn permission_profile(&self) -> PermissionProfile { - self.permission_profile.get().clone() + self.permission_profile_state.permission_profile().clone() } pub(super) fn active_permission_profile(&self) -> Option { - self.active_permission_profile.clone() + self.permission_profile_state.active_permission_profile() + } + + pub(super) fn apply_permission_profile_to_permissions( + &self, + permissions: &mut crate::config::Permissions, + ) { + permissions.set_permission_profile_state(self.permission_profile_state.clone()); + } + + #[cfg(test)] + pub(super) fn set_permission_profile_for_tests( + &mut self, + permission_profile: PermissionProfile, + ) -> ConstraintResult<()> { + self.permission_profile_state + .set_legacy_permission_profile(permission_profile) } pub(super) fn sandbox_policy(&self) -> SandboxPolicy { @@ -117,7 +136,7 @@ impl SessionConfiguration { .unwrap_or_else(|_| { let file_system_sandbox_policy = self.file_system_sandbox_policy(); codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( - self.permission_profile.get(), + self.permission_profile_state.permission_profile(), &file_system_sandbox_policy, self.network_sandbox_policy(), &self.cwd, @@ -126,11 +145,13 @@ impl SessionConfiguration { } pub(super) fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy { - self.permission_profile.get().file_system_sandbox_policy() + self.permission_profile().file_system_sandbox_policy() } pub(super) fn network_sandbox_policy(&self) -> NetworkSandboxPolicy { - self.permission_profile.get().network_sandbox_policy() + self.permission_profile_state + .permission_profile() + .network_sandbox_policy() } pub(super) fn thread_config_snapshot(&self) -> ThreadConfigSnapshot { @@ -227,16 +248,16 @@ impl SessionConfiguration { let active_permission_profile = updates.active_permission_profile.clone().or_else(|| { if permission_profile == self.permission_profile() { - self.active_permission_profile.clone() + self.active_permission_profile() } else { None } }); next_configuration.set_permission_profile_projection( permission_profile, + active_permission_profile, Some(¤t_file_system_sandbox_policy), )?; - next_configuration.active_permission_profile = active_permission_profile; } else if let Some(sandbox_policy) = updates.sandbox_policy.clone() { let file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_preserving_deny_entries( @@ -245,14 +266,15 @@ impl SessionConfiguration { ¤t_file_system_sandbox_policy, ); let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - next_configuration.permission_profile.set( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ), - )?; - next_configuration.active_permission_profile = None; + 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 @@ -266,13 +288,15 @@ impl SessionConfiguration { &next_configuration.cwd, ¤t_file_system_sandbox_policy, ); - next_configuration.permission_profile.set( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(¤t_sandbox_policy), - &file_system_sandbox_policy, - current_network_sandbox_policy, - ), - )?; + next_configuration + .permission_profile_state + .set_legacy_permission_profile( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(¤t_sandbox_policy), + &file_system_sandbox_policy, + current_network_sandbox_policy, + ), + )?; } if let Some(app_server_client_name) = updates.app_server_client_name.clone() { next_configuration.app_server_client_name = Some(app_server_client_name); @@ -286,6 +310,7 @@ impl SessionConfiguration { fn set_permission_profile_projection( &mut self, permission_profile: PermissionProfile, + active_permission_profile: Option, preserve_deny_reads_from: Option<&FileSystemSandboxPolicy>, ) -> ConstraintResult<()> { let enforcement = permission_profile.enforcement(); @@ -301,8 +326,11 @@ impl SessionConfiguration { &file_system_sandbox_policy, network_sandbox_policy, ); - self.permission_profile.set(effective_permission_profile)?; - Ok(()) + self.permission_profile_state.set_active_permission_profile( + effective_permission_profile, + active_permission_profile, + Vec::new(), + ) } } @@ -756,7 +784,7 @@ impl Session { let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy( spec, current_exec_policy.as_ref(), - config.permissions.permission_profile().get(), + config.permissions.permission_profile(), network_policy_decider.as_ref().map(Arc::clone), blocked_request_observer.as_ref().map(Arc::clone), managed_network_requirements_configured, @@ -818,10 +846,12 @@ impl Session { // before any MCP-related events. It is reasonable to consider // changing this to use Option or OnceCell, though the current // setup is straightforward enough and performs well. - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized( - &config.permissions.approval_policy, - config.permissions.permission_profile(), - ))), + mcp_connection_manager: Arc::new(RwLock::new( + McpConnectionManager::new_uninitialized_with_permission_profile( + &config.permissions.approval_policy, + config.permissions.permission_profile(), + ), + )), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, @@ -926,7 +956,9 @@ impl Session { initial_messages, network_proxy: session_network_proxy.filter(|_| { Self::managed_network_proxy_active_for_permission_profile( - session_configuration.permission_profile.get(), + session_configuration + .permission_profile_state() + .permission_profile(), ) }), rollout_path, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 50632ce81a21..81012419cb5d 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -869,8 +869,7 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow state.session_configuration.original_config_do_not_use = Arc::new(config); state .session_configuration - .permission_profile - .set(PermissionProfile::from_legacy_sandbox_policy( + .set_permission_profile_for_tests(PermissionProfile::from_legacy_sandbox_policy( &initial_policy, )) .expect("test setup should allow permission profile"); @@ -2168,9 +2167,9 @@ async fn session_permission_profile_materializes_runtime_workspace_roots() -> an }) .build() .await?; - let session_permission_profile = session_permission_profile_from_config(&config)?; - let file_system_policy = session_permission_profile - .get() + let session_permission_profile_state = session_permission_profile_state_from_config(&config)?; + let file_system_policy = session_permission_profile_state + .permission_profile() .file_system_sandbox_policy(); assert!( @@ -2915,8 +2914,7 @@ async fn set_rate_limits_retains_previous_credits() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -3019,8 +3017,7 @@ async fn set_rate_limits_updates_plan_type_when_present() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -3492,8 +3489,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -3556,13 +3552,15 @@ async fn session_configuration_apply_preserves_profile_file_system_policy_on_cwd }, ]); let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - session_configuration.permission_profile = codex_config::Constrained::allow_any( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ), - ); + session_configuration + .set_permission_profile_for_tests( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), + &file_system_sandbox_policy, + network_sandbox_policy, + ), + ) + .expect("set permission profile"); let updated = session_configuration .apply(&SessionSettingsUpdate { @@ -3597,13 +3595,15 @@ async fn session_configuration_apply_permission_profile_preserves_existing_deny_ ); existing_file_system_policy.glob_scan_max_depth = Some(2); existing_file_system_policy.entries.push(deny_entry.clone()); - session_configuration.permission_profile = codex_config::Constrained::allow_any( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&workspace_policy), - &existing_file_system_policy, - NetworkSandboxPolicy::Restricted, - ), - ); + session_configuration + .set_permission_profile_for_tests( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(&workspace_policy), + &existing_file_system_policy, + NetworkSandboxPolicy::Restricted, + ), + ) + .expect("set permission profile"); let requested_file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( &workspace_policy, @@ -3778,13 +3778,15 @@ async fn session_configuration_apply_rederives_legacy_file_system_policy_on_cwd_ &sandbox_policy, &session_configuration.cwd, ); - session_configuration.permission_profile = codex_config::Constrained::allow_any( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - NetworkSandboxPolicy::from(&sandbox_policy), - ), - ); + session_configuration + .set_permission_profile_for_tests( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), + &file_system_sandbox_policy, + NetworkSandboxPolicy::from(&sandbox_policy), + ), + ) + .expect("set permission profile"); let updated = session_configuration .apply(&SessionSettingsUpdate { @@ -3830,13 +3832,15 @@ async fn session_configuration_apply_preserves_absolute_cwd_write_root_on_cwd_up access: FileSystemAccessMode::Write, }, ]); - session_configuration.permission_profile = codex_config::Constrained::allow_any( - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::Managed, - &file_system_sandbox_policy, - NetworkSandboxPolicy::Restricted, - ), - ); + session_configuration + .set_permission_profile_for_tests( + PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::Managed, + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ), + ) + .expect("set permission profile"); let updated = session_configuration .apply(&SessionSettingsUpdate { @@ -4025,8 +4029,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -4134,8 +4137,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -4179,10 +4181,12 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { ); let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized( - &config.permissions.approval_policy, - config.permissions.permission_profile(), - ))), + mcp_connection_manager: Arc::new(RwLock::new( + McpConnectionManager::new_uninitialized_with_permission_profile( + &config.permissions.approval_policy, + config.permissions.permission_profile(), + ), + )), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, @@ -4366,8 +4370,7 @@ async fn make_session_with_config_and_rx( compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -4469,8 +4472,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -5986,8 +5988,7 @@ where compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), approvals_reviewer: config.approvals_reviewer, - permission_profile: config.permissions.permission_profile().clone(), - active_permission_profile: config.permissions.active_permission_profile(), + permission_profile_state: config.permissions.permission_profile_state().clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -6031,10 +6032,12 @@ where ); let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized( - &config.permissions.approval_policy, - config.permissions.permission_profile(), - ))), + mcp_connection_manager: Arc::new(RwLock::new( + McpConnectionManager::new_uninitialized_with_permission_profile( + &config.permissions.approval_policy, + config.permissions.permission_profile(), + ), + )), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 45dc06c7760a..eefa6bc9da3f 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -438,12 +438,8 @@ impl Session { per_turn_config.service_tier = session_configuration.service_tier.clone(); per_turn_config.personality = session_configuration.personality; per_turn_config.approvals_reviewer = session_configuration.approvals_reviewer; - per_turn_config - .permissions - .set_constrained_permission_profile_with_active_profile( - session_configuration.permission_profile.clone(), - session_configuration.active_permission_profile.clone(), - ); + session_configuration + .apply_permission_profile_to_permissions(&mut per_turn_config.permissions); let permission_profile = session_configuration.permission_profile(); let resolved_web_search_mode = resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &permission_profile); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 4fc6db14e456..c6a0007f1368 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -3900,7 +3900,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr #[tokio::test] async fn build_agent_spawn_config_uses_turn_context_values() { fn pick_allowed_sandbox_policy( - constraint: &crate::config::Constrained, + permissions: &crate::config::Permissions, base: SandboxPolicy, cwd: &std::path::Path, ) -> SandboxPolicy { @@ -3915,16 +3915,9 @@ async fn build_agent_spawn_config_uses_turn_context_values() { if *candidate == base { return false; } - let file_system_sandbox_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(candidate, cwd); - let network_sandbox_policy = NetworkSandboxPolicy::from(candidate); - let permission_profile = - PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(candidate), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - constraint.can_set(&permission_profile).is_ok() + permissions + .can_set_legacy_sandbox_policy(candidate, cwd) + .is_ok() }) .unwrap_or(base) } @@ -3948,7 +3941,7 @@ async fn build_agent_spawn_config_uses_turn_context_values() { #[allow(deprecated)] let turn_cwd = turn.cwd.clone(); let sandbox_policy = pick_allowed_sandbox_policy( - turn.config.permissions.permission_profile(), + &turn.config.permissions, turn.config.legacy_sandbox_policy(), turn_cwd.as_path(), ); diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index b4724edd297a..a71ca5fa3fe3 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -760,7 +760,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { event_processor.print_config_summary(&config, &prompt_summary, &session_configured); if !json_mode && let Some(message) = - codex_core::config::system_bwrap_warning(config.permissions.permission_profile().get()) + codex_core::config::system_bwrap_warning(config.permissions.permission_profile()) { event_processor.process_warning(message); } diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index b8b7b0a519e2..313971afdce8 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -174,7 +174,7 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R permissions: Permissions::from_approval_and_profile( Constrained::allow_any(AskForApproval::Never), Constrained::allow_any(PermissionProfile::read_only()), - ), + )?, approvals_reviewer: ApprovalsReviewer::User, enforce_residency: Constrained::allow_any(/*initial_value*/ None), hide_agent_reasoning: false, diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 807663189ddd..d77e3b31408e 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -301,7 +301,7 @@ impl App { } let permission_profile_override_value = permission_profile_override .is_some() - .then(|| self.config.permissions.permission_profile().get().clone()); + .then(|| self.config.permissions.permission_profile().clone()); if let Some(permission_profile) = permission_profile_override_value.as_ref() && let Err(err) = self .chat_widget diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index fb351991b613..811c24cc4d93 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1426,7 +1426,7 @@ impl App { return Ok(AppRunControl::Continue); } self.runtime_permission_profile_override = - Some(self.config.permissions.permission_profile().get().clone()); + Some(self.config.permissions.permission_profile().clone()); self.sync_active_thread_permission_settings_to_cached_session() .await; diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index e2b347775631..802cda3f80c6 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -67,7 +67,7 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) { let Some(message) = - codex_sandboxing::system_bwrap_warning(config.permissions.permission_profile().get()) + codex_sandboxing::system_bwrap_warning(config.permissions.permission_profile()) else { return; }; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index abdde1c9957c..25269deadf5f 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1639,8 +1639,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result< app.chat_widget .config_ref() .permissions - .permission_profile() - .get(), + .permission_profile(), &auto_review.permission_profile ); assert_eq!( @@ -1817,8 +1816,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review app.chat_widget .config_ref() .permissions - .permission_profile() - .get(), + .permission_profile(), &auto_review.permission_profile ); assert_eq!( @@ -3004,7 +3002,6 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() { .config_ref() .permissions .permission_profile() - .get() .clone(); assert_eq!( session.permission_profile, expected_permission_profile, @@ -3140,7 +3137,7 @@ async fn side_fork_config_inherits_parent_thread_runtime_settings() { fork_config.model_reasoning_effort, fork_config.service_tier.as_deref(), fork_config.permissions.approval_policy.value(), - fork_config.permissions.permission_profile().get(), + fork_config.permissions.permission_profile(), fork_config.approvals_reviewer, ), ( diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index d057baa40c6e..6afbc2de38f9 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -20,7 +20,6 @@ impl App { .config_ref() .permissions .permission_profile() - .get() .clone(); let active_permission_profile = self .chat_widget @@ -103,7 +102,6 @@ impl App { .config_ref() .permissions .permission_profile() - .get() .clone() } @@ -355,12 +353,11 @@ mod tests { .config_ref() .permissions .permission_profile() - .get() .clone(); assert_eq!(session.permission_profile, expected_permission_profile); assert_ne!( session.permission_profile, - app.config.permissions.permission_profile().get().clone(), + app.config.permissions.permission_profile().clone(), "thread/read fallback must use the active widget permissions rather than stale app \ config defaults" ); diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index cf341cb8298f..6f2e1c4dbd65 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -17,7 +17,7 @@ impl ChatWidget { let include_read_only = cfg!(target_os = "windows"); let current_approval = AskForApproval::from(self.config.permissions.approval_policy.value()); - let current_permission_profile = self.config.permissions.permission_profile().get().clone(); + let current_permission_profile = self.config.permissions.permission_profile().clone(); let guardian_approval_enabled = self.config.features.enabled(Feature::GuardianApproval); let current_review_policy = self.config.approvals_reviewer; let mut items: Vec = Vec::new(); diff --git a/codex-rs/tui/src/chatwidget/session_flow.rs b/codex-rs/tui/src/chatwidget/session_flow.rs index 58cbe9bb73d8..932263bcc16c 100644 --- a/codex-rs/tui/src/chatwidget/session_flow.rs +++ b/codex-rs/tui/src/chatwidget/session_flow.rs @@ -66,18 +66,25 @@ impl ChatWidget { let permission_sync = self .config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( session.permission_profile.clone(), session.active_permission_profile.clone(), ); if let Err(err) = permission_sync { tracing::warn!(%err, "failed to sync permissions from SessionConfigured"); - self.config + if let Err(replace_err) = self + .config .permissions - .set_constrained_permission_profile_with_active_profile( + .replace_permission_profile_from_session_snapshot( Constrained::allow_only(session.permission_profile.clone()), session.active_permission_profile.clone(), + ) + { + tracing::error!( + %replace_err, + "failed to replace permissions from SessionConfigured after constraint fallback" ); + } } self.config.approvals_reviewer = session.approvals_reviewer; self.status_line_project_root_name_cache = None; diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index e6c8fcad88bf..6e3ead97bbd4 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -293,7 +293,7 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { chat.set_permission_profile(updated_profile.clone()) .expect("set permission profile"); assert_eq!( - chat.config_ref().permissions.permission_profile().get(), + chat.config_ref().permissions.permission_profile(), &updated_profile, "local permission changes should replace SessionConfigured canonical permissions" ); diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index e69f2da3c8ca..65a390a00701 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -306,7 +306,7 @@ async fn status_permissions_named_read_only_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::read_only(), Some(ActivePermissionProfile::new( BUILT_IN_PERMISSION_PROFILE_READ_ONLY, @@ -335,7 +335,7 @@ async fn status_permissions_read_only_profile_shows_additional_writable_roots() .with_additional_writable_roots(config.cwd.as_path(), std::slice::from_ref(&extra_root)); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::from_runtime_permissions( &file_system_policy, NetworkSandboxPolicy::Restricted, @@ -363,7 +363,7 @@ async fn status_permissions_named_workspace_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write(), Some(ActivePermissionProfile::new( BUILT_IN_PERMISSION_PROFILE_WORKSPACE, @@ -389,7 +389,7 @@ async fn status_permissions_workspace_auto_review_shows_reviewer_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write(), Some(ActivePermissionProfile::new( BUILT_IN_PERMISSION_PROFILE_WORKSPACE, @@ -415,7 +415,7 @@ async fn status_permissions_named_profile_shows_additional_writable_roots() { let extra_root = test_path_buf("/workspace/extra").abs(); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write_with( std::slice::from_ref(&extra_root), NetworkSandboxPolicy::Restricted, @@ -451,7 +451,7 @@ async fn status_permissions_workspace_roots_show_additional_directories() { .set_workspace_roots(config.workspace_roots.clone()); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write(), Some(ActivePermissionProfile::new(":workspace")), ) @@ -474,7 +474,7 @@ async fn status_permissions_broadened_workspace_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write_with( &[], NetworkSandboxPolicy::Enabled, @@ -499,7 +499,7 @@ async fn status_permissions_user_defined_profile_shows_name() { let mut config = test_config(&temp_home).await; config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::read_only(), Some(ActivePermissionProfile::new("locked")), ) @@ -519,7 +519,7 @@ async fn status_snapshot_shows_active_user_defined_profile() { set_workspace_cwd(&mut config, test_path_buf("/workspace/tests").abs()); config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::read_only(), Some(ActivePermissionProfile::new("locked")), ) @@ -617,7 +617,7 @@ async fn status_snapshot_shows_auto_review_permissions() { config.approvals_reviewer = ApprovalsReviewer::AutoReview; config .permissions - .set_permission_profile_with_active_profile( + .set_permission_profile_from_session_snapshot( PermissionProfile::workspace_write(), Some(ActivePermissionProfile::new( BUILT_IN_PERMISSION_PROFILE_WORKSPACE,