openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-07 15:40:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: preserve reopened descendants under read denies

Browse Source 
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
viyatb-oai
2026-05-05 23:10:46 -07:00
parent a736cb55a2
commit e01fd53004
3 changed files with 274 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -220,7 +220,7 @@ struct FileSystemSemanticSignature {
/// Runtime matcher for read-deny entries in a filesystem sandbox policy.
pub struct ReadDenyMatcher {
denied_candidates: Vec<Vec<PathBuf>>,
exact_candidates: Vec<ResolvedFileSystemEntry>,
deny_read_matchers: Vec<GlobMatcher>,
invalid_pattern: bool,
}
@@ -236,13 +236,25 @@ impl ReadDenyMatcher {
return None;
}
// Exact roots are stored as all meaningful path spellings we can derive
// cheaply. This lets direct tool checks catch both a symlink path and
// its canonical target without changing the policy entries themselves.
let denied_candidates = file_system_sandbox_policy
.get_unreadable_roots_with_cwd(cwd)
// Exact entries are stored with all meaningful path spellings we can
// derive cheaply. That lets direct tool checks preserve the same
// most-specific precedence as the main resolver while still catching
// both a symlink path and its canonical target.
let exact_candidates = file_system_sandbox_policy
.resolved_entries_with_cwd(cwd)
.into_iter()
.map(|path| normalized_and_canonical_candidates(path.as_path()))
.flat_map(|entry| {
normalized_and_canonical_candidates(entry.path.as_path())
.into_iter()
.filter_map(move |path| {
AbsolutePathBuf::from_absolute_path(path).ok().map(|path| {
ResolvedFileSystemEntry {
path,
access: entry.access,
}
})
})
})
.collect();
// Pattern entries stay as policy-level globs. They are matched at read
// time here instead of being snapshotted to startup filesystem state.
@@ -259,7 +271,7 @@ impl ReadDenyMatcher {
})
.collect();
Some(Self {
denied_candidates,
exact_candidates,
deny_read_matchers,
invalid_pattern,
})
@@ -273,17 +285,20 @@ impl ReadDenyMatcher {
return true;
}
// Check exact roots against each candidate spelling before evaluating
// glob matchers. Exact entries are subtree denies; glob entries match
// according to the pattern compiler's path-separator rules.
// Exact entries preserve the same most-specific precedence as the
// policy resolver, across every candidate spelling we know about.
let path_candidates = normalized_and_canonical_candidates(path);
if self.denied_candidates.iter().any(|denied_candidates| {
path_candidates.iter().any(|candidate| {
denied_candidates.iter().any(|denied_candidate| {
candidate == denied_candidate || candidate.starts_with(denied_candidate)
})
if self
.exact_candidates
.iter()
.filter(|entry| {
path_candidates
.iter()
.any(|candidate| candidate.starts_with(entry.path.as_path()))
})
}) {
.max_by_key(|entry| resolved_entry_precedence(entry))
.is_some_and(|entry| !entry.access.can_read())
{
return true;
}
@@ -2858,6 +2873,36 @@ mod tests {
));
}
#[test]
fn exact_path_denies_honor_more_specific_read_allows() {
let temp = TempDir::new().expect("tempdir");
let denied_dir = AbsolutePathBuf::resolve_path_against_base("denied", temp.path());
let allowed_dir = denied_dir.join("allowed");
let allowed_file = allowed_dir.join("visible.txt");
let blocked_file = denied_dir.join("blocked.txt");
std::fs::create_dir_all(allowed_dir.as_path()).expect("create allowed dir");
std::fs::write(allowed_file.as_path(), "visible").expect("write allowed");
std::fs::write(blocked_file.as_path(), "blocked").expect("write blocked");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: denied_dir },
access: FileSystemAccessMode::None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: allowed_dir },
access: FileSystemAccessMode::Read,
},
]);
assert!(is_read_denied(blocked_file.as_path(), &policy, temp.path()));
assert!(!is_read_denied(
allowed_file.as_path(),
&policy,
temp.path()
));
}
#[cfg(unix)]
#[test]
fn canonical_target_matches_denied_symlink_alias() {

56
codex-rs/sandboxing/src/seatbelt.rs
View File

@@ -404,6 +404,49 @@ fn seatbelt_protected_metadata_name_regex(root: &AbsolutePathBuf, name: &str) ->
}
}
fn build_seatbelt_reopened_read_traversal_policy(
readable_roots: &[AbsolutePathBuf],
unreadable_roots: &[AbsolutePathBuf],
) -> (String, Vec<(String, PathBuf)>) {
let mut traversal_roots = BTreeMap::new();
for readable_root in readable_roots {
for unreadable_root in unreadable_roots.iter().filter(|unreadable_root| {
readable_root
.as_path()
.starts_with(unreadable_root.as_path())
}) {
let mut ancestor = readable_root.as_path().parent();
while let Some(path) = ancestor {
if !path.starts_with(unreadable_root.as_path()) {
break;
}
if let Ok(path) = AbsolutePathBuf::from_absolute_path(path) {
traversal_roots
.entry(path.to_string_lossy().to_string())
.or_insert(path);
}
if path == unreadable_root.as_path() {
break;
}
ancestor = path.parent();
}
}
}
let mut policy_components = Vec::new();
let mut params = Vec::new();
for (index, root) in traversal_roots.into_values().enumerate() {
let param = format!("READABLE_TRAVERSAL_ROOT_{index}");
params.push((param.clone(), root.into_path_buf()));
policy_components.push(format!(
"(allow file-read-metadata (literal (param \"{param}\")) (vnode-type DIRECTORY))"
));
}
(policy_components.join("\n"), params)
}
fn protected_metadata_names_for_writable_root(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
writable_root: &WritableRoot,
@@ -652,6 +695,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
)
};
let readable_roots = file_system_sandbox_policy.get_readable_roots_with_cwd(sandbox_policy_cwd);
let (file_read_policy, file_read_dir_params) =
if file_system_sandbox_policy.has_full_disk_read_access() {
if unreadable_roots.is_empty() {
@@ -665,7 +709,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
"READABLE_ROOT",
vec![SeatbeltAccessRoot {
root: root_absolute_path(),
excluded_subpaths: unreadable_roots,
excluded_subpaths: unreadable_roots.clone(),
protected_metadata_names: Vec::new(),
}],
);
@@ -678,9 +722,9 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
let (policy, params) = build_seatbelt_access_policy(
"file-read*",
"READABLE_ROOT",
file_system_sandbox_policy
.get_readable_roots_with_cwd(sandbox_policy_cwd)
.into_iter()
readable_roots
.iter()
.cloned()
.map(|root| SeatbeltAccessRoot {
excluded_subpaths: unreadable_roots
.iter()
@@ -701,6 +745,8 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
)
}
};
let (reopened_read_traversal_policy, reopened_read_traversal_dir_params) =
build_seatbelt_reopened_read_traversal_policy(&readable_roots, &unreadable_roots);
let proxy = proxy_policy_inputs(network, extra_allow_unix_sockets);
let network_policy =
@@ -712,6 +758,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
let mut policy_sections = vec![
MACOS_SEATBELT_BASE_POLICY.to_string(),
file_read_policy,
reopened_read_traversal_policy,
file_write_policy,
deny_read_policy,
network_policy,
@@ -724,6 +771,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
let dir_params = [
file_read_dir_params,
reopened_read_traversal_dir_params,
file_write_dir_params,
macos_dir_params(),
unix_socket_dir_params(&proxy),

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

@@ -285,6 +285,166 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() {
);
}
#[test]
fn reopened_readable_children_under_unreadable_roots_get_metadata_traversal() {
let unreadable = absolute_path("/tmp/codex-home");
let readable_file = absolute_path("/tmp/codex-home/.gitconfig");
let writable_dir = absolute_path("/tmp/codex-home/.cache/uv");
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: unreadable },
access: FileSystemAccessMode::None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: readable_file,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: writable_dir },
access: FileSystemAccessMode::Write,
},
]);
let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams {
command: vec!["/bin/true".to_string()],
file_system_sandbox_policy: &file_system_policy,
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
sandbox_policy_cwd: Path::new("/"),
enforce_managed_network: false,
network: None,
extra_allow_unix_sockets: &[],
});
let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/"));
let expected_unreadable_root = unreadable_roots.first().expect("expected unreadable root");
let policy = seatbelt_policy_arg(&args);
assert!(
policy.contains(
"(allow file-read-metadata (literal (param \"READABLE_TRAVERSAL_ROOT_0\")) (vnode-type DIRECTORY))"
),
"expected metadata traversal rule for reopened readable descendants:\n{policy}"
);
assert!(
policy.contains(
"(allow file-read-metadata (literal (param \"READABLE_TRAVERSAL_ROOT_1\")) (vnode-type DIRECTORY))"
),
"expected metadata traversal rule for reopened writable descendants:\n{policy}"
);
let traversal_definitions: Vec<String> = args
.iter()
.filter(|arg| arg.starts_with("-DREADABLE_TRAVERSAL_ROOT_"))
.cloned()
.collect();
assert_eq!(
traversal_definitions,
vec![
format!(
"-DREADABLE_TRAVERSAL_ROOT_0={}",
expected_unreadable_root.display()
),
format!(
"-DREADABLE_TRAVERSAL_ROOT_1={}",
expected_unreadable_root.join(".cache").display()
),
]
);
}
#[test]
fn reopened_children_under_unreadable_roots_work_under_seatbelt() {
let tmp = TempDir::new().expect("tempdir");
let parent = tmp.path().join("home");
let gitconfig = parent.join(".gitconfig");
let blocked = parent.join(".config").join("my-app").join(".env");
let writable_dir = parent.join(".cache").join("uv");
let writable_file = writable_dir.join("state.txt");
fs::create_dir_all(blocked.parent().expect("blocked parent")).expect("create blocked parent");
fs::create_dir_all(&writable_dir).expect("create writable dir");
fs::write(&gitconfig, "visible").expect("write readable child");
fs::write(&blocked, "secret").expect("write blocked child");
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::from_absolute_path(&parent).expect("absolute parent"),
},
access: FileSystemAccessMode::None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::from_absolute_path(&gitconfig).expect("absolute gitconfig"),
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::from_absolute_path(&writable_dir)
.expect("absolute writable dir"),
},
access: FileSystemAccessMode::Write,
},
]);
let run = |script: &str, path: &Path| {
let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams {
command: vec![
"/bin/bash".to_string(),
"-c".to_string(),
script.to_string(),
"bash".to_string(),
path.to_string_lossy().to_string(),
],
file_system_sandbox_policy: &file_system_policy,
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
sandbox_policy_cwd: tmp.path(),
enforce_managed_network: false,
network: None,
extra_allow_unix_sockets: &[],
});
Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE)
.args(&args)
.current_dir(tmp.path())
.output()
.expect("execute seatbelt command")
};
let ls_parent = run(r#"ls "$1" >/dev/null"#, &parent);
assert!(
!ls_parent.status.success(),
"parent listing should stay denied"
);
let read_allowed = run(r#"cat "$1" >/dev/null"#, &gitconfig);
assert!(
read_allowed.status.success(),
"allowed child read should work"
);
let read_blocked = run(r#"cat "$1" >/dev/null"#, &blocked);
assert!(
!read_blocked.status.success(),
"unlisted child should stay denied"
);
let write_allowed = run(r#"printf ok > "$1""#, &writable_file);
assert!(
write_allowed.status.success(),
"allowed writable child should accept writes"
);
assert_eq!(
fs::read_to_string(&writable_file).expect("read writable child"),
"ok"
);
}
#[test]
fn unreadable_globstar_slash_matches_zero_or_more_directories() {
let regex = seatbelt_regex_for_unreadable_glob("/tmp/repo/**/*.env");