From 4f6407fdeac691f47dce275c4d06887a9d111b58 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 21 Sep 2026 18:11:13 +0200 Subject: [PATCH] collapse the roles.json mount grant into the ManageRootAgent capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hive had two spellings of "this agent may act on agents that aren't its children": the `ManageRootAgent` capability, which nothing checked, and a `can_manage_top_level_agents` role in a third meta store, `roles.json`, which owned the real grant — the bind mounts that put another agent's state (rw) and config (ro) inside the holder's container. The two drifted independently, and with the parent/child hierarchy removed the role's set (`parent.is_none()`) silently became every agent while nothing said so. Collapse them. The mount grant now hangs off `Capability::ManageRootAgent`, looked up through the one capability path that already exists (`capabilities::has_cap` over `capabilities.json`) rather than a second mechanism. `roles.json` and everything that read, wrote or reconciled it is gone, along with its `meta.rs` staging and commit-label wiring; nothing in the tree reads that file any more. The enum variant keeps its name deliberately. Renaming it would turn every `manage_root_agent` already stored in `capabilities.json` into an unrecognised name that `prune_unknown` drops without asking. Its meaning, not its spelling, is what changed: "may manage any agent". The doc comment and the description string now say that. `top_level_agents()`/`top_level_agents_in()` are replaced by `all_agents()`/`all_agents_in()`. Under "manage any agent" the mounted set is every agent by definition, so the code states it instead of deriving it from a predicate that no longer discriminates — and the call-site comment explains that, because it otherwise reads as a widening. The holder is no longer bound as its own virtual child: that reproduced the own-state and own-config mounts exactly, so dropping it loses nothing. --- docs/agent-lifecycle/approvals.md | 19 +- docs/process/conventions.md | 2 +- docs/web-ui/dashboard.md | 2 +- hive-agent/src/mcp_config.rs | 5 +- hive-c0re/src/agent_config/topology.rs | 264 ++++--------------------- hive-c0re/src/lifecycle/host_config.rs | 35 ++-- hive-c0re/src/meta.rs | 14 +- hive-sh4re/src/permissions.rs | 21 +- 8 files changed, 100 insertions(+), 262 deletions(-) diff --git a/docs/agent-lifecycle/approvals.md b/docs/agent-lifecycle/approvals.md index 9cd7d632..19afec2b 100644 --- a/docs/agent-lifecycle/approvals.md +++ b/docs/agent-lifecycle/approvals.md @@ -496,19 +496,24 @@ forge into its own state dir, commit on a branch, open a PR**, and let the operator review and approve it. By design, no second, mount-shaped path reaches the same file without the review. -Agents holding the `can_manage_top_level_agents` topology role (see -`hive-c0re/src/agent_config/topology.rs`) get additional host-side -bind mounts via `set_nspawn_flags`: +Agents holding the `manage_root_agent` capability (granted per agent in +`capabilities.json`; see `hive-sh4re/src/permissions.rs`) get additional +host-side bind mounts via `set_nspawn_flags`: -- `/var/lib/hyperhive/agents/` → `/agents/` (RW) — all top-level - agents' proposed repos (not just direct children). +- `/var/lib/hyperhive/agents/` → `/agents/` (RW) — **every** agent's + proposed repo, not just direct children. The capability means "may + manage any agent", so the mounted set is every agent. - `/var/lib/hyperhive/applied/` → `/applied/` (RO) — every agent's authoritative applied repo, including `.git`. - `/var/lib/hyperhive/meta/` → `/meta/` (RO) — the swarm-wide deploy flake. -The root agent holds this role; a sub-manager that only manages a -subtree doesn't, and only has its direct children's config dirs. +An agent without the capability only has its direct children's config +dirs. + +⚠️ nspawn bind flags are baked at container start, so granting or +revoking this capability does not change any mount until that agent's +container is rebuilt/restarted. Each proposed repo (`/agents//config/`) is pre-configured with `applied` as a git remote pointing at diff --git a/docs/process/conventions.md b/docs/process/conventions.md index d21bc6a0..aefacf00 100644 --- a/docs/process/conventions.md +++ b/docs/process/conventions.md @@ -362,7 +362,7 @@ that allows the underlying resource access. | Capability | Effect | |---|---| -| `manage_root_agent` | may lifecycle-manage the root/manager agent via `kill`/`start`/`restart` | +| `manage_root_agent` | may manage *any* agent: bind-mounts every agent's state (rw) + config (ro) into the holder's container, plus `/applied` and `/meta` (ro). Takes effect on the holder's next container rebuild/restart | | `read_host_journal` | registers the `get_host_journal` MCP tool + serves `GET /journal-host` requests | **Config storage** — per-agent capabilities live in diff --git a/docs/web-ui/dashboard.md b/docs/web-ui/dashboard.md index d07ba185..7b9ba0c2 100644 --- a/docs/web-ui/dashboard.md +++ b/docs/web-ui/dashboard.md @@ -440,7 +440,7 @@ The current capabilities are: | Name | Effect | |------|--------| -| `manage_root_agent` | allows the `set_status` / lifecycle tools on the root agent | +| `manage_root_agent` | mounts every agent's state (rw) + config (ro) into this agent's container so it can manage/recover any agent | | `read_host_journal` | unlocks `get_host_journal` to read journald from inside a container | Each row is one agent. Columns are the capability names returned by diff --git a/hive-agent/src/mcp_config.rs b/hive-agent/src/mcp_config.rs index f24fbb7e..84c646df 100644 --- a/hive-agent/src/mcp_config.rs +++ b/hive-agent/src/mcp_config.rs @@ -49,8 +49,9 @@ fn allowed_capability_tools() -> Vec { let t = token.trim().to_ascii_lowercase(); match t.as_str() { "read_host_journal" => tools.push("get_host_journal".to_owned()), - // manage_root_agent doesn't expose a new MCP tool: it gates - // existing lifecycle tools via topology enforcement instead. + // manage_root_agent exposes no MCP tool at all: it is a + // host-side grant, enforced by hive-c0re when it computes the + // container's bind mounts. Nothing for the harness to register. "manage_root_agent" => {} unknown => { tracing::warn!(capability = %unknown, "unrecognised capability in HIVE_CAPABILITIES — skipped"); diff --git a/hive-c0re/src/agent_config/topology.rs b/hive-c0re/src/agent_config/topology.rs index 8dbff20c..894f7e53 100644 --- a/hive-c0re/src/agent_config/topology.rs +++ b/hive-c0re/src/agent_config/topology.rs @@ -4,9 +4,9 @@ //! alongside the meta `flake.nix`, so topology changes thread through //! the same git commit log as deploys. //! -//! Agent roles are stored alongside in `roles.json` as a flat map of -//! `name → [role, ...]`. Roles gate additional bind-mount grants; see -//! `lifecycle::set_nspawn_flags` for the consumer. +//! Broader-than-your-own-children bind-mount grants are **not** stored +//! here: they hang off the `ManageRootAgent` capability in +//! `capabilities.json`. See `lifecycle::set_nspawn_flags`. //! //! Format, rationale, read/reconcile/inject/surface flow, and target //! enforcement semantics: `docs/agent-lifecycle/agent-hierarchy.md::Where the tree lives`. @@ -77,30 +77,24 @@ pub fn children_of_in(topo: &BTreeMap>, name: &str) -> Ve .collect() } -/// Return every agent that has no parent in the topology. These are the -/// "top-level" agents a `can_manage_top_level_agents` role holder is -/// granted access to. No agent name is hardcoded — the set is derived -/// purely from topology structure. +/// Every agent the topology knows about, in name order. This is the set +/// a [`hive_sh4re::permissions::Capability::ManageRootAgent`] holder gets +/// bind-mounted, and it is deliberately unfiltered: that capability means +/// "may manage any agent", so the set is all of them. /// -/// In normal operation this is just the manager, but any agent the -/// operator explicitly places outside the hierarchy is also included. +/// It replaces a `top_level_agents()` that selected `parent.is_none()`. +/// With the hierarchy removed every agent is parentless, so the old +/// predicate already matched everything — keeping it would have hidden an +/// all-agents grant behind a filter that no longer filters. #[must_use] -pub fn top_level_agents() -> Vec { - top_level_agents_in(&read()) +pub fn all_agents() -> Vec { + all_agents_in(&read()) } -/// Pure form of [`top_level_agents`] for unit tests. +/// Pure form of [`all_agents`] for unit tests. #[must_use] -pub fn top_level_agents_in(topo: &BTreeMap>) -> Vec { - topo.iter() - .filter_map(|(name, parent)| { - if parent.is_none() { - Some(name.clone()) - } else { - None - } - }) - .collect() +pub fn all_agents_in(topo: &BTreeMap>) -> Vec { + topo.keys().cloned().collect() } /// Resolve a magic recipient sentinel (currently just @@ -270,16 +264,13 @@ pub fn apply_set_parent( /// but no container yet (provisioned, not yet spawned). They are KEPT /// (not dropped) so an explicit parent edge written before the first /// spawn survives until the first apply-commit, but they are NOT seeded with a -/// default parent here and NOT added to roles — that happens when the -/// container actually spawns and the name moves into `agent_names`. +/// default parent here — that happens when the container actually spawns +/// and the name moves into `agent_names`. pub fn reconcile(agent_names: &[String], pending: &[String]) -> std::io::Result { let (next, changed) = apply_reconcile(&read(), agent_names, pending); if changed { write(&next)?; } - // Keep roles in sync with the live agent set (pending agents get - // roles when they spawn). - reconcile_roles(agent_names)?; Ok(changed) } @@ -318,95 +309,6 @@ pub fn apply_reconcile( (next, changed) } -// --------------------------------------------------------------------------- -// Roles -// --------------------------------------------------------------------------- - -/// Agents with this role have every parentless agent in the topology -/// (see `top_level_agents`) added as virtual children for bind-mount -/// and config-change purposes. Enables recovery: if a top-level agent -/// is down, a role holder can still read its state and update its config. -/// -/// The manager receives this role by default on first `reconcile_roles` -/// call; operators can revoke it with `set_role`. -pub const ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS: &str = "can_manage_top_level_agents"; - -const ROLES_FILE: &str = "roles.json"; - -#[must_use] -pub fn roles_path() -> std::path::PathBuf { - crate::paths::meta_root().join(ROLES_FILE) -} - -/// Read the roles map from disk. Returns an empty map when absent or -/// unparsable — same safe-degradation pattern as `topology::read`. -#[must_use] -pub fn read_roles() -> BTreeMap> { - let Ok(raw) = std::fs::read_to_string(roles_path()) else { - return BTreeMap::new(); - }; - serde_json::from_str(&raw).unwrap_or_default() -} - -/// Persist the roles map. Sorted output keeps git diffs minimal. -pub fn write_roles(roles: &BTreeMap>) -> std::io::Result<()> { - let path = roles_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - let text = serde_json::to_string_pretty(roles) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - std::fs::write(&path, format!("{text}\n")) -} - -/// Return true when `name` holds `role`. -#[must_use] -pub fn has_role(name: &str, role: &str) -> bool { - has_role_in(&read_roles(), name, role) -} - -/// Pure form of [`has_role`] for unit tests. -#[must_use] -pub fn has_role_in(roles: &BTreeMap>, name: &str, role: &str) -> bool { - roles - .get(name) - .is_some_and(|rs| rs.iter().any(|r| r == role)) -} - -/// Reconcile `roles.json` against the current agent set: -/// - Seeds root's default `can_manage_top_level_agents` role on first -/// appearance. -/// - Drops entries for agents that no longer exist. -/// -/// Returns true when the file changed. -pub fn reconcile_roles(agent_names: &[String]) -> std::io::Result { - let mut roles = read_roles(); - let mut changed = false; - - let root = crate::lifecycle::MANAGER_NAME; - if agent_names.iter().any(|n| n == root) && !roles.contains_key(root) { - roles.insert( - root.to_owned(), - vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], - ); - changed = true; - } - - let known: std::collections::HashSet<_> = agent_names.iter().collect(); - roles.retain(|name, _| { - let keep = known.contains(name); - if !keep { - changed = true; - } - keep - }); - - if changed { - write_roles(&roles)?; - } - Ok(changed) -} - #[cfg(test)] mod tests { use super::*; @@ -678,120 +580,32 @@ mod tests { } #[test] - fn top_level_agents_in_returns_parentless_agents() { + fn all_agents_in_returns_every_name() { let topo = topo_three_level(); - // Only the manager has no parent (alice/bob/carol all have parents). - let top = top_level_agents_in(&topo); - assert_eq!(top, vec![crate::lifecycle::MANAGER_NAME]); - } - - #[test] - fn top_level_agents_in_multi_root_returns_all_parentless() { - let mut topo = topo_three_level(); - // Simulate a second parentless agent alongside the manager. - topo.insert("orphan".to_owned(), None); - let mut top = top_level_agents_in(&topo); - top.sort(); - let mut expected = vec![crate::lifecycle::MANAGER_NAME, "orphan"]; + let mut all = all_agents_in(&topo); + all.sort(); + let mut expected = vec![crate::lifecycle::MANAGER_NAME, "alice", "bob", "carol"]; expected.sort_unstable(); - assert_eq!(top, expected); + assert_eq!(all, expected); + } + + /// The set behind the `ManageRootAgent` mount grant must not depend on + /// `parent`: a capability holder manages an agent whether or not that + /// agent sits under someone. This is the assertion the old + /// `top_level_agents_in` could not have made. + #[test] + fn all_agents_in_includes_parented_agents() { + let mut topo = BTreeMap::new(); + topo.insert("alice".to_owned(), Some("bob".to_owned())); + topo.insert("bob".to_owned(), None); + let mut all = all_agents_in(&topo); + all.sort(); + assert_eq!(all, vec!["alice", "bob"]); } #[test] - fn top_level_agents_in_empty_topo_returns_empty() { + fn all_agents_in_empty_topo_returns_empty() { let topo = BTreeMap::new(); - assert!(top_level_agents_in(&topo).is_empty()); - } - - // ----------------------------------------------------------------------- - // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) - // ----------------------------------------------------------------------- - - #[test] - fn has_role_in_returns_true_when_role_held() { - let mut roles = BTreeMap::new(); - roles.insert( - "alice".to_owned(), - vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], - ); - assert!(has_role_in( - &roles, - "alice", - ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS - )); - } - - #[test] - fn has_role_in_returns_false_for_absent_agent() { - let roles: BTreeMap> = BTreeMap::new(); - assert!(!has_role_in( - &roles, - "alice", - ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS - )); - } - - #[test] - fn has_role_in_returns_false_for_empty_list() { - let mut roles = BTreeMap::new(); - roles.insert("alice".to_owned(), vec![]); - assert!(!has_role_in( - &roles, - "alice", - ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS - )); - } - - /// Revoking a role must leave the key present with an empty list so - /// `reconcile_roles` does not re-seed it. - #[test] - fn set_role_revoke_keeps_empty_entry_as_tombstone() { - let mgr = crate::lifecycle::MANAGER_NAME; - // Build an in-memory roles map as set_role would see it after granting. - let mut roles: BTreeMap> = BTreeMap::new(); - roles.insert( - mgr.to_owned(), - vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], - ); - - // Simulate the revoke path of set_role (in-memory, no disk). - let list = roles.entry(mgr.to_owned()).or_default(); - let held = list.iter().any(|r| r == ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); - assert!(held); - list.retain(|r| r != ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); - - // Key must still be present (tombstone), just with an empty list. - assert!(roles.contains_key(mgr), "empty entry must not be removed"); - assert!(roles[mgr].is_empty()); - } - - /// `reconcile_roles` must not re-seed the manager when its entry exists - /// but is empty (operator explicitly revoked the role). - #[test] - fn reconcile_roles_in_does_not_reseed_after_explicit_revoke() { - let mgr = crate::lifecycle::MANAGER_NAME; - let agent_names = [mgr.to_owned(), "alice".to_owned()]; - let mut roles: BTreeMap> = BTreeMap::new(); - // Tombstone: manager was seen before but all roles were revoked. - roles.insert(mgr.to_owned(), vec![]); - - let mgr_present = agent_names.iter().any(|n| n == mgr); - let should_seed = mgr_present && !roles.contains_key(mgr); - // should_seed must be false because manager key is present (tombstone). - assert!( - !should_seed, - "reconcile_roles must not re-seed an explicit revoke" - ); - } - - /// `reconcile_roles` seeds the manager on first appearance (no prior entry). - #[test] - fn reconcile_roles_in_seeds_root_when_absent() { - let mgr = crate::lifecycle::MANAGER_NAME; - let agent_names = [mgr.to_owned(), "alice".to_owned()]; - let roles: BTreeMap> = BTreeMap::new(); // empty - - let should_seed = agent_names.iter().any(|n| n == mgr) && !roles.contains_key(mgr); - assert!(should_seed, "reconcile_roles must seed manager when absent"); + assert!(all_agents_in(&topo).is_empty()); } } diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index faedfa1a..88b7ab75 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use hive_priv_sock::{BindMount, CredentialMount}; +use hive_sh4re::permissions::Capability; use crate::coordinator::{AgentPaths, HiveEnv}; @@ -323,18 +324,28 @@ async fn set_nspawn_flags( bind_child_agent_dirs(child, &mut binds); } - // `can_manage_top_level_agents` role: additionally mount every - // parentless agent in the topology as a virtual child. Enables - // recovery — a role holder can update those agents' configs even - // when they are down. Also grants RO access to /applied and /meta. - if crate::topology::has_role( - agent_name, - crate::topology::ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS, - ) { - let top_level = crate::topology::top_level_agents(); - for tl in &top_level { - if !direct_children.contains(tl) { - bind_child_agent_dirs(tl, &mut binds); + // `ManageRootAgent` capability: additionally mount *every* agent in + // the hive as a virtual child. Enables recovery — the holder can + // update another agent's config even when that agent is down. Also + // grants RO access to /applied and /meta. + // + // ⚠️ "every agent" reads as a widening next to the `children_of` + // mounts above, so: it is the definition of this capability, not an + // accident of how the set is computed. The grant used to hang off a + // `can_manage_top_level_agents` role and cover `top_level_agents()` + // — i.e. `parent.is_none()` — which was "everything outside the + // hierarchy". With the hierarchy gone (#4472) every agent is + // parentless, so that set *was* every agent anyway; the capability + // now says so out loud instead of deriving it from a field that no + // longer discriminates. + if crate::capabilities::has_cap(agent_name, Capability::ManageRootAgent) { + // Skipping self is a no-op, not a narrowing: `agent_notes_dir` is + // `agent_state_dir/state` and `config_bind_source` is shared, so + // binding the holder as its own virtual child reproduced the two + // own-dir mounts pushed above, byte for byte. + for other in crate::topology::all_agents() { + if other != agent_name && !direct_children.contains(&other) { + bind_child_agent_dirs(&other, &mut binds); } } // systemd-nspawn refuses to start a container whose bind diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 4f71ec35..e6c69200 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -51,12 +51,12 @@ pub struct AgentSpec { /// Stage every generated meta JSON file that exists: topology.json is /// regenerated by `reconcile` whenever the agent set changed; -/// tool-groups/capabilities/resource-limits/roles are created lazily on -/// first write (`set_groups`/`set_caps`/`set_limits`/role assignment) — -/// absent means every agent is on defaults, no file needed. Without -/// staging, an existing-but-untracked file (e.g. roles.json) shows up as -/// untracked in the meta repo, which can confuse nix's dirty-tree fetch. -/// `git add` is a no-op when content is unchanged. +/// tool-groups/capabilities/resource-limits are created lazily on +/// first write (`set_groups`/`set_caps`/`set_limits`) — absent means +/// every agent is on defaults, no file needed. Without staging, an +/// existing-but-untracked file shows up as untracked in the meta repo, +/// which can confuse nix's dirty-tree fetch. `git add` is a no-op when +/// content is unchanged. async fn stage_generated_meta_files(dir: &std::path::Path) -> Result<()> { for (path, name) in [ (crate::topology::topology_path(), "topology.json"), @@ -69,7 +69,6 @@ async fn stage_generated_meta_files(dir: &std::path::Path) -> Result<()> { crate::resource_limits::resource_limits_path(), "resource-limits.json", ), - (crate::topology::roles_path(), "roles.json"), ] { if path.exists() { git(dir, &["add", name]).await?; @@ -222,7 +221,6 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { "capabilities.json" => Some("capabilities"), "resource-limits.json" => Some("resource-limits"), "tool-groups.json" => Some("tool-groups"), - "roles.json" => Some("roles"), _ => None, }) .collect(); diff --git a/hive-sh4re/src/permissions.rs b/hive-sh4re/src/permissions.rs index d03515ad..6c80bb7d 100644 --- a/hive-sh4re/src/permissions.rs +++ b/hive-sh4re/src/permissions.rs @@ -367,11 +367,20 @@ impl ToolGroup { #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub enum Capability { - /// Agent can lifecycle-manage the root agent (kill/start/restart) - /// on behalf of the hive when the root has crashed. Named capability - /// for the existing manager privilege — future topology enforcement - /// will gate this via the capability system instead of the hardcoded - /// `container == MANAGER_CONTAINER` check. + /// Agent may manage *any* agent in the hive, not just its own + /// children. Concretely, hive-c0re bind-mounts every agent's state + /// dir (RW) and config dir (RO) into the holder's container, plus + /// `/applied` and `/meta` read-only, so the holder can recover an + /// agent that is down. See `lifecycle::host_config::set_nspawn_flags`. + /// + /// The name is historical: it was minted when the hive had a parent/ + /// child topology and "manage the root agent" was the widest grant + /// there was. With the hierarchy removed there is no privileged root + /// to single out, so the same grant now means "manage any agent". + /// Kept under the old spelling deliberately — renaming it would turn + /// every `manage_root_agent` already stored in `capabilities.json` + /// into an unrecognised name that `capabilities::prune_unknown` + /// silently drops. ManageRootAgent, /// Agent can read the full host journal via `GetHostJournal`. /// hive-c0re checks this capability before running journalctl. @@ -390,7 +399,7 @@ impl Capability { pub fn description(self) -> &'static str { match self { Self::ManageRootAgent => { - "lifecycle-manage the root/manager agent on hive crash recovery" + "manage any agent: every agent's state (rw) and config (ro) mounted for recovery" } Self::ReadHostJournal => "read host journald via get_host_journal MCP tool", }