diff --git a/Cargo.lock b/Cargo.lock index f5a37f9e..bd738290 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4701,6 +4701,7 @@ dependencies = [ "sha2 0.11.0", "swarm-authelia-bridge-sock", "swarm-queue-client", + "swarm-secret-client", "tokio", "tracing", "tracing-subscriber", diff --git a/hive-c0re/src/workers/credential.rs b/hive-c0re/src/workers/credential.rs index 81f5e998..72ee4745 100644 --- a/hive-c0re/src/workers/credential.rs +++ b/hive-c0re/src/workers/credential.rs @@ -21,7 +21,7 @@ use anyhow::{Context, Result}; use hive_types::Ident; use swarm_queue_client::CredentialNotice; -use swarm_secret_client::{SecretStore, path}; +use swarm_secret_client::{SecretStore, matrix}; /// Read the credential `notice` names and write it into the agent's state dir. /// @@ -36,13 +36,13 @@ pub async fn deliver(notice: &CredentialNotice, cert_role: &str) -> Result<()> { // 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) + let secret_path = matrix::account_path(¬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 + let credential: matrix::Credential = store .read(&secret_path) .await .with_context(|| format!("reading {secret_path} from the store"))?; @@ -52,11 +52,18 @@ pub async fn deliver(notice: &CredentialNotice, cert_role: &str) -> Result<()> { // arrives owned by `hive-core` at 0600 — the daemon wakes on it appearing // and cannot read it. hive-priv also builds the filename, so the name the // watcher globs for is decided in one place now. + // + // A homeserver is passed through when the stored credential carries one; + // hive-priv then writes the `matrix-account-.json` sidecar beside the + // token, which is how the daemon discovers an extra account's homeserver + // without a static `matrixAccounts` entry. Credentials written before that + // field existed carry `None`, and the sidecar is simply not written — the + // account then needs a configured entry, exactly as it did before. crate::priv_client::write_agent_matrix_token( agent.as_str(), - &value, + &credential.value, Some(¬ice.account), - None, + credential.homeserver.as_deref(), ) .await } @@ -67,16 +74,16 @@ mod tests { #[test] fn a_name_that_could_address_another_agent_is_refused() { - // `path::matrix_account` owns this rule; asserted here because this is + // `matrix::account_path` 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()); + assert!(matrix::account_path("../argus", "ccc").is_err()); + assert!(matrix::account_path("dmatrix", "../../etc/x").is_err()); + assert!(matrix::account_path("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` + // Two independent refusals, not one restated: `matrix::account_path` // guards the address in the *store*, and `Ident` guards the name this // module hands to hive-priv, which builds the on-disk path from it. for bad in ["../argus", "dmatrix/../argus", "Dmatrix", "d matrix", ""] { @@ -95,7 +102,7 @@ mod tests { fn an_account_name_off_the_queue_is_refused_for_the_store_path() { for bad in ["../argus", "a/b", "a b", ""] { assert!( - path::matrix_account("dmatrix", bad).is_err(), + matrix::account_path("dmatrix", bad).is_err(), "account {bad:?} must be refused" ); } @@ -103,7 +110,7 @@ mod tests { // passing because everything is refused. Uppercase and underscore are // deliberate — `matrixAccounts` is an attrset, so both are names an // operator can already write, and hive-priv must accept them too. - assert!(path::matrix_account("dmatrix", "ops-relay").is_ok()); - assert!(path::matrix_account("dmatrix", "Ops_Relay9").is_ok()); + assert!(matrix::account_path("dmatrix", "ops-relay").is_ok()); + assert!(matrix::account_path("dmatrix", "Ops_Relay9").is_ok()); } } diff --git a/nix/host-modules/swarm-controller.nix b/nix/host-modules/swarm-controller.nix index a88b483e..509cb7fd 100644 --- a/nix/host-modules/swarm-controller.nix +++ b/nix/host-modules/swarm-controller.nix @@ -18,6 +18,37 @@ let deployCfg = config.services.hyperhive.deploy; autheliaCfg = config.services.hyperhive.swarm.authelia; + # Where the secret store is, and whether this host holds the controller's + # own leaf for it. ⚠️ The controller's pair, NOT `deploy.bao.clientCertFile` + # — that one is the hive reader's, and its policy scopes a hive's own + # secrets. The two are deliberately separate identities. + # + # Gated on the leaf, never on `deploy.bao.enable`: a controller three + # networks away from the store holds a leaf issued out of band and wants + # exactly this wiring. Same rule ./glue-controller-bao-identity.nix states + # for the paths themselves. + baoCfg = config.services.hyperhive.swarm.bao; + haveBaoIdentity = + deployCfg.swarm-controller.baoClientCertFile != null + && deployCfg.swarm-controller.baoClientKeyFile != null; + + # `swarm_secret_client` reads these spellings explicitly rather than + # vaultrs's `VAULT_*` defaults — falling through to those builds a client + # with no identity and fails at the TLS handshake, naming neither. `%d` and + # not the paths themselves: see the `LoadCredential` below. + baoEnv = + lib.optionalAttrs haveBaoIdentity { + BAO_ADDR = "https://${baoCfg.domain}:${toString baoCfg.port}"; + BAO_CLIENT_CERT = "%d/bao-client.pem"; + BAO_CLIENT_KEY = "%d/bao-client-key.pem"; + } + // lib.optionalAttrs (haveBaoIdentity && deployCfg.bao.serverCaFile != null) { + # Absent means the system trust store — right for a real CA, wrong for + # the self-signed one ./glue-bao-tls.nix mints, which is why that file + # names this path rather than leaving it to a default. + BAO_CACERT = "%d/bao-ca.pem"; + }; + # What `swarmctl` needs in order to act on authelia from the host. # # Only set when authelia actually runs **here**: the controller can be @@ -645,7 +676,17 @@ in ] ++ lib.optional ( deployCfg.swarm-controller.forgeTokenFile != null - ) "forge-token:${deployCfg.swarm-controller.forgeTokenFile}"; + ) "forge-token:${deployCfg.swarm-controller.forgeTokenFile}" + # The store identity, same shape and same reason as hive-c0re's: the + # key is root-owned `0600` and this daemon runs as `swarm-controller`, + # so it never gets read access to the original file. + ++ lib.optionals haveBaoIdentity [ + "bao-client.pem:${deployCfg.swarm-controller.baoClientCertFile}" + "bao-client-key.pem:${deployCfg.swarm-controller.baoClientKeyFile}" + ] + ++ lib.optional ( + haveBaoIdentity && deployCfg.bao.serverCaFile != null + ) "bao-ca.pem:${deployCfg.bao.serverCaFile}"; # The placeholder default that makes the above non-fatal. # `LoadCredential=` takes priority over `SetCredential=`, so this is @@ -770,6 +811,7 @@ in // webhookEnv // authBridgeEnv // swarmNameEnv + // baoEnv // otelEnv; }; diff --git a/nix/module-eval.nix b/nix/module-eval.nix index e050c80c..76ee0295 100644 --- a/nix/module-eval.nix +++ b/nix/module-eval.nix @@ -570,6 +570,42 @@ let in c.baoClientCertFile == null && c.baoClientKeyFile == null; } + { + # Being *pointed at* a leaf and *being handed* one are different claims, + # and the options above were the first without the second — declared, + # defaulted, and read by nothing. This is the arm that makes them reach + # the process. + # + # ⚠️ The LoadCredential source is asserted, not just the `%d` name: the + # controller's leaf and the hive reader's are two identities with two + # policies, and wiring `deploy.bao.clientCertFile` here would satisfy + # every `%d`-only check while giving the daemon a policy that cannot + # write an agent's credential. + name = "the controller is handed its own store leaf, not the hive reader's"; + ok = + let + s = baoControllerHere.systemd.services; + in + s ? swarm-controller + && (s.swarm-controller.environment ? BAO_ADDR) + && (s.swarm-controller.environment.BAO_CLIENT_CERT or null) == "%d/bao-client.pem" + && (s.swarm-controller.environment.BAO_CLIENT_KEY or null) == "%d/bao-client-key.pem" + && builtins.elem "bao-client.pem:/var/lib/swarm-bao-pki/controller.pem" s.swarm-controller.serviceConfig.LoadCredential + && builtins.elem "bao-client-key.pem:/var/lib/swarm-bao-pki/controller-key.pem" s.swarm-controller.serviceConfig.LoadCredential; + } + { + # Absence arm for the one above, and what makes it mean anything: a + # controller with no leaf gets no store environment at all rather than + # variables naming files this host never receives. + name = "a controller with no store leaf is given no store environment"; + ok = + let + s = controllerNoStore.systemd.services; + in + s ? swarm-controller + && !(s.swarm-controller.environment ? BAO_ADDR) + && !(lib.any (c: lib.hasPrefix "bao-" c) s.swarm-controller.serviceConfig.LoadCredential); + } { # The CN is an interface between two files: the store writes a role that # matches it, the PKI mints a leaf that carries it. They read one option, diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index 32d2ce10..e6980f69 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -85,6 +85,11 @@ swarm-authelia-bridge-sock.workspace = true # not get to declare them privately. # swarm-queue-client = { workspace = true, features = ["kv"] } +# `matrix_account.rs` writes the credential this daemon's route accepts. Same +# crate the hive reads it back with, which is the point: the path, the field +# name and the object's shape are agreements between the two ends, and a +# second spelling here would be a store this hive could not read. +swarm-secret-client.workspace = true # `otel_http_client.rs`'s `AuthenticatedHttpClient` — an # `opentelemetry_http::HttpClient` impl authenticated with this crate's own # `swarm-queue-client` identity. Lives in this crate rather than diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 962a8dac..b7ca8b75 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -45,6 +45,7 @@ mod auth; mod config_pr; mod forge; mod issue_report; +mod matrix_account; mod otel_http_client; mod status; mod vcs_metrics; @@ -747,23 +748,13 @@ fn agent_status_reader( }) } -/// Both handlers below take the same hive name and reject it the same way. +/// A hive name that is shaped like one and names a hive this swarm has. /// /// Reports the status and the detail rather than a rendered /// `ProblemDetails`: that type is 232 bytes, which makes every `Result` in /// this path pay for the error case it usually does not take. The handlers /// render at the boundary, where the body is actually needed. -fn declaration_target( - state: &AppState, - hive: &str, -) -> Result<(Arc, String), (axum::http::StatusCode, String)> { - let writer = state.wanted.clone().ok_or_else(|| { - ( - axum::http::StatusCode::SERVICE_UNAVAILABLE, - "this deployment wired up no swarm queue, so there is nowhere to publish a declaration" - .to_owned(), - ) - })?; +fn swarm_hive(state: &AppState, hive: &str) -> Result { let hive = hive_types::Ident::parse(hive) .map_err(|reason| (axum::http::StatusCode::BAD_REQUEST, reason.to_owned()))? .into_string(); @@ -776,7 +767,25 @@ fn declaration_target( format!("{hive:?} is not a hive in this swarm"), )); } - Ok((writer, hive)) + Ok(hive) +} + +/// Both declaration handlers below take the same hive name and reject it the +/// same way, and need the writer besides. +fn declaration_target( + state: &AppState, + hive: &str, +) -> Result<(Arc, String), (axum::http::StatusCode, String)> { + // Before the name checks, so a deployment with no queue answers 503 + // whatever the caller spelled. + let writer = state.wanted.clone().ok_or_else(|| { + ( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "this deployment wired up no swarm queue, so there is nowhere to publish a declaration" + .to_owned(), + ) + })?; + Ok((writer, swarm_hive(state, hive)?)) } /// Declare what this swarm wants of one agent on one hive. @@ -1652,6 +1661,7 @@ fn build_app(state: AppState) -> axum::Router { .routes(routes!(get_agents)) .routes(routes!(get_agents_status)) .routes(routes!(set_agent_state)) + .routes(routes!(matrix_account::put_matrix_account)) .routes(routes!(get_hive_wanted)) .routes(routes!(issue_report::get_repos)) .routes(routes!(issue_report::get_issue_report_all)) diff --git a/swarm-controller/src/matrix_account.rs b/swarm-controller/src/matrix_account.rs new file mode 100644 index 00000000..626cb7f5 --- /dev/null +++ b/swarm-controller/src/matrix_account.rs @@ -0,0 +1,144 @@ +//! Give one agent an external matrix account: put the credential in the +//! swarm's secret store, then tell that agent's hive it is there. +//! +//! The hive end is `hive-c0re/src/workers/credential.rs`, which reads the +//! value under its own identity and writes it into the agent's state dir. The +//! notice carries only names, so the queue never holds the secret — see +//! [`swarm_queue_client::credential_subject`] for why that is a requirement +//! rather than a preference. +//! +//! ⚠️ Store first, notify second, and the order cannot be swapped: a notice +//! that overtakes its own write reaches a hive that reads nothing, and the +//! hive deliberately does not retry. + +use axum::Json; +use axum::extract::State; +use axum::http::StatusCode; +use serde::Deserialize; +use swarm_queue_client::{CredentialNotice, credential_subject}; +use swarm_secret_client::{SecretStore, matrix}; +use utoipa::ToSchema; + +use super::{AppState, error_problem, swarm_hive}; + +/// The cert-auth role this daemon logs into the secret store as. +/// +/// `nix/host-modules/swarm-bao.nix`'s `controllerPolicyName` creates the role, +/// names the policy after it, and `nix/module-eval.nix` pins the literal. +/// +/// ⚠️ Not the certificate's CN. The role *matches on* the CN +/// (`allowed_common_names`), so the two are deliberately different strings. +const CERT_ROLE: &str = "swarm-controller"; + +/// The credential to store for one agent's external matrix account. +#[derive(Debug, Deserialize, ToSchema)] +pub struct PutMatrixAccountRequest { + /// The access token. Never logged, and never returned by this route. + token: String, + /// The account's homeserver, when it is not this swarm's own. + /// + /// Stored beside the token rather than sent on the notice: a notice is a + /// queue message, so a homeserver carried there would exist only in + /// flight, with nowhere to reconstruct it from on a re-delivery. + #[schema(example = "https://matrix.example.org")] + homeserver: Option, +} + +/// Store an agent's external matrix account credential and notify its hive. +/// +/// Idempotent: the store keeps versions, so repeating a call replaces the +/// value the agent will next read rather than adding a second account. +#[utoipa::path( + put, + path = "/api/hives/{hive}/agents/{agent}/matrix-accounts/{account}", + params( + ("hive" = String, Path, description = "hive whose agent receives the credential"), + ("agent" = String, Path, description = "agent the credential is delivered to"), + ("account" = String, Path, description = "the external account this credential authenticates as"), + ), + request_body = PutMatrixAccountRequest, + responses( + (status = 200, description = "stored, and the hive has been told"), + (status = 400, description = "a name is not an identifier, the account name is not a single path segment, or the hive is not in this swarm (problem+json)", body = String), + (status = 503, description = "no swarm queue is wired up (problem+json)", body = String), + (status = 500, description = "the store write, the encode or the publish failed (problem+json)", body = String), + ), + tag = "agents" +)] +pub async fn put_matrix_account( + State(state): State, + axum::extract::Path((hive, agent, account)): axum::extract::Path<(String, String, String)>, + Json(req): Json, +) -> Result { + // The queue first, so a deployment that has none answers 503 whatever the + // caller spelled — and before the store is touched, so a request that + // could never be delivered does not leave a credential behind. + let Some(status) = state.status.as_ref() else { + return Err(error_problem( + StatusCode::SERVICE_UNAVAILABLE, + "this deployment wired up no swarm queue, so there is no hive to notify", + )); + }; + let hive = swarm_hive(&state, &hive).map_err(|(s, d)| error_problem(s, &d))?; + let agent = hive_types::Ident::parse(&agent) + .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? + .into_string(); + // Built before the store is reached, so a malformed account name costs a + // parse and not a login. `account_path` is the validator: the store's + // charset is its rule to state, not this handler's to restate. + let secret_path = matrix::account_path(&agent, &account) + .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; + + let store = SecretStore::from_env(CERT_ROLE).await.map_err(|e| { + tracing::warn!(error = %e, "connecting to the swarm secret store failed"); + error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) + })?; + store + .write( + &secret_path, + &matrix::Credential { + value: req.token, + homeserver: req.homeserver, + }, + ) + .await + .map_err(|e| { + // The path names the agent and the account; the value is not in it. + tracing::warn!(path = %secret_path, error = %e, "writing the credential failed"); + error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) + })?; + + let notice = CredentialNotice { + agent: agent.clone(), + account: account.clone(), + }; + let payload = serde_json::to_vec(¬ice).map_err(|e| { + error_problem( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("encoding the credential notice failed: {e}"), + ) + })?; + let subject = credential_subject(&hive); + let client = status.queue_client(); + client + .publish(subject.clone(), payload.into()) + .await + .map_err(|e| { + error_problem( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("publishing to {subject} failed: {e}"), + ) + })?; + // Flushed for the reason `publish_deploy` flushes: `publish` hands the + // message to the connection's write buffer and returns, so without this + // the response can outrun the notice it reports as sent. + client.flush().await.map_err(|e| { + error_problem( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("flushing the credential notice to {subject} failed: {e}"), + ) + })?; + + tracing::info!(%subject, %hive, %agent, %account, "credential stored; hive notified"); + Ok(StatusCode::OK) +} diff --git a/swarm-secret-client/README.md b/swarm-secret-client/README.md index 273865aa..6c4126e6 100644 --- a/swarm-secret-client/README.md +++ b/swarm-secret-client/README.md @@ -61,12 +61,23 @@ read an error about, and none of them needs a reachable store to happen. ## Names that arrive from elsewhere -`path::matrix_account` is fallible, which for a string formatter needs saying: +`matrix::account_path` is fallible, which for a string formatter needs saying: its segments are an agent name from the topology and an account name from that agent's own config. A `/` turns one agent's segment into another agent's directory and `..` walks out of the prefix entirely, so the charset it accepts is deliberately narrower than what the store would. +## One module per kind of secret + +`client` moves whatever type a caller names; it decodes nothing itself. What a +stored object _holds_ is stated in the module that also builds its path — +`matrix` today, and a second kind of swarm secret gets a module beside it. + +The split is deliberate. A single shared struct that grows one field per +consumer ends up carrying, on every path, a field only one path's reader has +ever heard of; and the two things a kind of secret must pin — where it lives +and what is in it — are one agreement that reads worse split across modules. + ## What this crate does not do It has no opinion on **what** a caller may read. That is the policy attached to diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index d2f00a14..74e61af8 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -1,6 +1,6 @@ //! A logged-in handle on the store, built from this deployment's environment. -use serde::{Deserialize, Serialize}; +use serde::{Serialize, de::DeserializeOwned}; use vaultrs::client::{Client, VaultClient, VaultClientSettingsBuilder}; use crate::{Error, path::MOUNT}; @@ -19,14 +19,6 @@ pub const ENV_CACERT: &str = "BAO_CACERT"; /// The auth mount a cert login goes through, unless a caller says otherwise. pub const DEFAULT_CERT_MOUNT: &str = "cert"; -#[derive(Serialize, Deserialize)] -struct Credential { - /// `glue-matrix-bao-token.nix` reads the store with - /// `bao kv get -field=value`, so this name is load-bearing for a reader - /// this crate does not control. Renaming it silently breaks that unit. - value: String, -} - /// Where the store is and which identity we present to it. /// /// Separate from the connect so the environment can be checked without one: @@ -138,25 +130,25 @@ impl SecretStore { Ok(Self { inner }) } - /// Read the credential stored at `path`. + /// Read the object stored at `path`, decoded as `T`. + /// + /// The shape belongs to whoever owns the path — see [`crate::matrix`] for + /// the one kind of secret that exists — so this moves bytes and knows no + /// fields. /// /// # Errors /// [`Error::Vault`] when the path does not exist, the token's policy does - /// not cover it, or the stored object has no `value` field. - pub async fn read(&self, path: &str) -> Result { - let c: Credential = vaultrs::kv2::read(&self.inner, MOUNT, path).await?; - Ok(c.value) + /// not cover it, or the stored object does not decode as `T`. + pub async fn read(&self, path: &str) -> Result { + Ok(vaultrs::kv2::read(&self.inner, MOUNT, path).await?) } - /// Write `value` as the credential at `path`, creating a new version. + /// Write `value` at `path`, creating a new version. /// /// # Errors /// [`Error::Vault`] when the token's policy does not cover the path. - pub async fn write(&self, path: &str, value: &str) -> Result<(), Error> { - let body = Credential { - value: value.to_owned(), - }; - vaultrs::kv2::set(&self.inner, MOUNT, path, &body).await?; + pub async fn write(&self, path: &str, value: &T) -> Result<(), Error> { + vaultrs::kv2::set(&self.inner, MOUNT, path, value).await?; Ok(()) } } @@ -233,16 +225,4 @@ mod tests { other => panic!("wanted an Identity error, got {other:?}"), } } - - #[test] - fn the_value_field_matches_what_the_nix_reader_asks_for() { - // The literal is the point: `bao kv get -field=value` is the other end - // of this agreement and lives in a file no Rust test can reach, so the - // name is pinned here rather than derived from the struct. - let json = serde_json::to_string(&Credential { - value: "t".to_owned(), - }) - .expect("a struct of one String serialises"); - assert_eq!(json, r#"{"value":"t"}"#); - } } diff --git a/swarm-secret-client/src/lib.rs b/swarm-secret-client/src/lib.rs index 02fa1efd..a146e0cd 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -2,12 +2,18 @@ //! reach it. //! //! The HTTP is [`vaultrs`]'s job. What this crate owns is the *agreements* — -//! the path a credential is written to and read from ([`path`]), the field its -//! bytes live in, and the translation from this deployment's environment into -//! a logged-in client ([`client`]). Each of those is a thing the controller and -//! a hive must say identically, so it is said once here. +//! the rules every path obeys ([`path`]), the translation from this +//! deployment's environment into a logged-in client ([`client`]), and, per kind +//! of secret, the path it lives at together with the fields it holds +//! ([`matrix`]). Each of those is a thing the controller and a hive must say +//! identically, so it is said once here. +//! +//! [`client`] is deliberately ignorant of all of it: it moves whatever type a +//! caller names, so a second kind of secret is a new module beside [`matrix`] +//! and not another field on a struct shared with it. pub mod client; +pub mod matrix; pub mod path; pub use client::SecretStore; diff --git a/swarm-secret-client/src/matrix.rs b/swarm-secret-client/src/matrix.rs new file mode 100644 index 00000000..19469b29 --- /dev/null +++ b/swarm-secret-client/src/matrix.rs @@ -0,0 +1,134 @@ +//! The matrix agreement: where an account's credential lives in the store, and +//! what the object at that path holds. +//! +//! Both halves are one agreement and neither end of it is senior, so they are +//! stated together here rather than split between the path module and the +//! client. Nothing in [`crate::client`] knows this shape — it moves whatever +//! type a caller names — so a second kind of swarm secret gets its own module +//! beside this one instead of another field on a shared struct. + +use serde::{Deserialize, Serialize}; + +use crate::{ + Error, + path::{AGENT_PREFIX, checked_segment}, +}; + +/// The path holding `agent`'s token for the external matrix account `account`. +/// +/// # Errors +/// [`Error::PathSegment`] when either name contains anything but +/// `[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)?; + checked_segment("account", account)?; + Ok(format!("{AGENT_PREFIX}/{agent}/matrix/{account}")) +} + +/// What an account's path holds: the token, plus the homeserver it belongs to. +/// +/// The homeserver rides with the token rather than on the queue notice that +/// triggers a delivery, because a notice is not persistence — re-delivering a +/// credential has to reconstruct it, and the store is the only thing that keeps +/// it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Credential { + /// `glue-matrix-bao-token.nix` reads the store with + /// `bao kv get -field=value`, so this name is load-bearing for a reader + /// this crate does not control. Renaming it silently breaks that unit. + pub value: String, + + /// Absent on every object written before this field existed, and KV2 keeps + /// those versions forever. **`Option` is what tolerates that** — serde + /// decodes a missing field to `None` for an optional type, so the type is + /// the compatibility guarantee and changing it to a bare `String` is what + /// would break every stored credential at once. + /// + /// `skip_serializing_if` is doing separate work: without it a token-only + /// credential serialises `"homeserver":null`, and this object is read by + /// `glue-matrix-bao-token.nix` with `bao kv get -field=value`. + #[serde(skip_serializing_if = "Option::is_none")] + pub homeserver: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_well_formed_pair_lands_under_the_agent_prefix() { + let p = account_path("atlas", "ops-relay").expect("both segments are legal"); + assert_eq!(p, "swarm/agents/atlas/matrix/ops-relay"); + assert!(p.starts_with(AGENT_PREFIX)); + } + + #[test] + fn a_segment_cannot_escape_its_own_directory() { + // Each of these is a *different* way to address another agent's tree, + // and the last two are the ones a charset check catches but a + // `contains("..")` check does not. + for bad in [ + "../argus", + "atlas/../argus", + "atlas/matrix", + "a b", + "a.b", + "", + ] { + assert!( + account_path(bad, "ops-relay").is_err(), + "agent segment {bad:?} must be refused" + ); + assert!( + account_path("atlas", bad).is_err(), + "account segment {bad:?} must be refused" + ); + } + } + + #[test] + fn the_legal_charset_is_actually_reachable() { + // The control for the test above: if `checked_segment` rejected + // everything, the escape cases would pass for the wrong reason. + assert!(account_path("a-b_C9", "d-e_F0").is_ok()); + } + + /// KV2 keeps every prior version, so objects written before `homeserver` + /// existed are still readable and still get decoded by this type. What + /// tolerates the absence is the field being `Option`, not any attribute: + /// making it a bare `String` would not surface as a migration, it would + /// surface as every previously-stored credential becoming unreadable at + /// once. + #[test] + fn a_credential_stored_before_the_homeserver_field_still_decodes() { + let old: Credential = serde_json::from_str(r#"{"value":"t"}"#) + .expect("an object written by the previous version must still decode"); + assert_eq!(old.value, "t"); + assert_eq!(old.homeserver, None); + + // Control: the field is genuinely read when present, so the arm above + // is about absence being tolerated rather than the field being ignored. + let new: Credential = serde_json::from_str(r#"{"value":"t","homeserver":"https://hs"}"#) + .expect("an object with the field decodes too"); + assert_eq!(new.homeserver.as_deref(), Some("https://hs")); + } + + #[test] + fn the_value_field_matches_what_the_nix_reader_asks_for() { + // The literal is the point: `bao kv get -field=value` is the other end + // of this agreement and lives in a file no Rust test can reach, so the + // name is pinned here rather than derived from the struct. + // + // It doubles as the compatibility control for `homeserver`: a + // token-only credential must still serialise to exactly these bytes, + // with no `homeserver` key at all, so adding the field cannot change + // what that unit reads. + let json = serde_json::to_string(&Credential { + value: "t".to_owned(), + homeserver: None, + }) + .expect("a struct of one String serialises"); + assert_eq!(json, r#"{"value":"t"}"#); + } +} diff --git a/swarm-secret-client/src/path.rs b/swarm-secret-client/src/path.rs index 8176488e..345ca0ed 100644 --- a/swarm-secret-client/src/path.rs +++ b/swarm-secret-client/src/path.rs @@ -1,8 +1,10 @@ -//! Where a credential lives, for both ends of the store. +//! The rules every path into the store obeys, whatever kind of secret it +//! addresses. //! //! The controller writes and a hive reads, and neither is senior to the other, -//! so the path they must agree on is built here rather than formatted at each -//! call site. +//! so what they must agree on is stated once here rather than at each call +//! site. A *particular* kind of secret builds its path from these pieces in its +//! own module — [`crate::matrix`] is the one that exists. use crate::Error; @@ -52,34 +54,15 @@ pub fn checked_segment(kind: &'static str, value: &str) -> Result<(), Error> { Ok(()) } -/// The path holding `agent`'s token for the external matrix account `account`. -/// -/// # Errors -/// [`Error::PathSegment`] when either name contains anything but -/// `[A-Za-z0-9_-]`, which is what keeps one agent's name from addressing -/// another agent's secret. -pub fn matrix_account(agent: &str, account: &str) -> Result { - checked_segment("agent", agent)?; - checked_segment("account", account)?; - Ok(format!("{AGENT_PREFIX}/{agent}/matrix/{account}")) -} - #[cfg(test)] mod tests { use super::*; #[test] - fn a_well_formed_pair_lands_under_the_agent_prefix() { - let p = matrix_account("atlas", "ops-relay").expect("both segments are legal"); - assert_eq!(p, "swarm/agents/atlas/matrix/ops-relay"); - assert!(p.starts_with(AGENT_PREFIX)); - } - - #[test] - fn a_segment_cannot_escape_its_own_directory() { - // Each of these is a *different* way to address another agent's tree, - // and the last two are the ones a charset check catches but a - // `contains("..")` check does not. + fn a_segment_that_could_change_a_paths_shape_is_refused() { + // Each of these is a *different* way to reach outside the segment the + // caller meant, and the last two are the ones a charset check catches + // but a `contains("..")` check does not. for bad in [ "../argus", "atlas/../argus", @@ -89,12 +72,8 @@ mod tests { "", ] { assert!( - matrix_account(bad, "ops-relay").is_err(), - "agent segment {bad:?} must be refused" - ); - assert!( - matrix_account("atlas", bad).is_err(), - "account segment {bad:?} must be refused" + checked_segment("agent", bad).is_err(), + "segment {bad:?} must be refused" ); } } @@ -102,7 +81,21 @@ mod tests { #[test] fn the_legal_charset_is_actually_reachable() { // The control for the test above: if `checked_segment` rejected - // everything, the escape cases would pass for the wrong reason. - assert!(matrix_account("a-b_C9", "d-e_F0").is_ok()); + // everything, the escape cases would pass for the wrong reason. The + // charset is the one `hive-priv` accepts for the same names on disk, + // so uppercase and underscore have to stay legal here. + assert!(checked_segment("agent", "a-b_C9").is_ok()); + assert!(checked_segment("account", "d-e_F0").is_ok()); + } + + #[test] + fn the_rejected_segment_is_named_in_the_error() { + // The caller usually got the name from config and needs to see which + // of the two it was. + let e = checked_segment("account", "a b").expect_err("a space is not legal"); + assert!( + matches!(e, Error::PathSegment { kind, ref value } if kind == "account" && value == "a b"), + "got {e:?}" + ); } }