diff --git a/lib/mcp/client/http.rb b/lib/mcp/client/http.rb index 0a45b8e6..f224508c 100644 --- a/lib/mcp/client/http.rb +++ b/lib/mcp/client/http.rb @@ -418,8 +418,10 @@ def send_request(request:) yield if block_given? response = begin - client.post("", request, session_headers.merge(request_metadata_headers(method, params))) do |req| - req.options.on_data = stream.on_data + redacting_authorization_header do + client.post("", request, session_headers.merge(request_metadata_headers(method, params))) do |req| + req.options.on_data = stream.on_data + end end rescue StreamAbort nil @@ -527,7 +529,7 @@ def send_request(request:) def send_notification(notification:) method = notification[:method] || notification["method"] - client.post("", notification, session_headers) + redacting_authorization_header { client.post("", notification, session_headers) } nil rescue Faraday::Error => e raise RequestHandlerError.new( @@ -555,7 +557,7 @@ def close end begin - client.delete("", nil, session_headers) + redacting_authorization_header { client.delete("", nil, session_headers) } rescue Faraday::ClientError => e raise unless [404, 405].include?(e.response&.dig(:status)) ensure @@ -1050,6 +1052,51 @@ def remaining_reconnection_budget(deadline) [deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC), 0.001].max end + # Runs a request on the transport's connection and, when Faraday raises, replaces the value of the `Authorization` header + # in the request headers the exception retains before it leaves the transport. + # Faraday's `raise_error` middleware keeps a reference to `env.request_headers` in `Faraday::Error#response` + # under `:request`, so the exception that becomes `RequestHandlerError#original_error` (and its `cause`) would otherwise print + # the bearer token through `inspect`, which is what error reporters and log lines do with it. The request is complete by + # the time the exception surfaces, and every request builds its own headers from the connection's defaults, + # so the rewrite reaches neither those defaults nor a later request. + def redacting_authorization_header + yield + rescue Faraday::Error => e + redact_authorization_header!(e) + raise + end + + # `raise_error` retains the request as a Hash under `:request`; a JSON middleware the customizer added raises + # `Faraday::ParsingError` with the `Faraday::Response` itself, whose `env` holds the request headers. + def redact_authorization_header!(error) + response = error.response + + if response.is_a?(Hash) + request = response[:request] + redact_authorization_header_in!(request, :headers) if request.is_a?(Hash) + elsif response.respond_to?(:env) && response.env.respond_to?(:request_headers) + redact_authorization_header_in!(response.env, :request_headers) + end + end + + # The headers are a `Faraday::Utils::Headers`, which spells the name `Authorization` whatever the caller wrote, + # unless a customizer middleware replaced them with a plain Hash or froze them: the name is matched regardless + # of case so such a Hash is covered too, and a frozen object is swapped for a copy, so the redaction never + # raises in place of the error it is redacting. + def redact_authorization_header_in!(holder, key) + headers = holder[key] + return unless headers.is_a?(Hash) + + names = headers.each_key.select { |name| name.to_s.casecmp?("authorization") } + return if names.empty? + + headers = holder[key] = headers.dup if headers.frozen? + + names.each do |name| + headers[name] = "[redacted]" + end + end + def require_faraday! require "faraday" rescue LoadError @@ -1162,7 +1209,7 @@ def dispatch_server_request(message) end def send_client_response(response) - client.post("", response, session_headers) + redacting_authorization_header { client.post("", response, session_headers) } end def parse_json_buffer(buffer, method, params) @@ -1225,12 +1272,14 @@ def await_response_after_disconnect(stream, method, params) read_timeout = remaining_reconnection_budget(deadline) reconnect_response = begin - client.get("") do |req| - req.headers.update(session_headers) - req.headers["Accept"] = SSE_ACCEPT_HEADER - req.headers[LAST_EVENT_ID_HEADER] = stream.last_event_id if stream.last_event_id - req.options.read_timeout = read_timeout - req.options.on_data = stream.on_data + redacting_authorization_header do + client.get("") do |req| + req.headers.update(session_headers) + req.headers["Accept"] = SSE_ACCEPT_HEADER + req.headers[LAST_EVENT_ID_HEADER] = stream.last_event_id if stream.last_event_id + req.options.read_timeout = read_timeout + req.options.on_data = stream.on_data + end end rescue StreamAbort # The awaited response arrived on the reconnected stream. diff --git a/test/mcp/client/http_test.rb b/test/mcp/client/http_test.rb index 57828e65..682939d4 100644 --- a/test/mcp/client/http_test.rb +++ b/test/mcp/client/http_test.rb @@ -417,6 +417,174 @@ def test_send_request_raises_unauthorized_error assert_equal({ method: "tools/list", params: nil }, error.request) end + def test_send_request_redacts_the_authorization_header_from_the_original_error + # Faraday keeps the request headers on the error it raises, so without redaction the bearer token + # would travel into logs through `original_error` (also the `cause`) and its `inspect`. + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) + stub_request(:post, url).to_return(status: 401) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + assert_same(error.original_error, error.cause) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_send_notification_redacts_the_authorization_header_from_the_original_error + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) + stub_request(:post, url).to_return(status: 500) + + error = assert_raises(RequestHandlerError) do + client.send_notification(notification: { jsonrpc: "2.0", method: "notifications/initialized" }) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_resuming_a_stream_redacts_the_authorization_header_from_the_original_error + # The server closes the stream after a priming event (SEP-1699), so the client resumes it with + # a GET carrying `Last-Event-ID`; that GET runs after the initial POST completes, still inside `send_request`. + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) + request = { jsonrpc: "2.0", id: "test_id", method: "tools/call", params: { name: "test_tool", arguments: {} } } + stub_request(:post, url).with(body: request.to_json).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "id: event-1\nretry: 10\ndata:\n\n", + ) + stub_request(:get, url).with(headers: { "Last-Event-ID" => "event-1" }).to_return(status: 500) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: request) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + assert_same(error.original_error, error.cause) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_answering_a_server_request_redacts_the_authorization_header_from_the_original_error + # The resumed stream carries a request from the server, which the client answers with a POST of its own. + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) + request = { jsonrpc: "2.0", id: "test_id", method: "tools/call", params: { name: "test_tool", arguments: {} } } + stub_request(:post, url).with(body: request.to_json).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "id: event-1\nretry: 10\ndata:\n\n", + ) + stub_request(:get, url).with(headers: { "Last-Event-ID" => "event-1" }).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "event: message\nid: event-2\ndata: {\"jsonrpc\":\"2.0\",\"id\":\"server_id\",\"method\":\"roots/list\"}\n\n", + ) + stub_request(:post, url).with { |answer| answer.body.include?("server_id") }.to_return(status: 500) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: request) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + assert_same(error.original_error, error.cause) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_answering_a_buffered_server_request_redacts_the_authorization_header_from_the_original_error + # An adapter without streaming support, like the test adapter, hands the SSE body over whole once + # the POST has completed, so the answer to a server request it carries is sent outside that POST + # and only the answer's own redaction covers it. + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("/") do |env| + if env.body.include?("server_id") + [500, {}, ""] + else + [ + 200, + { "Content-Type" => "text/event-stream" }, + "event: message\ndata: {\"jsonrpc\":\"2.0\",\"id\":\"server_id\",\"method\":\"roots/list\"}\n\n", + ] + end + end + end + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) do |faraday| + faraday.adapter(:test, stubs) + end + + error = assert_raises(RequestHandlerError) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + assert_same(error.original_error, error.cause) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_send_request_redacts_the_authorization_header_from_a_parsing_error + # A JSON middleware added through the connection block raises `Faraday::ParsingError` on a malformed body, + # and that error retains the `Faraday::Response` itself rather than the Hash `raise_error` builds. + # The body reaches the middleware through an adapter without streaming support, like the test adapter. + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("/") { [200, { "Content-Type" => "application/json" }, "{"] } + end + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) do |faraday| + faraday.response(:json) + faraday.adapter(:test, stubs) + end + + error = assert_raises(RequestHandlerError) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_instance_of(Faraday::ParsingError, error.original_error) + assert_equal("[redacted]", error.original_error.response.env.request_headers["Authorization"]) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_send_request_redacts_frozen_request_headers_through_a_copy + # A middleware that freezes the request headers must not turn the redaction into a `FrozenError` raised + # in place of the HTTP error, which would also skip the OAuth retry and the `RequestHandlerError` wrapping. + freezing = Class.new(Faraday::Middleware) do + def call(env) + env.request_headers.freeze + @app.call(env) + end + end + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) do |faraday| + faraday.use(freezing) + end + stub_request(:post, url).to_return(status: 401) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["Authorization"]) + refute_includes(error.original_error.inspect, "secret-token") + end + + def test_send_request_redacts_a_lowercase_authorization_key_in_replaced_request_headers + # A middleware that replaces the headers with a plain Hash loses the canonical spelling + # `Faraday::Utils::Headers` guarantees, so the name is matched regardless of case. + downcasing = Class.new(Faraday::Middleware) do + def call(env) + env.request_headers = env.request_headers.to_h.transform_keys(&:downcase) + @app.call(env) + end + end + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) do |faraday| + faraday.use(downcasing) + end + stub_request(:post, url).to_return(status: 401) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: { jsonrpc: "2.0", id: "test_id", method: "tools/list" }) + end + + assert_equal("[redacted]", error.original_error.response[:request][:headers]["authorization"]) + refute_includes(error.original_error.inspect, "secret-token") + end + def test_send_request_raises_forbidden_error request = { jsonrpc: "2.0", @@ -2010,6 +2178,27 @@ def test_close_propagates_unauthorized_and_still_clears_state assert_nil(client.session_id) end + def test_close_redacts_the_authorization_header_from_the_propagated_error + client = HTTP.new(url: url, headers: { "Authorization" => "Bearer secret-token" }) + stub_request(:post, url).to_return( + status: 200, + headers: { "Content-Type" => "application/json", "Mcp-Session-Id" => "session-abc" }, + body: { jsonrpc: "2.0", result: { protocolVersion: "2025-11-25" } }.to_json, + ) + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + stub_request(:delete, url).to_return(status: 401) + + error = assert_raises(Faraday::UnauthorizedError) do + client.close + end + + assert_equal("[redacted]", error.response[:request][:headers]["Authorization"]) + + # Only the `Authorization` header is replaced; the session header stays as it was sent. + assert_equal("session-abc", error.response[:request][:headers]["Mcp-Session-Id"]) + refute_includes(error.inspect, "secret-token") + end + def test_close_propagates_connection_failure_and_still_clears_state initialize_session stub_request(:delete, url).to_raise(Faraday::ConnectionFailed.new("connection refused")) diff --git a/test/mcp/client/oauth/http_oauth_test.rb b/test/mcp/client/oauth/http_oauth_test.rb index 0a3cd384..4f047bc6 100644 --- a/test/mcp/client/oauth/http_oauth_test.rb +++ b/test/mcp/client/oauth/http_oauth_test.rb @@ -308,6 +308,38 @@ def test_send_request_runs_the_oauth_flow_through_the_provider_customizer ) end + def test_send_request_keeps_the_challenge_and_the_retried_bearer_through_the_redaction + # The 401 error is redacted before the OAuth flow reads its `WWW-Authenticate` challenge, and the retry + # builds its own headers, so the flow still runs and the retried request carries the token it produced. + # The challenge names a metadata URL off the well-known paths, which are left unstubbed: were the challenge + # lost with the redaction, discovery would fall back to those paths and the flow would fail. + @prm_url = "https://srv.example.com/oauth/protected-resource" + stub_step_up_authorization_server + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer initial-token" } + ).to_return( + status: 401, + headers: { "WWW-Authenticate" => %(Bearer error="invalid_token", resource_metadata="#{@prm_url}") }, + body: "", + ) + stub_request(:post, @mcp_url).with( + headers: { "Authorization" => "Bearer escalated-token" } + ).to_return( + status: 200, + headers: { "Content-Type" => "application/json" }, + body: JSON.generate(jsonrpc: "2.0", id: "1", result: { ok: true }), + ) + provider = build_step_up_provider + + transport = HTTP.new(url: @mcp_url, oauth: provider) + response = transport.send_request(request: { jsonrpc: "2.0", id: "1", method: "tools/list" }) + + assert_equal({ "ok" => true }, response["result"]) + assert_equal("escalated-token", provider.access_token) + assert_requested(:post, @mcp_url, headers: { "Authorization" => "Bearer initial-token" }, times: 1) + assert_requested(:post, @mcp_url, headers: { "Authorization" => "Bearer escalated-token" }, times: 1) + end + def test_send_request_does_not_follow_a_resource_metadata_challenge_off_the_server_origin # End to end over the transport, which is where the header is actually parsed: # a server that answers 401 must not be able to name an unrelated host in