fix(credentials): extend prefer-dotenv policy to remaining resolve paths - #20929
Open
SandroHub013 wants to merge 1 commit into
Open
fix(credentials): extend prefer-dotenv policy to remaining resolve paths#20929SandroHub013 wants to merge 1 commit into
SandroHub013 wants to merge 1 commit into
Conversation
Extends the prefer-dotenv lookup policy from NousResearch#20602 to the credential-bearing call sites that PR leaves untouched: 1. `hermes_cli/runtime_provider.py::_resolve_azure_foundry_runtime` — the Azure Foundry API key now reads via `get_env_value_prefer_dotenv("AZURE_FOUNDRY_API_KEY")` so a freshly rotated key in `~/.hermes/.env` immediately wins over a stale shell-exported value. 2. `hermes_cli/nous_subscription.py` — the module-level `get_env_value` import is aliased to `get_env_value_prefer_dotenv`. Every call site in that file is a credential probe (EXA, FIRECRAWL, PARALLEL, TAVILY, ELEVENLABS, BROWSER_USE, BROWSERBASE_*, etc.) that gates subscription-feature availability, so the alias is the right unit of change — the 25 internal call sites stay textually identical. 3. `tools/tool_backend_helpers.py::fal_key_is_configured` — switched to `get_env_value_prefer_dotenv("FAL_KEY")` for the same reason. Display-only paths (`hermes status`, `doctor`, `setup`, `hermes_cli/mcp_config.py`) intentionally keep the old `get_env_value` semantics so diagnostic output continues to reflect what the user's shell sees. Without this follow-up, an Azure Foundry user who rotates their key in `~/.hermes/.env` would still hit "401 unauthorized" until they restart their shell, because `_resolve_azure_foundry_runtime` would keep handing the stale shell-exported key to the SDK on every request. Same shape of bug for every Nous-subscription tool probe. Depends on NousResearch#20602 (`fix/20591-resolve-prefers-dotenv`). This PR vendors a copy of `get_env_value_prefer_dotenv` in `hermes_cli/config.py` with a NOTE: comment marking it for reconciliation, so the change is testable on its own. Once NousResearch#20602 merges, drop the duplicate definition, keep all call-site changes. Closes part of NousResearch#20591.
alt-glitch
added
P2
teknium1
reviewed
Jul 12, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for tracing the remaining credential-resolution paths. The Azure Foundry and FAL paths are still environment-first on current main (hermes_cli/runtime_provider.py:1327-1328; tools/tool_backend_helpers.py:172-179), so the core issue remains valid.
Problems
- The module-wide
nous_subscriptionalias also changes non-credential inputs:HERMES_LOCAL_STT_COMMAND(hermes_cli/nous_subscription.py:243,434) and URL/config probes such asFIRECRAWL_API_URL/SEARXNG_URL(:410-413). Please make the preference change per secret-bearing call site instead. - The same subscription flow still reads OpenAI audio credentials through
resolve_openai_audio_api_key()(tools/tool_backend_helpers.py:141-146) and Modal tokens throughhas_direct_modal_credentials()(:90-99), bothos.environ-first. Those paths need an explicit scope decision.
Suggested changes
- Rebase the implementation concept on the existing scope-aware helper at
hermes_cli/config.py:7743-7766; merged #55528 already added it. - Add stale-shell-versus-dotenv coverage for each adjusted path.
This is an automated hermes-sweeper review.
| # stale shell-exported value would mask a freshly rotated .env key | ||
| # (issue #20591). Aliased to keep call sites unchanged. | ||
| from hermes_cli.config import ( # noqa: F401 | ||
| get_env_value_prefer_dotenv as get_env_value, |
Contributor
There was a problem hiding this comment.
This module-wide alias also changes non-secret configuration reads such as HERMES_LOCAL_STT_COMMAND, FIRECRAWL_API_URL, and SEARXNG_URL. Please use the prefer-dotenv helper only at the secret credential call sites rather than changing every get_env_value call in the module.
teknium1
added
sweeper:risk-security-boundary
teknium1
pushed a commit
that referenced
this pull request
Jul 28, 2026
…cope
`resolve_openai_audio_api_key()` reads the key that authenticates the audio
client straight from the process environment:
return (
os.getenv("VOICE_TOOLS_OPENAI_KEY", "")
or os.getenv("OPENAI_API_KEY", "")
).strip()
That value is not advisory. It flows through
`_resolve_openai_audio_client_config()` into `OpenAIClient(api_key=...)` for
TTS, and through `transcription_tools` for voice-note STT — both on the
per-turn tool path, inside the profile secret scope the gateway installs.
`agent/vertex_adapter` states the contract this breaks:
in a multiplex gateway serving several profiles from one process,
os.environ reflects whichever profile's .env happened to be loaded at
boot, not the profile the current turn belongs to. Reading it directly
here would let one profile mint tokens from — and get billed against —
a different profile's service-account file.
Reproduced with the real resolver, multiplexing on and profile A's scope
installed:
scope-aware get_secret -> sk-PROFILE-A-key
voice/STT resolver -> sk-PROFILE-B-key
So profile A's spoken reply and its users' voice notes are sent to OpenAI on
profile B's account, and billed there.
Route both reads through `agent.secret_scope.get_secret`, the same fix already
merged for the WeChat send path (#59662) and pending for QQ (#60420) — neither
covers the audio credential family. Under multiplexing the scope stays
authoritative, so a scope miss now yields no key instead of borrowing another
profile's; with multiplexing off `get_secret` falls through to `os.environ`
exactly as before, so single-profile deployments are untouched. The
VOICE_TOOLS_OPENAI_KEY > OPENAI_API_KEY precedence is unchanged.
Deliberately narrow: `fal_key_is_configured()` and
`has_direct_modal_credentials()` in this file are presence checks, not
authentication, and the former is already being reworked in open PR #20929.
tests/tools/test_tool_backend_helpers.py: the scope wins over another
profile's `os.environ`; a scope miss does not borrow another profile's key;
voice-key precedence holds inside a scope; and a control proves the
single-profile path still reads `os.environ`. The three isolation tests fail
on main; the control passes there. 320 passed across the helper, secret-scope,
and consumer suites (the fluctuating voice_mode/voice_cli failures are
pre-existing PulseAudio/ordering artifacts — the differing test passes 3/3 in
isolation on both main and this branch).
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…cope
`resolve_openai_audio_api_key()` reads the key that authenticates the audio
client straight from the process environment:
return (
os.getenv("VOICE_TOOLS_OPENAI_KEY", "")
or os.getenv("OPENAI_API_KEY", "")
).strip()
That value is not advisory. It flows through
`_resolve_openai_audio_client_config()` into `OpenAIClient(api_key=...)` for
TTS, and through `transcription_tools` for voice-note STT — both on the
per-turn tool path, inside the profile secret scope the gateway installs.
`agent/vertex_adapter` states the contract this breaks:
in a multiplex gateway serving several profiles from one process,
os.environ reflects whichever profile's .env happened to be loaded at
boot, not the profile the current turn belongs to. Reading it directly
here would let one profile mint tokens from — and get billed against —
a different profile's service-account file.
Reproduced with the real resolver, multiplexing on and profile A's scope
installed:
scope-aware get_secret -> sk-PROFILE-A-key
voice/STT resolver -> sk-PROFILE-B-key
So profile A's spoken reply and its users' voice notes are sent to OpenAI on
profile B's account, and billed there.
Route both reads through `agent.secret_scope.get_secret`, the same fix already
merged for the WeChat send path (NousResearch#59662) and pending for QQ (NousResearch#60420) — neither
covers the audio credential family. Under multiplexing the scope stays
authoritative, so a scope miss now yields no key instead of borrowing another
profile's; with multiplexing off `get_secret` falls through to `os.environ`
exactly as before, so single-profile deployments are untouched. The
VOICE_TOOLS_OPENAI_KEY > OPENAI_API_KEY precedence is unchanged.
Deliberately narrow: `fal_key_is_configured()` and
`has_direct_modal_credentials()` in this file are presence checks, not
authentication, and the former is already being reworked in open PR NousResearch#20929.
tests/tools/test_tool_backend_helpers.py: the scope wins over another
profile's `os.environ`; a scope miss does not borrow another profile's key;
voice-key precedence holds inside a scope; and a control proves the
single-profile path still reads `os.environ`. The three isolation tests fail
on main; the control passes there. 320 passed across the helper, secret-scope,
and consumer suites (the fluctuating voice_mode/voice_cli failures are
pre-existing PulseAudio/ordering artifacts — the differing test passes 3/3 in
isolation on both main and this branch).
prmartinow
pushed a commit
to prmartinow/hermes-agent
that referenced
this pull request
Aug 26, 2026
…cope
`resolve_openai_audio_api_key()` reads the key that authenticates the audio
client straight from the process environment:
return (
os.getenv("VOICE_TOOLS_OPENAI_KEY", "")
or os.getenv("OPENAI_API_KEY", "")
).strip()
That value is not advisory. It flows through
`_resolve_openai_audio_client_config()` into `OpenAIClient(api_key=...)` for
TTS, and through `transcription_tools` for voice-note STT — both on the
per-turn tool path, inside the profile secret scope the gateway installs.
`agent/vertex_adapter` states the contract this breaks:
in a multiplex gateway serving several profiles from one process,
os.environ reflects whichever profile's .env happened to be loaded at
boot, not the profile the current turn belongs to. Reading it directly
here would let one profile mint tokens from — and get billed against —
a different profile's service-account file.
Reproduced with the real resolver, multiplexing on and profile A's scope
installed:
scope-aware get_secret -> sk-PROFILE-A-key
voice/STT resolver -> sk-PROFILE-B-key
So profile A's spoken reply and its users' voice notes are sent to OpenAI on
profile B's account, and billed there.
Route both reads through `agent.secret_scope.get_secret`, the same fix already
merged for the WeChat send path (NousResearch#59662) and pending for QQ (NousResearch#60420) — neither
covers the audio credential family. Under multiplexing the scope stays
authoritative, so a scope miss now yields no key instead of borrowing another
profile's; with multiplexing off `get_secret` falls through to `os.environ`
exactly as before, so single-profile deployments are untouched. The
VOICE_TOOLS_OPENAI_KEY > OPENAI_API_KEY precedence is unchanged.
Deliberately narrow: `fal_key_is_configured()` and
`has_direct_modal_credentials()` in this file are presence checks, not
authentication, and the former is already being reworked in open PR NousResearch#20929.
tests/tools/test_tool_backend_helpers.py: the scope wins over another
profile's `os.environ`; a scope miss does not borrow another profile's key;
voice-key precedence holds inside a scope; and a control proves the
single-profile path still reads `os.environ`. The three isolation tests fail
on main; the control passes there. 320 passed across the helper, secret-scope,
and consumer suites (the fluctuating voice_mode/voice_cli failures are
pre-existing PulseAudio/ordering artifacts — the differing test passes 3/3 in
isolation on both main and this branch).
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
…cope
`resolve_openai_audio_api_key()` reads the key that authenticates the audio
client straight from the process environment:
return (
os.getenv("VOICE_TOOLS_OPENAI_KEY", "")
or os.getenv("OPENAI_API_KEY", "")
).strip()
That value is not advisory. It flows through
`_resolve_openai_audio_client_config()` into `OpenAIClient(api_key=...)` for
TTS, and through `transcription_tools` for voice-note STT — both on the
per-turn tool path, inside the profile secret scope the gateway installs.
`agent/vertex_adapter` states the contract this breaks:
in a multiplex gateway serving several profiles from one process,
os.environ reflects whichever profile's .env happened to be loaded at
boot, not the profile the current turn belongs to. Reading it directly
here would let one profile mint tokens from — and get billed against —
a different profile's service-account file.
Reproduced with the real resolver, multiplexing on and profile A's scope
installed:
scope-aware get_secret -> sk-PROFILE-A-key
voice/STT resolver -> sk-PROFILE-B-key
So profile A's spoken reply and its users' voice notes are sent to OpenAI on
profile B's account, and billed there.
Route both reads through `agent.secret_scope.get_secret`, the same fix already
merged for the WeChat send path (NousResearch#59662) and pending for QQ (NousResearch#60420) — neither
covers the audio credential family. Under multiplexing the scope stays
authoritative, so a scope miss now yields no key instead of borrowing another
profile's; with multiplexing off `get_secret` falls through to `os.environ`
exactly as before, so single-profile deployments are untouched. The
VOICE_TOOLS_OPENAI_KEY > OPENAI_API_KEY precedence is unchanged.
Deliberately narrow: `fal_key_is_configured()` and
`has_direct_modal_credentials()` in this file are presence checks, not
authentication, and the former is already being reworked in open PR NousResearch#20929.
tests/tools/test_tool_backend_helpers.py: the scope wins over another
profile's `os.environ`; a scope miss does not borrow another profile's key;
voice-key precedence holds inside a scope; and a control proves the
single-profile path still reads `os.environ`. The three isolation tests fail
on main; the control passes there. 320 passed across the helper, secret-scope,
and consumer suites (the fluctuating voice_mode/voice_cli failures are
pre-existing PulseAudio/ordering artifacts — the differing test passes 3/3 in
isolation on both main and this branch).
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.
fix(credentials): extend prefer-dotenv policy to remaining resolve paths
What
Extends the prefer-dotenv lookup policy from PR #20602 to the credential-bearing call sites that PR leaves untouched:
hermes_cli/runtime_provider.py::_resolve_azure_foundry_runtime— the Azure Foundry API key now reads viaget_env_value_prefer_dotenv("AZURE_FOUNDRY_API_KEY")so a freshly rotated key in~/.hermes/.envimmediately wins over a stale shell-exported value.hermes_cli/nous_subscription.py— the module-levelget_env_valueimport is aliased toget_env_value_prefer_dotenv. Every call site in that file is a credential probe (EXA, FIRECRAWL, PARALLEL, TAVILY, ELEVENLABS, BROWSER_USE, BROWSERBASE_API_KEY/PROJECT_ID, etc.) that gates subscription-feature availability, so the alias is the right unit of change — the 25 internal call sites stay textually identical.tools/tool_backend_helpers.py::fal_key_is_configured— switched toget_env_value_prefer_dotenv("FAL_KEY")for the same reason.Display-only paths (
hermes status,hermes doctor,hermes setup,hermes_cli/mcp_config.py) intentionally keep the oldget_env_valuesemantics so diagnostic output continues to reflect what the user's shell sees.Why
Issue #20591 — the credential pool reads stale
os.environvalues, so a freshly rotated.envkey gets shadowed by a value the user exported in.bashrc. PR #20602 fixed the centralhermes_cli/config.pyhelpers; this PR completes the migration on the credential-RESOLVE paths PR #20602 didn't reach.Without this follow-up, an Azure Foundry user who rotates their key in
~/.hermes/.envwould still hit "401 unauthorized" until they restart their shell, because_resolve_azure_foundry_runtimewould keep handing the stale shell-exported key to the SDK on every request. Same shape of bug for every Nous-subscription tool probe.Depends on
PR #20602 (
fix/20591-resolve-prefers-dotenv). That PR introducesget_env_value_prefer_dotenvinhermes_cli/config.pywith the same name and semantics this PR uses.This PR vendors a copy of the helper inline (with a
NOTE:comment marking it for reconciliation) so it is testable on its own. Rebase plan: once #20602 merges, drop the duplicate definition inhermes_cli/config.py, keep all call-site changes.Tests
New, self-contained:
tests/hermes_cli/test_runtime_provider_dotenv_priority.py(4 tests) — Azure Foundry resolver:.envwins over staleos.environ, env-only fallback, dotenv-only path, blank.envvalue falls back to env.tests/hermes_cli/test_nous_subscription_credential_probe.py(5 tests) — probes for web / browser / tts find dotenv-only credentials; "no credentials anywhere" yields all-False; module alias is pinned to the prefer-dotenv variant (regression guard against accidental revert during rebase).Existing regression suite still green:
ruff checkon the changedtests/hermes_cli/*files:All checks passed!Out of scope (intentional)
hermes_cli/setup.py,hermes_cli/status.py,hermes_cli/doctor.py,hermes_cli/mcp_config.py— these are diagnostic / display paths. Migrating them would makehermes statuslie about what the running shell would see. Left untouched on purpose.