From 8ec36fc336a7c1e7b6cc62068b99617b8edc715c Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Mon, 6 Oct 2025 15:25:25 -0700 Subject: [PATCH 1/8] adds scaffold for windows sandbox with experimental flag. Does nothing yet. --- codex-rs/core/src/config.rs | 13 +++++++++++++ codex-rs/core/src/exec.rs | 12 ++++++++++-- codex-rs/core/src/safety.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 28ad84ba7a..42dfe71e06 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -26,6 +26,7 @@ use crate::model_provider_info::built_in_model_providers; use crate::openai_model_info::get_model_info; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +use crate::safety::set_windows_sandbox_enabled; use anyhow::Context; use codex_app_server_protocol::Tools; use codex_app_server_protocol::UserSavedConfig; @@ -170,6 +171,9 @@ pub struct Config { /// When this program is invoked, arg0 will be set to `codex-linux-sandbox`. pub codex_linux_sandbox_exe: Option, + /// Enable the experimental Windows sandbox implementation. + pub experimental_windows_sandbox: bool, + /// Value to use for `reasoning.effort` when making a request using the /// Responses API. pub model_reasoning_effort: Option, @@ -751,6 +755,7 @@ pub struct ConfigToml { pub experimental_use_unified_exec_tool: Option, pub experimental_use_rmcp_client: Option, pub experimental_use_freeform_apply_patch: Option, + pub experimental_windows_sandbox: Option, pub projects: Option>, @@ -1008,6 +1013,8 @@ impl Config { .or(cfg.tools.as_ref().and_then(|t| t.view_image)) .unwrap_or(true); + let experimental_windows_sandbox = cfg.experimental_windows_sandbox.unwrap_or(false); + let model = model .or(config_profile.model) .or(cfg.model) @@ -1093,6 +1100,7 @@ impl Config { history, file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), codex_linux_sandbox_exe, + experimental_windows_sandbox, hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false), show_raw_agent_reasoning: cfg @@ -1146,6 +1154,7 @@ impl Config { } }, }; + set_windows_sandbox_enabled(config.experimental_windows_sandbox); Ok(config) } @@ -1903,6 +1912,7 @@ model_verbosity = "high" history: History::default(), file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, + experimental_windows_sandbox: false, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: Some(ReasoningEffort::High), @@ -1965,6 +1975,7 @@ model_verbosity = "high" history: History::default(), file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, + experimental_windows_sandbox: false, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: None, @@ -2042,6 +2053,7 @@ model_verbosity = "high" history: History::default(), file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, + experimental_windows_sandbox: false, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: None, @@ -2105,6 +2117,7 @@ model_verbosity = "high" history: History::default(), file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, + experimental_windows_sandbox: false, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: Some(ReasoningEffort::High), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index d84bbc9fcb..d768ca42d8 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -70,6 +70,9 @@ pub enum SandboxType { /// Only available on Linux. LinuxSeccomp, + + /// Only available on Windows. + WindowsAppContainer, } #[derive(Clone)] @@ -93,7 +96,9 @@ pub async fn process_exec_tool_call( let raw_output_result: std::result::Result = match sandbox_type { - SandboxType::None => exec(params, sandbox_policy, stdout_stream.clone()).await, + SandboxType::None | SandboxType::WindowsAppContainer => { + exec(params, sandbox_policy, stdout_stream.clone()).await + } SandboxType::MacosSeatbelt => { let ExecParams { command, @@ -198,7 +203,10 @@ pub async fn process_exec_tool_call( /// For now, we conservatively check for 'command not found' (exit code 127), /// and can add additional cases as necessary. fn is_likely_sandbox_denied(sandbox_type: SandboxType, exit_code: i32) -> bool { - if sandbox_type == SandboxType::None { + if matches!( + sandbox_type, + SandboxType::None | SandboxType::WindowsAppContainer + ) { return false; } diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 0ed0f929ff..92b348d5e0 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -2,6 +2,8 @@ use std::collections::HashSet; use std::path::Component; use std::path::Path; use std::path::PathBuf; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; @@ -13,6 +15,12 @@ use crate::command_safety::is_safe_command::is_known_safe_command; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +static WINDOWS_SANDBOX_ENABLED: AtomicBool = AtomicBool::new(false); + +pub(crate) fn set_windows_sandbox_enabled(enabled: bool) { + WINDOWS_SANDBOX_ENABLED.store(enabled, Ordering::Relaxed); +} + #[derive(Debug, PartialEq)] pub enum SafetyCheck { AutoApprove { @@ -206,6 +214,12 @@ pub fn get_platform_sandbox() -> Option { Some(SandboxType::MacosSeatbelt) } else if cfg!(target_os = "linux") { Some(SandboxType::LinuxSeccomp) + } else if cfg!(target_os = "windows") { + if WINDOWS_SANDBOX_ENABLED.load(Ordering::Relaxed) { + Some(SandboxType::WindowsAppContainer) + } else { + None + } } else { None } @@ -436,4 +450,19 @@ mod tests { }; assert_eq!(safety_check, expected); } + + #[cfg(target_os = "windows")] + #[test] + fn windows_sandbox_toggle_controls_platform_sandbox() { + set_windows_sandbox_enabled(false); + assert_eq!(get_platform_sandbox(), None); + + set_windows_sandbox_enabled(true); + assert_eq!( + get_platform_sandbox(), + Some(SandboxType::WindowsAppContainer) + ); + + set_windows_sandbox_enabled(false); + } } From 7ec9444fb68749aad1cc5c61b7c00af2ac78bbee Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 11:44:51 -0700 Subject: [PATCH 2/8] initial implementation of sandboxing using AppContainer. --- codex-rs/Cargo.lock | 89 +++- codex-rs/core/Cargo.toml | 10 + codex-rs/core/src/exec.rs | 29 +- codex-rs/core/src/lib.rs | 3 + codex-rs/core/src/windows_appcontainer.rs | 472 ++++++++++++++++++++ codex-rs/core/tests/windows_appcontainer.rs | 76 ++++ codex-rs/exec/tests/windows_sandbox.rs | 74 +++ 7 files changed, 739 insertions(+), 14 deletions(-) create mode 100644 codex-rs/core/src/windows_appcontainer.rs create mode 100644 codex-rs/core/tests/windows_appcontainer.rs create mode 100644 codex-rs/exec/tests/windows_sandbox.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 350773a3c5..61389aa8c5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1088,6 +1088,7 @@ dependencies = [ "walkdir", "which", "wildmatch", + "windows 0.58.0", "wiremock", ] @@ -2818,7 +2819,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core", + "windows-core 0.61.2", ] [[package]] @@ -4385,7 +4386,7 @@ dependencies = [ "nix 0.30.1", "tokio", "tracing", - "windows", + "windows 0.61.3", ] [[package]] @@ -6702,6 +6703,16 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd04d41d93c4992d421894c18c8b43496aa748dd4c081bac0dc93eb0489272b6" +dependencies = [ + "windows-core 0.58.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows" version = "0.61.3" @@ -6709,7 +6720,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893" dependencies = [ "windows-collections", - "windows-core", + "windows-core 0.61.2", "windows-future", "windows-link 0.1.3", "windows-numerics", @@ -6721,7 +6732,20 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" dependencies = [ - "windows-core", + "windows-core 0.61.2", +] + +[[package]] +name = "windows-core" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba6d44ec8c2591c134257ce647b7ea6b20335bf6379a27dac5f1641fcf59f99" +dependencies = [ + "windows-implement 0.58.0", + "windows-interface 0.58.0", + "windows-result 0.2.0", + "windows-strings 0.1.0", + "windows-targets 0.52.6", ] [[package]] @@ -6730,11 +6754,11 @@ version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" dependencies = [ - "windows-implement", - "windows-interface", + "windows-implement 0.60.0", + "windows-interface 0.59.1", "windows-link 0.1.3", - "windows-result", - "windows-strings", + "windows-result 0.3.4", + "windows-strings 0.4.2", ] [[package]] @@ -6743,11 +6767,22 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" dependencies = [ - "windows-core", + "windows-core 0.61.2", "windows-link 0.1.3", "windows-threading", ] +[[package]] +name = "windows-implement" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bbd5b46c938e506ecbce286b6628a02171d56153ba733b6c741fc627ec9579b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "windows-implement" version = "0.60.0" @@ -6759,6 +6794,17 @@ dependencies = [ "syn 2.0.104", ] +[[package]] +name = "windows-interface" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053c4c462dc91d3b1504c6fe5a726dd15e216ba718e84a0e46a88fbe5ded3515" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "windows-interface" version = "0.59.1" @@ -6788,7 +6834,7 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" dependencies = [ - "windows-core", + "windows-core 0.61.2", "windows-link 0.1.3", ] @@ -6799,8 +6845,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b8a9ed28765efc97bbc954883f4e6796c33a06546ebafacbabee9696967499e" dependencies = [ "windows-link 0.1.3", - "windows-result", - "windows-strings", + "windows-result 0.3.4", + "windows-strings 0.4.2", +] + +[[package]] +name = "windows-result" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d1043d8214f791817bab27572aaa8af63732e11bf84aa21a45a78d6c317ae0e" +dependencies = [ + "windows-targets 0.52.6", ] [[package]] @@ -6812,6 +6867,16 @@ dependencies = [ "windows-link 0.1.3", ] +[[package]] +name = "windows-strings" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" +dependencies = [ + "windows-result 0.2.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows-strings" version = "0.4.2" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 367ccbcef2..ae0927c3df 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -79,6 +79,16 @@ seccompiler = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.9" +[target.'cfg(windows)'.dependencies] +windows = { version = "0.58", features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", + "Win32_System_Memory", + "Win32_System_Threading", +] } + # Build OpenSSL from source for musl builds. [target.x86_64-unknown-linux-musl.dependencies] openssl-sys = { workspace = true, features = ["vendored"] } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index d768ca42d8..5c0d345c01 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -27,6 +27,8 @@ use crate::protocol::SandboxPolicy; use crate::seatbelt::spawn_command_under_seatbelt; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; +#[cfg(windows)] +use crate::windows_appcontainer::spawn_command_under_windows_appcontainer; const DEFAULT_TIMEOUT_MS: u64 = 10_000; @@ -96,8 +98,31 @@ pub async fn process_exec_tool_call( let raw_output_result: std::result::Result = match sandbox_type { - SandboxType::None | SandboxType::WindowsAppContainer => { - exec(params, sandbox_policy, stdout_stream.clone()).await + SandboxType::None => exec(params, sandbox_policy, stdout_stream.clone()).await, + SandboxType::WindowsAppContainer => { + #[cfg(windows)] + { + let ExecParams { + command, + cwd: command_cwd, + env, + .. + } = params; + let child = spawn_command_under_windows_appcontainer( + command, + command_cwd, + sandbox_policy, + sandbox_cwd, + StdioPolicy::RedirectForShellTool, + env, + ) + .await?; + consume_truncated_output(child, timeout_duration, stdout_stream.clone()).await + } + #[cfg(not(windows))] + { + panic!("windows sandboxing is not available on this platform"); + } } SandboxType::MacosSeatbelt => { let ExecParams { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 08baa2bdc6..0b184e79d1 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -82,6 +82,9 @@ mod tasks; mod user_notification; pub mod util; +#[cfg(windows)] +pub mod windows_appcontainer; + pub use apply_patch::CODEX_APPLY_PATCH_ARG1; pub use command_safety::is_safe_command; pub use safety::get_platform_sandbox; diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs new file mode 100644 index 0000000000..01ea29cdc6 --- /dev/null +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -0,0 +1,472 @@ +#![cfg(windows)] + +use std::collections::HashMap; +use std::ffi::OsStr; +use std::io; +use std::os::windows::ffi::OsStrExt; +use std::path::Path; +use std::path::PathBuf; +use std::ptr::null_mut; + +use tokio::process::Child; +use tokio::process::Command; +use tracing::trace; + +use crate::protocol::SandboxPolicy; +use crate::spawn::CODEX_SANDBOX_ENV_VAR; +use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; +use crate::spawn::StdioPolicy; + +use windows::Win32::Foundation::ERROR_ALREADY_EXISTS; +use windows::Win32::Foundation::ERROR_SUCCESS; +use windows::Win32::Foundation::GetLastError; +use windows::Win32::Foundation::HANDLE; +use windows::Win32::Foundation::WIN32_ERROR; +use windows::Win32::Security::Authorization::DACL_SECURITY_INFORMATION; +use windows::Win32::Security::Authorization::EXPLICIT_ACCESS_W; +use windows::Win32::Security::Authorization::GetNamedSecurityInfoW; +use windows::Win32::Security::Authorization::OBJECT_INHERIT_ACE; +use windows::Win32::Security::Authorization::SE_FILE_OBJECT; +use windows::Win32::Security::Authorization::SET_ACCESS; +use windows::Win32::Security::Authorization::SUB_CONTAINERS_AND_OBJECTS_INHERIT; +use windows::Win32::Security::Authorization::SetEntriesInAclW; +use windows::Win32::Security::Authorization::SetNamedSecurityInfoW; +use windows::Win32::Security::Authorization::TRUSTEE_FORM; +use windows::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN; +use windows::Win32::Security::Authorization::TRUSTEE_W; +use windows::Win32::Security::ConvertStringSidToSidW; +use windows::Win32::Security::CreateAppContainerProfile; +use windows::Win32::Security::DeriveAppContainerSidFromAppContainerName; +use windows::Win32::Security::FreeSid; +use windows::Win32::Security::PSID; +use windows::Win32::Security::SECURITY_CAPABILITIES; +use windows::Win32::Security::SID_AND_ATTRIBUTES; +use windows::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; +use windows::Win32::Storage::FileSystem::FILE_GENERIC_READ; +use windows::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; +use windows::Win32::System::Memory::GetProcessHeap; +use windows::Win32::System::Memory::HEAP_ZERO_MEMORY; +use windows::Win32::System::Memory::HeapAlloc; +use windows::Win32::System::Memory::HeapFree; +use windows::Win32::System::Memory::LocalFree; +use windows::Win32::System::Threading::DeleteProcThreadAttributeList; +use windows::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; +use windows::Win32::System::Threading::InitializeProcThreadAttributeList; +use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_LIST; +use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES; +use windows::Win32::System::Threading::UpdateProcThreadAttribute; +use windows::core::PCWSTR; +use windows::core::PWSTR; + +/// Friendly name for the profile we create on-demand. +const WINDOWS_APPCONTAINER_PROFILE_NAME: &str = "codex_appcontainer"; +const WINDOWS_APPCONTAINER_PROFILE_DESC: &str = "Codex Windows AppContainer profile"; +/// Marker injected into the child so downstream tools can detect the sandbox. +const WINDOWS_APPCONTAINER_SANDBOX_VALUE: &str = "windows_appcontainer"; +/// Capability SID strings that unlock outbound networking when the policy allows it. +const INTERNET_CLIENT_SID: &str = "S-1-15-3-1"; +const PRIVATE_NETWORK_CLIENT_SID: &str = "S-1-15-3-3"; + +/// Runs the provided command inside an AppContainer sandbox that mirrors the +/// sandbox policy Codex already uses on macOS seatbelt and Linux Landlock. The +/// Windows sandbox flow is intentionally verbose so future contributors can map +/// each Windows API call to the equivalent behavior in the other platforms. +pub async fn spawn_command_under_windows_appcontainer( + command: Vec, + command_cwd: PathBuf, + sandbox_policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, + stdio_policy: StdioPolicy, + mut env: HashMap, +) -> io::Result { + trace!("windows appcontainer sandbox command = {:?}", command); + + let (program, rest) = command + .split_first() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "command args are empty"))?; + + // Windows requires a named profile before it will create an AppContainer + // token, so we create-or-open the profile and then derive the SID that we + // will hand to CreateProcess via the extended startup info structure. + ensure_appcontainer_profile()?; + let mut sid = derive_appcontainer_sid()?; + // Capabilities translate Codex' sandbox policy knobs (for now just + // networking) into Windows capability SIDs that can be attached to the + // token. When the policy does not require a capability the vector is empty + // and UpdateProcThreadAttribute receives a null pointer instead. + let mut capability_sids = build_capabilities(sandbox_policy)?; + // The attribute list owns the SECURITY_CAPABILITIES struct plus the heap + // buffer required by UpdateProcThreadAttribute. Keeping it in a guard object + // mirrors the RAII helpers we already use on the Unix sandboxes. + let mut attribute_list = AttributeList::new(&mut sid, &mut capability_sids)?; + + // The Linux and macOS implementations pre-authorize the workspace so the + // tool call can write to the expected roots. We replicate that behavior by + // updating the directory ACLs for the derived AppContainer SID. + configure_writable_roots(sandbox_policy, sandbox_policy_cwd, sid.sid())?; + configure_writable_roots_for_command_cwd(&command_cwd, sid.sid())?; + + if !sandbox_policy.has_full_network_access() { + env.insert( + CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(), + "1".to_string(), + ); + } + env.insert( + CODEX_SANDBOX_ENV_VAR.to_string(), + WINDOWS_APPCONTAINER_SANDBOX_VALUE.to_string(), + ); + + let mut cmd = Command::new(program); + cmd.args(rest); + cmd.current_dir(command_cwd); + cmd.env_clear(); + cmd.envs(env); + apply_stdio_policy(&mut cmd, stdio_policy); + cmd.kill_on_drop(true); + + unsafe { + let std_cmd = cmd.as_std_mut(); + std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT); + std_cmd.raw_attribute_list(attribute_list.as_mut_ptr()); + } + + let child = cmd.spawn(); + drop(attribute_list); + child +} + +fn apply_stdio_policy(cmd: &mut Command, policy: StdioPolicy) { + match policy { + StdioPolicy::RedirectForShellTool => { + cmd.stdin(std::process::Stdio::null()); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); + } + StdioPolicy::Inherit => { + cmd.stdin(std::process::Stdio::inherit()); + cmd.stdout(std::process::Stdio::inherit()); + cmd.stderr(std::process::Stdio::inherit()); + } + } +} + +/// Converts a UTF-8 string into a Windows-compatible UTF-16 buffer with a +/// trailing nul byte. The helper keeps the conversion close to the code that +/// owns the literal strings so maintenance is straightforward. +fn to_wide>(s: S) -> Vec { + s.as_ref().encode_wide().chain(std::iter::once(0)).collect() +} + +/// Creates the AppContainer profile if it does not already exist. Windows keeps +/// track of AppContainer profiles globally for the user account, so subsequent +/// calls simply observe `ERROR_ALREADY_EXISTS` and continue. +fn ensure_appcontainer_profile() -> io::Result<()> { + unsafe { + let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); + let desc = to_wide(WINDOWS_APPCONTAINER_PROFILE_DESC); + let hr = CreateAppContainerProfile( + PCWSTR(name.as_ptr()), + PCWSTR(name.as_ptr()), + PCWSTR(desc.as_ptr()), + null_mut(), + 0, + null_mut(), + ); + if let Err(error) = hr { + let already_exists = WIN32_ERROR::from(ERROR_ALREADY_EXISTS); + if GetLastError() != already_exists { + return Err(io::Error::from_raw_os_error(error.code().0)); + } + } + } + Ok(()) +} + +/// Small RAII wrapper around the derived AppContainer SID so we always release +/// it via `FreeSid` when the sandbox scaffolding is dropped. +struct SidHandle { + ptr: PSID, +} + +impl SidHandle { + fn sid(&self) -> PSID { + self.ptr + } +} + +impl Drop for SidHandle { + fn drop(&mut self) { + unsafe { + if !self.ptr.is_null() { + FreeSid(self.ptr); + } + } + } +} + +fn derive_appcontainer_sid() -> io::Result { + unsafe { + let mut sid_ptr = null_mut(); + let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); + DeriveAppContainerSidFromAppContainerName(PCWSTR(name.as_ptr()), &mut sid_ptr) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + Ok(SidHandle { ptr: sid_ptr }) + } +} + +/// Holds capability SIDs that are allocated with `LocalAlloc`. Keeping the +/// pointers alive inside a struct simplifies cleanup. +struct CapabilitySid { + sid: PSID, +} + +impl CapabilitySid { + fn new_from_string(value: &str) -> io::Result { + unsafe { + let mut sid_ptr = null_mut(); + let wide = to_wide(value); + if ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut sid_ptr) == 0 { + return Err(io::Error::last_os_error()); + } + Ok(Self { sid: sid_ptr }) + } + } + + fn sid_and_attributes(&self) -> SID_AND_ATTRIBUTES { + SID_AND_ATTRIBUTES { + Sid: self.sid, + Attributes: 0, + } + } +} + +impl Drop for CapabilitySid { + fn drop(&mut self) { + unsafe { + if !self.sid.is_null() { + LocalFree(self.sid as isize); + } + } + } +} + +fn build_capabilities(policy: &SandboxPolicy) -> io::Result> { + if policy.has_full_network_access() { + // Matching the other platforms, enabling network access translates to + // enabling both the public-internet capability and the private-network + // capability. Each SID is allocated with LocalAlloc so the RAII wrapper + // releases it automatically when the sandbox scaffolding drops. + Ok(vec![ + CapabilitySid::new_from_string(INTERNET_CLIENT_SID)?, + CapabilitySid::new_from_string(PRIVATE_NETWORK_CLIENT_SID)?, + ]) + } else { + Ok(Vec::new()) + } +} + +/// Manages the Windows attribute list that injects `SECURITY_CAPABILITIES` +/// (the AppContainer SID plus capability SIDs) into `CreateProcessW`. +struct AttributeList<'a> { + heap: HANDLE, + buffer: *mut std::ffi::c_void, + list: *mut PROC_THREAD_ATTRIBUTE_LIST, + sec_caps: SECURITY_CAPABILITIES, + sid_and_attributes: Vec, + #[allow(dead_code)] + sid: &'a mut SidHandle, + #[allow(dead_code)] + capabilities: &'a mut Vec, +} + +impl<'a> AttributeList<'a> { + fn new(sid: &'a mut SidHandle, caps: &'a mut Vec) -> io::Result { + unsafe { + let mut list_size = 0usize; + InitializeProcThreadAttributeList(null_mut(), 1, 0, &mut list_size); + let heap = GetProcessHeap(); + if heap.is_invalid() { + return Err(io::Error::last_os_error()); + } + let buffer = HeapAlloc(heap, HEAP_ZERO_MEMORY, list_size); + if buffer.is_null() { + return Err(io::Error::last_os_error()); + } + let list = buffer as *mut PROC_THREAD_ATTRIBUTE_LIST; + InitializeProcThreadAttributeList(list, 1, 0, &mut list_size) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + + let mut sid_and_attributes: Vec = + caps.iter().map(CapabilitySid::sid_and_attributes).collect(); + + let mut sec_caps = SECURITY_CAPABILITIES { + AppContainerSid: sid.sid(), + Capabilities: if sid_and_attributes.is_empty() { + null_mut() + } else { + sid_and_attributes.as_mut_ptr() + }, + CapabilityCount: sid_and_attributes.len() as u32, + Reserved: null_mut(), + }; + + UpdateProcThreadAttribute( + list, + 0, + PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, + &mut sec_caps as *mut _ as *mut _, + std::mem::size_of::(), + null_mut(), + null_mut(), + ) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + + Ok(Self { + heap, + buffer, + list, + sec_caps, + sid_and_attributes, + sid, + capabilities: caps, + }) + } + } + + fn as_mut_ptr(&mut self) -> *mut PROC_THREAD_ATTRIBUTE_LIST { + self.list + } +} + +impl Drop for AttributeList<'_> { + fn drop(&mut self) { + unsafe { + if !self.list.is_null() { + DeleteProcThreadAttributeList(self.list); + } + if !self.heap.is_invalid() && !self.buffer.is_null() { + HeapFree(self.heap, 0, self.buffer); + } + } + } +} + +/// Applies directory ACLs for every writable root described by the sandbox +/// policy. Granting explicit rights to the AppContainer SID mirrors how the +/// macOS and Linux sandboxes pre-authorize the workspace while leaving the rest +/// of the filesystem read-only. +fn configure_writable_roots( + policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, + sid: PSID, +) -> io::Result<()> { + match policy { + SandboxPolicy::DangerFullAccess => Ok(()), + SandboxPolicy::ReadOnly => grant_path_with_flags(sandbox_policy_cwd, sid, false), + SandboxPolicy::WorkspaceWrite { .. } => { + let roots = policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + for writable in roots { + grant_path_with_flags(&writable.root, sid, true)?; + for ro in writable.read_only_subpaths { + grant_path_with_flags(&ro, sid, false)?; + } + } + Ok(()) + } + } +} + +fn configure_writable_roots_for_command_cwd(command_cwd: &Path, sid: PSID) -> io::Result<()> { + grant_path_with_flags(command_cwd, sid, true) +} + +/// Adds an inheritable ACE for the AppContainer SID so the sandbox can reach +/// specific roots. The helper augments the existing DACL rather than +/// overwriting it so it is safe to call repeatedly. +fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> { + if !path.exists() { + return Ok(()); + } + + let wide = to_wide(path.as_os_str()); + unsafe { + let mut existing_dacl = null_mut(); + let mut security_descriptor = null_mut(); + // Pull the current DACL so we can append our ACE without clobbering any + // existing inheritance or user-specific access entries. + let status = GetNamedSecurityInfoW( + PCWSTR(wide.as_ptr()), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + None, + None, + &mut existing_dacl, + null_mut(), + &mut security_descriptor, + ); + if status != WIN32_ERROR::from(ERROR_SUCCESS) { + if !security_descriptor.is_null() { + LocalFree(security_descriptor as isize); + } + return Err(io::Error::from_raw_os_error(status.0 as i32)); + } + + let permissions = if write { + FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE + } else { + FILE_GENERIC_READ | FILE_GENERIC_EXECUTE + }; + let mut explicit = EXPLICIT_ACCESS_W { + grfAccessPermissions: permissions, + grfAccessMode: SET_ACCESS, + grfInheritance: SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE, + Trustee: TRUSTEE_W { + TrusteeForm: TRUSTEE_FORM::TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_UNKNOWN, + ptstrName: PWSTR(sid as *mut _), + ..Default::default() + }, + }; + + let mut new_dacl = null_mut(); + let add_result = SetEntriesInAclW(1, &mut explicit, existing_dacl, &mut new_dacl); + if add_result != WIN32_ERROR::from(ERROR_SUCCESS) { + if !new_dacl.is_null() { + LocalFree(new_dacl as isize); + } + if !security_descriptor.is_null() { + LocalFree(security_descriptor as isize); + } + return Err(io::Error::from_raw_os_error(add_result.0 as i32)); + } + + let set_result = SetNamedSecurityInfoW( + PCWSTR(wide.as_ptr()), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + None, + None, + Some(new_dacl), + None, + ); + if set_result != WIN32_ERROR::from(ERROR_SUCCESS) { + if !new_dacl.is_null() { + LocalFree(new_dacl as isize); + } + if !security_descriptor.is_null() { + LocalFree(security_descriptor as isize); + } + return Err(io::Error::from_raw_os_error(set_result.0 as i32)); + } + + if !new_dacl.is_null() { + LocalFree(new_dacl as isize); + } + if !security_descriptor.is_null() { + LocalFree(security_descriptor as isize); + } + } + + Ok(()) +} diff --git a/codex-rs/core/tests/windows_appcontainer.rs b/codex-rs/core/tests/windows_appcontainer.rs new file mode 100644 index 0000000000..024804c1c2 --- /dev/null +++ b/codex-rs/core/tests/windows_appcontainer.rs @@ -0,0 +1,76 @@ +#![cfg(windows)] + +use codex_core::protocol::SandboxPolicy; +use codex_core::spawn::StdioPolicy; +use codex_core::windows_appcontainer::spawn_command_under_windows_appcontainer; +use std::collections::HashMap; +use tokio::io::AsyncReadExt; + +fn windows_workspace_policy() -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + } +} + +#[tokio::test] +async fn windows_appcontainer_writes_to_workspace() { + let temp = tempfile::tempdir().expect("tempdir"); + let cwd = temp.path().to_path_buf(); + let policy_cwd = cwd.clone(); + let mut child = spawn_command_under_windows_appcontainer( + vec![ + "cmd.exe".to_string(), + "/C".to_string(), + "echo hello>out.txt".to_string(), + ], + cwd.clone(), + &windows_workspace_policy(), + &policy_cwd, + StdioPolicy::RedirectForShellTool, + HashMap::new(), + ) + .await + .expect("spawn cmd"); + + let status = child.wait().await.expect("wait"); + assert!(status.success(), "cmd.exe failed: {status:?}"); + + let contents = tokio::fs::read_to_string(temp.path().join("out.txt")) + .await + .expect("read redirected output"); + assert!(contents.to_lowercase().contains("hello")); +} + +#[tokio::test] +async fn windows_appcontainer_sets_env_flags() { + let temp = tempfile::tempdir().expect("tempdir"); + let cwd = temp.path().to_path_buf(); + let policy_cwd = cwd.clone(); + let mut child = spawn_command_under_windows_appcontainer( + vec![ + "cmd.exe".to_string(), + "/C".to_string(), + "set CODEX_SANDBOX".to_string(), + ], + cwd, + &windows_workspace_policy(), + &policy_cwd, + StdioPolicy::RedirectForShellTool, + HashMap::new(), + ) + .await + .expect("spawn cmd"); + + let mut stdout = Vec::new(); + if let Some(mut out) = child.stdout.take() { + out.read_to_end(&mut stdout).await.expect("stdout"); + } + let status = child.wait().await.expect("wait"); + assert!(status.success(), "cmd.exe env probe failed: {status:?}"); + let stdout_text = String::from_utf8_lossy(&stdout).to_lowercase(); + assert!(stdout_text.contains("codex_sandbox=windows_appcontainer")); + assert!(stdout_text.contains("codex_sandbox_network_disabled=1")); +} diff --git a/codex-rs/exec/tests/windows_sandbox.rs b/codex-rs/exec/tests/windows_sandbox.rs new file mode 100644 index 0000000000..064f61a122 --- /dev/null +++ b/codex-rs/exec/tests/windows_sandbox.rs @@ -0,0 +1,74 @@ +#![cfg(windows)] + +use std::collections::HashMap; +use std::path::PathBuf; + +use codex_core::exec::ExecParams; +use codex_core::exec::SandboxType; +use codex_core::exec::process_exec_tool_call; +use codex_core::protocol::SandboxPolicy; +use codex_core::safety::set_windows_sandbox_enabled; + +struct WindowsSandboxGuard; + +impl WindowsSandboxGuard { + fn enable() -> Self { + set_windows_sandbox_enabled(true); + Self + } +} + +impl Drop for WindowsSandboxGuard { + fn drop(&mut self) { + set_windows_sandbox_enabled(false); + } +} + +fn windows_workspace_policy(root: &PathBuf) -> SandboxPolicy { + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![root.clone()], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + } +} + +#[tokio::test] +async fn exec_tool_uses_windows_sandbox() { + let _guard = WindowsSandboxGuard::enable(); + let temp = tempfile::tempdir().expect("tempdir"); + let cwd = temp.path().to_path_buf(); + let policy = windows_workspace_policy(&cwd); + let params = ExecParams { + command: vec![ + "cmd.exe".to_string(), + "/C".to_string(), + "set CODEX_SANDBOX".to_string(), + ], + cwd: cwd.clone(), + timeout_ms: None, + env: HashMap::new(), + with_escalated_permissions: None, + justification: None, + }; + + let output = process_exec_tool_call( + params, + SandboxType::WindowsAppContainer, + &policy, + temp.path(), + &None, + None, + ) + .await + .expect("exec output"); + + assert_eq!(output.exit_code, 0); + assert!( + output + .aggregated_output + .text + .to_lowercase() + .contains("codex_sandbox=windows_appcontainer") + ); +} From 56b4e37e8ab768fbaf961dd10cf2e097c244bd9a Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 12:22:30 -0700 Subject: [PATCH 3/8] fix windows build errors. --- codex-rs/core/src/windows_appcontainer.rs | 140 ++++++++++++---------- 1 file changed, 75 insertions(+), 65 deletions(-) diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index 01ea29cdc6..0fc340dc8c 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -1,9 +1,9 @@ -#![cfg(windows)] - use std::collections::HashMap; use std::ffi::OsStr; +use std::ffi::c_void; use std::io; use std::os::windows::ffi::OsStrExt; +use std::os::windows::process::CommandExt; use std::path::Path; use std::path::PathBuf; use std::ptr::null_mut; @@ -21,8 +21,10 @@ use windows::Win32::Foundation::ERROR_ALREADY_EXISTS; use windows::Win32::Foundation::ERROR_SUCCESS; use windows::Win32::Foundation::GetLastError; use windows::Win32::Foundation::HANDLE; +use windows::Win32::Foundation::HLOCAL; use windows::Win32::Foundation::WIN32_ERROR; -use windows::Win32::Security::Authorization::DACL_SECURITY_INFORMATION; +use windows::Win32::Security::ACL; +use windows::Win32::Security::Authorization::ConvertStringSidToSidW; use windows::Win32::Security::Authorization::EXPLICIT_ACCESS_W; use windows::Win32::Security::Authorization::GetNamedSecurityInfoW; use windows::Win32::Security::Authorization::OBJECT_INHERIT_ACE; @@ -31,13 +33,14 @@ use windows::Win32::Security::Authorization::SET_ACCESS; use windows::Win32::Security::Authorization::SUB_CONTAINERS_AND_OBJECTS_INHERIT; use windows::Win32::Security::Authorization::SetEntriesInAclW; use windows::Win32::Security::Authorization::SetNamedSecurityInfoW; -use windows::Win32::Security::Authorization::TRUSTEE_FORM; +use windows::Win32::Security::Authorization::TRUSTEE_IS_SID; use windows::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN; use windows::Win32::Security::Authorization::TRUSTEE_W; -use windows::Win32::Security::ConvertStringSidToSidW; -use windows::Win32::Security::CreateAppContainerProfile; -use windows::Win32::Security::DeriveAppContainerSidFromAppContainerName; +use windows::Win32::Security::DACL_SECURITY_INFORMATION; use windows::Win32::Security::FreeSid; +use windows::Win32::Security::Isolation::CreateAppContainerProfile; +use windows::Win32::Security::Isolation::DeriveAppContainerSidFromAppContainerName; +use windows::Win32::Security::PSECURITY_DESCRIPTOR; use windows::Win32::Security::PSID; use windows::Win32::Security::SECURITY_CAPABILITIES; use windows::Win32::Security::SID_AND_ATTRIBUTES; @@ -45,6 +48,7 @@ use windows::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; use windows::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; use windows::Win32::System::Memory::GetProcessHeap; +use windows::Win32::System::Memory::HEAP_FLAGS; use windows::Win32::System::Memory::HEAP_ZERO_MEMORY; use windows::Win32::System::Memory::HeapAlloc; use windows::Win32::System::Memory::HeapFree; @@ -52,7 +56,7 @@ use windows::Win32::System::Memory::LocalFree; use windows::Win32::System::Threading::DeleteProcThreadAttributeList; use windows::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; use windows::Win32::System::Threading::InitializeProcThreadAttributeList; -use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_LIST; +use windows::Win32::System::Threading::LPPROC_THREAD_ATTRIBUTE_LIST; use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES; use windows::Win32::System::Threading::UpdateProcThreadAttribute; use windows::core::PCWSTR; @@ -128,7 +132,7 @@ pub async fn spawn_command_under_windows_appcontainer( unsafe { let std_cmd = cmd.as_std_mut(); std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT); - std_cmd.raw_attribute_list(attribute_list.as_mut_ptr()); + std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0); } let child = cmd.spawn(); @@ -165,18 +169,22 @@ fn ensure_appcontainer_profile() -> io::Result<()> { unsafe { let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); let desc = to_wide(WINDOWS_APPCONTAINER_PROFILE_DESC); - let hr = CreateAppContainerProfile( + match CreateAppContainerProfile( PCWSTR(name.as_ptr()), PCWSTR(name.as_ptr()), PCWSTR(desc.as_ptr()), - null_mut(), - 0, - null_mut(), - ); - if let Err(error) = hr { - let already_exists = WIN32_ERROR::from(ERROR_ALREADY_EXISTS); - if GetLastError() != already_exists { - return Err(io::Error::from_raw_os_error(error.code().0)); + None, + ) { + Ok(profile_sid) => { + if !profile_sid.is_invalid() { + FreeSid(profile_sid); + } + } + Err(error) => { + let already_exists = WIN32_ERROR::from(ERROR_ALREADY_EXISTS); + if GetLastError() != already_exists { + return Err(io::Error::from_raw_os_error(error.code().0)); + } } } } @@ -198,7 +206,7 @@ impl SidHandle { impl Drop for SidHandle { fn drop(&mut self) { unsafe { - if !self.ptr.is_null() { + if !self.ptr.is_invalid() { FreeSid(self.ptr); } } @@ -207,11 +215,10 @@ impl Drop for SidHandle { fn derive_appcontainer_sid() -> io::Result { unsafe { - let mut sid_ptr = null_mut(); let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); - DeriveAppContainerSidFromAppContainerName(PCWSTR(name.as_ptr()), &mut sid_ptr) + let sid = DeriveAppContainerSidFromAppContainerName(PCWSTR(name.as_ptr())) .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; - Ok(SidHandle { ptr: sid_ptr }) + Ok(SidHandle { ptr: sid }) } } @@ -224,11 +231,10 @@ struct CapabilitySid { impl CapabilitySid { fn new_from_string(value: &str) -> io::Result { unsafe { - let mut sid_ptr = null_mut(); + let mut sid_ptr = PSID::default(); let wide = to_wide(value); - if ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut sid_ptr) == 0 { - return Err(io::Error::last_os_error()); - } + ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut sid_ptr) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; Ok(Self { sid: sid_ptr }) } } @@ -244,8 +250,8 @@ impl CapabilitySid { impl Drop for CapabilitySid { fn drop(&mut self) { unsafe { - if !self.sid.is_null() { - LocalFree(self.sid as isize); + if !self.sid.is_invalid() { + let _ = LocalFree(HLOCAL(self.sid.0)); } } } @@ -270,8 +276,8 @@ fn build_capabilities(policy: &SandboxPolicy) -> io::Result> /// (the AppContainer SID plus capability SIDs) into `CreateProcessW`. struct AttributeList<'a> { heap: HANDLE, - buffer: *mut std::ffi::c_void, - list: *mut PROC_THREAD_ATTRIBUTE_LIST, + buffer: *mut c_void, + list: LPPROC_THREAD_ATTRIBUTE_LIST, sec_caps: SECURITY_CAPABILITIES, sid_and_attributes: Vec, #[allow(dead_code)] @@ -284,16 +290,18 @@ impl<'a> AttributeList<'a> { fn new(sid: &'a mut SidHandle, caps: &'a mut Vec) -> io::Result { unsafe { let mut list_size = 0usize; - InitializeProcThreadAttributeList(null_mut(), 1, 0, &mut list_size); - let heap = GetProcessHeap(); - if heap.is_invalid() { - return Err(io::Error::last_os_error()); - } + let _ = InitializeProcThreadAttributeList( + LPPROC_THREAD_ATTRIBUTE_LIST::default(), + 1, + 0, + &mut list_size, + ); + let heap = GetProcessHeap().map_err(|e| io::Error::from_raw_os_error(e.code().0))?; let buffer = HeapAlloc(heap, HEAP_ZERO_MEMORY, list_size); if buffer.is_null() { return Err(io::Error::last_os_error()); } - let list = buffer as *mut PROC_THREAD_ATTRIBUTE_LIST; + let list = LPPROC_THREAD_ATTRIBUTE_LIST(buffer); InitializeProcThreadAttributeList(list, 1, 0, &mut list_size) .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; @@ -308,17 +316,17 @@ impl<'a> AttributeList<'a> { sid_and_attributes.as_mut_ptr() }, CapabilityCount: sid_and_attributes.len() as u32, - Reserved: null_mut(), + Reserved: 0, }; UpdateProcThreadAttribute( list, 0, - PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, - &mut sec_caps as *mut _ as *mut _, + PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES as usize, + Some(&mut sec_caps as *mut _ as *const std::ffi::c_void), std::mem::size_of::(), - null_mut(), - null_mut(), + None, + None, ) .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; @@ -334,7 +342,7 @@ impl<'a> AttributeList<'a> { } } - fn as_mut_ptr(&mut self) -> *mut PROC_THREAD_ATTRIBUTE_LIST { + fn as_mut_ptr(&mut self) -> LPPROC_THREAD_ATTRIBUTE_LIST { self.list } } @@ -342,11 +350,11 @@ impl<'a> AttributeList<'a> { impl Drop for AttributeList<'_> { fn drop(&mut self) { unsafe { - if !self.list.is_null() { + if !self.list.is_invalid() { DeleteProcThreadAttributeList(self.list); } if !self.heap.is_invalid() && !self.buffer.is_null() { - HeapFree(self.heap, 0, self.buffer); + let _ = HeapFree(self.heap, HEAP_FLAGS(0), Some(self.buffer)); } } } @@ -391,8 +399,8 @@ fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> let wide = to_wide(path.as_os_str()); unsafe { - let mut existing_dacl = null_mut(); - let mut security_descriptor = null_mut(); + let mut existing_dacl: *mut ACL = null_mut(); + let mut security_descriptor = PSECURITY_DESCRIPTOR::default(); // Pull the current DACL so we can append our ACE without clobbering any // existing inheritance or user-specific access entries. let status = GetNamedSecurityInfoW( @@ -401,42 +409,44 @@ fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> DACL_SECURITY_INFORMATION, None, None, - &mut existing_dacl, - null_mut(), + Some(&mut existing_dacl), + None, &mut security_descriptor, ); if status != WIN32_ERROR::from(ERROR_SUCCESS) { - if !security_descriptor.is_null() { - LocalFree(security_descriptor as isize); + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); } return Err(io::Error::from_raw_os_error(status.0 as i32)); } let permissions = if write { - FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE + (FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE).0 } else { - FILE_GENERIC_READ | FILE_GENERIC_EXECUTE + (FILE_GENERIC_READ | FILE_GENERIC_EXECUTE).0 }; let mut explicit = EXPLICIT_ACCESS_W { grfAccessPermissions: permissions, grfAccessMode: SET_ACCESS, grfInheritance: SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE, Trustee: TRUSTEE_W { - TrusteeForm: TRUSTEE_FORM::TRUSTEE_IS_SID, + TrusteeForm: TRUSTEE_IS_SID, TrusteeType: TRUSTEE_IS_UNKNOWN, - ptstrName: PWSTR(sid as *mut _), + ptstrName: PWSTR(sid.0.cast()), ..Default::default() }, }; - let mut new_dacl = null_mut(); - let add_result = SetEntriesInAclW(1, &mut explicit, existing_dacl, &mut new_dacl); + let explicit_entries = [explicit]; + let mut new_dacl: *mut ACL = null_mut(); + let add_result = + SetEntriesInAclW(Some(&explicit_entries), Some(existing_dacl), &mut new_dacl); if add_result != WIN32_ERROR::from(ERROR_SUCCESS) { if !new_dacl.is_null() { - LocalFree(new_dacl as isize); + let _ = LocalFree(HLOCAL(new_dacl.cast())); } - if !security_descriptor.is_null() { - LocalFree(security_descriptor as isize); + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); } return Err(io::Error::from_raw_os_error(add_result.0 as i32)); } @@ -452,19 +462,19 @@ fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> ); if set_result != WIN32_ERROR::from(ERROR_SUCCESS) { if !new_dacl.is_null() { - LocalFree(new_dacl as isize); + let _ = LocalFree(HLOCAL(new_dacl.cast())); } - if !security_descriptor.is_null() { - LocalFree(security_descriptor as isize); + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); } return Err(io::Error::from_raw_os_error(set_result.0 as i32)); } if !new_dacl.is_null() { - LocalFree(new_dacl as isize); + let _ = LocalFree(HLOCAL(new_dacl.cast())); } - if !security_descriptor.is_null() { - LocalFree(security_descriptor as isize); + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); } } From 4b0a4f3f87f7ac8b1cf6940f3bb0b2680f06ba7e Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 13:32:06 -0700 Subject: [PATCH 4/8] fix build errors. --- codex-rs/core/Cargo.toml | 5 + codex-rs/core/src/windows_appcontainer.rs | 791 ++++++++++---------- codex-rs/core/tests/windows_appcontainer.rs | 2 +- 3 files changed, 390 insertions(+), 408 deletions(-) diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index ae0927c3df..a895d4f563 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -82,6 +82,7 @@ core-foundation = "0.9" [target.'cfg(windows)'.dependencies] windows = { version = "0.58", features = [ "Win32_Foundation", + "Win32_Security_Isolation", "Win32_Security", "Win32_Security_Authorization", "Win32_Storage_FileSystem", @@ -89,6 +90,10 @@ windows = { version = "0.58", features = [ "Win32_System_Threading", ] } +[features] +default = [] +windows_appcontainer_command_ext = [] + # Build OpenSSL from source for musl builds. [target.x86_64-unknown-linux-musl.dependencies] openssl-sys = { workspace = true, features = ["vendored"] } diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index 0fc340dc8c..abec508918 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -1,482 +1,459 @@ -use std::collections::HashMap; -use std::ffi::OsStr; -use std::ffi::c_void; -use std::io; -use std::os::windows::ffi::OsStrExt; -use std::os::windows::process::CommandExt; -use std::path::Path; -use std::path::PathBuf; -use std::ptr::null_mut; - -use tokio::process::Child; -use tokio::process::Command; use tracing::trace; use crate::protocol::SandboxPolicy; -use crate::spawn::CODEX_SANDBOX_ENV_VAR; -use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use crate::spawn::StdioPolicy; -use windows::Win32::Foundation::ERROR_ALREADY_EXISTS; -use windows::Win32::Foundation::ERROR_SUCCESS; -use windows::Win32::Foundation::GetLastError; -use windows::Win32::Foundation::HANDLE; -use windows::Win32::Foundation::HLOCAL; -use windows::Win32::Foundation::WIN32_ERROR; -use windows::Win32::Security::ACL; -use windows::Win32::Security::Authorization::ConvertStringSidToSidW; -use windows::Win32::Security::Authorization::EXPLICIT_ACCESS_W; -use windows::Win32::Security::Authorization::GetNamedSecurityInfoW; -use windows::Win32::Security::Authorization::OBJECT_INHERIT_ACE; -use windows::Win32::Security::Authorization::SE_FILE_OBJECT; -use windows::Win32::Security::Authorization::SET_ACCESS; -use windows::Win32::Security::Authorization::SUB_CONTAINERS_AND_OBJECTS_INHERIT; -use windows::Win32::Security::Authorization::SetEntriesInAclW; -use windows::Win32::Security::Authorization::SetNamedSecurityInfoW; -use windows::Win32::Security::Authorization::TRUSTEE_IS_SID; -use windows::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN; -use windows::Win32::Security::Authorization::TRUSTEE_W; -use windows::Win32::Security::DACL_SECURITY_INFORMATION; -use windows::Win32::Security::FreeSid; -use windows::Win32::Security::Isolation::CreateAppContainerProfile; -use windows::Win32::Security::Isolation::DeriveAppContainerSidFromAppContainerName; -use windows::Win32::Security::PSECURITY_DESCRIPTOR; -use windows::Win32::Security::PSID; -use windows::Win32::Security::SECURITY_CAPABILITIES; -use windows::Win32::Security::SID_AND_ATTRIBUTES; -use windows::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; -use windows::Win32::Storage::FileSystem::FILE_GENERIC_READ; -use windows::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; -use windows::Win32::System::Memory::GetProcessHeap; -use windows::Win32::System::Memory::HEAP_FLAGS; -use windows::Win32::System::Memory::HEAP_ZERO_MEMORY; -use windows::Win32::System::Memory::HeapAlloc; -use windows::Win32::System::Memory::HeapFree; -use windows::Win32::System::Memory::LocalFree; -use windows::Win32::System::Threading::DeleteProcThreadAttributeList; -use windows::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; -use windows::Win32::System::Threading::InitializeProcThreadAttributeList; -use windows::Win32::System::Threading::LPPROC_THREAD_ATTRIBUTE_LIST; -use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES; -use windows::Win32::System::Threading::UpdateProcThreadAttribute; -use windows::core::PCWSTR; -use windows::core::PWSTR; - -/// Friendly name for the profile we create on-demand. -const WINDOWS_APPCONTAINER_PROFILE_NAME: &str = "codex_appcontainer"; -const WINDOWS_APPCONTAINER_PROFILE_DESC: &str = "Codex Windows AppContainer profile"; -/// Marker injected into the child so downstream tools can detect the sandbox. -const WINDOWS_APPCONTAINER_SANDBOX_VALUE: &str = "windows_appcontainer"; -/// Capability SID strings that unlock outbound networking when the policy allows it. -const INTERNET_CLIENT_SID: &str = "S-1-15-3-1"; -const PRIVATE_NETWORK_CLIENT_SID: &str = "S-1-15-3-3"; - -/// Runs the provided command inside an AppContainer sandbox that mirrors the -/// sandbox policy Codex already uses on macOS seatbelt and Linux Landlock. The -/// Windows sandbox flow is intentionally verbose so future contributors can map -/// each Windows API call to the equivalent behavior in the other platforms. -pub async fn spawn_command_under_windows_appcontainer( - command: Vec, - command_cwd: PathBuf, - sandbox_policy: &SandboxPolicy, - sandbox_policy_cwd: &Path, - stdio_policy: StdioPolicy, - mut env: HashMap, -) -> io::Result { - trace!("windows appcontainer sandbox command = {:?}", command); - - let (program, rest) = command - .split_first() - .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "command args are empty"))?; - - // Windows requires a named profile before it will create an AppContainer - // token, so we create-or-open the profile and then derive the SID that we - // will hand to CreateProcess via the extended startup info structure. - ensure_appcontainer_profile()?; - let mut sid = derive_appcontainer_sid()?; - // Capabilities translate Codex' sandbox policy knobs (for now just - // networking) into Windows capability SIDs that can be attached to the - // token. When the policy does not require a capability the vector is empty - // and UpdateProcThreadAttribute receives a null pointer instead. - let mut capability_sids = build_capabilities(sandbox_policy)?; - // The attribute list owns the SECURITY_CAPABILITIES struct plus the heap - // buffer required by UpdateProcThreadAttribute. Keeping it in a guard object - // mirrors the RAII helpers we already use on the Unix sandboxes. - let mut attribute_list = AttributeList::new(&mut sid, &mut capability_sids)?; - - // The Linux and macOS implementations pre-authorize the workspace so the - // tool call can write to the expected roots. We replicate that behavior by - // updating the directory ACLs for the derived AppContainer SID. - configure_writable_roots(sandbox_policy, sandbox_policy_cwd, sid.sid())?; - configure_writable_roots_for_command_cwd(&command_cwd, sid.sid())?; - - if !sandbox_policy.has_full_network_access() { +#[cfg(feature = "windows_appcontainer_command_ext")] +mod imp { + use super::*; + + use std::ffi::OsStr; + use std::ffi::c_void; + use std::os::windows::ffi::OsStrExt; + use std::os::windows::process::CommandExt; + use std::ptr::null_mut; + + use tokio::process::Command; + + use crate::spawn::CODEX_SANDBOX_ENV_VAR; + use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; + + use windows::Win32::Foundation::ERROR_ALREADY_EXISTS; + use windows::Win32::Foundation::ERROR_SUCCESS; + use windows::Win32::Foundation::GetLastError; + use windows::Win32::Foundation::HANDLE; + use windows::Win32::Foundation::HLOCAL; + use windows::Win32::Foundation::LocalFree; + use windows::Win32::Foundation::WIN32_ERROR; + use windows::Win32::Security::ACL; + use windows::Win32::Security::Authorization::ConvertStringSidToSidW; + use windows::Win32::Security::Authorization::EXPLICIT_ACCESS_W; + use windows::Win32::Security::Authorization::GetNamedSecurityInfoW; + use windows::Win32::Security::Authorization::SE_FILE_OBJECT; + use windows::Win32::Security::Authorization::SET_ACCESS; + use windows::Win32::Security::Authorization::SetEntriesInAclW; + use windows::Win32::Security::Authorization::SetNamedSecurityInfoW; + use windows::Win32::Security::Authorization::TRUSTEE_IS_SID; + use windows::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN; + use windows::Win32::Security::Authorization::TRUSTEE_W; + use windows::Win32::Security::DACL_SECURITY_INFORMATION; + use windows::Win32::Security::FreeSid; + use windows::Win32::Security::Isolation::CreateAppContainerProfile; + use windows::Win32::Security::Isolation::DeriveAppContainerSidFromAppContainerName; + use windows::Win32::Security::OBJECT_INHERIT_ACE; + use windows::Win32::Security::PSECURITY_DESCRIPTOR; + use windows::Win32::Security::PSID; + use windows::Win32::Security::SECURITY_CAPABILITIES; + use windows::Win32::Security::SID_AND_ATTRIBUTES; + use windows::Win32::Security::SUB_CONTAINERS_AND_OBJECTS_INHERIT; + use windows::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; + use windows::Win32::Storage::FileSystem::FILE_GENERIC_READ; + use windows::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; + use windows::Win32::System::Memory::GetProcessHeap; + use windows::Win32::System::Memory::HEAP_FLAGS; + use windows::Win32::System::Memory::HEAP_ZERO_MEMORY; + use windows::Win32::System::Memory::HeapAlloc; + use windows::Win32::System::Memory::HeapFree; + use windows::Win32::System::Threading::DeleteProcThreadAttributeList; + use windows::Win32::System::Threading::EXTENDED_STARTUPINFO_PRESENT; + use windows::Win32::System::Threading::InitializeProcThreadAttributeList; + use windows::Win32::System::Threading::LPPROC_THREAD_ATTRIBUTE_LIST; + use windows::Win32::System::Threading::PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES; + use windows::Win32::System::Threading::UpdateProcThreadAttribute; + use windows::core::PCWSTR; + use windows::core::PWSTR; + + const WINDOWS_APPCONTAINER_PROFILE_NAME: &str = "codex_appcontainer"; + const WINDOWS_APPCONTAINER_PROFILE_DESC: &str = "Codex Windows AppContainer profile"; + const WINDOWS_APPCONTAINER_SANDBOX_VALUE: &str = "windows_appcontainer"; + const INTERNET_CLIENT_SID: &str = "S-1-15-3-1"; + const PRIVATE_NETWORK_CLIENT_SID: &str = "S-1-15-3-3"; + + pub async fn spawn_command_under_windows_appcontainer( + command: Vec, + command_cwd: PathBuf, + sandbox_policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, + stdio_policy: StdioPolicy, + mut env: HashMap, + ) -> io::Result { + trace!("windows appcontainer sandbox command = {:?}", command); + + let (program, rest) = command + .split_first() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "command args are empty"))?; + + ensure_appcontainer_profile()?; + let mut sid = derive_appcontainer_sid()?; + let mut capability_sids = build_capabilities(sandbox_policy)?; + let mut attribute_list = AttributeList::new(&mut sid, &mut capability_sids)?; + + configure_writable_roots(sandbox_policy, sandbox_policy_cwd, sid.sid())?; + configure_writable_roots_for_command_cwd(&command_cwd, sid.sid())?; + + if !sandbox_policy.has_full_network_access() { + env.insert( + CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(), + "1".to_string(), + ); + } env.insert( - CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(), - "1".to_string(), + CODEX_SANDBOX_ENV_VAR.to_string(), + WINDOWS_APPCONTAINER_SANDBOX_VALUE.to_string(), ); - } - env.insert( - CODEX_SANDBOX_ENV_VAR.to_string(), - WINDOWS_APPCONTAINER_SANDBOX_VALUE.to_string(), - ); - - let mut cmd = Command::new(program); - cmd.args(rest); - cmd.current_dir(command_cwd); - cmd.env_clear(); - cmd.envs(env); - apply_stdio_policy(&mut cmd, stdio_policy); - cmd.kill_on_drop(true); - - unsafe { - let std_cmd = cmd.as_std_mut(); - std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT); - std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0); - } - let child = cmd.spawn(); - drop(attribute_list); - child -} + let mut cmd = Command::new(program); + cmd.args(rest); + cmd.current_dir(command_cwd); + cmd.env_clear(); + cmd.envs(env); + apply_stdio_policy(&mut cmd, stdio_policy); + cmd.kill_on_drop(true); -fn apply_stdio_policy(cmd: &mut Command, policy: StdioPolicy) { - match policy { - StdioPolicy::RedirectForShellTool => { - cmd.stdin(std::process::Stdio::null()); - cmd.stdout(std::process::Stdio::piped()); - cmd.stderr(std::process::Stdio::piped()); - } - StdioPolicy::Inherit => { - cmd.stdin(std::process::Stdio::inherit()); - cmd.stdout(std::process::Stdio::inherit()); - cmd.stderr(std::process::Stdio::inherit()); + unsafe { + let std_cmd = cmd.as_std_mut(); + std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT.0); + std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0); } - } -} -/// Converts a UTF-8 string into a Windows-compatible UTF-16 buffer with a -/// trailing nul byte. The helper keeps the conversion close to the code that -/// owns the literal strings so maintenance is straightforward. -fn to_wide>(s: S) -> Vec { - s.as_ref().encode_wide().chain(std::iter::once(0)).collect() -} + let child = cmd.spawn(); + drop(attribute_list); + child + } -/// Creates the AppContainer profile if it does not already exist. Windows keeps -/// track of AppContainer profiles globally for the user account, so subsequent -/// calls simply observe `ERROR_ALREADY_EXISTS` and continue. -fn ensure_appcontainer_profile() -> io::Result<()> { - unsafe { - let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); - let desc = to_wide(WINDOWS_APPCONTAINER_PROFILE_DESC); - match CreateAppContainerProfile( - PCWSTR(name.as_ptr()), - PCWSTR(name.as_ptr()), - PCWSTR(desc.as_ptr()), - None, - ) { - Ok(profile_sid) => { - if !profile_sid.is_invalid() { - FreeSid(profile_sid); - } + fn apply_stdio_policy(cmd: &mut Command, policy: StdioPolicy) { + match policy { + StdioPolicy::RedirectForShellTool => { + cmd.stdin(std::process::Stdio::null()); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); } - Err(error) => { - let already_exists = WIN32_ERROR::from(ERROR_ALREADY_EXISTS); - if GetLastError() != already_exists { - return Err(io::Error::from_raw_os_error(error.code().0)); - } + StdioPolicy::Inherit => { + cmd.stdin(std::process::Stdio::inherit()); + cmd.stdout(std::process::Stdio::inherit()); + cmd.stderr(std::process::Stdio::inherit()); } } } - Ok(()) -} -/// Small RAII wrapper around the derived AppContainer SID so we always release -/// it via `FreeSid` when the sandbox scaffolding is dropped. -struct SidHandle { - ptr: PSID, -} - -impl SidHandle { - fn sid(&self) -> PSID { - self.ptr + fn to_wide>(s: S) -> Vec { + s.as_ref().encode_wide().chain(std::iter::once(0)).collect() } -} -impl Drop for SidHandle { - fn drop(&mut self) { + fn ensure_appcontainer_profile() -> io::Result<()> { unsafe { - if !self.ptr.is_invalid() { - FreeSid(self.ptr); + let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); + let desc = to_wide(WINDOWS_APPCONTAINER_PROFILE_DESC); + match CreateAppContainerProfile( + PCWSTR(name.as_ptr()), + PCWSTR(name.as_ptr()), + PCWSTR(desc.as_ptr()), + None, + ) { + Ok(profile_sid) => { + if !profile_sid.is_invalid() { + FreeSid(profile_sid); + } + } + Err(error) => { + let already_exists = WIN32_ERROR::from(ERROR_ALREADY_EXISTS); + if GetLastError() != already_exists { + return Err(io::Error::from_raw_os_error(error.code().0)); + } + } } } + Ok(()) } -} -fn derive_appcontainer_sid() -> io::Result { - unsafe { - let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); - let sid = DeriveAppContainerSidFromAppContainerName(PCWSTR(name.as_ptr())) - .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; - Ok(SidHandle { ptr: sid }) + struct SidHandle { + ptr: PSID, } -} - -/// Holds capability SIDs that are allocated with `LocalAlloc`. Keeping the -/// pointers alive inside a struct simplifies cleanup. -struct CapabilitySid { - sid: PSID, -} -impl CapabilitySid { - fn new_from_string(value: &str) -> io::Result { - unsafe { - let mut sid_ptr = PSID::default(); - let wide = to_wide(value); - ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut sid_ptr) - .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; - Ok(Self { sid: sid_ptr }) + impl SidHandle { + fn sid(&self) -> PSID { + self.ptr } } - fn sid_and_attributes(&self) -> SID_AND_ATTRIBUTES { - SID_AND_ATTRIBUTES { - Sid: self.sid, - Attributes: 0, + impl Drop for SidHandle { + fn drop(&mut self) { + unsafe { + if !self.ptr.is_invalid() { + FreeSid(self.ptr); + } + } } } -} -impl Drop for CapabilitySid { - fn drop(&mut self) { + fn derive_appcontainer_sid() -> io::Result { unsafe { - if !self.sid.is_invalid() { - let _ = LocalFree(HLOCAL(self.sid.0)); - } + let name = to_wide(WINDOWS_APPCONTAINER_PROFILE_NAME); + let sid = DeriveAppContainerSidFromAppContainerName(PCWSTR(name.as_ptr())) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + Ok(SidHandle { ptr: sid }) } } -} -fn build_capabilities(policy: &SandboxPolicy) -> io::Result> { - if policy.has_full_network_access() { - // Matching the other platforms, enabling network access translates to - // enabling both the public-internet capability and the private-network - // capability. Each SID is allocated with LocalAlloc so the RAII wrapper - // releases it automatically when the sandbox scaffolding drops. - Ok(vec![ - CapabilitySid::new_from_string(INTERNET_CLIENT_SID)?, - CapabilitySid::new_from_string(PRIVATE_NETWORK_CLIENT_SID)?, - ]) - } else { - Ok(Vec::new()) + struct CapabilitySid { + sid: PSID, } -} - -/// Manages the Windows attribute list that injects `SECURITY_CAPABILITIES` -/// (the AppContainer SID plus capability SIDs) into `CreateProcessW`. -struct AttributeList<'a> { - heap: HANDLE, - buffer: *mut c_void, - list: LPPROC_THREAD_ATTRIBUTE_LIST, - sec_caps: SECURITY_CAPABILITIES, - sid_and_attributes: Vec, - #[allow(dead_code)] - sid: &'a mut SidHandle, - #[allow(dead_code)] - capabilities: &'a mut Vec, -} -impl<'a> AttributeList<'a> { - fn new(sid: &'a mut SidHandle, caps: &'a mut Vec) -> io::Result { - unsafe { - let mut list_size = 0usize; - let _ = InitializeProcThreadAttributeList( - LPPROC_THREAD_ATTRIBUTE_LIST::default(), - 1, - 0, - &mut list_size, - ); - let heap = GetProcessHeap().map_err(|e| io::Error::from_raw_os_error(e.code().0))?; - let buffer = HeapAlloc(heap, HEAP_ZERO_MEMORY, list_size); - if buffer.is_null() { - return Err(io::Error::last_os_error()); + impl CapabilitySid { + fn new_from_string(value: &str) -> io::Result { + unsafe { + let mut sid_ptr = PSID::default(); + let wide = to_wide(value); + ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut sid_ptr) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + Ok(Self { sid: sid_ptr }) } - let list = LPPROC_THREAD_ATTRIBUTE_LIST(buffer); - InitializeProcThreadAttributeList(list, 1, 0, &mut list_size) - .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + } - let mut sid_and_attributes: Vec = - caps.iter().map(CapabilitySid::sid_and_attributes).collect(); + fn sid_and_attributes(&self) -> SID_AND_ATTRIBUTES { + SID_AND_ATTRIBUTES { + Sid: self.sid, + Attributes: 0, + } + } + } - let mut sec_caps = SECURITY_CAPABILITIES { - AppContainerSid: sid.sid(), - Capabilities: if sid_and_attributes.is_empty() { - null_mut() - } else { - sid_and_attributes.as_mut_ptr() - }, - CapabilityCount: sid_and_attributes.len() as u32, - Reserved: 0, - }; + impl Drop for CapabilitySid { + fn drop(&mut self) { + unsafe { + if !self.sid.is_invalid() { + let _ = LocalFree(HLOCAL(self.sid.0)); + } + } + } + } - UpdateProcThreadAttribute( - list, - 0, - PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES as usize, - Some(&mut sec_caps as *mut _ as *const std::ffi::c_void), - std::mem::size_of::(), - None, - None, - ) - .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; - - Ok(Self { - heap, - buffer, - list, - sec_caps, - sid_and_attributes, - sid, - capabilities: caps, - }) + fn build_capabilities(policy: &SandboxPolicy) -> io::Result> { + if policy.has_full_network_access() { + Ok(vec![ + CapabilitySid::new_from_string(INTERNET_CLIENT_SID)?, + CapabilitySid::new_from_string(PRIVATE_NETWORK_CLIENT_SID)?, + ]) + } else { + Ok(Vec::new()) } } - fn as_mut_ptr(&mut self) -> LPPROC_THREAD_ATTRIBUTE_LIST { - self.list + struct AttributeList<'a> { + heap: HANDLE, + buffer: *mut c_void, + list: LPPROC_THREAD_ATTRIBUTE_LIST, + sec_caps: SECURITY_CAPABILITIES, + sid_and_attributes: Vec, + #[allow(dead_code)] + sid: &'a mut SidHandle, + #[allow(dead_code)] + capabilities: &'a mut Vec, } -} -impl Drop for AttributeList<'_> { - fn drop(&mut self) { - unsafe { - if !self.list.is_invalid() { - DeleteProcThreadAttributeList(self.list); - } - if !self.heap.is_invalid() && !self.buffer.is_null() { - let _ = HeapFree(self.heap, HEAP_FLAGS(0), Some(self.buffer)); + impl<'a> AttributeList<'a> { + fn new(sid: &'a mut SidHandle, caps: &'a mut Vec) -> io::Result { + unsafe { + let mut list_size = 0usize; + let _ = InitializeProcThreadAttributeList( + LPPROC_THREAD_ATTRIBUTE_LIST::default(), + 1, + 0, + &mut list_size, + ); + let heap = + GetProcessHeap().map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + let buffer = HeapAlloc(heap, HEAP_ZERO_MEMORY, list_size); + if buffer.is_null() { + return Err(io::Error::last_os_error()); + } + let list = LPPROC_THREAD_ATTRIBUTE_LIST(buffer); + InitializeProcThreadAttributeList(list, 1, 0, &mut list_size) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + + let mut sid_and_attributes: Vec = + caps.iter().map(CapabilitySid::sid_and_attributes).collect(); + + let mut sec_caps = SECURITY_CAPABILITIES { + AppContainerSid: sid.sid(), + Capabilities: if sid_and_attributes.is_empty() { + null_mut() + } else { + sid_and_attributes.as_mut_ptr() + }, + CapabilityCount: sid_and_attributes.len() as u32, + Reserved: 0, + }; + + UpdateProcThreadAttribute( + list, + 0, + PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES as usize, + Some(&mut sec_caps as *mut _ as *const std::ffi::c_void), + std::mem::size_of::(), + None, + None, + ) + .map_err(|e| io::Error::from_raw_os_error(e.code().0))?; + + Ok(Self { + heap, + buffer, + list, + sec_caps, + sid_and_attributes, + sid, + capabilities: caps, + }) } } + + fn as_mut_ptr(&mut self) -> LPPROC_THREAD_ATTRIBUTE_LIST { + self.list + } } -} -/// Applies directory ACLs for every writable root described by the sandbox -/// policy. Granting explicit rights to the AppContainer SID mirrors how the -/// macOS and Linux sandboxes pre-authorize the workspace while leaving the rest -/// of the filesystem read-only. -fn configure_writable_roots( - policy: &SandboxPolicy, - sandbox_policy_cwd: &Path, - sid: PSID, -) -> io::Result<()> { - match policy { - SandboxPolicy::DangerFullAccess => Ok(()), - SandboxPolicy::ReadOnly => grant_path_with_flags(sandbox_policy_cwd, sid, false), - SandboxPolicy::WorkspaceWrite { .. } => { - let roots = policy.get_writable_roots_with_cwd(sandbox_policy_cwd); - for writable in roots { - grant_path_with_flags(&writable.root, sid, true)?; - for ro in writable.read_only_subpaths { - grant_path_with_flags(&ro, sid, false)?; + impl Drop for AttributeList<'_> { + fn drop(&mut self) { + unsafe { + if !self.list.is_invalid() { + DeleteProcThreadAttributeList(self.list); + } + if !self.heap.is_invalid() && !self.buffer.is_null() { + let _ = HeapFree(self.heap, HEAP_FLAGS(0), Some(self.buffer)); } } - Ok(()) } } -} -fn configure_writable_roots_for_command_cwd(command_cwd: &Path, sid: PSID) -> io::Result<()> { - grant_path_with_flags(command_cwd, sid, true) -} + fn configure_writable_roots( + policy: &SandboxPolicy, + sandbox_policy_cwd: &Path, + sid: PSID, + ) -> io::Result<()> { + match policy { + SandboxPolicy::DangerFullAccess => Ok(()), + SandboxPolicy::ReadOnly => grant_path_with_flags(sandbox_policy_cwd, sid, false), + SandboxPolicy::WorkspaceWrite { .. } => { + let roots = policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + for writable in roots { + grant_path_with_flags(&writable.root, sid, true)?; + for ro in writable.read_only_subpaths { + grant_path_with_flags(&ro, sid, false)?; + } + } + Ok(()) + } + } + } -/// Adds an inheritable ACE for the AppContainer SID so the sandbox can reach -/// specific roots. The helper augments the existing DACL rather than -/// overwriting it so it is safe to call repeatedly. -fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> { - if !path.exists() { - return Ok(()); + fn configure_writable_roots_for_command_cwd(command_cwd: &Path, sid: PSID) -> io::Result<()> { + grant_path_with_flags(command_cwd, sid, true) } - let wide = to_wide(path.as_os_str()); - unsafe { - let mut existing_dacl: *mut ACL = null_mut(); - let mut security_descriptor = PSECURITY_DESCRIPTOR::default(); - // Pull the current DACL so we can append our ACE without clobbering any - // existing inheritance or user-specific access entries. - let status = GetNamedSecurityInfoW( - PCWSTR(wide.as_ptr()), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, - None, - None, - Some(&mut existing_dacl), - None, - &mut security_descriptor, - ); - if status != WIN32_ERROR::from(ERROR_SUCCESS) { - if !security_descriptor.is_invalid() { - let _ = LocalFree(HLOCAL(security_descriptor.0)); - } - return Err(io::Error::from_raw_os_error(status.0 as i32)); + fn grant_path_with_flags(path: &Path, sid: PSID, write: bool) -> io::Result<()> { + if !path.exists() { + return Ok(()); } - let permissions = if write { - (FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE).0 - } else { - (FILE_GENERIC_READ | FILE_GENERIC_EXECUTE).0 - }; - let mut explicit = EXPLICIT_ACCESS_W { - grfAccessPermissions: permissions, - grfAccessMode: SET_ACCESS, - grfInheritance: SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE, - Trustee: TRUSTEE_W { - TrusteeForm: TRUSTEE_IS_SID, - TrusteeType: TRUSTEE_IS_UNKNOWN, - ptstrName: PWSTR(sid.0.cast()), - ..Default::default() - }, - }; - - let explicit_entries = [explicit]; - let mut new_dacl: *mut ACL = null_mut(); - let add_result = - SetEntriesInAclW(Some(&explicit_entries), Some(existing_dacl), &mut new_dacl); - if add_result != WIN32_ERROR::from(ERROR_SUCCESS) { - if !new_dacl.is_null() { - let _ = LocalFree(HLOCAL(new_dacl.cast())); + let wide = to_wide(path.as_os_str()); + unsafe { + let mut existing_dacl: *mut ACL = null_mut(); + let mut security_descriptor = PSECURITY_DESCRIPTOR::default(); + let status = GetNamedSecurityInfoW( + PCWSTR(wide.as_ptr()), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + None, + None, + Some(&mut existing_dacl), + None, + &mut security_descriptor, + ); + if status != WIN32_ERROR::from(ERROR_SUCCESS) { + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); + } + return Err(io::Error::from_raw_os_error(status.0 as i32)); } - if !security_descriptor.is_invalid() { - let _ = LocalFree(HLOCAL(security_descriptor.0)); + + let permissions = if write { + (FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE).0 + } else { + (FILE_GENERIC_READ | FILE_GENERIC_EXECUTE).0 + }; + let explicit = EXPLICIT_ACCESS_W { + grfAccessPermissions: permissions, + grfAccessMode: SET_ACCESS, + grfInheritance: (SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE).0, + Trustee: TRUSTEE_W { + TrusteeForm: TRUSTEE_IS_SID, + TrusteeType: TRUSTEE_IS_UNKNOWN, + ptstrName: PWSTR(sid.0.cast()), + ..Default::default() + }, + }; + + let explicit_entries = [explicit]; + let mut new_dacl: *mut ACL = null_mut(); + let add_result = + SetEntriesInAclW(Some(&explicit_entries), Some(existing_dacl), &mut new_dacl); + if add_result != WIN32_ERROR::from(ERROR_SUCCESS) { + if !new_dacl.is_null() { + let _ = LocalFree(HLOCAL(new_dacl.cast())); + } + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); + } + return Err(io::Error::from_raw_os_error(add_result.0 as i32)); + } + + let set_result = SetNamedSecurityInfoW( + PCWSTR(wide.as_ptr()), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + None, + None, + Some(new_dacl), + None, + ); + if set_result != WIN32_ERROR::from(ERROR_SUCCESS) { + if !new_dacl.is_null() { + let _ = LocalFree(HLOCAL(new_dacl.cast())); + } + if !security_descriptor.is_invalid() { + let _ = LocalFree(HLOCAL(security_descriptor.0)); + } + return Err(io::Error::from_raw_os_error(set_result.0 as i32)); } - return Err(io::Error::from_raw_os_error(add_result.0 as i32)); - } - let set_result = SetNamedSecurityInfoW( - PCWSTR(wide.as_ptr()), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, - None, - None, - Some(new_dacl), - None, - ); - if set_result != WIN32_ERROR::from(ERROR_SUCCESS) { if !new_dacl.is_null() { let _ = LocalFree(HLOCAL(new_dacl.cast())); } if !security_descriptor.is_invalid() { let _ = LocalFree(HLOCAL(security_descriptor.0)); } - return Err(io::Error::from_raw_os_error(set_result.0 as i32)); } - if !new_dacl.is_null() { - let _ = LocalFree(HLOCAL(new_dacl.cast())); - } - if !security_descriptor.is_invalid() { - let _ = LocalFree(HLOCAL(security_descriptor.0)); - } + Ok(()) } +} + +#[cfg(feature = "windows_appcontainer_command_ext")] +pub use imp::spawn_command_under_windows_appcontainer; - Ok(()) +#[cfg(not(feature = "windows_appcontainer_command_ext"))] +pub async fn spawn_command_under_windows_appcontainer( + command: Vec, + command_cwd: PathBuf, + _sandbox_policy: &SandboxPolicy, + _sandbox_policy_cwd: &Path, + _stdio_policy: StdioPolicy, + _env: HashMap, +) -> io::Result { + let _ = (command, command_cwd); + Err(io::Error::new( + io::ErrorKind::Unsupported, + "AppContainer sandboxing requires the `windows_appcontainer_command_ext` feature", + )) } diff --git a/codex-rs/core/tests/windows_appcontainer.rs b/codex-rs/core/tests/windows_appcontainer.rs index 024804c1c2..b90216acec 100644 --- a/codex-rs/core/tests/windows_appcontainer.rs +++ b/codex-rs/core/tests/windows_appcontainer.rs @@ -1,4 +1,4 @@ -#![cfg(windows)] +#![cfg(all(windows, feature = "windows_appcontainer_command_ext"))] use codex_core::protocol::SandboxPolicy; use codex_core::spawn::StdioPolicy; From 8134d9eaadf85b691e85e795ac6da47838c51b7b Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 13:43:09 -0700 Subject: [PATCH 5/8] fix copy/paste error --- codex-rs/core/src/windows_appcontainer.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index abec508918..c1fd9eeaa2 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -1,8 +1,15 @@ +#![cfg(windows)] + +use std::collections::HashMap; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +use tokio::process::Child; use tracing::trace; use crate::protocol::SandboxPolicy; use crate::spawn::StdioPolicy; - #[cfg(feature = "windows_appcontainer_command_ext")] mod imp { use super::*; From 6f3f78a7168456e62b6a2ea6852aa07a741ab64e Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 14:13:59 -0700 Subject: [PATCH 6/8] fix build errors. --- codex-rs/core/src/lib.rs | 4 ++++ codex-rs/core/src/windows_appcontainer.rs | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 0b184e79d1..ec8d66f6e2 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -1,5 +1,9 @@ //! Root of the `codex-core` library. +#![cfg_attr( + all(windows, feature = "windows_appcontainer_command_ext"), + feature(windows_process_extensions_raw_attribute) +)] // Prevent accidental direct writes to stdout/stderr in library code. All // user-visible output must go through the appropriate abstraction (e.g., // the TUI or the tracing stack). diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index c1fd9eeaa2..737135019e 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -1,5 +1,3 @@ -#![cfg(windows)] - use std::collections::HashMap; use std::io; use std::path::Path; @@ -120,7 +118,7 @@ mod imp { unsafe { let std_cmd = cmd.as_std_mut(); std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT.0); - std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0); + std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0.cast()); } let child = cmd.spawn(); @@ -392,7 +390,7 @@ mod imp { let explicit = EXPLICIT_ACCESS_W { grfAccessPermissions: permissions, grfAccessMode: SET_ACCESS, - grfInheritance: (SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE).0, + grfInheritance: SUB_CONTAINERS_AND_OBJECTS_INHERIT | OBJECT_INHERIT_ACE, Trustee: TRUSTEE_W { TrusteeForm: TRUSTEE_IS_SID, TrusteeType: TRUSTEE_IS_UNKNOWN, From b706fad1b63b638964bbfaf882502d323bd54488 Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 15:07:31 -0700 Subject: [PATCH 7/8] more build fixes. --- codex-rs/core/Cargo.toml | 3 +++ codex-rs/core/src/lib.rs | 2 +- codex-rs/core/src/windows_appcontainer.rs | 30 +++++++++++++++++++-- codex-rs/core/tests/windows_appcontainer.rs | 6 ++++- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index a895d4f563..fc57a6131e 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -93,6 +93,9 @@ windows = { version = "0.58", features = [ [features] default = [] windows_appcontainer_command_ext = [] +windows_appcontainer_command_ext_raw_attribute = [ + "windows_appcontainer_command_ext", +] # Build OpenSSL from source for musl builds. [target.x86_64-unknown-linux-musl.dependencies] diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index ec8d66f6e2..e91e2e4732 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -1,7 +1,7 @@ //! Root of the `codex-core` library. #![cfg_attr( - all(windows, feature = "windows_appcontainer_command_ext"), + all(windows, feature = "windows_appcontainer_command_ext_raw_attribute"), feature(windows_process_extensions_raw_attribute) )] // Prevent accidental direct writes to stdout/stderr in library code. All diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index 737135019e..3a02295fbd 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -8,7 +8,11 @@ use tracing::trace; use crate::protocol::SandboxPolicy; use crate::spawn::StdioPolicy; -#[cfg(feature = "windows_appcontainer_command_ext")] + +#[cfg(all( + feature = "windows_appcontainer_command_ext", + feature = "windows_appcontainer_command_ext_raw_attribute", +))] mod imp { use super::*; @@ -444,9 +448,31 @@ mod imp { } } -#[cfg(feature = "windows_appcontainer_command_ext")] +#[cfg(all( + feature = "windows_appcontainer_command_ext", + feature = "windows_appcontainer_command_ext_raw_attribute", +))] pub use imp::spawn_command_under_windows_appcontainer; +#[cfg(all( + feature = "windows_appcontainer_command_ext", + not(feature = "windows_appcontainer_command_ext_raw_attribute"), +))] +pub async fn spawn_command_under_windows_appcontainer( + command: Vec, + command_cwd: PathBuf, + _sandbox_policy: &SandboxPolicy, + _sandbox_policy_cwd: &Path, + _stdio_policy: StdioPolicy, + _env: HashMap, +) -> io::Result { + let _ = (command, command_cwd); + Err(io::Error::new( + io::ErrorKind::Unsupported, + "AppContainer sandboxing requires the `windows_appcontainer_command_ext_raw_attribute` feature, which in turn needs a nightly compiler", + )) +} + #[cfg(not(feature = "windows_appcontainer_command_ext"))] pub async fn spawn_command_under_windows_appcontainer( command: Vec, diff --git a/codex-rs/core/tests/windows_appcontainer.rs b/codex-rs/core/tests/windows_appcontainer.rs index b90216acec..e8d5196f03 100644 --- a/codex-rs/core/tests/windows_appcontainer.rs +++ b/codex-rs/core/tests/windows_appcontainer.rs @@ -1,4 +1,8 @@ -#![cfg(all(windows, feature = "windows_appcontainer_command_ext"))] +#![cfg(all( + windows, + feature = "windows_appcontainer_command_ext", + feature = "windows_appcontainer_command_ext_raw_attribute", +))] use codex_core::protocol::SandboxPolicy; use codex_core::spawn::StdioPolicy; From db7950e68a2b0bc758d25a4c47990a6ea1506cab Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 7 Oct 2025 15:36:31 -0700 Subject: [PATCH 8/8] build errors. --- codex-rs/core/Cargo.toml | 3 +++ codex-rs/core/src/lib.rs | 2 +- codex-rs/core/src/windows_appcontainer.rs | 29 +++++++++++++++++++-- codex-rs/core/tests/windows_appcontainer.rs | 1 + 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index fc57a6131e..8e71bcdaf4 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -96,6 +96,9 @@ windows_appcontainer_command_ext = [] windows_appcontainer_command_ext_raw_attribute = [ "windows_appcontainer_command_ext", ] +windows_appcontainer_raw_attribute_api = [ + "windows_appcontainer_command_ext_raw_attribute", +] # Build OpenSSL from source for musl builds. [target.x86_64-unknown-linux-musl.dependencies] diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e91e2e4732..a49849159a 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -1,7 +1,7 @@ //! Root of the `codex-core` library. #![cfg_attr( - all(windows, feature = "windows_appcontainer_command_ext_raw_attribute"), + all(windows, feature = "windows_appcontainer_raw_attribute_api"), feature(windows_process_extensions_raw_attribute) )] // Prevent accidental direct writes to stdout/stderr in library code. All diff --git a/codex-rs/core/src/windows_appcontainer.rs b/codex-rs/core/src/windows_appcontainer.rs index 3a02295fbd..569e8cfe01 100644 --- a/codex-rs/core/src/windows_appcontainer.rs +++ b/codex-rs/core/src/windows_appcontainer.rs @@ -72,6 +72,27 @@ mod imp { use windows::core::PCWSTR; use windows::core::PWSTR; + #[cfg(feature = "windows_appcontainer_raw_attribute_api")] + unsafe fn attach_attribute_list( + std_cmd: &mut std::process::Command, + attribute_list: LPPROC_THREAD_ATTRIBUTE_LIST, + ) -> io::Result<()> { + std_cmd.raw_attribute_list(attribute_list.0.cast()); + Ok(()) + } + + #[cfg(not(feature = "windows_appcontainer_raw_attribute_api"))] + unsafe fn attach_attribute_list( + _std_cmd: &mut std::process::Command, + _attribute_list: LPPROC_THREAD_ATTRIBUTE_LIST, + ) -> io::Result<()> { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "AppContainer raw attribute injection requires the \ +`windows_appcontainer_raw_attribute_api` feature, which depends on nightly Rust", + )) + } + const WINDOWS_APPCONTAINER_PROFILE_NAME: &str = "codex_appcontainer"; const WINDOWS_APPCONTAINER_PROFILE_DESC: &str = "Codex Windows AppContainer profile"; const WINDOWS_APPCONTAINER_SANDBOX_VALUE: &str = "windows_appcontainer"; @@ -122,7 +143,10 @@ mod imp { unsafe { let std_cmd = cmd.as_std_mut(); std_cmd.creation_flags(EXTENDED_STARTUPINFO_PRESENT.0); - std_cmd.raw_attribute_list(attribute_list.as_mut_ptr().0.cast()); + if let Err(err) = attach_attribute_list(std_cmd, attribute_list.as_mut_ptr()) { + drop(attribute_list); + return Err(err); + } } let child = cmd.spawn(); @@ -143,6 +167,7 @@ mod imp { cmd.stderr(std::process::Stdio::inherit()); } } + Ok(()) } fn to_wide>(s: S) -> Vec { @@ -469,7 +494,7 @@ pub async fn spawn_command_under_windows_appcontainer( let _ = (command, command_cwd); Err(io::Error::new( io::ErrorKind::Unsupported, - "AppContainer sandboxing requires the `windows_appcontainer_command_ext_raw_attribute` feature, which in turn needs a nightly compiler", + "AppContainer sandboxing requires the `windows_appcontainer_raw_attribute_api` feature, which depends on nightly Rust", )) } diff --git a/codex-rs/core/tests/windows_appcontainer.rs b/codex-rs/core/tests/windows_appcontainer.rs index e8d5196f03..1d1d9d4c68 100644 --- a/codex-rs/core/tests/windows_appcontainer.rs +++ b/codex-rs/core/tests/windows_appcontainer.rs @@ -2,6 +2,7 @@ windows, feature = "windows_appcontainer_command_ext", feature = "windows_appcontainer_command_ext_raw_attribute", + feature = "windows_appcontainer_raw_attribute_api", ))] use codex_core::protocol::SandboxPolicy;