From 9203f17b0e3968a8c0761f869b1f0879a3b302b5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 5 Mar 2026 15:39:54 -0800 Subject: [PATCH] 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. --- codex-rs/core/src/seatbelt.rs | 127 ++++++++++++++++++++++++---------- 1 file changed, 92 insertions(+), 35 deletions(-) diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index c9c280120..8d556a8ee 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -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 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}" ); }