From c636f89fc534b90d6be46578fbdbe2bd3d10b8db Mon Sep 17 00:00:00 2001 From: David Wiesen Date: Tue, 28 Apr 2026 12:04:16 -0700 Subject: [PATCH] Fix elevated Windows sandbox write restriction scope --- .../windows-sandbox-rs/sandbox_smoketests.py | 22 +++++++++++++++++++ codex-rs/windows-sandbox-rs/src/token.rs | 11 ++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/sandbox_smoketests.py b/codex-rs/windows-sandbox-rs/sandbox_smoketests.py index f629a4c6d9..a373a16d56 100644 --- a/codex-rs/windows-sandbox-rs/sandbox_smoketests.py +++ b/codex-rs/windows-sandbox-rs/sandbox_smoketests.py @@ -248,6 +248,28 @@ def main() -> int: rc, out, err = run_sbx("workspace-write", ["cmd", "/c", f"echo nope > {outside_file}"], WS_ROOT) add("WS: write outside workspace denied", rc != 0 and assert_not_exists(outside_file), f"rc={rc}") + # 3a. WS: deny write outside workspace even when the target grants Everyone full control + world_writable_outside = OUTSIDE / "world-writable" + world_writable_file = world_writable_outside / "blocked.txt" + remove_if_exists(world_writable_file) + world_writable_outside.mkdir(parents=True, exist_ok=True) + subprocess.run( + ["icacls", str(world_writable_outside), "/grant", "Everyone:(F)"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + rc, out, err = run_sbx( + "workspace-write", + ["cmd", "/c", f"echo nope > {world_writable_file}"], + WS_ROOT, + ) + add( + "WS: world-writable outside workspace denied", + rc != 0 and assert_not_exists(world_writable_file), + f"rc={rc}", + ) + # 3b. WS: allow write in additional workspace root extra_target = EXTRA_ROOT / "extra_ok.txt" remove_if_exists(extra_target) diff --git a/codex-rs/windows-sandbox-rs/src/token.rs b/codex-rs/windows-sandbox-rs/src/token.rs index 7d7a6c5c41..3aa0023421 100644 --- a/codex-rs/windows-sandbox-rs/src/token.rs +++ b/codex-rs/windows-sandbox-rs/src/token.rs @@ -338,18 +338,15 @@ unsafe fn create_token_with_caps_from( let mut everyone = world_sid()?; let psid_everyone = everyone.as_mut_ptr() as *mut c_void; - // Exact order: Capabilities..., Logon, Everyone + // Only capability SIDs should participate in the write-restricted check. Including + // broad identities like the logon SID or Everyone would let unrelated filesystem ACLs + // satisfy the restricted write gate outside the workspace. let mut entries: Vec = - vec![std::mem::zeroed(); psid_capabilities.len() + 2]; + vec![std::mem::zeroed(); psid_capabilities.len()]; for (i, psid) in psid_capabilities.iter().enumerate() { entries[i].Sid = *psid; entries[i].Attributes = 0; } - let logon_idx = psid_capabilities.len(); - entries[logon_idx].Sid = psid_logon; - entries[logon_idx].Attributes = 0; - entries[logon_idx + 1].Sid = psid_everyone; - entries[logon_idx + 1].Attributes = 0; let mut new_token: HANDLE = 0; let flags = DISABLE_MAX_PRIVILEGE | LUA_TOKEN | WRITE_RESTRICTED;