Address Windows fs helper review feedback

codex-rs/exec-server/src/fs_sandbox.rs

@@ -30,7 +30,8 @@ use crate::rpc::invalid_request;
 
 const FS_HELPER_ENV_ALLOWLIST: &[&str] = &["PATH", "TMPDIR", "TMP", "TEMP"];
 #[cfg(target_os = "windows")]
-const WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST: &[&str] = &["USERNAME", "USERPROFILE"];
+#[path = "fs_sandbox_windows.rs"]
+mod windows;
 #[cfg(debug_assertions)]
 const FS_HELPER_BAZEL_BWRAP_ENV_ALLOWLIST: &[&str] = &[
     "CARGO_BIN_EXE_bwrap",
@@ -169,7 +170,7 @@ impl FileSystemSandboxRunner {
         #[cfg(target_os = "windows")]
         if command.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken {
             let request_file =
-                wrap_windows_sandbox_command(&mut command, &helper_paths.windows_wrapper)?;
+                windows::wrap_sandbox_exec_request(&mut command, &helper_paths.windows_wrapper)?;
             return Ok(PreparedFsHelperCommand::with_windows_wrapper_request_file(
                 command,
                 request_file,
@@ -188,7 +189,7 @@ struct FsHelperLaunchPaths {
 struct PreparedFsHelperCommand {
     request: SandboxExecRequest,
     #[cfg(target_os = "windows")]
-    _windows_wrapper_request_file: Option<WindowsSandboxWrapperRequestFile>,
+    _windows_wrapper_request_file: Option<windows::WindowsSandboxWrapperRequestFile>,
 }
 
 impl PreparedFsHelperCommand {
@@ -203,7 +204,7 @@ impl PreparedFsHelperCommand {
     #[cfg(target_os = "windows")]
     fn with_windows_wrapper_request_file(
         request: SandboxExecRequest,
-        request_file: WindowsSandboxWrapperRequestFile,
+        request_file: windows::WindowsSandboxWrapperRequestFile,
     ) -> Self {
         Self {
             request,
@@ -421,132 +422,6 @@ fn spawn_child_command(
     command.spawn().map_err(io_error)
 }
 
-#[cfg(target_os = "windows")]
-fn wrap_windows_sandbox_command(
-    request: &mut SandboxExecRequest,
-    helper: &AbsolutePathBuf,
-) -> Result<WindowsSandboxWrapperRequestFile, JSONRPCErrorError> {
-    let codex_home = codex_utils_home_dir::find_codex_home().map_err(|err| {
-        internal_error(format!(
-            "windows fs sandbox helper failed to resolve CODEX_HOME: {err}"
-        ))
-    })?;
-    let sandboxed_env = request.env.clone();
-    let wrapper_request =
-        codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile(
-            std::mem::take(&mut request.command),
-            request.cwd.clone(),
-            sandboxed_env,
-            request.permission_profile.clone(),
-            request.windows_sandbox_level,
-            request.windows_sandbox_private_desktop,
-            codex_home.to_path_buf(),
-        );
-    let request_file =
-        WindowsSandboxWrapperRequestFile::create(codex_home.as_path(), &wrapper_request)?;
-    let mut args = codex_windows_sandbox::create_windows_sandbox_command_args_for_request_file(
-        request_file.path.as_path(),
-    );
-    request.command = Vec::with_capacity(1 + args.len());
-    request
-        .command
-        .push(helper.as_path().to_string_lossy().into_owned());
-    request.command.append(&mut args);
-    request.sandbox = codex_sandboxing::SandboxType::None;
-    request.arg0 = None;
-    add_windows_wrapper_setup_env(&mut request.env);
-    Ok(request_file)
-}
-
-#[cfg(target_os = "windows")]
-fn add_windows_wrapper_setup_env(env: &mut HashMap<String, String>) {
-    add_windows_wrapper_setup_env_from_vars(env, std::env::vars_os());
-}
-
-#[cfg(target_os = "windows")]
-fn add_windows_wrapper_setup_env_from_vars(
-    env: &mut HashMap<String, String>,
-    vars: impl IntoIterator<Item = (std::ffi::OsString, std::ffi::OsString)>,
-) {
-    for (key, value) in vars {
-        let key = key.to_string_lossy().into_owned();
-        if !WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST
-            .iter()
-            .any(|allowed| key.eq_ignore_ascii_case(allowed))
-        {
-            continue;
-        }
-        if env
-            .keys()
-            .any(|existing| existing.eq_ignore_ascii_case(&key))
-        {
-            continue;
-        }
-        env.insert(key, value.to_string_lossy().into_owned());
-    }
-}
-
-#[cfg(target_os = "windows")]
-struct WindowsSandboxWrapperRequestFile {
-    path: std::path::PathBuf,
-}
-
-#[cfg(target_os = "windows")]
-impl WindowsSandboxWrapperRequestFile {
-    fn create(
-        codex_home: &std::path::Path,
-        request: &codex_windows_sandbox::WindowsSandboxWrapperRequest,
-    ) -> Result<Self, JSONRPCErrorError> {
-        let request_dir = windows_sandbox_wrapper_request_dir(codex_home);
-        std::fs::create_dir_all(&request_dir).map_err(|err| {
-            internal_error(format!(
-                "failed to create windows fs sandbox wrapper request dir {}: {err}",
-                request_dir.display()
-            ))
-        })?;
-        codex_windows_sandbox::ensure_current_user_cleanup_access(&request_dir).map_err(|err| {
-            internal_error(format!(
-                "failed to grant cleanup access to windows fs sandbox wrapper request dir {}: {err}",
-                request_dir.display()
-            ))
-        })?;
-        let path = request_dir.join(format!(
-            "fs-helper-wrapper-request-{}.json",
-            uuid::Uuid::new_v4()
-        ));
-        let request_json = serde_json::to_vec(request).map_err(json_error)?;
-        let mut file = std::fs::OpenOptions::new()
-            .write(true)
-            .create_new(true)
-            .open(&path)
-            .map_err(|err| {
-                internal_error(format!(
-                    "failed to create windows fs sandbox wrapper request file {}: {err}",
-                    path.display()
-                ))
-            })?;
-        std::io::Write::write_all(&mut file, &request_json).map_err(|err| {
-            internal_error(format!(
-                "failed to write windows fs sandbox wrapper request file {}: {err}",
-                path.display()
-            ))
-        })?;
-        Ok(Self { path })
-    }
-}
-
-#[cfg(target_os = "windows")]
-fn windows_sandbox_wrapper_request_dir(codex_home: &std::path::Path) -> std::path::PathBuf {
-    codex_windows_sandbox::sandbox_secrets_dir(codex_home).join("wrapper-requests")
-}
-
-#[cfg(target_os = "windows")]
-impl Drop for WindowsSandboxWrapperRequestFile {
-    fn drop(&mut self) {
-        let _ = std::fs::remove_file(&self.path);
-    }
-}
-
 fn io_error(err: std::io::Error) -> JSONRPCErrorError {
     internal_error(err.to_string())
 }
@@ -577,15 +452,11 @@ mod tests {
     use super::FileSystemSandboxRunner;
     use super::FsHelperLaunchPaths;
     use super::add_helper_runtime_permissions;
-    #[cfg(target_os = "windows")]
-    use super::add_windows_wrapper_setup_env_from_vars;
     use super::helper_env;
     use super::helper_env_from_vars;
     use super::helper_env_key_is_allowed;
     use super::helper_read_roots;
     use super::sandbox_cwd;
-    #[cfg(target_os = "windows")]
-    use super::windows_sandbox_wrapper_request_dir;
 
     #[test]
     fn helper_permissions_enable_minimal_reads_for_restricted_profile() {
@@ -725,43 +596,6 @@ mod tests {
         );
     }
 
-    #[cfg(target_os = "windows")]
-    #[test]
-    fn windows_wrapper_setup_env_preserves_only_setup_identity() {
-        let mut env = HashMap::from([("PATH".to_string(), r"C:\Windows\System32".to_string())]);
-
-        add_windows_wrapper_setup_env_from_vars(
-            &mut env,
-            [
-                ("USERNAME", "alice"),
-                ("USERPROFILE", r"C:\Users\alice"),
-                ("OPENAI_API_KEY", "secret"),
-            ]
-            .map(|(key, value)| (OsString::from(key), OsString::from(value))),
-        );
-
-        assert_eq!(
-            env,
-            HashMap::from([
-                ("PATH".to_string(), r"C:\Windows\System32".to_string()),
-                ("USERNAME".to_string(), "alice".to_string()),
-                ("USERPROFILE".to_string(), r"C:\Users\alice".to_string()),
-            ])
-        );
-    }
-
-    #[cfg(target_os = "windows")]
-    #[test]
-    fn windows_wrapper_request_dir_uses_sandbox_secrets() {
-        let codex_home = std::env::temp_dir().join("codex-home");
-        let sandbox_dir = codex_windows_sandbox::sandbox_dir(&codex_home);
-        let secrets_dir = codex_windows_sandbox::sandbox_secrets_dir(&codex_home);
-        let request_dir = windows_sandbox_wrapper_request_dir(&codex_home);
-
-        assert!(!request_dir.starts_with(sandbox_dir));
-        assert!(request_dir.starts_with(secrets_dir));
-    }
-
     #[test]
     fn sandbox_exec_request_carries_helper_env() {
         let Some((path_key, path)) = std::env::vars_os().find(|(key, _)| {

codex-rs/exec-server/src/fs_sandbox_windows.rs

@@ -0,0 +1,212 @@
+use std::collections::HashMap;
+use std::io::Write;
+
+use codex_app_server_protocol::JSONRPCErrorError;
+use codex_sandboxing::SandboxExecRequest;
+use codex_utils_absolute_path::AbsolutePathBuf;
+
+use crate::rpc::internal_error;
+
+const WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST: &[&str] = &["USERNAME", "USERPROFILE"];
+
+pub(super) fn wrap_sandbox_exec_request(
+    request: &mut SandboxExecRequest,
+    helper: &AbsolutePathBuf,
+) -> Result<WindowsSandboxWrapperRequestFile, JSONRPCErrorError> {
+    let codex_home = codex_utils_home_dir::find_codex_home().map_err(|err| {
+        internal_error(format!(
+            "windows fs sandbox helper failed to resolve CODEX_HOME: {err}"
+        ))
+    })?;
+    let sandboxed_env = request.env.clone();
+    let wrapper_request =
+        codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile(
+            std::mem::take(&mut request.command),
+            request.cwd.clone(),
+            sandboxed_env,
+            request.permission_profile.clone(),
+            request.windows_sandbox_level,
+            request.windows_sandbox_private_desktop,
+            codex_home.to_path_buf(),
+        );
+    let request_file =
+        WindowsSandboxWrapperRequestFile::create(codex_home.as_path(), &wrapper_request)?;
+    let mut args = codex_windows_sandbox::create_windows_sandbox_command_args_for_request_file(
+        request_file.path.as_path(),
+    );
+    request.command = Vec::with_capacity(1 + args.len());
+    request
+        .command
+        .push(helper.as_path().to_string_lossy().into_owned());
+    request.command.append(&mut args);
+    request.sandbox = codex_sandboxing::SandboxType::None;
+    request.arg0 = None;
+    add_wrapper_setup_env(&mut request.env);
+    Ok(request_file)
+}
+
+fn add_wrapper_setup_env(env: &mut HashMap<String, String>) {
+    add_wrapper_setup_env_from_vars(env, std::env::vars_os());
+}
+
+fn add_wrapper_setup_env_from_vars(
+    env: &mut HashMap<String, String>,
+    vars: impl IntoIterator<Item = (std::ffi::OsString, std::ffi::OsString)>,
+) {
+    for (key, value) in vars {
+        let key = key.to_string_lossy().into_owned();
+        if !WINDOWS_SANDBOX_WRAPPER_SETUP_ENV_ALLOWLIST
+            .iter()
+            .any(|allowed| key.eq_ignore_ascii_case(allowed))
+        {
+            continue;
+        }
+        if env
+            .keys()
+            .any(|existing| existing.eq_ignore_ascii_case(&key))
+        {
+            continue;
+        }
+        env.insert(key, value.to_string_lossy().into_owned());
+    }
+}
+
+pub(super) struct WindowsSandboxWrapperRequestFile {
+    path: std::path::PathBuf,
+}
+
+impl WindowsSandboxWrapperRequestFile {
+    fn create(
+        codex_home: &std::path::Path,
+        request: &codex_windows_sandbox::WindowsSandboxWrapperRequest,
+    ) -> Result<Self, JSONRPCErrorError> {
+        let request_dir = wrapper_request_dir(codex_home);
+        std::fs::create_dir_all(&request_dir).map_err(|err| {
+            internal_error(format!(
+                "failed to create windows fs sandbox wrapper request dir {}: {err}",
+                request_dir.display()
+            ))
+        })?;
+        codex_windows_sandbox::ensure_current_user_cleanup_access(&request_dir).map_err(|err| {
+            internal_error(format!(
+                "failed to grant cleanup access to windows fs sandbox wrapper request dir {}: {err}",
+                request_dir.display()
+            ))
+        })?;
+        let path = request_dir.join(format!(
+            "fs-helper-wrapper-request-{}.json",
+            uuid::Uuid::new_v4()
+        ));
+        let request_json = serde_json::to_vec(request).map_err(|err| {
+            internal_error(format!(
+                "failed to encode or decode fs sandbox helper message: {err}"
+            ))
+        })?;
+        let mut file = std::fs::OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(&path)
+            .map_err(|err| {
+                internal_error(format!(
+                    "failed to create windows fs sandbox wrapper request file {}: {err}",
+                    path.display()
+                ))
+            })?;
+        file.write_all(&request_json).map_err(|err| {
+            internal_error(format!(
+                "failed to write windows fs sandbox wrapper request file {}: {err}",
+                path.display()
+            ))
+        })?;
+        Ok(Self { path })
+    }
+}
+
+fn wrapper_request_dir(codex_home: &std::path::Path) -> std::path::PathBuf {
+    codex_windows_sandbox::sandbox_secrets_dir(codex_home).join("wrapper-requests")
+}
+
+impl Drop for WindowsSandboxWrapperRequestFile {
+    fn drop(&mut self) {
+        let _ = std::fs::remove_file(&self.path);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashMap;
+    use std::ffi::OsString;
+
+    use codex_protocol::config_types::WindowsSandboxLevel;
+    use codex_protocol::models::PermissionProfile;
+    use codex_protocol::permissions::NetworkSandboxPolicy;
+    use codex_utils_absolute_path::AbsolutePathBuf;
+    use pretty_assertions::assert_eq;
+
+    use super::WindowsSandboxWrapperRequestFile;
+    use super::add_wrapper_setup_env_from_vars;
+    use super::wrapper_request_dir;
+
+    #[test]
+    fn wrapper_setup_env_preserves_only_setup_identity() {
+        let mut env = HashMap::from([("PATH".to_string(), r"C:\Windows\System32".to_string())]);
+
+        add_wrapper_setup_env_from_vars(
+            &mut env,
+            [
+                ("USERNAME", "alice"),
+                ("USERPROFILE", r"C:\Users\alice"),
+                ("OPENAI_API_KEY", "secret"),
+            ]
+            .map(|(key, value)| (OsString::from(key), OsString::from(value))),
+        );
+
+        assert_eq!(
+            env,
+            HashMap::from([
+                ("PATH".to_string(), r"C:\Windows\System32".to_string()),
+                ("USERNAME".to_string(), "alice".to_string()),
+                ("USERPROFILE".to_string(), r"C:\Users\alice".to_string()),
+            ])
+        );
+    }
+
+    #[test]
+    fn wrapper_request_dir_uses_sandbox_secrets() {
+        let codex_home = std::env::temp_dir().join("codex-home");
+        let sandbox_dir = codex_windows_sandbox::sandbox_dir(&codex_home);
+        let secrets_dir = codex_windows_sandbox::sandbox_secrets_dir(&codex_home);
+        let request_dir = wrapper_request_dir(&codex_home);
+
+        assert!(!request_dir.starts_with(sandbox_dir));
+        assert!(request_dir.starts_with(secrets_dir));
+    }
+
+    #[test]
+    fn wrapper_request_file_is_removed_on_drop() {
+        let codex_home = tempfile::TempDir::new().expect("codex home");
+        let command_cwd =
+            AbsolutePathBuf::from_absolute_path(codex_home.path()).expect("absolute command cwd");
+        let request =
+            codex_windows_sandbox::create_windows_sandbox_wrapper_request_for_permission_profile(
+                vec!["helper.exe".to_string()],
+                command_cwd,
+                HashMap::new(),
+                PermissionProfile::External {
+                    network: NetworkSandboxPolicy::Restricted,
+                },
+                WindowsSandboxLevel::RestrictedToken,
+                /*windows_sandbox_private_desktop*/ false,
+                codex_home.path().to_path_buf(),
+            );
+
+        let request_file = WindowsSandboxWrapperRequestFile::create(codex_home.path(), &request)
+            .expect("create wrapper request file");
+        let path = request_file.path.clone();
+        assert!(path.exists());
+
+        drop(request_file);
+
+        assert!(!path.exists());
+    }
+}

codex-rs/exec-server/tests/windows_file_system_sandbox.rs

@@ -305,7 +305,7 @@ async fn elevated_fs_helper_rejects_unenforceable_reopened_writable_descendant()
 #[serial(codex_home)]
 async fn elevated_fs_helper_allows_writable_path_when_setup_is_available() -> Result<()> {
     if !process_is_elevated() {
-        eprintln!("skipping elevated fs-helper test: test process is not elevated");
+        eprintln!("skipping elevated fs-helper write: test process is not elevated");
         return Ok(());
     }
 

codex-rs/windows-sandbox-rs/src/acl.rs

@@ -492,28 +492,8 @@ pub unsafe fn add_allow_ace(path: &Path, psid: *mut c_void) -> Result<bool> {
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn current_user_cleanup_access_grants_delete_rights() -> Result<()> {
-        let temp_dir = tempfile::TempDir::new()?;
-        let request_dir = temp_dir.path().join("wrapper-requests");
-        std::fs::create_dir(&request_dir)?;
-
-        ensure_current_user_cleanup_access(&request_dir)?;
-
-        let real_user = std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string());
-        let real_user_sid = resolve_sid(&real_user)?;
-        assert!(path_mask_allows(
-            &request_dir,
-            &[real_user_sid.as_ptr() as *mut c_void],
-            DELETE | FILE_DELETE_CHILD,
-            /*require_all_bits*/ true,
-        )?);
-        Ok(())
-    }
-}
+#[path = "acl_tests.rs"]
+mod tests;
 
 /// Adds a deny ACE to prevent write/append/delete for the given SID on the target path.
 ///

codex-rs/windows-sandbox-rs/src/acl_tests.rs

@@ -0,0 +1,121 @@
+use std::ffi::c_void;
+use std::path::Path;
+
+use anyhow::Result;
+use anyhow::anyhow;
+use windows_sys::Win32::Foundation::CloseHandle;
+use windows_sys::Win32::Foundation::ERROR_SUCCESS;
+use windows_sys::Win32::Foundation::HLOCAL;
+use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
+use windows_sys::Win32::Foundation::LocalFree;
+use windows_sys::Win32::Security::ACL;
+use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W;
+use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
+use windows_sys::Win32::Security::Authorization::SetSecurityInfo;
+use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
+use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN;
+use windows_sys::Win32::Security::Authorization::TRUSTEE_W;
+use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
+use windows_sys::Win32::Security::PROTECTED_DACL_SECURITY_INFORMATION;
+use windows_sys::Win32::Storage::FileSystem::CreateFileW;
+use windows_sys::Win32::Storage::FileSystem::DELETE;
+use windows_sys::Win32::Storage::FileSystem::FILE_DELETE_CHILD;
+use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
+use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE;
+use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ;
+use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
+use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE;
+use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
+use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
+use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
+
+use crate::acl::ensure_current_user_cleanup_access;
+use crate::acl::path_mask_allows;
+use crate::winutil::resolve_sid;
+use crate::winutil::to_wide;
+
+const CONTAINER_INHERIT_ACE: u32 = 0x2;
+const OBJECT_INHERIT_ACE: u32 = 0x1;
+
+#[test]
+fn current_user_cleanup_access_grants_delete_rights() -> Result<()> {
+    let temp_dir = tempfile::TempDir::new()?;
+    let request_dir = temp_dir.path().join("wrapper-requests");
+    std::fs::create_dir(&request_dir)?;
+
+    let real_user = std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string());
+    let real_user_sid = resolve_sid(&real_user)?;
+    let real_user_sid = real_user_sid.as_ptr() as *mut c_void;
+    set_delete_less_allow_ace_for_test(&request_dir, real_user_sid)?;
+    assert!(!path_mask_allows(
+        &request_dir,
+        &[real_user_sid],
+        DELETE | FILE_DELETE_CHILD,
+        /*require_all_bits*/ true,
+    )?);
+
+    ensure_current_user_cleanup_access(&request_dir)?;
+
+    assert!(path_mask_allows(
+        &request_dir,
+        &[real_user_sid],
+        DELETE | FILE_DELETE_CHILD,
+        /*require_all_bits*/ true,
+    )?);
+    Ok(())
+}
+
+fn set_delete_less_allow_ace_for_test(path: &Path, psid: *mut c_void) -> Result<()> {
+    unsafe {
+        let trustee = TRUSTEE_W {
+            pMultipleTrustee: std::ptr::null_mut(),
+            MultipleTrusteeOperation: 0,
+            TrusteeForm: TRUSTEE_IS_SID,
+            TrusteeType: TRUSTEE_IS_UNKNOWN,
+            ptstrName: psid as *mut u16,
+        };
+        let mut explicit: EXPLICIT_ACCESS_W = std::mem::zeroed();
+        explicit.grfAccessPermissions =
+            FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE;
+        explicit.grfAccessMode = 2; // SET_ACCESS
+        explicit.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;
+        explicit.Trustee = trustee;
+        let mut p_new_dacl: *mut ACL = std::ptr::null_mut();
+        let code = SetEntriesInAclW(1, &explicit, std::ptr::null_mut(), &mut p_new_dacl);
+        if code != ERROR_SUCCESS {
+            return Err(anyhow!("SetEntriesInAclW failed: {code}"));
+        }
+
+        let desired = 0x00020000 | 0x00040000; // READ_CONTROL | WRITE_DAC
+        let h = CreateFileW(
+            to_wide(path).as_ptr(),
+            desired,
+            FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+            std::ptr::null_mut(),
+            OPEN_EXISTING,
+            FILE_FLAG_BACKUP_SEMANTICS,
+            0,
+        );
+        if h == 0 || h == INVALID_HANDLE_VALUE {
+            LocalFree(p_new_dacl as HLOCAL);
+            return Err(anyhow!("CreateFileW failed for {}", path.display()));
+        }
+        let code = SetSecurityInfo(
+            h,
+            1,
+            DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION,
+            std::ptr::null_mut(),
+            std::ptr::null_mut(),
+            p_new_dacl,
+            std::ptr::null_mut(),
+        );
+        CloseHandle(h);
+        if !p_new_dacl.is_null() {
+            LocalFree(p_new_dacl as HLOCAL);
+        }
+        if code != ERROR_SUCCESS {
+            return Err(anyhow!("SetSecurityInfo failed: {code}"));
+        }
+    }
+    Ok(())
+}

codex-rs/windows-sandbox-rs/src/wrapper.rs

@@ -91,18 +91,11 @@ async fn run_windows_sandbox_wrapper_args(args: Vec<String>) -> Result<i32> {
     run_windows_sandbox_wrapper_request(request).await
 }
 
-async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperRequest) -> Result<i32> {
-    if !request.codex_home.is_absolute() {
-        bail!(
-            "windows sandbox wrapper codex_home must be absolute: {}",
-            request.codex_home.display()
-        );
-    }
-    if request.command.is_empty() {
-        bail!("missing sandboxed command in windows sandbox wrapper request");
-    }
-
-    let env = request.env_map;
+async fn run_windows_sandbox_wrapper_request(
+    mut request: WindowsSandboxWrapperRequest,
+) -> Result<i32> {
+    validate_windows_sandbox_wrapper_request(&request)?;
+    let env_map = std::mem::take(&mut request.env_map);
     let workspace_roots = vec![request.command_cwd.clone()];
     let spawned = match request.windows_sandbox_level {
         WindowsSandboxLevel::Elevated => {
@@ -120,7 +113,7 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque
                 request.codex_home.as_path(),
                 request.command,
                 request.command_cwd.as_path(),
-                env,
+                env_map,
                 /*timeout_ms*/ None,
                 overrides.read_roots_override.as_deref(),
                 overrides.read_roots_include_platform_defaults,
@@ -148,7 +141,7 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque
                 request.codex_home.as_path(),
                 request.command,
                 request.command_cwd.as_path(),
-                env,
+                env_map,
                 /*timeout_ms*/ None,
                 &overrides.additional_deny_read_paths,
                 &overrides.additional_deny_write_paths,
@@ -163,6 +156,19 @@ async fn run_windows_sandbox_wrapper_request(request: WindowsSandboxWrapperReque
     Ok(crate::stdio_bridge::forward_sandbox_session_stdio(spawned).await)
 }
 
+fn validate_windows_sandbox_wrapper_request(request: &WindowsSandboxWrapperRequest) -> Result<()> {
+    if !request.codex_home.is_absolute() {
+        bail!(
+            "windows sandbox wrapper codex_home must be absolute: {}",
+            request.codex_home.display()
+        );
+    }
+    if request.command.is_empty() {
+        bail!("missing sandboxed command in windows sandbox wrapper request");
+    }
+    Ok(())
+}
+
 fn parse_windows_sandbox_wrapper_args(args: Vec<String>) -> Result<PathBuf> {
     let mut args = args.into_iter();
     let Some(flag) = args.next() else {
@@ -191,11 +197,24 @@ fn next_flag_value(args: &mut impl Iterator<Item = String>, flag: &str) -> Resul
 
 #[cfg(test)]
 mod tests {
+    use std::sync::Mutex;
+
     use codex_protocol::permissions::NetworkSandboxPolicy;
     use pretty_assertions::assert_eq;
+    use tempfile::TempDir;
+    use tokio::runtime::Builder;
 
     use super::*;
 
+    static WRAPPER_PROCESS_TEST_LOCK: Mutex<()> = Mutex::new(());
+
+    fn current_thread_runtime() -> tokio::runtime::Runtime {
+        Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .expect("build tokio runtime")
+    }
+
     #[test]
     fn windows_wrapper_args_reference_request_file_only() -> Result<()> {
         let args = create_windows_sandbox_command_args_for_request_file(Path::new(
@@ -256,4 +275,41 @@ mod tests {
         assert_eq!(request.windows_sandbox_private_desktop, true);
         Ok(())
     }
+
+    #[test]
+    fn windows_wrapper_request_env_reaches_sandboxed_command() -> Result<()> {
+        let _guard = WRAPPER_PROCESS_TEST_LOCK
+            .lock()
+            .expect("wrapper process test lock poisoned");
+        let runtime = current_thread_runtime();
+        runtime.block_on(async move {
+            let workspace = TempDir::new()?;
+            let workspace = AbsolutePathBuf::from_absolute_path(workspace.path())?;
+            let codex_home = TempDir::new()?;
+            let request = create_windows_sandbox_wrapper_request_for_permission_profile(
+                vec![
+                    r"C:\Windows\System32\cmd.exe".to_string(),
+                    "/d".to_string(),
+                    "/s".to_string(),
+                    "/c".to_string(),
+                    r#"if "%CODEX_WRAPPER_REQUEST_ENV%"=="from-request" (exit /b 0) else (exit /b 7)"#
+                        .to_string(),
+                ],
+                workspace,
+                HashMap::from([(
+                    "CODEX_WRAPPER_REQUEST_ENV".to_string(),
+                    "from-request".to_string(),
+                )]),
+                PermissionProfile::read_only(),
+                WindowsSandboxLevel::RestrictedToken,
+                /*windows_sandbox_private_desktop*/ true,
+                codex_home.path().to_path_buf(),
+            );
+
+            let exit_code = run_windows_sandbox_wrapper_request(request).await?;
+
+            assert_eq!(exit_code, 0);
+            Ok(())
+        })
+    }
 }