(superset) branch fix-jinja-undefined-error-estimate created (now 98156b4f6e9)

[el...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a change to branch fix-jinja-undefined-error-estimate
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 98156b4f6e9 fix(sqllab): wrap process_template() in QueryEstimationCommand to prevent raw UndefinedError leak

This branch includes the following new commits:

     new 98156b4f6e9 fix(sqllab): wrap process_template() in QueryEstimationCommand to prevent raw UndefinedError leak

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(superset) 01/01: fix(sqllab): wrap process_template() in QueryEstimationCommand to prevent raw UndefinedError leak

[el...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a commit to branch fix-jinja-undefined-error-estimate
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 98156b4f6e9536ed907ab89870e47d2720bfcbab
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Tue Aug 4 16:40:29 2026 +0000

    fix(sqllab): wrap process_template() in QueryEstimationCommand to prevent raw UndefinedError leak
    
    BaseTemplateProcessor.process_template() has a bare `raise` fallback for
    jinja2 UndefinedError on undefined attribute/subscript access (e.g.
    {{ foo.bar }}) that doesn't match its "is it a function call" heuristic.
    QueryEstimationCommand.run() called process_template() with no try/except
    around it, so this raw jinja2.exceptions.UndefinedError propagated past the
    API layer and hit Flask's catch-all handler, producing an opaque 500 instead
    of a typed 4xx error for a bad Jinja template typed into SQL Lab's cost
    estimator.
    
    Wrap the call in a try/except TemplateError and convert it into a
    SupersetErrorException(status=400), matching the broad-catch pattern
    ExecuteSqlCommand already uses for the same class of leak in the sibling
    execute.py command.
---
 superset/commands/sql_lab/estimate.py              | 13 +++++++-
 tests/unit_tests/commands/sql_lab/test_estimate.py | 38 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/superset/commands/sql_lab/estimate.py b/superset/commands/sql_lab/estimate.py
index 7d7ef228c2e..fd68e201c63 100644
--- a/superset/commands/sql_lab/estimate.py
+++ b/superset/commands/sql_lab/estimate.py
@@ -21,6 +21,7 @@ from typing import Any, TypedDict
 
 from flask import current_app as app
 from flask_babel import gettext as __
+from jinja2.exceptions import TemplateError
 
 from superset import db, is_feature_enabled, security_manager
 from superset.commands.base import BaseCommand
@@ -150,7 +151,17 @@ class QueryEstimationCommand(BaseCommand):
         sql = self._sql
         if self._template_params:
             template_processor = get_template_processor(self._database)
-            sql = template_processor.process_template(sql, **self._template_params)
+            try:
+                sql = template_processor.process_template(sql, **self._template_params)
+            except TemplateError as ex:
+                raise SupersetErrorException(
+                    SupersetError(
+                        message=str(ex),
+                        error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+                        level=ErrorLevel.ERROR,
+                    ),
+                    status=400,
+                ) from ex
 
         # Apply the same SQL security controls used by the execution path
         # (sql_lab.execute_sql_statements) so cost estimation cannot be used to
diff --git a/tests/unit_tests/commands/sql_lab/test_estimate.py b/tests/unit_tests/commands/sql_lab/test_estimate.py
index 61afd2bd97b..c59f2f7bc68 100644
--- a/tests/unit_tests/commands/sql_lab/test_estimate.py
+++ b/tests/unit_tests/commands/sql_lab/test_estimate.py
@@ -377,3 +377,41 @@ def test_apply_sql_security_propagates_engine_schema_gate(
 
     # RLS injection must not happen once the schema gate has rejected the query.
     mock_apply_rls.assert_not_called()
+
+
+# ---------------------------------------------------------------------------
+# process_template() error handling: raw jinja2 errors must not leak from run()
+# ---------------------------------------------------------------------------
+
+
+@patch("superset.commands.sql_lab.estimate.get_template_processor")
+@patch("superset.commands.sql_lab.estimate.security_manager", new_callable=MagicMock)
+@patch("superset.commands.sql_lab.estimate.db")
+def test_run_wraps_raw_jinja_undefined_error(
+    mock_db: MagicMock,
+    mock_security_manager: MagicMock,
+    mock_get_template_processor: MagicMock,
+) -> None:
+    """A raw jinja2 ``UndefinedError`` from ``process_template()`` (the
+    bare-raise fallback in ``BaseTemplateProcessor.process_template`` for
+    undefined attribute/subscript access, e.g. ``{{ foo.bar }}``) must not
+    leak past ``run()`` -- it should surface as a typed
+    ``SupersetErrorException``, matching the ``ExecuteSqlCommand`` sibling's
+    broad-catch pattern in ``commands/sql_lab/execute.py``."""
+    from jinja2.exceptions import UndefinedError
+
+    mock_database = MagicMock()
+    mock_db.session.query.return_value.get.return_value = mock_database
+    mock_security_manager.raise_for_access.return_value = None
+    mock_get_template_processor.return_value.process_template.side_effect = (
+        UndefinedError("'foo' is undefined")
+    )
+
+    command = QueryEstimationCommand(
+        _make_params(sql="SELECT {{ foo.bar }}", template_params={"x": 1})
+    )
+    with pytest.raises(SupersetErrorException) as exc_info:
+        command.run()
+
+    assert exc_info.value.status == 400
+    assert exc_info.value.error.error_type == SupersetErrorType.GENERIC_COMMAND_ERROR