Skip to content

fix(dashboard): let an explicit api_key win over the provider entry's stored one - #67806

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
Frowtek:fix/model-set-explicit-api-key-precedence
Jul 20, 2026
Merged

fix(dashboard): let an explicit api_key win over the provider entry's stored one#67806
teknium1 merged 1 commit into
NousResearch:mainfrom
Frowtek:fix/model-set-explicit-api-key-precedence

Conversation

@Frowtek

@Frowtek Frowtek commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

POST /api/model/set accepts an api_key and threads it into
_apply_main_model_assignment. The custom-endpoint work then added a
provider-entry fallback right after it — but unconditionally:

if not base_url and provider_entry.get("base_url"):
    base_url = provider_entry["base_url"]              # explicit wins
model_cfg = _apply_main_model_assignment(..., base_url, api_key)
if provider_entry.get("api_key"):
    model_cfg["api_key"] = provider_entry["api_key"]   # explicit LOSES

The two lines disagree about precedence. base_url fills only a gap; api_key
overwrites whatever the caller sent.

So rotating a key through this endpoint returns 200 and silently keeps the old
one:

request api_key : sk-NEW-ROTATED-KEY
stored  api_key : sk-STORED-OLD-KEY

That matters beyond the write itself: model.api_key outranks the environment at
client construction, so the stale key keeps authenticating and shadows anything the
operator configures next — the persistent-401 shape credential_lifecycle.py
documents as #62269.

A regression, not long-standing. Against 3d9789357^ the same request stores
sk-NEW-ROTATED-KEY.

Related Issue

Fixes #

Type of Change

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

Changes Made

  • hermes_cli/web_server.py — gate the fallback on not api_key, matching the
    base_url line directly above it.
  • tests/hermes_cli/test_web_server.py — explicit key wins; omitted key still
    adopts the entry's key (and the base_url fill still works).

Testing

test_set_model_main_honors_an_explicitly_supplied_api_key fails on main.
test_set_model_main_falls_back_to_the_provider_entry_key passes both ways — that
one exists so the fix can't regress the feature's intent in the other direction.

Web-server / model-switch / credential-lifecycle / config suites baseline-compared
against a clean origin/main worktree: identical 16 pre-existing failures,
627 → 629 passed.

Checklist

  • Bug is reproducible on main and covered by a failing-before/passing-after test
  • No regressions in the surrounding suite (baseline-compared against origin/main)
  • Change is scoped to the defect — no unrelated refactoring
  • Tested on Ubuntu 24.04

… stored one

POST /api/model/set accepts an api_key and threads it into
_apply_main_model_assignment. The custom-endpoint work then added a
provider-entry fallback right after it — but unconditionally:

    if not base_url and provider_entry.get("base_url"):
        base_url = provider_entry["base_url"]          # explicit wins
    model_cfg = _apply_main_model_assignment(..., base_url, api_key)
    if provider_entry.get("api_key"):
        model_cfg["api_key"] = provider_entry["api_key"]   # explicit LOSES

The two lines disagree about precedence. base_url fills only a gap; api_key
overwrites whatever the caller sent.

So rotating a key through this endpoint returns 200 and silently keeps the
old one:

    request api_key : sk-NEW-ROTATED-KEY
    stored  api_key : sk-STORED-OLD-KEY

That matters beyond the write itself: model.api_key outranks the environment
at client construction, so the stale key keeps authenticating and shadows
anything the operator configures next — the persistent-401 shape
credential_lifecycle.py documents as NousResearch#62269.

A regression, not long-standing. Against 3d97893^ the same request stores
sk-NEW-ROTATED-KEY.

Gate the fallback on `not api_key`, matching the base_url line directly above
it. Switching to a configured provider with no key in the request still adopts
the entry's key — pinned by its own test so the feature's intent doesn't
regress in the other direction.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #67802: both repair dashboard custom-provider persistence, but this fixes explicit API-key precedence in /api/model/set; #67802 preserves unmanaged fields during a custom-endpoint edit.

@teknium1

Copy link
Copy Markdown
Contributor

Infographic — credential-hygiene cluster (merged)

Covers the three directly-merged PRs from this cluster: #67806 (explicit api_key precedence), #67640 (master credential stores never mountable), #67635 (live transcript redaction). The remaining two (#67802, #67797) landed via salvage PR #68074.

credential-hygiene-triple-drop

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/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants