diff --git a/crates/runner/src/descriptors.rs b/crates/runner/src/descriptors.rs new file mode 100644 index 0000000..33d247c --- /dev/null +++ b/crates/runner/src/descriptors.rs @@ -0,0 +1,473 @@ +//! Child-side exclusion of unrelated descriptors (#79). +//! +//! A spawn passes on every descriptor that is not close-on-exec when it forks. macOS cannot +//! create a pipe, a socket pair or a pseudo-terminal close-on-exec atomically, so a child +//! spawned while another thread is between creating a descriptor and marking it inherits that +//! descriptor: another execution's pipe ends, or its PTY master and slave. The holder can then +//! delay that execution's completion, keep its terminal allocated, or read and write it. +//! +//! [`exclude_unrelated`] installs a `pre_exec` step that marks every descriptor above 2 +//! close-on-exec in the child, before exec: +//! - Linux: `close_range(3, ~0, CLOSE_RANGE_CLOEXEC)`. Where the kernel refuses it (before 5.11, +//! or under a seccomp policy), the child reads its descriptor-table size (`FDSize` in +//! `/proc/self/status`) and marks every descriptor below it. +//! - macOS: the child asks the kernel for its descriptor-table size (`proc_pidinfo` with +//! `PROC_PIDLISTFDS` and no buffer returns the table's slot count plus 20 entries) and marks +//! every descriptor below it. +//! +//! Every open descriptor indexes that table, so the walk does not depend on `RLIMIT_NOFILE`, +//! which may have been lowered below a descriptor that is still open. The standard descriptors +//! are kept, and descriptors already close-on-exec, such as the standard library's exec-error +//! pipe, are left alone, so a failed exec is still reported as a spawn error. If the step cannot +//! establish this (the table size is unavailable, or an open descriptor cannot be inspected or +//! marked) it returns the error and the spawn fails, instead of starting a child that may hold +//! an unrelated descriptor. Other platforms always fail this way. +//! +//! The step runs after fork and before exec. It does not allocate, take locks or panic. It +//! calls only `fcntl`, plus `getpid` and `proc_pidinfo` on macOS or `syscall` (`close_range`, +//! `openat`, `read`, `close`) on Linux, and it writes only its own stack and `errno`. +//! +//! Installing a `pre_exec` step makes std, and Tokio through it, create these children with +//! fork and exec instead of its `posix_spawn` fast path. Process ownership, the std and Tokio +//! child handles, reaping, output pumps, timeouts, the PTY path and lifecycle are unchanged. + +use std::io; + +/// Mark every descriptor above 2 close-on-exec in the child, before exec. +pub fn exclude_unrelated(command: &mut tokio::process::Command) -> &mut tokio::process::Command { + // SAFETY: `mark_unrelated` runs in the forked child and is restricted as the module + // documentation describes. + unsafe { command.pre_exec(|| mark_unrelated(Forced::NONE)) } +} + +/// [`exclude_unrelated`] for a standard-library command. +pub(crate) fn exclude_unrelated_std( + command: &mut std::process::Command, +) -> &mut std::process::Command { + use std::os::unix::process::CommandExt; + // SAFETY: as for `exclude_unrelated`. + unsafe { command.pre_exec(|| mark_unrelated(Forced::NONE)) } +} + +/// Failures the tests force. Production code passes `Forced::NONE`. +#[derive(Clone, Copy)] +struct Forced { + /// Skip `close_range` (Linux), so the table walk runs. + close_range: bool, + /// Report the descriptor-table size as unavailable. + table: bool, +} + +impl Forced { + const NONE: Self = Self { + close_range: false, + table: false, + }; +} + +/// Runs in the child: mark every descriptor above 2 close-on-exec, or fail. +fn mark_unrelated(forced: Forced) -> io::Result<()> { + #[cfg(target_os = "linux")] + { + if !forced.close_range && close_range_cloexec() { + return Ok(()); + } + } + #[cfg(not(target_os = "linux"))] + let _ = forced.close_range; + let end = if forced.table { + Err(io::Error::from_raw_os_error(libc::ENOTSUP)) + } else { + descriptor_table_size() + }?; + for fd in 3..end { + mark_close_on_exec(fd)?; + } + Ok(()) +} + +/// What `F_GETFD` reported for a descriptor number. +#[derive(Debug, PartialEq, Eq)] +enum Probe { + NotOpen, + CloseOnExec, + Inheritable(libc::c_int), + Failed(i32), +} + +/// Only `EBADF` means "not open"; any other failure is an error. +fn classify(flags: libc::c_int, errno: i32) -> Probe { + if flags < 0 { + if errno == libc::EBADF { + Probe::NotOpen + } else { + Probe::Failed(errno) + } + } else if flags & libc::FD_CLOEXEC != 0 { + Probe::CloseOnExec + } else { + Probe::Inheritable(flags) + } +} + +/// The current `errno`, read without allocating. +fn errno() -> i32 { + io::Error::last_os_error().raw_os_error().unwrap_or(0) +} + +/// Mark `fd` close-on-exec unless it is not open or already marked. +fn mark_close_on_exec(fd: libc::c_int) -> io::Result<()> { + // SAFETY: fcntl reads and sets only this process's descriptor flags. + let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) }; + let probe = classify(flags, if flags < 0 { errno() } else { 0 }); + match probe { + Probe::NotOpen | Probe::CloseOnExec => Ok(()), + Probe::Failed(code) => Err(io::Error::from_raw_os_error(code)), + Probe::Inheritable(flags) => { + // SAFETY: as above. + if unsafe { libc::fcntl(fd, libc::F_SETFD, flags | libc::FD_CLOEXEC) } < 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } + } +} + +/// An error for a call that failed, or `fallback` if `errno` was not set. +#[cfg(any(target_os = "macos", target_os = "linux"))] +fn os_error_or(fallback: i32) -> io::Error { + match errno() { + 0 => io::Error::from_raw_os_error(fallback), + code => io::Error::from_raw_os_error(code), + } +} + +/// Linux: one call marks every descriptor from 3 up. +#[cfg(target_os = "linux")] +fn close_range_cloexec() -> bool { + let first: libc::c_uint = 3; + // SAFETY: close_range with CLOSE_RANGE_CLOEXEC only sets this process's descriptor flags. + unsafe { + libc::syscall( + libc::SYS_close_range, + first, + libc::c_uint::MAX, + libc::CLOSE_RANGE_CLOEXEC, + ) == 0 + } +} + +/// macOS: the descriptor-table size. Without a buffer, `proc_pidinfo(PROC_PIDLISTFDS)` returns +/// `(fd_nfiles + 20) * sizeof(struct proc_fdinfo)`, and every open descriptor is below +/// `fd_nfiles`, the table's slot count. +#[cfg(target_os = "macos")] +fn descriptor_table_size() -> io::Result { + // SAFETY: getpid and a size-only proc_pidinfo read this process's state into no buffer. + let bytes = unsafe { + libc::proc_pidinfo( + libc::getpid(), + libc::PROC_PIDLISTFDS, + 0, + std::ptr::null_mut(), + 0, + ) + }; + let entry = std::mem::size_of::() as libc::c_int; + if bytes <= 0 { + return Err(os_error_or(libc::EIO)); + } + Ok(bytes / entry) +} + +/// Linux: the descriptor-table size, `FDSize` in `/proc/self/status`. Every open descriptor is +/// below it. +#[cfg(target_os = "linux")] +fn descriptor_table_size() -> io::Result { + let mut status = [0u8; 4096]; + // SAFETY: openat of a constant path; the descriptor is closed below. + let file = unsafe { + libc::syscall( + libc::SYS_openat, + libc::c_long::from(libc::AT_FDCWD), + c"/proc/self/status".as_ptr(), + libc::c_long::from(libc::O_RDONLY | libc::O_CLOEXEC), + ) + }; + if file < 0 { + return Err(os_error_or(libc::ENOENT)); + } + let mut filled = 0; + let read = loop { + let rest = status.get_mut(filled..).unwrap_or_default(); + if rest.is_empty() { + break Ok(()); + } + // SAFETY: read writes at most `rest.len()` bytes into the stack buffer. + let count = unsafe { libc::syscall(libc::SYS_read, file, rest.as_mut_ptr(), rest.len()) }; + if count == 0 { + break Ok(()); + } + if count < 0 { + if errno() == libc::EINTR { + continue; + } + break Err(os_error_or(libc::EIO)); + } + filled += count as usize; + }; + // SAFETY: closes the descriptor opened above. + unsafe { libc::syscall(libc::SYS_close, file) }; + read?; + parse_fd_size(status.get(..filled).unwrap_or_default()) + .ok_or_else(|| io::Error::from_raw_os_error(libc::ENODATA)) +} + +/// The `FDSize` value of a `/proc//status` text. +#[cfg(any(target_os = "linux", test))] +fn parse_fd_size(status: &[u8]) -> Option { + const KEY: &[u8] = b"\nFDSize:"; + let start = status.windows(KEY.len()).position(|window| window == KEY)? + KEY.len(); + let mut value: libc::c_int = 0; + let mut digits = 0; + for &byte in status.get(start..)? { + match byte { + b' ' | b'\t' if digits == 0 => {} + b'0'..=b'9' => { + value = value + .checked_mul(10)? + .checked_add(libc::c_int::from(byte - b'0'))?; + digits += 1; + } + _ => break, + } + } + (digits > 0).then_some(value) +} + +#[cfg(not(any(target_os = "macos", target_os = "linux")))] +fn descriptor_table_size() -> io::Result { + Err(io::Error::from_raw_os_error(libc::ENOTSUP)) +} + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; + use std::os::unix::process::CommandExt; + use std::process::{Command, Stdio}; + + /// A descriptor that is not close-on-exec at or above `at`: what another thread's creation + /// window, or a careless library, leaves behind. + fn inheritable_at(at: libc::c_int) -> OwnedFd { + // SAFETY: open and fcntl create descriptors that this test owns. + let null = unsafe { libc::open(c"/dev/null".as_ptr(), libc::O_RDONLY) }; + assert!(null >= 0, "open /dev/null"); + // SAFETY: open returned a new descriptor. + let null = unsafe { OwnedFd::from_raw_fd(null) }; + // SAFETY: F_DUPFD returns a new descriptor without close-on-exec. + let high = unsafe { libc::fcntl(null.as_raw_fd(), libc::F_DUPFD, at) }; + assert!(high >= at, "F_DUPFD {at}: {}", io::Error::last_os_error()); + // SAFETY: as above. + unsafe { OwnedFd::from_raw_fd(high) } + } + + pub(crate) fn inheritable_descriptor() -> OwnedFd { + inheritable_at(200) + } + + /// A shell command that reports whether it holds its stdout (a control) and then `fd`. + pub(crate) fn report_script(fd: libc::c_int) -> String { + format!( + "for n in 1 {fd}; do if [ -e /dev/fd/$n ]; then echo held; else echo clear; fi; done" + ) + } + + fn child_report(fd: libc::c_int, prepare: impl FnOnce(&mut Command)) -> String { + let mut command = Command::new("/bin/sh"); + command + .arg("-c") + .arg(report_script(fd)) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + prepare(&mut command); + let output = command.output().expect("spawn /bin/sh"); + String::from_utf8(output.stdout).expect("utf-8 report") + } + + fn forced(command: &mut Command, forced: Forced) { + // SAFETY: as for `exclude_unrelated`. + unsafe { + command.pre_exec(move || mark_unrelated(forced)); + } + } + + const WALK: Forced = Forced { + close_range: true, + table: false, + }; + + #[test] + fn unguarded_child_inherits_the_descriptor() { + let held = inheritable_descriptor(); + assert_eq!(child_report(held.as_raw_fd(), |_| {}), "held\nheld\n"); + } + + #[test] + fn guarded_child_keeps_only_its_standard_descriptors() { + let held = inheritable_descriptor(); + let report = child_report(held.as_raw_fd(), |command| { + exclude_unrelated_std(command); + }); + assert_eq!(report, "held\nclear\n"); + } + + #[test] + fn table_walk_excludes_the_descriptor() { + let held = inheritable_descriptor(); + let report = child_report(held.as_raw_fd(), |command| forced(command, WALK)); + assert_eq!(report, "held\nclear\n"); + } + + #[test] + fn unavailable_table_size_fails_the_spawn() { + let held = inheritable_descriptor(); + let dir = tempfile::tempdir().unwrap(); + let ran = dir.path().join("ran"); + let mut command = Command::new("/bin/sh"); + command + .arg("-c") + .arg(format!(": > '{}'", ran.display())) + .stdin(Stdio::null()); + forced( + &mut command, + Forced { + close_range: true, + table: true, + }, + ); + let error = command.spawn().expect_err("the spawn must fail"); + assert_eq!(error.raw_os_error(), Some(libc::ENOTSUP)); + assert!(!ran.exists(), "the child must not run"); + drop(held); + } + + #[test] + fn guarded_spawn_still_reports_a_failed_exec() { + let mut command = Command::new("/no/such/codespace-exec"); + exclude_unrelated_std(&mut command); + let error = command.spawn().expect_err("exec must fail"); + assert_eq!(error.kind(), io::ErrorKind::NotFound); + } + + #[test] + fn only_ebadf_means_not_open() { + assert_eq!(classify(-1, libc::EBADF), Probe::NotOpen); + assert_eq!(classify(-1, libc::EINVAL), Probe::Failed(libc::EINVAL)); + assert_eq!(classify(-1, libc::EIO), Probe::Failed(libc::EIO)); + assert_eq!(classify(libc::FD_CLOEXEC, 0), Probe::CloseOnExec); + assert_eq!(classify(0, 0), Probe::Inheritable(0)); + } + + #[test] + fn marking_sets_close_on_exec_and_skips_numbers_not_open() { + let held = inheritable_descriptor(); + let fd = held.as_raw_fd(); + // SAFETY: reads this test's own descriptor flags. + assert_eq!( + unsafe { libc::fcntl(fd, libc::F_GETFD) } & libc::FD_CLOEXEC, + 0 + ); + mark_close_on_exec(fd).unwrap(); + // SAFETY: as above. + assert_ne!( + unsafe { libc::fcntl(fd, libc::F_GETFD) } & libc::FD_CLOEXEC, + 0 + ); + // A number no test opens. + mark_close_on_exec(1 << 30).unwrap(); + } + + #[test] + fn table_size_covers_an_open_descriptor() { + let held = inheritable_descriptor(); + assert!(descriptor_table_size().unwrap() > held.as_raw_fd()); + } + + #[test] + fn fd_size_is_parsed_from_proc_status() { + assert_eq!( + parse_fd_size(b"Name:\tx\nPid:\t1\nFDSize:\t256\nGroups:\t\n"), + Some(256) + ); + assert_eq!(parse_fd_size(b"Name:\tx\nFDSize:\n"), None); + assert_eq!(parse_fd_size(b"Name:\tx\nFDSize:\t99999999999\n"), None); + assert_eq!(parse_fd_size(b"FDSize:\t64\n"), None); + } + + /// Set by the parent test so that its re-run copy performs the scenario. + const SCENARIO: &str = "CODESPACE_DESCRIPTOR_LIMIT_SCENARIO"; + + #[test] + fn descriptor_above_a_lowered_limit_is_excluded() { + if std::env::var_os(SCENARIO).is_some() { + lowered_limit_scenario(); + return; + } + // The scenario changes this process's descriptor limit, so it runs in a fresh copy of + // this test binary. + let output = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "descriptors::tests::descriptor_above_a_lowered_limit_is_excluded", + "--test-threads=1", + "--nocapture", + ]) + .env(SCENARIO, "1") + .output() + .unwrap(); + assert!( + output.status.success() && String::from_utf8_lossy(&output.stdout).contains("1 passed"), + "scenario failed:\n{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + + fn set_soft_limit(soft: libc::rlim_t) { + let mut limit = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + // SAFETY: getrlimit and setrlimit read and write one rlimit of this process. + unsafe { + assert_eq!(libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit), 0); + limit.rlim_cur = soft; + assert_eq!( + libc::setrlimit(libc::RLIMIT_NOFILE, &limit), + 0, + "setrlimit {soft}: {}", + io::Error::last_os_error() + ); + } + } + + /// A descriptor stays open above a soft `RLIMIT_NOFILE` lowered after it was opened. The + /// table walk must still exclude it. + fn lowered_limit_scenario() { + const HIGH: libc::c_int = 1500; + const LOWERED: libc::rlim_t = 1024; + set_soft_limit(2048); + let held = inheritable_at(HIGH); + set_soft_limit(LOWERED); + assert!(LOWERED <= held.as_raw_fd() as libc::rlim_t); + assert_eq!(child_report(held.as_raw_fd(), |_| {}), "held\nheld\n"); + let walked = child_report(held.as_raw_fd(), |command| forced(command, WALK)); + assert_eq!(walked, "held\nclear\n"); + let guarded = child_report(held.as_raw_fd(), |command| { + exclude_unrelated_std(command); + }); + assert_eq!(guarded, "held\nclear\n"); + } +} diff --git a/crates/runner/src/lib.rs b/crates/runner/src/lib.rs index 2430e52..d781b05 100644 --- a/crates/runner/src/lib.rs +++ b/crates/runner/src/lib.rs @@ -101,6 +101,7 @@ fn reject_symlink_ancestors(root: &Path, dest: &Path) -> Result<(), ErrorBody> { mod api; mod apply; +mod descriptors; mod files; mod linux_sandbox; mod patch_helper; @@ -119,6 +120,7 @@ pub use api::{ RunnerResizeResult, RunnerWriteStdin, DEFAULT_TIMEOUT_MS, MAX_OUTPUT_BYTES, }; pub use codespace_domain::{DEFAULT_FIND_LIMIT, DEFAULT_READ_LIMIT}; +pub use descriptors::exclude_unrelated; pub use files::VERSION_ABSENT; pub use patch_helper::ensure_helper_for_tests; pub use process::{ diff --git a/crates/runner/src/linux_sandbox.rs b/crates/runner/src/linux_sandbox.rs index 068c58d..6b0e2d7 100644 --- a/crates/runner/src/linux_sandbox.rs +++ b/crates/runner/src/linux_sandbox.rs @@ -77,6 +77,7 @@ fn probe_helper(helper: &Path) -> bool { .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); + crate::descriptors::exclude_unrelated_std(&mut child); let Ok(child) = child.spawn() else { return false; }; @@ -138,11 +139,14 @@ pub(crate) fn prepare_run_from_helper( argv: argv.to_vec(), }; let payload = serde_json::to_vec(&request).map_err(|err| spawn_failed(err.to_string()))?; - let spawned = Command::new(helper) + let mut command = Command::new(helper); + command .arg("prepare") .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) + .stderr(Stdio::piped()); + crate::descriptors::exclude_unrelated_std(&mut command); + let spawned = command .spawn() .map_err(|err| spawn_failed(format!("failed to spawn linux sandbox prepare: {err}")))?; prepare_from_child(helper, spawned, &payload, PREPARE_TIMEOUT, PREPARE_IO_LIMIT) diff --git a/crates/runner/src/patch_helper.rs b/crates/runner/src/patch_helper.rs index 8987d47..f734c9d 100644 --- a/crates/runner/src/patch_helper.rs +++ b/crates/runner/src/patch_helper.rs @@ -47,18 +47,19 @@ async fn invoke( check_only: bool, ) -> Result { let bin = helper_bin()?; - let mut child = Command::new(&bin) + let mut command = Command::new(&bin); + command .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) - .kill_on_drop(true) - .spawn() - .map_err(|err| { - ErrorBody::new( - ErrorCode::InvalidPatch, - format!("failed to spawn patch helper: {err}"), - ) - })?; + .kill_on_drop(true); + crate::descriptors::exclude_unrelated(&mut command); + let mut child = command.spawn().map_err(|err| { + ErrorBody::new( + ErrorCode::InvalidPatch, + format!("failed to spawn patch helper: {err}"), + ) + })?; let payload = serde_json::json!({ "op": op, "root": root, diff --git a/crates/runner/src/process.rs b/crates/runner/src/process.rs index f18d4ca..885cf6c 100644 --- a/crates/runner/src/process.rs +++ b/crates/runner/src/process.rs @@ -1,5 +1,6 @@ //! Managed workspace processes. Request lifetime is not process lifetime. -//! Pipe spawn uses `tokio::process::Command`. `tty: true` uses the isolated +//! Pipe spawn uses `tokio::process::Command`, whose children keep only their +//! standard descriptors (`descriptors`). `tty: true` uses the isolated //! `codespace-pty` adapter. When the Linux helper probe succeeds, both wrap //! the same helper argv. UDS dispatch lives in `UdsRunner`. @@ -289,6 +290,7 @@ impl InProcessRunner { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .kill_on_drop(true); + crate::descriptors::exclude_unrelated(&mut child); for (key, value) in spawn_env(&cwd, &req, launch.sandboxed) { child.env(key, value); } @@ -1485,6 +1487,30 @@ mod tests { assert!(eof, "SIGTERM on the helper must reap the sandbox tree"); } + #[tokio::test] + async fn pipe_child_keeps_only_its_standard_descriptors() { + // A descriptor that another thread's creation window could leave inheritable (#79). + let held = crate::descriptors::tests::inheritable_descriptor(); + let dir = tempdir().unwrap(); + let ws = workspace(dir.path()); + let runner = InProcessRunner::new(Arc::new(|_| {})); + let process_id = ProcessId("proc-descriptors".into()); + let script = + crate::descriptors::tests::report_script(std::os::fd::AsRawFd::as_raw_fd(&held)); + runner + .exec( + &ws, + RunnerExecRequest::for_host( + vec!["/bin/sh".into(), "-c".into(), script], + process_id.clone(), + Profile::WorkspaceWrite, + ), + ) + .await + .unwrap(); + assert_eq!(wait_chunk(&runner, &process_id).await, "held\nclear\n"); + } + #[tokio::test] async fn echo_status_is_exited_zero() { let dir = tempdir().unwrap(); diff --git a/crates/server/src/runtime.rs b/crates/server/src/runtime.rs index 0e52461..bd57d49 100644 --- a/crates/server/src/runtime.rs +++ b/crates/server/src/runtime.rs @@ -30,10 +30,7 @@ impl RuntimeProcess { let dir = allocate_private_runner_dir(runner_dir) .map_err(|err| anyhow!("private runner dir: {err}"))?; let socket = runner_socket_path(&dir); - let mut child = Command::new(bin) - .arg(&socket) - .stdin(Stdio::null()) - .kill_on_drop(true) + let mut child = worker_command(bin, &socket) .spawn() .with_context(|| format!("spawn {}", bin.display()))?; let shutdown = Arc::new(Notify::new()); @@ -119,6 +116,15 @@ impl Drop for RuntimeProcess { } } +/// The worker gets the socket path, a null stdin, the Gateway's stdout and stderr, and no other +/// descriptor of the Gateway's. +fn worker_command(bin: &Path, socket: &Path) -> Command { + let mut command = Command::new(bin); + command.arg(socket).stdin(Stdio::null()).kill_on_drop(true); + codespace_runner::exclude_unrelated(&mut command); + command +} + async fn wait_for_socket(socket: &Path, wait: &JoinHandle<()>) -> Result { for _ in 0..100 { if wait.is_finished() { @@ -142,6 +148,44 @@ mod tests { std::env::var_os("CODESPACE_RUNTIME_BIN").map(PathBuf::from) } + #[tokio::test] + async fn worker_keeps_only_its_standard_descriptors() { + use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; + // The server crate has no libc dependency; F_DUPFD is 0 on macOS and Linux. + extern "C" { + fn fcntl(fd: std::os::raw::c_int, cmd: std::os::raw::c_int, ...) + -> std::os::raw::c_int; + } + // A descriptor that another thread's creation window could leave inheritable (#79). + let null = std::fs::File::open("/dev/null").unwrap(); + // SAFETY: F_DUPFD returns a new descriptor without close-on-exec, owned below. + let held = unsafe { fcntl(null.as_raw_fd(), 0, 400) }; + assert!(held >= 400, "F_DUPFD"); + // SAFETY: as above. + let held = unsafe { OwnedFd::from_raw_fd(held) }; + let dir = tempfile::tempdir().unwrap(); + let report = dir.path().join("report"); + // `/bin/sh` reads the script given where the socket path goes; executing a freshly + // written file could meet ETXTBSY from a concurrent fork. + let script = dir.path().join("worker.sh"); + std::fs::write( + &script, + format!( + "for n in 1 {}; do if [ -e /dev/fd/$n ]; then echo held; else echo clear; fi; done > '{}'\n", + held.as_raw_fd(), + report.display() + ), + ) + .unwrap(); + let status = worker_command(Path::new("/bin/sh"), &script) + .status() + .await + .unwrap(); + assert!(status.success()); + let seen = std::fs::read_to_string(&report).unwrap(); + assert_eq!(seen, "held\nclear\n"); + } + #[tokio::test] async fn spawn_hello_then_drop_kills_worker() { let Some(bin) = runtime_bin() else { diff --git a/scripts/ci-policy.json b/scripts/ci-policy.json index 4b37558..d462aaa 100644 --- a/scripts/ci-policy.json +++ b/scripts/ci-policy.json @@ -74,7 +74,7 @@ }, "rust-macos/single": { "stages": ["macos-core", "dependencies"], "cache": "crates/file-system", - "compiles": ["file-system", "pty"] + "compiles": ["domain", "file-system", "linux-sandbox-protocol", "policy", "pty", "runner", "server", "store"] } } } diff --git a/scripts/tests/test_ci_plan.py b/scripts/tests/test_ci_plan.py index c18a32a..c8d11b1 100644 --- a/scripts/tests/test_ci_plan.py +++ b/scripts/tests/test_ci_plan.py @@ -42,14 +42,15 @@ def test_pty_selects_every_leg_that_compiles_it(self): def test_components_follow_the_crate_graph(self): rows = { 'crates/domain/src/lib.rs': ordered('rust-clippy/root', 'rust-clippy/adapters', 'rust-clippy/codex-adapters', - 'rust-unit/patch', 'rust-unit/codex-runtime', 'rust-integration/single'), + 'rust-unit/patch', 'rust-unit/codex-runtime', 'rust-integration/single', + 'rust-macos/single'), 'crates/runner/src/lib.rs': ordered('rust-clippy/root', 'rust-clippy/codex-adapters', - 'rust-unit/codex-runtime', 'rust-integration/single'), - 'crates/server/src/main.rs': ['rust-clippy/root', 'rust-integration/single'], - 'crates/store/src/lib.rs': ['rust-clippy/root', 'rust-integration/single'], + 'rust-unit/codex-runtime', 'rust-integration/single', 'rust-macos/single'), + 'crates/server/src/main.rs': ['rust-clippy/root', 'rust-integration/single', 'rust-macos/single'], + 'crates/store/src/lib.rs': ['rust-clippy/root', 'rust-integration/single', 'rust-macos/single'], 'crates/linux-sandbox-protocol/src/lib.rs': ordered( 'rust-clippy/root', 'rust-clippy/codex-adapters', 'rust-unit/codex-runtime', 'rust-unit/linux-sandbox-protocol', - 'rust-unit/linux-sandbox', 'rust-linux-isolation/single', 'rust-integration/single'), + 'rust-unit/linux-sandbox', 'rust-linux-isolation/single', 'rust-integration/single', 'rust-macos/single'), 'crates/patch/src/lib.rs': ['rust-clippy/adapters', 'rust-unit/patch', 'rust-integration/single'], 'crates/codex-runtime/src/lib.rs': ['rust-clippy/codex-adapters', 'rust-unit/codex-runtime', 'rust-integration/single'], diff --git a/scripts/validate-upstream.py b/scripts/validate-upstream.py index 1a64299..1d1fec3 100644 --- a/scripts/validate-upstream.py +++ b/scripts/validate-upstream.py @@ -52,7 +52,10 @@ def stages(): 'clippy-root': [cargo('clippy', 'root', '--all-targets', '--', '-D', 'warnings')], 'clippy-adapters': [cargo('clippy', a, '--all-targets', '--', '-D', 'warnings') for a in ('patch', 'pty', 'file-system')], 'clippy-codex': [cargo('clippy', a, '--all-targets', '--', '-D', 'warnings') for a in ('codex-runtime', 'linux-sandbox')], - 'macos-core': [cargo(action, a, *(['--all-targets', '--', '-D', 'warnings'] if action == 'clippy' else [])) for a in ('pty', 'file-system') for action in ('clippy', 'test')], + 'macos-core': [cargo(action, a, *(['--all-targets', '--', '-D', 'warnings'] if action == 'clippy' else [])) for a in ('pty', 'file-system') for action in ('clippy', 'test')] + # Descriptor hygiene of the runner host's spawns, whose macOS path the Linux legs cannot exercise. + + [cargo('test', 'root', '-p', 'codespace-runner', '--lib', '--', 'descriptors', 'missing_executable_is_process_spawn_failed'), + cargo('test', 'root', '-p', 'codespace-server', '--lib', '--', 'descriptors')], 'integration': [], 'linux-isolation': [cargo('build', 'linux-sandbox', '--bin', 'codespace-linux-sandbox'), cargo('test', 'linux-sandbox', '--test', 'isolation')], 'dependencies': [],