From 046827c921d4e933b7e8d17e043d733bd3873ce0 Mon Sep 17 00:00:00 2001 From: Justin Perea Date: Wed, 29 Jul 2026 05:12:08 -0400 Subject: [PATCH 1/2] fix(workflows): authorize trusted owner-only agent wakeups Co-authored-by: Justin Perea Signed-off-by: Justin Perea --- crates/buzz-acp/src/lib.rs | 230 ++++++++++++++++++++++++- crates/buzz-acp/src/relay.rs | 59 ++++++- crates/buzz-acp/src/setup_mode.rs | 17 +- crates/buzz-relay/src/workflow_sink.rs | 54 +++++- implementation-notes.md | 38 ++++ 5 files changed, 383 insertions(+), 15 deletions(-) create mode 100644 implementation-notes.md diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 811253e4ac..d8288cd84c 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -257,6 +257,60 @@ async fn author_allowed( } } +/// Return the workflow owner represented by a relay-signed message. +/// +/// Attribution is trusted only after verifying the NIP-11 relay signer, event +/// signature, message kind, and exact single-value workflow/actor tags. Any +/// ambiguity fails closed and falls back to the literal event signer. +fn trusted_workflow_actor( + event: &nostr::Event, + trusted_relay_pubkey: Option<&str>, +) -> Option { + let trusted_relay_pubkey = trusted_relay_pubkey?; + if event.pubkey.to_hex() != trusted_relay_pubkey + || event.kind.as_u16() as u32 != KIND_STREAM_MESSAGE + || event.verify().is_err() + { + return None; + } + + let workflow_tags: Vec<&[String]> = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("buzz:workflow")) + .map(nostr::Tag::as_slice) + .collect(); + if workflow_tags.len() != 1 + || workflow_tags[0].len() != 2 + || workflow_tags[0].get(1).map(String::as_str) != Some("true") + { + return None; + } + + let actor_tags: Vec<&[String]> = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().map(String::as_str) == Some("actor")) + .map(nostr::Tag::as_slice) + .collect(); + if actor_tags.len() != 1 || actor_tags[0].len() != 2 { + return None; + } + let actor = actor_tags[0].get(1).map(String::as_str)?; + if actor.len() != 64 || !actor.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + + PublicKey::from_hex(actor) + .ok() + .map(|pubkey| pubkey.to_hex()) +} + +/// Resolve the identity used by the inbound author gate. +fn effective_inbound_author(event: &nostr::Event, trusted_relay_pubkey: Option<&str>) -> String { + trusted_workflow_actor(event, trusted_relay_pubkey).unwrap_or_else(|| event.pubkey.to_hex()) +} + /// Resolve whether `channel_id` is a DM, for the inbound author gate. /// /// Resolution order: @@ -1406,6 +1460,20 @@ async fn tokio_main() -> Result<()> { tracing::info!("connected to relay at {}", config.relay_url); + let rest_client = relay.rest_client(); + let trusted_relay_pubkey = match rest_client.relay_self_pubkey().await { + Ok(pubkey) => { + tracing::info!("trusted relay workflow signer: {pubkey}"); + Some(pubkey) + } + Err(error) => { + tracing::warn!( + "failed to resolve trusted relay workflow signer; workflow attribution disabled: {error}" + ); + None + } + }; + relay .subscribe_membership_notifications() .await @@ -1600,8 +1668,8 @@ async fn tokio_main() -> Result<()> { .unwrap_or_else(|_| std::path::PathBuf::from("/")) .to_string_lossy() .to_string(), - rest_client: relay.rest_client(), - channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), + rest_client: rest_client.clone(), + channel_info: pool::ChannelInfoResolver::new(channel_info_map, rest_client), context_message_limit: config.context_message_limit, max_turns_per_session: config.max_turns_per_session, permission_mode: config.permission_mode, @@ -2215,7 +2283,10 @@ async fn tokio_main() -> Result<()> { // explicit pubkey list on top, for external people; // it never revokes same-owner team bots. { - let author = buzz_event.event.pubkey.to_hex(); + let author = effective_inbound_author( + &buzz_event.event, + trusted_relay_pubkey.as_deref(), + ); // DM hardening: resolve channel type (fail-closed // to DM) so allowlist/anyone modes cannot be // exercised by non-owner authors inside DMs. @@ -2233,7 +2304,8 @@ async fn tokio_main() -> Result<()> { if !allowed { tracing::debug!( channel_id = %buzz_event.channel_id, - author = %buzz_event.event.pubkey.to_hex(), + signer = %buzz_event.event.pubkey.to_hex(), + effective_author = %author, mode = %config.respond_to, is_dm, "inbound author gate — dropping event" @@ -4889,6 +4961,156 @@ mod author_gate_tests { } } +#[cfg(test)] +mod workflow_author_tests { + use super::*; + use nostr::{EventBuilder, Keys, Kind, Tag}; + + fn workflow_event(signer: &Keys, actor: &str, kind: u32, extra_tags: Vec) -> nostr::Event { + let mut tags = vec![ + Tag::parse(["buzz:workflow", "true"]).expect("workflow tag"), + Tag::parse(["actor", actor]).expect("actor tag"), + ]; + tags.extend(extra_tags); + EventBuilder::new(Kind::Custom(kind as u16), "nightly work") + .tags(tags) + .sign_with_keys(signer) + .expect("signed workflow event") + } + + #[test] + fn relay_signed_workflow_uses_verified_actor() { + let relay = Keys::generate(); + let actor = Keys::generate(); + let event = workflow_event( + &relay, + &actor.public_key().to_hex(), + KIND_STREAM_MESSAGE, + vec![], + ); + + assert_eq!( + trusted_workflow_actor(&event, Some(&relay.public_key().to_hex())), + Some(actor.public_key().to_hex()) + ); + assert_eq!( + effective_inbound_author(&event, Some(&relay.public_key().to_hex())), + actor.public_key().to_hex() + ); + } + + #[test] + fn forged_workflow_attribution_falls_back_to_literal_signer() { + let relay = Keys::generate(); + let attacker = Keys::generate(); + let actor = Keys::generate(); + let event = workflow_event( + &attacker, + &actor.public_key().to_hex(), + KIND_STREAM_MESSAGE, + vec![], + ); + + assert_eq!( + trusted_workflow_actor(&event, Some(&relay.public_key().to_hex())), + None + ); + assert_eq!( + effective_inbound_author(&event, Some(&relay.public_key().to_hex())), + attacker.public_key().to_hex() + ); + } + + #[test] + fn workflow_attribution_fails_closed_without_relay_identity_or_valid_signature() { + let relay = Keys::generate(); + let actor = Keys::generate(); + let event = workflow_event( + &relay, + &actor.public_key().to_hex(), + KIND_STREAM_MESSAGE, + vec![], + ); + assert_eq!(trusted_workflow_actor(&event, None), None); + + let mut tampered = event; + tampered.content = "tampered".into(); + assert_eq!( + trusted_workflow_actor(&tampered, Some(&relay.public_key().to_hex())), + None + ); + } + + #[test] + fn workflow_attribution_rejects_wrong_kind_and_ambiguous_tags() { + let relay = Keys::generate(); + let actor = Keys::generate(); + let actor_hex = actor.public_key().to_hex(); + + let wrong_kind = workflow_event(&relay, &actor_hex, 1, vec![]); + assert_eq!( + trusted_workflow_actor(&wrong_kind, Some(&relay.public_key().to_hex())), + None + ); + + let duplicate_actor = workflow_event( + &relay, + &actor_hex, + KIND_STREAM_MESSAGE, + vec![Tag::parse(["actor", &actor_hex]).expect("duplicate actor tag")], + ); + assert_eq!( + trusted_workflow_actor(&duplicate_actor, Some(&relay.public_key().to_hex())), + None + ); + + let duplicate_workflow = workflow_event( + &relay, + &actor_hex, + KIND_STREAM_MESSAGE, + vec![Tag::parse(["buzz:workflow", "true"]).expect("duplicate workflow tag")], + ); + assert_eq!( + trusted_workflow_actor(&duplicate_workflow, Some(&relay.public_key().to_hex())), + None + ); + + let malformed_actor = workflow_event(&relay, "not-a-pubkey", KIND_STREAM_MESSAGE, vec![]); + assert_eq!( + trusted_workflow_actor(&malformed_actor, Some(&relay.public_key().to_hex())), + None + ); + + let malformed_duplicate_actor = workflow_event( + &relay, + &actor_hex, + KIND_STREAM_MESSAGE, + vec![Tag::parse(["actor", &actor_hex, "unexpected"]).expect("malformed actor tag")], + ); + assert_eq!( + trusted_workflow_actor( + &malformed_duplicate_actor, + Some(&relay.public_key().to_hex()) + ), + None + ); + + let malformed_duplicate_workflow = workflow_event( + &relay, + &actor_hex, + KIND_STREAM_MESSAGE, + vec![Tag::parse(["buzz:workflow", "false"]).expect("malformed workflow tag")], + ); + assert_eq!( + trusted_workflow_actor( + &malformed_duplicate_workflow, + Some(&relay.public_key().to_hex()) + ), + None + ); + } +} + #[cfg(test)] mod observer_snapshot_race_tests { use super::*; diff --git a/crates/buzz-acp/src/relay.rs b/crates/buzz-acp/src/relay.rs index aea5cee077..118d392436 100644 --- a/crates/buzz-acp/src/relay.rs +++ b/crates/buzz-acp/src/relay.rs @@ -118,7 +118,7 @@ use buzz_core::kind::{ KIND_TYPING_INDICATOR, }; use futures_util::{SinkExt, StreamExt}; -use nostr::{Event, EventBuilder, Keys, Kind, RelayUrl, Tag}; +use nostr::{Event, EventBuilder, Keys, Kind, PublicKey, RelayUrl, Tag}; use serde_json::{json, Value}; use tokio::sync::mpsc; use tokio::time::timeout; @@ -259,6 +259,28 @@ fn unix_now_secs() -> u64 { } impl RestClient { + /// Fetch the relay's NIP-11 `self` pubkey from its public root document. + /// + /// This is intentionally unauthenticated metadata. Callers use it as the + /// expected signer for relay-authored events and must still verify each + /// event's id and signature locally. + pub async fn relay_self_pubkey(&self) -> Result { + let url = self.base_url.clone(); + let response = self + .request_with_retry("GET", "/", || { + self.http + .get(&url) + .header("Accept", "application/nostr+json") + .send() + }) + .await?; + let document: Value = response + .json() + .await + .map_err(|e| RelayError::Http(format!("invalid NIP-11 document: {e}")))?; + normalize_relay_self_pubkey(&document) + } + /// Sign a NIP-98 HTTP Auth event (kind:27235) for the given method/URL/body. /// /// Returns the `Authorization: Nostr ` header value (without the @@ -436,6 +458,22 @@ impl RestClient { } } +/// Validate and canonicalize the NIP-11 relay-info `self` pubkey. +fn normalize_relay_self_pubkey(document: &Value) -> Result { + let self_hex = document + .get("self") + .and_then(Value::as_str) + .ok_or_else(|| RelayError::Http("NIP-11 document missing 'self' field".into()))?; + if self_hex.len() != 64 || !self_hex.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(RelayError::Http(format!( + "NIP-11 'self' field is not a valid 64-hex pubkey: {self_hex}" + ))); + } + PublicKey::from_hex(self_hex) + .map(|pubkey| pubkey.to_hex()) + .map_err(|e| RelayError::Http(format!("invalid NIP-11 'self' pubkey: {e}"))) +} + /// Events the harness cares about. #[derive(Debug, Clone)] pub struct BuzzEvent { @@ -4008,6 +4046,25 @@ async fn wait_for_any_ok( mod tests { use super::*; + #[test] + fn normalize_relay_self_pubkey_accepts_and_canonicalizes_hex() { + let keys = Keys::generate(); + let upper = keys.public_key().to_hex().to_ascii_uppercase(); + let document = serde_json::json!({"self": upper}); + + assert_eq!( + normalize_relay_self_pubkey(&document).expect("valid relay self pubkey"), + keys.public_key().to_hex() + ); + } + + #[test] + fn normalize_relay_self_pubkey_rejects_missing_or_invalid_values() { + assert!(normalize_relay_self_pubkey(&serde_json::json!({})).is_err()); + assert!(normalize_relay_self_pubkey(&serde_json::json!({"self": "not-a-pubkey"})).is_err()); + assert!(normalize_relay_self_pubkey(&serde_json::json!({"self": "z".repeat(64)})).is_err()); + } + #[test] fn relay_ws_to_http_plain() { assert_eq!( diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..d58c308007 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -73,7 +73,7 @@ pub(crate) enum AcpAvailabilityStatus { use crate::{ author_allowed, config::Config, - event_mentions_agent, filter, + effective_inbound_author, event_mentions_agent, filter, relay::{HarnessRelay, RelayEventPublisher}, }; @@ -382,6 +382,18 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> let publisher = relay.event_publisher(); let rest_client = relay.rest_client(); + let trusted_relay_pubkey = match rest_client.relay_self_pubkey().await { + Ok(pubkey) => { + tracing::info!("setup-mode: trusted relay workflow signer: {pubkey}"); + Some(pubkey) + } + Err(error) => { + tracing::warn!( + "setup-mode: failed to resolve trusted relay workflow signer; workflow attribution disabled: {error}" + ); + None + } + }; let channel_info = crate::pool::ChannelInfoResolver::new(channel_info_map, rest_client.clone()); @@ -428,7 +440,8 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> // Apply the same author gate as normal mode so the nudge only goes // to authors the real agent would have answered. Same DM hardening: // in DMs only owner/siblings get a nudge (fail-closed on unknown type). - let author_hex = buzz_event.event.pubkey.to_hex(); + let author_hex = + effective_inbound_author(&buzz_event.event, trusted_relay_pubkey.as_deref()); let is_dm = crate::is_dm_channel(buzz_event.channel_id, &channel_info).await; let allowed = author_allowed( &config.respond_to, diff --git a/crates/buzz-relay/src/workflow_sink.rs b/crates/buzz-relay/src/workflow_sink.rs index 97c31c2561..f561b2c726 100644 --- a/crates/buzz-relay/src/workflow_sink.rs +++ b/crates/buzz-relay/src/workflow_sink.rs @@ -19,6 +19,28 @@ use uuid::Uuid; use crate::handlers::event::dispatch_persistent_event; use crate::state::AppState; +/// Build the relay-signed workflow message tags that identify its human or +/// agent owner and destination channel. +/// +/// The first `p` tag is retained as the legacy author-attribution contract. +/// `actor` is the unambiguous effective-author contract used by consumers that +/// cryptographically verify the relay signature before trusting attribution. +fn workflow_base_tags( + author_pubkey_hex: &str, + channel_id: &str, +) -> Result, ActionSinkError> { + Ok(vec![ + Tag::parse(["p", author_pubkey_hex]) + .map_err(|e| ActionSinkError::EventBuild(format!("p tag: {e}")))?, + Tag::parse(["actor", author_pubkey_hex]) + .map_err(|e| ActionSinkError::EventBuild(format!("actor tag: {e}")))?, + Tag::parse(["h", channel_id]) + .map_err(|e| ActionSinkError::EventBuild(format!("h tag: {e}")))?, + Tag::parse(["buzz:workflow", "true"]) + .map_err(|e| ActionSinkError::EventBuild(format!("workflow tag: {e}")))?, + ]) +} + /// Resolves `@Name` mentions in workflow message text to the pubkeys of the /// channel members they name, so the emitted kind:9 carries the `p` tags that /// ACP agent-wake (`event_mentions_agent`) is gated on. @@ -253,18 +275,12 @@ impl ActionSink for RelayActionSink { // 3. Build kind:9 Nostr event // - Signed by relay keypair (event.pubkey = relay pubkey) // - `p` tag attributes the message to the workflow owner + // - `actor` tag explicitly identifies the workflow owner // - `h` tag scopes to the channel (NIP-29, canonical UUID) // - `buzz:workflow` tag prevents recursive workflow triggering // - one `p` tag per `@Name` that resolves to a channel member, // so mentioned agents are woken (wake is `p`-tag gated) - let mut tags = vec![ - Tag::parse(["p", &author_pubkey_hex]) - .map_err(|e| ActionSinkError::EventBuild(format!("p tag: {e}")))?, - Tag::parse(["h", &channel_id_canonical]) - .map_err(|e| ActionSinkError::EventBuild(format!("h tag: {e}")))?, - Tag::parse(["buzz:workflow", "true"]) - .map_err(|e| ActionSinkError::EventBuild(format!("workflow tag: {e}")))?, - ]; + let mut tags = workflow_base_tags(&author_pubkey_hex, &channel_id_canonical)?; // Resolve `@Name` mentions to channel-member pubkeys and append a // `p` tag for each (skipping the author, already tagged above). A @@ -377,6 +393,19 @@ mod tests { std::iter::repeat_n(nibble, 64).collect() } + #[test] + fn workflow_base_tags_include_actor_and_legacy_attribution() { + let author = pk('a'); + let channel = Uuid::new_v4().to_string(); + let tags = workflow_base_tags(&author, &channel).expect("valid workflow base tags"); + let slices: Vec<&[String]> = tags.iter().map(Tag::as_slice).collect(); + + assert_eq!(slices[0], ["p", author.as_str()]); + assert!(slices.contains(&["actor".to_string(), author.clone()].as_slice())); + assert!(slices.contains(&["h".to_string(), channel].as_slice())); + assert!(slices.contains(&["buzz:workflow".to_string(), "true".to_string()].as_slice())); + } + #[test] fn resolves_exact_member_name() { let members = vec![m("Robby", &pk('a'))]; @@ -707,5 +736,14 @@ mod integration_tests { p_tag_targets.contains(&agent_hex.as_str()), "mentioned member {agent_hex} must be p-tagged so it wakes; got {p_tag_targets:?}" ); + assert!( + stored.event.tags.iter().any(|t| { + let slice = t.as_slice(); + slice.len() == 2 + && slice.first().map(String::as_str) == Some("actor") + && slice.get(1).map(String::as_str) == Some(author_hex.as_str()) + }), + "workflow owner must be explicitly attributed via actor tag" + ); } } diff --git a/implementation-notes.md b/implementation-notes.md new file mode 100644 index 0000000000..527b9747ec --- /dev/null +++ b/implementation-notes.md @@ -0,0 +1,38 @@ +# Implementation Notes + +## 2026-07-29 + +- Keep Bookmark Bee and other managed agents in `owner-only` mode. The fix must + not broaden the inbound author gate to every channel author. +- Add an explicit `actor` tag to relay-signed workflow messages while retaining + the existing first `p` attribution tag for compatibility with current + message consumers. +- Treat a workflow actor as authoritative in `buzz-acp` only when all of these + checks pass: + - the event signer matches the relay `self` pubkey fetched from NIP-11; + - the event signature verifies locally; + - the event is a stream message; + - there is exactly one `["buzz:workflow", "true"]` tag; + - there is exactly one valid 64-hex `actor` tag. +- If NIP-11 is unavailable or malformed, fail closed for workflow attribution. + Normal user-signed messages continue to use `event.pubkey`. +- Apply the same effective-author rule in normal and setup-listener modes so a + scheduled workflow receives either a real agent turn or an honest setup + nudge. +- Existing relay-signed workflow events without the new `actor` tag remain + rejected by `owner-only`. The relay and ACP harness both need the new version + before scheduled wakeups work. +- Deployment order is safe either way. Until both components are updated, the + current fail-closed behavior remains. +- Reject malformed duplicate `actor` or `buzz:workflow` tags, not just duplicate + well-formed tags. Trusting one valid tag while ignoring a second ambiguous tag + would weaken the "exactly one" contract. +- The Nostr pubkey parser accepts some surprising 64-hex fixtures such as all + zeroes and all `f`s, so malformed NIP-11 tests use wrong-length/non-hex input. + The trust boundary requires canonical 64-hex plus an event signature that + verifies under the corresponding relay key. +- Full `just ci` passed on the implementation worktree. `just test` completed + its unit phase, but the integration phase could not start because the local + Docker Desktop daemon was unavailable at + `/Users/justinperea/.docker/run/docker.sock`. The relay's new fast unit test + and all 636 `buzz-acp` library tests passed without Docker. From dd93e0ce5e66cd57dc622a6cf234d7adf1ce0174 Mon Sep 17 00:00:00 2001 From: Justin Perea Date: Tue, 4 Aug 2026 06:57:40 -0400 Subject: [PATCH 2/2] docs: record workflow wake refresh decisions Co-authored-by: Justin Perea Signed-off-by: Justin Perea --- implementation-notes.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/implementation-notes.md b/implementation-notes.md index 527b9747ec..b0c947e0c2 100644 --- a/implementation-notes.md +++ b/implementation-notes.md @@ -36,3 +36,21 @@ Docker Desktop daemon was unavailable at `/Users/justinperea/.docker/run/docker.sock`. The relay's new fast unit test and all 636 `buzz-acp` library tests passed without Docker. + +## 2026-08-04 refresh + +- Preserve the original tested branch and create + `fix/workflow-owner-only-agent-wake-v2` from current `origin/main` instead of + rebasing or force-pushing the existing review artifact. +- The original commit cherry-picked cleanly onto `feccf4eab`; this is a source + refresh only. The trust contract remains unchanged. +- An earlier upstream commit used a similar dedicated `workflow-owner` tag, + but it is not an ancestor of current `origin/main` and the current source no + longer contains that path. Do not treat commit history alone as proof that + the fix ships today. +- Justin's fork `main` remains at the original base commit, so the refreshed + branch must target the fork deliberately and must not be mistaken for a + production deployment. +- Live acceptance still requires two separately deployed artifacts: the relay + must emit the dedicated actor tag and the installed ACP harness must verify + it. A passing source branch cannot substitute for that end-to-end check.