mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
permissions: derive legacy exec policies at boundaries (#19737)
## Why After config and requirements store canonical profiles, exec requests should not cache a derived `SandboxPolicy`. The cached legacy value can drift from the richer profile state, and most execution paths already have the filesystem and network runtime policies they need. ## What Changed - Removes `sandbox_policy` from `codex_sandboxing::SandboxExecRequest` and `codex_core::sandboxing::ExecRequest`. - Adds an on-demand `ExecRequest::compatibility_sandbox_policy()` helper for the Windows and legacy call sites that still need a `SandboxPolicy` projection. - Updates Windows filesystem override setup and unified exec policy serialization to derive that compatibility policy at the boundary. - Updates Unix escalation reruns and direct shell requests to reconstruct exec requests from `PermissionProfile` plus runtime filesystem/network policy, without carrying a cached legacy policy. - Adjusts sandboxing manager tests to assert the effective profile rather than the removed legacy field. ## Verification - `cargo check -p codex-config -p codex-core -p codex-sandboxing -p codex-app-server -p codex-cli -p codex-tui` - `cargo test -p codex-sandboxing manager` - `cargo test -p codex-core exec_server_params_use_env_policy_overlay_contract` - `cargo test -p codex-core unix_escalation` - `cargo test -p codex-core exec::tests` - `cargo test -p codex-core sandboxing::tests`
This commit is contained in:
committed by
GitHub
Unverified
parent
523e4aa8e3
commit
a6ca39c630
@@ -321,10 +321,11 @@ pub fn build_exec_request(
|
||||
exec_req.windows_sandbox_level,
|
||||
exec_req.network.is_some(),
|
||||
);
|
||||
let sandbox_policy = exec_req.compatibility_sandbox_policy();
|
||||
exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend {
|
||||
resolve_windows_elevated_filesystem_overrides(
|
||||
exec_req.sandbox,
|
||||
&exec_req.sandbox_policy,
|
||||
&sandbox_policy,
|
||||
&exec_req.file_system_sandbox_policy,
|
||||
exec_req.network_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
@@ -333,7 +334,7 @@ pub fn build_exec_request(
|
||||
} else {
|
||||
resolve_windows_restricted_token_filesystem_overrides(
|
||||
exec_req.sandbox,
|
||||
&exec_req.sandbox_policy,
|
||||
&sandbox_policy,
|
||||
&exec_req.file_system_sandbox_policy,
|
||||
exec_req.network_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
@@ -349,6 +350,7 @@ pub(crate) async fn execute_exec_request(
|
||||
stdout_stream: Option<StdoutStream>,
|
||||
after_spawn: Option<Box<dyn FnOnce() + Send>>,
|
||||
) -> Result<ExecToolCallOutput> {
|
||||
let sandbox_policy = exec_request.compatibility_sandbox_policy();
|
||||
let ExecRequest {
|
||||
command,
|
||||
cwd,
|
||||
@@ -362,8 +364,6 @@ pub(crate) async fn execute_exec_request(
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
permission_profile: _,
|
||||
sandbox_policy,
|
||||
// TODO(mbolin): Use file_system_sandbox_policy instead of sandbox_policy.
|
||||
file_system_sandbox_policy: _,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides,
|
||||
|
||||
@@ -55,7 +55,6 @@ pub struct ExecRequest {
|
||||
pub windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
pub permission_profile: PermissionProfile,
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
pub file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
pub network_sandbox_policy: NetworkSandboxPolicy,
|
||||
pub(crate) windows_sandbox_filesystem_overrides: Option<WindowsSandboxFilesystemOverrides>,
|
||||
@@ -80,12 +79,6 @@ impl ExecRequest {
|
||||
let windows_sandbox_policy_cwd = cwd.clone();
|
||||
let (file_system_sandbox_policy, network_sandbox_policy) =
|
||||
permission_profile.to_runtime_permissions();
|
||||
let sandbox_policy = compatibility_sandbox_policy_for_permission_profile(
|
||||
&permission_profile,
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
cwd.as_path(),
|
||||
);
|
||||
Self {
|
||||
command,
|
||||
cwd,
|
||||
@@ -99,7 +92,6 @@ impl ExecRequest {
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
@@ -107,6 +99,15 @@ impl ExecRequest {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn compatibility_sandbox_policy(&self) -> SandboxPolicy {
|
||||
compatibility_sandbox_policy_for_permission_profile(
|
||||
&self.permission_profile,
|
||||
&self.file_system_sandbox_policy,
|
||||
self.network_sandbox_policy,
|
||||
self.windows_sandbox_policy_cwd.as_path(),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn from_sandbox_exec_request(
|
||||
request: SandboxExecRequest,
|
||||
options: ExecOptions,
|
||||
@@ -121,7 +122,6 @@ impl ExecRequest {
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
arg0,
|
||||
@@ -153,7 +153,6 @@ impl ExecRequest {
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
|
||||
@@ -29,7 +29,6 @@ use codex_protocol::protocol::ExecCommandBeginEvent;
|
||||
use codex_protocol::protocol::ExecCommandEndEvent;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_protocol::protocol::ExecCommandStatus;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_shell_command::parse_command::parse_command;
|
||||
@@ -195,7 +194,6 @@ pub(crate) async fn execute_user_shell_command(
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
permission_profile: permission_profile.clone(),
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
file_system_sandbox_policy: permission_profile.file_system_sandbox_policy(),
|
||||
network_sandbox_policy: permission_profile.network_sandbox_policy(),
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
|
||||
@@ -40,7 +40,6 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::GuardianCommandSource;
|
||||
use codex_protocol::protocol::NetworkPolicyRuleAction;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
@@ -143,7 +142,6 @@ pub(super) async fn try_run_zsh_fork(
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: _windows_sandbox_private_desktop,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: _windows_sandbox_filesystem_overrides,
|
||||
@@ -161,7 +159,6 @@ pub(super) async fn try_run_zsh_fork(
|
||||
command,
|
||||
cwd: sandbox_cwd,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
sandbox,
|
||||
@@ -260,7 +257,6 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
command: exec_request.command.clone(),
|
||||
cwd: exec_request.cwd.clone(),
|
||||
permission_profile: exec_request.permission_profile.clone(),
|
||||
sandbox_policy: exec_request.sandbox_policy.clone(),
|
||||
file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(),
|
||||
network_sandbox_policy: exec_request.network_sandbox_policy,
|
||||
sandbox: exec_request.sandbox,
|
||||
@@ -742,7 +738,6 @@ struct CoreShellCommandExecutor {
|
||||
command: Vec<String>,
|
||||
cwd: AbsolutePathBuf,
|
||||
permission_profile: PermissionProfile,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
network_sandbox_policy: NetworkSandboxPolicy,
|
||||
sandbox: SandboxType,
|
||||
@@ -796,7 +791,6 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: false,
|
||||
permission_profile: self.permission_profile.clone(),
|
||||
sandbox_policy: self.sandbox_policy.clone(),
|
||||
file_system_sandbox_policy: self.file_system_sandbox_policy.clone(),
|
||||
network_sandbox_policy: self.network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
|
||||
@@ -664,7 +664,8 @@ impl UnifiedExecProcessManager {
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
if request.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken {
|
||||
let policy_json = serde_json::to_string(&request.sandbox_policy).map_err(|err| {
|
||||
let sandbox_policy = request.compatibility_sandbox_policy();
|
||||
let policy_json = serde_json::to_string(&sandbox_policy).map_err(|err| {
|
||||
UnifiedExecError::create_process(format!(
|
||||
"failed to serialize Windows sandbox policy: {err}"
|
||||
))
|
||||
|
||||
@@ -110,7 +110,6 @@ fn exec_server_params_use_env_policy_overlay_contract() {
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
permission_profile,
|
||||
sandbox_policy,
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
|
||||
@@ -80,7 +80,6 @@ pub struct SandboxExecRequest {
|
||||
pub windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
pub permission_profile: PermissionProfile,
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
pub file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
pub network_sandbox_policy: NetworkSandboxPolicy,
|
||||
pub arg0: Option<String>,
|
||||
@@ -262,7 +261,6 @@ impl SandboxManager {
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
permission_profile: effective_permission_profile,
|
||||
sandbox_policy: effective_policy,
|
||||
file_system_sandbox_policy: effective_file_system_policy,
|
||||
network_sandbox_policy: effective_network_policy,
|
||||
arg0: arg0_override,
|
||||
|
||||
@@ -15,8 +15,6 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use dunce::canonicalize;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -152,9 +150,9 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() {
|
||||
.expect("transform");
|
||||
|
||||
assert_eq!(
|
||||
exec_request.sandbox_policy,
|
||||
SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Enabled,
|
||||
exec_request.permission_profile,
|
||||
PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Enabled,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user