diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index af785711d5..76036b406e 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -99,6 +99,7 @@ pub(super) fn build_deploy_payload( effective_provider, effective_prompt, merged_env, + crate::managed_agents::internal_build(), )) } @@ -112,7 +113,10 @@ pub(super) fn deploy_payload_json( effective_provider: Option, effective_prompt: Option, merged_env: std::collections::BTreeMap, + internal: bool, ) -> serde_json::Value { + let (respond_to, respond_to_allowlist) = + crate::managed_agents::projected_access_with_policy(record, internal); serde_json::json!({ "name": &record.name, "relay_url": relay_url, @@ -127,8 +131,8 @@ pub(super) fn deploy_payload_json( "idle_timeout_seconds": record.idle_timeout_seconds, "max_turn_duration_seconds": record.max_turn_duration_seconds, "parallelism": record.parallelism, - "respond_to": record.respond_to, - "respond_to_allowlist": &record.respond_to_allowlist, + "respond_to": respond_to, + "respond_to_allowlist": respond_to_allowlist, "env_vars": merged_env, }) } diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 03389d1d18..2beb52330f 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -398,6 +398,18 @@ fn legacy_avatar_empty_when_nothing_resolves() { // ── Provider deploy payload completeness ───────────────────────────────────── +fn deploy_payload_for_policy(record: &ManagedAgentRecord, internal: bool) -> serde_json::Value { + deploy_payload_json( + record, + "wss://relay.example".to_string(), + Some("gpt-x".to_string()), + Some("openai".to_string()), + None, + std::collections::BTreeMap::new(), + internal, + ) +} + /// Regression (PR #1667 review, Thufir): the provider deploy payload must /// carry every behavioral field the local spawn path applies — a field /// missing here silently strips it from provider-backed agents. @@ -429,14 +441,7 @@ fn deploy_payload_carries_the_full_behavioral_quad() { )) .expect("sample record"); - let payload = deploy_payload_json( - &record, - "wss://relay.example".to_string(), - Some("gpt-x".to_string()), - Some("openai".to_string()), - None, - std::collections::BTreeMap::new(), - ); + let payload = deploy_payload_for_policy(&record, false); assert_eq!(payload["parallelism"], 4); assert_eq!(payload["respond_to"], "allowlist"); @@ -445,3 +450,28 @@ fn deploy_payload_carries_the_full_behavioral_quad() { assert_eq!(payload["provider"], "openai"); assert_eq!(payload["relay_url"], "wss://relay.example"); } + +#[test] +fn internal_deploy_payload_clamps_stale_access() { + use crate::managed_agents::{BackendKind, RespondTo}; + + let mut record = bare_agent_record(None, None, None); + record.backend = BackendKind::Provider { + id: "provider".to_string(), + config: serde_json::json!({}), + }; + record.respond_to = RespondTo::Anyone; + record.respond_to_allowlist = vec!["a".repeat(64)]; + + let payload = deploy_payload_for_policy(&record, true); + + assert_eq!( + payload["respond_to"], "owner-only", + "internal deploy payload widened stale access" + ); + assert_eq!( + payload["respond_to_allowlist"], + serde_json::json!([]), + "internal deploy payload retained a stale allowlist" + ); +} diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 2a719eefec..9354fe3e36 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -340,6 +340,14 @@ fn event_d_tag(event: &nostr::Event) -> Result { /// `persona_from_event` sets `id = d_tag`, an in-app persona reuses its d-tag as /// the id and a re-received event stays idempotent (no duplicate row). fn apply_inbound_persona(personas: &mut Vec, inbound: AgentDefinition) { + apply_inbound_persona_with_policy(personas, inbound, crate::managed_agents::internal_build()); +} + +fn apply_inbound_persona_with_policy( + personas: &mut Vec, + inbound: AgentDefinition, + internal: bool, +) { let d_tag = persona_d_tag(&inbound); match personas .iter_mut() @@ -357,12 +365,17 @@ fn apply_inbound_persona(personas: &mut Vec, inbound: AgentDefi local.respond_to_allowlist = inbound.respond_to_allowlist; local.parallelism = inbound.parallelism; local.shared = inbound.shared; - crate::managed_agents::normalize_definition_access(local); + crate::managed_agents::access_policy::normalize_definition_access_with_policy( + local, internal, + ); local.updated_at = inbound.updated_at; } None => { let mut inbound = inbound; - crate::managed_agents::normalize_definition_access(&mut inbound); + crate::managed_agents::access_policy::normalize_definition_access_with_policy( + &mut inbound, + internal, + ); personas.push(inbound); } } diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 1005a83432..18c05997b4 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -4,7 +4,7 @@ use super::*; use std::collections::BTreeMap; -const UUID: &str = "11111111-2222-3333-4444-555555555555"; +const UUID: &str = "11111111-2222-3333-4444-555555555555"; // sadscan:disable sq.pii.cc.visa /// A local in-app persona: `source_team_persona_slug` is None, so its d-tag /// IS its UUID id. Carries env_vars + source_team that must survive a patch. @@ -151,6 +151,27 @@ fn no_local_match_inserts_inbound_reusing_d_tag_as_id() { assert_eq!(personas.len(), 2, "re-receive of inserted record no-ops"); } +#[test] +fn internal_policy_clamps_inbound_persona_insert() { + let d_tag = "99999999-8888-7777-6666-555555555555"; + let mut inbound = inbound_for(d_tag, "New"); + inbound.respond_to = Some("anyone".to_string()); + inbound.respond_to_allowlist = vec!["a".repeat(64)]; + let mut personas = Vec::new(); + + apply_inbound_persona_with_policy(&mut personas, inbound, true); + + let inserted = personas.first().expect("inbound persona inserted"); + assert_eq!( + inserted.respond_to, None, + "internal inbound persona insert retained stale access" + ); + assert!( + inserted.respond_to_allowlist.is_empty(), + "internal inbound persona insert retained a stale allowlist" + ); +} + // ── Managed-agent (30177) inbound ──────────────────────────────────────── const AGENT_PUBKEY: &str = "agentpubkeyhex0000000000000000000000000000000000000000000000000000"; diff --git a/desktop/src-tauri/src/managed_agents/access_policy.rs b/desktop/src-tauri/src/managed_agents/access_policy.rs index 094c8ec8f5..402ca4849a 100644 --- a/desktop/src-tauri/src/managed_agents/access_policy.rs +++ b/desktop/src-tauri/src/managed_agents/access_policy.rs @@ -2,6 +2,8 @@ use super::{validate_respond_to_allowlist, AgentDefinition, ManagedAgentRecord, RespondTo}; +pub(crate) type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); + /// Internal packaging sets `BUZZ_BUILD_INTERNAL`; OSS/custom builds do not. pub(crate) fn internal_build() -> bool { option_env!("BUZZ_DESKTOP_BUILD_INTERNAL").is_some() @@ -15,6 +17,64 @@ pub(crate) fn owner_only_with_policy(internal: bool) -> bool { internal } +/// Project effective access at a behavioral boundary. This is independent of +/// persistence normalization so stale or hand-edited records cannot widen an +/// internal agent's access when they are deployed or published. +pub(crate) fn projected_access_with_policy( + record: &ManagedAgentRecord, + internal: bool, +) -> (RespondTo, Vec) { + if owner_only_with_policy(internal) { + (RespondTo::OwnerOnly, Vec::new()) + } else { + (record.respond_to, record.respond_to_allowlist.clone()) + } +} + +/// Build the inbound-author access environment for a launched agent. The +/// explicit policy input keeps internal-build enforcement testable without +/// weakening the production caller's compile-time decision. +pub(crate) fn build_respond_to_env_with_policy( + record: &ManagedAgentRecord, + owner_hex: Option<&str>, + enforced_owner_only: bool, +) -> Result { + let respond_to = if enforced_owner_only { + RespondTo::OwnerOnly + } else { + record.respond_to + }; + let normalized = if enforced_owner_only { + Vec::new() + } else { + validate_respond_to_allowlist(&record.respond_to_allowlist)? + }; + if respond_to == RespondTo::Allowlist && normalized.is_empty() { + return Err( + "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), + ); + } + + let mut set = vec![("BUZZ_ACP_RESPOND_TO", respond_to.as_str().to_string())]; + let mut remove = Vec::new(); + if respond_to == RespondTo::Allowlist { + set.push(("BUZZ_ACP_RESPOND_TO_ALLOWLIST", normalized.join(","))); + } else { + remove.push("BUZZ_ACP_RESPOND_TO_ALLOWLIST"); + } + + if record.auth_tag.is_none() { + if let Some(owner) = owner_hex { + set.push(("BUZZ_ACP_AGENT_OWNER", owner.to_string())); + } else { + remove.push("BUZZ_ACP_AGENT_OWNER"); + } + } else { + remove.push("BUZZ_ACP_AGENT_OWNER"); + } + Ok((set, remove)) +} + /// Normalize a persisted/projected instance for the current distribution. pub(crate) fn normalize_managed_agent_access(record: &mut ManagedAgentRecord) -> bool { normalize_managed_agent_access_with_policy(record, internal_build()) diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 4a7b80079d..d39ba9d226 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -68,6 +68,13 @@ pub struct ManagedAgentEventContent { /// operational start/stop produces an identical projection and never /// republishes. pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventContent { + agent_event_content_with_policy(record, super::internal_build()) +} + +pub(crate) fn agent_event_content_with_policy( + record: &ManagedAgentRecord, + internal: bool, +) -> ManagedAgentEventContent { // Slimmed projection (NIP-AP "Slimming: kind:30177"): definition-linked // instances resolve prompt/model/provider/source_version through their // kind:30175 definition, so those fields are omitted from the wire. @@ -77,6 +84,7 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont // restore path. This branch retires once every record is // definition-backed (B5 backfill). let definition_linked = record.persona_id.is_some(); + let (respond_to, respond_to_allowlist) = super::projected_access_with_policy(record, internal); ManagedAgentEventContent { name: record.name.clone(), persona_id: record.persona_id.clone(), @@ -101,8 +109,8 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont record.persona_source_version.clone() }, parallelism: record.parallelism, - respond_to: record.respond_to, - respond_to_allowlist: record.respond_to_allowlist.clone(), + respond_to, + respond_to_allowlist, } } @@ -302,6 +310,29 @@ mod tests { ); } + #[test] + fn public_projection_preserves_configured_access() { + let projection = agent_event_content_with_policy(&sample_agent(), false); + + assert_eq!(projection.respond_to, RespondTo::Allowlist); + assert_eq!(projection.respond_to_allowlist, vec!["79be667e"]); + } + + #[test] + fn internal_projection_clamps_stale_access() { + let projection = agent_event_content_with_policy(&sample_agent(), true); + + assert_eq!( + projection.respond_to, + RespondTo::OwnerOnly, + "internal kind:30177 projection widened stale access" + ); + assert!( + projection.respond_to_allowlist.is_empty(), + "internal kind:30177 projection retained a stale allowlist" + ); + } + /// Slimming (NIP-AP): definition-linked records omit the definition quad; /// definition-less records keep emitting it (they ARE their own /// definition — old readers would otherwise wipe fields with no restore diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 3072058007..fbbfa062d8 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -5,7 +5,8 @@ pub(crate) mod agent_snapshot; pub(crate) mod team_snapshot; pub(crate) use access_policy::{ apply_update_access, internal_build, normalize_definition_access, - normalize_managed_agent_access, owner_only, resolve_create_access, + normalize_managed_agent_access, owner_only, projected_access_with_policy, + resolve_create_access, }; pub(crate) use agent_env::{ baked_build_env, build_buzz_agent_provider_defaults, discovery_env_with_baked_floor, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 9b66544af3..8c9f7ecc06 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -16,10 +16,9 @@ use crate::{ mod path; pub(in crate::managed_agents) use path::build_augmented_path; -pub(crate) use path::{ - build_respond_to_env_with_policy, compose_path_entries, should_skip_claude_executable, - should_use_inherited, RespondToEnv, -}; +pub(crate) use path::{compose_path_entries, should_skip_claude_executable, should_use_inherited}; + +pub(crate) use super::access_policy::{build_respond_to_env_with_policy, RespondToEnv}; mod metadata; pub(crate) use metadata::{ @@ -129,6 +128,13 @@ pub(crate) fn resolve_workspace_pair_key( ManagedAgentRuntimeKey::new(pubkey.to_string(), &effective_relay).ok() } +fn summary_access_with_policy( + record: &ManagedAgentRecord, + internal: bool, +) -> (super::types::RespondTo, Vec) { + super::projected_access_with_policy(record, internal) +} + pub fn build_managed_agent_summary( app: &AppHandle, record: &ManagedAgentRecord, @@ -296,7 +302,8 @@ pub fn build_managed_agent_summary( .unwrap_or("") .to_string(); - let owner_only = super::owner_only(); + let (respond_to, respond_to_allowlist) = + summary_access_with_policy(record, super::internal_build()); Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), name: record.name.clone(), @@ -336,16 +343,8 @@ pub fn build_managed_agent_summary( start_on_app_launch: record.start_on_app_launch, auto_restart_on_config_change: record.auto_restart_on_config_change, log_path, - respond_to: if owner_only { - super::types::RespondTo::OwnerOnly - } else { - record.respond_to - }, - respond_to_allowlist: if owner_only { - Vec::new() - } else { - record.respond_to_allowlist.clone() - }, + respond_to, + respond_to_allowlist, }) } @@ -994,5 +993,8 @@ pub fn start_managed_agent_process( Ok(()) } +#[cfg(test)] +mod test_fixtures; + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/path.rs b/desktop/src-tauri/src/managed_agents/runtime/path.rs index 07b6339467..efec0c903e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/path.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/path.rs @@ -2,51 +2,6 @@ use std::path::PathBuf; -use super::super::{ManagedAgentRecord, RespondTo}; - -pub(crate) type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); - -pub(crate) fn build_respond_to_env_with_policy( - record: &ManagedAgentRecord, - owner_hex: Option<&str>, - enforced_owner_only: bool, -) -> Result { - let respond_to = if enforced_owner_only { - RespondTo::OwnerOnly - } else { - record.respond_to - }; - let normalized = if enforced_owner_only { - Vec::new() - } else { - super::super::validate_respond_to_allowlist(&record.respond_to_allowlist)? - }; - if respond_to == RespondTo::Allowlist && normalized.is_empty() { - return Err( - "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), - ); - } - - let mut set = vec![("BUZZ_ACP_RESPOND_TO", respond_to.as_str().to_string())]; - let mut remove = Vec::new(); - if respond_to == RespondTo::Allowlist { - set.push(("BUZZ_ACP_RESPOND_TO_ALLOWLIST", normalized.join(","))); - } else { - remove.push("BUZZ_ACP_RESPOND_TO_ALLOWLIST"); - } - - if record.auth_tag.is_none() { - if let Some(owner) = owner_hex { - set.push(("BUZZ_ACP_AGENT_OWNER", owner.to_string())); - } else { - remove.push("BUZZ_ACP_AGENT_OWNER"); - } - } else { - remove.push("BUZZ_ACP_AGENT_OWNER"); - } - Ok((set, remove)) -} - /// Return `true` when `path` is a Windows batch shim (`.cmd` or `.bat`, /// case-insensitive) that cannot be passed directly to `CreateProcess`. /// diff --git a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs new file mode 100644 index 0000000000..1352bbbc18 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs @@ -0,0 +1,64 @@ +use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; + +/// Construct a minimal record fixture for runtime tests. +pub(super) fn fixture( + respond_to: RespondTo, + allowlist: Vec, + auth_tag: Option, +) -> ManagedAgentRecord { + ManagedAgentRecord { + pubkey: "p".into(), + name: "n".into(), + persona_id: None, + private_key_nsec: "nsec1fake".into(), + auth_tag, + relay_url: "ws://localhost:3000".into(), + avatar_url: None, + acp_command: "buzz-acp".into(), + agent_command: "goose".into(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: std::collections::BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: "now".into(), + updated_at: "now".into(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to, + respond_to_allowlist: allowlist, + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 3f6ee996f6..5415439a6b 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -117,73 +117,10 @@ fn unknown_command_returns_none() { // ── build_respond_to_env tests ─────────────────────────────────────── -use super::build_respond_to_env; +use super::test_fixtures::fixture; +use super::{build_respond_to_env, build_respond_to_env_with_policy}; use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; -/// Construct a minimal record fixture for env-building tests. Only the -/// fields read by `build_respond_to_env` matter here. -fn fixture( - respond_to: RespondTo, - allowlist: Vec, - auth_tag: Option, -) -> ManagedAgentRecord { - ManagedAgentRecord { - pubkey: "p".into(), - name: "n".into(), - persona_id: None, - private_key_nsec: "nsec1fake".into(), - auth_tag, - relay_url: "ws://localhost:3000".into(), - avatar_url: None, - acp_command: "buzz-acp".into(), - agent_command: "goose".into(), - agent_command_override: None, - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - env_vars: std::collections::BTreeMap::new(), - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: "now".into(), - updated_at: "now".into(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to, - respond_to_allowlist: allowlist, - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - } -} - #[test] fn build_env_owner_only_sets_mode_and_removes_others() { let rec = fixture(RespondTo::OwnerOnly, vec![], Some("tag".into())); @@ -237,6 +174,24 @@ fn build_env_anyone_omits_allowlist_var() { assert!(remove.contains(&"BUZZ_ACP_RESPOND_TO_ALLOWLIST")); } +#[test] +fn internal_policy_overrides_stale_anyone_record_at_runtime() { + let rec = fixture( + RespondTo::Anyone, + vec!["malformed stale allowlist".into()], + Some("tag".into()), + ); + let (set, remove) = build_respond_to_env_with_policy(&rec, Some("owner"), true).unwrap(); + let set_map: std::collections::HashMap<_, _> = set.into_iter().collect(); + + assert_eq!( + set_map.get("BUZZ_ACP_RESPOND_TO").map(String::as_str), + Some("owner-only") + ); + assert!(!set_map.contains_key("BUZZ_ACP_RESPOND_TO_ALLOWLIST")); + assert!(remove.contains(&"BUZZ_ACP_RESPOND_TO_ALLOWLIST")); +} + #[test] fn build_env_legacy_record_without_auth_tag_emits_agent_owner() { let rec = fixture(RespondTo::OwnerOnly, vec![], None); @@ -275,6 +230,23 @@ fn build_env_rejects_empty_allowlist_in_allowlist_mode() { assert!(err.contains("at least one pubkey")); } +#[test] +fn internal_summary_access_clamps_stale_record() { + let rec = fixture(RespondTo::Anyone, vec!["a".repeat(64)], Some("tag".into())); + + let (respond_to, allowlist) = super::summary_access_with_policy(&rec, true); + + assert_eq!( + respond_to, + RespondTo::OwnerOnly, + "internal summary widened stale access" + ); + assert!( + allowlist.is_empty(), + "internal summary retained a stale allowlist" + ); +} + // ── persona fixture helpers ───────────────────────────────────────── fn persona_with_provider(