openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Avoid sharing violations when repairing Windows runtime ACLs (#51822)

Browse Source 
## Why

Opening active EXE/DLL files with `MAXIMUM_ALLOWED` can request access that causes sharing violations during sandbox runtime ACL repair.

## What changed

Restrict `MAXIMUM_ALLOWED` opens to directories that need to suppress child ACL propagation. For files, request `READ_CONTROL | WRITE_DAC` only when an ACL update is needed.

Preserve the non-inheriting grant behavior from https://github.com/openai/codex/pull/49690, including existing permissions, deny ACEs, and null DACLs.

## Testing

Extend the Windows runtime repair regression test to hold `node.exe` open with `FILE_SHARE_READ` throughout repair and revalidation, while checking read/execute access and preserved denials.

GitOrigin-RevId: 2e087b51adc0221d0eda061fa0a34d2e98409662
This commit is contained in:
johnl-oai
2026-10-07 20:46:56 +00:00
committed by copyberry
parent 1c7c43cd85
commit dd12f892f1
2 changed files with 21 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/windows-sandbox-rs/src/acl.rs
+12 -5
View File
@@ -546,7 +546,14 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
access_mode: ACCESS_MODE,
inheritance: u32,
) -> Result<bool> {
let directory = if inheritance == 0 {
// Only directories need MAXIMUM_ALLOWED to suppress child ACL propagation.
// On active EXE/DLL files, it can request access that causes sharing violations.
let directory = if inheritance == 0
&& path
.metadata()
.with_context(|| format!("inspect ACL target {}", path.display()))?
.is_dir()
{
Some(
OpenOptions::new()
.access_mode(MAXIMUM_ALLOWED)
@@ -559,7 +566,7 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
None
};
// A root-only grant must preserve existing permissions and deny ACEs.
let access_mode = if directory.is_some() {
let access_mode = if inheritance == 0 {
GRANT_ACCESS
} else {
access_mode
@@ -610,7 +617,7 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
continue;
}
if !dacl_allow_mask_needs_refresh(p_dacl, *sid, allow_mask, disallow_mask)
|| directory.is_some() && p_dacl.is_null()
|| inheritance == 0 && p_dacl.is_null()
{
continue;
}
@@ -643,8 +650,8 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
if code2 != ERROR_SUCCESS {
return Err(anyhow!("SetEntriesInAclW failed: {code2}"));
}
// For inheriting grants, request ACL-write access only when an update
// is needed. Use the same long-path support as the read handle.
// For files and inheriting directory grants, request ACL-write access only
// when an update is needed. Use the same long-path support as the read handle.
let handle = match directory {
// MAXIMUM_ALLOWED suppresses propagation of existing inheritable ACEs too.
Some(directory) => directory,
codex-rs/windows-sandbox-rs/src/setup_provisioning/setup_runtime_bin_tests.rs
+9
View File
@@ -7,6 +7,7 @@ use crate::ensure_allow_mask_aces_with_inheritance;
use crate::path_mask_allows;
use pretty_assertions::assert_eq;
use std::fs;
use std::os::windows::fs::OpenOptionsExt;
use std::path::PathBuf;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::LocalFree;
@@ -20,6 +21,7 @@ use windows_sys::Win32::Storage::FileSystem::DELETE;
use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA;
use windows_sys::Win32::Storage::FileSystem::WRITE_DAC;
@@ -166,6 +168,13 @@ fn repairs_children_when_runtime_root_already_has_read_execute_access() {
.expect("deny runtime writes")
);
// Keep data-write and delete opens blocked throughout repair and revalidation.
let _reader = fs::OpenOptions::new()
.read(true)
.share_mode(FILE_SHARE_READ)
.open(&node)
.expect("hold runtime file open for reading");
for _ in 0..2 {
ensure_runtime_tree_readable(runtime.path(), sandbox_sid.as_ptr())
.expect("repair runtimes");