Harden and share Windows sandbox identity helpers (#45533)

## What changed

- Share process package-family queries, token user SID extraction, and account-name lookup through `codex_windows_sandbox`, and use them in the provisioning service.
- Bound token query sizes and validate SID pointers, revisions, and lengths before copying SIDs into owned storage.
- Use a drop guard to balance firewall COM initialization, and track the package lifecycle directory guard separately so cleanup releases it while keeping ancestor and home handles pinned.

## Testing

Add tests that verify copied SIDs outlive their query buffers and reject truncated token data, malformed SIDs, and invalid SID pointers.

GitOrigin-RevId: 2b893e4524d3e00cb114df05215beef7b9eebbe9

codex-rs/windows-sandbox-rs/Cargo.toml

@@ -80,6 +80,7 @@ features = [
     "Win32_System_Console",
     "Win32_System_RemoteDesktop",
     "Win32_Storage_FileSystem",
+    "Win32_Storage_Packaging_Appx",
     "Win32_System_Diagnostics_ToolHelp",
     "Win32_NetworkManagement_NetManagement",
     "Win32_NetworkManagement_WindowsFilteringPlatform",

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

@@ -85,6 +85,8 @@ mod logging;
 #[cfg(target_os = "windows")]
 mod no_reparse_dir;
 #[cfg(target_os = "windows")]
+mod package_identity;
+#[cfg(target_os = "windows")]
 mod path_normalization;
 #[cfg(target_os = "windows")]
 mod process;
@@ -303,6 +305,8 @@ pub use no_reparse_dir::open_directory_no_reparse;
 #[cfg(target_os = "windows")]
 pub use no_reparse_dir::validate_local_directory_path;
 #[cfg(target_os = "windows")]
+pub use package_identity::process_package_family;
+#[cfg(target_os = "windows")]
 pub use path_normalization::canonicalize_path;
 #[cfg(target_os = "windows")]
 pub use process::ConsoleMode;
@@ -412,6 +416,8 @@ pub use token::create_workspace_write_token_with_caps_from;
 #[cfg(target_os = "windows")]
 pub use token::get_current_token_for_restriction;
 #[cfg(target_os = "windows")]
+pub use token_user::get_user_sid_bytes;
+#[cfg(target_os = "windows")]
 pub use unified_exec::WindowsSandboxSessionRequest;
 #[cfg(target_os = "windows")]
 pub use unified_exec::spawn_windows_sandbox_session_elevated_for_permission_profile;
@@ -440,6 +446,8 @@ pub use windows_impl::run_windows_sandbox_legacy_preflight;
 #[cfg(target_os = "windows")]
 pub use winutil::SANDBOX_USERS_GROUP;
 #[cfg(target_os = "windows")]
+pub use winutil::account_name_from_sid;
+#[cfg(target_os = "windows")]
 pub use winutil::ensure_sandbox_users_group;
 #[cfg(target_os = "windows")]
 pub use winutil::local_user_flags;

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

@@ -0,0 +1,48 @@
+//! Queries held process package identities without trusting routing hints or environment state.
+
+use anyhow::Context;
+use anyhow::Result;
+use anyhow::bail;
+use std::io;
+use windows_sys::Win32::Foundation as foundation;
+use windows_sys::Win32::Foundation::HANDLE;
+use windows_sys::Win32::Storage::Packaging::Appx::GetPackageFamilyName;
+
+/// Reads a process's OS package family. Absence is not authorization.
+///
+/// # Safety
+/// The caller must keep the process handle valid throughout the query.
+pub unsafe fn process_package_family(process: HANDLE) -> Result<Option<String>> {
+    let mut length = 0;
+    let status = unsafe { GetPackageFamilyName(process, &mut length, std::ptr::null_mut()) };
+    if status == foundation::APPMODEL_ERROR_NO_PACKAGE {
+        return Ok(None);
+    }
+    if status != foundation::ERROR_INSUFFICIENT_BUFFER {
+        return Err(io::Error::from_raw_os_error(status as i32))
+            .context("query the process package family");
+    }
+    if length == 0 || length > 256 {
+        bail!("the process package family has an invalid length");
+    }
+
+    let mut buffer = vec![0_u16; length as usize];
+    let status = unsafe { GetPackageFamilyName(process, &mut length, buffer.as_mut_ptr()) };
+    if status != foundation::ERROR_SUCCESS {
+        return Err(io::Error::from_raw_os_error(status as i32))
+            .context("read the process package family");
+    }
+
+    let value = buffer
+        .get(..length as usize)
+        .context("the package-family API returned an invalid length")?;
+    let Some((&0, value)) = value.split_last() else {
+        bail!("the process package family is not null-terminated");
+    };
+    if value.is_empty() || value.contains(&0) {
+        bail!("the process package family is malformed");
+    }
+    Ok(Some(
+        String::from_utf16(value).context("the package family contains invalid UTF-16")?,
+    ))
+}

codex-rs/windows-sandbox-rs/src/setup_provisioning/firewall.rs

@@ -58,6 +58,28 @@ struct BlockRuleSpec<'a> {
     remote_ports: Option<&'a str>,
 }
 
+// Balance successful COM initialization on every return path.
+struct FirewallComApartment;
+
+impl FirewallComApartment {
+    fn initialize() -> Result<Self> {
+        let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
+        if hr.is_err() {
+            return Err(anyhow::Error::new(SetupFailure::new(
+                SetupErrorCode::HelperFirewallComInitFailed,
+                format!("CoInitializeEx failed: {hr:?}"),
+            )));
+        }
+        Ok(Self)
+    }
+}
+
+impl Drop for FirewallComApartment {
+    fn drop(&mut self) {
+        unsafe { CoUninitialize() };
+    }
+}
+
 pub fn ensure_offline_proxy_allowlist(
     offline_sid: &str,
     proxy_ports: &[u16],
@@ -66,15 +88,9 @@ pub fn ensure_offline_proxy_allowlist(
 ) -> Result<()> {
     let local_user_spec = format!("O:LSD:(A;;CC;;;{offline_sid})");
 
-    let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
-    if hr.is_err() {
-        return Err(anyhow::Error::new(SetupFailure::new(
-            SetupErrorCode::HelperFirewallComInitFailed,
-            format!("CoInitializeEx failed: {hr:?}"),
-        )));
-    }
+    let _apartment = FirewallComApartment::initialize()?;
 
-    let result = unsafe {
+    unsafe {
         (|| -> Result<()> {
             let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER)
                 .map_err(|err| {
@@ -154,26 +170,15 @@ pub fn ensure_offline_proxy_allowlist(
             }
             Ok(())
         })()
-    };
-
-    unsafe {
-        CoUninitialize();
     }
-    result
 }
 
 pub fn ensure_offline_network_blocks(offline_sid: &str, log: &mut dyn Write) -> Result<()> {
     let local_user_spec = format!("O:LSD:(A;;CC;;;{offline_sid})");
 
-    let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
-    if hr.is_err() {
-        return Err(anyhow::Error::new(SetupFailure::new(
-            SetupErrorCode::HelperFirewallComInitFailed,
-            format!("CoInitializeEx failed: {hr:?}"),
-        )));
-    }
+    let _apartment = FirewallComApartment::initialize()?;
 
-    let result = unsafe {
+    unsafe {
         (|| -> Result<()> {
             let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER)
                 .map_err(|err| {
@@ -221,12 +226,7 @@ pub fn ensure_offline_network_blocks(offline_sid: &str, log: &mut dyn Write) ->
             )?;
             Ok(())
         })()
-    };
-
-    unsafe {
-        CoUninitialize();
     }
-    result
 }
 
 fn remove_rule_if_present(

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

@@ -1,22 +1,27 @@
-//! Owns the token-user SID query used by sandbox token construction.
+//! Copies token user identities with bounded Windows queries and owned SID storage.
 
 use anyhow::Result;
 use anyhow::anyhow;
+use anyhow::ensure;
 use std::ffi::c_void;
 use windows_sys::Win32::Foundation::GetLastError;
 use windows_sys::Win32::Foundation::HANDLE;
-use windows_sys::Win32::Security::CopySid;
-use windows_sys::Win32::Security::GetLengthSid;
 use windows_sys::Win32::Security::GetTokenInformation;
+use windows_sys::Win32::Security::IsValidSid;
 use windows_sys::Win32::Security::TOKEN_USER;
 use windows_sys::Win32::Security::TokenUser;
 
-pub(crate) unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
+/// Copies a token's user SID into owned storage after a bounded TokenUser query.
+///
+/// # Safety
+/// The caller must keep a token handle with TOKEN_QUERY access valid during this call.
+pub unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
     let mut needed: u32 = 0;
     GetTokenInformation(h_token, TokenUser, std::ptr::null_mut(), 0, &mut needed);
-    if needed == 0 {
-        return Err(anyhow!("TokenUser size query returned 0"));
-    }
+    ensure!(
+        needed as usize >= std::mem::size_of::<TOKEN_USER>() && needed <= 4096,
+        "invalid TokenUser query size"
+    );
     let mut user_buf: Vec<u8> = vec![0u8; needed as usize];
     let ok = GetTokenInformation(
         h_token,
@@ -25,30 +30,43 @@ pub(crate) unsafe fn get_user_sid_bytes(h_token: HANDLE) -> Result<Vec<u8>> {
         needed,
         &mut needed,
     );
-    if ok == 0 || (needed as usize) < std::mem::size_of::<TOKEN_USER>() {
+    if ok == 0 {
         return Err(anyhow!(
             "GetTokenInformation(TokenUser) failed: {}",
             GetLastError()
         ));
     }
-    let token_user: TOKEN_USER = std::ptr::read_unaligned(user_buf.as_ptr() as *const TOKEN_USER);
-    let sid_len = GetLengthSid(token_user.User.Sid);
-    if sid_len == 0 {
-        return Err(anyhow!(
-            "GetLengthSid(TokenUser) failed: {}",
-            GetLastError()
-        ));
-    }
-    let mut user_sid_bytes = vec![0u8; sid_len as usize];
-    if CopySid(
-        sid_len,
-        user_sid_bytes.as_mut_ptr() as *mut c_void,
-        token_user.User.Sid,
-    ) == 0
-    {
-        return Err(anyhow!("CopySid(TokenUser) failed: {}", GetLastError()));
-    }
-    Ok(user_sid_bytes)
+    ensure!(
+        needed as usize <= user_buf.len(),
+        "invalid TokenUser result size"
+    );
+    decode_token_user(&user_buf[..needed as usize])
+}
+
+fn decode_token_user(buffer: &[u8]) -> Result<Vec<u8>> {
+    ensure!(
+        buffer.len() >= std::mem::size_of::<TOKEN_USER>(),
+        "truncated TokenUser"
+    );
+    let user = unsafe { std::ptr::read_unaligned(buffer.as_ptr().cast::<TOKEN_USER>()) };
+    copy_token_sid(buffer, user.User.Sid)
+}
+
+fn copy_token_sid(buffer: &[u8], pointer: *mut c_void) -> Result<Vec<u8>> {
+    // Bound the header and every subauthority before calling a SID API.
+    let offset = (pointer as usize).wrapping_sub(buffer.as_ptr() as usize);
+    ensure!(
+        buffer.len() >= 8 && offset <= buffer.len() - 8,
+        "invalid token SID pointer"
+    );
+    ensure!(buffer[offset] == 1, "invalid token SID revision");
+    let length = 8 + usize::from(buffer[offset + 1]) * 4;
+    ensure!(
+        length <= 68 && length <= buffer.len() - offset,
+        "invalid token SID size"
+    );
+    ensure!(unsafe { IsValidSid(pointer) } != 0, "invalid token SID");
+    Ok(buffer[offset..offset + length].to_vec())
 }
 
 #[cfg(test)]

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

@@ -1,15 +1,71 @@
-//! Checks the existing token-user query against real process and invalid handles.
+//! Verifies owned token-user SID results and rejects malformed query buffers.
 
-use super::get_user_sid_bytes;
-use anyhow::Result;
-use anyhow::ensure;
+use super::*;
+use crate::token::world_sid;
+use pretty_assertions::assert_eq;
 use std::os::windows::io::FromRawHandle;
 use std::os::windows::io::OwnedHandle;
-use windows_sys::Win32::Security::IsValidSid;
+use windows_sys::Win32::Security::SID_AND_ATTRIBUTES;
 use windows_sys::Win32::Security::TOKEN_QUERY;
 use windows_sys::Win32::System::Threading::GetCurrentProcess;
 use windows_sys::Win32::System::Threading::OpenProcessToken;
 
+fn user_buffer(sid: &[u8]) -> Vec<u8> {
+    let offset = std::mem::size_of::<TOKEN_USER>();
+    let mut buffer = vec![0; offset + sid.len()];
+    buffer[offset..].copy_from_slice(sid);
+    unsafe {
+        let user = TOKEN_USER {
+            User: SID_AND_ATTRIBUTES {
+                Sid: buffer.as_mut_ptr().add(offset).cast(),
+                Attributes: 0,
+            },
+        };
+        std::ptr::write_unaligned(buffer.as_mut_ptr().cast::<TOKEN_USER>(), user);
+    }
+    buffer
+}
+
+#[test]
+fn user_sid_is_owned_after_the_query_buffer_is_dropped() -> Result<()> {
+    let expected = unsafe { world_sid() }?;
+    let actual = {
+        let buffer = user_buffer(&expected);
+        decode_token_user(&buffer)?
+    };
+    assert_eq!(actual, expected);
+    Ok(())
+}
+
+#[test]
+fn rejects_truncated_user_and_malformed_sid() -> Result<()> {
+    assert!(decode_token_user(&[]).is_err());
+    let mut buffer = user_buffer(&unsafe { world_sid() }?);
+    let offset = std::mem::size_of::<TOKEN_USER>();
+    buffer[offset] = 2;
+    assert!(decode_token_user(&buffer).is_err());
+    buffer[offset] = 1;
+    buffer[offset + 1] = 15;
+    assert!(decode_token_user(&buffer).is_err());
+    for pointer in [std::ptr::null_mut(), unsafe {
+        buffer.as_mut_ptr().add(buffer.len() - 1).cast()
+    }] {
+        unsafe {
+            std::ptr::write_unaligned(
+                buffer.as_mut_ptr().cast::<TOKEN_USER>(),
+                TOKEN_USER {
+                    User: SID_AND_ATTRIBUTES {
+                        Sid: pointer,
+                        Attributes: 0,
+                    },
+                },
+            );
+        }
+        assert!(decode_token_user(&buffer).is_err());
+    }
+    Ok(())
+}
+
 #[test]
 fn queries_current_user_and_rejects_invalid_token() -> Result<()> {
     let mut raw = 0;

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

@@ -35,6 +35,40 @@ pub fn to_wide<S: AsRef<OsStr>>(s: S) -> Vec<u16> {
     v
 }
 
+/// Resolve a named Windows user without consulting environment variables.
+///
+/// # Safety
+/// `sid` must point to a valid SID for the duration of this call.
+pub unsafe fn account_name_from_sid(sid: *mut std::ffi::c_void) -> Result<String> {
+    use windows_sys::Win32::Security as security;
+    let mut name = [0_u16; 256];
+    let mut domain = [0_u16; 256];
+    let mut name_length = name.len() as u32;
+    let mut domain_length = domain.len() as u32;
+    let mut account_type: security::SID_NAME_USE = 0;
+    if unsafe {
+        security::LookupAccountSidW(
+            std::ptr::null(),
+            sid,
+            name.as_mut_ptr(),
+            &mut name_length,
+            domain.as_mut_ptr(),
+            &mut domain_length,
+            &mut account_type,
+        )
+    } == 0
+    {
+        return Err(std::io::Error::last_os_error()).context("resolve Windows account name");
+    }
+    anyhow::ensure!(
+        account_type == security::SidTypeUser && domain_length > 0,
+        "SID is not a named Windows user"
+    );
+    let name = String::from_utf16(&name[..name_length as usize])?;
+    let domain = String::from_utf16(&domain[..domain_length as usize])?;
+    Ok(format!("{domain}\\{name}"))
+}
+
 /// Quote a single Windows command-line argument following the rules used by
 /// CommandLineToArgvW/CRT so that spaces, quotes, and backslashes are preserved.
 /// Reference behavior matches Rust std::process::Command on Windows.

codex-rs/windows-sandbox-service/src/ipc/authentication.rs

@@ -13,7 +13,6 @@ use std::os::windows::ffi::OsStringExt;
 use std::os::windows::io::BorrowedHandle;
 use std::os::windows::io::IntoRawHandle;
 use std::path::PathBuf;
-use std::ptr;
 use windows_sys::Win32::Foundation::HANDLE;
 use windows_sys::Win32::Security as security;
 use windows_sys::Win32::Storage::FileSystem as filesystem;
@@ -44,13 +43,12 @@ pub(super) fn authenticate_client(
     std::thread::scope(|scope| {
         scope
             .spawn(|| {
-                if unsafe { pipes::ImpersonateNamedPipeClient(pipe) } == 0 {
-                    return Err(std::io::Error::last_os_error())
-                        .context("impersonate provisioning client");
-                }
-
-                let identity =
-                    authenticate_impersonated_client(authorized_process, sandbox_sid, request)?;
+                let identity = authenticate_impersonated_client(
+                    pipe,
+                    authorized_process,
+                    sandbox_sid,
+                    request,
+                )?;
                 let policy_result = match request {
                     // Registration does not provision resources or change sandbox policy.
                     ServiceRequest::RegisterInstallation { .. } => Ok(()),
@@ -71,10 +69,14 @@ pub(super) fn authenticate_client(
 }
 
 fn authenticate_impersonated_client(
+    pipe: HANDLE,
     authorized_process: &crate::package_identity::AuthorizedClientProcess,
     sandbox_sid: &[u8],
     request: &ServiceRequest,
 ) -> Result<ClientIdentity> {
+    if unsafe { pipes::ImpersonateNamedPipeClient(pipe) } == 0 {
+        return Err(std::io::Error::last_os_error()).context("impersonate provisioning client");
+    }
     let mut raw_token = 0;
     if unsafe {
         threading::OpenThreadToken(
@@ -123,44 +125,11 @@ fn authenticate_impersonated_client(
         bail!("sandbox accounts cannot request provisioning");
     }
 
-    let mut length = 0;
-    unsafe {
-        security::GetTokenInformation(
-            token.0,
-            security::TokenUser,
-            ptr::null_mut(),
-            0,
-            &mut length,
-        )
-    };
-    if length < size_of::<security::TOKEN_USER>() as u32 {
-        return Err(std::io::Error::last_os_error()).context("size provisioning client identity");
-    }
-    let mut user = vec![0_u8; length as usize];
-    if unsafe {
-        security::GetTokenInformation(
-            token.0,
-            security::TokenUser,
-            user.as_mut_ptr().cast(),
-            length,
-            &mut length,
-        )
-    } == 0
-    {
-        return Err(std::io::Error::last_os_error()).context("read provisioning client identity");
-    }
-    let sid = unsafe { ptr::read_unaligned(user.as_ptr().cast::<security::TOKEN_USER>()) }
-        .User
-        .Sid;
-    let sid_length = unsafe { security::GetLengthSid(sid) };
-    if sid_length == 0 {
-        return Err(std::io::Error::last_os_error()).context("size provisioning client SID");
-    }
-    let user_sid = string_from_sid_bytes(unsafe {
-        std::slice::from_raw_parts(sid.cast::<u8>(), sid_length as usize)
-    })
-    .map_err(anyhow::Error::msg)?;
-    let account = account_name(sid)?;
+    let user = unsafe { codex_windows_sandbox::get_user_sid_bytes(token.0) }
+        .context("read provisioning client identity")?;
+    let user_sid = string_from_sid_bytes(&user).map_err(anyhow::Error::msg)?;
+    let account = unsafe { codex_windows_sandbox::account_name_from_sid(user.as_ptr() as _) }
+        .context("resolve provisioning client account")?;
     let requested_home = match request {
         ServiceRequest::RegisterInstallation { codex_home } => codex_home,
         ServiceRequest::ProvisionSandbox(request) => &request.codex_home,
@@ -231,31 +200,3 @@ fn authenticate_impersonated_client(
         directory_handles: handles,
     })
 }
-
-fn account_name(sid: *mut c_void) -> Result<String> {
-    let mut name = [0_u16; 256];
-    let mut domain = [0_u16; 256];
-    let mut name_length = name.len() as u32;
-    let mut domain_length = domain.len() as u32;
-    let mut account_type: security::SID_NAME_USE = 0;
-    if unsafe {
-        security::LookupAccountSidW(
-            std::ptr::null(),
-            sid,
-            name.as_mut_ptr(),
-            &mut name_length,
-            domain.as_mut_ptr(),
-            &mut domain_length,
-            &mut account_type,
-        )
-    } == 0
-    {
-        return Err(std::io::Error::last_os_error()).context("resolve provisioning client name");
-    }
-    if account_type != security::SidTypeUser || domain_length == 0 {
-        bail!("provisioning client is not a named Windows user");
-    }
-    let name = String::from_utf16(&name[..name_length as usize])?;
-    let domain = String::from_utf16(&domain[..domain_length as usize])?;
-    Ok(format!("{domain}\\{name}"))
-}

codex-rs/windows-sandbox-service/src/package_identity.rs

@@ -1,8 +1,6 @@
 //! Binds provisioning requests to the packaged Codex client and its Windows user.
 
 use std::io;
-use std::mem::size_of;
-use std::ptr;
 #[cfg(debug_assertions)]
 use std::sync::atomic::AtomicBool;
 #[cfg(debug_assertions)]
@@ -14,13 +12,9 @@ use anyhow::bail;
 use windows_sys::Win32::Foundation as foundation;
 use windows_sys::Win32::Foundation::HANDLE;
 use windows_sys::Win32::Security as security;
-use windows_sys::Win32::Storage::Packaging::Appx;
 use windows_sys::Win32::System::Pipes;
 use windows_sys::Win32::System::Threading;
 
-const MAX_PACKAGE_FAMILY_LENGTH: usize = 256;
-const MAX_TOKEN_USER_BYTES: usize = 4096;
-
 #[cfg(debug_assertions)]
 static FOREGROUND_MODE: AtomicBool = AtomicBool::new(false);
 
@@ -58,15 +52,11 @@ pub(crate) fn authorize_client_process(pipe: HANDLE) -> Result<AuthorizedClientP
     }
     let process = OwnedHandle(process);
 
-    let client_family = package_family_name(
-        |length, buffer| unsafe { Appx::GetPackageFamilyName(process.0, length, buffer) },
-        "provisioning client",
-    )?;
-
-    let service_family = package_family_name(
-        |length, buffer| unsafe { Appx::GetCurrentPackageFamilyName(length, buffer) },
-        "provisioning service",
-    )?;
+    let client_family = unsafe { codex_windows_sandbox::process_package_family(process.0) }
+        .context("read provisioning client package identity")?;
+    let service_family =
+        unsafe { codex_windows_sandbox::process_package_family(Threading::GetCurrentProcess()) }
+            .context("read provisioning service package identity")?;
     match client_family {
         Some(client_family) => match service_family.as_deref() {
             Some(service_family) if client_family != service_family => {
@@ -104,95 +94,17 @@ pub(crate) fn authorize_client(
             .context("open the provisioning client process token");
     }
     let process_token = OwnedHandle(process_token);
-    let process_user = token_user(process_token.0).context("read the client process user")?;
-    let impersonated_user =
-        token_user(client_token).context("read the impersonated client user")?;
-    let process_sid = unsafe {
-        ptr::read_unaligned(process_user.as_ptr().cast::<security::TOKEN_USER>())
-            .User
-            .Sid
-    };
-    let impersonated_sid = unsafe {
-        ptr::read_unaligned(impersonated_user.as_ptr().cast::<security::TOKEN_USER>())
-            .User
-            .Sid
-    };
-    if process_sid.is_null()
-        || impersonated_sid.is_null()
-        || unsafe { security::EqualSid(process_sid, impersonated_sid) } == 0
-    {
+    let process_user = unsafe { codex_windows_sandbox::get_user_sid_bytes(process_token.0) }
+        .context("read the client process user")?;
+    let impersonated_user = unsafe { codex_windows_sandbox::get_user_sid_bytes(client_token) }
+        .context("read the impersonated client user")?;
+    if process_user != impersonated_user {
         bail!("provisioning client process does not belong to the impersonated user");
     }
 
     Ok(())
 }
 
-fn package_family_name(
-    mut query: impl FnMut(*mut u32, *mut u16) -> u32,
-    subject: &str,
-) -> Result<Option<String>> {
-    let mut length = 0;
-    let status = query(&mut length, ptr::null_mut());
-    if status == foundation::APPMODEL_ERROR_NO_PACKAGE {
-        return Ok(None);
-    }
-    if status != foundation::ERROR_INSUFFICIENT_BUFFER {
-        return Err(io::Error::from_raw_os_error(status as i32))
-            .with_context(|| format!("query the {subject} package family"));
-    }
-    if length == 0 || length as usize > MAX_PACKAGE_FAMILY_LENGTH {
-        bail!("the {subject} package family has an invalid length");
-    }
-
-    let mut buffer = vec![0_u16; length as usize];
-    let status = query(&mut length, buffer.as_mut_ptr());
-    if status != foundation::ERROR_SUCCESS {
-        return Err(io::Error::from_raw_os_error(status as i32))
-            .with_context(|| format!("read the {subject} package family"));
-    }
-
-    let value = buffer
-        .get(..length as usize)
-        .context("the package-family API returned an invalid length")?;
-    let Some((&0, value)) = value.split_last() else {
-        bail!("the {subject} package family is not null-terminated");
-    };
-    if value.is_empty() || value.contains(&0) {
-        bail!("the {subject} package family is malformed");
-    }
-    Ok(Some(
-        String::from_utf16(value).context("the package family contains invalid UTF-16")?,
-    ))
-}
-
-pub(crate) fn token_user(token: HANDLE) -> Result<Vec<u8>> {
-    let mut length = 0;
-    unsafe {
-        security::GetTokenInformation(token, security::TokenUser, ptr::null_mut(), 0, &mut length)
-    };
-    if length < size_of::<security::TOKEN_USER>() as u32 || length as usize > MAX_TOKEN_USER_BYTES {
-        bail!("the Windows token returned an invalid user identity length");
-    }
-
-    let mut buffer = vec![0_u8; length as usize];
-    if unsafe {
-        security::GetTokenInformation(
-            token,
-            security::TokenUser,
-            buffer.as_mut_ptr().cast(),
-            length,
-            &mut length,
-        )
-    } == 0
-    {
-        return Err(io::Error::last_os_error()).context("read the Windows token user");
-    }
-    if length < size_of::<security::TOKEN_USER>() as u32 || length as usize > buffer.len() {
-        bail!("the Windows token returned a malformed user identity");
-    }
-    Ok(buffer)
-}
-
 #[cfg(debug_assertions)]
 fn is_known_codex_package_family(package_family: &str) -> bool {
     matches!(

codex-rs/windows-sandbox-service/src/package_lifecycle.rs

@@ -39,8 +39,9 @@ mod cleanup;
 
 struct UserInstallation {
     codex_home: Option<PathBuf>,
-    // Ancestors, home, then a guard that prevents in-place junction conversion.
+    // Ancestors and home remain pinned until owner-scoped cleanup finishes.
     directory_handles: Vec<OwnedHandle>,
+    directory_guard: Option<OwnedHandle>,
     record: InstallationRecord,
     user_token: OwnedHandle,
     catalog: PackageCatalog,
@@ -98,6 +99,7 @@ impl PackageLifecycle {
         let saved_record = record.clone();
         with_owner_impersonation(user_token.0, || {
             let mut directory_handles = Vec::new();
+            let mut directory_guard = None;
             let codex_home = match crate::ipc::pin_existing_ancestors(
                 &record.codex_home,
                 &mut directory_handles,
@@ -114,7 +116,7 @@ impl PackageLifecycle {
                     filesystem::FILE_READ_ATTRIBUTES,
                     DirectoryOpenDisposition::OpenExisting,
                 )?);
-                directory_handles.push(OwnedHandle(guard.into_raw_handle() as HANDLE));
+                directory_guard = Some(OwnedHandle(guard.into_raw_handle() as HANDLE));
                 Ok(())
             }) {
                 Ok(()) => Some(record.codex_home.clone()),
@@ -132,6 +134,7 @@ impl PackageLifecycle {
             if let Some(installation) = active.as_mut() {
                 installation.codex_home = codex_home;
                 installation.directory_handles = directory_handles;
+                installation.directory_guard = directory_guard;
                 installation.record = record;
                 return Ok(());
             }
@@ -158,6 +161,7 @@ impl PackageLifecycle {
             active.replace(UserInstallation {
                 codex_home,
                 directory_handles,
+                directory_guard,
                 record,
                 user_token,
                 catalog,
@@ -209,18 +213,8 @@ impl PackageLifecycle {
             return Err(io::Error::last_os_error()).context("open the logged-in user's token");
         }
         let token = crate::ipc::OwnedHandle(raw_token);
-        let user = crate::package_identity::token_user(token.0)?;
-        let sid = unsafe { std::ptr::read_unaligned(user.as_ptr().cast::<security::TOKEN_USER>()) }
-            .User
-            .Sid;
-        let sid_length = unsafe { security::GetLengthSid(sid) };
-        if sid_length == 0 {
-            return Err(io::Error::last_os_error()).context("read the logged-in user's SID");
-        }
-        let user_sid = string_from_sid_bytes(unsafe {
-            std::slice::from_raw_parts(sid.cast::<u8>(), sid_length as usize)
-        })
-        .map_err(anyhow::Error::msg)?;
+        let user = unsafe { codex_windows_sandbox::get_user_sid_bytes(token.0) }?;
+        let user_sid = string_from_sid_bytes(&user).map_err(anyhow::Error::msg)?;
         ensure!(
             user_sid == record.user_sid,
             "logged-in user does not match the recorded sandbox owner"
@@ -245,10 +239,10 @@ impl PackageLifecycle {
     }
 }
 
-fn with_owner_impersonation(
+pub(crate) fn with_owner_impersonation<T>(
     user_token: HANDLE,
-    operation: impl FnOnce() -> Result<()>,
-) -> Result<()> {
+    operation: impl FnOnce() -> Result<T>,
+) -> Result<T> {
     if unsafe { security::ImpersonateLoggedOnUser(user_token) } == 0 {
         return Err(io::Error::last_os_error()).context("impersonate the sandbox owner");
     }

codex-rs/windows-sandbox-service/src/package_lifecycle/cleanup.rs

@@ -23,7 +23,7 @@ pub(super) fn clean_up(installation: &mut UserInstallation) -> Result<()> {
     let codex_home = installation.codex_home.clone();
     clean_up_packaged_windows_sandbox(codex_home.as_deref(), || {
         // Privileged file cleanup is finished; release the guard before owner-scoped removal.
-        installation.directory_handles.pop();
+        installation.directory_guard.take();
         let Some(desktop) = &installation.record.desktop_installation else {
             return Ok(());
         };