codeant-ai-for-open-source[bot] commented on code in PR #43388:
URL: https://github.com/apache/superset/pull/43388#discussion_r3831855030
##########
superset/mcp_service/chart/tool/restore_chart.py:
##########
@@ -116,6 +116,44 @@ async def restore_chart(
return RestoreChartResponse(success=False, error=msg,
error_type="NotFound")
chart_id = chart.id
+
+ # The lookup above deliberately bypasses the RBAC base filter (see
+ # _find_chart_for_restore), so enforce the restore audience *before*
+ # composing any response that embeds the chart's name: without this gate,
+ # iterating identifiers would disclose the existence and exact title of
+ # charts the caller cannot see (the web API answers 404 for those).
+ from superset import security_manager
+ from superset.exceptions import SupersetSecurityException
+
+ try:
+ security_manager.raise_for_editorship(chart)
+ except SupersetSecurityException:
+ from superset.daos.chart import ChartDAO
+
+ # Distinguish "visible but not an editor" from "outside the caller's
+ # RBAC scope": the latter must be indistinguishable from a chart that
+ # does not exist.
+ visible = ChartDAO.find_by_id_or_uuid(
Review Comment:
**Suggestion:** Database failures during `raise_for_editorship` or the
secondary `ChartDAO.find_by_id_or_uuid` query are not caught here. Since
editorship verification re-queries the soft-deleted chart relationship, a
`SQLAlchemyError` can escape as an unhandled tool error without calling
`_rollback()` or returning the established `LookupFailed` response. Catch
`SQLAlchemyError` around the entire authorization and visibility-check block,
roll back the session, and return the same database-error response used for the
initial lookup. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Chart restoration can fail with an unstructured tool error.
- ⚠️ Failed authorization queries can leave the request session needing
rollback.
- ⚠️ Users receive inconsistent errors during database outages.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/tool/restore_chart.py
**Line:** 128:136
**Comment:**
*Possible Bug: Database failures during `raise_for_editorship` or the
secondary `ChartDAO.find_by_id_or_uuid` query are not caught here. Since
editorship verification re-queries the soft-deleted chart relationship, a
`SQLAlchemyError` can escape as an unhandled tool error without calling
`_rollback()` or returning the established `LookupFailed` response. Catch
`SQLAlchemyError` around the entire authorization and visibility-check block,
roll back the session, and return the same database-error response used for the
initial lookup.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=377446db6cae8c0e75a88229c6e17c6a510322c6677226698ab112a855563fce&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=377446db6cae8c0e75a88229c6e17c6a510322c6677226698ab112a855563fce&reaction=dislike'>👎</a>
##########
superset/mcp_service/mcp_config.py:
##########
@@ -521,7 +539,14 @@ def create_default_mcp_auth_factory(app: Flask) ->
Optional[Any]:
if not (jwks_uri or public_key or secret):
logger.warning("MCP_AUTH_ENABLED is True but no JWT keys/secret
configured")
if not (api_key_enabled or guest_enabled):
- return None
+ # Fail closed: the surrounding bootstrap would otherwise turn
+ # a None provider into an unauthenticated server.
+ raise MCPAuthConfigError(
+ "MCP_AUTH_ENABLED is True but no JWT verification key is "
+ "configured; refusing to start an unauthenticated MCP "
+ "server. Set MCP_JWKS_URI, MCP_JWT_PUBLIC_KEY, or "
+ "MCP_JWT_SECRET (with MCP_JWT_ALGORITHM='HS256')."
+ )
Review Comment:
**Suggestion:** When JWT authentication is enabled together with API-key or
guest authentication, this condition allows startup with `jwt_verifier` still
set to `None` if no JWT key is configured. `_build_composite_verifier` then
installs only the fallback verifiers, silently disabling the explicitly enabled
JWT mode. The missing-key case must raise `MCPAuthConfigError` regardless of
whether another authentication mode is enabled. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Configured JWT clients cannot authenticate to MCP.
- ⚠️ API-key or guest authentication masks the JWT misconfiguration.
- ⚠️ Operators receive only a warning instead of startup failure.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/mcp_config.py
**Line:** 539:549
**Comment:**
*Security: When JWT authentication is enabled together with API-key or
guest authentication, this condition allows startup with `jwt_verifier` still
set to `None` if no JWT key is configured. `_build_composite_verifier` then
installs only the fallback verifiers, silently disabling the explicitly enabled
JWT mode. The missing-key case must raise `MCPAuthConfigError` regardless of
whether another authentication mode is enabled.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=e26ae5db04f36a3bb8d156bd8c12ab9efc6e6174fbfd8043f45fb235b1681076&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43388&comment_hash=e26ae5db04f36a3bb8d156bd8c12ab9efc6e6174fbfd8043f45fb235b1681076&reaction=dislike'>👎</a>
--
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]
