From 4732d6e7865e3a2b2263ad920df3a0b2b9630afc Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 14 Aug 2026 19:46:23 -0500 Subject: [PATCH] fix(cli): let the CLI take its gRPC token from the environment nunciod mints the gRPC bearer token into the OS keyring, so the keychain item's ACL trusts nunciod. nuncio-cli is a different binary, so reading that item raises an interactive consent prompt. Nothing can answer that prompt from CI, ssh, or a background shell, and the blocking keyring read never returns -- the CLI hung with no output and no error. Add an opt-in NUNCIO_GRPC_TOKEN override: when set and non-empty the CLI uses it verbatim and never touches the keyring. The keyring remains the default and is otherwise unchanged. The keyring read itself can no longer stall forever either. It now runs on a detached OS thread behind a bounded timeout that reports what is blocking and how to bypass it. A detached thread rather than spawn_blocking, because Tokio joins its blocking pool during runtime shutdown, which would just move the hang to process exit. Collapse the eight copies of the token-resolution block onto that one resolver so no call site can drift. Refs #419 --- crates/nuncio-cli/src/lib.rs | 2 +- crates/nuncio-cli/src/runner.rs | 273 ++++++++++++++---- .../nunciod/tests/headless_cli_token_test.rs | 249 ++++++++++++++++ docs/PATH-TO-MANUAL-USE.md | 28 ++ 4 files changed, 501 insertions(+), 51 deletions(-) create mode 100644 crates/nunciod/tests/headless_cli_token_test.rs diff --git a/crates/nuncio-cli/src/lib.rs b/crates/nuncio-cli/src/lib.rs index 3b35cac..ab4b8a6 100644 --- a/crates/nuncio-cli/src/lib.rs +++ b/crates/nuncio-cli/src/lib.rs @@ -12,4 +12,4 @@ pub use args::{ SystemSubcommand, }; pub use output::JsonResponse; -pub use runner::HeadlessRunner; +pub use runner::{HeadlessRunner, GRPC_TOKEN_ENV}; diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index 1d8ae7e..5bb57d5 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -5,6 +5,7 @@ use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; use nuncio_store::{DatabaseEngine, DatabaseError}; use serde_json::json; use std::sync::Arc; +use std::time::Duration; use thiserror::Error; use crate::args::{ @@ -14,6 +15,40 @@ use crate::args::{ use crate::output::{format_json, format_json_error, format_json_error_with_info}; +/// Environment variable that supplies the gRPC bearer token directly, bypassing +/// the OS keyring entirely. +/// +/// The keyring remains the default and only this client-side lookup honours the +/// override: `nunciod` always mints and owns the authoritative token, so the +/// daemon must never accept one from its environment. +/// +/// The override exists because the keyring item's ACL is bound to the process +/// that created it (`nunciod`). A *different* binary reading the same item — the +/// CLI — makes the platform keychain ask the logged-in user for consent, which +/// nothing can answer without an interactive desktop session. +pub const GRPC_TOKEN_ENV: &str = "NUNCIO_GRPC_TOKEN"; + +/// Upper bound on how long a keyring read may block before the CLI gives up and +/// reports an actionable error. Generous enough that a human can answer a +/// keychain consent prompt, short enough that an unattended run always +/// terminates instead of stalling forever. +const KEYRING_READ_TIMEOUT: Duration = Duration::from_secs(20); + +/// Normalizes a raw environment value into a usable token override. +/// +/// Surrounding whitespace is stripped (shell heredocs and `env` files routinely +/// add a trailing newline) and an empty value is treated as "not set", so +/// `NUNCIO_GRPC_TOKEN=` cannot silently authenticate with an empty token. +fn normalize_token_override(raw: Option) -> Option { + raw.map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) +} + +/// Reads the gRPC bearer token override from the process environment. +fn grpc_token_from_env() -> Option { + normalize_token_override(std::env::var(GRPC_TOKEN_ENV).ok()) +} + /// Parses a `--imap-mode`/`--smtp-mode` CLI string into `nuncio_core::TlsMode`. /// Rejects anything else rather than silently defaulting, since silently /// falling back to e.g. `Plain` for a typo'd mode would be a serious @@ -326,7 +361,9 @@ pub enum RunnerError { /// `nuncio.v1.Accounts` APIs, authenticated by a bearer token read from an /// injected [`SecretManager`] — production code uses /// [`SecretManager::production`] (the real OS keyring), while tests inject -/// [`SecretManager::mock`] so no test ever touches the real vault. +/// [`SecretManager::mock`] so no test ever touches the real vault. Setting +/// [`GRPC_TOKEN_ENV`] overrides that lookup entirely, which is how the CLI is +/// driven from a non-interactive session. /// /// `account add` persists through the daemon so the account (and its /// password, stored ONLY in the daemon's OS keyring vault) survives past @@ -337,6 +374,10 @@ pub struct HeadlessRunner { db: DatabaseEngine, secrets: Arc, grpc_addr: String, + // When present, the bearer token to send instead of consulting `secrets`. + // Resolved once at construction so a single CLI invocation cannot change + // which credential it authenticates with halfway through. + token_override: Option, // Kept only to hold the ephemeral database's backing directory open for // the runner's lifetime: dropping it would unlink the directory out from // under `db` while the pool may still need to open new connections. @@ -347,10 +388,13 @@ impl HeadlessRunner { /// Initialize a new `HeadlessRunner` with an ephemeral database, the /// real OS keyring vault ([`SecretManager::production`]), and the gRPC /// daemon address resolved from [`nuncio_proto::grpc_addr_from_env`]. + /// [`GRPC_TOKEN_ENV`], when set, supplies the bearer token instead of the + /// keyring. pub async fn ephemeral() -> Result { - Self::ephemeral_with( + Self::ephemeral_with_token_override( Arc::new(SecretManager::production()), nuncio_proto::grpc_addr_from_env(), + grpc_token_from_env(), ) .await } @@ -362,9 +406,28 @@ impl HeadlessRunner { /// `system status`: pass a [`SecretManager::mock`]-backed instance /// (never the real OS keyring) and the address of a test-local gRPC /// server. + /// The ambient [`GRPC_TOKEN_ENV`] override is deliberately NOT consulted + /// here, so a variable that happens to be exported in a developer's shell + /// can never change what an injected-vault test authenticates with; tests + /// that want the override pass it explicitly to + /// [`Self::ephemeral_with_token_override`]. pub async fn ephemeral_with( secrets: Arc, grpc_addr: String, + ) -> Result { + Self::ephemeral_with_token_override(secrets, grpc_addr, None).await + } + + /// Initialize a new `HeadlessRunner` with an ephemeral database, an + /// explicit secret vault + gRPC daemon address, and an explicit bearer + /// token override. + /// + /// `token_override`, when `Some`, is used verbatim as the `authorization: + /// Bearer` credential and the vault is never consulted for it. + pub async fn ephemeral_with_token_override( + secrets: Arc, + grpc_addr: String, + token_override: Option, ) -> Result { let (db, db_dir) = DatabaseEngine::connect_ephemeral() .await @@ -376,9 +439,59 @@ impl HeadlessRunner { secrets, _db_dir: db_dir, grpc_addr, + token_override: normalize_token_override(token_override), }) } + /// Resolves the gRPC bearer token, preferring an explicit override over the + /// OS keyring. + /// + /// The keyring read runs on a detached OS thread behind a bounded timeout + /// rather than inline. On macOS a keychain item whose ACL does not trust + /// this binary makes the read block on a GUI consent prompt that never + /// arrives in a headless session, and that call cannot be cancelled — so + /// the timeout must be able to abandon the thread. A plain + /// `std::thread::spawn` is used instead of `spawn_blocking` precisely + /// because Tokio joins its blocking pool during runtime shutdown, which + /// would reintroduce the hang at process exit; a detached thread is reaped + /// when the process ends. + /// + /// The returned error never carries token material — only the vault's own + /// failure description, which is keyed by account name, not secret value. + async fn resolve_grpc_token(&self) -> Result { + if let Some(token) = &self.token_override { + return Ok(token.clone()); + } + + let secrets = Arc::clone(&self.secrets); + let (tx, rx) = tokio::sync::oneshot::channel(); + std::thread::spawn(move || { + let resolved = secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .map(hex::encode) + .map_err(|e| e.to_string()); + // A closed receiver means the timeout already fired; nothing to do. + let _ = tx.send(resolved); + }); + + match tokio::time::timeout(KEYRING_READ_TIMEOUT, rx).await { + Ok(Ok(Ok(token))) => Ok(token), + Ok(Ok(Err(e))) => Err(format!("failed to read gRPC bearer token from vault: {e}")), + Ok(Err(_)) => { + Err("gRPC bearer token lookup ended without producing a result".to_string()) + } + Err(_) => Err(format!( + "timed out after {}s reading the gRPC bearer token from the OS keyring: the \ + keyring is most likely waiting on an interactive access-consent prompt that \ + cannot be answered in this session (the token belongs to nunciod, so another \ + binary reading it needs consent). Run this command from a logged-in desktop \ + session, or set {GRPC_TOKEN_ENV} to the daemon's bearer token to bypass the \ + keyring entirely", + KEYRING_READ_TIMEOUT.as_secs() + )), + } + } + /// Access the underlying `EventBus`. #[allow(dead_code)] pub fn event_bus(&self) -> &EventBus { @@ -1412,17 +1525,13 @@ impl HeadlessRunner { } } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Export` service at `self.grpc_addr`, shared by /// [`Self::handle_mail_export`]. async fn connect_export_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_export(&self.grpc_addr, &token) .await @@ -1524,17 +1633,13 @@ impl HeadlessRunner { } } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Audit` service at `self.grpc_addr`, shared by /// [`Self::handle_audit_list`] / [`Self::handle_audit_verify`]. async fn connect_audit_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_audit(&self.grpc_addr, &token) .await @@ -1994,17 +2099,13 @@ impl HeadlessRunner { } } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Filters` service at `self.grpc_addr`, shared by every /// `filter` handler above. async fn connect_filters_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_filters(&self.grpc_addr, &token) .await @@ -2459,51 +2560,39 @@ impl HeadlessRunner { } } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Accounts` service at `self.grpc_addr`, shared by /// [`Self::handle_add_account`] and [`Self::handle_accounts_list`]. async fn connect_accounts_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_accounts(&self.grpc_addr, &token) .await .map_err(|e| format!("nunciod daemon unreachable at {}: {e}", self.grpc_addr)) } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Mail` service at `self.grpc_addr`, shared by every /// `mail`/`folder` read-path handler above. async fn connect_mail_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_mail(&self.grpc_addr, &token) .await .map_err(|e| format!("nunciod daemon unreachable at {}: {e}", self.grpc_addr)) } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Calendar` service at `self.grpc_addr`, shared by /// [`Self::handle_cal_list`] and [`Self::handle_cal_sync`]. async fn connect_calendar_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_calendar(&self.grpc_addr, &token) .await @@ -2612,17 +2701,13 @@ impl HeadlessRunner { } } - /// Resolves the gRPC bearer token from the injected vault and dials the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]) and dials the /// `nuncio.v1.Contacts` service at `self.grpc_addr`, shared by every /// `contact` handler above. async fn connect_contacts_client( &self, ) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; nuncio_proto::client::connect_contacts(&self.grpc_addr, &token) .await @@ -2991,16 +3076,12 @@ impl HeadlessRunner { } } - /// Reads the gRPC bearer token from the injected vault, connects to the + /// Resolves the gRPC bearer token (see [`Self::resolve_grpc_token`]), connects to the /// daemon over gRPC, and calls `GetStatus`. Returns the full live health /// response on success, or a human-readable error string describing /// exactly what failed (vault, connection, or the RPC itself). async fn query_daemon_status(&self) -> Result { - let token_bytes = self - .secrets - .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) - .map_err(|e| format!("failed to read gRPC bearer token from vault: {e}"))?; - let token = hex::encode(token_bytes); + let token = self.resolve_grpc_token().await?; let mut client = nuncio_proto::client::connect_system(&self.grpc_addr, &token) .await @@ -3039,6 +3120,98 @@ fn sync_state_label(state: i32) -> &'static str { mod tests { use super::*; + #[test] + fn normalize_token_override_trims_and_rejects_blank_values() { + assert_eq!( + normalize_token_override(Some(" deadbeef\n".to_string())), + Some("deadbeef".to_string()) + ); + assert_eq!(normalize_token_override(Some(String::new())), None); + assert_eq!(normalize_token_override(Some(" ".to_string())), None); + assert_eq!(normalize_token_override(None), None); + } + + /// The env override must be picked up when set and must leave the runner on + /// the keyring path when it is unset or blank. + #[test] + fn grpc_token_from_env_reads_the_override_only_when_it_is_non_empty() { + std::env::remove_var(GRPC_TOKEN_ENV); + assert_eq!(grpc_token_from_env(), None); + + std::env::set_var(GRPC_TOKEN_ENV, ""); + assert_eq!(grpc_token_from_env(), None); + + std::env::set_var(GRPC_TOKEN_ENV, " c0ffee "); + assert_eq!(grpc_token_from_env(), Some("c0ffee".to_string())); + + std::env::remove_var(GRPC_TOKEN_ENV); + } + + /// An injected override must be used verbatim and must never reach the + /// vault -- proven here by giving the runner a vault whose token is + /// necessarily different from the override. + #[tokio::test] + async fn resolve_grpc_token_prefers_the_override_over_the_vault() { + let secrets = Arc::new(SecretManager::mock()); + let vault_token = hex::encode( + secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints a token"), + ); + + let runner = HeadlessRunner::ephemeral_with_token_override( + Arc::clone(&secrets), + "127.0.0.1:1".to_string(), + Some(" override-token ".to_string()), + ) + .await + .expect("runner init"); + + let resolved = runner + .resolve_grpc_token() + .await + .expect("override resolves without touching the vault"); + assert_eq!(resolved, "override-token"); + assert_ne!(resolved, vault_token); + } + + /// With no override the vault remains the source of the token. + #[tokio::test] + async fn resolve_grpc_token_falls_back_to_the_vault_without_an_override() { + let secrets = Arc::new(SecretManager::mock()); + let vault_token = hex::encode( + secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints a token"), + ); + + let runner = + HeadlessRunner::ephemeral_with(Arc::clone(&secrets), "127.0.0.1:1".to_string()) + .await + .expect("runner init"); + + assert_eq!( + runner + .resolve_grpc_token() + .await + .expect("vault resolves the token"), + vault_token + ); + } + + /// `ephemeral_with` must ignore an ambient override so an exported variable + /// in a developer's shell cannot silently change what tests authenticate + /// with. + #[tokio::test] + async fn ephemeral_with_ignores_an_ambient_env_override() { + let secrets = Arc::new(SecretManager::mock()); + let runner = + HeadlessRunner::ephemeral_with(Arc::clone(&secrets), "127.0.0.1:1".to_string()) + .await + .expect("runner init"); + assert_eq!(runner.token_override, None); + } + #[test] fn parse_tls_mode_accepts_all_valid_modes_and_rejects_garbage() { assert_eq!( diff --git a/crates/nunciod/tests/headless_cli_token_test.rs b/crates/nunciod/tests/headless_cli_token_test.rs new file mode 100644 index 0000000..d1b3e6e --- /dev/null +++ b/crates/nunciod/tests/headless_cli_token_test.rs @@ -0,0 +1,249 @@ +//! Headless bearer-token resolution for the reference CLI. +//! +//! `nunciod` mints the gRPC bearer token into the OS keyring, so the keyring +//! item's access control list trusts the *daemon* binary. Any other binary +//! reading that item -- the CLI -- is a different program, and platform +//! keychains answer that by asking the logged-in user for consent. Nothing can +//! answer that prompt from CI, an ssh session, or a background shell, so the +//! read blocks forever. `NUNCIO_GRPC_TOKEN` is the explicit, opt-in escape +//! hatch; the keyring stays the default. +//! +//! These tests boot the real daemon gRPC server on an ephemeral loopback port +//! (mock vault, no real keychain, no live network) and drive the real +//! `HeadlessRunner` against it to prove: +//! +//! - an override authenticates and the vault is never consulted for it; +//! - with no override the keyring path is unchanged and still authenticates; +//! - a wrong override fails honestly and the token never reaches the output +//! or the telemetry. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::sync::{Arc, Mutex, OnceLock}; + +use nuncio_cli::{Commands, HeadlessRunner, SystemSubcommand}; +use nuncio_core::EventBus; +use nuncio_filter::FilterEngine; +use nuncio_store::db::DatabaseEngine; +use nuncio_store::vault::{SecretManager, GRPC_TOKEN_ACCOUNT}; +use tokio::net::TcpListener; +use tracing::field::{Field, Visit}; +use tracing::span::Attributes; +use tracing::{Event, Id, Subscriber}; +use tracing_subscriber::layer::Context as LayerContext; +use tracing_subscriber::prelude::*; +use tracing_subscriber::registry::LookupSpan; + +/// A bearer token that is unmistakable if it ever leaks into output or logs. +const SENTINEL_TOKEN: &str = "SENTINELheadlessCliBearerTokenVvWw001"; + +/// Boots the real daemon gRPC server on an ephemeral loopback port, +/// authenticated by `token`. Returns the address; the temp dir is kept alive +/// by the returned guard. +async fn boot_daemon(secrets: Arc, token: String) -> std::net::SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral loopback port"); + let addr = listener.local_addr().expect("listener has local addr"); + let (db, db_dir) = DatabaseEngine::connect_ephemeral() + .await + .expect("connect ephemeral test db"); + let filter_engine = Arc::new(FilterEngine::new(Vec::new()).expect("empty rule set")); + tokio::spawn(async move { + // The temp dir must outlive the server, so it is moved into the task. + let _db_dir = db_dir; + let _ = nunciod::grpc::serve_on_listener( + listener, + Arc::new(EventBus::new()), + Arc::new(db), + filter_engine, + secrets, + token, + ) + .await; + }); + addr +} + +/// Runs `system status` through the reference CLI against `addr`. +async fn run_status(runner: &HeadlessRunner) -> String { + runner + .execute_command( + &Commands::System { + action: SystemSubcommand::Status, + }, + true, + ) + .await +} + +/// An explicitly supplied token must authenticate on its own, with the runner's +/// vault holding a *different* token -- which is only possible if the override +/// short-circuits the keyring entirely. +#[tokio::test] +async fn env_override_authenticates_without_consulting_the_vault() { + let daemon_secrets = Arc::new(SecretManager::mock()); + let token = hex::encode( + daemon_secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints gRPC bearer token"), + ); + let addr = boot_daemon(Arc::clone(&daemon_secrets), token.clone()).await; + + // A vault the CLI could only fail with: an independent mock mints an + // unrelated token. + let unrelated_vault = Arc::new(SecretManager::mock()); + let unrelated_token = hex::encode( + unrelated_vault + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints gRPC bearer token"), + ); + assert_ne!(unrelated_token, token); + + let runner = HeadlessRunner::ephemeral_with_token_override( + unrelated_vault, + addr.to_string(), + Some(token), + ) + .await + .expect("cli headless runner initializes"); + + let out = run_status(&runner).await; + assert!( + out.contains(r#""status":"ok""#), + "override should authenticate, got: {out}" + ); +} + +/// With no override the CLI must behave exactly as before: read the token from +/// the injected vault and authenticate with it. +#[tokio::test] +async fn keyring_path_is_unchanged_when_no_override_is_set() { + let secrets = Arc::new(SecretManager::mock()); + let token = hex::encode( + secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints gRPC bearer token"), + ); + let addr = boot_daemon(Arc::clone(&secrets), token).await; + + let runner = HeadlessRunner::ephemeral_with(secrets, addr.to_string()) + .await + .expect("cli headless runner initializes"); + + let out = run_status(&runner).await; + assert!( + out.contains(r#""status":"ok""#), + "vault-resolved token should authenticate, got: {out}" + ); +} + +// ---- Telemetry capture: the token may never appear in logs. ---- + +#[derive(Default)] +struct Sink(Mutex>); + +impl Sink { + fn push_all(&self, values: Vec) { + if let Ok(mut guard) = self.0.lock() { + guard.extend(values); + } + } + + fn snapshot(&self) -> Vec { + self.0.lock().map(|g| g.clone()).unwrap_or_default() + } +} + +struct FieldVisitor(Vec); + +impl Visit for FieldVisitor { + fn record_str(&mut self, field: &Field, value: &str) { + self.0.push(format!("{}={}", field.name(), value)); + } + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + self.0.push(format!("{}={:?}", field.name(), value)); + } +} + +struct CaptureLayer(Arc); + +impl tracing_subscriber::Layer for CaptureLayer +where + S: Subscriber + for<'a> LookupSpan<'a>, +{ + fn on_new_span(&self, attrs: &Attributes<'_>, _id: &Id, _ctx: LayerContext<'_, S>) { + let mut visitor = FieldVisitor(vec![format!("span={}", attrs.metadata().name())]); + attrs.record(&mut visitor); + self.0.push_all(visitor.0); + } + + fn on_event(&self, event: &Event<'_>, _ctx: LayerContext<'_, S>) { + let mut visitor = FieldVisitor(vec![format!("event={}", event.metadata().target())]); + event.record(&mut visitor); + self.0.push_all(visitor.0); + } +} + +/// One process-wide capturing subscriber, installed once: a global subscriber +/// can only be set once per test binary and every test here shares it. +fn capture_sink() -> Arc { + static SINK: OnceLock> = OnceLock::new(); + Arc::clone(SINK.get_or_init(|| { + let sink = Arc::new(Sink::default()); + let subscriber = tracing_subscriber::registry().with(CaptureLayer(Arc::clone(&sink))); + let _ = tracing::subscriber::set_global_default(subscriber); + sink + })) +} + +/// A wrong override must fail honestly rather than hang or fabricate a status, +/// and the token must never reach the command output or the telemetry. +#[tokio::test] +async fn rejected_override_never_echoes_the_token_to_output_or_logs() { + let sink = capture_sink(); + + let daemon_secrets = Arc::new(SecretManager::mock()); + let token = hex::encode( + daemon_secrets + .get_or_create_key_bytes(GRPC_TOKEN_ACCOUNT, 32) + .expect("mock vault mints gRPC bearer token"), + ); + let addr = boot_daemon(Arc::clone(&daemon_secrets), token).await; + + let runner = HeadlessRunner::ephemeral_with_token_override( + Arc::new(SecretManager::mock()), + addr.to_string(), + Some(SENTINEL_TOKEN.to_string()), + ) + .await + .expect("cli headless runner initializes"); + + let out = run_status(&runner).await; + assert!( + out.contains(r#""status":"error""#), + "a wrong token must be rejected, got: {out}" + ); + assert!( + !out.contains(SENTINEL_TOKEN), + "the bearer token must never be echoed in command output: {out}" + ); + + // Proves the capture is live, so the absence assertion below cannot pass + // by capturing nothing at all. + tracing::info!(marker = "headless-cli-token-capture-live", "capture probe"); + + let captured = sink.snapshot(); + assert!( + captured + .iter() + .any(|line| line.contains("headless-cli-token-capture-live")), + "the capturing subscriber must be recording telemetry" + ); + for line in &captured { + assert!( + !line.contains(SENTINEL_TOKEN), + "the bearer token leaked into telemetry: {line}" + ); + } +} diff --git a/docs/PATH-TO-MANUAL-USE.md b/docs/PATH-TO-MANUAL-USE.md index 072f535..fb2c599 100644 --- a/docs/PATH-TO-MANUAL-USE.md +++ b/docs/PATH-TO-MANUAL-USE.md @@ -51,6 +51,34 @@ cargo run -p nunciod # gRPC on 127.0.0.1:9420 cargo run -p nuncio-cli -- system status ``` +### Driving the CLI headlessly + +`nunciod` mints the gRPC bearer token into the OS keyring, so the keychain +item's ACL trusts **`nunciod`**. `nuncio-cli` is a different binary, so reading +the same item makes macOS ask the logged-in user for consent (*"nuncio-cli +wants to access key … in your keychain"*). Interactively you click Allow. In CI, +over ssh, or in a background shell there is nobody to click it — the read +blocks, and before this was fixed the CLI hung with no output at all. + +Set `NUNCIO_GRPC_TOKEN` to bypass the keyring for the CLI only: + +```bash +NUNCIO_GRPC_TOKEN= cargo run -p nuncio-cli -- system status +``` + +When it is set and non-empty the CLI uses it verbatim and never touches the +keyring. When it is unset, behaviour is unchanged: the token still comes from +the keyring, now behind a bounded timeout that reports an actionable error +instead of stalling forever. The token is the daemon's hex-encoded +`grpc-bearer-token` keychain item; read it once from a session that can answer +the prompt (`security find-generic-password -s mx.nuncio.vault -a +grpc-bearer-token -w`) and pass it through your own secret store. It is a +credential: never commit it, never log it, never echo it into a shell history +file. + +The daemon itself is unaffected — it *creates* the item rather than reading one +another binary owns, so its own startup raises no consent prompt. + Schema migrations were removed: databases are **created, never migrated**. A store from before that change errors on read rather than upgrading. Deleting it is correct — accounts and credentials live in the OS keyring and survive; cached -- 2.54.0