From c033b46eaadab7ae139928fbb5e5db03321c4cf1 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Tue, 12 May 2026 10:14:41 -0700 Subject: [PATCH] Fix Windows elevated unified-exec filesystem overrides --- codex-rs/cli/src/debug_sandbox.rs | 11 +- codex-rs/core/src/exec.rs | 159 +++++++++--------- .../core/src/unified_exec/process_manager.rs | 152 +++++++++-------- codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 60 ++----- .../src/unified_exec/backends/elevated.rs | 21 +-- .../src/unified_exec/session.rs | 92 ++++++++++ 6 files changed, 275 insertions(+), 220 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/unified_exec/session.rs diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 2722f69849..d7ddef0ed6 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -370,14 +370,13 @@ async fn run_command_under_windows_session( cwd.as_path(), env, None, - /*read_roots_override*/ None, - /*read_roots_include_platform_defaults*/ false, - /*write_roots_override*/ None, - /*deny_read_paths_override*/ &[], - /*deny_write_paths_override*/ &[], /*tty*/ false, /*stdin_open*/ true, config.permissions.windows_sandbox_private_desktop, + None, + false, + None, + &[], ) .await } else { @@ -389,8 +388,6 @@ async fn run_command_under_windows_session( cwd.as_path(), env, None, - /*additional_deny_read_paths*/ &[], - /*additional_deny_write_paths*/ &[], /*tty*/ false, /*stdin_open*/ true, config.permissions.windows_sandbox_private_desktop, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index a6d958b107..56cf0209a9 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -97,19 +97,17 @@ pub struct ExecParams { /// Resolved filesystem overrides for the Windows sandbox backends. /// -/// The elevated Windows backend consumes extra deny-read paths plus explicit -/// read and write roots during setup/refresh. The unelevated restricted-token -/// backend only consumes extra deny-write carveouts on top of the legacy -/// `WorkspaceWrite` allow set. Read-root overrides are layered on top of the -/// baseline helper roots that the elevated setup path needs to launch the -/// sandboxed command; split policies that opt into platform defaults carry -/// that explicitly with the override. +/// The unelevated restricted-token backend only consumes extra deny-write +/// carveouts on top of the legacy `WorkspaceWrite` allow set. The elevated +/// backend can also consume explicit read and write roots during setup/refresh. +/// Read-root overrides are layered on top of the baseline helper roots that the +/// elevated setup path needs to launch the sandboxed command. Split policies +/// that opt into platform defaults carry that explicitly with the override. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct WindowsSandboxFilesystemOverrides { pub(crate) read_roots_override: Option>, pub(crate) read_roots_include_platform_defaults: bool, pub(crate) write_roots_override: Option>, - pub(crate) additional_deny_read_paths: Vec, pub(crate) additional_deny_write_paths: Vec, } @@ -123,6 +121,35 @@ fn windows_sandbox_uses_elevated_backend( proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated) } +pub(crate) fn resolve_windows_filesystem_overrides_for_exec_request( + exec_request: &ExecRequest, +) -> std::result::Result, String> { + let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend( + exec_request.windows_sandbox_level, + exec_request.network.is_some(), + ); + let sandbox_policy = exec_request.compatibility_sandbox_policy(); + if use_windows_elevated_backend { + resolve_windows_elevated_filesystem_overrides( + exec_request.sandbox, + &sandbox_policy, + &exec_request.file_system_sandbox_policy, + exec_request.network_sandbox_policy, + &exec_request.windows_sandbox_policy_cwd, + use_windows_elevated_backend, + ) + } else { + resolve_windows_restricted_token_filesystem_overrides( + exec_request.sandbox, + &sandbox_policy, + &exec_request.file_system_sandbox_policy, + exec_request.network_sandbox_policy, + &exec_request.windows_sandbox_policy_cwd, + exec_request.windows_sandbox_level, + ) + } +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum ExecCapturePolicy { /// Shell-like execs keep the historical output cap and timeout behavior. @@ -390,31 +417,10 @@ pub fn build_exec_request( ExecRequest::from_sandbox_exec_request(request, options, windows_sandbox_policy_cwd) }) .map_err(CodexErr::from)?; - let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend( - exec_req.windows_sandbox_level, - exec_req.network.is_some(), - ); - let sandbox_policy = exec_req.compatibility_sandbox_policy(); - exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend { - resolve_windows_elevated_filesystem_overrides( - exec_req.sandbox, - &sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - use_windows_elevated_backend, - ) - } else { - resolve_windows_restricted_token_filesystem_overrides( - exec_req.sandbox, - &sandbox_policy, - &exec_req.file_system_sandbox_policy, - exec_req.network_sandbox_policy, - sandbox_cwd, - exec_req.windows_sandbox_level, - ) - } - .map_err(CodexErr::UnsupportedOperation)?; + exec_req.windows_sandbox_policy_cwd = sandbox_cwd.clone(); + exec_req.windows_sandbox_filesystem_overrides = + resolve_windows_filesystem_overrides_for_exec_request(&exec_req) + .map_err(CodexErr::UnsupportedOperation)?; Ok(exec_req) } @@ -566,7 +572,7 @@ async fn exec_windows_sandbox( ) -> Result { use crate::config::find_codex_home; use codex_windows_sandbox::run_windows_sandbox_capture_elevated; - use codex_windows_sandbox::run_windows_sandbox_capture_with_filesystem_overrides; + use codex_windows_sandbox::run_windows_sandbox_capture_with_extra_deny_write_paths; let ExecParams { command, @@ -607,10 +613,13 @@ async fn exec_windows_sandbox( let proxy_enforced = network.is_some(); let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced); let additional_deny_write_paths = windows_sandbox_filesystem_overrides - .map(|overrides| overrides.additional_deny_write_paths.clone()) - .unwrap_or_default(); - let additional_deny_read_paths = windows_sandbox_filesystem_overrides - .map(|overrides| overrides.additional_deny_read_paths.clone()) + .map(|overrides| { + overrides + .additional_deny_write_paths + .iter() + .map(AbsolutePathBuf::to_path_buf) + .collect::>() + }) .unwrap_or_default(); let elevated_read_roots_override = windows_sandbox_filesystem_overrides .and_then(|overrides| overrides.read_roots_override.clone()); @@ -618,6 +627,15 @@ async fn exec_windows_sandbox( .is_some_and(|overrides| overrides.read_roots_include_platform_defaults); let elevated_write_roots_override = windows_sandbox_filesystem_overrides .and_then(|overrides| overrides.write_roots_override.clone()); + let elevated_deny_write_paths = windows_sandbox_filesystem_overrides + .map(|overrides| { + overrides + .additional_deny_write_paths + .iter() + .map(AbsolutePathBuf::to_path_buf) + .collect::>() + }) + .unwrap_or_default(); let spawn_res = tokio::task::spawn_blocking(move || { if use_elevated { run_windows_sandbox_capture_elevated( @@ -635,12 +653,11 @@ async fn exec_windows_sandbox( read_roots_include_platform_defaults: elevated_read_roots_include_platform_defaults, write_roots_override: elevated_write_roots_override.as_deref(), - deny_read_paths_override: &additional_deny_read_paths, - deny_write_paths_override: &additional_deny_write_paths, + deny_write_paths_override: &elevated_deny_write_paths, }, ) } else { - run_windows_sandbox_capture_with_filesystem_overrides( + run_windows_sandbox_capture_with_extra_deny_write_paths( policy_str.as_str(), &sandbox_cwd, codex_home.as_ref(), @@ -648,7 +665,6 @@ async fn exec_windows_sandbox( &cwd, env, timeout_ms, - &additional_deny_read_paths, &additional_deny_write_paths, windows_sandbox_private_desktop, ) @@ -1041,24 +1057,22 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( )); } - // The restricted-token backend can still enforce split write restrictions, - // but its WRITE_RESTRICTED token does not make capability SID deny-read ACEs - // participate in read access checks. Read restrictions therefore require the - // elevated backend, even when the filesystem root remains readable. - if !windows_policy_has_root_read_access(file_system_sandbox_policy, sandbox_policy_cwd) { + if !file_system_sandbox_policy.has_full_disk_read_access() { return Err( "windows unelevated restricted-token sandbox cannot enforce split filesystem read restrictions directly; refusing to run unsandboxed" .to_string(), ); } - let additional_deny_read_paths = codex_windows_sandbox::resolve_windows_deny_read_paths( - file_system_sandbox_policy, - sandbox_policy_cwd, - )?; - if !additional_deny_read_paths.is_empty() { + if !file_system_sandbox_policy + .get_unreadable_roots_with_cwd(sandbox_policy_cwd) + .is_empty() + || !file_system_sandbox_policy + .get_unreadable_globs_with_cwd(sandbox_policy_cwd) + .is_empty() + { return Err( - "windows unelevated restricted-token sandbox cannot enforce deny-read restrictions directly; refusing to run unsandboxed" + "windows unelevated restricted-token sandbox cannot enforce unreadable split filesystem carveouts directly; refusing to run unsandboxed" .to_string(), ); } @@ -1125,7 +1139,7 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( } } - if additional_deny_read_paths.is_empty() && additional_deny_write_paths.is_empty() { + if additional_deny_write_paths.is_empty() { return Ok(None); } @@ -1133,7 +1147,6 @@ pub(crate) fn resolve_windows_restricted_token_filesystem_overrides( read_roots_override: None, read_roots_include_platform_defaults: false, write_roots_override: None, - additional_deny_read_paths, additional_deny_write_paths: additional_deny_write_paths .into_iter() .map(|path| AbsolutePathBuf::from_absolute_path(path).map_err(|err| err.to_string())) @@ -1147,16 +1160,6 @@ fn normalize_windows_override_path(path: &Path) -> std::result::Result bool { - let Some(root) = cwd.as_path().ancestors().last() else { - return false; - }; - file_system_sandbox_policy.can_read_path_with_cwd(root, cwd.as_path()) -} - pub(crate) fn resolve_windows_elevated_filesystem_overrides( sandbox: SandboxType, sandbox_policy: &SandboxPolicy, @@ -1180,10 +1183,18 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( )); } - let additional_deny_read_paths = codex_windows_sandbox::resolve_windows_deny_read_paths( - file_system_sandbox_policy, - sandbox_policy_cwd, - )?; + if !file_system_sandbox_policy + .get_unreadable_roots_with_cwd(sandbox_policy_cwd) + .is_empty() + || !file_system_sandbox_policy + .get_unreadable_globs_with_cwd(sandbox_policy_cwd) + .is_empty() + { + return Err( + "windows elevated sandbox cannot enforce unreadable split filesystem carveouts directly; refusing to run unsandboxed" + .to_string(), + ); + } let split_writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); @@ -1214,13 +1225,7 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( .collect(); let split_root_path_set: BTreeSet = split_root_paths.iter().cloned().collect(); - // `has_full_disk_read_access()` is intentionally false when deny-read - // entries exist. For Windows setup overrides, the important question is - // whether the baseline still reads from the filesystem root and only needs - // additional deny ACLs layered on top. - let split_has_root_read_access = - windows_policy_has_root_read_access(file_system_sandbox_policy, sandbox_policy_cwd); - let read_roots_override = if split_has_root_read_access { + let read_roots_override = if file_system_sandbox_policy.has_full_disk_read_access() { None } else { Some(split_readable_roots) @@ -1268,7 +1273,6 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( if read_roots_override.is_none() && write_roots_override.is_none() - && additional_deny_read_paths.is_empty() && additional_deny_write_paths.is_empty() { return Ok(None); @@ -1279,7 +1283,6 @@ pub(crate) fn resolve_windows_elevated_filesystem_overrides( && file_system_sandbox_policy.include_platform_defaults(), read_roots_override, write_roots_override, - additional_deny_read_paths, additional_deny_write_paths, })) } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 3032bd63f3..c0f1ddce04 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -37,6 +37,7 @@ use crate::unified_exec::ProcessStore; use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcessManager; +use crate::unified_exec::WARNING_UNIFIED_EXEC_PROCESSES; use crate::unified_exec::WriteStdinRequest; use crate::unified_exec::async_watcher::emit_exec_end_for_unified_exec; use crate::unified_exec::async_watcher::emit_failed_exec_end_for_unified_exec; @@ -53,7 +54,6 @@ use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; use codex_protocol::protocol::ExecCommandSource; -use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; @@ -371,7 +371,10 @@ impl UnifiedExecProcessManager { request: ExecCommandRequest, context: &UnifiedExecContext, ) -> Result { - let cwd = request.cwd.clone(); + let cwd = request + .workdir + .clone() + .unwrap_or_else(|| context.turn.cwd.clone()); let process = self .open_session_with_sandbox(&request, cwd.clone(), context) .await; @@ -826,11 +829,11 @@ impl UnifiedExecProcessManager { session: Arc::downgrade(&context.session), last_used: started_at, }; - let pruned_entry = { + let (number_processes, pruned_entry) = { let mut store = self.process_store.lock().await; let pruned_entry = Self::prune_processes_if_needed(&mut store); store.processes.insert(process_id, entry); - pruned_entry + (store.processes.len(), pruned_entry) }; // prune_processes_if_needed runs while holding process_store; do async // network-approval cleanup only after dropping that lock. @@ -839,6 +842,16 @@ impl UnifiedExecProcessManager { pruned_entry.process.terminate(); } + if number_processes >= WARNING_UNIFIED_EXEC_PROCESSES { + context + .session + .record_model_warning( + format!("The maximum number of unified exec processes you can keep open is {WARNING_UNIFIED_EXEC_PROCESSES} and you currently have {number_processes} processes open. Reuse older processes or close them to prevent automatic pruning of old processes"), + &context.turn + ) + .await; + }; + spawn_exit_watcher( Arc::clone(&process), Arc::clone(&context.session), @@ -875,68 +888,67 @@ impl UnifiedExecProcessManager { "windows sandbox: failed to resolve codex_home: {err}" )) })?; - let additional_deny_write_paths = request - .windows_sandbox_filesystem_overrides - .as_ref() - .map(|overrides| overrides.additional_deny_write_paths.clone()) - .unwrap_or_default(); - let additional_deny_read_paths = request - .windows_sandbox_filesystem_overrides - .as_ref() - .map(|overrides| overrides.additional_deny_read_paths.clone()) - .unwrap_or_default(); - let elevated_read_roots_override = request - .windows_sandbox_filesystem_overrides - .as_ref() - .and_then(|overrides| overrides.read_roots_override.clone()); - let elevated_read_roots_include_platform_defaults = request - .windows_sandbox_filesystem_overrides - .as_ref() - .is_some_and(|overrides| overrides.read_roots_include_platform_defaults); - let elevated_write_roots_override = request - .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( - policy_json.as_str(), - request.windows_sandbox_policy_cwd.as_path(), - 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( - policy_json.as_str(), - request.windows_sandbox_policy_cwd.as_path(), - 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 windows_sandbox_filesystem_overrides = + crate::exec::resolve_windows_filesystem_overrides_for_exec_request(request) + .map_err(UnifiedExecError::create_process)?; + let spawned = + match request.windows_sandbox_level { + codex_protocol::config_types::WindowsSandboxLevel::Elevated => { + let elevated_read_roots_override = windows_sandbox_filesystem_overrides + .as_ref() + .and_then(|overrides| overrides.read_roots_override.clone()); + let elevated_read_roots_include_platform_defaults = + windows_sandbox_filesystem_overrides.as_ref().is_some_and( + |overrides| overrides.read_roots_include_platform_defaults, + ); + let elevated_write_roots_override = windows_sandbox_filesystem_overrides + .as_ref() + .and_then(|overrides| overrides.write_roots_override.clone()); + let elevated_deny_write_paths = windows_sandbox_filesystem_overrides + .as_ref() + .map(|overrides| { + overrides + .additional_deny_write_paths + .iter() + .map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf) + .collect::>() + }) + .unwrap_or_default(); + codex_windows_sandbox::spawn_windows_sandbox_session_elevated( + policy_json.as_str(), + request.windows_sandbox_policy_cwd.as_path(), + codex_home.as_ref(), + request.command.clone(), + request.cwd.as_path(), + request.env.clone(), + None, + tty, + tty, + request.windows_sandbox_private_desktop, + elevated_read_roots_override.as_deref(), + elevated_read_roots_include_platform_defaults, + elevated_write_roots_override.as_deref(), + &elevated_deny_write_paths, + ) + .await + } + codex_protocol::config_types::WindowsSandboxLevel::RestrictedToken + | codex_protocol::config_types::WindowsSandboxLevel::Disabled => { + codex_windows_sandbox::spawn_windows_sandbox_session_legacy( + policy_json.as_str(), + request.windows_sandbox_policy_cwd.as_path(), + codex_home.as_ref(), + request.command.clone(), + request.cwd.as_path(), + request.env.clone(), + None, + tty, + tty, + request.windows_sandbox_private_desktop, + ) + .await + } + }; spawn_lifecycle.after_spawn(); return UnifiedExecProcess::from_spawned( spawned.map_err(|err| UnifiedExecError::create_process(err.to_string()))?, @@ -1028,9 +1040,7 @@ impl UnifiedExecProcessManager { approval_policy: context.turn.approval_policy.value(), permission_profile: context.turn.permission_profile(), file_system_sandbox_policy: &file_system_sandbox_policy, - // The process cwd may be model-controlled. Policy resolution - // stays anchored to the selected turn environment cwd instead. - sandbox_cwd: request.sandbox_cwd.as_path(), + sandbox_cwd: context.turn.cwd.as_path(), sandbox_permissions: if request.additional_permissions_preapproved { crate::sandboxing::SandboxPermissions::UseDefault } else { @@ -1044,8 +1054,6 @@ impl UnifiedExecProcessManager { hook_command: request.hook_command.clone(), process_id: request.process_id, cwd, - sandbox_cwd: request.sandbox_cwd.clone(), - environment: Arc::clone(&request.environment), env, exec_server_env_config: Some(exec_server_env_config), explicit_env_overrides: context.turn.shell_environment_policy.r#set.clone(), @@ -1062,7 +1070,7 @@ impl UnifiedExecProcessManager { session: context.session.clone(), turn: context.turn.clone(), call_id: context.call_id.clone(), - tool_name: ToolName::plain("exec_command"), + tool_name: "exec_command".to_string(), }; orchestrator .run( diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index b0cf0f3075..32cdf53278 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -5,8 +5,6 @@ use crate::allow::AllowDenyPaths; use crate::allow::compute_allow_paths; use crate::cap::load_or_create_cap_sids; use crate::cap::workspace_cap_sid_for_cwd; -use crate::deny_read_acl::apply_deny_read_acls; -use crate::deny_read_state::sync_persistent_deny_read_acls; use crate::env::apply_no_network_to_env; use crate::env::ensure_non_interactive_pager; use crate::env::inherit_path_env; @@ -27,7 +25,6 @@ use crate::token::get_logon_sid_bytes; use crate::workspace_acl::is_command_cwd_root; use crate::workspace_acl::protect_workspace_agents_dir; use crate::workspace_acl::protect_workspace_codex_dir; -use anyhow::Context; use anyhow::Result; use std::collections::HashMap; use std::ffi::c_void; @@ -214,34 +211,20 @@ pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) { } } -#[allow(clippy::too_many_arguments)] pub(crate) fn apply_legacy_session_acl_rules( policy: &SandboxPolicy, sandbox_policy_cwd: &Path, - codex_home: &Path, current_dir: &Path, env_map: &HashMap, psid_generic: &LocalSid, psid_workspace: Option<&LocalSid>, - cap_sid_str: &str, - additional_deny_read_paths: &[PathBuf], - additional_deny_write_paths: &[PathBuf], persist_aces: bool, -) -> Result> { - let AllowDenyPaths { allow, mut deny } = +) -> Vec { + let AllowDenyPaths { allow, deny } = compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map); let mut guards: Vec = Vec::new(); let canonical_cwd = canonicalize_path(current_dir); unsafe { - for path in additional_deny_write_paths { - // Explicit carveouts must exist before the command starts so the - // sandbox cannot create them under a writable parent first. - if !path.exists() { - std::fs::create_dir_all(path) - .with_context(|| format!("create deny-write path {}", path.display()))?; - } - deny.insert(path.clone()); - } for p in &allow { let psid = if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) && is_command_cwd_root(p, &canonical_cwd) @@ -262,19 +245,6 @@ pub(crate) fn apply_legacy_session_acl_rules( guards.push(p.clone()); } } - let applied_deny_read_paths = if persist_aces { - sync_persistent_deny_read_acls( - codex_home, - cap_sid_str, - additional_deny_read_paths, - psid_generic.as_ptr(), - )? - } else { - apply_deny_read_acls(additional_deny_read_paths, psid_generic.as_ptr())? - }; - if !persist_aces { - guards.extend(applied_deny_read_paths); - } allow_null_device(psid_generic.as_ptr()); if let Some(psid_workspace) = psid_workspace { allow_null_device(psid_workspace.as_ptr()); @@ -284,10 +254,9 @@ pub(crate) fn apply_legacy_session_acl_rules( } } } - Ok(guards) + guards } -#[allow(clippy::too_many_arguments)] pub(crate) fn prepare_elevated_spawn_context( policy_json_or_preset: &str, sandbox_policy_cwd: &Path, @@ -298,7 +267,6 @@ pub(crate) fn prepare_elevated_spawn_context( read_roots_override: Option<&[PathBuf]>, read_roots_include_platform_defaults: bool, write_roots_override: Option<&[PathBuf]>, - deny_read_paths_override: &[PathBuf], deny_write_paths_override: &[PathBuf], ) -> Result { let common = prepare_spawn_context_common( @@ -317,12 +285,15 @@ pub(crate) fn prepare_elevated_spawn_context( &common.current_dir, env_map, ); - let write_roots: Vec = allow.into_iter().collect(); - let deny_write_paths: Vec = deny.into_iter().collect(); - let computed_write_roots_override = if common.is_workspace_write { - Some(write_roots.as_slice()) + let computed_write_roots: Vec = allow.into_iter().collect(); + let computed_deny_write_paths: Vec = deny.into_iter().collect(); + let effective_write_roots = write_roots_override + .map(<[PathBuf]>::to_vec) + .or_else(|| common.is_workspace_write.then_some(computed_write_roots)); + let effective_deny_write_paths = if deny_write_paths_override.is_empty() { + computed_deny_write_paths } else { - None + deny_write_paths_override.to_vec() }; let sandbox_creds = require_logon_sandbox_creds( &common.policy, @@ -332,13 +303,8 @@ pub(crate) fn prepare_elevated_spawn_context( codex_home, read_roots_override, read_roots_include_platform_defaults, - write_roots_override.or(computed_write_roots_override), - deny_read_paths_override, - if deny_write_paths_override.is_empty() { - &deny_write_paths - } else { - deny_write_paths_override - }, + effective_write_roots.as_deref(), + &effective_deny_write_paths, /*proxy_enforced*/ false, )?; let caps = load_or_create_cap_sids(codex_home)?; 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 0d7e94a1ff..c958bd6163 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 @@ -10,7 +10,6 @@ use crate::ipc_framed::SpawnRequest; use crate::runner_client::spawn_runner_transport; use crate::spawn_prep::prepare_elevated_spawn_context; use anyhow::Result; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; use std::collections::HashMap; @@ -29,23 +28,14 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated( cwd: &Path, mut env_map: HashMap, timeout_ms: Option, - read_roots_override: Option<&[PathBuf]>, - read_roots_include_platform_defaults: bool, - write_roots_override: Option<&[PathBuf]>, - deny_read_paths_override: &[AbsolutePathBuf], - deny_write_paths_override: &[AbsolutePathBuf], tty: bool, stdin_open: bool, use_private_desktop: bool, + read_roots_override: Option<&[PathBuf]>, + read_roots_include_platform_defaults: bool, + write_roots_override: Option<&[PathBuf]>, + deny_write_paths_override: &[PathBuf], ) -> Result { - let deny_read_paths_override = deny_read_paths_override - .iter() - .map(AbsolutePathBuf::to_path_buf) - .collect::>(); - let deny_write_paths_override = deny_write_paths_override - .iter() - .map(AbsolutePathBuf::to_path_buf) - .collect::>(); let elevated = prepare_elevated_spawn_context( policy_json_or_preset, sandbox_policy_cwd, @@ -56,8 +46,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated( read_roots_override, read_roots_include_platform_defaults, write_roots_override, - &deny_read_paths_override, - &deny_write_paths_override, + deny_write_paths_override, )?; let spawn_request = SpawnRequest { diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/session.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/session.rs new file mode 100644 index 0000000000..d4252d6ab3 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/session.rs @@ -0,0 +1,92 @@ +//! Unified exec session spawner for Windows sandboxing. +//! +//! This module is the thin orchestration layer for Windows unified-exec sessions. +//! Backend-specific mechanics live in sibling modules: +//! - `backends::legacy` adapts the direct restricted-token spawn path into a live session. +//! - `backends::elevated` adapts the elevated command-runner IPC path into the same session API. +//! - `backends::windows_common` holds the small shared Windows backend helpers +//! used by both. + +mod backends; + +use anyhow::Result; +use codex_utils_pty::SpawnedProcess; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; + +#[allow(clippy::too_many_arguments)] +pub async fn spawn_windows_sandbox_session_legacy( + policy_json_or_preset: &str, + sandbox_policy_cwd: &Path, + codex_home: &Path, + command: Vec, + cwd: &Path, + env_map: HashMap, + timeout_ms: Option, + tty: bool, + stdin_open: bool, + use_private_desktop: bool, +) -> Result { + backends::legacy::spawn_windows_sandbox_session_legacy( + policy_json_or_preset, + sandbox_policy_cwd, + codex_home, + command, + cwd, + env_map, + timeout_ms, + tty, + stdin_open, + use_private_desktop, + ) + .await +} + +#[allow(clippy::too_many_arguments)] +pub async fn spawn_windows_sandbox_session_elevated( + policy_json_or_preset: &str, + sandbox_policy_cwd: &Path, + codex_home: &Path, + command: Vec, + cwd: &Path, + env_map: HashMap, + timeout_ms: Option, + tty: bool, + stdin_open: bool, + use_private_desktop: bool, + read_roots_override: Option<&[PathBuf]>, + read_roots_include_platform_defaults: bool, + write_roots_override: Option<&[PathBuf]>, + deny_write_paths_override: &[PathBuf], +) -> Result { + backends::elevated::spawn_windows_sandbox_session_elevated( + policy_json_or_preset, + sandbox_policy_cwd, + codex_home, + command, + cwd, + env_map, + timeout_ms, + tty, + stdin_open, + use_private_desktop, + read_roots_override, + read_roots_include_platform_defaults, + write_roots_override, + deny_write_paths_override, + ) + .await +} + +#[cfg(test)] +pub(crate) use backends::windows_common::finish_driver_spawn; +#[cfg(test)] +pub(crate) use backends::windows_common::make_runner_resizer; +#[cfg(test)] +pub(crate) use backends::windows_common::start_runner_pipe_writer; +#[cfg(test)] +pub(crate) use backends::windows_common::start_runner_stdin_writer; + +#[cfg(test)] +mod tests;