use a SandboxUsers group for ACLs instead of granting to each sandbox user separately (#8483)

This is more future-proof if we ever decide to add additional Sandbox
Users for new functionality

This also moves some more user-related code into a new file for code
cleanliness

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

@@ -0,0 +1,306 @@
+#![cfg(target_os = "windows")]
+
+use anyhow::Result;
+use base64::engine::general_purpose::STANDARD as BASE64;
+use base64::Engine;
+use rand::rngs::SmallRng;
+use rand::RngCore;
+use rand::SeedableRng;
+use serde::Serialize;
+use std::ffi::c_void;
+use std::ffi::OsStr;
+use std::fs::File;
+use std::path::Path;
+use std::path::PathBuf;
+use windows_sys::Win32::Foundation::GetLastError;
+use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
+use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success;
+use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAdd;
+use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAddMembers;
+use windows_sys::Win32::NetworkManagement::NetManagement::NetUserAdd;
+use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo;
+use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_INFO_1;
+use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_MEMBERS_INFO_3;
+use windows_sys::Win32::NetworkManagement::NetManagement::UF_DONT_EXPIRE_PASSWD;
+use windows_sys::Win32::NetworkManagement::NetManagement::UF_SCRIPT;
+use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1;
+use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1003;
+use windows_sys::Win32::NetworkManagement::NetManagement::USER_PRIV_USER;
+use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW;
+use windows_sys::Win32::Security::LookupAccountNameW;
+use windows_sys::Win32::Security::SID_NAME_USE;
+
+use codex_windows_sandbox::dpapi_protect;
+use codex_windows_sandbox::sandbox_dir;
+use codex_windows_sandbox::string_from_sid_bytes;
+use codex_windows_sandbox::to_wide;
+use codex_windows_sandbox::SETUP_VERSION;
+
+pub const SANDBOX_USERS_GROUP: &str = "CodexSandboxUsers";
+const SANDBOX_USERS_GROUP_COMMENT: &str = "Codex sandbox internal group (managed)";
+
+pub fn ensure_sandbox_users_group(log: &mut File) -> Result<()> {
+    ensure_local_group(SANDBOX_USERS_GROUP, SANDBOX_USERS_GROUP_COMMENT, log)
+}
+
+pub fn resolve_sandbox_users_group_sid() -> Result<Vec<u8>> {
+    resolve_sid(SANDBOX_USERS_GROUP)
+}
+
+pub fn provision_sandbox_users(
+    codex_home: &Path,
+    offline_username: &str,
+    online_username: &str,
+    log: &mut File,
+) -> Result<()> {
+    ensure_sandbox_users_group(log)?;
+    super::log_line(
+        log,
+        &format!("ensuring sandbox users offline={offline_username} online={online_username}"),
+    )?;
+    let offline_password = random_password();
+    let online_password = random_password();
+    ensure_sandbox_user(offline_username, &offline_password, log)?;
+    ensure_sandbox_user(online_username, &online_password, log)?;
+    write_secrets(
+        codex_home,
+        offline_username,
+        &offline_password,
+        online_username,
+        &online_password,
+    )?;
+    Ok(())
+}
+
+pub fn ensure_sandbox_user(username: &str, password: &str, log: &mut File) -> Result<()> {
+    ensure_local_user(username, password, log)?;
+    ensure_local_group_member(SANDBOX_USERS_GROUP, username)?;
+    Ok(())
+}
+
+pub fn ensure_local_user(name: &str, password: &str, log: &mut File) -> Result<()> {
+    let name_w = to_wide(OsStr::new(name));
+    let pwd_w = to_wide(OsStr::new(password));
+    unsafe {
+        let info = USER_INFO_1 {
+            usri1_name: name_w.as_ptr() as *mut u16,
+            usri1_password: pwd_w.as_ptr() as *mut u16,
+            usri1_password_age: 0,
+            usri1_priv: USER_PRIV_USER,
+            usri1_home_dir: std::ptr::null_mut(),
+            usri1_comment: std::ptr::null_mut(),
+            usri1_flags: UF_SCRIPT | UF_DONT_EXPIRE_PASSWD,
+            usri1_script_path: std::ptr::null_mut(),
+        };
+        let status = NetUserAdd(
+            std::ptr::null(),
+            1,
+            &info as *const _ as *mut u8,
+            std::ptr::null_mut(),
+        );
+        if status != NERR_Success {
+            // Try update password via level 1003.
+            let pw_info = USER_INFO_1003 {
+                usri1003_password: pwd_w.as_ptr() as *mut u16,
+            };
+            let upd = NetUserSetInfo(
+                std::ptr::null(),
+                name_w.as_ptr(),
+                1003,
+                &pw_info as *const _ as *mut u8,
+                std::ptr::null_mut(),
+            );
+            if upd != NERR_Success {
+                super::log_line(log, &format!("NetUserSetInfo failed for {name} code {upd}"))?;
+                return Err(anyhow::anyhow!(
+                    "failed to create/update user {name}, code {status}/{upd}"
+                ));
+            }
+        }
+
+        // Ensure the principal is a regular local user account.
+        let group = to_wide(OsStr::new("Users"));
+        let member = LOCALGROUP_MEMBERS_INFO_3 {
+            lgrmi3_domainandname: name_w.as_ptr() as *mut u16,
+        };
+        let _ = NetLocalGroupAddMembers(
+            std::ptr::null(),
+            group.as_ptr(),
+            3,
+            &member as *const _ as *mut u8,
+            1,
+        );
+    }
+    Ok(())
+}
+
+pub fn ensure_local_group(name: &str, comment: &str, log: &mut File) -> Result<()> {
+    const ERROR_ALIAS_EXISTS: u32 = 1379;
+    const NERR_GROUP_EXISTS: u32 = 2223;
+
+    let name_w = to_wide(OsStr::new(name));
+    let comment_w = to_wide(OsStr::new(comment));
+    unsafe {
+        let info = LOCALGROUP_INFO_1 {
+            lgrpi1_name: name_w.as_ptr() as *mut u16,
+            lgrpi1_comment: comment_w.as_ptr() as *mut u16,
+        };
+        let mut parm_err: u32 = 0;
+        let status = NetLocalGroupAdd(
+            std::ptr::null(),
+            1,
+            &info as *const _ as *mut u8,
+            &mut parm_err as *mut _,
+        );
+        if status != NERR_Success && status != ERROR_ALIAS_EXISTS && status != NERR_GROUP_EXISTS {
+            super::log_line(
+                log,
+                &format!("NetLocalGroupAdd failed for {name} code {status} parm_err={parm_err}"),
+            )?;
+            anyhow::bail!("failed to create local group {name}, code {status}");
+        }
+    }
+    Ok(())
+}
+
+pub fn ensure_local_group_member(group_name: &str, member_name: &str) -> Result<()> {
+    // If the member is already in the group, NetLocalGroupAddMembers may
+    // return an error code. We don't care.
+    let group_w = to_wide(OsStr::new(group_name));
+    let member_w = to_wide(OsStr::new(member_name));
+    unsafe {
+        let member = LOCALGROUP_MEMBERS_INFO_3 {
+            lgrmi3_domainandname: member_w.as_ptr() as *mut u16,
+        };
+        let _ = NetLocalGroupAddMembers(
+            std::ptr::null(),
+            group_w.as_ptr(),
+            3,
+            &member as *const _ as *mut u8,
+            1,
+        );
+    }
+    Ok(())
+}
+
+pub fn resolve_sid(name: &str) -> Result<Vec<u8>> {
+    let name_w = to_wide(OsStr::new(name));
+    let mut sid_buffer = vec![0u8; 68];
+    let mut sid_len: u32 = sid_buffer.len() as u32;
+    let mut domain: Vec<u16> = Vec::new();
+    let mut domain_len: u32 = 0;
+    let mut use_type: SID_NAME_USE = 0;
+    loop {
+        let ok = unsafe {
+            LookupAccountNameW(
+                std::ptr::null(),
+                name_w.as_ptr(),
+                sid_buffer.as_mut_ptr() as *mut c_void,
+                &mut sid_len,
+                domain.as_mut_ptr(),
+                &mut domain_len,
+                &mut use_type,
+            )
+        };
+        if ok != 0 {
+            sid_buffer.truncate(sid_len as usize);
+            return Ok(sid_buffer);
+        }
+        let err = unsafe { GetLastError() };
+        if err == ERROR_INSUFFICIENT_BUFFER {
+            sid_buffer.resize(sid_len as usize, 0);
+            domain.resize(domain_len as usize, 0);
+            continue;
+        }
+        return Err(anyhow::anyhow!(
+            "LookupAccountNameW failed for {name}: {err}"
+        ));
+    }
+}
+
+pub fn sid_bytes_to_psid(sid: &[u8]) -> Result<*mut c_void> {
+    let sid_str = string_from_sid_bytes(sid).map_err(anyhow::Error::msg)?;
+    let sid_w = to_wide(OsStr::new(&sid_str));
+    let mut psid: *mut c_void = std::ptr::null_mut();
+    if unsafe { ConvertStringSidToSidW(sid_w.as_ptr(), &mut psid) } == 0 {
+        return Err(anyhow::anyhow!(
+            "ConvertStringSidToSidW failed: {}",
+            unsafe { GetLastError() }
+        ));
+    }
+    Ok(psid)
+}
+
+fn random_password() -> String {
+    const CHARS: &[u8] =
+        b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*()-_=+";
+    let mut rng = SmallRng::from_entropy();
+    let mut buf = [0u8; 24];
+    rng.fill_bytes(&mut buf);
+    buf.iter()
+        .map(|b| {
+            let idx = (*b as usize) % CHARS.len();
+            CHARS[idx] as char
+        })
+        .collect()
+}
+
+#[derive(Serialize)]
+struct SandboxUserRecord {
+    username: String,
+    password: String,
+}
+
+#[derive(Serialize)]
+struct SandboxUsersFile {
+    version: u32,
+    offline: SandboxUserRecord,
+    online: SandboxUserRecord,
+}
+
+#[derive(Serialize)]
+struct SetupMarker {
+    version: u32,
+    offline_username: String,
+    online_username: String,
+    created_at: String,
+    read_roots: Vec<PathBuf>,
+    write_roots: Vec<PathBuf>,
+}
+
+fn write_secrets(
+    codex_home: &Path,
+    offline_user: &str,
+    offline_pwd: &str,
+    online_user: &str,
+    online_pwd: &str,
+) -> Result<()> {
+    let sandbox_dir = sandbox_dir(codex_home);
+    std::fs::create_dir_all(&sandbox_dir)?;
+    let offline_blob = dpapi_protect(offline_pwd.as_bytes())?;
+    let online_blob = dpapi_protect(online_pwd.as_bytes())?;
+    let users = SandboxUsersFile {
+        version: SETUP_VERSION,
+        offline: SandboxUserRecord {
+            username: offline_user.to_string(),
+            password: BASE64.encode(offline_blob),
+        },
+        online: SandboxUserRecord {
+            username: online_user.to_string(),
+            password: BASE64.encode(online_blob),
+        },
+    };
+    let marker = SetupMarker {
+        version: SETUP_VERSION,
+        offline_username: offline_user.to_string(),
+        online_username: online_user.to_string(),
+        created_at: chrono::Utc::now().to_rfc3339(),
+        read_roots: Vec::new(),
+        write_roots: Vec::new(),
+    };
+    let users_path = sandbox_dir.join("sandbox_users.json");
+    let marker_path = sandbox_dir.join("setup_marker.json");
+    std::fs::write(users_path, serde_json::to_vec_pretty(&users)?)?;
+    std::fs::write(marker_path, serde_json::to_vec_pretty(&marker)?)?;
+    Ok(())
+}

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

@@ -5,7 +5,6 @@ use anyhow::Result;
 use base64::engine::general_purpose::STANDARD as BASE64;
 use base64::Engine;
 use codex_windows_sandbox::convert_string_sid_to_sid;
-use codex_windows_sandbox::dpapi_protect;
 use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
 use codex_windows_sandbox::ensure_allow_write_aces;
 use codex_windows_sandbox::load_or_create_cap_sids;
@@ -13,11 +12,9 @@ use codex_windows_sandbox::log_note;
 use codex_windows_sandbox::path_mask_allows;
 use codex_windows_sandbox::sandbox_dir;
 use codex_windows_sandbox::string_from_sid_bytes;
+use codex_windows_sandbox::to_wide;
 use codex_windows_sandbox::LOG_FILE_NAME;
 use codex_windows_sandbox::SETUP_VERSION;
-use rand::rngs::SmallRng;
-use rand::RngCore;
-use rand::SeedableRng;
 use serde::Deserialize;
 use serde::Serialize;
 use std::collections::HashSet;
@@ -25,7 +22,6 @@ use std::ffi::c_void;
 use std::ffi::OsStr;
 use std::fs::File;
 use std::io::Write;
-use std::os::windows::ffi::OsStrExt;
 use std::os::windows::process::CommandExt;
 use std::path::Path;
 use std::path::PathBuf;
@@ -50,18 +46,7 @@ use windows::Win32::System::Com::CLSCTX_INPROC_SERVER;
 use windows::Win32::System::Com::COINIT_APARTMENTTHREADED;
 use windows_sys::Win32::Foundation::GetLastError;
 use windows_sys::Win32::Foundation::LocalFree;
-use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
 use windows_sys::Win32::Foundation::HLOCAL;
-use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success;
-use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAddMembers;
-use windows_sys::Win32::NetworkManagement::NetManagement::NetUserAdd;
-use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo;
-use windows_sys::Win32::NetworkManagement::NetManagement::LOCALGROUP_MEMBERS_INFO_3;
-use windows_sys::Win32::NetworkManagement::NetManagement::UF_DONT_EXPIRE_PASSWD;
-use windows_sys::Win32::NetworkManagement::NetManagement::UF_SCRIPT;
-use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1;
-use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1003;
-use windows_sys::Win32::NetworkManagement::NetManagement::USER_PRIV_USER;
 use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW;
 use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
 use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
@@ -70,12 +55,10 @@ use windows_sys::Win32::Security::Authorization::GRANT_ACCESS;
 use windows_sys::Win32::Security::Authorization::SE_FILE_OBJECT;
 use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
 use windows_sys::Win32::Security::Authorization::TRUSTEE_W;
-use windows_sys::Win32::Security::LookupAccountNameW;
 use windows_sys::Win32::Security::ACL;
 use windows_sys::Win32::Security::CONTAINER_INHERIT_ACE;
 use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
 use windows_sys::Win32::Security::OBJECT_INHERIT_ACE;
-use windows_sys::Win32::Security::SID_NAME_USE;
 use windows_sys::Win32::Storage::FileSystem::DELETE;
 use windows_sys::Win32::Storage::FileSystem::FILE_DELETE_CHILD;
 use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE;
@@ -83,8 +66,13 @@ use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ;
 use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
 
 mod read_acl_mutex;
+mod sandbox_users;
 use read_acl_mutex::acquire_read_acl_mutex;
 use read_acl_mutex::read_acl_mutex_exists;
+use sandbox_users::provision_sandbox_users;
+use sandbox_users::resolve_sandbox_users_group_sid;
+use sandbox_users::resolve_sid;
+use sandbox_users::sid_bytes_to_psid;
 
 #[derive(Debug, Clone, Deserialize, Serialize)]
 struct Payload {
@@ -114,158 +102,12 @@ impl Default for SetupMode {
     }
 }
 
-#[derive(Serialize)]
-struct SandboxUserRecord {
-    username: String,
-    password: String,
-}
-
-#[derive(Serialize)]
-struct SandboxUsersFile {
-    version: u32,
-    offline: SandboxUserRecord,
-    online: SandboxUserRecord,
-}
-
-#[derive(Serialize)]
-struct SetupMarker {
-    version: u32,
-    offline_username: String,
-    online_username: String,
-    created_at: String,
-    read_roots: Vec<PathBuf>,
-    write_roots: Vec<PathBuf>,
-}
-
 fn log_line(log: &mut File, msg: &str) -> Result<()> {
     let ts = chrono::Utc::now().to_rfc3339();
     writeln!(log, "[{ts}] {msg}")?;
     Ok(())
 }
 
-fn to_wide(s: &OsStr) -> Vec<u16> {
-    let mut v: Vec<u16> = s.encode_wide().collect();
-    v.push(0);
-    v
-}
-
-fn random_password() -> String {
-    const CHARS: &[u8] =
-        b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*()-_=+";
-    let mut rng = SmallRng::from_entropy();
-    let mut buf = [0u8; 24];
-    rng.fill_bytes(&mut buf);
-    buf.iter()
-        .map(|b| {
-            let idx = (*b as usize) % CHARS.len();
-            CHARS[idx] as char
-        })
-        .collect()
-}
-
-fn sid_bytes_to_psid(sid: &[u8]) -> Result<*mut c_void> {
-    let sid_str = string_from_sid_bytes(sid).map_err(anyhow::Error::msg)?;
-    let sid_w = to_wide(OsStr::new(&sid_str));
-    let mut psid: *mut c_void = std::ptr::null_mut();
-    if unsafe { ConvertStringSidToSidW(sid_w.as_ptr(), &mut psid) } == 0 {
-        return Err(anyhow::anyhow!(
-            "ConvertStringSidToSidW failed: {}",
-            unsafe { GetLastError() }
-        ));
-    }
-    Ok(psid)
-}
-
-fn ensure_local_user(name: &str, password: &str, log: &mut File) -> Result<()> {
-    let name_w = to_wide(OsStr::new(name));
-    let pwd_w = to_wide(OsStr::new(password));
-    unsafe {
-        let info = USER_INFO_1 {
-            usri1_name: name_w.as_ptr() as *mut u16,
-            usri1_password: pwd_w.as_ptr() as *mut u16,
-            usri1_password_age: 0,
-            usri1_priv: USER_PRIV_USER,
-            usri1_home_dir: std::ptr::null_mut(),
-            usri1_comment: std::ptr::null_mut(),
-            usri1_flags: UF_SCRIPT | UF_DONT_EXPIRE_PASSWD,
-            usri1_script_path: std::ptr::null_mut(),
-        };
-        let status = NetUserAdd(
-            std::ptr::null(),
-            1,
-            &info as *const _ as *mut u8,
-            std::ptr::null_mut(),
-        );
-        if status != NERR_Success {
-            // Try update password via level 1003.
-            let pw_info = USER_INFO_1003 {
-                usri1003_password: pwd_w.as_ptr() as *mut u16,
-            };
-            let upd = NetUserSetInfo(
-                std::ptr::null(),
-                name_w.as_ptr(),
-                1003,
-                &pw_info as *const _ as *mut u8,
-                std::ptr::null_mut(),
-            );
-            if upd != NERR_Success {
-                log_line(log, &format!("NetUserSetInfo failed for {name} code {upd}"))?;
-                return Err(anyhow::anyhow!(
-                    "failed to create/update user {name}, code {status}/{upd}"
-                ));
-            }
-        }
-        let group = to_wide(OsStr::new("Users"));
-        let member = LOCALGROUP_MEMBERS_INFO_3 {
-            lgrmi3_domainandname: name_w.as_ptr() as *mut u16,
-        };
-        let _ = NetLocalGroupAddMembers(
-            std::ptr::null(),
-            group.as_ptr(),
-            3,
-            &member as *const _ as *mut u8,
-            1,
-        );
-    }
-    Ok(())
-}
-
-fn resolve_sid(name: &str) -> Result<Vec<u8>> {
-    let name_w = to_wide(OsStr::new(name));
-    let mut sid_buffer = vec![0u8; 68];
-    let mut sid_len: u32 = sid_buffer.len() as u32;
-    let mut domain: Vec<u16> = Vec::new();
-    let mut domain_len: u32 = 0;
-    let mut use_type: SID_NAME_USE = 0;
-    loop {
-        let ok = unsafe {
-            LookupAccountNameW(
-                std::ptr::null(),
-                name_w.as_ptr(),
-                sid_buffer.as_mut_ptr() as *mut c_void,
-                &mut sid_len,
-                domain.as_mut_ptr(),
-                &mut domain_len,
-                &mut use_type,
-            )
-        };
-        if ok != 0 {
-            sid_buffer.truncate(sid_len as usize);
-            return Ok(sid_buffer);
-        }
-        let err = unsafe { GetLastError() };
-        if err == ERROR_INSUFFICIENT_BUFFER {
-            sid_buffer.resize(sid_len as usize, 0);
-            domain.resize(domain_len as usize, 0);
-            continue;
-        }
-        return Err(anyhow::anyhow!(
-            "LookupAccountNameW failed for {name}: {}",
-            err
-        ));
-    }
-}
-
 fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
     let mut read_payload = payload.clone();
     read_payload.mode = SetupMode::ReadAclsOnly;
@@ -285,8 +127,7 @@ fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
 }
 
 struct ReadAclSubjects<'a> {
-    offline_psid: *mut c_void,
-    online_psid: *mut c_void,
+    sandbox_group_psid: *mut c_void,
     rx_psids: &'a [*mut c_void],
 }
 
@@ -319,25 +160,16 @@ fn apply_read_acls(
         if builtin_has {
             continue;
         }
-        let offline_has = read_mask_allows_or_log(
+        let sandbox_has = read_mask_allows_or_log(
             root,
-            &[subjects.offline_psid],
-            Some("offline"),
+            &[subjects.sandbox_group_psid],
+            Some("sandbox_group"),
             access_mask,
             access_label,
             refresh_errors,
             log,
         )?;
-        let online_has = read_mask_allows_or_log(
-            root,
-            &[subjects.online_psid],
-            Some("online"),
-            access_mask,
-            access_label,
-            refresh_errors,
-            log,
-        )?;
-        if offline_has && online_has {
+        if sandbox_has {
             continue;
         }
         log_line(
@@ -347,55 +179,23 @@ fn apply_read_acls(
                 root.display()
             ),
         )?;
-        let mut successes = usize::from(offline_has) + usize::from(online_has);
-        let mut missing_psids: Vec<*mut c_void> = Vec::new();
-        let mut missing_labels: Vec<&str> = Vec::new();
-        if !offline_has {
-            missing_psids.push(subjects.offline_psid);
-            missing_labels.push("offline");
-        }
-        if !online_has {
-            missing_psids.push(subjects.online_psid);
-            missing_labels.push("online");
-        }
-        if !missing_psids.is_empty() {
-            let result = unsafe {
-                ensure_allow_mask_aces_with_inheritance(
-                    root,
-                    &missing_psids,
-                    access_mask,
-                    inheritance,
-                )
-            };
-            match result {
-                Ok(_) => {
-                    successes = 2;
-                }
-                Err(err) => {
-                    let label_list = missing_labels.join(", ");
-                    for label in &missing_labels {
-                        refresh_errors.push(format!(
-                            "grant {access_label} ACE failed on {} for {label}: {err}",
-                            root.display()
-                        ));
-                    }
-                    log_line(
-                        log,
-                        &format!(
-                            "grant {access_label} ACE failed on {} for {}: {err}",
-                            root.display(),
-                            label_list
-                        ),
-                    )?;
-                }
-            }
-        }
-        if successes == 2 {
-        } else {
+        let result = unsafe {
+            ensure_allow_mask_aces_with_inheritance(
+                root,
+                &[subjects.sandbox_group_psid],
+                access_mask,
+                inheritance,
+            )
+        };
+        if let Err(err) = result {
+            refresh_errors.push(format!(
+                "grant {access_label} ACE failed on {} for sandbox_group: {err}",
+                root.display()
+            ));
             log_line(
                 log,
                 &format!(
-                    "{access_label} ACE incomplete on {} (success {successes}/2)",
+                    "grant {access_label} ACE failed on {} for sandbox_group: {err}",
                     root.display()
                 ),
             )?;
@@ -515,7 +315,7 @@ fn run_netsh_firewall(sid: &str, log: &mut File) -> Result<()> {
 fn lock_sandbox_dir(
     dir: &Path,
     real_user: &str,
-    sandbox_user_sids: &[Vec<u8>],
+    sandbox_group_sid: &[u8],
     _log: &mut File,
 ) -> Result<()> {
     std::fs::create_dir_all(dir)?;
@@ -535,24 +335,15 @@ fn lock_sandbox_dir(
             real_sid,
             FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
         ),
+        (
+            sandbox_group_sid.to_vec(),
+            FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
+        ),
     ];
-    let sandbox_entries: Vec<(Vec<u8>, u32)> = sandbox_user_sids
-        .iter()
-        .map(|sid| {
-            (
-                sid.clone(),
-                FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
-            )
-        })
-        .collect();
     unsafe {
         let mut eas: Vec<EXPLICIT_ACCESS_W> = Vec::new();
         let mut sids: Vec<*mut c_void> = Vec::new();
-        for (sid_bytes, mask) in entries
-            .iter()
-            .map(|(s, m)| (s, *m))
-            .chain(sandbox_entries.iter().map(|(s, m)| (s, *m)))
-        {
+        for (sid_bytes, mask) in entries.iter().map(|(s, m)| (s, *m)) {
             let sid_str = string_from_sid_bytes(sid_bytes).map_err(anyhow::Error::msg)?;
             let sid_w = to_wide(OsStr::new(&sid_str));
             let mut psid: *mut c_void = std::ptr::null_mut();
@@ -617,45 +408,6 @@ fn lock_sandbox_dir(
     Ok(())
 }
 
-fn write_secrets(
-    codex_home: &Path,
-    offline_user: &str,
-    offline_pwd: &str,
-    online_user: &str,
-    online_pwd: &str,
-    _read_roots: &[PathBuf],
-    _write_roots: &[PathBuf],
-) -> Result<()> {
-    let sandbox_dir = sandbox_dir(codex_home);
-    std::fs::create_dir_all(&sandbox_dir)?;
-    let offline_blob = dpapi_protect(offline_pwd.as_bytes())?;
-    let online_blob = dpapi_protect(online_pwd.as_bytes())?;
-    let users = SandboxUsersFile {
-        version: SETUP_VERSION,
-        offline: SandboxUserRecord {
-            username: offline_user.to_string(),
-            password: BASE64.encode(offline_blob),
-        },
-        online: SandboxUserRecord {
-            username: online_user.to_string(),
-            password: BASE64.encode(online_blob),
-        },
-    };
-    let marker = SetupMarker {
-        version: SETUP_VERSION,
-        offline_username: offline_user.to_string(),
-        online_username: online_user.to_string(),
-        created_at: chrono::Utc::now().to_rfc3339(),
-        read_roots: Vec::new(),
-        write_roots: Vec::new(),
-    };
-    let users_path = sandbox_dir.join("sandbox_users.json");
-    let marker_path = sandbox_dir.join("setup_marker.json");
-    std::fs::write(users_path, serde_json::to_vec_pretty(&users)?)?;
-    std::fs::write(marker_path, serde_json::to_vec_pretty(&marker)?)?;
-    Ok(())
-}
-
 pub fn main() -> Result<()> {
     let ret = real_main();
     if let Err(e) = &ret {
@@ -723,10 +475,8 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
         }
     };
     log_line(log, "read-acl-only mode: applying read ACLs")?;
-    let offline_sid = resolve_sid(&payload.offline_username)?;
-    let online_sid = resolve_sid(&payload.online_username)?;
-    let offline_psid = sid_bytes_to_psid(&offline_sid)?;
-    let online_psid = sid_bytes_to_psid(&online_sid)?;
+    let sandbox_group_sid = resolve_sandbox_users_group_sid()?;
+    let sandbox_group_psid = sid_bytes_to_psid(&sandbox_group_sid)?;
     let mut refresh_errors: Vec<String> = Vec::new();
     let users_sid = resolve_sid("Users")?;
     let users_psid = sid_bytes_to_psid(&users_sid)?;
@@ -736,8 +486,7 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
     let everyone_psid = sid_bytes_to_psid(&everyone_sid)?;
     let rx_psids = vec![users_psid, auth_psid, everyone_psid];
     let subjects = ReadAclSubjects {
-        offline_psid,
-        online_psid,
+        sandbox_group_psid,
         rx_psids: &rx_psids,
     };
     apply_read_acls(
@@ -750,11 +499,8 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
         OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE,
     )?;
     unsafe {
-        if !offline_psid.is_null() {
-            LocalFree(offline_psid as HLOCAL);
-        }
-        if !online_psid.is_null() {
-            LocalFree(online_psid as HLOCAL);
+        if !sandbox_group_psid.is_null() {
+            LocalFree(sandbox_group_psid as HLOCAL);
         }
         if !users_psid.is_null() {
             LocalFree(users_psid as HLOCAL);
@@ -781,38 +527,21 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
 
 fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<()> {
     let refresh_only = payload.refresh_only;
-    let offline_pwd = if refresh_only {
-        None
-    } else {
-        Some(random_password())
-    };
-    let online_pwd = if refresh_only {
-        None
-    } else {
-        Some(random_password())
-    };
     if refresh_only {
     } else {
-        log_line(
-            log,
-            &format!(
-                "ensuring sandbox users offline={} online={}",
-                payload.offline_username, payload.online_username
-            ),
-        )?;
-        ensure_local_user(
+        provision_sandbox_users(
+            &payload.codex_home,
             &payload.offline_username,
-            offline_pwd.as_ref().unwrap(),
+            &payload.online_username,
             log,
         )?;
-        ensure_local_user(&payload.online_username, online_pwd.as_ref().unwrap(), log)?;
     }
     let offline_sid = resolve_sid(&payload.offline_username)?;
-    let online_sid = resolve_sid(&payload.online_username)?;
-    let offline_psid = sid_bytes_to_psid(&offline_sid)?;
-    let online_psid = sid_bytes_to_psid(&online_sid)?;
     let offline_sid_str = string_from_sid_bytes(&offline_sid).map_err(anyhow::Error::msg)?;
 
+    let sandbox_group_sid = resolve_sandbox_users_group_sid()?;
+    let sandbox_group_psid = sid_bytes_to_psid(&sandbox_group_sid)?;
+
     let caps = load_or_create_cap_sids(&payload.codex_home)?;
     let cap_psid = unsafe {
         convert_string_sid_to_sid(&caps.workspace)
@@ -844,8 +573,9 @@ fn run_setup_full(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 sandbox_group_sid_str =
+        string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
+    let sid_strings = vec![sandbox_group_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();
@@ -864,11 +594,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
             continue;
         }
         let mut need_grant = false;
-        for (label, psid) in [
-            ("offline", offline_psid),
-            ("online", online_psid),
-            ("cap", cap_psid),
-        ] {
+        for (label, psid) in [("sandbox_group", sandbox_group_psid), ("cap", cap_psid)] {
             let has = match path_mask_allows(root, &[psid], write_mask, true) {
                 Ok(h) => h,
                 Err(e) => {
@@ -896,7 +622,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
             log_line(
                 log,
                 &format!(
-                    "granting write ACE to {} for sandbox users and capability SID",
+                    "granting write ACE to {} for sandbox group and capability SID",
                     root.display()
                 ),
             )?;
@@ -964,25 +690,13 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
         lock_sandbox_dir(
             &sandbox_dir(&payload.codex_home),
             &payload.real_user,
-            &[offline_sid.clone(), online_sid.clone()],
+            &sandbox_group_sid,
             log,
         )?;
-        write_secrets(
-            &payload.codex_home,
-            &payload.offline_username,
-            offline_pwd.as_ref().unwrap(),
-            &payload.online_username,
-            online_pwd.as_ref().unwrap(),
-            &payload.read_roots,
-            &payload.write_roots,
-        )?;
     }
     unsafe {
-        if !offline_psid.is_null() {
-            LocalFree(offline_psid as HLOCAL);
-        }
-        if !online_psid.is_null() {
-            LocalFree(online_psid as HLOCAL);
+        if !sandbox_group_psid.is_null() {
+            LocalFree(sandbox_group_psid as HLOCAL);
         }
         if !cap_psid.is_null() {
             LocalFree(cap_psid as HLOCAL);