Land auth OTP + Google + mail (A1–A6) onto main, with restored HIGH pre-hijacking fix#305
Merged
Conversation
Passwordless sign-in for LISA accounts: request a 6-digit code, spend it, and you are in. Reading the mail is the proof of ownership, so one step both registers and authenticates — and the address comes out verified, which is what levels the free window from $1 to $5. - src/web/otp.ts: the code store behind the same file/Firestore seam as accounts.ts. One record per address, split into a challenge (hash, expiry, wrong-guess count) and a send budget (cooldown, daily cap) — the budget deliberately survives a spent code so redeeming one can't reset the mail-bombing cap. Codes are stored only as address-salted SHA-256 and compared in constant time; a code dies at the first of expiry, five wrong guesses, or being spent. - accounts.ts: ensureOtpAccount() — lookup and insert inside one mutation so a Firestore CAS retry can't create an address twice. Password material is now explicitly optional; a code-only account fails the password path against the decoy params like any other bad credential. - mailer.ts: the code mail, sharing one transport with the verification link. Without RESEND_API_KEY it degrades to a server log, so a real key is now a production requirement. - POST /api/auth/otp/request + /verify, pre-gate like the other sign-in surfaces, behind the same per-IP backstop as register/login. The request answer is identical whether or not the address has an account — no membership oracle. Password sign-in is untouched: App Review's demo account keeps working, and a code is an additional way in rather than a replacement. Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (A0/A1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Google ID-token verification, and the account rule that keeps one person on one balance no matter which door they come through. - src/web/googleAuth.ts: the sibling of cloudAuth.ts — RS256 over Google's JWKS, zero dependencies, JWKS fetch and clock injected so it tests offline. Both issuer spellings; audience must be a client id we actually run (an empty config rejects rather than vacuously passing); nonce verified when present (Google echoes it raw, unlike Apple's hash). email_verified is REQUIRED — the address is what binds an identity to an existing account, so an unproven one would be a takeover primitive. - accounts.ts: one address, one account. Email and Google accounts both claim their address (EMAIL_OWNER_KINDS), so every lookup spans both kinds: signing in by Google onto an address that already has an email account BINDS to it (same uid, same balance) rather than forking a second one, and a mailed code onto a Google-owned address signs into that same account. Apple is excluded on purpose — private-relay addresses are per-app aliases, not a claim on an inbox. Resolution is sub-first, so changing the address on a Google account keeps the LISA one. - POST /api/auth/google, pre-gate like /api/auth/apple, behind the same per-IP backstop. /api/auth/config now advertises the web client id so the login page knows whether to draw the button. Off unless LISA_GOOGLE_WEB_CLIENT_ID / LISA_GOOGLE_IOS_CLIENT_ID are set. Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (A3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The four clients for the A1 endpoints. Everywhere the shape is the same:
type an address, get six digits, you're in — and if the address is new
that same step creates the account, because reading the mail is the proof.
Passwords move one layer down ("Use a password instead") rather than away.
- web login page: code-first, with the step revealed only after a code is
actually sent, the confirmation naming the address, and editing the
address rewinding a step (an outstanding code belongs to the old one).
A burned or expired code returns to "send one" instead of leaving a
dead field. LOGIN_HTML joins the inline-<script> syntax guard.
- iOS: one form serves both Settings and onboarding, so they can't drift.
SignInCodeError carries the server's typed reason — status alone can't
tell "a code just went out" from "too many today", and those need
different words.
- Mac menu-bar app: "Email me a code", then whichever credential is
filled signs in (code first, else password).
- CLI: `lisa login` mails a code by default; `--password` keeps the old
path. Wrong digits retry in place while the code is still live.
Two fixes found while testing the CLI end-to-end:
- one readline interface per command instead of one per prompt. A fresh
interface drops what the previous reader buffered, so every prompt
after the first read nothing as soon as stdin wasn't a terminal.
- an unreachable server is now a typed `unreachable` result rather than
an exception caught by a blanket handler that reported every error —
including bugs — as "could not reach".
Also documents login/logout/billing in `lisa --help`, which never listed
them.
Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (A2).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- web: Google Identity Services renders its own button, drawn only when the instance advertises a web client id. Same nonce goes to Google and to our server (Google echoes it raw). The config fetch now feeds both providers, and the divider appears only when something sits above it. - iOS: authorization code + PKCE via ASWebAuthenticationSession, with no SDK — the flow is ~100 lines of OAuth, and doing it here matches how the server verifies Apple/Google tokens with node:crypto rather than a library. The verifier never leaves the device, so intercepting the redirect gains nothing; state guards against a redirect we didn't start; the session is ephemeral so signing out doesn't leave the next person silently signed in. ASWebAuthenticationSession intercepts the callback itself, so no URL scheme has to be registered in project.yml. The button sits BELOW Sign in with Apple — Guideline 4.8 wants SIWA offered with equal prominence wherever a third-party login is. /api/auth/config now advertises both client ids; each client draws its button only if its own id is present, so an instance can run Google on one surface and not the other. Client ids are public by design — they identify the app, they don't authorize it. Plan: docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md (A4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eploy them deploy.sh passes an explicit env allowlist, so the two Google client ids would have been dropped silently — a deploy that looked fine and left the button invisible. Added, with the sign-in-by-code dependency on RESEND_API_KEY spelled out in the header (no key ⇒ codes only reach the server log ⇒ nobody can sign in). Runbook gains Phase 8 (verify the Resend sending domain, with a curl self-check), Phase 9 (GCP OAuth consent + the Web and iOS client ids, plus the redeploy command with both new env groups) and Phase 10 (ship a build — deliberately NOT prescribing a version bump, since whether this folds into 1.1 or becomes 1.2 depends on ASC state the repo can't see). Plan doc marked implemented with the PR chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ess gate Sending identity moves to the mail.meetlisa.ai subdomain (verified in Resend) rather than the apex: the apex carries the domain's human mail and its reputation, and a bounce storm from transactional mail must not poison it. Code default and deployed env now name the SAME domain — the project this is modelled on drifted into a split-brain where the docs described one sender and production used another. - src/web/mailer.ts: HTML + plain text for both mails, hand-rolled with no template dependency (tables, inline styles, no <style> — Gmail strips it). A layout with heading/para/code/cta partials, and escapeHtml on everything that didn't come from the file. The two parts must say the same thing: multipart/alternative helps deliverability, but a text part that diverges from the HTML is itself a spam signal, and a test enforces that every link in the HTML is in the text too. - One exit, `deliver()`, emitting exactly one structured line per send with a fixed outcome vocabulary (success / skipped_no_key / send_failed). A non-2xx from Resend is a well-formed response, not a thrown error — logging it as loudly as a network failure is the whole point, since otherwise rate limits and unverified senders vanish. Recipients are redacted to ab***@domain.tld: logs aren't a PII-safe store, but the domain stays whole because delivery breaks by provider. - src/web/email-deliverability.ts: sign-in by code has a failure mode ordinary sign-in doesn't — a typo'd address doesn't bounce back to whoever typed it. They wait for a code that went elsewhere and burn a slot of their daily budget. So the address is checked BEFORE the budget is touched: deterministic typo lists first (which can suggest the fix, and which are careful that .co/.ne are real ccTLDs), then MX with an A-record fallback, raced against a timeout. It fails OPEN on every uncertainty — locking someone out because a resolver blinked is far worse than sending one mail into the void. All four clients surface the suggestion, so a typo is one tap from fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erge (HIGH) A HIGH fix that ultrareview landed on the stacked PRs (a6be431, 0f651a4) was overwritten when later branches in the chain re-touched accounts.ts, so the final merged code shipped the ORIGINAL vulnerable adoption paths. This restores it on both. The hole: /api/auth/register is open and unauthenticated, so an attacker can pre-create victim@x.co with a password they know. When the victim later signs in — by mailed code (ensureOtpAccount) or by Google (upsertGoogleAccount byEmail bind) — adoption flipped verified=true but KEPT the attacker's scrypt and did NOT rotate sessionVersion, so the attacker's password still authenticated the account the victim had just proven they own. markVerifiedByOwnershipProof(): on the untrusted unverified→verified transition, drop any pre-set password and rotate sessionVersion (killing any session minted from it). Only the FIRST verification rotates, so a later code sign-in doesn't log the owner out. Both adoption paths route through it. Tests: flipped the two assertions that pinned the vulnerable "password still works after adoption" behavior; added explicit attacker-scenario tests for both the OTP and Google paths, and a no-double-rotation test. NOTE (follow-up, lower severity): the verify-LINK path (confirmEmailVerification) can't get the same treatment — its legitimate flow is the owner setting their own password then clicking their own link, indistinguishable server-side from the attack, so dropping the password there would break normal signup. Needs a design decision (flagged for review), and requires the victim to click an unsolicited link, upgrading only the tier of an account the attacker already held. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # docs/PLAN_AUTH_OTP_GOOGLE_v1.0.md # src/cli.ts # src/web/accounts.test.ts # src/web/accounts.ts # src/web/mailer.ts # src/web/server.ts # src/web/verification.test.ts
CI caught a flake my local run raced past: the 'slow resolver' test used a never-settling promise, which trips node:test's pending-work detector on a slower event loop (ERR_TEST_FAILURE: 'Promise resolution is still pending', cascading cancelledByParent to its sibling). - checkDeliverable now clears its race timer in a finally and sets a flag so the abandoned lookup won't fire a second DNS query after the race is over — production hygiene, and it makes the timeout path deterministic. - the test replaces the never-promise with a releasable gate it drains before the assertion returns, so no promise outlives the test. 20× local loop clean; full suite 1355 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eal CI fix) The previous attempt made the leak worse. The timeout side is `new Promise((r) => setTimeout(() => r(admit), ms))`; clearing the timeout stops the callback from ever calling `r`, so that promise then dangles FOREVER — exactly the "Promise resolution is still pending" node:test flags on CI's slower event loop, and it happened in every deliverability test (the fast ones left a 3s timer promise pending after the lookup won). Fix: keep the resolver external and call it in `finally` with a `cancelled` sentinel, so the timer promise always settles regardless of who won the race. The race result is already locked in by then, so the late settle is inert. `decided` still stops the abandoned lookup from a second DNS query. Full-glob run (the CI invocation) 5x clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…k, real fix) Root cause finally pinned: CI runs node 22, I run node 24, and node 22's node:test has a stricter pending-promise detector. The "slow resolver" test used a promise that never settled; when the timeout won the race the lookup stayed parked on it forever, and node 22 flagged "Promise resolution is still pending" — which cascaded (cancelledByParent) to every sibling in the describe, so the whole "fails open" suite went red. Reproduced by downloading the node 22.23.1 binary and running the exact CI glob, then fixed against it: the stub resolver now settles on its OWN timer shortly AFTER the timeout, and the test outlasts it, so the abandoned lookup fully unwinds before teardown and nothing is left pending. The assertion still proves the timeout wins (returns before the resolver). Verified: node 22, full `src/**/*.test.ts` glob, 3× — 1355 pass, 0 leaks. (The earlier timer-settle fix in checkDeliverable was still correct and stays — it fixed the fast-path timer promise; this fixes the timeout path.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 25, 2026
Closed
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
1 participant
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.
Why this PR exists
The six stacked PRs #289–#293, #298 were squash-merged in order — but because they were stacked (each based on the previous branch, not
main), squashing #289 made GitHub mark #290–#298 as "merged" while only A1's commits actually landed onmain. This is the exact stacked-PR trap the runbook warns about (#170).Worse: a HIGH account-pre-hijacking fix that ultrareview added on the chain (
a6be431,0f651a4) was overwritten when later branches re-touchedaccounts.ts, so even the branch tips shipped the original vulnerable code.This PR lands the complete A1–A6 diff on
mainin one clean merge, with the HIGH fix restored on both adoption paths.What's here (net diff vs main)
src/web/otp.tssrc/web/googleAuth.ts, one-address-one-account bindingdeploy.shenv passthrough (Google client ids + RESEND — verified present)mail.meetlisa.ai(HTML+text, one logged exit, redaction, pre-send deliverability gate)The restored HIGH fix
/api/auth/registeris open, so an attacker can pre-createvictim@x.cowith a password they know. When the victim later signs in by code or by Google, adoption used to keep that password and not rotatesessionVersion→ the attacker's credential still authenticated the account the victim just proved they own.markVerifiedByOwnershipProof()now drops any pre-set password and rotatessessionVersionon the untrusted unverified→verified transition (only the first verification rotates, so the owner isn't logged out on later sign-ins). Both the OTP and Google-bind paths route through it. Explicit attacker-scenario tests for both.Known follow-up (lower severity, deliberately not fixed here): the verify-link path can't take the same treatment — its legitimate flow (owner sets own password, clicks own link) is indistinguishable server-side from the attack, so dropping the password there would break normal email+password signup. Needs a design decision; requires the victim to click an unsolicited link and only upgrades the tier of an account the attacker already held at $1.
Verification
npm test→ 1197 green, typecheck clean.swift build+ iOSswiftc -typecheckclean; iOS redirect-scheme derivation checked against the real OAuth plist.delivered, not spam-pending) through the production code path onmail.meetlisa.ai.Supersedes the un-landed #290–#293, #298. 🤖 Generated with Claude Code