diff --git a/crates/openshell-driver-vm/src/main.rs b/crates/openshell-driver-vm/src/main.rs index 0ab4743771..aac009bc59 100644 --- a/crates/openshell-driver-vm/src/main.rs +++ b/crates/openshell-driver-vm/src/main.rs @@ -322,17 +322,17 @@ async fn main() -> Result<()> { let listen_mode = compute_driver_listen_mode(&args).map_err(|err| miette::miette!("{err}"))?; - // Arm procguard so that if the gateway is killed (SIGKILL or crash) - // we also die. Without this the driver is reparented to init and - // keeps its per-sandbox VM launchers alive forever. Launchers have - // their own procguards (armed in `run_vm`) which cascade cleanup of - // the libkrun worker the moment this driver exits. - if let Err(err) = procguard::die_with_parent() { - tracing::warn!( - error = %err, - "procguard arm failed; gateway crashes may orphan this driver" - ); - } + // The managed driver must not serve after its gateway exits. Its own + // process group keeps terminal SIGINT away during the gateway shutdown + // sweep, so a failed parent-death guard must stop startup instead of + // leaving the driver and its VM launchers behind. + let expected_gateway_pid = match &listen_mode { + ComputeDriverListenMode::Unix { + expected_peer_pid, .. + } => *expected_peer_pid, + ComputeDriverListenMode::Tcp(_) => None, + }; + arm_driver_parent_guard(expected_gateway_pid)?; let driver = VmDriver::new(VmDriverConfig { allow_driver_config: args.admission_config_json.allow_driver_config, @@ -424,6 +424,43 @@ async fn main() -> Result<()> { result } +fn arm_driver_parent_guard(expected_gateway_pid: Option) -> Result<()> { + if let Some(expected) = expected_gateway_pid { + ensure_gateway_parent(expected)?; + } + + // Launchers arm their own procguards in `run_vm`, which cascade cleanup + // to libkrun workers when this driver exits. + match procguard::die_with_parent() { + Ok(()) => {} + Err(err) if expected_gateway_pid.is_some() => { + return Err(miette::miette!( + "failed to arm managed VM driver parent-death guard: {err}" + )); + } + Err(err) => { + tracing::warn!(error = %err, "procguard arm failed; driver may outlive its parent"); + } + } + + // A subreaper can adopt an orphan before procguard reads getppid(). In + // that case it could arm against the subreaper instead of the gateway. + if let Some(expected) = expected_gateway_pid { + ensure_gateway_parent(expected)?; + } + Ok(()) +} + +fn ensure_gateway_parent(expected: u32) -> Result<()> { + let actual = nix::unistd::getppid().as_raw(); + if i64::from(actual) != i64::from(expected) { + return Err(miette::miette!( + "expected gateway parent PID {expected}, found {actual}" + )); + } + Ok(()) +} + async fn shutdown_signal() { let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) .expect("install SIGTERM handler"); diff --git a/crates/openshell-driver-vm/tests/parent_death.rs b/crates/openshell-driver-vm/tests/parent_death.rs new file mode 100644 index 0000000000..62ea6d1e81 --- /dev/null +++ b/crates/openshell-driver-vm/tests/parent_death.rs @@ -0,0 +1,56 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::io::Read; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +#[test] +fn managed_driver_rejects_a_missing_gateway_parent() { + let state = tempfile::tempdir().expect("create driver state directory"); + let socket = state.path().join("driver.sock"); + let stale_gateway_pid = std::process::id().wrapping_add(1); + + let mut child = Command::new(env!("CARGO_BIN_EXE_openshell-driver-vm")) + .arg("--bind-socket") + .arg(&socket) + .arg("--expected-peer-pid") + .arg(stale_gateway_pid.to_string()) + .arg("--grpc-endpoint") + .arg("http://127.0.0.1:9") + .arg("--bootstrap-image") + .arg("example.invalid/bootstrap:latest") + .arg("--state-dir") + .arg(state.path()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .expect("start managed driver"); + + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + if let Some(status) = child.try_wait().expect("poll driver") { + break status; + } + if Instant::now() >= deadline { + child.kill().expect("stop driver after timeout"); + child.wait().expect("reap driver after timeout"); + panic!("driver kept running after its expected gateway parent was absent"); + } + std::thread::sleep(Duration::from_millis(20)); + }; + + let mut stderr = String::new(); + child + .stderr + .take() + .expect("driver stderr") + .read_to_string(&mut stderr) + .expect("read driver stderr"); + assert!(!status.success(), "driver unexpectedly succeeded: {stderr}"); + assert!( + stderr.contains("expected gateway parent PID"), + "driver did not reject the stale gateway parent: {stderr}" + ); + assert!(!socket.exists(), "driver created a socket after rejection"); +} diff --git a/crates/openshell-gateway/src/vm.rs b/crates/openshell-gateway/src/vm.rs index 2f9ea5cc8e..5b76a55c9c 100644 --- a/crates/openshell-gateway/src/vm.rs +++ b/crates/openshell-gateway/src/vm.rs @@ -539,11 +539,7 @@ pub async fn spawn( let guest_tls_paths = compute_driver_guest_tls_paths(vm_config)?; prepare_compute_driver_socket_path(vm_config, &socket_path)?; - let mut command = Command::new(&driver_bin); - command.kill_on_drop(true); - command.stdin(Stdio::null()); - command.stdout(Stdio::inherit()); - command.stderr(Stdio::inherit()); + let mut command = compute_driver_command(&driver_bin); command.arg("--bind-socket").arg(&socket_path); command.arg("--admission-config-json").arg( serde_json::to_string(&openshell_core::resource_admission::DriverAdmissionConfig { @@ -592,6 +588,7 @@ pub async fn spawn( driver_bin.display() )) })?; + forward_compute_driver_output(&mut child); let channel = wait_for_compute_driver(&socket_path, &mut child).await?; let process = Arc::new(ManagedDriverProcess::new(child, socket_path)); Ok(AcquiredRemoteDriverEndpoint::managed( @@ -599,6 +596,47 @@ pub async fn spawn( )) } +/// Base command for the VM compute-driver subprocess. +/// +/// The driver gets its own process group. Ctrl+C on a foreground gateway +/// sends SIGINT to the terminal's foreground process group; if the driver +/// shared the gateway's group it would exit on that signal before the +/// gateway's shutdown sweep could stop its sandboxes. In its own group the +/// driver keeps serving until the gateway stops it through +/// `ManagedDriverProcess::shutdown` after the sweep. A gateway that dies +/// without that shutdown still takes the driver with it: the driver arms a +/// parent-death signal at startup, which does not depend on the group. Pipe +/// stdout and stderr so the gateway can write the driver's logs to its own +/// terminal; the driver's background group must not write there when TOSTOP +/// is enabled. +#[cfg(unix)] +fn compute_driver_command(driver_bin: &Path) -> Command { + let mut command = Command::new(driver_bin); + command.kill_on_drop(true); + command.process_group(0); + command.stdin(Stdio::null()); + command.stdout(Stdio::piped()); + command.stderr(Stdio::piped()); + command +} + +#[cfg(unix)] +fn forward_compute_driver_output(child: &mut tokio::process::Child) { + let mut child_stdout = child.stdout.take().expect("vm driver stdout is piped"); + tokio::spawn(async move { + if let Err(error) = tokio::io::copy(&mut child_stdout, &mut tokio::io::stdout()).await { + tracing::warn!(%error, "failed to forward vm compute driver stdout"); + } + }); + + let mut child_stderr = child.stderr.take().expect("vm driver stderr is piped"); + tokio::spawn(async move { + if let Err(error) = tokio::io::copy(&mut child_stderr, &mut tokio::io::stderr()).await { + tracing::warn!(%error, "failed to forward vm compute driver stderr"); + } + }); +} + fn validate_vm_sandbox_identity(config: &VmComputeConfig) -> Result<()> { let range = openshell_policy::MIN_SANDBOX_UID..=openshell_policy::MAX_SANDBOX_UID; for (field, value) in [ @@ -797,10 +835,10 @@ async fn connect_compute_driver(socket_path: &Path) -> Result { mod tests { use super::{ VmComputeConfig, append_otlp_args, append_vm_identity_args, append_vm_network_trust_args, - append_vm_proxy_and_spiffe_args, append_vm_rootfs_tar_args, compute_driver_guest_tls_paths, - compute_driver_socket_path, current_euid, prepare_compute_driver_socket_path, - prepare_vm_state_dir, resolve_compute_driver_bin, resolve_driver_search_dirs, - validate_vm_sandbox_identity, + append_vm_proxy_and_spiffe_args, append_vm_rootfs_tar_args, compute_driver_command, + compute_driver_guest_tls_paths, compute_driver_socket_path, current_euid, + prepare_compute_driver_socket_path, prepare_vm_state_dir, resolve_compute_driver_bin, + resolve_driver_search_dirs, validate_vm_sandbox_identity, }; use openshell_core::{NetworkSupervisorTrustBundle, UpstreamProxyConfig}; use openshell_server::config_file::{OcsfOtlpConfig, OtlpConfig, OtlpTracesConfig}; @@ -845,6 +883,51 @@ mod tests { .expect("an explicit bootstrap image is valid"); } + #[tokio::test] + async fn vm_driver_command_runs_driver_in_its_own_process_group() { + use rustix::process::{Pid, getpgid}; + + let mut child = compute_driver_command(std::path::Path::new("sleep")) + .arg("30") + .spawn() + .expect("spawn stand-in driver"); + let pid = Pid::from_raw(i32::try_from(child.id().expect("child pid")).unwrap()) + .expect("child pid is non-zero"); + let child_group = getpgid(Some(pid)).expect("read child process group"); + let gateway_group = getpgid(None).expect("read own process group"); + child.kill().await.expect("stop stand-in driver"); + + assert_eq!( + child_group, pid, + "the driver must lead its own process group" + ); + assert_ne!( + child_group, gateway_group, + "terminal SIGINT to the gateway's group must not reach the driver" + ); + } + + #[tokio::test] + async fn vm_driver_command_pipes_both_output_streams() { + let child = compute_driver_command(std::path::Path::new("sh")) + .arg("-c") + .arg("printf 'driver stdout'; printf 'driver stderr' >&2") + .spawn() + .expect("run stand-in driver"); + + assert!(child.stdout.is_some(), "driver stdout must be piped"); + assert!(child.stderr.is_some(), "driver stderr must be piped"); + + let output = child + .wait_with_output() + .await + .expect("collect driver output"); + + assert!(output.status.success()); + assert_eq!(output.stdout, b"driver stdout"); + assert_eq!(output.stderr, b"driver stderr"); + } + #[test] fn vm_driver_command_includes_gateway_otlp_configuration() { let mut command = tokio::process::Command::new("openshell-driver-vm");