From c26d42ab69dd17c9ac2732aff8fc84aea8099c34 Mon Sep 17 00:00:00 2001 From: Parker Thompson Date: Thu, 14 Aug 2025 17:12:41 -0700 Subject: [PATCH] Fix AF_UNIX, sockpair, recvfrom in linux sandbox (#2309) When using codex-tui on a linux system I was unable to run `cargo clippy` inside of codex due to: ``` [pid 3548377] socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, [pid 3548370] close(8 [pid 3548377] <... socketpair resumed>0x7ffb97f4ed60) = -1 EPERM (Operation not permitted) ``` And ``` 3611300 <... recvfrom resumed>0x708b8b5cffe0, 8, 0, NULL, NULL) = -1 EPERM (Operation not permitted) ``` This PR: * Fixes a bug that disallowed AF_UNIX to allow it on `socket()` * Adds recvfrom() to the syscall allow list, this should be fine since we disable opening new sockets. But we should validate there is not a open socket inheritance issue. * Allow socketpair to be called for AF_UNIX * Adds tests for AF_UNIX components * All of which allows running `cargo clippy` within the sandbox on linux, and possibly other tooling using a fork server model + AF_UNIX comms. --- codex-rs/Cargo.lock | 1 + codex-rs/exec/Cargo.toml | 1 + codex-rs/exec/tests/sandbox.rs | 128 +++++++++++++++++++++++++ codex-rs/linux-sandbox/src/landlock.rs | 10 +- 4 files changed, 136 insertions(+), 4 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8f077bc0..ef07a36d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -751,6 +751,7 @@ dependencies = [ "codex-common", "codex-core", "codex-ollama", + "libc", "owo-colors", "predicates", "serde_json", diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index aee480d7..b7c20df3 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -41,5 +41,6 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } [dev-dependencies] assert_cmd = "2" +libc = "0.2" predicates = "3" tempfile = "3.13.0" diff --git a/codex-rs/exec/tests/sandbox.rs b/codex-rs/exec/tests/sandbox.rs index 8cc31bba..22caf8ab 100644 --- a/codex-rs/exec/tests/sandbox.rs +++ b/codex-rs/exec/tests/sandbox.rs @@ -4,7 +4,10 @@ use codex_core::protocol::SandboxPolicy; use codex_core::spawn::StdioPolicy; use std::collections::HashMap; +use std::future::Future; +use std::io; use std::path::PathBuf; +use std::process::ExitStatus; use tokio::process::Child; #[cfg(target_os = "macos")] @@ -90,3 +93,128 @@ if __name__ == '__main__': let status = child.wait().await.expect("should wait for child process"); assert!(status.success(), "python exited with {status:?}"); } + +fn unix_sock_body() { + unsafe { + let mut fds = [0i32; 2]; + let r = libc::socketpair(libc::AF_UNIX, libc::SOCK_DGRAM, 0, fds.as_mut_ptr()); + assert_eq!( + r, + 0, + "socketpair(AF_UNIX, SOCK_DGRAM) failed: {}", + io::Error::last_os_error() + ); + + let msg = b"hello_unix"; + // write() from one end (generic write is allowed) + let sent = libc::write(fds[0], msg.as_ptr() as *const libc::c_void, msg.len()); + assert!(sent >= 0, "write() failed: {}", io::Error::last_os_error()); + + // recvfrom() on the other end. We don’t need the address for socketpair, + // so we pass null pointers for src address. + let mut buf = [0u8; 64]; + let recvd = libc::recvfrom( + fds[1], + buf.as_mut_ptr() as *mut libc::c_void, + buf.len(), + 0, + std::ptr::null_mut(), + std::ptr::null_mut(), + ); + assert!( + recvd >= 0, + "recvfrom() failed: {}", + io::Error::last_os_error() + ); + + let recvd_slice = &buf[..(recvd as usize)]; + assert_eq!( + recvd_slice, + &msg[..], + "payload mismatch: sent {} bytes, got {} bytes", + msg.len(), + recvd + ); + + // Also exercise AF_UNIX stream socketpair quickly to ensure AF_UNIX in general works. + let mut sfds = [0i32; 2]; + let sr = libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, sfds.as_mut_ptr()); + assert_eq!( + sr, + 0, + "socketpair(AF_UNIX, SOCK_STREAM) failed: {}", + io::Error::last_os_error() + ); + let snt2 = libc::write(sfds[0], msg.as_ptr() as *const libc::c_void, msg.len()); + assert!( + snt2 >= 0, + "write(stream) failed: {}", + io::Error::last_os_error() + ); + let mut b2 = [0u8; 64]; + let rcv2 = libc::recv(sfds[1], b2.as_mut_ptr() as *mut libc::c_void, b2.len(), 0); + assert!( + rcv2 >= 0, + "recv(stream) failed: {}", + io::Error::last_os_error() + ); + + // Clean up + let _ = libc::close(sfds[0]); + let _ = libc::close(sfds[1]); + let _ = libc::close(fds[0]); + let _ = libc::close(fds[1]); + } +} + +#[tokio::test] +async fn allow_unix_socketpair_recvfrom() { + run_code_under_sandbox( + "allow_unix_socketpair_recvfrom", + &SandboxPolicy::ReadOnly, + || async { unix_sock_body() }, + ) + .await + .expect("should be able to reexec"); +} + +const IN_SANDBOX_ENV_VAR: &str = "IN_SANDBOX"; + +pub async fn run_code_under_sandbox( + test_selector: &str, + policy: &SandboxPolicy, + child_body: F, +) -> io::Result> +where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, +{ + if std::env::var(IN_SANDBOX_ENV_VAR).is_err() { + let exe = std::env::current_exe()?; + let mut cmds = vec![exe.to_string_lossy().into_owned(), "--exact".into()]; + let mut stdio_policy = StdioPolicy::RedirectForShellTool; + // Allow for us to pass forward --nocapture / use the right stdio policy. + if std::env::args().any(|a| a == "--nocapture") { + cmds.push("--nocapture".into()); + stdio_policy = StdioPolicy::Inherit; + } + cmds.push(test_selector.into()); + + // Your existing launcher: + let mut child = spawn_command_under_sandbox( + cmds, + policy, + std::env::current_dir().expect("should be able to get current dir"), + stdio_policy, + HashMap::from([("IN_SANDBOX".into(), "1".into())]), + ) + .await?; + + let status = child.wait().await?; + Ok(Some(status)) + } else { + // Child branch: run the provided body. + child_body().await; + Ok(None) + } +} diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index e13e3c8b..5bc96130 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -104,7 +104,9 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), deny_syscall(libc::SYS_sendto); deny_syscall(libc::SYS_sendmsg); deny_syscall(libc::SYS_sendmmsg); - deny_syscall(libc::SYS_recvfrom); + // NOTE: allowing recvfrom allows some tools like: `cargo clippy` to run + // with their socketpair + child processes for sub-proc management + // deny_syscall(libc::SYS_recvfrom); deny_syscall(libc::SYS_recvmsg); deny_syscall(libc::SYS_recvmmsg); deny_syscall(libc::SYS_getsockopt); @@ -115,12 +117,12 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new( 0, // first argument (domain) SeccompCmpArgLen::Dword, - SeccompCmpOp::Eq, + SeccompCmpOp::Ne, libc::AF_UNIX as u64, )?])?; - rules.insert(libc::SYS_socket, vec![unix_only_rule]); - rules.insert(libc::SYS_socketpair, vec![]); // always deny (Unix can use socketpair but fine, keep open?) + rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]); + rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); // always deny (Unix can use socketpair but fine, keep open?) let filter = SeccompFilter::new( rules,