boxmaker/docs/plans/M5b/02-secrets-move.md

# M5b task 02: the secret store moves to `bxcore`

**Branch:** `m5b` (run `git switch m5b`; `git status --short` must be empty, otherwise stop)
**Commit subject:** `bxcore, gatewayd, loopd: the secret store moves to bxcore::secrets`

## Goal

`gatewayd` reads its Mattermost token through its own secret store, `crates/gatewayd/src/secrets.rs`
(a systemd credential, an environment variable, or an owner-only file). M5c's roles need the same
store, so it moves to `bxcore`, the one crate every role may depend on (M5b spec, section 5). What
`gatewayd` does and prints does not change.

Four parts:

1. **`bxcore`** gains the module `secrets`, given whole (it is moved code). It holds `Secret`,
   `Loaded`, `SecretError`, `load`, `read_checked` and the two runbook pointers, as before, and now
   also `SecretSpec` and `SecretSource`, which were in `gatewayd`'s `config.rs`. The module is
   behind a Cargo feature, `secrets`, and `zeroize` becomes an optional dependency of `bxcore`
   that only the feature turns on.
2. **One change of shape**: the store is no longer only `gatewayd`'s, so `load` and `read_checked`
   take the program's name, `role`, as their first argument. Three sentences named `gatewayd`
   literally; they now name `role`. `gatewayd` passes `"gatewayd"`, so its sentences are the same
   bytes as before.
3. **`gatewayd`** enables the feature, uses `bxcore::secrets`, and loses its own copy: the module,
   `SecretSpec`, `SecretSource`, the `zeroize` dependency, and its two secret test files, which
   move to `bxcore/tests/`. Its configuration keeps `[secrets.<name>]` as it is:
   `Config::secrets` is still a `BTreeMap<String, SecretSpec>`, with `SecretSpec` from `bxcore`.
4. **`loopd`** never enables the feature: it holds no credentials. A given test,
   `loopd/tests/no_secrets.rs`, reads the manifests and fails if anything turns it on for `loopd`.

## Every place the secret store is used

This is the complete list. Each one is covered by an edit below or by a given file.

| Place | What happens there |
|---|---|
| `crates/gatewayd/src/secrets.rs` | deleted; its code is in the given `bxcore/src/secrets.rs` |
| `crates/gatewayd/src/config.rs` | `SecretSpec`, `SecretSource` and `impl SecretSpec` removed, imported from `bxcore` (edits G4 to G6) |
| `crates/gatewayd/src/main.rs` | imports `bxcore::secrets`, passes `"gatewayd"` to `load` (edits G7 to G9) |
| `crates/gatewayd/src/mm/rest.rs`, `crates/gatewayd/src/serve/mod.rs` | import `Secret` from `bxcore` (edits G10 to G13) |
| `crates/gatewayd/src/lib.rs` | no `pub mod secrets` (edit G3) |
| `crates/gatewayd/tests/secrets.rs`, `secrets_race.rs` | deleted; new versions are given in `crates/bxcore/tests/` |
| `crates/gatewayd/tests/config.rs`, `mm_edit.rs`, `mm_rest.rs`, `support/gateway.rs` | new versions are given: only an import changes |
| `crates/gatewayd/tests/main.rs` | not changed: it pins the sentences and pointers, and must still pass |

## Files

- Copy, from `docs/plans/M5b/files/` (the same path under it):
  - new: `crates/bxcore/src/secrets.rs`, `crates/bxcore/tests/secrets.rs`,
    `crates/bxcore/tests/secrets_race.rs`, `crates/bxcore/tests/support/tmp.rs`,
    `crates/loopd/tests/no_secrets.rs`
  - **new versions** of: `crates/gatewayd/tests/config.rs`, `crates/gatewayd/tests/mm_edit.rs`,
    `crates/gatewayd/tests/mm_rest.rs`, `crates/gatewayd/tests/support/gateway.rs`
- Delete with `git rm` (this task tells you to; they were copied in M4a):
  `crates/gatewayd/src/secrets.rs`, `crates/gatewayd/tests/secrets.rs`,
  `crates/gatewayd/tests/secrets_race.rs`
- Modify: `crates/bxcore/Cargo.toml`, `crates/bxcore/src/lib.rs`, `crates/gatewayd/Cargo.toml`,
  `crates/gatewayd/src/lib.rs`, `crates/gatewayd/src/config.rs`, `crates/gatewayd/src/main.rs`,
  `crates/gatewayd/src/mm/rest.rs`, `crates/gatewayd/src/serve/mod.rs`, `docs/dependencies.md`,
  `docs/implementer-log.md`
- `Cargo.lock` changes by itself when you build (`zeroize` moves from `gatewayd` to `bxcore`).

## Facts you need

- **Cargo features.** `zeroize = { workspace = true, optional = true }` makes a dependency that
  is built only when a feature asks for it; `secrets = ["dep:zeroize"]` in `[features]` is that
  feature. `#[cfg(feature = "secrets")]` on the line before `pub mod secrets;` compiles the module
  only with the feature. `bxcore = { workspace = true, features = ["secrets"] }` in `gatewayd`'s
  manifest turns it on for `gatewayd`.
- **`required-features`.** A `[[test]]` entry with `required-features = ["secrets"]` makes Cargo
  skip that test file, without a message, when the feature is off. So `cargo test -p bxcore` does
  not run `tests/secrets.rs` and `tests/secrets_race.rs`; `cargo test -p bxcore --features secrets`
  does.
- **The gate runs them.** `make gate` runs `cargo test --workspace`. Cargo builds `bxcore` once
  for the whole workspace, with every feature any member asks for, so `gatewayd`'s request turns
  `secrets` on and the two files run; the count in step 8 includes them. For the same reason the
  gate cannot see a module that is not behind the feature, which is why `no_secrets.rs` reads
  `bxcore/src/lib.rs`, and why step 7 builds `loopd` on its own (`cargo build -p loopd` builds
  `bxcore` with only what `loopd` asks for).
- **Tests may use `bxcore`.** A crate's integration tests can use its normal dependencies, so
  `gatewayd`'s tests write `use bxcore::secrets::Secret;`.
- **The closure for `env`.** `load` takes `env: &dyn Fn(&str) -> Option<OsString>`.
  `let env = |k: &str| std::env::var_os(k);` then `&env` fits it. The `: &str` is needed: without
  it the compiler cannot infer the closure's argument type (`var_os` is generic).
- **Where things live after this task**, for the edits:

  | Name | Signature |
  |---|---|
  | `bxcore::secrets::load` | `pub fn load(role: &str, name: &str, source: &SecretSource, env: &dyn Fn(&str) -> Option<OsString>) -> Result<Loaded, SecretError>` |
  | `bxcore::secrets::read_checked` | `pub fn read_checked(role: &str, path: &Path, between: &dyn Fn()) -> Result<Zeroizing<Vec<u8>>, String>` |
  | `bxcore::secrets::Secret` | `pub fn new(text: String) -> Secret`, `pub fn expose(&self) -> &str` |
  | `bxcore::secrets::SecretSpec` | `pub fn source(&self) -> Result<SecretSource, String>` |
  | `bxcore::secrets::SecretSource` | `Credential(String)`, `Env(String)`, `File(PathBuf)` |
  | `bxcore::secrets::{RUNBOOK, RUNBOOK_FILE}` | `&str` pointers, unchanged |

- The sentences that now name `role` (with `role = "gatewayd"` each is exactly what it was):
  `CREDENTIALS_DIRECTORY is not set: <role> was not started by systemd with
  LoadCredentialEncrypted=`, `<path> is not owned by the user <role> runs as`, and the warning
  `<role>: warning: secret <name> is read in plaintext from <path>; …`.

## The edits, exactly

Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file.

### `crates/bxcore/Cargo.toml`

**X1.** Replace the whole file with exactly this (it ends after the last line, with one newline):

```toml
[package]
name = "bxcore"
version = "0.1.0"
edition.workspace = true
rust-version.workspace = true
publish.workspace = true

[lints]
workspace = true

[dependencies]
serde.workspace = true
serde_json.workspace = true
humantime.workspace = true
emsha.workspace = true
zeroize = { workspace = true, optional = true }

[dev-dependencies]
toml.workspace = true

# The secret store (M5b spec, section 5). A role that reads secrets enables it; `loopd` never does,
# so its binary does not contain the code that reads them.
[features]
secrets = ["dep:zeroize"]

[[test]]
name = "secrets"
required-features = ["secrets"]

[[test]]
name = "secrets_race"
required-features = ["secrets"]
```

### `crates/bxcore/src/lib.rs`

**X2.** The line

```rust
//! Shared data types and the frame codec for Boxmaker. No policy and no I/O beyond frames.
```

becomes

```rust
//! Shared data types and the frame codec for Boxmaker. No policy and no I/O beyond frames, except
//! the secret store (`secrets`), behind the feature of that name.
```

**X3.** The lines

```rust
pub mod memory;
pub mod sha1;
```

become

```rust
pub mod memory;
#[cfg(feature = "secrets")]
pub mod secrets;
pub mod sha1;
```

### `crates/gatewayd/Cargo.toml`

**G1.** The line

```toml
bxcore.workspace = true
```

becomes

```toml
bxcore = { workspace = true, features = ["secrets"] }
```

**G2.** The lines

```toml
toml.workspace = true
zeroize.workspace = true
```

become

```toml
toml.workspace = true
```

### `crates/gatewayd/src/lib.rs`

**G3.** The lines

```rust
pub mod net;
pub mod secrets;
pub mod serve;
```

become

```rust
pub mod net;
pub mod serve;
```

### `crates/gatewayd/src/config.rs`

**G4.** The lines

```rust
use std::path::{Path, PathBuf};

use serde::Deserialize;
```

become

```rust
use std::path::{Path, PathBuf};

use bxcore::secrets::{SecretSource, SecretSpec};
use serde::Deserialize;
```

**G5.** The lines

```rust
    pub ca_file: Option<PathBuf>,
}

/// Where one secret comes from: exactly one of the three is set (checked by `Config::load`).
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Default)]
#[serde(deny_unknown_fields)]
pub struct SecretSpec {
    #[serde(default)]
    pub credential: Option<String>,
    #[serde(default)]
    pub env: Option<String>,
    #[serde(default)]
    pub file: Option<PathBuf>,
}

/// A checked `SecretSpec`.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum SecretSource {
    Credential(String),
    Env(String),
    File(PathBuf),
}

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct AllowConfig {
```

become

```rust
    pub ca_file: Option<PathBuf>,
}

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct AllowConfig {
```

**G6.** The lines

```rust
        self.paths.home.join("gateway/state.json")
    }
}

impl SecretSpec {
    /// Exactly one source, well formed.
    pub fn source(&self) -> Result<SecretSource, String> {
        // Exactly one of the three set, else "needs exactly one of credential, env and file".
        // credential: not empty, only ASCII letters, digits, _ . -. env: not empty, only A-Z, 0-9,
        // _. file: an absolute path. The messages are in the task.
        let mut count = 0;
        if self.credential.is_some() {
            count += 1;
        }
        if self.env.is_some() {
            count += 1;
        }
        if self.file.is_some() {
            count += 1;
        }
        if count != 1 {
            return Err("needs exactly one of credential, env and file".to_string());
        }
        if let Some(credential) = &self.credential {
            if credential.is_empty()
                || !credential.bytes().all(|b| {
                    b.is_ascii_alphabetic() || b.is_ascii_digit() || matches!(b, b'_' | b'.' | b'-')
                })
            {
                return Err(format!(
                    "credential {:?} is not a credential name (letters, digits, _ . -)",
                    credential
                ));
            }
            return Ok(SecretSource::Credential(credential.clone()));
        }
        if let Some(env) = &self.env {
            if env.is_empty()
                || !env
                    .bytes()
                    .all(|b| matches!(b, b'A'..=b'Z' | b'0'..=b'9' | b'_'))
            {
                return Err(format!(
                    "env {:?} is not a variable name (A-Z, 0-9, _)",
                    env
                ));
            }
            return Ok(SecretSource::Env(env.clone()));
        }
        // `count == 1` and the two above are `None`, so this is `Some`.
        let file = match &self.file {
            Some(file) => file,
            None => return Err("needs exactly one of credential, env and file".to_string()),
        };
        if !file.is_absolute() {
            return Err(format!(
                "file {:?} must be an absolute path",
                file.display()
            ));
        }
        Ok(SecretSource::File(file.clone()))
    }
}

/// A Mattermost id: 26 characters of `a-z0-9`.
```

become

```rust
        self.paths.home.join("gateway/state.json")
    }
}

/// A Mattermost id: 26 characters of `a-z0-9`.
```

The same code is in the given `bxcore/src/secrets.rs`; `Config::problem` and `token_source` call
`source()` as before, now through the import of G4.

### `crates/gatewayd/src/main.rs`

**G7.** The lines

```rust
use gatewayd::config::{Config, MATTERMOST_TOKEN};
use gatewayd::secrets;
```

become

```rust
use bxcore::secrets;
use gatewayd::config::{Config, MATTERMOST_TOKEN};
```

**G8.** In `serve`'s comment, the lines

```rust
    // 3. `let loaded = match secrets::load(MATTERMOST_TOKEN, &source, &|k| std::env::var_os(k))`:
    //    an Err(e) prints "gatewayd: {e}" (the error carries its own pointer).
```

become

```rust
    // 3. `let env = |k: &str| std::env::var_os(k);`, then
    //    `let loaded = match secrets::load("gatewayd", MATTERMOST_TOKEN, &source, &env)`: an Err(e)
    //    prints "gatewayd: {e}" (the error carries its own pointer).
```

**G9.** The line

```rust
    let loaded = match secrets::load(MATTERMOST_TOKEN, &source, &|k| std::env::var_os(k)) {
```

becomes

```rust
    let env = |k: &str| std::env::var_os(k);
    let loaded = match secrets::load("gatewayd", MATTERMOST_TOKEN, &source, &env) {
```

### `crates/gatewayd/src/mm/rest.rs`

**G10.** The lines

```rust
use serde::Deserialize;

use crate::config::valid_id;
```

become

```rust
use bxcore::secrets::Secret;
use serde::Deserialize;

use crate::config::valid_id;
```

**G11.** The lines

```rust
use crate::net::Connector;
use crate::secrets::Secret;
use crate::ws::conn::host_header;
```

become

```rust
use crate::net::Connector;
use crate::ws::conn::host_header;
```

### `crates/gatewayd/src/serve/mod.rs`

**G12.** The line

```rust
use bxcore::SessionId;
```

becomes

```rust
use bxcore::SessionId;
use bxcore::secrets::Secret;
```

**G13.** The lines

```rust
use crate::net::Connector;
use crate::secrets::Secret;
use crate::sessions::{Queues, Router};
```

become

```rust
use crate::net::Connector;
use crate::sessions::{Queues, Router};
```

### `docs/dependencies.md`

**D1.** The line

```text
| `zeroize` | 1.9.0 | `gatewayd` | Wipes a secret's memory when it is dropped (`Secret`). Already a dependency of `rustls`. Apache-2.0 OR MIT. |
```

becomes

```text
| `zeroize` | 1.9.0 | `bxcore`, feature `secrets` (enabled by `gatewayd`; never by `loopd`) | Wipes a secret's memory when it is dropped (`Secret`). Already a dependency of `rustls`. Apache-2.0 OR MIT. |
```

## Steps

- [ ] **1. Copy the given tests.** From the repository root, nine commands:

  ```sh
  mkdir -p crates/bxcore/tests/support
  cp docs/plans/M5b/files/crates/bxcore/tests/secrets.rs crates/bxcore/tests/
  cp docs/plans/M5b/files/crates/bxcore/tests/secrets_race.rs crates/bxcore/tests/
  cp docs/plans/M5b/files/crates/bxcore/tests/support/tmp.rs crates/bxcore/tests/support/
  cp docs/plans/M5b/files/crates/loopd/tests/no_secrets.rs crates/loopd/tests/
  cp docs/plans/M5b/files/crates/gatewayd/tests/config.rs crates/gatewayd/tests/
  cp docs/plans/M5b/files/crates/gatewayd/tests/mm_edit.rs crates/gatewayd/tests/
  cp docs/plans/M5b/files/crates/gatewayd/tests/mm_rest.rs crates/gatewayd/tests/
  cp docs/plans/M5b/files/crates/gatewayd/tests/support/gateway.rs crates/gatewayd/tests/support/
  ```

  Then `git status --short` lists four modified files (`config.rs`, `mm_edit.rs`, `mm_rest.rs`,
  `support/gateway.rs` under `crates/gatewayd/tests/`) and four new entries
  (`crates/bxcore/tests/secrets.rs`, `crates/bxcore/tests/secrets_race.rs`,
  `crates/bxcore/tests/support/`, `crates/loopd/tests/no_secrets.rs`).
- [ ] **2. See them fail.**
  - `cargo build --tests --keep-going -p bxcore -p gatewayd -p loopd`: ten test targets do not
    compile: `secrets` and `secrets_race` in `bxcore`; `config`, `mm_edit`, `mm_rest`, `serve`,
    `serve_answers`, `serve_approvals`, `serve_redirect` and `serve_restart` in `gatewayd` (the
    last five through `tests/support/gateway.rs`). Every error is `E0432`, unresolved import
    `bxcore::secrets`.
  - `cargo test -p loopd --test no_secrets`: two of its four tests fail,
    `only_the_feature_itself_turns_the_store_on` (`no [features]`) and
    `the_module_is_compiled_only_with_the_feature`; the other two pass.
- [ ] **3. `bxcore`.** Copy the module:

  ```sh
  cp docs/plans/M5b/files/crates/bxcore/src/secrets.rs crates/bxcore/src/
  ```

  Make edits X1, X2 and X3. Then:
  - `cargo test -p bxcore --features secrets`: every suite passes, among them the eleven tests of
    `secrets` and the five of `secrets_race`.
  - `cargo test -p bxcore`: every suite passes, and `tests/secrets.rs` does not run (see "Facts").
  - `cargo test -p loopd --test no_secrets`: all four pass.
- [ ] **4. `gatewayd`.** Delete its copy:

  ```sh
  git rm crates/gatewayd/src/secrets.rs crates/gatewayd/tests/secrets.rs \
    crates/gatewayd/tests/secrets_race.rs
  ```

  Make edits G1 to G13. `cargo test -p gatewayd`: every suite passes, among them `main`, whose
  tests pin the sentences and pointers.
- [ ] **5. `docs/dependencies.md`.** Make edit D1.
- [ ] **6. Format.** `cargo fmt --all`. Then `git diff --stat` lists no file this task does not
  name.
- [ ] **7. Check.** Run each command from the repository root, one at a time:

  ```sh
  grep -rn "zeroize\|crate::secrets\|gatewayd::secrets" crates/gatewayd
  cargo tree --offline -p loopd -e features | grep -c 'bxcore feature "secrets"'
  cargo tree --offline -p gatewayd -e features | grep -c 'bxcore feature "secrets"'
  cargo build --offline -p loopd
  git status --short crates/loopd
  git diff --stat -- crates/brokerd crates/memoryd crates/bxctl crates/inferproxy crates/toolkit
  ```

  Expected, in order: the `grep` prints nothing; the first `cargo tree` line prints `0` (`grep -c`
  exits 1 when it counts nothing; here that is the expected result); the second prints `1`; the
  build of `loopd` alone finishes without an error; `git status` prints only
  `?? crates/loopd/tests/no_secrets.rs`; the `git diff` prints nothing.
- [ ] **8. Run the gate.** `make gate`, with a shell timeout of 600000 ms. Expected last line:
  `gate: ok`, with 963 tests: 956 before this task; the thirteen tests of `gatewayd`'s two secret
  files now run in `bxcore` (eight in `secrets.rs`, five in `secrets_race.rs`); plus three new
  ones in `bxcore/tests/secrets.rs` and four in `loopd/tests/no_secrets.rs`.
- [ ] **9. Log and commit.** Stage with

  ```sh
  git add crates/bxcore crates/gatewayd crates/loopd docs/dependencies.md \
    docs/implementer-log.md Cargo.lock
  ```

  then commit as `AGENTS.md` says. `git status --short` must then be empty.

## Done when

- `make gate` prints `gate: ok` with 963 tests.
- `crates/gatewayd/src/secrets.rs` does not exist, and the three checks of step 7 hold.

## Stop and report if

- Any quoted "before" block is not in its file exactly as shown, or is there more than once.
- Step 2 shows a different result: another target failing, an error other than `E0432`, or
  another test of `no_secrets` failing.
- After step 3 or 4, a test fails. Do not change a test, and do not change the given
  `bxcore/src/secrets.rs`.
- `cargo tree` in step 7 shows `bxcore feature "secrets"` for `loopd`.
- A test in `brokerd`, `memoryd`, `bxctl`, `inferproxy` or `toolkit` fails in the gate: check once
  whether it fails without your change, then stop and report both results.
- The gate passes with a count other than 963.
- A comment in an edit does not fit the code under it. Quote it and stop.