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.
This commit is contained in:
atlas 2026-09-02 08:26:08 +02:00 committed by mara
commit 8284a7355a

View file

@ -513,7 +513,7 @@ async fn exec(
/// this itself). Best-effort — if the unit doesn't exist yet (first-time /// 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. /// create), `reset-failed` is a no-op and the start proceeds regardless.
async fn start_container(name: &str) -> Result<(String, String)> { async fn start_container(name: &str) -> Result<(String, String)> {
validate_container_name(name)?; validate_agent_name(name)?;
let machine = container_system_name(name); let machine = container_system_name(name);
let _ = Command::new("systemctl") let _ = Command::new("systemctl")
.args(["reset-failed", &format!("container@{machine}.service")]) .args(["reset-failed", &format!("container@{machine}.service")])
@ -587,7 +587,7 @@ fn write_extra_forge_account(
/// `StopContainer`. /// `StopContainer`.
async fn stop_container(name: &str) -> Result<(String, String)> { 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 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 /// right semantics for a forced shutdown after a graceful stop has already
/// been attempted. /// been attempted.
async fn kill_container(name: &str) -> Result<(String, String)> { async fn kill_container(name: &str) -> Result<(String, String)> {
validate_container_name(name)?; validate_agent_name(name)?;
let machine = container_system_name(name); let machine = container_system_name(name);
machinectl_run(&["kill", &machine, "--signal=SIGKILL"]).await machinectl_run(&["kill", &machine, "--signal=SIGKILL"]).await
} }
/// `DestroyContainer`. /// `DestroyContainer`.
async fn destroy_container(name: &str) -> Result<(String, String)> { 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 container_run(&["destroy", &container_system_name(name)]).await
} }
@ -765,7 +765,7 @@ async fn container_flake_action(
stream: bool, stream: bool,
writer: &mut OwnedWriteHalf, writer: &mut OwnedWriteHalf,
) -> Result<(String, String)> { ) -> Result<(String, String)> {
validate_container_name(name)?; validate_agent_name(name)?;
// The build is the multi-minute phase of this operation — give it the // The build is the multi-minute phase of this operation — give it the
// same live-line treatment `container_run_streaming` gives // same live-line treatment `container_run_streaming` gives
// `nixos-container` itself when the caller asked for it. Without // `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. /// Return the system container name for a logical agent name.
/// All agents (including the manager) use the `h-` prefix. /// 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 { fn container_system_name(name: &str) -> String {
format!("{AGENT_PREFIX}{name}") format!("{AGENT_PREFIX}{name}")
} }
@ -2733,15 +2740,6 @@ fn validate_agent_name(name: &str) -> Result<()> {
Ok(()) 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 /// Validate a system-level container name (already has `h-` prefix for
/// all agents including the manager, or is a sibling service name). /// all agents including the manager, or is a sibling service name).
fn validate_container_system_name(name: &str) -> Result<()> { fn validate_container_system_name(name: &str) -> Result<()> {