agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] Scope Windows sandbox write-root capability SIDs (#21479)

Browse Source 
## Summary
- fix by scoping Windows workspace-write capability SIDs to active
effective write roots
- build legacy/elevated tokens from only the active effective write
roots
- align setup/audit deny ACL handling with active root-specific SIDs

## Testing
- just fmt
- git diff --check --cached
- just argument-comment-lint
- cargo check -p codex-windows-sandbox --locked (blocked by libwebrtc ->
libyuv fetch: CONNECT tunnel failed, response 403)
This commit is contained in:
Adrian
2026-05-13 18:28:52 +00:00
committed by GitHub
parent 1ae811ddb2
commit 3d2a0b5517
9 changed files with 881 additions and 416 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/windows-sandbox-rs/src/audit.rs
+47 -33
View File
@@ -2,14 +2,16 @@ use crate::acl::add_deny_write_ace;
use crate::acl::path_mask_allows;
use crate::cap::cap_sid_file;
use crate::cap::load_or_create_cap_sids;
use crate::cap::workspace_write_cap_sid_for_root;
use crate::cap::workspace_write_root_contains_path;
use crate::logging::debug_log;
use crate::logging::log_note;
use crate::path_normalization::canonical_path_key;
use crate::policy::SandboxPolicy;
use crate::token::convert_string_sid_to_sid;
use crate::setup::effective_write_roots_for_setup;
use crate::token::LocalSid;
use crate::token::world_sid;
use anyhow::Result;
use anyhow::anyhow;
use std::collections::HashSet;
use std::ffi::OsStr;
use std::ffi::c_void;
@@ -231,6 +233,7 @@ pub fn apply_world_writable_scan_and_denies(
&flagged,
sandbox_policy,
cwd,
env_map,
logs_base_dir,
) {
log_note(
@@ -246,6 +249,7 @@ pub fn apply_capability_denies_for_world_writable(
flagged: &[PathBuf],
sandbox_policy: &SandboxPolicy,
cwd: &Path,
env_map: &std::collections::HashMap<String, String>,
logs_base_dir: Option<&Path>,
) -> Result<()> {
if flagged.is_empty() {
@@ -255,46 +259,56 @@ pub fn apply_capability_denies_for_world_writable(
let cap_path = cap_sid_file(codex_home);
let caps = load_or_create_cap_sids(codex_home)?;
std::fs::write(&cap_path, serde_json::to_string(&caps)?)?;
let (active_sid, workspace_roots): (*mut c_void, Vec<PathBuf>) = match sandbox_policy {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
let sid = unsafe { convert_string_sid_to_sid(&caps.workspace) }
.ok_or_else(|| anyhow!("ConvertStringSidToSidW failed for workspace capability"))?;
let mut roots: Vec<PathBuf> =
vec![dunce::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf())];
for root in writable_roots {
let candidate = root.as_path();
roots.push(dunce::canonicalize(candidate).unwrap_or_else(|_| root.to_path_buf()));
}
(sid, roots)
let (active_sids, workspace_roots): (Vec<LocalSid>, Vec<PathBuf>) = match sandbox_policy {
SandboxPolicy::WorkspaceWrite { .. } => {
let roots = effective_write_roots_for_setup(
sandbox_policy,
cwd,
cwd,
env_map,
codex_home,
/*write_roots_override*/ None,
);
let active_sids = roots
.iter()
.map(|root| {
workspace_write_cap_sid_for_root(codex_home, cwd, root)
.and_then(|sid| LocalSid::from_string(&sid))
})
.collect::<Result<Vec<_>>>()?;
(active_sids, roots)
}
SandboxPolicy::ReadOnly { .. } => {
(vec![LocalSid::from_string(&caps.readonly)?], Vec::new())
}
SandboxPolicy::ReadOnly { .. } => (
unsafe { convert_string_sid_to_sid(&caps.readonly) }
.ok_or_else(|| anyhow!("ConvertStringSidToSidW failed for readonly capability"))?,
Vec::new(),
),
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
return Ok(());
}
};
for path in flagged {
if workspace_roots.iter().any(|root| path.starts_with(root)) {
if workspace_roots
.iter()
.any(|root| workspace_write_root_contains_path(root, path))
{
continue;
}
let res = unsafe { add_deny_write_ace(path, active_sid) };
match res {
Ok(true) => log_note(
&format!("AUDIT: applied capability deny ACE to {}", path.display()),
logs_base_dir,
),
Ok(false) => {}
Err(err) => log_note(
&format!(
"AUDIT: failed to apply capability deny ACE to {}: {}",
path.display(),
err
for active_sid in &active_sids {
let res = unsafe { add_deny_write_ace(path, active_sid.as_ptr()) };
match res {
Ok(true) => log_note(
&format!("AUDIT: applied capability deny ACE to {}", path.display()),
logs_base_dir,
),
logs_base_dir,
),
Ok(false) => {}
Err(err) => log_note(
&format!(
"AUDIT: failed to apply capability deny ACE to {}: {}",
path.display(),
err
),
logs_base_dir,
),
}
}
}
Ok(())
codex-rs/windows-sandbox-rs/src/bin/setup_main/win.rs
+184 -62
View File
@@ -20,7 +20,6 @@ use codex_windows_sandbox::extract_setup_failure;
use codex_windows_sandbox::hide_newly_created_users;
use codex_windows_sandbox::install_wfp_filters;
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::sandbox_bin_dir;
@@ -29,7 +28,8 @@ use codex_windows_sandbox::sandbox_secrets_dir;
use codex_windows_sandbox::string_from_sid_bytes;
use codex_windows_sandbox::sync_persistent_deny_read_acls;
use codex_windows_sandbox::to_wide;
use codex_windows_sandbox::workspace_cap_sid_for_cwd;
use codex_windows_sandbox::workspace_write_cap_sid_for_root;
use codex_windows_sandbox::workspace_write_root_overlaps_path;
use codex_windows_sandbox::write_setup_error_report;
use serde::Deserialize;
use serde::Serialize;
@@ -120,6 +120,42 @@ fn log_line(log: &mut File, msg: &str) -> Result<()> {
Ok(())
}
fn workspace_write_cap_sids_for_path(
codex_home: &Path,
command_cwd: &Path,
write_roots: &[PathBuf],
path: &Path,
) -> Result<Vec<String>> {
let mut sid_strs = Vec::new();
for root in write_roots {
if workspace_write_root_overlaps_path(root, path) {
sid_strs.push(workspace_write_cap_sid_for_root(
codex_home,
command_cwd,
root,
)?);
}
}
if sid_strs.is_empty() {
if write_roots.is_empty() {
sid_strs.push(workspace_write_cap_sid_for_root(
codex_home,
command_cwd,
command_cwd,
)?);
} else {
for root in write_roots {
sid_strs.push(workspace_write_cap_sid_for_root(
codex_home,
command_cwd,
root,
)?);
}
}
}
Ok(sid_strs)
}
fn spawn_read_acl_helper(payload: &Payload, _log: &mut File) -> Result<()> {
let mut read_payload = payload.clone();
read_payload.mode = SetupMode::ReadAclsOnly;
@@ -566,25 +602,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
let sandbox_group_sid_str =
string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
let caps = load_or_create_cap_sids(&payload.codex_home).map_err(|err| {
anyhow::Error::new(SetupFailure::new(
SetupErrorCode::HelperCapabilitySidFailed,
format!("load or create capability SIDs failed: {err}"),
))
})?;
let cap_psid = unsafe {
convert_string_sid_to_sid(&caps.workspace).ok_or_else(|| {
anyhow::Error::new(SetupFailure::new(
SetupErrorCode::HelperCapabilitySidFailed,
format!("convert capability SID {} failed", caps.workspace),
))
})?
};
let workspace_sid_str = workspace_cap_sid_for_cwd(&payload.codex_home, &payload.command_cwd)?;
let workspace_psid = unsafe {
convert_string_sid_to_sid(&workspace_sid_str)
.ok_or_else(|| anyhow::anyhow!("convert workspace capability SID failed"))?
};
let mut refresh_errors: Vec<String> = Vec::new();
if !refresh_only {
let proxy_allowlist_result = firewall::ensure_offline_proxy_allowlist(
@@ -681,10 +698,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
)?;
}
let cap_sid_str = caps.workspace;
let write_mask =
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE | FILE_DELETE_CHILD;
let mut grant_tasks: Vec<PathBuf> = Vec::new();
let mut grant_tasks: Vec<(PathBuf, String)> = Vec::new();
let mut seen_deny_paths: HashSet<PathBuf> = HashSet::new();
let mut seen_write_roots: HashSet<PathBuf> = HashSet::new();
@@ -706,16 +722,17 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
let cap_label = if is_command_cwd {
"workspace_cap"
} else {
"cap"
"root_cap"
};
let cap_psid_for_root = if is_command_cwd {
workspace_psid
} else {
cap_psid
let root_cap_sid_str =
workspace_write_cap_sid_for_root(&payload.codex_home, &payload.command_cwd, root)?;
let root_cap_psid = unsafe {
convert_string_sid_to_sid(&root_cap_sid_str)
.ok_or_else(|| anyhow::anyhow!("convert write root capability SID failed"))?
};
for (label, psid) in [
("sandbox_group", sandbox_group_psid),
(cap_label, cap_psid_for_root),
(cap_label, root_cap_psid),
] {
let has =
match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) {
@@ -741,6 +758,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
need_grant = true;
}
}
unsafe {
LocalFree(root_cap_psid as HLOCAL);
}
if need_grant {
log_line(
log,
@@ -749,19 +769,14 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
root.display()
),
)?;
grant_tasks.push(root.clone());
grant_tasks.push((root.clone(), root_cap_sid_str));
}
}
let (tx, rx) = mpsc::channel::<(PathBuf, Result<bool>)>();
std::thread::scope(|scope| {
for root in grant_tasks {
let is_command_cwd = is_command_cwd_root(&root, &canonical_command_cwd);
let sid_strings = if is_command_cwd {
vec![sandbox_group_sid_str.clone(), workspace_sid_str.clone()]
} else {
vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
};
for (root, root_cap_sid_str) in grant_tasks {
let sid_strings = vec![sandbox_group_sid_str.clone(), root_cap_sid_str];
let tx = tx.clone();
scope.spawn(move || {
// Convert SID strings to psids locally in this thread.
@@ -823,27 +838,36 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
.with_context(|| format!("failed to create deny-write path {}", path.display()))?;
}
let canonical_path = canonicalize_path(path);
let deny_psid = if canonical_path.starts_with(&canonical_command_cwd) {
workspace_psid
} else {
cap_psid
};
let deny_sid_strs = workspace_write_cap_sids_for_path(
&payload.codex_home,
&payload.command_cwd,
&payload.write_roots,
path,
)?;
for deny_sid_str in deny_sid_strs {
let deny_psid = unsafe {
convert_string_sid_to_sid(&deny_sid_str)
.ok_or_else(|| anyhow::anyhow!("convert deny capability SID failed"))?
};
match unsafe { add_deny_write_ace(path, deny_psid) } {
Ok(true) => {
log_line(
log,
&format!("applied deny ACE to protect {}", path.display()),
)?;
match unsafe { add_deny_write_ace(path, deny_psid) } {
Ok(true) => {
log_line(
log,
&format!("applied deny ACE to protect {}", path.display()),
)?;
}
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()),
)?;
}
}
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()),
)?;
unsafe {
LocalFree(deny_psid as HLOCAL);
}
}
}
@@ -924,12 +948,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
if !sandbox_group_psid.is_null() {
LocalFree(sandbox_group_psid as HLOCAL);
}
if !cap_psid.is_null() {
LocalFree(cap_psid as HLOCAL);
}
if !workspace_psid.is_null() {
LocalFree(workspace_psid as HLOCAL);
}
}
if refresh_only && !refresh_errors.is_empty() {
log_line(
@@ -946,9 +964,13 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
mod tests {
use super::Payload;
use super::SETUP_VERSION;
use super::workspace_write_cap_sids_for_path;
use codex_otel::StatsigMetricsSettings;
use codex_windows_sandbox::load_or_create_cap_sids;
use codex_windows_sandbox::workspace_write_cap_sid_for_root;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::fs;
fn payload_json() -> serde_json::Value {
json!({
@@ -986,4 +1008,104 @@ mod tests {
})
);
}
#[test]
fn deny_path_under_active_root_uses_only_matching_root_sid() {
let temp = tempfile::tempdir().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let active_root = temp.path().join("active-root");
let stale_root = temp.path().join("stale-root");
let deny_path = active_root.join("protected");
fs::create_dir_all(&codex_home).expect("create codex home");
fs::create_dir_all(&workspace).expect("create workspace");
fs::create_dir_all(&active_root).expect("create active root");
fs::create_dir_all(&stale_root).expect("create stale root");
fs::create_dir_all(&deny_path).expect("create deny path");
let stale_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &stale_root)
.expect("stale sid");
let active_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &active_root)
.expect("active sid");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
let deny_sids = workspace_write_cap_sids_for_path(
&codex_home,
&workspace,
&[workspace.clone(), active_root.clone()],
&deny_path,
)
.expect("deny sids");
assert_eq!(deny_sids, vec![active_sid]);
assert!(!deny_sids.contains(&workspace_sid));
assert!(!deny_sids.contains(&stale_sid));
assert!(!deny_sids.contains(&caps.workspace));
}
#[test]
fn deny_path_outside_active_roots_falls_back_to_all_active_root_sids() {
let temp = tempfile::tempdir().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let active_root = temp.path().join("active-root");
let stale_root = temp.path().join("stale-root");
let deny_path = temp.path().join("outside-deny");
fs::create_dir_all(&codex_home).expect("create codex home");
fs::create_dir_all(&workspace).expect("create workspace");
fs::create_dir_all(&active_root).expect("create active root");
fs::create_dir_all(&stale_root).expect("create stale root");
fs::create_dir_all(&deny_path).expect("create deny path");
let stale_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &stale_root)
.expect("stale sid");
let active_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &active_root)
.expect("active sid");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
let deny_sids = workspace_write_cap_sids_for_path(
&codex_home,
&workspace,
&[workspace.clone(), active_root.clone()],
&deny_path,
)
.expect("deny sids");
assert_eq!(deny_sids.len(), 2);
assert!(deny_sids.contains(&workspace_sid));
assert!(deny_sids.contains(&active_sid));
assert!(!deny_sids.contains(&stale_sid));
assert!(!deny_sids.contains(&caps.workspace));
}
#[test]
fn deny_path_includes_nested_active_root_sid() {
let temp = tempfile::tempdir().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let protected_dir = workspace.join(".codex");
let nested_root = protected_dir.join("nested-root");
fs::create_dir_all(&codex_home).expect("create codex home");
fs::create_dir_all(&workspace).expect("create workspace");
fs::create_dir_all(&nested_root).expect("create nested root");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let nested_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &nested_root)
.expect("nested sid");
let deny_sids = workspace_write_cap_sids_for_path(
&codex_home,
&workspace,
&[workspace.clone(), nested_root],
&protected_dir,
)
.expect("deny sids");
assert_eq!(deny_sids, vec![workspace_sid, nested_sid]);
}
}
codex-rs/windows-sandbox-rs/src/cap.rs
+77
View File
@@ -1,4 +1,5 @@
use crate::path_normalization::canonical_path_key;
use crate::path_normalization::canonicalize_path;
use anyhow::Context;
use anyhow::Result;
use rand::RngCore;
@@ -22,6 +23,13 @@ pub struct CapSids {
/// without permanently affecting other workspaces.
#[serde(default)]
pub workspace_by_cwd: HashMap<String, String>,
/// Per-write-root capability SIDs keyed by canonicalized write-root path.
///
/// These are included in a workspace-write token only when the root is
/// currently allowed, so stale ACLs from earlier extra roots do not expand
/// later workspace sandboxes.
#[serde(default)]
pub writable_root_by_path: HashMap<String, String>,
}
pub fn cap_sid_file(codex_home: &Path) -> PathBuf {
@@ -61,6 +69,7 @@ pub fn load_or_create_cap_sids(codex_home: &Path) -> Result<CapSids> {
workspace: t.to_string(),
readonly: make_random_cap_sid_string(),
workspace_by_cwd: HashMap::new(),
writable_root_by_path: HashMap::new(),
};
persist_caps(&path, &caps)?;
return Ok(caps);
@@ -70,6 +79,7 @@ pub fn load_or_create_cap_sids(codex_home: &Path) -> Result<CapSids> {
workspace: make_random_cap_sid_string(),
readonly: make_random_cap_sid_string(),
workspace_by_cwd: HashMap::new(),
writable_root_by_path: HashMap::new(),
};
persist_caps(&path, &caps)?;
Ok(caps)
@@ -89,10 +99,50 @@ pub fn workspace_cap_sid_for_cwd(codex_home: &Path, cwd: &Path) -> Result<String
Ok(sid)
}
/// Returns the capability SID for an additional writable root, creating and persisting it if missing.
pub fn writable_root_cap_sid_for_path(codex_home: &Path, root: &Path) -> Result<String> {
let path = cap_sid_file(codex_home);
let mut caps = load_or_create_cap_sids(codex_home)?;
let key = canonical_path_key(root);
if let Some(sid) = caps.writable_root_by_path.get(&key) {
return Ok(sid.clone());
}
let sid = make_random_cap_sid_string();
caps.writable_root_by_path.insert(key, sid.clone());
persist_caps(&path, &caps)?;
Ok(sid)
}
pub fn workspace_write_cap_sid_for_root(
codex_home: &Path,
cwd: &Path,
root: &Path,
) -> Result<String> {
if canonical_path_key(root) == canonical_path_key(cwd) {
workspace_cap_sid_for_cwd(codex_home, cwd)
} else {
writable_root_cap_sid_for_path(codex_home, root)
}
}
pub fn workspace_write_root_contains_path(root: &Path, path: &Path) -> bool {
canonicalize_path(path).starts_with(canonicalize_path(root))
}
pub fn workspace_write_root_overlaps_path(root: &Path, path: &Path) -> bool {
workspace_write_root_contains_path(root, path) || workspace_write_root_contains_path(path, root)
}
pub fn workspace_write_root_specificity(root: &Path) -> usize {
canonicalize_path(root).components().count()
}
#[cfg(test)]
mod tests {
use super::load_or_create_cap_sids;
use super::workspace_cap_sid_for_cwd;
use super::workspace_write_cap_sid_for_root;
use super::writable_root_cap_sid_for_path;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
@@ -123,4 +173,31 @@ mod tests {
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
assert_eq!(caps.workspace_by_cwd.len(), 1);
}
#[test]
fn write_roots_get_path_scoped_sids() {
let temp = tempfile::tempdir().expect("tempdir");
let codex_home = temp.path().join("codex-home");
std::fs::create_dir_all(&codex_home).expect("create codex home");
let workspace = temp.path().join("workspace");
let extra_root = temp.path().join("extra-root");
std::fs::create_dir_all(&workspace).expect("create workspace");
std::fs::create_dir_all(&extra_root).expect("create extra root");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let extra_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &extra_root)
.expect("extra root sid");
assert_ne!(workspace_sid, extra_sid);
assert_eq!(
extra_sid,
writable_root_cap_sid_for_path(&codex_home, &extra_root).expect("extra root sid again")
);
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
assert_eq!(caps.workspace_by_cwd.len(), 1);
assert_eq!(caps.writable_root_by_path.len(), 1);
}
}
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
+23 -15
View File
@@ -24,6 +24,7 @@ mod windows_impl {
use super::ElevatedSandboxCaptureRequest;
use crate::acl::allow_null_device;
use crate::cap::load_or_create_cap_sids;
use crate::cap::workspace_write_cap_sid_for_root;
use crate::env::ensure_non_interactive_pager;
use crate::env::inherit_path_env;
use crate::env::normalize_null_device_env;
@@ -41,7 +42,8 @@ mod windows_impl {
use crate::runner_client::spawn_runner_transport;
use crate::sandbox_utils::ensure_codex_home_exists;
use crate::sandbox_utils::inject_git_safe_directory;
use crate::token::convert_string_sid_to_sid;
use crate::setup::effective_write_roots_for_setup;
use crate::token::LocalSid;
use anyhow::Result;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Path;
@@ -109,22 +111,28 @@ mod windows_impl {
anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing")
}
let caps = load_or_create_cap_sids(codex_home)?;
let (psid_to_use, cap_sids) = match &policy {
let (sid_for_null, cap_sids) = match &policy {
SandboxPolicy::ReadOnly { .. } => {
#[allow(clippy::unwrap_used)]
let psid = unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() };
(psid, vec![caps.readonly])
let sid = LocalSid::from_string(&caps.readonly)?;
(sid, vec![caps.readonly])
}
SandboxPolicy::WorkspaceWrite { .. } => {
#[allow(clippy::unwrap_used)]
let psid = unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() };
(
psid,
vec![
caps.workspace,
crate::cap::workspace_cap_sid_for_cwd(codex_home, cwd)?,
],
)
let write_roots = effective_write_roots_for_setup(
&policy,
sandbox_policy_cwd,
cwd,
&env_map,
codex_home,
write_roots_override,
);
let cap_sids = write_roots
.iter()
.map(|root| workspace_write_cap_sid_for_root(codex_home, cwd, root))
.collect::<Result<Vec<_>>>()?;
if cap_sids.is_empty() {
anyhow::bail!("workspace-write sandbox has no writable root capability SIDs");
}
(LocalSid::from_string(&cap_sids[0])?, cap_sids)
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("DangerFullAccess handled above")
@@ -132,7 +140,7 @@ mod windows_impl {
};
unsafe {
allow_null_device(psid_to_use);
allow_null_device(sid_for_null.as_ptr());
}
(|| -> Result<CaptureResult> {
codex-rs/windows-sandbox-rs/src/lib.rs
+78 -174
View File
@@ -110,6 +110,12 @@ pub use cap::load_or_create_cap_sids;
#[cfg(target_os = "windows")]
pub use cap::workspace_cap_sid_for_cwd;
#[cfg(target_os = "windows")]
pub use cap::workspace_write_cap_sid_for_root;
#[cfg(target_os = "windows")]
pub use cap::workspace_write_root_contains_path;
#[cfg(target_os = "windows")]
pub use cap::workspace_write_root_overlaps_path;
#[cfg(target_os = "windows")]
pub use conpty::ConptyInstance;
#[cfg(target_os = "windows")]
pub use conpty::spawn_conpty_process_as_user;
@@ -222,7 +228,7 @@ pub use setup_error::setup_error_path;
pub use setup_error::write_setup_error_report;
#[cfg(target_os = "windows")]
#[doc(hidden)]
pub use spawn_prep::LocalSid;
pub use token::LocalSid;
#[cfg(target_os = "windows")]
pub use token::convert_string_sid_to_sid;
#[cfg(target_os = "windows")]
@@ -273,34 +279,25 @@ pub use stub::run_windows_sandbox_legacy_preflight;
#[cfg(target_os = "windows")]
mod windows_impl {
use super::acl::add_allow_ace;
use super::acl::add_deny_write_ace;
use super::acl::allow_null_device;
use super::acl::revoke_ace;
use super::allow::AllowDenyPaths;
use super::allow::compute_allow_paths;
use super::cap::load_or_create_cap_sids;
use super::cap::workspace_cap_sid_for_cwd;
use super::deny_read_acl::apply_deny_read_acls;
use super::deny_read_state::sync_persistent_deny_read_acls;
use super::logging::log_failure;
use super::logging::log_success;
use super::path_normalization::canonicalize_path;
use super::policy::SandboxPolicy;
use super::process::create_process_as_user;
use super::sandbox_utils::ensure_codex_home_exists;
use super::spawn_prep::LegacyAclSids;
use super::spawn_prep::allow_null_device_for_workspace_write;
use super::spawn_prep::apply_legacy_session_acl_rules;
use super::spawn_prep::legacy_session_capability_roots;
use super::spawn_prep::prepare_legacy_session_security;
use super::spawn_prep::prepare_legacy_spawn_context;
use super::token::convert_string_sid_to_sid;
use super::token::create_workspace_write_token_with_caps_from;
use super::workspace_acl::is_command_cwd_root;
use anyhow::Context;
use super::spawn_prep::root_capability_sids;
use super::token::LocalSid;
use anyhow::Result;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashMap;
use std::ffi::c_void;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use std::ptr;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::GetLastError;
@@ -418,130 +415,36 @@ mod windows_impl {
if !additional_deny_read_paths.is_empty() {
anyhow::bail!("deny-read overrides require the elevated Windows sandbox backend");
}
let caps = load_or_create_cap_sids(codex_home)?;
let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe {
match &policy {
SandboxPolicy::ReadOnly { .. } => {
#[allow(clippy::expect_used)]
let psid =
convert_string_sid_to_sid(&caps.readonly).expect("valid readonly SID");
let (h, _) = super::token::create_readonly_token_with_cap(psid)?;
(h, psid, None)
}
SandboxPolicy::WorkspaceWrite { .. } => {
#[allow(clippy::expect_used)]
let psid_generic =
convert_string_sid_to_sid(&caps.workspace).expect("valid workspace SID");
let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
#[allow(clippy::expect_used)]
let psid_workspace =
convert_string_sid_to_sid(&ws_sid).expect("valid workspace SID");
let base = super::token::get_current_token_for_restriction()?;
let h_res = create_workspace_write_token_with_caps_from(
base,
&[psid_generic, psid_workspace],
);
windows_sys::Win32::Foundation::CloseHandle(base);
let h = h_res?;
(h, psid_generic, Some(psid_workspace))
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("DangerFullAccess handled above")
}
}
};
unsafe {
if is_workspace_write
&& let Ok(base) = super::token::get_current_token_for_restriction()
{
if let Ok(bytes) = super::token::get_logon_sid_bytes(base) {
let mut tmp = bytes;
let psid2 = tmp.as_mut_ptr() as *mut c_void;
allow_null_device(psid2);
}
windows_sys::Win32::Foundation::CloseHandle(base);
}
}
let capability_roots = legacy_session_capability_roots(
&policy,
sandbox_policy_cwd,
&current_dir,
&env_map,
codex_home,
);
let security = prepare_legacy_session_security(&policy, codex_home, cwd, capability_roots)?;
allow_null_device_for_workspace_write(is_workspace_write);
let persist_aces = is_workspace_write;
let AllowDenyPaths { allow, mut deny } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
for path in additional_deny_write_paths {
// Explicit deny-write carveouts must already exist when the process
// starts, otherwise it could create a missing path under a writable
// parent before the deny-write ACE exists.
if !path.exists() {
std::fs::create_dir_all(&path)
.with_context(|| format!("create deny-write path {}", path.display()))?;
}
deny.insert(path.clone());
}
let canonical_cwd = canonicalize_path(&current_dir);
let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new();
unsafe {
for p in &allow {
let psid = if is_workspace_write && is_command_cwd_root(p, &canonical_cwd) {
psid_workspace.unwrap_or(psid_generic)
} else {
psid_generic
};
if let Ok(added) = add_allow_ace(p, psid)
&& added
{
if persist_aces {
if p.is_dir() {
// best-effort seeding omitted intentionally
}
} else {
guards.push((p.clone(), psid));
}
}
}
for p in &deny {
if let Ok(added) = add_deny_write_ace(p, psid_generic)
&& added
&& !persist_aces
{
guards.push((p.clone(), psid_generic));
}
}
// Read denies are layered after allow/deny-write setup so they can
// override broad read grants for the sandbox principal without
// changing the existing write policy computation.
let applied_deny_read_paths = match if persist_aces {
sync_persistent_deny_read_acls(
codex_home,
&caps.workspace,
&additional_deny_read_paths,
psid_generic,
)
} else {
apply_deny_read_acls(&additional_deny_read_paths, psid_generic)
} {
Ok(paths) => paths,
Err(err) => {
if !persist_aces {
cleanup_acl_guards(&mut guards);
}
return Err(err);
}
};
if !persist_aces {
for path in applied_deny_read_paths {
guards.push((path, psid_generic));
}
}
allow_null_device(psid_generic);
if let Some(psid) = psid_workspace {
allow_null_device(psid);
}
}
let guards = apply_legacy_session_acl_rules(
&policy,
sandbox_policy_cwd,
codex_home,
&current_dir,
&env_map,
&additional_deny_read_paths,
&additional_deny_write_paths,
LegacyAclSids {
readonly_sid: security.readonly_sid.as_ref(),
readonly_sid_str: security.readonly_sid_str.as_deref(),
write_root_sids: &security.write_root_sids,
},
persist_aces,
)?;
let (stdin_pair, stdout_pair, stderr_pair) = unsafe { setup_stdio_pipes()? };
let ((in_r, in_w), (out_r, out_w), (err_r, err_w)) = (stdin_pair, stdout_pair, stderr_pair);
let spawn_res = unsafe {
create_process_as_user(
h_token,
security.h_token,
&command,
cwd,
&env_map,
@@ -553,7 +456,6 @@ mod windows_impl {
let created = match spawn_res {
Ok(v) => v,
Err(err) => {
cleanup_acl_guards(&mut guards);
unsafe {
CloseHandle(in_r);
CloseHandle(in_w);
@@ -561,7 +463,14 @@ mod windows_impl {
CloseHandle(out_w);
CloseHandle(err_r);
CloseHandle(err_w);
CloseHandle(h_token);
if !persist_aces {
for (p, sid_str) in &guards {
if let Ok(sid) = LocalSid::from_string(sid_str) {
revoke_ace(p, sid.as_ptr());
}
}
}
CloseHandle(security.h_token);
}
return Err(err);
}
@@ -643,7 +552,7 @@ mod windows_impl {
if pi.hProcess != 0 {
CloseHandle(pi.hProcess);
}
CloseHandle(h_token);
CloseHandle(security.h_token);
}
let _ = t_out.join();
let _ = t_err.join();
@@ -662,7 +571,13 @@ mod windows_impl {
}
if !persist_aces {
cleanup_acl_guards(&mut guards);
unsafe {
for (p, sid_str) in guards {
if let Ok(sid) = LocalSid::from_string(&sid_str) {
revoke_ace(&p, sid.as_ptr());
}
}
}
}
Ok(CaptureResult {
exit_code,
@@ -672,14 +587,6 @@ mod windows_impl {
})
}
fn cleanup_acl_guards(guards: &mut Vec<(PathBuf, *mut c_void)>) {
unsafe {
for (p, sid) in guards.drain(..) {
revoke_ace(&p, sid);
}
}
}
pub fn run_windows_sandbox_legacy_preflight(
sandbox_policy: &SandboxPolicy,
sandbox_policy_cwd: &Path,
@@ -693,33 +600,30 @@ mod windows_impl {
}
ensure_codex_home_exists(codex_home)?;
let caps = load_or_create_cap_sids(codex_home)?;
#[allow(clippy::expect_used)]
let psid_generic =
unsafe { convert_string_sid_to_sid(&caps.workspace) }.expect("valid workspace SID");
let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
#[allow(clippy::expect_used)]
let psid_workspace =
unsafe { convert_string_sid_to_sid(&ws_sid) }.expect("valid workspace SID");
let current_dir = cwd.to_path_buf();
let AllowDenyPaths { allow, deny } =
compute_allow_paths(sandbox_policy, sandbox_policy_cwd, &current_dir, env_map);
let canonical_cwd = canonicalize_path(&current_dir);
unsafe {
for p in &allow {
let psid = if is_command_cwd_root(p, &canonical_cwd) {
psid_workspace
} else {
psid_generic
};
let _ = add_allow_ace(p, psid);
}
for p in &deny {
let _ = add_deny_write_ace(p, psid_generic);
}
allow_null_device(psid_generic);
allow_null_device(psid_workspace);
}
let capability_roots = legacy_session_capability_roots(
sandbox_policy,
sandbox_policy_cwd,
&current_dir,
env_map,
codex_home,
);
let write_root_sids = root_capability_sids(codex_home, cwd, capability_roots)?;
let _guards = apply_legacy_session_acl_rules(
sandbox_policy,
sandbox_policy_cwd,
codex_home,
&current_dir,
env_map,
&[],
&[],
LegacyAclSids {
readonly_sid: None,
readonly_sid_str: None,
write_root_sids: &write_root_sids,
},
/*persist_aces*/ true,
)?;
Ok(())
}
codex-rs/windows-sandbox-rs/src/setup.rs
+88 -15
View File
@@ -413,6 +413,26 @@ pub(crate) fn gather_write_roots(
out
}
pub(crate) fn effective_write_roots_for_setup(
policy: &SandboxPolicy,
policy_cwd: &Path,
command_cwd: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
write_roots_override: Option<&[PathBuf]>,
) -> Vec<PathBuf> {
let write_roots = if let Some(roots) = write_roots_override {
canonical_existing(roots)
} else {
gather_write_roots(policy, policy_cwd, command_cwd, env_map)
};
let write_roots = expand_user_profile_root(write_roots);
let write_roots = filter_user_profile_root(write_roots);
let write_roots = filter_user_profile_root_exclusions(write_roots);
let write_roots = filter_ssh_config_dependency_roots(write_roots);
filter_sensitive_write_roots(write_roots, codex_home)
}
#[derive(Serialize)]
struct ElevationPayload {
version: u32,
@@ -761,21 +781,14 @@ fn build_payload_roots(
request: &SandboxSetupRequest<'_>,
overrides: &SetupRootOverrides,
) -> (Vec<PathBuf>, Vec<PathBuf>) {
let write_roots = if let Some(roots) = overrides.write_roots.as_deref() {
canonical_existing(roots)
} else {
gather_write_roots(
request.policy,
request.policy_cwd,
request.command_cwd,
request.env_map,
)
};
let write_roots = expand_user_profile_root(write_roots);
let write_roots = filter_user_profile_root(write_roots);
let write_roots = filter_user_profile_root_exclusions(write_roots);
let write_roots = filter_ssh_config_dependency_roots(write_roots);
let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home);
let write_roots = effective_write_roots_for_setup(
request.policy,
request.policy_cwd,
request.command_cwd,
request.env_map,
request.codex_home,
overrides.write_roots.as_deref(),
);
let mut read_roots = if let Some(roots) = overrides.read_roots.as_deref() {
// An explicit override is the split policy's complete readable set. Keep only the
// helper/platform roots the elevated setup needs; do not re-add legacy cwd/full-read roots.
@@ -1405,6 +1418,66 @@ mod tests {
);
}
#[test]
fn effective_write_roots_match_payload_filtering_for_overrides() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let command_cwd = tmp.path().join("workspace");
let extra_root = tmp.path().join("extra-root");
let sandbox_root = super::sandbox_dir(&codex_home);
fs::create_dir_all(&codex_home).expect("create codex home");
fs::create_dir_all(&command_cwd).expect("create workspace");
fs::create_dir_all(&extra_root).expect("create extra root");
fs::create_dir_all(&sandbox_root).expect("create sandbox root");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let override_roots = vec![
command_cwd.clone(),
extra_root.clone(),
codex_home.clone(),
sandbox_root.clone(),
];
let request = super::SandboxSetupRequest {
policy: &policy,
policy_cwd: &command_cwd,
command_cwd: &command_cwd,
env_map: &HashMap::new(),
codex_home: &codex_home,
proxy_enforced: false,
};
let overrides = super::SetupRootOverrides {
read_roots: None,
read_roots_include_platform_defaults: false,
write_roots: Some(override_roots.clone()),
deny_read_paths: None,
deny_write_paths: None,
};
let effective_write_roots = super::effective_write_roots_for_setup(
&policy,
&command_cwd,
&command_cwd,
&HashMap::new(),
&codex_home,
Some(&override_roots),
);
let (_read_roots, payload_write_roots) = build_payload_roots(&request, &overrides);
let expected_workspace = dunce::canonicalize(&command_cwd).expect("canonical workspace");
let expected_extra = dunce::canonicalize(&extra_root).expect("canonical extra root");
let forbidden_codex_home = dunce::canonicalize(&codex_home).expect("canonical codex home");
let forbidden_sandbox = dunce::canonicalize(&sandbox_root).expect("canonical sandbox root");
assert_eq!(effective_write_roots, payload_write_roots);
assert!(effective_write_roots.contains(&expected_workspace));
assert!(effective_write_roots.contains(&expected_extra));
assert!(!effective_write_roots.contains(&forbidden_codex_home));
assert!(!effective_write_roots.contains(&forbidden_sandbox));
}
#[test]
fn payload_deny_write_paths_merge_explicit_and_protected_children() {
let tmp = TempDir::new().expect("tempdir");
codex-rs/windows-sandbox-rs/src/spawn_prep.rs
+330 -88
View File
@@ -4,7 +4,10 @@ use crate::acl::allow_null_device;
use crate::allow::AllowDenyPaths;
use crate::allow::compute_allow_paths;
use crate::cap::load_or_create_cap_sids;
use crate::cap::workspace_cap_sid_for_cwd;
use crate::cap::workspace_write_cap_sid_for_root;
use crate::cap::workspace_write_root_contains_path;
use crate::cap::workspace_write_root_overlaps_path;
use crate::cap::workspace_write_root_specificity;
use crate::deny_read_acl::apply_deny_read_acls;
use crate::deny_read_state::sync_persistent_deny_read_acls;
use crate::env::apply_no_network_to_env;
@@ -19,7 +22,8 @@ use crate::policy::SandboxPolicy;
use crate::policy::parse_policy;
use crate::sandbox_utils::ensure_codex_home_exists;
use crate::sandbox_utils::inject_git_safe_directory;
use crate::token::convert_string_sid_to_sid;
use crate::setup::effective_write_roots_for_setup;
use crate::token::LocalSid;
use crate::token::create_readonly_token_with_cap;
use crate::token::create_workspace_write_token_with_caps_from;
use crate::token::get_current_token_for_restriction;
@@ -35,8 +39,6 @@ use std::path::Path;
use std::path::PathBuf;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::HANDLE;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::LocalFree;
pub(crate) struct SpawnContext {
pub(crate) policy: SandboxPolicy,
@@ -54,36 +56,21 @@ pub(crate) struct ElevatedSpawnContext {
pub(crate) struct LegacySessionSecurity {
pub(crate) h_token: HANDLE,
pub(crate) psid_generic: LocalSid,
pub(crate) psid_workspace: Option<LocalSid>,
pub(crate) cap_sid_str: String,
pub(crate) readonly_sid: Option<LocalSid>,
pub(crate) readonly_sid_str: Option<String>,
pub(crate) write_root_sids: Vec<RootCapabilitySid>,
}
/// Owns a SID allocated by `ConvertStringSidToSidW` and releases it with `LocalFree`.
pub struct LocalSid {
psid: *mut c_void,
pub(crate) struct RootCapabilitySid {
pub(crate) root: PathBuf,
pub(crate) sid: LocalSid,
pub(crate) sid_str: String,
}
impl LocalSid {
pub fn from_string(sid: &str) -> Result<Self> {
let psid = unsafe { convert_string_sid_to_sid(sid) }
.ok_or_else(|| anyhow::anyhow!("invalid SID string: {sid}"))?;
Ok(Self { psid })
}
pub fn as_ptr(&self) -> *mut c_void {
self.psid
}
}
impl Drop for LocalSid {
fn drop(&mut self) {
if !self.psid.is_null() {
unsafe {
LocalFree(self.psid as HLOCAL);
}
}
}
pub(crate) struct LegacyAclSids<'a> {
pub(crate) readonly_sid: Option<&'a LocalSid>,
pub(crate) readonly_sid_str: Option<&'a str>,
pub(crate) write_root_sids: &'a [RootCapabilitySid],
}
pub(crate) fn should_apply_network_block(policy: &SandboxPolicy) -> bool {
@@ -161,27 +148,31 @@ pub(crate) fn prepare_legacy_session_security(
policy: &SandboxPolicy,
codex_home: &Path,
cwd: &Path,
capability_roots: impl IntoIterator<Item = PathBuf>,
) -> Result<LegacySessionSecurity> {
let caps = load_or_create_cap_sids(codex_home)?;
let (h_token, psid_generic, psid_workspace, cap_sid_str) = unsafe {
let (h_token, readonly_sid, readonly_sid_str, write_root_sids) = unsafe {
match policy {
SandboxPolicy::ReadOnly { .. } => {
let psid = LocalSid::from_string(&caps.readonly)?;
let (h_token, _psid) = create_readonly_token_with_cap(psid.as_ptr())?;
(h_token, psid, None, caps.readonly)
(h_token, Some(psid), Some(caps.readonly), Vec::new())
}
SandboxPolicy::WorkspaceWrite { .. } => {
let psid_generic = LocalSid::from_string(&caps.workspace)?;
let workspace_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
let psid_workspace = LocalSid::from_string(&workspace_sid)?;
let write_root_sids = root_capability_sids(codex_home, cwd, capability_roots)?;
if write_root_sids.is_empty() {
anyhow::bail!("workspace-write sandbox has no writable root capability SIDs");
}
let base = get_current_token_for_restriction()?;
let h_token = create_workspace_write_token_with_caps_from(
base,
&[psid_generic.as_ptr(), psid_workspace.as_ptr()],
);
let cap_ptrs: Vec<*mut c_void> = write_root_sids
.iter()
.map(|root| root.sid.as_ptr())
.collect();
let h_token =
create_workspace_write_token_with_caps_from(base, cap_ptrs.as_slice());
CloseHandle(base);
let h_token = h_token?;
(h_token, psid_generic, Some(psid_workspace), caps.workspace)
(h_token, None, None, write_root_sids)
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("dangerous policies rejected before legacy session prep")
@@ -191,12 +182,80 @@ pub(crate) fn prepare_legacy_session_security(
Ok(LegacySessionSecurity {
h_token,
psid_generic,
psid_workspace,
cap_sid_str,
readonly_sid,
readonly_sid_str,
write_root_sids,
})
}
pub(crate) fn legacy_session_capability_roots(
policy: &SandboxPolicy,
policy_cwd: &Path,
current_dir: &Path,
env_map: &HashMap<String, String>,
codex_home: &Path,
) -> Vec<PathBuf> {
let allow_paths = compute_allow_paths(policy, policy_cwd, current_dir, env_map)
.allow
.into_iter()
.collect::<Vec<_>>();
if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) {
effective_write_roots_for_setup(
policy,
policy_cwd,
current_dir,
env_map,
codex_home,
Some(allow_paths.as_slice()),
)
} else {
allow_paths
}
}
pub(crate) fn root_capability_sids(
codex_home: &Path,
cwd: &Path,
allow_paths: impl IntoIterator<Item = PathBuf>,
) -> Result<Vec<RootCapabilitySid>> {
let mut roots: Vec<PathBuf> = allow_paths.into_iter().collect();
roots.sort_by_key(|root| canonicalize_path(root.as_path()));
roots.dedup_by(|a, b| canonicalize_path(a.as_path()) == canonicalize_path(b.as_path()));
let mut out = Vec::with_capacity(roots.len());
for root in roots {
let sid_str = workspace_write_cap_sid_for_root(codex_home, cwd, &root)?;
let sid = LocalSid::from_string(&sid_str)?;
out.push(RootCapabilitySid { root, sid, sid_str });
}
Ok(out)
}
fn matching_root_capability<'a>(
path: &Path,
root_sids: &'a [RootCapabilitySid],
) -> Option<&'a RootCapabilitySid> {
root_sids
.iter()
.filter(|root_sid| workspace_write_root_contains_path(&root_sid.root, path))
.max_by_key(|root_sid| workspace_write_root_specificity(&root_sid.root))
}
fn deny_root_capabilities_for_path<'a>(
path: &Path,
root_sids: &'a [RootCapabilitySid],
) -> Vec<&'a RootCapabilitySid> {
let matching_root_sids = root_sids
.iter()
.filter(|root_sid| workspace_write_root_overlaps_path(&root_sid.root, path))
.collect::<Vec<_>>();
if matching_root_sids.is_empty() {
root_sids.iter().collect()
} else {
matching_root_sids
}
}
pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) {
if !is_workspace_write {
return;
@@ -221,17 +280,14 @@ pub(crate) fn apply_legacy_session_acl_rules(
codex_home: &Path,
current_dir: &Path,
env_map: &HashMap<String, String>,
psid_generic: &LocalSid,
psid_workspace: Option<&LocalSid>,
cap_sid_str: &str,
additional_deny_read_paths: &[PathBuf],
additional_deny_write_paths: &[PathBuf],
acl_sids: LegacyAclSids<'_>,
persist_aces: bool,
) -> Result<Vec<PathBuf>> {
) -> Result<Vec<(PathBuf, String)>> {
let AllowDenyPaths { allow, mut deny } =
compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map);
let mut guards: Vec<PathBuf> = Vec::new();
let canonical_cwd = canonicalize_path(current_dir);
let mut guards: Vec<(PathBuf, String)> = Vec::new();
unsafe {
for path in additional_deny_write_paths {
// Explicit carveouts must exist before the command starts so the
@@ -242,45 +298,94 @@ pub(crate) fn apply_legacy_session_acl_rules(
}
deny.insert(path.clone());
}
for p in &allow {
let psid = if matches!(policy, SandboxPolicy::WorkspaceWrite { .. })
&& is_command_cwd_root(p, &canonical_cwd)
{
psid_workspace.unwrap_or(psid_generic).as_ptr()
} else {
psid_generic.as_ptr()
};
if matches!(add_allow_ace(p, psid), Ok(true)) && !persist_aces {
guards.push(p.clone());
if let Some(readonly_sid) = acl_sids.readonly_sid {
for p in &allow {
if matches!(add_allow_ace(p, readonly_sid.as_ptr()), Ok(true))
&& !persist_aces
&& let Some(readonly_sid_str) = acl_sids.readonly_sid_str
{
guards.push((p.clone(), readonly_sid_str.to_string()));
}
}
} else {
for p in &allow {
let Some(root_sid) = matching_root_capability(p, acl_sids.write_root_sids) else {
continue;
};
if matches!(add_allow_ace(p, root_sid.sid.as_ptr()), Ok(true)) && !persist_aces {
guards.push((p.clone(), root_sid.sid_str.clone()));
}
}
}
for p in &deny {
if let Ok(added) = add_deny_write_ace(p, psid_generic.as_ptr())
&& added
&& !persist_aces
{
guards.push(p.clone());
for root_sid in deny_root_capabilities_for_path(p, acl_sids.write_root_sids) {
if let Ok(added) = add_deny_write_ace(p, root_sid.sid.as_ptr())
&& added
&& !persist_aces
{
guards.push((p.clone(), root_sid.sid_str.clone()));
}
}
}
let applied_deny_read_paths = if persist_aces {
sync_persistent_deny_read_acls(
codex_home,
cap_sid_str,
additional_deny_read_paths,
psid_generic.as_ptr(),
)?
} else {
apply_deny_read_acls(additional_deny_read_paths, psid_generic.as_ptr())?
};
if !persist_aces {
guards.extend(applied_deny_read_paths);
if !additional_deny_read_paths.is_empty() {
if let Some(readonly_sid) = acl_sids.readonly_sid {
let Some(readonly_sid_str) = acl_sids.readonly_sid_str else {
anyhow::bail!("readonly capability SID string missing");
};
let applied_deny_read_paths = if persist_aces {
sync_persistent_deny_read_acls(
codex_home,
readonly_sid_str,
additional_deny_read_paths,
readonly_sid.as_ptr(),
)?
} else {
apply_deny_read_acls(additional_deny_read_paths, readonly_sid.as_ptr())?
};
if !persist_aces {
guards.extend(
applied_deny_read_paths
.into_iter()
.map(|path| (path, readonly_sid_str.to_string())),
);
}
} else {
for root_sid in acl_sids.write_root_sids {
let applied_deny_read_paths = if persist_aces {
sync_persistent_deny_read_acls(
codex_home,
&root_sid.sid_str,
additional_deny_read_paths,
root_sid.sid.as_ptr(),
)?
} else {
apply_deny_read_acls(additional_deny_read_paths, root_sid.sid.as_ptr())?
};
if !persist_aces {
guards.extend(
applied_deny_read_paths
.into_iter()
.map(|path| (path, root_sid.sid_str.clone())),
);
}
}
}
}
allow_null_device(psid_generic.as_ptr());
if let Some(psid_workspace) = psid_workspace {
allow_null_device(psid_workspace.as_ptr());
if persist_aces && matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) {
let _ = protect_workspace_codex_dir(current_dir, psid_workspace.as_ptr());
let _ = protect_workspace_agents_dir(current_dir, psid_workspace.as_ptr());
for root_sid in acl_sids.write_root_sids {
allow_null_device(root_sid.sid.as_ptr());
}
if let Some(readonly_sid) = acl_sids.readonly_sid {
allow_null_device(readonly_sid.as_ptr());
}
if persist_aces
&& matches!(policy, SandboxPolicy::WorkspaceWrite { .. })
&& let Some(workspace_sid) =
matching_root_capability(current_dir, acl_sids.write_root_sids)
{
let canonical_cwd = canonicalize_path(current_dir);
if is_command_cwd_root(&workspace_sid.root, &canonical_cwd) {
let _ = protect_workspace_codex_dir(current_dir, workspace_sid.sid.as_ptr());
let _ = protect_workspace_agents_dir(current_dir, workspace_sid.sid.as_ptr());
}
}
}
@@ -324,6 +429,24 @@ pub(crate) fn prepare_elevated_spawn_context(
} else {
None
};
let write_roots_for_setup = write_roots_override.or(computed_write_roots_override);
let effective_write_roots = if common.is_workspace_write {
effective_write_roots_for_setup(
&common.policy,
sandbox_policy_cwd,
&common.current_dir,
env_map,
codex_home,
write_roots_for_setup,
)
} else {
Vec::new()
};
let setup_write_roots_override = if common.is_workspace_write {
Some(effective_write_roots.as_slice())
} else {
write_roots_override
};
let sandbox_creds = require_logon_sandbox_creds(
&common.policy,
sandbox_policy_cwd,
@@ -332,7 +455,7 @@ pub(crate) fn prepare_elevated_spawn_context(
codex_home,
read_roots_override,
read_roots_include_platform_defaults,
write_roots_override.or(computed_write_roots_override),
setup_write_roots_override,
deny_read_paths_override,
if deny_write_paths_override.is_empty() {
&deny_write_paths
@@ -348,11 +471,14 @@ pub(crate) fn prepare_elevated_spawn_context(
vec![caps.readonly.clone()],
),
SandboxPolicy::WorkspaceWrite { .. } => {
let cap_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
(
LocalSid::from_string(&caps.workspace)?,
vec![caps.workspace.clone(), cap_sid],
)
let cap_sids = root_capability_sids(codex_home, cwd, effective_write_roots)?
.into_iter()
.map(|root_sid| root_sid.sid_str)
.collect::<Vec<_>>();
if cap_sids.is_empty() {
anyhow::bail!("workspace-write sandbox has no writable root capability SIDs");
}
(LocalSid::from_string(&cap_sids[0])?, cap_sids)
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("dangerous policies rejected before elevated session prep")
@@ -373,9 +499,15 @@ pub(crate) fn prepare_elevated_spawn_context(
#[cfg(test)]
mod tests {
use super::SandboxPolicy;
use super::deny_root_capabilities_for_path;
use super::legacy_session_capability_roots;
use super::prepare_legacy_spawn_context;
use super::prepare_spawn_context_common;
use super::root_capability_sids;
use super::should_apply_network_block;
use crate::cap::load_or_create_cap_sids;
use crate::cap::workspace_write_cap_sid_for_root;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use tempfile::TempDir;
@@ -450,4 +582,114 @@ mod tests {
Some(&"http://user.proxy:8080".to_string())
);
}
#[test]
fn root_capability_sids_only_include_active_roots() {
let temp = TempDir::new().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let active_root = temp.path().join("active-root");
let stale_root = temp.path().join("stale-root");
std::fs::create_dir_all(&codex_home).expect("create codex home");
std::fs::create_dir_all(&workspace).expect("create workspace");
std::fs::create_dir_all(&active_root).expect("create active root");
std::fs::create_dir_all(&stale_root).expect("create stale root");
let stale_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &stale_root)
.expect("stale sid");
let active_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &active_root)
.expect("active sid");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
let sid_strs = root_capability_sids(
&codex_home,
&workspace,
vec![workspace.clone(), active_root],
)
.expect("root capabilities")
.into_iter()
.map(|root_sid| root_sid.sid_str)
.collect::<Vec<_>>();
assert_eq!(sid_strs.len(), 2);
assert!(sid_strs.contains(&workspace_sid));
assert!(sid_strs.contains(&active_sid));
assert!(!sid_strs.contains(&stale_sid));
assert!(!sid_strs.contains(&caps.workspace));
}
#[test]
fn legacy_deny_path_includes_nested_active_root_sid() {
let temp = TempDir::new().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let protected_dir = workspace.join(".codex");
let nested_root = protected_dir.join("nested-root");
let unrelated_root = temp.path().join("unrelated-root");
std::fs::create_dir_all(&codex_home).expect("create codex home");
std::fs::create_dir_all(&workspace).expect("create workspace");
std::fs::create_dir_all(&nested_root).expect("create nested root");
std::fs::create_dir_all(&unrelated_root).expect("create unrelated root");
let workspace_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &workspace)
.expect("workspace sid");
let nested_sid = workspace_write_cap_sid_for_root(&codex_home, &workspace, &nested_root)
.expect("nested sid");
let unrelated_sid =
workspace_write_cap_sid_for_root(&codex_home, &workspace, &unrelated_root)
.expect("unrelated sid");
let root_sids = root_capability_sids(
&codex_home,
&workspace,
vec![workspace.clone(), nested_root, unrelated_root],
)
.expect("root capabilities");
let deny_sid_strs = deny_root_capabilities_for_path(&protected_dir, &root_sids)
.into_iter()
.map(|root_sid| root_sid.sid_str.clone())
.collect::<Vec<_>>();
assert_eq!(deny_sid_strs, vec![workspace_sid, nested_sid]);
assert!(!deny_sid_strs.contains(&unrelated_sid));
}
#[test]
fn legacy_capability_roots_use_effective_write_roots() {
let temp = TempDir::new().expect("tempdir");
let codex_home = temp.path().join("codex-home");
let workspace = temp.path().join("workspace");
let active_root = temp.path().join("active-root");
let sandbox_root = codex_home.join(".sandbox");
std::fs::create_dir_all(&codex_home).expect("create codex home");
std::fs::create_dir_all(&workspace).expect("create workspace");
std::fs::create_dir_all(&active_root).expect("create active root");
std::fs::create_dir_all(&sandbox_root).expect("create sandbox root");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![
AbsolutePathBuf::try_from(active_root.as_path()).expect("active root"),
AbsolutePathBuf::try_from(codex_home.as_path()).expect("codex home"),
AbsolutePathBuf::try_from(sandbox_root.as_path()).expect("sandbox root"),
],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let roots = legacy_session_capability_roots(
&policy,
&workspace,
&workspace,
&HashMap::new(),
&codex_home,
);
assert!(roots.contains(&dunce::canonicalize(&workspace).expect("workspace")));
assert!(roots.contains(&dunce::canonicalize(&active_root).expect("active root")));
assert!(!roots.contains(&dunce::canonicalize(&codex_home).expect("codex home")));
assert!(!roots.contains(&dunce::canonicalize(&sandbox_root).expect("sandbox root")));
}
}
codex-rs/windows-sandbox-rs/src/token.rs
+27
View File
@@ -139,6 +139,33 @@ pub unsafe fn convert_string_sid_to_sid(s: &str) -> Option<*mut c_void> {
if ok != 0 { Some(psid) } else { None }
}
/// Owns a SID allocated by `ConvertStringSidToSidW` and releases it with `LocalFree`.
pub struct LocalSid {
psid: *mut c_void,
}
impl LocalSid {
pub fn from_string(sid: &str) -> Result<Self> {
let psid = unsafe { convert_string_sid_to_sid(sid) }
.ok_or_else(|| anyhow!("invalid SID string: {sid}"))?;
Ok(Self { psid })
}
pub fn as_ptr(&self) -> *mut c_void {
self.psid
}
}
impl Drop for LocalSid {
fn drop(&mut self) {
if !self.psid.is_null() {
unsafe {
LocalFree(self.psid as HLOCAL);
}
}
}
}
/// # Safety
/// Caller must close the returned token handle.
pub unsafe fn get_current_token_for_restriction() -> Result<HANDLE> {
codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs
+27 -29
View File
@@ -10,11 +10,13 @@ use crate::process::StderrMode;
use crate::process::StdinMode;
use crate::process::read_handle_loop;
use crate::process::spawn_process_with_pipes;
use crate::spawn_prep::LocalSid;
use crate::spawn_prep::LegacyAclSids;
use crate::spawn_prep::allow_null_device_for_workspace_write;
use crate::spawn_prep::apply_legacy_session_acl_rules;
use crate::spawn_prep::legacy_session_capability_roots;
use crate::spawn_prep::prepare_legacy_session_security;
use crate::spawn_prep::prepare_legacy_spawn_context;
use crate::token::LocalSid;
use anyhow::Result;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_pty::ProcessDriver;
@@ -204,8 +206,7 @@ fn finalize_exit(
process_handle: Arc<StdMutex<Option<HANDLE>>>,
thread_handle: HANDLE,
output_join: std::thread::JoinHandle<()>,
guards: Vec<PathBuf>,
cap_sid: Option<String>,
guards: Vec<(PathBuf, String)>,
logs_base_dir: Option<&Path>,
command: Vec<String>,
) {
@@ -242,11 +243,9 @@ fn finalize_exit(
log_failure(&command, &format!("exit code {exit_code}"), logs_base_dir);
}
if let Some(cap_sid) = cap_sid
&& let Ok(sid) = LocalSid::from_string(&cap_sid)
{
unsafe {
for path in guards {
unsafe {
for (path, cap_sid) in guards {
if let Ok(sid) = LocalSid::from_string(&cap_sid) {
revoke_ace(&path, sid.as_ptr());
}
}
@@ -311,15 +310,19 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
if !additional_deny_read_paths.is_empty() {
anyhow::bail!("deny-read overrides require the elevated Windows sandbox backend");
}
let additional_deny_read_paths = additional_deny_read_paths
.iter()
.map(AbsolutePathBuf::to_path_buf)
.collect::<Vec<_>>();
let additional_deny_write_paths = additional_deny_write_paths
.iter()
.map(AbsolutePathBuf::to_path_buf)
.collect::<Vec<_>>();
let security = prepare_legacy_session_security(&common.policy, codex_home, cwd)?;
let capability_roots = legacy_session_capability_roots(
&common.policy,
sandbox_policy_cwd,
&common.current_dir,
&env_map,
codex_home,
);
let security =
prepare_legacy_session_security(&common.policy, codex_home, cwd, capability_roots)?;
allow_null_device_for_workspace_write(common.is_workspace_write);
let persist_aces = common.is_workspace_write;
@@ -329,11 +332,13 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
codex_home,
&common.current_dir,
&env_map,
&security.psid_generic,
security.psid_workspace.as_ref(),
&security.cap_sid_str,
&additional_deny_read_paths,
&[],
&additional_deny_write_paths,
LegacyAclSids {
readonly_sid: security.readonly_sid.as_ref(),
readonly_sid_str: security.readonly_sid_str.as_deref(),
write_root_sids: &security.write_root_sids,
},
persist_aces,
)?;
@@ -370,12 +375,11 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
Ok(handles) => handles,
Err(err) => {
unsafe {
if !persist_aces
&& !guards.is_empty()
&& let Ok(sid) = LocalSid::from_string(&security.cap_sid_str)
{
for path in &guards {
revoke_ace(path, sid.as_ptr());
if !persist_aces {
for (path, cap_sid) in &guards {
if let Ok(sid) = LocalSid::from_string(cap_sid) {
revoke_ace(path, sid.as_ptr());
}
}
}
CloseHandle(security.h_token);
@@ -389,11 +393,6 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
let wait_handle = Arc::clone(&process_handle);
let command_for_wait = command.clone();
let guards_for_wait = if persist_aces { Vec::new() } else { guards };
let cap_sid_for_wait = if guards_for_wait.is_empty() {
None
} else {
Some(security.cap_sid_str)
};
let hpc_for_wait = hpc_handle.clone();
std::thread::spawn(move || {
let _desktop = desktop;
@@ -425,7 +424,6 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
pi.hThread,
output_join,
guards_for_wait,
cap_sid_for_wait,
common.logs_base_dir.as_deref(),
command_for_wait,
);