Support Unix socket allowlists in macOS sandbox

[aaronl-openai]

Changes

Allows sandboxes to restrict overall network access while granting access to specific unix sockets on mac.

Details

• codex sandbox macos: adds a repeatable --allow-unix-socket option.
• codex-sandboxing: threads explicit Unix socket roots into the macOS Seatbelt profile generation.
• Preserves restricted network behavior when only Unix socket IPC is requested, and preserves full network behavior when full network is already enabled.

Verification

• cargo test -p codex-cli -p codex-sandboxing
• cargo build -p codex-cli --bin codex
• verified that codex sandbox macos --allow-unix-socket /tmp/test.sock -- test-client grants access as expected

[bolinfest]

proxy_policy_inputs() has only one callsite. Why not update it to take extra_allow_unix_sockets so that we don't have to introduce with_extra_unix_sockets() which takes mut self?

[bolinfest]

Maybe this also merits using a builder pattern, but we have these three things calling each other right now, each adding one param:

create_seatbelt_command_args() ->
create_seatbelt_command_args_for_policies() -> create_seatbelt_command_args_for_policies_with_extra_unix_sockets()

This feels a bit much: can we just have one function that takes a struct with the full set of arguments?

If we feel strongly, we could make the struct impl Default so it's easier to elide the arguments with trivial default values when constructing it.

[bolinfest]

Suggested change

	AbsolutePathBuf::from_absolute_path_checked(raw)
	AbsolutePathBuf::relative_to_current_dir(raw)

[bolinfest]

Since raw is &str, you don't need :? to print the Debug version of the value.

Suggested change

	.map_err(|err| format!("invalid path {raw:?}: {err}"))
	.map_err(|err| format!("invalid path {raw}: {err}"))

[bolinfest]

Instead of these top-level imports with #[cfg(target_os = "macos")], you might want to just do the imports inside the SandboxType::MacosSeatbelt case and then you can drop the cfg on each import.

[bolinfest]

This is better because if a new variant is added to the UnixDomainSocketPolicy enum, this forces the match to be updated:

Suggested change

	let has_unix_socket_access = matches!(
	proxy.unix_domain_socket_policy,
	UnixDomainSocketPolicy::AllowAll
	) || matches!(
	&proxy.unix_domain_socket_policy,
	UnixDomainSocketPolicy::Restricted { allowed } if !allowed.is_empty()
	);
	let has_unix_socket_access = match proxy.unix_domain_socket_policy {
	UnixDomainSocketPolicy::AllowAll => true,
	UnixDomainSocketPolicy::Restricted { allowed } => !allowed.is_empty()
	};

[bolinfest]

Maybe has_some_unix_socket_access is more accurate since it is true when Restricted is non-empty, but that is not quite a strong a statement as AllowAll?

[bolinfest]

So was it an oversight that the result of unix_socket_policy() was not included previously?

[aaronl-openai]

yeah, I think so. I think it mostly wasn’t an issue before because the unix socket policy was exercised through the proxy/restricted-network path.

[bolinfest]

nit: I would name this SeatbeltCommandParams since create_seatbelt_command_args() takes this struct as an argument, it doesn't create it.

[bolinfest]

Or even CreateSeatbeltCommandArgsParams because it's the "params" for the create_seatbelt_command_args() function?

[bolinfest]

We got burned by async_trait recently: #16630. Prefer native async traits, which effectively makes #[async_trait::async_trait] unnecessary.

[bolinfest]

Oh, I see, we already have:

#[async_trait]
pub trait ConfigReloader: Send + Sync {
    /// Human-readable description of where config is loaded from, for logs.
    fn source_label(&self) -> String;

    /// Return a freshly loaded state if a reload is needed; otherwise, return `None`.
    async fn maybe_reload(&self) -> Result<Option<ConfigState>>;

    /// Force a reload, regardless of whether a change was detected.
    async fn reload_now(&self) -> Result<ConfigState>;
}

OK, we can let this be for now, but I'll go after it in a follow-up...