Preserve native errors in Windows sandbox ACL diagnostics (#51896)

## Why

Windows sandbox setup diagnostics previously displayed only the outer error context, hiding native failure causes needed to diagnose ACL failures.

## What changed

Preserve full error chains in ACL refresh summaries, setup logs, and sandbox binary directory lock failure reports. Include the requested access rights in ACL update open errors, and retain recorded ACL failures even when writing their log entries fails.

## Testing

Add tests covering error chains for ACL checks, grants, and denies; preservation of accumulated failures when logging fails; and native directory-open errors surviving a structured setup report round trip on Windows.

GitOrigin-RevId: 0a44a56b13d13b52b92ec673aaa2da7bc6cf0753

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

@@ -560,7 +560,7 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
                 .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE)
                 .custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
                 .open(path)
-                .context("open ACL target for root-only update")?,
+                .context("open ACL target for root-only update (MAXIMUM_ALLOWED)")?,
         )
     } else {
         None
@@ -660,7 +660,7 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
                 .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE)
                 .custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
                 .open(path)
-                .context("open ACL target for update")?,
+                .context("open ACL target for update (READ_CONTROL | WRITE_DAC)")?,
         };
         let code3 = SetSecurityInfo(
             handle.as_raw_handle() as _,
@@ -910,7 +910,8 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res
             if deny_ace_already_present(&read_handle, path, psid, kind)? {
                 return Ok(false);
             }
-            return Err(write_error).context("open deny ACL target for update");
+            return Err(write_error)
+                .context("open deny ACL target for update (READ_CONTROL | WRITE_DAC)");
         }
     };
     if matches!(kind, DenyAceKind::Read) {

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

@@ -9,6 +9,8 @@ mod ssh_config_dependencies;
 pub mod environment_transport;
 #[cfg(any(windows, test))]
 mod launch_environment;
+#[cfg(any(windows, test))]
+mod setup_acl_error;
 
 use std::fmt;
 use std::sync::Arc;

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

@@ -0,0 +1,43 @@
+//! Preserve native ACL failure causes in both refresh summaries and setup logs.
+//! Recording a diagnostic never changes whether a failed ACL operation is fatal.
+
+use anyhow::Error;
+use anyhow::Result;
+use std::path::Path;
+
+pub(crate) enum WriteAclOperation {
+    Check,
+    Grant,
+    Deny,
+}
+
+pub(crate) fn record_acl_failure(
+    errors: &mut Vec<String>,
+    operation: WriteAclOperation,
+    path: &Path,
+    error: &Error,
+    write_log: impl FnOnce(&str) -> Result<()>,
+) -> Result<()> {
+    let detail = format!("{}: {error:#}", path.display());
+    let (summary, log) = match operation {
+        WriteAclOperation::Check => (
+            format!("write ACE check failed on {detail}"),
+            format!("write ACE check failed on {detail}; continuing"),
+        ),
+        WriteAclOperation::Grant => (
+            format!("write ACE failed on {detail}"),
+            format!("write ACE grant failed on {detail}"),
+        ),
+        WriteAclOperation::Deny => (
+            format!("deny ACE failed on {detail}"),
+            format!("deny ACE failed on {detail}"),
+        ),
+    };
+    // Retain the ACL failure even if writing its individual log line fails.
+    errors.push(summary);
+    write_log(&log)
+}
+
+#[cfg(test)]
+#[path = "setup_acl_error_tests.rs"]
+mod tests;

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

@@ -0,0 +1,74 @@
+use super::WriteAclOperation;
+use super::record_acl_failure;
+use anyhow::anyhow;
+use pretty_assertions::assert_eq;
+use std::io;
+use std::path::Path;
+
+#[test]
+fn records_each_acl_operation_with_its_native_error_chain() {
+    let path = Path::new("project/.git");
+    let native_error = io::Error::from_raw_os_error(5);
+    // The native wording is platform-dependent; the code and cause chain are not.
+    let native_message = native_error.to_string();
+    let error = anyhow::Error::new(native_error)
+        .context("open ACL target (READ_CONTROL | WRITE_DAC)")
+        .context("prepare protected path");
+    let detail = format!(
+        "project/.git: prepare protected path: open ACL target (READ_CONTROL | WRITE_DAC): {native_message}"
+    );
+
+    let mut errors = Vec::new();
+    let mut log = Vec::new();
+    for operation in [
+        WriteAclOperation::Check,
+        WriteAclOperation::Grant,
+        WriteAclOperation::Deny,
+    ] {
+        record_acl_failure(&mut errors, operation, path, &error, |message| {
+            log.push(message.to_owned());
+            Ok(())
+        })
+        .expect("record ACL failure");
+    }
+    assert_eq!(
+        (errors, log),
+        (
+            vec![
+                format!("write ACE check failed on {detail}"),
+                format!("write ACE failed on {detail}"),
+                format!("deny ACE failed on {detail}"),
+            ],
+            vec![
+                format!("write ACE check failed on {detail}; continuing"),
+                format!("write ACE grant failed on {detail}"),
+                format!("deny ACE failed on {detail}"),
+            ],
+        )
+    );
+}
+
+#[test]
+fn log_failure_preserves_acl_failure_and_previous_roots() {
+    let mut errors = vec!["previous root failed".to_string()];
+    let error = anyhow!("SetSecurityInfo failed: 87").context("grant write ACL");
+    let log_error = record_acl_failure(
+        &mut errors,
+        WriteAclOperation::Grant,
+        Path::new("virtual-drive/project"),
+        &error,
+        |_| Err(anyhow!("log unavailable")),
+    )
+    .expect_err("report log failure");
+    assert_eq!(
+        (errors, log_error.to_string()),
+        (
+            vec![
+                "previous root failed".to_string(),
+                "write ACE failed on virtual-drive/project: grant write ACL: SetSecurityInfo failed: 87"
+                    .to_string(),
+            ],
+            "log unavailable".to_string(),
+        )
+    );
+}

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

@@ -32,6 +32,8 @@ use crate::sandbox_bin_dir;
 use crate::sandbox_dir;
 use crate::sandbox_secrets_dir;
 use crate::set_local_user_flags;
+use crate::setup_acl_error::WriteAclOperation;
+use crate::setup_acl_error::record_acl_failure;
 use crate::setup_error_path;
 use crate::setup_log_writer;
 use crate::string_from_sid_bytes;
@@ -884,7 +886,7 @@ fn lock_sandbox_bin_dir(payload: &Payload, sandbox_group_sid: &[u8]) -> Result<(
         anyhow::Error::new(SetupFailure::new(
             SetupErrorCode::HelperSandboxLockFailed,
             format!(
-                "lock sandbox bin dir {} failed: {err}",
+                "lock sandbox bin dir {} failed: {err:#}",
                 sandbox_bin_dir(&payload.codex_home).display()
             ),
         ))
@@ -1058,18 +1060,12 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res
             match path_write_aces_need_refresh(root, &[sandbox_group_psid, root_cap_psid]) {
                 Ok(needs_refresh) => needs_refresh,
                 Err(e) => {
-                    refresh_errors.push(format!(
-                        "write ACE check failed on {}: {}",
-                        root.display(),
-                        e
-                    ));
-                    log_line(
-                        log,
-                        &format!(
-                            "write ACE check failed on {}: {}; continuing",
-                            root.display(),
-                            e
-                        ),
+                    record_acl_failure(
+                        &mut refresh_errors,
+                        WriteAclOperation::Check,
+                        root,
+                        &e,
+                        |message| log_line(log, message),
                     )?;
                     true
                 }
@@ -1121,10 +1117,12 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res
             match res {
                 Ok(_) => {}
                 Err(e) => {
-                    refresh_errors.push(format!("write ACE failed on {}: {}", root.display(), e));
-                    if log_line(
-                        log,
-                        &format!("write ACE grant failed on {}: {}", root.display(), e),
+                    if record_acl_failure(
+                        &mut refresh_errors,
+                        WriteAclOperation::Grant,
+                        &root,
+                        &e,
+                        |message| log_line(log, message),
                     )
                     .is_err()
                     {
@@ -1175,10 +1173,12 @@ fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Res
                 }
                 Ok(false) => {}
                 Err(err) => {
-                    refresh_errors.push(format!("deny ACE failed on {}: {err}", path.display()));
-                    log_line(
-                        log,
-                        &format!("deny ACE failed on {}: {err}", path.display()),
+                    record_acl_failure(
+                        &mut refresh_errors,
+                        WriteAclOperation::Deny,
+                        path,
+                        &err,
+                        |message| log_line(log, message),
                     )?;
                 }
             }

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

@@ -15,12 +15,54 @@ use super::lock_sandbox_dir;
 use super::resolve_sid;
 use super::sid_bytes_to_psid;
 use crate::ensure_allow_write_aces;
+use crate::extract_setup_failure;
 use crate::path_mask_allows;
+use crate::read_setup_error_report;
 use crate::workspace_write_cap_sid_for_root;
+use crate::write_setup_error_report;
+use pretty_assertions::assert_eq;
 use std::fs;
+use windows_sys::Win32::Foundation::ERROR_PATH_NOT_FOUND;
 use windows_sys::Win32::Foundation::HLOCAL;
 use windows_sys::Win32::Foundation::LocalFree;
 
+#[test]
+fn bin_lock_report_retains_native_open_error() {
+    let temp = tempfile::tempdir().expect("tempdir");
+    let mut payload: Payload =
+        serde_json::from_value(super::tests::payload_json()).expect("fixture payload");
+    // The no-reparse open cannot create a bin whose parent does not exist.
+    // It fails before resolving account SIDs or changing any permissions.
+    payload.codex_home = temp.path().join("missing-home");
+    payload.mode = SetupMode::ProvisionOnly;
+    let bin = super::sandbox_bin_dir(&payload.codex_home);
+    let error = lock_sandbox_bin_dir(&payload, &[]).expect_err("missing bin parent");
+    let failure = extract_setup_failure(&error).expect("structured lock failure");
+    let expected = crate::SetupFailure::new(
+        crate::SetupErrorCode::HelperSandboxLockFailed,
+        format!(
+            "lock sandbox bin dir {0} failed: open directory {0}: NtCreateFile: {1}",
+            bin.display(),
+            std::io::Error::from_raw_os_error(ERROR_PATH_NOT_FOUND as i32),
+        ),
+    );
+    assert_eq!(failure, &expected);
+
+    write_setup_error_report(
+        temp.path(),
+        &crate::SetupErrorReport {
+            code: failure.code,
+            message: failure.message.clone(),
+        },
+    )
+    .expect("write structured failure");
+    let report = read_setup_error_report(temp.path())
+        .expect("read structured failure")
+        .expect("failure report");
+    assert_eq!(crate::SetupFailure::from_report(report), expected);
+    assert!(!payload.codex_home.exists());
+}
+
 #[test]
 fn provision_only_locks_plain_directory_via_handle() {
     let temp = tempfile::tempdir().expect("tempdir");