diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 77966ea88c6..3558fd9f609 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -65,6 +65,30 @@ 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. + +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`, 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 +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/core/src/exec.rs b/codex-rs/core/src/exec.rs index 4f462821e5c..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,12 +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; @@ -817,41 +820,56 @@ async fn exec( } #[cfg_attr(not(target_os = "windows"), allow(dead_code))] -fn should_use_windows_restricted_token_sandbox( - sandbox: SandboxType, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, -) -> bool { - sandbox == SandboxType::WindowsRestrictedToken - && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted - && !matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } - ) +#[derive(Debug, PartialEq, Eq)] +struct WindowsRestrictedTokenSandboxSupport { + should_use: bool, + unsupported_reason: Option, } #[cfg(any(target_os = "windows", test))] -fn unsupported_windows_restricted_token_sandbox_reason( +fn windows_restricted_token_sandbox_support( 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, - sandbox_policy, - file_system_sandbox_policy, - ) { - return None; +) -> WindowsRestrictedTokenSandboxSupport { + if sandbox != SandboxType::WindowsRestrictedToken { + return WindowsRestrictedTokenSandboxSupport { + should_use: false, + unsupported_reason: None, + }; } - (sandbox == SandboxType::WindowsRestrictedToken).then(|| { - format!( + // 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. + let should_use = 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()); + + 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 10ba5734faf..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,12 +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, + 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() + ), + } ); } @@ -203,12 +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, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, + NetworkSandboxPolicy::Restricted, ), - true + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + } ); } @@ -220,16 +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, - &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] @@ -238,13 +255,46 @@ 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, + WindowsSandboxLevel::Disabled, &policy, &file_system_policy, NetworkSandboxPolicy::Restricted, ), - None + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: None, + } + ); +} + +#[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!( + 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=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" ); } @@ -260,13 +310,44 @@ 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, + WindowsSandboxLevel::Disabled, + &policy, + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ), + WindowsRestrictedTokenSandboxSupport { + should_use: true, + unsupported_reason: 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!( + windows_restricted_token_sandbox_support( SandboxType::WindowsRestrictedToken, + 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" ); } @@ -278,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 @@ -318,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, @@ -375,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, 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..cb4be527500 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -291,7 +291,7 @@ mod windows_impl { } if !policy.has_full_disk_read_access() { anyhow::bail!( - "Restricted read-only access is not yet supported by the Windows sandbox backend" + "Restricted read-only access requires the elevated Windows sandbox backend" ); } let caps = load_or_create_cap_sids(codex_home)?; 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))); + } }