From e3d0002688d0264f3271294af6045b5ac8833bd7 Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Mon, 20 Jul 2026 01:20:16 -0700 Subject: [PATCH] fix(native-sidecar): stop stranding child process.fd_read on mapped fds Last instance of the class fixed in the mapped-host-fd work: a guest-supplied fd was stat'ed against the kernel fd table with a bare `?`, but mapped host fds live in the per-process map. The resulting EBADF escapes to the process-event pump and strands the in-flight sync RPC, so the guest blocks until the ~31s bridge deadline. The sibling process.fd_write arm already converted its errors into a delivered RPC error; this one did not. Adds the regression test the class never had. crash_isolation.rs previously covered only a guest THROWING, which never reaches the sidecar's own error paths and so could not catch either production failure. The new phase drives a host-backed write beside a peer VM and asserts both properties: - it completes within the collector's deadline (catches the 31s hang; a test that only checked eventual success would pass while hanging, which is exactly how that failure went unnoticed), and - the peer VM still executes afterwards (catches the cross-tenant crash). Verified by negative control that the new phase actually executes rather than silently passing. --- .../src/execution/child_process.rs | 23 +++++- .../native-sidecar/tests/crash_isolation.rs | 79 +++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/crates/native-sidecar/src/execution/child_process.rs b/crates/native-sidecar/src/execution/child_process.rs index 5b442cbd80..d64764ef2d 100644 --- a/crates/native-sidecar/src/execution/child_process.rs +++ b/crates/native-sidecar/src/execution/child_process.rs @@ -6225,9 +6225,26 @@ where } if request.method == "process.fd_read" { let fd = javascript_sync_rpc_arg_u32(&request.args, 0, "fd_read fd")?; - let stat = kernel - .fd_stat(EXECUTION_DRIVER_NAME, child.kernel_pid, fd) - .map_err(kernel_error)?; + // Same class as the `fs.write` deferral pre-check: a guest-supplied + // fd was stat'ed against the kernel fd table with a bare `?`, but + // mapped host fds live in the per-process map instead. That raised + // EBADF, which escapes to the process-event pump and strands the + // in-flight sync RPC — the guest then blocks until the ~31s bridge + // deadline. The sibling `process.fd_write` arm above already + // converts its errors into a delivered RPC error; this one did not. + // + // A host-backed fd is never a kernel pipe, so it is never parked + // here; `Ok(false)` hands it to the normal RPC handler, which does + // its own mapped lookup and raises the correct errno if the fd is + // genuinely bad. + if child.mapped_host_fd(fd).is_some() { + return Ok(false); + } + let stat = match kernel.fd_stat(EXECUTION_DRIVER_NAME, child.kernel_pid, fd) { + Ok(stat) => stat, + Err(error) if error.code() == "EBADF" => return Ok(false), + Err(error) => return Err(kernel_error(error)), + }; if matches!( stat.filetype, agentos_kernel::fd_table::FILETYPE_REGULAR_FILE diff --git a/crates/native-sidecar/tests/crash_isolation.rs b/crates/native-sidecar/tests/crash_isolation.rs index d39090f4a1..606c0250d0 100644 --- a/crates/native-sidecar/tests/crash_isolation.rs +++ b/crates/native-sidecar/tests/crash_isolation.rs @@ -173,6 +173,85 @@ fn guest_failure_in_one_vm_does_not_break_peer_vm_execution() { assert_eq!(exit_code, 0); assert!(stderr.is_empty(), "unexpected follow-up stderr: {stderr}"); + + // Regression: a guest writing to a HOST-BACKED path (the VM cwd is a real + // host directory, so these go through mapped host fds, not the kernel VFS). + // + // This vector produced two successive production failures that the guest + // exception above does NOT cover, because a thrown JS error never reaches + // the sidecar's own error paths: + // + // 1. EBADF on a mapped fd escaped the process-event pump to `main`, which + // exit(1)'d — killing every co-located VM/actor (cross-tenant DoS). + // 2. After the pump stopped propagating it, the guest's sync-RPC response + // was never delivered and every guest file write hung ~31s. + // + // Both are asserted here: the DEADLINE inside collect_crash_process_output + // catches the hang, and the peer execution afterwards catches the crash. + // A test that only checks "did it eventually succeed" would pass while + // hanging, which is precisely how the second failure went unnoticed. + let write_entry = cwd.join("host_backed_write.cjs"); + write_fixture( + &write_entry, + "const fs = require(\"node:fs\");\n\ + const chunk = Buffer.alloc(64 * 1024, 65);\n\ + const fd = fs.openSync(\"host_backed_write.bin\", \"w\");\n\ + for (let i = 0; i < 16; i++) fs.writeSync(fd, chunk);\n\ + fs.closeSync(fd);\n\ + console.log(\"wrote\");\n", + ); + execute_wire( + &mut sidecar, + 8, + &connection_id, + &session_id, + &crash_vm_id, + "proc-host-write", + GuestRuntimeKind::JavaScript, + &write_entry, + Vec::new(), + ); + let (write_stdout, write_stderr, write_exit) = collect_crash_process_output( + &mut sidecar, + &connection_id, + &session_id, + &crash_vm_id, + "proc-host-write", + ); + assert_eq!( + write_exit, 0, + "host-backed write failed (stderr: {write_stderr})" + ); + assert!( + write_stdout.contains("wrote"), + "host-backed write produced no output: {write_stdout}" + ); + + // The sidecar is shared: the peer VM must still execute afterwards. This is + // the co-tenancy invariant the cross-tenant DoS violated. + execute_wire( + &mut sidecar, + 9, + &connection_id, + &session_id, + &healthy_vm_id, + "proc-healthy-3", + GuestRuntimeKind::JavaScript, + &healthy_entry, + Vec::new(), + ); + let (_stdout, peer_stderr, peer_exit) = collect_crash_process_output( + &mut sidecar, + &connection_id, + &session_id, + &healthy_vm_id, + "proc-healthy-3", + ); + assert_eq!( + peer_exit, 0, + "peer VM stopped working after a co-located host-backed write \ + (stderr: {peer_stderr})" + ); } fn collect_crash_process_output(