From b6779728268eb8a5ef72dfaa44b1246f3fe7177d Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 16:18:24 -0700 Subject: [PATCH 1/5] broker: add trusted-operation wire protocol to buzz-sdk Adds the request/result envelope for the Buzz trusted-operation broker, with agent CRUD as its first capability. This is protocol and validation only -- no host, no dispatch, no execution. The envelope is deliberately minimal: { type, protocolVersion, requestId, capabilityVersion, capability, args } It carries no owner, requester, or relay identity. Those are derived on the host from the verified frame, because a request body that could name its own owner would let any signer act on another owner's agents. There is no `context` and no `authorization` field yet: a field that looks security-bearing while enforcing nothing is worse than its absence. One gets added, as a discriminated object, when a real grant format and verifier exist. There is no client-computed digest. Idempotency is decided host-side, so only the host needs the hash -- shipping a second canonicalizer in a second language would freeze two implementations that must agree byte-for-byte forever. Retrying means resending identical bytes. Results are a three-variant discriminated union so invalid combinations are unrepresentable rather than merely discouraged: Succeeded { outcome } | Failed { error } | Indeterminate { error } Indeterminate is distinct from Failed on purpose. Failed promises no side effects took hold; Indeterminate promises nothing and demands reconciliation. Capabilities stay three separate names (agents.create/update/delete) rather than one agents.manage action union, so a host can permit one without permitting the others. Only business operations are addressable: signing, publishing, credential access, and tool execution are not capabilities and cannot be named. channelId appears only on create, where attachment genuinely needs it. Update and delete identify their target by the agent itself. Every args type is deny_unknown_fields, so a smuggled api key or nsec fails to deserialize instead of reaching a mutation, and outcome types can structurally hold only public identifiers. Signed-off-by: ss-dev-00 Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- crates/buzz-sdk/src/broker/capabilities.rs | 406 +++++++++++++++++++++ crates/buzz-sdk/src/broker/mod.rs | 401 ++++++++++++++++++++ crates/buzz-sdk/src/broker/tests.rs | 382 +++++++++++++++++++ crates/buzz-sdk/src/lib.rs | 1 + 4 files changed, 1190 insertions(+) create mode 100644 crates/buzz-sdk/src/broker/capabilities.rs create mode 100644 crates/buzz-sdk/src/broker/mod.rs create mode 100644 crates/buzz-sdk/src/broker/tests.rs 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; From b173f46e75c20699116ff46de14b7e71af858579 Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 16:31:29 -0700 Subject: [PATCH 2/5] broker: add host authority pipeline, execution log, and agents policy The trusted path lives in one Rust function: verified frame -> authorize -> validate -> claim -> dispatch -> complete It is a single function rather than coordinated steps because every stage boundary is somewhere 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. Identity arrives as VerifiedRequest, whose construction asserts that owner, requester, and relay scope came from the verified transport. Nothing in the request body can influence them, so a signer cannot name someone else as owner. A payload that tries fails to deserialize. The digest is computed here, over the exact decrypted bytes, after a size bound. No canonical encoding has to be agreed on and no second implementation can drift from this one. The execution log is keyed by (relay_scope, owner, requester, capability, request_id) and inserts `executing` before the first side effect. Terminal rows replay; a digest mismatch is a conflict, checked before state so a conflicting retry can never be answered with the first request's result; an existing `executing` row becomes `indeterminate` and is never blindly re-executed. Claim runs in an IMMEDIATE transaction -- a test races eight threads to show exactly one winner. This guarantees at-most-once plus indeterminate. Not exactly-once, and not durable completion: the log can prove execution started, never how far it got. Reconciling partial effects belongs to the capability that knows its own phases. Authorization is split from authentication. The broker proves who signed; whether that signer may manage agents is an agents.* scope rule, so a future capability cannot inherit it. The rule is that the requester must be one of this owner's managed agents, read from the authoritative Rust roster. It fails closed when the roster cannot be read, and refuses the owner key itself as a requester. Per the approved design, there is no channel restriction on the target of an update or delete: an owner's agent is theirs regardless of channel, and requiring co-membership would look tighter than it is while adding no real constraint. Channel appears only on create, where attachment needs it. Signed-off-by: ss-dev-00 Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- desktop/src-tauri/src/broker/agents_policy.rs | 213 ++++++++ desktop/src-tauri/src/broker/mod.rs | 71 +++ desktop/src-tauri/src/broker/pipeline.rs | 252 +++++++++ .../src-tauri/src/broker/pipeline/tests.rs | 511 ++++++++++++++++++ desktop/src-tauri/src/broker/store.rs | 405 ++++++++++++++ desktop/src-tauri/src/broker/store/tests.rs | 297 ++++++++++ desktop/src-tauri/src/broker/tests.rs | 43 ++ desktop/src-tauri/src/lib.rs | 1 + 8 files changed, 1793 insertions(+) create mode 100644 desktop/src-tauri/src/broker/agents_policy.rs create mode 100644 desktop/src-tauri/src/broker/mod.rs create mode 100644 desktop/src-tauri/src/broker/pipeline.rs create mode 100644 desktop/src-tauri/src/broker/pipeline/tests.rs create mode 100644 desktop/src-tauri/src/broker/store.rs create mode 100644 desktop/src-tauri/src/broker/store/tests.rs create mode 100644 desktop/src-tauri/src/broker/tests.rs 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..ea83ea2faf5 --- /dev/null +++ b/desktop/src-tauri/src/broker/agents_policy.rs @@ -0,0 +1,213 @@ +//! 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, 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 { + /// Pubkeys of every agent this owner manages. + fn agent_pubkeys(&self, owner_pubkey: &str) -> 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( + &self, + request: &VerifiedRequest, + capability: Capability, + _parsed: &BrokerRequest, + ) -> Result<(), BrokerError> { + 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) + .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(&self, _owner_pubkey: &str) -> Result, String> { + self.0.clone() + } + } + + 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() + } + + fn authorize( + roster: Result, String>, + requester: &str, + capability: Capability, + ) -> Result<(), BrokerError> { + AgentsAuthorizer::new(Roster(roster)).authorize(&request(requester), capability, &parsed()) + } + + #[test] + 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).is_ok()); + } + } + + #[test] + fn a_signer_who_is_not_this_owners_agent_is_refused() { + let error = + authorize(Ok(vec![AGENT.into()]), STRANGER, Capability::AgentsDelete).unwrap_err(); + assert!( + error.message.contains("not an agent managed by this owner"), + "unexpected: {}", + error.message + ); + } + + #[test] + fn an_empty_roster_refuses_everyone() { + assert!(authorize(Ok(Vec::new()), AGENT, Capability::AgentsCreate).is_err()); + } + + /// The owner key signing its own broker request would erase the delegation + /// boundary the broker exists to enforce. + #[test] + fn the_owner_key_is_not_a_requester() { + let error = authorize(Ok(vec![OWNER.into()]), OWNER, Capability::AgentsCreate).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. + #[test] + fn an_unreadable_roster_fails_closed() { + let error = authorize( + Err("keyring locked".into()), + AGENT, + Capability::AgentsUpdate, + ) + .unwrap_err(); + assert!( + error.message.contains("could not verify agent ownership"), + "unexpected: {}", + error.message + ); + } + + #[test] + fn pubkey_comparison_ignores_hex_case() { + assert!(authorize( + Ok(vec![AGENT.to_ascii_uppercase()]), + AGENT, + Capability::AgentsCreate + ) + .is_ok()); + } +} diff --git a/desktop/src-tauri/src/broker/mod.rs b/desktop/src-tauri/src/broker/mod.rs new file mode 100644 index 00000000000..3c5bcbda7b2 --- /dev/null +++ b/desktop/src-tauri/src/broker/mod.rs @@ -0,0 +1,71 @@ +//! 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 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..6c88c22a479 --- /dev/null +++ b/desktop/src-tauri/src/broker/pipeline.rs @@ -0,0 +1,252 @@ +//! 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. +//! +//! # 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 buzz_sdk_pkg::broker::{ + BrokerError, BrokerErrorCode, BrokerRequest, BrokerResponse, BrokerResult, Capability, + CapabilityOutcome, +}; + +use super::store::{self, ClaimOutcome, ExecutionKey, ExecutionState}; + +/// 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, +} + +/// 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 { + /// Authorize `request` for `capability`, or explain the refusal. + fn authorize( + &self, + request: &VerifiedRequest, + capability: Capability, + parsed: &BrokerRequest, + ) -> 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 { + /// 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(&self, request: &VerifiedRequest, parsed: &BrokerRequest) -> HandlerOutcome; +} + +/// Resolves a capability name to its handler. +pub trait CapabilityRegistry { + /// 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 conn: &'a mut rusqlite::Connection, + /// 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 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) { + 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(), + }; + + match store::claim(ctx.conn, &key, &digest, parsed.capability_version, ctx.now)? { + 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) { + 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}"))?; + store::complete(ctx.conn, &key, state, &result_json, ctx.now)?; + + 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..70fb18b85dc --- /dev/null +++ b/desktop/src-tauri/src/broker/pipeline/tests.rs @@ -0,0 +1,511 @@ +//! Pipeline tests: authority, idempotency, and crash behavior. + +use std::cell::RefCell; + +use buzz_sdk_pkg::broker::{ + AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, AgentsDeleteArgs, AgentsDeleteOutcome, + CapabilityArgs, +}; + +use super::*; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const OWNER: &str = "1111111111111111111111111111111111111111111111111111111111111111"; +const REQUESTER: &str = "2222222222222222222222222222222222222222222222222222222222222222"; +const AGENT: &str = "3333333333333333333333333333333333333333333333333333333333333333"; + +fn db() -> rusqlite::Connection { + super::super::store::open_in_memory() +} + +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( + &self, + _request: &VerifiedRequest, + _capability: Capability, + _parsed: &BrokerRequest, + ) -> Result<(), BrokerError> { + Ok(()) + } +} + +struct DenyAll; +impl Authorizer for DenyAll { + fn authorize( + &self, + _request: &VerifiedRequest, + _capability: Capability, + _parsed: &BrokerRequest, + ) -> Result<(), BrokerError> { + 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: RefCell, + outcome: fn() -> HandlerOutcome, +} + +impl CountingHandler { + fn new(outcome: fn() -> HandlerOutcome) -> Self { + Self { + calls: RefCell::new(0), + outcome, + } + } +} + +impl CapabilityHandler for CountingHandler { + fn execute(&self, _request: &VerifiedRequest, _parsed: &BrokerRequest) -> HandlerOutcome { + *self.calls.borrow_mut() += 1; + (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(), + })) +} + +fn run<'a>( + request: &VerifiedRequest, + authorizer: &'a dyn Authorizer, + registry: &'a dyn CapabilityRegistry, + conn: &'a mut rusqlite::Connection, +) -> BrokerResponse { + execute( + request, + BrokerContext { + authorizer, + registry, + conn, + now: 1000, + }, + ) + .unwrap() +} + +#[test] +fn a_valid_authorized_request_executes_once_and_returns_its_outcome() { + let mut conn = 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, &mut conn); + assert_eq!(response.request_id, "req-1"); + assert!(response.result.is_succeeded()); + assert!(!response.replayed); + assert_eq!(*handler.calls.borrow(), 1); +} + +/// The central idempotency property: replaying the identical request returns +/// the recorded outcome and does not run the handler a second time. +#[test] +fn replaying_an_identical_request_returns_the_record_without_re_executing() { + let mut conn = 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, &mut conn); + let second = run(&request, &AllowAll, ®istry, &mut conn); + + assert_eq!(*handler.calls.borrow(), 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. +#[test] +fn a_different_payload_under_the_same_request_id_is_refused() { + let mut conn = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + + run( + &verified(create_request("req-1")), + &AllowAll, + ®istry, + &mut conn, + ); + + 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, + &mut conn, + ); + + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::RequestIdConflict + ); + assert_eq!(*handler.calls.borrow(), 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. +#[test] +fn an_unauthorized_request_does_not_execute_or_leave_a_claim() { + let mut conn = 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, &mut conn); + assert_eq!( + denied.result.error().unwrap().code, + BrokerErrorCode::Unauthorized + ); + assert_eq!(*handler.calls.borrow(), 0); + + // The same request, now authorized, still runs normally. + let allowed = run(&request, &AllowAll, ®istry, &mut conn); + assert!(allowed.result.is_succeeded()); + assert!(!allowed.replayed); + assert_eq!(*handler.calls.borrow(), 1); +} + +/// A crash between the mutation and its record must surface as indeterminate, +/// never as a silent re-run. +#[test] +fn an_interrupted_execution_reports_indeterminate_and_never_re_executes() { + let mut conn = 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(); + store::claim(&mut conn, &key, &digest, 1, 500).unwrap(); + + let response = run(&request, &AllowAll, ®istry, &mut conn); + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::OutcomeUnknown + ); + assert_eq!( + *handler.calls.borrow(), + 0, + "an interrupted request must not be re-executed" + ); +} + +#[test] +fn a_failed_handler_is_recorded_and_replayed_as_failed() { + fn failure() -> HandlerOutcome { + HandlerOutcome::Failed(BrokerError::new( + BrokerErrorCode::CapabilityFailed, + "runtime not installed", + )) + } + let mut conn = 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, &mut conn); + 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, &mut conn); + assert!(second.replayed); + assert_eq!(first.result, second.result); + assert_eq!(*handler.calls.borrow(), 1); +} + +#[test] +fn an_indeterminate_handler_outcome_is_recorded_as_indeterminate() { + fn unknown() -> HandlerOutcome { + HandlerOutcome::Indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "published but could not confirm", + )) + } + let mut conn = 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, &mut conn); + assert!(matches!( + response.result, + BrokerResult::Indeterminate { .. } + )); + assert_eq!(*handler.calls.borrow(), 1); +} + +#[test] +fn a_malformed_payload_is_refused_without_a_claim() { + let mut conn = db(); + let response = run( + &verified(b"not json".to_vec()), + &AllowAll, + &NoHandlers, + &mut conn, + ); + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); +} + +#[test] +fn an_oversized_payload_is_refused() { + let mut conn = db(); + let response = run( + &verified(vec![b'x'; super::super::MAX_REQUEST_BYTES + 1]), + &AllowAll, + &NoHandlers, + &mut conn, + ); + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); +} + +#[test] +fn a_capability_this_host_does_not_offer_is_refused() { + let mut conn = db(); + let response = run( + &verified(create_request("req-1")), + &AllowAll, + &NoHandlers, + &mut conn, + ); + 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. +#[test] +fn the_same_request_id_from_two_requesters_are_separate_executions() { + let mut conn = 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, &mut conn).replayed); + assert!( + !run(&second, &AllowAll, ®istry, &mut conn).replayed, + "a different requester must not replay another's record" + ); + assert_eq!(*handler.calls.borrow(), 2); +} + +/// The pipeline must take identity from the verified transport, never from the +/// payload. A body claiming another owner cannot even deserialize. +#[test] +fn identity_in_the_payload_cannot_override_transport_identity() { + let mut conn = 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, + &mut conn, + ); + assert_eq!( + response.result.error().unwrap().code, + BrokerErrorCode::InvalidRequest + ); + assert_eq!(*handler.calls.borrow(), 0); +} + +/// The authorizer sees the transport-derived identity, so policy is written +/// against who actually signed. +#[test] +fn the_authorizer_receives_transport_derived_identity() { + struct Recording(RefCell>); + impl Authorizer for Recording { + fn authorize( + &self, + request: &VerifiedRequest, + capability: Capability, + _parsed: &BrokerRequest, + ) -> Result<(), BrokerError> { + self.0.borrow_mut().push(( + request.owner_pubkey.clone(), + request.requester_pubkey.clone(), + capability.as_str().to_string(), + )); + Ok(()) + } + } + + let mut conn = db(); + let handler = CountingHandler::new(success); + let registry = OneHandler { + capability: Capability::AgentsCreate, + handler: &handler, + }; + let authorizer = Recording(RefCell::new(Vec::new())); + + run( + &verified(create_request("req-1")), + &authorizer, + ®istry, + &mut conn, + ); + + assert_eq!( + authorizer.0.borrow().as_slice(), + &[( + OWNER.to_string(), + REQUESTER.to_string(), + "agents.create".to_string() + )] + ); +} + +#[test] +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 mut conn = 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, + &mut conn, + ); + + assert!(response.result.is_succeeded()); + assert_eq!(*handler.calls.borrow(), 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..aa417ebb8b9 --- /dev/null +++ b/desktop/src-tauri/src/broker/store.rs @@ -0,0 +1,405 @@ +//! 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}; + +/// 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 +} + +/// Read one execution row, if it exists. +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..807f2dbc73c 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; From 86617f5f5f71b59eefef24af4c45d0e60d727786 Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 17:04:23 -0700 Subject: [PATCH 3/5] broker: add agents.* capability handlers behind an async service seam The three capabilities dispatch to an AgentService trait rather than to the Tauri commands directly. Those commands need an AppHandle and a State, so depending on them here would make every handler untestable without booting an app and would entangle the capability layer with the desktop runtime. Behind the seam, handlers are pure and the tests below drive the real pipeline against a fake. The seams are async because the mutations they will call are: minting a key, writing the agent store, and publishing to the relay all await. That forces ExecutionLog to be a trait instead of a borrowed rusqlite::Connection, which is !Sync and must not be held across an .await. The production adapter confines each connection to one blocking task; the test double serializes behind a Mutex. Cross-call exclusion is SQLite's either way, since claim() does its read and insert inside a single IMMEDIATE transaction. ServiceError distinguishes Failed from Unknown. Failed promises no side effects persisted; Unknown promises nothing and becomes indeterminate. A service that mutated and then lost track must say Unknown, because reporting a clean failure would invite a retry of something that may already have happened. Nothing in AgentService returns secret material: a create reports the new agent's pubkey, never its nsec. A test serializes a create response and asserts the absence of nsec, privateKey, private_key, and secret, so the guarantee is checked rather than asserted in a comment. Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- crates/buzz-core/src/observer.rs | 25 +- desktop/src-tauri/src/broker/agents_policy.rs | 151 ++++---- desktop/src-tauri/src/broker/handlers.rs | 199 ++++++++++ .../src-tauri/src/broker/handlers/tests.rs | 355 ++++++++++++++++++ desktop/src-tauri/src/broker/mod.rs | 1 + desktop/src-tauri/src/broker/pipeline.rs | 86 ++++- .../src-tauri/src/broker/pipeline/tests.rs | 264 +++++++------ desktop/src-tauri/src/broker/store.rs | 114 ++++++ 8 files changed, 993 insertions(+), 202 deletions(-) create mode 100644 desktop/src-tauri/src/broker/handlers.rs create mode 100644 desktop/src-tauri/src/broker/handlers/tests.rs 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/desktop/src-tauri/src/broker/agents_policy.rs b/desktop/src-tauri/src/broker/agents_policy.rs index ea83ea2faf5..a634347b23e 100644 --- a/desktop/src-tauri/src/broker/agents_policy.rs +++ b/desktop/src-tauri/src/broker/agents_policy.rs @@ -25,15 +25,18 @@ use buzz_sdk_pkg::broker::{BrokerError, BrokerRequest, Capability}; -use super::pipeline::{Authorizer, VerifiedRequest}; +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 { +pub trait AgentRoster: Send + Sync { /// Pubkeys of every agent this owner manages. - fn agent_pubkeys(&self, owner_pubkey: &str) -> Result, String>; + fn agent_pubkeys<'a>( + &'a self, + owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>>; } /// `agents.*` authorization backed by an [`AgentRoster`]. @@ -49,47 +52,51 @@ impl AgentsAuthorizer { } impl Authorizer for AgentsAuthorizer { - fn authorize( - &self, - request: &VerifiedRequest, + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, capability: Capability, - _parsed: &BrokerRequest, - ) -> Result<(), BrokerError> { - 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) - .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", - )) - } + _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", + )) + } + }) } } @@ -106,8 +113,12 @@ mod tests { struct Roster(Result, String>); impl AgentRoster for Roster { - fn agent_pubkeys(&self, _owner_pubkey: &str) -> Result, String> { - self.0.clone() + fn agent_pubkeys<'a>( + &'a self, + _owner_pubkey: &'a str, + ) -> BoxFuture<'a, Result, String>> { + let result = self.0.clone(); + Box::pin(async move { result }) } } @@ -136,29 +147,35 @@ mod tests { .unwrap() } - fn authorize( + async fn authorize( roster: Result, String>, requester: &str, capability: Capability, ) -> Result<(), BrokerError> { - AgentsAuthorizer::new(Roster(roster)).authorize(&request(requester), capability, &parsed()) + let authorizer = AgentsAuthorizer::new(Roster(roster)); + let request = request(requester); + let parsed = parsed(); + authorizer.authorize(&request, capability, &parsed).await } - #[test] - fn an_owners_own_agent_may_manage_agents() { + #[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).is_ok()); + assert!(authorize(Ok(vec![AGENT.into()]), AGENT, capability) + .await + .is_ok()); } } - #[test] - fn a_signer_who_is_not_this_owners_agent_is_refused() { - let error = - authorize(Ok(vec![AGENT.into()]), STRANGER, Capability::AgentsDelete).unwrap_err(); + #[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: {}", @@ -166,16 +183,20 @@ mod tests { ); } - #[test] - fn an_empty_roster_refuses_everyone() { - assert!(authorize(Ok(Vec::new()), AGENT, Capability::AgentsCreate).is_err()); + #[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. - #[test] - fn the_owner_key_is_not_a_requester() { - let error = authorize(Ok(vec![OWNER.into()]), OWNER, Capability::AgentsCreate).unwrap_err(); + #[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 @@ -186,13 +207,14 @@ mod tests { } /// A roster that cannot be read must fail closed. - #[test] - fn an_unreadable_roster_fails_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"), @@ -201,13 +223,14 @@ mod tests { ); } - #[test] - fn pubkey_comparison_ignores_hex_case() { + #[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/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/mod.rs b/desktop/src-tauri/src/broker/mod.rs index 3c5bcbda7b2..af99b3659b1 100644 --- a/desktop/src-tauri/src/broker/mod.rs +++ b/desktop/src-tauri/src/broker/mod.rs @@ -33,6 +33,7 @@ use sha2::{Digest, Sha256}; pub mod agents_policy; +pub mod handlers; pub mod pipeline; pub mod store; diff --git a/desktop/src-tauri/src/broker/pipeline.rs b/desktop/src-tauri/src/broker/pipeline.rs index 6c88c22a479..52716fc6af9 100644 --- a/desktop/src-tauri/src/broker/pipeline.rs +++ b/desktop/src-tauri/src/broker/pipeline.rs @@ -12,6 +12,20 @@ //! 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 @@ -19,6 +33,9 @@ //! 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, @@ -26,6 +43,9 @@ use buzz_sdk_pkg::broker::{ 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`, @@ -43,6 +63,34 @@ pub struct VerifiedRequest { pub payload: Vec, } +/// 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 @@ -50,14 +98,14 @@ pub struct VerifiedRequest { /// *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 { +pub trait Authorizer: Send + Sync { /// Authorize `request` for `capability`, or explain the refusal. - fn authorize( - &self, - request: &VerifiedRequest, + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, capability: Capability, - parsed: &BrokerRequest, - ) -> Result<(), BrokerError>; + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>>; } /// The outcome of running one capability. @@ -75,17 +123,21 @@ pub enum HandlerOutcome { } /// One broker capability implementation. -pub trait CapabilityHandler { +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(&self, request: &VerifiedRequest, parsed: &BrokerRequest) -> HandlerOutcome; + fn execute<'a>( + &'a self, + request: &'a VerifiedRequest, + parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, HandlerOutcome>; } /// Resolves a capability name to its handler. -pub trait CapabilityRegistry { +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>; } @@ -97,7 +149,7 @@ pub struct BrokerContext<'a> { /// Capability implementations. pub registry: &'a dyn CapabilityRegistry, /// Durable execution log. - pub conn: &'a mut rusqlite::Connection, + pub log: &'a dyn ExecutionLog, /// Current unix time, injected so tests are not clock-dependent. pub now: i64, } @@ -108,7 +160,7 @@ pub struct BrokerContext<'a> { /// (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 fn execute( +pub async fn execute( request: &VerifiedRequest, ctx: BrokerContext<'_>, ) -> Result { @@ -152,7 +204,7 @@ pub fn execute( // 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) { + if let Err(error) = ctx.authorizer.authorize(request, capability, &parsed).await { return Ok(BrokerResponse::new( &request_id, BrokerResult::failed(error), @@ -177,7 +229,11 @@ pub fn execute( request_id: request_id.clone(), }; - match store::claim(ctx.conn, &key, &digest, parsed.capability_version, ctx.now)? { + 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())) @@ -208,7 +264,7 @@ pub fn execute( // 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) { + 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), @@ -221,7 +277,7 @@ pub fn execute( }; let result_json = serde_json::to_string(&result) .map_err(|error| format!("failed to encode broker result: {error}"))?; - store::complete(ctx.conn, &key, state, &result_json, ctx.now)?; + ctx.log.complete(key, state, result_json, ctx.now).await?; Ok(BrokerResponse::new(&request_id, result)) } diff --git a/desktop/src-tauri/src/broker/pipeline/tests.rs b/desktop/src-tauri/src/broker/pipeline/tests.rs index 70fb18b85dc..5fbf08f58c3 100644 --- a/desktop/src-tauri/src/broker/pipeline/tests.rs +++ b/desktop/src-tauri/src/broker/pipeline/tests.rs @@ -1,12 +1,14 @@ //! Pipeline tests: authority, idempotency, and crash behavior. -use std::cell::RefCell; +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"; @@ -14,8 +16,8 @@ const OWNER: &str = "11111111111111111111111111111111111111111111111111111111111 const REQUESTER: &str = "2222222222222222222222222222222222222222222222222222222222222222"; const AGENT: &str = "3333333333333333333333333333333333333333333333333333333333333333"; -fn db() -> rusqlite::Connection { - super::super::store::open_in_memory() +fn db() -> MemoryExecutionLog { + MemoryExecutionLog::new() } fn create_request(request_id: &str) -> Vec { @@ -46,47 +48,57 @@ fn verified(payload: Vec) -> VerifiedRequest { struct AllowAll; impl Authorizer for AllowAll { - fn authorize( - &self, - _request: &VerifiedRequest, + fn authorize<'a>( + &'a self, + _request: &'a VerifiedRequest, _capability: Capability, - _parsed: &BrokerRequest, - ) -> Result<(), BrokerError> { - Ok(()) + _parsed: &'a BrokerRequest, + ) -> BoxFuture<'a, Result<(), BrokerError>> { + Box::pin(async { Ok(()) }) } } struct DenyAll; impl Authorizer for DenyAll { - fn authorize( - &self, - _request: &VerifiedRequest, + fn authorize<'a>( + &'a self, + _request: &'a VerifiedRequest, _capability: Capability, - _parsed: &BrokerRequest, - ) -> Result<(), BrokerError> { - Err(BrokerError::unauthorized("not a member of that channel")) + _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: RefCell, + calls: AtomicUsize, outcome: fn() -> HandlerOutcome, } impl CountingHandler { fn new(outcome: fn() -> HandlerOutcome) -> Self { Self { - calls: RefCell::new(0), + calls: AtomicUsize::new(0), outcome, } } + + fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } } impl CapabilityHandler for CountingHandler { - fn execute(&self, _request: &VerifiedRequest, _parsed: &BrokerRequest) -> HandlerOutcome { - *self.calls.borrow_mut() += 1; - (self.outcome)() + 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)() + }) } } @@ -116,27 +128,28 @@ fn success() -> HandlerOutcome { })) } -fn run<'a>( +async fn run( request: &VerifiedRequest, - authorizer: &'a dyn Authorizer, - registry: &'a dyn CapabilityRegistry, - conn: &'a mut rusqlite::Connection, + authorizer: &dyn Authorizer, + registry: &dyn CapabilityRegistry, + log: &dyn ExecutionLog, ) -> BrokerResponse { execute( request, BrokerContext { authorizer, registry, - conn, + log, now: 1000, }, ) + .await .unwrap() } -#[test] -fn a_valid_authorized_request_executes_once_and_returns_its_outcome() { - let mut conn = db(); +#[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, @@ -144,18 +157,18 @@ fn a_valid_authorized_request_executes_once_and_returns_its_outcome() { }; let request = verified(create_request("req-1")); - let response = run(&request, &AllowAll, ®istry, &mut conn); + 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.borrow(), 1); + 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. -#[test] -fn replaying_an_identical_request_returns_the_record_without_re_executing() { - let mut conn = db(); +#[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, @@ -163,19 +176,19 @@ fn replaying_an_identical_request_returns_the_record_without_re_executing() { }; let request = verified(create_request("req-1")); - let first = run(&request, &AllowAll, ®istry, &mut conn); - let second = run(&request, &AllowAll, ®istry, &mut conn); + let first = run(&request, &AllowAll, ®istry, &log).await; + let second = run(&request, &AllowAll, ®istry, &log).await; - assert_eq!(*handler.calls.borrow(), 1, "handler must not run twice"); + 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. -#[test] -fn a_different_payload_under_the_same_request_id_is_refused() { - let mut conn = db(); +#[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, @@ -186,8 +199,9 @@ fn a_different_payload_under_the_same_request_id_is_refused() { &verified(create_request("req-1")), &AllowAll, ®istry, - &mut conn, - ); + &log, + ) + .await; let mut different = BrokerRequest::new( "req-1", @@ -207,21 +221,22 @@ fn a_different_payload_under_the_same_request_id_is_refused() { &verified(serde_json::to_vec(&different).unwrap()), &AllowAll, ®istry, - &mut conn, - ); + &log, + ) + .await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::RequestIdConflict ); - assert_eq!(*handler.calls.borrow(), 1, "the conflict must not execute"); + 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. -#[test] -fn an_unauthorized_request_does_not_execute_or_leave_a_claim() { - let mut conn = db(); +#[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, @@ -229,25 +244,25 @@ fn an_unauthorized_request_does_not_execute_or_leave_a_claim() { }; let request = verified(create_request("req-1")); - let denied = run(&request, &DenyAll, ®istry, &mut conn); + let denied = run(&request, &DenyAll, ®istry, &log).await; assert_eq!( denied.result.error().unwrap().code, BrokerErrorCode::Unauthorized ); - assert_eq!(*handler.calls.borrow(), 0); + assert_eq!(handler.calls(), 0); // The same request, now authorized, still runs normally. - let allowed = run(&request, &AllowAll, ®istry, &mut conn); + let allowed = run(&request, &AllowAll, ®istry, &log).await; assert!(allowed.result.is_succeeded()); assert!(!allowed.replayed); - assert_eq!(*handler.calls.borrow(), 1); + assert_eq!(handler.calls(), 1); } /// A crash between the mutation and its record must surface as indeterminate, /// never as a silent re-run. -#[test] -fn an_interrupted_execution_reports_indeterminate_and_never_re_executes() { - let mut conn = db(); +#[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, @@ -264,29 +279,30 @@ fn an_interrupted_execution_reports_indeterminate_and_never_re_executes() { request_id: "req-1".into(), }; let digest = super::super::request_digest(&request.payload).unwrap(); - store::claim(&mut conn, &key, &digest, 1, 500).unwrap(); + log.with_conn(|conn| store::claim(conn, &key, &digest, 1, 500)) + .unwrap(); - let response = run(&request, &AllowAll, ®istry, &mut conn); + let response = run(&request, &AllowAll, ®istry, &log).await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::OutcomeUnknown ); assert_eq!( - *handler.calls.borrow(), + handler.calls(), 0, "an interrupted request must not be re-executed" ); } -#[test] -fn a_failed_handler_is_recorded_and_replayed_as_failed() { +#[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 mut conn = db(); + let log = db(); let handler = CountingHandler::new(failure); let registry = OneHandler { capability: Capability::AgentsCreate, @@ -294,28 +310,28 @@ fn a_failed_handler_is_recorded_and_replayed_as_failed() { }; let request = verified(create_request("req-1")); - let first = run(&request, &AllowAll, ®istry, &mut conn); + 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, &mut conn); + let second = run(&request, &AllowAll, ®istry, &log).await; assert!(second.replayed); assert_eq!(first.result, second.result); - assert_eq!(*handler.calls.borrow(), 1); + assert_eq!(handler.calls(), 1); } -#[test] -fn an_indeterminate_handler_outcome_is_recorded_as_indeterminate() { +#[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 mut conn = db(); + let log = db(); let handler = CountingHandler::new(unknown); let registry = OneHandler { capability: Capability::AgentsCreate, @@ -323,53 +339,56 @@ fn an_indeterminate_handler_outcome_is_recorded_as_indeterminate() { }; let request = verified(create_request("req-1")); - let response = run(&request, &AllowAll, ®istry, &mut conn); + let response = run(&request, &AllowAll, ®istry, &log).await; assert!(matches!( response.result, BrokerResult::Indeterminate { .. } )); - assert_eq!(*handler.calls.borrow(), 1); + assert_eq!(handler.calls(), 1); } -#[test] -fn a_malformed_payload_is_refused_without_a_claim() { - let mut conn = db(); +#[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, - &mut conn, - ); + &log, + ) + .await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::InvalidRequest ); } -#[test] -fn an_oversized_payload_is_refused() { - let mut conn = db(); +#[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, - &mut conn, - ); + &log, + ) + .await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::InvalidRequest ); } -#[test] -fn a_capability_this_host_does_not_offer_is_refused() { - let mut conn = db(); +#[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, - &mut conn, - ); + &log, + ) + .await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::UnknownCapability @@ -378,9 +397,9 @@ fn a_capability_this_host_does_not_offer_is_refused() { /// Two requesters using the same request id are separate executions: the key /// includes the verified requester, so one cannot read the other's outcome. -#[test] -fn the_same_request_id_from_two_requesters_are_separate_executions() { - let mut conn = db(); +#[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, @@ -394,19 +413,19 @@ fn the_same_request_id_from_two_requesters_are_separate_executions() { ..verified(payload) }; - assert!(!run(&first, &AllowAll, ®istry, &mut conn).replayed); + assert!(!run(&first, &AllowAll, ®istry, &log).await.replayed); assert!( - !run(&second, &AllowAll, ®istry, &mut conn).replayed, + !run(&second, &AllowAll, ®istry, &log).await.replayed, "a different requester must not replay another's record" ); - assert_eq!(*handler.calls.borrow(), 2); + 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. -#[test] -fn identity_in_the_payload_cannot_override_transport_identity() { - let mut conn = db(); +#[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, @@ -422,53 +441,57 @@ fn identity_in_the_payload_cannot_override_transport_identity() { &verified(serde_json::to_vec(&json).unwrap()), &AllowAll, ®istry, - &mut conn, - ); + &log, + ) + .await; assert_eq!( response.result.error().unwrap().code, BrokerErrorCode::InvalidRequest ); - assert_eq!(*handler.calls.borrow(), 0); + assert_eq!(handler.calls(), 0); } /// The authorizer sees the transport-derived identity, so policy is written /// against who actually signed. -#[test] -fn the_authorizer_receives_transport_derived_identity() { - struct Recording(RefCell>); +#[tokio::test] +async fn the_authorizer_receives_transport_derived_identity() { + struct Recording(Mutex>); impl Authorizer for Recording { - fn authorize( - &self, - request: &VerifiedRequest, + fn authorize<'a>( + &'a self, + request: &'a VerifiedRequest, capability: Capability, - _parsed: &BrokerRequest, - ) -> Result<(), BrokerError> { - self.0.borrow_mut().push(( - request.owner_pubkey.clone(), - request.requester_pubkey.clone(), - capability.as_str().to_string(), - )); - Ok(()) + _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 mut conn = db(); + let log = db(); let handler = CountingHandler::new(success); let registry = OneHandler { capability: Capability::AgentsCreate, handler: &handler, }; - let authorizer = Recording(RefCell::new(Vec::new())); + let authorizer = Recording(Mutex::new(Vec::new())); run( &verified(create_request("req-1")), &authorizer, ®istry, - &mut conn, - ); + &log, + ) + .await; assert_eq!( - authorizer.0.borrow().as_slice(), + authorizer.0.lock().unwrap().as_slice(), &[( OWNER.to_string(), REQUESTER.to_string(), @@ -477,15 +500,15 @@ fn the_authorizer_receives_transport_derived_identity() { ); } -#[test] -fn delete_requests_route_to_the_delete_capability() { +#[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 mut conn = db(); + let log = db(); let handler = CountingHandler::new(deleted); let registry = OneHandler { capability: Capability::AgentsDelete, @@ -503,9 +526,10 @@ fn delete_requests_route_to_the_delete_capability() { &verified(serde_json::to_vec(&request).unwrap()), &AllowAll, ®istry, - &mut conn, - ); + &log, + ) + .await; assert!(response.result.is_succeeded()); - assert_eq!(*handler.calls.borrow(), 1); + assert_eq!(handler.calls(), 1); } diff --git a/desktop/src-tauri/src/broker/store.rs b/desktop/src-tauri/src/broker/store.rs index aa417ebb8b9..3c32fc34786 100644 --- a/desktop/src-tauri/src/broker/store.rs +++ b/desktop/src-tauri/src/broker/store.rs @@ -23,6 +23,8 @@ 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 { @@ -364,6 +366,118 @@ pub fn open_in_memory() -> Connection { 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. pub fn get(conn: &Connection, key: &ExecutionKey) -> Result, String> { conn.query_row( From 3a585fd8eddb9e3307cd9433b877f10af674ff0d Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 17:46:09 -0700 Subject: [PATCH 4/5] broker: back the agents.* capabilities with the real Desktop commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements `AgentService` and `AgentRoster` against the same commands the UI calls — `create_managed_agent`, `update_managed_agent`, `delete_managed_agent` — rather than reimplementing the mutations. Two agent creation routines would drift, and the broker's would be the one nobody looks at. The credential boundary is narrowed here: `create_managed_agent` returns the minted `private_key_nsec`, and this is the one stack frame that sees it. The outcome type has no field that could carry it, so the secret cannot reach the pipeline, the execution log, or the wire. Request translation is pure and tested directly, because the policy it encodes is worth pinning: - A brokered create neither spawns a process nor sets `start_on_app_launch`. A remote requester asking for an agent to exist has not asked for a local process to run on a schedule nobody at the keyboard chose. - It links no definition or team — that would pin someone else's config snapshot onto the new agent, a decision the request never made. - An update sends `None` for every field the request did not name. The broker has no "clear to default" verb, so it never emits the explicit null that would wipe stored config. - A requested runtime is resolved through the catalog before it can become a stored harness pin, and an unknown id fails here instead of surfacing later as an unrelated spawn error. - A delete leaves `force_remote_delete` unset, so the deployed-remote guard still holds: orphaning provisioned infrastructure stays a human decision. Channel attachment is its own failable step after the agent exists, so it maps to `Unknown` rather than `Failed` — telling a requester nothing happened while an agent sits in the roster would be the worse lie. Name targets resolve case-insensitively but must be unambiguous. Two agents can legitimately share a name, and guessing would mutate or delete an agent the requester never identified. The roster refuses to answer for an owner this host does not hold keys for, rather than authorizing against the wrong agent set. Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- .../src-tauri/src/broker/desktop_agents.rs | 390 ++++++++++++++++++ .../src/broker/desktop_agents/tests.rs | 176 ++++++++ desktop/src-tauri/src/broker/mod.rs | 1 + 3 files changed, 567 insertions(+) create mode 100644 desktop/src-tauri/src/broker/desktop_agents.rs create mode 100644 desktop/src-tauri/src/broker/desktop_agents/tests.rs 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/mod.rs b/desktop/src-tauri/src/broker/mod.rs index af99b3659b1..b5748d9d810 100644 --- a/desktop/src-tauri/src/broker/mod.rs +++ b/desktop/src-tauri/src/broker/mod.rs @@ -33,6 +33,7 @@ use sha2::{Digest, Sha256}; pub mod agents_policy; +pub mod desktop_agents; pub mod handlers; pub mod pipeline; pub mod store; From 1938c1dbbd0e7701f77e4ff995654949720c5a2c Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 19:55:51 -0700 Subject: [PATCH 5/5] broker: verify inbound frames and deliver signed results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the loop from a signed relay event to a signed result frame. The pipeline was already testable in isolation; this attaches the transport and the host's real parts, so nothing above the ingress layer learns what Tauri or the relay look like. `ingress::verify_frame` derives every trusted field from the frame rather than from anything a caller could assert. Owner, requester, and relay scope are transport-derived, which is why the wire envelope has no field for them: a payload that could name its own owner would be a payload that could pick its own authority. Verification refuses duplicate routing tags — a frame with two `agent` tags has two readings, and the one this host picks would not have to match the one the relay routed on. Freshness is re-checked here rather than trusted from the relay, because bounding replay of an old signed frame is not a hop this host controls. A frame that fails verification gets no answer at all. Emitting a signed refusal at an unverified sender would make any host a signing oracle aimed at a target of the sender's choosing. Frames that are simply not broker traffic — the overwhelming majority, since ordinary telemetry arrives on this same subscription — are ignored cheaply. `host.rs` keeps the command a transport hand-off rather than an authority hand-off: it takes a raw signed event and nothing else, so the renderer cannot say who is asking or which owner to act as. The return value is deliberately thin, since the requester's real answer travels in a signed frame; reporting an outcome through IPC would make the reply path look like a return value it is not. Results publish over the relay WebSocket because kind-24200 is rejected on the HTTP bridge, and as the owner's own identity with no NIP-OA auth tag — an auth tag is how a managed agent proves owner backing, and the owner is not one. On the TypeScript side the renderer forwards the *signed event*, never the decrypted plaintext, so the host verifies and decrypts independently. The routing check is shallow on purpose: it decides only whether a payload claims to be a broker request, not whether it is valid, so an ill-formed request still reaches the host and earns a structured refusal instead of silence. A broker request also returns before the session journal, because a payload with no seq, timestamp, or kind must not be filed as a transcript entry. Forwarding is fault-isolated: a broker host fault is not an observer transport fault, and must not be reported as one or tear down telemetry for every agent over a single failed mutation. `VerifiedRequest` gets a hand-written `Debug` that redacts the payload to a byte count. Decrypted request content includes system prompts, and a derived `Debug` would put them into any error or test failure that formats a request. Verified at this tree: 2916 desktop Tauri Rust tests (74 broker, 18 ingress), 5311 desktop TS tests, workspace clippy and both fmt gates clean. The two `buzz-pair-relay` integration failures are a pre-existing timing flake, reproduced on clean origin/main at 074561233 and independent of these crates. Signed-off-by: ss-dev-00 Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- desktop/src-tauri/src/broker/host.rs | 178 +++++ desktop/src-tauri/src/broker/ingress.rs | 323 +++++++++ desktop/src-tauri/src/broker/ingress/tests.rs | 614 ++++++++++++++++++ desktop/src-tauri/src/broker/mod.rs | 2 + desktop/src-tauri/src/broker/pipeline.rs | 16 + desktop/src-tauri/src/broker/store.rs | 5 + desktop/src-tauri/src/lib.rs | 1 + .../agents/brokerFrameForwarding.test.mjs | 126 ++++ .../features/agents/brokerFrameForwarding.ts | 76 +++ .../src/features/agents/observerRelayStore.ts | 22 +- 10 files changed, 1361 insertions(+), 2 deletions(-) create mode 100644 desktop/src-tauri/src/broker/host.rs create mode 100644 desktop/src-tauri/src/broker/ingress.rs create mode 100644 desktop/src-tauri/src/broker/ingress/tests.rs create mode 100644 desktop/src/features/agents/brokerFrameForwarding.test.mjs create mode 100644 desktop/src/features/agents/brokerFrameForwarding.ts 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 index b5748d9d810..4727b1ad4ab 100644 --- a/desktop/src-tauri/src/broker/mod.rs +++ b/desktop/src-tauri/src/broker/mod.rs @@ -35,6 +35,8 @@ 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; diff --git a/desktop/src-tauri/src/broker/pipeline.rs b/desktop/src-tauri/src/broker/pipeline.rs index 52716fc6af9..583bed49c18 100644 --- a/desktop/src-tauri/src/broker/pipeline.rs +++ b/desktop/src-tauri/src/broker/pipeline.rs @@ -63,6 +63,22 @@ pub struct VerifiedRequest { 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 diff --git a/desktop/src-tauri/src/broker/store.rs b/desktop/src-tauri/src/broker/store.rs index 3c32fc34786..845c1f52008 100644 --- a/desktop/src-tauri/src/broker/store.rs +++ b/desktop/src-tauri/src/broker/store.rs @@ -479,6 +479,11 @@ impl ExecutionLog for SqliteExecutionLog { } /// 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 diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 807f2dbc73c..35bca571330 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -611,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;