fix(3044): a parent's mount of a child's config is read-only
The parent's copy is for reading a child's config; a change to it is a PR
on the child's repo, made from a clone and merged after review. A
writable mount is a second path to the same file that skips that review,
which makes the boundary a convention rather than a permission.
Confirmed with ruth before flipping: it clones from the forge and opens a
PR, including for a brand-new child's first config.
The prose was the larger half. docs/approvals.md did not merely describe
the old mount, it *instructed* agents to use it ("can therefore edit,
commit, and submit changes for any of its direct children directly inside
its container"), and the doc comment in host_config.rs asserted a
dependency that never existed: the InitConfig seed runs as hive-c0re
against the host path, and read_only on a bind constrains writers inside
the container only. That comment is what produced issue #3206, now closed
as invalid.
This commit is contained in:
parent
d03549c40f
commit
0b6b3b755d
3 changed files with 59 additions and 39 deletions
|
|
@ -362,7 +362,7 @@ per container row.
|
||||||
## Two repos per agent
|
## Two repos per agent
|
||||||
|
|
||||||
```
|
```
|
||||||
/var/lib/hyperhive/agents/<name>/config/ proposed — submitting agent RW
|
/var/lib/hyperhive/agents/<name>/config/ proposed — parent mount is RO
|
||||||
└── <anything> # any files the submitting
|
└── <anything> # any files the submitting
|
||||||
# agent wants in the commit.
|
# agent wants in the commit.
|
||||||
# agent.nix is the
|
# agent.nix is the
|
||||||
|
|
@ -490,12 +490,17 @@ approval card. See `docs/web-ui.md`.
|
||||||
|
|
||||||
### Submitting agent's view of config repos
|
### Submitting agent's view of config repos
|
||||||
|
|
||||||
Every parent agent's container has its **direct children's** proposed
|
Every parent agent's container has its **direct children's** config
|
||||||
config repos bind-mounted read-write (topology-driven: `lifecycle.rs`
|
repos bind-mounted **read-only** (topology-driven: `lifecycle.rs` calls
|
||||||
calls `bind_child_agent_dirs` for each entry in
|
`bind_child_agent_dirs` for each entry in
|
||||||
`topology::children_of(agent_name)`). An agent with the `approvals`
|
`topology::children_of(agent_name)`). It is a copy to *read* a child's
|
||||||
tool group can therefore edit, commit, and submit changes for any of
|
current config — not an editing surface.
|
||||||
its direct children directly inside its container at `/agents/<child>/config/`.
|
|
||||||
|
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
|
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`)
|
(defined as `ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS` in `hive-c0re/src/agent_config/topology.rs`)
|
||||||
|
|
|
||||||
|
|
@ -314,12 +314,12 @@ Under `/var/lib/hyperhive/agents/<name>/`:
|
||||||
|
|
||||||
### Parent access to child state
|
### Parent access to child state
|
||||||
|
|
||||||
A parent agent gets each direct child's `state` and `config` dirs
|
A parent agent gets each direct child's `state` dir bind-mounted
|
||||||
bind-mounted **read-write** (`bind_child_agent_dirs` in
|
**read-write** and its `config` dir **read-only**
|
||||||
`lifecycle/host_config.rs`). The RW on `state` is deliberate, not an
|
(`bind_child_agent_dirs` in `lifecycle/host_config.rs`). The RW on
|
||||||
oversight: a parent manages its children, which includes writing into a
|
`state` is deliberate, not an oversight: a parent manages its children,
|
||||||
child's state for recovery (e.g. seeding notes, clearing a stuck
|
which includes writing into a child's state for recovery (e.g. seeding
|
||||||
sentinel) as well as reading it.
|
notes, clearing a stuck sentinel) as well as reading it.
|
||||||
|
|
||||||
**`harness` is not mounted at all.** It holds the child's own runtime
|
**`harness` is not mounted at all.** It holds the child's own runtime
|
||||||
material — `bash-tasks/`, the turn-stats and event sqlite dbs — and
|
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
|
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.
|
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
|
**`config` is read-only, including for the parent.** A config change is
|
||||||
a PR on the child's config repo, made from a clone, so the bind-mounted
|
a PR on the child's config repo, made from a clone and merged after
|
||||||
`config` dir is a read-only *copy* for reading a child's config — not a
|
review — so the bind-mounted `config` dir is a *copy to read*, never a
|
||||||
tree anyone edits in place. Narrowing it is tracked separately, and
|
tree anyone edits in place. Mounting it writable would leave a second
|
||||||
depends on relocating where `InitConfig` seeds.
|
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*
|
Per-child isolation still holds: a container only ever has its *own*
|
||||||
dirs plus its direct children's bind-mounted, never a sibling's.
|
dirs plus its direct children's bind-mounted, never a sibling's.
|
||||||
|
|
|
||||||
|
|
@ -71,37 +71,46 @@ async fn systemd_daemon_reload() -> Result<()> {
|
||||||
/// inside the container.
|
/// inside the container.
|
||||||
pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied";
|
pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied";
|
||||||
|
|
||||||
/// Append bind flags for `child`'s state and config dirs into `binds`,
|
/// Append bind flags for `child`'s state and config dirs into `binds`.
|
||||||
/// read-write. See docs/persistence.md ("Parent access to child state")
|
/// See docs/persistence.md ("Parent access to child state") for what a
|
||||||
/// for what a parent may touch and why. Creates missing host-side
|
/// parent may touch and why. Creates missing host-side directories so
|
||||||
/// directories so nspawn doesn't refuse to start; missing dirs are
|
/// nspawn doesn't refuse to start; missing dirs are non-fatal.
|
||||||
/// non-fatal.
|
|
||||||
///
|
///
|
||||||
/// **`harness` is deliberately absent.** It holds the child's own runtime
|
/// **Three dirs, three different answers** — the uniformity of the
|
||||||
/// material — `bash-tasks/`, turn-stats and event sqlite dbs — none of
|
/// original loop is what hid that:
|
||||||
/// 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
|
/// - `state` is **read-write**: a parent reads and writes a child's notes
|
||||||
/// that it becomes read-only, but `request_init_config` still has the
|
/// to recover it, which is the one case that needs to work while the
|
||||||
/// manager seed a new child's config in place, so flipping the mount
|
/// child is down.
|
||||||
/// before relocating that step breaks agent creation hive-wide.
|
/// - `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<BindMount>) {
|
fn bind_child_agent_dirs(child: &str, binds: &mut Vec<BindMount>) {
|
||||||
let Ok(child) = hive_types::Ident::parse(child) else {
|
let Ok(child) = hive_types::Ident::parse(child) else {
|
||||||
tracing::warn!(%child, "skipping child bind: invalid agent name");
|
tracing::warn!(%child, "skipping child bind: invalid agent name");
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
let child_root = crate::paths::agent_state_dir(&child);
|
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 host = child_root.join(sub);
|
||||||
let _ = std::fs::create_dir_all(&host);
|
let _ = std::fs::create_dir_all(&host);
|
||||||
binds.push(BindMount {
|
binds.push(BindMount {
|
||||||
host_path: host.to_string_lossy().into_owned(),
|
host_path: host.to_string_lossy().into_owned(),
|
||||||
container_path: format!("/agents/{child}/{sub}"),
|
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
|
// Topology-driven child mounts: every direct child of this agent gets
|
||||||
// its state, harness, and config dirs bind-mounted RW (parent reads +
|
// its state dir RW and its config dir RO. See `bind_child_agent_dirs`
|
||||||
// writes child state for recovery, and manages config). See
|
// for why each is what it is.
|
||||||
// `bind_child_agent_dirs`.
|
|
||||||
let direct_children = crate::topology::children_of(agent_name);
|
let direct_children = crate::topology::children_of(agent_name);
|
||||||
for child in &direct_children {
|
for child in &direct_children {
|
||||||
bind_child_agent_dirs(child, &mut binds);
|
bind_child_agent_dirs(child, &mut binds);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue