From f45e8e80e28cf3ca5a3073db36aa1da785de99a8 Mon Sep 17 00:00:00 2001 From: npub1tquskdu6yc4h8l7xxtceculxw600grekeq0xg2ukqfrwl7vrzg3quz3gmp <58390b379a262b73ffc632f19c73e6769ef40f36c81e642b960246eff9831222@buzz.block.builderlab.xyz> Date: Fri, 31 Jul 2026 14:20:28 -0700 Subject: [PATCH] fix(desktop): clamp internal agent access for every backend Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- desktop/src-tauri/src/commands/agents.rs | 1 - .../src/managed_agents/access_policy.rs | 109 ++++++++++-------- desktop/src-tauri/src/managed_agents/mod.rs | 2 +- .../src-tauri/src/managed_agents/runtime.rs | 72 ++---------- .../src/managed_agents/runtime/path.rs | 45 ++++++++ .../src/managed_agents/runtime/tests.rs | 20 +--- desktop/src/features/agents/AGENTS.md | 5 +- .../agents/ui/AgentDefinitionDialog.tsx | 10 +- .../agents/ui/AgentInstanceEditDialog.tsx | 9 +- .../agents/ui/PersonaAdvancedFields.tsx | 4 +- .../src/features/agents/ui/RespondToField.tsx | 2 +- .../channels/ui/EditRespondToDialog.tsx | 3 +- .../features/onboarding/welcomeGuide.test.mjs | 17 +-- .../src/features/onboarding/welcomeGuide.ts | 2 +- .../onboarding/welcomeKickoff.test.mjs | 37 +++--- desktop/src/shared/api/tauri.ts | 13 +-- desktop/tests/e2e/edit-agent.spec.ts | 6 +- 17 files changed, 157 insertions(+), 200 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index dfb241968d..85ded0b3a7 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -586,7 +586,6 @@ pub async fn create_managed_agent( let (requested_respond_to, respond_to_allowlist) = crate::managed_agents::resolve_create_access( - &input.backend, input.respond_to, &input.respond_to_allowlist, )?; diff --git a/desktop/src-tauri/src/managed_agents/access_policy.rs b/desktop/src-tauri/src/managed_agents/access_policy.rs index 94af3d1738..094c8ec8f5 100644 --- a/desktop/src-tauri/src/managed_agents/access_policy.rs +++ b/desktop/src-tauri/src/managed_agents/access_policy.rs @@ -1,24 +1,21 @@ //! Distribution policy for managed-agent inbound author access. -use super::{ - validate_respond_to_allowlist, AgentDefinition, BackendKind, ManagedAgentRecord, RespondTo, -}; +use super::{validate_respond_to_allowlist, AgentDefinition, ManagedAgentRecord, RespondTo}; /// 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() } -pub(crate) fn owner_only_for_backend(backend: &BackendKind) -> bool { - owner_only_for_backend_with_policy(backend, internal_build()) +pub(crate) fn owner_only() -> bool { + owner_only_with_policy(internal_build()) } -pub(crate) fn owner_only_for_backend_with_policy(backend: &BackendKind, internal: bool) -> bool { - internal && *backend == BackendKind::Local +pub(crate) fn owner_only_with_policy(internal: bool) -> bool { + internal } -/// Normalize a persisted/projected instance. Provider instances remain -/// configurable because the internal policy applies only to local execution. +/// 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()) } @@ -27,7 +24,7 @@ pub(crate) fn normalize_managed_agent_access_with_policy( record: &mut ManagedAgentRecord, internal: bool, ) -> bool { - if !owner_only_for_backend_with_policy(&record.backend, internal) { + if !owner_only_with_policy(internal) { return false; } let changed = @@ -38,8 +35,7 @@ pub(crate) fn normalize_managed_agent_access_with_policy( } /// Definitions are backend-neutral defaults. Internal builds store owner-only -/// defaults so every later local mint starts safe; provider instance policy is -/// still decided from its own backend at create/update/deploy time. +/// defaults so every later mint starts safe. pub(crate) fn normalize_definition_access(record: &mut AgentDefinition) -> bool { normalize_definition_access_with_policy(record, internal_build()) } @@ -58,25 +54,18 @@ pub(crate) fn normalize_definition_access_with_policy( } pub(crate) fn resolve_create_access( - backend: &BackendKind, requested_mode: Option, requested_allowlist: &[String], ) -> Result<(Option, Vec), String> { - resolve_create_access_with_policy( - backend, - requested_mode, - requested_allowlist, - internal_build(), - ) + resolve_create_access_with_policy(requested_mode, requested_allowlist, internal_build()) } pub(crate) fn resolve_create_access_with_policy( - backend: &BackendKind, requested_mode: Option, requested_allowlist: &[String], internal: bool, ) -> Result<(Option, Vec), String> { - if owner_only_for_backend_with_policy(backend, internal) { + if owner_only_with_policy(internal) { return Ok((Some(RespondTo::OwnerOnly), Vec::new())); } let allowlist = validate_respond_to_allowlist(requested_allowlist)?; @@ -107,7 +96,7 @@ pub(crate) fn apply_update_access_with_policy( requested_allowlist: Option<&[String]>, internal: bool, ) -> Result<(), String> { - if owner_only_for_backend_with_policy(&record.backend, internal) { + if owner_only_with_policy(internal) { record.respond_to = RespondTo::OwnerOnly; record.respond_to_allowlist.clear(); return Ok(()); @@ -133,6 +122,7 @@ pub(crate) fn apply_update_access_with_policy( #[cfg(test)] mod tests { use super::*; + use crate::managed_agents::BackendKind; fn record(backend: BackendKind) -> ManagedAgentRecord { let mut record: ManagedAgentRecord = serde_json::from_value(serde_json::json!({ @@ -150,39 +140,58 @@ mod tests { } #[test] - fn internal_create_clamps_local_but_not_provider() { - let local = resolve_create_access_with_policy( - &BackendKind::Local, - Some(RespondTo::Anyone), - &["bad".into()], - true, - ) - .unwrap(); - assert_eq!(local, (Some(RespondTo::OwnerOnly), Vec::new())); - - let provider = BackendKind::Provider { - id: "p".into(), - config: serde_json::json!({}), - }; + fn internal_create_clamps_every_agent() { let result = - resolve_create_access_with_policy(&provider, Some(RespondTo::Anyone), &[], true) + resolve_create_access_with_policy(Some(RespondTo::Anyone), &["bad".into()], true) .unwrap(); - assert_eq!(result, (Some(RespondTo::Anyone), Vec::new())); + assert_eq!(result, (Some(RespondTo::OwnerOnly), Vec::new())); } #[test] - fn internal_update_clamps_local_but_not_provider() { - let mut local = record(BackendKind::Local); - apply_update_access_with_policy(&mut local, None, None, true).unwrap(); - assert_eq!(local.respond_to, RespondTo::OwnerOnly); - assert!(local.respond_to_allowlist.is_empty()); + fn internal_update_clamps_local_and_provider() { + for (label, backend) in [ + ("local", BackendKind::Local), + ( + "provider", + BackendKind::Provider { + id: "p".into(), + config: serde_json::json!({}), + }, + ), + ] { + let mut record = record(backend); + apply_update_access_with_policy(&mut record, None, None, true).unwrap(); + assert_eq!( + record.respond_to, + RespondTo::OwnerOnly, + "internal update did not clamp {label} agent", + ); + assert!( + record.respond_to_allowlist.is_empty(), + "internal update did not clear {label} agent allowlist", + ); + } + } - let mut provider = record(BackendKind::Provider { - id: "p".into(), - config: serde_json::json!({}), - }); - apply_update_access_with_policy(&mut provider, None, None, true).unwrap(); - assert_eq!(provider.respond_to, RespondTo::Anyone); - assert_eq!(provider.respond_to_allowlist, vec!["stale"]); + #[test] + fn internal_normalization_clamps_local_and_provider() { + for (label, backend) in [ + ("local", BackendKind::Local), + ( + "provider", + BackendKind::Provider { + id: "p".into(), + config: serde_json::json!({}), + }, + ), + ] { + let mut record = record(backend); + assert!( + normalize_managed_agent_access_with_policy(&mut record, true), + "internal normalization did not change {label} agent", + ); + assert_eq!(record.respond_to, RespondTo::OwnerOnly); + assert!(record.respond_to_allowlist.is_empty()); + } } } diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index feaf18d825..3072058007 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -5,7 +5,7 @@ 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_for_backend, resolve_create_access, + normalize_managed_agent_access, owner_only, 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 75152dc999..9b66544af3 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -16,9 +16,10 @@ use crate::{ mod path; pub(in crate::managed_agents) use path::build_augmented_path; -pub(crate) use path::compose_path_entries; -pub(crate) use path::should_skip_claude_executable; -pub(crate) use path::should_use_inherited; +pub(crate) use path::{ + build_respond_to_env_with_policy, compose_path_entries, should_skip_claude_executable, + should_use_inherited, RespondToEnv, +}; mod metadata; pub(crate) use metadata::{ @@ -33,8 +34,6 @@ pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; mod sweep; pub(crate) use sweep::sweep_untracked_bundle_harnesses; -type RespondToEnv = (Vec<(&'static str, String)>, Vec<&'static str>); - mod process; #[cfg(test)] use process::{ @@ -297,6 +296,7 @@ pub fn build_managed_agent_summary( .unwrap_or("") .to_string(); + let owner_only = super::owner_only(); Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), name: record.name.clone(), @@ -336,12 +336,12 @@ 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 super::owner_only_for_backend(&record.backend) { + respond_to: if owner_only { super::types::RespondTo::OwnerOnly } else { record.respond_to }, - respond_to_allowlist: if super::owner_only_for_backend(&record.backend) { + respond_to_allowlist: if owner_only { Vec::new() } else { record.respond_to_allowlist.clone() @@ -389,63 +389,7 @@ pub(crate) fn build_respond_to_env( record: &ManagedAgentRecord, owner_hex: Option<&str>, ) -> Result { - build_respond_to_env_with_policy( - record, - owner_hex, - super::owner_only_for_backend(&record.backend), - ) -} - -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 { - super::types::RespondTo::OwnerOnly - } else { - record.respond_to - }; - - // Defensive re-validation: an on-disk record could have been hand-edited. - let normalized = if enforced_owner_only { - Vec::new() - } else { - super::types::validate_respond_to_allowlist(&record.respond_to_allowlist)? - }; - if respond_to == super::types::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<(&'static str, String)> = Vec::new(); - let mut remove: Vec<&'static str> = Vec::new(); - - set.push(("BUZZ_ACP_RESPOND_TO", respond_to.as_str().to_string())); - - if respond_to == super::types::RespondTo::Allowlist { - set.push(("BUZZ_ACP_RESPOND_TO_ALLOWLIST", normalized.join(","))); - } else { - remove.push("BUZZ_ACP_RESPOND_TO_ALLOWLIST"); - } - - // Legacy fallback: agents created before NIP-OA lack `auth_tag`. Without - // it the harness can't resolve the owner, and owner-dependent gate modes - // would drop every event. Forwarding the workspace owner pubkey via - // BUZZ_ACP_AGENT_OWNER keeps those records functional. Modern records - // (`auth_tag = Some(...)`) use `BUZZ_AUTH_TAG` as before. - 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)) + build_respond_to_env_with_policy(record, owner_hex, super::owner_only()) } pub(crate) fn configure_runtime_cli( diff --git a/desktop/src-tauri/src/managed_agents/runtime/path.rs b/desktop/src-tauri/src/managed_agents/runtime/path.rs index efec0c903e..07b6339467 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/path.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/path.rs @@ -2,6 +2,51 @@ 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/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 89d576262a..3f6ee996f6 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -117,7 +117,7 @@ fn unknown_command_returns_none() { // ── build_respond_to_env tests ─────────────────────────────────────── -use super::{build_respond_to_env, build_respond_to_env_with_policy}; +use super::build_respond_to_env; use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; /// Construct a minimal record fixture for env-building tests. Only the @@ -237,24 +237,6 @@ 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); diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index d9222c7032..a44bc38f17 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -146,7 +146,10 @@ with a TypeScript lookup table or an id comparison in a component. shown; when it *is* remote they picked that host from the selector themselves. Never synthesize a run location a surface doesn't have. Don't expose `respond-to`, `allowlist`, Nostr, or harness jargon in primary UI - copy. + copy. **Internal-build owner-only access is backend-independent.** When + `getAgentAccessOwnerOnly()` is true, every managed agent's access control is + locked to owner-only, including provider-backed agents. A provider backend + does not prove remote execution and must never create a policy carve-out. ## The tests that enforce this diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index cfc65788c6..5425131448 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -66,11 +66,7 @@ import { MODEL_DISCOVERY_LOADING_VALUE, usePersonaModelDiscovery, } from "./usePersonaModelDiscovery"; -import { - useAgentAccessOwnerOnlyQuery, - useBakedBuildEnvKeysQuery, - useRuntimeFileConfigQuery, -} from "../hooks"; +import { useBakedBuildEnvKeysQuery, useRuntimeFileConfigQuery } from "../hooks"; import { useAgentDialogDefaults } from "./useAgentDialogDefaults"; import { AgentDefaultsDialog } from "./AgentDefaultsDialog"; import { AgentHarnessField } from "./AgentHarnessField"; @@ -429,9 +425,6 @@ export function AgentDefinitionDialog({ setModel(nextPair.model); } const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); - const { data: agentAccessOwnerOnly } = useAgentAccessOwnerOnlyQuery({ - enabled: open, - }); const localModeGate = React.useMemo( () => computeLocalModeGate({ @@ -1025,7 +1018,6 @@ export function AgentDefinitionDialog({ hiddenEnvKeys={ topLevelSecretEnvVar ? [topLevelSecretEnvVar] : [] } - agentAccessOwnerOnly={agentAccessOwnerOnly === true} inheritedEnvVars={inheritedEnvVarsForAdvanced} model={model} modelTuningRuntimeId={runtime} diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 5658531347..c00981102b 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -63,9 +63,9 @@ import { type RuntimeModelProviderSelection, } from "./runtimeModelProviderSelection"; import { AgentCreationPreview } from "./AgentCreationPreview"; +import { InternalAgentAccessField } from "./InternalAgentAccessField"; import type { EnvVarsValue } from "./EnvVarsEditor"; import { useRequiredCredentialState } from "./useRequiredCredentialState"; -import { InternalAgentAccessField } from "./InternalAgentAccessField"; import { PersonaDropdownField } from "./PersonaDropdownField"; import { MODEL_DISCOVERY_LOADING_VALUE, @@ -911,7 +911,6 @@ export function AgentInstanceEditDialog({ )}
- {/* Agent name */}
- - {/* Provider (runtime) */}