mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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
This commit is contained in:
committed by
GitHub
Unverified
parent
6d09b6752d
commit
9c0eced391
@@ -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,
|
||||
})?
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
)),
|
||||
);
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user