Worked review 1: แอป SaaS multi-tenant
ฟีเจอร์
หัวข้อที่มีชื่อว่า “ฟีเจอร์”prompt ที่ให้ไปคือ: “เพิ่ม multi-tenant support: tenants, members และ projects ที่ scope ตาม tenant พร้อม RLS ให้ user เห็นแต่ข้อมูล tenant ตัวเอง บวก RPC สำหรับเชิญเพื่อนร่วมทีมด้วย email” สิ่งที่ตามมาคือ draft แรกแบบที่ AI assistant มักส่งกลับมาให้ตาม prompt นี้เป๊ะ — ทุก table สร้างได้, ทุก policy syntax ถูกต้อง, RPC compile ผ่าน และ manual test เร็ว ๆ ในฐานะ tenant owner ก็ทำงานได้จริง อ่านมันแบบที่ review checklist แนะนำ: schema กับ RLS ก่อน แล้วค่อย roles กับ keys แล้วค่อย client calls มันมี บั๊กจริง 10 ตัว — มากกว่าตัวอย่าง worked review ทั่วไปของ catalog โดยตั้งใจ เพราะแอปทรงนี้ (multi-tenant SaaS) คือที่ที่ความผิดพลาดแบบเดียวกันทบต้นข้าม table ได้ ดูที่ AI bug catalog สำหรับ pattern ทั่วไปเบื้องหลังส่วนใหญ่ (บั๊ก 1, 2, 4, 5, 6, 7, 8, 12/13 มี entry ตรงใน catalog อยู่แล้ว; สองตัวเป็นเรื่องเฉพาะของ multi-tenancy ที่โผล่ครั้งแรกที่นี่) ทุกข้อกล่าวอ้างด้านล่างถูกพิสูจน์บน local stack ของคอร์สนี้เอง (โปรเจกต์ probe, supabase status --workdir tools/probe, Postgres 17.6.x) — curl จริงยิง PostgREST กับ GoTrue ด้วย local JWT จริง, session psql จริง และ output explain analyze จริงบน table 50,000 แถว
โค้ดที่ AI สร้าง
หัวข้อที่มีชื่อว่า “โค้ดที่ AI สร้าง”-- reading-supabase/worked-review-1/tenants-migration.sql -- as generated, see review belowcreate table public.wr_tenants ( id uuid primary key default gen_random_uuid(), name text not null, created_at timestamptz not null default now());
create table public.wr_members ( tenant_id uuid not null references public.wr_tenants(id), user_id text not null references auth.users(id), role text not null default 'member', primary key (tenant_id, user_id));
create table public.wr_projects ( id uuid primary key default gen_random_uuid(), tenant_id uuid not null references public.wr_tenants(id), name text not null, created_by uuid references auth.users(id), created_at timestamptz not null default now());
alter table public.wr_projects enable row level security;
create policy wr_projects_select on public.wr_projects for select using ( tenant_id = ( select (raw_user_meta_data->>'tenant_id')::uuid from auth.users where id = auth.uid() ) );
create policy wr_projects_update on public.wr_projects for update using (true);-- no insert policy was ever written
create or replace function public.wr_invite_member(p_tenant_id uuid, p_email text, p_role text default 'member')returns public.wr_memberslanguage plpgsqlsecurity defineras $$declare v_user_id uuid; v_result public.wr_members;begin select id into v_user_id from auth.users where email = p_email; if v_user_id is null then raise exception 'no such user: %', p_email; end if; insert into wr_members (tenant_id, user_id, role) values (p_tenant_id, v_user_id, p_role) returning * into v_result; return v_result;end;$$;grant execute on function public.wr_invite_member(uuid, text, text) to authenticated;
create or replace function public.wr_handle_new_user()returns triggerlanguage plpgsqlas $$begin insert into public.wr_profiles (id, display_name) values (new.id, new.email); return new;end;$$;
create trigger wr_on_auth_user_created after insert on auth.users for each row execute function public.wr_handle_new_user();// reading-supabase/worked-review-1/tenant-client.ts -- as generated, see review belowimport { createClient } from '@supabase/supabase-js';
// AI comment: "service_role bypasses RLS so tenant switching always works"export const supabase = createClient( process.env.SUPABASE_URL!, process.env.SUPABASE_SERVICE_ROLE_KEY!,);
export async function getFirstProject(tenantId: string) { const { data } = await supabase .from('wr_projects') .select('*') .eq('tenant_id', tenantId) .single(); return data;}เดินรีวิวทีละจุด
หัวข้อที่มีชื่อว่า “เดินรีวิวทีละจุด”user_id text อ้างอิง auth.users(id) — migration รันไม่ได้ตั้งแต่ต้น
หัวข้อที่มีชื่อว่า “user_id text อ้างอิง auth.users(id) — migration รันไม่ได้ตั้งแต่ต้น”auth.users.id เป็น uuid การประกาศ wr_members.user_id เป็น text แล้วชี้ foreign key ไปหามัน ถูกปฏิเสธก่อนที่จะมีแถวไหนถูกเขียนเลยด้วยซ้ำ — พิสูจน์ด้วย create table จริง:
-- @expect-error foreign key constraint "wr_members_user_id_fkey" cannot be implementedcreate table public.wr_members ( tenant_id uuid not null references public.wr_tenants(id), user_id text not null references auth.users(id), role text not null default 'member', primary key (tenant_id, user_id));ERROR: foreign key constraint "wr_members_user_id_fkey" cannot be implementedDETAIL: Key columns "user_id" and "id" are of incompatible types: text and uuid.🐛 หา bug ในโค้ดนี้
user_id text not null references auth.users(id),เฉลย
นี่คือบั๊กเดียวในรีวิวนี้ที่ไม่มีวันไปถึง production — Postgres ปฏิเสธการสร้าง constraint ไปเลย ทำให้ supabase db push/--apply fail ดัง ๆ ใน CI ก่อนที่ RLS policy, RPC หรือ client call ไหน ๆ จะถูกเขียนถึง table นี้ด้วยซ้ำ ใส่ไว้ในรีวิวนี้เพราะมันเป็นนิสัยของ AI ที่พบบ่อยจริง ๆ: ใช้ text เป็น default กับทุกคอลัมน์ที่หน้าตาเป็น id แทนที่จะ match กับ primary key type จริงของ table ที่อ้างถึง (auth.users.id เป็น uuid เสมอ) วิธีแก้คือ user_id uuid not null references auth.users(id) on delete cascade — type ตรงกัน และดูบั๊กถัดไปว่าทำไม on delete cascade ถึงสำคัญด้วย
ไม่มี on delete cascade — ลบ tenant แล้ว fail ทันที
หัวข้อที่มีชื่อว่า “ไม่มี on delete cascade — ลบ tenant แล้ว fail ทันที”ไม่มี foreign key ไหนใน schema ที่ AI สร้างระบุ on delete cascade เลย Postgres เลย default เป็น no action พิสูจน์ด้วยการลอง delete tenant ที่มี project อยู่แล้ว:
-- ran directly against this review's schema, see output belowdelete from public.wr_tenants where id = '11111111-1111-1111-1111-111111111111';ERROR: update or delete on table "wr_tenants" violates foreign key constraint "wr_projects_tenant_id_fkey" on table "wr_projects"DETAIL: Key (id)=(11111111-1111-1111-1111-111111111111) is still referenced from table "wr_projects".🐛 หา bug ในโค้ดนี้
tenant_id uuid not null references public.wr_tenants(id),เฉลย
“ลบ tenant แล้ว fail ดัง ๆ” ฟังดูเหมือน safety feature จนกว่าจะเป็น code path เดียวที่ support engineer ต้องรีบทำภายใต้ความกดดันเวลา — และ on delete cascade ที่หายไปแบบเดียวกันบน wr_members.tenant_id/user_id หมายความว่าการลบ user account ตัวเดียว (auth.admin.deleteUser) จะทิ้ง membership row ที่กำพร้าไว้เงียบ ๆ แทน ซึ่งอันตรายกว่าในทิศทางตรงข้าม วิธีแก้คือใส่ on delete cascade บนทุก foreign key ที่ scope ตาม tenant อย่างตั้งใจ (ลบ tenant แล้ว member กับ project ของมันหายไปด้วย) แทนที่จะปล่อยไว้ที่ default ของ Postgres
เช็ค tenant ด้วยการอ่าน auth.users ตรง ๆ — policy มองไม่เห็นแถวไหนเลย ไม่ว่าใครก็ตาม
หัวข้อที่มีชื่อว่า “เช็ค tenant ด้วยการอ่าน auth.users ตรง ๆ — policy มองไม่เห็นแถวไหนเลย ไม่ว่าใครก็ตาม”policy select ที่ AI สร้างบน wr_projects อ่าน auth.users.raw_user_meta_data เพื่อหา tenant ของผู้เรียก auth.users เป็นของ supabase_auth_admin; ทั้ง anon และ authenticated ไม่ได้รับสิทธิ์อะไรบนมันเลย — ยืนยันตรง ๆ:
-- ran directly against this review's schema, see output belowselect grantee, privilege_type from information_schema.role_table_grantswhere table_schema = 'auth' and table_name = 'users' and grantee in ('anon', 'authenticated', 'service_role');(0 rows)พิสูจน์ด้วย request จริงในฐานะ member จริงของ tenant Globex ที่กำลังอ่าน project ของ tenant ตัวเอง:
curl -s "$SUPABASE_URL/rest/v1/wr_projects?select=name,tenant_id" \ -H "apikey: $SUPABASE_ANON_KEY" -H "Authorization: Bearer $BOB_TOKEN"{"code":"42501","details":null,"hint":"Grant the required privileges to the current role with: GRANT SELECT ON auth.users TO authenticated;","message":"permission denied for table users"}🐛 หา bug ในโค้ดนี้
using (tenant_id = ( select (raw_user_meta_data->>'tenant_id')::uuid from auth.users where id = auth.uid()))เฉลย
มีปัญหาแยกกันสองอย่างซ้อนอยู่ใน clause เดียว อย่างแรก อันที่พิสูจน์ไว้ข้างบน: authenticated ไม่มี grant อะไรเลยบน auth.users ในโปรเจกต์ Supabase มาตรฐาน ดังนั้น policy นี้ปฏิเสธทุกคนที่เรียก ตลอดเวลา — ไม่ใช่ “tenant ผิด” แต่คือ 42501 แข็ง ๆ บน query ที่ไม่ทันได้ไปถึงขั้น RLS filter ด้วยซ้ำ อย่างที่สอง ต่อให้ข้ามเรื่อง permission error ไป raw_user_meta_data ก็คือสิ่งที่ supabase.auth.updateUser({ data: { ... } }) เขียนพอดี — ผู้เรียกตั้ง tenant_id ของตัวเองเป็นค่าอะไรก็ได้ตามใจ ดังนั้นแม้แต่เวอร์ชัน security definer ของ query เดียวกันนี้ก็จะยอมให้ user self-assign membership เข้า tenant ไหนก็ได้ วิธีแก้คือ table wr_members เฉพาะที่เป็น single source of truth ของ tenant membership เช็คผ่าน helper security definer (wr_is_member(tenant_id)) — ไม่ใช้ metadata ของ auth.users สำหรับการตัดสินใจเรื่อง authorization เด็ดขาด
using (true) ทั้ง select และ update ไม่มี with check — cross-tenant hijack เต็มรูปแบบ
หัวข้อที่มีชื่อว่า “using (true) ทั้ง select และ update ไม่มี with check — cross-tenant hijack เต็มรูปแบบ”table wr_members ที่ AI สร้าง (ไม่ได้โชว์ด้านบน — สร้างเหมือน wr_projects เป๊ะ select/update เป็น using (true) ทั้งคู่) ทำผิดพลาดแบบเดียวกับที่ catalog บั๊ก 5 อธิบายไว้ แต่ข้าม tenant แทนที่จะข้ามแค่แถวใน table เดียว พิสูจน์ด้วย PATCH ข้าม tenant จริง: Alice (owner ของ Acme, tenant 1111…) ย้าย membership row ของ Bob เองจาก Globex เข้ามาที่ tenant ของตัวเอง เป็น owner:
curl -s -w "\nHTTP=%{http_code}\n" -X PATCH "$SUPABASE_URL/rest/v1/wr_members?user_id=eq.$BOB_ID" \ -H "apikey: $SUPABASE_ANON_KEY" -H "Authorization: Bearer $ALICE_TOKEN" \ -H "Content-Type: application/json" -H "Prefer: return=representation" \ -d '{"tenant_id":"11111111-1111-1111-1111-111111111111","role":"owner"}'[{"tenant_id":"11111111-1111-1111-1111-111111111111","user_id":"911ac6cd-81d5-428a-ab9b-d303f6cb60dd","role":"owner"}]HTTP=200ยืนยันฝั่ง server ในฐานะ postgres: แถวจริงของ Bob (ไม่มี SQLSTATE ไม่มี error ตรงไหนใน chain นี้เลย) ตอนนี้มี tenant_id เป็น Acme — Alice ให้สิทธิ์ตัวเอง และเอา Bob ออกจากบริษัทของเขาเอง ด้วย PATCH request เดียวที่ไม่ได้พูดถึง tenant ของ Bob เลยแม้แต่คำเดียว
🐛 หา bug ในโค้ดนี้
create policy wr_members_select on public.wr_membersfor select using (true);
create policy wr_members_update on public.wr_membersfor update using (true);-- no with check -- reused from using(true), so it is also trueเฉลย
select using (true) อ่านได้ว่า “team directory มองเห็นได้ทุกคน” — เป็นทางเลือกที่ดูสมเหตุสมผล อาจถึงขั้นตั้งใจ แต่พอจับคู่กับ update using (true) และไม่มี with check มันไม่ใช่เรื่อง visibility อีกต่อไป กลายเป็น “authenticated user คนไหนก็ได้ ใน tenant ไหนก็ได้ เขียน membership row ไหนให้เป็นรูปร่างอะไรก็ได้” — พิสูจน์ไว้ข้างบนด้วยการ reassign แถวที่ Alice ไม่มีความสัมพันธ์อะไรด้วยเลยจริง ๆ วิธีแก้ด้านล่างไม่ใช่การเขียน using clause ให้แคบลงบน update — แต่คือไม่มี update policy ทั่วไปบน table นี้เลย: การเปลี่ยน role กับ tenant ต้องอยู่หลัง RPC ที่เช็คสิทธิ์ (คู่ที่เหมาะกับ wr_invite_member คือ wr_set_member_role ที่ gate ด้วยเงื่อนไขว่าผู้เรียกต้องเป็น owner อยู่แล้ว) ไม่ใช่ update ทั้ง row แบบเหมารวมเด็ดขาด
security definer RPC ไม่มี set search_path = '' — insert ไปลงที่ table ของผู้โจมตีเงียบ ๆ
หัวข้อที่มีชื่อว่า “security definer RPC ไม่มี set search_path = '' — insert ไปลงที่ table ของผู้โจมตีเงียบ ๆ”wr_invite_member เป็น security definer ไม่มี set search_path owner ของ function (postgres) ไม่แตะ schema ไหนนอกจาก public เลย — แต่ search_path ของ session ฝั่งผู้เรียก ต่างหากที่ resolve การอ้างอิง wr_members แบบไม่ qualify schema ของ function ตอนเรียกจริง:
-- ran directly against this review's schema, see output belowcreate schema wr_evil;create table wr_evil.wr_members (tenant_id uuid, user_id uuid, role text);set search_path to wr_evil, public;select public.wr_invite_member('11111111-1111-1111-1111-111111111111', '[email protected]', 'owner');select * from wr_evil.wr_members;select count(*) as real_table_untouched from public.wr_members where user_id = '911ac6cd-81d5-428a-ab9b-d303f6cb60dd' and tenant_id = '11111111-1111-1111-1111-111111111111';current_setting-----------------wr_evil, public
wr_invite_member----------------------------------------------------------------------------------- (11111111-1111-1111-1111-111111111111,911ac6cd-81d5-428a-ab9b-d303f6cb60dd,owner)(1 row)
tenant_id | user_id | role-----------+---------+------ 1111... | 911a... | owner
real_table_untouched----------------------- 0returning * ของ function เองรายงานว่าสำเร็จ — แถวที่หน้าตาถูกต้องเป๊ะ — ในขณะที่ insert จริง (และแถวที่มันคืนมา) ทั้งคู่ resolve ไปที่ wr_evil.wr_members ไม่ใช่ table จริง public.wr_members ไม่ได้รับการเขียนนี้เลยแม้แต่นิดเดียว
🐛 หา bug ในโค้ดนี้
create or replace function public.wr_invite_member(p_tenant_id uuid, p_email text, p_role text default 'member')returns public.wr_memberslanguage plpgsqlsecurity defineras $$declarev_user_id uuid;v_result public.wr_members;beginselect id into v_user_id from auth.users where email = p_email;insert into wr_members (tenant_id, user_id, role) values (p_tenant_id, v_user_id, p_role)returning * into v_result;return v_result;end;$$;เฉลย
นี่คือฝาแฝดฝั่งเขียนของ entry 7 ใน AI bug catalog (ที่รั่วข้อมูลตอนอ่าน) — ที่นี่ set search_path = '' ที่หายไปตัวเดียวกันทำให้การเขียนถูกเบี่ยงเงียบ ๆ ซึ่งแย่กว่า: ไม่มีอะไรใน response ของ RPC ดูผิดปกติเลย ทำให้ไม่มีใคร review response แล้วจะสังเกตได้ว่า membership table จริงไม่เคยถูกแตะเลย วิธีแก้คือ set search_path = '' บวกกับ qualify schema ทุกจุดที่อ้างอิง (public.wr_members) พิสูจน์ด้านล่างว่า attacker search_path เดิมเป๊ะ ๆ ลงไปที่ table จริงได้ไม่ว่าจะยังไง
เปิด RLS แล้วแต่ไม่มี insert policy บน wr_projects
หัวข้อที่มีชื่อว่า “เปิด RLS แล้วแต่ไม่มี insert policy บน wr_projects”wr_projects มี select กับ update policy; ไม่มีใครเขียนของ insert เลย พิสูจน์ด้วย member จริงของ Acme ที่พยายามสร้าง project ของ tenant ตัวเอง:
curl -s -w "\nHTTP=%{http_code}\n" -X POST "$SUPABASE_URL/rest/v1/wr_projects" \ -H "apikey: $SUPABASE_ANON_KEY" -H "Authorization: Bearer $ALICE_TOKEN" \ -H "Content-Type: application/json" -H "Prefer: return=representation" \ -d '{"tenant_id":"11111111-1111-1111-1111-111111111111","name":"acme-new-project","created_by":"e0d41eff-5ce4-48ac-b48c-2fa3722d230d"}'{"code":"42501","details":null,"hint":null,"message":"new row violates row-level security policy for table \"wr_projects\""}HTTP=403🐛 หา bug ในโค้ดนี้
alter table public.wr_projects enable row level security;create policy wr_projects_select on public.wr_projects for select using (...);create policy wr_projects_update on public.wr_projects for update using (true);-- no insert policy was ever writtenเฉลย
ตรงกับ ai-bug-catalog entry 4 เป๊ะ — ช่องโหว่ RLS ที่พบบ่อยที่สุด เพราะ select คือสิ่งแรกที่ทุกคน test ด้วยมือ ส่วน insert แทบไม่มีใคร test จนกว่า user จริงจะกด “Create project” บน UI วิธีแก้คือ policy หนึ่งอันต่อหนึ่งคำสั่งที่ table ต้องใช้จริง: for insert with check ( public.wr_is_member(tenant_id) )
key service_role ถูกใส่เข้าไปใน client module ตรง ๆ
หัวข้อที่มีชื่อว่า “key service_role ถูกใส่เข้าไปใน client module ตรง ๆ”tenant-client.ts สร้าง createClient ด้วย SUPABASE_SERVICE_ROLE_KEY พร้อม comment ให้เหตุผลว่าเป็นทางแก้ให้ “tenant switching” พิสูจน์ด้วย key เดียวกันเป๊ะ กับ select * ข้ามทุก tenant ไม่มีการ scope Authorization เลย:
curl -s "$SUPABASE_URL/rest/v1/wr_projects?select=name,tenant_id" \ -H "apikey: $SUPABASE_SERVICE_ROLE_KEY" -H "Authorization: Bearer $SUPABASE_SERVICE_ROLE_KEY"[{"name":"acme-roadmap","tenant_id":"11111111-1111-1111-1111-111111111111"}, {"name":"globex-roadmap","tenant_id":"22222222-2222-2222-2222-222222222222"}]🐛 หา bug ในโค้ดนี้
export const supabase = createClient(process.env.SUPABASE_URL!,process.env.SUPABASE_SERVICE_ROLE_KEY!,);เฉลย
service_role bypass RLS ทั้งหมด — พิสูจน์ไว้ข้างบนว่าคืน project ของทั้งสอง tenant ในการเรียกครั้งเดียว ไม่มี policy ไหนถูกประเมินเลย “tenant switching always works” เป็นจริง และนั่นคือปัญหาทั้งหมด: มันทำงานเพราะไม่เหลือ boundary ของ tenant ให้ switch ระหว่าง กันแล้ว ถ้า tenant-client.ts ไปอยู่ใน browser (หรือ code path ไหนก็ตามที่ traffic ของ end user ไปถึง) ข้อมูลของทุก tenant ก็ไปด้วย วิธีแก้คือ key anon บวกกับ JWT ของผู้เรียกจริงในทุก client ที่ไม่ใช่ trusted server context — service_role อยู่ฝั่ง server เท่านั้น ตาม ai-bug-catalog entry 25
.single() บน query ที่คืนศูนย์แถวได้อย่างถูกต้องตามปกติ
หัวข้อที่มีชื่อว่า “.single() บน query ที่คืนศูนย์แถวได้อย่างถูกต้องตามปกติ”getFirstProject เรียก .single() — ซึ่งขอ PostgREST ด้วย Accept: application/vnd.pgrst.object+json — กับ tenant ที่ยังไม่มี project เลย พิสูจน์ตรง ๆ:
curl -s -w "\nHTTP=%{http_code}\n" "$SUPABASE_URL/rest/v1/wr_projects?select=*&name=eq.does-not-exist" \ -H "apikey: $SUPABASE_ANON_KEY" -H "Authorization: Bearer $ALICE_TOKEN" \ -H "Accept: application/vnd.pgrst.object+json"{"code":"PGRST116","details":"The result contains 0 rows","hint":null,"message":"Cannot coerce the result to a single JSON object"}HTTP=406🐛 หา bug ในโค้ดนี้
const { data } = await supabase.from('wr_projects').select('*').eq('tenant_id', tenantId).single();return data;เฉลย
.single() compile ผ่านสะอาดและทำงานได้ทุก happy path — บั๊กทั้งหมดอยู่ที่มันทำอะไรกับ tenant ใหม่เอี่ยมที่ยังไม่มี project เลย ซึ่งตรงกับ ai-bug-catalog entry 13 เป๊ะ: 406/PGRST116 จริง และ — เพราะ error ถูก destructure ทิ้งไปด้วยที่นี่ — เป็น error ที่ไม่ถูกจับด้วย วิธีแก้คือ .maybeSingle() (คืน data: null เมื่อศูนย์แถว ไม่มี error) เมื่อศูนย์แถวเป็นผลลัพธ์ที่ถูกต้องได้ บวกกับเช็ค error ไม่ว่าจะทางไหน
auth.uid() ไม่ถูก wrap — พิสูจน์ด้วย explain analyze จริงบน 50,000 แถว
หัวข้อที่มีชื่อว่า “auth.uid() ไม่ถูก wrap — พิสูจน์ด้วย explain analyze จริงบน 50,000 แถว”iteration หลังของ schema นี้เพิ่ม activity log, wr_activity_perf, ด้วย for select using ( created_by = auth.uid() ) — ไม่ wrap table 50,000 แถวเดียวกัน, JWT claim เดียวกัน, ไม่ wrap เทียบกับ wrap ใน (select auth.uid()):
-- unwrapped: auth.uid() re-evaluated once per row via current_setting()Seq Scan on wr_activity_perf (actual time=0.007..17.032 rows=50000 loops=1) Filter: (created_by = (COALESCE(NULLIF(current_setting('request.jwt.claim.sub', true), ''), ...))::uuid)Execution Time: 18.634 ms
-- wrapped in (select auth.uid()): computed once as an InitPlan, reused for every rowInitPlan 1 -> Result (actual time=0.002..0.003 rows=1 loops=1)Seq Scan on wr_activity_perf (actual time=0.009..2.424 rows=50000 loops=1) Filter: (created_by = (InitPlan 1).col1)Execution Time: 3.569 msประมาณ 5.2 เท่า บน table เดียวกันเป๊ะ กับ Mac เครื่องเดียวกันเป๊ะ — พิสูจน์รูปแบบเดียวกับ ai-bug-catalog entry 6 ทำซ้ำที่นี่เพราะ table activity/audit คือจุดที่แอป multi-tenant สะสมจำนวนแถวจนช่องว่างนี้เห็นชัดพอดี
🐛 หา bug ในโค้ดนี้
create policy wr_activity_perf_unwrapped on public.wr_activity_perffor select using ( created_by = auth.uid() );เฉลย
วิธีแก้คือวงเล็บล้วน ๆ: using ( created_by = (select auth.uid()) ) — ไม่มีอะไรอื่นใน policy เปลี่ยนเลย แล้ว planner ก็เปลี่ยนการเรียก function ต่อแถวให้กลายเป็น InitPlan ครั้งเดียว
profile trigger บน auth.users ไม่มี security definer
หัวข้อที่มีชื่อว่า “profile trigger บน auth.users ไม่มี security definer”wr_handle_new_user() insert เข้า public.wr_profiles ทุกครั้งที่มีการ signup ใหม่ แต่ไม่ใช่ security definer การ insert เข้า auth.users ทำโดย supabase_auth_admin (role ของ GoTrue เอง) — ซึ่งไม่มี grant อะไรบน public.wr_profiles เลย พิสูจน์ด้วยการ signup จริง:
curl -s -w "\nHTTP=%{http_code}\n" -X POST "$SUPABASE_URL/auth/v1/signup" \ -H "apikey: $SUPABASE_ANON_KEY" -H "Content-Type: application/json" \{"code":500,"error_code":"unexpected_failure","msg":"Database error saving new user","error_id":"e4f8a3db-653a-476d-927c-9ca46c758014"}HTTP=500error จริงเบื้องหลัง จาก log ของ supabase_auth_probe เองสำหรับ request_id นั้น:
error: failed to close prepared statement: ERROR: current transaction is aborted, commands ignored until end of transaction block (SQLSTATE 25P02): ERROR: permission denied for table wr_profiles (SQLSTATE 42501)🐛 หา bug ในโค้ดนี้
create or replace function public.wr_handle_new_user()returns triggerlanguage plpgsqlas $$begininsert into public.wr_profiles (id, display_name) values (new.id, new.email);return new;end;$$;เฉลย
นี่คือบั๊กเดียวในรีวิวนี้ที่ blast radius กว้างที่สุด: trigger บน auth.users รันบนทุกการ signup ดังนั้นนี่ไม่ใช่ “ฟีเจอร์ profile ใหม่พัง” — แต่คือ “ไม่มีใคร signup ได้อีกต่อไป” outage เต็มรูปแบบจาก function ที่หน้าตาธรรมดาที่สุด security definer คือทางแก้ที่ตรงจุด (function จะรันด้วยสิทธิ์ของ owner คือ postgres) — พิสูจน์ด้านล่างด้วย signup เดิมเป๊ะที่สำเร็จทันทีที่เพิ่มเข้าไป พร้อมกับ set search_path = '' เพราะโครงสร้างนี้เป็นบั๊กประเภทเดียวกับ RPC ด้านบนเป๊ะ
โค้ดที่แก้แล้ว
หัวข้อที่มีชื่อว่า “โค้ดที่แก้แล้ว”-- supabase/migrations/7401_wr_saas_schema.sqlcreate table public.wr_tenants ( id uuid primary key default gen_random_uuid(), name text not null, created_at timestamptz not null default now());
create table public.wr_members ( tenant_id uuid not null references public.wr_tenants(id) on delete cascade, user_id uuid not null references auth.users(id) on delete cascade, role text not null default 'member', primary key (tenant_id, user_id));
create table public.wr_projects ( id uuid primary key default gen_random_uuid(), tenant_id uuid not null references public.wr_tenants(id) on delete cascade, name text not null, created_by uuid references auth.users(id) on delete set null, created_at timestamptz not null default now());
create table public.wr_profiles ( id uuid primary key references auth.users(id) on delete cascade, display_name text);
-- security definer + search_path pinned: the ONLY place membership is checked fromcreate or replace function public.wr_is_member(p_tenant_id uuid)returns booleanlanguage sqlsecurity definerset search_path = ''stableas $$ select exists ( select 1 from public.wr_members m where m.tenant_id = p_tenant_id and m.user_id = auth.uid() );$$;
create or replace function public.wr_invite_member(p_tenant_id uuid, p_email text, p_role text default 'member')returns public.wr_memberslanguage plpgsqlsecurity definerset search_path = ''as $$declare v_user_id uuid; v_result public.wr_members;begin if not public.wr_is_member(p_tenant_id) then raise exception 'not a member of this tenant'; end if; select id into v_user_id from auth.users where email = p_email; if v_user_id is null then raise exception 'no such user: %', p_email; end if; insert into public.wr_members (tenant_id, user_id, role) values (p_tenant_id, v_user_id, p_role) returning * into v_result; return v_result;end;$$;grant execute on function public.wr_invite_member(uuid, text, text) to authenticated;
create or replace function public.wr_handle_new_user()returns triggerlanguage plpgsqlsecurity definerset search_path = ''as $$begin insert into public.wr_profiles (id, display_name) values (new.id, new.email); return new;end;$$;
create trigger wr_on_auth_user_created after insert on auth.users for each row execute function public.wr_handle_new_user();
alter table public.wr_tenants enable row level security;alter table public.wr_members enable row level security;alter table public.wr_projects enable row level security;alter table public.wr_profiles enable row level security;
create policy wr_members_select on public.wr_members for select using ( public.wr_is_member(tenant_id) );-- deliberately no general update policy -- role/tenant changes only via wr_invite_member
create policy wr_projects_select on public.wr_projects for select using ( public.wr_is_member(tenant_id) );
create policy wr_projects_insert on public.wr_projects for insert with check ( public.wr_is_member(tenant_id) );
create policy wr_projects_update on public.wr_projects for update using ( public.wr_is_member(tenant_id) ) with check ( public.wr_is_member(tenant_id) );
create policy wr_profiles_select on public.wr_profiles for select using ( id = (select auth.uid()) );// @runimport { createClient } from '@supabase/supabase-js';
const url = process.env.SUPABASE_URL!;const anonKey = process.env.SUPABASE_ANON_KEY!;const serviceKey = process.env.SUPABASE_SERVICE_ROLE_KEY!;const admin = createClient(url, serviceKey);const stamp = Date.now();
async function makeTenantAndOwner(tenantName: string, email: string) { const { data: tenant, error: tErr } = await admin .from('wr_tenants') .insert({ name: tenantName }) .select() .single(); if (tErr) throw tErr;
const { data: created, error: uErr } = await admin.auth.admin.createUser({ email, password: 'wr-Passw0rd!', email_confirm: true, }); if (uErr) throw uErr;
const { error: mErr } = await admin .from('wr_members') .insert({ tenant_id: tenant.id, user_id: created.user.id, role: 'owner' }); if (mErr) throw mErr;
// A fresh, throwaway client for sign-in -- calling signInWithPassword on // `admin` itself would swap ITS session to this user's, so every later // admin.from(...) call would run as that user (RLS-scoped), not service_role. const signInClient = createClient(url, anonKey); const { data: signIn, error: sErr } = await signInClient.auth.signInWithPassword({ email, password: 'wr-Passw0rd!', }); if (sErr) throw sErr;
return { tenant, userId: created.user.id, token: signIn.session!.access_token };}
async function main() { const acme = await makeTenantAndOwner('wr-run-acme', `wr+run-alice-${stamp}@example.com`); const globex = await makeTenantAndOwner('wr-run-globex', `wr+run-bob-${stamp}@example.com`);
await admin.from('wr_projects').insert([ { tenant_id: acme.tenant.id, name: 'acme-roadmap', created_by: acme.userId }, { tenant_id: globex.tenant.id, name: 'globex-roadmap', created_by: globex.userId }, ]);
const aliceClient = createClient(url, anonKey, { global: { headers: { Authorization: `Bearer ${acme.token}` } }, }); const bobClient = createClient(url, anonKey, { global: { headers: { Authorization: `Bearer ${globex.token}` } }, });
const { data: aliceSees } = await aliceClient.from('wr_projects').select('name'); const { data: bobSees } = await bobClient.from('wr_projects').select('name');
if (aliceSees?.length !== 1 || aliceSees[0].name !== 'acme-roadmap') { throw new Error(`isolation broken: alice saw ${JSON.stringify(aliceSees)}`); } if (bobSees?.length !== 1 || bobSees[0].name !== 'globex-roadmap') { throw new Error(`isolation broken: bob saw ${JSON.stringify(bobSees)}`); } console.log('alice sees only', aliceSees.map((p) => p.name)); console.log('bob sees only', bobSees.map((p) => p.name));
const { error: hijackErr, data: hijackData } = await aliceClient .from('wr_members') .update({ tenant_id: acme.tenant.id, role: 'owner' }) .eq('user_id', globex.userId) .select(); if (hijackErr) throw hijackErr; if (hijackData && hijackData.length > 0) { throw new Error('cross-tenant hijack succeeded -- fix regressed'); } console.log('cross-tenant membership hijack correctly blocked, 0 rows matched');
await admin.auth.admin.deleteUser(acme.userId); await admin.auth.admin.deleteUser(globex.userId); await admin.from('wr_tenants').delete().in('id', [acme.tenant.id, globex.tenant.id]);
console.log('two-tenant isolation test passed');}
main().catch((err) => { console.error(err); throw err;});รันจริงกับ local stack ของคอร์สนี้เอง (--run ของ harness ทำซ้ำอันนี้ตอน gate):
alice sees only [ 'acme-roadmap' ]bob sees only [ 'globex-roadmap' ]cross-tenant membership hijack correctly blocked, 0 rows matchedtwo-tenant isolation test passedสิ่งที่เปลี่ยนและทำไม
หัวข้อที่มีชื่อว่า “สิ่งที่เปลี่ยนและทำไม”| # | บั๊ก | ทางแก้ |
|---|---|---|
| 1 | user_id text อ้างอิง auth.users(id) | user_id uuid — ไม่งั้นได้ ERROR: foreign key constraint ... cannot be implemented จริง |
| 2 | ไม่มี on delete cascade บน FK ที่ scope ตาม tenant | เพิ่มทั่วทุกจุด — พิสูจน์แล้ว: ลบ tenant ที่มี project fail ด้วย FK violation จริง |
| 3 | เช็ค tenant ด้วย auth.users.raw_user_meta_data | table wr_members เฉพาะ + helper wr_is_member() — พิสูจน์แล้ว: ของเดิมปฏิเสธทุกคนด้วย 42501 เพราะไม่มี grant บน auth.users |
| 4 | using (true) บน select/update ไม่มี with check บน wr_members | ไม่มี update policy ทั่วไปเลย — พิสูจน์แล้ว: Alice ย้าย membership row ของ Bob ข้าม tenant ได้ใน PATCH เดียว |
| 5 | security definer RPC ไม่มี set search_path = '' | set search_path = '' + qualify schema — พิสูจน์แล้ว: attacker search_path เบี่ยง insert ไปที่ shadow table ได้เงียบ ๆ ถ้าไม่แก้ |
| 6 | ไม่มี insert policy บน wr_projects | for insert with check ( wr_is_member(tenant_id) ) — ไม่งั้นได้ 42501 จริง |
| 7 | key service_role อยู่ใน client module | key anon + JWT ของผู้เรียก — พิสูจน์แล้ว: service_role คืนข้อมูลทุก tenant ในครั้งเดียว ไม่มี scope เลย |
| 8 | .single() บน query ที่คืนศูนย์แถวได้ | .maybeSingle() — ไม่งั้นได้ PGRST116/406 จริง |
| 9 | auth.uid() ไม่ wrap ใน policy ที่ถูกเรียกบ่อย | (select auth.uid()) — พิสูจน์แล้วประมาณ 5.2 เท่า จาก explain analyze จริงบน 50,000 แถว |
| 10 | trigger บน auth.users ไม่มี security definer | เพิ่มแล้ว — พิสูจน์แล้ว: ทุก signup fail ด้วย permission denied for table wr_profiles ถ้าไม่แก้ |
สิ่งที่ reviewer เขียนใน PR
หัวข้อที่มีชื่อว่า “สิ่งที่ reviewer เขียนใน PR”อันนี้ merge ตามสภาพไม่ได้ — มี 10 เรื่อง จัดกลุ่มตามว่ามันจะกัดตอนไหนจริง ๆ:
deploy ไม่ผ่านด้วยซ้ำ:
wr_members.user_id textอ้างอิงauth.users(id)(uuid) —create tableเองก็ fail ใช้uuidทำให้ signup พังทุกครั้งทันที: 2. trigger สร้าง profile บน
auth.usersไม่ใช่security definer— นี่คือ outage เต็มรูปแบบ ไม่ใช่บั๊กแค่ฟีเจอร์เดียว เพิ่มsecurity definer set search_path = ''ข้อมูลรั่ว/เสียหายแบบเงียบ ๆ: 3. policy เลือก tenant อ่าน
auth.usersตรง ๆ —authenticatedไม่มี grant บนมันเลย ปฏิเสธ user จริงทุกคนตรง ๆ ใช้ tablewr_members4.wr_membersเป็นusing (true)ทั้งselectและupdateไม่มีwith check— ผมย้าย member row ของอีก tenant มาพิสูจน์แล้ว ตัด update policy ทิ้ง แล้วเปลี่ยน role/tenant ผ่าน invite RPC เท่านั้น 5.wr_invite_memberเป็นsecurity definerไม่มีset search_path— ผมเบี่ยง insert ของมันไปที่ shadow table ได้ด้วยแค่เปลี่ยน sessionsearch_pathpin มันซะ 6.service_roleอยู่ในtenant-client.ts— ถ้าโค้ดนี้ไปอยู่ตรงไหนที่ traffic ของ user ไปถึง ข้อมูลทุก tenant ก็ไปด้วยจะ 403/500 กับ user จริงคนแรก: 7. ไม่มี insert policy บน
wr_projects— สร้าง project แล้ว 403 8..single()กับ tenant ที่ไม่มี project เลย —406/PGRST116จริง ไม่ถูกจับด้วยเพราะไม่ได้ destructureerrorเลยด้วยซ้ำ 9. ไม่มีon delete cascadeที่ไหนเลย — ลบ tenant จะ fail ใน prod เหมือนที่ fail ที่นี่เป๊ะจะช้าลง ไม่ใช่ผิด: 10.
auth.uid()ไม่ wrap บน policy ของ activity log — วันนี้ก็ช้าประมาณ 5 เท่าแล้วที่ 50k แถว แย่ลงเรื่อย ๆ ตามจำนวนแถว wrap มันซะยินดี pair ด้วย — ไม่มีอันไหนต้อง redesign เลย ส่วนใหญ่แค่ย้ายจุดเช็คไปที่ถูกที่
Mental Model
หัวข้อที่มีชื่อว่า “Mental Model”สามในสิบบั๊กนี้ไม่เคยแสดง error อะไรเลยตอน manual test แบบ happy-path (3, 4, และกรณี service_role ของ 7) — มันดูเหมือนมี access มากกว่า ที่ตั้งใจ หรือ access ที่ถูกต้องเป๊ะ จนกว่าจะมีผู้เรียกคนใดคนหนึ่งหรือแถวใดแถวหนึ่งเฉพาะถูกลอง ที่เหลือแบ่งชัดเจนตาม เวลา ที่มันโผล่: ตัวหนึ่ง (1) ถูก Postgres เองจับก่อน deploy; สองตัว (2, 10) ถูกต้องแต่ช้าหรือเปราะบางมากกว่าจะผิด; และอีกสี่ตัวที่เหลือ (6, 8, 9 และ trigger ซึ่งคือ 10 ในตาราง) เงียบจนกว่า operation ตรงจุดนั้นจริง ๆ — การ insert, การ lookup ที่ได้ศูนย์แถว, การลบ tenant, การ signup — จะไปแตะ path ที่หายไป แอป multi-tenant คูณปัญหานี้ทวีคูณ: ความผิดพลาดแบบไม่มี insert policy ที่เป็นบั๊กเดียวในแอป blog table เดียว กลายเป็นบั๊ก ต่อ table และความผิดพลาดแบบใช้ auth.users เป็น authorization โผล่ทุกจุดที่เช็คว่า “ผู้เรียกคนนี้อยู่ tenant ไหน” ไม่ใช่แค่ครั้งเดียว
Trade-offs
หัวข้อที่มีชื่อว่า “Trade-offs”- helper
security definerอย่างwr_is_member()รวมทุกการเช็ค membership ไว้ที่ function เดียว — แลกมาด้วยชั้นอ้อมเพิ่ม: อ่านusingclause ของ policy อย่างเดียวไม่บอกเงื่อนไขจริงอีกต่อไป ต้องไปอ่าน helper ด้วย และ RLS ของทุก table ตอนนี้ขึ้นกับ function ตัวนี้ตัวเดียวว่าถูกต้องอยู่เสมอ - การตัด update policy ทั่วไปบน
wr_membersออกทั้งหมด (ส่งการเปลี่ยน role/tenant ผ่านwr_invite_memberเท่านั้น) ปิด cross-tenant hijack ได้สมบูรณ์ — แลกมาด้วยการเสียความสะดวกของPATCHธรรมดาสำหรับอะไรก็ตามที่เกี่ยวกับ membership แม้แต่ field ที่ไม่มีพิษภัยที่ฟีเจอร์ในอนาคตอาจอยากให้ member แก้เองได้ นั่นต้องมี policy scope แคบเฉพาะของตัวเอง อย่างตั้งใจ ไม่ใช่ใช้ตัวนี้ซ้ำ on delete cascadeบนทุก foreign key ที่ scope ตาม tenant ทำให้การลบ tenant เป็น operation เดียวจบครบ — แลกมาด้วยการไม่มีขั้นตอนยืนยันที่ชั้น database เลย: cascade ตัวเดียวกันที่ทำให้ cleanup ง่าย ก็ทำให้delete from wr_tenantsที่พลาดกู้คืนไม่ได้ถ้าไม่มี backup เร็วพอ ๆ กับที่มันสะดวก
ความเข้าใจผิดที่พบบ่อย
หัวข้อที่มีชื่อว่า “ความเข้าใจผิดที่พบบ่อย”- “ถ้า policy compile ผ่านแล้วคืน 200 แปลว่ามันทำตามที่ตั้งใจ” พิสูจน์ไว้ข้างบนสองครั้งในทิศตรงข้ามกัน: policy ที่อ่าน
auth.usersคืน403แข็ง ๆ กับทุกคน (ดูเหมือนพัง แต่จริง ๆ แค่ผิด) ในขณะที่using (true)บนwr_membersคืน200สะอาด ๆ ตอน cross-tenant hijack (ดูเหมือนโอเค แต่จริง ๆ คือบั๊ก) - “
security definerคือทางแก้เวลา RLS มาขวาง” พิสูจน์ไว้ข้างบน:security definerที่ไม่มีset search_path = ''ไม่ได้แค่ข้าม RLS ตามที่ตั้งใจ (งานจริงของมัน) — มันยังปล่อยให้ session ของ ผู้เรียก hijack ว่าชื่อที่ไม่ qualify schema จะ resolve ไปที่ table ไหน เงียบ ๆ ทั้งฝั่งอ่านและ — โชว์ครั้งแรกในคอร์สนี้ที่นี่ — ฝั่งเขียนด้วย - “trigger บน
auth.usersถูก scope อยู่แค่ฟีเจอร์ที่มันถูกเพิ่มเข้าไปให้” พิสูจน์ไว้ข้างบน: มันรันทุกการ signup ทุก user จากทุกฟีเจอร์ — บั๊ก permission ใน trigger ที่เขียนไว้ให้ “ฟีเจอร์ profile ใหม่” กลายเป็น “ทั้งแอป onboard ใครไม่ได้เลย” นี่คือเหตุผลที่security definerสำคัญกับ trigger บนauth.usersยิ่งกว่า RPC ธรรมดาด้วยซ้ำ
flowchart TD A["signup -- trigger, no security definer"] -->|"10: permission denied, 500"| A B["wr_members: user_id text"] -->|"1: create table fails"| B C["wr_projects select: reads auth.users"] -->|"3: 42501, no grant"| C D["wr_members: using(true), no with check"] -->|"4: cross-tenant hijack"| D E["wr_invite_member: no search_path"] -->|"5: insert redirected"| E F["wr_projects: no insert policy"] -->|"6: 42501 on create"| F G["tenant-client.ts: service_role"] -->|"7: full bypass"| G H[".single() on empty tenant"] -->|"8: PGRST116"| H I["no on delete cascade"] -->|"2: tenant delete fails"| I J["activity log: auth.uid() unwrapped"] -->|"9: ~5.2x slower"| J