diff --git a/.sampo/changesets/mcp-capture-bounds.md b/.sampo/changesets/mcp-capture-bounds.md new file mode 100644 index 000000000..88d1ddc7d --- /dev/null +++ b/.sampo/changesets/mcp-capture-bounds.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +MCP analytics: bound capture work and close feedback leaks. `$mcp_intent` and the `send_feedback` fields are redacted only as far as truncation can keep, so a megabyte-long intent no longer stalls capture for seconds, and the start of long text is kept as before. Captured tool arguments are limited to 100 entries and depth 10 before they are sanitized, credential-named arguments are captured as `[redacted]` without being read, and deeply nested arguments no longer raise. Object `$mcp_feedback_` extras redact identifiers and known credential formats in keys, identifiers in numbers, are limited to 100 entries and depth 10, and capture non-JSON values as text. diff --git a/posthog/exception_utils.py b/posthog/exception_utils.py index a6f5883b2..a7428a561 100644 --- a/posthog/exception_utils.py +++ b/posthog/exception_utils.py @@ -1465,15 +1465,20 @@ def _looks_like_secret(value): """Last-resort credential check, layered cheapest first.""" if _PEM_PRIVATE_KEY_MARKER in value: return True - n = len(value) - if n < _SECRET_MIN_LENGTH: - return False - if _is_high_entropy_secret(value): + if len(value) >= _SECRET_MIN_LENGTH and _is_high_entropy_secret(value): return True # Known formats the entropy gate misses (e.g. AWS AKIA ids: only two char classes). - if n <= _KNOWN_SECRET_MAX_SCAN_LENGTH: - return _KNOWN_SECRET_RE.search(value) is not None - return False + return _looks_like_known_secret(value) + + +def _looks_like_known_secret(value): + """A PEM private key or a known vendor credential format, with no entropy guess.""" + if _PEM_PRIVATE_KEY_MARKER in value: + return True + return ( + _SECRET_MIN_LENGTH <= len(value) <= _KNOWN_SECRET_MAX_SCAN_LENGTH + and _KNOWN_SECRET_RE.search(value) is not None + ) def _mask_string(value, config): diff --git a/posthog/mcp/_sanitization.py b/posthog/mcp/_sanitization.py index 426d2d29f..dbb0a8141 100644 --- a/posthog/mcp/_sanitization.py +++ b/posthog/mcp/_sanitization.py @@ -10,16 +10,25 @@ from __future__ import annotations +import os import re -from typing import Any, Dict, List, Optional, Set, Tuple +import sys +from typing import Any, Callable, Dict, List, Optional, Set, Tuple from urllib.parse import SplitResult, parse_qsl, urlencode, urlsplit, urlunsplit from ._event_types import MCPAnalyticsEventType +from ._truncation import ( + _MAX_USER_INTENT_LENGTH, + _REDACTED_VALUE, + _TRUNCATION_SUFFIX, + MAX_BREADTH, + normalize, +) # SDK-injected arguments stripped from captured $mcp_parameters (they surface as # dedicated properties: $mcp_intent and $mcp_conversation_id). _INJECTED_ARGUMENT_NAMES = ("context", "conversation_id") -_REDACTED_VALUE = "[redacted]" +_CAPTURED_REQUEST_FIELDS = ("id", "jsonrpc", "method", "params") _BINARY_DATA_MARKER = "[binary data redacted - not supported by PostHog MCP analytics]" _ENCODED_REDACTED_VALUE = "%5Bredacted%5D" _BASE64_PATTERN = re.compile(r"^[A-Za-z0-9+/\n\r]+=*$") @@ -410,6 +419,25 @@ def _names_a_credential(value: str) -> bool: r"(? str: + """``redact`` over ``value``, at a cost bounded by what truncation keeps.""" + if len(value) <= _MAX_USER_INTENT_LENGTH + _SCAN_MARGIN: + return redact(value) + kept = os.path.commonprefix( + [ + redact(value[: _MAX_USER_INTENT_LENGTH + _SCAN_MARGIN]), + redact(value[:_MAX_USER_INTENT_LENGTH]), + ] + ) + return kept + _TRUNCATION_SUFFIX + def _is_record(value: Any) -> bool: return isinstance(value, dict) @@ -430,10 +458,10 @@ def _is_binary_blob(value: str) -> bool: def _sanitize_text(value: str) -> str: - return _sanitize_urls(_redact_credentials(value)) + return _sanitize_urls(_redact_credentials(value, _is_secret)) -def _redact_credentials(value: str) -> str: +def _redact_credentials(value: str, is_secret: Callable[[str], bool]) -> str: """Redact PostHog tokens, then any other word that reads as a credential. Both run before the URL pass, because rewriting a URL changes the text they @@ -442,7 +470,9 @@ def _redact_credentials(value: str) -> str: entropy detector scans. Running the URL pass last loses nothing — it only redacts or percent-encodes, so it never exposes a credential the detectors could have matched.""" - return _redact_secret_tokens(_POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value)) + return _redact_secret_tokens( + _POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value), is_secret + ) def sanitize_free_text(value: Any) -> Any: @@ -461,7 +491,11 @@ def sanitize_free_text(value: Any) -> Any: return sanitize_captured_value(value) if _is_binary_blob(value): return _BINARY_DATA_MARKER - return _sanitize_urls(redact_pii(_redact_credentials(value))) + return _redact_bounded(value, _redact_free_text) + + +def _redact_free_text(value: str) -> str: + return _sanitize_urls(redact_pii(_redact_credentials(value, _is_secret))) def _sanitize_resource_name(value: Any) -> Any: @@ -476,7 +510,7 @@ def _sanitize_resource_name(value: Any) -> Any: return _sanitize_urls(_POSTHOG_TOKEN_PATTERN.sub(_REDACTED_VALUE, value)) -def _redact_secret_tokens(value: str) -> str: +def _redact_secret_tokens(value: str, is_secret: Callable[[str], bool]) -> str: """Redact credential-looking words, leaving the surrounding text intact. The PostHog-token pattern above only knows ``phc_``/``phx_``; a failure @@ -492,13 +526,25 @@ def _redact_secret_tokens(value: str) -> str: because no single word in it looks like a credential. """ if " " not in value: - return _REDACTED_VALUE if _is_secret(value) else 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(" ") + _REDACTED_VALUE if is_secret(word) else word for word in value.split(" ") ) def _is_secret(word: str) -> bool: + from posthog.exception_utils import _looks_like_secret + + return _judge_without_markers(word, _looks_like_secret) + + +def _is_known_secret(word: str) -> bool: + from posthog.exception_utils import _looks_like_known_secret + + return _judge_without_markers(word, _looks_like_known_secret) + + +def _judge_without_markers(word: str, looks_like_secret: Callable[[str], bool]) -> bool: # Judge the word without this sanitizer's own markers. A value can be # sanitized twice (a response's content blocks are), and the marker's # character mix is enough to push a short uri like @@ -507,9 +553,7 @@ def _is_secret(word: str) -> bool: # the word keeps a real credential written around one detectable. word = word.replace(_ENCODED_REDACTED_VALUE, "").replace(_REDACTED_VALUE, "") try: - from posthog.exception_utils import _looks_like_secret - - return bool(word) and _looks_like_secret(word) + return bool(word) and looks_like_secret(word) except Exception: # noqa: BLE001 - redaction must never break capture return False @@ -588,35 +632,69 @@ def redact_pii(value: Any) -> Any: def sanitize_captured_value(value: Any) -> Any: - return _sanitize_value_with(value, _sanitize_string) + return _sanitize_value_with(value, _sanitize_captured_leaf, lambda key: key) def sanitize_free_text_value(value: Any) -> Any: - """:func:`sanitize_captured_value` with the free-text string pass + """:func:`sanitize_captured_value` with the free-text pass (:func:`sanitize_free_text`) on every string leaf, so nested agent-narrated - values get structured-PII redaction in the load-bearing order too.""" - return _sanitize_value_with(value, sanitize_free_text) + values get structured-PII redaction in the load-bearing order too. Keys and + numbers get it as well, since a value sent as JSON text carries them: a + number that holds an identifier becomes ``[redacted]``, so the text stays + valid JSON.""" + return _sanitize_value_with( + value, _sanitize_free_text_leaf, _sanitize_free_text_key + ) -def _sanitize_value_with(value: Any, sanitize_string_fn: Any) -> Any: - if value is None: - return value +def _sanitize_captured_leaf(value: Any) -> Any: + return _sanitize_string(value) if isinstance(value, str) else value + + +def _sanitize_free_text_leaf(value: Any) -> Any: if isinstance(value, str): - return sanitize_string_fn(value) + return sanitize_free_text(value) + # bool is an int subclass and passes through unchanged. + if isinstance(value, (int, float)) and not isinstance(value, bool): + try: + text = str(value) + except ValueError: # an int past Python's digit limit for str() + return _REDACTED_VALUE + return value if sanitize_free_text(text) == text else _REDACTED_VALUE + return value + + +def _sanitize_free_text_key(key: Any) -> Any: + """A key is a name, so it keeps the known credential formats but skips the + entropy detector, which reads ``Get_Organization_Memberships`` as a credential: + two such keys would both become ``[redacted]`` and one value would be lost.""" + if not isinstance(key, str): + return _sanitize_free_text_leaf(key) + return _redact_bounded( + key, + lambda text: _sanitize_urls( + redact_pii(_redact_credentials(text, _is_known_secret)) + ), + ) + + +def _sanitize_value_with( + value: Any, + sanitize_leaf: Callable[[Any], Any], + sanitize_key: Callable[[Any], Any], +) -> Any: if isinstance(value, list): - return [_sanitize_value_with(item, sanitize_string_fn) for item in value] - # bool is an int subclass; both pass through unchanged. + return [ + _sanitize_value_with(item, sanitize_leaf, sanitize_key) for item in value + ] if not isinstance(value, dict): - return value - - result: Dict[str, Any] = {} - for key, nested in value.items(): - result[key] = ( - _REDACTED_VALUE - if _should_redact_key(str(key)) - else _sanitize_value_with(nested, sanitize_string_fn) - ) - return result + return sanitize_leaf(value) + return { + sanitize_key(key): _REDACTED_VALUE + if _should_redact_key(str(key)) + else _sanitize_value_with(nested, sanitize_leaf, sanitize_key) + for key, nested in value.items() + } def sanitize_event(event: Dict[str, Any]) -> Dict[str, Any]: @@ -754,55 +832,53 @@ def build_captured_mcp_parameters( strip_llm_model: bool = False, strip_argument_names: Optional[Set[str]] = None, ) -> Dict[str, Any]: - """Build the sanitized ``$mcp_parameters`` payload from a request, stripping - the injected ``context`` argument before logging.""" - if not _is_record(request): - return {"request": sanitize_captured_value(request)} - - captured_request: Dict[str, Any] = {} - for key in ("id", "jsonrpc", "method"): - if key in request: - captured_request[key] = sanitize_captured_value(request[key]) - + """Build the sanitized ``$mcp_parameters`` payload from a request: its id, + JSON-RPC version, method and params, without the SDK-injected arguments. The + entry and nesting limits apply before sanitizing, so a call with a huge + argument object costs what a call at those limits costs.""" names_to_strip = strip_argument_names if names_to_strip is None: names_to_strip = set(_INJECTED_ARGUMENT_NAMES) if strip_llm_model: names_to_strip.add("llm_model") - if "params" in request: - captured_request["params"] = _build_captured_mcp_params( - request["params"], strip_argument_names=names_to_strip - ) - - return {"request": captured_request} + return sanitize_captured_value( + limit_entries({"request": _pick_captured_fields(request, names_to_strip)}) + ) -def _build_captured_mcp_params(params: Any, *, strip_argument_names: Set[str]) -> Any: - if not _is_record(params): - return sanitize_captured_value(params) +def limit_entries(value: Any) -> Any: + """``value`` within the entry and nesting limits, strings left whole, so a + sanitizer reads at most what the event can keep and still sees past a + string's cut. A credential key's value is replaced unread, as the sanitizer + would replace it.""" + return normalize( + value, max_string_length=sys.maxsize, redacts_key=_should_redact_key + ) - captured: Dict[str, Any] = {} - for key, value in params.items(): - captured[key] = ( - _build_captured_mcp_arguments( - value, strip_argument_names=strip_argument_names - ) - if key == "arguments" - else sanitize_captured_value(value) - ) - return captured +def _pick_captured_fields(request: Any, strip_argument_names: Set[str]) -> Any: + if not _is_record(request): + return request + fields = {key: request[key] for key in _CAPTURED_REQUEST_FIELDS if key in request} + params = fields.get("params") + if isinstance(params, dict) and _is_record(params.get("arguments")): + fields["params"] = { + **params, + "arguments": _kept_arguments(params["arguments"], strip_argument_names), + } + return fields -def _build_captured_mcp_arguments( - arguments: Any, *, strip_argument_names: Set[str] -) -> Any: - if not _is_record(arguments): - return sanitize_captured_value(arguments) - captured: Dict[str, Any] = {} +def _kept_arguments( + arguments: Dict[str, Any], strip_argument_names: Set[str] +) -> Dict[str, Any]: + """The arguments without the stripped names, which the entry limit does not + count. Copies one more than the limit, enough for it to see that more remain.""" + kept: Dict[str, Any] = {} for key, value in arguments.items(): - if key in strip_argument_names: - continue - captured[key] = sanitize_captured_value(value) - return captured + if key not in strip_argument_names: + kept[key] = value + if len(kept) > MAX_BREADTH: + break + return kept diff --git a/posthog/mcp/_truncation.py b/posthog/mcp/_truncation.py index 382c64639..3ad060c8e 100644 --- a/posthog/mcp/_truncation.py +++ b/posthog/mcp/_truncation.py @@ -18,7 +18,8 @@ import json import math from datetime import datetime -from typing import Any, Dict, List, Optional +from dataclasses import dataclass +from typing import Any, Callable, Dict, List, Optional, Set from ._server_build import MAX_SERVER_BUILD_LENGTH @@ -35,6 +36,7 @@ _MAX_CONTENT_TEXT_LENGTH = 32_768 _TRUNCATION_SUFFIX = "..." +_REDACTED_VALUE = "[redacted]" _METADATA_FIELDS = ( ("user_intent", _MAX_USER_INTENT_LENGTH), @@ -60,10 +62,23 @@ def normalize( depth: int = MAX_DEPTH, max_breadth: int = MAX_BREADTH, max_string_length: int = MAX_STRING_LENGTH, + redacts_key: Optional[Callable[[str], bool]] = None, ) -> Any: """Recursively normalize a value: cap strings, coerce non-serializable - values, convert datetimes, detect cycles, and bound depth/breadth.""" - return _visit(value, depth, max_breadth, max_string_length, set()) + values, convert datetimes, detect cycles, and bound depth/breadth. A key + ``redacts_key`` accepts gets ``[redacted]`` without its value being read.""" + return _visit( + value, depth, max_breadth, max_string_length, _Walk(redacts_key, set()) + ) + + +@dataclass +class _Walk: + """What one walk shares: which keys to redact unread, and the containers on + the current path.""" + + redacts_key: Optional[Callable[[str], bool]] + memo: Set[int] def _visit( @@ -71,7 +86,7 @@ def _visit( remaining_depth: int, max_breadth: int, max_string_length: int, - memo: set, + walk: _Walk, ) -> Any: if value is None: return None @@ -93,28 +108,28 @@ def _visit( if isinstance(value, (list, tuple)): oid = id(value) - if oid in memo: + if oid in walk.memo: return "[Circular ~]" if remaining_depth <= 0: return "[Array]" - memo.add(oid) + walk.memo.add(oid) result: Any = _visit_array( - list(value), remaining_depth - 1, max_breadth, max_string_length, memo + list(value), remaining_depth - 1, max_breadth, max_string_length, walk ) - memo.discard(oid) + walk.memo.discard(oid) return result if isinstance(value, dict): oid = id(value) - if oid in memo: + if oid in walk.memo: return "[Circular ~]" if remaining_depth <= 0: return "[Object]" - memo.add(oid) + walk.memo.add(oid) result = _visit_object( - value, remaining_depth - 1, max_breadth, max_string_length, memo + value, remaining_depth - 1, max_breadth, max_string_length, walk ) - memo.discard(oid) + walk.memo.discard(oid) return result return str(value) @@ -125,7 +140,7 @@ def _visit_array( remaining_depth: int, max_breadth: int, max_string_length: int, - memo: set, + walk: _Walk, ) -> List[Any]: result: List[Any] = [] for i, item in enumerate(arr): @@ -133,7 +148,7 @@ def _visit_array( result.append("[MaxProperties ~]") break result.append( - _visit(item, remaining_depth, max_breadth, max_string_length, memo) + _visit(item, remaining_depth, max_breadth, max_string_length, walk) ) return result @@ -143,7 +158,7 @@ def _visit_object( remaining_depth: int, max_breadth: int, max_string_length: int, - memo: set, + walk: _Walk, ) -> Dict[Any, Any]: result: Dict[Any, Any] = {} count = 0 @@ -151,8 +166,11 @@ def _visit_object( if count >= max_breadth: result["..."] = "[MaxProperties ~]" break - result[key] = _visit(val, remaining_depth, max_breadth, max_string_length, memo) count += 1 + if walk.redacts_key is not None and walk.redacts_key(str(key)): + result[key] = _REDACTED_VALUE + continue + result[key] = _visit(val, remaining_depth, max_breadth, max_string_length, walk) return result diff --git a/posthog/mcp/feedback.py b/posthog/mcp/feedback.py index 5cb732b6c..42a22d101 100644 --- a/posthog/mcp/feedback.py +++ b/posthog/mcp/feedback.py @@ -10,7 +10,7 @@ from typing import Any, Dict, Optional, Union from ._internal import _maybe_await -from ._sanitization import sanitize_free_text, sanitize_free_text_value +from ._sanitization import limit_entries, sanitize_free_text, sanitize_free_text_value from ._truncation import truncate_text from .constants import PostHogMCPAnalyticsProperty from .logger import log @@ -258,16 +258,18 @@ def _capture_free_text(value: str) -> str: def _capture_extra_value(value: Any) -> Any: - """A declared extra is agent-supplied like the core free-text fields, so its - string leaves get the same free-text pass (with the key-based redaction - ``sanitize_free_text_value`` keeps for nested objects), then non-scalars are - JSON-stringified and everything is bounded. Unserializable values are dropped.""" - sanitized = sanitize_free_text_value(value) - if isinstance(sanitized, str): - return truncate_text(sanitized, _MAX_FEEDBACK_TEXT_LENGTH) - if sanitized is None or isinstance(sanitized, (bool, int, float)): - return sanitized + """A declared extra is agent-supplied like the core free-text fields, so it + gets the same free-text pass, then non-scalars are JSON-stringified and + everything is bounded. A top-level number or boolean is captured as sent. + Sanitizing before stringifying reads each key as written, where the JSON text + would glue an escape's letter to the identifier after it. A non-JSON value + inside an object is captured as its text, as in ``$mcp_parameters``.""" + if value is None or isinstance(value, (bool, int, float)): + return value try: + sanitized = sanitize_free_text_value(limit_entries(value)) + if isinstance(sanitized, str): + return truncate_text(sanitized, _MAX_FEEDBACK_TEXT_LENGTH) return truncate_text(json.dumps(sanitized), _MAX_FEEDBACK_TEXT_LENGTH) except Exception: # noqa: BLE001 - capture must never raise into the tool path return None diff --git a/posthog/test/mcp/test_feedback.py b/posthog/test/mcp/test_feedback.py index 5110a0d60..049b42788 100644 --- a/posthog/test/mcp/test_feedback.py +++ b/posthog/test/mcp/test_feedback.py @@ -1,5 +1,7 @@ """Tests for the ``send_feedback`` virtual tool (the ``collect_feedback`` option).""" +import json + import mcp.types as mcp_types import pytest from mcp.server.fastmcp import FastMCP @@ -234,7 +236,7 @@ def test_properties_carry_all_fields_and_redact_pii(): def test_properties_bound_free_text_and_tool_name(): report = parse_feedback_report( - {"feedback_type": "issue", "summary": "s" * 5000, "tool_name": "t" * 500} + {"feedback_type": "issue", "summary": "s" * 4000, "tool_name": "t" * 500} ) props = build_feedback_event_properties(report) assert len(props["$mcp_feedback_summary"]) == 2048 @@ -378,6 +380,47 @@ def test_nested_extras_keep_key_based_redaction(): assert '"note": "ping [redacted]"' in props["$mcp_feedback_meta"] +@pytest.mark.parametrize( + ("meta", "identifier", "expected"), + [ + ({"card\n4111111111111111": 1}, "4111111111111111", None), + ({"ada@example.com": "x"}, "ada@example.com", {"[redacted]": "x"}), + ({"n": 4111111111111111}, "4111111111111111", {"n": "[redacted]"}), + ({"n": [4111111111111111, 7]}, "4111111111111111", {"n": ["[redacted]", 7]}), + ( + {"Get_Organization_Memberships": 1, "ISO_8601_timestamp_UTC": 5}, + "[redacted]", + {"Get_Organization_Memberships": 1, "ISO_8601_timestamp_UTC": 5}, + ), + ({"n": 10**5000}, "0000000000", {"n": "[redacted]"}), + ({"sk-proj-" + "Ab9z" * 8: 1}, "Ab9zAb9z", {"[redacted]": 1}), + ({"key sk-proj-" + "Ab9z" * 8: 1}, "Ab9zAb9z", {"key [redacted]": 1}), + ], +) +def test_object_extras_redact_identifiers_in_keys_and_numbers( + meta, identifier, expected +): + options = CollectFeedbackOptions(extra_properties={"meta": {"type": "object"}}) + report = parse_feedback_report( + {"feedback_type": "issue", "summary": "s", "meta": meta}, options + ) + captured = build_feedback_event_properties(report)["$mcp_feedback_meta"] + assert identifier not in captured + assert expected is None or json.loads(captured) == expected + + +@pytest.mark.parametrize( + ("schema", "value"), + [({"type": "number"}, 4111111111111111), ({"type": "boolean"}, True)], +) +def test_top_level_scalar_extras_are_captured_as_sent(schema, value): + options = CollectFeedbackOptions(extra_properties={"meta": schema}) + report = parse_feedback_report( + {"feedback_type": "issue", "summary": "s", "meta": value}, options + ) + assert build_feedback_event_properties(report)["$mcp_feedback_meta"] == value + + def test_intent_joins_summary_and_details(): report = parse_feedback_report(_REPORT_ARGS) assert ( @@ -1109,3 +1152,15 @@ def test_send_feedback_result_shape(): result = send_feedback_result() assert result["content"][0]["type"] == "text" assert result["content"][0]["text"] == send_feedback_result_text() + + +def test_object_extras_are_limited_before_they_are_sanitized(): + deep = "leaf" + for _ in range(5000): + deep = {"child": deep} + options = CollectFeedbackOptions(extra_properties={"meta": {"type": "object"}}) + report = parse_feedback_report( + {"feedback_type": "issue", "summary": "s", "meta": deep}, options + ) + captured = build_feedback_event_properties(report)["$mcp_feedback_meta"] + assert "[Object]" in captured diff --git a/posthog/test/mcp/test_pipeline.py b/posthog/test/mcp/test_pipeline.py index ea7457023..c3397df92 100644 --- a/posthog/test/mcp/test_pipeline.py +++ b/posthog/test/mcp/test_pipeline.py @@ -1,5 +1,6 @@ """Unit tests for the MCP analytics core pipeline (Milestone 1, no server).""" +import time from datetime import datetime, timezone import pytest @@ -20,7 +21,12 @@ sanitize_event, ) from posthog.mcp._sink import McpCaptureOptions, process_mcp_event -from posthog.mcp._truncation import MAX_EVENT_BYTES, normalize, truncate_event +from posthog.mcp._truncation import ( + MAX_EVENT_BYTES, + normalize, + truncate_event, + truncate_text, +) # --- ids --------------------------------------------------------------------- @@ -729,6 +735,66 @@ def test_sanitize_event_resource_name_keeps_identifiers_and_redacts_uris( assert result["resource_name"] == expected +def _captured_intent(intent: str) -> str: + return truncate_event(sanitize_event({"user_intent": intent}))["user_intent"] + + +def _straddling(identifier: str, cut: int) -> str: + """An intent whose start shrinks by about 1500 characters when redacted, with + ``identifier`` starting a few characters before the ``cut``-th.""" + shrinking_start = "see https://h/?token=" + "x" * 1500 + padding = ("w " * 1000)[: cut - 4 - len(shrinking_start) - 1] + return f"{shrinking_start} {padding}{identifier} " + "tail " * 2000 + + +@pytest.mark.parametrize( + "intent", + [ + pytest.param("1-1." * 250_000, id="separated digits"), + pytest.param("lorem ipsum " * 100_000, id="prose"), + ], +) +def test_intent_scan_is_bounded_by_what_truncation_keeps(intent): + started = time.perf_counter() + _captured_intent(intent) + assert time.perf_counter() - started < 0.5 + + +@pytest.mark.parametrize("cut", [2048, 3072], ids=["at the limit", "at the margin"]) +@pytest.mark.parametrize( + ("identifier", "fragment"), + [ + ("4111 1111 1111 1111", "4111"), + ("123 45 6789", "123"), + ("(415) 555 0142", "(415)"), + ("https://admin:hunter2@db.example.com/x", "hunter2"), + ("+44\u00a0 20 7946 0958", "+44"), + ], +) +def test_intent_scan_never_splits_an_identifier_at_its_cut(identifier, fragment, cut): + captured = _captured_intent(_straddling(identifier, cut)) + assert fragment not in captured + + +@pytest.mark.parametrize( + "intent", + [ + pytest.param("s" * 9000, id="one long word"), + pytest.param("请帮我查询" * 1800, id="long CJK"), + pytest.param("请帮我查询" * 900, id="CJK"), + pytest.param("item\n" * 1000, id="newline list"), + pytest.param("Open /" + "very/long/path/" * 300, id="prose then a long path"), + ], +) +def test_intent_with_no_word_boundary_keeps_its_start(intent): + assert _captured_intent(intent) == truncate_text(intent, 2048) + + +def test_intent_scan_keeps_what_truncation_keeps(): + intent = "lorem ipsum " * 1000 + assert _captured_intent(intent) == truncate_text(intent, 2048) + + def test_sanitize_event_redacts_pii_from_intent(): event = { "user_intent": "Looking up orders for jane.doe@acme.com and calling +1 (415) 555-0142 about a refund.", @@ -1056,6 +1122,65 @@ def test_build_captured_mcp_parameters_only_strips_sdk_owned_model( assert captured["request"]["params"]["arguments"].get("llm_model") == expected_model +class _CountingDict(dict): + """A dict that counts the entries anything reads through ``items()``.""" + + reads = 0 + + def items(self): + for item in super().items(): + _CountingDict.reads += 1 + yield item + + +def _nested_counting_dicts(depth: int) -> _CountingDict: + nested = _CountingDict(leaf="x") + for _ in range(depth - 1): + nested = _CountingDict(child=nested) + return nested + + +@pytest.mark.parametrize( + ("arguments", "max_reads"), + [ + pytest.param( + _CountingDict({f"a{i}": i for i in range(10_000)}), 101, id="wide" + ), + pytest.param({"deep": _nested_counting_dicts(1000)}, 10, id="deep"), + pytest.param( + {"password": _CountingDict({f"a{i}": i for i in range(10)})}, + 0, + id="credential key", + ), + ], +) +def test_captured_parameters_read_only_what_the_limits_keep(arguments, max_reads): + _CountingDict.reads = 0 + build_captured_mcp_parameters({"params": {"arguments": arguments}}) + assert _CountingDict.reads <= max_reads + + +def test_captured_parameters_do_not_count_stripped_arguments_toward_the_limit(): + arguments = { + "context": "intent", + "conversation_id": "c", + **{f"a{i}": i for i in range(100)}, + } + captured = build_captured_mcp_parameters({"params": {"arguments": arguments}}) + assert captured["request"]["params"]["arguments"] == { + f"a{i}": i for i in range(100) + } + + +def test_captured_parameters_redact_a_credential_key_unread(): + arguments = {"password": {"nested": "hunter2"}, "q": "x"} + captured = build_captured_mcp_parameters({"params": {"arguments": arguments}}) + assert captured["request"]["params"]["arguments"] == { + "password": "[redacted]", + "q": "x", + } + + async def test_process_mcp_event_basic(): event = { "event_type": MCPAnalyticsEventType.MCP_TOOLS_CALL, diff --git a/posthog/test/mcp/test_unknown_tool.py b/posthog/test/mcp/test_unknown_tool.py index 8d1396d26..cc9dfa657 100644 --- a/posthog/test/mcp/test_unknown_tool.py +++ b/posthog/test/mcp/test_unknown_tool.py @@ -329,7 +329,7 @@ async def test_an_unknown_tool_name_is_cut_to_256_characters(build): server = build() instrument(server, client, MCPAnalyticsOptions(enable_conversation_id=False)) - await _response(server, "x" * 5000, {}) + await _response(server, "x" * 4000, {}) await flush_background() [unknown] = events_named(client, "$mcp_unknown_tool")