You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/09/17 10:59:50 UTC

(superset) branch mcp-specific-denial-errors updated (ff9f301f31d -> c5bb07bd3e2)

This is an automated email from the ASF dual-hosted git repository.

aminghadersohi pushed a change to branch mcp-specific-denial-errors
in repository https://gitbox.apache.org/repos/asf/superset.git


 discard ff9f301f31d fix(mcp): return a specific error per failure class on tool calls
     new c5bb07bd3e2 fix(mcp): return a specific error per failure class on tool calls

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (ff9f301f31d)
            \
             N -- N -- N   refs/heads/mcp-specific-denial-errors (c5bb07bd3e2)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 superset/mcp_service/middleware.py                        | 12 +++++++-----
 tests/unit_tests/mcp_service/test_error_classification.py |  4 +++-
 2 files changed, 10 insertions(+), 6 deletions(-)


(superset) 01/01: fix(mcp): return a specific error per failure class on tool calls

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aminghadersohi pushed a commit to branch mcp-specific-denial-errors
in repository https://gitbox.apache.org/repos/asf/superset.git

commit c5bb07bd3e239ee73ff749dfd2126b301ff98421
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Thu Sep 17 10:50:52 2026 +0000

    fix(mcp): return a specific error per failure class on tool calls
    
    FastMCP re-raises everything a tool body throws as
    `ToolError(f"Error calling tool {name!r}: {e}") from e` before any
    middleware error hook runs. GlobalErrorHandlerMiddleware matched its
    first branch, `isinstance(error, ToolError)`, and re-raised — so its
    entire per-type branch chain was unreachable for tool failures and the
    caller got whatever `str(exc)` happened to say.
    
    An RBAC denial, a query over a dropped table, and an internal defect
    were therefore indistinguishable to an MCP client, and raw driver output
    (connection strings) and server paths bypassed sanitization entirely.
    Argument errors were unaffected: FastMCP raises those outside the tool
    body, so they were the only class still reaching the branch chain.
    
    Classify on the unwrapped cause, and add a datasource branch so a failed
    query says so instead of implying a bad tool name or arguments.
    
    Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
 superset/mcp_service/middleware.py                 | 176 ++++++++++--
 .../mcp_service/test_error_classification.py       | 312 +++++++++++++++++++++
 2 files changed, 470 insertions(+), 18 deletions(-)

diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py
index 439de294b43..379ad90b259 100644
--- a/superset/mcp_service/middleware.py
+++ b/superset/mcp_service/middleware.py
@@ -30,7 +30,7 @@ from fastmcp.server.middleware.middleware import CallNext
 from fastmcp.tools.tool import Tool, ToolResult
 from flask import g
 from pydantic import ValidationError
-from sqlalchemy.exc import OperationalError, TimeoutError
+from sqlalchemy.exc import OperationalError, SQLAlchemyError, TimeoutError
 from starlette.exceptions import HTTPException
 
 from superset.commands.exceptions import (
@@ -38,7 +38,19 @@ from superset.commands.exceptions import (
     ForbiddenError,
     ObjectNotFoundError,
 )
-from superset.exceptions import SupersetException, SupersetSecurityException
+from superset.errors import SupersetErrorType
+from superset.exceptions import (
+    ColumnNotFoundException,
+    DatabaseNotFound,
+    QueryObjectValidationError,
+    SupersetErrorsException,
+    SupersetException,
+    SupersetGenericDBErrorException,
+    SupersetParseError,
+    SupersetSecurityException,
+    SupersetTimeoutException,
+    SupersetVizException,
+)
 from superset.extensions import event_logger, stats_logger_manager
 from superset.mcp_service.auth import (
     _get_app_context_manager,
@@ -165,6 +177,105 @@ def _invoke_error_hook(error: Exception, hook_context: dict[str, Any]) -> None:
         logger.warning("MCP_ERROR_HOOK raised an exception: %s", hook_error)
 
 
+def _unwrap_tool_error(error: Exception) -> Exception:
+    """Return the original exception behind FastMCP's ``ToolError`` wrapper.
+
+    FastMCP catches every non-``FastMCPError`` raised inside a tool body and
+    re-raises it as ``ToolError(f"Error calling tool {name!r}: {e}") from e``
+    *before* any middleware error hook runs (see ``FastMCP._call_tool``). By
+    the time :class:`GlobalErrorHandlerMiddleware` sees a tool failure, the
+    concrete type — ``MCPPermissionDeniedError``, ``SupersetException``,
+    ``OperationalError`` — is no longer the exception itself, only its
+    ``__cause__``. Classifying the wrapper instead of the cause collapses
+    every distinct failure into one undifferentiated message.
+
+    A ``ToolError`` raised deliberately by tool code is re-raised by FastMCP
+    untouched and therefore carries no ``__cause__``; it is already formatted
+    for MCP and is returned as-is.
+    """
+    if isinstance(error, ToolError) and error.__cause__ is not None:
+        cause = error.__cause__
+        if isinstance(cause, Exception):
+            return cause
+    return error
+
+
+# Exception classes that mean "the query behind this tool failed", not "the
+# caller used the tool wrong". The tool name and arguments were valid; the
+# datasource, table, column, or connection it reads is broken or gone.
+_DATASOURCE_ERROR_EXCEPTIONS = (
+    ColumnNotFoundException,
+    DatabaseNotFound,
+    QueryObjectValidationError,
+    SupersetGenericDBErrorException,
+    SupersetParseError,
+    SupersetTimeoutException,
+    SupersetVizException,
+    SQLAlchemyError,
+)
+
+# ``SupersetError.error_type`` values in the DB-engine, viz, and SQL Lab
+# families. Superset raises a bare ``SupersetErrorException`` for many of
+# these, so the exception class alone is not enough to classify them.
+_DATASOURCE_ERROR_TYPES = frozenset(
+    {
+        SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR,
+        SupersetErrorType.CONNECTION_DATABASE_TIMEOUT,
+        SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
+        SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
+        SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
+        SupersetErrorType.DATABASE_NOT_FOUND_ERROR,
+        SupersetErrorType.FAILED_FETCHING_DATASOURCE_INFO_ERROR,
+        SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
+        SupersetErrorType.INVALID_SQL_ERROR,
+        SupersetErrorType.OBJECT_DOES_NOT_EXIST_ERROR,
+        SupersetErrorType.RESULTS_BACKEND_ERROR,
+        SupersetErrorType.SCHEMA_DOES_NOT_EXIST_ERROR,
+        SupersetErrorType.SQLLAB_TIMEOUT_ERROR,
+        SupersetErrorType.SYNTAX_ERROR,
+        SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
+        SupersetErrorType.TABLE_NOT_FOUND_ERROR,
+        SupersetErrorType.UNKNOWN_DATASOURCE_TYPE_ERROR,
+        SupersetErrorType.VIZ_GET_DF_ERROR,
+    }
+)
+
+
+def _datasource_error_reason(error: Exception) -> str | None:
+    """Return the enumerated reason for a datasource failure, if any.
+
+    Only ``SupersetErrorType`` members are returned — they are a closed,
+    non-sensitive vocabulary. Raw driver output (which can carry SQL, table
+    contents, or connection strings) is never surfaced from here.
+    """
+    errors = getattr(error, "errors", None)
+    single_error = getattr(error, "error", None)
+    if isinstance(error, SupersetErrorsException) and errors:
+        # SupersetErrorsException carries a list of SupersetError.
+        error_type = getattr(errors[0], "error_type", None)
+    elif single_error is not None:
+        # SupersetErrorException carries a single SupersetError.
+        error_type = getattr(single_error, "error_type", None)
+    else:
+        # Plain SupersetException exposes error_type directly.
+        error_type = getattr(error, "error_type", None)
+    if error_type not in _DATASOURCE_ERROR_TYPES:
+        return None
+    return str(getattr(error_type, "value", error_type))
+
+
+def _is_datasource_error(error: Exception) -> bool:
+    """Classify a failure as coming from the datasource behind the tool.
+
+    ``SupersetSecurityException`` subclasses ``SupersetErrorException``, so
+    callers must check for permission failures *before* calling this.
+    """
+    return (
+        isinstance(error, _DATASOURCE_ERROR_EXCEPTIONS)
+        or _datasource_error_reason(error) is not None
+    )
+
+
 # Errors caused by the LLM/user — expected in normal MCP operation.
 # Agents send bad params, try tools they lack access to, request nonexistent
 # resources. These are 400-class errors and should be logged at WARNING.
@@ -625,15 +736,11 @@ class LoggingMiddleware(Middleware):
                 )
             return result
         except Exception as exc:
-            # GlobalErrorHandlerMiddleware (inner) wraps tool exceptions in
-            # ToolError with the original attached as __cause__; unwrap it so
-            # error_type and the user/system classification reflect the real
-            # failure rather than the ToolError wrapper.
-            original = (
-                exc.__cause__
-                if isinstance(exc, ToolError) and exc.__cause__ is not None
-                else exc
-            )
+            # Tool exceptions arrive wrapped in ToolError with the original
+            # attached as __cause__; unwrap it so error_type and the
+            # user/system classification reflect the real failure rather than
+            # the ToolError wrapper.
+            original = _unwrap_tool_error(exc)
             error_type = type(original).__name__
             raised_is_user_error = _is_user_error(original)
             success = False
@@ -954,12 +1061,23 @@ class GlobalErrorHandlerMiddleware(Middleware):
 
     async def _handle_error(  # noqa: C901
         self,
-        error: Exception,
+        wrapped_error: Exception,
         context: MiddlewareContext,
         tool_name: str,
         duration_ms: int,
     ) -> None:
-        """Handle different types of errors with appropriate responses"""
+        """Handle different types of errors with appropriate responses.
+
+        ``wrapped_error`` is what reached the middleware; ``error`` is the
+        real failure. FastMCP re-raises everything a tool body throws as
+        ``ToolError(...) from e`` before any middleware runs, so classifying
+        the exception as received would funnel an RBAC denial, a dead table,
+        and an internal bug into the same message. Every decision below —
+        log severity, error-tracker capture, and the client-facing text — is
+        therefore made on the unwrapped cause. See :func:`_unwrap_tool_error`.
+        """
+        error = _unwrap_tool_error(wrapped_error)
+
         # Extract user context for logging
         user_id = None
         try:
@@ -1027,7 +1145,8 @@ class GlobalErrorHandlerMiddleware(Middleware):
 
         # Handle specific error types with appropriate responses
         if isinstance(error, ToolError):
-            # Tool errors are already formatted for MCP
+            # A ToolError that survived _unwrap_tool_error was raised
+            # deliberately by tool code and is already formatted for MCP.
             raise error
         elif isinstance(error, ValidationError):
             # Pydantic validation errors
@@ -1056,12 +1175,19 @@ class GlobalErrorHandlerMiddleware(Middleware):
                 f"Service error in {tool_name}: {_sanitize_error_for_logging(error)}"
             ) 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.
+            # MCP RBAC permission denied. Rendered from the exception's own
+            # structured fields ("Permission denied: <permission> on
+            # <resource>") rather than through _sanitize_error_for_logging,
+            # which flattens every PermissionError to "Access denied" and
+            # would throw away the two facts that make the denial
+            # actionable. Must come before the generic PermissionError
+            # branch because MCPPermissionDeniedError inherits from it.
             raise ToolError(str(error)) from error
         elif isinstance(error, PermissionError):
-            # Permission/authorization errors
+            # Authorization failures that are not raised by the MCP RBAC
+            # decorator still get the "Permission denied" shape, so callers
+            # can tell a denial from a malformed call no matter which layer
+            # refused them.
             raise ToolError(
                 f"Permission denied for {tool_name}: "
                 f"You don't have access to this resource."
@@ -1083,6 +1209,20 @@ class GlobalErrorHandlerMiddleware(Middleware):
                 f"Permission denied for {tool_name}: "
                 f"{_sanitize_error_for_logging(error)}"
             ) from error
+        elif _is_datasource_error(error):
+            # The query behind the tool failed. The tool name and arguments
+            # were fine, so the caller must NOT be told to re-check the tool
+            # schema — that advice fits an argument error and sends them
+            # down the wrong path for a dropped table or a dead connection.
+            # Only the enumerated SupersetErrorType is echoed; raw driver
+            # output could carry SQL or connection details.
+            reason = _datasource_error_reason(error) or type(error).__name__
+            raise ToolError(
+                f"Datasource error in {tool_name}: the query against the "
+                f"underlying datasource failed ({reason}). The tool name and "
+                f"arguments were valid — the datasource, table, or column it "
+                f"reads may be missing, renamed, or unreachable."
+            ) from error
         elif isinstance(error, SupersetException):
             # Other Superset errors — .status determines severity (already
             # classified by _is_user_error above for log level)
diff --git a/tests/unit_tests/mcp_service/test_error_classification.py b/tests/unit_tests/mcp_service/test_error_classification.py
new file mode 100644
index 00000000000..cfb2eb9fc68
--- /dev/null
+++ b/tests/unit_tests/mcp_service/test_error_classification.py
@@ -0,0 +1,312 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Per-failure-class error classification for MCP tool calls.
+
+Three unrelated failures used to reach MCP clients as one undifferentiated
+message, because FastMCP re-raises everything a tool body throws as
+``ToolError(f"Error calling tool {name!r}: {e}") from e`` *before* any
+middleware error hook runs. ``GlobalErrorHandlerMiddleware`` then matched its
+first branch, ``isinstance(error, ToolError)``, and re-raised — so its entire
+per-type branch chain was unreachable for tool failures, and the client got
+whatever ``str(exc)`` happened to say.
+
+The rest of the suite exercises that chain by injecting *raw* exceptions into
+``on_message``, which is why the gap went unnoticed: no test drove a tool
+exception through FastMCP's wrapping first. These tests do, end to end.
+"""
+
+from typing import Any
+
+import pytest
+from fastmcp import FastMCP
+from fastmcp.client import Client
+from fastmcp.exceptions import ToolError
+
+from superset.commands.exceptions import ForbiddenError
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import (
+    SupersetErrorException,
+    SupersetException,
+    SupersetSecurityException,
+)
+from superset.mcp_service.auth import MCPPermissionDeniedError
+from superset.mcp_service.middleware import (
+    _datasource_error_reason,
+    _is_datasource_error,
+    _unwrap_tool_error,
+    GlobalErrorHandlerMiddleware,
+    ToolResultCompatibilityMiddleware,
+)
+
+# A connection string and an internal path, planted in the exception messages
+# below to prove neither reaches the caller.
+LEAKED_DSN = "postgresql://admin:hunter2@db.internal/prod"
+LEAKED_PATH = "/srv/app/superset/mcp_service/chart/tool/get_chart_data.py"
+
+
+def _dead_table_error() -> SupersetErrorException:
+    """The upstream failure a chart over a dropped table produces."""
+    return SupersetErrorException(
+        SupersetError(
+            message=f'relation "orders" does not exist ({LEAKED_DSN})',
+            error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
+            level=ErrorLevel.ERROR,
+        )
+    )
+
+
+def _build_server() -> FastMCP:
+    """A FastMCP server with one tool per failure class.
+
+    ``LoggingMiddleware`` is deliberately omitted: it needs a Flask app
+    context, and it does not participate in classifying the response.
+    """
+    mcp: FastMCP = FastMCP("error-classification")
+
+    @mcp.tool
+    def rbac_denied(unused: int = 1) -> str:
+        """RBAC denial raised by the @tool permission decorator."""
+        raise MCPPermissionDeniedError(
+            permission_name="can_get",
+            view_name="User",
+            tool_name="rbac_denied",
+        )
+
+    @mcp.tool
+    def forbidden(unused: int = 1) -> str:
+        """Authorization refused below the MCP layer."""
+        raise ForbiddenError()
+
+    @mcp.tool
+    def dead_table(unused: int = 1) -> str:
+        """Well-formed call whose datasource is gone."""
+        raise _dead_table_error()
+
+    @mcp.tool
+    def needs_id(id: int) -> str:  # noqa: A002
+        """Schema mismatch: callers guess ``identifier`` for ``id``."""
+        return "ok"
+
+    @mcp.tool
+    def internal_bug(unused: int = 1) -> str:
+        """A genuine server-side defect."""
+        raise RuntimeError(f"unexpected failure in {LEAKED_PATH}")
+
+    mcp.add_middleware(
+        ToolResultCompatibilityMiddleware(structured_output_enabled=False)
+    )
+    mcp.add_middleware(GlobalErrorHandlerMiddleware())
+    return mcp
+
+
+async def _call(tool: str, arguments: dict[str, Any] | None = None) -> str:
+    """Return the client-facing text for a failing tool call."""
+    async with Client(_build_server()) as client:
+        result = await client.call_tool(tool, arguments or {}, raise_on_error=False)
+    assert result.is_error, f"{tool} was expected to fail"
+    return result.content[0].text
+
+
+class TestFastMCPWrappingPrecondition:
+    """Guard the assumption the classification fix is built on."""
+
+    @pytest.mark.asyncio
+    async def test_tool_exceptions_arrive_wrapped_in_tool_error(self) -> None:
+        """FastMCP wraps tool exceptions before middleware sees them.
+
+        If a FastMCP upgrade stops doing this, ``_unwrap_tool_error`` becomes
+        a no-op and the branch chain keeps working — but this test failing is
+        the signal that the comments explaining *why* it exists are stale.
+        """
+        seen: list[Exception] = []
+
+        mcp: FastMCP = FastMCP("precondition")
+
+        @mcp.tool
+        def boom(unused: int = 1) -> str:
+            raise PermissionError("denied")
+
+        class Capture(ToolResultCompatibilityMiddleware):
+            async def on_call_tool(self, context: Any, call_next: Any) -> Any:
+                try:
+                    return await call_next(context)
+                except Exception as exc:  # noqa: BLE001
+                    seen.append(exc)
+                    raise
+
+        mcp.add_middleware(Capture(structured_output_enabled=False))
+
+        async with Client(mcp) as client:
+            await client.call_tool("boom", {}, raise_on_error=False)
+
+        assert len(seen) == 1
+        assert isinstance(seen[0], ToolError)
+        assert isinstance(seen[0].__cause__, PermissionError)
+
+
+class TestUnwrapToolError:
+    """``_unwrap_tool_error`` must recover the cause without eating
+    deliberately raised ToolErrors."""
+
+    def test_returns_cause_of_wrapped_error(self) -> None:
+        cause = MCPPermissionDeniedError(permission_name="can_read", view_name="Chart")
+        wrapped = ToolError("Error calling tool 'list_charts': denied")
+        wrapped.__cause__ = cause
+
+        assert _unwrap_tool_error(wrapped) is cause
+
+    def test_passes_through_deliberate_tool_error(self) -> None:
+        """A ToolError raised by tool code has no cause and is already
+        formatted for MCP, so it must survive untouched."""
+        deliberate = ToolError("'search_tools' cannot be called via the proxy")
+
+        assert _unwrap_tool_error(deliberate) is deliberate
+
+    def test_passes_through_unwrapped_exception(self) -> None:
+        error = ValueError("page must be positive")
+
+        assert _unwrap_tool_error(error) is error
+
+
+class TestDatasourceErrorClassification:
+    """Only genuine query/datasource failures take the datasource branch."""
+
+    def test_classifies_missing_table(self) -> None:
+        assert _is_datasource_error(_dead_table_error()) is True
+
+    def test_reason_is_the_enumerated_error_type(self) -> None:
+        assert (
+            _datasource_error_reason(_dead_table_error())
+            == SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR.value
+        )
+
+    def test_security_exception_is_not_a_datasource_error(self) -> None:
+        """SupersetSecurityException subclasses SupersetErrorException, so the
+        permission branch must win — a denied table is not a broken table."""
+        error = SupersetSecurityException(
+            SupersetError(
+                message="denied",
+                error_type=SupersetErrorType.TABLE_SECURITY_ACCESS_ERROR,
+                level=ErrorLevel.ERROR,
+            )
+        )
+
+        assert _is_datasource_error(error) is False
+
+    def test_unrelated_superset_exception_is_not_a_datasource_error(self) -> None:
+        assert _is_datasource_error(SupersetException("something else")) is False
+
+    def test_reason_is_none_for_unlisted_error_type(self) -> None:
+        error = SupersetErrorException(
+            SupersetError(
+                message="nope",
+                error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+                level=ErrorLevel.ERROR,
+            )
+        )
+
+        assert _datasource_error_reason(error) is None
+
+
+class TestPerClassClientFacingErrors:
+    """Each failure class reaches the caller as its own honest message."""
+
+    @pytest.mark.asyncio
+    async def test_rbac_denial_uses_permission_denied_shape(self) -> None:
+        """An RBAC denial names the permission and the resource, and never
+        tells the caller to go re-read the tool schema."""
+        message = await _call("rbac_denied")
+
+        assert "Permission denied: can_get on User" in message
+        assert "Validation error" not in message
+        assert "inputSchema" not in message
+        assert "Internal error" not in message
+
+    @pytest.mark.asyncio
+    async def test_non_rbac_authorization_failure_also_says_permission_denied(
+        self,
+    ) -> None:
+        """Denials raised below the MCP RBAC decorator get the same shape, so
+        a caller can recognise a denial regardless of which layer refused."""
+        message = await _call("forbidden")
+
+        assert "Permission denied" in message
+
+    @pytest.mark.asyncio
+    async def test_schema_mismatch_keeps_validation_guidance(self) -> None:
+        """Wrong argument names still get the advice that actually fits:
+        the offending fields, so the caller can match the inputSchema."""
+        message = await _call("needs_id", {"identifier": 5})
+
+        assert "Validation error in needs_id" in message
+        assert "id" in message
+        assert "identifier" in message
+        assert "Permission denied" not in message
+
+    @pytest.mark.asyncio
+    async def test_datasource_failure_blames_the_query_not_the_caller(self) -> None:
+        """A dropped table says the datasource failed and states the call was
+        valid — the previous message implied a bad tool name or arguments."""
+        message = await _call("dead_table")
+
+        assert "Datasource error in dead_table" in message
+        assert SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR.value in message
+        assert "arguments were valid" in message
+        assert "Permission denied" not in message
+        assert "Validation error" not in message
+
+    @pytest.mark.asyncio
+    async def test_internal_error_is_opaque_and_carries_an_error_id(self) -> None:
+        message = await _call("internal_bug")
+
+        assert "Internal error in internal_bug" in message
+        assert "Error ID:" in message
+
+    @pytest.mark.asyncio
+    async def test_the_four_classes_do_not_share_a_message(self) -> None:
+        """The regression in one assertion: conflating these is the bug."""
+        messages = [
+            await _call("rbac_denied"),
+            await _call("needs_id", {"identifier": 5}),
+            await _call("dead_table"),
+            await _call("internal_bug"),
+        ]
+
+        assert len(set(messages)) == len(messages)
+
+
+class TestNoNewDisclosure:
+    """Classification must not widen what a failure discloses."""
+
+    @pytest.mark.asyncio
+    async def test_datasource_error_does_not_echo_raw_driver_output(self) -> None:
+        """Only the enumerated SupersetErrorType is surfaced; the driver
+        message (which carried a DSN) is not."""
+        message = await _call("dead_table")
+
+        assert LEAKED_DSN not in message
+        assert "hunter2" not in message
+        assert "db.internal" not in message
+        assert "orders" not in message
+
+    @pytest.mark.asyncio
+    async def test_internal_error_does_not_echo_server_paths(self) -> None:
+        message = await _call("internal_bug")
+
+        assert LEAKED_PATH not in message
+        assert "mcp_service" not in message