fix(sql_lab): raise 400 not 500 on malformed Jinja during CSV export access check - #43866
fix(sql_lab): raise 400 not 500 on malformed Jinja during CSV export access check#43866EnxDev wants to merge 3 commits into
Conversation
…access check SqlResultExportCommand.validate() only caught SupersetSecurityException around raise_for_access(), so a raw jinja2 TemplateError from re-parsing unrendered Jinja surfaced as an opaque 500. Sibling call sites already guard this with except TemplateError; this brings export.py in line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Code Review Agent Run #e6be6fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43866 +/- ##
=======================================
Coverage 79.42% 79.42%
=======================================
Files 2895 2895
Lines 167997 168000 +3
Branches 38903 38903
=======================================
+ Hits 133434 133440 +6
+ Misses 32064 32061 -3
Partials 2499 2499
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #1e0fc3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
1 participant
SUMMARY
SqlResultExportCommand.validate(), used by the SQL Lab CSV export endpoint, callsself._query.raise_for_access().This call is currently guarded only against
SupersetSecurityException.raise_for_access()may re-parse the query’s unrendered Jinja throughprocess_jinja_sql(). If the template is invalid for example, because of an unclosed{% if %}this can raise a rawjinja2.exceptions.TemplateError. Because the error isn’t caught, it results in an opaque HTTP 500 response instead of a structured client error.Other call sites in the same subsystem already handle this case.
superset/sqllab/api.py,superset/commands/sql_lab/estimate.py, andsuperset/commands/sql_lab/results.pycatchTemplateErroraround the sameraise_for_access()call.This behavior was introduced in #43145, where
export.pywas listed as a follow-up but was missed.This change adds an
except TemplateErrorclause alongside the existingSupersetSecurityExceptionhandler invalidate().It raises a
SupersetErrorExceptionwithstatus=400anderror_type=GENERIC_COMMAND_ERROR, matching the existing implementation in the other SQL Lab paths.This is an additive error-handling change only. Existing success and failure paths remain unchanged.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Added
TestSqlResultExportCommand.test_validation_malformed_jinjatotests/integration_tests/sql_lab/commands_tests.py. The test follows the existingtest_validation_malformed_jinjainTestSqlExecutionResultsCommand.Run the integration tests:
To test manually:
ENABLE_TEMPLATE_PROCESSING.{% if %}.400response with a clear error message instead of a500.ADDITIONAL INFORMATION