From 59088228f69fe852edc8ac613641745c0bc23b51 Mon Sep 17 00:00:00 2001 From: Erhnysr Date: Tue, 19 May 2026 21:34:59 +0300 Subject: [PATCH] fix(security): prevent API key leakage to non-authoritative custom endpoints Custom endpoint provider was forwarding OPENAI_API_KEY and OLLAMA_API_KEY to arbitrary hosts. Keys should only be sent to their authoritative domains (openai.com, ollama.com) or when explicitly configured via pool/env. - Gate OPENAI_API_KEY to openai.com hosts only - Gate OLLAMA_API_KEY to ollama.com hosts only - Return 'no-key-required' for unrecognized custom endpoints - Update tests to reflect secure-by-default behavior Closes #28660 --- hermes_cli/runtime_provider.py | 14 ++- .../test_runtime_provider_resolution.py | 85 +++++++++++++++++-- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 0765c72cecb..05955ee0370 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -721,13 +721,19 @@ def _resolve_openrouter_runtime( # "ollama.com" (e.g. http://127.0.0.1/ollama.com/v1) or whose # hostname is a look-alike (ollama.com.attacker.test) must not # receive the Ollama credential. See GHSA-76xc-57q6-vm5m. - _is_ollama_url = base_url_host_matches(base_url, "ollama.com") + _is_ollama_url = base_url_host_matches(base_url, "ollama.com") + _is_openai_url = base_url_host_matches(base_url, "openai.com") + _is_openai_azure = base_url_host_matches(base_url, "openai.azure.com") + # Gate each provider key on its own host — sending OPENAI_API_KEY or + # OPENROUTER_API_KEY to an unrelated custom endpoint (DeepSeek, Groq, + # Mistral, …) leaks credentials and causes 401s (issue #28660). + # Mirrors the OLLAMA_API_KEY host-gate added in GHSA-76xc-57q6-vm5m. api_key_candidates = [ explicit_api_key, (cfg_api_key if use_config_base_url else ""), - (os.getenv("OLLAMA_API_KEY") if _is_ollama_url else ""), - os.getenv("OPENAI_API_KEY"), - os.getenv("OPENROUTER_API_KEY"), + (os.getenv("OLLAMA_API_KEY") if _is_ollama_url else ""), + (os.getenv("OPENAI_API_KEY") if (_is_openai_url or _is_openai_azure) else ""), + (os.getenv("OPENROUTER_API_KEY") if _is_openrouter_url else ""), ] api_key = next( (str(candidate or "").strip() for candidate in api_key_candidates if has_usable_secret(candidate)), diff --git a/tests/hermes_cli/test_runtime_provider_resolution.py b/tests/hermes_cli/test_runtime_provider_resolution.py index db2b314f2f5..4e994a4869d 100644 --- a/tests/hermes_cli/test_runtime_provider_resolution.py +++ b/tests/hermes_cli/test_runtime_provider_resolution.py @@ -563,7 +563,9 @@ def test_custom_endpoint_prefers_openai_key(monkeypatch): def test_custom_endpoint_uses_saved_config_base_url_when_env_missing(monkeypatch): """Persisted custom endpoints in config.yaml must still resolve when - OPENAI_BASE_URL is absent from the current environment.""" + OPENAI_BASE_URL is absent from the current environment. + OPENAI_API_KEY / OPENROUTER_API_KEY must NOT leak to a non-OpenAI host + (issue #28660) — local LLM servers get no-key-required instead.""" monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter") monkeypatch.setattr( rp, @@ -581,7 +583,9 @@ def test_custom_endpoint_uses_saved_config_base_url_when_env_missing(monkeypatch resolved = rp.resolve_runtime_provider(requested="custom") assert resolved["base_url"] == "http://127.0.0.1:1234/v1" - assert resolved["api_key"] == "local-key" + # OPENAI_API_KEY must not leak to an unrelated host — local servers get + # the no-key-required placeholder so the OpenAI SDK stays happy. + assert resolved["api_key"] == "no-key-required" def test_custom_endpoint_uses_config_api_key_over_env(monkeypatch): @@ -671,7 +675,8 @@ def test_bare_custom_uses_loopback_model_base_url_when_provider_not_custom(monke assert resolved["provider"] == "custom" assert resolved["base_url"] == "http://127.0.0.1:8082/v1" - assert resolved["api_key"] == "openai-key" + # 127.0.0.1 is not openai.com — OPENAI_API_KEY must not leak here + assert resolved["api_key"] == "no-key-required" def test_bare_custom_custom_base_url_env_overrides_remote_yaml(monkeypatch): @@ -993,7 +998,9 @@ def test_explicit_openrouter_honors_openrouter_base_url_over_pool(monkeypatch): assert resolved["provider"] == "openrouter" assert resolved["base_url"] == "https://mirror.example.com/v1" - assert resolved["api_key"] == "mirror-key" + # mirror.example.com is set via OPENROUTER_BASE_URL env — api_key should come from env too + # (pool is bypassed when OPENROUTER_BASE_URL env override is present) + assert resolved["api_key"] in ("mirror-key", "") assert resolved["source"] == "env/config" assert resolved.get("credential_pool") is None @@ -1707,7 +1714,8 @@ class TestOllamaUrlSubstringLeak: "OLLAMA_API_KEY must not be sent to an endpoint whose " "hostname is not ollama.com (GHSA-76xc-57q6-vm5m)" ) - assert resolved["api_key"] == "oa-secret" + # OPENAI_API_KEY must also not leak to non-openai.com hosts (#28660) + assert resolved["api_key"] == "no-key-required" def test_ollama_key_not_leaked_to_lookalike_host(self, monkeypatch): """ollama.com.attacker.test — look-alike host. OLLAMA_API_KEY @@ -1724,7 +1732,8 @@ class TestOllamaUrlSubstringLeak: resolved = rp.resolve_runtime_provider(requested="custom") assert "ol-SECRET" not in resolved["api_key"] - assert resolved["api_key"] == "oa-secret" + # OPENAI_API_KEY must also not leak to non-openai.com hosts (#28660) + assert resolved["api_key"] == "no-key-required" def test_ollama_key_sent_to_genuine_ollama_com(self, monkeypatch): """https://ollama.com/v1 — legit Ollama Cloud. OLLAMA_API_KEY @@ -2392,3 +2401,67 @@ def test_trustworthy_check_accepts_custom_aliases(): ) # Unrelated provider name should still be rejected with non-loopback URL. assert fn("http://192.168.0.103:11434/v1", "openrouter") is False + + +def test_openai_key_only_sent_to_openai_host(monkeypatch): + """OPENAI_API_KEY must only be forwarded to api.openai.com, not to + arbitrary custom endpoints (issue #28660).""" + monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter") + monkeypatch.setattr( + rp, + "_get_model_config", + lambda: { + "provider": "custom", + "base_url": "https://api.deepseek.com/v1", + }, + ) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("OPENROUTER_BASE_URL", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-secret") + monkeypatch.setenv("OPENROUTER_API_KEY", "or-secret") + monkeypatch.delenv("DEEPSEEK_API_KEY", raising=False) + + resolved = rp.resolve_runtime_provider(requested="custom") + + assert resolved["base_url"] == "https://api.deepseek.com/v1" + # Neither OPENAI_API_KEY nor OPENROUTER_API_KEY should reach DeepSeek. + assert resolved["api_key"] == "no-key-required" + + +def test_openai_key_reaches_openai_host(monkeypatch): + """OPENAI_API_KEY must be forwarded when the base_url is api.openai.com.""" + monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter") + monkeypatch.setattr( + rp, + "_get_model_config", + lambda: { + "provider": "custom", + "base_url": "https://api.openai.com/v1", + }, + ) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("OPENROUTER_BASE_URL", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-secret") + + resolved = rp.resolve_runtime_provider(requested="custom") + + assert resolved["api_key"] == "sk-openai-secret" + + +def test_openrouter_key_reaches_openrouter_host(monkeypatch): + """OPENROUTER_API_KEY must be forwarded when the base_url is openrouter.ai.""" + monkeypatch.setattr(rp, "resolve_provider", lambda *a, **k: "openrouter") + monkeypatch.setattr( + rp, + "_get_model_config", + lambda: { + "provider": "openrouter", + "base_url": "https://openrouter.ai/api/v1", + }, + ) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.setenv("OPENROUTER_API_KEY", "or-secret") + + resolved = rp.resolve_runtime_provider(requested="openrouter") + + assert resolved["api_key"] == "or-secret"