fix: scope windows sandbox capability sids by writable root

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

@@ -15,11 +15,13 @@ use crate::path_normalization::canonical_path_key;
 pub struct CapSids {
     pub workspace: String,
     pub readonly: String,
-    /// Per-workspace capability SIDs keyed by canonicalized CWD string.
+    /// Path-scoped capability SIDs keyed by canonicalized writable-root strings.
     ///
-    /// This is used to isolate workspaces from other workspace sandbox writes and to
-    /// apply per-workspace denies (e.g. protect `CWD/.codex`)
-    /// without permanently affecting other workspaces.
+    /// This is used to isolate writable roots from one another so stale ACL grants on
+    /// one workspace do not automatically authorize later sessions in unrelated roots.
+    ///
+    /// The serialized field name is kept for backwards compatibility with existing
+    /// `cap_sid` files on disk.
     #[serde(default)]
     pub workspace_by_cwd: HashMap<String, String>,
 }
@@ -77,9 +79,14 @@ pub fn load_or_create_cap_sids(codex_home: &Path) -> Result<CapSids> {
 
 /// Returns the workspace-specific capability SID for `cwd`, creating and persisting it if missing.
 pub fn workspace_cap_sid_for_cwd(codex_home: &Path, cwd: &Path) -> Result<String> {
+    write_cap_sid_for_root(codex_home, cwd)
+}
+
+/// Returns the path-scoped capability SID for `root`, creating and persisting it if missing.
+pub fn write_cap_sid_for_root(codex_home: &Path, root: &Path) -> Result<String> {
     let path = cap_sid_file(codex_home);
     let mut caps = load_or_create_cap_sids(codex_home)?;
-    let key = canonical_path_key(cwd);
+    let key = canonical_path_key(root);
     if let Some(sid) = caps.workspace_by_cwd.get(&key) {
         return Ok(sid.clone());
     }
@@ -92,8 +99,10 @@ pub fn workspace_cap_sid_for_cwd(codex_home: &Path, cwd: &Path) -> Result<String
 #[cfg(test)]
 mod tests {
     use super::load_or_create_cap_sids;
+    use super::write_cap_sid_for_root;
     use super::workspace_cap_sid_for_cwd;
     use pretty_assertions::assert_eq;
+    use std::collections::HashSet;
     use std::path::PathBuf;
 
     #[test]
@@ -118,4 +127,25 @@ mod tests {
         let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
         assert_eq!(caps.workspace_by_cwd.len(), 1);
     }
+
+    #[test]
+    fn distinct_writable_roots_get_distinct_sids() {
+        let temp = tempfile::tempdir().expect("tempdir");
+        let codex_home = temp.path().join("codex-home");
+        std::fs::create_dir_all(&codex_home).expect("create codex home");
+
+        let workspace_a = temp.path().join("WorkspaceA");
+        let workspace_b = temp.path().join("WorkspaceB");
+        std::fs::create_dir_all(&workspace_a).expect("create workspace a");
+        std::fs::create_dir_all(&workspace_b).expect("create workspace b");
+
+        let sid_a = write_cap_sid_for_root(&codex_home, &workspace_a).expect("sid a");
+        let sid_b = write_cap_sid_for_root(&codex_home, &workspace_b).expect("sid b");
+
+        assert_ne!(sid_a, sid_b);
+
+        let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
+        let values: HashSet<_> = caps.workspace_by_cwd.values().cloned().collect();
+        assert_eq!(values.len(), 2);
+    }
 }

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

@@ -3,6 +3,7 @@ mod windows_impl {
     use crate::allow::compute_allow_paths;
     use crate::allow::AllowDenyPaths;
     use crate::cap::load_or_create_cap_sids;
+    use crate::cap::write_cap_sid_for_root;
     use crate::env::ensure_non_interactive_pager;
     use crate::env::inherit_path_env;
     use crate::env::normalize_null_device_env;
@@ -239,25 +240,40 @@ mod windows_impl {
             anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing")
         }
         let caps = load_or_create_cap_sids(codex_home)?;
+        let AllowDenyPaths { allow, deny: _ } =
+            compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
+        let canonical_cwd = crate::path_normalization::canonicalize_path(&current_dir);
+        let mut allow_roots: Vec<PathBuf> = allow.into_iter().collect();
+        allow_roots.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
         let (psid_to_use, cap_sids) = match &policy {
             SandboxPolicy::ReadOnly { .. } => (
                 unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() },
                 vec![caps.readonly.clone()],
             ),
-            SandboxPolicy::WorkspaceWrite { .. } => (
-                unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() },
-                vec![
-                    caps.workspace.clone(),
-                    crate::cap::workspace_cap_sid_for_cwd(codex_home, cwd)?,
-                ],
-            ),
+            SandboxPolicy::WorkspaceWrite { .. } => {
+                let mut cap_sids = Vec::with_capacity(allow_roots.len());
+                for root in &allow_roots {
+                    let sid = if crate::workspace_acl::is_command_cwd_root(root, &canonical_cwd) {
+                        crate::cap::workspace_cap_sid_for_cwd(codex_home, root)?
+                    } else {
+                        write_cap_sid_for_root(codex_home, root)?
+                    };
+                    cap_sids.push(sid);
+                }
+                let psid_to_use = unsafe {
+                    convert_string_sid_to_sid(
+                        cap_sids
+                            .first()
+                            .expect("workspace-write should always have a writable root"),
+                    )
+                    .unwrap()
+                };
+                (psid_to_use, cap_sids)
+            }
             SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
                 unreachable!("DangerFullAccess handled above")
             }
         };
-
-        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.
         unsafe {
             allow_null_device(psid_to_use);

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

@@ -64,6 +64,8 @@ pub use cap::load_or_create_cap_sids;
 #[cfg(target_os = "windows")]
 pub use cap::workspace_cap_sid_for_cwd;
 #[cfg(target_os = "windows")]
+pub use cap::write_cap_sid_for_root;
+#[cfg(target_os = "windows")]
 pub use conpty::spawn_conpty_process_as_user;
 #[cfg(target_os = "windows")]
 pub use dpapi::protect as dpapi_protect;
@@ -179,6 +181,7 @@ mod windows_impl {
     use super::allow::AllowDenyPaths;
     use super::cap::load_or_create_cap_sids;
     use super::cap::workspace_cap_sid_for_cwd;
+    use super::cap::write_cap_sid_for_root;
     use super::env::apply_no_network_to_env;
     use super::env::ensure_non_interactive_pager;
     use super::env::normalize_null_device_env;
@@ -213,6 +216,40 @@ mod windows_impl {
 
     type PipeHandles = ((HANDLE, HANDLE), (HANDLE, HANDLE), (HANDLE, HANDLE));
 
+    fn sorted_allow_roots(allow: &std::collections::HashSet<PathBuf>) -> Vec<PathBuf> {
+        let mut roots: Vec<PathBuf> = allow.iter().cloned().collect();
+        roots.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
+        roots
+    }
+
+    fn capability_sid_strings_for_allow_roots(
+        codex_home: &Path,
+        allow_roots: &[PathBuf],
+        canonical_cwd: &Path,
+    ) -> Result<HashMap<PathBuf, String>> {
+        let mut sid_by_root = HashMap::new();
+        for root in allow_roots {
+            let sid = if is_command_cwd_root(root, canonical_cwd) {
+                workspace_cap_sid_for_cwd(codex_home, root)?
+            } else {
+                write_cap_sid_for_root(codex_home, root)?
+            };
+            sid_by_root.insert(root.clone(), sid);
+        }
+        Ok(sid_by_root)
+    }
+
+    fn capability_sid_for_path<'a>(
+        sid_by_root: &'a HashMap<PathBuf, *mut c_void>,
+        sorted_allow_roots: &[PathBuf],
+        path: &Path,
+    ) -> Option<*mut c_void> {
+        sorted_allow_roots
+            .iter()
+            .find(|root| path.starts_with(root))
+            .and_then(|root| sid_by_root.get(root).copied())
+    }
+
     fn should_apply_network_block(policy: &SandboxPolicy) -> bool {
         !policy.has_full_network_access()
     }
@@ -282,6 +319,11 @@ mod windows_impl {
         let logs_base_dir = Some(sandbox_base.as_path());
         log_start(&command, logs_base_dir);
         let is_workspace_write = matches!(&policy, SandboxPolicy::WorkspaceWrite { .. });
+        let current_dir = cwd.to_path_buf();
+        let AllowDenyPaths { allow, deny } =
+            compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
+        let canonical_cwd = canonicalize_path(&current_dir);
+        let sorted_allow_roots = sorted_allow_roots(&allow);
 
         if matches!(
             &policy,
@@ -295,25 +337,38 @@ mod windows_impl {
             );
         }
         let caps = load_or_create_cap_sids(codex_home)?;
-        let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe {
+        let sid_strings_by_root = capability_sid_strings_for_allow_roots(
+            codex_home,
+            &sorted_allow_roots,
+            &canonical_cwd,
+        )?;
+        let (h_token, psid_readonly, psids_by_root): (
+            HANDLE,
+            Option<*mut c_void>,
+            HashMap<PathBuf, *mut c_void>,
+        ) = unsafe {
             match &policy {
                 SandboxPolicy::ReadOnly { .. } => {
                     let psid = convert_string_sid_to_sid(&caps.readonly).unwrap();
                     let (h, _) = super::token::create_readonly_token_with_cap(psid)?;
-                    (h, psid, None)
+                    (h, Some(psid), HashMap::new())
                 }
                 SandboxPolicy::WorkspaceWrite { .. } => {
-                    let psid_generic = convert_string_sid_to_sid(&caps.workspace).unwrap();
-                    let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
-                    let psid_workspace = convert_string_sid_to_sid(&ws_sid).unwrap();
+                    let mut psids_by_root = HashMap::new();
+                    let mut token_caps = Vec::with_capacity(sorted_allow_roots.len());
+                    for root in &sorted_allow_roots {
+                        let sid = sid_strings_by_root
+                            .get(root)
+                            .expect("missing writable-root SID");
+                        let psid = convert_string_sid_to_sid(sid).unwrap();
+                        token_caps.push(psid);
+                        psids_by_root.insert(root.clone(), psid);
+                    }
                     let base = super::token::get_current_token_for_restriction()?;
-                    let h_res = create_workspace_write_token_with_caps_from(
-                        base,
-                        &[psid_generic, psid_workspace],
-                    );
+                    let h_res = create_workspace_write_token_with_caps_from(base, &token_caps);
                     windows_sys::Win32::Foundation::CloseHandle(base);
                     let h = h_res?;
-                    (h, psid_generic, Some(psid_workspace))
+                    (h, None, psids_by_root)
                 }
                 SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
                     unreachable!("DangerFullAccess handled above")
@@ -335,16 +390,15 @@ mod windows_impl {
         }
 
         let persist_aces = is_workspace_write;
-        let AllowDenyPaths { allow, deny } =
-            compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
-        let canonical_cwd = canonicalize_path(&current_dir);
         let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new();
         unsafe {
             for p in &allow {
-                let psid = if is_workspace_write && is_command_cwd_root(p, &canonical_cwd) {
-                    psid_workspace.unwrap_or(psid_generic)
+                let Some(psid) = (if is_workspace_write {
+                    capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
                 } else {
-                    psid_generic
+                    psid_readonly
+                }) else {
+                    continue;
                 };
                 if let Ok(added) = add_allow_ace(p, psid) {
                     if added {
@@ -359,15 +413,28 @@ mod windows_impl {
                 }
             }
             for p in &deny {
-                if let Ok(added) = add_deny_write_ace(p, psid_generic) {
+                let Some(psid) = (if is_workspace_write {
+                    capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
+                } else {
+                    psid_readonly
+                }) else {
+                    continue;
+                };
+                if let Ok(added) = add_deny_write_ace(p, psid) {
                     if added && !persist_aces {
-                        guards.push((p.clone(), psid_generic));
+                        guards.push((p.clone(), psid));
                     }
                 }
             }
-            allow_null_device(psid_generic);
-            if let Some(psid) = psid_workspace {
+            if let Some(psid) = psid_readonly {
                 allow_null_device(psid);
+            }
+            for psid in psids_by_root.values().copied() {
+                allow_null_device(psid);
+            }
+            if let Some(psid) =
+                capability_sid_for_path(&psids_by_root, &sorted_allow_roots, &current_dir)
+            {
                 let _ = protect_workspace_codex_dir(&current_dir, psid);
                 let _ = protect_workspace_agents_dir(&current_dir, psid);
             }
@@ -525,33 +592,49 @@ mod windows_impl {
         }
 
         ensure_codex_home_exists(codex_home)?;
-        let caps = load_or_create_cap_sids(codex_home)?;
-        let psid_generic =
-            unsafe { convert_string_sid_to_sid(&caps.workspace) }.expect("valid workspace SID");
-        let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
-        let psid_workspace =
-            unsafe { convert_string_sid_to_sid(&ws_sid) }.expect("valid workspace SID");
         let current_dir = cwd.to_path_buf();
         let AllowDenyPaths { allow, deny } =
             compute_allow_paths(sandbox_policy, sandbox_policy_cwd, &current_dir, env_map);
         let canonical_cwd = canonicalize_path(&current_dir);
+        let sorted_allow_roots = sorted_allow_roots(&allow);
+        let sid_strings_by_root = capability_sid_strings_for_allow_roots(
+            codex_home,
+            &sorted_allow_roots,
+            &canonical_cwd,
+        )?;
+        let mut psids_by_root = HashMap::new();
+        for root in &sorted_allow_roots {
+            let sid = sid_strings_by_root
+                .get(root)
+                .expect("missing writable-root SID");
+            let psid = unsafe { convert_string_sid_to_sid(sid) }.expect("valid workspace SID");
+            psids_by_root.insert(root.clone(), psid);
+        }
 
         unsafe {
             for p in &allow {
-                let psid = if is_command_cwd_root(p, &canonical_cwd) {
-                    psid_workspace
-                } else {
-                    psid_generic
+                let Some(psid) = capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
+                else {
+                    continue;
                 };
                 let _ = add_allow_ace(p, psid);
             }
             for p in &deny {
-                let _ = add_deny_write_ace(p, psid_generic);
+                let Some(psid) = capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
+                else {
+                    continue;
+                };
+                let _ = add_deny_write_ace(p, psid);
+            }
+            for psid in psids_by_root.values().copied() {
+                allow_null_device(psid);
+            }
+            if let Some(psid) =
+                capability_sid_for_path(&psids_by_root, &sorted_allow_roots, &current_dir)
+            {
+                let _ = protect_workspace_codex_dir(&current_dir, psid);
+                let _ = protect_workspace_agents_dir(&current_dir, psid);
             }
-            allow_null_device(psid_generic);
-            allow_null_device(psid_workspace);
-            let _ = protect_workspace_codex_dir(&current_dir, psid_workspace);
-            let _ = protect_workspace_agents_dir(&current_dir, psid_workspace);
         }
 
         Ok(())

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

@@ -13,7 +13,6 @@ use codex_windows_sandbox::ensure_allow_write_aces;
 use codex_windows_sandbox::extract_setup_failure;
 use codex_windows_sandbox::hide_newly_created_users;
 use codex_windows_sandbox::is_command_cwd_root;
-use codex_windows_sandbox::load_or_create_cap_sids;
 use codex_windows_sandbox::log_note;
 use codex_windows_sandbox::path_mask_allows;
 use codex_windows_sandbox::protect_workspace_agents_dir;
@@ -24,6 +23,7 @@ use codex_windows_sandbox::sandbox_secrets_dir;
 use codex_windows_sandbox::string_from_sid_bytes;
 use codex_windows_sandbox::to_wide;
 use codex_windows_sandbox::workspace_cap_sid_for_cwd;
+use codex_windows_sandbox::write_cap_sid_for_root;
 use codex_windows_sandbox::write_setup_error_report;
 use codex_windows_sandbox::SetupErrorCode;
 use codex_windows_sandbox::SetupErrorReport;
@@ -551,25 +551,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         ))
     })?;
 
-    let caps = load_or_create_cap_sids(&payload.codex_home).map_err(|err| {
-        anyhow::Error::new(SetupFailure::new(
-            SetupErrorCode::HelperCapabilitySidFailed,
-            format!("load or create capability SIDs failed: {err}"),
-        ))
-    })?;
-    let cap_psid = unsafe {
-        convert_string_sid_to_sid(&caps.workspace).ok_or_else(|| {
-            anyhow::Error::new(SetupFailure::new(
-                SetupErrorCode::HelperCapabilitySidFailed,
-                format!("convert capability SID {} failed", caps.workspace),
-            ))
-        })?
-    };
-    let workspace_sid_str = workspace_cap_sid_for_cwd(&payload.codex_home, &payload.command_cwd)?;
-    let workspace_psid = unsafe {
-        convert_string_sid_to_sid(&workspace_sid_str)
-            .ok_or_else(|| anyhow::anyhow!("convert workspace capability SID failed"))?
-    };
     let mut refresh_errors: Vec<String> = Vec::new();
     if !refresh_only {
         let firewall_result = firewall::ensure_offline_outbound_block(&offline_sid_str, log);
@@ -616,7 +597,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         }
     }
 
-    let cap_sid_str = caps.workspace.clone();
     let sandbox_group_sid_str =
         string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
     let write_mask =
@@ -639,19 +619,18 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         }
         let mut need_grant = false;
         let is_command_cwd = is_command_cwd_root(root, &canonical_command_cwd);
-        let cap_label = if is_command_cwd {
-            "workspace_cap"
+        let cap_sid_str_for_root = if is_command_cwd {
+            workspace_cap_sid_for_cwd(&payload.codex_home, root)?
         } else {
-            "cap"
+            write_cap_sid_for_root(&payload.codex_home, root)?
         };
-        let cap_psid_for_root = if is_command_cwd {
-            workspace_psid
-        } else {
-            cap_psid
+        let cap_psid_for_root = unsafe {
+            convert_string_sid_to_sid(&cap_sid_str_for_root)
+                .ok_or_else(|| anyhow::anyhow!("convert writable-root capability SID failed"))?
         };
         for (label, psid) in [
             ("sandbox_group", sandbox_group_psid),
-            (cap_label, cap_psid_for_root),
+            ("path_cap", cap_psid_for_root),
         ] {
             let has =
                 match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) {
@@ -694,9 +673,15 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         for root in grant_tasks {
             let is_command_cwd = is_command_cwd_root(&root, &canonical_command_cwd);
             let sid_strings = if is_command_cwd {
-                vec![sandbox_group_sid_str.clone(), workspace_sid_str.clone()]
+                vec![
+                    sandbox_group_sid_str.clone(),
+                    workspace_cap_sid_for_cwd(&payload.codex_home, &root)?,
+                ]
             } else {
-                vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
+                vec![
+                    sandbox_group_sid_str.clone(),
+                    write_cap_sid_for_root(&payload.codex_home, &root)?,
+                ]
             };
             let tx = tx.clone();
             scope.spawn(move || {