diff --git a/docs/plans/M5b/02-secrets-move.md b/docs/plans/M5b/02-secrets-move.md new file mode 100644 index 0000000..a9bfd8b --- /dev/null +++ b/docs/plans/M5b/02-secrets-move.md @@ -0,0 +1,581 @@ +# 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.]` as it is: + `Config::secrets` is still a `BTreeMap`, 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`. + `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) -> Result` | + | `bxcore::secrets::read_checked` | `pub fn read_checked(role: &str, path: &Path, between: &dyn Fn()) -> Result>, String>` | + | `bxcore::secrets::Secret` | `pub fn new(text: String) -> Secret`, `pub fn expose(&self) -> &str` | + | `bxcore::secrets::SecretSpec` | `pub fn source(&self) -> Result` | + | `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: was not started by systemd with + LoadCredentialEncrypted=`, ` is not owned by the user runs as`, and the warning + `: warning: secret is read in plaintext from ; …`. + +## 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, +} + +/// 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, + #[serde(default)] + pub env: Option, + #[serde(default)] + pub file: Option, +} + +/// 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, +} + +#[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 { + // 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. diff --git a/docs/plans/M5b/files/crates/bxcore/src/secrets.rs b/docs/plans/M5b/files/crates/bxcore/src/secrets.rs new file mode 100644 index 0000000..3359444 --- /dev/null +++ b/docs/plans/M5b/files/crates/bxcore/src/secrets.rs @@ -0,0 +1,312 @@ +//! The secret store, for every role that needs one: a secret from a systemd credential, an +//! environment variable, or an owner-only file (M4a spec, section 4; M5b spec, section 5). A +//! `Secret` cannot be printed. Behind the feature `secrets`: `loopd` holds no credentials and never +//! enables it. + +use std::ffi::OsString; +use std::io::Read; +use std::os::unix::fs::MetadataExt; +use std::path::{Path, PathBuf}; + +use serde::Deserialize; +use zeroize::Zeroizing; + +pub const RUNBOOK: &str = "see docs/runbook.md#secret-unavailable"; +pub const RUNBOOK_FILE: &str = "see docs/runbook.md#secret-in-a-file"; + +/// `[secrets.]` in a role's configuration: exactly one of the three is set (checked by +/// `source`). Our own format: unknown keys are errors. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Default)] +#[serde(deny_unknown_fields)] +pub struct SecretSpec { + #[serde(default)] + pub credential: Option, + #[serde(default)] + pub env: Option, + #[serde(default)] + pub file: Option, +} + +/// A checked `SecretSpec`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SecretSource { + Credential(String), + Env(String), + File(PathBuf), +} + +impl SecretSpec { + /// Exactly one source, well formed. + pub fn source(&self) -> Result { + // 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. + 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 secret's text. No `Display`; `Debug` shows nothing of it; wiped when dropped. +pub struct Secret(Zeroizing); + +impl Secret { + pub fn new(text: String) -> Secret { + Secret(Zeroizing::new(text)) + } + + /// The text, for the one place that must send it. + pub fn expose(&self) -> &str { + &self.0 + } +} + +impl std::fmt::Debug for Secret { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Secret(…)") + } +} + +/// A loaded secret, and the warning to print for it, if any. +#[derive(Debug)] +pub struct Loaded { + pub secret: Secret, + pub warning: Option, +} + +#[derive(Debug)] +pub struct SecretError { + pub name: String, + pub why: String, +} + +impl std::fmt::Display for SecretError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "secret {}: {}\n{RUNBOOK}", self.name, self.why) + } +} + +impl std::error::Error for SecretError {} + +/// Load secret `name` from `source` for the program `role` (`"gatewayd"`), which the sentences +/// name. `env` reads an environment variable (in a role, `std::env::var_os`); tests pass their +/// own. +pub fn load( + role: &str, + name: &str, + source: &SecretSource, + env: &dyn Fn(&str) -> Option, +) -> Result { + // By source (M4a spec, section 4). Credential: read $CREDENTIALS_DIRECTORY/ (through + // `env`, not std::env); an unset variable, or a file that cannot be read, is an error. Env: the + // variable, UTF-8; unset is an error. File: `read_checked` checks the path, opens and re-checks + // the same file handle, reads it, and a warning is set. Every value goes through `value`. + // Every error is a `SecretError` naming the secret, never the value. + match source { + SecretSource::Credential(cred) => { + let dir = match env("CREDENTIALS_DIRECTORY") { + Some(dir) => dir, + None => { + return Err(SecretError { + name: name.to_string(), + why: format!( + "CREDENTIALS_DIRECTORY is not set: {role} was not started by systemd with LoadCredentialEncrypted=" + ), + }); + } + }; + let path = Path::new(dir.as_os_str()).join(cred); + let bytes = match std::fs::read(&path) { + Ok(bytes) => bytes, + Err(e) => { + return Err(SecretError { + name: name.to_string(), + why: format!("cannot read the credential {}: {}", path.display(), e), + }); + } + }; + let secret = value(bytes).map_err(|why| SecretError { + name: name.to_string(), + why, + })?; + Ok(Loaded { + secret, + warning: None, + }) + } + SecretSource::Env(var) => { + let val = match env(var) { + Some(val) => val, + None => { + return Err(SecretError { + name: name.to_string(), + why: format!("the environment variable {} is not set", var), + }); + } + }; + let text = match val.to_str() { + Some(text) => text, + None => { + return Err(SecretError { + name: name.to_string(), + why: format!("the environment variable {} is not UTF-8", var), + }); + } + }; + let secret = value(text.as_bytes().to_vec()).map_err(|why| SecretError { + name: name.to_string(), + why, + })?; + Ok(Loaded { + secret, + warning: None, + }) + } + SecretSource::File(path) => { + let mut bytes = match read_checked(role, path, &|| {}) { + Ok(bytes) => bytes, + Err(why) => { + return Err(SecretError { + name: name.to_string(), + why, + }); + } + }; + let secret = value(std::mem::take(&mut *bytes)).map_err(|why| SecretError { + name: name.to_string(), + why, + })?; + Ok(Loaded { + secret, + warning: Some(format!( + "{role}: warning: secret {} is read in plaintext from {}; a systemd credential keeps it encrypted at rest ({RUNBOOK_FILE})", + name, + path.display() + )), + }) + } + } +} + +/// The bytes of an owner-only regular file, read from the very file that was checked. `role` +/// names the program in the owner check's sentence. `between` runs after the path is checked and +/// before it is opened: `load` passes `&|| {}`; tests use it to swap the file. +pub fn read_checked( + role: &str, + path: &Path, + between: &dyn Fn(), +) -> Result>, String> { + // The path is checked (absolute, not a link, a regular file), then opened, and the checks + // that matter are made on the open handle: the same file as the path named, owned by this + // user, no group or other bits. The race it closes is M4a review, finding 2. + if !path.is_absolute() { + return Err(format!("{} is not an absolute path", path.display())); + } + let named = std::fs::symlink_metadata(path) + .map_err(|e| format!("cannot read {}: {}", path.display(), e))?; + if named.file_type().is_symlink() { + return Err(format!("{} is a symbolic link", path.display())); + } + if !named.file_type().is_file() { + return Err(format!("{} is not a regular file", path.display())); + } + between(); + let mut file = + std::fs::File::open(path).map_err(|e| format!("cannot read {}: {}", path.display(), e))?; + let opened = file + .metadata() + .map_err(|e| format!("cannot read {}: {}", path.display(), e))?; + if (opened.dev(), opened.ino()) != (named.dev(), named.ino()) { + return Err(format!("{} changed while it was read", path.display())); + } + let owner = user_id()?; + if opened.uid() != owner { + return Err(format!( + "{} is not owned by the user {role} runs as", + path.display() + )); + } + let mode = opened.mode() & 0o777; + if mode & 0o077 != 0 { + return Err(format!( + "{} has mode {:03o}; only the owner may read it (0600 or 0400)", + path.display(), + mode + )); + } + let mut bytes = Zeroizing::new(Vec::new()); + file.read_to_end(&mut bytes) + .map_err(|e| format!("cannot read {}: {}", path.display(), e))?; + Ok(bytes) +} + +/// The user id this process runs as: the owner of a pipe it has just made. `std` has no call for +/// it, and `/proc/self` is Linux only. +fn user_id() -> Result { + let (reader, _writer) = std::io::pipe().map_err(|e| format!("cannot make a pipe: {e}"))?; + let pipe = std::fs::File::from(std::os::fd::OwnedFd::from(reader)); + let meta = pipe + .metadata() + .map_err(|e| format!("cannot read a pipe: {e}"))?; + Ok(meta.uid()) +} + +/// The text without one trailing newline; not empty; UTF-8. +fn value(bytes: Vec) -> Result { + // Keep the bytes in `Zeroizing` until they are inside the `Secret`. + let bytes = Zeroizing::new(bytes); + let text = std::str::from_utf8(&bytes).map_err(|_| "the value is not UTF-8".to_string())?; + let text = text.strip_suffix('\n').unwrap_or(text); + if text.is_empty() { + return Err("the value is empty".to_string()); + } + Ok(Secret::new(text.to_string())) +} diff --git a/docs/plans/M5b/files/crates/bxcore/tests/secrets.rs b/docs/plans/M5b/files/crates/bxcore/tests/secrets.rs new file mode 100644 index 0000000..822d94a --- /dev/null +++ b/docs/plans/M5b/files/crates/bxcore/tests/secrets.rs @@ -0,0 +1,285 @@ +//! The secret store (M4a spec, section 4), moved from `gatewayd` to `bxcore` in M5b (spec, section +//! 5) with its behaviour unchanged. The environment is passed in as a function, so no test changes +//! the process's environment. Run with `--features secrets`. Do not edit. + +#[path = "support/tmp.rs"] +mod tmp; + +use std::collections::HashMap; +use std::ffi::OsString; +use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; + +use bxcore::secrets::{RUNBOOK, RUNBOOK_FILE, SecretSource, SecretSpec, load}; +use tmp::TempDir; + +const TOKEN: &str = "s3cret-t0ken-value"; + +fn env_of(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option + use<> { + let map: HashMap = pairs + .iter() + .map(|(k, v)| (k.to_string(), OsString::from(v))) + .collect(); + move |k| map.get(k).cloned() +} + +fn owner_file(dir: &TempDir, name: &str, text: &str, mode: u32) -> PathBuf { + let path = dir.write(name, text); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)).unwrap(); + path +} + +fn refused(source: &SecretSource, env: &dyn Fn(&str) -> Option, word: &str) { + let e = load("gatewayd", "mattermost_token", source, env).expect_err(word); + let text = e.to_string(); + assert!(text.contains(word), "{word}: {text}"); + assert!(text.starts_with("secret mattermost_token: "), "{text}"); + assert!(text.ends_with(RUNBOOK), "{text}"); + assert!( + !text.contains(TOKEN), + "a refusal never shows the value: {text}" + ); +} + +#[test] +fn a_systemd_credential() { + let dir = TempDir::new("cred"); + dir.write("creds/mattermost-token", &format!("{TOKEN}\n")); + let creds = dir.path().join("creds"); + let env = env_of(&[("CREDENTIALS_DIRECTORY", creds.to_str().unwrap())]); + let got = load( + "gatewayd", + "mattermost_token", + &SecretSource::Credential("mattermost-token".into()), + &env, + ) + .unwrap(); + assert_eq!( + got.secret.expose(), + TOKEN, + "one trailing newline is removed" + ); + assert_eq!(got.warning, None); +} + +#[test] +fn a_credential_outside_systemd_or_missing_is_refused() { + let src = SecretSource::Credential("mattermost-token".into()); + refused(&src, &env_of(&[]), "CREDENTIALS_DIRECTORY is not set"); + let dir = TempDir::new("cred-missing"); + refused( + &src, + &env_of(&[("CREDENTIALS_DIRECTORY", dir.path().to_str().unwrap())]), + "cannot read the credential", + ); +} + +#[test] +fn an_environment_variable() { + let got = load( + "gatewayd", + "mattermost_token", + &SecretSource::Env("MM".into()), + &env_of(&[("MM", TOKEN)]), + ) + .unwrap(); + assert_eq!(got.secret.expose(), TOKEN); + assert_eq!(got.warning, None, "only a file warns"); + refused(&SecretSource::Env("MM".into()), &env_of(&[]), "is not set"); + refused( + &SecretSource::Env("MM".into()), + &env_of(&[("MM", "")]), + "empty", + ); +} + +#[test] +fn an_owner_only_file_is_read_with_a_warning() { + let dir = TempDir::new("file"); + for mode in [0o600, 0o400] { + let path = owner_file( + &dir, + &format!("token-{mode:o}"), + &format!("{TOKEN}\n"), + mode, + ); + let got = load( + "gatewayd", + "mattermost_token", + &SecretSource::File(path.clone()), + &env_of(&[]), + ) + .unwrap(); + assert_eq!(got.secret.expose(), TOKEN); + let warning = got.warning.expect("a file secret warns"); + assert!( + warning.starts_with( + "gatewayd: warning: secret mattermost_token is read in plaintext from " + ), + "{warning}" + ); + assert!(warning.contains(path.to_str().unwrap()), "{warning}"); + assert!(warning.contains(RUNBOOK_FILE), "{warning}"); + assert!(!warning.contains(TOKEN)); + } +} + +#[test] +fn a_file_anyone_else_can_read_or_that_is_not_a_plain_file_is_refused() { + let dir = TempDir::new("file-bad"); + for (mode, _) in [ + (0o640, "group"), + (0o604, "other"), + (0o644, "both"), + (0o660, "group write"), + ] { + let path = owner_file(&dir, &format!("t-{mode:o}"), TOKEN, mode); + refused( + &SecretSource::File(path), + &env_of(&[]), + "only the owner may read it", + ); + } + let target = owner_file(&dir, "real", TOKEN, 0o600); + let link = dir.path().join("link"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + refused(&SecretSource::File(link), &env_of(&[]), "symbolic link"); + std::fs::create_dir(dir.path().join("adir")).unwrap(); + refused( + &SecretSource::File(dir.path().join("adir")), + &env_of(&[]), + "not a regular file", + ); + refused( + &SecretSource::File(dir.path().join("missing")), + &env_of(&[]), + "cannot read", + ); + refused( + &SecretSource::File(PathBuf::from("relative/token")), + &env_of(&[]), + "absolute", + ); +} + +#[test] +fn a_file_another_user_owns_is_refused() { + // A regular file that root owns on Linux, NixOS and macOS; the tests never run as root. + refused( + &SecretSource::File(PathBuf::from("/etc/passwd")), + &env_of(&[]), + "/etc/passwd is not owned by the user gatewayd runs as", + ); +} + +#[test] +fn empty_and_non_utf8_values_are_refused() { + let dir = TempDir::new("value"); + refused( + &SecretSource::File(owner_file(&dir, "empty", "", 0o600)), + &env_of(&[]), + "empty", + ); + refused( + &SecretSource::File(owner_file(&dir, "nl", "\n", 0o600)), + &env_of(&[]), + "empty", + ); + let path = dir.path().join("bin"); + std::fs::write(&path, [0xff, 0xfe]).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + refused(&SecretSource::File(path), &env_of(&[]), "not UTF-8"); +} + +#[test] +fn only_one_trailing_newline_is_removed_and_spaces_stay() { + let dir = TempDir::new("trim"); + let path = owner_file(&dir, "t", " a b \n\n", 0o600); + let got = load("gatewayd", "x", &SecretSource::File(path), &env_of(&[])).unwrap(); + assert_eq!(got.secret.expose(), " a b \n"); +} + +#[test] +fn a_secret_prints_nothing_of_itself() { + let got = load( + "gatewayd", + "mattermost_token", + &SecretSource::Env("MM".into()), + &env_of(&[("MM", TOKEN)]), + ) + .unwrap(); + let shown = format!("{:?} {:?}", got.secret, got); + assert!(!shown.contains(TOKEN), "{shown}"); + assert!(shown.contains("Secret(…)"), "{shown}"); +} + +#[test] +fn the_sentences_name_the_role_that_loads() { + let e = load( + "memoryd", + "honcho_key", + &SecretSource::Credential("honcho-key".into()), + &env_of(&[]), + ) + .expect_err("no CREDENTIALS_DIRECTORY"); + assert!( + e.to_string() + .contains("memoryd was not started by systemd with LoadCredentialEncrypted="), + "{e}" + ); + let dir = TempDir::new("role"); + let path = owner_file(&dir, "key", TOKEN, 0o600); + let got = load( + "memoryd", + "honcho_key", + &SecretSource::File(path), + &env_of(&[]), + ) + .unwrap(); + let warning = got.warning.expect("a file secret warns"); + assert!( + warning.starts_with("memoryd: warning: secret honcho_key is read in plaintext from "), + "{warning}" + ); + let e = load( + "memoryd", + "honcho_key", + &SecretSource::File(PathBuf::from("/etc/passwd")), + &env_of(&[]), + ) + .expect_err("root owns it"); + assert!( + e.to_string() + .contains("/etc/passwd is not owned by the user memoryd runs as"), + "{e}" + ); +} + +#[test] +fn a_spec_names_exactly_one_well_formed_source() { + let spec = |text: &str| toml::from_str::(text).unwrap().source(); + assert_eq!( + spec("credential = \"mattermost-token\""), + Ok(SecretSource::Credential("mattermost-token".into())) + ); + assert_eq!(spec("env = \"MM_2\""), Ok(SecretSource::Env("MM_2".into()))); + assert_eq!( + spec("file = \"/run/token\""), + Ok(SecretSource::File(PathBuf::from("/run/token"))) + ); + for (text, word) in [ + ("", "needs exactly one of credential, env and file"), + ("env = \"A\"\nfile = \"/t\"", "needs exactly one"), + ("credential = \"\"", "is not a credential name"), + ("credential = \"a/b\"", "is not a credential name"), + ("env = \"mm\"", "is not a variable name"), + ("file = \"token\"", "must be an absolute path"), + ] { + let why = spec(text).expect_err(text); + assert!(why.contains(word), "{text:?}: {why}"); + } + assert!( + toml::from_str::("env = \"A\"\npath = \"/t\"").is_err(), + "unknown keys are refused" + ); +} diff --git a/docs/plans/M5b/files/crates/bxcore/tests/secrets_race.rs b/docs/plans/M5b/files/crates/bxcore/tests/secrets_race.rs new file mode 100644 index 0000000..76a171a --- /dev/null +++ b/docs/plans/M5b/files/crates/bxcore/tests/secrets_race.rs @@ -0,0 +1,92 @@ +//! A secret file is read from the file that was checked, never from whatever the path names a +//! moment later (M4a review, finding 2). `read_checked` runs `between` after checking the path and +//! before opening it; each test swaps something there. Moved from `gatewayd` in M5b; run with +//! `--features secrets`. Do not edit. + +#[path = "support/tmp.rs"] +mod tmp; + +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; + +use bxcore::secrets::read_checked; +use tmp::TempDir; + +const TOKEN: &str = "the-real-token"; +const OTHER: &str = "a-file-the-owner-never-chose"; + +fn owner_file(dir: &TempDir, name: &str, text: &str) -> PathBuf { + let path = dir.write(name, text); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + path +} + +fn refused(path: &Path, between: &dyn Fn(), word: &str) { + let why = read_checked("gatewayd", path, between).expect_err(word); + assert!(why.contains(word), "{word}: {why}"); + assert!(why.contains(&path.display().to_string()), "{why}"); + assert!( + !why.contains(TOKEN) && !why.contains(OTHER), + "never a value: {why}" + ); +} + +#[test] +fn an_untouched_file_is_read() { + let dir = TempDir::new("race-ok"); + let path = owner_file(&dir, "token", &format!("{TOKEN}\n")); + let bytes = read_checked("gatewayd", &path, &|| {}).unwrap(); + assert_eq!(bytes.as_slice(), format!("{TOKEN}\n").as_bytes()); +} + +#[test] +fn a_file_swapped_for_a_link_is_refused() { + let dir = TempDir::new("race-link"); + let path = owner_file(&dir, "token", TOKEN); + let other = owner_file(&dir, "other", OTHER); + let swap = || { + std::fs::remove_file(&path).unwrap(); + std::os::unix::fs::symlink(&other, &path).unwrap(); + }; + refused(&path, &swap, "changed while it was read"); +} + +#[test] +fn a_file_swapped_for_another_file_is_refused() { + let dir = TempDir::new("race-rename"); + let path = owner_file(&dir, "token", TOKEN); + let other = owner_file(&dir, "other", OTHER); + let swap = || std::fs::rename(&other, &path).unwrap(); + refused(&path, &swap, "changed while it was read"); +} + +#[test] +fn the_checks_hold_for_the_file_that_is_read() { + let dir = TempDir::new("race-mode"); + let path = owner_file(&dir, "token", TOKEN); + let widen = || { + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap(); + }; + refused(&path, &widen, "has mode 644"); +} + +#[test] +fn the_path_checks_still_come_first() { + let dir = TempDir::new("race-first"); + let other = owner_file(&dir, "other", OTHER); + let link = dir.path().join("link"); + std::os::unix::fs::symlink(&other, &link).unwrap(); + refused( + &link, + &|| panic!("never reached for a link"), + "is a symbolic link", + ); + refused( + dir.path(), + &|| panic!("never reached for a directory"), + "is not a regular file", + ); + let relative = Path::new("relative/token"); + let why = read_checked("gatewayd", relative, &|| panic!("never reached")).unwrap_err(); + assert!(why.contains("is not an absolute path"), "{why}"); +} diff --git a/docs/plans/M5b/files/crates/bxcore/tests/support/tmp.rs b/docs/plans/M5b/files/crates/bxcore/tests/support/tmp.rs new file mode 100644 index 0000000..3a1229a --- /dev/null +++ b/docs/plans/M5b/files/crates/bxcore/tests/support/tmp.rs @@ -0,0 +1,40 @@ +//! Temporary directories for tests. Do not edit. + +#![allow(dead_code)] // each test file uses a different part of this module + +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU32, Ordering}; + +static NEXT: AtomicU32 = AtomicU32::new(0); + +pub struct TempDir(PathBuf); + +impl TempDir { + pub fn new(tag: &str) -> TempDir { + let n = NEXT.fetch_add(1, Ordering::SeqCst); + let path = std::env::temp_dir().join(format!("bx-{tag}-{}-{n}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).unwrap(); + TempDir(path) + } + + pub fn path(&self) -> &Path { + &self.0 + } + + /// Writes `text` to `name` inside the directory and returns the full path. + pub fn write(&self, name: &str, text: &str) -> PathBuf { + let path = self.0.join(name); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&path, text).unwrap(); + path + } +} + +impl Drop for TempDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} diff --git a/docs/plans/M5b/files/crates/gatewayd/tests/config.rs b/docs/plans/M5b/files/crates/gatewayd/tests/config.rs new file mode 100644 index 0000000..6c5b906 --- /dev/null +++ b/docs/plans/M5b/files/crates/gatewayd/tests/config.rs @@ -0,0 +1,247 @@ +//! `gatewayd.toml` (M4a spec, section 3). Do not edit. + +#[path = "support/tmp.rs"] +mod tmp; + +use std::path::PathBuf; + +use bxcore::secrets::SecretSource; +use gatewayd::config::{Config, ConfigError, ServerUrl, parse_url, valid_id}; +use tmp::TempDir; + +const OWNER: &str = "abcdefghijklmnopqrstuvwxyz"; +const CHANNEL: &str = "0123456789abcdefghijklmnop"; + +fn minimal() -> String { + format!( + "[mattermost]\nurl = \"https://straylight.scylla-hammerhead.ts.net\"\n\ + [secrets.mattermost_token]\ncredential = \"mattermost-token\"\n\ + [allow]\nusers = [\"{OWNER}\"]\n" + ) +} + +fn load(text: &str) -> Result { + let dir = TempDir::new("cfg"); + let path = dir.write("gatewayd.toml", text); + Config::load(&path) +} + +fn invalid(text: &str) -> String { + match load(text) { + Err(ConfigError::Invalid(_, why)) => why, + other => panic!("expected Invalid for {text:?}, got {other:?}"), + } +} + +#[test] +fn a_minimal_config_gets_every_default() { + let c = load(&minimal()).unwrap(); + assert_eq!( + c.server().unwrap(), + ServerUrl { + tls: true, + host: "straylight.scylla-hammerhead.ts.net".to_string(), + port: 443 + } + ); + assert_eq!(c.mattermost.ca_file, None); + assert_eq!( + c.token_source().unwrap(), + SecretSource::Credential("mattermost-token".to_string()) + ); + assert_eq!(c.allow.users, vec![OWNER.to_string()]); + assert!(c.allow.channels.is_empty()); + assert_eq!((c.limits.queue, c.limits.typing_every_ms), (20, 3_000)); + assert_eq!( + (c.limits.ping_every_ms, c.limits.dead_after_ms), + (30_000, 60_000) + ); + let home = std::env::var_os("BOXMAKER_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("/var/lib/boxmaker")); + assert_eq!(c.loop_socket(), home.join("run/loop/loop.sock")); + assert_eq!(c.state_path(), home.join("gateway/state.json")); +} + +#[test] +fn every_value_can_be_set() { + let text = format!( + "[mattermost]\nurl = \"http://127.0.0.1:8065\"\nca_file = \"/etc/boxmaker/ca.pem\"\n\ + [secrets.mattermost_token]\nfile = \"/home/k/.config/boxmaker/token\"\n\ + [allow]\nusers = [\"{OWNER}\"]\nchannels = [\"{CHANNEL}\"]\n\ + [loop]\nsocket = \"/run/l.sock\"\n[paths]\nhome = \"/h\"\n\ + [limits]\nqueue = 5\ntyping_every_ms = 1\nping_every_ms = 2\ndead_after_ms = 3\n" + ); + let c = load(&text).unwrap(); + assert_eq!( + c.server().unwrap(), + ServerUrl { + tls: false, + host: "127.0.0.1".to_string(), + port: 8065 + } + ); + assert_eq!( + c.mattermost.ca_file, + Some(PathBuf::from("/etc/boxmaker/ca.pem")) + ); + assert_eq!( + c.token_source().unwrap(), + SecretSource::File(PathBuf::from("/home/k/.config/boxmaker/token")) + ); + assert_eq!(c.allow.channels, vec![CHANNEL.to_string()]); + assert_eq!(c.loop_socket(), PathBuf::from("/run/l.sock")); + assert_eq!(c.state_path(), PathBuf::from("/h/gateway/state.json")); + assert_eq!( + ( + c.limits.queue, + c.limits.typing_every_ms, + c.limits.ping_every_ms, + c.limits.dead_after_ms + ), + (5, 1, 2, 3) + ); +} + +#[test] +fn an_env_secret() { + let text = minimal().replace( + "credential = \"mattermost-token\"", + "env = \"BOXMAKER_MM_TOKEN\"", + ); + assert_eq!( + load(&text).unwrap().token_source().unwrap(), + SecretSource::Env("BOXMAKER_MM_TOKEN".to_string()) + ); +} + +#[test] +fn urls() { + for (url, tls, host, port) in [ + ("https://a.example", true, "a.example", 443), + ("https://a.example:8443", true, "a.example", 8443), + ("http://localhost", false, "localhost", 80), + ("http://127.0.0.1:8065", false, "127.0.0.1", 8065), + ] { + assert_eq!( + parse_url(url), + Ok(ServerUrl { + tls, + host: host.to_string(), + port + }), + "{url}" + ); + } + for url in [ + "", + "a.example", + "ftp://a.example", + "https://", + "https://a.example/", + "https://a.example/api", + "https://A.example", + "https://a.example:0", + "https://a.example:65536", + "https://a.example:0443", + "https://a.example:", + "https://user@a.example", + "https://a.example?x", + "https://.a.example", + "https://a.example.", + "https://[::1]:443", + "https:// a.example", + ] { + assert!(parse_url(url).is_err(), "{url:?} must be refused"); + } +} + +#[test] +fn ids() { + assert!(valid_id(OWNER)); + assert!(valid_id(CHANNEL)); + for id in [ + "", + "abc", + "abcdefghijklmnopqrstuvwxyZ", + "abcdefghijklmnopqrstuvwxy-", + "abcdefghijklmnopqrstuvwxyza", + ] { + assert!(!valid_id(id), "{id:?}"); + } +} + +#[test] +fn bad_values_are_named() { + let token = "credential = \"mattermost-token\""; + let cases: Vec<(String, &str)> = vec![ + ( + minimal().replace( + "https://straylight.scylla-hammerhead.ts.net", + "https://x.example/path", + ), + "url", + ), + ( + minimal() + .replace( + "[secrets.mattermost_token]", + "[mattermost2]\n[secrets.other]", + ) + .replace("[mattermost2]\n", ""), + "mattermost_token", + ), + ( + minimal().replace(token, "credential = \"a b\""), + "credential", + ), + (minimal().replace(token, "env = \"lower\""), "env"), + ( + minimal().replace(token, "file = \"relative/token\""), + "absolute", + ), + ( + minimal().replace(token, "credential = \"x\"\nenv = \"Y\""), + "exactly one", + ), + (minimal().replace(token, ""), "exactly one"), + ( + minimal().replace(&format!("[\"{OWNER}\"]"), "[]"), + "users is empty", + ), + (minimal().replace(OWNER, "tooshort"), "not a Mattermost id"), + ( + format!("{}channels = [\"NOTANID\"]\n", minimal()), + "not a Mattermost id", + ), + ( + minimal().replace("[mattermost]\n", "[mattermost]\nca_file = \"ca.pem\"\n"), + "ca_file", + ), + (format!("{}[limits]\nqueue = 0\n", minimal()), "queue"), + ( + format!("{}[limits]\ndead_after_ms = 0\n", minimal()), + "dead_after_ms", + ), + ]; + for (text, word) in cases { + let why = invalid(&text); + assert!(why.contains(word), "{word}: {why}"); + } +} + +#[test] +fn unknown_keys_and_missing_tables_are_parse_errors() { + for text in [ + format!("{}[allow2]\n", minimal()), + minimal().replace("[mattermost]\n", "[mattermost]\nproxy = \"x\"\n"), + minimal().replace( + "credential = \"mattermost-token\"", + "credential = \"t\"\nkeyring = \"x\"", + ), + minimal().replace(&format!("[allow]\nusers = [\"{OWNER}\"]\n"), ""), + format!("{}[limits]\nqueue = -1\n", minimal()), + ] { + assert!(matches!(load(&text), Err(ConfigError::Parse(..))), "{text}"); + } +} diff --git a/docs/plans/M5b/files/crates/gatewayd/tests/mm_edit.rs b/docs/plans/M5b/files/crates/gatewayd/tests/mm_edit.rs new file mode 100644 index 0000000..f2d92b7 --- /dev/null +++ b/docs/plans/M5b/files/crates/gatewayd/tests/mm_edit.rs @@ -0,0 +1,129 @@ +//! Editing a post and looking up a username, against a scripted server (M4b spec, sections 6 and +//! 7). Do not edit. + +#[path = "support/http_server.rs"] +mod http_server; +#[path = "support/tls_server.rs"] +mod tls_server; + +use std::net::SocketAddr; +use std::time::Duration; + +use bxcore::secrets::Secret; +use gatewayd::config::ServerUrl; +use gatewayd::mm::MmError; +use gatewayd::mm::rest::Client; +use gatewayd::net::Connector; +use http_server::{reply, serve_http}; +use serde_json::json; + +const BOT: &str = "b0000000000000000000000000"; +const KYLE: &str = "k0000000000000000000000000"; +const DM: &str = "d0000000000000000000000000"; +const POST: &str = "p0000000000000000000000001"; +const WAIT: Duration = Duration::from_secs(5); + +fn client(addr: SocketAddr) -> Client { + let url = ServerUrl { + tls: false, + host: "localhost".to_string(), + port: addr.port(), + }; + Client::new( + Connector::new(url, None).unwrap(), + Secret::new("TOKEN".to_string()), + WAIT, + ) +} + +#[test] +fn a_post_is_patched_with_its_new_text() { + let (addr, rx) = serve_http(None, |_, r| { + let text = r.json()["message"].as_str().unwrap().to_string(); + let post = json!({"id": POST, "user_id": BOT, "channel_id": DM, "root_id": "", "message": text, "create_at": 5, "is_pinned": false}); + reply(200, "", &post.to_string()) + }); + client(addr) + .patch_post( + POST, + "**Approval 42**\n✅ Approved by @kyle: the call runs.", + ) + .unwrap(); + let r = rx.recv_timeout(WAIT).unwrap(); + assert_eq!( + (r.method.as_str(), r.path.as_str()), + ("PUT", format!("/api/v4/posts/{POST}/patch").as_str()) + ); + assert!( + r.head.contains("Authorization: Bearer TOKEN\r\n"), + "{}", + r.head + ); + assert_eq!( + r.json(), + json!({"message": "**Approval 42**\n✅ Approved by @kyle: the call runs."}) + ); +} + +#[test] +fn a_post_that_is_not_an_id_is_never_sent() { + let (addr, rx) = serve_http(None, |_, _| reply(200, "", "{}")); + for bad in ["../users/me", "", "P0000000000000000000000001"] { + assert!( + matches!(client(addr).patch_post(bad, "x"), Err(MmError::Json(_))), + "{bad:?}" + ); + assert!( + matches!(client(addr).username(bad), Err(MmError::Json(_))), + "{bad:?}" + ); + } + std::thread::sleep(Duration::from_millis(100)); + assert!(rx.try_recv().is_err(), "nothing was sent"); +} + +#[test] +fn a_refused_edit_is_an_error() { + let (addr, _rx) = serve_http(None, |_, _| { + reply( + 403, + "", + r#"{"id":"api.post.update_post.permissions_details.app_error"}"#, + ) + }); + assert!(matches!( + client(addr).patch_post(POST, "x"), + Err(MmError::Auth(403)) + )); +} + +#[test] +fn the_username_of_a_user() { + let (addr, rx) = serve_http(None, |_, _| { + reply( + 200, + "", + &json!({"id": KYLE, "username": "kyle", "first_name": "K", "roles": "system_user"}) + .to_string(), + ) + }); + assert_eq!(client(addr).username(KYLE).unwrap(), "kyle"); + let r = rx.recv_timeout(WAIT).unwrap(); + assert_eq!( + (r.method.as_str(), r.path.as_str()), + ("GET", format!("/api/v4/users/{KYLE}").as_str()) + ); + for bad in [ + json!({"id": KYLE}), + json!({"id": KYLE, "username": ""}), + json!({"id": KYLE, "username": "Kyle\n@channel"}), + json!({"id": KYLE, "username": "x".repeat(65)}), + ] { + let body = bad.to_string(); + let (addr, _rx) = serve_http(None, move |_, _| reply(200, "", &body)); + assert!( + matches!(client(addr).username(KYLE), Err(MmError::Json(_))), + "{bad}" + ); + } +} diff --git a/docs/plans/M5b/files/crates/gatewayd/tests/mm_rest.rs b/docs/plans/M5b/files/crates/gatewayd/tests/mm_rest.rs new file mode 100644 index 0000000..c7fe5bc --- /dev/null +++ b/docs/plans/M5b/files/crates/gatewayd/tests/mm_rest.rs @@ -0,0 +1,242 @@ +//! Mattermost's REST calls against a scripted server: the requests on the wire, and what a 401, +//! 403, 429, 5xx or 404 does (M4a spec, sections 5 and 9). Do not edit. + +#[path = "support/http_server.rs"] +mod http_server; +#[path = "support/tls_server.rs"] +mod tls_server; + +use std::net::SocketAddr; +use std::sync::mpsc::Receiver; +use std::time::{Duration, Instant}; + +use bxcore::secrets::Secret; +use gatewayd::config::ServerUrl; +use gatewayd::mm::MmError; +use gatewayd::mm::rest::Client; +use gatewayd::net::Connector; +use http_server::{Request, reply, serve_http}; +use serde_json::json; +use tls_server::{fixture, server_config}; + +const BOT: &str = "b0000000000000000000000000"; +const KYLE: &str = "k0000000000000000000000000"; +const DM: &str = "d0000000000000000000000000"; +const POST: &str = "p0000000000000000000000001"; + +fn client(addr: SocketAddr, tls: bool) -> Client { + let url = ServerUrl { + tls, + host: "localhost".to_string(), + port: addr.port(), + }; + let ca = tls.then(|| fixture("test-ca.pem")); + let connector = Connector::new(url, ca.as_deref()).unwrap(); + Client::new( + connector, + Secret::new("TOKEN".to_string()), + Duration::from_secs(5), + ) +} + +fn me_body() -> String { + json!({"id": BOT, "username": "boxmaker-straylight", "roles": "system_user", "is_bot": true}) + .to_string() +} + +fn requests(rx: &Receiver) -> Vec { + rx.try_iter().collect() +} + +#[test] +fn me_plain_and_over_tls() { + for tls in [false, true] { + let config = tls.then(|| server_config("server")); + let (addr, rx) = serve_http(config, |_, _| reply(200, "", &me_body())); + let me = client(addr, tls).me().unwrap(); + assert_eq!( + (me.id.as_str(), me.username.as_str()), + (BOT, "boxmaker-straylight") + ); + let r = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!( + (r.method.as_str(), r.path.as_str()), + ("GET", "/api/v4/users/me") + ); + assert!( + r.head.contains("Authorization: Bearer TOKEN\r\n"), + "{}", + r.head + ); + assert!( + r.head + .contains(&format!("Host: localhost:{}\r\n", addr.port())), + "{}", + r.head + ); + } +} + +#[test] +fn create_post_sends_the_thread_and_reads_the_post_back() { + let (addr, rx) = serve_http(None, |_, r| { + let mut p = r.json(); + p["id"] = json!(POST); + p["user_id"] = json!(BOT); + p["create_at"] = json!(5); + reply(201, "", &p.to_string()) + }); + let post = client(addr, false) + .create_post(DM, KYLE, "an answer\nin two lines") + .unwrap(); + assert_eq!((post.id.as_str(), post.root_id.as_str()), (POST, KYLE)); + let r = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!( + (r.method.as_str(), r.path.as_str()), + ("POST", "/api/v4/posts") + ); + assert!( + r.head.contains("Content-Type: application/json\r\n"), + "{}", + r.head + ); + assert_eq!( + r.json(), + json!({"channel_id": DM, "root_id": KYLE, "message": "an answer\nin two lines"}) + ); +} + +#[test] +fn posts_since_and_the_direct_channel() { + let (addr, rx) = serve_http(None, |_, r| { + if r.path.contains("/posts?since=") { + reply(200, "", r#"{"order":[],"posts":{}}"#) + } else { + reply(201, "", &json!({"id": DM, "type": "D"}).to_string()) + } + }); + let c = client(addr, false); + assert!( + c.posts_since(DM, 1_758_650_000_000) + .unwrap() + .posts + .is_empty() + ); + assert_eq!(c.direct_channel(BOT, KYLE).unwrap(), DM); + let rs = requests(&rx); + assert_eq!( + rs[0].path, + format!("/api/v4/channels/{DM}/posts?since=1758650000000") + ); + assert_eq!( + (rs[1].method.as_str(), rs[1].path.as_str()), + ("POST", "/api/v4/channels/direct") + ); + assert_eq!(rs[1].json(), json!([BOT, KYLE])); +} + +#[test] +fn a_channel_that_is_not_an_id_is_never_sent() { + let (addr, rx) = serve_http(None, |_, _| reply(200, "", r#"{"order":[],"posts":{}}"#)); + let err = client(addr, false) + .posts_since("../users/me?x=", 0) + .unwrap_err(); + assert!(matches!(err, MmError::Json(_)), "{err}"); + std::thread::sleep(Duration::from_millis(100)); + assert!(requests(&rx).is_empty()); +} + +#[test] +fn a_refused_token_stops_at_once() { + for status in [401, 403] { + let (addr, rx) = serve_http(None, move |_, _| { + reply( + status, + "", + r#"{"id":"api.context.session_expired.app_error"}"#, + ) + }); + let err = client(addr, false).me().unwrap_err(); + assert!(matches!(err, MmError::Auth(s) if s == status), "{err}"); + std::thread::sleep(Duration::from_millis(100)); + assert_eq!(requests(&rx).len(), 1, "no retry on {status}"); + } +} + +#[test] +fn a_server_error_is_tried_twice_more() { + let (addr, rx) = serve_http(None, |n, _| { + if n < 2 { + reply(503, "", "{}") + } else { + reply(200, "", &me_body()) + } + }); + assert!(client(addr, false).me().is_ok()); + assert_eq!(requests(&rx).len(), 3); + + let (addr, rx) = serve_http(None, |_, _| reply(502, "", "{}")); + let err = client(addr, false).me().unwrap_err(); + assert!(matches!(err, MmError::Status(502, _)), "{err}"); + std::thread::sleep(Duration::from_millis(100)); + assert_eq!(requests(&rx).len(), 3); +} + +#[test] +fn a_rate_limit_is_waited_out() { + let (addr, rx) = serve_http(None, |n, _| { + if n == 0 { + reply(429, "X-Ratelimit-Reset: 1\r\n", "{}") + } else { + reply(200, "", &me_body()) + } + }); + let started = Instant::now(); + assert!(client(addr, false).me().is_ok()); + assert!( + started.elapsed() >= Duration::from_millis(900), + "{:?}", + started.elapsed() + ); + assert_eq!(requests(&rx).len(), 2); + + let (addr, _rx) = serve_http(None, |_, _| reply(429, "X-Ratelimit-Reset: 1\r\n", "{}")); + assert!(matches!( + client(addr, false).me(), + Err(MmError::RateLimited(_)) + )); +} + +#[test] +fn other_statuses_keep_the_start_of_the_body() { + let long = format!("{{\"message\":\"{}\"}}", "x".repeat(1000)); + let (addr, _rx) = serve_http(None, move |_, _| reply(404, "", &long)); + let Err(MmError::Status(404, body)) = client(addr, false).me() else { + panic!("not a 404") + }; + assert_eq!(body.chars().count(), 200); +} + +#[test] +fn answers_that_are_not_what_we_expect() { + for body in [ + "not json", + r#"{"id":"short","username":"x"}"#, + r#"{"id":"b0000000000000000000000000","username":""}"#, + ] { + let (addr, _rx) = serve_http(None, move |_, _| reply(200, "", body)); + assert!( + matches!(client(addr, false).me(), Err(MmError::Json(_))), + "{body}" + ); + } +} + +#[test] +fn nobody_listening_is_a_network_error() { + let port = std::net::TcpListener::bind("127.0.0.1:0") + .unwrap() + .local_addr() + .unwrap(); + assert!(matches!(client(port, false).me(), Err(MmError::Net(_)))); +} diff --git a/docs/plans/M5b/files/crates/gatewayd/tests/support/gateway.rs b/docs/plans/M5b/files/crates/gatewayd/tests/support/gateway.rs new file mode 100644 index 0000000..8d25106 --- /dev/null +++ b/docs/plans/M5b/files/crates/gatewayd/tests/support/gateway.rs @@ -0,0 +1,153 @@ +//! Running `gatewayd`'s serve loop in a test, against the fake Mattermost and the fake `loopd`. +//! Do not edit. + +#![allow(dead_code)] // each test file uses a different part of this module + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex, mpsc}; +use std::thread::JoinHandle; +use std::time::Duration; + +use bxcore::Turn; +use bxcore::secrets::Secret; +use gatewayd::config::Config; +use gatewayd::serve::{Stop, Tuning, run}; + +use crate::fake_loop::{done, serve_loop}; +use crate::fake_mm::{FakeMm, KYLE, WsPeer}; +use crate::tmp::TempDir; + +pub const SHARED: &str = "c0000000000000000000000000"; +pub const OTHER: &str = "o0000000000000000000000000"; +pub const WAIT: Duration = Duration::from_secs(5); + +pub struct Running { + pub stop: Arc, + pub log: Arc>>, + pub handle: Option>, +} + +impl Running { + pub fn log(&self) -> Vec { + self.log.lock().unwrap().clone() + } + + pub fn wait_log(&self, part: &str) -> Vec { + let until = std::time::Instant::now() + WAIT; + while !self.log().iter().any(|l| l.contains(part)) { + assert!( + std::time::Instant::now() < until, + "no log line with {part:?}: {:?}", + self.log() + ); + std::thread::sleep(Duration::from_millis(10)); + } + self.log() + } + + /// The `Stop` `run` returns by itself within 5 s; after that it is stopped, and the test fails. + pub fn join_within(mut self) -> Stop { + let handle = self.handle.take().unwrap(); + let until = std::time::Instant::now() + WAIT; + while !handle.is_finished() && std::time::Instant::now() < until { + std::thread::sleep(Duration::from_millis(10)); + } + self.stop.store(true, Ordering::SeqCst); + let stop = handle.join().unwrap(); + assert!(!matches!(stop, Stop::Asked), "run did not stop by itself"); + stop + } + + pub fn finish(mut self) -> Stop { + self.stop.store(true, Ordering::SeqCst); + self.handle.take().unwrap().join().unwrap() + } +} + +impl Drop for Running { + fn drop(&mut self) { + self.stop.store(true, Ordering::SeqCst); + // Wait for `run` to finish before the test's home is removed, or it can recreate its + // socket directory there afterwards (tip T44). The wait has a limit (T27). + if let Some(handle) = self.handle.take() { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !handle.is_finished() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + if handle.is_finished() { + let _ = handle.join(); + } + } + } +} + +pub fn config(home: &TempDir, url: &str, extra: &str) -> Config { + let text = format!( + r#" +[mattermost] +url = "{url}" +[secrets.mattermost_token] +env = "NOT_READ_BY_RUN" +[allow] +users = ["{KYLE}"] +channels = ["{SHARED}"] +[paths] +home = "{}" +[limits] +typing_every_ms = 100 +{extra} +"#, + home.path().display() + ); + Config::parse(&text).unwrap() +} + +pub fn start(config: Config) -> Running { + let stop = Arc::new(AtomicBool::new(false)); + let log = Arc::new(Mutex::new(Vec::new())); + let tuning = Tuning { + backoff: vec![Duration::from_millis(50)], + poll: Duration::from_millis(20), + rest_timeout: WAIT, + notice: Duration::from_millis(300), + }; + let (s, l) = (Arc::clone(&stop), Arc::clone(&log)); + let handle = std::thread::spawn(move || { + let sink: gatewayd::serve::Log = + Arc::new(move |line: &str| l.lock().unwrap().push(line.to_string())); + run(config, Secret::new("TOKEN".to_string()), tuning, sink, &s) + }); + Running { + stop, + log, + handle: Some(handle), + } +} + +/// A fake loop that answers every turn with "answer to ", after `delay`. +pub fn answering(home: &TempDir, delay: Duration) -> mpsc::Receiver { + serve_loop(&home.path().join("run/loop/loop.sock"), move |_, turn| { + std::thread::sleep(delay); + vec![done(&format!("answer to {}", turn.content))] + }) +} + +pub fn read_state(home: &TempDir) -> serde_json::Value { + let text = std::fs::read_to_string(home.path().join("gateway/state.json")).unwrap(); + serde_json::from_str(&text).unwrap() +} + +pub fn loop_dir(home: &TempDir) { + std::fs::create_dir_all(home.path().join("run/loop")).unwrap(); +} + +/// Start with a fake Mattermost and a fake loop; the first WebSocket is returned. +pub fn up(home: &TempDir, delay: Duration) -> (FakeMm, Running, mpsc::Receiver, WsPeer) { + loop_dir(home); + let turns = answering(home, delay); + let mm = FakeMm::start(); + let running = start(config(home, &mm.url(), "")); + let ws = mm.next_ws(WAIT); + running.wait_log("gatewayd: connected to "); + (mm, running, turns, ws) +} diff --git a/docs/plans/M5b/files/crates/loopd/tests/no_secrets.rs b/docs/plans/M5b/files/crates/loopd/tests/no_secrets.rs new file mode 100644 index 0000000..fbdd8bf --- /dev/null +++ b/docs/plans/M5b/files/crates/loopd/tests/no_secrets.rs @@ -0,0 +1,95 @@ +//! `loopd` holds no credentials, so no build of it enables `bxcore`'s feature `secrets` (M5b spec, +//! section 5), and its binary does not contain the code that reads them. Three manifests decide +//! that: `loopd`'s own, the workspace's entry for `bxcore`, and `bxcore`'s feature table. They are +//! parsed, so every spelling TOML allows is read. `make gate` builds the workspace with the feature +//! on (for `gatewayd`), so it cannot see a module that is not behind the feature; the last test +//! checks that line of `bxcore`'s `lib.rs`. Do not edit. + +use std::path::{Path, PathBuf}; + +fn manifest(path: &Path) -> toml::Table { + let text = std::fs::read_to_string(path) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", path.display())); + text.parse::() + .unwrap_or_else(|e| panic!("cannot parse {}: {e}", path.display())) +} + +fn crates() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("..") +} + +/// Every string in `value`, at any depth, that turns the feature on: `secrets`, `bxcore/secrets` +/// or `bxcore?/secrets`. +fn enabling(value: &toml::Value, found: &mut Vec) { + match value { + toml::Value::String(s) if s == "secrets" || s.ends_with("/secrets") => { + found.push(s.clone()) + } + toml::Value::Array(items) => items.iter().for_each(|v| enabling(v, found)), + toml::Value::Table(table) => table.values().for_each(|v| enabling(v, found)), + _ => {} + } +} + +#[test] +fn loopd_never_names_the_feature() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"); + let table = manifest(&path); + assert!(table.contains_key("dependencies"), "{}", path.display()); + let mut found = Vec::new(); + enabling(&toml::Value::Table(table), &mut found); + assert!(found.is_empty(), "{} enables {found:?}", path.display()); +} + +#[test] +fn the_workspace_entry_for_bxcore_enables_nothing() { + let path = crates().join("../Cargo.toml"); + let table = manifest(&path); + let entry = table + .get("workspace") + .and_then(|w| w.get("dependencies")) + .and_then(|d| d.get("bxcore")) + .unwrap_or_else(|| panic!("{}: no [workspace.dependencies] bxcore", path.display())); + assert_eq!(entry.get("features"), None, "{entry:?}"); +} + +#[test] +fn only_the_feature_itself_turns_the_store_on() { + let path = crates().join("bxcore/Cargo.toml"); + let table = manifest(&path); + let features = table + .get("features") + .and_then(toml::Value::as_table) + .unwrap_or_else(|| panic!("{}: no [features]", path.display())); + assert!(features.contains_key("secrets"), "{features:?}"); + for (name, list) in features { + if name == "secrets" { + continue; + } + let mut found = Vec::new(); + enabling(list, &mut found); + assert!(found.is_empty(), "feature {name} turns on {found:?}"); + } + let zeroize = table + .get("dependencies") + .and_then(|d| d.get("zeroize")) + .unwrap_or_else(|| panic!("{}: no zeroize", path.display())); + assert_eq!( + zeroize.get("optional").and_then(toml::Value::as_bool), + Some(true), + "zeroize is built only with the feature" + ); +} + +#[test] +fn the_module_is_compiled_only_with_the_feature() { + let path = crates().join("bxcore/src/lib.rs"); + let text = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", path.display())); + assert!( + text.contains("#[cfg(feature = \"secrets\")]\npub mod secrets;\n"), + "{}: `pub mod secrets;` must follow `#[cfg(feature = \"secrets\")]`", + path.display() + ); + assert_eq!(text.matches("mod secrets").count(), 1, "{}", path.display()); +}