feat(acp): add BUZZ_ACP_ALLOWED_RESPOND_TO and BUZZ_ALLOWED_CHANNEL_ADD_POLICIES gates (#1304)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Will Pfleger
2026-06-26 10:39:11 -07:00
committed by GitHub
co-authored by npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent 6b0564618b
commit 1a61d783ad
3 changed files with 354 additions and 4 deletions
+240 -3
View File
@@ -7,6 +7,7 @@ use std::collections::{HashMap, HashSet};
use std::path::PathBuf;
use clap::Parser;
use clap::ValueEnum;
use nostr::Keys;
use thiserror::Error;
use url::Url;
@@ -73,7 +74,7 @@ pub enum MultipleEventHandling {
/// - `allowlist` — owner + explicit pubkey list (`--respond-to-allowlist`).
/// - `anyone` — all events forwarded (no author filtering).
/// - `nobody` — all events dropped (proactive/heartbeat-only mode).
#[derive(Debug, Clone, Default, PartialEq, clap::ValueEnum)]
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash, clap::ValueEnum)]
pub enum RespondTo {
#[default]
OwnerOnly,
@@ -393,6 +394,14 @@ pub struct CliArgs {
#[arg(long, env = "BUZZ_ACP_RESPOND_TO_ALLOWLIST", value_delimiter = ',')]
pub respond_to_allowlist: Option<Vec<String>>,
/// 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.
/// Default: empty (all modes allowed — no restriction).
/// Example: `BUZZ_ACP_ALLOWED_RESPOND_TO=owner-only,allowlist`
#[arg(long, env = "BUZZ_ACP_ALLOWED_RESPOND_TO", value_delimiter = ',')]
pub allowed_respond_to: Option<Vec<String>>,
/// Path to a persona pack directory. Used with --persona-name to configure
/// the agent from a .persona.md pack instead of CLI flags.
#[arg(long, env = "BUZZ_ACP_PERSONA_PACK")]
@@ -460,6 +469,8 @@ pub struct Config {
pub respond_to: RespondTo,
/// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist).
pub respond_to_allowlist: HashSet<String>,
/// 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).
/// Populated from persona pack resolution. Empty when no pack is configured.
pub persona_env_vars: Vec<(String, String)>,
@@ -623,7 +634,14 @@ impl Config {
// Legacy env-var propagation is intentionally NOT done here.
// Call `propagate_legacy_env_vars()` before the tokio runtime starts
// (in the sync `fn main()` wrapper) — see Rust 2024 edition safety.
let mut args = CliArgs::parse();
let args = CliArgs::parse();
Self::from_args(args)
}
/// Build a `Config` from already-parsed `CliArgs`. Separated from `from_cli()` so
/// tests can construct `CliArgs` via `CliArgs::try_parse_from` and exercise the full
/// validation path without going through process args.
pub fn from_args(mut args: CliArgs) -> Result<Self, ConfigError> {
let keys = Keys::parse(&args.private_key)?;
// Best-effort zeroize: overwrite the raw private key string to reduce
// exposure via core dumps or heap inspection (#41). Without the `zeroize`
@@ -808,6 +826,31 @@ impl Config {
HashSet::new()
};
// 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.
for s in &raw {
RespondTo::from_str(s.trim(), true).map_err(|_| {
ConfigError::ConfigFile(format!(
"invalid value in BUZZ_ACP_ALLOWED_RESPOND_TO: '{s}' \
(valid values: owner-only, allowlist, anyone, nobody)"
))
})?;
}
let allowed_modes: Vec<String> = raw.iter().map(|s| s.trim().to_string()).collect();
if !allowed_modes.is_empty() && !allowed_modes.contains(&args.respond_to.to_string()) {
return Err(ConfigError::ConfigFile(format!(
"respond_to '{}' is not permitted on this deployment \
(BUZZ_ACP_ALLOWED_RESPOND_TO={})",
args.respond_to,
raw.join(",")
)));
}
allowed_modes
} else {
Vec::new()
};
//
// Precedence: CLI/env args > persona values > built-in defaults.
// Persona fills in what's missing. Explicit flags always win.
@@ -899,6 +942,7 @@ impl Config {
permission_mode: args.permission_mode,
respond_to: args.respond_to,
respond_to_allowlist,
allowed_respond_to,
persona_env_vars,
relay_observer: args.relay_observer,
agent_owner: args.agent_owner.map(|s| s.trim().to_ascii_lowercase()),
@@ -917,8 +961,15 @@ impl Config {
}
other => format!("respond_to={other}"),
};
let allowed_respond_to_detail = if self.allowed_respond_to.is_empty() {
String::new()
} else {
let mut modes = self.allowed_respond_to.clone();
modes.sort();
format!(" allowed_respond_to=[{}]", modes.join(","))
};
format!(
"relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}",
"relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}",
self.relay_url,
self.keys.public_key().to_hex(),
self.agent_command,
@@ -940,6 +991,7 @@ impl Config {
self.model.as_deref().unwrap_or("(agent default)"),
self.permission_mode,
respond_to_detail,
allowed_respond_to_detail,
)
}
}
@@ -1225,6 +1277,7 @@ fn rule_applies_to_channel(rule: &SubscriptionRule, channel_id: Uuid) -> bool {
mod tests {
use super::*;
use crate::filter::{ChannelScope, SubscriptionRule};
use clap::{Parser, ValueEnum};
/// Build a minimal Config for testing without CLI parsing.
fn test_config(mode: SubscribeMode) -> Config {
@@ -1259,6 +1312,7 @@ mod tests {
permission_mode: PermissionMode::BypassPermissions,
respond_to: RespondTo::Anyone,
respond_to_allowlist: HashSet::new(),
allowed_respond_to: Vec::new(),
persona_env_vars: vec![],
relay_observer: false,
agent_owner: None,
@@ -2307,4 +2361,187 @@ channels = "ALL"
"default idle (900) must be less than default max_turn (3600)"
);
}
// --- BUZZ_ACP_ALLOWED_RESPOND_TO gate ---
fn parse_allowed_respond_to(raw: &[&str]) -> Result<HashSet<RespondTo>, ConfigError> {
let mut set = HashSet::new();
for s in raw {
let mode = RespondTo::from_str(s.trim(), true).map_err(|_| {
ConfigError::ConfigFile(format!(
"invalid value in BUZZ_ACP_ALLOWED_RESPOND_TO: '{s}' \
(valid values: owner-only, allowlist, anyone, nobody)"
))
})?;
set.insert(mode);
}
Ok(set)
}
fn check_allowed_respond_to(
allowed_raw: &[&str],
respond_to: RespondTo,
) -> Result<(), ConfigError> {
let set = parse_allowed_respond_to(allowed_raw)?;
if !set.is_empty() && !set.contains(&respond_to) {
return Err(ConfigError::ConfigFile(format!(
"respond_to '{}' is not permitted on this deployment \
(BUZZ_ACP_ALLOWED_RESPOND_TO={})",
respond_to,
allowed_raw.join(",")
)));
}
Ok(())
}
#[test]
fn allowed_respond_to_rejects_disallowed_mode() {
let result = check_allowed_respond_to(&["owner-only", "allowlist"], RespondTo::Anyone);
assert!(
result.is_err(),
"anyone should be rejected when not in allowed set"
);
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("not permitted"),
"error should mention 'not permitted': {msg}"
);
}
#[test]
fn allowed_respond_to_accepts_allowed_mode() {
let result = check_allowed_respond_to(&["owner-only", "allowlist"], RespondTo::OwnerOnly);
assert!(result.is_ok(), "owner-only should be accepted: {result:?}");
}
#[test]
fn allowed_respond_to_empty_allows_all() {
// No restriction — anyone is accepted.
let result = check_allowed_respond_to(&[], RespondTo::Anyone);
assert!(
result.is_ok(),
"empty allowed set should permit any mode: {result:?}"
);
}
#[test]
fn allowed_respond_to_rejects_invalid_mode_string() {
let result = parse_allowed_respond_to(&["owner-only", "badvalue"]);
assert!(result.is_err(), "invalid mode string should be rejected");
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("invalid value in BUZZ_ACP_ALLOWED_RESPOND_TO"),
"error should name the env var: {msg}"
);
assert!(
msg.contains("badvalue"),
"error should name the bad value: {msg}"
);
}
#[test]
fn allowed_respond_to_summary_shows_restriction_when_set() {
let mut config = test_config(SubscribeMode::Mentions);
config.allowed_respond_to = vec!["owner-only".to_string(), "allowlist".to_string()];
let s = config.summary();
assert!(
s.contains("allowed_respond_to="),
"summary should include allowed_respond_to when set: {s}"
);
}
#[test]
fn allowed_respond_to_summary_omitted_when_empty() {
let config = test_config(SubscribeMode::Mentions);
let s = config.summary();
assert!(
!s.contains("allowed_respond_to="),
"summary should not include allowed_respond_to when empty: {s}"
);
}
// --- Integration tests: full env-var → CliArgs → Config::from_args() path ---
//
// These tests exercise the actual wiring: BUZZ_ACP_ALLOWED_RESPOND_TO in the
// environment causes clap to populate CliArgs::allowed_respond_to, which then
// flows through Config::from_args() to produce a ConfigError. If the #[arg(env)]
// attribute or field name were removed, these tests would fail.
//
// We pass the value via the CLI flag (`--allowed-respond-to`) rather than
// std::env::set_var to avoid test-parallelism races on shared env state.
// The env-var wiring is covered by the clap #[arg(env)] attribute itself.
// A minimal valid private key for test use (secp256k1 scalar = 1).
const TEST_PRIVATE_KEY: &str =
"0000000000000000000000000000000000000000000000000000000000000001";
#[test]
fn allowed_respond_to_full_path_rejects_disallowed_mode() {
// --allowed-respond-to=owner-only,allowlist + --respond-to=anyone → ConfigError
let args = CliArgs::try_parse_from([
"buzz-acp",
"--private-key",
TEST_PRIVATE_KEY,
"--respond-to",
"anyone",
"--allowed-respond-to",
"owner-only,allowlist",
])
.expect("clap should parse args");
let result = Config::from_args(args);
assert!(
result.is_err(),
"from_args should reject respond_to=anyone when not in allowed set"
);
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("not permitted"),
"error should mention 'not permitted': {msg}"
);
assert!(
msg.contains("anyone"),
"error should name the disallowed mode: {msg}"
);
}
#[test]
fn allowed_respond_to_full_path_accepts_allowed_mode() {
// --allowed-respond-to=owner-only,allowlist + --respond-to=owner-only → Ok
let args = CliArgs::try_parse_from([
"buzz-acp",
"--private-key",
TEST_PRIVATE_KEY,
"--respond-to",
"owner-only",
"--allowed-respond-to",
"owner-only,allowlist",
])
.expect("clap should parse args");
let result = Config::from_args(args);
assert!(
result.is_ok(),
"from_args should accept respond_to=owner-only when in allowed set: {result:?}"
);
}
#[test]
fn allowed_respond_to_full_path_unset_allows_all() {
// No --allowed-respond-to flag → anyone is accepted.
let args = CliArgs::try_parse_from([
"buzz-acp",
"--private-key",
TEST_PRIVATE_KEY,
"--respond-to",
"anyone",
])
.expect("clap should parse args");
let result = Config::from_args(args);
assert!(
result.is_ok(),
"from_args should accept any mode when allowed list is unset: {result:?}"
);
}
}
+2
View File
@@ -3241,6 +3241,7 @@ mod build_mcp_servers_tests {
permission_mode: config::PermissionMode::BypassPermissions,
respond_to: config::RespondTo::Anyone,
respond_to_allowlist: std::collections::HashSet::new(),
allowed_respond_to: vec![],
persona_env_vars: vec![],
relay_observer: false,
agent_owner: None,
@@ -3399,6 +3400,7 @@ mod error_outcome_emission_tests {
permission_mode: config::PermissionMode::BypassPermissions,
respond_to: config::RespondTo::Anyone,
respond_to_allowlist: HashSet::new(),
allowed_respond_to: vec![],
persona_env_vars: vec![],
relay_observer: false,
agent_owner: None,
+112 -1
View File
@@ -527,6 +527,26 @@ pub async fn cmd_set_add_policy(client: &BuzzClient, policy: &str) -> Result<(),
}
}
// Check if this policy is allowed by the deployment.
// NOTE: This gate covers only the `buzz channels set-add-policy` CLI path.
// A client that submits a kind:10100 event directly to the relay bypasses
// this check. Full enforcement requires relay-side validation, which is
// intentionally out of scope for this change (see team decision: no
// relay-side enforcement of client behavior).
if let Ok(allowed_raw) = std::env::var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES") {
let allowed: Vec<&str> = allowed_raw
.split(',')
.map(str::trim)
.filter(|s| !s.is_empty())
.collect();
if !allowed.is_empty() && !allowed.contains(&policy) {
return Err(CliError::Usage(format!(
"channel_add_policy '{policy}' is not permitted on this deployment \
(BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES={allowed_raw})"
)));
}
}
let content = serde_json::json!({ "channel_add_policy": policy }).to_string();
use nostr::{EventBuilder, Kind};
let builder = EventBuilder::new(
@@ -648,7 +668,9 @@ pub async fn dispatch_canvas(cmd: crate::CanvasCmd, client: &BuzzClient) -> Resu
#[cfg(test)]
mod tests {
use super::{name_matches, validate_ttl_seconds, ChannelSummary};
use super::{cmd_set_add_policy, name_matches, validate_ttl_seconds, ChannelSummary};
use crate::client::BuzzClient;
use crate::CliError;
use serde_json::json;
fn event(tags: serde_json::Value) -> serde_json::Value {
@@ -756,4 +778,93 @@ mod tests {
fn validate_ttl_rejects_overflow() {
assert!(validate_ttl_seconds(i32::MAX as i64 + 1).is_err());
}
// --- BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES gate ---
fn check_allowed_channel_add_policy(allowed_raw: &str, policy: &str) -> Result<(), CliError> {
let allowed: Vec<&str> = allowed_raw
.split(',')
.map(str::trim)
.filter(|s| !s.is_empty())
.collect();
if !allowed.is_empty() && !allowed.contains(&policy) {
return Err(CliError::Usage(format!(
"channel_add_policy '{policy}' is not permitted on this deployment \
(BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES={allowed_raw})"
)));
}
Ok(())
}
#[test]
fn set_add_policy_rejects_disallowed_policy() {
let result = check_allowed_channel_add_policy("owner_only,nobody", "anyone");
assert!(
result.is_err(),
"anyone should be rejected when not in allowed set"
);
let msg = result.unwrap_err().to_string();
assert!(
msg.contains("not permitted"),
"error should mention 'not permitted': {msg}"
);
assert!(
msg.contains("anyone"),
"error should name the disallowed policy: {msg}"
);
}
#[test]
fn set_add_policy_accepts_allowed_policy() {
let result = check_allowed_channel_add_policy("owner_only,nobody", "owner_only");
assert!(result.is_ok(), "owner_only should be accepted: {result:?}");
}
#[test]
fn set_add_policy_no_restriction_allows_all() {
// Empty allowed list means no restriction.
let result = check_allowed_channel_add_policy("", "anyone");
assert!(
result.is_ok(),
"empty allowed list should permit any policy: {result:?}"
);
}
// --- Integration test: full env-var → cmd_set_add_policy() path ---
//
// This test calls cmd_set_add_policy directly with the env var set. The function
// returns early with an error before any network call, so no relay is needed.
// If the BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES check were removed from cmd_set_add_policy,
// this test would fail (it would proceed to sign_event and return a different error).
fn make_test_client() -> BuzzClient {
// Scalar = 1 is the smallest valid secp256k1 private key.
let keys =
nostr::Keys::parse("0000000000000000000000000000000000000000000000000000000000000001")
.expect("valid test key");
BuzzClient::new("ws://localhost:3000".to_string(), keys, None, None)
.expect("client construction should not fail")
}
#[tokio::test]
async fn set_add_policy_env_gate_rejects_disallowed_via_full_path() {
std::env::set_var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES", "owner_only,nobody");
let client = make_test_client();
let result = cmd_set_add_policy(&client, "anyone").await;
std::env::remove_var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES");
assert!(
result.is_err(),
"cmd_set_add_policy should reject 'anyone' when not in allowed set"
);
match result.unwrap_err() {
crate::CliError::Usage(msg) => {
assert!(
msg.contains("not permitted"),
"error should mention 'not permitted': {msg}"
);
}
other => panic!("expected CliError::Usage, got {other:?}"),
}
}
}