From 8284a7355a2bb8cd25710e89cad162cc23fa2556 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Wed, 2 Sep 2026 08:26:08 +0200
Subject: [PATCH] hive-priv: a validator whose doc claimed a check it never
 performed

`validate_container_name`'s doc comment read "check it maps to a
hive-managed container". The body checked characters. Nothing in it
ever tested hive-managed-ness, so at the privsep boundary it asserted
a security property that was not there -- the kind of comment a future
reader relies on when deciding a name is already confined.

The confinement is real, it just lives elsewhere: all five callers
immediately apply `container_system_name`, so a caller naming
`hive-forge` reaches `h-hive-forge`, not the forge. That guarantee now
sits on `container_system_name`, which is the function that provides
it.

The `SIBLING_CONTAINERS` early-return in that validator was dead in
every path, not merely unused today: `validate_name_chars` accepts
`[a-z0-9-]`, so all three sibling names pass the fallthrough
identically -- both arms, same result. Infra containers reach the
lifecycle verbs through `ControlInfraContainer` and the
`InfraContainer` enum, which carries no free-form name at all, so no
sibling name reaches these functions. No test pinned the branch.

With the dead branch gone the function was character-for-character
`validate_agent_name`, so it is deleted and its five call sites use
that directly. `SIBLING_CONTAINERS` stays imported --
`validate_container_system_name` still needs it, and there the same
early-return IS load-bearing: without it an unprefixed sibling name
hits the bail.

No behaviour change. Gated: fmt, clippy -D warnings, 21 tests, rustdoc
under CI's docs-rustdoc lints (the two new intra-doc links resolve),
push-lints.
---
 hive-priv/src/main.rs | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs
index cdeac79b..571a7976 100644
--- a/hive-priv/src/main.rs
+++ b/hive-priv/src/main.rs
@@ -513,7 +513,7 @@ async fn exec(
 /// this itself). Best-effort — if the unit doesn't exist yet (first-time
 /// create), `reset-failed` is a no-op and the start proceeds regardless.
 async fn start_container(name: &str) -> Result<(String, String)> {
-    validate_container_name(name)?;
+    validate_agent_name(name)?;
     let machine = container_system_name(name);
     let _ = Command::new("systemctl")
         .args(["reset-failed", &format!("container@{machine}.service")])
@@ -587,7 +587,7 @@ fn write_extra_forge_account(
 
 /// `StopContainer`.
 async fn stop_container(name: &str) -> Result<(String, String)> {
-    validate_container_name(name)?;
+    validate_agent_name(name)?;
     stop_and_release(&container_system_name(name)).await
 }
 
@@ -596,14 +596,14 @@ async fn stop_container(name: &str) -> Result<(String, String)> {
 /// right semantics for a forced shutdown after a graceful stop has already
 /// been attempted.
 async fn kill_container(name: &str) -> Result<(String, String)> {
-    validate_container_name(name)?;
+    validate_agent_name(name)?;
     let machine = container_system_name(name);
     machinectl_run(&["kill", &machine, "--signal=SIGKILL"]).await
 }
 
 /// `DestroyContainer`.
 async fn destroy_container(name: &str) -> Result<(String, String)> {
-    validate_container_name(name)?;
+    validate_agent_name(name)?;
     container_run(&["destroy", &container_system_name(name)]).await
 }
 
@@ -765,7 +765,7 @@ async fn container_flake_action(
     stream: bool,
     writer: &mut OwnedWriteHalf,
 ) -> Result<(String, String)> {
-    validate_container_name(name)?;
+    validate_agent_name(name)?;
     // The build is the multi-minute phase of this operation — give it the
     // same live-line treatment `container_run_streaming` gives
     // `nixos-container` itself when the caller asked for it. Without
@@ -2722,6 +2722,13 @@ async fn sync_gateway_nginx() -> Result<(String, String)> {
 
 /// Return the system container name for a logical agent name.
 /// All agents (including the manager) use the `h-` prefix.
+///
+/// **This prefix is what confines the lifecycle verbs to agent containers** —
+/// not their name validation, which only checks characters. A caller cannot
+/// name `hive-forge` and reach it: it becomes `h-hive-forge`. Infra containers
+/// are reached through [`PrivRequest::ControlInfraContainer`] and the
+/// [`InfraContainer`] enum instead, which is why no lifecycle verb here takes
+/// a sibling name.
 fn container_system_name(name: &str) -> String {
     format!("{AGENT_PREFIX}{name}")
 }
@@ -2733,15 +2740,6 @@ fn validate_agent_name(name: &str) -> Result<()> {
     Ok(())
 }
 
-/// Validate a logical agent name and check it maps to a hive-managed container.
-fn validate_container_name(name: &str) -> Result<()> {
-    if SIBLING_CONTAINERS.contains(&name) {
-        return Ok(());
-    }
-    validate_name_chars(name)?;
-    Ok(())
-}
-
 /// Validate a system-level container name (already has `h-` prefix for
 /// all agents including the manager, or is a sibling service name).
 fn validate_container_system_name(name: &str) -> Result<()> {