Skip to content

feat(buzz-acp): --allow-dm-allowlist opt-in for explicit allowlist in DMs #5013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joe-rodgers wants to merge 1 commit into
base: main
Choose a base branch
from
+204 −3
Open

feat(buzz-acp): --allow-dm-allowlist opt-in for explicit allowlist in DMs #5013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion crates/buzz-acp/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,16 @@ pub struct CliArgs {
#[arg(long, env = "BUZZ_ACP_RESPOND_TO_ALLOWLIST", value_delimiter = ',')]
pub respond_to_allowlist: Option<Vec<String>>,

/// 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.
Expand Down Expand Up @@ -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<String>,
/// 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<String>,
/// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL, BUZZ_AGENT_MODEL).
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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}"),
};
Expand Down Expand Up @@ -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,
Expand Down
171 changes: 169 additions & 2 deletions crates/buzz-acp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
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,
};
}
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -4456,6 +4484,7 @@ mod author_gate_tests {
&allowlist,
SIBLING,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4474,6 +4503,7 @@ mod author_gate_tests {
&allowlist,
EXTERNAL,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4492,6 +4522,7 @@ mod author_gate_tests {
&allowlist,
STRANGER,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4510,6 +4541,7 @@ mod author_gate_tests {
&allowlist,
OWNER,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4531,6 +4563,7 @@ mod author_gate_tests {
&HashSet::new(),
STRANGER,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4549,6 +4582,7 @@ mod author_gate_tests {
&HashSet::new(),
who,
false,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -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() {
Expand All @@ -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"
);
}

Expand All @@ -4592,6 +4753,7 @@ mod author_gate_tests {
&HashSet::new(),
STRANGER,
true,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4615,6 +4777,7 @@ mod author_gate_tests {
&HashSet::new(),
who,
true,
false,
&cache,
&dummy_rest_client()
)
Expand All @@ -4634,6 +4797,7 @@ mod author_gate_tests {
&HashSet::new(),
OWNER,
true,
false,
&cache,
&dummy_rest_client()
)
Expand Down Expand Up @@ -4771,6 +4935,7 @@ mod author_gate_tests {
&allowlist,
EXTERNAL,
is_dm,
false,
&owner_cache,
&dummy_rest_client(),
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/buzz-acp/src/setup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down