diff --git a/app/__tests__/api/waitlist-signup-write-authz.test.ts b/app/__tests__/api/waitlist-signup-write-authz.test.ts new file mode 100644 index 000000000..b88909f23 --- /dev/null +++ b/app/__tests__/api/waitlist-signup-write-authz.test.ts @@ -0,0 +1,109 @@ +/** + * GH#2503 — the waitlist table must have no anon write path. + * + * Every anti-abuse control on signup (referral-code requirement, Turnstile, + * honeypot, dwell time, disposable-email block, bot-UA filter, per-IP and + * per-referral-code caps, wallet-signature verification) lives in + * POST /api/waitlist/signup. That makes the route a security boundary only if + * it is the ONLY path that can write the table. + * + * It was not. The RLS policy read `for insert to anon with check (true)` — an + * unconditional grant to the role the PUBLISHABLE key maps to — so a direct + * PostgREST INSERT skipped every control above and could choose `created_at` + * (waitlist_position orders by created_at ASC, so: position #1), `tier`, + * `referral_code` and `referred_by_code` (leaderboard fraud). + * + * These assertions guard the source in the same style as the sibling + * waitlist-signup-*.test.ts files, because the property is structural: it is + * about which credential may write, which no request-level test can observe. + */ + +import { describe, it, expect } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; + +const ROUTE_PATH = path.resolve( + __dirname, + "../../app/api/waitlist/signup/route.ts", +); +const SCHEMA_PATH = path.resolve( + __dirname, + "../../../supabase-waitlist-schema.sql", +); + +/** + * Strip `--` line comments before matching statements. + * + * This file is mostly prose: it explains the anon/publishable-key model at + * length, and the GH#2503 note deliberately quotes `create policy ... to anon` + * to describe the regression it is guarding against. Matching statements + * against the raw text picks that sentence up as a policy and reads forward to + * the next `;`, which is how the first version of this test failed. Parse the + * SQL, not the commentary. + */ +function sqlStatements(schema: string): string[] { + const code = schema + .split("\n") + .map((line) => line.replace(/--.*$/, "")) + .join("\n"); + return code.match(/create policy[\s\S]*?;/gi) ?? []; +} + +describe("GH#2503: waitlist writes are server-only", () => { + it("grants the waitlist INSERT policy to service_role, not anon", () => { + const schema = fs.readFileSync(SCHEMA_PATH, "utf8"); + + const insertPolicies = sqlStatements(schema).filter((p) => + /for\s+insert/i.test(p), + ); + + expect(insertPolicies.length).toBeGreaterThan(0); + for (const policy of insertPolicies) { + expect(policy).toMatch(/to\s+service_role/i); + expect(policy).not.toMatch(/to\s+anon/i); + } + }); + + it("leaves no INSERT/UPDATE/DELETE policy granted to anon", () => { + const schema = fs.readFileSync(SCHEMA_PATH, "utf8"); + const anonWritePolicies = sqlStatements(schema).filter( + (p) => /to\s+anon/i.test(p) && /for\s+(insert|update|delete)/i.test(p), + ); + + expect(anonWritePolicies).toEqual([]); + }); + + it("inserts through the service-role client, not the publishable one", () => { + const source = fs.readFileSync(ROUTE_PATH, "utf8"); + + // The binding the insert is performed on must come from the service client. + const insertBinding = source.match( + /const\s+(\w+)\s*=\s*getWaitlistServiceSupabase\(\);[\s\S]*?await\s+\1\s*\.from\("waitlist"\)\s*\.insert\(/, + ); + expect(insertBinding).not.toBeNull(); + + // And no insert may be performed on a binding built from the anon client. + const anonInsert = source.match( + /const\s+(\w+)\s*=\s*getWaitlistSupabase\(\);[\s\S]*?await\s+\1\s*\.from\("waitlist"\)\s*\.insert\(/, + ); + expect(anonInsert).toBeNull(); + }); + + it("still uses the anon client only for the anon-granted RPC", () => { + const source = fs.readFileSync(ROUTE_PATH, "utf8"); + const schema = fs.readFileSync(SCHEMA_PATH, "utf8"); + + // getWaitlistSupabase() may remain for reads that go through a + // SECURITY DEFINER function anon is explicitly granted — that is the + // designed use of the publishable key and is not a write path. + const anonUses = source.match(/getWaitlistSupabase\(\)\s*\.\s*(\w+)/g) ?? []; + for (const use of anonUses) { + expect(use).toMatch(/\.rpc$/); + } + + // Guard the grant this depends on, so revoking it upstream is not silent. + expect(schema).toMatch( + /grant execute on function public\.waitlist_referral_code_exists\(text\) to anon;/, + ); + }); +}); diff --git a/app/app/api/waitlist/signup/route.ts b/app/app/api/waitlist/signup/route.ts index 9b125037a..4640d0728 100644 --- a/app/app/api/waitlist/signup/route.ts +++ b/app/app/api/waitlist/signup/route.ts @@ -758,7 +758,14 @@ export async function POST(req: Request) { // fresh random 8-char Crockford code; retry with a new one // • waitlist_pubkey_key or waitlist_email_unique_idx — the same user // re-submitting; idempotent, mark as duplicate and move on - const supabase = getWaitlistSupabase(); + // GH#2503: this insert must go through the SERVICE-ROLE client. The waitlist + // table's INSERT policy is scoped to service_role; the publishable/anon key + // has no write policy, because every anti-abuse control this route applies + // (referral code, Turnstile, honeypot, dwell time, disposable-email block, + // bot-UA filter, per-IP and per-code caps, signature verification) is only a + // boundary if this route is the ONLY write path. It was not, while anon could + // INSERT directly against PostgREST with a key that is public by design. + const supabase = getWaitlistServiceSupabase(); const baseRow: Record = { twitter_handle, source, diff --git a/supabase-waitlist-schema.sql b/supabase-waitlist-schema.sql index 82a952e2b..1e3af878d 100644 --- a/supabase-waitlist-schema.sql +++ b/supabase-waitlist-schema.sql @@ -6,7 +6,9 @@ -- end state. -- -- Design: --- - Anonymous users insert via the publishable key (RLS allows insert only). +-- - Inserts are server-only, via WAITLIST_SUPABASE_SERVICE_ROLE_KEY (GH#2503). +-- The publishable key can no longer write; it is only used for anon-callable +-- SECURITY DEFINER RPCs. -- - Server-side route /api/waitlist/signup verifies the wallet signature -- BEFORE inserting, so RLS-allowed inserts are gated on real ownership. -- - SELECT is denied to anon (privacy: don't leak the email-list-equivalent). @@ -216,12 +218,33 @@ alter table public.waitlist enable row level security; drop policy if exists "anon insert" on public.waitlist; drop policy if exists "deny select" on public.waitlist; +drop policy if exists "service_role insert" on public.waitlist; --- Anon can insert (server-side route validates the signature first). -create policy "anon insert" +-- GH#2503: writes are server-only. +-- +-- This previously read `to anon with check (true)` — an unconditional grant to +-- the role the PUBLISHABLE key maps to. Every anti-abuse control (referral-code +-- requirement, Turnstile, honeypot, dwell time, disposable-email block, bot-UA +-- filter, per-IP and per-code caps, wallet-signature verification) lives in +-- POST /api/waitlist/signup, so that route was being treated as the security +-- boundary while the database accepted writes from anyone holding a key that is +-- public by design. A direct PostgREST INSERT skipped all of it and could set +-- created_at (→ position #1, since waitlist_position orders by created_at ASC), +-- tier, referral_code and referred_by_code (→ leaderboard fraud) at will. +-- +-- Note on what actually closes it: the protection comes from REMOVING the anon +-- policy, not from adding the one below. With RLS enabled and no policy +-- applying to anon, anon INSERT is denied by default; service_role holds +-- BYPASSRLS and would write regardless. The policy is written out anyway so the +-- intended writer is stated in the schema rather than implied by an absence, +-- and so a future `create policy ... to anon` reads as the regression it is. +-- +-- The signup route uses getWaitlistServiceSupabase() for this insert. SELECT / +-- UPDATE / DELETE remain unpolicied for anon, as before. +create policy "service_role insert" on public.waitlist for insert - to anon + to service_role with check (true); -- Anon cannot read individual rows. Intentionally no select policy → @@ -325,13 +348,16 @@ grant execute on function public.waitlist_referral_code_exists(text) to anon; -- WHY IT'S TAMPER-RESISTANT: -- • referred_by_code is set at INSERT time by the signup route. The route -- never UPDATEs it afterwards. --- • The RLS policy on the waitlist table grants anon INSERT only — no --- UPDATE or DELETE policy exists, so anon cannot mutate the column. +-- • The RLS policy on the waitlist table grants INSERT to service_role only +-- (GH#2503) — anon holds no write policy at all, and no UPDATE or DELETE +-- policy exists for any role, so the column cannot be mutated after insert. -- • This function reads `count(*)` of rows that reference each code; an -- attacker would need INSERT access to a row with a chosen --- referred_by_code (which they have — but their inserted row is then --- subject to the same RLS, and the route validates `referred_by_code` --- points at a real code before accepting). They cannot decrement, edit +-- referred_by_code. Before GH#2503 they had exactly that: this rationale +-- leaned on "the route validates referred_by_code", but the route was not +-- the only write path — a publishable-key INSERT never reached it. With +-- writes scoped to service_role the route IS the only path, so that +-- sentence is now true rather than assumed. They cannot decrement, edit -- ownership of, or hide existing referrals. -- • Service role bypasses RLS but the service-role key lives in operator -- env (Vercel project + local .env), not in the browser bundle.