mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Improve macOS Seatbelt network and unix socket handling (#12702)
This improves macOS Seatbelt handling for sandboxed tool processes. ## Changes - Allow dual-stack local binding in proxy-managed sessions, while still keeping traffic limited to loopback and configured proxy endpoints. - Replace the old generic unix-socket path rule with explicit AF_UNIX permissions for socket creation, bind, and outbound connect. - Keep explicitly approved wrapper sockets connect-only. Local helper servers are less likely to fail when binding on macOS. Tools using local unix-socket IPC should work more reliably under the sandbox. Full-network sessions, proxy fail-closed behavior, and proxy lifecycle are unchanged.
This commit is contained in:
@@ -129,7 +129,7 @@ impl Default for UnixDomainSocketPolicy {
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct UnixSocketPathParam {
|
||||
key: String,
|
||||
index: usize,
|
||||
path: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
@@ -200,17 +200,23 @@ fn unix_socket_path_params(proxy: &ProxyPolicyInputs) -> Vec<UnixSocketPathParam
|
||||
deduped_paths
|
||||
.into_values()
|
||||
.enumerate()
|
||||
.map(|(index, path)| UnixSocketPathParam {
|
||||
key: format!("UNIX_SOCKET_PATH_{index}"),
|
||||
path,
|
||||
})
|
||||
.map(|(index, path)| UnixSocketPathParam { index, path })
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn unix_socket_path_param_key(index: usize) -> String {
|
||||
format!("UNIX_SOCKET_PATH_{index}")
|
||||
}
|
||||
|
||||
fn unix_socket_dir_params(proxy: &ProxyPolicyInputs) -> Vec<(String, PathBuf)> {
|
||||
unix_socket_path_params(proxy)
|
||||
.into_iter()
|
||||
.map(|param| (param.key, param.path.into_path_buf()))
|
||||
.map(|param| {
|
||||
(
|
||||
unix_socket_path_param_key(param.index),
|
||||
param.path.into_path_buf(),
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -218,17 +224,39 @@ fn unix_socket_dir_params(proxy: &ProxyPolicyInputs) -> Vec<(String, PathBuf)> {
|
||||
/// When non-empty, the returned string is newline-terminated so callers can
|
||||
/// append it directly to larger policy blocks.
|
||||
fn unix_socket_policy(proxy: &ProxyPolicyInputs) -> String {
|
||||
let socket_params = unix_socket_path_params(proxy);
|
||||
let has_unix_socket_access = matches!(
|
||||
proxy.unix_domain_socket_policy,
|
||||
UnixDomainSocketPolicy::AllowAll
|
||||
) || !socket_params.is_empty();
|
||||
if !has_unix_socket_access {
|
||||
return String::new();
|
||||
}
|
||||
|
||||
let mut policy = String::new();
|
||||
policy.push_str("(allow system-socket (socket-domain AF_UNIX))\n");
|
||||
if matches!(
|
||||
proxy.unix_domain_socket_policy,
|
||||
UnixDomainSocketPolicy::AllowAll
|
||||
) {
|
||||
return "(allow network* (subpath \"/\"))\n".to_string();
|
||||
// Keep AllowAll genuinely broad here; path qualifiers look narrower
|
||||
// without a clear macOS behavioral benefit.
|
||||
policy.push_str("(allow network-bind (local unix-socket))\n");
|
||||
policy.push_str("(allow network-outbound (remote unix-socket))\n");
|
||||
return policy;
|
||||
}
|
||||
|
||||
unix_socket_path_params(proxy)
|
||||
.iter()
|
||||
.map(|param| format!("(allow network* (subpath (param \"{}\")))\n", param.key))
|
||||
.collect()
|
||||
for param in socket_params {
|
||||
let key = unix_socket_path_param_key(param.index);
|
||||
// Use subpath so allowlists cover sockets created beneath approved directories.
|
||||
policy.push_str(&format!(
|
||||
"(allow network-bind (local unix-socket (subpath (param \"{key}\"))))\n"
|
||||
));
|
||||
policy.push_str(&format!(
|
||||
"(allow network-outbound (remote unix-socket (subpath (param \"{key}\"))))\n"
|
||||
));
|
||||
}
|
||||
policy
|
||||
}
|
||||
|
||||
fn dynamic_network_policy(
|
||||
@@ -236,11 +264,12 @@ fn dynamic_network_policy(
|
||||
enforce_managed_network: bool,
|
||||
proxy: &ProxyPolicyInputs,
|
||||
) -> String {
|
||||
if !proxy.ports.is_empty() {
|
||||
let mut policy =
|
||||
String::from("; allow outbound access only to configured loopback proxy endpoints\n");
|
||||
let should_use_restricted_network_policy =
|
||||
!proxy.ports.is_empty() || proxy.has_proxy_config || enforce_managed_network;
|
||||
if should_use_restricted_network_policy {
|
||||
let mut policy = String::new();
|
||||
if proxy.allow_local_binding {
|
||||
policy.push_str("; allow localhost-only binding and loopback traffic\n");
|
||||
policy.push_str("; allow loopback local binding and loopback traffic\n");
|
||||
policy.push_str("(allow network-bind (local ip \"localhost:*\"))\n");
|
||||
policy.push_str("(allow network-inbound (local ip \"localhost:*\"))\n");
|
||||
policy.push_str("(allow network-outbound (remote ip \"localhost:*\"))\n");
|
||||
@@ -258,18 +287,6 @@ fn dynamic_network_policy(
|
||||
return format!("{policy}{MACOS_SEATBELT_NETWORK_POLICY}");
|
||||
}
|
||||
|
||||
if proxy.has_proxy_config {
|
||||
// Proxy configuration is present but we could not infer any valid loopback endpoints.
|
||||
// Fail closed to avoid silently widening network access in proxy-enforced sessions.
|
||||
return String::new();
|
||||
}
|
||||
|
||||
if enforce_managed_network {
|
||||
// Managed network requirements are active but no usable proxy endpoints
|
||||
// are available. Fail closed for network access.
|
||||
return String::new();
|
||||
}
|
||||
|
||||
if sandbox_policy.has_full_network_access() {
|
||||
// No proxy env is configured: retain the existing full-network behavior.
|
||||
format!(
|
||||
@@ -681,7 +698,7 @@ sys.exit(0 if allowed else 13)
|
||||
|
||||
assert!(
|
||||
policy.contains("(allow network-bind (local ip \"localhost:*\"))"),
|
||||
"policy should allow loopback binding when explicitly enabled:\n{policy}"
|
||||
"policy should allow loopback local binding when explicitly enabled:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(allow network-inbound (local ip \"localhost:*\"))"),
|
||||
@@ -698,7 +715,7 @@ sys.exit(0 if allowed else 13)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dynamic_network_policy_fails_closed_when_proxy_config_without_ports() {
|
||||
fn dynamic_network_policy_preserves_restricted_policy_when_proxy_config_without_ports() {
|
||||
let policy = dynamic_network_policy(
|
||||
&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
@@ -716,6 +733,10 @@ sys.exit(0 if allowed else 13)
|
||||
},
|
||||
);
|
||||
|
||||
assert!(
|
||||
policy.contains("(socket-domain AF_SYSTEM)"),
|
||||
"policy should keep the restricted network profile when proxy config is present without ports:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
!policy.contains("\n(allow network-outbound)\n"),
|
||||
"policy should not include blanket outbound allowance when proxy config is present without ports:\n{policy}"
|
||||
@@ -727,7 +748,8 @@ sys.exit(0 if allowed else 13)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dynamic_network_policy_fails_closed_for_managed_network_without_proxy_config() {
|
||||
fn dynamic_network_policy_preserves_restricted_policy_for_managed_network_without_proxy_config()
|
||||
{
|
||||
let policy = dynamic_network_policy(
|
||||
&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
@@ -745,7 +767,14 @@ sys.exit(0 if allowed else 13)
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(policy, "");
|
||||
assert!(
|
||||
policy.contains("(socket-domain AF_SYSTEM)"),
|
||||
"policy should keep the restricted network profile when managed network is active without proxy endpoints:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
!policy.contains("\n(allow network-outbound)\n"),
|
||||
"policy should not include blanket outbound allowance when managed network is active without proxy endpoints:\n{policy}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -764,8 +793,24 @@ sys.exit(0 if allowed else 13)
|
||||
);
|
||||
|
||||
assert!(
|
||||
policy.contains("(allow network* (subpath (param \"UNIX_SOCKET_PATH_0\")))"),
|
||||
"policy should allow explicitly configured unix sockets:\n{policy}"
|
||||
policy.contains("(allow system-socket (socket-domain AF_UNIX))"),
|
||||
"policy should allow AF_UNIX socket creation for configured unix sockets:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains(
|
||||
"(allow network-bind (local unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))"
|
||||
),
|
||||
"policy should allow binding explicitly configured unix sockets:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains(
|
||||
"(allow network-outbound (remote unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))"
|
||||
),
|
||||
"policy should allow connecting to explicitly configured unix sockets:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
!policy.contains("(allow network* (subpath"),
|
||||
"policy should no longer use the generic subpath unix-socket rules:\n{policy}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -839,8 +884,20 @@ sys.exit(0 if allowed else 13)
|
||||
);
|
||||
|
||||
assert!(
|
||||
policy.contains("(allow network* (subpath \"/\"))"),
|
||||
"policy should allow all unix sockets when flag is enabled:\n{policy}"
|
||||
policy.contains("(allow system-socket (socket-domain AF_UNIX))"),
|
||||
"policy should allow AF_UNIX socket creation when unix sockets are enabled:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(allow network-bind (local unix-socket))"),
|
||||
"policy should allow binding unix sockets when enabled:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(allow network-outbound (remote unix-socket))"),
|
||||
"policy should allow connecting to unix sockets when enabled:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
!policy.contains("(allow network* (subpath"),
|
||||
"policy should no longer use the generic subpath unix-socket rules:\n{policy}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user