diff --git a/docs/boundary.md b/docs/boundary.md index e5093371..868a051e 100644 --- a/docs/boundary.md +++ b/docs/boundary.md @@ -94,40 +94,6 @@ couldn't connect the way the socket unit's `SocketGroup` grant intends. Requiring socket activation everywhere means dev and prod take the exact same path and the group grant always holds. -### the per-agent socket dir - -`/run/hive-agent//` is shared by **three principals that share no -group**, which is why its mode is what it is: - -| principal | reaches | needs | -|---|---|---| -| the agent's harness | binds + unlinks `agent.sock`, `web.sock` | owner, `rwx` | -| `hive-c0re` | dials `agent.sock` (todo wakes) | traverse | -| the gateway's nginx | dials `web.sock` | traverse | - -The last two land in "other", so the dir is **`0751`, owned by the -agent's container uid/gid** — `o=--x` is traverse without listing, and -both sockets are `0666`, which is all a dialer needs. - -**Ownership is declared, not repaired.** The tmpfiles.d entry written by -`SyncAgentTmpfiles` names the uid/gid directly. Do not add a chown -alongside it: `d` re-applies on every boot *and* every agent -spawn/destroy, so ownership set afterwards is reverted the next time any -agent changes — which is exactly how this dir spent a long time at -`0777 root root` while a privileged chown appeared to be fixing it. - -The mode is load-bearing, not cosmetic. Write permission on a -*directory* is what confers the right to unlink its entries, whoever owns -them, and the sticky bit is the only thing that would restrain that (it -is not set here). A world-writable socket dir therefore lets anything -able to reach the path delete an agent's socket and bind its own — and -the gateway container has all of `/run/hive-agent` bind-mounted in. -Dropping `o=w` removes that permission rather than qualifying it. - -⚠️ Contrast `/shared`, which *is* sticky world-writable (`1777`): it has -many legitimate writers, so sticky is the best available answer there. -This dir has exactly one writer, so it needs no world write at all. - ### host admin socket access (`hivectl`) `hivectl` drives the whole hive — spawn / kill / destroy / rebuild / diff --git a/docs/gateway.md b/docs/gateway.md index 1a725575..69540319 100644 --- a/docs/gateway.md +++ b/docs/gateway.md @@ -95,12 +95,6 @@ now set unconditionally for every agent. The mechanism: bind, not file bind — file bind-mounts don't survive the harness's `unlink + bind(2)` cycle on socket replace. Per-agent subdir keeps each agent's container blind to siblings' sockets. - - The dir is `0751`, owned by the agent's container uid/gid, so - nginx reaches `web.sock` through `o=--x` (traverse) and the socket's - own `0666`. The gateway is one of three principals sharing that dir - and does not own its ownership rules — see - [`docs/boundary.md`](boundary.md#the-per-agent-socket-dir). 3. **Marker gate**. After successful `bind_unix`, the harness drops `/hyperhive-socket-bound` next to the socket. c0re's `agent_sockets::write` filters its JSON map by marker presence — diff --git a/docs/security.md b/docs/security.md index d0ed66d3..6a2b5e1b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -212,6 +212,7 @@ known operations; there is no arbitrary command pass-through: | `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`CPUWeight=`/`IOWeight=` systemd drop-in for agent container | | `RemoveServiceDropin` | remove `container@.service.d/` drop-in on destroy | | `DaemonReload` | `systemctl daemon-reload` | +| `ChownSocketDir` / `ChmodSocketDir` | chown/chmod `/run/hive-agent//` socket directory | | `RunForgeAdmin` | `nixos-container run hive-forge -- runuser -u forgejo -- forgejo admin ` | | `WriteAgentForgeToken` / `WriteAgentMatrixToken` | write `0600` credential file into agent state dir | | `RestartMatrixDaemon` | `systemctl --machine=h- restart hive-matrix-daemon.service` | diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index 7a7c76a5..1574f1a5 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -10,8 +10,8 @@ use hive_priv_sock::{BindMount, CredentialMount}; use crate::coordinator::{AgentPaths, HiveEnv}; use super::{ - AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, bridge_gateway_ip, - container_claude_mount, container_name, validate, + AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, agent_uid_gid, + bridge_gateway_ip, container_claude_mount, container_name, validate, }; /// Re-apply the per-container host-side config: nspawn flags (bind @@ -289,11 +289,16 @@ async fn set_nspawn_flags( let socket_dir = crate::agent_sockets::agent_dir_for(agent_name); std::fs::create_dir_all(&socket_dir) .with_context(|| format!("create {}", socket_dir.display()))?; - // Ownership is NOT repaired here. The dir's owner + mode are declared by - // the tmpfiles.d entry (`SyncAgentTmpfiles`), which is the mechanism that - // re-applies on every boot and every spawn — so a chown made here was - // silently reverted the next time any agent was spawned or destroyed. - // This `create_dir_all` only covers the window before that sync lands. + // Chown to the agent user so the non-root harness can bind(2) here. + // Falls back to 0777 on first spawn when uid lookup returns None + // (container /etc/passwd not yet rendered). + if let Some((uid, gid)) = agent_uid_gid(agent_name) { + if let Err(e) = crate::priv_client::chown_socket_dir(agent_name, uid, gid).await { + tracing::warn!(%agent_name, error = ?e, "chown socket dir failed"); + } + } else if let Err(e) = crate::priv_client::chmod_socket_dir(agent_name, 0o777).await { + tracing::warn!(%agent_name, error = ?e, "chmod socket dir failed"); + } binds.push(BindMount { host_path: socket_dir.to_string_lossy().into_owned(), container_path: socket_dir.to_string_lossy().into_owned(), diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index 4bd00109..a69b71a9 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -712,16 +712,6 @@ pub async fn sync_tmpfiles() { Ok(containers) => containers .into_iter() .filter_map(|c| c.strip_prefix(AGENT_PREFIX).map(str::to_owned)) - .map(|name| { - // Resolved here, not in hive-priv: the mapping lives in the - // container's /etc/passwd, which is c0re's to read. `None` - // until the container's first boot renders it. - let (uid, gid) = match agent_uid_gid(&name) { - Some((uid, gid)) => (Some(uid), Some(gid)), - None => (None, None), - }; - hive_priv_sock::AgentTmpfilesEntry { name, uid, gid } - }) .collect::>(), Err(e) => { tracing::warn!(error = ?e, "sync_tmpfiles: list failed; skipping"); diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index f2f45ed2..4663db86 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_priv_sock::{ - AgentTmpfilesEntry, BindMount, CredentialMount, InfraAction, InfraContainer, JournalQuery, - NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream, + BindMount, CredentialMount, InfraAction, InfraContainer, JournalQuery, NetworkIsolation, + PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream, }; use std::os::fd::{AsRawFd as _, OwnedFd, RawFd}; @@ -338,6 +338,23 @@ pub async fn reload_gateway_nginx() -> Result<()> { ok(call(&PrivRequest::ReloadGatewayNginx).await?) } +pub async fn chown_socket_dir(agent_name: &str, uid: u32, gid: u32) -> Result<()> { + ok(call(&PrivRequest::ChownSocketDir { + agent_name: agent_name.to_owned(), + uid, + gid, + }) + .await?) +} + +pub async fn chmod_socket_dir(agent_name: &str, mode: u32) -> Result<()> { + ok(call(&PrivRequest::ChmodSocketDir { + agent_name: agent_name.to_owned(), + mode, + }) + .await?) +} + /// Run `forgejo admin ` inside the `hive-forge` container via /// hive-priv (which runs as root and can nsenter into the container). /// Returns `(stdout, stderr)` on success. @@ -635,16 +652,15 @@ pub async fn send_agent_snapshot_to_file( Ok(stdout) } -/// Write `/etc/tmpfiles.d/hyperhive-agents.conf` for `agents` and immediately -/// apply it with `systemd-tmpfiles --create`. Each entry carries the agent's -/// container uid/gid so the socket dir's ownership is *declared* here rather -/// than corrected afterwards. See [`PrivRequest::SyncAgentTmpfiles`]. +/// Write `/etc/tmpfiles.d/hyperhive-agents.conf` for `agents` (logical names, +/// e.g. `"atlas"`) and immediately apply it with `systemd-tmpfiles --create`. +/// See [`PrivRequest::SyncAgentTmpfiles`] for the full semantics. /// /// # Errors /// /// Returns an error if the priv socket call fails, if any agent name is /// invalid, or if `systemd-tmpfiles --create` exits non-zero. -pub async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<()> { +pub async fn sync_agent_tmpfiles(agents: &[String]) -> Result<()> { ok(call(&PrivRequest::SyncAgentTmpfiles { agents: agents.to_vec(), }) diff --git a/hive-priv-sock/src/lib.rs b/hive-priv-sock/src/lib.rs index 1ec43637..0c006a4b 100644 --- a/hive-priv-sock/src/lib.rs +++ b/hive-priv-sock/src/lib.rs @@ -375,6 +375,19 @@ pub enum PrivRequest { /// namespace via the machine bus (forbidden for unprivileged users). ReloadGatewayNginx, + // --- Socket dir ownership --- + /// Set ownership of `/run/hive-agent//` to `uid:gid`. + /// Called by `lifecycle::set_nspawn_flags` after `create_dir_all`. + ChownSocketDir { + agent_name: String, + uid: u32, + gid: u32, + }, + + /// Set mode of `/run/hive-agent//`. + /// Fallback when uid lookup returns `None` on first spawn. + ChmodSocketDir { agent_name: String, mode: u32 }, + // --- Forge admin CLI --- /// Run `forgejo admin ` inside the `hive-forge` container as the /// `forgejo` unix user. hive-priv executes: @@ -741,34 +754,12 @@ pub enum PrivRequest { /// Called at hive-c0re startup and after every agent spawn / destroy. /// Agents are logical names (validated by `validate_agent_name`). SyncAgentTmpfiles { - /// One entry per live agent. hive-priv validates each name before - /// writing any path component derived from it. - agents: Vec, + /// Logical agent names (e.g. `"atlas"`, `"ruth"`). hive-priv validates + /// each name before writing any path component derived from it. + agents: Vec, }, } -/// One agent's runtime-dir declaration for `SyncAgentTmpfiles`. -/// -/// Carries the container uid/gid so the tmpfiles entry can *declare* who owns -/// `/run/hive-agent/` instead of having it corrected afterwards by a -/// privileged chown. The two mechanisms used to fight: the tmpfiles line wrote -/// `0777 root root` and a follow-up `ChownSocketDir` narrowed it, but any -/// later spawn or destroy re-applied the file and reset *every* agent's dir -/// back to world-writable. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct AgentTmpfilesEntry { - /// Logical agent name (e.g. `"atlas"`, `"ruth"`). - pub name: String, - /// Container uid/gid of the agent user, when known. - /// - /// `None` only before the container's `/etc/passwd` has been rendered - /// (first boot). The dir must stay writable by the not-yet-identifiable - /// harness in that window, so hive-priv falls back to the historical - /// permissive mode for that one agent; the next sync tightens it. - pub uid: Option, - pub gid: Option, -} - /// Response from the privileged helper. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PrivResponse { diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index fb9b8ee7..b2091554 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -22,10 +22,10 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; use hive_priv_sock::{ - AGENT_PREFIX, AGENT_RUNTIME_ROOT, AGENT_STATE_ROOT, AgentTmpfilesEntry, BindMount, - CredentialMount, InfraAction, InfraContainer, JournalQuery, META_DIR, MIGRATE_STAGING_ROOT, - NetworkIsolation, PAUSED_MARKER_FILE, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, - PrivStream, PrivStreamLine, SIBLING_CONTAINERS, + AGENT_PREFIX, AGENT_RUNTIME_ROOT, AGENT_STATE_ROOT, BindMount, CredentialMount, InfraAction, + InfraContainer, JournalQuery, META_DIR, MIGRATE_STAGING_ROOT, NetworkIsolation, + PAUSED_MARKER_FILE, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream, + PrivStreamLine, SIBLING_CONTAINERS, }; use tokio::io::{AsyncWriteExt, BufReader}; use tokio::net::unix::OwnedWriteHalf; @@ -418,6 +418,17 @@ async fn exec( PrivRequest::ReloadGatewayNginx => sync_gateway_nginx().await, + PrivRequest::ChownSocketDir { + ref agent_name, + uid, + gid, + } => chown_socket_dir(agent_name, uid, gid), + + PrivRequest::ChmodSocketDir { + ref agent_name, + mode, + } => chmod_socket_dir(agent_name, mode), + PrivRequest::RunForgeAdmin { ref args } => { for arg in args { validate_forge_admin_arg(arg)?; @@ -703,6 +714,26 @@ fn remove_service_dropin(container: &str) -> Result<(String, String)> { Ok((String::new(), String::new())) } +/// `ChownSocketDir` — chown the agent's host socket dir to its +/// container uid/gid. +fn chown_socket_dir(agent_name: &str, uid: u32, gid: u32) -> Result<(String, String)> { + validate_agent_name(agent_name)?; + let path = socket_dir_path(agent_name); + std::os::unix::fs::chown(&path, Some(uid), Some(gid)) + .with_context(|| format!("chown {} to {uid}:{gid}", path.display()))?; + Ok((String::new(), String::new())) +} + +/// `ChmodSocketDir` — set the mode on the agent's host socket dir. +fn chmod_socket_dir(agent_name: &str, mode: u32) -> Result<(String, String)> { + use std::os::unix::fs::PermissionsExt as _; + validate_agent_name(agent_name)?; + let path = socket_dir_path(agent_name); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)) + .with_context(|| format!("chmod {:o} {}", mode, path.display()))?; + Ok((String::new(), String::new())) +} + /// `WriteResourceLimits` — drop the systemd resource settings into the /// container service's drop-in dir, together with a /// `ConditionPathIsDirectory=` guard on the agent's MCP runtime dir. @@ -2174,6 +2205,11 @@ fn container_system_name(name: &str) -> String { format!("{AGENT_PREFIX}{name}") } +/// Path of the per-agent unix-socket dir on the host. +fn socket_dir_path(agent_name: &str) -> PathBuf { + PathBuf::from(format!("{SOCKET_DIR_ROOT}/{agent_name}")) +} + /// Validate a logical agent name (the name hive-c0re uses internally, /// before the `h-` container prefix is applied). fn validate_agent_name(name: &str) -> Result<()> { @@ -2391,10 +2427,10 @@ fn write_bridge_dns_marker(container: &str, isolation: Option<&NetworkIsolation> /// - `/run/hive-agent/` (web socket dir, bind-mounted into container) const TMPFILES_PATH: &str = "/etc/tmpfiles.d/hyperhive-agents.conf"; -async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, String)> { +async fn sync_agent_tmpfiles(agents: &[String]) -> Result<(String, String)> { use std::fmt::Write as _; - for entry in agents { - validate_agent_name(&entry.name)?; + for name in agents { + validate_agent_name(name)?; } // Build tmpfiles.d content. Root dirs first, then per-agent. @@ -2405,52 +2441,20 @@ async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, S // tmpfiles.d entry here ensures it exists before hive-c0re starts (boot race). content.push_str("d /run/hyperhive 0750 hive-core hive-core -\n"); writeln!(content, "d {AGENT_RUNTIME_ROOT} 0755 hive-core hive-core -").ok(); - // `hive-core`, not root: c0re does the `create_dir_all` for a new agent's - // subdir itself, so a root-owned parent EACCESes on the first spawn of a - // fresh host. This must stay in step with the identical rule in - // `nix/host-modules/hive-gateway/default.nix` — the two files declared - // different owners for this one path, and which won depended on the order - // systemd happened to read them in. - writeln!(content, "d {SOCKET_DIR_ROOT} 0755 hive-core hive-core -").ok(); + writeln!(content, "d {SOCKET_DIR_ROOT} 0755 root root -").ok(); // Per-agent dirs. - for entry in agents { - let name = &entry.name; + for name in agents { writeln!( content, "d {AGENT_RUNTIME_ROOT}/{name} 0755 hive-core hive-core -" ) .ok(); - // The agent's socket dir. Three principals need it and no two share a - // group, so the mode has to say so explicitly: - // - // owner = the agent user rwx binds + unlinks agent.sock/web.sock - // other = --x traverse only, no listing - // - // "other" covers hive-c0re (dials agent.sock) and the gateway's nginx - // (dials web.sock, and has all of /run/hive-agent bind-mounted in). - // Both sockets are 0666, so traversal is all they need. - // - // 0751 rather than the historical 0777 is a fix, not a tidy-up: - // write permission on a *directory* is what confers the right to - // unlink its entries, whoever owns them — the sticky bit is the only - // thing that would restrain that, and it was never set here. So the - // old world-writable mode let anything able to reach the path delete - // an agent's socket, bind its own, and receive that agent's todos. - // Dropping `o=w` removes that permission outright rather than - // qualifying it. Declaring the owner here also ends the tug-of-war - // with the - // old ChownSocketDir: `d` re-applies on every sync, so a chown made - // afterwards was reset by the next agent's spawn. - if let (Some(uid), Some(gid)) = (entry.uid, entry.gid) { - writeln!(content, "d {SOCKET_DIR_ROOT}/{name} 0751 {uid} {gid} -").ok(); - } else { - // Before the container's /etc/passwd exists there is no uid to - // name, and the harness must still be able to bind. Keep the old - // permissive mode for that agent alone; the next sync (any spawn - // or destroy, or c0re restart) resolves the uid and tightens it. - tracing::info!(%name, "tmpfiles.d: agent uid unknown, deferring 0751 on socket dir"); - writeln!(content, "d {SOCKET_DIR_ROOT}/{name} 0777 root root -").ok(); - } + // 0777: agent harness (non-root uid) must bind sockets here. + // `d` adjusts mode/owner on existing dirs; world-writable matches + // the chmod_socket_dir(0o777) fallback so a runtime re-sync doesn't + // break a live agent's socket dir. host_config's chown_socket_dir + // tightens ownership afterwards when the agent uid is available. + writeln!(content, "d {SOCKET_DIR_ROOT}/{name} 0777 root root -").ok(); } // Atomic write: write to a tmp file then rename so a concurrent reader diff --git a/nix/host-modules/hive-gateway/default.nix b/nix/host-modules/hive-gateway/default.nix index f813692e..a9df82c5 100644 --- a/nix/host-modules/hive-gateway/default.nix +++ b/nix/host-modules/hive-gateway/default.nix @@ -77,9 +77,6 @@ in # header so nginx can start + include the file before c0re writes # its first real content (f = create-if-absent, no overwrite). systemd.tmpfiles.rules = [ - # Must stay in step with the identical rule hive-priv generates into - # /etc/tmpfiles.d/hyperhive-agents.conf — the two used to declare - # different owners for this path. "d /run/hive-agent 0755 hive-core hive-core - -" "d /var/lib/hyperhive 0755 root root - -" "d /var/lib/hyperhive/gateway 0755 root root - -"