mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Use turn environment settings for sandbox execution (#40771)
## Why Tool execution can use an environment whose sandbox configuration differs from the turn-wide configuration. Sandbox selection and process setup need to follow the environment that owns the tool request. ## What changed - Source Windows sandbox level, legacy Landlock mode, and Windows private desktop settings from the selected turn environment across tool orchestration, unified exec, patch application, and zsh fork escalation. - Preserve Windows sandbox level overrides in restorable thread settings and invalidate MCP configuration when that level changes. ## Testing - Add a resume test that verifies an elevated Windows sandbox override survives thread restoration. GitOrigin-RevId: 9d8fc46404919f5ae97c9ed712c0932a1f095e36
This commit is contained in:
@@ -2,10 +2,10 @@ use crate::function_tool::FunctionCallError;
|
||||
use crate::safety::SafetyCheck;
|
||||
use crate::safety::assess_patch_safety;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::FileSystemSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -21,19 +21,17 @@ pub(crate) struct ApplyPatchRuntimeInvocation {
|
||||
|
||||
pub(crate) fn prepare_apply_patch(
|
||||
turn_context: &TurnContext,
|
||||
permission_profile: &PermissionProfile,
|
||||
turn_environment: &TurnEnvironment,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
action: ApplyPatchAction,
|
||||
) -> Result<ApplyPatchRuntimeInvocation, FunctionCallError> {
|
||||
// TODO(anp): Reconcile patch approval with TurnEnvironment::sandbox_context
|
||||
// so the Windows backend matches the environment that will apply the patch.
|
||||
match assess_patch_safety(
|
||||
&action,
|
||||
turn_context.approval_policy(),
|
||||
permission_profile,
|
||||
turn_environment.permission_profile(),
|
||||
file_system_sandbox_policy,
|
||||
&action.cwd,
|
||||
turn_context.windows_sandbox_level,
|
||||
turn_environment.config().windows_sandbox_level,
|
||||
) {
|
||||
SafetyCheck::AutoApprove => Ok(ApplyPatchRuntimeInvocation {
|
||||
action,
|
||||
|
||||
@@ -37,14 +37,12 @@ impl Session {
|
||||
next: &SessionConfiguration,
|
||||
updates: &SessionSettingsUpdate,
|
||||
) -> bool {
|
||||
// TODO(anp): Reconcile invalidation with TurnEnvironment::sandbox_context.
|
||||
// This gap predates that API: an internal Windows-level-only settings update
|
||||
// can leave the published MCP configuration stale.
|
||||
current.cwd() != next.cwd()
|
||||
|| current.step_settings.approval_policy.value()
|
||||
!= next.step_settings.approval_policy.value()
|
||||
|| current.step_settings.approvals_reviewer != next.step_settings.approvals_reviewer
|
||||
|| current.permission_profile() != next.permission_profile()
|
||||
|| current.windows_sandbox_level != next.windows_sandbox_level
|
||||
|| updates.environments.as_ref().is_some_and(|environments| {
|
||||
environments.environments != self.services.turn_environments.selections()
|
||||
})
|
||||
|
||||
@@ -293,13 +293,11 @@ impl SessionConfiguration {
|
||||
approvals_reviewer: Some(self.step_settings.approvals_reviewer),
|
||||
permission_profile: Some(self.permission_profile()),
|
||||
active_permission_profile: self.active_permission_profile(),
|
||||
windows_sandbox_level: Some(self.windows_sandbox_level),
|
||||
summary: self.step_settings.reasoning_summary,
|
||||
service_tier: Some(self.step_settings.service_tier.clone()),
|
||||
collaboration_mode: Some(self.step_settings.collaboration_mode.clone()),
|
||||
personality: self.step_settings.personality,
|
||||
// TODO(anp): Include the runtime Windows sandbox override in restored settings.
|
||||
// This omission predates TurnEnvironment::sandbox_context; rebuilding FromThread
|
||||
// environments falls back to Config instead of retaining that override.
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -565,7 +565,7 @@ async fn execute_verified_patch(
|
||||
.unwrap_or_else(|_| patch_permissions_without_path_matching(&action));
|
||||
let apply = apply_patch::prepare_apply_patch(
|
||||
tool_ctx.step_context.turn.as_ref(),
|
||||
turn_environment.permission_profile(),
|
||||
&turn_environment,
|
||||
&file_system_sandbox_policy,
|
||||
action,
|
||||
)?;
|
||||
|
||||
@@ -161,13 +161,11 @@ impl ExecCommandHandler {
|
||||
|
||||
// Remote executors enforce URI-native sandbox policy themselves. Only a host-local
|
||||
// sandbox needs a native cwd for resolving paths nested in the permissions config.
|
||||
// TODO(anp): Reconcile this backend choice with TurnEnvironment::sandbox_context
|
||||
// so the native-cwd requirement follows the selected environment's sandbox.
|
||||
let requires_host_native_cwd = !environment.is_remote()
|
||||
&& SandboxManager::new().select_initial(
|
||||
turn_environment.permission_profile(),
|
||||
SandboxablePreference::Auto,
|
||||
turn.windows_sandbox_level,
|
||||
turn_environment.config().windows_sandbox_level,
|
||||
turn.network.is_some(),
|
||||
) != SandboxType::None;
|
||||
// `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is
|
||||
|
||||
@@ -145,7 +145,8 @@ impl ToolOrchestrator {
|
||||
let mut already_approved = false;
|
||||
|
||||
let environment = tool.turn_environment(req);
|
||||
let owner_network_policy = environment.config().network_policy.is_some();
|
||||
let sandbox_config = environment.config();
|
||||
let owner_network_policy = sandbox_config.network_policy.is_some();
|
||||
if owner_network_policy
|
||||
&& tool
|
||||
.sandbox_permissions(req)
|
||||
@@ -269,21 +270,17 @@ impl ToolOrchestrator {
|
||||
managed_network_active,
|
||||
),
|
||||
};
|
||||
// TODO(anp): Reconcile backend selection and both attempts with
|
||||
// TurnEnvironment::sandbox_context; turn-wide inputs can differ from owner config.
|
||||
let initial_sandbox = if sandbox_requested && !executor_managed_process_sandbox {
|
||||
self.sandbox.select_initial(
|
||||
&permissions,
|
||||
sandbox_preference,
|
||||
turn_ctx.windows_sandbox_level,
|
||||
sandbox_config.windows_sandbox_level,
|
||||
managed_network_active,
|
||||
)
|
||||
} else {
|
||||
SandboxType::None
|
||||
};
|
||||
|
||||
// Platform-specific flag gating is handled by SandboxManager::select_initial.
|
||||
let use_legacy_landlock = turn_ctx.config.features.use_legacy_landlock();
|
||||
let sandbox_policy_cwd = tool
|
||||
.sandbox_cwd(req)
|
||||
.cloned()
|
||||
@@ -298,12 +295,9 @@ impl ToolOrchestrator {
|
||||
sandbox_cwd: &sandbox_policy_cwd,
|
||||
workspace_roots,
|
||||
codex_linux_sandbox_exe: turn_ctx.config.codex_linux_sandbox_exe.as_ref(),
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level: turn_ctx.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_ctx
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
use_legacy_landlock: sandbox_config.use_legacy_landlock,
|
||||
windows_sandbox_level: sandbox_config.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: sandbox_config.windows_sandbox_private_desktop,
|
||||
network_denial_cancellation_token: None,
|
||||
network_proxy: None,
|
||||
};
|
||||
@@ -454,7 +448,7 @@ impl ToolOrchestrator {
|
||||
self.sandbox.select_initial(
|
||||
&permissions,
|
||||
sandbox_preference,
|
||||
turn_ctx.windows_sandbox_level,
|
||||
sandbox_config.windows_sandbox_level,
|
||||
managed_network_active,
|
||||
)
|
||||
} else {
|
||||
@@ -475,12 +469,9 @@ impl ToolOrchestrator {
|
||||
sandbox_cwd: &sandbox_policy_cwd,
|
||||
workspace_roots,
|
||||
codex_linux_sandbox_exe: retry_codex_linux_sandbox_exe,
|
||||
use_legacy_landlock,
|
||||
windows_sandbox_level: turn_ctx.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_ctx
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
use_legacy_landlock: sandbox_config.use_legacy_landlock,
|
||||
windows_sandbox_level: sandbox_config.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: sandbox_config.windows_sandbox_private_desktop,
|
||||
network_denial_cancellation_token: None,
|
||||
network_proxy: None,
|
||||
};
|
||||
|
||||
@@ -140,7 +140,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
sandbox_policy_cwd,
|
||||
windows_sandbox_workspace_roots: exec_request.windows_sandbox_workspace_roots.clone(),
|
||||
codex_linux_sandbox_exe: ctx.step_context.turn.config.codex_linux_sandbox_exe.clone(),
|
||||
use_legacy_landlock: ctx.step_context.turn.config.features.use_legacy_landlock(),
|
||||
use_legacy_landlock: req.turn_environment.config().use_legacy_landlock,
|
||||
};
|
||||
let escalation_policy = CoreShellActionProvider {
|
||||
policy: Arc::clone(&exec_policy),
|
||||
|
||||
@@ -1269,7 +1269,7 @@ impl UnifiedExecProcessManager {
|
||||
approval_policy: turn.approval_policy(),
|
||||
permission_profile: request.turn_environment.permission_profile().clone(),
|
||||
environment_policy: request.turn_environment.config().exec_policy.as_ref(),
|
||||
windows_sandbox_level: turn.windows_sandbox_level,
|
||||
windows_sandbox_level: request.turn_environment.config().windows_sandbox_level,
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use anyhow::Result;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::ByteRange;
|
||||
@@ -19,6 +20,38 @@ use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_restores_windows_sandbox_override() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex();
|
||||
let initial = builder.build(&server).await?;
|
||||
core_test_support::submit_thread_settings(
|
||||
&initial.codex,
|
||||
ThreadSettingsOverrides {
|
||||
windows_sandbox_level: Some(WindowsSandboxLevel::Elevated),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
initial.codex.ensure_rollout_materialized().await;
|
||||
let settings = initial.codex.restorable_thread_settings().await;
|
||||
|
||||
let resumed = builder.restart(&server, &initial).await?;
|
||||
resumed.codex.restore_thread_settings(settings).await?;
|
||||
|
||||
assert_eq!(
|
||||
resumed
|
||||
.codex
|
||||
.restorable_thread_settings()
|
||||
.await
|
||||
.windows_sandbox_level,
|
||||
Some(WindowsSandboxLevel::Elevated)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user