Harden Windows sandbox provisioning file handling (#42342)

## Why

Windows sandbox provisioning performs privileged work beneath a requested Codex home. Provisioning paths and output entries must remain bound to the files and directories that were validated, without following reparse points or writing through caller-controlled hard links.

## What changed

- Add reusable no-reparse directory opens, path validation, directory guards, and handle-relative atomic file replacement.
- Use fresh or retained handles for provisioning logs, sandbox credentials, setup markers, and error reports in service provisioning mode.
- Add service-side validation for framed provisioning requests, proxy settings, fixed-drive Codex homes, and pinned sandbox directories.

## Testing

Add Windows tests covering junction and unsafe-path rejection, hard-link-safe output replacement, retained output handles, request framing and normalization, and ancestor pinning.

GitOrigin-RevId: 2ec4f0d15404c3387df1414bb0c1915cf452ecf2

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

@@ -18,6 +18,7 @@ workspace = true
 
 [dependencies]
 anyhow = { workspace = true }
+codex-windows-sandbox = { workspace = true }
 
 [target.'cfg(windows)'.dependencies.windows-sys]
 version = "0.52"

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

@@ -1,10 +1,23 @@
 //! Disabled provisioning IPC until authenticated request handling is installed.
 
+// Keep these private until the authenticated transport is connected.
+#[allow(dead_code)]
+mod home;
+#[allow(dead_code)]
+mod request;
+
 use anyhow::Result;
+#[cfg(test)]
+use home::pin_existing_ancestors;
+#[cfg(test)]
+use request::ProvisioningRequest;
+#[cfg(test)]
+use request::validate_request;
 use std::sync::Arc;
 use std::sync::atomic::AtomicBool;
 
 pub(crate) const PIPE_NAME: &str = r"\\.\pipe\OpenAI.CodexSandbox";
+const MAX_REQUEST_BYTES: usize = 4096;
 
 pub(crate) fn run(
     _shutdown: Arc<AtomicBool>,
@@ -14,3 +27,7 @@ pub(crate) fn run(
 }
 
 pub(crate) fn wake() {}
+
+#[cfg(test)]
+#[path = "ipc_tests.rs"]
+mod tests;

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

@@ -0,0 +1,158 @@
+//! Validates and pins local Codex home and sandbox directories, keeping sandbox
+//! leaves nonempty so they cannot become junctions during provisioning.
+
+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::open_directory_no_reparse;
+use codex_windows_sandbox::to_wide;
+use codex_windows_sandbox::validate_local_directory_path;
+use std::os::windows::fs::MetadataExt;
+use std::os::windows::io::BorrowedHandle;
+use std::os::windows::io::IntoRawHandle;
+use std::path::Component;
+use std::path::Path;
+use std::path::PathBuf;
+use windows_sys::Win32::Foundation as foundation;
+use windows_sys::Win32::Foundation::HANDLE;
+use windows_sys::Win32::Storage::FileSystem as filesystem;
+
+const DRIVE_FIXED: u32 = 3;
+
+pub(super) struct OwnedHandle(pub(super) HANDLE);
+
+impl Drop for OwnedHandle {
+    fn drop(&mut self) {
+        if self.0 != 0 && self.0 != foundation::INVALID_HANDLE_VALUE {
+            unsafe { foundation::CloseHandle(self.0) };
+        }
+    }
+}
+
+pub(super) fn prepare_codex_home(requested: &Path) -> Result<(PathBuf, Vec<OwnedHandle>)> {
+    let mut handles = Vec::new();
+    validate_local_directory_path(requested)?;
+    let requested_root = requested
+        .ancestors()
+        .last()
+        .context("find the root of the requested Codex home")?;
+    if unsafe { filesystem::GetDriveTypeW(to_wide(requested_root.as_os_str()).as_ptr()) }
+        != DRIVE_FIXED
+    {
+        bail!("Codex home must be located on a fixed local drive");
+    }
+    let parent = requested
+        .parent()
+        .context("Codex home must have an existing parent directory")?;
+    pin_existing_ancestors(parent, &mut handles)?;
+    handles.push(pin_directory(
+        requested,
+        filesystem::FILE_READ_ATTRIBUTES,
+        DirectoryOpenDisposition::OpenOrCreate,
+    )?);
+    let home = requested
+        .canonicalize()
+        .with_context(|| format!("canonicalize Codex home {}", requested.display()))?;
+    validate_local_directory_path(&home)?;
+    let root = home
+        .ancestors()
+        .last()
+        .context("find the root of the requested Codex home")?;
+    if unsafe { filesystem::GetDriveTypeW(to_wide(root.as_os_str()).as_ptr()) } != DRIVE_FIXED {
+        bail!("Codex home must be located on a fixed local drive");
+    }
+    if home != requested {
+        pin_existing_ancestors(&home, &mut handles)?;
+    }
+    for (index, directory) in [
+        home.as_path(),
+        &home.join(".sandbox"),
+        &home.join(".sandbox-secrets"),
+        &home.join(".sandbox-bin"),
+    ]
+    .into_iter()
+    .enumerate()
+    {
+        let mut access = filesystem::FILE_READ_ATTRIBUTES
+            | filesystem::FILE_ADD_FILE
+            | filesystem::FILE_ADD_SUBDIRECTORY;
+        if index != 0 {
+            access |= filesystem::WRITE_DAC;
+        }
+        let handle = pin_directory(directory, access, DirectoryOpenDisposition::OpenOrCreate)
+            .with_context(|| {
+            if index == 0 {
+                format!(
+                    "requesting user must be permitted to write Codex home {}",
+                    directory.display()
+                )
+            } else {
+                format!(
+                    "requesting user must be permitted to write and modify sandbox directory ACL {}",
+                    directory.display()
+                )
+            }
+        })?;
+        if index != 0 {
+            // The handle is owned here and remains live until the guard is installed.
+            let directory_handle = unsafe { BorrowedHandle::borrow_raw(handle.0 as _) };
+            let guard = create_directory_guard(directory_handle)?;
+            // Reject conversion before guard creation; after creation the retained
+            // file prevents this directory from becoming empty and being converted.
+            drop(pin_directory(
+                directory,
+                filesystem::FILE_READ_ATTRIBUTES,
+                DirectoryOpenDisposition::OpenExisting,
+            )?);
+            handles.push(OwnedHandle(guard.into_raw_handle() as HANDLE));
+        }
+        handles.push(handle);
+        if index == 0 {
+            continue;
+        }
+        for entry in std::fs::read_dir(directory)? {
+            let entry = entry?;
+            let metadata = entry.path().symlink_metadata()?;
+            if metadata.file_attributes() & filesystem::FILE_ATTRIBUTE_REPARSE_POINT != 0 {
+                bail!("refusing reparse point {}", entry.path().display());
+            }
+        }
+    }
+
+    // Pinned parent handles prevent directory replacement, but existing helper file
+    // opens remain pathname-based; hardlink/leaf-file races need a follow-up redesign.
+    Ok((home, handles))
+}
+
+pub(super) fn pin_existing_ancestors(path: &Path, handles: &mut Vec<OwnedHandle>) -> Result<()> {
+    let mut current = PathBuf::new();
+    for component in path.components() {
+        current.push(component.as_os_str());
+        if !matches!(component, Component::Prefix(_)) && current.is_absolute() {
+            handles.push(pin_directory(
+                &current,
+                filesystem::FILE_READ_ATTRIBUTES,
+                DirectoryOpenDisposition::OpenExisting,
+            )?);
+        }
+    }
+    Ok(())
+}
+
+fn pin_directory(
+    path: &Path,
+    access: u32,
+    disposition: DirectoryOpenDisposition,
+) -> Result<OwnedHandle> {
+    let handle = open_directory_no_reparse(
+        path,
+        // Metadata-only access does not enforce no-delete sharing.
+        access | filesystem::FILE_TRAVERSE,
+        filesystem::FILE_SHARE_READ | filesystem::FILE_SHARE_WRITE,
+        disposition,
+    )
+    .with_context(|| format!("pin {}", path.display()))?;
+    Ok(OwnedHandle(handle.into_raw_handle() as HANDLE))
+}

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

@@ -0,0 +1,71 @@
+//! Validates framed provisioning requests and normalizes proxy settings.
+
+use anyhow::Context;
+use anyhow::Result;
+use anyhow::bail;
+use codex_windows_sandbox::PROVISIONING_PROTOCOL_VERSION;
+use codex_windows_sandbox::ProvisioningMessage;
+use codex_windows_sandbox::WindowsSandboxProvisioningSettings;
+use codex_windows_sandbox::WindowsSandboxProxyListeners;
+use codex_windows_sandbox::read_provisioning_frame;
+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(super) fn validate_request(request: &[u8]) -> Result<ProvisioningRequest> {
+    if request.len() > MAX_REQUEST_BYTES {
+        bail!("provisioning request exceeds size limit");
+    }
+    let mut reader = request;
+    let frame = read_provisioning_frame(&mut reader)
+        .context("invalid framed provisioning request")?
+        .context("provisioning client sent an empty request")?;
+    if !reader.is_empty() {
+        bail!("provisioning requests must contain exactly one IPC frame");
+    }
+    if frame.version != PROVISIONING_PROTOCOL_VERSION {
+        bail!(
+            "unsupported provisioning request version: {}",
+            frame.version
+        );
+    }
+    let ProvisioningMessage::ProvisionSandboxRequest { payload: request } = frame.message else {
+        bail!("expected a sandbox provisioning 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");
+    }
+    let mut settings = request.settings;
+    let mut listeners = request.listeners;
+    for ports in [
+        &mut settings.proxy_ports,
+        &mut listeners.http_ports,
+        &mut listeners.socks_ports,
+    ] {
+        if ports.contains(&0) {
+            bail!("provisioning request includes an invalid proxy port");
+        }
+        ports.sort_unstable();
+        ports.dedup();
+    }
+    if listeners
+        .http_ports
+        .iter()
+        .chain(&listeners.socks_ports)
+        .any(|port| !settings.proxy_ports.contains(port))
+    {
+        bail!("provisioning listener is absent from the proxy settings");
+    }
+    Ok(ProvisioningRequest {
+        codex_home: PathBuf::from(request.codex_home),
+        listeners,
+        settings,
+    })
+}

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

@@ -0,0 +1,225 @@
+use super::ProvisioningRequest;
+use super::pin_existing_ancestors;
+use super::validate_request;
+use codex_windows_sandbox::FramedProvisioningMessage;
+use codex_windows_sandbox::PROVISIONING_PROTOCOL_VERSION;
+use codex_windows_sandbox::ProvisioningMessage;
+use codex_windows_sandbox::SandboxProvisioningRequest;
+use codex_windows_sandbox::SandboxProvisioningResponse;
+use codex_windows_sandbox::WindowsSandboxProvisioningSettings;
+use codex_windows_sandbox::WindowsSandboxProxyListeners;
+use codex_windows_sandbox::write_provisioning_frame;
+use std::path::PathBuf;
+
+fn framed_request(request: SandboxProvisioningRequest) -> Vec<u8> {
+    let mut frame = Vec::new();
+    write_provisioning_frame(
+        &mut frame,
+        &FramedProvisioningMessage {
+            version: PROVISIONING_PROTOCOL_VERSION,
+            message: ProvisioningMessage::ProvisionSandboxRequest { payload: request },
+        },
+    )
+    .unwrap();
+    frame
+}
+
+#[test]
+fn provisioning_request_preserves_home_spaces_and_unicode() {
+    let request = framed_request(SandboxProvisioningRequest {
+        codex_home: "D:\\Codex Homes\\Jos\u{00e9}\\.codex".to_string(),
+        settings: WindowsSandboxProvisioningSettings::default(),
+        listeners: WindowsSandboxProxyListeners::default(),
+    });
+    assert_eq!(
+        validate_request(&request).unwrap(),
+        ProvisioningRequest {
+            codex_home: PathBuf::from("D:\\Codex Homes\\Jos\u{00e9}\\.codex"),
+            listeners: WindowsSandboxProxyListeners::default(),
+            settings: WindowsSandboxProvisioningSettings::default(),
+        }
+    );
+}
+
+#[test]
+fn structured_provisioning_request_carries_normalized_proxy_settings() {
+    for (http_port, socks_port, proxy_ports) in
+        [(8081, 3128, vec![3128, 8081]), (8081, 8081, vec![8081])]
+    {
+        let request = framed_request(SandboxProvisioningRequest {
+            codex_home: "D:\\Codex Homes\\Jos\u{00e9}\\.codex".to_string(),
+            settings: WindowsSandboxProvisioningSettings {
+                proxy_ports: vec![http_port, socks_port, http_port],
+                allow_local_binding: true,
+            },
+            listeners: WindowsSandboxProxyListeners {
+                http_ports: vec![http_port, http_port],
+                socks_ports: vec![socks_port],
+            },
+        });
+        assert_eq!(
+            validate_request(&request).unwrap(),
+            ProvisioningRequest {
+                codex_home: PathBuf::from("D:\\Codex Homes\\Jos\u{00e9}\\.codex"),
+                listeners: WindowsSandboxProxyListeners {
+                    http_ports: vec![http_port],
+                    socks_ports: vec![socks_port],
+                },
+                settings: WindowsSandboxProvisioningSettings {
+                    proxy_ports,
+                    allow_local_binding: true,
+                },
+            },
+        );
+    }
+}
+
+#[test]
+fn structured_provisioning_request_accepts_independent_and_additional_proxy_ports() {
+    for (http_ports, socks_ports, proxy_ports) in [
+        (vec![3128], vec![], vec![3128]),
+        (vec![], vec![1080], vec![1080]),
+        (vec![3128, 8080], vec![1080], vec![1080, 1082, 3128, 8080]),
+    ] {
+        let settings = WindowsSandboxProvisioningSettings {
+            proxy_ports,
+            allow_local_binding: false,
+        };
+        let listeners = WindowsSandboxProxyListeners {
+            http_ports,
+            socks_ports,
+        };
+        let request = framed_request(SandboxProvisioningRequest {
+            codex_home: r"C:\Users\alice\.codex".to_string(),
+            settings: settings.clone(),
+            listeners: listeners.clone(),
+        });
+        assert_eq!(
+            validate_request(&request).unwrap(),
+            ProvisioningRequest {
+                codex_home: PathBuf::from(r"C:\Users\alice\.codex"),
+                settings,
+                listeners,
+            }
+        );
+    }
+}
+
+#[test]
+fn structured_provisioning_request_accepts_disabled_listeners() {
+    let request = framed_request(SandboxProvisioningRequest {
+        codex_home: r"C:\Users\alice\.codex".to_string(),
+        settings: WindowsSandboxProvisioningSettings::default(),
+        listeners: WindowsSandboxProxyListeners::default(),
+    });
+    assert_eq!(
+        validate_request(&request).unwrap(),
+        ProvisioningRequest {
+            codex_home: PathBuf::from(r"C:\Users\alice\.codex"),
+            listeners: WindowsSandboxProxyListeners::default(),
+            settings: WindowsSandboxProvisioningSettings::default(),
+        }
+    );
+}
+
+#[test]
+fn structured_provisioning_request_requires_exact_version_fields_and_framing() {
+    let valid = SandboxProvisioningRequest {
+        codex_home: r"C:\Users\alice\.codex".to_string(),
+        settings: WindowsSandboxProvisioningSettings::default(),
+        listeners: WindowsSandboxProxyListeners::default(),
+    };
+    let mut invalid_version = Vec::new();
+    write_provisioning_frame(
+        &mut invalid_version,
+        &FramedProvisioningMessage {
+            version: PROVISIONING_PROTOCOL_VERSION + 1,
+            message: ProvisioningMessage::ProvisionSandboxRequest {
+                payload: valid.clone(),
+            },
+        },
+    )
+    .unwrap();
+    assert!(validate_request(&invalid_version).is_err());
+
+    let mut unexpected_message = Vec::new();
+    write_provisioning_frame(
+        &mut unexpected_message,
+        &FramedProvisioningMessage {
+            version: PROVISIONING_PROTOCOL_VERSION,
+            message: ProvisioningMessage::ProvisionSandboxResponse {
+                payload: SandboxProvisioningResponse::Ok,
+            },
+        },
+    )
+    .unwrap();
+    assert!(validate_request(&unexpected_message).is_err());
+
+    let valid = framed_request(valid);
+    assert!(validate_request(&valid[..valid.len() - 1]).is_err());
+    let mut duplicate = valid.clone();
+    duplicate.extend_from_slice(&valid);
+    assert!(validate_request(&duplicate).is_err());
+}
+
+#[test]
+fn structured_provisioning_request_rejects_invalid_or_inconsistent_ports() {
+    for (proxy_ports, http_ports, socks_ports) in [
+        (vec![0], vec![], vec![]),
+        (vec![3128], vec![0], vec![]),
+        (vec![3128], vec![3128], vec![0]),
+        (vec![3128], vec![3128], vec![8081]),
+    ] {
+        let request = framed_request(SandboxProvisioningRequest {
+            codex_home: r"C:\Users\alice\.codex".to_string(),
+            settings: WindowsSandboxProvisioningSettings {
+                proxy_ports,
+                allow_local_binding: false,
+            },
+            listeners: WindowsSandboxProxyListeners {
+                http_ports,
+                socks_ports,
+            },
+        });
+        assert!(validate_request(&request).is_err());
+    }
+}
+
+#[test]
+fn provisioning_request_rejects_empty_control_characters_and_invalid_utf8() {
+    for home in ["", "C:\\safe\0evil", "C:\\safe\rmore", "C:\\safe\nmore"] {
+        let request = framed_request(SandboxProvisioningRequest {
+            codex_home: home.to_string(),
+            settings: WindowsSandboxProvisioningSettings::default(),
+            listeners: WindowsSandboxProxyListeners::default(),
+        });
+        assert!(validate_request(&request).is_err());
+    }
+
+    let mut invalid_utf8 = framed_request(SandboxProvisioningRequest {
+        codex_home: r"C:\Users\alice\.codex".to_string(),
+        settings: WindowsSandboxProvisioningSettings::default(),
+        listeners: WindowsSandboxProxyListeners::default(),
+    });
+    invalid_utf8[std::mem::size_of::<u32>()] = 0xff;
+    assert!(validate_request(&invalid_utf8).is_err());
+}
+
+#[test]
+fn pin_existing_ancestors_accepts_drive_and_verbatim_drive_roots() {
+    let executable = std::env::current_exe().unwrap().canonicalize().unwrap();
+    let verbatim_root = executable.ancestors().last().unwrap();
+    let ordinary_root = PathBuf::from(
+        verbatim_root
+            .to_str()
+            .unwrap()
+            .strip_prefix(r"\\?\")
+            .unwrap(),
+    );
+
+    for root in [ordinary_root.as_path(), verbatim_root] {
+        let mut handles = Vec::new();
+        pin_existing_ancestors(root, &mut handles).unwrap();
+        assert_eq!(handles.len(), 1);
+    }
+}