openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-17 12:23:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: scope windows sandbox capability sids by writable root

Browse Source
This commit is contained in:
David Wiesen
2026-03-24 09:30:23 -07:00
parent 68baac7cf4
commit e81f9a3a0b
4 changed files with 195 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
codex-rs/windows-sandbox-rs/src/cap.rs
View File

@@ -15,11 +15,13 @@ use crate::path_normalization::canonical_path_key;
pub struct CapSids {
pub workspace: String,
pub readonly: String,
/// Per-workspace capability SIDs keyed by canonicalized CWD string.
/// Path-scoped capability SIDs keyed by canonicalized writable-root strings.
///
/// This is used to isolate workspaces from other workspace sandbox writes and to
/// apply per-workspace denies (e.g. protect `CWD/.codex`)
/// without permanently affecting other workspaces.
/// This is used to isolate writable roots from one another so stale ACL grants on
/// one workspace do not automatically authorize later sessions in unrelated roots.
///
/// The serialized field name is kept for backwards compatibility with existing
/// `cap_sid` files on disk.
#[serde(default)]
pub workspace_by_cwd: HashMap<String, String>,
}
@@ -77,9 +79,14 @@ pub fn load_or_create_cap_sids(codex_home: &Path) -> Result<CapSids> {
/// Returns the workspace-specific capability SID for `cwd`, creating and persisting it if missing.
pub fn workspace_cap_sid_for_cwd(codex_home: &Path, cwd: &Path) -> Result<String> {
write_cap_sid_for_root(codex_home, cwd)
}
/// Returns the path-scoped capability SID for `root`, creating and persisting it if missing.
pub fn write_cap_sid_for_root(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(cwd);
let key = canonical_path_key(root);
if let Some(sid) = caps.workspace_by_cwd.get(&key) {
return Ok(sid.clone());
}
@@ -92,8 +99,10 @@ pub fn workspace_cap_sid_for_cwd(codex_home: &Path, cwd: &Path) -> Result<String
#[cfg(test)]
mod tests {
use super::load_or_create_cap_sids;
use super::write_cap_sid_for_root;
use super::workspace_cap_sid_for_cwd;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::path::PathBuf;
#[test]
@@ -118,4 +127,25 @@ mod tests {
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
assert_eq!(caps.workspace_by_cwd.len(), 1);
}
#[test]
fn distinct_writable_roots_get_distinct_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_a = temp.path().join("WorkspaceA");
let workspace_b = temp.path().join("WorkspaceB");
std::fs::create_dir_all(&workspace_a).expect("create workspace a");
std::fs::create_dir_all(&workspace_b).expect("create workspace b");
let sid_a = write_cap_sid_for_root(&codex_home, &workspace_a).expect("sid a");
let sid_b = write_cap_sid_for_root(&codex_home, &workspace_b).expect("sid b");
assert_ne!(sid_a, sid_b);
let caps = load_or_create_cap_sids(&codex_home).expect("load caps");
let values: HashSet<_> = caps.workspace_by_cwd.values().cloned().collect();
assert_eq!(values.len(), 2);
}
}

36
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
View File

@@ -3,6 +3,7 @@ mod windows_impl {
use crate::allow::compute_allow_paths;
use crate::allow::AllowDenyPaths;
use crate::cap::load_or_create_cap_sids;
use crate::cap::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;
@@ -239,25 +240,40 @@ mod windows_impl {
anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing")
}
let caps = load_or_create_cap_sids(codex_home)?;
let AllowDenyPaths { allow, deny: _ } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
let canonical_cwd = crate::path_normalization::canonicalize_path(&current_dir);
let mut allow_roots: Vec<PathBuf> = allow.into_iter().collect();
allow_roots.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
let (psid_to_use, cap_sids) = match &policy {
SandboxPolicy::ReadOnly { .. } => (
unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() },
vec![caps.readonly.clone()],
),
SandboxPolicy::WorkspaceWrite { .. } => (
unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() },
vec![
caps.workspace.clone(),
crate::cap::workspace_cap_sid_for_cwd(codex_home, cwd)?,
],
),
SandboxPolicy::WorkspaceWrite { .. } => {
let mut cap_sids = Vec::with_capacity(allow_roots.len());
for root in &allow_roots {
let sid = if crate::workspace_acl::is_command_cwd_root(root, &canonical_cwd) {
crate::cap::workspace_cap_sid_for_cwd(codex_home, root)?
} else {
write_cap_sid_for_root(codex_home, root)?
};
cap_sids.push(sid);
}
let psid_to_use = unsafe {
convert_string_sid_to_sid(
cap_sids
.first()
.expect("workspace-write should always have a writable root"),
)
.unwrap()
};
(psid_to_use, cap_sids)
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("DangerFullAccess handled above")
}
};
let AllowDenyPaths { allow: _, deny: _ } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
// Deny/allow ACEs are now applied during setup; avoid per-command churn.
unsafe {
allow_null_device(psid_to_use);

153
codex-rs/windows-sandbox-rs/src/lib.rs
View File

@@ -64,6 +64,8 @@ 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::write_cap_sid_for_root;
#[cfg(target_os = "windows")]
pub use conpty::spawn_conpty_process_as_user;
#[cfg(target_os = "windows")]
pub use dpapi::protect as dpapi_protect;
@@ -179,6 +181,7 @@ mod windows_impl {
use super::allow::AllowDenyPaths;
use super::cap::load_or_create_cap_sids;
use super::cap::workspace_cap_sid_for_cwd;
use super::cap::write_cap_sid_for_root;
use super::env::apply_no_network_to_env;
use super::env::ensure_non_interactive_pager;
use super::env::normalize_null_device_env;
@@ -213,6 +216,40 @@ mod windows_impl {
type PipeHandles = ((HANDLE, HANDLE), (HANDLE, HANDLE), (HANDLE, HANDLE));
fn sorted_allow_roots(allow: &std::collections::HashSet<PathBuf>) -> Vec<PathBuf> {
let mut roots: Vec<PathBuf> = allow.iter().cloned().collect();
roots.sort_by_key(|path| std::cmp::Reverse(path.components().count()));
roots
}
fn capability_sid_strings_for_allow_roots(
codex_home: &Path,
allow_roots: &[PathBuf],
canonical_cwd: &Path,
) -> Result<HashMap<PathBuf, String>> {
let mut sid_by_root = HashMap::new();
for root in allow_roots {
let sid = if is_command_cwd_root(root, canonical_cwd) {
workspace_cap_sid_for_cwd(codex_home, root)?
} else {
write_cap_sid_for_root(codex_home, root)?
};
sid_by_root.insert(root.clone(), sid);
}
Ok(sid_by_root)
}
fn capability_sid_for_path<'a>(
sid_by_root: &'a HashMap<PathBuf, *mut c_void>,
sorted_allow_roots: &[PathBuf],
path: &Path,
) -> Option<*mut c_void> {
sorted_allow_roots
.iter()
.find(|root| path.starts_with(root))
.and_then(|root| sid_by_root.get(root).copied())
}
fn should_apply_network_block(policy: &SandboxPolicy) -> bool {
!policy.has_full_network_access()
}
@@ -282,6 +319,11 @@ mod windows_impl {
let logs_base_dir = Some(sandbox_base.as_path());
log_start(&command, logs_base_dir);
let is_workspace_write = matches!(&policy, SandboxPolicy::WorkspaceWrite { .. });
let current_dir = cwd.to_path_buf();
let AllowDenyPaths { allow, deny } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
let canonical_cwd = canonicalize_path(&current_dir);
let sorted_allow_roots = sorted_allow_roots(&allow);
if matches!(
&policy,
@@ -295,25 +337,38 @@ mod windows_impl {
);
}
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 {
let sid_strings_by_root = capability_sid_strings_for_allow_roots(
codex_home,
&sorted_allow_roots,
&canonical_cwd,
)?;
let (h_token, psid_readonly, psids_by_root): (
HANDLE,
Option<*mut c_void>,
HashMap<PathBuf, *mut c_void>,
) = unsafe {
match &policy {
SandboxPolicy::ReadOnly { .. } => {
let psid = convert_string_sid_to_sid(&caps.readonly).unwrap();
let (h, _) = super::token::create_readonly_token_with_cap(psid)?;
(h, psid, None)
(h, Some(psid), HashMap::new())
}
SandboxPolicy::WorkspaceWrite { .. } => {
let psid_generic = convert_string_sid_to_sid(&caps.workspace).unwrap();
let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
let psid_workspace = convert_string_sid_to_sid(&ws_sid).unwrap();
let mut psids_by_root = HashMap::new();
let mut token_caps = Vec::with_capacity(sorted_allow_roots.len());
for root in &sorted_allow_roots {
let sid = sid_strings_by_root
.get(root)
.expect("missing writable-root SID");
let psid = convert_string_sid_to_sid(sid).unwrap();
token_caps.push(psid);
psids_by_root.insert(root.clone(), psid);
}
let base = super::token::get_current_token_for_restriction()?;
let h_res = create_workspace_write_token_with_caps_from(
base,
&[psid_generic, psid_workspace],
);
let h_res = create_workspace_write_token_with_caps_from(base, &token_caps);
windows_sys::Win32::Foundation::CloseHandle(base);
let h = h_res?;
(h, psid_generic, Some(psid_workspace))
(h, None, psids_by_root)
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("DangerFullAccess handled above")
@@ -335,16 +390,15 @@ mod windows_impl {
}
let persist_aces = is_workspace_write;
let AllowDenyPaths { allow, deny } =
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
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)
let Some(psid) = (if is_workspace_write {
capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
} else {
psid_generic
psid_readonly
}) else {
continue;
};
if let Ok(added) = add_allow_ace(p, psid) {
if added {
@@ -359,15 +413,28 @@ mod windows_impl {
}
}
for p in &deny {
if let Ok(added) = add_deny_write_ace(p, psid_generic) {
let Some(psid) = (if is_workspace_write {
capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
} else {
psid_readonly
}) else {
continue;
};
if let Ok(added) = add_deny_write_ace(p, psid) {
if added && !persist_aces {
guards.push((p.clone(), psid_generic));
guards.push((p.clone(), psid));
}
}
}
allow_null_device(psid_generic);
if let Some(psid) = psid_workspace {
if let Some(psid) = psid_readonly {
allow_null_device(psid);
}
for psid in psids_by_root.values().copied() {
allow_null_device(psid);
}
if let Some(psid) =
capability_sid_for_path(&psids_by_root, &sorted_allow_roots, &current_dir)
{
let _ = protect_workspace_codex_dir(&current_dir, psid);
let _ = protect_workspace_agents_dir(&current_dir, psid);
}
@@ -525,33 +592,49 @@ mod windows_impl {
}
ensure_codex_home_exists(codex_home)?;
let caps = load_or_create_cap_sids(codex_home)?;
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)?;
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);
let sorted_allow_roots = sorted_allow_roots(&allow);
let sid_strings_by_root = capability_sid_strings_for_allow_roots(
codex_home,
&sorted_allow_roots,
&canonical_cwd,
)?;
let mut psids_by_root = HashMap::new();
for root in &sorted_allow_roots {
let sid = sid_strings_by_root
.get(root)
.expect("missing writable-root SID");
let psid = unsafe { convert_string_sid_to_sid(sid) }.expect("valid workspace SID");
psids_by_root.insert(root.clone(), psid);
}
unsafe {
for p in &allow {
let psid = if is_command_cwd_root(p, &canonical_cwd) {
psid_workspace
} else {
psid_generic
let Some(psid) = capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
else {
continue;
};
let _ = add_allow_ace(p, psid);
}
for p in &deny {
let _ = add_deny_write_ace(p, psid_generic);
let Some(psid) = capability_sid_for_path(&psids_by_root, &sorted_allow_roots, p)
else {
continue;
};
let _ = add_deny_write_ace(p, psid);
}
for psid in psids_by_root.values().copied() {
allow_null_device(psid);
}
if let Some(psid) =
capability_sid_for_path(&psids_by_root, &sorted_allow_roots, &current_dir)
{
let _ = protect_workspace_codex_dir(&current_dir, psid);
let _ = protect_workspace_agents_dir(&current_dir, psid);
}
allow_null_device(psid_generic);
allow_null_device(psid_workspace);
let _ = protect_workspace_codex_dir(&current_dir, psid_workspace);
let _ = protect_workspace_agents_dir(&current_dir, psid_workspace);
}
Ok(())

47
codex-rs/windows-sandbox-rs/src/setup_main_win.rs
View File

@@ -13,7 +13,6 @@ use codex_windows_sandbox::ensure_allow_write_aces;
use codex_windows_sandbox::extract_setup_failure;
use codex_windows_sandbox::hide_newly_created_users;
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::protect_workspace_agents_dir;
@@ -24,6 +23,7 @@ use codex_windows_sandbox::sandbox_secrets_dir;
use codex_windows_sandbox::string_from_sid_bytes;
use codex_windows_sandbox::to_wide;
use codex_windows_sandbox::workspace_cap_sid_for_cwd;
use codex_windows_sandbox::write_cap_sid_for_root;
use codex_windows_sandbox::write_setup_error_report;
use codex_windows_sandbox::SetupErrorCode;
use codex_windows_sandbox::SetupErrorReport;
@@ -551,25 +551,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
))
})?;
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 firewall_result = firewall::ensure_offline_outbound_block(&offline_sid_str, log);
@@ -616,7 +597,6 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
}
}
let cap_sid_str = caps.workspace.clone();
let sandbox_group_sid_str =
string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
let write_mask =
@@ -639,19 +619,18 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
}
let mut need_grant = false;
let is_command_cwd = is_command_cwd_root(root, &canonical_command_cwd);
let cap_label = if is_command_cwd {
"workspace_cap"
let cap_sid_str_for_root = if is_command_cwd {
workspace_cap_sid_for_cwd(&payload.codex_home, root)?
} else {
"cap"
write_cap_sid_for_root(&payload.codex_home, root)?
};
let cap_psid_for_root = if is_command_cwd {
workspace_psid
} else {
cap_psid
let cap_psid_for_root = unsafe {
convert_string_sid_to_sid(&cap_sid_str_for_root)
.ok_or_else(|| anyhow::anyhow!("convert writable-root capability SID failed"))?
};
for (label, psid) in [
("sandbox_group", sandbox_group_psid),
(cap_label, cap_psid_for_root),
("path_cap", cap_psid_for_root),
] {
let has =
match path_mask_allows(root, &[psid], write_mask, /*require_all_bits*/ true) {
@@ -694,9 +673,15 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
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()]
vec![
sandbox_group_sid_str.clone(),
workspace_cap_sid_for_cwd(&payload.codex_home, &root)?,
]
} else {
vec![sandbox_group_sid_str.clone(), cap_sid_str.clone()]
vec![
sandbox_group_sid_str.clone(),
write_cap_sid_for_root(&payload.codex_home, &root)?,
]
};
let tx = tx.clone();
scope.spawn(move || {