bug fixes and perf improvements for elevated sandbox setup

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

@@ -251,7 +251,7 @@ pub fn apply_capability_denies_for_world_writable(
     }
     std::fs::create_dir_all(codex_home)?;
     let cap_path = cap_sid_file(codex_home);
-    let caps = load_or_create_cap_sids(codex_home);
+    let caps = load_or_create_cap_sids(codex_home)?;
     std::fs::write(&cap_path, serde_json::to_string(&caps)?)?;
     let (active_sid, workspace_roots): (*mut c_void, Vec<PathBuf>) = match sandbox_policy {
         SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {

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

@@ -1,3 +1,5 @@
+use anyhow::Context;
+use anyhow::Result;
 use rand::rngs::SmallRng;
 use rand::RngCore;
 use rand::SeedableRng;
@@ -26,25 +28,39 @@ fn make_random_cap_sid_string() -> String {
     format!("S-1-5-21-{}-{}-{}-{}", a, b, c, d)
 }
 
-pub fn load_or_create_cap_sids(codex_home: &Path) -> CapSids {
+fn persist_caps(path: &Path, caps: &CapSids) -> Result<()> {
+    if let Some(dir) = path.parent() {
+        fs::create_dir_all(dir)
+            .with_context(|| format!("create cap sid dir {}", dir.display()))?;
+    }
+    let json = serde_json::to_string(caps)?;
+    fs::write(path, json).with_context(|| format!("write cap sid file {}", path.display()))?;
+    Ok(())
+}
+
+pub fn load_or_create_cap_sids(codex_home: &Path) -> Result<CapSids> {
     let path = cap_sid_file(codex_home);
     if path.exists() {
-        if let Ok(txt) = fs::read_to_string(&path) {
-            let t = txt.trim();
-            if t.starts_with('{') && t.ends_with('}') {
-                if let Ok(obj) = serde_json::from_str::<CapSids>(t) {
-                    return obj;
-                }
-            } else if !t.is_empty() {
-                return CapSids {
-                    workspace: t.to_string(),
-                    readonly: make_random_cap_sid_string(),
-                };
+        let txt = fs::read_to_string(&path)
+            .with_context(|| format!("read cap sid file {}", path.display()))?;
+        let t = txt.trim();
+        if t.starts_with('{') && t.ends_with('}') {
+            if let Ok(obj) = serde_json::from_str::<CapSids>(t) {
+                return Ok(obj);
             }
+        } else if !t.is_empty() {
+            let caps = CapSids {
+                workspace: t.to_string(),
+                readonly: make_random_cap_sid_string(),
+            };
+            persist_caps(&path, &caps)?;
+            return Ok(caps);
         }
     }
-    CapSids {
+    let caps = CapSids {
         workspace: make_random_cap_sid_string(),
         readonly: make_random_cap_sid_string(),
-    }
+    };
+    persist_caps(&path, &caps)?;
+    Ok(caps)
 }

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

@@ -2,7 +2,6 @@ mod windows_impl {
     use crate::acl::allow_null_device;
     use crate::allow::compute_allow_paths;
     use crate::allow::AllowDenyPaths;
-    use crate::cap::cap_sid_file;
     use crate::cap::load_or_create_cap_sids;
     use crate::env::ensure_non_interactive_pager;
     use crate::env::inherit_path_env;
@@ -53,13 +52,6 @@ mod windows_impl {
     use windows_sys::Win32::System::Threading::STARTUPINFOW;
 
     /// Ensures the parent directory of a path exists before writing to it.
-    fn ensure_dir(p: &Path) -> Result<()> {
-        if let Some(d) = p.parent() {
-            std::fs::create_dir_all(d)?;
-        }
-        Ok(())
-    }
-
     /// Walks upward from `start` to locate the git worktree root, following gitfile redirects.
     fn find_git_root(start: &Path) -> Option<PathBuf> {
         let mut cur = dunce::canonicalize(start).ok()?;
@@ -246,44 +238,26 @@ mod windows_impl {
         let sandbox_creds =
             require_logon_sandbox_creds(&policy, sandbox_policy_cwd, cwd, &env_map, codex_home)?;
         log_note("cli creds ready", logs_base_dir);
-        let cap_sid_path = cap_sid_file(codex_home);
-
         // Build capability SID for ACL grants.
+        if matches!(&policy, SandboxPolicy::DangerFullAccess) {
+            anyhow::bail!("DangerFullAccess is not supported for sandboxing")
+        }
+        let caps = load_or_create_cap_sids(codex_home)?;
         let (psid_to_use, cap_sid_str) = match &policy {
-            SandboxPolicy::ReadOnly => {
-                let caps = load_or_create_cap_sids(codex_home);
-                ensure_dir(&cap_sid_path)?;
-                fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
-                (
-                    unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() },
-                    caps.readonly.clone(),
-                )
-            }
-            SandboxPolicy::WorkspaceWrite { .. } => {
-                let caps = load_or_create_cap_sids(codex_home);
-                ensure_dir(&cap_sid_path)?;
-                fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
-                (
-                    unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() },
-                    caps.workspace.clone(),
-                )
-            }
-            SandboxPolicy::DangerFullAccess => {
-                anyhow::bail!("DangerFullAccess is not supported for sandboxing")
-            }
+            SandboxPolicy::ReadOnly => (
+                unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() },
+                caps.readonly.clone(),
+            ),
+            SandboxPolicy::WorkspaceWrite { .. } => (
+                unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() },
+                caps.workspace.clone(),
+            ),
+            SandboxPolicy::DangerFullAccess => unreachable!("DangerFullAccess handled above"),
         };
 
-        let AllowDenyPaths { allow, deny } =
+        let AllowDenyPaths { allow: _, deny: _ } =
             compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
         // Deny/allow ACEs are now applied during setup; avoid per-command churn.
-        log_note(
-            &format!(
-                "cli skipping per-command ACL grants (allow_count={} deny_count={})",
-                allow.len(),
-                deny.len()
-            ),
-            logs_base_dir,
-        );
         unsafe {
             allow_null_device(psid_to_use);
         }

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

@@ -85,7 +85,6 @@ mod windows_impl {
     use super::acl::revoke_ace;
     use super::allow::compute_allow_paths;
     use super::allow::AllowDenyPaths;
-    use super::cap::cap_sid_file;
     use super::cap::load_or_create_cap_sids;
     use super::env::apply_no_network_to_env;
     use super::env::ensure_non_interactive_pager;
@@ -104,7 +103,6 @@ mod windows_impl {
     use anyhow::Result;
     use std::collections::HashMap;
     use std::ffi::c_void;
-    use std::fs;
     use std::io;
     use std::path::Path;
     use std::path::PathBuf;
@@ -130,13 +128,6 @@ mod windows_impl {
         !policy.has_full_network_access()
     }
 
-    fn ensure_dir(p: &Path) -> Result<()> {
-        if let Some(d) = p.parent() {
-            std::fs::create_dir_all(d)?;
-        }
-        Ok(())
-    }
-
     fn ensure_codex_home_exists(p: &Path) -> Result<()> {
         std::fs::create_dir_all(p)?;
         Ok(())
@@ -194,32 +185,28 @@ mod windows_impl {
             apply_no_network_to_env(&mut env_map)?;
         }
         ensure_codex_home_exists(codex_home)?;
-
         let current_dir = cwd.to_path_buf();
-        let logs_base_dir = Some(codex_home);
+        let sandbox_base = codex_home.join(".sandbox");
+        std::fs::create_dir_all(&sandbox_base)?;
+        let logs_base_dir = Some(sandbox_base.as_path());
         log_start(&command, logs_base_dir);
-        let cap_sid_path = cap_sid_file(codex_home);
         let is_workspace_write = matches!(&policy, SandboxPolicy::WorkspaceWrite { .. });
 
+        if matches!(&policy, SandboxPolicy::DangerFullAccess) {
+            anyhow::bail!("DangerFullAccess is not supported for sandboxing")
+        }
+        let caps = load_or_create_cap_sids(codex_home)?;
         let (h_token, psid_to_use): (HANDLE, *mut c_void) = unsafe {
             match &policy {
                 SandboxPolicy::ReadOnly => {
-                    let caps = load_or_create_cap_sids(codex_home);
-                    ensure_dir(&cap_sid_path)?;
-                    fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
                     let psid = convert_string_sid_to_sid(&caps.readonly).unwrap();
                     super::token::create_readonly_token_with_cap(psid)?
                 }
                 SandboxPolicy::WorkspaceWrite { .. } => {
-                    let caps = load_or_create_cap_sids(codex_home);
-                    ensure_dir(&cap_sid_path)?;
-                    fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
                     let psid = convert_string_sid_to_sid(&caps.workspace).unwrap();
                     super::token::create_workspace_write_token_with_cap(psid)?
                 }
-                SandboxPolicy::DangerFullAccess => {
-                    anyhow::bail!("DangerFullAccess is not supported for sandboxing")
-                }
+                SandboxPolicy::DangerFullAccess => unreachable!("DangerFullAccess handled above"),
             }
         };
 

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

@@ -20,6 +20,7 @@ use rand::RngCore;
 use rand::SeedableRng;
 use serde::Deserialize;
 use serde::Serialize;
+use std::collections::HashSet;
 use std::ffi::c_void;
 use std::ffi::OsStr;
 use std::fs::File;
@@ -392,8 +393,8 @@ fn run_netsh_firewall(sid: &str, log: &mut File) -> Result<()> {
             log_line(
                 log,
                 &format!(
-                "firewall rule configured via COM with LocalUserAuthorizedList={local_user_spec}"
-            ),
+                    "firewall rule configured via COM with LocalUserAuthorizedList={local_user_spec}"
+                ),
             )?;
             Ok(())
         })()
@@ -647,7 +648,7 @@ fn run_setup(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<()> {
             string_from_sid_bytes(&online_sid).map_err(anyhow::Error::msg)?
         ),
     )?;
-    let caps = load_or_create_cap_sids(&payload.codex_home);
+    let caps = load_or_create_cap_sids(&payload.codex_home)?;
     let cap_psid = unsafe {
         convert_string_sid_to_sid(&caps.workspace)
             .ok_or_else(|| anyhow::anyhow!("convert capability SID failed"))?
@@ -758,20 +759,26 @@ fn run_setup(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<()> {
         }
     }
 
+    let cap_sid_str = caps.workspace.clone();
+    let online_sid_str = string_from_sid_bytes(&online_sid).map_err(anyhow::Error::msg)?;
+    let sid_strings = vec![offline_sid_str.clone(), online_sid_str, cap_sid_str];
+    let write_mask =
+        FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | FILE_DELETE_CHILD;
+    let mut grant_tasks: Vec<PathBuf> = Vec::new();
+
+    let mut seen_write_roots: HashSet<PathBuf> = HashSet::new();
+
     for root in &payload.write_roots {
+        if !seen_write_roots.insert(root.clone()) {
+            continue;
+        }
         if !root.exists() {
             log_line(
                 log,
                 &format!("write root {} missing; skipping", root.display()),
             )?;
             continue;
         }
-        let sids = vec![offline_psid, online_psid, cap_psid];
-        let write_mask = FILE_GENERIC_READ
-            | FILE_GENERIC_WRITE
-            | FILE_GENERIC_EXECUTE
-            | DELETE
-            | FILE_DELETE_CHILD;
         let mut need_grant = false;
         for (label, psid) in [
             ("offline", offline_psid),
@@ -817,35 +824,76 @@ fn run_setup(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<()> {
                     root.display()
                 ),
             )?;
-            match unsafe { ensure_allow_write_aces(root, &sids) } {
-                Ok(res) => {
-                    log_line(
+            grant_tasks.push(root.clone());
+        } else {
+            log_line(
+                log,
+                &format!(
+                    "write ACE already present for all sandbox SIDs on {}",
+                    root.display()
+                ),
+            )?;
+        }
+    }
+
+    let (tx, rx) = mpsc::channel::<(PathBuf, Result<bool>)>();
+    std::thread::scope(|scope| {
+        for root in grant_tasks {
+            let sid_strings = sid_strings.clone();
+            let tx = tx.clone();
+            scope.spawn(move || {
+                // Convert SID strings to psids locally in this thread.
+                let mut psids: Vec<*mut c_void> = Vec::new();
+                for sid_str in &sid_strings {
+                    if let Some(psid) = unsafe { convert_string_sid_to_sid(sid_str) } {
+                        psids.push(psid);
+                    } else {
+                        let _ = tx.send((root.clone(), Err(anyhow::anyhow!("convert SID failed"))));
+                        return;
+                    }
+                }
+
+                let res = unsafe { ensure_allow_write_aces(&root, &psids) };
+
+                for psid in psids {
+                    unsafe {
+                        LocalFree(psid as HLOCAL);
+                    }
+                }
+                let _ = tx.send((root, res));
+            });
+        }
+        drop(tx);
+        for (root, res) in rx {
+            match res {
+                Ok(added) => {
+                    if log_line(
                         log,
                         &format!(
                             "write ACE {} on {}",
-                            if res { "added" } else { "already present" },
+                            if added { "added" } else { "already present" },
                             root.display()
                         ),
-                    )?;
+                    )
+                    .is_err()
+                    {
+                        // ignore log errors inside scoped thread
+                    }
                 }
                 Err(e) => {
                     refresh_errors.push(format!("write ACE failed on {}: {}", root.display(), e));
-                    log_line(
+                    if log_line(
                         log,
                         &format!("write ACE grant failed on {}: {}", root.display(), e),
-                    )?;
+                    )
+                    .is_err()
+                    {
+                        // ignore log errors inside scoped thread
+                    }
                 }
             }
-        } else {
-            log_line(
-                log,
-                &format!(
-                    "write ACE already present for all sandbox SIDs on {}",
-                    root.display()
-                ),
-            )?;
         }
-    }
+    });
 
     if refresh_only {
         log_line(