From 56ea9ef0f6909f63514096dbf1c4f9fda34fb44b Mon Sep 17 00:00:00 2001 From: Kanji Kawanabe Date: Sun, 13 Sep 2026 16:43:25 -0600 Subject: [PATCH] fix(acp): admit explicit allowlist authors only in verified DMs Signed-off-by: Kanji Kawanabe (cherry picked from commit 2ac51d96bcbd0572bfc4ce5567f0c34b087a77da) Signed-off-by: Kanji Kawanabe (cherry picked from commit 02e1ffc9763d9d7c4b6e2323fcc2822eca6d6583) Signed-off-by: Kanji Kawanabe --- crates/buzz-acp/README.md | 13 +- .../src/author_gate_tests/dm_allowlist.rs | 112 ++++++++++++++++ crates/buzz-acp/src/config.rs | 2 +- crates/buzz-acp/src/lib.rs | 126 ++++++++++++------ 4 files changed, 208 insertions(+), 45 deletions(-) create mode 100644 crates/buzz-acp/src/author_gate_tests/dm_allowlist.rs diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index 3d011eb8ebb..42d11723b89 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -144,9 +144,18 @@ Controls which authors' events the harness forwards to the agent. Events from di |------|----------| | `owner-only` | Forward only events from the agent's registered owner. If no owner is set, all events are dropped until the owner is resolved. | | `allowlist` | Forward events from the listed pubkeys plus the owner. | -| `anyone` | Forward all events (no author filtering). | +| `anyone` | Forward all authors in verified non-DM channels; DMs remain owner/sibling-only. | | `nobody` | Drop all inbound events. Agent only acts on heartbeat prompts. | +In a verified DM, `allowlist` admits explicitly listed public keys plus the owner +and verified sibling agents. `owner-only` and `anyone` admit only owner/siblings; +`nobody` denies everyone. Removing a key from the effective allowlist revokes its +admission. Missing, failed, or unrecognized channel metadata never grants an +external author access, including under `allowlist` or `anyone`; resolution is +retried for later events. This policy is shared by normal and setup listeners. +Signature checks, relay membership, workflow attribution, and session/audience +boundaries still apply. + Relay-signed workflow messages delegate to their recorded owner only when they explicitly target this agent with authenticated workflow-mention provenance. The owner tag means that owner scheduled the workflow; it does not claim that @@ -173,7 +182,7 @@ buzz messages send --channel --reply-to \ --mention --content '!cancel' ``` -> **Note:** The default mode is `owner-only`. Agents without a registered `agent_owner_pubkey` will not respond to any events until the owner is resolved. Set `--respond-to anyone` to disable the gate entirely. +> **Note:** The default mode is `owner-only`. Agents without a registered `agent_owner_pubkey` will not respond to any events until the owner is resolved. `--respond-to anyone` broadens verified non-DM channels only. **Examples:** diff --git a/crates/buzz-acp/src/author_gate_tests/dm_allowlist.rs b/crates/buzz-acp/src/author_gate_tests/dm_allowlist.rs new file mode 100644 index 00000000000..54a1821b285 --- /dev/null +++ b/crates/buzz-acp/src/author_gate_tests/dm_allowlist.rs @@ -0,0 +1,112 @@ +//! Exercise real signed human messages through both production listener boundaries. +use super::*; + +#[tokio::test] +async fn signed_dm_policy_matrix_and_revocation() { + let (metadata_rest, metadata_server) = nip11_server(serde_json::json!([])).await; + for listener in [ListenerBoundary::Normal, ListenerBoundary::Setup] { + let sender = nostr::Keys::generate(); + let author = sender.public_key().to_hex(); + let agent = nostr::Keys::generate().public_key().to_hex(); + let relay = nostr::Keys::generate().public_key().to_hex(); + let (mut gate, rest, server) = connected_gate(&relay, &agent).await; + for channel_type in ["dm", "unknown", "unexpected", "stream", "forum"] { + let channel_id = Uuid::new_v4(); + let channel_info = pool::ChannelInfoResolver::new( + HashMap::from([( + channel_id, + relay::ChannelInfo { + name: "policy test".into(), + channel_type: channel_type.into(), + description: None, + }, + )]), + metadata_rest.clone(), + ); + for mode in [ + RespondTo::Allowlist, + RespondTo::OwnerOnly, + RespondTo::Anyone, + RespondTo::Nobody, + ] { + for principal in ["external", "owner", "sibling"] { + let cache = OwnerCache::new(Some(if principal == "owner" { + author.clone() + } else { + nostr::Keys::generate().public_key().to_hex() + })); + cache.cache_sibling(author.clone(), principal == "sibling"); + // The same gate sees admission, removal, and re-addition. + for listed in [true, false, true] { + let allowlist = if listed { + HashSet::from([author.clone()]) + } else { + HashSet::new() + }; + let event = nostr::EventBuilder::new( + nostr::Kind::Custom(KIND_STREAM_MESSAGE as u16), + "Please reply in this conversation", + ) + .tags([ + nostr::Tag::parse(["h", &channel_id.to_string()]).unwrap(), + nostr::Tag::parse(["p", &agent]).unwrap(), + ]) + .sign_with_keys(&sender) + .unwrap(); + event.verify().unwrap(); + let event = relay::BuzzEvent { + connection_generation: 0, + channel_id, + event, + }; + let authorized = match listener { + ListenerBoundary::Normal => { + authorize_normal_listener_event( + &mut gate, + event, + &mode, + &allowlist, + &cache, + &channel_info, + &rest, + ) + .await + } + ListenerBoundary::Setup => { + setup_mode::authorize_setup_listener_event( + &mut gate, + event, + &mode, + &allowlist, + &cache, + &channel_info, + &rest, + ) + .await + } + }; + let known = matches!(channel_type, "dm" | "stream" | "forum"); + let expected = match mode { + RespondTo::Nobody => false, + _ if principal != "external" => true, + RespondTo::Allowlist => known && listed, + RespondTo::Anyone => matches!(channel_type, "stream" | "forum"), + RespondTo::OwnerOnly => false, + }; + assert_eq!( + authorized.is_some(), + expected, + "{} {channel_type} {mode} {principal} listed={listed}", + listener.name() + ); + if let Some(authorized) = authorized { + assert_eq!(authorized.into_parts().1, author); + } + } + } + } + } + server.abort(); + } + metadata_server.abort(); +} diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index b4d27903c62..0e3551e2efb 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -89,7 +89,7 @@ pub enum MultipleEventHandling { /// /// - `owner-only` — only the agent's registered owner (default). /// - `allowlist` — owner + explicit pubkey list (`--respond-to-allowlist`). -/// - `anyone` — all events forwarded (no author filtering). +/// - `anyone` — all verified non-DM channels; DMs stay owner/sibling-only. /// - `nobody` — all events dropped (proactive/heartbeat-only mode). #[derive(Debug, Clone, Default, PartialEq, Eq, Hash, clap::ValueEnum)] pub enum RespondTo { diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 6c272187f3c..acf34085ac6 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -336,8 +336,8 @@ fn effective_prompt_author( /// dropping the load inside it fails the construction regressions. mod inbound_author_gate { use super::{ - effective_prompt_author, is_dm_channel, is_owner_or_sibling, pool, refresh_relay_self, - relay, OwnerCache, RespondTo, + effective_prompt_author, is_owner_or_sibling, pool, refresh_relay_self, relay, OwnerCache, + RespondTo, }; use std::collections::HashSet; @@ -365,6 +365,24 @@ mod inbound_author_gate { } } + #[derive(Clone, Copy)] + enum ChannelTrust { + Unknown, + Dm, + Channel, + } + + impl ChannelTrust { + #[cfg(test)] + fn known(is_dm: bool) -> Self { + if is_dm { + Self::Dm + } else { + Self::Channel + } + } + } + /// Apply the configured raw-author policy after trusted workflow attribution. /// /// This stays private to the gate module so neither listener can bypass @@ -373,24 +391,20 @@ mod inbound_author_gate { respond_to: &RespondTo, allowlist: &HashSet, author: &str, - is_dm: bool, + channel_trust: ChannelTrust, owner_cache: &OwnerCache, rest_client: &relay::RestClient, ) -> bool { - if is_dm { - return match respond_to { - RespondTo::Nobody => false, - _ => is_owner_or_sibling(author, owner_cache, rest_client).await, - }; - } match respond_to { - RespondTo::Anyone => true, RespondTo::Nobody => false, - RespondTo::OwnerOnly => is_owner_or_sibling(author, owner_cache, rest_client).await, - RespondTo::Allowlist => { - allowlist.contains(author) - || is_owner_or_sibling(author, owner_cache, rest_client).await + RespondTo::Anyone if matches!(channel_trust, ChannelTrust::Channel) => true, + RespondTo::Allowlist + if !matches!(channel_trust, ChannelTrust::Unknown) + && allowlist.contains(author) => + { + true } + _ => is_owner_or_sibling(author, owner_cache, rest_client).await, } } @@ -407,7 +421,7 @@ mod inbound_author_gate { respond_to, allowlist, author, - is_dm, + ChannelTrust::known(is_dm), owner_cache, rest_client, ) @@ -482,12 +496,23 @@ mod inbound_author_gate { self.refreshed_generation = Some(buzz_event.connection_generation); } } - let is_dm = is_dm_channel(buzz_event.channel_id, channel_info).await; + // Unknown metadata must not inherit the explicit DM allowlist exception. + let channel_trust = match channel_info + .resolve_channel_metadata(buzz_event.channel_id) + .await + { + Some(info) => match info.channel_type.as_str() { + "dm" => ChannelTrust::Dm, + "stream" | "forum" => ChannelTrust::Channel, + _ => ChannelTrust::Unknown, + }, + None => ChannelTrust::Unknown, + }; self.evaluate_with_channel_trust( &buzz_event.event, respond_to, allowlist, - is_dm, + channel_trust, owner_cache, rest_client, ) @@ -499,7 +524,7 @@ mod inbound_author_gate { event: &nostr::Event, respond_to: &RespondTo, allowlist: &HashSet, - is_dm: bool, + channel_trust: ChannelTrust, owner_cache: &OwnerCache, rest_client: &relay::RestClient, ) -> InboundAuthorGateDecision { @@ -509,7 +534,7 @@ mod inbound_author_gate { respond_to, allowlist, &effective_author, - is_dm, + channel_trust, owner_cache, rest_client, ) @@ -517,7 +542,7 @@ mod inbound_author_gate { InboundAuthorGateDecision { effective_author, allowed, - is_dm, + is_dm: !matches!(channel_trust, ChannelTrust::Channel), } } @@ -571,7 +596,7 @@ mod inbound_author_gate { event, respond_to, allowlist, - is_dm, + ChannelTrust::known(is_dm), owner_cache, rest_client, ) @@ -6735,6 +6760,8 @@ mod workflow_owner_tests { mod author_gate_tests { use super::*; + mod dm_allowlist; + /// A `RestClient` for tests. The author-gate decisions exercised here all /// resolve from the owner pubkey or sibling cache before any HTTP call, so /// this client is never actually used to make a request. @@ -7034,7 +7061,8 @@ mod author_gate_tests { /// Both production boundaries must retain DM classification when composing /// trusted workflow attribution with configured author policy. External - /// allowlist entries and `Anyone` stay denied in a DM; owner and sibling + /// allowlist entries are admitted only in verified DMs; `Anyone` stays + /// owner/sibling-only. Owner and sibling /// principals remain allowed; `Nobody` remains absolute. #[tokio::test] async fn production_listener_boundaries_enforce_dm_author_policy() { @@ -7043,7 +7071,7 @@ mod author_gate_tests { let relay_hex = relay_keys.public_key().to_hex(); let external = nostr::Keys::generate().public_key().to_hex(); let external_allowlist = HashSet::from([external.clone()]); - let denied_external = listener_boundary_scenario(ListenerBoundaryScenario { + let allowed_external = listener_boundary_scenario(ListenerBoundaryScenario { listener, relay_keys: &relay_keys, workflow_owner: &external, @@ -7059,8 +7087,8 @@ mod author_gate_tests { }) .await; assert!( - !denied_external.1, - "{} listener must deny an external allowlist entry in a DM", + allowed_external.1, + "{} listener must admit an explicit allowlist entry in a verified DM", listener.name() ); @@ -7842,15 +7870,15 @@ mod author_gate_tests { // // In a DM, clients auto-p-tag every participant, and an agent can be // asked to open a DM with a third party. The gate must therefore ignore - // the allowlist and `anyone` mode inside DMs: only owner + verified - // siblings fire turns. + // implicit mentions and `anyone` mode inside DMs. Only owner, verified + // siblings, or explicitly allowlisted principals in verified DMs fire turns. #[tokio::test] - async fn test_dm_rejects_allowlisted_external_pubkey() { + async fn test_dm_admits_allowlisted_external_pubkey() { let cache = cache_with_sibling(); let allowlist = HashSet::from([EXTERNAL.to_string()]); assert!( - !inbound_author_gate::test_author_allowed( + inbound_author_gate::test_author_allowed( &RespondTo::Allowlist, &allowlist, EXTERNAL, @@ -7859,7 +7887,7 @@ mod author_gate_tests { &dummy_rest_client() ) .await, - "an allowlisted external pubkey must NOT fire a turn inside a DM" + "an allowlisted external pubkey may fire a turn inside a verified DM" ); } @@ -8043,23 +8071,37 @@ mod author_gate_tests { let id = Uuid::new_v4(); let discovered = relay::merge_discovered_channels(vec![id], &serde_json::json!([])); let channel_info = resolver(discovered); - let owner_cache = cache_with_sibling(); - let allowlist = HashSet::from([EXTERNAL.to_string()]); - - let is_dm = is_dm_channel(id, &channel_info).await; - assert!(is_dm, "unknown startup metadata must fail closed as DM"); - assert!( - !inbound_author_gate::test_author_allowed( + let sender = nostr::Keys::generate(); + let author = sender.public_key().to_hex(); + let owner_cache = OwnerCache::new(None); + let allowlist = HashSet::from([author]); + let agent = nostr::Keys::generate().public_key().to_hex(); + let relay = nostr::Keys::generate().public_key().to_hex(); + let (mut gate, rest, server) = connected_gate(&relay, &agent).await; + let event = relay::BuzzEvent { + connection_generation: 0, + channel_id: id, + event: nostr::EventBuilder::new(nostr::Kind::TextNote, "hello") + .sign_with_keys(&sender) + .unwrap(), + }; + assert!(is_dm_channel(id, &channel_info).await); + let decision = gate + .evaluate_listener_event( + &event, &RespondTo::Allowlist, &allowlist, - EXTERNAL, - is_dm, &owner_cache, - &dummy_rest_client(), + &channel_info, + &rest, ) - .await, - "an external author must not pass when startup discovery omitted metadata" + .await; + assert!(decision.is_dm); + assert!( + !decision.allowed, + "missing metadata must not grant the DM exception" ); + server.abort(); } #[tokio::test]