-
Notifications
You must be signed in to change notification settings - Fork 77
feat(mcp): emit $mcp_error_message and $mcp_error_type #882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a52a78a
004c745
9fda7cc
021823f
fddfaf1
94ac113
e9c490d
5ed2086
07dd1c8
e335876
45f5630
d7f2f39
f8f4c1a
3c14ab0
c988e85
fb4e0ff
ff81024
68d8727
36a1e7d
1485c11
d76ac48
7ab0498
80c9c35
c61f8ff
8b7e06c
c86b06d
c5542de
de4250f
c47606f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| posthog: minor | ||
| --- | ||
|
|
||
| feat(mcp): emit `$mcp_error_message` and `$mcp_error_type` on failed MCP events. The reason a tool call failed previously lived only on the sibling `$exception` event, so PostHog's failures view — which reads the scalars off the primary event — showed empty error rows for every Python-backed MCP server, and switching off `enable_exception_autocapture` removed the reason entirely. Both values are read from the same `$exception_list` the sibling carries, so the two surfaces can never disagree, and the message inherits the existing 2048-character cap. `PostHogMCP.capture_tool_call()` and `capture_tools_list()` take a new optional `error_type` for custom dispatchers that want a coarse category (`"validation"`, `"timeout"`) instead of the thrown class name. Exception messages are also redacted before they leave — previously nothing sanitized the error payload, so the `$exception` sibling had been shipping them raw. Credential-looking words go through the SDK's own detector (entropy, known key formats, PEM markers), per word, so a message like `auth failed for sk-...` keeps its diagnostic text and loses only the key. Parity with `@posthog/mcp`, which sanitizes exception values the same way. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,38 @@ def _should_redact_key(key: str) -> bool: | |
| def _sanitize_string(value: str) -> str: | ||
| if len(value) >= _SIZE_GATE and _BASE64_PATTERN.match(value): | ||
| return "[binary data redacted - not supported by PostHog MCP analytics]" | ||
| return _POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value) | ||
| return _redact_secret_tokens(_POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value)) | ||
|
|
||
|
|
||
| def _redact_secret_tokens(value: str) -> str: | ||
| """Redact credential-looking words, leaving the surrounding text intact. | ||
|
|
||
| The PostHog-token pattern above only knows ``phc_``/``phx_``; a failure | ||
| message like ``auth failed for sk-proj-...`` carries someone else's key. | ||
| Rather than enumerate every vendor's format — an arms race that fails | ||
| quietly in both directions — this reuses the SDK's own detector | ||
| (``exception_utils._looks_like_secret``: entropy, known formats such as AWS | ||
| key ids, PEM markers), which the code-variables path already ships. | ||
|
|
||
| Applied per whitespace-separated token, not to the whole string: redacting | ||
| an entire exception message would destroy the diagnostic value that | ||
| ``$mcp_error_message`` exists to provide, and ordinary prose is left alone | ||
| because no single word in it looks like a credential. | ||
| """ | ||
| if " " not in value: | ||
| return _REDACTED_VALUE if _is_secret(value) else value | ||
| return " ".join( | ||
| _REDACTED_VALUE if _is_secret(word) else word for word in value.split(" ") | ||
| ) | ||
|
|
||
|
|
||
| def _is_secret(word: str) -> bool: | ||
| try: | ||
| from posthog.exception_utils import _looks_like_secret | ||
|
|
||
| return bool(word) and _looks_like_secret(word) | ||
| except Exception: # noqa: BLE001 - redaction must never break capture | ||
| return False | ||
|
|
||
|
|
||
| def sanitize_captured_value(value: Any) -> Any: | ||
|
|
@@ -64,8 +95,8 @@ def sanitize_captured_value(value: Any) -> Any: | |
|
|
||
|
|
||
| def sanitize_event(event: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Sanitize an event's response, parameters, and user_intent. Returns a new | ||
| shallow copy; does not mutate the input.""" | ||
| """Sanitize an event's response, parameters, user_intent and error. Returns | ||
| a new shallow copy; does not mutate the input.""" | ||
| result = {**event} | ||
|
|
||
| if result.get("response") is not None: | ||
|
|
@@ -79,9 +110,41 @@ def sanitize_event(event: Dict[str, Any]) -> Dict[str, Any]: | |
| if result.get("user_intent") is not None: | ||
| result["user_intent"] = sanitize_captured_value(result["user_intent"]) | ||
|
|
||
| # An exception message is free text a server wrote, and it reaches PostHog | ||
| # on the $exception sibling and — since it is also surfaced as | ||
| # $mcp_error_message — on the primary event, so run it through the same | ||
| # sanitizer as every other captured value. | ||
| # | ||
| # That sanitizer redacts PostHog tokens and sensitive-looking keys; it is | ||
| # deliberately not a general credential scrubber, because enumerating every | ||
| # vendor's key format is an arms race that fails quietly in both directions. | ||
| # A host with strict requirements should gate free text in `before_send`. | ||
| # Same scope as @posthog/mcp's sanitizeCapturedValue. | ||
| if result.get("error") is not None: | ||
| result["error"] = _sanitize_exception_values(result["error"]) | ||
|
|
||
| return result | ||
|
|
||
|
|
||
| def _sanitize_exception_values(error: Any) -> Any: | ||
| """Redact the ``value`` of every frame in an ``$exception_list``, leaving | ||
| the rest of the error-tracking shape untouched.""" | ||
| if not isinstance(error, dict): | ||
| return error | ||
| exception_list = error.get("$exception_list") | ||
| if not isinstance(exception_list, list): | ||
| return error | ||
| return { | ||
| **error, | ||
| "$exception_list": [ | ||
| {**exception, "value": sanitize_captured_value(exception.get("value"))} | ||
|
veria-ai[bot] marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right and my earlier reasoning was wrong — fixed in c61f8ff. I'd argued to veria that enumerating vendor key formats is an arms race, but that doesn't apply here: posthog-python already ships |
||
| if isinstance(exception, dict) | ||
| else exception | ||
| for exception in exception_list | ||
| ], | ||
| } | ||
|
|
||
|
|
||
| def _sanitize_response(response: Any) -> Any: | ||
| if response is None or not isinstance(response, (dict, list, str)): | ||
| return sanitize_captured_value(response) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a failed MCP call's exception message contains a token or another sensitive value,
_add_error_detailscopies the unsanitized$exception_listvalue into$mcp_error_message, causing sensitive text to be transmitted on the primary event even when exception autocapture is disabled.How this was verified: The MCP sanitizer processes response, parameters, and user intent but not the error payload read by this mapping.
Knowledge Base Used: MCP Instrumentation (posthog/mcp)
Prompt To Fix With AI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid, and broader than reported — fixed in c988e85.
sanitize_eventcovered response/parameters/user_intent but nevererror, so the$exceptionsibling has been shipping unredacted exception messages since long before this property existed (@posthog/mcpsanitizes it viasanitizeExceptionValues; Python never ported that). Now every$exception_listframe'svaluegoes through the same sanitizer, which covers both surfaces since the scalar is derived after sanitization runs — regression test verified to fail without the fix.