fix(auth): filter tools/list against the normalized server name

The tools/list response filter was given the proxy path (e.g. "myserver/mcp")
while the access check earlier in the same request used the registered name
("myserver"), stripped via _registered_server_from_proxy_path in
_authorize_forwarded_mcp_body. A scope document keyed on the registered name
matched nothing under the suffixed key, so every tool was removed.

The failure mode is worse than the bug. The client gets a valid JSON-RPC
response with an empty tools array: not an error, so nothing surfaces. The
connector looks healthy, the server is listed, and it has no tools. The only
signal was the filter's own before/after counts in the auth-server log.

Strip the transport suffix at the call site, matching what the access check in
the same request already did, and document the contract on
filter_tools_list_response so a future caller holding a proxy path does not
reintroduce it.

Two diagnostics, because the reporter is right that the failure mode is worse
than the defect:

  - Log which scope entry matched. before/after counts alone cannot tell a
    correct filter from a scope-key mismatch; scopes_matched can.
  - When a server has no server_access entry in ANY of the caller's scopes and
    all tools were dropped, log ERROR with remediation. That is a configuration
    error, not an empty allowlist, and the two are indistinguishable in the
    response.

The empty result still stays empty: authorization continues to fail closed, and
the response shape is unchanged, so no client sees different behaviour. Only the
log output differs. The diagnostic uses get_server_scopes_bulk, so it costs one
round-trip regardless of scope count, against the N*M per-tool lookups the
filter already issues.

The diagnostic is fully guarded: filter_tools_list_response documents "never
raises", so a scope-store failure or an unexpected shape must not turn a
successful tools/list into a 500. A test pins that.

_patch_scope_repo_allow_all now also stubs get_server_scopes_bulk. Without it
the diagnostic would receive a bare AsyncMock and the shared helper would let
tests pass for the wrong reason.

Fixes #1647

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: Evolver-sweden

auth_server/server.py

@@ -1868,6 +1868,56 @@ async def validate_server_tool_access(
         return False  # Deny access on error
 
 
+async def _scopes_with_server_entry(
+    server_name: str,
+    user_scopes: list[str],
+) -> list[str]:
+    """Return the caller's scopes that carry a ``server_access`` entry for a server.
+
+    Diagnostics only -- this never affects an authorization decision. It answers
+    "did the scope lookup find anything at all for this server", which is what
+    separates a configuration error (no entry in any scope, so every tool is
+    filtered out) from a correctly empty allowlist (an entry exists but grants no
+    tools). Those two are indistinguishable in the response: both produce a valid
+    JSON-RPC result with an empty tools array.
+
+    Uses the bulk lookup, so this costs one round-trip regardless of how many
+    scopes the caller has. The per-tool checks in the caller already issue one
+    lookup per scope per tool, so this is negligible next to them.
+
+    Args:
+        server_name: Registered server name (the scope key, no transport suffix).
+        user_scopes: Scopes resolved for the caller.
+
+    Returns:
+        The matching scope names, in the order the caller's scopes were given.
+        Empty when no scope carries an entry for this server. Never raises.
+    """
+    matched: list[str] = []
+    # The whole body is guarded: the caller documents "never raises", and this
+    # runs purely for a log line, so a lookup or shape surprise must never turn
+    # a successful tools/list into a 500.
+    try:
+        scope_repo = get_scope_repository()
+        scope_rules = await scope_repo.get_server_scopes_bulk(user_scopes)
+        if not isinstance(scope_rules, dict):
+            return []
+        for scope in user_scopes:
+            entries = scope_rules.get(scope)
+            if not isinstance(entries, list):
+                continue
+            for server_config in entries:
+                if not isinstance(server_config, dict):
+                    continue
+                if _server_names_match(server_config.get("server"), server_name):
+                    matched.append(scope)
+                    break
+    except Exception as exc:
+        logger.debug(f"_scopes_with_server_entry: scope lookup failed: {exc}")
+        return []
+    return matched
+
+
 async def filter_tools_list_response(
     server_name: str,
     user_scopes: list[str],
@@ -1884,7 +1934,11 @@ async def filter_tools_list_response(
     tools: ["*"] / ["all"]).
 
     Args:
-        server_name: Name of the MCP server whose tools/list is being filtered.
+        server_name: The REGISTERED server name, i.e. the scope key, with no
+            transport suffix. Callers holding a proxy path (``myserver/mcp``)
+            must run it through _registered_server_from_proxy_path first --
+            passing the suffixed form silently matches no scope entry and
+            filters every tool out (issue #1647).
         user_scopes: Scopes resolved for the caller.
         tools_list: The raw tools array from the upstream JSON-RPC result.
 
@@ -1924,10 +1978,32 @@ async def filter_tools_list_response(
             kept.append(tool)
 
     after_count = len(kept)
+
+    # Which scope entry actually matched. Without this the before/after counts
+    # alone cannot tell a correct filter from a scope-key mismatch (issue #1647).
+    matched_scopes = await _scopes_with_server_entry(server_name, user_scopes)
     logger.info(
         f"filter_tools_list_response: server={server_name} "
+        f"scopes_matched={matched_scopes} "
         f"before={before_count} after={after_count}"
     )
+
+    # An empty result stays empty -- authorization always fails closed. But
+    # "no scope entry exists for this server" is a configuration error, not an
+    # empty allowlist, and the two are indistinguishable to the client: both
+    # yield a valid JSON-RPC result with zero tools, so the connector looks
+    # healthy and nothing surfaces to the user. Say so loudly instead.
+    if before_count and not after_count and not matched_scopes:
+        logger.error(
+            f"filter_tools_list_response: server={server_name} has no server_access entry "
+            f"in any of the caller's {len(user_scopes)} scope(s), so all {before_count} "
+            f"upstream tools were filtered out. The client receives a healthy-looking "
+            f"response with zero tools. This is a configuration error, not an empty "
+            f"allowlist: add a server_access entry for '{server_name}' to the caller's "
+            f"scope document, and check that it uses the registered server name without "
+            f"a transport suffix (e.g. 'myserver', not 'myserver/mcp')."
+        )
+
     return kept
 
 
@@ -7359,8 +7435,14 @@ async def mcp_proxy(
 
     result = parsed.get("result") if isinstance(parsed, dict) else None
     if isinstance(result, dict) and isinstance(result.get("tools"), list):
+        # server_name here is the proxy path (e.g. "myserver/mcp"). The scope
+        # allowlist is keyed on the registered name, and the access check
+        # earlier in this same request already stripped the transport suffix
+        # via _authorize_forwarded_mcp_body. Strip it identically, or the
+        # filter looks up a key that does not exist and drops every tool
+        # (issue #1647).
         filtered = await filter_tools_list_response(
-            server_name,
+            _registered_server_from_proxy_path(server_name),
             user_scopes,
             result["tools"],
         )

tests/auth_server/unit/test_server.py

@@ -3575,7 +3575,13 @@ async def _get_server_scopes(scope_name: str):
             return [{"server": "*", "methods": ["*"], "tools": ["*"]}]
         return []
 
+    async def _get_server_scopes_bulk(scope_names: list[str]):
+        return {s: await _get_server_scopes(s) for s in scope_names if await _get_server_scopes(s)}
+
     repo.get_server_scopes.side_effect = _get_server_scopes
+    # filter_tools_list_response's diagnostic (_scopes_with_server_entry) reads
+    # the bulk method; without a stub it would get a bare AsyncMock return value.
+    repo.get_server_scopes_bulk.side_effect = _get_server_scopes_bulk
     return patch("auth_server.server.get_scope_repository", return_value=repo)
 
 
@@ -5991,3 +5997,237 @@ async def _boom(token, server):
 
         assert response.status_code == 403
         assert vend_called["n"] == 0  # denial happened before any vend/outbound work
+
+
+# =============================================================================
+# TOOLS/LIST FILTER SCOPE-KEY NORMALIZATION (issue #1647)
+# =============================================================================
+
+
+def _patch_scope_repo(rules_by_scope: dict[str, list[dict]]):
+    """Patch get_scope_repository with an explicit scope -> server_access map.
+
+    Stubs both the per-scope and the bulk lookup, because
+    validate_server_tool_access uses the former and the filter's diagnostic uses
+    the latter; stubbing only one lets a test pass for the wrong reason.
+    """
+    repo = AsyncMock()
+
+    async def _get_server_scopes(scope_name: str):
+        return rules_by_scope.get(scope_name, [])
+
+    async def _get_server_scopes_bulk(scope_names: list[str]):
+        return {s: rules_by_scope[s] for s in scope_names if rules_by_scope.get(s)}
+
+    repo.get_server_scopes.side_effect = _get_server_scopes
+    repo.get_server_scopes_bulk.side_effect = _get_server_scopes_bulk
+    return patch("auth_server.server.get_scope_repository", return_value=repo)
+
+
+class TestToolsListFilterScopeKey:
+    """The tools/list filter must get the registered name, not the proxy path.
+
+    The access check earlier in the same request strips the transport suffix via
+    _registered_server_from_proxy_path, but the filter call site did not. A scope
+    document keyed on the registered name then matched nothing, so every tool was
+    removed and the client got a valid JSON-RPC response with an empty tools
+    array: no error, connector looks healthy, server listed with no tools.
+    """
+
+    @pytest.mark.parametrize(
+        ("proxy_path", "expected_scope_key"),
+        [
+            ("office-docs/mcp", "office-docs"),
+            ("office-docs/sse", "office-docs"),
+            ("office-docs/messages", "office-docs"),
+            ("office-docs", "office-docs"),
+        ],
+    )
+    def test_filter_receives_normalized_server_name(self, proxy_path, expected_scope_key):
+        """The suffixed proxy path must be stripped before the filter sees it."""
+        import auth_server.server as server_module
+
+        seen: dict[str, str] = {}
+
+        async def _capture(server_name, user_scopes, tools):
+            seen["server_name"] = server_name
+            return tools
+
+        upstream_resp = _build_mock_upstream_response(
+            status_code=200,
+            headers={"content-type": "application/json"},
+            body=b'{"jsonrpc":"2.0","id":1,"result":{"tools":[{"name":"t1"}]}}',
+        )
+
+        with (
+            _patch_httpx_async_client(upstream_resp),
+            _patch_scope_repo_allow_all(),
+            patch.object(server_module, "_read_mcp_filter_enabled", return_value=True),
+            patch.object(server_module, "filter_tools_list_response", side_effect=_capture),
+        ):
+            client = TestClient(server_module.app)
+            response = client.post(
+                f"/mcp-proxy/{proxy_path}",
+                json={"jsonrpc": "2.0", "id": 1, "method": "tools/list"},
+                headers=_mcp_proxy_token_headers(server_name="office-docs"),
+            )
+
+        assert response.status_code == 200
+        assert seen["server_name"] == expected_scope_key
+
+    def test_federated_peer_server_key_is_preserved(self):
+        """Only the transport tail is stripped; peer/server must survive intact."""
+        import auth_server.server as server_module
+
+        seen: dict[str, str] = {}
+
+        async def _capture(server_name, user_scopes, tools):
+            seen["server_name"] = server_name
+            return tools
+
+        upstream_resp = _build_mock_upstream_response(
+            status_code=200,
+            headers={"content-type": "application/json"},
+            body=b'{"jsonrpc":"2.0","id":1,"result":{"tools":[{"name":"t1"}]}}',
+        )
+
+        with (
+            _patch_httpx_async_client(upstream_resp),
+            _patch_scope_repo_allow_all(),
+            patch.object(server_module, "_read_mcp_filter_enabled", return_value=True),
+            patch.object(server_module, "filter_tools_list_response", side_effect=_capture),
+        ):
+            client = TestClient(server_module.app)
+            response = client.post(
+                "/mcp-proxy/peer-lob-1/cloudflare-docs/mcp",
+                json={"jsonrpc": "2.0", "id": 1, "method": "tools/list"},
+                headers=_mcp_proxy_token_headers(server_name="peer-lob-1/cloudflare-docs"),
+            )
+
+        assert response.status_code == 200
+        assert seen["server_name"] == "peer-lob-1/cloudflare-docs"
+
+    def test_suffixed_key_would_have_filtered_everything(self):
+        """Pin the failure mode itself, so the bug cannot come back unnoticed.
+
+        A scope document keyed on the registered name grants nothing when looked
+        up under the suffixed path. This asserts the mechanism the fix avoids,
+        independently of which call site does the stripping.
+        """
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        rules = {"grp": [{"server": "office-docs", "methods": ["*"], "tools": ["t1", "t2"]}]}
+        tools = [{"name": "t1"}, {"name": "t2"}]
+
+        with _patch_scope_repo(rules):
+            normalized = asyncio.run(filter_tools_list_response("office-docs", ["grp"], tools))
+            suffixed = asyncio.run(filter_tools_list_response("office-docs/mcp", ["grp"], tools))
+
+        assert len(normalized) == 2
+        assert suffixed == []
+
+
+class TestToolsListFilterDiagnostics:
+    """A silently empty tools array must be distinguishable from a real denial.
+
+    The filter fails closed either way -- that is not negotiable -- but "no
+    server_access entry exists for this server" is a configuration error, while
+    "an entry exists and grants no tools" is a correct empty allowlist. They look
+    identical to the client, so the logs have to separate them.
+    """
+
+    def test_missing_scope_entry_logs_error(self, caplog):
+        """No entry anywhere for the server: log ERROR naming the server."""
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        rules = {"grp": [{"server": "some-other-server", "methods": ["*"], "tools": ["*"]}]}
+        tools = [{"name": "t1"}, {"name": "t2"}]
+
+        with _patch_scope_repo(rules), caplog.at_level(logging.ERROR, logger="auth_server.server"):
+            kept = asyncio.run(filter_tools_list_response("office-docs", ["grp"], tools))
+
+        assert kept == []  # still fails closed
+        errors = [r.message for r in caplog.records if r.levelno == logging.ERROR]
+        assert any("office-docs" in m and "no server_access entry" in m for m in errors), errors
+
+    def test_empty_allowlist_does_not_log_error(self, caplog):
+        """An entry that grants no tools is legitimate, so no ERROR."""
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        rules = {"grp": [{"server": "office-docs", "methods": ["tools/list"], "tools": []}]}
+        tools = [{"name": "t1"}]
+
+        with _patch_scope_repo(rules), caplog.at_level(logging.ERROR, logger="auth_server.server"):
+            kept = asyncio.run(filter_tools_list_response("office-docs", ["grp"], tools))
+
+        assert kept == []
+        assert not [r for r in caplog.records if r.levelno == logging.ERROR]
+
+    def test_no_error_when_tools_survive(self, caplog):
+        """A successful filter must not log an ERROR."""
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        rules = {"grp": [{"server": "office-docs", "methods": ["*"], "tools": ["t1"]}]}
+        tools = [{"name": "t1"}, {"name": "t2"}]
+
+        with _patch_scope_repo(rules), caplog.at_level(logging.INFO, logger="auth_server.server"):
+            kept = asyncio.run(filter_tools_list_response("office-docs", ["grp"], tools))
+
+        assert [t["name"] for t in kept] == ["t1"]
+        assert not [r for r in caplog.records if r.levelno >= logging.ERROR]
+
+    def test_info_line_names_the_matching_scope(self, caplog):
+        """before/after counts alone cannot diagnose a scope-key mismatch.
+
+        Logging which scope entry matched is what turns this class of bug from a
+        multi-day hunt into a single grep.
+        """
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        rules = {
+            "grp-a": [{"server": "office-docs", "methods": ["*"], "tools": ["t1"]}],
+            "grp-b": [{"server": "some-other-server", "methods": ["*"], "tools": ["*"]}],
+        }
+        tools = [{"name": "t1"}]
+
+        with (
+            _patch_scope_repo(rules),
+            caplog.at_level(logging.INFO, logger="auth_server.server"),
+        ):
+            asyncio.run(filter_tools_list_response("office-docs", ["grp-a", "grp-b"], tools))
+
+        lines = [r.message for r in caplog.records if "filter_tools_list_response:" in r.message]
+        assert any("scopes_matched=['grp-a']" in m for m in lines), lines
+
+    def test_diagnostic_never_breaks_the_filter(self):
+        """A broken scope repository must not turn tools/list into a 500.
+
+        The filter documents "never raises"; the diagnostic is only a log line,
+        so a lookup failure or an unexpected shape has to stay swallowed.
+        """
+        import asyncio
+
+        from auth_server.server import filter_tools_list_response
+
+        repo = AsyncMock()
+
+        async def _get_server_scopes(scope_name: str):
+            return [{"server": "office-docs", "methods": ["*"], "tools": ["*"]}]
+
+        repo.get_server_scopes.side_effect = _get_server_scopes
+        repo.get_server_scopes_bulk.side_effect = RuntimeError("scope store unreachable")
+
+        with patch("auth_server.server.get_scope_repository", return_value=repo):
+            kept = asyncio.run(filter_tools_list_response("office-docs", ["grp"], [{"name": "t1"}]))
+
+        assert [t["name"] for t in kept] == ["t1"]