From 0df9e4094096aea59d23f6cc305d3989ff799399 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 19 Jun 2026 00:50:00 +0200 Subject: [PATCH] hivectl: collapse infra allowlist + restart/control ops onto SIBLING_CONTAINERS Per review: RESTARTABLE_INFRA_CONTAINERS and the new CONTROLLABLE_INFRA_CONTAINERS were near-identical subsets of SIBLING_CONTAINERS. Drop both and validate infra lifecycle ops against SIBLING_CONTAINERS directly (all four infra containers; hive-c0re is never in it, so it can't stop itself). This also makes hive-matrix restartable, including via an infra_admin agent's restart tool. Collapse the two priv ops too: RestartInfraContainer is gone; ControlInfraContainer { action } is the single op (restart = action: Restart). priv_client's restart_infra_container is now a thin wrapper over control_infra_container. --- hive-c0re/src/agent_server.rs | 10 ++--- hive-c0re/src/priv_client.rs | 20 ++++------ hive-priv/src/main.rs | 50 +++++------------------ hive-sh4re/src/priv_proto.rs | 75 +++++++++++------------------------ 4 files changed, 46 insertions(+), 109 deletions(-) diff --git a/hive-c0re/src/agent_server.rs b/hive-c0re/src/agent_server.rs index 68d00ccd..2808fe70 100644 --- a/hive-c0re/src/agent_server.rs +++ b/hive-c0re/src/agent_server.rs @@ -518,10 +518,10 @@ async fn handle_start_child(coord: &Arc, agent: &str, name: &str) - async fn handle_restart_child(coord: &Arc, agent: &str, name: &str) -> AgentResponse { // Infra-container restart: an agent holding the `infra_admin` // capability can restart a hive infrastructure container (hive-ci / - // hive-gateway / hive-forge) 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::RESTARTABLE_INFRA_CONTAINERS.contains(&name) { + // 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; } if let Some(err) = require_child(agent, name, "restart") { @@ -541,7 +541,7 @@ 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 `RESTARTABLE_INFRA_CONTAINERS`; this gates on the +/// 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. async fn handle_restart_infra( diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index 4993d0eb..b6a25b5f 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -283,24 +283,20 @@ pub async fn restart_matrix_daemon(agent_name: &str) -> Result<()> { .await?) } -/// Restart a hive infrastructure container (hive-ci / hive-gateway / -/// hive-forge) on the host via `systemctl restart -/// container@.service`. hive-priv re-validates `container` -/// against its root-side allowlist; callers must already have checked -/// the requesting agent holds the `infra_admin` capability. +/// Restart a hive infrastructure container on the host (thin wrapper over +/// [`control_infra_container`] with `action = Restart`). hive-priv +/// 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<()> { - ok(call(&PrivRequest::RestartInfraContainer { - container: container.to_owned(), - }) - .await?) + 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 -/// (`CONTROLLABLE_INFRA_CONTAINERS`). Used by the hive-wide `hivectl stop` / -/// `hivectl start` flow. +/// `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(), diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index e3a0515b..47e9cce1 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, CONTROLLABLE_INFRA_CONTAINERS, InfraAction, - JournalQuery, MANAGER_NAME, META_DIR, NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, - PrivResponse, PrivStream, PrivStreamLine, RESTARTABLE_INFRA_CONTAINERS, SIBLING_CONTAINERS, + AGENT_PREFIX, AGENT_STATE_ROOT, BindMount, InfraAction, 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; @@ -265,10 +265,6 @@ async fn exec(req: PrivRequest, writer: &mut OwnedWriteHalf) -> Result<(String, restart_matrix_daemon(agent_name).await } - PrivRequest::RestartInfraContainer { ref container } => { - restart_infra_container(container).await - } - PrivRequest::ControlInfraContainer { ref container, action, @@ -399,43 +395,15 @@ async fn restart_matrix_daemon(agent_name: &str) -> Result<(String, String)> { )) } -/// `RestartInfraContainer` — restart a hive infrastructure container on -/// the host via `systemctl restart container@.service`. The -/// `container` is validated against `RESTARTABLE_INFRA_CONTAINERS` here, -/// root-side, so this is the authoritative allowlist even though -/// hive-c0re also gates on the caller's `infra_admin` capability. -async fn restart_infra_container(container: &str) -> Result<(String, String)> { - if !RESTARTABLE_INFRA_CONTAINERS.contains(&container) { - bail!("container {container:?} is not a restartable hive infra container"); - } - let unit = format!("container@{container}.service"); - let out = Command::new("systemctl") - .args(["restart", &unit]) - .output() - .await - .with_context(|| format!("systemctl restart {unit}"))?; - if !out.status.success() { - bail!( - "systemctl restart {unit} exited {}: {}", - out.status, - String::from_utf8_lossy(&out.stderr).trim() - ); - } - tracing::info!(target: "infra-restart", "restarted {unit}"); - Ok(( - String::from_utf8_lossy(&out.stdout).into_owned(), - String::from_utf8_lossy(&out.stderr).into_owned(), - )) -} - /// `ControlInfraContainer` — start/stop/restart a hive infrastructure /// container via `systemctl container@.service`. The -/// `container` is validated against `CONTROLLABLE_INFRA_CONTAINERS` here, -/// root-side; this is the authoritative allowlist (hive-c0re itself can -/// never appear in it, so a hive-wide stop can't sever the daemon socket -/// the request arrived on). +/// `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 !CONTROLLABLE_INFRA_CONTAINERS.contains(&container) { + if !SIBLING_CONTAINERS.contains(&container) { bail!("container {container:?} is not a controllable hive infra container"); } let verb = action.systemctl_verb(); diff --git a/hive-sh4re/src/priv_proto.rs b/hive-sh4re/src/priv_proto.rs index 8265e4ff..1b4a3e38 100644 --- a/hive-sh4re/src/priv_proto.rs +++ b/hive-sh4re/src/priv_proto.rs @@ -15,28 +15,16 @@ pub const MANAGER_NAME: &str = "ruth"; /// Sub-agent container prefix. System container name = `h-`. pub const AGENT_PREFIX: &str = "h-"; -/// Sibling service containers managed by hive-c0re. +/// Sibling service containers managed by hive-c0re. This doubles as the +/// authoritative allowlist for infra lifecycle ops +/// ([`PrivRequest::ControlInfraContainer`]): any of these four may be +/// started / stopped / restarted (by the hive-wide `hivectl stop`/`start` +/// flow or an `infra_admin` agent's `restart`). `hive-c0re` is deliberately +/// absent — stopping it would sever the very socket the request arrived on. +/// hive-priv re-validates against this list root-side, so it's authoritative +/// regardless of what the caller sends. pub const SIBLING_CONTAINERS: &[&str] = &["hive-forge", "hive-matrix", "hive-gateway", "hive-ci"]; -/// Infra containers an agent holding the `infra_admin` capability may -/// restart via the `restart` MCP tool. A deliberate subset of -/// [`SIBLING_CONTAINERS`]: hive-matrix is excluded (kicking the matrix -/// backend mid-sync is its own concern) and hive-c0re is excluded -/// entirely (a self-restart would sever the very socket the request -/// arrived on). hive-priv re-validates against this list root-side, so -/// it is the authoritative allowlist regardless of what the caller sends. -pub const RESTARTABLE_INFRA_CONTAINERS: &[&str] = &["hive-ci", "hive-gateway", "hive-forge"]; - -/// Infra containers hive-c0re may stop/start/restart hive-wide for the -/// `hivectl stop` / `hivectl start` operator flow. Superset of -/// [`RESTARTABLE_INFRA_CONTAINERS`]: it adds `hive-matrix`, because a full -/// stop is a deliberate operator action (unlike the disruptive mid-sync -/// *restart* the `infra_admin` MCP path forbids). `hive-c0re` is still -/// excluded — it runs the daemon servicing the request and must never stop -/// itself. hive-priv re-validates against this list root-side. -pub const CONTROLLABLE_INFRA_CONTAINERS: &[&str] = - &["hive-ci", "hive-gateway", "hive-forge", "hive-matrix"]; - /// Lifecycle verb for [`PrivRequest::ControlInfraContainer`]. Maps directly /// to `systemctl container@.service`. #[derive(Debug, Clone, Copy, Serialize, Deserialize)] @@ -336,24 +324,15 @@ pub enum PrivRequest { agent_name: String, }, - /// Restart a hive infrastructure container on the host via - /// `systemctl restart container@.service`. hive-priv - /// validates `container` against [`RESTARTABLE_INFRA_CONTAINERS`] - /// before acting — the root-side allowlist is authoritative. Used - /// by hive-c0re to service a `restart` request from an agent that - /// holds the `infra_admin` capability. - RestartInfraContainer { - /// Infra container name (e.g. `hive-ci`); must be in - /// [`RESTARTABLE_INFRA_CONTAINERS`]. - container: String, - }, - - /// Start/stop/restart a hive infrastructure container on the host via - /// `systemctl container@.service`. hive-priv - /// validates `container` against [`CONTROLLABLE_INFRA_CONTAINERS`] - /// root-side. Generalises [`PrivRequest::RestartInfraContainer`] for the - /// hive-wide `hivectl stop` / `hivectl start` operator flow. + /// 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`). ControlInfraContainer { + /// Infra container name (e.g. `hive-ci`); must be in + /// [`SIBLING_CONTAINERS`]. container: String, action: InfraAction, }, @@ -414,21 +393,15 @@ pub enum PrivEvent { #[cfg(test)] mod tests { - use super::{RESTARTABLE_INFRA_CONTAINERS, SIBLING_CONTAINERS}; + use super::SIBLING_CONTAINERS; #[test] - fn restartable_infra_is_a_safe_subset_of_siblings() { - // Every restartable infra container must be a known sibling. - for c in RESTARTABLE_INFRA_CONTAINERS { - assert!( - SIBLING_CONTAINERS.contains(c), - "{c} is not a managed sibling container" - ); - } - // hive-matrix and hive-c0re are deliberately excluded: kicking the - // matrix backend mid-sync is its own concern, and a self-restart of - // c0re would sever the request socket. - assert!(!RESTARTABLE_INFRA_CONTAINERS.contains(&"hive-matrix")); - assert!(!RESTARTABLE_INFRA_CONTAINERS.contains(&"hive-c0re")); + fn infra_control_allowlist_excludes_c0re_includes_matrix() { + // SIBLING_CONTAINERS is the authoritative allowlist for infra + // lifecycle ops. hive-c0re must NEVER be in it — stopping the daemon + // would sever the socket the request arrived on. + assert!(!SIBLING_CONTAINERS.contains(&"hive-c0re")); + // hive-matrix IS controllable (operator can stop/start/restart it). + assert!(SIBLING_CONTAINERS.contains(&"hive-matrix")); } }