From 80ea36c852fc7c0b2daeee4bddc30c037d7ef2ed Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Wed, 3 Jun 2026 11:56:51 -0700 Subject: [PATCH] fix(db_engine_specs): escape schema name in regex; document safe filter pattern (#40642) Co-authored-by: Claude Code --- superset/db_engine_specs/base.py | 6 +- superset/jinja_context.py | 16 ++++-- tests/unit_tests/db_engine_specs/test_base.py | 56 +++++++++++++++++++ 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 9b8d56c5115..5c33a3fd1f0 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -1746,7 +1746,8 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods raise cls.get_dbapi_mapped_exception(ex) from ex if schema and cls.try_remove_schema_from_table_name: - tables = {re.sub(f"^{schema}\\.", "", table) for table in tables} + escaped_schema = re.escape(schema) + tables = {re.sub(f"^{escaped_schema}\\.", "", table) for table in tables} return tables @classmethod @@ -1774,7 +1775,8 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods raise cls.get_dbapi_mapped_exception(ex) from ex if schema and cls.try_remove_schema_from_table_name: - views = {re.sub(f"^{schema}\\.", "", view) for view in views} + escaped_schema = re.escape(schema) + views = {re.sub(f"^{escaped_schema}\\.", "", view) for view in views} return views @classmethod diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 1c8b3d1f859..a575b4b5cae 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -356,6 +356,18 @@ class ExtraCache: - you want to have the ability for filter inside the main query for speed purposes + Always use the ``where_in`` filter for list membership rather than + building SQL by hand. The filter renders values with dialect-safe quoting + (via SQLAlchemy's ``literal_binds`` compilation) instead of interpolating + them directly into the SQL string. + + .. warning:: + + Do not manually escape filter values (for example, with + ``replace("'", "''")``). Hand-rolled escaping is error-prone and easy + to get wrong across dialects. Rely on the ``where_in`` filter so values + are quoted safely by the engine. + Usage example:: @@ -377,10 +389,6 @@ class ExtraCache: AND full_name IN {{ filter.get('val')|where_in }} {%- endif -%} - {%- if filter.get('op') == 'LIKE' -%} - AND - full_name LIKE '{{ filter.get('val') | replace("'", "''") }}' - {%- endif -%} {%- endfor -%} UNION ALL SELECT diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py index 5eae41458de..b0e416e6081 100644 --- a/tests/unit_tests/db_engine_specs/test_base.py +++ b/tests/unit_tests/db_engine_specs/test_base.py @@ -1283,3 +1283,59 @@ def test_start_oauth2_dance_falls_back_to_url_for(mocker: MockerFixture) -> None error = exc_info.value.error assert error.extra["redirect_uri"] == fallback_uri + + +def test_get_table_names_strips_schema_with_regex_metacharacters( + mocker: MockerFixture, +) -> None: + """ + Test that get_table_names strips a schema prefix containing regex + metacharacters without raising and without mangling unrelated names. + """ + schema = "a.b(c)" + + inspector = mocker.MagicMock() + inspector.get_table_names.return_value = [ + f"{schema}.orders", + "axbc.other", + ] + + database = mocker.MagicMock() + + spec = BaseEngineSpec + mocker.patch.object(spec, "try_remove_schema_from_table_name", True) + + tables = spec.get_table_names(database, inspector, schema) + + # The real schema prefix is stripped; the look-alike name is left intact + # because the metacharacters are escaped before being used as a regex. + # "axbc.other" would match the old unescaped pattern ^a.b(c)\. and be + # incorrectly stripped — the escaped version correctly preserves it. + assert tables == {"orders", "axbc.other"} + + +def test_get_view_names_strips_schema_with_regex_metacharacters( + mocker: MockerFixture, +) -> None: + """ + Test that get_view_names strips a schema prefix containing regex + metacharacters without raising and without mangling unrelated names. + """ + schema = "a.b(c)" + + inspector = mocker.MagicMock() + inspector.get_view_names.return_value = [ + f"{schema}.report", + "axbc.other", + ] + + database = mocker.MagicMock() + + spec = BaseEngineSpec + mocker.patch.object(spec, "try_remove_schema_from_table_name", True) + + views = spec.get_view_names(database, inspector, schema) + + # "axbc.other" would match the old unescaped pattern ^a.b(c)\. and be + # incorrectly stripped — the escaped version correctly preserves it. + assert views == {"report", "axbc.other"}