fix(db): lock increment_daily_usage and admin analytics RPCs to service_role
This commit is contained in:
Yun Chan
parent
e69fe0335d
commit
63bc0ebe69
2 changed files with 483 additions and 0 deletions
|
|
@ -0,0 +1,200 @@
|
|||
-- ============================================================================
|
||||
-- 20260929000001_function_acl_hardening.sql
|
||||
--
|
||||
-- Function execution ACL hardening.
|
||||
--
|
||||
-- Background
|
||||
-- Supabase's default privileges give every function that postgres creates in
|
||||
-- schema public a *direct* EXECUTE grant to anon, authenticated and
|
||||
-- service_role. `REVOKE ... FROM PUBLIC` therefore does not lock anon out.
|
||||
-- Three early migrations only revoked PUBLIC (and sometimes authenticated):
|
||||
-- * 20260409000003 increment_daily_usage -> anon could add any (negative)
|
||||
-- amount to any user's daily_usage, wiping or exhausting quota.
|
||||
-- * 20260413000004 admin_usage_by_feature / admin_top_users / admin_dau ->
|
||||
-- SECURITY DEFINER bodies with no role predicate, callable by anon and
|
||||
-- every signed-in user; admin_top_users leaks user ids and names.
|
||||
--
|
||||
-- Policy (single place; enforced by tests/function-acl.integration.sql)
|
||||
-- * No SECURITY DEFINER function in schema public is executable by anon.
|
||||
-- * SECURITY DEFINER trigger functions are executable by nobody but the
|
||||
-- owner and service_role (triggers do not check EXECUTE when they fire).
|
||||
-- * authenticated may execute a SECURITY DEFINER function only if it is on
|
||||
-- the reviewed allowlist in tests/function-acl.integration.sql.
|
||||
-- * Every SECURITY DEFINER function pins search_path.
|
||||
-- New migrations keep writing their own
|
||||
-- REVOKE ALL ON FUNCTION ... FROM PUBLIC, anon[, authenticated];
|
||||
-- GRANT EXECUTE ON FUNCTION ... TO <the roles that need it>;
|
||||
-- and add authenticated RPCs to the test allowlist. An omission is caught by
|
||||
-- the local integration test (it is not run by CI yet).
|
||||
--
|
||||
-- Why default privileges are NOT changed here
|
||||
-- A deny-by-default `ALTER DEFAULT PRIVILEGES` only works for anon if it is
|
||||
-- the schema-less (global) form, because anon inherits the built-in PUBLIC
|
||||
-- EXECUTE default. The global form also strips PUBLIC EXECUTE from pg_temp
|
||||
-- helpers created by postgres, which breaks every integration test that
|
||||
-- calls pg_temp.assert_true()/act_as() after SET ROLE authenticated, and it
|
||||
-- would silently change the ACL of functions created by later migrations
|
||||
-- that are being written against the current defaults. The schema-scoped
|
||||
-- form alone is a no-op for anon. The sweep below plus the catalog test give
|
||||
-- the same guarantee without those side effects.
|
||||
-- ============================================================================
|
||||
|
||||
-- ----------------------------------------------------------------------------
|
||||
-- 1. Sweep: apply the policy to every existing SECURITY DEFINER function in
|
||||
-- schema public. authenticated/service_role access is preserved exactly
|
||||
-- (except for trigger functions, which cannot be called directly anyway);
|
||||
-- only anon/PUBLIC access is removed.
|
||||
-- ----------------------------------------------------------------------------
|
||||
DO $$
|
||||
DECLARE
|
||||
fn record;
|
||||
target text;
|
||||
keep_authenticated boolean;
|
||||
keep_service_role boolean;
|
||||
BEGIN
|
||||
FOR fn IN
|
||||
SELECT p.oid,
|
||||
n.nspname,
|
||||
p.proname,
|
||||
pg_get_function_identity_arguments(p.oid) AS identity_args,
|
||||
p.prorettype = 'trigger'::regtype AS is_trigger
|
||||
FROM pg_proc p
|
||||
JOIN pg_namespace n ON n.oid = p.pronamespace
|
||||
WHERE n.nspname = 'public'
|
||||
AND p.prosecdef
|
||||
LOOP
|
||||
target := format('%I.%I(%s)', fn.nspname, fn.proname, fn.identity_args);
|
||||
keep_authenticated := NOT fn.is_trigger
|
||||
AND has_function_privilege('authenticated', fn.oid, 'EXECUTE');
|
||||
keep_service_role := has_function_privilege('service_role', fn.oid, 'EXECUTE');
|
||||
|
||||
EXECUTE format('REVOKE ALL ON ROUTINE %s FROM PUBLIC, anon', target);
|
||||
IF fn.is_trigger THEN
|
||||
EXECUTE format('REVOKE ALL ON ROUTINE %s FROM authenticated', target);
|
||||
END IF;
|
||||
|
||||
-- Re-grant access that previously flowed only through PUBLIC.
|
||||
IF keep_authenticated THEN
|
||||
EXECUTE format('GRANT EXECUTE ON ROUTINE %s TO authenticated', target);
|
||||
END IF;
|
||||
IF keep_service_role THEN
|
||||
EXECUTE format('GRANT EXECUTE ON ROUTINE %s TO service_role', target);
|
||||
END IF;
|
||||
END LOOP;
|
||||
END;
|
||||
$$;
|
||||
|
||||
-- ----------------------------------------------------------------------------
|
||||
-- 2. increment_daily_usage: service_role only, and the counter can no longer
|
||||
-- go below zero. Negative amounts stay allowed for the service-role refund
|
||||
-- path (functions/_shared/embedding-quota.ts releases reserved units with a
|
||||
-- negative amount), but the stored total is clamped at 0.
|
||||
-- ----------------------------------------------------------------------------
|
||||
CREATE OR REPLACE FUNCTION public.increment_daily_usage(
|
||||
p_user_id uuid,
|
||||
p_feature text,
|
||||
p_amount integer DEFAULT 1
|
||||
)
|
||||
RETURNS integer
|
||||
LANGUAGE plpgsql
|
||||
SECURITY DEFINER
|
||||
SET search_path = ''
|
||||
AS $$
|
||||
DECLARE
|
||||
v_new_count integer;
|
||||
BEGIN
|
||||
IF p_user_id IS NULL OR p_feature IS NULL OR p_amount IS NULL THEN
|
||||
RAISE EXCEPTION 'increment_daily_usage: arguments must not be null'
|
||||
USING ERRCODE = '22004';
|
||||
END IF;
|
||||
|
||||
INSERT INTO public.daily_usage (user_id, date, feature, count)
|
||||
VALUES (p_user_id, CURRENT_DATE, p_feature, greatest(p_amount, 0))
|
||||
ON CONFLICT (user_id, date, feature)
|
||||
DO UPDATE SET count = greatest(public.daily_usage.count + p_amount, 0)
|
||||
RETURNING count INTO v_new_count;
|
||||
|
||||
RETURN v_new_count;
|
||||
END;
|
||||
$$;
|
||||
|
||||
REVOKE ALL ON FUNCTION public.increment_daily_usage(uuid, text, integer)
|
||||
FROM PUBLIC, anon, authenticated;
|
||||
GRANT EXECUTE ON FUNCTION public.increment_daily_usage(uuid, text, integer)
|
||||
TO service_role;
|
||||
|
||||
-- ----------------------------------------------------------------------------
|
||||
-- 3. daily_usage.count is a usage counter; negative totals are never valid.
|
||||
-- Legitimate decrements (quota reservation release) already clamp with
|
||||
-- greatest(count - 1, 0). Any negative row can only come from the abuse
|
||||
-- above, so it is normalised to 0 before the constraint is validated.
|
||||
-- ----------------------------------------------------------------------------
|
||||
UPDATE public.daily_usage
|
||||
SET count = 0
|
||||
WHERE count < 0;
|
||||
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM pg_constraint
|
||||
WHERE conrelid = 'public.daily_usage'::regclass
|
||||
AND conname = 'daily_usage_count_nonnegative'
|
||||
) THEN
|
||||
ALTER TABLE public.daily_usage
|
||||
ADD CONSTRAINT daily_usage_count_nonnegative CHECK (count >= 0);
|
||||
END IF;
|
||||
END;
|
||||
$$;
|
||||
|
||||
-- ----------------------------------------------------------------------------
|
||||
-- 4. Platform analytics RPCs: service_role only. Their bodies have no role
|
||||
-- predicate and run as definer (RLS bypassed), so no end-user role may call
|
||||
-- them. The admin console reads Supabase with the service-role key, like
|
||||
-- the other admin_*_v1 RPCs.
|
||||
-- ----------------------------------------------------------------------------
|
||||
REVOKE ALL ON FUNCTION public.admin_usage_by_feature(date, date)
|
||||
FROM PUBLIC, anon, authenticated;
|
||||
GRANT EXECUTE ON FUNCTION public.admin_usage_by_feature(date, date)
|
||||
TO service_role;
|
||||
|
||||
REVOKE ALL ON FUNCTION public.admin_top_users(date, date, integer)
|
||||
FROM PUBLIC, anon, authenticated;
|
||||
GRANT EXECUTE ON FUNCTION public.admin_top_users(date, date, integer)
|
||||
TO service_role;
|
||||
|
||||
REVOKE ALL ON FUNCTION public.admin_dau(date, date)
|
||||
FROM PUBLIC, anon, authenticated;
|
||||
GRANT EXECUTE ON FUNCTION public.admin_dau(date, date)
|
||||
TO service_role;
|
||||
|
||||
COMMENT ON FUNCTION public.increment_daily_usage(uuid, text, integer) IS
|
||||
'service_role only. Adds p_amount to today''s usage counter, clamped at 0.';
|
||||
COMMENT ON FUNCTION public.admin_usage_by_feature(date, date) IS
|
||||
'service_role only. Platform analytics; no caller role predicate in body.';
|
||||
COMMENT ON FUNCTION public.admin_top_users(date, date, integer) IS
|
||||
'service_role only. Returns user ids and names; no caller role predicate in body.';
|
||||
COMMENT ON FUNCTION public.admin_dau(date, date) IS
|
||||
'service_role only. Platform analytics; no caller role predicate in body.';
|
||||
|
||||
-- ----------------------------------------------------------------------------
|
||||
-- 5. Self-check: fail the migration if any public SECURITY DEFINER function is
|
||||
-- still executable by anon.
|
||||
-- ----------------------------------------------------------------------------
|
||||
DO $$
|
||||
DECLARE
|
||||
offenders text;
|
||||
BEGIN
|
||||
SELECT string_agg(p.oid::regprocedure::text, ', ' ORDER BY p.oid::regprocedure::text)
|
||||
INTO offenders
|
||||
FROM pg_proc p
|
||||
WHERE p.pronamespace = 'public'::regnamespace
|
||||
AND p.prosecdef
|
||||
AND has_function_privilege('anon', p.oid, 'EXECUTE');
|
||||
|
||||
IF offenders IS NOT NULL THEN
|
||||
RAISE EXCEPTION 'function_acl_hardening: anon can still execute SECURITY DEFINER functions: %',
|
||||
offenders;
|
||||
END IF;
|
||||
END;
|
||||
$$;
|
||||
Loading…
Add table
Add a link
Reference in a new issue