mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[sandboxing] Remove Darwin user cache write from Seatbelt network policy (#21443)
## Summary 1. Removes the broad `DARWIN_USER_CACHE_DIR` write rule from the macOS Seatbelt network policy. 2. Removes the now unused policy parameter plumbing for that cache path. 3. Adds sandboxing coverage that keeps `com.apple.trustd.agent` for TLS while rejecting the cache write rule. ## Why This closes the exact cache poisoning boundary. The earlier `gh` TLS issue is now covered by trustd access, so the cache write is no longer needed. ## Validation 1. Rust formatting passed. 2. The sandboxing crate tests passed. 3. Local macOS Seatbelt repro with patched policy passed. `gh api` returned `21442` without the cache write rule.
This commit is contained in:
committed by
GitHub
Unverified
parent
5b87bd2845
commit
5733e00c79
@@ -12,7 +12,6 @@ use std::collections::BTreeMap;
|
||||
use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::VecDeque;
|
||||
use std::ffi::CStr;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tracing::warn;
|
||||
@@ -725,7 +724,6 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
|
||||
let dir_params = [
|
||||
file_read_dir_params,
|
||||
file_write_dir_params,
|
||||
macos_dir_params(),
|
||||
unix_socket_dir_params(&proxy),
|
||||
]
|
||||
.concat();
|
||||
@@ -742,32 +740,6 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
|
||||
seatbelt_args
|
||||
}
|
||||
|
||||
/// Wraps libc::confstr to return a String.
|
||||
fn confstr(name: libc::c_int) -> Option<String> {
|
||||
let mut buf = vec![0_i8; (libc::PATH_MAX as usize) + 1];
|
||||
let len = unsafe { libc::confstr(name, buf.as_mut_ptr(), buf.len()) };
|
||||
if len == 0 {
|
||||
return None;
|
||||
}
|
||||
// confstr guarantees NUL-termination when len > 0.
|
||||
let cstr = unsafe { CStr::from_ptr(buf.as_ptr()) };
|
||||
cstr.to_str().ok().map(ToString::to_string)
|
||||
}
|
||||
|
||||
/// Wraps confstr to return a canonicalized PathBuf.
|
||||
fn confstr_path(name: libc::c_int) -> Option<PathBuf> {
|
||||
let s = confstr(name)?;
|
||||
let path = PathBuf::from(s);
|
||||
path.canonicalize().ok().or(Some(path))
|
||||
}
|
||||
|
||||
fn macos_dir_params() -> Vec<(String, PathBuf)> {
|
||||
if let Some(p) = confstr_path(libc::_CS_DARWIN_USER_CACHE_DIR) {
|
||||
return vec![("DARWIN_USER_CACHE_DIR".to_string(), p)];
|
||||
}
|
||||
vec![]
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "seatbelt_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
)
|
||||
|
||||
(allow mach-lookup
|
||||
; Used to look up the _CS_DARWIN_USER_CACHE_DIR in the sandbox.
|
||||
; Used by platform helpers that resolve user directory locations.
|
||||
(global-name "com.apple.bsd.dirhelper")
|
||||
(global-name "com.apple.system.opendirectoryd.membership")
|
||||
|
||||
@@ -29,7 +29,3 @@
|
||||
(allow sysctl-read
|
||||
(sysctl-name-regex #"^net.routetable")
|
||||
)
|
||||
|
||||
(allow file-write*
|
||||
(subpath (param "DARWIN_USER_CACHE_DIR"))
|
||||
)
|
||||
|
||||
@@ -7,7 +7,6 @@ use super::build_seatbelt_unreadable_glob_policy;
|
||||
use super::create_seatbelt_command_args;
|
||||
use super::create_seatbelt_command_args_for_legacy_policy;
|
||||
use super::dynamic_network_policy;
|
||||
use super::macos_dir_params;
|
||||
use super::normalize_path_for_sandbox;
|
||||
use super::seatbelt_regex_for_unreadable_glob;
|
||||
use super::unix_socket_dir_params;
|
||||
@@ -161,6 +160,29 @@ fn create_seatbelt_args_routes_network_through_proxy_ports() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dynamic_network_policy_allows_tls_without_darwin_user_cache_write() {
|
||||
let policy = dynamic_network_policy(
|
||||
&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
},
|
||||
/*enforce_managed_network*/ false,
|
||||
&ProxyPolicyInputs::default(),
|
||||
);
|
||||
|
||||
assert!(
|
||||
policy.contains("(global-name \"com.apple.trustd.agent\")"),
|
||||
"policy should keep trustd agent access for TLS certificate verification:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
!policy.contains("DARWIN_USER_CACHE_DIR"),
|
||||
"network policy should not grant broad user cache writes:\n{policy}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() {
|
||||
let unreadable = absolute_path("/tmp/codex-unreadable");
|
||||
@@ -941,14 +963,6 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
writable_definitions, expected_definitions,
|
||||
"unexpected writable-root parameter definitions in {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:#?}"
|
||||
);
|
||||
}
|
||||
|
||||
let command_index = args
|
||||
.iter()
|
||||
.position(|arg| arg == "--")
|
||||
@@ -1311,14 +1325,6 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
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:#?}"
|
||||
);
|
||||
}
|
||||
|
||||
let command_index = args
|
||||
.iter()
|
||||
.position(|arg| arg == "--")
|
||||
|
||||
Reference in New Issue
Block a user