eschutho opened a new pull request, #43605:
URL: https://github.com/apache/superset/pull/43605
### SUMMARY
A malformed Jinja SQL template on a query-backed datasource can make
`security_manager.raise_for_access(query=...)` raise a raw
`jinja2.exceptions.TemplateError`, which propagates uncaught out of the
explore-permalink API as an opaque HTTP 500 instead of a proper 4xx.
**PROBLEM**
When a query-backed datasource carries malformed Jinja (e.g. an unclosed `{%
if %}`), `security_manager.raise_for_access(query=...)` re-parses the
unrendered SQL via `process_jinja_sql` and raises a raw
`jinja2.exceptions.TemplateError`. That exception propagates uncaught through:
`superset/explore/utils.py::check_query_access` → `check_datasource_access`
→ `check_access` (imported as `check_chart_access`) →
`CreateExplorePermalinkCommand.run()` / `GetExplorePermalinkCommand.run()` →
`ExplorePermalinkRestApi.post()` / `.get()`
None of the command or API `except` chains cover `TemplateError`, so
flask-appbuilder's `@safe` decorator turns it into an opaque **500** rather
than a meaningful **4xx**.
**FIX**
- Catch `jinja2.exceptions.TemplateError` around the
`check_chart_access(...)` call in both
`superset/commands/explore/permalink/create.py` and `get.py`, re-raising it as
the existing `SupersetTemplateException` (HTTP **422**).
- Add an `except SupersetTemplateException as ex: return
self.response(ex.status, message=str(ex))` clause to both the `post()` and
`get()` handlers in `superset/explore/permalink/api.py`. flask-appbuilder's
`@safe` decorator intercepts the exception before Superset's global
`SupersetException` error handler can, so the API layer needs its own explicit
catch — the same reason the sibling `form_data` API fix needs it in #43470.
This is a strict additive-catch: no behavior changes on any non-error path.
This is the same bug class as #42366, #42401, #43423, #43433, and #43470.
The closest sibling is **#43470**, which fixes the identical root call for the
`form_data` API family — same root call, different consumer (form_data vs
permalink). #43470 is **not yet merged**; this PR does **not** depend on or
touch it (`superset/explore/utils.py` is untouched here) — they are independent
files.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API error-handling change (500 → 422).
### TESTING INSTRUCTIONS
Two new regression unit tests were added:
-
`tests/unit_tests/commands/explore/permalink/test_create.py::test_create_permalink_malformed_jinja_template`
-
`tests/unit_tests/commands/explore/permalink/test_get.py::test_get_permalink_malformed_jinja_template`
Each mocks `check_chart_access` with a
`jinja2.exceptions.TemplateSyntaxError` side-effect (a `TemplateError`
subclass) and asserts the command raises `SupersetTemplateException` instead of
leaking the raw `TemplateError`.
```
pytest tests/unit_tests/commands/explore/permalink/
```
Verified they fail on pre-fix code: `git stash` the three source-file
changes (keeping the tests), re-run — both fail with the raw
`TemplateSyntaxError` propagating; restore the fix — both pass.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] 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
--
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]
