Skip to content

Accepted argument values are echoed into responses and logs #315

Open

Description

@ViperJuice

#297 stops validation errors from echoing what they
rejected. A value that passes validation, such as a token put in the
wrong string field, is still echoed wherever a handler looks it up,
refuses it or reports on it. Most of these are also returned in a
structured output field by design. Sites on 7680445:

  • routing names:
  • lookups / policy refusals:
    • handlers.py:1341/:1359 (describe, details only);
    • :1510/:1543/:1569 (make_error(tool_id=...) in errors);
    • :2426/:2541/:3010/:3083/:3192 (Server '{server_name}' ..., and
      into the audit error via _lifecycle_output :2898-2908);
    • :3888/:4044, :5585 (Job '{job_id}' not found);
    • tasks_* blocked-by-policy :5938/:6021/:6065/:6157;
    • client/manager.py:3847/:3896 (Task not found: {server}::{task_id}).
  • format / allowlist refusals of a schema-valid string:
    • handlers.py:4549/:4558 (Env var '{env_var}' is not permitted);
    • :4567-4581 via env_store.py:23 ({name!r});
    • :5410 (unsafe package identifier {package!r});
    • :5427-5433 (disallowed env_vars, operator_safe);
    • client/manager.py:4208/:4217 (Invalid request_id format: {request_id}, Invalid local_id);
    • config/loader.py:778-780/:854 (the policy path).
  • free text echoed by design:
    • refresh reason (handlers.py:1857, INFO log);
    • request_capability query (:3860 log, :3866/:3869 output);
    • search_registry query (:5384);
    • submit_feedback title/description (:4681-4682).
  • invoke's own log lines name tool_id (:1657/:1688/:1734/:1817).

Proposed class fix: give identifier fields a pattern so token-shaped input
fails validation (and is then described without its value by #297). For
example tool_id can reuse scoped_advisor_audit._TOOL_ID_PATTERN; add
patterns for server_name, task_id, job_id, request_id and
env_var. Then route the routing-name log lines through a fixed text as
#297 did for Unknown tool. This changes advertised schemas, so it
belongs with, or after, piece B of #236.

  • N5, resources/read / prompts/get: this is part of the follow-up
    above, not this plan. It is a lookup echo, not a validation error. The
    SDK writes the traceback for any exception a request handler raises, so
    silencing it means raising MCPError, which changes the wire error code.
  • N4, the SDK's malformed-envelope echo (streamable_http.py:550):

Source: the non-goals of the plan for #297 (#314); line numbers are on 7680445.

🤖 Generated with Claude Code

Activity

  1. ViperJuice commented on Sep 29, 2026

    @ViperJuice
    CollaboratorAuthor

    One more site, found in review of #314: client/manager.py:2037-2041 logs a rejected downstream pagination cursor verbatim (unusable cursor ({raw_cursor!r})). This is not an exception path, so it belongs on this list rather than in #297.

    🤖 Generated with Claude Code

  2. ViperJuice commented on Sep 29, 2026

    @ViperJuice
    CollaboratorAuthor

    Two more sites, found in review of #314. Neither is a validation-error path:

    • the MCP SDK's INFO log line that quotes an unknown session ID sent to the gateway's streamable-HTTP transport;
    • pmcp's DEBUG log line that quotes the request's accept header.

    🤖 Generated with Claude Code

  3. ViperJuice commented on Sep 30, 2026

    @ViperJuice
    CollaboratorAuthor

    Two more pre-existing, operator/config-side echoes, found by the claude seat in round 7 of the #314 panel (the #297 plan). Recorded here rather than in #297 because they are accepted-config or launch-environment values, not tool-argument validation:

    • src/pmcp/config/loader.py:494 logs a rejected allowPrivateRegistry value with %r. Triggered by a call to gateway.search_registry. Repro: ~/.mcp.json with "allowPrivateRegistry": "SENT…" gives a WARNING containing the value. Fix: log only the value's type.
    • PMCP_PORT (and three other environment variables) go through a bare int(): PMCP_PORT=SENT… python -m pmcp prints Fatal error: invalid literal for int() … 'SENT…' to stderr (not to gateway.log). Two warnings in src/pmcp/client/manager.py do the same with %r. Visible only to whoever launches the process. Fix: fixed-text messages naming the variable, not its value.
  4. ViperJuice commented on Oct 1, 2026

    @ViperJuice
    CollaboratorAuthor

    Update from the round-9 panel on #314 (the #297 plan): the mcp SDK's DEBUG logging of JSON-RPC message payloads (incoming accepted frames, and outgoing client messages with the caller's arguments) is moving INTO #297's revision 10. A downstream payload that pmcp later rejects (e.g. a task ttl) was logged by the SDK before validation, so it breaks #297's own guarantee. The fix replaces every SDK record that renders a message payload with a structural description, which also removes the accepted-value traffic-logging part of this issue. The lookup-mismatch and config echoes listed above stay here.

  5. ViperJuice commented on Oct 1, 2026

    @ViperJuice
    CollaboratorAuthor

    Scope note from round 10 of the #314 panel: on the serving side, sse_starlette logs every outbound SSE chunk at DEBUG — the full body of each response pmcp sends its caller, including proxied downstream results. It never carries a value pmcp rejected, so it is outside #297 and stays here with the other accepted-value echoes. (#297 rev 10 covers the mcp SDK's own message-object rendering on the client side.)

  6. ViperJuice commented on Oct 1, 2026

    @ViperJuice
    CollaboratorAuthor

    Correction from round 11 of the #314 panel: the _fetch_listing_pages cursor log (tools/list returned an unusable cursor (…), rendering raw_cursor!r at WARNING), which an earlier comment on this issue listed, echoes a value pmcp REJECTS (a non-string cursor), so it moves into #297's revision 12 together with a sweep for every other hand-written rejection log that renders the rejected value. It is no longer tracked here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions