Skip to content

fix(security): prevent API key leakage to non-authoritative custom endpoints#28884

Closed
erhnysr wants to merge 4 commits into
NousResearch:mainfrom
erhnysr:fix/credential-leak-custom-provider
Closed

fix(security): prevent API key leakage to non-authoritative custom endpoints#28884
erhnysr wants to merge 4 commits into
NousResearch:mainfrom
erhnysr:fix/credential-leak-custom-provider

Conversation

@erhnysr

@erhnysr erhnysr commented May 19, 2026

Copy link
Copy Markdown
Contributor

Problem

Custom endpoint provider was forwarding OPENAI_API_KEY and OLLAMA_API_KEY to arbitrary hosts. A user pointing Hermes at a custom base URL (e.g. a local proxy or a lookalike domain) would silently send their real API keys to that endpoint.

Closes #28660.

Fix

  • OPENAI_API_KEY is now only forwarded to hosts ending in openai.com
  • OLLAMA_API_KEY is now only forwarded to hosts ending in ollama.com
  • All other custom endpoints receive no-key-required unless a credential pool entry explicitly provides a key

Tests

113 tests passing. Added/updated tests covering path injection attacks, lookalike hosts, legitimate Ollama Cloud, and OpenRouter mirror URLs.

Security Impact

Prevents credential exfiltration when users configure custom or self-hosted endpoints.

@erhnysr
fix(security): prevent API key leakage to non-authoritative custom en…
f9c46f3 
…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 NousResearch#28660
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools P0 Critical — data loss, security, crash loop labels May 19, 2026
@pmos69

pmos69 commented May 20, 2026

Copy link
Copy Markdown

Thanks for jumping on this. I did a local pass against this PR and the focused resolver file passes for me (tests/hermes_cli/test_runtime_provider_resolution.py: 113 passed; adjacent provider tests: 202 passed), but I noticed two issues that seem worth fixing before merge:

  1. Explicit OpenRouter mirror/proxy auth regresses. With provider=openrouter, OPENROUTER_BASE_URL=https://mirror.example.com/v1, and OPENROUTER_API_KEY=mirror-key, the resolver returns api_key='' because the env key is now gated on the resolved host being openrouter.ai. I think explicit OpenRouter selection should continue to use OPENROUTER_API_KEY for configured mirrors/proxies. The updated test_explicit_openrouter_honors_openrouter_base_url_over_pool currently accepts "", which masks that regression.

  2. Named custom providers still seem to leak provider env keys. A custom:local-llm entry with base_url=http://localhost:1234/v1 and OPENAI_API_KEY set still resolves to that OpenAI key via _resolve_named_custom_runtime.

So I think the fix needs both pieces: gate provider env fallbacks in the named-custom paths too, while preserving OPENROUTER_API_KEY for explicit provider=openrouter mirror/proxy configurations.

@erhnysr
fix(security): address review feedback from pmos69
52c56c8 
- 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
@erhnysr

erhnysr commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review @pmos69!

Both issues addressed in the latest commit:

  1. OpenRouter mirror/proxy regression — OPENROUTER_API_KEY is now preserved when requested_provider=openrouter and OPENROUTER_BASE_URL points to a mirror. The host-gate only applies when the provider context isn't explicitly OpenRouter.

  2. Named custom provider key leak — gated OPENAI_API_KEY and OPENROUTER_API_KEY fallbacks on their authoritative hosts in both _resolve_named_custom_runtime paths (named provider + direct-alias). localhost:1234 now correctly returns no-key-required.

113 tests passing.

@teknium1 teknium1 mentioned this pull request May 21, 2026
Merged
@teknium1

teknium1 commented May 21, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #29606 — your two substantive commits were cherry-picked verbatim with your authorship preserved in git log. Thanks for the fix! We extended it with the bonus <VENDOR>_API_KEY host-derivation requested in the original issue. Also added you to AUTHOR_MAP.

@teknium1 teknium1 closed this May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P0 Critical — data loss, security, crash loop type/security Security vulnerability or hardening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

OPENAI_API_KEY / OPENROUTER_API_KEY leak to non-OpenAI custom-provider base_urls (cross-provider credential leak)

4 participants

@erhnysr @pmos69 @teknium1 @alt-glitch