-
Notifications
You must be signed in to change notification settings - Fork 35
fix(waitlist): scope table writes to service_role — anon insert bypassed every control (GH#2503) #2504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(waitlist): scope table writes to service_role — anon insert bypassed every control (GH#2503) #2504
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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;/, | ||
| ); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Comment on lines
+221
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Drop the deployed legacy Line 221 drops only Drop the legacy 🤖 Prompt for AI Agents |
||
| 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. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: dcccrypto/percolator-launch
Length of output: 1935
🏁 Script executed:
Repository: dcccrypto/percolator-launch
Length of output: 9654
Parse waitlist write policies into role sets.
sqlStatements()only returnsCREATE POLICY, so anALTER POLICY public.waitlist ... TO anonchanges would not be included. Current text checks would also passTO service_role, anonbecause it containsto service_role. Scope checks topublic.waitlist, parse every policyTOlist, require exactlyservice_role, and includeALTER POLICYmutations in this guard.🤖 Prompt for AI Agents