bito-code-review[bot] commented on code in PR #44494:
URL: https://github.com/apache/superset/pull/44494#discussion_r4067606520
##########
superset/commands/chart/update.py:
##########
@@ -181,6 +185,12 @@ def validate(self) -> None: # noqa: C901
exceptions.append(ex)
raise_if_managed_externally(self._model, ChartForbiddenError)
else:
+ # ``raise_for_access`` admits a guest for every chart on the
+ # dashboard its token embeds, and a guest holds no write
+ # capability, so deny explicitly. The regular update path gets this
+ # from ``is_editor``, which never treats a guest as an editor.
+ if security_manager.is_guest_user():
+ raise ChartForbiddenError()
try:
security_manager.raise_for_access(chart=self._model)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-284: Access Control Relaxed</b></div>
<div id="fix">
Relaxing `raise_for_editorship` to `raise_for_access(chart=...)` lets
viewers and datasource-only users rewrite the chart's stored query context —
executable state that report execution loads and runs. `raise_for_access`
(manager.py:5097-5136) admits viewers and datasource-access users, not just
editors. The guest deny closes only the guest hole. Consider scoping the
relaxation to report executors.
([CWE-284](https://cwe.mitre.org/data/definitions/284.html))
</div>
</div>
<small><i>Code Review Run #ef12da</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]
