swarm: name the agent client after its hive, not after "agent"

`agent-<hive>` reads as "the agent named <hive>" — which is the one thing that
identity does not carry, since it is minted per hive. It becomes
`hive-<hive>-agent`: the hive's own id, extended.

The rename is not a string swap. `hive-foo-agent` satisfies the hive parse too
(it strips to a hive named `foo-agent`), so the responder's agent rule now runs
BEFORE its hive rule — most specific wins. Hive-first would have handed every
agent its hive's grant, including writing that hive's status key, with nothing
to report it: the client authenticates and is merely able to do more than it
should.

`Policy::new`'s overlap check goes with the prefix it was written for. The
invariant the suffix form needs instead is that the suffix is non-empty: an
empty one makes `strip_suffix` succeed on every hive id, so the two principals
become one string and whichever arm runs first answers for both.

The suffix form also introduces a collision the prefix form did not have: a hive
genuinely named `foo-agent` mints `hive-foo-agent`, which is hive `foo`'s agent
id. The responder cannot see it — it has no roster, deliberately — so
`swarm-authelia.nix` asserts at eval that no hive name ends with the suffix. The
existing duplicate-id assertion does not cover this: it fires only when both
`foo` and `foo-agent` are on the roster, and with `foo-agent` alone there is no
duplicate, just a hive quietly receiving its agents' grant.

A test written by analogy with `the_prefix_alone_names_no_hive` failed, correctly
— `hive--agent` is a hive named `-agent` under the hive parse, which this module
cannot rule out. It now asserts only the part this module owns: no empty hive
name is ever expanded into a subject.

nix/host-modules/swarm-authelia.nix

@@ -156,17 +156,24 @@ let
   # stop being static, which is the same problem the users-database writer
   # already solves for identities.
   #
+  # 🏷️ The id EXTENDS the hive's own (`hive-<name>-agent`) rather than taking a
+  # prefix of its own (`agent-<name>`), because that reads as *the agent called
+  # `<name>`* — which is the one thing this identity does not carry. The cost is
+  # that the two ids are no longer distinguishable by prefix, so the responder's
+  # agent rule must be tried BEFORE its hive rule; `policy.rs` says so at the
+  # match site, and the assertion below covers the case that ordering cannot.
+  #
   # Mirrors `hiveClients` field for field so the two stay comparable. The
   # signing algorithm is not load-bearing here — this client's only consumer is
   # the queue's auth-callout responder, which *introspects* rather than
   # verifying offline — but it matches its sibling rather than inventing a
   # second answer to a question nobody asked.
   agentClients = lib.mapAttrsToList (name: _: {
-    id = "${cfg.agentClientPrefix}${name}";
+    id = "${cfg.hiveClientPrefix}${name}${cfg.agentClientSuffix}";
     description = "HyperHive agents on hive ${name}";
     kind = "machine";
     redirectUris = [ ];
-    audience = [ "${cfg.agentClientPrefix}${name}" ];
+    audience = [ "${cfg.hiveClientPrefix}${name}${cfg.agentClientSuffix}" ];
     accessTokenSignedResponseAlg = "RS256";
   }) hyperhiveCfg.swarm.hives;
 
@@ -446,7 +453,7 @@ in
         Mint machine clients per hive in
         {option}`services.hyperhive.swarm.hives`, so each hive can
         authenticate to swarm services as itself: `hive-<name>` for the
-        hive's own daemons, and `agent-<name>` for the agent containers
+        hive's own daemons, and `hive-<name>-agent` for the agent containers
         running on it.
 
         Two clients rather than one because they are not the same
@@ -706,19 +713,22 @@ in
       '';
     };
 
-    agentClientPrefix = lib.mkOption {
+    agentClientSuffix = lib.mkOption {
       type = lib.types.str;
       readOnly = true;
-      default = "agent-";
+      default = "-agent";
       description = ''
-        Prefix of the OAuth2 client id minted for the *agents* of each hive
+        Suffix appended to a hive's own client id to name the client its
-        in `services.hyperhive.swarm.hives` — agents on hive `alpha` all
+        *agent containers* present — agents on hive `alpha` all present
-        present
+        `${config.services.hyperhive.swarm.authelia.hiveClientPrefix}alpha${config.services.hyperhive.swarm.authelia.agentClientSuffix}`.
-        `${config.services.hyperhive.swarm.authelia.agentClientPrefix}alpha`.
         Read-only for the same reason as `hiveClientPrefix`, and read by the
         same consumer with the same failure mode: a split spelling denies
         every agent as a timeout.
 
+        A suffix on the hive's id rather than a prefix of its own, because
+        `agent-alpha` reads as *the agent named alpha* — which is precisely
+        what this identity does not say.
+
         One id per hive rather than per agent, because agents are created at
         runtime and a per-agent client would make creating one a config
         change plus a reload. The consequence is that this identity says
@@ -937,6 +947,38 @@ in
             + "services.hyperhive.swarm.hives; rename the hive or the "
             + "colliding client.";
         }
+        {
+          # A hive whose name ENDS with the agent suffix mints an id the
+          # queue's responder reads as somebody else's agents:
+          # `hive-foo-agent` parses as *the agents of hive foo* before it
+          # parses as *the hive foo-agent*, because the agent rule is the
+          # more specific one and therefore runs first.
+          #
+          # ⚠️ NOT covered by the duplicate-id assertion above, and the gap is
+          # the interesting half. That one fires only when BOTH `foo` and
+          # `foo-agent` are on the roster, because only then are two clients
+          # actually named the same string. With `foo-agent` alone there is no
+          # duplicate and nothing to see — the hive simply receives its
+          # agents' grant instead of its own, and a NATS denial arrives as a
+          # timeout, so the symptom names nothing.
+          #
+          # Checkable here and nowhere downstream: the responder runs in a
+          # container with no view of the roster (`policy.rs`'s `hive_name`
+          # says why that is deliberate), so this is the last place that knows
+          # both the naming scheme and the set of names.
+          assertion =
+            !cfg.oidc.hiveIdentities
+            || !lib.any (h: lib.hasSuffix cfg.agentClientSuffix h) (lib.attrNames hyperhiveCfg.swarm.hives);
+          message =
+            "services.hyperhive.swarm.hives contains "
+            + lib.concatMapStringsSep ", " (h: "'${h}'") (
+              lib.filter (h: lib.hasSuffix cfg.agentClientSuffix h) (lib.attrNames hyperhiveCfg.swarm.hives)
+            )
+            + " — names ending with '${cfg.agentClientSuffix}', the suffix that "
+            + "marks a hive's agent containers. Such a hive's own client id is "
+            + "indistinguishable from another hive's agent client, and the "
+            + "queue resolves it as the agents. Rename the hive.";
+        }
         # The two obligations `authelia.bearer.authz` carries. Authelia
         # enforces both itself — but in its `preStart` validator, so a
         # violation produces a green `nixos-rebuild switch` and an authelia

nix/host-modules/swarm-nats.nix

@@ -769,7 +769,7 @@ in
                 # so a drift here is silent at the point of change and
                 # misattributed at the point of failure.
                 "--hive-client-prefix ${lib.escapeShellArg autheliaCfg.hiveClientPrefix}"
-                "--agent-client-prefix ${lib.escapeShellArg autheliaCfg.agentClientPrefix}"
+                "--agent-client-suffix ${lib.escapeShellArg autheliaCfg.agentClientSuffix}"
                 "--reader-client ${lib.escapeShellArg controllerCfg.queueClientId}"
               ];
               # Every credential arrives by `LoadCredential` and is named

swarm-nats-auth/src/main.rs

@@ -82,15 +82,20 @@ struct Args {
     #[arg(long, default_value = "hive-")]
     hive_client_prefix: String,
 
-    /// Client-id prefix that marks an agent container. `swarm-authelia.nix`
+    /// Suffix that marks an agent container, appended to the hive's own client
-    /// mints one machine client per roster entry as `agent-<hive>` — per
+    /// id: `swarm-authelia.nix` mints one machine client per roster entry as
-    /// **hive**, not per agent, because agents are created at runtime and a
+    /// `hive-<name>-agent` — per **hive**, not per agent, because agents are
-    /// per-agent client would make creating one a config change plus a reload.
+    /// created at runtime and a per-agent client would make creating one a
+    /// config change plus a reload.
     ///
     /// So this identity says which hive an agent belongs to and never which
     /// agent: two agents on one hive are indistinguishable to this responder.
-    #[arg(long, default_value = "agent-")]
+    ///
-    agent_client_prefix: String,
+    /// A suffix on the hive's id rather than a prefix of its own, because
+    /// `agent-<name>` reads as *the agent called `<name>`* — the one thing
+    /// this identity does not carry.
+    #[arg(long, default_value = "-agent")]
+    agent_client_suffix: String,
 
     /// Client ids allowed to read every hive's status. Repeatable. The
     /// default is the swarm controller, which is the only reader that exists.
@@ -108,7 +113,8 @@ struct Args {
     hive_publish_subjects: Vec<String>,
 
     /// Subjects an agent may publish to, with `{hive}` standing for the hive
-    /// its identity names. Repeatable, empty by default.
+    /// its identity names — `hive-alpha-agent` expands it to `alpha`.
+    /// Repeatable, empty by default.
     ///
     /// Empty means agents get **no grant at all** rather than a grant that can
     /// do nothing — the identity exists, and what it may say is a deployment's
@@ -161,7 +167,7 @@ async fn main() -> anyhow::Result<()> {
     // silently over-broad grant is not.
     let policy = policy::Policy::new(
         args.hive_client_prefix.clone(),
-        args.agent_client_prefix.clone(),
+        args.agent_client_suffix.clone(),
         swarm_queue_client::status::BUCKET.to_owned(),
         args.reader_clients.clone(),
         args.hive_publish_subjects.clone(),

swarm-nats-auth/src/policy.rs

@@ -40,7 +40,7 @@ pub struct Permissions {
 #[derive(Debug, Clone)]
 pub struct Policy {
     hive_prefix: String,
-    agent_prefix: String,
+    agent_suffix: String,
     bucket: String,
     readers: Vec<String>,
     extra_hive_subjects: Vec<String>,
@@ -53,11 +53,16 @@ const HIVE_PLACEHOLDER: &str = "{hive}";
 
 impl Policy {
     /// `hive_prefix` is the client-id prefix that marks a hive and
-    /// `agent_prefix` the one that marks a hive's agent containers; `bucket` is
+    /// `agent_suffix` what a hive's agent containers carry **on top of** it —
-    /// the KV bucket hives report status in, `readers` the client ids allowed
+    /// `hive-<name>` and `hive-<name>-agent`. `bucket` is the KV bucket hives
-    /// to read every hive's key, and `extra_hive_subjects` /
+    /// report status in, `readers` the client ids allowed to read every hive's
-    /// `extra_agent_subjects` additional subjects each may publish to (with
+    /// key, and `extra_hive_subjects` / `extra_agent_subjects` additional
-    /// `{hive}` standing for the hive's own name).
+    /// subjects each may publish to (with `{hive}` standing for the hive's own
+    /// name).
+    ///
+    /// The agent id is deliberately the hive's id extended, not a second
+    /// top-level prefix: `agent-<name>` reads as *the agent called `<name>`*,
+    /// which is the one thing this identity does not carry.
     ///
     /// # Errors
     ///
@@ -68,13 +73,13 @@ impl Policy {
     /// checking at the call site is deliberate: it makes an unscoped policy
     /// unconstructible instead of merely unlikely.
     ///
-    /// Two prefixes where one of them is a prefix of the other is refused for
+    /// An empty `agent_suffix` is refused for the same reason: every hive id
-    /// the same reason: the arms are tried in order, so the overlap does not
+    /// would also parse as its own agent id, and the arms are tried in order,
-    /// error at match time — it silently hands one principal the other's
+    /// so the overlap does not error at match time — it silently hands one
-    /// grant.
+    /// principal the other's grant.
     pub fn new(
         hive_prefix: String,
-        agent_prefix: String,
+        agent_suffix: String,
         bucket: String,
         readers: Vec<String>,
         extra_hive_subjects: Vec<String>,
@@ -103,23 +108,24 @@ impl Policy {
                  rather than a per-hive namespace"
             );
         }
-        // Checked here rather than trusted from nix, because the two prefixes
+        // Checked here rather than trusted from nix, because these are two
-        // are two flags and nothing downstream compares them. `permissions`
+        // flags and nothing downstream compares them. An agent id is a hive id
-        // tries the hive arm first, so an agent prefix of `""` — or either one
+        // plus this suffix, so an empty one makes `strip_suffix` succeed on
-        // being a prefix of the other — would route agents into the hive grant
+        // every hive id and the two principals become the same string. What
-        // with no error anywhere: a client that authenticates fine and is
+        // that costs is not a parse error but a **grant**: whichever arm runs
-        // granted more than it should have, which is the one failure this
+        // first answers for both, and a client that authenticates fine is
-        // module must not have.
+        // handed more than it should have — the one failure this module must
-        if hive_prefix.starts_with(&agent_prefix) || agent_prefix.starts_with(&hive_prefix) {
+        // not have.
+        if agent_suffix.is_empty() {
             anyhow::bail!(
-                "--hive-client-prefix {hive_prefix:?} and --agent-client-prefix \
+                "--agent-client-suffix is empty: an agent id is a hive id plus this suffix, \
-                 {agent_prefix:?} overlap: one is a prefix of the other, so a client id \
+                 so every hive would also parse as its own agents and one of the two would \
-                 matching the longer one is granted by whichever rule is tried first"
+                 silently receive the other's grant"
             );
         }
         Ok(Self {
             hive_prefix,
-            agent_prefix,
+            agent_suffix,
             bucket,
             readers,
             extra_hive_subjects,
@@ -133,11 +139,15 @@ impl Policy {
     /// a connected client with no permissions still holds a slot and still
     /// looks admitted in the logs, which is a worse answer than a refusal.
     pub fn permissions(&self, client_id: &str) -> Option<Permissions> {
-        if let Some(hive) = self.hive_name(client_id) {
+        // ⚠️ THE AGENT ARM RUNS FIRST, AND THE ORDER IS LOAD-BEARING. An agent
-            return Some(Permissions {
+        // id is a hive id with a suffix, so `hive-foo-agent` satisfies the hive
-                publish: self.hive_subjects(hive),
+        // arm too — it strips to the hive `foo-agent`. Hive-first would
-            });
+        // therefore hand every agent its hive's grant, including writing that
-        }
+        // hive's status key, and nothing would report it: the client
+        // authenticates, connects, and is simply able to do more than it
+        // should. Most specific wins, so the more specific test is the one
+        // that has to be asked first.
+        //
         // An agent's identity names its hive, never the agent — see
         // `Self::agent_hive`. The grant is whatever the deployment configured
         // for agents, expanded for that hive, and **nothing when it configured
@@ -147,6 +157,11 @@ impl Policy {
             let publish = self.agent_subjects(hive);
             return (!publish.is_empty()).then_some(Permissions { publish });
         }
+        if let Some(hive) = self.hive_name(client_id) {
+            return Some(Permissions {
+                publish: self.hive_subjects(hive),
+            });
+        }
         if self.readers.iter().any(|r| r == client_id) {
             return Some(Permissions {
                 publish: self.reader_subjects(),
@@ -186,14 +201,24 @@ impl Policy {
 
     /// The hive whose agents a client id names, when it names one.
     ///
-    /// Deliberately the same shape as [`Self::hive_name`] one prefix over, and
+    /// [`Self::hive_name`]'s caveats all apply — this is that parse with the
-    /// every caveat above applies unchanged. One more on top: the agent client
+    /// suffix stripped as well, so `hive-foo-agent` answers `foo`. Two more on
-    /// is minted per **hive**, so this answers *whose agents*, never *which
+    /// top:
-    /// agent*. Two agents on one hive are indistinguishable here — not a gap in
+    ///
-    /// the parsing but the identity itself, which does not carry the agent.
+    /// - The agent client is minted per **hive**, so this answers *whose
+    ///   agents*, never *which agent*. Two agents on one hive are
+    ///   indistinguishable here — not a gap in the parsing but the identity
+    ///   itself, which does not carry the agent.
+    /// - A hive genuinely named `foo-agent` mints `hive-foo-agent` and is
+    ///   therefore **indistinguishable from hive `foo`'s agents** right here.
+    ///   Unguardable from inside this responder, which has no roster (see
+    ///   [`Self::hive_name`]) — `swarm-authelia.nix` asserts it at eval, where
+    ///   the roster is known, so the deployment fails to build rather than
+    ///   minting two principals with one id.
     fn agent_hive<'a>(&self, client_id: &'a str) -> Option<&'a str> {
         client_id
-            .strip_prefix(&self.agent_prefix)
+            .strip_prefix(&self.hive_prefix)?
+            .strip_suffix(&self.agent_suffix)
             .filter(|name| !name.is_empty())
     }
 
@@ -495,7 +520,7 @@ mod tests {
     fn policy() -> Policy {
         Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             vec!["swarm-controller".to_owned()],
             Vec::new(),
@@ -510,7 +535,7 @@ mod tests {
     fn policy_with_agent_subject() -> Policy {
         Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             vec!["swarm-controller".to_owned()],
             Vec::new(),
@@ -613,7 +638,7 @@ mod tests {
         // way to remove it - silently, and only in the deployment that set it.
         let err = Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             Vec::new(),
             vec!["$SWARM.events.all".to_owned()],
@@ -633,7 +658,7 @@ mod tests {
         // from the option being broken.
         Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             Vec::new(),
             vec!["$SWARM.events.{hive}.>".to_owned()],
@@ -1041,7 +1066,7 @@ mod tests {
         // another's events even though its status key is scoped.
         let p = Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             Vec::new(),
             vec!["$SWARM.events.{hive}.>".to_owned()],
@@ -1059,7 +1084,7 @@ mod tests {
         // A grant with an empty publish list would let it connect and then
         // fail on every publish, which reads as a broken queue rather than as
         // a deployment that configured nothing.
-        assert!(policy().permissions("agent-alpha").is_none());
+        assert!(policy().permissions("hive-alpha-agent").is_none());
     }
 
     #[test]
@@ -1067,18 +1092,28 @@ mod tests {
         // The presence control for the test above - without it, "refused"
         // would also be the answer if the agent arm never matched at all.
         let g = policy_with_agent_subject()
-            .permissions("agent-alpha")
+            .permissions("hive-alpha-agent")
             .expect("an agent with a configured subject is admitted");
         assert_eq!(g.publish, vec!["$SWARM.term.alpha.>".to_owned()]);
     }
 
     #[test]
     fn an_agent_does_not_get_its_hives_grant() {
-        // The whole point of a second prefix. An agent holds a credential that
+        // An agent holds a credential that sits in a container; a hive's grant
-        // sits in a container; a hive's grant includes writing that hive's
+        // includes writing that hive's status key, which an agent must not be
-        // status key, which an agent must not be able to forge.
+        // able to forge.
+        //
+        // 🩸 This became a REGRESSION test when the id moved from `agent-<hive>`
+        // to `hive-<hive>-agent`: the agent id now satisfies the HIVE parse too
+        // (`hive-alpha-agent` strips to a hive named `alpha-agent`), so a
+        // hive-first arm order hands every agent its hive's grant, and nothing
+        // reports it — the client authenticates and is merely able to do more
+        // than it should. Together with the `assert_eq` in
+        // `an_agent_publishes_inside_its_own_hives_namespace` (which pins the
+        // subject to `alpha`, not `alpha-agent`) this is what holds the
+        // ordering in place.
         let g = policy_with_agent_subject()
-            .permissions("agent-alpha")
+            .permissions("hive-alpha-agent")
             .expect("an agent is admitted");
         assert!(!g.publish.iter().any(|s| s.starts_with("$KV.")));
         assert!(!g.publish.iter().any(|s| s.starts_with("$JS.API.")));
@@ -1088,7 +1123,7 @@ mod tests {
     fn an_agent_subject_without_the_placeholder_is_refused() {
         let err = Policy::new(
             "hive-".to_owned(),
-            "agent-".to_owned(),
+            "-agent".to_owned(),
             "hive-status".to_owned(),
             Vec::new(),
             Vec::new(),
@@ -1102,27 +1137,42 @@ mod tests {
     }
 
     #[test]
-    fn overlapping_prefixes_are_refused() {
+    fn an_empty_agent_suffix_is_refused() {
-        // 🩸 The arms are tried in order, so an overlap does not error at match
+        // 🩸 An agent id is a hive id plus the suffix, so an empty suffix makes
-        // time - it hands one principal the other's grant. Both directions,
+        // `strip_suffix` succeed on every hive id: the two principals become
-        // because which one wins depends only on the order above.
+        // one string, and whichever arm runs first answers for both. That is a
-        for (hive, agent) in [("hive-", "hive-agent-"), ("agent-x-", "agent-"), ("h", "h")] {
+        // GRANT, not a parse error, which is why it has to fail at
-            Policy::new(
+        // construction.
-                hive.to_owned(),
+        Policy::new(
-                agent.to_owned(),
+            "hive-".to_owned(),
-                "hive-status".to_owned(),
+            String::new(),
-                Vec::new(),
+            "hive-status".to_owned(),
-                Vec::new(),
+            Vec::new(),
-                Vec::new(),
+            Vec::new(),
-            )
+            Vec::new(),
-            .expect_err("prefixes where one contains the other must not construct");
+        )
-        }
+        .expect_err("an empty agent suffix must not construct");
     }
 
     #[test]
-    fn the_agent_prefix_alone_names_no_hive() {
+    fn the_agent_suffix_alone_names_no_hive() {
-        // Same trap as `the_prefix_alone_names_no_hive`: `agent-` with nothing
+        // `hive--agent` is the prefix and the suffix with nothing between them.
-        // after it would expand `{hive}` to the empty string.
+        // The agent arm must refuse it rather than expand `{hive}` to the empty
-        assert!(policy_with_agent_subject().permissions("agent-").is_none());
+        // string and hand out `$SWARM.term..>`.
+        //
+        // 🩸 Written first as `is_none()`, by analogy with
+        // `the_prefix_alone_names_no_hive`, and it FAILED — correctly. The
+        // analogy does not hold: under the hive parse the same id is a hive
+        // named `-agent`, which is a non-empty name this responder cannot rule
+        // out, because it has no roster. `swarm-authelia.nix` refuses such a
+        // name at eval, where the roster is known. So the assertion here is
+        // narrowed to the only part this module owns.
+        let g = policy_with_agent_subject().permissions("hive--agent");
+        assert!(
+            !g.iter()
+                .flat_map(|p| &p.publish)
+                .any(|s| s.contains("$SWARM.term..")),
+            "an empty hive name must never be expanded into a subject: {g:?}"
+        );
     }
 }