diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index dab61be30a..5a88709190 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -458,6 +458,16 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_RESPOND_TO_ALLOWLIST", value_delimiter = ',')] pub respond_to_allowlist: Option>, + /// Opt in to letting an *explicit* `--respond-to-allowlist` also apply + /// inside DMs. Default off. + /// + /// Without this, DMs are owner/sibling-only regardless of `--respond-to` + /// (see the DM hardening notes on the inbound author gate). With it, + /// `--respond-to=allowlist` additionally admits the listed pubkeys in DMs. + /// It never loosens `--respond-to=anyone` or `--respond-to=nobody`. + #[arg(long, env = "BUZZ_ACP_ALLOW_DM_ALLOWLIST")] + pub allow_dm_allowlist: bool, + /// Comma-separated list of allowed `--respond-to` modes. /// When set, the harness rejects startup if `--respond-to` is not in this list. /// Modes: owner-only, allowlist, anyone, nobody. @@ -537,6 +547,10 @@ pub struct Config { pub respond_to: RespondTo, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). pub respond_to_allowlist: HashSet, + /// Whether the explicit `respond_to_allowlist` also applies inside DMs. + /// Opt-in via `--allow-dm-allowlist` / `BUZZ_ACP_ALLOW_DM_ALLOWLIST`. + /// Never affects `RespondTo::Anyone` or `RespondTo::Nobody`. + pub allow_dm_allowlist: bool, /// Allowed `respond_to` modes. Empty = all modes allowed. pub allowed_respond_to: Vec, /// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL, BUZZ_AGENT_MODEL). @@ -1010,6 +1024,15 @@ impl Config { HashSet::new() }; + // The DM allowlist opt-in is only meaningful for allowlist mode; it is + // deliberately inert (and never loosening) under anyone/nobody/owner-only. + let allow_dm_allowlist = args.allow_dm_allowlist; + if allow_dm_allowlist && args.respond_to != RespondTo::Allowlist { + tracing::warn!( + "--allow-dm-allowlist has no effect when --respond-to is not 'allowlist'" + ); + } + // Validate respond_to against the allowed set. let allowed_respond_to = if let Some(raw) = args.allowed_respond_to { // Validate each entry is a known RespondTo mode. @@ -1094,6 +1117,7 @@ impl Config { permission_mode: args.permission_mode, respond_to: args.respond_to, respond_to_allowlist, + allow_dm_allowlist, allowed_respond_to, persona_env_vars, has_generated_codex_config, @@ -1111,7 +1135,15 @@ impl Config { pub fn summary(&self) -> String { let respond_to_detail = match &self.respond_to { RespondTo::Allowlist => { - format!("respond_to=allowlist({})", self.respond_to_allowlist.len()) + let dm = if self.allow_dm_allowlist { + " dm_allowlist=on" + } else { + "" + }; + format!( + "respond_to=allowlist({}){dm}", + self.respond_to_allowlist.len() + ) } other => format!("respond_to={other}"), }; @@ -1464,6 +1496,7 @@ mod tests { permission_mode: PermissionMode::BypassPermissions, respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), + allow_dm_allowlist: false, allowed_respond_to: Vec::new(), persona_env_vars: vec![], has_generated_codex_config: false, diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..c557e6bbe4 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -232,17 +232,44 @@ async fn is_owner_or_sibling( /// siblings may fire a turn — the explicit allowlist and `anyone` mode do /// NOT apply inside DMs. `Nobody` still drops everything. Callers must /// resolve `is_dm` fail-closed: unknown channel type ⇒ treat as DM. +/// +/// # DM allowlist opt-in (`allow_dm_allowlist`) +/// +/// Owner-only DMs are the safe default, but they are surprising when the +/// operator has *deliberately* named a human on `--respond-to-allowlist`: +/// that person can talk to the agent in a stream channel and then finds +/// every DM silently dropped, which is indistinguishable from a broken +/// agent. `--allow-dm-allowlist` opts a deployment out of that surprise: +/// when it is set *and* the mode is `Allowlist`, the explicit pubkey list +/// also admits authors inside DMs. +/// +/// The tradeoff being accepted: inside a DM, an allowlisted pubkey can +/// prompt the agent with no channel-membership context around it — no +/// stream, no other participants, no audit trail anyone else can see. The +/// transitive-grant hole stays closed, because admission is still gated on +/// an explicit list the operator wrote by hand; landing in a DM with the +/// agent grants nothing by itself. That is exactly why the opt-in is scoped +/// to `Allowlist` alone: `Anyone` inside a DM would restore the original +/// hole (any pubkey that gets into a DM could prompt the agent), so it +/// remains owner/sibling-only regardless of this flag, and `Nobody` still +/// drops everything including the owner. async fn author_allowed( respond_to: &RespondTo, allowlist: &HashSet, author: &str, is_dm: bool, + allow_dm_allowlist: bool, owner_cache: &OwnerCache, rest_client: &relay::RestClient, ) -> bool { if is_dm { return match respond_to { RespondTo::Nobody => false, + // Opt-in only, and only for the explicit list — never `Anyone`. + RespondTo::Allowlist if allow_dm_allowlist => { + allowlist.contains(author) + || is_owner_or_sibling(author, owner_cache, rest_client).await + } _ => is_owner_or_sibling(author, owner_cache, rest_client).await, }; } @@ -2154,6 +2181,7 @@ async fn tokio_main() -> Result<()> { &config.respond_to_allowlist, &author, is_dm, + config.allow_dm_allowlist, &owner_cache, &ctx.rest_client, ) @@ -4456,6 +4484,7 @@ mod author_gate_tests { &allowlist, SIBLING, false, + false, &cache, &dummy_rest_client() ) @@ -4474,6 +4503,7 @@ mod author_gate_tests { &allowlist, EXTERNAL, false, + false, &cache, &dummy_rest_client() ) @@ -4492,6 +4522,7 @@ mod author_gate_tests { &allowlist, STRANGER, false, + false, &cache, &dummy_rest_client() ) @@ -4510,6 +4541,7 @@ mod author_gate_tests { &allowlist, OWNER, false, + false, &cache, &dummy_rest_client() ) @@ -4531,6 +4563,7 @@ mod author_gate_tests { &HashSet::new(), STRANGER, false, + false, &cache, &dummy_rest_client() ) @@ -4549,6 +4582,7 @@ mod author_gate_tests { &HashSet::new(), who, false, + false, &cache, &dummy_rest_client() ) @@ -4563,7 +4597,10 @@ 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. + // siblings fire turns. `--allow-dm-allowlist` (the `allow_dm_allowlist` + // argument) opts back in to the *explicit* allowlist inside DMs, and + // nothing else; the flag-off cases below are the default behaviour and + // must not change. #[tokio::test] async fn test_dm_rejects_allowlisted_external_pubkey() { @@ -4575,11 +4612,135 @@ mod author_gate_tests { &allowlist, EXTERNAL, true, + // allow_dm_allowlist OFF — the default. + false, &cache, &dummy_rest_client() ) .await, - "an allowlisted external pubkey must NOT fire a turn inside a DM" + "by default an allowlisted external pubkey must NOT fire a turn inside a DM" + ); + } + + #[tokio::test] + async fn test_dm_allowlist_optin_admits_allowlisted_external_pubkey() { + let cache = cache_with_sibling(); + let allowlist = HashSet::from([EXTERNAL.to_string()]); + assert!( + author_allowed( + &RespondTo::Allowlist, + &allowlist, + EXTERNAL, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "with allow_dm_allowlist on, an explicitly allowlisted pubkey must fire a turn in a DM" + ); + } + + #[tokio::test] + async fn test_dm_allowlist_optin_still_rejects_unlisted_stranger() { + let cache = cache_with_sibling(); + let allowlist = HashSet::from([EXTERNAL.to_string()]); + assert!( + !author_allowed( + &RespondTo::Allowlist, + &allowlist, + STRANGER, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "allow_dm_allowlist must only admit the explicit list — a stranger stays dropped in a DM" + ); + } + + #[tokio::test] + async fn test_dm_allowlist_optin_does_not_loosen_anyone() { + let cache = cache_with_sibling(); + // A non-empty list is present but the mode is `anyone`: neither the + // listed pubkey nor an unlisted stranger may pass inside a DM. + let allowlist = HashSet::from([EXTERNAL.to_string()]); + for who in [STRANGER, EXTERNAL] { + assert!( + !author_allowed( + &RespondTo::Anyone, + &allowlist, + who, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "respond_to=anyone in a DM must stay owner/sibling-only even with allow_dm_allowlist on" + ); + } + } + + #[tokio::test] + async fn test_dm_allowlist_optin_does_not_loosen_nobody() { + let cache = cache_with_sibling(); + let allowlist = HashSet::from([EXTERNAL.to_string(), OWNER.to_string()]); + for who in [OWNER, SIBLING, EXTERNAL, STRANGER] { + assert!( + !author_allowed( + &RespondTo::Nobody, + &allowlist, + who, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "respond_to=nobody must drop everything in a DM even with allow_dm_allowlist on" + ); + } + } + + #[tokio::test] + async fn test_dm_allowlist_optin_still_admits_owner_and_sibling() { + let cache = cache_with_sibling(); + let allowlist = HashSet::from([EXTERNAL.to_string()]); + for (who, label) in [(OWNER, "owner"), (SIBLING, "sibling")] { + assert!( + author_allowed( + &RespondTo::Allowlist, + &allowlist, + who, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "the {label} must remain admitted in a DM with allow_dm_allowlist on" + ); + } + } + + #[tokio::test] + async fn test_dm_allowlist_optin_does_not_affect_owner_only_mode() { + let cache = cache_with_sibling(); + let allowlist = HashSet::from([EXTERNAL.to_string()]); + assert!( + !author_allowed( + &RespondTo::OwnerOnly, + &allowlist, + EXTERNAL, + true, + true, + &cache, + &dummy_rest_client() + ) + .await, + "respond_to=owner-only must ignore the allowlist in a DM even with allow_dm_allowlist on" ); } @@ -4592,6 +4753,7 @@ mod author_gate_tests { &HashSet::new(), STRANGER, true, + false, &cache, &dummy_rest_client() ) @@ -4615,6 +4777,7 @@ mod author_gate_tests { &HashSet::new(), who, true, + false, &cache, &dummy_rest_client() ) @@ -4634,6 +4797,7 @@ mod author_gate_tests { &HashSet::new(), OWNER, true, + false, &cache, &dummy_rest_client() ) @@ -4771,6 +4935,7 @@ mod author_gate_tests { &allowlist, EXTERNAL, is_dm, + false, &owner_cache, &dummy_rest_client(), ) @@ -5027,6 +5192,7 @@ mod build_mcp_servers_tests { permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), + allow_dm_allowlist: false, allowed_respond_to: vec![], persona_env_vars: vec![], has_generated_codex_config: false, @@ -5248,6 +5414,7 @@ mod error_outcome_emission_tests { permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), + allow_dm_allowlist: false, allowed_respond_to: vec![], persona_env_vars: vec![], has_generated_codex_config: false, diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..a42a1891ec 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -435,6 +435,7 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> &config.respond_to_allowlist, &author_hex, is_dm, + config.allow_dm_allowlist, &owner_cache, &rest_client, )