agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Enforce workspace metadata protections in Seatbelt (#19847)

Browse Source 
## Summary

Translate FileSystemSandboxPolicy project root metadata carveouts into
macOS Seatbelt rules.

## Scope

1. Thread protected metadata names into Seatbelt access roots.
2. Ask FileSystemSandboxPolicy whether each metadata carveout is
writable.
3. Emit Seatbelt deny rules that block creating or replacing protected
metadata names under writable roots.
4. Add coverage for first time metadata creation and read only
carveouts.

## Reviewer Focus

1. This PR only covers the macOS sandbox adapter.
2. The policy decision comes from FileSystemSandboxPolicy.
3. Read only subpath carveouts and metadata protection checks should
compose cleanly.

## Stack

1. Policy primitive: #19846
2. macOS Seatbelt adapter: this PR
3. Shell preflight UX: #19848
4. Runtime profile propagation: #19849
5. Linux bubblewrap adapter: #19852

## Validation

1. formatting for codex sandboxing
2. codex sandboxing package tests
This commit is contained in:
evawong-oai
2026-04-28 10:13:00 -07:00
committed by GitHub
Unverified
parent f6797c3ac6
commit 0670d8971a
2 changed files with 181 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/sandboxing/src/seatbelt.rs
+52 -2
View File
@@ -4,7 +4,9 @@ use codex_network_proxy::has_proxy_url_env_vars;
use codex_network_proxy::proxy_url_env_value;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::WritableRoot;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
@@ -328,6 +330,7 @@ fn root_absolute_path() -> AbsolutePathBuf {
struct SeatbeltAccessRoot {
root: AbsolutePathBuf,
excluded_subpaths: Vec<AbsolutePathBuf>,
protected_metadata_names: Vec<String>,
}
fn build_seatbelt_access_policy(
@@ -342,9 +345,11 @@ 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.protected_metadata_names.is_empty()
{
policy_components.push(format!("(subpath (param \"{root_param}\"))"));
continue;
}
@@ -367,6 +372,11 @@ fn build_seatbelt_access_policy(
"(require-not (subpath (param \"{excluded_param}\")))"
));
}
for metadata_name in access_root.protected_metadata_names {
let regex =
seatbelt_protected_metadata_name_regex(&root, &metadata_name).replace('"', "\\\"");
require_parts.push(format!(r#"(require-not (regex #"{regex}"))"#));
}
policy_components.push(format!("(require-all {} )", require_parts.join(" ")));
}
@@ -380,6 +390,38 @@ fn build_seatbelt_access_policy(
}
}
fn seatbelt_protected_metadata_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 protected_metadata_names_for_writable_root(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
writable_root: &WritableRoot,
cwd: &Path,
) -> Vec<String> {
let mut names = writable_root.protected_metadata_names.clone();
for name in PROTECTED_METADATA_PATH_NAMES {
if names.iter().any(|existing| existing == name) {
continue;
}
let path = writable_root.root.join(*name);
if !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd) {
names.push((*name).to_string());
}
}
names
}
fn build_seatbelt_unreadable_glob_policy(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
@@ -586,6 +628,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
vec![SeatbeltAccessRoot {
root: root_absolute_path(),
excluded_subpaths: unreadable_roots.clone(),
protected_metadata_names: Vec::new(),
}],
)
}
@@ -597,6 +640,11 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
.get_writable_roots_with_cwd(sandbox_policy_cwd)
.into_iter()
.map(|root| SeatbeltAccessRoot {
protected_metadata_names: protected_metadata_names_for_writable_root(
file_system_sandbox_policy,
&root,
sandbox_policy_cwd,
),
root: root.root,
excluded_subpaths: root.read_only_subpaths,
})
@@ -618,6 +666,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
vec![SeatbeltAccessRoot {
root: root_absolute_path(),
excluded_subpaths: unreadable_roots,
protected_metadata_names: Vec::new(),
}],
);
(
@@ -638,6 +687,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
.filter(|path| path.as_path().starts_with(root.as_path()))
.cloned()
.collect(),
protected_metadata_names: Vec::new(),
root,
})
.collect(),
codex-rs/sandboxing/src/seatbelt_tests.rs
+129 -90
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::PROTECTED_METADATA_PATH_NAMES;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -59,6 +60,26 @@ fn seatbelt_policy_arg(args: &[String]) -> &str {
.expect("seatbelt args should include policy text")
}
fn seatbelt_protected_metadata_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);
PROTECTED_METADATA_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]
@@ -185,6 +206,12 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"),
"expected write carveout in policy:\n{policy}"
);
assert!(
policy.contains(&seatbelt_protected_metadata_name_requirements(Path::new(
"/"
))),
"expected metadata protection regex deny requirements in policy:\n{policy}"
);
assert!(
args.iter().any(
|arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display())
@@ -773,12 +800,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 workspace metadata 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,
@@ -841,8 +869,22 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
"expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}",
);
assert!(
policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"),
"expected second explicit writable root grant in policy:\n{policy_text}",
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&cwd.canonicalize().expect("canonicalize cwd")
)),
"expected cwd metadata protection regex requirements in policy:\n{policy_text}",
);
assert!(
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&vulnerable_root_canonical
)),
"expected populated root metadata protection regex requirements in policy:\n{policy_text}",
);
assert!(
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&empty_root_canonical
)),
"expected empty root metadata protection regex requirements in policy:\n{policy_text}",
);
let expected_definitions = [
@@ -1014,7 +1056,7 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
}
#[test]
fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_descendant_carveouts() {
fn create_seatbelt_args_block_first_time_dot_codex_creation_with_metadata_name_regex() {
let tmp = TempDir::new().expect("tempdir");
let repo_root = tmp.path().join("repo");
fs::create_dir_all(&repo_root).expect("create repo root");
@@ -1056,12 +1098,10 @@ 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\")))"),
"expected exact .codex carveout in policy:\n{policy_text}"
);
assert!(
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
"expected descendant .codex carveout in policy:\n{policy_text}"
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&repo_root.canonicalize().expect("canonicalize repo root")
)),
"expected metadata protection regex requirements in policy:\n{policy_text}"
);
}
@@ -1165,19 +1205,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 workspace metadata 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 protected
// metadata checks are done properly for cwd.
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
@@ -1206,92 +1247,87 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
/*network*/ None,
);
let tmpdir_env_var = std::env::var("TMPDIR")
let slash_tmp = PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp");
let policy_text = seatbelt_policy_arg(&args);
assert!(
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&vulnerable_root_canonical
)),
"expected cwd metadata protection regex requirements in policy:\n{policy_text}",
);
assert!(
policy_text.contains(&seatbelt_protected_metadata_name_requirements(&slash_tmp)),
"expected /tmp metadata protection regex requirements in policy:\n{policy_text}",
);
if let Some(tmpdir_env_var) = std::env::var("TMPDIR")
.ok()
.map(PathBuf::from)
.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"))) )"#
} else {
""
};
// Build the expected policy text using a raw string for readability.
// Note that the policy includes:
// - the base policy,
// - read-only access to the filesystem,
// - write access to WRITABLE_ROOT_0 (but not its metadata subpaths), 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"))) (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}
)
"#,
);
let mut expected_args = vec![
"-p".to_string(),
expected_policy,
format!(
"-DWRITABLE_ROOT_0={}",
vulnerable_root_canonical.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
dot_git_canonical.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
dot_codex_canonical.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
vulnerable_root_canonical.join(".agents").to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1={}",
PathBuf::from("/tmp")
.canonicalize()
.expect("canonicalize /tmp")
.to_string_lossy()
),
];
if let Some(p) = tmpdir_env_var {
expected_args.push(format!("-DWRITABLE_ROOT_2={p}"));
expected_args.push(format!(
"-DWRITABLE_ROOT_2_EXCLUDED_0={}",
dot_git_canonical.to_string_lossy()
));
expected_args.push(format!(
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
vulnerable_root_canonical.join(".agents").to_string_lossy()
));
expected_args.push(format!(
"-DWRITABLE_ROOT_2_EXCLUDED_2={}",
dot_codex_canonical.to_string_lossy()
));
{
assert!(
policy_text.contains(&seatbelt_protected_metadata_name_requirements(
&tmpdir_env_var
)),
"expected TMPDIR metadata protection regex requirements in policy:\n{policy_text}",
);
}
expected_args.extend(
macos_dir_params()
.into_iter()
.map(|(key, value)| format!("-D{key}={value}", value = value.to_string_lossy())),
let expected_root = format!(
"-DWRITABLE_ROOT_0={}",
vulnerable_root_canonical.to_string_lossy()
);
assert!(
args.contains(&expected_root),
"missing {expected_root}: {args:#?}"
);
let expected_dot_git = format!(
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
dot_git_canonical.to_string_lossy()
);
assert!(
args.contains(&expected_dot_git),
"missing {expected_dot_git}: {args:#?}"
);
let expected_dot_codex = format!(
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
dot_codex_canonical.to_string_lossy()
);
assert!(
args.contains(&expected_dot_codex),
"missing {expected_dot_codex}: {args:#?}"
);
let unexpected_dot_agents = format!(
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
dot_agents_canonical.to_string_lossy()
);
assert!(
!args.contains(&unexpected_dot_agents),
"missing .agents should be handled by regex rather than materialized as a path param: {args:#?}"
);
let expected_slash_tmp = format!("-DWRITABLE_ROOT_1={}", slash_tmp.to_string_lossy());
assert!(
args.contains(&expected_slash_tmp),
"missing {expected_slash_tmp}: {args:#?}"
);
for (key, value) in macos_dir_params() {
let expected_definition = format!("-D{key}={}", value.to_string_lossy());
assert!(
args.contains(&expected_definition),
"expected definition arg `{expected_definition}` in {args:#?}"
);
}
expected_args.push("--".to_string());
expected_args.extend(shell_command);
assert_eq!(expected_args, args);
let command_index = args
.iter()
.position(|arg| arg == "--")
.expect("seatbelt args should include command separator");
assert_eq!(args[command_index + 1..], shell_command);
}
struct PopulatedTmp {
/// Path containing a .git and .codex subfolder.
/// Path containing protected metadata 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
@@ -1301,9 +1337,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 protected metadata subfolders.
empty_root: PathBuf,
/// Canonicalized version of `empty_root`.
empty_root_canonical: PathBuf,
@@ -1337,12 +1374,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,