From 042d30f5fa19391da1ed9b98dafbbc0196512f26 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Thu, 27 Aug 2026 16:46:25 +0000 Subject: [PATCH] fix(explore): catch TemplateError when validating access for query-backed permalinks A malformed Jinja SQL template on a query-backed datasource makes security_manager.raise_for_access() raise a raw jinja2 TemplateError via process_jinja_sql. This propagated unwrapped through check_chart_access in the explore permalink create/get commands and out of the API handlers, surfacing as an opaque 500 instead of a proper 4xx. Wrap the check_chart_access call in both commands to convert TemplateError into a SupersetTemplateException (422), and handle that exception in the permalink API post()/get() handlers. Co-Authored-By: Claude Opus 4.8 --- superset/commands/explore/permalink/create.py | 7 ++- superset/commands/explore/permalink/get.py | 7 ++- superset/explore/permalink/api.py | 5 ++ .../commands/explore/permalink/__init__.py | 16 ++++++ .../commands/explore/permalink/test_create.py | 45 +++++++++++++++ .../commands/explore/permalink/test_get.py | 57 +++++++++++++++++++ 6 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 tests/unit_tests/commands/explore/permalink/__init__.py create mode 100644 tests/unit_tests/commands/explore/permalink/test_create.py create mode 100644 tests/unit_tests/commands/explore/permalink/test_get.py diff --git a/superset/commands/explore/permalink/create.py b/superset/commands/explore/permalink/create.py index 926b9ba919f..e2e24fb9911 100644 --- a/superset/commands/explore/permalink/create.py +++ b/superset/commands/explore/permalink/create.py @@ -18,11 +18,13 @@ import logging from functools import partial from typing import Any, Optional +from jinja2.exceptions import TemplateError from sqlalchemy.exc import SQLAlchemyError from superset import db from superset.commands.explore.permalink.base import BaseExplorePermalinkCommand from superset.daos.key_value import KeyValueDAO +from superset.exceptions import SupersetTemplateException from superset.explore.permalink.exceptions import ExplorePermalinkCreateFailedError from superset.explore.utils import check_access as check_chart_access from superset.key_value.exceptions import ( @@ -58,7 +60,10 @@ class CreateExplorePermalinkCommand(BaseExplorePermalinkCommand): d_id, d_type = self.datasource.split("__") datasource_id = int(d_id) datasource_type = DatasourceType(d_type) - check_chart_access(datasource_id, self.chart_id, datasource_type) + try: + check_chart_access(datasource_id, self.chart_id, datasource_type) + except TemplateError as ex: + raise SupersetTemplateException(str(ex)) from ex value = { "chartId": self.chart_id, "datasourceId": datasource_id, diff --git a/superset/commands/explore/permalink/get.py b/superset/commands/explore/permalink/get.py index 7dc1db40df2..58d196182d1 100644 --- a/superset/commands/explore/permalink/get.py +++ b/superset/commands/explore/permalink/get.py @@ -17,11 +17,13 @@ import logging from typing import Optional +from jinja2.exceptions import TemplateError from sqlalchemy.exc import SQLAlchemyError from superset.commands.dataset.exceptions import DatasetNotFoundError from superset.commands.explore.permalink.base import BaseExplorePermalinkCommand from superset.daos.key_value import KeyValueDAO +from superset.exceptions import SupersetTemplateException from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError from superset.explore.permalink.types import ExplorePermalinkValue from superset.explore.utils import check_access as check_chart_access @@ -54,7 +56,10 @@ class GetExplorePermalinkCommand(BaseExplorePermalinkCommand): datasource_type = DatasourceType( value.get("datasourceType", DatasourceType.TABLE) ) - check_chart_access(datasource_id, chart_id, datasource_type) + try: + check_chart_access(datasource_id, chart_id, datasource_type) + except TemplateError as ex: + raise SupersetTemplateException(str(ex)) from ex return value return None except ( diff --git a/superset/explore/permalink/api.py b/superset/explore/permalink/api.py index a5542257195..685e9d11dab 100644 --- a/superset/explore/permalink/api.py +++ b/superset/explore/permalink/api.py @@ -31,6 +31,7 @@ from superset.commands.dataset.exceptions import ( from superset.commands.explore.permalink.create import CreateExplorePermalinkCommand from superset.commands.explore.permalink.get import GetExplorePermalinkCommand from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.exceptions import SupersetTemplateException from superset.explore.permalink.exceptions import ExplorePermalinkInvalidStateError from superset.explore.permalink.schemas import ExplorePermalinkStateSchema from superset.extensions import event_logger @@ -107,6 +108,8 @@ class ExplorePermalinkRestApi(BaseSupersetApi): return self.response(403, message=str(ex)) except (ChartNotFoundError, DatasetNotFoundError) as ex: return self.response(404, message=str(ex)) + except SupersetTemplateException as ex: + return self.response(ex.status, message=str(ex)) @expose("/permalink/", methods=("GET",)) @protect() @@ -162,3 +165,5 @@ class ExplorePermalinkRestApi(BaseSupersetApi): return self.response(403, message=str(ex)) except (ChartNotFoundError, DatasetNotFoundError) as ex: return self.response(404, message=str(ex)) + except SupersetTemplateException as ex: + return self.response(ex.status, message=str(ex)) diff --git a/tests/unit_tests/commands/explore/permalink/__init__.py b/tests/unit_tests/commands/explore/permalink/__init__.py new file mode 100644 index 00000000000..13a83393a91 --- /dev/null +++ b/tests/unit_tests/commands/explore/permalink/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/unit_tests/commands/explore/permalink/test_create.py b/tests/unit_tests/commands/explore/permalink/test_create.py new file mode 100644 index 00000000000..9da6bb970ce --- /dev/null +++ b/tests/unit_tests/commands/explore/permalink/test_create.py @@ -0,0 +1,45 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from unittest.mock import patch + +import pytest +from jinja2.exceptions import TemplateError, TemplateSyntaxError + +from superset.commands.explore.permalink.create import CreateExplorePermalinkCommand +from superset.exceptions import SupersetTemplateException + +check_chart_access = "superset.commands.explore.permalink.create.check_chart_access" + + +def test_create_permalink_malformed_jinja_template() -> None: + # ``check_chart_access`` funnels into ``raise_for_access`` which re-parses the + # query's unrendered Jinja via ``process_jinja_sql`` and can raise a raw + # ``TemplateError`` (e.g. an unclosed ``{% if %}``). ``TemplateSyntaxError`` is + # a subclass of ``TemplateError``. It must surface as a + # ``SupersetTemplateException`` (422), not propagate as an opaque 500. + assert issubclass(TemplateSyntaxError, TemplateError) + + command = CreateExplorePermalinkCommand( + {"formData": {"datasource": "1__table", "slice_id": 1}} + ) + + with patch( + check_chart_access, + side_effect=TemplateSyntaxError("unexpected end of template", lineno=1), + ): + with pytest.raises(SupersetTemplateException): + command.run() diff --git a/tests/unit_tests/commands/explore/permalink/test_get.py b/tests/unit_tests/commands/explore/permalink/test_get.py new file mode 100644 index 00000000000..a411de51e68 --- /dev/null +++ b/tests/unit_tests/commands/explore/permalink/test_get.py @@ -0,0 +1,57 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from unittest.mock import patch + +import pytest +from jinja2.exceptions import TemplateError, TemplateSyntaxError + +from superset.commands.explore.permalink.get import GetExplorePermalinkCommand +from superset.exceptions import SupersetTemplateException +from superset.utils.core import DatasourceType + +check_chart_access = "superset.commands.explore.permalink.get.check_chart_access" +decode_permalink_id = "superset.commands.explore.permalink.get.decode_permalink_id" +get_value = "superset.daos.key_value.KeyValueDAO.get_value" + + +def test_get_permalink_malformed_jinja_template() -> None: + # ``check_chart_access`` funnels into ``raise_for_access`` which re-parses the + # query's unrendered Jinja via ``process_jinja_sql`` and can raise a raw + # ``TemplateError`` (e.g. an unclosed ``{% if %}``). ``TemplateSyntaxError`` is + # a subclass of ``TemplateError``. It must surface as a + # ``SupersetTemplateException`` (422), not propagate as an opaque 500. + assert issubclass(TemplateSyntaxError, TemplateError) + + command = GetExplorePermalinkCommand("thisisallmocked") + + with ( + patch(decode_permalink_id, return_value="123456"), + patch( + get_value, + return_value={ + "chartId": 1, + "datasourceId": 1, + "datasourceType": DatasourceType.TABLE.value, + }, + ), + patch( + check_chart_access, + side_effect=TemplateSyntaxError("unexpected end of template", lineno=1), + ), + ): + with pytest.raises(SupersetTemplateException): + command.run()