diff --git a/Cargo.lock b/Cargo.lock index 0b9e8577ec..82b255abf1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,6 +896,7 @@ dependencies = [ "dirs", "hex", "infer", + "nix 0.31.3", "nostr", "rand 0.10.1", "reqwest 0.13.4", @@ -907,6 +908,7 @@ dependencies = [ "tokio", "url", "uuid", + "zeroize", ] [[package]] diff --git a/crates/buzz-cli/Cargo.toml b/crates/buzz-cli/Cargo.toml index a12260b526..3a62949480 100644 --- a/crates/buzz-cli/Cargo.toml +++ b/crates/buzz-cli/Cargo.toml @@ -79,6 +79,17 @@ buzz-ws-client = { path = "../buzz-ws-client" } # Random number generation — full jitter for exponential backoff in with_retry rand = { workspace = true } +# Zeroizes the private key string in memory on drop — defense in depth for +# --private-key / --private-key-fd secret handling +zeroize = "1" + +[target.'cfg(unix)'.dependencies] +# Closes the raw inherited fd after reading `--private-key-fd` — opening +# `/dev/fd/` duplicates the descriptor, so the original must be closed +# explicitly or it leaks for the process lifetime. `unistd::close` is +# unconditionally available, so no extra feature flags are needed. +nix = { version = "0.31", default-features = false } + [dev-dependencies] # Scratch files for channel-templates.json fixtures in tests tempfile = "3" diff --git a/crates/buzz-cli/README.md b/crates/buzz-cli/README.md index a8c668cf06..5b97d927da 100644 --- a/crates/buzz-cli/README.md +++ b/crates/buzz-cli/README.md @@ -20,6 +20,13 @@ export BUZZ_PRIVATE_KEY="nsec1..." buzz channels list ``` +For handoff scenarios where the key shouldn't appear in argv, shell history, +or the environment (e.g. a parent process/harness spawning `buzz`), pass +`--private-key-fd ` instead: the CLI reads the key from the inherited +file descriptor `/dev/fd/` (fd must be >= 3; Linux/macOS only). +`--private-key-fd` and `--private-key`/`BUZZ_PRIVATE_KEY` are mutually +exclusive. + ## Usage All output is JSON on stdout. Errors are JSON on stderr. Exit codes: 0=ok, 1=user error, 2=network, 3=auth, 4=other, 5=write conflict. @@ -156,6 +163,7 @@ stored rules in `validation_error` so an owner can remove and repair them. | `upload` | `file` | Upload a file to the Blossom store | | `pack` | `validate` | Validate a persona pack (local, no relay) | | | `inspect` | Inspect a persona pack (local, no relay) | +| `capabilities` | | Report security capabilities as versioned JSON (local, no relay, no private key) | | `mem` | `ls` | List non-tombstoned memories | | | `get` | Print memory value to stdout | | | `hash` | Print SHA-256 hex of memory value | diff --git a/crates/buzz-cli/TESTING.md b/crates/buzz-cli/TESTING.md index 4b7257aba7..d3ce1c0e81 100644 --- a/crates/buzz-cli/TESTING.md +++ b/crates/buzz-cli/TESTING.md @@ -502,7 +502,7 @@ buzz users set-profile 2>&1; echo "exit: $?" # Exit 3: No auth configured env -u BUZZ_PRIVATE_KEY \ cargo run -p buzz-cli -- channels list 2>&1; echo "exit: $?" -# stderr: {"error":"auth_error","message":"auth error: BUZZ_PRIVATE_KEY is required (use --private-key or set env var)"} +# stderr: {"error":"auth_error","message":"auth error: BUZZ_PRIVATE_KEY is required (use --private-key, --private-key-fd, or set env var)"} # exit: 3 # Not-found returns null, not an error (exit 0) @@ -522,10 +522,20 @@ Test authentication. BUZZ_PRIVATE_KEY="nsec1..." buzz channels list | jq . # Should succeed +# Private key via fd (--private-key-fd) +exec 3< <(printf '%s' "nsec1...") +buzz --private-key-fd 3 channels list | jq . +exec 3<&- +# Should succeed + +# --private-key and --private-key-fd together → clap usage error +buzz --private-key "nsec1..." --private-key-fd 3 channels list 2>&1; echo "exit: $?" +# exit: 1 (clap conflicts_with error) + # No auth → exit 3 env -u BUZZ_PRIVATE_KEY \ cargo run -p buzz-cli -- channels list 2>&1; echo "exit: $?" -# stderr: {"error":"auth_error","message":"auth error: BUZZ_PRIVATE_KEY is required (use --private-key or set env var)"} +# stderr: {"error":"auth_error","message":"auth error: BUZZ_PRIVATE_KEY is required (use --private-key, --private-key-fd, or set env var)"} # exit: 3 ``` @@ -606,3 +616,4 @@ buzz channels delete --channel "$FORUM_ID" | jq . | 59 | `notes get` | ☐ | By name, by naddr, --content-only, cross-author, ambiguous → exit 1 | | 60 | `notes ls` | ☐ | Own, --author all, --tag, --limit | | 61 | `notes rm` | ☐ | Delete→get 404, double-delete idempotent, missing slug → NotFound | +| 62 | `capabilities` | ☐ | Local, no relay, no private key; versioned JSON security-capability report | diff --git a/crates/buzz-cli/src/commands/capabilities.rs b/crates/buzz-cli/src/commands/capabilities.rs new file mode 100644 index 0000000000..543d99e99c --- /dev/null +++ b/crates/buzz-cli/src/commands/capabilities.rs @@ -0,0 +1,175 @@ +//! `buzz capabilities` — machine-readable security-contract report. +//! +//! This command runs entirely locally: it makes no relay connection, reads no +//! private key material, and does not inspect `BUZZ_PRIVATE_KEY`, +//! `BUZZ_AUTH_TAG`, or any other environment variable. It exists so that +//! downstream automated verifiers (e.g. a "doctor" tool checking that a given +//! `buzz` build actually implements the `--private-key-fd` security +//! properties) have something more reliable to check than grepping +//! `--help` text. +//! +//! The emitted JSON is a **stable, versioned attestation contract** for +//! downstream tooling: +//! +//! - `schema_version` identifies the *shape* of the object. Bump it (and +//! never repurpose or remove an existing field's meaning) if the shape of +//! this JSON changes — additive-only evolution should still bump it so +//! strict downstream parsers can detect the change. +//! - `capability_revision` identifies the specific implementation revision of +//! the `--private-key-fd` feature being attested to +//! (`secure_private_key_fd_v1`). If the underlying security properties +//! change (e.g. a guarantee is weakened, removed, or a new one is added), +//! mint a new revision string rather than silently redefining what the old +//! one means. + +use serde::Serialize; + +use crate::error::CliError; +use crate::{MAX_PRIVATE_KEY_FD, MIN_PRIVATE_KEY_FD, PRIVATE_KEY_FD_MAX_LEN}; + +/// `schema_version` of the JSON object emitted by [`cmd_show`]. +const SCHEMA_VERSION: u32 = 1; + +/// Stable identifier for the exact set of `--private-key-fd` security +/// properties this report attests to. Only bump this if the contract's +/// meaning changes (e.g. one of the guarantees below is weakened, removed, or +/// a new one is added) — not on every unrelated code change. +const CAPABILITY_REVISION: &str = "secure_private_key_fd_v1"; + +/// Whether `--private-key-fd` is actually usable on this build. This must +/// fail closed: it is only `true` when compiled for Unix, where +/// [`crate::read_key_from_fd`] has a real `/dev/fd`-backed implementation. On +/// any other target (e.g. Windows) the flag is parsed by clap but the +/// underlying read always errors out, so this reports `false` rather than +/// claiming a working feature. +#[cfg(unix)] +const PRIVATE_KEY_FD_SUPPORTED: bool = true; + +#[cfg(not(unix))] +const PRIVATE_KEY_FD_SUPPORTED: bool = false; + +/// Security-relevant details of the `--private-key-fd` implementation. +/// +/// All fields other than `supported` describe the implementation as it +/// exists in the Unix build and stay fixed regardless of target platform — +/// `supported` alone is what tells a downstream consumer whether this +/// platform's build actually backs those properties with a real +/// implementation. +#[derive(Serialize)] +struct PrivateKeyFdCapabilities { + /// `true` only on Unix targets, where `--private-key-fd` has a real + /// `/dev/fd`-backed implementation. Fails closed on other platforms. + supported: bool, + /// How the key material is handed to the process. + transport: &'static str, + /// Minimum accepted file descriptor number (inclusive). + min_fd: u32, + /// Maximum accepted file descriptor number (inclusive). + max_fd: u32, + /// Maximum number of bytes read from the fd for the key. + max_key_bytes: usize, + /// Whether the original fd passed via `--private-key-fd` is closed after + /// being read. + closes_original_fd: bool, + /// Whether the in-memory buffer holding the key is zeroized. + zeroizes_input: bool, + /// Whether `--help` hides the live `BUZZ_PRIVATE_KEY` env value. + help_env_value_redacted: bool, +} + +/// Top-level `buzz capabilities` report. +#[derive(Serialize)] +struct CapabilitiesReport { + /// Shape version of this JSON object. Bump on any shape change. + schema_version: u32, + /// Implementation revision identifier for the `--private-key-fd` + /// security properties attested to below. + capability_revision: &'static str, + private_key_fd: PrivateKeyFdCapabilities, +} + +impl Default for CapabilitiesReport { + fn default() -> Self { + Self { + schema_version: SCHEMA_VERSION, + capability_revision: CAPABILITY_REVISION, + private_key_fd: PrivateKeyFdCapabilities { + supported: PRIVATE_KEY_FD_SUPPORTED, + transport: "inherited_fd", + min_fd: MIN_PRIVATE_KEY_FD, + max_fd: MAX_PRIVATE_KEY_FD, + max_key_bytes: PRIVATE_KEY_FD_MAX_LEN, + closes_original_fd: true, + zeroizes_input: true, + help_env_value_redacted: true, + }, + } + } +} + +/// Run `buzz capabilities`. +/// +/// Prints one JSON object to stdout describing the security properties of +/// the `--private-key-fd` implementation and returns `Ok(())` (exit code 0). +/// Reads no environment variables and performs no I/O beyond stdout. +pub fn cmd_show() -> Result<(), CliError> { + let report = CapabilitiesReport::default(); + let json = serde_json::to_string(&report) + .map_err(|e| CliError::Other(format!("failed to serialize capabilities report: {e}")))?; + println!("{json}"); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn schema_version_and_revision_are_stable() { + assert_eq!(SCHEMA_VERSION, 1); + assert_eq!(CAPABILITY_REVISION, "secure_private_key_fd_v1"); + } + + #[test] + fn private_key_fd_supported_matches_platform() { + // Fails closed: only true on Unix, where read_key_from_fd has a real + // /dev/fd-backed implementation. + assert_eq!(PRIVATE_KEY_FD_SUPPORTED, cfg!(unix)); + } + + #[test] + fn cmd_show_succeeds() { + assert!(cmd_show().is_ok()); + } + + #[test] + fn report_matches_expected_contract() { + let report = CapabilitiesReport::default(); + assert_eq!(report.schema_version, 1); + assert_eq!(report.capability_revision, "secure_private_key_fd_v1"); + assert_eq!(report.private_key_fd.transport, "inherited_fd"); + assert_eq!(report.private_key_fd.min_fd, 3); + assert_eq!(report.private_key_fd.max_fd, 1024); + assert_eq!(report.private_key_fd.max_key_bytes, 256); + assert!(report.private_key_fd.closes_original_fd); + assert!(report.private_key_fd.zeroizes_input); + assert!(report.private_key_fd.help_env_value_redacted); + assert_eq!(report.private_key_fd.supported, cfg!(unix)); + } + + #[test] + fn serializes_to_expected_json_shape() { + let report = CapabilitiesReport::default(); + let value: serde_json::Value = serde_json::to_value(&report).unwrap(); + assert_eq!(value["schema_version"], 1); + assert_eq!(value["capability_revision"], "secure_private_key_fd_v1"); + assert_eq!(value["private_key_fd"]["transport"], "inherited_fd"); + assert_eq!(value["private_key_fd"]["min_fd"], 3); + assert_eq!(value["private_key_fd"]["max_fd"], 1024); + assert_eq!(value["private_key_fd"]["max_key_bytes"], 256); + assert_eq!(value["private_key_fd"]["closes_original_fd"], true); + assert_eq!(value["private_key_fd"]["zeroizes_input"], true); + assert_eq!(value["private_key_fd"]["help_env_value_redacted"], true); + assert_eq!(value["private_key_fd"]["supported"], cfg!(unix)); + } +} diff --git a/crates/buzz-cli/src/commands/mod.rs b/crates/buzz-cli/src/commands/mod.rs index 8691590636..f4fd37b67e 100644 --- a/crates/buzz-cli/src/commands/mod.rs +++ b/crates/buzz-cli/src/commands/mod.rs @@ -1,4 +1,5 @@ pub mod agents; +pub mod capabilities; pub mod channel_templates; pub mod channels; pub mod dms; diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index d5c6b6f9ab..bf345f7dbf 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -56,10 +56,16 @@ Buzz CLI — interact with a Buzz relay Configuration (flags override env vars): BUZZ_RELAY_URL Relay base URL [default: http://localhost:3000] - BUZZ_PRIVATE_KEY Nostr private key (hex or nsec) [required] + BUZZ_PRIVATE_KEY Nostr private key (hex or nsec) [required, unless --private-key-fd] BUZZ_AUTH_TAG NIP-OA auth tag JSON [optional] +--private-key-fd is an alternative to --private-key/BUZZ_PRIVATE_KEY for +handing off the private key via an inherited file descriptor (e.g. from a +parent process/harness) instead of argv or the environment. + The 'pack' subcommand runs locally and does not require a relay connection. +The 'capabilities' subcommand runs locally, reads no private key/env vars, +and prints a versioned JSON report of the CLI's security-relevant behavior. Exit codes: 0=ok 1=bad input 2=relay/network error 3=auth error 4=other 5=write conflict Errors are JSON on stderr: {\"error\": \"\", \"message\": \"\"}" @@ -70,9 +76,22 @@ struct Cli { relay: String, /// Nostr private key (hex or nsec). This is the CLI's identity. - #[arg(long, env = "BUZZ_PRIVATE_KEY")] + #[arg( + long, + env = "BUZZ_PRIVATE_KEY", + hide_env_values = true, + conflicts_with = "private_key_fd" + )] private_key: Option, + /// File descriptor (>= 3) to read the Nostr private key from, as an + /// alternative to --private-key/BUZZ_PRIVATE_KEY. Intended for a parent + /// process/harness to hand off the key without it appearing in argv, + /// shell history, or the environment. Reads at most 256 bytes from + /// `/dev/fd/` (Linux/macOS only — no `/dev/fd` on Windows). + #[arg(long, value_parser = parse_private_key_fd)] + private_key_fd: Option, + /// NIP-OA auth tag JSON (owner attestation). Injected into every signed event. #[arg(long, env = "BUZZ_AUTH_TAG")] auth_tag: Option, @@ -85,6 +104,156 @@ struct Cli { command: Cmd, } +/// Lowest file descriptor accepted for `--private-key-fd`. Rejects +/// stdin/stdout/stderr (0/1/2) so a caller can't accidentally point at a +/// standard stream. +const MIN_PRIVATE_KEY_FD: u32 = 3; + +/// Highest file descriptor accepted for `--private-key-fd`, matching a +/// typical Linux soft fd-limit sanity bound. +const MAX_PRIVATE_KEY_FD: u32 = 1024; + +/// Maximum number of bytes read from the private-key fd. One byte beyond +/// this (257) is used as a sentinel to detect oversized input without +/// buffering unbounded data. +const PRIVATE_KEY_FD_MAX_LEN: usize = 256; + +/// clap `value_parser` for `--private-key-fd`: validates the fd is within +/// `[MIN_PRIVATE_KEY_FD, MAX_PRIVATE_KEY_FD]` before it ever reaches `run()`. +fn parse_private_key_fd(s: &str) -> Result { + let fd: u32 = s + .parse() + .map_err(|_| format!("invalid file descriptor '{s}': must be a non-negative integer"))?; + if !(MIN_PRIVATE_KEY_FD..=MAX_PRIVATE_KEY_FD).contains(&fd) { + return Err(format!( + "file descriptor {fd} out of range: must be between {MIN_PRIVATE_KEY_FD} and {MAX_PRIVATE_KEY_FD} (0/1/2 are reserved for stdio)" + )); + } + Ok(fd) +} + +/// Closes a raw file descriptor when dropped. +/// +/// Used to close the *original* fd handed to `--private-key-fd` once +/// [`read_key_from_fd`] is done with it. `std::fs::File::open("/dev/fd/")` +/// returns a duplicate descriptor, so `File`'s own `Drop` never touches the +/// original — this guard is the only thing that closes it, so there is no +/// double-close: the two descriptors are distinct kernel fd table entries +/// closed independently. `nix::unistd::close` takes a plain [`RawFd`] (no +/// `unsafe`, no reconstructing ownership via `FromRawFd`) and is safe to call +/// here because this guard is the fd's sole owner for the scope of the +/// function. +#[cfg(unix)] +struct FdCloseGuard(std::os::fd::RawFd); + +#[cfg(unix)] +impl Drop for FdCloseGuard { + fn drop(&mut self) { + // Best-effort: the fd is being discarded either way, and the error + // (if any) carries no key material — nothing actionable to do with + // it here. + let _ = nix::unistd::close(self.0); + } +} + +/// Reads the Nostr private key from an inherited file descriptor. +/// +/// Opens `/dev/fd/` (Linux/macOS only — there is no `/dev/fd` on +/// Windows) and reads at most [`PRIVATE_KEY_FD_MAX_LEN`] bytes. The trailing +/// `\n` (and a preceding `\r`, if present) is stripped; no other trimming is +/// performed so a malformed key is not silently altered. The returned string +/// is wrapped in [`zeroize::Zeroizing`] so it is scrubbed from memory when +/// dropped. +/// +/// Opening `/dev/fd/` gives back a *duplicate* of the inherited +/// descriptor — dropping the resulting `File` only closes that duplicate, it +/// does not close `fd` itself. `fd` is therefore closed explicitly via +/// [`nix::unistd::close`] on every return path (success, oversized/empty/ +/// non-UTF-8 key errors, and I/O failure) so the original descriptor never +/// leaks for the rest of the process's lifetime. +#[cfg(unix)] +fn read_key_from_fd(fd: u32) -> Result, CliError> { + use std::os::fd::RawFd; + + let raw_fd = fd as RawFd; + // Closed unconditionally when this function returns (success or any + // error path below), since `_fd_guard`'s `Drop` runs regardless. + let _fd_guard = FdCloseGuard(raw_fd); + + let file = std::fs::File::open(format!("/dev/fd/{fd}")) + .map_err(|_| CliError::Auth(format!("failed to read private key from fd {fd}")))?; + + read_key_from_reader(file, fd) +} + +/// Non-Unix fallback for [`read_key_from_fd`]. There is no `/dev/fd` (or +/// portable inherited-fd API) on Windows, so `--private-key-fd` fails closed +/// here rather than falling back to argv/env — clap's `conflicts_with` on +/// `private_key`/`private_key_fd` means a caller who reaches this function +/// has no other key source available in this invocation anyway. The error +/// carries no fd value or key content, matching the sanitized-error +/// contract of the Unix implementation. +#[cfg(not(unix))] +fn read_key_from_fd(_fd: u32) -> Result, CliError> { + Err(CliError::Usage( + "--private-key-fd is not supported on this platform (Linux/macOS only); use --private-key or BUZZ_PRIVATE_KEY instead".into(), + )) +} + +/// Core read/validate/parse logic shared by [`read_key_from_fd`], factored +/// out so it can be exercised in tests against a synthetic [`std::io::Read`] +/// that fails mid-read — something a real fd can't easily simulate. +/// `fd` is used only for error-message context, not for any I/O here. +fn read_key_from_reader( + mut reader: R, + fd: u32, +) -> Result, CliError> { + use std::io::Read; + use zeroize::Zeroize; + + // Read into a `Zeroizing` buffer from the start so a partial read + // followed by an I/O error still gets its bytes scrubbed — an ordinary + // `Vec` would drop those bytes unzeroized on the error path. + let mut buf = zeroize::Zeroizing::new(Vec::with_capacity(PRIVATE_KEY_FD_MAX_LEN + 1)); + reader + .by_ref() + .take((PRIVATE_KEY_FD_MAX_LEN + 1) as u64) + .read_to_end(&mut buf) + .map_err(|_| CliError::Auth(format!("failed to read private key from fd {fd}")))?; + + if buf.len() > PRIVATE_KEY_FD_MAX_LEN { + return Err(CliError::Key( + "private key from fd exceeds maximum length".into(), + )); + } + + // Strip a single trailing newline (and preceding \r), nothing more. + if buf.last() == Some(&b'\n') { + buf.pop(); + if buf.last() == Some(&b'\r') { + buf.pop(); + } + } + + if buf.is_empty() { + return Err(CliError::Key("private key from fd is empty".into())); + } + + // Take the raw bytes out of `buf` (leaving an empty, harmless Vec behind + // for `buf`'s own Drop) so we can hand them to `String::from_utf8` + // without cloning. On the UTF-8 error path the invalid bytes are + // recovered from the error and explicitly zeroized before returning, + // since they never make it into the final `Zeroizing`. + let raw = std::mem::take(&mut *buf); + let key = String::from_utf8(raw).map_err(|e| { + let mut invalid = e.into_bytes(); + invalid.zeroize(); + CliError::Key("private key from fd is not valid UTF-8".into()) + })?; + + Ok(zeroize::Zeroizing::new(key)) +} + #[derive(Clone, clap::ValueEnum)] pub enum ChannelType { #[value(name = "stream")] @@ -224,6 +393,9 @@ enum Cmd { /// Community moderation — reports queue, bans, timeouts, audit trail #[command(subcommand)] Moderation(ModerationCmd), + /// Report security-relevant CLI capabilities as versioned JSON (local, + /// no relay connection or private key needed) + Capabilities, } #[derive(Clone, Copy, clap::ValueEnum)] @@ -1716,6 +1888,14 @@ pub enum ModerationCmd { } async fn run(cli: Cli) -> Result<(), CliError> { + // `capabilities` is local-only and must run before any private-key + // parsing, env var reads, or relay/client construction — it exists + // specifically so a caller can introspect the CLI's security contract + // without needing a key or a relay. + if let Cmd::Capabilities = cli.command { + return commands::capabilities::cmd_show(); + } + let relay_url = client::normalize_relay_url(&cli.relay); // Pack commands are local-only — no relay connection needed. @@ -1728,11 +1908,29 @@ async fn run(cli: Cli) -> Result<(), CliError> { // Auth: private key is required for all relay operations. // The keypair IS the identity — no tokens, no other auth. - let private_key_str = cli.private_key.ok_or_else(|| { - CliError::Auth("BUZZ_PRIVATE_KEY is required (use --private-key or set env var)".into()) - })?; - let keys = Keys::parse(&private_key_str) - .map_err(|e| CliError::Key(format!("invalid BUZZ_PRIVATE_KEY: {e}")))?; + // + // clap's `conflicts_with` on `private_key`/`private_key_fd` rejects any + // invocation that sets both before `run()` is ever called (this also + // covers BUZZ_PRIVATE_KEY, since clap treats an env-resolved value as + // populating the same field as the flag), so there is no stale + // BUZZ_PRIVATE_KEY to scrub here. + let private_key_str: zeroize::Zeroizing = if let Some(fd) = cli.private_key_fd { + read_key_from_fd(fd)? + } else { + let raw = cli.private_key.ok_or_else(|| { + CliError::Auth( + "BUZZ_PRIVATE_KEY is required (use --private-key, --private-key-fd, or set env var)" + .into(), + ) + })?; + zeroize::Zeroizing::new(raw) + }; + let keys = Keys::parse(private_key_str.as_str()) + .map_err(|e| CliError::Key(format!("invalid private key: {e}")))?; + // The encoded key string has served its purpose once `Keys::parse` + // succeeds — drop it immediately rather than letting it live across the + // rest of this async command (auth-tag verification, relay I/O, dispatch). + drop(private_key_str); // NIP-OA: parse and verify the auth tag if provided. let (auth_tag, auth_tag_json) = match cli.auth_tag { @@ -1774,6 +1972,7 @@ async fn run(cli: Cli) -> Result<(), CliError> { Cmd::Mem(sub) => commands::mem::dispatch(sub, &client).await, Cmd::Moderation(sub) => commands::moderation::dispatch(sub, &client, &cli.format).await, Cmd::Pack(_) => unreachable!("handled above"), + Cmd::Capabilities => unreachable!("handled above"), } } @@ -1793,6 +1992,7 @@ mod tests { let expected_groups: Vec<&str> = vec![ "agents", "canvas", + "capabilities", "channels", "dms", "emoji", @@ -2017,4 +2217,273 @@ mod tests { ); } } + + // ---- --private-key-fd ---- + + /// Writes `contents` to a temp file, rewinds it, and returns a raw fd + /// that fully transfers ownership to the caller via `into_raw_fd` — no + /// `File` value survives to close it a second time. This mirrors the real + /// scenario `read_key_from_fd` is built for: a fd handed off exclusively + /// by a parent process, with nothing else in this process holding it. + /// Keeping a `File` alive alongside the fd (as a prior version of this + /// helper did) would double-close the descriptor once `read_key_from_fd` + /// itself closes it — exactly the reuse hazard the fd-close fix guards + /// against. + #[cfg(unix)] + fn fd_with_contents(contents: &[u8]) -> u32 { + use std::io::{Seek, SeekFrom, Write}; + use std::os::fd::IntoRawFd; + + let mut file = tempfile::tempfile().expect("create tempfile"); + file.write_all(contents).expect("write tempfile"); + file.seek(SeekFrom::Start(0)).expect("seek tempfile"); + file.into_raw_fd() as u32 + } + + /// True if `fd` is closed. Probing a raw fd's validity without `unsafe` + /// (no `BorrowedFd::borrow_raw`, no reconstructing an `OwnedFd` via + /// `FromRawFd`) means we can't hand it to `nix::fcntl::fcntl` (which + /// requires `AsFd`). Instead, mirror what `read_key_from_fd` itself does: + /// attempt to open `/dev/fd/`. A closed fd has no entry to open. + #[cfg(unix)] + fn fd_is_closed(fd: u32) -> bool { + std::fs::File::open(format!("/dev/fd/{fd}")).is_err() + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_reads_valid_key() { + let synthetic_key = "0".repeat(64); // synthetic hex-shaped key, not a real secret + let fd = fd_with_contents(synthetic_key.as_bytes()); + + let result = read_key_from_fd(fd).expect("expected key to be read"); + assert_eq!(result.as_str(), synthetic_key); + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_strips_trailing_newline() { + let synthetic_key = "1".repeat(64); + let with_newline = format!("{synthetic_key}\n"); + let fd = fd_with_contents(with_newline.as_bytes()); + + let result = read_key_from_fd(fd).expect("expected key to be read"); + assert_eq!(result.as_str(), synthetic_key); + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_strips_trailing_crlf() { + let synthetic_key = "2".repeat(64); + let with_crlf = format!("{synthetic_key}\r\n"); + let fd = fd_with_contents(with_crlf.as_bytes()); + + let result = read_key_from_fd(fd).expect("expected key to be read"); + assert_eq!(result.as_str(), synthetic_key); + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_missing_fd_is_sanitized_auth_error() { + // A large fd number, almost certainly not open in this process. + let fd = MAX_PRIVATE_KEY_FD; + let err = read_key_from_fd(fd).expect_err("expected missing fd to error"); + match err { + CliError::Auth(msg) => { + assert!(msg.contains(&fd.to_string()), "message: {msg}"); + // No OS error internals (path context, errno text) leaked. + assert!(!msg.to_lowercase().contains("os error"), "message: {msg}"); + } + other => panic!("expected Auth error, got {other:?}"), + } + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_empty_is_key_error() { + let fd = fd_with_contents(b""); + let err = read_key_from_fd(fd).expect_err("expected empty content to error"); + match err { + CliError::Key(msg) => assert_eq!(msg, "private key from fd is empty"), + other => panic!("expected Key error, got {other:?}"), + } + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_oversized_is_key_error() { + let oversized = "a".repeat(PRIVATE_KEY_FD_MAX_LEN + 1); + let fd = fd_with_contents(oversized.as_bytes()); + let err = read_key_from_fd(fd).expect_err("expected oversized content to error"); + match err { + CliError::Key(msg) => { + assert_eq!(msg, "private key from fd exceeds maximum length"); + assert!(!msg.contains(&oversized), "oversized content leaked: {msg}"); + } + other => panic!("expected Key error, got {other:?}"), + } + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_non_utf8_is_key_error() { + let invalid_utf8: &[u8] = &[0xff, 0xfe, 0xfd]; + let fd = fd_with_contents(invalid_utf8); + let err = read_key_from_fd(fd).expect_err("expected non-utf8 content to error"); + match err { + CliError::Key(msg) => { + assert_eq!(msg, "private key from fd is not valid UTF-8"); + } + other => panic!("expected Key error, got {other:?}"), + } + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_closes_original_fd_on_success() { + let synthetic_key = "3".repeat(64); + // `fd_with_contents` transfers sole ownership of the fd via + // `into_raw_fd` — nothing else in this process can close it, so an + // EBADF probe afterward can only be explained by `read_key_from_fd` + // itself having closed the original descriptor (not just the + // `/dev/fd/` duplicate it opens internally). + let fd = fd_with_contents(synthetic_key.as_bytes()); + + read_key_from_fd(fd).expect("expected key to be read"); + + assert!( + fd_is_closed(fd), + "expected original fd {fd} to be closed (EBADF) after read_key_from_fd returns" + ); + } + + #[test] + #[cfg(unix)] + fn read_key_from_fd_closes_original_fd_on_error() { + // Oversized content still errors, but the original fd must be closed + // on this path just as reliably as on success. + let oversized = "a".repeat(PRIVATE_KEY_FD_MAX_LEN + 1); + let fd = fd_with_contents(oversized.as_bytes()); + + read_key_from_fd(fd).expect_err("expected oversized content to error"); + + assert!( + fd_is_closed(fd), + "expected original fd {fd} to be closed (EBADF) after an error path" + ); + } + + /// A `Read` impl that yields one chunk of bytes, then fails — simulating + /// a fd that becomes unreadable partway through, without needing a real + /// fd or any network/process setup. + struct FlakyReader { + chunk: &'static [u8], + served: bool, + } + + impl std::io::Read for FlakyReader { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.served { + return Err(std::io::Error::other("synthetic read failure")); + } + self.served = true; + let n = self.chunk.len().min(buf.len()); + buf[..n].copy_from_slice(&self.chunk[..n]); + Ok(n) + } + } + + #[test] + fn read_key_from_reader_partial_read_then_error_is_sanitized() { + // Exercises the guarded (`Zeroizing`-first) buffer path on a partial + // read that never reaches a valid key: the reader hands back some + // secret-shaped bytes, then fails before EOF. The buffer must have + // been `Zeroizing` from before that first successful chunk landed — + // otherwise those bytes would sit in a plain `Vec` with no scrub on + // the error return below. + let reader = FlakyReader { + chunk: b"nsec1partialsecretbytesxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + served: false, + }; + + let err = read_key_from_reader(reader, 7).expect_err("expected read failure to propagate"); + match err { + CliError::Auth(msg) => { + assert!(msg.contains('7'), "message: {msg}"); + assert!( + !msg.contains("nsec1partialsecretbytes"), + "partial content leaked: {msg}" + ); + assert!(!msg.to_lowercase().contains("os error"), "message: {msg}"); + } + other => panic!("expected Auth error, got {other:?}"), + } + } + + #[test] + fn parse_private_key_fd_rejects_stdio_fds() { + for fd in ["0", "1", "2"] { + assert!( + parse_private_key_fd(fd).is_err(), + "fd {fd} should be rejected" + ); + } + } + + #[test] + fn parse_private_key_fd_rejects_out_of_range_and_non_numeric() { + assert!(parse_private_key_fd("-1").is_err()); + assert!(parse_private_key_fd("not-a-number").is_err()); + assert!(parse_private_key_fd("99999").is_err()); + } + + #[test] + fn parse_private_key_fd_accepts_valid_range() { + assert_eq!(parse_private_key_fd("3").unwrap(), 3); + assert_eq!(parse_private_key_fd("1024").unwrap(), 1024); + assert_eq!(parse_private_key_fd("42").unwrap(), 42); + } + + #[test] + fn private_key_and_private_key_fd_conflict() { + let synthetic_key = "nsec1testonlysyntheticvaluexxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; + let result = Cli::try_parse_from([ + "buzz", + "--private-key", + synthetic_key, + "--private-key-fd", + "3", + "channels", + "list", + ]); + assert!( + result.is_err(), + "expected clap to reject conflicting private-key flags" + ); + } + + #[test] + fn help_output_does_not_leak_key_material() { + // --help text should describe the flags without embedding any + // secret-shaped value. + let help = Cli::command().render_long_help().to_string(); + assert!(help.contains("--private-key-fd")); + assert!(help.contains("--private-key")); + // Sanity: no synthetic key value should ever appear in static help text. + assert!(!help.contains("nsec1testonlysynthetic")); + } + + #[test] + #[cfg(unix)] + fn errors_do_not_leak_key_material() { + // Non-UTF8 bytes stand in for "secret-shaped" content that must + // never surface in Debug/Display output of the resulting error. + let secret_marker: &[u8] = b"nsec1testonlysyntheticvalue\xff\xfe"; + let fd = fd_with_contents(secret_marker); + let err = read_key_from_fd(fd).expect_err("expected non-utf8 content to error"); + let debug = format!("{err:?}"); + let display = err.to_string(); + assert!(!debug.contains("nsec1testonlysyntheticvalue")); + assert!(!display.contains("nsec1testonlysyntheticvalue")); + } } diff --git a/crates/buzz-cli/tests/capabilities.rs b/crates/buzz-cli/tests/capabilities.rs new file mode 100644 index 0000000000..443797aef3 --- /dev/null +++ b/crates/buzz-cli/tests/capabilities.rs @@ -0,0 +1,86 @@ +//! Integration tests for `buzz capabilities` — the machine-readable +//! security-contract report consumed by downstream automated verifiers. +//! +//! These spawn the real `buzz` binary as a subprocess (rather than calling +//! the library function in-process) so they also exercise CLI dispatch: that +//! `capabilities` is reachable, exits 0, and is unaffected by env vars, +//! exactly as an external doctor tool would observe it. + +use std::process::Command; + +const SYNTHETIC_NSEC: &str = "nsec1synthetic-do-not-use-0000000000000000000000000000000000"; + +fn run_capabilities(env_private_key: Option<&str>) -> (i32, String, String) { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_buzz")); + cmd.arg("capabilities"); + if let Some(key) = env_private_key { + cmd.env("BUZZ_PRIVATE_KEY", key); + } else { + cmd.env_remove("BUZZ_PRIVATE_KEY"); + } + let output = cmd.output().expect("failed to spawn buzz binary"); + ( + output.status.code().unwrap_or(-1), + String::from_utf8_lossy(&output.stdout).to_string(), + String::from_utf8_lossy(&output.stderr).to_string(), + ) +} + +fn expected_json() -> serde_json::Value { + serde_json::json!({ + "schema_version": 1, + "capability_revision": "secure_private_key_fd_v1", + "private_key_fd": { + "supported": cfg!(unix), + "transport": "inherited_fd", + "min_fd": 3, + "max_fd": 1024, + "max_key_bytes": 256, + "closes_original_fd": true, + "zeroizes_input": true, + "help_env_value_redacted": true, + }, + }) +} + +#[test] +fn capabilities_without_private_key_env_returns_expected_json() { + let (code, stdout, stderr) = run_capabilities(None); + assert_eq!(code, 0, "expected exit 0, stderr: {stderr}"); + + let parsed: serde_json::Value = + serde_json::from_str(stdout.trim()).expect("stdout should be valid JSON"); + assert_eq!(parsed, expected_json(), "unexpected capabilities JSON"); +} + +#[test] +fn capabilities_ignores_private_key_env_and_leaks_nothing() { + let (code, stdout, stderr) = run_capabilities(Some(SYNTHETIC_NSEC)); + assert_eq!(code, 0, "expected exit 0, stderr: {stderr}"); + + let parsed: serde_json::Value = + serde_json::from_str(stdout.trim()).expect("stdout should be valid JSON"); + assert_eq!( + parsed, + expected_json(), + "BUZZ_PRIVATE_KEY must have zero effect on capabilities output" + ); + assert!( + !stdout.contains(SYNTHETIC_NSEC), + "capabilities stdout leaked the synthetic private key:\n{stdout}" + ); + assert!( + !stderr.contains(SYNTHETIC_NSEC), + "capabilities stderr leaked the synthetic private key:\n{stderr}" + ); +} + +#[test] +#[cfg(unix)] +fn private_key_fd_is_supported_on_unix() { + let (code, stdout, stderr) = run_capabilities(None); + assert_eq!(code, 0, "expected exit 0, stderr: {stderr}"); + let parsed: serde_json::Value = + serde_json::from_str(stdout.trim()).expect("stdout should be valid JSON"); + assert_eq!(parsed["private_key_fd"]["supported"], true); +} diff --git a/crates/buzz-cli/tests/help_env_redaction.rs b/crates/buzz-cli/tests/help_env_redaction.rs new file mode 100644 index 0000000000..556b57dcf9 --- /dev/null +++ b/crates/buzz-cli/tests/help_env_redaction.rs @@ -0,0 +1,52 @@ +//! Regression test for the `--help` secret-leak fix: clap's `env` attribute +//! on `--private-key` used to render the *current value* of `BUZZ_PRIVATE_KEY` +//! inline in `--help` output (e.g. `[env: BUZZ_PRIVATE_KEY=nsec1...]`). This +//! spawns the real `buzz` binary as a subprocess with a synthetic secret set +//! only in the child's environment, so it exercises actual clap rendering +//! without mutating the test process's own environment (which would be +//! unsound under parallel test execution). + +use std::process::Command; + +const SYNTHETIC_NSEC: &str = "nsec1synthetic-do-not-use-0000000000000000000000000000000000"; + +fn run_help_with_secret_env(args: &[&str]) -> String { + let output = Command::new(env!("CARGO_BIN_EXE_buzz")) + .args(args) + .env("BUZZ_PRIVATE_KEY", SYNTHETIC_NSEC) + .output() + .expect("failed to spawn buzz binary"); + + // clap prints --help to stdout normally, but be defensive and check both + // streams so a future clap/config change can't silently reintroduce the + // leak via stderr. + format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) +} + +#[test] +fn long_help_does_not_leak_private_key_env_value() { + let combined = run_help_with_secret_env(&["--help"]); + assert!( + !combined.contains(SYNTHETIC_NSEC), + "--help leaked the BUZZ_PRIVATE_KEY value:\n{combined}" + ); + // The env var name itself is expected/allowed to appear (documents how + // to configure the key) — only the value must be hidden. + assert!( + combined.contains("BUZZ_PRIVATE_KEY"), + "--help should still name BUZZ_PRIVATE_KEY so users know how to set it" + ); +} + +#[test] +fn short_help_does_not_leak_private_key_env_value() { + let combined = run_help_with_secret_env(&["-h"]); + assert!( + !combined.contains(SYNTHETIC_NSEC), + "-h leaked the BUZZ_PRIVATE_KEY value:\n{combined}" + ); +}