From a061788d4356a8fcc2e9fdb488be412d99d8d22d Mon Sep 17 00:00:00 2001 From: solyanviktor-star <233359899+solyanviktor-star@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:44:03 +0300 Subject: [PATCH] security(providers): strip credential headers on cross-host redirects in fetch_models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch_models() sends Authorization: Bearer plus any default_headers (x-api-key etc.) via urllib.request.urlopen, and urllib's redirect handler forwards every header when following a 3xx — including to a different host. A catalog endpoint (or a compromised/misconfigured proxy in front of it) answering with a redirect to another origin therefore received the provider API key. Install an HTTPRedirectHandler that drops authorization, x-api-key, api-key, x-goog-api-key and cookie when the redirect target hostname differs from the original request, mirroring the pattern already used in skills/creative/comfyui/scripts/_common.py. Same-host redirects keep credentials so legitimate path-level redirects still work. Co-Authored-By: Claude Fable 5 --- providers/base.py | 25 +++++++- tests/providers/test_fetch_models_base_url.py | 60 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/providers/base.py b/providers/base.py index 4a045a6765d..469a818198f 100644 --- a/providers/base.py +++ b/providers/base.py @@ -194,6 +194,7 @@ class ProviderProfile: url = effective_base.rstrip("/") + "/models" import json + import urllib.parse import urllib.request req = urllib.request.Request(url) @@ -207,8 +208,30 @@ class ProviderProfile: for k, v in self.default_headers.items(): req.add_header(k, v) + # urllib keeps every header — including Authorization — when it + # follows a redirect, so a catalog endpoint answering 3xx to a + # different host would receive the provider API key. Drop + # credential headers on cross-host redirects. + sensitive = {"authorization", "x-api-key", "api-key", "x-goog-api-key", "cookie"} + original_host = (urllib.parse.urlparse(url).hostname or "").lower() + + class _StripAuthOnCrossHostRedirect(urllib.request.HTTPRedirectHandler): + def redirect_request(self, redirected, fp, code, msg, hdrs, newurl): + new_req = super().redirect_request( + redirected, fp, code, msg, hdrs, newurl + ) + if new_req is not None: + new_host = (urllib.parse.urlparse(newurl).hostname or "").lower() + if new_host != original_host: + for header in list(new_req.headers): + if header.lower() in sensitive: + new_req.remove_header(header) + return new_req + + opener = urllib.request.build_opener(_StripAuthOnCrossHostRedirect()) + try: - with urllib.request.urlopen(req, timeout=timeout) as resp: + with opener.open(req, timeout=timeout) as resp: data = json.loads(resp.read().decode()) items = data if isinstance(data, list) else data.get("data", []) return [m["id"] for m in items if isinstance(m, dict) and "id" in m] diff --git a/tests/providers/test_fetch_models_base_url.py b/tests/providers/test_fetch_models_base_url.py index 5db1f61d1cd..e046f4edb27 100644 --- a/tests/providers/test_fetch_models_base_url.py +++ b/tests/providers/test_fetch_models_base_url.py @@ -117,6 +117,66 @@ class TestCustomProviderBaseUrlPassthrough: server.shutdown() +class _RedirectingHandler(BaseHTTPRequestHandler): + """Redirects /models to another hostname and records received headers.""" + + redirect_host = "localhost" # resolves to the same server, different hostname + received_headers: dict = {} + + def do_GET(self): + if self.path.rstrip("/") == "/models": + self.send_response(302) + self.send_header( + "Location", + f"http://{self.redirect_host}:{self.server.server_address[1]}/redirected", + ) + self.end_headers() + else: + type(self).received_headers = dict(self.headers) + body = json.dumps({"data": [{"id": "redirected-model"}]}).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(body) + + def log_message(self, format, *args): + pass + + +class TestFetchModelsRedirectCredentialStripping: + """Credential headers must not follow a redirect to a different host.""" + + def _run(self, redirect_host): + _RedirectingHandler.redirect_host = redirect_host + _RedirectingHandler.received_headers = {} + server = HTTPServer(("127.0.0.1", 0), _RedirectingHandler) + port = server.server_address[1] + Thread(target=server.serve_forever, daemon=True).start() + try: + profile = ProviderProfile( + name="test", + base_url=f"http://127.0.0.1:{port}", + default_headers={"x-api-key": "default-header-secret"}, + ) + result = profile.fetch_models(api_key="bearer-secret") + finally: + server.shutdown() + headers = {k.lower(): v for k, v in _RedirectingHandler.received_headers.items()} + return result, headers + + def test_cross_host_redirect_strips_credentials(self): + result, headers = self._run(redirect_host="localhost") + assert result == ["redirected-model"] # fetch itself still works + assert "authorization" not in headers + assert "x-api-key" not in headers + + def test_same_host_redirect_keeps_credentials(self): + result, headers = self._run(redirect_host="127.0.0.1") + assert result == ["redirected-model"] + assert headers.get("authorization") == "Bearer bearer-secret" + assert headers.get("x-api-key") == "default-header-secret" + + class TestModelPickerBaseUrlIntegration: """The /model picker path should pass model.base_url to fetch_models."""