diff --git a/posthog/mcp/_sanitization.py b/posthog/mcp/_sanitization.py index fd547f41..13bc6994 100644 --- a/posthog/mcp/_sanitization.py +++ b/posthog/mcp/_sanitization.py @@ -27,6 +27,75 @@ re.IGNORECASE, ) +# PII redaction for the agent-narrated intent string only. $mcp_intent is free +# text the calling LLM writes into the injected `context` argument, so it can +# carry personal data the model read aloud despite being told not to. We redact +# well-defined *structured identifiers* — the kind regex can match with high +# precision. Person names and postal addresses are deliberately out of scope: +# they need an NER model that a client SDK cannot ship, and naive patterns would +# over-redact ordinary prose. Patterns are ordered so an earlier pass never eats +# digits a later pass needs (email before phone, IPs before phone, cards before +# the generic phone pass). See `redact_pii`. +# +# The `\d`/`\w`-based patterns are compiled with re.ASCII to match the JS +# semantics they are ported from: JS `\d`/`\w`/`\b` are ASCII-only, whereas +# Python's default is Unicode and would over-match (e.g. Unicode digits). +# +# Horizontal Unicode spaces (NBSP, narrow NBSP, ideographic space, ...) are what +# appear when text is copied from web pages or PDFs. `redact_pii` normalizes them +# to an ASCII space first so the separator-based card/phone/SSN candidates match +# them instead of leaking the identifier they group. +_UNICODE_SPACE_PATTERN = re.compile("[\u00a0\u1680\u2000-\u200a\u202f\u205f\u3000]") +# Quantifiers are bounded to RFC-ish limits (local-part <=64, domain <=255, +# TLD <=24) rather than open-ended `+`. Unbounded `+` here is quadratic: on a +# long run of local-part chars with no valid `.tld`, `sub` rescans from every +# start position. $mcp_intent is attacker-influenceable free text seen before +# truncation, so an open-ended pattern is a reachable event-loop stall. +_EMAIL_PATTERN = re.compile( + r"[A-Za-z0-9._%+-]{1,64}@[A-Za-z0-9.-]{1,255}\.[A-Za-z]{2,24}" +) +_IPV4_PATTERN = re.compile( + r"\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b", + re.ASCII, +) +# Four forms: full 8-group, `::`-terminated (`2001:db8::`), a middle `::` +# (`2001:db8::8a2e:1`), and a leading `::` (`::1`). The compressed branches use a +# `(?=13 digits optionally grouped by a single space, dot, dash, or +# slash. This only marks the numeric region; `_redact_card_in_match` then looks +# for the actual card as a run of whole separator-delimited groups that passes +# Luhn, so an adjacent field such as an expiry (`4111 1111 1111 1111 12/30`) is +# not absorbed into a failing check that would leak the card. +_CREDIT_CARD_CANDIDATE_PATTERN = re.compile(r"\b\d(?:[ ./-]?\d){12,}\b", re.ASCII) +# Matches each separator-delimited digit group inside a card candidate. +_DIGIT_GROUP_PATTERN = re.compile(r"\d+", re.ASCII) +# Phone matching is structural rather than "any 10-15 digits", so dates +# (`2024-01-15 12:30`) and dotted versions are not mistaken for numbers. Two +# forms: a North-American 3-3-4 grouping, and an international number that must +# start with `+` and a country code. The area code is either `(415)` (the +# separator after it is optional, so `(415)555-0142` matches) or a bare `415` +# that must be followed by a separator (space, dot, dash, or slash) — so a bare +# digit run is never taken for a phone number. +_PHONE_NANP_PATTERN = re.compile( + r"(? bool: return isinstance(value, dict) @@ -73,6 +142,79 @@ def _is_secret(word: str) -> bool: return False +def _passes_luhn(digits: str) -> bool: + total = 0 + double = False + for index in range(len(digits) - 1, -1, -1): + digit = ord(digits[index]) - 48 + if digit < 0 or digit > 9: + return False + if double: + digit *= 2 + if digit > 9: + digit -= 9 + total += digit + double = not double + return total % 10 == 0 + + +def _redact_card_in_match(match: re.Match[str]) -> str: + """Within a card candidate, redact every actual card — each run of whole + separator-delimited digit groups whose joined digits are 13-19 long and pass + Luhn — leaving any adjacent field (an expiry, a following ID) in place. For + each starting group it takes the longest such run, redacts it, then resumes + scanning after it so a second card in the same span (e.g. two numbers listed + together) is caught too. Checking group-aligned runs rather than arbitrary + digit windows keeps the false-positive rate at Luhn's own ~1-in-10, instead of + letting a chance-valid sub-window of an ordinary long ID trigger redaction.""" + text = match.group(0) + groups = [ + (m.group(0), m.start(), m.end()) for m in _DIGIT_GROUP_PATTERN.finditer(text) + ] + output = "" + cursor = 0 + first = 0 + while first < len(groups): + digits = "" + matched_last = -1 + for last in range(first, len(groups)): + digits += groups[last][0] + if len(digits) > 19: + break + if len(digits) >= 13 and _passes_luhn(digits): + matched_last = last + if matched_last >= 0: + output += text[cursor : groups[first][1]] + _REDACTED_VALUE + cursor = groups[matched_last][2] + first = matched_last + 1 + else: + first += 1 + return output + text[cursor:] + + +def redact_pii(value: Any) -> Any: + """Redact structured personal identifiers (emails, IP addresses, credit-card + numbers, US SSNs, and phone numbers) from a free-text string. Intended for the + agent-narrated $mcp_intent value only — not for structured tool parameters or + responses, where the same shapes are often legitimate data. Horizontal Unicode + spaces are first normalized to an ASCII space so copy-pasted identifiers still + match. Returns a new string; leaves the input's identifiers untouched when + nothing matches. Non-string input (e.g. a non-string ``user_intent`` reaching + the custom-event API) is returned unchanged, matching the pass-through + behavior of ``sanitize_captured_value`` for non-str values.""" + if not isinstance(value, str): + return value + result = _UNICODE_SPACE_PATTERN.sub(" ", value) + result = _EMAIL_PATTERN.sub(_REDACTED_VALUE, result) + result = _IPV4_PATTERN.sub(_REDACTED_VALUE, result) + result = _IPV6_PATTERN.sub(_REDACTED_VALUE, result) + result = _CREDIT_CARD_CANDIDATE_PATTERN.sub(_redact_card_in_match, result) + result = _US_SSN_PATTERN.sub(_REDACTED_VALUE, result) + result = _PHONE_NANP_PATTERN.sub(_REDACTED_VALUE, result) + result = _PHONE_INTL_PATTERN.sub(_REDACTED_VALUE, result) + return result + + def sanitize_captured_value(value: Any) -> Any: if value is None: return value @@ -106,9 +248,15 @@ def sanitize_event(event: Dict[str, Any]) -> Dict[str, Any]: result["parameters"] = sanitize_captured_value(result["parameters"]) # The intent comes straight from an agent-narrated `context` string, so it - # can contain a secret the LLM read aloud. Redact it like any other value. + # can contain a secret the LLM read aloud or personal data it narrated about + # the user. Redact it like any other captured value, then strip structured + # PII (emails, phone numbers, IPs, cards, SSNs) rather than shipping it raw + # as $mcp_intent. PII redaction is scoped to the intent only — structured + # tool parameters and responses often hold the same shapes as legitimate data. if result.get("user_intent") is not None: - result["user_intent"] = sanitize_captured_value(result["user_intent"]) + result["user_intent"] = redact_pii( + 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 diff --git a/posthog/mcp/constants.py b/posthog/mcp/constants.py index f999eae1..8c9f156b 100644 --- a/posthog/mcp/constants.py +++ b/posthog/mcp/constants.py @@ -18,13 +18,13 @@ INACTIVITY_TIMEOUT_IN_MINUTES = 30 DEFAULT_CONTEXT_PARAMETER_DESCRIPTION = ( - "Explain why you are calling this tool and how it fits into the user's overall goal. " - "This parameter is used for analytics and user intent tracking. YOU MUST provide 15-25 " - "words (count carefully). NEVER use first person ('I', 'we', 'you') - maintain " - "third-person perspective. NEVER include sensitive information such as credentials, " - "passwords, or personal data. Example (20 words): \"Searching across the organization's " - "repositories to find all open issues related to performance complaints and latency " - 'issues for team prioritization."' + "Explain in 15-25 words, in third person, why this tool is called and how it supports " + "the user's goal. For analytics only. You MUST describe only the abstract purpose of the " + "tool call. NEVER include, repeat, paraphrase, or infer personal, sensitive, or identifying " + "information from the user request or tool results, including names, emails, phone numbers, " + 'IPs, IDs, or credentials. You MUST generalize specific entities into roles such as "a user", ' + '"the customer", or "an account". Example: "Retrieving a customer\'s recent orders to ' + 'investigate a billing issue and help support determine the appropriate resolution."' ) DEFAULT_CONVERSATION_ID_DESCRIPTION = ( diff --git a/posthog/test/mcp/test_pipeline.py b/posthog/test/mcp/test_pipeline.py index f57c1572..e8766b79 100644 --- a/posthog/test/mcp/test_pipeline.py +++ b/posthog/test/mcp/test_pipeline.py @@ -2,6 +2,8 @@ from datetime import datetime, timezone +import pytest + from posthog.mcp.constants import ( POSTHOG_MCP_ANALYTICS_SOURCE, PostHogMCPAnalyticsEvent, @@ -13,6 +15,7 @@ from posthog.mcp._posthog_events import build_posthog_capture_events from posthog.mcp._sanitization import ( build_captured_mcp_parameters, + redact_pii, sanitize_captured_value, sanitize_event, ) @@ -108,6 +111,242 @@ def test_sanitize_does_not_mutate_input(): assert event["parameters"]["token"] == "phx_aaaaaaaaaaaaaaaaaaaaaaaa" +# --- intent PII redaction ---------------------------------------------------- + +_NBSP = "\u00a0" +_NNBSP = "\u202f" + + +@pytest.mark.parametrize( + "label, text, expected", + [ + ( + "an email address", + "Looking up orders for jane.doe@acme.co.uk before refunding.", + "Looking up orders for [redacted] before refunding.", + ), + ( + "an email with a maximal 64-char local part", + f"from {'a' * 64}@example.com now", + "from [redacted] now", + ), + ( + "an IPv4 address", + "Blocking traffic from 203.0.113.42 after abuse.", + "Blocking traffic from [redacted] after abuse.", + ), + ( + "an IPv6 address with a middle ::", + "Tracing request from 2001:db8::ff00:42:8329 across the mesh.", + "Tracing request from [redacted] across the mesh.", + ), + ( + "an IPv6 address ending in ::", + "Routing host 2001:db8:: for now.", + "Routing host [redacted] for now.", + ), + ( + "an IPv6 loopback ::1", + "Health check from ::1 passed.", + "Health check from [redacted] passed.", + ), + ( + "a NANP phone with dashes", + "Reference ticket for number 415-555-0142 escalation.", + "Reference ticket for number [redacted] escalation.", + ), + ( + "a NANP phone with slashes", + "Call the customer on 415/555/0142 today.", + "Call the customer on [redacted] today.", + ), + ( + "a NANP phone with parens and +1", + "Calling back on +1 (415) 555-0142 about the outage.", + "Calling back on [redacted] about the outage.", + ), + ( + "a NANP phone with a parenthesized area code and no following separator", + "Reaching them at (415)555-0142 today.", + "Reaching them at [redacted] today.", + ), + ( + "an international phone with a + country code", + "Ring +44 (0) 20 7946 0958 please.", + "Ring [redacted] please.", + ), + ( + "a phone grouped with NBSP spaces", + f"Calling the customer on 415{_NNBSP}555{_NNBSP}0132 today.", + "Calling the customer on [redacted] today.", + ), + ( + "a Luhn-valid card with spaces", + "Charging the saved card 4111 1111 1111 1111 for the renewal.", + "Charging the saved card [redacted] for the renewal.", + ), + ( + "a card grouped with dots", + "Charging card 4111.1111.1111.1111 today.", + "Charging card [redacted] today.", + ), + ( + "a card grouped with slashes", + "Charging card 4111/1111/1111/1111 today.", + "Charging card [redacted] today.", + ), + ( + "a card grouped with NBSP spaces", + f"Charging card 4111{_NBSP}1111{_NBSP}1111{_NBSP}1111 now.", + "Charging card [redacted] now.", + ), + ( + "a card without absorbing an adjacent expiry field", + "Charging card 4111 1111 1111 1111 12/30 for renewal.", + "Charging card [redacted] 12/30 for renewal.", + ), + ( + "every card when two appear in one span", + "Moving funds 4111 1111 1111 1111 5555 5555 5555 4444 now.", + "Moving funds [redacted] [redacted] now.", + ), + ( + "an SSN with dashes", + "Verifying SSN 123-45-6789 for the claim.", + "Verifying SSN [redacted] for the claim.", + ), + ( + "an SSN with spaces", + "Verifying SSN 123 45 6789 for the claim.", + "Verifying SSN [redacted] for the claim.", + ), + ( + "an SSN with dots", + "Verifying SSN 123.45.6789 for the claim.", + "Verifying SSN [redacted] for the claim.", + ), + ], +) +def test_redact_pii_redacts(label, text, expected): + assert redact_pii(text) == expected + + +@pytest.mark.parametrize( + "label, text", + [ + ( + "a bare numeric identifier without grouping", + "Fetching record 4155550142 from the ledger service.", + ), + ( + "a bare 9-digit number that is not an SSN", + "Looking up record 123456789 in the ledger.", + ), + ( + "a Luhn-invalid long digit run", + "Correlating with order 1234567890123456 in the warehouse.", + ), + ( + "a date and time that resembles a phone number", + "Deploying at 2024-01-15 12:30 UTC after review.", + ), + ( + "a dotted version/build number", + "Upgrading to build 2024.11.05.1830 for the team.", + ), + ( + "a C++ scope expression that resembles IPv6", + "Calling std::bad and std::vector helpers for the team.", + ), + ( + "ordinary prose with versions, dates, and code separators", + "Upgrading to v1.2.3 on 2024-01-15 by refactoring std::vector usage.", + ), + ( + "prose with no personal data", + "Searching the organization repositories to prioritize open performance issues.", + ), + ], +) +def test_redact_pii_leaves_untouched(label, text): + assert redact_pii(text) == text + + +def test_redact_pii_redacts_multiple_identifiers(): + assert ( + redact_pii( + "Emailing bob@example.com and calling +1-202-555-0170 about the issue." + ) + == "Emailing [redacted] and calling [redacted] about the issue." + ) + + +def test_redact_pii_is_not_quadratic_on_pathological_input(): + # A 100k-char run with an `@` but no valid TLD is the worst case for an + # unbounded email pattern. With bounded quantifiers this stays linear; a + # regression to `+` would blow up the runtime instead. + import time + + pathological = f"{'a' * 50_000}@{'a' * 50_000}" + start = time.monotonic() + assert redact_pii(pathological) == pathological + assert time.monotonic() - start < 1.0 + + +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.", + } + result = sanitize_event(event) + assert ( + result["user_intent"] + == "Looking up orders for [redacted] and calling [redacted] about a refund." + ) + + +def test_sanitize_event_composes_pii_and_token_redaction_on_intent(): + event = { + "user_intent": "Rotating token phc_123456789012345678901234567890 for user carol@example.org." + } + result = sanitize_event(event) + assert result["user_intent"] == "Rotating token [redacted] for user [redacted]." + + +def test_sanitize_event_does_not_redact_pii_shapes_from_structured_data(): + event = { + "user_intent": "Enriching the profile for dave@example.com from the CRM.", + "parameters": {"email": "dave@example.com", "ip": "203.0.113.42"}, + "response": { + "content": [ + {"type": "text", "text": "Matched dave@example.com at 203.0.113.42."} + ] + }, + } + result = sanitize_event(event) + assert result["user_intent"] == "Enriching the profile for [redacted] from the CRM." + # Structured tool data keeps the same shapes: they are often legitimate here. + assert result["parameters"] == {"email": "dave@example.com", "ip": "203.0.113.42"} + assert ( + result["response"]["content"][0]["text"] + == "Matched dave@example.com at 203.0.113.42." + ) + + +def test_sanitize_event_does_not_mutate_intent(): + original = "Paging on-call about ticket from user@example.com right now." + event = {"user_intent": original} + sanitize_event(event) + assert event["user_intent"] == original + + +def test_sanitize_event_passes_through_non_string_intent(): + # user_intent is typed as Any on the custom-event API (Event = Dict[str, Any]), + # so a non-string value must not raise; it should pass through unchanged, same + # as sanitize_captured_value does for other non-str/list/dict values. + result = sanitize_event({"user_intent": 123}) + assert result["user_intent"] == 123 + + # --- truncation --------------------------------------------------------------