Harden reset-admin-password: invalidate other sessions, audit-log it, and test it - #32
Merged
Merged
Conversation
… and test it The existing `reset-admin-password` CLI (a local/root-only recovery tool for a forgotten dashboard password) hashed and stored the new password directly, duplicating logic from POST /auth/password rather than reusing it - and unlike that endpoint, it never bumped webui.session_epoch, so a reset left any other existing dashboard session (possibly the one an attacker has) valid for up to its 8-hour idle timeout. It also wrote no audit entry, so a local reset was invisible in System Logs, and had zero test coverage (no Dispatch.Service.Tests project exists to test Program.cs directly). Extracted the shared logic into AuthEndpoints.SetPasswordAsync: validates, hashes (bcrypt-12), stores the hash, and bumps the session epoch when replacing an existing password - both the HTTP endpoint and the CLI now call this one method, so they can no longer drift apart. The CLI also now writes an audit entry (actor "local-cli", distinct from "admin") so a local reset shows up in System Logs like any other admin-password change. Added AdminPasswordResetTests against SetPasswordAsync directly (mirroring how migrate-database's logic is tested via DatabaseMigrator rather than through Program.cs's CLI entry point): weak passwords are rejected and persist nothing, first-run does not bump the epoch (no prior sessions to revoke), and resetting an existing password does bump it. Documented the command for the first time since it was dropped from the old README when docs moved to the Astro site: a "Forgot the admin password?" section in dispatch-docs' security.md, and a new §17.3 subsection + §1.1 delta bullet in docs/SPEC.md (previously undocumented there entirely). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…for real Ran reset-admin-password against a real running service (not just unit tests against the extracted logic) to check the claim in the previous commit that it "invalidates every other session" - it didn't. A live cookie stayed authenticated:true after the reset, even though the DB write and epoch bump were both correct. Root cause: OnValidatePrincipal (ServiceCollectionExtensions.cs) checked the session epoch against ConfigCache, an in-memory snapshot that is only refreshed within the same request that itself wrote a config change (every /config/* endpoint calls cache.LoadAsync after writing - /auth/password never did, and couldn't have fixed this alone anyway). reset-admin-password runs as a SEPARATE OS process from the running service, so there was never any way for it to reach into that process's ConfigCache - the bump was correct in the database and simply invisible to the already-running server. This is a pre-existing bug, not something the previous commit introduced: the dashboard's own in-app password change had the exact same gap, just harder to notice since nothing exercised it end to end before. Fixed by reading the session epoch live from IConfigRepository in OnValidatePrincipal instead of ConfigCache - the one check on the hot path that must reflect a change no matter which process made it. Verified again against a real running service: old cookie now correctly flips to authenticated:false immediately after a CLI reset, old password gets 401, new password logs in. Also fixed two smaller things the live test surfaced: the CLI's audit message and final console line both unconditionally said "reset" even on a genuine first-run password set (no prior password, nothing to invalidate) - now they match the same hadPassword distinction /auth/password already made. Added SessionEpochInvalidationTests: a real HTTP request through the actual cookie-auth pipeline (not just the extracted SetPasswordAsync logic, which already had its own tests and would not have caught this) confirming a cookie issued before an externally-written epoch bump is rejected after it - using its own HttpClient and restoring config state in `finally` so it doesn't leak into the other seven test classes sharing this WebTestHost fixture. Ran 5x locally to check for order-dependence; stable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
chrismuench
added a commit
that referenced
this pull request
Aug 16, 2026
Version -> 0.7.2 (Directory.Build.props): the reset-admin-password hardening (session invalidation fix, audit logging, tests, docs) merged in #32. CHANGELOG: documented the fix and the newly-documented recovery command. Co-authored-by: Chris Muench <chris@MacBook-Air-3.local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Summary
Investigated a bug report ("we need a way for an admin to reset the local admin password if they forget it, if they have local admin or sudo on the box"). This already existed (
Dispatch.Service reset-admin-password, added months ago) - the actual gaps were:Dispatch.Service.Testsproject exists, and the reset logic was inline inProgram.cs.docs/SPEC.mdat all.A real bug, found by actually running this against a live service
First commit here extracted the shared password-set logic (
AuthEndpoints.SetPasswordAsync) and had the CLI bumpwebui.session_epochlike/auth/passworddoes, with unit tests against that logic. All green.Then I actually ran the built CLI against a real running service instead of stopping at unit tests - started the service, logged in, ran
reset-admin-passwordfrom a separate process while it was running, and checked the old cookie. It was stillauthenticated:true. The "fix" didn't work.Root cause:
OnValidatePrincipal(ServiceCollectionExtensions.cs) checked the session epoch againstConfigCache, an in-memory snapshot only refreshed within the same request that itself wrote a change (every/config/*endpoint callscache.LoadAsyncafter writing its own change).reset-admin-passwordruns as a separate OS process - there's no way for it to touch the running service's in-memory cache. The database write was correct; the running process just never saw it. This is a pre-existing bug that also affects the dashboard's own in-app password change (same code path), not something the first commit introduced - it was just never exercised end-to-end before.Fixed by reading the epoch live from
IConfigRepositoryinOnValidatePrincipalinstead ofConfigCache. Re-ran the same live test: old cookie now correctly flips toauthenticated:falseimmediately, old password gets 401, new password works.Changes
AuthEndpoints.SetPasswordAsync: shared validate → hash → store → bump-epoch-if-replacing logic for bothPOST /auth/passwordand the CLI.Program.cs'sreset-admin-password: calls the shared method, writes an audit entry (actor: local-cli), and now correctly distinguishes "first-run set" vs "reset" in both the audit message and the console output (the live test caught this being wrong too - it said "reset" even on a fresh install with no prior password).ServiceCollectionExtensions.cs:OnValidatePrincipalreads the session epoch live, not fromConfigCache- the actual fix.AdminPasswordResetTests.cs: unit tests againstSetPasswordAsyncdirectly.SessionEpochInvalidationTests.cs: an integration test through the real cookie-auth pipeline (a real HTTP request, not just the extracted logic) confirming a cookie issued before an externally-written epoch bump gets rejected after it. Uses its ownHttpClientand restores config state infinallyso it doesn't leak into the other seven test classes sharing theWebTestHostfixture (ran 5x locally to check for order-dependence).docs/SPEC.md: new §17.3 "Password recovery" subsection + a §1.1 delta bullet.dispatch-docs'security.md(separate repo, pushed already): a "Forgot the admin password?" section with the per-platform commands.Test plan
dotnet build Dispatch.slnx- cleandotnet test tests/Dispatch.Web.Tests- 105/105 pass, run 5x to check for flakinessdotnet test tests/Dispatch.Core.Tests- 116/116 pass (unaffected)ServiceCollectionExtensionsfix (reproducing the bug) and after (confirming it)no-em-dashlint)🤖 Generated with Claude Code