From c70a5eb4b7012e1d4e270fd533b07247689e2fe0 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Mar 2026 10:08:02 -0700 Subject: [PATCH 1/4] feat: support restricted windows read-only sandboxing --- codex-rs/core/README.md | 21 +++ .../src/elevated/command_runner_win.rs | 5 - .../windows-sandbox-rs/src/elevated_impl.rs | 5 - codex-rs/windows-sandbox-rs/src/lib.rs | 5 - .../src/setup_orchestrator.rs | 176 ++++++++++++++++-- 5 files changed, 183 insertions(+), 29 deletions(-) diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 77966ea88c6..96aaea076c7 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -65,6 +65,27 @@ falls back to the vendored bubblewrap path otherwise. When `/usr/bin/bwrap` is missing, Codex also surfaces a startup warning through its normal notification path instead of printing directly from the sandbox helper. +### Windows + +Legacy `SandboxPolicy` / `sandbox_mode` configs are still supported on +Windows. + +Both Windows execution paths, the unelevated restricted-token backend and the +elevated setup/runner backend, now support legacy `ReadOnlyAccess::Restricted` +for `read-only` and `workspace-write` policies. Restricted read access honors +explicit readable roots plus the command `cwd`, and keeps writable roots +readable when `workspace-write` is used. + +When `include_platform_defaults = true`, Windows adds backend-managed system +read roots required for basic execution, such as `C:\Windows`, +`C:\Program Files`, `C:\Program Files (x86)`, and `C:\ProgramData`. When it is +`false`, those extra system roots are omitted. + +New `[permissions]` / split filesystem policies remain supported on Windows +only when they round-trip through the legacy `SandboxPolicy` model without +changing semantics. Richer split-only carveouts still fail closed instead of +running with weaker enforcement. + ### All Platforms Expects the binary containing `codex-core` to simulate the virtual `apply_patch` CLI when `arg1` is `--codex-run-as-apply-patch`. See the `codex-arg0` crate for details. diff --git a/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs b/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs index 93956fb41d2..87b0e2a8128 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated/command_runner_win.rs @@ -199,11 +199,6 @@ fn spawn_ipc_process( ); let policy = parse_policy(&req.policy_json_or_preset).context("parse policy_json_or_preset")?; - if !policy.has_full_disk_read_access() { - anyhow::bail!( - "Restricted read-only access is not yet supported by the Windows sandbox backend" - ); - } let mut cap_psids: Vec<*mut c_void> = Vec::new(); for sid in &req.cap_sids { let Some(psid) = (unsafe { convert_string_sid_to_sid(sid) }) else { diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index b925e83743b..f6c4563e171 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -238,11 +238,6 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } - if !policy.has_full_disk_read_access() { - anyhow::bail!( - "Restricted read-only access is not yet supported by the Windows sandbox backend" - ); - } let caps = load_or_create_cap_sids(codex_home)?; let (psid_to_use, cap_sids) = match &policy { SandboxPolicy::ReadOnly { .. } => ( diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 51751b9cb92..cbb32db351d 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -289,11 +289,6 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } - if !policy.has_full_disk_read_access() { - anyhow::bail!( - "Restricted read-only access is not yet supported by the Windows sandbox backend" - ); - } let caps = load_or_create_cap_sids(codex_home)?; let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe { match &policy { diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index d43d8b85fc2..317a4d467c0 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -51,6 +51,12 @@ const USERPROFILE_READ_ROOT_EXCLUSIONS: &[&str] = &[ ".pki", ".terraform.d", ]; +const WINDOWS_PLATFORM_DEFAULT_READ_ROOTS: &[&str] = &[ + r"C:\Windows", + r"C:\Program Files", + r"C:\Program Files (x86)", + r"C:\ProgramData", +]; pub fn sandbox_dir(codex_home: &Path) -> PathBuf { codex_home.join(".sandbox") @@ -281,12 +287,8 @@ fn profile_read_roots(user_profile: &Path) -> Vec { .collect() } -pub(crate) fn gather_read_roots( - command_cwd: &Path, - policy: &SandboxPolicy, - codex_home: &Path, -) -> Vec { - let mut roots: Vec = Vec::new(); +fn gather_helper_read_roots(codex_home: &Path) -> Vec { + let mut roots = Vec::new(); if let Ok(exe) = std::env::current_exe() { if let Some(dir) = exe.parent() { roots.push(dir.to_path_buf()); @@ -295,14 +297,20 @@ pub(crate) fn gather_read_roots( let helper_dir = helper_bin_dir(codex_home); let _ = std::fs::create_dir_all(&helper_dir); roots.push(helper_dir); - for p in [ - PathBuf::from(r"C:\Windows"), - PathBuf::from(r"C:\Program Files"), - PathBuf::from(r"C:\Program Files (x86)"), - PathBuf::from(r"C:\ProgramData"), - ] { - roots.push(p); - } + roots +} + +fn gather_legacy_full_read_roots( + command_cwd: &Path, + policy: &SandboxPolicy, + codex_home: &Path, +) -> Vec { + let mut roots = gather_helper_read_roots(codex_home); + roots.extend( + WINDOWS_PLATFORM_DEFAULT_READ_ROOTS + .iter() + .map(PathBuf::from), + ); if let Ok(up) = std::env::var("USERPROFILE") { roots.extend(profile_read_roots(Path::new(&up))); } @@ -315,6 +323,40 @@ pub(crate) fn gather_read_roots( canonical_existing(&roots) } +fn gather_restricted_read_roots( + command_cwd: &Path, + policy: &SandboxPolicy, + codex_home: &Path, +) -> Vec { + let mut roots = gather_helper_read_roots(codex_home); + if policy.include_platform_defaults() { + roots.extend( + WINDOWS_PLATFORM_DEFAULT_READ_ROOTS + .iter() + .map(PathBuf::from), + ); + } + roots.extend( + policy + .get_readable_roots_with_cwd(command_cwd) + .into_iter() + .map(|path| path.to_path_buf()), + ); + canonical_existing(&roots) +} + +pub(crate) fn gather_read_roots( + command_cwd: &Path, + policy: &SandboxPolicy, + codex_home: &Path, +) -> Vec { + if policy.has_full_disk_read_access() { + gather_legacy_full_read_roots(command_cwd, policy, codex_home) + } else { + gather_restricted_read_roots(command_cwd, policy, codex_home) + } +} + pub(crate) fn gather_write_roots( policy: &SandboxPolicy, policy_cwd: &Path, @@ -629,16 +671,27 @@ fn filter_sensitive_write_roots(mut roots: Vec, codex_home: &Path) -> V #[cfg(test)] mod tests { + use super::gather_legacy_full_read_roots; use super::gather_read_roots; use super::profile_read_roots; + use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS; use crate::helper_materialization::helper_bin_dir; use crate::policy::SandboxPolicy; + use codex_protocol::protocol::ReadOnlyAccess; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashSet; use std::fs; use std::path::PathBuf; use tempfile::TempDir; + fn canonical_windows_platform_default_roots() -> Vec { + WINDOWS_PLATFORM_DEFAULT_READ_ROOTS + .iter() + .map(|path| dunce::canonicalize(path).unwrap_or_else(|_| PathBuf::from(path))) + .collect() + } + #[test] fn profile_read_roots_excludes_configured_top_level_entries() { let tmp = TempDir::new().expect("tempdir"); @@ -684,4 +737,99 @@ mod tests { assert!(roots.contains(&expected)); } + + #[test] + fn restricted_read_roots_skip_platform_defaults_when_disabled() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let command_cwd = tmp.path().join("workspace"); + let readable_root = tmp.path().join("docs"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&readable_root).expect("create readable root"); + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![AbsolutePathBuf::from_absolute_path(&readable_root) + .expect("absolute readable root")], + }, + network_access: false, + }; + + let roots = gather_read_roots(&command_cwd, &policy, &codex_home); + let expected_helper = + dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir"); + let expected_cwd = dunce::canonicalize(&command_cwd).expect("canonical workspace"); + let expected_readable = + dunce::canonicalize(&readable_root).expect("canonical readable root"); + + assert!(roots.contains(&expected_helper)); + assert!(roots.contains(&expected_cwd)); + assert!(roots.contains(&expected_readable)); + assert!(canonical_windows_platform_default_roots() + .into_iter() + .all(|path| !roots.contains(&path))); + } + + #[test] + fn restricted_read_roots_include_platform_defaults_when_enabled() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let command_cwd = tmp.path().join("workspace"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: false, + }; + + let roots = gather_read_roots(&command_cwd, &policy, &codex_home); + + assert!(canonical_windows_platform_default_roots() + .into_iter() + .all(|path| roots.contains(&path))); + } + + #[test] + fn restricted_workspace_write_roots_remain_readable() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let command_cwd = tmp.path().join("workspace"); + let writable_root = tmp.path().join("extra-write-root"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + fs::create_dir_all(&writable_root).expect("create writable root"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::from_absolute_path(&writable_root) + .expect("absolute writable root")], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let roots = gather_read_roots(&command_cwd, &policy, &codex_home); + let expected_writable = + dunce::canonicalize(&writable_root).expect("canonical writable root"); + + assert!(roots.contains(&expected_writable)); + } + + #[test] + fn full_read_roots_preserve_legacy_platform_defaults() { + let tmp = TempDir::new().expect("tempdir"); + let codex_home = tmp.path().join("codex-home"); + let command_cwd = tmp.path().join("workspace"); + fs::create_dir_all(&command_cwd).expect("create workspace"); + let policy = SandboxPolicy::new_read_only_policy(); + + let roots = gather_legacy_full_read_roots(&command_cwd, &policy, &codex_home); + + assert!(canonical_windows_platform_default_roots() + .into_iter() + .all(|path| roots.contains(&path))); + } } From f532d2fdf78db5a11cbfcf7e4e54976d3f599393 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Mar 2026 11:02:32 -0700 Subject: [PATCH 2/4] fix: keep restricted read elevated-only on windows --- codex-rs/core/README.md | 15 +++++++----- codex-rs/core/src/exec.rs | 1 + codex-rs/core/src/exec_tests.rs | 32 ++++++++++++++++++++++++++ codex-rs/windows-sandbox-rs/src/lib.rs | 5 ++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 96aaea076c7..3558fd9f609 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -70,16 +70,19 @@ path instead of printing directly from the sandbox helper. Legacy `SandboxPolicy` / `sandbox_mode` configs are still supported on Windows. -Both Windows execution paths, the unelevated restricted-token backend and the -elevated setup/runner backend, now support legacy `ReadOnlyAccess::Restricted` +The elevated setup/runner backend supports legacy `ReadOnlyAccess::Restricted` for `read-only` and `workspace-write` policies. Restricted read access honors explicit readable roots plus the command `cwd`, and keeps writable roots readable when `workspace-write` is used. -When `include_platform_defaults = true`, Windows adds backend-managed system -read roots required for basic execution, such as `C:\Windows`, -`C:\Program Files`, `C:\Program Files (x86)`, and `C:\ProgramData`. When it is -`false`, those extra system roots are omitted. +When `include_platform_defaults = true`, the elevated Windows backend adds +backend-managed system read roots required for basic execution, such as +`C:\Windows`, `C:\Program Files`, `C:\Program Files (x86)`, and +`C:\ProgramData`. When it is `false`, those extra system roots are omitted. + +The unelevated restricted-token backend still supports the legacy full-read +Windows model only. Restricted read-only policies continue to fail closed there +instead of running with weaker read enforcement. New `[permissions]` / split filesystem policies remain supported on Windows only when they round-trip through the legacy `SandboxPolicy` model without diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 4f462821e5c..53283571a29 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -823,6 +823,7 @@ fn should_use_windows_restricted_token_sandbox( file_system_sandbox_policy: &FileSystemSandboxPolicy, ) -> bool { sandbox == SandboxType::WindowsRestrictedToken + && sandbox_policy.has_full_disk_read_access() && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted && !matches!( sandbox_policy, diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 10ba5734faf..3c02c5af290 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -248,6 +248,38 @@ fn windows_restricted_token_allows_legacy_restricted_policies() { ); } +#[test] +fn windows_restricted_token_rejects_restricted_read_only_policies() { + let policy = SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![], + }, + network_access: false, + }; + let file_system_policy = FileSystemSandboxPolicy::from(&policy); + + assert_eq!( + should_use_windows_restricted_token_sandbox( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + ), + false + ); + assert_eq!( + unsupported_windows_restricted_token_sandbox_reason( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ), + Some( + "windows sandbox backend cannot enforce file_system=Restricted, network=Restricted, legacy_policy=ReadOnly { access: Restricted { include_platform_defaults: true, readable_roots: [] }, network_access: false }; refusing to run unsandboxed".to_string() + ) + ); +} + #[test] fn windows_restricted_token_allows_legacy_workspace_write_policies() { let policy = SandboxPolicy::WorkspaceWrite { diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index cbb32db351d..cb4be527500 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -289,6 +289,11 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access requires the elevated Windows sandbox backend" + ); + } let caps = load_or_create_cap_sids(codex_home)?; let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe { match &policy { From 933d96b55bec56a58e5ee3ac68b43d62ff0465e3 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Mar 2026 11:54:03 -0700 Subject: [PATCH 3/4] fix: allow elevated windows sandbox restricted read --- codex-rs/core/src/exec.rs | 13 ++++++++++- codex-rs/core/src/exec_tests.rs | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 53283571a29..57d8cacdf63 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -767,6 +767,7 @@ async fn exec( if sandbox == SandboxType::WindowsRestrictedToken { if let Some(reason) = unsupported_windows_restricted_token_sandbox_reason( sandbox, + params.windows_sandbox_level, sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, @@ -819,27 +820,37 @@ async fn exec( #[cfg_attr(not(target_os = "windows"), allow(dead_code))] fn should_use_windows_restricted_token_sandbox( sandbox: SandboxType, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, ) -> bool { + // Windows currently reuses SandboxType::WindowsRestrictedToken for both + // the legacy restricted-token backend and the elevated setup/runner path. + // The sandbox level decides whether restricted read-only policies are + // supported. sandbox == SandboxType::WindowsRestrictedToken - && sandbox_policy.has_full_disk_read_access() && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted && !matches!( sandbox_policy, SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } ) + && (matches!( + windows_sandbox_level, + codex_protocol::config_types::WindowsSandboxLevel::Elevated + ) || sandbox_policy.has_full_disk_read_access()) } #[cfg(any(target_os = "windows", test))] fn unsupported_windows_restricted_token_sandbox_reason( sandbox: SandboxType, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, ) -> Option { if should_use_windows_restricted_token_sandbox( sandbox, + windows_sandbox_level, sandbox_policy, file_system_sandbox_policy, ) { diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 3c02c5af290..595d3ccc928 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -190,6 +190,7 @@ fn windows_restricted_token_skips_external_sandbox_policies() { assert_eq!( should_use_windows_restricted_token_sandbox( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, ), @@ -205,6 +206,7 @@ fn windows_restricted_token_runs_for_legacy_restricted_policies() { assert_eq!( should_use_windows_restricted_token_sandbox( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, ), @@ -222,6 +224,7 @@ fn windows_restricted_token_rejects_network_only_restrictions() { assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, @@ -240,6 +243,7 @@ fn windows_restricted_token_allows_legacy_restricted_policies() { assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, @@ -262,6 +266,7 @@ fn windows_restricted_token_rejects_restricted_read_only_policies() { assert_eq!( should_use_windows_restricted_token_sandbox( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, ), @@ -270,6 +275,7 @@ fn windows_restricted_token_rejects_restricted_read_only_policies() { assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, @@ -294,6 +300,39 @@ fn windows_restricted_token_allows_legacy_workspace_write_policies() { assert_eq!( unsupported_windows_restricted_token_sandbox_reason( SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ), + None + ); +} + +#[test] +fn windows_elevated_sandbox_allows_restricted_read_only_policies() { + let policy = SandboxPolicy::ReadOnly { + access: codex_protocol::protocol::ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![], + }, + network_access: false, + }; + let file_system_policy = FileSystemSandboxPolicy::from(&policy); + + assert_eq!( + should_use_windows_restricted_token_sandbox( + SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Elevated, + &policy, + &file_system_policy, + ), + true + ); + assert_eq!( + unsupported_windows_restricted_token_sandbox_reason( + SandboxType::WindowsRestrictedToken, + codex_protocol::config_types::WindowsSandboxLevel::Elevated, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, From d1049328a6e85f5e1832a13817427e21d76d6202 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 17 Mar 2026 18:43:52 -0700 Subject: [PATCH 4/4] fix: simplify windows sandbox support checks --- codex-rs/core/src/exec.rs | 64 +++++++++--------- codex-rs/core/src/exec_tests.rs | 112 +++++++++++++++++--------------- 2 files changed, 96 insertions(+), 80 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 57d8cacdf63..3569917b5ca 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -34,6 +34,7 @@ use crate::spawn::spawn_child_async; use crate::text_encoding::bytes_to_string_smart; use crate::tools::sandboxing::SandboxablePreference; use codex_network_proxy::NetworkProxy; +#[cfg(any(target_os = "windows", test))] use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -765,13 +766,14 @@ async fn exec( ) -> Result { #[cfg(target_os = "windows")] if sandbox == SandboxType::WindowsRestrictedToken { - if let Some(reason) = unsupported_windows_restricted_token_sandbox_reason( + let support = windows_restricted_token_sandbox_support( sandbox, params.windows_sandbox_level, sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - ) { + ); + if let Some(reason) = support.unsupported_reason { return Err(CodexErr::Io(io::Error::other(reason))); } return exec_windows_sandbox(params, sandbox_policy).await; @@ -818,18 +820,32 @@ async fn exec( } #[cfg_attr(not(target_os = "windows"), allow(dead_code))] -fn should_use_windows_restricted_token_sandbox( +#[derive(Debug, PartialEq, Eq)] +struct WindowsRestrictedTokenSandboxSupport { + should_use: bool, + unsupported_reason: Option, +} + +#[cfg(any(target_os = "windows", test))] +fn windows_restricted_token_sandbox_support( sandbox: SandboxType, windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, sandbox_policy: &SandboxPolicy, file_system_sandbox_policy: &FileSystemSandboxPolicy, -) -> bool { + network_sandbox_policy: NetworkSandboxPolicy, +) -> WindowsRestrictedTokenSandboxSupport { + if sandbox != SandboxType::WindowsRestrictedToken { + return WindowsRestrictedTokenSandboxSupport { + should_use: false, + unsupported_reason: None, + }; + } + // Windows currently reuses SandboxType::WindowsRestrictedToken for both // the legacy restricted-token backend and the elevated setup/runner path. // The sandbox level decides whether restricted read-only policies are // supported. - sandbox == SandboxType::WindowsRestrictedToken - && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted + let should_use = file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted && !matches!( sandbox_policy, SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } @@ -837,33 +853,23 @@ fn should_use_windows_restricted_token_sandbox( && (matches!( windows_sandbox_level, codex_protocol::config_types::WindowsSandboxLevel::Elevated - ) || sandbox_policy.has_full_disk_read_access()) -} + ) || sandbox_policy.has_full_disk_read_access()); -#[cfg(any(target_os = "windows", test))] -fn unsupported_windows_restricted_token_sandbox_reason( - sandbox: SandboxType, - windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, -) -> Option { - if should_use_windows_restricted_token_sandbox( - sandbox, - windows_sandbox_level, - sandbox_policy, - file_system_sandbox_policy, - ) { - return None; - } - - (sandbox == SandboxType::WindowsRestrictedToken).then(|| { - format!( + let unsupported_reason = if should_use { + None + } else { + Some(format!( "windows sandbox backend cannot enforce file_system={:?}, network={network_sandbox_policy:?}, legacy_policy={sandbox_policy:?}; refusing to run unsandboxed", file_system_sandbox_policy.kind, - ) - }) + )) + }; + + WindowsRestrictedTokenSandboxSupport { + should_use, + unsupported_reason, + } } + /// Consumes the output of a child process, truncating it so it is suitable for /// use as the output of a `shell` tool call. Also enforces specified timeout. async fn consume_truncated_output( diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 595d3ccc928..0b5254f43d3 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_protocol::config_types::WindowsSandboxLevel; use pretty_assertions::assert_eq; use std::time::Duration; use tokio::io::AsyncWriteExt; @@ -188,13 +189,19 @@ fn windows_restricted_token_skips_external_sandbox_policies() { let file_system_policy = FileSystemSandboxPolicy::restricted(vec![]); assert_eq!( - should_use_windows_restricted_token_sandbox( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, + NetworkSandboxPolicy::Restricted, ), - false + WindowsRestrictedTokenSandboxSupport { + should_use: false, + unsupported_reason: Some( + "windows sandbox backend cannot enforce file_system=Restricted, network=Restricted, legacy_policy=ExternalSandbox { network_access: Restricted }; refusing to run unsandboxed".to_string() + ), + } ); } @@ -204,13 +211,17 @@ fn windows_restricted_token_runs_for_legacy_restricted_policies() { let file_system_policy = FileSystemSandboxPolicy::restricted(vec![]); assert_eq!( - should_use_windows_restricted_token_sandbox( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, + NetworkSandboxPolicy::Restricted, ), - true + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + } ); } @@ -222,17 +233,20 @@ fn windows_restricted_token_rejects_network_only_restrictions() { let file_system_policy = FileSystemSandboxPolicy::unrestricted(); assert_eq!( - unsupported_windows_restricted_token_sandbox_reason( - SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, - &policy, - &file_system_policy, - NetworkSandboxPolicy::Restricted, - ), - Some( + windows_restricted_token_sandbox_support( + SandboxType::WindowsRestrictedToken, + WindowsSandboxLevel::Disabled, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ), + WindowsRestrictedTokenSandboxSupport { + should_use: false, + unsupported_reason: Some( "windows sandbox backend cannot enforce file_system=Unrestricted, network=Restricted, legacy_policy=ExternalSandbox { network_access: Restricted }; refusing to run unsandboxed".to_string() - ) - ); + ), + } + ); } #[test] @@ -241,14 +255,17 @@ fn windows_restricted_token_allows_legacy_restricted_policies() { let file_system_policy = FileSystemSandboxPolicy::restricted(vec![]); assert_eq!( - unsupported_windows_restricted_token_sandbox_reason( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, ), - None + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + } ); } @@ -264,25 +281,20 @@ fn windows_restricted_token_rejects_restricted_read_only_policies() { let file_system_policy = FileSystemSandboxPolicy::from(&policy); assert_eq!( - should_use_windows_restricted_token_sandbox( - SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, - &policy, - &file_system_policy, - ), - false - ); - assert_eq!( - unsupported_windows_restricted_token_sandbox_reason( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, ), - Some( - "windows sandbox backend cannot enforce file_system=Restricted, network=Restricted, legacy_policy=ReadOnly { access: Restricted { include_platform_defaults: true, readable_roots: [] }, network_access: false }; refusing to run unsandboxed".to_string() - ) + WindowsRestrictedTokenSandboxSupport { + should_use: false, + unsupported_reason: Some( + "windows sandbox backend cannot enforce file_system=Restricted, network=Restricted, legacy_policy=ReadOnly { access: Restricted { include_platform_defaults: true, readable_roots: [] }, network_access: false }; refusing to run unsandboxed".to_string() + ), + }, + "restricted-token should fail closed for restricted read-only policies" ); } @@ -298,14 +310,17 @@ fn windows_restricted_token_allows_legacy_workspace_write_policies() { let file_system_policy = FileSystemSandboxPolicy::from(&policy); assert_eq!( - unsupported_windows_restricted_token_sandbox_reason( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, ), - None + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + } ); } @@ -321,23 +336,18 @@ fn windows_elevated_sandbox_allows_restricted_read_only_policies() { let file_system_policy = FileSystemSandboxPolicy::from(&policy); assert_eq!( - should_use_windows_restricted_token_sandbox( - SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Elevated, - &policy, - &file_system_policy, - ), - true - ); - assert_eq!( - unsupported_windows_restricted_token_sandbox_reason( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, - codex_protocol::config_types::WindowsSandboxLevel::Elevated, + WindowsSandboxLevel::Elevated, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, ), - None + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + }, + "elevated Windows sandbox should keep restricted read-only support enabled" ); } @@ -349,7 +359,7 @@ fn process_exec_tool_call_uses_platform_sandbox_for_network_only_restrictions() select_process_exec_tool_sandbox_type( &FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Restricted, - codex_protocol::config_types::WindowsSandboxLevel::Disabled, + WindowsSandboxLevel::Disabled, false, ), expected @@ -389,7 +399,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()> env, network: None, sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, + windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, justification: None, arg0: None, @@ -446,7 +456,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> { env, network: None, sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, + windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, justification: None, arg0: None,