From 9ca3f8ec01d9a5062c347bbab433f68c957c2ced Mon Sep 17 00:00:00 2001
From: Amin Ghadersohi <amin.ghadersohi@gmail.com>
Date: Wed, 20 May 2026 21:49:39 +0000
Subject: [PATCH] fix(mcp): deny all tools on invalid credentials in search
 filtering

Split the combined except clause in _tool_allowed_for_current_user so
that PermissionError (bad API key) returns False for every tool,
matching RBACToolVisibilityMiddleware's fail-closed behaviour.
ValueError (no auth source configured) retains the existing public-tool
fallback. Adds a test to cover the new deny-all path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 superset/mcp_service/server.py                | 10 +++++---
 .../mcp_service/test_tool_search_transform.py | 24 +++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/superset/mcp_service/server.py b/superset/mcp_service/server.py
index 92166606e43..08d5d915996 100644
--- a/superset/mcp_service/server.py
+++ b/superset/mcp_service/server.py
@@ -410,9 +410,13 @@ def _tool_allowed_for_current_user(tool: Any) -> bool:
         if not getattr(g, "user", None):
             try:
                 g.user = get_user_from_request()
-            except (ValueError, PermissionError):
-                # Can't resolve user; only hide protected tools. Public tools
-                # (no _class_permission_name) pass through regardless.
+            except PermissionError:
+                # Invalid credentials (bad API key) → deny all, matching
+                # RBACToolVisibilityMiddleware's fail-closed behaviour.
+                return False
+            except ValueError:
+                # No auth source configured → only pass public tools
+                # (those with no class-level permission requirement).
                 func = getattr(tool, "fn", tool)
                 return not getattr(func, "_class_permission_name", None)
 
diff --git a/tests/unit_tests/mcp_service/test_tool_search_transform.py b/tests/unit_tests/mcp_service/test_tool_search_transform.py
index 3798166c601..b8f301c9692 100644
--- a/tests/unit_tests/mcp_service/test_tool_search_transform.py
+++ b/tests/unit_tests/mcp_service/test_tool_search_transform.py
@@ -901,6 +901,30 @@ def test_tool_search_permission_filter_hides_protected_tools_without_user() -> N
     assert result == [public]
 
 
+def test_tool_search_permission_filter_denies_all_on_invalid_credentials() -> None:
+    """Invalid credentials (PermissionError) deny all tools, including public ones."""
+    app = Flask(__name__)
+    app.config["MCP_RBAC_ENABLED"] = True
+
+    def protected_tool():
+        pass
+
+    setattr(protected_tool, CLASS_PERMISSION_ATTR, "Dataset")
+    setattr(protected_tool, METHOD_PERMISSION_ATTR, "read")
+
+    protected = SimpleNamespace(fn=protected_tool)
+    public = SimpleNamespace(fn=lambda: None)
+
+    with app.app_context():
+        with patch(
+            "superset.mcp_service.auth.get_user_from_request",
+            side_effect=PermissionError("Invalid API key"),
+        ):
+            result = _filter_tools_by_current_user_permission([protected, public])
+
+    assert result == []
+
+
 def test_tool_search_filter_hides_metadata_tools_without_access() -> None:
     """Privacy-marked tools are hidden even if broad Dataset read exists."""
     app = Flask(__name__)