fix(join): stop rotating a returning member's webmail password - #104
Merged
Conversation
handleJoin called setWebmailPassword unconditionally, so every visit to join@ minted a fresh Roundcube password -- including for members who registered and verified long ago. join@ is the address people remember, so returning members type it out of habit. Doing so silently invalidated their webmail login: the replacement password scrolled past once in the join output, the old one was already dead, and nothing in the session said a credential had changed. The failure surfaces later, as "my password stopped working", with no way to connect it back to having typed join@ -- and it reads like a compromised account, which is an alarming thing to hand someone over a no-op visit. Mint the password only while actually onboarding: a new key registering, or an account that completes email verification in this session. A returning member is told the password is unchanged and pointed at passwd@, which already sets one password across git, mail and chat and is key-gated, so it doubles as the forgot-password path. Also replace the bare webmail URL line shown when no password is minted with the full url/login block, so the address and where to get credentials are always visible, and distinguish "unchanged" (returning) from "not set yet" (onboarding, but Mailu was unreachable). Verified by inspection only: this box has no Go toolchain (repo needs 1.26), so the build is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
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.
The bug
handleJoincallssetWebmailPasswordunconditionally (cmd/agentbbs/main.go:795), so every visit tojoin@mints a fresh Roundcube password — including for members who registered and verified long ago.Tracing an existing, verified member through
handleJoin:UserByFingerprint→ found, existing accountif !found→ skipped, no re-registrationif !u.EmailVerified→ skipped, already verifiedsetWebmailPassword(u)— runs anywayjoin@is the address people remember, so returning members type it out of habit. That silently invalidates their webmail login: the replacement password scrolls past once in the join output, the old one is already dead, and nothing in the session says a credential changed.The failure surfaces later as "my password stopped working", with nothing connecting it back to having typed
join@. It also reads like a compromised mailbox — an alarming conclusion to hand someone over what was a no-op visit.This was found by diagnosing exactly that: a member locked out of webmail who assumed the server had been breached.
The fix
Mint a webmail password only while actually onboarding — a new key registering, or an account completing email verification in this session:
A returning member now gets "You're already a member — nothing changed", is told the password is unchanged, and is pointed at
ssh passwd@— which already sets one password across git, mail and chat, and is key-gated, so it doubles as the forgot-password path.Rotating a live credential should never be a side effect of saying hello.
Also
The old
else if a.webmailURL != ""branch appended a bare• webmail <url>line. Replaced with the full url/login block so the address and where to get credentials are always visible, and the two no-password cases are now distinguished:unchanged — forgot it? ssh passwd@— returning member, password untouchednot set yet — set one: ssh passwd@— onboarding, but Mailu was unreachable and minting failedPreviously both collapsed into the same bare URL line.
Verification
Inspection only — I could not build this. There's no Go toolchain on the box I'm working from (repo needs Go 1.26), so compilation is left to CI, which runs
go build/go testonpull_request. Please confirm CI is green before merging.Reviewed by hand:
onboarding,webmailPWandleadare all used (no unused-variable errors); the expression-lessswitchis exhaustive and falls through to the original no-op whenwebmailURLis empty.No test added —
handleJoinhas no existing test coverage and requires anssh.Session; the branch logic isn't reachable without extracting it first. Worth a follow-up.🤖 Generated with Claude Code