From 96e1006ba967f4598ad069b4c2862fd0460ae61f Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:19:30 +0200 Subject: [PATCH] security: reject base URLs that would leak the API key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every request carries the key in an Authorization header, and nothing checked where that header was going. `base_url` was concatenated with `endpoint_path` and handed to Net::HTTP, which turns TLS on only when the scheme happens to be https. So `base_url: "http://api.typesafe.ai"` — a typo, a copied snippet, a value read from config — sent `Bearer ` in cleartext, silently. A URL carrying userinfo or a fragment was worse than silent: `https://trusted.example@evil.example/p#x` reads as trusted.example, resolves to evil.example, and the appended endpoint path lands inside the fragment instead of the request path. Validate the base URL once, when the client is built, before any key goes out: it has to be an absolute http(s) URL with a host, no userinfo, and no query or fragment, and it has to be https unless the host is loopback, where nothing leaves the machine. Anything else raises ConfigurationError. The error message rewrites userinfo to "..." so a password pasted into a base URL does not end up in a backtrace. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 ++ README.md | 9 ++- lib/ruby_decision_model/client.rb | 2 + lib/ruby_decision_model/providers/base.rb | 40 ++++++++++ test/base_url_test.rb | 92 +++++++++++++++++++++++ 5 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 test/base_url_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..5f7e0dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +- `base_url` is now validated when the client is built: it must be an absolute + `https` URL with no userinfo, query, or fragment. Plain `http` is allowed only + for loopback hosts. Anything else raises `ConfigurationError` instead of + sending the API key in cleartext or to a host the URL does not name. + ## 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..0526e65 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,13 @@ client.model # => "jev-latest" (resolved after aliasing) Both providers send `User-Agent: ruby_decision_model/`. +`base_url` is validated when the client is built. It must be an absolute +`https` URL with no userinfo, query, or fragment; plain `http` is accepted only +for `localhost`, `127.0.0.1`, and `[::1]`, where nothing leaves the machine. +Anything else raises `ConfigurationError`, because every request carries the API +key in an `Authorization` header and a plaintext or redirected base URL hands +that key to whoever is listening. + ### Model aliases Each provider resolves a few friendly names to its own canonical model name. @@ -171,7 +178,7 @@ return is still accepted and treated as having no headers, which means no | Error | Meaning | | --- | --- | -| `ConfigurationError` | No provider could be resolved, missing api_key, unknown provider, or bad `retry:` value | +| `ConfigurationError` | No provider could be resolved, missing api_key, unknown provider, bad `base_url`, or bad `retry:` value | | `RequestError` | Questions hash was empty | | `TransportError` (`TimeoutError`) | Network or timeout failure after retries, carries `#cause_error` | | `ApiError` | Non-2xx response, carries `#status`, `#body`, and `#headers` | diff --git a/lib/ruby_decision_model/client.rb b/lib/ruby_decision_model/client.rb index 3457726..eaa0351 100644 --- a/lib/ruby_decision_model/client.rb +++ b/lib/ruby_decision_model/client.rb @@ -40,6 +40,8 @@ def initialize(provider: nil, api_key: nil, model: nil, base_url: nil, timeout: "api_key is required for #{@provider.name}: pass api_key: or set #{@provider.env_var}" end + @provider.validate_base_url! + @model = @provider.resolve_model(model) @timeout = timeout @transport = transport || default_transport diff --git a/lib/ruby_decision_model/providers/base.rb b/lib/ruby_decision_model/providers/base.rb index 85f7089..62b6c6d 100644 --- a/lib/ruby_decision_model/providers/base.rb +++ b/lib/ruby_decision_model/providers/base.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "json" +require "uri" module RubyDecisionModel module Providers @@ -59,6 +60,45 @@ def api_key? !(api_key.nil? || api_key.to_s.strip.empty?) end + # Hosts that never leave the machine, where plain http carries no risk. + LOOPBACK_HOSTS = %w[localhost 127.0.0.1 ::1 [::1]].freeze + + # Every request carries the API key in an Authorization header, so the + # base URL has to be one that keeps it secret. Raises unless it is an + # absolute http(s) URL, and unless it is https or a loopback host. + # Called once when a Client is built, so a bad URL fails before the + # first request rather than after the key has already gone out. + def validate_base_url! + uri = begin + URI.parse(base_url) + rescue URI::InvalidURIError => e + raise ConfigurationError, "base_url is not a valid URL: #{base_url.inspect} (#{e.message})" + end + + unless uri.is_a?(URI::HTTP) && !uri.host.to_s.empty? + raise ConfigurationError, "base_url must be an absolute http(s) URL, got #{base_url.inspect}" + end + + unless uri.userinfo.nil? + raise ConfigurationError, "base_url must not carry userinfo, got #{base_url.sub(uri.userinfo, '...').inspect}" + end + + # endpoint_path is appended to base_url as a string. A query or a + # fragment would swallow it -- "https://host/p?x=1" + "/v1/systemone" + # posts to /p with the path buried in the query -- so reject both + # rather than build a URL that points somewhere else than it reads. + unless uri.query.nil? && uri.fragment.nil? + raise ConfigurationError, + "base_url must not carry a query or fragment, got #{base_url.inspect}" + end + + return if uri.is_a?(URI::HTTPS) || LOOPBACK_HOSTS.include?(uri.host.downcase) + + raise ConfigurationError, + "base_url must use https: over plain http the #{env_var} bearer token is sent in " \ + "cleartext. Got #{base_url.inspect}. http is allowed only for #{LOOPBACK_HOSTS.join(', ')}." + end + # Nil or blank means the provider default. Known aliases resolve to the # provider's canonical name. Anything else passes through untouched. def resolve_model(model) diff --git a/test/base_url_test.rb b/test/base_url_test.rb new file mode 100644 index 0000000..168e06f --- /dev/null +++ b/test/base_url_test.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require "test_helper" + +# The API key travels in an Authorization header on every request, so the base +# URL is the thing that decides whether it stays secret. +class BaseUrlTest < Minitest::Test + def build(base_url) + RubyDecisionModel::Client.new( + provider: :typesafe, api_key: "ts-secret", base_url: base_url, transport: FakeTransport.new([]) + ) + end + + def test_https_base_url_is_accepted + client = build("https://api.typesafe.ai") + assert_equal "https://api.typesafe.ai/v1/systemone", client.provider.url + end + + def test_plain_http_base_url_is_rejected + error = assert_raises(RubyDecisionModel::ConfigurationError) { build("http://api.typesafe.ai") } + assert_match(/https/, error.message) + assert_match(/cleartext/, error.message) + end + + def test_plain_http_is_allowed_for_loopback_hosts + %w[http://localhost:4000 http://127.0.0.1:4000 http://[::1]:4000].each do |url| + client = build(url) + assert_equal "#{url}/v1/systemone", client.provider.url + end + end + + def test_loopback_host_matching_ignores_case + client = build("http://LOCALHOST:4000") + assert_equal "http://LOCALHOST:4000/v1/systemone", client.provider.url + end + + def test_non_http_scheme_is_rejected + error = assert_raises(RubyDecisionModel::ConfigurationError) { build("ftp://api.typesafe.ai") } + assert_match(/absolute http\(s\) URL/, error.message) + end + + def test_base_url_without_a_host_is_rejected + error = assert_raises(RubyDecisionModel::ConfigurationError) { build("api.typesafe.ai") } + assert_match(/absolute http\(s\) URL/, error.message) + end + + def test_unparseable_base_url_is_rejected + error = assert_raises(RubyDecisionModel::ConfigurationError) { build("https://exa mple.com") } + assert_match(/not a valid URL/, error.message) + end + + def test_userinfo_in_base_url_is_rejected_and_not_echoed + error = assert_raises(RubyDecisionModel::ConfigurationError) { build("https://user:pw@api.typesafe.ai") } + assert_match(/userinfo/, error.message) + refute_includes error.message, "pw" + end + + def test_query_or_fragment_in_base_url_is_rejected + # "https://api.typesafe.ai/p?x=1" + "/v1/systemone" would POST to /p. + ["https://api.typesafe.ai/p?x=1", "https://api.typesafe.ai/p#frag"].each do |url| + error = assert_raises(RubyDecisionModel::ConfigurationError) { build(url) } + assert_match(/query or fragment/, error.message) + end + end + + def test_userinfo_host_confusion_is_rejected + # Reads as trusted.example, resolves to evil.example. + error = assert_raises(RubyDecisionModel::ConfigurationError) do + build("https://trusted.example@evil.example/p") + end + assert_match(/userinfo/, error.message) + end + + def test_env_supplied_http_base_url_is_rejected_too + error = assert_raises(RubyDecisionModel::ConfigurationError) do + RubyDecisionModel::Client.new( + provider: RubyDecisionModel::Providers::Typesafe.new(api_key: "k", base_url: "http://evil.example.com"), + transport: FakeTransport.new([]) + ) + end + assert_match(/cleartext/, error.message) + end + + def test_the_default_base_urls_pass_validation + with_env("TYPESAFE_API_KEY" => "ts-key", "OPENROUTER_API_KEY" => "or-key") do + RubyDecisionModel::Providers.names.each do |name| + client = RubyDecisionModel::Client.new(provider: name, transport: FakeTransport.new([])) + assert_match(%r{\Ahttps://}, client.base_url) + end + end + end +end