protocol: derive effective file access from filesystem policies (#13440)

## Why

`#13434` and `#13439` introduce split filesystem and network policies,
but the only code that could answer basic filesystem questions like "is
access effectively unrestricted?" or "which roots are readable and
writable for this cwd?" still lived on the legacy `SandboxPolicy` path.

That would force later backends to either keep projecting through
`SandboxPolicy` or duplicate path-resolution logic. This PR moves those
queries onto `FileSystemSandboxPolicy` itself so later runtime and
platform changes can consume the split policy directly.

## What changed

- added `FileSystemSandboxPolicy` helpers for full-read/full-write
checks, platform-default reads, readable roots, writable roots, and
explicit unreadable roots resolved against a cwd
- added a shared helper for the default read-only carveouts under
writable roots so the legacy and split-policy paths stay aligned
- added protocol coverage for full-access detection and derived
readable, writable, and unreadable roots

## Verification

- added protocol coverage in `protocol/src/protocol.rs` and
`protocol/src/permissions.rs` for full-root access and derived
filesystem roots
- 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/13440).
* #13453
* #13452
* #13451
* #13449
* #13448
* #13445
* __->__ #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
This commit is contained in:
Michael Bolin
2026-03-06 19:49:29 -08:00
committed by GitHub
parent 22ac6b9aaa
commit b52c18e414
2 changed files with 479 additions and 40 deletions

View File

@@ -61,6 +61,13 @@ pub use crate::approvals::NetworkApprovalContext;
pub use crate::approvals::NetworkApprovalProtocol;
pub use crate::approvals::NetworkPolicyAmendment;
pub use crate::approvals::NetworkPolicyRuleAction;
pub use crate::permissions::FileSystemAccessMode;
pub use crate::permissions::FileSystemPath;
pub use crate::permissions::FileSystemSandboxEntry;
pub use crate::permissions::FileSystemSandboxKind;
pub use crate::permissions::FileSystemSandboxPolicy;
pub use crate::permissions::FileSystemSpecialPath;
pub use crate::permissions::NetworkSandboxPolicy;
pub use crate::request_user_input::RequestUserInputEvent;
/// Open/close tags for special user-input blocks. Used across crates to avoid
@@ -542,7 +549,6 @@ impl NetworkAccess {
matches!(self, NetworkAccess::Enabled)
}
}
fn default_include_platform_defaults() -> bool {
true
}
@@ -883,45 +889,11 @@ impl SandboxPolicy {
// For each root, compute subpaths that should remain read-only.
roots
.into_iter()
.map(|writable_root| {
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
#[allow(clippy::expect_used)]
let top_level_git = writable_root
.join(".git")
.expect(".git is a valid relative path");
// This applies to typical repos (directory .git), worktrees/submodules
// (file .git with gitdir pointer), and bare repos when the gitdir is the
// writable root itself.
let top_level_git_is_file = top_level_git.as_path().is_file();
let top_level_git_is_dir = top_level_git.as_path().is_dir();
if top_level_git_is_dir || top_level_git_is_file {
if top_level_git_is_file
&& is_git_pointer_file(&top_level_git)
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
&& !subpaths
.iter()
.any(|subpath| subpath.as_path() == gitdir.as_path())
{
subpaths.push(gitdir);
}
subpaths.push(top_level_git);
}
// Make .agents/skills and .codex/config.toml and
// related files read-only to the agent, by default.
for subdir in &[".agents", ".codex"] {
#[allow(clippy::expect_used)]
let top_level_codex =
writable_root.join(subdir).expect("valid relative path");
if top_level_codex.as_path().is_dir() {
subpaths.push(top_level_codex);
}
}
WritableRoot {
root: writable_root,
read_only_subpaths: subpaths,
}
.map(|writable_root| WritableRoot {
read_only_subpaths: default_read_only_subpaths_for_writable_root(
&writable_root,
),
root: writable_root,
})
.collect()
}
@@ -929,6 +901,49 @@ impl SandboxPolicy {
}
}
fn default_read_only_subpaths_for_writable_root(
writable_root: &AbsolutePathBuf,
) -> Vec<AbsolutePathBuf> {
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
#[allow(clippy::expect_used)]
let top_level_git = writable_root
.join(".git")
.expect(".git is a valid relative path");
// This applies to typical repos (directory .git), worktrees/submodules
// (file .git with gitdir pointer), and bare repos when the gitdir is the
// writable root itself.
let top_level_git_is_file = top_level_git.as_path().is_file();
let top_level_git_is_dir = top_level_git.as_path().is_dir();
if top_level_git_is_dir || top_level_git_is_file {
if top_level_git_is_file
&& is_git_pointer_file(&top_level_git)
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
{
subpaths.push(gitdir);
}
subpaths.push(top_level_git);
}
// Make .agents/skills and .codex/config.toml and related files read-only
// to the agent, by default.
for subdir in &[".agents", ".codex"] {
#[allow(clippy::expect_used)]
let top_level_codex = writable_root.join(subdir).expect("valid relative path");
if top_level_codex.as_path().is_dir() {
subpaths.push(top_level_codex);
}
}
let mut deduped = Vec::with_capacity(subpaths.len());
let mut seen = HashSet::new();
for path in subpaths {
if seen.insert(path.to_path_buf()) {
deduped.push(path);
}
}
deduped
}
fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {
path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git"))
}
@@ -3156,11 +3171,68 @@ mod tests {
use crate::permissions::FileSystemPath;
use crate::permissions::FileSystemSandboxEntry;
use crate::permissions::FileSystemSandboxPolicy;
use crate::permissions::FileSystemSpecialPath;
use crate::permissions::NetworkSandboxPolicy;
use anyhow::Result;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::NamedTempFile;
use tempfile::TempDir;
fn sorted_paths(paths: Vec<AbsolutePathBuf>) -> Vec<PathBuf> {
let mut sorted: Vec<PathBuf> = paths.into_iter().map(|path| path.to_path_buf()).collect();
sorted.sort();
sorted
}
fn sorted_writable_roots(roots: Vec<WritableRoot>) -> Vec<(PathBuf, Vec<PathBuf>)> {
let mut sorted_roots: Vec<(PathBuf, Vec<PathBuf>)> = roots
.into_iter()
.map(|root| {
let mut read_only_subpaths: Vec<PathBuf> = root
.read_only_subpaths
.into_iter()
.map(|path| path.to_path_buf())
.collect();
read_only_subpaths.sort();
(root.root.to_path_buf(), read_only_subpaths)
})
.collect();
sorted_roots.sort_by(|left, right| left.0.cmp(&right.0));
sorted_roots
}
fn assert_same_sandbox_policy_semantics(
expected: &SandboxPolicy,
actual: &SandboxPolicy,
cwd: &Path,
) {
assert_eq!(
actual.has_full_disk_read_access(),
expected.has_full_disk_read_access()
);
assert_eq!(
actual.has_full_disk_write_access(),
expected.has_full_disk_write_access()
);
assert_eq!(
actual.has_full_network_access(),
expected.has_full_network_access()
);
assert_eq!(
actual.include_platform_defaults(),
expected.include_platform_defaults()
);
assert_eq!(
sorted_paths(actual.get_readable_roots_with_cwd(cwd)),
sorted_paths(expected.get_readable_roots_with_cwd(cwd))
);
assert_eq!(
sorted_writable_roots(actual.get_writable_roots_with_cwd(cwd)),
sorted_writable_roots(expected.get_writable_roots_with_cwd(cwd))
);
}
#[test]
fn external_sandbox_reports_full_access_flags() {
@@ -3241,6 +3313,97 @@ mod tests {
}
}
#[test]
fn restricted_file_system_policy_reports_full_access_from_root_entries() {
let read_only = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
}]);
assert!(read_only.has_full_disk_read_access());
assert!(!read_only.has_full_disk_write_access());
assert!(!read_only.include_platform_defaults());
let writable = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
}]);
assert!(writable.has_full_disk_read_access());
assert!(writable.has_full_disk_write_access());
}
#[test]
fn restricted_file_system_policy_derives_effective_paths() {
let cwd = TempDir::new().expect("tempdir");
std::fs::create_dir_all(cwd.path().join(".agents")).expect("create .agents");
std::fs::create_dir_all(cwd.path().join(".codex")).expect("create .codex");
let cwd_absolute =
AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir");
let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path())
.expect("resolve unreadable path");
let agents = AbsolutePathBuf::resolve_path_against_base(".agents", cwd.path())
.expect("resolve .agents");
let codex = AbsolutePathBuf::resolve_path_against_base(".codex", cwd.path())
.expect("resolve .codex");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Minimal,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: secret.clone(),
},
access: FileSystemAccessMode::None,
},
]);
assert!(!policy.has_full_disk_read_access());
assert!(!policy.has_full_disk_write_access());
assert!(policy.include_platform_defaults());
assert_eq!(
policy.get_readable_roots_with_cwd(cwd.path()),
vec![cwd_absolute]
);
assert_eq!(
policy.get_unreadable_roots_with_cwd(cwd.path()),
vec![secret.clone()]
);
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
assert_eq!(writable_roots.len(), 1);
assert_eq!(writable_roots[0].root.as_path(), cwd.path());
assert!(
writable_roots[0]
.read_only_subpaths
.iter()
.any(|path| path.as_path() == secret.as_path())
);
assert!(
writable_roots[0]
.read_only_subpaths
.iter()
.any(|path| path.as_path() == agents.as_path())
);
assert!(
writable_roots[0]
.read_only_subpaths
.iter()
.any(|path| path.as_path() == codex.as_path())
);
}
#[test]
fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() {
let cwd = if cfg!(windows) {
@@ -3271,6 +3434,60 @@ mod tests {
);
}
#[test]
fn legacy_sandbox_policy_semantics_survive_split_bridge() {
let cwd = TempDir::new().expect("tempdir");
let readable_root = AbsolutePathBuf::resolve_path_against_base("readable", cwd.path())
.expect("resolve readable root");
let writable_root = AbsolutePathBuf::resolve_path_against_base("writable", cwd.path())
.expect("resolve writable root");
let policies = [
SandboxPolicy::DangerFullAccess,
SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
},
SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Enabled,
},
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::FullAccess,
network_access: false,
},
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![readable_root.clone()],
},
network_access: true,
},
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::FullAccess,
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![writable_root],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![readable_root],
},
network_access: true,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: true,
},
];
for expected in policies {
let actual = FileSystemSandboxPolicy::from(&expected)
.to_legacy_sandbox_policy(NetworkSandboxPolicy::from(&expected), cwd.path())
.expect("legacy bridge should preserve legacy policy semantics");
assert_same_sandbox_policy_semantics(&expected, &actual, cwd.path());
}
}
#[test]
fn item_started_event_from_web_search_emits_begin_event() {
let event = ItemStartedEvent {