From a40e61ebe0b6904877b6b9e70f383a0ed489dd64 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Fri, 6 Nov 2026 16:52:26 -0300 Subject: [PATCH 1/7] feat(mcp): redact personal data in tool arguments, results and errors by default We told OpenAI's plugin directory review that the MCP SDKs redact personal data in tool payloads by default. sdk-specs#190 specifies it; this is the JS reference port. What: - @posthog/core: redactPayloadPii, the intent pass narrowed for payloads (public, unglued IPv4 only; phones only with +/00/011 or a parenthesized area code; a bare JSON-number card is written "[redacted]" quoted). sanitizeCapturedValue runs it, when redactPayloadPersonalData is set, inside the existing redactCapturedHead cut between the credential and URL passes, on object keys (collisions keep the key that sorts first by code point), and on Luhn-valid card numbers. - @posthog/mcp: new redactPayloadPersonalData option (default true) on instrument() and PostHogMCP, applied to $mcp_parameters, $mcp_response, exception values (cut at the 2,048 error limit) and resource read URIs. $mcp_input_keys drops names the token, URL or payload pass would change. $mcp_feedback_ JSON text is sanitized as free text, keys included. Tested: - pnpm --filter @posthog/mcp test:unit: 1049 passed (the new tests failed first) - pnpm --filter @posthog/mcp lint, check-types: clean - pnpm --filter @posthog/mcp build && test:integration: green (2 expected failures) - packages/core test:unit: 2356 passed; browser webmcp.test.ts: 46 passed Reviewer notes: - Per the spec's order (personal data before URLs), a URL with user:pass@host reads as an email and is redacted whole, read URIs included. Two existing expectations changed accordingly. - @posthog/core is listed in the changeset: mcp depends on it unbundled. --- .changeset/mcp-payload-personal-data.md | 6 + packages/core/src/privacy.ts | 126 +++++++++++++++++- .../src/__tests__/e2e-sanitization.test.ts | 104 +++++++++++++++ packages/mcp/src/__tests__/feedback.test.ts | 3 +- .../mcp/src/__tests__/mcp-payloads.test.ts | 55 ++++++++ .../mcp/src/__tests__/posthog-mcp.test.ts | 15 +++ .../mcp/src/__tests__/sanitization.test.ts | 121 +++++++++++++---- .../string-method-registration.test.ts | 5 +- packages/mcp/src/__tests__/tool-input.test.ts | 16 +++ packages/mcp/src/extensions/capture.ts | 1 + packages/mcp/src/extensions/feedback.ts | 2 +- packages/mcp/src/extensions/mcp-payloads.ts | 7 +- packages/mcp/src/extensions/posthog-mcp.ts | 9 +- packages/mcp/src/extensions/sanitization.ts | 57 +++++--- packages/mcp/src/extensions/sink.ts | 6 +- packages/mcp/src/extensions/tool-input.ts | 7 +- packages/mcp/src/extensions/truncation.ts | 2 +- packages/mcp/src/types.ts | 11 ++ 18 files changed, 490 insertions(+), 63 deletions(-) create mode 100644 .changeset/mcp-payload-personal-data.md diff --git a/.changeset/mcp-payload-personal-data.md b/.changeset/mcp-payload-personal-data.md new file mode 100644 index 0000000000..69e3016e2a --- /dev/null +++ b/.changeset/mcp-payload-personal-data.md @@ -0,0 +1,6 @@ +--- +'@posthog/mcp': minor +'@posthog/core': minor +--- + +Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers, US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON number becomes the string `"[redacted]"`, so a result's JSON text stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version after a space, `:`, or `@` (`node:22.11.0.3`), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. diff --git a/packages/core/src/privacy.ts b/packages/core/src/privacy.ts index 4e4ec322fd..45230e717e 100644 --- a/packages/core/src/privacy.ts +++ b/packages/core/src/privacy.ts @@ -6,6 +6,8 @@ export interface TextSanitizationOptions { maxStringLength: number truncationSuffix: string + /** Applies {@link redactPayloadPii} to captured strings and object keys, and redacts numbers that pass as card numbers. */ + redactPayloadPersonalData?: boolean } const REDACTED_VALUE = '[redacted]' @@ -121,6 +123,24 @@ const DIGIT_GROUP_PATTERN = /\d+/g const PHONE_NANP_PATTERN = /(^|[^\w+])((?:\+?1[ ./-]?)?(?:\(\d{3}\)[ ./-]?|\d{3}[ ./-])\d{3}[ ./-]\d{4})(?![\w])/g const PHONE_INTL_PATTERN = /(^|[^\w])(\+\d{1,3}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g +// The payload pass is narrower because tool payloads carry identifiers that only +// look like personal data: versions glued to a letter, digit, or `.`, internal +// addresses, and `415-555-0142`-shaped order numbers. +const PAYLOAD_IPV4_PATTERN = + /(^|[^A-Za-z0-9.])((?:(?: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))(?!\d|\.\d)/g +const NON_PUBLIC_IPV4_RANGES = [ + '127.0.0.0/8', + '10.0.0.0/8', + '172.16.0.0/12', + '192.168.0.0/16', + '169.254.0.0/16', + '0.0.0.0/32', +] +const PHONE_PARENTHESIZED_PATTERN = /(^|[^\w+])((?:\+?1[ ./-]?)?\(\d{3}\)[ ./-]?\d{3}[ ./-]\d{4})(?![\w])/g +const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g +const JSON_VALUE_START_PATTERN = /[:,[]\s*$/ +const JSON_VALUE_END_PATTERN = /^\s*[,}\]]/ + type JsonRecord = Record function isRecord(value: unknown): value is JsonRecord { @@ -556,9 +576,10 @@ function headLength(value: string, scannedHeadLength: number): number { } function sanitizeString(value: string, options: TextSanitizationOptions): string { + const redactPersonalData = options.redactPayloadPersonalData ? redactPayloadPii : (head: string) => head return isBinaryBlob(value) ? BINARY_REDACTED_VALUE - : redactCapturedHead(value, (head) => redactUrls(redactCredentials(head)), options) + : redactCapturedHead(value, (head) => redactUrls(redactPersonalData(redactCredentials(head))), options) } /** @@ -660,8 +681,68 @@ export function redactPii(value: string): string { return result } +function ipv4ToNumber(address: string): number { + return address.split('.').reduce((value, octet) => value * 256 + Number(octet), 0) +} + +function isPublicIpv4(address: string): boolean { + const value = ipv4ToNumber(address) + return !NON_PUBLIC_IPV4_RANGES.some((range) => { + const [network, prefixLength] = range.split('/') + const size = 2 ** (32 - Number(prefixLength)) + return Math.floor(value / size) === ipv4ToNumber(network) / size + }) +} + +/** + * {@link redactCardInMatch}, writing a card that stands as a bare JSON number as + * the string `"[redacted]"`, so a tool result's JSON text copy stays valid JSON. + */ +function redactCardKeepingJson(match: string, offset: number, text: string): string { + const redacted = redactCardInMatch(match) + const end = offset + match.length + return redacted === REDACTED_VALUE && + JSON_VALUE_START_PATTERN.test(text.slice(Math.max(0, offset - 64), offset)) && + JSON_VALUE_END_PATTERN.test(text.slice(end, end + 64)) + ? `"${REDACTED_VALUE}"` + : redacted +} + +function isCardNumber(value: number): boolean { + const digits = String(value) + return /^\d{13,19}$/.test(digits) && passesLuhn(digits) +} + +/** + * {@link redactPii} narrowed for tool arguments, results, and errors: an IPv4 + * address only when public and not glued to a version, and a phone number only + * with an international prefix or a parenthesized area code. A string with + * nothing to redact comes back as received, Unicode spaces included. + */ +export function redactPayloadPii(value: string): string { + const normalized = value.replace(UNICODE_SPACE_PATTERN, ' ') + let result = normalized.replace(EMAIL_PATTERN, REDACTED_VALUE) + result = result.replace(PAYLOAD_IPV4_PATTERN, (match, prefix: string, address: string) => + isPublicIpv4(address) ? prefix + REDACTED_VALUE : match + ) + result = result.replace(IPV6_PATTERN, `$1${REDACTED_VALUE}`) + result = result.replace(CREDIT_CARD_CANDIDATE_PATTERN, redactCardKeepingJson) + result = result.replace(US_SSN_PATTERN, REDACTED_VALUE) + result = result.replace(PHONE_PARENTHESIZED_PATTERN, `$1${REDACTED_VALUE}`) + result = result.replace(PAYLOAD_PHONE_INTL_PATTERN, `$1${REDACTED_VALUE}`) + return result === normalized ? value : result +} + export function sanitizeCapturedValue(value: unknown, options: TextSanitizationOptions): unknown { - return sanitizeValueWith(value, (stringValue) => sanitizeString(stringValue, options)) + const string = (stringValue: string): string => sanitizeString(stringValue, options) + if (!options.redactPayloadPersonalData) { + return sanitizeValueWith(value, { string }) + } + return sanitizeValueWith(value, { + string, + key: (key) => redactCapturedHead(key, redactPayloadPii, options), + number: (number) => (isCardNumber(number) ? REDACTED_VALUE : number), + }) } /** @@ -670,20 +751,44 @@ export function sanitizeCapturedValue(value: unknown, options: TextSanitizationO * values get structured-PII redaction in the load-bearing order too. */ export function sanitizeFreeTextValue(value: unknown, options: TextSanitizationOptions): unknown { - return sanitizeValueWith(value, (stringValue) => sanitizeFreeText(stringValue, options)) + return sanitizeValueWith(value, { string: (stringValue) => sanitizeFreeText(stringValue, options) }) +} + +/** What a walk rewrites. Object keys and numbers are kept as received unless a pass is given for them. */ +interface ValuePasses { + string: (value: string) => string + key?: (key: string) => string + number?: (value: number) => unknown } -function sanitizeValueWith(value: unknown, sanitizeStringFn: (value: string) => string): unknown { +/** Orders by Unicode code point, which `<` does not for characters outside the Basic Multilingual Plane. */ +function compareCodePoints(left: string, right: string): number { + for (let index = 0; index < left.length && index < right.length;) { + const leftPoint = left.codePointAt(index)! + const rightPoint = right.codePointAt(index)! + if (leftPoint !== rightPoint) { + return leftPoint - rightPoint + } + index += leftPoint > 0xffff ? 2 : 1 + } + return left.length - right.length +} + +function sanitizeValueWith(value: unknown, passes: ValuePasses): unknown { if (value == null) { return value } if (typeof value === 'string') { - return sanitizeStringFn(value) + return passes.string(value) + } + + if (typeof value === 'number' && passes.number) { + return passes.number(value) } if (Array.isArray(value)) { - return value.map((item) => sanitizeValueWith(item, sanitizeStringFn)) + return value.map((item) => sanitizeValueWith(item, passes)) } if (value instanceof Date) { @@ -695,8 +800,15 @@ function sanitizeValueWith(value: unknown, sanitizeStringFn: (value: string) => } const result: JsonRecord = {} + const receivedKeys = new Map() for (const [key, nestedValue] of Object.entries(value)) { - result[key] = shouldRedactKey(key) ? REDACTED_VALUE : sanitizeValueWith(nestedValue, sanitizeStringFn) + const sanitizedKey = passes.key ? passes.key(key) : key + const keptKey = receivedKeys.get(sanitizedKey) + if (keptKey !== undefined && compareCodePoints(keptKey, key) <= 0) { + continue + } + receivedKeys.set(sanitizedKey, key) + result[sanitizedKey] = shouldRedactKey(key) ? REDACTED_VALUE : sanitizeValueWith(nestedValue, passes) } return result } diff --git a/packages/mcp/src/__tests__/e2e-sanitization.test.ts b/packages/mcp/src/__tests__/e2e-sanitization.test.ts index d8f58b71ca..0eacc7b676 100644 --- a/packages/mcp/src/__tests__/e2e-sanitization.test.ts +++ b/packages/mcp/src/__tests__/e2e-sanitization.test.ts @@ -268,3 +268,107 @@ describe('E2E Sanitization - real MCP tool calls', () => { } }) }) + +describe('E2E payload personal data', () => { + const accounts = '{"accounts":{"ada@example.com":{"plan":"pro"}},"card":4111111111111111,"count":3}' + + type Properties = Record + const text = (value: unknown) => JSON.stringify(value) + const firstText = (properties: Properties) => + (properties.$mcp_response as { content: { text: string }[] }).content[0].text + + it.each([ + { + scenario: 'personal data in arguments and results is redacted by default', + options: {}, + args: { email: 'ada@example.com', limit: 5 }, + result: async () => ({ + content: [{ type: 'text' as const, text: 'Customer ada@example.com last seen from 203.0.113.7' }], + }), + check: (call: Properties) => { + expect(text(call.$mcp_parameters)).not.toContain('ada@example.com') + expect((call.$mcp_parameters as any).request.params.arguments).toEqual({ email: '[redacted]', limit: 5 }) + expect(text(call.$mcp_response)).not.toContain('ada@example.com') + expect(text(call.$mcp_response)).not.toContain('203.0.113.7') + }, + }, + { + scenario: 'a personal key and a card number in a result are redacted and its JSON stays valid', + options: {}, + args: {}, + result: async () => ({ + content: [{ type: 'text' as const, text: accounts }], + structuredContent: JSON.parse(accounts), + }), + check: (call: Properties) => { + expect(text(call.$mcp_response)).not.toContain('ada@example.com') + expect(text(call.$mcp_response)).not.toContain('4111111111111111') + expect(firstText(call)).toBe('{"accounts":{"[redacted]":{"plan":"pro"}},"card":"[redacted]","count":3}') + }, + }, + { + scenario: 'identifiers that only look like personal data are kept in payloads', + options: {}, + args: {}, + result: async () => ({ + content: [ + { + type: 'text' as const, + text: 'Order 415-555-0142 built with v22.11.0.3, db at 127.0.0.1:5432, client ip:203.0.113.7, support +1 415 555 0142', + }, + ], + }), + check: (call: Properties) => { + expect(firstText(call)).toBe( + 'Order 415-555-0142 built with v22.11.0.3, db at 127.0.0.1:5432, client ip:[redacted], support [redacted]' + ) + }, + }, + { + scenario: 'turning the option off keeps personal data in payloads but not in intent', + options: { redactPayloadPersonalData: false }, + args: { email: 'ada@example.com', context: 'look up ada@example.com' }, + result: async () => ({ content: [{ type: 'text' as const, text: 'Customer ada@example.com' }] }), + check: (call: Properties) => { + expect(text(call.$mcp_parameters)).toContain('ada@example.com') + expect(text(call.$mcp_response)).toContain('ada@example.com') + expect(call.$mcp_intent).toBe('look up [redacted]') + }, + }, + { + scenario: 'personal data in error text and exceptions is redacted by default', + options: {}, + args: {}, + result: async (): Promise => { + throw new Error('No account for ada@example.com') + }, + check: (call: Properties, everything: unknown) => { + expect(call.$mcp_error_message).toBe('No account for [redacted]') + expect(text(everything)).not.toContain('ada@example.com') + }, + }, + ])('$scenario', async ({ options, args, result, check }) => { + const { server, client, cleanup } = await setupTestServerAndClient() + const eventCapture = new EventCapture() + await eventCapture.start() + + try { + const { instrument } = await import('../index') + await instrument(server, fakePostHog(), { enableConversationId: false, ...options }) + server.registerTool( + 'lookup', + { inputSchema: { email: z.string().optional(), limit: z.number().optional() } }, + result as never + ) + + await client.request({ method: 'tools/call', params: { name: 'lookup', arguments: args } }, CallToolResultSchema) + await new Promise((resolve) => setTimeout(resolve, 100)) + + const [call] = eventCapture.findCapturesByEvent('$mcp_tool_call') + check(call.properties, eventCapture.getCaptures()) + } finally { + await eventCapture.stop() + await cleanup() + } + }) +}) diff --git a/packages/mcp/src/__tests__/feedback.test.ts b/packages/mcp/src/__tests__/feedback.test.ts index af817d3b9e..cdd5ab348f 100644 --- a/packages/mcp/src/__tests__/feedback.test.ts +++ b/packages/mcp/src/__tests__/feedback.test.ts @@ -496,13 +496,14 @@ describe('collectFeedback (send_feedback virtual tool)', () => { feedback_type: 'issue', summary: 'A tool failed.', reporter: 'jane@example.com', - meta: { contact: 'john@example.com' }, + meta: { contact: 'john@example.com', 'ada@example.com': 'vip' }, }) await new Promise((r) => setTimeout(r, 50)) const p = capture.findCapturesByEvent(PostHogMCPAnalyticsEvent.Feedback)[0].properties expect(p.$mcp_feedback_reporter).toBe('[redacted]') expect(p.$mcp_feedback_meta).not.toContain('john@example.com') + expect(p.$mcp_feedback_meta).not.toContain('ada@example.com') expect(p.$mcp_feedback_meta).toContain('[redacted]') await capture.stop() diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index c156e4a6fd..ffc590aa50 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -1,5 +1,6 @@ import { buildCapturedMcpParameters, + redactPayloadPii, redactPii, sanitizeCapturedValue, sanitizeFreeTextValue, @@ -528,6 +529,60 @@ describe('redactPii', () => { }) }) +describe('redactPayloadPii', () => { + it.each([ + ['an email address', 'Customer ada@example.com', 'Customer [redacted]'], + ['a public IPv4 after a space', 'last seen from 203.0.113.7', 'last seen from [redacted]'], + ['a public IPv4 after a colon', 'client ip:203.0.113.7', 'client ip:[redacted]'], + ['a public IPv4 after an @', 'user@203.0.113.7', 'user@[redacted]'], + ['a public IPv4 in a URL', 'http://203.0.113.7/', 'http://[redacted]/'], + ['a public IPv4 just outside 172.16.0.0/12', 'peer 172.32.0.1', 'peer [redacted]'], + ['a four-part version after a colon, which reads like an address', 'node:22.11.0.3', 'node:[redacted]'], + ['an IPv6 address', 'from 2001:db8::ff00:42:8329 now', 'from [redacted] now'], + ['a phone with + and a country code', 'support +1 415 555 0142', 'support [redacted]'], + ['a phone with the 00 prefix', 'call 0044 20 7946 0958 today', 'call [redacted] today'], + ['a phone with the 011 prefix', 'call 011 44 20 7946 0958 today', 'call [redacted] today'], + ['a phone with a parenthesized area code', 'call (415) 555-0142 today', 'call [redacted] today'], + ['a phone with +1 and a parenthesized area code', 'call +1 (415) 555-0142 today', 'call [redacted] today'], + ['a Luhn-valid card', 'card 4111 1111 1111 1111 on file', 'card [redacted] on file'], + ['a US SSN', 'SSN 123-45-6789', 'SSN [redacted]'], + [ + 'a card written as a JSON number, quoted', + '{"card":4111111111111111,"count":3}', + '{"card":"[redacted]","count":3}', + ], + ['a card in a JSON array, quoted', '[ 4111111111111111 ]', '[ "[redacted]" ]'], + ['a card inside a JSON string, unquoted', '{"note":"4111111111111111"}', '{"note":"[redacted]"}'], + ])('redacts %s', (_label, input, expected) => { + expect(redactPayloadPii(input)).toBe(expected) + }) + + it.each([ + ['a US phone without a prefix or parentheses', 'Order 415-555-0142'], + ['a version after a letter', 'built with v22.11.0.3'], + ['a five-part version', 'release 1.2.3.4.5'], + ['an address followed by a digit-led field', 'release 203.0.113.7.4'], + ['a public IPv4 directly after a letter', 'IP203.0.113.7'], + ['a loopback address with a port', 'db at 127.0.0.1:5432'], + ['a 10/8 address', 'host 10.20.30.40'], + ['a 172.16/12 address', 'host 172.31.255.1'], + ['a 192.168/16 address', 'host 192.168.1.10'], + ['a link-local address', 'metadata 169.254.169.254'], + ['the unspecified address', 'bind 0.0.0.0'], + ['a Luhn-invalid long digit run', 'order 1234567890123456'], + ['a date and time', 'at 2024-01-15 12:30 UTC'], + ])('keeps %s', (_label, input) => { + expect(redactPayloadPii(input)).toBe(input) + }) + + it.each([ + ['a US phone without a prefix', 'Order 415-555-0142'], + ['a loopback address', 'db at 127.0.0.1'], + ])('leaves the intent pass redacting %s', (_label, input) => { + expect(redactPii(input)).not.toBe(input) + }) +}) + describe('sanitizing strings longer than truncation keeps', () => { const WINDOW = 131_072 const token = 'phc_123456789012345678901234567890' diff --git a/packages/mcp/src/__tests__/posthog-mcp.test.ts b/packages/mcp/src/__tests__/posthog-mcp.test.ts index 2b3b5d3548..c052751221 100644 --- a/packages/mcp/src/__tests__/posthog-mcp.test.ts +++ b/packages/mcp/src/__tests__/posthog-mcp.test.ts @@ -253,6 +253,21 @@ describe('PostHogMCP', () => { } }) + it.each([ + [{}, '[redacted]'], + [{ redactPayloadPersonalData: false }, 'ada@example.com'], + ])('applies the payload personal data pass to manual captures with options %j', async (options, email) => { + const client = newClient(options) + try { + client.captureToolCall({ toolName: 'lookup', distinctId: 'user-123', parameters: { email: 'ada@example.com' } }) + await tick() + + expect(onlyCapture(PostHogMCPAnalyticsEvent.ToolCall).properties.$mcp_parameters).toEqual({ email }) + } finally { + await client.shutdown() + } + }) + it.each([ { name: 'trims a direct model value', diff --git a/packages/mcp/src/__tests__/sanitization.test.ts b/packages/mcp/src/__tests__/sanitization.test.ts index e0586ea22c..ba6d79b803 100644 --- a/packages/mcp/src/__tests__/sanitization.test.ts +++ b/packages/mcp/src/__tests__/sanitization.test.ts @@ -492,14 +492,17 @@ describe('sanitizeEvent - exception values', () => { describe('sanitizeEvent - resource name', () => { // `$identify` names its request the same way a read does, so gating redaction // on the read event type used to publish the raw address alongside the person. - it.each(['mcp:resources/read', 'identify', '$exception'])( - 'redacts credentials from a %s resource name', - (eventType) => { - const event = makeEvent({ eventType, resourceName: 'https://fakeuser:fakepass@example.com/guide' }) + // On a read the personal data pass runs first and takes `fakepass@example.com` + // for an email, so the URL no longer parses and is redacted whole. + it.each([ + ['mcp:resources/read', '[redacted]'], + ['identify', 'https://%5Bredacted%5D@example.com/guide'], + ['$exception', 'https://%5Bredacted%5D@example.com/guide'], + ])('redacts credentials from a %s resource name', (eventType, expected) => { + const event = makeEvent({ eventType, resourceName: 'https://fakeuser:fakepass@example.com/guide' }) - expect(sanitizeEvent(event).resourceName).toBe('https://%5Bredacted%5D@example.com/guide') - } - ) + expect(sanitizeEvent(event).resourceName).toBe(expected) + }) }) describe('sanitizeEvent - intent PII redaction', () => { @@ -558,22 +561,25 @@ describe('sanitizeEvent - intent PII redaction', () => { ) }) - it('does not redact the same PII shapes from structured parameters or responses', () => { - const event = makeEvent({ - userIntent: '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.' }] }, - }) - - const result = sanitizeEvent(event) - - expect(result.userIntent).toBe('Enriching the profile for [redacted] from the CRM.') - // Structured tool data keeps the same shapes: they are often legitimate here. - expect(result.parameters).toEqual({ email: 'dave@example.com', ip: '203.0.113.42' }) - expect((result.response as { content: { text: string }[] }).content[0].text).toBe( - 'Matched dave@example.com at 203.0.113.42.' - ) - }) + it.each([ + [true, '[redacted]', 'Matched [redacted] at [redacted].'], + [false, 'dave@example.com', 'Matched dave@example.com at 203.0.113.42.'], + ])( + 'redacts the intent whatever the payload option, and payloads only when it is on (%s)', + (redactPayloadPersonalData, email, text) => { + const event = makeEvent({ + userIntent: 'Enriching the profile for dave@example.com from the CRM.', + parameters: { email: 'dave@example.com', limit: 5 }, + response: { content: [{ type: 'text', text: 'Matched dave@example.com at 203.0.113.42.' }] }, + }) + + const result = sanitizeEvent(event, { redactPayloadPersonalData }) + + expect(result.userIntent).toBe('Enriching the profile for [redacted] from the CRM.') + expect(result.parameters).toEqual({ email, limit: 5 }) + expect((result.response as { content: { text: string }[] }).content[0].text).toBe(text) + } + ) it('does not mutate the original intent string', () => { const original = 'Paging on-call about ticket from user@example.com right now.' @@ -585,6 +591,75 @@ describe('sanitizeEvent - intent PII redaction', () => { }) }) +describe('sanitizeEvent - payload personal data', () => { + const accounts = '{"accounts":{"ada@example.com":{"plan":"pro"}},"card":4111111111111111,"count":3}' + + it('redacts a personal key and a card number and keeps the JSON text copy valid', () => { + const event = makeEvent({ + response: { content: [{ type: 'text', text: accounts }], structuredContent: JSON.parse(accounts) }, + }) + + const response = sanitizeEvent(event).response as { content: { text: string }[]; structuredContent: unknown } + + const redacted = { accounts: { '[redacted]': { plan: 'pro' } }, card: '[redacted]', count: 3 } + expect(response.structuredContent).toEqual(redacted) + expect(JSON.parse(response.content[0].text)).toEqual(redacted) + }) + + it.each([ + [{ 'bob@example.com': 'second', 'ada@example.com': 'first' }], + [{ 'ada@example.com': 'first', 'bob@example.com': 'second' }], + ])('keeps the entry whose received key sorts first when redacted keys collide (%j)', (parameters) => { + expect(sanitizeEvent(makeEvent({ parameters })).parameters).toEqual({ '[redacted]': 'first' }) + }) + + it.each([ + ['a non-card number', 1234567890123456], + ['a small number', 42], + ['a boolean', true], + ])('keeps %s', (_label, value) => { + expect(sanitizeEvent(makeEvent({ parameters: { value } })).parameters).toEqual({ value }) + }) + + it.each([ + [true, 'No account for [redacted]', 'file:///users/[redacted]/notes.md'], + [false, 'No account for ada@example.com', 'file:///users/ada@example.com/notes.md'], + ])( + 'applies to exception values and a read resource URI only when on (%s)', + (redactPayloadPersonalData, value, uri) => { + const event = makeEvent({ + eventType: 'mcp:resources/read', + resourceName: 'file:///users/ada@example.com/notes.md', + isError: true, + error: { + $exception_list: [ + { type: 'Error', value: 'No account for ada@example.com', mechanism: { type: 'generic', handled: true } }, + ], + $exception_level: 'error', + }, + }) + + const result = sanitizeEvent(event, { redactPayloadPersonalData }) + + expect(result.error?.$exception_list[0].value).toBe(value) + expect(result.resourceName).toBe(uri) + } + ) + + it('redacts an address split by the exception value limit whole', () => { + const value = `${'x'.repeat(2_040)} ada@example.com ${'y'.repeat(5_000)}` + const event = makeEvent({ + isError: true, + error: { + $exception_list: [{ type: 'Error', value, mechanism: { type: 'generic', handled: true } }], + $exception_level: 'error', + }, + }) + + expect(sanitizeEvent(event).error?.$exception_list[0].value).not.toContain('ada@') + }) +}) + describe('sanitizeEvent - integration', () => { it('should sanitize both parameters and response in a full event', () => { const largeBase64 = makeLargeBase64() diff --git a/packages/mcp/src/__tests__/string-method-registration.test.ts b/packages/mcp/src/__tests__/string-method-registration.test.ts index a36a4ab460..1886e5c57a 100644 --- a/packages/mcp/src/__tests__/string-method-registration.test.ts +++ b/packages/mcp/src/__tests__/string-method-registration.test.ts @@ -178,8 +178,9 @@ describe('setRequestHandler with string method names (MCP SDK v2)', () => { it.each([ ['file:///guide.md', 'file:///guide.md', false], - ['https://fakeuser:fakepass@example.com/guide', 'https://%5Bredacted%5D@example.com/guide', false], - ['https://fakeuser:fakepass@example.com/guide', 'https://%5Bredacted%5D@example.com/guide', true], + // The personal data pass reads `fakepass@example.com` as an email before the URL pass runs. + ['https://fakeuser:fakepass@example.com/guide', '[redacted]', false], + ['https://fakeuser:fakepass@example.com/guide', '[redacted]', true], [ 'https://example.com/guide?token=fakesecret&chapter=intro', 'https://example.com/guide?token=%5Bredacted%5D&chapter=intro', diff --git a/packages/mcp/src/__tests__/tool-input.test.ts b/packages/mcp/src/__tests__/tool-input.test.ts index 051070a189..3d884b6bec 100644 --- a/packages/mcp/src/__tests__/tool-input.test.ts +++ b/packages/mcp/src/__tests__/tool-input.test.ts @@ -81,6 +81,22 @@ describe('getToolInputProperties', () => { }) }) + it.each([ + ['personal data', 'ada@example.com'], + ['a PostHog token', 'phc_AbCdEfGhIjKlMnOpQrStUvWx'], + ['URL credentials', 'https://svc:hunter2@internal.test/doc'], + ])('does not record a declared name carrying %s', (_label, name) => { + expect( + getToolInputProperties( + { id: 1, [name]: 1 }, + { properties: { id: {}, [name]: {} } }, + { + shouldRecordInputKey: () => true, + } + ) + ).toEqual({ $mcp_input_keys: ['id', '[redacted]'] }) + }) + describe('inputAliases', () => { const schema = { properties: { id: {}, key: {}, name: {} } } const inputAliases = { id: ['experimentId', 'experiment_id'], key: ['flagKey'], name: ['title'] } diff --git a/packages/mcp/src/extensions/capture.ts b/packages/mcp/src/extensions/capture.ts index 500d1197f7..ce5c56bef2 100644 --- a/packages/mcp/src/extensions/capture.ts +++ b/packages/mcp/src/extensions/capture.ts @@ -101,6 +101,7 @@ export function captureEvent( return sink.capture(fullEvent, { enableExceptionAutocapture: data.options.enableExceptionAutocapture ?? true, + redactPayloadPersonalData: data.options.redactPayloadPersonalData ?? true, beforeSend: data.options.beforeSend, }) } diff --git a/packages/mcp/src/extensions/feedback.ts b/packages/mcp/src/extensions/feedback.ts index bd73b3722d..ebc8b38dee 100644 --- a/packages/mcp/src/extensions/feedback.ts +++ b/packages/mcp/src/extensions/feedback.ts @@ -263,7 +263,7 @@ function captureExtraValue(value: unknown): unknown { return sanitized } try { - return truncateFeedbackText(JSON.stringify(sanitized), MAX_FEEDBACK_TEXT_LENGTH) + return truncateFeedbackText(sanitizeFreeText(JSON.stringify(sanitized)), MAX_FEEDBACK_TEXT_LENGTH) } catch { return undefined } diff --git a/packages/mcp/src/extensions/mcp-payloads.ts b/packages/mcp/src/extensions/mcp-payloads.ts index 6c6898119f..75c37d0eec 100644 --- a/packages/mcp/src/extensions/mcp-payloads.ts +++ b/packages/mcp/src/extensions/mcp-payloads.ts @@ -4,6 +4,7 @@ // Licensed under the MIT License: https://github.com/agentcathq/agentcat-typescript-sdk/blob/main/LICENSE import { + redactPayloadPii, redactPii, sanitizeCapturedValue as sanitizeCoreCapturedValue, sanitizeFreeText as sanitizeCoreFreeText, @@ -24,14 +25,14 @@ function isRecord(value: unknown): value is JsonRecord { return !!value && typeof value === 'object' && !Array.isArray(value) } -export { redactPii } +export { redactPayloadPii, redactPii } export function sanitizeFreeText(value: string): string { return sanitizeCoreFreeText(value, SANITIZATION_OPTIONS) } -export function sanitizeCapturedValue(value: unknown): unknown { - return sanitizeCoreCapturedValue(value, SANITIZATION_OPTIONS) +export function sanitizeCapturedValue(value: unknown, options?: Partial): unknown { + return sanitizeCoreCapturedValue(value, { ...SANITIZATION_OPTIONS, ...options }) } export function sanitizeFreeTextValue(value: unknown): unknown { diff --git a/packages/mcp/src/extensions/posthog-mcp.ts b/packages/mcp/src/extensions/posthog-mcp.ts index aa5dedf037..cbc8969bc7 100644 --- a/packages/mcp/src/extensions/posthog-mcp.ts +++ b/packages/mcp/src/extensions/posthog-mcp.ts @@ -92,6 +92,8 @@ export interface PostHogMCPOptions extends PostHogOptions { * `llm_model` argument as fallback. On by default; `false` disables it. */ captureModel?: MCPAnalyticsOptions['captureModel'] + /** See {@link MCPAnalyticsOptions.redactPayloadPersonalData}. */ + redactPayloadPersonalData?: boolean /** * Correlate calls with an agent-carried `conversation_id` and a derived * PostHog session id. On by default; `false` leaves schemas, arguments, tool @@ -159,6 +161,7 @@ export class PostHogMCP extends PostHog { readonly #feedbackOptions: CollectFeedbackOptions | undefined readonly #captureModel: MCPAnalyticsOptions['captureModel'] readonly #enableConversationId: boolean + readonly #redactPayloadPersonalData: boolean readonly #serverBuild: string | undefined readonly #analyticsParameterOwnership = new Map() @@ -177,6 +180,7 @@ export class PostHogMCP extends PostHog { } this.#captureModel = options.captureModel this.#enableConversationId = options.enableConversationId ?? true + this.#redactPayloadPersonalData = options.redactPayloadPersonalData ?? true this.#serverBuild = serverBuild applyMcpLibIdentity(this) } @@ -507,7 +511,10 @@ export class PostHogMCP extends PostHog { #emit(event: McpEvent): void { event.serverBuild = this.#serverBuild void this.#sink - .capture(event, { enableExceptionAutocapture: this.options.enableExceptionAutocapture ?? true }) + .capture(event, { + enableExceptionAutocapture: this.options.enableExceptionAutocapture ?? true, + redactPayloadPersonalData: this.#redactPayloadPersonalData, + }) .catch((error) => log(`Warning: PostHogMCP failed to capture event - ${error}`)) } diff --git a/packages/mcp/src/extensions/sanitization.ts b/packages/mcp/src/extensions/sanitization.ts index c96edff074..e7c7f9c390 100644 --- a/packages/mcp/src/extensions/sanitization.ts +++ b/packages/mcp/src/extensions/sanitization.ts @@ -4,9 +4,16 @@ // Licensed under the MIT License: https://github.com/agentcathq/agentcat-typescript-sdk/blob/main/LICENSE import type { ErrorProperties, Event, McpEvent } from '../types' +import { MCPAnalyticsEventType } from './event-types' import { sanitizeCapturedValue, sanitizeFreeText } from './mcp-payloads' +import { MAX_ERROR_MESSAGE_LENGTH } from './truncation' type SanitizedRecord = Record +type Sanitize = (value: unknown) => unknown + +export interface PayloadSanitizationOptions { + redactPayloadPersonalData: boolean +} function isRecord(value: unknown): value is SanitizedRecord { return !!value && typeof value === 'object' && !Array.isArray(value) @@ -21,22 +28,29 @@ function isRecord(value: unknown): value is SanitizedRecord { * This is a synchronous operation that returns a new object without mutating the original. * It should run after customer redaction in the event pipeline. */ -export function sanitizeEvent(event: T): T { +export function sanitizeEvent( + event: T, + { redactPayloadPersonalData }: PayloadSanitizationOptions = { redactPayloadPersonalData: true } +): T { const result = { ...event } + const sanitizePayload: Sanitize = (value) => sanitizeCapturedValue(value, { redactPayloadPersonalData }) if (result.response != null) { - result.response = sanitizeResponse(result.response) + result.response = sanitizeResponse(result.response, sanitizePayload) } if (result.parameters != null) { - result.parameters = sanitizeParameters(result.parameters) + result.parameters = sanitizeParameters(result.parameters, sanitizePayload) } // Every event type, not just `resources/read`: the `$exception` sibling // carries the same name, and a tool or prompt name is free text an // application can spell as a URL too. if (result.resourceName != null) { - result.resourceName = sanitizeCapturedValue(result.resourceName) as string + result.resourceName = sanitizeCapturedValue(result.resourceName, { + redactPayloadPersonalData: + redactPayloadPersonalData && result.eventType === MCPAnalyticsEventType.mcpResourcesRead, + }) as string } // The intent comes straight from an agent-narrated `context` string, so it can @@ -55,7 +69,7 @@ export function sanitizeEvent(event: T): T { } if (result.error != null) { - result.error = sanitizeExceptionValues(result.error) + result.error = sanitizeExceptionValues(result.error, redactPayloadPersonalData) } return result @@ -65,7 +79,7 @@ export function sanitizeEvent(event: T): T { * Sanitizes exception messages before they fan out to both the primary MCP * event's error-message property and the `$exception` sibling. */ -function sanitizeExceptionValues(error: ErrorProperties): ErrorProperties { +function sanitizeExceptionValues(error: ErrorProperties, redactPayloadPersonalData: boolean): ErrorProperties { if (!Array.isArray(error.$exception_list)) { return error } @@ -74,7 +88,10 @@ function sanitizeExceptionValues(error: ErrorProperties): ErrorProperties { ...error, $exception_list: error.$exception_list.map((exception) => ({ ...exception, - value: sanitizeCapturedValue(exception.value) as string, + value: sanitizeCapturedValue(exception.value, { + redactPayloadPersonalData, + maxStringLength: MAX_ERROR_MESSAGE_LENGTH, + }) as string, })), } } @@ -83,28 +100,28 @@ function sanitizeExceptionValues(error: ErrorProperties): ErrorProperties { * Sanitizes response content blocks by replacing non-text content types * with informative redaction messages. */ -function sanitizeResponse(response: unknown): unknown { +function sanitizeResponse(response: unknown, sanitize: Sanitize): unknown { // Sanitize each kept block on its own, so the pass never scans the data of replaced blocks. if (isRecord(response) && Array.isArray(response.content)) { - const result = sanitizeCapturedValue({ ...response, content: [] }) as SanitizedRecord - result.content = response.content.map(sanitizeContentBlock) + const result = sanitize({ ...response, content: [] }) as SanitizedRecord + result.content = response.content.map((block) => sanitizeContentBlock(block, sanitize)) return result } - return sanitizeCapturedValue(response) + return sanitize(response) } /** * Sanitizes a single content block based on its type discriminator. */ -function sanitizeContentBlock(block: unknown): unknown { +function sanitizeContentBlock(block: unknown, sanitize: Sanitize): unknown { if (!isRecord(block)) { - return sanitizeCapturedValue(block) + return sanitize(block) } switch (block.type) { case 'text': case 'resource_link': - return sanitizeCapturedValue(block) + return sanitize(block) case 'image': return { @@ -119,12 +136,12 @@ function sanitizeContentBlock(block: unknown): unknown { } case 'resource': - return sanitizeResourceBlock(block) + return sanitizeResourceBlock(block, sanitize) default: return { type: 'text', - text: `[unsupported content type "${sanitizeCapturedValue(block.type)}" redacted - not supported by PostHog MCP analytics]`, + text: `[unsupported content type "${sanitize(block.type)}" redacted - not supported by PostHog MCP analytics]`, } } } @@ -134,20 +151,20 @@ function sanitizeContentBlock(block: unknown): unknown { * BlobResourceContents (has `blob` field) are redacted. * TextResourceContents (has `text` field) pass through. */ -function sanitizeResourceBlock(block: SanitizedRecord): unknown { +function sanitizeResourceBlock(block: SanitizedRecord, sanitize: Sanitize): unknown { if (isRecord(block.resource) && 'blob' in block.resource) { return { type: 'text', text: '[binary resource content redacted - not supported by PostHog MCP analytics]', } } - return sanitizeCapturedValue(block) + return sanitize(block) } /** * Recursively scans parameters for large base64-encoded strings and replaces them. * Uses a size gate (10KB) to avoid regex testing on small strings. */ -function sanitizeParameters(obj: unknown): unknown { - return sanitizeCapturedValue(obj) +function sanitizeParameters(obj: unknown, sanitize: Sanitize): unknown { + return sanitize(obj) } diff --git a/packages/mcp/src/extensions/sink.ts b/packages/mcp/src/extensions/sink.ts index fe33c0c67b..99fc3ca1eb 100644 --- a/packages/mcp/src/extensions/sink.ts +++ b/packages/mcp/src/extensions/sink.ts @@ -10,11 +10,11 @@ import type { BeforeSendFn, Event, McpEvent } from '../types' import { log, type LoggerFn } from './logger' import { type PostHogCaptureEvent, buildPostHogCaptureEvents } from './posthog-events' import { newPrefixedId } from './ids' -import { sanitizeEvent } from './sanitization' +import { type PayloadSanitizationOptions, sanitizeEvent } from './sanitization' import { truncateEvent } from './truncation' /** Per-event toggles consulted by the sink when fanning out an event. */ -export interface McpCaptureOptions { +export interface McpCaptureOptions extends PayloadSanitizationOptions { enableExceptionAutocapture: boolean /** Inspect/modify/drop hook applied to each fanned-out payload before capture. */ beforeSend?: BeforeSendFn @@ -39,7 +39,7 @@ export async function processMcpEvent( let processed: McpEvent = event try { - processed = sanitizeEvent(processed) + processed = sanitizeEvent(processed, options) } catch (err) { logger(`Failed to sanitize event: ${err}`) return null diff --git a/packages/mcp/src/extensions/tool-input.ts b/packages/mcp/src/extensions/tool-input.ts index c485c761d1..353b3a85c1 100644 --- a/packages/mcp/src/extensions/tool-input.ts +++ b/packages/mcp/src/extensions/tool-input.ts @@ -1,5 +1,6 @@ import type { InputAliasMap, JsonRecord, ShouldRecordInputKeyFn, ToolInputOptions } from '../types' import { PostHogMCPAnalyticsProperty } from './constants' +import { sanitizeCapturedValue } from './mcp-payloads' import { getObjectShape, isZodRawShapeCompat, unwrapInputSchema } from './mcp-sdk-compat' const MAX_INPUT_KEYS = 20 @@ -71,7 +72,11 @@ export function getToolInputProperties(input: unknown, inputSchema?: unknown, op let hasRedacted = false for (const key of keys) { const isDeclared = known.has(key) - if (key.length <= MAX_KEY_LENGTH && shouldRecord(record, key, isDeclared)) { + if ( + key.length <= MAX_KEY_LENGTH && + sanitizeCapturedValue(key, { redactPayloadPersonalData: true }) === key && + shouldRecord(record, key, isDeclared) + ) { ;(isDeclared ? declared : undeclared).push(key) } else { hasRedacted = true diff --git a/packages/mcp/src/extensions/truncation.ts b/packages/mcp/src/extensions/truncation.ts index 73ca70baff..137417777e 100644 --- a/packages/mcp/src/extensions/truncation.ts +++ b/packages/mcp/src/extensions/truncation.ts @@ -13,7 +13,7 @@ export const MAX_EVENT_BYTES = 102_400 // 100KB // --- Field-level limit constants --- const MAX_USER_INTENT_LENGTH = 2048 -const MAX_ERROR_MESSAGE_LENGTH = 2048 +export const MAX_ERROR_MESSAGE_LENGTH = 2048 const MAX_RESOURCE_NAME_LENGTH = 256 const MAX_METADATA_LENGTH = 256 const MAX_STACK_FRAMES = 50 diff --git a/packages/mcp/src/types.ts b/packages/mcp/src/types.ts index f620f0bfb5..e7f5ed8811 100644 --- a/packages/mcp/src/types.ts +++ b/packages/mcp/src/types.ts @@ -150,6 +150,17 @@ export interface MCPAnalyticsOptions { * fanning out into PostHog error tracking. */ enableExceptionAutocapture?: boolean + /** + * Redact personal data from tool arguments, results, error messages, exception + * values, and resource read URIs. Defaults to `true`. Emails, Luhn-valid card + * numbers, US social security numbers, IPv6 and public IPv4 addresses, and + * phone numbers written with an international prefix or a parenthesized area + * code become `[redacted]`. Set to `false` when your tools return personal + * data on purpose: `beforeSend` can drop a value but cannot restore one. The + * intent and feedback text keep their redaction, and credentials are redacted + * everywhere either way. + */ + redactPayloadPersonalData?: boolean /** Inject a required `context` parameter on every tool to capture user intent. */ context?: boolean | MCPAnalyticsContextOptions /** From 7c0527a61e185750aa2cc5a63c2d59de2e3b70f1 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Fri, 6 Nov 2026 17:12:17 -0300 Subject: [PATCH 2/7] fix(mcp): redact JSON text copies as structure, keep floats, bound payload cost The adversarial gate on the payload personal data pass found data it leaked or corrupted. This fixes them. What and why: - JSON escapes: read as text, the letter of `\n`, `\t`, `\uXXXX` is a word character, so cards, SSNs, IPs and phones after it leaked and an email swallowed the `n` (invalid `\[` escape). A string that parses as a JSON object or array, within the length the passes already read (4x the string limit), is now sanitized as the structure it encodes (the same passes as structuredContent: leaves, keys, card-number integers) and written back with JSON.stringify; unchanged text comes back byte for byte. Free text (intent, feedback) gets the same path with its own passes, which covers feedback extras' keys. JSON nested too deep to walk falls back to the text pass instead of dropping the event. - Floats: the payload card candidate no longer takes `.` as a separator or starts after `-`/`.` or ends before `.digit`/an exponent; 95 in 1,000 random floats produced invalid JSON. The `00`/`011` phone prefix no longer matches after `.` (it ate `620.0119256973267`), found while verifying. - Card then SSN: a card is one run of 13-19 digits or a run grouped 4-4-4-4, 4-4-4-4-3, 4-6-5 or 4-6-4, so `4111111111111111 078-05-1120` no longer swallows `078`. Shared with the intent pass. - Cost: sanitizeEvent applies the entry and nesting limits first, as the spec orders; strings are still cut by truncateEvent after the passes. Gate probe, processMcpEvent with 100k string leaves: on 249-262 ms -> 0.4-0.5 ms, off 89-96 ms -> 0.2 ms (3 runs each, median of 5). - Changeset names the remaining default false positives. Tested: - pnpm --filter @posthog/mcp test:unit: 1131 passed; the 61 new cases failed first (the escapes x patterns grid, floats, card+SSN, feedback keys, deep nesting, the 100k-leaf cost bound) - pnpm --filter @posthog/core test:unit: 2356 passed; browser webmcp.test.ts: 46 passed - lint and check-types clean for mcp and core - pnpm --filter @posthog/mcp build && test:integration: green (same 2 expected failures per suite as before) Reviewer notes: - Redacted JSON text is re-serialized compactly; integers past 2^53 in it round as they do in structuredContent. A card written `4111111111111111e0` is redacted by value in parsed JSON. - JSON text longer than the parse cut keeps the text pass, so an escape there can still hide an identifier. --- .changeset/mcp-payload-personal-data.md | 2 +- packages/core/src/privacy.ts | 166 ++++++++++++++---- packages/mcp/src/__tests__/feedback.test.ts | 25 +++ .../mcp/src/__tests__/mcp-payloads.test.ts | 78 ++++++++ packages/mcp/src/__tests__/sink.test.ts | 39 +++- packages/mcp/src/extensions/sanitization.ts | 4 +- packages/mcp/src/extensions/truncation.ts | 15 ++ 7 files changed, 289 insertions(+), 40 deletions(-) diff --git a/.changeset/mcp-payload-personal-data.md b/.changeset/mcp-payload-personal-data.md index 69e3016e2a..5cbd3ebd57 100644 --- a/.changeset/mcp-payload-personal-data.md +++ b/.changeset/mcp-payload-personal-data.md @@ -3,4 +3,4 @@ '@posthog/core': minor --- -Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers, US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON number becomes the string `"[redacted]"`, so a result's JSON text stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version after a space, `:`, or `@` (`node:22.11.0.3`), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. +Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped 4-4-4-4, 4-4-4-4-3, 4-6-5, or 4-6-4 by spaces or dashes), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON integer becomes the string `"[redacted]"`; negative numbers and fractions are kept. A JSON text copy of a result is redacted as the structure it encodes, so it agrees with `structuredContent` and stays valid JSON; when something in it is redacted it is written back compactly, without its original formatting. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. diff --git a/packages/core/src/privacy.ts b/packages/core/src/privacy.ts index 45230e717e..3674c24639 100644 --- a/packages/core/src/privacy.ts +++ b/packages/core/src/privacy.ts @@ -113,6 +113,13 @@ const US_SSN_PATTERN = /\b\d{3}[ .-]\d{2}[ .-]\d{4}\b/g const CREDIT_CARD_CANDIDATE_PATTERN = /\b\d(?:[ ./-]?\d){12,}\b/g // Matches each separator-delimited digit group inside a card candidate. const DIGIT_GROUP_PATTERN = /\d+/g +// How cards are written when grouped, longest first. An ungrouped card is one run of 13–19 digits. +const CARD_GROUPINGS = [ + [4, 4, 4, 4, 3], + [4, 4, 4, 4], + [4, 6, 5], + [4, 6, 4], +] // 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 @@ -136,10 +143,16 @@ const NON_PUBLIC_IPV4_RANGES = [ '169.254.0.0/16', '0.0.0.0/32', ] +// Digits grouped only by single spaces or dashes: a `.` makes a fraction or a +// version, not a card. A leading `-` or `.` and a trailing `.` and digit or +// exponent belong to a number the run is only part of. +const PAYLOAD_CARD_CANDIDATE_PATTERN = /(^|[^\w.-])(\d(?:[ -]?\d){12,})(?!\w|\.\d)/g const PHONE_PARENTHESIZED_PATTERN = /(^|[^\w+])((?:\+?1[ ./-]?)?\(\d{3}\)[ ./-]?\d{3}[ ./-]\d{4})(?![\w])/g -const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g +// Not after a `.`, where `00` or `011` starts a fraction's digits. +const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w.])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g const JSON_VALUE_START_PATTERN = /[:,[]\s*$/ const JSON_VALUE_END_PATTERN = /^\s*[,}\]]/ +const JSON_CONTAINER_START_PATTERN = /^\s*[[{]/ type JsonRecord = Record @@ -554,7 +567,7 @@ function redactCapturedHead( redact: (value: string) => string, options: TextSanitizationOptions ): string { - const scannedHeadLength = 4 * options.maxStringLength + const scannedHeadLength = scannedLength(options) if (value.length > scannedHeadLength) { const head = redact(value.slice(0, headLength(value, scannedHeadLength))) // Twice what truncation keeps, so a match split where the head was cut sits @@ -566,6 +579,33 @@ function redactCapturedHead( return redact(value) } +/** How much of a captured string the passes read. */ +function scannedLength(options: TextSanitizationOptions): number { + return 4 * options.maxStringLength +} + +/** + * A JSON object or array written as text, sanitized as the structure it encodes + * and written back with `JSON.stringify`, or undefined when `value` is not one or + * is longer than the passes read. Read as text, a JSON escape hides what follows + * it: in `"x\nada@example.com"` the `n` is a word character, so an address or + * number glued to it is missed or cut in half. Text with nothing to redact comes + * back as received, formatting included. JSON nested too deep to walk is left to + * the text pass rather than dropping the event. + */ +function sanitizeJsonText(value: string, passes: ValuePasses, options: TextSanitizationOptions): string | undefined { + if (value.length > scannedLength(options) || !JSON_CONTAINER_START_PATTERN.test(value)) { + return undefined + } + try { + const parsed: unknown = JSON.parse(value) + const sanitized = JSON.stringify(sanitizeValueWith(parsed, passes)) + return sanitized === JSON.stringify(parsed) ? value : sanitized + } catch { + return undefined + } +} + /** The scanned head, shortened to end just after the last character that ends a URL. */ function headLength(value: string, scannedHeadLength: number): number { let end = scannedHeadLength @@ -576,10 +616,16 @@ function headLength(value: string, scannedHeadLength: number): number { } function sanitizeString(value: string, options: TextSanitizationOptions): string { - const redactPersonalData = options.redactPayloadPersonalData ? redactPayloadPii : (head: string) => head - return isBinaryBlob(value) - ? BINARY_REDACTED_VALUE - : redactCapturedHead(value, (head) => redactUrls(redactPersonalData(redactCredentials(head))), options) + if (isBinaryBlob(value)) { + return BINARY_REDACTED_VALUE + } + if (!options.redactPayloadPersonalData) { + return redactCapturedHead(value, (head) => redactUrls(redactCredentials(head)), options) + } + return ( + sanitizeJsonText(value, payloadPasses(options), options) ?? + redactCapturedHead(value, (head) => redactUrls(redactPayloadPii(redactCredentials(head))), options) + ) } /** @@ -598,7 +644,10 @@ export function sanitizeFreeText(value: string, options: TextSanitizationOptions if (isBinaryBlob(value)) { return BINARY_REDACTED_VALUE } - return redactCapturedHead(value, (head) => redactUrls(redactPii(redactCredentials(head))), options) + return ( + sanitizeJsonText(value, freeTextPasses(options), options) ?? + redactCapturedHead(value, (head) => redactUrls(redactPii(redactCredentials(head))), options) + ) } function passesLuhn(digits: string): boolean { @@ -621,16 +670,42 @@ function passesLuhn(digits: string): boolean { return sum % 10 === 0 } +type DigitGroup = { digits: string; start: number; end: number } + +/** + * The index of the last group of the card that starts at `groups[first]`, or -1. + * A card is one group of 13–19 digits or a run in one of {@link CARD_GROUPINGS}, + * passing Luhn. Requiring a card's own grouping keeps a card from absorbing the + * start of a field after it (`4111111111111111 078-05-1120`). + */ +function cardEnd(groups: DigitGroup[], first: number): number { + const single = groups[first].digits + if (single.length >= 13) { + return single.length <= 19 && passesLuhn(single) ? first : -1 + } + for (const grouping of CARD_GROUPINGS) { + const run = groups.slice(first, first + grouping.length) + if ( + run.length === grouping.length && + run.every((group, index) => group.digits.length === grouping[index]) && + passesLuhn(run.map((group) => group.digits).join('')) + ) { + return first + grouping.length - 1 + } + } + return -1 +} + // A card candidate can span more than one card plus adjacent fields (e.g. an // expiry, or a second card). This redacts every card in it: scanning // left-to-right over the whole separator-delimited digit groups, at each start it -// takes the longest run whose joined digits are 13–19 long and pass Luhn, redacts +// takes the longest run in a card's grouping that passes Luhn (see `cardEnd`), redacts // just that span, and resumes after it; a group that begins no valid run is kept // and skipped. 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. function redactCardInMatch(match: string): string { - const groups: { digits: string; start: number; end: number }[] = [] + const groups: DigitGroup[] = [] for (let m = DIGIT_GROUP_PATTERN.exec(match); m !== null; m = DIGIT_GROUP_PATTERN.exec(match)) { groups.push({ digits: m[0], start: m.index, end: m.index + m[0].length }) } @@ -638,17 +713,7 @@ function redactCardInMatch(match: string): string { let cursor = 0 let first = 0 while (first < groups.length) { - let digits = '' - let matchedLast = -1 - for (let last = first; last < groups.length; last++) { - digits += groups[last].digits - if (digits.length > 19) { - break - } - if (digits.length >= 13 && passesLuhn(digits)) { - matchedLast = last - } - } + const matchedLast = cardEnd(groups, first) if (matchedLast >= 0) { output += match.slice(cursor, groups[first].start) + REDACTED_VALUE cursor = groups[matchedLast].end @@ -698,14 +763,24 @@ function isPublicIpv4(address: string): boolean { * {@link redactCardInMatch}, writing a card that stands as a bare JSON number as * the string `"[redacted]"`, so a tool result's JSON text copy stays valid JSON. */ -function redactCardKeepingJson(match: string, offset: number, text: string): string { - const redacted = redactCardInMatch(match) - const end = offset + match.length - return redacted === REDACTED_VALUE && - JSON_VALUE_START_PATTERN.test(text.slice(Math.max(0, offset - 64), offset)) && +function redactCardKeepingJson( + _match: string, + prefix: string, + candidate: string, + offset: number, + text: string +): string { + const redacted = redactCardInMatch(candidate) + const start = offset + prefix.length + const end = start + candidate.length + return ( + prefix + + (redacted === REDACTED_VALUE && + JSON_VALUE_START_PATTERN.test(text.slice(Math.max(0, start - 64), start)) && JSON_VALUE_END_PATTERN.test(text.slice(end, end + 64)) - ? `"${REDACTED_VALUE}"` - : redacted + ? `"${REDACTED_VALUE}"` + : redacted) + ) } function isCardNumber(value: number): boolean { @@ -726,7 +801,7 @@ export function redactPayloadPii(value: string): string { isPublicIpv4(address) ? prefix + REDACTED_VALUE : match ) result = result.replace(IPV6_PATTERN, `$1${REDACTED_VALUE}`) - result = result.replace(CREDIT_CARD_CANDIDATE_PATTERN, redactCardKeepingJson) + result = result.replace(PAYLOAD_CARD_CANDIDATE_PATTERN, redactCardKeepingJson) result = result.replace(US_SSN_PATTERN, REDACTED_VALUE) result = result.replace(PHONE_PARENTHESIZED_PATTERN, `$1${REDACTED_VALUE}`) result = result.replace(PAYLOAD_PHONE_INTL_PATTERN, `$1${REDACTED_VALUE}`) @@ -734,15 +809,34 @@ export function redactPayloadPii(value: string): string { } export function sanitizeCapturedValue(value: unknown, options: TextSanitizationOptions): unknown { - const string = (stringValue: string): string => sanitizeString(stringValue, options) - if (!options.redactPayloadPersonalData) { - return sanitizeValueWith(value, { string }) - } - return sanitizeValueWith(value, { - string, + return sanitizeValueWith( + value, + options.redactPayloadPersonalData + ? payloadPasses(options) + : { string: (stringValue) => sanitizeString(stringValue, options) } + ) +} + +function redactCardNumber(value: number): unknown { + return isCardNumber(value) ? REDACTED_VALUE : value +} + +/** What the payload personal data pass rewrites in a structure. */ +function payloadPasses(options: TextSanitizationOptions): ValuePasses { + return { + string: (value) => sanitizeString(value, options), key: (key) => redactCapturedHead(key, redactPayloadPii, options), - number: (number) => (isCardNumber(number) ? REDACTED_VALUE : number), - }) + number: redactCardNumber, + } +} + +/** What the free-text pass rewrites in JSON written as free text. */ +function freeTextPasses(options: TextSanitizationOptions): ValuePasses { + return { + string: (value) => sanitizeFreeText(value, options), + key: (key) => redactCapturedHead(key, redactPii, options), + number: redactCardNumber, + } } /** diff --git a/packages/mcp/src/__tests__/feedback.test.ts b/packages/mcp/src/__tests__/feedback.test.ts index cdd5ab348f..441836b8c1 100644 --- a/packages/mcp/src/__tests__/feedback.test.ts +++ b/packages/mcp/src/__tests__/feedback.test.ts @@ -509,6 +509,31 @@ describe('collectFeedback (send_feedback virtual tool)', () => { await capture.stop() }) + it.each([ + ['a card', { 'card\n4111111111111111': 1 }, '4111111111111111'], + ['an SSN', { 'ssn\n078-05-1120': 1 }, '078-05-1120'], + ['a public IPv4', { 'ip\n203.0.113.77': 1 }, '203.0.113.77'], + ['a phone', { 'tel\n+1 415 555 0177': 1 }, '555 0177'], + ['an IPv6', { nested: { 'x\t2001:db8::7334': 1 } }, '2001:db8::7334'], + ['a card-number integer', { n: 4111111111111111 }, '4111111111111111'], + ])( + 'redacts %s in a stringified extra after a JSON escape, keeping valid JSON', + async (_label, meta, identifier) => { + const capture = new EventCapture() + await capture.start() + instrument(server, fakePostHog(), { collectFeedback: { extraProperties: { meta: { type: 'object' } } } }) + + await callTool(client, SEND_FEEDBACK, { feedback_type: 'issue', summary: 'A tool failed.', meta }) + + await new Promise((r) => setTimeout(r, 50)) + const captured = capture.findCapturesByEvent(PostHogMCPAnalyticsEvent.Feedback)[0].properties.$mcp_feedback_meta + expect(captured).not.toContain(identifier) + expect(() => JSON.parse(captured as string)).not.toThrow() + + await capture.stop() + } + ) + it('does not log the agent-supplied summary', async () => { const logger = vi.fn() instrument(server, fakePostHog(), { collectFeedback: true, logger }) diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index ffc590aa50..558c111a77 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -489,6 +489,8 @@ describe('redactPii', () => { 'Moving funds 4111 1111 1111 1111 5555 5555 5555 4444 now.', 'Moving funds [redacted] [redacted] now.', ], + ['a card followed by an SSN', 'Card 4111111111111111 078-05-1120 on file.', 'Card [redacted] [redacted] on file.'], + ['a card grouped 4-6-5', 'Charging card 3782 822463 10005 today.', 'Charging card [redacted] today.'], ['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.'], @@ -546,6 +548,7 @@ describe('redactPayloadPii', () => { ['a phone with +1 and a parenthesized area code', 'call +1 (415) 555-0142 today', 'call [redacted] today'], ['a Luhn-valid card', 'card 4111 1111 1111 1111 on file', 'card [redacted] on file'], ['a US SSN', 'SSN 123-45-6789', 'SSN [redacted]'], + ['a card followed by an SSN', 'card 4111111111111111 078-05-1120', 'card [redacted] [redacted]'], [ 'a card written as a JSON number, quoted', '{"card":4111111111111111,"count":3}', @@ -571,6 +574,14 @@ describe('redactPayloadPii', () => { ['the unspecified address', 'bind 0.0.0.0'], ['a Luhn-invalid long digit run', 'order 1234567890123456'], ['a date and time', 'at 2024-01-15 12:30 UTC'], + ['a float whose fraction passes the card checksum', '{"v":710.4032039642334}'], + ['a fraction of 16 digits', '[0.4111111111111111]'], + ['a negative card-shaped number', '[-4111111111111111]'], + ['a card-shaped number with a fraction', '[4111111111111111.5]'], + ['a card-shaped number with an exponent', '[4111111111111111e0]'], + ['a card grouped with dots', 'ref 4111.1111.1111.1111'], + ['a fraction starting 011', '[620.0119256973267]'], + ['a fraction starting 00', '[0.0012345678901]'], ])('keeps %s', (_label, input) => { expect(redactPayloadPii(input)).toBe(input) }) @@ -583,6 +594,73 @@ describe('redactPayloadPii', () => { }) }) +describe('payload JSON text copies', () => { + const sanitize = (value: unknown): unknown => sanitizeCapturedValue(value, { redactPayloadPersonalData: true }) + const identifiers = [ + ['an email', 'ada@example.com'], + ['a card', '4111111111111111'], + ['an SSN', '078-05-1120'], + ['a public IPv4', '203.0.113.7'], + ['an IPv6', '2001:db8::7334'], + ['a phone', '+1 415 555 0142'], + ] + const escapes = ['\\n', '\\t', '\\r', '\\b', '\\f', '\\u00e9', '\\\\', '\\"', '\\/'] + const cases = identifiers.flatMap(([label, identifier]) => + escapes.map((escape) => [label, escape, identifier] as const) + ) + + it.each(cases)('redacts %s after %s in a value and a key, keeping valid JSON', (_label, escape, identifier) => { + const text = `{"note":"x${escape}${identifier}","x${escape}${identifier}":1}` + + const redacted = sanitize(text) as string + + expect(redacted).not.toContain(identifier) + expect(JSON.parse(redacted)).toEqual(sanitize(JSON.parse(text))) + }) + + it.each([ + ['a float whose fraction passes the card checksum', '{"v":710.4032039642334}'], + ['a fraction of 16 digits', '[0.4111111111111111]'], + ['a negative card-shaped number', '[-4111111111111111]'], + ['a card-shaped number with a fraction', '[4111111111111111.5]'], + ['a card-shaped number scaled to a fraction', '[4111111111111111e-3]'], + ['pretty-printed JSON with nothing to redact', JSON.stringify({ id: 'ord_1', total: 12.5 }, null, 2)], + ])('keeps %s byte for byte', (_label, text) => { + expect(sanitize(text)).toBe(text) + }) + + describe('random floats', () => { + let seed = 42 + const random = (): number => (seed = (seed * 1103515245 + 12345) % 2 ** 31) / 2 ** 31 + const floats = Array.from({ length: 10_000 }, () => random() * 1000) + + it('keeps them in short JSON text copies', () => { + const texts = floats.slice(0, 1000).map((v) => JSON.stringify({ v })) + + expect(texts.filter((text) => sanitize(text) !== text)).toEqual([]) + }) + + it('keeps them in a JSON text copy longer than the passes parse', () => { + const text = JSON.stringify(floats) + + expect(sanitize(text)).toBe(text) + }) + }) + + it('falls back to the text pass on JSON nested too deep to walk', () => { + const depth = 50_000 + const text = `${'['.repeat(depth)}"ada@example.com"${']'.repeat(depth)}` + + expect(sanitize(text)).toBe(`${'['.repeat(depth)}"[redacted]"${']'.repeat(depth)}`) + }) + + it('redacts a card-number integer and re-serializes the redacted structure', () => { + const text = JSON.stringify({ card: 4111111111111111, note: 'x\nada@example.com', count: 3 }, null, 2) + + expect(sanitize(text)).toBe('{"card":"[redacted]","note":"x\\n[redacted]","count":3}') + }) +}) + describe('sanitizing strings longer than truncation keeps', () => { const WINDOW = 131_072 const token = 'phc_123456789012345678901234567890' diff --git a/packages/mcp/src/__tests__/sink.test.ts b/packages/mcp/src/__tests__/sink.test.ts index 89645b0518..6f998812c1 100644 --- a/packages/mcp/src/__tests__/sink.test.ts +++ b/packages/mcp/src/__tests__/sink.test.ts @@ -1,6 +1,6 @@ import type { EventMessage, PostHog } from 'posthog-node' -import { McpEventSink } from '../extensions/sink' +import { McpEventSink, processMcpEvent } from '../extensions/sink' import { MCPAnalyticsEventType } from '../extensions/event-types' import type { McpEvent } from '../types' @@ -63,3 +63,40 @@ describe('McpEventSink groups forwarding', () => { expect('groups' in calls[0]).toBe(false) }) }) + +describe('processMcpEvent cost', () => { + const rowsEvent = (rows: number): McpEvent => ({ + eventType: MCPAnalyticsEventType.mcpToolsCall, + sessionId: 'session-1', + timestamp: new Date(), + parameters: {}, + response: { + content: [{ type: 'text', text: 'ok' }], + structuredContent: { + rows: Array.from( + { length: rows }, + (_, i) => `customer ${i} order ORD-${i} status shipped 2026-11-06T12:00:00Z` + ), + }, + }, + }) + + async function medianMs(event: McpEvent): Promise { + const options = { enableExceptionAutocapture: true, redactPayloadPersonalData: true } + await processMcpEvent(event, options, () => {}) + const runs: number[] = [] + for (let run = 0; run < 5; run++) { + const start = performance.now() + await processMcpEvent(event, options, () => {}) + runs.push(performance.now() - start) + } + return runs.sort((left, right) => left - right)[2] + } + + it('applies the entry limit before sanitizing, so entries past it cost nothing', async () => { + const atLimit = await medianMs(rowsEvent(100)) + const farPastLimit = await medianMs(rowsEvent(100_000)) + + expect(farPastLimit).toBeLessThan(10 * atLimit + 5) + }) +}) diff --git a/packages/mcp/src/extensions/sanitization.ts b/packages/mcp/src/extensions/sanitization.ts index e7c7f9c390..dd7c3bb15b 100644 --- a/packages/mcp/src/extensions/sanitization.ts +++ b/packages/mcp/src/extensions/sanitization.ts @@ -6,7 +6,7 @@ import type { ErrorProperties, Event, McpEvent } from '../types' import { MCPAnalyticsEventType } from './event-types' import { sanitizeCapturedValue, sanitizeFreeText } from './mcp-payloads' -import { MAX_ERROR_MESSAGE_LENGTH } from './truncation' +import { limitPayloadEntries, MAX_ERROR_MESSAGE_LENGTH } from './truncation' type SanitizedRecord = Record type Sanitize = (value: unknown) => unknown @@ -32,7 +32,7 @@ export function sanitizeEvent( event: T, { redactPayloadPersonalData }: PayloadSanitizationOptions = { redactPayloadPersonalData: true } ): T { - const result = { ...event } + const result = limitPayloadEntries(event) const sanitizePayload: Sanitize = (value) => sanitizeCapturedValue(value, { redactPayloadPersonalData }) if (result.response != null) { diff --git a/packages/mcp/src/extensions/truncation.ts b/packages/mcp/src/extensions/truncation.ts index 137417777e..91eedd5b37 100644 --- a/packages/mcp/src/extensions/truncation.ts +++ b/packages/mcp/src/extensions/truncation.ts @@ -425,6 +425,21 @@ function normalizeReducibleFields(event: MutableEvent, depth: number): MutableEv return reduced } +/** + * Applies the entry and nesting limits to the payloads the sanitizer reads, so + * its work is bounded by those limits rather than by the payload. Strings are + * cut later, by {@link truncateEvent}, after the sanitizer has read past the cut. + */ +export function limitPayloadEntries(event: T): T { + const result: MutableEvent = { ...event } + for (const field of ['parameters', 'response'] as const) { + if (result[field] != null) { + result[field] = normalize(result[field], MAX_DEPTH, MAX_BREADTH, Infinity) + } + } + return result as T +} + /** * Applies layered truncation to an event: * 1. Field-level string limits (userIntent, resourceName, metadata fields, error.message) From 7c039ad5682ed08fceaab3eaa36bed0a137c62c1 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Fri, 6 Nov 2026 17:33:37 -0300 Subject: [PATCH 3/7] fix(mcp): match personal data on escape-masked text instead of re-serializing JSON What: the text pass (intent, payload, and the PostHog token pass) now matches every pattern on a same-length copy of the string in which each JSON/JS escape (\n \t \r \b \f \/ \\ \" \uXXXX) reads as NUL and Unicode spaces read as ASCII spaces, and splices each redaction into the original at the same offsets. sanitizeJsonText and the parse/re-serialize path are gone. Cards: the CARD_GROUPINGS requirement is gone; each start takes the shortest Luhn-valid 13-19 digit run of whole groups, so a trailing SSN is left for its own pattern. The payload card candidate accepts a card glued to a letter by - or . and still rejects negatives and fractions. A card that is a bare JSON number outside any string (quote parity on the masked text) is written "[redacted]" in both passes. Object walks (core sanitizeValueWith, mcp normalize) build results with Object.fromEntries, so a parsed __proto__ member survives. buildCapturedMcpParameters applies the entry and nesting limits (limitEntries) before sanitizing. Why: two rounds of parse-and-reserialize kept failing the re-gate (duplicate keys, integers past 2^53, __proto__, NDJSON, embedded JSON, BOM, lost formatting). Masking needs none of those special cases: no match contains an escape, so JSON stays valid and clean text stays byte for byte. The mask char is not a separator, so 4111\n1111\n... is not joined into a spaced card. The PostHog token pass reads the masked text too, because "x\nphc_..." leaked a token after the parse path was removed (and leaks on main). Tested: - pnpm --filter @posthog/core test:unit: 2356 passed, 1 skipped - pnpm --filter @posthog/mcp test:unit: 1702 passed (325 of them fail on 7c0527a61) - browser vitest src/__tests__/webmcp.test.ts: 46 passed - lint and check-types for core and mcp, pnpm format: clean - pnpm --filter @posthog/mcp build && test:integration: 37/39, the 2 declared expected failures - re-gate probes probe1-3 and perf, plus zz-regate-* tests: no leaks, intent card table matches main row for row; 100k-leaf arguments build in 8.9 ms vs 0.1 ms for 100, the rest being V8 key enumeration (Object.keys on 100k keys alone is 8.2 ms) Reviewer notes: the context argument is now dropped after the entry limit, so it counts toward the 100 kept arguments. A payload card after a non-letter - or . (": -4111...") is read as a negative number and kept. --- .changeset/mcp-payload-personal-data.md | 2 +- packages/core/src/privacy.ts | 261 ++++++++---------- .../mcp/src/__tests__/mcp-payloads.test.ts | 166 +++++++++-- packages/mcp/src/__tests__/sink.test.ts | 29 ++ packages/mcp/src/extensions/mcp-payloads.ts | 58 ++-- packages/mcp/src/extensions/truncation.ts | 16 +- 6 files changed, 318 insertions(+), 214 deletions(-) diff --git a/.changeset/mcp-payload-personal-data.md b/.changeset/mcp-payload-personal-data.md index 5cbd3ebd57..ce01fced39 100644 --- a/.changeset/mcp-payload-personal-data.md +++ b/.changeset/mcp-payload-personal-data.md @@ -3,4 +3,4 @@ '@posthog/core': minor --- -Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped 4-4-4-4, 4-4-4-4-3, 4-6-5, or 4-6-4 by spaces or dashes), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON integer becomes the string `"[redacted]"`; negative numbers and fractions are kept. A JSON text copy of a result is redacted as the structure it encodes, so it agrees with `structuredContent` and stays valid JSON; when something in it is redacted it is written back compactly, without its original formatting. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. +Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped by single spaces or dashes, not line breaks), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON integer becomes the string `"[redacted]"`; negative numbers and fractions are kept. A JSON or JavaScript string escape (`\n`, `\"`, `\u00e9`) no longer hides an identifier or a PostHog token written right after it, and a JSON text copy of a result is redacted in place, so it keeps its formatting and stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. diff --git a/packages/core/src/privacy.ts b/packages/core/src/privacy.ts index 3674c24639..6569c05af3 100644 --- a/packages/core/src/privacy.ts +++ b/packages/core/src/privacy.ts @@ -113,13 +113,6 @@ const US_SSN_PATTERN = /\b\d{3}[ .-]\d{2}[ .-]\d{4}\b/g const CREDIT_CARD_CANDIDATE_PATTERN = /\b\d(?:[ ./-]?\d){12,}\b/g // Matches each separator-delimited digit group inside a card candidate. const DIGIT_GROUP_PATTERN = /\d+/g -// How cards are written when grouped, longest first. An ungrouped card is one run of 13–19 digits. -const CARD_GROUPINGS = [ - [4, 4, 4, 4, 3], - [4, 4, 4, 4], - [4, 6, 5], - [4, 6, 4], -] // 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 @@ -144,21 +137,23 @@ const NON_PUBLIC_IPV4_RANGES = [ '0.0.0.0/32', ] // Digits grouped only by single spaces or dashes: a `.` makes a fraction or a -// version, not a card. A leading `-` or `.` and a trailing `.` and digit or -// exponent belong to a number the run is only part of. -const PAYLOAD_CARD_CANDIDATE_PATTERN = /(^|[^\w.-])(\d(?:[ -]?\d){12,})(?!\w|\.\d)/g +// version, not a card. A leading `-` or `.` belongs to a negative number or a +// fraction unless a letter comes before it (`card-4111…`, `No.4111…`), and a +// trailing `.` and digit or exponent to a number the run is only part of. +const PAYLOAD_CARD_CANDIDATE_PATTERN = /(^|[^\w.-]|[A-Za-z_][.-])(\d(?:[ -]?\d){12,})(?!\w|\.\d)/g const PHONE_PARENTHESIZED_PATTERN = /(^|[^\w+])((?:\+?1[ ./-]?)?\(\d{3}\)[ ./-]?\d{3}[ ./-]\d{4})(?![\w])/g // Not after a `.`, where `00` or `011` starts a fraction's digits. const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w.])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g const JSON_VALUE_START_PATTERN = /[:,[]\s*$/ const JSON_VALUE_END_PATTERN = /^\s*[,}\]]/ -const JSON_CONTAINER_START_PATTERN = /^\s*[[{]/ - -type JsonRecord = Record - -function isRecord(value: unknown): value is JsonRecord { - return !!value && typeof value === 'object' && !Array.isArray(value) -} +// A JSON or JavaScript string escape. Read as text, its letter or hex digits glue +// to what follows: `"x\nada@example.com"` holds the address `nada@example.com`, +// and `"x\n4111…"` has no boundary before the card. +const ESCAPE_PATTERN = /\\(?:["\\/bfnrt]|u[0-9A-Fa-f]{4})/g +// What the identifier patterns read in place of each escape character: neither a +// word character nor any pattern's separator, so a masked escape ends an +// identifier without joining the digit groups on either side of it into one. +const ESCAPE_MASK = '\u0000' function shouldRedactKey(key: string): boolean { return SENSITIVE_KEY_PATTERN.test(key) @@ -546,7 +541,7 @@ function isBinaryBlob(value: string): boolean { /** Replaces PostHog API keys wherever they appear in a string. */ function redactCredentials(value: string): string { - return value.replace(POSTHOG_TOKEN_PATTERN, REDACTED_VALUE) + return redactIdentifiers(value, [[POSTHOG_TOKEN_PATTERN, redactWhole]]) } /** Replaces every URL-shaped match, credentials and all. */ @@ -567,7 +562,7 @@ function redactCapturedHead( redact: (value: string) => string, options: TextSanitizationOptions ): string { - const scannedHeadLength = scannedLength(options) + const scannedHeadLength = 4 * options.maxStringLength if (value.length > scannedHeadLength) { const head = redact(value.slice(0, headLength(value, scannedHeadLength))) // Twice what truncation keeps, so a match split where the head was cut sits @@ -579,33 +574,6 @@ function redactCapturedHead( return redact(value) } -/** How much of a captured string the passes read. */ -function scannedLength(options: TextSanitizationOptions): number { - return 4 * options.maxStringLength -} - -/** - * A JSON object or array written as text, sanitized as the structure it encodes - * and written back with `JSON.stringify`, or undefined when `value` is not one or - * is longer than the passes read. Read as text, a JSON escape hides what follows - * it: in `"x\nada@example.com"` the `n` is a word character, so an address or - * number glued to it is missed or cut in half. Text with nothing to redact comes - * back as received, formatting included. JSON nested too deep to walk is left to - * the text pass rather than dropping the event. - */ -function sanitizeJsonText(value: string, passes: ValuePasses, options: TextSanitizationOptions): string | undefined { - if (value.length > scannedLength(options) || !JSON_CONTAINER_START_PATTERN.test(value)) { - return undefined - } - try { - const parsed: unknown = JSON.parse(value) - const sanitized = JSON.stringify(sanitizeValueWith(parsed, passes)) - return sanitized === JSON.stringify(parsed) ? value : sanitized - } catch { - return undefined - } -} - /** The scanned head, shortened to end just after the last character that ends a URL. */ function headLength(value: string, scannedHeadLength: number): number { let end = scannedHeadLength @@ -619,13 +587,8 @@ function sanitizeString(value: string, options: TextSanitizationOptions): string if (isBinaryBlob(value)) { return BINARY_REDACTED_VALUE } - if (!options.redactPayloadPersonalData) { - return redactCapturedHead(value, (head) => redactUrls(redactCredentials(head)), options) - } - return ( - sanitizeJsonText(value, payloadPasses(options), options) ?? - redactCapturedHead(value, (head) => redactUrls(redactPayloadPii(redactCredentials(head))), options) - ) + const redactPersonalData = options.redactPayloadPersonalData ? redactPayloadPii : (head: string) => head + return redactCapturedHead(value, (head) => redactUrls(redactPersonalData(redactCredentials(head))), options) } /** @@ -644,10 +607,7 @@ export function sanitizeFreeText(value: string, options: TextSanitizationOptions if (isBinaryBlob(value)) { return BINARY_REDACTED_VALUE } - return ( - sanitizeJsonText(value, freeTextPasses(options), options) ?? - redactCapturedHead(value, (head) => redactUrls(redactPii(redactCredentials(head))), options) - ) + return redactCapturedHead(value, (head) => redactUrls(redactPii(redactCredentials(head))), options) } function passesLuhn(digits: string): boolean { @@ -670,42 +630,18 @@ function passesLuhn(digits: string): boolean { return sum % 10 === 0 } -type DigitGroup = { digits: string; start: number; end: number } - -/** - * The index of the last group of the card that starts at `groups[first]`, or -1. - * A card is one group of 13–19 digits or a run in one of {@link CARD_GROUPINGS}, - * passing Luhn. Requiring a card's own grouping keeps a card from absorbing the - * start of a field after it (`4111111111111111 078-05-1120`). - */ -function cardEnd(groups: DigitGroup[], first: number): number { - const single = groups[first].digits - if (single.length >= 13) { - return single.length <= 19 && passesLuhn(single) ? first : -1 - } - for (const grouping of CARD_GROUPINGS) { - const run = groups.slice(first, first + grouping.length) - if ( - run.length === grouping.length && - run.every((group, index) => group.digits.length === grouping[index]) && - passesLuhn(run.map((group) => group.digits).join('')) - ) { - return first + grouping.length - 1 - } - } - return -1 -} - // A card candidate can span more than one card plus adjacent fields (e.g. an // expiry, or a second card). This redacts every card in it: scanning // left-to-right over the whole separator-delimited digit groups, at each start it -// takes the longest run in a card's grouping that passes Luhn (see `cardEnd`), redacts +// takes the shortest run whose joined digits are 13–19 long and pass Luhn, redacts // just that span, and resumes after it; a group that begins no valid run is kept -// and skipped. 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. +// and skipped. The shortest run leaves a field after the card, such as the SSN in +// `4111111111111111 078-05-1120`, for its own pattern. 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. function redactCardInMatch(match: string): string { - const groups: DigitGroup[] = [] + const groups: { digits: string; start: number; end: number }[] = [] for (let m = DIGIT_GROUP_PATTERN.exec(match); m !== null; m = DIGIT_GROUP_PATTERN.exec(match)) { groups.push({ digits: m[0], start: m.index, end: m.index + m[0].length }) } @@ -713,7 +649,18 @@ function redactCardInMatch(match: string): string { let cursor = 0 let first = 0 while (first < groups.length) { - const matchedLast = cardEnd(groups, first) + let digits = '' + let matchedLast = -1 + for (let last = first; last < groups.length; last++) { + digits += groups[last].digits + if (digits.length > 19) { + break + } + if (digits.length >= 13 && passesLuhn(digits)) { + matchedLast = last + break + } + } if (matchedLast >= 0) { output += match.slice(cursor, groups[first].start) + REDACTED_VALUE cursor = groups[matchedLast].end @@ -735,15 +682,56 @@ function redactCardInMatch(match: string): string { * nothing matches. */ export function redactPii(value: string): string { - let result = value.replace(UNICODE_SPACE_PATTERN, ' ') - result = result.replace(EMAIL_PATTERN, REDACTED_VALUE) - result = result.replace(IPV4_PATTERN, REDACTED_VALUE) - result = result.replace(IPV6_PATTERN, `$1${REDACTED_VALUE}`) - result = result.replace(CREDIT_CARD_CANDIDATE_PATTERN, redactCardInMatch) - result = result.replace(US_SSN_PATTERN, REDACTED_VALUE) - result = result.replace(PHONE_NANP_PATTERN, `$1${REDACTED_VALUE}`) - result = result.replace(PHONE_INTL_PATTERN, `$1${REDACTED_VALUE}`) - return result + return redactIdentifiers(value, [ + [EMAIL_PATTERN, redactWhole], + [IPV4_PATTERN, redactWhole], + [IPV6_PATTERN, redactWhole], + [CREDIT_CARD_CANDIDATE_PATTERN, cardRedaction()], + [US_SSN_PATTERN, redactWhole], + [PHONE_NANP_PATTERN, redactWhole], + [PHONE_INTL_PATTERN, redactWhole], + ]) +} + +/** What one identifier is replaced with, given where it starts in the text the patterns read. */ +type Redaction = (identifier: string, start: number, masked: string) => string + +const redactWhole: Redaction = () => REDACTED_VALUE + +/** + * Applies each pattern in turn, matching it against a copy of `value` that has + * the same length: horizontal Unicode spaces read as an ASCII space and every + * escape character as {@link ESCAPE_MASK}. A match never contains a masked + * character, so each replacement lands at the same offsets in `value`, no escape + * is ever cut in half, and text outside a match comes back byte for byte. + * + * A pattern matches its identifier whole, or captures the boundary before it in + * group 1 and the identifier in group 2. + */ +function redactIdentifiers(value: string, rules: [RegExp, Redaction][]): string { + let text = value + let masked = value + .replace(UNICODE_SPACE_PATTERN, ' ') + .replace(ESCAPE_PATTERN, (escape) => ESCAPE_MASK.repeat(escape.length)) + for (const [pattern, redact] of rules) { + let redactedText = '' + let redactedMasked = '' + let cursor = 0 + pattern.lastIndex = 0 + for (let match = pattern.exec(masked); match !== null; match = pattern.exec(masked)) { + const identifier = match[2] ?? match[0] + const start = match.index + match[0].length - identifier.length + const replacement = redact(identifier, start, masked) + if (replacement !== identifier) { + redactedText += text.slice(cursor, start) + replacement + redactedMasked += masked.slice(cursor, start) + replacement + cursor = start + identifier.length + } + } + text = redactedText + text.slice(cursor) + masked = redactedMasked + masked.slice(cursor) + } + return text } function ipv4ToNumber(address: string): number { @@ -760,27 +748,30 @@ function isPublicIpv4(address: string): boolean { } /** - * {@link redactCardInMatch}, writing a card that stands as a bare JSON number as - * the string `"[redacted]"`, so a tool result's JSON text copy stays valid JSON. + * {@link redactCardInMatch} for one text, writing a card that stands as a bare + * JSON number as the string `"[redacted]"`, so JSON text stays valid JSON. A card + * stands outside every JSON string when an even number of quotes precede it; + * escaped quotes are masked, so they are not counted. The count carries over from + * one card to the next, so a text full of cards is read once, not once per card. */ -function redactCardKeepingJson( - _match: string, - prefix: string, - candidate: string, - offset: number, - text: string -): string { - const redacted = redactCardInMatch(candidate) - const start = offset + prefix.length - const end = start + candidate.length - return ( - prefix + - (redacted === REDACTED_VALUE && - JSON_VALUE_START_PATTERN.test(text.slice(Math.max(0, start - 64), start)) && - JSON_VALUE_END_PATTERN.test(text.slice(end, end + 64)) +function cardRedaction(): Redaction { + let quotes = 0 + let counted = 0 + return (candidate, start, masked) => { + const redacted = redactCardInMatch(candidate) + if (redacted !== REDACTED_VALUE) { + return redacted + } + for (; counted < start; counted++) { + quotes += masked[counted] === '"' ? 1 : 0 + } + const end = start + candidate.length + return quotes % 2 === 0 && + JSON_VALUE_START_PATTERN.test(masked.slice(Math.max(0, start - 64), start)) && + JSON_VALUE_END_PATTERN.test(masked.slice(end, end + 64)) ? `"${REDACTED_VALUE}"` - : redacted) - ) + : REDACTED_VALUE + } } function isCardNumber(value: number): boolean { @@ -795,17 +786,15 @@ function isCardNumber(value: number): boolean { * nothing to redact comes back as received, Unicode spaces included. */ export function redactPayloadPii(value: string): string { - const normalized = value.replace(UNICODE_SPACE_PATTERN, ' ') - let result = normalized.replace(EMAIL_PATTERN, REDACTED_VALUE) - result = result.replace(PAYLOAD_IPV4_PATTERN, (match, prefix: string, address: string) => - isPublicIpv4(address) ? prefix + REDACTED_VALUE : match - ) - result = result.replace(IPV6_PATTERN, `$1${REDACTED_VALUE}`) - result = result.replace(PAYLOAD_CARD_CANDIDATE_PATTERN, redactCardKeepingJson) - result = result.replace(US_SSN_PATTERN, REDACTED_VALUE) - result = result.replace(PHONE_PARENTHESIZED_PATTERN, `$1${REDACTED_VALUE}`) - result = result.replace(PAYLOAD_PHONE_INTL_PATTERN, `$1${REDACTED_VALUE}`) - return result === normalized ? value : result + return redactIdentifiers(value, [ + [EMAIL_PATTERN, redactWhole], + [PAYLOAD_IPV4_PATTERN, (address) => (isPublicIpv4(address) ? REDACTED_VALUE : address)], + [IPV6_PATTERN, redactWhole], + [PAYLOAD_CARD_CANDIDATE_PATTERN, cardRedaction()], + [US_SSN_PATTERN, redactWhole], + [PHONE_PARENTHESIZED_PATTERN, redactWhole], + [PAYLOAD_PHONE_INTL_PATTERN, redactWhole], + ]) } export function sanitizeCapturedValue(value: unknown, options: TextSanitizationOptions): unknown { @@ -830,15 +819,6 @@ function payloadPasses(options: TextSanitizationOptions): ValuePasses { } } -/** What the free-text pass rewrites in JSON written as free text. */ -function freeTextPasses(options: TextSanitizationOptions): ValuePasses { - return { - string: (value) => sanitizeFreeText(value, options), - key: (key) => redactCapturedHead(key, redactPii, options), - number: redactCardNumber, - } -} - /** * {@link sanitizeCapturedValue} with the intent-grade string pass * ({@link sanitizeFreeText}) on every string leaf, so nested agent-narrated @@ -893,7 +873,7 @@ function sanitizeValueWith(value: unknown, passes: ValuePasses): unknown { return value } - const result: JsonRecord = {} + const result = new Map() const receivedKeys = new Map() for (const [key, nestedValue] of Object.entries(value)) { const sanitizedKey = passes.key ? passes.key(key) : key @@ -902,7 +882,8 @@ function sanitizeValueWith(value: unknown, passes: ValuePasses): unknown { continue } receivedKeys.set(sanitizedKey, key) - result[sanitizedKey] = shouldRedactKey(key) ? REDACTED_VALUE : sanitizeValueWith(nestedValue, passes) + result.set(sanitizedKey, shouldRedactKey(key) ? REDACTED_VALUE : sanitizeValueWith(nestedValue, passes)) } - return result + // Defined rather than assigned, so a parsed `__proto__` member stays a member instead of setting the prototype. + return Object.fromEntries(result) } diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index 558c111a77..c4d742cbc7 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -46,6 +46,29 @@ describe('buildCapturedMcpParameters', () => { }) }) + it('keeps a parsed __proto__ argument', () => { + const request = JSON.parse('{"method":"tools/call","params":{"name":"t","arguments":{"__proto__":{"x":1}}}}') + + expect(JSON.stringify(buildCapturedMcpParameters(request))).toBe( + '{"request":{"method":"tools/call","params":{"name":"t","arguments":{"__proto__":{"x":1}}}}}' + ) + }) + + it.each([100, 100_000])('reads no more than the entry limit of %i arguments', (count) => { + let reads = 0 + const args: Record = {} + for (let index = 0; index < count; index++) { + Object.defineProperty(args, `k${index}`, { + enumerable: true, + get: () => (reads++, `customer ${index} order ORD-${index}`), + }) + } + + buildCapturedMcpParameters({ method: 'tools/call', params: { name: 't', arguments: args } }) + + expect(reads).toBeLessThanOrEqual(200) + }) + it('redacts PostHog tokens from captured string values', () => { const parameters = buildCapturedMcpParameters({ method: 'tools/call', @@ -580,6 +603,7 @@ describe('redactPayloadPii', () => { ['a card-shaped number with a fraction', '[4111111111111111.5]'], ['a card-shaped number with an exponent', '[4111111111111111e0]'], ['a card grouped with dots', 'ref 4111.1111.1111.1111'], + ['a negative card-shaped JSON member', '{"n":-4111111111111111}'], ['a fraction starting 011', '[620.0119256973267]'], ['a fraction starting 00', '[0.0012345678901]'], ])('keeps %s', (_label, input) => { @@ -594,8 +618,51 @@ describe('redactPayloadPii', () => { }) }) -describe('payload JSON text copies', () => { - const sanitize = (value: unknown): unknown => sanitizeCapturedValue(value, { redactPayloadPersonalData: true }) +describe('card numbers', () => { + it.each([ + ['grouped 4-4-4-3', 'amex 3782 8224 6310 005', 'amex [redacted]', 'amex [redacted]'], + ['grouped 4-4-5', 'visa 4222 2222 22222', 'visa [redacted]', 'visa [redacted]'], + ['grouped 4-4-4-1', 'visa 4222 2222 2222 2', 'visa [redacted]', 'visa [redacted]'], + ['grouped 5-5-5-1', 'pan 41111 11111 11111 1', 'pan [redacted]', 'pan [redacted]'], + ['grouped in pairs', 'pan 41 11 11 11 11 11 11 11', 'pan [redacted]', 'pan [redacted]'], + ['grouped 4-4-4-4-3', 'pan 6212 3456 7890 1234 569', 'pan [redacted]', 'pan [redacted]'], + ['glued to a word by a dash', 'card-4111111111111111', 'card-[redacted]', 'card-[redacted]'], + ['glued to a word by a dot', 'No.4111111111111111', 'No.[redacted]', 'No.[redacted]'], + ['grouped by dashes and glued by one', 'ref-4111-1111-1111-1111', 'ref-[redacted]', 'ref-[redacted]'], + [ + 'followed by an SSN', + 'card 4111111111111111 078-05-1120', + 'card [redacted] [redacted]', + 'card [redacted] [redacted]', + ], + [ + 'grouped by mixed separators with a dot', + 'card 4111-1111 1111.1111 ok', + 'card [redacted] ok', + 'card 4111-1111 1111.1111 ok', + ], + ['grouped by slashes', 'card 4111/1111/1111/1111', 'card [redacted]', 'card 4111/1111/1111/1111'], + ['with a fraction', 'cards 4111111111111111.1 x', 'cards [redacted].1 x', 'cards 4111111111111111.1 x'], + ])('%s: %s', (_label, input, intent, payload) => { + expect(redactPii(input)).toBe(intent) + expect(redactPayloadPii(input)).toBe(payload) + }) + + it.each([ + ['escaped line breaks', '{"pan":"4111\\n1111\\n1111\\n1111"}'], + ['line breaks', '4111\n1111\n1111\n1111'], + ['an escaped line break inside a phone number', '{"phone":"+44\\n20 7946 0958"}'], + ])('does not join digit groups across %s', (_label, input) => { + expect(redactPii(input)).toBe(input) + expect(redactPayloadPii(input)).toBe(input) + }) +}) + +describe('JSON text', () => { + const passes = [ + ['payload', (value: unknown): unknown => sanitizeCapturedValue(value, { redactPayloadPersonalData: true })], + ['intent', sanitizeFreeTextValue], + ] as const const identifiers = [ ['an email', 'ada@example.com'], ['a card', '4111111111111111'], @@ -603,33 +670,87 @@ describe('payload JSON text copies', () => { ['a public IPv4', '203.0.113.7'], ['an IPv6', '2001:db8::7334'], ['a phone', '+1 415 555 0142'], - ] - const escapes = ['\\n', '\\t', '\\r', '\\b', '\\f', '\\u00e9', '\\\\', '\\"', '\\/'] - const cases = identifiers.flatMap(([label, identifier]) => - escapes.map((escape) => [label, escape, identifier] as const) - ) + ['a PostHog token', 'phc_123456789012345678901234567890'], + ] as const + const escapes = ['\\n', '\\t', '\\r', '\\b', '\\f', '\\/', '\\\\', '\\"', '\\u00e9'] + const containers = [ + ['plain', (json: string) => json], + ['embedded in prose', (json: string) => `Result: ${json}`], + ['NDJSON', (json: string) => `{"a":1}\n${json}`], + ['after a BOM', (json: string) => `\uFEFF${json}`], + ] as const + const member = (escape: string, value: string): string => `{"note":"x${escape}${value}","x${escape}${value}":1}` + + describe.each(passes)('through the %s pass', (_pass, sanitize) => { + it.each( + identifiers.flatMap(([label, identifier]) => + escapes.flatMap((escape) => containers.map(([name, wrap]) => [label, escape, name, identifier, wrap] as const)) + ) + )('redacts %s after %s, %s, in place', (_label, escape, _container, identifier, wrap) => { + expect(sanitize(wrap(member(escape, identifier)))).toBe(wrap(member(escape, '[redacted]'))) + }) - it.each(cases)('redacts %s after %s in a value and a key, keeping valid JSON', (_label, escape, identifier) => { - const text = `{"note":"x${escape}${identifier}","x${escape}${identifier}":1}` + it.each(escapes.flatMap((escape) => containers.map(([name, wrap]) => [escape, name, wrap] as const)))( + 'keeps text with %s and nothing to redact byte for byte, %s', + (escape, _container, wrap) => { + const text = wrap(`{"note":"x${escape}y 12.5${escape}4111","x${escape}":[-1.5e3]}`) + + expect(sanitize(text)).toBe(text) + } + ) - const redacted = sanitize(text) as string + it.each([ + ['duplicate keys', '{"email":"ada@example.com","email":null}', '{"email":"[redacted]","email":null}'], + ['a 17-digit card integer', '{"pan":62123456789012347}', '{"pan":"[redacted]"}'], + ['a 19-digit card integer', '[6212345678901234569, 1]', '["[redacted]", 1]'], + [ + 'a __proto__ member', + '{"__proto__":{"email":"ada@example.com"},"y":2}', + '{"__proto__":{"email":"[redacted]"},"y":2}', + ], + ['a card between commas inside a string', '{"note":"a, 4111111111111111, b"}', '{"note":"a, [redacted], b"}'], + [ + 'pretty-printed JSON, keeping its formatting', + JSON.stringify({ card: 4111111111111111, note: 'x\nada@example.com', count: 3 }, null, 2), + JSON.stringify({ card: '[redacted]', note: 'x\n[redacted]', count: 3 }, null, 2), + ], + [ + 'JSON nested deeper than any walk', + `${'['.repeat(50_000)}"ada@example.com"${']'.repeat(50_000)}`, + `${'['.repeat(50_000)}"[redacted]"${']'.repeat(50_000)}`, + ], + ])('redacts %s in place', (_label, text, expected) => { + expect(sanitize(text)).toBe(expected) + }) - expect(redacted).not.toContain(identifier) - expect(JSON.parse(redacted)).toEqual(sanitize(JSON.parse(text))) + it.each([ + ['a __proto__ member', '{"__proto__":{"x":1},"y":2}'], + ['an integer above 2^53', '{"id":9007199254740993,"n":-0}'], + ['pretty-printed JSON with nothing to redact', JSON.stringify({ id: 'ord_1', total: 12.5 }, null, 2)], + ])('keeps %s byte for byte', (_label, text) => { + expect(sanitize(text)).toBe(text) + }) }) it.each([ ['a float whose fraction passes the card checksum', '{"v":710.4032039642334}'], ['a fraction of 16 digits', '[0.4111111111111111]'], ['a negative card-shaped number', '[-4111111111111111]'], + ['a negative card-shaped member', '{"n":-4111111111111111}'], ['a card-shaped number with a fraction', '[4111111111111111.5]'], ['a card-shaped number scaled to a fraction', '[4111111111111111e-3]'], - ['pretty-printed JSON with nothing to redact', JSON.stringify({ id: 'ord_1', total: 12.5 }, null, 2)], - ])('keeps %s byte for byte', (_label, text) => { - expect(sanitize(text)).toBe(text) + ])('keeps %s byte for byte through the payload pass', (_label, text) => { + expect(sanitizeCapturedValue(text, { redactPayloadPersonalData: true })).toBe(text) + }) + + it.each(escapes)('redacts a PostHog token after %s with the payload option off', (escape) => { + const text = member(escape, 'phc_123456789012345678901234567890') + + expect(sanitizeCapturedValue(text)).toBe(member(escape, '[redacted]')) }) describe('random floats', () => { + const sanitize = (value: unknown): unknown => sanitizeCapturedValue(value, { redactPayloadPersonalData: true }) let seed = 42 const random = (): number => (seed = (seed * 1103515245 + 12345) % 2 ** 31) / 2 ** 31 const floats = Array.from({ length: 10_000 }, () => random() * 1000) @@ -640,25 +761,12 @@ describe('payload JSON text copies', () => { expect(texts.filter((text) => sanitize(text) !== text)).toEqual([]) }) - it('keeps them in a JSON text copy longer than the passes parse', () => { + it('keeps them in a JSON text copy longer than the scanned window', () => { const text = JSON.stringify(floats) expect(sanitize(text)).toBe(text) }) }) - - it('falls back to the text pass on JSON nested too deep to walk', () => { - const depth = 50_000 - const text = `${'['.repeat(depth)}"ada@example.com"${']'.repeat(depth)}` - - expect(sanitize(text)).toBe(`${'['.repeat(depth)}"[redacted]"${']'.repeat(depth)}`) - }) - - it('redacts a card-number integer and re-serializes the redacted structure', () => { - const text = JSON.stringify({ card: 4111111111111111, note: 'x\nada@example.com', count: 3 }, null, 2) - - expect(sanitize(text)).toBe('{"card":"[redacted]","note":"x\\n[redacted]","count":3}') - }) }) describe('sanitizing strings longer than truncation keeps', () => { diff --git a/packages/mcp/src/__tests__/sink.test.ts b/packages/mcp/src/__tests__/sink.test.ts index 6f998812c1..547ef0547a 100644 --- a/packages/mcp/src/__tests__/sink.test.ts +++ b/packages/mcp/src/__tests__/sink.test.ts @@ -100,3 +100,32 @@ describe('processMcpEvent cost', () => { expect(farPastLimit).toBeLessThan(10 * atLimit + 5) }) }) + +describe('processMcpEvent __proto__ members', () => { + it.each([ + [ + 'with personal data', + '{"__proto__":{"email":"ada@example.com"},"y":2}', + '{"__proto__":{"email":"[redacted]"},"y":2}', + ], + ['with nothing to redact', '{"__proto__":{"x":1},"y":2}', '{"__proto__":{"x":1},"y":2}'], + ])('keeps a parsed one %s in parameters, structuredContent, and its text copy', async (_label, json, expected) => { + const event: McpEvent = { + eventType: MCPAnalyticsEventType.mcpToolsCall, + sessionId: 'session-1', + timestamp: new Date(), + parameters: JSON.parse(json), + response: { content: [{ type: 'text', text: json }], structuredContent: JSON.parse(json) }, + } + + const processed = await processMcpEvent(event, { + enableExceptionAutocapture: false, + redactPayloadPersonalData: true, + }) + const response = processed!.event.response as { content: { text: string }[]; structuredContent: unknown } + + expect(JSON.stringify(processed!.event.parameters)).toBe(expected) + expect(JSON.stringify(response.structuredContent)).toBe(expected) + expect(response.content[0].text).toBe(expected) + }) +}) diff --git a/packages/mcp/src/extensions/mcp-payloads.ts b/packages/mcp/src/extensions/mcp-payloads.ts index 75c37d0eec..5e29416ef8 100644 --- a/packages/mcp/src/extensions/mcp-payloads.ts +++ b/packages/mcp/src/extensions/mcp-payloads.ts @@ -11,9 +11,10 @@ import { sanitizeFreeTextValue as sanitizeCoreFreeTextValue, type TextSanitizationOptions, } from '@posthog/core' -import { MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' +import { limitEntries, MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' const CONTEXT_ARGUMENT_NAME = 'context' +const CAPTURED_REQUEST_FIELDS = ['id', 'jsonrpc', 'method', 'params'] const SANITIZATION_OPTIONS: TextSanitizationOptions = { maxStringLength: MAX_STRING_LENGTH, truncationSuffix: TRUNCATION_SUFFIX, @@ -39,48 +40,27 @@ export function sanitizeFreeTextValue(value: unknown): unknown { return sanitizeCoreFreeTextValue(value, SANITIZATION_OPTIONS) } -function buildCapturedMcpArguments(argumentsValue: unknown): unknown { - if (!isRecord(argumentsValue)) { - return sanitizeCapturedValue(argumentsValue) - } - - const capturedArguments: JsonRecord = {} - for (const [key, value] of Object.entries(argumentsValue)) { - if (key === CONTEXT_ARGUMENT_NAME) { - continue - } - capturedArguments[key] = sanitizeCapturedValue(value) - } - return capturedArguments -} - -function buildCapturedMcpParams(params: unknown): unknown { - if (!isRecord(params)) { - return sanitizeCapturedValue(params) - } - - const capturedParams: JsonRecord = {} - for (const [key, value] of Object.entries(params)) { - capturedParams[key] = key === 'arguments' ? buildCapturedMcpArguments(value) : sanitizeCapturedValue(value) - } - return capturedParams +/** + * The request as captured: its id, JSON-RPC version, method, and params, without + * the injected `context` argument, which `$mcp_intent` carries. The entry and + * nesting limits apply before the sanitizer reads anything, so a call with a huge + * argument object costs what a call at those limits costs. + */ +export function buildCapturedMcpParameters(request: unknown): JsonRecord { + const captured = limitEntries({ request: pickCapturedFields(request) }) as { request: unknown } + omitContextArgument(captured.request) + return sanitizeCapturedValue(captured) as JsonRecord } -export function buildCapturedMcpParameters(request: unknown): JsonRecord { +function pickCapturedFields(request: unknown): unknown { if (!isRecord(request)) { - return { request: sanitizeCapturedValue(request) } - } - - const capturedRequest: JsonRecord = {} - for (const key of ['id', 'jsonrpc', 'method'] as const) { - if (key in request) { - capturedRequest[key] = sanitizeCapturedValue(request[key]) - } + return request } + return Object.fromEntries(CAPTURED_REQUEST_FIELDS.filter((key) => key in request).map((key) => [key, request[key]])) +} - if ('params' in request) { - capturedRequest.params = buildCapturedMcpParams(request.params) +function omitContextArgument(request: unknown): void { + if (isRecord(request) && isRecord(request.params) && isRecord(request.params.arguments)) { + delete request.params.arguments[CONTEXT_ARGUMENT_NAME] } - - return { request: capturedRequest } } diff --git a/packages/mcp/src/extensions/truncation.ts b/packages/mcp/src/extensions/truncation.ts index 91eedd5b37..7dd811f74e 100644 --- a/packages/mcp/src/extensions/truncation.ts +++ b/packages/mcp/src/extensions/truncation.ts @@ -161,24 +161,25 @@ function visitObject( maxStringLength: number, memo: WeakSet ): Record { - const result: Record = {} + const entries: [string, unknown][] = [] const keys = Object.keys(obj) let count = 0 for (const key of keys) { if (count >= maxBreadth) { - result['...'] = '[MaxProperties ~]' + entries.push(['...', '[MaxProperties ~]']) break } // Skip undefined values — matches JSON.stringify behavior (omits undefined properties) if (obj[key] === undefined) { continue } - result[key] = visit(obj[key], remainingDepth, maxBreadth, maxStringLength, memo) + entries.push([key, visit(obj[key], remainingDepth, maxBreadth, maxStringLength, memo)]) count++ } - return result + // Defined rather than assigned, so a parsed `__proto__` member stays a member instead of setting the prototype. + return Object.fromEntries(entries) } // --- Field-level truncation helpers --- @@ -434,12 +435,17 @@ export function limitPayloadEntries(event: T): T { const result: MutableEvent = { ...event } for (const field of ['parameters', 'response'] as const) { if (result[field] != null) { - result[field] = normalize(result[field], MAX_DEPTH, MAX_BREADTH, Infinity) + result[field] = limitEntries(result[field]) } } return result as T } +/** {@link limitPayloadEntries} for one payload, such as the parameters as they are captured. */ +export function limitEntries(payload: unknown): unknown { + return normalize(payload, MAX_DEPTH, MAX_BREADTH, Infinity) +} + /** * Applies layered truncation to an event: * 1. Field-level string limits (userIntent, resourceName, metadata fields, error.message) From 707642dc6a3838a4d58aced5b09ef996f20a8324 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Fri, 6 Nov 2026 18:18:26 -0300 Subject: [PATCH 4/7] fix(mcp): read printed card groupings, decoded escapes and whole JSON numbers What: - Payload cards: an unseparated 13-19 digit run, or groups printed as cards are (4-4-4-4, 4-4-4-4-3, 4-6-4, 4-6-5), Luhn-valid. The intent keeps the shortest Luhn run of any whole groups. Both are found by a loop instead of a backtracking pattern. The payload 00/011 phone prefix no longer counts after a dash (874-426-0063). - Escapes: \uXXXX reads as the character it encodes (a Unicode space as a space, a non-ASCII character as %, a control, quote or backslash as NUL). The same applies to literal non-ASCII text. A match maps back over whole escapes, so the output stays valid JSON. - JSON numbers in value position are kept or replaced whole by "[redacted]", whatever the indent. Payload: a non-negative token whose value is an integer card (4111...0, 4111...e0). Intent: that, or any token the card pass would touch. A text that is only a card becomes [redacted]. - The head scanned for a long string is cut at the window edge unless a URL crosses it; then it is cut where the URL starts. - buildCapturedMcpParameters drops `context` before the argument limit, copying at most 101 arguments. Why: the gate found payload false positives (date pairs 20%, phone pairs 42%, ZIP+4 26%, id lists 10%), escape-hidden emails and spaced cards, a fixed 64-char lookback that broke indented JSON, partial number tokens (-[redacted], [redacted].0), whole-value scans of whitespace-free input (745ms at 8MB), a stack overflow at 12MB, and a lost 100th argument. Tested: - pnpm --filter @posthog/mcp test:unit: 1775 passed (64 new cases fail on 7c039ad56) - pnpm --filter @posthog/core test:unit: 2356 passed, 1 skipped - browser vitest src/__tests__/webmcp.test.ts: 46 passed - lint, check-types (core, mcp), pnpm format: clean - pnpm --filter @posthog/mcp build && test:integration: exit 0, nest 37/39 with the 2 declared expected failures - gate3 probes: p4_fuzz 0 regressions in 200k; intent output identical to 7c039ad56 on 300k escape-free strings; p11_fp payload 0.0% on all six generators; p12_indent valid at 2/4/10; payload `1-1.` 25-42ms from 1MB to 20MB Reviewer notes: a URL that starts the window and runs past it is still judged on the whole value, as on main, so `a:` plus 12MB costs seconds. Direct redactPii/redactPayloadPii calls on 12MB no longer throw but take seconds. Negative payload cards stay kept. --- .changeset/mcp-payload-personal-data.md | 2 +- packages/core/src/privacy.ts | 449 +++++++++++++----- .../mcp/src/__tests__/mcp-payloads.test.ts | 195 +++++++- packages/mcp/src/extensions/mcp-payloads.ts | 32 +- 4 files changed, 543 insertions(+), 135 deletions(-) diff --git a/.changeset/mcp-payload-personal-data.md b/.changeset/mcp-payload-personal-data.md index ce01fced39..183f48419c 100644 --- a/.changeset/mcp-payload-personal-data.md +++ b/.changeset/mcp-payload-personal-data.md @@ -3,4 +3,4 @@ '@posthog/core': minor --- -Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails, Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped by single spaces or dashes, not line breaks), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number sent as a JSON integer becomes the string `"[redacted]"`; negative numbers and fractions are kept. A JSON or JavaScript string escape (`\n`, `\"`, `\u00e9`) no longer hides an identifier or a PostHog token written right after it, and a JSON text copy of a result is redacted in place, so it keeps its formatting and stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. +Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails (an accented local part such as `josé@example.com` included), Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped the way cards are printed: 4-4-4-4, 4-4-4-4-3, 4-6-5, or 4-6-4, by single spaces or dashes), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number written as a JSON number, `4111111111111111.0` and `4111111111111111e0` included, becomes the string `"[redacted]"`; negative numbers and fractions with digits after the point are kept. A text that is only a card number becomes `[redacted]`, unquoted. A JSON or JavaScript string escape (`\n`, `\"`, `\u00e9`) no longer hides an identifier or a PostHog token written right after it, an identifier written with `\uXXXX` escapes (`ada\u0040example.com`, `4111\u00201111\u00201111\u00201111`) is read as the characters they encode, and a JSON text copy of a result is redacted in place, so it keeps its formatting and stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. diff --git a/packages/core/src/privacy.ts b/packages/core/src/privacy.ts index 6569c05af3..1eb538e64e 100644 --- a/packages/core/src/privacy.ts +++ b/packages/core/src/privacy.ts @@ -105,14 +105,14 @@ const IPV6_PATTERN = // A separator (space, dot, or dash) is required between the 3-2-4 groups so bare // 9-digit IDs are never mistaken for an SSN. const US_SSN_PATTERN = /\b\d{3}[ .-]\d{2}[ .-]\d{4}\b/g -// A run of >=13 digits optionally grouped by a single space, dot, dash, or -// slash. This only marks the numeric region; `redactCardInMatch` 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. -const CREDIT_CARD_CANDIDATE_PATTERN = /\b\d(?:[ ./-]?\d){12,}\b/g -// Matches each separator-delimited digit group inside a card candidate. -const DIGIT_GROUP_PATTERN = /\d+/g +// How cards are printed when grouped, shortest first so a field after a card is +// left for its own pattern. An ungrouped card is one run of 13–19 digits. +const CARD_GROUPINGS = [ + [4, 4, 4, 4], + [4, 4, 4, 4, 3], + [4, 6, 4], + [4, 6, 5], +] // 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 @@ -136,24 +136,27 @@ const NON_PUBLIC_IPV4_RANGES = [ '169.254.0.0/16', '0.0.0.0/32', ] -// Digits grouped only by single spaces or dashes: a `.` makes a fraction or a -// version, not a card. A leading `-` or `.` belongs to a negative number or a -// fraction unless a letter comes before it (`card-4111…`, `No.4111…`), and a -// trailing `.` and digit or exponent to a number the run is only part of. -const PAYLOAD_CARD_CANDIDATE_PATTERN = /(^|[^\w.-]|[A-Za-z_][.-])(\d(?:[ -]?\d){12,})(?!\w|\.\d)/g const PHONE_PARENTHESIZED_PATTERN = /(^|[^\w+])((?:\+?1[ ./-]?)?\(\d{3}\)[ ./-]?\d{3}[ ./-]\d{4})(?![\w])/g -// Not after a `.`, where `00` or `011` starts a fraction's digits. -const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w.])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g -const JSON_VALUE_START_PATTERN = /[:,[]\s*$/ -const JSON_VALUE_END_PATTERN = /^\s*[,}\]]/ +// Not after a `.`, where `00` or `011` starts a fraction's digits, nor after a +// `-`, where it is the last group of a dashed number (`874-426-0063`). +const PAYLOAD_PHONE_INTL_PATTERN = /(^|[^\w.-])((?:\+|00|011)[ .-]?[1-9]\d{0,2}(?:[ ./()-]{0,2}\d){7,13})(?![\w])/g +// A number in JSON value position: after `:`, `,` or `[`, and before `,`, `}` or `]`. +const JSON_NUMBER_PATTERN = /([:,[]\s*)(-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?)(?=\s*[,}\]])/g +const JSON_NUMBER_PARTS_PATTERN = /^-?(\d+)(?:\.(\d+))?(?:[eE]([+-]?\d+))?$/ +const WORD_CHARACTER_PATTERN = /\w/ // A JSON or JavaScript string escape. Read as text, its letter or hex digits glue // to what follows: `"x\nada@example.com"` holds the address `nada@example.com`, // and `"x\n4111…"` has no boundary before the card. const ESCAPE_PATTERN = /\\(?:["\\/bfnrt]|u[0-9A-Fa-f]{4})/g -// What the identifier patterns read in place of each escape character: neither a -// word character nor any pattern's separator, so a masked escape ends an -// identifier without joining the digit groups on either side of it into one. +// What the identifier patterns read in place of an escaped control character, +// quote, or backslash: neither a word character nor any pattern's separator, so +// it ends an identifier without joining the digit groups on either side of it. const ESCAPE_MASK = '\u0000' +// What they read in place of a non-ASCII character other than a space, escaped or +// not: legal in an address's local part, so `josé@example.com` is redacted whole, +// but not a word character, so a card written right before one still ends there. +const NON_ASCII_STAND_IN = '%' +const NON_ASCII_PATTERN = /[\u0080-\uffff]/g function shouldRedactKey(key: string): boolean { return SENSITIVE_KEY_PATTERN.test(key) @@ -541,7 +544,7 @@ function isBinaryBlob(value: string): boolean { /** Replaces PostHog API keys wherever they appear in a string. */ function redactCredentials(value: string): string { - return redactIdentifiers(value, [[POSTHOG_TOKEN_PATTERN, redactWhole]]) + return redactIdentifiers(value, [[matching(POSTHOG_TOKEN_PATTERN), redactWhole]]) } /** Replaces every URL-shaped match, credentials and all. */ @@ -574,13 +577,17 @@ function redactCapturedHead( return redact(value) } -/** The scanned head, shortened to end just after the last character that ends a URL. */ +/** + * The scanned head, shortened only to where a URL that would cross the cut + * starts. Any URL after the last character that ends one runs to the cut. + */ function headLength(value: string, scannedHeadLength: number): number { - let end = scannedHeadLength - while (end > 0 && !URL_END_PATTERN.test(value[end - 1])) { - end-- + let lastUrlEnd = scannedHeadLength + while (lastUrlEnd > 0 && !URL_END_PATTERN.test(value[lastUrlEnd - 1])) { + lastUrlEnd-- } - return end + const crossingUrl = URL_PATTERN_ONCE.exec(value.slice(lastUrlEnd, scannedHeadLength)) + return crossingUrl ? lastUrlEnd + crossingUrl.index : scannedHeadLength } function sanitizeString(value: string, options: TextSanitizationOptions): string { @@ -630,46 +637,160 @@ function passesLuhn(digits: string): boolean { return sum % 10 === 0 } -// A card candidate can span more than one card plus adjacent fields (e.g. an -// expiry, or a second card). This redacts every card in it: scanning -// left-to-right over the whole separator-delimited digit groups, at each start it -// takes the shortest run whose joined digits are 13–19 long and pass Luhn, redacts -// just that span, and resumes after it; a group that begins no valid run is kept -// and skipped. The shortest run leaves a field after the card, such as the SSN in -// `4111111111111111 078-05-1120`, for its own pattern. 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. -function redactCardInMatch(match: string): string { - const groups: { digits: string; start: number; end: number }[] = [] - for (let m = DIGIT_GROUP_PATTERN.exec(match); m !== null; m = DIGIT_GROUP_PATTERN.exec(match)) { - groups.push({ digits: m[0], start: m.index, end: m.index + m[0].length }) +/** Where the card that starts at digit group `first` of a run ends, or -1. */ +type CardEnd = (run: string, first: number) => number + +/** How a pass reads card numbers: what joins digit groups, where a run of groups may start and end, and which groups make a card. */ +interface CardReading { + separators: string + startsAt: (text: string, index: number) => boolean + endsAt: (text: string, index: number) => boolean + cardEnd: CardEnd +} + +function isDigitAt(text: string, index: number): boolean { + const code = text.charCodeAt(index) + return code >= 48 && code <= 57 +} + +function isWordAt(text: string, index: number): boolean { + return index >= 0 && index < text.length && WORD_CHARACTER_PATTERN.test(text[index]) +} + +function digitsEnd(text: string, index: number): number { + let end = index + while (isDigitAt(text, end)) { + end++ } - let output = '' - let cursor = 0 - let first = 0 - while (first < groups.length) { + return end +} + +/** + * The intent reads any run of whole groups joined by one space, dot, dash, or + * slash: at each group, the shortest run whose 13–19 digits pass Luhn. The + * shortest leaves a field after the card, such as the SSN in + * `4111111111111111 078-05-1120`, for its own pattern. + */ +const INTENT_CARDS: CardReading = { + separators: ' ./-', + startsAt: (text, index) => !isWordAt(text, index - 1), + endsAt: (text, index) => !isWordAt(text, index), + cardEnd: (run, first) => { let digits = '' - let matchedLast = -1 - for (let last = first; last < groups.length; last++) { - digits += groups[last].digits + for (let groupStart = first; groupStart < run.length;) { + const groupEnd = digitsEnd(run, groupStart) + digits += run.slice(groupStart, groupEnd) if (digits.length > 19) { - break + return -1 } if (digits.length >= 13 && passesLuhn(digits)) { - matchedLast = last - break + return groupEnd } + groupStart = groupEnd + 1 } - if (matchedLast >= 0) { - output += match.slice(cursor, groups[first].start) + REDACTED_VALUE - cursor = groups[matchedLast].end - first = matchedLast + 1 + return -1 + }, +} + +/** + * The payload reads only digits grouped by single spaces or dashes, the way + * cards are printed: one group of 13–19 digits or one of {@link CARD_GROUPINGS}. + * Any other grouping is a date, a phone number, or a list of ids. A `.` makes a + * fraction or a version, so a leading `-` or `.` belongs to a negative number or + * a fraction unless a letter comes before it (`card-4111…`, `No.4111…`), and a + * trailing `.` and digit or a letter to a number the run is only part of. + */ +const PAYLOAD_CARDS: CardReading = { + separators: ' -', + startsAt: (text, index) => { + const before = text[index - 1] + return ( + index === 0 || + (before === '.' || before === '-' ? /[A-Za-z_]/.test(text[index - 2] ?? '') : !isWordAt(text, index - 1)) + ) + }, + endsAt: (text, index) => !isWordAt(text, index) && !(text[index] === '.' && isDigitAt(text, index + 1)), + cardEnd: (run, first) => { + const lengths: number[] = [] + for (let groupStart = first; groupStart < run.length && lengths.length < 5;) { + const groupEnd = digitsEnd(run, groupStart) + lengths.push(groupEnd - groupStart) + groupStart = groupEnd + 1 + } + if (lengths[0] >= 13) { + return isCardDigits(run.slice(first, first + lengths[0])) ? first + lengths[0] : -1 + } + for (const grouping of CARD_GROUPINGS) { + const end = first + grouping.reduce((sum, length) => sum + length + 1, -1) + if ( + grouping.every((length, index) => lengths[index] === length) && + passesLuhn(run.slice(first, end).replace(/\D/g, '')) + ) { + return end + } + } + return -1 + }, +} + +/** + * Finds the next run of 13 or more digits in whole groups that `reading` allows. + * A loop rather than a pattern: a backtracking pattern over a megabyte-long run + * of digit groups overflows the stack. + */ +function findCardRun(reading: CardReading): Finder { + return (text, from) => { + for (let start = from; start < text.length; start++) { + if (!isDigitAt(text, start) || isDigitAt(text, start - 1) || !reading.startsAt(text, start)) { + continue + } + let digits = 0 + let end = -1 + let groupStart = start + for (;;) { + const groupEnd = digitsEnd(text, groupStart) + digits += groupEnd - groupStart + if (digits >= 13 && reading.endsAt(text, groupEnd)) { + end = groupEnd + } + groupStart = groupEnd + 1 + if (!reading.separators.includes(text[groupEnd]) || !isDigitAt(text, groupStart)) { + break + } + } + if (end >= 0) { + return [start, end] + } + start = groupStart - 1 + } + return null + } +} + +/** + * Redacts every card in a run of digit groups. A run can hold more than one card + * plus adjacent fields, such as an expiry or a second card, so the groups are + * scanned left to right: a card is redacted and the scan resumes after it, and a + * group that starts no card is kept. + */ +function redactCardsInRun(run: string, cardEnd: CardEnd): string { + let output = '' + let cursor = 0 + for (let first = 0; first < run.length;) { + const end = cardEnd(run, first) + if (end >= 0) { + output += run.slice(cursor, first) + REDACTED_VALUE + cursor = end + first = end + 1 } else { - first++ + first = digitsEnd(run, first) + 1 } } - return output + match.slice(cursor) + return output + run.slice(cursor) +} + +function cardRule(reading: CardReading): Rule { + return [findCardRun(reading), (run) => redactCardsInRun(run, reading.cardEnd)] } /** @@ -683,53 +804,133 @@ function redactCardInMatch(match: string): string { */ export function redactPii(value: string): string { return redactIdentifiers(value, [ - [EMAIL_PATTERN, redactWhole], - [IPV4_PATTERN, redactWhole], - [IPV6_PATTERN, redactWhole], - [CREDIT_CARD_CANDIDATE_PATTERN, cardRedaction()], - [US_SSN_PATTERN, redactWhole], - [PHONE_NANP_PATTERN, redactWhole], - [PHONE_INTL_PATTERN, redactWhole], + [matching(EMAIL_PATTERN), redactWhole], + [matching(IPV4_PATTERN), redactWhole], + [matching(IPV6_PATTERN), redactWhole], + [matching(JSON_NUMBER_PATTERN), jsonNumberRedaction((token) => isIntegerCard(token) || holdsIntentCard(token))], + cardRule(INTENT_CARDS), + [matching(US_SSN_PATTERN), redactWhole], + [matching(PHONE_NANP_PATTERN), redactWhole], + [matching(PHONE_INTL_PATTERN), redactWhole], ]) } +function holdsIntentCard(text: string): boolean { + return redactIdentifiers(text, [cardRule(INTENT_CARDS)]) !== text +} + /** What one identifier is replaced with, given where it starts in the text the patterns read. */ -type Redaction = (identifier: string, start: number, masked: string) => string +type Redaction = (identifier: string, start: number, read: string) => string + +/** Where the next identifier at or after `from` starts and ends in the text the patterns read, if any. */ +type Finder = (read: string, from: number) => [start: number, end: number] | null + +type Rule = [find: Finder, redact: Redaction] const redactWhole: Redaction = () => REDACTED_VALUE /** - * Applies each pattern in turn, matching it against a copy of `value` that has - * the same length: horizontal Unicode spaces read as an ASCII space and every - * escape character as {@link ESCAPE_MASK}. A match never contains a masked - * character, so each replacement lands at the same offsets in `value`, no escape - * is ever cut in half, and text outside a match comes back byte for byte. - * - * A pattern matches its identifier whole, or captures the boundary before it in - * group 1 and the identifier in group 2. + * A pattern that matches its identifier whole, or captures the boundary before + * it in group 1 and the identifier in group 2. + */ +function matching(pattern: RegExp): Finder { + return (read, from) => { + pattern.lastIndex = from + const match = pattern.exec(read) + if (match === null) { + return null + } + const end = match.index + match[0].length + return [end - (match[2] ?? match[0]).length, end] + } +} + +/** A text as the identifier patterns read it, and where each of its characters came from. */ +interface EscapeReading { + read: string + /** The offset in the text of the character the patterns read at `index`; `read.length` maps to the text's end. */ + offset: (index: number) => number +} + +/** Horizontal Unicode spaces read as an ASCII space, and other non-ASCII characters as {@link NON_ASCII_STAND_IN}. */ +function readCharacters(text: string): string { + return text.replace(UNICODE_SPACE_PATTERN, ' ').replace(NON_ASCII_PATTERN, NON_ASCII_STAND_IN) +} + +/** + * Reads `text` as {@link readCharacters} does, and each escape in it as one + * character: an escaped control character, quote, or backslash as + * {@link ESCAPE_MASK}, and any other `\uXXXX` as the character it encodes would + * read, so `ada\u0040example.com` and `4111\u00201111…` read as what they are. */ -function redactIdentifiers(value: string, rules: [RegExp, Redaction][]): string { +function readEscapes(text: string): EscapeReading { + const spaced = readCharacters(text) + if (!text.includes('\\')) { + return { read: spaced, offset: (index) => index } + } + const offsets = new Int32Array(text.length + 1) + let read = '' + let cursor = 0 + const copyUpTo = (end: number): void => { + for (let index = cursor; index < end; index++) { + offsets[read.length + index - cursor] = index + } + read += spaced.slice(cursor, end) + } + for (const escape of spaced.matchAll(ESCAPE_PATTERN)) { + copyUpTo(escape.index) + offsets[read.length] = escape.index + read += escapeStandIn(escape[0]) + cursor = escape.index + escape[0].length + } + copyUpTo(text.length) + offsets[read.length] = text.length + return { read, offset: (index) => offsets[index] } +} + +function escapeStandIn(escape: string): string { + if (escape[1] !== 'u') { + return ESCAPE_MASK + } + const code = parseInt(escape.slice(2), 16) + if ( + code < 0x20 || + (code >= 0x7f && code <= 0x9f) || + code === 0x2028 || + code === 0x2029 || + code === 0x22 || + code === 0x5c + ) { + return ESCAPE_MASK + } + return readCharacters(String.fromCharCode(code)) +} + +/** + * Applies each rule in turn to `value` as {@link readEscapes} reads it, and + * splices each replacement over the characters the identifier was read from. + * An identifier that covers part of an escape covers all of it, so the text + * stays valid JSON, and text outside an identifier comes back byte for byte. + */ +function redactIdentifiers(value: string, rules: Rule[]): string { let text = value - let masked = value - .replace(UNICODE_SPACE_PATTERN, ' ') - .replace(ESCAPE_PATTERN, (escape) => ESCAPE_MASK.repeat(escape.length)) - for (const [pattern, redact] of rules) { - let redactedText = '' - let redactedMasked = '' + let reading = readEscapes(text) + for (const [find, redact] of rules) { + const { read, offset } = reading + let redacted = '' let cursor = 0 - pattern.lastIndex = 0 - for (let match = pattern.exec(masked); match !== null; match = pattern.exec(masked)) { - const identifier = match[2] ?? match[0] - const start = match.index + match[0].length - identifier.length - const replacement = redact(identifier, start, masked) + for (let span = find(read, 0); span !== null; span = find(read, span[1])) { + const identifier = read.slice(span[0], span[1]) + const replacement = redact(identifier, span[0], read) if (replacement !== identifier) { - redactedText += text.slice(cursor, start) + replacement - redactedMasked += masked.slice(cursor, start) + replacement - cursor = start + identifier.length + redacted += text.slice(cursor, offset(span[0])) + replacement + cursor = offset(span[1]) } } - text = redactedText + text.slice(cursor) - masked = redactedMasked + masked.slice(cursor) + if (cursor > 0) { + text = redacted + text.slice(cursor) + reading = readEscapes(text) + } } return text } @@ -748,37 +949,52 @@ function isPublicIpv4(address: string): boolean { } /** - * {@link redactCardInMatch} for one text, writing a card that stands as a bare - * JSON number as the string `"[redacted]"`, so JSON text stays valid JSON. A card - * stands outside every JSON string when an even number of quotes precede it; - * escaped quotes are masked, so they are not counted. The count carries over from - * one card to the next, so a text full of cards is read once, not once per card. + * Writes a JSON number for which `isCard` holds as the string `"[redacted]"`, + * whole, so JSON text stays valid JSON. A number stands outside every JSON + * string when an even number of quotes precede it; escaped quotes are masked, so + * they are not counted. The count carries over from one number to the next, so a + * text full of numbers is read once, not once per number. */ -function cardRedaction(): Redaction { +function jsonNumberRedaction(isCard: (token: string) => boolean): Redaction { let quotes = 0 let counted = 0 - return (candidate, start, masked) => { - const redacted = redactCardInMatch(candidate) - if (redacted !== REDACTED_VALUE) { - return redacted - } + return (token, start, read) => { for (; counted < start; counted++) { - quotes += masked[counted] === '"' ? 1 : 0 + quotes += read[counted] === '"' ? 1 : 0 } - const end = start + candidate.length - return quotes % 2 === 0 && - JSON_VALUE_START_PATTERN.test(masked.slice(Math.max(0, start - 64), start)) && - JSON_VALUE_END_PATTERN.test(masked.slice(end, end + 64)) - ? `"${REDACTED_VALUE}"` - : REDACTED_VALUE + return quotes % 2 === 0 && isCard(token) ? `"${REDACTED_VALUE}"` : token } } -function isCardNumber(value: number): boolean { - const digits = String(value) +function isCardDigits(digits: string): boolean { return /^\d{13,19}$/.test(digits) && passesLuhn(digits) } +function isCardNumber(value: number): boolean { + return isCardDigits(String(value)) +} + +/** + * Whether a JSON number token equals a card number, read from its digits rather + * than parsed, so a 19-digit card past 2^53 is judged exactly: + * `4111111111111111.0` and `41111111111111110e-1` are both the card. + */ +function isIntegerCard(token: string): boolean { + const [, whole, fraction = '', exponent = '0'] = JSON_NUMBER_PARTS_PATTERN.exec(token) ?? [] + if (whole === undefined) { + return false + } + const digits = whole + fraction + const significant = digits.replace(/^0+/, '') + const integerLength = whole.length + Number(exponent) - (digits.length - significant.length) + return ( + integerLength >= 13 && + integerLength <= 19 && + !/[1-9]/.test(significant.slice(integerLength)) && + isCardDigits(significant.slice(0, integerLength).padEnd(integerLength, '0')) + ) +} + /** * {@link redactPii} narrowed for tool arguments, results, and errors: an IPv4 * address only when public and not glued to a version, and a phone number only @@ -787,13 +1003,14 @@ function isCardNumber(value: number): boolean { */ export function redactPayloadPii(value: string): string { return redactIdentifiers(value, [ - [EMAIL_PATTERN, redactWhole], - [PAYLOAD_IPV4_PATTERN, (address) => (isPublicIpv4(address) ? REDACTED_VALUE : address)], - [IPV6_PATTERN, redactWhole], - [PAYLOAD_CARD_CANDIDATE_PATTERN, cardRedaction()], - [US_SSN_PATTERN, redactWhole], - [PHONE_PARENTHESIZED_PATTERN, redactWhole], - [PAYLOAD_PHONE_INTL_PATTERN, redactWhole], + [matching(EMAIL_PATTERN), redactWhole], + [matching(PAYLOAD_IPV4_PATTERN), (address) => (isPublicIpv4(address) ? REDACTED_VALUE : address)], + [matching(IPV6_PATTERN), redactWhole], + [matching(JSON_NUMBER_PATTERN), jsonNumberRedaction((token) => token[0] !== '-' && isIntegerCard(token))], + cardRule(PAYLOAD_CARDS), + [matching(US_SSN_PATTERN), redactWhole], + [matching(PHONE_PARENTHESIZED_PATTERN), redactWhole], + [matching(PAYLOAD_PHONE_INTL_PATTERN), redactWhole], ]) } diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index c4d742cbc7..c4a91866b0 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -3,6 +3,7 @@ import { redactPayloadPii, redactPii, sanitizeCapturedValue, + sanitizeFreeText, sanitizeFreeTextValue, } from '../extensions/mcp-payloads' @@ -69,6 +70,38 @@ describe('buildCapturedMcpParameters', () => { expect(reads).toBeLessThanOrEqual(200) }) + const realArguments = (count: number): [string, number][] => + Array.from({ length: count }, (_, index) => [`k${index}`, index]) + + it.each([ + ['first', 100], + ['first', 101], + ['in the middle', 100], + ['last', 100], + ['last', 101], + ['absent', 100], + ['absent', 101], + ])('keeps the first 100 real arguments with context %s among %i', (position, count) => { + const real = realArguments(count) + const context: [string, string][] = position === 'absent' ? [] : [['context', 'why']] + const entries = + position === 'first' + ? [...context, ...real] + : position === 'last' + ? [...real, ...context] + : [...real.slice(0, 50), ...context, ...real.slice(50)] + + const parameters = buildCapturedMcpParameters({ + method: 'tools/call', + params: { name: 't', arguments: Object.fromEntries(entries) }, + }) as { request: { params: { arguments: Record } } } + + expect(parameters.request.params.arguments).toEqual({ + ...Object.fromEntries(real.slice(0, 100)), + ...(count > 100 ? { '...': '[MaxProperties ~]' } : {}), + }) + }) + it('redacts PostHog tokens from captured string values', () => { const parameters = buildCapturedMcpParameters({ method: 'tools/call', @@ -456,6 +489,7 @@ describe('redactPii', () => { 'Looking up orders for [redacted] before refunding.', ], ['an email with a maximal 64-char local part', `from ${'a'.repeat(64)}@example.com now`, 'from [redacted] now'], + ['an email with an accented local part', 'Writing to josé@example.com now.', 'Writing to [redacted] now.'], [ 'an IPv4 address', 'Blocking traffic from 203.0.113.42 after abuse.', @@ -557,6 +591,7 @@ describe('redactPii', () => { describe('redactPayloadPii', () => { it.each([ ['an email address', 'Customer ada@example.com', 'Customer [redacted]'], + ['an email address with an accented local part', 'Customer josé@example.com', 'Customer [redacted]'], ['a public IPv4 after a space', 'last seen from 203.0.113.7', 'last seen from [redacted]'], ['a public IPv4 after a colon', 'client ip:203.0.113.7', 'client ip:[redacted]'], ['a public IPv4 after an @', 'user@203.0.113.7', 'user@[redacted]'], @@ -601,15 +636,44 @@ describe('redactPayloadPii', () => { ['a fraction of 16 digits', '[0.4111111111111111]'], ['a negative card-shaped number', '[-4111111111111111]'], ['a card-shaped number with a fraction', '[4111111111111111.5]'], - ['a card-shaped number with an exponent', '[4111111111111111e0]'], ['a card grouped with dots', 'ref 4111.1111.1111.1111'], ['a negative card-shaped JSON member', '{"n":-4111111111111111}'], ['a fraction starting 011', '[620.0119256973267]'], ['a fraction starting 00', '[0.0012345678901]'], + ['two dates', '2024-08-12 2022-03-03'], + ['two dates whose tail passes the card checksum', '2027-03-14 2020-08-20'], + ['two US phone-shaped numbers', '385-353-6556 200-426-7992'], + ['two US phone-shaped numbers, the first passing the checksum', '813-545-4304 607-403-3631'], + ['a US phone-shaped number ending in a zero-padded group', '874-426-0063 247-900-7184'], + ['six-digit ids', '769677 908755 886773'], + ['ZIP+4 codes', '98432-1669 62609-6852'], ])('keeps %s', (_label, input) => { expect(redactPayloadPii(input)).toBe(input) }) + describe('ordinary number lists', () => { + let seed = 99 + const random = (low: number, high: number): number => + low + Math.floor(((seed = (seed * 1103515245 + 12345) % 2 ** 31) / 2 ** 31) * (high - low + 1)) + const pad = (value: number, length: number): string => String(value).padStart(length, '0') + const date = (): string => `${random(2015, 2027)}-${pad(random(1, 12), 2)}-${pad(random(1, 28), 2)}` + const phone = (): string => `${random(200, 999)}-${random(200, 999)}-${pad(random(0, 9999), 4)}` + + it.each([ + ['date pairs', () => `${date()} ${date()}`], + ['US phone-shaped pairs', () => `${phone()} ${phone()}`], + ['six-digit id lists', () => `${random(100000, 999999)} ${random(100000, 999999)} ${random(100000, 999999)}`], + [ + 'ZIP+4 lists', + () => `${random(10000, 99999)}-${random(1000, 9999)} ${random(10000, 99999)}-${random(1000, 9999)}`, + ], + ])('keeps every one of 2000 %s', (_label, generate) => { + const lists = Array.from({ length: 2000 }, generate) + + expect(lists.filter((list) => redactPayloadPii(list) !== list)).toEqual([]) + }) + }) + it.each([ ['a US phone without a prefix', 'Order 415-555-0142'], ['a loopback address', 'db at 127.0.0.1'], @@ -620,12 +684,26 @@ describe('redactPayloadPii', () => { describe('card numbers', () => { it.each([ - ['grouped 4-4-4-3', 'amex 3782 8224 6310 005', 'amex [redacted]', 'amex [redacted]'], - ['grouped 4-4-5', 'visa 4222 2222 22222', 'visa [redacted]', 'visa [redacted]'], - ['grouped 4-4-4-1', 'visa 4222 2222 2222 2', 'visa [redacted]', 'visa [redacted]'], - ['grouped 5-5-5-1', 'pan 41111 11111 11111 1', 'pan [redacted]', 'pan [redacted]'], - ['grouped in pairs', 'pan 41 11 11 11 11 11 11 11', 'pan [redacted]', 'pan [redacted]'], + ['grouped 4-4-4-3', 'amex 3782 8224 6310 005', 'amex [redacted]', 'amex 3782 8224 6310 005'], + ['grouped 4-4-5', 'visa 4222 2222 22222', 'visa [redacted]', 'visa 4222 2222 22222'], + ['grouped 4-4-4-1', 'visa 4222 2222 2222 2', 'visa [redacted]', 'visa 4222 2222 2222 2'], + ['grouped 5-5-5-1', 'pan 41111 11111 11111 1', 'pan [redacted]', 'pan 41111 11111 11111 1'], + ['grouped in pairs', 'pan 41 11 11 11 11 11 11 11', 'pan [redacted]', 'pan 41 11 11 11 11 11 11 11'], ['grouped 4-4-4-4-3', 'pan 6212 3456 7890 1234 569', 'pan [redacted]', 'pan [redacted]'], + ['grouped 4-6-5', 'amex 3782 822463 10005', 'amex [redacted]', 'amex [redacted]'], + ['grouped 4-6-4', 'diners 3056 930902 5904', 'diners [redacted]', 'diners [redacted]'], + [ + 'grouped 4-4-4-4 after a leading group', + 'ref 1234 4111 1111 1111 1111', + 'ref 1234 [redacted]', + 'ref 1234 [redacted]', + ], + [ + 'grouped 4-4-4-4 and followed by an SSN', + 'card 4111 1111 1111 1111 078-05-1120', + 'card [redacted] [redacted]', + 'card [redacted] [redacted]', + ], ['glued to a word by a dash', 'card-4111111111111111', 'card-[redacted]', 'card-[redacted]'], ['glued to a word by a dot', 'No.4111111111111111', 'No.[redacted]', 'No.[redacted]'], ['grouped by dashes and glued by one', 'ref-4111-1111-1111-1111', 'ref-[redacted]', 'ref-[redacted]'], @@ -684,12 +762,67 @@ describe('JSON text', () => { describe.each(passes)('through the %s pass', (_pass, sanitize) => { it.each( identifiers.flatMap(([label, identifier]) => - escapes.flatMap((escape) => containers.map(([name, wrap]) => [label, escape, name, identifier, wrap] as const)) + escapes + // An escaped letter reads as part of the address after it; see the escaped characters below. + .filter((escape) => !(label === 'an email' && escape === '\\u00e9')) + .flatMap((escape) => containers.map(([name, wrap]) => [label, escape, name, identifier, wrap] as const)) ) )('redacts %s after %s, %s, in place', (_label, escape, _container, identifier, wrap) => { expect(sanitize(wrap(member(escape, identifier)))).toBe(wrap(member(escape, '[redacted]'))) }) + it.each([ + ['an address with an escaped @', '{"e":"ada\\u0040example.com"}', '{"e":"[redacted]"}'], + ['an address with an escaped dot', '{"e":"ada@example\\u002ecom"}', '{"e":"[redacted]"}'], + ['an address with an escaped last letter', '{"e":"ada@example.co\\u006d"}', '{"e":"[redacted]"}'], + ['an address with an escaped accented letter', '{"e":"jos\\u00e9@example.com"}', '{"e":"[redacted]"}'], + ['an address after an escaped letter, with it', '{"e":"x\\u00e9ada@example.com"}', '{"e":"[redacted]"}'], + ['an address after an escaped line break', '{"e":"x\\u000aada@example.com"}', '{"e":"x\\u000a[redacted]"}'], + ['a card grouped by escaped spaces', '{"c":"4111\\u00201111\\u00201111\\u00201111"}', '{"c":"[redacted]"}'], + ['a card grouped by escaped NNBSP', '{"c":"4111\\u202f1111\\u202f1111\\u202f1111"}', '{"c":"[redacted]"}'], + ['a card grouped by escaped dashes', '{"c":"4111\\u002d1111\\u002d1111\\u002d1111"}', '{"c":"[redacted]"}'], + ['a card before an escaped letter', '{"c":"4111111111111111\\u00e9"}', '{"c":"[redacted]\\u00e9"}'], + ['an SSN grouped by escaped dashes', '{"s":"078\\u002d05\\u002d1120"}', '{"s":"[redacted]"}'], + ['a phone grouped by escaped spaces', '{"p":"+1\\u0020415\\u0020555\\u00200142"}', '{"p":"[redacted]"}'], + ['a phone grouped by an escaped NBSP', '{"p":"+44\\u00a020 7946 0958"}', '{"p":"[redacted]"}'], + ['a public IPv4 with an escaped dot', '{"i":"203.0.113\\u002e7"}', '{"i":"[redacted]"}'], + ['a PostHog token with an escaped letter', '{"t":"phc_\\u0061bcdefghijklmnopqrstuvwxyz"}', '{"t":"[redacted]"}'], + ['a card after an escaped quote', '{"a":"\\u0022","n":4111111111111111}', '{"a":"\\u0022","n":"[redacted]"}'], + ])('redacts %s, keeping the JSON valid', (_label, text, expected) => { + const sanitized = sanitize(text) as string + + expect(sanitized).toBe(expected) + expect(() => JSON.parse(sanitized)).not.toThrow() + }) + + it.each([ + ['a card with a zero fraction', '{"n":4111111111111111.0}'], + ['a card with an exponent', '[4111111111111111e0]'], + ['a card written with a fraction and an exponent', '[41111111111111110e-1]'], + ['a 19-digit card with a zero fraction', '[6212345678901234569.00]'], + ])('replaces %s written as a JSON number whole', (_label, text) => { + const sanitized = sanitize(text) as string + const parsed: unknown = JSON.parse(sanitized) + + expect(JSON.stringify(parsed)).not.toMatch(/\d{13}/) + expect(sanitized).toContain('"[redacted]"') + }) + + it.each([2, 4, 10])('quotes a card nested 7 levels deep in JSON indented by %i', (indent) => { + let value: unknown = [4111111111111111] + for (let level = 0; level < 6; level++) { + value = { level: value } + } + + expect(JSON.parse(sanitize(JSON.stringify(value, null, indent)) as string)).toEqual( + JSON.parse(JSON.stringify(value).replace('4111111111111111', '"[redacted]"')) + ) + }) + + it('reads a text that is only a card number as prose, leaving it unquoted', () => { + expect(sanitize('4111111111111111')).toBe('[redacted]') + }) + it.each(escapes.flatMap((escape) => containers.map(([name, wrap]) => [escape, name, wrap] as const)))( 'keeps text with %s and nothing to redact byte for byte, %s', (escape, _container, wrap) => { @@ -743,6 +876,10 @@ describe('JSON text', () => { expect(sanitizeCapturedValue(text, { redactPayloadPersonalData: true })).toBe(text) }) + it('replaces a negative card written as a JSON number whole through the intent pass', () => { + expect(sanitizeFreeTextValue('{"a":-4111111111111111}')).toBe('{"a":"[redacted]"}') + }) + it.each(escapes)('redacts a PostHog token after %s with the payload option off', (escape) => { const text = member(escape, 'phc_123456789012345678901234567890') @@ -764,8 +901,46 @@ describe('JSON text', () => { it('keeps them in a JSON text copy longer than the scanned window', () => { const text = JSON.stringify(floats) - expect(sanitize(text)).toBe(text) + expect(sanitize(text)).toBe(`${text.slice(0, 131_072)}...`) + }) + }) +}) + +describe('sanitizing a long string with no place to cut', () => { + const medianMilliseconds = (run: () => unknown): number => { + const times = Array.from({ length: 5 }, () => { + const start = performance.now() + run() + return performance.now() - start }) + return times.sort((a, b) => a - b)[2] + } + const sanitizers = [ + ['payload', (value: string) => sanitizeCapturedValue(value, { redactPayloadPersonalData: true })], + ['payload option off', (value: string) => sanitizeCapturedValue(value)], + ['intent', (value: string) => sanitizeFreeText(value)], + ] as const + const units = [ + ['digits joined by - and .', '1-1.'], + ['comma-separated ids', '1000000,'], + ] as const + + it.each( + sanitizers.flatMap(([name, sanitize]) => units.map(([label, unit]) => [name, label, unit, sanitize] as const)) + )('costs the %s pass on 8MB of %s about what it costs on 1MB', (_name, _label, unit, sanitize) => { + const small = unit.repeat(1_000_000 / unit.length) + const large = unit.repeat(8_000_000 / unit.length) + sanitize(small) + + expect(medianMilliseconds(() => sanitize(large)) / medianMilliseconds(() => sanitize(small))).toBeLessThan(3) + }) + + it.each([ + ['redactPii', redactPii], + ['redactPayloadPii', redactPayloadPii], + ['the intent pass', sanitizeFreeText], + ] as const)('%s does not overflow the stack on 12MB of digits joined by - and .', (_name, redact) => { + expect(() => redact('1-1.'.repeat(3_000_000))).not.toThrow() }) }) @@ -775,8 +950,8 @@ describe('sanitizing strings longer than truncation keeps', () => { const filler = (length: number): string => 'lorem ipsum '.repeat(Math.ceil(length / 12)).slice(0, length) it.each([ - ['plain text', filler(5_000_000), `${filler(WINDOW - 2)}...`], - ['a token cut at the window edge', filler(WINDOW - 32) + token, `${filler(WINDOW - 32)}...`], + ['plain text', filler(5_000_000), `${filler(WINDOW)}...`], + ['a token cut at the window edge', filler(WINDOW - 32) + token, `${filler(WINDOW - 32)}[redacted]...`], [ 'a credential URL cut at the window edge', `${filler(WINDOW - 44)}https://example.com/?api_key=secretsecretsecretsecret more`, diff --git a/packages/mcp/src/extensions/mcp-payloads.ts b/packages/mcp/src/extensions/mcp-payloads.ts index 5e29416ef8..971bb3d00f 100644 --- a/packages/mcp/src/extensions/mcp-payloads.ts +++ b/packages/mcp/src/extensions/mcp-payloads.ts @@ -11,7 +11,7 @@ import { sanitizeFreeTextValue as sanitizeCoreFreeTextValue, type TextSanitizationOptions, } from '@posthog/core' -import { limitEntries, MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' +import { limitEntries, MAX_BREADTH, MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' const CONTEXT_ARGUMENT_NAME = 'context' const CAPTURED_REQUEST_FIELDS = ['id', 'jsonrpc', 'method', 'params'] @@ -47,20 +47,36 @@ export function sanitizeFreeTextValue(value: unknown): unknown { * argument object costs what a call at those limits costs. */ export function buildCapturedMcpParameters(request: unknown): JsonRecord { - const captured = limitEntries({ request: pickCapturedFields(request) }) as { request: unknown } - omitContextArgument(captured.request) - return sanitizeCapturedValue(captured) as JsonRecord + return sanitizeCapturedValue(limitEntries({ request: pickCapturedFields(request) })) as JsonRecord } function pickCapturedFields(request: unknown): unknown { if (!isRecord(request)) { return request } - return Object.fromEntries(CAPTURED_REQUEST_FIELDS.filter((key) => key in request).map((key) => [key, request[key]])) + const fields = Object.fromEntries( + CAPTURED_REQUEST_FIELDS.filter((key) => key in request).map((key) => [key, request[key]]) + ) + const { params } = fields + if (isRecord(params) && isRecord(params.arguments) && Object.hasOwn(params.arguments, CONTEXT_ARGUMENT_NAME)) { + fields.params = { ...params, arguments: argumentsWithoutContext(params.arguments) } + } + return fields } -function omitContextArgument(request: unknown): void { - if (isRecord(request) && isRecord(request.params) && isRecord(request.params.arguments)) { - delete request.params.arguments[CONTEXT_ARGUMENT_NAME] +/** + * The arguments without `context`, so it does not count toward the entry limit. + * Copies one more than the limit, enough for the limit to see that more remain. + */ +function argumentsWithoutContext(args: JsonRecord): JsonRecord { + const kept: [string, unknown][] = [] + for (const key of Object.keys(args)) { + if (kept.length > MAX_BREADTH) { + break + } + if (key !== CONTEXT_ARGUMENT_NAME) { + kept.push([key, args[key]]) + } } + return Object.fromEntries(kept) } From ed74ee24e87ada0732ca97c3509e1fe7cbde64e8 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Fri, 6 Nov 2026 18:31:23 -0300 Subject: [PATCH 5/7] fix(mcp): replace a URL that runs past the scan window instead of reading it whole What: when a URL crosses the 131072-character scan window, sanitizeString and sanitizeFreeText now replace it whole with `[redacted]` and stop at the window edge. Before, the cut moved back to where that URL started, and a value opening with one fell back to processing the whole string. Object keys keep the plain window (they get no URL pass). Why: cost must follow the property limits, not the payload. `a:` + 12MB took 9.8-16s through the intent pass and 3-3.6s through the payload pass. Cutting the URL at the edge instead would leak, since `https:secretuser...` without its `@` reads as having no userinfo. The trailing URL is found after the personal-data pass, so an email written into a URL's host (`john@ex.com:...`) is still redacted. Behavior change from main (#5186), noted in the changeset: a URL longer than the rest of the window is now `[redacted]` and nothing after it is captured. A `data:` image inlined in a long HTML page is the visible case. Tested: - New tests failed on 707642dc6: 8MB/1MB cost ratio 5.3-9.4 (limit 3) for a value opening with a URL whose userinfo and token sit past the window, on the intent, payload-on and payload-off passes; window outputs for that value and for the four pinned URL cases (credential URL at the edge, credentials past the window, field count past the window, pre-signed URLs) plus the inlined-image test. - pnpm --filter @posthog/core test:unit: 2356 passed, 1 skipped - pnpm --filter @posthog/mcp test:unit: 1779 passed - browser webmcp.test.ts: 46 passed - pnpm format, lint (core, mcp, oxlint), check-types (core, mcp): clean - pnpm --filter @posthog/mcp build && test:integration: exit 0 (2 declared expected failures) - a: + 12MB now 26ms intent, 23ms payload on, 2ms payload off; p6/p8/p9 probes flat across 1-20MB. Reviewer note: the shrink fallback (head under 2x the limit after redaction, cut on whitespace) still reads the whole string. 8MB of 1024-char pre-signed URL lines costs 2.2s vs 0.3s for 1MB. Not addressed here. --- .changeset/mcp-payload-personal-data.md | 2 +- packages/core/src/privacy.ts | 73 +++++++++++++------ .../mcp/src/__tests__/mcp-payloads.test.ts | 44 ++++++----- 3 files changed, 78 insertions(+), 41 deletions(-) diff --git a/.changeset/mcp-payload-personal-data.md b/.changeset/mcp-payload-personal-data.md index 183f48419c..cc17e10328 100644 --- a/.changeset/mcp-payload-personal-data.md +++ b/.changeset/mcp-payload-personal-data.md @@ -3,4 +3,4 @@ '@posthog/core': minor --- -Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails (an accented local part such as `josé@example.com` included), Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped the way cards are printed: 4-4-4-4, 4-4-4-4-3, 4-6-5, or 4-6-4, by single spaces or dashes), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number written as a JSON number, `4111111111111111.0` and `4111111111111111e0` included, becomes the string `"[redacted]"`; negative numbers and fractions with digits after the point are kept. A text that is only a card number becomes `[redacted]`, unquoted. A JSON or JavaScript string escape (`\n`, `\"`, `\u00e9`) no longer hides an identifier or a PostHog token written right after it, an identifier written with `\uXXXX` escapes (`ada\u0040example.com`, `4111\u00201111\u00201111\u00201111`) is read as the characters they encode, and a JSON text copy of a result is redacted in place, so it keeps its formatting and stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. +Tool arguments, tool results, error messages, exception values, and resource read URIs now have personal data redacted by default, as the intent already did. Emails (an accented local part such as `josé@example.com` included), Luhn-valid card numbers (13 to 19 digits, ungrouped or grouped the way cards are printed: 4-4-4-4, 4-4-4-4-3, 4-6-5, or 4-6-4, by single spaces or dashes), US social security numbers, IPv6 and public IPv4 addresses, and phone numbers written with an international prefix (`+`, `00`, `011`) or a parenthesized area code become `[redacted]`, in string values and object keys at any depth. A card number written as a JSON number, `4111111111111111.0` and `4111111111111111e0` included, becomes the string `"[redacted]"`; negative numbers and fractions with digits after the point are kept. A text that is only a card number becomes `[redacted]`, unquoted. A JSON or JavaScript string escape (`\n`, `\"`, `\u00e9`) no longer hides an identifier or a PostHog token written right after it, an identifier written with `\uXXXX` escapes (`ada\u0040example.com`, `4111\u00201111\u00201111\u00201111`) is read as the characters they encode, and a JSON text copy of a result is redacted in place, so it keeps its formatting and stays valid JSON. Private, loopback, and link-local addresses, versions written after a letter (`v1.2.3.4`), and `415-555-0142`-style numbers are kept. Some identifiers still read as personal data and are redacted: `ddd-dd-dddd` references (read as social security numbers), a four-part version at the start of a string or after a space, `:`, or `@` (`22.11.0.3`, `node:22.11.0.3`), zero-padded ids starting `00` (`0012345678`, read as an international prefix), `+`-signed numbers and ISO 6709 coordinates (`+1500000000`, `+40.6894-074.0447/`, read as phone numbers), a URL with a user and password (redacted whole), and about 1 in 10 numeric ids of 13 to 19 digits (they pass the card checksum). Argument names these rules would change are no longer listed in `$mcp_input_keys`. If your tools return personal data on purpose, set `redactPayloadPersonalData: false` on `instrument()` or `PostHogMCP`. `beforeSend` can drop a value but cannot restore one. With the option off, the intent and feedback text keep their redaction, and credentials are redacted everywhere. A string longer than four times the captured string limit is redacted on its first part only, the part truncation can keep, so a multi-megabyte tool result costs about what a captured one does. A URL that runs past that part, such as a `data:` image in an HTML page or an address whose user or token sits after it, is now replaced whole by `[redacted]` and nothing after it is captured, with the option on or off. It was read in full before. diff --git a/packages/core/src/privacy.ts b/packages/core/src/privacy.ts index 1eb538e64e..e779e0c905 100644 --- a/packages/core/src/privacy.ts +++ b/packages/core/src/privacy.ts @@ -552,11 +552,14 @@ function redactUrls(value: string): string { return sanitizeUrlsInString(value, { allowNestedUrls: true, stripPunctuation: true }) } +function scanWindowLength(options: TextSanitizationOptions): number { + return 4 * options.maxStringLength +} + /** - * Runs `redact` over only the head of `value` that truncation can keep, so a - * multi-megabyte tool result costs about the same as the part captured. The head - * ends where no URL crosses it, because a URL is redacted or kept on the whole - * address. Redaction shrinks text, so a head that ends up too short to cover + * Runs `redact` over only the window of `value` that truncation can keep, so a + * multi-megabyte tool result costs about the same as the part captured. + * Redaction shrinks text, so a window that ends up too short to cover * truncation's cut falls back to the whole string. Either way the kept part * reads exactly as if the whole string were redacted. */ @@ -565,29 +568,53 @@ function redactCapturedHead( redact: (value: string) => string, options: TextSanitizationOptions ): string { - const scannedHeadLength = 4 * options.maxStringLength - if (value.length > scannedHeadLength) { - const head = redact(value.slice(0, headLength(value, scannedHeadLength))) - // Twice what truncation keeps, so a match split where the head was cut sits - // past the part truncation keeps. - if (head.length >= 2 * options.maxStringLength) { - return head + options.truncationSuffix - } - } - return redact(value) + const windowLength = scanWindowLength(options) + return value.length > windowLength + ? keepHead(value, redact(value.slice(0, windowLength)), redact, options) + : redact(value) } /** - * The scanned head, shortened only to where a URL that would cross the cut - * starts. Any URL after the last character that ends one runs to the cut. + * {@link redactCapturedHead} with the URL pass after `redact`. A URL is redacted + * or kept on the whole address, and the part that decides it, such as the `@` + * closing a user, can sit past the window, so a URL that runs past the window is + * replaced whole and nothing after it is read. */ -function headLength(value: string, scannedHeadLength: number): number { - let lastUrlEnd = scannedHeadLength - while (lastUrlEnd > 0 && !URL_END_PATTERN.test(value[lastUrlEnd - 1])) { +function redactCapturedText( + value: string, + redact: (value: string) => string, + options: TextSanitizationOptions +): string { + const windowLength = scanWindowLength(options) + if (value.length <= windowLength) { + return redactUrls(redact(value)) + } + const head = redact(value.slice(0, windowLength)) + const urlStart = URL_END_PATTERN.test(value[windowLength]) ? -1 : trailingUrlStart(head) + return urlStart === -1 + ? keepHead(value, redactUrls(head), (whole) => redactUrls(redact(whole)), options) + : redactUrls(head.slice(0, urlStart)) + REDACTED_VALUE + options.truncationSuffix +} + +function keepHead( + value: string, + head: string, + redact: (value: string) => string, + options: TextSanitizationOptions +): string { + // Twice what truncation keeps, so a match split where the head was cut sits + // past the part truncation keeps. + return head.length >= 2 * options.maxStringLength ? head + options.truncationSuffix : redact(value) +} + +/** Where the URL that runs to the end of `text` starts, or -1. */ +function trailingUrlStart(text: string): number { + let lastUrlEnd = text.length + while (lastUrlEnd > 0 && !URL_END_PATTERN.test(text[lastUrlEnd - 1])) { lastUrlEnd-- } - const crossingUrl = URL_PATTERN_ONCE.exec(value.slice(lastUrlEnd, scannedHeadLength)) - return crossingUrl ? lastUrlEnd + crossingUrl.index : scannedHeadLength + const url = URL_PATTERN_ONCE.exec(text.slice(lastUrlEnd)) + return url ? lastUrlEnd + url.index : -1 } function sanitizeString(value: string, options: TextSanitizationOptions): string { @@ -595,7 +622,7 @@ function sanitizeString(value: string, options: TextSanitizationOptions): string return BINARY_REDACTED_VALUE } const redactPersonalData = options.redactPayloadPersonalData ? redactPayloadPii : (head: string) => head - return redactCapturedHead(value, (head) => redactUrls(redactPersonalData(redactCredentials(head))), options) + return redactCapturedText(value, (head) => redactPersonalData(redactCredentials(head)), options) } /** @@ -614,7 +641,7 @@ export function sanitizeFreeText(value: string, options: TextSanitizationOptions if (isBinaryBlob(value)) { return BINARY_REDACTED_VALUE } - return redactCapturedHead(value, (head) => redactUrls(redactPii(redactCredentials(head))), options) + return redactCapturedText(value, (head) => redactPii(redactCredentials(head)), options) } function passesLuhn(digits: string): boolean { diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index c4a91866b0..004d863bf8 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -470,7 +470,7 @@ describe('URL credential redaction', () => { const html = `

Hi

logo` const authoritySearch = vi.spyOn(String.prototype, 'matchAll') try { - expect(sanitizeCapturedValue(html)).toBe(html) + expect(sanitizeCapturedValue(html)).toBe('

Hi

isIntegerCard(token) || holdsIntentCard(token))], + jsonNumberRule(value, (token) => isIntegerCard(token) || holdsIntentCard(token)), cardRule(INTENT_CARDS), [matching(US_SSN_PATTERN), redactWhole], [matching(PHONE_NANP_PATTERN), redactWhole], @@ -993,19 +999,38 @@ function isPublicIpv4(address: string): boolean { /** * Writes a JSON number for which `isCard` holds as the string `"[redacted]"`, - * whole, so JSON text stays valid JSON. A number stands outside every JSON - * string when an even number of quotes precede it; escaped quotes are masked, so - * they are not counted. The count carries over from one number to the next, so a - * text full of numbers is read once, not once per number. + * whole, so JSON text stays valid JSON. Only when `text` is JSON: in other text + * the quote would end a URL match early and hide the credentials after it, so the + * card is left for the card pass. A number stands outside every JSON string when + * an even number of quotes precede it; escaped quotes are masked, so they are not + * counted. The count carries over from one number to the next, so a text full of + * numbers is read once, not once per number, and `text` is parsed at most once. */ -function jsonNumberRedaction(isCard: (token: string) => boolean): Redaction { +function jsonNumberRule(text: string, isCard: (token: string) => boolean): Rule { let quotes = 0 let counted = 0 - return (token, start, read) => { - for (; counted < start; counted++) { - quotes += read[counted] === '"' ? 1 : 0 - } - return quotes % 2 === 0 && isCard(token) ? `"${REDACTED_VALUE}"` : token + let json: boolean | undefined + return [ + matching(JSON_NUMBER_PATTERN), + (token, start, read) => { + for (; counted < start; counted++) { + quotes += read[counted] === '"' ? 1 : 0 + } + if (quotes % 2 !== 0 || !isCard(token)) { + return token + } + json ??= isJsonText(text) + return json ? `"${REDACTED_VALUE}"` : token + }, + ] +} + +function isJsonText(text: string): boolean { + try { + JSON.parse(text.replace(LEADING_BOM_PATTERN, '')) + return true + } catch { + return false } } @@ -1049,7 +1074,7 @@ export function redactPayloadPii(value: string): string { [matching(EMAIL_PATTERN), redactWhole], [matching(PAYLOAD_IPV4_PATTERN), (address) => (isPublicIpv4(address) ? REDACTED_VALUE : address)], [matching(IPV6_PATTERN), redactWhole], - [matching(JSON_NUMBER_PATTERN), jsonNumberRedaction((token) => token[0] !== '-' && isIntegerCard(token))], + jsonNumberRule(value, (token) => token[0] !== '-' && isIntegerCard(token)), cardRule(PAYLOAD_CARDS), [matching(US_SSN_PATTERN), redactWhole], [matching(PHONE_PARENTHESIZED_PATTERN), redactWhole], @@ -1057,24 +1082,28 @@ export function redactPayloadPii(value: string): string { ]) } -export function sanitizeCapturedValue(value: unknown, options: TextSanitizationOptions): unknown { +export function sanitizeCapturedValue(value: unknown, options: CapturedValueOptions): unknown { return sanitizeValueWith( value, options.redactPayloadPersonalData ? payloadPasses(options) - : { string: (stringValue) => sanitizeString(stringValue, options) } + : { string: (stringValue) => sanitizeString(stringValue, options), key: (key) => cutKey(key, options) } ) } +function cutKey(key: string, options: CapturedValueOptions): string { + return key.length > options.maxKeyLength ? key.slice(0, options.maxKeyLength) + options.truncationSuffix : key +} + function redactCardNumber(value: number): unknown { return isCardNumber(value) ? REDACTED_VALUE : value } /** What the payload personal data pass rewrites in a structure. */ -function payloadPasses(options: TextSanitizationOptions): ValuePasses { +function payloadPasses(options: CapturedValueOptions): ValuePasses { return { string: (value) => sanitizeString(value, options), - key: (key) => redactCapturedHead(key, redactPayloadPii, options), + key: (key) => cutKey(redactCapturedHead(key, redactPayloadPii, options), options), number: redactCardNumber, } } diff --git a/packages/mcp/src/__tests__/mcp-payloads.test.ts b/packages/mcp/src/__tests__/mcp-payloads.test.ts index a38ad744ff..6da0572545 100644 --- a/packages/mcp/src/__tests__/mcp-payloads.test.ts +++ b/packages/mcp/src/__tests__/mcp-payloads.test.ts @@ -6,10 +6,15 @@ import { sanitizeFreeText, sanitizeFreeTextValue, } from '../extensions/mcp-payloads' +import { sanitizeEvent } from '../extensions/sanitization' +import type { McpEvent } from '../types' describe('buildCapturedMcpParameters', () => { + const captured = (request: unknown): unknown => + sanitizeEvent({ parameters: buildCapturedMcpParameters(request) } as McpEvent).parameters + it('captures useful tool-call inputs without transport internals or duplicated intent', () => { - const parameters = buildCapturedMcpParameters({ + const parameters = captured({ id: 102, jsonrpc: '2.0', method: 'tools/call', @@ -103,7 +108,7 @@ describe('buildCapturedMcpParameters', () => { }) it('redacts PostHog tokens from captured string values', () => { - const parameters = buildCapturedMcpParameters({ + const parameters = captured({ method: 'tools/call', params: { name: 'projects-get', @@ -478,6 +483,35 @@ describe('URL credential redaction', () => { }) }) +describe('URLs carrying a card-like number', () => { + const passes = [ + ['intent', (value: string): unknown => sanitizeFreeText(value)], + ['payload', (value: string): unknown => sanitizeCapturedValue(value, { redactPayloadPersonalData: true })], + ['payload with personal data off', (value: string): unknown => sanitizeCapturedValue(value)], + ] as const + const texts = [ + ['in brackets', 'GET https://api.example.com/v1/orders?ids=[4111111111111111]&access_token=s3cr3t-value -> 200'], + ['between commas', 'fetch https://svc.example.com/cb?a=,4111111111111111,&access_token=s3cr3t-value done'], + ['after a colon', 'fetch https://svc.example.com/cb?a=:4111111111111111,&access_token=s3cr3t-value done'], + [ + 'in a log line opening with a bracket', + '[INFO] https://svc.example.com/cb?ids=[4111111111111111]&token=s3cr3t-value', + ], + ['in the userinfo', 'open https://s3cr3t-value:4111111111111111,x@example.com/a please'], + ] as const + + it.each(passes.flatMap(([pass, sanitize]) => texts.map(([label, text]) => [pass, label, text, sanitize] as const)))( + 'redacts the credentials through the %s pass, card %s', + (_pass, _label, text, sanitize) => { + expect(sanitize(text)).not.toContain('s3cr3t-value') + } + ) + + it.each(passes.slice(0, 2))('redacts the card unquoted through the %s pass', (_pass, sanitize) => { + expect(sanitize('ids [4111111111111111] ok')).toBe('ids [[redacted]] ok') + }) +}) + describe('redactPii', () => { const NBSP = String.fromCharCode(0x00a0) const NNBSP = String.fromCharCode(0x202f) @@ -836,6 +870,7 @@ describe('JSON text', () => { ['duplicate keys', '{"email":"ada@example.com","email":null}', '{"email":"[redacted]","email":null}'], ['a 17-digit card integer', '{"pan":62123456789012347}', '{"pan":"[redacted]"}'], ['a 19-digit card integer', '[6212345678901234569, 1]', '["[redacted]", 1]'], + ['a card after a BOM', '\uFEFF{"n":4111111111111111}', '\uFEFF{"n":"[redacted]"}'], [ 'a __proto__ member', '{"__proto__":{"email":"ada@example.com"},"y":2}', diff --git a/packages/mcp/src/__tests__/sink.test.ts b/packages/mcp/src/__tests__/sink.test.ts index 547ef0547a..63ff5225a2 100644 --- a/packages/mcp/src/__tests__/sink.test.ts +++ b/packages/mcp/src/__tests__/sink.test.ts @@ -1,5 +1,6 @@ import type { EventMessage, PostHog } from 'posthog-node' +import { buildCapturedMcpParameters } from '../extensions/mcp-payloads' import { McpEventSink, processMcpEvent } from '../extensions/sink' import { MCPAnalyticsEventType } from '../extensions/event-types' import type { McpEvent } from '../types' @@ -129,3 +130,60 @@ describe('processMcpEvent __proto__ members', () => { expect(response.content[0].text).toBe(expected) }) }) + +describe('processMcpEvent identifiers at the scan window edge', () => { + const window = 131_072 + const filler = (length: number): string => { + const unit = `${'x'.repeat(50)}@ex.com ` + return unit.repeat(Math.ceil(length / unit.length)).slice(0, length - 1) + ' ' + } + const process = (event: Partial, redactPayloadPersonalData = true): ReturnType => + processMcpEvent( + { eventType: MCPAnalyticsEventType.mcpToolsCall, sessionId: 'session-1', timestamp: new Date(), ...event }, + { enableExceptionAutocapture: false, redactPayloadPersonalData } + ) + const around = (text: string, needle: string): string | null => { + const index = text.indexOf(needle) + return index < 0 ? null : text.slice(Math.max(0, index - 20), index + needle.length + 20) + } + + it.each([ + ['ada.lovelace@example.com', 12], + ['078-05-1120', 8], + ['4111 1111 1111 1111', 14], + ['+44 20 7946 0958', 8], + ['203.0.113.77', 8], + ])('redacts %s in an argument that the window cuts after %i characters', async (identifier, kept) => { + const argument = `${filler(window - kept)}${identifier} tail` + const parameters = buildCapturedMcpParameters({ + method: 'tools/call', + params: { name: 'echo', arguments: { text: argument } }, + }) + + const processed = await process({ parameters }) + + expect(around(JSON.stringify(processed!.event.parameters), identifier.slice(0, kept))).toBeNull() + }) + + it.each([ + ['response', true], + ['parameters', true], + ['response', false], + ['parameters', false], + ] as const)( + 'cuts an object key in the %s to the key limit, past an address the window splits, with personal data %s', + async (field, redactPayloadPersonalData) => { + const key = `${'k'.repeat(window - 13)} ada.lovelace@example.com` + const payload = { content: [{ type: 'text', text: 'ok' }], [key]: 1 } + + const processed = await process({ [field]: payload }, redactPayloadPersonalData) + + expect( + around(JSON.stringify(processed!.captures.map((capture) => capture.properties)), 'ada.lovelace') + ).toBeNull() + expect( + Math.max(...Object.keys(processed!.event[field] as object).map((name) => name.length)) + ).toBeLessThanOrEqual(256 + 3) + } + ) +}) diff --git a/packages/mcp/src/extensions/mcp-payloads.ts b/packages/mcp/src/extensions/mcp-payloads.ts index 971bb3d00f..fb111d5f79 100644 --- a/packages/mcp/src/extensions/mcp-payloads.ts +++ b/packages/mcp/src/extensions/mcp-payloads.ts @@ -9,14 +9,15 @@ import { sanitizeCapturedValue as sanitizeCoreCapturedValue, sanitizeFreeText as sanitizeCoreFreeText, sanitizeFreeTextValue as sanitizeCoreFreeTextValue, - type TextSanitizationOptions, + type CapturedValueOptions, } from '@posthog/core' -import { limitEntries, MAX_BREADTH, MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' +import { limitEntries, MAX_BREADTH, MAX_KEY_LENGTH, MAX_STRING_LENGTH, TRUNCATION_SUFFIX } from './truncation' const CONTEXT_ARGUMENT_NAME = 'context' const CAPTURED_REQUEST_FIELDS = ['id', 'jsonrpc', 'method', 'params'] -const SANITIZATION_OPTIONS: TextSanitizationOptions = { +const SANITIZATION_OPTIONS: CapturedValueOptions = { maxStringLength: MAX_STRING_LENGTH, + maxKeyLength: MAX_KEY_LENGTH, truncationSuffix: TRUNCATION_SUFFIX, } @@ -32,7 +33,7 @@ export function sanitizeFreeText(value: string): string { return sanitizeCoreFreeText(value, SANITIZATION_OPTIONS) } -export function sanitizeCapturedValue(value: unknown, options?: Partial): unknown { +export function sanitizeCapturedValue(value: unknown, options?: Partial): unknown { return sanitizeCoreCapturedValue(value, { ...SANITIZATION_OPTIONS, ...options }) } @@ -43,11 +44,12 @@ export function sanitizeFreeTextValue(value: unknown): unknown { /** * The request as captured: its id, JSON-RPC version, method, and params, without * the injected `context` argument, which `$mcp_intent` carries. The entry and - * nesting limits apply before the sanitizer reads anything, so a call with a huge - * argument object costs what a call at those limits costs. + * nesting limits apply here, so a call with a huge argument object costs what a + * call at those limits costs. It is not sanitized here: `sanitizeEvent` does that + * once, with the payload personal data option. */ export function buildCapturedMcpParameters(request: unknown): JsonRecord { - return sanitizeCapturedValue(limitEntries({ request: pickCapturedFields(request) })) as JsonRecord + return limitEntries({ request: pickCapturedFields(request) }) as JsonRecord } function pickCapturedFields(request: unknown): unknown { diff --git a/packages/mcp/src/extensions/truncation.ts b/packages/mcp/src/extensions/truncation.ts index 7dd811f74e..54a496555f 100644 --- a/packages/mcp/src/extensions/truncation.ts +++ b/packages/mcp/src/extensions/truncation.ts @@ -9,6 +9,7 @@ import type { ErrorProperties, Event, StackFrame, McpEvent } from '../types' export const MAX_DEPTH = 10 export const MAX_BREADTH = 100 export const MAX_STRING_LENGTH = 32_768 // 32KB +export const MAX_KEY_LENGTH = 256 export const MAX_EVENT_BYTES = 102_400 // 100KB // --- Field-level limit constants ---