fix: declare the agent socket dir's owner in tmpfiles, not by chown after
/run/hive-agent/<name> was 0777 root root in steady state, not just during first spawn. A directory without the sticky bit lets any user unlink files in it, and the gateway container has all of /run/hive-agent bind-mounted in, so anything that could reach the path could delete an agent's agent.sock, bind its own, and receive that agent's todos from hive-c0re. Two mechanisms were writing the dir and undoing each other: the tmpfiles.d entry wrote 0777 root root, then hive-c0re round-tripped through hive-priv's ChownSocketDir to narrow it. `d` re-asserts mode and owner on every apply and the file is regenerated on any agent's spawn or destroy, so every such event reset every agent's dir back to world-writable. SyncAgentTmpfiles now carries each agent's container uid/gid and the entry declares the answer: 0751 <uid> <gid>. Three principals need the dir and no two share a group -- the harness binds its sockets (owner rwx), hive-c0re dials agent.sock and the gateway's nginx dials web.sock (both only need traverse, and both sockets are already 0666). Deletes ChownSocketDir and ChmodSocketDir, both priv_client wrappers, the either/or in host_config with its two swallowed warn!s, and the now-dead socket_dir_path -- two verbs off the privileged helper's surface and one round-trip off every agent spawn. Also makes the two tmpfiles rules for /run/hive-agent itself agree: the gateway module said hive-core, the generated file said root, and which won depended on the order systemd read them in.
This commit is contained in:
parent
642377c5e0
commit
3fc1588e83
8 changed files with 110 additions and 103 deletions
|
|
@ -95,6 +95,21 @@ 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.
|
||||
|
||||
**Ownership of that dir is declared, not repaired.** The tmpfiles.d
|
||||
entry written by `SyncAgentTmpfiles` names the agent's container
|
||||
uid/gid directly — `d /run/hive-agent/<name> 0751 <uid> <gid> -`.
|
||||
Three principals need the dir and no two share a group: the harness
|
||||
(owner, `rwx`, binds + unlinks its sockets), `hive-c0re` (dials
|
||||
`agent.sock`) and the gateway's nginx (dials `web.sock`, and has all
|
||||
of `/run/hive-agent` bind-mounted in). The latter two only need
|
||||
traverse, which is what `o=--x` grants; both sockets are `0666`.
|
||||
Do **not** reintroduce a chown here: tmpfiles re-applies this entry
|
||||
on every boot *and* every agent spawn/destroy, so any ownership set
|
||||
afterwards is reverted the next time any agent changes. The mode is
|
||||
also load-bearing — a directory without the sticky bit lets any user
|
||||
unlink files in it, so a world-writable socket dir would let anything
|
||||
that can reach the path replace an agent's socket with its own.
|
||||
3. **Marker gate**. After successful `bind_unix`, the harness drops
|
||||
`<dir>/hyperhive-socket-bound` next to the socket. c0re's
|
||||
`agent_sockets::write` filters its JSON map by marker presence —
|
||||
|
|
|
|||
|
|
@ -212,7 +212,6 @@ known operations; there is no arbitrary command pass-through:
|
|||
| `WriteResourceLimits` | write `CPUQuota=`/`MemoryMax=`/`CPUWeight=`/`IOWeight=` systemd drop-in for agent container |
|
||||
| `RemoveServiceDropin` | remove `container@<name>.service.d/` drop-in on destroy |
|
||||
| `DaemonReload` | `systemctl daemon-reload` |
|
||||
| `ChownSocketDir` / `ChmodSocketDir` | chown/chmod `/run/hive-agent/<name>/` socket directory |
|
||||
| `RunForgeAdmin` | `nixos-container run hive-forge -- runuser -u forgejo -- forgejo admin <args>` |
|
||||
| `WriteAgentForgeToken` / `WriteAgentMatrixToken` | write `0600` credential file into agent state dir |
|
||||
| `RestartMatrixDaemon` | `systemctl --machine=h-<name> restart hive-matrix-daemon.service` |
|
||||
|
|
|
|||
|
|
@ -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, agent_uid_gid,
|
||||
bridge_gateway_ip, container_claude_mount, container_name, validate,
|
||||
AGENT_PREFIX, CONTAINER_RUNTIME_MOUNT, CONTAINER_SHARED_MOUNT, bridge_gateway_ip,
|
||||
container_claude_mount, container_name, validate,
|
||||
};
|
||||
|
||||
/// Re-apply the per-container host-side config: nspawn flags (bind
|
||||
|
|
@ -289,16 +289,11 @@ 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()))?;
|
||||
// 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");
|
||||
}
|
||||
// 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.
|
||||
binds.push(BindMount {
|
||||
host_path: socket_dir.to_string_lossy().into_owned(),
|
||||
container_path: socket_dir.to_string_lossy().into_owned(),
|
||||
|
|
|
|||
|
|
@ -712,6 +712,16 @@ 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::<Vec<_>>(),
|
||||
Err(e) => {
|
||||
tracing::warn!(error = ?e, "sync_tmpfiles: list failed; skipping");
|
||||
|
|
|
|||
|
|
@ -8,8 +8,8 @@
|
|||
|
||||
use anyhow::{Context as _, Result, bail};
|
||||
use hive_priv_sock::{
|
||||
BindMount, CredentialMount, InfraAction, InfraContainer, JournalQuery, NetworkIsolation,
|
||||
PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream,
|
||||
AgentTmpfilesEntry, BindMount, CredentialMount, InfraAction, InfraContainer, JournalQuery,
|
||||
NetworkIsolation, PRIV_SOCK, PrivEvent, PrivRequest, PrivResponse, PrivStream,
|
||||
};
|
||||
use std::os::fd::{AsRawFd as _, OwnedFd, RawFd};
|
||||
|
||||
|
|
@ -338,23 +338,6 @@ 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 <args>` inside the `hive-forge` container via
|
||||
/// hive-priv (which runs as root and can nsenter into the container).
|
||||
/// Returns `(stdout, stderr)` on success.
|
||||
|
|
@ -652,15 +635,16 @@ pub async fn send_agent_snapshot_to_file(
|
|||
Ok(stdout)
|
||||
}
|
||||
|
||||
/// 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.
|
||||
/// 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`].
|
||||
///
|
||||
/// # 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: &[String]) -> Result<()> {
|
||||
pub async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<()> {
|
||||
ok(call(&PrivRequest::SyncAgentTmpfiles {
|
||||
agents: agents.to_vec(),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -375,19 +375,6 @@ pub enum PrivRequest {
|
|||
/// namespace via the machine bus (forbidden for unprivileged users).
|
||||
ReloadGatewayNginx,
|
||||
|
||||
// --- Socket dir ownership ---
|
||||
/// Set ownership of `/run/hive-agent/<agent_name>/` 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/<agent_name>/`.
|
||||
/// Fallback when uid lookup returns `None` on first spawn.
|
||||
ChmodSocketDir { agent_name: String, mode: u32 },
|
||||
|
||||
// --- Forge admin CLI ---
|
||||
/// Run `forgejo admin <args>` inside the `hive-forge` container as the
|
||||
/// `forgejo` unix user. hive-priv executes:
|
||||
|
|
@ -754,12 +741,34 @@ pub enum PrivRequest {
|
|||
/// Called at hive-c0re startup and after every agent spawn / destroy.
|
||||
/// Agents are logical names (validated by `validate_agent_name`).
|
||||
SyncAgentTmpfiles {
|
||||
/// Logical agent names (e.g. `"atlas"`, `"ruth"`). hive-priv validates
|
||||
/// each name before writing any path component derived from it.
|
||||
agents: Vec<String>,
|
||||
/// One entry per live agent. hive-priv validates each name before
|
||||
/// writing any path component derived from it.
|
||||
agents: Vec<AgentTmpfilesEntry>,
|
||||
},
|
||||
}
|
||||
|
||||
/// One agent's runtime-dir declaration for `SyncAgentTmpfiles`.
|
||||
///
|
||||
/// Carries the container uid/gid so the tmpfiles entry can *declare* who owns
|
||||
/// `/run/hive-agent/<name>` 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<u32>,
|
||||
pub gid: Option<u32>,
|
||||
}
|
||||
|
||||
/// Response from the privileged helper.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct PrivResponse {
|
||||
|
|
|
|||
|
|
@ -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, 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, AgentTmpfilesEntry, 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,17 +418,6 @@ 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)?;
|
||||
|
|
@ -714,26 +703,6 @@ 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.
|
||||
|
|
@ -2205,11 +2174,6 @@ 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<()> {
|
||||
|
|
@ -2427,10 +2391,10 @@ fn write_bridge_dns_marker(container: &str, isolation: Option<&NetworkIsolation>
|
|||
/// - `/run/hive-agent/<name>` (web socket dir, bind-mounted into container)
|
||||
const TMPFILES_PATH: &str = "/etc/tmpfiles.d/hyperhive-agents.conf";
|
||||
|
||||
async fn sync_agent_tmpfiles(agents: &[String]) -> Result<(String, String)> {
|
||||
async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, String)> {
|
||||
use std::fmt::Write as _;
|
||||
for name in agents {
|
||||
validate_agent_name(name)?;
|
||||
for entry in agents {
|
||||
validate_agent_name(&entry.name)?;
|
||||
}
|
||||
|
||||
// Build tmpfiles.d content. Root dirs first, then per-agent.
|
||||
|
|
@ -2441,20 +2405,48 @@ async fn sync_agent_tmpfiles(agents: &[String]) -> Result<(String, String)> {
|
|||
// 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();
|
||||
writeln!(content, "d {SOCKET_DIR_ROOT} 0755 root root -").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();
|
||||
// Per-agent dirs.
|
||||
for name in agents {
|
||||
for entry in agents {
|
||||
let name = &entry.name;
|
||||
writeln!(
|
||||
content,
|
||||
"d {AGENT_RUNTIME_ROOT}/{name} 0755 hive-core hive-core -"
|
||||
)
|
||||
.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();
|
||||
// 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: a
|
||||
// directory without the sticky bit lets *any* user unlink files in it,
|
||||
// so world-writable here means anything that can reach the path could
|
||||
// delete an agent's socket, bind its own, and receive that agent's
|
||||
// todos. 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();
|
||||
}
|
||||
}
|
||||
|
||||
// Atomic write: write to a tmp file then rename so a concurrent reader
|
||||
|
|
|
|||
|
|
@ -77,6 +77,9 @@ 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 - -"
|
||||
|
|
|
|||
Loading…
Reference in a new issue