bito-code-review[bot] commented on code in PR #38683:
URL: https://github.com/apache/superset/pull/38683#discussion_r2943740829
##########
superset/models/helpers.py:
##########
@@ -2049,15 +2053,20 @@ def get_from_clause(
if parsed_script.statements:
default_schema = self.database.get_default_schema(self.catalog)
try:
- for statement in parsed_script.statements:
+ # Only regenerate the SQL if RLS predicates were actually
applied.
+ # Unnecessary round-tripping through sqlglot can alter SQL in
+ # dialect-specific ways (e.g. dropping column aliases,
rewriting
+ # Redshift-specific syntax) and break virtual dataset queries.
+ if any(
apply_rls(
self.database,
self.catalog,
self.schema or default_schema or "",
statement,
)
- # Regenerate the SQL after RLS application
- from_sql = parsed_script.format()
+ for statement in parsed_script.statements
+ ):
+ from_sql = parsed_script.format()
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>RLS Short-Circuit Bug</b></div>
<div id="fix">
The any() generator expression short-circuits after the first apply_rls()
returns True, skipping RLS application to subsequent statements. This can
bypass RLS on later statements in virtual datasets, leading to potential
security issues. Apply to all statements before checking.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
applied = [
apply_rls(
self.database,
self.catalog,
self.schema or default_schema or "",
statement,
)
for statement in parsed_script.statements
]
if any(applied):
from_sql = parsed_script.format()
````
</div>
</details>
</div>
<small><i>Code Review Run #a1ed00</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]
