Skip to content

fix(web): clear stale api-alias credential on provider switch in main-model assignment - #49551

Closed
ly-wang19 wants to merge 1 commit into
NousResearch:mainfrom
ly-wang19:fix/web-clear-stale-api-alias-on-provider-switch
Closed

fix(web): clear stale api-alias credential on provider switch in main-model assignment#49551
ly-wang19 wants to merge 1 commit into
NousResearch:mainfrom
ly-wang19:fix/web-clear-stale-api-alias-on-provider-switch

Conversation

@ly-wang19

Copy link
Copy Markdown
Contributor

Summary

Follow-up to c253b0738 ("clear stale endpoint credentials across switches").

That commit added clear_model_endpoint_credentials() to scrub an old endpoint's inline secret — api_key, the legacy api alias, and api_mode — when the web UI switches the main model to a different provider. But _apply_main_model_assignment (hermes_cli/web_server.py) gates the key-scrub path on model_cfg["api_key"] being truthy:

if api_key.strip():
    model_cfg["api_key"] = api_key.strip()
    model_cfg.pop("api", None)
elif model_cfg.get("api_key") and new_provider != prev_provider:   # ← api_key only
    clear_model_endpoint_credentials(model_cfg, clear_api_mode=False)
if new_provider != prev_provider:
    clear_model_endpoint_credentials(model_cfg, clear_api_key=False)  # clears api_mode only

So when the stale secret lives only under the legacy api alias (no api_key), a provider switch fails that elif, and the second call uses clear_api_key=False — the secret under api is never cleared and survives in config.yaml.

This matters because model.api is a live credential read path — _resolve_openrouter_runtime() reads for k in ("api_key", "api") and feeds it as the api_key candidate on the custom/auto branch. So endpoint A's plaintext secret persists and becomes the active credential the next time resolution flows through a custom endpoint — the exact "secrets in config.yaml… contaminate later custom resolution" harm clear_model_endpoint_credentials's own docstring names.

It's also an inconsistency: the sibling persistence sites (the two gateway model-picker paths and the aux-slot path) call the helper unconditionally on a non-custom switch and already scrub api; only this caller had the api_key-only gate.

Reproduced:

_apply_main_model_assignment(
    {"provider": "custom", "api": "sk-secret-A", "base_url": "https://endpoint-A/v1"},
    "openrouter", "m",
)
# before: {"provider": "openrouter", "api": "sk-secret-A", "base_url": "", "default": "m"}  ← stale secret survives
# after:  {"provider": "openrouter", "base_url": "", "default": "m"}

Fix

Widen the guard to fire on either field:

elif (model_cfg.get("api_key") or model_cfg.get("api")) and new_provider != prev_provider:

The same-provider re-pick (preserves the key) and explicit-new-key paths are unchanged.

Tests

Added the legacy-api-alias case to test_apply_main_model_assignment_base_url_and_context_reconcile in tests/hermes_cli/test_web_server.py (asserts both api and api_key are absent after a provider switch). Fails without the fix (the api alias survives); passes with it.

…in-model assignment

c253b07 added clear_model_endpoint_credentials() to scrub an old endpoint's
inline secret (api_key, the legacy `api` alias, api_mode) when the web UI
switches the main model to a different provider. But _apply_main_model_assignment
gates the key-scrub path on model_cfg["api_key"] being truthy, so when the stale
secret lives only under the legacy `api` alias (no api_key), a provider switch
never clears it — the secret survives in config.yaml.

model.api is a live credential read path (_resolve_openrouter_runtime reads
`for k in ("api_key", "api")`), so the old endpoint's key contaminates a later
custom resolution — the exact harm clear_model_endpoint_credentials documents.
The sibling persistence sites (the gateway model-picker paths and the aux-slot
path) call the helper unconditionally on a non-custom switch and already scrub
`api`; only this caller had the api_key-only gate.

Widen the guard to fire on either field. The same-provider re-pick and
explicit-new-key paths are unchanged. Adds the api-alias case to the assignment
test (it fails without the fix).
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools labels Jun 20, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The premise holds on current main: hermes_cli/web_server.py:1137 clears credentials only for a populated api_key, while hermes_cli/config.py:4571-4573 defines credential cleanup to remove both api_key and the legacy api alias. The runtime still reads both aliases at hermes_cli/runtime_provider.py:1059-1063.

The proposed guard makes the existing cleanup path cover the missing alias-only state without changing same-provider or explicit-key behavior. The added regression case directly exercises that state.

Automated hermes-sweeper review.

@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-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
teknium1 added a commit that referenced this pull request Jul 20, 2026
teknium1 added a commit that referenced this pull request Jul 20, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #68125 with your commit's authorship preserved in git log (rebase-merge). Your fix was picked up in a post-merge sweep of the credential-hygiene cluster (#67806/#67797/#68074) — the premise held exactly as you described: the switch-clears-the-key path never fired when the stale secret lived under the legacy api alias only. Thanks for the focused fix and the test.

@teknium1 teknium1 closed this Jul 20, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
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 comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

3 participants