Use process groups and clean orphaned sandbox data

Author: richiemcilroy

crates/sandchest-node/src/firecracker.rs

@@ -106,6 +106,7 @@ impl FirecrackerVm {
             .stdout(Stdio::piped())
             .stderr(Stdio::piped())
             .kill_on_drop(true)
+            .process_group(0)
             .spawn()
             .map_err(|e| FirecrackerError::Spawn(format!("failed to spawn firecracker: {}", e)))?;
 
@@ -217,7 +218,7 @@ impl FirecrackerVm {
         #[cfg(unix)]
         if let Some(pid) = self.child.id() {
             unsafe {
-                libc::kill(pid as i32, libc::SIGTERM);
+                libc::kill(-(pid as i32), libc::SIGTERM);
             }
 
             // Wait up to 5 seconds for graceful exit
@@ -226,7 +227,10 @@ impl FirecrackerVm {
 
             if graceful.is_err() {
                 warn!(sandbox_id = %self.sandbox_id, "Firecracker did not exit gracefully, sending SIGKILL");
-                let _ = self.child.kill().await;
+                unsafe {
+                    libc::kill(-(pid as i32), libc::SIGKILL);
+                }
+                let _ = self.child.wait().await;
             }
         }
 

crates/sandchest-node/src/jailer.rs

@@ -258,6 +258,9 @@ pub fn build_jailer_command(
         .stderr(Stdio::piped())
         .kill_on_drop(true);
 
+    #[cfg(unix)]
+    cmd.process_group(0);
+
     cmd
 }
 

crates/sandchest-node/src/sandbox.rs

@@ -594,6 +594,7 @@ impl SandboxManager {
                 .stdout(std::process::Stdio::piped())
                 .stderr(std::process::Stdio::piped())
                 .kill_on_drop(true)
+                .process_group(0)
                 .spawn()
                 .map_err(|e| {
                     SandboxError::CreateFailed(format!("failed to spawn firecracker: {}", e))
@@ -1080,6 +1081,7 @@ impl SandboxManager {
                 .stdout(std::process::Stdio::piped())
                 .stderr(std::process::Stdio::piped())
                 .kill_on_drop(true)
+                .process_group(0)
                 .spawn()
             {
                 Ok(c) => c,
@@ -1214,6 +1216,7 @@ impl SandboxManager {
         info!(sandbox_id = %sandbox_id, "destroying sandbox");
 
         self.set_status(sandbox_id, SandboxStatus::Stopping).await;
+        let paths = self.sandbox_paths(sandbox_id);
 
         // Get the network slot before removing sandbox info
         let network_slot = {
@@ -1229,6 +1232,18 @@ impl SandboxManager {
             }
         }
 
+        // Best-effort filesystem cleanup even if the VM handle was missing.
+        if Path::new(&paths.data_dir).exists() {
+            if let Err(e) = tokio::fs::remove_dir_all(&paths.data_dir).await {
+                error!(
+                    sandbox_id = %sandbox_id,
+                    dir = %paths.data_dir,
+                    error = %e,
+                    "failed to clean up sandbox data directory"
+                );
+            }
+        }
+
         // Tear down networking
         if let Some(slot) = network_slot {
             network::teardown_network(sandbox_id, slot).await;
@@ -1872,6 +1887,21 @@ mod tests {
         assert!(result.is_ok());
     }
 
+    #[tokio::test]
+    async fn destroy_sandbox_cleans_up_orphaned_disk_without_vm_handle() {
+        let manager = SandboxManager::new(test_node_config());
+        let sandbox_dir = std::path::Path::new("/tmp/sandchest-test")
+            .join("sandboxes")
+            .join("sb_orphaned");
+        let _ = std::fs::remove_dir_all(&sandbox_dir);
+        std::fs::create_dir_all(&sandbox_dir).unwrap();
+        std::fs::write(sandbox_dir.join("rootfs.ext4"), b"orphaned").unwrap();
+
+        let result = manager.destroy_sandbox("sb_orphaned").await;
+        assert!(result.is_ok());
+        assert!(!sandbox_dir.exists());
+    }
+
     #[tokio::test]
     async fn event_dropped_when_channel_full() {
         let (tx, _rx) = crate::events::channel(1);