From 9ff20e1166f182548eec69b5b3e3901eb92f408a Mon Sep 17 00:00:00 2001
From: Boris Tyshkevich <68195949+bvt123@users.noreply.github.com>
Date: Wed, 4 Mar 2026 09:38:33 +0100
Subject: [PATCH] Enforce read-only mode by blocking write SQL

Fixes #56
---
 README.md                     |  2 +-
 pkg/clickhouse/client.go      |  3 ++
 pkg/clickhouse/client_test.go | 63 ++++++++++++++++++++---------------
 pkg/server/server.go          |  4 +--
 4 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/README.md b/README.md
index 22d4f43..458aafd 100644
--- a/README.md
+++ b/README.md
@@ -424,7 +424,7 @@ go test -v ./cmd/altinity-mcp/...
 - `--clickhouse-username`: Username
 - `--clickhouse-password`: Password
 - `--clickhouse-protocol`: Protocol (http/tcp)
-- `--read-only`: Read-only mode
+- `--read-only`: Read-only mode (prevents non-read SQL and avoids setting session variables)
 - `--clickhouse-max-execution-time`: Query timeout in seconds
 - `--clickhouse-http-headers`: HTTP headers for ClickHouse requests (key=value pairs)
 
diff --git a/pkg/clickhouse/client.go b/pkg/clickhouse/client.go
index a61ec19..57a871e 100644
--- a/pkg/clickhouse/client.go
+++ b/pkg/clickhouse/client.go
@@ -317,6 +317,9 @@ func scanRow(rows driver.Rows) ([]interface{}, error) {
 // ExecuteQuery executes a SQL query and returns results
 // For non-SELECT queries (DDL, DML) will return single row with `OK`
 func (c *Client) ExecuteQuery(ctx context.Context, query string, args ...interface{}) (*QueryResult, error) {
+	if c.config.ReadOnly && !isSelectQuery(query) {
+		return nil, fmt.Errorf("query rejected: read-only mode allows only SELECT/WITH/SHOW/DESC/EXISTS/EXPLAIN statements")
+	}
 	if isSelectQuery(query) {
 		return c.executeSelect(ctx, query, args...)
 	}
diff --git a/pkg/clickhouse/client_test.go b/pkg/clickhouse/client_test.go
index 8276e51..9271f97 100644
--- a/pkg/clickhouse/client_test.go
+++ b/pkg/clickhouse/client_test.go
@@ -149,34 +149,45 @@ func TestClientOperations(t *testing.T) {
 }
 
 func TestClientErrorPaths(t *testing.T) {
-    t.Run("ping_failure", func(t *testing.T) {
-        cfg := &config.ClickHouseConfig{Host: "127.0.0.1", Port: 65000, Database: "default", Username: "default", Protocol: config.TCPProtocol}
-        ctx := context.Background()
-        client, err := NewClient(ctx, *cfg)
-        require.Error(t, err)
-        require.Nil(t, client)
-    })
+	t.Run("ping_failure", func(t *testing.T) {
+		cfg := &config.ClickHouseConfig{Host: "127.0.0.1", Port: 65000, Database: "default", Username: "default", Protocol: config.TCPProtocol}
+		ctx := context.Background()
+		client, err := NewClient(ctx, *cfg)
+		require.Error(t, err)
+		require.Nil(t, client)
+	})
+
+	t.Run("describe_table_not_exists", func(t *testing.T) {
+		cfg := setupClickHouseContainer(t)
+		ctx := context.Background()
+		client, err := NewClient(ctx, *cfg)
+		require.NoError(t, err)
+		defer func() { _ = client.Close() }()
+		_, err = client.DescribeTable(ctx, cfg.Database, "not_exists")
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "columns not found")
+	})
 
-    t.Run("describe_table_not_exists", func(t *testing.T) {
-        cfg := setupClickHouseContainer(t)
-        ctx := context.Background()
-        client, err := NewClient(ctx, *cfg)
-        require.NoError(t, err)
-        defer func() { _ = client.Close() }()
-        _, err = client.DescribeTable(ctx, cfg.Database, "not_exists")
-        require.Error(t, err)
-        require.Contains(t, err.Error(), "columns not found")
-    })
+	t.Run("non_select_error", func(t *testing.T) {
+		cfg := setupClickHouseContainer(t)
+		ctx := context.Background()
+		client, err := NewClient(ctx, *cfg)
+		require.NoError(t, err)
+		defer func() { _ = client.Close() }()
+		_, err = client.ExecuteQuery(ctx, "CREATE TABLE broken ENGINE = Memory")
+		require.Error(t, err)
+	})
 
-    t.Run("non_select_error", func(t *testing.T) {
-        cfg := setupClickHouseContainer(t)
-        ctx := context.Background()
-        client, err := NewClient(ctx, *cfg)
-        require.NoError(t, err)
-        defer func() { _ = client.Close() }()
-        _, err = client.ExecuteQuery(ctx, "CREATE TABLE broken ENGINE = Memory")
-        require.Error(t, err)
-    })
+	t.Run("read_only_blocks_non_select", func(t *testing.T) {
+		client := &Client{
+			config: config.ClickHouseConfig{
+				ReadOnly: true,
+			},
+		}
+		_, err := client.ExecuteQuery(context.Background(), "INSERT INTO t VALUES (1)")
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "read-only mode allows only")
+	})
 }
 
 // TestUtilityFunctions tests utility functions
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 32f5a2c..bb7d066 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -290,7 +290,7 @@ func RegisterTools(srv AltinityMCPServer) {
 			"properties": map[string]any{
 				"query": map[string]any{
 					"type":        "string",
-					"description": "SQL query to execute (SELECT, INSERT, CREATE, etc.)",
+					"description": "SQL query to execute. In read-only mode, only SELECT/WITH/SHOW/DESC/EXISTS/EXPLAIN are allowed.",
 				},
 				"limit": map[string]any{
 					"type":        "number",
@@ -1014,7 +1014,7 @@ func (s *ClickHouseJWEServer) ServeOpenAPISchema(w http.ResponseWriter, r *http.
 							"name":        "query",
 							"in":          "query",
 							"required":    true,
-							"description": "SQL to execute (SELECT, INSERT, etc.).",
+							"description": "SQL to execute. In read-only mode, only SELECT/WITH/SHOW/DESC/EXISTS/EXPLAIN are allowed.",
 							"schema":      map[string]interface{}{"type": "string"},
 						},
 						{