bito-code-review[bot] commented on code in PR #41753:
URL: https://github.com/apache/superset/pull/41753#discussion_r3532265147
##########
superset/mcp_service/chart/tool/get_chart_info.py:
##########
@@ -106,9 +106,13 @@ async def _validate_chart_dataset_access(
Logs any non-fatal warnings (e.g., virtual dataset warnings) via ctx.
"""
from superset.daos.chart import ChartDAO
+ from superset.mcp_service import guest_scope
if not result.id:
return None
+ # Guests read via the dashboard context, not dataset RBAC; skip the
perm-check.
+ if guest_scope.is_guest_read():
+ return None
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing guest-skip test coverage</b></div>
<div id="fix">
The new guest skip-guard at lines 114-115 bypasses dataset access validation
for embedded guests, but the test suite has no coverage for this code path. No
test mocks guest_scope.is_guest_read() or asserts the early-return behavior for
a guest user. Without a regression test, future refactors of the guest-skip
logic (e.g., flipping the condition or changing the early-return scope) will go
undetected.
</div>
</div>
<small><i>Code Review Run #341acc</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
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -292,6 +300,7 @@ def generate(self) -> ASCIIPreview | ChartError:
force=False,
)
+ self._authorize_guest_query(query_context)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing guest authorization tests</b></div>
<div id="fix">
The new `_authorize_guest_query` call (line 303) and the companion
`is_guest_read` skip (lines 1257-1261) have zero test coverage — grep over
`test_get_chart_preview.py` returns no hits for `is_guest_read` or
`authorize_query`. BITO.md adaptive rules [11730] and [11731] require
comprehensive unit tests for new MCP tools and their authorization paths.
</div>
</div>
<small><i>Code Review Run #341acc</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]
