Skip to content

[add] dockercmd --reset-password - #152

Merged
malickyeu merged 5 commits into
mainfrom
feat/reset-password
Aug 6, 2026
Merged

[add] dockercmd --reset-password#152
malickyeu merged 5 commits into
mainfrom
feat/reset-password

Conversation

@malickyeu

Copy link
Copy Markdown
Contributor

Summary

dockercmd --reset-password <user> — a way back into an instance whose admin password is lost.

An admin can reset someone else's password from the UI. The last admin locking themselves out had nobody to ask, and this app deliberately gives nobody a way to reset another account's second factor. That state was terminal. It came up because the maintainer locked themselves out of their own instance twice while running the screenshot generator.

Why this is safe to offer

It grants nothing new. The token signing secret (jwt_secret) is a row in the same SQLite database this command opens, so anyone who can run it can already mint themselves a valid admin session directly — no password, no second factor. The capability is the filesystem, not this command; all this does is make the legitimate use of it survivable. (docs/deployment.md already says the same thing about backups, for the same reason.)

That argument is what the design has to protect, so:

  • Offline only, against the data dir. Never an HTTP endpoint — that would break the argument entirely.
  • The password is read from the terminal, never taken as an argument. An argument lands in shell history and in /proc/<pid>/cmdline, readable by any local user on most systems, and that would be a new leak. Piped input is refused for the same reason.
  • Every session for the account is ended — rows deleted and the epoch bumped, mirroring auth.Service.SetPassword. A reset that leaves a stolen session alive is the opposite of what someone reaching for this needs.
  • The second factor is left alone. Whoever holds the files can bypass it anyway, but the command should not do it for them, so "nobody resets another account's second factor" stays true and stays easy to state.
  • Audited as auth.password.reset, like any other privileged action.
  • LDAP accounts are refused: their password belongs to the directory, and writing a local one would store a credential the login path never consults.

auth.MinPasswordLength replaces the three literal 10s, so the offline path cannot quietly disagree with the online one.

Tests

Seven, in cmd/dockercmd/resetpassword_test.go: sessions ended, epoch bumped, credential replaced (and the old one refused), second factor untouched, audited, LDAP refused, unknown account refused, and flag parsing including -- and a following flag.

Mutation-verified. One of them passed for the wrong reason first: the fixture created a session with no ExpiresAt, and ListSessions filters on expires_at > now, so it was never listed and the assertion held with the deletion removed. The fixture now gives the session a real expiry and asserts it is live before the reset.

Type of change

  • Bug fix
  • New feature
  • Docs only
  • Refactor / chore

Checklist

  • go test -short ./... and go vet ./... pass
  • gofmt gate is clean
  • Frontend type-checks — no UI change
  • Rebuilt web/dist — nothing under web/src changed
  • Added tests for the change
  • Updated docs/ and CHANGELOG.md

Notes for reviewers

The claim worth attacking is the safety argument itself. If jwt_secret were ever moved out of the data dir — to an env var, a KMS, a file the app user cannot read — this command would become a genuine privilege escalation rather than a convenience, and would need rethinking. Worth a look at whether anything in flight moves it.

docs/audit.md gains an auth.password.reset row only after #149 merges; that file's action table lives there and does not exist on main yet. I will add it once #149 lands rather than conflict with it.

The last admin locking themselves out had no way back: an admin can reset someone
else's password from the UI, and nobody can reset another account's second factor
by design. That state was terminal.

Offering it costs nothing. The token signing secret is a row in the same database
this opens, so anyone who can run it could already mint an admin session directly
— the capability is the filesystem, not the command. So it stays offline against
the data dir, reads the password from the terminal rather than an argument (which
would land in shell history and /proc/<pid>/cmdline, and THAT would be new), ends
every session for the account as the UI path does, and leaves the second factor
alone so "nobody resets another account's second factor" stays true.

MinPasswordLength replaces the three literal 10s, so the offline path cannot
quietly disagree with the online one.

The session test needed a second look: the fixture's session had no expiry, and
ListSessions filters on expires_at > now — so it passed with the deletion removed.
It now asserts the fixture has a live session before resetting.
Copilot AI lite review requested due to automatic review settings August 6, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

"Offline" reads as "stop the service first", which is not true and would make
the recovery path sound heavier than it is. Verified against a running instance:
the active session answers 401 the moment the reset lands, the old password is
refused and the new one works, with no restart — the server re-reads both the
password and the session epoch per request.
Copilot AI review requested due to automatic review settings August 6, 2026 15:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

It refused "ldap" by name while the passwordless path, changed earlier today for
exactly this reason, allowlists local accounts instead. A denylist accepts the
next auth source somebody adds — the rule is "accounts whose password this app
owns", not "not LDAP".

Tests cover oidc and saml, and that both spellings of a local account ('' and
'local') are still resettable, so the guard cannot quietly become a ban.
Copilot AI review requested due to automatic review settings August 6, 2026 15:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 6, 2026 16:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The safety argument survived review — the signing secret really is a row in the
same database, so the command grants nothing filesystem access did not. What did
not survive was two of its own tests and two operational hazards.

Two tests guarded nothing. "Leaves the second factor alone" counted factors, so a
SWAP — delete the owner's, insert your own — passed it; it now compares identity.
The password floor lived only in the prompt, which no test touches and which the
test helper bypasses entirely, so it moved to where the write happens.

Two hazards, both on the packaged install this exists for. The prompt now comes
BEFORE the database is opened: a Ctrl-C at it left root-owned -wal/-shm files in
the service user's directory, after which the service could not write its own
database — a failed recovery must not be worse than none. And it refuses to
create a database rather than answering "no account called admin" from a
directory that was empty a moment ago, which is what the documented
"sudo dockercmd --reset-password admin" did, because standalone actions do not
read the config file the package ships.

Also honest now about what it does not do: API and MCP tokens are not sessions
and are not revoked, and the second-factor promise is conditional on the
localhost 2FA exemption. Added to --help and the man page, and ErrWeakPassword
derives its number from MinPasswordLength instead of repeating it.
@malickyeu
malickyeu force-pushed the feat/reset-password branch from aec3175 to 117f021 Compare August 6, 2026 16:04
@malickyeu
malickyeu merged commit c1377b8 into main Aug 6, 2026
2 of 4 checks passed
@malickyeu
malickyeu deleted the feat/reset-password branch August 6, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants