Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion desktop/src-tauri/src/commands/personas/inbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
10 changes: 9 additions & 1 deletion desktop/src-tauri/src/commands/team_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -270,6 +272,8 @@ fn build_team_export_snapshot(
memory_level: MemoryLevel,
memory_entries_by_persona: &std::collections::HashMap<String, Vec<AgentSnapshotMemoryEntry>>,
) -> Result<TeamSnapshot, String> {
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()
Expand Down Expand Up @@ -461,6 +465,10 @@ pub async fn preview_team_snapshot_import(
) -> Result<TeamSnapshotImportPreview, String> {
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,
Expand Down
85 changes: 85 additions & 0 deletions desktop/src-tauri/src/commands/team_snapshot/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]);
Expand Down
6 changes: 5 additions & 1 deletion desktop/src-tauri/src/commands/teams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -317,6 +317,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result<Tea
let name = trim_required(&input.name, "Team name")?;
let description = trim_optional(input.description);
let instructions = trim_optional(input.instructions);
validate_team_definition_text(&name, instructions.as_deref())?;
let now = now_iso();

let _store_guard = state
Expand Down Expand Up @@ -363,6 +364,9 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result<Tea
let name = trim_required(&input.name, "Team name")?;
let description = trim_optional(input.description);
let instructions = trim_optional(input.instructions);
// Validate before the store lock so a rejected update cannot leave a
// partial in-memory write, let alone a teams.json write.
validate_team_definition_text(&name, instructions.as_deref())?;

let _store_guard = state
.managed_agents_store_lock
Expand Down
70 changes: 65 additions & 5 deletions desktop/src-tauri/src/managed_agents/definition_validation.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Validation for human-reviewed agent definition text.
//! Validation for human-reviewed agent and team definition text.
//!
//! Shared definitions are executable configuration: `system_prompt` is shown
//! to a person, then delivered verbatim to an ACP harness. Characters that
//! consume input bytes without a visible glyph break that review invariant and
//! are rejected rather than silently stripped.
//! Shared definitions are executable configuration: agent `system_prompt` and
//! team instructions are shown to a person, then delivered verbatim to an ACP
//! harness. Characters that consume input bytes without a visible glyph break
//! that review invariant and are rejected rather than silently stripped.

use regex::Regex;
use std::sync::LazyLock;
Expand Down Expand Up @@ -60,6 +60,38 @@ pub(crate) fn validate_managed_agent_definition_text(
validate_agent_definition_text(name, executable_prompt)
}

/// Validate the human-visible fields of a team definition.
///
/// Team names and instructions are shared and executed at launch, so they use
/// the same reviewable-text contract as agent definitions. Absent instructions
/// are allowed; present instructions must be reviewable.
pub(crate) fn validate_team_definition_text(
name: &str,
instructions: Option<&str>,
) -> 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,
Expand Down Expand Up @@ -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()
);
}
}
1 change: 1 addition & 0 deletions desktop/src-tauri/src/managed_agents/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
21 changes: 20 additions & 1 deletion desktop/src-tauri/src/managed_agents/team_events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<EventBuilder, String> {
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 =
Expand Down Expand Up @@ -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();
Expand Down