codeant-ai-for-open-source[bot] commented on code in PR #43394:
URL: https://github.com/apache/superset/pull/43394#discussion_r3831857239
##########
superset/commands/sql_lab/execute.py:
##########
@@ -151,6 +151,15 @@ def _run_sql_json_exec_from_scratch(self) ->
SqlJsonExecutionStatus:
self._validate_access(query,
self._execution_context.template_params)
self._execution_context.set_query(query)
rendered_query =
self._sql_query_render.render(self._execution_context)
+ # The check above authorizes a render of query.sql +
template_params
+ # performed before rendering, so that macros with side effects are
+ # gated before they run. self._sql_query_render.render() above is
an
+ # independent second render of the same source; for a
+ # nondeterministic template (e.g. one using Jinja's `random` filter
+ # to pick a table) the two renders can diverge, letting a query
+ # read a table the first check never saw. Re-validate the literal
+ # rendered text that is about to execute.
+ self._validate_rendered_access(query, rendered_query)
Review Comment:
**Suggestion:** The re-validation covers `rendered_query`, but SQL Lab later
passes each block through `database.mutate_sql_based_on_config` before
execution. Because `SQL_QUERY_MUTATOR` can rewrite the statement, the SQL
reaching the database can reference different tables or statements than the
text authorized here. Validate the post-mutation SQL immediately before
execution, or ensure the exact authorized text is what the executor runs.
[security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Custom SQL mutators can execute unauthorized table references.
- ⚠️ SQL Lab authorization covers pre-mutation SQL only.
- ⚠️ Both synchronous and asynchronous execution paths are affected.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/sql_lab/execute.py
**Line:** 162:162
**Comment:**
*Security: The re-validation covers `rendered_query`, but SQL Lab later
passes each block through `database.mutate_sql_based_on_config` before
execution. Because `SQL_QUERY_MUTATOR` can rewrite the statement, the SQL
reaching the database can reference different tables or statements than the
text authorized here. Validate the post-mutation SQL immediately before
execution, or ensure the exact authorized text is what the executor runs.
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%2F43394&comment_hash=9592a299bfb984c22bb3e0b04cd3604ee88ace0e3beb2009e2ff2b3ec46f5f9a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43394&comment_hash=9592a299bfb984c22bb3e0b04cd3604ee88ace0e3beb2009e2ff2b3ec46f5f9a&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]
