From 08b55ee36d2b4de7af31eeb719b5cc3a82c35dba Mon Sep 17 00:00:00 2001 From: Sylvain Tremblay Date: Thu, 28 May 2026 13:45:12 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(harmony=5Fconfig):=20unified=20config?= =?UTF-8?q?=20layer=20=E2=80=94=20ConfigClient,=20ConfigClass,=20masking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Net-diff PR (2 of 4) splitting feat/unified-config-and-secrets. harmony_config + harmony_config_derive; compiles against master's harmony_secret. - ConfigClass + #[config(secret)] derive; class plumbed through ConfigSource - ConfigManager -> ConfigClient rename + for_namespace + Builder - per-class secret masking: input echoes '*', output renders '****' - get_or_prompt persists to every writable source - SQLite dropped from the canonical chain (cleartext-at-rest) + namespaced - prompt banner; serde-rename caveat docs; store-error logging - docs: ADR-020-1 names OPENBAO_URL/VAULT_ADDR; ROADMAP/01 + firewall_pair rename Co-Authored-By: Claude Opus 4.7 --- ROADMAP/01-config-crate.md | 12 +- ...0-1-zitadel-openbao-secure-config-store.md | 4 +- harmony/src/domain/topology/firewall_pair.rs | 2 +- harmony_config/examples/basic.rs | 8 +- harmony_config/examples/openbao_chain.rs | 565 ++++++--- harmony_config/examples/prompting.rs | 8 +- harmony_config/src/lib.rs | 1067 +++++++++++++++-- harmony_config/src/source/env.rs | 15 +- harmony_config/src/source/local_file.rs | 24 +- harmony_config/src/source/prompt.rs | 222 +++- harmony_config/src/source/sqlite.rs | 58 +- harmony_config/src/source/store.rs | 33 +- harmony_config_derive/src/lib.rs | 84 +- 13 files changed, 1846 insertions(+), 256 deletions(-) diff --git a/ROADMAP/01-config-crate.md b/ROADMAP/01-config-crate.md index 9829cd16..fbbcf945 100644 --- a/ROADMAP/01-config-crate.md +++ b/ROADMAP/01-config-crate.md @@ -9,7 +9,7 @@ Make `harmony_config` production-ready with a seamless first-run experience: clo `harmony_config` now has: - `Config` trait + `#[derive(Config)]` macro -- `ConfigManager` with ordered source chain +- `ConfigClient` with ordered source chain - Five `ConfigSource` implementations: - `EnvSource` — reads `HARMONY_CONFIG_{KEY}` env vars - `LocalFileSource` — reads/writes `{key}.json` files from a directory @@ -140,7 +140,7 @@ for source in &self.sources { ┌─────────────────────────────────────────────────────────────────────┐ │ Harmony CLI / App │ │ │ -│ ConfigManager: │ +│ ConfigClient: │ │ 1. EnvSource ← HARMONY_CONFIG_* env vars (highest priority) │ │ 2. SqliteSource ← ~/.local/share/harmony/config/config.db │ │ 3. StoreSource ← OpenBao (team-scale, via Zitadel OIDC) │ @@ -448,11 +448,11 @@ The example uses `StoreSource` with token auth to avoid the | `OPENBAO_KV_MOUNT` | No | `"secret"` | KV v2 engine mount path. **Also used as userpass auth mount -- this is a bug.** | | `OPENBAO_SKIP_TLS` | No | `false` | Set `"true"` to disable TLS verification | -**Note**: `OpenbaoSecretStore::new()` is `async` and **requires a running OpenBao** at construction time (it validates the token if using cached auth). If OpenBao is unreachable during construction, the call will fail. The graceful fallback only applies to `StoreSource::get()` calls after construction -- the `ConfigManager` must be built with a live store, or the store must be wrapped in a lazy initialization pattern. +**Note**: `OpenbaoSecretStore::new()` is `async` and **requires a running OpenBao** at construction time (it validates the token if using cached auth). If OpenBao is unreachable during construction, the call will fail. The graceful fallback only applies to `StoreSource::get()` calls after construction -- the `ConfigClient` must be built with a live store, or the store must be wrapped in a lazy initialization pattern. ```rust // harmony_config/examples/openbao_chain.rs -use harmony_config::{ConfigManager, EnvSource, SqliteSource, StoreSource}; +use harmony_config::{ConfigClient, EnvSource, SqliteSource, StoreSource}; use harmony_secret::OpenbaoSecretStore; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -517,7 +517,7 @@ async fn main() -> anyhow::Result<()> { vec![env_source, sqlite] }; - let manager = ConfigManager::new(sources); + let manager = ConfigClient::new(sources); // Scenario 1: get() with nothing stored -- returns NotFound let result = manager.get::().await; @@ -620,4 +620,4 @@ match self.store.get_raw(&self.namespace, key).await { 6. **Graceful fallback**: `StoreSource::get()` returns `Ok(None)` on any error (connection refused, timeout, etc.), allowing the chain to fall through to the next source. This ensures OpenBao unavailability doesn't break the config chain. -7. **StoreSource errors don't block chain**: When OpenBao is unreachable, `StoreSource::get()` returns `Ok(None)` and the `ConfigManager` continues to the next source (typically `SqliteSource`). This is validated by `test_store_source_error_falls_through_to_sqlite` and `test_store_source_not_found_falls_through_to_sqlite`. +7. **StoreSource errors don't block chain**: When OpenBao is unreachable, `StoreSource::get()` returns `Ok(None)` and the `ConfigClient` continues to the next source (typically `SqliteSource`). This is validated by `test_store_source_error_falls_through_to_sqlite` and `test_store_source_not_found_falls_through_to_sqlite`. diff --git a/docs/adr/020-1-zitadel-openbao-secure-config-store.md b/docs/adr/020-1-zitadel-openbao-secure-config-store.md index cac11576..51ef401c 100644 --- a/docs/adr/020-1-zitadel-openbao-secure-config-store.md +++ b/docs/adr/020-1-zitadel-openbao-secure-config-store.md @@ -211,11 +211,13 @@ The `bound_audiences` claim ties the role to the specific Harmony Zitadel applic For organizations running their own infrastructure, the same architecture applies. The operator deploys Zitadel and OpenBao using Harmony's existing `ZitadelScore` and `OpenbaoScore`. The only configuration needed is three environment variables (or their equivalents in the bootstrap config): - `HARMONY_SSO_URL` — the Zitadel instance URL. -- `HARMONY_SECRETS_URL` — the OpenBao instance URL. +- `OPENBAO_URL` (or `VAULT_ADDR`) — the OpenBao instance URL. - `HARMONY_SSO_CLIENT_ID` — the Zitadel application client ID. None of these are secrets. They can be committed to an infrastructure repository or distributed via any convenient channel. +`OPENBAO_URL`/`VAULT_ADDR` is named after the backend on purpose: it is read only by the OpenBao adapter, never by domain or Score code, and reusing the standard Vault/OpenBao variable names gives operators interop with existing tooling. The tool-agnostic seam lives one layer up — `HARMONY_SECRET_STORE` selects which backend is active, and each backend then reads its own connection params (e.g. `HARMONY_SECRET_INFISICAL_*`). An earlier draft of this ADR proposed a single agnostic `HARMONY_SECRETS_URL`, but it was never implemented: different stores don't share a connection shape (Infisical needs a project and client credentials; AWS Secrets Manager has no URL at all), so one "secrets URL" would force OpenBao's model onto every backend. + ## Consequences ### Positive diff --git a/harmony/src/domain/topology/firewall_pair.rs b/harmony/src/domain/topology/firewall_pair.rs index 09cecc5a..76bbb53f 100644 --- a/harmony/src/domain/topology/firewall_pair.rs +++ b/harmony/src/domain/topology/firewall_pair.rs @@ -62,7 +62,7 @@ impl FirewallPairTopology { pub async fn opnsense_from_config() -> Self { // TODO: both firewalls share the same credentials. Once named config // instances are available (ROADMAP/11), use per-device credentials: - // ConfigManager::get_named::("fw-primary") + // ConfigClient::get_named::("fw-primary") let ssh_creds = SecretManager::get_or_prompt::() .await .expect("Failed to get SSH credentials"); diff --git a/harmony_config/examples/basic.rs b/harmony_config/examples/basic.rs index 2b192492..f4560612 100644 --- a/harmony_config/examples/basic.rs +++ b/harmony_config/examples/basic.rs @@ -12,7 +12,7 @@ use std::sync::Arc; -use harmony_config::{Config, ConfigManager, EnvSource, SqliteSource}; +use harmony_config::{Config, ConfigClient, EnvSource, SqliteSource}; use log::info; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -36,8 +36,10 @@ impl Default for TestConfig { async fn main() -> anyhow::Result<()> { env_logger::init(); - let sqlite = SqliteSource::default().await?; - let manager = ConfigManager::new(vec![Arc::new(EnvSource), Arc::new(sqlite)]); + // Namespace the SQLite file so this example's state doesn't + // collide with other harmony binaries that also use SqliteSource. + let sqlite = SqliteSource::for_namespace("harmony_config-basic-example").await?; + let manager = ConfigClient::new(vec![Arc::new(EnvSource), Arc::new(sqlite)]); info!("1. Attempting to get TestConfig (expect NotFound on first run)..."); match manager.get::().await { diff --git a/harmony_config/examples/openbao_chain.rs b/harmony_config/examples/openbao_chain.rs index 84fd8870..88f0b11c 100644 --- a/harmony_config/examples/openbao_chain.rs +++ b/harmony_config/examples/openbao_chain.rs @@ -1,60 +1,131 @@ -//! End-to-end example: harmony_config with OpenBao as a ConfigSource +//! # Dev-binary template: harmony_config against a harmony-sso-example stack //! -//! This example demonstrates the full config resolution chain: -//! EnvSource → SqliteSource → StoreSource +//! This example is the consumer-side counterpart to +//! `examples/harmony_sso`. Where `harmony_sso` puts the SSO + OpenBao +//! infrastructure in place, this example shows how a developer writes +//! a new harmony binary that *uses* that infrastructure to manage its +//! configuration and secrets. //! -//! When OpenBao is unreachable, the chain gracefully falls through to SQLite. +//! ## Prerequisite — the harmony-sso-example stack must be running //! -//! **Prerequisites**: -//! - OpenBao must be initialized and unsealed -//! - KV v2 engine must be enabled at the `OPENBAO_KV_MOUNT` path (default: `secret`) -//! - Auth method must be enabled at the `OPENBAO_AUTH_MOUNT` path (default: `userpass`) +//! Before running this example, run the harmony-sso-example demo at +//! least once so that: //! -//! **Environment variables**: -//! - `OPENBAO_URL` (required for OpenBao): URL of the OpenBao server -//! - `OPENBAO_TOKEN` (optional): Use token auth instead of userpass -//! - `OPENBAO_USERNAME` + `OPENBAO_PASSWORD` (optional): Userpass auth -//! - `OPENBAO_KV_MOUNT` (default: `secret`): KV v2 engine mount path -//! - `OPENBAO_AUTH_MOUNT` (default: `userpass`): Auth method mount path -//! - `OPENBAO_SKIP_TLS` (default: `false`): Skip TLS verification -//! - `HARMONY_SSO_URL` + `HARMONY_SSO_CLIENT_ID` (optional): Zitadel OIDC device flow (RFC 8628) +//! - the local k3d cluster `harmony-example` is up, +//! - OpenBao is deployed, initialised, unsealed, and reachable through +//! the ingress at `bao.harmony.local:8080`, +//! - Zitadel is deployed with a device-code OAuth application named +//! `harmony-cli`, reachable through the ingress at +//! `sso.harmony.local:8080`, +//! - `/etc/hosts` resolves both hostnames to `127.0.0.1`. //! -//! **Run**: +//! Run the prereq stack: //! ```bash -//! # Without OpenBao (SqliteSource only): -//! cargo run --example openbao_chain -//! -//! # With OpenBao (full chain): -//! export OPENBAO_URL="http://127.0.0.1:8200" -//! export OPENBAO_TOKEN="" -//! cargo run --example openbao_chain +//! cargo run -p example-harmony-sso //! ``` //! -//! **Setup OpenBao** (if needed): +//! Then run this example: //! ```bash -//! # Port-forward to local OpenBao -//! kubectl port-forward svc/openbao -n openbao 8200:8200 & -//! -//! # Initialize (one-time) -//! kubectl exec -n openbao openbao-0 -- bao operator init -//! -//! # Enable KV and userpass (one-time) -//! kubectl exec -n openbao openbao-0 -- bao secrets enable -path=secret kv-v2 -//! kubectl exec -n openbao openbao-0 -- bao auth enable userpass -//! -//! # Create test user -//! kubectl exec -n openbao openbao-0 -- bao write auth/userpass/users/testuser \ -//! password="testpass" policies="default" +//! cargo run -p harmony_config --example openbao_chain //! ``` +//! +//! ## Use cases demonstrated +//! +//! 1. Build a `ConfigClient` with the chain a real harmony binary +//! would use: `EnvSource → StoreSource → PromptSource`. +//! OpenBao is the team-scale source of truth. There is no local +//! cache layer by default — SQLite is opt-in only +//! (`builder(..).with_sqlite()`) because it stores values as +//! cleartext on disk and can't safely hold Secret-class config. +//! 2. Authenticate to OpenBao via the Zitadel device flow on first run, +//! then cache the session so subsequent runs are silent. +//! 3. `manager.get::()` — read existing config. +//! 4. `manager.set(&value)` — persist to every writable source. +//! 5. `manager.get_or_prompt::()` — interactively gather config the +//! first time, then cache it. +//! 6. **Edit existing config** — when `get` returns a cached value the +//! example shows it and asks the operator whether to update it. +//! If they say yes, `PromptSource::prompt_for` collects new values +//! and `manager.set` persists them. This is the canonical +//! "re-run to reconfigure" loop for a long-lived binary. +//! 7. Round-trip both a `ConfigClass::Standard` struct (`AppConfig`) and +//! a `ConfigClass::Secret` struct (`DatabaseCredentials`) through the +//! same chain — no per-call ceremony. +//! 8. **Masked input on Secret-class structs** (`ApiCredentials`). +//! `#[config(secret)]` fields are read via `inquire::Password` +//! (echoed as `*`) when the chain falls through to +//! `PromptSource`. The Standard `client_id` field uses normal +//! text input. The same step shows the +//! "cached → confirm → re-prompt" update loop applied to a +//! Secret struct. +//! 9. **Safe logging via `.masked()`.** Every log line that prints a +//! `Config` value goes through `value.masked()`, which masks +//! `#[config(secret)]` fields as `"****"`. Standard-class +//! values render unmodified, so the pattern is safe-by-default +//! at every call site. +//! 10. Demonstrate the env-var override: `HARMONY_CONFIG_AppConfig` +//! short-circuits the chain. +//! 11. Auth-method override: `OPENBAO_TOKEN` or +//! `OPENBAO_USERNAME`+`OPENBAO_PASSWORD` bypass SSO for testing +//! against a non-SSO OpenBao. +//! +//! ## Configuration — built in code, passed to the builder +//! +//! Connection params are computed in plain Rust and handed to +//! `ConfigClient::builder("...").openbao_overrides(...)`, so the +//! example never mutates the process environment to configure +//! itself. (Rust 2024 makes `std::env::set_var` `unsafe`; we'd +//! rather not pay that cost for plumbing that can live as data.) +//! Any env var that's already set takes precedence, so you can +//! still point the example at a different OpenBao by exporting +//! variables from your shell: +//! +//! | Variable | In-code default | Source of truth | +//! |---|---|---| +//! | `OPENBAO_URL` | `http://bao.harmony.local:8080` | this file | +//! | `HARMONY_SSO_URL` | `http://sso.harmony.local:8080` | this file | +//! | `HARMONY_SSO_CLIENT_ID` | auto-discovered from harmony's cache | **see disclaimer below** | +//! | `OPENBAO_KV_MOUNT` | builder default `secret` | env | +//! | `OPENBAO_AUTH_MOUNT` | builder default `jwt` | env, matches `OpenbaoSetupScore` | +//! | `OPENBAO_TOKEN` | (unset) | optional override | +//! | `OPENBAO_USERNAME`/`OPENBAO_PASSWORD` | (unset) | optional override | +//! +//! ## ⚠ `HARMONY_SSO_CLIENT_ID` auto-discovery — example scaffolding only +//! +//! In production, `HARMONY_SSO_CLIENT_ID` is configured by whatever +//! provisions the binary's environment — a systemd unit, a Kubernetes +//! ConfigMap, a `direnv`/`.envrc` file, your CI job, etc. The value is +//! set externally; the binary just reads it. +//! +//! This example, however, runs locally against a stack that is brought +//! up via `examples/harmony_sso`, which generates the client_id +//! dynamically (it varies per Zitadel install). To make the example +//! "just work" without forcing the operator to look the value up +//! manually, we **read it out of harmony's own cache file at +//! `~/.local/share/harmony/zitadel/client-config.json`** and export it +//! back to the environment. +//! +//! **Do not copy the auto-discovery helper into a real harmony binary.** +//! It reads from a path that is an implementation detail of +//! `ZitadelSetupScore`, not a public contract. Production binaries +//! must take `HARMONY_SSO_CLIENT_ID` from the environment. -use std::sync::Arc; +use std::path::PathBuf; -use harmony_config::{Config, ConfigManager, ConfigSource, EnvSource, SqliteSource, StoreSource}; -use harmony_secret::OpenbaoSecretStore; +use harmony_config::{Config, ConfigClient, ConfigExt, OpenbaoConnection, PromptSource}; +use inquire::Confirm; +use log::info; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] +// --------------------------------------------------------------------------- +// Config types +// --------------------------------------------------------------------------- + +/// Plain operational config. Standard class — no `#[config(secret)]` +/// fields, so backends are free to log / display / cache it however +/// they like. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] struct AppConfig { host: String, port: u16, @@ -63,130 +134,354 @@ struct AppConfig { impl Default for AppConfig { fn default() -> Self { Self { - host: "localhost".to_string(), - port: 8080, + host: "production.example.com".to_string(), + port: 443, } } } -impl Config for AppConfig { - const KEY: &'static str = "AppConfig"; +/// Mixed-sensitivity config. The `password` field is tagged +/// `#[config(secret)]`, which elevates the whole struct to +/// `ConfigClass::Secret`. The struct still round-trips through the +/// same `ConfigClient` chain — the class is a backend hint, not a +/// dispatch. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] +struct DatabaseCredentials { + host: String, + username: String, + #[config(secret)] + password: String, } -async fn build_manager() -> ConfigManager { - let sqlite = Arc::new( - SqliteSource::default() - .await - .expect("Failed to open SQLite database"), - ); +impl Default for DatabaseCredentials { + fn default() -> Self { + Self { + host: "db.example.com".to_string(), + username: "app_rw".to_string(), + password: "rotate-me-please".to_string(), + } + } +} - let env_source: Arc = Arc::new(EnvSource); +/// A struct intentionally lacking a `Default` impl so we can show the +/// `get_or_prompt` flow: on first run there is no cached value and no +/// in-code default, so the manager prompts interactively, stores the +/// answers, and subsequent runs find the value without prompting. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] +struct UserPreferences { + display_name: String, + theme: String, +} - let openbao_url = std::env::var("OPENBAO_URL") - .or_else(|_| std::env::var("VAULT_ADDR")) - .ok(); +/// Secret-class struct without a `Default` impl. Exists specifically +/// to exercise the masked-prompt path: on first run the chain falls +/// through to `PromptSource`, the walker sees +/// `Config::CLASS == Secret`, and routes each tagged field through +/// `inquire::Password` (echoed as `*`) instead of `inquire::Text`. +/// `client_id` stays a plain text prompt because it isn't tagged. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] +struct ApiCredentials { + client_id: String, + #[config(secret)] + client_secret: String, + #[config(secret)] + refresh_token: String, +} - match openbao_url { - Some(url) => { - let kv_mount = - std::env::var("OPENBAO_KV_MOUNT").unwrap_or_else(|_| "secret".to_string()); - let auth_mount = - std::env::var("OPENBAO_AUTH_MOUNT").unwrap_or_else(|_| "userpass".to_string()); - let skip_tls = std::env::var("OPENBAO_SKIP_TLS") - .map(|v| v == "true") - .unwrap_or(false); +// --------------------------------------------------------------------------- +// Connection params — built in code, passed to ConfigClient::builder +// --------------------------------------------------------------------------- - match OpenbaoSecretStore::new( - url, - kv_mount, - auth_mount, - skip_tls, - std::env::var("OPENBAO_TOKEN").ok(), - std::env::var("OPENBAO_USERNAME").ok(), - std::env::var("OPENBAO_PASSWORD").ok(), - std::env::var("HARMONY_SSO_URL").ok(), - std::env::var("HARMONY_SSO_CLIENT_ID").ok(), - None, - None, - ) - .await - { - Ok(store) => { - let store_source: Arc = - Arc::new(StoreSource::new("harmony".to_string(), store)); - println!("OpenBao connected. Full chain: env → sqlite → openbao"); - ConfigManager::new(vec![env_source, Arc::clone(&sqlite) as _, store_source]) - } - Err(e) => { - eprintln!( - "Warning: OpenBao unavailable ({e}), using local chain: env → sqlite" - ); - ConfigManager::new(vec![env_source, sqlite]) - } +/// Path harmony writes to when `ZitadelSetupScore` provisions the +/// `harmony-cli` device-code application. **Implementation detail of +/// the harmony-sso-example stack** — not a public contract. Used here +/// only to spare the operator from manually exporting +/// `HARMONY_SSO_CLIENT_ID` after every `cargo run -p example-harmony-sso`. +fn zitadel_client_config_path() -> Option { + Some( + directories::BaseDirs::new()? + .data_dir() + .join("harmony") + .join("zitadel") + .join("client-config.json"), + ) +} + +/// Auto-discovery for the `harmony-cli` OIDC client_id, from harmony's +/// own cache file. +/// +/// ⚠ Example scaffolding only. A real harmony binary must read +/// `HARMONY_SSO_CLIENT_ID` from the environment — it is configured by +/// whatever provisions the binary (systemd, Kubernetes, direnv, CI). +/// Do not copy this helper into production code. +fn discover_zitadel_client_id() -> Option { + let path = zitadel_client_config_path()?; + let bytes = std::fs::read(&path).ok()?; + let v: serde_json::Value = serde_json::from_slice(&bytes).ok()?; + v.get("apps")? + .get("harmony-cli")? + .as_str() + .map(str::to_string) +} + +/// Build the OpenBao connection params that target the +/// harmony-sso-example stack. We compute the values in plain Rust +/// and pass them through `ConfigClientBuilder::openbao_overrides`, +/// so the example doesn't have to mutate the process environment to +/// configure itself. +/// +/// Each field honours an existing env var if set (so an operator +/// running this against a different OpenBao can override from their +/// shell); otherwise we fall back to the harmony-sso defaults. +fn harmony_sso_example_connection() -> anyhow::Result { + let sso_client_id = std::env::var("HARMONY_SSO_CLIENT_ID") + .ok() + .or_else(|| { + let discovered = discover_zitadel_client_id(); + if let Some(ref id) = discovered { + info!( + "openbao_chain: auto-discovered HARMONY_SSO_CLIENT_ID={id} \ + from harmony's local cache (EXAMPLE-ONLY; production binaries \ + must read this from the environment)" + ); } - } - None => { - println!("OPENBAO_URL not set. Using local chain: env → sqlite"); - ConfigManager::new(vec![env_source, sqlite]) - } - } + discovered + }) + .ok_or_else(|| { + anyhow::anyhow!( + "HARMONY_SSO_CLIENT_ID is not set and no client-config cache was \ + found at {:?}. Either:\n (a) bring up the harmony-sso-example \ + stack first (`cargo run -p example-harmony-sso`), which writes \ + that file, or\n (b) export HARMONY_SSO_CLIENT_ID yourself \ + pointing at your Zitadel app.", + zitadel_client_config_path() + ) + })?; + + Ok(OpenbaoConnection { + url: Some( + std::env::var("OPENBAO_URL") + .unwrap_or_else(|_| "http://bao.harmony.local:8080".to_string()), + ), + sso_url: Some( + std::env::var("HARMONY_SSO_URL") + .unwrap_or_else(|_| "http://sso.harmony.local:8080".to_string()), + ), + sso_client_id: Some(sso_client_id), + // `kv_mount` (defaults to "secret") and `auth_mount` + // (defaults to "jwt") match what OpenbaoSetupScore + // configures in harmony-sso-example, so we leave them at + // None to use the builder's defaults. Token / userpass + // overrides are intentionally absent — SSO is the happy + // path; export OPENBAO_TOKEN from your shell to force the + // token-auth fallback. + ..Default::default() + }) } -#[tokio::main] -async fn main() -> anyhow::Result<()> { - env_logger::init(); +// (Chain construction is `ConfigClient::builder(...)` — see main().) - let manager = build_manager().await; +// --------------------------------------------------------------------------- +// Use-case walkthroughs +// --------------------------------------------------------------------------- - println!("\n=== harmony_config OpenBao Chain Demo ===\n"); - - println!("1. Attempting to get AppConfig (expect NotFound on first run)..."); - match manager.get::().await { - Ok(config) => { - println!(" Found: {:?}", config); +/// First time around there's nothing stored, so we set a default; +/// subsequent runs read the stored value back. Logs the struct's +/// `CLASS` so the operator can see Standard / Secret routing. +/// +/// Logs route through `.masked()` so any `#[config(secret)]` field +/// renders as `****` in the output. For Standard-class types this +/// is a no-op (`SECRET_FIELDS` is empty), so it's safe-by-default +/// to always go through `.masked()` for log lines that print a +/// `Config` value. +async fn demo_round_trip(manager: &ConfigClient, default_value: T) -> anyhow::Result<()> +where + T: Config + std::fmt::Debug + Clone + PartialEq, +{ + info!( + "[{key}] CLASS={class:?}, attempting read…", + key = T::KEY, + class = T::CLASS, + ); + match manager.get::().await { + Ok(found) => { + info!("[{}] read existing value: {:?}", T::KEY, found.masked()); } Err(harmony_config::ConfigError::NotFound { .. }) => { - println!(" NotFound - no config stored yet"); - } - Err(e) => { - println!(" Error: {:?}", e); + info!("[{}] not found — storing default and reading back", T::KEY); + manager.set(&default_value).await?; + let retrieved: T = manager.get().await?; + anyhow::ensure!( + retrieved == default_value, + "Round-trip mismatch for {}: stored != retrieved", + T::KEY + ); + info!("[{}] round-trip verified: {:?}", T::KEY, retrieved.masked()); } + Err(e) => return Err(e.into()), } + Ok(()) +} - println!("\n2. Setting AppConfig via set()..."); - let config = AppConfig { - host: "production.example.com".to_string(), - port: 443, - }; - manager.set(&config).await?; - println!(" Set: {:?}", config); - - println!("\n3. Getting AppConfig back..."); - let retrieved: AppConfig = manager.get().await?; - println!(" Retrieved: {:?}", retrieved); - assert_eq!(config, retrieved); - - println!("\n4. Demonstrating env override..."); - println!(" HARMONY_CONFIG_AppConfig env var overrides all backends"); - let env_config = AppConfig { - host: "env-override.example.com".to_string(), +/// Shows that `HARMONY_CONFIG_` always wins, regardless of what's +/// in OpenBao. Useful for per-process tweaks (a CI job, a +/// one-off dry-run) without mutating shared state. +/// +/// This is the **only** `unsafe` block in the example — and it's not +/// a workaround. The env-var override mechanism is the feature +/// being demonstrated; calling `set_var` here is the demo, not +/// configuration plumbing. (For chain configuration we pass +/// `OpenbaoConnection` through the builder, which avoids +/// `env::set_var` entirely.) +async fn demo_env_override(manager: &ConfigClient) -> anyhow::Result<()> { + info!("[env-override] setting HARMONY_CONFIG_AppConfig in-process"); + let override_value = AppConfig { + host: "ci-override.example.com".to_string(), port: 9090, }; + // SAFETY: `std::env::set_var` is unsafe under Rust 2024 because + // POSIX `setenv` races against concurrent libc reads on other + // threads. We're inside the tokio current-thread runtime and no + // other harmony code is reading `HARMONY_CONFIG_AppConfig` + // outside this manager's `EnvSource::get`, which we await + // immediately after the write. Single-threaded + bounded + // window → no race. unsafe { std::env::set_var( "HARMONY_CONFIG_AppConfig", - serde_json::to_string(&env_config)?, + serde_json::to_string(&override_value)?, ); } - let from_env: AppConfig = manager.get().await?; - println!(" Got from env: {:?}", from_env); - assert_eq!(env_config.host, "env-override.example.com"); + let seen: AppConfig = manager.get().await?; + anyhow::ensure!( + seen == override_value, + "env override did not win: got {seen:?}, expected {override_value:?}" + ); + info!("[env-override] env source short-circuited the chain: {seen:?}"); + // SAFETY: same conditions as the set_var above. unsafe { std::env::remove_var("HARMONY_CONFIG_AppConfig"); } + Ok(()) +} - println!("\n=== Done! ==="); - println!("Config persisted at ~/.local/share/harmony/config/config.db"); +/// Two flows in one step, depending on whether the value is already +/// cached: +/// +/// - **Cold run (cache miss).** No cached value and no `Default` impl +/// on `T`, so `get_or_prompt` prompts interactively (via `inquire`, +/// requires a TTY) and persists the answers. +/// +/// - **Subsequent run (cache hit).** The value is found in OpenBao +/// without prompting. We then *show* the cached value and +/// ask via `inquire::Confirm` whether the operator wants to update +/// it. On "yes", `PromptSource::prompt_for` re-collects every +/// field and `manager.set` persists the new value to every +/// writable source. On "no", the cached value stands. +/// +/// When `T::CLASS == Secret`, the prompt walker uses +/// `inquire::Password` for `#[config(secret)]` fields — input is +/// echoed as `*` instead of the real characters. All log lines +/// route through `.masked()` so cached / updated values render +/// with their secret fields replaced by `"****"`. +/// +/// Both branches require a TTY; on a CI runner with no terminal the +/// step fails — that's the same contract every interactive harmony +/// binary has. +async fn demo_get_or_prompt(manager: &ConfigClient) -> anyhow::Result<()> +where + T: Config + std::fmt::Debug + Clone + PartialEq, +{ + info!("[{key}] CLASS={class:?}", key = T::KEY, class = T::CLASS,); + + match manager.get::().await { + Ok(existing) => { + info!("[{}] cached value found: {:?}", T::KEY, existing.masked()); + let want_change = Confirm::new(&format!("Update `{}` with new values?", T::KEY)) + .with_default(false) + .prompt() + .map_err(|e| anyhow::anyhow!("Confirm prompt failed: {e}"))?; + if want_change { + // PromptSource::prompt_for holds the process-wide + // prompt mutex, so any background log noise gets + // serialised against the prompt. For Secret-class + // T it also routes #[config(secret)] string fields + // through inquire::Password (input echoed as `*`). + let updated: T = PromptSource::new().prompt_for().await?; + manager.set(&updated).await?; + info!( + "[{}] updated and persisted to every writable source: {:?}", + T::KEY, + updated.masked(), + ); + } else { + info!("[{}] keeping cached value (no update requested)", T::KEY); + } + } + Err(harmony_config::ConfigError::NotFound { .. }) => { + info!( + "[{}] no cached value — get_or_prompt will gather + persist", + T::KEY + ); + let resolved: T = manager.get_or_prompt().await?; + info!("[{}] resolved: {:?}", T::KEY, resolved.masked()); + } + Err(e) => return Err(e.into()), + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Entry point +// --------------------------------------------------------------------------- + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info")) + .format_timestamp_secs() + .init(); + + // Compute connection params in plain Rust (no env mutation — + // see module-level note about avoiding `unsafe`) and pass them + // to the builder. A production binary leaves this off and + // relies on env vars set by its deployment. + let openbao = harmony_sso_example_connection()?; + let manager = ConfigClient::builder("harmony") + .openbao_overrides(openbao) + .build() + .await?; + + println!("\n=== harmony_config dev-binary demo ===\n"); + + info!("Step 1/5 — round-trip Standard-class struct (AppConfig)"); + demo_round_trip(&manager, AppConfig::default()).await?; + + info!( + "Step 2/5 — round-trip Secret-class struct (DatabaseCredentials); logs route through .masked()" + ); + demo_round_trip(&manager, DatabaseCredentials::default()).await?; + + info!("Step 3/5 — env-var override demo"); + demo_env_override(&manager).await?; + + info!("Step 4/5 — get_or_prompt on a Standard struct (UserPreferences); plain text prompts"); + demo_get_or_prompt::(&manager).await?; + + info!( + "Step 5/5 — get_or_prompt on a Secret struct (ApiCredentials); \ + #[config(secret)] fields are read with inquire::Password (`*` masked input)" + ); + demo_get_or_prompt::(&manager).await?; + + println!("\n=== Done. ==="); + println!("Stored values live in:"); + println!( + " • OpenBao @ secret/harmony/ (browse via http://bao.harmony.local:8080/ui)" + ); + println!(" • OIDC cache @ ~/.local/share/harmony/secrets/oidc_session_*"); + println!( + " (no local SQLite cache — not in the default chain; opt in with builder().with_sqlite())" + ); Ok(()) } diff --git a/harmony_config/examples/prompting.rs b/harmony_config/examples/prompting.rs index e7342584..1018e211 100644 --- a/harmony_config/examples/prompting.rs +++ b/harmony_config/examples/prompting.rs @@ -12,7 +12,7 @@ use std::sync::Arc; -use harmony_config::{Config, ConfigManager, EnvSource, PromptSource, SqliteSource}; +use harmony_config::{Config, ConfigClient, EnvSource, PromptSource, SqliteSource}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -27,8 +27,10 @@ struct UserConfig { async fn main() -> anyhow::Result<()> { env_logger::init(); - let sqlite = SqliteSource::default().await?; - let manager = ConfigManager::new(vec![ + // Namespace the SQLite file so this example's state doesn't + // collide with other harmony binaries that also use SqliteSource. + let sqlite = SqliteSource::for_namespace("harmony_config-prompting-example").await?; + let manager = ConfigClient::new(vec![ Arc::new(EnvSource), Arc::new(sqlite), Arc::new(PromptSource::new()), diff --git a/harmony_config/src/lib.rs b/harmony_config/src/lib.rs index 64086c39..710d1c0a 100644 --- a/harmony_config/src/lib.rs +++ b/harmony_config/src/lib.rs @@ -1,9 +1,16 @@ +// Lets the derive macro emit `::harmony_config::ConfigClass` (etc.) and have +// it resolve both inside this crate (lib + tests) and outside (examples, +// downstream consumers). Without this alias, the macro would need separate +// emission paths for "called from within harmony_config" vs everywhere else, +// and `proc-macro-crate` cannot distinguish examples from the lib. +extern crate self as harmony_config; + mod source; use async_trait::async_trait; use directories::ProjectDirs; use interactive_parse::InteractiveParseObj; -use log::debug; +use log::{debug, warn}; use schemars::JsonSchema; use serde::{Serialize, de::DeserializeOwned}; use std::path::PathBuf; @@ -57,40 +64,243 @@ pub enum ConfigError { SqliteError(String), } +/// Tells the storage backend how to treat a `Config` value. +/// +/// The class is a hint passed through the `ConfigSource` chain. Sources are +/// free to ignore it; backends that care (OpenBao routing, future audit +/// logging, terminal masking in `PromptSource`) inspect it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ConfigClass { + /// Plaintext storage is acceptable. The default for any `Config` + /// struct that does not contain `#[config(secret)]` fields. + Standard, + /// Must be treated as a secret: encrypted at rest, masked in UI, + /// audited where the backend supports it. Elevated automatically + /// when any field carries `#[config(secret)]` or the struct itself + /// is annotated. + Secret, +} + pub trait Config: Serialize + DeserializeOwned + JsonSchema + InteractiveParseObj + Sized { const KEY: &'static str; + const CLASS: ConfigClass = ConfigClass::Standard; + /// Names of fields carrying `#[config(secret)]`. The derive macro + /// populates this; manual `impl Config for` and Standard-class + /// structs see the trait default of `&[]`. Used by + /// `PromptSource` to switch a field to `inquire::Password` and by + /// the `Masked` wrapper to replace those field values with + /// `"****"` in formatted output. + /// + /// **Constraint — do not combine `#[config(secret)]` with + /// `#[serde(rename = "…")]` or `#[serde(rename_all = "…")]` on a + /// secret field.** The names recorded here are the Rust field + /// *idents*, but both `Masked` and the secret prompt match against + /// the *serialized* name (serde_json keys / schemars properties). A + /// renamed secret field would therefore fail to match and its value + /// would render and prompt in **cleartext**. Until the derive learns + /// to read serde rename attributes, keep secret field names + /// identical in Rust and serialized form. + const SECRET_FIELDS: &'static [&'static str] = &[]; } +/// Safe-display wrapper produced by [`ConfigExt::masked`]. +/// +/// `{:?}` on a `Masked` renders the value via serde-JSON, then +/// replaces every field listed in `T::SECRET_FIELDS` with the +/// literal string `"****"`. Standard-class values render as +/// unmodified JSON. Use this anywhere a Config value might end up +/// in a log line or an error message: +/// +/// ```ignore +/// info!("loaded: {:?}", cfg.masked()); +/// ``` +pub struct Masked<'a, T: Config>(&'a T); + +impl std::fmt::Debug for Masked<'_, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut value = serde_json::to_value(self.0).map_err(|_| std::fmt::Error)?; + if T::CLASS == ConfigClass::Secret { + mask_value(&mut value, T::SECRET_FIELDS); + } + // Pretty-print so masked values stay readable in multiline + // log output. `{:#}` on serde_json::Value is the indented + // form. + write!(f, "{value:#}") + } +} + +/// Recursively replace every `obj[name]` entry whose `name` is in +/// `secret_fields` with `"****"`. Non-object payloads (e.g. a Config +/// implemented on a tuple) are left as-is — secret fields are a +/// per-named-field concept. +fn mask_value(value: &mut serde_json::Value, secret_fields: &[&str]) { + if let serde_json::Value::Object(map) = value { + for (k, v) in map.iter_mut() { + if secret_fields.contains(&k.as_str()) { + *v = serde_json::Value::String("****".to_string()); + } else { + // Walk nested objects too so a Standard struct + // nested inside a Secret one still gets its + // matching secret-field-named leaves redacted if + // the user has them in SECRET_FIELDS (rare today, + // but cheap to support). + mask_value(v, secret_fields); + } + } + } else if let serde_json::Value::Array(items) = value { + for item in items.iter_mut() { + mask_value(item, secret_fields); + } + } +} + +/// Extension trait that adds [`masked`](Self::masked) to every +/// `Config` type. Provided via blanket impl; no boilerplate per +/// struct. +pub trait ConfigExt: Config { + /// Wrap `self` in a safe-display [`Masked`] view. Secret-tagged + /// fields render as `"****"`; other fields render as their + /// JSON form. Use in log lines / error messages instead of + /// passing the raw value through `{:?}`. + fn masked(&self) -> Masked<'_, Self> { + Masked(self) + } +} + +impl ConfigExt for T {} + #[async_trait] pub trait ConfigSource: Send + Sync { - async fn get(&self, key: &str) -> Result, ConfigError>; + /// Fetch a JSON value for `key`. The `class` is a hint from the + /// caller's `Config` impl; sources may ignore it (most do today) + /// or use it to route, mask, or audit. Returning `Ok(None)` + /// means "not present here, try the next source"; an `Err` + /// short-circuits the chain. + async fn get( + &self, + class: ConfigClass, + key: &str, + ) -> Result, ConfigError>; - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError>; + /// Persist `value` under `key`. Same class semantics as `get`. + async fn set( + &self, + class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError>; fn should_persist(&self) -> bool { true } } -pub struct ConfigManager { +pub struct ConfigClient { sources: Vec>, } -impl ConfigManager { +impl ConfigClient { pub fn new(sources: Vec>) -> Self { Self { sources } } + /// Construct the canonical chain a harmony binary should use: + /// + /// `EnvSource → StoreSource → PromptSource` + /// + /// `namespace` scopes the OpenBao key path so multiple harmony + /// binaries on the same machine don't collide. Pick a stable + /// string per binary (the package name is a sane default). + /// + /// **SQLite is deliberately NOT in the canonical chain.** It + /// stores values as cleartext JSON on local disk, which can't + /// safely hold `ConfigClass::Secret` config. Opt in explicitly + /// with [`ConfigClientBuilder::with_sqlite`] only when you + /// know the data is non-secret and you want an offline cache. + /// + /// Equivalent to `ConfigClient::builder(namespace).build()` — + /// reach for [`ConfigClient::builder`] if you want the canonical + /// chain minus one or two sources (e.g. a non-interactive + /// binary that omits `PromptSource`), or plus SQLite. + /// + /// OpenBao connection settings are read from the standard env + /// vars: `OPENBAO_URL` (or `VAULT_ADDR`), `OPENBAO_KV_MOUNT` + /// (default `secret`), `OPENBAO_AUTH_MOUNT` (default `jwt`), + /// `OPENBAO_SKIP_TLS`. Auth picks the first available of + /// `OPENBAO_TOKEN` → cached token on disk → Zitadel OIDC device + /// flow (`HARMONY_SSO_URL` + `HARMONY_SSO_CLIENT_ID`) → userpass + /// (`OPENBAO_USERNAME` + `OPENBAO_PASSWORD`). + /// + /// If `OPENBAO_URL` is not set or the connection fails, the + /// chain degrades to `EnvSource → PromptSource` — there is no + /// local persistence layer by default, so values resolve from + /// env overrides or re-prompt. + /// + /// For chains that need extra sources, custom ordering, or + /// mocks in tests, use [`ConfigClient::new`] directly with + /// sources you build yourself. + pub async fn for_namespace(namespace: &str) -> Result { + Self::builder(namespace).build().await + } + + /// Start a builder for the canonical chain, scoped to + /// `namespace`. Env, OpenBao, and prompt sources are enabled by + /// default; chain `.without_env()`, `.without_openbao()`, or + /// `.without_prompt()` to drop any of them. `.build()` returns a + /// `ConfigClient` with the surviving sources in canonical order. + /// + /// SQLite is **off by default** (it can't safely hold + /// Secret-class values — see [`SqliteSource`]); opt in with + /// [`with_sqlite`](ConfigClientBuilder::with_sqlite) only for + /// non-secret offline caching. + /// + /// # Examples + /// + /// ```no_run + /// # use harmony_config::ConfigClient; + /// # async fn _ex() -> Result<(), harmony_config::ConfigError> { + /// // Canonical chain — equivalent to `for_namespace`. + /// let manager = ConfigClient::builder("my-app").build().await?; + /// + /// // Headless / CI binary: no interactive prompt. + /// let manager = ConfigClient::builder("my-app") + /// .without_prompt() + /// .build() + /// .await?; + /// + /// // Opt into the local SQLite cache (non-secret config only). + /// let manager = ConfigClient::builder("my-app") + /// .with_sqlite() + /// .build() + /// .await?; + /// # Ok(()) } + /// ``` + pub fn builder(namespace: impl Into) -> ConfigClientBuilder { + ConfigClientBuilder::new(namespace.into()) + } + pub async fn get(&self) -> Result { for source in &self.sources { - if let Some(value) = source.get(T::KEY).await? { - let config: T = - serde_json::from_value(value).map_err(|e| ConfigError::Deserialization { - key: T::KEY.to_string(), - source: e, - })?; - debug!("Retrieved config for key {} from source", T::KEY); - return Ok(config); + if let Some(value) = source.get(T::CLASS, T::KEY).await? { + // Per ADR 020: a deserialization failure means the stored + // value is shaped for a different version of the struct + // (branch switch, rename, type change). Treat it as a + // cache miss for THIS source and fall through. The next + // source — or finally PromptSource via get_or_prompt — + // will overwrite the stale entry. + match serde_json::from_value::(value) { + Ok(config) => { + debug!("Retrieved config for key {} from source", T::KEY); + return Ok(config); + } + Err(e) => { + warn!( + "Stale or incompatible value for key {} in source; falling through ({})", + T::KEY, + e + ); + } + } } } Err(ConfigError::NotFound { @@ -102,24 +312,19 @@ impl ConfigManager { match self.get::().await { Ok(config) => Ok(config), Err(ConfigError::NotFound { .. }) => { - let config = - T::parse_to_obj().map_err(|e| ConfigError::PromptError(e.to_string()))?; - - let value = - serde_json::to_value(&config).map_err(|e| ConfigError::Serialization { - key: T::KEY.to_string(), - source: e, - })?; - - for source in &self.sources { - if !source.should_persist() { - continue; - } - if source.set(T::KEY, &value).await.is_ok() { - break; - } - } - + // Route the prompt through `PromptSource::prompt_for` so + // that the process-wide prompt mutex (see prompt.rs) + // serialises concurrent prompts and prevents background + // tokio task log output from corrupting the terminal. + let config = PromptSource::new().prompt_for::().await?; + // Persist via the same code path as a direct `set` so + // every writable source receives the new value. The + // previous implementation stopped at the first + // successful write, so a local cache earlier in the + // chain could absorb the value and the team-scale + // OpenBao store would never see it — defeating the + // point of having OpenBao in the chain at all. + self.set(&config).await?; Ok(config) } Err(e) => Err(e), @@ -133,22 +338,263 @@ impl ConfigManager { })?; for source in &self.sources { - source.set(T::KEY, &value).await?; + source.set(T::CLASS, T::KEY, &value).await?; } Ok(()) } } -static CONFIG_MANAGER: Mutex>> = Mutex::const_new(None); +/// Builder for [`ConfigClient`]. Defaults to the canonical chain +/// (env → openbao → prompt); `.without_X()` drops a default-on +/// source, `.with_sqlite()` opts into the local cache. `.build()` +/// assembles the surviving sources in canonical order and applies +/// the same OpenBao auto-detect-and-fallback logic +/// [`ConfigClient::for_namespace`] uses. +/// +/// SQLite is off by default: it stores cleartext on disk and can't +/// safely hold Secret-class config (see [`SqliteSource`]). +/// +/// Built by [`ConfigClient::builder`]. +pub struct ConfigClientBuilder { + namespace: String, + include_env: bool, + include_openbao: bool, + include_sqlite: bool, + include_prompt: bool, + openbao_overrides: Option, +} + +/// Explicit OpenBao connection params, used to override (per-field) +/// the env-var lookup that [`ConfigClientBuilder::build`] performs +/// by default. +/// +/// Any field left `None` falls back to its env var +/// (`url` ← `OPENBAO_URL` or `VAULT_ADDR`, `kv_mount` ← +/// `OPENBAO_KV_MOUNT`, etc). Setting a field bypasses the env-var +/// lookup for that field only — partial overrides are valid. +/// +/// Use via [`ConfigClientBuilder::openbao_overrides`]. +#[derive(Debug, Clone, Default)] +pub struct OpenbaoConnection { + /// `OPENBAO_URL` / `VAULT_ADDR`. The OpenBao server URL. + pub url: Option, + /// `OPENBAO_KV_MOUNT`. Defaults to `"secret"` when both this and + /// the env var are unset. + pub kv_mount: Option, + /// `OPENBAO_AUTH_MOUNT`. Defaults to `"jwt"` when both this and + /// the env var are unset. + pub auth_mount: Option, + /// `OPENBAO_SKIP_TLS`. Defaults to `false`. + pub skip_tls: Option, + /// `OPENBAO_TOKEN`. + pub token: Option, + /// `OPENBAO_USERNAME`. + pub username: Option, + /// `OPENBAO_PASSWORD`. + pub password: Option, + /// `HARMONY_SSO_URL`. + pub sso_url: Option, + /// `HARMONY_SSO_CLIENT_ID`. + pub sso_client_id: Option, +} + +impl ConfigClientBuilder { + fn new(namespace: String) -> Self { + Self { + namespace, + include_env: true, + include_openbao: true, + // SQLite is opt-in (cleartext-on-disk, unsafe for + // secrets); the other three are on by default. + include_sqlite: false, + include_prompt: true, + openbao_overrides: None, + } + } + + /// Drop `EnvSource` — `HARMONY_CONFIG_` env vars no longer + /// override the chain. + pub fn without_env(mut self) -> Self { + self.include_env = false; + self + } + + /// Drop the OpenBao `StoreSource`. The chain runs entirely off + /// local sources and any in-process env vars. + pub fn without_openbao(mut self) -> Self { + self.include_openbao = false; + self + } + + /// Opt into the `SqliteSource` local cache at + /// `//config.db`. Off by default because + /// SQLite stores values as cleartext JSON and therefore can't + /// safely hold `ConfigClass::Secret` config. Enable this only + /// when you know the config is non-secret and you want an + /// offline cache that survives an OpenBao outage. The source is + /// inserted between OpenBao and the prompt in the chain. + pub fn with_sqlite(mut self) -> Self { + self.include_sqlite = true; + self + } + + /// Drop `PromptSource`. `get_or_prompt` will return `NotFound` + /// instead of opening an interactive prompt — useful for + /// headless / CI binaries. + pub fn without_prompt(mut self) -> Self { + self.include_prompt = false; + self + } + + /// Provide explicit OpenBao connection params, bypassing the + /// env-var lookup for any field that is `Some`. Fields left at + /// `None` still fall back to the corresponding env var, so a + /// partial override (e.g. only `url`) leaves everything else + /// using the env-var contract. + /// + /// Production binaries usually don't need this — env vars are + /// set by the deployment (systemd, k8s, direnv). Use this when + /// you want to drive the connection from code without mutating + /// the process environment (examples, integration tests, + /// multi-tenant binaries pointing at multiple OpenBao + /// instances). + pub fn openbao_overrides(mut self, conn: OpenbaoConnection) -> Self { + self.openbao_overrides = Some(conn); + self + } + + /// Assemble the chain. Errors propagate from + /// `SqliteSource::for_namespace` when SQLite is opted in via + /// `with_sqlite`; OpenBao connection failures are non-fatal + /// (logged as `warn` and the OpenBao source is silently dropped + /// from the chain), so a missing OpenBao stack doesn't break + /// local development. + pub async fn build(self) -> Result { + let env_source: Option> = if self.include_env { + Some(Arc::new(EnvSource)) + } else { + None + }; + + let store_source: Option> = if self.include_openbao { + build_openbao_source(&self.namespace, self.openbao_overrides.as_ref()).await + } else { + None + }; + + let sqlite_source: Option> = if self.include_sqlite { + Some(Arc::new( + source::sqlite::SqliteSource::for_namespace(&self.namespace).await?, + )) + } else { + None + }; + + let prompt_source: Option> = if self.include_prompt { + Some(Arc::new(PromptSource::new())) + } else { + None + }; + + let sources: Vec> = + [env_source, store_source, sqlite_source, prompt_source] + .into_iter() + .flatten() + .collect(); + + debug!( + "ConfigClientBuilder({:?}): built chain with {} source(s)", + self.namespace, + sources.len(), + ); + + Ok(ConfigClient::new(sources)) + } +} + +/// Try to construct an OpenBao-backed `StoreSource`. Each parameter +/// is sourced from `overrides` first (per-field), then from the +/// corresponding env var. Returns `None` (with a `warn!`) when no +/// URL is available or the connection fails — callers treat that as +/// "OpenBao silently absent from the chain", matching the +/// for_namespace fallback behaviour. +async fn build_openbao_source( + namespace: &str, + overrides: Option<&OpenbaoConnection>, +) -> Option> { + // Per-field merge: override wins, env is the fallback. + let override_string = |selector: fn(&OpenbaoConnection) -> Option<&String>, env_key: &str| { + overrides + .and_then(selector) + .cloned() + .or_else(|| std::env::var(env_key).ok()) + }; + let override_bool = |selector: fn(&OpenbaoConnection) -> Option, env_key: &str| { + overrides + .and_then(selector) + .or_else(|| std::env::var(env_key).ok().map(|v| v == "true")) + }; + + // `OPENBAO_URL` falls back to `VAULT_ADDR` as a second env-var name. + let url = overrides.and_then(|o| o.url.clone()).or_else(|| { + std::env::var("OPENBAO_URL") + .or_else(|_| std::env::var("VAULT_ADDR")) + .ok() + }); + + let Some(url) = url else { + warn!("ConfigClientBuilder({namespace:?}): OpenBao URL not set, OpenBao source omitted"); + return None; + }; + + let kv_mount = override_string(|o| o.kv_mount.as_ref(), "OPENBAO_KV_MOUNT") + .unwrap_or_else(|| "secret".to_string()); + let auth_mount = override_string(|o| o.auth_mount.as_ref(), "OPENBAO_AUTH_MOUNT") + .unwrap_or_else(|| "jwt".to_string()); + let skip_tls = override_bool(|o| o.skip_tls, "OPENBAO_SKIP_TLS").unwrap_or(false); + + match harmony_secret::OpenbaoSecretStore::new( + url, + kv_mount, + auth_mount, + skip_tls, + override_string(|o| o.token.as_ref(), "OPENBAO_TOKEN"), + override_string(|o| o.username.as_ref(), "OPENBAO_USERNAME"), + override_string(|o| o.password.as_ref(), "OPENBAO_PASSWORD"), + override_string(|o| o.sso_url.as_ref(), "HARMONY_SSO_URL"), + override_string(|o| o.sso_client_id.as_ref(), "HARMONY_SSO_CLIENT_ID"), + // `jwt_role` / `jwt_auth_mount` left default — + // OpenbaoSecretStore uses "harmony-developer" / "jwt", + // matching what OpenbaoSetupScore configures. + None, + None, + ) + .await + { + Ok(store) => { + debug!("ConfigClientBuilder({namespace:?}): OpenBao connected"); + Some(Arc::new(StoreSource::new(namespace.to_string(), store))) + } + Err(e) => { + warn!( + "ConfigClientBuilder({namespace:?}): OpenBao unreachable ({e}), \ + OpenBao source omitted" + ); + None + } + } +} + +static CONFIG_CLIENT: Mutex>> = Mutex::const_new(None); pub async fn init(sources: Vec>) { - let mut manager = CONFIG_MANAGER.lock().await; - *manager = Some(Arc::new(ConfigManager::new(sources))); + let mut manager = CONFIG_CLIENT.lock().await; + *manager = Some(Arc::new(ConfigClient::new(sources))); } pub async fn get() -> Result { - let manager = CONFIG_MANAGER.lock().await; + let manager = CONFIG_CLIENT.lock().await; manager .as_ref() .ok_or(ConfigError::NoSources)? @@ -157,7 +603,7 @@ pub async fn get() -> Result { } pub async fn get_or_prompt() -> Result { - let manager = CONFIG_MANAGER.lock().await; + let manager = CONFIG_CLIENT.lock().await; manager .as_ref() .ok_or(ConfigError::NoSources)? @@ -166,7 +612,7 @@ pub async fn get_or_prompt() -> Result { } pub async fn set(config: &T) -> Result<(), ConfigError> { - let manager = CONFIG_MANAGER.lock().await; + let manager = CONFIG_CLIENT.lock().await; manager .as_ref() .ok_or(ConfigError::NoSources)? @@ -229,6 +675,10 @@ mod tests { data: std::sync::Mutex>, get_count: AtomicUsize, set_count: AtomicUsize, + /// Records each `(class, key)` pair observed on `get` / `set` + /// so tests can assert that `ConfigClient` forwards `T::CLASS` + /// to sources unchanged. + observed: std::sync::Mutex>, } impl MockSource { @@ -237,6 +687,7 @@ mod tests { data: std::sync::Mutex::new(std::collections::HashMap::new()), get_count: AtomicUsize::new(0), set_count: AtomicUsize::new(0), + observed: std::sync::Mutex::new(Vec::new()), } } @@ -245,6 +696,7 @@ mod tests { data: std::sync::Mutex::new(data), get_count: AtomicUsize::new(0), set_count: AtomicUsize::new(0), + observed: std::sync::Mutex::new(Vec::new()), } } @@ -255,18 +707,39 @@ mod tests { fn set_call_count(&self) -> usize { self.set_count.load(Ordering::SeqCst) } + + fn observed(&self) -> Vec<(ConfigClass, String, &'static str)> { + self.observed.lock().unwrap().clone() + } } #[async_trait] impl ConfigSource for MockSource { - async fn get(&self, key: &str) -> Result, ConfigError> { + async fn get( + &self, + class: ConfigClass, + key: &str, + ) -> Result, ConfigError> { self.get_count.fetch_add(1, Ordering::SeqCst); + self.observed + .lock() + .unwrap() + .push((class, key.to_string(), "get")); let data = self.data.lock().unwrap(); Ok(data.get(key).cloned()) } - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError> { self.set_count.fetch_add(1, Ordering::SeqCst); + self.observed + .lock() + .unwrap() + .push((class, key.to_string(), "set")); let mut data = self.data.lock().unwrap(); data.insert(key.to_string(), value.clone()); Ok(()) @@ -286,7 +759,7 @@ mod tests { ); let source = Arc::new(MockSource::with_data(data)); - let manager = ConfigManager::new(vec![source.clone()]); + let manager = ConfigClient::new(vec![source.clone()]); let result: TestConfig = manager.get().await.unwrap(); assert_eq!(result, config); @@ -307,7 +780,7 @@ mod tests { let source1 = Arc::new(MockSource::new()); let source2 = Arc::new(MockSource::with_data(data2)); - let manager = ConfigManager::new(vec![source1.clone(), source2.clone()]); + let manager = ConfigClient::new(vec![source1.clone(), source2.clone()]); let result: TestConfig = manager.get().await.unwrap(); assert_eq!(result, config); @@ -318,7 +791,7 @@ mod tests { #[tokio::test] async fn test_get_returns_not_found_when_no_source_has_key() { let source = Arc::new(MockSource::new()); - let manager = ConfigManager::new(vec![source.clone()]); + let manager = ConfigClient::new(vec![source.clone()]); let result: Result = manager.get().await; assert!(matches!(result, Err(ConfigError::NotFound { .. }))); @@ -326,7 +799,7 @@ mod tests { #[tokio::test] async fn test_get_returns_error_with_no_sources() { - let manager = ConfigManager::new(vec![]); + let manager = ConfigClient::new(vec![]); let result: Result = manager.get().await; assert!(matches!(result, Err(ConfigError::NotFound { .. }))); @@ -341,7 +814,7 @@ mod tests { let source1 = Arc::new(MockSource::new()); let source2 = Arc::new(MockSource::new()); - let manager = ConfigManager::new(vec![source1.clone(), source2.clone()]); + let manager = ConfigClient::new(vec![source1.clone(), source2.clone()]); manager.set(&config).await.unwrap(); @@ -352,6 +825,310 @@ mod tests { assert_eq!(result1, config); } + #[tokio::test] + async fn test_derive_macro_emits_standard_class_with_no_secret_fields() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct PlainConfig { + host: String, + port: u16, + } + + assert_eq!(PlainConfig::KEY, "PlainConfig"); + assert_eq!(PlainConfig::CLASS, ConfigClass::Standard); + } + + #[tokio::test] + async fn test_derive_macro_emits_secret_class_when_field_is_tagged() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct WithSecretField { + host: String, + #[config(secret)] + password: String, + } + + assert_eq!(WithSecretField::CLASS, ConfigClass::Secret); + } + + #[tokio::test] + async fn test_derive_macro_emits_secret_class_when_struct_is_tagged() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + #[config(secret)] + struct AllSecret { + token: String, + } + + assert_eq!(AllSecret::CLASS, ConfigClass::Secret); + } + + // -- SECRET_FIELDS derivation ----------------------------------------- + + #[tokio::test] + async fn test_derive_emits_empty_secret_fields_for_standard_struct() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct StandardPlain { + host: String, + port: u16, + } + + assert_eq!(StandardPlain::CLASS, ConfigClass::Standard); + assert_eq!(StandardPlain::SECRET_FIELDS, &[] as &[&str]); + } + + #[tokio::test] + async fn test_derive_emits_tagged_fields_for_per_field_secret() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct MixedSecret { + host: String, + #[config(secret)] + password: String, + #[config(secret)] + api_key: String, + non_secret: u16, + } + + assert_eq!(MixedSecret::CLASS, ConfigClass::Secret); + // Names emitted in struct-field order; only the tagged ones. + assert_eq!(MixedSecret::SECRET_FIELDS, &["password", "api_key"]); + } + + #[tokio::test] + async fn test_derive_emits_all_fields_for_struct_level_secret() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + #[config(secret)] + struct WholeStructSecret { + token: String, + refresh: String, + count: u32, + } + + assert_eq!(WholeStructSecret::CLASS, ConfigClass::Secret); + assert_eq!( + WholeStructSecret::SECRET_FIELDS, + &["token", "refresh", "count"] + ); + } + + // -- Masked wrapper / ConfigExt::masked() ----------------------------- + + #[tokio::test] + async fn test_masked_replaces_secret_fields_with_stars() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct Mixed { + host: String, + #[config(secret)] + password: String, + } + + let v = Mixed { + host: "db.example.com".to_string(), + password: "hunter2".to_string(), + }; + let s = format!("{:?}", v.masked()); + assert!(s.contains("db.example.com"), "host should be visible: {s}"); + assert!( + s.contains("\"****\""), + "password should render as ****: {s}" + ); + assert!(!s.contains("hunter2"), "raw password must not appear: {s}"); + } + + #[tokio::test] + async fn test_masked_passes_standard_struct_through_unmodified() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct PlainStandard { + name: String, + count: u32, + } + + let v = PlainStandard { + name: "alice".to_string(), + count: 7, + }; + let s = format!("{:?}", v.masked()); + assert!(s.contains("alice")); + assert!(s.contains('7')); + assert!(!s.contains("****")); + } + + #[tokio::test] + async fn test_masked_masks_every_field_for_struct_level_secret() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + #[config(secret)] + struct AllSecret { + token: String, + refresh: String, + } + + let v = AllSecret { + token: "tok123".to_string(), + refresh: "ref456".to_string(), + }; + let s = format!("{:?}", v.masked()); + assert!(!s.contains("tok123"), "token must be masked: {s}"); + assert!(!s.contains("ref456"), "refresh must be masked: {s}"); + // Both fields should render as "****". + assert_eq!(s.matches("\"****\"").count(), 2, "got: {s}"); + } + + #[tokio::test] + async fn test_manual_impl_defaults_to_standard_class() { + // The trait gives CLASS a default of Standard so existing manual + // impls (like TestConfig above) compile without explicit updates. + assert_eq!(TestConfig::CLASS, ConfigClass::Standard); + } + + // -- ConfigClientBuilder ---------------------------------------------- + // + // Each test scopes the SqliteSource namespace to a unique + // `__cmbuilder__` prefix so the on-disk files don't collide with + // other tests or developer state at `~/.local/share/harmony/`. + // We always `.without_openbao()` in these tests since the suite + // must not depend on a reachable OpenBao instance. + + #[tokio::test] + async fn test_builder_default_minus_openbao_has_two_sources() { + // SQLite is off by default, so the canonical chain minus + // OpenBao is env + prompt = 2 sources. + let m = ConfigClient::builder("__cmbuilder__minus_openbao") + .without_openbao() + .build() + .await + .unwrap(); + assert_eq!(m.sources.len(), 2); + } + + #[tokio::test] + async fn test_builder_default_omits_sqlite() { + // env + prompt (no openbao reachable, no sqlite opted in). + let without = ConfigClient::builder("__cmbuilder__no_sqlite") + .without_openbao() + .build() + .await + .unwrap(); + assert_eq!(without.sources.len(), 2); + + // Opting in adds exactly one source (env + sqlite + prompt). + let with = ConfigClient::builder("__cmbuilder__with_sqlite") + .without_openbao() + .with_sqlite() + .build() + .await + .unwrap(); + assert_eq!(with.sources.len(), 3); + } + + #[tokio::test] + async fn test_builder_drops_prompt() { + let m = ConfigClient::builder("__cmbuilder__no_prompt") + .without_openbao() + .without_prompt() + .build() + .await + .unwrap(); + // env only (sqlite off by default, openbao + prompt dropped). + assert_eq!(m.sources.len(), 1); + } + + #[tokio::test] + async fn test_builder_all_disabled_yields_empty_chain() { + let m = ConfigClient::builder("__cmbuilder__empty") + .without_env() + .without_openbao() + .without_prompt() + .build() + .await + .unwrap(); + // sqlite was never opted in, the other three dropped → empty. + assert_eq!(m.sources.len(), 0); + + // And confirm get falls through to NotFound cleanly. + let res: Result = m.get().await; + assert!(matches!(res, Err(ConfigError::NotFound { .. }))); + } + + #[tokio::test] + async fn test_builder_methods_chain_freely() { + // Confirms the with_/without_ methods return Self and compose. + let _ = ConfigClient::builder("__cmbuilder__chain") + .without_env() + .without_openbao() + .with_sqlite() + .without_prompt() + .build() + .await + .unwrap(); + } + + #[tokio::test] + async fn test_config_manager_forwards_standard_class_to_sources_on_set() { + let source = Arc::new(MockSource::new()); + let manager = ConfigClient::new(vec![source.clone()]); + let config = TestConfig { + name: "x".into(), + count: 1, + }; + + manager.set(&config).await.unwrap(); + + let observed = source.observed(); + assert_eq!(observed.len(), 1); + let (class, key, op) = &observed[0]; + assert_eq!(*class, ConfigClass::Standard); + assert_eq!(key, "TestConfig"); + assert_eq!(*op, "set"); + } + + #[tokio::test] + async fn test_config_manager_forwards_secret_class_to_sources_on_set() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + struct SecretConfig { + #[config(secret)] + password: String, + } + + assert_eq!(SecretConfig::CLASS, ConfigClass::Secret); + + let source = Arc::new(MockSource::new()); + let manager = ConfigClient::new(vec![source.clone()]); + manager + .set(&SecretConfig { + password: "hunter2".into(), + }) + .await + .unwrap(); + + let (class, key, op) = source.observed().pop().unwrap(); + assert_eq!(class, ConfigClass::Secret); + assert_eq!(key, "SecretConfig"); + assert_eq!(op, "set"); + } + + #[tokio::test] + async fn test_config_manager_forwards_class_to_every_source_on_get() { + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] + #[config(secret)] + struct StructLevelSecret { + token: String, + } + + // Two empty sources so the chain probes both before returning NotFound. + let s1 = Arc::new(MockSource::new()); + let s2 = Arc::new(MockSource::new()); + let manager = ConfigClient::new(vec![s1.clone(), s2.clone()]); + + let res: Result = manager.get().await; + assert!(matches!(res, Err(ConfigError::NotFound { .. }))); + + for src in [&s1, &s2] { + let observed = src.observed(); + assert_eq!(observed.len(), 1); + let (class, key, op) = &observed[0]; + assert_eq!(*class, ConfigClass::Secret); + assert_eq!(key, "StructLevelSecret"); + assert_eq!(*op, "get"); + } + } + #[tokio::test] async fn test_derive_macro_generates_correct_key() { #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] @@ -372,7 +1149,12 @@ mod tests { let env_var = setup_env_vars("TestConfig", Some(r#"{"name":"from_env","count":7}"#)); let source = EnvSource; - let result = source.get(&env_var.replace("HARMONY_CONFIG_", "")).await; + let result = source + .get( + ConfigClass::Standard, + &env_var.replace("HARMONY_CONFIG_", ""), + ) + .await; unsafe { std::env::remove_var(&env_var); @@ -396,7 +1178,12 @@ mod tests { .unwrap(); rt.block_on(async { let source = EnvSource; - let result = source.get(&env_var.replace("HARMONY_CONFIG_", "")).await; + let result = source + .get( + ConfigClass::Standard, + &env_var.replace("HARMONY_CONFIG_", ""), + ) + .await; assert!(result.unwrap().is_none()); }); }); @@ -408,7 +1195,12 @@ mod tests { let env_var = setup_env_vars("TestConfig", Some("not valid json")); let source = EnvSource; - let result = source.get(&env_var.replace("HARMONY_CONFIG_", "")).await; + let result = source + .get( + ConfigClass::Standard, + &env_var.replace("HARMONY_CONFIG_", ""), + ) + .await; unsafe { std::env::remove_var(&env_var); @@ -430,7 +1222,11 @@ mod tests { std::fs::write(&config_path, serde_json::to_string(&config).unwrap()).unwrap(); let source = LocalFileSource::new(dir.path().to_path_buf()); - let result = source.get("TestConfig").await.unwrap().unwrap(); + let result = source + .get(ConfigClass::Standard, "TestConfig") + .await + .unwrap() + .unwrap(); let parsed: TestConfig = serde_json::from_value(result).unwrap(); assert_eq!(parsed, config); @@ -442,7 +1238,10 @@ mod tests { let dir = tempdir().unwrap(); let source = LocalFileSource::new(dir.path().to_path_buf()); - let result = source.get("NonExistentConfig").await.unwrap(); + let result = source + .get(ConfigClass::Standard, "NonExistentConfig") + .await + .unwrap(); assert!(result.is_none()); } @@ -459,7 +1258,11 @@ mod tests { }; source - .set("TestConfig", &serde_json::to_value(&config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config).unwrap(), + ) .await .unwrap(); @@ -484,11 +1287,19 @@ mod tests { }; source - .set("TestConfig", &serde_json::to_value(&config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config).unwrap(), + ) .await .unwrap(); - let result = source.get("TestConfig").await.unwrap().unwrap(); + let result = source + .get(ConfigClass::Standard, "TestConfig") + .await + .unwrap() + .unwrap(); let parsed: TestConfig = serde_json::from_value(result).unwrap(); assert_eq!(parsed, config); @@ -502,7 +1313,10 @@ mod tests { let path = temp_file.path().to_path_buf(); let source = SqliteSource::open(path).await.unwrap(); - let result = source.get("NonExistentConfig").await.unwrap(); + let result = source + .get(ConfigClass::Standard, "NonExistentConfig") + .await + .unwrap(); assert!(result.is_none()); } @@ -525,15 +1339,27 @@ mod tests { }; source - .set("TestConfig", &serde_json::to_value(&config1).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config1).unwrap(), + ) .await .unwrap(); source - .set("TestConfig", &serde_json::to_value(&config2).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config2).unwrap(), + ) .await .unwrap(); - let result = source.get("TestConfig").await.unwrap().unwrap(); + let result = source + .get(ConfigClass::Standard, "TestConfig") + .await + .unwrap() + .unwrap(); let parsed: TestConfig = serde_json::from_value(result).unwrap(); assert_eq!(parsed, config2); @@ -561,17 +1387,33 @@ mod tests { let (r1, r2) = tokio::join!( async { source - .set("key1", &serde_json::to_value(&config1).unwrap()) + .set( + ConfigClass::Standard, + "key1", + &serde_json::to_value(&config1).unwrap(), + ) .await .unwrap(); - source.get("key1").await.unwrap().unwrap() + source + .get(ConfigClass::Standard, "key1") + .await + .unwrap() + .unwrap() }, async { source - .set("key2", &serde_json::to_value(&config2).unwrap()) + .set( + ConfigClass::Standard, + "key2", + &serde_json::to_value(&config2).unwrap(), + ) .await .unwrap(); - source.get("key2").await.unwrap().unwrap() + source + .get(ConfigClass::Standard, "key2") + .await + .unwrap() + .unwrap() } ); @@ -608,7 +1450,7 @@ mod tests { let source1 = Arc::new(MockSource::with_data(data)); let source2 = Arc::new(MockSource::new()); - let manager = ConfigManager::new(vec![source1.clone(), source2.clone()]); + let manager = ConfigClient::new(vec![source1.clone(), source2.clone()]); let result: TestConfig = manager.get_or_prompt().await.unwrap(); assert_eq!(result, config); @@ -626,7 +1468,7 @@ mod tests { .unwrap(); let sqlite = Arc::new(sqlite); - let manager = ConfigManager::new(vec![sqlite.clone()]); + let manager = ConfigClient::new(vec![sqlite.clone()]); let config = TestConfig { name: "from_sqlite".to_string(), @@ -634,7 +1476,11 @@ mod tests { }; sqlite - .set("TestConfig", &serde_json::to_value(&config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config).unwrap(), + ) .await .unwrap(); @@ -653,7 +1499,7 @@ mod tests { let sqlite = Arc::new(sqlite); let env_source = Arc::new(EnvSource); - let manager = ConfigManager::new(vec![env_source.clone(), sqlite.clone()]); + let manager = ConfigClient::new(vec![env_source.clone(), sqlite.clone()]); let sqlite_config = TestConfig { name: "from_sqlite".to_string(), @@ -665,7 +1511,11 @@ mod tests { }; sqlite - .set("TestConfig", &serde_json::to_value(&sqlite_config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&sqlite_config).unwrap(), + ) .await .unwrap(); @@ -684,7 +1534,13 @@ mod tests { } #[tokio::test] - async fn test_branch_switching_scenario_deserialization_error() { + async fn test_branch_switching_scenario_deserialization_falls_through() { + // ADR 020 contract: a stale value (wrong shape for the current + // struct, e.g. after a branch switch that renamed or retyped a + // field) should be treated as a cache miss for that source. The + // chain falls through; if no source has a valid value, the result + // is `NotFound`, which `get_or_prompt` turns into a re-prompt + // that overwrites the stale entry. use tempfile::NamedTempFile; let temp_file = NamedTempFile::new().unwrap(); @@ -693,22 +1549,61 @@ mod tests { .unwrap(); let sqlite = Arc::new(sqlite); - let manager = ConfigManager::new(vec![sqlite.clone()]); + let manager = ConfigClient::new(vec![sqlite.clone()]); let old_config = serde_json::json!({ "name": "old_config", "count": "not_a_number" }); - sqlite.set("TestConfig", &old_config).await.unwrap(); + sqlite + .set(ConfigClass::Standard, "TestConfig", &old_config) + .await + .unwrap(); let result: Result = manager.get().await; - assert!(matches!(result, Err(ConfigError::Deserialization { .. }))); + assert!(matches!(result, Err(ConfigError::NotFound { .. }))); + } + + #[tokio::test] + async fn test_deserialization_failure_falls_through_to_next_source() { + // First source has a stale value; second source has a fresh one. + // The chain should skip past the stale entry and resolve from + // the second source. + use tempfile::NamedTempFile; + + let temp_file = NamedTempFile::new().unwrap(); + let stale_sqlite = SqliteSource::open(temp_file.path().to_path_buf()) + .await + .unwrap(); + let stale_sqlite = Arc::new(stale_sqlite); + + let stale = serde_json::json!({"name": "stale", "count": "not_a_number"}); + stale_sqlite + .set(ConfigClass::Standard, "TestConfig", &stale) + .await + .unwrap(); + + let fresh_config = TestConfig { + name: "fresh".to_string(), + count: 7, + }; + let mut fresh_data = std::collections::HashMap::new(); + fresh_data.insert( + "TestConfig".to_string(), + serde_json::to_value(&fresh_config).unwrap(), + ); + let fresh = Arc::new(MockSource::with_data(fresh_data)); + + let manager = ConfigClient::new(vec![stale_sqlite.clone(), fresh.clone()]); + + let result: TestConfig = manager.get().await.unwrap(); + assert_eq!(result, fresh_config); } #[tokio::test] async fn test_prompt_source_always_returns_none() { let source = PromptSource::new(); - let result = source.get("AnyKey").await.unwrap(); + let result = source.get(ConfigClass::Standard, "AnyKey").await.unwrap(); assert!(result.is_none()); } @@ -716,7 +1611,11 @@ mod tests { async fn test_prompt_source_set_is_noop() { let source = PromptSource::new(); let result = source - .set("AnyKey", &serde_json::json!({"test": "value"})) + .set( + ConfigClass::Standard, + "AnyKey", + &serde_json::json!({"test": "value"}), + ) .await; assert!(result.is_ok()); } @@ -726,13 +1625,17 @@ mod tests { let source = PromptSource::new(); source .set( + ConfigClass::Standard, "TestConfig", &serde_json::json!({"name": "test", "count": 42}), ) .await .unwrap(); - let result = source.get("TestConfig").await.unwrap(); + let result = source + .get(ConfigClass::Standard, "TestConfig") + .await + .unwrap(); assert!(result.is_none()); } @@ -750,7 +1653,7 @@ mod tests { let prompt_source = Arc::new(PromptSource::new()); let manager = - ConfigManager::new(vec![source1.clone(), sqlite.clone(), prompt_source.clone()]); + ConfigClient::new(vec![source1.clone(), sqlite.clone(), prompt_source.clone()]); let result: Result = manager.get().await; assert!(matches!(result, Err(ConfigError::NotFound { .. }))); @@ -781,14 +1684,18 @@ mod tests { let store_source = Arc::new(StoreSource::new("test".to_string(), AlwaysErrorStore)); - let manager = ConfigManager::new(vec![store_source.clone(), sqlite.clone()]); + let manager = ConfigClient::new(vec![store_source.clone(), sqlite.clone()]); let config = TestConfig { name: "from_sqlite".to_string(), count: 42, }; sqlite - .set("TestConfig", &serde_json::to_value(&config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config).unwrap(), + ) .await .unwrap(); @@ -825,14 +1732,18 @@ mod tests { let store_source = Arc::new(StoreSource::new("test".to_string(), NeverFindsStore)); - let manager = ConfigManager::new(vec![store_source.clone(), sqlite.clone()]); + let manager = ConfigClient::new(vec![store_source.clone(), sqlite.clone()]); let config = TestConfig { name: "from_sqlite".to_string(), count: 99, }; sqlite - .set("TestConfig", &serde_json::to_value(&config).unwrap()) + .set( + ConfigClass::Standard, + "TestConfig", + &serde_json::to_value(&config).unwrap(), + ) .await .unwrap(); diff --git a/harmony_config/src/source/env.rs b/harmony_config/src/source/env.rs index 5e201598..261eab80 100644 --- a/harmony_config/src/source/env.rs +++ b/harmony_config/src/source/env.rs @@ -1,4 +1,4 @@ -use crate::{ConfigError, ConfigSource}; +use crate::{ConfigClass, ConfigError, ConfigSource}; use async_trait::async_trait; pub struct EnvSource; @@ -9,7 +9,11 @@ fn env_key_for(config_key: &str) -> String { #[async_trait] impl ConfigSource for EnvSource { - async fn get(&self, key: &str) -> Result, ConfigError> { + async fn get( + &self, + _class: ConfigClass, + key: &str, + ) -> Result, ConfigError> { let env_key = env_key_for(key); match std::env::var(&env_key) { @@ -27,7 +31,12 @@ impl ConfigSource for EnvSource { } } - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + _class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError> { let env_key = env_key_for(key); let json_string = serde_json::to_string(value).map_err(|e| ConfigError::Serialization { key: key.to_string(), diff --git a/harmony_config/src/source/local_file.rs b/harmony_config/src/source/local_file.rs index f803b2cc..8769e0b8 100644 --- a/harmony_config/src/source/local_file.rs +++ b/harmony_config/src/source/local_file.rs @@ -2,8 +2,17 @@ use async_trait::async_trait; use std::path::PathBuf; use tokio::fs; -use crate::{ConfigError, ConfigSource}; +use crate::{ConfigClass, ConfigError, ConfigSource}; +/// Local file-backed config source (`/.json`). +/// +/// ⚠️ **SECURITY: stores values as cleartext JSON on local disk and +/// ignores the `ConfigClass` hint.** Like [`SqliteSource`](crate::SqliteSource), +/// it must not be used to cache `ConfigClass::Secret` config — a +/// secret written here is plaintext on disk. It is not part of the +/// canonical chain; use it only via an explicit +/// [`ConfigClient::new`](crate::ConfigClient::new) for non-secret +/// config. pub struct LocalFileSource { base_path: PathBuf, } @@ -24,7 +33,11 @@ impl LocalFileSource { #[async_trait] impl ConfigSource for LocalFileSource { - async fn get(&self, key: &str) -> Result, ConfigError> { + async fn get( + &self, + _class: ConfigClass, + key: &str, + ) -> Result, ConfigError> { let path = self.file_path_for(key); match fs::read(&path).await { @@ -45,7 +58,12 @@ impl ConfigSource for LocalFileSource { } } - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + _class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError> { fs::create_dir_all(&self.base_path).await?; let path = self.file_path_for(key); diff --git a/harmony_config/src/source/prompt.rs b/harmony_config/src/source/prompt.rs index 79a5ea2b..64815dd6 100644 --- a/harmony_config/src/source/prompt.rs +++ b/harmony_config/src/source/prompt.rs @@ -1,22 +1,57 @@ use async_trait::async_trait; -use std::sync::Arc; +use inquire::{CustomType, Password, PasswordDisplayMode, Text}; +use schemars::schema::{InstanceType, RootSchema, Schema, SchemaObject, SingleOrVec}; +use tokio::sync::Mutex; -use crate::{ConfigError, ConfigSource}; +use crate::{Config, ConfigClass, ConfigError, ConfigSource}; -pub struct PromptSource { - #[allow(dead_code)] - writer: Option>, -} +/// Process-wide async mutex guarding interactive prompts. ADR 020 +/// motivates this: `inquire` assumes exclusive terminal ownership, and +/// background tokio tasks that emit log output during a prompt corrupt +/// the terminal state. Holding this lock across the prompt serialises +/// concurrent prompts. +/// +/// The mutex does not silence background log output — that is a logger +/// concern. It only ensures two `harmony_config` prompts do not run at +/// the same time on the same terminal. +static PROMPT_MUTEX: Mutex<()> = Mutex::const_new(()); + +pub struct PromptSource; impl PromptSource { pub fn new() -> Self { - Self { writer: None } + Self } - #[allow(dead_code)] - pub fn with_writer(writer: Arc) -> Self { - Self { - writer: Some(writer), + /// Prompt the user for a `T` while holding the process-wide prompt + /// mutex. Returns the parsed value or a `ConfigError::PromptError` + /// if the user cancels or input parsing fails. + /// + /// `Standard`-class structs flow through `interactive_parse`'s + /// `parse_to_obj` — full nested-struct / enum / Option support. + /// `Secret`-class structs flow through a narrower walker that + /// uses `inquire::Password` for fields tagged `#[config(secret)]`. + /// The walker only supports flat primitive fields (String, + /// integer, bool); a Secret struct with non-primitive fields + /// returns a clear `PromptError`. + pub async fn prompt_for(&self) -> Result { + let _guard = PROMPT_MUTEX.lock().await; + let banner = format!( + "── Configuring `{}` ({:?}) — please fill the fields below ──", + T::KEY, + T::CLASS, + ); + // inquire renders prompts on stderr, so we match that channel + // and surround the banner with blank lines so it stays + // visually separate from preceding log output. + eprintln!(); + eprintln!("{banner}"); + eprintln!(); + match T::CLASS { + ConfigClass::Standard => { + T::parse_to_obj().map_err(|e| ConfigError::PromptError(e.to_string())) + } + ConfigClass::Secret => prompt_secret_struct::(), } } } @@ -29,11 +64,20 @@ impl Default for PromptSource { #[async_trait] impl ConfigSource for PromptSource { - async fn get(&self, _key: &str) -> Result, ConfigError> { + async fn get( + &self, + _class: ConfigClass, + _key: &str, + ) -> Result, ConfigError> { Ok(None) } - async fn set(&self, _key: &str, _value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + _class: ConfigClass, + _key: &str, + _value: &serde_json::Value, + ) -> Result<(), ConfigError> { Ok(()) } @@ -41,3 +85,155 @@ impl ConfigSource for PromptSource { false } } + +/// Schema-walking prompt for `Secret`-class structs. Renders +/// `inquire::Password` for fields named in `T::SECRET_FIELDS` (when +/// the field is a string); other fields fall through to +/// type-appropriate prompts. Fails fast on non-primitive field +/// types — Secret structs with nested data are not yet supported +/// because the masking story for nested fields isn't designed. +fn prompt_secret_struct() -> Result { + let root: RootSchema = schemars::schema_for!(T); + let object = root.schema.object.as_deref().ok_or_else(|| { + ConfigError::PromptError(format!( + "Secret-class struct `{}` does not have a JSON-object schema; the \ + secret-aware prompt walker only supports plain structs", + T::KEY + )) + })?; + + let mut json = serde_json::Map::with_capacity(object.properties.len()); + for (field_name, field_schema) in &object.properties { + let field_object = schema_object(field_schema, T::KEY, field_name)?; + let is_secret = T::SECRET_FIELDS.contains(&field_name.as_str()); + let value = prompt_field::(field_name, field_object, is_secret)?; + json.insert(field_name.clone(), value); + } + + serde_json::from_value(serde_json::Value::Object(json)).map_err(|e| { + ConfigError::PromptError(format!( + "Failed to deserialize prompted values into `{}`: {e}", + T::KEY + )) + }) +} + +fn schema_object<'a>( + schema: &'a Schema, + struct_name: &str, + field_name: &str, +) -> Result<&'a SchemaObject, ConfigError> { + match schema { + Schema::Object(obj) => Ok(obj), + Schema::Bool(_) => Err(ConfigError::PromptError(format!( + "Secret-class struct `{struct_name}` field `{field_name}` resolved \ + to a `true`/`false` JSON-schema, which the secret-aware prompt \ + walker can't handle. Use a concrete type." + ))), + } +} + +/// Pick the right `inquire` widget based on the JsonSchema for one +/// field. Strings tagged secret use `Password`; everything else uses +/// the type-appropriate text input. Returns `Err` for shapes the +/// walker doesn't support — never silently falls back to an unmasked +/// prompt. +fn prompt_field( + field_name: &str, + schema: &SchemaObject, + is_secret: bool, +) -> Result { + let instance_type = schema.instance_type.as_ref().ok_or_else(|| { + ConfigError::PromptError(format!( + "Secret-class struct `{}` field `{field_name}` has no `instance_type` \ + in its JSON-schema; the secret-aware prompt walker only handles \ + flat primitive fields (string / integer / bool). Either flatten \ + the struct or open an issue to extend the walker.", + T::KEY + )) + })?; + + let single = match instance_type { + SingleOrVec::Single(boxed) => **boxed, + SingleOrVec::Vec(_) => { + return Err(ConfigError::PromptError(format!( + "Secret-class struct `{}` field `{field_name}` has a multi-type \ + JSON-schema (e.g. nullable). The secret-aware prompt walker \ + only handles single primitive types. Either flatten or open \ + an issue.", + T::KEY + ))); + } + }; + + let label = format!("{field_name}:"); + match single { + InstanceType::String => { + if is_secret { + // `PasswordDisplayMode::Masked` echoes `*` per keystroke as + // the operator types, instead of inquire's default + // `Hidden` mode (which displays nothing until the user + // hits enter). Live feedback on a hidden field is the + // usual UX expectation; full-hide is too quiet. + let raw = Password::new(&label) + .with_display_mode(PasswordDisplayMode::Masked) + .without_confirmation() + .prompt() + .map_err(|e| { + ConfigError::PromptError(format!( + "Password prompt for `{}::{field_name}` failed: {e}", + T::KEY + )) + })?; + Ok(serde_json::Value::String(raw)) + } else { + let raw = Text::new(&label).prompt().map_err(|e| { + ConfigError::PromptError(format!( + "Text prompt for `{}::{field_name}` failed: {e}", + T::KEY + )) + })?; + Ok(serde_json::Value::String(raw)) + } + } + InstanceType::Integer => { + // `i64` covers the integer types we'll see in Config + // structs (u16 ports, u32 counts, signed counters). The + // resulting JSON number deserialises back into the + // narrower type via serde. + let n = CustomType::::new(&label).prompt().map_err(|e| { + ConfigError::PromptError(format!( + "Integer prompt for `{}::{field_name}` failed: {e}", + T::KEY + )) + })?; + Ok(serde_json::json!(n)) + } + InstanceType::Number => { + let n = CustomType::::new(&label).prompt().map_err(|e| { + ConfigError::PromptError(format!( + "Number prompt for `{}::{field_name}` failed: {e}", + T::KEY + )) + })?; + Ok(serde_json::json!(n)) + } + InstanceType::Boolean => { + let b = CustomType::::new(&label).prompt().map_err(|e| { + ConfigError::PromptError(format!( + "Boolean prompt for `{}::{field_name}` failed: {e}", + T::KEY + )) + })?; + Ok(serde_json::Value::Bool(b)) + } + other => Err(ConfigError::PromptError(format!( + "Secret-class struct `{}` field `{field_name}` has unsupported \ + JSON-schema type `{other:?}`. The secret-aware prompt walker only \ + handles flat primitives (string, integer, number, boolean). Split \ + the nested data into a separate Standard-class struct, or open an \ + issue to extend the walker.", + T::KEY + ))), + } +} diff --git a/harmony_config/src/source/sqlite.rs b/harmony_config/src/source/sqlite.rs index c39568ef..2a2d77ad 100644 --- a/harmony_config/src/source/sqlite.rs +++ b/harmony_config/src/source/sqlite.rs @@ -3,8 +3,28 @@ use sqlx::{SqlitePool, sqlite::SqlitePoolOptions}; use std::path::PathBuf; use tokio::fs; -use crate::{ConfigError, ConfigSource}; +use crate::{ConfigClass, ConfigError, ConfigSource}; +/// Local SQLite-backed config cache. +/// +/// ⚠️ **SECURITY: this source stores every value as cleartext JSON in +/// a file on local disk (`//config.db`).** It +/// applies no encryption and ignores the `ConfigClass` hint — a +/// `ConfigClass::Secret` value written here lands in plaintext, +/// readable by anyone with filesystem access. SQLite is therefore +/// **deliberately excluded from the canonical chain** +/// ([`ConfigClient::for_namespace`](crate::ConfigClient::for_namespace) / +/// [`ConfigClient::builder`](crate::ConfigClient::builder)); it is +/// opt-in only via +/// [`ConfigClientBuilder::with_sqlite`](crate::ConfigClientBuilder::with_sqlite). +/// +/// Only enable it when you are certain the config flowing through the +/// manager is **non-secret** (Standard class) and you want an offline +/// cache that survives an OpenBao outage. For secrets, rely on OpenBao +/// — it encrypts at rest and gates access via policy. Per-class +/// at-rest handling (encrypted local cache, or skipping Secret-class +/// writes) is tracked as future work; until then, treat this source +/// as plaintext. pub struct SqliteSource { pool: SqlitePool, } @@ -37,6 +57,29 @@ impl SqliteSource { Ok(Self { pool }) } + /// Open a SQLite source scoped to `namespace`. The database file + /// lives at `//config.db`, isolating state + /// from other harmony binaries on the same machine. Use the same + /// namespace string you pass to `StoreSource::new(...)` so the + /// two persistence layers stay symmetric for one binary. + pub async fn for_namespace(namespace: &str) -> Result { + let path = crate::default_config_dir() + .ok_or_else(|| { + ConfigError::SqliteError("Could not determine default config directory".into()) + })? + .join(namespace) + .join("config.db"); + Self::open(path).await + } + + /// Legacy un-namespaced default at `/config.db`. + /// Every caller using this method shares the same SQLite file + /// and the same single `config` table — two harmony binaries + /// with overlapping `Config::KEY`s will overwrite each other. + /// Prefer [`SqliteSource::for_namespace`]. + #[deprecated( + note = "Use SqliteSource::for_namespace(...) so multiple harmony binaries don't share one config.db" + )] pub async fn default() -> Result { let path = crate::default_config_dir() .ok_or_else(|| { @@ -49,7 +92,11 @@ impl SqliteSource { #[async_trait] impl ConfigSource for SqliteSource { - async fn get(&self, key: &str) -> Result, ConfigError> { + async fn get( + &self, + _class: ConfigClass, + key: &str, + ) -> Result, ConfigError> { let row: Option<(String,)> = sqlx::query_as("SELECT value FROM config WHERE key = ?") .bind(key) .fetch_optional(&self.pool) @@ -69,7 +116,12 @@ impl ConfigSource for SqliteSource { } } - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + _class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError> { let json_string = serde_json::to_string(value).map_err(|e| ConfigError::Serialization { key: key.to_string(), source: e, diff --git a/harmony_config/src/source/store.rs b/harmony_config/src/source/store.rs index a8bd8ad5..de2bb1f9 100644 --- a/harmony_config/src/source/store.rs +++ b/harmony_config/src/source/store.rs @@ -1,7 +1,8 @@ use async_trait::async_trait; use harmony_secret::SecretStore; +use log::warn; -use crate::{ConfigError, ConfigSource}; +use crate::{ConfigClass, ConfigError, ConfigSource}; pub struct StoreSource { namespace: String, @@ -16,7 +17,17 @@ impl StoreSource { #[async_trait] impl ConfigSource for StoreSource { - async fn get(&self, key: &str) -> Result, ConfigError> { + async fn get( + &self, + _class: ConfigClass, + key: &str, + ) -> Result, ConfigError> { + // TODO(ADR 020-1): propagate `_class` to `SecretStore` when the + // path-taxonomy work lands. Today the SecretStore trait does not + // accept a class hint, so a per-class routing change here would + // need a corresponding signature change in harmony_secret. Keep + // the parameter in our trait so the ConfigSource boundary + // matches ADR 020 even before the storage backend can act on it. match self.store.get_raw(&self.namespace, key).await { Ok(bytes) => { let value: serde_json::Value = @@ -27,11 +38,25 @@ impl ConfigSource for StoreSource { Ok(Some(value)) } Err(harmony_secret::SecretStoreError::NotFound { .. }) => Ok(None), - Err(_) => Ok(None), + // Log before swallowing: a down/misconfigured OpenBao must not look identical to "key absent". + Err(e) => { + warn!( + "StoreSource: get for key '{key}' failed ({e}); treating as \ + absent and falling through to the next source" + ); + Ok(None) + } } } - async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { + async fn set( + &self, + _class: ConfigClass, + key: &str, + value: &serde_json::Value, + ) -> Result<(), ConfigError> { + // TODO(ADR 020-1): see `get` above — `_class` is accepted at the + // ConfigSource boundary but not yet forwarded to SecretStore. let bytes = serde_json::to_vec(value).map_err(|e| ConfigError::Serialization { key: key.to_string(), source: e, diff --git a/harmony_config_derive/src/lib.rs b/harmony_config_derive/src/lib.rs index b5a0681d..dfc5aef7 100644 --- a/harmony_config_derive/src/lib.rs +++ b/harmony_config_derive/src/lib.rs @@ -1,17 +1,34 @@ use proc_macro::TokenStream; use proc_macro_crate::{FoundCrate, crate_name}; use quote::quote; -use syn::{DeriveInput, Ident, parse_macro_input}; +use syn::{Attribute, Data, DeriveInput, Fields, Ident, parse_macro_input}; -#[proc_macro_derive(Config)] +/// Derives `harmony_config::Config`, emitting `KEY` (the struct name), +/// `CLASS`, and `SECRET_FIELDS`. +/// +/// Mark a field — or the whole struct — `#[config(secret)]` to elevate +/// it to `ConfigClass::Secret` and record the tagged field names in +/// `SECRET_FIELDS` (used for output masking and password-style prompts). +/// +/// **Constraint:** do not combine `#[config(secret)]` with +/// `#[serde(rename = "…")]` / `#[serde(rename_all = "…")]` on the same +/// field. Masking and secret prompts match the *serialized* name, but +/// this macro records the Rust ident, so a renamed secret would leak in +/// cleartext. See `Config::SECRET_FIELDS`. +#[proc_macro_derive(Config, attributes(config))] pub fn derive_config(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let struct_ident = &input.ident; let key = struct_ident.to_string(); + // Always emit a fully qualified path. `proc-macro-crate` returns + // `FoundCrate::Itself` for examples in the same package, but examples + // are separate binary crates — their `crate::` root does not contain + // harmony_config's items. Using `::harmony_config::` (or the renamed + // import) works everywhere. let config_crate_path = match crate_name("harmony_config") { - Ok(FoundCrate::Itself) => quote!(crate), + Ok(FoundCrate::Itself) => quote!(::harmony_config), Ok(FoundCrate::Name(name)) => { let ident = Ident::new(&name, proc_macro2::Span::call_site()); quote!(::#ident) @@ -23,11 +40,72 @@ pub fn derive_config(input: TokenStream) -> TokenStream { } }; + let struct_level_secret = has_secret_attr(&input.attrs); + + // Collect the names of every field tagged `#[config(secret)]`. + // When the WHOLE struct is `#[config(secret)]`, every named field + // is considered secret. Tuple / unit structs contribute no names. + let mut secret_field_names: Vec = Vec::new(); + if let Data::Struct(ref data) = input.data + && let Fields::Named(named) = &data.fields + { + for field in &named.named { + let Some(ident) = field.ident.as_ref() else { + continue; + }; + if struct_level_secret || has_secret_attr(&field.attrs) { + secret_field_names.push(ident.to_string()); + } + } + } + + let is_secret = struct_level_secret || !secret_field_names.is_empty(); + + let class = if is_secret { + quote!(#config_crate_path::ConfigClass::Secret) + } else { + quote!(#config_crate_path::ConfigClass::Standard) + }; + + let secret_fields_const = if secret_field_names.is_empty() { + // Standard structs and structs without named secret fields + // get the trait default (`&[]`); omitting the const keeps + // the generated impl smaller. + quote!() + } else { + let names = secret_field_names.iter().map(|n| quote!(#n)); + quote! { + const SECRET_FIELDS: &'static [&'static str] = &[ #( #names ),* ]; + } + }; + let expanded = quote! { impl #config_crate_path::Config for #struct_ident { const KEY: &'static str = #key; + const CLASS: #config_crate_path::ConfigClass = #class; + #secret_fields_const } }; TokenStream::from(expanded) } + +/// Returns true if `attrs` carries `#[config(secret)]`. +fn has_secret_attr(attrs: &[Attribute]) -> bool { + attrs.iter().any(|attr| { + if !attr.path().is_ident("config") { + return false; + } + // `#[config(secret)]` parses as a list-style meta with a single + // `secret` ident. Any other form (e.g. unknown keys) is ignored + // for now; future attributes will be added explicitly. + let mut found = false; + let _ = attr.parse_nested_meta(|meta| { + if meta.path.is_ident("secret") { + found = true; + } + Ok(()) + }); + found + }) +} -- 2.39.5 From aa709aa68b9d2980478da26890649132bd157b50 Mon Sep 17 00:00:00 2001 From: Sylvain Tremblay Date: Thu, 28 May 2026 13:46:48 -0400 Subject: [PATCH 2/3] fix(harmony_config): migrate example-harmony-sso call site to ConfigClient The ConfigManager->ConfigClient rename in this PR updates its in-tree consumer so master stays green. Minimal 2-line call-site fix; the example's full ADR-020 rework lands in the example PR. Co-Authored-By: Claude Opus 4.7 --- examples/harmony_sso/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/harmony_sso/src/main.rs b/examples/harmony_sso/src/main.rs index 8bd09a59..1fe895b3 100644 --- a/examples/harmony_sso/src/main.rs +++ b/examples/harmony_sso/src/main.rs @@ -10,7 +10,7 @@ use harmony::modules::zitadel::{ }; use harmony::score::Score; use harmony::topology::{K8sclient, Topology}; -use harmony_config::{Config, ConfigManager, EnvSource, StoreSource}; +use harmony_config::{Config, ConfigClient, EnvSource, StoreSource}; use harmony_k8s::K8sClient; use harmony_secret::OpenbaoSecretStore; use k3d_rs::{K3d, PortMapping}; @@ -380,7 +380,7 @@ async fn main() -> anyhow::Result<()> { .await .context("SSO authentication failed")?; - let manager = ConfigManager::new(vec![ + let manager = ConfigClient::new(vec![ Arc::new(EnvSource) as Arc, Arc::new(StoreSource::new("harmony".to_string(), store)), ]); -- 2.39.5 From f37ab6aba1a7d77cc2c0a53b18cb04b1d258aaf2 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Fri, 29 May 2026 12:12:41 -0400 Subject: [PATCH 3/3] feat: harmony_config densification of code and comments, remove much bloat --- harmony_config/examples/openbao_chain.rs | 445 ++---------------- harmony_config/src/lib.rs | 570 +++-------------------- harmony_config/src/source/env.rs | 9 +- harmony_config/src/source/local_file.rs | 10 +- harmony_config/src/source/prompt.rs | 91 ++-- harmony_config/src/source/sqlite.rs | 49 +- harmony_config/src/source/store.rs | 10 +- harmony_config_derive/src/lib.rs | 36 +- 8 files changed, 144 insertions(+), 1076 deletions(-) diff --git a/harmony_config/examples/openbao_chain.rs b/harmony_config/examples/openbao_chain.rs index 88f0b11c..23f8dccc 100644 --- a/harmony_config/examples/openbao_chain.rs +++ b/harmony_config/examples/openbao_chain.rs @@ -1,130 +1,23 @@ -//! # Dev-binary template: harmony_config against a harmony-sso-example stack +//! Dev-binary template: `ConfigClient` against an OpenBao-backed chain. //! -//! This example is the consumer-side counterpart to -//! `examples/harmony_sso`. Where `harmony_sso` puts the SSO + OpenBao -//! infrastructure in place, this example shows how a developer writes -//! a new harmony binary that *uses* that infrastructure to manage its -//! configuration and secrets. +//! Bring up the OpenBao + Zitadel stack first (`cargo run -p +//! example-harmony-sso`), then export the connection env vars and run this: //! -//! ## Prerequisite — the harmony-sso-example stack must be running -//! -//! Before running this example, run the harmony-sso-example demo at -//! least once so that: -//! -//! - the local k3d cluster `harmony-example` is up, -//! - OpenBao is deployed, initialised, unsealed, and reachable through -//! the ingress at `bao.harmony.local:8080`, -//! - Zitadel is deployed with a device-code OAuth application named -//! `harmony-cli`, reachable through the ingress at -//! `sso.harmony.local:8080`, -//! - `/etc/hosts` resolves both hostnames to `127.0.0.1`. -//! -//! Run the prereq stack: -//! ```bash -//! cargo run -p example-harmony-sso -//! ``` -//! -//! Then run this example: //! ```bash +//! export OPENBAO_URL=http://bao.harmony.local:8080 +//! export HARMONY_SSO_URL=http://sso.harmony.local:8080 +//! export HARMONY_SSO_CLIENT_ID= # or OPENBAO_TOKEN= //! cargo run -p harmony_config --example openbao_chain //! ``` //! -//! ## Use cases demonstrated -//! -//! 1. Build a `ConfigClient` with the chain a real harmony binary -//! would use: `EnvSource → StoreSource → PromptSource`. -//! OpenBao is the team-scale source of truth. There is no local -//! cache layer by default — SQLite is opt-in only -//! (`builder(..).with_sqlite()`) because it stores values as -//! cleartext on disk and can't safely hold Secret-class config. -//! 2. Authenticate to OpenBao via the Zitadel device flow on first run, -//! then cache the session so subsequent runs are silent. -//! 3. `manager.get::()` — read existing config. -//! 4. `manager.set(&value)` — persist to every writable source. -//! 5. `manager.get_or_prompt::()` — interactively gather config the -//! first time, then cache it. -//! 6. **Edit existing config** — when `get` returns a cached value the -//! example shows it and asks the operator whether to update it. -//! If they say yes, `PromptSource::prompt_for` collects new values -//! and `manager.set` persists them. This is the canonical -//! "re-run to reconfigure" loop for a long-lived binary. -//! 7. Round-trip both a `ConfigClass::Standard` struct (`AppConfig`) and -//! a `ConfigClass::Secret` struct (`DatabaseCredentials`) through the -//! same chain — no per-call ceremony. -//! 8. **Masked input on Secret-class structs** (`ApiCredentials`). -//! `#[config(secret)]` fields are read via `inquire::Password` -//! (echoed as `*`) when the chain falls through to -//! `PromptSource`. The Standard `client_id` field uses normal -//! text input. The same step shows the -//! "cached → confirm → re-prompt" update loop applied to a -//! Secret struct. -//! 9. **Safe logging via `.masked()`.** Every log line that prints a -//! `Config` value goes through `value.masked()`, which masks -//! `#[config(secret)]` fields as `"****"`. Standard-class -//! values render unmodified, so the pattern is safe-by-default -//! at every call site. -//! 10. Demonstrate the env-var override: `HARMONY_CONFIG_AppConfig` -//! short-circuits the chain. -//! 11. Auth-method override: `OPENBAO_TOKEN` or -//! `OPENBAO_USERNAME`+`OPENBAO_PASSWORD` bypass SSO for testing -//! against a non-SSO OpenBao. -//! -//! ## Configuration — built in code, passed to the builder -//! -//! Connection params are computed in plain Rust and handed to -//! `ConfigClient::builder("...").openbao_overrides(...)`, so the -//! example never mutates the process environment to configure -//! itself. (Rust 2024 makes `std::env::set_var` `unsafe`; we'd -//! rather not pay that cost for plumbing that can live as data.) -//! Any env var that's already set takes precedence, so you can -//! still point the example at a different OpenBao by exporting -//! variables from your shell: -//! -//! | Variable | In-code default | Source of truth | -//! |---|---|---| -//! | `OPENBAO_URL` | `http://bao.harmony.local:8080` | this file | -//! | `HARMONY_SSO_URL` | `http://sso.harmony.local:8080` | this file | -//! | `HARMONY_SSO_CLIENT_ID` | auto-discovered from harmony's cache | **see disclaimer below** | -//! | `OPENBAO_KV_MOUNT` | builder default `secret` | env | -//! | `OPENBAO_AUTH_MOUNT` | builder default `jwt` | env, matches `OpenbaoSetupScore` | -//! | `OPENBAO_TOKEN` | (unset) | optional override | -//! | `OPENBAO_USERNAME`/`OPENBAO_PASSWORD` | (unset) | optional override | -//! -//! ## ⚠ `HARMONY_SSO_CLIENT_ID` auto-discovery — example scaffolding only -//! -//! In production, `HARMONY_SSO_CLIENT_ID` is configured by whatever -//! provisions the binary's environment — a systemd unit, a Kubernetes -//! ConfigMap, a `direnv`/`.envrc` file, your CI job, etc. The value is -//! set externally; the binary just reads it. -//! -//! This example, however, runs locally against a stack that is brought -//! up via `examples/harmony_sso`, which generates the client_id -//! dynamically (it varies per Zitadel install). To make the example -//! "just work" without forcing the operator to look the value up -//! manually, we **read it out of harmony's own cache file at -//! `~/.local/share/harmony/zitadel/client-config.json`** and export it -//! back to the environment. -//! -//! **Do not copy the auto-discovery helper into a real harmony binary.** -//! It reads from a path that is an implementation detail of -//! `ZitadelSetupScore`, not a public contract. Production binaries -//! must take `HARMONY_SSO_CLIENT_ID` from the environment. +//! If OpenBao is unreachable the chain degrades to env → prompt, so the +//! round-trip steps below need a reachable OpenBao to persist. -use std::path::PathBuf; - -use harmony_config::{Config, ConfigClient, ConfigExt, OpenbaoConnection, PromptSource}; -use inquire::Confirm; +use harmony_config::{Config, ConfigClient, ConfigExt}; use log::info; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -// --------------------------------------------------------------------------- -// Config types -// --------------------------------------------------------------------------- - -/// Plain operational config. Standard class — no `#[config(secret)]` -/// fields, so backends are free to log / display / cache it however -/// they like. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] struct AppConfig { host: String, @@ -140,11 +33,8 @@ impl Default for AppConfig { } } -/// Mixed-sensitivity config. The `password` field is tagged -/// `#[config(secret)]`, which elevates the whole struct to -/// `ConfigClass::Secret`. The struct still round-trips through the -/// same `ConfigClient` chain — the class is a backend hint, not a -/// dispatch. +// `password` is `#[config(secret)]`, so the whole struct is Secret-class: +// masked in logs and prompted via `inquire::Password`. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] struct DatabaseCredentials { host: String, @@ -163,325 +53,44 @@ impl Default for DatabaseCredentials { } } -/// A struct intentionally lacking a `Default` impl so we can show the -/// `get_or_prompt` flow: on first run there is no cached value and no -/// in-code default, so the manager prompts interactively, stores the -/// answers, and subsequent runs find the value without prompting. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] -struct UserPreferences { - display_name: String, - theme: String, -} - -/// Secret-class struct without a `Default` impl. Exists specifically -/// to exercise the masked-prompt path: on first run the chain falls -/// through to `PromptSource`, the walker sees -/// `Config::CLASS == Secret`, and routes each tagged field through -/// `inquire::Password` (echoed as `*`) instead of `inquire::Text`. -/// `client_id` stays a plain text prompt because it isn't tagged. +// No `Default`, so `get_or_prompt` falls through to an interactive prompt. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq, Config)] struct ApiCredentials { client_id: String, #[config(secret)] client_secret: String, - #[config(secret)] - refresh_token: String, } -// --------------------------------------------------------------------------- -// Connection params — built in code, passed to ConfigClient::builder -// --------------------------------------------------------------------------- - -/// Path harmony writes to when `ZitadelSetupScore` provisions the -/// `harmony-cli` device-code application. **Implementation detail of -/// the harmony-sso-example stack** — not a public contract. Used here -/// only to spare the operator from manually exporting -/// `HARMONY_SSO_CLIENT_ID` after every `cargo run -p example-harmony-sso`. -fn zitadel_client_config_path() -> Option { - Some( - directories::BaseDirs::new()? - .data_dir() - .join("harmony") - .join("zitadel") - .join("client-config.json"), - ) -} - -/// Auto-discovery for the `harmony-cli` OIDC client_id, from harmony's -/// own cache file. -/// -/// ⚠ Example scaffolding only. A real harmony binary must read -/// `HARMONY_SSO_CLIENT_ID` from the environment — it is configured by -/// whatever provisions the binary (systemd, Kubernetes, direnv, CI). -/// Do not copy this helper into production code. -fn discover_zitadel_client_id() -> Option { - let path = zitadel_client_config_path()?; - let bytes = std::fs::read(&path).ok()?; - let v: serde_json::Value = serde_json::from_slice(&bytes).ok()?; - v.get("apps")? - .get("harmony-cli")? - .as_str() - .map(str::to_string) -} - -/// Build the OpenBao connection params that target the -/// harmony-sso-example stack. We compute the values in plain Rust -/// and pass them through `ConfigClientBuilder::openbao_overrides`, -/// so the example doesn't have to mutate the process environment to -/// configure itself. -/// -/// Each field honours an existing env var if set (so an operator -/// running this against a different OpenBao can override from their -/// shell); otherwise we fall back to the harmony-sso defaults. -fn harmony_sso_example_connection() -> anyhow::Result { - let sso_client_id = std::env::var("HARMONY_SSO_CLIENT_ID") - .ok() - .or_else(|| { - let discovered = discover_zitadel_client_id(); - if let Some(ref id) = discovered { - info!( - "openbao_chain: auto-discovered HARMONY_SSO_CLIENT_ID={id} \ - from harmony's local cache (EXAMPLE-ONLY; production binaries \ - must read this from the environment)" - ); - } - discovered - }) - .ok_or_else(|| { - anyhow::anyhow!( - "HARMONY_SSO_CLIENT_ID is not set and no client-config cache was \ - found at {:?}. Either:\n (a) bring up the harmony-sso-example \ - stack first (`cargo run -p example-harmony-sso`), which writes \ - that file, or\n (b) export HARMONY_SSO_CLIENT_ID yourself \ - pointing at your Zitadel app.", - zitadel_client_config_path() - ) - })?; - - Ok(OpenbaoConnection { - url: Some( - std::env::var("OPENBAO_URL") - .unwrap_or_else(|_| "http://bao.harmony.local:8080".to_string()), - ), - sso_url: Some( - std::env::var("HARMONY_SSO_URL") - .unwrap_or_else(|_| "http://sso.harmony.local:8080".to_string()), - ), - sso_client_id: Some(sso_client_id), - // `kv_mount` (defaults to "secret") and `auth_mount` - // (defaults to "jwt") match what OpenbaoSetupScore - // configures in harmony-sso-example, so we leave them at - // None to use the builder's defaults. Token / userpass - // overrides are intentionally absent — SSO is the happy - // path; export OPENBAO_TOKEN from your shell to force the - // token-auth fallback. - ..Default::default() - }) -} - -// (Chain construction is `ConfigClient::builder(...)` — see main().) - -// --------------------------------------------------------------------------- -// Use-case walkthroughs -// --------------------------------------------------------------------------- - -/// First time around there's nothing stored, so we set a default; -/// subsequent runs read the stored value back. Logs the struct's -/// `CLASS` so the operator can see Standard / Secret routing. -/// -/// Logs route through `.masked()` so any `#[config(secret)]` field -/// renders as `****` in the output. For Standard-class types this -/// is a no-op (`SECRET_FIELDS` is empty), so it's safe-by-default -/// to always go through `.masked()` for log lines that print a -/// `Config` value. -async fn demo_round_trip(manager: &ConfigClient, default_value: T) -> anyhow::Result<()> +// Read `T`, or store `default` and read it back to prove the round-trip. +async fn round_trip(client: &ConfigClient, default: T) -> anyhow::Result<()> where T: Config + std::fmt::Debug + Clone + PartialEq, { - info!( - "[{key}] CLASS={class:?}, attempting read…", - key = T::KEY, - class = T::CLASS, - ); - match manager.get::().await { - Ok(found) => { - info!("[{}] read existing value: {:?}", T::KEY, found.masked()); - } + match client.get::().await { + Ok(found) => info!("[{}] read existing: {:?}", T::KEY, found.masked()), Err(harmony_config::ConfigError::NotFound { .. }) => { - info!("[{}] not found — storing default and reading back", T::KEY); - manager.set(&default_value).await?; - let retrieved: T = manager.get().await?; - anyhow::ensure!( - retrieved == default_value, - "Round-trip mismatch for {}: stored != retrieved", - T::KEY - ); - info!("[{}] round-trip verified: {:?}", T::KEY, retrieved.masked()); + client.set(&default).await?; + let back: T = client.get().await?; + anyhow::ensure!(back == default, "round-trip mismatch for {}", T::KEY); + info!("[{}] stored + verified: {:?}", T::KEY, back.masked()); } Err(e) => return Err(e.into()), } Ok(()) } -/// Shows that `HARMONY_CONFIG_` always wins, regardless of what's -/// in OpenBao. Useful for per-process tweaks (a CI job, a -/// one-off dry-run) without mutating shared state. -/// -/// This is the **only** `unsafe` block in the example — and it's not -/// a workaround. The env-var override mechanism is the feature -/// being demonstrated; calling `set_var` here is the demo, not -/// configuration plumbing. (For chain configuration we pass -/// `OpenbaoConnection` through the builder, which avoids -/// `env::set_var` entirely.) -async fn demo_env_override(manager: &ConfigClient) -> anyhow::Result<()> { - info!("[env-override] setting HARMONY_CONFIG_AppConfig in-process"); - let override_value = AppConfig { - host: "ci-override.example.com".to_string(), - port: 9090, - }; - // SAFETY: `std::env::set_var` is unsafe under Rust 2024 because - // POSIX `setenv` races against concurrent libc reads on other - // threads. We're inside the tokio current-thread runtime and no - // other harmony code is reading `HARMONY_CONFIG_AppConfig` - // outside this manager's `EnvSource::get`, which we await - // immediately after the write. Single-threaded + bounded - // window → no race. - unsafe { - std::env::set_var( - "HARMONY_CONFIG_AppConfig", - serde_json::to_string(&override_value)?, - ); - } - let seen: AppConfig = manager.get().await?; - anyhow::ensure!( - seen == override_value, - "env override did not win: got {seen:?}, expected {override_value:?}" - ); - info!("[env-override] env source short-circuited the chain: {seen:?}"); - // SAFETY: same conditions as the set_var above. - unsafe { - std::env::remove_var("HARMONY_CONFIG_AppConfig"); - } - Ok(()) -} - -/// Two flows in one step, depending on whether the value is already -/// cached: -/// -/// - **Cold run (cache miss).** No cached value and no `Default` impl -/// on `T`, so `get_or_prompt` prompts interactively (via `inquire`, -/// requires a TTY) and persists the answers. -/// -/// - **Subsequent run (cache hit).** The value is found in OpenBao -/// without prompting. We then *show* the cached value and -/// ask via `inquire::Confirm` whether the operator wants to update -/// it. On "yes", `PromptSource::prompt_for` re-collects every -/// field and `manager.set` persists the new value to every -/// writable source. On "no", the cached value stands. -/// -/// When `T::CLASS == Secret`, the prompt walker uses -/// `inquire::Password` for `#[config(secret)]` fields — input is -/// echoed as `*` instead of the real characters. All log lines -/// route through `.masked()` so cached / updated values render -/// with their secret fields replaced by `"****"`. -/// -/// Both branches require a TTY; on a CI runner with no terminal the -/// step fails — that's the same contract every interactive harmony -/// binary has. -async fn demo_get_or_prompt(manager: &ConfigClient) -> anyhow::Result<()> -where - T: Config + std::fmt::Debug + Clone + PartialEq, -{ - info!("[{key}] CLASS={class:?}", key = T::KEY, class = T::CLASS,); - - match manager.get::().await { - Ok(existing) => { - info!("[{}] cached value found: {:?}", T::KEY, existing.masked()); - let want_change = Confirm::new(&format!("Update `{}` with new values?", T::KEY)) - .with_default(false) - .prompt() - .map_err(|e| anyhow::anyhow!("Confirm prompt failed: {e}"))?; - if want_change { - // PromptSource::prompt_for holds the process-wide - // prompt mutex, so any background log noise gets - // serialised against the prompt. For Secret-class - // T it also routes #[config(secret)] string fields - // through inquire::Password (input echoed as `*`). - let updated: T = PromptSource::new().prompt_for().await?; - manager.set(&updated).await?; - info!( - "[{}] updated and persisted to every writable source: {:?}", - T::KEY, - updated.masked(), - ); - } else { - info!("[{}] keeping cached value (no update requested)", T::KEY); - } - } - Err(harmony_config::ConfigError::NotFound { .. }) => { - info!( - "[{}] no cached value — get_or_prompt will gather + persist", - T::KEY - ); - let resolved: T = manager.get_or_prompt().await?; - info!("[{}] resolved: {:?}", T::KEY, resolved.masked()); - } - Err(e) => return Err(e.into()), - } - Ok(()) -} - -// --------------------------------------------------------------------------- -// Entry point -// --------------------------------------------------------------------------- - #[tokio::main] async fn main() -> anyhow::Result<()> { - env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info")) - .format_timestamp_secs() - .init(); + env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info")).init(); - // Compute connection params in plain Rust (no env mutation — - // see module-level note about avoiding `unsafe`) and pass them - // to the builder. A production binary leaves this off and - // relies on env vars set by its deployment. - let openbao = harmony_sso_example_connection()?; - let manager = ConfigClient::builder("harmony") - .openbao_overrides(openbao) - .build() - .await?; + let client = ConfigClient::for_namespace("harmony").await; - println!("\n=== harmony_config dev-binary demo ===\n"); + round_trip(&client, AppConfig::default()).await?; + round_trip(&client, DatabaseCredentials::default()).await?; - info!("Step 1/5 — round-trip Standard-class struct (AppConfig)"); - demo_round_trip(&manager, AppConfig::default()).await?; - - info!( - "Step 2/5 — round-trip Secret-class struct (DatabaseCredentials); logs route through .masked()" - ); - demo_round_trip(&manager, DatabaseCredentials::default()).await?; - - info!("Step 3/5 — env-var override demo"); - demo_env_override(&manager).await?; - - info!("Step 4/5 — get_or_prompt on a Standard struct (UserPreferences); plain text prompts"); - demo_get_or_prompt::(&manager).await?; - - info!( - "Step 5/5 — get_or_prompt on a Secret struct (ApiCredentials); \ - #[config(secret)] fields are read with inquire::Password (`*` masked input)" - ); - demo_get_or_prompt::(&manager).await?; - - println!("\n=== Done. ==="); - println!("Stored values live in:"); - println!( - " • OpenBao @ secret/harmony/ (browse via http://bao.harmony.local:8080/ui)" - ); - println!(" • OIDC cache @ ~/.local/share/harmony/secrets/oidc_session_*"); - println!( - " (no local SQLite cache — not in the default chain; opt in with builder().with_sqlite())" - ); + // No default + Secret class: prompts, with `client_secret` read via Password. + let api: ApiCredentials = client.get_or_prompt().await?; + info!("[{}] resolved: {:?}", ApiCredentials::KEY, api.masked()); Ok(()) } diff --git a/harmony_config/src/lib.rs b/harmony_config/src/lib.rs index 710d1c0a..dde30471 100644 --- a/harmony_config/src/lib.rs +++ b/harmony_config/src/lib.rs @@ -1,8 +1,5 @@ -// Lets the derive macro emit `::harmony_config::ConfigClass` (etc.) and have -// it resolve both inside this crate (lib + tests) and outside (examples, -// downstream consumers). Without this alias, the macro would need separate -// emission paths for "called from within harmony_config" vs everywhere else, -// and `proc-macro-crate` cannot distinguish examples from the lib. +// Lets the derive macro emit `::harmony_config::…` paths that resolve both +// inside this crate and in downstream consumers. extern crate self as harmony_config; mod source; @@ -10,7 +7,7 @@ mod source; use async_trait::async_trait; use directories::ProjectDirs; use interactive_parse::InteractiveParseObj; -use log::{debug, warn}; +use log::warn; use schemars::JsonSchema; use serde::{Serialize, de::DeserializeOwned}; use std::path::PathBuf; @@ -64,104 +61,44 @@ pub enum ConfigError { SqliteError(String), } -/// Tells the storage backend how to treat a `Config` value. -/// -/// The class is a hint passed through the `ConfigSource` chain. Sources are -/// free to ignore it; backends that care (OpenBao routing, future audit -/// logging, terminal masking in `PromptSource`) inspect it. +/// Hint passed through the `ConfigSource` chain; backends that care +/// (masking, prompting) inspect it, others ignore it. Elevated to `Secret` +/// automatically when any field — or the struct — is `#[config(secret)]`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ConfigClass { - /// Plaintext storage is acceptable. The default for any `Config` - /// struct that does not contain `#[config(secret)]` fields. Standard, - /// Must be treated as a secret: encrypted at rest, masked in UI, - /// audited where the backend supports it. Elevated automatically - /// when any field carries `#[config(secret)]` or the struct itself - /// is annotated. Secret, } pub trait Config: Serialize + DeserializeOwned + JsonSchema + InteractiveParseObj + Sized { const KEY: &'static str; const CLASS: ConfigClass = ConfigClass::Standard; - /// Names of fields carrying `#[config(secret)]`. The derive macro - /// populates this; manual `impl Config for` and Standard-class - /// structs see the trait default of `&[]`. Used by - /// `PromptSource` to switch a field to `inquire::Password` and by - /// the `Masked` wrapper to replace those field values with - /// `"****"` in formatted output. - /// - /// **Constraint — do not combine `#[config(secret)]` with - /// `#[serde(rename = "…")]` or `#[serde(rename_all = "…")]` on a - /// secret field.** The names recorded here are the Rust field - /// *idents*, but both `Masked` and the secret prompt match against - /// the *serialized* name (serde_json keys / schemars properties). A - /// renamed secret field would therefore fail to match and its value - /// would render and prompt in **cleartext**. Until the derive learns - /// to read serde rename attributes, keep secret field names - /// identical in Rust and serialized form. + // Serialized names of `#[config(secret)]` fields, used for masking and + // password prompts. Matching is on the serialized name, so a secret + // field must not be `#[serde(rename)]`d — the derive records the Rust + // ident, and a rename would leak the value in cleartext. const SECRET_FIELDS: &'static [&'static str] = &[]; } -/// Safe-display wrapper produced by [`ConfigExt::masked`]. -/// -/// `{:?}` on a `Masked` renders the value via serde-JSON, then -/// replaces every field listed in `T::SECRET_FIELDS` with the -/// literal string `"****"`. Standard-class values render as -/// unmodified JSON. Use this anywhere a Config value might end up -/// in a log line or an error message: -/// -/// ```ignore -/// info!("loaded: {:?}", cfg.masked()); -/// ``` +/// Safe-display wrapper: `{:?}` renders `T::SECRET_FIELDS` as `"****"`. +/// Use it on any `Config` value headed for a log line or error message. pub struct Masked<'a, T: Config>(&'a T); impl std::fmt::Debug for Masked<'_, T> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut value = serde_json::to_value(self.0).map_err(|_| std::fmt::Error)?; - if T::CLASS == ConfigClass::Secret { - mask_value(&mut value, T::SECRET_FIELDS); + if let serde_json::Value::Object(map) = &mut value { + for name in T::SECRET_FIELDS { + if let Some(v) = map.get_mut(*name) { + *v = serde_json::Value::String("****".to_string()); + } + } } - // Pretty-print so masked values stay readable in multiline - // log output. `{:#}` on serde_json::Value is the indented - // form. write!(f, "{value:#}") } } -/// Recursively replace every `obj[name]` entry whose `name` is in -/// `secret_fields` with `"****"`. Non-object payloads (e.g. a Config -/// implemented on a tuple) are left as-is — secret fields are a -/// per-named-field concept. -fn mask_value(value: &mut serde_json::Value, secret_fields: &[&str]) { - if let serde_json::Value::Object(map) = value { - for (k, v) in map.iter_mut() { - if secret_fields.contains(&k.as_str()) { - *v = serde_json::Value::String("****".to_string()); - } else { - // Walk nested objects too so a Standard struct - // nested inside a Secret one still gets its - // matching secret-field-named leaves redacted if - // the user has them in SECRET_FIELDS (rare today, - // but cheap to support). - mask_value(v, secret_fields); - } - } - } else if let serde_json::Value::Array(items) = value { - for item in items.iter_mut() { - mask_value(item, secret_fields); - } - } -} - -/// Extension trait that adds [`masked`](Self::masked) to every -/// `Config` type. Provided via blanket impl; no boilerplate per -/// struct. pub trait ConfigExt: Config { - /// Wrap `self` in a safe-display [`Masked`] view. Secret-tagged - /// fields render as `"****"`; other fields render as their - /// JSON form. Use in log lines / error messages instead of - /// passing the raw value through `{:?}`. fn masked(&self) -> Masked<'_, Self> { Masked(self) } @@ -171,28 +108,20 @@ impl ConfigExt for T {} #[async_trait] pub trait ConfigSource: Send + Sync { - /// Fetch a JSON value for `key`. The `class` is a hint from the - /// caller's `Config` impl; sources may ignore it (most do today) - /// or use it to route, mask, or audit. Returning `Ok(None)` - /// means "not present here, try the next source"; an `Err` - /// short-circuits the chain. + // `Ok(None)` means "absent here, try the next source"; `Err` + // short-circuits the chain. `class` is a hint most sources ignore. async fn get( &self, class: ConfigClass, key: &str, ) -> Result, ConfigError>; - /// Persist `value` under `key`. Same class semantics as `get`. async fn set( &self, class: ConfigClass, key: &str, value: &serde_json::Value, ) -> Result<(), ConfigError>; - - fn should_persist(&self) -> bool { - true - } } pub struct ConfigClient { @@ -204,102 +133,34 @@ impl ConfigClient { Self { sources } } - /// Construct the canonical chain a harmony binary should use: - /// - /// `EnvSource → StoreSource → PromptSource` - /// - /// `namespace` scopes the OpenBao key path so multiple harmony - /// binaries on the same machine don't collide. Pick a stable - /// string per binary (the package name is a sane default). - /// - /// **SQLite is deliberately NOT in the canonical chain.** It - /// stores values as cleartext JSON on local disk, which can't - /// safely hold `ConfigClass::Secret` config. Opt in explicitly - /// with [`ConfigClientBuilder::with_sqlite`] only when you - /// know the data is non-secret and you want an offline cache. - /// - /// Equivalent to `ConfigClient::builder(namespace).build()` — - /// reach for [`ConfigClient::builder`] if you want the canonical - /// chain minus one or two sources (e.g. a non-interactive - /// binary that omits `PromptSource`), or plus SQLite. - /// - /// OpenBao connection settings are read from the standard env - /// vars: `OPENBAO_URL` (or `VAULT_ADDR`), `OPENBAO_KV_MOUNT` - /// (default `secret`), `OPENBAO_AUTH_MOUNT` (default `jwt`), - /// `OPENBAO_SKIP_TLS`. Auth picks the first available of - /// `OPENBAO_TOKEN` → cached token on disk → Zitadel OIDC device - /// flow (`HARMONY_SSO_URL` + `HARMONY_SSO_CLIENT_ID`) → userpass - /// (`OPENBAO_USERNAME` + `OPENBAO_PASSWORD`). - /// - /// If `OPENBAO_URL` is not set or the connection fails, the - /// chain degrades to `EnvSource → PromptSource` — there is no - /// local persistence layer by default, so values resolve from - /// env overrides or re-prompt. - /// - /// For chains that need extra sources, custom ordering, or - /// mocks in tests, use [`ConfigClient::new`] directly with - /// sources you build yourself. - pub async fn for_namespace(namespace: &str) -> Result { - Self::builder(namespace).build().await - } - - /// Start a builder for the canonical chain, scoped to - /// `namespace`. Env, OpenBao, and prompt sources are enabled by - /// default; chain `.without_env()`, `.without_openbao()`, or - /// `.without_prompt()` to drop any of them. `.build()` returns a - /// `ConfigClient` with the surviving sources in canonical order. - /// - /// SQLite is **off by default** (it can't safely hold - /// Secret-class values — see [`SqliteSource`]); opt in with - /// [`with_sqlite`](ConfigClientBuilder::with_sqlite) only for - /// non-secret offline caching. - /// - /// # Examples - /// - /// ```no_run - /// # use harmony_config::ConfigClient; - /// # async fn _ex() -> Result<(), harmony_config::ConfigError> { - /// // Canonical chain — equivalent to `for_namespace`. - /// let manager = ConfigClient::builder("my-app").build().await?; - /// - /// // Headless / CI binary: no interactive prompt. - /// let manager = ConfigClient::builder("my-app") - /// .without_prompt() - /// .build() - /// .await?; - /// - /// // Opt into the local SQLite cache (non-secret config only). - /// let manager = ConfigClient::builder("my-app") - /// .with_sqlite() - /// .build() - /// .await?; - /// # Ok(()) } - /// ``` - pub fn builder(namespace: impl Into) -> ConfigClientBuilder { - ConfigClientBuilder::new(namespace.into()) + /// Canonical chain `EnvSource → OpenBao → PromptSource`, scoped to + /// `namespace`. OpenBao is configured from env vars (see + /// [`openbao_from_env`]) and dropped from the chain if unset or + /// unreachable. SQLite is excluded on purpose: it stores cleartext on + /// disk and can't safely hold `Secret`-class config. For other chains, + /// build the sources yourself and pass them to [`ConfigClient::new`]. + pub async fn for_namespace(namespace: &str) -> Self { + let mut sources: Vec> = vec![Arc::new(EnvSource)]; + if let Some(store) = openbao_from_env(namespace).await { + sources.push(store); + } + sources.push(Arc::new(PromptSource::new())); + Self::new(sources) } pub async fn get(&self) -> Result { for source in &self.sources { if let Some(value) = source.get(T::CLASS, T::KEY).await? { - // Per ADR 020: a deserialization failure means the stored - // value is shaped for a different version of the struct - // (branch switch, rename, type change). Treat it as a - // cache miss for THIS source and fall through. The next - // source — or finally PromptSource via get_or_prompt — - // will overwrite the stale entry. + // A deser failure means the stored value is shaped for a + // different version of the struct (branch switch, rename); + // treat it as a miss for this source and fall through so a + // later source — or a re-prompt — overwrites the stale entry. match serde_json::from_value::(value) { - Ok(config) => { - debug!("Retrieved config for key {} from source", T::KEY); - return Ok(config); - } - Err(e) => { - warn!( - "Stale or incompatible value for key {} in source; falling through ({})", - T::KEY, - e - ); - } + Ok(config) => return Ok(config), + Err(e) => warn!( + "Stale value for key {} in source; falling through ({e})", + T::KEY + ), } } } @@ -312,18 +173,7 @@ impl ConfigClient { match self.get::().await { Ok(config) => Ok(config), Err(ConfigError::NotFound { .. }) => { - // Route the prompt through `PromptSource::prompt_for` so - // that the process-wide prompt mutex (see prompt.rs) - // serialises concurrent prompts and prevents background - // tokio task log output from corrupting the terminal. let config = PromptSource::new().prompt_for::().await?; - // Persist via the same code path as a direct `set` so - // every writable source receives the new value. The - // previous implementation stopped at the first - // successful write, so a local cache earlier in the - // chain could absorb the value and the team-scale - // OpenBao store would never see it — defeating the - // point of having OpenBao in the chain at all. self.set(&config).await?; Ok(config) } @@ -345,242 +195,40 @@ impl ConfigClient { } } -/// Builder for [`ConfigClient`]. Defaults to the canonical chain -/// (env → openbao → prompt); `.without_X()` drops a default-on -/// source, `.with_sqlite()` opts into the local cache. `.build()` -/// assembles the surviving sources in canonical order and applies -/// the same OpenBao auto-detect-and-fallback logic -/// [`ConfigClient::for_namespace`] uses. -/// -/// SQLite is off by default: it stores cleartext on disk and can't -/// safely hold Secret-class config (see [`SqliteSource`]). -/// -/// Built by [`ConfigClient::builder`]. -pub struct ConfigClientBuilder { - namespace: String, - include_env: bool, - include_openbao: bool, - include_sqlite: bool, - include_prompt: bool, - openbao_overrides: Option, -} - -/// Explicit OpenBao connection params, used to override (per-field) -/// the env-var lookup that [`ConfigClientBuilder::build`] performs -/// by default. -/// -/// Any field left `None` falls back to its env var -/// (`url` ← `OPENBAO_URL` or `VAULT_ADDR`, `kv_mount` ← -/// `OPENBAO_KV_MOUNT`, etc). Setting a field bypasses the env-var -/// lookup for that field only — partial overrides are valid. -/// -/// Use via [`ConfigClientBuilder::openbao_overrides`]. -#[derive(Debug, Clone, Default)] -pub struct OpenbaoConnection { - /// `OPENBAO_URL` / `VAULT_ADDR`. The OpenBao server URL. - pub url: Option, - /// `OPENBAO_KV_MOUNT`. Defaults to `"secret"` when both this and - /// the env var are unset. - pub kv_mount: Option, - /// `OPENBAO_AUTH_MOUNT`. Defaults to `"jwt"` when both this and - /// the env var are unset. - pub auth_mount: Option, - /// `OPENBAO_SKIP_TLS`. Defaults to `false`. - pub skip_tls: Option, - /// `OPENBAO_TOKEN`. - pub token: Option, - /// `OPENBAO_USERNAME`. - pub username: Option, - /// `OPENBAO_PASSWORD`. - pub password: Option, - /// `HARMONY_SSO_URL`. - pub sso_url: Option, - /// `HARMONY_SSO_CLIENT_ID`. - pub sso_client_id: Option, -} - -impl ConfigClientBuilder { - fn new(namespace: String) -> Self { - Self { - namespace, - include_env: true, - include_openbao: true, - // SQLite is opt-in (cleartext-on-disk, unsafe for - // secrets); the other three are on by default. - include_sqlite: false, - include_prompt: true, - openbao_overrides: None, - } - } - - /// Drop `EnvSource` — `HARMONY_CONFIG_` env vars no longer - /// override the chain. - pub fn without_env(mut self) -> Self { - self.include_env = false; - self - } - - /// Drop the OpenBao `StoreSource`. The chain runs entirely off - /// local sources and any in-process env vars. - pub fn without_openbao(mut self) -> Self { - self.include_openbao = false; - self - } - - /// Opt into the `SqliteSource` local cache at - /// `//config.db`. Off by default because - /// SQLite stores values as cleartext JSON and therefore can't - /// safely hold `ConfigClass::Secret` config. Enable this only - /// when you know the config is non-secret and you want an - /// offline cache that survives an OpenBao outage. The source is - /// inserted between OpenBao and the prompt in the chain. - pub fn with_sqlite(mut self) -> Self { - self.include_sqlite = true; - self - } - - /// Drop `PromptSource`. `get_or_prompt` will return `NotFound` - /// instead of opening an interactive prompt — useful for - /// headless / CI binaries. - pub fn without_prompt(mut self) -> Self { - self.include_prompt = false; - self - } - - /// Provide explicit OpenBao connection params, bypassing the - /// env-var lookup for any field that is `Some`. Fields left at - /// `None` still fall back to the corresponding env var, so a - /// partial override (e.g. only `url`) leaves everything else - /// using the env-var contract. - /// - /// Production binaries usually don't need this — env vars are - /// set by the deployment (systemd, k8s, direnv). Use this when - /// you want to drive the connection from code without mutating - /// the process environment (examples, integration tests, - /// multi-tenant binaries pointing at multiple OpenBao - /// instances). - pub fn openbao_overrides(mut self, conn: OpenbaoConnection) -> Self { - self.openbao_overrides = Some(conn); - self - } - - /// Assemble the chain. Errors propagate from - /// `SqliteSource::for_namespace` when SQLite is opted in via - /// `with_sqlite`; OpenBao connection failures are non-fatal - /// (logged as `warn` and the OpenBao source is silently dropped - /// from the chain), so a missing OpenBao stack doesn't break - /// local development. - pub async fn build(self) -> Result { - let env_source: Option> = if self.include_env { - Some(Arc::new(EnvSource)) - } else { - None - }; - - let store_source: Option> = if self.include_openbao { - build_openbao_source(&self.namespace, self.openbao_overrides.as_ref()).await - } else { - None - }; - - let sqlite_source: Option> = if self.include_sqlite { - Some(Arc::new( - source::sqlite::SqliteSource::for_namespace(&self.namespace).await?, - )) - } else { - None - }; - - let prompt_source: Option> = if self.include_prompt { - Some(Arc::new(PromptSource::new())) - } else { - None - }; - - let sources: Vec> = - [env_source, store_source, sqlite_source, prompt_source] - .into_iter() - .flatten() - .collect(); - - debug!( - "ConfigClientBuilder({:?}): built chain with {} source(s)", - self.namespace, - sources.len(), - ); - - Ok(ConfigClient::new(sources)) - } -} - -/// Try to construct an OpenBao-backed `StoreSource`. Each parameter -/// is sourced from `overrides` first (per-field), then from the -/// corresponding env var. Returns `None` (with a `warn!`) when no -/// URL is available or the connection fails — callers treat that as -/// "OpenBao silently absent from the chain", matching the -/// for_namespace fallback behaviour. -async fn build_openbao_source( - namespace: &str, - overrides: Option<&OpenbaoConnection>, -) -> Option> { - // Per-field merge: override wins, env is the fallback. - let override_string = |selector: fn(&OpenbaoConnection) -> Option<&String>, env_key: &str| { - overrides - .and_then(selector) - .cloned() - .or_else(|| std::env::var(env_key).ok()) - }; - let override_bool = |selector: fn(&OpenbaoConnection) -> Option, env_key: &str| { - overrides - .and_then(selector) - .or_else(|| std::env::var(env_key).ok().map(|v| v == "true")) - }; - - // `OPENBAO_URL` falls back to `VAULT_ADDR` as a second env-var name. - let url = overrides.and_then(|o| o.url.clone()).or_else(|| { - std::env::var("OPENBAO_URL") - .or_else(|_| std::env::var("VAULT_ADDR")) - .ok() - }); - - let Some(url) = url else { - warn!("ConfigClientBuilder({namespace:?}): OpenBao URL not set, OpenBao source omitted"); +/// Build an OpenBao-backed `StoreSource` from env vars, or `None` (with a +/// `warn!`) when `OPENBAO_URL`/`VAULT_ADDR` is unset or the connection +/// fails — so a missing OpenBao degrades the chain instead of breaking +/// startup. `jwt_role`/`jwt_auth_mount` are left to `OpenbaoSecretStore`'s +/// defaults, matching what `OpenbaoSetupScore` configures. +async fn openbao_from_env(namespace: &str) -> Option> { + let Some(url) = std::env::var("OPENBAO_URL") + .or_else(|_| std::env::var("VAULT_ADDR")) + .ok() + else { + warn!("OpenBao URL not set; OpenBao source omitted from chain"); return None; }; - let kv_mount = override_string(|o| o.kv_mount.as_ref(), "OPENBAO_KV_MOUNT") - .unwrap_or_else(|| "secret".to_string()); - let auth_mount = override_string(|o| o.auth_mount.as_ref(), "OPENBAO_AUTH_MOUNT") - .unwrap_or_else(|| "jwt".to_string()); - let skip_tls = override_bool(|o| o.skip_tls, "OPENBAO_SKIP_TLS").unwrap_or(false); - - match harmony_secret::OpenbaoSecretStore::new( + let env = |k: &str| std::env::var(k).ok(); + let store = harmony_secret::OpenbaoSecretStore::new( url, - kv_mount, - auth_mount, - skip_tls, - override_string(|o| o.token.as_ref(), "OPENBAO_TOKEN"), - override_string(|o| o.username.as_ref(), "OPENBAO_USERNAME"), - override_string(|o| o.password.as_ref(), "OPENBAO_PASSWORD"), - override_string(|o| o.sso_url.as_ref(), "HARMONY_SSO_URL"), - override_string(|o| o.sso_client_id.as_ref(), "HARMONY_SSO_CLIENT_ID"), - // `jwt_role` / `jwt_auth_mount` left default — - // OpenbaoSecretStore uses "harmony-developer" / "jwt", - // matching what OpenbaoSetupScore configures. + env("OPENBAO_KV_MOUNT").unwrap_or_else(|| "secret".to_string()), + env("OPENBAO_AUTH_MOUNT").unwrap_or_else(|| "jwt".to_string()), + env("OPENBAO_SKIP_TLS").as_deref() == Some("true"), + env("OPENBAO_TOKEN"), + env("OPENBAO_USERNAME"), + env("OPENBAO_PASSWORD"), + env("HARMONY_SSO_URL"), + env("HARMONY_SSO_CLIENT_ID"), None, None, ) - .await - { - Ok(store) => { - debug!("ConfigClientBuilder({namespace:?}): OpenBao connected"); - Some(Arc::new(StoreSource::new(namespace.to_string(), store))) - } + .await; + + match store { + Ok(store) => Some(Arc::new(StoreSource::new(namespace.to_string(), store))), Err(e) => { - warn!( - "ConfigClientBuilder({namespace:?}): OpenBao unreachable ({e}), \ - OpenBao source omitted" - ); + warn!("OpenBao unreachable ({e}); source omitted from chain"); None } } @@ -977,88 +625,6 @@ mod tests { assert_eq!(TestConfig::CLASS, ConfigClass::Standard); } - // -- ConfigClientBuilder ---------------------------------------------- - // - // Each test scopes the SqliteSource namespace to a unique - // `__cmbuilder__` prefix so the on-disk files don't collide with - // other tests or developer state at `~/.local/share/harmony/`. - // We always `.without_openbao()` in these tests since the suite - // must not depend on a reachable OpenBao instance. - - #[tokio::test] - async fn test_builder_default_minus_openbao_has_two_sources() { - // SQLite is off by default, so the canonical chain minus - // OpenBao is env + prompt = 2 sources. - let m = ConfigClient::builder("__cmbuilder__minus_openbao") - .without_openbao() - .build() - .await - .unwrap(); - assert_eq!(m.sources.len(), 2); - } - - #[tokio::test] - async fn test_builder_default_omits_sqlite() { - // env + prompt (no openbao reachable, no sqlite opted in). - let without = ConfigClient::builder("__cmbuilder__no_sqlite") - .without_openbao() - .build() - .await - .unwrap(); - assert_eq!(without.sources.len(), 2); - - // Opting in adds exactly one source (env + sqlite + prompt). - let with = ConfigClient::builder("__cmbuilder__with_sqlite") - .without_openbao() - .with_sqlite() - .build() - .await - .unwrap(); - assert_eq!(with.sources.len(), 3); - } - - #[tokio::test] - async fn test_builder_drops_prompt() { - let m = ConfigClient::builder("__cmbuilder__no_prompt") - .without_openbao() - .without_prompt() - .build() - .await - .unwrap(); - // env only (sqlite off by default, openbao + prompt dropped). - assert_eq!(m.sources.len(), 1); - } - - #[tokio::test] - async fn test_builder_all_disabled_yields_empty_chain() { - let m = ConfigClient::builder("__cmbuilder__empty") - .without_env() - .without_openbao() - .without_prompt() - .build() - .await - .unwrap(); - // sqlite was never opted in, the other three dropped → empty. - assert_eq!(m.sources.len(), 0); - - // And confirm get falls through to NotFound cleanly. - let res: Result = m.get().await; - assert!(matches!(res, Err(ConfigError::NotFound { .. }))); - } - - #[tokio::test] - async fn test_builder_methods_chain_freely() { - // Confirms the with_/without_ methods return Self and compose. - let _ = ConfigClient::builder("__cmbuilder__chain") - .without_env() - .without_openbao() - .with_sqlite() - .without_prompt() - .build() - .await - .unwrap(); - } - #[tokio::test] async fn test_config_manager_forwards_standard_class_to_sources_on_set() { let source = Arc::new(MockSource::new()); diff --git a/harmony_config/src/source/env.rs b/harmony_config/src/source/env.rs index 261eab80..e976bbf3 100644 --- a/harmony_config/src/source/env.rs +++ b/harmony_config/src/source/env.rs @@ -43,16 +43,11 @@ impl ConfigSource for EnvSource { source: e, })?; - // SAFETY: Setting environment variables is generally safe in single-threaded contexts. - // In multi-threaded contexts, this could cause races, but is acceptable for this use case - // as config is typically set once at startup. + // Rust 2024 makes `set_var` unsafe (data race if another thread reads + // env concurrently); config is set once at startup, so this is safe. unsafe { std::env::set_var(&env_key, &json_string); } Ok(()) } - - fn should_persist(&self) -> bool { - false - } } diff --git a/harmony_config/src/source/local_file.rs b/harmony_config/src/source/local_file.rs index 8769e0b8..c435cab1 100644 --- a/harmony_config/src/source/local_file.rs +++ b/harmony_config/src/source/local_file.rs @@ -6,13 +6,9 @@ use crate::{ConfigClass, ConfigError, ConfigSource}; /// Local file-backed config source (`/.json`). /// -/// ⚠️ **SECURITY: stores values as cleartext JSON on local disk and -/// ignores the `ConfigClass` hint.** Like [`SqliteSource`](crate::SqliteSource), -/// it must not be used to cache `ConfigClass::Secret` config — a -/// secret written here is plaintext on disk. It is not part of the -/// canonical chain; use it only via an explicit -/// [`ConfigClient::new`](crate::ConfigClient::new) for non-secret -/// config. +/// ⚠️ Cleartext on disk and ignores `ConfigClass`, like +/// [`SqliteSource`](crate::SqliteSource) — non-secret config only, via an +/// explicit [`ConfigClient::new`](crate::ConfigClient::new). pub struct LocalFileSource { base_path: PathBuf, } diff --git a/harmony_config/src/source/prompt.rs b/harmony_config/src/source/prompt.rs index 64815dd6..f25ea13b 100644 --- a/harmony_config/src/source/prompt.rs +++ b/harmony_config/src/source/prompt.rs @@ -5,15 +5,8 @@ use tokio::sync::Mutex; use crate::{Config, ConfigClass, ConfigError, ConfigSource}; -/// Process-wide async mutex guarding interactive prompts. ADR 020 -/// motivates this: `inquire` assumes exclusive terminal ownership, and -/// background tokio tasks that emit log output during a prompt corrupt -/// the terminal state. Holding this lock across the prompt serialises -/// concurrent prompts. -/// -/// The mutex does not silence background log output — that is a logger -/// concern. It only ensures two `harmony_config` prompts do not run at -/// the same time on the same terminal. +// Serialises interactive prompts process-wide: `inquire` assumes exclusive +// terminal ownership, so concurrent prompts would corrupt the terminal. static PROMPT_MUTEX: Mutex<()> = Mutex::const_new(()); pub struct PromptSource; @@ -23,17 +16,9 @@ impl PromptSource { Self } - /// Prompt the user for a `T` while holding the process-wide prompt - /// mutex. Returns the parsed value or a `ConfigError::PromptError` - /// if the user cancels or input parsing fails. - /// - /// `Standard`-class structs flow through `interactive_parse`'s - /// `parse_to_obj` — full nested-struct / enum / Option support. - /// `Secret`-class structs flow through a narrower walker that - /// uses `inquire::Password` for fields tagged `#[config(secret)]`. - /// The walker only supports flat primitive fields (String, - /// integer, bool); a Secret struct with non-primitive fields - /// returns a clear `PromptError`. + /// Prompt for a `T`. `Standard` structs go through `interactive_parse` + /// (full nested support); `Secret` structs go through a flat-field + /// walker that reads `#[config(secret)]` fields via `inquire::Password`. pub async fn prompt_for(&self) -> Result { let _guard = PROMPT_MUTEX.lock().await; let banner = format!( @@ -41,9 +26,8 @@ impl PromptSource { T::KEY, T::CLASS, ); - // inquire renders prompts on stderr, so we match that channel - // and surround the banner with blank lines so it stays - // visually separate from preceding log output. + // inquire renders on stderr; match that channel and pad with blank + // lines so the banner stays separate from preceding log output. eprintln!(); eprintln!("{banner}"); eprintln!(); @@ -80,24 +64,17 @@ impl ConfigSource for PromptSource { ) -> Result<(), ConfigError> { Ok(()) } - - fn should_persist(&self) -> bool { - false - } } -/// Schema-walking prompt for `Secret`-class structs. Renders -/// `inquire::Password` for fields named in `T::SECRET_FIELDS` (when -/// the field is a string); other fields fall through to -/// type-appropriate prompts. Fails fast on non-primitive field -/// types — Secret structs with nested data are not yet supported -/// because the masking story for nested fields isn't designed. +// Walks a Secret struct's schema, reading `T::SECRET_FIELDS` via Password +// and other fields via type-appropriate prompts. Only flat primitive +// fields are supported; anything else returns a clear PromptError. fn prompt_secret_struct() -> Result { let root: RootSchema = schemars::schema_for!(T); let object = root.schema.object.as_deref().ok_or_else(|| { ConfigError::PromptError(format!( - "Secret-class struct `{}` does not have a JSON-object schema; the \ - secret-aware prompt walker only supports plain structs", + "Secret struct `{}` has no JSON-object schema; the walker only \ + supports plain structs", T::KEY )) })?; @@ -126,18 +103,15 @@ fn schema_object<'a>( match schema { Schema::Object(obj) => Ok(obj), Schema::Bool(_) => Err(ConfigError::PromptError(format!( - "Secret-class struct `{struct_name}` field `{field_name}` resolved \ - to a `true`/`false` JSON-schema, which the secret-aware prompt \ - walker can't handle. Use a concrete type." + "Secret struct `{struct_name}` field `{field_name}` resolved to a \ + boolean JSON-schema; use a concrete type." ))), } } -/// Pick the right `inquire` widget based on the JsonSchema for one -/// field. Strings tagged secret use `Password`; everything else uses -/// the type-appropriate text input. Returns `Err` for shapes the -/// walker doesn't support — never silently falls back to an unmasked -/// prompt. +// Picks the inquire widget for one field from its JsonSchema. Secret +// strings use Password; other primitives use type-appropriate input. +// Returns Err for unsupported shapes — never a silent unmasked fallback. fn prompt_field( field_name: &str, schema: &SchemaObject, @@ -145,10 +119,8 @@ fn prompt_field( ) -> Result { let instance_type = schema.instance_type.as_ref().ok_or_else(|| { ConfigError::PromptError(format!( - "Secret-class struct `{}` field `{field_name}` has no `instance_type` \ - in its JSON-schema; the secret-aware prompt walker only handles \ - flat primitive fields (string / integer / bool). Either flatten \ - the struct or open an issue to extend the walker.", + "Secret struct `{}` field `{field_name}` is not a flat primitive \ + (string/integer/number/bool); flatten it or extend the walker.", T::KEY )) })?; @@ -157,10 +129,8 @@ fn prompt_field( SingleOrVec::Single(boxed) => **boxed, SingleOrVec::Vec(_) => { return Err(ConfigError::PromptError(format!( - "Secret-class struct `{}` field `{field_name}` has a multi-type \ - JSON-schema (e.g. nullable). The secret-aware prompt walker \ - only handles single primitive types. Either flatten or open \ - an issue.", + "Secret struct `{}` field `{field_name}` has a multi-type schema \ + (e.g. nullable); the walker only handles single primitives.", T::KEY ))); } @@ -170,11 +140,8 @@ fn prompt_field( match single { InstanceType::String => { if is_secret { - // `PasswordDisplayMode::Masked` echoes `*` per keystroke as - // the operator types, instead of inquire's default - // `Hidden` mode (which displays nothing until the user - // hits enter). Live feedback on a hidden field is the - // usual UX expectation; full-hide is too quiet. + // Masked mode echoes `*` per keystroke (inquire's default + // Hidden mode shows nothing until enter). let raw = Password::new(&label) .with_display_mode(PasswordDisplayMode::Masked) .without_confirmation() @@ -197,10 +164,7 @@ fn prompt_field( } } InstanceType::Integer => { - // `i64` covers the integer types we'll see in Config - // structs (u16 ports, u32 counts, signed counters). The - // resulting JSON number deserialises back into the - // narrower type via serde. + // i64 covers Config integer fields; serde narrows on deserialize. let n = CustomType::::new(&label).prompt().map_err(|e| { ConfigError::PromptError(format!( "Integer prompt for `{}::{field_name}` failed: {e}", @@ -228,11 +192,8 @@ fn prompt_field( Ok(serde_json::Value::Bool(b)) } other => Err(ConfigError::PromptError(format!( - "Secret-class struct `{}` field `{field_name}` has unsupported \ - JSON-schema type `{other:?}`. The secret-aware prompt walker only \ - handles flat primitives (string, integer, number, boolean). Split \ - the nested data into a separate Standard-class struct, or open an \ - issue to extend the walker.", + "Secret struct `{}` field `{field_name}` has unsupported type \ + `{other:?}`; the walker only handles flat primitives.", T::KEY ))), } diff --git a/harmony_config/src/source/sqlite.rs b/harmony_config/src/source/sqlite.rs index 2a2d77ad..eae3c39a 100644 --- a/harmony_config/src/source/sqlite.rs +++ b/harmony_config/src/source/sqlite.rs @@ -5,26 +5,13 @@ use tokio::fs; use crate::{ConfigClass, ConfigError, ConfigSource}; -/// Local SQLite-backed config cache. +/// Local SQLite-backed config cache at `//config.db`. /// -/// ⚠️ **SECURITY: this source stores every value as cleartext JSON in -/// a file on local disk (`//config.db`).** It -/// applies no encryption and ignores the `ConfigClass` hint — a -/// `ConfigClass::Secret` value written here lands in plaintext, -/// readable by anyone with filesystem access. SQLite is therefore -/// **deliberately excluded from the canonical chain** -/// ([`ConfigClient::for_namespace`](crate::ConfigClient::for_namespace) / -/// [`ConfigClient::builder`](crate::ConfigClient::builder)); it is -/// opt-in only via -/// [`ConfigClientBuilder::with_sqlite`](crate::ConfigClientBuilder::with_sqlite). -/// -/// Only enable it when you are certain the config flowing through the -/// manager is **non-secret** (Standard class) and you want an offline -/// cache that survives an OpenBao outage. For secrets, rely on OpenBao -/// — it encrypts at rest and gates access via policy. Per-class -/// at-rest handling (encrypted local cache, or skipping Secret-class -/// writes) is tracked as future work; until then, treat this source -/// as plaintext. +/// ⚠️ Stores values as cleartext JSON and ignores `ConfigClass`, so it must +/// never hold `Secret`-class config — it is excluded from the canonical +/// chain ([`ConfigClient::for_namespace`](crate::ConfigClient::for_namespace)) +/// and only usable via an explicit [`ConfigClient::new`](crate::ConfigClient::new) +/// for non-secret offline caching. pub struct SqliteSource { pool: SqlitePool, } @@ -57,11 +44,8 @@ impl SqliteSource { Ok(Self { pool }) } - /// Open a SQLite source scoped to `namespace`. The database file - /// lives at `//config.db`, isolating state - /// from other harmony binaries on the same machine. Use the same - /// namespace string you pass to `StoreSource::new(...)` so the - /// two persistence layers stay symmetric for one binary. + /// Scope the database to `namespace` so harmony binaries on one machine + /// don't share a `config.db`. Use the same namespace as `StoreSource`. pub async fn for_namespace(namespace: &str) -> Result { let path = crate::default_config_dir() .ok_or_else(|| { @@ -71,23 +55,6 @@ impl SqliteSource { .join("config.db"); Self::open(path).await } - - /// Legacy un-namespaced default at `/config.db`. - /// Every caller using this method shares the same SQLite file - /// and the same single `config` table — two harmony binaries - /// with overlapping `Config::KEY`s will overwrite each other. - /// Prefer [`SqliteSource::for_namespace`]. - #[deprecated( - note = "Use SqliteSource::for_namespace(...) so multiple harmony binaries don't share one config.db" - )] - pub async fn default() -> Result { - let path = crate::default_config_dir() - .ok_or_else(|| { - ConfigError::SqliteError("Could not determine default config directory".into()) - })? - .join("config.db"); - Self::open(path).await - } } #[async_trait] diff --git a/harmony_config/src/source/store.rs b/harmony_config/src/source/store.rs index de2bb1f9..a4ba4e48 100644 --- a/harmony_config/src/source/store.rs +++ b/harmony_config/src/source/store.rs @@ -16,18 +16,14 @@ impl StoreSource { } #[async_trait] +// TODO(ADR 020-1): forward `_class` to `SecretStore` once that trait accepts +// a class hint; kept here so the ConfigSource boundary already matches ADR 020. impl ConfigSource for StoreSource { async fn get( &self, _class: ConfigClass, key: &str, ) -> Result, ConfigError> { - // TODO(ADR 020-1): propagate `_class` to `SecretStore` when the - // path-taxonomy work lands. Today the SecretStore trait does not - // accept a class hint, so a per-class routing change here would - // need a corresponding signature change in harmony_secret. Keep - // the parameter in our trait so the ConfigSource boundary - // matches ADR 020 even before the storage backend can act on it. match self.store.get_raw(&self.namespace, key).await { Ok(bytes) => { let value: serde_json::Value = @@ -55,8 +51,6 @@ impl ConfigSource for StoreSource { key: &str, value: &serde_json::Value, ) -> Result<(), ConfigError> { - // TODO(ADR 020-1): see `get` above — `_class` is accepted at the - // ConfigSource boundary but not yet forwarded to SecretStore. let bytes = serde_json::to_vec(value).map_err(|e| ConfigError::Serialization { key: key.to_string(), source: e, diff --git a/harmony_config_derive/src/lib.rs b/harmony_config_derive/src/lib.rs index dfc5aef7..ecdd4958 100644 --- a/harmony_config_derive/src/lib.rs +++ b/harmony_config_derive/src/lib.rs @@ -3,18 +3,9 @@ use proc_macro_crate::{FoundCrate, crate_name}; use quote::quote; use syn::{Attribute, Data, DeriveInput, Fields, Ident, parse_macro_input}; -/// Derives `harmony_config::Config`, emitting `KEY` (the struct name), -/// `CLASS`, and `SECRET_FIELDS`. -/// -/// Mark a field — or the whole struct — `#[config(secret)]` to elevate -/// it to `ConfigClass::Secret` and record the tagged field names in -/// `SECRET_FIELDS` (used for output masking and password-style prompts). -/// -/// **Constraint:** do not combine `#[config(secret)]` with -/// `#[serde(rename = "…")]` / `#[serde(rename_all = "…")]` on the same -/// field. Masking and secret prompts match the *serialized* name, but -/// this macro records the Rust ident, so a renamed secret would leak in -/// cleartext. See `Config::SECRET_FIELDS`. +/// Derives `Config`, emitting `KEY`, `CLASS`, and `SECRET_FIELDS`. Mark a +/// field — or the whole struct — `#[config(secret)]` to elevate it to +/// `ConfigClass::Secret`. See `Config::SECRET_FIELDS` for the no-rename rule. #[proc_macro_derive(Config, attributes(config))] pub fn derive_config(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); @@ -22,11 +13,9 @@ pub fn derive_config(input: TokenStream) -> TokenStream { let key = struct_ident.to_string(); - // Always emit a fully qualified path. `proc-macro-crate` returns - // `FoundCrate::Itself` for examples in the same package, but examples - // are separate binary crates — their `crate::` root does not contain - // harmony_config's items. Using `::harmony_config::` (or the renamed - // import) works everywhere. + // Always emit a fully-qualified path: examples are separate binary + // crates whose `crate::` root lacks harmony_config's items, so the + // `extern crate self` alias in the lib makes `::harmony_config` resolve. let config_crate_path = match crate_name("harmony_config") { Ok(FoundCrate::Itself) => quote!(::harmony_config), Ok(FoundCrate::Name(name)) => { @@ -42,9 +31,7 @@ pub fn derive_config(input: TokenStream) -> TokenStream { let struct_level_secret = has_secret_attr(&input.attrs); - // Collect the names of every field tagged `#[config(secret)]`. - // When the WHOLE struct is `#[config(secret)]`, every named field - // is considered secret. Tuple / unit structs contribute no names. + // A struct-level `#[config(secret)]` makes every named field secret. let mut secret_field_names: Vec = Vec::new(); if let Data::Struct(ref data) = input.data && let Fields::Named(named) = &data.fields @@ -68,10 +55,7 @@ pub fn derive_config(input: TokenStream) -> TokenStream { }; let secret_fields_const = if secret_field_names.is_empty() { - // Standard structs and structs without named secret fields - // get the trait default (`&[]`); omitting the const keeps - // the generated impl smaller. - quote!() + quote!() // fall back to the trait default `&[]` } else { let names = secret_field_names.iter().map(|n| quote!(#n)); quote! { @@ -90,15 +74,11 @@ pub fn derive_config(input: TokenStream) -> TokenStream { TokenStream::from(expanded) } -/// Returns true if `attrs` carries `#[config(secret)]`. fn has_secret_attr(attrs: &[Attribute]) -> bool { attrs.iter().any(|attr| { if !attr.path().is_ident("config") { return false; } - // `#[config(secret)]` parses as a list-style meta with a single - // `secret` ident. Any other form (e.g. unknown keys) is ignored - // for now; future attributes will be added explicitly. let mut found = false; let _ = attr.parse_nested_meta(|meta| { if meta.path.is_ident("secret") { -- 2.39.5