From 0df1fb166f82c592204bffb00f4d0fbfd32f4bd9 Mon Sep 17 00:00:00 2001
From: "fix-it-felix-sentry[bot]"
 <260785270+fix-it-felix-sentry[bot]@users.noreply.github.com>
Date: Mon, 23 Mar 2026 16:50:07 +0000
Subject: [PATCH] Fix SQL injection vulnerability in cluster_name_override
 parameter

Validates cluster_name_override to only allow alphanumeric characters,
underscores, and hyphens, preventing SQL injection attacks through
malicious cluster names.

The cluster_name_override parameter is passed directly from user input
via the Flask API endpoint /run_copy_table_query and was previously used
without validation in SQL queries, creating a critical SQL injection
vulnerability at:
- Line 62: f"{db_table} ON CLUSTER '{cluster_name}'"
- Line 91: f"FROM clusterAllReplicas('{cluster_name}', system.tables)"

This fix adds strict input validation using a regex pattern to reject
any cluster names containing special characters that could be used for
SQL injection attacks.

Also added comprehensive test coverage to verify that malicious input
is properly rejected.

Fixes: https://linear.app/getsentry/issue/VULN-1340
Fixes: https://linear.app/getsentry/issue/PF-75

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 snuba/admin/clickhouse/copy_tables.py      |  7 +++++
 tests/admin/clickhouse/test_copy_tables.py | 33 ++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/snuba/admin/clickhouse/copy_tables.py b/snuba/admin/clickhouse/copy_tables.py
index 52cdd90bb78..0ba6bccfeac 100644
--- a/snuba/admin/clickhouse/copy_tables.py
+++ b/snuba/admin/clickhouse/copy_tables.py
@@ -1,3 +1,4 @@
+import re
 from dataclasses import dataclass
 from typing import MutableMapping, Optional, Sequence, Tuple, TypedDict
 
@@ -140,6 +141,12 @@ def copy_tables(
     if skip_on_cluster:
         cluster_name = None
     elif cluster_name_override:
+        # Validate cluster_name_override to prevent SQL injection
+        # Only allow alphanumeric characters, underscores, and hyphens
+        if not re.match(r"^[a-zA-Z0-9_-]+$", cluster_name_override):
+            raise ValueError(
+                "Invalid cluster name: only alphanumeric characters, underscores, and hyphens are allowed"
+            )
         cluster_name = cluster_name_override
     elif not cluster.is_single_node():
         cluster_name = storage.get_cluster().get_clickhouse_cluster_name()
diff --git a/tests/admin/clickhouse/test_copy_tables.py b/tests/admin/clickhouse/test_copy_tables.py
index ac696e375c2..f16febdf8e5 100644
--- a/tests/admin/clickhouse/test_copy_tables.py
+++ b/tests/admin/clickhouse/test_copy_tables.py
@@ -243,6 +243,39 @@ def test_copy_tables_cluster_name_override() -> None:
     assert result["cluster_name"] == override
 
 
+@pytest.mark.redis_db
+@pytest.mark.custom_clickhouse_db
+def test_copy_tables_cluster_name_override_sql_injection_prevention() -> None:
+    """
+    Test that cluster_name_override rejects invalid characters to prevent SQL injection.
+    Only alphanumeric characters, underscores, and hyphens should be allowed.
+    """
+    run_migrations()
+    host = os.environ.get("CLICKHOUSE_HOST", "127.0.0.1")
+
+    # Test various SQL injection attempts
+    invalid_cluster_names = [
+        "'; DROP TABLE users; --",
+        "cluster'; DROP TABLE users; --",
+        "cluster' OR '1'='1",
+        'cluster"; DROP TABLE users; --',
+        "cluster name with spaces",
+        "cluster;name",
+        "cluster'name",
+        "cluster(name)",
+        "cluster.name",
+    ]
+
+    for invalid_name in invalid_cluster_names:
+        with pytest.raises(ValueError, match="Invalid cluster name"):
+            copy_tables(
+                source_host=host,
+                storage_name="outcomes_raw",
+                dry_run=True,
+                cluster_name_override=invalid_name,
+            )
+
+
 @pytest.mark.redis_db
 @pytest.mark.custom_clickhouse_db
 def test_verify_tables_on_replicas() -> None: