fix(auth): harden the desktop login-code flow (GTM-93)

[deepme987]

Review follow-ups on #1222, raised as a separate PR against that branch rather than pushed onto it — @benceruleanlu owns the design, so these are proposals to accept/reject, not a fait accompli. Merge into #1222 if you agree.

Every fix has a test that fails without it. Nothing here changes the flow's shape.

What

Fix	Why
Re-check the view's origin before injecting the session	The inject script carries the Firebase refresh token into the page's main world. Minutes elapse between flow start and injection (browser sign-in + post-sign-in hold) — ample time for the view to have navigated away. Applied to the legacy bridge too, which has the same gap.
Bound the grant timings	> 0 alone admits Infinity, which makes the code deadline unreachable — and that deadline is the poll loop's only exit. The loop then never terminates and its banner/listener cleanup never runs. A seconds/ms unit slip is the realistic source, so clamp: poll_interval to [1,30]s, expires_in to ≤900s.
Retry 408/429	Each sign-in issues tens of exchange polls, so a rate limiter on that endpoint is expected. Treating it as terminal killed a sign-in whose code and browser tab were both still valid.
Send code_challenge_method=S256	RFC 7636 §4.3 defaults an omitted method to plain. Ingest pins S256 server-side, so this is not exploitable today — it makes the contract explicit on the wire instead of an implicit cross-repo convention.
Emit error_status on sign_in_failed	The raw message stays out (it can carry response bodies), but without the HTTP status every failure collapses into one error_class/error_bucket pair. An old backend (404), a verifier mismatch (403), and a 5xx are indistinguishable — exactly when the rollout needs them separated.
Fail safe if signInViaDesktopLoginCode throws	It was the only await outside a try, and the call site is fire-and-forget. An unexpected throw meant no flow, no fallback, no sign_in_failed, and an unhandled rejection — a Sign in button that silently does nothing.

On the legacy-bridge origin guard

The same injection gap exists on the loopback path, so the guard is applied there too — but it only enforces when the start origin was determinable, so it cannot regress a sign-in where getURL() comes back empty. That path is shipped code; the new path's guard is strict.

Testing

The PKCE binding had no coverage: the suite passed with the challenge downgraded to plaintext, with an unrelated verifier sent at exchange, and with code_challenge dropped entirely — because index.test.ts mocks ./client and only ever asserted exchangeDesktopLoginCode call counts, never its arguments. Added an assertion that the verifier presented at exchange is the preimage of the challenge presented at create.

Mutation-checked — each of these now fails the suite:

• code_challenge: codeChallengeS256(v) → code_challenge: v (plaintext downgrade)
• exchange sends a freshly minted, unrelated verifier
• the origin guard is removed before the inject

123 tests green in src/main/auth (was 108). Typecheck + lint clean.

Not in this PR (deliberately)

• Device-code phishing. Nothing binds a code to the machine that minted it, so an attacker can mint a code, send a victim the real cloud.comfy.org URL, and take over the account on approval. PKCE doesn't help — the attacker is the legitimate PKCE client. The standard mitigation is a short user_code shown on the desktop that the approval page requires you to match (RFC 8628 §5.4; it's why Microsoft added number matching). That's a design change across three repos and needs an owner decision, not a patch.
• One pre-existing issue in the fallback path, reported separately through security rather than a public PR.

GTM-93 · follow-up to #1222

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f08cdec3-1651-48a8-8887-9c1aad8031d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch deepme987/desktop/login-code-hardening/GTM-93

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch deepme987/desktop/login-code-hardening/GTM-93

---

_{Comment @coderabbitai help to get the list of available commands.}