mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Use resolved filesystem permissions in Windows sandbox policies (#59233)
## What changed Use `resolve_for_enforcement` when preparing Windows restricted-token and elevated sandbox overrides, preserving read-only carveouts at volume roots. Expand deny-read ACL targets from the resolved permissions. Use the same resolution in MXC policy preparation when volume roots do not require materialization, while retaining special handling for symbolic root denies. ## Testing Add regression coverage for read-only carveouts in a volume-root workspace. Extend the Windows package smoke test to verify that a helper-created log matching a deny glob cannot be read while a neighboring allowed file remains readable. GitOrigin-RevId: cee0834444464b3a7b0c8dbeecb8abcd5828b4b0
This commit is contained in:
@@ -8,12 +8,14 @@ use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::permissions::FileSystemPolicyProjection;
|
||||
use codex_protocol::protocol::FileSystemAccessMode;
|
||||
use codex_protocol::protocol::FileSystemPath;
|
||||
use codex_protocol::protocol::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::FileSystemSpecialPath;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_windows_sandbox::expand_windows_deny_read_paths;
|
||||
use codex_windows_sandbox::resolve_windows_deny_read_paths;
|
||||
use mxc_sdk::mxc_common::cmdline::CommandLineContext;
|
||||
use mxc_sdk::mxc_common::cmdline::CommandLineError;
|
||||
@@ -153,10 +155,26 @@ pub(super) fn build_request(
|
||||
#[cfg(not(windows))]
|
||||
let read_volumes = volumes.clone();
|
||||
let fs = materialize_volume_roots(policy.clone(), &volumes, &read_volumes)?;
|
||||
let roots = if full_disk_write {
|
||||
Vec::new()
|
||||
let absolute_cwd = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
.map_err(|error| PolicyError::PolicyResolution(error.to_string()))?;
|
||||
let (roots, readable_roots, denied_paths) = if fs == policy {
|
||||
let resolved =
|
||||
fs.resolve_for_enforcement(cwd, FileSystemPolicyProjection::PreserveMutableRoots)?;
|
||||
let denies = expand_windows_deny_read_paths(&resolved, &absolute_cwd);
|
||||
(resolved.writable_roots, resolved.readable_roots, denies)
|
||||
} else {
|
||||
fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd)
|
||||
// A symbolic root deny sets the default on every volume. Expanding it into ACL
|
||||
// targets would mask narrower grants and probe disconnected drives.
|
||||
let roots = if full_disk_write {
|
||||
Vec::new()
|
||||
} else {
|
||||
fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd)
|
||||
};
|
||||
(
|
||||
roots,
|
||||
fs.get_readable_roots_with_cwd(cwd),
|
||||
resolve_windows_deny_read_paths(&policy, &absolute_cwd),
|
||||
)
|
||||
};
|
||||
let mut write = collect_paths(roots.iter().map(|root| root.root.to_path_buf()))?;
|
||||
if full_disk_write {
|
||||
@@ -171,17 +189,13 @@ pub(super) fn build_request(
|
||||
}
|
||||
}
|
||||
let mut read = collect_paths(
|
||||
fs.get_readable_roots_with_cwd(cwd)
|
||||
readable_roots
|
||||
.into_iter()
|
||||
.filter(|path| !fs.can_write_local_path_with_cwd(path.as_path(), cwd))
|
||||
.map(|path| path.to_path_buf()),
|
||||
)?;
|
||||
let absolute_cwd = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
.map_err(|error| PolicyError::PolicyResolution(error.to_string()))?;
|
||||
// A symbolic root deny is the default, not a recursive native mask that
|
||||
// should erase narrower grants. Only materialize grants and carveouts.
|
||||
let deny = collect_paths(
|
||||
resolve_windows_deny_read_paths(&policy, &absolute_cwd)
|
||||
denied_paths
|
||||
.map_err(PolicyError::PolicyResolution)?
|
||||
.into_iter()
|
||||
.map(AbsolutePathBuf::into_path_buf),
|
||||
|
||||
@@ -842,3 +842,32 @@ fn transform_for_direct_spawn_windows_materializes_inner_helper() {
|
||||
);
|
||||
assert!(materialized_helper.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restricted_token_preserves_read_only_carveouts_at_a_volume_root() -> anyhow::Result<()> {
|
||||
let cwd = AbsolutePathBuf::current_dir()?;
|
||||
let root = AbsolutePathBuf::from_absolute_path(cwd.ancestors().last().expect("volume root"))?;
|
||||
// Exercise policy preparation without installing ACLs on the test host.
|
||||
let read_only = root.join("codex-test-read-only");
|
||||
let mut file_system = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
);
|
||||
file_system.entries.push(FileSystemSandboxEntry::new(
|
||||
read_only.clone().into(),
|
||||
FileSystemAccessMode::Read,
|
||||
));
|
||||
let profile =
|
||||
PermissionProfile::from_runtime_permissions(&file_system, NetworkSandboxPolicy::Restricted);
|
||||
let overrides = crate::resolve_windows_restricted_token_filesystem_overrides(
|
||||
SandboxType::WindowsRestrictedToken,
|
||||
&profile,
|
||||
&root,
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
)
|
||||
.expect("prepare volume-root workspace")
|
||||
.expect("read-only child");
|
||||
assert_eq!(overrides.additional_deny_write_paths, [read_only]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemPolicyProjection;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::WritableRoot;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -119,10 +120,11 @@ pub fn resolve_windows_restricted_token_filesystem_overrides(
|
||||
);
|
||||
}
|
||||
|
||||
let additional_deny_read_paths = codex_windows_sandbox::resolve_windows_deny_read_paths(
|
||||
&file_system_sandbox_policy,
|
||||
sandbox_policy_cwd,
|
||||
)?;
|
||||
let resolved = file_system_sandbox_policy
|
||||
.resolve_for_enforcement(sandbox_policy_cwd, FileSystemPolicyProjection::Native)
|
||||
.map_err(|err| err.to_string())?;
|
||||
let additional_deny_read_paths =
|
||||
codex_windows_sandbox::expand_windows_deny_read_paths(&resolved, sandbox_policy_cwd)?;
|
||||
if !additional_deny_read_paths.is_empty() {
|
||||
return Err(
|
||||
"windows unelevated restricted-token sandbox cannot enforce deny-read restrictions directly; refusing to run unsandboxed"
|
||||
@@ -135,8 +137,7 @@ pub fn resolve_windows_restricted_token_filesystem_overrides(
|
||||
sandbox_policy_cwd.as_path(),
|
||||
);
|
||||
let legacy_writable_roots = legacy_projection.get_writable_roots_with_cwd(sandbox_policy_cwd);
|
||||
let split_writable_roots =
|
||||
file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd);
|
||||
let split_writable_roots = resolved.writable_roots;
|
||||
let legacy_root_paths: BTreeSet<PathBuf> = legacy_writable_roots
|
||||
.iter()
|
||||
.map(|root| normalize_windows_override_path(root.root.as_path()))
|
||||
@@ -153,21 +154,11 @@ pub fn resolve_windows_restricted_token_filesystem_overrides(
|
||||
);
|
||||
}
|
||||
|
||||
for writable_root in &split_writable_roots {
|
||||
for read_only_subpath in &writable_root.read_only_subpaths {
|
||||
if split_writable_roots.iter().any(|candidate| {
|
||||
candidate.root.as_path() != writable_root.root.as_path()
|
||||
&& candidate
|
||||
.root
|
||||
.as_path()
|
||||
.starts_with(read_only_subpath.as_path())
|
||||
}) {
|
||||
return Err(
|
||||
"windows unelevated restricted-token sandbox cannot reopen writable descendants under read-only carveouts directly; refusing to run unsandboxed"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
if has_reopened_writable_descendant(&split_writable_roots) {
|
||||
return Err(
|
||||
"windows unelevated restricted-token sandbox cannot reopen writable descendants under read-only carveouts directly; refusing to run unsandboxed"
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
let mut additional_deny_write_paths = BTreeSet::new();
|
||||
@@ -247,13 +238,13 @@ pub fn resolve_windows_elevated_filesystem_overrides(
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let _ = workload_env;
|
||||
|
||||
let additional_deny_read_paths = codex_windows_sandbox::resolve_windows_deny_read_paths(
|
||||
&file_system_sandbox_policy,
|
||||
sandbox_policy_cwd,
|
||||
)?;
|
||||
let resolved = file_system_sandbox_policy
|
||||
.resolve_for_enforcement(sandbox_policy_cwd, FileSystemPolicyProjection::Native)
|
||||
.map_err(|err| err.to_string())?;
|
||||
let additional_deny_read_paths =
|
||||
codex_windows_sandbox::expand_windows_deny_read_paths(&resolved, sandbox_policy_cwd)?;
|
||||
|
||||
let split_writable_roots =
|
||||
file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd);
|
||||
let split_writable_roots = resolved.writable_roots;
|
||||
if has_reopened_writable_descendant(&split_writable_roots) {
|
||||
return Err(
|
||||
"windows elevated sandbox cannot reopen writable descendants under read-only carveouts directly; refusing to run unsandboxed"
|
||||
@@ -281,8 +272,8 @@ pub fn resolve_windows_elevated_filesystem_overrides(
|
||||
.iter()
|
||||
.map(|root| normalize_path(root.root.to_path_buf()))
|
||||
.collect();
|
||||
let split_readable_roots: Vec<PathBuf> = file_system_sandbox_policy
|
||||
.get_readable_roots_with_cwd(sandbox_policy_cwd)
|
||||
let split_readable_roots: Vec<PathBuf> = resolved
|
||||
.readable_roots
|
||||
.into_iter()
|
||||
.map(AbsolutePathBuf::into_path_buf)
|
||||
.map(&normalize_path)
|
||||
@@ -362,7 +353,7 @@ pub fn resolve_windows_elevated_filesystem_overrides(
|
||||
|
||||
Ok(Some(WindowsSandboxFilesystemOverrides {
|
||||
read_roots_include_platform_defaults: read_roots_override.is_some()
|
||||
&& file_system_sandbox_policy.include_platform_defaults(),
|
||||
&& resolved.include_platform_defaults,
|
||||
read_roots_override,
|
||||
write_roots_override,
|
||||
additional_deny_read_paths,
|
||||
|
||||
@@ -3,6 +3,7 @@ use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::ReadDenyMatcher;
|
||||
use codex_protocol::permissions::ResolvedFileSystemPermissions;
|
||||
use codex_protocol::permissions::windows_deny_read_glob_scan;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::HashMap;
|
||||
@@ -38,7 +39,27 @@ pub fn resolve_windows_deny_read_paths(
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
cwd: &AbsolutePathBuf,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
resolve_deny_read_paths(file_system_sandbox_policy, cwd, |_| {})
|
||||
expand_deny_read_paths(
|
||||
&file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd),
|
||||
&file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd),
|
||||
file_system_sandbox_policy.glob_scan_max_depth,
|
||||
cwd,
|
||||
|_| {},
|
||||
)
|
||||
}
|
||||
|
||||
/// Expand the denies from an already resolved filesystem policy into Windows ACL targets.
|
||||
pub fn expand_windows_deny_read_paths(
|
||||
permissions: &ResolvedFileSystemPermissions,
|
||||
cwd: &AbsolutePathBuf,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
expand_deny_read_paths(
|
||||
&permissions.unreadable_roots,
|
||||
&permissions.unreadable_globs,
|
||||
permissions.glob_scan_max_depth,
|
||||
cwd,
|
||||
|_| {},
|
||||
)
|
||||
}
|
||||
|
||||
/// Resolve deny paths with the launcher's scanner in an explicit command environment.
|
||||
@@ -49,24 +70,31 @@ pub fn resolve_windows_deny_read_paths_in_environment(
|
||||
env: &HashMap<String, String>,
|
||||
command_cwd: &Path,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
resolve_deny_read_paths(file_system_sandbox_policy, cwd, |command| {
|
||||
command.env_clear().envs(env).current_dir(command_cwd);
|
||||
})
|
||||
expand_deny_read_paths(
|
||||
&file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd),
|
||||
&file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd),
|
||||
file_system_sandbox_policy.glob_scan_max_depth,
|
||||
cwd,
|
||||
|command| {
|
||||
command.env_clear().envs(env).current_dir(command_cwd);
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
fn resolve_deny_read_paths(
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
fn expand_deny_read_paths(
|
||||
unreadable_roots: &[AbsolutePathBuf],
|
||||
unreadable_globs: &[String],
|
||||
glob_scan_max_depth: Option<usize>,
|
||||
cwd: &AbsolutePathBuf,
|
||||
configure: impl Fn(&mut Command),
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
let mut paths = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
|
||||
for path in file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd.as_path()) {
|
||||
push_absolute_path(&mut paths, &mut seen, path.into_path_buf())?;
|
||||
for path in unreadable_roots {
|
||||
push_absolute_path(&mut paths, &mut seen, path.as_path())?;
|
||||
}
|
||||
|
||||
let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd.as_path());
|
||||
if unreadable_globs.is_empty() {
|
||||
return Ok(paths);
|
||||
}
|
||||
@@ -88,10 +116,7 @@ fn resolve_deny_read_paths(
|
||||
return Ok(paths);
|
||||
};
|
||||
|
||||
let scan_plans = glob_scan_plans(
|
||||
&unreadable_globs,
|
||||
file_system_sandbox_policy.glob_scan_max_depth,
|
||||
)?;
|
||||
let scan_plans = glob_scan_plans(unreadable_globs, glob_scan_max_depth)?;
|
||||
|
||||
for scan_plan in scan_plans {
|
||||
if !scan_plan.root.exists() {
|
||||
@@ -200,9 +225,9 @@ fn ripgrep_files(
|
||||
fn push_absolute_path(
|
||||
paths: &mut Vec<AbsolutePathBuf>,
|
||||
seen: &mut HashSet<PathBuf>,
|
||||
path: PathBuf,
|
||||
path: impl AsRef<Path>,
|
||||
) -> Result<(), String> {
|
||||
let absolute_path = AbsolutePathBuf::from_absolute_path(dunce::simplified(&path))
|
||||
let absolute_path = AbsolutePathBuf::from_absolute_path(dunce::simplified(path.as_ref()))
|
||||
.map_err(|err| err.to_string())?;
|
||||
if seen.insert(absolute_path.to_path_buf()) {
|
||||
paths.push(absolute_path);
|
||||
|
||||
@@ -73,7 +73,7 @@ impl GlobWalker<'_> {
|
||||
None => self.matcher.is_local_path_read_denied(path),
|
||||
};
|
||||
if denied {
|
||||
push_absolute_path(self.paths, self.seen_paths, path.to_path_buf())?;
|
||||
push_absolute_path(self.paths, self.seen_paths, path)?;
|
||||
}
|
||||
if !file_type.is_dir()
|
||||
|| self.max_depth.is_some_and(|limit| depth >= limit)
|
||||
|
||||
@@ -266,6 +266,7 @@ pub use deny_read_acl::apply_deny_read_acls;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use deny_read_acl::plan_deny_read_acl_paths;
|
||||
pub use deny_read_resolver::GLOB_SCAN_PROGRAM;
|
||||
pub use deny_read_resolver::expand_windows_deny_read_paths;
|
||||
pub use deny_read_resolver::resolve_windows_deny_read_paths;
|
||||
pub use deny_read_resolver::resolve_windows_deny_read_paths_in_environment;
|
||||
#[cfg(target_os = "windows")]
|
||||
|
||||
@@ -26,6 +26,7 @@ import subprocess
|
||||
import sys
|
||||
import tomllib
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from datetime import UTC, datetime
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -155,28 +156,68 @@ def test_packaged_executor_initializes_and_enforces_sandbox(
|
||||
assert reported_environment["executorVersion"] == metadata["version"]
|
||||
process.stdin.write('{"method":"initialized","params":{}}\n')
|
||||
|
||||
# Restricted writes always launch the filesystem helper, even when allowed.
|
||||
for request_id, path in [(2, workspace / "allowed.txt"), (3, blocked)]:
|
||||
def file_request(
|
||||
request_id: int,
|
||||
method: str,
|
||||
path: Path,
|
||||
*,
|
||||
timeout: int = 45,
|
||||
**params: str,
|
||||
) -> dict:
|
||||
process.stdin.write(
|
||||
json.dumps(
|
||||
{
|
||||
"id": request_id,
|
||||
"method": "fs/writeFile",
|
||||
"method": method,
|
||||
"params": {
|
||||
"path": path.as_uri(),
|
||||
"dataBase64": "b2s=",
|
||||
"sandbox": sandbox,
|
||||
**params,
|
||||
},
|
||||
}
|
||||
)
|
||||
+ "\n"
|
||||
)
|
||||
process.stdin.flush()
|
||||
return json.loads(
|
||||
reader.submit(process.stdout.readline).result(timeout=timeout)
|
||||
)
|
||||
|
||||
if windows:
|
||||
# Resolving the inner helper creates today's log. The first ACL scan must see it.
|
||||
log_dir = home / ".sandbox"
|
||||
log_dir.mkdir(exist_ok=True)
|
||||
sibling = log_dir / "allowed.txt"
|
||||
sibling.write_bytes(b"ok")
|
||||
day = datetime.now(UTC).date()
|
||||
log = log_dir / f"sandbox.{day}.log"
|
||||
assert not log.exists()
|
||||
sandbox["permissions"]["file_system"]["entries"].append(
|
||||
{
|
||||
"path": {
|
||||
"type": "glob_pattern",
|
||||
"pattern": (log_dir / "*.log").as_posix(),
|
||||
},
|
||||
"access": "deny",
|
||||
}
|
||||
)
|
||||
# The first Windows helper can provision sandbox accounts before replying.
|
||||
response = json.loads(
|
||||
reader.submit(process.stdout.readline).result(
|
||||
timeout=180 if windows and request_id == 2 else 45
|
||||
)
|
||||
denied = file_request(5, "fs/readFile", log, timeout=180)
|
||||
if not log.exists() and day != datetime.now(UTC).date():
|
||||
# The helper crossed UTC midnight before creating its log.
|
||||
log = next(log_dir.glob("sandbox.*.log"))
|
||||
denied = file_request(7, "fs/readFile", log)
|
||||
assert "Access is denied" in denied.get("error", {}).get("message", "")
|
||||
assert log.is_file()
|
||||
assert file_request(6, "fs/readFile", sibling) == {
|
||||
"id": 6,
|
||||
"result": {"dataBase64": "b2s="},
|
||||
}
|
||||
|
||||
# Restricted writes always launch the filesystem helper, even when allowed.
|
||||
for request_id, path in [(2, workspace / "allowed.txt"), (3, blocked)]:
|
||||
response = file_request(
|
||||
request_id, "fs/writeFile", path, dataBase64="b2s="
|
||||
)
|
||||
if request_id == 2:
|
||||
assert response == {"id": 2, "result": {}}, response
|
||||
|
||||
Reference in New Issue
Block a user