Skip to content

refactor(billing): security_invoker views + RLS on base tables #709

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.

Sign up for GitHub

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

Merged
softmarshmallow merged 2 commits into from
May 7, 2026
Merged

refactor(billing): security_invoker views + RLS on base tables #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
393588a
refactor(billing): security_invoker views + RLS on base tables
softmarshmallow May 7, 2026
f1b70a6
fix(billing): close USAGE-grant escalation + lint script hardening
softmarshmallow May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,35 @@ 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":
#!/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 \

@coderabbitai coderabbitai Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
# Verify current interpolation is unquoted in recipe
rg -n 'output=\(psql \{\{url\}\}' justfile

Repository: gridaco/grida

Length of output: 39

🏁 Script executed:

# Read the justfile around line 43 to see the context
sed -n '40,50p' justfile

Repository: gridaco/grida

Length of output: 585

Quote the url argument in the psql call.

Line 43 currently passes {{url}} unquoted to psql. Change to "{{url}}" to safely handle DSNs containing shell-significant characters (spaces, wildcards, dollar signs, etc.).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@justfile` at line 43, The psql invocation passes the DSN/template variable
unquoted which can break for DSNs with spaces or shell-sensitive chars; update
the command that builds output (the psql call that currently uses {{url}}) to
wrap the URL in double quotes ("{{url}}") so the shell treats it as a single
safe argument to psql; ensure the change is applied where the output=$(psql
{{url}} -X -qtA -F$'\t' --pset=pager=off \) command is defined.

-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

# Dev setup
dev packages:
pnpm dev:packages
Expand Down
160 changes: 160 additions & 0 deletions supabase/migrations/20260507223000_grida_billing_security_invoker.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
-- 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;

@chatgpt-codex-connector chatgpt-codex-connector Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revoke internal function execute rights before schema access

Granting authenticated USAGE on grida_billing makes every routine in that schema callable by authenticated users unless EXECUTE is explicitly revoked, and the existing billing migration defines multiple SECURITY DEFINER functions in grida_billing without corresponding REVOKE ... ON FUNCTION grida_billing.* FROM PUBLIC (only the public.fn_billing_* wrappers are locked down). This opens a privilege-escalation path where any signed-in user can invoke internal mutating functions (for example, event projector/customer attach paths) directly and bypass the intended service-role-only boundary.

Useful? React with 👍 / 👎.



-- ─── 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;
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())
)
);

-- 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).


-- ─── 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;
112 changes: 87 additions & 25 deletions supabase/schemas/grida_billing.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,25 @@
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;
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;


---------------------------------------------------------------------
Expand All @@ -41,11 +53,23 @@
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())
)
);
Comment on lines +65 to +72

@coderabbitai coderabbitai Bot May 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify indexes exist on public.organization_member(user_id) and
# public.organization(owner_id) — the columns referenced in the new RLS policies.
rg -n --type sql -i 'create.*index.*organization_member.*user_id\|create.*index.*user_id.*organization_member' \
  || echo "No index on organization_member.user_id found"

rg -n --type sql -i 'create.*index.*organization.*owner_id\|create.*index.*owner_id.*organization[^_]' \
  || echo "No index on organization.owner_id found"

Repository: gridaco/grida

Length of output: 143

Add indexes on RLS join-key columns to avoid sequential scans during policy evaluation.

All three new SELECT policies subquery public tables without indexes on their join conditions:

Policy Subquery join column Status
member_can_select (account/subscription) public.organization_member.user_id Missing
owner_can_select (audit) public.organization.owner_id Missing

Each RLS-gated row scan will trigger a sequential table scan, once per candidate row. Under load this becomes a hot path.

Add these indexes to the public schema:

SQL Fix 
CREATE INDEX IF NOT EXISTS organization_member_user_id_idx
  ON public.organization_member (user_id);

CREATE INDEX IF NOT EXISTS organization_owner_id_idx
  ON public.organization (owner_id);

Applies to lines: 60-67, 119-126, 261-268

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/schemas/grida_billing.sql` around lines 60 - 67, The RLS policies
(e.g., member_can_select and owner_can_select) reference public table join
columns without indexes, causing sequential scans during policy evaluation; add
indexes for public.organization_member.user_id and public.organization.owner_id
(create indexes named organization_member_user_id_idx and
organization_owner_id_idx respectively) so policy subqueries used by the
account/subscription/audit policies can use index lookups instead of sequential
scans; ensure to use IF NOT EXISTS semantics when adding these indexes so
repeated migrations are safe.



---------------------------------------------------------------------
Expand Down Expand Up @@ -91,11 +115,20 @@
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())
)
);


---------------------------------------------------------------------
Expand Down Expand Up @@ -223,11 +256,27 @@
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())
)
);

-- 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);


-- ============================================================================
Expand Down Expand Up @@ -545,7 +594,7 @@

-- Map Stripe price → plan via the catalogue. Fail closed: an unknown or
-- newly-provisioned price must not silently project as 'pro' (would
-- mis-entitle the org). RAISE so Stripe retries while ops fixes the

Check warning on line 597 in supabase/schemas/grida_billing.sql

View workflow job for this annotation

GitHub Actions / typos 

"mis" should be "miss" or "mist".
-- catalogue mapping.
v_plan := NULL;
IF v_price_id IS NOT NULL THEN
Expand Down Expand Up @@ -787,16 +836,31 @@
$$;


-- 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)
-- ============================================================================

---------------------------------------------------------------------
-- [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,
Expand All @@ -811,14 +875,13 @@
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;


---------------------------------------------------------------------
Expand All @@ -828,7 +891,7 @@
---------------------------------------------------------------------

CREATE OR REPLACE VIEW public.v_billing_audit
WITH (security_invoker = false)
WITH (security_invoker = true)
AS
SELECT
a.id,
Expand All @@ -850,13 +913,12 @@
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;


---------------------------------------------------------------------
Expand Down
Loading
Loading