Skip to content

Truncate untrusted peer-controlled values before logging/raising#2238

Draft
maxisbey wants to merge 1 commit intomainfrom
truncate-untrusted-log-values
Draft

Truncate untrusted peer-controlled values before logging/raising#2238
maxisbey wants to merge 1 commit intomainfrom
truncate-untrusted-log-values

Conversation

@maxisbey
Copy link
Contributor

@maxisbey maxisbey commented Mar 6, 2026

Follow-up sweep after 3041fd0 (which truncated a client-controlled session ID before logging). Found ~30 more locations where values controlled by the peer are interpolated unbounded into log messages, exception messages, or HTTP error responses.

Motivation and Context

A peer (client→server or server→client) can send an arbitrarily large value in a header, query param, JSON-RPC field, or OAuth parameter. When that value lands in a logger.warning(f"...{value}") or raise Error(f"...{value}"), it can:

  • Flood log storage / blow out log aggregation pipelines
  • Create multi-MB exception messages that propagate through handlers
  • Get echoed back in HTTP error bodies (reflection amplification)

Example: POST /messages/?session_id=<5MB of non-hex garbage>UUID() raises ValueErrorlogger.warning(f"Received invalid session ID: {session_id_param}") writes 5MB to logs.

What's changed

Server-side (client-controlled): sse.py session_id query param · transport_security.py Host/Origin headers · mcpserver tool/prompt names + resource URIs in "Unknown X" errors · streamable_http.py protocol-version header · auth handlers client_id/scope/redirect_uri · task handlers task_id/cursor

Client-side (server-controlled): streamable_http.py session-id response header, content-type, raw result dict · session.py protocol_version · session_group.py MCPError str · SSE event names

Protocol-level (shared/session.py): null-ID error.message · unnormalizable response ID · unknown-request-ID error (was dumping entire SessionMessage repr = full wire payload, now logs just the ID) · dropped full JSONRPCNotification repr from validation-failure warning

Also removed three debug logs in server/sse.py that stringified the full POST body/parsed message on every request via eager f-string eval (CPU waste even with DEBUG off).

Truncation lengths: 32 for version strings, 64 for IDs/tokens, 128 for names/headers, 256 for URIs/messages, 512 for result dicts.

How Has This Been Tested?

Existing test suite passes (1126 tests). No new tests added — truncation doesn't introduce new branches and the limits are well above any legitimate value.

Breaking Changes

None. Error/log messages change slightly (truncated), but no API surface changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Deferred for separate review (require design decisions, not mechanical truncation):

Location Issue
shared/session.py:361 exc_info=True → Pydantic ValidationError traceback echoes input_value=. Can't truncate a traceback.
server/auth/errors.py:5 stringify_pydantic_error uses e['msg'] — for discriminated-union errors, Pydantic puts the full tag verbatim in msg (verified: 1000-char grant_type → 1116-char error). Shared helper.
authorize.py state RFC 6749 requires state returned unchanged. Fix belongs in a Pydantic max_length field constraint, not response truncation.
client/auth/utils.py:225, oauth2.py:410 Full response.text/body in OAuthRegistrationError/OAuthTokenError. Only debug info user gets on failed token exchange — worth a length discussion.
client/session.py:345,347 jsonschema.ValidationError.__str__ includes full failing instance. Could switch to e.message (one-liner).
MCPError.__str__, SessionMessage.__repr__, RequestId max_length Architectural amplifiers — MCPError.__str__ returns wire error.message verbatim, SessionMessage dataclass repr includes full nested payload, RequestId = ...|str has no length cap. Behavior/schema changes.

Follow-up to 3041fd0 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 3041fd0)
- 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant