Add preserved path name policy primitive

This commit is contained in:
Eva Wong
2026-04-24 15:13:06 -07:00
parent d9a8d4d98a
commit b733879c4b
10 changed files with 436 additions and 228 deletions

View File

@@ -106,6 +106,7 @@ pub(crate) struct BwrapArgs {
pub args: Vec<String>,
pub preserved_files: Vec<File>,
pub synthetic_mount_targets: Vec<SyntheticMountTarget>,
pub protected_create_targets: Vec<ProtectedCreateTarget>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -138,6 +139,23 @@ pub(crate) struct SyntheticMountTarget {
pre_existing_path: Option<FileIdentity>,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct ProtectedCreateTarget {
path: PathBuf,
}
impl ProtectedCreateTarget {
pub(crate) fn missing(path: &Path) -> Self {
Self {
path: path.to_path_buf(),
}
}
pub(crate) fn path(&self) -> &Path {
&self.path
}
}
impl SyntheticMountTarget {
pub(crate) fn missing(path: &Path) -> Self {
Self {
@@ -228,6 +246,7 @@ pub(crate) fn create_bwrap_command_args(
args: command,
preserved_files: Vec::new(),
synthetic_mount_targets: Vec::new(),
protected_create_targets: Vec::new(),
})
} else {
Ok(create_bwrap_flags_full_filesystem(command, options))
@@ -268,6 +287,7 @@ fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOption
args,
preserved_files: Vec::new(),
synthetic_mount_targets: Vec::new(),
protected_create_targets: Vec::new(),
}
}
@@ -283,6 +303,7 @@ fn create_bwrap_flags(
args: filesystem_args,
preserved_files,
synthetic_mount_targets,
protected_create_targets,
} = create_filesystem_args(
file_system_sandbox_policy,
sandbox_policy_cwd,
@@ -321,6 +342,7 @@ fn create_bwrap_flags(
args,
preserved_files,
synthetic_mount_targets,
protected_create_targets,
})
}
@@ -361,6 +383,7 @@ fn create_filesystem_args(
writable_roots.push(WritableRoot {
root: AbsolutePathBuf::from_absolute_path("/")?,
read_only_subpaths: Vec::new(),
preserved_path_names: Vec::new(),
});
}
let mut unreadable_roots = file_system_sandbox_policy
@@ -456,6 +479,7 @@ fn create_filesystem_args(
args,
preserved_files: Vec::new(),
synthetic_mount_targets: Vec::new(),
protected_create_targets: Vec::new(),
};
let mut allowed_write_paths = Vec::with_capacity(writable_roots.len());
for writable_root in &writable_roots {
@@ -518,6 +542,13 @@ fn create_filesystem_args(
if let Some(target) = &symlink_target {
read_only_subpaths = remap_paths_for_symlink_target(read_only_subpaths, root, target);
}
append_protected_create_targets_for_writable_root(
&mut bwrap_args,
writable_root,
root,
symlink_target.as_deref(),
&read_only_subpaths,
);
read_only_subpaths.sort_by_key(|path| path_depth(path));
for subpath in read_only_subpaths {
append_read_only_subpath_args(&mut bwrap_args, &subpath, &allowed_write_paths)?;
@@ -555,6 +586,29 @@ fn create_filesystem_args(
Ok(bwrap_args)
}
fn append_protected_create_targets_for_writable_root(
bwrap_args: &mut BwrapArgs,
writable_root: &WritableRoot,
root: &Path,
symlink_target: Option<&Path>,
read_only_subpaths: &[PathBuf],
) {
for name in &writable_root.preserved_path_names {
let mut path = root.join(name);
if let Some(target) = symlink_target
&& let Ok(relative_path) = path.strip_prefix(root)
{
path = target.join(relative_path);
}
if read_only_subpaths.iter().any(|subpath| subpath == &path) || path.exists() {
continue;
}
bwrap_args
.protected_create_targets
.push(ProtectedCreateTarget::missing(&path));
}
}
fn expand_unreadable_globs_with_ripgrep(
patterns: &[String],
cwd: &Path,
@@ -1669,7 +1723,12 @@ mod tests {
);
assert!(
!synthetic_mount_target_paths(&args).contains(&dot_git),
"missing child .git should not be tracked for post-bwrap cleanup",
"missing child .git should not be materialized as a synthetic mount target",
);
assert_eq!(
protected_create_target_paths(&args),
vec![dot_git],
"missing child .git is enforced by the preserved-name sandbox policy",
);
}
@@ -2399,4 +2458,11 @@ mod tests {
.map(|target| target.path().to_path_buf())
.collect()
}
fn protected_create_target_paths(args: &BwrapArgs) -> Vec<PathBuf> {
args.protected_create_targets
.iter()
.map(|target| target.path().to_path_buf())
.collect()
}
}

View File

@@ -32,6 +32,7 @@ const FORWARDED_SIGNALS: &[libc::c_int] =
&[libc::SIGHUP, libc::SIGINT, libc::SIGQUIT, libc::SIGTERM];
const SYNTHETIC_MOUNT_MARKER_SYNTHETIC: &[u8] = b"synthetic\n";
const SYNTHETIC_MOUNT_MARKER_EXISTING: &[u8] = b"existing\n";
const PROTECTED_CREATE_MARKER: &[u8] = b"protected-create\n";
#[derive(Debug)]
struct SyntheticMountTargetRegistration {
@@ -40,6 +41,13 @@ struct SyntheticMountTargetRegistration {
marker_dir: PathBuf,
}
#[derive(Debug)]
struct ProtectedCreateTargetRegistration {
target: crate::bwrap::ProtectedCreateTarget,
marker_file: PathBuf,
marker_dir: PathBuf,
}
#[derive(Debug, Parser)]
/// CLI surface for the Linux sandbox helper.
///
@@ -501,6 +509,7 @@ fn build_bwrap_argv(
args: argv,
preserved_files: bwrap_args.preserved_files,
synthetic_mount_targets: bwrap_args.synthetic_mount_targets,
protected_create_targets: bwrap_args.protected_create_targets,
}
}
@@ -590,7 +599,9 @@ fn resolve_true_command() -> String {
}
fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
if bwrap_args.synthetic_mount_targets.is_empty() {
if bwrap_args.synthetic_mount_targets.is_empty()
&& bwrap_args.protected_create_targets.is_empty()
{
exec_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}
run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args);
@@ -601,8 +612,11 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
args,
preserved_files,
synthetic_mount_targets,
protected_create_targets,
} = bwrap_args;
let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
let protected_create_registrations =
register_protected_create_targets(&protected_create_targets);
let parent_pid = unsafe { libc::getpid() };
let pid = unsafe { libc::fork() };
if pid < 0 {
@@ -624,7 +638,9 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
let status = wait_for_bwrap_child(pid);
BWRAP_CHILD_PID.store(0, Ordering::SeqCst);
cleanup_synthetic_mount_targets(&synthetic_mount_registrations);
exit_with_wait_status(status);
let protected_create_violation =
cleanup_protected_create_targets(&protected_create_registrations);
exit_with_wait_status_or_policy_violation(status, protected_create_violation);
}
fn terminate_with_parent(parent_pid: libc::pid_t) {
@@ -729,6 +745,37 @@ fn register_synthetic_mount_targets(
})
}
fn register_protected_create_targets(
targets: &[crate::bwrap::ProtectedCreateTarget],
) -> Vec<ProtectedCreateTargetRegistration> {
with_synthetic_mount_registry_lock(|| {
targets
.iter()
.map(|target| {
let marker_dir = synthetic_mount_marker_dir(target.path());
fs::create_dir_all(&marker_dir).unwrap_or_else(|err| {
panic!(
"failed to create protected create marker directory {}: {err}",
marker_dir.display()
)
});
let marker_file = marker_dir.join(std::process::id().to_string());
fs::write(&marker_file, PROTECTED_CREATE_MARKER).unwrap_or_else(|err| {
panic!(
"failed to register protected create target {}: {err}",
target.path().display()
)
});
ProtectedCreateTargetRegistration {
target: target.clone(),
marker_file,
marker_dir,
}
})
.collect()
})
}
fn synthetic_mount_marker_contents(target: &crate::bwrap::SyntheticMountTarget) -> &'static [u8] {
if target.preserves_pre_existing_path() {
SYNTHETIC_MOUNT_MARKER_EXISTING
@@ -831,6 +878,75 @@ fn cleanup_synthetic_mount_targets(targets: &[SyntheticMountTargetRegistration])
});
}
fn cleanup_protected_create_targets(targets: &[ProtectedCreateTargetRegistration]) -> bool {
with_synthetic_mount_registry_lock(|| {
for target in targets.iter().rev() {
match fs::remove_file(&target.marker_file) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) => panic!(
"failed to unregister protected create target {}: {err}",
target.target.path().display()
),
}
}
let mut violation = false;
for target in targets.iter().rev() {
if synthetic_mount_marker_dir_has_active_process(&target.marker_dir) {
if target.target.path().exists() {
violation = true;
}
continue;
}
violation |= remove_protected_create_target(&target.target);
match fs::remove_dir(&target.marker_dir) {
Ok(()) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {}
Err(err) => panic!(
"failed to remove protected create marker directory {}: {err}",
target.marker_dir.display()
),
}
}
violation
})
}
fn remove_protected_create_target(target: &crate::bwrap::ProtectedCreateTarget) -> bool {
let path = target.path();
let metadata = match fs::symlink_metadata(path) {
Ok(metadata) => metadata,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return false,
Err(err) => panic!(
"failed to inspect protected create target {}: {err}",
path.display()
),
};
if metadata.is_dir() {
fs::remove_dir_all(path).unwrap_or_else(|err| {
panic!(
"failed to remove protected create target directory {}: {err}",
path.display()
)
});
} else {
fs::remove_file(path).unwrap_or_else(|err| {
panic!(
"failed to remove protected create target file {}: {err}",
path.display()
)
});
}
eprintln!(
"sandbox blocked creation of preserved workspace metadata path {}",
path.display()
);
true
}
fn remove_synthetic_mount_target(target: &crate::bwrap::SyntheticMountTarget) {
let path = target.path();
let metadata = match fs::symlink_metadata(path) {
@@ -947,6 +1063,17 @@ fn exit_with_wait_status(status: libc::c_int) -> ! {
std::process::exit(1);
}
fn exit_with_wait_status_or_policy_violation(
status: libc::c_int,
protected_create_violation: bool,
) -> ! {
if protected_create_violation && libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0 {
std::process::exit(1);
}
exit_with_wait_status(status);
}
/// Run a short-lived bubblewrap preflight in a child process and capture stderr.
///
/// Strategy:
@@ -964,8 +1091,11 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
args,
preserved_files,
synthetic_mount_targets,
protected_create_targets,
} = bwrap_args;
let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
let protected_create_registrations =
register_protected_create_targets(&protected_create_targets);
let mut pipe_fds = [0; 2];
let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) };
@@ -1009,6 +1139,7 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str
wait_for_bwrap_child(pid);
cleanup_synthetic_mount_targets(&synthetic_mount_registrations);
cleanup_protected_create_targets(&protected_create_registrations);
String::from_utf8_lossy(&stderr_bytes).into_owned()
}

View File

@@ -352,6 +352,43 @@ fn cleanup_synthetic_mount_targets_preserves_real_pre_existing_empty_file() {
assert!(empty_file.exists());
}
#[test]
fn cleanup_protected_create_targets_removes_created_path_and_reports_violation() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let dot_git = temp_dir.path().join(".git");
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
let registrations = register_protected_create_targets(&[target]);
std::fs::create_dir(&dot_git).expect("create protected path");
let violation = cleanup_protected_create_targets(&registrations);
assert!(violation);
assert!(!dot_git.exists());
}
#[test]
fn cleanup_protected_create_targets_waits_for_other_active_registrations() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let dot_git = temp_dir.path().join(".git");
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
let registrations = register_protected_create_targets(std::slice::from_ref(&target));
let active_marker = registrations[0].marker_dir.join("1");
std::fs::write(&active_marker, PROTECTED_CREATE_MARKER).expect("write active marker");
std::fs::write(&dot_git, "").expect("create protected path");
let violation = cleanup_protected_create_targets(&registrations);
assert!(violation);
assert!(dot_git.exists());
std::fs::remove_file(active_marker).expect("remove active marker");
let registrations = register_protected_create_targets(std::slice::from_ref(&target));
let violation = cleanup_protected_create_targets(&registrations);
assert!(violation);
assert!(!dot_git.exists());
}
#[test]
fn bwrap_signal_forwarder_terminates_child_and_keeps_parent_alive() {
let supervisor_pid = unsafe { libc::fork() };

View File

@@ -57,21 +57,16 @@ pub fn forbidden_agent_preserved_path_write(
let target = resolve_candidate_path(path, cwd)?;
let (preserved_path, preserved_name) = first_preserved_component(target.as_path())?;
if has_explicit_write_entry_for_path(file_system_sandbox_policy, &preserved_path, cwd) {
if has_explicit_write_entry_for_preserved_path(
file_system_sandbox_policy,
&preserved_path,
target.as_path(),
cwd,
) {
return None;
}
if !file_system_sandbox_policy.can_write_path_with_cwd(preserved_path.as_path(), cwd) {
return Some(preserved_name);
}
// A child directory of an existing Git repository must keep normal Git
// discovery working, so the sandbox layer intentionally does not
// materialize a missing child `.git`. Block that creation at command time.
if preserved_name == PRESERVED_GIT_PATH_NAME
&& !preserved_path.as_path().exists()
&& has_ancestor_git_metadata(preserved_path.as_path())
{
if !file_system_sandbox_policy.can_write_path_with_cwd(target.as_path(), cwd) {
return Some(preserved_name);
}
@@ -512,7 +507,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,
@@ -617,7 +612,28 @@ impl FileSystemSandboxPolicy {
}
pub fn can_write_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool {
self.resolve_access_with_cwd(path, cwd).can_write()
if !self.resolve_access_with_cwd(path, cwd).can_write() {
return false;
}
if self.has_full_disk_write_access() {
return true;
}
!self.is_preserved_path_write_denied(path, cwd)
}
fn is_preserved_path_write_denied(&self, path: &Path, cwd: &Path) -> bool {
if !matches!(self.kind, FileSystemSandboxKind::Restricted) {
return false;
}
let Some(target) = resolve_candidate_path(path, cwd) else {
return true;
};
let Some((preserved_path, _)) = first_preserved_component(target.as_path()) else {
return false;
};
!has_explicit_write_entry_for_preserved_path(self, &preserved_path, target.as_path(), cwd)
}
pub fn with_additional_readable_roots(
@@ -802,6 +818,10 @@ impl FileSystemSandboxPolicy {
}),
);
WritableRoot {
preserved_path_names: default_preserved_path_names_for_writable_root(
&root,
&resolved_entries,
),
root,
// Preserve literal in-root protected paths like `.git` and
// `.codex` so downstream sandboxes can still detect and mask
@@ -1387,6 +1407,20 @@ pub(crate) fn default_read_only_subpaths_for_writable_root(
dedup_absolute_paths(subpaths, /*normalize_effective_paths*/ false)
}
fn default_preserved_path_names_for_writable_root(
writable_root: &AbsolutePathBuf,
entries: &[ResolvedFileSystemEntry],
) -> Vec<String> {
PRESERVED_PATH_NAMES
.iter()
.filter(|name| {
let path = writable_root.join(**name);
!has_explicit_resolved_write_entry(entries, &path)
})
.map(|name| (*name).to_string())
.collect()
}
fn append_default_read_only_project_root_subpath_if_no_explicit_rule(
entries: &mut Vec<FileSystemSandboxEntry>,
subpath: impl Into<PathBuf>,
@@ -1467,8 +1501,9 @@ fn legacy_non_cwd_writable_roots(
dedup_absolute_paths(roots, /*normalize_effective_paths*/ true)
.into_iter()
.map(|root| WritableRoot {
preserved_path_names: default_preserved_path_names_for_writable_root(&root, &[]),
read_only_subpaths: default_read_only_subpaths_for_writable_root(
&root, /*protect_missing_dot_codex*/ false,
&root, /*protect_missing_preserved_paths*/ false,
),
root,
})
@@ -1482,6 +1517,15 @@ fn has_explicit_resolved_path_entry(
entries.iter().any(|entry| &entry.path == path)
}
fn has_explicit_resolved_write_entry(
entries: &[ResolvedFileSystemEntry],
path: &AbsolutePathBuf,
) -> bool {
entries
.iter()
.any(|entry| entry.access.can_write() && &entry.path == path)
}
fn first_preserved_component(path: &Path) -> Option<(AbsolutePathBuf, &'static str)> {
let mut candidate = PathBuf::new();
for component in path.components() {
@@ -1501,15 +1545,17 @@ fn preserved_path_name(name: &OsStr) -> Option<&'static str> {
.find(|preserved| name == OsStr::new(preserved))
}
fn has_explicit_write_entry_for_path(
fn has_explicit_write_entry_for_preserved_path(
policy: &FileSystemSandboxPolicy,
path: &AbsolutePathBuf,
preserved_path: &AbsolutePathBuf,
target: &Path,
cwd: &Path,
) -> bool {
policy
.resolved_entries_with_cwd(cwd)
.iter()
.any(|entry| entry.access.can_write() && &entry.path == path)
policy.resolved_entries_with_cwd(cwd).iter().any(|entry| {
entry.access.can_write()
&& target.starts_with(entry.path.as_path())
&& entry.path.as_path().starts_with(preserved_path.as_path())
})
}
fn has_ancestor_git_metadata(path: &Path) -> bool {
@@ -1768,8 +1814,15 @@ mod tests {
!writable_roots[0]
.read_only_subpaths
.contains(&expected_dot_git),
"missing child .git under an existing parent repo is enforced by command policy so Git discovery still works"
"missing child .git under an existing parent repo stays absent so Git discovery still works"
);
assert!(
writable_roots[0]
.preserved_path_names
.contains(&".git".to_string()),
"missing child .git creation is denied by the preserved-name policy primitive"
);
assert!(!policy.can_write_path_with_cwd(expected_dot_git.join("config").as_path(), &cwd));
assert!(
writable_roots[0]
.read_only_subpaths
@@ -1855,18 +1908,36 @@ mod tests {
.contains(&explicit_dot_git),
"explicit .git rule should win over the default preserved path carveout"
);
assert!(
!workspace_root
.preserved_path_names
.contains(&".git".to_string()),
"explicit .git rule should win over the preserved-name policy"
);
assert!(
!workspace_root
.read_only_subpaths
.contains(&explicit_dot_agents),
"explicit .agents rule should win over the default preserved path carveout"
);
assert!(
!workspace_root
.preserved_path_names
.contains(&".agents".to_string()),
"explicit .agents rule should win over the preserved-name policy"
);
assert!(
!workspace_root
.read_only_subpaths
.contains(&explicit_dot_codex),
"explicit .codex rule should win over the default protected carveout"
);
assert!(
!workspace_root
.preserved_path_names
.contains(&".codex".to_string()),
"explicit .codex rule should win over the preserved-name policy"
);
assert!(
policy.can_write_path_with_cwd(
explicit_dot_codex.join("config.toml").as_path(),
@@ -1913,8 +1984,7 @@ mod tests {
exclude_slash_tmp: true,
};
let file_system_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path());
let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy);
assert!(
!file_system_policy

View File

@@ -83,6 +83,7 @@ pub use crate::permissions::FileSystemSandboxKind;
pub use crate::permissions::FileSystemSandboxPolicy;
pub use crate::permissions::FileSystemSpecialPath;
pub use crate::permissions::NetworkSandboxPolicy;
use crate::permissions::PRESERVED_PATH_NAMES;
use crate::permissions::default_read_only_subpaths_for_writable_root;
pub use crate::request_permissions::RequestPermissionsArgs;
pub use crate::request_user_input::RequestUserInputEvent;
@@ -1091,6 +1092,10 @@ pub struct WritableRoot {
/// By construction, these subpaths are all under `root`.
pub read_only_subpaths: Vec<AbsolutePathBuf>,
/// Path component names that must not be created or replaced under `root`
/// unless the policy grants an explicit write rule for that preserved path.
pub preserved_path_names: Vec<String>,
}
impl WritableRoot {
@@ -1107,8 +1112,31 @@ impl WritableRoot {
}
}
if self.path_contains_preserved_name(path) {
return false;
}
true
}
fn path_contains_preserved_name(&self, path: &Path) -> bool {
let Ok(relative_path) = path.strip_prefix(&self.root) else {
return false;
};
relative_path.components().any(|component| {
self.preserved_path_names
.iter()
.any(|name| component.as_os_str() == std::ffi::OsStr::new(name))
})
}
}
fn default_preserved_path_names() -> Vec<String> {
PRESERVED_PATH_NAMES
.iter()
.map(|name| (*name).to_string())
.collect()
}
impl FromStr for SandboxPolicy {
@@ -1257,6 +1285,7 @@ impl SandboxPolicy {
&writable_root,
/*protect_missing_preserved_paths*/ true,
),
preserved_path_names: default_preserved_path_names(),
root: writable_root,
}
})

View File

@@ -328,6 +328,7 @@ fn root_absolute_path() -> AbsolutePathBuf {
struct SeatbeltAccessRoot {
root: AbsolutePathBuf,
excluded_subpaths: Vec<AbsolutePathBuf>,
preserved_path_names: Vec<String>,
}
fn build_seatbelt_access_policy(
@@ -342,9 +343,9 @@ fn build_seatbelt_access_policy(
let root =
normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root);
let root_param = format!("{param_prefix}_{index}");
params.push((root_param.clone(), root.into_path_buf()));
params.push((root_param.clone(), root.clone().into_path_buf()));
if access_root.excluded_subpaths.is_empty() {
if access_root.excluded_subpaths.is_empty() && access_root.preserved_path_names.is_empty() {
policy_components.push(format!("(subpath (param \"{root_param}\"))"));
continue;
}
@@ -367,6 +368,11 @@ fn build_seatbelt_access_policy(
"(require-not (subpath (param \"{excluded_param}\")))"
));
}
for preserved_name in access_root.preserved_path_names {
let regex =
seatbelt_preserved_path_name_regex(&root, &preserved_name).replace('"', "\\\"");
require_parts.push(format!(r#"(require-not (regex #"{regex}"))"#));
}
policy_components.push(format!("(require-all {} )", require_parts.join(" ")));
}
@@ -380,6 +386,20 @@ fn build_seatbelt_access_policy(
}
}
fn seatbelt_preserved_path_name_regex(root: &AbsolutePathBuf, name: &str) -> String {
let mut root = root.to_string_lossy().to_string();
while root.len() > 1 && root.ends_with('/') {
root.pop();
}
let root = regex_lite::escape(&root);
let name = regex_lite::escape(name);
if root == "/" {
format!(r#"^/(.*/)?{name}(/.*)?$"#)
} else {
format!(r#"^{root}/(.*/)?{name}(/.*)?$"#)
}
}
fn build_seatbelt_unreadable_glob_policy(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
@@ -586,6 +606,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
vec![SeatbeltAccessRoot {
root: root_absolute_path(),
excluded_subpaths: unreadable_roots.clone(),
preserved_path_names: Vec::new(),
}],
)
}
@@ -599,6 +620,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
.map(|root| SeatbeltAccessRoot {
root: root.root,
excluded_subpaths: root.read_only_subpaths,
preserved_path_names: root.preserved_path_names,
})
.collect(),
)
@@ -618,6 +640,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
vec![SeatbeltAccessRoot {
root: root_absolute_path(),
excluded_subpaths: unreadable_roots,
preserved_path_names: Vec::new(),
}],
);
(
@@ -638,6 +661,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
.filter(|path| path.as_path().starts_with(root.as_path()))
.cloned()
.collect(),
preserved_path_names: Vec::new(),
root,
})
.collect(),

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::permissions::PRESERVED_PATH_NAMES;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -60,6 +61,26 @@ fn seatbelt_policy_arg(args: &[String]) -> &str {
.expect("seatbelt args should include policy text")
}
fn seatbelt_preserved_path_name_requirements(root: &Path) -> String {
let mut root = root.to_string_lossy().to_string();
while root.len() > 1 && root.ends_with('/') {
root.pop();
}
let root = regex_lite::escape(&root);
PRESERVED_PATH_NAMES
.iter()
.map(|name| {
let name = regex_lite::escape(name);
if root == "/" {
format!(r#"(require-not (regex #"^/(.*/)?{name}(/.*)?$"))"#)
} else {
format!(r#"(require-not (regex #"^{root}/(.*/)?{name}(/.*)?$"))"#)
}
})
.collect::<Vec<_>>()
.join(" ")
}
struct TestConfigReloader;
#[async_trait::async_trait]
@@ -1279,11 +1300,20 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
.and_then(|p| p.canonicalize().ok())
.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"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_5"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_5"))) )"#
let slash_tmp = PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp");
let tempdir_policy_entry = if let Some(p) = &tmpdir_env_var {
let preserved_requirements = seatbelt_preserved_path_name_requirements(Path::new(p));
format!(
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"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_5"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_5"))) {preserved_requirements} )"#
)
} else {
""
String::new()
};
let root_0_preserved_requirements =
seatbelt_preserved_path_name_requirements(&vulnerable_root_canonical);
let root_1_preserved_requirements = seatbelt_preserved_path_name_requirements(&slash_tmp);
// Build the expected policy text using a raw string for readability.
// Note that the policy includes:
@@ -1296,7 +1326,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
; 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"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) ) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_2"))) ){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"))) {root_0_preserved_requirements} ) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_2"))) {root_1_preserved_requirements} ){tempdir_policy_entry}
)
"#,
@@ -1321,36 +1351,18 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
dot_codex_canonical.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1={}",
PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp")
.to_string_lossy()
),
format!("-DWRITABLE_ROOT_1={}", slash_tmp.to_string_lossy()),
format!(
"-DWRITABLE_ROOT_1_EXCLUDED_0={}",
PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp")
.join(".git")
.to_string_lossy()
slash_tmp.join(".git").to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1_EXCLUDED_1={}",
PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp")
.join(".agents")
.to_string_lossy()
slash_tmp.join(".agents").to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1_EXCLUDED_2={}",
PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp")
.join(".codex")
.to_string_lossy()
slash_tmp.join(".codex").to_string_lossy()
),
];

View File

@@ -1,5 +1,4 @@
use std::path::Path;
use std::path::PathBuf;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::forbidden_agent_preserved_path_write;
@@ -9,18 +8,6 @@ pub fn preserved_path_write_forbidden_reason(
cwd: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> Option<String> {
let commands = crate::bash::parse_shell_lc_plain_commands(command)
.or_else(|| crate::bash::parse_shell_lc_command_word_prefixes(command))
.unwrap_or_else(|| vec![command.to_vec()]);
for simple_command in commands {
if let Some(name) =
simple_command_preserved_path_write(&simple_command, cwd, file_system_sandbox_policy)
{
return Some(preserved_path_write_reason(name));
}
}
if let Some(targets) = crate::bash::parse_shell_lc_write_redirection_targets(command) {
for target in targets {
if let Some(name) = forbidden_agent_preserved_path_write(
@@ -39,89 +26,6 @@ fn preserved_path_write_reason(name: &str) -> String {
format!("command targets preserved workspace metadata path `{name}`")
}
fn simple_command_preserved_path_write(
command: &[String],
cwd: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> Option<&'static str> {
let program = command.first().map(|program| {
Path::new(program)
.file_name()
.and_then(|name| name.to_str())
.unwrap_or(program)
})?;
match program {
"git" => git_init_preserved_path_write(command, cwd, file_system_sandbox_policy),
"touch" | "mkdir" | "rm" | "rmdir" | "ln" | "mv" | "cp" | "install" => command
.iter()
.skip(1)
.filter(|arg| !arg.starts_with('-'))
.find_map(|arg| {
forbidden_agent_preserved_path_write(
Path::new(arg),
cwd,
file_system_sandbox_policy,
)
}),
_ => None,
}
}
fn git_init_preserved_path_write(
command: &[String],
cwd: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> Option<&'static str> {
let mut git_cwd = PathBuf::from(cwd);
let mut index = 1;
while index < command.len() {
match command[index].as_str() {
"-C" => {
let next = command.get(index + 1)?;
git_cwd = resolve_shell_operand(Path::new(next), &git_cwd);
index += 2;
}
"--" => {
index += 1;
break;
}
arg if arg.starts_with('-') => {
index += 1;
}
_ => break,
}
}
if command.get(index).map(String::as_str) != Some("init") {
return None;
}
let init_target = command
.iter()
.skip(index + 1)
.find(|arg| !arg.starts_with('-'))
.map_or_else(
|| git_cwd.clone(),
|arg| resolve_shell_operand(Path::new(arg), &git_cwd),
);
forbidden_agent_preserved_path_write(
init_target.join(".git").as_path(),
&git_cwd,
file_system_sandbox_policy,
)
}
fn resolve_shell_operand(path: &Path, cwd: &Path) -> PathBuf {
if path.is_absolute() {
path.to_path_buf()
} else {
cwd.join(path)
}
}
#[cfg(test)]
mod tests {
use std::path::Path;
@@ -160,7 +64,7 @@ mod tests {
}
}
fn legacy_workspace_write_policy(cwd: &Path) -> FileSystemSandboxPolicy {
fn legacy_workspace_write_policy() -> FileSystemSandboxPolicy {
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: ReadOnlyAccess::Restricted {
@@ -171,31 +75,7 @@ mod tests {
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd)
}
#[test]
fn preserved_path_detector_blocks_git_init_under_parent_repo() {
let repo = TestDir::new("git-init-under-parent-repo");
std::fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
std::fs::create_dir(&cwd).expect("create cwd");
let policy = legacy_workspace_write_policy(&cwd);
let reason = preserved_path_write_forbidden_reason(
&[
"/bin/bash".to_string(),
"-lc".to_string(),
"git init".to_string(),
],
&cwd,
&policy,
);
assert_eq!(
reason,
Some("command targets preserved workspace metadata path `.git`".to_string())
);
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy)
}
#[test]
@@ -204,7 +84,7 @@ mod tests {
std::fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
std::fs::create_dir(&cwd).expect("create cwd");
let policy = legacy_workspace_write_policy(&cwd);
let policy = legacy_workspace_write_policy();
let reason = preserved_path_write_forbidden_reason(
&[
@@ -220,9 +100,9 @@ mod tests {
}
#[test]
fn preserved_path_detector_blocks_direct_preserved_path_writes() {
fn preserved_path_detector_leaves_direct_writes_to_sandbox_policy() {
let cwd = TestDir::new("direct-preserved-path-writes");
let policy = legacy_workspace_write_policy(cwd.path());
let policy = legacy_workspace_write_policy();
let reason = preserved_path_write_forbidden_reason(
&[
@@ -234,10 +114,7 @@ mod tests {
&policy,
);
assert_eq!(
reason,
Some("command targets preserved workspace metadata path `.git`".to_string())
);
assert_eq!(reason, None);
}
#[test]
@@ -246,7 +123,7 @@ mod tests {
std::fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
std::fs::create_dir(&cwd).expect("create cwd");
let policy = legacy_workspace_write_policy(&cwd);
let policy = legacy_workspace_write_policy();
let reason = preserved_path_write_forbidden_reason(
&[
@@ -263,28 +140,4 @@ mod tests {
Some("command targets preserved workspace metadata path `.git`".to_string())
);
}
#[test]
fn preserved_path_detector_blocks_git_init_inside_complex_script() {
let repo = TestDir::new("git-init-inside-complex-script");
std::fs::create_dir(repo.path().join(".git")).expect("create parent .git");
let cwd = repo.path().join("sub");
std::fs::create_dir(&cwd).expect("create cwd");
let policy = legacy_workspace_write_policy(&cwd);
let reason = preserved_path_write_forbidden_reason(
&[
"/bin/bash".to_string(),
"-lc".to_string(),
"set -e\nif git init -q; then\n exit 22\nfi".to_string(),
],
&cwd,
&policy,
);
assert_eq!(
reason,
Some("command targets preserved workspace metadata path `.git`".to_string())
);
}
}

View File

@@ -617,7 +617,6 @@ impl App {
runtime_permission_profile_for_turn_start(
self.runtime_sandbox_policy_override.as_ref(),
sandbox_policy,
cwd.as_path(),
),
model.to_string(),
effort,
@@ -1490,7 +1489,6 @@ impl App {
fn runtime_permission_profile_for_turn_start(
runtime_sandbox_policy_override: Option<&SandboxPolicy>,
sandbox_policy: &SandboxPolicy,
cwd: &std::path::Path,
) -> Option<codex_protocol::models::PermissionProfile> {
runtime_sandbox_policy_override?;
match sandbox_policy {
@@ -1498,10 +1496,7 @@ fn runtime_permission_profile_for_turn_start(
SandboxPolicy::ReadOnly { .. }
| SandboxPolicy::WorkspaceWrite { .. }
| SandboxPolicy::DangerFullAccess => Some(
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
sandbox_policy,
cwd,
),
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(sandbox_policy),
),
}
}
@@ -1514,28 +1509,23 @@ mod tests {
#[test]
fn runtime_permission_profile_for_turn_start_only_when_sandbox_was_overridden() {
let cwd = std::path::Path::new("/tmp/project");
let sandbox_policy = SandboxPolicy::DangerFullAccess;
assert_eq!(
runtime_permission_profile_for_turn_start(
/*runtime_sandbox_policy_override*/ None,
&sandbox_policy,
cwd,
),
None
);
let profile =
runtime_permission_profile_for_turn_start(Some(&sandbox_policy), &sandbox_policy, cwd)
runtime_permission_profile_for_turn_start(Some(&sandbox_policy), &sandbox_policy)
.expect("runtime sandbox override should send active permissions");
assert_eq!(
profile,
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
&sandbox_policy,
cwd,
)
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy)
);
}
}

View File

@@ -1529,10 +1529,8 @@ mod tests {
#[test]
fn embedded_turn_start_permission_overrides_send_runtime_profile_only_when_provided() {
let cwd = std::path::Path::new("/tmp/project");
let sandbox_policy = SandboxPolicy::DangerFullAccess;
let permission_profile =
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy, cwd);
let permission_profile = PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy);
assert_eq!(
turn_start_permission_overrides(
@@ -1555,10 +1553,8 @@ mod tests {
#[test]
fn remote_turn_start_permission_overrides_keep_legacy_sandbox_policy() {
let cwd = std::path::Path::new("/tmp/project");
let sandbox_policy = SandboxPolicy::DangerFullAccess;
let permission_profile =
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy, cwd);
let permission_profile = PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy);
assert_eq!(
turn_start_permission_overrides(