diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 439dea79c6..289637530a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2412,6 +2412,7 @@ dependencies = [ "hmac 0.12.1", "httpdate", "jsonwebtoken", + "libc", "owo-colors", "pretty_assertions", "rand 0.9.3", @@ -3786,6 +3787,7 @@ dependencies = [ "codex-process-hardening", "codex-protocol", "codex-sandboxing", + "codex-uds", "codex-utils-absolute-path", "globset", "landlock", @@ -4466,6 +4468,7 @@ dependencies = [ "codex-network-proxy", "codex-otel", "codex-protocol", + "codex-uds", "codex-utils-absolute-path", "codex-utils-home-dir", "codex-utils-path-uri", @@ -4866,6 +4869,7 @@ name = "codex-uds" version = "0.0.0" dependencies = [ "async-io", + "libc", "pretty_assertions", "tempfile", "tokio", diff --git a/codex-rs/app-server-transport/Cargo.toml b/codex-rs/app-server-transport/Cargo.toml index ef56ca54a0..9b91662c40 100644 --- a/codex-rs/app-server-transport/Cargo.toml +++ b/codex-rs/app-server-transport/Cargo.toml @@ -57,6 +57,7 @@ url = { workspace = true } uuid = { workspace = true, features = ["serde", "v7"] } [target.'cfg(unix)'.dependencies] +libc = { workspace = true } signal-hook = { workspace = true } [dev-dependencies] diff --git a/codex-rs/app-server-transport/src/lib.rs b/codex-rs/app-server-transport/src/lib.rs index 6eacd60a8b..f10b2d3810 100644 --- a/codex-rs/app-server-transport/src/lib.rs +++ b/codex-rs/app-server-transport/src/lib.rs @@ -37,7 +37,6 @@ pub use transport::app_server_control_socket_path; pub use transport::app_server_startup_lock_path; pub use transport::auth; pub use transport::daemon_recovery_file_path; -pub use transport::prepare_control_socket_path; pub use transport::start_control_socket_acceptor; pub use transport::start_remote_control; pub use transport::start_stdio_connection; diff --git a/codex-rs/app-server-transport/src/transport/mod.rs b/codex-rs/app-server-transport/src/transport/mod.rs index 56faef69b0..8931fdfb7f 100644 --- a/codex-rs/app-server-transport/src/transport/mod.rs +++ b/codex-rs/app-server-transport/src/transport/mod.rs @@ -46,7 +46,6 @@ pub use stdio::start_stdio_connection; pub use unix_socket::AppServerStartupLock; pub use unix_socket::DaemonShutdownAccess; pub use unix_socket::acquire_app_server_startup_lock; -pub use unix_socket::prepare_control_socket_path; pub use unix_socket::start_control_socket_acceptor; pub use websocket::start_websocket_acceptor; diff --git a/codex-rs/app-server-transport/src/transport/unix_socket.rs b/codex-rs/app-server-transport/src/transport/unix_socket.rs index 0d55d58ea5..0d6e0a9b7c 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket.rs @@ -39,6 +39,45 @@ pub async fn start_control_socket_acceptor( shutdown_token: CancellationToken, daemon_shutdown_access: DaemonShutdownAccess, ) -> IoResult> { + #[cfg(unix)] + let (socket_path, rendezvous_path, _startup_lock) = { + use std::os::unix::fs::MetadataExt; + + if let Some(parent) = socket_path.as_path().parent() { + // The advertised path is only an alias; preserve existing parents. + match tokio::fs::DirBuilder::new() + .mode(0o700) + .create(parent) + .await + { + Ok(()) => {} + Err(error) if error.kind() == ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + let metadata = std::fs::metadata(parent)?; + if !metadata.is_dir() + || (metadata.uid() != 0 && metadata.uid() != unsafe { libc::geteuid() }) + || (metadata.mode() & 0o022 != 0 && metadata.mode() & 0o1000 == 0) + { + return Err(std::io::Error::new( + ErrorKind::PermissionDenied, + "socket parent must be owned by the user or root and prevent other users from replacing socket entries", + )); + } + } + codex_uds::prepare_shared_daemon_socket_directory()?; + let physical_path = protected_socket_path(socket_path.as_path())?; + let lock = acquire_app_server_startup_lock(AbsolutePathBuf::from_absolute_path_checked( + physical_path.with_extension("lock"), + )?) + .await?; + prepare_control_socket_path(socket_path.as_path()).await?; + ( + AbsolutePathBuf::from_absolute_path_checked(physical_path)?, + socket_path, + lock, + ) + }; #[cfg(windows)] let (socket_path, directory_guard) = { if let Some(parent) = socket_path.as_path().parent() { @@ -51,10 +90,17 @@ pub async fn start_control_socket_acceptor( let listener = UnixListener::bind(socket_path.as_path()).await?; let socket_guard = ControlSocketFileGuard { socket_path, + #[cfg(unix)] + rendezvous_path, #[cfg(windows)] _directory_guard: directory_guard, }; set_control_socket_permissions(socket_guard.socket_path.as_path()).await?; + #[cfg(unix)] + std::os::unix::fs::symlink( + socket_guard.socket_path.as_path(), + socket_guard.rendezvous_path.as_path(), + )?; info!( socket_path = %socket_guard.socket_path.display(), "app-server control socket listening" @@ -155,11 +201,8 @@ async fn run_daemon_shutdown( .await; } -pub async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { - if let Some(parent) = socket_path.parent() { - codex_uds::prepare_private_socket_directory(parent).await?; - } - +// Unix callers hold the physical socket's startup lock through bind and publication. +async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { #[cfg(windows)] let (socket_path, _directory_guard) = codex_uds::validate_private_socket_path(socket_path)?; #[cfg(windows)] @@ -177,7 +220,7 @@ pub async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { ), )); } - Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()), + Err(err) if err.kind() == ErrorKind::NotFound => {} Err(err) if err.kind() == ErrorKind::ConnectionRefused => {} Err(err) => { if !socket_path.exists() { @@ -187,6 +230,15 @@ pub async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { } } + // A crashed daemon can leave a dangling rendezvous symlink. Only recognize + // our own deterministic alias; never remove an arbitrary symlink target. + #[cfg(unix)] + if std::fs::read_link(socket_path).ok().as_deref() + == Some(protected_socket_path(socket_path)?.as_path()) + { + return tokio::fs::remove_file(socket_path).await; + } + if !socket_path.try_exists()? { return Ok(()); } @@ -203,6 +255,23 @@ pub async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { tokio::fs::remove_file(socket_path).await } +#[cfg(unix)] +fn protected_socket_path(rendezvous_path: &Path) -> IoResult { + use sha2::Digest; + use sha2::Sha256; + use std::os::unix::ffi::OsStrExt; + + let parent = rendezvous_path.parent().ok_or_else(|| { + std::io::Error::new(ErrorKind::InvalidInput, "socket path must have a parent") + })?; + let name = rendezvous_path.file_name().ok_or_else(|| { + std::io::Error::new(ErrorKind::InvalidInput, "socket path must have a filename") + })?; + let path = std::fs::canonicalize(parent)?.join(name); + let hash = Sha256::digest(path.as_os_str().as_bytes()); + Ok(codex_uds::shared_daemon_socket_directory()?.join(format!("{hash:x}"))) +} + pub struct AppServerStartupLock { _file: std::fs::File, } @@ -245,6 +314,8 @@ async fn set_control_socket_permissions(_socket_path: &Path) -> IoResult<()> { struct ControlSocketFileGuard { socket_path: AbsolutePathBuf, + #[cfg(unix)] + rendezvous_path: AbsolutePathBuf, // Keep the directory pinned until after the socket file is removed in Drop. #[cfg(windows)] _directory_guard: std::os::windows::io::OwnedHandle, @@ -252,6 +323,14 @@ struct ControlSocketFileGuard { impl Drop for ControlSocketFileGuard { fn drop(&mut self) { + #[cfg(unix)] + if std::fs::read_link(self.rendezvous_path.as_path()) + .ok() + .as_deref() + == Some(self.socket_path.as_path()) + { + let _ = std::fs::remove_file(self.rendezvous_path.as_path()); + } if let Err(err) = std::fs::remove_file(self.socket_path.as_path()) && err.kind() != ErrorKind::NotFound { diff --git a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs index 55637c015a..b83a2eb094 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs @@ -246,6 +246,35 @@ async fn app_server_startup_lock_serializes_waiters() { .expect("second startup lock should succeed"); } +#[cfg(unix)] +#[tokio::test] +async fn control_socket_rejects_writable_parent_without_changing_permissions() { + use std::os::unix::fs::PermissionsExt; + + let directory = tempfile::tempdir().unwrap(); + std::fs::set_permissions(directory.path(), std::fs::Permissions::from_mode(0o777)).unwrap(); + let path = AbsolutePathBuf::from_absolute_path(directory.path().join("rpc.sock")).unwrap(); + let (tx, _rx) = mpsc::channel(CHANNEL_CAPACITY); + let error = start_control_socket_acceptor( + path.clone(), + tx, + CancellationToken::new(), + DaemonShutdownAccess::Disabled, + ) + .await + .unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied); + assert_eq!( + std::fs::metadata(directory.path()) + .unwrap() + .permissions() + .mode() + & 0o777, + 0o777 + ); + assert!(std::fs::symlink_metadata(path).is_err()); +} + #[cfg(unix)] #[tokio::test] async fn control_socket_file_is_private_after_bind() { @@ -253,6 +282,9 @@ async fn control_socket_file_is_private_after_bind() { let temp_dir = tempfile::TempDir::new().expect("temp dir"); let socket_path = test_socket_path(temp_dir.path()); + let parent = socket_path.as_path().parent().unwrap(); + std::fs::create_dir_all(parent).unwrap(); + std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o755)).unwrap(); let (transport_event_tx, _transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let shutdown_token = CancellationToken::new(); @@ -269,9 +301,60 @@ async fn control_socket_file_is_private_after_bind() { .await .expect("socket metadata should exist"); assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!( + std::fs::metadata(parent).unwrap().permissions().mode() & 0o777, + 0o755 + ); + let physical_path = std::fs::read_link(socket_path.as_path()).expect("rendezvous symlink"); + assert_eq!( + physical_path.parent(), + Some( + codex_uds::shared_daemon_socket_directory() + .unwrap() + .as_path() + ) + ); shutdown_token.cancel(); accept_handle.await.expect("acceptor should join"); + assert!(!physical_path.exists()); + assert!(std::fs::symlink_metadata(socket_path.as_path()).is_err()); + + // Simulate a dangling rendezvous left by an interrupted cleanup. + std::os::unix::fs::symlink(&physical_path, socket_path.as_path()).unwrap(); + let (sender, _receiver) = mpsc::channel::(CHANNEL_CAPACITY); + let shutdown = CancellationToken::new(); + let (first, second) = tokio::join!( + start_control_socket_acceptor( + socket_path.clone(), + sender.clone(), + shutdown.clone(), + DaemonShutdownAccess::Disabled, + ), + start_control_socket_acceptor( + socket_path.clone(), + sender, + shutdown.clone(), + DaemonShutdownAccess::Disabled, + ), + ); + let (acceptor, error) = match (first, second) { + (Ok(acceptor), Err(error)) | (Err(error), Ok(acceptor)) => (acceptor, error), + _ => panic!("exactly one concurrent restart should replace the stale symlink"), + }; + assert_eq!(error.kind(), std::io::ErrorKind::AddrInUse); + let _client = connect_to_socket(socket_path.as_path()).await.unwrap(); + + // Cleanup must not remove a replacement at the advertised path. + std::fs::remove_file(socket_path.as_path()).unwrap(); + std::fs::write(socket_path.as_path(), b"replacement").unwrap(); + shutdown.cancel(); + acceptor.await.unwrap(); + assert_eq!( + std::fs::read(socket_path.as_path()).unwrap(), + b"replacement" + ); + assert!(!physical_path.exists()); } #[cfg(windows)] diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 2c598458ed..e0149fa3ce 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -43,7 +43,6 @@ use crate::transport::TransportEvent; use crate::transport::acquire_app_server_startup_lock; use crate::transport::app_server_startup_lock_path; use crate::transport::auth::policy_from_settings; -use crate::transport::prepare_control_socket_path; use crate::transport::route_outgoing_envelope; use crate::transport::start_control_socket_acceptor; use crate::transport::start_remote_control; @@ -633,10 +632,9 @@ pub async fn run_main_with_transport_options( codex_core::otel_init::record_process_start(otel.as_ref(), OTEL_SERVICE_NAME); codex_core::otel_init::install_sqlite_telemetry(otel.as_ref(), OTEL_SERVICE_NAME); let unix_socket_startup_lock = match &transport { - AppServerTransport::UnixSocket { socket_path } => { + AppServerTransport::UnixSocket { .. } => { let startup_lock_path = app_server_startup_lock_path(&codex_home)?; let startup_lock = acquire_app_server_startup_lock(startup_lock_path).await?; - prepare_control_socket_path(socket_path.as_path()).await?; Some(startup_lock) } _ => None, diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 37493f58b8..a94162f032 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -30,7 +30,6 @@ pub(crate) use codex_app_server_transport::acquire_app_server_startup_lock; pub use codex_app_server_transport::app_server_control_socket_path; pub(crate) use codex_app_server_transport::app_server_startup_lock_path; pub use codex_app_server_transport::auth; -pub(crate) use codex_app_server_transport::prepare_control_socket_path; pub(crate) use codex_app_server_transport::start_control_socket_acceptor; pub(crate) use codex_app_server_transport::start_remote_control; pub(crate) use codex_app_server_transport::start_stdio_connection; diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 7655189c7e..17b3fa14b1 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -23,6 +23,7 @@ codex-network-proxy = { workspace = true } codex-process-hardening = { workspace = true } codex-protocol = { workspace = true } codex-sandboxing = { workspace = true } +codex-uds = { workspace = true } codex-utils-absolute-path = { workspace = true } globset = { workspace = true } landlock = { workspace = true } diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 07c6796470..e7904d3ed3 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -37,12 +37,9 @@ commands that would enter the bubblewrap path. - WSL2 uses the normal Linux bubblewrap path. - WSL1 is not supported for bubblewrap sandboxing; Codex rejects sandboxed shell commands that would require the bubblewrap path before invoking `bwrap`. -- Legacy Landlock + mount protections remain available as an explicit legacy - fallback path. -- Set `features.use_legacy_landlock = true` (or CLI `-c use_legacy_landlock=true`) - to force the legacy Landlock fallback. -- The legacy Landlock fallback is used only when the split filesystem policy is - sandbox-equivalent to the legacy model after `cwd` resolution. +- Filesystem-restricted execution requires bubblewrap. The legacy Landlock + option is rejected for these policies because it cannot isolate app-server + Unix sockets. Disable `features.use_legacy_landlock` when upgrading. - Split-only filesystem policies that do not round-trip through the legacy `SandboxPolicy` model stay on bubblewrap so nested read-only or denied carveouts are preserved. diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 0d84b9f444..418d28db03 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -368,6 +368,8 @@ fn create_bwrap_flags( } // Request a user namespace explicitly rather than relying on bubblewrap's // auto-enable behavior, which is skipped when the caller runs as uid 0. + // This also blocks host procfs root/cwd/fd links through ptrace permission + // checks, including when a container requires retaining the host procfs. args.push("--unshare-user".to_string()); args.push("--unshare-pid".to_string()); args.push("--unshare-ipc".to_string()); @@ -420,6 +422,13 @@ fn create_filesystem_args( cwd: &Path, options: BwrapOptions, ) -> Result { + let daemon_directory = codex_uds::prepare_shared_daemon_socket_directory()?; + crate::daemon_mounts::reject_daemon_mount_aliases( + &daemon_directory, + options + .mask_wslg_distro + .then_some(Path::new(WSLG_DISTRO_ROOT)), + )?; let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd); // Bubblewrap requires bind mount targets to exist. Skip missing writable // roots so mixed-platform configs can keep harmless paths for other @@ -494,7 +503,7 @@ fn create_filesystem_args( unreadable_roots.sort(); unreadable_roots.dedup(); - let args = if file_system_sandbox_policy.has_full_disk_read_access() { + let mut args = if file_system_sandbox_policy.has_full_disk_read_access() { // Read-only root, then mount a minimal device tree. // In bubblewrap (`bubblewrap.c`, `SETUP_MOUNT_DEV`), `--dev /dev` // creates the standard minimal nodes: null, zero, full, random, @@ -562,11 +571,15 @@ fn create_filesystem_args( args.push("--ro-bind".to_string()); args.push(path_to_string(&mount_root)); args.push(path_to_string(&mount_root)); + append_daemon_socket_mask(&mut args, &mount_root, &daemon_directory)?; } } args }; + if args[0] == "--ro-bind" { + append_daemon_socket_mask(&mut args, Path::new("/"), &daemon_directory)?; + } let mut bwrap_args = BwrapArgs { args, preserved_files: Vec::new(), @@ -624,6 +637,7 @@ fn create_filesystem_args( bwrap_args.args.push("--bind".to_string()); bwrap_args.args.push(path_to_string(mount_root)); bwrap_args.args.push(path_to_string(mount_root)); + append_daemon_socket_mask(&mut bwrap_args.args, mount_root, &daemon_directory)?; let mut read_only_subpaths: Vec = writable_root .read_only_subpaths @@ -761,6 +775,33 @@ fn append_metadata_path_masks_for_writable_root( } } +// Mask immediately after every bind that exposes the directory, before policy +// deny masks can hide its parent. Use the mount destination for read-root aliases. +fn append_daemon_socket_mask( + args: &mut Vec, + mount_root: &Path, + daemon_directory: &Path, +) -> Result<()> { + let source = fs::canonicalize(mount_root)?; + if source.starts_with(daemon_directory) { + return Err(CodexErr::Fatal( + "app-server socket directory cannot be a sandbox mount root".to_string(), + )); + } + if let Ok(relative) = daemon_directory.strip_prefix(source) { + let destination = path_to_string(&mount_root.join(relative)); + args.extend([ + "--perms".to_string(), + "000".to_string(), + "--tmpfs".to_string(), + destination.clone(), + "--remount-ro".to_string(), + destination, + ]); + } + Ok(()) +} + fn expand_unreadable_globs_with_ripgrep( patterns: &[String], cwd: &Path, @@ -1134,6 +1175,11 @@ fn append_read_only_subpath_args( bwrap_args.args.push("--ro-bind".to_string()); bwrap_args.args.push(path_to_string(subpath)); bwrap_args.args.push(path_to_string(subpath)); + append_daemon_socket_mask( + &mut bwrap_args.args, + subpath, + &codex_uds::shared_daemon_socket_directory()?, + )?; } Ok(()) } @@ -2129,6 +2175,8 @@ mod tests { PathBuf::from("/dev/.codex"), ] ); + let daemon_directory = + path_to_string(&codex_uds::shared_daemon_socket_directory().unwrap()); assert_eq!( args.args, vec![ @@ -2139,10 +2187,22 @@ mod tests { // Recreate a writable /dev inside the sandbox. "--dev".to_string(), "/dev".to_string(), + "--perms".to_string(), + "000".to_string(), + "--tmpfs".to_string(), + daemon_directory.clone(), + "--remount-ro".to_string(), + daemon_directory.clone(), // Make the writable root itself writable again. "--bind".to_string(), "/".to_string(), "/".to_string(), + "--perms".to_string(), + "000".to_string(), + "--tmpfs".to_string(), + daemon_directory.clone(), + "--remount-ro".to_string(), + daemon_directory, // Mask the default metadata path names under the writable root. // Because the root is `/` in this test, these carveout paths // appear directly below `/`. @@ -2196,6 +2256,44 @@ mod tests { ); } + #[test] + fn read_only_tmp_carveout_keeps_daemon_sockets_masked() { + let policy = FileSystemSandboxPolicy::restricted( + [ + ("/", FileSystemAccessMode::Write), + ("/tmp", FileSystemAccessMode::Read), + ] + .into_iter() + .map(|(path, access)| { + FileSystemSandboxEntry::new( + AbsolutePathBuf::from_absolute_path(path).unwrap().into(), + access, + ) + }) + .collect(), + ); + let args = create_filesystem_args(&policy, Path::new("/"), BwrapOptions::default()) + .expect("filesystem args"); + let tmp_bind = args + .args + .windows(3) + .position(|window| window == ["--ro-bind", "/tmp", "/tmp"]) + .expect("read-only tmp bind"); + let daemon_directory = + path_to_string(&codex_uds::shared_daemon_socket_directory().unwrap()); + assert_eq!( + &args.args[tmp_bind + 3..tmp_bind + 9], + [ + "--perms", + "000", + "--tmpfs", + &daemon_directory, + "--remount-ro", + &daemon_directory + ], + ); + } + #[test] fn restricted_read_only_uses_scoped_read_roots_instead_of_erroring() { let temp_dir = TempDir::new().expect("temp dir"); diff --git a/codex-rs/linux-sandbox/src/daemon_mounts.rs b/codex-rs/linux-sandbox/src/daemon_mounts.rs new file mode 100644 index 0000000000..713688b59c --- /dev/null +++ b/codex-rs/linux-sandbox/src/daemon_mounts.rs @@ -0,0 +1,115 @@ +//! Reject host mount aliases that would bypass the privileged socket directory mask. +//! Mount roots describe filesystem identity; canonical paths alone miss bind mounts. + +use std::collections::BTreeSet; +use std::fs; +use std::io; +use std::os::unix::ffi::OsStringExt; +use std::os::unix::fs::MetadataExt; +use std::path::Path; +use std::path::PathBuf; + +pub(crate) fn reject_daemon_mount_aliases( + directory: &Path, + masked_root: Option<&Path>, +) -> io::Result<()> { + let device = fs::metadata(directory)?.dev(); + check_mounts( + directory, + &format!("{}:{}", libc::major(device), libc::minor(device)), + &fs::read("/proc/self/mountinfo")?, + masked_root, + ) +} + +fn check_mounts( + directory: &Path, + device: &str, + mountinfo: &[u8], + masked_root: Option<&Path>, +) -> io::Result<()> { + let invalid = || io::Error::other("cannot establish app-server socket mount isolation"); + let mut mounts = Vec::new(); + let mut locations = BTreeSet::new(); + for line in mountinfo + .split(|byte| *byte == b'\n') + .filter(|line| !line.is_empty()) + { + let fields: Vec<_> = line.split(|byte| *byte == b' ').take(5).collect(); + let [_, _, mount_device, root, destination] = fields.as_slice() else { + return Err(invalid()); + }; + let root = mount_path(root)?; + let destination = mount_path(destination)?; + if *mount_device == device.as_bytes() + && let Ok(relative) = directory.strip_prefix(&destination) + { + locations.insert(root.join(relative)); + } + mounts.push((*mount_device, root, destination)); + } + // Overmounts can leave hidden entries in mountinfo. Require every possible + // containing mount to agree instead of guessing which root is visible. + if locations.len() != 1 { + return Err(invalid()); + } + let location = locations.into_iter().next().ok_or_else(invalid)?; + for (mount_device, root, destination) in &mounts { + // Nested mounts can introduce another filesystem (or an individual socket) under the mask. + let nested = destination != directory && destination.starts_with(directory); + let alias = if *mount_device == device.as_bytes() { + if let Ok(relative) = location.strip_prefix(root) { + Some(destination.join(relative)) + } else if root.starts_with(&location) { + Some(destination.clone()) + } else { + None + } + } else { + None + }; + if nested + || alias.is_some_and(|path| { + !path.starts_with(directory) + && !masked_root.is_some_and(|root| path.starts_with(root)) + }) + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "app-server socket directory has an unsupported host mount at {}; remove the bind-mount alias or nested mount before starting the sandbox", + destination.display() + ), + )); + } + } + Ok(()) +} + +fn mount_path(encoded: &[u8]) -> io::Result { + let mut decoded = Vec::new(); + let mut bytes = encoded.iter().copied(); + while let Some(byte) = bytes.next() { + decoded.push(if byte == b'\\' { + let digits: Vec<_> = bytes.by_ref().take(3).collect(); + match digits.as_slice() { + b"040" => b' ', + b"011" => b'\t', + b"012" => b'\n', + b"134" => b'\\', + _ => return Err(io::Error::other("invalid mountinfo path escape")), + } + } else { + byte + }); + } + let path = PathBuf::from(std::ffi::OsString::from_vec(decoded)); + if !path.is_absolute() { + return Err(io::Error::other("mountinfo path is not absolute")); + } + Ok(path) +} + +#[cfg(test)] +#[path = "daemon_mounts_tests.rs"] +mod tests; diff --git a/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs new file mode 100644 index 0000000000..04e0fe5d25 --- /dev/null +++ b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs @@ -0,0 +1,63 @@ +use super::*; +use pretty_assertions::assert_eq; +use test_case::test_case; + +// Most cases have no independently masked subtree. +fn check_mounts(directory: &Path, device: &str, mountinfo: &[u8]) -> io::Result<()> { + super::check_mounts(directory, device, mountinfo, /*masked_root*/ None) +} + +#[test_case("/tmp", "/host-tmp", false; "ancestor alias")] +#[test_case("/tmp/codex-daemon-1000", "/alias", false; "directory alias")] +#[test_case("/tmp/codex-daemon-1000/rpc.sock", "/alias.sock", false; "socket alias")] +#[test_case("/", "/host", false; "root alias")] +#[test_case("/workspace", "/project", true; "unrelated project bind")] +#[test_case("/tmp", "/tmp", true; "same location")] +#[test_case("/tmp", "/host\\040tmp", false; "escaped alias")] +#[test_case("/other", "/tmp/codex-daemon-1000/nested", false; "nested mount")] +fn rejects_only_mounts_that_compromise_the_directory(root: &str, destination: &str, allowed: bool) { + let mounts = + format!("1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 {root} {destination} rw - ext4 disk rw\n"); + assert_eq!( + check_mounts( + Path::new("/tmp/codex-daemon-1000"), + "0:1", + mounts.as_bytes() + ) + .is_ok(), + allowed + ); +} + +#[test] +fn rejects_alias_when_tmp_is_itself_a_bind_mount() { + let mounts = b"1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 /backing/tmp /tmp rw - ext4 disk rw\n"; + assert!(check_mounts(Path::new("/tmp/codex-daemon-1000"), "0:1", mounts).is_err()); + // A hidden deeper mount must not override the actual /tmp backing location. + let hidden = [ + mounts.as_slice(), + b"3 1 0:1 /tmp/codex-daemon-1000 /tmp/codex-daemon-1000 rw - ext4 disk rw\n", + ] + .concat(); + assert!(check_mounts(Path::new("/tmp/codex-daemon-1000"), "0:1", &hidden).is_err()); +} + +#[test] +fn accepts_private_tmp_filesystem_but_rejects_ambiguous_stacked_mounts() { + let mounts = "1 0 0:1 / / rw - ext4 disk rw\n2 1 0:2 / /tmp rw - tmpfs tmpfs rw\n"; + let directory = Path::new("/tmp/codex-daemon-1000"); + assert!(check_mounts(directory, "0:2", mounts.as_bytes()).is_ok()); + let stacked = format!("{mounts}3 2 0:2 /other /tmp rw - tmpfs tmpfs rw\n"); + assert!(check_mounts(directory, "0:2", stacked.as_bytes()).is_err()); +} + +#[test] +fn masked_wslg_alias_does_not_allow_other_exposed_aliases() { + let mounts = "1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 / /mnt/wslg/distro rw - ext4 disk rw\n"; + let directory = Path::new("/tmp/codex-daemon-1000"); + let mask = Some(Path::new(crate::bwrap::WSLG_DISTRO_ROOT)); + assert!(check_mounts(directory, "0:1", mounts.as_bytes()).is_err()); + assert!(super::check_mounts(directory, "0:1", mounts.as_bytes(), mask).is_ok()); + let exposed = format!("{mounts}3 1 0:1 /tmp /host-tmp rw - ext4 disk rw\n"); + assert!(super::check_mounts(directory, "0:1", exposed.as_bytes(), mask).is_err()); +} diff --git a/codex-rs/linux-sandbox/src/lib.rs b/codex-rs/linux-sandbox/src/lib.rs index 0cf64a8647..e4056e1589 100644 --- a/codex-rs/linux-sandbox/src/lib.rs +++ b/codex-rs/linux-sandbox/src/lib.rs @@ -10,6 +10,8 @@ mod bundled_bwrap; #[cfg(target_os = "linux")] mod bwrap; #[cfg(target_os = "linux")] +mod daemon_mounts; +#[cfg(target_os = "linux")] mod exec_util; #[cfg(target_os = "linux")] mod fd_mount; diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 83da2e69c2..1f22911aed 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -186,14 +186,7 @@ pub fn run_main() -> ! { file_system_sandbox_policy, network_sandbox_policy, } = resolve_permission_profile(permission_profile).unwrap_or_else(|err| panic!("{err}")); - ensure_legacy_landlock_mode_supports_policy( - use_legacy_landlock, - &file_system_sandbox_policy, - network_sandbox_policy, - allow_network_for_proxy, - &sandbox_policy_cwd, - Path::new(WSL_INTEROP_DIR), - ); + ensure_legacy_landlock_mode_supports_policy(use_legacy_landlock, &file_system_sandbox_policy); // Inner stage: apply seccomp/no_new_privs after bubblewrap has already // established the filesystem view. @@ -412,30 +405,9 @@ fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_legacy_la fn ensure_legacy_landlock_mode_supports_policy( use_legacy_landlock: bool, file_system_sandbox_policy: &FileSystemSandboxPolicy, - network_sandbox_policy: NetworkSandboxPolicy, - allow_network_for_proxy: bool, - sandbox_policy_cwd: &Path, - wsl_interop_dir: &Path, ) { - if use_legacy_landlock - && file_system_sandbox_policy - .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd) - { - panic!( - "permission profiles requiring direct runtime enforcement are incompatible with --use-legacy-landlock" - ); - } - if use_legacy_landlock - && network_sandbox_policy.is_enabled() - && !allow_network_for_proxy - && !file_system_sandbox_policy.has_full_disk_write_access() - // Interop can use another binfmt handler or a newly created socket. - // An active endpoint probe would race with sandboxed command startup. - && wsl_interop_dir.is_dir() - { - panic!( - "legacy Landlock cannot isolate WSL Windows interop with network access enabled; use bubblewrap or restrict network access" - ); + if use_legacy_landlock && !file_system_sandbox_policy.has_full_disk_write_access() { + panic!("filesystem-restricted execution requires bubblewrap to isolate app-server sockets"); } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index 0030dc7fd6..0f9291d5ce 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -72,6 +72,10 @@ fn inserts_bwrap_argv0_before_command_separator() { /*supports_argv0*/ true, "/tmp/codex-arg0-session/codex-linux-sandbox".to_string(), ); + let daemon_directory = codex_uds::shared_daemon_socket_directory() + .unwrap() + .display() + .to_string(); assert_eq!( argv, vec![ @@ -83,6 +87,12 @@ fn inserts_bwrap_argv0_before_command_separator() { "/".to_string(), "--dev".to_string(), "/dev".to_string(), + "--perms".to_string(), + "000".to_string(), + "--tmpfs".to_string(), + daemon_directory.clone(), + "--remount-ro".to_string(), + daemon_directory, "--unshare-user".to_string(), "--unshare-pid".to_string(), "--unshare-ipc".to_string(), @@ -741,66 +751,11 @@ fn apply_seccomp_then_exec_with_legacy_landlock_panics() { } #[test] -fn legacy_landlock_rejects_split_only_filesystem_policies() { - let temp_dir = tempfile::TempDir::new().expect("tempdir"); - let docs = temp_dir.path().join("docs"); - std::fs::create_dir_all(&docs).expect("create docs"); - let docs = AbsolutePathBuf::from_absolute_path(&docs).expect("absolute docs"); - let policy = FileSystemSandboxPolicy::restricted(vec![ - codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Special { - value: codex_protocol::permissions::FileSystemSpecialPath::Root, - }, - access: codex_protocol::permissions::FileSystemAccessMode::Read, - missing_path_behavior: None, - }, - codex_protocol::permissions::FileSystemSandboxEntry { - path: docs.into(), - access: codex_protocol::permissions::FileSystemAccessMode::Write, - missing_path_behavior: None, - }, - ]); - - let result = std::panic::catch_unwind(|| { - ensure_legacy_landlock_mode_supports_policy( - /*use_legacy_landlock*/ true, - &policy, - NetworkSandboxPolicy::Restricted, - /*allow_network_for_proxy*/ false, - temp_dir.path(), - &temp_dir.path().join("WSL"), - ); - }); - - assert!(result.is_err()); -} - -#[test] -fn legacy_landlock_rejects_full_network_when_wsl_interop_is_available() { - let temp_dir = tempfile::TempDir::new().expect("tempdir"); - let wsl_interop_dir = temp_dir.path().join("WSL"); - std::fs::create_dir(&wsl_interop_dir).expect("create interop directory"); - let policy = read_only_file_system_policy(); - - let result = std::panic::catch_unwind(|| { - ensure_legacy_landlock_mode_supports_policy( - /*use_legacy_landlock*/ true, - &policy, - NetworkSandboxPolicy::Enabled, - /*allow_network_for_proxy*/ false, - temp_dir.path(), - &wsl_interop_dir, - ); - }); - assert!(result.is_err()); - +#[should_panic(expected = "filesystem-restricted execution requires bubblewrap")] +fn legacy_landlock_cannot_bypass_daemon_socket_isolation() { ensure_legacy_landlock_mode_supports_policy( /*use_legacy_landlock*/ true, - &policy, - NetworkSandboxPolicy::Enabled, - /*allow_network_for_proxy*/ true, - temp_dir.path(), - &wsl_interop_dir, + &read_only_file_system_policy(), ); } diff --git a/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs b/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs new file mode 100644 index 0000000000..4eba019b51 --- /dev/null +++ b/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs @@ -0,0 +1,167 @@ +use super::*; +use pretty_assertions::assert_eq; +use std::os::fd::AsRawFd; +use std::os::unix::net::UnixListener; +use std::os::unix::net::UnixStream; + +#[tokio::test] +async fn daemon_socket_bind_mount_alias_rejects_sandbox_startup() { + if should_skip_bwrap_tests().await { + return; + } + let root = codex_uds::prepare_shared_daemon_socket_directory().unwrap(); + let private = tempfile::tempdir_in(&root).unwrap(); + let alias = tempfile::tempdir().unwrap(); + let _listener = UnixListener::bind(private.path().join("rpc.sock")).unwrap(); + let profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::read_only(), + NetworkSandboxPolicy::Enabled, + ); + let script = r#" +import pathlib, socket, subprocess, sys +root, alias, endpoint, helper, profile = sys.argv[1:] +mount = subprocess.run(['mount', '--bind', root, alias], capture_output=True) +if mount.returncode: + sys.exit(77) +try: + socket.socket(socket.AF_UNIX).connect(str(pathlib.Path(alias) / endpoint)) + result = subprocess.run([helper, '--sandbox-policy-cwd', '/', '--permission-profile', profile, + '--', 'sh', '-c', 'echo command-started'], capture_output=True) + assert result.returncode != 0, result + assert b'unsupported host mount' in result.stderr, result + assert b'command-started' not in result.stdout, result +finally: + subprocess.run(['umount', alias], check=True) +"#; + let output = tokio::process::Command::new("unshare") + .args([ + "--user", + "--map-current-user", + "--keep-caps", + "--mount", + "--propagation", + "private", + "--", + ]) + .args(["python3", "-c", script]) + .arg(&root) + .arg(alias.path()) + .arg(private.path().strip_prefix(&root).unwrap().join("rpc.sock")) + .arg(codex_linux_sandbox_exe()) + .arg(serde_json::to_string(&profile).unwrap()) + .kill_on_drop(true) + .output() + .await; + let Ok(output) = output else { + eprintln!("skipping bind-mount test: unshare is unavailable"); + return; + }; + if output.status.code() == Some(77) + || String::from_utf8_lossy(&output.stderr).starts_with("unshare:") + { + eprintln!("skipping bind-mount test: user/mount namespaces are unavailable"); + return; + } + assert_eq!(output.status.code(), Some(0), "{output:?}"); +} + +#[tokio::test] +async fn daemon_sockets_are_hidden_with_network_and_tmp_write_grants() { + if should_skip_bwrap_tests().await { + return; + } + let root = codex_uds::prepare_shared_daemon_socket_directory().unwrap(); + let private = tempfile::tempdir_in(&root).unwrap(); + let workspace = tempfile::tempdir().unwrap(); + let socket = private.path().join("rpc.sock"); + let alias = workspace.path().join("alias.sock"); + let unrelated = workspace.path().join("other.sock"); + let directory_fd = std::fs::File::open(private.path()).unwrap(); + let _daemon = UnixListener::bind(&socket).unwrap(); + let _other = UnixListener::bind(&unrelated).unwrap(); + std::os::unix::fs::symlink(&socket, &alias).unwrap(); + UnixStream::connect(&alias).expect("host can reach daemon"); + let script = r#" +import os, socket, sys +for path in [sys.argv[1], sys.argv[2], '/proc/' + sys.argv[5] + '/root' + sys.argv[1]]: + try: + socket.socket(socket.AF_UNIX).connect(path) + except OSError: + pass + else: + raise AssertionError('daemon reachable: ' + path) +try: + os.link(sys.argv[1], sys.argv[4] + '/hardlink') +except OSError: + pass +else: + raise AssertionError('daemon hardlink created') +socket.socket(socket.AF_UNIX).connect(sys.argv[3]) +s = socket.socket(socket.AF_UNIX) +s.bind(sys.argv[4] + '/local.sock') +s.listen(1) +c = socket.socket(socket.AF_UNIX) +c.connect(sys.argv[4] + '/local.sock') +p, _ = s.accept() +c.sendall(b'ok') +assert p.recv(2) == b'ok' +os.unlink(sys.argv[4] + '/local.sock') +# Main already hides procfs when isolating WSL interop. +if sys.argv[6] == 'host-proc' and not os.path.isdir('/run/WSL'): + assert os.path.isfile('/proc/self/status') + assert os.path.isdir('/proc/' + sys.argv[5]) + for name in ['root', 'cwd', 'fd/' + sys.argv[7]]: + try: + os.readlink('/proc/' + sys.argv[5] + '/' + name) + except PermissionError: + pass + else: + raise AssertionError('host process link accessible: ' + name) +"#; + let profile = PermissionProfile::workspace_write_with( + &[AbsolutePathBuf::from_absolute_path("/tmp").unwrap()], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ false, + ); + let mut env = create_env_from_core_vars(); + env.insert( + "CODEX_HOME".to_string(), + workspace.path().display().to_string(), + ); + env.insert("TMPDIR".to_string(), workspace.path().display().to_string()); + // Inherited procfs remains useful in containers that prohibit a fresh mount. + // Its host-process magic links must still respect the user namespace boundary. + for proc_mode in ["fresh-proc", "host-proc"] { + let mut command = tokio::process::Command::new(codex_linux_sandbox_exe()); + command + .arg("--sandbox-policy-cwd") + .arg(workspace.path()) + .arg("--permission-profile") + .arg(serde_json::to_string(&profile).unwrap()) + .envs(&env) + .current_dir(workspace.path()); + if proc_mode == "host-proc" { + command.arg("--no-proc"); + } + let output = command + .args([ + "--", + "python3", + "-c", + script, + socket.to_str().unwrap(), + alias.to_str().unwrap(), + unrelated.to_str().unwrap(), + workspace.path().to_str().unwrap(), + &std::process::id().to_string(), + proc_mode, + &directory_fd.as_raw_fd().to_string(), + ]) + .kill_on_drop(true) + .output() + .await + .unwrap(); + assert_eq!(output.status.code(), Some(0), "{output:?}"); + } +} diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index a6915ba2da..9052a057c9 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -561,59 +561,6 @@ async fn wsl_interop_cannot_reenter_distro_as_root_with_network_access() { assert_ne!(output.stdout.text.trim(), "0"); } -#[tokio::test] -async fn legacy_landlock_rejects_wsl_interop_with_network_access() { - let Some(powershell) = wsl_windows_executable(r"WindowsPowerShell\v1.0\powershell.exe").await - else { - return; - }; - if wsl_baseline_output( - &powershell, - &["-NoProfile", "-NonInteractive", "-Command", "exit 0"], - ) - .await - .is_none() - { - eprintln!("skipping legacy WSL interop test: Windows interop is unavailable on the host"); - return; - } - - let safe_profile = PermissionProfile::from_runtime_permissions( - &FileSystemSandboxPolicy::read_only(), - NetworkSandboxPolicy::Restricted, - ); - let linux_output = run_cmd_result_with_permission_profile( - &["/bin/true"], - safe_profile, - NETWORK_TIMEOUT_MS, - /*use_legacy_landlock*/ true, - ) - .await - .expect("legacy Landlock should run a Linux command"); - assert_eq!(linux_output.exit_code, 0); - - let unsafe_profile = PermissionProfile::from_runtime_permissions( - &FileSystemSandboxPolicy::read_only(), - NetworkSandboxPolicy::Enabled, - ); - let output = expect_denied( - run_cmd_result_with_permission_profile( - &["/bin/true"], - unsafe_profile, - NETWORK_TIMEOUT_MS, - /*use_legacy_landlock*/ true, - ) - .await, - "legacy Landlock must reject restricted filesystem access with full network access on WSL", - ); - assert!( - output - .stderr - .text - .contains("legacy Landlock cannot isolate WSL Windows interop") - ); -} - #[tokio::test] async fn test_root_read() { run_cmd(&["ls", "-l", "/bin"], &[], SHORT_TIMEOUT_MS).await; @@ -767,20 +714,19 @@ async fn sandbox_ignores_missing_writable_roots_under_bwrap() { #[tokio::test] async fn test_no_new_privs_is_enabled() { let output = run_cmd_output( - &["bash", "-lc", "grep '^NoNewPrivs:' /proc/self/status"], + &[ + "python3", + "-c", + "import ctypes; print(ctypes.CDLL(None).prctl(39, 0, 0, 0, 0))", + ], &[], // We have seen timeouts when running this test in CI on GitHub, // so we are using a generous timeout until we can diagnose further. LONG_TIMEOUT_MS, ) .await; - let line = output - .stdout - .text - .lines() - .find(|line| line.starts_with("NoNewPrivs:")) - .unwrap_or(""); - assert_eq!(line.trim(), "NoNewPrivs:\t1"); + assert_eq!(output.exit_code, 0); + assert_eq!(output.stdout.text, "1\n"); } #[tokio::test] @@ -792,19 +738,16 @@ async fn sandboxed_command_has_no_effective_or_permitted_capabilities() { let output = run_cmd_output( &[ - "bash", - "-lc", - "awk '$1 == \"CapPrm:\" || $1 == \"CapEff:\" { print $1, $2 }' /proc/self/status", + "python3", + "-c", + "import ctypes; h=(ctypes.c_uint*2)(0x20080522,0); d=(ctypes.c_uint*6)(); assert ctypes.CDLL(None).capget(h,d)==0; print(d[0],d[1],d[3],d[4])", ], &[], LONG_TIMEOUT_MS, ) .await; - - assert_eq!( - output.stdout.text, - "CapPrm: 0000000000000000\nCapEff: 0000000000000000\n" - ); + assert_eq!(output.exit_code, 0); + assert_eq!(output.stdout.text, "0 0 0 0\n"); } #[tokio::test] @@ -1622,3 +1565,6 @@ async fn sandbox_blocks_dev_tcp_redirection() { // all images ship bash, so we guard against 127 as well. assert_network_blocked(&["bash", "-c", "echo hi > /dev/tcp/127.0.0.1/80"]).await; } + +#[path = "daemon_sockets_tests.rs"] +mod daemon_sockets_tests; diff --git a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs index e0b321a219..6aca07aef9 100644 --- a/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs +++ b/codex-rs/linux-sandbox/tests/suite/managed_proxy.rs @@ -246,7 +246,7 @@ async fn assert_seccomp_filtered_namespace_reaper( &[ "bash", "-c", - "if [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; printf '%s\\n' \"$PPID\"; awk '$1 == \"Seccomp:\" && FNR == NR { print $2 } $1 == \"Seccomp_filters:\" { print $2 }' /proc/1/status /proc/self/status", + "if [ ! -r /proc/self/status ] || [ ! -r /proc/1/status ] || [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; printf '%s\\n' \"$PPID\"; awk '$1 == \"Seccomp:\" && FNR == NR { print $2 } $1 == \"Seccomp_filters:\" { print $2 }' /proc/1/status /proc/self/status", ], permission_profile, allow_network_for_proxy, @@ -286,7 +286,7 @@ async fn namespace_reaper_collects_orphaned_descendants() { &[ "bash", "-c", - "if [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; orphan=$(bash -c 'sleep 0.05 /dev/null 2>&1 & printf \"%s\\n\" \"$!\"'); for _ in $(seq 1 100); do if [ ! -e \"/proc/$orphan\" ]; then printf 'orphan reaped\\n'; exit 0; fi; sleep 0.01; done; exit 1", + "if [ ! -r /proc/self/status ] || [ ! -r /proc/1/status ] || [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; orphan=$(bash -c 'sleep 0.05 /dev/null 2>&1 & printf \"%s\\n\" \"$!\"'); for _ in $(seq 1 100); do if [ ! -e \"/proc/$orphan\" ]; then printf 'orphan reaped\\n'; exit 0; fi; sleep 0.01; done; exit 1", ], &PermissionProfile::read_only(), /*allow_network_for_proxy*/ false, @@ -717,18 +717,25 @@ fn handoff_client() { return; }; assert!(std::env::var_os(PROXY_ATTRIBUTION_TOKEN_ENV_KEY).is_none()); - let inherited_sockets = std::fs::read_dir("/proc/self/fd") - .expect("enumerate inherited descriptors") - .filter_map(Result::ok) - .filter_map(|entry| { - let target = std::fs::read_link(entry.path()).ok()?; - target - .to_string_lossy() - .starts_with("socket:") - .then(|| (entry.file_name(), target)) - }) - .collect::>(); - assert_eq!(inherited_sockets, Vec::new(), "privileged socket leaked"); + match std::fs::read_dir("/proc/self/fd") { + Ok(entries) => { + let inherited_sockets = entries + .filter_map(Result::ok) + .filter_map(|entry| { + let target = std::fs::read_link(entry.path()).ok()?; + target + .to_string_lossy() + .starts_with("socket:") + .then(|| (entry.file_name(), target)) + }) + .collect::>(); + assert_eq!(inherited_sockets, Vec::new(), "privileged socket leaked"); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + eprintln!("skipping descriptor enumeration: sandbox proc is unavailable"); + } + Err(error) => panic!("enumerate inherited descriptors: {error}"), + } let shared = std::env::var("CODEX_TEST_HANDOFF_SHARED").expect("shared directory"); let shared = Path::new(&shared); diff --git a/codex-rs/sandboxing/Cargo.toml b/codex-rs/sandboxing/Cargo.toml index 3950fa44d7..14549e19af 100644 --- a/codex-rs/sandboxing/Cargo.toml +++ b/codex-rs/sandboxing/Cargo.toml @@ -17,6 +17,7 @@ anyhow = { workspace = true } codex-mxc-sandbox = { workspace = true } codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } +codex-uds = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-path-uri = { workspace = true } codex-utils-pty = { workspace = true } diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index 765c89d7dd..dc9583873e 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -1040,6 +1040,18 @@ pub(crate) fn create_seatbelt_command_args_with_profile( file_write_policy, network_policy, ]; + // Network grants and Unix-socket allowlists must never reopen the + // privileged app-server RPC transport to filesystem-restricted commands. + if !file_system_sandbox_policy.has_full_disk_write_access() { + let directory = codex_uds::shared_daemon_socket_directory() + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + let directory = serde_json::to_string(&directory.to_string_lossy()) + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + policy_sections.push(format!( + "(deny file-read* file-write* (subpath {directory}))\n\ + (deny network-outbound (remote unix-socket (subpath {directory})))" + )); + } if file_system_sandbox_policy.has_full_disk_read_access() { policy_sections.push(MACOS_SEATBELT_PREFERENCES_POLICY.to_string()); } @@ -1086,3 +1098,7 @@ pub(crate) fn create_seatbelt_command_args_with_profile( #[cfg(test)] #[path = "seatbelt_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "seatbelt_daemon_socket_tests.rs"] +mod daemon_socket_tests; diff --git a/codex-rs/sandboxing/src/seatbelt_daemon_socket_tests.rs b/codex-rs/sandboxing/src/seatbelt_daemon_socket_tests.rs new file mode 100644 index 0000000000..5c316edc36 --- /dev/null +++ b/codex-rs/sandboxing/src/seatbelt_daemon_socket_tests.rs @@ -0,0 +1,86 @@ +use super::CreateSeatbeltCommandArgsParams; +use super::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +use super::create_seatbelt_command_args; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use std::os::unix::net::UnixListener; +use std::os::unix::net::UnixStream; +use std::process::Command; + +#[test] +fn daemon_sockets_are_denied_despite_network_and_tmp_write_grants() { + let root = codex_uds::prepare_shared_daemon_socket_directory().unwrap(); + let private = tempfile::tempdir_in(&root).unwrap(); + let workspace = tempfile::tempdir().unwrap(); + let socket = private.path().join("rpc.sock"); + let alias = workspace.path().join("alias.sock"); + let unrelated = workspace.path().join("other.sock"); + let _daemon = UnixListener::bind(&socket).unwrap(); + let _other = UnixListener::bind(&unrelated).unwrap(); + std::os::unix::fs::symlink(&socket, &alias).unwrap(); + UnixStream::connect(&alias).expect("host can reach daemon"); + let script = r#" +import os, socket, sys +for path in sys.argv[1:3]: + try: + socket.socket(socket.AF_UNIX).connect(path) + except OSError: + pass + else: + raise AssertionError('daemon reachable: ' + path) +try: + os.link(sys.argv[1], sys.argv[4] + '/hardlink') +except OSError: + pass +else: + raise AssertionError('daemon hardlink created') +socket.socket(socket.AF_UNIX).connect(sys.argv[3]) +s = socket.socket(socket.AF_UNIX) +s.bind(sys.argv[4] + '/local.sock') +s.listen(1) +c = socket.socket(socket.AF_UNIX) +c.connect(sys.argv[4] + '/local.sock') +p, _ = s.accept() +c.sendall(b'ok') +assert p.recv(2) == b'ok' +"#; + let command = vec![ + "/usr/bin/python3".to_string(), + "-c".to_string(), + script.to_string(), + socket.display().to_string(), + alias.display().to_string(), + unrelated.display().to_string(), + workspace.path().display().to_string(), + ]; + let policy = FileSystemSandboxPolicy::workspace_write( + &[AbsolutePathBuf::from_absolute_path("/tmp").unwrap()], + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command, + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Enabled, + sandbox_policy_cwd: workspace.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[AbsolutePathBuf::from_absolute_path(root).unwrap()], + }) + .unwrap(); + let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(args) + .env("CODEX_HOME", workspace.path()) + .output() + .unwrap(); + assert_eq!( + output.status.code(), + Some(0), + "{}", + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/codex-rs/uds/Cargo.toml b/codex-rs/uds/Cargo.toml index 9748c630b8..a1f199ed46 100644 --- a/codex-rs/uds/Cargo.toml +++ b/codex-rs/uds/Cargo.toml @@ -15,6 +15,9 @@ workspace = true [dependencies] tokio = { workspace = true, features = ["fs", "net", "rt"] } +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } + [target.'cfg(windows)'.dependencies] async-io = { workspace = true } tokio-util = { workspace = true, features = ["compat"] } diff --git a/codex-rs/uds/src/daemon_directory.rs b/codex-rs/uds/src/daemon_directory.rs new file mode 100644 index 0000000000..90c3c103bc --- /dev/null +++ b/codex-rs/uds/src/daemon_directory.rs @@ -0,0 +1,39 @@ +//! The host-local rendezvous root for privileged app-server RPC sockets. +//! +//! Every listener uses this root, which sandboxes hide even before a daemon +//! starts. It must not depend on HOME, TMPDIR, CODEX_HOME, or command settings. + +use std::fs; +use std::io; +use std::os::unix::fs::DirBuilderExt; +use std::os::unix::fs::MetadataExt; +use std::path::PathBuf; + +/// Returns the fixed executor-local directory that every sandbox must hide. +pub fn shared_daemon_socket_directory() -> io::Result { + // Resolve the system alias /tmp -> /private/tmp on macOS. + let temporary_root = fs::canonicalize("/tmp")?; + let uid = unsafe { libc::geteuid() }; + Ok(temporary_root.join(format!("codex-daemon-{uid}"))) +} + +/// Creates the reserved directory, rejecting symlinks and unsafe existing owners or modes. +pub fn prepare_shared_daemon_socket_directory() -> io::Result { + let directory = shared_daemon_socket_directory()?; + match fs::DirBuilder::new().mode(0o700).create(&directory) { + Ok(()) => {} + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + let metadata = fs::symlink_metadata(&directory)?; + if !metadata.is_dir() + || metadata.uid() != unsafe { libc::geteuid() } + || metadata.mode() & 0o777 != 0o700 + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "app-server socket directory must be a user-owned directory with mode 0700", + )); + } + Ok(directory) +} diff --git a/codex-rs/uds/src/lib.rs b/codex-rs/uds/src/lib.rs index a237d1060b..cecd09ddef 100644 --- a/codex-rs/uds/src/lib.rs +++ b/codex-rs/uds/src/lib.rs @@ -1,5 +1,12 @@ //! Cross-platform async Unix domain socket helpers. +#[cfg(unix)] +mod daemon_directory; +#[cfg(unix)] +pub use daemon_directory::prepare_shared_daemon_socket_directory; +#[cfg(unix)] +pub use daemon_directory::shared_daemon_socket_directory; + use std::io::Result as IoResult; use std::path::Path; use std::pin::Pin;