From 393588a508ecf3cd32fc78eda496d7d687bae594 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 7 May 2026 23:06:29 +0900 Subject: [PATCH 1/2] refactor(billing): security_invoker views + RLS on base tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert grida_billing's API surface to the canonical Supabase pattern: security_invoker=true views over RLS-protected base tables, replacing SECURITY DEFINER views over a fully-locked schema. Functionally equivalent for every legitimate consumer; satisfies splinter lint 0010 (security_definer_view) which the Dashboard's Database Advisors flagged. What changed: - New migration 20260507223000_grida_billing_security_invoker.sql. The pushed 20260506132900 stays untouched. - grida_billing schema now grants USAGE to authenticated + service_role (security_invoker views resolve `grida_billing.*` as the caller). - account / subscription / audit drop their RESTRICTIVE deny-all policies and gain SELECT-only permissive policies (member_can_select for the first two, owner_can_select for audit). Writes stay service-role-only. - product_catalogue / stripe_event remain fully locked — no API consumer reads them. - v_billing_subscription / v_billing_audit flip to security_invoker=true. Their auth-side WHERE clauses move to the base-table policies; the views keep only the business filter (`status <> 'canceled'`). - Both views explicitly REVOKE from PUBLIC, anon, authenticated and re-GRANT to authenticated + service_role. Closes Supabase's default ALL-on-public privilege creep that was making anon implicitly able to discover them via GraphQL introspection. Behavioral equivalence (verified): - Authenticated org members see the same rows pre/post (auth filter just moved from view body to RLS policy). - Random users with no org → 0 rows, both before and after. - Owners read v_billing_audit; non-owner members blocked, both before and after. - anon: previously "implicit SELECT, returns 0 rows via NULL auth.uid()" → now "no SELECT grant, returns permission_denied". Strictly better posture; no legitimate flow affected. Tests: - pgTAP §13 (RLS denial contract) rewritten for the new model: 10 anon-denial assertions + 3 authenticated-non-member-sees-zero + 2 locked-table denials + 5 INSERT denials = 20 total. Plan stays at 57; all tests pass. - E2E billing suite (lifecycle, idempotency, tampered-signature) unchanged — webhook projector path untouched. - pnpm --filter editor typecheck: clean. - pnpm lint: 0 errors. - 476 editor unit tests: pass. Tooling: - New `just lint-supabase-db` recipe runs Splinter advisors against the local DB. CLI doesn't ship a native `supabase db check` yet (supabase/cli#3839 open); inline curl + psql + awk suffices. Recipe links the upstream tracking issue + our comment so future maintainers can drop it when the CLI ships native support. Sibling reading: - supabase/cli#3839 — feature request for first-class CLI advisor command (we left a comment with our use case + recipe at #issuecomment-4397668907) --- justfile | 19 +++ ...7223000_grida_billing_security_invoker.sql | 136 ++++++++++++++++++ supabase/schemas/grida_billing.sql | 92 ++++++++---- supabase/tests/test_grida_billing_test.sql | 97 +++++++------ 4 files changed, 275 insertions(+), 69 deletions(-) create mode 100644 supabase/migrations/20260507223000_grida_billing_security_invoker.sql diff --git a/justfile b/justfile index 5e40b0e2d..5a29db54a 100644 --- a/justfile +++ b/justfile @@ -18,6 +18,25 @@ check: test: pnpm turbo test +# Run Supabase Splinter advisors against the local database — same set +# of security + performance lints the Dashboard's "Database Advisors" +# page shows. CLI doesn't ship a native command for this yet; we curl +# the upstream `splinter.sql` and pipe through psql, then awk to filter +# the wide row format down to `[level] name: detail`. +# Requires `supabase start`. Pass `levels=WARN,ERROR,INFO` for strict. +# +# Tracking upstream: +# https://github.com/supabase/cli/issues/3839 +# https://github.com/supabase/cli/issues/3839#issuecomment-4397668907 (our comment) +# Drop this recipe when the CLI ships `supabase db check` or equivalent. +lint-supabase-db url="postgresql://postgres:postgres@127.0.0.1:54322/postgres" levels="WARN,ERROR": + @curl -sSfL https://raw.githubusercontent.com/supabase/splinter/main/splinter.sql -o /tmp/splinter.sql + @psql {{url}} -X -qtA -F$'\t' --pset=pager=off \ + -c "BEGIN READ ONLY;" -f /tmp/splinter.sql -c "COMMIT;" \ + | awk -F'\t' -v lvls="^({{replace(levels, ",", "|")}})$" \ + 'NF >= 7 && $3 ~ lvls { print " [" $3 "] " $1 ": " $7 }' \ + | sort -u + # Dev setup dev packages: pnpm dev:packages diff --git a/supabase/migrations/20260507223000_grida_billing_security_invoker.sql b/supabase/migrations/20260507223000_grida_billing_security_invoker.sql new file mode 100644 index 000000000..047c46ff6 --- /dev/null +++ b/supabase/migrations/20260507223000_grida_billing_security_invoker.sql @@ -0,0 +1,136 @@ +-- Convert grida_billing's API surface to the canonical Supabase pattern: +-- security_invoker = true views over RLS-protected base tables, instead +-- of SECURITY DEFINER views over a fully-locked schema. Functionally +-- equivalent for the API contract; satisfies splinter lint 0010 +-- (security_definer_view) and aligns with the rest of the codebase. +-- +-- Key shifts: +-- 1. authenticated gets USAGE on grida_billing (needed because +-- security_invoker views resolve `grida_billing.*` as the caller). +-- 2. account / subscription / audit lose their "deny everything" +-- RESTRICTIVE policies. They get SELECT-only PERMISSIVE policies +-- that mirror what the views' WHERE clauses used to enforce. Writes +-- stay service-role only (no INSERT/UPDATE/DELETE grant). +-- 3. product_catalogue / stripe_event remain fully locked — no API +-- consumer reads them. +-- 4. Both public.v_billing_* views flip to security_invoker = true and +-- drop their auth-side WHERE clauses (RLS now does the row scoping). +-- Their grants are scrubbed of Supabase's default-ALL on `public` +-- so anon doesn't see them in the GraphQL schema. +-- +-- All operations are idempotent. + +BEGIN; + +-- ─── 1. Schema USAGE ────────────────────────────────────────────── +REVOKE ALL ON SCHEMA grida_billing FROM PUBLIC; +GRANT USAGE ON SCHEMA grida_billing TO authenticated, service_role; + + +-- ─── 2a. account ───────────────────────────────────────────────── +DROP POLICY IF EXISTS default_deny_authenticated ON grida_billing.account; +DROP POLICY IF EXISTS default_deny_anon ON grida_billing.account; +DROP POLICY IF EXISTS member_can_select ON grida_billing.account; + +GRANT SELECT ON TABLE grida_billing.account TO authenticated; +CREATE POLICY member_can_select ON grida_billing.account + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT om.organization_id FROM public.organization_member om + WHERE om.user_id = (SELECT auth.uid()) + ) + ); + + +-- ─── 2b. subscription ──────────────────────────────────────────── +DROP POLICY IF EXISTS default_deny_authenticated ON grida_billing.subscription; +DROP POLICY IF EXISTS default_deny_anon ON grida_billing.subscription; +DROP POLICY IF EXISTS member_can_select ON grida_billing.subscription; + +GRANT SELECT ON TABLE grida_billing.subscription TO authenticated; +CREATE POLICY member_can_select ON grida_billing.subscription + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT om.organization_id FROM public.organization_member om + WHERE om.user_id = (SELECT auth.uid()) + ) + ); + + +-- ─── 2c. audit (owner-only) ────────────────────────────────────── +DROP POLICY IF EXISTS default_deny_authenticated ON grida_billing.audit; +DROP POLICY IF EXISTS default_deny_anon ON grida_billing.audit; +DROP POLICY IF EXISTS owner_can_select ON grida_billing.audit; + +GRANT SELECT ON TABLE grida_billing.audit TO authenticated; +CREATE POLICY owner_can_select ON grida_billing.audit + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT o.id FROM public.organization o + WHERE o.owner_id = (SELECT auth.uid()) + ) + ); + + +-- ─── 3. product_catalogue and stripe_event stay locked ────────── +-- (no changes — no API consumer; service_role-only access). + + +-- ─── 4a. v_billing_subscription ───────────────────────────────── +CREATE OR REPLACE VIEW public.v_billing_subscription +WITH (security_invoker = true) +AS +SELECT + s.organization_id, + s.plan, + s.is_free, + s.status, + s.quantity, + s.cancel_at_period_end, + s.current_period_start, + s.current_period_end, + s.stripe_subscription_id, + acc.stripe_customer_id +FROM grida_billing.subscription s +LEFT JOIN grida_billing.account acc ON acc.organization_id = s.organization_id +WHERE s.status <> 'canceled'; +-- Row scoping is enforced by `member_can_select` on the underlying tables. + +REVOKE ALL ON public.v_billing_subscription FROM PUBLIC, anon, authenticated; +GRANT SELECT ON public.v_billing_subscription TO authenticated, service_role; + + +-- ─── 4b. v_billing_audit ──────────────────────────────────────── +CREATE OR REPLACE VIEW public.v_billing_audit +WITH (security_invoker = true) +AS +SELECT + a.id, + a.organization_id, + a.user_id, + a.operation, + a.stripe_event_id, + a.stripe_subscription_id, + a.stripe_invoice_id, + a.stripe_customer_id, + a.member_user_id, + a.prev_quantity, + a.new_quantity, + a.plan, + a.status, + a.event_type, + a.billing_reason, + a.attempt_count, + a.amount_cents, + a.note, + a.created_at +FROM grida_billing.audit a; +-- Owner-only filter is enforced by `owner_can_select` on grida_billing.audit. + +REVOKE ALL ON public.v_billing_audit FROM PUBLIC, anon, authenticated; +GRANT SELECT ON public.v_billing_audit TO authenticated, service_role; + +COMMIT; diff --git a/supabase/schemas/grida_billing.sql b/supabase/schemas/grida_billing.sql index 85dde312f..4c65f50bd 100644 --- a/supabase/schemas/grida_billing.sql +++ b/supabase/schemas/grida_billing.sql @@ -20,7 +20,14 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA extensions; CREATE SCHEMA IF NOT EXISTS grida_billing; ALTER SCHEMA grida_billing OWNER TO postgres; --- Schema lockdown. No USAGE for anon/authenticated. +-- Schema USAGE: authenticated needs it because security_invoker=true views +-- in `public` resolve `grida_billing.*` as the calling role. Granting USAGE +-- alone exposes nothing — table-level GRANTs (further down) decide what +-- authenticated can actually read. anon has no USAGE here at all; the +-- public views aren't granted to anon either, so anon never reaches in. +REVOKE ALL ON SCHEMA grida_billing FROM PUBLIC; +GRANT USAGE ON SCHEMA grida_billing TO authenticated, service_role; + ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA grida_billing GRANT ALL ON TABLES TO service_role; ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA grida_billing GRANT ALL ON ROUTINES TO service_role; ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA grida_billing GRANT ALL ON SEQUENCES TO service_role; @@ -41,11 +48,23 @@ CREATE TABLE grida_billing.account ( updated_at timestamptz NOT NULL DEFAULT now() ); +-- Read access pattern: authenticated org members may SELECT their account +-- row through the SELECT grant + permissive policy below. Writes stay +-- service-role only (no INSERT/UPDATE/DELETE grant; no permissive policy +-- for those ops → RLS denies). anon has no grant at all and cannot reach +-- the table. ALTER TABLE grida_billing.account ENABLE ROW LEVEL SECURITY; -CREATE POLICY default_deny_authenticated ON grida_billing.account AS RESTRICTIVE FOR ALL TO authenticated USING (false) WITH CHECK (false); -CREATE POLICY default_deny_anon ON grida_billing.account AS RESTRICTIVE FOR ALL TO anon USING (false) WITH CHECK (false); REVOKE ALL ON TABLE grida_billing.account FROM anon, authenticated; -GRANT ALL ON TABLE grida_billing.account TO service_role; +GRANT ALL ON TABLE grida_billing.account TO service_role; +GRANT SELECT ON TABLE grida_billing.account TO authenticated; +CREATE POLICY member_can_select ON grida_billing.account + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT om.organization_id FROM public.organization_member om + WHERE om.user_id = (SELECT auth.uid()) + ) + ); --------------------------------------------------------------------- @@ -91,11 +110,20 @@ CREATE UNIQUE INDEX subscription_one_active_per_org_idx CREATE INDEX subscription_organization_id_idx ON grida_billing.subscription (organization_id); +-- Same pattern as account: members SELECT their org's row; writes are +-- webhook-projector / service-role only. ALTER TABLE grida_billing.subscription ENABLE ROW LEVEL SECURITY; -CREATE POLICY default_deny_authenticated ON grida_billing.subscription AS RESTRICTIVE FOR ALL TO authenticated USING (false) WITH CHECK (false); -CREATE POLICY default_deny_anon ON grida_billing.subscription AS RESTRICTIVE FOR ALL TO anon USING (false) WITH CHECK (false); REVOKE ALL ON TABLE grida_billing.subscription FROM anon, authenticated; -GRANT ALL ON TABLE grida_billing.subscription TO service_role; +GRANT ALL ON TABLE grida_billing.subscription TO service_role; +GRANT SELECT ON TABLE grida_billing.subscription TO authenticated; +CREATE POLICY member_can_select ON grida_billing.subscription + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT om.organization_id FROM public.organization_member om + WHERE om.user_id = (SELECT auth.uid()) + ) + ); --------------------------------------------------------------------- @@ -223,11 +251,21 @@ CREATE INDEX audit_stripe_invoice_id_idx ON grida_billing.audit (stripe_invoice_id) WHERE stripe_invoice_id IS NOT NULL; +-- Owner-only read: only the org owner sees audit rows (TC-BILLING-OPS-009/010). +-- Members other than the owner are excluded by the policy USING clause. +-- Writes stay webhook-projector only. ALTER TABLE grida_billing.audit ENABLE ROW LEVEL SECURITY; -CREATE POLICY default_deny_authenticated ON grida_billing.audit AS RESTRICTIVE FOR ALL TO authenticated USING (false) WITH CHECK (false); -CREATE POLICY default_deny_anon ON grida_billing.audit AS RESTRICTIVE FOR ALL TO anon USING (false) WITH CHECK (false); REVOKE ALL ON TABLE grida_billing.audit FROM anon, authenticated; -GRANT ALL ON TABLE grida_billing.audit TO service_role; +GRANT ALL ON TABLE grida_billing.audit TO service_role; +GRANT SELECT ON TABLE grida_billing.audit TO authenticated; +CREATE POLICY owner_can_select ON grida_billing.audit + FOR SELECT TO authenticated + USING ( + organization_id IN ( + SELECT o.id FROM public.organization o + WHERE o.owner_id = (SELECT auth.uid()) + ) + ); -- ============================================================================ @@ -793,10 +831,16 @@ $$; --------------------------------------------------------------------- -- [public.v_billing_subscription] +-- +-- security_invoker = true: the view runs as the caller, so RLS on the +-- base tables (grida_billing.subscription / .account) does the row +-- filtering. The view body keeps only the business filter +-- (status <> 'canceled') — auth-side scoping is done by the table-level +-- `member_can_select` policies. This satisfies splinter lint 0010. --------------------------------------------------------------------- CREATE OR REPLACE VIEW public.v_billing_subscription -WITH (security_invoker = false) +WITH (security_invoker = true) AS SELECT s.organization_id, @@ -811,14 +855,13 @@ SELECT acc.stripe_customer_id FROM grida_billing.subscription s LEFT JOIN grida_billing.account acc ON acc.organization_id = s.organization_id -WHERE s.status <> 'canceled' - AND s.organization_id IN ( - SELECT om.organization_id - FROM public.organization_member om - WHERE om.user_id = (SELECT auth.uid()) - ); +WHERE s.status <> 'canceled'; -GRANT SELECT ON public.v_billing_subscription TO authenticated, service_role; +-- REVOKE before GRANT: Supabase's default privileges on `public` grant ALL +-- to anon/authenticated/service_role on new objects. Strip first, then add +-- back only the roles that should actually see the view. +REVOKE ALL ON public.v_billing_subscription FROM PUBLIC, anon, authenticated; +GRANT SELECT ON public.v_billing_subscription TO authenticated, service_role; --------------------------------------------------------------------- @@ -828,7 +871,7 @@ GRANT SELECT ON public.v_billing_subscription TO authenticated, service_role; --------------------------------------------------------------------- CREATE OR REPLACE VIEW public.v_billing_audit -WITH (security_invoker = false) +WITH (security_invoker = true) AS SELECT a.id, @@ -850,13 +893,12 @@ SELECT a.amount_cents, a.note, a.created_at -FROM grida_billing.audit a -WHERE a.organization_id IN ( - SELECT o.id FROM public.organization o - WHERE o.owner_id = (SELECT auth.uid()) -); +FROM grida_billing.audit a; +-- Owner-only filter is enforced by the `owner_can_select` RLS policy on +-- grida_billing.audit (security_invoker = true means RLS runs as caller). -GRANT SELECT ON public.v_billing_audit TO authenticated, service_role; +REVOKE ALL ON public.v_billing_audit FROM PUBLIC, anon, authenticated; +GRANT SELECT ON public.v_billing_audit TO authenticated, service_role; --------------------------------------------------------------------- diff --git a/supabase/tests/test_grida_billing_test.sql b/supabase/tests/test_grida_billing_test.sql index da7e63ba8..41b24aa2b 100644 --- a/supabase/tests/test_grida_billing_test.sql +++ b/supabase/tests/test_grida_billing_test.sql @@ -393,101 +393,110 @@ SELECT is( ); -- --------------------------------------------------------------------- --- 13. Direct RLS / GRANT denial on grida_billing internal tables. --- The schema is locked-down: REVOKE ALL FROM anon, authenticated + --- RESTRICTIVE deny policies. Verify both `anon` and `authenticated` --- get permission_denied (SQLSTATE 42501) on read AND write paths so --- a future stray GRANT or policy loosen-up gets caught here. --- Tables: account, subscription, product_catalogue, stripe_event, audit. +-- 13. Direct RLS / GRANT contract on grida_billing internal tables. +-- Pattern A (post-PR): account/subscription/audit are SELECT-able by +-- authenticated org members (filtered by table-level RLS); the views +-- run as the caller via security_invoker=true. catalogue/stripe_event +-- remain fully locked (no API consumer reads them). Writes are +-- service-role-only on every table. anon has no grant on any table. +-- +-- This section catches privilege drift in either direction: +-- - A future GRANT to anon (any table) +-- - A future write GRANT to authenticated +-- - A future GRANT to authenticated on the locked tables -- --------------------------------------------------------------------- --- authenticated role. -SET LOCAL ROLE authenticated; -SELECT set_config('request.jwt.claim.sub', current_setting('test.insider_uid'), true); +-- ── anon: zero access on every internal table. ───────────────────── +SET LOCAL ROLE anon; SELECT throws_ok( $$ SELECT 1 FROM grida_billing.account LIMIT 1 $$, '42501', NULL, - 'authenticated cannot SELECT grida_billing.account' + 'anon cannot SELECT grida_billing.account' ); SELECT throws_ok( $$ INSERT INTO grida_billing.account (organization_id) VALUES (1) $$, '42501', NULL, - 'authenticated cannot INSERT grida_billing.account' + 'anon cannot INSERT grida_billing.account' ); SELECT throws_ok( $$ SELECT 1 FROM grida_billing.subscription LIMIT 1 $$, '42501', NULL, - 'authenticated cannot SELECT grida_billing.subscription' + 'anon cannot SELECT grida_billing.subscription' ); SELECT throws_ok( $$ INSERT INTO grida_billing.subscription (organization_id, plan, is_free, status) VALUES (1, 'free', true, 'active') $$, '42501', NULL, - 'authenticated cannot INSERT grida_billing.subscription' + 'anon cannot INSERT grida_billing.subscription' ); SELECT throws_ok( $$ SELECT 1 FROM grida_billing.product_catalogue LIMIT 1 $$, '42501', NULL, - 'authenticated cannot SELECT grida_billing.product_catalogue' + 'anon cannot SELECT grida_billing.product_catalogue' ); SELECT throws_ok( $$ INSERT INTO grida_billing.product_catalogue (id, kind) VALUES ('plan.x', 'plan') $$, '42501', NULL, - 'authenticated cannot INSERT grida_billing.product_catalogue' + 'anon cannot INSERT grida_billing.product_catalogue' ); SELECT throws_ok( $$ SELECT 1 FROM grida_billing.stripe_event LIMIT 1 $$, '42501', NULL, - 'authenticated cannot SELECT grida_billing.stripe_event' + 'anon cannot SELECT grida_billing.stripe_event' ); SELECT throws_ok( $$ INSERT INTO grida_billing.stripe_event (id, type) VALUES ('evt_x', 'customer.created') $$, '42501', NULL, - 'authenticated cannot INSERT grida_billing.stripe_event' + 'anon cannot INSERT grida_billing.stripe_event' ); SELECT throws_ok( $$ SELECT 1 FROM grida_billing.audit LIMIT 1 $$, '42501', NULL, - 'authenticated cannot SELECT grida_billing.audit' + 'anon cannot SELECT grida_billing.audit' ); SELECT throws_ok( $$ INSERT INTO grida_billing.audit (organization_id, operation) VALUES (1, 'customer_attach') $$, '42501', NULL, - 'authenticated cannot INSERT grida_billing.audit' + 'anon cannot INSERT grida_billing.audit' ); --- anon role (no JWT, no auth.uid()). -SET LOCAL ROLE anon; +-- ── authenticated, no org membership: SELECTs are filtered to 0 rows +-- on the open tables, hard-denied on the locked tables, INSERTs hard- +-- denied everywhere. ─────────────────────────────────────────────── +SET LOCAL ROLE authenticated; +SELECT set_config('request.jwt.claim.sub', current_setting('test.random_uid'), true); -SELECT throws_ok( - $$ SELECT 1 FROM grida_billing.account LIMIT 1 $$, '42501', NULL, - 'anon cannot SELECT grida_billing.account' +SELECT is( + (SELECT count(*) FROM grida_billing.account)::bigint, 0::bigint, + 'authenticated non-member sees 0 rows from grida_billing.account (RLS-filtered)' ); -SELECT throws_ok( - $$ INSERT INTO grida_billing.account (organization_id) VALUES (1) $$, '42501', NULL, - 'anon cannot INSERT grida_billing.account' +SELECT is( + (SELECT count(*) FROM grida_billing.subscription)::bigint, 0::bigint, + 'authenticated non-member sees 0 rows from grida_billing.subscription' ); -SELECT throws_ok( - $$ SELECT 1 FROM grida_billing.subscription LIMIT 1 $$, '42501', NULL, - 'anon cannot SELECT grida_billing.subscription' +SELECT is( + (SELECT count(*) FROM grida_billing.audit)::bigint, 0::bigint, + 'authenticated non-owner sees 0 rows from grida_billing.audit' ); + SELECT throws_ok( - $$ INSERT INTO grida_billing.subscription (organization_id, plan, is_free, status) VALUES (1, 'free', true, 'active') $$, '42501', NULL, - 'anon cannot INSERT grida_billing.subscription' + $$ SELECT 1 FROM grida_billing.product_catalogue LIMIT 1 $$, '42501', NULL, + 'authenticated cannot SELECT grida_billing.product_catalogue (locked)' ); SELECT throws_ok( - $$ SELECT 1 FROM grida_billing.product_catalogue LIMIT 1 $$, '42501', NULL, - 'anon cannot SELECT grida_billing.product_catalogue' + $$ SELECT 1 FROM grida_billing.stripe_event LIMIT 1 $$, '42501', NULL, + 'authenticated cannot SELECT grida_billing.stripe_event (locked)' ); + SELECT throws_ok( - $$ INSERT INTO grida_billing.product_catalogue (id, kind) VALUES ('plan.x', 'plan') $$, '42501', NULL, - 'anon cannot INSERT grida_billing.product_catalogue' + $$ INSERT INTO grida_billing.account (organization_id) VALUES (999) $$, '42501', NULL, + 'authenticated cannot INSERT grida_billing.account' ); SELECT throws_ok( - $$ SELECT 1 FROM grida_billing.stripe_event LIMIT 1 $$, '42501', NULL, - 'anon cannot SELECT grida_billing.stripe_event' + $$ INSERT INTO grida_billing.subscription (organization_id, plan, is_free, status) VALUES (999, 'free', true, 'active') $$, '42501', NULL, + 'authenticated cannot INSERT grida_billing.subscription' ); SELECT throws_ok( - $$ INSERT INTO grida_billing.stripe_event (id, type) VALUES ('evt_x', 'customer.created') $$, '42501', NULL, - 'anon cannot INSERT grida_billing.stripe_event' + $$ INSERT INTO grida_billing.product_catalogue (id, kind) VALUES ('plan.x', 'plan') $$, '42501', NULL, + 'authenticated cannot INSERT grida_billing.product_catalogue' ); SELECT throws_ok( - $$ SELECT 1 FROM grida_billing.audit LIMIT 1 $$, '42501', NULL, - 'anon cannot SELECT grida_billing.audit' + $$ INSERT INTO grida_billing.stripe_event (id, type) VALUES ('evt_x', 'customer.created') $$, '42501', NULL, + 'authenticated cannot INSERT grida_billing.stripe_event' ); SELECT throws_ok( - $$ INSERT INTO grida_billing.audit (organization_id, operation) VALUES (1, 'customer_attach') $$, '42501', NULL, - 'anon cannot INSERT grida_billing.audit' + $$ INSERT INTO grida_billing.audit (organization_id, operation) VALUES (999, 'customer_attach') $$, '42501', NULL, + 'authenticated cannot INSERT grida_billing.audit' ); RESET ROLE; From f1b70a600e644486fa83e2b3e27038a0817048b6 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 7 May 2026 23:35:29 +0900 Subject: [PATCH 2/2] fix(billing): close USAGE-grant escalation + lint script hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR review fixes from #709. Directly amends the unmerged Pattern A migration (`20260507223000_grida_billing_security_invoker.sql`) so production gets one clean migration instead of a delta-on-delta. Codex P1 — privilege escalation closed: Granting USAGE on `grida_billing` to authenticated (required for security_invoker views to resolve `grida_billing.*` as the caller) also made every internal SECURITY DEFINER routine reachable, because EXECUTE on functions defaults to PUBLIC and authenticated inherits PUBLIC. The original ALTER DEFAULT PRIVILEGES line only revoked from authenticated/anon, not PUBLIC, so authenticated could call the webhook projector (`fn_apply_stripe_event`) directly with arbitrary payloads and bypass signature verification. Fix: explicit `REVOKE EXECUTE ON ALL FUNCTIONS IN SCHEMA grida_billing FROM PUBLIC, anon, authenticated` for existing routines, and `ALTER DEFAULT PRIVILEGES … REVOKE EXECUTE … FROM PUBLIC` for future ones. Verified post-migration: only `postgres` and `service_role` retain EXECUTE on every grida_billing.fn_* and tg_*. CodeRabbit #4 (partial) — RLS join-key index: `owner_can_select` on grida_billing.audit subqueries `public.organization` by `owner_id`, which has no index. Without it, every RLS evaluation on audit reads triggers a seq scan. Cheap fix. (CodeRabbit also flagged `organization_member.user_id`, but that index already exists — verified.) CodeRabbit #2 — splinter download race + integrity: `/tmp/splinter.sql` collides under parallel CI matrix runs. Switched to `mktemp -t splinter.XXXXXX.sql` with `trap … EXIT` cleanup so concurrent invocations get isolated paths. CodeRabbit #3 — pipeline error swallowing: `just` runs `/bin/sh` without pipefail; a `psql … | awk … | sort` failure used to exit 0 with empty output (no signal that linting never ran). Switched the recipe to a bash shebang with `set -euo pipefail` and capture-then-pipe, so connection/auth/schema-drift failures propagate. Verified: a bogus connection URL now exits non-zero with a clear psql error. Verification: - `supabase db reset` → both migrations apply clean - `supabase test db` → 135 pgTAP tests pass - `pnpm --filter editor typecheck` → clean - `pnpm lint` → 0 errors - `just lint-supabase-db` → no `security_definer_view` findings; the only remaining warnings are by-design GraphQL-discoverability notices on the two billing views (out of scope, see PR description) - `pg_indexes` confirms `organization_owner_id_idx` is present - `information_schema.routine_privileges` confirms only postgres + service_role hold EXECUTE on every grida_billing.fn_* / tg_* E2E suite not re-run (requires `next dev` on :3000, which the sandbox correctly refuses to autostart). Zero TS/projector code touched in this commit, so the suite's pass/fail is unchanged from prior cycle. --- justfile | 16 ++++++++++--- ...7223000_grida_billing_security_invoker.sql | 24 +++++++++++++++++++ supabase/schemas/grida_billing.sql | 20 ++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/justfile b/justfile index 5a29db54a..07873ee58 100644 --- a/justfile +++ b/justfile @@ -30,9 +30,19 @@ test: # https://github.com/supabase/cli/issues/3839#issuecomment-4397668907 (our comment) # Drop this recipe when the CLI ships `supabase db check` or equivalent. lint-supabase-db url="postgresql://postgres:postgres@127.0.0.1:54322/postgres" levels="WARN,ERROR": - @curl -sSfL https://raw.githubusercontent.com/supabase/splinter/main/splinter.sql -o /tmp/splinter.sql - @psql {{url}} -X -qtA -F$'\t' --pset=pager=off \ - -c "BEGIN READ ONLY;" -f /tmp/splinter.sql -c "COMMIT;" \ + #!/usr/bin/env bash + # `set -euo pipefail` upgrades from the default sh-without-pipefail — + # otherwise a psql failure (connection refused, schema drift) gets + # swallowed by the awk/sort downstream and the recipe exits 0 silently. + set -euo pipefail + splinter=$(mktemp -t splinter.XXXXXX.sql) + trap 'rm -f "$splinter"' EXIT + curl -sSfL https://raw.githubusercontent.com/supabase/splinter/main/splinter.sql -o "$splinter" + # Capture before filtering so any psql error propagates instead of being + # masked by an empty result set. + output=$(psql {{url}} -X -qtA -F$'\t' --pset=pager=off \ + -c "BEGIN READ ONLY;" -f "$splinter" -c "COMMIT;") + printf '%s\n' "$output" \ | awk -F'\t' -v lvls="^({{replace(levels, ",", "|")}})$" \ 'NF >= 7 && $3 ~ lvls { print " [" $3 "] " $1 ": " $7 }' \ | sort -u diff --git a/supabase/migrations/20260507223000_grida_billing_security_invoker.sql b/supabase/migrations/20260507223000_grida_billing_security_invoker.sql index 047c46ff6..d302a5f23 100644 --- a/supabase/migrations/20260507223000_grida_billing_security_invoker.sql +++ b/supabase/migrations/20260507223000_grida_billing_security_invoker.sql @@ -27,6 +27,23 @@ REVOKE ALL ON SCHEMA grida_billing FROM PUBLIC; GRANT USAGE ON SCHEMA grida_billing TO authenticated, service_role; +-- ─── 1a. Lock internal SECURITY DEFINER routines ────────────────── +-- Schema USAGE alone makes every existing function in grida_billing +-- callable by authenticated, because EXECUTE on functions is granted +-- to PUBLIC by default in Postgres and authenticated inherits PUBLIC. +-- The DEFAULT PRIVILEGES line in the original migration only revoked +-- from authenticated/anon explicitly — not from PUBLIC — so existing +-- routines still let signed-in users call e.g. fn_apply_stripe_event +-- directly and bypass webhook-signature verification. +-- +-- Strip every existing function in grida_billing to service_role-only, +-- then patch the DEFAULT PRIVILEGES so future functions are locked +-- down at create time too. +REVOKE EXECUTE ON ALL FUNCTIONS IN SCHEMA grida_billing FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA grida_billing TO service_role; +ALTER DEFAULT PRIVILEGES IN SCHEMA grida_billing REVOKE EXECUTE ON ROUTINES FROM PUBLIC; + + -- ─── 2a. account ───────────────────────────────────────────────── DROP POLICY IF EXISTS default_deny_authenticated ON grida_billing.account; DROP POLICY IF EXISTS default_deny_anon ON grida_billing.account; @@ -74,6 +91,13 @@ CREATE POLICY owner_can_select ON grida_billing.audit ) ); +-- The owner_can_select subquery looks up `public.organization` by +-- `owner_id`, which has no index by default. Without this, every RLS +-- evaluation on the audit table would seq-scan organization. Cheap fix. +-- (organization_member.user_id is already indexed.) +CREATE INDEX IF NOT EXISTS organization_owner_id_idx + ON public.organization (owner_id); + -- ─── 3. product_catalogue and stripe_event stay locked ────────── -- (no changes — no API consumer; service_role-only access). diff --git a/supabase/schemas/grida_billing.sql b/supabase/schemas/grida_billing.sql index 4c65f50bd..be8e2d85e 100644 --- a/supabase/schemas/grida_billing.sql +++ b/supabase/schemas/grida_billing.sql @@ -34,6 +34,11 @@ ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA grida_billing GRANT ALL ON ALTER DEFAULT PRIVILEGES IN SCHEMA grida_billing REVOKE ALL ON TABLES FROM authenticated, anon; ALTER DEFAULT PRIVILEGES IN SCHEMA grida_billing REVOKE ALL ON ROUTINES FROM authenticated, anon; ALTER DEFAULT PRIVILEGES IN SCHEMA grida_billing REVOKE ALL ON SEQUENCES FROM authenticated, anon; +-- Default-privileges only revoke from authenticated/anon explicitly; PUBLIC +-- still inherits EXECUTE on every routine and authenticated inherits PUBLIC. +-- Strip PUBLIC too so the schema USAGE we grant above doesn't make signed-in +-- users able to call internal SECURITY DEFINER functions like the projector. +ALTER DEFAULT PRIVILEGES IN SCHEMA grida_billing REVOKE EXECUTE ON ROUTINES FROM PUBLIC; --------------------------------------------------------------------- @@ -267,6 +272,12 @@ CREATE POLICY owner_can_select ON grida_billing.audit ) ); +-- The owner_can_select subquery looks up `public.organization` by owner_id, +-- which has no other index. Without this, every RLS evaluation on the +-- audit table seq-scans organization. Cheap fix. +CREATE INDEX IF NOT EXISTS organization_owner_id_idx + ON public.organization (owner_id); + -- ============================================================================ -- Functions @@ -825,6 +836,15 @@ AS $$ $$; +-- All grida_billing.* routines stay service-role only. The schema USAGE +-- granted to authenticated above makes them reachable; explicit revoke +-- here closes the privilege-escalation path PUBLIC EXECUTE would create +-- (e.g. signed-in users calling fn_apply_stripe_event directly and +-- bypassing webhook signature verification). +REVOKE EXECUTE ON ALL FUNCTIONS IN SCHEMA grida_billing FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA grida_billing TO service_role; + + -- ============================================================================ -- public.* wrapper surface (PostgREST) -- ============================================================================