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
22 changes: 21 additions & 1 deletion desktop/src-tauri/src/commands/personas/inbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,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 @@ -168,7 +174,8 @@ fn reconcile_inbound_persona_event_blocking(
}
KIND_TEAM => {
let mut teams = load_teams(&app)?;
apply_inbound_team(&mut teams, d_tag, team_content_from_event(&event)?);
// Parsed and validated above, before retention.
apply_inbound_team(&mut teams, d_tag, inbound_team.expect("team parsed above"));
save_teams(&app, &teams)?;
}
KIND_MANAGED_AGENT => {
Expand Down Expand Up @@ -198,6 +205,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 @@ -733,3 +733,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());
}
11 changes: 11 additions & 0 deletions desktop/src-tauri/src/commands/team_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,17 @@ pub(crate) fn build_import_team(
if name.is_empty() {
return Err("Team snapshot name is empty.".to_string());
}
// Same review contract the agent-snapshot path applies to its definition
// text (`agent_snapshot.rs`). A snapshot is a file from outside this
// install, and a team's instructions are runtime-layered into every member
// deployment -- so this is the one place they are reviewed before they can
// execute. Phase 1 runs before key minting and before any store write, so a
// rejection here leaves nothing behind.
crate::managed_agents::validate_team_definition_text(
name,
snapshot.team.instructions.as_deref(),
)
.map_err(|error| format!("Team snapshot is unsafe: {error}"))?;

Ok(TeamRecord {
id: Uuid::new_v4().to_string(),
Expand Down
56 changes: 56 additions & 0 deletions desktop/src-tauri/src/commands/team_snapshot/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,3 +761,59 @@ mod egress_guard_boundary {
assert!(err.contains("key-backup material"), "{err}");
}
}

// ── Snapshot text review contract ────────────────────────────────────────
//
// A team snapshot is a file from outside this install and its `instructions`
// are runtime-layered into every member deployment, so import is the one place
// they are reviewed before they can execute. The agent-snapshot path already
// validates its definition text; this covers the team wrapper.

fn snapshot_with_team_text(name: &str, instructions: Option<&str>) -> TeamSnapshot {
let mut snap = snapshot(vec![member("Alice")]);
snap.team.name = name.to_string();
snap.team.instructions = instructions.map(str::to_string);
snap
}

fn import_team(snap: &TeamSnapshot) -> Result<TeamRecord, String> {
build_import_team(snap, vec!["persona-1".to_string()], "now")
}

#[test]
fn a_snapshot_with_concealed_instructions_is_refused() {
let snap = snapshot_with_team_text("Review Team", Some("Be thorough.\u{200B}"));
let error = import_team(&snap).unwrap_err();
assert!(error.starts_with("Team snapshot is unsafe"), "{error}");
assert!(error.contains("U+200B"), "{error}");
}

#[test]
fn a_snapshot_with_a_concealed_team_name_is_refused() {
let snap = snapshot_with_team_text("Review\u{202E} Team", Some("Be thorough."));
assert!(import_team(&snap).is_err());
}

#[test]
fn an_ordinary_snapshot_still_imports() {
let snap = snapshot_with_team_text("Review Team", Some("Be thorough.\n\tCheck tests."));
let team = import_team(&snap).unwrap();
assert_eq!(team.name, "Review Team");
assert_eq!(
team.instructions.as_deref(),
Some("Be thorough.\n\tCheck tests.")
);
}

#[test]
fn a_snapshot_carrying_no_instructions_still_imports() {
let team = import_team(&snapshot_with_team_text("Review Team", None)).unwrap();
assert_eq!(team.instructions, None);
}

#[test]
fn an_empty_team_name_is_still_refused_with_its_own_message() {
// The pre-existing emptiness check runs first and keeps its wording.
let error = import_team(&snapshot_with_team_text(" ", Some("Be thorough."))).unwrap_err();
assert_eq!(error, "Team snapshot name is empty.");
}
7 changes: 6 additions & 1 deletion desktop/src-tauri/src/commands/teams.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use crate::{
app_state::AppState,
managed_agents::{
delete_team_with_cascade, ensure_persona_ids_are_active, load_personas, load_teams,
save_teams, try_regenerate_nest, CreateTeamRequest, TeamRecord, UpdateTeamRequest,
save_teams, try_regenerate_nest, validate_team_definition_text, CreateTeamRequest,
TeamRecord, UpdateTeamRequest,
},
util::now_iso,
};
Expand Down Expand Up @@ -148,6 +149,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 @@ -189,6 +193,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 @@ -21,24 +21,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 the human-reviewed definition text carried by a managed agent.
Expand Down Expand Up @@ -157,6 +192,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
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 @@ -54,6 +54,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