openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-15 12:08:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Register Windows desktop uninstall ownership before sandbox setup (#45224)

Browse Source 
## Why

Desktop uninstall cleanup needs an installation owner even when the user has not signed in or configured the Windows sandbox. Recording ownership only during provisioning leaves those installations unregistered.

## What changed

- Add an authenticated installation registration request and attempt it during Windows desktop stdio initialization, with a five-second timeout before the initialization response.
- Persist ownership independently of provisioning, preserve existing desktop ownership, and prevent another user or home from replacing the registered owner.
- Validate write authority and retain directory handles and guards to protect the registered home against junction conversion through privileged cleanup.
- Preserve existing CLI homes during desktop uninstall while removing their `CodexSandboxUsers` ACL entries. Report ACL revocation errors and avoid propagating unchanged ACLs.
- Grant the owner `WRITE_DAC` on `.sandbox-bin` and allow elevated-helper fallback when older permissions need repair or the service cannot establish an uninstall watcher.

## Testing

Add tests for installation registration without sandbox settings and for preserving a child's null DACL when revoking an absent SID from its parent.

GitOrigin-RevId: fb48923e9d76758d1bf5b50c7305aa60f91629db
This commit is contained in:
chess
2026-09-13 12:57:42 +00:00
committed by copyberry
parent 1715e55076
commit 36f0dbe796
20 changed files with 446 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/Cargo.lock generated
View File

@@ -5220,6 +5220,7 @@ dependencies = [
"codex-config",
"codex-core",
"codex-windows-sandbox",
"pretty_assertions",
"tokio",
"toml 0.9.11+spec-1.1.0",
"windows 0.58.0",

16
codex-rs/app-server/src/request_processors/initialize_processor.rs
View File

@@ -174,6 +174,22 @@ impl InitializeRequestProcessor {
*suffix = Some(user_agent_suffix);
}
#[cfg(windows)]
if matches!(session.origin, ConnectionOrigin::Stdio) && name == "Codex Desktop" {
// Uninstall ownership must not depend on account sign-in or sandbox setup.
// Keep this bounded attempt ahead of the response; background registration can race uninstall.
let home = codex_home.clone();
if !matches!(
tokio::task::spawn_blocking(move || {
codex_windows_sandbox::register_desktop_installation(&home)
})
.await,
Ok(Ok(()))
) {
tracing::warn!("could not register desktop uninstall ownership");
}
}
let user_agent = get_codex_user_agent();
let response = InitializeResponse {
user_agent,

37
codex-rs/windows-sandbox-rs/src/acl.rs
View File

@@ -899,7 +899,11 @@ pub unsafe fn add_deny_read_ace(path: &Path, psid: *mut c_void) -> Result<bool>
#[path = "acl_tests.rs"]
mod tests;
pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) {
/// Removes explicit ACEs for one SID and propagates the updated inherited ACL.
///
/// # Safety
/// Caller must pass a valid SID pointer and have authority to edit the target DACL.
pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) -> Result<()> {
let mut p_sd: *mut c_void = std::ptr::null_mut();
let mut p_dacl: *mut ACL = std::ptr::null_mut();
let code = GetNamedSecurityInfoW(
@@ -916,7 +920,14 @@ pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) {
if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL);
}
return;
return acl_api_result(path, "GetNamedSecurityInfoW", code);
}
if p_dacl.is_null() {
// A null DACL has no ACE to revoke; replacing it with an empty ACL would deny access.
if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL);
}
return Ok(());
}
let trustee = TRUSTEE_W {
pMultipleTrustee: std::ptr::null_mut(),
@@ -931,9 +942,17 @@ pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) {
explicit.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;
explicit.Trustee = trustee;
let mut p_new_dacl: *mut ACL = std::ptr::null_mut();
let code2 = SetEntriesInAclW(1, &explicit, p_dacl, &mut p_new_dacl);
if code2 == ERROR_SUCCESS {
let _ = SetNamedSecurityInfoW(
let result = acl_api_result(
path,
"SetEntriesInAclW",
SetEntriesInAclW(1, &explicit, p_dacl, &mut p_new_dacl),
)
.and_then(|()| {
// REVOKE_ACCESS only removes ACEs. An unchanged ACL must not propagate inheritance.
if (*p_new_dacl).AceCount == (*p_dacl).AceCount {
return Ok(());
}
let code = SetNamedSecurityInfoW(
to_wide(path).as_ptr() as *mut u16,
1,
DACL_SECURITY_INFORMATION,
@@ -942,13 +961,15 @@ pub unsafe fn revoke_ace(path: &Path, psid: *mut c_void) {
p_new_dacl,
std::ptr::null_mut(),
);
if !p_new_dacl.is_null() {
LocalFree(p_new_dacl as HLOCAL);
}
acl_api_result(path, "SetNamedSecurityInfoW", code)
});
if !p_new_dacl.is_null() {
LocalFree(p_new_dacl as HLOCAL);
}
if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL);
}
result
}
/// Grants RX to the null device for the given SID to support stdout/stderr redirection.

57
codex-rs/windows-sandbox-rs/src/acl_tests.rs
View File

@@ -6,6 +6,15 @@ use pretty_assertions::assert_eq;
use std::fs::OpenOptions;
use std::os::windows::fs::OpenOptionsExt;
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW;
use windows_sys::Win32::Security::Authorization::SDDL_REVISION_1;
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
use windows_sys::Win32::Security::GetSecurityDescriptorControl;
use windows_sys::Win32::Security::SE_DACL_PROTECTED;
use windows_sys::Win32::Security::SetFileSecurityW;
use windows_sys::Win32::Security::UNPROTECTED_DACL_SECURITY_INFORMATION;
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
use windows_sys::Win32::Storage::FileSystem::READ_CONTROL;
@@ -53,3 +62,51 @@ fn existing_deny_ace_is_visible_without_write_dac() {
.expect("read existing deny ACE");
assert!(already_present);
}
#[test]
fn revoking_absent_sid_preserves_child_null_dacl() {
let parent = tempfile::tempdir().expect("parent directory");
let child = parent.path().join("child");
std::fs::create_dir(&child).expect("child directory");
let sid = LocalSid::from_string("S-1-5-21-10-20-30-40").expect("absent SID");
let other_sid = LocalSid::from_string("S-1-5-21-10-20-30-41").expect("inherited SID");
unsafe {
super::add_allow_ace(parent.path(), other_sid.as_ptr()).expect("inheritable parent ACE");
let mut descriptor = std::ptr::null_mut();
assert_ne!(
ConvertStringSecurityDescriptorToSecurityDescriptorW(
crate::winutil::to_wide("D:NO_ACCESS_CONTROL").as_ptr(),
SDDL_REVISION_1,
&mut descriptor,
std::ptr::null_mut(),
),
0,
);
// The legacy setter preserves a null DACL without applying automatic inheritance.
let set = SetFileSecurityW(
crate::winutil::to_wide(&child).as_ptr(),
DACL_SECURITY_INFORMATION | UNPROTECTED_DACL_SECURITY_INFORMATION,
descriptor,
);
LocalFree(descriptor as HLOCAL);
assert_ne!(set, 0, "set an unprotected null DACL");
for revoke in [false, true] {
if revoke {
super::revoke_ace(parent.path(), sid.as_ptr()).expect("revoke absent SID");
}
let (dacl, descriptor) = super::fetch_dacl_handle(&child).expect("child permissions");
let mut control = 0;
let mut revision = 0;
let valid = GetSecurityDescriptorControl(descriptor, &mut control, &mut revision);
LocalFree(descriptor as HLOCAL);
assert_ne!(valid, 0, "read child inheritance flags");
assert_eq!(control & SE_DACL_PROTECTED, 0, "child permits inheritance");
assert!(
dacl.is_null(),
"revocation must preserve the child's null DACL"
);
}
}
}

2
codex-rs/windows-sandbox-rs/src/deny_read_acl.rs
View File

@@ -81,7 +81,7 @@ pub unsafe fn apply_deny_read_acls(paths: &[PathBuf], psid: *mut c_void) -> Resu
Ok(added) => added,
Err(err) => {
for added_path in &added_in_this_call {
revoke_ace(added_path, psid);
let _ = revoke_ace(added_path, psid);
}
return Err(err);
}

2
codex-rs/windows-sandbox-rs/src/deny_read_state.rs
View File

@@ -51,7 +51,7 @@ pub unsafe fn sync_persistent_deny_read_acls(
for path in previous_paths {
if !desired_keys.contains(&lexical_path_key(&path)) {
revoke_ace(&path, psid);
let _ = revoke_ace(&path, psid);
}
}

4
codex-rs/windows-sandbox-rs/src/lib.rs
View File

@@ -200,6 +200,8 @@ pub use acl::path_or_child_file_has_standard_user_mutation_allow;
#[cfg(target_os = "windows")]
pub use acl::path_write_aces_need_refresh;
#[cfg(target_os = "windows")]
pub use acl::revoke_ace;
#[cfg(target_os = "windows")]
pub use audit::apply_world_writable_scan_and_denies_for_permissions;
#[cfg(target_os = "windows")]
pub use cap::load_or_create_cap_sids;
@@ -319,6 +321,8 @@ pub use provisioning_client::WindowsSandboxProvisioningOutcome;
#[cfg(target_os = "windows")]
pub use provisioning_client::provision_windows_sandbox_via_service;
#[cfg(target_os = "windows")]
pub use provisioning_client::register_desktop_installation;
#[cfg(target_os = "windows")]
pub use provisioning_protocol::FramedProvisioningMessage;
#[cfg(target_os = "windows")]
pub use provisioning_protocol::PROVISIONING_PROTOCOL_VERSION;

53
codex-rs/windows-sandbox-rs/src/provisioning_client.rs
View File

@@ -119,9 +119,44 @@ pub fn provision_windows_sandbox_via_service(
},
};
let deadline = Instant::now() + PROVISIONING_TIMEOUT;
match send_service_request(request, PROVISIONING_TIMEOUT)? {
crate::SandboxProvisioningResponse::Ok => {
Ok(WindowsSandboxProvisioningOutcome::Provisioned)
}
crate::SandboxProvisioningResponse::Unavailable => {
Ok(WindowsSandboxProvisioningOutcome::Unavailable)
}
crate::SandboxProvisioningResponse::Error { message } => Err(anyhow!(message)),
}
}
/// Records desktop uninstall ownership without creating or enabling a sandbox.
pub fn register_desktop_installation(codex_home: &Path) -> anyhow::Result<()> {
let request = crate::FramedProvisioningMessage {
version: crate::PROVISIONING_PROTOCOL_VERSION,
message: crate::ProvisioningMessage::RegisterInstallationRequest {
codex_home: codex_home
.to_str()
.context("desktop home is not valid UTF-8")?
.to_owned(),
},
};
match send_service_request(request, Duration::from_secs(5))? {
crate::SandboxProvisioningResponse::Ok => Ok(()),
crate::SandboxProvisioningResponse::Unavailable => {
bail!("desktop uninstall registration service is unavailable")
}
crate::SandboxProvisioningResponse::Error { message } => Err(anyhow!(message)),
}
}
fn send_service_request(
request: crate::FramedProvisioningMessage,
timeout: Duration,
) -> anyhow::Result<crate::SandboxProvisioningResponse> {
let deadline = Instant::now() + timeout;
let Some(mut pipe) = connect(deadline)? else {
return Ok(WindowsSandboxProvisioningOutcome::Unavailable);
return Ok(crate::SandboxProvisioningResponse::Unavailable);
};
let response = (|| -> anyhow::Result<crate::FramedProvisioningMessage> {
verify_server(pipe.as_raw_handle() as HANDLE)
@@ -156,26 +191,18 @@ pub fn provision_windows_sandbox_via_service(
)
}) =>
{
return Ok(WindowsSandboxProvisioningOutcome::Unavailable);
return Ok(crate::SandboxProvisioningResponse::Unavailable);
}
Err(error) => return Err(error),
};
if response.version != crate::PROVISIONING_PROTOCOL_VERSION {
return Ok(WindowsSandboxProvisioningOutcome::Unavailable);
return Ok(crate::SandboxProvisioningResponse::Unavailable);
}
let crate::ProvisioningMessage::ProvisionSandboxResponse { payload } = response.message else {
bail!("unexpected sandbox provisioning response message");
};
match payload {
crate::SandboxProvisioningResponse::Ok => {
Ok(WindowsSandboxProvisioningOutcome::Provisioned)
}
crate::SandboxProvisioningResponse::Unavailable => {
Ok(WindowsSandboxProvisioningOutcome::Unavailable)
}
crate::SandboxProvisioningResponse::Error { message } => Err(anyhow!(message)),
}
Ok(payload)
}
fn connect(deadline: Instant) -> anyhow::Result<Option<File>> {

4
codex-rs/windows-sandbox-rs/src/provisioning_protocol.rs
View File

@@ -25,6 +25,10 @@ pub struct FramedProvisioningMessage {
#[derive(Debug, Serialize, Deserialize, Clone)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ProvisioningMessage {
/// Records uninstall ownership without provisioning sandbox resources.
RegisterInstallationRequest {
codex_home: String,
},
ProvisionSandboxRequest {
payload: SandboxProvisioningRequest,
},

3
codex-rs/windows-sandbox-rs/src/setup_provisioning.rs
View File

@@ -813,13 +813,14 @@ fn lock_persistent_sandbox_dirs(payload: &Payload, sandbox_group_sid: &[u8]) ->
}
fn lock_sandbox_bin_dir(payload: &Payload, sandbox_group_sid: &[u8]) -> Result<()> {
// The owner's unelevated refresh must be able to reapply this protected DACL.
lock_sandbox_dir(
&sandbox_bin_dir(&payload.codex_home),
&payload.real_user,
sandbox_group_sid,
GRANT_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | WRITE_DAC,
DaclInheritance::Protected,
payload.mode,
)

3
codex-rs/windows-sandbox-service/Cargo.toml
View File

@@ -25,6 +25,9 @@ codex-windows-sandbox = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt", "sync", "time"] }
toml = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
[target.'cfg(windows)'.dependencies.windows]
version = "0.58"
features = [

29
codex-rs/windows-sandbox-service/src/ipc.rs
View File

@@ -1,6 +1,6 @@
//! Authenticated local IPC for the Windows sandbox provisioning service.
//! Configuration parse failures and unsupported home drives defer provisioning to
//! the client's elevated helper.
//! Expected service limitations defer provisioning to the client's elevated helper;
//! authentication and policy rejections remain errors.
//! Shutdown wakeups are retried until the listener connects or stops.
mod authentication;
@@ -21,9 +21,11 @@ use codex_windows_sandbox::string_from_sid_bytes;
use codex_windows_sandbox::to_wide;
use codex_windows_sandbox::write_provisioning_frame;
pub(crate) use home::OwnedHandle;
pub(crate) use home::pin_directory;
pub(crate) use home::pin_existing_ancestors;
#[cfg(test)]
use request::ProvisioningRequest;
pub(crate) use request::ServiceRequest;
use request::validate_request;
use std::mem::size_of;
use std::ptr;
@@ -48,6 +50,18 @@ const MAX_RESPONSE_MESSAGE_BYTES: usize = 512;
const REQUEST_IDLE_TIMEOUT: Duration = Duration::from_secs(5);
const PIPE_USER_ACCESS: &str = "0x0012019b";
/// The service cannot complete this request; the interactive setup helper may still work.
#[derive(Debug)]
pub(crate) struct ServiceUnavailable(pub(crate) &'static str);
impl std::fmt::Display for ServiceUnavailable {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str(self.0)
}
}
impl std::error::Error for ServiceUnavailable {}
struct SecurityDescriptor(security::PSECURITY_DESCRIPTOR);
impl Drop for SecurityDescriptor {
@@ -65,7 +79,7 @@ enum PipeConnection {
pub(crate) fn run(
shutdown: Arc<AtomicBool>,
on_ready: impl FnOnce() -> Result<()>,
on_authenticated_user: impl Fn(&InstallationRecord, OwnedHandle) -> Result<()>,
register_installation: impl Fn(InstallationRecord, OwnedHandle) -> Result<()>,
on_session_change: impl Fn() -> Result<()>,
) -> Result<()> {
let sandbox_sid = ensure_sandbox_users_group()?;
@@ -134,11 +148,12 @@ pub(crate) fn run(
&authorized_process,
&sandbox_sid,
&shutdown,
&on_authenticated_user,
&register_installation,
);
let response = match result {
Ok(response) => response,
Err(error) if error.is::<home::UnsupportedHomeDrive>() => {
Err(error) if error.is::<ServiceUnavailable>() => {
eprintln!("sandbox provisioning service unavailable: {error:#}");
SandboxProvisioningResponse::Unavailable
}
Err(error) => {
@@ -253,7 +268,7 @@ fn handle_request(
authorized_process: &crate::package_identity::AuthorizedClientProcess,
sandbox_sid: &[u8],
shutdown: &AtomicBool,
on_authenticated_user: &dyn Fn(&InstallationRecord, OwnedHandle) -> Result<()>,
register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result<()>,
) -> Result<SandboxProvisioningResponse> {
let deadline = Instant::now() + REQUEST_IDLE_TIMEOUT;
let mut request = [0_u8; MAX_REQUEST_BYTES];
@@ -335,7 +350,7 @@ fn handle_request(
return Err(error)
.context("requested sandbox settings violate administrator-controlled machine policy");
}
crate::provisioning::run(identity, request.settings, on_authenticated_user)
crate::provisioning::run(identity, request, register_installation)
}
fn is_config_parse_error(error: &anyhow::Error) -> bool {

93
codex-rs/windows-sandbox-service/src/ipc/authentication.rs
View File

@@ -3,20 +3,26 @@
use anyhow::Context;
use anyhow::Result;
use anyhow::bail;
use codex_windows_sandbox::DirectoryOpenDisposition;
use codex_windows_sandbox::create_directory_guard;
use codex_windows_sandbox::string_from_sid_bytes;
use std::ffi::OsString;
use std::ffi::c_void;
use std::mem::size_of;
use std::path::Path;
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;
use windows_sys::Win32::System::Pipes as pipes;
use windows_sys::Win32::System::Threading as threading;
use super::home::OwnedHandle;
use super::home::prepare_codex_home;
use super::request::ProvisioningRequest;
use super::request::ServiceRequest;
pub(crate) struct ClientIdentity {
pub(crate) account: String,
@@ -25,7 +31,7 @@ pub(crate) struct ClientIdentity {
pub(crate) session_id: u32,
pub(crate) token: OwnedHandle,
pub(crate) desktop_installation: Option<crate::installation_record::DesktopInstallation>,
// Retained by both the service and helper throughout provisioning.
// Pins and guards remain live through registration and the provisioning helper.
pub(crate) directory_handles: Vec<OwnedHandle>,
}
@@ -33,7 +39,7 @@ pub(super) fn authenticate_client(
pipe: HANDLE,
authorized_process: &crate::package_identity::AuthorizedClientProcess,
sandbox_sid: &[u8],
request: &ProvisioningRequest,
request: &ServiceRequest,
) -> Result<(ClientIdentity, Result<()>)> {
std::thread::scope(|scope| {
scope
@@ -43,17 +49,20 @@ pub(super) fn authenticate_client(
.context("impersonate provisioning client");
}
let identity = authenticate_impersonated_client(
authorized_process,
sandbox_sid,
&request.codex_home,
)?;
let policy_result = crate::machine_policy::validate_provisioning_settings(
&identity.codex_home,
&request.settings,
&request.listeners,
identity.token.0,
);
let identity =
authenticate_impersonated_client(authorized_process, sandbox_sid, request)?;
let policy_result = match request {
// Registration does not provision resources or change sandbox policy.
ServiceRequest::RegisterInstallation { .. } => Ok(()),
ServiceRequest::ProvisionSandbox(request) => {
crate::machine_policy::validate_provisioning_settings(
&identity.codex_home,
&request.settings,
&request.listeners,
identity.token.0,
)
}
};
Ok((identity, policy_result))
})
.join()
@@ -64,7 +73,7 @@ pub(super) fn authenticate_client(
fn authenticate_impersonated_client(
authorized_process: &crate::package_identity::AuthorizedClientProcess,
sandbox_sid: &[u8],
requested_home: &Path,
request: &ServiceRequest,
) -> Result<ClientIdentity> {
let mut raw_token = 0;
if unsafe {
@@ -152,7 +161,57 @@ fn authenticate_impersonated_client(
})
.map_err(anyhow::Error::msg)?;
let account = account_name(sid)?;
let (codex_home, handles) = prepare_codex_home(requested_home)?;
let requested_home = match request {
ServiceRequest::RegisterInstallation { codex_home } => codex_home,
ServiceRequest::ProvisionSandbox(request) => &request.codex_home,
};
let (codex_home, handles) = match request {
ServiceRequest::ProvisionSandbox(_) => prepare_codex_home(requested_home)?,
ServiceRequest::RegisterInstallation { .. } => {
// Registration must not create the sandbox directories or change their ACLs.
codex_windows_sandbox::validate_local_directory_path(requested_home)?;
let mut handles = Vec::new();
super::home::pin_existing_ancestors(requested_home, &mut handles)?;
// Uninstall removes sandbox files as SYSTEM; read access cannot grant that authority.
let home = super::home::pin_directory(
requested_home,
filesystem::FILE_ADD_FILE
| filesystem::FILE_ADD_SUBDIRECTORY
| filesystem::WRITE_DAC,
DirectoryOpenDisposition::OpenExisting,
)?;
// Bind the guard to the authorized directory before resolving its pathname.
let guard = create_directory_guard(unsafe { BorrowedHandle::borrow_raw(home.0 as _) })?;
drop(super::home::pin_directory(
requested_home,
filesystem::FILE_READ_ATTRIBUTES,
DirectoryOpenDisposition::OpenExisting,
)?);
// Preserve the authorized directory's literal name (including trailing dots).
let mut buffer = vec![0_u16; 260];
let path = loop {
let length = unsafe {
filesystem::GetFinalPathNameByHandleW(
home.0,
buffer.as_mut_ptr(),
buffer.len() as u32,
filesystem::FILE_NAME_NORMALIZED | filesystem::VOLUME_NAME_DOS,
)
};
if length == 0 {
return Err(std::io::Error::last_os_error())
.context("resolve authorized desktop home");
}
if (length as usize) < buffer.len() {
break PathBuf::from(OsString::from_wide(&buffer[..length as usize]));
}
buffer.resize(length as usize, /*value*/ 0);
};
handles.push(home);
handles.push(OwnedHandle(guard.into_raw_handle() as HANDLE));
(path, handles)
}
};
let desktop_installation =
crate::installation_record::read_desktop_installation(&codex_home, token.0)
.inspect_err(|_| {

35
codex-rs/windows-sandbox-service/src/ipc/home.rs
View File

@@ -19,20 +19,10 @@ use windows_sys::Win32::Foundation as foundation;
use windows_sys::Win32::Foundation::HANDLE;
use windows_sys::Win32::Storage::FileSystem as filesystem;
use super::ServiceUnavailable;
const DRIVE_FIXED: u32 = 3;
/// The service does not support this drive; the interactive helper may still work.
#[derive(Debug)]
pub(super) struct UnsupportedHomeDrive;
impl std::fmt::Display for UnsupportedHomeDrive {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("Codex home must be located on a fixed local drive")
}
}
impl std::error::Error for UnsupportedHomeDrive {}
pub(crate) struct OwnedHandle(pub(crate) HANDLE);
impl Drop for OwnedHandle {
@@ -53,7 +43,7 @@ pub(super) fn prepare_codex_home(requested: &Path) -> Result<(PathBuf, Vec<Owned
if unsafe { filesystem::GetDriveTypeW(to_wide(requested_root.as_os_str()).as_ptr()) }
!= DRIVE_FIXED
{
return Err(UnsupportedHomeDrive.into());
return Err(ServiceUnavailable("Codex home must be located on a fixed local drive").into());
}
let parent = requested
.parent()
@@ -73,16 +63,17 @@ pub(super) fn prepare_codex_home(requested: &Path) -> Result<(PathBuf, Vec<Owned
.last()
.context("find the root of the requested Codex home")?;
if unsafe { filesystem::GetDriveTypeW(to_wide(root.as_os_str()).as_ptr()) } != DRIVE_FIXED {
return Err(UnsupportedHomeDrive.into());
return Err(ServiceUnavailable("Codex home must be located on a fixed local drive").into());
}
if home != requested {
pin_existing_ancestors(&home, &mut handles)?;
}
let sandbox_bin = home.join(".sandbox-bin");
for (index, directory) in [
home.as_path(),
&home.join(".sandbox"),
&home.join(".sandbox-secrets"),
&home.join(".sandbox-bin"),
&sandbox_bin,
]
.into_iter()
.enumerate()
@@ -94,6 +85,18 @@ pub(super) fn prepare_codex_home(requested: &Path) -> Result<(PathBuf, Vec<Owned
access |= filesystem::WRITE_DAC;
}
let handle = pin_directory(directory, access, DirectoryOpenDisposition::OpenOrCreate)
.map_err(|error| {
// Older elevated installs omitted WRITE_DAC; let the interactive helper repair it.
if directory == sandbox_bin.as_path()
&& error.downcast_ref::<std::io::Error>().is_some_and(|error| {
error.raw_os_error() == Some(foundation::ERROR_ACCESS_DENIED as i32)
})
{
error.context(ServiceUnavailable("sandbox binary directory needs elevated repair"))
} else {
error
}
})
.with_context(|| {
if index == 0 {
format!(
@@ -153,7 +156,7 @@ pub(crate) fn pin_existing_ancestors(path: &Path, handles: &mut Vec<OwnedHandle>
Ok(())
}
fn pin_directory(
pub(crate) fn pin_directory(
path: &Path,
access: u32,
disposition: DirectoryOpenDisposition,

42
codex-rs/windows-sandbox-service/src/ipc/request.rs
View File

@@ -13,13 +13,19 @@ use std::path::PathBuf;
use super::MAX_REQUEST_BYTES;
#[derive(Debug, Eq, PartialEq)]
pub(super) struct ProvisioningRequest {
pub(super) codex_home: PathBuf,
pub(super) listeners: WindowsSandboxProxyListeners,
pub(super) settings: WindowsSandboxProvisioningSettings,
pub(crate) enum ServiceRequest {
RegisterInstallation { codex_home: PathBuf },
ProvisionSandbox(ProvisioningRequest),
}
pub(super) fn validate_request(request: &[u8]) -> Result<ProvisioningRequest> {
#[derive(Debug, Eq, PartialEq)]
pub(crate) struct ProvisioningRequest {
pub(super) codex_home: PathBuf,
pub(super) listeners: WindowsSandboxProxyListeners,
pub(crate) settings: WindowsSandboxProvisioningSettings,
}
pub(super) fn validate_request(request: &[u8]) -> Result<ServiceRequest> {
if request.len() > MAX_REQUEST_BYTES {
bail!("provisioning request exceeds size limit");
}
@@ -36,12 +42,17 @@ pub(super) fn validate_request(request: &[u8]) -> Result<ProvisioningRequest> {
frame.version
);
}
let ProvisioningMessage::ProvisionSandboxRequest { payload: request } = frame.message else {
bail!("expected a sandbox provisioning request");
let request = match frame.message {
ProvisioningMessage::RegisterInstallationRequest { codex_home } => {
validate_home(&codex_home)?;
return Ok(ServiceRequest::RegisterInstallation {
codex_home: PathBuf::from(codex_home),
});
}
ProvisioningMessage::ProvisionSandboxRequest { payload } => payload,
ProvisioningMessage::ProvisionSandboxResponse { .. } => bail!("expected a service request"),
};
if request.codex_home.is_empty() || request.codex_home.contains(['\0', '\r', '\n']) {
bail!("Codex home is empty or contains an invalid control character");
}
validate_home(&request.codex_home)?;
let mut settings = request.settings;
let mut listeners = request.listeners;
for ports in [
@@ -63,9 +74,16 @@ pub(super) fn validate_request(request: &[u8]) -> Result<ProvisioningRequest> {
{
bail!("provisioning listener is absent from the proxy settings");
}
Ok(ProvisioningRequest {
Ok(ServiceRequest::ProvisionSandbox(ProvisioningRequest {
codex_home: PathBuf::from(request.codex_home),
listeners,
settings,
})
}))
}
fn validate_home(home: &str) -> Result<()> {
if home.is_empty() || home.contains(['\0', '\r', '\n']) {
bail!("Codex home is empty or contains an invalid control character");
}
Ok(())
}

39
codex-rs/windows-sandbox-service/src/ipc_tests.rs
View File

@@ -1,6 +1,7 @@
use super::OwnedHandle;
use super::PipeConnection;
use super::ProvisioningRequest;
use super::ServiceRequest;
use super::accept_pipe_connection;
use super::is_config_parse_error;
use super::pin_existing_ancestors;
@@ -17,6 +18,7 @@ use codex_windows_sandbox::WindowsSandboxProxyListeners;
use codex_windows_sandbox::read_provisioning_frame;
use codex_windows_sandbox::to_wide;
use codex_windows_sandbox::write_provisioning_frame;
use pretty_assertions::assert_eq;
use std::path::Path;
use std::path::PathBuf;
use std::ptr;
@@ -113,11 +115,11 @@ fn provisioning_request_preserves_home_spaces_and_unicode() {
});
assert_eq!(
validate_request(&request).unwrap(),
ProvisioningRequest {
ServiceRequest::ProvisionSandbox(ProvisioningRequest {
codex_home: PathBuf::from("D:\\Codex Homes\\Jos\u{00e9}\\.codex"),
listeners: WindowsSandboxProxyListeners::default(),
settings: WindowsSandboxProvisioningSettings::default(),
}
})
);
}
@@ -139,7 +141,7 @@ fn structured_provisioning_request_carries_normalized_proxy_settings() {
});
assert_eq!(
validate_request(&request).unwrap(),
ProvisioningRequest {
ServiceRequest::ProvisionSandbox(ProvisioningRequest {
codex_home: PathBuf::from("D:\\Codex Homes\\Jos\u{00e9}\\.codex"),
listeners: WindowsSandboxProxyListeners {
http_ports: vec![http_port],
@@ -149,7 +151,7 @@ fn structured_provisioning_request_carries_normalized_proxy_settings() {
proxy_ports,
allow_local_binding: true,
},
},
}),
);
}
}
@@ -176,11 +178,11 @@ fn structured_provisioning_request_accepts_independent_and_additional_proxy_port
});
assert_eq!(
validate_request(&request).unwrap(),
ProvisioningRequest {
ServiceRequest::ProvisionSandbox(ProvisioningRequest {
codex_home: PathBuf::from(r"C:\Users\alice\.codex"),
settings,
listeners,
}
})
);
}
}
@@ -194,11 +196,11 @@ fn structured_provisioning_request_accepts_disabled_listeners() {
});
assert_eq!(
validate_request(&request).unwrap(),
ProvisioningRequest {
ServiceRequest::ProvisionSandbox(ProvisioningRequest {
codex_home: PathBuf::from(r"C:\Users\alice\.codex"),
listeners: WindowsSandboxProxyListeners::default(),
settings: WindowsSandboxProvisioningSettings::default(),
}
})
);
}
@@ -496,3 +498,24 @@ fn pipe_descriptor_denies_sandbox_group_before_interactive_users() {
let interactive = descriptor.find("(A;;0x0012019b;;;IU)").unwrap();
assert!(deny < interactive);
}
#[test]
fn installation_registration_requires_no_sandbox_settings() {
let mut frame = Vec::new();
write_provisioning_frame(
&mut frame,
&FramedProvisioningMessage {
version: PROVISIONING_PROTOCOL_VERSION,
message: ProvisioningMessage::RegisterInstallationRequest {
codex_home: r"C:\Users\alice\.codex".to_string(),
},
},
)
.unwrap();
assert_eq!(
validate_request(&frame).unwrap(),
ServiceRequest::RegisterInstallation {
codex_home: PathBuf::from(r"C:\Users\alice\.codex")
},
);
}

90
codex-rs/windows-sandbox-service/src/package_lifecycle.rs
View File

@@ -2,6 +2,8 @@
use std::cell::RefCell;
use std::io;
use std::os::windows::io::BorrowedHandle;
use std::os::windows::io::IntoRawHandle;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::AtomicBool;
@@ -10,6 +12,8 @@ use std::sync::atomic::Ordering;
use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use codex_windows_sandbox::DirectoryOpenDisposition;
use codex_windows_sandbox::create_directory_guard;
use codex_windows_sandbox::string_from_sid_bytes;
use windows::ApplicationModel::Package;
use windows::ApplicationModel::PackageCatalog;
@@ -22,12 +26,12 @@ use windows::Win32::System::WinRT::RoUninitialize;
use windows::core::HSTRING;
use windows_sys::Win32::Foundation::HANDLE;
use windows_sys::Win32::Security as security;
use windows_sys::Win32::Storage::FileSystem as filesystem;
use windows_sys::Win32::System::RemoteDesktop::WTS_CURRENT_SERVER_HANDLE;
use windows_sys::Win32::System::RemoteDesktop::WTSEnumerateSessionsW;
use windows_sys::Win32::System::RemoteDesktop::WTSFreeMemory;
use windows_sys::Win32::System::RemoteDesktop::WTSQueryUserToken;
use crate::installation_record::DesktopInstallation;
use crate::installation_record::InstallationRecord;
use crate::ipc::OwnedHandle;
@@ -35,8 +39,9 @@ mod cleanup;
struct UserInstallation {
codex_home: Option<PathBuf>,
// Ancestors, home, then a guard that prevents in-place junction conversion.
directory_handles: Vec<OwnedHandle>,
desktop_installation: Option<DesktopInstallation>,
record: InstallationRecord,
user_token: OwnedHandle,
catalog: PackageCatalog,
token: EventRegistrationToken,
@@ -59,22 +64,59 @@ impl PackageLifecycle {
})
}
pub(crate) fn watch_authenticated_user(
pub(crate) fn register_authenticated_user(
&self,
installation: &InstallationRecord,
mut record: InstallationRecord,
user_token: OwnedHandle,
) -> Result<()> {
if self.installation.borrow().is_some() {
let mut active = self.installation.borrow_mut();
let previous = match active.as_ref() {
Some(installation) => Some(installation.record.clone()),
None => crate::installation_record::load()?,
};
if let Some(previous) = previous {
// A missing watcher does not retire its owner; other clients can use elevated setup.
ensure!(
previous.user_sid == record.user_sid && previous.codex_home == record.codex_home,
crate::ipc::ServiceUnavailable(
"installation is already registered to a different owner or home"
)
);
record.desktop_installation = previous
.desktop_installation
.or(record.desktop_installation);
}
crate::installation_record::save(&record)?;
if let Some(installation) = active.as_mut()
&& installation.codex_home.is_some()
{
// A restored watcher must immediately use newly registered desktop ownership.
installation.record = record;
return Ok(());
}
with_owner_impersonation(user_token.0, || {
let mut directory_handles = Vec::new();
let codex_home = match crate::ipc::pin_existing_ancestors(
&installation.codex_home,
&record.codex_home,
&mut directory_handles,
) {
Ok(()) => Some(installation.codex_home.clone()),
)
.and_then(|()| {
let home = directory_handles
.last()
.context("pin the registered home")?;
let guard =
create_directory_guard(unsafe { BorrowedHandle::borrow_raw(home.0 as _) })?;
// Reject a conversion that happened before the handle-relative guard was created.
drop(crate::ipc::pin_directory(
&record.codex_home,
filesystem::FILE_READ_ATTRIBUTES,
DirectoryOpenDisposition::OpenExisting,
)?);
directory_handles.push(OwnedHandle(guard.into_raw_handle() as HANDLE));
Ok(())
}) {
Ok(()) => Some(record.codex_home.clone()),
Err(error) => {
directory_handles.clear();
crate::service::log_error(
@@ -86,8 +128,15 @@ impl PackageLifecycle {
None
}
};
let catalog = PackageCatalog::OpenForCurrentUser()
.context("open the authenticated user's package catalog")?;
if let Some(installation) = active.as_mut() {
installation.codex_home = codex_home;
installation.directory_handles = directory_handles;
installation.record = record;
return Ok(());
}
let catalog = PackageCatalog::OpenForCurrentUser().context(
crate::ipc::ServiceUnavailable("open the authenticated user's package catalog"),
)?;
let package_name = self.package_name.clone();
let uninstalling = Arc::clone(&self.uninstalling);
let token = catalog
@@ -102,15 +151,17 @@ impl PackageLifecycle {
}
Ok(())
}))
.context("subscribe to authenticated package uninstall notifications")?;
self.installation.replace(Some(UserInstallation {
.context(crate::ipc::ServiceUnavailable(
"subscribe to authenticated package uninstall notifications",
))?;
active.replace(UserInstallation {
codex_home,
directory_handles,
desktop_installation: installation.desktop_installation.clone(),
record,
user_token,
catalog,
token,
}));
});
Ok(())
})
}
@@ -173,14 +224,13 @@ impl PackageLifecycle {
"logged-in user does not match the recorded sandbox owner"
);
self.watch_authenticated_user(&record, token)?;
if session_id != record.session_id {
crate::installation_record::save(&crate::installation_record::InstallationRecord {
self.register_authenticated_user(
InstallationRecord {
session_id,
..record
})?;
}
Ok(())
},
token,
)
}
pub(crate) fn clean_up(&self) -> Result<()> {

27
codex-rs/windows-sandbox-service/src/package_lifecycle/cleanup.rs
View File

@@ -7,6 +7,8 @@ use anyhow::Context;
use anyhow::Result;
use anyhow::ensure;
use codex_windows_sandbox::clean_up_packaged_windows_sandbox;
use codex_windows_sandbox::resolve_sid;
use codex_windows_sandbox::revoke_ace;
use super::UserInstallation;
use super::with_owner_impersonation;
@@ -20,7 +22,9 @@ pub(super) fn clean_up(installation: &mut UserInstallation) -> Result<()> {
crate::installation_record::remove()?;
let codex_home = installation.codex_home.clone();
clean_up_packaged_windows_sandbox(codex_home.as_deref(), || {
let Some(desktop) = &installation.desktop_installation else {
// Privileged file cleanup is finished; release the guard before owner-scoped removal.
installation.directory_handles.pop();
let Some(desktop) = &installation.record.desktop_installation else {
return Ok(());
};
// The marker is user-writable. It must never authorize deletion as LocalSystem.
@@ -33,12 +37,21 @@ pub(super) fn clean_up(installation: &mut UserInstallation) -> Result<()> {
errors.push(error.to_string());
}
};
if let Some(home) = &codex_home
&& desktop.created_codex_home
{
// Release the home itself so it can be deleted; keep its ancestors pinned.
installation.directory_handles.pop();
record_error(std::fs::remove_dir_all(home));
if let Some(home) = &codex_home {
if desktop.created_codex_home {
// Release the home itself so it can be deleted; keep its ancestors pinned.
installation.directory_handles.pop();
record_error(std::fs::remove_dir_all(home));
} else {
// Preserve CLI data without leaving inherited permissions for the deleted group.
record_error(
resolve_sid("CodexSandboxUsers")
.and_then(|mut sid| unsafe {
revoke_ace(home, sid.as_mut_ptr().cast())
})
.map_err(io::Error::other),
);
}
}
// The cache may have been created after provisioning. Pin it only for cleanup.
let mut cache_directory_handles = Vec::new();

37
codex-rs/windows-sandbox-service/src/provisioning.rs
View File

@@ -8,7 +8,6 @@ use anyhow::Context;
use anyhow::Result;
use anyhow::bail;
use codex_windows_sandbox::SandboxProvisioningResponse;
use codex_windows_sandbox::WindowsSandboxProvisioningSettings;
use codex_windows_sandbox::run_elevated_provisioning_setup_with_retained_handles;
use codex_windows_sandbox::sandbox_setup_is_complete_with_settings;
use windows_sys::Win32::Storage::FileSystem as filesystem;
@@ -16,28 +15,31 @@ use windows_sys::Win32::Storage::FileSystem as filesystem;
use crate::installation_record::InstallationRecord;
use crate::ipc::ClientIdentity;
use crate::ipc::OwnedHandle;
use crate::ipc::ServiceRequest;
pub(crate) fn run(
identity: ClientIdentity,
settings: WindowsSandboxProvisioningSettings,
on_authenticated_user: &dyn Fn(&InstallationRecord, OwnedHandle) -> Result<()>,
request: ServiceRequest,
register_installation: &dyn Fn(InstallationRecord, OwnedHandle) -> Result<()>,
) -> Result<SandboxProvisioningResponse> {
// A policy-rejected request must not choose the uninstall owner. Use the
// token already authenticated above instead of impersonating the pipe again.
let previous = crate::installation_record::load()?.filter(|record| {
record.user_sid == identity.user_sid && record.codex_home == identity.codex_home
});
let installation = InstallationRecord {
codex_home: identity.codex_home.clone(),
user_sid: identity.user_sid,
session_id: identity.session_id,
desktop_installation: previous
.and_then(|record| record.desktop_installation)
.or(identity.desktop_installation),
register_installation(
InstallationRecord {
codex_home: identity.codex_home.clone(),
user_sid: identity.user_sid,
session_id: identity.session_id,
desktop_installation: identity.desktop_installation,
},
identity.token,
)?;
let request = match request {
ServiceRequest::RegisterInstallation { .. } => {
return Ok(SandboxProvisioningResponse::Ok);
}
ServiceRequest::ProvisionSandbox(request) => request,
};
on_authenticated_user(&installation, identity.token)?;
if sandbox_setup_is_complete_with_settings(&identity.codex_home, &settings) {
crate::service::record_provisioned_user(&installation)?;
if sandbox_setup_is_complete_with_settings(&identity.codex_home, &request.settings) {
return Ok(SandboxProvisioningResponse::Ok);
}
let helper = std::env::current_exe()
@@ -63,11 +65,10 @@ pub(crate) fn run(
match run_elevated_provisioning_setup_with_retained_handles(
&identity.codex_home,
&identity.account,
settings,
request.settings,
&retained_handles,
) {
Ok(()) => {
crate::service::record_provisioned_user(&installation)?;
crate::service::log_information(
crate::service::EVENT_PROVISIONING_SUCCEEDED,
"Codex sandbox provisioning completed successfully.",

18
codex-rs/windows-sandbox-service/src/service.rs
View File

@@ -175,14 +175,7 @@ fn service_main_inner(state: &ServiceState) -> Result<()> {
Ok(())
},
|installation, user_token| {
if let Err(error) = package_lifecycle.watch_authenticated_user(installation, user_token)
{
log_error(
EVENT_SERVICE_FAILED,
&format!("unable to observe package uninstall: {error:#}"),
);
}
Ok(())
package_lifecycle.register_authenticated_user(installation, user_token)
},
|| {
let session = state.changed_session.swap(u32::MAX, Ordering::AcqRel);
@@ -276,15 +269,6 @@ pub(crate) fn log_information(event_id: u32, message: &str) {
log_event(EVENTLOG_INFORMATION_TYPE, event_id, message);
}
pub(crate) fn record_provisioned_user(
installation: &crate::installation_record::InstallationRecord,
) -> Result<()> {
if SERVICE_STATE.get().is_some() {
crate::installation_record::save(installation)?;
}
Ok(())
}
pub(crate) fn log_error(event_id: u32, message: &str) {
log_event(EVENTLOG_ERROR_TYPE, event_id, message);
}