eschutho opened a new pull request, #43423:
URL: https://github.com/apache/superset/pull/43423
### SUMMARY
When tagging a saved SQL Lab query,
`CreateCustomTagCommand._validate_object_access()` calls
`security_manager.raise_for_access(query=target_object)`, wrapped only in
`except SupersetSecurityException:`.
For a user who lacks blanket database-level access and relies on
per-table/dataset permissions, `raise_for_access()` parses the query's
Jinja-templated SQL via `superset/sql/parse.py::process_jinja_sql()` (which
calls `processor.env.parse(sql)`) to determine referenced tables. As its
docstring states, that can raise a raw `jinja2.exceptions.TemplateError` (e.g.
`TemplateSyntaxError` on malformed Jinja such as an unclosed `{% if %}` block).
Because that exception is not a `SupersetSecurityException`, it escaped the
narrow `except` clause and propagated as an unhandled 500 instead of a proper
Superset validation error.
**Fix:** widen the `except` clause in `_validate_object_access` to `except
(SupersetSecurityException, TemplateError):`, keeping the file's existing
append-to-`exceptions`-list convention, and reword the message slightly since a
template-render failure isn't strictly an access denial.
### PROBLEM
Tagging a saved SQL Lab query whose SQL contains malformed Jinja returned an
unhandled 500 (raw `jinja2.TemplateSyntaxError`) rather than a
`TagInvalidError` validation response, for users authorized via
per-table/dataset permissions.
### FIX
`superset/commands/tag/create.py`: catch `TemplateError` alongside
`SupersetSecurityException` in
`CreateCustomTagCommand._validate_object_access`, mapping the template-parse
failure into the collected validation exceptions.
### TESTING INSTRUCTIONS
- Added `test_validate_object_access_query_malformed_jinja` in
`tests/unit_tests/tags/commands/create_test.py`, which exercises the query path
with malformed Jinja SQL (`SELECT * FROM {% if %} broken`) and forces the
per-table authorization path (`can_access_database` → `False`), asserting it
surfaces as `TagInvalidError`.
- Verified fail-before/pass-after: on the pre-fix code the test fails with
the raw `jinja2.exceptions.TemplateSyntaxError` escaping through
`process_jinja_sql`; post-fix it passes.
- Full file green: `pytest tests/unit_tests/tags/commands/create_test.py` →
3 passed.
- `ruff check` and `ruff format --check` pass on the changed files.
### ADDITIONAL INFORMATION
This is the same recurring bug class as PRs #42366, #42401, #42757, #42917,
#43145, and #43226 — a raw jinja2 `TemplateError` escaping a
`raise_for_access()`/SQL-parsing call site instead of being mapped to a
Superset exception. `superset/commands/sql_lab/results.py` already establishes
the local pattern of catching `TemplateError` alongside
`SupersetSecurityException`.
Note: the sibling site `superset/commands/tag/delete.py` has the identical
duplicated `_validate_object_access` method with the same bug. It is
intentionally left untouched here and will be addressed in a follow-up PR.
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
