mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Preserve Windows sandbox proxy settings in guardian sessions (#35036)
## Why Guardian review commands can run without the proxy-port environment used to configure the parent session. Reconciling persistent Windows sandbox settings for those commands can discard the parent's proxy configuration. ## What changed - Add a session-level Windows sandbox proxy-settings mode and use `Preserve` for guardian review sessions while keeping `Reconcile` as the default. - Carry the mode through unified exec and the exec-server sandbox context to Windows process launches. - In preserve mode, reuse the proxy settings recorded by the existing sandbox setup marker. ## Testing Added coverage for guardian session configuration, exec-server transport, and preserving an existing setup marker when proxy-port environment variables are absent. GitOrigin-RevId: 214655c6e6d97360906431773dc0de0fdda2db6e
This commit is contained in:
@@ -113,6 +113,7 @@ async fn exec_command_with_tty(
|
||||
.open_session_with_prepared_exec_env(
|
||||
process_id,
|
||||
&request,
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
|
||||
tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
turn.environments
|
||||
@@ -753,6 +754,7 @@ async fn completed_pipe_commands_preserve_exit_code() -> anyhow::Result<()> {
|
||||
.open_session_with_prepared_exec_env(
|
||||
/*process_id*/ 1234,
|
||||
&request,
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
|
||||
/*tty*/ false,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
&environment,
|
||||
@@ -793,6 +795,7 @@ async fn unified_exec_uses_remote_exec_server_when_configured() -> anyhow::Resul
|
||||
.open_session_with_prepared_exec_env(
|
||||
/*process_id*/ 1234,
|
||||
&request,
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
|
||||
/*tty*/ true,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
remote_test_env.environment(),
|
||||
@@ -851,6 +854,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<()
|
||||
.open_session_with_prepared_exec_env(
|
||||
/*process_id*/ 1234,
|
||||
&request,
|
||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
|
||||
/*tty*/ true,
|
||||
Box::new(TestSpawnLifecycle {
|
||||
inherited_fds: vec![42],
|
||||
|
||||
@@ -177,9 +177,14 @@ fn exec_server_env_for_request(
|
||||
fn exec_server_params_for_request(
|
||||
process_id: i32,
|
||||
request: &ExecRequest,
|
||||
windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode,
|
||||
tty: bool,
|
||||
) -> codex_exec_server::ExecParams {
|
||||
let (env_policy, env) = exec_server_env_for_request(request);
|
||||
let sandbox = request.exec_server_sandbox.clone().map(|mut sandbox| {
|
||||
sandbox.windows_sandbox_proxy_settings_mode = Some(windows_sandbox_proxy_settings_mode);
|
||||
sandbox
|
||||
});
|
||||
// Sandbox retries reuse the unified-exec ID but start a distinct executor process.
|
||||
let exec_server_process_id = if request.exec_server_sandbox.is_some() {
|
||||
format!("{process_id}-{}", Uuid::new_v4())
|
||||
@@ -195,7 +200,7 @@ fn exec_server_params_for_request(
|
||||
tty,
|
||||
pipe_stdin: false,
|
||||
arg0: request.arg0.clone(),
|
||||
sandbox: request.exec_server_sandbox.clone(),
|
||||
sandbox,
|
||||
enforce_managed_network: request.exec_server_enforce_managed_network,
|
||||
managed_network: request.exec_server_managed_network.clone(),
|
||||
network_proxy: request.exec_server_network_proxy.clone(),
|
||||
@@ -1003,6 +1008,7 @@ impl UnifiedExecProcessManager {
|
||||
network_proxy_launch: Option<codex_network_proxy::RemoteNetworkProxyLaunchConfig>,
|
||||
environment_id: Option<&str>,
|
||||
exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode,
|
||||
tty: bool,
|
||||
spawn_lifecycle: SpawnLifecycleHandle,
|
||||
environment: &codex_exec_server::Environment,
|
||||
@@ -1018,6 +1024,7 @@ impl UnifiedExecProcessManager {
|
||||
self.open_session_with_prepared_exec_env(
|
||||
process_id,
|
||||
&request,
|
||||
windows_sandbox_proxy_settings_mode,
|
||||
tty,
|
||||
spawn_lifecycle,
|
||||
environment,
|
||||
@@ -1038,6 +1045,7 @@ impl UnifiedExecProcessManager {
|
||||
&self,
|
||||
process_id: i32,
|
||||
request: &ExecRequest,
|
||||
windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode,
|
||||
tty: bool,
|
||||
mut spawn_lifecycle: SpawnLifecycleHandle,
|
||||
environment: &codex_exec_server::Environment,
|
||||
@@ -1053,7 +1061,12 @@ impl UnifiedExecProcessManager {
|
||||
|
||||
let started = environment
|
||||
.get_exec_backend()
|
||||
.start(exec_server_params_for_request(process_id, request, tty))
|
||||
.start(exec_server_params_for_request(
|
||||
process_id,
|
||||
request,
|
||||
windows_sandbox_proxy_settings_mode,
|
||||
tty,
|
||||
))
|
||||
.await
|
||||
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
|
||||
spawn_lifecycle.after_spawn();
|
||||
@@ -1100,22 +1113,21 @@ impl UnifiedExecProcessManager {
|
||||
None::<String>
|
||||
}
|
||||
};
|
||||
let windows_sandbox = if request.sandbox
|
||||
== codex_sandboxing::SandboxType::WindowsRestrictedToken
|
||||
{
|
||||
Some(codex_sandboxing::WindowsSandboxSpawnRequest {
|
||||
permission_profile: &request.permission_profile,
|
||||
workspace_roots: &request.windows_sandbox_workspace_roots,
|
||||
windows_sandbox_level: request.windows_sandbox_level,
|
||||
proxy_enforced: request.network.is_some(),
|
||||
network_proxy_restricting_sid: network_proxy_restricting_sid.as_deref(),
|
||||
proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
|
||||
filesystem_overrides: request.windows_sandbox_filesystem_overrides.as_ref(),
|
||||
use_private_desktop: request.windows_sandbox_private_desktop,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let windows_sandbox =
|
||||
if request.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken {
|
||||
Some(codex_sandboxing::WindowsSandboxSpawnRequest {
|
||||
permission_profile: &request.permission_profile,
|
||||
workspace_roots: &request.windows_sandbox_workspace_roots,
|
||||
windows_sandbox_level: request.windows_sandbox_level,
|
||||
proxy_enforced: request.network.is_some(),
|
||||
network_proxy_restricting_sid: network_proxy_restricting_sid.as_deref(),
|
||||
proxy_settings_mode: windows_sandbox_proxy_settings_mode,
|
||||
filesystem_overrides: request.windows_sandbox_filesystem_overrides.as_ref(),
|
||||
use_private_desktop: request.windows_sandbox_private_desktop,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let spawn_result = codex_sandboxing::spawn_process(codex_sandboxing::SpawnRequest {
|
||||
command: &request.command,
|
||||
cwd: native_cwd.as_path(),
|
||||
|
||||
@@ -177,8 +177,16 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() {
|
||||
exec_server_network_proxy: None,
|
||||
};
|
||||
|
||||
let params =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
let proxy_settings_mode = codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve;
|
||||
let params_for_request = |request: &ExecRequest| {
|
||||
exec_server_params_for_request(
|
||||
/*process_id*/ 123,
|
||||
request,
|
||||
proxy_settings_mode,
|
||||
/*tty*/ true,
|
||||
)
|
||||
};
|
||||
let params = params_for_request(&request);
|
||||
|
||||
assert_eq!(params.process_id.as_str(), "123");
|
||||
assert_eq!(params.cwd, request.cwd);
|
||||
@@ -200,10 +208,15 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() {
|
||||
request.exec_server_sandbox = Some(
|
||||
codex_exec_server::FileSystemSandboxContext::from_permission_profile(permission_profile),
|
||||
);
|
||||
let first =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
let second =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
let first = params_for_request(&request);
|
||||
let second = params_for_request(&request);
|
||||
assert_eq!(
|
||||
first
|
||||
.sandbox
|
||||
.as_ref()
|
||||
.and_then(|sandbox| sandbox.windows_sandbox_proxy_settings_mode),
|
||||
Some(codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve)
|
||||
);
|
||||
assert!(first.process_id.as_str().starts_with("123-"));
|
||||
assert!(second.process_id.as_str().starts_with("123-"));
|
||||
assert_ne!(first.process_id, second.process_id);
|
||||
|
||||
Reference in New Issue
Block a user