use a junction for the cwd while read ACLs are being applied (#8444)

The elevated setup synchronously applies read/write ACLs to any
workspace roots.

However, until we apply *read* permission to the full path, powershell
cannot use some roots as a cwd as it needs access to all parts of the
path in order to apply it as the working directory for a command.

The solution is, while the async read-ACL part of setup is running, use
a "junction" that lives in C:\Users\CodexSandbox{Offline|Online} that
points to the cwd.

Once the read ACLs are applied, we stop using the junction.

-----

this PR also removes some dead code and overly-verbose logging, and has
some light refactoring to the ACL-related functions
This commit is contained in:
iceweasel-oai 2025-12-22 12:23:13 -08:00 committed by GitHub
parent 7809e36a92
commit d65fe38b2c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 273 additions and 324 deletions

View file

@ -9,7 +9,6 @@ use windows_sys::Win32::Foundation::ERROR_SUCCESS;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
use windows_sys::Win32::Security::AclSizeInformation;
use windows_sys::Win32::Security::Authorization::GetEffectiveRightsFromAclW;
use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW;
use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
use windows_sys::Win32::Security::Authorization::SetEntriesInAclW;
@ -258,100 +257,18 @@ pub unsafe fn dacl_has_write_deny_for_sid(p_dacl: *mut ACL, psid: *mut c_void) -
false
}
// Compute effective rights for a trustee SID against a DACL and decide if write is effectively allowed.
// This accounts for deny ACEs and ordering; falls back to a conservative per-ACE scan if the API fails.
#[allow(dead_code)]
pub unsafe fn dacl_effective_allows_write(p_dacl: *mut ACL, psid: *mut c_void) -> bool {
if p_dacl.is_null() {
return false;
}
let trustee = TRUSTEE_W {
pMultipleTrustee: std::ptr::null_mut(),
MultipleTrusteeOperation: 0,
TrusteeForm: TRUSTEE_IS_SID,
TrusteeType: TRUSTEE_IS_UNKNOWN,
ptstrName: psid as *mut u16,
};
let mut access: u32 = 0;
let ok = GetEffectiveRightsFromAclW(p_dacl, &trustee, &mut access);
if ok == ERROR_SUCCESS {
// Map generic bits to avoid “missing” write when generic permissions are present.
let mut mapped_access = access;
if (access & GENERIC_WRITE_MASK) != 0 {
mapped_access |= FILE_GENERIC_WRITE | FILE_WRITE_DATA | FILE_APPEND_DATA;
}
if (access & READ_CONTROL) != 0 {
mapped_access |= FILE_GENERIC_READ;
}
let write_bits = FILE_GENERIC_WRITE
| FILE_WRITE_DATA
| FILE_APPEND_DATA
| FILE_WRITE_EA
| FILE_WRITE_ATTRIBUTES;
return (mapped_access & write_bits) != 0;
}
// Fallback: simple allow ACE scan (already ignores inherit-only)
dacl_has_write_allow_for_sid(p_dacl, psid)
}
#[allow(dead_code)]
pub unsafe fn dacl_effective_allows_mask(
p_dacl: *mut ACL,
psid: *mut c_void,
desired_mask: u32,
) -> bool {
if p_dacl.is_null() {
return false;
}
use windows_sys::Win32::Security::Authorization::GetEffectiveRightsFromAclW;
use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_SID;
use windows_sys::Win32::Security::Authorization::TRUSTEE_IS_UNKNOWN;
use windows_sys::Win32::Security::Authorization::TRUSTEE_W;
let trustee = TRUSTEE_W {
pMultipleTrustee: std::ptr::null_mut(),
MultipleTrusteeOperation: 0,
TrusteeForm: TRUSTEE_IS_SID,
TrusteeType: TRUSTEE_IS_UNKNOWN,
ptstrName: psid as *mut u16,
};
let mut access: u32 = 0;
let ok = GetEffectiveRightsFromAclW(p_dacl, &trustee, &mut access);
if ok == ERROR_SUCCESS {
// Map generic bits to avoid “missing” when generic permissions are present.
let mut mapped_access = access;
if (access & GENERIC_WRITE_MASK) != 0 {
mapped_access |= FILE_GENERIC_WRITE | FILE_WRITE_DATA | FILE_APPEND_DATA;
}
if (access & READ_CONTROL) != 0 {
mapped_access |= FILE_GENERIC_READ;
}
return (mapped_access & desired_mask) == desired_mask;
}
// Fallbacks on error: if write bits are requested, reuse the write helper; otherwise fail closed.
if (desired_mask & FILE_GENERIC_WRITE) != 0 {
return dacl_effective_allows_write(p_dacl, psid);
}
false
}
#[allow(dead_code)]
const WRITE_ALLOW_MASK: u32 = FILE_GENERIC_READ
| FILE_GENERIC_WRITE
| FILE_GENERIC_EXECUTE
| DELETE
| FILE_DELETE_CHILD;
/// Ensure all provided SIDs have an allow ACE with the requested mask on the path.
/// Returns true if any ACE was added.
///
/// # Safety
/// Caller must pass valid SID pointers and an existing path; free the returned security descriptor with `LocalFree`.
#[allow(dead_code)]
pub unsafe fn ensure_allow_mask_aces(
unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
path: &Path,
sids: &[*mut c_void],
allow_mask: u32,
inheritance: u32,
) -> Result<bool> {
let (p_dacl, p_sd) = fetch_dacl_handle(path)?;
let mut entries: Vec<EXPLICIT_ACCESS_W> = Vec::new();
@ -362,7 +279,7 @@ pub unsafe fn ensure_allow_mask_aces(
entries.push(EXPLICIT_ACCESS_W {
grfAccessPermissions: allow_mask,
grfAccessMode: 2, // SET_ACCESS
grfInheritance: CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
grfInheritance: inheritance,
Trustee: TRUSTEE_W {
pMultipleTrustee: std::ptr::null_mut(),
MultipleTrusteeOperation: 0,
@ -393,6 +310,9 @@ pub unsafe fn ensure_allow_mask_aces(
);
if code3 == ERROR_SUCCESS {
added = true;
if !p_new_dacl.is_null() {
LocalFree(p_new_dacl as HLOCAL);
}
} else {
if !p_new_dacl.is_null() {
LocalFree(p_new_dacl as HLOCAL);
@ -402,9 +322,6 @@ pub unsafe fn ensure_allow_mask_aces(
}
return Err(anyhow!("SetNamedSecurityInfoW failed: {}", code3));
}
if !p_new_dacl.is_null() {
LocalFree(p_new_dacl as HLOCAL);
}
} else {
if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL);
@ -418,12 +335,43 @@ pub unsafe fn ensure_allow_mask_aces(
Ok(added)
}
/// Ensure all provided SIDs have an allow ACE with the requested mask on the path.
/// Returns true if any ACE was added.
///
/// # Safety
/// Caller must pass valid SID pointers and an existing path; free the returned security descriptor with `LocalFree`.
pub unsafe fn ensure_allow_mask_aces_with_inheritance(
path: &Path,
sids: &[*mut c_void],
allow_mask: u32,
inheritance: u32,
) -> Result<bool> {
ensure_allow_mask_aces_with_inheritance_impl(path, sids, allow_mask, inheritance)
}
/// Ensure all provided SIDs have an allow ACE with the requested mask on the path.
/// Returns true if any ACE was added.
///
/// # Safety
/// Caller must pass valid SID pointers and an existing path; free the returned security descriptor with `LocalFree`.
pub unsafe fn ensure_allow_mask_aces(
path: &Path,
sids: &[*mut c_void],
allow_mask: u32,
) -> Result<bool> {
ensure_allow_mask_aces_with_inheritance(
path,
sids,
allow_mask,
CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
)
}
/// Ensure all provided SIDs have a write-capable allow ACE on the path.
/// Returns true if any ACE was added.
///
/// # Safety
/// Caller must pass valid SID pointers and an existing path; free the returned security descriptor with `LocalFree`.
#[allow(dead_code)]
pub unsafe fn ensure_allow_write_aces(path: &Path, sids: &[*mut c_void]) -> Result<bool> {
ensure_allow_mask_aces(path, sids, WRITE_ALLOW_MASK)
}

View file

@ -33,6 +33,12 @@ use windows_sys::Win32::System::Threading::TerminateProcess;
use windows_sys::Win32::System::Threading::WaitForSingleObject;
use windows_sys::Win32::System::Threading::INFINITE;
#[path = "cwd_junction.rs"]
mod cwd_junction;
#[allow(dead_code)]
mod read_acl_mutex;
#[derive(Debug, Deserialize)]
struct RunnerRequest {
policy_json_or_preset: String,
@ -146,16 +152,52 @@ pub fn main() -> Result<()> {
let h_stdin = open_pipe(&req.stdin_pipe, FILE_GENERIC_READ)?;
let h_stdout = open_pipe(&req.stdout_pipe, FILE_GENERIC_WRITE)?;
let h_stderr = open_pipe(&req.stderr_pipe, FILE_GENERIC_WRITE)?;
let stdio = Some((h_stdin, h_stdout, h_stderr));
// While the read-ACL helper is running, PowerShell can fail to start in the requested CWD due
// to unreadable ancestors. Use a junction CWD for that window; once the helper finishes, go
// back to using the real requested CWD (no probing, no extra state).
let use_junction = match read_acl_mutex::read_acl_mutex_exists() {
Ok(exists) => exists,
Err(err) => {
// Fail-safe: if we can't determine the state, assume the helper might be running and
// use the junction path to avoid CWD failures on unreadable ancestors.
log_note(
&format!("junction: read_acl_mutex_exists failed: {err}; assuming read ACL helper is running"),
log_dir,
);
true
}
};
if use_junction {
log_note(
"junction: read ACL helper running; using junction CWD",
log_dir,
);
}
let effective_cwd = if use_junction {
cwd_junction::create_cwd_junction(&req.cwd, log_dir).unwrap_or_else(|| req.cwd.clone())
} else {
req.cwd.clone()
};
log_note(
&format!(
"runner: effective cwd={} (requested {})",
effective_cwd.display(),
req.cwd.display()
),
log_dir,
);
// Build command and env, spawn with CreateProcessAsUserW.
let spawn_result = unsafe {
create_process_as_user(
h_token,
&req.command,
&req.cwd,
&effective_cwd,
&req.env_map,
Some(&req.codex_home),
Some((h_stdin, h_stdout, h_stderr)),
stdio,
)
};
let (proc_info, _si) = match spawn_result {
@ -219,9 +261,5 @@ pub fn main() -> Result<()> {
if exit_code != 0 {
eprintln!("runner child exited with code {}", exit_code);
}
log_note(
&format!("runner exit pid={} code={}", proc_info.hProcess, exit_code),
log_dir,
);
std::process::exit(exit_code);
}

View file

@ -0,0 +1,142 @@
#![cfg(target_os = "windows")]
use codex_windows_sandbox::log_note;
use std::collections::hash_map::DefaultHasher;
use std::hash::Hash;
use std::hash::Hasher;
use std::os::windows::fs::MetadataExt as _;
use std::os::windows::process::CommandExt as _;
use std::path::Path;
use std::path::PathBuf;
use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT;
fn junction_name_for_path(path: &Path) -> String {
let mut hasher = DefaultHasher::new();
path.to_string_lossy().hash(&mut hasher);
format!("{:x}", hasher.finish())
}
fn junction_root_for_userprofile(userprofile: &str) -> PathBuf {
PathBuf::from(userprofile)
.join(".codex")
.join(".sandbox")
.join("cwd")
}
pub fn create_cwd_junction(requested_cwd: &Path, log_dir: Option<&Path>) -> Option<PathBuf> {
let userprofile = std::env::var("USERPROFILE").ok()?;
let junction_root = junction_root_for_userprofile(&userprofile);
if let Err(err) = std::fs::create_dir_all(&junction_root) {
log_note(
&format!(
"junction: failed to create {}: {err}",
junction_root.display()
),
log_dir,
);
return None;
}
let junction_path = junction_root.join(junction_name_for_path(requested_cwd));
if junction_path.exists() {
// Reuse an existing junction if it looks like a reparse point; this keeps the hot path
// cheap and avoids repeatedly shelling out to mklink.
match std::fs::symlink_metadata(&junction_path) {
Ok(md) if (md.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT) != 0 => {
log_note(
&format!("junction: reusing existing {}", junction_path.display()),
log_dir,
);
return Some(junction_path);
}
Ok(_) => {
// Unexpected: something else exists at this path (regular directory/file). We'll
// try to remove it if possible and recreate the junction.
log_note(
&format!(
"junction: existing path is not a reparse point, recreating {}",
junction_path.display()
),
log_dir,
);
}
Err(err) => {
log_note(
&format!(
"junction: failed to stat existing {}: {err}",
junction_path.display()
),
log_dir,
);
return None;
}
}
if let Err(err) = std::fs::remove_dir(&junction_path) {
log_note(
&format!(
"junction: failed to remove existing {}: {err}",
junction_path.display()
),
log_dir,
);
return None;
}
}
let link = junction_path.to_string_lossy().to_string();
let target = requested_cwd.to_string_lossy().to_string();
// Use `cmd /c` so we can call `mklink` (a cmd builtin). We must quote paths so CWDs
// containing spaces work reliably.
//
// IMPORTANT: `std::process::Command::args()` will apply Windows quoting/escaping rules when
// constructing the command line. Passing a single argument that itself contains quotes can
// confuse `cmd.exe` and cause mklink to fail with "syntax is incorrect". We avoid that by
// using `raw_arg` to pass the tokens exactly as `cmd.exe` expects to parse them.
//
// Paths cannot contain quotes on Windows, so no extra escaping is needed here.
let link_quoted = format!("\"{link}\"");
let target_quoted = format!("\"{target}\"");
log_note(
&format!("junction: creating via cmd /c mklink /J {link_quoted} {target_quoted}"),
log_dir,
);
let output = match std::process::Command::new("cmd")
.raw_arg("/c")
.raw_arg("mklink")
.raw_arg("/J")
.raw_arg(&link_quoted)
.raw_arg(&target_quoted)
.output()
{
Ok(output) => output,
Err(err) => {
log_note(&format!("junction: mklink failed to run: {err}"), log_dir);
return None;
}
};
if output.status.success() && junction_path.exists() {
log_note(
&format!(
"junction: created {} -> {}",
junction_path.display(),
requested_cwd.display()
),
log_dir,
);
return Some(junction_path);
}
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);
log_note(
&format!(
"junction: mklink failed status={:?} stdout={} stderr={}",
output.status,
stdout.trim(),
stderr.trim()
),
log_dir,
);
None
}

View file

@ -7,7 +7,6 @@ mod windows_impl {
use crate::env::inherit_path_env;
use crate::env::normalize_null_device_env;
use crate::identity::require_logon_sandbox_creds;
use crate::logging::debug_log;
use crate::logging::log_failure;
use crate::logging::log_note;
use crate::logging::log_start;
@ -15,7 +14,6 @@ mod windows_impl {
use crate::policy::parse_policy;
use crate::policy::SandboxPolicy;
use crate::token::convert_string_sid_to_sid;
use crate::winutil::format_last_error;
use crate::winutil::quote_windows_arg;
use crate::winutil::to_wide;
use anyhow::Result;
@ -94,7 +92,7 @@ mod windows_impl {
fn inject_git_safe_directory(
env_map: &mut HashMap<String, String>,
cwd: &Path,
logs_base_dir: Option<&Path>,
_logs_base_dir: Option<&Path>,
) {
if let Some(git_root) = find_git_root(cwd) {
let mut cfg_count: usize = env_map
@ -109,10 +107,6 @@ mod windows_impl {
env_map.insert(format!("GIT_CONFIG_VALUE_{cfg_count}"), git_path);
cfg_count += 1;
env_map.insert("GIT_CONFIG_COUNT".to_string(), cfg_count.to_string());
log_note(
&format!("injected git safe.directory for {}", git_root.display()),
logs_base_dir,
);
}
}
@ -237,7 +231,6 @@ mod windows_impl {
log_start(&command, logs_base_dir);
let sandbox_creds =
require_logon_sandbox_creds(&policy, sandbox_policy_cwd, cwd, &env_map, codex_home)?;
log_note("cli creds ready", logs_base_dir);
// Build capability SID for ACL grants.
if matches!(
&policy,
@ -283,13 +276,6 @@ mod windows_impl {
&stderr_name,
PIPE_ACCESS_DUPLEX | PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
)?;
log_note(
&format!(
"cli pipes created stdin={} stdout={} stderr={}",
stdin_name, stdout_name, stderr_name
),
logs_base_dir,
);
// Launch runner as sandbox user via CreateProcessWithLogonW.
let runner_exe = find_runner_exe();
@ -326,10 +312,6 @@ mod windows_impl {
);
return Err(e.into());
}
log_note(
&format!("cli request file written path={}", req_file.display()),
logs_base_dir,
);
let runner_full_cmd = format!(
"{} {}",
quote_windows_arg(&runner_cmdline),
@ -338,14 +320,6 @@ mod windows_impl {
let mut cmdline_vec: Vec<u16> = to_wide(&runner_full_cmd);
let exe_w: Vec<u16> = to_wide(&runner_cmdline);
let cwd_w: Vec<u16> = to_wide(cwd);
log_note(
&format!("cli prep done request_file={}", req_file.display()),
logs_base_dir,
);
log_note(
&format!("cli about to launch runner cmd={}", runner_full_cmd),
logs_base_dir,
);
// Minimal CPWL launch: inherit env, no desktop override, no handle inheritance.
let env_block: Option<Vec<u16>> = None;
@ -380,23 +354,8 @@ mod windows_impl {
};
if spawn_res == 0 {
let err = unsafe { GetLastError() } as i32;
let dbg = format!(
"CreateProcessWithLogonW failed: {} ({}) | cwd={} | cmd={} | req_file={} | env=inherit | si_flags={}",
err,
format_last_error(err),
cwd.display(),
runner_full_cmd,
req_file.display(),
si.dwFlags,
);
debug_log(&dbg, logs_base_dir);
log_note(&dbg, logs_base_dir);
return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {}", err));
}
log_note(
&format!("cli runner launched pid={}", pi.hProcess),
logs_base_dir,
);
// Pipes are no longer passed as std handles; no stdin payload is sent.
connect_pipe(h_stdin_pipe)?;

View file

@ -29,7 +29,6 @@ pub fn ensure_non_interactive_pager(env_map: &mut HashMap<String, String>) {
}
// Keep PATH and PATHEXT stable for callers that rely on inheriting the parent process env.
#[allow(dead_code)]
pub fn inherit_path_env(env_map: &mut HashMap<String, String>) {
if !env_map.contains_key("PATH") {
if let Ok(path) = env::var("PATH") {

View file

@ -20,6 +20,8 @@ pub use acl::allow_null_device;
#[cfg(target_os = "windows")]
pub use acl::ensure_allow_mask_aces;
#[cfg(target_os = "windows")]
pub use acl::ensure_allow_mask_aces_with_inheritance;
#[cfg(target_os = "windows")]
pub use acl::ensure_allow_write_aces;
#[cfg(target_os = "windows")]
pub use acl::fetch_dacl_handle;

View file

@ -16,22 +16,12 @@ use windows_sys::Win32::System::Console::GetStdHandle;
use windows_sys::Win32::System::Console::STD_ERROR_HANDLE;
use windows_sys::Win32::System::Console::STD_INPUT_HANDLE;
use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE;
use windows_sys::Win32::System::JobObjects::AssignProcessToJobObject;
use windows_sys::Win32::System::JobObjects::CreateJobObjectW;
use windows_sys::Win32::System::JobObjects::JobObjectExtendedLimitInformation;
use windows_sys::Win32::System::JobObjects::SetInformationJobObject;
use windows_sys::Win32::System::JobObjects::JOBOBJECT_EXTENDED_LIMIT_INFORMATION;
use windows_sys::Win32::System::JobObjects::JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
use windows_sys::Win32::System::Threading::CreateProcessAsUserW;
use windows_sys::Win32::System::Threading::GetExitCodeProcess;
use windows_sys::Win32::System::Threading::WaitForSingleObject;
use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT;
use windows_sys::Win32::System::Threading::INFINITE;
use windows_sys::Win32::System::Threading::PROCESS_INFORMATION;
use windows_sys::Win32::System::Threading::STARTF_USESTDHANDLES;
use windows_sys::Win32::System::Threading::STARTUPINFOW;
#[allow(dead_code)]
pub fn make_env_block(env: &HashMap<String, String>) -> Vec<u16> {
let mut items: Vec<(String, String)> =
env.iter().map(|(k, v)| (k.clone(), v.clone())).collect();
@ -51,7 +41,6 @@ pub fn make_env_block(env: &HashMap<String, String>) -> Vec<u16> {
w
}
#[allow(dead_code)]
unsafe fn ensure_inheritable_stdio(si: &mut STARTUPINFOW) -> Result<()> {
for kind in [STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE] {
let h = GetStdHandle(kind);
@ -72,7 +61,6 @@ unsafe fn ensure_inheritable_stdio(si: &mut STARTUPINFOW) -> Result<()> {
/// # Safety
/// Caller must provide a valid primary token handle (`h_token`) with appropriate access,
/// and the `argv`, `cwd`, and `env_map` must remain valid for the duration of the call.
#[allow(dead_code)]
pub unsafe fn create_process_as_user(
h_token: HANDLE,
argv: &[String],
@ -148,56 +136,3 @@ pub unsafe fn create_process_as_user(
}
Ok((pi, si))
}
/// # Safety
/// Caller must provide valid process information handles.
#[allow(dead_code)]
pub unsafe fn wait_process_and_exitcode(pi: &PROCESS_INFORMATION) -> Result<i32> {
let res = WaitForSingleObject(pi.hProcess, INFINITE);
if res != 0 {
return Err(anyhow!("WaitForSingleObject failed: {}", GetLastError()));
}
let mut code: u32 = 0;
if GetExitCodeProcess(pi.hProcess, &mut code) == 0 {
return Err(anyhow!("GetExitCodeProcess failed: {}", GetLastError()));
}
Ok(code as i32)
}
/// # Safety
/// Caller must close the returned job handle.
#[allow(dead_code)]
pub unsafe fn create_job_kill_on_close() -> Result<HANDLE> {
let h = CreateJobObjectW(std::ptr::null_mut(), std::ptr::null());
if h == 0 {
return Err(anyhow!("CreateJobObjectW failed: {}", GetLastError()));
}
let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = std::mem::zeroed();
limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
let ok = SetInformationJobObject(
h,
JobObjectExtendedLimitInformation,
&mut limits as *mut _ as *mut c_void,
std::mem::size_of::<JOBOBJECT_EXTENDED_LIMIT_INFORMATION>() as u32,
);
if ok == 0 {
return Err(anyhow!(
"SetInformationJobObject failed: {}",
GetLastError()
));
}
Ok(h)
}
/// # Safety
/// Caller must pass valid handles for a job object and a process.
#[allow(dead_code)]
pub unsafe fn assign_to_job(h_job: HANDLE, h_process: HANDLE) -> Result<()> {
if AssignProcessToJobObject(h_job, h_process) == 0 {
return Err(anyhow!(
"AssignProcessToJobObject failed: {}",
GetLastError()
));
}
Ok(())
}

View file

@ -6,7 +6,7 @@ use base64::engine::general_purpose::STANDARD as BASE64;
use base64::Engine;
use codex_windows_sandbox::convert_string_sid_to_sid;
use codex_windows_sandbox::dpapi_protect;
use codex_windows_sandbox::ensure_allow_mask_aces;
use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
use codex_windows_sandbox::ensure_allow_write_aces;
use codex_windows_sandbox::load_or_create_cap_sids;
use codex_windows_sandbox::log_note;
@ -32,7 +32,6 @@ use std::path::PathBuf;
use std::process::Command;
use std::process::Stdio;
use std::sync::mpsc;
use std::time::Instant;
use windows::core::Interface;
use windows::core::BSTR;
use windows::Win32::Foundation::VARIANT_TRUE;
@ -267,14 +266,14 @@ fn resolve_sid(name: &str) -> Result<Vec<u8>> {
}
}
fn spawn_read_acl_helper(payload: &Payload, log: &mut File) -> Result<()> {
fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
let mut read_payload = payload.clone();
read_payload.mode = SetupMode::ReadAclsOnly;
read_payload.refresh_only = true;
let payload_json = serde_json::to_vec(&read_payload)?;
let payload_b64 = BASE64.encode(payload_json);
let exe = std::env::current_exe().context("locate setup helper")?;
let child = Command::new(&exe)
Command::new(&exe)
.arg(payload_b64)
.stdin(Stdio::null())
.stdout(Stdio::null())
@ -282,8 +281,6 @@ fn spawn_read_acl_helper(payload: &Payload, log: &mut File) -> Result<()> {
.creation_flags(0x08000000) // CREATE_NO_WINDOW
.spawn()
.context("spawn read ACL helper")?;
let pid = child.id();
log_line(log, &format!("spawned read ACL helper pid={pid}"))?;
Ok(())
}
@ -295,20 +292,18 @@ struct ReadAclSubjects<'a> {
fn apply_read_acls(
read_roots: &[PathBuf],
subjects: ReadAclSubjects<'_>,
subjects: &ReadAclSubjects<'_>,
log: &mut File,
refresh_errors: &mut Vec<String>,
access_mask: u32,
access_label: &str,
inheritance: u32,
) -> Result<()> {
log_line(
log,
&format!("read ACL: processing {} read roots", read_roots.len()),
)?;
let read_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE;
for root in read_roots {
if !root.exists() {
log_line(
log,
&format!("read root {} missing; skipping", root.display()),
&format!("{access_label} root {} missing; skipping", root.display()),
)?;
continue;
}
@ -316,25 +311,20 @@ fn apply_read_acls(
root,
subjects.rx_psids,
None,
read_mask,
access_mask,
access_label,
refresh_errors,
log,
)?;
if builtin_has {
log_line(
log,
&format!(
"Users/AU/Everyone already has RX on {}; skipping",
root.display()
),
)?;
continue;
}
let offline_has = read_mask_allows_or_log(
root,
&[subjects.offline_psid],
Some("offline"),
read_mask,
access_mask,
access_label,
refresh_errors,
log,
)?;
@ -342,23 +332,20 @@ fn apply_read_acls(
root,
&[subjects.online_psid],
Some("online"),
read_mask,
access_mask,
access_label,
refresh_errors,
log,
)?;
if offline_has && online_has {
log_line(
log,
&format!(
"sandbox users already have RX on {}; skipping",
root.display()
),
)?;
continue;
}
log_line(
log,
&format!("granting read ACE to {} for sandbox users", root.display()),
&format!(
"granting {access_label} ACE to {} for sandbox users",
root.display()
),
)?;
let mut successes = usize::from(offline_has) + usize::from(online_has);
let mut missing_psids: Vec<*mut c_void> = Vec::new();
@ -372,33 +359,30 @@ fn apply_read_acls(
missing_labels.push("online");
}
if !missing_psids.is_empty() {
let started = Instant::now();
match unsafe { ensure_allow_mask_aces(root, &missing_psids, read_mask) } {
let result = unsafe {
ensure_allow_mask_aces_with_inheritance(
root,
&missing_psids,
access_mask,
inheritance,
)
};
match result {
Ok(_) => {
let elapsed_ms = started.elapsed().as_millis();
successes = 2;
log_line(
log,
&format!(
"grant read ACE succeeded on {} for {} in {elapsed_ms}ms",
root.display(),
missing_labels.join(", ")
),
)?;
}
Err(err) => {
let elapsed_ms = started.elapsed().as_millis();
let label_list = missing_labels.join(", ");
for label in &missing_labels {
refresh_errors.push(format!(
"grant read ACE failed on {} for {label}: {err}",
"grant {access_label} ACE failed on {} for {label}: {err}",
root.display()
));
}
log_line(
log,
&format!(
"grant read ACE failed on {} for {} in {elapsed_ms}ms: {err}",
"grant {access_label} ACE failed on {} for {}: {err}",
root.display(),
label_list
),
@ -407,12 +391,11 @@ fn apply_read_acls(
}
}
if successes == 2 {
log_line(log, &format!("granted read ACE to {}", root.display()))?;
} else {
log_line(
log,
&format!(
"read ACE incomplete on {} (success {successes}/2)",
"{access_label} ACE incomplete on {} (success {successes}/2)",
root.display()
),
)?;
@ -426,6 +409,7 @@ fn read_mask_allows_or_log(
psids: &[*mut c_void],
label: Option<&str>,
read_mask: u32,
access_label: &str,
refresh_errors: &mut Vec<String>,
log: &mut File,
) -> Result<bool> {
@ -436,7 +420,7 @@ fn read_mask_allows_or_log(
.map(|value| format!(" for {value}"))
.unwrap_or_default();
refresh_errors.push(format!(
"read mask check failed on {}{}: {}",
"{access_label} mask check failed on {}{}: {}",
root.display(),
label_suffix,
e
@ -444,7 +428,7 @@ fn read_mask_allows_or_log(
log_line(
log,
&format!(
"read mask check failed on {}{}: {}; continuing",
"{access_label} mask check failed on {}{}: {}; continuing",
root.display(),
label_suffix,
e
@ -532,7 +516,7 @@ fn lock_sandbox_dir(
dir: &Path,
real_user: &str,
sandbox_user_sids: &[Vec<u8>],
log: &mut File,
_log: &mut File,
) -> Result<()> {
std::fs::create_dir_all(dir)?;
let system_sid = resolve_sid("SYSTEM")?;
@ -630,10 +614,6 @@ fn lock_sandbox_dir(
}
}
}
log_line(
log,
&format!("sandbox dir ACL applied at {}", dir.display()),
)?;
Ok(())
}
@ -719,8 +699,6 @@ fn real_main() -> Result<()> {
.append(true)
.open(&log_path)
.context("open log")?;
log_line(&mut log, "setup binary started")?;
log_note("setup binary started", Some(sbx_dir.as_path()));
let result = run_setup(&payload, &mut log, &sbx_dir);
if let Err(err) = &result {
let _ = log_line(&mut log, &format!("setup error: {err:?}"));
@ -762,7 +740,15 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
online_psid,
rx_psids: &rx_psids,
};
apply_read_acls(&payload.read_roots, subjects, log, &mut refresh_errors)?;
apply_read_acls(
&payload.read_roots,
&subjects,
log,
&mut refresh_errors,
FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
"read",
OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE,
)?;
unsafe {
if !offline_psid.is_null() {
LocalFree(offline_psid as HLOCAL);
@ -806,7 +792,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
Some(random_password())
};
if refresh_only {
log_line(log, "refresh-only mode: skipping user creation/firewall")?;
} else {
log_line(
log,
@ -827,33 +812,17 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
let offline_psid = sid_bytes_to_psid(&offline_sid)?;
let online_psid = sid_bytes_to_psid(&online_sid)?;
let offline_sid_str = string_from_sid_bytes(&offline_sid).map_err(anyhow::Error::msg)?;
log_line(
log,
&format!(
"resolved SIDs offline={} online={}",
offline_sid_str,
string_from_sid_bytes(&online_sid).map_err(anyhow::Error::msg)?
),
)?;
let caps = load_or_create_cap_sids(&payload.codex_home)?;
let cap_psid = unsafe {
convert_string_sid_to_sid(&caps.workspace)
.ok_or_else(|| anyhow::anyhow!("convert capability SID failed"))?
};
let mut refresh_errors: Vec<String> = Vec::new();
log_line(log, &format!("resolved capability SID {}", caps.workspace))?;
if !refresh_only {
run_netsh_firewall(&offline_sid_str, log)?;
}
log_line(
log,
&format!(
"refresh: delegating {} read roots; processing {} write roots",
payload.read_roots.len(),
payload.write_roots.len()
),
)?;
if payload.read_roots.is_empty() {
log_line(log, "no read roots to grant; skipping read ACL helper")?;
} else {
@ -919,14 +888,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
false
}
};
log_line(
log,
&format!(
"write check {label} on {} => {}",
root.display(),
if has { "present" } else { "missing" }
),
)?;
if !has {
need_grant = true;
}
@ -940,14 +901,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
),
)?;
grant_tasks.push(root.clone());
} else {
log_line(
log,
&format!(
"write ACE already present for all sandbox SIDs on {}",
root.display()
),
)?;
}
}
@ -981,20 +934,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
drop(tx);
for (root, res) in rx {
match res {
Ok(added) => {
if log_line(
log,
&format!(
"write ACE {} on {}",
if added { "added" } else { "already present" },
root.display()
),
)
.is_err()
{
// ignore log errors inside scoped thread
}
}
Ok(_) => {}
Err(e) => {
refresh_errors.push(format!("write ACE failed on {}: {}", root.display(), e));
if log_line(
@ -1027,7 +967,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&[offline_sid.clone(), online_sid.clone()],
log,
)?;
log_line(log, "sandbox dir ACL applied")?;
write_secrets(
&payload.codex_home,
&payload.offline_username,
@ -1037,10 +976,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&payload.read_roots,
&payload.write_roots,
)?;
log_line(
log,
"sandbox users and marker written (sandbox_users.json, setup_marker.json)",
)?;
}
unsafe {
if !offline_psid.is_null() {
@ -1060,7 +995,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
)?;
anyhow::bail!("setup refresh had errors");
}
log_line(log, "setup binary completed")?;
log_note("setup binary completed", Some(sbx_dir));
Ok(())
}

View file

@ -80,10 +80,6 @@ pub fn run_setup_refresh(
let json = serde_json::to_vec(&payload)?;
let b64 = BASE64_STANDARD.encode(json);
let exe = find_setup_exe();
log_note(
&format!("setup refresh: invoking {}", exe.display()),
Some(&sandbox_dir(codex_home)),
);
// Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC.
let mut cmd = Command::new(&exe);
cmd.arg(&b64).stdout(Stdio::null()).stderr(Stdio::null());

View file

@ -144,7 +144,6 @@ pub unsafe fn convert_string_sid_to_sid(s: &str) -> Option<*mut c_void> {
/// # Safety
/// Caller must close the returned token handle.
#[allow(dead_code)]
pub unsafe fn get_current_token_for_restriction() -> Result<HANDLE> {
let desired = TOKEN_DUPLICATE
| TOKEN_QUERY
@ -285,7 +284,6 @@ unsafe fn enable_single_privilege(h_token: HANDLE, name: &str) -> Result<()> {
/// # Safety
/// Caller must close the returned token handle.
#[allow(dead_code)]
pub unsafe fn create_workspace_write_token_with_cap(
psid_capability: *mut c_void,
) -> Result<(HANDLE, *mut c_void)> {
@ -297,7 +295,6 @@ pub unsafe fn create_workspace_write_token_with_cap(
/// # Safety
/// Caller must close the returned token handle.
#[allow(dead_code)]
pub unsafe fn create_readonly_token_with_cap(
psid_capability: *mut c_void,
) -> Result<(HANDLE, *mut c_void)> {

View file

@ -85,7 +85,6 @@ pub fn format_last_error(err: i32) -> String {
}
}
#[allow(dead_code)]
pub fn string_from_sid_bytes(sid: &[u8]) -> Result<String, String> {
unsafe {
let mut str_ptr: *mut u16 = std::ptr::null_mut();