msyavuz opened a new pull request, #42796:
URL: https://github.com/apache/superset/pull/42796
### SUMMARY
Errors raised while running a query are relayed to the client verbatim so
chart authors can fix them. For an embedded (guest token) viewer that detail is
both unusable and sensitive: engine errors routinely quote catalog, schema,
table and column names of the underlying warehouse, and a viewer of an embedded
dashboard has no way to act on them.
For a guest-token principal only, the error message is replaced with a
generic `An error occurred while fetching the data.`, `error_type` is
normalized to `GENERIC_BACKEND_ERROR`, and `extra` (engine name, and for some
types the offending SQL) is dropped. The full error is still logged server-side.
An allowlist keeps errors Superset authors itself verbatim, since those
describe an access decision or a client-side condition rather than echoing the
database: security/access denials, OAuth2 redirects, timeouts, payload
validation, frontend errors. Every response to a non-guest principal is
unchanged.
New `superset/utils/error_sanitization.py`, applied at the four points where
an error reaches a client:
- `json_error_response` — the funnel for the app-level error handlers and
`handle_api_exception`
- chart data `400`/`422` responses (`ChartDataQueryFailedError` carried the
raw engine text as its message)
- the per-query `error` / `stacktrace` fields in a `200` chart data payload
— next to the existing guest strip of `query`
- async (`GLOBAL_ASYNC_QUERIES`) job errors; guest detection works in the
worker because the job restores the guest user from its `guest_token`
Design notes: this is an allowlist rather than a denylist, so a newly added
error type is redacted by default rather than leaking until someone remembers
to list it. Bare-string errors are left alone at `401/403/404/429`, which
report an authorization or routing decision rather than a query failure.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no UI change. The response body for a guest changes:
```
# before
400 {"message": "Error: no such table: table_that_does_not_exist"}
# after
400 {"message": "An error occurred while fetching the data."}
```
### TESTING INSTRUCTIONS
`pytest tests/unit_tests/utils/test_error_sanitization.py
tests/unit_tests/charts/test_chart_data_api.py
tests/unit_tests/views/test_error_handling.py
tests/unit_tests/tasks/test_async_queries.py`
`test_get_data_response_redacts_a_real_engine_error_for_guests` drives an
actual failing query (a dataset pointing at a missing table) rather than a
hand-written message, and asserts the object name still reaches an authorized
user but not a guest.
Manually: enable `EMBEDDED_SUPERSET`, embed a dashboard whose chart points
at a dropped/renamed table, and load it with a guest token — `POST
/api/v1/chart/data` returns the generic message. Load the same chart as a
logged-in user and the engine's error is unchanged.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: No
- [x] Required feature flags: `EMBEDDED_SUPERSET` — the redaction only
applies to guest-token principals, which only exist when embedding is enabled
- [ ] Changes UI: No — backend only. An embedded viewer sees a different
error string; no component changes.
- [ ] Includes DB Migration: No
- [ ] Introduces new feature or API: No — behavior change on existing
endpoints, noted in `UPDATING.md`
- [ ] Removes existing feature or API: No
--
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]
