fix(mcp): close approval/read-only bypass in run_query

[ymadd]

Summary

The MCP run_query approval/read-only gates classify a query by its leading keyword, so a stacked payload such as SELECT 1; DROP TABLE users was tagged as a clean read (select) and slipped past both the read-only gate and the approval gate. Separately, when an approver edited an approved query before confirming, the edit was executed without being re-classified — so an approved SELECT could be flipped into a DELETE/DROP.

This PR makes the classifier fail closed on multi-statement payloads and re-enforces the gates on the approver-edited query.

The bypass

1. Multi-statement payload — classify_query_kind("SELECT 1; DROP TABLE users") returned "select" because only the first keyword was inspected. Both gates rely on kind != "select" to block, so the write reached the driver.
2. Approver-edited query — on approval, edited_query replaced the original query but the original kind was kept, so read-only was never re-checked against what actually runs.

Changes

• ai_activity.rs — classify_query_kind now returns "unknown" (fail closed) when a ; is followed by more SQL. String literals are stripped under both the SQL-standard ('') and MySQL backslash-escape (\') readings, and a trailing statement under either reading trips the gate — so a payload can't hide a ; separator by exploiting whichever dialect we don't assume (e.g. SELECT '\''; DROP TABLE users).
• mcp/mod.rs — tool_run_query re-classifies the approver-edited query and re-applies the read-only gate before execution, updating the audit record with the effective query/kind.

Test plan

• cargo test --lib ai_activity — 54 passing, incl. new cases:
  • SELECT 1; DROP … / …; DELETE … / …; UPDATE … → unknown
  • MySQL '\''; DROP … cannot hide the separator → unknown
  • legitimate ; inside string literals / comments / '' escape boundary → stays select
  • single trailing ; keeps its kind
• cargo check --lib clean

Notes

Rebased on current main (incl. #256). The execution layer's prepared-statement protocol already rejected most stacked queries, but the classifier is the documented fail-closed gate — this restores that contract.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)

• src-tauri/src/ai_activity.rs — Multi-statement detection with dual-mode string stripping (standard + MySQL backslash escapes). has_trailing_statements correctly identifies real statement separators after stripping literals/comments. strip_impl cleanly separates dialect-aware escaping from the public strip_strings_and_comments API.
• src-tauri/src/ai_activity_tests.rs — Comprehensive test coverage for multi-statement payloads, trailing semicolons, escaped quotes under both SQL-standard and MySQL interpretations, and comment/string embedding.
• src-tauri/src/mcp/mod.rs — Re-classifies and re-enforces read-only mode on edited queries after approval, closing the approver-editing bypass.

---

_{Reviewed by kimi-k2.6 · 426,420 tokens}