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>
This commit is contained in:
Amin Ghadersohi
2026-05-20 21:49:39 +00:00
parent 094cbf6972
commit 9ca3f8ec01
2 changed files with 31 additions and 3 deletions

View File

@@ -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)

View File

@@ -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__)