From 9c0eced39196671646862e9c918d09535ff7bff6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 23 Apr 2026 12:46:19 -0700 Subject: [PATCH] shell-escalation: carry resolved permission profiles (#18287) ## Why Shell escalation still has adapter code that expects a legacy sandbox policy, but command approvals should carry the resolved `PermissionProfile` so callers can reason about the granted permissions canonically. ## What changed This introduces profile-shaped resolved escalation permissions while retaining the derived legacy sandbox policy for the Unix escalation adapter. It updates approval types, the escalation server protocol, and tests that inspect escalated command permissions. ## Verification - `cargo test -p codex-core --test all handle_container_exec_ -- --nocapture` - `cargo test -p codex-core --test all handle_sandbox_ -- --nocapture` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18287). * #18288 * __->__ #18287 --- .../tools/runtimes/shell/unix_escalation.rs | 38 +++++++++++-------- .../runtimes/shell/unix_escalation_tests.rs | 12 +++--- codex-rs/protocol/src/approvals.rs | 17 +++++---- codex-rs/shell-escalation/src/lib.rs | 4 +- .../src/unix/escalate_server.rs | 16 ++++---- codex-rs/shell-escalation/src/unix/mod.rs | 2 +- 6 files changed, 52 insertions(+), 37 deletions(-) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 83075c367..b29e9c593 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -53,8 +53,8 @@ use codex_shell_escalation::EscalationPolicy; use codex_shell_escalation::EscalationSession; use codex_shell_escalation::ExecParams; use codex_shell_escalation::ExecResult; -use codex_shell_escalation::Permissions as EscalatedPermissions; use codex_shell_escalation::PreparedExec; +use codex_shell_escalation::ResolvedPermissionProfile; use codex_shell_escalation::ShellCommandExecutor; use codex_shell_escalation::Stopwatch; use codex_utils_absolute_path::AbsolutePathBuf; @@ -370,13 +370,17 @@ impl CoreShellActionProvider { .map(|_| { // Shell request additional permissions were already normalized and // merged into the first-attempt sandbox policy. - EscalationExecution::Permissions(EscalationPermissions::Permissions( - EscalatedPermissions { - sandbox_policy: sandbox_policy.clone(), - file_system_sandbox_policy: file_system_sandbox_policy.clone(), - network_sandbox_policy, - }, - )) + EscalationExecution::Permissions( + EscalationPermissions::ResolvedPermissionProfile( + ResolvedPermissionProfile { + permission_profile: PermissionProfile::from_runtime_permissions( + file_system_sandbox_policy, + network_sandbox_policy, + ), + sandbox_policy: sandbox_policy.clone(), + }, + ), + ) }) .unwrap_or(EscalationExecution::TurnDefault), } @@ -842,9 +846,9 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { additional_permissions: None, })? } - EscalationExecution::Permissions(EscalationPermissions::PermissionProfile( - permission_profile, - )) => { + EscalationExecution::Permissions( + EscalationPermissions::AdditionalPermissionProfile(permission_profile), + ) => { // Merge additive permissions into the existing turn/request sandbox policy. self.prepare_sandboxed_exec(PrepareSandboxedExecParams { command, @@ -856,15 +860,19 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { additional_permissions: Some(permission_profile), })? } - EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => { - // Use a fully specified sandbox policy instead of merging into the turn policy. + EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( + permissions, + )) => { + // Use a fully specified permission profile instead of merging into the turn policy. + let (file_system_sandbox_policy, network_sandbox_policy) = + permissions.permission_profile.to_runtime_permissions(); self.prepare_sandboxed_exec(PrepareSandboxedExecParams { command, workdir, env, sandbox_policy: &permissions.sandbox_policy, - file_system_sandbox_policy: &permissions.file_system_sandbox_policy, - network_sandbox_policy: permissions.network_sandbox_policy, + file_system_sandbox_policy: &file_system_sandbox_policy, + network_sandbox_policy, additional_permissions: None, })? } diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index deaaf067e..a2b6ca145 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -33,7 +33,7 @@ use codex_sandboxing::SandboxType; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::ExecResult; -use codex_shell_escalation::Permissions as EscalatedPermissions; +use codex_shell_escalation::ResolvedPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::Value; @@ -317,11 +317,13 @@ fn shell_request_escalation_execution_is_explicit() { network_sandbox_policy, Some(&requested_permissions), ), - EscalationExecution::Permissions(EscalationPermissions::Permissions( - EscalatedPermissions { + EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( + ResolvedPermissionProfile { + permission_profile: PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + network_sandbox_policy, + ), sandbox_policy, - file_system_sandbox_policy, - network_sandbox_policy, }, )), ); diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 8799db4a6..2b9aea272 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -1,8 +1,6 @@ use crate::mcp::RequestId; use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; -use crate::permissions::FileSystemSandboxPolicy; -use crate::permissions::NetworkSandboxPolicy; use crate::protocol::FileChange; use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; @@ -16,18 +14,23 @@ use std::collections::HashMap; use std::path::PathBuf; use ts_rs::TS; +/// Fully resolved permissions for rerunning an intercepted child process. +/// +/// `permission_profile` is the canonical permission model. `sandbox_policy` +/// remains as the legacy adapter for sandbox backends that still require it. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Permissions { +pub struct ResolvedPermissionProfile { + pub permission_profile: PermissionProfile, pub sandbox_policy: SandboxPolicy, - pub file_system_sandbox_policy: FileSystemSandboxPolicy, - pub network_sandbox_policy: NetworkSandboxPolicy, } #[allow(clippy::large_enum_variant)] #[derive(Debug, Clone, PartialEq, Eq)] pub enum EscalationPermissions { - PermissionProfile(PermissionProfile), - Permissions(Permissions), + /// Permissions to merge with the active turn permissions. + AdditionalPermissionProfile(PermissionProfile), + /// Fully resolved permissions that should replace the active turn permissions. + ResolvedPermissionProfile(ResolvedPermissionProfile), } /// Proposed execpolicy change to allow commands starting with this prefix. diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs index 6c7e1ec60..0f3c0afd3 100644 --- a/codex-rs/shell-escalation/src/lib.rs +++ b/codex-rs/shell-escalation/src/lib.rs @@ -22,10 +22,10 @@ pub use unix::ExecParams; #[cfg(unix)] pub use unix::ExecResult; #[cfg(unix)] -pub use unix::Permissions; -#[cfg(unix)] pub use unix::PreparedExec; #[cfg(unix)] +pub use unix::ResolvedPermissionProfile; +#[cfg(unix)] pub use unix::ShellCommandExecutor; #[cfg(unix)] pub use unix::Stopwatch; diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 42e31f6a2..ce73f262f 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -922,7 +922,7 @@ mod tests { server, Arc::new(DeterministicEscalationPolicy { decision: EscalationDecision::escalate(EscalationExecution::Permissions( - EscalationPermissions::PermissionProfile(PermissionProfile { + EscalationPermissions::AdditionalPermissionProfile(PermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), }), @@ -931,12 +931,14 @@ mod tests { )), }), Arc::new(PermissionAssertingShellCommandExecutor { - expected_permissions: EscalationPermissions::PermissionProfile(PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - ..Default::default() - }), + expected_permissions: EscalationPermissions::AdditionalPermissionProfile( + PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + ..Default::default() + }, + ), }), CancellationToken::new(), CancellationToken::new(), diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs index b5d999411..8fb868844 100644 --- a/codex-rs/shell-escalation/src/unix/mod.rs +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -76,4 +76,4 @@ pub use self::escalation_policy::EscalationPolicy; pub use self::execve_wrapper::main_execve_wrapper; pub use self::stopwatch::Stopwatch; pub use codex_protocol::approvals::EscalationPermissions; -pub use codex_protocol::approvals::Permissions; +pub use codex_protocol::approvals::ResolvedPermissionProfile;