Skip to content

fix(dashboard): suppress credential source on pool delete to prevent resurrection - #55219

Closed
kaishi00 wants to merge 1 commit into
NousResearch:mainfrom
kaishi00:fix/dashboard-credential-delete-resurrect
Closed

kaishi00 wants to merge 1 commit into
NousResearch:mainfrom
kaishi00:fix/dashboard-credential-delete-resurrect

Conversation

@kaishi00

Copy link
Copy Markdown
Contributor

Problem

Deleting a credential from the credential pool via the dashboard (System Settings → Credentials → Remove) shows a success toast "Credential removed", but the entry reappears on page refresh.

Root Cause

The dashboard DELETE endpoint calls pool.remove_index(index), which correctly removes the entry from auth.json. However, the next load_pool() call re-creates it via _seed_from_env() because the environment variable is still present in .env.

The CLI command hermes auth remove handles this correctly by calling suppress_credential_source(provider, removed.source) (auth_commands.py:475), which marks the source as suppressed so seeding functions skip it.

The dashboard endpoint was missing this call.

Fix

Add suppress_credential_source() after successful removal in the DELETE handler, matching the CLI behavior.

Testing

  • Before fix: delete credential → refresh → entry reappears
  • After fix: delete credential → refresh → entry stays gone

Fixes #55217

…resurrection

The dashboard DELETE /api/credentials/pool endpoint removed the entry
from auth.json but did not call suppress_credential_source(). The next
load_pool() call re-seeded the entry from the matching env var, making
deletes appear to silently fail (toast shows success, entry reappears
on refresh).

The CLI hermes auth remove command already handles this correctly
(auth_commands.py:475). This applies the same suppression to the
dashboard endpoint.

Fixes NousResearch#55217.
@alt-glitch alt-glitch added type/bug Something isn't working comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have labels Jun 29, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: LGTM

Clean fix — suppresses credential source on pool delete to prevent resurrection. The 10-line change is minimal and well-targeted.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the dashboard/CLI parity gap. Current main still has it: hermes_cli/web_server.py:11558 removes the pool entry and returns success at :11564 without suppression, while agent/credential_pool.py:2411 reloads environment sources and :2211-2227 re-upserts OpenRouter unless suppressed.

Problems

  • The new except Exception around suppress_credential_source() returns a successful delete even when persistence fails. That leaves the source unsuppressed and permits the exact refresh-time resurrection this change targets.
  • Please add a dashboard regression test. tests/hermes_cli/test_dashboard_admin_endpoints.py:134-156 currently exercises only a manual credential, not an env-seeded source followed by a fresh load_pool().

Suggested changes

  • Treat suppression failure as a failed DELETE response rather than logging and returning {\"ok\": true}.
  • Seed an env credential in an isolated HERMES_HOME, delete it through the endpoint, then assert a fresh pool load remains empty.

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 15, 2026
@kaishi00

Copy link
Copy Markdown
Contributor Author

Our issue #55217 has been resolved by #67429 (merged). This PR is now superseded — closing it out.

Thanks to @teknium1 for the fix. The find_removal_step() dispatch approach is cleaner than our direct suppress_credential_source() call.

@kaishi00 kaishi00 closed this Jul 19, 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/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Dashboard credential pool delete succeeds but entries reappear on refresh

4 participants