diff --git a/docs/approvals.md b/docs/approvals.md index b48eefd4..05ca03dd 100644 --- a/docs/approvals.md +++ b/docs/approvals.md @@ -362,7 +362,7 @@ per container row. ## Two repos per agent ``` -/var/lib/hyperhive/agents//config/ proposed — submitting agent RW +/var/lib/hyperhive/agents//config/ proposed — parent mount is RO └── # any files the submitting # agent wants in the commit. # agent.nix is the @@ -490,12 +490,17 @@ approval card. See `docs/web-ui.md`. ### Submitting agent's view of config repos -Every parent agent's container has its **direct children's** proposed -config repos bind-mounted read-write (topology-driven: `lifecycle.rs` -calls `bind_child_agent_dirs` for each entry in -`topology::children_of(agent_name)`). An agent with the `approvals` -tool group can therefore edit, commit, and submit changes for any of -its direct children directly inside its container at `/agents//config/`. +Every parent agent's container has its **direct children's** config +repos bind-mounted **read-only** (topology-driven: `lifecycle.rs` calls +`bind_child_agent_dirs` for each entry in +`topology::children_of(agent_name)`). It is a copy to *read* a child's +current config — not an editing surface. + +An agent with the `approvals` tool group submits a change the same way +any other change is made: **clone the child's config repo from the +forge into its own state dir, commit on a branch, open a PR**, and let +the operator review and approve it. There is deliberately no second, +mount-shaped path that reaches the same file without the review. Agents holding the `can_manage_top_level_agents` topology role (defined as `ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS` in `hive-c0re/src/agent_config/topology.rs`) diff --git a/docs/persistence.md b/docs/persistence.md index 770567b3..7024b115 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -314,12 +314,12 @@ Under `/var/lib/hyperhive/agents//`: ### Parent access to child state -A parent agent gets each direct child's `state` and `config` dirs -bind-mounted **read-write** (`bind_child_agent_dirs` in -`lifecycle/host_config.rs`). The RW on `state` is deliberate, not an -oversight: 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. +A parent agent gets each direct child's `state` dir bind-mounted +**read-write** and its `config` dir **read-only** +(`bind_child_agent_dirs` in `lifecycle/host_config.rs`). The RW on +`state` is deliberate, not an oversight: 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. **`harness` is not mounted at all.** It holds the child's own runtime material — `bash-tasks/`, the turn-stats and event sqlite dbs — and @@ -329,11 +329,18 @@ never an argument so much as the side-effect of one loop treating all three dirs alike. hive-c0re reads a child's harness dir **directly on the host** when it wants those stats, which needs no mount into the parent. -`config` is still RW, and **nothing justifies that**: a config change is -a PR on the child's config repo, made from a clone, so the bind-mounted -`config` dir is a read-only *copy* for reading a child's config — not a -tree anyone edits in place. Narrowing it is tracked separately, and -depends on relocating where `InitConfig` seeds. +**`config` is read-only, including for the parent.** A config change is +a PR on the child's config repo, made from a clone and merged after +review — so the bind-mounted `config` dir is a *copy to read*, never a +tree anyone edits in place. Mounting it writable would leave a second +path to the same file that skips the review entirely, which makes the +boundary a convention rather than a permission. + +⚠️ Not to be confused with the seeding done when an `InitConfig` +approval resolves: that writes the child's initial config repo as +**hive-c0re, against the host path**, and `read_only` on a bind +constrains writers *inside* a container only. The two are unrelated, and +reading them as the same thing is what kept this mount writable. Per-child isolation still holds: a container only ever has its *own* dirs plus its direct children's bind-mounted, never a sibling's. diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index b3203627..8d5ac725 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -71,37 +71,46 @@ async fn systemd_daemon_reload() -> Result<()> { /// inside the container. pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied"; -/// Append bind flags for `child`'s state and config dirs into `binds`, -/// read-write. See docs/persistence.md ("Parent access to child state") -/// for what a parent may touch and why. Creates missing host-side -/// directories so nspawn doesn't refuse to start; missing dirs are -/// non-fatal. +/// Append bind flags for `child`'s state and config dirs into `binds`. +/// See docs/persistence.md ("Parent access to child state") for what a +/// parent may touch and why. Creates missing host-side directories so +/// nspawn doesn't refuse to start; missing dirs are non-fatal. /// -/// **`harness` is deliberately absent.** It holds the child's own runtime -/// material — `bash-tasks/`, turn-stats and event sqlite dbs — none of -/// which a parent has a stated reason to read, let alone write. It was -/// mounted only because the loop treated all three dirs alike; the three -/// have three different answers, and the uniformity is what hid that. -/// hive-c0re still reads a child's harness dir directly on the host -/// (`stats::hive_stats`), which needs no bind mount into the parent. +/// **Three dirs, three different answers** — the uniformity of the +/// original loop is what hid that: /// -/// `config` is RW here **pending** the sequenced change: the ruling is -/// that it becomes read-only, but `request_init_config` still has the -/// manager seed a new child's config in place, so flipping the mount -/// before relocating that step breaks agent creation hive-wide. +/// - `state` is **read-write**: a parent reads and writes a child's notes +/// to recover it, which is the one case that needs to work while the +/// child is down. +/// - `config` is **read-only**, and read-only for the *parent* is the +/// point: a config change is a PR against the child's repo on the +/// forge, reviewed and merged, never an edit in place. A writable mount +/// here is a second path to the same file that skips the review — the +/// boundary would then be a convention rather than a permission. +/// - `harness` is **absent entirely**. It holds the child's own runtime +/// material — `bash-tasks/`, turn-stats and event sqlite dbs — none of +/// which a parent has a stated reason to read, let alone write. +/// hive-c0re still reads it directly on the host (`stats::hive_stats`), +/// which needs no bind mount into the parent. +/// +/// ⚠️ The seeding done at `InitConfig` approval is **not** affected by the +/// `config` flag and must not be read as a reason to widen it: that runs +/// as hive-c0re against the host path (see `actions.rs`, which seeds the +/// repo and wires its forge remote inline), and `read_only` on a bind +/// constrains writers *inside* the container only. fn bind_child_agent_dirs(child: &str, binds: &mut Vec) { let Ok(child) = hive_types::Ident::parse(child) else { tracing::warn!(%child, "skipping child bind: invalid agent name"); return; }; let child_root = crate::paths::agent_state_dir(&child); - for sub in ["state", "config"] { + for (sub, read_only) in [("state", false), ("config", true)] { let host = child_root.join(sub); let _ = std::fs::create_dir_all(&host); binds.push(BindMount { host_path: host.to_string_lossy().into_owned(), container_path: format!("/agents/{child}/{sub}"), - read_only: false, + read_only, }); } } @@ -252,9 +261,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 (parent reads + - // writes child state for recovery, and manages config). See - // `bind_child_agent_dirs`. + // its state dir RW and its config dir RO. See `bind_child_agent_dirs` + // for why each is what it is. let direct_children = crate::topology::children_of(agent_name); for child in &direct_children { bind_child_agent_dirs(child, &mut binds);