diff --git a/crates/buzz-core/src/observer.rs b/crates/buzz-core/src/observer.rs index 8347bda0541..d9ec08c60b1 100644 --- a/crates/buzz-core/src/observer.rs +++ b/crates/buzz-core/src/observer.rs @@ -80,11 +80,22 @@ pub fn encrypt_observer_payload( Ok(encrypted) } -/// NIP-44 decrypt and deserialize an observer payload from `event`. -pub fn decrypt_observer_payload( +/// NIP-44 decrypt an observer payload from `event` and return the plaintext +/// JSON **verbatim**. +/// +/// Callers that only need a typed value should prefer +/// [`decrypt_observer_payload`]. This exists for the one caller that must hash +/// the bytes it received — the trusted-operation broker's idempotency digest. +/// Deserializing and re-serializing first would make that digest depend on a +/// canonical encoding both ends have to agree on; hashing the plaintext as it +/// arrived needs no such agreement. +/// +/// The returned `String` is the caller's to zeroize if it is sensitive; the +/// intermediate copies held here are wiped before return. +pub fn decrypt_observer_plaintext( recipient_keys: &Keys, event: &Event, -) -> Result { +) -> Result { if !content_looks_like_nip44(&event.content) { return Err(ObserverPayloadError::InvalidCiphertextLength( event.content.len(), @@ -104,7 +115,15 @@ pub fn decrypt_observer_payload( got, }); } + Ok(plaintext) +} +/// NIP-44 decrypt and deserialize an observer payload from `event`. +pub fn decrypt_observer_payload( + recipient_keys: &Keys, + event: &Event, +) -> Result { + let mut plaintext = decrypt_observer_plaintext(recipient_keys, event)?; let result = serde_json::from_str(&plaintext); plaintext.zeroize(); Ok(result?) diff --git a/crates/buzz-sdk/src/broker/capabilities.rs b/crates/buzz-sdk/src/broker/capabilities.rs new file mode 100644 index 00000000000..5fce6073bae --- /dev/null +++ b/crates/buzz-sdk/src/broker/capabilities.rs @@ -0,0 +1,406 @@ +//! Broker capabilities — the named business operations a host will perform. +//! +//! A capability is the unit of policy: `agents.create`, `agents.update`, and +//! `agents.delete` are separate names so a host can permit one without +//! permitting the others. There is deliberately no `agents.manage` action +//! union, which would collapse three policy decisions into one. +//! +//! Every args type is `deny_unknown_fields`, so an api key, env-var map, or +//! nsec smuggled into a payload fails to deserialize instead of reaching a +//! mutation. Every outcome type can structurally hold only public identifiers — +//! it cannot carry the owner nsec or a freshly minted agent secret. + +use serde::{Deserialize, Serialize}; + +use crate::SdkError; + +/// Maximum characters in a display name or agent name. +pub const MAX_NAME_CHARS: usize = 120; + +/// Maximum characters in a system prompt. +pub const MAX_PROMPT_CHARS: usize = 20_000; + +/// Maximum characters in a short scalar field (runtime, provider, model). +pub const MAX_SCALAR_CHARS: usize = 300; + +/// Inbound author gate modes a requester may ask for. +/// +/// `allowlist` is deliberately absent: it needs a pubkey list this narrow +/// request shape does not carry, and a mode without its list would mint an +/// agent nobody can talk to. +pub const RESPOND_TO_MODES: [&str; 2] = ["owner-only", "anyone"]; + +/// A capability name the broker can dispatch. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Capability { + /// Mint a managed agent and attach it to a channel. + AgentsCreate, + /// Patch one managed agent belonging to the owner. + AgentsUpdate, + /// Remove one managed agent belonging to the owner. + AgentsDelete, +} + +impl Capability { + /// Stable wire name. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::AgentsCreate => "agents.create", + Self::AgentsUpdate => "agents.update", + Self::AgentsDelete => "agents.delete", + } + } + + /// The capability contract version this build implements. + #[must_use] + pub fn current_version(self) -> u16 { + match self { + Self::AgentsCreate | Self::AgentsUpdate | Self::AgentsDelete => 1, + } + } + + /// Resolve a wire name. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for an unknown capability name. + pub fn parse(name: &str) -> Result { + match name { + "agents.create" => Ok(Self::AgentsCreate), + "agents.update" => Ok(Self::AgentsUpdate), + "agents.delete" => Ok(Self::AgentsDelete), + other => Err(SdkError::InvalidInput(format!( + "unknown broker capability \"{other}\"" + ))), + } + } +} + +/// Which agent an update or delete targets. +/// +/// Exactly one selector, because a request naming the target twice is ambiguous +/// and the host would have to guess which wins. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum AgentTarget { + /// Target by agent pubkey (64 lowercase hex characters). + Pubkey(String), + /// Target by the agent's current name. + Name(String), +} + +impl AgentTarget { + /// Validate and normalize the selector. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for an empty value, an over-long + /// name, or a pubkey that is not 64 hex characters. + pub fn validated(&self) -> Result { + match self { + Self::Pubkey(pubkey) => { + let pubkey = required(pubkey, "agent pubkey", 64)?; + if pubkey.len() != 64 || !pubkey.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(SdkError::InvalidInput( + "agent pubkey must be 64 hex characters".into(), + )); + } + Ok(Self::Pubkey(pubkey.to_ascii_lowercase())) + } + Self::Name(name) => Ok(Self::Name(required(name, "agent name", MAX_NAME_CHARS)?)), + } + } +} + +/// Arguments for `agents.create`. +/// +/// `channelId` is present because creation genuinely needs it: the new agent is +/// attached to that channel. Update and delete carry no channel, since the +/// target is identified by the agent itself. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsCreateArgs { + /// Channel the new agent is attached to. + pub channel_id: String, + /// Name for the new agent. + pub display_name: String, + /// Instructions the new agent runs with. + pub system_prompt: String, + /// Preferred harness id; the host refuses a runtime it cannot resolve. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub runtime: Option, + /// Inference provider. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + /// Model identifier, interpreted relative to the runtime. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, + /// Inbound author gate mode; absent = the host's owner-only default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub respond_to: Option, +} + +impl AgentsCreateArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed channel UUID, an + /// empty or over-long name or prompt, or an unsupported respond-to mode. + pub fn validated(&self) -> Result { + Ok(Self { + channel_id: channel(&self.channel_id)?, + display_name: required(&self.display_name, "display name", MAX_NAME_CHARS)?, + system_prompt: required(&self.system_prompt, "system prompt", MAX_PROMPT_CHARS)?, + runtime: optional(self.runtime.as_ref(), "runtime", MAX_SCALAR_CHARS)?, + provider: optional(self.provider.as_ref(), "provider", MAX_SCALAR_CHARS)?, + model: optional(self.model.as_ref(), "model", MAX_SCALAR_CHARS)?, + respond_to: respond_to(self.respond_to.as_ref())?, + }) + } +} + +/// Arguments for `agents.update`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsUpdateArgs { + /// Which agent to patch. + pub target: AgentTarget, + /// Rename the agent. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub display_name: Option, + /// Replacement instructions. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub system_prompt: Option, + /// Harness id to pin. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub runtime: Option, + /// Inference provider. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + /// Model identifier. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, + /// Inbound author gate mode. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub respond_to: Option, +} + +impl AgentsUpdateArgs { + /// Validate and normalize, requiring at least one field to change. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed target, an over-long + /// field, an unsupported respond-to mode, or a request that changes nothing. + pub fn validated(&self) -> Result { + let normalized = Self { + target: self.target.validated()?, + display_name: optional(self.display_name.as_ref(), "display name", MAX_NAME_CHARS)?, + system_prompt: optional( + self.system_prompt.as_ref(), + "system prompt", + MAX_PROMPT_CHARS, + )?, + runtime: optional(self.runtime.as_ref(), "runtime", MAX_SCALAR_CHARS)?, + provider: optional(self.provider.as_ref(), "provider", MAX_SCALAR_CHARS)?, + model: optional(self.model.as_ref(), "model", MAX_SCALAR_CHARS)?, + respond_to: respond_to(self.respond_to.as_ref())?, + }; + let unchanged = normalized.display_name.is_none() + && normalized.system_prompt.is_none() + && normalized.runtime.is_none() + && normalized.provider.is_none() + && normalized.model.is_none() + && normalized.respond_to.is_none(); + if unchanged { + return Err(SdkError::InvalidInput( + "include at least one field to update".into(), + )); + } + Ok(normalized) + } +} + +/// Arguments for `agents.delete`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsDeleteArgs { + /// Which agent to remove. + pub target: AgentTarget, +} + +impl AgentsDeleteArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed target selector. + pub fn validated(&self) -> Result { + Ok(Self { + target: self.target.validated()?, + }) + } +} + +/// A capability name paired with its strictly typed arguments. +/// +/// Flattened into [`super::BrokerRequest`], so the wire form is +/// `{ "capability": "agents.create", "args": { … } }` and an args shape can +/// never be paired with the wrong capability name. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "capability", content = "args")] +pub enum CapabilityArgs { + /// Mint a managed agent. + #[serde(rename = "agents.create")] + AgentsCreate(AgentsCreateArgs), + /// Patch a managed agent. + #[serde(rename = "agents.update")] + AgentsUpdate(AgentsUpdateArgs), + /// Remove a managed agent. + #[serde(rename = "agents.delete")] + AgentsDelete(AgentsDeleteArgs), +} + +impl CapabilityArgs { + /// The capability these args belong to. + #[must_use] + pub fn capability(&self) -> Capability { + match self { + Self::AgentsCreate(_) => Capability::AgentsCreate, + Self::AgentsUpdate(_) => Capability::AgentsUpdate, + Self::AgentsDelete(_) => Capability::AgentsDelete, + } + } + + /// Validate the arguments in place. + /// + /// # Errors + /// + /// Propagates the per-capability validation error. + pub fn validate(&self) -> Result<(), SdkError> { + match self { + Self::AgentsCreate(args) => args.validated().map(|_| ()), + Self::AgentsUpdate(args) => args.validated().map(|_| ()), + Self::AgentsDelete(args) => args.validated().map(|_| ()), + } + } + + /// Return a normalized copy with every field validated. + /// + /// # Errors + /// + /// Propagates the per-capability validation error. + pub fn validated(&self) -> Result { + Ok(match self { + Self::AgentsCreate(args) => Self::AgentsCreate(args.validated()?), + Self::AgentsUpdate(args) => Self::AgentsUpdate(args.validated()?), + Self::AgentsDelete(args) => Self::AgentsDelete(args.validated()?), + }) + } +} + +/// Outcome of a successful `agents.create`. +/// +/// Carries the new agent's **public** key only. There is no field for the +/// minted secret: the nsec stays on the host, and this type could not transport +/// it even if a handler tried. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsCreateOutcome { + /// The new agent's pubkey (hex). + pub agent_pubkey: String, + /// The new agent's name as stored. + pub display_name: String, + /// Channel the agent was attached to. + pub channel_id: String, +} + +/// Outcome of a successful `agents.update`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsUpdateOutcome { + /// The patched agent's pubkey (hex). + pub agent_pubkey: String, + /// The agent's name after the update. + pub display_name: String, + /// Names of the fields the host actually changed, sorted. + pub updated_fields: Vec, +} + +/// Outcome of a successful `agents.delete`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsDeleteOutcome { + /// The removed agent's pubkey (hex). + pub agent_pubkey: String, + /// The removed agent's name. + pub display_name: String, +} + +/// A capability-specific success payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "capability", content = "outcome")] +pub enum CapabilityOutcome { + /// `agents.create` succeeded. + #[serde(rename = "agents.create")] + AgentsCreate(AgentsCreateOutcome), + /// `agents.update` succeeded. + #[serde(rename = "agents.update")] + AgentsUpdate(AgentsUpdateOutcome), + /// `agents.delete` succeeded. + #[serde(rename = "agents.delete")] + AgentsDelete(AgentsDeleteOutcome), +} + +impl CapabilityOutcome { + /// The capability that produced this outcome. + #[must_use] + pub fn capability(&self) -> Capability { + match self { + Self::AgentsCreate(_) => Capability::AgentsCreate, + Self::AgentsUpdate(_) => Capability::AgentsUpdate, + Self::AgentsDelete(_) => Capability::AgentsDelete, + } + } +} + +// ── Shared validators ─────────────────────────────────────────────────────── + +fn required(value: &str, label: &str, max: usize) -> Result { + let value = value.trim(); + if value.is_empty() { + return Err(SdkError::InvalidInput(format!("{label} must not be empty"))); + } + if value.chars().count() > max { + return Err(SdkError::InvalidInput(format!( + "{label} is too long (max {max} characters)" + ))); + } + Ok(value.to_owned()) +} + +fn optional(value: Option<&String>, label: &str, max: usize) -> Result, SdkError> { + value.map(|value| required(value, label, max)).transpose() +} + +fn channel(value: &str) -> Result { + let value = required(value, "channel", 128)?; + uuid::Uuid::parse_str(&value) + .map_err(|_| SdkError::InvalidInput(format!("invalid channel UUID: {value}")))?; + Ok(value) +} + +fn respond_to(value: Option<&String>) -> Result, SdkError> { + let value = optional(value, "respond-to", MAX_SCALAR_CHARS)?; + if let Some(mode) = value.as_deref() { + if !RESPOND_TO_MODES.contains(&mode) { + return Err(SdkError::InvalidInput(format!( + "respond-to must be one of {}", + RESPOND_TO_MODES.join(", ") + ))); + } + } + Ok(value) +} diff --git a/crates/buzz-sdk/src/broker/mod.rs b/crates/buzz-sdk/src/broker/mod.rs new file mode 100644 index 00000000000..157c08cc1c4 --- /dev/null +++ b/crates/buzz-sdk/src/broker/mod.rs @@ -0,0 +1,401 @@ +//! Buzz trusted-operation broker — wire protocol. +//! +//! The broker lets a requester ask an owner's host to perform a small set of +//! named business operations ("capabilities") that require the owner's +//! credentials. It exposes business capabilities only: never signing, +//! publishing, credential access, or arbitrary tool execution. +//! +//! # Mental model +//! +//! ```text +//! requester → BrokerRequest → (signed, encrypted frame) → host broker +//! host: verify → authorize → validate → claim → dispatch → BrokerResult +//! ``` +//! +//! # What is *not* in this envelope +//! +//! Owner, requester, and relay scope are **transport-derived** on the host from +//! the verified frame. They are deliberately absent here: a request body that +//! could name its own owner would let any signer act on another owner's agents. +//! +//! There is no `authorization` field yet. One gets added, as a discriminated +//! object, when a real grant format and verifier exist — not before, so that no +//! field looks security-bearing while enforcing nothing. +//! +//! There is no client-computed digest. Idempotency is decided host-side; see +//! [`BrokerRequest`] for the retry contract. + +use serde::{Deserialize, Serialize}; + +use crate::SdkError; + +pub mod capabilities; + +pub use capabilities::{ + AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, + AgentsUpdateArgs, AgentsUpdateOutcome, Capability, CapabilityArgs, CapabilityOutcome, +}; + +/// Wire `type` discriminator for a broker request payload. +pub const BROKER_REQUEST_TYPE: &str = "broker_request"; + +/// Wire `type` discriminator for a broker result payload. +pub const BROKER_RESULT_TYPE: &str = "broker_result"; + +/// Current broker protocol version. +/// +/// There is no "absent means 1" compatibility rule: the protocol is unshipped, +/// so `protocolVersion` is required and an unknown value is rejected outright. +pub const BROKER_PROTOCOL_VERSION: u16 = 1; + +/// Maximum accepted length of a `requestId`, in bytes. +pub const MAX_REQUEST_ID_LEN: usize = 128; + +/// A request to execute one broker capability. +/// +/// # Retry contract +/// +/// Retrying an operation means **resending the identical serialized request** +/// with the same `requestId`. The host hashes what it receives and compares +/// that digest against the digest recorded under the same idempotency key: +/// +/// - same key, same digest → the recorded outcome is replayed, nothing re-runs +/// - same key, different digest → rejected as a request-ID conflict +/// +/// Callers therefore must not regenerate, re-order, or re-render the payload +/// between attempts. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct BrokerRequest { + /// Payload discriminator — must equal [`BROKER_REQUEST_TYPE`]. + pub r#type: String, + /// Protocol version — must equal [`BROKER_PROTOCOL_VERSION`]. + pub protocol_version: u16, + /// Caller-chosen idempotency key, unique per logical operation. + pub request_id: String, + /// Capability contract version the caller wrote `args` against. + pub capability_version: u16, + /// The capability to invoke, with its strictly typed arguments. + #[serde(flatten)] + pub capability: CapabilityArgs, +} + +impl BrokerRequest { + /// Build a request for `capability` with the current protocol version. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] if `request_id` is empty, longer than + /// [`MAX_REQUEST_ID_LEN`], or contains non-printable-ASCII bytes. + pub fn new( + request_id: impl Into, + capability: CapabilityArgs, + ) -> Result { + let request_id = request_id.into(); + let request = Self { + r#type: BROKER_REQUEST_TYPE.to_string(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id, + capability_version: capability.capability().current_version(), + capability, + }; + request.validate()?; + Ok(request) + } + + /// The capability this request invokes. + #[must_use] + pub fn capability(&self) -> Capability { + self.capability.capability() + } + + /// Validate every field the host must agree on before executing anything. + /// + /// Callers validate before sealing a frame; the host revalidates after + /// decrypting, because only the host's verdict is authoritative. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a wrong `type`, an unsupported + /// `protocolVersion` or `capabilityVersion`, a malformed `requestId`, or + /// capability arguments that fail their own validation. + pub fn validate(&self) -> Result<(), SdkError> { + if self.r#type != BROKER_REQUEST_TYPE { + return Err(SdkError::InvalidInput(format!( + "broker request type must be \"{BROKER_REQUEST_TYPE}\", got \"{}\"", + self.r#type + ))); + } + if self.protocol_version != BROKER_PROTOCOL_VERSION { + return Err(SdkError::InvalidInput(format!( + "unsupported broker protocolVersion {} (expected {BROKER_PROTOCOL_VERSION})", + self.protocol_version + ))); + } + validate_request_id(&self.request_id)?; + let capability = self.capability(); + if self.capability_version != capability.current_version() { + return Err(SdkError::InvalidInput(format!( + "unsupported capabilityVersion {} for {} (expected {})", + self.capability_version, + capability.as_str(), + capability.current_version() + ))); + } + self.capability.validate() + } +} + +/// Validate a `requestId`: non-empty, bounded, printable ASCII. +/// +/// The bound and character set exist because this value becomes part of a +/// durable primary key and appears in audit records. +/// +/// # Errors +/// +/// Returns [`SdkError::InvalidInput`] when the id is empty, exceeds +/// [`MAX_REQUEST_ID_LEN`] bytes, or contains a byte outside `0x21..=0x7e`. +pub fn validate_request_id(request_id: &str) -> Result<(), SdkError> { + if request_id.is_empty() { + return Err(SdkError::InvalidInput("requestId must not be empty".into())); + } + if request_id.len() > MAX_REQUEST_ID_LEN { + return Err(SdkError::InvalidInput(format!( + "requestId exceeds {MAX_REQUEST_ID_LEN} bytes (got {})", + request_id.len() + ))); + } + if let Some(bad) = request_id + .bytes() + .find(|b| !(0x21..=0x7e).contains(b)) + .map(|b| format!("0x{b:02x}")) + { + return Err(SdkError::InvalidInput(format!( + "requestId must be printable ASCII without spaces (found byte {bad})" + ))); + } + Ok(()) +} + +/// Machine-readable broker error code. +/// +/// These name failures the *broker* is responsible for. Capability-specific +/// failures arrive as [`BrokerErrorCode::CapabilityFailed`] with detail in the +/// accompanying message. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum BrokerErrorCode { + /// The envelope or capability arguments failed validation. + InvalidRequest, + /// The `protocolVersion` is not supported by this host. + UnsupportedProtocolVersion, + /// The capability name is unknown to this host. + UnknownCapability, + /// The `capabilityVersion` is not supported for this capability. + UnsupportedCapabilityVersion, + /// The requester's signature or frame could not be verified. + Unauthenticated, + /// The requester is authenticated but not permitted this operation. + Unauthorized, + /// Reuse of a `requestId` with different request content. + RequestIdConflict, + /// The capability handler ran and reported a domain failure. + CapabilityFailed, + /// The host could not determine whether side effects occurred. + /// + /// Only ever paired with [`BrokerResult::Indeterminate`]. + OutcomeUnknown, + /// An unexpected host-side fault. + Internal, +} + +impl BrokerErrorCode { + /// Stable wire string for this code. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::InvalidRequest => "invalid_request", + Self::UnsupportedProtocolVersion => "unsupported_protocol_version", + Self::UnknownCapability => "unknown_capability", + Self::UnsupportedCapabilityVersion => "unsupported_capability_version", + Self::Unauthenticated => "unauthenticated", + Self::Unauthorized => "unauthorized", + Self::RequestIdConflict => "request_id_conflict", + Self::CapabilityFailed => "capability_failed", + Self::OutcomeUnknown => "outcome_unknown", + Self::Internal => "internal", + } + } +} + +/// A broker error: a machine-readable code plus a human-readable message. +/// +/// Messages are for operators and must never carry secrets — no nsec, no +/// credentials, no decrypted payloads. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct BrokerError { + /// Machine-readable failure code. + pub code: BrokerErrorCode, + /// Operator-facing description. Secret-free. + pub message: String, +} + +impl BrokerError { + /// Construct an error from a code and message. + pub fn new(code: BrokerErrorCode, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + /// An [`BrokerErrorCode::InvalidRequest`] error. + pub fn invalid_request(message: impl Into) -> Self { + Self::new(BrokerErrorCode::InvalidRequest, message) + } + + /// An [`BrokerErrorCode::Unauthorized`] error. + pub fn unauthorized(message: impl Into) -> Self { + Self::new(BrokerErrorCode::Unauthorized, message) + } +} + +/// The terminal disposition of a broker request. +/// +/// A discriminated union, so "succeeded with an error" and "failed with an +/// outcome" are unrepresentable rather than merely discouraged. +/// +/// [`Self::Indeterminate`] is distinct from [`Self::Failed`] on purpose: +/// `Failed` promises no side effects took hold, while `Indeterminate` promises +/// nothing at all and demands operator or capability-level reconciliation. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "status", rename_all = "snake_case")] +pub enum BrokerResult { + /// The capability completed and produced this outcome. + Succeeded { + /// Capability-specific success payload. + #[serde(flatten)] + outcome: CapabilityOutcome, + }, + /// The capability did not complete; no side effects are expected to persist. + Failed { + /// Why it failed. + error: BrokerError, + }, + /// Whether side effects occurred could not be determined. + Indeterminate { + /// What is unknown, and why. + error: BrokerError, + }, +} + +impl BrokerResult { + /// A successful result carrying `outcome`. + #[must_use] + pub fn succeeded(outcome: CapabilityOutcome) -> Self { + Self::Succeeded { outcome } + } + + /// A failed result carrying `error`. + #[must_use] + pub fn failed(error: BrokerError) -> Self { + Self::Failed { error } + } + + /// An indeterminate result carrying `error`. + #[must_use] + pub fn indeterminate(error: BrokerError) -> Self { + Self::Indeterminate { error } + } + + /// Whether this is a terminal success. + #[must_use] + pub fn is_succeeded(&self) -> bool { + matches!(self, Self::Succeeded { .. }) + } + + /// The error, for the two non-success variants. + #[must_use] + pub fn error(&self) -> Option<&BrokerError> { + match self { + Self::Succeeded { .. } => None, + Self::Failed { error } | Self::Indeterminate { error } => Some(error), + } + } +} + +/// A broker result addressed back to the requester. +/// +/// `replayed` is **response metadata**: it describes this delivery, not the +/// domain outcome, and is never persisted as part of the stored result. A +/// replayed response is byte-identical in `result` to the original. +/// +/// Note: this struct cannot use `deny_unknown_fields`, because serde does not +/// support combining it with `#[serde(flatten)]`. Strictness is enforced where +/// it guards execution — on [`BrokerRequest`] and each capability args type. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct BrokerResponse { + /// Payload discriminator — must equal [`BROKER_RESULT_TYPE`]. + pub r#type: String, + /// Protocol version — must equal [`BROKER_PROTOCOL_VERSION`]. + pub protocol_version: u16, + /// Correlates with the originating [`BrokerRequest::request_id`]. + pub request_id: String, + /// The terminal disposition. + #[serde(flatten)] + pub result: BrokerResult, + /// True when this response replays a previously recorded outcome. + #[serde(default, skip_serializing_if = "is_false")] + pub replayed: bool, +} + +fn is_false(value: &bool) -> bool { + !*value +} + +impl BrokerResponse { + /// Build a fresh (non-replayed) response for `request_id`. + pub fn new(request_id: impl Into, result: BrokerResult) -> Self { + Self { + r#type: BROKER_RESULT_TYPE.to_string(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id: request_id.into(), + result, + replayed: false, + } + } + + /// Mark this response as replaying a recorded outcome. + #[must_use] + pub fn replayed(mut self) -> Self { + self.replayed = true; + self + } + + /// Validate discriminator, version, and request id. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] on a wrong `type`, an unsupported + /// `protocolVersion`, or a malformed `requestId`. + pub fn validate(&self) -> Result<(), SdkError> { + if self.r#type != BROKER_RESULT_TYPE { + return Err(SdkError::InvalidInput(format!( + "broker result type must be \"{BROKER_RESULT_TYPE}\", got \"{}\"", + self.r#type + ))); + } + if self.protocol_version != BROKER_PROTOCOL_VERSION { + return Err(SdkError::InvalidInput(format!( + "unsupported broker protocolVersion {} (expected {BROKER_PROTOCOL_VERSION})", + self.protocol_version + ))); + } + validate_request_id(&self.request_id) + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/buzz-sdk/src/broker/tests.rs b/crates/buzz-sdk/src/broker/tests.rs new file mode 100644 index 00000000000..3a3c2df1d68 --- /dev/null +++ b/crates/buzz-sdk/src/broker/tests.rs @@ -0,0 +1,382 @@ +//! Wire-contract tests for the broker envelope. + +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const PUBKEY: &str = "a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971"; + +fn create_args() -> CapabilityArgs { + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Research helper".into(), + system_prompt: "Find sources.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }) +} + +#[test] +fn request_serializes_capability_beside_args() { + let request = BrokerRequest::new("req-1", create_args()).unwrap(); + let json = serde_json::to_value(&request).unwrap(); + + assert_eq!(json["type"], BROKER_REQUEST_TYPE); + assert_eq!(json["protocolVersion"], 1); + assert_eq!(json["requestId"], "req-1"); + assert_eq!(json["capability"], "agents.create"); + assert_eq!(json["capabilityVersion"], 1); + assert_eq!(json["args"]["displayName"], "Research helper"); + + // Unset optionals stay off the wire, so the payload cannot imply a + // runtime/provider/model the caller never chose. + assert!(json["args"].get("runtime").is_none()); + assert!(json["args"].get("respondTo").is_none()); + + let parsed: BrokerRequest = serde_json::from_value(json).unwrap(); + assert_eq!(parsed, request); +} + +/// The envelope must not carry owner/requester/relay identity: those are +/// transport-derived. A body that could name its own owner would let any +/// signer act on another owner's agents. +#[test] +fn request_has_no_caller_supplied_identity_or_context() { + let json = serde_json::to_value(BrokerRequest::new("req-1", create_args()).unwrap()).unwrap(); + let object = json.as_object().unwrap(); + + for forbidden in [ + "ownerPubkey", + "owner", + "requesterPubkey", + "requester", + "relayUrl", + "relayScope", + "context", + "authorizer", + "scope", + "expiry", + "authorization", + "requestDigest", + "digest", + ] { + assert!( + !object.contains_key(forbidden), + "broker request must not carry \"{forbidden}\"" + ); + } + + let mut keys = object.keys().map(String::as_str).collect::>(); + keys.sort_unstable(); + assert_eq!( + keys, + vec![ + "args", + "capability", + "capabilityVersion", + "protocolVersion", + "requestId", + "type", + ] + ); +} + +#[test] +fn unknown_request_field_is_rejected() { + let mut json = + serde_json::to_value(BrokerRequest::new("req-1", create_args()).unwrap()).unwrap(); + json.as_object_mut() + .unwrap() + .insert("ownerPubkey".into(), serde_json::json!(PUBKEY)); + + let error = serde_json::from_value::(json).unwrap_err(); + assert!( + error.to_string().contains("ownerPubkey"), + "unexpected error: {error}" + ); +} + +/// A secret smuggled into args must fail to deserialize rather than reach a +/// mutation. +#[test] +fn secret_bearing_args_fail_to_deserialize() { + let json = serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "capability": "agents.create", + "capabilityVersion": 1, + "args": { + "channelId": CHANNEL, + "displayName": "Sneaky", + "systemPrompt": "hi", + "envVars": { "ANTHROPIC_API_KEY": "sk-live" }, + }, + }); + assert!(serde_json::from_value::(json).is_err()); +} + +#[test] +fn wrong_protocol_version_is_refused() { + let mut request = BrokerRequest::new("req-1", create_args()).unwrap(); + request.protocol_version = 2; + let error = request.validate().unwrap_err().to_string(); + assert!(error.contains("protocolVersion"), "unexpected: {error}"); +} + +#[test] +fn wrong_capability_version_is_refused() { + let mut request = BrokerRequest::new("req-1", create_args()).unwrap(); + request.capability_version = 7; + let error = request.validate().unwrap_err().to_string(); + assert!(error.contains("capabilityVersion"), "unexpected: {error}"); +} + +#[test] +fn unknown_capability_name_is_refused() { + let json = serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "capability": "agents.exfiltrate", + "capabilityVersion": 1, + "args": {}, + }); + assert!(serde_json::from_value::(json).is_err()); + assert!(Capability::parse("agents.exfiltrate").is_err()); +} + +/// Signing, publishing, and credential access are not capabilities. Only +/// business operations are addressable. +#[test] +fn only_business_capabilities_are_addressable() { + for forbidden in [ + "sign", + "publish", + "keys.export", + "identity.nsec", + "tool.exec", + "agents.manage", + "agents.start", + ] { + assert!( + Capability::parse(forbidden).is_err(), + "\"{forbidden}\" must not be a capability" + ); + } + for allowed in ["agents.create", "agents.update", "agents.delete"] { + assert_eq!(Capability::parse(allowed).unwrap().as_str(), allowed); + } +} + +#[test] +fn request_id_must_be_present_bounded_and_printable() { + assert!(BrokerRequest::new("", create_args()).is_err()); + assert!(BrokerRequest::new("a".repeat(MAX_REQUEST_ID_LEN), create_args()).is_ok()); + assert!(BrokerRequest::new("a".repeat(MAX_REQUEST_ID_LEN + 1), create_args()).is_err()); + assert!(BrokerRequest::new("has space", create_args()).is_err()); + assert!(BrokerRequest::new("has\nnewline", create_args()).is_err()); +} + +#[test] +fn update_requires_at_least_one_change() { + let empty = AgentsUpdateArgs { + target: AgentTarget::Pubkey(PUBKEY.into()), + display_name: None, + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + }; + let error = empty.validated().unwrap_err().to_string(); + assert!(error.contains("at least one field"), "unexpected: {error}"); +} + +#[test] +fn target_pubkey_must_be_hex_and_normalizes_case() { + assert!(AgentTarget::Pubkey("nothex".into()).validated().is_err()); + assert!(AgentTarget::Pubkey(PUBKEY[..40].into()) + .validated() + .is_err()); + let upper = AgentTarget::Pubkey(PUBKEY.to_ascii_uppercase()); + assert_eq!( + upper.validated().unwrap(), + AgentTarget::Pubkey(PUBKEY.into()) + ); +} + +#[test] +fn create_rejects_a_malformed_channel_uuid() { + let args = AgentsCreateArgs { + channel_id: "not-a-uuid".into(), + display_name: "A".into(), + system_prompt: "B".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }; + let error = args.validated().unwrap_err().to_string(); + assert!(error.contains("channel"), "unexpected: {error}"); +} + +#[test] +fn create_rejects_an_unsupported_respond_to_mode() { + let args = AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "A".into(), + system_prompt: "B".into(), + runtime: None, + provider: None, + model: None, + respond_to: Some("allowlist".into()), + }; + assert!(args.validated().is_err()); +} + +/// Update and delete carry no channel: per the approved design, channel +/// appears only where the operation needs it (create attachment). +#[test] +fn update_and_delete_carry_no_channel() { + let update = serde_json::to_value(AgentsUpdateArgs { + target: AgentTarget::Pubkey(PUBKEY.into()), + display_name: Some("New".into()), + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + }) + .unwrap(); + assert!(update.get("channelId").is_none()); + + let delete = serde_json::to_value(AgentsDeleteArgs { + target: AgentTarget::Pubkey(PUBKEY.into()), + }) + .unwrap(); + assert!(delete.get("channelId").is_none()); +} + +// ── Results ───────────────────────────────────────────────────────────────── + +#[test] +fn succeeded_result_carries_outcome_and_no_error() { + let response = BrokerResponse::new( + "req-1", + BrokerResult::succeeded(CapabilityOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: PUBKEY.into(), + display_name: "Research helper".into(), + channel_id: CHANNEL.into(), + })), + ); + response.validate().unwrap(); + + let json = serde_json::to_value(&response).unwrap(); + assert_eq!(json["type"], BROKER_RESULT_TYPE); + assert_eq!(json["status"], "succeeded"); + assert_eq!(json["capability"], "agents.create"); + assert_eq!(json["outcome"]["agentPubkey"], PUBKEY); + assert!(json.get("error").is_none()); + // `replayed` is response metadata and stays off the wire when false. + assert!(json.get("replayed").is_none()); + + let parsed: BrokerResponse = serde_json::from_value(json).unwrap(); + assert_eq!(parsed, response); + assert!(parsed.result.is_succeeded()); + assert!(parsed.result.error().is_none()); +} + +#[test] +fn failed_and_indeterminate_are_distinct_and_carry_no_outcome() { + let failed = BrokerResult::failed(BrokerError::new( + BrokerErrorCode::CapabilityFailed, + "runtime not installed", + )); + let failed_json = serde_json::to_value(BrokerResponse::new("r", failed.clone())).unwrap(); + assert_eq!(failed_json["status"], "failed"); + assert_eq!(failed_json["error"]["code"], "capability_failed"); + assert!(failed_json.get("outcome").is_none()); + + let indeterminate = BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "host restarted mid-execution", + )); + let json = serde_json::to_value(BrokerResponse::new("r", indeterminate.clone())).unwrap(); + assert_eq!(json["status"], "indeterminate"); + assert_eq!(json["error"]["code"], "outcome_unknown"); + assert!(json.get("outcome").is_none()); + + assert_ne!(failed, indeterminate); + assert!(!indeterminate.is_succeeded()); +} + +#[test] +fn replay_metadata_rides_the_response_not_the_result() { + let result = BrokerResult::succeeded(CapabilityOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: PUBKEY.into(), + display_name: "Gone".into(), + })); + let fresh = BrokerResponse::new("req-9", result.clone()); + let replayed = BrokerResponse::new("req-9", result.clone()).replayed(); + + // The domain outcome is identical; only the delivery metadata differs. + assert_eq!(fresh.result, replayed.result); + assert!(!fresh.replayed); + assert!(replayed.replayed); + assert_eq!( + serde_json::to_value(&replayed).unwrap()["replayed"], + serde_json::json!(true) + ); + + // `replayed` is not part of the stored result encoding. + let stored = serde_json::to_value(&result).unwrap(); + assert!(stored.get("replayed").is_none()); +} + +#[test] +fn every_error_code_has_a_stable_wire_string() { + for (code, expected) in [ + (BrokerErrorCode::InvalidRequest, "invalid_request"), + ( + BrokerErrorCode::UnsupportedProtocolVersion, + "unsupported_protocol_version", + ), + (BrokerErrorCode::UnknownCapability, "unknown_capability"), + ( + BrokerErrorCode::UnsupportedCapabilityVersion, + "unsupported_capability_version", + ), + (BrokerErrorCode::Unauthenticated, "unauthenticated"), + (BrokerErrorCode::Unauthorized, "unauthorized"), + (BrokerErrorCode::RequestIdConflict, "request_id_conflict"), + (BrokerErrorCode::CapabilityFailed, "capability_failed"), + (BrokerErrorCode::OutcomeUnknown, "outcome_unknown"), + (BrokerErrorCode::Internal, "internal"), + ] { + assert_eq!(code.as_str(), expected); + assert_eq!( + serde_json::to_value(code).unwrap(), + serde_json::json!(expected) + ); + } +} + +/// An outcome type must be structurally unable to carry secret material. +#[test] +fn outcomes_cannot_carry_secrets() { + let json = serde_json::json!({ + "capability": "agents.create", + "outcome": { + "agentPubkey": PUBKEY, + "displayName": "A", + "channelId": CHANNEL, + "privateKeyNsec": "nsec1deadbeef", + }, + }); + assert!( + serde_json::from_value::(json).is_err(), + "an outcome carrying a secret must not deserialize" + ); +} diff --git a/crates/buzz-sdk/src/lib.rs b/crates/buzz-sdk/src/lib.rs index 4ee0cd4c882..845505c56d5 100644 --- a/crates/buzz-sdk/src/lib.rs +++ b/crates/buzz-sdk/src/lib.rs @@ -12,6 +12,7 @@ //! The caller signs with their own keys: `builder.sign_with_keys(&keys)?`. //! No keys are held here. No network calls are made. +pub mod broker; pub mod builders; pub mod mentions; pub mod nip_oa; diff --git a/desktop/src-tauri/src/broker/agents_policy.rs b/desktop/src-tauri/src/broker/agents_policy.rs new file mode 100644 index 00000000000..a634347b23e --- /dev/null +++ b/desktop/src-tauri/src/broker/agents_policy.rs @@ -0,0 +1,236 @@ +//! Authorization policy for the `agents.*` capabilities. +//! +//! # Why this is not broker policy +//! +//! The broker authenticates: it proves who signed a frame and that the frame +//! was sealed for this owner. Whether that signer may *manage agents* is a +//! property of the `agents.*` capability family, not of brokering in general — +//! a future capability with different scope rules must not inherit these. +//! +//! # The rule +//! +//! A requester may manage this owner's agents when the requester **is** one of +//! this owner's managed agents. That is the owner↔requester binding, and it is +//! read from the authoritative Rust roster +//! (`managed_agents::storage::load_managed_agents`), never from anything the +//! caller sent. +//! +//! There is deliberately **no channel restriction on the target** of an update +//! or delete. An owner's agent is the owner's agent regardless of which channel +//! it happens to sit in, and requiring channel co-membership would have made the +//! rule look tighter than it is while adding no real constraint: a requester +//! that can create an agent in a channel could always add its target there +//! first. Channel appears only where the operation genuinely needs it — the +//! attachment on `agents.create`. + +use buzz_sdk_pkg::broker::{BrokerError, BrokerRequest, Capability}; + +use super::pipeline::{Authorizer, BoxFuture, VerifiedRequest}; + +/// The set of agents an owner controls, as far as authorization is concerned. +/// +/// A trait so policy can be tested without a Tauri app handle or a real agent +/// store, and so a non-Desktop host can supply its own roster. +pub trait AgentRoster: Send + Sync { + /// Pubkeys of every agent this owner manages. + fn agent_pubkeys<'a>( + &'a self, + owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>>; +} + +/// `agents.*` authorization backed by an [`AgentRoster`]. +pub struct AgentsAuthorizer { + roster: R, +} + +impl AgentsAuthorizer { + /// Build an authorizer over `roster`. + pub fn new(roster: R) -> Self { + Self { roster } + } +} + +impl Authorizer for AgentsAuthorizer { + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, + capability: Capability, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>> { + Box::pin(async move { + match capability { + Capability::AgentsCreate | Capability::AgentsUpdate | Capability::AgentsDelete => {} + } + + // The requester must never be able to act as the owner directly: + // that would mean a frame signed by the owner key was treated as a + // delegated request, collapsing the distinction the broker exists + // to maintain. + if request + .requester_pubkey + .eq_ignore_ascii_case(&request.owner_pubkey) + { + return Err(BrokerError::unauthorized( + "the owner key is not a broker requester", + )); + } + + let agents = self + .roster + .agent_pubkeys(&request.owner_pubkey) + .await + .map_err(|error| { + // A roster we cannot read is a closed door, not an open one. + BrokerError::unauthorized(format!("could not verify agent ownership: {error}")) + })?; + + let bound = agents + .iter() + .any(|pubkey| pubkey.eq_ignore_ascii_case(&request.requester_pubkey)); + + if bound { + Ok(()) + } else { + Err(BrokerError::unauthorized( + "requester is not an agent managed by this owner", + )) + } + }) + } +} + +#[cfg(test)] +mod tests { + use buzz_sdk_pkg::broker::{AgentsCreateArgs, CapabilityArgs}; + + use super::*; + + const OWNER: &str = "1111111111111111111111111111111111111111111111111111111111111111"; + const AGENT: &str = "2222222222222222222222222222222222222222222222222222222222222222"; + const STRANGER: &str = "3333333333333333333333333333333333333333333333333333333333333333"; + const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; + + struct Roster(Result, String>); + impl AgentRoster for Roster { + fn agent_pubkeys<'a>( + &'a self, + _owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>> { + let result = self.0.clone(); + Box::pin(async move { result }) + } + } + + fn request(requester: &str) -> VerifiedRequest { + VerifiedRequest { + owner_pubkey: OWNER.into(), + requester_pubkey: requester.into(), + relay_scope: "wss://relay.example".into(), + payload: Vec::new(), + } + } + + fn parsed() -> BrokerRequest { + BrokerRequest::new( + "req-1", + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ) + .unwrap() + } + + async fn authorize( + roster: Result, String>, + requester: &str, + capability: Capability, + ) -> Result<(), BrokerError> { + let authorizer = AgentsAuthorizer::new(Roster(roster)); + let request = request(requester); + let parsed = parsed(); + authorizer.authorize(&request, capability, &parsed).await + } + + #[tokio::test] + async fn an_owners_own_agent_may_manage_agents() { + for capability in [ + Capability::AgentsCreate, + Capability::AgentsUpdate, + Capability::AgentsDelete, + ] { + assert!(authorize(Ok(vec![AGENT.into()]), AGENT, capability) + .await + .is_ok()); + } + } + + #[tokio::test] + async fn a_signer_who_is_not_this_owners_agent_is_refused() { + let error = authorize(Ok(vec![AGENT.into()]), STRANGER, Capability::AgentsDelete) + .await + .unwrap_err(); + assert!( + error.message.contains("not an agent managed by this owner"), + "unexpected: {}", + error.message + ); + } + + #[tokio::test] + async fn an_empty_roster_refuses_everyone() { + assert!(authorize(Ok(Vec::new()), AGENT, Capability::AgentsCreate) + .await + .is_err()); + } + + /// The owner key signing its own broker request would erase the delegation + /// boundary the broker exists to enforce. + #[tokio::test] + async fn the_owner_key_is_not_a_requester() { + let error = authorize(Ok(vec![OWNER.into()]), OWNER, Capability::AgentsCreate) + .await + .unwrap_err(); + assert!( + error + .message + .contains("owner key is not a broker requester"), + "unexpected: {}", + error.message + ); + } + + /// A roster that cannot be read must fail closed. + #[tokio::test] + async fn an_unreadable_roster_fails_closed() { + let error = authorize( + Err("keyring locked".into()), + AGENT, + Capability::AgentsUpdate, + ) + .await + .unwrap_err(); + assert!( + error.message.contains("could not verify agent ownership"), + "unexpected: {}", + error.message + ); + } + + #[tokio::test] + async fn pubkey_comparison_ignores_hex_case() { + assert!(authorize( + Ok(vec![AGENT.to_ascii_uppercase()]), + AGENT, + Capability::AgentsCreate + ) + .await + .is_ok()); + } +} diff --git a/desktop/src-tauri/src/broker/desktop_agents.rs b/desktop/src-tauri/src/broker/desktop_agents.rs new file mode 100644 index 00000000000..e0169d26228 --- /dev/null +++ b/desktop/src-tauri/src/broker/desktop_agents.rs @@ -0,0 +1,390 @@ +//! The Desktop-backed [`AgentService`] and [`AgentRoster`]. +//! +//! This is the only place in the broker that knows about Tauri. It translates a +//! validated capability request into the same commands the UI calls — +//! `create_managed_agent`, `update_managed_agent`, `delete_managed_agent` — so a +//! brokered mutation and a hand-driven one take one code path. Reimplementing +//! the mutation here would mean two agent-creation routines drifting apart, and +//! the broker's would be the one nobody looks at. +//! +//! # Where the credential boundary sits +//! +//! Nothing here returns secret material. `create_managed_agent` hands back a +//! `CreateManagedAgentResponse` that *does* contain the minted +//! `private_key_nsec`; [`DesktopAgentService::create`] reads only the summary +//! and drops the response, and [`AgentsCreateOutcome`] has no field that could +//! hold it. That is the narrowing point: the secret exists on one stack frame +//! inside this file and never reaches the pipeline, the store, or the wire. +//! +//! # Why the request translation is a separate pure function +//! +//! [`create_request`] and [`update_request`] are pure and tested directly. +//! Everything they decide — which optional fields become `None`, that a +//! brokered create never spawns a process — is policy worth pinning in a test, +//! and none of it needs an `AppHandle`. + +use buzz_sdk_pkg::broker::{ + AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, + AgentsUpdateArgs, AgentsUpdateOutcome, +}; +use tauri::{AppHandle, Manager}; + +use crate::app_state::AppState; +use crate::managed_agents::{ + load_managed_agents, CreateManagedAgentRequest, ManagedAgentRecord, RespondTo, + UpdateManagedAgentRequest, DEFAULT_ACP_COMMAND, +}; + +use super::agents_policy::AgentRoster; +use super::handlers::{AgentService, ServiceError}; +use super::pipeline::BoxFuture; + +/// Build the create request the Desktop command expects. +/// +/// A brokered create is deliberately *not* spawned (`spawn_after_create: +/// false`) and does not auto-start (`start_on_app_launch: false`). A remote +/// requester asking for an agent to exist has not asked for a local process to +/// be running, and starting one would consume host resources on a schedule +/// nobody at the keyboard chose. The owner starts it from the UI. +/// +/// `persona_id` and `team_id` stay `None`: linking a definition pins a snapshot +/// of someone else's config onto the new agent, which is a second decision the +/// request never made. +fn create_request(args: &AgentsCreateArgs) -> Result { + let respond_to = args + .respond_to + .as_deref() + .map(RespondTo::parse_wire) + .transpose()?; + + Ok(CreateManagedAgentRequest { + name: args.display_name.clone(), + persona_id: None, + team_id: None, + relay_url: None, + acp_command: Some(DEFAULT_ACP_COMMAND.to_string()), + // `agent_command` is the harness pin. A requested runtime is resolved to + // its command here rather than passed through raw, so an arbitrary + // string can never become an executed command. + agent_command: args + .runtime + .as_deref() + .map(resolve_runtime_command) + .transpose()?, + harness_override: args.runtime.is_some(), + agent_args: Vec::new(), + mcp_command: None, + turn_timeout_seconds: None, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: None, + system_prompt: Some(args.system_prompt.clone()), + avatar_url: None, + model: args.model.clone(), + provider: args.provider.clone(), + env_vars: std::collections::BTreeMap::new(), + spawn_after_create: false, + start_on_app_launch: false, + backend: crate::managed_agents::BackendKind::Local, + respond_to, + respond_to_allowlist: Vec::new(), + relay_mesh: None, + }) +} + +/// Resolve a requested runtime id to its harness command. +/// +/// Refuses an id the catalog does not know instead of storing it: an +/// unresolvable pin produces an agent that cannot start, and the failure would +/// surface much later as a spawn error with no connection to this request. +fn resolve_runtime_command(runtime: &str) -> Result { + crate::managed_agents::command_for_runtime_id(runtime) + .ok_or_else(|| format!("unknown runtime \"{runtime}\"")) +} + +/// Build the update request, and report which fields it will change. +/// +/// The field list is derived from the request rather than from a diff of the +/// record, because the requester asked for these fields specifically. The +/// outcome names exactly what was asked for and applied. +fn update_request( + pubkey: &str, + args: &AgentsUpdateArgs, +) -> Result<(UpdateManagedAgentRequest, Vec), String> { + let mut changed = Vec::new(); + if args.display_name.is_some() { + changed.push("displayName".to_string()); + } + if args.system_prompt.is_some() { + changed.push("systemPrompt".to_string()); + } + if args.runtime.is_some() { + changed.push("runtime".to_string()); + } + if args.provider.is_some() { + changed.push("provider".to_string()); + } + if args.model.is_some() { + changed.push("model".to_string()); + } + if args.respond_to.is_some() { + changed.push("respondTo".to_string()); + } + changed.sort(); + + let respond_to = args + .respond_to + .as_deref() + .map(RespondTo::parse_wire) + .transpose()?; + + let request = UpdateManagedAgentRequest { + pubkey: pubkey.to_string(), + name: args.display_name.clone(), + // `Option>`: absent leaves the stored value alone. The broker + // has no "clear to default" verb, so it never sends `Some(None)` — a + // requester cannot blank a field it did not name. + model: args.model.clone().map(Some), + system_prompt: args.system_prompt.clone().map(Some), + env_vars: None, + parallelism: None, + turn_timeout_seconds: None, + relay_url: None, + acp_command: None, + agent_command: args + .runtime + .as_deref() + .map(resolve_runtime_command) + .transpose()?, + harness_override: args.runtime.is_some(), + agent_args: None, + mcp_command: None, + provider: args.provider.clone().map(Some), + respond_to, + // Allowlist mode is not reachable through this capability (the SDK + // rejects the mode), so there is never a list to replace. + respond_to_allowlist: None, + }; + Ok((request, changed)) +} + +/// Add `agent_pubkey` to `channel_id` as a bot member. +/// +/// The same owner-signed kind-9000 event `add_channel_members` publishes. It is +/// spelled out here rather than routed through that command because the command +/// swallows per-pubkey failures into a result array, and a broker create must +/// treat a failed attachment as a reportable outcome, not a field in a summary. +async fn attach_to_channel( + app: &AppHandle, + channel_id: &str, + agent_pubkey: &str, +) -> Result<(), String> { + let uuid = uuid::Uuid::parse_str(channel_id) + .map_err(|_| format!("invalid channel UUID: {channel_id}"))?; + let builder = crate::events::build_add_member(uuid, agent_pubkey, Some("bot"))?; + let state = app.state::(); + crate::relay::submit_event(builder, &state).await?; + Ok(()) +} + +/// Resolve a target selector against the owner's roster. +/// +/// Name matching is case-insensitive and must be unambiguous: two agents +/// sharing a name is a real state, and picking one arbitrarily would delete or +/// rewrite the wrong agent. +fn resolve_target( + records: &[ManagedAgentRecord], + target: &AgentTarget, +) -> Result { + match target { + AgentTarget::Pubkey(pubkey) => records + .iter() + .find(|record| record.pubkey.eq_ignore_ascii_case(pubkey)) + .cloned() + .ok_or_else(|| format!("no managed agent with pubkey {pubkey}")), + AgentTarget::Name(name) => { + let mut matches = records + .iter() + .filter(|record| record.name.eq_ignore_ascii_case(name)); + let first = matches + .next() + .cloned() + .ok_or_else(|| format!("no managed agent named \"{name}\""))?; + if matches.next().is_some() { + return Err(format!( + "more than one managed agent is named \"{name}\"; target it by pubkey" + )); + } + Ok(first) + } + } +} + +/// [`AgentService`] over the Desktop agent commands. +pub struct DesktopAgentService { + app: AppHandle, +} + +impl DesktopAgentService { + /// Bind the service to a running app. + pub fn new(app: AppHandle) -> Self { + Self { app } + } + + /// Snapshot the owner's agent records. + fn records(&self) -> Result, String> { + let state = self.app.state::(); + let _guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + load_managed_agents(&self.app) + } +} + +impl AgentService for DesktopAgentService { + fn create<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsCreateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let request = create_request(args).map_err(ServiceError::Failed)?; + let response = crate::commands::create_managed_agent( + request, + self.app.clone(), + self.app.state::(), + ) + .await + .map_err(ServiceError::Failed)?; + + let agent_pubkey = response.agent.pubkey.clone(); + let display_name = response.agent.name.clone(); + // The minted nsec dies with `response` at the end of this scope. + drop(response); + + // Attachment is a second, separately failable step. The agent + // already exists at this point, so a failure here is + // `Unknown`, not `Failed`: reporting "failed" would tell the + // requester nothing happened while an agent sits in the roster. + attach_to_channel(&self.app, &args.channel_id, &agent_pubkey) + .await + .map_err(|error| { + ServiceError::Unknown(format!( + "agent {agent_pubkey} was created but could not be added to channel \ + {}: {error}", + args.channel_id + )) + })?; + + Ok(AgentsCreateOutcome { + agent_pubkey, + display_name, + channel_id: args.channel_id.clone(), + }) + }) + } + + fn update<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsUpdateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let records = self.records().map_err(ServiceError::Failed)?; + let record = resolve_target(&records, &args.target).map_err(ServiceError::Failed)?; + let (request, updated_fields) = + update_request(&record.pubkey, args).map_err(ServiceError::Failed)?; + + let response = crate::commands::update_managed_agent( + request, + self.app.clone(), + self.app.state::(), + ) + .await + .map_err(ServiceError::Failed)?; + + Ok(AgentsUpdateOutcome { + agent_pubkey: response.agent.pubkey, + display_name: response.agent.name, + updated_fields, + }) + }) + } + + fn delete<'a>( + &'a self, + _owner_pubkey: &'a str, + args: &'a AgentsDeleteArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + let records = self.records().map_err(ServiceError::Failed)?; + let record = resolve_target(&records, &args.target).map_err(ServiceError::Failed)?; + let agent_pubkey = record.pubkey.clone(); + let display_name = record.name.clone(); + + // `force_remote_delete: None` keeps the deployed-remote guard in + // force. Orphaning provisioned remote infrastructure is a decision + // for a human looking at the warning, not something a brokered + // request may force. + crate::commands::delete_managed_agent(agent_pubkey.clone(), None, self.app.clone()) + .await + .map_err(ServiceError::Failed)?; + + Ok(AgentsDeleteOutcome { + agent_pubkey, + display_name, + }) + }) + } +} + +/// [`AgentRoster`] reading the authoritative Desktop agent store. +pub struct DesktopAgentRoster { + app: AppHandle, +} + +impl DesktopAgentRoster { + /// Bind the roster to a running app. + pub fn new(app: AppHandle) -> Self { + Self { app } + } +} + +impl AgentRoster for DesktopAgentRoster { + fn agent_pubkeys<'a>( + &'a self, + owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + // The store holds exactly one owner's agents — the owner whose keys + // this app instance holds. A request naming a different owner + // cannot be answered from here, and answering it from this roster + // anyway would authorize against the wrong set. + let state = self.app.state::(); + let host_owner = state + .keys + .lock() + .map_err(|error| error.to_string())? + .public_key() + .to_hex(); + if !host_owner.eq_ignore_ascii_case(owner_pubkey) { + return Err(format!( + "this host holds credentials for {host_owner}, not {owner_pubkey}" + )); + } + + let _guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + Ok(load_managed_agents(&self.app)? + .into_iter() + .map(|record| record.pubkey) + .collect()) + }) + } +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/desktop_agents/tests.rs b/desktop/src-tauri/src/broker/desktop_agents/tests.rs new file mode 100644 index 00000000000..a826e2f9cc6 --- /dev/null +++ b/desktop/src-tauri/src/broker/desktop_agents/tests.rs @@ -0,0 +1,176 @@ +//! Tests for request translation and target resolution. +//! +//! These cover the decisions this module makes on its own — which fields a +//! brokered mutation may touch, and which agent a selector resolves to. The +//! Tauri-calling parts are not covered here: they hold no logic beyond +//! forwarding and the `Failed`/`Unknown` split, and testing them would mean +//! booting an app to observe a call it already delegates. + +use buzz_sdk_pkg::broker::AgentTarget; + +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; + +fn create_args() -> AgentsCreateArgs { + AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help with things.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + } +} + +fn update_args() -> AgentsUpdateArgs { + AgentsUpdateArgs { + target: AgentTarget::Name("helper".into()), + display_name: None, + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + } +} + +/// Records are built from JSON, matching `managed_agents::reconcile::tests` — +/// `ManagedAgentRecord` has no `Default`, and enumerating its fifty-odd fields +/// here would break on every unrelated schema addition. +fn record(pubkey: &str, name: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "{name}", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "You are a test agent.", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"# + )) + .unwrap() +} + +/// A remote request for an agent to exist is not a request to run a process on +/// the owner's machine, now or at every launch. +#[test] +fn a_brokered_create_neither_spawns_nor_auto_starts() { + let request = create_request(&create_args()).unwrap(); + assert!(!request.spawn_after_create); + assert!(!request.start_on_app_launch); +} + +/// Linking a definition would pin someone else's config snapshot onto the new +/// agent — a decision the request never expressed. +#[test] +fn a_brokered_create_links_no_definition_or_team() { + let request = create_request(&create_args()).unwrap(); + assert!(request.persona_id.is_none()); + assert!(request.team_id.is_none()); + assert!(request.env_vars.is_empty()); + assert_eq!(request.backend, crate::managed_agents::BackendKind::Local); +} + +#[test] +fn create_carries_the_requested_name_and_prompt() { + let request = create_request(&create_args()).unwrap(); + assert_eq!(request.name, "Helper"); + assert_eq!(request.system_prompt.as_deref(), Some("Help with things.")); +} + +#[test] +fn create_translates_a_respond_to_mode() { + let mut args = create_args(); + args.respond_to = Some("anyone".into()); + let request = create_request(&args).unwrap(); + assert_eq!(request.respond_to, Some(RespondTo::Anyone)); +} + +/// An unresolvable runtime must fail here, not later as a spawn error with no +/// connection back to the request that caused it. +#[test] +fn create_refuses_a_runtime_the_catalog_does_not_know() { + let mut args = create_args(); + args.runtime = Some("not-a-real-runtime".into()); + let error = create_request(&args).unwrap_err(); + assert!(error.contains("unknown runtime"), "unexpected: {error}"); +} + +/// The broker has no "clear this field" verb, so an unnamed field must stay +/// absent rather than becoming an explicit null that wipes stored config. +#[test] +fn update_leaves_unrequested_fields_untouched() { + let (request, changed) = update_request("abc", &update_args()).unwrap(); + assert!(changed.is_empty()); + assert!(request.name.is_none()); + assert!(request.model.is_none()); + assert!(request.system_prompt.is_none()); + assert!(request.provider.is_none()); + assert!(request.respond_to.is_none()); + // Fields the capability does not expose at all. + assert!(request.env_vars.is_none()); + assert!(request.relay_url.is_none()); + assert!(request.agent_args.is_none()); + assert!(request.respond_to_allowlist.is_none()); +} + +#[test] +fn update_sets_only_the_named_fields_and_reports_them_sorted() { + let mut args = update_args(); + args.model = Some("some-model".into()); + args.display_name = Some("Renamed".into()); + let (request, changed) = update_request("abc", &args).unwrap(); + + assert_eq!(changed, vec!["displayName", "model"]); + assert_eq!(request.name.as_deref(), Some("Renamed")); + assert_eq!(request.model, Some(Some("some-model".to_string()))); + assert!(request.system_prompt.is_none()); +} + +#[test] +fn update_resolves_a_runtime_to_a_command_and_marks_it_a_pin() { + let mut args = update_args(); + args.runtime = Some("buzz-agent".into()); + let (request, changed) = update_request("abc", &args).unwrap(); + assert_eq!(changed, vec!["runtime"]); + assert!(request.agent_command.is_some()); + assert!(request.harness_override); +} + +#[test] +fn a_pubkey_target_resolves_case_insensitively() { + let records = vec![record("aabb", "helper")]; + let resolved = resolve_target(&records, &AgentTarget::Pubkey("AABB".into())).unwrap(); + assert_eq!(resolved.pubkey, "aabb"); +} + +#[test] +fn a_name_target_resolves_case_insensitively() { + let records = vec![record("aabb", "Helper")]; + let resolved = resolve_target(&records, &AgentTarget::Name("helper".into())).unwrap(); + assert_eq!(resolved.pubkey, "aabb"); +} + +/// Two agents can legitimately share a name. Picking one would mutate or delete +/// an agent the requester did not identify. +#[test] +fn an_ambiguous_name_is_refused_rather_than_guessed() { + let records = vec![record("aabb", "helper"), record("ccdd", "Helper")]; + let error = resolve_target(&records, &AgentTarget::Name("helper".into())).unwrap_err(); + assert!(error.contains("more than one"), "unexpected: {error}"); + assert!(error.contains("by pubkey"), "unexpected: {error}"); +} + +#[test] +fn a_missing_target_is_an_error() { + let records = vec![record("aabb", "helper")]; + assert!(resolve_target(&records, &AgentTarget::Name("ghost".into())).is_err()); + assert!(resolve_target(&records, &AgentTarget::Pubkey("ffff".into())).is_err()); +} diff --git a/desktop/src-tauri/src/broker/handlers.rs b/desktop/src-tauri/src/broker/handlers.rs new file mode 100644 index 00000000000..fcb0e3f0310 --- /dev/null +++ b/desktop/src-tauri/src/broker/handlers.rs @@ -0,0 +1,199 @@ +//! The three `agents.*` capability handlers. +//! +//! Each handler translates a validated broker request into a call on +//! [`AgentService`] and translates the result back into a +//! [`HandlerOutcome`]. They contain no policy (that is `agents_policy`), no +//! idempotency logic (that is `store`), and no transport concerns. +//! +//! # Why a trait instead of calling the commands directly +//! +//! The agent mutations live behind Tauri command signatures that need an +//! `AppHandle` and a `State`. Depending on those directly here would make every +//! handler untestable without booting an app, and would leave the capability +//! layer entangled with the desktop runtime. [`AgentService`] is the seam: the +//! handlers are pure and tested against a fake, and the Tauri-backed +//! implementation stays thin enough to read in one screen. + +use buzz_sdk_pkg::broker::{ + AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, AgentsUpdateArgs, + AgentsUpdateOutcome, BrokerError, BrokerErrorCode, BrokerRequest, Capability, CapabilityArgs, + CapabilityOutcome, +}; + +use super::pipeline::{ + BoxFuture, CapabilityHandler, CapabilityRegistry, HandlerOutcome, VerifiedRequest, +}; + +/// Why a domain mutation did not succeed. +/// +/// The distinction is the whole point: [`Self::Failed`] asserts that no side +/// effects persisted, while [`Self::Unknown`] asserts nothing. A service that +/// mutated and then lost track must return `Unknown` so the broker records +/// `indeterminate` instead of implying a clean failure. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ServiceError { + /// The operation did not take effect. + Failed(String), + /// Whether the operation took effect is unknown. + Unknown(String), +} + +impl ServiceError { + fn into_outcome(self) -> HandlerOutcome { + match self { + Self::Failed(message) => { + HandlerOutcome::Failed(BrokerError::new(BrokerErrorCode::CapabilityFailed, message)) + } + Self::Unknown(message) => HandlerOutcome::Indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + message, + )), + } + } +} + +/// The agent mutations the broker can perform on an owner's behalf. +/// +/// Implementations own credential access. Nothing in this trait returns secret +/// material: a create reports the new agent's public key, never its nsec. +/// +/// Async because the real implementation mints a key, writes the agent store, +/// and publishes to the relay. `Send + Sync` so a handler can be dispatched +/// from the async pipeline. +pub trait AgentService: Send + Sync { + /// Mint an agent and attach it to the requested channel. + fn create<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsCreateArgs, + ) -> BoxFuture<'a, Result>; + + /// Patch one of the owner's agents. + fn update<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsUpdateArgs, + ) -> BoxFuture<'a, Result>; + + /// Remove one of the owner's agents. + fn delete<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsDeleteArgs, + ) -> BoxFuture<'a, Result>; +} + +/// Dispatches the three `agents.*` capabilities to an [`AgentService`]. +pub struct AgentsHandlers { + service: S, +} + +impl AgentsHandlers { + /// Wrap `service`. + pub fn new(service: S) -> Self { + Self { service } + } +} + +/// Handler for one capability, borrowing the shared service. +struct Handler<'a, S> { + service: &'a S, + capability: Capability, +} + +impl CapabilityHandler for Handler<'_, S> { + fn execute<'a>( + &'a self, + request: &'a VerifiedRequest, + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, HandlerOutcome> { + Box::pin(async move { + // The pipeline validated the envelope, but the args it validated are + // the ones this handler must act on -- so it re-normalizes rather + // than trusting a separately parsed copy. + let args = match parsed.capability.validated() { + Ok(args) => args, + Err(error) => { + return HandlerOutcome::Failed(BrokerError::invalid_request(error.to_string())) + } + }; + + // A capability name that disagrees with its args is a routing bug, + // not a caller error, and must never reach a mutation. + if args.capability() != self.capability { + return HandlerOutcome::Failed(BrokerError::new( + BrokerErrorCode::Internal, + "broker routed a request to the wrong capability handler", + )); + } + + let owner = &request.owner_pubkey; + match &args { + CapabilityArgs::AgentsCreate(args) => { + match self.service.create(owner, args).await { + Ok(outcome) => { + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsCreate(outcome)) + } + Err(error) => error.into_outcome(), + } + } + CapabilityArgs::AgentsUpdate(args) => { + match self.service.update(owner, args).await { + Ok(outcome) => { + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsUpdate(outcome)) + } + Err(error) => error.into_outcome(), + } + } + CapabilityArgs::AgentsDelete(args) => { + match self.service.delete(owner, args).await { + Ok(outcome) => { + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsDelete(outcome)) + } + Err(error) => error.into_outcome(), + } + } + } + }) + } +} + +/// Registry over the three `agents.*` handlers. +pub struct AgentsRegistry<'a, S> { + create: Handler<'a, S>, + update: Handler<'a, S>, + delete: Handler<'a, S>, +} + +impl AgentsHandlers { + /// Build the registry this host offers. + pub fn registry(&self) -> AgentsRegistry<'_, S> { + AgentsRegistry { + create: Handler { + service: &self.service, + capability: Capability::AgentsCreate, + }, + update: Handler { + service: &self.service, + capability: Capability::AgentsUpdate, + }, + delete: Handler { + service: &self.service, + capability: Capability::AgentsDelete, + }, + } + } +} + +impl CapabilityRegistry for AgentsRegistry<'_, S> { + fn handler(&self, capability: Capability) -> Option<&dyn CapabilityHandler> { + Some(match capability { + Capability::AgentsCreate => &self.create, + Capability::AgentsUpdate => &self.update, + Capability::AgentsDelete => &self.delete, + }) + } +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/handlers/tests.rs b/desktop/src-tauri/src/broker/handlers/tests.rs new file mode 100644 index 00000000000..9bec4ab386d --- /dev/null +++ b/desktop/src-tauri/src/broker/handlers/tests.rs @@ -0,0 +1,355 @@ +//! Handler tests, driven through the real pipeline against a fake service. + +use std::sync::Mutex; + +use buzz_sdk_pkg::broker::{AgentTarget, BrokerResponse, BrokerResult}; + +use super::super::agents_policy::{AgentRoster, AgentsAuthorizer}; +use super::super::pipeline::{execute, BrokerContext}; +use super::super::store::MemoryExecutionLog; +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const OWNER: &str = "1111111111111111111111111111111111111111111111111111111111111111"; +const REQUESTER: &str = "2222222222222222222222222222222222222222222222222222222222222222"; +const AGENT: &str = "3333333333333333333333333333333333333333333333333333333333333333"; + +/// Records every call so a test can assert what the domain layer was asked to +/// do -- and that it was asked exactly once. +/// +/// `Mutex` rather than `RefCell` because [`AgentService`] is `Send + Sync`: the +/// pipeline dispatches handlers from async code, so a fake has to be shareable +/// the same way a real service is. +#[derive(Default)] +struct FakeService { + calls: Mutex>, + create: Option>, + update: Option>, + delete: Option>, +} + +impl FakeService { + fn creating() -> Self { + Self { + create: Some(Ok(AgentsCreateOutcome { + agent_pubkey: AGENT.into(), + display_name: "Helper".into(), + channel_id: CHANNEL.into(), + })), + ..Self::default() + } + } + + fn record(&self, call: String) { + self.calls.lock().expect("fake service poisoned").push(call); + } + + fn calls(&self) -> Vec { + self.calls.lock().expect("fake service poisoned").clone() + } +} + +impl AgentService for FakeService { + fn create<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsCreateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + self.record(format!("create:{owner_pubkey}:{}", args.display_name)); + self.create + .clone() + .unwrap_or(Err(ServiceError::Failed("no create configured".into()))) + }) + } + + fn update<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsUpdateArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + self.record(format!("update:{owner_pubkey}:{:?}", args.target)); + self.update + .clone() + .unwrap_or(Err(ServiceError::Failed("no update configured".into()))) + }) + } + + fn delete<'a>( + &'a self, + owner_pubkey: &'a str, + args: &'a AgentsDeleteArgs, + ) -> BoxFuture<'a, Result> { + Box::pin(async move { + self.record(format!("delete:{owner_pubkey}:{:?}", args.target)); + self.delete + .clone() + .unwrap_or(Err(ServiceError::Failed("no delete configured".into()))) + }) + } +} + +struct OwnerRoster; +impl AgentRoster for OwnerRoster { + fn agent_pubkeys<'a>( + &'a self, + _owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async { Ok(vec![REQUESTER.into()]) }) + } +} + +async fn run(payload: Vec, service: FakeService) -> (BrokerResponse, Vec) { + let handlers = AgentsHandlers::new(service); + let registry = handlers.registry(); + let authorizer = AgentsAuthorizer::new(OwnerRoster); + let log = MemoryExecutionLog::new(); + + let request = VerifiedRequest { + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + relay_scope: "wss://relay.example".into(), + payload, + }; + let response = execute( + &request, + BrokerContext { + authorizer: &authorizer, + registry: ®istry, + log: &log, + now: 1000, + }, + ) + .await + .unwrap(); + let calls = handlers.service.calls(); + (response, calls) +} + +fn payload(args: CapabilityArgs) -> Vec { + serde_json::to_vec(&BrokerRequest::new("req-1", args).unwrap()).unwrap() +} + +fn create_args() -> CapabilityArgs { + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }) +} + +#[tokio::test] +async fn create_reaches_the_service_and_reports_the_new_agent() { + let (response, calls) = run(payload(create_args()), FakeService::creating()).await; + + match response.result { + BrokerResult::Succeeded { + outcome: CapabilityOutcome::AgentsCreate(outcome), + } => { + assert_eq!(outcome.agent_pubkey, AGENT); + assert_eq!(outcome.channel_id, CHANNEL); + } + other => panic!("expected create success, got {other:?}"), + } + assert_eq!(calls, vec![format!("create:{OWNER}:Helper")]); +} + +/// The owner the service acts for comes from the verified transport, not the +/// payload -- so a handler can never be aimed at another owner's agents. +#[tokio::test] +async fn the_service_receives_the_transport_derived_owner() { + let (_, calls) = run(payload(create_args()), FakeService::creating()).await; + assert!(calls[0].contains(OWNER), "unexpected call: {}", calls[0]); +} + +#[tokio::test] +async fn update_and_delete_route_to_their_own_service_methods() { + let update = FakeService { + update: Some(Ok(AgentsUpdateOutcome { + agent_pubkey: AGENT.into(), + display_name: "Renamed".into(), + updated_fields: vec!["displayName".into()], + })), + ..FakeService::default() + }; + let (response, calls) = run( + payload(CapabilityArgs::AgentsUpdate(AgentsUpdateArgs { + target: AgentTarget::Pubkey(AGENT.into()), + display_name: Some("Renamed".into()), + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + })), + update, + ) + .await; + assert!(response.result.is_succeeded()); + assert!(calls[0].starts_with("update:"), "unexpected: {}", calls[0]); + + let delete = FakeService { + delete: Some(Ok(AgentsDeleteOutcome { + agent_pubkey: AGENT.into(), + display_name: "Gone".into(), + })), + ..FakeService::default() + }; + let (response, calls) = run( + payload(CapabilityArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(AGENT.into()), + })), + delete, + ) + .await; + assert!(response.result.is_succeeded()); + assert!(calls[0].starts_with("delete:"), "unexpected: {}", calls[0]); +} + +/// A service that failed cleanly must produce `failed`, which promises no side +/// effects took hold. +#[tokio::test] +async fn a_clean_service_failure_becomes_failed() { + let service = FakeService { + create: Some(Err(ServiceError::Failed("runtime not installed".into()))), + ..FakeService::default() + }; + let (response, _) = run(payload(create_args()), service).await; + + match &response.result { + BrokerResult::Failed { error } => { + assert_eq!(error.code, BrokerErrorCode::CapabilityFailed); + assert!(error.message.contains("runtime not installed")); + } + other => panic!("expected failed, got {other:?}"), + } +} + +/// A service that cannot tell whether it mutated must produce `indeterminate`, +/// never `failed` -- the difference is what stops a caller from safely retrying +/// something that may already have happened. +#[tokio::test] +async fn an_unknown_service_outcome_becomes_indeterminate_not_failed() { + let service = FakeService { + create: Some(Err(ServiceError::Unknown( + "agent minted but profile publish unconfirmed".into(), + ))), + ..FakeService::default() + }; + let (response, _) = run(payload(create_args()), service).await; + + match &response.result { + BrokerResult::Indeterminate { error } => { + assert_eq!(error.code, BrokerErrorCode::OutcomeUnknown); + } + other => panic!("expected indeterminate, got {other:?}"), + } +} + +/// A requester that is not one of the owner's agents must never reach the +/// service at all. +#[tokio::test] +async fn an_unauthorized_requester_never_reaches_the_service() { + struct EmptyRoster; + impl AgentRoster for EmptyRoster { + fn agent_pubkeys<'a>( + &'a self, + _owner: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async { Ok(Vec::new()) }) + } + } + + let handlers = AgentsHandlers::new(FakeService::creating()); + let registry = handlers.registry(); + let authorizer = AgentsAuthorizer::new(EmptyRoster); + let log = MemoryExecutionLog::new(); + + let request = VerifiedRequest { + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + relay_scope: "wss://relay.example".into(), + payload: payload(create_args()), + }; + let response = execute( + &request, + BrokerContext { + authorizer: &authorizer, + registry: ®istry, + log: &log, + now: 1000, + }, + ) + .await + .unwrap(); + + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::Unauthorized + ); + assert!( + handlers.service.calls().is_empty(), + "an unauthorized request must not touch the domain layer" + ); +} + +/// End-to-end idempotency through the real handler stack: the domain mutation +/// must run once even though the request arrives twice. +#[tokio::test] +async fn a_replayed_request_does_not_mutate_twice() { + let handlers = AgentsHandlers::new(FakeService::creating()); + let registry = handlers.registry(); + let authorizer = AgentsAuthorizer::new(OwnerRoster); + let log = MemoryExecutionLog::new(); + + let request = VerifiedRequest { + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + relay_scope: "wss://relay.example".into(), + payload: payload(create_args()), + }; + let run_once = || async { + execute( + &request, + BrokerContext { + authorizer: &authorizer, + registry: ®istry, + log: &log, + now: 1000, + }, + ) + .await + .unwrap() + }; + + let first = run_once().await; + let second = run_once().await; + + assert!(first.result.is_succeeded()); + assert_eq!(first.result, second.result); + assert!(second.replayed); + assert_eq!( + handlers.service.calls().len(), + 1, + "the domain mutation must run exactly once" + ); +} + +/// A create outcome cannot carry the minted secret, by construction. +#[tokio::test] +async fn a_create_outcome_never_carries_the_minted_nsec() { + let (response, _) = run(payload(create_args()), FakeService::creating()).await; + let json = serde_json::to_string(&response).unwrap(); + assert!(!json.contains("nsec"), "response leaked a secret: {json}"); + for forbidden in ["privateKey", "private_key", "secret"] { + assert!( + !json.contains(forbidden), + "response leaked {forbidden}: {json}" + ); + } +} diff --git a/desktop/src-tauri/src/broker/host.rs b/desktop/src-tauri/src/broker/host.rs new file mode 100644 index 00000000000..0643e0d2839 --- /dev/null +++ b/desktop/src-tauri/src/broker/host.rs @@ -0,0 +1,178 @@ +//! Wiring the broker into the running Desktop app. +//! +//! Everything above this file is testable without Tauri. This is where the +//! host's real parts get attached: the owner's keys, the agent roster, the agent +//! commands, the on-disk execution log, and the relay. +//! +//! # The command is a transport hand-off, not an authority hand-off +//! +//! [`broker_handle_observer_frame`] takes a raw signed event and nothing else. +//! The frontend cannot tell it who is asking, which owner to act as, or whether +//! a request is authorized — those come from the frame's signature and from +//! `AppState`. All the frontend does is forward bytes it already received on its +//! observer subscription, and it learns only whether the frame was broker +//! traffic. That keeps the trust boundary next to the credentials while reusing +//! the subscription that already exists. + +use std::path::PathBuf; + +use nostr::{Event, JsonUtil, Keys}; +use tauri::{AppHandle, Manager, State}; + +use crate::app_state::AppState; + +use super::agents_policy::AgentsAuthorizer; +use super::desktop_agents::{DesktopAgentRoster, DesktopAgentService}; +use super::handlers::AgentsHandlers; +use super::ingress::{handle_frame, FrameRejection, Handled, ResultPublisher}; +use super::pipeline::{BoxFuture, BrokerContext}; +use super::store::{scoped_broker_db_path, SqliteExecutionLog}; + +/// Seconds allowed for a result frame to reach the relay and be acknowledged. +const RESULT_PUBLISH_TIMEOUT_SECS: u64 = 30; + +/// Publishes result frames over the relay WebSocket. +/// +/// Kind-24200 frames are only accepted on the relay's WebSocket path — the HTTP +/// bridge that every other Desktop publish uses rejects the kind outright — so +/// this opens a short-lived authenticated connection per result rather than +/// reusing `relay::submit_event`. Results are rare (one per brokered mutation), +/// which is what makes a per-result connection acceptable; a busier capability +/// would want a shared session instead. +pub struct WsResultPublisher { + relay_url: String, + keys: Keys, +} + +impl WsResultPublisher { + /// Publish as `keys` against `relay_url`. + pub fn new(relay_url: String, keys: Keys) -> Self { + Self { relay_url, keys } + } +} + +impl ResultPublisher for WsResultPublisher { + fn publish(&self, event: Event) -> BoxFuture<'_, Result<(), String>> { + Box::pin(async move { + buzz_ws_client_pkg::publish_event( + &self.relay_url, + event, + &self.keys, + // No NIP-OA auth tag: this publishes as the owner's own + // identity, which the relay authenticates directly. An auth tag + // is how a *managed agent* proves owner backing, and the owner + // is not one. + None, + RESULT_PUBLISH_TIMEOUT_SECS, + ) + .await + .map_err(|error| format!("failed to publish broker result: {error}"))?; + Ok(()) + }) + } +} + +/// What the caller learns about a forwarded frame. +/// +/// Deliberately thin. A requester's outcome travels to the requester in a signed +/// result frame; telling the forwarding renderer what a brokered operation did +/// would make the reply path look like an IPC return value when it is not. +#[derive(Debug, Clone, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct FrameHandled { + /// True when the frame was a broker request this host executed. + pub broker_request: bool, + /// Correlation id, when there was a parseable one. + pub request_id: Option, + /// True when the result frame reached the relay. + /// + /// `false` with `broker_request: true` means the operation ran and its + /// outcome is recorded, but the requester did not hear it. A retry with the + /// same `requestId` replays the recorded outcome rather than re-running. + pub result_delivered: bool, +} + +/// Resolve the execution log path for this owner and relay. +fn execution_log(app: &AppHandle, relay_url: &str, owner_pubkey: &str) -> Result { + let base = app + .path() + .app_data_dir() + .map_err(|error| format!("failed to resolve app data dir: {error}"))?; + Ok(scoped_broker_db_path(&base, relay_url, owner_pubkey)) +} + +/// Execute a broker request carried by an inbound observer frame. +/// +/// `event_json` is a signed relay event the caller received on its observer +/// subscription. Frames that are not broker requests are ignored cheaply; that +/// is the common case, since ordinary agent telemetry arrives here too. +/// +/// # Errors +/// +/// Returns an error for a host fault: unavailable owner keys, an unreadable +/// execution log, or an event that will not parse. A frame that fails +/// verification is not an error — it is reported as `brokerRequest: false`, +/// because a signed refusal addressed at an unverified sender is exactly what +/// this must not emit. +#[tauri::command] +pub async fn broker_handle_observer_frame( + event_json: String, + app: AppHandle, + state: State<'_, AppState>, +) -> Result { + let event = Event::from_json(&event_json) + .map_err(|error| format!("invalid observer event: {error}"))?; + let keys = state.signing_keys()?; + let relay_url = crate::relay::relay_ws_url_with_override(&state); + let owner_pubkey = keys.public_key().to_hex(); + + let log = SqliteExecutionLog::new(execution_log(&app, &relay_url, &owner_pubkey)?); + let authorizer = AgentsAuthorizer::new(DesktopAgentRoster::new(app.clone())); + let handlers = AgentsHandlers::new(DesktopAgentService::new(app.clone())); + let registry = handlers.registry(); + let publisher = WsResultPublisher::new(relay_url.clone(), keys.clone()); + let now = chrono::Utc::now().timestamp(); + + let handled = handle_frame( + &keys, + &event, + &relay_url, + now, + BrokerContext { + authorizer: &authorizer, + registry: ®istry, + log: &log, + now, + }, + &publisher, + ) + .await?; + + Ok(match handled { + Handled::Ignored(rejection) => { + if let FrameRejection::Rejected(message) = &rejection { + // A frame that claimed to be broker traffic and failed + // verification is worth seeing; ordinary telemetry is not. + eprintln!("buzz-desktop: broker: refused an inbound frame: {message}"); + } + FrameHandled { + broker_request: false, + request_id: None, + result_delivered: false, + } + } + Handled::Executed { response, delivery } => { + if let Err(error) = &delivery { + eprintln!( + "buzz-desktop: broker: result for {} was not delivered: {error}", + response.request_id + ); + } + FrameHandled { + broker_request: true, + request_id: Some(response.request_id.clone()), + result_delivered: delivery.is_ok(), + } + } + }) +} diff --git a/desktop/src-tauri/src/broker/ingress.rs b/desktop/src-tauri/src/broker/ingress.rs new file mode 100644 index 00000000000..5e114b1d162 --- /dev/null +++ b/desktop/src-tauri/src/broker/ingress.rs @@ -0,0 +1,323 @@ +//! Broker ingress: a verified frame in, a signed result out. +//! +//! This module owns the two transport edges the pipeline deliberately does not: +//! turning a received observer frame into a [`VerifiedRequest`], and turning the +//! resulting [`BrokerResponse`] into a frame addressed back to the requester. +//! +//! ```text +//! signed frame → verify → decrypt → pipeline::execute → encrypt → signed result +//! ``` +//! +//! # Why the request is the whole plaintext +//! +//! A broker request is the *entire* decrypted frame payload — a bare +//! [`BrokerRequest`] JSON object. It is not wrapped in the ACP observer +//! envelope, and that is a load-bearing choice: the envelope carries per-attempt +//! fields (`seq`, `timestamp`), and the idempotency digest is a hash of the +//! bytes as received. Hashing an envelope would make a legitimate retry — same +//! `requestId`, same operation, a second later — hash differently and be refused +//! as a request-ID conflict. Keeping the envelope out of the plaintext means the +//! only way the digest changes is if the request itself changed. +//! +//! Frame-level identity (`created_at`, event id, signature) varies freely +//! between attempts and is *outside* the hashed bytes by construction. +//! +//! # Why a rejected frame is silent +//! +//! [`verify_frame`] failures produce no result frame. A frame that fails +//! verification has no trustworthy requester to address a reply to, and +//! answering one anyway would let anybody make this host emit signed frames at a +//! target of their choosing. Only a frame that verified — and that actually +//! claims to be a broker request — can be answered. + +use buzz_core_pkg::observer::{ + decrypt_observer_plaintext, encrypt_observer_payload, OBSERVER_AGENT_TAG, + OBSERVER_FRAME_CONTROL, OBSERVER_FRAME_TAG, OBSERVER_FRAME_TELEMETRY, +}; +use buzz_sdk_pkg::broker::{BrokerResponse, BROKER_REQUEST_TYPE}; +use buzz_sdk_pkg::kind::KIND_AGENT_OBSERVER_FRAME; +use nostr::{Event, Keys, Kind, PublicKey}; +use zeroize::Zeroize; + +use super::pipeline::{execute, BoxFuture, BrokerContext, VerifiedRequest}; +use super::MAX_REQUEST_BYTES; + +/// Largest accepted clock skew between a frame and this host, in seconds. +/// +/// Matches the relay's observer-frame freshness window. Enforced again here +/// because the relay's check protects the relay: a host that trusted it would be +/// trusting a hop it does not control to bound replay of an old signed frame. +pub const MAX_FRAME_SKEW_SECS: i64 = 300; + +/// Why an inbound frame is not a broker request this host will act on. +/// +/// [`Self::NotBrokerTraffic`] is the ordinary case, not an error: every observer +/// telemetry frame an owner receives reaches this check, and almost none are +/// broker requests. It is separated from [`Self::Rejected`] so a caller can log +/// a real refusal without logging every agent heartbeat. +#[derive(Debug, PartialEq, Eq)] +pub enum FrameRejection { + /// The frame is well-formed observer traffic that is not a broker request. + NotBrokerTraffic, + /// The frame claims to be broker traffic but failed verification. + Rejected(String), +} + +impl FrameRejection { + fn rejected(message: impl Into) -> Self { + Self::Rejected(message.into()) + } +} + +/// Verify an inbound observer frame and decrypt it into a [`VerifiedRequest`]. +/// +/// Every field of the returned request is derived from the frame this host +/// verified, never from the request body: +/// +/// - `owner_pubkey` is *this host's own* public key. It is not read from the +/// frame at all, so no signer can name a different owner. +/// - `requester_pubkey` is the verified signer. +/// - `relay_scope` is the relay the caller received the frame on. +/// +/// # Errors +/// +/// Returns [`FrameRejection::NotBrokerTraffic`] for a frame that is not a broker +/// request, and [`FrameRejection::Rejected`] when a frame that claims to be one +/// fails verification: bad id or signature, wrong kind, missing or duplicated +/// routing tags, a direction other than requester-to-owner telemetry, a +/// timestamp outside [`MAX_FRAME_SKEW_SECS`], an undecryptable body, or a +/// payload larger than [`MAX_REQUEST_BYTES`]. +pub fn verify_frame( + owner_keys: &Keys, + event: &Event, + relay_scope: &str, + now: i64, +) -> Result { + if event.kind != Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16) { + return Err(FrameRejection::NotBrokerTraffic); + } + // Cheap structural gates run before the expensive ones, but nothing that + // could admit a frame is skipped: id and signature are checked before the + // content is decrypted, and the content is bounded before it is parsed. + if single_tag(event, OBSERVER_FRAME_TAG).as_deref() != Some(OBSERVER_FRAME_TELEMETRY) { + return Err(FrameRejection::NotBrokerTraffic); + } + + if !event.verify_id() { + return Err(FrameRejection::rejected("observer frame has an invalid id")); + } + if !event.verify_signature() { + return Err(FrameRejection::rejected( + "observer frame has an invalid signature", + )); + } + + let skew = event.created_at.as_secs() as i64 - now; + if skew.abs() > MAX_FRAME_SKEW_SECS { + return Err(FrameRejection::rejected(format!( + "observer frame timestamp is {skew}s from now, outside the ±{MAX_FRAME_SKEW_SECS}s window" + ))); + } + + let owner = owner_keys.public_key(); + let recipient = single_pubkey_tag(event, "p") + .ok_or_else(|| FrameRejection::rejected("observer frame has no single p tag"))?; + if recipient != owner { + // Addressed to a different owner. This host holds no credentials that + // could act on it, and cannot decrypt it either. + return Err(FrameRejection::NotBrokerTraffic); + } + + let agent = single_pubkey_tag(event, OBSERVER_AGENT_TAG) + .ok_or_else(|| FrameRejection::rejected("observer frame has no single agent tag"))?; + // Requester-to-owner telemetry only: the signer must be the agent the frame + // names. A frame signed by anyone else claiming to be an agent's telemetry + // would let a third party borrow that agent's authorization. + if event.pubkey != agent { + return Err(FrameRejection::rejected( + "observer frame signer is not the agent it names", + )); + } + if agent == owner { + // The owner key signing owner-addressed telemetry is not a delegated + // request; policy refuses it too, but it never becomes a request here. + return Err(FrameRejection::NotBrokerTraffic); + } + + let mut plaintext = decrypt_observer_plaintext(owner_keys, event) + .map_err(|error| FrameRejection::rejected(format!("frame decrypt failed: {error}")))?; + let outcome = classify_plaintext(&plaintext); + let verified = match outcome { + Ok(()) => Ok(VerifiedRequest { + owner_pubkey: owner.to_hex(), + requester_pubkey: event.pubkey.to_hex(), + relay_scope: relay_scope.to_string(), + payload: plaintext.as_bytes().to_vec(), + }), + Err(rejection) => Err(rejection), + }; + plaintext.zeroize(); + verified +} + +/// Decide whether a decrypted payload is a broker request worth dispatching. +/// +/// Deliberately does not validate the request: an ill-formed *broker* request +/// must reach the pipeline so the requester gets a `invalid_request` answer, +/// while a payload that never claimed to be one must not be answered at all. +fn classify_plaintext(plaintext: &str) -> Result<(), FrameRejection> { + if plaintext.len() > MAX_REQUEST_BYTES { + return Err(FrameRejection::rejected(format!( + "broker request payload exceeds {MAX_REQUEST_BYTES} bytes (got {})", + plaintext.len() + ))); + } + let peeked: serde_json::Value = + serde_json::from_str(plaintext).map_err(|_| FrameRejection::NotBrokerTraffic)?; + if peeked.get("type").and_then(serde_json::Value::as_str) == Some(BROKER_REQUEST_TYPE) { + Ok(()) + } else { + Err(FrameRejection::NotBrokerTraffic) + } +} + +fn single_tag(event: &Event, name: &str) -> Option { + let mut values = event + .tags + .iter() + .filter(|tag| tag.kind().to_string() == name) + .filter_map(|tag| tag.content()); + let value = values.next()?; + // A duplicated routing tag is ambiguous, and picking one would let a frame + // present a different route to this host than it did to the relay. + if values.next().is_some() { + return None; + } + Some(value.to_string()) +} + +fn single_pubkey_tag(event: &Event, name: &str) -> Option { + PublicKey::from_hex(&single_tag(event, name)?).ok() +} + +/// Build the signed frame that carries `response` back to `requester`. +/// +/// The result travels as an owner-to-agent *control* frame: encrypted to the +/// requester, signed by the owner, addressed with the requester as both +/// recipient and agent. That is the only direction the relay routes owner-signed +/// observer traffic in, and it means a result is readable by exactly the agent +/// that asked for it. +/// +/// # Errors +/// +/// Returns an error if `requester` is not a valid hex pubkey, or if the response +/// cannot be encrypted, built, or signed. +pub fn build_result_frame( + owner_keys: &Keys, + requester: &str, + response: &BrokerResponse, +) -> Result { + let requester = PublicKey::from_hex(requester.trim()) + .map_err(|error| format!("invalid requester pubkey: {error}"))?; + let requester_hex = requester.to_hex(); + let encrypted = encrypt_observer_payload(owner_keys, &requester, response) + .map_err(|error| format!("failed to encrypt broker result: {error}"))?; + buzz_sdk_pkg::build_agent_observer_frame( + &requester_hex, + &requester_hex, + OBSERVER_FRAME_CONTROL, + &encrypted, + ) + .map_err(|error| format!("failed to build broker result frame: {error}"))? + .sign_with_keys(owner_keys) + .map_err(|error| format!("failed to sign broker result frame: {error}")) +} + +/// Delivers a signed result frame to the relay. +/// +/// A trait because kind-24200 frames are only accepted over the relay's +/// WebSocket path — the HTTP bridge rejects the kind outright — so delivery is a +/// different transport from every other Desktop publish. Isolating it here keeps +/// that fact in one place and lets the ingress path be tested without a relay. +pub trait ResultPublisher: Send + Sync { + /// Publish `event`, or report why delivery failed. + fn publish(&self, event: Event) -> BoxFuture<'_, Result<(), String>>; +} + +/// The outcome of handling one inbound frame. +#[derive(Debug)] +pub enum Handled { + /// The frame was not a broker request for this host. + Ignored(FrameRejection), + /// The request produced `response`, and delivery reported `delivery`. + /// + /// `delivery` is separate from the response on purpose: a request can + /// execute successfully and still fail to deliver its result. That must not + /// be reported as an execution failure, and it must not retry the execution + /// — the recorded outcome is already durable and a retry with the same + /// `requestId` replays it. + Executed { + /// The terminal response the pipeline produced. + response: Box, + /// Whether the result frame reached the relay. + delivery: Result<(), String>, + }, +} + +/// Verify, execute, and answer one inbound frame. +/// +/// # Errors +/// +/// Returns `Err` only for a host fault that leaves no deliverable response — +/// an unreachable execution log, for instance. Refusals the requester should +/// hear about come back inside [`Handled::Executed`]. +pub async fn handle_frame( + owner_keys: &Keys, + event: &Event, + relay_scope: &str, + now: i64, + ctx: BrokerContext<'_>, + publisher: &dyn ResultPublisher, +) -> Result { + let request = match verify_frame(owner_keys, event, relay_scope, now) { + Ok(request) => request, + Err(rejection) => return Ok(Handled::Ignored(rejection)), + }; + + let requester = request.requester_pubkey.clone(); + let response = execute(&request, ctx).await?; + + // Audit line: identities, capability scope, and disposition. No prompts, no + // arguments, no decrypted payload — an audit trail that leaks the thing it + // audits is worse than none. + eprintln!( + "buzz-desktop: broker: request={} requester={} scope={} disposition={} replayed={}", + response.request_id, + requester, + relay_scope, + disposition(&response), + response.replayed + ); + + let delivery = match build_result_frame(owner_keys, &requester, &response) { + Ok(frame) => publisher.publish(frame).await, + Err(error) => Err(error), + }; + Ok(Handled::Executed { + response: Box::new(response), + delivery, + }) +} + +/// Audit word for a response's terminal disposition. +fn disposition(response: &BrokerResponse) -> &'static str { + use buzz_sdk_pkg::broker::BrokerResult; + match response.result { + BrokerResult::Succeeded { .. } => "succeeded", + BrokerResult::Failed { .. } => "failed", + BrokerResult::Indeterminate { .. } => "indeterminate", + } +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/ingress/tests.rs b/desktop/src-tauri/src/broker/ingress/tests.rs new file mode 100644 index 00000000000..be265387ab3 --- /dev/null +++ b/desktop/src-tauri/src/broker/ingress/tests.rs @@ -0,0 +1,614 @@ +//! Ingress tests: frame verification, the digest boundary, and result delivery. + +use std::sync::Mutex; + +use buzz_sdk_pkg::broker::{ + AgentsCreateArgs, AgentsCreateOutcome, BrokerErrorCode, BrokerRequest, BrokerResult, + Capability, CapabilityArgs, CapabilityOutcome, +}; +use nostr::{EventBuilder, Tag, Timestamp}; + +use super::super::pipeline::{Authorizer, CapabilityHandler, CapabilityRegistry, HandlerOutcome}; +use super::super::store::MemoryExecutionLog; +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const RELAY: &str = "wss://relay.example"; +const NOW: i64 = 1_700_000_000; + +fn request(request_id: &str) -> BrokerRequest { + BrokerRequest::new( + request_id, + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ) + .expect("valid request") +} + +/// Seal `payload` as requester-to-owner telemetry, the shape a real request has. +fn frame(requester: &Keys, owner: &Keys, payload: &serde_json::Value, created_at: i64) -> Event { + let encrypted = + encrypt_observer_payload(requester, &owner.public_key(), payload).expect("encrypt payload"); + EventBuilder::new(Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16), encrypted) + .tags([ + Tag::parse(["p", &owner.public_key().to_hex()]).expect("p tag"), + Tag::parse([OBSERVER_AGENT_TAG, &requester.public_key().to_hex()]).expect("agent tag"), + Tag::parse([OBSERVER_FRAME_TAG, OBSERVER_FRAME_TELEMETRY]).expect("frame tag"), + ]) + .custom_created_at(Timestamp::from(created_at as u64)) + .sign_with_keys(requester) + .expect("sign frame") +} + +fn request_frame(requester: &Keys, owner: &Keys, request: &BrokerRequest) -> Event { + frame( + requester, + owner, + &serde_json::to_value(request).expect("request json"), + NOW, + ) +} + +struct AllowAll; +impl Authorizer for AllowAll { + fn authorize<'a>( + &'a self, + _request: &'a VerifiedRequest, + _capability: Capability, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), buzz_sdk_pkg::broker::BrokerError>> { + Box::pin(async { Ok(()) }) + } +} + +/// Records every execution so a test can prove a retry did not re-run one. +struct RecordingHandler { + calls: Mutex>, +} + +impl RecordingHandler { + fn new() -> Self { + Self { + calls: Mutex::new(Vec::new()), + } + } + + fn calls(&self) -> usize { + self.calls.lock().expect("calls").len() + } +} + +impl CapabilityHandler for RecordingHandler { + fn execute<'a>( + &'a self, + _request: &'a VerifiedRequest, + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, HandlerOutcome> { + Box::pin(async move { + self.calls + .lock() + .expect("calls") + .push(parsed.request_id.clone()); + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: "44".repeat(32), + display_name: "Helper".into(), + channel_id: CHANNEL.into(), + })) + }) + } +} + +struct OneHandler<'a>(&'a RecordingHandler); +impl CapabilityRegistry for OneHandler<'_> { + fn handler(&self, _capability: Capability) -> Option<&dyn CapabilityHandler> { + Some(self.0) + } +} + +/// Captures published frames instead of reaching a relay. +struct CapturingPublisher { + published: Mutex>, + fail: Option, +} + +impl CapturingPublisher { + fn new() -> Self { + Self { + published: Mutex::new(Vec::new()), + fail: None, + } + } + + fn failing(message: &str) -> Self { + Self { + published: Mutex::new(Vec::new()), + fail: Some(message.to_string()), + } + } + + fn frames(&self) -> Vec { + self.published.lock().expect("published").clone() + } +} + +impl ResultPublisher for CapturingPublisher { + fn publish(&self, event: Event) -> BoxFuture<'_, Result<(), String>> { + Box::pin(async move { + self.published.lock().expect("published").push(event); + match &self.fail { + Some(message) => Err(message.clone()), + None => Ok(()), + } + }) + } +} + +struct Harness { + owner: Keys, + requester: Keys, + log: MemoryExecutionLog, + handler: RecordingHandler, +} + +impl Harness { + fn new() -> Self { + Self { + owner: Keys::generate(), + requester: Keys::generate(), + log: MemoryExecutionLog::new(), + handler: RecordingHandler::new(), + } + } + + async fn handle(&self, event: &Event, publisher: &dyn ResultPublisher) -> Handled { + let registry = OneHandler(&self.handler); + handle_frame( + &self.owner, + event, + RELAY, + NOW, + BrokerContext { + authorizer: &AllowAll, + registry: ®istry, + log: &self.log, + now: NOW, + }, + publisher, + ) + .await + .expect("no host fault") + } +} + +fn response_of(handled: &Handled) -> &BrokerResponse { + match handled { + Handled::Executed { response, .. } => response, + Handled::Ignored(rejection) => panic!("expected execution, got {rejection:?}"), + } +} + +fn rejection_of(handled: &Handled) -> &FrameRejection { + match handled { + Handled::Ignored(rejection) => rejection, + Handled::Executed { response, .. } => panic!("expected rejection, got {response:?}"), + } +} + +// ── identity is transport-derived ─────────────────────────────────────────── + +#[test] +fn a_verified_frame_yields_transport_derived_identity() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let event = request_frame(&requester, &owner, &request("req-1")); + + let verified = verify_frame(&owner, &event, RELAY, NOW).expect("verified"); + assert_eq!(verified.owner_pubkey, owner.public_key().to_hex()); + assert_eq!(verified.requester_pubkey, requester.public_key().to_hex()); + assert_eq!(verified.relay_scope, RELAY); +} + +/// The owner is this host's own key, never anything the frame carried — so a +/// request body that names another owner changes nothing about who is acted on. +#[test] +fn a_payload_cannot_name_its_own_owner_or_requester() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let mut payload = serde_json::to_value(request("req-1")).expect("json"); + payload["ownerPubkey"] = serde_json::json!("99".repeat(32)); + payload["requesterPubkey"] = serde_json::json!("88".repeat(32)); + let event = frame(&requester, &owner, &payload, NOW); + + let verified = verify_frame(&owner, &event, RELAY, NOW).expect("verified"); + assert_eq!(verified.owner_pubkey, owner.public_key().to_hex()); + assert_eq!(verified.requester_pubkey, requester.public_key().to_hex()); +} + +// ── the digest boundary ───────────────────────────────────────────────────── + +/// The load-bearing retry property: two attempts at the same operation differ in +/// every frame-level field — event id, signature, `created_at` — and must still +/// replay rather than conflict. If the digest ever covered frame or envelope +/// metadata, this test fails with `request_id_conflict`. +#[tokio::test] +async fn a_retry_with_a_new_frame_replays_instead_of_conflicting() { + let harness = Harness::new(); + let request = request("req-retry"); + let publisher = CapturingPublisher::new(); + + let first = request_frame(&harness.requester, &harness.owner, &request); + let second = frame( + &harness.requester, + &harness.owner, + &serde_json::to_value(&request).expect("json"), + NOW + 60, + ); + assert_ne!(first.id, second.id, "retry must be a distinct frame"); + assert_ne!(first.created_at, second.created_at); + + let first = harness.handle(&first, &publisher).await; + assert!(response_of(&first).result.is_succeeded()); + assert!(!response_of(&first).replayed); + + let second = harness.handle(&second, &publisher).await; + let second = response_of(&second); + assert!( + second.result.is_succeeded(), + "retry must replay the recorded success, got {:?}", + second.result + ); + assert!(second.replayed, "retry must be marked as a replay"); + assert_eq!(harness.handler.calls(), 1, "the handler must run once"); +} + +/// The other half of the contract: same `requestId`, genuinely different +/// request. This must be refused rather than answered with the first outcome. +#[tokio::test] +async fn the_same_request_id_with_different_content_still_conflicts() { + let harness = Harness::new(); + let publisher = CapturingPublisher::new(); + + let first = request_frame(&harness.requester, &harness.owner, &request("req-1")); + harness.handle(&first, &publisher).await; + + let mut changed = request("req-1"); + if let CapabilityArgs::AgentsCreate(args) = &mut changed.capability { + args.display_name = "Different".into(); + } + let second = request_frame(&harness.requester, &harness.owner, &changed); + let response = harness.handle(&second, &publisher).await; + assert_eq!( + response_of(&response) + .result + .error() + .map(|error| error.code), + Some(BrokerErrorCode::RequestIdConflict) + ); + assert_eq!(harness.handler.calls(), 1); +} + +// ── what is refused, and what is silently ignored ─────────────────────────── + +#[test] +fn ordinary_telemetry_is_not_broker_traffic() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let event = frame( + &requester, + &owner, + &serde_json::json!({"kind": "turn_started", "seq": 4}), + NOW, + ); + assert_eq!( + verify_frame(&owner, &event, RELAY, NOW).unwrap_err(), + FrameRejection::NotBrokerTraffic + ); +} + +#[test] +fn a_frame_for_another_owner_is_not_this_hosts_traffic() { + let owner = Keys::generate(); + let other_owner = Keys::generate(); + let requester = Keys::generate(); + let event = request_frame(&requester, &other_owner, &request("req-1")); + assert_eq!( + verify_frame(&owner, &event, RELAY, NOW).unwrap_err(), + FrameRejection::NotBrokerTraffic + ); +} + +/// A frame signed by someone other than the agent it names would let a third +/// party borrow that agent's authorization. +#[test] +fn a_signer_that_is_not_the_named_agent_is_refused() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let impostor = Keys::generate(); + let encrypted = encrypt_observer_payload( + &impostor, + &owner.public_key(), + &serde_json::to_value(request("req-1")).expect("json"), + ) + .expect("encrypt"); + let event = EventBuilder::new(Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16), encrypted) + .tags([ + Tag::parse(["p", &owner.public_key().to_hex()]).expect("p tag"), + Tag::parse([OBSERVER_AGENT_TAG, &requester.public_key().to_hex()]).expect("agent tag"), + Tag::parse([OBSERVER_FRAME_TAG, OBSERVER_FRAME_TELEMETRY]).expect("frame tag"), + ]) + .custom_created_at(Timestamp::from(NOW as u64)) + .sign_with_keys(&impostor) + .expect("sign"); + + let error = verify_frame(&owner, &event, RELAY, NOW).unwrap_err(); + assert!( + matches!(&error, FrameRejection::Rejected(message) if message.contains("not the agent it names")), + "unexpected: {error:?}" + ); +} + +#[test] +fn a_stale_frame_is_refused() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let event = frame( + &requester, + &owner, + &serde_json::to_value(request("req-1")).expect("json"), + NOW - MAX_FRAME_SKEW_SECS - 1, + ); + let error = verify_frame(&owner, &event, RELAY, NOW).unwrap_err(); + assert!( + matches!(&error, FrameRejection::Rejected(message) if message.contains("outside the")), + "unexpected: {error:?}" + ); +} + +#[test] +fn a_future_dated_frame_is_refused() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let event = frame( + &requester, + &owner, + &serde_json::to_value(request("req-1")).expect("json"), + NOW + MAX_FRAME_SKEW_SECS + 1, + ); + assert!(matches!( + verify_frame(&owner, &event, RELAY, NOW), + Err(FrameRejection::Rejected(_)) + )); +} + +/// A tampered frame must be caught here, not merely trusted from the relay. +#[test] +fn a_frame_with_a_broken_signature_is_refused() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let other = Keys::generate(); + let event = request_frame(&requester, &owner, &request("req-1")); + let mut forged = event.clone(); + forged.pubkey = other.public_key(); + + let error = verify_frame(&owner, &forged, RELAY, NOW).unwrap_err(); + assert!( + matches!(&error, FrameRejection::Rejected(message) if message.contains("invalid id") + || message.contains("invalid signature")), + "unexpected: {error:?}" + ); +} + +#[test] +fn a_duplicated_routing_tag_is_refused() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let encrypted = encrypt_observer_payload( + &requester, + &owner.public_key(), + &serde_json::to_value(request("req-1")).expect("json"), + ) + .expect("encrypt"); + let event = EventBuilder::new(Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16), encrypted) + .tags([ + Tag::parse(["p", &owner.public_key().to_hex()]).expect("p tag"), + Tag::parse(["p", &Keys::generate().public_key().to_hex()]).expect("second p tag"), + Tag::parse([OBSERVER_AGENT_TAG, &requester.public_key().to_hex()]).expect("agent tag"), + Tag::parse([OBSERVER_FRAME_TAG, OBSERVER_FRAME_TELEMETRY]).expect("frame tag"), + ]) + .custom_created_at(Timestamp::from(NOW as u64)) + .sign_with_keys(&requester) + .expect("sign"); + + assert!(matches!( + verify_frame(&owner, &event, RELAY, NOW), + Err(FrameRejection::Rejected(_)) + )); +} + +#[test] +fn a_control_frame_is_not_an_inbound_request() { + let owner = Keys::generate(); + let requester = Keys::generate(); + let encrypted = encrypt_observer_payload( + &owner, + &requester.public_key(), + &serde_json::to_value(request("req-1")).expect("json"), + ) + .expect("encrypt"); + let event = EventBuilder::new(Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16), encrypted) + .tags([ + Tag::parse(["p", &requester.public_key().to_hex()]).expect("p tag"), + Tag::parse([OBSERVER_AGENT_TAG, &requester.public_key().to_hex()]).expect("agent tag"), + Tag::parse([OBSERVER_FRAME_TAG, OBSERVER_FRAME_CONTROL]).expect("frame tag"), + ]) + .custom_created_at(Timestamp::from(NOW as u64)) + .sign_with_keys(&owner) + .expect("sign"); + + assert_eq!( + verify_frame(&owner, &event, RELAY, NOW).unwrap_err(), + FrameRejection::NotBrokerTraffic + ); +} + +#[tokio::test] +async fn an_ignored_frame_never_reaches_a_handler_or_publishes() { + let harness = Harness::new(); + let publisher = CapturingPublisher::new(); + let event = frame( + &harness.requester, + &harness.owner, + &serde_json::json!({"kind": "turn_started"}), + NOW, + ); + + let handled = harness.handle(&event, &publisher).await; + assert_eq!(rejection_of(&handled), &FrameRejection::NotBrokerTraffic); + assert_eq!(harness.handler.calls(), 0); + assert!(publisher.frames().is_empty()); +} + +/// A malformed *broker* request must be answered — the requester needs to hear +/// `invalid_request` — while never reaching a capability. +#[tokio::test] +async fn a_malformed_broker_request_is_answered_but_not_executed() { + let harness = Harness::new(); + let publisher = CapturingPublisher::new(); + let event = frame( + &harness.requester, + &harness.owner, + &serde_json::json!({"type": "broker_request", "requestId": "req-1"}), + NOW, + ); + + let handled = harness.handle(&event, &publisher).await; + assert_eq!( + response_of(&handled).result.error().map(|error| error.code), + Some(BrokerErrorCode::InvalidRequest) + ); + assert_eq!(harness.handler.calls(), 0); + assert_eq!( + publisher.frames().len(), + 1, + "the refusal is still delivered" + ); +} + +// ── result delivery ───────────────────────────────────────────────────────── + +/// The requester must be able to read its own result: the frame is a control +/// frame encrypted to the requester and signed by the owner. +#[tokio::test] +async fn the_result_frame_is_readable_by_the_requester_and_signed_by_the_owner() { + let harness = Harness::new(); + let publisher = CapturingPublisher::new(); + let event = request_frame(&harness.requester, &harness.owner, &request("req-1")); + + harness.handle(&event, &publisher).await; + let frames = publisher.frames(); + assert_eq!(frames.len(), 1); + let result_frame = &frames[0]; + + assert_eq!(result_frame.pubkey, harness.owner.public_key()); + assert!(result_frame.verify_signature()); + assert_eq!( + result_frame.kind, + Kind::Custom(KIND_AGENT_OBSERVER_FRAME as u16) + ); + assert_eq!( + single_tag(result_frame, OBSERVER_FRAME_TAG).as_deref(), + Some(OBSERVER_FRAME_CONTROL), + "an owner-signed result must travel as a control frame" + ); + assert_eq!( + single_pubkey_tag(result_frame, "p"), + Some(harness.requester.public_key()) + ); + + let decrypted: BrokerResponse = + buzz_core_pkg::observer::decrypt_observer_payload(&harness.requester, result_frame) + .expect("requester can read its result"); + decrypted.validate().expect("a valid result envelope"); + assert_eq!(decrypted.request_id, "req-1"); + assert!(decrypted.result.is_succeeded()); +} + +/// The published result carries no secret material — the create outcome names +/// the new agent's public key only. +#[tokio::test] +async fn a_result_frame_carries_no_secret_material() { + let harness = Harness::new(); + let publisher = CapturingPublisher::new(); + let event = request_frame(&harness.requester, &harness.owner, &request("req-1")); + + harness.handle(&event, &publisher).await; + let decrypted: serde_json::Value = buzz_core_pkg::observer::decrypt_observer_payload( + &harness.requester, + &publisher.frames()[0], + ) + .expect("decrypt"); + let json = serde_json::to_string(&decrypted).expect("json"); + for forbidden in ["nsec", "privateKey", "private_key", "secret"] { + assert!( + !json.contains(forbidden), + "result leaked {forbidden}: {json}" + ); + } +} + +/// A delivery failure must not be reported as an execution failure, and must not +/// undo the recorded outcome: the retry replays the same success. +#[tokio::test] +async fn a_delivery_failure_leaves_the_recorded_outcome_intact() { + let harness = Harness::new(); + let failing = CapturingPublisher::failing("relay unreachable"); + let request = request("req-1"); + let event = request_frame(&harness.requester, &harness.owner, &request); + + let handled = harness.handle(&event, &failing).await; + match &handled { + Handled::Executed { response, delivery } => { + assert!(response.result.is_succeeded()); + assert_eq!(delivery.as_ref().unwrap_err(), "relay unreachable"); + } + Handled::Ignored(rejection) => panic!("expected execution, got {rejection:?}"), + } + + let ok = CapturingPublisher::new(); + let retry = frame( + &harness.requester, + &harness.owner, + &serde_json::to_value(&request).expect("json"), + NOW + 5, + ); + let handled = harness.handle(&retry, &ok).await; + let response = response_of(&handled); + assert!(response.result.is_succeeded()); + assert!(response.replayed); + assert_eq!( + harness.handler.calls(), + 1, + "a failed delivery must not cause a re-execution" + ); +} + +#[test] +fn a_result_frame_for_a_malformed_requester_is_an_error_not_a_panic() { + let owner = Keys::generate(); + let response = BrokerResponse::new( + "req-1", + BrokerResult::succeeded(CapabilityOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: "44".repeat(32), + display_name: "Helper".into(), + channel_id: CHANNEL.into(), + })), + ); + assert!(build_result_frame(&owner, "not-a-pubkey", &response).is_err()); +} diff --git a/desktop/src-tauri/src/broker/mod.rs b/desktop/src-tauri/src/broker/mod.rs new file mode 100644 index 00000000000..4727b1ad4ab --- /dev/null +++ b/desktop/src-tauri/src/broker/mod.rs @@ -0,0 +1,75 @@ +//! Buzz trusted-operation broker — host implementation. +//! +//! The broker performs a small set of named business operations that need the +//! owner's credentials, on behalf of a requester who cannot hold them. Desktop +//! is host #1; the boundary is drawn so a hosted authority can replace it. +//! +//! # Why this lives entirely in Rust +//! +//! Rust already owns every primitive this depends on: signature verification +//! and NIP-44 decrypt (`commands::identity`), owner key custody +//! (`AppState::signing_keys`), the authoritative agent roster +//! (`managed_agents::storage`), and every agent mutation +//! (`commands::agents`, `commands::agent_models_update`). Routing authority +//! through the frontend would move the trust boundary away from the credentials +//! and split one authority operation across IPC — where the inputs are +//! spoofable and a crash can land between the mutation and its record. The +//! frontend may forward frames and render diagnostics; it is not the security +//! principal. +//! +//! # Guarantee +//! +//! **At-most-once execution plus `indeterminate`** — not exactly-once, and not +//! durable completion. See [`store`] for why, and for what a capability must +//! own to do better. +//! +//! # Delivery is ephemeral +//! +//! Requests arrive as kind-24200 observer frames, which the relay routes to +//! connected subscribers without storing. Execution while the host is offline +//! is therefore unsupported: no queue holds the request, and the requester's +//! wait expires. That is a deliberate limit of this transport, not an oversight. + +use sha2::{Digest, Sha256}; + +pub mod agents_policy; +pub mod desktop_agents; +pub mod handlers; +pub mod host; +pub mod ingress; +pub mod pipeline; +pub mod store; + +/// Maximum accepted size of a decrypted broker request payload, in bytes. +/// +/// Bounded before hashing or parsing so an oversized frame cannot force +/// unbounded work. Generous next to the largest legitimate request (a 20k-char +/// system prompt) and far below the observer plaintext ceiling. +pub const MAX_REQUEST_BYTES: usize = 96 * 1024; + +/// Hash the exact decrypted request bytes. +/// +/// The digest is computed **only here**, on the host, from the bytes as +/// received. Nothing re-serializes the request first, so there is no canonical +/// encoding to agree on and no second implementation that could drift from this +/// one. A retry that resends identical bytes hashes identically; a retry that +/// changes anything hashes differently and is refused as a conflict rather than +/// silently answered with the first request's outcome. +/// +/// # Errors +/// +/// Returns an error when the payload exceeds [`MAX_REQUEST_BYTES`]. +pub fn request_digest(payload: &[u8]) -> Result { + if payload.len() > MAX_REQUEST_BYTES { + return Err(format!( + "broker request exceeds {MAX_REQUEST_BYTES} bytes (got {})", + payload.len() + )); + } + let mut hasher = Sha256::new(); + hasher.update(payload); + Ok(hex::encode(hasher.finalize())) +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/pipeline.rs b/desktop/src-tauri/src/broker/pipeline.rs new file mode 100644 index 00000000000..583bed49c18 --- /dev/null +++ b/desktop/src-tauri/src/broker/pipeline.rs @@ -0,0 +1,324 @@ +//! The broker authority pipeline. +//! +//! One function owns the whole trusted path for a request: +//! +//! ```text +//! verified frame → authorize → validate → claim → dispatch → complete → response +//! ``` +//! +//! It is deliberately a single Rust function rather than a chain of steps +//! coordinated from elsewhere. Every stage boundary is a place where a caller +//! could otherwise substitute its own answer for "who is asking" or "has this +//! already run", and a crash between two coordinated steps is a side effect +//! nobody recorded. +//! +//! # Why the seams are async +//! +//! The capability this ships first mutates through `create_managed_agent` / +//! `update_managed_agent` / `delete_managed_agent`, which are `async` and +//! publish to the relay. So [`CapabilityHandler`], [`Authorizer`], and +//! [`ExecutionLog`] are all async — modelled on [`crate::archive::sync`]'s +//! `ArchiveSyncIo`, the repo's existing injected-async-IO seam. +//! +//! `rusqlite::Connection` is `!Sync` and must not be held across an `.await`, +//! so the pipeline never touches one: it talks to [`ExecutionLog`], whose +//! production implementation confines each connection to a single blocking +//! task. That is why the durable log is a trait here rather than a borrowed +//! connection. +//! +//! # What this module does not decide +//! +//! It does not verify signatures or decrypt frames — that already happened, and +//! its result arrives as [`VerifiedRequest`], which can only be built from +//! transport-derived identity. It does not implement capabilities either; +//! those are [`CapabilityHandler`]s. + +use std::future::Future; +use std::pin::Pin; + +use buzz_sdk_pkg::broker::{ + BrokerError, BrokerErrorCode, BrokerRequest, BrokerResponse, BrokerResult, Capability, + CapabilityOutcome, +}; + +use super::store::{self, ClaimOutcome, ExecutionKey, ExecutionState}; + +/// Boxed future alias, matching [`crate::archive::sync`]'s injected-IO seams. +pub type BoxFuture<'a, T> = Pin + Send + 'a>>; + +/// A request whose frame has already been verified and decrypted. +/// +/// Constructing this type is the assertion that `owner_pubkey`, +/// `requester_pubkey`, and `relay_scope` came from the **verified transport**, +/// not from the request body. Nothing in [`BrokerRequest`] can influence them, +/// which is what prevents a signer from naming someone else as owner. +pub struct VerifiedRequest { + /// Owner whose credentials back this host. + pub owner_pubkey: String, + /// Verified signer of the frame. + pub requester_pubkey: String, + /// Relay the frame arrived on. + pub relay_scope: String, + /// The exact decrypted payload bytes, used for the idempotency digest. + pub payload: Vec, +} + +/// Redacts the payload. +/// +/// `payload` is decrypted request content — a system prompt, for instance. It is +/// not a credential, but it is not log material either, and a derived `Debug` +/// would put it into any error message or test failure that formats a request. +impl std::fmt::Debug for VerifiedRequest { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("VerifiedRequest") + .field("owner_pubkey", &self.owner_pubkey) + .field("requester_pubkey", &self.requester_pubkey) + .field("relay_scope", &self.relay_scope) + .field("payload_bytes", &self.payload.len()) + .finish() + } +} + +/// The durable execution log, as the pipeline needs it. +/// +/// A trait rather than a `&mut Connection` because the pipeline is async and +/// `rusqlite::Connection` is `!Sync`: an implementation must keep each +/// connection inside one blocking task, which only it can arrange. +pub trait ExecutionLog: Send + Sync { + /// Claim `key` for execution, or report why it cannot be claimed. + /// + /// Must insert `executing` before returning [`ClaimOutcome::Claimed`], and + /// must never return `Claimed` twice for one key. + fn claim( + &self, + key: ExecutionKey, + request_digest: String, + capability_version: u16, + now: i64, + ) -> BoxFuture<'_, Result>; + + /// Record the terminal `state` and `result_json` for a claimed key. + fn complete( + &self, + key: ExecutionKey, + state: ExecutionState, + result_json: String, + now: i64, + ) -> BoxFuture<'_, Result<(), String>>; +} + +/// Whether a requester may invoke a capability. +/// +/// Base authentication (a verified signer, a real owner binding) is the +/// broker's job and has happened before this point. This trait is the +/// *capability scope* decision, which is domain policy: channel membership, for +/// instance, is an `agents.*` rule rather than something the broker asserts +/// about every capability it might ever host. +pub trait Authorizer: Send + Sync { + /// Authorize `request` for `capability`, or explain the refusal. + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, + capability: Capability, + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>>; +} + +/// The outcome of running one capability. +/// +/// `Failed` asserts no side effects persisted. `Indeterminate` asserts nothing: +/// a handler returns it when it cannot tell, and the broker records it so the +/// request is never silently retried. +pub enum HandlerOutcome { + /// Completed, with this outcome. + Succeeded(CapabilityOutcome), + /// Did not complete; no side effects persisted. + Failed(BrokerError), + /// Whether side effects persisted is unknown. + Indeterminate(BrokerError), +} + +/// One broker capability implementation. +pub trait CapabilityHandler: Send + Sync { + /// Execute the capability. + /// + /// Called at most once per idempotency key. A handler that mutates in + /// several phases and cannot report which completed must return + /// [`HandlerOutcome::Indeterminate`] rather than guess. + fn execute<'a>( + &'a self, + request: &'a VerifiedRequest, + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, HandlerOutcome>; +} + +/// Resolves a capability name to its handler. +pub trait CapabilityRegistry: Send + Sync { + /// The handler for `capability`, or `None` when this host does not offer it. + fn handler(&self, capability: Capability) -> Option<&dyn CapabilityHandler>; +} + +/// Everything the pipeline needs that is not the request itself. +pub struct BrokerContext<'a> { + /// Capability scope policy. + pub authorizer: &'a dyn Authorizer, + /// Capability implementations. + pub registry: &'a dyn CapabilityRegistry, + /// Durable execution log. + pub log: &'a dyn ExecutionLog, + /// Current unix time, injected so tests are not clock-dependent. + pub now: i64, +} + +/// Run one broker request end to end and produce the response to deliver. +/// +/// Returns `Err` only for a host fault that leaves no deliverable response +/// (a database that cannot be reached, say). Every refusal a requester should +/// hear about — invalid, unauthorized, conflicting, interrupted — comes back as +/// an `Ok` response describing it. +pub async fn execute( + request: &VerifiedRequest, + ctx: BrokerContext<'_>, +) -> Result { + // Bound and hash the payload before parsing it. The digest is over the + // exact bytes received, so a retry that resends them matches without any + // canonical encoding having to be agreed on. + let digest = match super::request_digest(&request.payload) { + Ok(digest) => digest, + Err(error) => { + return Ok(BrokerResponse::new( + UNKNOWN_REQUEST_ID, + BrokerResult::failed(BrokerError::invalid_request(error)), + )) + } + }; + + let parsed: BrokerRequest = match serde_json::from_slice(&request.payload) { + Ok(parsed) => parsed, + Err(error) => { + // Unparseable: there is no trustworthy request id to echo, and + // inventing one would let a malformed frame address someone + // else's execution record. + return Ok(BrokerResponse::new( + UNKNOWN_REQUEST_ID, + BrokerResult::failed(BrokerError::invalid_request(format!( + "malformed broker request: {error}" + ))), + )); + } + }; + + let request_id = parsed.request_id.clone(); + if let Err(error) = parsed.validate() { + return Ok(BrokerResponse::new( + &request_id, + BrokerResult::failed(BrokerError::invalid_request(error.to_string())), + )); + } + + let capability = parsed.capability(); + + // Authorize before claiming: a refused request must leave no trace that + // could later be replayed as though it had been accepted. + if let Err(error) = ctx.authorizer.authorize(request, capability, &parsed).await { + return Ok(BrokerResponse::new( + &request_id, + BrokerResult::failed(error), + )); + } + + let Some(handler) = ctx.registry.handler(capability) else { + return Ok(BrokerResponse::new( + &request_id, + BrokerResult::failed(BrokerError::new( + BrokerErrorCode::UnknownCapability, + format!("this host does not offer {}", capability.as_str()), + )), + )); + }; + + let key = ExecutionKey { + relay_scope: store::normalized_relay_scope(&request.relay_scope).to_string(), + owner_pubkey: request.owner_pubkey.clone(), + requester_pubkey: request.requester_pubkey.clone(), + capability: capability.as_str().to_string(), + request_id: request_id.clone(), + }; + + let claim = ctx + .log + .claim(key.clone(), digest, parsed.capability_version, ctx.now) + .await?; + match claim { + ClaimOutcome::Claimed => {} + ClaimOutcome::Replay(record) => { + return Ok(replay_response(&request_id, record.result_json.as_deref())) + } + ClaimOutcome::Interrupted(_) => { + // A previous attempt started and never reported. Side effects may + // exist, so this is reported rather than retried. + return Ok(BrokerResponse::new( + &request_id, + BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "a previous attempt at this request did not complete; \ + its effects are unknown and it will not be retried automatically", + )), + )); + } + ClaimOutcome::DigestConflict { .. } => { + return Ok(BrokerResponse::new( + &request_id, + BrokerResult::failed(BrokerError::new( + BrokerErrorCode::RequestIdConflict, + "this requestId was already used for a different request; \ + retry with the identical payload or choose a new requestId", + )), + )); + } + } + + // The row is now `executing`. Everything after this point must reach + // `complete`, or the next attempt correctly reports `indeterminate`. + let result = match handler.execute(request, &parsed).await { + HandlerOutcome::Succeeded(outcome) => BrokerResult::succeeded(outcome), + HandlerOutcome::Failed(error) => BrokerResult::failed(error), + HandlerOutcome::Indeterminate(error) => BrokerResult::indeterminate(error), + }; + + let state = match &result { + BrokerResult::Succeeded { .. } => ExecutionState::Succeeded, + BrokerResult::Failed { .. } => ExecutionState::Failed, + BrokerResult::Indeterminate { .. } => ExecutionState::Indeterminate, + }; + let result_json = serde_json::to_string(&result) + .map_err(|error| format!("failed to encode broker result: {error}"))?; + ctx.log.complete(key, state, result_json, ctx.now).await?; + + Ok(BrokerResponse::new(&request_id, result)) +} + +/// Request id used when the payload is too malformed to yield one. +const UNKNOWN_REQUEST_ID: &str = "unknown"; + +/// Rebuild a response from a stored terminal result. +/// +/// A row that is terminal but has no decodable result cannot be re-executed — +/// its side effects already happened — so it degrades to `indeterminate`. +fn replay_response(request_id: &str, result_json: Option<&str>) -> BrokerResponse { + let stored = result_json.and_then(|json| serde_json::from_str::(json).ok()); + match stored { + Some(result) => BrokerResponse::new(request_id, result).replayed(), + None => BrokerResponse::new( + request_id, + BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "this request already ran, but its recorded outcome could not be read", + )), + ) + .replayed(), + } +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/pipeline/tests.rs b/desktop/src-tauri/src/broker/pipeline/tests.rs new file mode 100644 index 00000000000..5fbf08f58c3 --- /dev/null +++ b/desktop/src-tauri/src/broker/pipeline/tests.rs @@ -0,0 +1,535 @@ +//! Pipeline tests: authority, idempotency, and crash behavior. + +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Mutex; + +use buzz_sdk_pkg::broker::{ + AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, + CapabilityArgs, +}; + +use super::super::store::MemoryExecutionLog; +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const OWNER: &str = "1111111111111111111111111111111111111111111111111111111111111111"; +const REQUESTER: &str = "2222222222222222222222222222222222222222222222222222222222222222"; +const AGENT: &str = "3333333333333333333333333333333333333333333333333333333333333333"; + +fn db() -> MemoryExecutionLog { + MemoryExecutionLog::new() +} + +fn create_request(request_id: &str) -> Vec { + let request = BrokerRequest::new( + request_id, + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "Help.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ) + .unwrap(); + serde_json::to_vec(&request).unwrap() +} + +fn verified(payload: Vec) -> VerifiedRequest { + VerifiedRequest { + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + relay_scope: "wss://relay.example".into(), + payload, + } +} + +struct AllowAll; +impl Authorizer for AllowAll { + fn authorize<'a>( + &'a self, + _request: &'a VerifiedRequest, + _capability: Capability, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>> { + Box::pin(async { Ok(()) }) + } +} + +struct DenyAll; +impl Authorizer for DenyAll { + fn authorize<'a>( + &'a self, + _request: &'a VerifiedRequest, + _capability: Capability, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>> { + Box::pin(async { Err(BrokerError::unauthorized("not a member of that channel")) }) + } +} + +/// Counts executions so a test can prove a handler ran at most once. +struct CountingHandler { + calls: AtomicUsize, + outcome: fn() -> HandlerOutcome, +} + +impl CountingHandler { + fn new(outcome: fn() -> HandlerOutcome) -> Self { + Self { + calls: AtomicUsize::new(0), + outcome, + } + } + + fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } +} + +impl CapabilityHandler for CountingHandler { + fn execute<'a>( + &'a self, + _request: &'a VerifiedRequest, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, HandlerOutcome> { + Box::pin(async move { + self.calls.fetch_add(1, Ordering::SeqCst); + (self.outcome)() + }) + } +} + +struct OneHandler<'a> { + capability: Capability, + handler: &'a dyn CapabilityHandler, +} + +impl CapabilityRegistry for OneHandler<'_> { + fn handler(&self, capability: Capability) -> Option<&dyn CapabilityHandler> { + (capability == self.capability).then_some(self.handler) + } +} + +struct NoHandlers; +impl CapabilityRegistry for NoHandlers { + fn handler(&self, _capability: Capability) -> Option<&dyn CapabilityHandler> { + None + } +} + +fn success() -> HandlerOutcome { + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: AGENT.into(), + display_name: "Helper".into(), + channel_id: CHANNEL.into(), + })) +} + +async fn run( + request: &VerifiedRequest, + authorizer: &dyn Authorizer, + registry: &dyn CapabilityRegistry, + log: &dyn ExecutionLog, +) -> BrokerResponse { + execute( + request, + BrokerContext { + authorizer, + registry, + log, + now: 1000, + }, + ) + .await + .unwrap() +} + +#[tokio::test] +async fn a_valid_authorized_request_executes_once_and_returns_its_outcome() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + let response = run(&request, &AllowAll, ®istry, &log).await; + assert_eq!(response.request_id, "req-1"); + assert!(response.result.is_succeeded()); + assert!(!response.replayed); + assert_eq!(handler.calls(), 1); +} + +/// The central idempotency property: replaying the identical request returns +/// the recorded outcome and does not run the handler a second time. +#[tokio::test] +async fn replaying_an_identical_request_returns_the_record_without_re_executing() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + let first = run(&request, &AllowAll, ®istry, &log).await; + let second = run(&request, &AllowAll, ®istry, &log).await; + + assert_eq!(handler.calls(), 1, "handler must not run twice"); + assert!(second.replayed); + assert_eq!(first.result, second.result, "replay must be identical"); +} + +/// Same request id, different content: the caller must be told, not handed +/// someone else's outcome. +#[tokio::test] +async fn a_different_payload_under_the_same_request_id_is_refused() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + + run( + &verified(create_request("req-1")), + &AllowAll, + ®istry, + &log, + ) + .await; + + let mut different = BrokerRequest::new( + "req-1", + CapabilityArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Different agent".into(), + system_prompt: "Help.".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ) + .unwrap(); + different.request_id = "req-1".into(); + let response = run( + &verified(serde_json::to_vec(&different).unwrap()), + &AllowAll, + ®istry, + &log, + ) + .await; + + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::RequestIdConflict + ); + assert_eq!(handler.calls(), 1, "the conflict must not execute"); +} + +/// An unauthorized request must not leave a claim behind: otherwise a later +/// authorized retry would be answered from a refusal. +#[tokio::test] +async fn an_unauthorized_request_does_not_execute_or_leave_a_claim() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + let denied = run(&request, &DenyAll, ®istry, &log).await; + assert_eq!( + denied.result.error().unwrap().code, + BrokerErrorCode::Unauthorized + ); + assert_eq!(handler.calls(), 0); + + // The same request, now authorized, still runs normally. + let allowed = run(&request, &AllowAll, ®istry, &log).await; + assert!(allowed.result.is_succeeded()); + assert!(!allowed.replayed); + assert_eq!(handler.calls(), 1); +} + +/// A crash between the mutation and its record must surface as indeterminate, +/// never as a silent re-run. +#[tokio::test] +async fn an_interrupted_execution_reports_indeterminate_and_never_re_executes() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + // Simulate a process that claimed the key and died before completing. + let key = ExecutionKey { + relay_scope: "wss://relay.example".into(), + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + capability: "agents.create".into(), + request_id: "req-1".into(), + }; + let digest = super::super::request_digest(&request.payload).unwrap(); + log.with_conn(|conn| store::claim(conn, &key, &digest, 1, 500)) + .unwrap(); + + let response = run(&request, &AllowAll, ®istry, &log).await; + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::OutcomeUnknown + ); + assert_eq!( + handler.calls(), + 0, + "an interrupted request must not be re-executed" + ); +} + +#[tokio::test] +async fn a_failed_handler_is_recorded_and_replayed_as_failed() { + fn failure() -> HandlerOutcome { + HandlerOutcome::Failed(BrokerError::new( + BrokerErrorCode::CapabilityFailed, + "runtime not installed", + )) + } + let log = db(); + let handler = CountingHandler::new(failure); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + let first = run(&request, &AllowAll, ®istry, &log).await; + assert_eq!( + first.result.error().unwrap().code, + BrokerErrorCode::CapabilityFailed + ); + + // A failure is terminal: retrying replays it rather than re-running. + let second = run(&request, &AllowAll, ®istry, &log).await; + assert!(second.replayed); + assert_eq!(first.result, second.result); + assert_eq!(handler.calls(), 1); +} + +#[tokio::test] +async fn an_indeterminate_handler_outcome_is_recorded_as_indeterminate() { + fn unknown() -> HandlerOutcome { + HandlerOutcome::Indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "published but could not confirm", + )) + } + let log = db(); + let handler = CountingHandler::new(unknown); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let request = verified(create_request("req-1")); + + let response = run(&request, &AllowAll, ®istry, &log).await; + assert!(matches!( + response.result, + BrokerResult::Indeterminate { .. } + )); + assert_eq!(handler.calls(), 1); +} + +#[tokio::test] +async fn a_malformed_payload_is_refused_without_a_claim() { + let log = db(); + let response = run( + &verified(b"not json".to_vec()), + &AllowAll, + &NoHandlers, + &log, + ) + .await; + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); +} + +#[tokio::test] +async fn an_oversized_payload_is_refused() { + let log = db(); + let response = run( + &verified(vec![b'x'; super::super::MAX_REQUEST_BYTES + 1]), + &AllowAll, + &NoHandlers, + &log, + ) + .await; + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); +} + +#[tokio::test] +async fn a_capability_this_host_does_not_offer_is_refused() { + let log = db(); + let response = run( + &verified(create_request("req-1")), + &AllowAll, + &NoHandlers, + &log, + ) + .await; + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::UnknownCapability + ); +} + +/// Two requesters using the same request id are separate executions: the key +/// includes the verified requester, so one cannot read the other's outcome. +#[tokio::test] +async fn the_same_request_id_from_two_requesters_are_separate_executions() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + + let payload = create_request("req-1"); + let first = verified(payload.clone()); + let second = VerifiedRequest { + requester_pubkey: AGENT.into(), + ..verified(payload) + }; + + assert!(!run(&first, &AllowAll, ®istry, &log).await.replayed); + assert!( + !run(&second, &AllowAll, ®istry, &log).await.replayed, + "a different requester must not replay another's record" + ); + assert_eq!(handler.calls(), 2); +} + +/// The pipeline must take identity from the verified transport, never from the +/// payload. A body claiming another owner cannot even deserialize. +#[tokio::test] +async fn identity_in_the_payload_cannot_override_transport_identity() { + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + + let mut json: serde_json::Value = serde_json::from_slice(&create_request("req-1")).unwrap(); + json.as_object_mut() + .unwrap() + .insert("ownerPubkey".into(), serde_json::json!(AGENT)); + + let response = run( + &verified(serde_json::to_vec(&json).unwrap()), + &AllowAll, + ®istry, + &log, + ) + .await; + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); + assert_eq!(handler.calls(), 0); +} + +/// The authorizer sees the transport-derived identity, so policy is written +/// against who actually signed. +#[tokio::test] +async fn the_authorizer_receives_transport_derived_identity() { + struct Recording(Mutex>); + impl Authorizer for Recording { + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, + capability: Capability, + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>> { + Box::pin(async move { + self.0.lock().unwrap().push(( + request.owner_pubkey.clone(), + request.requester_pubkey.clone(), + capability.as_str().to_string(), + )); + Ok(()) + }) + } + } + + let log = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let authorizer = Recording(Mutex::new(Vec::new())); + + run( + &verified(create_request("req-1")), + &authorizer, + ®istry, + &log, + ) + .await; + + assert_eq!( + authorizer.0.lock().unwrap().as_slice(), + &[( + OWNER.to_string(), + REQUESTER.to_string(), + "agents.create".to_string() + )] + ); +} + +#[tokio::test] +async fn delete_requests_route_to_the_delete_capability() { + fn deleted() -> HandlerOutcome { + HandlerOutcome::Succeeded(CapabilityOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: AGENT.into(), + display_name: "Gone".into(), + })) + } + let log = db(); + let handler = CountingHandler::new(deleted); + let registry = OneHandler { + capability: Capability::AgentsDelete, + handler: &handler, + }; + + let request = BrokerRequest::new( + "req-del", + CapabilityArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(AGENT.into()), + }), + ) + .unwrap(); + let response = run( + &verified(serde_json::to_vec(&request).unwrap()), + &AllowAll, + ®istry, + &log, + ) + .await; + + assert!(response.result.is_succeeded()); + assert_eq!(handler.calls(), 1); +} diff --git a/desktop/src-tauri/src/broker/store.rs b/desktop/src-tauri/src/broker/store.rs new file mode 100644 index 00000000000..845c1f52008 --- /dev/null +++ b/desktop/src-tauri/src/broker/store.rs @@ -0,0 +1,524 @@ +//! Durable execution log for broker requests. +//! +//! One row per `(relay_scope, owner, requester, capability, request_id)`. The +//! row is inserted in state `executing` **before the first side effect**, so a +//! request that crashes mid-flight leaves evidence behind rather than looking +//! like it never ran. +//! +//! # What this buys, and what it does not +//! +//! This log delivers **at-most-once execution plus `indeterminate`**. It does +//! not deliver exactly-once or durable completion. A crash between a mutation +//! and the recording of its result is not recoverable from here — the log can +//! prove that execution *started*, never how far it got. Reconciling partial +//! side effects is the owning capability's job, because only it knows what its +//! phases were. +//! +//! WAL + `busy_timeout=5000` matches `managed_agents/retention.rs` and +//! `archive/store.rs`. + +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use rusqlite::{params, Connection, OptionalExtension}; +use sha2::{Digest, Sha256}; + +use super::pipeline::{BoxFuture, ExecutionLog}; + +/// Terminal or in-flight state of a broker request. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExecutionState { + /// A handler was dispatched and has not reported back. + Executing, + /// The handler completed successfully. + Succeeded, + /// The handler failed without leaving side effects. + Failed, + /// Whether side effects took hold is unknown. + Indeterminate, +} + +impl ExecutionState { + /// Stable database encoding. + pub fn as_str(self) -> &'static str { + match self { + Self::Executing => "executing", + Self::Succeeded => "succeeded", + Self::Failed => "failed", + Self::Indeterminate => "indeterminate", + } + } + + /// Parse the database encoding. + fn parse(value: &str) -> Result { + match value { + "executing" => Ok(Self::Executing), + "succeeded" => Ok(Self::Succeeded), + "failed" => Ok(Self::Failed), + "indeterminate" => Ok(Self::Indeterminate), + other => Err(format!("unknown broker execution state \"{other}\"")), + } + } + + /// Whether this state is final. + pub fn is_terminal(self) -> bool { + !matches!(self, Self::Executing) + } +} + +/// Identity of one broker request, as derived by the host. +/// +/// Owner, requester, and relay scope come from the verified frame — never from +/// the request body — so a caller cannot address another owner's execution log. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExecutionKey { + /// Relay the request arrived on, normalized. + pub relay_scope: String, + /// Owner whose credentials would be used. + pub owner_pubkey: String, + /// Verified signer of the request frame. + pub requester_pubkey: String, + /// Capability wire name. + pub capability: String, + /// Caller-chosen idempotency key. + pub request_id: String, +} + +/// A recorded execution row. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExecutionRecord { + /// Current state. + pub state: ExecutionState, + /// Digest of the request that claimed this key. + pub request_digest: String, + /// Capability contract version that claimed this key. + pub capability_version: u16, + /// Serialized terminal result, absent while `executing`. + pub result_json: Option, +} + +/// What a claim attempt established. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ClaimOutcome { + /// This caller owns the execution and must proceed. + Claimed, + /// A terminal result already exists; replay it without re-executing. + Replay(ExecutionRecord), + /// A previous attempt started and never finished. + /// + /// The row has been moved to `indeterminate` and must not be re-executed: + /// side effects may already exist. + Interrupted(ExecutionRecord), + /// The key was claimed by a request with different content. + DigestConflict { + /// Digest recorded by the first request. + expected: String, + }, +} + +const SCHEMA: &str = " +CREATE TABLE IF NOT EXISTS broker_executions ( + relay_scope TEXT NOT NULL, + owner_pubkey TEXT NOT NULL, + requester_pubkey TEXT NOT NULL, + capability TEXT NOT NULL, + request_id TEXT NOT NULL, + request_digest TEXT NOT NULL, + capability_version INTEGER NOT NULL, + state TEXT NOT NULL, + result_json TEXT, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + PRIMARY KEY (relay_scope, owner_pubkey, requester_pubkey, capability, request_id) +); +"; + +/// Relay-URL form that identifies a broker scope. +/// +/// Mirrors `managed_agents::retention`: equivalent workspace URLs (surrounding +/// space, trailing slash) must resolve to one scope, so "same relay" can never +/// disagree with "same database". +pub fn normalized_relay_scope(relay_url: &str) -> &str { + relay_url.trim().trim_end_matches('/') +} + +/// Resolve the broker database path for a relay + owner pair. +/// +/// The normalized scope is hashed so relay URLs never become path components. +pub fn scoped_broker_db_path(base_dir: &Path, relay_url: &str, owner_pubkey: &str) -> PathBuf { + let normalized_relay = normalized_relay_scope(relay_url); + let mut hasher = Sha256::new(); + hasher.update(owner_pubkey.trim().to_ascii_lowercase().as_bytes()); + hasher.update(b"\0"); + hasher.update(normalized_relay.as_bytes()); + let scope_id = hex::encode(hasher.finalize()); + base_dir.join("broker").join(format!("{scope_id}.db")) +} + +/// Open (or create) the broker execution database. +pub fn open_broker_db(path: &Path) -> Result { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|e| format!("failed to create broker dir: {e}"))?; + } + + let conn = Connection::open(path).map_err(|e| format!("failed to open broker db: {e}"))?; + + conn.pragma_update(None, "busy_timeout", 5000) + .map_err(|e| format!("failed to set busy_timeout: {e}"))?; + set_wal_mode(&conn)?; + + conn.execute_batch(SCHEMA) + .map_err(|e| format!("failed to initialize broker schema: {e}"))?; + + Ok(conn) +} + +fn set_wal_mode(conn: &Connection) -> Result<(), String> { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + match conn.pragma_update(None, "journal_mode", "WAL") { + Ok(()) => return Ok(()), + Err(error) if sqlite_is_busy(&error) && Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(25)); + } + Err(error) => return Err(format!("failed to set WAL mode: {error}")), + } + } +} + +fn sqlite_is_busy(error: &rusqlite::Error) -> bool { + matches!( + error, + rusqlite::Error::SqliteFailure(inner, _) + if matches!( + inner.code, + rusqlite::ErrorCode::DatabaseBusy | rusqlite::ErrorCode::DatabaseLocked + ) + ) +} + +/// Claim `key` for execution, or report why it cannot be claimed. +/// +/// The insert and the read happen in one `IMMEDIATE` transaction, so two +/// concurrent frames carrying the same request cannot both believe they claimed +/// it. This is the only function that may transition a row into `executing`, +/// and it never returns [`ClaimOutcome::Claimed`] for a key that already has a +/// row. +/// +/// An existing `executing` row is treated as interrupted: it is moved to +/// `indeterminate` and returned. Re-executing it would risk duplicating side +/// effects that the log cannot see. +pub fn claim( + conn: &mut Connection, + key: &ExecutionKey, + request_digest: &str, + capability_version: u16, + now: i64, +) -> Result { + let tx = conn + .transaction_with_behavior(rusqlite::TransactionBehavior::Immediate) + .map_err(|e| format!("failed to begin broker claim: {e}"))?; + + let existing = tx + .query_row( + "SELECT state, request_digest, capability_version, result_json + FROM broker_executions + WHERE relay_scope = ?1 AND owner_pubkey = ?2 AND requester_pubkey = ?3 + AND capability = ?4 AND request_id = ?5", + params![ + key.relay_scope, + key.owner_pubkey, + key.requester_pubkey, + key.capability, + key.request_id + ], + |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, Option>(3)?, + )) + }, + ) + .optional() + .map_err(|e| format!("failed to read broker execution: {e}"))?; + + let outcome = match existing { + None => { + tx.execute( + "INSERT INTO broker_executions ( + relay_scope, owner_pubkey, requester_pubkey, capability, request_id, + request_digest, capability_version, state, result_json, + created_at, updated_at + ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, NULL, ?9, ?9)", + params![ + key.relay_scope, + key.owner_pubkey, + key.requester_pubkey, + key.capability, + key.request_id, + request_digest, + i64::from(capability_version), + ExecutionState::Executing.as_str(), + now + ], + ) + .map_err(|e| format!("failed to claim broker execution: {e}"))?; + ClaimOutcome::Claimed + } + Some((state, digest, version, result_json)) => { + // A different payload under the same id is a caller bug, and the + // recorded outcome does not describe it. Checked before state so a + // conflicting retry can never be answered with someone else's result. + if digest != request_digest { + ClaimOutcome::DigestConflict { expected: digest } + } else { + let state = ExecutionState::parse(&state)?; + let version = u16::try_from(version) + .map_err(|_| format!("stored capability_version {version} out of range"))?; + let record = ExecutionRecord { + state, + request_digest: digest, + capability_version: version, + result_json, + }; + if state.is_terminal() { + ClaimOutcome::Replay(record) + } else { + tx.execute( + "UPDATE broker_executions SET state = ?1, updated_at = ?2 + WHERE relay_scope = ?3 AND owner_pubkey = ?4 + AND requester_pubkey = ?5 AND capability = ?6 + AND request_id = ?7", + params![ + ExecutionState::Indeterminate.as_str(), + now, + key.relay_scope, + key.owner_pubkey, + key.requester_pubkey, + key.capability, + key.request_id + ], + ) + .map_err(|e| format!("failed to mark broker execution interrupted: {e}"))?; + ClaimOutcome::Interrupted(ExecutionRecord { + state: ExecutionState::Indeterminate, + ..record + }) + } + } + } + }; + + tx.commit() + .map_err(|e| format!("failed to commit broker claim: {e}"))?; + Ok(outcome) +} + +/// Record the terminal state and result for a claimed key. +/// +/// Rejects a non-terminal state: only [`claim`] may write `executing`. +pub fn complete( + conn: &Connection, + key: &ExecutionKey, + state: ExecutionState, + result_json: &str, + now: i64, +) -> Result<(), String> { + if !state.is_terminal() { + return Err(format!( + "cannot complete a broker execution as \"{}\"", + state.as_str() + )); + } + let changed = conn + .execute( + "UPDATE broker_executions + SET state = ?1, result_json = ?2, updated_at = ?3 + WHERE relay_scope = ?4 AND owner_pubkey = ?5 AND requester_pubkey = ?6 + AND capability = ?7 AND request_id = ?8", + params![ + state.as_str(), + result_json, + now, + key.relay_scope, + key.owner_pubkey, + key.requester_pubkey, + key.capability, + key.request_id + ], + ) + .map_err(|e| format!("failed to complete broker execution: {e}"))?; + if changed == 0 { + return Err("broker execution row is missing; refusing to record a result".into()); + } + Ok(()) +} + +/// Open an in-memory execution log with the real schema. +/// +/// For tests that need the real claim/complete behavior without a temp file. +#[cfg(test)] +pub fn open_in_memory() -> Connection { + let conn = Connection::open_in_memory().expect("in-memory sqlite"); + conn.execute_batch(SCHEMA).expect("broker schema"); + conn +} + +/// In-memory [`ExecutionLog`] running the real [`claim`] / [`complete`] logic. +/// +/// Tests get the production idempotency behavior without touching the +/// filesystem. `Connection` is `Send` but not `Sync`, so the mutex is what makes +/// it shareable behind the `Send + Sync` trait — and holding the lock for the +/// whole call serializes claims the same way the file-backed `IMMEDIATE` +/// transaction does. +#[cfg(test)] +pub struct MemoryExecutionLog(std::sync::Mutex); + +#[cfg(test)] +impl MemoryExecutionLog { + /// A fresh, empty log. + pub fn new() -> Self { + Self(std::sync::Mutex::new(open_in_memory())) + } + + /// Run `body` against the underlying connection. + /// + /// Lets a test plant a row directly — simulating a process that claimed a + /// key and died — without exposing the connection itself. + pub fn with_conn(&self, body: impl FnOnce(&mut Connection) -> T) -> T { + body(&mut self.0.lock().expect("broker test log poisoned")) + } +} + +#[cfg(test)] +impl ExecutionLog for MemoryExecutionLog { + fn claim( + &self, + key: ExecutionKey, + request_digest: String, + capability_version: u16, + now: i64, + ) -> BoxFuture<'_, Result> { + Box::pin(async move { + self.with_conn(|conn| claim(conn, &key, &request_digest, capability_version, now)) + }) + } + + fn complete( + &self, + key: ExecutionKey, + state: ExecutionState, + result_json: String, + now: i64, + ) -> BoxFuture<'_, Result<(), String>> { + Box::pin( + async move { self.with_conn(|conn| complete(conn, &key, state, &result_json, now)) }, + ) + } +} + +/// File-backed [`ExecutionLog`] that keeps every `!Sync` connection inside one +/// blocking task. +/// +/// The pipeline is async, so it can never hold a `rusqlite::Connection` across +/// an `.await`. This adapter is where that constraint is discharged: each call +/// opens its own connection on the blocking pool, finishes its transaction, and +/// drops it before the future resolves. Cross-call exclusion is SQLite's, not +/// this type's — [`claim`] does its read and insert inside one `IMMEDIATE` +/// transaction, which is what makes two concurrent claims safe even though they +/// use different connections. +pub struct SqliteExecutionLog { + path: PathBuf, +} + +impl SqliteExecutionLog { + /// Log stored at `path`. The file and its parent are created on first use. + pub fn new(path: PathBuf) -> Self { + Self { path } + } +} + +impl ExecutionLog for SqliteExecutionLog { + fn claim( + &self, + key: ExecutionKey, + request_digest: String, + capability_version: u16, + now: i64, + ) -> BoxFuture<'_, Result> { + let path = self.path.clone(); + Box::pin(async move { + tokio::task::spawn_blocking(move || { + let mut conn = open_broker_db(&path)?; + claim(&mut conn, &key, &request_digest, capability_version, now) + }) + .await + .map_err(|error| format!("broker claim task failed: {error}"))? + }) + } + + fn complete( + &self, + key: ExecutionKey, + state: ExecutionState, + result_json: String, + now: i64, + ) -> BoxFuture<'_, Result<(), String>> { + let path = self.path.clone(); + Box::pin(async move { + tokio::task::spawn_blocking(move || { + let conn = open_broker_db(&path)?; + complete(&conn, &key, state, &result_json, now) + }) + .await + .map_err(|error| format!("broker complete task failed: {error}"))? + }) + } +} + +/// Read one execution row, if it exists. +/// +/// Test-only: the pipeline never reads a record outside [`claim`], which returns +/// what it found in the same transaction that decided whether to claim. A +/// separate read would be a second, racier way to ask the same question. +#[cfg(test)] +pub fn get(conn: &Connection, key: &ExecutionKey) -> Result, String> { + conn.query_row( + "SELECT state, request_digest, capability_version, result_json + FROM broker_executions + WHERE relay_scope = ?1 AND owner_pubkey = ?2 AND requester_pubkey = ?3 + AND capability = ?4 AND request_id = ?5", + params![ + key.relay_scope, + key.owner_pubkey, + key.requester_pubkey, + key.capability, + key.request_id + ], + |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, Option>(3)?, + )) + }, + ) + .optional() + .map_err(|e| format!("failed to read broker execution: {e}"))? + .map(|(state, request_digest, version, result_json)| { + Ok(ExecutionRecord { + state: ExecutionState::parse(&state)?, + request_digest, + capability_version: u16::try_from(version) + .map_err(|_| format!("stored capability_version {version} out of range"))?, + result_json, + }) + }) + .transpose() +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/broker/store/tests.rs b/desktop/src-tauri/src/broker/store/tests.rs new file mode 100644 index 00000000000..9157d28af17 --- /dev/null +++ b/desktop/src-tauri/src/broker/store/tests.rs @@ -0,0 +1,297 @@ +//! Durability and idempotency tests for the broker execution log. + +use super::*; + +const RELAY: &str = "wss://relay.example"; +const OWNER: &str = "1111111111111111111111111111111111111111111111111111111111111111"; +const REQUESTER: &str = "2222222222222222222222222222222222222222222222222222222222222222"; +const OTHER: &str = "3333333333333333333333333333333333333333333333333333333333333333"; +const DIGEST: &str = "aaaa"; + +fn db() -> Connection { + let conn = Connection::open_in_memory().unwrap(); + conn.execute_batch(SCHEMA).unwrap(); + conn +} + +fn key() -> ExecutionKey { + ExecutionKey { + relay_scope: RELAY.into(), + owner_pubkey: OWNER.into(), + requester_pubkey: REQUESTER.into(), + capability: "agents.create".into(), + request_id: "req-1".into(), + } +} + +#[test] +fn first_claim_wins_and_records_executing_before_any_side_effect() { + let mut conn = db(); + assert_eq!( + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(), + ClaimOutcome::Claimed + ); + + // The row exists and is `executing` even though nothing has completed: + // that is what makes a crash visible instead of invisible. + let record = get(&conn, &key()).unwrap().unwrap(); + assert_eq!(record.state, ExecutionState::Executing); + assert_eq!(record.request_digest, DIGEST); + assert!(record.result_json.is_none()); +} + +#[test] +fn terminal_row_replays_the_recorded_result_instead_of_re_executing() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + complete( + &conn, + &key(), + ExecutionState::Succeeded, + r#"{"status":"succeeded"}"#, + 200, + ) + .unwrap(); + + match claim(&mut conn, &key(), DIGEST, 1, 300).unwrap() { + ClaimOutcome::Replay(record) => { + assert_eq!(record.state, ExecutionState::Succeeded); + assert_eq!(record.result_json.unwrap(), r#"{"status":"succeeded"}"#); + } + other => panic!("expected replay, got {other:?}"), + } +} + +#[test] +fn a_failed_result_also_replays_rather_than_retrying() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + complete( + &conn, + &key(), + ExecutionState::Failed, + r#"{"status":"failed"}"#, + 200, + ) + .unwrap(); + + match claim(&mut conn, &key(), DIGEST, 1, 300).unwrap() { + ClaimOutcome::Replay(record) => assert_eq!(record.state, ExecutionState::Failed), + other => panic!("expected replay, got {other:?}"), + } +} + +/// The core safety property: a request that started and never finished must +/// never be blindly re-executed, because side effects may already exist. +#[test] +fn an_interrupted_execution_becomes_indeterminate_and_is_never_re_executed() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + // No `complete` — simulates a crash mid-execution. + + match claim(&mut conn, &key(), DIGEST, 1, 200).unwrap() { + ClaimOutcome::Interrupted(record) => { + assert_eq!(record.state, ExecutionState::Indeterminate); + } + other => panic!("expected interrupted, got {other:?}"), + } + assert_eq!( + get(&conn, &key()).unwrap().unwrap().state, + ExecutionState::Indeterminate + ); + + // And it stays indeterminate: a third attempt still refuses to run. + match claim(&mut conn, &key(), DIGEST, 1, 300).unwrap() { + ClaimOutcome::Replay(record) => { + assert_eq!(record.state, ExecutionState::Indeterminate) + } + other => panic!("expected replay of indeterminate, got {other:?}"), + } +} + +#[test] +fn same_request_id_with_different_content_is_a_conflict() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + + match claim(&mut conn, &key(), "bbbb", 1, 200).unwrap() { + ClaimOutcome::DigestConflict { expected } => assert_eq!(expected, DIGEST), + other => panic!("expected conflict, got {other:?}"), + } +} + +/// A conflicting retry must never be answered with the first request's result. +#[test] +fn digest_conflict_is_reported_even_when_a_result_already_exists() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + complete( + &conn, + &key(), + ExecutionState::Succeeded, + r#"{"status":"succeeded"}"#, + 200, + ) + .unwrap(); + + match claim(&mut conn, &key(), "different", 1, 300).unwrap() { + ClaimOutcome::DigestConflict { expected } => assert_eq!(expected, DIGEST), + other => panic!("a conflicting payload must not receive the stored result: {other:?}"), + } +} + +/// The key includes requester and owner, so one requester's id cannot collide +/// with another's — or leak another's recorded outcome. +#[test] +fn the_key_separates_requesters_owners_relays_and_capabilities() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + + let variants = [ + ExecutionKey { + requester_pubkey: OTHER.into(), + ..key() + }, + ExecutionKey { + owner_pubkey: OTHER.into(), + ..key() + }, + ExecutionKey { + relay_scope: "wss://other.example".into(), + ..key() + }, + ExecutionKey { + capability: "agents.delete".into(), + ..key() + }, + ]; + for variant in variants { + assert_eq!( + claim(&mut conn, &variant, DIGEST, 1, 100).unwrap(), + ClaimOutcome::Claimed, + "{variant:?} must be a distinct execution" + ); + } +} + +#[test] +fn complete_refuses_a_non_terminal_state() { + let mut conn = db(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + let error = complete(&conn, &key(), ExecutionState::Executing, "{}", 200).unwrap_err(); + assert!(error.contains("cannot complete"), "unexpected: {error}"); +} + +#[test] +fn complete_refuses_an_unclaimed_key() { + let conn = db(); + let error = complete(&conn, &key(), ExecutionState::Succeeded, "{}", 200).unwrap_err(); + assert!(error.contains("missing"), "unexpected: {error}"); +} + +#[test] +fn states_round_trip_through_their_database_encoding() { + for state in [ + ExecutionState::Executing, + ExecutionState::Succeeded, + ExecutionState::Failed, + ExecutionState::Indeterminate, + ] { + assert_eq!(ExecutionState::parse(state.as_str()).unwrap(), state); + } + assert!(ExecutionState::parse("bogus").is_err()); + assert!(!ExecutionState::Executing.is_terminal()); + for terminal in [ + ExecutionState::Succeeded, + ExecutionState::Failed, + ExecutionState::Indeterminate, + ] { + assert!(terminal.is_terminal()); + } +} + +/// Equivalent relay URLs must resolve to one scope, so "same relay" cannot +/// disagree with "same database". +#[test] +fn relay_scope_and_db_path_normalize_equivalent_urls() { + assert_eq!( + normalized_relay_scope(" wss://r.example/ "), + "wss://r.example" + ); + + let base = Path::new("/tmp/nest"); + let a = scoped_broker_db_path(base, "wss://r.example", OWNER); + let b = scoped_broker_db_path(base, " wss://r.example/ ", &OWNER.to_ascii_uppercase()); + assert_eq!(a, b); + + // Different owner or relay must not share a database. + assert_ne!(a, scoped_broker_db_path(base, "wss://r.example", OTHER)); + assert_ne!(a, scoped_broker_db_path(base, "wss://other.example", OWNER)); + + // The relay URL never becomes a path component. + assert!(!a.to_string_lossy().contains("r.example")); +} + +#[test] +fn opening_the_db_twice_is_idempotent_and_preserves_rows() { + let dir = tempfile::tempdir().unwrap(); + let path = scoped_broker_db_path(dir.path(), RELAY, OWNER); + + let mut conn = open_broker_db(&path).unwrap(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap(); + complete(&conn, &key(), ExecutionState::Succeeded, "{}", 200).unwrap(); + drop(conn); + + let conn = open_broker_db(&path).unwrap(); + assert_eq!( + get(&conn, &key()).unwrap().unwrap().state, + ExecutionState::Succeeded + ); +} + +/// Two racing frames carrying the same request must not both believe they +/// claimed it — exactly one executes, the other is told to stand down. +#[test] +fn concurrent_claims_produce_exactly_one_winner() { + use std::sync::{Arc, Barrier}; + + let dir = tempfile::tempdir().unwrap(); + let path = scoped_broker_db_path(dir.path(), RELAY, OWNER); + // Materialize the schema before the threads race on it. + drop(open_broker_db(&path).unwrap()); + + let threads = 8; + let barrier = Arc::new(Barrier::new(threads)); + let handles: Vec<_> = (0..threads) + .map(|_| { + let path = path.clone(); + let barrier = Arc::clone(&barrier); + std::thread::spawn(move || { + let mut conn = open_broker_db(&path).unwrap(); + barrier.wait(); + claim(&mut conn, &key(), DIGEST, 1, 100).unwrap() + }) + }) + .collect(); + + let outcomes: Vec<_> = handles.into_iter().map(|h| h.join().unwrap()).collect(); + let claimed = outcomes + .iter() + .filter(|o| matches!(o, ClaimOutcome::Claimed)) + .count(); + assert_eq!(claimed, 1, "exactly one claim must win: {outcomes:?}"); + + // Every loser saw the in-flight row rather than a free slot. + for outcome in outcomes + .iter() + .filter(|o| !matches!(o, ClaimOutcome::Claimed)) + { + assert!( + matches!( + outcome, + ClaimOutcome::Interrupted(_) | ClaimOutcome::Replay(_) + ), + "unexpected loser outcome: {outcome:?}" + ); + } +} diff --git a/desktop/src-tauri/src/broker/tests.rs b/desktop/src-tauri/src/broker/tests.rs new file mode 100644 index 00000000000..2ec0f045628 --- /dev/null +++ b/desktop/src-tauri/src/broker/tests.rs @@ -0,0 +1,43 @@ +//! Digest tests. + +use super::*; + +#[test] +fn identical_bytes_hash_identically_and_any_change_does_not() { + let payload = br#"{"type":"broker_request","requestId":"req-1"}"#; + assert_eq!( + request_digest(payload).unwrap(), + request_digest(payload).unwrap() + ); + + // Byte-level difference, including key order, is a different request. + let reordered = br#"{"requestId":"req-1","type":"broker_request"}"#; + assert_ne!( + request_digest(payload).unwrap(), + request_digest(reordered).unwrap() + ); +} + +#[test] +fn digest_is_hex_sha256_of_the_exact_bytes() { + // Pinned against an independent SHA-256 of the empty input, so a change to + // the hashing scheme fails loudly rather than silently invalidating every + // recorded digest. + assert_eq!( + request_digest(b"").unwrap(), + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + ); + assert_eq!( + request_digest(b"abc").unwrap(), + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + ); +} + +#[test] +fn an_oversized_payload_is_refused_before_hashing() { + let too_big = vec![b'x'; MAX_REQUEST_BYTES + 1]; + let error = request_digest(&too_big).unwrap_err(); + assert!(error.contains("exceeds"), "unexpected: {error}"); + + assert!(request_digest(&vec![b'x'; MAX_REQUEST_BYTES]).is_ok()); +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 71a5eb3806e..35bca571330 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -2,6 +2,7 @@ mod app_menu; mod app_state; mod archive; +mod broker; mod builderlab; mod commands; mod deep_link; @@ -610,6 +611,7 @@ pub fn run() { sign_out, decrypt_observer_event, build_observer_control_event, + broker::host::broker_handle_observer_frame, create_auth_event, nip44_encrypt_to_self, nip44_decrypt_from_self, diff --git a/desktop/src/features/agents/brokerFrameForwarding.test.mjs b/desktop/src/features/agents/brokerFrameForwarding.test.mjs new file mode 100644 index 00000000000..c444a15baf5 --- /dev/null +++ b/desktop/src/features/agents/brokerFrameForwarding.test.mjs @@ -0,0 +1,126 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + BROKER_REQUEST_TYPE, + isBrokerRequestPayload, +} from "./brokerFrameForwarding.ts"; + +test("a broker request payload is recognized by its type discriminator", () => { + assert.equal( + isBrokerRequestPayload({ + type: BROKER_REQUEST_TYPE, + protocolVersion: 1, + requestId: "req-1", + }), + true, + ); +}); + +test("an ill-formed broker request is still forwarded", () => { + // Validation belongs to the host: a request the renderer judged invalid and + // dropped would leave the requester waiting with no answer at all. + assert.equal(isBrokerRequestPayload({ type: BROKER_REQUEST_TYPE }), true); +}); + +test("agent telemetry is not forwarded", () => { + for (const payload of [ + { kind: "turn_started", seq: 3 }, + { type: "agent_management_request", action: "create" }, + { type: "broker_result", requestId: "req-1" }, + null, + undefined, + "broker_request", + 42, + ]) { + assert.equal( + isBrokerRequestPayload(payload), + false, + `unexpectedly treated as a broker request: ${JSON.stringify(payload)}`, + ); + } +}); + +// ── Forwarding: signed-event payload and fault isolation ────────────────────── +// +// The Tauri IPC bridge is stubbed at globalThis.__TAURI_INTERNALS__.invoke, the +// same pattern acpRuntimesQuery.test.mjs uses, so the command name and payload +// are observed directly. + +const SIGNED_FRAME = { + id: "ffff", + pubkey: "aaaa", + created_at: 1, + kind: 24200, + tags: [["frame", "telemetry"]], + content: "nip44-ciphertext", + sig: "bbbb", +}; + +function stubInvoke(handler) { + // @tauri-apps/api reads window.__TAURI_INTERNALS__, so the shim needs a + // window; jsdom is not loaded for this plain-helper test. + const hadWindow = typeof globalThis.window !== "undefined"; + if (!hadWindow) { + globalThis.window = globalThis; + } + const previous = globalThis.window.__TAURI_INTERNALS__; + const calls = []; + globalThis.window.__TAURI_INTERNALS__ = { + invoke: async (command, payload) => { + calls.push({ command, payload }); + return handler(command, payload); + }, + }; + return { + calls, + restore() { + globalThis.window.__TAURI_INTERNALS__ = previous; + if (!hadWindow) { + delete globalThis.window; + } + }, + }; +} + +test("forwarding hands the host the signed event, not the plaintext", async () => { + const stub = stubInvoke(() => ({ + brokerRequest: true, + requestId: "req-1", + resultDelivered: true, + })); + try { + const { forwardBrokerFrame } = await import("./brokerFrameForwarding.ts"); + const handled = await forwardBrokerFrame(SIGNED_FRAME); + + assert.equal(stub.calls.length, 1); + assert.equal(stub.calls[0].command, "broker_handle_observer_frame"); + // The host must re-verify and re-decrypt from the bytes the requester + // signed; a decrypted payload would make the renderer the authority on what + // the request says. + assert.deepEqual(JSON.parse(stub.calls[0].payload.eventJson), SIGNED_FRAME); + assert.equal(handled.requestId, "req-1"); + } finally { + stub.restore(); + } +}); + +test("a broker host fault does not escape to the observer subscription", async () => { + // Regression: forwarding used to run inside the observer decrypt try/catch, so + // a broker fault surfaced as "Observer event decrypt failed" and tore down the + // telemetry subscription for every agent over one failed mutation. + const stub = stubInvoke(() => { + throw new Error("owner keys unavailable"); + }); + const consoleError = console.error; + console.error = () => {}; + try { + const { forwardBrokerFrameIsolated } = await import( + "./brokerFrameForwarding.ts" + ); + assert.equal(await forwardBrokerFrameIsolated(SIGNED_FRAME), null); + } finally { + console.error = consoleError; + stub.restore(); + } +}); diff --git a/desktop/src/features/agents/brokerFrameForwarding.ts b/desktop/src/features/agents/brokerFrameForwarding.ts new file mode 100644 index 00000000000..b9fcc706b32 --- /dev/null +++ b/desktop/src/features/agents/brokerFrameForwarding.ts @@ -0,0 +1,76 @@ +import { invokeTauri } from "@/shared/api/tauri"; +import type { RelayEvent } from "@/shared/api/types"; + +/** + * Wire `type` discriminator on a broker request payload. Must match + * `BROKER_REQUEST_TYPE` in `crates/buzz-sdk/src/broker/mod.rs`. + */ +export const BROKER_REQUEST_TYPE = "broker_request"; + +export type BrokerFrameHandled = { + brokerRequest: boolean; + requestId: string | null; + resultDelivered: boolean; +}; + +/** + * True when a decrypted observer payload is a broker request rather than agent + * telemetry. + * + * This is a **routing** decision, not a trust decision. Rust re-verifies the + * signature, re-derives the owner and requester from the frame, re-decrypts, and + * re-validates before anything executes — so a wrong answer here can only cause + * a broker request to be missed (the requester's wait expires) or a non-request + * to be forwarded and ignored. It cannot cause an unauthorized mutation. + * + * The check is deliberately shallow: the renderer must not decide whether a + * request is *valid*, only whether it claims to be one. An ill-formed broker + * request has to reach the host so the requester gets a structured refusal + * instead of silence. + */ +export function isBrokerRequestPayload(payload: unknown): boolean { + return ( + typeof payload === "object" && + payload !== null && + (payload as { type?: unknown }).type === BROKER_REQUEST_TYPE + ); +} + +/** + * Hand a signed observer frame to the Rust broker for execution. + * + * The **raw signed event** is forwarded, not the decrypted payload: the host + * must verify and decrypt it itself, from bytes the requester signed. Passing + * the plaintext would make the renderer the thing that decides what a request + * says, which is precisely the authority split the broker exists to avoid. + */ +export async function forwardBrokerFrame( + event: RelayEvent, +): Promise { + return invokeTauri("broker_handle_observer_frame", { + eventJson: JSON.stringify(event), + }); +} + +/** + * Forward a broker frame without letting a host fault escape to the caller. + * + * The observer subscription carries agent telemetry for the whole app. A broker + * host fault is a different subsystem's failure, so it must not surface as an + * observer transport error — doing so would report a misleading cause and tear + * down telemetry for every agent over a failed mutation. The requester is + * unaffected either way: it learns the outcome from its own signed result frame, + * or hears nothing and retries with the same `requestId`. + * + * Returns `null` when forwarding failed. + */ +export async function forwardBrokerFrameIsolated( + event: RelayEvent, +): Promise { + try { + return await forwardBrokerFrame(event); + } catch (error) { + console.error("Broker frame forwarding failed", error); + return null; + } +} diff --git a/desktop/src/features/agents/observerRelayStore.ts b/desktop/src/features/agents/observerRelayStore.ts index 68fa290ad25..60143c422d2 100644 --- a/desktop/src/features/agents/observerRelayStore.ts +++ b/desktop/src/features/agents/observerRelayStore.ts @@ -11,6 +11,10 @@ import { parseAgentManagementRequest, type AgentManagementRequest, } from "./agentManagement"; +import { + forwardBrokerFrameIsolated, + isBrokerRequestPayload, +} from "./brokerFrameForwarding"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { useQueryClient } from "@tanstack/react-query"; import { agentConfigSurfaceQueryKey } from "@/features/agents/hooks"; @@ -559,11 +563,25 @@ async function handleRelayObserverEvent( } try { - const parsed = (await decryptObserverEvent(event)) as ObserverEvent; + const parsed = await decryptObserverEvent(event); if (activeGeneration !== generation) { return; } - processLiveObserverEvents(agentPubkey, unwrapObserverBatch(parsed)); + // Broker requests are not observer telemetry: the payload is a bare + // BrokerRequest, with no seq/timestamp/kind, so appending it to the session + // journal would file a mutation request as a transcript entry. Route it to + // the host instead, and forward the *signed event* rather than this + // plaintext — the host verifies and decrypts it again itself. + if (isBrokerRequestPayload(parsed)) { + // Isolated: a broker host fault must not be reported as — or tear down — + // the observer telemetry subscription. See forwardBrokerFrameIsolated. + await forwardBrokerFrameIsolated(event); + return; + } + processLiveObserverEvents( + agentPubkey, + unwrapObserverBatch(parsed as ObserverEvent), + ); } catch (error) { if (activeGeneration !== generation) { return;