diff --git a/Cargo.lock b/Cargo.lock index bd738290..f5a37f9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4701,7 +4701,6 @@ 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 72ee4745..81f5e998 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, matrix}; +use swarm_secret_client::{SecretStore, path}; /// 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 = matrix::account_path(¬ice.agent, ¬ice.account) + 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 credential: matrix::Credential = store + let value = store .read(&secret_path) .await .with_context(|| format!("reading {secret_path} from the store"))?; @@ -52,18 +52,11 @@ 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(), - &credential.value, + &value, Some(¬ice.account), - credential.homeserver.as_deref(), + None, ) .await } @@ -74,16 +67,16 @@ mod tests { #[test] fn a_name_that_could_address_another_agent_is_refused() { - // `matrix::account_path` owns this rule; asserted here because this is + // `path::matrix_account` owns this rule; asserted here because this is // the module that feeds it names off the wire. - 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()); + 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: `matrix::account_path` + // Two independent refusals, not one restated: `path::matrix_account` // 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", ""] { @@ -102,7 +95,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!( - matrix::account_path("dmatrix", bad).is_err(), + path::matrix_account("dmatrix", bad).is_err(), "account {bad:?} must be refused" ); } @@ -110,7 +103,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!(matrix::account_path("dmatrix", "ops-relay").is_ok()); - assert!(matrix::account_path("dmatrix", "Ops_Relay9").is_ok()); + assert!(path::matrix_account("dmatrix", "ops-relay").is_ok()); + assert!(path::matrix_account("dmatrix", "Ops_Relay9").is_ok()); } } diff --git a/nix/host-modules/swarm-controller.nix b/nix/host-modules/swarm-controller.nix index 509cb7fd..a88b483e 100644 --- a/nix/host-modules/swarm-controller.nix +++ b/nix/host-modules/swarm-controller.nix @@ -18,37 +18,6 @@ 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 @@ -676,17 +645,7 @@ in ] ++ lib.optional ( deployCfg.swarm-controller.forgeTokenFile != null - ) "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}"; + ) "forge-token:${deployCfg.swarm-controller.forgeTokenFile}"; # The placeholder default that makes the above non-fatal. # `LoadCredential=` takes priority over `SetCredential=`, so this is @@ -811,7 +770,6 @@ in // webhookEnv // authBridgeEnv // swarmNameEnv - // baoEnv // otelEnv; }; diff --git a/nix/module-eval.nix b/nix/module-eval.nix index 76ee0295..e050c80c 100644 --- a/nix/module-eval.nix +++ b/nix/module-eval.nix @@ -570,42 +570,6 @@ 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 e6980f69..32d2ce10 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -85,11 +85,6 @@ 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 b7ca8b75..962a8dac 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -45,7 +45,6 @@ mod auth; mod config_pr; mod forge; mod issue_report; -mod matrix_account; mod otel_http_client; mod status; mod vcs_metrics; @@ -748,13 +747,23 @@ fn agent_status_reader( }) } -/// A hive name that is shaped like one and names a hive this swarm has. +/// Both handlers below take the same hive name and reject it the same way. /// /// 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 swarm_hive(state: &AppState, hive: &str) -> Result { +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(), + ) + })?; let hive = hive_types::Ident::parse(hive) .map_err(|reason| (axum::http::StatusCode::BAD_REQUEST, reason.to_owned()))? .into_string(); @@ -767,25 +776,7 @@ fn swarm_hive(state: &AppState, hive: &str) -> Result 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)?)) + Ok((writer, hive)) } /// Declare what this swarm wants of one agent on one hive. @@ -1661,7 +1652,6 @@ 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 deleted file mode 100644 index 626cb7f5..00000000 --- a/swarm-controller/src/matrix_account.rs +++ /dev/null @@ -1,144 +0,0 @@ -//! 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 6c4126e6..273865aa 100644 --- a/swarm-secret-client/README.md +++ b/swarm-secret-client/README.md @@ -61,23 +61,12 @@ read an error about, and none of them needs a reachable store to happen. ## Names that arrive from elsewhere -`matrix::account_path` is fallible, which for a string formatter needs saying: +`path::matrix_account` 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 74e61af8..d2f00a14 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::{Serialize, de::DeserializeOwned}; +use serde::{Deserialize, Serialize}; use vaultrs::client::{Client, VaultClient, VaultClientSettingsBuilder}; use crate::{Error, path::MOUNT}; @@ -19,6 +19,14 @@ 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: @@ -130,25 +138,25 @@ impl SecretStore { Ok(Self { inner }) } - /// 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. + /// Read the credential stored at `path`. /// /// # Errors /// [`Error::Vault`] when the path does not exist, the token's policy does - /// 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?) + /// 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) } - /// Write `value` at `path`, creating a new version. + /// Write `value` as the credential 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: &T) -> Result<(), Error> { - vaultrs::kv2::set(&self.inner, MOUNT, path, value).await?; + 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?; Ok(()) } } @@ -225,4 +233,16 @@ 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 a146e0cd..02fa1efd 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -2,18 +2,12 @@ //! reach it. //! //! The HTTP is [`vaultrs`]'s job. What this crate owns is the *agreements* — -//! 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. +//! 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. 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 deleted file mode 100644 index 19469b29..00000000 --- a/swarm-secret-client/src/matrix.rs +++ /dev/null @@ -1,134 +0,0 @@ -//! 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 345ca0ed..8176488e 100644 --- a/swarm-secret-client/src/path.rs +++ b/swarm-secret-client/src/path.rs @@ -1,10 +1,8 @@ -//! The rules every path into the store obeys, whatever kind of secret it -//! addresses. +//! Where a credential lives, for both ends of the store. //! //! The controller writes and a hive reads, and neither is senior to the other, -//! 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. +//! so the path they must agree on is built here rather than formatted at each +//! call site. use crate::Error; @@ -54,15 +52,34 @@ 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_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. + 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. for bad in [ "../argus", "atlas/../argus", @@ -72,8 +89,12 @@ mod tests { "", ] { assert!( - checked_segment("agent", bad).is_err(), - "segment {bad:?} must be refused" + 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" ); } } @@ -81,21 +102,7 @@ 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. 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:?}" - ); + // everything, the escape cases would pass for the wrong reason. + assert!(matrix_account("a-b_C9", "d-e_F0").is_ok()); } }