diff --git a/docs/persistence.md b/docs/persistence.md index fd870523..770567b3 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -314,19 +314,29 @@ Under `/var/lib/hyperhive/agents//`: ### Parent access to child state -A parent agent gets each direct child's `state`, `harness`, and -`config` dirs bind-mounted **read-write** (`bind_child_agent_dirs` in -`lifecycle.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. `config` and `harness` are RW too, but **nothing -justifies that for `config`**: 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. Per-child isolation still holds: a container only ever has -its *own* dirs plus its direct children's bind-mounted, never a -sibling's. +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. + +**`harness` is not mounted at all.** It holds the child's own runtime +material — `bash-tasks/`, the turn-stats and event sqlite dbs — and +nothing argues for a parent reading it, let alone writing it. It used to +be mounted RW for "the same management reasons" as `state`, which was +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. + +Per-child isolation still holds: a container only ever has its *own* +dirs plus its direct children's bind-mounted, never a sibling's. Under `/var/lib/hyperhive/applied//` — the hive-c0re-only applied repo. Tracks `flake.nix` (module-only boilerplate; never diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index 7a7c76a5..b3203627 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -71,18 +71,31 @@ async fn systemd_daemon_reload() -> Result<()> { /// inside the container. pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied"; -/// Append bind flags for `child`'s state, harness, and config dirs into -/// `binds`, all read-write. The RW on `state` is deliberate (recovery), -/// not an oversight; see docs/persistence.md ("Parent access to child -/// state") for the rationale. 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`, +/// 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. +/// +/// **`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. +/// +/// `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. 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", "harness", "config"] { + for sub in ["state", "config"] { let host = child_root.join(sub); let _ = std::fs::create_dir_all(&host); binds.push(BindMount { @@ -338,3 +351,55 @@ async fn set_nspawn_flags( // Delegate the actual conf-file rewrite to hive-priv (runs as root). crate::priv_client::write_nspawn_flags(container, &binds, isolation, &load_creds).await } + +#[cfg(test)] +mod tests { + use super::{BindMount, bind_child_agent_dirs}; + + fn child_binds() -> Vec { + let mut binds = Vec::new(); + bind_child_agent_dirs("kiddo", &mut binds); + binds + } + + /// The boundary, asserted as a whole rather than per-dir: a parent + /// sees a child's `state` and `config`, and nothing else. + #[test] + fn parent_sees_only_child_state_and_config() { + let paths: Vec = child_binds() + .into_iter() + .map(|b| b.container_path) + .collect(); + assert_eq!(paths, ["/agents/kiddo/state", "/agents/kiddo/config"]); + } + + /// The regression this exists for. `harness` holds the child's own + /// runtime material and was only ever mounted because one loop + /// treated all three dirs alike — re-adding it to that loop is a + /// one-word change that nothing else would catch. + #[test] + fn parent_never_sees_a_child_harness_dir() { + for bind in child_binds() { + assert!( + !bind.container_path.contains("harness"), + "child harness must not be bound into a parent: {}", + bind.container_path + ); + assert!( + !bind.host_path.contains("harness"), + "child harness must not be bound into a parent: {}", + bind.host_path + ); + } + } + + /// An unparseable name yields no mounts at all — the guard must fail + /// closed, since the alternative is a path built from unvalidated + /// input. + #[test] + fn an_invalid_child_name_binds_nothing() { + let mut binds = Vec::new(); + bind_child_agent_dirs("../escape", &mut binds); + assert!(binds.is_empty()); + } +}