Skip to content

feat(mcp): pass every HTTP request header to MCP callbacks - #320

Open
lucasheriques wants to merge 5 commits into
mainfrom
lucas/mcp-auth-header
Open

lucasheriques wants to merge 5 commits into
mainfrom
lucas/mcp-auth-header

Conversation

@lucasheriques

@lucasheriques lucasheriques commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

MCP identify, intent_fallback and event_properties callbacks read request headers from extra['headers']. But RequestScope.headers_from_env copied only four headers into it: user-agent, x-anthropic-client, mcp-session-id and mcp-protocol-version. So the identify-by-bearer-token pattern from the TypeScript (getRequestHeaders) and Python (get_request_headers) docs always got nil in Ruby, and every caller stayed anonymous.

headers_from_env now returns every request header with lowercase names, matching the other two SDKs:

  • every HTTP_* Rack env key;
  • CONTENT_TYPE and CONTENT_LENGTH, which Rack stores without the prefix;
  • not HTTP_VERSION, which WEBrick and Thin use for the request line's protocol.

Callbacks get a copy of the Hash (one per request, which the callbacks share), because instrumentation reads mcp-session-id and mcp-protocol-version from the request's headers after callbacks run. (An earlier revision froze it instead. PostHog Review pointed out that a FrozenError message includes the whole Hash, credentials included, and the callback rescue paths log that message.)

Privacy

No header value reaches a captured event through the SDK:

  • Identity#captured_extra keeps only scalar values from extra, so $identify drops the headers Hash.
  • TransportIdentity.stamp reads only user-agent and x-anthropic-client.
  • $mcp_parameters is built from the request, never from extra.
  • Key-based sanitization also redacts authorization, cookie and x-api-key if one ever got in.

The limit is the same as in Python. A callback that returns a raw header value in identity properties or event_properties sends it unredacted. The changeset and the docs say so.

Design choice

I considered a smaller fix: add authorization to the old allowlist. I rejected it. An allowlist would fail the same way for x-api-key, tenant headers, and every other header someone resolves identity from. It also never protected privacy, because the four allowlisted headers weren't captured as a Hash either. The capture paths listed above are what keep values out of events.

The sdk-specs MCP Analytics spec doesn't say which headers callbacks receive, so this PR doesn't diverge from it.

💚 How did you test it?

  • spec/posthog/mcp/http_transport_spec.rb has a new parameterized example. It sends authorization and x-tenant-id through a real StreamableHTTPTransport and asserts two things. identify resolves the user from the header. No serialized event, $identify included, contains the value. It failed before the change, with anonymous instead of user-1.
  • Mutation check. When captured_extra was changed to keep Hash values, the x-tenant-id row failed. The authorization row still passed, because key-based redaction caught it. That shows both layers are tested.
  • spec/posthog/mcp/request_scope_spec.rb is a new parameterized unit spec for headers_from_env. It covers prefixed headers, the unprefixed headers, HTTP_VERSION, other CGI keys, empty values, non-String values, a non-Hash env.
  • Another new example has identify delete user-agent and forge mcp-session-id in its copy. $mcp_tool_call keeps both values. It fails without the copy.
  • bundle exec rspec: 1905 examples, 0 failures, 40 pending. The pending examples are existing and gem-gated.
  • rubocop is clean on the touched files.

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed. A stacked posthog.com PR updates the Ruby identify sample. It must merge after this ships.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file. posthog-ruby minor.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Built with Claude Code (pstack poteto-mode), as a follow-up to the MCP Analytics GA docs audit (PostHog/posthog.com#21653). The pre-PR gate was a pstack interrogate review by Claude Opus and Claude Sonnet. Fixed from it: the missing content-type and content-length, the fake version header, callback mutation of shared headers, and the changeset wording. Dismissed:

  • the underscore/dash header collision, which comes from Rack's CGI env;
  • dropping empty values, which keeps the existing behavior where empty and absent look the same;
  • a per-callback test matrix, because all three callbacks get the same memoized extra object.

identify, intent_fallback and event_properties read extra['headers'], but
RequestScope only copied four headers into it (user-agent, x-anthropic-client,
mcp-session-id, mcp-protocol-version). So identify-by-bearer-token, the pattern
the TS (getRequestHeaders) and Python (get_request_headers) docs show, always
saw nil in Ruby. headers_from_env now maps every HTTP_* Rack env key to its
lowercase header name, matching the other SDKs.

No header value is captured. Identity#captured_extra keeps only scalars from
extra (the headers Hash is dropped from $identify), TransportIdentity stamps
user-agent and x-anthropic-client alone, and key-based sanitization redacts
authorization / x-api-key / cookie as a second layer.

Tested: new parameterized spec in http_transport_spec drives a real
StreamableHTTPTransport with `authorization` and `x-tenant-id`. identify
resolves the user from the header, and no captured event's JSON contains the
value. Red before the change ("anonymous"). Mutation check: letting
captured_extra keep Hash values fails the x-tenant-id case (authorization is
still caught by redaction). `bundle exec rspec`: 1896 examples, 0 failures,
40 pending (pre-existing gem-gated). rubocop clean on touched files.
Pre-PR gate fixes (opus + sonnet interrogate, both raised each):
- Rack stores Content-Type/Content-Length as CONTENT_TYPE/CONTENT_LENGTH, so
  they were missing; TS and Python expose them.
- HTTP_VERSION is the request line's protocol on WEBrick/Thin, not a header.
- Freeze the Hash: callbacks share it with session/protocol resolution.
- Changeset and docstring now say the guarantee covers SDK capture paths,
  not raw values a callback returns.

Tested: new parameterized request_scope_spec for headers_from_env. Full
bundle exec rspec green.
@lucasheriques lucasheriques self-assigned this Oct 27, 2026
@lucasheriques
lucasheriques requested a review from a team as a code owner October 27, 2026 22:45
lucasheriques added a commit to PostHog/posthog.com that referenced this pull request Oct 27, 2026
PostHog/posthog-ruby#320 passes every HTTP request header to MCP callbacks in
extra["headers"]. The Ruby identify samples in identifying-users.mdx and
installation/ruby.mdx now resolve the user from the bearer token, like the TS
and Python samples, instead of saying Ruby can't read Authorization and
pointing at a session lookup or the custom dispatcher. installation/ruby.mdx
also carries the "don't return raw header values" warning.

Gated on posthog-ruby 3.29.0. Confirm the version once #320 ships, then merge.
Tested: diff reviewed. No local node_modules, so fix-mdx/vale run in CI.
lucasheriques added a commit to PostHog/posthog.com that referenced this pull request Oct 27, 2026
PostHog/posthog-ruby#320 passes every HTTP request header to MCP callbacks in
extra["headers"]. The Ruby identify samples in identifying-users.mdx and
installation/ruby.mdx now resolve the user from the bearer token, like the TS
and Python samples, instead of saying Ruby can't read Authorization and
pointing at a session lookup or the custom dispatcher. installation/ruby.mdx
also carries the "don't return raw header values" warning.

Gated on posthog-ruby 3.29.0: confirm the version once #320 ships, then merge.
Tested: diff reviewed. No local node_modules, so fix-mdx/vale run in CI.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 1efeaaf: 1 Should 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 thread lib/posthog/mcp/request_scope.rb Outdated
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

posthog-ruby-sync Compliance Report

Date: 2026-10-28T14:59:47.194578+00:00
Duration: 56064ms

⚠️ Some Tests Failed

39/47 tests passed, 8 failed


Capture Tests

⚠️ 23/30 tests passed, 7 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 9ms
Format Validation.Event Has Uuid ✅ 7ms
Format Validation.Event Has Lib Properties ✅ 7ms
Format Validation.Distinct Id Is String ✅ 9ms
Format Validation.Token Is Present ✅ 8ms
Format Validation.Custom Properties Preserved ✅ 7ms
Format Validation.Event Has Timestamp ✅ 7ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 7ms
Retry Behavior.Retries On 503 ✅ 5247ms
Retry Behavior.Does Not Retry On 400 ❌ 7ms
Retry Behavior.Does Not Retry On 401 ❌ 5ms
Retry Behavior.Respects Retry After Header ✅ 8016ms
Retry Behavior.Implements Backoff ❌ 231ms
Retry Behavior.Retries On 500 ✅ 5142ms
Retry Behavior.Retries On 502 ✅ 5113ms
Retry Behavior.Retries On 504 ✅ 5115ms
Retry Behavior.Max Retries Respected ❌ 562ms
Deduplication.Generates Unique Uuids ✅ 21ms
Deduplication.Preserves Uuid On Retry ✅ 5116ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10388ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5170ms
Deduplication.No Duplicate Events In Batch ✅ 18ms
Deduplication.Different Events Have Different Uuids ✅ 9ms
Compression.Sends Gzip When Enabled ✅ 6ms
Batch Format.Uses Proper Batch Structure ✅ 7ms
Batch Format.Flush With No Events Sends Nothing ✅ 4ms
Batch Format.Multiple Events Batched Together ❌ 16ms
Error Handling.Does Not Retry On 403 ❌ 5ms
Error Handling.Does Not Retry On 413 ❌ 6ms
Error Handling.Retries On 408 ✅ 5153ms

Failures

retry_behavior.does_not_retry_on_400

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

retry_behavior.does_not_retry_on_401

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

retry_behavior.implements_backoff

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

retry_behavior.max_retries_respected

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

batch_format.multiple_events_batched_together

Expected 1 requests, got 5

error_handling.does_not_retry_on_403

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

error_handling.does_not_retry_on_413

500, message='Internal Server Error', url='http://sdk-adapter:8080/capture'

Feature_Flags Tests

⚠️ 16/17 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 8ms
Request Payload.Flags Request Uses V2 Query Param ✅ 8ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 8ms
Request Payload.Flags Request Omits Authorization Header ✅ 9ms
Request Payload.Token In Flags Body Matches Init ✅ 7ms
Request Payload.Groups Round Trip ✅ 7ms
Request Payload.Groups Default To Empty Object ✅ 6ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 6ms
Request Payload.Disable Geoip Omitted Defaults To False ❌ 7ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 7ms
Request Lifecycle.No Flags Request On Init Alone ✅ 2ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 6ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 9ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 7ms
Retry Behavior.Retries Flags On 502 ✅ 109ms
Retry Behavior.Retries Flags On 504 ✅ 110ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 8ms

Failures

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

posthog-ruby-async Compliance Report

Date: 2026-10-28T15:00:27.160087+00:00
Duration: 98201ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

✅ 30/30 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 10ms
Format Validation.Event Has Uuid ✅ 106ms
Format Validation.Event Has Lib Properties ✅ 109ms
Format Validation.Distinct Id Is String ✅ 107ms
Format Validation.Token Is Present ✅ 107ms
Format Validation.Custom Properties Preserved ✅ 107ms
Format Validation.Event Has Timestamp ✅ 108ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 110ms
Retry Behavior.Retries On 503 ✅ 5312ms
Retry Behavior.Does Not Retry On 400 ✅ 2109ms
Retry Behavior.Does Not Retry On 401 ✅ 2109ms
Retry Behavior.Respects Retry After Header ✅ 8017ms
Retry Behavior.Implements Backoff ✅ 15514ms
Retry Behavior.Retries On 500 ✅ 5212ms
Retry Behavior.Retries On 502 ✅ 5213ms
Retry Behavior.Retries On 504 ✅ 5212ms
Retry Behavior.Max Retries Respected ✅ 15611ms
Deduplication.Generates Unique Uuids ✅ 114ms
Deduplication.Preserves Uuid On Retry ✅ 5213ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10318ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5215ms
Deduplication.No Duplicate Events In Batch ✅ 112ms
Deduplication.Different Events Have Different Uuids ✅ 107ms
Compression.Sends Gzip When Enabled ✅ 107ms
Batch Format.Uses Proper Batch Structure ✅ 107ms
Batch Format.Flush With No Events Sends Nothing ✅ 5ms
Batch Format.Multiple Events Batched Together ✅ 110ms
Error Handling.Does Not Retry On 403 ✅ 2109ms
Error Handling.Does Not Retry On 413 ✅ 2109ms
Error Handling.Retries On 408 ✅ 5212ms

Feature_Flags Tests

⚠️ 16/17 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 108ms
Request Payload.Flags Request Uses V2 Query Param ✅ 108ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 108ms
Request Payload.Flags Request Omits Authorization Header ✅ 110ms
Request Payload.Token In Flags Body Matches Init ✅ 108ms
Request Payload.Groups Round Trip ✅ 107ms
Request Payload.Groups Default To Empty Object ✅ 107ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 107ms
Request Payload.Disable Geoip Omitted Defaults To False ❌ 107ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 108ms
Request Lifecycle.No Flags Request On Init Alone ✅ 3ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 106ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 112ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 106ms
Retry Behavior.Retries Flags On 502 ✅ 210ms
Retry Behavior.Retries Flags On 504 ✅ 242ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 109ms

Failures

request_payload.disable_geoip_omitted_defaults_to_false

Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['distinct_id', 'groups', 'person_properties', 'group_properties', 'flag_keys_to_evaluate', 'token']

PostHog Review P2: a callback that mutates a frozen Hash raises FrozenError,
whose message inspects the whole Hash (authorization, cookies), and the
callback rescue paths log e.message through Log.debug. Instrumentation#extra
now hands callbacks @headers.dup, so a mutation stays in the callback's copy
and session/protocol resolution still reads the request's own headers.

Tested: new http_transport_spec example where identify deletes user-agent and
forges mcp-session-id; $mcp_tool_call keeps both. Fails without .dup. Full
bundle exec rspec: 1905 examples, 0 failures. rubocop clean.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — escalated to a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

The sensitive-header and callback-contract changes have tests but no independent assurance on the current head; the older bot review supplies no specific concern to verify. Request review from Rafael Audibert, a teammate, or an agent reviewer covering these changes before re-requesting.

  • Risky territory without independent assurance: Auth or security-sensitive surface / crypto and secrets — lib/posthog/mcp/request_scope.rb now exposes authorization, cookies, API keys, and other request headers to application callbacks instead of a four-header allowlist.; Public API contracts — lib/posthog/mcp/request_scope.rb changes the headers available in callback arguments, and lib/posthog/mcp/instrumentation.rb gives those callbacks a copied header hash.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 44L, 2F substantive, 104L/5F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (104L, 5F, cross-cutting, feat)
stamphog 2.6.1 .stamphog/policy.yml @ 226d454 · reviewed head 226d454

@stamphog stamphog Bot removed the stamphog label Oct 28, 2026
@lucasheriques

Copy link
Copy Markdown
Contributor Author

@PostHog review --deep

@posthog

posthog Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

@lucasheriques PostHog Review is reviewing this pull request.

@posthog

posthog Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

PostHog Review · Deep · reviewed f7ee18d

Posted 1 finding: 1 Consider.

Step Status Result
Prepare the diff done 5 files, 1 chunk
Pick perspectives done 3 perspectives
Run review passes done 2 issues
Merge overlapping findings done 2 → 1
Validate findings done 1 kept, 0 dismissed
Publish done 1 posted (view review)

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved yet — waiting on the conditions below.

PostHog Review is still reviewing this commit, so stamphog does not approve yet. The review runs again on the next push, or you can re-request it once that review finishes.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 44L, 2F substantive, 104L/5F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (104L, 5F, cross-cutting, feat)
stamphog 2.6.1 .stamphog/policy.yml @ f7ee18d · reviewed head f7ee18d

Comment thread lib/posthog/mcp/instrumentation.rb Outdated

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 has an unresolved correctness concern, and there is no qualifying independent assurance for the security-sensitive change. Duplicate header String values, add the requested in-place mutation regression test, and ask @PostHog or Rafael Audibert to review the updated head before re-requesting.

  • PostHog Review (Deep) reviewed the current head: 1 finding, posted on the PR.
  • posthog[bot] reviewed the current head.
  • @PostHog on lib/posthog/mcp/instrumentation.rb: the callback header Hash is only shallow-copied. RequestScope retains the Rack environment's String values, so in-place callback edits still alter original credentials and headers used for subsequent session resolution. The current test covers deletion and replacement, not mutation of shared Strings.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 44L, 2F substantive, 104L/5F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (104L, 5F, cross-cutting, feat)
stamphog 2.6.1 .stamphog/policy.yml @ f7ee18d · reviewed head f7ee18d

@stamphog stamphog Bot removed the stamphog label Oct 28, 2026
@headers.dup shared the Rack env's String values, so an in-place edit in a
callback (delete_prefix!, replace) changed env['HTTP_AUTHORIZATION'] and the
mcp-session-id the SDK resolves the session from. Copy each value too.

Tested: the parameterized http_transport spec now edits values in place and
asserts the env and $session_id are unchanged; it failed before the fix
(env became forged/''/secret). bundle exec rspec: 1906 examples, 0 failures,
40 pending. rubocop clean.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — escalated to a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

The security-sensitive callback change has tests but no current-head independent assurance; the supplied bot reviews cover older commits and give no concrete concern to verify. Request review from Rafael Audibert, another teammate, or an agent reviewer on the current head before re-requesting.

  • Risky territory without independent assurance: Auth or security-sensitive surface / crypto and secrets — lib/posthog/mcp/request_scope.rb now makes credential-bearing headers, including authorization tokens and API keys, available to analytics callbacks.; Public API contracts — lib/posthog/mcp/request_scope.rb and lib/posthog/mcp/instrumentation.rb expand the headers supplied to application callbacks and give those callbacks a separately copied header map.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 45L, 2F substantive, 121L/5F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (121L, 5F, cross-cutting, feat)
stamphog 2.6.1 .stamphog/policy.yml @ 7808184 · reviewed head 7808184

@stamphog stamphog Bot removed the stamphog label Oct 28, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant