aminghadersohi commented on code in PR #41922:
URL: https://github.com/apache/superset/pull/41922#discussion_r3582904236
##########
superset/mcp_service/chart/tool/get_chart_type_schema.py:
##########
@@ -172,6 +172,7 @@ def _get_chart_type_schema_impl(
@tool(
tags=["discovery"],
+ class_permission_name="Chart",
annotations=ToolAnnotations(
Review Comment:
Good catch, and you're right that this is a real runtime behavior change,
not just metadata — updated the PR description to say so explicitly instead of
claiming 'No tool behavior changes.'
To be precise about the effect: `get_chart_type_schema` was the only one of
62 tools missing `class_permission_name`, so with `MCP_RBAC_ENABLED` it
previously fail-opened (authenticated, no permission check). Adding
`class_permission_name="Chart"` closes that gap and makes it require Chart read
permission, consistent with every other chart-scoped tool (`get_chart_info`,
`list_charts`, etc.). Since it's a pure schema-lookup tool (no dataset access),
Chart read is the correct/minimal permission for it.
This is an intentional fix, not an accidental behavior change — but you're
right the PR description shouldn't have said 'no behavior changes' when this
one line does change enforcement. Fixed.
--
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]
