From 4f75e2a71a25fb9a31979c860ab5ef5e36deb475 Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 17 Oct 2026 23:40:53 +0200 Subject: [PATCH] hive-priv refuses bind-mount edits on the swarm's forge and matrix containers The forge and matrix containers belong to the services host, not to a hive. hive-priv's container-name validator still accepted hive-forge and hive-matrix for WriteNspawnFlags, WriteResourceLimits and RemoveServiceDropin; hive-c0re only ever sends h- names there. - hive-priv-sock/src/lib.rs: drop SIBLING_CONTAINERS, its only user was the validator, and its test. - hive-priv/src/main.rs: the validator accepts only h- and returns the logical name, so write_resource_limits loses its non-h- fallback; test that all three requests refuse both names. - hive-priv/README.md, docs/trust-boundary/security.md: the allowlist no longer lists the forge and matrix. Fixes #5508 --- docs/trust-boundary/security.md | 7 ++-- hive-priv-sock/src/lib.rs | 27 +----------- hive-priv/README.md | 7 ++-- hive-priv/src/main.rs | 74 ++++++++++++++++++++++----------- 4 files changed, 57 insertions(+), 58 deletions(-) diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index f282684f..a0a57a9b 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -314,9 +314,10 @@ known operations; there is no arbitrary command pass-through: **Container allowlist** — `hive-priv` validates every request against an allowlist before any operation: the allowlist accepts only names -matching the agent-name convention (char-validated) or the known -sibling service containers (`hive-forge`, `hive-matrix`), -and rejects arbitrary container names. `hive-gateway` is a host unit, +matching the agent-name convention (`h-` prefix, char-validated) +and rejects every other container name, the swarm's `hive-forge` and +`hive-matrix` included: those belong to the host that runs them, not to +a hive. `hive-gateway` is a host unit, not a container, so it's not in this list — see `ReloadGatewayNginx` above for how the hard-coded unit name scopes its access instead. diff --git a/hive-priv-sock/src/lib.rs b/hive-priv-sock/src/lib.rs index fe56b6de..86e3679f 100644 --- a/hive-priv-sock/src/lib.rs +++ b/hive-priv-sock/src/lib.rs @@ -28,21 +28,6 @@ pub const PAUSED_MARKER_FILE: &str = "paused"; /// Sub-agent container prefix. System container name = `h-`. pub const AGENT_PREFIX: &str = "h-"; -/// Sibling service containers managed by hive-c0re. This doubles as the -/// authoritative allowlist for the requests that name a container as a -/// string (bind-mount edits): only these — or a valid agent -/// name — are accepted. `hive-c0re` is deliberately absent; so is the -/// gateway, whose nginx is a plain host unit rather than a container. -/// hive-priv re-validates against this list root-side, so it's authoritative -/// regardless of what the caller sends. -/// -/// ⚠️ This is the *container-name* allowlist, not the lifecycle one: -/// [`InfraContainer`] is what gates -/// [`PrivRequest::ControlInfraContainer`], and no name in this list has a -/// variant there: the forge and matrix are swarm services, started and -/// stopped by systemd on the host that runs them, never from a hive. -pub const SIBLING_CONTAINERS: &[&str] = &["hive-forge", "hive-matrix"]; - /// Lifecycle verb for [`PrivRequest::ControlInfraContainer`]. Maps directly /// to `systemctl `, where the unit comes from /// [`InfraContainer::service_unit`]. @@ -659,17 +644,7 @@ pub enum PrivEvent { #[cfg(test)] mod tests { - use super::{InfraContainer, SIBLING_CONTAINERS}; - - #[test] - fn sibling_container_allowlist_excludes_c0re() { - // SIBLING_CONTAINERS is the authoritative allowlist for the - // requests that name a container as a string. hive-c0re must NEVER - // be in it — stopping the daemon would sever the socket the request - // arrived on. - assert!(!SIBLING_CONTAINERS.contains(&"hive-c0re")); - assert!(SIBLING_CONTAINERS.contains(&"hive-matrix")); - } + use super::InfraContainer; #[test] fn infra_control_names_no_c0re_or_swarm_service() { diff --git a/hive-priv/README.md b/hive-priv/README.md index 5b25b715..7718aa31 100644 --- a/hive-priv/README.md +++ b/hive-priv/README.md @@ -16,10 +16,9 @@ See `docs/trust-boundary/boundary.md` and `docs/trust-boundary/security.md` for ## Security model -- **Strict allowlist.** Every request is validated against a container-name - allowlist before any filesystem or process operation — only names matching the - hive convention (`h-*`, the manager container, known sibling service - containers) are accepted. +- **Strict allowlist.** hive-priv checks every request against a container-name + allowlist before any filesystem or process operation, and accepts only names + matching the hive convention (`h-*`, the manager container included). - **No pass-through.** Every `PrivRequest` variant maps to a single known operation; there is no arbitrary-command escape hatch. - **Socket-activated, always.** systemd binds `/run/hive/priv.sock` diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index fbffcc53..c000d0e2 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -9,8 +9,7 @@ //! **Security model**: every request is validated against a strict //! container-name allowlist before any filesystem or process operation. //! Only containers whose names match the hive convention (`h-*`, -//! the manager container, or known sibling service containers) are -//! accepted. Every variant maps to a single known operation — no +//! the manager container included) are accepted. Every variant maps to a single known operation — no //! arbitrary command pass-through. //! //! **Socket activation**: when systemd passes the listener socket via @@ -25,7 +24,7 @@ use hive_priv_sock::{ AGENT_PREFIX, AGENT_RUNTIME_ROOT, AGENT_STATE_ROOT, BindMount, CredentialMount, HiveFacts, InfraAction, InfraContainer, MEASURE_AGENT_DISK_TIMEOUT, META_DIR, MIGRATE_STAGING_ROOT, NetworkIsolation, PAUSED_MARKER_FILE, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, - PrivStream, PrivStreamLine, SIBLING_CONTAINERS, + PrivStream, PrivStreamLine, }; use tokio::io::{AsyncWriteExt, BufReader}; use tokio::net::unix::OwnedWriteHalf; @@ -1073,11 +1072,7 @@ fn write_resource_limits( cpu_weight: Option, io_weight: Option, ) -> Result<(String, String)> { - validate_container_system_name(container)?; - // Derive the logical agent name (strip h- prefix) to form the runtime - // dir path. Falls back to the full container name for infra containers - // that don't use the h- prefix. - let logical = container.strip_prefix(AGENT_PREFIX).unwrap_or(container); + let logical = validate_container_system_name(container)?; let runtime_dir = format!("{AGENT_RUNTIME_ROOT}/{logical}"); let dir = format!("/run/systemd/system/container@{container}.service.d"); std::fs::create_dir_all(&dir).with_context(|| format!("create {dir}"))?; @@ -2784,17 +2779,16 @@ fn validate_agent_name(name: &str) -> Result<()> { 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<()> { - if SIBLING_CONTAINERS.contains(&name) { - return Ok(()); - } - if let Some(suffix) = name.strip_prefix(AGENT_PREFIX) { - validate_name_chars(suffix)?; - return Ok(()); - } - bail!("container name {name:?} is not managed by hive"); +/// Validate a system-level container name (`h-` prefix for all agents, +/// the manager included) and return the logical agent name after it. +/// Anything else is refused, the swarm's forge and matrix containers +/// included: they belong to the host that runs them, not to a hive. +fn validate_container_system_name(name: &str) -> Result<&str> { + let Some(suffix) = name.strip_prefix(AGENT_PREFIX) else { + bail!("container name {name:?} is not managed by hive"); + }; + validate_name_chars(suffix)?; + Ok(suffix) } fn validate_name_chars(name: &str) -> Result<()> { @@ -3160,14 +3154,15 @@ fn ensure_socket_dir_in(root: &Path, name: &str) -> Result<()> { #[cfg(test)] mod tests { use super::{ - BindMount, BoundedRun, Command, HiveFacts, InfraAction, InfraContainer, OwnedFd, - PAUSED_MARKER_FILE, PrivRequest, StagedFile, agent_disk_paths, check_fd_agreement, + BindMount, BoundedRun, Command, HiveFacts, InfraAction, InfraContainer, NetworkIsolation, + OwnedFd, PAUSED_MARKER_FILE, PrivRequest, StagedFile, agent_disk_paths, check_fd_agreement, contains_secret_shaped_run, describe_forge_admin, du_command, du_total, ensure_plain_filename, ensure_socket_dir_in, exec_measure_agent_disk, git_overlay_flags, - limits_dropin_body, open_dir, open_export_dest, parse_du_total, partial_name, publish_file, - redact_secret_line, remove_marker_in, render_hive_facts, run_bounded, single_output_path, - toplevel_attr, validate_credential_name, validate_snapshot_name, write_agent_dir_file, - write_bridge_dns_marker_in, + handle_write_nspawn_flags, limits_dropin_body, open_dir, open_export_dest, parse_du_total, + partial_name, publish_file, redact_secret_line, remove_marker_in, remove_service_dropin, + render_hive_facts, run_bounded, single_output_path, toplevel_attr, + validate_container_system_name, validate_credential_name, validate_snapshot_name, + write_agent_dir_file, write_bridge_dns_marker_in, write_resource_limits, }; use std::path::PathBuf; use std::sync::atomic::{AtomicU32, Ordering}; @@ -4137,6 +4132,35 @@ mod tests { assert!(validate_snapshot_name("hive-valid").is_ok()); } + /// The swarm's forge and matrix containers are not the hive's: every + /// request that names a container as a string refuses them before + /// touching the filesystem. Control: an agent container name passes. + #[test] + fn container_name_requests_refuse_swarm_service_containers() { + let isolation = NetworkIsolation { + bridge: "hive0".to_owned(), + gateway_ip: "10.42.0.1".to_owned(), + }; + let facts = HiveFacts { + name: None, + domain: None, + }; + for name in ["hive-forge", "hive-matrix"] { + let errs = [ + handle_write_nspawn_flags(name, &[], &isolation, &[], &facts).unwrap_err(), + write_resource_limits(name, "4G", "200%", None, None).unwrap_err(), + remove_service_dropin(name).unwrap_err(), + ]; + for e in errs { + assert!( + e.to_string().contains("is not managed by hive"), + "{name}: {e:#}" + ); + } + } + assert_eq!(validate_container_system_name("h-atlas").unwrap(), "atlas"); + } + /// `ensure_plain_filename` is the gate between an agent-writable /// directory and a root-privileged file write. `.`/`..` would resolve /// to the directory itself or its parent; any `/` climbs into a path