mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Diagnose sandbox-owned paths during Windows ACL refresh
This commit is contained in:
@@ -14,6 +14,7 @@ use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
|
||||
use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
|
||||
use windows_sys::Win32::Security::Authorization::SetNamedSecurityInfoW;
|
||||
use windows_sys::Win32::Security::Authorization::SetSecurityInfo;
|
||||
use windows_sys::Win32::Security::Authorization::ConvertSidToStringSidW;
|
||||
use windows_sys::Win32::Security::Authorization::EXPLICIT_ACCESS_W;
|
||||
use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
|
||||
use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN;
|
||||
@@ -28,6 +29,7 @@ use windows_sys::Win32::Security::ACL;
|
||||
use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
|
||||
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
|
||||
use windows_sys::Win32::Security::GENERIC_MAPPING;
|
||||
use windows_sys::Win32::Security::OWNER_SECURITY_INFORMATION;
|
||||
use windows_sys::Win32::Storage::FileSystem::CreateFileW;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
|
||||
@@ -92,6 +94,52 @@ pub unsafe fn fetch_dacl_handle(path: &Path) -> Result<(*mut ACL, *mut c_void)>
|
||||
Ok((p_dacl, p_sd))
|
||||
}
|
||||
|
||||
pub fn path_owner_sid_string(path: &Path) -> Result<Option<String>> {
|
||||
unsafe {
|
||||
let mut owner: *mut c_void = std::ptr::null_mut();
|
||||
let mut p_sd: *mut c_void = std::ptr::null_mut();
|
||||
let code = GetNamedSecurityInfoW(
|
||||
to_wide(path).as_ptr() as *mut u16,
|
||||
1, // SE_FILE_OBJECT
|
||||
OWNER_SECURITY_INFORMATION,
|
||||
&mut owner,
|
||||
std::ptr::null_mut(),
|
||||
std::ptr::null_mut(),
|
||||
std::ptr::null_mut(),
|
||||
&mut p_sd,
|
||||
);
|
||||
if code != ERROR_SUCCESS {
|
||||
return Err(anyhow!("GetNamedSecurityInfoW owner failed: {code}"));
|
||||
}
|
||||
if owner.is_null() {
|
||||
if !p_sd.is_null() {
|
||||
LocalFree(p_sd as HLOCAL);
|
||||
}
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let mut sid_string_ptr: *mut u16 = std::ptr::null_mut();
|
||||
let ok = ConvertSidToStringSidW(owner, &mut sid_string_ptr);
|
||||
if ok == 0 || sid_string_ptr.is_null() {
|
||||
if !p_sd.is_null() {
|
||||
LocalFree(p_sd as HLOCAL);
|
||||
}
|
||||
return Err(anyhow!("ConvertSidToStringSidW owner failed"));
|
||||
}
|
||||
let mut len = 0;
|
||||
while *sid_string_ptr.add(len) != 0 {
|
||||
len += 1;
|
||||
}
|
||||
let sid =
|
||||
String::from_utf16_lossy(std::slice::from_raw_parts(sid_string_ptr, len as usize));
|
||||
LocalFree(sid_string_ptr as HLOCAL);
|
||||
if !p_sd.is_null() {
|
||||
LocalFree(p_sd as HLOCAL);
|
||||
}
|
||||
Ok(Some(sid))
|
||||
}
|
||||
}
|
||||
|
||||
/// Fast mask-based check: does an ACE for provided SIDs grant the desired mask? Skips inherit-only.
|
||||
/// When `require_all_bits` is true, all bits in `desired_mask` must be present; otherwise any bit suffices.
|
||||
pub unsafe fn dacl_mask_allows(
|
||||
|
||||
@@ -62,6 +62,8 @@ pub use acl::fetch_dacl_handle;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use acl::path_mask_allows;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use acl::path_owner_sid_string;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use audit::apply_world_writable_scan_and_denies;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use cap::load_or_create_cap_sids;
|
||||
|
||||
@@ -21,6 +21,7 @@ use codex_windows_sandbox::is_command_cwd_root;
|
||||
use codex_windows_sandbox::load_or_create_cap_sids;
|
||||
use codex_windows_sandbox::log_note;
|
||||
use codex_windows_sandbox::path_mask_allows;
|
||||
use codex_windows_sandbox::path_owner_sid_string;
|
||||
use codex_windows_sandbox::protect_workspace_agents_dir;
|
||||
use codex_windows_sandbox::protect_workspace_codex_dir;
|
||||
use codex_windows_sandbox::sandbox_bin_dir;
|
||||
@@ -114,6 +115,27 @@ fn log_line(log: &mut File, msg: &str) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn add_write_ace_owner_context(
|
||||
path: &Path,
|
||||
err: anyhow::Error,
|
||||
offline_sandbox_sid: &str,
|
||||
offline_sandbox_username: &str,
|
||||
) -> anyhow::Error {
|
||||
let owner = match path_owner_sid_string(path) {
|
||||
Ok(Some(owner)) => owner,
|
||||
Ok(None) => return err.context("path has no owner SID"),
|
||||
Err(owner_err) => return err.context(format!("owner lookup failed: {owner_err}")),
|
||||
};
|
||||
|
||||
if owner.eq_ignore_ascii_case(offline_sandbox_sid) {
|
||||
return err.context(format!(
|
||||
"path owner is {offline_sandbox_username}; repair ownership of this workspace path as the normal Windows user"
|
||||
));
|
||||
}
|
||||
|
||||
err.context(format!("path owner SID is {owner}"))
|
||||
}
|
||||
|
||||
fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
|
||||
let mut read_payload = payload.clone();
|
||||
read_payload.mode = SetupMode::ReadAclsOnly;
|
||||
@@ -718,6 +740,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
|
||||
};
|
||||
let tx = tx.clone();
|
||||
let offline_sid_str = offline_sid_str.clone();
|
||||
let offline_username = payload.offline_username.clone();
|
||||
scope.spawn(move || {
|
||||
// Convert SID strings to psids locally in this thread.
|
||||
let mut psids: Vec<*mut c_void> = Vec::new();
|
||||
@@ -730,7 +754,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
}
|
||||
}
|
||||
|
||||
let res = unsafe { ensure_allow_write_aces(&root, &psids) };
|
||||
let res = unsafe { ensure_allow_write_aces(&root, &psids) }.map_err(|err| {
|
||||
add_write_ace_owner_context(&root, err, &offline_sid_str, &offline_username)
|
||||
});
|
||||
|
||||
for psid in psids {
|
||||
unsafe {
|
||||
|
||||
Reference in New Issue
Block a user