This is an automated email from the ASF dual-hosted git repository. aminghadersohi pushed a commit to branch work-pr-39604 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 35ac1ce13d0d3e7253aaa3364180cbf153b325ec Author: Amin Ghadersohi <[email protected]> AuthorDate: Fri May 22 02:21:45 2026 +0000 fix(mcp): fix MCPPermissionDeniedError handler order and visibility test patch targets MCPPermissionDeniedError(PermissionError) was being caught by the generic PermissionError branch in _handle_error before reaching its own handler, so the ToolError message was sanitized to "You don't have access to this resource." instead of the structured permission message. Move the MCPPermissionDeniedError branch above PermissionError so the subclass is matched first. Also fix four visibility test patch targets: auth.py imports security_manager at module level via `from superset import security_manager`, so tests must patch `superset.mcp_service.auth.security_manager` (not `superset.security_manager`) to intercept calls inside auth.py. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- superset/mcp_service/middleware.py | 8 +++++--- tests/unit_tests/mcp_service/test_auth_rbac.py | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py index 685022c3a9f..02688a761f6 100644 --- a/superset/mcp_service/middleware.py +++ b/superset/mcp_service/middleware.py @@ -632,6 +632,11 @@ class GlobalErrorHandlerMiddleware(Middleware): elif isinstance(error, HTTPException): # HTTP errors from screenshot endpoints or API calls raise ToolError(f"Service error in {tool_name}: {error.detail}") from error + elif isinstance(error, MCPPermissionDeniedError): + # MCP RBAC permission denied — convert to structured ToolError. + # Must come before the generic PermissionError branch because + # MCPPermissionDeniedError inherits from PermissionError. + raise ToolError(str(error)) from error elif isinstance(error, PermissionError): # Permission/authorization errors raise ToolError( @@ -648,9 +653,6 @@ class GlobalErrorHandlerMiddleware(Middleware): raise ToolError( f"Invalid request for {tool_name}: {_sanitize_error_for_logging(error)}" ) from error - elif isinstance(error, MCPPermissionDeniedError): - # MCP RBAC permission denied — convert to structured ToolError - raise ToolError(str(error)) from error elif isinstance(error, (ForbiddenError, SupersetSecurityException)): # Superset access denied — agent tried a tool it can't use raise ToolError( diff --git a/tests/unit_tests/mcp_service/test_auth_rbac.py b/tests/unit_tests/mcp_service/test_auth_rbac.py index 64a9684eaec..55a97dacea7 100644 --- a/tests/unit_tests/mcp_service/test_auth_rbac.py +++ b/tests/unit_tests/mcp_service/test_auth_rbac.py @@ -280,7 +280,7 @@ def test_visibility_allowed_tool(app_context) -> None: mock_sm = MagicMock() mock_sm.can_access = MagicMock(return_value=True) - with patch("superset.security_manager", mock_sm): + with patch("superset.mcp_service.auth.security_manager", mock_sm): result = is_tool_visible_to_current_user(tool) assert result is True @@ -295,7 +295,7 @@ def test_visibility_denied_tool(app_context) -> None: mock_sm = MagicMock() mock_sm.can_access = MagicMock(return_value=False) - with patch("superset.security_manager", mock_sm): + with patch("superset.mcp_service.auth.security_manager", mock_sm): result = is_tool_visible_to_current_user(tool) assert result is False @@ -312,7 +312,7 @@ def test_visibility_data_model_metadata_denied(app_context) -> None: mock_sm = MagicMock() mock_sm.can_access = MagicMock(return_value=True) with ( - patch("superset.security_manager", mock_sm), + patch("superset.mcp_service.auth.security_manager", mock_sm), patch( "superset.mcp_service.privacy.user_can_view_data_model_metadata", return_value=False, @@ -334,7 +334,7 @@ def test_visibility_data_model_metadata_allowed(app_context) -> None: mock_sm = MagicMock() mock_sm.can_access = MagicMock(return_value=True) with ( - patch("superset.security_manager", mock_sm), + patch("superset.mcp_service.auth.security_manager", mock_sm), patch( "superset.mcp_service.privacy.user_can_view_data_model_metadata", return_value=True,
