Avoid redundant Windows sandbox ACL rewrites

codex-rs/core/src/tools/spec_tests.rs

@@ -296,31 +296,6 @@ fn build_specs_with_unavailable_tools(
     )
 }
 
-#[tokio::test]
-async fn model_provided_unified_exec_is_blocked_for_windows_sandboxed_policies() {
-    let mut model_info = model_info_from_models_json("gpt-5.4").await;
-    model_info.shell_type = ConfigShellToolType::UnifiedExec;
-    let features = Features::with_defaults();
-    let available_models = Vec::new();
-    let config = ToolsConfig::new(&ToolsConfigParams {
-        model_info: &model_info,
-        available_models: &available_models,
-        features: &features,
-        image_generation_tool_auth_allowed: true,
-        web_search_mode: Some(WebSearchMode::Cached),
-        session_source: SessionSource::Cli,
-        permission_profile: &PermissionProfile::workspace_write(),
-        windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
-    });
-
-    let expected_shell_type = if cfg!(target_os = "windows") {
-        ConfigShellToolType::ShellCommand
-    } else {
-        ConfigShellToolType::UnifiedExec
-    };
-    assert_eq!(config.shell_type, expected_shell_type);
-}
-
 #[tokio::test]
 async fn get_memory_requires_feature_flag() {
     let config = test_config().await;

codex-rs/tools/src/tool_config.rs

@@ -134,8 +134,7 @@ impl ToolsConfig {
             image_generation_tool_auth_allowed,
             web_search_mode,
             session_source,
-            permission_profile,
-            windows_sandbox_level,
+            ..
         } = params;
         let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform);
         let include_code_mode = features.enabled(Feature::CodeMode);
@@ -165,26 +164,25 @@ impl ToolsConfig {
             } else {
                 ShellCommandBackendConfig::Classic
             };
-        let unified_exec_allowed = unified_exec_allowed_in_environment(
-            cfg!(target_os = "windows"),
-            permission_profile,
-            *windows_sandbox_level,
-        );
+        let unified_exec_enabled = features.enabled(Feature::UnifiedExec);
+        let model_shell_type = match model_info.shell_type {
+            ConfigShellToolType::UnifiedExec if !unified_exec_enabled => {
+                ConfigShellToolType::ShellCommand
+            }
+            other => other,
+        };
         let shell_type = if !features.enabled(Feature::ShellTool) {
             ConfigShellToolType::Disabled
         } else if features.enabled(Feature::ShellZshFork) {
             ConfigShellToolType::ShellCommand
-        } else if features.enabled(Feature::UnifiedExec) && unified_exec_allowed {
+        } else if unified_exec_enabled {
             if codex_utils_pty::conpty_supported() {
                 ConfigShellToolType::UnifiedExec
             } else {
                 ConfigShellToolType::ShellCommand
             }
-        } else if model_info.shell_type == ConfigShellToolType::UnifiedExec && !unified_exec_allowed
-        {
-            ConfigShellToolType::ShellCommand
         } else {
-            model_info.shell_type
+            model_shell_type
         };
 
         let apply_patch_tool_type = match model_info.apply_patch_tool_type {
@@ -320,23 +318,6 @@ fn supports_image_generation(model_info: &ModelInfo) -> bool {
     model_info.input_modalities.contains(&InputModality::Image)
 }
 
-fn unified_exec_allowed_in_environment(
-    is_windows: bool,
-    permission_profile: &PermissionProfile,
-    windows_sandbox_level: WindowsSandboxLevel,
-) -> bool {
-    let managed_sandbox_required = match permission_profile {
-        PermissionProfile::Managed {
-            file_system,
-            network,
-        } => !file_system.to_sandbox_policy().has_full_disk_write_access() || !network.is_enabled(),
-        PermissionProfile::Disabled | PermissionProfile::External { .. } => false,
-    };
-    !(is_windows
-        && windows_sandbox_level != WindowsSandboxLevel::Disabled
-        && managed_sandbox_required)
-}
-
 #[cfg(test)]
 #[path = "tool_config_tests.rs"]
 mod tests;

codex-rs/tools/src/tool_config_tests.rs

@@ -3,12 +3,10 @@ use codex_features::Feature;
 use codex_features::Features;
 use codex_protocol::config_types::WebSearchMode;
 use codex_protocol::config_types::WindowsSandboxLevel;
-use codex_protocol::models::ManagedFileSystemPermissions;
 use codex_protocol::models::PermissionProfile;
 use codex_protocol::openai_models::ConfigShellToolType;
 use codex_protocol::openai_models::InputModality;
 use codex_protocol::openai_models::ModelInfo;
-use codex_protocol::permissions::NetworkSandboxPolicy;
 use codex_protocol::protocol::SessionSource;
 use codex_protocol::protocol::SubAgentSource;
 use codex_utils_absolute_path::AbsolutePathBuf;
@@ -52,42 +50,50 @@ fn model_info() -> ModelInfo {
 }
 
 #[test]
-fn unified_exec_is_blocked_for_windows_managed_profiles_only() {
-    assert!(!unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::read_only(),
-        WindowsSandboxLevel::RestrictedToken,
-    ));
-    assert!(!unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::workspace_write(),
-        WindowsSandboxLevel::RestrictedToken,
-    ));
-    assert!(unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::Disabled,
-        WindowsSandboxLevel::RestrictedToken,
-    ));
-    assert!(unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::External {
-            network: Default::default(),
-        },
-        WindowsSandboxLevel::RestrictedToken,
-    ));
-    assert!(unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::Managed {
-            file_system: ManagedFileSystemPermissions::Unrestricted,
-            network: NetworkSandboxPolicy::Enabled,
-        },
-        WindowsSandboxLevel::RestrictedToken,
-    ));
-    assert!(unified_exec_allowed_in_environment(
-        /*is_windows*/ true,
-        &PermissionProfile::Disabled,
-        WindowsSandboxLevel::Disabled,
-    ));
+fn model_provided_unified_exec_requires_feature_flag() {
+    let model_info = model_info();
+    let mut features = Features::with_defaults();
+    features.disable(Feature::UnifiedExec);
+
+    let available_models = Vec::new();
+    let tools_config = ToolsConfig::new(&ToolsConfigParams {
+        model_info: &model_info,
+        available_models: &available_models,
+        features: &features,
+        image_generation_tool_auth_allowed: true,
+        web_search_mode: Some(WebSearchMode::Cached),
+        session_source: SessionSource::Cli,
+        permission_profile: &PermissionProfile::Disabled,
+        windows_sandbox_level: WindowsSandboxLevel::Disabled,
+    });
+
+    assert_eq!(tools_config.shell_type, ConfigShellToolType::ShellCommand);
+}
+
+#[test]
+fn unified_exec_can_be_enabled_for_restricted_token_workspace_write() {
+    let model_info = model_info();
+    let mut features = Features::with_defaults();
+    features.enable(Feature::UnifiedExec);
+
+    let available_models = Vec::new();
+    let tools_config = ToolsConfig::new(&ToolsConfigParams {
+        model_info: &model_info,
+        available_models: &available_models,
+        features: &features,
+        image_generation_tool_auth_allowed: true,
+        web_search_mode: Some(WebSearchMode::Cached),
+        session_source: SessionSource::Cli,
+        permission_profile: &PermissionProfile::workspace_write(),
+        windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
+    });
+
+    let expected_shell_type = if codex_utils_pty::conpty_supported() {
+        ConfigShellToolType::UnifiedExec
+    } else {
+        ConfigShellToolType::ShellCommand
+    };
+    assert_eq!(tools_config.shell_type, expected_shell_type);
 }
 
 #[test]

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

@@ -173,6 +173,92 @@ pub fn path_mask_allows(
     }
 }
 
+/// Aggregate allow check across all matching ACEs for the provided SIDs.
+///
+/// This is closer to Windows effective-access evaluation than `dacl_mask_allows` because
+/// a token can satisfy a requested mask through multiple allow ACEs across different groups
+/// or capability SIDs.
+pub unsafe fn dacl_mask_allows_aggregate(
+    p_dacl: *mut ACL,
+    psids: &[*mut c_void],
+    desired_mask: u32,
+    require_all_bits: bool,
+) -> bool {
+    if p_dacl.is_null() {
+        return false;
+    }
+    let mut info: ACL_SIZE_INFORMATION = std::mem::zeroed();
+    let ok = GetAclInformation(
+        p_dacl as *const ACL,
+        &mut info as *mut _ as *mut c_void,
+        std::mem::size_of::<ACL_SIZE_INFORMATION>() as u32,
+        AclSizeInformation,
+    );
+    if ok == 0 {
+        return false;
+    }
+    let mapping = GENERIC_MAPPING {
+        GenericRead: FILE_GENERIC_READ,
+        GenericWrite: FILE_GENERIC_WRITE,
+        GenericExecute: FILE_GENERIC_EXECUTE,
+        GenericAll: FILE_ALL_ACCESS,
+    };
+    let mut granted_mask = 0u32;
+    for i in 0..(info.AceCount as usize) {
+        let mut p_ace: *mut c_void = std::ptr::null_mut();
+        if GetAce(p_dacl as *const ACL, i as u32, &mut p_ace) == 0 {
+            continue;
+        }
+        let hdr = &*(p_ace as *const ACE_HEADER);
+        if hdr.AceType != 0 {
+            continue; // not ACCESS_ALLOWED
+        }
+        if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 {
+            continue;
+        }
+        let base = p_ace as usize;
+        let sid_ptr =
+            (base + std::mem::size_of::<ACE_HEADER>() + std::mem::size_of::<u32>()) as *mut c_void;
+        let mut matched = false;
+        for sid in psids {
+            if EqualSid(sid_ptr, *sid) != 0 {
+                matched = true;
+                break;
+            }
+        }
+        if !matched {
+            continue;
+        }
+        let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE);
+        let mut mask = ace.Mask;
+        MapGenericMask(&mut mask, &mapping);
+        granted_mask |= mask;
+        if (require_all_bits && (granted_mask & desired_mask) == desired_mask)
+            || (!require_all_bits && (granted_mask & desired_mask) != 0)
+        {
+            return true;
+        }
+    }
+    false
+}
+
+/// Path-based wrapper around the aggregate mask check (single DACL fetch).
+pub fn path_mask_allows_aggregate(
+    path: &Path,
+    psids: &[*mut c_void],
+    desired_mask: u32,
+    require_all_bits: bool,
+) -> Result<bool> {
+    unsafe {
+        let (p_dacl, sd) = fetch_dacl_handle(path)?;
+        let has = dacl_mask_allows_aggregate(p_dacl, psids, desired_mask, require_all_bits);
+        if !sd.is_null() {
+            LocalFree(sd as HLOCAL);
+        }
+        Ok(has)
+    }
+}
+
 pub unsafe fn dacl_has_write_allow_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -> bool {
     if p_dacl.is_null() {
         return false;

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

@@ -89,6 +89,8 @@ pub use acl::fetch_dacl_handle;
 #[cfg(target_os = "windows")]
 pub use acl::path_mask_allows;
 #[cfg(target_os = "windows")]
+pub use acl::path_mask_allows_aggregate;
+#[cfg(target_os = "windows")]
 pub use audit::apply_world_writable_scan_and_denies;
 #[cfg(target_os = "windows")]
 pub use cap::load_or_create_cap_sids;

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

@@ -22,6 +22,7 @@ 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::path_mask_allows_aggregate;
 use codex_windows_sandbox::sandbox_bin_dir;
 use codex_windows_sandbox::sandbox_dir;
 use codex_windows_sandbox::sandbox_secrets_dir;
@@ -669,33 +670,32 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         } else {
             cap_psid
         };
-        for (label, psid) in [
-            ("sandbox_group", sandbox_group_psid),
-            (cap_label, cap_psid_for_root),
-        ] {
-            let has =
-                match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) {
-                    Ok(h) => h,
-                    Err(e) => {
-                        refresh_errors.push(format!(
-                            "write mask check failed on {} for {label}: {}",
-                            root.display(),
-                            e
-                        ));
-                        log_line(
-                            log,
-                            &format!(
-                                "write mask check failed on {} for {label}: {}; continuing",
-                                root.display(),
-                                e
-                            ),
-                        )?;
-                        false
-                    }
-                };
-            if !has {
-                need_grant = true;
+        let has_effective_write = match path_mask_allows_aggregate(
+            root,
+            &[sandbox_group_psid, cap_psid_for_root],
+            write_mask,
+            /*require_all_bits*/ true,
+        ) {
+            Ok(h) => h,
+            Err(e) => {
+                refresh_errors.push(format!(
+                    "write mask check failed on {} for sandbox_group+{cap_label}: {}",
+                    root.display(),
+                    e
+                ));
+                log_line(
+                    log,
+                    &format!(
+                        "write mask check failed on {} for sandbox_group+{cap_label}: {}; continuing",
+                        root.display(),
+                        e
+                    ),
+                )?;
+                false
             }
+        };
+        if !has_effective_write {
+            need_grant = true;
         }
         if need_grant {
             log_line(