mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Prevent filesystem-root read denies in the Windows sandbox (#44327)
## Why The elevated Windows sandbox requires effective `:root` read access and cannot safely enforce policies that deny reads at the filesystem root. ## What changed - Validate root read access before elevated setup, service provisioning, and setup refresh, rejecting root-denying globs before expansion. - Reject filesystem-root deny-read ACL targets after canonicalization and check the opened handle before applying an ACL, so aliases cannot bypass the root guard. - Read and update deny ACLs through the same handle, and allow an existing deny ACE to be recognized when write access to the ACL is unavailable. ## Testing Add regression coverage for default root denial, root-denying globs, allowed root reads with narrower deny rules, handle-based root detection, and reading existing deny ACEs without `WRITE_DAC`. GitOrigin-RevId: b904e7cdbc09ee0147e1818c898b0382b5820c1b
This commit is contained in:
@@ -123,14 +123,17 @@ impl WindowsSandboxRequestProcessor {
|
||||
let service_setup_request = setup_request.clone();
|
||||
let service_setup_start = Instant::now();
|
||||
tokio::task::spawn_blocking(move || -> anyhow::Result<()> {
|
||||
if codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
let Ok(permissions) = codex_windows_sandbox::ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_workspace_roots(
|
||||
&service_setup_request.permission_profile,
|
||||
&service_setup_request.workspace_roots,
|
||||
).is_err() {
|
||||
) else {
|
||||
// The existing setup path can still succeed for completed
|
||||
// provisioning without resolving the current profile.
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
permissions.validate_elevated_filesystem_policy(
|
||||
&service_setup_request.command_cwd,
|
||||
)?;
|
||||
// The shared setup path below handles helper fallback and
|
||||
// refreshes workspace ACLs after provisioning.
|
||||
codex_windows_sandbox::provision_windows_sandbox_via_service(
|
||||
|
||||
@@ -3,6 +3,9 @@ use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::process_exec_tool_call;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupMode;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupRequest;
|
||||
use codex_core::windows_sandbox::run_windows_sandbox_setup;
|
||||
use codex_core::windows_sandbox::sandbox_setup_is_complete;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
@@ -229,13 +232,12 @@ fn windows_sandbox_cli_preserves_managed_deny_reads_across_launches() -> anyhow:
|
||||
\n\
|
||||
[permissions.managed-deny-test.filesystem]\n\
|
||||
\":minimal\" = \"read\"\n\
|
||||
\"{}\" = \"read\"\n\
|
||||
\":root\" = \"read\"\n\
|
||||
\"{}\" = \"write\"\n\
|
||||
\"{}\" = \"deny\"\n\
|
||||
\n\
|
||||
[permissions.managed-deny-test.network]\n\
|
||||
enabled = false\n",
|
||||
escape_toml_path(&fixture_root),
|
||||
escape_toml_path(&work),
|
||||
escape_toml_path(&denied),
|
||||
),
|
||||
@@ -262,6 +264,43 @@ fn windows_sandbox_cli_preserves_managed_deny_reads_across_launches() -> anyhow:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial(codex_home)]
|
||||
async fn windows_elevated_setup_rejects_default_root_deny() -> anyhow::Result<()> {
|
||||
let codex_home = codex_home_for_windows_sandbox_test("windows-elevated-root-deny-codex-home")?;
|
||||
let workspace = TempDir::new()?;
|
||||
let cwd = dunce::canonicalize(workspace.path())?.abs();
|
||||
let file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
missing_path_behavior: None,
|
||||
}]);
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
|
||||
let err = run_windows_sandbox_setup(WindowsSandboxSetupRequest {
|
||||
mode: WindowsSandboxSetupMode::Elevated,
|
||||
permission_profile,
|
||||
workspace_roots: vec![cwd.clone()],
|
||||
command_cwd: cwd.to_path_buf(),
|
||||
env_map: HashMap::new(),
|
||||
codex_home: codex_home.path().to_path_buf(),
|
||||
})
|
||||
.await
|
||||
.expect_err("elevated setup should reject default root deny");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"elevated Windows sandbox requires effective `:root` read access"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial(codex_home)]
|
||||
async fn windows_restricted_token_rejects_exact_and_glob_deny_read_policy() -> anyhow::Result<()> {
|
||||
|
||||
@@ -1,9 +1,14 @@
|
||||
use crate::winutil::resolve_sid;
|
||||
use crate::winutil::sid_bytes_from_string;
|
||||
use crate::winutil::to_wide;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::ensure;
|
||||
use std::ffi::c_void;
|
||||
use std::fs::OpenOptions;
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::path::Path;
|
||||
use windows_sys::Win32::Foundation::CloseHandle;
|
||||
use windows_sys::Win32::Foundation::ERROR_SUCCESS;
|
||||
@@ -49,8 +54,10 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA;
|
||||
use windows_sys::Win32::Storage::FileSystem::GetFinalPathNameByHandleW;
|
||||
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
|
||||
use windows_sys::Win32::Storage::FileSystem::READ_CONTROL;
|
||||
use windows_sys::Win32::Storage::FileSystem::VOLUME_NAME_NONE;
|
||||
use windows_sys::Win32::Storage::FileSystem::WRITE_DAC;
|
||||
use windows_sys::Win32::Storage::FileSystem::WRITE_OWNER;
|
||||
const SE_KERNEL_OBJECT: u32 = 6;
|
||||
@@ -740,11 +747,60 @@ impl DenyAceKind {
|
||||
}
|
||||
}
|
||||
|
||||
unsafe fn deny_ace_already_present(
|
||||
handle: &std::fs::File,
|
||||
path: &Path,
|
||||
psid: *mut c_void,
|
||||
kind: DenyAceKind,
|
||||
) -> Result<bool> {
|
||||
let (mut p_dacl, mut p_sd) = (std::ptr::null_mut(), std::ptr::null_mut());
|
||||
let code = GetSecurityInfo(
|
||||
handle.as_raw_handle() as _,
|
||||
1,
|
||||
DACL_SECURITY_INFORMATION,
|
||||
std::ptr::null_mut(),
|
||||
std::ptr::null_mut(),
|
||||
&mut p_dacl,
|
||||
std::ptr::null_mut(),
|
||||
&mut p_sd,
|
||||
);
|
||||
let result =
|
||||
acl_api_result(path, "GetSecurityInfo", code).map(|()| kind.already_present(p_dacl, psid));
|
||||
if !p_sd.is_null() {
|
||||
LocalFree(p_sd as HLOCAL);
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Result<bool> {
|
||||
let handle = match OpenOptions::new()
|
||||
.access_mode(READ_CONTROL | WRITE_DAC)
|
||||
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
|
||||
.open(path)
|
||||
{
|
||||
Ok(handle) => handle,
|
||||
Err(write_error) => {
|
||||
let read_handle = OpenOptions::new()
|
||||
.access_mode(READ_CONTROL)
|
||||
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
|
||||
.open(path)
|
||||
.with_context(|| format!("open deny ACL target {}", path.display()))?;
|
||||
if matches!(kind, DenyAceKind::Read) {
|
||||
ensure_handle_is_not_filesystem_root(&read_handle, path)?;
|
||||
}
|
||||
if deny_ace_already_present(&read_handle, path, psid, kind)? {
|
||||
return Ok(false);
|
||||
}
|
||||
return Err(write_error).context("open deny ACL target for update");
|
||||
}
|
||||
};
|
||||
if matches!(kind, DenyAceKind::Read) {
|
||||
ensure_handle_is_not_filesystem_root(&handle, path)?;
|
||||
}
|
||||
let mut p_sd: *mut c_void = std::ptr::null_mut();
|
||||
let mut p_dacl: *mut ACL = std::ptr::null_mut();
|
||||
let code = GetNamedSecurityInfoW(
|
||||
to_wide(path).as_ptr(),
|
||||
let code = GetSecurityInfo(
|
||||
handle.as_raw_handle() as _,
|
||||
1,
|
||||
DACL_SECURITY_INFORMATION,
|
||||
std::ptr::null_mut(),
|
||||
@@ -758,7 +814,7 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res
|
||||
LocalFree(p_sd as HLOCAL);
|
||||
}
|
||||
return Err(anyhow!(
|
||||
"GetNamedSecurityInfoW failed for {}: {code}",
|
||||
"GetSecurityInfo failed for {}: {code}",
|
||||
path.display()
|
||||
));
|
||||
}
|
||||
@@ -782,8 +838,8 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res
|
||||
let result = if let Err(err) = acl_api_result(path, "SetEntriesInAclW", code2) {
|
||||
Err(err)
|
||||
} else {
|
||||
let code3 = SetNamedSecurityInfoW(
|
||||
to_wide(path).as_ptr() as *mut u16,
|
||||
let code3 = SetSecurityInfo(
|
||||
handle.as_raw_handle() as _,
|
||||
1,
|
||||
DACL_SECURITY_INFORMATION,
|
||||
std::ptr::null_mut(),
|
||||
@@ -791,7 +847,7 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res
|
||||
p_new_dacl,
|
||||
std::ptr::null_mut(),
|
||||
);
|
||||
acl_api_result(path, "SetNamedSecurityInfoW", code3).map(|()| true)
|
||||
acl_api_result(path, "SetSecurityInfo", code3).map(|()| true)
|
||||
};
|
||||
if !p_new_dacl.is_null() {
|
||||
LocalFree(p_new_dacl as HLOCAL);
|
||||
@@ -804,6 +860,28 @@ unsafe fn add_deny_ace(path: &Path, psid: *mut c_void, kind: DenyAceKind) -> Res
|
||||
result
|
||||
}
|
||||
|
||||
fn ensure_handle_is_not_filesystem_root(handle: &std::fs::File, path: &Path) -> Result<()> {
|
||||
let mut buffer = [0_u16; 2];
|
||||
let length = unsafe {
|
||||
GetFinalPathNameByHandleW(
|
||||
handle.as_raw_handle() as _,
|
||||
buffer.as_mut_ptr(),
|
||||
buffer.len() as u32,
|
||||
VOLUME_NAME_NONE,
|
||||
)
|
||||
};
|
||||
if length == 0 {
|
||||
return Err(std::io::Error::last_os_error())
|
||||
.with_context(|| format!("resolve deny-read ACL target {}", path.display()));
|
||||
}
|
||||
ensure!(
|
||||
length != 1 || buffer[0] != b'\\' as u16,
|
||||
"refusing to apply a deny-read ACE to filesystem root {}",
|
||||
path.display()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Adds a deny ACE to prevent reads for the given SID on the target path.
|
||||
///
|
||||
/// `SetEntriesInAclW` places newly-created deny ACEs before allow ACEs, which
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
use super::acl_api_result;
|
||||
use super::deny_ace_already_present;
|
||||
use super::ensure_handle_is_not_filesystem_root;
|
||||
use crate::token::LocalSid;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs::OpenOptions;
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
|
||||
use windows_sys::Win32::Storage::FileSystem::READ_CONTROL;
|
||||
|
||||
#[test]
|
||||
fn deny_ace_update_failure_is_an_error() {
|
||||
@@ -13,3 +20,36 @@ fn deny_ace_update_failure_is_an_error() {
|
||||
r"SetNamedSecurityInfoW failed for C:\world-writable: 5"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deny_read_root_check_uses_the_open_handle() {
|
||||
let cwd = std::env::current_dir().expect("current directory");
|
||||
let root = cwd.ancestors().last().expect("filesystem root");
|
||||
let root_directory = OpenOptions::new()
|
||||
.access_mode(READ_CONTROL)
|
||||
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
|
||||
.open(root)
|
||||
.expect("open filesystem root");
|
||||
let diagnostic_path = std::path::Path::new(r"C:\not-a-root");
|
||||
|
||||
let error = ensure_handle_is_not_filesystem_root(&root_directory, diagnostic_path)
|
||||
.expect_err("classification must follow the root handle, not the diagnostic path");
|
||||
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
r"refusing to apply a deny-read ACE to filesystem root C:\not-a-root"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn existing_deny_ace_is_visible_without_write_dac() {
|
||||
let target = tempfile::NamedTempFile::new().expect("temporary file");
|
||||
let sid = LocalSid::from_string("S-1-5-21-10-20-30-40").expect("test SID");
|
||||
let path = target.path();
|
||||
let psid = sid.as_ptr();
|
||||
assert!(unsafe { super::add_deny_read_ace(path, psid) }.expect("add deny ACE"));
|
||||
let already_present =
|
||||
unsafe { deny_ace_already_present(target.as_file(), path, psid, super::DenyAceKind::Read) }
|
||||
.expect("read existing deny ACE");
|
||||
assert!(already_present);
|
||||
}
|
||||
|
||||
@@ -262,7 +262,10 @@ fn apply_capability_denies_for_world_writable_for_permissions(
|
||||
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)?)?;
|
||||
if !permissions.is_enforceable_by_windows_sandbox() {
|
||||
if permissions
|
||||
.validate_elevated_filesystem_policy(cwd)
|
||||
.is_err()
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
let (active_sids, workspace_roots): (Vec<LocalSid>, Vec<PathBuf>) =
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use crate::acl::add_deny_read_ace;
|
||||
use crate::acl::revoke_ace;
|
||||
use crate::path_normalization::canonicalize_path;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use std::collections::HashSet;
|
||||
@@ -14,17 +13,33 @@ use std::path::PathBuf;
|
||||
/// canonical target. The lexical path covers the path users configured and lets
|
||||
/// missing exact denies be materialized later; the canonical path also covers
|
||||
/// an existing reparse-point target so a sandbox cannot read the same object
|
||||
/// through the resolved location.
|
||||
pub fn plan_deny_read_acl_paths(paths: &[PathBuf]) -> Vec<PathBuf> {
|
||||
/// through the resolved location. Filesystem roots are rejected after this
|
||||
/// canonicalization so aliases cannot install a root deny ACE.
|
||||
pub fn plan_deny_read_acl_paths(paths: &[PathBuf]) -> Result<Vec<PathBuf>> {
|
||||
let mut planned = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
for path in paths {
|
||||
push_planned_path(&mut planned, &mut seen, path.to_path_buf());
|
||||
if path.exists() {
|
||||
push_planned_path(&mut planned, &mut seen, canonicalize_path(path));
|
||||
}
|
||||
let canonical = match dunce::canonicalize(path) {
|
||||
Ok(canonical) => canonical,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
|
||||
Err(err) => {
|
||||
return Err(err)
|
||||
.with_context(|| format!("canonicalize deny-read path {}", path.display()));
|
||||
}
|
||||
};
|
||||
push_planned_path(&mut planned, &mut seen, canonical);
|
||||
}
|
||||
planned
|
||||
if let Some(root) = planned
|
||||
.iter()
|
||||
.find(|path| path.is_absolute() && path.parent().is_none())
|
||||
{
|
||||
anyhow::bail!(
|
||||
"refusing to apply a deny-read ACE to filesystem root {}",
|
||||
root.display()
|
||||
);
|
||||
}
|
||||
Ok(planned)
|
||||
}
|
||||
|
||||
fn push_planned_path(planned: &mut Vec<PathBuf>, seen: &mut HashSet<String>, path: PathBuf) {
|
||||
@@ -49,7 +64,7 @@ pub(crate) fn lexical_path_key(path: &Path) -> String {
|
||||
/// # Safety
|
||||
/// Caller must pass a valid SID pointer for the sandbox principal being denied.
|
||||
pub unsafe fn apply_deny_read_acls(paths: &[PathBuf], psid: *mut c_void) -> Result<Vec<PathBuf>> {
|
||||
let planned = plan_deny_read_acl_paths(paths);
|
||||
let planned = plan_deny_read_acl_paths(paths)?;
|
||||
let mut applied = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
let mut added_in_this_call: Vec<PathBuf> = Vec::new();
|
||||
@@ -93,7 +108,7 @@ mod tests {
|
||||
let missing = tmp.path().join("future-secret.env");
|
||||
|
||||
assert_eq!(
|
||||
plan_deny_read_acl_paths(std::slice::from_ref(&missing)),
|
||||
plan_deny_read_acl_paths(std::slice::from_ref(&missing)).expect("plan deny-read ACL"),
|
||||
vec![missing]
|
||||
);
|
||||
}
|
||||
@@ -105,6 +120,7 @@ mod tests {
|
||||
std::fs::write(&existing, "secret").expect("write secret");
|
||||
|
||||
let planned: HashSet<PathBuf> = plan_deny_read_acl_paths(std::slice::from_ref(&existing))
|
||||
.expect("plan deny-read ACL")
|
||||
.into_iter()
|
||||
.collect();
|
||||
let expected: HashSet<PathBuf> = [
|
||||
@@ -116,4 +132,21 @@ mod tests {
|
||||
|
||||
assert_eq!(planned, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plan_rejects_file_system_root() {
|
||||
let cwd = std::env::current_dir().expect("current directory");
|
||||
let root = cwd.ancestors().last().expect("filesystem root");
|
||||
|
||||
let err = plan_deny_read_acl_paths(&[root.to_path_buf()])
|
||||
.expect_err("filesystem root must not receive a deny-read ACE");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
format!(
|
||||
"refusing to apply a deny-read ACE to filesystem root {}",
|
||||
root.display()
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -131,7 +131,7 @@ impl DesktopPolicy {
|
||||
write_roots: write_roots.into_iter().collect(),
|
||||
deny_read_paths: plan_deny_read_acl_paths(
|
||||
overrides.deny_read_paths.as_deref().unwrap_or_default(),
|
||||
)
|
||||
)?
|
||||
.into_iter()
|
||||
.collect(),
|
||||
deny_write_paths: build_payload_deny_write_paths(&request, overrides.deny_write_paths)
|
||||
|
||||
@@ -6,6 +6,7 @@ use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath::Root;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::permissions::ReadDenyMatcher;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
@@ -99,8 +100,20 @@ impl ResolvedWindowsSandboxPermissions {
|
||||
self.network
|
||||
}
|
||||
|
||||
pub(crate) fn is_enforceable_by_windows_sandbox(&self) -> bool {
|
||||
matches!(self.file_system.kind, FileSystemSandboxKind::Restricted)
|
||||
/// Rejects filesystem policies that the elevated Windows sandbox cannot
|
||||
/// enforce safely.
|
||||
pub fn validate_elevated_filesystem_policy(&self, cwd: &Path) -> Result<()> {
|
||||
let root = cwd
|
||||
.ancestors()
|
||||
.last()
|
||||
.ok_or_else(|| anyhow::anyhow!("command cwd has no filesystem root"))?;
|
||||
let root_is_denied = ReadDenyMatcher::try_new_for_local_paths(&self.file_system, cwd)
|
||||
.map_err(anyhow::Error::msg)?
|
||||
.is_some_and(|matcher| matcher.is_local_path_read_denied(root));
|
||||
if !self.file_system.can_read_local_path_with_cwd(root, cwd) || root_is_denied {
|
||||
anyhow::bail!("elevated Windows sandbox requires effective `:root` read access");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) fn has_full_disk_read_access(&self) -> bool {
|
||||
@@ -223,6 +236,16 @@ mod tests {
|
||||
vec![AbsolutePathBuf::from_absolute_path(root).expect("absolute workspace root")]
|
||||
}
|
||||
|
||||
fn permission_profile_with_entries(entries: Vec<FileSystemSandboxEntry>) -> PermissionProfile {
|
||||
PermissionProfile::Managed {
|
||||
file_system: ManagedFileSystemPermissions::Restricted {
|
||||
entries,
|
||||
glob_scan_max_depth: None,
|
||||
},
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_profile_workspace_write_uses_windows_temp_env_vars() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
@@ -464,6 +487,96 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn elevated_filesystem_policy_requires_effective_root_read_access() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let cwd = tmp.path().join("workspace");
|
||||
let denied_path = tmp.path().join("private");
|
||||
std::fs::create_dir_all(&cwd).expect("create cwd");
|
||||
|
||||
let default_root_denied = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&permission_profile_with_entries(vec![FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
FileSystemAccessMode::Write,
|
||||
)]),
|
||||
)
|
||||
.expect("managed permission profile");
|
||||
let root_read_with_carveout =
|
||||
ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&permission_profile_with_entries(vec![
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Path {
|
||||
path: AbsolutePathBuf::from_absolute_path(&denied_path)
|
||||
.expect("absolute denied path")
|
||||
.into(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
),
|
||||
]),
|
||||
)
|
||||
.expect("managed permission profile");
|
||||
let root = cwd.ancestors().last().expect("filesystem root");
|
||||
let root_read_with_explicit_path =
|
||||
ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&permission_profile_with_entries(vec![FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Path {
|
||||
path: AbsolutePathBuf::from_absolute_path(root)
|
||||
.expect("absolute root")
|
||||
.into(),
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
)]),
|
||||
)
|
||||
.expect("managed permission profile");
|
||||
let root_read_with_root_deny_glob =
|
||||
ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&permission_profile_with_entries(vec![
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: root.join("**").display().to_string(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
),
|
||||
]),
|
||||
)
|
||||
.expect("managed permission profile");
|
||||
|
||||
assert!(
|
||||
default_root_denied
|
||||
.validate_elevated_filesystem_policy(&cwd)
|
||||
.is_err()
|
||||
);
|
||||
assert!(
|
||||
root_read_with_carveout
|
||||
.validate_elevated_filesystem_policy(&cwd)
|
||||
.is_ok()
|
||||
);
|
||||
assert!(
|
||||
root_read_with_explicit_path
|
||||
.validate_elevated_filesystem_policy(&cwd)
|
||||
.is_ok()
|
||||
);
|
||||
assert!(
|
||||
root_read_with_root_deny_glob
|
||||
.validate_elevated_filesystem_policy(&cwd)
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn token_mode_rejects_full_disk_write_entries() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
|
||||
@@ -235,6 +235,7 @@ pub fn run_setup_refresh(
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
permissions.validate_elevated_filesystem_policy(command_cwd)?;
|
||||
let deny_read_paths =
|
||||
setup_refresh_deny_read_paths(permission_profile, workspace_roots, command_cwd)?;
|
||||
run_setup_refresh_inner(
|
||||
@@ -278,6 +279,7 @@ pub fn run_setup_refresh_with_extra_read_roots(
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
permissions.validate_elevated_filesystem_policy(command_cwd)?;
|
||||
let deny_read_paths =
|
||||
setup_refresh_deny_read_paths(permission_profile, workspace_roots, command_cwd)?;
|
||||
let mut read_roots = gather_read_roots(command_cwd, &permissions, env_map, codex_home);
|
||||
@@ -325,9 +327,9 @@ fn run_setup_refresh_inner(
|
||||
overrides: SetupRootOverrides,
|
||||
offline_proxy_settings_override: Option<&OfflineProxySettings>,
|
||||
) -> Result<()> {
|
||||
if !request.permissions.is_enforceable_by_windows_sandbox() {
|
||||
anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup");
|
||||
}
|
||||
request
|
||||
.permissions
|
||||
.validate_elevated_filesystem_policy(request.command_cwd)?;
|
||||
let (read_roots, write_roots) = build_payload_roots(&request, &overrides);
|
||||
let deny_read_paths = build_payload_deny_read_paths(overrides.deny_read_paths);
|
||||
let deny_write_paths = build_payload_deny_write_paths(&request, overrides.deny_write_paths);
|
||||
@@ -1052,9 +1054,9 @@ fn run_elevated_setup_inner(
|
||||
request: SandboxSetupRequest<'_>,
|
||||
offline_proxy_settings_override: Option<&OfflineProxySettings>,
|
||||
) -> Result<()> {
|
||||
if !request.permissions.is_enforceable_by_windows_sandbox() {
|
||||
anyhow::bail!("unsupported filesystem permissions for Windows sandbox setup");
|
||||
}
|
||||
request
|
||||
.permissions
|
||||
.validate_elevated_filesystem_policy(request.command_cwd)?;
|
||||
// Ensure the shared sandbox directory exists before we send it to the elevated helper.
|
||||
let sbx_dir = sandbox_dir(request.codex_home);
|
||||
std::fs::create_dir_all(&sbx_dir).map_err(|err| {
|
||||
@@ -1409,6 +1411,7 @@ mod tests {
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::permissions::project_roots_glob_pattern;
|
||||
@@ -1764,6 +1767,56 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn setup_refresh_rejects_root_globs_before_expansion() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let command_cwd = tmp.path().join("workspace");
|
||||
let codex_home = tmp.path().join("codex-home");
|
||||
fs::create_dir_all(&command_cwd).expect("create workspace");
|
||||
let root = command_cwd.ancestors().last().expect("filesystem root");
|
||||
let mut file_system = FileSystemSandboxPolicy::read_only();
|
||||
file_system.entries.push(FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: root.join("**").display().to_string(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
));
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
let workspace_roots = workspace_roots_for(&command_cwd);
|
||||
let env_map = HashMap::new();
|
||||
let errors = [
|
||||
super::run_setup_refresh(
|
||||
&permission_profile,
|
||||
&workspace_roots,
|
||||
&command_cwd,
|
||||
&env_map,
|
||||
&codex_home,
|
||||
/*proxy_enforced*/ false,
|
||||
)
|
||||
.expect_err("root glob must be rejected before expansion"),
|
||||
super::run_setup_refresh_with_extra_read_roots(
|
||||
&permission_profile,
|
||||
&workspace_roots,
|
||||
&command_cwd,
|
||||
&env_map,
|
||||
&codex_home,
|
||||
Vec::new(),
|
||||
/*proxy_enforced*/ false,
|
||||
)
|
||||
.expect_err("root glob must be rejected before expansion"),
|
||||
];
|
||||
|
||||
let expected =
|
||||
"elevated Windows sandbox requires effective `:root` read access".to_string();
|
||||
assert_eq!(
|
||||
errors.map(|err| err.to_string()),
|
||||
[expected.clone(), expected]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn setup_refresh_rejects_invalid_deny_read_globs() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user