jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-30 22:11:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Harden the broker contract against review

Browse Source 
Five contract changes from the quality review, each closing a gap where the
contract documented a promise it did not structurally keep.

Retry means identical bytes, so the client trait can no longer take a typed
value it would have to reserialize. `BrokerRequest::prepare` validates and
serializes once into a `PreparedRequest`; `BrokerClient::execute` takes that,
and every attempt sends the same body. Two serializations of one value may
differ across serde versions, which would have surfaced as a spurious
requestId conflict on retry.

The read cursor is now an opaque host-issued string, not inclusive Unix
seconds. Second-precision paging can loop or skip when more events than
`limit` share a second, and a timestamp cursor would have committed every
future host to one ordering strategy. Cursors are validated for shape only
(bounded, printable ASCII) and never parsed.

Reads return signed Nostr events. `BrokerMessage` is a transparent newtype
over `nostr::Event` with a local `verify()`; the previous projection gave up
authorship provenance for nothing, since Schnorr verification needs no relay.
Ancestry and mentions are derived from the signed tags rather than stored
beside them, so no field can disagree with the signature.

Response validation is request-aware. `validate_for(&PreparedRequest)`
correlates requestId and rejects an outcome whose action is not the request's,
so a caller matching on the outcome enum cannot silently take the wrong
branch. `ActionOutcome::validate` checks the identifiers and cursors an
outcome asserts, and `OutcomeUnknown` paired with `Failed` is now rejected as
self-contradictory.

Auth is a host verdict, not transport. `CredentialRejected` is gone;
`Unauthenticated` arrives inside `Failed`, which carries the promise that
nothing ran. Transport errors are reserved for the absence of a usable
envelope (`NoEnvelope { status, detail }`), and clients are told to parse an
envelope regardless of HTTP status.

The no-secret invariant no longer rests on a source scanner, which aliases and
neutral names bypass and legitimate names trip. It is now an exact wire
key-set table over all nine args and nine outcome types plus
`deny_unknown_fields`, so no field can be added without a reviewer changing
that table. Fixtures populate every optional field, since the table can only
pin what serializes.

Density: fluent builders and per-outcome validators removed, repeated
rationale reduced to one canonical place with cross-references, tautological
tests dropped or merged.

buzz-sdk: 283 tests pass. Workspace clippy, fmt, and doc clean.
Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz>
Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
ss-dev-00
2026-08-24 18:38:12 -07:00
parent 9563c27e99
commit 7b2e207957
4 changed files with 1073 additions and 648 deletions
Download Patch File Download Diff File Expand all files Collapse all files

303
crates/buzz-sdk/src/broker/actions.rs
View File

@@ -1,22 +1,19 @@
//! Broker actions — the closed set of operations an agent may ask a host to perform.
//!
//! An action is the unit of policy. `message.post` and `agents.delete` are
//! separate names so a host can permit one without permitting the other, and so
//! a later policy layer has something per-operation to attach to. There is
//! deliberately no `sign` action and no `publish` action: an interface that can
//! sign arbitrary bytes is a signing oracle, and a host holding it could not
//! reason about what it authorized.
//! An action is the unit of policy; see the [module docs](super) for why this is
//! an operation enum rather than a signing primitive.
//!
//! Two consequences of that choice are visible in the types below. Every args
//! type is `deny_unknown_fields`, so a credential or environment map smuggled
//! into a payload fails to deserialize instead of reaching an executor. Every
//! outcome type can structurally hold only public identifiers — no field of any
//! outcome can transport key material.
//! and outcome type is `deny_unknown_fields`, so a credential or environment map
//! smuggled into a payload fails to deserialize instead of reaching an executor.
//! And the wire key set of every type is pinned by test, so a secret-bearing
//! field cannot be added without a test failing.
use serde::{Deserialize, Serialize};
use crate::SdkError;
use buzz_core::engram::validate_slug;
use nostr::Event;
/// Maximum characters in a display name or agent name.
pub const MAX_NAME_CHARS: usize = 120;
@@ -42,6 +39,9 @@ pub const MAX_MENTIONS: usize = 50;
/// Maximum events a single read may return.
pub const MAX_PAGE_LIMIT: u32 = 500;
/// 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
@@ -53,8 +53,7 @@ pub const RESPOND_TO_MODES: [&str; 2] = ["owner-only", "anyone"];
///
/// #6467 asks for identity to be separable from signing. This type is that
/// separation made structural: it holds 64 hex characters of public key and has
/// no counterpart in this module for the corresponding secret. Nothing here can
/// name, hold, or ask for one.
/// no counterpart in this module for the corresponding secret.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[serde(try_from = "String", into = "String")]
pub struct PubkeyHex(String);
@@ -164,11 +163,9 @@ impl Action {
/// Whether a host may refuse this action without harming the agent.
///
/// #6467 requires non-essential signed housekeeping to be skippable, so
/// that an agent can still run where it is unavailable. For a best-effort
/// action, [`super::BrokerErrorCode::Unsupported`] is a normal answer and a
/// caller must carry on. For every other action, `Unsupported` means the
/// agent cannot do its job on this host and should say so out loud.
/// #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)
@@ -194,16 +191,10 @@ impl Action {
/// Reads are actions like any other, because #6467 requires a host with no
/// relay route of its own to be able to serve them.
///
/// One action covers three scopes, because they differ only by filter and a
/// separate name per scope would be a separate policy decision for the same
/// underlying permission — *may this agent see this channel*. `rootEventId`
/// narrows to one thread; `mentionsOnly` narrows to messages that mention the
/// requester, which is the wake path.
///
/// `since` is an inclusive lower bound in Unix seconds, matching the relay's
/// filter granularity. A caller must de-duplicate by event id: second-precision
/// cursors can straddle events, so consecutive pages may overlap.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
/// 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.
@@ -211,68 +202,45 @@ pub struct ChannelReadArgs {
/// Narrow to one thread by its root event.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub root_event_id: Option<String>,
/// Narrow to messages mentioning the requester.
/// Narrow to messages mentioning the requester — the wake path.
///
/// The requester is never named. The host answers for the identity it
/// authenticated; a body that could ask "what mentions *someone else*"
/// would be a body that picks its own subject.
/// The requester is never named; see [`super::BrokerRequest`] on why no body
/// names its own subject.
#[serde(default, skip_serializing_if = "is_false")]
pub mentions_only: bool,
/// Inclusive lower bound, Unix seconds. Absent = host's default window.
/// Opaque position to resume from, as returned in [`MessagePage::next_cursor`].
///
/// Absent on a first read, which starts at the host's default window. A
/// cursor is **opaque**: callers must round-trip it verbatim and must not
/// parse, compare, or synthesize one. That is deliberate — a timestamp
/// cursor cannot page safely when more events than `limit` share one
/// second, and it would commit every future host to one relay ordering
/// strategy. The host defines ordering and cursor stability, including
/// whether a cursor stays valid across restarts.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub since: Option<u64>,
pub cursor: Option<String>,
/// Maximum events to return, capped at [`MAX_PAGE_LIMIT`].
#[serde(default, skip_serializing_if = "Option::is_none")]
pub limit: Option<u32>,
}
impl ChannelReadArgs {
/// Read a whole channel.
/// Read a whole channel from the host's default window.
#[must_use]
pub fn channel(channel_id: impl Into<String>) -> Self {
Self {
channel_id: channel_id.into(),
root_event_id: None,
mentions_only: false,
since: None,
limit: None,
..Self::default()
}
}
/// Narrow this read to one thread.
#[must_use]
pub fn in_thread(mut self, root_event_id: impl Into<String>) -> Self {
self.root_event_id = Some(root_event_id.into());
self
}
/// Narrow this read to messages mentioning the requester.
#[must_use]
pub fn mentions_only(mut self) -> Self {
self.mentions_only = true;
self
}
/// Start this read at `since` (inclusive, Unix seconds).
#[must_use]
pub fn since(mut self, since: u64) -> Self {
self.since = Some(since);
self
}
/// Cap this read at `limit` events.
#[must_use]
pub fn limit(mut self, limit: u32) -> Self {
self.limit = Some(limit);
self
}
/// Validate and normalize.
///
/// # Errors
///
/// Returns [`SdkError::InvalidInput`] for a malformed channel UUID, a
/// malformed root event id, or an out-of-range limit.
/// malformed root event id, an over-long or non-printable cursor, or an
/// out-of-range limit.
pub fn validated(&self) -> Result<Self, SdkError> {
Ok(Self {
channel_id: channel(&self.channel_id)?,
@@ -282,7 +250,7 @@ impl ChannelReadArgs {
.map(|id| event_id(id, "rootEventId"))
.transpose()?,
mentions_only: self.mentions_only,
since: self.since,
cursor: self.cursor.as_deref().map(cursor).transpose()?,
limit: limit(self.limit)?,
})
}
@@ -434,10 +402,9 @@ impl ProfileSetArgs {
/// Arguments for `storage.address`.
///
/// Encrypted-memory records are addressed by a tag derived from a conversation
/// key, so deriving one needs the secret this contract exists to avoid holding.
/// #6467 lists that derivation among the operations to route through the
/// interface, which is why it is an action rather than a local computation.
/// Deriving a record's address needs the secret this contract exists to avoid
/// holding, which is why #6467 lists it among the operations to route through
/// the interface rather than compute locally.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct StorageAddressArgs {
@@ -461,10 +428,8 @@ impl StorageAddressArgs {
// ── Agent arguments ─────────────────────────────────────────────────────────
/// Which agent an update or delete targets.
///
/// Exactly one selector, because a request naming the target twice is ambiguous
/// and the host would have to guess which wins.
/// 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 {
@@ -490,10 +455,9 @@ impl AgentTarget {
/// Arguments for `agents.create`.
///
/// There is no owner field. The owner of a created agent is whoever the host
/// authenticated for this request — an agent creating an agent owns it, and the
/// chain of ownership ends at a human. A request that could name its own
/// authority would let any caller mint agents under someone else.
/// There is no owner field: the owner is whoever the host authenticated, so an
/// agent creating an agent owns it. See the [module docs](super) on ownership
/// recursion, and [`super::BrokerRequest`] on why no body names its own subject.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct AgentsCreateArgs {
@@ -707,48 +671,88 @@ impl ActionArgs {
// ── Outcomes ────────────────────────────────────────────────────────────────
/// One message returned by a read.
/// One message returned by a read: the signed Nostr event, verbatim.
///
/// A projection, not a Nostr event: no signature, no tag array. A caller that
/// needs to verify signatures needs the relay, which is exactly what a
/// broker-only deployment does not have. Trust here is trust in the host.
/// The event is authoritative. It is carried whole — signature and tags
/// included — rather than reduced to a projection, because Schnorr verification
/// is local and needs no relay: see [`Self::verify`]. A keyless agent therefore
/// still gets independently verifiable authorship and content, and only has to
/// trust the host for *completeness* (that no message was withheld) and for
/// authorization. Reducing to a projection would have widened that trust to
/// cover authorship too, and dropped the tags later consumers need.
///
/// The wire form is the event's own JSON, so this adds no envelope of its own.
/// Ancestry and mentions are exposed as derived accessors rather than sibling
/// fields, so there is nothing that can disagree with the signed bytes.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct BrokerMessage {
/// Event id (hex).
pub event_id: String,
/// Author's pubkey.
pub author_pubkey: PubkeyHex,
/// Event kind.
pub kind: u32,
/// Creation time, Unix seconds.
pub created_at: u64,
/// Message body.
pub content: String,
/// Thread root, when this message is threaded.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub root_event_id: Option<String>,
/// Immediate parent, when this message is a reply.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub parent_event_id: Option<String>,
/// Pubkeys this message mentions.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub mentions: Vec<PubkeyHex>,
#[serde(transparent)]
pub struct BrokerMessage(pub Event);
impl BrokerMessage {
/// The signed event.
#[must_use]
pub fn event(&self) -> &Event {
&self.0
}
/// Verify the event's id and Schnorr signature.
///
/// This is the caller's provenance check and it is entirely local. A host
/// that fabricated or altered a message fails here regardless of what it
/// claims. It is deliberately *not* called by
/// [`super::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, SdkError> {
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<String> {
self.0
.tags
.public_keys()
.map(nostr::PublicKey::to_hex)
.collect()
}
}
/// Outcome of any read action.
///
/// `nextCursor` is the value to pass as `since` on the following call. It is
/// 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.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct MessagePage {
/// Messages in ascending `createdAt` order.
/// Messages in the host's declared order.
pub messages: Vec<BrokerMessage>,
/// Cursor for the next page, if more may exist.
/// Opaque cursor to pass as [`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, skip_serializing_if = "Option::is_none")]
pub next_cursor: Option<u64>,
pub next_cursor: Option<String>,
}
/// Outcome of an action that published one event.
@@ -780,9 +784,10 @@ pub struct StorageAddress {
/// Outcome of a successful `agents.create`.
///
/// Carries the new agent's **public** key only. There is no field for the
/// minted secret: it stays on the host, and this type could not transport it
/// even if a handler tried.
/// Carries the new agent's **public** key only: pubkey, name, channel, and
/// nothing else. There is no field for the minted secret — it stays on the
/// host — and `deny_unknown_fields` plus a test pinning this exact key set 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 {
@@ -865,6 +870,50 @@ impl ActionOutcome {
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, and a caller that trusted either would fail later and further
/// away. Signature verification is deliberately *not* here — that is
/// [`BrokerMessage::verify`], and whether to pay for it is the caller's
/// choice.
///
/// # 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(())
}
}
fn is_false(value: &bool) -> bool {
@@ -941,6 +990,30 @@ fn limit(value: Option<u32>) -> Result<Option<u32>, SdkError> {
}
}
/// 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<String, SdkError> {
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<Option<String>, SdkError> {
let value = optional(value, "respond-to", MAX_SCALAR_CHARS)?;
if let Some(mode) = value.as_deref() {

82
crates/buzz-sdk/src/broker/client.rs
View File

@@ -7,21 +7,21 @@
//! Authorization: Bearer <opaque session credential>
//! Content-Type: application/json
//!
//! <BrokerRequest as JSON>
//! <PreparedRequest::body(), verbatim>
//! ```
//!
//! The response body is a [`BrokerResponse`] as JSON. Every terminal
//! disposition — including [`super::BrokerResult::Failed`] — is a well-formed
//! envelope, so a host that has an answer returns `200` and puts the verdict in
//! `status`. HTTP status codes carry no information the envelope does not, and a
//! client must read the envelope rather than the status line.
//! disposition the *host* reached — including [`super::BrokerResult::Failed`],
//! and including a rejected credential — is a well-formed envelope returned with
//! `200`, because the verdict lives in `status` and a second copy in the status
//! line could only ever disagree with it.
//!
//! A non-2xx response is only meaningful when there is *no* envelope: the
//! credential was rejected before dispatch, the path does not exist, or an
//! intermediary answered instead of the host. Those surface as
//! [`BrokerTransportError`], which is not a `Failed` result — it means no answer
//! arrived, so the caller learned nothing about side effects and may retry with
//! identical bytes per the [`BrokerRequest`] retry contract.
//! A client must nonetheless **attempt to parse an envelope regardless of HTTP
//! status**. A host or intermediary may map dispositions onto conventional
//! statuses for observability or for middleware that cannot read bodies; if a
//! valid envelope is present, it is the answer and the status line is
//! decoration. Only when no envelope can be parsed does the status matter, and
//! then only as detail for an operator: see [`BrokerTransportError`].
//!
//! # The credential
//!
@@ -29,6 +29,12 @@
//! received at startup and can only replay; it is not a key, not a signature,
//! and not derived from anything the agent holds.
//!
//! A rejected credential is a host verdict, not a transport failure: it arrives
//! as `Failed` with [`super::BrokerErrorCode::Unauthenticated`], which carries
//! the promise every `Failed` carries — the action did not run. Modelling it as
//! a transport error would have thrown that knowledge away and told the caller
//! to reconcile something that provably never happened.
//!
//! Binding a credential to a specific (agent, conversation) pair — so that a
//! leaked token cannot act as a different agent or outside the conversation it
//! was issued for — **is the host's concern**. This contract documents the
@@ -43,7 +49,7 @@
use std::future::Future;
use std::pin::Pin;
use super::{BrokerRequest, BrokerResponse};
use super::{BrokerResponse, PreparedRequest};
/// Path of the single broker endpoint.
pub const BROKER_ACTION_PATH: &str = "/v1/action";
@@ -51,20 +57,38 @@ pub const BROKER_ACTION_PATH: &str = "/v1/action";
/// Header carrying the opaque session credential, as `Bearer <credential>`.
pub const BROKER_CREDENTIAL_HEADER: &str = "authorization";
/// A transport-level failure: no [`BrokerResponse`] was obtained.
/// No usable [`BrokerResponse`] was obtained, so the request's fate is unknown.
///
/// Distinct from [`super::BrokerResult::Failed`], which is a host's considered
/// verdict. These variants all mean the same thing to a caller — the request's
/// fate is unknown — and differ only in what to tell an operator.
/// 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 — not even a rejected credential, which is
/// [`super::BrokerErrorCode::Unauthenticated`] inside a `Failed` envelope. 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),
/// The credential was rejected before the host produced an envelope.
#[error("broker rejected the session credential")]
CredentialRejected,
/// A response arrived but was not a well-formed [`BrokerResponse`].
/// 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, or a malformed outcome.
///
/// See [`BrokerResponse::validate_for`].
#[error("malformed broker response: {0}")]
MalformedResponse(String),
}
@@ -83,14 +107,22 @@ pub type BrokerFuture<'a> =
/// [`super::Action`] inside the request, so adding an action never changes this
/// trait and an implementation never needs updating to carry one.
///
/// The argument is a [`PreparedRequest`] rather than a typed request so that an
/// implementation cannot reserialize between attempts; see the
/// [`super::BrokerRequest`] retry contract.
///
/// Implementations must be usable as `dyn BrokerClient`.
pub trait BrokerClient: Send + Sync {
/// Execute `request` and return the host's terminal response.
/// Send `request`'s frozen bytes and return the host's terminal response.
///
/// Implementations should call [`BrokerResponse::validate_for`] before
/// returning `Ok`, so a caller never has to consider a response that does
/// not answer its request.
///
/// # Errors
///
/// Returns [`BrokerTransportError`] when no response could be obtained.
/// A host that answered — even to refuse — returns `Ok` with the verdict in
/// [`BrokerResponse::result`].
fn execute<'a>(&'a self, request: &'a BrokerRequest) -> BrokerFuture<'a>;
/// Returns [`BrokerTransportError`] when no usable response could be
/// obtained. A host that answered — even to refuse — returns `Ok` with the
/// verdict in [`BrokerResponse::result`].
fn execute<'a>(&'a self, request: &'a PreparedRequest) -> BrokerFuture<'a>;
}

175
crates/buzz-sdk/src/broker/mod.rs
View File

@@ -38,14 +38,28 @@
//!
//! | #6467 requirement | Where |
//! |---|---|
//! | Identity separable from signing; pubkey-only identity | [`PubkeyHex`] is the only identity type. No secret-key type exists in this module, and no args or outcome field can hold one. |
//! | Secret-dependent operations funnelled through the interface | Event signing is subsumed by the write actions (`message.post`, `message.reply`, `reaction.add`, `profile.set`, `agents.*`) — the agent states intent, the host signs. Encrypted-storage address derivation is [`Action::StorageAddress`]. NIP-42 relay auth, NIP-44 encryption, and NIP-98 request auth are **not** actions: they are mechanisms internal to whoever holds the key, and this contract never asks for them by name. |
//! | Relay auth is a control-flow choice | Nothing here mentions relay authentication. An agent using this contract does not authenticate to a relay, so there is no local auth step to skip. |
//! | Identity separable from signing; pubkey-only identity | [`PubkeyHex`] is the only identity type; no secret-key type exists in this module. |
//! | Secret-dependent operations funnelled through the interface | Event signing is subsumed by the write actions (`message.post`, `message.reply`, `reaction.add`, `profile.set`, `agents.*`) — the agent states intent, the host signs. Encrypted-storage address derivation is [`Action::StorageAddress`]. NIP-42 relay auth, NIP-44 encryption, and NIP-98 request auth are **not** actions: they are mechanisms internal to whoever holds the key. |
//! | Relay auth is a control-flow choice | Nothing here mentions relay authentication, so there is no local auth step to skip. |
//! | Every relay-touching op, reads included, routable | [`Action::ChannelRead`] covers channel, thread, and mention-feed reads. No action assumes the caller can reach a relay. |
//! | Non-essential housekeeping skippable | [`Action::is_best_effort`] marks such actions; a host answers [`BrokerErrorCode::Unsupported`] and the agent carries on. |
//! | No secret leaks to children via env | No args type carries an environment map, and `deny_unknown_fields` means one cannot be smuggled in. Process spawning is a host concern this contract cannot express. |
//! | Lives in the shared client layer | `buzz-sdk`, which the CLI and the harness already depend on. |
//!
//! # The no-secret rule
//!
//! No secret key material crosses this boundary in either direction. What
//! enforces it is the wire schema, not a comment: every args and outcome type is
//! `deny_unknown_fields`, and tests pin each type's exact key set, so a
//! secret-bearing field cannot be added without a test failing.
//! [`AgentsCreateOutcome`] is the case that matters — it returns public identity
//! only, never the key it just minted.
//!
//! Two limits are worth stating. A `String` field can physically hold secret
//! text, so keeping secrets out of message content and error messages is host
//! policy this contract cannot enforce. And nothing stops a host from *holding*
//! keys — that is the point; it stops one from handing them over.
//!
//! # Ownership recursion
//!
//! [`Action::AgentsCreate`] has no owner field. The owner of a created agent is
@@ -115,23 +129,30 @@ pub const MAX_REQUEST_ID_LEN: usize = 128;
///
/// # What this envelope deliberately omits
///
/// There is no requester, owner, 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.
/// There is 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** — that one rule is why
/// `channel.read` cannot ask about another identity's mentions, why
/// `profile.set` has no subject, and why `agents.create` has no owner.
///
/// # Retry contract
///
/// Retrying means **resending the identical serialized request** with the same
/// `requestId`. The host hashes what it receives and compares that digest
/// against the digest recorded under the same idempotency key:
/// Retrying means resending the identical serialized request with the same
/// `requestId`, which is why a client never sends this type directly: call
/// [`Self::prepare`] to freeze it into a [`PreparedRequest`] and hand *that* to
/// [`BrokerClient::execute`]. The host hashes the bytes it receives and compares
/// that digest against the digest recorded under the same idempotency key:
///
/// - same key, same digest → the recorded outcome is replayed, nothing re-runs
/// - same key, different digest → rejected as a request-ID conflict
///
/// Callers must therefore not regenerate, re-order, or re-render the payload
/// between attempts. There is no client-computed digest field: idempotency is
/// decided host-side, and a caller-supplied digest would be a claim the host
/// has to recompute anyway.
/// A typed value cannot carry that guarantee. Two serializations of one value
/// can differ in bytes across serde versions or implementations, and the
/// difference would surface as a spurious [`BrokerErrorCode::RequestIdConflict`]
/// on retry. Serializing once removes the possibility rather than warning about
/// it. There is no client-computed digest field: idempotency is decided
/// host-side, and a caller-supplied digest would be a claim the host has to
/// recompute anyway.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields, rename_all = "camelCase")]
pub struct BrokerRequest {
@@ -174,6 +195,23 @@ impl BrokerRequest {
self.action.action()
}
/// Validate, then serialize once into the bytes every attempt will send.
///
/// # Errors
///
/// Returns [`SdkError`] when [`Self::validate`] fails, or
/// [`SdkError::InvalidInput`] if serialization fails.
pub fn prepare(self) -> Result<PreparedRequest, SdkError> {
self.validate()?;
let body = serde_json::to_vec(&self).map_err(|e| {
SdkError::InvalidInput(format!("broker request is not serializable: {e}"))
})?;
Ok(PreparedRequest {
request: self,
body,
})
}
/// Validate every field the host must agree on before executing anything.
///
/// Callers validate before sending; the host revalidates on receipt,
@@ -242,8 +280,48 @@ pub fn validate_request_id(request_id: &str) -> Result<(), SdkError> {
Ok(())
}
/// Machine-readable broker error code.
/// A validated request together with the exact bytes to send.
///
/// This is what [`BrokerClient::execute`] takes, so the retry contract is
/// structural rather than documented: the first attempt and every retry send
/// `body` verbatim, and no implementation gets the chance to reserialize.
///
/// Construct one with [`BrokerRequest::prepare`]. The typed [`BrokerRequest`] is
/// kept alongside so a client can correlate the response without reparsing —
/// see [`BrokerResponse::validate_for`].
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PreparedRequest {
request: BrokerRequest,
body: Vec<u8>,
}
impl PreparedRequest {
/// The frozen JSON body. Every attempt sends exactly these bytes.
#[must_use]
pub fn body(&self) -> &[u8] {
&self.body
}
/// The typed request these bytes encode.
#[must_use]
pub fn request(&self) -> &BrokerRequest {
&self.request
}
/// 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.
@@ -265,6 +343,10 @@ pub enum BrokerErrorCode {
/// 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,
@@ -456,10 +538,16 @@ impl BrokerResponse {
/// 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`, or a malformed `requestId`.
/// `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!(
@@ -473,7 +561,62 @@ impl BrokerResponse {
self.protocol_version
)));
}
validate_request_id(&self.request_id)
validate_request_id(&self.request_id)?;
match &self.result {
BrokerResult::Succeeded { outcome } => outcome.validate()?,
// `OutcomeUnknown` is the one code that describes not knowing
// whether side effects landed. Paired with `Failed` — which promises
// they did not — it would be self-contradictory, so it is rejected
// rather than left for each caller to notice.
BrokerResult::Failed { error } if error.code == BrokerErrorCode::OutcomeUnknown => {
return Err(SdkError::InvalidInput(
"outcome_unknown is only valid with an indeterminate status".into(),
));
}
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
/// host (or a confused proxy) could return a `message.post` success to a
/// `channel.read`, and a caller matching on the outcome enum would quietly
/// take the wrong branch. This is the check that makes such a response
/// unusable instead of merely surprising, which is why
/// [`BrokerClient::execute`] implementations are expected to run it before
/// returning `Ok` and to report a failure as
/// [`BrokerTransportError::MalformedResponse`].
///
/// 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 or a
/// success outcome names a different action than the request.
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()
)));
}
}
Ok(())
}
}

1161
crates/buzz-sdk/src/broker/tests.rs
View File

File diff suppressed because it is too large Load Diff