From 8f8076b8edb8f7bae3cb0f28e6264bd6b27ec9db Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Tue, 14 Jul 2026 18:26:12 +0200
Subject: [PATCH] fix(#2391): mandatory hive- prefixed snapshot label, nest
 subvol snapshot create/delete
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per mara's PR review:
- snapshot label is now mandatory (was optional w/ timestamp default)
  and must start with "hive-" — hive-priv enforces this as an
  allow-list on top of the existing credential-name charset check, so
  only hivectl-issued labels can reach the btrfs shellout.
- nest under `subvol snapshot create`/`subvol snapshot delete`
  instead of othering delete as a separate top-level `delete-snapshot`
  verb.

Per argus's review:
- regenerate docs/tools/hivectl-cli.md (hivectl markdown-docs) to
  include the new subcommands — CI's hivectl-docs-fresh check compares
  this file against generated output.
---
 docs/tools/hivectl-cli.md    | 46 +++++++++++++++++++++++++
 hive-c0re/src/bin/hivectl.rs | 67 +++++++++++++++++++++---------------
 hive-priv/src/main.rs        | 18 ++++++++--
 hive-sh4re/src/priv_proto.rs |  7 ++--
 4 files changed, 106 insertions(+), 32 deletions(-)

diff --git a/docs/tools/hivectl-cli.md b/docs/tools/hivectl-cli.md
index 6c5b5d34..f2de9575 100644
--- a/docs/tools/hivectl-cli.md
+++ b/docs/tools/hivectl-cli.md
@@ -38,6 +38,9 @@ This document contains the help content for the `hivectl` command-line program.
 * [`hivectl quota limit`↴](#hivectl-quota-limit)
 * [`hivectl subvol`↴](#hivectl-subvol)
 * [`hivectl subvol upgrade`↴](#hivectl-subvol-upgrade)
+* [`hivectl subvol snapshot`↴](#hivectl-subvol-snapshot)
+* [`hivectl subvol snapshot create`↴](#hivectl-subvol-snapshot-create)
+* [`hivectl subvol snapshot delete`↴](#hivectl-subvol-snapshot-delete)
 * [`hivectl open`↴](#hivectl-open)
 * [`hivectl completions`↴](#hivectl-completions)
 
@@ -548,6 +551,7 @@ New agents get a btrfs subvolume state root automatically (when the host FS is b
 ###### **Subcommands:**
 
 * `upgrade` — Convert an existing plain-dir agent state root into a btrfs subvolume in place. Stops the agent (so its state bind-mount is released), migrates `…/agents/<name>/` to a subvolume preserving ownership/permissions/xattrs, then restarts it. Idempotent (no-op if already a subvolume) and safe (the original dir is left untouched on any failure before the final swap). Requires `--yes` since it bounces the agent and moves its state
+* `snapshot` — Read-only snapshots of an agent's state subvolume — the first step of the (in-progress) inter-hive migration path, or a manual point-in-time backup
 
 
 
@@ -567,6 +571,48 @@ Convert an existing plain-dir agent state root into a btrfs subvolume in place.
 
 
 
+## `hivectl subvol snapshot`
+
+Read-only snapshots of an agent's state subvolume — the first step of the (in-progress) inter-hive migration path, or a manual point-in-time backup
+
+**Usage:** `hivectl subvol snapshot <COMMAND>`
+
+###### **Subcommands:**
+
+* `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`
+
+
+
+## `hivectl subvol snapshot create`
+
+Create a read-only snapshot. Agent must already be a subvolume (`subvol upgrade` first). Prints the snapshot's host path
+
+**Usage:** `hivectl subvol snapshot create --label <LABEL> <NAME>`
+
+###### **Arguments:**
+
+* `<NAME>` — Agent name (e.g. `damocles`, `iris`)
+
+###### **Options:**
+
+* `--label <LABEL>` — Snapshot label. Mandatory, and must start with `hive-` — the prefix doubles as an allow-list hive-priv checks so only hivectl-issued snapshot names can reach the `btrfs subvolume snapshot` shellout
+
+
+
+## `hivectl subvol snapshot delete`
+
+Delete a snapshot created by `subvol snapshot create`
+
+**Usage:** `hivectl subvol snapshot delete <NAME> <LABEL>`
+
+###### **Arguments:**
+
+* `<NAME>` — Agent name the snapshot belongs to
+* `<LABEL>` — Snapshot label passed to `subvol snapshot create --label`
+
+
+
 ## `hivectl open`
 
 Print (and best-effort open in a browser) a hive web surface URL.
diff --git a/hive-c0re/src/bin/hivectl.rs b/hive-c0re/src/bin/hivectl.rs
index bdcc6c48..f3bbe5f7 100644
--- a/hive-c0re/src/bin/hivectl.rs
+++ b/hive-c0re/src/bin/hivectl.rs
@@ -620,22 +620,34 @@ enum SubvolCmd {
         #[arg(long)]
         yes: bool,
     },
-    /// Create a read-only snapshot of an agent's state subvolume — the
-    /// first step of the (in-progress) inter-hive migration path, or a
-    /// manual point-in-time backup. Agent must already be a subvolume
-    /// (`subvol upgrade` first). Prints the snapshot's host path.
+    /// Read-only snapshots of an agent's state subvolume — the first step
+    /// of the (in-progress) inter-hive migration path, or a manual
+    /// point-in-time backup.
     Snapshot {
+        #[command(subcommand)]
+        cmd: SnapshotCmd,
+    },
+}
+
+#[derive(Subcommand)]
+enum SnapshotCmd {
+    /// Create a read-only snapshot. Agent must already be a subvolume
+    /// (`subvol upgrade` first). Prints the snapshot's host path.
+    Create {
         /// Agent name (e.g. `damocles`, `iris`).
         name: String,
-        /// Snapshot label (`[A-Za-z0-9_.-]`); defaults to a UTC timestamp.
+        /// Snapshot label. Mandatory, and must start with `hive-` — the
+        /// prefix doubles as an allow-list hive-priv checks so only
+        /// hivectl-issued snapshot names can reach the `btrfs subvolume
+        /// snapshot` shellout.
         #[arg(long)]
-        label: Option<String>,
+        label: String,
     },
-    /// Delete a snapshot created by `subvol snapshot`.
-    DeleteSnapshot {
+    /// Delete a snapshot created by `subvol snapshot create`.
+    Delete {
         /// Agent name the snapshot belongs to.
         name: String,
-        /// Snapshot label passed to `subvol snapshot --label`.
+        /// Snapshot label passed to `subvol snapshot create --label`.
         label: String,
     },
 }
@@ -711,10 +723,10 @@ async fn main() -> Result<()> {
         Cmd::Restart { scope, graceful } => restart(&socket, scope.to_scope(), graceful).await,
         Cmd::Subvol { cmd } => match cmd {
             SubvolCmd::Upgrade { name, yes } => subvol_upgrade(&socket, &name, yes).await,
-            SubvolCmd::Snapshot { name, label } => subvol_snapshot(&name, label).await,
-            SubvolCmd::DeleteSnapshot { name, label } => {
-                subvol_delete_snapshot(&name, &label).await
-            }
+            SubvolCmd::Snapshot { cmd } => match cmd {
+                SnapshotCmd::Create { name, label } => subvol_snapshot_create(&name, label).await,
+                SnapshotCmd::Delete { name, label } => subvol_snapshot_delete(&name, &label).await,
+            },
         },
         Cmd::Choom {
             name,
@@ -1725,21 +1737,20 @@ async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> {
     Ok(())
 }
 
-/// `subvol snapshot <agent> [--label <label>]` — create a read-only btrfs
-/// snapshot of an agent's state subvolume. Unlike `upgrade`, this does NOT
-/// stop the agent: btrfs snapshots are atomic + consistent to take against a
-/// live subvolume. Default label is a unix-timestamp so repeated calls don't
-/// collide without the caller having to think of a name.
-async fn subvol_snapshot(name: &str, label: Option<String>) -> Result<()> {
+/// `subvol snapshot create <agent> --label <label>` — create a read-only
+/// btrfs snapshot of an agent's state subvolume. Unlike `upgrade`, this does
+/// NOT stop the agent: btrfs snapshots are atomic + consistent to take
+/// against a live subvolume. `label` is mandatory and must start with
+/// `hive-` — hive-priv enforces the same prefix as an allow-list, so this
+/// check is belt-and-suspenders (fail fast client-side with a clear
+/// message).
+async fn subvol_snapshot_create(name: &str, label: String) -> Result<()> {
     if !agent_exists(name)? {
         bail!("no agent named {name:?} (no state dir under the agents root)");
     }
-    let label = label.unwrap_or_else(|| {
-        let secs = std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .map_or(0, |d| d.as_secs());
-        format!("snap-{secs}")
-    });
+    if !label.starts_with("hive-") {
+        bail!("snapshot label {label:?} must start with \"hive-\"");
+    }
     let path = hive_c0re::priv_client::snapshot_agent_subvolume(name, &label)
         .await
         .with_context(|| format!("snapshot {name} state subvolume (label {label:?})"))?;
@@ -1747,9 +1758,9 @@ async fn subvol_snapshot(name: &str, label: Option<String>) -> Result<()> {
     Ok(())
 }
 
-/// `subvol delete-snapshot <agent> <label>` — remove a snapshot created by
-/// `subvol snapshot`.
-async fn subvol_delete_snapshot(name: &str, label: &str) -> Result<()> {
+/// `subvol snapshot delete <agent> <label>` — remove a snapshot created by
+/// `subvol snapshot create`.
+async fn subvol_snapshot_delete(name: &str, label: &str) -> Result<()> {
     hive_c0re::priv_client::delete_agent_snapshot(name, label)
         .await
         .with_context(|| format!("delete {name} snapshot (label {label:?})"))?;
diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs
index da280759..453108ce 100644
--- a/hive-priv/src/main.rs
+++ b/hive-priv/src/main.rs
@@ -370,7 +370,7 @@ async fn exec(req: PrivRequest, writer: &mut OwnedWriteHalf) -> Result<(String,
             ref snapshot_name,
         } => {
             validate_agent_name(agent_name)?;
-            validate_credential_name(snapshot_name)?;
+            validate_snapshot_name(snapshot_name)?;
             snapshot_agent_subvolume(agent_name, snapshot_name).await
         }
 
@@ -379,7 +379,7 @@ async fn exec(req: PrivRequest, writer: &mut OwnedWriteHalf) -> Result<(String,
             ref snapshot_name,
         } => {
             validate_agent_name(agent_name)?;
-            validate_credential_name(snapshot_name)?;
+            validate_snapshot_name(snapshot_name)?;
             delete_agent_snapshot(agent_name, snapshot_name).await
         }
 
@@ -430,6 +430,20 @@ fn handle_write_nspawn_flags(
     Ok((String::new(), String::new()))
 }
 
+/// A btrfs snapshot label must start with `hive-` — this doubles as an
+/// allow-list: only names hivectl itself constructs (or an operator who
+/// knows the convention) can reach the `btrfs subvolume snapshot`/`delete`
+/// shellouts, so an arbitrary caller can't use the snapshot ops to probe or
+/// churn unrelated paths under `AGENT_STATE_ROOT`. Beyond the prefix, the
+/// same charset restriction as [`validate_credential_name`] applies (it's
+/// interpolated straight into a filesystem path).
+fn validate_snapshot_name(name: &str) -> Result<()> {
+    if !name.starts_with("hive-") {
+        bail!("invalid snapshot label {name:?}: must start with \"hive-\"");
+    }
+    validate_credential_name(name)
+}
+
 /// A systemd credential id must be a short token — restrict to
 /// `[A-Za-z0-9_.-]` so it can't inject extra `--load-credential` argv or
 /// break the `name:path` shape.
diff --git a/hive-sh4re/src/priv_proto.rs b/hive-sh4re/src/priv_proto.rs
index 27719a69..098d3e89 100644
--- a/hive-sh4re/src/priv_proto.rs
+++ b/hive-sh4re/src/priv_proto.rs
@@ -595,8 +595,11 @@ pub enum PrivRequest {
     SnapshotAgentSubvolume {
         /// Logical agent name (validated by `validate_agent_name`).
         agent_name: String,
-        /// Snapshot label (validated like a credential name:
-        /// non-empty `[A-Za-z0-9_.-]`); becomes part of the snapshot path.
+        /// Snapshot label. Must start with `hive-` — the prefix doubles as
+        /// an allow-list hive-priv enforces so only hivectl-issued names
+        /// can reach the `btrfs subvolume snapshot` shellout — and
+        /// otherwise follows the same charset as a credential name
+        /// (non-empty `[A-Za-z0-9_.-]`); becomes part of the snapshot path.
         snapshot_name: String,
     },