Skip to content

feat(mcp): add MCP-023, tool prints to stdout, corrupting a stdio transport - #82

Open
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/mcp-observability
Open

feat(mcp): add MCP-023, tool prints to stdout, corrupting a stdio transport#82
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/mcp-observability

Conversation

@bradAGI

@bradAGI bradAGI commented Aug 24, 2026

Copy link
Copy Markdown

MCP had no observability rule. OpenAI ships OAI-010 and ADK ships ADK-009 for the same print() pattern — both at low severity, framed as a lost-diagnostic problem.

It isn't a lost-diagnostic problem here, which is why this ships at medium instead. On a stdio server — the FastMCP default, and the usual way an editor or desktop client launches a server — stdout is the protocol channel. The client reads newline-delimited JSON-RPC messages from it, so a loose print interleaves with those frames: the client hits a parse error on a line that isn't JSON and, depending on the client, drops the response, logs a protocol error, or tears down the connection.

The reason it's worth flagging rather than leaving to review is that the failure doesn't look like its cause. The symptom is a tool call that returns nothing, or a server that disconnects mid-session; the print that broke it reads like harmless debugging and sits nowhere near the error.

OAI-010's explanation already anticipates exactly this — it warns that a tool "ever exposed over an MCP stdio server" will corrupt the stream. This rule is that case as the primary reading rather than the footnote.

Confidence is 0.7 rather than higher because a server run over HTTP or SSE escapes the corruption — though it still loses the diagnostic, since the model only ever sees the return value. The fix names stderr specifically, which the transport leaves alone and clients typically capture as server logs, plus the caveat about nothing in the process reconfiguring logging onto stdout.

Verification — engine built at main:

$ trustabl rules validate .
OK: 86 rule pack(s), 207 rule(s) valid under rule schema version 14

Fire (print(f"looking up {order_id}") in a FastMCP tool handler): MCP-023
Silent (logger.info(...) with basicConfig(stream=sys.stderr)): no findings

has_print_call matches a bare print callee, so pprint and other attribute calls don't false-positive.

No new predicates, so no schema_version bump.

…nsport

MCP had no observability rule. OpenAI ships OAI-010 and ADK ships ADK-009
for the same pattern, both at low severity as a lost-diagnostic problem.

It is not a lost-diagnostic problem here, which is why this ships at
medium rather than low. On a stdio server — the FastMCP default, and the
usual way an editor or desktop client launches a server — stdout is the
protocol channel, so a print interleaves with the newline-delimited
JSON-RPC frames the client is parsing. The client hits a parse error on a
line that is not JSON and drops the response or tears down the
connection, and the symptom does not resemble the cause: a tool call that
returns nothing, or a mid-session disconnect, traced back to a print that
reads like harmless debugging.

Confidence 0.7 rather than higher because a server run over HTTP or SSE
escapes the corruption, though it still loses the diagnostic. The fix
names stderr specifically, which the transport leaves alone.
@eulestadt

Copy link
Copy Markdown

Heads-up on an ID overlap, and it's already resolved on my side — no action needed from you.

This PR and my #80 both shipped as MCP-023 about four minutes apart. Since a duplicated rule ID is a loader refusal at startup rather than a test failure, two green PRs merging an hour apart would have broken every scan until someone renumbered.

Your stack claims 023 through 028 (#82, #85, #88, #91), so yielding was one edit for me versus five for you. I've renumbered mine to MCP-029 across the rules, fixture and rulebook PRs. MCP-023 is yours; nothing here needs to change.

While I was checking the ranges: #52 and #81 both claim LC-007 — the network-timeout pair and the error-contract rule. Whichever lands second will need the same treatment. Mentioning it only because I'd just done the exercise.

Separately, #55's note that PredHasRaise can't support a TS error-contract rule was correct and I've opened trustabl#169 to close it — tsHandlerFacts now records throws / try_catch and both predicates branch on IsTSOrJS the way PredHasShellCall already did. It's engine-only and merges on its own, so TS counterparts to CSDK-005 / OAI-008 / ADK-005 / MCP-006 become writable after it. All yours if you want them.

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.

2 participants