openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Prevent protected-path rename bypasses in macOS Seatbelt (#39623)

Browse Source 
## Why

Renaming a writable directory could move protected descendants outside the
paths covered by their read-only or unreadable policy entries.

## What changed

- Deny unlink-style writes to directory ancestors of protected paths, placing
  these rules after broader write allowances in the generated Seatbelt policy.
- Apply full write denies to unreadable glob matches and protect their directory
  ancestors against renames.
- Support brace alternation and backslash escapes when translating filesystem
  globs into Seatbelt regular expressions.

## Testing

Add coverage for ancestor rename attempts across explicit read-only and deny
entries, multiple glob forms, canonicalized glob paths, and the new glob syntax.

GitOrigin-RevId: edfda9d4c2bc7ba2ecd6b67744446871e526084f
This commit is contained in:
Dylan Hurd
2026-08-20 06:47:30 +00:00
committed by copyberry
parent f1e06b3865
commit 52e387daca
2 changed files with 362 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/sandboxing/src/seatbelt.rs
+101 -21
View File
@@ -528,29 +528,38 @@ fn build_seatbelt_unreadable_glob_policy(
cwd: &Path,
) -> String {
// Seatbelt does not understand the filesystem policy's glob syntax directly.
// Convert each unreadable pattern into an anchored regex deny rule and apply
// it to both reads and unlink-style writes so a denied path cannot be probed
// through destructive filesystem operations.
// Convert each unreadable pattern into anchored read/write denies. Also deny
// renaming directories that could contain matches so the protected files
// cannot be moved beyond the path where the glob applies.
let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd);
if unreadable_globs.is_empty() {
return String::new();
}
let mut policy_components = Vec::new();
for pattern in unreadable_globs {
let mut regexes = BTreeSet::new();
if let Some(regex) = seatbelt_regex_for_unreadable_glob(&pattern) {
regexes.insert(regex);
let mut patterns = BTreeSet::from([pattern.clone()]);
if let Some(pattern) = canonicalize_glob_static_prefix_for_sandbox(&pattern) {
patterns.insert(pattern);
}
if let Some(pattern) = canonicalize_glob_static_prefix_for_sandbox(&pattern)
&& let Some(regex) = seatbelt_regex_for_unreadable_glob(&pattern)
{
regexes.insert(regex);
}
for regex in regexes {
for pattern in patterns {
let Some(regex) = seatbelt_regex_for_unreadable_glob(&pattern) else {
continue;
};
let regex = regex.replace('"', "\\\"");
policy_components.push(format!(r#"(deny file-read* (regex #"{regex}"))"#));
policy_components.push(format!(r#"(deny file-write-unlink (regex #"{regex}"))"#));
policy_components.push(format!(r#"(deny file-write* (regex #"{regex}"))"#));
for ancestor in Path::new(&pattern).ancestors().skip(1) {
let Some(regex) = ancestor
.to_str()
.and_then(|path| seatbelt_regex_for_glob(path, GlobMatch::Exact))
else {
continue;
};
let regex = regex.replace('"', "\\\"");
policy_components.push(format!(
r#"(deny file-write-unlink (require-all (vnode-type DIRECTORY) (regex #"{regex}")))"#
));
}
}
}
@@ -560,7 +569,7 @@ fn build_seatbelt_unreadable_glob_policy(
fn canonicalize_glob_static_prefix_for_sandbox(pattern: &str) -> Option<String> {
let first_glob_index = pattern
.char_indices()
.find_map(|(index, ch)| matches!(ch, '*' | '?' | '[' | ']').then_some(index));
.find_map(|(index, ch)| matches!(ch, '*' | '?' | '[' | ']' | '{' | '\\').then_some(index));
let Some(first_glob_index) = first_glob_index else {
return normalize_path_for_sandbox(Path::new(pattern))
.map(|path| path.to_string_lossy().to_string());
@@ -583,18 +592,30 @@ fn canonicalize_glob_static_prefix_for_sandbox(pattern: &str) -> Option<String>
(normalized_pattern != pattern).then_some(normalized_pattern)
}
#[derive(Clone, Copy, Eq, PartialEq)]
enum GlobMatch {
Exact,
Subtree,
}
fn seatbelt_regex_for_unreadable_glob(pattern: &str) -> Option<String> {
seatbelt_regex_for_glob(pattern, GlobMatch::Subtree)
}
fn seatbelt_regex_for_glob(pattern: &str, glob_match: GlobMatch) -> Option<String> {
if pattern.is_empty() {
return None;
}
// Translate the supported git-style glob subset into a Seatbelt regex:
// `*` and `?` stay within one path component, `**/` can consume zero or
// more components, and closed character classes remain character classes.
// A pattern with no glob metacharacters is treated as exact path plus subtree.
// more components, closed character classes remain character classes,
// brace groups become alternations, and backslashes escape metacharacters.
// Literal patterns also match descendants unless exact matching is requested.
let mut regex = String::from("^");
let mut chars = pattern.chars().collect::<VecDeque<_>>();
let mut saw_glob = false;
let mut alternate_depth = 0;
while let Some(ch) = chars.pop_front() {
match ch {
@@ -616,6 +637,23 @@ fn seatbelt_regex_for_unreadable_glob(pattern: &str) -> Option<String> {
saw_glob = true;
regex.push_str("[^/]");
}
'\\' => {
if let Some(escaped) = chars.pop_front() {
regex.push_str(&regex_lite::escape(&escaped.to_string()));
} else {
regex.push_str("\\\\");
}
}
'{' => {
saw_glob = true;
alternate_depth += 1;
regex.push('(');
}
'}' if alternate_depth > 0 => {
alternate_depth -= 1;
regex.push(')');
}
',' if alternate_depth > 0 => regex.push('|'),
'[' => {
saw_glob = true;
let mut class = Vec::new();
@@ -660,7 +698,14 @@ fn seatbelt_regex_for_unreadable_glob(pattern: &str) -> Option<String> {
}
}
if !saw_glob {
// Path ancestors can end inside a brace alternative when a branch contains
// a separator, such as `/repo/{private/nested,other}` -> `/repo/{private`.
// Close those partial groups so their directory prefixes remain protected.
for _ in 0..alternate_depth {
regex.push(')');
}
if !saw_glob && glob_match == GlobMatch::Subtree {
regex.push_str("(/.*)?");
}
regex.push('$');
@@ -730,6 +775,31 @@ pub(crate) fn create_seatbelt_command_args_with_profile(
let unreadable_roots =
file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd);
let writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd);
// Protect ancestors of read-only paths so renaming a writable directory
// cannot move its descendants outside their policy carveouts.
let mut protected_ancestors = BTreeSet::new();
for writable_root in &writable_roots {
let root = normalize_path_for_sandbox(writable_root.root.as_path())
.unwrap_or_else(|| writable_root.root.clone());
for protected_directory in writable_root.read_only_subpaths.iter().filter_map(|path| {
normalize_path_for_sandbox(path.as_path())
.unwrap_or_else(|| path.clone())
.parent()
}) {
for ancestor in protected_directory.ancestors() {
if !ancestor.as_path().starts_with(root.as_path()) {
break;
}
protected_ancestors.insert(ancestor);
}
}
}
let protected_ancestor_params: Vec<(String, PathBuf)> = protected_ancestors
.into_iter()
.enumerate()
.map(|(index, path)| (format!("PROTECTED_ANCESTOR_{index}"), path.into_path_buf()))
.collect();
let (file_write_policy, file_write_dir_params) =
if file_system_sandbox_policy.has_full_disk_write_access() {
if unreadable_roots.is_empty() {
@@ -751,8 +821,7 @@ pub(crate) fn create_seatbelt_command_args_with_profile(
} else {
build_seatbelt_access_policy(
SeatbeltAccessKind::Write,
file_system_sandbox_policy
.get_writable_roots_with_cwd(sandbox_policy_cwd)
writable_roots
.into_iter()
.map(|root| SeatbeltAccessRoot {
protected_metadata_names: protected_metadata_names_for_writable_root(
@@ -832,7 +901,6 @@ pub(crate) fn create_seatbelt_command_args_with_profile(
MACOS_SEATBELT_BASE_POLICY.to_string(),
file_read_policy,
file_write_policy,
deny_read_policy,
network_policy,
];
if include_platform_defaults {
@@ -841,12 +909,24 @@ pub(crate) fn create_seatbelt_command_args_with_profile(
policy_sections.push(MACOS_PROCESS_APPLICATIONS_READ_POLICY.to_string());
}
}
policy_sections.push(deny_read_policy);
// Renaming an allowed ancestor relocates its protected descendants past
// their pathname carveouts. Keep these denies last so no broader allowance
// can reopen the unlink operation used by rename.
policy_sections.extend(
protected_ancestor_params.iter().map(|(key, _)| {
format!(
"(deny file-write-unlink (require-all (vnode-type DIRECTORY) (literal (param \"{key}\"))))"
)
}),
);
let full_policy = policy_sections.join("\n");
let dir_params = [
file_read_dir_params,
file_write_dir_params,
protected_ancestor_params,
unix_socket_dir_params(&proxy),
]
.concat();
codex-rs/sandboxing/src/seatbelt_tests.rs
+261 -21
View File
@@ -1,4 +1,5 @@
use super::CreateSeatbeltCommandArgsParams;
use super::GlobMatch;
use super::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
use super::MACOS_SEATBELT_BASE_POLICY;
use super::MacosSeatbeltProfile;
@@ -10,6 +11,7 @@ use super::create_seatbelt_command_args_for_legacy_policy;
use super::create_seatbelt_command_args_with_profile;
use super::dynamic_network_policy;
use super::normalize_path_for_sandbox;
use super::seatbelt_regex_for_glob;
use super::seatbelt_regex_for_unreadable_glob;
use super::unix_socket_dir_params;
use super::unix_socket_policy;
@@ -357,6 +359,207 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
);
}
#[test]
fn nested_protected_paths_cannot_be_bypassed_by_renaming_ancestors() {
#[derive(Clone, Copy, Debug)]
enum Protection {
Deny,
Read,
DenyGlob(&'static str),
DenyGlobAboveRoot,
}
for protection in [
Protection::DenyGlobAboveRoot,
Protection::DenyGlob(".githu?"),
Protection::DenyGlob("{.github,.gitlab}"),
Protection::DenyGlob(r".githu\b"),
Protection::DenyGlob("*"),
Protection::Deny,
Protection::Read,
] {
let temp_dir = TempDir::new().expect("temp dir");
let workspace = temp_dir.path().join("workspace");
let github = workspace.join(".github");
let workflows = github.join("workflows");
let protected_file = workflows.join("release.yml");
fs::create_dir_all(&workflows).expect("create workflows directory");
fs::write(&protected_file, "original workflow").expect("write protected workflow");
let workspace = workspace.canonicalize().expect("canonicalize workspace");
let destination = workspace.with_file_name("destination");
fs::create_dir(&destination).expect("create destination");
let github = github
.canonicalize()
.expect("canonicalize github directory");
let workflows = workflows
.canonicalize()
.expect("canonicalize workflows directory");
let protected_file = protected_file
.canonicalize()
.expect("canonicalize protected workflow");
let workspace_root =
AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace");
let protected_path =
AbsolutePathBuf::from_absolute_path(&protected_file).expect("absolute protected path");
let (protected_path, protected_access) = match protection {
Protection::Deny => (
FileSystemPath::Path {
path: protected_path.into(),
},
FileSystemAccessMode::Deny,
),
Protection::Read => (
FileSystemPath::Path {
path: protected_path.into(),
},
FileSystemAccessMode::Read,
),
Protection::DenyGlob(component) => (
FileSystemPath::GlobPattern {
pattern: format!("{}/{component}/workflows/*.yml", workspace.display()),
},
FileSystemAccessMode::Deny,
),
Protection::DenyGlobAboveRoot => (
FileSystemPath::GlobPattern {
pattern: format!(
"{}/*/.github/workflows/*.yml",
workspace.parent().expect("workspace parent").display()
),
},
FileSystemAccessMode::Deny,
),
};
let glob_pattern = match &protected_path {
FileSystemPath::GlobPattern { pattern } => Some(pattern.clone()),
_ => None,
};
let mut file_system_policy = FileSystemSandboxPolicy::read_only();
for root in [&workspace, &destination] {
file_system_policy.entries.push(FileSystemSandboxEntry::new(
AbsolutePathBuf::from_absolute_path(root)
.expect("absolute writable root")
.into(),
FileSystemAccessMode::Write,
));
}
file_system_policy
.entries
.extend(PROTECTED_METADATA_PATH_NAMES.iter().map(|name| {
FileSystemSandboxEntry::new(
workspace_root.join(*name).into(),
FileSystemAccessMode::Write,
)
}));
let unprotected_policy = file_system_policy.clone();
file_system_policy.entries.push(FileSystemSandboxEntry::new(
protected_path,
protected_access,
));
for protected_ancestor in [&workspace, &github, &workflows] {
let renamed_ancestor = if protected_ancestor == &workspace {
destination.join("moved")
} else {
protected_ancestor.with_extension("renamed")
};
let seatbelt_args = |policy| {
create_seatbelt_command_args(CreateSeatbeltCommandArgsParams {
command: vec![
"/bin/mv".to_string(),
protected_ancestor.display().to_string(),
renamed_ancestor.display().to_string(),
],
file_system_sandbox_policy: policy,
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
sandbox_policy_cwd: &workspace,
enforce_managed_network: false,
managed_network: None,
environment_id: None,
network: None,
extra_allow_unix_sockets: &[],
})
.expect("create seatbelt policy")
};
let args = seatbelt_args(&file_system_policy);
let policy = seatbelt_policy_arg(&args);
let ancestor_deny = match protection {
Protection::Deny | Protection::Read => {
let parameter = args
.iter()
.find_map(|arg| {
let (name, path) = arg.strip_prefix("-D")?.split_once('=')?;
(name.starts_with("PROTECTED_ANCESTOR_")
&& Path::new(path) == protected_ancestor)
.then_some(name)
})
.expect("protected ancestor should have a policy parameter");
format!(
"(deny file-write-unlink (require-all (vnode-type DIRECTORY) (literal (param \"{parameter}\"))))"
)
}
Protection::DenyGlob(_) | Protection::DenyGlobAboveRoot => {
let pattern = Path::new(glob_pattern.as_deref().expect("deny glob pattern"))
.ancestors()
.find(|path| {
path.components().count() == protected_ancestor.components().count()
})
.and_then(Path::to_str)
.expect("ancestor glob pattern");
let regex = seatbelt_regex_for_glob(pattern, GlobMatch::Exact)
.expect("ancestor glob should produce a regex");
format!(
r#"(deny file-write-unlink (require-all (vnode-type DIRECTORY) (regex #"{regex}")))"#
)
}
};
assert!(
policy
.find(&ancestor_deny)
.expect("protected ancestor deny should be present")
> policy
.rfind("(allow file-write*")
.expect("writable root allowance should be present"),
"protected ancestor denies must follow broader allowances: {policy}"
);
let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE)
.args(args)
.current_dir(temp_dir.path())
.output()
.expect("execute seatbelt command");
let stderr = String::from_utf8_lossy(&output.stderr);
if !output.status.success()
&& stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted")
{
return;
}
assert!(
!output.status.success(),
"{protection:?} path must not become accessible by renaming {}",
protected_ancestor.display()
);
assert_eq!(
fs::read_to_string(&protected_file).expect("read protected workflow"),
"original workflow"
);
if matches!(
protection,
Protection::DenyGlob(_) | Protection::DenyGlobAboveRoot
) && protected_ancestor == &workflows
{
let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE)
.args(seatbelt_args(&unprotected_policy))
.current_dir(temp_dir.path())
.output()
.expect("execute control seatbelt command");
assert!(output.status.success(), "unprotected ancestor: {output:?}");
}
}
}
}
#[test]
fn prepared_managed_network_context_allows_only_its_proxy_ports() {
let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
@@ -473,6 +676,40 @@ fn unreadable_globs_use_git_style_component_matching() {
assert!(!regex.is_match("/tmp/repo/app/fileab.txt"));
}
#[test]
fn unreadable_globs_support_brace_alternation() {
let regex = seatbelt_regex_for_unreadable_glob("/tmp/repo/{.env,secrets.yml}");
assert_eq!(regex.as_deref(), Some(r"^/tmp/repo/(\.env|secrets\.yml)$"));
let regex = regex_lite::Regex::new(regex.as_deref().expect("glob should compile"))
.expect("regex should compile");
assert!(regex.is_match("/tmp/repo/.env"));
assert!(regex.is_match("/tmp/repo/secrets.yml"));
assert!(!regex.is_match("/tmp/repo/notes.txt"));
}
#[test]
fn glob_ancestors_close_brace_alternatives_split_by_path_separators() {
let regex = seatbelt_regex_for_glob("/tmp/repo/{private", GlobMatch::Exact);
assert_eq!(regex.as_deref(), Some(r"^/tmp/repo/(private)$"));
let regex = regex_lite::Regex::new(regex.as_deref().expect("ancestor glob should compile"))
.expect("ancestor regex should compile");
assert!(regex.is_match("/tmp/repo/private"));
assert!(!regex.is_match("/tmp/repo/other"));
}
#[test]
fn unreadable_globs_support_backslash_escapes() {
let regex = seatbelt_regex_for_unreadable_glob(r"/tmp/repo/config\?.env");
assert_eq!(regex.as_deref(), Some(r"^/tmp/repo/config\?\.env(/.*)?$"));
let regex = regex_lite::Regex::new(regex.as_deref().expect("glob should compile"))
.expect("regex should compile");
assert!(regex.is_match("/tmp/repo/config?.env"));
assert!(!regex.is_match("/tmp/repo/config1.env"));
}
#[test]
fn unreadable_globs_treat_unclosed_character_classes_as_literals() {
let regex = seatbelt_regex_for_unreadable_glob("/tmp/repo/[*.env");
@@ -496,29 +733,32 @@ fn unreadable_glob_policy_includes_canonicalized_static_prefix() {
fs::create_dir(&real_root).expect("create real root");
symlink(&real_root, &link_root).expect("create symlinked root");
let pattern = format!("{}/**/*.env", link_root.display());
let canonical_pattern = format!(
"{}/**/*.env",
real_root
.canonicalize()
.expect("canonicalize real root")
.display()
);
let expected_regex = seatbelt_regex_for_unreadable_glob(&canonical_pattern)
.expect("canonical glob should compile");
let mut policy = FileSystemSandboxPolicy::default();
policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern { pattern },
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
});
let canonical_root = real_root.canonicalize().expect("canonicalize real root");
for suffix in ["**/*.env", "{app,service}/*.env", r"secret\?.env"] {
let pattern = format!("{}/{suffix}", link_root.display());
let canonical_pattern = format!("{}/{suffix}", canonical_root.display());
let expected_regex = seatbelt_regex_for_unreadable_glob(&canonical_pattern)
.expect("canonical glob should compile");
let mut policy = FileSystemSandboxPolicy::default();
policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern { pattern },
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
});
let seatbelt_policy = build_seatbelt_unreadable_glob_policy(&policy, temp_dir.path());
let seatbelt_policy = build_seatbelt_unreadable_glob_policy(&policy, temp_dir.path());
assert!(
seatbelt_policy.contains(&format!(r#"(deny file-read* (regex #"{expected_regex}"))"#)),
"expected canonicalized glob regex in policy:\n{seatbelt_policy}"
);
assert!(
seatbelt_policy.contains(&format!(r#"(deny file-read* (regex #"{expected_regex}"))"#)),
"expected canonicalized glob regex in policy:\n{seatbelt_policy}"
);
assert!(
seatbelt_policy.contains(&format!(
r#"(deny file-write* (regex #"{expected_regex}"))"#
)),
"expected canonicalized glob write deny in policy:\n{seatbelt_policy}"
);
}
}
#[test]