From 7f31a1c49e7f8090517c92cd0a47854403c6b5cc Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:24:46 +0200 Subject: [PATCH] security: put a ceiling on the response body Nothing bounded how much the client would read. Net::HTTP buffers a whole response before handing it over, so a hostile endpoint, a proxy serving a runaway error page, or a compromised upstream could hand back gigabytes and the first code with an opinion about it was JSON.parse, by which time the bytes were already resident. Error bodies were worse: they are kept on the ApiError, so an oversized 500 body outlived the request. Cap it. max_response_bytes defaults to 10 MiB, which no decision response approaches -- the entire request is capped at 64k tokens and the answers are smaller than the questions -- so the limit only ever bites on something that has gone wrong. The default transport now reads in chunks and stops at the ceiling rather than collecting the body first, and an honest Content-Length over the limit ends it before a single chunk arrives. A body from a custom transport is measured on the way back, which cannot un-read it but does keep it out of the parser and off the exception. ResponseTooLarge is a RubyDecisionModel::Error, so the retry loop passes it straight through: reading a too-large body again would only be slower. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 10 ++ README.md | 12 ++ lib/ruby_decision_model/client.rb | 67 ++++++++++- lib/ruby_decision_model/errors.rb | 12 ++ test/response_size_test.rb | 194 ++++++++++++++++++++++++++++++ 5 files changed, 292 insertions(+), 3 deletions(-) create mode 100644 test/response_size_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..b8bcae6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +- Added `max_response_bytes` (10 MiB by default, `nil` to disable) and the + `ResponseTooLarge` error. Response bodies had no ceiling: the default + transport buffered whatever the endpoint sent before anything could look at + it, and an error body was then retained on the exception. The default + transport now rejects an oversized `Content-Length` before reading and + otherwise stops mid-stream; bodies from custom transports are measured + before they reach the parser. + ## 0.1.0 - 2026-09-18 Provider-neutral release. One `Client`, two providers behind it. diff --git a/README.md b/README.md index 79638d6..5a0f36e 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ RubyDecisionModel::Client.new( base_url: nil, # overrides the provider base URL timeout: 5, # open and read timeout in seconds retry: { max_retries: 2 }, # RetryPolicy or a Hash of overrides + max_response_bytes: 10485760, # ceiling on a response body; nil disables transport: nil # see Transport ) @@ -167,6 +168,16 @@ that accepts `url:`, `headers:`, `body:` and returns return is still accepted and treated as having no headers, which means no `Retry-After` support and a nil `request_id`. +## Response size + +A response body is read into memory before it can be parsed, so the client caps +it at `max_response_bytes`, 10 MiB by default. The default transport checks +`Content-Length` first and otherwise stops mid-stream once the ceiling is +passed, so an endpoint offering an endless body never gets to fill the process. +A body returned by a custom transport is measured too, before it reaches the +JSON parser or an `ApiError`. Over the limit raises `ResponseTooLarge`, which +is never retried. `nil` disables the check. + ## Errors | Error | Meaning | @@ -180,6 +191,7 @@ return is still accepted and treated as having no headers, which means no | `UnprocessableEntity` | 422 (never retried) | | `RateLimited` | 429 (retried) | | `Overloaded` | 529 (retried) | +| `ResponseTooLarge` | Response body passed `max_response_bytes`, carries `#bytes` and `#limit` | | `InvalidResponse` | Body wasn't JSON, wasn't a Hash, or an answer was malformed | | `MissingAnswers` | One or more question ids came back missing or wrong-typed, carries `#missing` | diff --git a/lib/ruby_decision_model/client.rb b/lib/ruby_decision_model/client.rb index 3457726..183816a 100644 --- a/lib/ruby_decision_model/client.rb +++ b/lib/ruby_decision_model/client.rb @@ -17,7 +17,7 @@ class Client RETRYABLE_STATUSES = RetryPolicy::DEFAULT_STATUSES RETRYABLE_EXCEPTIONS = (RetryPolicy::TIMEOUT_EXCEPTIONS + RetryPolicy::CONNECTION_EXCEPTIONS).freeze - attr_reader :provider, :model, :retry_policy, :timeout + attr_reader :provider, :model, :retry_policy, :timeout, :max_response_bytes # provider: :open_router, :typesafe, or a Providers::Base instance. When # nil, api_key: alone selects OpenRouter; otherwise the @@ -31,8 +31,16 @@ class Client # retry: a RetryPolicy or a Hash of overrides. # random: callable returning a Float in 0...1, used for backoff jitter. # clock: callable returning monotonic seconds, used for total_timeout. + # The client reads a response into memory before it can parse it, so it + # needs a ceiling. 10 MiB is far more than any decision response: the + # whole request is capped at 64k tokens, and the answers are smaller than + # the questions. It exists to bound a hostile or broken endpoint, not to + # be tuned. + DEFAULT_MAX_RESPONSE_BYTES = 10 * 1024 * 1024 + def initialize(provider: nil, api_key: nil, model: nil, base_url: nil, timeout: 5, transport: nil, sleeper: ->(seconds) { sleep(seconds) }, retry: {}, + max_response_bytes: DEFAULT_MAX_RESPONSE_BYTES, random: -> { rand }, clock: -> { Process.clock_gettime(Process::CLOCK_MONOTONIC) }) @provider = resolve_provider(provider, api_key: api_key, base_url: base_url) unless @provider.api_key? @@ -42,6 +50,7 @@ def initialize(provider: nil, api_key: nil, model: nil, base_url: nil, timeout: @model = @provider.resolve_model(model) @timeout = timeout + @max_response_bytes = validate_max_response_bytes(max_response_bytes) @transport = transport || default_transport @sleeper = sleeper @retry_policy = RetryPolicy.from(binding.local_variable_get(:retry)) @@ -100,9 +109,60 @@ def default_transport headers.each { |k, v| request[k] = v } request.body = body - response = http.request(request) - [response.code.to_i, response.body, response.each_header.to_h] + read_capped(http, request) + end + end + + # Net::HTTP buffers a whole response before handing it over, so a huge + # body -- from a hostile endpoint, a proxy error page, or an upstream + # having a bad day -- is in memory before anything gets to reject it. + # Read it in chunks instead and stop at the ceiling. Content-Length, when + # the server sends an honest one, ends it before a single chunk arrives. + def read_capped(http, request) + limit = @max_response_bytes + result = nil + + http.request(request) do |response| + headers = response.each_header.to_h + status = response.code.to_i + declared = Integer(response["content-length"].to_s, exception: false) + raise_too_large(declared, limit) if limit && declared && declared > limit + + body = +"" + response.read_body do |chunk| + body << chunk + raise_too_large(body.bytesize, limit) if limit && body.bytesize > limit + end + + result = [status, body, headers] end + + result + end + + def raise_too_large(bytes, limit) + raise ResponseTooLarge.new( + "response body exceeds max_response_bytes (#{bytes} > #{limit} bytes)", + bytes: bytes, limit: limit + ) + end + + def validate_max_response_bytes(limit) + return limit if limit.nil? + return limit if limit.is_a?(Integer) && limit.positive? + + raise ConfigurationError, + "max_response_bytes must be nil or a positive Integer, got #{limit.inspect}" + end + + # A custom transport does its own reading, so the ceiling is checked again + # on whatever it returns. The bytes are already in memory by then, but + # they never reach the JSON parser or an error object that outlives them. + def enforce_response_limit!(response_body) + return if @max_response_bytes.nil? || response_body.nil? + + size = response_body.to_s.bytesize + raise_too_large(size, @max_response_bytes) if size > @max_response_bytes end def perform_with_retry(url:, headers:, body:) @@ -115,6 +175,7 @@ def perform_with_retry(url:, headers:, body:) status, response_body, response_headers = normalize_transport_result( @transport.call(url: url, headers: headers, body: body) ) + enforce_response_limit!(response_body) rescue Error raise rescue StandardError => e diff --git a/lib/ruby_decision_model/errors.rb b/lib/ruby_decision_model/errors.rb index d35c51a..79bdbba 100644 --- a/lib/ruby_decision_model/errors.rb +++ b/lib/ruby_decision_model/errors.rb @@ -39,6 +39,18 @@ class RateLimited < ApiError; end class Overloaded < ApiError; end + # The response was larger than max_response_bytes. Carries how far the + # client got before it gave up, and the limit it was measured against. + class ResponseTooLarge < Error + attr_reader :bytes, :limit + + def initialize(message, bytes: nil, limit: nil) + super(message) + @bytes = bytes + @limit = limit + end + end + class InvalidResponse < Error attr_reader :answers diff --git a/test/response_size_test.rb b/test/response_size_test.rb new file mode 100644 index 0000000..a677770 --- /dev/null +++ b/test/response_size_test.rb @@ -0,0 +1,194 @@ +# frozen_string_literal: true + +require "test_helper" +require "socket" + +# A one-shot HTTP server that answers on loopback, so the default Net::HTTP +# transport is exercised for real rather than stubbed. +class TinyServer + attr_reader :port, :bytes_written + + def initialize(&responder) + @server = TCPServer.new("127.0.0.1", 0) + @port = @server.addr[1] + @responder = responder + @bytes_written = 0 + @thread = Thread.new { serve } + end + + def close + @thread.kill + @server.close + rescue IOError + nil + end + + private + + def serve + loop do + socket = @server.accept + read_request(socket) + @responder.call(socket, self) + socket.close + rescue StandardError + nil + end + end + + def read_request(socket) + length = 0 + while (line = socket.gets) + break if line == "\r\n" + + length = Regexp.last_match(1).to_i if line =~ /\AContent-Length:\s*(\d+)/i + end + socket.read(length) if length.positive? + end + + public + + def write(socket, data) + socket.write(data) + @bytes_written += data.bytesize + end +end + +class ResponseSizeTest < Minitest::Test + def questions + { "urgent" => RubyDecisionModel::Questions.noul("Is this urgent?") } + end + + def success_body + JSON.generate( + "answers" => { "urgent" => { "type" => "noul", "noul" => 0.5 } }, + "usage" => { "input_tokens" => 1, "output_tokens" => 1 } + ) + end + + def build_client(transport, **options) + RubyDecisionModel::Client.new( + **{ api_key: "test-key", transport: transport, sleeper: no_sleep }.merge(options) + ) + end + + # --- configuration --- + + def test_default_limit_is_ten_mebibytes + assert_equal 10 * 1024 * 1024, build_client(FakeTransport.new([])).max_response_bytes + end + + def test_limit_must_be_nil_or_a_positive_integer + [0, -1, 1.5, "10"].each do |bad| + assert_raises(RubyDecisionModel::ConfigurationError, "expected #{bad.inspect} to be rejected") do + build_client(FakeTransport.new([]), max_response_bytes: bad) + end + end + assert_nil build_client(FakeTransport.new([]), max_response_bytes: nil).max_response_bytes + end + + # --- custom transports are checked too --- + + def test_an_oversized_body_from_a_custom_transport_is_rejected_before_parsing + transport = FakeTransport.new([[200, "x" * 100]]) + client = build_client(transport, max_response_bytes: 50) + + error = assert_raises(RubyDecisionModel::ResponseTooLarge) { client.ask(state: {}, questions: questions) } + + assert_equal 100, error.bytes + assert_equal 50, error.limit + end + + def test_an_oversized_error_body_is_rejected_too + # A 500 body is retained on the exception, so it needs the same ceiling. + transport = FakeTransport.new([[500, "x" * 100]]) + client = build_client(transport, max_response_bytes: 50, retry: { max_retries: 0 }) + + assert_raises(RubyDecisionModel::ResponseTooLarge) { client.ask(state: {}, questions: questions) } + end + + def test_an_oversized_response_is_not_retried + transport = FakeTransport.new([[200, "x" * 100]]) + client = build_client(transport, max_response_bytes: 50) + + assert_raises(RubyDecisionModel::ResponseTooLarge) { client.ask(state: {}, questions: questions) } + assert_equal 1, transport.calls.length + end + + def test_a_body_at_the_limit_is_accepted + body = success_body + transport = FakeTransport.new([[200, body]]) + client = build_client(transport, max_response_bytes: body.bytesize) + + assert_in_delta 0.5, client.ask(state: {}, questions: questions)["urgent"].noul + end + + def test_a_nil_limit_disables_the_check + transport = FakeTransport.new([[200, success_body]]) + client = build_client(transport, max_response_bytes: nil) + + assert_in_delta 0.5, client.ask(state: {}, questions: questions)["urgent"].noul + end + + # --- the default transport stops reading rather than buffering it all --- + + def test_the_default_transport_stops_reading_an_oversized_chunked_body + # No Content-Length: the only way to know the size is to read it, which is + # the case the cap has to handle by stopping mid-stream. + server = TinyServer.new do |socket, srv| + srv.write(socket, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n" \ + "Transfer-Encoding: chunked\r\n\r\n") + 600.times { srv.write(socket, "400\r\n#{'x' * 1024}\r\n") } + srv.write(socket, "0\r\n\r\n") + end + + client = RubyDecisionModel::Client.new( + api_key: "k", base_url: "http://127.0.0.1:#{server.port}", max_response_bytes: 64 * 1024, + sleeper: no_sleep, retry: { max_retries: 0 } + ) + + error = assert_raises(RubyDecisionModel::ResponseTooLarge) { client.ask(state: {}, questions: questions) } + + assert_operator error.bytes, :>, 64 * 1024 + # 600 KiB was on offer; the client gave up long before it had all of it. + assert_operator server.bytes_written, :<, 600 * 1024 + ensure + server&.close + end + + def test_the_default_transport_rejects_an_oversized_content_length_before_reading + server = TinyServer.new do |socket, srv| + srv.write(socket, "HTTP/1.1 200 OK\r\nContent-Length: 1048576\r\n\r\n") + srv.write(socket, "x" * 1024) + end + + client = RubyDecisionModel::Client.new( + api_key: "k", base_url: "http://127.0.0.1:#{server.port}", max_response_bytes: 1024, + sleeper: no_sleep, retry: { max_retries: 0 } + ) + + error = assert_raises(RubyDecisionModel::ResponseTooLarge) { client.ask(state: {}, questions: questions) } + + assert_equal 1_048_576, error.bytes + ensure + server&.close + end + + def test_the_default_transport_returns_a_body_within_the_limit + body = success_body + server = TinyServer.new do |socket, srv| + srv.write(socket, "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n" \ + "Content-Length: #{body.bytesize}\r\n\r\n#{body}") + end + + client = RubyDecisionModel::Client.new( + api_key: "k", base_url: "http://127.0.0.1:#{server.port}", sleeper: no_sleep + ) + + response = client.ask(state: {}, questions: questions) + + assert_in_delta 0.5, response["urgent"].noul + ensure + server&.close + end +end