Prevent token pollution during sync re-auth by wesm · Pull Request #203 · wesm/msgvault

wesm · 2026-03-15T19:02:20Z

Summary

No browser auto-launch during sync re-auth: When sync needs to re-authorize an expired/revoked token, it prints the OAuth URL with clear account context instead of silently opening a browser. This prevents accidentally selecting the wrong Google account when multiple accounts need re-auth.
login_hint parameter: The OAuth URL now includes login_hint=<email> so Google pre-selects the correct account in the chooser.
Post-auth email validation: After every authorization, the token is validated against the Gmail profile API before being saved. Wrong-account tokens are rejected with a clear error. Validation failures (timeout, API errors) also prevent saving, preserving the existing token.
Gmail alias handling: Validation accepts legitimate aliases (case differences, dot-insensitive gmail.com, googlemail.com ↔ gmail.com equivalence) while still rejecting different accounts.

Test plan

Existing TestGetTokenSourceWithReauth tests updated and passing — now assert AuthorizeManual is called (not Authorize) for the reauth path
New TestSameGoogleAccount and TestNormalizeGmailAddress tests for alias matching
Full test suite passes
Manual test: msgvault sync with expired token shows URL + account context without opening browser

🤖 Generated with Claude Code

Three changes to prevent OAuth tokens being stored for the wrong account when sync triggers re-authorization for multiple accounts: 1. Don't auto-launch browser during sync re-auth. Instead, print the authorization URL with clear account context so the user knows exactly which account to select. (AuthorizeManual) 2. Add login_hint parameter to the OAuth URL so Google pre-selects the correct account in the chooser. 3. Validate token email after authorization by calling the Gmail profile API. If the authorized account doesn't match the expected email, the token is deleted and an actionable error is returned. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Address two review findings: 1. Validate the token *before* persisting it to disk. If validation fails for any reason (network error, API error, email mismatch), the token is never saved, so an existing valid token is never overwritten with an unverified one. 2. Wrap the Gmail profile validation call in a 10s context timeout to prevent indefinite hangs if the API stalls after the browser callback completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…nual auth Finding 1: Don't delete the old token before re-auth succeeds. The authorize method now validates and saves atomically — if validation fails (timeout, API error, mismatch), the existing token is untouched. Removed DeleteToken from the tokenReauthorizer interface and from getTokenSourceWithReauth since it's no longer needed there. Finding 2: Accept Gmail aliases and googlemail.com addresses. Renamed validateTokenEmail to resolveTokenEmail which returns the canonical email from the profile API. Adds sameGoogleAccount() which handles case-insensitive match, dot-insensitive gmail.com, and googlemail.com ↔ gmail.com equivalence. The token is saved under the canonical address. Finding 3: Track Authorize and AuthorizeManual calls separately in the mock. Tests now assert that getTokenSourceWithReauth specifically calls AuthorizeManual (not Authorize) for the invalid_grant reauth path, so a regression to browser auto-launch would fail the test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The previous commit saved tokens under the canonical email returned by the Gmail profile API. This broke token lookup: the rest of the app (HasToken, TokenSource, source records) keys on the user-supplied identifier. If the canonical address differed (case, dots, googlemail), the token file would be unreachable. Fix: always save under the original identifier after validation confirms the account matches. sameGoogleAccount() still rejects wrong-account selection — it just no longer renames the file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T19:08:05Z

roborev: Combined Review (`fcb77e4`)

Summary Verdict: The PR successfully introduces token validation and manual re-auth, but contains medium-severity issues related to email alias handling and address normalization.

Medium

Alias-aware auth breaks verify lookups
The new
auth flow explicitly accepts Gmail aliases and saves the token under the original identifier in internal/oauth/oauth.go, but verify still looks up the source by Gmail’s canonical profile address in cmd/msgvault/cmd/verify.go#L88. A flow like add-account first.last@gmail.com can now succeed, yet msgvault verify first.last@gmail.com will fetch firstlast@gmail.com from Gmail and report "account not found" because the DB row is keyed by the original identifier.
- Suggested fix: Use the
  requested CLI identifier for source lookup in verify, or add a canonical-to-identifier mapping and normalize all source lookups consistently.
Plus-addressing not stripped during normalization
The normalization logic in internal/oauth/oauth.go:303 (normalizeGmailAddress) does
not strip plus-addressing (e.g., user+alias@gmail.com). If a user registered the account in the CLI using a plus-alias, authorization will fail because the Gmail profile API returns the base canonical address (user@gmail.com).
- Suggested fix: Strip the
  + and any subsequent characters from the local part before comparison: if plus := strings.Index(local, "+"); plus >= 0 { local = local[:plus] }.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Verifies that when the canonical Gmail address differs from the user-supplied identifier (dotted alias, case, googlemail.com), the token is saved under the original identifier — not the canonical one. This is the key invariant that prevents breaking HasToken/TokenSource lookups elsewhere in the app. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T19:13:00Z

roborev: Combined Review (`0bbd388`)

The changes successfully harden the OAuth re-authorization flow and token validation, but introduce a functional regression regarding email alias and plus-addressing support
.

Medium

File: internal/oauth/oauth.go
- Finding: The alias-matching logic (resolveTokenEmail / normalizeGmailAddress) hard-rejects any
  authorized account whose profile email is not an exact match, a dotted gmail.com equivalent, or a googlemail.com equivalent. The Gmail API returns the base canonical address, which means existing accounts configured under other valid aliases (such as user+tag@gmail.com) or non-primary
  Workspace aliases will incorrectly fail add-account or re-auth with a token mismatch error. This is a functional regression.
- Suggested Fix: Update the normalization logic to handle plus-addressing by stripping the + character and any trailing characters from the local part before comparing (e.g
  ., if idx := strings.Index(local, "+"); idx >= 0 { local = local[:idx] }). Alternatively, keep using the original identifier while storing a verified canonical address separately, rather than treating every non-primary address as the wrong account.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

The previous regression test manually called sameGoogleAccount() and saveToken(inputEmail, ...), so it would still pass even if authorize() switched back to saving under canonicalEmail. Fix: add a profileURL field to Manager (defaults to the real Gmail endpoint, overridden in tests). The test now stands up an httptest server returning a different canonical email, calls resolveTokenEmail through it, and verifies the token is saved under the original identifier. Also adds TestResolveTokenEmail_RejectsMismatch to cover the wrong-account rejection path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T19:45:22Z

roborev: Combined Review (`fe7b2b5`)

Summary Verdict: The PR introduces a safer manual re-authorization flow with account validation, but contains one medium severity issue regarding
non-Gmail aliases that needs addressing.

Findings

Medium

internal/oauth/oauth.go: The new resolveTokenEmail / sameGoogleAccount validation logic only accepts exact
matches plus gmail.com/googlemail.com normalization. Accounts added under a Google Workspace alias or other secondary login address will now fail add-account and sync re-auth with a token mismatch error, even when the token belongs to the same mailbox.
- Suggested fix: If
  non-Gmail aliases cannot be proven automatically, fall back to an explicit interactive confirmation path instead of a hard reject. Add a regression test covering the scenario where the alias does not match the primary email on a non-gmail.com domain.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Review #9805 found TestTokenSavedUnderOriginalIdentifier called resolveTokenEmail + saveToken manually, so a regression in authorize() saving under the canonical email would go undetected. Fix: add browserFlowFn field to Manager for test injection, then rewrite both persistence-key and mismatch tests to call Authorize() directly. The mismatch test also verifies no token is persisted on rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Google Workspace accounts may return a primary address from the profile API that differs from the alias the user added. Since we can't verify Workspace aliases without admin API access, accept same-domain mismatches with a logged warning. Gmail/googlemail domains are excluded from this relaxation since their aliases are fully handled by normalizeGmailAddress. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Revert the auto-accept for same-domain Workspace mismatches — it allowed token pollution (bob@company.com saved under alice's identifier). Instead, reject all unverifiable mismatches and suggest re-adding with the primary address shown by the profile API. Also assert no token exists under the profile address on mismatch rejection (#9806). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T20:08:49Z

roborev: Combined Review (`37c4f58`)

Summary Verdict: The changes improve OAuth token handling by validating before saving, but introduce a token pollution risk for Workspace domains.

Medium

internal
/oauth/oauth.go (lines 326-349, 372-388)
resolveTokenEmail() accepts any non-Gmail address on the same domain as
a "possible Workspace alias". In practice, this means authorizing an account like alice@company.com while reauthing billing@company.com will be treated as valid, binding the stored token to the wrong mailbox. This weakens the new anti-token-pollution control into a domain-only check, allowing potential
syncing or deletion against unintended data.
Suggested fix: Reject non-exact Workspace mismatches by default. If alias support is required, require explicit user confirmation or stronger verification of the alias mapping rather than auto-accepting same-domain accounts.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

The error message no longer suggests `add-account` since that advice is wrong during re-auth (would orphan the existing source). Callers can provide context-appropriate remediation. Also add no-persist assertions to TestAuthorize_WorkspaceAliasMismatch for both the alias and primary addresses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

resolveTokenEmail now returns *TokenMismatchError with Expected and Actual fields instead of a formatted string. This lets each caller provide context-appropriate remediation: - add-account: suggests re-adding with the primary address - sync re-auth: wraps with neutral "re-authorize" message - deletions: wraps with neutral "authorize" message Tests verify the error type via errors.As and check both Expected and Actual fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

When --force re-auth hits a mismatch, suggesting add-account with the primary address would create a second source and orphan the existing one. Only show the add-account guidance for brand-new accounts where no source exists yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T20:36:15Z

roborev: Combined Review (`684cc98`)

Verdict: The OAuth validation improvements are
security-positive, but introduce a functional regression for Google Workspace aliases.

Medium

internal/oauth/oauth.go (in resolveTokenEmail / sameGoogleAccount): Non
-Gmail accounts fall back to exact-match-only validation. A legitimate Google Workspace sign-in via alias@company.com will be rejected if the Gmail profile API returns the primary address primary@company.com, even though the user selected the correct account. This is a real regression for existing alias
-based setups, making later sync re-auth impossible for those accounts.
Suggested fix: Do not hard-reject same-account Workspace alias cases; allow them with a warning and continue saving under the original identifier, or add an explicit override/migration path.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev-ci · 2026-03-15T20:40:00Z

roborev: Combined Review (`787033d`)

Verdict: The PR improves OAuth validation and re-authorization flows, but introduces medium-severity regressions related to
handling Gmail and Workspace aliases.

Medium Severity Findings

Alias support is incomplete and breaks verify for Gmail dot/googlemail aliases.
The new flow explicitly accepts Gmail aliases and saves the token under the original identifier, but verify still resolves the account by Gmail’s canonical profile address
. For example: adding first.last@gmail.com returns firstlast@gmail.com from Gmail, the source stays keyed as first.last@gmail.com, and verify reports the account as missing from the database.
Suggested fix: Normalize/store a canonical
account identifier consistently, or teach callers like verify to map canonical profile emails back to the original configured identifier. Add an end-to-end test for add-account + verify with a dot alias.
Refs: internal/oauth/oauth.go (in authorize () / sameGoogleAccount()), cmd/msgvault/cmd/verify.go (in GetSourceByIdentifier(profile.EmailAddress))
Workspace alias accounts can be stranded on re-auth.
For non-Gmail domains, sameGoogleAccount() only
allows exact-match addresses. During sync re-auth, AuthorizeManual() now validates against the profile’s primary email and fails closed on any alias mismatch. This prevents token pollution, but means an existing account configured with a valid Workspace alias can no longer recover once its token expires, even if the browser flow authentic
ates the right mailbox.
Suggested fix: Pick a consistent alias policy and enforce it end-to-end. Either reject alias identifiers up front in add-account, or accept same-account Workspace mismatches with explicit warning/migration logic and test the re-auth path.
*Refs
*: internal/oauth/oauth.go (in resolveTokenEmail() / sameGoogleAccount()), cmd/msgvault/cmd/root.go (in getTokenSourceWithReauth())

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

The add-account suggestion on mismatch should only appear when no Gmail source exists for the identifier. Using --force as a proxy missed cases where the token was deleted externally but the source still exists, and suppressed the hint for legitimate --force on brand-new accounts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GetSourcesByIdentifier returns all source types. A non-Gmail source (mbox, imap, apple-mail) sharing the same identifier would suppress the add-account hint even though no Gmail source exists. Filter to Gmail sources specifically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Two fixes for alias handling: 1. verify: look up source by user-supplied email, not canonical profile address. The source is keyed under the original identifier from add-account. 2. re-auth mismatch: show remove + re-add instructions with the primary address so Workspace alias accounts have a clear recovery path instead of a dead-end error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Covers three cases: no sources (hint shown), non-Gmail source only (hint shown), and Gmail source exists (hint suppressed). This logic regressed across multiple commits so it deserves explicit coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

1. verify: use findGmailSource() instead of GetSourceByIdentifier() to avoid counting the wrong archive when Gmail and mbox/imap sources share the same identifier. 2. re-auth hint: include --type gmail in the remove-account suggestion since the identifier may be shared across source types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T21:08:42Z

roborev: Combined Review (`8d18bf0`)

Verdict: The changes
strengthen the OAuth flow and token persistence, but introduce a medium-severity issue with error handling in source lookups.

Medium

Error Suppression in findGmailSource()
findGmailSource() converts any GetSourcesByIdentifier() failure into nil. In verify, that turns
real DB/query errors into a successful "Gmail account ... not found" exit, which masks corruption/schema issues and returns exit code 0.
Suggested fix: Make findGmailSource return (*store.Source, error) and propagate the error in callers instead of treating it as
“not found”.
References: cmd/msgvault/cmd/addaccount.go, [cmd/msgvault/cmd/verify.go](/tmp/msgvault-review
/cmd/msgvault/cmd/verify.go#L95)

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

A query failure in GetSourcesByIdentifier was silently converted to "account not found" with exit code 0. Now findGmailSource returns an error, and verify propagates it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The mismatch handler now calls findGmailSource directly and propagates lookup errors instead of swallowing them via hasGmailSource. Remove the now-unused hasGmailSource wrapper and update the test to exercise findGmailSource. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T21:49:29Z

roborev: Combined Review (`094bbbb`)

Verdict: The changes improve token validation and security, but introduce a functional regression regarding Gmail plus-address aliases.

Medium

[internal/oauth/oauth.go](/home/rob
orev/repos/msgvault/internal/oauth/oauth.go): In the new sameGoogleAccount / normalizeGmailAddress logic, user+tag@gmail.com and user@gmail.com represent the same Gmail account, but the new comparison only normalizes dots and google mail.com. This means Authorize will now reject valid Gmail plus-address aliases as a TokenMismatchError, which is a real regression for users who use tagged Gmail addresses as their account identifier.
Suggested fix: Strip the +suffix from the local part for gmail.com / googlemail.com before comparing, and add regression coverage for both domains.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Gmail ignores +suffix in the local part, so user+tag@gmail.com is the same account as user@gmail.com. normalizeGmailAddress now strips the +suffix before comparing, preventing false TokenMismatchError for users who use tagged Gmail addresses as their account identifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

roborev-ci · 2026-03-15T23:04:09Z

roborev: Combined Review (`ea4624f`)

These commits harden Gmail OAuth validation and handle account alias cases, but introduce a potential issue with Google Workspace alias re-authentication.

Medium

cmd/msgvault/cmd/root.go and internal/oauth/oauth.go: Existing Google Workspace accounts added under an alias become effectively unrecoverable on re-auth. sameGoogleAccount now rejects non-gmail.com alias/primary pairs, and the recovery text in getTokenSourceWithReauth tells the user to run remove-account, which irreversibly deletes the local archive. That turns a credential mismatch into a destructive recovery path. Suggested fix: add a non-destructive identifier migration path (rename-account/source+
token rename), or keep same-domain Workspace alias re-auth recoverable with a warning instead of directing users to remove-account.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

wesm · 2026-03-15T23:06:57Z

"⏺ This is a theoretical concern about a narrow edge case, and the suggested fix (add a
rename-account command) is new scope, not a bug in this PR.

The scenario: someone added alias@company.com, it worked, token expires, re-auth fails because
profile returns primary@company.com. The error suggests remove-account + add-account. Yes,
remove-account deletes local data — but that's clearly documented in remove-account --help, and
the user can choose not to follow the suggestion.

A rename-account migration command is a reasonable future feature but not a prerequisite for this
PR. The current behavior is safe (reject unverifiable mismatches), the error is informative, and
the user is in control."

wesm and others added 4 commits March 15, 2026 08:03

wesm and others added 3 commits March 15, 2026 14:50

wesm and others added 3 commits March 15, 2026 15:11

wesm and others added 5 commits March 15, 2026 15:41

wesm mentioned this pull request Mar 15, 2026

Support multiple tokens in config.toml for multiple account syncs #201

Closed

wesm and others added 2 commits March 15, 2026 16:42

wesm merged commit e4abd98 into main Mar 15, 2026
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent token pollution during sync re-auth#203

Prevent token pollution during sync re-auth#203
wesm merged 20 commits intomainfrom
fix/sync-reauth-no-launch-browser

wesm commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

roborev-ci bot commented Mar 15, 2026

Uh oh!

wesm commented Mar 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

wesm commented Mar 15, 2026

Summary

Test plan

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (fcb77e4)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (0bbd388)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (fe7b2b5)

Findings

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (37c4f58)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (684cc98)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (787033d)

Medium Severity Findings

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (8d18bf0)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (094bbbb)

Medium

Uh oh!

roborev-ci bot commented Mar 15, 2026

roborev: Combined Review (ea4624f)

Medium

Uh oh!

wesm commented Mar 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

roborev: Combined Review (`fcb77e4`)

roborev: Combined Review (`0bbd388`)

roborev: Combined Review (`fe7b2b5`)

roborev: Combined Review (`37c4f58`)

roborev: Combined Review (`684cc98`)

roborev: Combined Review (`787033d`)

roborev: Combined Review (`8d18bf0`)

roborev: Combined Review (`094bbbb`)

roborev: Combined Review (`ea4624f`)