diff --git a/Cargo.lock b/Cargo.lock index 3b60dc4579..fe82cce0bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -766,6 +766,7 @@ version = "0.1.0" dependencies = [ "anyhow", "base64", + "buzz-cli", "buzz-core", "buzz-persona", "buzz-sdk", @@ -986,6 +987,7 @@ dependencies = [ "tokio-util", "tracing", "tracing-subscriber", + "uuid", "windows-sys 0.61.2", "zeroize", ] diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d047849806..c971bcc318 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -20,6 +20,7 @@ path = "src/main.rs" buzz-core = { workspace = true } buzz-sdk = { workspace = true } buzz-persona = { path = "../buzz-persona" } +buzz-cli = { path = "../buzz-cli" } # Nostr nostr = { workspace = true } diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index c0147baf1b..b82cf2bdc9 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -20,6 +20,20 @@ use crate::usage::{TurnUsage, UsageTracker}; /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Signing material belongs to the harness or its dedicated broker, never to +/// the model subprocess. Keep this list at the final spawn boundary: callers +/// may add arbitrary persona environment entries, but they cannot reintroduce +/// a raw signing capability through `AcpClient::spawn`. +const WORKER_RESTRICTED_ENV_KEYS: &[&str] = &[ + "BUZZ_PRIVATE_KEY", + "BUZZ_AUTH_TAG", + "NOSTR_PRIVATE_KEY", + "BUZZ_ACP_PRIVATE_KEY", + // The durable ledger contains signed events (and potentially their NIP-OA + // tag). Workers do not need its filesystem location to use relay_reply. + "BUZZ_ACP_STATE_DIR", +]; + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -423,6 +437,14 @@ impl AcpClient { // Callers MUST still call shutdown().await for guaranteed cleanup. .kill_on_drop(true); + // Start from the ambient environment but explicitly remove every raw + // signing credential before any persona configuration is applied. + // This protects against both desktop-injected credentials and callers + // that accidentally pass a reserved key in `extra_env`. + for key in WORKER_RESTRICTED_ENV_KEYS { + cmd.env_remove(key); + } + // Per-persona env vars (e.g., GOOSE_PROVIDER, BUZZ_AGENT_PROVIDER). // For most keys, operator precedence wins: skip injection if already set // in the parent environment. @@ -448,6 +470,13 @@ impl AcpClient { let codex_merge_active = codex_config_value.is_some(); for (key, value) in extra_env { + if WORKER_RESTRICTED_ENV_KEYS.contains(&key.as_str()) { + tracing::warn!( + key, + "refusing to inject a broker-restricted value into worker" + ); + continue; + } if key == "CODEX_CONFIG" && codex_merge_active { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 9a1b74c276..3c339a2b9f 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -240,7 +240,7 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_RELAY_URL", default_value = "ws://localhost:3000")] pub relay_url: String, - #[arg(long, env = "BUZZ_PRIVATE_KEY")] + #[arg(long, env = "BUZZ_PRIVATE_KEY", hide_env_values = true)] pub private_key: String, /// Agent owner pubkey (64-char hex). Used for --respond-to=owner-only gate. diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index b11d96d8f7..03c28853a6 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -9,6 +9,7 @@ mod pool; mod pool_lifecycle; mod queue; mod relay; +mod reply_broker; mod setup_mode; mod usage; @@ -35,7 +36,7 @@ use config::{ }; use filter::SubscriptionRule; use futures_util::FutureExt; -use nostr::{PublicKey, ToBech32}; +use nostr::PublicKey; use pool::{ AgentPool, ControlSignal, IdleSwitchResult, OwnedAgent, PromptContext, PromptOutcome, PromptResult, PromptSource, SessionState, TimeoutKind, @@ -1314,6 +1315,18 @@ async fn tokio_main() -> Result<()> { ); } + let broker_state_dir = reply_broker::state_dir_from_env(&config.config_path) + .map_err(|e| anyhow::anyhow!("reply broker state setup failed: {e}"))?; + let reply_broker = reply_broker::start( + config.relay_url.clone(), + config.keys.clone(), + std::env::var("BUZZ_AUTH_TAG") + .ok() + .filter(|tag| !tag.is_empty()), + broker_state_dir, + ) + .map_err(|e| anyhow::anyhow!("reply broker startup failed: {e}"))?; + let mut pool = if config.lazy_pool { AgentPool::from_slots((0..config.agents).map(|_| None).collect()) } else { @@ -1528,7 +1541,7 @@ async fn tokio_main() -> Result<()> { let base_prompt_content = config.base_prompt_content.take(); let ctx = Arc::new(PromptContext { - mcp_servers: build_mcp_servers(&config), + mcp_servers: build_mcp_servers(&config, Some(reply_broker.endpoint())), initial_message: config.initial_message.clone(), idle_timeout: Duration::from_secs(config.idle_timeout_secs), max_turn_duration: Duration::from_secs(config.max_turn_duration_secs), @@ -4139,7 +4152,10 @@ async fn run_models(args: ModelsArgs) -> Result<()> { Ok(()) } -fn build_mcp_servers(config: &Config) -> Vec { +fn build_mcp_servers( + config: &Config, + reply_broker: Option<&reply_broker::ReplyBrokerEndpoint>, +) -> Vec { if config.mcp_command.is_empty() { return vec![]; } @@ -4152,32 +4168,20 @@ fn build_mcp_servers(config: &Config) -> Vec { command: config.mcp_command.clone(), args: vec![], env: { - let mut env = vec![ - EnvVar { - name: "BUZZ_RELAY_URL".into(), - value: config.relay_url.clone(), - }, - EnvVar { - name: "BUZZ_PRIVATE_KEY".into(), - // bech32 encoding of a valid secret key is infallible. - // Panic here is correct: injecting a bogus secret would cause - // delayed, hard-to-diagnose agent failures downstream. - value: config - .keys - .secret_key() - .to_bech32() - .expect("secret key bech32 encoding should never fail"), - }, - ]; - // Forward BUZZ_AUTH_TAG (NIP-OA owner attestation credential) - // so the MCP server can attach it to every signed event. - if let Ok(auth_tag) = std::env::var("BUZZ_AUTH_TAG") { - if !auth_tag.is_empty() { - env.push(EnvVar { - name: "BUZZ_AUTH_TAG".into(), - value: auth_tag, - }); - } + // Worker MCP servers receive no signing key or owner attestation. + // `BUZZ_REPLY_BROKER_*` is a narrowly scoped local capability: it + // can create one schema-validated reply through the harness broker, + // never run arbitrary Buzz CLI operations or sign arbitrary events. + let mut env = Vec::new(); + if let Some(broker) = reply_broker { + env.push(EnvVar { + name: "BUZZ_REPLY_BROKER_URL".into(), + value: broker.url.clone(), + }); + env.push(EnvVar { + name: "BUZZ_REPLY_BROKER_CAPABILITY".into(), + value: broker.capability.clone(), + }); } // Forward the agent's display name so dev-mcp can use it as the git // author name instead of the raw npub. Read from the process env @@ -5004,50 +5008,40 @@ mod build_mcp_servers_tests { #[test] fn session_new_mcp_server_has_required_fields() { let config = test_config(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); assert_eq!(servers.len(), 1); let server = &servers[0]; assert_eq!(server.name, "test-mcp-server"); let names: Vec<&str> = server.env.iter().map(|e| e.name.as_str()).collect(); assert!( - names.contains(&"BUZZ_RELAY_URL"), - "missing BUZZ_RELAY_URL; got {names:?}" - ); - assert!( - names.contains(&"BUZZ_PRIVATE_KEY"), - "missing BUZZ_PRIVATE_KEY; got {names:?}" + names.is_empty(), + "workers must not receive relay credentials: {names:?}" ); } #[test] - fn session_new_mcp_server_forwards_buzz_auth_tag() { - let _guard = ENV_LOCK.lock().unwrap(); - std::env::set_var("BUZZ_AUTH_TAG", "test-attestation-tag"); + fn session_new_mcp_server_receives_only_reply_broker_capability() { let config = test_config(); - let servers = build_mcp_servers(&config); - std::env::remove_var("BUZZ_AUTH_TAG"); + let broker = reply_broker::ReplyBrokerEndpoint { + url: "tcp://127.0.0.1:9999".into(), + capability: "scoped-capability".into(), + }; + let servers = build_mcp_servers(&config, Some(&broker)); let server = &servers[0]; - let auth_tag_env = server.env.iter().find(|e| e.name == "BUZZ_AUTH_TAG"); + let names: Vec<&str> = server.env.iter().map(|entry| entry.name.as_str()).collect(); assert!( - auth_tag_env.is_some(), - "BUZZ_AUTH_TAG should be forwarded when set" + names.contains(&"BUZZ_REPLY_BROKER_URL") + && names.contains(&"BUZZ_REPLY_BROKER_CAPABILITY"), + "missing reply capability: {names:?}" + ); + assert!( + !names + .iter() + .any(|name| name.contains("PRIVATE_KEY") || name.contains("AUTH_TAG")), + "signing credentials must never appear in the worker MCP config: {names:?}" ); - assert_eq!(auth_tag_env.unwrap().value, "test-attestation-tag"); - } - - #[test] - fn session_new_mcp_server_skips_empty_buzz_auth_tag() { - let _guard = ENV_LOCK.lock().unwrap(); - std::env::set_var("BUZZ_AUTH_TAG", ""); - let config = test_config(); - let servers = build_mcp_servers(&config); - std::env::remove_var("BUZZ_AUTH_TAG"); - - let server = &servers[0]; - let has_auth_tag = server.env.iter().any(|e| e.name == "BUZZ_AUTH_TAG"); - assert!(!has_auth_tag, "empty BUZZ_AUTH_TAG should not be forwarded"); } #[test] @@ -5055,7 +5049,7 @@ mod build_mcp_servers_tests { let _guard = ENV_LOCK.lock().unwrap(); std::env::set_var("BUZZ_ACP_DISPLAY_NAME", "Duncan"); let config = test_config(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); let entry = servers[0] @@ -5074,7 +5068,7 @@ mod build_mcp_servers_tests { let _guard = ENV_LOCK.lock().unwrap(); std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); let config = test_config(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); // Absent, not empty-valued: dev-mcp distinguishes the two and only // falls back to the npub when the key is missing or blank. @@ -5092,7 +5086,7 @@ mod build_mcp_servers_tests { let _guard = ENV_LOCK.lock().unwrap(); std::env::set_var("BUZZ_ACP_DISPLAY_NAME", ""); let config = test_config(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); assert!( @@ -5108,7 +5102,7 @@ mod build_mcp_servers_tests { fn empty_mcp_command_returns_no_servers() { let mut config = test_config(); config.mcp_command = "".into(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); assert!( servers.is_empty(), "empty mcp_command should produce no MCP servers" @@ -5119,7 +5113,7 @@ mod build_mcp_servers_tests { fn absolute_path_mcp_command_uses_file_stem_as_name() { let mut config = test_config(); config.mcp_command = "/opt/bin/my-mcp-server".into(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); assert_eq!(servers.len(), 1); assert_eq!(servers[0].name, "my-mcp-server"); } @@ -5140,7 +5134,7 @@ mod build_mcp_servers_tests { // Confirm a non-empty command with no stem (e.g. just a dot) also falls back. config.mcp_command = ".".into(); - let servers = build_mcp_servers(&config); + let servers = build_mcp_servers(&config, None); assert_eq!(servers.len(), 1); assert_eq!( servers[0].name, "mcp", diff --git a/crates/buzz-acp/src/reply_broker.rs b/crates/buzz-acp/src/reply_broker.rs new file mode 100644 index 0000000000..b96bb90021 --- /dev/null +++ b/crates/buzz-acp/src/reply_broker.rs @@ -0,0 +1,564 @@ +//! Narrow, durable reply broker for managed-agent workers. +//! +//! The ACP harness owns the Nostr signing key. Workers receive only a scoped +//! loopback capability which can create a kind-9 reply to an existing message; +//! they never receive the signing key or owner attestation. + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use buzz_cli::BuzzClient; +use buzz_sdk::{build_message, nip_oa::parse_auth_tag, ThreadRef}; +use nostr::{Event, EventId, Keys}; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::net::{TcpListener, TcpStream}; +use tokio::sync::{oneshot, Mutex}; +use uuid::Uuid; + +const MAX_REQUEST_BYTES: usize = 128 * 1024; +const MAX_CONTENT_BYTES: usize = 64 * 1024; + +/// Opaque loopback connection details passed to the worker MCP server. +#[derive(Clone, Debug)] +pub struct ReplyBrokerEndpoint { + pub url: String, + pub capability: String, +} + +/// Keeps the broker alive for the lifetime of the harness. +pub struct ReplyBroker { + endpoint: ReplyBrokerEndpoint, + shutdown: Option>, +} + +impl ReplyBroker { + pub fn endpoint(&self) -> &ReplyBrokerEndpoint { + &self.endpoint + } +} + +impl Drop for ReplyBroker { + fn drop(&mut self) { + if let Some(shutdown) = self.shutdown.take() { + let _ = shutdown.send(()); + } + } +} + +#[derive(Clone)] +struct BrokerState { + capability: String, + client: Arc, + ledger_dir: PathBuf, + // A reply idempotency key must be serialized across the read → sign → + // persist boundary. One lock keeps that invariant straightforward; reply + // publication is infrequent and a second request receives the exact first + // event rather than a second freshly signed event. + ledger_lock: Arc>, +} + +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +struct ReplyRequest { + capability: String, + channel_id: String, + reply_to: String, + content: String, + idempotency_key: String, +} + +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +struct LedgerRequest { + channel_id: String, + reply_to: String, + content: String, + idempotency_key: String, +} + +#[derive(Debug, Deserialize, Serialize)] +struct LedgerEntry { + request: LedgerRequest, + event: Event, + event_id: String, + state: LedgerState, + run: ReplyRun, +} + +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum LedgerState { + Prepared, + Sent, + DeliveryUnknown, +} + +/// Minimal durable continuation state for the broker-owned delivery phase. +/// Recovery remains deliberately disabled: an expired lease is evidence for an +/// operator, never authority for this broker to publish a different message. +#[derive(Debug, Deserialize, Serialize)] +struct ReplyRun { + source_channel: String, + source_event_id: String, + objective: String, + owner_pubkey: String, + next_action: String, + acceptance_evidence: Option, + state: ReplyRunState, + current_thread_event: String, + progress_lease_expires_at: i64, + hard_deadline_at: i64, + recovery_count: u8, + terminal_reason: Option, +} + +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum ReplyRunState { + Owned, + Complete, + Blocked, +} + +#[derive(Debug, Serialize)] +struct ReplyResponse { + ok: bool, + event_id: Option, + state: Option, + error: Option, +} + +impl ReplyResponse { + fn success(event_id: String, state: LedgerState) -> Self { + Self { + ok: true, + event_id: Some(event_id), + state: Some(state), + error: None, + } + } + + fn error(error: impl Into) -> Self { + Self { + ok: false, + event_id: None, + state: None, + error: Some(error.into()), + } + } +} + +/// Start a broker bound only to loopback. It accepts one JSON request per +/// connection and emits one JSON response, both newline-delimited. +pub fn start( + relay_url: String, + keys: Keys, + auth_tag_json: Option, + state_dir: PathBuf, +) -> Result { + let auth_tag = auth_tag_json + .as_deref() + .map(parse_auth_tag) + .transpose() + .map_err(|e| format!("invalid broker auth tag: {e}"))?; + let client = BuzzClient::new(relay_url, keys, auth_tag, auth_tag_json) + .map_err(|e| format!("reply broker client setup failed: {e}"))?; + let ledger_dir = state_dir.join("reply-ledger"); + create_private_dir(&ledger_dir)?; + + // A loopback listener is intentionally not a signing API. The capability + // authorizes only the constrained reply schema below; it cannot select an + // event kind, signer, relay, auth tag, or arbitrary destination. + let listener = std::net::TcpListener::bind("127.0.0.1:0") + .map_err(|e| format!("reply broker bind failed: {e}"))?; + listener + .set_nonblocking(true) + .map_err(|e| format!("reply broker nonblocking setup failed: {e}"))?; + let address = listener + .local_addr() + .map_err(|e| format!("reply broker address failed: {e}"))?; + let listener = TcpListener::from_std(listener) + .map_err(|e| format!("reply broker async setup failed: {e}"))?; + let endpoint = ReplyBrokerEndpoint { + url: format!("tcp://{address}"), + capability: Uuid::new_v4().simple().to_string(), + }; + let state = BrokerState { + capability: endpoint.capability.clone(), + client: Arc::new(client), + ledger_dir, + ledger_lock: Arc::new(Mutex::new(())), + }; + let (shutdown_tx, mut shutdown_rx) = oneshot::channel(); + tokio::spawn(async move { + loop { + tokio::select! { + _ = &mut shutdown_rx => break, + accepted = listener.accept() => match accepted { + Ok((stream, _)) => { + let state = state.clone(); + tokio::spawn(async move { serve_connection(stream, state).await; }); + } + Err(error) => { + tracing::warn!("reply broker accept failed: {error}"); + break; + } + } + } + } + }); + + Ok(ReplyBroker { + endpoint, + shutdown: Some(shutdown_tx), + }) +} + +pub fn state_dir_from_env(config_path: &Path) -> Result { + let dir = std::env::var_os("BUZZ_ACP_STATE_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| { + config_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("buzz-acp-state") + }); + create_private_dir(&dir)?; + Ok(dir) +} + +async fn serve_connection(stream: TcpStream, state: BrokerState) { + let (read, mut write) = stream.into_split(); + let mut lines = BufReader::new(read).lines(); + let response = match lines.next_line().await { + Ok(Some(line)) if line.len() <= MAX_REQUEST_BYTES => match serde_json::from_str(&line) { + Ok(request) => handle_request(&state, request).await, + Err(error) => ReplyResponse::error(format!("invalid reply request: {error}")), + }, + Ok(Some(_)) => ReplyResponse::error("reply request exceeds size limit"), + Ok(None) => ReplyResponse::error("empty reply request"), + Err(error) => ReplyResponse::error(format!("reply request read failed: {error}")), + }; + if let Ok(payload) = serde_json::to_vec(&response) { + let _ = write.write_all(&payload).await; + let _ = write.write_all(b"\n").await; + } +} + +async fn handle_request(state: &BrokerState, request: ReplyRequest) -> ReplyResponse { + if request.capability != state.capability { + return ReplyResponse::error("reply capability rejected"); + } + if request.content.is_empty() || request.content.len() > MAX_CONTENT_BYTES { + return ReplyResponse::error("reply content must contain 1..=65536 bytes"); + } + if request.content.contains('\0') { + return ReplyResponse::error("reply content must not contain NUL bytes"); + } + if contains_protected_secret_marker(&request.content) { + return ReplyResponse::error( + "reply content appears to contain a protected Buzz signing credential", + ); + } + if request.idempotency_key.is_empty() || request.idempotency_key.len() > 128 { + return ReplyResponse::error("idempotency_key must contain 1..=128 bytes"); + } + if !is_hex64(&request.reply_to) { + return ReplyResponse::error("reply_to must be a 64-character hexadecimal event id"); + } + let channel_id = match request.channel_id.parse::() { + Ok(channel_id) => channel_id, + Err(_) => return ReplyResponse::error("channel_id must be a UUID"), + }; + + let _guard = state.ledger_lock.lock().await; + let ledger_path = ledger_path(&state.ledger_dir, &request.idempotency_key); + let ledger_request = LedgerRequest { + channel_id: request.channel_id.clone(), + reply_to: request.reply_to.clone(), + content: request.content.clone(), + idempotency_key: request.idempotency_key.clone(), + }; + + let mut entry = match read_ledger(&ledger_path) { + Ok(Some(entry)) => { + if entry.request != ledger_request { + return ReplyResponse::error( + "idempotency_key is already bound to a different reply", + ); + } + entry + } + Ok(None) => match prepare_entry(state, channel_id, &request, ledger_request).await { + Ok(entry) => { + if let Err(error) = write_ledger(&ledger_path, &entry) { + return ReplyResponse::error(error); + } + entry + } + Err(error) => return ReplyResponse::error(error), + }, + Err(error) => return ReplyResponse::error(error), + }; + + if entry.state == LedgerState::Sent { + return ReplyResponse::success(entry.event_id, LedgerState::Sent); + } + + // A previous caller may have lost the response after the relay accepted + // the event. Reconcile by deterministic event id before any resend. + match event_exists(&state.client, &entry.event_id).await { + Ok(true) => { + mark_sent(&mut entry); + if let Err(error) = write_ledger(&ledger_path, &entry) { + return ReplyResponse::error(error); + } + ReplyResponse::success(entry.event_id, LedgerState::Sent) + } + Ok(false) => match state.client.submit_event(entry.event.clone()).await { + Ok(_) => { + mark_sent(&mut entry); + if let Err(error) = write_ledger(&ledger_path, &entry) { + return ReplyResponse::error(error); + } + ReplyResponse::success(entry.event_id, LedgerState::Sent) + } + Err(error) => { + mark_delivery_unknown(&mut entry); + let _ = write_ledger(&ledger_path, &entry); + ReplyResponse::error(format!( + "reply delivery is unknown for {}: {error}", + entry.event_id + )) + } + }, + Err(error) => { + mark_delivery_unknown(&mut entry); + let _ = write_ledger(&ledger_path, &entry); + ReplyResponse::error(format!( + "reply delivery reconciliation failed for {}: {error}", + entry.event_id + )) + } + } +} + +async fn prepare_entry( + state: &BrokerState, + channel_id: Uuid, + request: &ReplyRequest, + ledger_request: LedgerRequest, +) -> Result { + let parent = fetch_parent(&state.client, &request.reply_to).await?; + let parent_channel = channel_from_event(&parent)?; + if parent_channel != channel_id { + return Err("reply_to does not belong to channel_id".into()); + } + let parent_id = EventId::from_hex(&request.reply_to) + .map_err(|_| "reply_to must be a valid event id".to_string())?; + let root_id = find_root_from_event(&parent) + .and_then(|id| EventId::from_hex(&id).ok()) + .unwrap_or(parent_id); + let thread = ThreadRef { + root_event_id: root_id, + parent_event_id: parent_id, + }; + let builder = build_message(channel_id, &request.content, Some(&thread), &[], false, &[]) + .map_err(|e| format!("reply construction failed: {e}"))?; + let event = state + .client + .sign_event(builder) + .map_err(|e| format!("reply signing failed: {e}"))?; + let now = chrono::Utc::now().timestamp(); + Ok(LedgerEntry { + request: ledger_request, + event_id: event.id.to_hex(), + event, + state: LedgerState::Prepared, + run: ReplyRun { + source_channel: request.channel_id.clone(), + source_event_id: request.reply_to.clone(), + objective: "publish a constrained reply".into(), + owner_pubkey: state.client.keys().public_key().to_hex(), + next_action: "reconcile or submit the exact persisted event".into(), + acceptance_evidence: None, + state: ReplyRunState::Owned, + current_thread_event: request.reply_to.clone(), + progress_lease_expires_at: now + 15 * 60, + hard_deadline_at: now + 60 * 60, + recovery_count: 0, + terminal_reason: None, + }, + }) +} + +fn mark_sent(entry: &mut LedgerEntry) { + entry.state = LedgerState::Sent; + entry.run.state = ReplyRunState::Complete; + entry.run.next_action = "none".into(); + entry.run.acceptance_evidence = Some(entry.event_id.clone()); + entry.run.terminal_reason = Some("delivery_confirmed".into()); +} + +fn mark_delivery_unknown(entry: &mut LedgerEntry) { + entry.state = LedgerState::DeliveryUnknown; + entry.run.state = ReplyRunState::Blocked; + entry.run.next_action = "query the persisted event id before any retry".into(); + entry.run.terminal_reason = Some("delivery_unknown".into()); +} + +async fn fetch_parent(client: &BuzzClient, event_id: &str) -> Result { + let raw = client + .query(&serde_json::json!({ "ids": [event_id], "limit": 1 })) + .await + .map_err(|e| format!("parent lookup failed: {e}"))?; + serde_json::from_str::(&raw) + .map_err(|e| format!("parent lookup JSON failed: {e}"))? + .as_array() + .and_then(|events| events.first()) + .cloned() + .ok_or_else(|| "reply parent was not found".into()) +} + +async fn event_exists(client: &BuzzClient, event_id: &str) -> Result { + let raw = client + .query(&serde_json::json!({ "ids": [event_id], "limit": 1 })) + .await + .map_err(|e| format!("event lookup failed: {e}"))?; + let events: serde_json::Value = + serde_json::from_str(&raw).map_err(|e| format!("event lookup JSON failed: {e}"))?; + Ok(events.as_array().is_some_and(|events| !events.is_empty())) +} + +fn channel_from_event(event: &serde_json::Value) -> Result { + event + .get("tags") + .and_then(serde_json::Value::as_array) + .and_then(|tags| { + tags.iter().find_map(|tag| { + let parts = tag.as_array()?; + (parts.first()?.as_str()? == "h") + .then(|| parts.get(1)?.as_str()) + .flatten() + }) + }) + .ok_or_else(|| "reply parent is missing a channel tag".to_string())? + .parse::() + .map_err(|_| "reply parent has an invalid channel tag".into()) +} + +fn find_root_from_event(event: &serde_json::Value) -> Option { + let tags = event.get("tags")?.as_array()?; + let mut reply = None; + for tag in tags { + let parts = tag.as_array()?; + if parts.len() >= 4 && parts.first()?.as_str()? == "e" && is_hex64(parts.get(1)?.as_str()?) + { + match parts.get(3)?.as_str()? { + "root" => return Some(parts.get(1)?.as_str()?.to_string()), + "reply" => reply = Some(parts.get(1)?.as_str()?.to_string()), + _ => {} + } + } + } + reply +} + +fn ledger_path(dir: &Path, idempotency_key: &str) -> PathBuf { + let digest = Sha256::digest(idempotency_key.as_bytes()); + dir.join(format!("{}.json", hex::encode(digest))) +} + +fn read_ledger(path: &Path) -> Result, String> { + match std::fs::read(path) { + Ok(bytes) => serde_json::from_slice(&bytes) + .map(Some) + .map_err(|e| format!("reply ledger is invalid: {e}")), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(format!("reply ledger read failed: {error}")), + } +} + +fn write_ledger(path: &Path, entry: &LedgerEntry) -> Result<(), String> { + let bytes = + serde_json::to_vec(entry).map_err(|e| format!("reply ledger encode failed: {e}"))?; + let temporary = path.with_extension(format!("{}.tmp", Uuid::new_v4().simple())); + std::fs::write(&temporary, bytes).map_err(|e| format!("reply ledger write failed: {e}"))?; + restrict_file(&temporary)?; + std::fs::rename(&temporary, path).map_err(|e| format!("reply ledger commit failed: {e}")) +} + +fn create_private_dir(path: &Path) -> Result<(), String> { + std::fs::create_dir_all(path).map_err(|e| format!("reply state directory failed: {e}"))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)) + .map_err(|e| format!("reply state directory permissions failed: {e}"))?; + } + Ok(()) +} + +fn restrict_file(path: &Path) -> Result<(), String> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) + .map_err(|e| format!("reply ledger permissions failed: {e}"))?; + } + Ok(()) +} + +fn is_hex64(value: &str) -> bool { + value.len() == 64 && value.chars().all(|character| character.is_ascii_hexdigit()) +} + +fn contains_protected_secret_marker(content: &str) -> bool { + let lower = content.to_ascii_lowercase(); + lower.contains("nsec1") + || lower.contains("buzz_private_key") + || lower.contains("nostr_private_key") + || lower.contains("buzz_auth_tag") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ledger_path_does_not_expose_idempotency_key() { + let path = ledger_path(Path::new("/state"), "reply:private/input"); + assert_eq!(path.parent(), Some(Path::new("/state"))); + assert!(path + .file_name() + .unwrap() + .to_string_lossy() + .ends_with(".json")); + assert!(!path.to_string_lossy().contains("private/input")); + } + + #[test] + fn root_parser_prefers_root_over_reply() { + let event = serde_json::json!({ + "tags": [ + ["e", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "", "reply"], + ["e", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "", "root"] + ] + }); + assert_eq!( + find_root_from_event(&event).as_deref(), + Some("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + ); + } + + #[test] + fn protected_signing_markers_are_never_publishable() { + assert!(contains_protected_secret_marker( + "BUZZ_PRIVATE_KEY=redacted" + )); + assert!(contains_protected_secret_marker("nsec1example")); + assert!(!contains_protected_secret_marker( + "A normal delivery report." + )); + } +} diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index 6ab81a082d..2fd18458f3 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -5,7 +5,7 @@ mod error; mod validate; use clap::{Parser, Subcommand}; -use client::BuzzClient; +pub use client::BuzzClient; use error::CliError; use nostr::Keys; use uuid::Uuid; @@ -82,11 +82,11 @@ 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)] private_key: Option, /// NIP-OA auth tag JSON (owner attestation). Injected into every signed event. - #[arg(long, env = "BUZZ_AUTH_TAG")] + #[arg(long, env = "BUZZ_AUTH_TAG", hide_env_values = true)] auth_tag: Option, /// Output format: 'json' (default, full fields) or 'compact' (reduced fields). @@ -1803,6 +1803,45 @@ mod tests { Cli::command().debug_assert(); } + #[test] + fn top_level_help_never_renders_environment_values() { + const TEST_PRIVATE_KEY: &str = "nsec1testhelpmustneverrender"; + const TEST_AUTH_TAG: &str = "[\"auth\",\"test-help-value\"]"; + + let old_private_key = std::env::var_os("BUZZ_PRIVATE_KEY"); + let old_auth_tag = std::env::var_os("BUZZ_AUTH_TAG"); + // Environment mutation is process-global. This test restores both values + // before asserting so its sentinel cannot leak into other test cases. + unsafe { + std::env::set_var("BUZZ_PRIVATE_KEY", TEST_PRIVATE_KEY); + std::env::set_var("BUZZ_AUTH_TAG", TEST_AUTH_TAG); + } + let mut help = Vec::new(); + Cli::command() + .write_long_help(&mut help) + .expect("render CLI help"); + let help = String::from_utf8(help).expect("help is UTF-8"); + unsafe { + match old_private_key { + Some(value) => std::env::set_var("BUZZ_PRIVATE_KEY", value), + None => std::env::remove_var("BUZZ_PRIVATE_KEY"), + } + match old_auth_tag { + Some(value) => std::env::set_var("BUZZ_AUTH_TAG", value), + None => std::env::remove_var("BUZZ_AUTH_TAG"), + } + } + + assert!( + !help.contains(TEST_PRIVATE_KEY), + "top-level help must not render a private-key environment value" + ); + assert!( + !help.contains(TEST_AUTH_TAG), + "top-level help must not render an auth-tag environment value" + ); + } + #[test] fn command_inventory_is_stable() { let expected_groups: Vec<&str> = vec![ diff --git a/crates/buzz-dev-mcp/Cargo.toml b/crates/buzz-dev-mcp/Cargo.toml index 8b711b8063..ef9bdf0013 100644 --- a/crates/buzz-dev-mcp/Cargo.toml +++ b/crates/buzz-dev-mcp/Cargo.toml @@ -23,6 +23,7 @@ tokio = { workspace = true } tokio-util = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +uuid = { workspace = true } rmcp = { workspace = true } schemars = { workspace = true } similar = "3" diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index 9b98974802..f7b54d0d62 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -12,6 +12,7 @@ use std::sync::Arc; mod paths; mod read_file; +mod reply; mod rg; mod shell; mod shim; @@ -39,7 +40,7 @@ impl DevMcp { #[tool( name = "shell", - description = "Run a shell command (bash by default; set `BUZZ_SHELL` to use cmd, PowerShell, or another shell). Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 600000 (10 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g -C --files), tree (flags: -d ; shows line counts), and buzz (Buzz relay CLI — run buzz --help for commands)." + description = "Run a shell command (bash by default; set `BUZZ_SHELL` to use cmd, PowerShell, or another shell). Ephemeral process per call. Output tail-truncated to ~8KB for the LLM; full output (first 10MB) saved to artifact file. timeout_ms defaults to 120000 (2 min) if omitted; capped at 600000 (10 min). For long-running commands (git push with hooks, cargo build, test suites), use 300000+. On PATH: rg (prefer over grep; flags: -n -i -l -g -C --files) and tree (flags: -d ; shows line counts). Use relay_reply, not raw Buzz CLI, to publish a Buzz reply." )] async fn shell( &self, @@ -96,6 +97,17 @@ impl DevMcp { } } + #[tool( + name = "relay_reply", + description = "Send one durable reply to a Buzz message through the harness-owned broker. Use this for all Buzz replies: it accepts only channel_id, reply_to, content, and an idempotency key, and never exposes a signing key. If delivery outcome is unknown, retry the exact same request with the returned idempotency_key; do not create a fresh reply." + )] + async fn relay_reply( + &self, + Parameters(params): Parameters, + ) -> Result { + reply::send(params).await + } + /// Hook: called by the agent before honoring end_turn. Returns /// non-empty objection text iff items remain open. #[tool( diff --git a/crates/buzz-dev-mcp/src/reply.rs b/crates/buzz-dev-mcp/src/reply.rs new file mode 100644 index 0000000000..1deeef7eb6 --- /dev/null +++ b/crates/buzz-dev-mcp/src/reply.rs @@ -0,0 +1,118 @@ +//! Client for the harness-owned constrained reply broker. + +use rmcp::ErrorData; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::net::TcpStream; + +const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); +const RESPONSE_LIMIT: usize = 16 * 1024; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct ReplyParams { + /// UUID of the current Buzz channel. + pub channel_id: String, + /// 64-character hexadecimal ID of the message being replied to. + pub reply_to: String, + /// Markdown message content. The broker enforces Buzz's 64 KiB limit. + pub content: String, + /// Stable key for retrying this exact reply. Omit on a first attempt; reuse + /// the returned key only when the original outcome is unknown. + #[serde(default)] + pub idempotency_key: Option, +} + +#[derive(Serialize)] +struct BrokerRequest { + capability: String, + channel_id: String, + reply_to: String, + content: String, + idempotency_key: String, +} + +#[derive(Deserialize)] +struct BrokerResponse { + ok: bool, + event_id: Option, + #[allow(dead_code)] + state: Option, + error: Option, +} + +pub async fn send(params: ReplyParams) -> Result { + let endpoint = std::env::var("BUZZ_REPLY_BROKER_URL").map_err(|_| { + ErrorData::internal_error( + "relay reply broker is not configured; raw Buzz publishing is unavailable to workers", + None, + ) + })?; + let capability = std::env::var("BUZZ_REPLY_BROKER_CAPABILITY").map_err(|_| { + ErrorData::internal_error("relay reply broker capability is unavailable", None) + })?; + let address = endpoint + .strip_prefix("tcp://") + .ok_or_else(|| ErrorData::internal_error("relay reply broker endpoint is invalid", None))?; + let idempotency_key = params + .idempotency_key + .unwrap_or_else(|| uuid::Uuid::new_v4().simple().to_string()); + let request = BrokerRequest { + capability, + channel_id: params.channel_id, + reply_to: params.reply_to, + content: params.content, + idempotency_key: idempotency_key.clone(), + }; + let payload = serde_json::to_vec(&request).map_err(|error| { + ErrorData::internal_error(format!("reply request encode failed: {error}"), None) + })?; + let mut stream = tokio::time::timeout(CONNECT_TIMEOUT, TcpStream::connect(address)) + .await + .map_err(|_| ErrorData::internal_error("relay reply broker connection timed out", None))? + .map_err(|error| { + ErrorData::internal_error(format!("relay reply broker unavailable: {error}"), None) + })?; + stream.write_all(&payload).await.map_err(|error| { + ErrorData::internal_error(format!("relay reply broker write failed: {error}"), None) + })?; + stream.write_all(b"\n").await.map_err(|error| { + ErrorData::internal_error(format!("relay reply broker write failed: {error}"), None) + })?; + let mut response = String::new(); + let mut reader = BufReader::new(stream); + let bytes = tokio::time::timeout(CONNECT_TIMEOUT, reader.read_line(&mut response)) + .await + .map_err(|_| ErrorData::internal_error("relay reply broker response timed out", None))? + .map_err(|error| { + ErrorData::internal_error(format!("relay reply broker read failed: {error}"), None) + })?; + if bytes == 0 || response.len() > RESPONSE_LIMIT { + return Err(ErrorData::internal_error( + "relay reply broker returned an invalid response", + None, + )); + } + let response: BrokerResponse = serde_json::from_str(&response).map_err(|error| { + ErrorData::internal_error( + format!("relay reply broker response decode failed: {error}"), + None, + ) + })?; + if response.ok { + let event_id = response.event_id.ok_or_else(|| { + ErrorData::internal_error("relay reply broker omitted the event id", None) + })?; + Ok(format!( + "Reply accepted. event_id={event_id}; idempotency_key={idempotency_key}" + )) + } else { + let error = response + .error + .unwrap_or_else(|| "relay reply broker rejected the reply".into()); + Err(ErrorData::internal_error( + format!("{error}; idempotency_key={idempotency_key}"), + None, + )) + } +} diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 7aa95b1d87..66ca4444f0 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -74,12 +74,11 @@ impl SharedState { fn build_bootstrap(cwd: &Path, shell_hint: &str) -> String { let stack = detect_stack(cwd); - let buzz_hint = - if std::env::var("BUZZ_RELAY_URL").is_ok() && std::env::var("BUZZ_PRIVATE_KEY").is_ok() { - "\nBuzz relay configured. Run `buzz --help` to see available commands.\n" - } else { - "" - }; + let buzz_hint = if std::env::var("BUZZ_REPLY_BROKER_URL").is_ok() { + "\nBuzz replies are available through the relay_reply tool; raw Buzz CLI publishing is intentionally unavailable.\n" + } else { + "" + }; format!( "Working directory: {}\n\ Detected stack: {}\n\ @@ -167,8 +166,9 @@ pub async fn run( cmd.arg(shell_arg).arg(&p.command); cmd.current_dir(&workdir); cmd.env("PATH", &state.shim.path_env); - // NOSTR_PRIVATE_KEY is already removed from this process's env (shim.rs). - // BUZZ_PRIVATE_KEY is intentionally inherited — the buzz CLI needs it. + // Raw signing credentials are removed before this MCP server starts. + // Shell children receive no signing capability; relay_reply is the only + // available write path and is constrained by the harness broker. for (k, v) in &state.shim.git_env { cmd.env(k, v); } diff --git a/crates/buzz-dev-mcp/src/shim.rs b/crates/buzz-dev-mcp/src/shim.rs index cccf0e6eca..4d2dc91600 100644 --- a/crates/buzz-dev-mcp/src/shim.rs +++ b/crates/buzz-dev-mcp/src/shim.rs @@ -11,9 +11,9 @@ use zeroize::Zeroize; /// builds ephemeral `GIT_CONFIG_*` env vars, then removes the env var /// 3. Prepends the shim dir to PATH /// -/// Shell children receive `path_env`, `git_env`, and `BUZZ_PRIVATE_KEY` (for -/// the buzz CLI). `NOSTR_PRIVATE_KEY` is removed from the process env after -/// the keyfile is written — git helpers read from the keyfile only. +/// Shell children receive `path_env` and `git_env`, never raw signing +/// credentials. `NOSTR_PRIVATE_KEY` is removed from the process env after the +/// keyfile is written — git helpers read from the keyfile only. /// Cleaned up on drop (TempDir). pub struct Shim { _dir: TempDir, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index f3b4cb67fd..347b0acce2 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -462,6 +462,14 @@ pub fn spawn_agent_child( return Err(error); } let runtime_key = ManagedAgentRuntimeKey::new(record.pubkey.clone(), relay_url)?; + // Durable broker ledger: scoped to this exact agent+relay runtime pair so + // a restart can reconcile a previously signed reply without crossing a + // relay boundary or sharing state with another agent identity. + let reply_state_dir = super::managed_agents_base_dir(app)? + .join("reply-state") + .join(runtime_key.runtime_id()); + std::fs::create_dir_all(&reply_state_dir) + .map_err(|error| format!("failed to create managed reply state directory: {error}"))?; // Resolve the effective harness (agent command) from the linked persona, so // persona harness edits propagate on the next spawn; an explicit per-agent // override wins. `agent_args` and `mcp_command` are pure derivations of the @@ -579,6 +587,7 @@ pub fn spawn_agent_child( command.env("RUST_LOG", child_rust_log_filter()); command.env("BUZZ_PRIVATE_KEY", &record.private_key_nsec); command.env("BUZZ_RELAY_URL", &effective_relay_url); + command.env("BUZZ_ACP_STATE_DIR", &reply_state_dir); command.env("BUZZ_ACP_LAZY_POOL", if lazy { "true" } else { "false" }); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(","));