feat(mcp): capture resource discovery and reads - #928
Open
lucasheriques wants to merge 4 commits into
Open
Conversation
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewThis pull request adds capture of MCP resource discovery and resource read operations, including sanitization of resource URIs and related event data. Two issues have already been addressed, with one denial-of-service concern remaining. An MCP client can submit a URI with an excessive number of query fields, causing synchronous parsing to consume disproportionate memory and block the server event loop; exploitation depends on the client being able to issue resource-read requests. Open issues (1)
Fixed/addressed: 2 · PR risk: 4/10 |
Contributor
posthog-python Compliance ReportDate: 2026-09-08 22:08:44 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if event_type == MCPAnalyticsEventType.MCP_RESOURCES_READ | ||
| else None, | ||
| "parameters": build_captured_mcp_parameters(request), | ||
| "response": _wrap_response(response) if response is not None else None, |
Contributor
There was a problem hiding this comment.
Resource reads now unconditionally include the full returned response in $mcp_response. Text resources can contain private documents, configuration, or credentials, but the sanitizer only masks sensitive dictionary keys and recognizable token patterns; ordinary sensitive text remains unchanged and is sent to PostHog. Capture only resource metadata by default, or require explicit opt-in before exporting resource bodies.
How this was verified: A successful resources/read result flows through _wrap_response into the PostHog capture pipeline, and the added tests confirm that its text content is emitted verbatim.
Knowledge Base Used: MCP event and session processing
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/mcp/_instrumentation.py
Line: 863
Comment:
**Resource bodies leak sensitive data**
Resource reads now unconditionally include the full returned response in `$mcp_response`. Text resources can contain private documents, configuration, or credentials, but the sanitizer only masks sensitive dictionary keys and recognizable token patterns; ordinary sensitive text remains unchanged and is sent to PostHog. Capture only resource metadata by default, or require explicit opt-in before exporting resource bodies.
**How this was verified:** A successful `resources/read` result flows through `_wrap_response` into the PostHog capture pipeline, and the added tests confirm that its text content is emitted verbatim.
**Knowledge Base Used:** [MCP event and session processing](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-python/-/docs/mcp-event-and-session-processing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Author
Apply existing credential redaction to resource-read names before the primary event and exception sibling are built. Preserve the original URI and resource result or exception received by the caller. Extend the existing resource tests with successful and failing reads containing an invented token; the two new cases fail before this fix under each MCP major. Document the capture boundary and before_send. Validation: MCP v1 245 passed; MCP v2 225 passed and 13 expected skips. Ruff lint and formatting pass. Mypy baseline passes (227 source files).
Parse captured URLs to remove userinfo and credential query values, including common signed URL fields. Apply the same sanitization to URLs inside exception messages without changing handler requests or responses. Document the limits of key-based URL redaction. Verification: reproduced the credential leak before the fix. MCP v1 suite: 260 passed; v2 suite: 240 passed, 13 skipped. Ruff check and format passed; mypy baseline passed for 227 files. Regression coverage includes encoded keys, duplicate query parameters, malformed URLs, and success/error events.
| value = match.group(0) | ||
| try: | ||
| url = urlsplit(value) | ||
| query = parse_qsl(url.query, keep_blank_values=True) |
There was a problem hiding this comment.
Low: Unbounded URL query parsing
An MCP client can issue resources/read with a URI containing a very large number of &-separated query fields. parse_qsl materializes every field synchronously before the resource-name and event-size truncation runs, allowing repeated requests to consume disproportionate memory and block the server event loop. Bound the URL length before parsing and set a conservative max_num_fields, redacting the whole URL when either limit is exceeded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
MCP server authors cannot tell whether clients discover or read their resources.
This leaves resource-based integrations absent from MCP Analytics even when tool tracking works.
Resource bodies stay out of analytics because resources may contain private documents or credentials.
💚 How did you test it?
📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Codex implemented this change with the
debugging-mcp-analytics,writing-tests, andwriting-pr-descriptionsskills. The paired JavaScript implementation uses the same event contract to prevent SDK drift.