diff --git a/superset/config.py b/superset/config.py index 2f83c65e4d6f..5c20497b85b0 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1347,6 +1347,13 @@ 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 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. 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 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..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 ( @@ -54,6 +55,43 @@ 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; 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 SQLGLOT_DIALECTS = { "base": Dialects.DIALECT, @@ -581,6 +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. """ + _check_script_length(script, engine) dialect = SQLGLOT_DIALECTS.get(engine) try: statements = sqlglot.parse(script, dialect=dialect) @@ -950,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( @@ -1476,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) @@ -1667,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")