diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx index df299acaf8d..1279b03f85a 100644 --- a/docs/admin_docs/configuration/mcp-server.mdx +++ b/docs/admin_docs/configuration/mcp-server.mdx @@ -502,6 +502,7 @@ All MCP settings go in `superset_config.py`. Defaults are defined in `superset/m | `MCP_DEBUG` | `False` | Enable debug logging | | `MCP_DEV_USERNAME` | -- | Superset username for development mode (no auth) | | `MCP_RBAC_ENABLED` | `True` | Enforce Superset's role-based access control on MCP tool calls. When `True`, each tool checks that the authenticated user has the required FAB permission before executing. Disable only for testing or trusted-network deployments. | +| `MCP_DISABLED_TOOLS` | `set()` | Set of tool names to remove from the MCP server at startup. Disabled tools are never advertised to AI clients during tool discovery. Useful when a custom extension tool should replace a built-in Superset tool. See [Disabling built-in tools](#disabling-built-in-tools). | ### Authentication @@ -825,6 +826,32 @@ while True: page += 1 ``` +## Disabling built-in tools + +If you have deployed a custom tool via a Superset extension that supersedes one of the built-in Superset tools, you can suppress the built-in version so AI clients only discover your replacement. Disabled tools are removed from the server at startup and are never advertised during tool discovery. + +Set `MCP_DISABLED_TOOLS` in your `superset_config.py` to a set of tool names: + +```python +# superset_config.py + +# Disable one tool +MCP_DISABLED_TOOLS = {"execute_sql"} + +# Disable multiple tools +MCP_DISABLED_TOOLS = {"execute_sql", "health_check"} +``` + +Tool names match the function name used in the `@tool` decorator (e.g., `execute_sql`, `list_charts`, `health_check`). Extension-prefixed tools can also be disabled using their full prefixed name: + +```python +MCP_DISABLED_TOOLS = {"extensions.myorg.myextension.some_tool"} +``` + +:::note +Specifying a tool name that does not exist logs a warning at startup and is otherwise ignored — it will not prevent the server from starting. +::: + ## Security Best Practices - **Use TLS** for all production MCP endpoints -- place the server behind a reverse proxy with HTTPS diff --git a/superset/mcp_service/app.py b/superset/mcp_service/app.py index 7ab6a7a774b..0a68d168a07 100644 --- a/superset/mcp_service/app.py +++ b/superset/mcp_service/app.py @@ -30,18 +30,68 @@ from fastmcp.server.middleware import Middleware logger = logging.getLogger(__name__) +# --------------------------------------------------------------------------- +# Prose snippets that reference get_instance_info. +# These are included in the generated instructions only when that tool is +# enabled; each snippet is a plain string constant so they can be read +# independently of the filtering logic in get_default_instructions(). +# --------------------------------------------------------------------------- +_SNIPPET_FEATURE_AVAILABILITY = ( + "Feature Availability:\n" + "- Call get_instance_info to discover accessible menus for the current user.\n" + "- Do NOT assume features exist; always check get_instance_info first.\n" + "\n" +) +_SNIPPET_INSTANCE_INFO_ROLE_BULLET = ( + "- get_instance_info returns current_user.roles" + ' (e.g., ["Admin"], ["Alpha"], ["Viewer"]).\n' +) +_SNIPPET_ACCESSIBLE_MENUS_BULLET = ( + "- If you are unsure about a user's capabilities," + " check their accessible_menus in\n" + " feature_availability from get_instance_info.\n" +) +_SNIPPET_UNSURE_GUIDANCE = ( + "\nIf you are unsure which tool to use, start with get_instance_info\n" + "or use the quickstart prompt for an interactive guide.\n" +) +_SNIPPET_CONNECT_GUIDANCE = ( + "\nWhen you first connect, call get_instance_info to learn the user's identity.\n" + "Greet them by their first name (from current_user) and offer to help.\n" +) -def get_default_instructions(branding: str = "Apache Superset") -> str: + +def get_default_instructions( + branding: str = "Apache Superset", + disabled_tools: set[str] | None = None, +) -> str: """Get default instructions with configurable branding. + Tool bullet-point lines for any tool name in ``disabled_tools`` are + omitted so that LLM clients are never told to call a tool that has been + suppressed via ``MCP_DISABLED_TOOLS``. + Args: branding: Product name to use in instructions (e.g., "ACME Analytics", "Apache Superset") + disabled_tools: Set of tool names to omit from the tool listing. + When ``None`` (default) all tools are included. Returns: Formatted instructions string with branding applied """ - return f""" + _disabled = disabled_tools or set() + + # Prose sections that reference get_instance_info are omitted when that + # tool is disabled so the LLM is never directed to call a removed tool. + _show = "get_instance_info" not in _disabled + _feature_availability = _SNIPPET_FEATURE_AVAILABILITY if _show else "" + _instance_info_role_bullet = _SNIPPET_INSTANCE_INFO_ROLE_BULLET if _show else "" + _accessible_menus_bullet = _SNIPPET_ACCESSIBLE_MENUS_BULLET if _show else "" + _unsure_guidance = _SNIPPET_UNSURE_GUIDANCE if _show else "" + _connect_guidance = _SNIPPET_CONNECT_GUIDANCE if _show else "" + + instructions = f""" You are connected to the {branding} MCP (Model Context Protocol) service. This service provides programmatic access to {branding} dashboards, charts, datasets, SQL Lab, and instance metadata via a comprehensive set of tools. @@ -302,13 +352,8 @@ Input format: - Tool request parameters accept structured objects (dicts/JSON) - FastMCP 3.1+ handles Pydantic BaseModel parameters natively -Feature Availability: -- Call get_instance_info to discover accessible menus for the current user. -- Do NOT assume features exist; always check get_instance_info first. - -Permission Awareness: -- get_instance_info returns current_user.roles (e.g., ["Admin"], ["Alpha"], ["Viewer"]). -- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets, +{_feature_availability}Permission Awareness: +{_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets, charts, dashboards, or running SQL). - Do NOT disclose dashboard access lists, dashboard owners, chart owners, dataset owners, workspace admins, or other users' names, usernames, email addresses, @@ -332,15 +377,38 @@ Permission Awareness: 1. Explain that they may not have access to the requested resources 2. Suggest they ask a workspace admin to grant them access or share content with them 3. Offer to help with what they CAN do (e.g., viewing dashboards they have access to) -- If you are unsure about a user's capabilities, check their accessible_menus in - feature_availability from get_instance_info. +{_accessible_menus_bullet}{_unsure_guidance}{_connect_guidance}""" + if not _disabled: + return instructions -If you are unsure which tool to use, start with get_instance_info -or use the quickstart prompt for an interactive guide. - -When you first connect, call get_instance_info to learn the user's identity. -Greet them by their first name (from current_user) and offer to help. -""" + # Strip any line that mentions a disabled tool — this covers both the + # "- tool_name: ..." bullet entries and all prose/workflow references + # (request wrapper examples, workflow steps, CRITICAL RULES, etc.). + # Tool names are specific enough (e.g. execute_sql, generate_chart) that + # false positives are not a practical concern. + # + # Bullet continuation lines (indented lines belonging to a disabled bullet) + # are also dropped via the skip_continuation flag. + filtered_lines = [] + skip_continuation = False + for line in instructions.splitlines(keepends=True): + stripped = line.lstrip() + if stripped.startswith("- "): + tool_part = stripped[2:].split(":")[0].strip() + if tool_part in _disabled: + skip_continuation = True + continue + skip_continuation = False + elif skip_continuation and stripped and not stripped.startswith("- "): + # Indented continuation line of the previous disabled bullet — skip + continue + else: + skip_continuation = False + # Drop any prose line that names a disabled tool + if any(tool in line for tool in _disabled): + continue + filtered_lines.append(line) + return "".join(filtered_lines) # For backwards compatibility, keep DEFAULT_INSTRUCTIONS pointing to default branding @@ -569,6 +637,25 @@ from superset.mcp_service.system.tool import ( # noqa: F401, E402 ) +def _remove_disabled_tools(disabled_tools: set[str]) -> None: + """Remove tools listed in MCP_DISABLED_TOOLS from the global MCP instance. + + Disabled tools are removed before the server starts serving requests so they + are never advertised to AI clients during tool discovery. Users configure + this via MCP_DISABLED_TOOLS in superset_config.py. + """ + for tool_name in disabled_tools: + try: + mcp.local_provider.remove_tool(tool_name) + logger.info("Disabled MCP tool: %s (MCP_DISABLED_TOOLS)", tool_name) + except KeyError: + logger.warning( + "MCP_DISABLED_TOOLS: tool %r not found — " + "check the tool name is correct", + tool_name, + ) + + def init_fastmcp_server( name: str | None = None, instructions: str | None = None, @@ -608,8 +695,14 @@ def init_fastmcp_server( # Apply branding defaults if not explicitly provided if name is None: name = default_name + + # Remove disabled tools BEFORE generating instructions so that the + # instructions never advertise tools that clients cannot actually call. + disabled_tools: set[str] = flask_app.config.get("MCP_DISABLED_TOOLS", set()) + _remove_disabled_tools(disabled_tools) + if instructions is None: - instructions = get_default_instructions(branding) + instructions = get_default_instructions(branding, disabled_tools) # Configure the global mcp instance with provided settings. # Tools are already registered on this instance via @tool decorator imports above. diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py index 65fdeba095d..d12b44bbc87 100644 --- a/superset/mcp_service/mcp_config.py +++ b/superset/mcp_service/mcp_config.py @@ -56,6 +56,19 @@ MCP_DEBUG = False # against the FAB security_manager before execution. MCP_RBAC_ENABLED = True +# MCP Disabled Tools - a set of tool names to remove from the MCP server at +# startup. Disabled tools are silently omitted from tool discovery, so AI +# clients never see them. Use this when a Superset-provided tool conflicts with +# a custom tool added via an extension and you want to suppress the built-in +# version. +# +# Example: +# MCP_DISABLED_TOOLS = {"execute_sql", "health_check"} +# +# Extension-prefixed tools can also be disabled using their full name: +# MCP_DISABLED_TOOLS = {"extensions.myorg.myext.some_tool"} +MCP_DISABLED_TOOLS: set[str] = set() + # MCP JWT Debug Errors - controls server-side JWT debug logging. # When False (default), uses the default JWTVerifier with minimal logging. # When True, uses DetailedJWTVerifier with tiered logging: @@ -402,6 +415,7 @@ def get_mcp_config(app_config: Dict[str, Any] | None = None) -> Dict[str, Any]: "MCP_SERVICE_PORT": MCP_SERVICE_PORT, "MCP_DEBUG": MCP_DEBUG, "MCP_RBAC_ENABLED": MCP_RBAC_ENABLED, + "MCP_DISABLED_TOOLS": set(MCP_DISABLED_TOOLS), **MCP_SESSION_CONFIG, **MCP_CSRF_CONFIG, } diff --git a/tests/unit_tests/mcp_service/test_mcp_config.py b/tests/unit_tests/mcp_service/test_mcp_config.py index 074ce23b541..7cc6d2809d6 100644 --- a/tests/unit_tests/mcp_service/test_mcp_config.py +++ b/tests/unit_tests/mcp_service/test_mcp_config.py @@ -105,11 +105,22 @@ def test_get_default_instructions_forbid_disclosing_other_user_access_or_roles() assert "direct them to their workspace admin" in instructions +def _mock_flask_config(app_name: str) -> MagicMock: + """Return a Flask app mock whose config.get() returns correct types per key.""" + mock = MagicMock() + mock.config.get.side_effect = lambda key, default=None: ( + app_name + if key == "APP_NAME" + else set() + if key == "MCP_DISABLED_TOOLS" + else default + ) + return mock + + def test_init_fastmcp_server_with_default_app_name(): """Test that default APP_NAME produces Superset branding.""" - # Mock Flask app config with default APP_NAME - mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = "Superset" + mock_flask_app = _mock_flask_config("Superset") # Patch at the import location to avoid actual Flask app creation with patch.dict( @@ -127,9 +138,7 @@ def test_init_fastmcp_server_with_default_app_name(): def test_init_fastmcp_server_with_custom_app_name(): """Test that custom APP_NAME produces branded instructions.""" custom_app_name = "ACME Analytics" - # Mock Flask app config with custom APP_NAME - mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = custom_app_name + mock_flask_app = _mock_flask_config(custom_app_name) # Patch at the import location to avoid actual Flask app creation with patch.dict( @@ -149,10 +158,7 @@ def test_init_fastmcp_server_derives_server_name_from_app_name(): """Test that server name is derived from APP_NAME.""" custom_app_name = "DataViz Platform" expected_server_name = f"{custom_app_name} MCP Server" - - # Mock Flask app config - mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = custom_app_name + mock_flask_app = _mock_flask_config(custom_app_name) # Patch at the import location to avoid actual Flask app creation with patch.dict( @@ -168,8 +174,7 @@ def test_init_fastmcp_server_derives_server_name_from_app_name(): def test_init_fastmcp_server_applies_auth_to_global_instance(): """Test that auth is applied to the global mcp instance, not a new one.""" - mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = "Superset" + mock_flask_app = _mock_flask_config("Superset") mock_auth = MagicMock() with patch.dict( @@ -187,8 +192,7 @@ def test_init_fastmcp_server_applies_auth_to_global_instance(): def test_init_fastmcp_server_applies_middleware_to_global_instance(): """Test that middleware is added to the global mcp instance.""" - mock_flask_app = MagicMock() - mock_flask_app.config.get.return_value = "Superset" + mock_flask_app = _mock_flask_config("Superset") mock_mw = MagicMock() with patch.dict( @@ -200,3 +204,23 @@ def test_init_fastmcp_server_applies_middleware_to_global_instance(): # Middleware should be added via add_middleware mock_mcp.add_middleware.assert_called_once_with(mock_mw) + + +def test_get_mcp_config_includes_mcp_disabled_tools_key() -> None: + """get_mcp_config must include MCP_DISABLED_TOOLS in its defaults dict so the + key is available in flask_app.config for the standalone server startup path.""" + from superset.mcp_service.mcp_config import get_mcp_config + + config = get_mcp_config() + assert "MCP_DISABLED_TOOLS" in config + assert config["MCP_DISABLED_TOOLS"] == set() + + +def test_get_mcp_config_respects_app_config_override() -> None: + """When app_config provides MCP_DISABLED_TOOLS, it takes precedence over the + module-level default.""" + from superset.mcp_service.mcp_config import get_mcp_config + + custom = {"execute_sql", "health_check"} + config = get_mcp_config({"MCP_DISABLED_TOOLS": custom}) + assert config["MCP_DISABLED_TOOLS"] == custom diff --git a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py index 9307a671659..00a94fa78e9 100644 --- a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py +++ b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py @@ -18,6 +18,10 @@ """Test MCP app imports and tool/prompt registration.""" import asyncio +import logging +from unittest.mock import MagicMock, patch + +from superset.mcp_service.app import get_default_instructions, init_fastmcp_server, mcp def _run(coro): @@ -95,3 +99,188 @@ def test_mcp_packages_discoverable_by_setuptools(): f"MCP sub-packages missing __init__.py (will be excluded from " f"setuptools distributions): {missing}" ) + + +# --------------------------------------------------------------------------- +# MCP_DISABLED_TOOLS tests +# --------------------------------------------------------------------------- + + +def _make_flask_app_mock(disabled_tools: set[str]) -> MagicMock: + """Return a minimal Flask app mock with MCP_DISABLED_TOOLS configured.""" + flask_app = MagicMock() + flask_app.config.get.side_effect = lambda key, default=None: ( + disabled_tools if key == "MCP_DISABLED_TOOLS" else default + ) + return flask_app + + +def test_disabled_tools_are_removed_from_mcp_server() -> None: + """Tools listed in MCP_DISABLED_TOOLS are removed before the server starts.""" + + flask_app = _make_flask_app_mock({"health_check", "list_charts"}) + + with ( + patch( + "superset.mcp_service.flask_singleton.app", + flask_app, + ), + patch.object(mcp.local_provider, "remove_tool") as mock_remove, + ): + init_fastmcp_server() + + removed = {call.args[0] for call in mock_remove.call_args_list} + assert "health_check" in removed + assert "list_charts" in removed + + +def test_unknown_disabled_tool_logs_warning_not_raises(caplog) -> None: + """An unknown tool name in MCP_DISABLED_TOOLS logs a warning and does not crash.""" + + flask_app = _make_flask_app_mock({"nonexistent_tool_xyz"}) + + with ( + patch( + "superset.mcp_service.flask_singleton.app", + flask_app, + ), + patch.object( + mcp.local_provider, + "remove_tool", + side_effect=KeyError("nonexistent_tool_xyz"), + ), + caplog.at_level(logging.WARNING, logger="superset.mcp_service.app"), + ): + # Must not raise + init_fastmcp_server() + + assert "nonexistent_tool_xyz" in caplog.text + assert "MCP_DISABLED_TOOLS" in caplog.text + + +def test_empty_disabled_tools_removes_nothing() -> None: + """An empty MCP_DISABLED_TOOLS set leaves all tools registered.""" + + flask_app = _make_flask_app_mock(set()) + + with ( + patch( + "superset.mcp_service.flask_singleton.app", + flask_app, + ), + patch.object(mcp.local_provider, "remove_tool") as mock_remove, + ): + init_fastmcp_server() + + mock_remove.assert_not_called() + + +def test_disabled_tools_read_from_flask_app_config() -> None: + """MCP_DISABLED_TOOLS is read from flask_app.config, matching the standard + Superset pattern where users set overrides in superset_config.py, which + create_app() loads into Flask config before any command runs.""" + from superset.mcp_service.app import init_fastmcp_server, mcp + + flask_app = _make_flask_app_mock({"health_check"}) + + with ( + patch( + "superset.mcp_service.flask_singleton.app", + flask_app, + ), + patch.object(mcp.local_provider, "remove_tool") as mock_remove, + ): + init_fastmcp_server() + + removed = {call.args[0] for call in mock_remove.call_args_list} + assert "health_check" in removed + + +# --------------------------------------------------------------------------- +# get_default_instructions disabled_tools filtering tests +# --------------------------------------------------------------------------- + + +def test_disabled_tools_absent_from_instructions() -> None: + """Tools in disabled_tools must not appear as bullet lines in instructions.""" + instructions = get_default_instructions( + disabled_tools={"execute_sql", "health_check"} + ) + + # The bullet-point entries for disabled tools must be gone + assert "- execute_sql:" not in instructions + assert "- health_check:" not in instructions + # Non-disabled tools must still be present + assert "- list_charts:" in instructions + assert "- list_dashboards:" in instructions + + +def test_disabling_get_instance_info_removes_all_prose_references() -> None: + """Disabling get_instance_info must remove ALL prose references to it, + not only the bullet-point entry in the Available tools section.""" + instructions = get_default_instructions(disabled_tools={"get_instance_info"}) + + # Bullet entry must be gone + assert "- get_instance_info:" not in instructions + # Prose directives that instruct the LLM to call the tool must also be gone + assert "start with get_instance_info" not in instructions + assert "call get_instance_info" not in instructions + assert "check their accessible_menus in" not in instructions + assert "Feature Availability" not in instructions + # Instructions for other tools must be unaffected + assert "- list_charts:" in instructions + assert "- execute_sql:" in instructions + + +def test_disabling_execute_sql_removes_all_prose_references() -> None: + """Disabling execute_sql must remove all workflow and example lines that + mention it, not only the bullet-point entry.""" + instructions = get_default_instructions(disabled_tools={"execute_sql"}) + + # Bullet entry must be gone + assert "- execute_sql:" not in instructions + # Workflow steps and request wrapper examples must also be gone + assert "execute_sql(" not in instructions + assert "execute_sql" not in instructions + # Instructions for unrelated tools must be unaffected + assert "- list_charts:" in instructions + assert "- get_instance_info:" in instructions + + +def test_no_disabled_tools_returns_full_instructions() -> None: + """Passing no disabled_tools (or empty set) returns the full instructions.""" + full = get_default_instructions() + also_full = get_default_instructions(disabled_tools=set()) + + assert "- execute_sql:" in full + assert "- health_check:" in full + assert full == also_full + + +def test_instructions_generated_after_disabled_tools_removed() -> None: + """init_fastmcp_server generates instructions AFTER removing disabled tools, + so the instructions never advertise tools that clients cannot call.""" + flask_app = _make_flask_app_mock({"execute_sql"}) + + captured: list[str] = [] + + def fake_get_instructions( + branding: str = "Apache Superset", + disabled_tools: set[str] | None = None, + ) -> str: + captured.append(str(disabled_tools)) + return f"instructions for {branding}" + + with ( + patch("superset.mcp_service.flask_singleton.app", flask_app), + patch.object(mcp.local_provider, "remove_tool"), + patch( + "superset.mcp_service.app.get_default_instructions", + fake_get_instructions, + ), + ): + init_fastmcp_server() + + # get_default_instructions must have been called with the disabled set + assert len(captured) == 1 + assert "execute_sql" in captured[0]