swarm-secret-client: carry the homeserver with the credential, not on the notice

A delivered matrix account needs two things: the token and the homeserver it
belongs to. Only the token was stored, so the homeserver had to ride on the
queue notice — and a notice is not persistence. Re-delivering a credential
(agent moved, hive re-provisioned, token rotated) has to reconstruct it from
somewhere, and there is nowhere; keeping it separately at swarm level would be
a second store for one logical object, free to drift from the first.

So `Credential` grows a `homeserver` field and `read`/`write` carry the whole
object rather than a bare string.

`value` keeps its name. `nix/host-modules/glue-matrix-bao-token.nix` reads the
store with `bao kv get -field=value` and is the only nix reader of it, checked
rather than assumed — so this had to be an addition, never a rename.

Two compatibility properties, both of which fail silently if broken:

KV2 keeps every prior version, so objects written before this field existed
are still decoded by this type. What tolerates their absence is the field
being `Option` — a bare `String` would not fail as a migration, every stored
credential would become unreadable at once. The new test pins that, with a
presence control so the arm is about absence being tolerated rather than the
field being ignored.

`skip_serializing_if` keeps a token-only credential serialising to exactly the
bytes the previous version wrote, with no `homeserver` key rather than a null,
which is what that nix reader would otherwise trip over. The existing test
pinning `{"value":"t"}` proves it and became the control for free.

Mutation testing earned its place here: `#[serde(default)]` was in the first
draft and its comment claimed it was what made old objects decode. Dropping it
changed nothing — serde already decodes a missing field to `None` for an
optional type — so the attribute was redundant and the comment was wrong about
its own mechanism. Both removed rather than left to mislead the next reader.

The delivery half needed no change: `write_agent_matrix_token` already took a
homeserver and already wrote the `matrix-account-<name>.json` sidecar beside
the token. `deliver` simply stops passing `None`. A credential stored without
one still works exactly as before — no sidecar, and the account needs a
configured entry.

Refs #3726

hive-c0re/src/workers/credential.rs

@@ -42,7 +42,7 @@ pub async fn deliver(notice: &CredentialNotice, cert_role: &str) -> Result<()> {
     let store = SecretStore::from_env(cert_role)
         .await
         .context("connecting to the swarm secret store")?;
-    let value = store
+    let 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-<name>.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(&notice.account),
-        None,
+        credential.homeserver.as_deref(),
     )
     .await
 }

swarm-secret-client/src/client.rs

@@ -19,12 +19,31 @@ 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 {
+/// What a credential path holds: the secret itself, plus the configuration a
+/// reader needs to use it.
+///
+/// The homeserver rides here rather than on the queue notice because a notice
+/// is not persistence — re-delivering a credential (agent moved, hive
+/// re-provisioned, token rotated) has to reconstruct it from somewhere, 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.
-    value: String,
+    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<String>,
 }
 
 /// Where the store is and which identity we present to it.
@@ -143,20 +162,16 @@ impl SecretStore {
     /// # 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<String, Error> {
-        let c: Credential = vaultrs::kv2::read(&self.inner, MOUNT, path).await?;
-        Ok(c.value)
+    pub async fn read(&self, path: &str) -> Result<Credential, Error> {
+        Ok(vaultrs::kv2::read(&self.inner, MOUNT, path).await?)
     }
 
-    /// Write `value` as the credential at `path`, creating a new version.
+    /// Write `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: &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, credential: &Credential) -> Result<(), Error> {
+        vaultrs::kv2::set(&self.inner, MOUNT, path, credential).await?;
         Ok(())
     }
 }
@@ -234,13 +249,40 @@ mod tests {
         }
     }
 
+    /// 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. Mutation-tested — dropping `#[serde(default)]` changed nothing,
+    /// which is how the redundant attribute was found and removed.
+    #[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"}"#);