fix(tags): catch SupersetParseError and composite a ValidationError on tag-delete access

Address review on #43433:
- Also catch SupersetParseError (unresolvable partition macro) alongside
  TemplateError when authorizing a tagged saved query, mirroring the
  create-path sibling. It is a SupersetErrorException sibling, not a
  TemplateError, so it was still escaping as an unhandled 500 -- the exact
  class this PR set out to fix.
- Append a ValidationError (new TagAccessValidationError, field_name='tags')
  instead of TaggedObjectDeleteFailedError so it composites cleanly into
  TagInvalidError.normalized_messages() (which the single-object DELETE route
  calls) instead of risking an AttributeError.
- Strengthen the test to assert via the public normalized_messages() and add
  a SupersetParseError case.

superset/commands/tag/delete.py

@@ -18,12 +18,14 @@ import logging
 from functools import partial
 from typing import Any
 
+from jinja2.exceptions import TemplateError
 from marshmallow import ValidationError
 
 from superset import security_manager
 from superset.commands.base import BaseCommand
 from superset.commands.exceptions import TagNotFoundValidationError
 from superset.commands.tag.exceptions import (
+    TagAccessValidationError,
     TagDeleteFailedError,
     TagDeleteForbiddenValidationError,
     TaggedObjectDeleteFailedError,
@@ -32,7 +34,7 @@ from superset.commands.tag.exceptions import (
 )
 from superset.commands.tag.utils import to_object_model, to_object_type
 from superset.daos.tag import TagDAO
-from superset.exceptions import SupersetSecurityException
+from superset.exceptions import SupersetParseError, SupersetSecurityException
 from superset.tags.models import ObjectType, TagType
 from superset.utils.decorators import on_error, transaction
 from superset.views.base import DeleteMixin
@@ -110,7 +112,29 @@ class DeleteTaggedObjectCommand(DeleteMixin, BaseCommand):
             elif object_type == ObjectType.chart:
                 security_manager.raise_for_access(chart=target_object)
             elif object_type == ObjectType.query:
-                security_manager.raise_for_access(query=target_object)
+                # Authorizing a query without blanket database access parses
+                # its Jinja-templated SQL. Malformed Jinja (``TemplateError``)
+                # or a partition macro that references a table which cannot be
+                # resolved statically (``SupersetParseError``) is a validation
+                # failure, not an opaque 500. Append a ``ValidationError`` so it
+                # composites cleanly into ``TagInvalidError`` (the delete route
+                # calls ``normalized_messages()`` on it).
+                try:
+                    security_manager.raise_for_access(query=target_object)
+                except (TemplateError, SupersetParseError) as ex:
+                    logger.warning(
+                        "Failed to render Jinja SQL while validating access "
+                        "for %s %s: %s",
+                        object_type,
+                        object_id,
+                        ex,
+                    )
+                    exceptions.append(
+                        TagAccessValidationError(
+                            f"Access validation failed for {object_type} "
+                            f"{object_id}: {ex}"
+                        )
+                    )
             elif object_type == ObjectType.dataset:
                 security_manager.raise_for_access(datasource=target_object)
             else:

superset/commands/tag/exceptions.py

@@ -57,6 +57,18 @@ class TagDeleteForbiddenValidationError(ValidationError):
         super().__init__(message, field_name="tags")
 
 
+class TagAccessValidationError(ValidationError):
+    """The access check for a tagged object could not be completed -- e.g. a
+    saved query whose Jinja-templated SQL is malformed (``TemplateError``) or
+    references a table that cannot be resolved statically (``SupersetParseError``).
+    A ``ValidationError`` so it composites into ``TagInvalidError`` and supports
+    ``CommandInvalidError.normalized_messages()`` rather than crashing it.
+    """
+
+    def __init__(self, message: str) -> None:
+        super().__init__(message, field_name="tags")
+
+
 class TaggedObjectDeleteFailedError(DeleteFailedError):
     message = _("Tagged Object could not be deleted.")
 

tests/unit_tests/tags/commands/delete_test.py

@@ -17,6 +17,7 @@
 from unittest.mock import PropertyMock
 
 import pytest
+from jinja2.exceptions import TemplateSyntaxError
 from pytest_mock import MockerFixture
 from sqlalchemy.orm.session import Session
 
@@ -196,3 +197,106 @@ def test_delete_tags_command_not_found_reports_normalized_messages(
     messages = excinfo.value.normalized_messages()
     assert "tags" in messages
     assert "not found" in messages["tags"][0]
+
+
+@pytest.fixture
+def session_with_data(session: Session):
+    from superset.models.core import Database
+    from superset.models.sql_lab import SavedQuery
+    from superset.tags.models import ObjectType, Tag, TaggedObject
+
+    engine = session.get_bind()
+    Tag.metadata.create_all(engine)  # pylint: disable=no-member
+
+    database = Database(database_name="my_database", sqlalchemy_uri="postgresql://")
+
+    saved_query = SavedQuery(
+        id=1, label="test_query", database=database, sql="select {{ unclosed"
+    )
+
+    tag = Tag(name="test_name", description="test_description")
+
+    session.add(database)
+    session.add(saved_query)
+    session.add(tag)
+    session.commit()
+
+    session.add(
+        TaggedObject(object_id=saved_query.id, object_type=ObjectType.query, tag=tag)
+    )
+    session.commit()
+
+    return session
+
+
+def test_delete_command_query_template_error_becomes_validation_error(
+    session_with_data: Session, mocker: MockerFixture
+):
+    """Regression test: a Jinja ``TemplateError`` raised while authorizing a
+    query must surface as ``TagInvalidError`` (422), not an opaque 500 -- and
+    it must be composited as a ``ValidationError`` so ``normalized_messages()``
+    (called by the single-object DELETE route) aggregates it instead of raising
+    ``AttributeError``.
+
+    ``raise_for_access`` is mocked directly so the test stays hermetic and does
+    not depend on a live database to reach ``process_jinja_sql``.
+    """
+    from superset.commands.tag.delete import DeleteTaggedObjectCommand
+    from superset.commands.tag.exceptions import TagInvalidError
+    from superset.models.sql_lab import SavedQuery
+    from superset.tags.models import ObjectType
+
+    query = session_with_data.query(SavedQuery).first()
+
+    template_error_message = "unexpected end of template"
+    mocker.patch(
+        "superset.security.SupersetSecurityManager.raise_for_access",
+        side_effect=TemplateSyntaxError(template_error_message, lineno=1),
+    )
+
+    with pytest.raises(TagInvalidError) as excinfo:
+        DeleteTaggedObjectCommand(
+            object_type=ObjectType.query,
+            object_id=query.id,
+            tag="test_name",
+        ).validate()
+
+    # Must aggregate via the public accessor (proves it is a ValidationError),
+    # and the real template error text must be preserved for server-side debugging.
+    messages = excinfo.value.normalized_messages()
+    assert "tags" in messages
+    assert template_error_message in " ".join(messages["tags"])
+
+
+def test_delete_command_query_parse_error_becomes_validation_error(
+    session_with_data: Session, mocker: MockerFixture
+):
+    """A ``SupersetParseError`` (unresolvable partition macro) raised from the
+    same ``raise_for_access`` call is a ``SupersetErrorException`` sibling --
+    not a ``TemplateError`` -- so it was previously uncaught and swallowed into
+    a 500. It must be caught alongside ``TemplateError`` and surfaced as a 422.
+    """
+    from superset.commands.tag.delete import DeleteTaggedObjectCommand
+    from superset.commands.tag.exceptions import TagInvalidError
+    from superset.exceptions import SupersetParseError
+    from superset.models.sql_lab import SavedQuery
+    from superset.tags.models import ObjectType
+
+    query = session_with_data.query(SavedQuery).first()
+
+    parse_error_message = "cannot statically determine table for partition macro"
+    mocker.patch(
+        "superset.security.SupersetSecurityManager.raise_for_access",
+        side_effect=SupersetParseError(sql="SELECT 1", message=parse_error_message),
+    )
+
+    with pytest.raises(TagInvalidError) as excinfo:
+        DeleteTaggedObjectCommand(
+            object_type=ObjectType.query,
+            object_id=query.id,
+            tag="test_name",
+        ).validate()
+
+    messages = excinfo.value.normalized_messages()
+    assert "tags" in messages
+    assert parse_error_message in " ".join(messages["tags"])