bito-code-review[bot] commented on code in PR #37973:
URL: https://github.com/apache/superset/pull/37973#discussion_r2888934717
##########
tests/unit_tests/security/api_test.py:
##########
@@ -29,6 +29,7 @@ def test_csrf_not_exempt(app_context: None) -> None:
Test that REST API is not exempt from CSRF.
"""
assert {blueprint.name for blueprint in csrf._exempt_blueprints} == {
+ "ApiKeyApi",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test name/comment mismatch</b></div>
<div id="fix">
The test function name 'test_csrf_not_exempt' and docstring claiming 'REST
API is not exempt from CSRF' contradict the code, which asserts that these API
blueprints (including the newly added 'ApiKeyApi') are exempt. This mismatch
could confuse maintainers. The code correctly verifies that API blueprints
using token-based auth are exempt from CSRF, as they don't rely on cookies.
</div>
</div>
<small><i>Code Review Run #8d1294</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/auth.py:
##########
@@ -127,8 +159,8 @@ def get_user_from_request() -> User:
raise ValueError(
"No authenticated user found. Tried:\n"
+ "\n".join(f" - {d}" for d in details)
- + "\n\nEither pass a valid JWT bearer token or configure "
- "MCP_DEV_USERNAME for development."
+ + "\n\nEither pass a valid API key (Bearer sst_...), "
+ "JWT token, or configure MCP_DEV_USERNAME for development."
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Configurable prefix not reflected in error
message</b></div>
<div id="fix">
The error message hardcodes the default API key prefix 'sst_', but since
FAB_API_KEY_PREFIXES is configurable, the message should dynamically reflect
the actual configured prefixes to avoid misleading users who have customized
the prefix.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- details.append("MCP_DEV_USERNAME is not configured")
- raise ValueError(
- "No authenticated user found. Tried:\\n"
- + "\\n".join(f" - {d}" for d in details)
- + "\\n\\nEither pass a valid API key (Bearer sst_...), "
- "JWT token, or configure MCP_DEV_USERNAME for development."
- )
+ details.append("MCP_DEV_USERNAME is not configured")
+ prefixes = current_app.config.get("FAB_API_KEY_PREFIXES", ["sst_"])
+ raise ValueError(
- "No authenticated user found. Tried:\\n"
- + "\\n".join(f" - {d}" for d in details)
- + f"\\n\\nEither pass a valid API key (Bearer
{prefixes[0]}...), "
- "JWT token, or configure MCP_DEV_USERNAME for development."
- )
```
</div>
</details>
</div>
<small><i>Code Review Run #8d1294</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]
