Repository navigation
fix(mcp): bound intent and parameter capture work, redact feedback extra keys and numbers - #1217
Open
lucasheriques wants to merge 8 commits into
Open
lucasheriques wants to merge 8 commits into
lucasheriques wants to merge 8 commits into
Conversation
…edback extras Each fails on main: a 1MB separated-digit intent scans for seconds, captured parameters read every argument and recurse past the depth limit (a 1000-deep argument raises RecursionError), a top-level credential-named argument is captured whole, and object feedback extras keep identifiers in keys and numbers. Tested: pytest posthog/test/mcp/test_pipeline.py -k 'intent_scan or captured_parameters' and posthog/test/mcp/test_feedback.py -k 'extras_redact or scalar_extras'. The new cases fail; the guard cases pass.
…tra keys and numbers - sanitize_free_text redacts only a head of 2x the intent limit (4096 chars), cut at a space no pass matches across: not after a digit or a card/SSN/phone separator. A head with no such space is one long word whose entropy verdict depends on all of it, so it is captured as [redacted]. 12MB of "1-1." went from ~41s to under 1ms. - build_captured_mcp_parameters strips the injected arguments, applies the 100-entry and depth-10 limits (normalize, strings whole), then sanitizes. A credential-named key is redacted without reading its value. This also redacts a top-level argument named e.g. "password", which main captured whole, and fixes a RecursionError on deeply nested arguments. - Feedback object extras: keys and numbers get the free-text pass, a number whose text changes becomes "[redacted]", and the extra is limited before sanitizing (a 5000-deep extra raised RecursionError). Top-level number and boolean extras are captured as sent. Tested: pytest posthog/test/mcp (711 passed, 11 skipped); ruff format --check, ruff check, mypy | mypy-baseline filter all clean. The straddle tests fail when the cut ignores digits and separators. Reviewer notes: two existing tests now use 4000-char words instead of 5000 so they stay below the scanned head; a no-space intent over 4096 chars is [redacted] rather than truncated. Card and SSN matching in redact_pii is unchanged. Large string leaves in $mcp_parameters are still scanned in full.
Contributor
posthog-python Compliance ReportDate: 2026-11-13T23:18:30.201647+00:00 ✅ All Tests Passed!271/271 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_V4 Tests✅ 150/150 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
…ck extras - The head cut read the character before an ASCII space, but redact_pii first turns Unicode spaces into ASCII ones, so "+44<NBSP> 7911..." was cut mid-number and its digits leaked once earlier redactions shrank the head. The check now reads the normalized text. - Free text up to 8192 chars (4x the limit) is redacted whole, as on main, so CJK, newline- or tab-separated text and short prose before a long path keep their content. The head cut applies only above that; worst case is bounded. - Feedback extra keys skip the entropy detector, as resource names do: it read names like Get_Organization_Memberships as credentials, and two such keys collapsed into one "[redacted]" entry. Keys still get the token, PII and URL passes, bounded the same way. - An int past Python's 4300-digit str() limit inside an extra is "[redacted]" instead of raising, and _capture_extra_value keeps sanitizing inside its try, so one extra can no longer drop the whole $mcp_feedback event. Tested: pytest posthog/test/mcp (717 passed, 11 skipped); ruff and mypy baseline clean; the adversaries' repro scripts print the required output and their differential fuzzer finds 0 head-vs-whole mismatches on seeds 1, 3, 5, 7.
lucasheriques
marked this pull request as ready for review
November 9, 2026 15:17
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
SDK performance: posthog-pythonBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The credential-disclosure concern from @veria-ai remains unaddressed despite the reported no-findings review. Retain known credential-format checks for feedback keys, add regression coverage, and request review from Georgis Andonis or a security-aware teammate before re-requesting.
- Author wrote 69% of the modified lines and has 9 merged PRs in these paths (familiarity MODERATE).
- PostHog Review (Standard) reviewed the current head: no findings.
- @veria-ai on posthog/mcp/_sanitization.py: the feedback-key sanitizer still applies only the PostHog-specific token pattern, personal-data redaction, and URL sanitization, bypassing the broader credential checks used for string values. Non-PostHog credential-shaped keys can therefore remain in outbound feedback metadata. The discussion from @veria-ai[bot] repeats this still-unfixed concern.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 272L, 3F substantive, 469L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (469L, 7F, two-areas, fix) |
| stamphog 2.7.0 | .stamphog/policy.yml @ 311d11b · reviewed head 311d11b |
dustinbyrne
reviewed
Nov 9, 2026
dustinbyrne
left a comment
dustinbyrne
left a comment
Contributor
There was a problem hiding this comment.
The bounded traversal and feedback key/number redaction fixes are verified. One intentional long-text retention change still needs reconciliation with the current MCP Payload privacy contract; details inline.
AI-assisted review.
…scan; keep known credential formats in feedback keys Long free text (intent, feedback fields, feedback keys) now follows the sdk-specs bounded scan: redact the text cut at limit+1024 and at the limit, keep their common prefix. Text with no ASCII-space boundary (CJK, one long word) keeps its start instead of becoming [redacted]. Feedback keys keep the known-format credential checks (sk-proj-..., PEM) and still skip entropy. Tested: pytest posthog/test/mcp + test_exception_capture (756 passed, 11 skipped); straddle tests fail under either one-scan mutation; ruff, mypy baseline clean.
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed d526a20: 1 Must fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
Comment on lines
+433
to
+437
| kept = os.path.commonprefix( | ||
| [ | ||
| redact(value[: _MAX_USER_INTENT_LENGTH + _SCAN_MARGIN]), | ||
| redact(value[:_MAX_USER_INTENT_LENGTH]), | ||
| ] |
Contributor
There was a problem hiding this comment.
P1 · Redact URL credentials that extend past both scan cuts
With intent "https://admin:" + "p" * 4000 + "@localhost/x", both slices end before @, so neither URL pass recognizes userinfo. _redact_bounded retains their raw common prefix and captures roughly 2 KB of the password in $mcp_intent; the previous full pass removes it. Preserve the privacy contract (AGENTS.md:21) by redacting incomplete URLs at the scan boundary.
Contributor
Author
There was a problem hiding this comment.
Fixed in 6b49022. When the word crossing the longer scan cut starts inside what truncation keeps and opens a credential whose end neither scan reached — an unterminated URL authority (no /, ?, # after the last ://) or a JWT (eyJ) — _redact_bounded now replaces everything from that word on with [redacted] instead of trusting the two prefixes' agreement (_unfinished_credential_start). Those are the two detectors that only match once they see the far end of the span (userinfo's @, the JWT's second .); query/JSON keys are seen before their values and the vendor formats match on a prefix, so they were already safe. CJK, long words and long paths keep their start.
New parameterized rows in test_intent_scan_fails_closed_on_a_credential_unfinished_at_both_cuts: https://admin:" + "p"*4000 + "@localhost/x, a password with an @ before the cuts, a nested ?next=https://admin:... URL, and a 1631-char signed JWT at offset 1550 (last . at 3154). All four failed on d526a20 and are redacted on origin/main; all pass now. pytest posthog/test/mcp: 815 passed.
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The current-head review raises a security concern that the code still exhibits; the discussion bot's all-clear claim does not resolve it. Fix @PostHog's inline finding in posthog/mcp/_sanitization.py, add a regression test for credentials extending beyond both scan cuts, and request fresh review from a teammate or agent; Georgis Andonis or Aleksander Błaszkiewicz are suggested reviewers.
- Author wrote 61% of the modified lines and has 9 merged PRs in these paths (familiarity MODERATE).
- PostHog Review (Standard) reviewed the current head: 1 finding, posted on the PR.
- posthog[bot] reviewed the current head.
- @PostHog, posthog/mcp/_sanitization.py: The URL-credential exposure remains unfixed. _redact_bounded sanitizes only the first 2048 and 3072 characters and retains their common prefix. For an HTTPS URL with username admin and a 4000-character repeated-letter password, both cuts precede the userinfo delimiter; the URL sanitizer leaves these incomplete authorities unchanged, retaining password text in captured intent. The full URL would have its userinfo removed.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 309L, 4F substantive, 500L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (500L, 8F, two-areas, fix) |
| stamphog 2.8.0 | .stamphog/policy.yml @ d526a20 · reviewed head d526a20 |
dustinbyrne
reviewed
Nov 12, 2026
dustinbyrne
left a comment
dustinbyrne
left a comment
Contributor
There was a problem hiding this comment.
The earlier long-text retention issue and known-format feedback-key cases are addressed. One introduced bounded-scan privacy regression remains when both cuts end inside URL userinfo; details inline.
AI-assisted review.
| """``redact`` over ``value``, at a cost bounded by what truncation keeps.""" | ||
| if len(value) <= _MAX_USER_INTENT_LENGTH + _SCAN_MARGIN: | ||
| return redact(value) | ||
| kept = os.path.commonprefix( |
Contributor
There was a problem hiding this comment.
F1 — [P1] Fail closed when both cuts leave URL userinfo incomplete
Severity: IMPORTANT. Disposition: REQUIRED. Relationship: Introduced.
Location: posthog/mcp/_sanitization.py:433, RIGHT.
A valid URL whose userinfo continues past both scan cuts can now leave password text in $mcp_intent, whereas base redacted it:
text = "https://admin:" + "p" * 4000 + "@localhost/x"
client.capture_tool_call("search_docs", intent=text, duration_ms=1)
client.flush()This result is established by source analysis, not an executed reproduction. The @ is at offset 4014, after both cuts at 2048 and 3072. Both prefixes lack the delimiter _redact_userinfo requires. The entropy check excludes URLs containing ://; none of the known-format/PII passes matches this fixture; URL sanitation returns each prefix unchanged. Their common prefix is therefore the first 2048 raw characters. Final truncation retains https://admin:, 2031 password characters, and ....
On merge base 7eabca81234311dfd86e516c4354c5322f73bfc5, the complete URL reaches the same userinfo sanitizer before truncation, so its credentials are removed. This final URL is valid and below the existing 8192-character URL bound. PostHogMCP.capture_tool_call(intent=...) accepts it; instrumented intent reaches the same sanitizer. Default later capture/backend processing does not repair the value.
The package promises URL username/password redaction, and the canonical privacy requirement expressly includes URL userinfo. The implementation follows the prescribed two-cut method, but its safety explanation only covers matches no longer than the margin. Agreement between two unfinished scans does not establish credential safety for longer spans.
The trigger is unusually long narrated credential data. The source-determined impact is disclosure of a password prefix, not a complete reusable credential or account takeover. The same helper handles feedback free text; ordinary structured parameter/response/error sanitation is not newly affected here.
Consider failing closed on credential-bearing authorities left unfinished at a scan boundary while preserving bounded processing. Keep the repaired CJK/prose prefix behavior and existing short-straddle tests; add a case whose userinfo terminator lies beyond both cuts. Qualify the helper’s safety explanation and add equivalent specification coverage rather than restoring unbounded scanning or blanket replacement of space-free text.
Test sketch — not executed:
from unittest.mock import patch
from posthog.mcp import PostHogMCP
def captured_intent(text):
client = PostHogMCP(
"phc_test", send=False, enable_exception_autocapture=False
)
try:
with patch.object(client, "capture") as captured:
client.capture_tool_call("search_docs", intent=text, duration_ms=1)
client.flush()
assert captured.call_count == 1
return captured.call_args.kwargs["properties"]["$mcp_intent"]
finally:
client.shutdown()
def test_url_userinfo_spanning_both_cuts_is_not_captured():
value = captured_intent("https://admin:" + "p" * 4000 + "@localhost/x")
assert "p" * 32 not in value
assert len(value) <= 2048
def test_short_url_userinfo_is_redacted():
value = captured_intent("https://admin:review-only-password@localhost/x")
assert "review-only-password" not in value
def test_long_cjk_retains_prefix():
text = "请帮我查询" * 1800
value = captured_intent(text)
assert value.startswith(text[:1000])
assert len(value) <= 2048Run equivalent inputs on base 7eabca81234311dfd86e516c4354c5322f73bfc5 and head d526a20de727f45a596e808cb0672ee124021168:
pytest -q test_followup_regressions.py \
-k 'url_userinfo or long_cjk or ordinary_short_intent' --timeout=30
Expected: all assertions pass. Code-inferred: base passes; head fails the long-URL assertion, while short-URL/CJK controls pass. No actual local base/head result is claimed. Mandatory memory-limit setup failed before any SDK import (RLIMIT_AS, with DATA/RSS alternatives also rejected); a suitable container runtime was unavailable. A memory-enforced, offline, credential-free sandbox is needed to execute the sketch. Existing CI passes short URL-boundary and CJK cases but does not cover this both-cuts-inside-one-authority case. No fix was applied or retested.
Contributor
Author
There was a problem hiding this comment.
Fixed in 6b49022. When the word crossing the longer scan cut starts inside what truncation keeps and opens a credential whose end neither scan reached — an unterminated URL authority (no /, ?, # after the last ://) or a JWT (eyJ) — _redact_bounded now replaces everything from that word on with [redacted] instead of trusting the two prefixes' agreement (_unfinished_credential_start). Those are the two detectors that only match once they see the far end of the span (userinfo's @, the JWT's second .); query/JSON keys are seen before their values and the vendor formats match on a prefix, so they were already safe. CJK, long words and long paths keep their start.
New parameterized rows in test_intent_scan_fails_closed_on_a_credential_unfinished_at_both_cuts: https://admin:" + "p"*4000 + "@localhost/x, a password with an @ before the cuts, a nested ?next=https://admin:... URL, and a 1631-char signed JWT at offset 1550 (last . at 3154). All four failed on d526a20 and are redacted on origin/main; all pass now. pytest posthog/test/mcp: 815 passed.
6 tasks done
Conflicts resolved: - _truncation.py: keep main's _visit_container extraction, threading this branch's _Walk (redacts_key + memo) through it. - _sanitization.py: keep main's _redact_json_members pass ahead of the token passes, with this branch's is_secret parameter; keep main's extracted _sanitize_url_fragment alongside this branch's limit_entries rewrite of the captured-parameters builders. - test_pipeline.py: union of imports. Tested: uv run pytest posthog/test/mcp -> 810 passed, 11 skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h scan cuts The bounded intent scan keeps the common prefix of two redactions, which is only safe for matches shorter than the margin. URL userinfo (needs its `@`) and a JWT (needs its second `.`) can run past both cuts, so neither scan recognized them and their raw start landed in $mcp_intent, where main redacted it. When the word crossing the longer cut starts inside what truncation keeps and opens an unterminated URL authority or a JWT, everything from that word on is now replaced by [redacted]. Long CJK, long words and long paths keep their start as before. Tested (TDD): four parameterized rows (url password past both cuts, an `@` inside the password, a nested url, veria's 1631-char JWT at offset 1550) failed on the previous head and are confirmed redacted by origin/main; plus a long-url-path control. uv run pytest posthog/test/mcp -> 815 passed, 11 skipped; ruff check/format clean; mypy --config-file mypy.ini posthog/mcp | mypy-baseline filter -> clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment on lines
+438
to
+440
| kept = os.path.commonprefix( | ||
| [redact(value[:scan_end]), redact(value[:_MAX_USER_INTENT_LENGTH])] | ||
| ) |
Contributor
There was a problem hiding this comment.
P2 · Fail closed on credential-shaped tokens spanning both cuts
For an intent containing "Ab1_" * 600 + "Xy2Z_9mK4jV5qR7tS1wE6uI0oP3lC8dF-" * 160, the previous full pass returns [redacted], but _redact_bounded captures 2,045 raw characters. Both slices have insufficient entropy to trigger _is_secret, although the complete token does. The common-prefix check therefore exposes a previously protected credential; handle unfinished credential-shaped tokens conservatively.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Three fixes to what
posthog.mcpcaptures, ported from PostHog/posthog-js#5607. Each one is shown below as before (main) and after (this PR). The outputs come from running one scratch script against the merge base (2c35f04) and this branch. Timings are single runs on a laptop.On
main:$mcp_intentis redacted in full before truncation keeps 2048 characters, so a long intent stalls capture for seconds.RecursionError, and a top-level argument namedpasswordis captured whole.$mcp_feedback_<name>object extras send emails and card numbers that sit in keys or in number values.This PR adds no personal-data redaction to
$mcp_parameters,$mcp_responseor error text. It does not change which identifiersredact_piimatches.What changes, by example
1. Free text is redacted only as far as truncation can keep it
sanitize_free_text(used for$mcp_intent, thesend_feedbacktext fields and keys, and unknown tool names) follows the sdk-specs bounded scan. Text up to 3072 characters (the 2048 limit plus a 1024 margin) is redacted whole. Longer text is redacted twice, once cut at 3072 and once cut at 2048. Capture keeps the common prefix of the two results and appends.... A match split by the 2048 cut differs between the two results. A match split by the 3072 cut lies past the end of the second result. So neither reaches the capture, and the cost no longer grows with the input.|
$mcp_intent|main| This PR ||
"1-1." * 2048(8 KB) | not measured separately | 14 ms,1-1.1-1.1-1.…(2048 chars) ||
"1-1." * 250_000(1 MB) | 3.6 s,1-1.1-1.1-1.…(2048 chars) | 14 ms, same output ||
"1-1." * 3_000_000(12 MB) | 41.2 s,1-1.1-1.1-1.…(2048 chars) | 14 ms, same output ||
"1-1." * 3_000_000(12 MB) | 41.2 s,1-1.1-1.1-1.…(2048 chars) | under 0.1 ms,[redacted]||
"lorem ipsum " * 87_382(1 MB) | 218 ms,lorem ipsum lorem…(2048 chars) | 0.7 ms, same output ||
"s" * 5000,"s" * 9000or"请帮我查询" * 1800| first 2048 chars | same ||
"s" * 5000or"请帮我查询" * 900| first 2048 chars | same ||
"s" * 9000(one 9000-char word) |sss…(2048 chars) |[redacted]||
Refund for jane.doe@acme.com, card 4111 1111 1111 1111|Refund for [redacted], card [redacted]| same |Tests pin the bounded scan. A card, an SSN, a phone number (one with an NBSP before a space too) or a URL with userinfo straddles the 2048 cut or the 3072 cut, after a start that shrinks by 1500 characters when redacted. None of their digits or credentials reach the capture. Dropping either of the two redactions fails three of those cases.
2. Tool arguments are limited before they are sanitized
build_captured_mcp_parametersnow strips the injected arguments (context,conversation_id, and the SDK-ownedllm_model). Then it applies the 100-entry and depth-10 limits throughnormalize, with strings kept whole. Only after that does it sanitize. A credential-named key is written as[redacted]without its value being read. Read counts come from adictsubclass that counts entries read throughitems().main"...": "[MaxProperties ~]"RecursionError{"deep": {"child": … "[Object]"}}, cut at depth 10 of$mcp_parameters{"password": "hunter2", "q": "x", "context": "…"}{"password": "hunter2", "q": "x"}{"password": "[redacted]", "q": "x"}{"context": …, "conversation_id": …, a0 … a99}On
mainthe credential-key rule only ran inside nested objects, so a top-level argument namedpassword,tokenorapi_keyreached$mcp_parameterswhole.3. Feedback object extras redact identifiers in keys and numbers
A host can declare extra report fields for the
send_feedbacktool. An object extra is sent as JSON text, andmainsanitized only its string values. Now each string and number inside it gets the same free-text pass as$mcp_intent. A number whose text that pass changes becomes the string"[redacted]", so the JSON stays valid. Keys get the token pass, the known-format credential check (sk-proj-…, PEM and the other vendor formats), and the PII and URL passes. They skip only the entropy detector. Otherwise names likeGet_Organization_Membershipsread as credentials, and two such keys collapse into one[redacted]entry. The extra is also limited to 100 entries and depth 10 before it is sanitized.Declared extras
meta(object) andscore(number), sent as:{ "meta": {"ada@example.com": "x", "card\n4111111111111111": 1, "n": 4111111111111111}, "score": 4111111111111111 }main$mcp_feedback_meta{"ada@example.com": "x", "card\n4111111111111111": 1, "n": 4111111111111111}{"[redacted]": "x", "card\n[redacted]": 1, "n": "[redacted]"}$mcp_feedback_score41111111111111114111111111111111(top-level numbers are sent as is, per spec)metanested 5000 deepRecursionError, whole$mcp_feedbackevent dropped{"child": … "[Object]"}, cut at depth 10meta={"Get_Organization_Memberships": 1, "ISO_8601_timestamp_UTC": 5}meta={"n": 10**5000}json.dumpsfails){"n": "[redacted]"}Keys are sanitized before the object is stringified. Sanitizing the JSON text instead would miss the first key: in
card\n4111…the escape'snis a word character, so the card has no word boundary.What does not change
Identifier matching in
redact_piistays as it is, including card and SSN overlap. Both runs printed the same output:redact_piiinputmainand this PR4111 1111 1111 1111 12/30[redacted] 12/30SSN 078-05-1120SSN [redacted]4111-1111-1111-1111 and 078-05-1120[redacted] and [redacted]id 4111111111111112(fails Luhn)id 4111111111111112ip 10.0.0.1, +44 20 7946 0958ip [redacted], [redacted]$mcp_parametersstill gets no personal-data redaction.{"email": "ada@example.com", "note": "call 415-555-0142"}is captured as is on both.main. Longer intents keep their start, as onmain.$mcp_parametersand responses are still scanned in full, andsanitize_eventsanitizesparametersa second time, as onmain. A 12 MB string argument costs about 3 s across the two passes. This PR bounds entries and depth only. Bounding captured strings the way posthog-jsredactCapturedHeaddoes is a follow-up.Tradeoffs
$mcp_parametersdoes, wheremaindropped the extra."[redacted]". Two keys that both redact to[redacted]collapse into the last one.Where to look
posthog/mcp/_sanitization.py_redact_boundedand_SCAN_MARGIN(the bounded scan), and_sanitize_free_text_keywith_is_known_secret(keys)posthog/mcp/_sanitization.pybuild_captured_mcp_parameters,_pick_captured_fields,_kept_arguments,limit_entries: pick fields, strip, limit, then sanitizeposthog/mcp/_sanitization.py_sanitize_value_withwith its leaf and key functions;_sanitize_free_text_leaf(the number rule),_sanitize_free_text_keyposthog/mcp/_truncation.pynormalize's optionalredacts_key, and_Walkcarrying itposthog/mcp/feedback.py_capture_extra_value: scalars as is, objects limited and then walkedposthog/test/mcp/test_pipeline.py,test_feedback.py💚 How did you test it?
Each fix has a parameterized test that failed on
mainfirst. The test commit lands before the fix.pytest posthog/test/mcp posthog/test/test_exception_capture.py: 756 passed, 11 skipped, after mergingmain.ruff format --check .,ruff check ., andmypy --no-site-packages --config-file mypy.ini . | mypy-baseline filterare all clean, using the tool versions pinned inuv.lock._redact_boundedis dropped (3 cases each).📝 Checklist
If releasing new changes
sampo addto generate a changeset file (.sampo/changesets/mcp-capture-bounds.md)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus 5.5) wrote this from a brief that came out of the posthog-js#5607 review. Those review rounds rejected every change to card and SSN overlap handling, so
redact_piimatching is left alone here.The first version cut at an ASCII space that no pattern matches across. Review pointed to the sdk-specs bounded scan (cut at limit + 1024 and at the limit, keep the common prefix), and this PR now follows it. The space rule turned long space-free text into
[redacted], and it depended on a hand-kept list of characters a number continues across, which the NBSP leak showed is easy to get wrong. Four Sonnet reviewers made a simplify pass. The interrogate gate ran an Opus adversary and a Sonnet adversary, each in its own checkout. Both reproduced an NBSP phone-number leak at the cut and the loss of long space-free text. The Sonnet adversary also found key collisions from the entropy detector and a 4300-digit int that dropped the whole feedback event. All four are fixed in the last fix commit, each with a test, and the adversaries' scripts now print the required output. Their differential fuzzer (head redaction against whole-string redaction) finds 0 mismatches on four seeds. Both also flagged that large string arguments are still scanned in full; that is out of scope here and listed above.