From 58b543446647557832d95806c67e2cf67a6b8ce0 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 8 Jun 2026 19:38:40 +0200 Subject: [PATCH] fix(#946): drop hive-priv self-bind fallback (require socket activation) + clarify child-state rw is intentional --- hive-c0re/src/lifecycle.rs | 12 +++++++- hive-priv/src/main.rs | 57 ++++++++++++++++++-------------------- nix/modules/hive-c0re.nix | 3 -- 3 files changed, 38 insertions(+), 34 deletions(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index e3edb85a..d71020cb 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1103,6 +1103,15 @@ const HOST_SHARED_ROOT: &str = "/var/lib/hyperhive/shared"; /// `binds`. All three are RW so the parent can read/write state and /// submit config-change requests. Creates missing host-side directories /// so nspawn doesn't refuse to start; missing dirs are non-fatal. +/// Mount a child agent's `state`, `harness`, and `config` dirs into the +/// parent, all read-write. The RW on `state` is deliberate (not just a +/// read mount): a parent manages its children, which includes writing +/// into a child's state for recovery (e.g. seeding notes / clearing a +/// stuck sentinel) as well as reading it. `config` is RW because the +/// parent authors proposed config changes for the child (the approval +/// flow commits into the child's config repo). `harness` is RW for the +/// same management reasons. Per-child isolation still holds: a child +/// only ever has its *own* dirs bind-mounted, never a sibling's. fn bind_child_agent_dirs(child: &str, binds: &mut Vec) { let state_dir = format!("{HOST_AGENTS_ROOT}/{child}/state"); let harness_dir = format!("{HOST_AGENTS_ROOT}/{child}/harness"); @@ -1221,7 +1230,8 @@ async fn set_nspawn_flags( // Topology-driven child mounts: every direct child of this agent gets // its state, harness, and config dirs bind-mounted RW so the parent - // can read state and manage config. + // can read AND write child state (recovery) and manage its config. + // See `bind_child_agent_dirs` for why state is RW, not read-only. let direct_children = crate::topology::children_of(agent_name); for child in &direct_children { bind_child_agent_dirs(child, &mut binds); diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index cdfcb3a4..09c6ef78 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -57,9 +57,14 @@ async fn main() -> Result<()> { } fn socket_listener() -> Result { - use std::os::unix::fs::PermissionsExt as _; - // Socket activation: systemd passes the socket as fd 3 when - // LISTEN_FDS >= 1 and LISTEN_PID matches our pid. + // hive-priv is ALWAYS socket-activated: systemd's `hive-priv.socket` + // unit binds `/run/hive/priv.sock` (SocketGroup=hive-core, mode 0660) + // and passes it as fd 3 via LISTEN_FDS. We require that — there is + // intentionally no self-bind fallback, so dev and prod take the exact + // same path. (The old fallback re-bound the socket itself as root's + // primary group, never `hive-core`, so a hive-core client couldn't + // connect the way the socket unit's grant intends; dropping it removes + // that dev/prod divergence.) let listen_fds: Option = std::env::var("LISTEN_FDS") .ok() .and_then(|s| s.parse().ok()); @@ -67,35 +72,27 @@ fn socket_listener() -> Result { .ok() .and_then(|s| s.parse().ok()); - if let (Some(n), Some(p)) = (listen_fds, listen_pid) - && n >= 1 - && p == std::process::id() - { - // SAFETY: systemd has passed us a ready UnixListener on fd 3. - let std_listener = unsafe { - use std::os::unix::io::FromRawFd; - std::os::unix::net::UnixListener::from_raw_fd(3) - }; - std_listener - .set_nonblocking(true) - .context("set socket non-blocking")?; - let listener = - tokio::net::UnixListener::from_std(std_listener).context("wrap systemd socket")?; - tracing::info!("using systemd-activated socket"); - return Ok(listener); + let activated = + matches!(listen_fds, Some(n) if n >= 1) && listen_pid == Some(std::process::id()); + if !activated { + bail!( + "hive-priv requires systemd socket activation (expected LISTEN_FDS>=1 + \ + LISTEN_PID= for {PRIV_SOCK}); run it via the hive-priv.socket unit, \ + not directly" + ); } - // Fallback: bind the socket ourselves. - let path = Path::new(PRIV_SOCK); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?; - } - let _ = std::fs::remove_file(path); - let listener = UnixListener::bind(path).with_context(|| format!("bind {PRIV_SOCK}"))?; - // Mode 0660: only the hive-core group can connect. - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o660)) - .context("chmod priv.sock")?; - tracing::info!(path = PRIV_SOCK, "bound priv socket"); + // SAFETY: systemd has passed us a ready UnixListener on fd 3. + let std_listener = unsafe { + use std::os::unix::io::FromRawFd; + std::os::unix::net::UnixListener::from_raw_fd(3) + }; + std_listener + .set_nonblocking(true) + .context("set socket non-blocking")?; + let listener = + tokio::net::UnixListener::from_std(std_listener).context("wrap systemd socket")?; + tracing::info!("using systemd-activated socket"); Ok(listener) } diff --git a/nix/modules/hive-c0re.nix b/nix/modules/hive-c0re.nix index d6425ebf..b4ee2907 100644 --- a/nix/modules/hive-c0re.nix +++ b/nix/modules/hive-c0re.nix @@ -894,8 +894,6 @@ in # Why each entry is needed: # /etc/nixos-containers — writes .conf (bind mounts, # network isolation, nspawn flags) - # /run/hive — fallback socket bind if LISTEN_FDS is - # absent (normal path: socket-activated) # /run/hive-agent — chown/chmod per-agent socket directories # /run/systemd — container@ unit drop-ins (resource limits) # + machinectl / systemd-machined state @@ -914,7 +912,6 @@ in ProtectSystem = "strict"; ReadWritePaths = [ "/etc/nixos-containers" - "/run/hive" "/run/hive-agent" "/run/systemd" "/run/lock"