diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index 714d4850..e7d43c87 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -140,9 +140,9 @@ pub fn spawn( /// already runs at boot, and **deploy this agent**, answered by the same /// rebuild insert the operator's own verb makes. /// -/// Only the deploy event carries a payload, because only it is *addressed*: -/// a swarm-wide subject leaves the addressing nowhere to live but the body. -/// Even then it is a trigger — which hive, which agent — never the config, +/// Only the deploy event carries a payload, and only the agent name: its +/// subject already names the hive, so this one listens on its own rather than +/// filtering a swarm-wide feed. Even then it is a trigger, never the config, /// which git owns. /// /// # A missed message costs the two events very differently @@ -185,8 +185,10 @@ async fn drain_swarm_events( return; } }; - let deploy_subject = swarm_queue_client::DEPLOY_SUBJECT; - let mut deploy_sub = match client.subscribe(deploy_subject).await { + // This hive's own deploy subject, not a swarm-wide one: the controller + // addresses each hive, so nothing arrives here that is not for us. + let deploy_subject = swarm_queue_client::deploy_subject(&hive); + let mut deploy_sub = match client.subscribe(deploy_subject.clone()).await { Ok(sub) => sub, Err(e) => { tracing::warn!( @@ -219,7 +221,7 @@ async fn drain_swarm_events( tracing::warn!(subject = %deploy_subject, "swarm events: deploy subscription closed"); return; }; - handle_deploy_request(&coord, &hive, &msg.payload); + handle_deploy_request(&coord, &msg.payload); } _ = shutdown.changed() => { tracing::info!("swarm events: shutdown signal received"); @@ -229,18 +231,12 @@ async fn drain_swarm_events( } } -/// Act on one deploy request, if it is addressed to this hive. +/// Act on one deploy request. Everything that arrives on this hive's own +/// deploy subject is for this hive, so there is nothing to filter. /// -/// Every hive receives every message — that is what the swarm-wide subject -/// buys — so *not* being the addressee is the ordinary case and logs at -/// `debug`, not `warn`. A malformed payload is different: the controller and -/// this end share one type, so a decode failure means they disagree about it, -/// and that is worth saying out loud. -fn handle_deploy_request( - coord: &std::sync::Arc, - hive: &str, - payload: &[u8], -) { +/// A payload that will not decode is worth a `warn`: the controller and this +/// end share one type, so a decode failure means they disagree about it. +fn handle_deploy_request(coord: &std::sync::Arc, payload: &[u8]) { let request: swarm_queue_client::DeployRequest = match serde_json::from_slice(payload) { Ok(request) => request, Err(e) => { @@ -248,10 +244,6 @@ fn handle_deploy_request( return; } }; - if request.hive != hive { - tracing::debug!(target_hive = %request.hive, agent = %request.agent, "swarm events: deploy for another hive"); - return; - } // The same insert the operator's own `rebuild` verb makes, relock and // all: "deploy this agent" means here exactly what it already meant, and // a swarm-triggered rebuild that quietly did something narrower would be diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index d7657dfb..7a82c0ef 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -91,10 +91,8 @@ enum SwarmNodeKind { /// /// Carries the hive, unlike every variant above — this is the node /// `InitAgentConfigRepo`'s doc points at when it says the hive belongs - /// on the node that sends the deploy message. The subject is swarm-wide - /// and the addressing rides in the payload; see - /// `swarm_queue_client::DEPLOY_SUBJECT` for why it is not a per-hive - /// family. + /// on the node that sends the deploy message. It names the subject the + /// message goes to, one per hive, so no other hive is woken by it. TriggerDeploy { hive: String, agent: String }, } @@ -272,16 +270,16 @@ async fn publish_deploy( ) -> hive_jobq::scheduler::Outcome { use hive_jobq::scheduler::Outcome; - let subject = swarm_queue_client::DEPLOY_SUBJECT; + // One subject per hive, so the other hives are never woken by this. + let subject = swarm_queue_client::deploy_subject(hive); let request = swarm_queue_client::DeployRequest { - hive: hive.to_owned(), agent: agent.to_owned(), }; let payload = match serde_json::to_vec(&request) { Ok(payload) => payload, Err(e) => return Outcome::Failed(format!("encoding the deploy request failed: {e}")), }; - if let Err(e) = client.publish(subject, payload.into()).await { + if let Err(e) = client.publish(subject.clone(), payload.into()).await { return Outcome::Failed(format!("publishing to {subject} failed: {e}")); } if let Err(e) = client.flush().await { diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 43cfff04..ae5900c3 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -278,12 +278,14 @@ impl Policy { // about a change, with nothing in the controller's log to say a // permission was the reason. swarm_queue_client::KNOWLEDGE_SUBJECT.to_owned(), - // The deploy event, same shape and the same failure mode as the - // knowledge event above — one literal subject, one writer. Also - // deliberately not a per-hive family: this responder scopes - // publish only, so a per-hive subject would not stop a hive - // reading another's. See the const's own doc. - swarm_queue_client::DEPLOY_SUBJECT.to_owned(), + // The deploy events: one subject per hive, so a hive is not woken + // by a deploy meant for another. Granted as a wildcard because + // this responder has no roster — it cannot enumerate hives, and a + // grant that had to track one would be a second place to get the + // list wrong (see `hive_name`'s doc for the same argument about + // admission). Same failure mode as the knowledge event above: a + // refused publish reaches the client as a timeout. + swarm_queue_client::DEPLOY_SUBJECT_WILDCARD.to_owned(), ]); subjects } @@ -380,36 +382,40 @@ mod tests { } #[test] - fn a_reader_may_publish_the_deploy_event() { + fn a_reader_may_publish_a_deploy_event_to_any_hive() { let p = policy().permissions("swarm-controller").expect("a reader"); assert!( p.publish - .contains(&swarm_queue_client::DEPLOY_SUBJECT.to_owned()), - "the controller is the only publisher of this event; without the \ + .contains(&swarm_queue_client::DEPLOY_SUBJECT_WILDCARD.to_owned()), + "the controller is the only publisher of these events; without the \ grant its publish is refused, and a refusal arrives as a timeout" ); } #[test] - fn a_hive_may_not_publish_the_deploy_event_to_anyone_including_itself() { + fn a_hive_may_not_publish_a_deploy_event_to_anyone_including_itself() { // Same arm as the knowledge event's, and it matters more here: a forged // knowledge event makes a hive re-read a repo, while a forged deploy - // event makes it rebuild and restart a named agent. One shared subject - // means a single forged message reaches every hive in the swarm. + // event makes it rebuild and restart a named agent. // - // Note this is the half `sub` scoping would not fix even once it lands: - // reading another hive's deploy message is a confidentiality question, - // *sending* one is this. + // Both directions asserted, because splitting the subject per hive + // makes them separate strings for the first time: a hive must reach + // neither another hive's deploy subject nor its own. Nothing in the + // grant should mention this family at all. let p = policy() .permissions("hive-alpha") .expect("a hive is admitted"); - assert!( - !p.publish - .iter() - .any(|s| s == swarm_queue_client::DEPLOY_SUBJECT), - "a hive must not publish the deploy event: {:?}", - p.publish - ); + for forbidden in [ + swarm_queue_client::deploy_subject("beta"), + swarm_queue_client::deploy_subject("alpha"), + swarm_queue_client::DEPLOY_SUBJECT_WILDCARD.to_owned(), + ] { + assert!( + !p.publish.contains(&forbidden), + "a hive must not publish {forbidden}: {:?}", + p.publish + ); + } } #[test] diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index a9e839ee..0d561af8 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -173,26 +173,35 @@ pub mod status; /// permitted at all — speaks neither `jetstream` nor `kv`. pub const KNOWLEDGE_SUBJECT: &str = "$SWARM.knowledge"; -/// The subject the swarm controller publishes on to ask a hive to rebuild one -/// of its agents. Same shape as [`KNOWLEDGE_SUBJECT`]: one writer, every hive -/// subscribes, and it lives here for the same three-crate reason. +/// The subject the swarm controller publishes on to ask `hive` to rebuild one +/// of its agents. Lives here for the same three-crate reason as +/// [`KNOWLEDGE_SUBJECT`], but unlike it this is a **family, one subject per +/// hive** — a hive subscribes to its own and is never woken by a deploy meant +/// for someone else. /// -/// # Swarm-wide, not per-hive, and that is deliberate -/// -/// A `$SWARM.deploy.` family would *look* like isolation and provide -/// none: the auth-callout responder scopes **publish** only, leaving `sub` -/// unrestricted, so any hive could subscribe to another's subject just as -/// easily as to its own. Until `sub` is scoped, the per-hive split costs a -/// wider grant and buys nothing — so the addressing lives in the payload and -/// each hive filters on its own name, exactly as the knowledge event fans out. -pub const DEPLOY_SUBJECT: &str = "$SWARM.deploy"; +/// ⚠️ **That split is about noise, not confidentiality.** The auth-callout +/// responder scopes *publish* only and leaves `sub` unrestricted, so a hive +/// that wanted another's messages could still subscribe to them. What the +/// family buys is that it does not receive them by default. +#[must_use] +pub fn deploy_subject(hive: &str) -> String { + format!("{DEPLOY_SUBJECT_PREFIX}.{hive}") +} -/// What a [`DEPLOY_SUBJECT`] message carries. +/// The publish grant covering every [`deploy_subject`], for the one client +/// that may send them. A wildcard rather than a subject per hive because the +/// responder has no roster — it cannot enumerate hives, and a grant it had to +/// keep in step with one would be a second place to get the list wrong. +pub const DEPLOY_SUBJECT_WILDCARD: &str = "$SWARM.deploy.*"; + +/// Shared by [`deploy_subject`] and [`DEPLOY_SUBJECT_WILDCARD`] so the two +/// cannot drift into naming different families. +const DEPLOY_SUBJECT_PREFIX: &str = "$SWARM.deploy"; + +/// What a [`deploy_subject`] message carries. /// -/// The knowledge event has no payload — every hive does the same thing on -/// receipt. This one is addressed, so both ends have to agree on the fields, -/// which is why the type lives beside the subject rather than in whichever -/// crate happened to need it first. +/// Only the agent: the subject already names the hive, and repeating it here +/// would be two places stating one fact, free to disagree. /// /// ⚠️ **A trigger, not the config.** The hive already tracks the agent's /// config repo; putting desired state on the wire would make this message a @@ -200,9 +209,6 @@ pub const DEPLOY_SUBJECT: &str = "$SWARM.deploy"; /// missed a message would then be wrong rather than merely late. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct DeployRequest { - /// Which hive should act. Every hive receives the message; the one whose - /// own name this matches is the one that rebuilds. - pub hive: String, /// The agent to rebuild, as the swarm knows it. pub agent: String, }