mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Defer exec-server sandbox preparation to the execution host (#60663)
## Why Remote commands can use workspace paths from a different operating system. Preparing them on the client should preserve those paths and command arguments for the execution host. ## What changed - Build exec-server requests directly, carrying the native command and sandbox context without applying the client's sandbox transform. - Resolve workspace permissions and runtime read access in `SandboxManager` on the execution host, including helper binaries and validated managed-network CA bundles. - Add explicit backend selection and required platform sandbox selection, rejecting launches when the required sandbox is unavailable. ## Testing Extend request tests to cover foreign workspace paths, Unicode arguments, and empty arguments. Extend the remote process test on Unix to verify that restricted reads allow the sandbox helper and validated CA bundle while denying an untrusted certificate file. GitOrigin-RevId: 5fc02ac4208912ecefb80a212742389afc14ac05
This commit is contained in:
@@ -37,6 +37,7 @@ use codex_protocol::protocol::ExecCommandOutputDeltaEvent;
|
||||
use codex_protocol::protocol::ExecOutputStream;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxSelection;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
@@ -409,7 +410,8 @@ pub async fn build_exec_request(
|
||||
command,
|
||||
permissions: permission_profile,
|
||||
workspace_roots: None,
|
||||
sandbox: sandbox_type,
|
||||
runtime_readable_paths: &[],
|
||||
selection: SandboxSelection::Explicit(sandbox_type),
|
||||
enforce_managed_network,
|
||||
environment_id: network_environment_id.as_deref(),
|
||||
network: network.as_ref(),
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxSelection;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
@@ -165,7 +166,8 @@ impl ShellSnapshotSandbox {
|
||||
},
|
||||
permissions: &self.permissions,
|
||||
workspace_roots: None,
|
||||
sandbox: self.sandbox,
|
||||
runtime_readable_paths: &[],
|
||||
selection: SandboxSelection::Explicit(self.sandbox),
|
||||
enforce_managed_network: self.enforce_managed_network,
|
||||
environment_id: Some(&self.environment_id),
|
||||
network: self.network.as_ref(),
|
||||
|
||||
@@ -44,7 +44,6 @@ use codex_core_plugins::PluginMetricsSidecar;
|
||||
use codex_features::Feature;
|
||||
use codex_file_system::WindowsSandboxSelection;
|
||||
use codex_network_proxy::CREDENTIAL_BROKER_ACTIVE_ENV_KEY;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_network_proxy::RemoteCredentialMasking;
|
||||
use codex_network_proxy::RemoteNetworkProxyConfig;
|
||||
@@ -122,26 +121,6 @@ fn unified_exec_options(
|
||||
}
|
||||
}
|
||||
|
||||
fn build_unified_exec_sandbox_command(
|
||||
command: &[String],
|
||||
cwd: &PathUri,
|
||||
env: &HashMap<String, String>,
|
||||
managed_network: Option<ManagedNetworkSandboxContext>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
) -> Result<SandboxCommand, ToolError> {
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
.ok_or_else(|| ToolError::Rejected("command args are empty".to_string()))?;
|
||||
Ok(SandboxCommand {
|
||||
program: program.clone().into(),
|
||||
args: args.to_vec(),
|
||||
cwd: cwd.clone(),
|
||||
env: env.clone(),
|
||||
managed_network,
|
||||
additional_permissions,
|
||||
})
|
||||
}
|
||||
|
||||
impl<'a> UnifiedExecRuntime<'a> {
|
||||
/// Creates a runtime bound to the shared unified-exec process manager.
|
||||
pub fn new(manager: &'a UnifiedExecProcessManager) -> Self {
|
||||
@@ -596,10 +575,11 @@ impl<'a> ToolRuntime<UnifiedExecRequest<'_>, UnifiedExecAttempt> for UnifiedExec
|
||||
prepared.permissions.additional_permissions.as_ref(),
|
||||
internal_permissions.as_ref(),
|
||||
);
|
||||
let executor_managed = self.uses_executor_managed_process_sandbox(req);
|
||||
let permissions = TerminalPermissions::for_launch(
|
||||
&prepared.turn_environment,
|
||||
&ctx.step_context.turn,
|
||||
if self.uses_executor_managed_process_sandbox(req) {
|
||||
if executor_managed {
|
||||
TerminalSandboxSource::Executor
|
||||
} else {
|
||||
TerminalSandboxSource::Native
|
||||
@@ -624,31 +604,50 @@ impl<'a> ToolRuntime<UnifiedExecRequest<'_>, UnifiedExecAttempt> for UnifiedExec
|
||||
permissions
|
||||
};
|
||||
|
||||
let command = build_unified_exec_sandbox_command(
|
||||
&command,
|
||||
&prepared.cwd,
|
||||
&env,
|
||||
managed_network_context,
|
||||
additional_permissions,
|
||||
)
|
||||
.map_err(|error| match error {
|
||||
ToolError::Rejected(_) => {
|
||||
ToolError::Rejected("missing command line for PTY".to_string())
|
||||
}
|
||||
error @ ToolError::Codex(_) => error,
|
||||
})?;
|
||||
let Some((program, args)) = command.split_first() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"missing command line for PTY".to_string(),
|
||||
));
|
||||
};
|
||||
let options = unified_exec_options(attempt.network_denial_cancellation_token.clone());
|
||||
let request = if executor_managed {
|
||||
attempt
|
||||
.env_for_exec_server(
|
||||
command,
|
||||
prepared.cwd.clone(),
|
||||
env,
|
||||
managed_network_context,
|
||||
additional_permissions.as_ref(),
|
||||
options,
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
let command = SandboxCommand {
|
||||
program: program.as_str().into(),
|
||||
args: args.to_vec(),
|
||||
cwd: prepared.cwd.clone(),
|
||||
env,
|
||||
managed_network: managed_network_context,
|
||||
additional_permissions,
|
||||
};
|
||||
attempt
|
||||
.env_for(
|
||||
command,
|
||||
options,
|
||||
managed_network,
|
||||
Some(&prepared.turn_environment.selection.environment_id),
|
||||
)
|
||||
.await
|
||||
}
|
||||
.map_err(ToolError::Codex)?;
|
||||
let process = self
|
||||
.manager
|
||||
.open_session_with_exec_env(
|
||||
req.process_id,
|
||||
ctx,
|
||||
command,
|
||||
options,
|
||||
attempt,
|
||||
request,
|
||||
managed_network,
|
||||
network_proxy_launch,
|
||||
/*environment_id*/ Some(&prepared.turn_environment.selection.environment_id),
|
||||
req.exec_server_env_config.clone(),
|
||||
req.shell_snapshot.clone(),
|
||||
windows_sandbox_proxy_settings_mode,
|
||||
|
||||
@@ -14,17 +14,21 @@ use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
||||
use codex_file_system::FileSystemSandboxContext;
|
||||
use codex_file_system::WindowsSandboxSelection;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
pub(crate) use codex_protocol::sandbox::SandboxOverride;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxExecRequest;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxSelection;
|
||||
use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
@@ -505,7 +509,8 @@ impl<'a> SandboxAttempt<'a> {
|
||||
command,
|
||||
permissions: self.permissions,
|
||||
workspace_roots: None,
|
||||
sandbox: self.sandbox,
|
||||
runtime_readable_paths: &[],
|
||||
selection: SandboxSelection::Explicit(self.sandbox),
|
||||
enforce_managed_network: self.enforce_managed_network,
|
||||
environment_id,
|
||||
network,
|
||||
@@ -527,32 +532,28 @@ impl<'a> SandboxAttempt<'a> {
|
||||
|
||||
pub async fn env_for_exec_server(
|
||||
&self,
|
||||
command: SandboxCommand,
|
||||
argv: Vec<String>,
|
||||
cwd: PathUri,
|
||||
env: HashMap<String, String>,
|
||||
managed_network: Option<ManagedNetworkSandboxContext>,
|
||||
additional_permissions: Option<&AdditionalPermissionProfile>,
|
||||
options: ExecOptions,
|
||||
) -> Result<crate::sandboxing::ExecRequest, CodexErr> {
|
||||
let managed_network = command.managed_network.clone();
|
||||
let exec_server_permissions = effective_permission_profile(
|
||||
self.exec_server_permissions,
|
||||
command.additional_permissions.as_ref(),
|
||||
);
|
||||
let mut request = self
|
||||
.manager
|
||||
.transform(SandboxTransformRequest {
|
||||
command,
|
||||
permissions: self.permissions,
|
||||
workspace_roots: None,
|
||||
// The exec-server must receive the native command, not this host's wrapper.
|
||||
sandbox: SandboxType::None,
|
||||
enforce_managed_network: self.enforce_managed_network,
|
||||
environment_id: None,
|
||||
network: None,
|
||||
sandbox_policy_cwd: self.sandbox_cwd,
|
||||
sandbox_exe: None,
|
||||
use_legacy_landlock: self.use_legacy_landlock,
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
})?
|
||||
.into_command_request();
|
||||
request.sandbox_override = self.sandbox_override;
|
||||
let exec_server_permissions =
|
||||
effective_permission_profile(self.exec_server_permissions, additional_permissions);
|
||||
let request = SandboxExecRequest {
|
||||
command: argv,
|
||||
cwd,
|
||||
env,
|
||||
network: None,
|
||||
network_environment_id: None,
|
||||
sandbox: SandboxType::None,
|
||||
sandbox_override: self.sandbox_override,
|
||||
sandbox_policy_cwd: self.sandbox_cwd.clone(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
permission_profile: exec_server_permissions.clone(),
|
||||
arg0: None,
|
||||
};
|
||||
let mut exec_request =
|
||||
crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, Vec::new())
|
||||
.await?;
|
||||
|
||||
@@ -363,15 +363,16 @@ async fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backe
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
let cwd: AbsolutePathBuf = std::env::current_dir()
|
||||
.expect("current dir")
|
||||
.try_into()
|
||||
.expect("absolute cwd");
|
||||
let cwd_uri = PathUri::from_abs_path(&cwd);
|
||||
let cwd_uri = PathUri::parse(if cfg!(windows) {
|
||||
"file:///workspace"
|
||||
} else {
|
||||
"file:///C:/workspace"
|
||||
})
|
||||
.expect("foreign workspace");
|
||||
let exec_server_permissions = codex_protocol::models::PermissionProfile::workspace_write();
|
||||
let permissions = exec_server_permissions
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(std::slice::from_ref(&cwd));
|
||||
.materialize_project_roots_with_path_uris(std::slice::from_ref(&cwd_uri));
|
||||
let manager = SandboxManager::new();
|
||||
let mut attempt = SandboxAttempt {
|
||||
sandbox_override: SandboxOverride::NoOverride,
|
||||
@@ -395,32 +396,33 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
allow_local_binding: false,
|
||||
..Default::default()
|
||||
};
|
||||
let command = || SandboxCommand {
|
||||
program: "/bin/bash".into(),
|
||||
args: vec!["-lc".to_string(), "pwd".to_string()],
|
||||
cwd: cwd_uri.clone(),
|
||||
env: HashMap::new(),
|
||||
managed_network: Some(managed_network.clone()),
|
||||
additional_permissions: None,
|
||||
let argv = vec![
|
||||
"/bin/bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"printf 'café au lait'".to_string(),
|
||||
String::new(),
|
||||
];
|
||||
let prepare = async |attempt: &SandboxAttempt<'_>| {
|
||||
attempt
|
||||
.env_for_exec_server(
|
||||
argv.clone(),
|
||||
cwd_uri.clone(),
|
||||
HashMap::new(),
|
||||
Some(managed_network.clone()),
|
||||
/*additional_permissions*/ None,
|
||||
crate::sandboxing::ExecOptions {
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
},
|
||||
)
|
||||
.await
|
||||
};
|
||||
let options = || crate::sandboxing::ExecOptions {
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let request = attempt
|
||||
.env_for_exec_server(command(), options())
|
||||
let request = prepare(&attempt)
|
||||
.await
|
||||
.expect("prepare remote exec request");
|
||||
assert!(!attempt.is_escalated());
|
||||
|
||||
assert_eq!(
|
||||
request.command,
|
||||
vec![
|
||||
"/bin/bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"pwd".to_string()
|
||||
]
|
||||
);
|
||||
assert_eq!(request.command, argv);
|
||||
assert_eq!(request.arg0, None);
|
||||
assert_eq!(request.sandbox, SandboxType::None);
|
||||
assert_eq!(
|
||||
@@ -433,9 +435,9 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
user_home_dir: None,
|
||||
temporary_directories: None,
|
||||
windows_sandbox_selection: if cfg!(windows) {
|
||||
codex_file_system::WindowsSandboxSelection::RestrictedToken
|
||||
} else {
|
||||
codex_file_system::WindowsSandboxSelection::Disabled
|
||||
} else {
|
||||
codex_file_system::WindowsSandboxSelection::RestrictedToken
|
||||
},
|
||||
windows_sandbox_proxy_settings_mode: None,
|
||||
use_legacy_landlock: false,
|
||||
@@ -448,8 +450,7 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
);
|
||||
|
||||
attempt.sandbox_override = SandboxOverride::EscalatedSandboxWithRestrictions;
|
||||
let escalated_request = attempt
|
||||
.env_for_exec_server(command(), options())
|
||||
let escalated_request = prepare(&attempt)
|
||||
.await
|
||||
.expect("prepare approved escalation with retained sandbox restrictions");
|
||||
let mut expected_context = request.exec_server_sandbox.expect("sandbox context");
|
||||
@@ -460,8 +461,7 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
);
|
||||
|
||||
attempt.sandbox_requested = false;
|
||||
let request = attempt
|
||||
.env_for_exec_server(command(), options())
|
||||
let request = prepare(&attempt)
|
||||
.await
|
||||
.expect("prepare unsandboxed remote exec request");
|
||||
assert!(attempt.is_escalated());
|
||||
|
||||
@@ -27,7 +27,6 @@ use crate::exec_env::inject_session_env;
|
||||
use crate::exec_env::set_tool_call_id_env_var;
|
||||
use crate::guardian::GuardianReviewContext;
|
||||
use crate::plugins::metrics::finish_and_track_measurements;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::ExecServerEnvConfig;
|
||||
use crate::tools::ApprovalContext;
|
||||
@@ -42,7 +41,6 @@ use crate::tools::runtimes::is_managed_proxy_env_var;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecAttempt;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecRuntime;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::unified_exec::ExecCommandRequest;
|
||||
@@ -84,7 +82,6 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_protocol::protocol::TerminalInteractionEvent;
|
||||
use codex_protocol::shell_environment::is_non_inheritable_env_var;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_tools::ToolName;
|
||||
use codex_utils_head_tail_buffer::HeadTailBuffer;
|
||||
use codex_utils_output_truncation::approx_tokens_from_byte_count;
|
||||
@@ -1310,36 +1307,29 @@ impl UnifiedExecProcessManager {
|
||||
&self,
|
||||
process_id: i32,
|
||||
tool_ctx: &ToolCtx,
|
||||
command: SandboxCommand,
|
||||
options: ExecOptions,
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
request: ExecRequest,
|
||||
network: Option<&NetworkProxy>,
|
||||
network_proxy_launch: Option<codex_network_proxy::RemoteNetworkProxyLaunchConfig>,
|
||||
environment_id: Option<&str>,
|
||||
exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
shell_snapshot: Option<codex_exec_server::ShellSnapshotRequest>,
|
||||
windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode,
|
||||
tty: bool,
|
||||
environment: &codex_exec_server::Environment,
|
||||
) -> Result<UnifiedExecProcess, ToolError> {
|
||||
let mut request = if environment.is_remote() || shell_snapshot.is_some() {
|
||||
attempt.env_for_exec_server(command, options).await
|
||||
} else {
|
||||
attempt
|
||||
.env_for(command, options, network, environment_id)
|
||||
.await
|
||||
}
|
||||
.map_err(ToolError::Codex)?;
|
||||
let network_policy_decider = network_proxy_launch
|
||||
.as_ref()
|
||||
.filter(|launch| launch.policy_decision_timeout_ms.is_some())
|
||||
.and_then(|launch| network?.remote_policy_decider(launch.proxy.allow_local_binding));
|
||||
if environment.is_remote() && network.is_some() && network_proxy_launch.is_none() {
|
||||
request.exec_server_enforce_managed_network = false;
|
||||
}
|
||||
request.exec_server_network_proxy = network_proxy_launch;
|
||||
request.exec_server_env_config = exec_server_env_config;
|
||||
request.exec_server_shell_snapshot = shell_snapshot;
|
||||
let missing_remote_proxy =
|
||||
environment.is_remote() && network.is_some() && network_proxy_launch.is_none();
|
||||
let request = ExecRequest {
|
||||
exec_server_enforce_managed_network: request.exec_server_enforce_managed_network
|
||||
&& !missing_remote_proxy,
|
||||
exec_server_network_proxy: network_proxy_launch,
|
||||
exec_server_env_config,
|
||||
exec_server_shell_snapshot: shell_snapshot,
|
||||
..request
|
||||
};
|
||||
self.open_session_with_prepared_exec_env(
|
||||
process_id,
|
||||
&request,
|
||||
|
||||
Reference in New Issue
Block a user