codeant-ai-for-open-source[bot] commented on code in PR #42929:
URL: https://github.com/apache/superset/pull/42929#discussion_r3742447985
##########
superset/commands/report/alert.py:
##########
@@ -181,6 +183,18 @@ def _get_alert_metadata_from_object(self) -> dict[str,
Any]:
"execution_id": self._execution_id,
}
+ def _validate_rendered_sql(self, rendered_sql: str) -> None:
+ """
+ Enforce SQL-level constraints on the rendered alert query: a single
+ statement, and no mutations unless the database allows DML.
+ """
+ database = self._report_schedule.database
+ script = SQLScript(rendered_sql, engine=database.backend)
+ if len(script.statements) > 1:
+ raise AlertQueryError(message=_("Alert query must be a single
statement"))
+ if script.has_mutation() and not database.allow_dml:
Review Comment:
**Suggestion:** The validation only rejects more than one statement, so an
empty or comment-only alert query with zero parsed statements passes both
save-time and execution-time validation. `apply_limit_to_sql` then indexes the
last statement and raises an error when the alert runs instead of returning a
field validation error. Require exactly one statement by rejecting zero
statements as well. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Invalid alerts fail only when scheduled.
- ⚠️ Users receive generic execution errors.
- ⚠️ Alert configuration validation permits unusable schedules.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9fbe1ef6b4c14b82aa06f2cd1284b165&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=9fbe1ef6b4c14b82aa06f2cd1284b165&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/commands/report/alert.py
**Line:** 193:195
**Comment:**
*Incorrect Condition Logic: The validation only rejects more than one
statement, so an empty or comment-only alert query with zero parsed statements
passes both save-time and execution-time validation. `apply_limit_to_sql` then
indexes the last statement and raises an error when the alert runs instead of
returning a field validation error. Require exactly one statement by rejecting
zero statements as well.
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%2F42929&comment_hash=206211c032d0a43e444f7e1163e7d7131c28bfcdfe194304234ae63eea314d83&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42929&comment_hash=206211c032d0a43e444f7e1163e7d7131c28bfcdfe194304234ae63eea314d83&reaction=dislike'>👎</a>
##########
superset/commands/report/alert.py:
##########
@@ -196,6 +210,7 @@ def _execute_query(self) -> pd.DataFrame:
try:
rendered_sql =
sql_template.process_template(self._report_schedule.sql)
Review Comment:
**Suggestion:** The authorization and read-only checks run against
`rendered_sql`, but execution uses `limited_rendered_sql` after the
configurable SQL mutator has transformed it. A mutator can add statements,
change referenced tables, or introduce mutations after these checks, so the
database may execute SQL that was never authorized. Validate the final SQL
immediately before execution, or ensure the authorization and mutation checks
cover the exact SQL sent to the database. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mutated alert SQL can bypass matching authorization checks.
- ⚠️ Configured SQL mutators can introduce execution-time query failures.
- ⚠️ Alert execution differs from validated SQL behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3ad932bc6f194760a294e936fbcc88be&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=3ad932bc6f194760a294e936fbcc88be&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/commands/report/alert.py
**Line:** 212:223
**Comment:**
*Security: The authorization and read-only checks run against
`rendered_sql`, but execution uses `limited_rendered_sql` after the
configurable SQL mutator has transformed it. A mutator can add statements,
change referenced tables, or introduce mutations after these checks, so the
database may execute SQL that was never authorized. Validate the final SQL
immediately before execution, or ensure the authorization and mutation checks
cover the exact SQL sent to the database.
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%2F42929&comment_hash=82cc22958e160932cb522b9aad6ba809067a0129a706b42a4fc65832d3cf7269&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42929&comment_hash=82cc22958e160932cb522b9aad6ba809067a0129a706b42a4fc65832d3cf7269&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]
