fix(sandbox): document unsafe blocks and enforce SAFETY comments with clippy

crates/openshell-sandbox/src/bin/seccomp-perf.rs

@@ -3,6 +3,10 @@
 
 //! Microbenchmark entry point for the production seccomp network broker.
 
+// Crate-scoped: Cargo rejects mixing `[lints] workspace = true` with a
+// crate-specific lint table (see #5188).
+#![warn(clippy::undocumented_unsafe_blocks)]
+
 #[cfg(target_os = "linux")]
 use std::net::SocketAddr;
 

crates/openshell-sandbox/src/boundary_server.rs

@@ -4884,6 +4884,8 @@ mod linux {
                 .map_err(|_| {
                     io::Error::new(io::ErrorKind::InvalidInput, "sockaddr_vm exceeds socklen_t")
                 })?;
+            // SAFETY: socket() creates a fresh descriptor and takes only
+            // scalar arguments; the result is checked below before any use.
             let raw_fd = unsafe {
                 libc::socket(
                     libc::AF_VSOCK,
@@ -4894,6 +4896,8 @@ mod linux {
             if raw_fd < 0 {
                 return Err(io::Error::last_os_error());
             }
+            // SAFETY: raw_fd was checked non-negative above and is uniquely
+            // owned; this is its single wrap into an OwnedFd.
             let fd = unsafe { OwnedFd::from_raw_fd(raw_fd) };
             let address = libc::sockaddr_vm {
                 svm_family: family,
@@ -4902,6 +4906,8 @@ mod linux {
                 svm_cid: libc::VMADDR_CID_ANY,
                 svm_zero: [0; 4],
             };
+            // SAFETY: bind operates on the live OwnedFd with a fully
+            // initialized sockaddr_vm of the declared length.
             let result = unsafe {
                 libc::bind(
                     fd.as_raw_fd(),
@@ -4912,6 +4918,7 @@ mod linux {
             if result < 0 {
                 return Err(io::Error::last_os_error());
             }
+            // SAFETY: listen operates on the live, bound OwnedFd.
             if unsafe { libc::listen(fd.as_raw_fd(), 16) } < 0 {
                 return Err(io::Error::last_os_error());
             }
@@ -4935,6 +4942,9 @@ mod linux {
                     listener,
                     server_config,
                 } => {
+                    // SAFETY: accept4 takes the live listener descriptor with
+                    // SOCK_CLOEXEC and a valid null address buffer; the
+                    // returned descriptor is checked below before any use.
                     let raw_fd = unsafe {
                         libc::accept4(
                             listener.as_raw_fd(),
@@ -4946,8 +4956,11 @@ mod linux {
                     if raw_fd < 0 {
                         Err(io::Error::last_os_error())
                     } else {
+                        // SAFETY: raw_fd was checked non-negative above and
+                        // is uniquely owned; this is its single wrap.
+                        let stream = unsafe { File::from_raw_fd(raw_fd) };
                         Ok(ControlStream::PendingTls {
-                            stream: PlainControlStream::Vsock(unsafe { File::from_raw_fd(raw_fd) }),
+                            stream: PlainControlStream::Vsock(stream),
                             server_config: server_config.clone(),
                         })
                     }
@@ -5142,8 +5155,26 @@ mod linux {
         ) -> io::Result<openshell_isolation_interface::contract::BoundaryDuplexStream> {
             match self {
                 Self::Vsock(file) => {
-                    let stream =
-                        unsafe { std::os::unix::net::UnixStream::from_raw_fd(file.into_raw_fd()) };
+                    let raw_fd = file.into_raw_fd();
+                    // SAFETY: `into_raw_fd` handed over a valid, open,
+                    // solely-owned descriptor exactly once, so this is its
+                    // single re-wrap. The descriptor is an AF_VSOCK stream
+                    // reinterpreted as a Unix stream; the stream only ever
+                    // sees family-agnostic operations (fcntl, read/write,
+                    // epoll registration via `from_std`, and shutdown), which
+                    // behave the same on AF_VSOCK streams. `peer_addr`,
+                    // `local_addr`, and `peer_cred` must never be called on
+                    // it: they would fail at runtime on a vsock descriptor.
+                    // The surface is fenced by this private enum and by the
+                    // `Box<dyn DuplexStream>` erasure in `into_tokio`. The
+                    // dial side performs the same reinterpretation in the
+                    // `RuntimeInitiatedVsock` arm of `dial_supervisor`, but
+                    // files its vsock descriptors under the `Unix` variant
+                    // (not `Vsock`) and uses only `take_error()` (a
+                    // SOL_SOCKET getsockopt) and `writable()`, both
+                    // family-agnostic; the allowed-ops list above covers
+                    // this accept-side stream only.
+                    let stream = unsafe { std::os::unix::net::UnixStream::from_raw_fd(raw_fd) };
                     stream.set_nonblocking(true)?;
                     Ok(Box::new(tokio::net::UnixStream::from_std(stream)?))
                 }

crates/openshell-sandbox/src/lib.rs

@@ -3,6 +3,11 @@
 
 //! Capability-free in-workload sandbox boundary.
 
+// Crate-scoped: Cargo rejects mixing `[lints] workspace = true` with a
+// crate-specific lint table (see #5188).
+#![warn(clippy::undocumented_unsafe_blocks)]
+#![cfg_attr(test, allow(clippy::undocumented_unsafe_blocks))]
+
 #[cfg(target_os = "linux")]
 mod accept_errors;
 pub mod binary_pins;

crates/openshell-sandbox/src/linux/seccomp_notify.rs

@@ -831,8 +831,6 @@ fn verify_notification_sizes() -> io::Result<()> {
 }
 
 fn ioctl_ptr(fd: RawFd, request: libc::c_ulong, argument: *mut libc::c_void) -> io::Result<i64> {
-    // SAFETY: every caller supplies the UAPI structure encoded into `request`,
-    // alive and writable for the ioctl duration.
     // `libc::ioctl` models the request as `c_ulong` for glibc and `c_int`
     // for musl. Linux UAPI request values fit both representations.
     #[cfg(target_env = "musl")]
@@ -841,6 +839,8 @@ fn ioctl_ptr(fd: RawFd, request: libc::c_ulong, argument: *mut libc::c_void) ->
     })?;
     #[cfg(target_env = "musl")]
     let request = libc::c_int::from_ne_bytes(request.to_ne_bytes());
+    // SAFETY: every caller supplies the UAPI structure encoded into `request`,
+    // alive and writable for the ioctl duration.
     let result = unsafe { libc::ioctl(fd, request, argument) };
     if result < 0 {
         Err(io::Error::last_os_error())

crates/openshell-sandbox/src/main.rs

@@ -3,6 +3,11 @@
 
 //! `OpenShell` capability-free in-workload sandbox boundary.
 
+// Crate-scoped: Cargo rejects mixing `[lints] workspace = true` with a
+// crate-specific lint table (see #5188).
+#![warn(clippy::undocumented_unsafe_blocks)]
+#![cfg_attr(test, allow(clippy::undocumented_unsafe_blocks))]
+
 #[cfg(target_os = "linux")]
 use std::mem::size_of;
 use std::path::Path;
@@ -685,6 +690,10 @@ fn probe_socket_virtualization() -> Result<()> {
                 {
                     return Err(std::io::Error::last_os_error()).into_diagnostic();
                 }
+                // SAFETY: the retained source is the exact injected OFD and
+                // the payload buffer stays live for its declared length;
+                // MSG_NOSIGNAL keeps a refused send from raising SIGPIPE in
+                // the dispatcher thread.
                 let sent = unsafe {
                     libc::send(
                         retained.as_raw_fd(),
@@ -796,6 +805,15 @@ fn set_child_core_limit() -> std::io::Result<()> {
         .map_err(std::io::Error::from)
 }
 
+/// Reset every catchable signal disposition in the calling process to its
+/// default.
+///
+/// # Safety
+///
+/// Destroys all installed handlers for signals 1 through 64 (skipping the
+/// uncatchable SIGKILL and SIGSTOP) in the calling process. Must only run
+/// in the forked pre-exec child, never in the launcher parent, which still
+/// needs its own dispositions.
 #[cfg(target_os = "linux")]
 #[allow(unsafe_code)]
 unsafe fn reset_child_signal_dispositions() -> std::io::Result<()> {
@@ -1217,43 +1235,44 @@ fn probe_child_self_protection(sandbox_tgid: libc::pid_t, socket: libc::c_int) -
         )
     };
     require_probe_errno(read, libc::EPERM, "process_vm_readv sandbox")?;
-    // SAFETY: signal zero would only probe process existence if the filter did
-    // not reject the trusted sandbox target.
     require_probe_errno(
+        // SAFETY: signal zero would only probe process existence if the
+        // filter did not reject the trusted sandbox target.
         isize::try_from(unsafe { libc::kill(sandbox_tgid, 0) }).expect("kill result fits isize"),
         libc::EPERM,
         "kill sandbox",
     )?;
     // SAFETY: scalar syscall arguments request a read-only resource query;
     // the child filter rejects non-self targets.
+    let prlimit_result = isize::try_from(unsafe {
+        libc::syscall(libc::SYS_prlimit64, sandbox_tgid, libc::RLIMIT_CORE, 0, 0)
+    })
+    .expect("prlimit result fits isize");
+    require_probe_errno(prlimit_result, libc::EPERM, "prlimit sandbox")?;
+    // SAFETY: signal zero on the process group would only probe group
+    // existence if the filter did not reject the trusted sandbox target.
+    let group_kill_result = unsafe { libc::kill(-sandbox_tgid, 0) };
     require_probe_errno(
-        isize::try_from(unsafe {
-            libc::syscall(libc::SYS_prlimit64, sandbox_tgid, libc::RLIMIT_CORE, 0, 0)
-        })
-        .expect("prlimit result fits isize"),
-        libc::EPERM,
-        "prlimit sandbox",
-    )?;
-    require_probe_errno(
-        isize::try_from(unsafe { libc::kill(-sandbox_tgid, 0) }).expect("kill result fits isize"),
+        isize::try_from(group_kill_result).expect("kill result fits isize"),
         libc::EPERM,
         "process-group signal",
     )?;
+    // SAFETY: F_SETOWN would only record the sandbox target as the owner of
+    // the child's own socket if the filter did not reject it first.
+    let fcntl_result = unsafe { libc::fcntl(socket, libc::F_SETOWN, sandbox_tgid) };
     require_probe_errno(
-        isize::try_from(unsafe { libc::fcntl(socket, libc::F_SETOWN, sandbox_tgid) })
-            .expect("fcntl result fits isize"),
+        isize::try_from(fcntl_result).expect("fcntl result fits isize"),
         libc::EPERM,
         "fcntl F_SETOWN",
     )?;
     let mut owner = sandbox_tgid;
-    require_probe_errno(
-        isize::try_from(unsafe {
-            libc::syscall(libc::SYS_ioctl, socket, 0x8901_u32, &raw mut owner)
-        })
-        .expect("ioctl result fits isize"),
-        libc::EPERM,
-        "ioctl FIOSETOWN",
-    )?;
+    // SAFETY: FIOSETOWN would only record the sandbox target as the owner of
+    // the child's own socket if the filter did not reject it first.
+    let ioctl_result = isize::try_from(unsafe {
+        libc::syscall(libc::SYS_ioctl, socket, 0x8901_u32, &raw mut owner)
+    })
+    .expect("ioctl result fits isize");
+    require_probe_errno(ioctl_result, libc::EPERM, "ioctl FIOSETOWN")?;
     Ok(())
 }
 
@@ -2039,4 +2058,149 @@ mod tests {
         let final_path = dest_dir.join("openshell-sandbox");
         assert!(final_path.exists(), "binary should land inside dest dir");
     }
+
+    /// Environment variable marking the re-executed probe child process.
+    const PROBE_CHILD_ENV: &str = "OPENSHELL_SANDBOX_PROBE_CHILD";
+
+    /// Re-executes `test_path` (the full libtest path, module prefix
+    /// included) in a fresh process so it can change its rlimits and load
+    /// seccomp filters without contaminating the test harness. The child
+    /// asserts on its own probe results; this parent only requires a
+    /// successful exit. A fresh executable process, unlike a raw `fork` child
+    /// of the multithreaded test harness, may allocate freely after the
+    /// filter is installed.
+    fn run_isolated_probe_child(test_path: &str) {
+        let output = std::process::Command::new(std::env::current_exe().expect("test executable"))
+            .args(["--exact", test_path, "--nocapture"])
+            .env(PROBE_CHILD_ENV, "1")
+            .output()
+            .expect("start isolated probe child process");
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        assert!(
+            output.status.success(),
+            "isolated probe child failed:\nstdout: {stdout}\nstderr: {}",
+            String::from_utf8_lossy(&output.stderr)
+        );
+        // `--exact` matches full test paths; a wrong name would silently
+        // select zero tests and exit successfully.
+        assert!(
+            stdout.contains("1 passed"),
+            "isolated probe child matched no test:\nstdout: {stdout}"
+        );
+    }
+
+    /// Prepares a probe child: zeroes the core-dump limit the probe requires
+    /// and allows the unprivileged seccomp install.
+    #[allow(unsafe_code)]
+    fn prepare_probe_child() -> std::io::Result<()> {
+        nix::sys::resource::setrlimit(nix::sys::resource::Resource::RLIMIT_CORE, 0, 0)
+            .map_err(std::io::Error::from)?;
+        // SAFETY: PR_SET_NO_NEW_PRIVS is a one-way scalar transition required
+        // before loading an unprivileged seccomp filter.
+        if unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) } < 0 {
+            return Err(std::io::Error::last_os_error());
+        }
+        Ok(())
+    }
+
+    /// The production child filter rejects every self-protection probe with
+    /// EPERM. Running `probe_child_self_protection` against that exact filter
+    /// keeps each hoisted probe result wired to its own errno check. The test
+    /// re-executes itself in a fresh process so the rlimit and filter changes
+    /// never touch the test harness process.
+    #[test]
+    fn probe_child_self_protection_accepts_the_production_child_filter() {
+        if std::env::var_os(PROBE_CHILD_ENV).is_none() {
+            run_isolated_probe_child(
+                "tests::probe_child_self_protection_accepts_the_production_child_filter",
+            );
+            return;
+        }
+        prepare_probe_child().expect("prepare probe child");
+        let tgid = std::process::id();
+        let sandbox_tgid = libc::pid_t::try_from(tgid).expect("test pid fits pid_t");
+        let mut program = openshell_sandbox::linux::child_seccomp::prepare(tgid)
+            .expect("prepare production child filter");
+        program.install().expect("install production child filter");
+        probe_child_self_protection(sandbox_tgid, -1)
+            .expect("probes pass under the production child filter");
+    }
+
+    /// Denies every probe except the FIOSETOWN ioctl with EPERM; the kernel
+    /// then rejects the ioctl's invalid descriptor with EBADF. The
+    /// distinct errno only satisfies the check that belongs to the ioctl probe
+    /// when every check runs directly after its own syscall.
+    fn probe_adjacency_filter() -> seccompiler::BpfProgram {
+        use seccompiler::{
+            SeccompAction, SeccompCmpArgLen, SeccompCmpOp, SeccompCondition, SeccompFilter,
+            SeccompRule,
+        };
+        use std::collections::BTreeMap;
+
+        let mut rules: BTreeMap<i64, Vec<SeccompRule>> = BTreeMap::new();
+        rules.insert(libc::SYS_process_vm_readv, Vec::new());
+        rules.insert(libc::SYS_kill, Vec::new());
+        rules.insert(
+            libc::SYS_prlimit64,
+            vec![
+                SeccompRule::new(vec![
+                    SeccompCondition::new(0, SeccompCmpArgLen::Dword, SeccompCmpOp::Ne, 0)
+                        .expect("valid prlimit64 argument condition"),
+                ])
+                .expect("valid prlimit64 rule"),
+            ],
+        );
+        rules.insert(
+            libc::SYS_fcntl,
+            vec![
+                SeccompRule::new(vec![
+                    SeccompCondition::new(
+                        1,
+                        SeccompCmpArgLen::Dword,
+                        SeccompCmpOp::Eq,
+                        u64::from(libc::F_SETOWN.cast_unsigned()),
+                    )
+                    .expect("valid fcntl command condition"),
+                ])
+                .expect("valid fcntl rule"),
+            ],
+        );
+        let architecture = std::env::consts::ARCH
+            .try_into()
+            .expect("supported target architecture");
+        SeccompFilter::new(
+            rules,
+            SeccompAction::Allow,
+            SeccompAction::Errno(libc::EPERM.cast_unsigned()),
+            architecture,
+        )
+        .expect("build probe filter")
+        .try_into()
+        .expect("compile probe filter")
+    }
+
+    #[test]
+    fn probe_child_self_protection_checks_each_syscall_errno_independently() {
+        if std::env::var_os(PROBE_CHILD_ENV).is_none() {
+            run_isolated_probe_child(
+                "tests::probe_child_self_protection_checks_each_syscall_errno_independently",
+            );
+            return;
+        }
+        prepare_probe_child().expect("prepare probe child");
+        let filter = probe_adjacency_filter();
+        seccompiler::apply_filter(&filter).expect("install adjacency filter");
+        let sandbox_tgid = libc::pid_t::try_from(std::process::id()).expect("test pid fits pid_t");
+        // The invalid descriptor is enough: the fcntl probe is rejected by
+        // the filter before the kernel validates it, and the unfiltered
+        // ioctl probe reports EBADF from the kernel.
+        let error = probe_child_self_protection(sandbox_tgid, -1)
+            .expect_err("adjacency filter must let the ioctl probe through");
+        assert!(
+            error
+                .to_string()
+                .contains("ioctl FIOSETOWN was not rejected with errno"),
+            "unexpected probe failure: {error}"
+        );
+    }
 }