Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 60 additions & 11 deletions lib/mcp/client/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
189 changes: 189 additions & 0 deletions test/mcp/client/http_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"))
Expand Down
32 changes: 32 additions & 0 deletions test/mcp/client/oauth/http_oauth_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading