From 2979fcf5d567deabfd7cf114aee7efc8f6840e7a Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 11 Sep 2026 20:19:47 +0200 Subject: [PATCH] swarm-secret-client: give the store one namespace instead of one prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The crate had a single path convention and it was per-agent: `swarm/agents//matrix/`. The secrets still to move into the store do not fit it — one belongs to a hive, one to a swarm service, one to the controller itself — so each would have picked its own shape, and each would have been a separate grant to get wrong. mara ruled the scheme on the epic: `swarm///`, over `agents`, `hives`, `services` and `controller`. This lands it. `Kind` is an enum rather than free strings for one reason: the store's grant is written in nix and cannot be reached from Rust, so a misspelled kind is a 403 at provision time and not a compile error. `Kind::ALL` lets a test enumerate the set instead of restating it, which is what makes adding a kind a deliberate edit rather than an accidental grant. Note `Kind` sits beside `checked_segment`'s existing `kind` argument, which means something else entirely — the label of the name being validated. They are not the same concept and should not be merged. Nothing about the rendered policy changes. `policy::render` still grants read on the agent kind alone; the other kinds are absent on purpose, because what a hive may read of its own kind is a boundary question and not a consequence of the namespace growing. The controller's write grant likewise stays scoped to `agents/` — it widens when a path outside it gains a writer, not when the kinds are declared. Verified: `cargo test -p swarm-secret-client` 23 passed, 0 failed. The two tests pinning the rendered strings (`the_document_grants_read_over_the_whole_agent_prefix` and matrix's path assertion) still assert the same literals they did before, which is what shows this is a faithful port rather than a reshape. `nix fmt` 710 emitted, 10 formatted, 0 changed; the three scripts/check-*.sh lints pass with the change staged. No reference to the removed `path::AGENT_PREFIX` survives in the crate or in nix — checked with a scoped pattern, because the unqualified name also belongs to hive-host-sock's container prefix and greps for it are answering a different question. --- nix/module-eval.nix | 10 ++- swarm-secret-client/src/lib.rs | 5 +- swarm-secret-client/src/matrix.rs | 10 ++- swarm-secret-client/src/path.rs | 117 +++++++++++++++++++++++++++--- swarm-secret-client/src/policy.rs | 12 ++- 5 files changed, 135 insertions(+), 19 deletions(-) diff --git a/nix/module-eval.nix b/nix/module-eval.nix index 74b04cc9..7eb5dcd5 100644 --- a/nix/module-eval.nix +++ b/nix/module-eval.nix @@ -747,9 +747,13 @@ let # The policy authorising this route lives in another file, and nothing # else relates the grants to the paths the code actually writes. # - # `secret/data/` is KV v2's ACL prefix; `swarm/agents` is - # `swarm_secret_client::path::AGENT_PREFIX`, whose value that crate - # pins in its own test. + # `secret/data/` is KV v2's ACL prefix; `swarm` is + # `swarm_secret_client::path::ROOT` and `agents` is + # `Kind::Agent.as_str()`, both of which that crate pins in its own test. + # + # The grant is still the agent kind alone because nothing writes another + # one yet. It widens when a path outside `agents/` gains a writer, not + # when the kinds are declared. name = "the controller may write agent credentials, and only under the agent prefix"; ok = let diff --git a/swarm-secret-client/src/lib.rs b/swarm-secret-client/src/lib.rs index d54d5d68..baf77e10 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -32,7 +32,10 @@ pub enum Error { /// meant. See [`path`]. #[error("{kind} name {value:?} is not a single path segment of [A-Za-z0-9_-]")] PathSegment { - /// Which name was rejected — `agent` or `account`. + /// Which name was rejected. A principal's kind in the singular + /// (`agent`, `hive`, `service`, `controller`) when the name addresses + /// one, or what the name is to the secret otherwise — `account`, for + /// a matrix credential. kind: &'static str, /// The offending value, quoted in the message because the caller /// usually got it from config and needs to see which one. diff --git a/swarm-secret-client/src/matrix.rs b/swarm-secret-client/src/matrix.rs index 19469b29..0a640b9b 100644 --- a/swarm-secret-client/src/matrix.rs +++ b/swarm-secret-client/src/matrix.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use crate::{ Error, - path::{AGENT_PREFIX, checked_segment}, + path::{Kind, checked_segment, principal_prefix}, }; /// The path holding `agent`'s token for the external matrix account `account`. @@ -21,9 +21,9 @@ use crate::{ /// `[A-Za-z0-9_-]`, which is what keeps one agent's name from addressing /// another agent's secret. pub fn account_path(agent: &str, account: &str) -> Result { - checked_segment("agent", agent)?; + let prefix = principal_prefix(Kind::Agent, agent)?; checked_segment("account", account)?; - Ok(format!("{AGENT_PREFIX}/{agent}/matrix/{account}")) + Ok(format!("{prefix}/matrix/{account}")) } /// What an account's path holds: the token, plus the homeserver it belongs to. @@ -59,8 +59,10 @@ mod tests { #[test] fn a_well_formed_pair_lands_under_the_agent_prefix() { let p = account_path("atlas", "ops-relay").expect("both segments are legal"); + // Spelled out rather than rebuilt from the same pieces the code uses: + // a test that composes `ROOT` and `Kind::Agent` would keep passing + // through a rename that moves every stored credential. assert_eq!(p, "swarm/agents/atlas/matrix/ops-relay"); - assert!(p.starts_with(AGENT_PREFIX)); } #[test] diff --git a/swarm-secret-client/src/path.rs b/swarm-secret-client/src/path.rs index 0bf04309..5efa7f8c 100644 --- a/swarm-secret-client/src/path.rs +++ b/swarm-secret-client/src/path.rs @@ -15,8 +15,73 @@ use crate::Error; /// be two ways to say one thing. pub const MOUNT: &str = "secret"; -/// The prefix under [`MOUNT`] owned by per-agent credentials. -pub const AGENT_PREFIX: &str = "swarm/agents"; +/// The root under [`MOUNT`] that every swarm secret lives beneath. +pub const ROOT: &str = "swarm"; + +/// Whose secret it is — the second segment of every path. +/// +/// An enum rather than free strings so the set is closed: the store's grant is +/// written against these segments and cannot be reached from Rust, so a +/// misspelled kind is a 403 at provision time rather than anything a compiler +/// sees. [`Kind::ALL`] exists so a test can enumerate the set instead of +/// restating it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Kind { + /// One agent container's own secrets. + Agent, + /// One hive's secrets, held on behalf of whatever runs there. An identity + /// minted per hive rather than per agent lands here even though an agent + /// is what uses it. + Hive, + /// One swarm service — the things a swarm runs beside the controller. + Service, + /// The controller itself. There is one per swarm, so the name segment + /// below it does not vary; the shape stays uniform anyway, because one + /// grant pattern over `/` is cheaper than a special case. + Controller, +} + +impl Kind { + /// Every kind, so callers that must cover the whole set can iterate rather + /// than restate it — a second list is a list that drifts. + pub const ALL: [Kind; 4] = [Kind::Agent, Kind::Hive, Kind::Service, Kind::Controller]; + + /// The path segment, which is also what the store's grant is written + /// against. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Kind::Agent => "agents", + Kind::Hive => "hives", + Kind::Service => "services", + Kind::Controller => "controller", + } + } + + /// What to call the name in an error — singular, because the message reads + /// "hive name ... is not a single path segment". + #[must_use] + pub const fn label(self) -> &'static str { + match self { + Kind::Agent => "agent", + Kind::Hive => "hive", + Kind::Service => "service", + Kind::Controller => "controller", + } + } +} + +/// `swarm//` — everything one principal owns, and the only way to +/// build the head of a path. +/// +/// # Errors +/// [`Error::PathSegment`] when `name` is not a single segment of +/// `[A-Za-z0-9_-]`, which is what keeps one principal's name from addressing +/// another's secrets. +pub fn principal_prefix(kind: Kind, name: &str) -> Result { + checked_segment(kind.label(), name)?; + Ok(format!("{ROOT}/{}/{name}", kind.as_str())) +} /// A path segment that cannot change the path's shape. /// @@ -100,13 +165,47 @@ mod tests { } #[test] - fn the_bao_policy_grants_exactly_these_two_values() { - // Renaming either constant is a silent 403 at provision time, not a - // compile error: the controller's grant spells them out in - // `nix/host-modules/swarm-bao.nix` (`controllerPolicyText`), which no - // Rust change can reach. Editing here means editing there, and - // `nix/module-eval.nix` asserts the other side of the same pair. + fn the_bao_policy_is_written_against_exactly_these_segments() { + // Renaming any of these is a silent 403 at provision time, not a + // compile error: the controller's grant spells the mount and root out + // in `nix/host-modules/swarm-bao.nix` (`controllerPolicyText`), which + // no Rust change can reach. Editing here means editing there, and + // `nix/module-eval.nix` asserts the other side. assert_eq!(MOUNT, "secret"); - assert_eq!(AGENT_PREFIX, "swarm/agents"); + assert_eq!(ROOT, "swarm"); + + // Iterated, not listed: the grant covers `/*`, so a kind added + // without a segment here would be granted by accident rather than by + // decision. Spelling each one out is what makes adding a kind a + // deliberate edit. + let segments: Vec<&str> = Kind::ALL.iter().map(|k| k.as_str()).collect(); + assert_eq!(segments, ["agents", "hives", "services", "controller"]); + } + + #[test] + fn a_kind_cannot_share_a_segment_or_a_label_with_another() { + // Two kinds resolving to one segment would silently merge two + // principals' secrets into one directory; two sharing a label would + // make the error name the wrong one. + for (i, a) in Kind::ALL.iter().enumerate() { + for b in &Kind::ALL[i + 1..] { + assert_ne!(a.as_str(), b.as_str(), "{a:?} and {b:?} share a segment"); + assert_ne!(a.label(), b.label(), "{a:?} and {b:?} share a label"); + } + } + } + + #[test] + fn a_principal_prefix_refuses_a_name_that_would_escape_it() { + // The control for the arm below: a well-formed name really does build. + assert_eq!( + principal_prefix(Kind::Hive, "alpha").expect("a plain name is legal"), + "swarm/hives/alpha" + ); + let e = principal_prefix(Kind::Hive, "../atlas").expect_err("a traversal is not"); + assert!( + matches!(e, Error::PathSegment { kind, .. } if kind == "hive"), + "the error must name the principal in the singular, got {e:?}" + ); } } diff --git a/swarm-secret-client/src/policy.rs b/swarm-secret-client/src/policy.rs index de45771e..cd796346 100644 --- a/swarm-secret-client/src/policy.rs +++ b/swarm-secret-client/src/policy.rs @@ -17,7 +17,7 @@ use crate::{ Error, - path::{AGENT_PREFIX, MOUNT, checked_segment}, + path::{Kind, MOUNT, ROOT, checked_segment}, }; /// Namespace for a hive's own policy and cert-auth role. @@ -45,9 +45,17 @@ pub fn hive_object_name(hive: &str) -> Result { /// object rather than derived state with a re-emission to get wrong. /// /// Read-only: the controller mints these and never reads one back. +/// ⚠️ Still the agent kind alone. The other kinds are deliberately absent: a +/// hive has no business reading a service's or the controller's credentials, +/// and what a hive may read of its *own* kind is a boundary question this +/// module's header answers only for agents. Widening it is a decision, not a +/// consequence of the namespace growing. #[must_use] pub fn render() -> String { - format!("path \"{MOUNT}/data/{AGENT_PREFIX}/*\" {{\n capabilities = [\"read\"]\n}}\n") + format!( + "path \"{MOUNT}/data/{ROOT}/{}/*\" {{\n capabilities = [\"read\"]\n}}\n", + Kind::Agent.as_str() + ) } #[cfg(test)]