Skip to content

fix: store custom provider API keys in .env instead of config.yaml - #8382

Closed
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/custom-provider-api-key-env-8316
Closed

fix: store custom provider API keys in .env instead of config.yaml#8382
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/custom-provider-api-key-env-8316

Conversation

@konsisumer

Copy link
Copy Markdown
Contributor

Summary

Fixes #8316 — custom providers stored API keys directly in config.yaml, unlike standard providers which use environment variables via .env.

  • Adds _custom_provider_env_var() to derive a deterministic env var name from the provider URL (e.g. CUSTOM_API_SILICONFLOW_CN_API_KEY)
  • _save_custom_provider() now stores an api_key_env reference in config.yaml and saves the actual key to .env via save_env_value()
  • _model_flow_custom() and _model_flow_named_custom() updated to use api_key_env instead of inline api_key
  • runtime_provider.py resolves api_key_env from environment at runtime
  • Backward compatible: falls back to reading api_key if api_key_env is not present, and migrates legacy entries on next save

Test plan

  • Add a custom provider via hermes model and verify API key appears in ~/.hermes/.env (not config.yaml)
  • Verify config.yaml contains api_key_env: CUSTOM_<HOST>_API_KEY instead of api_key: <raw_key>
  • Verify existing custom providers with inline api_key still work (backward compatibility)
  • Verify existing custom providers get migrated to api_key_env on next save

@konsisumer
konsisumer marked this pull request as ready for review April 12, 2026 15:05
@konsisumer
konsisumer force-pushed the fix/custom-provider-api-key-env-8316 branch 6 times, most recently from 8465c13 to 3f2e968 Compare April 18, 2026 07:05
@konsisumer

Copy link
Copy Markdown
Contributor Author

CI failures here are pre-existing on main and not caused by this PR:

  • tests/hermes_cli/test_web_server.py::TestBuildSchemaFromConfig::test_no_single_field_categories — fails on main@cf012a05 with the same Category 'code_execution' has only 1 field(s) assertion. actions/checkout on pull_request events checks out the PR merged with main, so this PR inherits the failure.
  • tests/tools/test_browser_camofox_state.py::TestCamofoxConfigDefaults::test_config_version_matches_current_schema — fails on main@cf012a05 with assert 19 == 18. hermes_cli/config.py bumped _config_version to 19 on main, but the test still asserts == 18.
  • nix (ubuntu-latest) — fails on main@cf012a05 too (separate npm-deps hash-mismatch / alibabacloud build issue).

The PR's own diff touches only custom-provider env handling; all 12891 other tests pass. Happy to rebase once main is green, but fixing these would be unrelated scope.

@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto main and fixed both CI test failures:

The Nix check failure is a pre-existing infrastructure issue on main (alibabacloud-credentials-api missing setuptools build dep) unrelated to this PR. Local pytest: 2 previously-failing tests now pass.


autocontrib · pr-repair-72ce7d61 · 2026-04-19T15:52:29Z

@konsisumer
konsisumer force-pushed the fix/custom-provider-api-key-env-8316 branch from a948847 to dc27dc8 Compare April 19, 2026 15:52
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto main. One conflict in hermes_cli/web_server.py (_CATEGORY_MERGE["code_execution"]) — resolved by taking main's value ("agent") which already contained the fix. The previous fix(tests): update schema and config-version assertions to match main commit was dropped as empty (both _config_version == 19 and code_execution → agent were already in main). Docker build-and-push check won't run on PRs anymore after the path-filter CI update merged to main (2f67ef92). Local pytest on all PR-touched test files: 137 passed.


autocontrib · pr-repair-c7fd83ff · 2026-04-20T02:12:59Z

@konsisumer
konsisumer force-pushed the fix/custom-provider-api-key-env-8316 branch from dc27dc8 to 3855d21 Compare April 20, 2026 02:13
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto origin/main and fixed two pre-existing test failures that appeared on main after the last rebase:

  • test_switch_to_minimax_does_not_resolve_anthropic_token: switch_model() now prunes _fallback_chain on provider change, but the test bypassed __init__ without seeding that attribute. Added _fallback_chain = [] to the manual stub.
  • test_upload_encrypted_room_uses_file_payload: _upload_and_send() does from mautrix.crypto.attachments import encrypt_attachment at runtime for encrypted rooms. The test was missing patch.dict(sys.modules, _make_fake_mautrix()) — added it.

All PR-touched tests pass locally (98 tests). Remaining local failures are macOS-specific pre-existing issues (/private/var/ prefix in _SENSITIVE_PATH_PREFIXES blocking temp-dir writes) and unrelated to this PR's diff.

@konsisumer
konsisumer force-pushed the fix/custom-provider-api-key-env-8316 branch from 3855d21 to 20f4f56 Compare April 22, 2026 09:40
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #4674 which proposed the same api_key_env approach. This PR appears to be a more complete implementation.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels Apr 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #4674.

@konsisumer

konsisumer commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the pointer @alt-glitch. Looking at #4674@iws17's approach is cleaner: it stays focused on the feature request (add `api_key_env` support) without the broader .env migration this PR introduces. Closing in favor of #4674. Happy to see that one move forward.

@konsisumer konsisumer closed this Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: [SECURITY] Custom providers store API keys directly in config.yaml instead of using environment variables

2 participants