collapse the roles.json mount grant into the ManageRootAgent capability
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.
This commit is contained in:
atlas
mara
parent
5ec0ce90fd
commit
4f6407fdea
8 changed files with 98 additions and 260 deletions
|
|
@ -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<String, Option<String>>, 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<String> {
|
||||
top_level_agents_in(&read())
|
||||
pub fn all_agents() -> Vec<String> {
|
||||
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<String, Option<String>>) -> Vec<String> {
|
||||
topo.iter()
|
||||
.filter_map(|(name, parent)| {
|
||||
if parent.is_none() {
|
||||
Some(name.clone())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
pub fn all_agents_in(topo: &BTreeMap<String, Option<String>>) -> Vec<String> {
|
||||
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<bool> {
|
||||
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<String, Vec<String>> {
|
||||
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<String, Vec<String>>) -> 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<String, Vec<String>>, 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<bool> {
|
||||
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<String, Vec<String>> = 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<String, Vec<String>> = 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<String, Vec<String>> = 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<String, Vec<String>> = 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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
Loading…
Reference in a new issue