fix(agent): guard internal Aion CLI command overrides (#538)

kaizhou-lab · web-flow · commit f141233b0cbe · 2026-06-29T20:51:22.000+08:00
## Summary - reject command overrides for the internal Aion CLI agent - ignore and mask legacy Aion CLI command overrides at registry/read time - add migration 016 to clear polluted internal Aion CLI override data ## Tests - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-ai-agent decode_row_ignores_internal_aion_cli_command_override - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-app --test agent_integration_e2e internal_aion_cli_rejects_command_override - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-db --test cron_assistant_first_migration migration_016_clears_internal_aion_cli_command_override_only - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo fmt --all -- --check - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-ai-agent - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-db - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo clippy -p aionui-ai-agent -p aionui-db -p aionui-app -- -D warnings - CARGO_TARGET_DIR=/tmp/aioncore-codex-target cargo test -p aionui-app --test agent_integration_e2e - CARGO_TARGET_DIR=/tmp/aioncore-codex-target just push -u origin fix/internal-aion-cli-override-guard ## Investigation notes Sentry #81 shows PUT /api/agents/632f31d2/overrides polluted the internal Aion CLI row with command_override values like a web URL and `irm https://claude.ai/install.ps1 | iex`. The backend must treat this internal commandless agent as non-overridable regardless of caller. --------- Co-authored-by: zk <>
diff --git a/crates/aionui-ai-agent/src/registry.rs b/crates/aionui-ai-agent/src/registry.rs
@@ -191,8 +191,8 @@ impl AgentRegistry {
         for meta in guard.values_mut() {
             let (path, reason) = probe_with_reason(meta);
             meta.resolved_command = path;
-            meta.available = meta.resolved_command.is_some()
-                || (meta.enabled && meta.command.is_none() && meta.agent_source == AgentSource::Internal);
+            meta.available = meta.resolved_command.is_some() || is_internal_commandless_agent(meta);
+            let reason = if meta.available { None } else { reason };
             log_probe_result(meta, &reason);
         }
         log_availability_summary(guard.values(), "AgentRegistry refresh_availability complete");
@@ -464,15 +464,37 @@ fn decode_row(row: AgentMetadataRow) -> Option<(AgentMetadata, Option<Unavailabl
     // Layered on top of seed truth at this single projection point so both
     // the runtime spawn (factory) and the probe (availability) observe the
     // same merged command/env without either needing extra plumbing.
-    meta.has_command_override = meta_command_override(&command_override_raw).is_some();
-    meta.env_override_key_count = parse_env_override(&env_override_raw)
+    let command_override = meta_command_override(&command_override_raw);
+    let is_internal_aion_cli = is_internal_aion_cli(&meta);
+    if is_internal_aion_cli && command_override.is_some() {
+        warn!(
+            id = %meta.id,
+            name = %meta.name,
+            "Ignoring command override for internal Aion CLI agent"
+        );
+    }
+    let env_override = parse_env_override(&env_override_raw);
+    if is_internal_aion_cli && env_override.as_ref().is_some_and(|entries| !entries.is_empty()) {
+        warn!(
+            id = %meta.id,
+            name = %meta.name,
+            "Ignoring environment overrides for internal Aion CLI agent"
+        );
+    }
+
+    meta.has_command_override = command_override.is_some() && !is_internal_aion_cli;
+    meta.env_override_key_count = env_override
+        .as_ref()
+        .filter(|_| !is_internal_aion_cli)
         .map(|v| v.iter().filter(|e| !is_blocked_override_env_key(&e.name)).count())
         .unwrap_or(0);
 
-    if let Some(path) = meta_command_override(&command_override_raw) {
+    if is_internal_aion_cli {
+        meta.command = None;
+    } else if let Some(path) = command_override {
         meta.command = Some(path);
     }
-    if let Some(extra) = parse_env_override(&env_override_raw) {
+    if !is_internal_aion_cli && let Some(extra) = env_override {
         for entry in extra {
             if is_blocked_override_env_key(&entry.name) {
                 tracing::warn!(key = %entry.name, "env override: blocked key skipped");
@@ -484,11 +506,19 @@ fn decode_row(row: AgentMetadataRow) -> Option<(AgentMetadata, Option<Unavailabl
 
     let (path, reason) = probe_with_reason(&meta);
     meta.resolved_command = path;
-    meta.available = meta.resolved_command.is_some()
-        || (meta.enabled && meta.command.is_none() && meta.agent_source == AgentSource::Internal);
+    meta.available = meta.resolved_command.is_some() || is_internal_commandless_agent(&meta);
+    let reason = if meta.available { None } else { reason };
     Some((meta, reason))
 }
 
+fn is_internal_aion_cli(meta: &AgentMetadata) -> bool {
+    meta.agent_type == AgentType::Aionrs && meta.agent_source == AgentSource::Internal
+}
+
+fn is_internal_commandless_agent(meta: &AgentMetadata) -> bool {
+    meta.enabled && meta.command.is_none() && meta.agent_source == AgentSource::Internal
+}
+
 /// Wrapper around [`probe_resolved_command`] that returns both the
 /// resolved path (if any) and the failure reason as a tuple, so the
 /// hydrate / refresh loops can persist the path and emit a single
@@ -1294,6 +1324,61 @@ mod tests {
         assert_eq!(meta.command.as_deref(), Some("/opt/factory/bin/droid"));
     }
 
+    #[test]
+    fn decode_row_ignores_internal_aion_cli_overrides() {
+        use aionui_db::AgentMetadataRow;
+        let row = AgentMetadataRow {
+            id: "632f31d2".to_string(),
+            icon: None,
+            name: "Aion CLI".to_string(),
+            name_i18n: None,
+            description: None,
+            description_i18n: None,
+            backend: None,
+            agent_type: "aionrs".to_string(),
+            agent_source: "internal".to_string(),
+            agent_source_info: None,
+            enabled: true,
+            command: None,
+            command_override: Some("irm https://claude.ai/install.ps1 | iex".to_string()),
+            args: None,
+            env: None,
+            native_skills_dirs: None,
+            behavior_policy: None,
+            yolo_id: None,
+            agent_capabilities: None,
+            auth_methods: None,
+            config_options: None,
+            available_modes: None,
+            available_models: None,
+            available_commands: None,
+            sort_order: 0,
+            last_check_status: None,
+            last_check_kind: None,
+            last_check_error_code: None,
+            last_check_error_message: None,
+            last_check_guidance: None,
+            last_check_latency_ms: None,
+            last_check_at: None,
+            last_success_at: None,
+            last_failure_at: None,
+            env_override: Some(
+                r#"[{"name":"ANTHROPIC_API_KEY","value":"sk-x"},{"name":"PATH","value":"/evil"}]"#.to_string(),
+            ),
+            created_at: 0,
+            updated_at: 0,
+        };
+        let (meta, reason) = super::decode_row(row).expect("decodes");
+        assert_eq!(meta.agent_type, AgentType::Aionrs);
+        assert_eq!(meta.agent_source, AgentSource::Internal);
+        assert_eq!(meta.command, None);
+        assert!(!meta.has_command_override);
+        assert_eq!(meta.env_override_key_count, 0);
+        assert!(meta.env.is_empty());
+        assert!(meta.available);
+        assert!(reason.is_none());
+    }
+
     #[test]
     fn decode_row_appends_env_override_and_skips_blocked() {
         use aionui_db::AgentMetadataRow;
diff --git a/crates/aionui-ai-agent/src/services/agent.rs b/crates/aionui-ai-agent/src/services/agent.rs
@@ -140,7 +140,8 @@ impl AgentService {
         req: aionui_api_types::SetAgentOverridesRequest,
     ) -> Result<AgentManagementRow, AgentError> {
         let repo = self.registry.repo_handle();
-        repo.get(id)
+        let row = repo
+            .get(id)
             .await
             .map_err(|e| AgentError::internal(format!("repo.get: {e}")))?
             .ok_or_else(|| AgentError::not_found(format!("Agent '{id}' not found")))?;
@@ -151,6 +152,14 @@ impl AgentService {
             .map(str::trim)
             .filter(|s| !s.is_empty())
             .map(str::to_owned);
+        let has_env_override = req
+            .env_override
+            .as_ref()
+            .is_some_and(|entries| entries.iter().any(|entry| !entry.name.trim().is_empty()));
+
+        if (command_override.is_some() || has_env_override) && is_internal_aion_cli_row(&row) {
+            return Err(AgentError::bad_request("Internal Aion CLI does not support overrides"));
+        }
 
         let env_json = match req.env_override {
             Some(entries) if !entries.is_empty() => Some(
@@ -163,12 +172,8 @@ impl AgentService {
         repo.update_agent_overrides(id, command_override.as_deref(), env_json.as_deref())
             .await
             .map_err(|e| AgentError::internal(format!("repo.update_agent_overrides: {e}")))?;
-        self.registry.invalidate_and_rehydrate().await?;
 
-        self.availability
-            .management_row_by_id(id)
-            .await
-            .ok_or_else(|| AgentError::not_found(format!("Agent '{id}' not found")))
+        self.availability.run_manual_health_check(id).await
     }
 
     pub async fn get_agent_overrides(&self, id: &str) -> Result<aionui_api_types::AgentOverridesResponse, AgentError> {
@@ -187,8 +192,16 @@ impl AgentService {
             .unwrap_or_default();
 
         Ok(aionui_api_types::AgentOverridesResponse {
-            command_override: row.command_override,
+            command_override: if is_internal_aion_cli_row(&row) {
+                None
+            } else {
+                row.command_override
+            },
             env_override,
         })
     }
 }
+
+fn is_internal_aion_cli_row(row: &aionui_db::AgentMetadataRow) -> bool {
+    row.agent_type.eq_ignore_ascii_case("aionrs") && row.agent_source.eq_ignore_ascii_case("internal")
+}
diff --git a/crates/aionui-app/tests/agent_integration_e2e.rs b/crates/aionui-app/tests/agent_integration_e2e.rs
@@ -628,12 +628,15 @@ async fn agent_overrides_roundtrip_and_management_summary() {
 
     // PUT overrides
     let body = json!({
-        "command_override": "/real/bin/ovr",
+        "command_override": "true",
         "env_override": [{"name": "ANTHROPIC_API_KEY", "value": "sk-x"}, {"name": "PATH", "value": "/evil"}]
     });
     let req = json_with_token("PUT", "/api/agents/ovr-agent/overrides", body, &token, &csrf);
     let resp = app.clone().oneshot(req).await.unwrap();
     assert_eq!(resp.status(), StatusCode::OK);
+    let put_body = body_json(resp).await;
+    assert_eq!(put_body["data"]["last_check_kind"], "manual");
+    assert_eq!(put_body["data"]["last_check_status"], "offline");
 
     // management row: safe fields, blocked PATH not counted
     let mreq = get_with_token("/api/agents/management", &token);
@@ -659,7 +662,35 @@ async fn agent_overrides_roundtrip_and_management_summary() {
     // GET overrides: plaintext echo
     let greq = get_with_token("/api/agents/ovr-agent/overrides", &token);
     let gbody = body_json(app.clone().oneshot(greq).await.unwrap()).await;
-    assert_eq!(gbody["data"]["command_override"], "/real/bin/ovr");
+    assert_eq!(gbody["data"]["command_override"], "true");
     let envs = gbody["data"]["env_override"].as_array().unwrap();
     assert!(envs.iter().any(|e| e["name"] == "ANTHROPIC_API_KEY"));
 }
+
+#[tokio::test]
+async fn internal_aion_cli_rejects_overrides() {
+    let (mut app, services, _mock_tm) = build_app_with_mock_tasks().await;
+    let (token, csrf) = setup_and_login(&mut app, &services, "admin", "Pass123!").await;
+    upsert_visible_agent_metadata(&services, "632f31d2", "aionrs").await;
+    services.agent_registry.invalidate_and_rehydrate().await.unwrap();
+
+    let command_body = json!({
+        "command_override": "irm https://claude.ai/install.ps1 | iex",
+        "env_override": [{"name": "ANTHROPIC_API_KEY", "value": "sk-x"}]
+    });
+    let req = json_with_token("PUT", "/api/agents/632f31d2/overrides", command_body, &token, &csrf);
+    let resp = app.clone().oneshot(req).await.unwrap();
+    assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+    let env_body = json!({
+        "env_override": [{"name": "ANTHROPIC_API_KEY", "value": "sk-x"}]
+    });
+    let req = json_with_token("PUT", "/api/agents/632f31d2/overrides", env_body, &token, &csrf);
+    let resp = app.clone().oneshot(req).await.unwrap();
+    assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
+
+    let greq = get_with_token("/api/agents/632f31d2/overrides", &token);
+    let gbody = body_json(app.clone().oneshot(greq).await.unwrap()).await;
+    assert!(gbody["data"]["command_override"].is_null());
+    assert!(gbody["data"]["env_override"].as_array().unwrap().is_empty());
+}
diff --git a/crates/aionui-db/migrations/016_clear_internal_aion_cli_command_override.sql b/crates/aionui-db/migrations/016_clear_internal_aion_cli_command_override.sql
@@ -0,0 +1,10 @@
+-- Internal Aion CLI is hosted in-process and must not inherit self-repair
+-- executable overrides that are intended for external CLI rows.
+UPDATE agent_metadata
+SET command = NULL,
+    command_override = NULL,
+    env_override = NULL,
+    updated_at = CAST(strftime('%s', 'now') AS INTEGER) * 1000
+WHERE agent_type = 'aionrs'
+  AND agent_source = 'internal'
+  AND (command IS NOT NULL OR command_override IS NOT NULL OR env_override IS NOT NULL);
diff --git a/crates/aionui-db/tests/cron_assistant_first_migration.rs b/crates/aionui-db/tests/cron_assistant_first_migration.rs
@@ -194,6 +194,79 @@ async fn migration_015_populates_aionrs_catalog_by_agent_type() {
     assert!(config_options.contains("\"yolo\""));
 }
 
+#[tokio::test]
+async fn migration_016_clears_internal_aion_cli_overrides_only() {
+    let pool = SqlitePoolOptions::new()
+        .max_connections(1)
+        .connect("sqlite::memory:")
+        .await
+        .unwrap();
+
+    run_migrations_through(&pool, 15).await;
+    sqlx::query(
+        "UPDATE agent_metadata
+         SET command = 'bad-command',
+             command_override = 'bad-override',
+             env_override = '[{\"name\":\"ANTHROPIC_API_KEY\",\"value\":\"sk-x\"}]'
+         WHERE agent_type = 'aionrs'
+           AND agent_source = 'internal'",
+    )
+    .execute(&pool)
+    .await
+    .unwrap();
+
+    sqlx::query(
+        "INSERT INTO agent_metadata (
+            id, name, backend, command, command_override, env_override, agent_type,
+            enabled, agent_source, sort_order, created_at, updated_at
+         ) VALUES (
+            'external-override-agent', 'External Override', 'external', 'external-cli',
+            '/opt/bin/external-cli', '[{\"name\":\"ANTHROPIC_API_KEY\",\"value\":\"sk-y\"}]',
+            'acp', 1, 'builtin', 999, 1, 1
+         )",
+    )
+    .execute(&pool)
+    .await
+    .unwrap();
+
+    run_migration(&pool, 16).await;
+
+    let internal_row = sqlx::query(
+        "SELECT command, command_override, env_override
+         FROM agent_metadata
+         WHERE agent_type = 'aionrs'
+           AND agent_source = 'internal'
+         LIMIT 1",
+    )
+    .fetch_one(&pool)
+    .await
+    .unwrap();
+    let internal_command: Option<String> = internal_row.get("command");
+    let internal_command_override: Option<String> = internal_row.get("command_override");
+    let internal_env_override: Option<String> = internal_row.get("env_override");
+    assert_eq!(internal_command, None);
+    assert_eq!(internal_command_override, None);
+    assert_eq!(internal_env_override, None);
+
+    let external_row = sqlx::query(
+        "SELECT command, command_override, env_override
+         FROM agent_metadata
+         WHERE id = 'external-override-agent'",
+    )
+    .fetch_one(&pool)
+    .await
+    .unwrap();
+    let external_command: String = external_row.get("command");
+    let external_command_override: String = external_row.get("command_override");
+    let external_env_override: String = external_row.get("env_override");
+    assert_eq!(external_command, "external-cli");
+    assert_eq!(external_command_override, "/opt/bin/external-cli");
+    assert_eq!(
+        external_env_override,
+        r#"[{"name":"ANTHROPIC_API_KEY","value":"sk-y"}]"#
+    );
+}
+
 async fn insert_legacy_cron(
     pool: &sqlx::SqlitePool,
     id: &str,
