fix(security): gate OPENAI_API_KEY / OPENROUTER_API_KEY env fallback on host (#28660)#29606
Merged
Conversation
…dpoints 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
- Preserve OPENROUTER_API_KEY for explicit mirror/proxy configs when requested provider is openrouter and OPENROUTER_BASE_URL is set - Gate OPENAI_API_KEY and OPENROUTER_API_KEY in named custom provider path (_resolve_named_custom_runtime) on authoritative hosts - Gate same keys in direct-alias path - Update tests to reflect secure-by-default behavior for local endpoints
…fallback After #28660's host-gating fix, users with provider=custom and base_url pointed at a commercial endpoint (DeepSeek, Groq, Mistral, …) hit no-key-required even when they had the vendor-named env var set (DEEPSEEK_API_KEY, GROQ_API_KEY, …). The issue author flagged this as 'what users intuitively expect'. Adds _host_derived_api_key() to derive an env var name from the base URL host using the *registrable* label (second-to-last). Appended to all three api_key_candidates chains (_resolve_named_custom_runtime direct-alias path, named-custom path, _resolve_openrouter_runtime non-openrouter branch). Lookalike resistance: api.deepseek.com.attacker.test resolves to vendor label 'attacker', NOT 'deepseek' — DEEPSEEK_API_KEY stays put. IPs and loopback yield no vendor label. Already-handled vendors (OPENAI/OPENROUTER/ OLLAMA) are filtered to prevent bypass of the explicit host-gated paths. Adds 6 tests covering positive paths (DeepSeek, Groq), the lookalike attack, loopback rejection, the already-handled-vendor filter, and direct helper unit tests. Also adds erhnysr to AUTHOR_MAP.
Contributor
🔎 Lint report:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvages @erhnysr's #28884 and extends it with the bonus
<VENDOR>_API_KEYderivation requested in the issue.Summary
Closes #28660.
provider: custom+base_url: https://api.deepseek.com/v1no longer silently sendsOPENAI_API_KEYto DeepSeek. Vendor-named env vars (DEEPSEEK_API_KEY,GROQ_API_KEY,MISTRAL_API_KEY, …) are now derived from the host and picked up automatically.Changes
hermes_cli/runtime_provider.py: gateOPENAI_API_KEYonopenai.com/openai.azure.com,OPENROUTER_API_KEYonopenrouter.ai, mirroring the existingOLLAMA_API_KEYhost-gate. Applied at all 3 sites (_resolve_named_custom_runtimedirect-alias path, named-custom path,_resolve_openrouter_runtimenon-openrouter branch)._host_derived_api_key(): new helper, takes the registrable label (labels[-2]) soapi.deepseek.com→DEEPSEEK_API_KEYbutapi.deepseek.com.attacker.test→ATTACKER_API_KEY(the lookalike host picks the attacker's label, not the spoofed vendor). IPs/loopback return "". Already-handled vendors (OPENAI/OPENROUTER/OLLAMA) are filtered to prevent bypass.scripts/release.py: AUTHOR_MAP entry for @erhnysr.Validation
OPENAI_API_KEYset, base=deepseek.comno-key-requiredDEEPSEEK_API_KEYset, base=deepseek.comno-key-requireddk-real-deepseekDEEPSEEK_API_KEYsetno-key-required(attacker label derived)OPENAI_API_KEYset, base=openai.comOPENAI_API_KEYset, base=127.0.0.1no-key-requiredTargeted tests: 125/125 passing. Full
tests/hermes_cli/suite: 4976/4976 (one unrelated xdist-order flake, passes in isolation).Original PR #28884 by @erhnysr (whose two substantive commits are preserved here verbatim) will be closed pointing here.
Infographic