codeant-ai-for-open-source[bot] commented on code in PR #29554:
URL: https://github.com/apache/superset/pull/29554#discussion_r3450044549
##########
superset/utils/decorators.py:
##########
@@ -227,7 +227,7 @@ def on_error(
logger.exception(ex.exception)
if reraise:
- raise reraise() from ex
+ raise reraise(ex) from ex
Review Comment:
**Suggestion:** Passing the caught exception into `reraise(...)` changes the
constructor contract for every transaction-wrapped command error and overrides
their curated user-facing messages with raw backend exception text. This can
leak internal SQL/database details in API responses and break existing error
semantics; instantiate the target exception without using the caught exception
as the positional message (or pass it via a dedicated `exception` kwarg only
where supported). [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard create API leaks raw database error details.
- ⚠️ Other transaction-wrapped commands expose backend exception messages.
- ⚠️ Breaks curated CommandException messages in API responses.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger dashboard creation via HTTP `POST /api/v1/dashboard/`, which is
implemented by
`DashboardRestApi.post` in `superset/dashboards/api.py` around line 721 (see
`def
post(self) -> Response` in the snippet starting at file line 711).
2. In `DashboardRestApi.post`, after schema validation, the code calls
`CreateDashboardCommand(item).run()` at
`superset/dashboards/api.py:749-751`, delegating
the actual creation logic to the command layer.
3. `CreateDashboardCommand.run` in `superset/commands/dashboard/create.py`
(snippet
starting at file line 35, with the `@transaction(on_error=partial(on_error,
reraise=DashboardCreateFailedError))` decorator at approximately line 43)
executes
`DashboardDAO.create(...)`; if this raises a `SQLAlchemyError` (e.g.,
constraint violation
or SQL parse error), the `transaction` wrapper in
`superset/utils/decorators.py` (lines
~235-88) catches the exception and calls the configured `on_error(ex)`
callback.
4. The default `on_error` callback in `superset/utils/decorators.py`
(snippet starting at
file line 180, where line 229 is `if reraise:` and line 230 is `raise
reraise(ex) from
ex`) sees that `ex` is a `SQLAlchemyError` and `reraise` is
`DashboardCreateFailedError`,
so it executes `raise DashboardCreateFailedError(ex) from ex`; because
`DashboardCreateFailedError` ultimately inherits `SupersetException(message:
str = "",
exception: Optional[Exception] = None, ...)`, this positional `ex` is
treated as the
*message* rather than the wrapped exception, overwriting the curated
class-level message
and causing `DashboardRestApi.post`'s `except DashboardCreateFailedError as
ex: ... return
self.response_422(message=str(ex))` (around
`superset/dashboards/api.py:764-770`) to
return the raw database error text (potentially including SQL and engine
details) to the
client instead of the intended generic error string.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1be341513db74294b71889ed9664f5f3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1be341513db74294b71889ed9664f5f3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/decorators.py
**Line:** 229:230
**Comment:**
*Security: Passing the caught exception into `reraise(...)` changes the
constructor contract for every transaction-wrapped command error and overrides
their curated user-facing messages with raw backend exception text. This can
leak internal SQL/database details in API responses and break existing error
semantics; instantiate the target exception without using the caught exception
as the positional message (or pass it via a dedicated `exception` kwarg only
where supported).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F29554&comment_hash=647bed19df08c6c5dd063b98dfbc2ed19f3a2fb7e25fdcb14921ba04b46bdc3a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F29554&comment_hash=647bed19df08c6c5dd063b98dfbc2ed19f3a2fb7e25fdcb14921ba04b46bdc3a&reaction=dislike'>👎</a>
--
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]
