Derive CORS allowed origins from DB, remove ALLOWED_ORIGINS env var

[pstaylor-patrick]

Summary

• Created lib/cors.ts — shared CORS utility that queries oauth_client.allowed_origin from the DB with a 5-minute in-memory cache, replacing the ALLOWED_ORIGINS env var
• Moved CORS out of middleware into route handlers (token, userinfo, authorize), since Next.js Edge Runtime middleware can't use node-postgres
• Fixed broken/permissive OPTIONS handlers — token OPTIONS was missing CORS headers entirely, userinfo OPTIONS was allowing any origin
• Removed ALLOWED_ORIGINS from apphosting.yaml, firebase-secrets.sh, .env.example, and add-client.ts next-steps output

Motivation

Every time a new OAuth client was registered, the ALLOWED_ORIGINS env var had to be manually updated and redeployed. The DB (oauth_client.allowed_origin) already has the source of truth — now CORS origins are derived from it automatically.

Test plan

• npx tsc --noEmit passes (pre-existing .next cache error is unrelated)
• OAuth flow works end-to-end with a registered client origin
• OPTIONS preflight returns correct CORS headers for a registered client's origin
• Unregistered origins get no CORS headers
• ALLOWED_ORIGINS is no longer referenced anywhere in codebase

🤖 Generated with Claude Code

[pstaylor-patrick]

Code Review Summary

This PR removes the ALLOWED_ORIGINS env var and derives CORS allowed origins directly from the oauth_client.allowed_origin DB column, with a 5-minute in-memory cache. The refactor consolidates duplicated CORS logic into a shared lib/cors.ts module and moves CORS handling from Edge middleware into route handlers (necessary because Edge Runtime cannot use node-postgres).

What works well:

• Clean extraction of duplicated CORS logic into a single module with clear separation between preflight handling and per-response header injection
• Smart dual-mode design: strict per-client validation when clientId is known, broad registered-origin check for preflight/error paths
• Proper Vary: Origin header included (previously missing in some paths)
• Good cleanup of env var references across apphosting.yaml, firebase-secrets.sh, .env.example, and add-client.ts
• Preflight correctly returns 204 (not 200)

Top risks:

1. Module-level cache has a minor race condition on concurrent cache-miss (P1)
2. Error responses on token/userinfo endpoints use broad origin check instead of per-client check, leaking that an origin is registered for some client (P4, P5)
3. pnpm-lock.yaml added -- unclear if intentional or accidental (P6)
4. Unrelated plans/migration-plan.md included in the diff (P7)

Findings:

• P1 [warning] -- Cache race condition on concurrent refresh
• P2 [warning] -- Silent deny on unknown origins could use a comment
• P3 [suggestion] -- Fast-path cache check before per-client DB query
• P4 [warning] -- Token error responses use broad origin check
• P5 [warning] -- Userinfo error responses use broad origin check
• P6 [suggestion] -- pnpm-lock.yaml may be unintended
• P7 [suggestion] -- Unrelated migration plan in PR

No blockers found. The CORS logic is correct and the security posture is improved over the previous state (which had a userinfo OPTIONS handler that allowed any origin). Good to merge after addressing the warnings.

[pstaylor-patrick]

Re-review: All 7 original findings addressed

Verdict: APPROVE (posted as COMMENT because GitHub blocks self-approval) -- all original findings (P1-P7) have been properly fixed. One new minor suggestion below.

Verification of original findings

#	Finding	Fix commit	Status
P1	Cache race on concurrent refresh	a6d682b	Verified: concurrent callers now share a single DB query via pendingRefresh
P2	Silent deny needs comment	1e2c999	Verified: comment at line 58 explains intent
P3	Fast-path cache check before per-client query	b26939a	Verified: isAllowedOrigin() guard at line 77 short-circuits before DB hit
P4	Token error responses use broad origin check	4278cf7	Verified: all addCorsHeaders calls in token route now include clientId where available
P5	Userinfo error responses use broad origin check	fa83bb6	Verified: comment at lines 18-19 documents that clientId is unavailable pre-validation
P6	pnpm-lock.yaml unintended	e3b5e3f	Verified: no longer in diff
P7	Unrelated migration plan	9a6265d	Verified: no longer in diff

New finding (non-blocking)

P1 [suggestion] -- getAllowedOrigins() has no .catch() on the pending promise. A transient DB failure would permanently poison pendingRefresh until process restart. See inline comment for suggested fix.

Summary

The CORS refactor is clean and well-structured. The shared lib/cors.ts module correctly handles preflight, per-client strict validation, and broad origin checks. The Vary: Origin header is properly set. The middleware cleanup is correct. No blockers remain -- good to merge.