Skip to content

fix(cli): store custom endpoint API key in .env instead of config.yaml - #57557

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-57547-custom-endpoint-env-key
Closed

fix(cli): store custom endpoint API key in .env instead of config.yaml#57557
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-57547-custom-endpoint-env-key

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When configuring a "Custom endpoint" via hermes model, the API key was stored
directly in config.yaml under model.api_key and custom_providers[].api_key.
This is a security concern: when Hermes edits its own config file, the raw API key
can leak into the model's context window.

This fix derives an environment variable name from the endpoint hostname (e.g.
api.featherless.aiFEATHERLESS_API_KEY), saves the key to ~/.hermes/.env,
and stores a ${VAR} reference in config.yaml instead. The existing
_expand_env_vars() mechanism in load_config() resolves these references at
runtime.

Related Issue

Fixes #57547

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • 🔒 Security fix

Changes Made

  • hermes_cli/model_setup_flows.py: Added _derive_custom_endpoint_env_var() helper that extracts the hostname from a custom endpoint URL, strips common prefixes/TLDs, and produces an UPPER_SNAKE_CASE_API_KEY env var name.
  • hermes_cli/model_setup_flows.py: Modified _model_flow_custom() to save the API key to ~/.hermes/.env via save_env_value() and store ${ENV_VAR} reference in config.yaml instead of the raw key.
  • hermes_cli/main.py: Extended _save_custom_provider() with a key_env parameter. When provided, stores key_env (not raw api_key) in the custom_providers entry. The runtime already resolves key_env via os.getenv() in auxiliary_client.py.
  • tests/cli/test_cli_provider_resolution.py: Updated test_model_flow_custom_persists_selected_api_mode to expect ${CODEX_EXAMPLE_API_KEY} instead of the raw key, and to verify key_env propagation. Added test_derive_custom_endpoint_env_var covering 8 URL patterns.

How to Test

  1. Run python -m pytest tests/cli/test_cli_provider_resolution.py -x -q — all 24 tests should pass.
  2. Run python -m pytest tests/hermes_cli/test_runtime_provider_resolution.py -x -q -k "custom" — all 48 tests should pass.
  3. Manual: run hermes model, select "Custom endpoint", enter a URL and API key. Verify the key appears in ~/.hermes/.env (not in config.yaml) and that ${ENV_VAR} is stored in config.yaml under model.api_key.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/cli/test_cli_provider_resolution.py -q and all 24 tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A (pure Python, urllib.parse and save_env_value are cross-platform)
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

When configuring a Custom endpoint via `hermes model`, the API key was
stored directly in config.yaml. This leaks credentials into the model
context when Hermes edits its own config.

Derive an env var name from the hostname (e.g. api.featherless.ai ->
FEATHERLESS_API_KEY), save the key to ~/.hermes/.env, and store a
${VAR} reference in config.yaml. The existing _expand_env_vars()
mechanism resolves these at runtime.

Fixes NousResearch#57547
@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 area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jul 3, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

Security evidence: the core fix moves custom endpoint API keys from config.yaml to .env, and the current-main/PR-head probe shows the raw key no longer lands in model.api_key or custom_providers[].api_key; however, CodeRabbit's major collision finding is source-backed because _derive_custom_endpoint_env_var() still normalizes custom endpoint hosts into provider-global names such as OPENAI_API_KEY, OPENROUTER_API_KEY, GROQ_API_KEY, and TOGETHER_API_KEY, and _model_flow_custom() then writes the entered custom endpoint key with save_env_value(_custom_key_env, effective_key), so configuring a custom endpoint that normalizes to one of those names can overwrite an existing standard provider credential in .env instead of creating a custom-scoped secret. I also checked the merge tree against current GitHub main eb99f82; the focused tests passed as 24 passed for tests/cli/test_cli_provider_resolution.py and 48 passed, 99 deselected for tests/hermes_cli/test_runtime_provider_resolution.py -k custom; the runtime custom-provider paths resolve key_env without persisting the raw secret; and CodeRabbit's malformed-port note is non-blocking for this security fix, while git diff --check still reports a final blank line in tests/cli/test_cli_provider_resolution.py.

Please namespace derived env vars for custom endpoints so they cannot equal provider-global credentials, while keeping distinct custom hosts stable and unique. Add regression coverage for a host that currently collides with an existing provider env var, and fix the trailing blank line before the next pass.

Signed: GPT-5.5-xhigh in Codex

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough security review, egilewski.

You're right — _derive_custom_endpoint_env_var() can normalize custom endpoint hosts into provider-global names (e.g., api.openai.comOPENAI_API_KEY), which would overwrite existing standard provider credentials in .env. This is a real collision risk.

Plan:

  1. Namespace derived env vars with a HERMES_CUSTOM_ prefix (e.g., HERMES_CUSTOM_OPENAI_API_KEY for a custom endpoint resolving to OpenAI's host) to prevent collision with provider-global credentials
  2. Add regression coverage for a host that currently collides with an existing provider env var
  3. Fix the trailing blank line in tests/cli/test_cli_provider_resolution.py

Will push a fix shortly.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving a real plaintext-secret persistence path out of config.yaml; current main still writes the key at hermes_cli/model_setup_flows.py:955 and hermes_cli/main.py:3827.

Problems

  • hermes_cli/model_setup_flows.py:66 derives OPENAI_API_KEY for https://api.openai.com/v1, and line 815 writes the custom key to that name. This can replace a standard provider credential. The collision is also acknowledged in the PR discussion.
  • Prefix/TLD stripping also makes different custom endpoints share a name; a namespace prefix alone does not make the derivation unique.

Suggested changes

  • Use a custom-only, deterministic name based on the full endpoint identity (or a stable digest), so it cannot equal provider-global keys or another custom endpoint's key.
  • Add regressions for preserving OPENAI_API_KEY and for distinct URLs that normalize to the same current name. Add a temp-HERMES_HOME persistence/resolution test rather than only mocking save_env_value.

Automated hermes-sweeper review.

if not hostname:
hostname = "custom"

return f"{hostname.upper()}_API_KEY"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This derives OPENAI_API_KEY for https://api.openai.com/v1 (and similarly provider-global names for other hosts). Line 815 then overwrites that variable in .env with the custom-endpoint credential. Please use a custom-only, endpoint-unique name; a HERMES_CUSTOM_ prefix alone must also retain enough of the full endpoint identity to avoid custom-to-custom collisions.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@asorry75

Copy link
Copy Markdown

Hi liuhao1024! Thanks for this fix — I ran into the same issue from the Desktop UI side and traced the code path before discovering your PR here.

Your fix covers the CLI path (model_setup_flows.py + main.py), but the Desktop settings panel uses a separate code path in web_server.py (_write_custom_endpoint) that still stores the key in plaintext. I submitted PR #69488 to cover that path, using the same key_env + save_env_value approach.

Not competing — just complementing. Both PRs together should fully resolve #57547 and #69449.

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have and removed P2 Medium — degraded but workaround exists labels Jul 22, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

Thanks @liuhao1024 — you were first on this, back in early July, and you caught the write path the other PRs missed: hermes model's custom-endpoint flow was writing the plaintext key too, on both the model block and the custom_providers entry. That's folded into #71141 with you as a co-author, including the key_env swap on re-save of a known URL.

One thing worth flagging, since your own test asserted it: _derive_custom_endpoint_env_var("http://127.0.0.1:8080/v1") returns 127_0_0_1_8080_API_KEY, which save_env_value rejects — _ENV_VAR_NAME_RE is ^[A-Za-z_][A-Za-z0-9_]*$, so a digit can't come first. The flow test didn't catch it because save_env_value was mocked, but a real local endpoint would have raised on save. The replacement uses a shared helper with a fixed HERMES_CUSTOM_ prefix, so the name is valid by construction and the Desktop and CLI paths agree on it.

Closing as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: "Custom endpoint" model provider places API key inside config.yaml, easily leaking into context when Hermes updates its own config.

6 participants