fix(cli): prevent crashes and edge-case bugs in auth flows

[appleboy]

Summary

Fixes a set of latent crash and edge-case bugs across the OAuth flows, surfaced by a full-codebase review, plus two behavior-preserving simplifications. The standout is a server-driven panic in the device flow (a negative interval reaches time.NewTicker, which panics and crashes the CLI). All changes are accompanied by a passing test suite and three new regression tests.

AI Authorship

• No AI was used in this PR
• AI was used. Details:
  • Tool / model: Claude Opus 4.8 (1M context) via Claude Code
  • AI-authored files: all 9 — auth.go, browser_flow.go, config.go, device_flow.go, main.go, token_cmd.go, tui/styles.go, polling_test.go, extra_claims_test.go
  • Human line-by-line reviewed: none yet — author will review before merge. Reviewers should read the core paths line-by-line (see Reviewer guide).

Change classification

• Leaf node (local impact)
• Core code (broad impact — needs line-by-line review)

Touches the OAuth device/browser auth flows, token refresh, and config resolution. A bug here affects every authentication.

Verification

• Unit tests — full existing suite passes (go test ./...)
• Integration tests — device/browser poll flows exercised via httptest servers
• New regression tests (3): negative poll-interval (must not panic), plus extra-claims key-trim and whitespace-only-key rejection
• Stress / soak test: N/A — no new long-running code; the poll loop is covered by existing timeout tests
• Manual verification: make fmt, go build ./..., go vet ./..., make lint (0 issues), go test ./... — all green. The negative-interval test returns in 0.20s (confirming the clamp short-circuits the 5s poll), proving it no longer panics.

Verifiability check

• Inputs and outputs are documented (commit body + inline comments on each fix)
• Reviewer can judge correctness via tests + signatures for most files
• Failures surface via existing error paths / tests

Security check (PR touches the OAuth server interface)

• No secrets in code
• All external inputs validated — hardens server expires_in/interval and a null JWT payload against malformed/malicious server responses
• Errors don't leak internals — existing browser/terminal error sanitization unchanged
• Server URL normalized (trailing-slash trim) so default endpoints can't malform

Risk & rollback

• Risk: Low–medium. The slow_down backoff now grows 1.5×/step (was compounding) — timing-only, still capped at 60s. The duration-config dedup (parseDurationConfig) preserves the exact same returns/warnings (covered by TestRefreshThresholdConfig). The server-URL trailing-slash trim is strictly safer (a deployment relying on a literal trailing slash is normalized, not broken).
• Rollback: revert the single commit fix(cli): prevent crashes and edge-case bugs in auth flows.

Reviewer guide

• Read carefully:
  • device_flow.go — non-positive interval clamp (panic fix) and the rewritten slow_down backoff math
  • browser_flow.go — the timer goroutine is now joined (WaitGroup) before return; verify the LIFO defer order (close(done) then Wait) closes the send-on-closed-channel race
  • config.go — parseDurationConfig dedup, server-URL trim placement (after validation), and parseExtraClaimPair key trimming via strings.Cut
• Spot-check OK (tests + signatures suffice): tui/styles.go (>= 24 off-by-one), token_cmd.go (null-payload guard), auth.go (*storage = *newStorage), main.go (useCached closure extraction), and the two test files.

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

❌ Patch coverage is 58.33333% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
main.go	0.00%	6 Missing ⚠️
browser_flow.go	0.00%	3 Missing ⚠️
config.go	90.00%	2 Missing ⚠️
token_cmd.go	0.00%	1 Missing and 1 partial ⚠️
auth.go	0.00%	1 Missing ⚠️
tui/styles.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR hardens the CLI’s OAuth authentication flows against malformed/malicious server responses and other edge cases that could previously cause crashes or incorrect behavior, and it introduces small refactors to reduce duplication while preserving behavior.

Changes:

• Prevents device-flow crashes by clamping non-positive polling intervals before creating tickers, and adjusts slow_down backoff growth to be linear (1.5× each step) rather than super-exponential.
• Improves robustness and consistency across auth/config handling: trims trailing slashes on SERVER_URL, deduplicates duration parsing logic, trims --extra-claims keys, and rejects JWT payloads that decode to JSON null.
• Adds regression tests covering negative polling intervals and extra-claims key trimming/whitespace-only key rejection.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
device_flow.go	Clamps server-provided poll interval to avoid time.NewTicker panics; revises slow_down backoff math.
browser_flow.go	Joins the timer-update goroutine via WaitGroup to avoid send-after-close panics on updates.
config.go	Validates callback port range, trims trailing slashes on ServerURL, trims extra-claims keys, and deduplicates duration parsing via parseDurationConfig.
auth.go	Copies refreshed token storage wholesale to avoid silently dropping fields on refresh.
main.go	Extracts a small useCached closure to reduce repeated assignments in cached-token paths.
token_cmd.go	Rejects JWT payloads that decode as JSON null to avoid treating malformed tokens as empty-but-valid claims.
tui/styles.go	Fixes a boundary condition so exactly-24-hour durations format as days/hours.
polling_test.go	Adds regression coverage ensuring negative device polling intervals don’t panic and return via context timeout.
extra_claims_test.go	Adds regression coverage for key whitespace trimming and whitespace-only key rejection.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.