From b622003f74aa5bf9b659786452813299a25e4897 Mon Sep 17 00:00:00 2001 From: Bradley Axen Date: Wed, 26 Aug 2026 19:00:45 -0700 Subject: [PATCH] broker: define the agent-to-broker action contract (#6742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the interface half of #6467. This is the contract for how a keyless agent asks a host to act on its behalf — and only that. No host, no transport, no signing, no Desktop code; the only implementation is a test double. An agent that holds no secret key still has to read its channels, answer a mention, react, keep its profile, address its memory, and create the agents it needs. This PR defines the vocabulary for those requests and the rules both sides must follow. **#6467's open question, answered: per-operation, not "sign this".** A raw signing primitive tells the host *who* is asking but not *what for*, so the only policies it can express are all-or-nothing. A closed set of named operations lets a host serve channel reads while refusing agent creation, and gives any later policy layer something to attach to. The cost is that adding an operation means changing the contract — which is the point. Signing, publishing, and the auth/encryption NIPs are deliberately *not* operations; they are mechanisms inside whoever holds the key, and a test pins that none of those names resolve. **Nine operations in v1:** read a channel (whole channel, one thread, or mentions-only — these differ by filter, not permission, so they share one operation), post, reply, react, set profile, derive a storage address, and create / update / delete agents. Presence and typing are deferred as housekeeping a host may decline regardless; streaming is deferred, so waking on a mention is a polled mentions-only read. Each operation has typed, validated arguments and a typed outcome. **What the contract enforces structurally rather than by instruction:** - *Retry means identical bytes.* A request is validated and serialized exactly once and the transport only ever sees that frozen form. There is no way to re-render it, so a retry can't drift and be mistaken for a conflicting request. - *Read cursors are opaque.* Host-issued strings, validated for shape only, never parsed or compared. The host owns ordering and paging stability instead of being locked into timestamps. - *Reads return signed Nostr events.* A keyless agent verifies authorship and content locally and trusts the host only for completeness and authorization. Reply ancestry and mentions are derived from the signed tags, so nothing can disagree with the signature. - *Validation is the only door to a response.* The transport primitive hands back an unjudged envelope; the only response a caller can actually obtain is one already checked against the request it answers — request id correlates, the outcome's operation matches, identifiers and cursors are well-formed. A mismatch counts as "no answer", not a failure verdict. The raw primitive can't be invoked from outside the crate (pinned by compile-failure checks), so skipping that step isn't an option. - *Authority is never asserted by a body.* The envelope has no requester, owner, scope, or credential field; all of that comes from the session the host authenticated. Ownership of a created agent is implicitly the requester, never a field — which is what lets agents own agents while the chain still ends at a human. - *No secret crosses the wire.* Agent creation returns a pubkey and handle, never the minted key. Every payload type rejects unknown fields, and a test pins the exact JSON key set of all eighteen argument and outcome types, so no field can be added without a reviewer touching that table. Two honest limits: a string can physically hold secret text, so keeping secrets out of message content is host policy; and nothing stops a host from *holding* keys — it just may not hand them over. - *Null is never legal; omission is the only spelling of absence.* Every optional member rejects an explicit null. Otherwise null and omitted would be indistinguishable, and a contradiction check on the response envelope could be skipped by sending one (a real hole in an earlier revision). Coverage is derived by walking every path in the real fixtures, so new optional members are covered automatically. Serializers that emit null by default (Go without `omitempty`, Python's default dump) need configuring — called out in the module docs. **Wire mapping.** One POST endpoint, JSON body is the frozen request, opaque bearer credential in the auth header. Every verdict the host reached — including failure and including a rejected credential — comes back as a well-formed envelope with 200, because a second copy of the verdict in the status line could only disagree with it. Clients still try to parse an envelope regardless of status (intermediaries may remap) and fall back to the status only when none is present. A rejected credential is a host verdict carrying the promise "the action did not run"; transport errors are reserved for "nothing can be concluded about side effects." Binding the credential to a specific agent and conversation is the host's job and deliberately unenforceable here. **Relationship to #6543.** Envelope, request-id rules, protocol version, the three-way succeeded / failed / indeterminate result, error codes, and the agent-management argument shapes carry over. Changed deliberately: pubkeys are a validated type rather than bare strings, "capability" became "action" throughout, and a new "unsupported" error lets a host decline a best-effort operation without it reading as a fault. Both PRs land in the same module path — whichever merges second needs a hand reconcile, which I own as the author of #6543. **Validation at `7c853e0fb`.** 299 tests in the sdk crate (full package run), workspace tests green apart from the pre-existing relay timeout flake, which I reproduced on unmodified `main`. Clippy with warnings denied, fmt, warning-free docs, and the pre-push gate all pass. Review findings were closed with mutation tests — each fix verified by reverting it and watching a named test fail. One of those exposed a gap in my own coverage (response-side fields weren't pinned); that's fixed and floored separately from the request side so neither can mask the other. **Size.** About 5,400 lines across 8 files, roughly half tests. Every source file is under the repo's 1,000-line cap except the test file; #6759 splits it mechanically, is stacked on this branch, retargets to `main` after merge, and lands before PR 2 of the stack. --------- Signed-off-by: ss-dev-00 Signed-off-by: Bradley Axen Signed-off-by: ss-core-02 Co-authored-by: ss-dev-00 Co-authored-by: ss-core-02 --- Cargo.toml | 2 +- crates/buzz-sdk/src/broker/actions/args.rs | 563 ++++ crates/buzz-sdk/src/broker/actions/mod.rs | 414 +++ .../buzz-sdk/src/broker/actions/outcomes.rs | 298 ++ crates/buzz-sdk/src/broker/client.rs | 223 ++ crates/buzz-sdk/src/broker/correlate.rs | 92 + crates/buzz-sdk/src/broker/mod.rs | 754 +++++ crates/buzz-sdk/src/broker/tests.rs | 2811 +++++++++++++++++ crates/buzz-sdk/src/broker/wire.rs | 107 + crates/buzz-sdk/src/lib.rs | 1 + 10 files changed, 5264 insertions(+), 1 deletion(-) create mode 100644 crates/buzz-sdk/src/broker/actions/args.rs create mode 100644 crates/buzz-sdk/src/broker/actions/mod.rs create mode 100644 crates/buzz-sdk/src/broker/actions/outcomes.rs create mode 100644 crates/buzz-sdk/src/broker/client.rs create mode 100644 crates/buzz-sdk/src/broker/correlate.rs create mode 100644 crates/buzz-sdk/src/broker/mod.rs create mode 100644 crates/buzz-sdk/src/broker/tests.rs create mode 100644 crates/buzz-sdk/src/broker/wire.rs diff --git a/Cargo.toml b/Cargo.toml index 78816ff482..8f5f949d74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,7 +77,7 @@ postcard = { version = "1", default-features = false, features = ["use-std"] # Inter-relay mesh transport (buzz-relay-mesh) iroh = { version = "1.0.0-rc.0", default-features = false, features = ["tls-ring"] } -serde_json = "1" +serde_json = { version = "1", features = ["raw_value"] } serde_yaml = "0.9" evalexpr = "11" cron = "0.16" diff --git a/crates/buzz-sdk/src/broker/actions/args.rs b/crates/buzz-sdk/src/broker/actions/args.rs new file mode 100644 index 0000000000..1be4546ee1 --- /dev/null +++ b/crates/buzz-sdk/src/broker/actions/args.rs @@ -0,0 +1,563 @@ +//! Argument types — one per [`Action`], plus the tagged union that pairs a +//! wire action name with its arguments. Each type carries a `validated()` +//! returning a normalized copy; the shared validators and the contract-wide +//! strictness rules live in the [parent module](super). + +use serde::{Deserialize, Serialize}; + +use super::{ + absent_or_valued, absent_or_valued_hex64, channel, channel_id, content, cursor, event_id, + hex64_field, is_false, limit, mentions, optional, required, respond_to, validate_slug, Action, + PubkeyHex, DEFAULT_PAGE_LIMIT, MAX_ABOUT_CHARS, MAX_EMOJI_CHARS, MAX_NAME_CHARS, + MAX_PROMPT_CHARS, MAX_SCALAR_CHARS, +}; +use crate::SdkError; + +/// Arguments for `channel.read` — the one read action. +/// +/// One action covers channel, thread, and mention-feed scope, because they +/// differ only by filter and a name per scope would split one permission — +/// *may this agent see this channel* — across three policy decisions. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct ChannelReadArgs { + /// Channel to read. + #[serde(deserialize_with = "channel_id")] + pub channel_id: String, + /// Narrow to one thread by its root event. + #[serde( + default, + deserialize_with = "absent_or_valued_hex64", + skip_serializing_if = "Option::is_none" + )] + pub root_event_id: Option, + /// Narrow to messages mentioning the requester — the wake path. The + /// requester is never named; no body names its own subject. + #[serde(default, skip_serializing_if = "is_false")] + pub mentions_only: bool, + /// Opaque position to resume from, as returned in [`super::outcomes::MessagePage::next_cursor`]. + /// + /// Absent on a first read, which starts at the host's default window. + /// Callers must round-trip a cursor verbatim, never parse or synthesize + /// one: the host defines ordering and cursor stability, including whether + /// a cursor stays valid across restarts. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub cursor: Option, + /// Maximum events to return, capped at [`super::MAX_PAGE_LIMIT`]. + /// + /// Absent means [`super::DEFAULT_PAGE_LIMIT`], not "unbounded": see + /// [`Self::effective_limit`]. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub limit: Option, +} + +impl ChannelReadArgs { + /// The page size a response to these arguments is held to: explicit + /// `limit` when set, otherwise [`super::DEFAULT_PAGE_LIMIT`] — omitting a + /// limit asks for a sensible page, not an unbounded one. + #[must_use] + pub fn effective_limit(&self) -> u32 { + self.limit.unwrap_or(DEFAULT_PAGE_LIMIT) + } + + /// Read a whole channel from the host's default window. + #[must_use] + pub fn channel(channel_id: impl Into) -> Self { + Self { + channel_id: channel_id.into(), + ..Self::default() + } + } + + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed channel UUID, a + /// malformed root event id, an over-long or non-printable cursor, or an + /// out-of-range limit. + pub fn validated(&self) -> Result { + Ok(Self { + channel_id: channel(&self.channel_id)?, + root_event_id: self + .root_event_id + .as_deref() + .map(|id| event_id(id, "rootEventId")) + .transpose()?, + mentions_only: self.mentions_only, + cursor: self.cursor.as_deref().map(cursor).transpose()?, + limit: limit(self.limit)?, + }) + } +} + +// ── Write arguments ───────────────────────────────────────────────────────── + +/// Arguments for `message.post`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct MessagePostArgs { + /// Channel to post in. + #[serde(deserialize_with = "channel_id")] + pub channel_id: String, + /// Message body. + pub content: String, + /// Pubkeys to notify. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub mentions: Vec, +} + +impl MessagePostArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed channel UUID or empty + /// content, [`SdkError::ContentTooLarge`] for oversized content, and + /// [`SdkError::TooManyMentions`] past [`super::MAX_MENTIONS`]. + pub fn validated(&self) -> Result { + Ok(Self { + channel_id: channel(&self.channel_id)?, + content: content(&self.content)?, + mentions: mentions(&self.mentions)?, + }) + } +} + +/// Arguments for `message.reply`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct MessageReplyArgs { + /// Channel containing the parent. + #[serde(deserialize_with = "channel_id")] + pub channel_id: String, + /// Event being replied to. + #[serde(deserialize_with = "hex64_field")] + pub reply_to_event_id: String, + /// Reply body. + pub content: String, + /// Pubkeys to notify. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub mentions: Vec, +} + +impl MessageReplyArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed channel UUID, a + /// malformed event id, or empty content; [`SdkError::ContentTooLarge`] for + /// oversized content; [`SdkError::TooManyMentions`] past [`super::MAX_MENTIONS`]. + pub fn validated(&self) -> Result { + Ok(Self { + channel_id: channel(&self.channel_id)?, + reply_to_event_id: event_id(&self.reply_to_event_id, "replyToEventId")?, + content: content(&self.content)?, + mentions: mentions(&self.mentions)?, + }) + } +} + +/// Arguments for `reaction.add`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct ReactionAddArgs { + /// Channel containing the target. + #[serde(deserialize_with = "channel_id")] + pub channel_id: String, + /// Event being reacted to. + #[serde(deserialize_with = "hex64_field")] + pub target_event_id: String, + /// Reaction payload — an emoji or a `:shortcode:`. + pub reaction: String, +} + +impl ReactionAddArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed channel UUID, a + /// malformed event id, or an empty reaction, and [`SdkError::EmojiTooLong`] + /// past [`MAX_EMOJI_CHARS`]. + pub fn validated(&self) -> Result { + let reaction = self.reaction.trim(); + if reaction.is_empty() { + return Err(SdkError::InvalidInput("reaction must not be empty".into())); + } + if reaction.chars().count() > MAX_EMOJI_CHARS { + return Err(SdkError::EmojiTooLong); + } + Ok(Self { + channel_id: channel(&self.channel_id)?, + target_event_id: event_id(&self.target_event_id, "targetEventId")?, + reaction: reaction.to_owned(), + }) + } +} + +/// Arguments for `profile.set`. +/// +/// Only the requester's own profile is addressable, so there is no subject +/// field. Absent fields are left as they are; the host does not clear them. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct ProfileSetArgs { + /// Replacement display name. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub display_name: Option, + /// Replacement bio. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub about: Option, + /// Replacement avatar URL. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub picture: Option, +} + +impl ProfileSetArgs { + /// Validate and normalize, requiring at least one field to change. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for an over-long field or a request + /// that changes nothing. + pub fn validated(&self) -> Result { + let normalized = Self { + display_name: optional(self.display_name.as_ref(), "display name", MAX_NAME_CHARS)?, + about: optional(self.about.as_ref(), "about", MAX_ABOUT_CHARS)?, + picture: optional(self.picture.as_ref(), "picture", MAX_SCALAR_CHARS)?, + }; + if normalized.display_name.is_none() + && normalized.about.is_none() + && normalized.picture.is_none() + { + return Err(SdkError::InvalidInput( + "include at least one profile field to set".into(), + )); + } + Ok(normalized) + } +} + +/// Arguments for `storage.address`. +/// +/// Deriving a record's address needs the secret this contract exists to avoid +/// holding, which is why it routes through the interface. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct StorageAddressArgs { + /// Memory slug — `core` or `mem/…`, per NIP-AE. + pub slug: String, +} + +impl StorageAddressArgs { + /// Validate and normalize. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] when the slug fails the NIP-AE + /// grammar. + pub fn validated(&self) -> Result { + let slug = required(&self.slug, "slug", 255)?; + validate_slug(&slug).map_err(|e| SdkError::InvalidInput(e.to_string()))?; + Ok(Self { slug }) + } +} + +// ── Agent arguments ───────────────────────────────────────────────────────── + +/// Which agent an update or delete targets — exactly one selector, so a host +/// never has to guess which of two names wins. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum AgentTarget { + /// Target by agent pubkey. + Pubkey(PubkeyHex), + /// Target by the agent's current name. + Name(String), +} + +impl AgentTarget { + /// Validate and normalize the selector. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for an empty or over-long name. + pub fn validated(&self) -> Result { + match self { + Self::Pubkey(pubkey) => Ok(Self::Pubkey(PubkeyHex::parse(pubkey.as_str())?)), + Self::Name(name) => Ok(Self::Name(required(name, "agent name", MAX_NAME_CHARS)?)), + } + } +} + +/// Arguments for `agents.create`. +/// +/// There is no owner field: the owner is whoever the host authenticated. See +/// the [contract docs](crate::broker) on ownership recursion. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsCreateArgs { + /// Channel the new agent is attached to. + #[serde(deserialize_with = "channel_id")] + 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, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub runtime: Option, + /// Inference provider. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub provider: Option, + /// Model identifier, interpreted relative to the runtime. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub model: Option, + /// Inbound author gate mode; absent = the host's owner-only default. + #[serde( + default, + deserialize_with = "absent_or_valued", + 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, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub display_name: Option, + /// Replacement instructions. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub system_prompt: Option, + /// Harness id to pin. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub runtime: Option, + /// Inference provider. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub provider: Option, + /// Model identifier. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub model: Option, + /// Inbound author gate mode. + #[serde( + default, + deserialize_with = "absent_or_valued", + 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()?, + }) + } +} + +// ── Action union ──────────────────────────────────────────────────────────── + +/// An action name paired with its strictly typed arguments. +/// +/// Flattened into [`crate::broker::BrokerRequest`], so the wire form is +/// `{ "action": "message.post", "args": { … } }` and an args shape can never be +/// paired with the wrong action name. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "action", content = "args", deny_unknown_fields)] +pub enum ActionArgs { + /// Read a channel, thread, or mention feed. + #[serde(rename = "channel.read")] + ChannelRead(ChannelReadArgs), + /// Post a message. + #[serde(rename = "message.post")] + MessagePost(MessagePostArgs), + /// Reply to a message. + #[serde(rename = "message.reply")] + MessageReply(MessageReplyArgs), + /// React to a message. + #[serde(rename = "reaction.add")] + ReactionAdd(ReactionAddArgs), + /// Set the requester's profile. + #[serde(rename = "profile.set")] + ProfileSet(ProfileSetArgs), + /// Derive an encrypted-memory address. + #[serde(rename = "storage.address")] + StorageAddress(StorageAddressArgs), + /// 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 ActionArgs { + /// The action these args belong to. + #[must_use] + pub fn action(&self) -> Action { + match self { + Self::ChannelRead(_) => Action::ChannelRead, + Self::MessagePost(_) => Action::MessagePost, + Self::MessageReply(_) => Action::MessageReply, + Self::ReactionAdd(_) => Action::ReactionAdd, + Self::ProfileSet(_) => Action::ProfileSet, + Self::StorageAddress(_) => Action::StorageAddress, + Self::AgentsCreate(_) => Action::AgentsCreate, + Self::AgentsUpdate(_) => Action::AgentsUpdate, + Self::AgentsDelete(_) => Action::AgentsDelete, + } + } + + /// Return a normalized copy with every field validated. + /// + /// There is deliberately no non-consuming `validate(&self)` beside this; + /// see [`crate::broker::BrokerRequest::validated`] for the trap it was. + /// + /// # Errors + /// + /// Propagates the per-action validation error. + pub fn validated(&self) -> Result { + Ok(match self { + Self::ChannelRead(args) => Self::ChannelRead(args.validated()?), + Self::MessagePost(args) => Self::MessagePost(args.validated()?), + Self::MessageReply(args) => Self::MessageReply(args.validated()?), + Self::ReactionAdd(args) => Self::ReactionAdd(args.validated()?), + Self::ProfileSet(args) => Self::ProfileSet(args.validated()?), + Self::StorageAddress(args) => Self::StorageAddress(args.validated()?), + Self::AgentsCreate(args) => Self::AgentsCreate(args.validated()?), + Self::AgentsUpdate(args) => Self::AgentsUpdate(args.validated()?), + Self::AgentsDelete(args) => Self::AgentsDelete(args.validated()?), + }) + } +} diff --git a/crates/buzz-sdk/src/broker/actions/mod.rs b/crates/buzz-sdk/src/broker/actions/mod.rs new file mode 100644 index 0000000000..c6f15cf42e --- /dev/null +++ b/crates/buzz-sdk/src/broker/actions/mod.rs @@ -0,0 +1,414 @@ +//! Broker actions — the closed set of operations an agent may ask a host to +//! perform. [`Action`] and the shared validators live here; the payload types +//! are split into [`args`] and [`outcomes`] so each side of a call reviews on +//! its own. + +use serde::{Deserialize, Serialize}; + +use crate::SdkError; +use buzz_core::engram::validate_slug; + +pub mod args; +pub mod outcomes; + +pub use args::{ + ActionArgs, AgentTarget, AgentsCreateArgs, AgentsDeleteArgs, AgentsUpdateArgs, ChannelReadArgs, + MessagePostArgs, MessageReplyArgs, ProfileSetArgs, ReactionAddArgs, StorageAddressArgs, +}; +pub use outcomes::{ + ActionOutcome, AgentsCreateOutcome, AgentsDeleteOutcome, AgentsUpdateOutcome, BrokerMessage, + EventPublished, MessagePage, StorageAddress, +}; + +/// 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; + +/// Maximum characters in a profile `about` blurb. +pub const MAX_ABOUT_CHARS: usize = 2_000; + +/// Maximum bytes of message content, matching the SDK's channel-message cap. +pub const MAX_CONTENT_BYTES: usize = 64 * 1024; + +/// Maximum characters in a reaction payload (emoji or `:shortcode:`). +pub const MAX_EMOJI_CHARS: usize = 66; + +/// Maximum mentions attachable to one message. +pub const MAX_MENTIONS: usize = 50; + +/// Maximum events a single read may return. +pub const MAX_PAGE_LIMIT: u32 = 500; + +/// Events a read returns when the request sets no explicit `limit`. +/// +/// A caller that omits `limit` is not agreeing to an unbounded page, so this is +/// the number a response is held to in that case — see +/// [`crate::broker::BrokerResponse::validate_for`]. It is deliberately well +/// under [`MAX_PAGE_LIMIT`]: the cap is what a host may ever send, this is what +/// it may send unasked. +pub const DEFAULT_PAGE_LIMIT: u32 = 100; + +/// Maximum accepted length of a read cursor, in bytes. +pub const MAX_CURSOR_LEN: usize = 256; + +/// Inbound author gate modes a requester may ask for. +/// +/// `allowlist` is deliberately absent: it needs a pubkey list this 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 public key in lowercase hex — the only identity this contract has. No +/// secret-key counterpart exists in this module (#6467's identity/signing +/// separation, made structural). +/// +/// A value of this type is a **real x-only secp256k1 point**, not just 64 hex +/// characters — most 32-byte values lie on no curve. Accepting shape alone +/// would defer the first real rejection to whichever consumer eventually +/// converts the string to a key, after the request was already accepted. The +/// curve check is the `nostr` crate's, so the contract and the events it +/// carries agree on what a key is by construction. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] +pub struct PubkeyHex(String); + +impl PubkeyHex { + /// Parse a 64-character hex x-only public key, normalizing to lowercase. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] unless `value` is exactly 64 hex + /// characters **and** those bytes are a point on secp256k1. + pub fn parse(value: impl AsRef) -> Result { + let value = value.as_ref().trim(); + if value.len() != 64 || !value.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(SdkError::InvalidInput( + "pubkey must be 64 hex characters".into(), + )); + } + let value = value.to_ascii_lowercase(); + // `from_hex` only decodes hex; `xonly` is what actually rejects a + // value that is not on the curve. + nostr::PublicKey::from_hex(&value) + .and_then(|key| key.xonly().map(|_| ())) + .map_err(|_| { + SdkError::InvalidInput("pubkey is not a valid secp256k1 x-only public key".into()) + })?; + Ok(Self(value)) + } + + /// The hex representation. + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl TryFrom for PubkeyHex { + type Error = SdkError; + + fn try_from(value: String) -> Result { + Self::parse(value) + } +} + +impl From for String { + fn from(value: PubkeyHex) -> Self { + value.0 + } +} + +impl std::fmt::Display for PubkeyHex { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +/// An action name the broker can dispatch. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Action { + /// Read messages from a channel, thread, or mention feed after a cursor. + ChannelRead, + /// Post a top-level channel message. + MessagePost, + /// Reply to an existing message. + MessageReply, + /// React to an existing message. + ReactionAdd, + /// Publish the requester's own profile metadata. + ProfileSet, + /// Derive the address of one encrypted-memory record. + StorageAddress, + /// Mint a managed agent owned by the requester. + AgentsCreate, + /// Patch a managed agent the requester owns. + AgentsUpdate, + /// Remove a managed agent the requester owns. + AgentsDelete, +} + +impl Action { + /// Every action in this protocol version, in wire-name order. + pub const ALL: [Self; 9] = [ + Self::AgentsCreate, + Self::AgentsDelete, + Self::AgentsUpdate, + Self::ChannelRead, + Self::MessagePost, + Self::MessageReply, + Self::ProfileSet, + Self::ReactionAdd, + Self::StorageAddress, + ]; + + /// Stable wire name. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::ChannelRead => "channel.read", + Self::MessagePost => "message.post", + Self::MessageReply => "message.reply", + Self::ReactionAdd => "reaction.add", + Self::ProfileSet => "profile.set", + Self::StorageAddress => "storage.address", + Self::AgentsCreate => "agents.create", + Self::AgentsUpdate => "agents.update", + Self::AgentsDelete => "agents.delete", + } + } + + /// The action contract version this build implements. + #[must_use] + pub fn current_version(self) -> u16 { + 1 + } + + /// Whether a host may refuse this action without harming the agent. + /// + /// #6467 requires non-essential signed housekeeping to be skippable, so an + /// agent can still run where it is unavailable. See + /// [`super::BrokerErrorCode::Unsupported`] for how a caller reacts. + #[must_use] + pub fn is_best_effort(self) -> bool { + matches!(self, Self::ReactionAdd) + } + + /// Resolve a wire name. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for an unknown action name. + pub fn parse(name: &str) -> Result { + Self::ALL + .into_iter() + .find(|action| action.as_str() == name) + .ok_or_else(|| SdkError::InvalidInput(format!("unknown broker action \"{name}\""))) + } +} + +// ── Shared validators ─────────────────────────────────────────────────────── + +fn is_false(value: &bool) -> bool { + !*value +} + +/// Deserialize an optional member that may be **absent but never `null`**. +/// +/// This is the contract's one spelling-of-absence rule, and this is its +/// canonical rationale. `#[serde(default)] Option` maps an explicit `null` +/// to `None`, *indistinguishable from absent* to downstream code — so a reader +/// that decides something from absence (the status match in +/// [`crate::broker::BrokerResponse`], or +/// [`args::ChannelReadArgs::effective_limit`]) would silently treat a member +/// the sender did supply as one it did not. In the response envelope that was +/// a real hole: `{"status":"failed","outcome":null}` parsed as a plain failure +/// and skipped the per-status contradiction check. Rejecting `null` outright +/// leaves exactly one way to say "absent" and no layer guessing what a +/// present-but-empty member meant. +/// +/// Used with `#[serde(default, deserialize_with = "…")]`: serde calls this only +/// when the key is present, so reaching the `None` arm below means the member +/// was present and `null`. `deny_unknown_fields` stays in force alongside it. +/// +/// A required member of a non-`Option` type already rejects `null` as a type +/// error; the guard is only load-bearing where `Option` plus `default` would +/// otherwise conflate `null` with absent. +pub(super) fn absent_or_valued<'de, T, D>(deserializer: D) -> Result, D::Error> +where + T: Deserialize<'de>, + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + + match Option::::deserialize(deserializer)? { + Some(value) => Ok(Some(value)), + None => Err(D::Error::custom( + "must not be null; omit the member to mean absent", + )), + } +} + +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() +} + +/// Validate a channel id and return its **canonical** spelling — lowercase +/// hyphenated. `Uuid::parse_str` accepts several spellings of one channel, and +/// freezing the caller's spelling would make the host's canonical echo of the +/// same identity look like a mismatch in +/// [`crate::broker::BrokerResponse::validate_for`]. Same treatment +/// [`PubkeyHex::parse`] gives the other identity in this contract. +fn channel(value: &str) -> Result { + let value = required(value, "channel", 128)?; + uuid::Uuid::parse_str(&value) + .map(|id| id.as_hyphenated().to_string()) + .map_err(|_| SdkError::InvalidInput(format!("invalid channel UUID: {value}"))) +} + +/// Deserialize a `channelId`, canonicalizing it and rejecting a non-UUID. +/// +/// The wire is the one door a validator cannot cover: fields holding a channel +/// id are public `String`s, so a payload parsed from JSON reaches a caller +/// without passing through any `validated()`. Delegating to [`channel`] keeps +/// the wire form and the constructed form canonicalized by the same code. +pub(super) fn channel_id<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + + let raw = String::deserialize(deserializer)?; + channel(&raw).map_err(D::Error::custom) +} + +fn event_id(value: &str, label: &str) -> Result { + let value = required(value, label, 64)?; + if value.len() != 64 || !value.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(SdkError::InvalidInput(format!( + "{label} must be 64 hex characters" + ))); + } + Ok(value.to_ascii_lowercase()) +} + +/// Deserialize a 64-hex identifier (`eventId`, `dTag`), lowercasing it — +/// the [`channel_id`] rule applied to the contract's other multi-spelling +/// identities. The label is generic because serde already reports which +/// member failed. +pub(super) fn hex64_field<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + + let raw = String::deserialize(deserializer)?; + event_id(&raw, "identifier").map_err(D::Error::custom) +} + +/// [`hex64_field`] for an optional member: `null` is still rejected (see +/// [`absent_or_valued`]). One function because `deserialize_with` takes one, +/// and both rules apply to the same member. +pub(super) fn absent_or_valued_hex64<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + use serde::de::Error as _; + + match Option::::deserialize(deserializer)? { + Some(raw) => Ok(Some( + event_id(&raw, "identifier").map_err(D::Error::custom)?, + )), + None => Err(D::Error::custom( + "must not be null; omit the member to mean absent", + )), + } +} + +fn content(value: &str) -> Result { + if value.trim().is_empty() { + return Err(SdkError::InvalidInput("content must not be empty".into())); + } + if value.len() > MAX_CONTENT_BYTES { + return Err(SdkError::ContentTooLarge { + max: MAX_CONTENT_BYTES, + got: value.len(), + }); + } + Ok(value.to_owned()) +} + +fn mentions(values: &[PubkeyHex]) -> Result, SdkError> { + if values.len() > MAX_MENTIONS { + return Err(SdkError::TooManyMentions); + } + values + .iter() + .map(|pubkey| PubkeyHex::parse(pubkey.as_str())) + .collect() +} + +fn limit(value: Option) -> Result, SdkError> { + match value { + None => Ok(None), + Some(0) => Err(SdkError::InvalidInput("limit must be at least 1".into())), + Some(limit) if limit > MAX_PAGE_LIMIT => Err(SdkError::InvalidInput(format!( + "limit exceeds {MAX_PAGE_LIMIT} (got {limit})" + ))), + Some(limit) => Ok(Some(limit)), + } +} + +/// Validate an opaque read cursor: printable ASCII, bounded, never parsed. +/// +/// The bound exists so a host cannot be made to store an unbounded token; the +/// character set keeps it safe to log. Nothing here interprets the value. +fn cursor(value: &str) -> Result { + if value.is_empty() { + return Err(SdkError::InvalidInput( + "cursor must not be empty (omit it to start from the host's default window)".into(), + )); + } + if value.len() > MAX_CURSOR_LEN { + return Err(SdkError::InvalidInput(format!( + "cursor exceeds {MAX_CURSOR_LEN} bytes (got {})", + value.len() + ))); + } + if !value.bytes().all(|b| (0x21..=0x7e).contains(&b)) { + return Err(SdkError::InvalidInput( + "cursor must be printable ASCII without spaces".into(), + )); + } + Ok(value.to_owned()) +} + +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/actions/outcomes.rs b/crates/buzz-sdk/src/broker/actions/outcomes.rs new file mode 100644 index 0000000000..064f3ed3aa --- /dev/null +++ b/crates/buzz-sdk/src/broker/actions/outcomes.rs @@ -0,0 +1,298 @@ +//! Outcome types — the success payload of each [`Action`], and the tagged union +//! that pairs a wire action name with its outcome. Outcomes are shared where +//! actions agree on what success means: the four event-publishing actions all +//! return [`EventPublished`]. + +use serde::{Deserialize, Serialize}; + +use super::{ + absent_or_valued, channel, channel_id, cursor, event_id, hex64_field, required, Action, + PubkeyHex, MAX_NAME_CHARS, MAX_PAGE_LIMIT, +}; +use crate::SdkError; +use nostr::{Event, EventId, Kind, PublicKey, Tags, Timestamp}; + +/// The seven canonical members of a Nostr event object, and nothing else. +/// +/// `nostr`'s own `Event` deserializer accepts and *discards* unknown members, +/// which would put read results outside the contract's strict-wire rule. +/// Routing through a `deny_unknown_fields` intermediary restores the rule at +/// the one place the contract does not own the type. +/// +/// Field names are the wire names from NIP-01 (`created_at`, not `createdAt`) — +/// this is the event's own encoding, not ours to rename. +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct StrictEvent { + id: EventId, + pubkey: PublicKey, + created_at: Timestamp, + kind: Kind, + tags: Tags, + content: String, + sig: nostr::secp256k1::schnorr::Signature, +} + +/// One message returned by a read: the signed Nostr event, verbatim. +/// +/// The event is carried whole — signature and tags included — rather than +/// reduced to a projection, because Schnorr verification is local (see +/// [`Self::verify`]): a keyless agent gets independently verifiable authorship +/// and content, and only trusts the host for *completeness* and authorization. +/// Ancestry and mentions are derived accessors rather than sibling fields, so +/// nothing can disagree with the signed bytes. +/// +/// Deserialization is **strict**, via a private `deny_unknown_fields` +/// intermediary; serialization is the event's own, so the wire form is +/// unchanged. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(transparent)] +pub struct BrokerMessage(pub Event); + +impl<'de> Deserialize<'de> for BrokerMessage { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let strict = StrictEvent::deserialize(deserializer)?; + Ok(Self(Event::new( + strict.id, + strict.pubkey, + strict.created_at, + strict.kind, + strict.tags, + strict.content, + strict.sig, + ))) + } +} + +impl BrokerMessage { + /// The signed event. + #[must_use] + pub fn event(&self) -> &Event { + &self.0 + } + + /// Verify the event's id and Schnorr signature — entirely local; a host + /// that fabricated or altered a message fails here regardless of what it + /// claims. Deliberately *not* called by + /// [`crate::broker::BrokerResponse::validate_for`]: whether to pay for + /// verification, and what to do when it fails, is the caller's policy. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] when the id does not match the + /// content or the signature does not match the author. + pub fn verify(&self) -> Result<(), SdkError> { + self.0.verify().map_err(|e| { + SdkError::InvalidInput(format!("broker returned an unverifiable event: {e}")) + }) + } + + /// The author's pubkey, in this contract's identity type. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] if the event's author is not + /// expressible as 64 hex characters. + pub fn author(&self) -> Result { + PubkeyHex::parse(self.0.pubkey.to_hex()) + } + + /// NIP-10 `root`/`reply` ancestry, parsed from the signed tags. + #[must_use] + pub fn thread(&self) -> buzz_core::nip10::ThreadMarkers { + buzz_core::nip10::parse_thread_markers(&self.0.tags) + } + + /// Pubkeys this message mentions, from the signed `p` tags. + #[must_use] + pub fn mentions(&self) -> Vec { + self.0 + .tags + .public_keys() + .map(nostr::PublicKey::to_hex) + .collect() + } +} + +/// Outcome of any read action. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct MessagePage { + /// Messages in the host's declared order. + pub messages: Vec, + /// Opaque cursor to pass as [`super::args::ChannelReadArgs::cursor`] on the next call. + /// + /// Absent when the host has nothing further, which is how a caller learns + /// to stop rather than by comparing lengths against a limit it may not have + /// set. + #[serde( + default, + deserialize_with = "absent_or_valued", + skip_serializing_if = "Option::is_none" + )] + pub next_cursor: Option, +} + +/// Outcome of an action that published one event. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EventPublished { + /// The published event's id (hex). + #[serde(deserialize_with = "hex64_field")] + pub event_id: String, + /// The published event's kind. + pub kind: u32, + /// Creation time the host stamped, Unix seconds. + pub created_at: u64, +} + +/// Outcome of `storage.address`. +/// +/// Addressing material only. A `d` tag is a keyed hash of the slug, so it +/// identifies a record without revealing the slug or the key that derived it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct StorageAddress { + /// Author the record is addressed under. + pub author_pubkey: PubkeyHex, + /// Event kind holding the record. + pub kind: u32, + /// Derived `d` tag (64 hex characters). + #[serde(deserialize_with = "hex64_field")] + pub d_tag: String, +} + +/// Outcome of a successful `agents.create`. +/// +/// Carries the new agent's **public** identity only — there is no field for +/// the minted secret, and `deny_unknown_fields` plus the key-set test is what +/// enforces that rather than a comment. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentsCreateOutcome { + /// The new agent's pubkey. + pub agent_pubkey: PubkeyHex, + /// The new agent's name as stored. + pub display_name: String, + /// Channel the agent was attached to. + #[serde(deserialize_with = "channel_id")] + 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. + pub agent_pubkey: PubkeyHex, + /// 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. + pub agent_pubkey: PubkeyHex, + /// The removed agent's name. + pub display_name: String, +} + +/// An action-specific success payload. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "action", content = "outcome", deny_unknown_fields)] +pub enum ActionOutcome { + /// `channel.read` succeeded. + #[serde(rename = "channel.read")] + ChannelRead(MessagePage), + /// `message.post` succeeded. + #[serde(rename = "message.post")] + MessagePost(EventPublished), + /// `message.reply` succeeded. + #[serde(rename = "message.reply")] + MessageReply(EventPublished), + /// `reaction.add` succeeded. + #[serde(rename = "reaction.add")] + ReactionAdd(EventPublished), + /// `profile.set` succeeded. + #[serde(rename = "profile.set")] + ProfileSet(EventPublished), + /// `storage.address` succeeded. + #[serde(rename = "storage.address")] + StorageAddress(StorageAddress), + /// `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 ActionOutcome { + /// The action that produced this outcome. + #[must_use] + pub fn action(&self) -> Action { + match self { + Self::ChannelRead(_) => Action::ChannelRead, + Self::MessagePost(_) => Action::MessagePost, + Self::MessageReply(_) => Action::MessageReply, + Self::ReactionAdd(_) => Action::ReactionAdd, + Self::ProfileSet(_) => Action::ProfileSet, + Self::StorageAddress(_) => Action::StorageAddress, + Self::AgentsCreate(_) => Action::AgentsCreate, + Self::AgentsUpdate(_) => Action::AgentsUpdate, + Self::AgentsDelete(_) => Action::AgentsDelete, + } + } + + /// Validate the identifiers and cursors this outcome asserts. + /// + /// A well-typed outcome can still carry a malformed id or an unusable + /// cursor. Signature verification is deliberately *not* here — see + /// [`BrokerMessage::verify`]. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] for a malformed event id, `d` tag, + /// channel UUID, or cursor, an empty name, or an over-long page. + pub fn validate(&self) -> Result<(), SdkError> { + match self { + Self::ChannelRead(page) => { + if page.messages.len() > MAX_PAGE_LIMIT as usize { + return Err(SdkError::InvalidInput(format!( + "page holds {} messages, over the {MAX_PAGE_LIMIT} cap", + page.messages.len() + ))); + } + page.next_cursor.as_deref().map(cursor).transpose()?; + } + Self::MessagePost(published) + | Self::MessageReply(published) + | Self::ReactionAdd(published) + | Self::ProfileSet(published) => { + event_id(&published.event_id, "eventId")?; + } + Self::StorageAddress(address) => { + event_id(&address.d_tag, "dTag")?; + } + Self::AgentsCreate(outcome) => { + channel(&outcome.channel_id)?; + required(&outcome.display_name, "display name", MAX_NAME_CHARS)?; + } + Self::AgentsUpdate(AgentsUpdateOutcome { display_name, .. }) + | Self::AgentsDelete(AgentsDeleteOutcome { display_name, .. }) => { + required(display_name, "display name", MAX_NAME_CHARS)?; + } + } + Ok(()) + } +} diff --git a/crates/buzz-sdk/src/broker/client.rs b/crates/buzz-sdk/src/broker/client.rs new file mode 100644 index 0000000000..3de4dbb896 --- /dev/null +++ b/crates/buzz-sdk/src/broker/client.rs @@ -0,0 +1,223 @@ +//! Client trait and HTTP binding for the broker contract. +//! +//! # HTTP binding +//! +//! ```text +//! POST /v1/action +//! Authorization: Bearer +//! Content-Type: application/json +//! +//! +//! ``` +//! +//! The response body is a [`BrokerResponse`] as JSON. Every terminal +//! disposition the *host* reached — including a rejected credential — is a +//! well-formed envelope returned with `200`: the verdict lives in `status`, +//! and a second copy in the status line could only ever disagree with it. A +//! client must nonetheless **attempt to parse an envelope regardless of HTTP +//! status** (an intermediary may map dispositions onto statuses); if a valid +//! envelope is present, it is the answer. Only when no envelope can be parsed +//! does the status matter, and then only as operator detail — see +//! [`BrokerTransportError`]. +//! +//! # The credential +//! +//! The credential is **opaque to this contract**: a bearer token the agent +//! received at startup and can only replay — not a key, not a signature. A +//! rejected credential is a host verdict, not a transport failure: it arrives +//! as `Failed` with [`super::BrokerErrorCode::Unauthenticated`], which carries +//! the promise that the action did not run. +//! +//! Binding a credential to a specific (agent, conversation) pair **is the +//! host's concern**; the request body carries no requester and no scope, +//! precisely so the host's binding is the only thing that decides authority. +//! Since the credential is the whole of the agent's authority, serving this +//! over anything but a loopback socket or TLS is publishing it. + +use std::future::Future; +use std::pin::Pin; + +use super::{BrokerResponse, BrokerResult, PreparedRequest}; + +/// Path of the single broker endpoint. +pub const BROKER_ACTION_PATH: &str = "/v1/action"; + +/// Header carrying the opaque session credential, as `Bearer `. +pub const BROKER_CREDENTIAL_HEADER: &str = "authorization"; + +/// No usable [`BrokerResponse`] was obtained, so the request's fate is unknown. +/// +/// Every variant means the same thing to a caller: nothing can be concluded +/// about side effects, and the only safe next step is to retry the identical +/// bytes (which the host will deduplicate) or to reconcile by reading state. +/// The variants differ only in what to tell an operator. Host verdicts never +/// appear here — this type is strictly for the absence of an answer. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum BrokerTransportError { + /// The host could not be reached, or the connection failed mid-request. + #[error("broker unreachable: {0}")] + Unreachable(String), + /// An HTTP response arrived carrying no parseable envelope. + /// + /// Typically an intermediary answering instead of the host: a proxy `401`, + /// a `404` for a missing route, a `502`. The status is recorded for + /// operators and carries no contractual meaning — an intermediary's `401` + /// does not prove the host never ran the action. + #[error("no broker envelope in HTTP {status} response: {detail}")] + NoEnvelope { + /// The HTTP status observed. + status: u16, + /// Operator-facing detail about what arrived instead. + detail: String, + }, + /// An envelope arrived but did not validate against the request that was + /// sent — wrong `requestId`, wrong action, a malformed outcome, or a status + /// contradicting its own error code. + /// + /// A host that answers something other than what was asked has given no + /// verdict at all, which is why this is a transport failure rather than a + /// `Failed` result. + #[error("malformed broker response: {0}")] + MalformedResponse(String), +} + +/// A future returned by [`BrokerClient::send`]. +/// +/// Spelled as a boxed future rather than `async fn` in the trait because this +/// trait must be object-safe: the harness holds one client and must not know +/// whether it talks to an in-process host or an HTTP one. +pub type BrokerFuture<'a> = + Pin> + Send + 'a>>; + +/// A future returned by [`BrokerClientExt::execute`]. +pub type ValidatedFuture<'a> = + Pin> + Send + 'a>>; + +/// A host response that has been checked against the request it answers. +/// +/// The only way to obtain one is [`ValidatedResponse::validate`], which +/// [`BrokerClientExt::execute`] calls — so correlation is not advice an +/// implementation may skip. [`BrokerResponse::validate_for`] remains public for +/// a host validating its own output, but a client never has to remember to +/// call it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ValidatedResponse(BrokerResponse); + +impl ValidatedResponse { + /// Check `response` against the request it claims to answer. + /// + /// # Errors + /// + /// Returns [`BrokerTransportError::MalformedResponse`] when the response + /// does not correlate, carries an outcome for a different action, asserts a + /// malformed identifier, or pairs a status with a code that contradicts it. + /// A response that fails here is not a host verdict — nothing can be + /// concluded about side effects from it. + pub fn validate( + response: BrokerResponse, + request: &PreparedRequest, + ) -> Result { + response + .validate_for(request) + .map_err(|e| BrokerTransportError::MalformedResponse(e.to_string()))?; + Ok(Self(response)) + } + + /// The terminal disposition the host reached. + #[must_use] + pub fn result(&self) -> &BrokerResult { + &self.0.result + } + + /// The correlated `requestId`. + #[must_use] + pub fn request_id(&self) -> &str { + &self.0.request_id + } + + /// Whether the host replayed a previously recorded outcome. + #[must_use] + pub fn replayed(&self) -> bool { + self.0.replayed + } + + /// The underlying envelope, for logging or re-serialization. + #[must_use] + pub fn envelope(&self) -> &BrokerResponse { + &self.0 + } + + /// Consume this wrapper, yielding the validated envelope. + #[must_use] + pub fn into_envelope(self) -> BrokerResponse { + self.0 + } +} + +/// Permission to call [`BrokerClient::send`], which only +/// [`BrokerClientExt::execute`] can mint. +/// +/// This is what makes validation *structurally* the only door. `send` must be +/// public — an out-of-crate implementation has to define it — but a caller +/// must not be able to invoke it and receive an uncorrelated envelope. A token +/// with a private field satisfies both: any crate can accept one in a +/// signature, only this module can construct one. An implementation should +/// ignore its value; it carries no data. +/// +/// An implementation that stashes and exposes the envelope it saw has +/// deliberately built an exfiltrating transport — a different thing from a +/// caller forgetting to correlate. Closing the accidental path is the goal. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Dispatch(()); + +/// Something that can execute broker requests. +/// +/// One method, because there is one endpoint: every operation is an +/// [`super::Action`] inside the request, so adding an action never changes +/// this trait. This is the **transport primitive** — frozen bytes out, one +/// envelope back — not the caller's interface; callers use +/// [`BrokerClientExt::execute`], where response correlation happens. +/// +/// Implementations must be usable as `dyn BrokerClient`. +pub trait BrokerClient: Send + Sync { + /// Send `request`'s frozen bytes and return whatever envelope came back. + /// + /// An implementation's whole job is transport: send + /// [`PreparedRequest::body`] verbatim, parse an envelope regardless of + /// HTTP status, and return it unjudged. The [`Dispatch`] argument is why + /// this cannot be called directly by an outside caller; that is deliberate. + /// + /// # Errors + /// + /// Returns [`BrokerTransportError`] when no envelope could be obtained or + /// parsed. A host that answered — even to refuse — returns `Ok`, with the + /// verdict in [`BrokerResponse::result`]. + fn send<'a>(&'a self, request: &'a PreparedRequest, dispatch: Dispatch) -> BrokerFuture<'a>; +} + +/// The caller-facing half of [`BrokerClient`]: send, then validate. +/// +/// Blanket-implemented for every [`BrokerClient`], including `dyn BrokerClient`, +/// and **not overridable** — coherence forbids a second implementation, so there +/// is exactly one definition of what validating a response means and no client +/// can weaken it. +pub trait BrokerClientExt: BrokerClient { + /// Send `request` and return a response already checked against it. + /// + /// # Errors + /// + /// Returns [`BrokerTransportError`] when no envelope arrived, or + /// [`BrokerTransportError::MalformedResponse`] when the envelope that + /// arrived does not answer `request`. Both mean the same thing to a caller: + /// no verdict, so nothing is known about side effects. + fn execute<'a>(&'a self, request: &'a PreparedRequest) -> ValidatedFuture<'a>; +} + +impl BrokerClientExt for C { + fn execute<'a>(&'a self, request: &'a PreparedRequest) -> ValidatedFuture<'a> { + Box::pin(async move { + let response = self.send(request, Dispatch(())).await?; + ValidatedResponse::validate(response, request) + }) + } +} diff --git a/crates/buzz-sdk/src/broker/correlate.rs b/crates/buzz-sdk/src/broker/correlate.rs new file mode 100644 index 0000000000..b6d4baf6ef --- /dev/null +++ b/crates/buzz-sdk/src/broker/correlate.rs @@ -0,0 +1,92 @@ +//! Response-to-request identity correlation. +//! +//! Split from [`super`] to keep that file within the repo's 1,000-line +//! ceiling; the rule it implements is part of response validation. + +use super::{ActionArgs, ActionOutcome, AgentTarget, PubkeyHex, SdkError}; + +/// Reject an outcome that echoes back a different identity than the request +/// supplied. +/// +/// What is and is not compared, and why comparison is on parsed identities +/// rather than bytes, is documented once on +/// [`BrokerResponse::validate_for`][super::BrokerResponse::validate_for] — the +/// public entry point a host author reads. +/// +/// The match below is exhaustive over [`ActionArgs`], so adding an action is a +/// compile error here rather than a silent default to "not compared". +pub(super) fn correlate_identities( + args: &ActionArgs, + outcome: &ActionOutcome, +) -> Result<(), SdkError> { + /// Compare two channel ids as UUIDs, so two spellings of one channel match. + /// + /// An unparseable id on either side is a mismatch rather than an error: + /// `validate`/`validated` already reject a malformed channel id with a + /// precise message, and duplicating that verdict here would report a + /// correlation failure for what is really a malformed payload. + fn same_channel(requested: &str, returned: &str) -> Result<(), SdkError> { + let parse = |value: &str| uuid::Uuid::parse_str(value).ok(); + match (parse(requested), parse(returned)) { + (Some(requested_id), Some(returned_id)) if requested_id == returned_id => Ok(()), + _ => Err(mismatch("channelId", requested, returned)), + } + } + + /// Compare two pubkeys. [`PubkeyHex::parse`] is the type's only + /// constructor and lowercases, so comparing typed values *is* the parsed + /// comparison. + fn same_pubkey(requested: &PubkeyHex, returned: &PubkeyHex) -> Result<(), SdkError> { + if requested == returned { + return Ok(()); + } + Err(mismatch( + "agentPubkey", + requested.as_str(), + returned.as_str(), + )) + } + + /// The error every mismatch reports, naming the field and both spellings. + fn mismatch(field: &str, requested: &str, returned: &str) -> SdkError { + SdkError::InvalidInput(format!( + "response {field} \"{returned}\" does not match the requested \"{requested}\"" + )) + } + + /// The pubkey a target names immutably, if it names one. + fn targeted(target: &AgentTarget) -> Option<&PubkeyHex> { + match target { + AgentTarget::Pubkey(pubkey) => Some(pubkey), + AgentTarget::Name(_) => None, + } + } + + match (args, outcome) { + (ActionArgs::AgentsCreate(args), ActionOutcome::AgentsCreate(outcome)) => { + same_channel(&args.channel_id, &outcome.channel_id) + } + (ActionArgs::AgentsUpdate(args), ActionOutcome::AgentsUpdate(outcome)) => { + match targeted(&args.target) { + Some(requested) => same_pubkey(requested, &outcome.agent_pubkey), + None => Ok(()), + } + } + (ActionArgs::AgentsDelete(args), ActionOutcome::AgentsDelete(outcome)) => { + match targeted(&args.target) { + Some(requested) => same_pubkey(requested, &outcome.agent_pubkey), + None => Ok(()), + } + } + // These outcomes echo no identity the request supplied. + (ActionArgs::ChannelRead(_), _) + | (ActionArgs::MessagePost(_), _) + | (ActionArgs::MessageReply(_), _) + | (ActionArgs::ReactionAdd(_), _) + | (ActionArgs::ProfileSet(_), _) + | (ActionArgs::StorageAddress(_), _) + | (ActionArgs::AgentsCreate(_), _) + | (ActionArgs::AgentsUpdate(_), _) + | (ActionArgs::AgentsDelete(_), _) => Ok(()), + } +} diff --git a/crates/buzz-sdk/src/broker/mod.rs b/crates/buzz-sdk/src/broker/mod.rs new file mode 100644 index 0000000000..e1117cc810 --- /dev/null +++ b/crates/buzz-sdk/src/broker/mod.rs @@ -0,0 +1,754 @@ +//! Agent ↔ broker contract — the operations an agent asks a host to perform. +//! +//! This module is a **contract only**: the request envelope, the closed set of +//! [`Action`]s, the result shape, the HTTP binding, and a client trait. No +//! host, no transport, no signing. The full design rationale lives in the +//! English spec (`docs/agent-broker.md`); doc comments here explain only what +//! the code cannot say itself. +//! +//! ```text +//! agent → BrokerRequest → (POST /v1/action, bearer credential) → host +//! host: authenticate → authorize → validate → execute → BrokerResponse +//! ``` +//! +//! The agent holds its public key and a session credential — no secret key, no +//! relay connection. Everything it wants to do, reading included, is an action. +//! Actions are named business operations rather than a `sign(bytes)` primitive +//! so a host can hold per-operation policy; that and the rest of the +//! [#6467](https://github.com/block/buzz/issues/6467) mapping are covered in +//! the spec. +//! +//! # Contract-wide rules +//! +//! - **No secret crosses this boundary, in either direction.** Every wire type +//! is strict — unknown members are rejected at every depth, and each type's +//! exact key set is pinned by test. Where a derive would have left a lax +//! reader ([`BrokerResponse`], [`BrokerMessage`], [`BrokerResult`]), the type +//! documents how its strictness is restored. +//! - **Identities have exactly one spelling.** UUIDs and hex admit several +//! legal spellings, so every identity is canonicalized at both doors — +//! `validated()` and deserialization. See [`actions`]'s shared validators. +//! - **Omission is the only spelling of absence.** An explicit `null` is +//! rejected anywhere, at any depth. Canonical rationale on the +//! `absent_or_valued` guard in [`actions`]; host implementers must configure +//! serializers to omit unset members. +//! - **No request names its own subject.** Requester, owner, and scope are +//! derived from the authenticated credential; see [`BrokerRequest`]. This is +//! also why `agents.create` has no owner field — the creator owns the agent, +//! and the ownership chain always terminates at a human. Bounding its depth +//! is a host concern. +//! +//! Two limits worth stating: a `String` field can physically hold secret text, +//! so keeping secrets out of content and error messages is host policy; and +//! nothing stops a host from *holding* keys — that is the point. It stops one +//! from handing them over. +//! +//! # Deferred operations +//! +//! Not in v1, all purely additive later: memory read/write (intent-level +//! operations over the encrypted store — until then [`Action::StorageAddress`] +//! only addresses a record, and the key holder remains the only reader/writer), +//! `presence.set`, `typing.set`, and streaming reads (waking on a mention is +//! `channel.read` with `mentionsOnly`, polled). +//! +//! # Non-goals +//! +//! Hosts (auth, idempotency storage, execution), transports, relay changes, +//! grant/authorization fields (added when a real verifier exists, not before), +//! and secret-key custody. [`BrokerClient`] exists so in-process and HTTP +//! implementations are interchangeable; neither is here. + +use serde::{Deserialize, Serialize}; + +use crate::SdkError; + +pub mod actions; +pub mod client; +mod correlate; +mod wire; + +use actions::absent_or_valued; +pub use actions::{ + Action, ActionArgs, ActionOutcome, AgentTarget, AgentsCreateArgs, AgentsCreateOutcome, + AgentsDeleteArgs, AgentsDeleteOutcome, AgentsUpdateArgs, AgentsUpdateOutcome, BrokerMessage, + ChannelReadArgs, EventPublished, MessagePage, MessagePostArgs, MessageReplyArgs, + ProfileSetArgs, PubkeyHex, ReactionAddArgs, StorageAddress, StorageAddressArgs, +}; +pub use client::{ + BrokerClient, BrokerClientExt, BrokerFuture, BrokerTransportError, Dispatch, ValidatedFuture, + ValidatedResponse, BROKER_ACTION_PATH, BROKER_CREDENTIAL_HEADER, +}; + +/// Wire `type` discriminator for a broker request payload. +pub const BROKER_REQUEST_TYPE: &str = "broker_request"; + +/// Wire `type` discriminator for a broker response 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 action. +/// +/// There is deliberately no requester, owner, scope, or relay field: those are +/// derived by the host from the authenticated session credential. **A body +/// that could name its own subject would let any caller act as anyone.** +/// +/// # Retry contract +/// +/// Retrying means resending the identical bytes with the same `requestId` — +/// the host compares a digest of the bytes against what it recorded under that +/// idempotency key (same digest → replay the recorded outcome; different → +/// [`BrokerErrorCode::RequestIdConflict`]). Two serializations of one value +/// can differ in bytes, so a client never sends this type directly: call +/// [`Self::prepare`] to freeze it into a [`PreparedRequest`] and hand *that* +/// to [`BrokerClientExt::execute`]. +#[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, + /// Action contract version the caller wrote `args` against. + pub action_version: u16, + /// The action to invoke, with its strictly typed arguments. + #[serde(flatten)] + pub action: ActionArgs, +} + +impl BrokerRequest { + /// Build a request for `action` at the current protocol version. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] if `request_id` is empty, longer than + /// [`MAX_REQUEST_ID_LEN`], or not printable ASCII, or if the action's + /// arguments fail validation. + pub fn new(request_id: impl Into, action: ActionArgs) -> Result { + let request_id = request_id.into(); + validate_request_id(&request_id)?; + // Store the normalized copy, not the caller's, so a padded-but-valid + // value cannot travel in the frozen body. + let action = action.validated()?; + Ok(Self { + r#type: BROKER_REQUEST_TYPE.to_string(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id, + action_version: action.action().current_version(), + action, + }) + } + + /// The action this request invokes. + #[must_use] + pub fn action(&self) -> Action { + self.action.action() + } + + /// Validate and normalize into the only form execution-side code accepts. + /// + /// This is the **one normalization door**, and it consumes the request. + /// There is deliberately no non-consuming `validate(&self)`: a verdict + /// about a value it does not replace can drift from the value the caller + /// keeps holding (an earlier one validated a normalized copy, discarded + /// it, and let the caller execute the un-normalized original). The only + /// way to learn a request is valid is to receive the normalized + /// [`ValidatedRequest`]. + /// + /// # Errors + /// + /// Returns [`SdkError`] for a wrong `type`, an unsupported + /// `protocolVersion` or `actionVersion`, a malformed `requestId`, or + /// arguments that fail their own validation. + pub fn validated(mut self) -> Result { + self.validate_envelope()?; + self.action = self.action.validated()?; + Ok(ValidatedRequest(self)) + } + + /// Validate and normalize, then serialize once into the bytes every attempt + /// will send — [`Self::validated`] followed by [`ValidatedRequest::prepare`]. + /// + /// # Errors + /// + /// Returns [`SdkError`] when [`Self::validated`] fails, or + /// [`SdkError::InvalidInput`] if serialization fails. + pub fn prepare(self) -> Result { + self.validated()?.prepare() + } + + /// Validate everything except the action arguments, which + /// [`Self::validated`] normalizes in the same step. + fn validate_envelope(&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 action = self.action(); + if self.action_version != action.current_version() { + return Err(SdkError::InvalidInput(format!( + "unsupported actionVersion {} for {} (expected {})", + self.action_version, + action.as_str(), + action.current_version() + ))); + } + Ok(()) + } +} + +/// Validate a `requestId`: non-empty, bounded, printable ASCII without spaces. +/// +/// The bound and character set exist because this value becomes part of a +/// durable idempotency 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(()) +} + +/// A [`BrokerRequest`] that has been validated **and normalized**. +/// +/// The type execution-side code accepts. The only way to obtain one is +/// [`BrokerRequest::validated`], which normalizes on the way through, so +/// holding one proves the value carries what the validator approved — not the +/// caller's spelling of it. +/// +/// The inner request is private with no borrowing accessor: a borrow would let +/// execution-side code clone it, mutate a public field, and execute the result. +/// [`Self::into_request`] consumes the wrapper for a host that needs to move +/// the envelope onward; what it yields is no longer evidence of anything. +/// +/// A host that receives bytes builds one the same way a client does — parse, +/// call `validated()`, execute what comes back — since only its own verdict is +/// authoritative. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ValidatedRequest(BrokerRequest); + +impl ValidatedRequest { + /// The action to execute, with its normalized arguments. + #[must_use] + pub fn args(&self) -> &ActionArgs { + &self.0.action + } + + /// The action being invoked. + #[must_use] + pub fn action(&self) -> Action { + self.0.action() + } + + /// The idempotency key the host keys replay on. + #[must_use] + pub fn request_id(&self) -> &str { + &self.0.request_id + } + + /// Freeze the normalized request into the bytes every attempt will send. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] if serialization fails. + pub fn prepare(self) -> Result { + let body = serde_json::to_vec(&self.0).map_err(|e| { + SdkError::InvalidInput(format!("broker request is not serializable: {e}")) + })?; + Ok(PreparedRequest { + request: self.0, + body, + }) + } + + /// Consume this wrapper, yielding the normalized envelope — a plain + /// [`BrokerRequest`] with public fields, no longer evidence that anything + /// was validated, which is why this consumes rather than borrows. + #[must_use] + pub fn into_request(self) -> BrokerRequest { + self.0 + } +} + +/// A validated request together with the exact bytes to send. +/// +/// This is what [`BrokerClient::send`] takes, so the retry contract is +/// structural: every attempt sends `body` verbatim, and no implementation gets +/// the chance to reserialize. The typed request is deliberately not exposed — +/// only the correlation metadata ([`Self::request_id`], [`Self::action`]) an +/// implementation legitimately needs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PreparedRequest { + request: BrokerRequest, + body: Vec, +} + +impl PreparedRequest { + /// The frozen JSON body. Every attempt sends exactly these bytes. + #[must_use] + pub fn body(&self) -> &[u8] { + &self.body + } + + /// The idempotency key the host keys replay on. + #[must_use] + pub fn request_id(&self) -> &str { + &self.request.request_id + } + + /// The action being invoked. + #[must_use] + pub fn action(&self) -> Action { + self.request.action() + } +} + +/// Machine-readable broker error code. +/// +/// These name failures the *broker* is responsible for; failures inside an +/// action arrive as [`BrokerErrorCode::ActionFailed`] with detail in the +/// message. +/// +/// # Which status a code may carry +/// +/// A code and a [`BrokerResult`] status are two statements about the same +/// thing — whether side effects landed — so they cannot be paired freely. +/// `Failed` promises no side effects took hold; `Indeterminate` promises +/// nothing. This is the whole table, and it lives only here: +/// +/// | Code | with `failed` | with `indeterminate` | +/// |---|---|---| +/// | `outcome_unknown` | no | yes | +/// | `internal` | yes | yes | +/// | every other code | yes | no | +/// +/// [`Self::Internal`] is the one code legitimately either: a host fault before +/// dispatch is a known no-op, the same fault mid-execution is not. +/// [`Self::may_be_failed`] and [`Self::may_be_indeterminate`] are this table in +/// code, consulted by [`BrokerResponse::validate`], which rejects a mismatched +/// pairing as malformed rather than trusting either half. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum BrokerErrorCode { + /// The envelope or action arguments failed validation. + InvalidRequest, + /// The `protocolVersion` is not supported by this host. + UnsupportedProtocolVersion, + /// The action name is unknown to this host. + UnknownAction, + /// The `actionVersion` is not supported for this action. + UnsupportedActionVersion, + /// The host knows this action but does not offer it. + /// + /// For an action where [`Action::is_best_effort`] holds, this is a normal + /// answer and the agent carries on. Otherwise the agent cannot do its job + /// on this host. + Unsupported, + /// The session credential was missing, malformed, or rejected. + /// + /// A host verdict, delivered as [`BrokerResult::Failed`], never as a + /// transport error: the request was refused before execution, so the caller + /// knows no side effects occurred. + Unauthenticated, + /// The requester is authenticated but not permitted this action. + Unauthorized, + /// Reuse of a `requestId` with different request content. + RequestIdConflict, + /// The action ran and reported a domain failure. + ActionFailed, + /// The host could not determine whether side effects occurred. + 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::UnknownAction => "unknown_action", + Self::UnsupportedActionVersion => "unsupported_action_version", + Self::Unsupported => "unsupported", + Self::Unauthenticated => "unauthenticated", + Self::Unauthorized => "unauthorized", + Self::RequestIdConflict => "request_id_conflict", + Self::ActionFailed => "action_failed", + Self::OutcomeUnknown => "outcome_unknown", + Self::Internal => "internal", + } + } + + /// Whether this code may appear with [`BrokerResult::Failed`]. + /// + /// One half of the table documented on [`BrokerErrorCode`], written as an + /// exhaustive match so adding a code forces a decision here. + #[must_use] + pub fn may_be_failed(self) -> bool { + match self { + Self::InvalidRequest + | Self::UnsupportedProtocolVersion + | Self::UnknownAction + | Self::UnsupportedActionVersion + | Self::Unsupported + | Self::Unauthenticated + | Self::Unauthorized + | Self::RequestIdConflict + | Self::ActionFailed + | Self::Internal => true, + Self::OutcomeUnknown => false, + } + } + + /// Whether this code may appear with [`BrokerResult::Indeterminate`] — + /// the other half of the table documented on [`BrokerErrorCode`]. + #[must_use] + pub fn may_be_indeterminate(self) -> bool { + match self { + Self::OutcomeUnknown | Self::Internal => true, + Self::InvalidRequest + | Self::UnsupportedProtocolVersion + | Self::UnknownAction + | Self::UnsupportedActionVersion + | Self::Unsupported + | Self::Unauthenticated + | Self::Unauthorized + | Self::RequestIdConflict + | Self::ActionFailed => false, + } + } +} + +/// 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::Unsupported`] error. + pub fn unsupported(message: impl Into) -> Self { + Self::new(BrokerErrorCode::Unsupported, 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. [`Self::Indeterminate`] is distinct from +/// [`Self::Failed`] on purpose: `Failed` promises no side effects took hold, +/// `Indeterminate` promises nothing and demands reconciliation. Which +/// [`BrokerErrorCode`] may carry which status is a closed table on that type. +/// +/// # Why this type is not [`Deserialize`] +/// +/// Its members reach the wire only flattened into [`BrokerResponse`], whose +/// strict reader enforces the exact key set per status. A derived reader here +/// was a second, laxer door onto the same bytes — it accepted and dropped +/// members the envelope rejects — and two copies of a strictness check drift. +/// [`Serialize`] is retained (it produces the envelope's flattened wire form), +/// so this is a read-side restriction only. Nothing is lost: a bare +/// `{"status": …}` object is not a payload this contract defines. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(tag = "status", rename_all = "snake_case")] +pub enum BrokerResult { + /// The action completed and produced this outcome. + Succeeded { + /// Action-specific success payload. + #[serde(flatten)] + outcome: ActionOutcome, + }, + /// The action 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: ActionOutcome) -> 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 } + } + + /// The outcome, when this is a success. + #[must_use] + pub fn outcome(&self) -> Option<&ActionOutcome> { + match self { + Self::Succeeded { outcome } => Some(outcome), + Self::Failed { .. } | Self::Indeterminate { .. } => None, + } + } + + /// 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. +/// +/// # Why deserialization goes through an intermediary +/// +/// `#[serde(flatten)]` on `result` silently disables `deny_unknown_fields`, so +/// the derived reader accepted and discarded unknown members — exactly how a +/// secret-bearing host field crosses a boundary unnoticed. [`Deserialize`] +/// therefore routes through a private strict wire form (see `wire.rs`) with an +/// exact key set per status; anything else fails to parse and surfaces as +/// [`BrokerTransportError::MalformedResponse`]. Serialization is unchanged, and +/// a round-trip test pins that the strict reader accepts what the writer emits. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[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. + /// + /// A plain `bool`, so it needs no explicit null guard: `null` already fails + /// as a type error rather than defaulting to `false`. + #[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. + /// + /// This checks only what a response asserts about itself. It cannot tell + /// whether the response answers the request that was sent — for that, and + /// for outcome-field validation, use [`Self::validate_for`]. A client should + /// always prefer `validate_for`. + /// + /// # Errors + /// + /// Returns [`SdkError::InvalidInput`] on a wrong `type`, an unsupported + /// `protocolVersion`, a malformed `requestId`, an outcome with malformed + /// identifiers, or an error code paired with the wrong status. + 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)?; + match &self.result { + BrokerResult::Succeeded { outcome } => outcome.validate()?, + // A forbidden code/status pairing is a response contradicting + // itself; neither half can be trusted, so it is rejected. The + // table lives on `BrokerErrorCode`. + BrokerResult::Failed { error } if !error.code.may_be_failed() => { + return Err(SdkError::InvalidInput(format!( + "{} is not a valid code for a failed status", + error.code.as_str() + ))); + } + BrokerResult::Indeterminate { error } if !error.code.may_be_indeterminate() => { + return Err(SdkError::InvalidInput(format!( + "{} is not a valid code for an indeterminate status", + error.code.as_str() + ))); + } + BrokerResult::Failed { .. } | BrokerResult::Indeterminate { .. } => {} + } + Ok(()) + } + + /// Validate this response *as the answer to `request`*. + /// + /// A response that validates in isolation can still be the wrong answer — + /// a success for a different action, or for the wrong subject. A client + /// never calls this directly: [`BrokerClientExt::execute`] runs it for + /// every implementation and returns a [`ValidatedResponse`]. It stays + /// public for a host validating its own output. Signature verification of + /// read results is deliberately not included; see [`BrokerMessage::verify`]. + /// + /// # Errors + /// + /// Returns everything [`Self::validate`] returns, plus + /// [`SdkError::InvalidInput`] when the `requestId` does not correlate, a + /// success outcome names a different action than the request, a success + /// outcome echoes a different identity than the request supplied, or a + /// read returned more messages than the request allowed. + /// + /// # What identity correlation compares + /// + /// Every identity the request supplies and the outcome echoes must name + /// the same thing. Most outcomes echo nothing prior (host-minted ids, or a + /// page with no `channelId` echo; `storage.address` deliberately omits the + /// slug, whose `d` tag is a keyed hash of it). What remains: `agents.create` + /// compares `channelId` as UUIDs; `agents.update`/`agents.delete` compare + /// `agentPubkey` when targeted by pubkey. A name target is resolved + /// host-side and unverifiable by construction — a rename may be the very + /// thing the call performed. + /// + /// **Comparison is on parsed identities, never on bytes**: both identity + /// types admit more than one legal spelling, and a byte comparison would + /// reject a correct answer spelled differently — a worse failure than the + /// one this check exists to catch. + pub fn validate_for(&self, request: &PreparedRequest) -> Result<(), SdkError> { + self.validate()?; + if self.request_id != request.request_id() { + return Err(SdkError::InvalidInput(format!( + "response requestId \"{}\" does not match request \"{}\"", + self.request_id, + request.request_id() + ))); + } + if let BrokerResult::Succeeded { outcome } = &self.result { + let expected = request.action(); + if outcome.action() != expected { + return Err(SdkError::InvalidInput(format!( + "response carries a {} outcome for a {} request", + outcome.action().as_str(), + expected.as_str() + ))); + } + correlate::correlate_identities(&request.request.action, outcome)?; + // `ActionOutcome::validate` never sees the request, so it can only + // enforce the protocol-wide cap; the request's own limit is + // applied here, the one place both halves are in scope. + if let ( + ActionArgs::ChannelRead(args), + ActionOutcome::ChannelRead(MessagePage { messages, .. }), + ) = (&request.request.action, outcome) + { + let allowed = args.effective_limit() as usize; + if messages.len() > allowed { + return Err(SdkError::InvalidInput(format!( + "read returned {} messages for a limit of {allowed}", + messages.len() + ))); + } + } + } + Ok(()) + } +} + +#[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 0000000000..14a55005ae --- /dev/null +++ b/crates/buzz-sdk/src/broker/tests.rs @@ -0,0 +1,2811 @@ +//! Contract tests for the broker envelope, actions, and client trait. + +use super::*; +use nostr::{EventBuilder, Keys, Kind, Tag}; + +const CHANNEL: &str = "b2c38ca8-9ec3-411e-bab5-f9deab34d52e"; +const PUBKEY: &str = "a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971"; +const EVENT: &str = "78d47c4f36a2d048f45b57a31d964a3ce239f0fc46162c5d7c90db2b5aa52bc6"; + +fn pubkey() -> PubkeyHex { + PubkeyHex::parse(PUBKEY).expect("fixture pubkey is valid hex") +} + +/// A genuinely signed event, so read fixtures exercise real verification rather +/// than a hand-built value that could never verify. +fn signed_message(keys: &Keys) -> BrokerMessage { + let event = EventBuilder::new(Kind::Custom(9), "hello") + .tags([ + Tag::parse(["h", CHANNEL]).expect("h tag"), + Tag::parse(["e", EVENT, "", "root"]).expect("e tag"), + Tag::parse(["p", PUBKEY]).expect("p tag"), + ]) + .sign_with_keys(keys) + .expect("fixture event signs"); + BrokerMessage(event) +} + +/// Every [`BrokerErrorCode`] variant, so code-driven tables cannot silently skip +/// one: [`error_codes_have_stable_wire_strings`] pins this list against the enum. +fn all_error_codes() -> [BrokerErrorCode; 11] { + use BrokerErrorCode as E; + [ + E::InvalidRequest, + E::UnsupportedProtocolVersion, + E::UnknownAction, + E::UnsupportedActionVersion, + E::Unsupported, + E::Unauthenticated, + E::Unauthorized, + E::RequestIdConflict, + E::ActionFailed, + E::OutcomeUnknown, + E::Internal, + ] +} + +/// One valid `args` value per action, so table-driven tests cannot silently +/// skip an action: [`fixtures_cover_every_action`] pins the coverage. +fn action_fixtures() -> Vec { + vec![ + ActionArgs::ChannelRead(ChannelReadArgs { + channel_id: CHANNEL.into(), + root_event_id: Some(EVENT.into()), + mentions_only: true, + cursor: Some("opaque-host-cursor-v1".into()), + limit: Some(50), + }), + ActionArgs::MessagePost(MessagePostArgs { + channel_id: CHANNEL.into(), + content: "shipping the contract".into(), + mentions: vec![pubkey()], + }), + ActionArgs::MessageReply(MessageReplyArgs { + channel_id: CHANNEL.into(), + reply_to_event_id: EVENT.into(), + content: "agreed".into(), + mentions: vec![pubkey()], + }), + ActionArgs::ReactionAdd(ReactionAddArgs { + channel_id: CHANNEL.into(), + target_event_id: EVENT.into(), + reaction: "🎉".into(), + }), + ActionArgs::ProfileSet(ProfileSetArgs { + display_name: Some("ss-dev-00".into()), + about: Some("implementation".into()), + picture: Some("https://example.invalid/avatar.png".into()), + }), + ActionArgs::StorageAddress(StorageAddressArgs { + slug: "mem/broker-foundation".into(), + }), + ActionArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Research helper".into(), + system_prompt: "Find sources.".into(), + runtime: Some("buzz-acp".into()), + provider: Some("anthropic".into()), + model: Some("claude-sonnet-4-5".into()), + respond_to: Some("owner-only".into()), + }), + ActionArgs::AgentsUpdate(AgentsUpdateArgs { + target: AgentTarget::Pubkey(pubkey()), + display_name: Some("Research helper v2".into()), + system_prompt: Some("Find better sources.".into()), + runtime: Some("buzz-acp".into()), + provider: Some("anthropic".into()), + model: Some("claude-sonnet-4-5".into()), + respond_to: Some("anyone".into()), + }), + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name("Research helper".into()), + }), + ] +} + +/// One outcome per action, matching the fixture order above. +fn outcome_fixtures(keys: &Keys) -> Vec { + let page = MessagePage { + messages: vec![signed_message(keys)], + next_cursor: Some("opaque-host-cursor-v2".into()), + }; + let published = EventPublished { + event_id: EVENT.into(), + kind: 9, + created_at: 1_764_000_003, + }; + vec![ + ActionOutcome::ChannelRead(page), + ActionOutcome::MessagePost(published.clone()), + ActionOutcome::MessageReply(published.clone()), + ActionOutcome::ReactionAdd(published.clone()), + ActionOutcome::ProfileSet(published), + ActionOutcome::StorageAddress(StorageAddress { + author_pubkey: pubkey(), + kind: 30174, + d_tag: EVENT.into(), + }), + ActionOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: pubkey(), + display_name: "Research helper".into(), + channel_id: CHANNEL.into(), + }), + ActionOutcome::AgentsUpdate(AgentsUpdateOutcome { + agent_pubkey: pubkey(), + display_name: "Research helper v2".into(), + updated_fields: vec!["displayName".into()], + }), + ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: pubkey(), + display_name: "Research helper".into(), + }), + ] +} + +fn prepared(args: ActionArgs) -> PreparedRequest { + BrokerRequest::new("req-1", args) + .expect("fixture request builds") + .prepare() + .expect("fixture request prepares") +} + +/// Sorted JSON object keys of `value`, for exact-schema assertions. +fn keys_of(value: &serde_json::Value) -> Vec { + let mut keys: Vec = value + .as_object() + .expect("expected a JSON object") + .keys() + .cloned() + .collect(); + keys.sort(); + keys +} + +// ── Coverage ──────────────────────────────────────────────────────────────── + +/// The fixture tables are the input to every table-driven test below, so an +/// action added without a fixture would be silently untested. This is the guard. +#[test] +fn fixtures_cover_every_action() { + let keys = Keys::generate(); + let mut from_args: Vec<&str> = action_fixtures() + .iter() + .map(|args| args.action().as_str()) + .collect(); + let mut from_outcomes: Vec<&str> = outcome_fixtures(&keys) + .iter() + .map(|outcome| outcome.action().as_str()) + .collect(); + let mut declared: Vec<&str> = Action::ALL.iter().map(|a| a.as_str()).collect(); + + from_args.sort_unstable(); + from_outcomes.sort_unstable(); + declared.sort_unstable(); + + assert_eq!(from_args, declared, "every action needs an args fixture"); + assert_eq!( + from_outcomes, declared, + "every action needs an outcome fixture" + ); + + let mut unique = declared.clone(); + unique.dedup(); + assert_eq!(unique.len(), declared.len(), "wire names must be unique"); +} + +// ── Envelope round-trip ───────────────────────────────────────────────────── + +#[test] +fn every_action_round_trips_through_a_request_envelope() { + for args in action_fixtures() { + let action = args.action(); + let request = BrokerRequest::new("req-1", args) + .unwrap_or_else(|e| panic!("{} fixture must validate: {e}", action.as_str())); + + let json = serde_json::to_value(&request).expect("request serializes"); + assert_eq!(json["type"], BROKER_REQUEST_TYPE); + assert_eq!(json["protocolVersion"], 1); + assert_eq!(json["requestId"], "req-1"); + assert_eq!(json["actionVersion"], 1); + assert_eq!( + json["action"], + action.as_str(), + "{} must name itself on the wire", + action.as_str() + ); + assert!( + json.get("args").is_some(), + "{} must carry an args object", + action.as_str() + ); + + let parsed: BrokerRequest = serde_json::from_value(json) + .unwrap_or_else(|e| panic!("{} must deserialize: {e}", action.as_str())); + assert_eq!(parsed, request); + parsed.validated().expect("round-tripped request is valid"); + } +} + +#[test] +fn every_outcome_round_trips_through_a_response_envelope() { + let signer = Keys::generate(); + for outcome in outcome_fixtures(&signer) { + let action = outcome.action(); + let response = BrokerResponse::new("req-1", BrokerResult::succeeded(outcome.clone())); + response.validate().expect("response is valid"); + + let json = serde_json::to_value(&response).expect("response serializes"); + assert_eq!(json["type"], BROKER_RESULT_TYPE); + assert_eq!(json["status"], "succeeded"); + assert_eq!(json["action"], action.as_str()); + assert!(json.get("error").is_none(), "a success carries no error"); + // `replayed` is delivery metadata and stays off the wire when false. + assert!(json.get("replayed").is_none()); + + let parsed: BrokerResponse = serde_json::from_value(json) + .unwrap_or_else(|e| panic!("{} outcome must deserialize: {e}", action.as_str())); + assert_eq!(parsed, response); + assert_eq!(parsed.result.outcome(), Some(&outcome)); + assert!(parsed.result.error().is_none()); + } +} + +/// Args and outcome share the `action` discriminator, so a payload can never +/// pair one action's name with another's shape. +#[test] +fn an_args_shape_cannot_be_paired_with_another_action_name() { + let json = serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "actionVersion": 1, + "action": "agents.delete", + "args": { "channelId": CHANNEL, "content": "not a delete" }, + }); + assert!(serde_json::from_value::(json).is_err()); +} + +/// `#[serde(flatten)]` silently disables `deny_unknown_fields`, so the response +/// envelope — the one payload here that needs `flatten` for its wire shape — read +/// as strict while accepting and discarding extra keys. Every rejection below +/// parsed cleanly before the strict intermediary existed. +/// +/// The request envelope has the same `flatten` but *not* the same hole: its +/// `ActionArgs` is adjacently tagged, contributing exactly `action` and `args`, +/// so `deny_unknown_fields` still applies to the whole set. That is pinned in +/// [`a_request_envelope_rejects_anything_outside_its_exact_key_set`] rather than +/// assumed. +#[test] +fn a_response_envelope_rejects_anything_outside_its_exact_key_set() { + let succeeded = || { + serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "succeeded", + "action": "agents.delete", + "outcome": { "agentPubkey": PUBKEY, "displayName": "Gone" }, + }) + }; + let failed = || { + serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "failed", + "error": { "code": "action_failed", "message": "no" }, + }) + }; + assert!(serde_json::from_value::(succeeded()).is_ok()); + assert!(serde_json::from_value::(failed()).is_ok()); + + let mut rejected: Vec<(&str, serde_json::Value)> = Vec::new(); + + // An unknown top-level key, including one that reads as key material. + for extra in ["hostNote", "secretKey", "credential"] { + let mut json = succeeded(); + json[extra] = serde_json::json!("nsec1deadbeef"); + rejected.push((extra, json)); + let mut json = failed(); + json[extra] = serde_json::json!("nsec1deadbeef"); + rejected.push((extra, json)); + } + + // Members the declared status does not admit. Each of these is a + // contradiction the type system already forbids in Rust, and the envelope + // used to accept it on the wire and drop the half it could not represent. + let mut error_beside_success = succeeded(); + error_beside_success["error"] = serde_json::json!({ "code": "internal", "message": "?" }); + rejected.push(("error beside a success", error_beside_success)); + + let mut outcome_beside_failure = failed(); + outcome_beside_failure["action"] = serde_json::json!("agents.delete"); + outcome_beside_failure["outcome"] = + serde_json::json!({ "agentPubkey": PUBKEY, "displayName": "Gone" }); + rejected.push(("outcome beside a failure", outcome_beside_failure)); + + let mut outcome_beside_indeterminate = failed(); + outcome_beside_indeterminate["status"] = serde_json::json!("indeterminate"); + outcome_beside_indeterminate["error"] = + serde_json::json!({ "code": "outcome_unknown", "message": "?" }); + outcome_beside_indeterminate["action"] = serde_json::json!("agents.delete"); + outcome_beside_indeterminate["outcome"] = + serde_json::json!({ "agentPubkey": PUBKEY, "displayName": "Gone" }); + rejected.push(( + "outcome beside an indeterminate", + outcome_beside_indeterminate, + )); + + // Missing the member its status requires. + let mut no_outcome = succeeded(); + no_outcome.as_object_mut().unwrap().remove("outcome"); + rejected.push(("success with no outcome", no_outcome)); + let mut no_error = failed(); + no_error.as_object_mut().unwrap().remove("error"); + rejected.push(("failure with no error", no_error)); + + // An unknown status is not a fourth disposition to ignore. + for status in ["succeeded_partially", "pending", "SUCCEEDED", ""] { + let mut json = failed(); + json["status"] = serde_json::json!(status); + rejected.push(("unknown status", json)); + } + + // Strictness still reaches inside the outcome. + let mut extra_in_outcome = succeeded(); + extra_in_outcome["outcome"]["secretKey"] = serde_json::json!("nsec1deadbeef"); + rejected.push(("unknown key inside the outcome", extra_in_outcome)); + + let mut extra_in_error = failed(); + extra_in_error["error"]["secretKey"] = serde_json::json!("nsec1deadbeef"); + rejected.push(("unknown key inside the error", extra_in_error)); + + for (what, json) in rejected { + assert!( + serde_json::from_value::(json.clone()).is_err(), + "{what} must not deserialize: {json}" + ); + } +} + +/// Strict deserialization must not have narrowed what the writer emits: the +/// wire form is still the flattened one, and the strict reader is its inverse for +/// every status, with and without the optional `replayed`. +#[test] +fn the_strict_reader_accepts_exactly_what_the_writer_emits() { + let signer = Keys::generate(); + let mut results: Vec = outcome_fixtures(&signer) + .into_iter() + .map(BrokerResult::succeeded) + .collect(); + results.push(BrokerResult::failed(BrokerError::new( + BrokerErrorCode::ActionFailed, + "runtime not installed", + ))); + results.push(BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "host restarted mid-execution", + ))); + + for result in results { + for replayed in [false, true] { + let response = if replayed { + BrokerResponse::new("req-1", result.clone()).replayed() + } else { + BrokerResponse::new("req-1", result.clone()) + }; + let json = serde_json::to_value(&response).expect("response serializes"); + let parsed: BrokerResponse = serde_json::from_value(json.clone()) + .unwrap_or_else(|e| panic!("strict reader rejected our own bytes {json}: {e}")); + assert_eq!(parsed, response); + assert_eq!(parsed.replayed, replayed); + } + } +} + +/// The request envelope flattens too, so it was checked for the same hole. It +/// does not have one — `ActionArgs` is adjacently tagged and contributes exactly +/// `action` and `args`, leaving `deny_unknown_fields` in force — and this pins +/// that, so the request side cannot regress into the response side's bug. +#[test] +fn a_request_envelope_rejects_anything_outside_its_exact_key_set() { + let valid = || { + serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "actionVersion": 1, + "action": "channel.read", + "args": { "channelId": CHANNEL }, + }) + }; + assert!(serde_json::from_value::(valid()).is_ok()); + + // Unknown top-level key, beside the flattened discriminator, and inside the + // args — all four positions a smuggled field could take. + for extra in ["hostNote", "secretKey", "onBehalfOf", "envVars"] { + let mut json = valid(); + json[extra] = serde_json::json!("nsec1deadbeef"); + assert!( + serde_json::from_value::(json.clone()).is_err(), + "a request carrying top-level \"{extra}\" must not deserialize: {json}" + ); + + let mut json = valid(); + json["args"][extra] = serde_json::json!("nsec1deadbeef"); + assert!( + serde_json::from_value::(json.clone()).is_err(), + "a request carrying \"{extra}\" inside args must not deserialize: {json}" + ); + } + + // A second discriminator-shaped key is not a place to hide one either. + let mut extra_tag = valid(); + extra_tag["outcome"] = serde_json::json!({}); + assert!(serde_json::from_value::(extra_tag).is_err()); + + // Missing required members, so the pin cannot pass by accepting anything. + for missing in [ + "type", + "protocolVersion", + "requestId", + "actionVersion", + "args", + ] { + let mut json = valid(); + json.as_object_mut().unwrap().remove(missing); + assert!( + serde_json::from_value::(json).is_err(), + "a request missing \"{missing}\" must not deserialize" + ); + } +} + +/// Every JSON-pointer path to an object member reachable in `value`, including +/// members nested inside arrays, so a null-injection table cannot miss one. +fn member_paths(value: &serde_json::Value, prefix: &str, out: &mut Vec) { + match value { + serde_json::Value::Object(map) => { + for (key, child) in map { + // Escape per RFC 6901, so a key containing `/` or `~` still + // addresses the member it names. + let escaped = key.replace('~', "~0").replace('/', "~1"); + let path = format!("{prefix}/{escaped}"); + out.push(path.clone()); + member_paths(child, &path, out); + } + } + serde_json::Value::Array(items) => { + for (index, child) in items.iter().enumerate() { + member_paths(child, &format!("{prefix}/{index}"), out); + } + } + _ => {} + } +} + +/// The bug this guards: `#[serde(default)] Option` maps an explicit `null` to +/// `None`, which is indistinguishable from *absent*. The response envelope decides +/// its shape from absence, so `{"status":"failed","action":null,"outcome":null}` +/// and a succeeded response with `"error":null` both parsed as well-formed and +/// skipped the per-status contradiction check entirely — a malformed envelope +/// validating `Ok`. +/// +/// The rule adopted in response is uniform and therefore checkable: **no member +/// anywhere in this contract accepts an explicit `null`.** Nothing here emits one +/// (`skip_serializing_if` omits instead), so `null` is a second spelling of +/// "absent" that the contract simply does not define. One spelling means no layer +/// has to decide what a present-but-null member meant. +/// +/// This walks the real fixtures rather than a hand-written list of members, so an +/// optional field added later is covered without anyone remembering to add it +/// here. +#[test] +fn no_member_of_any_payload_accepts_an_explicit_null() { + let keys = Keys::generate(); + + // Requests: every action, with every optional member populated. + for args in action_fixtures() { + let request = BrokerRequest::new("req-1", args).expect("fixture request builds"); + let valid = serde_json::to_value(&request).expect("request serializes"); + // The untouched fixture must parse, or nulling members below would + // "reject" for a reason that has nothing to do with null. + assert_eq!( + serde_json::from_value::(valid.clone()).expect("fixture parses"), + request, + ); + + let mut paths = Vec::new(); + member_paths(&valid, "", &mut paths); + assert!( + paths.len() > 1, + "fixture should expose several members: {valid}" + ); + for path in paths { + let mut json = valid.clone(); + *json.pointer_mut(&path).expect("path addresses a member") = serde_json::Value::Null; + assert!( + serde_json::from_value::(json.clone()).is_err(), + "request with null at \"{path}\" must not deserialize: {json}" + ); + } + } + + // Responses: every outcome, plus both error-carrying statuses. + let mut results: Vec = outcome_fixtures(&keys) + .into_iter() + .map(BrokerResult::succeeded) + .collect(); + results.push(BrokerResult::failed(BrokerError::new( + BrokerErrorCode::ActionFailed, + "runtime not installed", + ))); + results.push(BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::OutcomeUnknown, + "host restarted mid-execution", + ))); + + for result in results { + let response = BrokerResponse::new("req-1", result).replayed(); + let valid = serde_json::to_value(&response).expect("response serializes"); + assert_eq!( + serde_json::from_value::(valid.clone()).expect("fixture parses"), + response, + ); + + let mut paths = Vec::new(); + member_paths(&valid, "", &mut paths); + for path in paths { + let mut json = valid.clone(); + *json.pointer_mut(&path).expect("path addresses a member") = serde_json::Value::Null; + assert!( + serde_json::from_value::(json.clone()).is_err(), + "response with null at \"{path}\" must not deserialize: {json}" + ); + } + } + // The two `bool` members carry no explicit guard, because `null` already + // fails as a type error rather than defaulting to `false`. Pin that, so the + // docs saying so cannot drift and so a later change to `Option` — which + // *would* need the guard — fails here. + let mut json = serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "actionVersion": 1, + "action": "channel.read", + "args": { "channelId": CHANNEL, "mentionsOnly": serde_json::Value::Null }, + }); + assert!( + serde_json::from_value::(json.clone()).is_err(), + "a null mentionsOnly must not deserialize: {json}" + ); + json = serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "failed", + "error": { "code": "action_failed", "message": "no" }, + "replayed": serde_json::Value::Null, + }); + assert!( + serde_json::from_value::(json.clone()).is_err(), + "a null replayed must not deserialize: {json}" + ); +} + +/// The exact repro that reached `Ok`: a member the declared status does not admit, +/// supplied as `null` rather than as a value. The fixtures above cannot cover this +/// — a serialized response never contains the member its status forbids — so each +/// status-incompatible member is injected here by name. +/// +/// This is the case that makes the null hole a contract bug rather than a +/// tidiness one: these envelopes contradict themselves, and before the fix +/// `validate()` returned `Ok(())` on all of them. +#[test] +fn a_status_incompatible_member_is_rejected_as_null_not_only_as_a_value() { + let succeeded = || { + serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "succeeded", + "action": "agents.delete", + "outcome": { "agentPubkey": PUBKEY, "displayName": "Gone" }, + }) + }; + let failed = || { + serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "failed", + "error": { "code": "action_failed", "message": "no" }, + }) + }; + + let mut cases: Vec<(String, serde_json::Value)> = Vec::new(); + + // `error` is the member a success does not admit. + let mut json = succeeded(); + json["error"] = serde_json::Value::Null; + cases.push(("null error beside a success".into(), json)); + + // `action` and `outcome` are the members the two failure statuses do not + // admit — individually and together, since the original report showed both. + for status in ["failed", "indeterminate"] { + let base = || { + let mut json = failed(); + json["status"] = serde_json::json!(status); + if status == "indeterminate" { + json["error"] = serde_json::json!({ "code": "outcome_unknown", "message": "?" }); + } + json + }; + for member in ["action", "outcome"] { + let mut json = base(); + json[member] = serde_json::Value::Null; + cases.push((format!("null {member} beside a {status}"), json)); + } + let mut json = base(); + json["action"] = serde_json::Value::Null; + json["outcome"] = serde_json::Value::Null; + cases.push((format!("null action and outcome beside a {status}"), json)); + } + + for (what, json) in cases { + let parsed = serde_json::from_value::(json.clone()); + // Assert on the parse, not on `validate()`: a response that parses and + // then fails validation would still have to be *reported* by a caller + // that remembered to validate. Rejecting at the boundary means a + // malformed envelope never becomes a value at all. + assert!( + parsed.is_err(), + "{what} must not deserialize, but parsed as {:?} which validates {:?}: {json}", + parsed.as_ref().ok(), + parsed.as_ref().map(BrokerResponse::validate).ok(), + ); + } +} + +// ── Envelope rejection ────────────────────────────────────────────────────── + +/// Unknown names must not resolve, and neither must the *mechanism* names this +/// contract deliberately refuses to expose: an interface that can sign arbitrary +/// bytes is a signing oracle. +#[test] +fn only_declared_action_names_resolve() { + for action in Action::ALL { + assert_eq!(Action::parse(action.as_str()).unwrap(), action); + } + for rejected in [ + "channel.write", + "agents.exfiltrate", + "", + "channel.read ", + "sign", + "sign_event", + "publish", + "nip44.encrypt", + "nip44.decrypt", + "nip42.auth", + "nip98.auth", + "keys.export", + "identity.nsec", + "presence.set", + "typing.set", + ] { + assert!( + Action::parse(rejected).is_err(), + "\"{rejected}\" must not parse as an action" + ); + } + + let json = serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "actionVersion": 1, + "action": "agents.exfiltrate", + "args": {}, + }); + assert!(serde_json::from_value::(json).is_err()); +} + +#[test] +fn envelope_metadata_must_match_this_protocol_version() { + let args = || { + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(pubkey()), + }) + }; + let failed = || BrokerResult::failed(BrokerError::unsupported("no")); + + for bad in [0_u16, 2, 999] { + let mut request = BrokerRequest::new("req-1", args()).unwrap(); + request.protocol_version = bad; + let error = request.validated().unwrap_err().to_string(); + assert!(error.contains("protocolVersion"), "unexpected: {error}"); + + let mut response = BrokerResponse::new("req-1", failed()); + response.protocol_version = bad; + assert!(response.validate().is_err()); + } + + let mut wrong_action_version = BrokerRequest::new("req-1", args()).unwrap(); + wrong_action_version.action_version = 7; + let error = wrong_action_version.validated().unwrap_err().to_string(); + assert!(error.contains("actionVersion"), "unexpected: {error}"); + + let mut wrong_request_type = BrokerRequest::new("req-1", args()).unwrap(); + wrong_request_type.r#type = BROKER_RESULT_TYPE.into(); + assert!(wrong_request_type.validated().is_err()); + + let mut wrong_response_type = BrokerResponse::new("req-1", failed()); + wrong_response_type.r#type = BROKER_REQUEST_TYPE.into(); + assert!(wrong_response_type.validate().is_err()); +} + +#[test] +fn request_id_must_be_present_bounded_and_printable() { + let args = || { + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(pubkey()), + }) + }; + for (id, valid) in [ + ("", false), + ("has space", false), + ("has\nnewline", false), + ("has\u{7f}del", false), + ("req/1-a.b:c", true), + ] { + assert_eq!( + BrokerRequest::new(id, args()).is_ok(), + valid, + "requestId {id:?} validity" + ); + } + assert!(BrokerRequest::new("a".repeat(MAX_REQUEST_ID_LEN), args()).is_ok()); + assert!(BrokerRequest::new("a".repeat(MAX_REQUEST_ID_LEN + 1), args()).is_err()); +} + +/// Duplicate object keys are rejected everywhere the envelope reads, including +/// inside the `outcome` object. +/// +/// serde's derived readers reject a repeated field, so most of this contract got +/// duplicate rejection for free. `outcome` did not: the strict intermediary held +/// it as a `serde_json::Value` before re-deserializing it under its action tag, +/// and buffering through `Value` silently collapses duplicates last-wins. That +/// made `outcome` the one place where a reader could see a value the envelope's +/// own strictness never vetted — so it now re-parses the original bytes via +/// `RawValue`. +/// +/// Each case asserts the de-duplicated form parses first, so a rejection cannot +/// be a rejection of the surrounding fixture. +#[test] +fn a_duplicate_object_key_is_rejected_at_every_depth() { + let outcome = format!(r#"{{"agentPubkey":"{PUBKEY}","displayName":"n"}}"#); + let response = |body: &str| { + format!( + r#"{{"type":"{BROKER_RESULT_TYPE}","protocolVersion":1,"requestId":"r","status":"succeeded","action":"agents.delete","outcome":{body}}}"# + ) + }; + + serde_json::from_str::(&response(&outcome)) + .expect("the de-duplicated response parses"); + let cases = [ + ( + "inside the outcome object", + response(&format!( + r#"{{"agentPubkey":"{PUBKEY}","displayName":"first","displayName":"second"}}"# + )), + ), + ( + "a top-level envelope member", + format!( + r#"{{"type":"{BROKER_RESULT_TYPE}","protocolVersion":1,"requestId":"r","requestId":"evil","status":"succeeded","action":"agents.delete","outcome":{outcome}}}"# + ), + ), + ( + "a flattened member", + format!( + r#"{{"type":"{BROKER_RESULT_TYPE}","protocolVersion":1,"requestId":"r","status":"succeeded","action":"agents.delete","action":"agents.update","outcome":{outcome}}}"# + ), + ), + ( + "inside a typed error payload", + format!( + r#"{{"type":"{BROKER_RESULT_TYPE}","protocolVersion":1,"requestId":"r","status":"failed","error":{{"code":"unauthorized","message":"a","message":"b"}}}}"# + ), + ), + ]; + for (where_, json) in cases { + assert!( + serde_json::from_str::(&json).is_err(), + "a duplicate key {where_} must not deserialize" + ); + } + + // The request envelope too, where `args` is typed rather than buffered. + let request = |args: &str| { + format!( + r#"{{"type":"{BROKER_REQUEST_TYPE}","protocolVersion":1,"requestId":"r","actionVersion":1,"action":"agents.delete","args":{args}}}"# + ) + }; + serde_json::from_str::(&request(r#"{"target":{"name":"good"}}"#)) + .expect("the de-duplicated request parses"); + assert!( + serde_json::from_str::(&request( + r#"{"target":{"name":"good"},"target":{"name":"evil"}}"# + )) + .is_err(), + "a duplicate key inside args must not deserialize" + ); +} + +// ── Wire schemas: the enforceable no-secret invariant ─────────────────────── + +/// The exact wire key set of every args and outcome type, with every optional +/// field populated so nothing escapes the pin by being absent — plus the two +/// envelopes, whose own key sets are now equally enforceable. +/// +/// This table *is* the no-secret invariant. Combined with +/// `deny_unknown_fields`, it means no field — secret-bearing or otherwise — can +/// be added to this contract without a reviewer changing a line here. The +/// `agents.create` outcome is the case that matters: public identity only, never +/// the key the host just minted. +/// +/// The envelopes are here because a key set nobody pins is a key set a field can +/// be added to. The response envelope in particular admits a *different* exact +/// set per status, which is what its strict deserializer enforces. +#[test] +fn every_payload_has_an_exact_and_secret_free_wire_schema() { + let signer = Keys::generate(); + let expected: Vec<(&str, Vec<&str>)> = vec![ + // Envelopes. Every optional member present, so the pin covers the + // widest shape each may take. + ( + "request/envelope", + vec![ + "action", + "actionVersion", + "args", + "protocolVersion", + "requestId", + "type", + ], + ), + ( + "response/envelope/succeeded", + vec![ + "action", + "outcome", + "protocolVersion", + "replayed", + "requestId", + "status", + "type", + ], + ), + ( + "response/envelope/failed", + vec![ + "error", + "protocolVersion", + "replayed", + "requestId", + "status", + "type", + ], + ), + ( + "response/envelope/indeterminate", + vec![ + "error", + "protocolVersion", + "replayed", + "requestId", + "status", + "type", + ], + ), + ("error", vec!["code", "message"]), + // Args, fully populated (optional fields present). + ( + "channel.read/args", + vec![ + "channelId", + "cursor", + "limit", + "mentionsOnly", + "rootEventId", + ], + ), + ( + "message.post/args", + vec!["channelId", "content", "mentions"], + ), + ( + "message.reply/args", + vec!["channelId", "content", "mentions", "replyToEventId"], + ), + ( + "reaction.add/args", + vec!["channelId", "reaction", "targetEventId"], + ), + ("profile.set/args", vec!["about", "displayName", "picture"]), + ("storage.address/args", vec!["slug"]), + ( + "agents.create/args", + vec![ + "channelId", + "displayName", + "model", + "provider", + "respondTo", + "runtime", + "systemPrompt", + ], + ), + ( + "agents.update/args", + vec![ + "displayName", + "model", + "provider", + "respondTo", + "runtime", + "systemPrompt", + "target", + ], + ), + ("agents.delete/args", vec!["target"]), + // Outcomes. + ("channel.read/outcome", vec!["messages", "nextCursor"]), + ("message.post/outcome", vec!["createdAt", "eventId", "kind"]), + ( + "message.reply/outcome", + vec!["createdAt", "eventId", "kind"], + ), + ("reaction.add/outcome", vec!["createdAt", "eventId", "kind"]), + ("profile.set/outcome", vec!["createdAt", "eventId", "kind"]), + ( + "storage.address/outcome", + vec!["authorPubkey", "dTag", "kind"], + ), + ( + "agents.create/outcome", + vec!["agentPubkey", "channelId", "displayName"], + ), + ( + "agents.update/outcome", + vec!["agentPubkey", "displayName", "updatedFields"], + ), + ("agents.delete/outcome", vec!["agentPubkey", "displayName"]), + ]; + + let mut actual: Vec<(String, Vec)> = Vec::new(); + // Envelopes first, in the same order as the table above. `replayed` is set + // so the widest shape is what gets pinned. + let request = BrokerRequest::new( + "req-1", + ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL)), + ) + .expect("envelope fixture builds"); + actual.push(( + "request/envelope".to_string(), + keys_of(&serde_json::to_value(&request).expect("request serializes")), + )); + for (name, result) in [ + ( + "succeeded", + BrokerResult::succeeded(ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: pubkey(), + display_name: "Gone".into(), + })), + ), + ( + "failed", + BrokerResult::failed(BrokerError::new(BrokerErrorCode::ActionFailed, "no")), + ), + ( + "indeterminate", + BrokerResult::indeterminate(BrokerError::new(BrokerErrorCode::OutcomeUnknown, "?")), + ), + ] { + let response = BrokerResponse::new("req-1", result).replayed(); + actual.push(( + format!("response/envelope/{name}"), + keys_of(&serde_json::to_value(&response).expect("response serializes")), + )); + } + actual.push(( + "error".to_string(), + keys_of( + &serde_json::to_value(BrokerError::new(BrokerErrorCode::Internal, "?")) + .expect("error serializes"), + ), + )); + for args in action_fixtures() { + let json = serde_json::to_value(&args).expect("args serialize"); + actual.push(( + format!("{}/args", args.action().as_str()), + keys_of(&json["args"]), + )); + } + for outcome in outcome_fixtures(&signer) { + let json = serde_json::to_value(&outcome).expect("outcome serializes"); + actual.push(( + format!("{}/outcome", outcome.action().as_str()), + keys_of(&json["outcome"]), + )); + } + + let expected: Vec<(String, Vec)> = expected + .into_iter() + .map(|(name, keys)| { + ( + name.to_string(), + keys.into_iter().map(str::to_string).collect(), + ) + }) + .collect(); + assert_eq!( + actual, expected, + "a payload's wire keys changed — confirm no field can carry key material" + ); + + // And no key anywhere in the contract even *looks* like secret material. + for (name, keys) in &actual { + for key in keys { + let lower = key.to_ascii_lowercase(); + for forbidden in ["secret", "private", "nsec", "seckey", "credential", "token"] { + assert!( + !lower.contains(forbidden), + "{name} exposes \"{key}\", which reads as secret material" + ); + } + } + } +} + +/// The envelope must not carry requester, owner, or scope: those are derived by +/// the host from the credential. A body that could name its own subject would +/// let any caller act as anyone — the same reason `agents.create` has no owner. +#[test] +fn no_payload_can_name_its_own_authority() { + let request = serde_json::to_value( + BrokerRequest::new( + "req-1", + ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL)), + ) + .unwrap(), + ) + .unwrap(); + assert_eq!( + keys_of(&request), + vec![ + "action", + "actionVersion", + "args", + "protocolVersion", + "requestId", + "type" + ] + ); + + // Authority-naming fields are rejected, not ignored, wherever they appear. + for rejected in [ + serde_json::json!({ + "channelId": CHANNEL, "displayName": "A", "systemPrompt": "B", + "ownerPubkey": PUBKEY, + }), + serde_json::json!({ + "channelId": CHANNEL, "displayName": "A", "systemPrompt": "B", + "onBehalfOf": PUBKEY, + }), + serde_json::json!({ + "channelId": CHANNEL, "displayName": "A", "systemPrompt": "B", + "envVars": { "ANTHROPIC_API_KEY": "sk-live" }, + }), + serde_json::json!({ + "channelId": CHANNEL, "displayName": "A", "systemPrompt": "B", + "secretKey": "nsec1deadbeef", + }), + ] { + assert!( + serde_json::from_value::(rejected.clone()).is_err(), + "must reject: {rejected}" + ); + } + + // A read cannot ask about someone else's mentions, and a profile write + // cannot name a subject. + assert!(serde_json::from_value::( + serde_json::json!({ "channelId": CHANNEL, "mentionsOf": PUBKEY }) + ) + .is_err()); + assert!(serde_json::from_value::( + serde_json::json!({ "displayName": "A", "pubkey": PUBKEY }) + ) + .is_err()); + + // An outcome cannot smuggle a minted secret past the schema either. + for extra in ["nsec", "secretKey", "seckey", "credential"] { + let mut outcome = serde_json::json!({ + "agentPubkey": PUBKEY, "displayName": "A", "channelId": CHANNEL, + }); + outcome[extra] = serde_json::json!("nsec1deadbeef"); + let json = serde_json::json!({ "action": "agents.create", "outcome": outcome }); + assert!( + serde_json::from_value::(json).is_err(), + "an outcome carrying \"{extra}\" must not deserialize" + ); + } +} + +/// The nested action enums are strict about their own key set, not just about +/// the payload inside it. +/// +/// `ActionArgs`/`ActionOutcome` are adjacently tagged, so their wire form is the +/// two-key object `{action, args}` / `{action, outcome}`. Without +/// `deny_unknown_fields` on the enum itself, a *sibling* of those two keys is +/// silently ignored — and these types are public and wire-facing, so a host +/// author can deserialize one directly rather than through the envelope. The +/// envelope's own strictness does not cover that door. +#[test] +fn a_nested_action_object_rejects_siblings_of_its_two_keys() { + // The valid two-key forms must pass untouched, so a rejection below cannot + // be a rejection of the fixture itself. + let args = serde_json::json!({ + "action": "agents.delete", "args": { "target": { "name": "helper" } }, + }); + let outcome = serde_json::json!({ + "action": "agents.delete", + "outcome": { "agentPubkey": PUBKEY, "displayName": "Gone" }, + }); + serde_json::from_value::(args.clone()).expect("the exact args shape deserializes"); + serde_json::from_value::(outcome.clone()) + .expect("the exact outcome shape deserializes"); + + for extra in ["secretKey", "nsec", "outcome", "unexpected"] { + let mut probe = args.clone(); + probe[extra] = serde_json::json!("x"); + assert!( + serde_json::from_value::(probe).is_err(), + "ActionArgs must reject the sibling key \"{extra}\"" + ); + } + for extra in ["secretKey", "nsec", "args", "unexpected"] { + let mut probe = outcome.clone(); + probe[extra] = serde_json::json!("x"); + assert!( + serde_json::from_value::(probe).is_err(), + "ActionOutcome must reject the sibling key \"{extra}\"" + ); + } +} + +#[test] +fn pubkey_hex_rejects_anything_but_a_public_key() { + assert!(PubkeyHex::parse("nothex").is_err()); + assert!(PubkeyHex::parse(&PUBKEY[..40]).is_err()); + assert!(PubkeyHex::parse(format!("{PUBKEY}00")).is_err()); + assert!(PubkeyHex::parse("nsec1deadbeef").is_err()); + // Normalizes case, so two spellings of one key cannot look like two keys. + assert_eq!( + PubkeyHex::parse(PUBKEY.to_ascii_uppercase()).unwrap(), + pubkey() + ); + // And it enforces that through serde, not only through the constructor. + assert!(serde_json::from_value::(serde_json::json!("nothex")).is_err()); +} + +/// 64 hex characters is a *shape*; a public key is a point on secp256k1. Most +/// 32-byte values are not one, so accepting shape alone let this type's name +/// promise something it never checked, and deferred the first real rejection to +/// whichever consumer eventually converted the string to a key — by which point +/// the request had already been accepted. +/// +/// The fixtures are ordered the way the type is used: the real key must pass +/// untouched first, so a rejection below is about the curve check and not about a +/// probe that would have failed for any input. +#[test] +fn a_pubkey_must_be_a_point_on_the_curve_not_merely_hex() { + /// The check this type now delegates to, spelled out independently: a value + /// is a key only if it converts to an x-only key, which is what `xonly` + /// does. `from_hex` alone is a hex decode and answers nothing — asking only + /// it is how the gap survived a `nostr`-backed check in the first place. + fn is_a_point(hex: &str) -> bool { + nostr::PublicKey::from_hex(hex) + .and_then(|key| key.xonly().map(|_| ())) + .is_ok() + } + + // A real key passes, in both spellings, and is unchanged by the new check. + assert!(is_a_point(PUBKEY), "fixture must be a real point"); + assert_eq!( + PubkeyHex::parse(PUBKEY).expect("a real key parses"), + pubkey() + ); + assert_eq!( + PubkeyHex::parse(PUBKEY.to_ascii_uppercase()).expect("case is still normalized"), + pubkey() + ); + + // Well-formed hex that is not on the curve. Each is asserted to be a + // non-point first, so the rejection cannot be for an unrelated reason. + // + // The last fixture is the important one: `x = 5` is a perfectly in-range + // field element, so it is not rejected for overflowing the field the way the + // first three are — there is simply no y with y² = x³ + 7. A check that + // only bounds the value against the field prime would accept it, so this is + // what pins the test to a real curve check rather than a range check. + for junk in [ + "f".repeat(64), + "0".repeat(64), + // The field prime p itself: out of range by exactly one. + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f".into(), + format!("{:0>64}", 5), + ] { + assert!( + !is_a_point(&junk), + "fixture \"{junk}\" must not be a valid point" + ); + let error = PubkeyHex::parse(&junk) + .expect_err("64 hex characters that are not a point must be rejected") + .to_string(); + assert!( + error.contains("x-only"), + "rejection must name the curve, not the shape: {error}" + ); + // The serde door takes the same check: `PubkeyHex` deserializes through + // `parse`, so a host cannot ship a non-point where a constructor would + // have refused one. + assert!( + serde_json::from_value::(serde_json::json!(junk)).is_err(), + "the wire door must reject the non-point \"{junk}\" too" + ); + // And through a payload, since that is the shape a host actually sends. + // The honest form parses, so this rejects for the key and not the shape. + let target = |value: &str| { + serde_json::json!({ + "action": "agents.delete", + "args": { "target": { "pubkey": value } }, + }) + }; + serde_json::from_value::(target(PUBKEY)) + .expect("a real key in a target payload must parse"); + assert!( + serde_json::from_value::(target(&junk)).is_err(), + "an agents.delete target must reject the non-point \"{junk}\"" + ); + } +} + +// ── Argument validation ───────────────────────────────────────────────────── + +/// Boundaries of every shared validator, in one table. +#[test] +fn validators_accept_and_reject_at_their_boundaries() { + let read = |mutate: fn(&mut ChannelReadArgs)| { + let mut args = ChannelReadArgs::channel(CHANNEL); + mutate(&mut args); + args.validated().is_ok() + }; + let post = |content: String, mentions: Vec| { + MessagePostArgs { + channel_id: CHANNEL.into(), + content, + mentions, + } + .validated() + }; + let react = |reaction: String| { + ReactionAddArgs { + channel_id: CHANNEL.into(), + target_event_id: EVENT.into(), + reaction, + } + .validated() + }; + let slug = |slug: &str| StorageAddressArgs { slug: slug.into() }.validated().is_ok(); + + // Channel UUID, thread id, limit, and opaque cursor. + assert!(ChannelReadArgs::channel("not-a-uuid").validated().is_err()); + assert!(!read(|a| a.root_event_id = Some("nothex".into()))); + assert!(read(|a| a.root_event_id = Some(EVENT.into()))); + assert!(!read(|a| a.limit = Some(0))); + assert!(read(|a| a.limit = Some(actions::MAX_PAGE_LIMIT))); + assert!(!read(|a| a.limit = Some(actions::MAX_PAGE_LIMIT + 1))); + assert!(!read(|a| a.cursor = Some(String::new()))); + assert!(!read(|a| a.cursor = Some("has space".into()))); + assert!(read( + |a| a.cursor = Some("a".repeat(actions::MAX_CURSOR_LEN)) + )); + assert!(!read( + |a| a.cursor = Some("a".repeat(actions::MAX_CURSOR_LEN + 1)) + )); + + // Content, mentions, reaction payload. + assert!(post(" ".into(), vec![]).is_err()); + assert!(matches!( + post("x".repeat(actions::MAX_CONTENT_BYTES + 1), vec![]).unwrap_err(), + SdkError::ContentTooLarge { .. } + )); + assert!(post("hi".into(), vec![pubkey(); actions::MAX_MENTIONS]).is_ok()); + assert!(matches!( + post("hi".into(), vec![pubkey(); actions::MAX_MENTIONS + 1]).unwrap_err(), + SdkError::TooManyMentions + )); + assert!(react(" ".into()).is_err()); + assert!(react(":shipit:".into()).is_ok()); + assert!(matches!( + react("a".repeat(actions::MAX_EMOJI_CHARS + 1)).unwrap_err(), + SdkError::EmojiTooLong + )); + + // NIP-AE slug grammar for encrypted-memory addressing. + assert!(slug("core")); + assert!(slug("mem/broker-foundation")); + assert!(!slug("")); + assert!(!slug("Core")); + assert!(!slug("secrets")); + assert!(!slug("mem/Bad Slug")); + + // Patch-shaped writes must change something, and reject unknown modes. + let profile_error = ProfileSetArgs { + display_name: None, + about: None, + picture: None, + } + .validated() + .unwrap_err() + .to_string(); + assert!(profile_error.contains("at least one"), "{profile_error}"); + let update = |respond_to: Option<&str>, name: Option<&str>| { + AgentsUpdateArgs { + target: AgentTarget::Pubkey(pubkey()), + display_name: name.map(str::to_string), + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: respond_to.map(str::to_string), + } + .validated() + }; + assert!(update(None, None) + .unwrap_err() + .to_string() + .contains("at least one field")); + assert!(update(Some("anyone"), None).is_ok()); + assert!(update(Some("allowlist"), Some("A")).is_err()); + assert!(AgentsDeleteArgs { + target: AgentTarget::Name(" ".into()), + } + .validated() + .is_err()); +} + +/// Validation must be **inseparable from normalization**: there must be no way +/// to learn that a request is valid while still holding the un-normalized value. +/// +/// The bug this pins: `validate(&self)` called the arguments' `validated()`, +/// which *computes* a normalized copy, then dropped the copy and returned +/// `Ok(())`. A hand-built request targeting `" helper "` therefore passed +/// validation and still carried the padding, so a host that trusted the verdict +/// and executed the struct looked up a name the validator never approved. +/// `prepare()` was safe, but a host cannot force its callers through the client's +/// outgoing path. +/// +/// The fix is typed, so this test asserts the *shape* of the API and not just one +/// call's behaviour: the only route to a verdict is `validated()`, which consumes +/// the request and hands back a `ValidatedRequest` whose arguments are already +/// normalized. The un-normalized value is gone rather than sitting beside its +/// approved copy. That the old method no longer exists is enforced at compile +/// time by every other caller in this file having had to change. +#[test] +fn a_request_cannot_be_validated_without_being_normalized() { + let padded = || { + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name(" helper ".into()), + }) + }; + let trimmed = ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name("helper".into()), + }); + + // A request built by hand, bypassing `new` — the shape the reviewer used. + let hand_built = BrokerRequest { + r#type: BROKER_REQUEST_TYPE.into(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id: "req-trap".into(), + action_version: 1, + action: padded(), + }; + let validated = hand_built + .validated() + .expect("a padded name is valid, just not canonical"); + + // The verdict and the normalized value are the same object, so an executor + // holding the verdict cannot be holding the padding. + assert_eq!( + validated.args(), + &trimmed, + "a validated request must carry the normalized arguments" + ); + assert_eq!(validated.action(), Action::AgentsDelete); + assert_eq!(validated.request_id(), "req-trap"); + + // Freezing from the verdict carries the normalized value onto the wire. + let body = String::from_utf8( + validated + .clone() + .prepare() + .expect("prepares") + .body() + .to_vec(), + ) + .expect("body is utf8"); + assert!( + body.contains(r#""name":"helper""#) && !body.contains(" helper "), + "frozen body still carries the unnormalized name: {body}" + ); + + // Moving the envelope onward yields the normalized request, not the input. + assert_eq!(validated.into_request().action, trimmed); + + // The envelope is still checked, so `validated` is not merely a normalizer: + // an invalid envelope produces no verdict at all. + let bad_version = BrokerRequest { + r#type: BROKER_REQUEST_TYPE.into(), + protocol_version: 99, + request_id: "req-trap".into(), + action_version: 1, + action: padded(), + }; + assert!(bad_version.validated().is_err()); + + // And arguments that cannot be normalized are rejected rather than + // normalized to something the caller did not ask for. + let empty_name = BrokerRequest { + r#type: BROKER_REQUEST_TYPE.into(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id: "req-trap".into(), + action_version: 1, + action: ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name(" ".into()), + }), + }; + assert!(empty_name.validated().is_err()); +} + +/// Validation normalizes, so the frozen body must carry the normalized value — +/// not the caller's. Otherwise a padded selector passes validation and the host +/// executes something the validator never approved: it looks up `" helper "`, +/// or publishes a padded reaction. +/// +/// Both construction paths are checked, because `BrokerRequest`'s fields are +/// public and it is `Deserialize`, so `prepare` is reachable without ever going +/// through `new`. +#[test] +fn the_frozen_body_carries_exactly_what_validation_approved() { + // Path 1: through `new`, which stores the normalized action. + let request = BrokerRequest::new( + "req-normalize", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name(" helper ".into()), + }), + ) + .expect("a padded name is valid, just not canonical"); + assert_eq!( + request.action, + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name("helper".into()), + }), + "`new` must store the normalized copy" + ); + let body = String::from_utf8(request.prepare().expect("prepares").body().to_vec()) + .expect("body is utf8"); + assert!( + body.contains(r#""name":"helper""#) && !body.contains(" helper "), + "frozen body still carries the unnormalized name: {body}" + ); + + // Path 2: a struct literal that bypasses `new` entirely. + let bypassed = BrokerRequest { + r#type: BROKER_REQUEST_TYPE.to_string(), + protocol_version: BROKER_PROTOCOL_VERSION, + request_id: "req-bypass".into(), + action_version: 1, + action: ActionArgs::ReactionAdd(ReactionAddArgs { + channel_id: CHANNEL.into(), + target_event_id: EVENT.into(), + reaction: " \u{1f41d} ".into(), + }), + }; + let body = String::from_utf8(bypassed.prepare().expect("prepares").body().to_vec()) + .expect("body is utf8"); + assert!( + body.contains("\"reaction\":\"\u{1f41d}\""), + "frozen body did not normalize a padded reaction: {body}" + ); + + // Normalization is idempotent, so a second freeze is byte-identical: the + // retry contract still holds through the new path. + let once = BrokerRequest::new( + "req-idem", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name(" helper ".into()), + }), + ) + .unwrap() + .prepare() + .unwrap(); + let twice = BrokerRequest::new( + "req-idem", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name("helper".into()), + }), + ) + .unwrap() + .prepare() + .unwrap(); + assert_eq!( + once.body(), + twice.body(), + "a padded and a pre-trimmed request must freeze to the same bytes" + ); +} + +/// Correlation must reject an outcome that echoes a different identity than the +/// request supplied — `requestId` plus action is not enough, because a host +/// routing bug can return a well-formed success for the wrong subject. +/// +/// Table-driven over every request/outcome identity pair, so the enumeration in +/// `correlate_identities`' doc table is pinned by a test rather than asserted in +/// prose. Each case builds the *matching* response first and requires it to pass, +/// so a case cannot "reject" for an unrelated reason. +#[test] +fn correlation_rejects_an_outcome_naming_a_different_subject() { + let requested = pubkey(); + let other = + PubkeyHex::parse("b02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971") + .expect("valid hex"); + let other_channel = "c2c38ca8-9ec3-411e-bab5-f9deab34d52e"; + + // (action, matching outcome, mismatched outcome or None when nothing is + // comparable). A `None` documents an inherent gap, not an oversight. + let cases: Vec<(&str, ActionArgs, ActionOutcome, Option)> = vec![ + ( + "agents.create echoes channelId", + ActionArgs::AgentsCreate(AgentsCreateArgs { + channel_id: CHANNEL.into(), + display_name: "Helper".into(), + system_prompt: "be useful".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ActionOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: requested.clone(), + display_name: "Helper".into(), + channel_id: CHANNEL.into(), + }), + Some(ActionOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: requested.clone(), + display_name: "Helper".into(), + channel_id: other_channel.into(), + })), + ), + ( + "agents.update targeted by pubkey echoes agentPubkey", + ActionArgs::AgentsUpdate(AgentsUpdateArgs { + target: AgentTarget::Pubkey(requested.clone()), + display_name: Some("Renamed".into()), + system_prompt: None, + runtime: None, + provider: None, + model: None, + respond_to: None, + }), + ActionOutcome::AgentsUpdate(AgentsUpdateOutcome { + agent_pubkey: requested.clone(), + display_name: "Renamed".into(), + updated_fields: vec!["displayName".into()], + }), + Some(ActionOutcome::AgentsUpdate(AgentsUpdateOutcome { + agent_pubkey: other.clone(), + display_name: "Renamed".into(), + updated_fields: vec!["displayName".into()], + })), + ), + ( + "agents.delete targeted by pubkey echoes agentPubkey", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(requested.clone()), + }), + ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: requested.clone(), + display_name: "Gone".into(), + }), + Some(ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: other.clone(), + display_name: "Gone".into(), + })), + ), + ( + // Inherent gap: the host resolves the name, and the rename may be + // exactly what this call performed, so no pubkey is comparable. + "agents.delete targeted by name compares nothing", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Name("helper".into()), + }), + ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: other.clone(), + display_name: "helper".into(), + }), + None, + ), + ( + // Host-minted identifiers only; nothing the request supplied is echoed. + "message.post echoes no requested identity", + ActionArgs::MessagePost(MessagePostArgs { + channel_id: CHANNEL.into(), + content: "hi".into(), + mentions: vec![], + }), + ActionOutcome::MessagePost(EventPublished { + event_id: EVENT.into(), + kind: 9, + created_at: 1, + }), + None, + ), + ]; + + for (label, args, matching, mismatched) in cases { + let request = BrokerRequest::new("req-correlate", args) + .expect("fixture args validate") + .prepare() + .expect("fixture prepares"); + BrokerResponse::new("req-correlate", BrokerResult::succeeded(matching)) + .validate_for(&request) + .unwrap_or_else(|e| panic!("{label}: the matching outcome must pass, got {e}")); + if let Some(mismatched) = mismatched { + let err = BrokerResponse::new("req-correlate", BrokerResult::succeeded(mismatched)) + .validate_for(&request) + .expect_err(&format!("{label}: a mismatched identity must be rejected")); + assert!( + matches!(err, SdkError::InvalidInput(_)), + "{label}: expected InvalidInput, got {err:?}" + ); + } + } +} + +// ── Identities have one spelling ──────────────────────────────────────────── + +/// Every legal spelling of a channel UUID names one channel, so a request and a +/// response that spell it differently must still correlate. +/// +/// The bug: `Uuid::parse_str` accepts uppercase, unhyphenated, braced, and +/// `urn:uuid:` forms, `channel()` returned the caller's spelling untouched, and +/// correlation compared bytes — so an uppercase request against a host's +/// canonical lowercase echo of the *same* channel failed `validate_for`. That is +/// worse than the mismatch the check exists to catch: it makes a correct host +/// unusable. +/// +/// Two independent guards close it, and each is asserted separately below so +/// neither can be the only thing holding: canonicalize where a value enters, and +/// compare parsed identities rather than bytes. +#[test] +fn one_identity_spelled_two_ways_still_correlates() { + let spellings = [ + CHANNEL.to_ascii_uppercase(), + CHANNEL.replace('-', ""), + format!("{{{CHANNEL}}}"), + format!("urn:uuid:{CHANNEL}"), + CHANNEL.to_string(), + ]; + + let create = |channel_id: &str| { + ActionArgs::AgentsCreate(AgentsCreateArgs { + channel_id: channel_id.into(), + display_name: "Helper".into(), + system_prompt: "be useful".into(), + runtime: None, + provider: None, + model: None, + respond_to: None, + }) + }; + let echo = |channel_id: &str| { + BrokerResult::succeeded(ActionOutcome::AgentsCreate(AgentsCreateOutcome { + agent_pubkey: pubkey(), + display_name: "Helper".into(), + channel_id: channel_id.into(), + })) + }; + + for spelling in &spellings { + // Guard 1: the frozen body carries the canonical spelling, not the + // caller's, so what the host receives is what correlation will compare. + let request = BrokerRequest::new("req-spelling", create(spelling)) + .expect("every legal UUID spelling validates"); + let body = String::from_utf8(request.prepare().expect("prepares").body().to_vec()) + .expect("body is utf8"); + assert!( + body.contains(&format!("\"channelId\":\"{CHANNEL}\"")), + "frozen body did not canonicalize \"{spelling}\": {body}" + ); + + // And through the wire door too, which no `validated()` covers: a parsed + // request reaches a caller canonical. + let parsed: BrokerRequest = serde_json::from_value(serde_json::json!({ + "type": BROKER_REQUEST_TYPE, + "protocolVersion": 1, + "requestId": "req-spelling", + "actionVersion": 1, + "action": "channel.read", + "args": { "channelId": spelling }, + })) + .unwrap_or_else(|e| panic!("\"{spelling}\" must parse: {e}")); + assert_eq!( + parsed.action, + ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL)), + "the wire door did not canonicalize \"{spelling}\"" + ); + + // Guard 2: correlation compares parsed identities, so every spelling on + // either side correlates even if guard 1 were absent. + let prepared = BrokerRequest::new("req-spelling", create(spelling)) + .expect("validates") + .prepare() + .expect("prepares"); + for returned in &spellings { + BrokerResponse::new("req-spelling", echo(returned)) + .validate_for(&prepared) + .unwrap_or_else(|e| { + panic!("request \"{spelling}\" vs echo \"{returned}\" must correlate: {e}") + }); + } + } + + // A genuinely different channel is still rejected, so the fix widened what + // counts as equal without weakening the check. + let prepared = BrokerRequest::new("req-spelling", create(CHANNEL)) + .expect("validates") + .prepare() + .expect("prepares"); + let err = BrokerResponse::new("req-spelling", echo("c2c38ca8-9ec3-411e-bab5-f9deab34d52e")) + .validate_for(&prepared) + .expect_err("a different channel must still be rejected"); + assert!(matches!(err, SdkError::InvalidInput(_)), "{err:?}"); +} + +/// The same treatment for the contract's other multi-spelling identities: hex. +/// +/// A pubkey was already canonicalized by `PubkeyHex::parse`, which is also its +/// serde path — this pins that it is, so the `agentPubkey` rows of the +/// correlation table cannot regress into a byte comparison of two cases. Event +/// ids and `d` tags are plain `String`s and were *not* normalized on the wire, +/// only in `validated()`, so those are the ones this changes. +#[test] +fn hex_identities_are_canonical_through_every_door() { + // Pubkey: mixed-case target vs lowercase echo correlates, both directions. + let upper = PubkeyHex::parse(PUBKEY.to_ascii_uppercase()).expect("valid hex"); + let request = BrokerRequest::new( + "req-hex", + ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(upper), + }), + ) + .expect("validates") + .prepare() + .expect("prepares"); + BrokerResponse::new( + "req-hex", + BrokerResult::succeeded(ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: pubkey(), + display_name: "Gone".into(), + })), + ) + .validate_for(&request) + .expect("two cases of one pubkey are one identity"); + + // Event ids and d tags: the wire door lowercases, so a parsed value equals a + // constructed one and neither carries the sender's case. + let parsed: ActionArgs = serde_json::from_value(serde_json::json!({ + "action": "reaction.add", + "args": { + "channelId": CHANNEL, + "targetEventId": EVENT.to_ascii_uppercase(), + "reaction": "\u{1f41d}", + }, + })) + .expect("an uppercase event id parses"); + assert_eq!( + parsed, + ActionArgs::ReactionAdd(ReactionAddArgs { + channel_id: CHANNEL.into(), + target_event_id: EVENT.into(), + reaction: "\u{1f41d}".into(), + }), + "the wire door did not lowercase targetEventId" + ); + + let parsed: ActionOutcome = serde_json::from_value(serde_json::json!({ + "action": "storage.address", + "outcome": { + "authorPubkey": PUBKEY.to_ascii_uppercase(), + "kind": 30078, + "dTag": EVENT.to_ascii_uppercase(), + }, + })) + .expect("an uppercase d tag parses"); + assert_eq!( + parsed, + ActionOutcome::StorageAddress(StorageAddress { + author_pubkey: pubkey(), + kind: 30078, + d_tag: EVENT.into(), + }), + "the wire door did not lowercase dTag or authorPubkey" + ); + + // The optional identity member takes the same door, and still rejects null. + let read: ActionArgs = serde_json::from_value(serde_json::json!({ + "action": "channel.read", + "args": { "channelId": CHANNEL, "rootEventId": EVENT.to_ascii_uppercase() }, + })) + .expect("an uppercase root event id parses"); + assert_eq!( + read, + ActionArgs::ChannelRead(ChannelReadArgs { + channel_id: CHANNEL.into(), + root_event_id: Some(EVENT.into()), + ..ChannelReadArgs::default() + }), + ); + assert!( + serde_json::from_value::(serde_json::json!({ + "action": "channel.read", + "args": { "channelId": CHANNEL, "rootEventId": serde_json::Value::Null }, + })) + .is_err(), + "canonicalizing must not have replaced the null guard" + ); + + // A malformed identity is still a parse failure, so the new doors reject + // rather than merely normalize. + for bad in ["nothex", "", &EVENT[..40], &format!("{EVENT}00")] { + assert!( + serde_json::from_value::(serde_json::json!({ + "action": "channel.read", + "args": { "channelId": CHANNEL, "rootEventId": bad }, + })) + .is_err(), + "rootEventId \"{bad}\" must not deserialize" + ); + } + assert!( + serde_json::from_value::(serde_json::json!({ + "action": "channel.read", + "args": { "channelId": "not-a-uuid" }, + })) + .is_err(), + "a non-UUID channelId must not deserialize" + ); +} + +/// `BrokerResult` must have **no wire door of its own**, so the strict envelope is +/// the only way to read a result. +/// +/// The bug: the exported result type derived its own reader, which accepted and +/// dropped arbitrary siblings — `status: failed` beside an `error` and a +/// `secretKey`, or a succeeded result beside an `error`. A consumer parsing the +/// result type directly therefore got an `Ok` value whose complete wire shape had +/// never been vetted, while the identical bytes failed through the envelope. +/// +/// Removing the door is checked at compile time, because a runtime test cannot +/// call a `Deserialize` impl that does not exist. `absence_of_a_reader` resolves to +/// the inherent function only when the bound holds, so this is a genuine negative +/// assertion rather than a comment. +#[test] +fn the_result_type_has_no_deserializer_of_its_own() { + struct Probe(std::marker::PhantomData); + + trait NoReader { + fn absence_of_a_reader() -> bool { + true + } + } + impl NoReader for Probe {} + + impl Probe { + fn absence_of_a_reader() -> bool { + false + } + } + + // The probe must be able to see a reader that *is* there, or its `true` + // means nothing. + assert!( + !Probe::::absence_of_a_reader(), + "probe is broken: it reports no reader for a type that has one" + ); + assert!( + !Probe::::absence_of_a_reader(), + "probe is broken: it reports no reader for a type that has one" + ); + assert!( + Probe::::absence_of_a_reader(), + "BrokerResult must not be Deserialize: it is a second, lax wire door" + ); + + // And the exact byte sequences the old direct reader accepted are rejected + // through the one door that remains. Each is the envelope form of what + // bugs-00 reported, since a bare result object is no longer parseable at all. + let envelope = |extra: serde_json::Value| { + let mut json = serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + }); + for (key, value) in extra.as_object().expect("object").clone() { + json[key] = value; + } + json + }; + let reported = [ + ( + "failed with an error and a secretKey", + serde_json::json!({ + "status": "failed", + "error": { "code": "action_failed", "message": "no" }, + "secretKey": "nsec1deadbeef", + }), + ), + ( + "succeeded beside an error", + serde_json::json!({ + "status": "succeeded", + "action": "agents.delete", + "outcome": { "agentPubkey": PUBKEY, "displayName": "Gone" }, + "error": { "code": "action_failed", "message": "no" }, + }), + ), + ]; + for (what, body) in reported { + let json = envelope(body); + assert!( + serde_json::from_value::(json.clone()).is_err(), + "{what} must not deserialize through the envelope either: {json}" + ); + } +} + +/// Derived coverage for the canonicalization rule, so a *newly added* identity +/// member is covered without anyone remembering to extend a list. +/// +/// The two tests above name the members that exist today. This one walks the real +/// fixtures — requests *and* responses, since both directions carry identities +/// through separate code — finds every member whose name marks it as an identity, +/// re-spells its value, and requires the payload to parse back to the canonical +/// value. A field added later with the wrong (or no) `deserialize_with` fails here. +/// +/// Matching on the member *name* is the point: the naming convention is what a +/// reviewer sees, so if a member is named like an identity it is held to the +/// identity rule. A member holding an identity under some other name would escape +/// this, which is why the audit above is by type as well. +/// +/// The suffix match is case-insensitive on purpose. An earlier revision matched +/// `"EventId"` exactly, which silently skipped the outcome member spelled +/// `eventId` and left every response-side door unpinned — a mutation removing +/// that door survived. Matching how a *reader* groups these names, rather than +/// how one of them happens to be capitalized, is what closes that gap. +#[test] +fn every_identity_shaped_member_is_canonicalized_on_the_wire() { + /// A member-name suffix and how a sender might legally re-spell its value. + type Respelling = (&'static str, fn(&str) -> String); + + // Every identity in this contract is hex or a UUID, so case is the + // re-spelling they all admit; `channelId` additionally admits the forms + // covered by `one_identity_spelled_two_ways_still_correlates`. + let respellings: [Respelling; 4] = [ + ("channelid", |v| v.to_ascii_uppercase()), + ("eventid", |v| v.to_ascii_uppercase()), + ("pubkey", |v| v.to_ascii_uppercase()), + ("dtag", |v| v.to_ascii_uppercase()), + ]; + + /// Re-spell every identity-named member of `valid` in turn and require the + /// payload to parse back to `original`. Returns how many members it checked. + fn respell_each(valid: &serde_json::Value, original: &T, respellings: &[Respelling]) -> usize + where + T: serde::de::DeserializeOwned + PartialEq + std::fmt::Debug, + { + let mut checked = 0; + let mut paths = Vec::new(); + member_paths(valid, "", &mut paths); + for path in paths { + let Some(name) = path.rsplit('/').next() else { + continue; + }; + let lowered = name.to_ascii_lowercase(); + let Some((_, respell)) = respellings + .iter() + .find(|(suffix, _)| lowered.ends_with(suffix)) + else { + continue; + }; + let Some(current) = valid + .pointer(&path) + .expect("path addresses a member") + .as_str() + else { + continue; + }; + let respelled = respell(current); + if respelled == current { + continue; + } + + let mut json = valid.clone(); + *json.pointer_mut(&path).expect("path addresses a member") = + serde_json::Value::String(respelled.clone()); + let parsed: T = serde_json::from_value(json) + .unwrap_or_else(|e| panic!("\"{respelled}\" at {path} must parse: {e}")); + assert_eq!( + &parsed, original, + "member {path} did not canonicalize \"{respelled}\" back to \"{current}\"" + ); + checked += 1; + } + checked + } + + let mut request_members = 0; + for args in action_fixtures() { + let request = BrokerRequest::new("req-canon", args).expect("fixture request builds"); + let valid = serde_json::to_value(&request).expect("request serializes"); + request_members += respell_each(&valid, &request, &respellings); + } + + // The response side carries identities too — `agents.create` echoes a + // `channelId`, `storage.address` a `dTag`, the publishing outcomes an + // `eventId` and an `authorPubkey` — and those doors are separate code from + // the request side's. + let keys = Keys::generate(); + let mut response_members = 0; + for outcome in outcome_fixtures(&keys) { + let response = BrokerResponse::new("req-canon", BrokerResult::succeeded(outcome)); + let valid = serde_json::to_value(&response).expect("response serializes"); + response_members += respell_each(&valid, &response, &respellings); + } + + // Guard the guard: a rule that silently matched nothing would pass forever. + // The two directions are floored *separately* on purpose — one combined + // total would be satisfied by the request side alone, which is exactly the + // blind spot that let a response-side door go unpinned. + assert!( + request_members >= 8, + "expected identity members across the request fixtures, checked {request_members}" + ); + assert!( + response_members >= 6, + "expected identity members across the response fixtures, checked {response_members}" + ); +} + +// ── Reads carry verifiable provenance ─────────────────────────────────────── + +/// A read returns the signed event, so a keyless caller can check authorship +/// itself. A host that tampered with content fails verification locally, with no +/// relay involved — which is why this contract does not settle for a projection. +#[test] +fn read_results_are_signed_events_a_keyless_caller_can_verify() { + let signer = Keys::generate(); + let message = signed_message(&signer); + message.verify().expect("a genuinely signed event verifies"); + assert_eq!( + message.author().unwrap().as_str(), + signer.public_key().to_hex() + ); + assert_eq!(message.thread().root.as_deref(), Some(EVENT)); + assert_eq!(message.mentions(), vec![PUBKEY.to_string()]); + + // Tamper with the content: the id no longer matches, so verification fails + // even though every other field is untouched. + let mut json = serde_json::to_value(&message).unwrap(); + json["content"] = serde_json::json!("a message the author never wrote"); + let tampered: BrokerMessage = + serde_json::from_value(json).expect("a tampered event still parses"); + assert!( + tampered.verify().is_err(), + "tampering must be locally detectable" + ); + + // The wire form is the event's own JSON — no wrapper of its own to disagree + // with the signed bytes. + let wire = serde_json::to_value(&message).unwrap(); + assert_eq!( + keys_of(&wire), + vec![ + "content", + "created_at", + "id", + "kind", + "pubkey", + "sig", + "tags" + ] + ); +} + +/// The one type here the contract does not own. `nostr`'s `Event` deserializer +/// accepts and discards unknown members, so a genuinely signed event could carry +/// an extra `secretKey` and parse clean — the no-secret rule stopping at the +/// envelope boundary instead of reaching inside it. Deserializing through a +/// `deny_unknown_fields` intermediary closes that, and this drives the injection +/// on a real signed event so nothing is rejected for a bad signature instead. +#[test] +fn an_event_object_cannot_smuggle_a_member_past_the_seven_canonical_ones() { + let signer = Keys::generate(); + let message = signed_message(&signer); + let wire = serde_json::to_value(&message).expect("event serializes"); + + // The baseline: untouched, this same JSON parses and verifies. + let parsed: BrokerMessage = + serde_json::from_value(wire.clone()).expect("a signed event round-trips"); + parsed.verify().expect("and still verifies"); + + for extra in ["secretKey", "nsec", "seckey", "credential", "hostNote"] { + let mut smuggled = wire.clone(); + smuggled[extra] = serde_json::json!("nsec1deadbeef"); + assert!( + serde_json::from_value::(smuggled.clone()).is_err(), + "an event carrying \"{extra}\" must not deserialize: {smuggled}" + ); + + // And not through the outcome or the envelope either — the rejection has + // to hold at every depth a read result travels. + let outcome = serde_json::json!({ + "action": "channel.read", + "outcome": { "messages": [smuggled.clone()] }, + }); + assert!( + serde_json::from_value::(outcome).is_err(), + "an outcome holding an event with \"{extra}\" must not deserialize" + ); + let envelope = serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": "req-1", + "status": "succeeded", + "action": "channel.read", + "outcome": { "messages": [smuggled] }, + }); + assert!( + serde_json::from_value::(envelope).is_err(), + "a response holding an event with \"{extra}\" must not deserialize" + ); + } + + // Dropping a canonical member is a parse failure too, not a default. + for missing in [ + "id", + "pubkey", + "created_at", + "kind", + "tags", + "content", + "sig", + ] { + let mut json = wire.clone(); + json.as_object_mut().unwrap().remove(missing); + assert!( + serde_json::from_value::(json).is_err(), + "an event missing \"{missing}\" must not deserialize" + ); + } +} + +#[test] +fn a_page_is_bounded_and_its_cursor_opaque() { + let signer = Keys::generate(); + let page = |messages: Vec, next_cursor: Option<&str>| { + ActionOutcome::ChannelRead(MessagePage { + messages, + next_cursor: next_cursor.map(str::to_string), + }) + .validate() + }; + assert!(page(vec![], None).is_ok()); + assert!(page(vec![signed_message(&signer)], Some("c1")).is_ok()); + assert!(page(vec![], Some("")).is_err()); + assert!(page(vec![], Some("has space")).is_err()); + assert!(page( + vec![signed_message(&signer); actions::MAX_PAGE_LIMIT as usize + 1], + None + ) + .is_err()); +} + +/// The protocol cap is not the caller's limit. `ActionOutcome::validate` never +/// sees the request, so on its own it would let a host answer a one-message read +/// with five hundred — within the cap, and still an overrun of what was asked. +/// The request's own number is therefore enforced where both halves are in +/// scope, and an absent `limit` is held to [`actions::DEFAULT_PAGE_LIMIT`] +/// rather than treated as consent to an unbounded page. +#[test] +fn a_read_page_is_bounded_by_the_limit_its_own_request_asked_for() { + let signer = Keys::generate(); + let page = |count: usize| { + BrokerResult::succeeded(ActionOutcome::ChannelRead(MessagePage { + messages: vec![signed_message(&signer); count], + next_cursor: None, + })) + }; + + // Explicit limits, and the absent case — which is the one a host could + // otherwise read as "as many as you like". + for limit in [Some(1_u32), Some(2), Some(actions::MAX_PAGE_LIMIT), None] { + let args = ChannelReadArgs { + channel_id: CHANNEL.into(), + limit, + ..ChannelReadArgs::default() + }; + let allowed = limit.unwrap_or(actions::DEFAULT_PAGE_LIMIT) as usize; + assert_eq!( + args.effective_limit() as usize, + allowed, + "effective_limit must not diverge from the documented default" + ); + let request = prepared(ActionArgs::ChannelRead(args)); + + BrokerResponse::new(request.request_id(), page(allowed)) + .validate_for(&request) + .unwrap_or_else(|e| panic!("a page exactly at a limit of {allowed} is allowed: {e}")); + BrokerResponse::new(request.request_id(), page(allowed - 1)) + .validate_for(&request) + .unwrap_or_else(|e| panic!("a short page is allowed: {e}")); + + // One over is rejected — including one over the default, which is the + // case an unlimited request would have smuggled through. At the + // protocol cap the outcome's own bound fires first, which is a rejection + // for a different (and also correct) reason, so only the message below + // the cap is pinned to the request's number. + let over = + BrokerResponse::new(request.request_id(), page(allowed + 1)).validate_for(&request); + let error = over.unwrap_err().to_string(); + if allowed < actions::MAX_PAGE_LIMIT as usize { + assert!( + error.contains(&format!("limit of {allowed}")), + "unexpected error for a limit of {allowed}: {error}" + ); + } + } + + // The default is a real bound, not the cap under another name: a host that + // answers an unlimited read with a cap-sized page is still overrunning it. + const { + assert!(actions::DEFAULT_PAGE_LIMIT < actions::MAX_PAGE_LIMIT); + } + let unlimited = prepared(ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL))); + assert!(BrokerResponse::new( + unlimited.request_id(), + page(actions::MAX_PAGE_LIMIT as usize) + ) + .validate_for(&unlimited) + .is_err()); +} + +// ── Results ───────────────────────────────────────────────────────────────── + +#[test] +fn failed_and_indeterminate_are_distinct_and_carry_no_outcome() { + let failed = BrokerResult::failed(BrokerError::new( + BrokerErrorCode::ActionFailed, + "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"], "action_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!(failed.outcome().is_none()); + assert!(indeterminate.outcome().is_none()); +} + +/// A code and a status are two statements about the same fact — whether side +/// effects landed — so the contract fixes which pairings are meaningful and +/// rejects the rest. Driven across every code × both statuses, so adding a code +/// forces a decision here. +#[test] +fn status_and_error_code_must_agree_about_side_effects() { + use BrokerErrorCode as E; + for code in all_error_codes() { + let failed = + BrokerResponse::new("req-1", BrokerResult::failed(BrokerError::new(code, "?"))) + .validate(); + let indeterminate = BrokerResponse::new( + "req-1", + BrokerResult::indeterminate(BrokerError::new(code, "?")), + ) + .validate(); + + // The table, spelled out independently of the predicates it checks: a + // second copy is the point, since a test that asked `may_be_failed()` + // would pass for any implementation of it. Exhaustive with no wildcard, + // so a new code cannot inherit an answer — it must be decided here too. + let (failed_ok, indeterminate_ok) = match code { + E::InvalidRequest + | E::UnsupportedProtocolVersion + | E::UnknownAction + | E::UnsupportedActionVersion + | E::Unsupported + | E::Unauthenticated + | E::Unauthorized + | E::RequestIdConflict + | E::ActionFailed => (true, false), + E::OutcomeUnknown => (false, true), + E::Internal => (true, true), + }; + + assert_eq!( + failed.is_ok(), + failed_ok, + "{} with a failed status: {failed:?}", + code.as_str() + ); + assert_eq!( + indeterminate.is_ok(), + indeterminate_ok, + "{} with an indeterminate status: {indeterminate:?}", + code.as_str() + ); + assert_eq!(code.may_be_failed(), failed_ok); + assert_eq!(code.may_be_indeterminate(), indeterminate_ok); + } + + // The two directions review found, named: a rejected credential is a + // known-fate refusal and cannot claim not to know, and `outcome_unknown` + // cannot claim a clean failure. + let error = BrokerResponse::new( + "req-1", + BrokerResult::indeterminate(BrokerError::new(E::Unauthenticated, "credential rejected")), + ) + .validate() + .unwrap_err() + .to_string(); + assert!(error.contains("unauthenticated"), "unexpected: {error}"); + let error = BrokerResponse::new( + "req-1", + BrokerResult::failed(BrokerError::new(E::OutcomeUnknown, "?")), + ) + .validate() + .unwrap_err() + .to_string(); + assert!(error.contains("outcome_unknown"), "unexpected: {error}"); +} + +#[test] +fn replay_metadata_rides_the_response_not_the_result() { + let result = BrokerResult::succeeded(ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: pubkey(), + 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. + assert!(serde_json::to_value(&result) + .unwrap() + .get("replayed") + .is_none()); +} + +/// A response that validates in isolation can still be the wrong answer. This is +/// the check that makes a mismatched outcome unusable rather than merely +/// surprising. +#[test] +fn response_validation_is_request_aware() { + let signer = Keys::generate(); + let request = prepared(ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL))); + let page = ActionOutcome::ChannelRead(MessagePage { + messages: vec![signed_message(&signer)], + next_cursor: None, + }); + + BrokerResponse::new(request.request_id(), BrokerResult::succeeded(page.clone())) + .validate_for(&request) + .expect("the right outcome for the right request"); + + // Wrong action: a post receipt is not an answer to a read. + let wrong_action = BrokerResponse::new( + request.request_id(), + BrokerResult::succeeded(ActionOutcome::MessagePost(EventPublished { + event_id: EVENT.into(), + kind: 9, + created_at: 1, + })), + ); + wrong_action + .validate() + .expect("it is well-formed on its own — that is the point"); + let error = wrong_action.validate_for(&request).unwrap_err().to_string(); + assert!(error.contains("message.post"), "unexpected: {error}"); + + // Wrong correlation id. + let error = BrokerResponse::new("req-other", BrokerResult::succeeded(page)) + .validate_for(&request) + .unwrap_err() + .to_string(); + assert!(error.contains("requestId"), "unexpected: {error}"); + + // Malformed identifiers inside an otherwise well-shaped outcome. + let bad_id = BrokerResponse::new( + request.request_id(), + BrokerResult::succeeded(ActionOutcome::ChannelRead(MessagePage { + messages: vec![], + next_cursor: Some("not a cursor".into()), + })), + ); + assert!(bad_id.validate_for(&request).is_err()); + + let post = prepared(ActionArgs::MessagePost(MessagePostArgs { + channel_id: CHANNEL.into(), + content: "hi".into(), + mentions: vec![], + })); + let bad_event_id = BrokerResponse::new( + post.request_id(), + BrokerResult::succeeded(ActionOutcome::MessagePost(EventPublished { + event_id: "nothex".into(), + kind: 9, + created_at: 1, + })), + ); + assert!(bad_event_id.validate_for(&post).is_err()); + + // A failure needs no outcome to match, only correlation. + BrokerResponse::new( + request.request_id(), + BrokerResult::failed(BrokerError::unauthorized("not your channel")), + ) + .validate_for(&request) + .expect("a refusal answers any action"); +} + +// ── Retry is identical bytes ──────────────────────────────────────────────── + +/// The retry contract is byte identity, so the client takes frozen bytes rather +/// than a typed value it would have to reserialize. Preparing once and reading +/// `body()` twice is the only way to send the same request twice. +#[test] +fn preparing_a_request_freezes_the_bytes_every_attempt_sends() { + let request = BrokerRequest::new( + "req-idem", + ActionArgs::MessagePost(MessagePostArgs { + channel_id: CHANNEL.into(), + content: "exactly once".into(), + mentions: vec![pubkey()], + }), + ) + .unwrap(); + let prepared = request.clone().prepare().expect("valid request prepares"); + + assert_eq!( + prepared.body(), + prepared.body(), + "body is frozen, not re-rendered" + ); + // Correlation metadata is all a transport gets. There is deliberately no + // accessor for the typed request: one would let an implementation serialize + // the value a second time, which is the possibility freezing removes. + assert_eq!(prepared.request_id(), "req-idem"); + assert_eq!(prepared.action(), Action::MessagePost); + + // The frozen bytes are the envelope, and they parse back to the same value. + let parsed: BrokerRequest = + serde_json::from_slice(prepared.body()).expect("frozen body is the envelope"); + assert_eq!(parsed, request); + + // Preparing validates, so an invalid request never reaches a transport. + let invalid = BrokerRequest { + r#type: BROKER_REQUEST_TYPE.into(), + protocol_version: 99, + request_id: "req-bad".into(), + action_version: 1, + action: ActionArgs::AgentsDelete(AgentsDeleteArgs { + target: AgentTarget::Pubkey(pubkey()), + }), + }; + assert!(invalid.prepare().is_err()); +} + +/// The hand-written [`BrokerErrorCode::as_str`] and serde's derived name are two +/// encodings of one wire string, so each is pinned against the other and the +/// whole set is pinned against this literal — a rename in either fails here. +/// This is also what pins [`all_error_codes`] against the enum: a new variant +/// missing from that fixture changes the joined string and fails here. +#[test] +fn error_codes_have_stable_wire_strings() { + let codes = all_error_codes(); + for code in codes { + assert_eq!( + serde_json::to_value(code).unwrap(), + serde_json::json!(code.as_str()), + "as_str and the serde name must not drift" + ); + } + assert_eq!( + codes.map(BrokerErrorCode::as_str).join(","), + "invalid_request,unsupported_protocol_version,unknown_action,\ +unsupported_action_version,unsupported,unauthenticated,unauthorized,\ +request_id_conflict,action_failed,outcome_unknown,internal" + ); +} + +// ── Client trait ──────────────────────────────────────────────────────────── + +/// A test double, and the only implementation in this crate. It exists to prove +/// the trait is object-safe and usable behind `dyn`, which is what lets an +/// in-process host and an HTTP client be interchangeable. +/// +/// Note what it does *not* do: it never calls `validate_for`. It cannot — it has +/// no way to build a [`ValidatedResponse`] except through the blanket +/// [`BrokerClientExt::execute`], which is the whole point of splitting the +/// trait. A deliberately hostile implementation is still forced through the +/// same check. +struct DoubleBroker { + response: Result, +} + +impl BrokerClient for DoubleBroker { + fn send<'a>(&'a self, request: &'a PreparedRequest, _: Dispatch) -> BrokerFuture<'a> { + // A real implementation sends `request.body()` verbatim. The double + // stands in for a host that answers under the id it was asked with, and + // returns the envelope unjudged. + let response = self.response.clone().map(|mut response| { + response.request_id = request.request_id().to_string(); + response + }); + Box::pin(async move { response }) + } +} + +fn block_on(future: F) -> F::Output { + // A hand-rolled park-free executor: the double's future is always ready, so + // one poll suffices and pulling in a runtime would be the heavier choice. + use std::task::{Context, Poll, Wake, Waker}; + struct NoopWake; + impl Wake for NoopWake { + fn wake(self: std::sync::Arc) {} + } + let waker = Waker::from(std::sync::Arc::new(NoopWake)); + let mut context = Context::from_waker(&waker); + let mut future = Box::pin(future); + match future.as_mut().poll(&mut context) { + Poll::Ready(output) => output, + Poll::Pending => panic!("test double must not park"), + } +} + +#[test] +fn the_client_trait_is_object_safe_and_returns_a_validated_host_verdict() { + let request = prepared(ActionArgs::ChannelRead(ChannelReadArgs { + channel_id: CHANNEL.into(), + mentions_only: true, + ..ChannelReadArgs::default() + })); + + let succeeded: Box = Box::new(DoubleBroker { + response: Ok(BrokerResponse::new( + "placeholder", + BrokerResult::succeeded(ActionOutcome::ChannelRead(MessagePage { + messages: vec![], + next_cursor: None, + })), + )), + }); + // `execute` is available on `dyn BrokerClient` and is the only way to a + // `ValidatedResponse` — the caller does no correlation of its own. + let response = block_on(succeeded.execute(&request)).expect("double answers"); + assert_eq!(response.request_id(), "req-1"); + assert!(response.result().outcome().is_some()); + assert!(!response.replayed()); + + // A refusal — including a rejected credential — is still an answer: `Ok` + // with the verdict in the envelope. + for code in [ + BrokerErrorCode::Unauthorized, + BrokerErrorCode::Unauthenticated, + ] { + let refused: Box = Box::new(DoubleBroker { + response: Ok(BrokerResponse::new( + "placeholder", + BrokerResult::failed(BrokerError::new(code, "no")), + )), + }); + let response = + block_on(refused.execute(&request)).expect("a refusal is not a transport error"); + assert_eq!(response.result().error().map(|e| e.code), Some(code)); + } + + // No usable answer at all is a transport error, and says nothing about side + // effects. An intermediary's status is operator detail, not a verdict. + for error in [ + BrokerTransportError::Unreachable("connection reset".into()), + BrokerTransportError::NoEnvelope { + status: 401, + detail: "proxy denied".into(), + }, + BrokerTransportError::MalformedResponse("not json".into()), + ] { + let broken: Box = Box::new(DoubleBroker { + response: Err(error.clone()), + }); + assert_eq!(block_on(broken.execute(&request)).unwrap_err(), error); + } +} + +/// The double returns whatever it is given, unvalidated — a hostile client +/// cannot do otherwise. `execute` is still the only door, so the mismatch +/// surfaces as a transport failure and never reaches a caller as `Ok`. +#[test] +fn a_client_cannot_hand_back_a_response_that_answers_a_different_request() { + let request = prepared(ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL))); + + // Wrong action for this request. + let confused: Box = Box::new(DoubleBroker { + response: Ok(BrokerResponse::new( + "placeholder", + BrokerResult::succeeded(ActionOutcome::AgentsDelete(AgentsDeleteOutcome { + agent_pubkey: pubkey(), + display_name: "Gone".into(), + })), + )), + }); + // The envelope is well-formed in isolation — that is exactly why `send` + // cannot be the caller's door. `execute` is the only reachable one (a + // `Dispatch` token cannot be built outside the client module), and it + // rejects the mismatch rather than passing it on. + assert!(matches!( + block_on(confused.execute(&request)).unwrap_err(), + BrokerTransportError::MalformedResponse(_) + )); + + // Malformed identifiers inside an otherwise well-shaped outcome, too. + let bad_cursor: Box = Box::new(DoubleBroker { + response: Ok(BrokerResponse::new( + "placeholder", + BrokerResult::succeeded(ActionOutcome::ChannelRead(MessagePage { + messages: vec![], + next_cursor: Some("not a cursor".into()), + })), + )), + }); + assert!(matches!( + block_on(bad_cursor.execute(&request)).unwrap_err(), + BrokerTransportError::MalformedResponse(_) + )); + + // A status contradicting its own code, which is how the review reached this: + // `unauthenticated` is a known pre-dispatch refusal, so claiming not to know + // the fate is not a verdict `execute` may pass on as `Ok`. + let contradictory: Box = Box::new(DoubleBroker { + response: Ok(BrokerResponse::new( + "placeholder", + BrokerResult::indeterminate(BrokerError::new( + BrokerErrorCode::Unauthenticated, + "credential rejected", + )), + )), + }); + assert!(matches!( + block_on(contradictory.execute(&request)).unwrap_err(), + BrokerTransportError::MalformedResponse(_) + )); +} + +/// A second double, parsing bytes the way a real HTTP client does, because the +/// strict-envelope and strict-event guards live in `Deserialize` and the typed +/// double above can never exercise them: it hands back a value that was never on +/// a wire. +/// +/// This is the shape the bug actually had — bytes arriving from a host — and what +/// the caller sees now is [`BrokerTransportError::MalformedResponse`], not an +/// `Ok` whose extra members were quietly dropped. +struct WireBroker { + body: Vec, +} + +impl BrokerClient for WireBroker { + fn send<'a>(&'a self, _: &'a PreparedRequest, _: Dispatch) -> BrokerFuture<'a> { + // Exactly a transport's job: parse an envelope, and report the absence + // of one as a transport failure. + let parsed = serde_json::from_slice::(&self.body) + .map_err(|e| BrokerTransportError::MalformedResponse(e.to_string())); + Box::pin(async move { parsed }) + } +} + +#[test] +fn bytes_carrying_more_than_the_contract_declares_never_reach_a_caller_as_ok() { + let signer = Keys::generate(); + let request = prepared(ActionArgs::ChannelRead(ChannelReadArgs::channel(CHANNEL))); + let event = serde_json::to_value(signed_message(&signer)).expect("event serializes"); + let envelope = || { + serde_json::json!({ + "type": BROKER_RESULT_TYPE, + "protocolVersion": 1, + "requestId": request.request_id(), + "status": "succeeded", + "action": "channel.read", + "outcome": { "messages": [event.clone()] }, + }) + }; + + // The honest bytes are accepted, so the rejections below are about the + // smuggled members and not about this fixture being unparseable. + let client = WireBroker { + body: serde_json::to_vec(&envelope()).unwrap(), + }; + let response = block_on(client.execute(&request)).expect("honest bytes are an answer"); + assert!(response.result().outcome().is_some()); + + // A key at each depth: on the envelope, inside the outcome, and inside the + // signed event — the last being the one `nostr` would have discarded. + let mut on_envelope = envelope(); + on_envelope["secretKey"] = serde_json::json!("nsec1deadbeef"); + let mut in_outcome = envelope(); + in_outcome["outcome"]["secretKey"] = serde_json::json!("nsec1deadbeef"); + let mut in_event = envelope(); + in_event["outcome"]["messages"][0]["secretKey"] = serde_json::json!("nsec1deadbeef"); + // And the contradiction the envelope could previously hold on the wire. + let mut error_beside_success = envelope(); + error_beside_success["error"] = serde_json::json!({ "code": "internal", "message": "?" }); + + for (what, json) in [ + ("on the envelope", on_envelope), + ("inside the outcome", in_outcome), + ("inside the event", in_event), + ("an error beside a success", error_beside_success), + ] { + let client = WireBroker { + body: serde_json::to_vec(&json).unwrap(), + }; + assert!( + matches!( + block_on(client.execute(&request)), + Err(BrokerTransportError::MalformedResponse(_)) + ), + "{what}: must not reach the caller as Ok" + ); + } +} diff --git a/crates/buzz-sdk/src/broker/wire.rs b/crates/buzz-sdk/src/broker/wire.rs new file mode 100644 index 0000000000..851d1a165f --- /dev/null +++ b/crates/buzz-sdk/src/broker/wire.rs @@ -0,0 +1,107 @@ +//! The strict wire form of a [`BrokerResponse`]. +//! +//! Split from [`super`] to keep that file within the repo's 1,000-line ceiling. +//! This is the response side's only reader, so it is the one place the envelope's +//! strictness is defined. + +use serde::Deserialize; + +use super::{absent_or_valued, ActionOutcome, BrokerError, BrokerResponse, BrokerResult}; + +/// The strict wire form of a [`BrokerResponse`]: every key spelled out, no +/// `flatten`, so `deny_unknown_fields` is actually in force. +/// +/// The status-specific members are `Option` only because one struct describes +/// three shapes; the status match below requires the exact set per status. +/// They deserialize through [`absent_or_valued`] because the match reads +/// `None` as *absent*, and plain `#[serde(default)]` would map an explicit +/// `null` to the same `None` — letting a contradictory response like +/// `{"status":"failed","outcome":null}` skip the check. +/// +/// `outcome` is held as a `RawValue` and re-parsed, so this reader is +/// JSON-specific — which is fine, JSON is the only encoding this contract has +/// ever specified. +#[derive(Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +struct WireResponse { + r#type: String, + protocol_version: u16, + request_id: String, + status: String, + #[serde(default, deserialize_with = "absent_or_valued")] + action: Option, + #[serde(default, deserialize_with = "absent_or_valued")] + outcome: Option>, + #[serde(default, deserialize_with = "absent_or_valued")] + error: Option, + #[serde(default)] + replayed: bool, +} + +impl<'de> Deserialize<'de> for BrokerResponse { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + use serde::de::Error as _; + + let wire = WireResponse::deserialize(deserializer)?; + + // One arm per status, each naming the members that status may carry. + // Anything present that this status does not admit is rejected here, so + // "succeeded with an error" cannot be parsed and then ignored. + let result = match wire.status.as_str() { + "succeeded" => { + if wire.error.is_some() { + return Err(D::Error::custom( + "a succeeded response must not carry an error", + )); + } + let action = wire + .action + .ok_or_else(|| D::Error::missing_field("action"))?; + let outcome = wire + .outcome + .ok_or_else(|| D::Error::missing_field("outcome"))?; + // Re-parse the outcome under its action tag from the original + // bytes — not via `serde_json::Value`, which collapses + // duplicate keys last-wins — so each outcome type's + // `deny_unknown_fields` applies to the bytes as sent. + let tagged = format!( + "{{\"action\":{},\"outcome\":{}}}", + serde_json::to_string(&action).map_err(D::Error::custom)?, + outcome.get() + ); + let outcome: ActionOutcome = + serde_json::from_str(&tagged).map_err(D::Error::custom)?; + BrokerResult::Succeeded { outcome } + } + status @ ("failed" | "indeterminate") => { + if wire.action.is_some() || wire.outcome.is_some() { + return Err(D::Error::custom(format!( + "a {status} response must not carry an action or outcome" + ))); + } + let error = wire.error.ok_or_else(|| D::Error::missing_field("error"))?; + if status == "failed" { + BrokerResult::Failed { error } + } else { + BrokerResult::Indeterminate { error } + } + } + other => { + return Err(D::Error::custom(format!( + "unknown broker result status \"{other}\"" + ))) + } + }; + + Ok(Self { + r#type: wire.r#type, + protocol_version: wire.protocol_version, + request_id: wire.request_id, + result, + replayed: wire.replayed, + }) + } +} diff --git a/crates/buzz-sdk/src/lib.rs b/crates/buzz-sdk/src/lib.rs index 4ee0cd4c88..845505c56d 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;