|
| 1 | +# Config Query Tool Implementation Plan |
| 2 | + |
| 3 | +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| 4 | +
|
| 5 | +**Goal:** Add a server-enforced SQL query tool to `/mcp/config` so MCP clients can inspect and modify ProxySQL configuration without a growing pile of dedicated verbs. |
| 6 | + |
| 7 | +**Architecture:** Keep the `/mcp/config` endpoint as the admin/config surface, but add a single `query` tool that executes SQL against the admin database through `admindb->execute_statement()`. The MCP server will reject unsafe statements before execution using a local policy gate that blocks DDL, attachment, pragma, and other hazardous statements while still allowing controlled reads and writes. |
| 8 | + |
| 9 | +**Tech Stack:** C++17, `SQLite3DB`, `nlohmann::json`, existing MCP tool-handler framework, TAP tests. |
| 10 | + |
| 11 | +--- |
| 12 | + |
| 13 | +### Task 1: Add the config query tool contract |
| 14 | + |
| 15 | +**Files:** |
| 16 | +- Modify: `plugins/genai/include/Config_Tool_Handler.h` |
| 17 | +- Modify: `plugins/genai/src/tool_handlers/Config_Tool_Handler.cpp` |
| 18 | + |
| 19 | +- [ ] **Step 1: Update the tool list and dispatch** |
| 20 | + |
| 21 | +Add a new `query` tool to `/mcp/config` with this input schema: |
| 22 | + |
| 23 | +```cpp |
| 24 | +tools.push_back(create_tool_description( |
| 25 | + "query", |
| 26 | + "Execute constrained SQL against the ProxySQL admin/config database", |
| 27 | + { |
| 28 | + {"type", "object"}, |
| 29 | + {"properties", { |
| 30 | + {"sql", { |
| 31 | + {"type", "string"}, |
| 32 | + {"description", "Single SQL statement to execute"} |
| 33 | + }}, |
| 34 | + {"limit", { |
| 35 | + {"type", "integer"}, |
| 36 | + {"description", "Optional row limit for result sets"} |
| 37 | + }} |
| 38 | + }}, |
| 39 | + {"required", {"sql"}} |
| 40 | + } |
| 41 | +)); |
| 42 | +``` |
| 43 | +
|
| 44 | +Route `execute_tool("query", ...)` to a new private helper that validates the SQL text, runs it through `GloAdmin->admindb`, and returns a structured result. |
| 45 | +
|
| 46 | +- [ ] **Step 2: Add the execution helper** |
| 47 | +
|
| 48 | +Implement a helper in `Config_Tool_Handler.cpp` that: |
| 49 | +
|
| 50 | +```cpp |
| 51 | +json handle_query(const std::string& sql, int limit); |
| 52 | +``` |
| 53 | + |
| 54 | +The helper should: |
| 55 | + |
| 56 | +- reject empty SQL |
| 57 | +- reject multi-statement SQL |
| 58 | +- reject forbidden statement classes |
| 59 | +- execute the statement with `GloAdmin->admindb->execute_statement(...)` |
| 60 | +- convert any resultset with `MCP_Tool_Handler::resultset_to_json(...)` |
| 61 | +- return a JSON object containing at least: |
| 62 | + - `sql` |
| 63 | + - `rows_affected` |
| 64 | + - `columns` |
| 65 | + - `rows` |
| 66 | + - `message` for non-row statements |
| 67 | + |
| 68 | +- [ ] **Step 3: Keep existing config verbs intact** |
| 69 | + |
| 70 | +Leave `get_config`, `set_config`, `list_variables`, and `get_status` in place for convenience. Keep `reload_config` for now, but do not expand it in this task. |
| 71 | + |
| 72 | +- [ ] **Step 4: Commit** |
| 73 | + |
| 74 | +```bash |
| 75 | +git add plugins/genai/include/Config_Tool_Handler.h plugins/genai/src/tool_handlers/Config_Tool_Handler.cpp |
| 76 | +git commit -m "feat(genai): add constrained config query tool" |
| 77 | +``` |
| 78 | + |
| 79 | +### Task 2: Enforce SQL safety on the server side |
| 80 | + |
| 81 | +**Files:** |
| 82 | +- Modify: `plugins/genai/src/tool_handlers/Config_Tool_Handler.cpp` |
| 83 | + |
| 84 | +- [ ] **Step 1: Add a SQL policy helper** |
| 85 | + |
| 86 | +Implement a local helper that validates the SQL string before execution. The first pass should reject: |
| 87 | + |
| 88 | +```cpp |
| 89 | +PRAGMA |
| 90 | +ATTACH |
| 91 | +DETACH |
| 92 | +DROP |
| 93 | +ALTER |
| 94 | +CREATE |
| 95 | +TRUNCATE |
| 96 | +VACUUM |
| 97 | +REINDEX |
| 98 | +LOAD_EXTENSION |
| 99 | +``` |
| 100 | + |
| 101 | +Also reject: |
| 102 | + |
| 103 | +- empty statements |
| 104 | +- semicolon-separated multi-statements |
| 105 | +- leading SQL comments that hide a forbidden first token |
| 106 | + |
| 107 | +The helper should accept controlled DML and queries such as `SELECT`, `WITH`, `INSERT`, `UPDATE`, `DELETE`, and `REPLACE`. |
| 108 | + |
| 109 | +```cpp |
| 110 | +bool is_allowed_config_sql(const std::string& sql, std::string& error); |
| 111 | +``` |
| 112 | +
|
| 113 | +- [ ] **Step 2: Add execution guardrails** |
| 114 | +
|
| 115 | +Before execution, apply the policy helper and return an error response when the SQL is blocked. Use a clear error message that names the blocked token/class so the client can adapt. |
| 116 | +
|
| 117 | +Also apply a hard row cap to result sets if the caller passes a `limit`, and clamp the value to a sane upper bound inside the handler. |
| 118 | +
|
| 119 | +- [ ] **Step 3: Commit** |
| 120 | +
|
| 121 | +```bash |
| 122 | +git add plugins/genai/src/tool_handlers/Config_Tool_Handler.cpp |
| 123 | +git commit -m "feat(genai): enforce config query sql policy" |
| 124 | +``` |
| 125 | + |
| 126 | +### Task 3: Add tests for allowed and blocked SQL |
| 127 | + |
| 128 | +**Files:** |
| 129 | +- Create: `test/tap/tests/unit/genai_config_tool_unit-t.cpp` |
| 130 | +- Modify: `test/tap/tests/unit/Makefile` |
| 131 | + |
| 132 | +- [ ] **Step 1: Write the unit test** |
| 133 | + |
| 134 | +Add a unit test that instantiates `Config_Tool_Handler` with a minimal MCP handler and checks these cases: |
| 135 | + |
| 136 | +```cpp |
| 137 | +ok(handler.execute_tool("query", json{{"sql", "SELECT variable_name FROM global_variables LIMIT 1"}})["success"] == true, |
| 138 | + "SELECT is allowed"); |
| 139 | +ok(handler.execute_tool("query", json{{"sql", "UPDATE global_variables SET variable_value='1' WHERE variable_name='x'"}})["success"] == true, |
| 140 | + "UPDATE is allowed"); |
| 141 | +ok(handler.execute_tool("query", json{{"sql", "PRAGMA journal_mode"}})["success"] == false, |
| 142 | + "PRAGMA is blocked"); |
| 143 | +ok(handler.execute_tool("query", json{{"sql", "DROP TABLE global_variables"}})["success"] == false, |
| 144 | + "DROP is blocked"); |
| 145 | +ok(handler.execute_tool("query", json{{"sql", "SELECT 1; SELECT 2"}})["success"] == false, |
| 146 | + "multi-statement input is blocked"); |
| 147 | +``` |
| 148 | +
|
| 149 | +- [ ] **Step 2: Build and run the test** |
| 150 | +
|
| 151 | +Run the targeted unit test binary from `test/tap/tests/unit/Makefile` and verify the new assertions pass. |
| 152 | +
|
| 153 | +- [ ] **Step 3: Commit** |
| 154 | +
|
| 155 | +```bash |
| 156 | +git add test/tap/tests/unit/genai_config_tool_unit-t.cpp test/tap/tests/unit/Makefile |
| 157 | +git commit -m "test(genai): cover config query policy" |
| 158 | +``` |
| 159 | + |
| 160 | +### Task 4: Verify the endpoint contract end to end |
| 161 | + |
| 162 | +**Files:** |
| 163 | +- Modify: `plugins/genai/src/ProxySQL_MCP_Server.cpp` only if the config tool name or endpoint wiring needs adjustment |
| 164 | +- Test: existing MCP integration tests or a new TAP integration test under `test/tap/tests/` |
| 165 | + |
| 166 | +- [ ] **Step 1: Verify endpoint registration** |
| 167 | + |
| 168 | +Confirm `/mcp/config` still registers through the existing server wiring and that the new `query` tool appears in `tools/list`. |
| 169 | + |
| 170 | +- [ ] **Step 2: Add an integration smoke test** |
| 171 | + |
| 172 | +Add a TAP test that: |
| 173 | + |
| 174 | +```cpp |
| 175 | +handler.execute_tool("get_config", json{{"variable_name", "mcp_enabled"}}); |
| 176 | +handler.execute_tool("query", json{{"sql", "SELECT variable_name FROM global_variables LIMIT 1"}}); |
| 177 | +handler.execute_tool("query", json{{"sql", "PRAGMA journal_mode"}}); |
| 178 | +``` |
| 179 | +
|
| 180 | +and checks that the first two succeed and the last one is rejected. |
| 181 | +
|
| 182 | +- [ ] **Step 3: Commit** |
| 183 | +
|
| 184 | +```bash |
| 185 | +git add plugins/genai/src/ProxySQL_MCP_Server.cpp test/tap/tests/<new-or-existing-integration-test> |
| 186 | +git commit -m "test(genai): verify config query endpoint" |
| 187 | +``` |
| 188 | + |
| 189 | +--- |
| 190 | + |
| 191 | +### Coverage Check |
| 192 | + |
| 193 | +- `/mcp/config` query tool: Task 1 |
| 194 | +- Server-side SQL policy: Task 2 |
| 195 | +- Regression coverage: Task 3 |
| 196 | +- End-to-end endpoint behavior: Task 4 |
| 197 | + |
0 commit comments