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
25 changes: 24 additions & 1 deletion desktop/src-tauri/src/commands/personas/inbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ fn reconcile_inbound_persona_event_blocking<R: tauri::Runtime>(
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 @@ -299,12 +305,16 @@ fn reconcile_inbound_persona_event_blocking<R: tauri::Runtime>(
}
KIND_TEAM => {
let team_id = d_tag.clone();
// Parsed and validated above, before retention — reuse instead of
// re-parsing so an unsafe event never reaches the local store.
let inbound = inbound_team
.ok_or_else(|| "team content was not parsed before retention".to_string())?;
let outcome = commit_inbound_with_store(&conn, &inbound_retained_event, || {
let mut teams = load_teams(&app)?;
commit_inbound_team(
&mut teams,
d_tag,
team_content_from_event(&event)?,
inbound,
|teams| save_teams(&app, teams),
|| load_managed_agents(&app),
|records| save_managed_agents(&app, records),
Expand Down Expand Up @@ -457,6 +467,19 @@ fn validate_inbound_persona_definition(persona: &AgentDefinition) -> Result<(),
.map_err(|error| format!("Inbound persona definition is unsafe: {error}"))
}

/// Team `instructions` are runtime-layered into every member deployment, so an
/// inbound team carries executable text under the same review contract as a
/// persona. The wire type is a double option: absent means "publisher predates
/// always-publish, preserve local" and `null` means "explicitly cleared" --
/// neither delivers text to validate.
fn validate_inbound_team_definition(team: &TeamEventContent) -> Result<(), String> {
crate::managed_agents::validate_team_definition_text(
&team.name,
team.instructions.clone().flatten().as_deref(),
)
.map_err(|error| format!("Inbound team definition is unsafe: {error}"))
}

fn validate_inbound_managed_agent_definition(
managed_agent: &ManagedAgentEventContent,
) -> Result<(), String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,3 +918,49 @@ fn inbound_definition_less_agent_accepts_visible_multiline_prompt() {

assert!(validate_inbound_managed_agent_definition(&inbound).is_ok());
}

// ── Inbound team definition gate ─────────────────────────────────────────
//
// Team `instructions` are runtime-layered into every member deployment, so an
// inbound kind:30176 carries executable text. It is now parsed and validated
// alongside persona and managed-agent content -- before retention -- so an
// unsafe team stays out of both the retention database and the local store.

#[test]
fn inbound_team_with_concealed_instructions_is_rejected() {
let mut content = team_content("Release Team");
content.instructions = Some(Some("Ship it.\u{200B}".to_string()));

let error = validate_inbound_team_definition(&content).unwrap_err();
assert!(
error.starts_with("Inbound team definition is unsafe"),
"{error}"
);
assert!(error.contains("U+200B"), "{error}");
}

#[test]
fn inbound_team_with_a_concealed_name_is_rejected() {
let content = team_content("Release\u{202E} Team");
assert!(validate_inbound_team_definition(&content).is_err());
}

#[test]
fn an_ordinary_inbound_team_is_accepted() {
assert!(validate_inbound_team_definition(&team_content("Release Team")).is_ok());
}

#[test]
fn inbound_team_omitting_instructions_carries_no_text_to_validate() {
// Absent means "publisher predates always-publish, preserve local" and
// `null` means "explicitly cleared". Neither delivers text, and neither may
// be rejected as if it had.
assert!(
validate_inbound_team_definition(&team_content_omitting_optional_fields("Release Team"))
.is_ok()
);

let mut cleared = team_content("Release Team");
cleared.instructions = Some(None);
assert!(validate_inbound_team_definition(&cleared).is_ok());
}
7 changes: 6 additions & 1 deletion desktop/src-tauri/src/commands/teams/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ 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,
AgentDefinition, CreateTeamRequest, TeamRecord, UpdateTeamRequest,
validate_team_definition_text, AgentDefinition, CreateTeamRequest, TeamRecord,
UpdateTeamRequest,
},
util::now_iso,
};
Expand Down Expand Up @@ -425,6 +426,9 @@ 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);
// Before the store lock and before any load/save, so a rejected team
// cannot leave a partial write behind.
validate_team_definition_text(&name, instructions.as_deref())?;
let now = now_iso();

let _store_guard = state
Expand Down Expand Up @@ -475,6 +479,7 @@ 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_team_definition_text(&name, instructions.as_deref())?;

let _store_guard = state
.managed_agents_store_lock
Expand Down
125 changes: 115 additions & 10 deletions desktop/src-tauri/src/managed_agents/definition_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,59 @@ pub(crate) fn validate_agent_definition_text(
display_name: &str,
system_prompt: &str,
) -> Result<(), String> {
if display_name.trim().is_empty() {
return Err("Display name is required".to_string());
validate_reviewed_text(
display_name,
"Display name",
system_prompt,
"Agent instructions",
)
}

/// Validate the human-reviewed text carried by a team.
///
/// A team's `instructions` are runtime-layered into every member deployment,
/// so they are executable text under the same review contract as an agent
/// definition. A team carrying no instructions has no executable text and only
/// its name is checked.
pub(crate) fn validate_team_definition_text(
name: &str,
instructions: Option<&str>,
) -> Result<(), String> {
validate_reviewed_text(
name,
"Team name",
instructions.unwrap_or_default(),
"Team instructions",
)
}

/// Shared contract for reviewed-then-executed text. The labels differ per
/// surface so the error a person sees names the field they were editing; the
/// limits and the invisible-character rules are deliberately identical.
fn validate_reviewed_text(
name: &str,
name_label: &str,
instructions: &str,
instructions_label: &str,
) -> Result<(), String> {
if name.trim().is_empty() {
return Err(format!("{name_label} is required"));
}
let display_name_chars = display_name.chars().count();
if display_name_chars > MAX_DISPLAY_NAME_CHARS {
let name_chars = name.chars().count();
if name_chars > MAX_DISPLAY_NAME_CHARS {
return Err(format!(
"Display name is too long ({display_name_chars} characters, max {MAX_DISPLAY_NAME_CHARS})"
"{name_label} is too long ({name_chars} characters, max {MAX_DISPLAY_NAME_CHARS})"
));
}
if system_prompt.len() > MAX_SYSTEM_PROMPT_BYTES {
if instructions.len() > MAX_SYSTEM_PROMPT_BYTES {
return Err(format!(
"Agent instructions are too long ({} bytes, max {MAX_SYSTEM_PROMPT_BYTES})",
system_prompt.len()
"{instructions_label} are too long ({} bytes, max {MAX_SYSTEM_PROMPT_BYTES})",
instructions.len()
));
}

validate_visible_text(display_name, "Display name", false)?;
validate_visible_text(system_prompt, "Agent instructions", true)
validate_visible_text(name, name_label, false)?;
validate_visible_text(instructions, instructions_label, true)
}

/// Validate an optional public agent description: max 280 characters and the
Expand Down Expand Up @@ -183,6 +218,76 @@ fn is_default_ignorable(character: char) -> bool {
mod tests {
use super::*;

// ── Team text: same contract, team-shaped labels ─────────────────────────
//
// Team `instructions` are runtime-layered into every member deployment, so
// they are executed exactly like an agent's `system_prompt`. Before this
// they were the one shared executable text with no review contract at all,
// which made the same hidden characters safer in a team than in the agent
// wrapped by it.

#[test]
fn team_text_rejects_the_same_invisible_characters_as_an_agent() {
for character in [
'\u{00AD}',
'\u{034F}',
'\u{200B}',
'\u{202E}',
'\u{2060}',
'\u{2066}',
'\u{3164}',
'\u{E007F}',
] {
let name = format!("Release{character} Team");
let instructions = format!("Ship the release.{character}");
assert!(validate_team_definition_text(&name, Some("Ship it.")).is_err());
assert!(validate_team_definition_text("Release Team", Some(&instructions)).is_err());
}
}

#[test]
fn team_text_accepts_ordinary_whitespace_and_emoji() {
assert!(validate_team_definition_text(
"Release Team 🚀",
Some("Ship the release.\n\tPost the ledger row 🚀")
)
.is_ok());
}

#[test]
fn a_team_without_instructions_carries_no_executable_text() {
assert!(validate_team_definition_text("Release Team", None).is_ok());
}

#[test]
fn team_errors_name_the_team_field_the_person_was_editing() {
let name_error = validate_team_definition_text("", Some("Ship it.")).unwrap_err();
assert!(
name_error.starts_with("Team name"),
"expected a team-shaped error, got {name_error}"
);

let long_instructions = "x".repeat(MAX_SYSTEM_PROMPT_BYTES + 1);
let instructions_error =
validate_team_definition_text("Release Team", Some(&long_instructions)).unwrap_err();
assert!(
instructions_error.starts_with("Team instructions"),
"expected a team-shaped error, got {instructions_error}"
);
}

#[test]
fn agent_error_wording_is_unchanged_by_the_shared_contract() {
assert_eq!(
validate_agent_definition_text("", "Review code.").unwrap_err(),
"Display name is required"
);
let long_prompt = "x".repeat(MAX_SYSTEM_PROMPT_BYTES + 1);
assert!(validate_agent_definition_text("Reviewer", &long_prompt)
.unwrap_err()
.starts_with("Agent instructions are too long"));
}

#[test]
fn accepts_plain_multiline_instructions() {
assert!(validate_agent_definition_text(
Expand Down
3 changes: 2 additions & 1 deletion desktop/src-tauri/src/managed_agents/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ pub(crate) fn lock_path_mutex() -> std::sync::MutexGuard<'static, ()> {
pub use backend::*;
pub(crate) use definition_validation::{
validate_agent_definition_text, validate_agent_description_text,
validate_managed_agent_definition_text, validate_visible_text,
validate_managed_agent_definition_text, validate_team_definition_text,
validate_visible_text,
};
pub use discovery::*;
pub use env_vars::*;
Expand Down