You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The app ships a custom OAuth-state CSRF layer (src/lib/auth/oauth-state.ts + an oauth_states table) that has zero production callers. The real CSRF defense is Supabase's built-in PKCE state — both OAuth entry points say so in comments. The custom layer is therefore dead code, and worse, its table carries fully-public RLS (WITH CHECK (true) / USING (true) on INSERT/SELECT/UPDATE) — live attack surface (anyone with the anon key can flood, read, or overwrite rows) for a table nothing consumes.
Recommendation: delete the full surface. Surfaced by the systematic code review in #181 (deliberately not done there — touches the migration + prod DDL, bigger than a review-scoped change).
Evidence it's unused
Zero production importers of src/lib/auth/oauth-state.ts — the only importer is its own test file (verified: grep for generateOAuthState/validateOAuthState/oauth-state across src/ returns only the module + its __tests__).
src/components/auth/OAuthButtons/OAuthButtons.tsx:29 — no import; comment: "Supabase handles CSRF protection via built-in state parameter (PKCE flow) — No need to manually manage state tokens". Calls supabase.auth.signInWithOAuth({...}) with no manual state.
src/app/auth/callback/page.tsx:36 — no import; comment: "Supabase handles state validation internally - no manual check needed". Never validates a state token.
Why it's a liability, not defense-in-depth
The oauth_states RLS is fully public (supabase/migrations/20251006_complete_monolithic_setup.sql L340-361): public INSERT (WITH CHECK (true)), public SELECT (USING (true)), public UPDATE (USING (true)), public DELETE-if-expired. Anyone with the anon key can spam/read/overwrite it — pure attack surface for a table with no consumer.
The design is also weaker than it looks even if it were wired in: session_id is self-reported from sessionStorage by the same client, and both generateOAuthState/validateOAuthState fail open. So it wouldn't add meaningful protection beyond PKCE.
Keep-vs-delete (honest): the only "keep" argument is defense-in-depth if a fork disabled Supabase PKCE — but the layer isn't wired into the flow, so it provides zero protection today, and such a fork would have to re-integrate it from scratch anyway. Keeping dead code "just in case" is latent risk (public-write table), not real defense-in-depth. Net: delete.
Migration (EDIT THE MONOLITH DIRECTLY — do NOT create a separate migration file, per CLAUDE.md): in supabase/migrations/20251006_complete_monolithic_setup.sql, remove the oauth_statesCREATE TABLE (L317-328), 3 indexes (L330-332), 4 RLS policies (L340-361), 2 COMMENTs (L334, L363), and the summary-comment mention (L2324).
Prod DDL: apply a live DROP TABLE oauth_states CASCADE; to prod via the Supabase Management API (POST /v1/projects/{ref}/database/query, JSON payload built from a FILE — mirrors the Drop #49 legacy schema leftovers (profiles, audit_logs, handle_new_user, handle_updated_at) #170 legacy-table drop). It holds only transient OAuth tokens (5-min expiry) → safe to drop with no backfill. Verify post-drop that the table + its 4 policies are gone.
Reset script: remove the deleteAllFromTable('oauth_states', 'OAuth states') call at scripts/reset-database.ts:274.
Generated types: regenerate src/lib/supabase/types.ts (the oauth_states Row/Insert/Update block is L406-444) via GET /v1/projects/{ref}/types/typescript.
Teardown:supabase/migrations/999_drop_all_tables.sql:42 (DROP TABLE IF EXISTS oauth_states CASCADE;) — may stay as a harmless no-op or be removed for tidiness.
No cleanup fn/trigger/cron to remove — none exists (the table is fully inert; the only "cleanup" was the dead client-side cleanupExpiredStates).
Docs cleanup (recommended — they actively TEACH the removed pattern)
The OAuth blog post advertises the manual generate/validate flow that contradicts the real PKCE code, so it's teaching a dead (and insecure-looking) pattern:
public/blog/authentication-supabase-oauth.md (and its compiled copy src/lib/blog/blog-data.json) — update the code samples to the actual signInWithOAuth PKCE flow.
pnpm type-check + pnpm lint clean after the module/test/type removals.
Full test suite green (nothing else imports the module).
Prod: confirm oauth_states table + its 4 policies are dropped (query information_schema.tables / pg_policies), and OAuth sign-in still works (PKCE was always the real path).
Fresh-install parity: re-running the monolith on a clean DB no longer creates the table.
Estimated: S (small — deletions + one live DROP + type regen + doc touch-ups).
Origin: code review #181. Related follow-up: #182 (finish group message E2E encryption).
Summary
The app ships a custom OAuth-state CSRF layer (
src/lib/auth/oauth-state.ts+ anoauth_statestable) that has zero production callers. The real CSRF defense is Supabase's built-in PKCE state — both OAuth entry points say so in comments. The custom layer is therefore dead code, and worse, its table carries fully-public RLS (WITH CHECK (true)/USING (true)on INSERT/SELECT/UPDATE) — live attack surface (anyone with the anon key can flood, read, or overwrite rows) for a table nothing consumes.Recommendation: delete the full surface. Surfaced by the systematic code review in #181 (deliberately not done there — touches the migration + prod DDL, bigger than a review-scoped change).
Evidence it's unused
src/lib/auth/oauth-state.ts— the only importer is its own test file (verified: grep forgenerateOAuthState/validateOAuthState/oauth-stateacrosssrc/returns only the module + its__tests__).src/components/auth/OAuthButtons/OAuthButtons.tsx:29— no import; comment: "Supabase handles CSRF protection via built-in state parameter (PKCE flow) — No need to manually manage state tokens". Callssupabase.auth.signInWithOAuth({...})with no manual state.src/app/auth/callback/page.tsx:36— no import; comment: "Supabase handles state validation internally - no manual check needed". Never validates a state token.Why it's a liability, not defense-in-depth
oauth_statesRLS is fully public (supabase/migrations/20251006_complete_monolithic_setup.sqlL340-361): public INSERT (WITH CHECK (true)), public SELECT (USING (true)), public UPDATE (USING (true)), public DELETE-if-expired. Anyone with the anon key can spam/read/overwrite it — pure attack surface for a table with no consumer.session_idis self-reported fromsessionStorageby the same client, and bothgenerateOAuthState/validateOAuthStatefail open. So it wouldn't add meaningful protection beyond PKCE.Full removal surface (checklist)
src/lib/auth/oauth-state.ts(214 lines —generateOAuthState,validateOAuthState,cleanupExpiredStates, private helpersgenerateUUID/getSessionId).src/lib/auth/__tests__/oauth-state.test.ts(19 tests / 6 describes; self-contained in-memory mock — no shared-fixture impact).supabase/migrations/20251006_complete_monolithic_setup.sql, remove theoauth_statesCREATE TABLE(L317-328), 3 indexes (L330-332), 4 RLS policies (L340-361), 2COMMENTs (L334, L363), and the summary-comment mention (L2324).DROP TABLE oauth_states CASCADE;to prod via the Supabase Management API (POST /v1/projects/{ref}/database/query, JSON payload built from a FILE — mirrors the Drop #49 legacy schema leftovers (profiles, audit_logs, handle_new_user, handle_updated_at) #170 legacy-table drop). It holds only transient OAuth tokens (5-min expiry) → safe to drop with no backfill. Verify post-drop that the table + its 4 policies are gone.deleteAllFromTable('oauth_states', 'OAuth states')call atscripts/reset-database.ts:274.src/lib/supabase/types.ts(theoauth_statesRow/Insert/Update block is L406-444) viaGET /v1/projects/{ref}/types/typescript.supabase/migrations/999_drop_all_tables.sql:42(DROP TABLE IF EXISTS oauth_states CASCADE;) — may stay as a harmless no-op or be removed for tidiness.cleanupExpiredStates).Docs cleanup (recommended — they actively TEACH the removed pattern)
The OAuth blog post advertises the manual generate/validate flow that contradicts the real PKCE code, so it's teaching a dead (and insecure-looking) pattern:
public/blog/authentication-supabase-oauth.md(and its compiled copysrc/lib/blog/blog-data.json) — update the code samples to the actualsignInWithOAuthPKCE flow.docs/specs/017-security-hardening/(contracts/oauth-csrf.md, data-model.md, tasks.md, etc.),docs/specs/040-feature-040-code/,features/foundation/005-security-hardening/spec.md:18,scripts/audit/truth-table.json:148.Verification
pnpm type-check+pnpm lintclean after the module/test/type removals.oauth_statestable + its 4 policies are dropped (queryinformation_schema.tables/pg_policies), and OAuth sign-in still works (PKCE was always the real path).Estimated: S (small — deletions + one live DROP + type regen + doc touch-ups).
Origin: code review #181. Related follow-up: #182 (finish group message E2E encryption).