security: enforce application-layer validation for DDL/DML in read-only mode - #151
shuvajyotikar13 wants to merge 2 commits into
Conversation
…ly mode Refactor _validate_query_for_destructive_ops to enhance security checks for read-only access and clarify documentation.
|
Hey there @shuvajyotikar13 sorry for the delay here and thanks for the contribution. I just checked this with clickhouse-connect 1.8.0 against recent server versions (e.g. 26.3 and 26.8). The older server did have |
2 participants
Description
The Problem:
Authorization Bypass in Default Configuration
The current implementation of _validate_query_for_destructive_ops contains a logic flaw that creates a "fail-open" security state when the server is deployed in its default read-only mode (allow_write_access = False).
Specifically, the function returns early without inspecting the SQL string if write access is disabled, delegating all security responsibility to the clickhouse-connect driver's readonly=1 flag. However, ClickHouse evaluates permissions server-side; if the authenticated database user possesses elevated privileges, the server silently overrides the driver's read-only request. This allows an autonomous agent or a malicious actor (via prompt injection) to execute destructive DDL commands like DROP TABLE despite the server being configured for safety.
The Fix: Application-Layer Regex Firewall
This PR introduces a strict validation gate that executes regardless of the allow_write_access setting.
Comment Stripping: The patch strips SQL comments (-- and /* */) before evaluation to prevent attackers from "hiding" destructive commands from the regex.
Prefix Allow-listing: When in read-only mode, the server now enforces a strict allow-list of safe SQL prefixes: SELECT, WITH, SHOW, DESCRIBE, EXPLAIN, and EXISTS.
Protocol Compliance: Ensures that any violation results in a standard ToolError, which is handled gracefully by the FastMCP framework.
Verification & Testing
I have added a new test suite, tests/test_clickhouse_security.py, which mocks the server configuration to prove the bypass exists in the unpatched version.
Test 1 (Read-Only Logic Bypass): Confirms that a DROP TABLE command is ignored by the current validation logic when allow_write_access is False. (Fails on main, Passes with this PR).
Test 2 (Intent Validation): Confirms that the existing regex logic is still respected when writes are enabled but drops are restricted.
Enterprise Impact
In an enterprise agentic workflow, this vulnerability represents a critical Privilege Escalation vector. By enforcing validation at the application layer, we ensure that even if a database user is over-privileged, the MCP server acts as a definitive guardrail against accidental or malicious data destruction.