mirror of
https://github.com/openai/codex.git
synced 2026-11-07 16:35:32 +00:00
Delegate remote process sandboxing to the executor (#37480)
## What changed - Preserve executor-native working directories, workspace roots, and permission profiles when preparing remote `exec_command` requests instead of resolving them through the host platform. - Send sandbox intent to the remote executor, including a restricted-token default for Windows executors and `-NoProfile` for elevated PowerShell commands. - Reject intercepted cross-platform `apply_patch` commands when filesystem writes are restricted, while continuing to allow them for unsandboxed or full-disk-write profiles. ## Testing Extended remote unified-exec coverage for foreign Windows workspaces, elevated PowerShell, sandbox metadata, and restricted and unrestricted intercepted patches. GitOrigin-RevId: 9fd328879fa2c15594fd58e40b6b32e8ba0847e7
This commit is contained in:
1 parent
92fb33b758
commit
511262b984
8 files changed
+284
-44
No files matched your search
@@ -152,23 +152,22 @@ impl ExecCommandHandler {
|
||||
let environment = Arc::clone(&turn_environment.environment);
|
||||
let fs = environment.get_filesystem();
|
||||
|
||||
// A foreign cwd cannot seed the AbsolutePathBufGuard used to resolve relative paths in the
|
||||
// permissions config below. Consult the configured platform-sandbox requirement before
|
||||
// deciding whether parsing may continue without that base path.
|
||||
let sandbox = SandboxManager::new().select_initial(
|
||||
turn_environment.permission_profile(),
|
||||
SandboxablePreference::Auto,
|
||||
turn.windows_sandbox_level,
|
||||
turn.network.is_some(),
|
||||
);
|
||||
// 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.
|
||||
let requires_host_native_cwd = !environment.is_remote()
|
||||
&& SandboxManager::new().select_initial(
|
||||
turn_environment.permission_profile(),
|
||||
SandboxablePreference::Auto,
|
||||
turn.windows_sandbox_level,
|
||||
turn.network.is_some(),
|
||||
) != SandboxType::None;
|
||||
// `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is
|
||||
// representable as `/C:/repo` on POSIX. Require the inferred convention to match too.
|
||||
let cwd_uses_native_convention =
|
||||
cwd.infer_path_convention() == Some(PathConvention::native());
|
||||
// TODO(anp): Remove this parsing split once sandboxing supports foreign paths.
|
||||
let native_cwd = match cwd.to_abs_path() {
|
||||
Ok(cwd) if cwd_uses_native_convention => Some(cwd),
|
||||
_ if sandbox == SandboxType::None => None,
|
||||
_ if !requires_host_native_cwd => None,
|
||||
Err(err) => return Err(FunctionCallError::RespondToModel(err.to_string())),
|
||||
Ok(_) => {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
@@ -183,9 +182,8 @@ impl ExecCommandHandler {
|
||||
parse_arguments_with_base_path(&arguments, native_cwd)?
|
||||
}
|
||||
None => {
|
||||
// Parsing without a base only skips relative-path resolution inside the
|
||||
// permissions config. That is safe only for a truly unsandboxed attempt;
|
||||
// sandboxed attempts fall through and return the conversion error below.
|
||||
// Foreign executor cwd values cannot seed this host's AbsolutePathBufGuard.
|
||||
// Sandbox intent and URI-native roots are still sent to the executor.
|
||||
parse_arguments(&arguments)?
|
||||
}
|
||||
};
|
||||
@@ -239,6 +237,23 @@ impl ExecCommandHandler {
|
||||
)
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
let command = resolved_command.command;
|
||||
if environment.is_remote()
|
||||
&& !cwd_uses_native_convention
|
||||
&& !turn_environment
|
||||
.permission_profile()
|
||||
.file_system_sandbox_policy()
|
||||
.has_full_disk_write_access()
|
||||
&& matches!(
|
||||
codex_apply_patch::maybe_parse_apply_patch(&command, &cwd),
|
||||
codex_apply_patch::MaybeApplyPatch::Body(_)
|
||||
)
|
||||
{
|
||||
// CA-781: patch verification reads executor files before process sandboxing applies.
|
||||
manager.release_process_id(process_id).await;
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"cross-platform remote apply_patch is unavailable until executor-side filesystem sandboxing is supported".to_string(),
|
||||
));
|
||||
}
|
||||
let shell_type = resolved_command.shell_type;
|
||||
let command_for_display = codex_shell_command::parse_command::shlex_join(&command);
|
||||
|
||||
|
||||
@@ -151,8 +151,14 @@ impl ToolOrchestrator {
|
||||
|
||||
let environment = tool.turn_environment(req);
|
||||
let workspace_roots = environment.workspace_roots();
|
||||
let executor_managed_process_sandbox = tool.uses_executor_managed_process_sandbox(req);
|
||||
let permission_profile = environment.permission_profile();
|
||||
let permissions = environment.permission_profile_with_workspace_roots();
|
||||
let permissions = if executor_managed_process_sandbox {
|
||||
// Executor-native roots remain symbolic until the executor applies its own sandbox.
|
||||
permission_profile.clone()
|
||||
} else {
|
||||
environment.permission_profile_with_workspace_roots()
|
||||
};
|
||||
let file_system_sandbox_policy = permissions.file_system_sandbox_policy();
|
||||
let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| {
|
||||
default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy)
|
||||
@@ -230,7 +236,7 @@ impl ToolOrchestrator {
|
||||
managed_network_active,
|
||||
),
|
||||
};
|
||||
let initial_sandbox = if sandbox_requested {
|
||||
let initial_sandbox = if sandbox_requested && !executor_managed_process_sandbox {
|
||||
self.sandbox.select_initial(
|
||||
&permissions,
|
||||
sandbox_preference,
|
||||
@@ -415,7 +421,8 @@ impl ToolOrchestrator {
|
||||
sandbox_preference,
|
||||
managed_network_active,
|
||||
);
|
||||
let retry_sandbox = if retry_sandbox_requested {
|
||||
let retry_sandbox = if retry_sandbox_requested && !executor_managed_process_sandbox
|
||||
{
|
||||
self.sandbox.select_initial(
|
||||
&permissions,
|
||||
sandbox_preference,
|
||||
|
||||
@@ -24,7 +24,6 @@ pub(crate) use codex_network_proxy::strip_managed_proxy_env;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashMap;
|
||||
@@ -179,11 +178,11 @@ pub(crate) fn apply_zsh_fork_path_prepend(
|
||||
pub(crate) fn disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
command: &[String],
|
||||
shell_type: Option<&ShellType>,
|
||||
sandbox: SandboxType,
|
||||
sandbox_requested: bool,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
) -> Vec<String> {
|
||||
if shell_type != Some(&ShellType::PowerShell)
|
||||
|| sandbox != SandboxType::WindowsRestrictedToken
|
||||
|| !sandbox_requested
|
||||
|| windows_sandbox_level != WindowsSandboxLevel::Elevated
|
||||
|| command.is_empty()
|
||||
{
|
||||
@@ -428,7 +427,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::PowerShell),
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
/*sandbox_requested*/ true,
|
||||
WindowsSandboxLevel::Elevated,
|
||||
);
|
||||
|
||||
@@ -454,7 +453,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::PowerShell),
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
/*sandbox_requested*/ true,
|
||||
WindowsSandboxLevel::Elevated,
|
||||
);
|
||||
|
||||
@@ -481,7 +480,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::PowerShell),
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
/*sandbox_requested*/ true,
|
||||
WindowsSandboxLevel::Elevated,
|
||||
);
|
||||
|
||||
@@ -499,7 +498,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::PowerShell),
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
/*sandbox_requested*/ true,
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
);
|
||||
|
||||
@@ -517,7 +516,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::PowerShell),
|
||||
SandboxType::None,
|
||||
/*sandbox_requested*/ false,
|
||||
WindowsSandboxLevel::Elevated,
|
||||
);
|
||||
|
||||
@@ -535,7 +534,7 @@ mod disable_powershell_profile_tests {
|
||||
let rewritten = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&ShellType::Bash),
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
/*sandbox_requested*/ true,
|
||||
WindowsSandboxLevel::Elevated,
|
||||
);
|
||||
|
||||
|
||||
@@ -237,7 +237,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
let command = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
req.shell_type.as_ref(),
|
||||
attempt.sandbox,
|
||||
attempt.sandbox_requested,
|
||||
attempt.windows_sandbox_level,
|
||||
);
|
||||
let command = if matches!(shell.shell_type, ShellType::PowerShell) {
|
||||
|
||||
@@ -192,6 +192,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
&req.turn_environment
|
||||
}
|
||||
|
||||
fn uses_executor_managed_process_sandbox(&self, req: &UnifiedExecRequest) -> bool {
|
||||
req.turn_environment.environment.is_remote()
|
||||
}
|
||||
|
||||
fn sandbox_cwd<'b>(&self, req: &'b UnifiedExecRequest) -> Option<&'b PathUri> {
|
||||
Some(&req.sandbox_cwd)
|
||||
}
|
||||
@@ -363,7 +367,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
let command = disable_powershell_profile_for_elevated_windows_sandbox(
|
||||
&command,
|
||||
Some(&req.shell_type),
|
||||
attempt.sandbox,
|
||||
attempt.sandbox_requested,
|
||||
attempt.windows_sandbox_level,
|
||||
);
|
||||
let command = if matches!(req.shell_type, ShellType::PowerShell) {
|
||||
|
||||
@@ -15,6 +15,7 @@ use crate::tools::network_approval::NetworkApprovalSpec;
|
||||
use codex_file_system::FileSystemSandboxContext;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
@@ -27,6 +28,7 @@ use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
use codex_tools::ToolName;
|
||||
use codex_utils_path_uri::PathConvention;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::Future;
|
||||
use serde::Serialize;
|
||||
@@ -360,6 +362,10 @@ pub(crate) enum ToolError {
|
||||
pub(crate) trait ToolRuntime<Req, Out>: Approvable<Req> + Sandboxable {
|
||||
fn turn_environment<'a>(&self, req: &'a Req) -> &'a TurnEnvironment;
|
||||
|
||||
fn uses_executor_managed_process_sandbox(&self, _req: &Req) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx) -> Option<NetworkApprovalSpec> {
|
||||
None
|
||||
}
|
||||
@@ -472,11 +478,24 @@ impl<'a> SandboxAttempt<'a> {
|
||||
crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, Vec::new());
|
||||
exec_request.exec_server_managed_network = managed_network;
|
||||
if self.sandbox_requested {
|
||||
// This level comes from the orchestrator's config, so `Disabled` means Windows
|
||||
// sandboxing is irrelevant on a non-Windows host. A Windows executor would instead
|
||||
// treat it as unable to enforce the requested sandbox. Select its baseline restricted
|
||||
// token backend while preserving explicitly configured levels and same-OS behavior.
|
||||
let windows_sandbox_level = if self.windows_sandbox_level
|
||||
== WindowsSandboxLevel::Disabled
|
||||
&& self.sandbox_cwd.infer_path_convention() == Some(PathConvention::Windows)
|
||||
&& PathConvention::native() != PathConvention::Windows
|
||||
{
|
||||
WindowsSandboxLevel::RestrictedToken
|
||||
} else {
|
||||
self.windows_sandbox_level
|
||||
};
|
||||
exec_request.exec_server_sandbox = Some(FileSystemSandboxContext {
|
||||
permissions: exec_server_permissions.into(),
|
||||
cwd: Some(exec_request.windows_sandbox_policy_cwd.clone()),
|
||||
workspace_roots: self.workspace_roots.to_vec(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self.windows_sandbox_private_desktop,
|
||||
windows_sandbox_proxy_settings_mode: None,
|
||||
use_legacy_landlock: self.use_legacy_landlock,
|
||||
|
||||
Reference in new issue
Block a user