bito-code-review[bot] commented on code in PR #44968:
URL: https://github.com/apache/superset/pull/44968#discussion_r4180091984
##########
superset/mcp_service/sql_lab/tool/execute_sql.py:
##########
@@ -210,12 +248,17 @@ async def execute_sql(request: ExecuteSqlRequest, ctx:
Context) -> ExecuteSqlRes
await ctx.error("Query could not be parsed for access
validation")
return _invalid_sql_response()
+ rendered_sql = await _authorize_rendered_sql(
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Redundant double authorization</b></div>
<div id="fix">
`_authorize_rendered_sql` (line 251) re-renders and re-authorizes
`request.sql`, duplicating the `raise_for_access` at line 227, which already
renders via `process_jinja_sql` and checks access. Every query now
renders/parses twice with two different renderers. Since execution uses
`rendered_sql`, the first check is redundant; consider removing it so only the
rendered SQL is authorized.
</div>
</div>
<small><i>Code Review Run #483b91</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
