diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index c322e6cb6e8..e804584c473 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -194,6 +194,12 @@ fn reconcile_inbound_persona_event_blocking( if let Some(managed_agent) = &inbound_managed_agent { validate_inbound_managed_agent_definition(managed_agent)?; } + let inbound_team = (kind == KIND_TEAM) + .then(|| team_content_from_event(&event)) + .transpose()?; + if let Some(team) = &inbound_team { + validate_inbound_team_definition(team)?; + } let d_tag = match &inbound_persona { Some(persona) => persona_d_tag(persona), None => event_d_tag(&event)?, @@ -255,10 +261,12 @@ fn reconcile_inbound_persona_event_blocking( } KIND_TEAM => { let mut teams = load_teams(&app)?; + let team = inbound_team + .ok_or_else(|| "team content was not parsed before retention".to_string())?; commit_inbound_team( &mut teams, d_tag, - team_content_from_event(&event)?, + team, |teams| save_teams(&app, teams), || load_managed_agents(&app), |records| save_managed_agents(&app, records), @@ -359,6 +367,15 @@ fn validate_inbound_managed_agent_definition( .map_err(|error| format!("Inbound managed-agent definition is unsafe: {error}")) } +fn validate_inbound_team_definition(team: &TeamEventContent) -> Result<(), String> { + let instructions = match &team.instructions { + Some(inner) => inner.as_deref(), + None => None, + }; + crate::managed_agents::validate_team_definition_text(&team.name, instructions) + .map_err(|error| format!("Inbound team definition is unsafe: {error}")) +} + /// Parse an inbound wire event and enforce the signature gate. Everything /// downstream trusts `event.pubkey` (ownership routing, tombstone scoping, /// behavioral-quad application), so a forged pubkey must die here — the 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 c0526222151..b1332481721 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -909,3 +909,32 @@ fn inbound_definition_less_agent_accepts_visible_multiline_prompt() { assert!(validate_inbound_managed_agent_definition(&inbound).is_ok()); } + +#[test] +fn inbound_team_rejects_invisible_instructions() { + let mut inbound = team_content("Review Team"); + inbound.instructions = Some(Some("Be\u{200B} thorough.".to_string())); + + let error = validate_inbound_team_definition(&inbound) + .expect_err("relay sync must reject invisible team instructions"); + + assert!(error.contains("U+200B")); +} + +#[test] +fn inbound_team_rejects_bidirectional_name() { + let inbound = team_content("Review\u{202E} Team"); + + let error = validate_inbound_team_definition(&inbound) + .expect_err("relay sync must reject bidirectional team names"); + + assert!(error.contains("U+202E")); +} + +#[test] +fn inbound_team_accepts_visible_multiline_instructions() { + let mut inbound = team_content("Review Team"); + inbound.instructions = Some(Some("Review changes.\n\tCall out risks.".to_string())); + + assert!(validate_inbound_team_definition(&inbound).is_ok()); +} diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 8315f39a362..f1658bce14d 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -18,7 +18,8 @@ use crate::{ managed_agents::{ agent_snapshot::{build_snapshot, AgentSnapshot, AgentSnapshotMemoryEntry, MemoryLevel}, load_managed_agents, load_personas, load_teams, load_teams_readonly, save_managed_agents, - save_personas, save_teams, AgentDefinition, ManagedAgentRecord, TeamRecord, + save_personas, save_teams, validate_team_definition_text, AgentDefinition, + ManagedAgentRecord, TeamRecord, }, relay::{effective_agent_relay_url, relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -164,6 +165,7 @@ pub(crate) fn build_import_team( if name.is_empty() { return Err("Team snapshot name is empty.".to_string()); } + validate_team_definition_text(name, snapshot.team.instructions.as_deref())?; Ok(TeamRecord { id: Uuid::new_v4().to_string(), @@ -270,6 +272,8 @@ fn build_team_export_snapshot( memory_level: MemoryLevel, memory_entries_by_persona: &std::collections::HashMap>, ) -> Result { + validate_team_definition_text(&team.name, team.instructions.as_deref()) + .map_err(|error| format!("Team snapshot is unsafe to export: {error}"))?; let members = team .persona_ids .iter() @@ -461,6 +465,10 @@ pub async fn preview_team_snapshot_import( ) -> Result { tokio::task::spawn_blocking(move || { let snapshot = decode_team_snapshot_from_bytes(&file_bytes)?; + validate_team_definition_text( + snapshot.team.name.trim(), + snapshot.team.instructions.as_deref(), + )?; let members: Vec<_> = snapshot.members.iter().map(member_preview).collect(); Ok(TeamSnapshotImportPreview { name: snapshot.team.name, diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index a466228160a..ed587be899a 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -330,6 +330,91 @@ fn team_import_definitions_are_built_for_all_members() { assert_eq!(definitions[0].system_prompt, "Alice prompt"); } +#[test] +fn team_import_rejects_invisible_team_instructions() { + let mut source = snapshot(vec![member("Alice")]); + source.team.instructions = Some("Be\u{200B} thorough.".to_string()); + + let error = build_import_team(&source, vec!["alice".to_string()], "now") + .expect_err("snapshot import must reject invisible team instructions"); + + assert!(error.contains("U+200B")); +} + +#[test] +fn team_import_rejects_bidirectional_team_name() { + let mut source = snapshot(vec![member("Alice")]); + source.team.name = "Review\u{202E} Team".to_string(); + + let error = build_import_team(&source, vec!["alice".to_string()], "now") + .expect_err("snapshot import must reject bidirectional team names"); + + assert!(error.contains("U+202E")); +} + +#[test] +fn team_export_rejects_unsafe_name_and_instructions() { + let definitions = vec![AgentDefinition { + id: "alice".to_string(), + display_name: "Alice".to_string(), + avatar_url: None, + system_prompt: "Alice prompt".to_string(), + runtime: Some("goose".to_string()), + model: None, + provider: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + env_vars: Default::default(), + respond_to: None, + respond_to_allowlist: vec![], + parallelism: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }]; + let mut team = TeamRecord { + id: "review".to_string(), + name: "Review Team".to_string(), + description: None, + instructions: Some("Be thorough.".to_string()), + persona_ids: vec!["alice".to_string()], + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "now".to_string(), + updated_at: "now".to_string(), + }; + + team.name = "Review\u{200B} Team".to_string(); + let error = build_team_export_snapshot( + &team, + &definitions, + &[], + MemoryLevel::None, + &std::collections::HashMap::new(), + ) + .expect_err("export must reject an invisible team name"); + assert!(error.contains("U+200B"), "unexpected error: {error}"); + + team.name = "Review Team".to_string(); + team.instructions = Some("Be\u{202E} thorough.".to_string()); + let error = build_team_export_snapshot( + &team, + &definitions, + &[], + MemoryLevel::None, + &std::collections::HashMap::new(), + ) + .expect_err("export must reject bidi formatting in team instructions"); + assert!(error.contains("U+202E"), "unexpected error: {error}"); +} + #[test] fn team_import_keeps_or_clears_every_member_allowlist_with_one_toggle() { let source = snapshot(vec![member("Alice"), member("Bob")]); diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index e17c5bdb247..a6366bdf1e2 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -6,7 +6,7 @@ use crate::{ managed_agents::{ delete_team_with_cascade, ensure_persona_ids_are_active, load_managed_agents, load_personas, load_teams, save_managed_agents, save_teams, try_regenerate_nest, - CreateTeamRequest, TeamRecord, UpdateTeamRequest, + validate_team_definition_text, CreateTeamRequest, TeamRecord, UpdateTeamRequest, }, util::now_iso, }; @@ -317,6 +317,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result Result, +) -> Result<(), String> { + if name.trim().is_empty() { + return Err("Team name is required".to_string()); + } + let name_chars = name.chars().count(); + if name_chars > MAX_DISPLAY_NAME_CHARS { + return Err(format!( + "Team name is too long ({name_chars} characters, max {MAX_DISPLAY_NAME_CHARS})" + )); + } + validate_visible_text(name, "Team name", false)?; + + let Some(instructions) = instructions else { + return Ok(()); + }; + if instructions.len() > MAX_SYSTEM_PROMPT_BYTES { + return Err(format!( + "Team instructions are too long ({} bytes, max {MAX_SYSTEM_PROMPT_BYTES})", + instructions.len() + )); + } + validate_visible_text(instructions, "Team instructions", true) +} + fn validate_visible_text( value: &str, label: &str, @@ -267,4 +299,32 @@ mod tests { ) .is_ok()); } + + #[test] + fn accepts_plain_team_name_and_multiline_instructions() { + assert!(validate_team_definition_text( + "Review Team 🐝", + Some("Review changes.\n\tCall out security risks."), + ) + .is_ok()); + assert!(validate_team_definition_text("Review Team", None).is_ok()); + } + + #[test] + fn rejects_invisible_characters_in_team_name_or_instructions() { + assert!( + validate_team_definition_text("Review\u{200B} Team", Some("Be thorough.")).is_err() + ); + assert!( + validate_team_definition_text("Review Team", Some("Be\u{200B} thorough.")).is_err() + ); + } + + #[test] + fn enforces_team_name_and_instruction_bounds() { + assert!(validate_team_definition_text(&"a".repeat(129), None).is_err()); + assert!( + validate_team_definition_text("Review Team", Some(&"a".repeat(64 * 1024 + 1))).is_err() + ); + } } diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 16234aa3d69..df410becda0 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -55,6 +55,7 @@ pub(crate) fn lock_path_mutex() -> std::sync::MutexGuard<'static, ()> { pub use backend::*; pub(crate) use definition_validation::{ validate_agent_definition_text, validate_managed_agent_definition_text, + validate_team_definition_text, }; pub use discovery::*; pub use env_vars::*; diff --git a/desktop/src-tauri/src/managed_agents/team_events.rs b/desktop/src-tauri/src/managed_agents/team_events.rs index 64861c0dec6..51801eb03eb 100644 --- a/desktop/src-tauri/src/managed_agents/team_events.rs +++ b/desktop/src-tauri/src/managed_agents/team_events.rs @@ -64,8 +64,12 @@ pub fn team_event_content(record: &TeamRecord) -> TeamEventContent { /// Build a kind:30176 event from a `TeamRecord`. /// -/// Returns an unsigned `EventBuilder` — the caller signs and submits. +/// Returns an unsigned `EventBuilder` — the caller signs and submits. Unsafe +/// names or instructions fail here so a dirty `teams.json` cannot be signed +/// during startup migration, matching [`super::agent_events::build_agent_event`]. pub fn build_team_event(record: &TeamRecord) -> Result { + super::validate_team_definition_text(&record.name, record.instructions.as_deref()) + .map_err(|error| format!("Team definition is unsafe to publish: {error}"))?; let content = serde_json::to_string(&team_event_content(record)) .map_err(|e| format!("failed to serialize team content: {e}"))?; let tags = @@ -129,6 +133,21 @@ mod tests { assert_eq!(event.kind.as_u16() as u32, KIND_TEAM); } + #[test] + fn publication_rejects_unsafe_team_name_and_instructions() { + let mut unsafe_name = sample_team(); + unsafe_name.name = "Review\u{200B} Team".to_string(); + let error = build_team_event(&unsafe_name) + .expect_err("publication must reject an invisible team name"); + assert!(error.contains("U+200B"), "unexpected error: {error}"); + + let mut unsafe_instructions = sample_team(); + unsafe_instructions.instructions = Some("Be\u{202E} thorough.".to_string()); + let error = build_team_event(&unsafe_instructions) + .expect_err("publication must reject bidi formatting in team instructions"); + assert!(error.contains("U+202E"), "unexpected error: {error}"); + } + #[test] fn d_tag_is_team_id() { let builder = build_team_event(&sample_team()).unwrap();