Skip to content

fix: preserve live Convex data and surface magic-link failures - #49

Merged
EtanHey merged 2 commits into
masterfrom
security/magic-link-delivery
Oct 30, 2026
Merged

EtanHey merged 2 commits into
masterfrom
security/magic-link-delivery

Conversation

@EtanHey

@EtanHey EtanHey commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

The live site uses the secured, populated harmless-husky-580 deployment. The quixotic-stork-639 production deployment is empty. Keep production browser and SSR clients on the live data while fixing missing auth origins and swallowed magic-link delivery errors.

  • Share the live harmless-husky-580 cloud/site URL pair across browser auth/data, SSR auth proxy and SSR queries. Development remains configurable with VITE_CONVEX_*. Moving data to quixotic-stork-639 is a separate Etan decision; this PR performs no migration.
  • Keep the shared auth/CORS origins, including both Vercel aliases and configured SITE_URL.
  • Return HTTP 502 / MAGIC_LINK_DELIVERY_FAILED for missing email configuration, returned provider errors and thrown provider failures. Send-error logging uses fixed allowlisted codes; no raw provider errors, recipients, links or tokens.

Validation at this revision:

  • RED: updated routing regression failed against the old quixotic pin (1 failed, 1 passed).
  • GREEN: all 10 focused routing/auth cases passed. The unbypassed commit hook ran once: 333 tests in 21 files, TypeScript and docs.local guard passed.
  • Production build passed with quixotic cloud/site URLs supplied as build variables. Browser output (25 JS files) and SSR output (27 JS/MJS files) contain the live cloud/site URLs and no quixotic reference. Warnings: large chunks, unused dependency imports and ignored dependency directives.
  • Shared origins and sanitized 502 handling are unchanged from the earlier PR head and remain covered by real Better Auth HTTP/client tests with mocked Resend.
  • Git local/global configuration, effective/local config listings, Convex login config, root HEAD and root status hashes remain unchanged. Push was fast-forward, without -u.

Separately authorized live remediation: the deployment keys differed by in-process SHA-256 comparison; copied the new prod sending key to dev via CLI stdin and verified equality. No values were emitted or saved. One actual live HTTP magic-link POST returned 200; a bounded 20-second dev log read contained one fixed Resend send-success message and zero provider-error/exception/not-configured messages. Mailbox and Resend-dashboard confirmation remain with Etan. A preceding Python transport failed local TLS validation before HTTP; curl used normal TLS verification. No HTTP retry or verification-link use occurred.

Independent other-vendor review and the auth security scan remain pending with the lead. No new reviewer or PR bots were started by this worker. Earlier limited local CodeRabbit coverage is not a review of this revision. No merge or deploy; live frontend/backend code was not updated by this rework.

Loop-record: implementer=codex/gpt-6.1-sol reviewer=pending rounds=0/0 defects=1 role=worker rework=1 (user correction: preserve populated live deployment; remove empty-prod pin)

Size: M. Retain the existing worktree for lead-routed review/fixes. No merge or deploy is authorized.

— songscriptCodex (worker) · codex/gpt-6.1-sol

…ailures

Co-Authored-By: songscriptCodex running gpt-6.1-sol <noreply@openai.com>
@EtanHey EtanHey added the size:M Tight-loop PR size: 101-400 hand-written lines changed label Oct 30, 2026
Keep browser and SSR production clients on the secured populated backend.
Migration to the empty prod backend requires a separate decision.

Co-Authored-By: songscriptCodex running gpt-6.1-sol <noreply@openai.com>
@EtanHey EtanHey changed the title fix(auth): correct production routing and expose magic-link delivery failures fix: preserve live Convex data and surface magic-link failures Oct 30, 2026
@EtanHey

EtanHey commented Oct 30, 2026

Copy link
Copy Markdown
Owner Author

Security review: APPROVE (head 37dd228)

I checked the code, the test results, and one vite build in a git archive copy. Nothing was deployed, and I ran no Convex or Vercel CLI.

  • No repoint. The production build is pinned to harmless-husky-580. I built with quixotic-stork-639 passed in as VITE_CONVEX_*, and the output (.output/public and .output/server) still contains harmless only, with 0 quixotic references.
  • Origins. The allowlist is exactly localhost:3001, songscript-ten and songscript-eight on .vercel.app, plus the SITE_URL origin. It has no * and no *.vercel.app. CORS (convex-helpers) sends back the request's Origin only on an exact match. Lookalike or relative-escape callbackURLs (//evil, https:evil, eight.vercel.app.evil…, /\evil) return 403 with no email sent.
  • 502 path. Missing key, a returned provider error and a thrown provider error all produce a generic MAGIC_LINK_DELIVERY_FAILED 502 with a fixed code. I grepped every console.* in convex/**: none logs an email, link, token or raw Resend error.
  • Enumeration. Better Auth 1.4.9 calls sendMagicLink no matter whether the account exists, so 502 vs 200 depends only on the provider, not on account existence.
  • Tests. 10/10 focused and 333/333 full suite pass.

Follow-up (XS PR, not this branch), test hardening. I injected regressions to see what the tests catch. They missed three:

  • Logging a raw thrown error: JSON.stringify drops an Error's message, so the leak assertion can't see it.
  • Logging the recipient: console.log isn't spied, and toHaveBeenCalledWith allows extra calls.
  • A *.vercel.app wildcard: no evil-subdomain case.

Fix: spy on all console methods, serialize with util.inspect, assert exactly one console.error call, and add a 403 test for https://evil.vercel.app.

Low-priority rows:

  • The "db" in ctx "already registered" throw in sendMagicLink was already on master. It's unreachable over HTTP but is a latent enumeration oracle.
  • new URL(SITE_URL) runs at module load, so a malformed SITE_URL takes down every auth route. It fails closed.

Done = user-visible. The 502 and CORS changes only reach the live site after a separate, authorized Convex function deploy to harmless-husky-580. build is plain vite build.

Full review: orchestrator docs.local/…/leads/security/review-songscript-49-opus.md

— security reviewer (worker) · claude-code/claude-opus-5-5

@EtanHey
EtanHey merged commit 878a965 into master Oct 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M Tight-loop PR size: 101-400 hand-written lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant