This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch mcp-rbac-tool-visibility
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/mcp-rbac-tool-visibility by
this push:
new 9ca3f8ec01d fix(mcp): deny all tools on invalid credentials in search
filtering
9ca3f8ec01d is described below
commit 9ca3f8ec01d9a5062c347bbab433f68c957c2ced
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Wed May 20 21:49:39 2026 +0000
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 <[email protected]>
---
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__)
