Skip to content

Commit a28f209

Browse files
Policy-GuruPolicy-Guru
committed
feat(acp): isolate signing behind durable reply broker
Co-authored-by: Ryan Laubscher <[email protected]> Signed-off-by: Ryan Laubscher <[email protected]>
1 parent 3a4bf51 commit a28f209

13 files changed

Lines changed: 849 additions & 80 deletions

File tree

‎Cargo.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/buzz-acp/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ path = "src/main.rs"
2020
buzz-core = { workspace = true }
2121
buzz-sdk = { workspace = true }
2222
buzz-persona = { path = "../buzz-persona" }
23+
buzz-cli = { path = "../buzz-cli" }
2324

2425
# Nostr
2526
nostr = { workspace = true }

‎crates/buzz-acp/src/acp.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,20 @@ use crate::usage::{TurnUsage, UsageTracker};
2020
/// Lines exceeding this limit are rejected to prevent OOM from rogue agents.
2121
const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB
2222

23+
/// Signing material belongs to the harness or its dedicated broker, never to
24+
/// the model subprocess. Keep this list at the final spawn boundary: callers
25+
/// may add arbitrary persona environment entries, but they cannot reintroduce
26+
/// a raw signing capability through `AcpClient::spawn`.
27+
const WORKER_RESTRICTED_ENV_KEYS: &[&str] = &[
28+
"BUZZ_PRIVATE_KEY",
29+
"BUZZ_AUTH_TAG",
30+
"NOSTR_PRIVATE_KEY",
31+
"BUZZ_ACP_PRIVATE_KEY",
32+
// The durable ledger contains signed events (and potentially their NIP-OA
33+
// tag). Workers do not need its filesystem location to use relay_reply.
34+
"BUZZ_ACP_STATE_DIR",
35+
];
36+
2337
/// An MCP server configuration passed to `session/new`.
2438
///
2539
/// Corresponds to the `McpServerStdio` variant in the ACP schema.
@@ -423,6 +437,14 @@ impl AcpClient {
423437
// Callers MUST still call shutdown().await for guaranteed cleanup.
424438
.kill_on_drop(true);
425439

440+
// Start from the ambient environment but explicitly remove every raw
441+
// signing credential before any persona configuration is applied.
442+
// This protects against both desktop-injected credentials and callers
443+
// that accidentally pass a reserved key in `extra_env`.
444+
for key in WORKER_RESTRICTED_ENV_KEYS {
445+
cmd.env_remove(key);
446+
}
447+
426448
// Per-persona env vars (e.g., GOOSE_PROVIDER, BUZZ_AGENT_PROVIDER).
427449
// For most keys, operator precedence wins: skip injection if already set
428450
// in the parent environment.
@@ -448,6 +470,13 @@ impl AcpClient {
448470
let codex_merge_active = codex_config_value.is_some();
449471

450472
for (key, value) in extra_env {
473+
if WORKER_RESTRICTED_ENV_KEYS.contains(&key.as_str()) {
474+
tracing::warn!(
475+
key,
476+
"refusing to inject a broker-restricted value into worker"
477+
);
478+
continue;
479+
}
451480
if key == "CODEX_CONFIG" && codex_merge_active {
452481
// Handled by build_codex_config_env; skip here to avoid double-setting.
453482
continue;

‎crates/buzz-acp/src/config.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ pub struct CliArgs {
240240
#[arg(long, env = "BUZZ_RELAY_URL", default_value = "ws://localhost:3000")]
241241
pub relay_url: String,
242242

243-
#[arg(long, env = "BUZZ_PRIVATE_KEY")]
243+
#[arg(long, env = "BUZZ_PRIVATE_KEY", hide_env_values = true)]
244244
pub private_key: String,
245245

246246
/// Agent owner pubkey (64-char hex). Used for --respond-to=owner-only gate.

‎crates/buzz-acp/src/lib.rs‎

Lines changed: 58 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ mod pool;
99
mod pool_lifecycle;
1010
mod queue;
1111
mod relay;
12+
mod reply_broker;
1213
mod setup_mode;
1314
mod usage;
1415

@@ -35,7 +36,7 @@ use config::{
3536
};
3637
use filter::SubscriptionRule;
3738
use futures_util::FutureExt;
38-
use nostr::{PublicKey, ToBech32};
39+
use nostr::PublicKey;
3940
use pool::{
4041
AgentPool, ControlSignal, IdleSwitchResult, OwnedAgent, PromptContext, PromptOutcome,
4142
PromptResult, PromptSource, SessionState, TimeoutKind,
@@ -1314,6 +1315,18 @@ async fn tokio_main() -> Result<()> {
13141315
);
13151316
}
13161317

1318+
let broker_state_dir = reply_broker::state_dir_from_env(&config.config_path)
1319+
.map_err(|e| anyhow::anyhow!("reply broker state setup failed: {e}"))?;
1320+
let reply_broker = reply_broker::start(
1321+
config.relay_url.clone(),
1322+
config.keys.clone(),
1323+
std::env::var("BUZZ_AUTH_TAG")
1324+
.ok()
1325+
.filter(|tag| !tag.is_empty()),
1326+
broker_state_dir,
1327+
)
1328+
.map_err(|e| anyhow::anyhow!("reply broker startup failed: {e}"))?;
1329+
13171330
let mut pool = if config.lazy_pool {
13181331
AgentPool::from_slots((0..config.agents).map(|_| None).collect())
13191332
} else {
@@ -1528,7 +1541,7 @@ async fn tokio_main() -> Result<()> {
15281541

15291542
let base_prompt_content = config.base_prompt_content.take();
15301543
let ctx = Arc::new(PromptContext {
1531-
mcp_servers: build_mcp_servers(&config),
1544+
mcp_servers: build_mcp_servers(&config, Some(reply_broker.endpoint())),
15321545
initial_message: config.initial_message.clone(),
15331546
idle_timeout: Duration::from_secs(config.idle_timeout_secs),
15341547
max_turn_duration: Duration::from_secs(config.max_turn_duration_secs),
@@ -4139,7 +4152,10 @@ async fn run_models(args: ModelsArgs) -> Result<()> {
41394152
Ok(())
41404153
}
41414154

4142-
fn build_mcp_servers(config: &Config) -> Vec<McpServer> {
4155+
fn build_mcp_servers(
4156+
config: &Config,
4157+
reply_broker: Option<&reply_broker::ReplyBrokerEndpoint>,
4158+
) -> Vec<McpServer> {
41434159
if config.mcp_command.is_empty() {
41444160
return vec![];
41454161
}
@@ -4152,32 +4168,20 @@ fn build_mcp_servers(config: &Config) -> Vec<McpServer> {
41524168
command: config.mcp_command.clone(),
41534169
args: vec![],
41544170
env: {
4155-
let mut env = vec![
4156-
EnvVar {
4157-
name: "BUZZ_RELAY_URL".into(),
4158-
value: config.relay_url.clone(),
4159-
},
4160-
EnvVar {
4161-
name: "BUZZ_PRIVATE_KEY".into(),
4162-
// bech32 encoding of a valid secret key is infallible.
4163-
// Panic here is correct: injecting a bogus secret would cause
4164-
// delayed, hard-to-diagnose agent failures downstream.
4165-
value: config
4166-
.keys
4167-
.secret_key()
4168-
.to_bech32()
4169-
.expect("secret key bech32 encoding should never fail"),
4170-
},
4171-
];
4172-
// Forward BUZZ_AUTH_TAG (NIP-OA owner attestation credential)
4173-
// so the MCP server can attach it to every signed event.
4174-
if let Ok(auth_tag) = std::env::var("BUZZ_AUTH_TAG") {
4175-
if !auth_tag.is_empty() {
4176-
env.push(EnvVar {
4177-
name: "BUZZ_AUTH_TAG".into(),
4178-
value: auth_tag,
4179-
});
4180-
}
4171+
// Worker MCP servers receive no signing key or owner attestation.
4172+
// `BUZZ_REPLY_BROKER_*` is a narrowly scoped local capability: it
4173+
// can create one schema-validated reply through the harness broker,
4174+
// never run arbitrary Buzz CLI operations or sign arbitrary events.
4175+
let mut env = Vec::new();
4176+
if let Some(broker) = reply_broker {
4177+
env.push(EnvVar {
4178+
name: "BUZZ_REPLY_BROKER_URL".into(),
4179+
value: broker.url.clone(),
4180+
});
4181+
env.push(EnvVar {
4182+
name: "BUZZ_REPLY_BROKER_CAPABILITY".into(),
4183+
value: broker.capability.clone(),
4184+
});
41814185
}
41824186
// Forward the agent's display name so dev-mcp can use it as the git
41834187
// author name instead of the raw npub. Read from the process env
@@ -5004,58 +5008,48 @@ mod build_mcp_servers_tests {
50045008
#[test]
50055009
fn session_new_mcp_server_has_required_fields() {
50065010
let config = test_config();
5007-
let servers = build_mcp_servers(&config);
5011+
let servers = build_mcp_servers(&config, None);
50085012
assert_eq!(servers.len(), 1);
50095013
let server = &servers[0];
50105014
assert_eq!(server.name, "test-mcp-server");
50115015

50125016
let names: Vec<&str> = server.env.iter().map(|e| e.name.as_str()).collect();
50135017
assert!(
5014-
names.contains(&"BUZZ_RELAY_URL"),
5015-
"missing BUZZ_RELAY_URL; got {names:?}"
5016-
);
5017-
assert!(
5018-
names.contains(&"BUZZ_PRIVATE_KEY"),
5019-
"missing BUZZ_PRIVATE_KEY; got {names:?}"
5018+
names.is_empty(),
5019+
"workers must not receive relay credentials: {names:?}"
50205020
);
50215021
}
50225022

50235023
#[test]
5024-
fn session_new_mcp_server_forwards_buzz_auth_tag() {
5025-
let _guard = ENV_LOCK.lock().unwrap();
5026-
std::env::set_var("BUZZ_AUTH_TAG", "test-attestation-tag");
5024+
fn session_new_mcp_server_receives_only_reply_broker_capability() {
50275025
let config = test_config();
5028-
let servers = build_mcp_servers(&config);
5029-
std::env::remove_var("BUZZ_AUTH_TAG");
5026+
let broker = reply_broker::ReplyBrokerEndpoint {
5027+
url: "tcp://127.0.0.1:9999".into(),
5028+
capability: "scoped-capability".into(),
5029+
};
5030+
let servers = build_mcp_servers(&config, Some(&broker));
50305031

50315032
let server = &servers[0];
5032-
let auth_tag_env = server.env.iter().find(|e| e.name == "BUZZ_AUTH_TAG");
5033+
let names: Vec<&str> = server.env.iter().map(|entry| entry.name.as_str()).collect();
50335034
assert!(
5034-
auth_tag_env.is_some(),
5035-
"BUZZ_AUTH_TAG should be forwarded when set"
5035+
names.contains(&"BUZZ_REPLY_BROKER_URL")
5036+
&& names.contains(&"BUZZ_REPLY_BROKER_CAPABILITY"),
5037+
"missing reply capability: {names:?}"
5038+
);
5039+
assert!(
5040+
!names
5041+
.iter()
5042+
.any(|name| name.contains("PRIVATE_KEY") || name.contains("AUTH_TAG")),
5043+
"signing credentials must never appear in the worker MCP config: {names:?}"
50365044
);
5037-
assert_eq!(auth_tag_env.unwrap().value, "test-attestation-tag");
5038-
}
5039-
5040-
#[test]
5041-
fn session_new_mcp_server_skips_empty_buzz_auth_tag() {
5042-
let _guard = ENV_LOCK.lock().unwrap();
5043-
std::env::set_var("BUZZ_AUTH_TAG", "");
5044-
let config = test_config();
5045-
let servers = build_mcp_servers(&config);
5046-
std::env::remove_var("BUZZ_AUTH_TAG");
5047-
5048-
let server = &servers[0];
5049-
let has_auth_tag = server.env.iter().any(|e| e.name == "BUZZ_AUTH_TAG");
5050-
assert!(!has_auth_tag, "empty BUZZ_AUTH_TAG should not be forwarded");
50515045
}
50525046

50535047
#[test]
50545048
fn test_display_name_set_is_forwarded_to_mcp_server() {
50555049
let _guard = ENV_LOCK.lock().unwrap();
50565050
std::env::set_var("BUZZ_ACP_DISPLAY_NAME", "Duncan");
50575051
let config = test_config();
5058-
let servers = build_mcp_servers(&config);
5052+
let servers = build_mcp_servers(&config, None);
50595053
std::env::remove_var("BUZZ_ACP_DISPLAY_NAME");
50605054

50615055
let entry = servers[0]
@@ -5074,7 +5068,7 @@ mod build_mcp_servers_tests {
50745068
let _guard = ENV_LOCK.lock().unwrap();
50755069
std::env::remove_var("BUZZ_ACP_DISPLAY_NAME");
50765070
let config = test_config();
5077-
let servers = build_mcp_servers(&config);
5071+
let servers = build_mcp_servers(&config, None);
50785072

50795073
// Absent, not empty-valued: dev-mcp distinguishes the two and only
50805074
// falls back to the npub when the key is missing or blank.
@@ -5092,7 +5086,7 @@ mod build_mcp_servers_tests {
50925086
let _guard = ENV_LOCK.lock().unwrap();
50935087
std::env::set_var("BUZZ_ACP_DISPLAY_NAME", "");
50945088
let config = test_config();
5095-
let servers = build_mcp_servers(&config);
5089+
let servers = build_mcp_servers(&config, None);
50965090
std::env::remove_var("BUZZ_ACP_DISPLAY_NAME");
50975091

50985092
assert!(
@@ -5108,7 +5102,7 @@ mod build_mcp_servers_tests {
51085102
fn empty_mcp_command_returns_no_servers() {
51095103
let mut config = test_config();
51105104
config.mcp_command = "".into();
5111-
let servers = build_mcp_servers(&config);
5105+
let servers = build_mcp_servers(&config, None);
51125106
assert!(
51135107
servers.is_empty(),
51145108
"empty mcp_command should produce no MCP servers"
@@ -5119,7 +5113,7 @@ mod build_mcp_servers_tests {
51195113
fn absolute_path_mcp_command_uses_file_stem_as_name() {
51205114
let mut config = test_config();
51215115
config.mcp_command = "/opt/bin/my-mcp-server".into();
5122-
let servers = build_mcp_servers(&config);
5116+
let servers = build_mcp_servers(&config, None);
51235117
assert_eq!(servers.len(), 1);
51245118
assert_eq!(servers[0].name, "my-mcp-server");
51255119
}
@@ -5140,7 +5134,7 @@ mod build_mcp_servers_tests {
51405134

51415135
// Confirm a non-empty command with no stem (e.g. just a dot) also falls back.
51425136
config.mcp_command = ".".into();
5143-
let servers = build_mcp_servers(&config);
5137+
let servers = build_mcp_servers(&config, None);
51445138
assert_eq!(servers.len(), 1);
51455139
assert_eq!(
51465140
servers[0].name, "mcp",

0 commit comments

Comments
 (0)