diff --git a/.changeset/mcp-request-headers.md b/.changeset/mcp-request-headers.md new file mode 100644 index 0000000..e3acf42 --- /dev/null +++ b/.changeset/mcp-request-headers.md @@ -0,0 +1,5 @@ +--- +"posthog-ruby": minor +--- + +MCP: `identify`, `intent_fallback` and `event_properties` callbacks now receive every HTTP request header in `extra['headers']` (lowercase names, in a copy, so a callback that changes it cannot change how the SDK resolves the session), so a callback can resolve a user from an `Authorization` bearer token or an API key. The SDK doesn't capture this header bag automatically, and the existing `User-Agent` and `X-Anthropic-Client` telemetry (`$mcp_client_user_agent`, `$mcp_vendor_client`) is unchanged. Resolve a header to an id instead of returning its raw value in identity properties or `event_properties`, which are sent unredacted. diff --git a/lib/posthog/mcp/instrumentation.rb b/lib/posthog/mcp/instrumentation.rb index 09ed4e3..c339e2e 100644 --- a/lib/posthog/mcp/instrumentation.rb +++ b/lib/posthog/mcp/instrumentation.rb @@ -588,7 +588,9 @@ def extra 'session_id' => (@session.respond_to?(:session_id) ? @session.session_id : nil) || header_session_id, 'request_id' => @request_id, 'protocol_version' => protocol_version, - 'headers' => @headers, + # A deep copy, because session and protocol resolution read @headers after + # callbacks run, and the values are the Rack env's own Strings. + 'headers' => @headers.transform_values(&:dup), 'session' => @session } end diff --git a/lib/posthog/mcp/request_scope.rb b/lib/posthog/mcp/request_scope.rb index b39281a..aedf914 100644 --- a/lib/posthog/mcp/request_scope.rb +++ b/lib/posthog/mcp/request_scope.rb @@ -17,14 +17,6 @@ module RequestScope KEY = :posthog_mcp_request_scope FIBER_STORAGE = Fiber.respond_to?(:[]) && Fiber.respond_to?(:[]=) - # Rack `env` keys for the headers the integration reads, by header name. - HEADER_ENV_KEYS = { - 'user-agent' => 'HTTP_USER_AGENT', - 'x-anthropic-client' => 'HTTP_X_ANTHROPIC_CLIENT', - 'mcp-session-id' => 'HTTP_MCP_SESSION_ID', - 'mcp-protocol-version' => 'HTTP_MCP_PROTOCOL_VERSION' - }.freeze - module_function # @return [Hash, nil] `{headers:, transport:, mint:, session_id:}` for the in-flight HTTP request, @@ -46,20 +38,39 @@ def current=(value) end end - # @param headers [Hash{String => String}] lowercase header names - # @param transport [Symbol] `:http` for the Streamable HTTP transport, `:other` - # for a transport that publishes no headers (stdio, a custom dispatcher) + # Rack `env` keys for the two request headers Rack stores without `HTTP_`. + UNPREFIXED_HEADER_ENV_KEYS = { 'CONTENT_TYPE' => 'content-type', 'CONTENT_LENGTH' => 'content-length' }.freeze + # The request line's protocol, which some servers (WEBrick, Thin) put in `env`. + PROTOCOL_ENV_KEY = 'HTTP_VERSION' + + # Every request header, so `identify`, `intent_fallback` and `event_properties` + # can read the ones they need (an `authorization` bearer token, an API key) + # from `extra['headers']`. The SDK doesn't capture this bag: {Identity} keeps + # only scalars from `extra`. {TransportIdentity} still stamps `user-agent` and + # `x-anthropic-client`, as before. A callback that returns a raw header value in + # identity properties or `event_properties` sends it, unredacted. + # # @param env [Hash] a Rack `env` - # @return [Hash{String => String}] the headers {Instrumentation} reads, lowercase + # @return [Hash{String => String}] lowercase header names def headers_from_env(env) return {} unless env.is_a?(Hash) - HEADER_ENV_KEYS.each_with_object({}) do |(name, env_key), acc| - value = env[env_key] - acc[name] = value if value.is_a?(String) && !value.empty? + env.each_with_object({}) do |(key, value), acc| + name = header_name(key) + acc[name] = value if name && value.is_a?(String) && !value.empty? end end + def header_name(env_key) + return UNPREFIXED_HEADER_ENV_KEYS[env_key] unless env_key.is_a?(String) && env_key.start_with?('HTTP_') + return nil if env_key == PROTOCOL_ENV_KEY + + env_key.delete_prefix('HTTP_').downcase.tr('_', '-') + end + + # @param headers [Hash{String => String}] lowercase header names + # @param transport [Symbol] `:http` for the Streamable HTTP transport, `:other` + # for a transport that publishes no headers (stdio, a custom dispatcher) def with(headers:, transport: :http) previous = current self.current = { headers: headers, transport: transport, mint: nil, session_id: nil } diff --git a/spec/posthog/mcp/http_transport_spec.rb b/spec/posthog/mcp/http_transport_spec.rb index aa8878f..2aa6d71 100644 --- a/spec/posthog/mcp/http_transport_spec.rb +++ b/spec/posthog/mcp/http_transport_spec.rb @@ -95,6 +95,54 @@ def parse(response) expect(logs.count { |m| m.include?('fragment across requests') }).to eq(1) end + { + 'authorization' => 'Bearer secret-token-should-not-leak', + 'x-tenant-id' => 'tenant-should-not-leak' + }.each do |header, secret| + it "hands identify the #{header} header without capturing it on any event" do + identify = ->(_request, extra) { { distinct_id: extra['headers'][header] == secret ? 'user-1' : 'anonymous' } } + PostHog::MCP.instrument(server, client, identify: identify, enable_conversation_id: false) + token = transport.call(env_for(initialize_body, header => secret))[1]['mcp-session-id'] + transport.call(env_for(rpc(2, 'tools/call', { name: 'ping_tool', arguments: {} }), 'mcp-session-id' => token, header => secret)) + + events = drain_events(client) + expect(events_named(events, '$mcp_tool_call').map { |e| e[:distinct_id] }).to eq(['user-1']) + expect(events_named(events, '$identify').size).to eq(1) + expect(events.map { |e| JSON.generate(e) }.grep(/#{secret}/)).to eq([]) + end + end + + { + 'replaces entries' => lambda { |headers| + headers.delete('user-agent') + headers['authorization'] = 'forged' + headers['mcp-session-id'] = 'forged' + }, + 'edits values in place' => lambda { |headers| + headers['user-agent'].clear + headers['authorization'].delete_prefix!('Bearer ') + headers['mcp-session-id'].replace('forged') + } + }.each do |change, mutate| + it "keeps the request's headers when a callback #{change} in its copy" do + identify = lambda do |_request, extra| + mutate.call(extra['headers']) + { distinct_id: 'user-1' } + end + PostHog::MCP.instrument(server, client, identify: identify, enable_conversation_id: false) + token = transport.call(env_for(initialize_body, 'user-agent' => +'claude-code/2.1.0'))[1]['mcp-session-id'] + env = env_for(rpc(2, 'tools/call', { name: 'ping_tool', arguments: {} }), + 'mcp-session-id' => token.dup, 'user-agent' => +'claude-code/2.1.0', 'authorization' => +'Bearer secret') + transport.call(env) + + expect(env.values_at('HTTP_MCP_SESSION_ID', 'HTTP_USER_AGENT', 'HTTP_AUTHORIZATION')) + .to eq([token, 'claude-code/2.1.0', 'Bearer secret']) + call = events_named(drain_events(client), '$mcp_tool_call').first + expect(call[:properties]).to include('$session_id' => PostHog::MCP.decode_session_id(token).session_id, + '$mcp_client_user_agent' => 'claude-code/2.1.0') + end + end + it 'leaves uninstrumented servers alone' do response = transport.call(env_for(initialize_body)) expect(response[0]).to eq(200) diff --git a/spec/posthog/mcp/request_scope_spec.rb b/spec/posthog/mcp/request_scope_spec.rb new file mode 100644 index 0000000..4f5cdb9 --- /dev/null +++ b/spec/posthog/mcp/request_scope_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' + +RSpec.describe PostHog::MCP::RequestScope do + describe '.headers_from_env' do + { + 'a prefixed header' => [{ 'HTTP_AUTHORIZATION' => 'Bearer t' }, { 'authorization' => 'Bearer t' }], + 'a multi-word header' => [{ 'HTTP_X_TENANT_ID' => 'acme' }, { 'x-tenant-id' => 'acme' }], + 'the headers Rack stores unprefixed' => [{ 'CONTENT_TYPE' => 'application/json', 'CONTENT_LENGTH' => '42' }, + { 'content-type' => 'application/json', 'content-length' => '42' }], + 'the protocol some servers store as HTTP_VERSION' => [{ 'HTTP_VERSION' => 'HTTP/1.1' }, {}], + 'other CGI variables' => [{ 'REQUEST_METHOD' => 'POST', 'PATH_INFO' => '/', 'rack.input' => 'x' }, {}], + 'an empty value' => [{ 'HTTP_X_API_KEY' => '' }, {}], + 'a non-String value' => [{ 'HTTP_X_COUNT' => 1 }, {}], + 'a non-Hash env' => [nil, {}] + }.each do |label, (env, expected)| + it "maps #{label}" do + expect(described_class.headers_from_env(env)).to eq(expected) + end + end + end +end