Harden shared agent instruction review

desktop/src-tauri/src/commands/agent_models.rs

@@ -5,6 +5,7 @@ use serde::Deserialize;
 use tauri::{AppHandle, State};
 
 use super::agent_model_process::run_agent_models_command;
+use super::managed_agent_definition::apply_model_provider_prompt_update;
 // The map-only lookup is reached solely from the base-URL helpers that exist for
 // their unit tests; discovery itself always goes through the process-env variant.
 #[cfg(test)]
@@ -696,35 +697,6 @@ use databricks::{
 };
 use databricks::{discover_databricks_models, DatabricksAuthIntent};
 
-/// Apply an `UpdateManagedAgentRequest`'s model/provider/system_prompt patch
-/// to `record`, enforcing the linked-instance write guard: a definition-linked
-/// record's model/provider/prompt are definition-authoritative (see
-/// `effective_config::resolve_linked`), so writes to these three fields are
-/// silently dropped for a linked instance rather than persisting a byte the
-/// resolver will never read. Definition-less instances accept the patch
-/// as-is. Extracted so the guard is exercised by both `update_managed_agent`
-/// and its regression tests — a test that reimplements this check instead of
-/// calling it can go green after the real guard is deleted.
-fn apply_model_provider_prompt_update(
-    record: &mut crate::managed_agents::ManagedAgentRecord,
-    model: Option<Option<String>>,
-    provider: Option<Option<String>>,
-    system_prompt: Option<Option<String>>,
-) {
-    if record.persona_id.is_some() {
-        return;
-    }
-    if let Some(model_update) = model {
-        record.model = model_update;
-    }
-    if let Some(provider_update) = provider {
-        record.provider = provider_update;
-    }
-    if let Some(prompt_update) = system_prompt {
-        record.system_prompt = prompt_update;
-    }
-}
-
 /// Update mutable fields on an existing managed agent record.
 ///
 /// Does NOT auto-restart the agent. Runtime config changes (system prompt,
@@ -769,7 +741,7 @@ pub async fn update_managed_agent(
             input.model,
             input.provider,
             input.system_prompt,
-        );
+        )?;
         if let Some(parallelism) = input.parallelism {
             record.parallelism = parallelism;
         }

desktop/src-tauri/src/commands/agent_models_tests.rs

@@ -509,7 +509,8 @@ fn linked_instance_ignores_model_provider_prompt_writes() {
         Some(Some("explicit-model".to_string())),
         Some(Some("explicit-prov".to_string())),
         Some(Some("explicit-prompt".to_string())),
-    );
+    )
+    .unwrap();
 
     assert!(
         record.model.is_none(),
@@ -560,7 +561,8 @@ fn definition_less_instance_accepts_model_provider_prompt_writes() {
         Some(Some("new-model".to_string())),
         Some(Some("new-prov".to_string())),
         Some(Some("new-prompt".to_string())),
-    );
+    )
+    .unwrap();
 
     assert_eq!(record.model.as_deref(), Some("new-model"));
     assert_eq!(record.provider.as_deref(), Some("new-prov"));

desktop/src-tauri/src/commands/agents.rs

@@ -1,6 +1,8 @@
 use nostr::{Keys, ToBech32};
 use tauri::{AppHandle, State};
 
+use super::managed_agent_definition::validate_create_definition;
+
 use crate::{
     app_state::AppState,
     managed_agents::{
@@ -568,15 +570,13 @@ pub async fn create_managed_agent(
     state: State<'_, AppState>,
 ) -> Result<CreateManagedAgentResponse, String> {
     let name = input.name.trim().to_string();
-    if name.is_empty() {
-        return Err("agent name is required".to_string());
-    }
     let requested_persona_id = input
         .persona_id
         .as_deref()
         .map(str::trim)
         .filter(|value| !value.is_empty())
         .map(str::to_string);
+    validate_create_definition(&name, requested_persona_id.as_deref(), &input)?;
     if let Some(parallelism) = input.parallelism {
         if !(1..=32).contains(&parallelism) {
             return Err("parallelism must be between 1 and 32".to_string());

desktop/src-tauri/src/commands/managed_agent_definition.rs

@@ -0,0 +1,124 @@
+//! Managed-agent definition validation at local mutation boundaries.
+
+use crate::managed_agents::{CreateManagedAgentRequest, ManagedAgentRecord};
+
+pub(super) fn validate_create_definition(
+    name: &str,
+    persona_id: Option<&str>,
+    input: &CreateManagedAgentRequest,
+) -> Result<(), String> {
+    validate_definition_fields(name, persona_id, input.system_prompt.as_deref())
+}
+
+fn validate_definition_fields(
+    name: &str,
+    persona_id: Option<&str>,
+    system_prompt: Option<&str>,
+) -> Result<(), String> {
+    crate::managed_agents::validate_managed_agent_definition_text(name, persona_id, system_prompt)
+        .map_err(|error| format!("Managed agent definition is unsafe: {error}"))
+}
+
+/// Apply definition-owned update fields, then validate the complete
+/// prospective definition before the caller can persist it.
+pub(super) fn apply_model_provider_prompt_update(
+    record: &mut ManagedAgentRecord,
+    model: Option<Option<String>>,
+    provider: Option<Option<String>>,
+    system_prompt: Option<Option<String>>,
+) -> Result<(), String> {
+    if record.persona_id.is_none() {
+        if let Some(model_update) = model {
+            record.model = model_update;
+        }
+        if let Some(provider_update) = provider {
+            record.provider = provider_update;
+        }
+        if let Some(prompt_update) = system_prompt {
+            record.system_prompt = prompt_update;
+        }
+    }
+
+    validate_definition_fields(
+        &record.name,
+        record.persona_id.as_deref(),
+        record.system_prompt.as_deref(),
+    )
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn standalone_record() -> ManagedAgentRecord {
+        serde_json::from_value(serde_json::json!({
+            "pubkey": "standalone1",
+            "name": "standalone-agent",
+            "private_key_nsec": "nsec1fake",
+            "relay_url": "wss://localhost:3000",
+            "acp_command": "buzz-acp",
+            "agent_command": "goose",
+            "agent_args": [],
+            "mcp_command": "",
+            "turn_timeout_seconds": 320,
+            "system_prompt": "safe prompt",
+            "model": null,
+            "provider": null,
+            "env_vars": {},
+            "created_at": "2026-01-01T00:00:00Z",
+            "updated_at": "2026-01-01T00:00:00Z",
+            "last_started_at": null,
+            "last_stopped_at": null,
+            "last_exit_code": null,
+            "last_error": null
+        }))
+        .expect("standalone agent record")
+    }
+
+    fn create_request(system_prompt: &str) -> CreateManagedAgentRequest {
+        serde_json::from_value(serde_json::json!({
+            "name": "Reviewer",
+            "systemPrompt": system_prompt
+        }))
+        .expect("create request")
+    }
+
+    #[test]
+    fn create_rejects_invisible_definition_less_name_or_prompt() {
+        for (name, prompt, code) in [
+            ("Review\u{200B}er", "Review code.", "U+200B"),
+            ("Reviewer", "Review\u{202E} code.", "U+202E"),
+        ] {
+            let input = create_request(prompt);
+            let error = validate_create_definition(name, None, &input)
+                .expect_err("create must reject unsafe definition text");
+            assert!(error.contains(code), "unexpected error: {error}");
+        }
+    }
+
+    #[test]
+    fn create_accepts_visible_multiline_definition_less_prompt() {
+        let input = create_request("Review changes.\n\tCall out security risks.");
+        validate_create_definition("Reviewer 🐝", None, &input)
+            .expect("visible multiline instructions should remain valid");
+    }
+
+    #[test]
+    fn update_rejects_invisible_definition_less_name_or_prompt() {
+        let mut unsafe_prompt = standalone_record();
+        let error = apply_model_provider_prompt_update(
+            &mut unsafe_prompt,
+            None,
+            None,
+            Some(Some("Review\u{200B} code.".to_string())),
+        )
+        .expect_err("definition-less prompt update must reject invisible text");
+        assert!(error.contains("U+200B"), "unexpected error: {error}");
+
+        let mut unsafe_name = standalone_record();
+        unsafe_name.name = "Review\u{202E}er".to_string();
+        let error = apply_model_provider_prompt_update(&mut unsafe_name, None, None, None)
+            .expect_err("definition-less name update must reject formatting controls");
+        assert!(error.contains("U+202E"), "unexpected error: {error}");
+    }
+}

desktop/src-tauri/src/commands/mod.rs

@@ -25,6 +25,7 @@ mod identity_archive;
 mod join_policy;
 mod legacy_storage;
 mod link_preview;
+mod managed_agent_definition;
 pub(crate) mod media;
 mod media_animated;
 mod media_download;

desktop/src-tauri/src/commands/personas/create.rs

@@ -7,8 +7,8 @@ use uuid::Uuid;
 use crate::{
     app_state::AppState,
     managed_agents::{
-        apply_persona_behavior, load_personas, save_personas, try_regenerate_nest, AgentDefinition,
-        CatalogSource, CreatePersonaRequest,
+        apply_persona_behavior, load_personas, save_personas, try_regenerate_nest,
+        validate_agent_definition_text, AgentDefinition, CatalogSource, CreatePersonaRequest,
     },
     util::now_iso,
 };
@@ -25,7 +25,10 @@ pub async fn create_persona(
         let state = app.state::<AppState>();
         let display_name = trim_required(&input.display_name, "Display name")?;
         // System prompt optional: core memory is auto-injected. Empty is valid.
-        let system_prompt = input.system_prompt.trim().to_string();
+        // Preserve it byte-for-byte: shared/import review surfaces show this
+        // exact string before the ACP harness executes it.
+        let system_prompt = input.system_prompt.clone();
+        validate_agent_definition_text(&display_name, &system_prompt)?;
         let avatar_url = trim_optional(input.avatar_url);
         let runtime = trim_optional(input.runtime);
         let model = trim_optional(input.model);

desktop/src-tauri/src/commands/personas/inbound.rs

@@ -102,12 +102,21 @@ fn reconcile_inbound_persona_event_blocking(
 
     // The d-tag identifies the record within its kind. Persona derives it from
     // the parsed record (`persona_d_tag`); team/agent carry it as the event's
-    // d-tag directly. The persona is parsed once here and reused in the apply
-    // branch below — team/agent content is parsed in-branch since their d-tag
-    // comes from the event tag, not the content.
+    // d-tag directly. Definition-bearing content is parsed and validated once
+    // here, before retention, then reused in the apply branch below. This keeps
+    // an unsafe event out of both the retention database and the local store.
     let inbound_persona = (kind == KIND_PERSONA)
         .then(|| persona_from_event(&event))
         .transpose()?;
+    if let Some(persona) = &inbound_persona {
+        validate_inbound_persona_definition(persona)?;
+    }
+    let inbound_managed_agent = (kind == KIND_MANAGED_AGENT)
+        .then(|| managed_agent_content_from_event(&event))
+        .transpose()?;
+    if let Some(managed_agent) = &inbound_managed_agent {
+        validate_inbound_managed_agent_definition(managed_agent)?;
+    }
     let d_tag = match &inbound_persona {
         Some(persona) => persona_d_tag(persona),
         None => event_d_tag(&event)?,
@@ -164,11 +173,10 @@ fn reconcile_inbound_persona_event_blocking(
         }
         KIND_MANAGED_AGENT => {
             let mut agents = load_managed_agents(&app)?;
-            apply_inbound_managed_agent(
-                &mut agents,
-                &d_tag,
-                managed_agent_content_from_event(&event)?,
-            );
+            let managed_agent = inbound_managed_agent.ok_or_else(|| {
+                "managed-agent content was not parsed before retention".to_string()
+            })?;
+            apply_inbound_managed_agent(&mut agents, &d_tag, managed_agent);
             save_managed_agents(&app, &agents)?;
         }
         _ => unreachable!("kind gated above"),
@@ -182,6 +190,25 @@ fn reconcile_inbound_persona_event_blocking(
     Ok(())
 }
 
+fn validate_inbound_persona_definition(persona: &AgentDefinition) -> Result<(), String> {
+    crate::managed_agents::validate_agent_definition_text(
+        &persona.display_name,
+        &persona.system_prompt,
+    )
+    .map_err(|error| format!("Inbound persona definition is unsafe: {error}"))
+}
+
+fn validate_inbound_managed_agent_definition(
+    managed_agent: &ManagedAgentEventContent,
+) -> Result<(), String> {
+    crate::managed_agents::validate_managed_agent_definition_text(
+        &managed_agent.name,
+        managed_agent.persona_id.as_deref(),
+        managed_agent.system_prompt.as_deref(),
+    )
+    .map_err(|error| format!("Inbound managed-agent 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

desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs

@@ -4,7 +4,7 @@
 use super::*;
 use std::collections::BTreeMap;
 
-const UUID: &str = "11111111-2222-3333-4444-555555555555";
+const UUID: &str = "11111111-2222-3333-4444-555555555555"; // sadscan:disable sq.pii.cc.visa -- fixed test UUID
 
 /// A local in-app persona: `source_team_persona_slug` is None, so its d-tag
 /// IS its UUID id. Carries env_vars + source_team that must survive a patch.
@@ -673,3 +673,63 @@ fn inbound_gate_accepts_validly_signed_event() {
     let parsed = parse_verified_inbound_event(&event.as_json()).unwrap();
     assert_eq!(parsed.pubkey, keys.public_key());
 }
+
+#[test]
+fn inbound_persona_rejects_invisible_definition_text() {
+    let mut inbound = inbound_for("unsafe", "Remote");
+    inbound.system_prompt = "Review\u{200B} code.".to_string();
+
+    let error = validate_inbound_persona_definition(&inbound)
+        .expect_err("relay sync must reject invisible instructions");
+
+    assert!(error.contains("U+200B"));
+}
+
+fn inbound_managed_agent_content(
+    name: &str,
+    persona_id: Option<&str>,
+    system_prompt: Option<&str>,
+) -> crate::managed_agents::agent_events::ManagedAgentEventContent {
+    crate::managed_agents::agent_events::ManagedAgentEventContent {
+        name: name.to_string(),
+        persona_id: persona_id.map(str::to_string),
+        system_prompt: system_prompt.map(str::to_string),
+        model: None,
+        provider: None,
+        persona_source_version: None,
+        parallelism: 1,
+        respond_to: crate::managed_agents::RespondTo::OwnerOnly,
+        respond_to_allowlist: vec![],
+    }
+}
+
+#[test]
+fn inbound_definition_less_agent_rejects_invisible_prompt() {
+    let inbound = inbound_managed_agent_content("Remote Agent", None, Some("Review\u{200B} code."));
+
+    let error = validate_inbound_managed_agent_definition(&inbound)
+        .expect_err("definition-less sync must reject invisible instructions");
+
+    assert!(error.contains("U+200B"));
+}
+
+#[test]
+fn inbound_managed_agent_rejects_bidirectional_name() {
+    let inbound = inbound_managed_agent_content("Remote\u{202E} Agent", None, None);
+
+    let error = validate_inbound_managed_agent_definition(&inbound)
+        .expect_err("managed-agent sync must reject bidirectional names");
+
+    assert!(error.contains("U+202E"));
+}
+
+#[test]
+fn inbound_definition_less_agent_accepts_visible_multiline_prompt() {
+    let inbound = inbound_managed_agent_content(
+        "Remote Agent",
+        None,
+        Some("Review code.\n\tCall out security risks."),
+    );
+
+    assert!(validate_inbound_managed_agent_definition(&inbound).is_ok());
+}