bito-code-review[bot] commented on code in PR #43896:
URL: https://github.com/apache/superset/pull/43896#discussion_r4189386207
##########
superset/utils/error_sanitization.py:
##########
@@ -100,38 +116,75 @@ def is_sanitization_required() -> bool:
# Never let identifying the principal break the error handler itself.
logger.warning(
"Could not resolve the request principal while deciding whether to
"
- "sanitize an error response; treating it as a non-guest request.",
+ "sanitize an error response; falling back to the presence of a
guest "
+ "token.",
exc_info=True,
)
- return False
+ if not has_request_context():
+ # No request to inspect (e.g. a Celery worker running under
+ # ``override_user``). Fail closed: a guest principal may be active
+ # and the redacted payload is delivered to the embedded viewer.
+ return True
+ # ``.get`` on the config keeps the header read from raising even if the
+ # key is somehow absent.
+ header_name = current_app.config.get("GUEST_TOKEN_HEADER_NAME")
+ if header_name and request.headers.get(header_name):
+ return True
+ try:
+ # ``request.form`` parses the body lazily on first access. A body
+ # whose first parse (in the request loader) already failed part-way
+ # is left partially consumed and re-parses as an empty form; no
guest
+ # was authenticated on such a request, so it is treated as
token-free.
+ return bool(request.form.get("guest_token"))
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated guest-token detection</b></div>
<div id="fix">
The new fallback re-implements the guest-token detection already in
`security/manager.py`
(`request.headers.get(get_conf()["GUEST_TOKEN_HEADER_NAME"]) or
request.form.get("guest_token")`). It also diverges:
`current_app.config.get(...)` here vs `get_conf()["..."]` there. A future
change to the header/field name or config access in one place silently desyncs
the other. Consider a shared helper.
</div>
</div>
<small><i>Code Review Run #635ee4</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]
