rusackas opened a new pull request, #41162:
URL: https://github.com/apache/superset/pull/41162
### SUMMARY
Resolves code-scanning alerts #2461 (`superset/views/base.py:146`) and #2540
(`superset/views/base.py:218`), both `py/stack-trace-exposure` ("Information
exposure through an exception").
The two flagged sinks are the generic JSON response helpers `json_success`
(line 146) and `BaseSupersetView.json_response` (line 218). CodeQL traces
exception-derived strings reaching them and warns that a stack trace could be
exposed to the client.
**What this PR changes (scoped to `base.py`):** `get_error_msg()` is the
canonical stack-trace source in this file (`traceback.format_exc()`). This PR
makes its safety contract explicit and stronger:
- The **full traceback is now always logged server-side** (`logger.error`),
independent of what is returned to the client, so operators never lose
debugging detail.
- The raw traceback is only ever placed in the client-facing response when
the operator has **explicitly opted in via the `SHOW_STACKTRACE` config
(default `False`)** — the existing, documented behavior — now clearly commented
so both readers and scanners can see the gate.
- Adds regression tests covering the hidden-by-default, opt-in, and
always-logged-server-side behaviors.
Error detail is still fully retained in server logs; only the client-facing
leakage is gated.
### Why DRAFT — design judgment call
I want a maintainer gut-check before this is merged, because the change is
deliberately narrow:
1. **The genuine stack-trace exposure was already mitigated.** Raw
tracebacks (`get_error_msg`, `viz.get_stacktrace`) are already gated behind
`SHOW_STACKTRACE`, which defaults to `False`. PR #40636 recently did the same
for the viz payload's `stacktrace` field. This PR tightens and documents that
contract in `base.py` rather than introducing a new control.
2. **The residual flows CodeQL tracks into sinks 146/218 originate in
callers, not `base.py`** — specifically `Api.time_range` in
`superset/views/api.py` (which interpolates `str(error)` from a `ValueError`
into a `"Unexpected time range: ..."` message) and `viz.py`
(`message=str(ex)`). Those are *error messages*, not stack traces. Per
[`SECURITY.md`](https://github.com/apache/superset/blob/master/SECURITY.md)
(out-of-scope list), "information disclosure through error messages ... that
does not enable a further concrete exploit" is explicitly out of scope. I
intentionally did **not** touch those callers, because (a) sanitizing the
generic `json_success`/`json_response` helpers themselves would corrupt all
legitimate payloads, and (b) blanking those user-facing messages would degrade
UX for no in-scope security gain.
So: if the team considers the `SHOW_STACKTRACE` gating sufficient (I believe
it is, and SECURITY.md backs that up), these two alerts can reasonably be
**dismissed as "won't fix / out of scope"** with this PR's hardening + tests as
the supporting rationale. If instead we want CodeQL fully green, the follow-up
is to also sanitize the `str(ex)` interpolations in `api.py`/`viz.py` — happy
to extend, but that exceeds the "base.py only" scope I held to here.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only error-handling change.
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/views/test_base.py -k get_error_msg
```
- With `SHOW_STACKTRACE=False` (default): response message is generic
("Stacktrace is hidden..."), no traceback or exception text leaks; full
traceback is logged server-side.
- With `SHOW_STACKTRACE=True`: raw traceback is returned (documented opt-in
debugging behavior).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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]
