From 7a03ce096a074f82f5c52987e1d9fb3a82339c97 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 23:28:31 +0200 Subject: [PATCH] hive-c0re: deliver an agent's credential from the store to its state dir mara on #4015: "not merging code without callers", and on the same PR "see issue, we decided what the first thing should be". #3726 decided it: the controller writes a token to the store and tells the hive; the hive reads it back and writes /agents//state/matrix-token- at 0600, where matrix.nix's existing systemd.paths glob re-fires the daemon. So this is the hive half of that, and the library's first caller. The notice names a credential and never carries one, and deploy_subject's own doc is why: the auth-callout responder scopes publish and leaves sub unrestricted, so a hive that wanted another's messages could subscribe to them. A secret in that payload would be readable swarm-wide. The value is read from the store under the reading hive's own certificate, where the store's policy is what actually scopes it. Two boundaries guard the two addresses, and they are not the same check. `path::matrix_account` guards the address in the store. `Ident` guards the address on disk -- `agent_state_dir` takes one, so an unvalidated name off the queue cannot reach a directory. I had written the first and assumed it covered both; the compiler refused the `&str` and was right. `token_path` now takes the newtype so a call site cannot forget. The write is atomic because the path-watcher fires on the file appearing: written in place it would be visible while partial, and the daemon would read a truncated credential exactly once, which is the hardest possible failure to reproduce. The temp name is dot-prefixed so it cannot match the `matrix-token*` glob on its way past. The publish grant is here because without it the failure is invisible. policy.rs already says why for its siblings: a refused publish reaches the client as a timeout, so the symptom is a hive that never receives a credential with nothing in either log naming a permission. Two tests: the controller may publish, a hive may not -- its own subject included. A forged notice leaks nothing, but it would make a hive fetch and overwrite a token file for a name the forger chose. Refs #3726 --- Cargo.lock | 1 + hive-c0re/Cargo.toml | 1 + hive-c0re/src/swarm_status.rs | 59 +++++++++++- hive-c0re/src/workers/credential.rs | 144 ++++++++++++++++++++++++++++ hive-c0re/src/workers/mod.rs | 1 + swarm-nats-auth/src/policy.rs | 41 ++++++++ swarm-queue-client/src/lib.rs | 36 +++++++ 7 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 hive-c0re/src/workers/credential.rs diff --git a/Cargo.lock b/Cargo.lock index f356b36c..f5a37f9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1767,6 +1767,7 @@ dependencies = [ "serde_json", "sha2 0.11.0", "swarm-queue-client", + "swarm-secret-client", "tempfile", "tokio", "tokio-stream", diff --git a/hive-c0re/Cargo.toml b/hive-c0re/Cargo.toml index a1c9084f..895dc27f 100644 --- a/hive-c0re/Cargo.toml +++ b/hive-c0re/Cargo.toml @@ -54,6 +54,7 @@ serde_json.workspace = true # the swarm controller reads it with, and `kv` for the same reason: the # bucket's name and creation config belong to neither end of it alone. swarm-queue-client = { workspace = true, features = ["kv", "notices"] } +swarm-secret-client.workspace = true tokio.workspace = true tokio-stream.workspace = true tracing.workspace = true diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index aadbffac..b0c63070 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -198,7 +198,24 @@ async fn drain_swarm_events( return; } }; - tracing::info!(%subject, %deploy_subject, "swarm events: listening"); + // Also this hive's own, and for a second reason on top of the deploy + // subject's: the payload names an agent in *this* hive's state dir, so a + // notice for another hive is not merely noise, it is unactionable here. + let credential_subject = swarm_queue_client::credential_subject(&hive); + let mut credential_sub = match client.subscribe(credential_subject.clone()).await { + Ok(sub) => sub, + Err(e) => { + tracing::warn!( + subject = %credential_subject, error = %e, + "swarm events: subscribe failed; this hive will not hear credential notices" + ); + return; + } + }; + tracing::info!( + %subject, %deploy_subject, %credential_subject, + "swarm events: listening" + ); loop { tokio::select! { @@ -223,6 +240,13 @@ async fn drain_swarm_events( }; handle_deploy_request(&coord, &msg.payload).await; } + msg = credential_sub.next() => { + let Some(msg) = msg else { + tracing::warn!(subject = %credential_subject, "swarm events: credential subscription closed"); + return; + }; + handle_credential_notice(&hive, &msg.payload).await; + } _ = shutdown.changed() => { tracing::info!("swarm events: shutdown signal received"); return; @@ -236,6 +260,39 @@ async fn drain_swarm_events( /// /// 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. +/// Deliver the credential a [`swarm_queue_client::CredentialNotice`] names. +/// +/// `hive` doubles as the cert-auth role this hive logs into the store as: +/// `glue-bao-tls.nix` mints the client certificate with the hive name as its +/// CN, and a bao cert role matches on CN — so the two share a name by +/// construction rather than by convention. +/// +/// ⚠️ Nothing here can log the secret, and that is structural rather than +/// careful: the notice carries only names, and `deliver` writes the value +/// without returning it. +async fn handle_credential_notice(hive: &str, payload: &[u8]) { + let notice: swarm_queue_client::CredentialNotice = match serde_json::from_slice(payload) { + Ok(notice) => notice, + Err(e) => { + tracing::warn!(error = %e, "swarm events: undecodable credential notice"); + return; + } + }; + if let Err(e) = crate::workers::credential::deliver(¬ice, hive).await { + // Warn rather than retry: the controller republishes, and a hive that + // spun here would hold the queue task off its other two subjects. + tracing::warn!( + agent = %notice.agent, account = %notice.account, error = ?e, + "swarm events: credential delivery failed" + ); + return; + } + tracing::info!( + agent = %notice.agent, account = %notice.account, + "swarm events: credential delivered" + ); +} + async fn handle_deploy_request( coord: &std::sync::Arc, payload: &[u8], diff --git a/hive-c0re/src/workers/credential.rs b/hive-c0re/src/workers/credential.rs new file mode 100644 index 00000000..a6f11933 --- /dev/null +++ b/hive-c0re/src/workers/credential.rs @@ -0,0 +1,144 @@ +//! Delivering an agent's external-account credential from the swarm's secret +//! store into that agent's own state dir. +//! +//! The controller publishes a [`CredentialNotice`] naming an agent and an +//! account; this reads the value out of the store and writes it where the +//! agent's matrix daemon already watches for it. Nothing here activates +//! anything: `nix/agent-modules/matrix.nix` has a `systemd.paths` unit +//! globbing `/agents/*/state/matrix-token*` that re-fires the daemon when a +//! token appears, so arrival is the whole trigger. +//! +//! 🔑 The notice carries no secret — see [`swarm_queue_client::credential_subject`] +//! for why that is a requirement rather than a preference. The value is read +//! from the store under this hive's own identity. + +use std::os::unix::fs::PermissionsExt as _; +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use hive_types::Ident; +use swarm_queue_client::CredentialNotice; +use swarm_secret_client::{SecretStore, path}; + +use crate::paths::agent_state_dir; + +/// The basename every matrix token must start with. +/// +/// `nix/agent-modules/matrix.nix` asserts the same prefix on every configured +/// `tokenFile`, because its path-watcher globs for it. A name written here +/// that did not match would land a file the daemon never notices — no error, +/// just a credential that silently never arrives. +const TOKEN_PREFIX: &str = "matrix-token"; + +/// Where `agent`'s credential for `account` is written. +/// +/// Takes an [`Ident`] rather than a `&str` because the name arrives off the +/// queue: `agent_state_dir` addresses a directory, and an unvalidated name +/// there is a path-traversal argument. The compiler refusing the `&str` is the +/// check — nothing here has to remember to perform one. +#[must_use] +pub fn token_path(agent: &Ident, account: &str) -> PathBuf { + agent_state_dir(agent).join(format!("{TOKEN_PREFIX}-{account}")) +} + +/// Read the credential `notice` names and write it into the agent's state dir. +/// +/// `cert_role` is the role on the store's `cert` auth mount whose policy scopes +/// what this hive may read. +/// +/// # Errors +/// The store refusing, being unreachable, or holding nothing at that path; a +/// name that is not a single path segment; or the write failing. +pub async fn deliver(notice: &CredentialNotice, cert_role: &str) -> Result<()> { + // Parsed before anything is read, so a malformed name costs a decode and + // not a round trip to the store. + let agent = Ident::parse(¬ice.agent) + .map_err(|e| anyhow::anyhow!("agent name {:?} off the queue: {e}", notice.agent))?; + let secret_path = path::matrix_account(¬ice.agent, ¬ice.account) + .context("building the credential's path in the store")?; + + let store = SecretStore::from_env(cert_role) + .await + .context("connecting to the swarm secret store")?; + let value = store + .read(&secret_path) + .await + .with_context(|| format!("reading {secret_path} from the store"))?; + + write_token(&token_path(&agent, ¬ice.account), &value) +} + +/// Write `value` to `dest` at `0600`, atomically. +/// +/// Atomic because the daemon's path-watcher fires on the file *appearing*: a +/// token written in place would be visible while still partial, and the daemon +/// would read a truncated credential exactly once, at the moment it is hardest +/// to reproduce. The temp name is dot-prefixed so it cannot match the +/// `matrix-token*` glob on its way past. +fn write_token(dest: &PathBuf, value: &str) -> Result<()> { + let dir = dest + .parent() + .context("a token path always has a parent state dir")?; + let name = dest + .file_name() + .context("a token path always has a file name")?; + let mut tmp = dir.join("."); + tmp.as_mut_os_string().push(name); + tmp.as_mut_os_string().push(".partial"); + + std::fs::write(&tmp, value).with_context(|| format!("writing {}", tmp.display()))?; + // Before the rename, so the file is never briefly readable by others under + // its final name. + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600)) + .with_context(|| format!("restricting {}", tmp.display()))?; + std::fs::rename(&tmp, dest).with_context(|| format!("publishing {}", dest.display())) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_written_name_matches_the_glob_the_daemon_watches() { + // The other end of this agreement is an assertion in + // `nix/agent-modules/matrix.nix` and a `systemd.paths` glob — neither + // reachable from a Rust test, so the prefix is pinned here. + let p = token_path(&Ident::parse("dmatrix").expect("a legal agent name"), "ccc"); + let name = p.file_name().unwrap().to_str().unwrap(); + assert!(name.starts_with("matrix-token"), "got {name}"); + assert_eq!(name, "matrix-token-ccc"); + } + + #[test] + fn the_partial_file_cannot_match_that_glob() { + // A temp name starting with `matrix-token` would be picked up + // mid-write; the dot prefix is what stops it. + let p = token_path(&Ident::parse("dmatrix").expect("a legal agent name"), "ccc"); + let name = p.file_name().unwrap().to_str().unwrap(); + let tmp = format!(".{name}.partial"); + assert!(!tmp.starts_with("matrix-token"), "got {tmp}"); + } + + #[test] + fn a_name_that_could_address_another_agent_is_refused() { + // `path::matrix_account` owns this rule; asserted here because this is + // the module that feeds it names off the wire. + assert!(path::matrix_account("../argus", "ccc").is_err()); + assert!(path::matrix_account("dmatrix", "../../etc/x").is_err()); + assert!(path::matrix_account("dmatrix", "ccc").is_ok()); + } + + #[test] + fn an_agent_name_off_the_queue_must_pass_the_ident_parser_too() { + // Two independent refusals, not one restated: `path::matrix_account` + // guards the address in the *store*, `Ident` guards the address on + // *disk*. `token_path` cannot even be called without the second, + // which is why it takes an `Ident` rather than validating internally. + for bad in ["../argus", "dmatrix/../argus", "Dmatrix", "d matrix", ""] { + assert!(Ident::parse(bad).is_err(), "{bad:?} must be refused"); + } + // The control: without it, a parser that rejected everything would + // satisfy the loop above. + assert!(Ident::parse("dmatrix").is_ok()); + } +} diff --git a/hive-c0re/src/workers/mod.rs b/hive-c0re/src/workers/mod.rs index da62baaa..bc6f232b 100644 --- a/hive-c0re/src/workers/mod.rs +++ b/hive-c0re/src/workers/mod.rs @@ -8,6 +8,7 @@ pub mod agent_sockets; pub mod auto_update; pub mod crash_watch; +pub mod credential; pub mod knowledge; pub mod mcp_sockets; pub mod scheduled_prompts_worker; diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 4a0b5520..2383f2e0 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -352,6 +352,17 @@ impl Policy { // 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(), + // The credential notices: same per-hive family and same wildcard + // reasoning as the deploy events, and the same timeout-not-error + // failure if this line is missing. + // + // 🔑 Worth being explicit that this grant is not a confidentiality + // boundary, because it looks like one. `sub` is unrestricted, so + // any hive could subscribe to another's notices — which is exactly + // why the payload names a credential and never carries one. What + // scopes the secret is the store's own policy at read time, under + // the reading hive's certificate. + swarm_queue_client::CREDENTIAL_SUBJECT_WILDCARD.to_owned(), // The wanted-state buckets — one per hive, all created and written // by this single client. // @@ -581,6 +592,36 @@ mod tests { ); } + #[test] + fn a_reader_may_publish_a_credential_notice_to_any_hive() { + let p = policy().permissions("swarm-controller").expect("a reader"); + assert!( + p.publish + .contains(&swarm_queue_client::CREDENTIAL_SUBJECT_WILDCARD.to_owned()), + "without this grant the controller's publish is refused, and a \ + refusal arrives as a timeout — a hive that silently never receives \ + a credential, with nothing in either log saying why" + ); + } + + #[test] + fn a_hive_may_not_publish_a_credential_notice() { + // A forged notice cannot leak a secret — the payload carries none — but + // it can make a hive fetch and overwrite an agent's token file with + // whatever the store holds for a name the forger chose. + let p = policy() + .permissions("hive-alpha") + .expect("a hive is admitted"); + assert!( + !p.publish + .iter() + .any(|s| s == swarm_queue_client::CREDENTIAL_SUBJECT_WILDCARD + || s == &swarm_queue_client::credential_subject("hive-alpha")), + "a hive must not publish credential notices, its own included: {:?}", + p.publish + ); + } + #[test] 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 diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index f1098326..809304ff 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -212,6 +212,42 @@ pub const DEPLOY_SUBJECT_WILDCARD: &str = "$SWARM.deploy.*"; /// cannot drift into naming different families. const DEPLOY_SUBJECT_PREFIX: &str = "$SWARM.deploy"; +/// The subject the controller publishes on to tell `hive` that a credential +/// for one of its agents is waiting in the secret store. Same per-hive family +/// as [`deploy_subject`], here for the same three-crate reason. +/// +/// 🔑 **The message NAMES a credential and never carries one**, and the note +/// on [`deploy_subject`] is why: the family split buys quiet, not +/// confidentiality — the auth-callout responder scopes `pub` and leaves `sub` +/// unrestricted, so any hive that wanted another's messages could subscribe to +/// them. A secret in this payload would be readable swarm-wide. The hive reads +/// the value from the store under its own identity instead, where the store's +/// policy is the thing that actually scopes it. +#[must_use] +pub fn credential_subject(hive: &str) -> String { + format!("{CREDENTIAL_SUBJECT_PREFIX}.{hive}") +} + +/// The publish grant covering every [`credential_subject`] — a wildcard for +/// the same no-roster reason as [`DEPLOY_SUBJECT_WILDCARD`]. +pub const CREDENTIAL_SUBJECT_WILDCARD: &str = "$SWARM.credential.*"; + +/// Shared by [`credential_subject`] and [`CREDENTIAL_SUBJECT_WILDCARD`] so the +/// two cannot drift into naming different families. +const CREDENTIAL_SUBJECT_PREFIX: &str = "$SWARM.credential"; + +/// What a [`credential_subject`] message says: which agent's credential +/// changed, and which account it belongs to. Deliberately the whole payload — +/// anything more would be either derivable by the reader or a secret that +/// must not be on the wire. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct CredentialNotice { + /// The agent whose state dir receives the credential. + pub agent: String, + /// The external account the credential authenticates as. + pub account: String, +} + /// What a [`deploy_subject`] message carries. /// /// Only the agent: the subject already names the hive, and repeating it here