From f34bbcc250a7130dae86750157da3712828fbdf7 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Fri, 6 Mar 2026 17:34:03 +0000 Subject: [PATCH] Truncate untrusted peer-controlled values before logging/raising Follow-up to 3041fd0b which truncated a client-controlled session ID. A sweep of the codebase found ~30 more locations where values controlled by the other side of the connection are interpolated unbounded into log messages, exception messages, or HTTP error responses. Server-side (client-controlled input): - sse.py: session_id query param (direct analog of 3041fd0b) - transport_security.py: Host/Origin headers at WARNING - mcpserver: tool names, prompt names, resource URIs in 'Unknown X' errors and logger.exception calls - streamable_http.py: mcp-protocol-version header echoed in 400 body - auth handlers: client_id, scope, redirect_uri echoed in error responses - task handlers: task_id, pagination cursor in error messages Client-side (server-controlled input): - streamable_http.py: mcp-session-id response header, content-type header, raw InitializeResult dict - session.py: protocol_version from InitializeResult - session_group.py: MCPError str (= server's error.message verbatim) - sse.py/streamable_http.py: SSE event names Protocol-level (shared/session.py): - error.message from null-ID JSONRPCError - Response ID in 'cannot be normalized' warning - Unknown-request-ID error: was logging entire SessionMessage repr (full wire payload), now logs just the truncated ID - Dropped full JSONRPCNotification repr from validation-failure warning Also removed three debug logs in server/sse.py that stringified the full request body / parsed message on every POST via eager f-string evaluation. Truncation lengths: 32 for version strings, 64 for IDs/tokens, 128 for names/headers, 256 for URIs/messages, 512 for result dicts. --- src/mcp/client/session.py | 2 +- src/mcp/client/session_group.py | 6 +++--- src/mcp/client/sse.py | 2 +- src/mcp/client/streamable_http.py | 14 ++++++++------ src/mcp/server/auth/handlers/authorize.py | 2 +- src/mcp/server/auth/handlers/register.py | 2 +- src/mcp/server/auth/handlers/token.py | 4 +++- src/mcp/server/experimental/task_result_handler.py | 2 +- src/mcp/server/lowlevel/experimental.py | 2 +- src/mcp/server/mcpserver/prompts/manager.py | 2 +- .../server/mcpserver/resources/resource_manager.py | 2 +- src/mcp/server/mcpserver/server.py | 10 +++++----- src/mcp/server/mcpserver/tools/tool_manager.py | 2 +- src/mcp/server/sse.py | 5 +---- src/mcp/server/streamable_http.py | 2 +- src/mcp/server/transport_security.py | 4 ++-- src/mcp/shared/auth.py | 4 ++-- .../experimental/tasks/in_memory_task_store.py | 2 +- src/mcp/shared/session.py | 10 ++++++---- 19 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/mcp/client/session.py b/src/mcp/client/session.py index a0ca751bd..058fb18f0 100644 --- a/src/mcp/client/session.py +++ b/src/mcp/client/session.py @@ -183,7 +183,7 @@ async def initialize(self) -> types.InitializeResult: ) if result.protocol_version not in SUPPORTED_PROTOCOL_VERSIONS: - raise RuntimeError(f"Unsupported protocol version from the server: {result.protocol_version}") + raise RuntimeError(f"Unsupported protocol version from the server: {str(result.protocol_version)[:32]}") self._server_capabilities = result.capabilities diff --git a/src/mcp/client/session_group.py b/src/mcp/client/session_group.py index 961021264..ebae8d142 100644 --- a/src/mcp/client/session_group.py +++ b/src/mcp/client/session_group.py @@ -352,7 +352,7 @@ async def _aggregate_components(self, server_info: types.Implementation, session prompts_temp[name] = prompt component_names.prompts.add(name) except MCPError as err: # pragma: no cover - logging.warning(f"Could not fetch prompts: {err}") + logging.warning(f"Could not fetch prompts: {str(err)[:256]}") # Query the server for its resources and aggregate to list. try: @@ -362,7 +362,7 @@ async def _aggregate_components(self, server_info: types.Implementation, session resources_temp[name] = resource component_names.resources.add(name) except MCPError as err: # pragma: no cover - logging.warning(f"Could not fetch resources: {err}") + logging.warning(f"Could not fetch resources: {str(err)[:256]}") # Query the server for its tools and aggregate to list. try: @@ -373,7 +373,7 @@ async def _aggregate_components(self, server_info: types.Implementation, session tool_to_session_temp[name] = session component_names.tools.add(name) except MCPError as err: # pragma: no cover - logging.warning(f"Could not fetch tools: {err}") + logging.warning(f"Could not fetch tools: {str(err)[:256]}") # Clean up exit stack for session if we couldn't retrieve anything # from the server. diff --git a/src/mcp/client/sse.py b/src/mcp/client/sse.py index 61026aa0c..009ee0e73 100644 --- a/src/mcp/client/sse.py +++ b/src/mcp/client/sse.py @@ -119,7 +119,7 @@ async def sse_reader(task_status: TaskStatus[str] = anyio.TASK_STATUS_IGNORED): session_message = SessionMessage(message) await read_stream_writer.send(session_message) case _: # pragma: no cover - logger.warning(f"Unknown SSE event: {sse.event}") # pragma: no cover + logger.warning(f"Unknown SSE event: {sse.event[:64]}") # pragma: no cover except SSEError as sse_exc: # pragma: lax no cover logger.exception("Encountered SSE exception") raise sse_exc diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index 9f3dd5e0b..7e51256a7 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -112,7 +112,7 @@ def _maybe_extract_session_id_from_response(self, response: httpx.Response) -> N new_session_id = response.headers.get(MCP_SESSION_ID) if new_session_id: self.session_id = new_session_id - logger.info(f"Received session ID: {self.session_id}") + logger.info(f"Received session ID: {new_session_id[:64]}") def _maybe_extract_protocol_version_from_message(self, message: JSONRPCMessage) -> None: """Extract protocol version from initialization response message.""" @@ -121,10 +121,10 @@ def _maybe_extract_protocol_version_from_message(self, message: JSONRPCMessage) # Parse the result as InitializeResult for type safety init_result = InitializeResult.model_validate(message.result, by_name=False) self.protocol_version = str(init_result.protocol_version) - logger.info(f"Negotiated protocol version: {self.protocol_version}") + logger.info(f"Negotiated protocol version: {self.protocol_version[:32]}") except Exception: # pragma: no cover logger.warning("Failed to parse initialization response as InitializeResult", exc_info=True) - logger.warning(f"Raw result: {message.result}") + logger.warning(f"Raw result: {str(message.result)[:512]}") async def _handle_sse_event( self, @@ -175,7 +175,7 @@ async def _handle_sse_event( await read_stream_writer.send(exc) return False else: # pragma: no cover - logger.warning(f"Unknown SSE event: {sse.event}") + logger.warning(f"Unknown SSE event: {sse.event[:64]}") return False async def handle_get_stream(self, client: httpx.AsyncClient, read_stream_writer: StreamWriter) -> None: @@ -295,8 +295,10 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: elif content_type.startswith("text/event-stream"): await self._handle_sse_response(response, ctx, is_initialization) else: - logger.error(f"Unexpected content type: {content_type}") - error_data = ErrorData(code=INVALID_REQUEST, message=f"Unexpected content type: {content_type}") + logger.error(f"Unexpected content type: {content_type[:64]}") + error_data = ErrorData( + code=INVALID_REQUEST, message=f"Unexpected content type: {content_type[:64]}" + ) error_msg = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data)) await ctx.read_stream_writer.send(error_msg) diff --git a/src/mcp/server/auth/handlers/authorize.py b/src/mcp/server/auth/handlers/authorize.py index dec6713b1..24286a32b 100644 --- a/src/mcp/server/auth/handlers/authorize.py +++ b/src/mcp/server/auth/handlers/authorize.py @@ -171,7 +171,7 @@ async def error_response( # For client_id validation errors, return direct error (no redirect) return await error_response( error="invalid_request", - error_description=f"Client ID '{auth_request.client_id}' not found", + error_description=f"Client ID '{auth_request.client_id[:128]}' not found", attempt_load_client=False, ) diff --git a/src/mcp/server/auth/handlers/register.py b/src/mcp/server/auth/handlers/register.py index 79eb0fb0c..0d8116cec 100644 --- a/src/mcp/server/auth/handlers/register.py +++ b/src/mcp/server/auth/handlers/register.py @@ -66,7 +66,7 @@ async def handle(self, request: Request) -> Response: content=RegistrationErrorResponse( error="invalid_client_metadata", error_description="Requested scopes are not valid: " - f"{', '.join(requested_scopes - valid_scopes)}", + f"{', '.join(requested_scopes - valid_scopes)[:256]}", ), status_code=400, ) diff --git a/src/mcp/server/auth/handlers/token.py b/src/mcp/server/auth/handlers/token.py index 534a478a9..572a3da0a 100644 --- a/src/mcp/server/auth/handlers/token.py +++ b/src/mcp/server/auth/handlers/token.py @@ -206,7 +206,9 @@ async def handle(self, request: Request): return self.response( TokenErrorResponse( error="invalid_scope", - error_description=(f"cannot request scope `{scope}` not provided by refresh token"), + error_description=( + f"cannot request scope `{scope[:128]}` not provided by refresh token" + ), ) ) diff --git a/src/mcp/server/experimental/task_result_handler.py b/src/mcp/server/experimental/task_result_handler.py index b2268bc1c..cd5f39203 100644 --- a/src/mcp/server/experimental/task_result_handler.py +++ b/src/mcp/server/experimental/task_result_handler.py @@ -103,7 +103,7 @@ async def handle( while True: task = await self._store.get_task(task_id) if task is None: - raise MCPError(code=INVALID_PARAMS, message=f"Task not found: {task_id}") + raise MCPError(code=INVALID_PARAMS, message=f"Task not found: {task_id[:64]}") await self._deliver_queued_messages(task_id, session, request_id) diff --git a/src/mcp/server/lowlevel/experimental.py b/src/mcp/server/lowlevel/experimental.py index 5a907b640..420b88580 100644 --- a/src/mcp/server/lowlevel/experimental.py +++ b/src/mcp/server/lowlevel/experimental.py @@ -161,7 +161,7 @@ async def _default_get_task( ) -> GetTaskResult: task = await task_support.store.get_task(params.task_id) if task is None: - raise MCPError(code=INVALID_PARAMS, message=f"Task not found: {params.task_id}") + raise MCPError(code=INVALID_PARAMS, message=f"Task not found: {params.task_id[:64]}") return GetTaskResult( task_id=task.task_id, status=task.status, diff --git a/src/mcp/server/mcpserver/prompts/manager.py b/src/mcp/server/mcpserver/prompts/manager.py index 28a7a6e98..9e2f25f9f 100644 --- a/src/mcp/server/mcpserver/prompts/manager.py +++ b/src/mcp/server/mcpserver/prompts/manager.py @@ -54,6 +54,6 @@ async def render_prompt( """Render a prompt by name with arguments.""" prompt = self.get_prompt(name) if not prompt: - raise ValueError(f"Unknown prompt: {name}") + raise ValueError(f"Unknown prompt: {name[:128]}") return await prompt.render(arguments, context) diff --git a/src/mcp/server/mcpserver/resources/resource_manager.py b/src/mcp/server/mcpserver/resources/resource_manager.py index 6bf17376d..63dee2bf4 100644 --- a/src/mcp/server/mcpserver/resources/resource_manager.py +++ b/src/mcp/server/mcpserver/resources/resource_manager.py @@ -97,7 +97,7 @@ async def get_resource(self, uri: AnyUrl | str, context: Context[LifespanContext except Exception as e: # pragma: no cover raise ValueError(f"Error creating resource from template: {e}") - raise ValueError(f"Unknown resource: {uri}") + raise ValueError(f"Unknown resource: {str(uri)[:256]}") def list_resources(self) -> list[Resource]: """List all registered resources.""" diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 2a7a58117..7474091a0 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -439,15 +439,15 @@ async def read_resource( try: resource = await self._resource_manager.get_resource(uri, context) except ValueError: - raise ResourceError(f"Unknown resource: {uri}") + raise ResourceError(f"Unknown resource: {str(uri)[:256]}") try: content = await resource.read() return [ReadResourceContents(content=content, mime_type=resource.mime_type, meta=resource.meta)] except Exception as exc: - logger.exception(f"Error getting resource {uri}") + logger.exception(f"Error getting resource {str(uri)[:256]}") # If an exception happens when reading the resource, we should not leak the exception to the client. - raise ResourceError(f"Error reading resource {uri}") from exc + raise ResourceError(f"Error reading resource {str(uri)[:256]}") from exc def add_tool( self, @@ -1090,7 +1090,7 @@ async def get_prompt( try: prompt = self._prompt_manager.get_prompt(name) if not prompt: - raise ValueError(f"Unknown prompt: {name}") + raise ValueError(f"Unknown prompt: {name[:128]}") messages = await prompt.render(arguments, context) @@ -1099,5 +1099,5 @@ async def get_prompt( messages=pydantic_core.to_jsonable_python(messages), ) except Exception as e: - logger.exception(f"Error getting prompt {name}") + logger.exception(f"Error getting prompt {name[:128]}") raise ValueError(str(e)) diff --git a/src/mcp/server/mcpserver/tools/tool_manager.py b/src/mcp/server/mcpserver/tools/tool_manager.py index 32ed54797..d7401e29e 100644 --- a/src/mcp/server/mcpserver/tools/tool_manager.py +++ b/src/mcp/server/mcpserver/tools/tool_manager.py @@ -87,6 +87,6 @@ async def call_tool( """Call a tool by name with arguments.""" tool = self.get_tool(name) if not tool: - raise ToolError(f"Unknown tool: {name}") + raise ToolError(f"Unknown tool: {name[:128]}") return await tool.run(arguments, context, convert_result=convert_result) diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py index 9dcee67f7..1c22969ea 100644 --- a/src/mcp/server/sse.py +++ b/src/mcp/server/sse.py @@ -214,7 +214,7 @@ async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) session_id = UUID(hex=session_id_param) logger.debug(f"Parsed session ID: {session_id}") except ValueError: - logger.warning(f"Received invalid session ID: {session_id_param}") + logger.warning(f"Received invalid session ID: {session_id_param[:64]}") response = Response("Invalid session ID", status_code=400) return await response(scope, receive, send) @@ -225,11 +225,9 @@ async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) return await response(scope, receive, send) body = await request.body() - logger.debug(f"Received JSON: {body}") try: message = types.jsonrpc_message_adapter.validate_json(body, by_name=False) - logger.debug(f"Validated client message: {message}") except ValidationError as err: logger.exception("Failed to parse message") response = Response("Could not parse message", status_code=400) @@ -240,7 +238,6 @@ async def handle_post_message(self, scope: Scope, receive: Receive, send: Send) # Pass the ASGI scope for framework-agnostic access to request data metadata = ServerMessageMetadata(request_context=request) session_message = SessionMessage(message, metadata=metadata) - logger.debug(f"Sending session message to writer: {session_message}") response = Response("Accepted", status_code=202) await response(scope, receive, send) await writer.send(session_message) diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py index 04aed345e..c9bba06d3 100644 --- a/src/mcp/server/streamable_http.py +++ b/src/mcp/server/streamable_http.py @@ -849,7 +849,7 @@ async def _validate_protocol_version(self, request: Request, send: Send) -> bool if protocol_version not in SUPPORTED_PROTOCOL_VERSIONS: # pragma: no cover supported_versions = ", ".join(SUPPORTED_PROTOCOL_VERSIONS) response = self._create_error_response( - f"Bad Request: Unsupported protocol version: {protocol_version}. " + f"Bad Request: Unsupported protocol version: {protocol_version[:32]}. " + f"Supported versions: {supported_versions}", HTTPStatus.BAD_REQUEST, ) diff --git a/src/mcp/server/transport_security.py b/src/mcp/server/transport_security.py index 1ed9842c0..02e4e15c0 100644 --- a/src/mcp/server/transport_security.py +++ b/src/mcp/server/transport_security.py @@ -59,7 +59,7 @@ def _validate_host(self, host: str | None) -> bool: # pragma: no cover if host.startswith(base_host + ":"): return True - logger.warning(f"Invalid Host header: {host}") + logger.warning(f"Invalid Host header: {host[:128]}") return False def _validate_origin(self, origin: str | None) -> bool: # pragma: no cover @@ -81,7 +81,7 @@ def _validate_origin(self, origin: str | None) -> bool: # pragma: no cover if origin.startswith(base_origin + ":"): return True - logger.warning(f"Invalid Origin header: {origin}") + logger.warning(f"Invalid Origin header: {origin[:128]}") return False def _validate_content_type(self, content_type: str | None) -> bool: diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index ca5b7b45a..fc0d316b3 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -74,14 +74,14 @@ def validate_scope(self, requested_scope: str | None) -> list[str] | None: allowed_scopes = [] if self.scope is None else self.scope.split(" ") for scope in requested_scopes: if scope not in allowed_scopes: # pragma: no branch - raise InvalidScopeError(f"Client was not registered with scope {scope}") + raise InvalidScopeError(f"Client was not registered with scope {scope[:128]}") return requested_scopes # pragma: no cover def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl: if redirect_uri is not None: # Validate redirect_uri against client's registered redirect URIs if self.redirect_uris is None or redirect_uri not in self.redirect_uris: - raise InvalidRedirectUriError(f"Redirect URI '{redirect_uri}' not registered for client") + raise InvalidRedirectUriError(f"Redirect URI '{str(redirect_uri)[:256]}' not registered for client") return redirect_uri elif self.redirect_uris is not None and len(self.redirect_uris) == 1: return self.redirect_uris[0] diff --git a/src/mcp/shared/experimental/tasks/in_memory_task_store.py b/src/mcp/shared/experimental/tasks/in_memory_task_store.py index 42f4fb703..807b2ba8f 100644 --- a/src/mcp/shared/experimental/tasks/in_memory_task_store.py +++ b/src/mcp/shared/experimental/tasks/in_memory_task_store.py @@ -169,7 +169,7 @@ async def list_tasks( cursor_index = all_task_ids.index(cursor) start_index = cursor_index + 1 except ValueError: - raise ValueError(f"Invalid cursor: {cursor}") + raise ValueError(f"Invalid cursor: {cursor[:64]}") page_task_ids = all_task_ids[start_index : start_index + self._page_size] tasks = [Task(**self._tasks[tid].task.model_dump()) for tid in page_task_ids] diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index b617d702f..14222060a 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -400,7 +400,7 @@ async def _receive_loop(self) -> None: except Exception: # For other validation errors, log and continue logging.warning( # pragma: no cover - f"Failed to validate notification:. Message was: {message.message}", + "Failed to validate notification", exc_info=True, ) else: # Response or error @@ -445,7 +445,7 @@ def _normalize_request_id(self, response_id: RequestId) -> RequestId: try: return int(response_id) except ValueError: - logging.warning(f"Response ID {response_id!r} cannot be normalized to match pending requests") + logging.warning(f"Response ID {response_id[:64]!r} cannot be normalized to match pending requests") return response_id async def _handle_response(self, message: SessionMessage) -> None: @@ -464,7 +464,7 @@ async def _handle_response(self, message: SessionMessage) -> None: if message.message.id is None: # Narrows to JSONRPCError since JSONRPCResponse.id is always RequestId error = message.message.error - logging.warning(f"Received error with null ID: {error.message}") + logging.warning(f"Received error with null ID: {error.message[:256]}") await self._handle_incoming(MCPError(error.code, error.message, error.data)) return # Normalize response ID to handle type mismatches (e.g., "0" vs 0) @@ -488,7 +488,9 @@ async def _handle_response(self, message: SessionMessage) -> None: if stream: await stream.send(message.message) else: - await self._handle_incoming(RuntimeError(f"Received response with an unknown request ID: {message}")) + await self._handle_incoming( + RuntimeError(f"Received response with an unknown request ID: {str(response_id)[:64]}") + ) async def _received_request(self, responder: RequestResponder[ReceiveRequestT, SendResultT]) -> None: """Can be overridden by subclasses to handle a request without needing to