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 index 0641c1edc2..a5a75ed2cc 100644 --- a/crates/buzz-sdk/src/broker/actions/args.rs +++ b/crates/buzz-sdk/src/broker/actions/args.rs @@ -505,7 +505,7 @@ impl AgentsDeleteArgs { /// `{ "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")] +#[serde(tag = "action", content = "args", deny_unknown_fields)] pub enum ActionArgs { /// Read a channel, thread, or mention feed. #[serde(rename = "channel.read")] diff --git a/crates/buzz-sdk/src/broker/actions/outcomes.rs b/crates/buzz-sdk/src/broker/actions/outcomes.rs index 3f5b5e52ea..2590722749 100644 --- a/crates/buzz-sdk/src/broker/actions/outcomes.rs +++ b/crates/buzz-sdk/src/broker/actions/outcomes.rs @@ -219,7 +219,7 @@ pub struct AgentsDeleteOutcome { /// An action-specific success payload. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "action", content = "outcome")] +#[serde(tag = "action", content = "outcome", deny_unknown_fields)] pub enum ActionOutcome { /// `channel.read` succeeded. #[serde(rename = "channel.read")] diff --git a/crates/buzz-sdk/src/broker/correlate.rs b/crates/buzz-sdk/src/broker/correlate.rs new file mode 100644 index 0000000000..f9f7bf735c --- /dev/null +++ b/crates/buzz-sdk/src/broker/correlate.rs @@ -0,0 +1,75 @@ +//! 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. +/// +/// The table of what is and is not compared, with the reasoning for each, is on +/// [`BrokerResponse::validate_for`][super::BrokerResponse::validate_for] — the +/// public entry point a host author reads. It is documented there rather than +/// here so there is one copy to keep true. +/// +/// 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 one echoed identity, naming the field in the error. + fn same(field: &str, requested: &str, returned: &str) -> Result<(), SdkError> { + if requested == returned { + return Ok(()); + } + Err(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("channelId", &args.channel_id, &outcome.channel_id) + } + (ActionArgs::AgentsUpdate(args), ActionOutcome::AgentsUpdate(outcome)) => { + match targeted(&args.target) { + Some(requested) => same( + "agentPubkey", + requested.as_str(), + outcome.agent_pubkey.as_str(), + ), + None => Ok(()), + } + } + (ActionArgs::AgentsDelete(args), ActionOutcome::AgentsDelete(outcome)) => { + match targeted(&args.target) { + Some(requested) => same( + "agentPubkey", + requested.as_str(), + outcome.agent_pubkey.as_str(), + ), + None => Ok(()), + } + } + // These outcomes echo no identity the request supplied; see the table. + (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 index a0ad36087b..3823047221 100644 --- a/crates/buzz-sdk/src/broker/mod.rs +++ b/crates/buzz-sdk/src/broker/mod.rs @@ -55,16 +55,32 @@ //! [`AgentsCreateOutcome`] is the case that matters — it returns public identity //! only, never the key it just minted. //! -//! Strictness has to hold at every layer, or the outermost one decides. Two +//! Strictness has to hold at every layer, or the outermost one decides. Three //! places needed explicit work, because each had a hole a derive left open: //! [`BrokerResponse`] cannot combine `deny_unknown_fields` with the `flatten` -//! that produces its wire shape, and [`BrokerMessage`] wraps `nostr`'s `Event`, -//! whose own deserializer discards unknown members. Both now deserialize through -//! private strict intermediaries, so the rule reaches *inside* the envelope and -//! inside each event object — a host cannot ship a `secretKey` beside `sig` and -//! have it silently trimmed. +//! that produces its wire shape; [`BrokerMessage`] wraps `nostr`'s `Event`, +//! whose own deserializer discards unknown members; and [`ActionArgs`] / +//! [`ActionOutcome`] are adjacently tagged, so without `deny_unknown_fields` on +//! the enum itself a sibling of their two keys was ignored when either type was +//! deserialized directly — and both are public and wire-facing, so that is a +//! real door, not a hypothetical one. The first two now deserialize through +//! private strict intermediaries and the third denies unknown fields, so the +//! rule reaches *inside* the envelope, inside each event object, and around each +//! nested action object — a host cannot ship a `secretKey` beside `sig`, or +//! beside `args`, and have it silently trimmed. //! -//! A third hole was subtler and is closed the same way — see +//! # Duplicate members are rejected +//! +//! A key may appear at most once in any object. serde's derived readers reject a +//! repeated field, so most of this contract gets that for free — but the strict +//! response intermediary originally buffered `outcome` through a +//! `serde_json::Value`, and that collapses duplicates last-wins. `outcome` was +//! therefore the one place a reader could observe a value the envelope's own +//! strictness never vetted, so it now re-parses the original bytes. The practical +//! consequence for a host author is the same as the no-null rule: emit each +//! member once, and do not rely on a later occurrence overriding an earlier one. +//! +//! A fourth hole was subtler and is closed the same way — see //! [Optional members](#optional-members-omission-is-the-only-spelling-of-absence). //! //! Two limits are worth stating. A `String` field can physically hold secret @@ -138,6 +154,7 @@ use crate::SdkError; pub mod actions; pub mod client; +mod correlate; use actions::absent_or_valued; pub use actions::{ @@ -219,15 +236,21 @@ impl BrokerRequest { /// [`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 = Self { + let request_id = request_id.into(); + validate_request_id(&request_id)?; + // Store the normalized copy, not the caller's. `validated` both checks + // and normalizes (trimming names, lowercasing pubkeys), so keeping the + // original would let a padded selector pass validation and then travel + // in the frozen body — the host would look up something the validator + // never approved. + let action = action.validated()?; + Ok(Self { r#type: BROKER_REQUEST_TYPE.to_string(), protocol_version: BROKER_PROTOCOL_VERSION, - request_id: request_id.into(), + request_id, action_version: action.action().current_version(), action, - }; - request.validate()?; - Ok(request) + }) } /// The action this request invokes. @@ -236,14 +259,25 @@ impl BrokerRequest { self.action.action() } - /// Validate, then serialize once into the bytes every attempt will send. + /// Validate and normalize, then serialize once into the bytes every attempt + /// will send. + /// + /// This re-validates even though [`Self::new`] already did. That is not + /// redundant: every field of this struct is public and the type is + /// [`Deserialize`], so a value can reach here without ever passing through + /// [`Self::new`] — built with a struct literal, parsed from JSON, or mutated + /// after construction. Freezing bytes is the last point where anything can + /// be checked, so it normalizes here too and serializes the normalized copy. + /// Both paths therefore satisfy the same invariant: the frozen body contains + /// exactly what validation approved. /// /// # Errors /// /// Returns [`SdkError`] when [`Self::validate`] fails, or /// [`SdkError::InvalidInput`] if serialization fails. - pub fn prepare(self) -> Result { - self.validate()?; + pub fn prepare(mut self) -> Result { + self.validate_envelope()?; + self.action = self.action.validated()?; let body = serde_json::to_vec(&self).map_err(|e| { SdkError::InvalidInput(format!("broker request is not serializable: {e}")) })?; @@ -264,6 +298,16 @@ impl BrokerRequest { /// `protocolVersion` or `actionVersion`, a malformed `requestId`, or /// arguments that fail their own validation. pub fn validate(&self) -> Result<(), SdkError> { + self.validate_envelope()?; + self.action.validate() + } + + /// Validate everything except the action arguments. + /// + /// Split out so [`Self::prepare`] can check the envelope and then take the + /// normalized arguments from a single `validated` call, rather than + /// validating the arguments twice on the way to freezing bytes. + 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 \"{}\"", @@ -286,7 +330,7 @@ impl BrokerRequest { action.current_version() ))); } - self.action.validate() + Ok(()) } } @@ -671,7 +715,7 @@ struct WireResponse { #[serde(default, deserialize_with = "absent_or_valued")] action: Option, #[serde(default, deserialize_with = "absent_or_valued")] - outcome: Option, + outcome: Option>, #[serde(default, deserialize_with = "absent_or_valued")] error: Option, #[serde(default)] @@ -706,9 +750,17 @@ impl<'de> Deserialize<'de> for BrokerResponse { // Re-deserialize the outcome under its action tag, which is how // the adjacently-tagged `ActionOutcome` — and the // `deny_unknown_fields` on each outcome type — get applied. - let tagged = serde_json::json!({ "action": action, "outcome": outcome }); + // Re-deserialize from the original bytes, not from a + // `serde_json::Value`: buffering through `Value` collapses + // duplicate keys last-wins, which would let `outcome` carry a + // duplicate that no other part of this contract accepts. + let tagged = format!( + "{{\"action\":{},\"outcome\":{}}}", + serde_json::to_string(&action).map_err(D::Error::custom)?, + outcome.get() + ); let outcome: ActionOutcome = - serde_json::from_value(tagged).map_err(D::Error::custom)?; + serde_json::from_str(&tagged).map_err(D::Error::custom)?; BrokerResult::Succeeded { outcome } } status @ ("failed" | "indeterminate") => { @@ -834,8 +886,33 @@ impl BrokerResponse { /// /// Returns everything [`Self::validate`] returns, plus /// [`SdkError::InvalidInput`] when the `requestId` does not correlate, a - /// success outcome names a different action than the request, or a read - /// returned more messages than the request allowed. + /// 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 + /// + /// `requestId` plus action is not enough: a host routing bug can return a + /// well-formed success for the wrong *subject*. Every identity the request + /// supplies and the outcome echoes must match exactly. This is the whole + /// table: + /// + /// | Action | compared | not compared, and why | + /// |---|---|---| + /// | `channel.read` | — | outcome carries a page and cursor, no echo of `channelId` | + /// | `message.post` | — | outcome is `eventId`/`kind`/`createdAt`, all host-minted | + /// | `message.reply` | — | same; the parent id is not echoed | + /// | `reaction.add` | — | same | + /// | `profile.set` | — | same | + /// | `storage.address` | — | `slug` is deliberately absent from the outcome: a `d` tag is a keyed hash of it, and echoing the slug would defeat that | + /// | `agents.create` | `channelId` | pubkey and name are newly minted, so there is nothing prior to compare | + /// | `agents.update` | `agentPubkey` when targeted by pubkey | a name target is resolved host-side; `displayName` may be exactly what this call changed | + /// | `agents.delete` | `agentPubkey` when targeted by pubkey | ditto for a name target | + /// + /// A name-targeted `agents.update`/`agents.delete` is the one case where the + /// caller asked for an identity it cannot verify in the reply. That is + /// inherent: the host resolves the name, and a rename may be the very thing + /// the call performed. pub fn validate_for(&self, request: &PreparedRequest) -> Result<(), SdkError> { self.validate()?; if self.request_id != request.request_id() { @@ -854,6 +931,7 @@ impl BrokerResponse { expected.as_str() ))); } + correlate::correlate_identities(&request.request.action, outcome)?; // `ActionOutcome::validate` can only enforce the protocol-wide cap, // because it never sees the request. A page bounded only by that cap // still overruns a caller that asked for one message and was handed diff --git a/crates/buzz-sdk/src/broker/tests.rs b/crates/buzz-sdk/src/broker/tests.rs index 7372af386c..c5d518b56f 100644 --- a/crates/buzz-sdk/src/broker/tests.rs +++ b/crates/buzz-sdk/src/broker/tests.rs @@ -770,6 +770,80 @@ fn request_id_must_be_present_bounded_and_printable() { 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 @@ -1071,6 +1145,48 @@ fn no_payload_can_name_its_own_authority() { } } +/// 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()); @@ -1191,6 +1307,211 @@ fn validators_accept_and_reject_at_their_boundaries() { .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:?}" + ); + } + } +} + // ── Reads carry verifiable provenance ─────────────────────────────────────── /// A read returns the signed event, so a keyless caller can check authorship