fix(cli): gate bakes on free space, and always remove the staging dir

CHANGELOG.md

@@ -33,6 +33,26 @@ snapshot is fully published, so a tag always holds a consistent
 src == dst guard comment duplication and makes the chain-unpack tail
 bail instead of resolving an empty path.
 
+### Bakes refuse to start on a nearly full disk, and clean up after themselves
+
+Running out of space mid-write does not fail cleanly — it leaves a
+*corrupt* artifact: a rootfs whose package files contain other files'
+bytes, or a truncated `memory.bin`. Inside a guest that surfaces much
+later as `uname: option requires an argument`, `Exec format error`, or
+`EBADMSG` on a `/var/lib/dpkg` entry, which reads like a broken build
+rather than a broken image. `forkd parent build` (conversion) and
+`forkd snapshot` now check free space on the target filesystem first and
+refuse with an explicit error below a 5 GiB reserve
+(`FORKD_MIN_FREE_GIB` to override). The check is advisory: if `statvfs`
+cannot run, it warns and continues rather than blocking work on a broken
+measurement.
+
+`forkd snapshot` also removes its staging dir on every exit now, not only
+on success. A failure after the volatile artifacts were written — boot
+timeout, snapshot error, publish error, interrupt — used to leave a fully
+written `memory.bin` (GBs) beside the snapshot dir that nothing ever
+collected.
+
 ### A failed boot no longer orphans its Firecracker process
 
 `Vm::boot` spawned Firecracker and then issued the `/boot-source`,

crates/forkd-cli/src/doctor.rs

@@ -519,6 +519,59 @@ fn check_docker_daemon() -> Check {
     }
 }
 
+/// Free bytes on the filesystem holding `path` (`statvfs` `f_bavail`
+/// × `f_frsize`) — the space actually available to a non-root writer.
+///
+/// `path` need not exist: the call walks up to the nearest existing
+/// parent, so callers can gate a directory they are about to create.
+/// Shared by `forkd doctor` and the bake preflight
+/// (`require_free_space` in `main.rs`), which is the point of taking a
+/// path argument — the doctor's own hardcoded snapshot dir is not the
+/// only filesystem a bake writes to.
+///
+/// Returns `Err` on Unix when no ancestor exists or `statvfs` fails.
+#[cfg(unix)]
+pub(crate) fn available_bytes(path: &std::path::Path) -> std::io::Result<u64> {
+    use std::os::unix::ffi::OsStrExt;
+    let mut probe = path.to_path_buf();
+    while !probe.exists() {
+        match probe.parent() {
+            Some(p) if p.as_os_str() != probe.as_os_str() => probe = p.to_path_buf(),
+            _ => {
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    format!("no existing ancestor of {}", path.display()),
+                ))
+            }
+        }
+    }
+    let c_path = std::ffi::CString::new(probe.as_os_str().as_bytes()).map_err(|_| {
+        std::io::Error::new(
+            std::io::ErrorKind::InvalidInput,
+            format!("path contains NUL: {}", probe.display()),
+        )
+    })?;
+    let mut buf: libc::statvfs = unsafe { std::mem::zeroed() };
+    let rc = unsafe { libc::statvfs(c_path.as_ptr(), &mut buf) };
+    if rc != 0 {
+        return Err(std::io::Error::last_os_error());
+    }
+    Ok((buf.f_bavail as u64).saturating_mul(buf.f_frsize as u64))
+}
+
+/// Non-Unix stub: there is no portable free-space call here, and
+/// reporting a number would be worse than reporting none.
+#[cfg(not(unix))]
+pub(crate) fn available_bytes(path: &std::path::Path) -> std::io::Result<u64> {
+    Err(std::io::Error::new(
+        std::io::ErrorKind::Unsupported,
+        format!(
+            "free-space check is Unix-only ({} not probed)",
+            path.display()
+        ),
+    ))
+}
+
 fn check_snapshot_dir_space() -> Check {
     #[cfg(unix)]
     {
@@ -531,30 +584,15 @@ fn check_snapshot_dir_space() -> Check {
             .map(std::path::PathBuf::from)
             .unwrap_or_else(|| home.join(".local/share"));
         let dir = xdg.join("forkd/snapshots");
-        // statvfs needs a path that exists; walk up to an existing parent.
-        let mut probe = dir.clone();
-        while !probe.exists() {
-            match probe.parent() {
-                Some(p) if p.as_os_str() != probe.as_os_str() => probe = p.to_path_buf(),
-                _ => return Check::warn("snapshot dir space", "no path to stat", ""),
-            }
-        }
-        use std::os::unix::ffi::OsStrExt;
-        let c_path = match std::ffi::CString::new(probe.as_os_str().as_bytes()) {
-            Ok(c) => c,
-            Err(_) => return Check::warn("snapshot dir space", "bad path", ""),
+        let avail_bytes = match available_bytes(&dir) {
+            Ok(bytes) => bytes,
+            Err(e) => return Check::warn("snapshot dir space", format!("statvfs failed: {e}"), ""),
         };
-        let mut buf: libc::statvfs = unsafe { std::mem::zeroed() };
-        let rc = unsafe { libc::statvfs(c_path.as_ptr(), &mut buf) };
-        if rc != 0 {
-            return Check::warn("snapshot dir space", "statvfs failed", "");
-        }
-        let avail_bytes = (buf.f_bavail as u64).saturating_mul(buf.f_frsize as u64);
         let avail_gib = avail_bytes as f64 / 1024.0 / 1024.0 / 1024.0;
         if avail_gib >= 5.0 {
             Check::pass(
                 "snapshot dir space",
-                format!("{avail_gib:.1} GiB free at {}", probe.display()),
+                format!("{avail_gib:.1} GiB free at {}", dir.display()),
             )
         } else if avail_gib >= 1.0 {
             Check::warn(

crates/forkd-cli/src/main.rs

@@ -2185,6 +2185,12 @@ fn parent_build_cmd(
     // The build script does sudo internally for mount/chroot. Run as
     // current user; the user is expected to run `sudo forkd ...` once
     // for the whole pipeline (kvm + netns + bind mount all need root).
+    // Conversion is the first multi-GB write of the pipeline: it unpacks a
+    // full image into a fresh ext4, and an ENOSPC partway through yields a
+    // rootfs with foreign content in package files (a swapped
+    // `/usr/bin/uname`, `EBADMSG` on `/var/lib/dpkg` entries) that passes
+    // every "does it run" check. Refuse before the first byte.
+    require_free_space(&out, "convert an image to a rootfs")?;
     let mut cmd = std::process::Command::new("bash");
     cmd.arg(&script)
         .arg(&image)
@@ -2746,6 +2752,57 @@ fn eval_cmd(target: String, child: Option<String>, code: Vec<String>) -> Result<
     Ok(())
 }
 
+/// Free-space reserve a bake must have before it starts writing. Matches
+/// `forkd doctor`'s "recommended ≥5 GiB for warmed parent rootfs +
+/// memory.bin". Override with `FORKD_MIN_FREE_GIB` (e.g. on a host whose
+/// artifacts are known to be small).
+const DEFAULT_MIN_FREE_GIB: f64 = 5.0;
+
+fn min_free_gib() -> f64 {
+    std::env::var("FORKD_MIN_FREE_GIB")
+        .ok()
+        .and_then(|v| v.parse::<f64>().ok())
+        .filter(|g| *g >= 0.0)
+        .unwrap_or(DEFAULT_MIN_FREE_GIB)
+}
+
+/// Refuse to start a step that writes gigabytes when its target
+/// filesystem is nearly full.
+///
+/// Running out of space partway through does not fail cleanly: the
+/// artifact is left *corrupt* rather than obviously incomplete — a
+/// half-written ext4 or a truncated `memory.bin` — and that surfaces
+/// much later as random guest breakage (a swapped `/usr/bin/uname`, a
+/// `EBADMSG` on a directory entry, an `Exec format error`) that reads
+/// like a broken build rather than a broken image. One clear error here
+/// is worth a lot of downstream debugging.
+///
+/// The probe is advisory by design: if `statvfs` cannot run, warn and
+/// continue rather than block work on a broken measurement.
+fn require_free_space(path: &std::path::Path, what: &str) -> Result<()> {
+    const GIB: f64 = 1024.0 * 1024.0 * 1024.0;
+    let reserve = min_free_gib() * GIB;
+    match crate::doctor::available_bytes(path) {
+        Ok(avail) if (avail as f64) < reserve => bail!(
+            "refusing to {what}: only {:.1} GiB free on the filesystem holding {} \
+             (need {:.1} GiB).\n   A write that runs out of space leaves a corrupt \
+             artifact, not an incomplete one — free space and retry, or lower the \
+             reserve with FORKD_MIN_FREE_GIB.",
+            avail as f64 / GIB,
+            path.display(),
+            min_free_gib(),
+        ),
+        Ok(_) => Ok(()),
+        Err(e) => {
+            eprintln!(
+                "    note: could not check free space for {} ({e})",
+                path.display()
+            );
+            Ok(())
+        }
+    }
+}
+
 #[allow(clippy::too_many_arguments)] // mirrors the CLI flag surface 1-to-1
 fn snapshot_cmd(
     tag: Option<String>,
@@ -2850,6 +2907,10 @@ fn snapshot_cmd(
     // was working around: since the baseline is never written to, it
     // never has uncommitted journal transactions or dirty metadata.
     let snap_dir = snapshot_dir(&tag);
+    // A bake writes the cloned rootfs plus a fresh `memory.bin` here; on a
+    // nearly full filesystem the result is a corrupt snapshot rather than a
+    // failed command, so stop before the first write.
+    require_free_space(&snap_dir, "bake a snapshot")?;
     // Distinct staging dir beside the target. The pid suffix keeps
     // concurrent runs from colliding; the `staging-` prefix keeps it
     // out of SNAPSHOT_FILES / list_local enumeration.
@@ -2860,6 +2921,13 @@ fn snapshot_cmd(
         std::fs::remove_dir_all(&staging_dir)
             .with_context(|| format!("remove stale staging dir {}", staging_dir.display()))?;
     }
+    // Sweep the staging dir on every exit from here on. Only the success
+    // path used to remove it, so any `?` after the files were written —
+    // boot timeout, snapshot error, publish error, interrupt — left a
+    // fully-written `memory.bin` (GBs) behind, and nothing ever collected
+    // it. Declared before `rootfs_rollback` so the two unwind in reverse:
+    // the rootfs is restored first, then the staging shell is removed.
+    let _staging_guard = StagingDirGuard(staging_dir.clone());
 
     // src == dst guard (review #295 blocker 3 / 2026-08-22): reject
     // cloning the baseline into the snapshot's own final rootfs path. See
@@ -3093,9 +3161,9 @@ fn snapshot_cmd(
             ),
         }
     }
-    // Drop the staged-only dir (now empty of the files we renamed, or
-    // holding only a leftover on a partial failure).
-    let _ = std::fs::remove_dir_all(&staging_dir);
+    // The staged-only dir is removed by `_staging_guard` on scope exit —
+    // on success it holds nothing but the shell the files were renamed out
+    // of, and on failure it must not survive at all.
     eprintln!("    published snapshot → {}", snap_dir.display());
 
     // Parent VM is dead and the snapshot lives under data_dir; work_dir
@@ -3139,6 +3207,25 @@ fn cleanup_workdir(work_dir: &std::path::Path) {
     }
 }
 
+/// Removes a bake's staging dir when it goes out of scope.
+///
+/// The volatile artifacts (`vmstate`, `memory.bin`, `snapshot.json`) are
+/// written here and renamed into the snapshot dir at publish, so the dir
+/// is scratch in every outcome. Only the success path used to remove it,
+/// which meant any failure after the files were written — boot timeout,
+/// snapshot error, publish error, interrupt — left a fully written
+/// `memory.bin` behind for good. Holds the path, not a handle, so the
+/// successful rename of the files out of the dir is unaffected.
+struct StagingDirGuard(std::path::PathBuf);
+
+impl Drop for StagingDirGuard {
+    fn drop(&mut self) {
+        // Best-effort and NotFound-tolerant: a successful publish may have
+        // already emptied it, and a failure may have removed it.
+        let _ = std::fs::remove_dir_all(&self.0);
+    }
+}
+
 /// Rollback guard for re-baking an existing tag (review #295 blocker 1,
 /// 2026-08-22 r9).
 ///
@@ -4496,6 +4583,57 @@ mod tests {
         );
     }
 
+    /// The staging guard is the only thing between a failed bake and a
+    /// multi-GB `memory.bin` left on disk forever: only the success path
+    /// used to clean up.
+    #[test]
+    fn staging_dir_guard_removes_the_dir_on_drop() {
+        let tmp = tempfile::tempdir().unwrap();
+        let staging = tmp.path().join("tag.staging-4242");
+        std::fs::create_dir_all(&staging).unwrap();
+        std::fs::write(staging.join("memory.bin"), b"GBS-OF-GUEST-RAM").unwrap();
+        {
+            let _guard = StagingDirGuard(staging.clone());
+            assert!(staging.join("memory.bin").exists());
+        }
+        assert!(
+            !staging.exists(),
+            "dropping the guard must remove the staging dir and its contents"
+        );
+    }
+
+    /// Drop must tolerate a dir that is not there: the success path may
+    /// have removed it, and a stale sweep may have beaten the guard to it.
+    #[test]
+    fn staging_dir_guard_tolerates_a_missing_dir() {
+        let tmp = tempfile::tempdir().unwrap();
+        let staging = tmp.path().join("never-created.staging-4242");
+        {
+            let _guard = StagingDirGuard(staging.clone());
+        }
+        assert!(!staging.exists());
+    }
+
+    /// The free-space probe must work for a path that does not exist yet —
+    /// both bake gates run before the directory is created — and must
+    /// report the same filesystem as an existing path under it.
+    #[cfg(unix)]
+    #[test]
+    fn available_bytes_walks_up_to_an_existing_ancestor() {
+        let tmp = tempfile::tempdir().unwrap();
+        let deep = tmp.path().join("a/b/c");
+        let bytes = crate::doctor::available_bytes(&deep).expect("statvfs via an ancestor");
+        assert!(bytes > 0, "a real filesystem must report free bytes");
+        // Not assert_eq: parallel tests write to the same filesystem between
+        // the two probes, so allow drift rather than flake.
+        let ancestor = crate::doctor::available_bytes(tmp.path()).expect("statvfs");
+        assert!(
+            bytes.abs_diff(ancestor) < 1024 * 1024 * 1024,
+            "a missing descendant must resolve to the same filesystem as its ancestor \
+             ({bytes} vs {ancestor})"
+        );
+    }
+
     /// Review #295 r9 blocker 1: the rollback guard must restore the
     /// preserved previous rootfs and remove the partial clone when the
     /// bake fails (drop while armed), and must NOT touch anything when