diff --git a/hive-c0re/src/agent_server.rs b/hive-c0re/src/agent_server.rs index f59af5ce..8db42dfd 100644 --- a/hive-c0re/src/agent_server.rs +++ b/hive-c0re/src/agent_server.rs @@ -535,10 +535,11 @@ async fn handle_restart_child(coord: &Arc, agent: &str, name: &str) // Infra-container restart: an agent holding the `infra_admin` // capability can restart a hive infrastructure container (hive-ci / // hive-gateway / hive-forge / hive-matrix) by passing its name to the - // same restart tool. These names are never agent children, so this - // branch is disjoint from the child-restart path below. - if hive_sh4re::priv_proto::SIBLING_CONTAINERS.contains(&name) { - return handle_restart_infra(coord, agent, name).await; + // same restart tool. The `InfraContainer` enum parse both recognises + // these (never agent children, so disjoint from the child path below) + // and yields the typed value the restart path needs. + if let Ok(container) = name.parse::() { + return handle_restart_infra(coord, agent, container).await; } if let Some(err) = require_child(agent, name, "restart") { return err; @@ -556,15 +557,16 @@ async fn handle_restart_child(coord: &Arc, agent: &str, name: &str) } /// Restart a hive infrastructure container on behalf of an agent that -/// holds the `infra_admin` capability. The container name is already -/// known to be in `SIBLING_CONTAINERS`; this gates on the -/// capability and routes the systemctl restart through hive-priv (which -/// re-validates the name root-side). Direct, not approval-gated. +/// holds the `infra_admin` capability. The `container` is already a valid +/// [`InfraContainer`] (the caller parsed it); this gates on the capability +/// and routes the systemctl restart through hive-priv. Direct, not +/// approval-gated. async fn handle_restart_infra( coord: &Arc, agent: &str, - container: &str, + container: hive_sh4re::priv_proto::InfraContainer, ) -> AgentResponse { + let name = container.unit_name(); // Record the attempt in the operator-visible privileged-action audit // trail, then emit a live `AuditEntryAdded` so the dashboard audit view // appends it off `/dashboard/stream`. Best-effort: `record` returns the @@ -572,27 +574,26 @@ async fn handle_restart_infra( // row so the stored + streamed views can't drift. `action` is stable so // the dashboard can group/filter. let audit = |outcome: crate::audit_log::AuditOutcome, detail: Option<&str>| { - if let Some(entry) = - coord - .audit_log - .record(agent, "restart_infra", container, outcome, detail) + if let Some(entry) = coord + .audit_log + .record(agent, "restart_infra", name, outcome, detail) { coord.emit_audit_entry(entry); } }; if !crate::capabilities::has_cap(agent, hive_sh4re::Capability::InfraAdmin) { - tracing::warn!(%agent, %container, "agent: infra restart denied (no infra_admin capability)"); + tracing::warn!(%agent, %name, "agent: infra restart denied (no infra_admin capability)"); audit( crate::audit_log::AuditOutcome::Err, Some("denied: missing infra_admin capability"), ); return AgentResponse::Err { message: format!( - "restarting infra container `{container}` requires the `infra_admin` capability" + "restarting infra container `{name}` requires the `infra_admin` capability" ), }; } - tracing::info!(%agent, %container, "agent: restart infra container"); + tracing::info!(%agent, %name, "agent: restart infra container"); match crate::priv_client::restart_infra_container(container).await { Ok(()) => { audit(crate::audit_log::AuditOutcome::Ok, None); diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index 55c1cf58..87c92472 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -8,8 +8,8 @@ use anyhow::{Context as _, Result, bail}; use hive_sh4re::priv_proto::{ - BindMount, InfraAction, JournalQuery, NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, - PrivResponse, PrivStream, + BindMount, InfraAction, InfraContainer, JournalQuery, NetworkIsolation, PRIV_SOCK, PrivEvent, + PrivRequest, PrivResponse, PrivStream, }; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixStream; @@ -288,21 +288,17 @@ pub async fn restart_matrix_daemon(agent_name: &str) -> Result<()> { /// re-validates `container` against its root-side allowlist; callers must /// already have checked the requesting agent holds the `infra_admin` /// capability. -pub async fn restart_infra_container(container: &str) -> Result<()> { +pub async fn restart_infra_container(container: InfraContainer) -> Result<()> { control_infra_container(container, InfraAction::Restart).await } /// Start / stop / restart a hive infrastructure container (`hive-ci`, /// `hive-gateway`, `hive-forge`, `hive-matrix`) on the host via `systemctl -/// container@.service`. hive-priv re-validates -/// `container` against its root-side allowlist (`SIBLING_CONTAINERS`). Used -/// by the hive-wide `hivectl stop` / `hivectl start` flow. -pub async fn control_infra_container(container: &str, action: InfraAction) -> Result<()> { - ok(call(&PrivRequest::ControlInfraContainer { - container: container.to_owned(), - action, - }) - .await?) +/// container@.service`. The [`InfraContainer`] enum is +/// the allowlist — hive-priv needs no name re-validation. Used by the +/// hive-wide `hivectl stop` / `hivectl start` flow. +pub async fn control_infra_container(container: InfraContainer, action: InfraAction) -> Result<()> { + ok(call(&PrivRequest::ControlInfraContainer { container, action }).await?) } /// Ensure the agent's persistent state root is a btrfs subvolume when the diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index 10ce93f7..8fc56bc5 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -2,7 +2,7 @@ use std::path::Path; use std::sync::Arc; use anyhow::{Context, Result}; -use hive_sh4re::priv_proto::InfraAction; +use hive_sh4re::priv_proto::{InfraAction, InfraContainer}; use hive_sh4re::{HostRequest, HostResponse, LifecycleScope}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::{UnixListener, UnixStream}; @@ -229,7 +229,7 @@ async fn handle_restart_all() -> Result { async fn handle_stop( coord: &Arc, agents: &[String], - infra: &[&str], + infra: &[InfraContainer], graceful: bool, ) -> Result { tracing::info!(?agents, ?infra, graceful, "stop"); @@ -267,11 +267,12 @@ async fn handle_stop( } for &container in infra { + let name = container.unit_name(); match crate::priv_client::control_infra_container(container, InfraAction::Stop).await { - Ok(()) => ok_items.push(container.to_owned()), + Ok(()) => ok_items.push(name.to_owned()), Err(e) => { - tracing::warn!(%container, error = ?e, "stop: infra stop failed"); - errors.push(format!("{container}: {e:#}")); + tracing::warn!(%name, error = ?e, "stop: infra stop failed"); + errors.push(format!("{name}: {e:#}")); } } } @@ -283,17 +284,18 @@ async fn handle_stop( /// inverse of [`handle_stop`]. Infra comes up before agents so the agents /// find forge/matrix/gateway ready. Per-target failures aggregated. Callers /// resolve the [`LifecycleScope`] to these explicit name lists up front. -async fn handle_start(agents: &[String], infra: &[&str]) -> Result { +async fn handle_start(agents: &[String], infra: &[InfraContainer]) -> Result { tracing::info!(?agents, ?infra, "start"); let mut ok_items: Vec = Vec::new(); let mut errors: Vec = Vec::new(); for &container in infra { + let name = container.unit_name(); match crate::priv_client::control_infra_container(container, InfraAction::Start).await { - Ok(()) => ok_items.push(container.to_owned()), + Ok(()) => ok_items.push(name.to_owned()), Err(e) => { - tracing::warn!(%container, error = ?e, "start: infra start failed"); - errors.push(format!("{container}: {e:#}")); + tracing::warn!(%name, error = ?e, "start: infra start failed"); + errors.push(format!("{name}: {e:#}")); } } } @@ -333,23 +335,23 @@ async fn scoped_agents(scope: &LifecycleScope) -> Result> { Ok(set.into_iter().collect()) } -/// Resolve which infra container names a scope targets. An "everything" -/// scope (no flags set) selects all controllable infra; otherwise each set -/// flag maps to its container. Fixed order for deterministic output. -fn scoped_infra(scope: &LifecycleScope) -> Vec<&'static str> { +/// Resolve which infra containers a scope targets. An "everything" scope +/// (no flags set) selects all controllable infra; otherwise each set flag +/// maps to its [`InfraContainer`]. Fixed order for deterministic output. +fn scoped_infra(scope: &LifecycleScope) -> Vec { let everything = scope.is_everything(); let mut out = Vec::new(); if everything || scope.ci { - out.push("hive-ci"); + out.push(InfraContainer::Ci); } if everything || scope.forge { - out.push("hive-forge"); + out.push(InfraContainer::Forge); } if everything || scope.gateway { - out.push("hive-gateway"); + out.push(InfraContainer::Gateway); } if everything || scope.matrix { - out.push("hive-matrix"); + out.push(InfraContainer::Matrix); } out } diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index 012af4d5..53403394 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -21,9 +21,9 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; use hive_sh4re::priv_proto::{ - AGENT_PREFIX, AGENT_STATE_ROOT, BindMount, InfraAction, JournalQuery, MANAGER_NAME, META_DIR, - NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream, PrivStreamLine, - SIBLING_CONTAINERS, + AGENT_PREFIX, AGENT_STATE_ROOT, BindMount, InfraAction, InfraContainer, JournalQuery, + MANAGER_NAME, META_DIR, NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, + PrivStream, PrivStreamLine, SIBLING_CONTAINERS, }; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::unix::OwnedWriteHalf; @@ -268,10 +268,9 @@ async fn exec(req: PrivRequest, writer: &mut OwnedWriteHalf) -> Result<(String, restart_matrix_daemon(agent_name).await } - PrivRequest::ControlInfraContainer { - ref container, - action, - } => control_infra_container(container, action).await, + PrivRequest::ControlInfraContainer { container, action } => { + control_infra_container(container, action).await + } PrivRequest::EnsureAgentSubvolume { ref agent_name } => { validate_agent_name(agent_name)?; @@ -430,17 +429,17 @@ async fn restart_matrix_daemon(agent_name: &str) -> Result<(String, String)> { /// `ControlInfraContainer` — start/stop/restart a hive infrastructure /// container via `systemctl container@.service`. The -/// `container` is validated against `SIBLING_CONTAINERS` here, root-side; -/// this is the authoritative allowlist (hive-c0re is never in it, so a -/// stop can't sever the daemon socket the request arrived on). Serves both -/// the hive-wide `hivectl stop`/`start` flow and an `infra_admin` agent's -/// `restart` (action = Restart). -async fn control_infra_container(container: &str, action: InfraAction) -> Result<(String, String)> { - if !SIBLING_CONTAINERS.contains(&container) { - bail!("container {container:?} is not a controllable hive infra container"); - } +/// [`InfraContainer`] enum is the allowlist: serde already rejected any +/// unknown / unsafe name (hive-c0re has no variant, so a stop can't sever +/// the daemon socket) at deserialisation, so no root-side `.contains()` +/// check is needed here. Serves both the hive-wide `hivectl stop`/`start` +/// flow and an `infra_admin` agent's `restart` (action = Restart). +async fn control_infra_container( + container: InfraContainer, + action: InfraAction, +) -> Result<(String, String)> { let verb = action.systemctl_verb(); - let unit = format!("container@{container}.service"); + let unit = format!("container@{}.service", container.unit_name()); let out = Command::new("systemctl") .args([verb, &unit]) .output() diff --git a/hive-sh4re/src/priv_proto.rs b/hive-sh4re/src/priv_proto.rs index 48ef4d1d..8dba0c27 100644 --- a/hive-sh4re/src/priv_proto.rs +++ b/hive-sh4re/src/priv_proto.rs @@ -46,6 +46,59 @@ impl InfraAction { } } +/// A hive infrastructure container that can be controlled (start / stop / +/// restart) via [`PrivRequest::ControlInfraContainer`]. The variants ARE +/// the allowlist: serde rejects any other value at the wire boundary, so an +/// unknown or unsafe target — notably `hive-c0re`, which has no variant and +/// would sever the daemon socket — is *unrepresentable* rather than caught +/// by a runtime check. The c0re↔hive-priv wire form uses serde's default +/// variant naming (`"Ci"`, `"Forge"`, …); it's an internal protocol (both +/// ends rebuild together) so it needn't match the container name. +/// [`unit_name`](Self::unit_name) is the separate systemd / container name +/// (`hive-ci`). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum InfraContainer { + Ci, + Forge, + Gateway, + Matrix, +} + +impl InfraContainer { + /// Every controllable infra container. The source of truth that + /// [`SIBLING_CONTAINERS`] is kept consistent with (see the test). + pub const ALL: [InfraContainer; 4] = [ + InfraContainer::Ci, + InfraContainer::Forge, + InfraContainer::Gateway, + InfraContainer::Matrix, + ]; + + /// The container / systemd-unit name, e.g. `hive-ci` → + /// `container@hive-ci.service`. (Distinct from the serde wire form, + /// which is the default variant name `"Ci"`.) + #[must_use] + pub fn unit_name(self) -> &'static str { + match self { + InfraContainer::Ci => "hive-ci", + InfraContainer::Forge => "hive-forge", + InfraContainer::Gateway => "hive-gateway", + InfraContainer::Matrix => "hive-matrix", + } + } +} + +impl std::str::FromStr for InfraContainer { + type Err = (); + + /// Parse a container name (`hive-ci`, …) into a variant. Used to decide + /// whether an MCP `restart()` target is a controllable infra + /// container. `Err(())` for anything that isn't one. + fn from_str(s: &str) -> Result { + Self::ALL.into_iter().find(|c| c.unit_name() == s).ok_or(()) + } +} + /// Host path of the meta flake. The flake ref for agent `` is /// `{META_DIR}#{name}`, derived by `hive-priv` — never passed over the wire. pub const META_DIR: &str = "/var/lib/hyperhive/meta"; @@ -325,15 +378,14 @@ pub enum PrivRequest { }, /// Start / stop / restart a hive infrastructure container on the host - /// via `systemctl container@.service`. hive-priv - /// validates `container` against [`SIBLING_CONTAINERS`] root-side (the - /// authoritative allowlist; `hive-c0re` is never in it). Serves both the - /// hive-wide `hivectl stop` / `hivectl start` flow and an `infra_admin` - /// agent's `restart` (with `action = Restart`). + /// via `systemctl container@.service`. The + /// [`InfraContainer`] enum is the allowlist — serde rejects unknown / + /// unsafe names (notably `hive-c0re`, which has no variant) at the wire + /// boundary, so no root-side `.contains()` check is needed. Serves both + /// the hive-wide `hivectl stop` / `hivectl start` flow and an + /// `infra_admin` agent's `restart` (with `action = Restart`). ControlInfraContainer { - /// Infra container name (e.g. `hive-ci`); must be in - /// [`SIBLING_CONTAINERS`]. - container: String, + container: InfraContainer, action: InfraAction, }, @@ -486,7 +538,7 @@ pub enum PrivEvent { #[cfg(test)] mod tests { - use super::SIBLING_CONTAINERS; + use super::{InfraContainer, SIBLING_CONTAINERS}; #[test] fn infra_control_allowlist_excludes_c0re_includes_matrix() { @@ -497,4 +549,29 @@ mod tests { // hive-matrix IS controllable (operator can stop/start/restart it). assert!(SIBLING_CONTAINERS.contains(&"hive-matrix")); } + + #[test] + fn infra_container_enum_matches_sibling_containers() { + // The InfraContainer enum (the control-path allowlist) and the + // SIBLING_CONTAINERS slice (the general container-name validator) + // must list exactly the same four containers — they're separate + // surfaces for the same set, so keep them in lockstep. + let mut from_enum: Vec<&str> = InfraContainer::ALL.iter().map(|c| c.unit_name()).collect(); + from_enum.sort_unstable(); + let mut from_slice: Vec<&str> = SIBLING_CONTAINERS.to_vec(); + from_slice.sort_unstable(); + assert_eq!(from_enum, from_slice); + // hive-c0re has no variant — unrepresentable, can't be controlled. + assert!("hive-c0re".parse::().is_err()); + } + + #[test] + fn infra_container_name_round_trips() { + // `unit_name` is the single source of truth for the wire form (the + // serde impls + FromStr all key off it), so a name→variant→name + // round-trip proves the mapping is consistent in both directions. + for c in InfraContainer::ALL { + assert_eq!(c.unit_name().parse::(), Ok(c)); + } + } }