Fix Windows sandbox identity and runtime permissions

Resolve the invoking Windows account through the OS instead of relying on
USERNAME, which sandbox launchers may omit.

Repair missing read/execute access on existing runtime files and directories
during sandbox refresh. Preserve explicit and inherited deny permissions,
skip reparse paths, and tolerate entries removed during traversal.

Add Windows regression coverage for descendant permissions, repeated repair,
deny preservation, and absence of newly granted write, delete, or ACL-management
rights.

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

@@ -21,9 +21,12 @@ use windows_sys::Win32::Security::ACE_HEADER;
 use windows_sys::Win32::Security::ACL;
 use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
 use windows_sys::Win32::Security::AclSizeInformation;
+use windows_sys::Win32::Security::Authorization::ACCESS_MODE;
 use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W;
+use windows_sys::Win32::Security::Authorization::GRANT_ACCESS;
 use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW;
 use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
+use windows_sys::Win32::Security::Authorization::SET_ACCESS;
 use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
 use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
 use windows_sys::Win32::Security::Authorization::SetSecurityInfo;
@@ -68,6 +71,8 @@ const ACCESS_ALLOWED_ACE_TYPE: u8 = 0;
 const ACCESS_DENIED_ACE_TYPE: u8 = 1;
 const GENERIC_READ_MASK: u32 = 0x8000_0000;
 const GENERIC_WRITE_MASK: u32 = 0x4000_0000;
+const GENERIC_EXECUTE_MASK: u32 = 0x2000_0000;
+const GENERIC_ALL_MASK: u32 = 0x1000_0000;
 const DENY_ACCESS: i32 = 3;
 // TrustedInstaller is a deterministic service SID, not a machine-local account SID.
 const TRUSTED_INSTALLER_SID: &str =
@@ -444,6 +449,10 @@ pub unsafe fn dacl_has_write_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -
 }
 
 pub unsafe fn dacl_has_read_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -> bool {
+    dacl_has_deny_mask_for_sid(p_dacl, psid, FILE_GENERIC_READ | GENERIC_READ_MASK)
+}
+
+unsafe fn dacl_has_deny_mask_for_sid(p_dacl: *mut ACL, psid: *mut c_void, deny_mask: u32) -> bool {
     if p_dacl.is_null() {
         return false;
     }
@@ -457,7 +466,6 @@ pub unsafe fn dacl_has_read_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) ->
     if ok == 0 {
         return false;
     }
-    let deny_read_mask = FILE_GENERIC_READ | GENERIC_READ_MASK;
     for i in 0..info.AceCount {
         let mut p_ace: *mut c_void = std::ptr::null_mut();
         if GetAce(p_dacl as *const ACL, i, &mut p_ace) == 0 {
@@ -474,7 +482,7 @@ pub unsafe fn dacl_has_read_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) ->
         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;
-        if EqualSid(sid_ptr, psid) != 0 && (ace.Mask & deny_read_mask) != 0 {
+        if EqualSid(sid_ptr, psid) != 0 && (ace.Mask & deny_mask) != 0 {
             return true;
         }
     }
@@ -522,17 +530,33 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
     sids: &[*mut c_void],
     allow_mask: u32,
     disallow_mask: u32,
+    access_mode: ACCESS_MODE,
     inheritance: u32,
 ) -> Result<bool> {
     let (p_dacl, p_sd) = fetch_dacl_handle(path)?;
     let mut entries: Vec<EXPLICIT_ACCESS_W> = Vec::new();
     for sid in sids {
+        // An explicit allow can outrank an inherited deny. All file generic rights
+        // overlap read/execute through READ_CONTROL or SYNCHRONIZE.
+        if access_mode == GRANT_ACCESS
+            && dacl_has_deny_mask_for_sid(
+                p_dacl,
+                *sid,
+                allow_mask
+                    | GENERIC_READ_MASK
+                    | GENERIC_WRITE_MASK
+                    | GENERIC_EXECUTE_MASK
+                    | GENERIC_ALL_MASK,
+            )
+        {
+            continue;
+        }
         if !dacl_allow_mask_needs_refresh(p_dacl, *sid, allow_mask, disallow_mask) {
             continue;
         }
         entries.push(EXPLICIT_ACCESS_W {
             grfAccessPermissions: allow_mask,
-            grfAccessMode: 2, // SET_ACCESS
+            grfAccessMode: access_mode,
             grfInheritance: inheritance,
             Trustee: TRUSTEE_W {
                 pMultipleTrustee: std::ptr::null_mut(),
@@ -605,6 +629,27 @@ pub unsafe fn ensure_allow_mask_aces_with_inheritance(
         sids,
         allow_mask,
         /*disallow_mask*/ 0,
+        SET_ACCESS,
+        inheritance,
+    )
+}
+
+/// Ensure read/execute grants without replacing existing entries or overriding denies.
+/// Returns true if any ACE was added.
+///
+/// # Safety
+/// Caller must pass valid SID pointers and an existing path.
+pub unsafe fn grant_read_execute_aces(
+    path: &Path,
+    sids: &[*mut c_void],
+    inheritance: u32,
+) -> Result<bool> {
+    ensure_allow_mask_aces_with_inheritance_impl(
+        path,
+        sids,
+        FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
+        /*disallow_mask*/ 0,
+        GRANT_ACCESS,
         inheritance,
     )
 }
@@ -638,6 +683,7 @@ pub unsafe fn ensure_allow_write_aces(path: &Path, sids: &[*mut c_void]) -> Resu
         sids,
         WRITE_ALLOW_MASK,
         FILE_DELETE_CHILD,
+        SET_ACCESS,
         CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
     )
 }

codex-rs/windows-sandbox-rs/src/bin/setup_main/win/setup_runtime_bin.rs

@@ -1,10 +1,14 @@
 use std::ffi::c_void;
 use std::io::Write;
 use std::os::windows::fs::MetadataExt as _;
+use std::path::Path;
 use std::path::PathBuf;
 
+use anyhow::Context;
 use anyhow::Result;
+use anyhow::ensure;
 use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
+use codex_windows_sandbox::grant_read_execute_aces;
 use codex_windows_sandbox::path_mask_allows;
 use windows_sys::Win32::Security::CONTAINER_INHERIT_ACE;
 use windows_sys::Win32::Security::OBJECT_INHERIT_ACE;
@@ -22,8 +26,9 @@ pub(super) fn ensure_codex_app_runtime_paths_readable(
     log: &mut dyn Write,
 ) -> Result<()> {
     let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE;
+    let local_app_data = local_app_data_root();
     let runtime_paths = runtime_paths(
-        local_app_data_root(),
+        local_app_data.clone(),
         std::env::var_os("USERPROFILE").map(PathBuf::from),
     );
 
@@ -89,6 +94,93 @@ pub(super) fn ensure_codex_app_runtime_paths_readable(
             )?;
         }
     }
+    if let Some(local_app_data) = local_app_data {
+        let runtime_root = local_app_data.join("OpenAI").join("Codex").join("runtimes");
+        if let Err(err) = ensure_runtime_tree_readable(&runtime_root, sandbox_group_psid) {
+            let message = format!("runtime read/execute validation failed: {err:#}");
+            super::log_line(log, &message)?;
+            refresh_errors.push(message);
+        }
+    }
+    Ok(())
+}
+
+fn ensure_runtime_tree_readable(
+    runtime_root: &Path,
+    sandbox_group_psid: *mut c_void,
+) -> Result<()> {
+    for ancestor in runtime_root.ancestors() {
+        let metadata = match std::fs::symlink_metadata(ancestor) {
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()),
+            result => result
+                .with_context(|| format!("inspect runtime ancestor {}", ancestor.display()))?,
+        };
+        if !metadata.is_dir() || metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 {
+            return Ok(());
+        }
+    }
+
+    let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE;
+    let mut paths = vec![runtime_root.to_path_buf()];
+    while let Some(path) = paths.pop() {
+        let metadata = match std::fs::symlink_metadata(&path) {
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
+            result => result.with_context(|| format!("inspect runtime path {}", path.display()))?,
+        };
+        if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 {
+            continue;
+        }
+
+        // A readable parent does not prove that existing runtime children inherited its ACL.
+        let access_result = unsafe {
+            grant_read_execute_aces(
+                &path,
+                &[sandbox_group_psid],
+                if metadata.is_dir() {
+                    OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE
+                } else {
+                    0
+                },
+            )
+        }
+        .and_then(|changed| {
+            if changed {
+                ensure!(
+                    path_mask_allows(
+                        &path,
+                        &[sandbox_group_psid],
+                        read_execute_mask,
+                        /*require_all_bits*/ true,
+                    )?,
+                    "runtime read/execute access still missing"
+                );
+            }
+            Ok(())
+        });
+        if let Err(err) = access_result {
+            if std::fs::symlink_metadata(&path)
+                .is_err_and(|err| err.kind() == std::io::ErrorKind::NotFound)
+            {
+                continue;
+            }
+            return Err(err).with_context(|| {
+                format!("validate runtime read/execute access on {}", path.display())
+            });
+        }
+        if metadata.is_dir() {
+            let entries = match std::fs::read_dir(&path) {
+                Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
+                result => result
+                    .with_context(|| format!("enumerate runtime directory {}", path.display()))?,
+            };
+            for entry in entries {
+                match entry {
+                    Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
+                    result => paths.push(result?.path()),
+                }
+            }
+        }
+    }
     Ok(())
 }
 

codex-rs/windows-sandbox-rs/src/bin/setup_main/win/setup_runtime_bin_tests.rs

@@ -1,6 +1,124 @@
+use super::ensure_runtime_tree_readable;
 use super::runtime_paths;
+use codex_windows_sandbox::LocalSid;
+use codex_windows_sandbox::add_deny_read_ace;
+use codex_windows_sandbox::add_deny_write_ace;
+use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
+use codex_windows_sandbox::path_mask_allows;
 use pretty_assertions::assert_eq;
+use std::fs;
 use std::path::PathBuf;
+use windows_sys::Win32::Storage::FileSystem::DELETE;
+use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
+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_WRITE_DATA;
+use windows_sys::Win32::Storage::FileSystem::WRITE_DAC;
+
+#[test]
+fn repairs_children_when_runtime_root_already_has_read_execute_access() {
+    let runtime = tempfile::tempdir().expect("runtime directory");
+    let node = runtime.path().join("bin/node.exe");
+    let module = runtime.path().join("sky/dist/project/index.js");
+    for path in [&node, &module] {
+        fs::create_dir_all(path.parent().expect("parent")).expect("runtime subdirectory");
+        fs::write(path, b"runtime content").expect("runtime file");
+    }
+    let sandbox_sid = LocalSid::from_string("S-1-5-21-10-20-30-40").expect("test SID");
+    let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE;
+    unsafe {
+        ensure_allow_mask_aces_with_inheritance(
+            runtime.path(),
+            &[sandbox_sid.as_ptr()],
+            read_execute_mask,
+            /*inheritance*/ 0,
+        )
+    }
+    .expect("grant root-only read/execute access");
+    assert!(
+        path_mask_allows(
+            runtime.path(),
+            &[sandbox_sid.as_ptr()],
+            read_execute_mask,
+            /*require_all_bits*/ true,
+        )
+        .expect("root access")
+    );
+    for path in [&node, &module] {
+        assert!(
+            !path_mask_allows(
+                path,
+                &[sandbox_sid.as_ptr()],
+                read_execute_mask,
+                /*require_all_bits*/ true,
+            )
+            .expect("child access before repair")
+        );
+    }
+
+    let denied_directory = runtime.path().join("denied");
+    fs::create_dir(&denied_directory).expect("denied directory");
+    assert!(
+        unsafe { add_deny_read_ace(&denied_directory, sandbox_sid.as_ptr()) }
+            .expect("deny directory reads")
+    );
+    let denied_child = denied_directory.join("private.js");
+    fs::write(&denied_child, b"private runtime content").expect("denied child");
+    let write_denied_file = runtime.path().join("read-only.js");
+    fs::write(&write_denied_file, b"read-only runtime content").expect("write-denied file");
+    assert!(
+        unsafe { add_deny_write_ace(&write_denied_file, sandbox_sid.as_ptr()) }
+            .expect("deny runtime writes")
+    );
+
+    for _ in 0..2 {
+        ensure_runtime_tree_readable(runtime.path(), sandbox_sid.as_ptr())
+            .expect("repair runtimes");
+        for path in [&denied_directory, &denied_child] {
+            assert!(
+                !unsafe { add_deny_read_ace(path, sandbox_sid.as_ptr()) }
+                    .expect("read denial must survive repair")
+            );
+            assert!(
+                !path_mask_allows(
+                    path,
+                    &[sandbox_sid.as_ptr()],
+                    read_execute_mask,
+                    /*require_all_bits*/ false,
+                )
+                .expect("repair must not add an allow ahead of inherited denial")
+            );
+        }
+        assert!(
+            !unsafe { add_deny_write_ace(&write_denied_file, sandbox_sid.as_ptr()) }
+                .expect("write denial must survive repair")
+        );
+        for path in [
+            node.as_path(),
+            module.as_path(),
+            module.parent().expect("module directory"),
+        ] {
+            assert!(
+                path_mask_allows(
+                    path,
+                    &[sandbox_sid.as_ptr()],
+                    read_execute_mask,
+                    /*require_all_bits*/ true,
+                )
+                .expect("child access after repair")
+            );
+            assert!(
+                !path_mask_allows(
+                    path,
+                    &[sandbox_sid.as_ptr()],
+                    FILE_WRITE_DATA | FILE_APPEND_DATA | DELETE | WRITE_DAC,
+                    /*require_all_bits*/ false,
+                )
+                .expect("repair must not grant write or ACL management")
+            );
+        }
+    }
+}
 
 #[test]
 fn runtime_paths_include_desktop_parent_and_primary_runtime_roots() {

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

@@ -7,6 +7,7 @@ use crate::setup::SandboxSetupRequest;
 use crate::setup::SetupRootOverrides;
 use crate::spawn_prep::legacy_session_capability_roots;
 use crate::spawn_prep::prepare_legacy_session_security;
+use crate::winutil::current_account_name;
 use anyhow::Result;
 use codex_protocol::models::PermissionProfile;
 use codex_protocol::permissions::NetworkSandboxPolicy;
@@ -20,10 +21,6 @@ use std::os::windows::io::FromRawHandle;
 use std::os::windows::io::OwnedHandle;
 use std::path::Path;
 use tempfile::TempDir;
-use windows_sys::Win32::NetworkManagement::NetManagement::DNLEN;
-use windows_sys::Win32::NetworkManagement::NetManagement::UNLEN;
-use windows_sys::Win32::Security::Authentication::Identity::GetUserNameExW;
-use windows_sys::Win32::Security::Authentication::Identity::NameSamCompatible;
 
 #[test]
 fn private_desktop_rejects_interactive_and_injected_names() {
@@ -219,16 +216,6 @@ fn legacy_desktop_reuses_only_equivalent_permissions() -> Result<()> {
     Ok(())
 }
 
-fn current_account_name() -> Result<String> {
-    // Bazel does not provide USERDOMAIN or USERNAME in the test environment.
-    let mut account = [0; (DNLEN + UNLEN + 2) as usize];
-    let mut length = account.len() as u32;
-    if unsafe { GetUserNameExW(NameSamCompatible, account.as_mut_ptr(), &mut length) } == 0 {
-        return Err(std::io::Error::last_os_error().into());
-    }
-    Ok(String::from_utf16(&account[..length as usize])?)
-}
-
 fn workspace_permissions(workspace: &Path) -> Result<ResolvedWindowsSandboxPermissions> {
     ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
         &PermissionProfile::workspace_write_with(

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

@@ -170,6 +170,8 @@ pub use acl::ensure_allow_write_aces;
 #[cfg(target_os = "windows")]
 pub use acl::fetch_dacl_handle;
 #[cfg(target_os = "windows")]
+pub use acl::grant_read_execute_aces;
+#[cfg(target_os = "windows")]
 pub use acl::path_has_standard_user_mutation_allow;
 #[cfg(target_os = "windows")]
 pub use acl::path_has_trusted_system_owner;

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

@@ -33,6 +33,7 @@ use crate::setup_error::extract_failure;
 use crate::setup_error::failure;
 use crate::setup_error::read_setup_error_report;
 use crate::ssh_config_dependencies::ssh_config_dependency_paths;
+use crate::winutil::current_account_name;
 use anyhow::Result;
 use anyhow::anyhow;
 use base64::Engine;
@@ -348,7 +349,7 @@ fn run_setup_refresh_inner(
         proxy_ports: offline_proxy_settings.proxy_ports,
         allow_local_binding: offline_proxy_settings.allow_local_binding,
         otel: None,
-        real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()),
+        real_user: current_account_name()?,
         mode: SetupMode::Full,
         refresh_only: true,
     };
@@ -1065,7 +1066,7 @@ fn run_elevated_setup_inner(
             format!("failed to create sandbox dir {}: {err}", sbx_dir.display()),
         )
     })?;
-    let payload = elevated_provisioning_payload(&request, offline_proxy_settings_override);
+    let payload = elevated_provisioning_payload(&request, offline_proxy_settings_override)?;
     let needs_elevation = !is_elevated().map_err(|err| {
         failure(
             SetupErrorCode::OrchestratorElevationCheckFailed,
@@ -1078,10 +1079,10 @@ fn run_elevated_setup_inner(
 fn elevated_provisioning_payload(
     request: &SandboxSetupRequest<'_>,
     offline_proxy_settings_override: Option<&OfflineProxySettings>,
-) -> ElevationPayload {
+) -> Result<ElevationPayload> {
     let offline_proxy_settings =
         offline_proxy_settings_for_request(request, offline_proxy_settings_override);
-    ElevationPayload {
+    Ok(ElevationPayload {
         version: SETUP_VERSION,
         offline_username: OFFLINE_USERNAME.to_string(),
         online_username: ONLINE_USERNAME.to_string(),
@@ -1093,11 +1094,11 @@ fn elevated_provisioning_payload(
         deny_write_paths: Vec::new(),
         proxy_ports: offline_proxy_settings.proxy_ports,
         allow_local_binding: offline_proxy_settings.allow_local_binding,
-        real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()),
+        real_user: current_account_name()?,
         otel: codex_otel::global_statsig_metrics_settings(),
         mode: SetupMode::InteractiveProvision,
         refresh_only: false,
-    }
+    })
 }
 
 pub fn run_elevated_provisioning_setup(
@@ -1600,7 +1601,8 @@ mod tests {
 
         let payload = super::elevated_provisioning_payload(
             &request, /*offline_proxy_settings_override*/ None,
-        );
+        )
+        .expect("elevated provisioning payload");
 
         assert_eq!(payload.command_cwd, codex_home);
         assert_eq!(payload.read_roots, Vec::<PathBuf>::new());

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

@@ -6,6 +6,7 @@ use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
 use windows_sys::Win32::Foundation::GetLastError;
 use windows_sys::Win32::Foundation::HLOCAL;
 use windows_sys::Win32::Foundation::LocalFree;
+use windows_sys::Win32::NetworkManagement::NetManagement::DNLEN;
 use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_INFO_1;
 use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success;
 use windows_sys::Win32::NetworkManagement::NetManagement::NERR_UserNotFound;
@@ -13,8 +14,11 @@ use windows_sys::Win32::NetworkManagement::NetManagement::NetApiBufferFree;
 use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAdd;
 use windows_sys::Win32::NetworkManagement::NetManagement::NetUserGetInfo;
 use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo;
+use windows_sys::Win32::NetworkManagement::NetManagement::UNLEN;
 use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1;
 use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1008;
+use windows_sys::Win32::Security::Authentication::Identity::GetUserNameExW;
+use windows_sys::Win32::Security::Authentication::Identity::NameSamCompatible;
 use windows_sys::Win32::Security::Authorization::ConvertSidToStringSidW;
 use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW;
 use windows_sys::Win32::Security::CopySid;
@@ -29,6 +33,16 @@ use windows_sys::Win32::System::Diagnostics::Debug::FormatMessageW;
 pub const SANDBOX_USERS_GROUP: &str = "CodexSandboxUsers";
 const SANDBOX_USERS_GROUP_COMMENT: &str = "Codex sandbox internal group (managed)";
 
+pub(crate) fn current_account_name() -> Result<String> {
+    // Sandbox launchers can filter USERNAME, so resolve the caller from its Windows token.
+    let mut account = [0; (DNLEN + UNLEN + 2) as usize];
+    let mut length = account.len() as u32;
+    if unsafe { GetUserNameExW(NameSamCompatible, account.as_mut_ptr(), &mut length) } == 0 {
+        return Err(std::io::Error::last_os_error()).context("resolve current Windows account");
+    }
+    Ok(String::from_utf16(&account[..length as usize])?)
+}
+
 pub fn to_wide<S: AsRef<OsStr>>(s: S) -> Vec<u16> {
     let mut v: Vec<u16> = s.as_ref().encode_wide().collect();
     v.push(0);