Compare commits

...

2 Commits

Author SHA1 Message Date
sayan-oai
db4b04fec4 Update models.json 2026-03-08 05:24:15 +00:00
Michael Bolin
3b5fe5ca35 protocol: keep root carveouts sandboxed (#13452)
## Why

A restricted filesystem policy that grants `:root` read or write access
but also carries explicit deny entries should still behave like scoped
access with carveouts, not like unrestricted disk access.

Without that distinction, later platform backends cannot preserve
blocked subpaths under root-level permissions because the protocol layer
reports the policy as fully unrestricted.

## What changed

- taught `FileSystemSandboxPolicy` to treat root access plus explicit
deny entries as scoped access rather than full-disk access
- derived readable and writable roots from the filesystem root when root
access is combined with carveouts, while preserving the denied paths as
read-only subpaths
- added protocol coverage for root-write policies with carveouts and a
core sandboxing regression so those policies still require platform
sandboxing

## Verification

- added protocol coverage in `protocol/src/permissions.rs` and
`protocol/src/protocol.rs` for root access with explicit carveouts
- added platform-sandbox regression coverage in
`core/src/sandboxing/mod.rs`
- verified the current PR state with `just clippy`




---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13452).
* #13453
* __->__ #13452
* #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
2026-03-07 21:15:47 -08:00
4 changed files with 187 additions and 37 deletions

File diff suppressed because one or more lines are too long

View File

@@ -673,6 +673,32 @@ mod tests {
);
}
#[test]
fn root_write_policy_with_carveouts_still_uses_platform_sandbox() {
let blocked = AbsolutePathBuf::resolve_path_against_base(
"blocked",
std::env::current_dir().expect("current dir"),
)
.expect("blocked path");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: blocked },
access: FileSystemAccessMode::None,
},
]);
assert_eq!(
should_require_platform_sandbox(&policy, NetworkSandboxPolicy::Enabled, false),
true
);
}
#[test]
fn full_access_restricted_policy_still_uses_platform_sandbox_for_restricted_network() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {

View File

@@ -123,6 +123,25 @@ impl Default for FileSystemSandboxPolicy {
}
impl FileSystemSandboxPolicy {
fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool {
matches!(self.kind, FileSystemSandboxKind::Restricted)
&& self.entries.iter().any(|entry| {
matches!(
&entry.path,
FileSystemPath::Special { value }
if matches!(value, FileSystemSpecialPath::Root) && predicate(entry.access)
)
})
}
fn has_explicit_deny_entries(&self) -> bool {
matches!(self.kind, FileSystemSandboxKind::Restricted)
&& self
.entries
.iter()
.any(|entry| entry.access == FileSystemAccessMode::None)
}
pub fn unrestricted() -> Self {
Self {
kind: FileSystemSandboxKind::Unrestricted,
@@ -148,13 +167,10 @@ impl FileSystemSandboxPolicy {
pub fn has_full_disk_read_access(&self) -> bool {
match self.kind {
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
matches!(
&entry.path,
FileSystemPath::Special { value }
if matches!(value, FileSystemSpecialPath::Root) && entry.access.can_read()
)
}),
FileSystemSandboxKind::Restricted => {
self.has_root_access(FileSystemAccessMode::can_read)
&& !self.has_explicit_deny_entries()
}
}
}
@@ -162,14 +178,10 @@ impl FileSystemSandboxPolicy {
pub fn has_full_disk_write_access(&self) -> bool {
match self.kind {
FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
matches!(
&entry.path,
FileSystemPath::Special { value }
if matches!(value, FileSystemSpecialPath::Root)
&& entry.access.can_write()
)
}),
FileSystemSandboxKind::Restricted => {
self.has_root_access(FileSystemAccessMode::can_write)
&& !self.has_explicit_deny_entries()
}
}
}
@@ -194,11 +206,24 @@ impl FileSystemSandboxPolicy {
}
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
let mut readable_roots = Vec::new();
if self.has_root_access(FileSystemAccessMode::can_read)
&& let Some(cwd_absolute) = cwd_absolute.as_ref()
{
readable_roots.push(absolute_root_path_for_cwd(cwd_absolute));
}
dedup_absolute_paths(
self.entries
.iter()
.filter(|entry| entry.access.can_read())
.filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
readable_roots
.into_iter()
.chain(
self.entries
.iter()
.filter(|entry| entry.access.can_read())
.filter_map(|entry| {
resolve_file_system_path(&entry.path, cwd_absolute.as_ref())
}),
)
.collect(),
)
}
@@ -212,11 +237,24 @@ impl FileSystemSandboxPolicy {
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
let unreadable_roots = self.get_unreadable_roots_with_cwd(cwd);
let mut writable_roots = Vec::new();
if self.has_root_access(FileSystemAccessMode::can_write)
&& let Some(cwd_absolute) = cwd_absolute.as_ref()
{
writable_roots.push(absolute_root_path_for_cwd(cwd_absolute));
}
dedup_absolute_paths(
self.entries
.iter()
.filter(|entry| entry.access.can_write())
.filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
writable_roots
.into_iter()
.chain(
self.entries
.iter()
.filter(|entry| entry.access.can_write())
.filter_map(|entry| {
resolve_file_system_path(&entry.path, cwd_absolute.as_ref())
}),
)
.collect(),
)
.into_iter()
@@ -543,6 +581,16 @@ fn resolve_file_system_path(
}
}
fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf {
let root = cwd
.as_path()
.ancestors()
.last()
.unwrap_or_else(|| panic!("cwd must have a filesystem root"));
AbsolutePathBuf::from_absolute_path(root)
.unwrap_or_else(|err| panic!("cwd root must be an absolute path: {err}"))
}
fn resolve_file_system_special_path(
value: &FileSystemSpecialPath,
cwd: Option<&AbsolutePathBuf>,

View File

@@ -3352,6 +3352,56 @@ mod tests {
assert!(writable.has_full_disk_write_access());
}
#[test]
fn restricted_file_system_policy_treats_root_with_carveouts_as_scoped_access() {
let cwd = TempDir::new().expect("tempdir");
let cwd_absolute =
AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir");
let root = cwd_absolute
.as_path()
.ancestors()
.last()
.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
.expect("filesystem root");
let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path())
.expect("resolve blocked");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: blocked.clone(),
},
access: FileSystemAccessMode::None,
},
]);
assert!(!policy.has_full_disk_read_access());
assert!(!policy.has_full_disk_write_access());
assert_eq!(
policy.get_readable_roots_with_cwd(cwd.path()),
vec![root.clone()]
);
assert_eq!(
policy.get_unreadable_roots_with_cwd(cwd.path()),
vec![blocked.clone()]
);
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
assert_eq!(writable_roots.len(), 1);
assert_eq!(writable_roots[0].root, root);
assert!(
writable_roots[0]
.read_only_subpaths
.iter()
.any(|path| path.as_path() == blocked.as_path())
);
}
#[test]
fn restricted_file_system_policy_derives_effective_paths() {
let cwd = TempDir::new().expect("tempdir");