From 6d48b936581a3ca478258cd9d42e389098add1f2 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 8 Jun 2026 11:06:59 -0700 Subject: [PATCH] fix Windows unified exec sandbox backend selection --- codex-rs/cli/src/debug_sandbox.rs | 1 + codex-rs/core/src/exec.rs | 2 +- .../core/src/unified_exec/process_manager.rs | 79 ++++---- codex-rs/core/tests/suite/windows_sandbox.rs | 185 ++++++++++++++++++ codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 3 +- .../src/unified_exec/backends/elevated.rs | 2 + .../src/unified_exec/mod.rs | 2 + 7 files changed, 233 insertions(+), 41 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 6d59bf7d23..f7193d90f8 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -385,6 +385,7 @@ async fn run_command_under_windows_session( cwd.as_path(), env, None, + /*proxy_enforced*/ false, /*read_roots_override*/ None, /*read_roots_include_platform_defaults*/ false, /*write_roots_override*/ None, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 756a65edd8..4800975fa8 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -113,7 +113,7 @@ pub(crate) struct WindowsSandboxFilesystemOverrides { pub(crate) additional_deny_write_paths: Vec, } -fn windows_sandbox_uses_elevated_backend( +pub(crate) fn windows_sandbox_uses_elevated_backend( sandbox_level: WindowsSandboxLevel, proxy_enforced: bool, ) -> bool { diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index cb455b113b..64074eff55 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -893,45 +893,46 @@ impl UnifiedExecProcessManager { .windows_sandbox_filesystem_overrides .as_ref() .and_then(|overrides| overrides.write_roots_override.clone()); - let spawned = match request.windows_sandbox_level { - codex_protocol::config_types::WindowsSandboxLevel::Elevated => { - codex_windows_sandbox::spawn_windows_sandbox_session_elevated_for_permission_profile( - &request.permission_profile, - request.windows_sandbox_workspace_roots.as_slice(), - codex_home.as_ref(), - request.command.clone(), - request.cwd.as_path(), - request.env.clone(), - None, - elevated_read_roots_override.as_deref(), - elevated_read_roots_include_platform_defaults, - elevated_write_roots_override.as_deref(), - &additional_deny_read_paths, - &additional_deny_write_paths, - tty, - tty, - request.windows_sandbox_private_desktop, - ) - .await - } - codex_protocol::config_types::WindowsSandboxLevel::RestrictedToken - | codex_protocol::config_types::WindowsSandboxLevel::Disabled => { - codex_windows_sandbox::spawn_windows_sandbox_session_legacy( - &request.permission_profile, - request.windows_sandbox_workspace_roots.as_slice(), - codex_home.as_ref(), - request.command.clone(), - request.cwd.as_path(), - request.env.clone(), - None, - &additional_deny_read_paths, - &additional_deny_write_paths, - tty, - tty, - request.windows_sandbox_private_desktop, - ) - .await - } + let proxy_enforced = request.network.is_some(); + let spawned = if crate::exec::windows_sandbox_uses_elevated_backend( + request.windows_sandbox_level, + proxy_enforced, + ) { + codex_windows_sandbox::spawn_windows_sandbox_session_elevated_for_permission_profile( + &request.permission_profile, + request.windows_sandbox_workspace_roots.as_slice(), + codex_home.as_ref(), + request.command.clone(), + request.cwd.as_path(), + request.env.clone(), + None, + proxy_enforced, + elevated_read_roots_override.as_deref(), + elevated_read_roots_include_platform_defaults, + elevated_write_roots_override.as_deref(), + &additional_deny_read_paths, + &additional_deny_write_paths, + tty, + tty, + request.windows_sandbox_private_desktop, + ) + .await + } else { + codex_windows_sandbox::spawn_windows_sandbox_session_legacy( + &request.permission_profile, + request.windows_sandbox_workspace_roots.as_slice(), + codex_home.as_ref(), + request.command.clone(), + request.cwd.as_path(), + request.env.clone(), + None, + &additional_deny_read_paths, + &additional_deny_write_paths, + tty, + tty, + request.windows_sandbox_private_desktop, + ) + .await }; spawn_lifecycle.after_spawn(); return UnifiedExecProcess::from_spawned( diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index 74d7f6a23b..10956b8009 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -4,17 +4,37 @@ use codex_core::exec::ExecParams; use codex_core::exec::process_exec_tool_call; use codex_core::sandboxing::SandboxPermissions; use codex_core::windows_sandbox::sandbox_setup_is_complete; +use codex_core::windows_sandbox::windows_sandbox_level_from_config; +use codex_features::Feature; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::ResponseItem; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::user_input::UserInput; use core_test_support::PathExt; +use core_test_support::managed_network_requirements_loader; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_with_timeout; use pretty_assertions::assert_eq; +use serde_json::json; use serial_test::serial; use std::collections::HashMap; use std::ffi::OsString; @@ -320,3 +340,168 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh ); Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial(codex_home)] +async fn windows_unified_exec_managed_network_enforces_deny_read() -> anyhow::Result<()> { + let codex_home = + codex_home_for_windows_sandbox_test("windows-unified-exec-managed-network-codex-home")?; + let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str()); + stage_windows_sandbox_helpers()?; + + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: FileSystemAccessMode::Deny, + }, + ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + ); + let permission_profile_for_config = permission_profile.clone(); + + let server = start_mock_server().await; + let mut builder = test_codex() + .with_cloud_config_bundle(managed_network_requirements_loader()) + .with_config(move |config| { + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config.set_windows_sandbox_enabled(true); + config.set_windows_elevated_sandbox_enabled(false); + config + .permissions + .set_permission_profile(permission_profile_for_config) + .expect("set permission profile"); + }); + let test = builder.build(&server).await?; + assert!( + test.config.permissions.network.is_some(), + "expected managed network proxy config to be present" + ); + assert_eq!( + windows_sandbox_level_from_config(&test.config), + WindowsSandboxLevel::RestrictedToken + ); + + std::fs::write( + test.config.cwd.join("secret.env"), + "managed network secret\n", + )?; + std::fs::write(test.config.cwd.join("public.txt"), "public ok\n")?; + + let call_id = "windows-unified-exec-managed-network-deny-read"; + let args = json!({ + "cmd": "cmd.exe /D /C \"(type secret.env 1>NUL 2>NUL && echo SECRET-READ || echo SECRET-DENIED) & type public.txt\"", + "yield_time_ms": 10_000, + }); + mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.config.cwd.as_path()); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "read the fixture files".into(), + text_elements: Vec::new(), + }], + environments: None, + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + cwd: Some(test.config.cwd.clone()), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let output = wait_for_event_with_timeout( + &test.codex, + |event| { + matches!( + event, + EventMsg::RawResponseItem(raw) + if matches!( + &raw.item, + ResponseItem::FunctionCallOutput { + call_id: output_call_id, + .. + } if output_call_id == call_id + ) + ) + }, + tokio::time::Duration::from_secs(30), + ) + .await; + let EventMsg::RawResponseItem(raw) = output else { + unreachable!("matched raw response item"); + }; + let ResponseItem::FunctionCallOutput { output, .. } = raw.item else { + unreachable!("matched function call output"); + }; + let output = output + .text_content() + .expect("function call output should contain text"); + + assert!( + output.contains("SECRET-DENIED"), + "deny-read should block the secret: {output}" + ); + assert!( + !output.contains("SECRET-READ") && !output.contains("managed network secret"), + "denied file contents leaked into unified exec output: {output}" + ); + assert!( + output.contains("public ok"), + "allowed reads should still work: {output}" + ); + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + Ok(()) +} diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index 7bdd59bf53..d53b2fc2a7 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -351,6 +351,7 @@ pub(crate) fn prepare_elevated_spawn_context_for_permissions( cwd: &Path, env_map: &mut HashMap, command: &[String], + proxy_enforced: bool, read_roots_override: Option<&[PathBuf]>, read_roots_include_platform_defaults: bool, write_roots_override: Option<&[PathBuf]>, @@ -410,7 +411,7 @@ pub(crate) fn prepare_elevated_spawn_context_for_permissions( } else { deny_write_paths_override }, - /*proxy_enforced*/ false, + proxy_enforced, )?; let caps = load_or_create_cap_sids(codex_home)?; let (psid_to_use, cap_sids) = if uses_write_capabilities { diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs index 0a2af6d7ad..5816fbb38e 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs @@ -32,6 +32,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil cwd: &Path, mut env_map: HashMap, timeout_ms: Option, + proxy_enforced: bool, read_roots_override: Option<&[PathBuf]>, read_roots_include_platform_defaults: bool, write_roots_override: Option<&[PathBuf]>, @@ -60,6 +61,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated_for_permission_profil cwd, &mut env_map, &command, + proxy_enforced, read_roots_override, read_roots_include_platform_defaults, write_roots_override, diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs index 2fe8b80ee1..4b42610282 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/mod.rs @@ -58,6 +58,7 @@ pub async fn spawn_windows_sandbox_session_elevated_for_permission_profile( cwd: &Path, env_map: HashMap, timeout_ms: Option, + proxy_enforced: bool, read_roots_override: Option<&[PathBuf]>, read_roots_include_platform_defaults: bool, write_roots_override: Option<&[PathBuf]>, @@ -75,6 +76,7 @@ pub async fn spawn_windows_sandbox_session_elevated_for_permission_profile( cwd, env_map, timeout_ms, + proxy_enforced, read_roots_override, read_roots_include_platform_defaults, write_roots_override,