From 60a253a2f6e9ff481aa93c0e6bc74a306281057f Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Tue, 14 Jul 2026 19:16:49 +0200
Subject: [PATCH] fix(#1763): address argus review on PR #2430

- regenerate docs/tools/hivectl-cli.md for the new `subvol snapshot send` verb
- close the TOCTOU on the no-overwrite guard: File::options().create_new(true)
  (O_CREAT|O_EXCL) instead of exists()-then-create, so the guarantee is
  atomic against a concurrent request racing the same dest filename
- warn (not silently swallow) if cleaning up a partial export after a
  failed btrfs send itself fails, so a stuck garbage file masquerading
  as a completed export is visible in the log
---
 docs/tools/hivectl-cli.md | 20 ++++++++++++++++++++
 hive-priv/src/main.rs     | 33 +++++++++++++++++++++++++--------
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/docs/tools/hivectl-cli.md b/docs/tools/hivectl-cli.md
index f2de9575..99825efd 100644
--- a/docs/tools/hivectl-cli.md
+++ b/docs/tools/hivectl-cli.md
@@ -41,6 +41,7 @@ This document contains the help content for the `hivectl` command-line program.
 * [`hivectl subvol snapshot`↴](#hivectl-subvol-snapshot)
 * [`hivectl subvol snapshot create`↴](#hivectl-subvol-snapshot-create)
 * [`hivectl subvol snapshot delete`↴](#hivectl-subvol-snapshot-delete)
+* [`hivectl subvol snapshot send`↴](#hivectl-subvol-snapshot-send)
 * [`hivectl open`↴](#hivectl-open)
 * [`hivectl completions`↴](#hivectl-completions)
 
@@ -581,6 +582,7 @@ Read-only snapshots of an agent's state subvolume — the first step of the (in-
 
 * `create` — Create a read-only snapshot. Agent must already be a subvolume (`subvol upgrade` first). Prints the snapshot's host path
 * `delete` — Delete a snapshot created by `subvol snapshot create`
+* `send` — Export a snapshot to a local file via `btrfs send` (the local-file half of inter-hive migration transport; the cross-hive `ssh ... btrfs receive` leg isn't wired up yet). Also useful standalone as a point-in-time backup: a full send with no `--parent` produces a self-contained archive of the snapshot
 
 
 
@@ -613,6 +615,24 @@ Delete a snapshot created by `subvol snapshot create`
 
 
 
+## `hivectl subvol snapshot send`
+
+Export a snapshot to a local file via `btrfs send` (the local-file half of inter-hive migration transport; the cross-hive `ssh ... btrfs receive` leg isn't wired up yet). Also useful standalone as a point-in-time backup: a full send with no `--parent` produces a self-contained archive of the snapshot
+
+**Usage:** `hivectl subvol snapshot send [OPTIONS] --dest <DEST> <NAME> <LABEL>`
+
+###### **Arguments:**
+
+* `<NAME>` — Agent name the snapshot belongs to
+* `<LABEL>` — Snapshot label passed to `subvol snapshot create --label`
+
+###### **Options:**
+
+* `--parent <PARENT>` — Optional parent snapshot label for an incremental send (`btrfs send -p`) — must be an existing, older snapshot of the same agent. Omit for a full send
+* `--dest <DEST>` — Destination filename (not a path) under the migrate-staging dir. Refused if it already exists
+
+
+
 ## `hivectl open`
 
 Print (and best-effort open in a browser) a hive web surface URL.
diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs
index ba7e256f..3b117275 100644
--- a/hive-priv/src/main.rs
+++ b/hive-priv/src/main.rs
@@ -1220,15 +1220,23 @@ async fn send_agent_snapshot_to_file(
     std::fs::create_dir_all(MIGRATE_STAGING_ROOT)
         .with_context(|| format!("create {MIGRATE_STAGING_ROOT}"))?;
     let dest = Path::new(MIGRATE_STAGING_ROOT).join(dest_file_name);
-    if dest.exists() {
-        bail!(
+    // `create_new` (O_CREAT|O_EXCL) makes the no-overwrite guarantee atomic
+    // instead of a check-then-create race against a concurrent request.
+    let dest_file = match std::fs::File::options()
+        .write(true)
+        .create_new(true)
+        .open(&dest)
+    {
+        Ok(f) => f,
+        Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => bail!(
             "{} already exists — pick a different destination or remove it first \
              (send never overwrites an existing export)",
             dest.display()
-        );
-    }
-    let dest_file =
-        std::fs::File::create(&dest).with_context(|| format!("create {}", dest.display()))?;
+        ),
+        Err(e) => {
+            return Err(e).with_context(|| format!("create {}", dest.display()));
+        }
+    };
 
     let mut cmd = Command::new("btrfs");
     cmd.arg("send");
@@ -1254,8 +1262,17 @@ async fn send_agent_snapshot_to_file(
         .with_context(|| format!("wait on btrfs send {}", snap.display()))?;
     if !out.status.success() {
         // Clean up a partial/failed export so a retry doesn't trip the
-        // "already exists" guard on garbage.
-        let _ = std::fs::remove_file(&dest);
+        // "already exists" guard on garbage. Best-effort: warn (don't fail
+        // the whole call over it) if removal itself fails, so a stuck
+        // partial file that later masquerades as a completed export is at
+        // least visible in the log.
+        if let Err(rm_err) = std::fs::remove_file(&dest) {
+            tracing::warn!(
+                dest = %dest.display(), error = %rm_err,
+                "failed to remove partial export after btrfs send failure — \
+                 next attempt at this dest will hit the already-exists guard"
+            );
+        }
         bail!(
             "btrfs send {} failed: {}",
             snap.display(),