From 5a6f1d57277e858abf34734550224abde794bcae Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 28 May 2026 13:33:43 +0100 Subject: [PATCH 1/2] fix(sql): cap SQL parser input length via SQL_MAX_PARSE_LENGTH config Adds a configurable upper bound on the size of SQL scripts accepted by the SQL parser. Scripts longer than SQL_MAX_PARSE_LENGTH (default 1,000,000 characters) are rejected before being passed to sqlglot. The check sits in SQLStatement._parse, so it applies to every code path that goes through SQLScript, including SQL Lab execute, format, RLS rewriting, dataset SQL, and database engine spec helpers. Set SQL_MAX_PARSE_LENGTH to None to disable. --- superset/config.py | 5 +++++ superset/sql/parse.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/superset/config.py b/superset/config.py index 2f83c65e4d6f..8310d0e63279 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1347,6 +1347,11 @@ class D3TimeFormat(TypedDict, total=False): # Max payload size (MB) for SQL Lab to prevent browser hangs with large results. SQLLAB_PAYLOAD_MAX_MB = None +# Maximum length, in characters, of a SQL script accepted by the SQL parser. +# Scripts longer than this are rejected before being handed to sqlglot, which +# bounds parser memory and CPU usage. Set to None to disable the check. +SQL_MAX_PARSE_LENGTH: int | None = 1_000_000 + # Force refresh while auto-refresh in dashboard DASHBOARD_AUTO_REFRESH_MODE: Literal["fetch", "force"] = "force" # Dashboard auto refresh intervals diff --git a/superset/sql/parse.py b/superset/sql/parse.py index c4983b6d3ef9..328115a51132 100644 --- a/superset/sql/parse.py +++ b/superset/sql/parse.py @@ -54,6 +54,12 @@ logger = logging.getLogger(__name__) +# Fallback parse-length bound applied when no Flask app context is active +# (Alembic migrations, scripts, isolated unit tests). The runtime value is +# read from `SQL_MAX_PARSE_LENGTH` in app config. +_DEFAULT_MAX_PARSE_LENGTH: int | None = 1_000_000 + + # mapping between DB engine specs and sqlglot dialects SQLGLOT_DIALECTS = { "base": Dialects.DIALECT, @@ -571,6 +577,31 @@ def __init__( self._dialect = SQLGLOT_DIALECTS.get(engine) super().__init__(statement, engine, ast) + @classmethod + def _check_script_length(cls, script: str, engine: str) -> None: + """ + Reject scripts longer than the configured maximum length before they + reach the parser. + """ + try: + from flask import current_app + + max_length = current_app.config.get( + "SQL_MAX_PARSE_LENGTH", _DEFAULT_MAX_PARSE_LENGTH + ) + except RuntimeError: + max_length = _DEFAULT_MAX_PARSE_LENGTH + + if max_length is not None and len(script) > max_length: + raise SupersetParseError( + script, + engine, + message=( + f"SQL script length ({len(script)} characters) exceeds " + f"the configured maximum of {max_length}." + ), + ) + @classmethod def _parse(cls, script: str, engine: str) -> list[exp.Expression]: """ @@ -581,6 +612,7 @@ def _parse(cls, script: str, engine: str) -> list[exp.Expression]: supports backticks natively. This handles cases like "Other" database type where users may have MySQL-compatible syntax with backtick-quoted table names. """ + cls._check_script_length(script, engine) dialect = SQLGLOT_DIALECTS.get(engine) try: statements = sqlglot.parse(script, dialect=dialect) From b66c7a741185ec853fa78ce1972c3cdcf8533bb0 Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 28 May 2026 13:50:58 +0100 Subject: [PATCH 2/2] fix(sql): close bypass surfaces and switch parse-length cap to bytes Follow-up on the parse-length gate. Three blocking gaps: 1. The gate at the top of SQLStatement._parse missed three other call sites in the same module that hand strings directly to sqlglot.parse_one: SQLStatement.parse_predicate, the extract_tables_from_statement helper that builds a pseudo SELECT from an exp.Command literal, and the standalone transpile_to_dialect entry point. Any of these could be reached without going through SQLStatement._parse, so the previous single-site check was bypassable. Pulled the check out of SQLStatement and into a module-level helper, then called it from all four sqlglot.parse/parse_one sites so the bound cannot be bypassed by a direct caller. 2. The cap was in Unicode code points, not bytes. A 1M-codepoint string of four-byte characters is up to 4MB of payload that the parser still has to ingest. Switched to UTF-8 byte length so the bound directly reflects parser memory and CPU exposure. 3. The "current_app.config.get + except RuntimeError" pattern is not the codebase idiom for "config-with-fallback-outside-app". Replaced with `has_app_context()`, which matches the pattern already used in sql_lab.py, models/core.py, and others. Tests added in tests/unit_tests/sql/parse_tests.py: - accept exactly at the cap (boundary) - reject one byte over the cap - reject when codepoint count is under the cap but byte count is over - SQL_MAX_PARSE_LENGTH=None disables the gate - app-config value overrides the module fallback - SQLScript short-circuits sqlglot.parse on over-cap input (spy asserts zero calls, covers the MySQL-backtick double-parse path) - SQLStatement.parse_predicate is gated - transpile_to_dialect is gated Co-Authored-By: Claude Opus 4.7 --- superset/config.py | 6 +- superset/sql/parse.py | 69 +++++++++++-------- tests/unit_tests/sql/parse_tests.py | 101 ++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 31 deletions(-) diff --git a/superset/config.py b/superset/config.py index 8310d0e63279..5c20497b85b0 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1347,9 +1347,11 @@ class D3TimeFormat(TypedDict, total=False): # Max payload size (MB) for SQL Lab to prevent browser hangs with large results. SQLLAB_PAYLOAD_MAX_MB = None -# Maximum length, in characters, of a SQL script accepted by the SQL parser. +# Maximum UTF-8 byte length of a SQL script accepted by the SQL parser. # Scripts longer than this are rejected before being handed to sqlglot, which -# bounds parser memory and CPU usage. Set to None to disable the check. +# bounds parser memory and CPU usage. The bound is in bytes (not Unicode +# code points) so multi-byte payloads cannot exceed the intended memory cap. +# Set to None to disable the check. SQL_MAX_PARSE_LENGTH: int | None = 1_000_000 # Force refresh while auto-refresh in dashboard diff --git a/superset/sql/parse.py b/superset/sql/parse.py index 328115a51132..a34d943dcbc8 100644 --- a/superset/sql/parse.py +++ b/superset/sql/parse.py @@ -27,6 +27,7 @@ from typing import Any, Generic, Optional, TYPE_CHECKING, TypeVar import sqlglot +from flask import current_app, has_app_context from jinja2 import nodes, Template from sqlglot import exp from sqlglot.dialects.dialect import ( @@ -56,8 +57,39 @@ # Fallback parse-length bound applied when no Flask app context is active # (Alembic migrations, scripts, isolated unit tests). The runtime value is -# read from `SQL_MAX_PARSE_LENGTH` in app config. -_DEFAULT_MAX_PARSE_LENGTH: int | None = 1_000_000 +# read from `SQL_MAX_PARSE_LENGTH` in app config; keep these two in sync. +_DEFAULT_MAX_PARSE_LENGTH: int = 1_000_000 + + +def _check_script_length(script: str, engine: str | None) -> None: + """ + Reject scripts whose UTF-8 byte length exceeds the configured maximum + before they reach sqlglot. Sits at every code path in this module that + hands a string to ``sqlglot.parse`` or ``sqlglot.parse_one`` so the + bound cannot be bypassed by a direct caller. + + The check is in bytes, not Unicode code points, because the + threat model is parser memory and CPU on the encoded payload that + sqlglot ingests. + """ + if has_app_context(): + max_length = current_app.config.get( + "SQL_MAX_PARSE_LENGTH", _DEFAULT_MAX_PARSE_LENGTH + ) + else: + max_length = _DEFAULT_MAX_PARSE_LENGTH + + if max_length is None: + return + if (byte_length := len(script.encode("utf-8"))) > max_length: + raise SupersetParseError( + script, + engine, + message=( + f"SQL script length ({byte_length} bytes) exceeds the " + f"configured maximum of {max_length} bytes." + ), + ) # mapping between DB engine specs and sqlglot dialects @@ -577,31 +609,6 @@ def __init__( self._dialect = SQLGLOT_DIALECTS.get(engine) super().__init__(statement, engine, ast) - @classmethod - def _check_script_length(cls, script: str, engine: str) -> None: - """ - Reject scripts longer than the configured maximum length before they - reach the parser. - """ - try: - from flask import current_app - - max_length = current_app.config.get( - "SQL_MAX_PARSE_LENGTH", _DEFAULT_MAX_PARSE_LENGTH - ) - except RuntimeError: - max_length = _DEFAULT_MAX_PARSE_LENGTH - - if max_length is not None and len(script) > max_length: - raise SupersetParseError( - script, - engine, - message=( - f"SQL script length ({len(script)} characters) exceeds " - f"the configured maximum of {max_length}." - ), - ) - @classmethod def _parse(cls, script: str, engine: str) -> list[exp.Expression]: """ @@ -612,7 +619,7 @@ def _parse(cls, script: str, engine: str) -> list[exp.Expression]: supports backticks natively. This handles cases like "Other" database type where users may have MySQL-compatible syntax with backtick-quoted table names. """ - cls._check_script_length(script, engine) + _check_script_length(script, engine) dialect = SQLGLOT_DIALECTS.get(engine) try: statements = sqlglot.parse(script, dialect=dialect) @@ -982,6 +989,7 @@ def parse_predicate(self, predicate: str) -> exp.Expression: :param predicate: The predicate to parse. :return: The parsed predicate. """ + _check_script_length(predicate, self.engine) return sqlglot.parse_one(predicate, dialect=self._dialect) def apply_rls( @@ -1508,8 +1516,10 @@ def extract_tables_from_statement( if not literal: return set() + pseudo_sql = f"SELECT {literal.this}" + _check_script_length(pseudo_sql, None) try: - pseudo_query = sqlglot.parse_one(f"SELECT {literal.this}", dialect=dialect) + pseudo_query = sqlglot.parse_one(pseudo_sql, dialect=dialect) except ParseError: return set() sources = pseudo_query.find_all(exp.Table) @@ -1699,6 +1709,7 @@ def transpile_to_dialect( # Get source dialect (default to generic if not specified) source_dialect = SQLGLOT_DIALECTS.get(source_engine) if source_engine else Dialect + _check_script_length(sql, source_engine) try: parsed = sqlglot.parse_one(sql, dialect=source_dialect) return Dialect.get_or_raise(target_dialect).generate( diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py index b6d0c5580530..d365848358b5 100644 --- a/tests/unit_tests/sql/parse_tests.py +++ b/tests/unit_tests/sql/parse_tests.py @@ -18,12 +18,14 @@ import pytest +import sqlglot from pytest_mock import MockerFixture from sqlglot import Dialects, exp, parse_one from superset.exceptions import QueryClauseValidationException, SupersetParseError from superset.jinja_context import JinjaTemplateProcessor from superset.sql.parse import ( + _check_script_length, CTASMethod, extract_tables_from_statement, JinjaSQLResult, @@ -40,6 +42,7 @@ SQLStatement, Table, tokenize_kql, + transpile_to_dialect, ) from tests.integration_tests.conftest import with_feature_flags @@ -3311,3 +3314,101 @@ def test_backtick_invalid_sql_still_fails() -> None: sql = "SELECT * FROM `table` WHERE" with pytest.raises(SupersetParseError): SQLScript(sql, "base") + + +# --------------------------------------------------------------------------- +# SQL_MAX_PARSE_LENGTH gate +# --------------------------------------------------------------------------- + + +@pytest.fixture +def _small_parse_cap(mocker: MockerFixture) -> None: + """ + Pin the parse-length cap to 100 bytes and force the no-app-context + fallback path so tests are decoupled from the suite's Flask config. + """ + mocker.patch("superset.sql.parse._DEFAULT_MAX_PARSE_LENGTH", 100) + mocker.patch("superset.sql.parse.has_app_context", return_value=False) + + +@pytest.mark.usefixtures("_small_parse_cap") +def test_check_script_length_accepts_at_boundary() -> None: + """A script exactly at the configured cap is accepted.""" + _check_script_length("a" * 100, "postgresql") + + +@pytest.mark.usefixtures("_small_parse_cap") +def test_check_script_length_rejects_one_over() -> None: + """One byte above the cap is rejected before sqlglot runs.""" + with pytest.raises(SupersetParseError) as excinfo: + _check_script_length("a" * 101, "postgresql") + assert "exceeds the configured maximum" in str(excinfo.value) + + +def test_check_script_length_counts_utf8_bytes(mocker: MockerFixture) -> None: + """ + The cap is in UTF-8 bytes, not code points. A multi-byte char string + whose char-count is under the cap but byte-count is over must reject. + """ + mocker.patch("superset.sql.parse._DEFAULT_MAX_PARSE_LENGTH", 30) + mocker.patch("superset.sql.parse.has_app_context", return_value=False) + # 20 emoji * 4 UTF-8 bytes each = 80 bytes, well over the 30-byte cap + payload = "\U0001f600" * 20 + assert len(payload) == 20 # code points under the cap + with pytest.raises(SupersetParseError): + _check_script_length(payload, "postgresql") + + +def test_check_script_length_disabled_when_config_none( + mocker: MockerFixture, +) -> None: + """Setting SQL_MAX_PARSE_LENGTH=None disables the check entirely.""" + fake_app = mocker.MagicMock() + fake_app.config = {"SQL_MAX_PARSE_LENGTH": None} + mocker.patch("superset.sql.parse.has_app_context", return_value=True) + mocker.patch("superset.sql.parse.current_app", fake_app) + _check_script_length("a" * 10_000_000, "postgresql") + + +def test_check_script_length_uses_app_config_when_present( + mocker: MockerFixture, +) -> None: + """When an app context is active, the runtime config value wins.""" + fake_app = mocker.MagicMock() + fake_app.config = {"SQL_MAX_PARSE_LENGTH": 50} + mocker.patch("superset.sql.parse.has_app_context", return_value=True) + mocker.patch("superset.sql.parse.current_app", fake_app) + with pytest.raises(SupersetParseError): + _check_script_length("a" * 51, "postgresql") + + +@pytest.mark.usefixtures("_small_parse_cap") +def test_sqlscript_gate_short_circuits_before_sqlglot( + mocker: MockerFixture, +) -> None: + """ + SQLScript construction must reject an over-cap script before any call + to sqlglot.parse, including the MySQL-backtick fallback path. Captures + the original behaviour the PR is closing: the previous code parsed + twice on backtick failures, so the cap MUST short-circuit both. + """ + spy = mocker.spy(sqlglot, "parse") + over_cap_with_backtick = "SELECT * FROM `t` -- " + "x" * 200 + with pytest.raises(SupersetParseError): + SQLScript(over_cap_with_backtick, "base") + assert spy.call_count == 0, "length gate failed to short-circuit sqlglot.parse" + + +@pytest.mark.usefixtures("_small_parse_cap") +def test_parse_predicate_length_check() -> None: + """SQLStatement.parse_predicate also goes through the length gate.""" + stmt = SQLStatement("SELECT 1", "postgresql") + with pytest.raises(SupersetParseError): + stmt.parse_predicate("x" * 101) + + +@pytest.mark.usefixtures("_small_parse_cap") +def test_transpile_to_dialect_length_check() -> None: + """The standalone transpile_to_dialect entry point also gates input.""" + with pytest.raises(SupersetParseError): + transpile_to_dialect("x" * 101, target_engine="mysql")