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

Reserve missing preserved sandbox paths

Browse Source
This commit is contained in:
Eva Wong
2026-04-17 15:54:56 -07:00
parent 4ed22fc7d2
commit 90d5f4304f
6 changed files with 627 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

302
codex-rs/linux-sandbox/src/bwrap.rs
View File

@@ -15,9 +15,11 @@ use std::collections::HashSet;
use std::ffi::OsString;
use std::fs;
use std::fs::File;
use std::fs::Metadata;
use std::io;
use std::os::fd::AsRawFd;
use std::os::unix::ffi::OsStringExt;
use std::os::unix::fs::MetadataExt;
use std::path::Path;
use std::path::PathBuf;
use std::process::Command;
@@ -101,6 +103,61 @@ impl BwrapNetworkMode {
pub(crate) struct BwrapArgs {
pub args: Vec<String>,
pub preserved_files: Vec<File>,
pub synthetic_mount_targets: Vec<SyntheticMountTarget>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
struct FileIdentity {
dev: u64,
ino: u64,
}
impl FileIdentity {
fn from_metadata(metadata: &Metadata) -> Self {
Self {
dev: metadata.dev(),
ino: metadata.ino(),
}
}
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct SyntheticMountTarget {
path: PathBuf,
// If an empty preserved path was already present, remember its inode so
// cleanup does not delete a real pre-existing file.
pre_existing_file: Option<FileIdentity>,
}
impl SyntheticMountTarget {
pub(crate) fn missing(path: &Path) -> Self {
Self {
path: path.to_path_buf(),
pre_existing_file: None,
}
}
fn existing_empty_file(path: &Path, metadata: &Metadata) -> Self {
Self {
path: path.to_path_buf(),
pre_existing_file: Some(FileIdentity::from_metadata(metadata)),
}
}
pub(crate) fn path(&self) -> &Path {
&self.path
}
pub(crate) fn should_remove_after_bwrap(&self, metadata: &Metadata) -> bool {
if !metadata.file_type().is_file() || metadata.len() != 0 {
return false;
}
match self.pre_existing_file {
Some(pre_existing_file) => pre_existing_file != FileIdentity::from_metadata(metadata),
None => true,
}
}
}
/// Wrap a command with bubblewrap so the filesystem is read-only by default,
@@ -126,6 +183,7 @@ pub(crate) fn create_bwrap_command_args(
Ok(BwrapArgs {
args: command,
preserved_files: Vec::new(),
synthetic_mount_targets: Vec::new(),
})
} else {
Ok(create_bwrap_flags_full_filesystem(command, options))
@@ -165,6 +223,7 @@ fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOption
BwrapArgs {
args,
preserved_files: Vec::new(),
synthetic_mount_targets: Vec::new(),
}
}
@@ -179,6 +238,7 @@ fn create_bwrap_flags(
let BwrapArgs {
args: filesystem_args,
preserved_files,
synthetic_mount_targets,
} = create_filesystem_args(
file_system_sandbox_policy,
sandbox_policy_cwd,
@@ -216,6 +276,7 @@ fn create_bwrap_flags(
Ok(BwrapArgs {
args,
preserved_files,
synthetic_mount_targets,
})
}
@@ -348,6 +409,7 @@ fn create_filesystem_args(
args
};
let mut preserved_files = Vec::new();
let mut synthetic_mount_targets = Vec::new();
let mut allowed_write_paths = Vec::with_capacity(writable_roots.len());
for writable_root in &writable_roots {
let root = writable_root.root.as_path();
@@ -381,6 +443,7 @@ fn create_filesystem_args(
append_unreadable_root_args(
&mut args,
&mut preserved_files,
&mut synthetic_mount_targets,
unreadable_root,
&allowed_write_paths,
)?;
@@ -416,7 +479,13 @@ fn create_filesystem_args(
}
read_only_subpaths.sort_by_key(|path| path_depth(path));
for subpath in read_only_subpaths {
append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths)?;
append_read_only_subpath_args(
&mut args,
&mut preserved_files,
&mut synthetic_mount_targets,
&subpath,
&allowed_write_paths,
)?;
}
let mut nested_unreadable_roots: Vec<PathBuf> = unreadable_roots
.iter()
@@ -432,6 +501,7 @@ fn create_filesystem_args(
append_unreadable_root_args(
&mut args,
&mut preserved_files,
&mut synthetic_mount_targets,
&unreadable_root,
&allowed_write_paths,
)?;
@@ -453,6 +523,7 @@ fn create_filesystem_args(
append_unreadable_root_args(
&mut args,
&mut preserved_files,
&mut synthetic_mount_targets,
&unreadable_root,
&allowed_write_paths,
)?;
@@ -461,6 +532,7 @@ fn create_filesystem_args(
Ok(BwrapArgs {
args,
preserved_files,
synthetic_mount_targets,
})
}
@@ -787,6 +859,8 @@ fn append_mount_target_parent_dir_args(args: &mut Vec<String>, mount_target: &Pa
fn append_read_only_subpath_args(
args: &mut Vec<String>,
preserved_files: &mut Vec<File>,
synthetic_mount_targets: &mut Vec<SyntheticMountTarget>,
subpath: &Path,
allowed_write_paths: &[PathBuf],
) -> Result<()> {
@@ -804,13 +878,32 @@ fn append_read_only_subpath_args(
)));
}
if let Some(metadata) = transient_empty_preserved_file_metadata(subpath)
&& is_within_allowed_write_paths(subpath, allowed_write_paths)
{
// Another concurrent bwrap setup can leave a zero-byte mount target at
// a missing preserved path. Treat it like the missing case instead of
// binding that transient host path as the stable source.
append_existing_empty_file_bind_data_args(
args,
preserved_files,
synthetic_mount_targets,
subpath,
&metadata,
)?;
return Ok(());
}
if !subpath.exists() {
if let Some(first_missing_component) = find_first_non_existent_component(subpath)
&& is_within_allowed_write_paths(&first_missing_component, allowed_write_paths)
{
args.push("--ro-bind".to_string());
args.push("/dev/null".to_string());
args.push(path_to_string(&first_missing_component));
append_missing_empty_file_bind_data_args(
args,
preserved_files,
synthetic_mount_targets,
&first_missing_component,
)?;
}
return Ok(());
}
@@ -823,9 +916,48 @@ fn append_read_only_subpath_args(
Ok(())
}
fn append_empty_file_bind_data_args(
args: &mut Vec<String>,
preserved_files: &mut Vec<File>,
path: &Path,
) -> Result<()> {
if preserved_files.is_empty() {
preserved_files.push(File::open("/dev/null")?);
}
let null_fd = preserved_files[0].as_raw_fd().to_string();
args.push("--ro-bind-data".to_string());
args.push(null_fd);
args.push(path_to_string(path));
Ok(())
}
fn append_missing_empty_file_bind_data_args(
args: &mut Vec<String>,
preserved_files: &mut Vec<File>,
synthetic_mount_targets: &mut Vec<SyntheticMountTarget>,
path: &Path,
) -> Result<()> {
append_empty_file_bind_data_args(args, preserved_files, path)?;
synthetic_mount_targets.push(SyntheticMountTarget::missing(path));
Ok(())
}
fn append_existing_empty_file_bind_data_args(
args: &mut Vec<String>,
preserved_files: &mut Vec<File>,
synthetic_mount_targets: &mut Vec<SyntheticMountTarget>,
path: &Path,
metadata: &Metadata,
) -> Result<()> {
append_empty_file_bind_data_args(args, preserved_files, path)?;
synthetic_mount_targets.push(SyntheticMountTarget::existing_empty_file(path, metadata));
Ok(())
}
fn append_unreadable_root_args(
args: &mut Vec<String>,
preserved_files: &mut Vec<File>,
synthetic_mount_targets: &mut Vec<SyntheticMountTarget>,
unreadable_root: &Path,
allowed_write_paths: &[PathBuf],
) -> Result<()> {
@@ -850,9 +982,12 @@ fn append_unreadable_root_args(
if let Some(first_missing_component) = find_first_non_existent_component(unreadable_root)
&& is_within_allowed_write_paths(&first_missing_component, allowed_write_paths)
{
args.push("--ro-bind".to_string());
args.push("/dev/null".to_string());
args.push(path_to_string(&first_missing_component));
append_missing_empty_file_bind_data_args(
args,
preserved_files,
synthetic_mount_targets,
&first_missing_component,
)?;
}
return Ok(());
}
@@ -901,16 +1036,9 @@ fn append_existing_unreadable_path_args(
return Ok(());
}
if preserved_files.is_empty() {
preserved_files.push(File::open("/dev/null")?);
}
let null_fd = preserved_files[0].as_raw_fd().to_string();
args.push("--perms".to_string());
args.push("000".to_string());
args.push("--ro-bind-data".to_string());
args.push(null_fd);
args.push(path_to_string(unreadable_root));
Ok(())
append_empty_file_bind_data_args(args, preserved_files, unreadable_root)
}
/// Returns true when `path` is under any allowed writable root.
@@ -920,6 +1048,24 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) -
.any(|root| path.starts_with(root))
}
fn transient_empty_preserved_file_metadata(path: &Path) -> Option<Metadata> {
if !has_preserved_path_name(path) {
return None;
}
let metadata = fs::symlink_metadata(path).ok()?;
if metadata.file_type().is_file() && metadata.len() == 0 {
Some(metadata)
} else {
None
}
}
fn has_preserved_path_name(path: &Path) -> bool {
path.file_name()
.is_some_and(|name| name == ".git" || name == ".agents" || name == ".codex")
}
fn first_writable_symlink_component_in_path(
target_path: &Path,
allowed_write_paths: &[PathBuf],
@@ -1358,6 +1504,80 @@ mod tests {
assert!(message.contains(&real_linked_private_str), "{message}");
}
#[test]
fn missing_read_only_subpath_uses_empty_file_bind_data() {
let temp_dir = TempDir::new().expect("temp dir");
let workspace = temp_dir.path().join("workspace");
let blocked = workspace.join("blocked");
std::fs::create_dir_all(&workspace).expect("create workspace");
let workspace_root =
AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace");
let blocked_root = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: workspace_root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: blocked_root },
access: FileSystemAccessMode::Read,
},
]);
let args =
create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
.expect("filesystem args");
assert_empty_file_bound_without_perms(&args.args, &blocked);
assert_eq!(args.preserved_files.len(), 1);
assert_eq!(synthetic_mount_target_paths(&args), vec![blocked.clone()]);
assert!(
!blocked.exists(),
"missing path mask should not materialize host-side preserved paths at arg construction time",
);
}
#[test]
fn transient_empty_preserved_file_uses_empty_file_bind_data() {
let temp_dir = TempDir::new().expect("temp dir");
let workspace = temp_dir.path().join("workspace");
let dot_git = workspace.join(".git");
std::fs::create_dir_all(&workspace).expect("create workspace");
File::create(&dot_git).expect("create empty .git file");
let workspace_root =
AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: workspace_root,
},
access: FileSystemAccessMode::Write,
}]);
let args =
create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
.expect("filesystem args");
let dot_git_str = path_to_string(&dot_git);
assert_empty_file_bound_without_perms(&args.args, &dot_git);
assert_eq!(synthetic_mount_target_paths(&args), vec![dot_git.clone()]);
assert!(
!args
.args
.windows(3)
.any(|window| window == ["--ro-bind", dot_git_str.as_str(), dot_git_str.as_str()]),
"transient empty preserved file should not be treated as a stable bind source",
);
let metadata = std::fs::symlink_metadata(&dot_git).expect("stat .git");
assert!(
!args.synthetic_mount_targets[0].should_remove_after_bwrap(&metadata),
"pre-existing empty preserved files must not be cleaned up as synthetic targets",
);
}
#[test]
fn ignores_missing_writable_roots() {
let temp_dir = TempDir::new().expect("temp dir");
@@ -1411,6 +1631,16 @@ mod tests {
NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH,
)
.expect("bwrap fs args");
assert_eq!(args.preserved_files.len(), 1);
assert_eq!(
synthetic_mount_target_paths(&args),
vec![
PathBuf::from("/.git"),
PathBuf::from("/.agents"),
PathBuf::from("/.codex"),
]
);
let null_fd = args.preserved_files[0].as_raw_fd().to_string();
assert_eq!(
args.args,
vec![
@@ -1425,11 +1655,17 @@ mod tests {
"--bind".to_string(),
"/".to_string(),
"/".to_string(),
// Mask the default protected .codex subpath under that writable
// Mask the default preserved .codex subpath under that writable
// root. Because the root is `/` in this test, the carveout path
// appears as `/.codex`.
"--ro-bind".to_string(),
"/dev/null".to_string(),
// appears as `/.codex`, `/.agents`, and `/.git`.
"--ro-bind-data".to_string(),
null_fd.clone(),
"/.git".to_string(),
"--ro-bind-data".to_string(),
null_fd.clone(),
"/.agents".to_string(),
"--ro-bind-data".to_string(),
null_fd,
"/.codex".to_string(),
// Rebind /dev after the root bind so device nodes remain
// writable/usable inside the writable root.
@@ -1899,6 +2135,7 @@ mod tests {
let blocked_file_str = path_to_string(blocked_file.as_path());
assert_eq!(args.preserved_files.len(), 1);
assert!(args.synthetic_mount_targets.is_empty());
assert!(args.args.windows(5).any(|window| {
window[0] == "--perms"
&& window[1] == "000"
@@ -2002,4 +2239,31 @@ mod tests {
"expected file mask for {path}: {args:#?}"
);
}
/// Assert that `path` is backed by an fd-supplied empty file without
/// changing the next mount operation's permissions.
fn assert_empty_file_bound_without_perms(args: &[String], path: &Path) {
let path = path_to_string(path);
assert!(
args.windows(3)
.any(|window| { window[0] == "--ro-bind-data" && window[2] == path }),
"expected empty file bind for {path}: {args:#?}"
);
assert!(
!args.windows(5).any(|window| {
window[0] == "--perms"
&& window[1] == "000"
&& window[2] == "--ro-bind-data"
&& window[4] == path
}),
"missing path bind should not set explicit file perms for {path}: {args:#?}"
);
}
fn synthetic_mount_target_paths(args: &BwrapArgs) -> Vec<PathBuf> {
args.synthetic_mount_targets
.iter()
.map(|target| target.path().to_path_buf())
.collect()
}
}

78
codex-rs/linux-sandbox/src/linux_run_main.rs
View File

@@ -1,6 +1,7 @@
use clap::Parser;
use std::ffi::CString;
use std::fmt;
use std::fs;
use std::fs::File;
use std::io::Read;
use std::os::fd::FromRawFd;
@@ -443,7 +444,7 @@ fn run_bwrap_with_proc_fallback(
options,
);
apply_inner_command_argv0(&mut bwrap_args.args);
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
run_or_exec_bwrap(bwrap_args);
}
fn bwrap_network_mode(
@@ -480,6 +481,7 @@ fn build_bwrap_argv(
crate::bwrap::BwrapArgs {
args: argv,
preserved_files: bwrap_args.preserved_files,
synthetic_mount_targets: bwrap_args.synthetic_mount_targets,
}
}
@@ -568,6 +570,78 @@ fn resolve_true_command() -> String {
"true".to_string()
}
fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
if bwrap_args.synthetic_mount_targets.is_empty() {
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}
run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args);
}
fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
let synthetic_mount_targets = bwrap_args.synthetic_mount_targets.clone();
let pid = unsafe { libc::fork() };
if pid < 0 {
let err = std::io::Error::last_os_error();
panic!("failed to fork for bubblewrap: {err}");
}
if pid == 0 {
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}
let mut status: libc::c_int = 0;
let wait_res = unsafe { libc::waitpid(pid, &mut status as *mut libc::c_int, 0) };
if wait_res < 0 {
let err = std::io::Error::last_os_error();
panic!("waitpid failed for bubblewrap child: {err}");
}
cleanup_synthetic_mount_targets(&synthetic_mount_targets);
exit_with_wait_status(status);
}
fn cleanup_synthetic_mount_targets(targets: &[crate::bwrap::SyntheticMountTarget]) {
for target in targets.iter().rev() {
let path = target.path();
let metadata = match fs::symlink_metadata(path) {
Ok(metadata) => metadata,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
Err(err) => panic!(
"failed to inspect synthetic bubblewrap mount target {}: {err}",
path.display()
),
};
if !target.should_remove_after_bwrap(&metadata) {
continue;
}
match fs::remove_file(path) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => panic!(
"failed to remove synthetic bubblewrap mount target {}: {err}",
path.display()
),
}
}
}
fn exit_with_wait_status(status: libc::c_int) -> ! {
if libc::WIFEXITED(status) {
std::process::exit(libc::WEXITSTATUS(status));
}
if libc::WIFSIGNALED(status) {
let signal = libc::WTERMSIG(status);
unsafe {
libc::signal(signal, libc::SIG_DFL);
libc::kill(libc::getpid(), signal);
}
std::process::exit(128 + signal);
}
std::process::exit(1);
}
/// Run a short-lived bubblewrap preflight in a child process and capture stderr.
///
/// Strategy:
@@ -581,6 +655,7 @@ fn resolve_true_command() -> String {
/// command, and reads are bounded to a fixed max size.
fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> String {
const MAX_PREFLIGHT_STDERR_BYTES: u64 = 64 * 1024;
let synthetic_mount_targets = bwrap_args.synthetic_mount_targets.clone();
let mut pipe_fds = [0; 2];
let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) };
@@ -628,6 +703,7 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
let err = std::io::Error::last_os_error();
panic!("waitpid failed for bubblewrap child: {err}");
}
cleanup_synthetic_mount_targets(&synthetic_mount_targets);
String::from_utf8_lossy(&stderr_bytes).into_owned()
}

23
codex-rs/linux-sandbox/src/linux_run_main_tests.rs
View File

@@ -255,6 +255,29 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() {
assert!(argv.iter().any(|arg| arg == "--"));
}
#[test]
fn cleanup_synthetic_mount_targets_removes_only_empty_files() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let empty_file = temp_dir.path().join(".agents");
let non_empty_file = temp_dir.path().join(".codex");
let missing_file = temp_dir.path().join(".missing");
std::fs::write(&empty_file, "").expect("write empty file");
std::fs::write(&non_empty_file, "keep").expect("write nonempty file");
cleanup_synthetic_mount_targets(&[
crate::bwrap::SyntheticMountTarget::missing(&empty_file),
crate::bwrap::SyntheticMountTarget::missing(&non_empty_file),
crate::bwrap::SyntheticMountTarget::missing(&missing_file),
]);
assert!(!empty_file.exists());
assert_eq!(
std::fs::read_to_string(&non_empty_file).expect("read nonempty file"),
"keep"
);
assert!(!missing_file.exists());
}
#[test]
fn managed_proxy_inner_command_includes_route_spec() {
let sandbox_policy = SandboxPolicy::new_read_only_policy();

110
codex-rs/protocol/src/permissions.rs
View File

@@ -483,7 +483,7 @@ impl FileSystemSandboxPolicy {
if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) {
for protected_path in default_read_only_subpaths_for_writable_root(
&cwd_root, /*protect_missing_dot_codex*/ true,
&cwd_root, /*protect_missing_preserved_paths*/ true,
) {
append_default_read_only_path_if_no_explicit_rule(
&mut file_system_policy.entries,
@@ -494,7 +494,7 @@ impl FileSystemSandboxPolicy {
for writable_root in writable_roots {
for protected_path in default_read_only_subpaths_for_writable_root(
writable_root,
/*protect_missing_dot_codex*/ false,
/*protect_missing_preserved_paths*/ false,
) {
append_default_read_only_path_if_no_explicit_rule(
&mut file_system_policy.entries,
@@ -687,14 +687,17 @@ impl FileSystemSandboxPolicy {
.iter()
.filter(|path| normalize_effective_absolute_path((*path).clone()) == root)
.collect();
let protect_missing_dot_codex = AbsolutePathBuf::from_absolute_path(cwd)
let protect_missing_preserved_paths = AbsolutePathBuf::from_absolute_path(cwd)
.ok()
.is_some_and(|cwd| normalize_effective_absolute_path(cwd) == root);
let mut read_only_subpaths: Vec<AbsolutePathBuf> =
default_read_only_subpaths_for_writable_root(&root, protect_missing_dot_codex)
.into_iter()
.filter(|path| !has_explicit_resolved_path_entry(&resolved_entries, path))
.collect();
default_read_only_subpaths_for_writable_root(
&root,
protect_missing_preserved_paths,
)
.into_iter()
.filter(|path| !has_explicit_resolved_path_entry(&resolved_entries, path))
.collect();
// Narrower explicit non-write entries carve out broader writable roots.
// More specific write entries still remain writable because they appear
// as separate WritableRoot values and are checked independently.
@@ -1300,7 +1303,7 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf {
fn default_read_only_subpaths_for_writable_root(
writable_root: &AbsolutePathBuf,
protect_missing_dot_codex: bool,
protect_missing_preserved_paths: bool,
) -> Vec<AbsolutePathBuf> {
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
let top_level_git = writable_root.join(".git");
@@ -1309,7 +1312,7 @@ fn default_read_only_subpaths_for_writable_root(
// 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_dir || top_level_git_is_file || protect_missing_preserved_paths {
if top_level_git_is_file
&& is_git_pointer_file(&top_level_git)
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
@@ -1320,16 +1323,16 @@ fn default_read_only_subpaths_for_writable_root(
}
let top_level_agents = writable_root.join(".agents");
if top_level_agents.as_path().is_dir() {
if protect_missing_preserved_paths || top_level_agents.as_path().is_dir() {
subpaths.push(top_level_agents);
}
// Keep top-level project metadata under .codex read-only to the agent by
// Keep top-level preserved paths under .codex read-only to the agent by
// default. For the workspace root itself, protect it even before the
// directory exists so first-time creation still goes through the
// protected-path approval flow.
// preserved path approval flow.
let top_level_codex = writable_root.join(".codex");
if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
if protect_missing_preserved_paths || top_level_codex.as_path().is_dir() {
subpaths.push(top_level_codex);
}
@@ -1540,12 +1543,14 @@ mod tests {
#[cfg(unix)]
#[test]
fn writable_roots_proactively_protect_missing_dot_codex() {
fn writable_roots_proactively_protect_missing_preserved_paths() {
let cwd = TempDir::new().expect("tempdir");
let expected_root = AbsolutePathBuf::from_absolute_path(
cwd.path().canonicalize().expect("canonicalize cwd"),
)
.expect("absolute canonical root");
let expected_dot_git = expected_root.join(".git");
let expected_dot_agents = expected_root.join(".agents");
let expected_dot_codex = expected_root.join(".codex");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
@@ -1558,6 +1563,16 @@ mod tests {
let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
assert_eq!(writable_roots.len(), 1);
assert_eq!(writable_roots[0].root, expected_root);
assert!(
writable_roots[0]
.read_only_subpaths
.contains(&expected_dot_git)
);
assert!(
writable_roots[0]
.read_only_subpaths
.contains(&expected_dot_agents)
);
assert!(
writable_roots[0]
.read_only_subpaths
@@ -1613,12 +1628,14 @@ mod tests {
#[cfg(unix)]
#[test]
fn writable_roots_skip_default_dot_codex_when_explicit_user_rule_exists() {
fn writable_roots_skip_default_preserved_paths_when_explicit_user_rule_exists() {
let cwd = TempDir::new().expect("tempdir");
let expected_root = AbsolutePathBuf::from_absolute_path(
cwd.path().canonicalize().expect("canonicalize cwd"),
)
.expect("absolute canonical root");
let explicit_dot_git = expected_root.join(".git");
let explicit_dot_agents = expected_root.join(".agents");
let explicit_dot_codex = expected_root.join(".codex");
let policy = FileSystemSandboxPolicy::restricted(vec![
@@ -1628,6 +1645,18 @@ mod tests {
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: explicit_dot_git.clone(),
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: explicit_dot_agents.clone(),
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: explicit_dot_codex.clone(),
@@ -1641,6 +1670,18 @@ mod tests {
.iter()
.find(|root| root.root == expected_root)
.expect("workspace writable root");
assert!(
!workspace_root
.read_only_subpaths
.contains(&explicit_dot_git),
"explicit .git rule should win over the default preserved path carveout"
);
assert!(
!workspace_root
.read_only_subpaths
.contains(&explicit_dot_agents),
"explicit .agents rule should win over the default preserved path carveout"
);
assert!(
!workspace_root
.read_only_subpaths
@@ -1656,8 +1697,9 @@ mod tests {
}
#[test]
fn legacy_workspace_write_projection_blocks_missing_dot_codex_writes() {
fn legacy_workspace_write_projection_blocks_missing_preserved_path_writes() {
let cwd = TempDir::new().expect("tempdir");
let dot_git_config = cwd.path().join(".git").join("config");
let dot_codex_config = cwd.path().join(".codex").join("config.toml");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
@@ -1669,12 +1711,27 @@ mod tests {
let file_system_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, cwd.path());
assert!(!file_system_policy.can_write_path_with_cwd(&dot_git_config, cwd.path()));
assert!(!file_system_policy.can_write_path_with_cwd(&dot_codex_config, cwd.path()));
}
#[test]
fn legacy_workspace_write_projection_accepts_relative_cwd() {
let relative_cwd = Path::new("workspace");
let expected_dot_git = AbsolutePathBuf::from_absolute_path(
std::env::current_dir()
.expect("current dir")
.join(relative_cwd)
.join(".git"),
)
.expect("absolute dot git");
let expected_dot_agents = AbsolutePathBuf::from_absolute_path(
std::env::current_dir()
.expect("current dir")
.join(relative_cwd)
.join(".agents"),
)
.expect("absolute dot agents");
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(
std::env::current_dir()
.expect("current dir")
@@ -1707,6 +1764,18 @@ mod tests {
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: expected_dot_git,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: expected_dot_agents,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: expected_dot_codex,
@@ -1715,10 +1784,19 @@ mod tests {
},
])
);
assert!(
!file_system_policy.can_write_path_with_cwd(Path::new(".git/config"), relative_cwd,)
);
assert!(
!file_system_policy
.can_write_path_with_cwd(Path::new(".codex/config.toml"), relative_cwd,)
);
assert!(
!file_system_policy.can_write_path_with_cwd(
Path::new(".agents/skills/example/SKILL.md"),
relative_cwd,
)
);
}
#[cfg(unix)]

64
codex-rs/protocol/src/protocol.rs
View File

@@ -1253,13 +1253,13 @@ impl SandboxPolicy {
roots
.into_iter()
.map(|writable_root| {
let protect_missing_dot_codex = cwd_root
let protect_missing_preserved_paths = cwd_root
.as_ref()
.is_some_and(|cwd_root| cwd_root == &writable_root);
WritableRoot {
read_only_subpaths: default_read_only_subpaths_for_writable_root(
&writable_root,
protect_missing_dot_codex,
protect_missing_preserved_paths,
),
root: writable_root,
}
@@ -1272,7 +1272,7 @@ impl SandboxPolicy {
fn default_read_only_subpaths_for_writable_root(
writable_root: &AbsolutePathBuf,
protect_missing_dot_codex: bool,
protect_missing_preserved_paths: bool,
) -> Vec<AbsolutePathBuf> {
let mut subpaths: Vec<AbsolutePathBuf> = Vec::new();
let top_level_git = writable_root.join(".git");
@@ -1281,7 +1281,7 @@ fn default_read_only_subpaths_for_writable_root(
// 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_dir || top_level_git_is_file || protect_missing_preserved_paths {
if top_level_git_is_file
&& is_git_pointer_file(&top_level_git)
&& let Some(gitdir) = resolve_gitdir_from_file(&top_level_git)
@@ -1292,16 +1292,16 @@ fn default_read_only_subpaths_for_writable_root(
}
let top_level_agents = writable_root.join(".agents");
if top_level_agents.as_path().is_dir() {
if protect_missing_preserved_paths || top_level_agents.as_path().is_dir() {
subpaths.push(top_level_agents);
}
// Keep top-level project metadata under .codex read-only to the agent by
// Keep top-level preserved paths under .codex read-only to the agent by
// default. For the workspace root itself, protect it even before the
// directory exists so first-time creation still goes through the
// protected-path approval flow.
// preserved path approval flow.
let top_level_codex = writable_root.join(".codex");
if protect_missing_dot_codex || top_level_codex.as_path().is_dir() {
if protect_missing_preserved_paths || top_level_codex.as_path().is_dir() {
subpaths.push(top_level_codex);
}
@@ -4375,7 +4375,6 @@ mod tests {
#[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 canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path())
.expect("canonicalize cwd");
@@ -4456,6 +4455,11 @@ mod tests {
.expect("canonical docs/public");
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
.expect("canonical .codex");
let expected_dot_git = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".git"))
.expect("canonical .git");
let expected_dot_agents =
AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents"))
.expect("canonical .agents");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
@@ -4480,7 +4484,9 @@ mod tests {
(
canonical_cwd,
vec![
expected_dot_agents.to_path_buf(),
expected_dot_codex.to_path_buf(),
expected_dot_git.to_path_buf(),
expected_docs.to_path_buf()
],
),
@@ -4489,6 +4495,46 @@ mod tests {
);
}
#[test]
fn legacy_workspace_write_nested_readable_root_stays_writable() {
let cwd = TempDir::new().expect("tempdir");
let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path());
let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path())
.expect("canonicalize cwd");
let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex"))
.expect("canonical .codex");
let expected_dot_git = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".git"))
.expect("canonical .git");
let expected_dot_agents =
AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents"))
.expect("canonical .agents");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
assert_eq!(
sorted_writable_roots(
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, cwd.path())
.get_writable_roots_with_cwd(cwd.path())
),
vec![(
canonical_cwd,
vec![
expected_dot_agents.to_path_buf(),
expected_dot_codex.to_path_buf(),
expected_dot_git.to_path_buf()
]
)]
);
}
#[test]
fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() {
let cwd = if cfg!(windows) {

109
codex-rs/sandboxing/src/seatbelt_tests.rs
View File

@@ -26,6 +26,7 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -200,8 +201,10 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
writable_definitions,
vec![
"-DWRITABLE_ROOT_0=/".to_string(),
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.codex".to_string(),
format!("-DWRITABLE_ROOT_0_EXCLUDED_1={}", unreadable_root.display()),
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.git".to_string(),
"-DWRITABLE_ROOT_0_EXCLUDED_1=/.agents".to_string(),
"-DWRITABLE_ROOT_0_EXCLUDED_2=/.codex".to_string(),
format!("-DWRITABLE_ROOT_0_EXCLUDED_3={}", unreadable_root.display()),
],
"unexpected write carveout parameters in args: {args:#?}"
);
@@ -349,6 +352,49 @@ fn seatbelt_args_without_extension_profile_keep_legacy_preferences_read_access()
assert!(!policy.contains("(allow user-preference-write)"));
}
#[test]
fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() {
let tmp = TempDir::new().expect("tempdir");
let cwd = tmp.path().join("workspace");
fs::create_dir_all(cwd.join("docs")).expect("create docs");
let docs = AbsolutePathBuf::from_absolute_path(cwd.join("docs")).expect("absolute docs");
let args = create_seatbelt_command_args_for_legacy_policy(
vec!["/bin/true".to_string()],
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
include_platform_defaults: true,
readable_roots: vec![docs.clone()],
},
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
cwd.as_path(),
/*enforce_managed_network*/ false,
/*network*/ None,
);
assert!(
!args
.iter()
.any(|arg| arg.ends_with(&format!("={}", docs.as_path().display()))),
"legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}",
);
assert!(
args.iter()
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
&& arg.ends_with("/workspace/.agents")),
"expected proactive .agents carveout for cwd root: {args:#?}",
);
assert!(
args.iter()
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
&& arg.ends_with("/workspace/.codex")),
"expected proactive .codex carveout for cwd root: {args:#?}",
);
}
#[test]
fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() {
let policy = dynamic_network_policy(
@@ -773,12 +819,13 @@ fn create_seatbelt_args_full_network_with_proxy_is_still_proxy_only() {
#[test]
fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
// Create a temporary workspace with two writable roots: one containing
// top-level .git and .codex directories and one without them.
// top-level preserved paths and one without them.
let tmp = TempDir::new().expect("tempdir");
let PopulatedTmp {
vulnerable_root,
vulnerable_root_canonical,
dot_git_canonical,
dot_agents_canonical: _,
dot_codex_canonical,
empty_root,
empty_root_canonical,
@@ -828,12 +875,20 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
);
assert!(
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_0"),
"expected cwd .git carveout in policy:\n{policy_text}",
);
assert!(
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_1"),
"expected cwd .agents carveout in policy:\n{policy_text}",
);
assert!(
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_2"),
"expected cwd .codex carveout in policy:\n{policy_text}",
);
assert!(
policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0")
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"),
"expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}",
"expected explicit writable root preserved path carveouts in policy:\n{policy_text}",
);
assert!(
policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"),
@@ -849,6 +904,20 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
cwd.canonicalize()
.expect("canonicalize cwd")
.join(".git")
.display()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
cwd.canonicalize()
.expect("canonicalize cwd")
.join(".agents")
.display()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
cwd.canonicalize()
.expect("canonicalize cwd")
.join(".codex")
@@ -1037,11 +1106,11 @@ fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_desce
let policy_text = seatbelt_policy_arg(&args);
assert!(
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_2\")))"),
"expected exact .codex carveout in policy:\n{policy_text}"
);
assert!(
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_2\")))"),
"expected descendant .codex carveout in policy:\n{policy_text}"
);
}
@@ -1146,19 +1215,20 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() {
#[test]
fn create_seatbelt_args_for_cwd_as_git_repo() {
// Create a temporary workspace with two writable roots: one containing
// top-level .git and .codex directories and one without them.
// top-level preserved paths and one without them.
let tmp = TempDir::new().expect("tempdir");
let PopulatedTmp {
vulnerable_root,
vulnerable_root_canonical,
dot_git_canonical,
dot_agents_canonical,
dot_codex_canonical,
..
} = populate_tmpdir(tmp.path());
// Build a policy that does not specify any writable_roots, but does
// use the default ones (cwd and TMPDIR) and verifies the `.git` and
// `.codex` checks are done properly for cwd.
// use the default ones (cwd and TMPDIR) and verifies the preserved
// path checks are done properly for cwd.
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
@@ -1194,7 +1264,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
.map(|p| p.to_string_lossy().to_string());
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) )"#
r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) )"#
} else {
""
};
@@ -1203,13 +1273,13 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
// Note that the policy includes:
// - the base policy,
// - read-only access to the filesystem,
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2.
// - write access to WRITABLE_ROOT_0 (but not its preserved paths), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2.
let expected_policy = format!(
r#"{MACOS_SEATBELT_BASE_POLICY}
; allow read-only file operations
(allow file-read*)
(allow file-write*
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
)
"#,
@@ -1228,6 +1298,10 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
dot_agents_canonical.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
dot_codex_canonical.to_string_lossy()
),
format!(
@@ -1247,6 +1321,10 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
));
expected_args.push(format!(
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
dot_agents_canonical.to_string_lossy()
));
expected_args.push(format!(
"-DWRITABLE_ROOT_2_EXCLUDED_2={}",
dot_codex_canonical.to_string_lossy()
));
}
@@ -1264,7 +1342,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
}
struct PopulatedTmp {
/// Path containing a .git and .codex subfolder.
/// Path containing preserved subfolders.
/// For the purposes of this test, we consider this a "vulnerable" root
/// because a bad actor could write to .git/hooks/pre-commit so an
/// unsuspecting user would run code as privileged the next time they
@@ -1274,9 +1352,10 @@ struct PopulatedTmp {
vulnerable_root: PathBuf,
vulnerable_root_canonical: PathBuf,
dot_git_canonical: PathBuf,
dot_agents_canonical: PathBuf,
dot_codex_canonical: PathBuf,
/// Path without .git or .codex subfolders.
/// Path without preserved subfolders.
empty_root: PathBuf,
/// Canonicalized version of `empty_root`.
empty_root_canonical: PathBuf,
@@ -1310,12 +1389,14 @@ fn populate_tmpdir(tmp: &Path) -> PopulatedTmp {
.canonicalize()
.expect("canonicalize vulnerable_root");
let dot_git_canonical = vulnerable_root_canonical.join(".git");
let dot_agents_canonical = vulnerable_root_canonical.join(".agents");
let dot_codex_canonical = vulnerable_root_canonical.join(".codex");
let empty_root_canonical = empty_root.canonicalize().expect("canonicalize empty_root");
PopulatedTmp {
vulnerable_root,
vulnerable_root_canonical,
dot_git_canonical,
dot_agents_canonical,
dot_codex_canonical,
empty_root,
empty_root_canonical,