auth: read tool name off context.message directly; trace call_next failures
In FastMCP's on_call_tool hook the middleware context is already MiddlewareContext[CallToolRequestParams] (per fastmcp's own middleware.py:158), so tool name lives at context.message.name, not at context.message.params.name — the latter always returned None, silently breaking the PUBLIC_TOOLS bypass for get_health and making the per-tool ACL short-circuit. Also wrap call_next in a traced helper that logs any exception with a full traceback and logs the success-path result type. During the Pallas↔Mnemosyne shakedown the tool results were coming back to fast-agent as the literal string "object NoneType can't be used in 'await' expression" with no trace in either process — that's Python's TypeError for 'await X' where X is None. If that TypeError is raised inside FastMCP dispatch we want the frame in Mnemosyne's own log rather than having Pallas's aggregator turn it into a terse CallToolResult(isError=True) with no stack.
This commit is contained in:
@@ -252,7 +252,9 @@ class MCPAuthMiddleware(Middleware):
|
||||
)
|
||||
|
||||
if require_auth and tool_name in self._PUBLIC_TOOLS:
|
||||
return await call_next(context)
|
||||
return await self._call_next_with_trace(
|
||||
tool_name, call_next, context
|
||||
)
|
||||
|
||||
token_string = self._extract_token()
|
||||
|
||||
@@ -297,7 +299,40 @@ class MCPAuthMiddleware(Middleware):
|
||||
if claims is not None:
|
||||
await fastmcp_ctx.set_state(STATE_KEY_CLAIMS, claims)
|
||||
|
||||
return await call_next(context)
|
||||
return await self._call_next_with_trace(tool_name, call_next, context)
|
||||
|
||||
@staticmethod
|
||||
async def _call_next_with_trace(tool_name, call_next, context):
|
||||
"""Run ``call_next`` and log any exception with a full traceback.
|
||||
|
||||
During the Pallas↔Mnemosyne shakedown we were seeing tool results
|
||||
come back to fast-agent as the string ``"object NoneType can't be
|
||||
used in 'await' expression"`` with no trace anywhere in either
|
||||
process. That string is Python's ``TypeError`` for ``await X``
|
||||
where ``X`` is ``None`` (i.e. someone awaited a non-coroutine).
|
||||
If that TypeError is raised inside the FastMCP dispatch we want
|
||||
the full traceback in Mnemosyne's own log rather than silently
|
||||
letting it propagate back to Pallas where the aggregator turns
|
||||
it into a ``CallToolResult(isError=True)`` and we lose the frame.
|
||||
|
||||
We also log the *type* of the successful return so we can verify
|
||||
later that FastMCP is returning ``ToolResult`` / ``CallToolResult``
|
||||
the way we expect. Keep at INFO until green, demote to DEBUG
|
||||
afterwards.
|
||||
"""
|
||||
try:
|
||||
result = await call_next(context)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"mcp_auth.call_next_failed tool=%s", tool_name
|
||||
)
|
||||
raise
|
||||
logger.info(
|
||||
"mcp_auth.call_next_ok tool=%s result_type=%s",
|
||||
tool_name,
|
||||
type(result).__name__,
|
||||
)
|
||||
return result
|
||||
|
||||
@staticmethod
|
||||
def _extract_token() -> str | None:
|
||||
@@ -345,9 +380,27 @@ class MCPAuthMiddleware(Middleware):
|
||||
|
||||
@staticmethod
|
||||
def _extract_tool_name(context: MiddlewareContext) -> str | None:
|
||||
"""Pull the tool name off a FastMCP ``on_call_tool`` context.
|
||||
|
||||
In FastMCP middleware, ``context.message`` inside ``on_call_tool``
|
||||
is already a ``CallToolRequestParams`` (see
|
||||
``fastmcp.server.middleware.middleware.Middleware.on_call_tool``
|
||||
signature: ``MiddlewareContext[mt.CallToolRequestParams]``), so
|
||||
``name`` lives directly on ``context.message`` — there is no
|
||||
nested ``.params``. The older ``message.params.name`` access we
|
||||
had here always returned ``None``, which caused the public-tools
|
||||
bypass to silently miss ``get_health`` and made the per-tool ACL
|
||||
short-circuit. Fall back to ``.params.name`` only as a legacy
|
||||
safety net in case the shape ever diverges.
|
||||
"""
|
||||
msg = getattr(context, "message", None)
|
||||
params = getattr(msg, "params", None) if msg else None
|
||||
return getattr(params, "name", None)
|
||||
if msg is None:
|
||||
return None
|
||||
name = getattr(msg, "name", None)
|
||||
if name:
|
||||
return name
|
||||
params = getattr(msg, "params", None)
|
||||
return getattr(params, "name", None) if params is not None else None
|
||||
|
||||
|
||||
# --- Helpers ---------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user