From 8966ac3f2ef060a36c72be9603d76b0e08d0e1f4 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 31 Jul 2026 07:46:39 +0000 Subject: [PATCH 1/2] fix(ads): revoke PUBLIC execute on the SECURITY DEFINER ad RPCs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phases 4-6 each ended with "revoke execute ... from anon, authenticated; grant execute ... to service_role", which reads as service-role-only and is not. Postgres grants EXECUTE to PUBLIC by default on every new function, and anon/authenticated inherit it — revoking their explicit grants leaves the PUBLIC grant untouched. The ACL stayed "=X/postgres | postgres=X/postgres | service_role=X/postgres", where the leading =X is PUBLIC, and has_function_privilege('anon', …) was true. ad_charge_click is SECURITY DEFINER, so anyone holding the publishable anon key could POST /rest/v1/rpc/ad_charge_click and forge valid clicks against any funded campaign — draining its credits and daily budget — while accruing publisher earnings to a slot they own, which the self-deal guard permits because slot owner and campaign owner differ. The ad_payouts solvency trigger capped the cash blast radius at lifetime deposits and no payout has ever executed, so nothing was withdrawn. But advertiser credits and every delivery metric were forgeable. ad_apply_deposit_bonus was reachable the same way. Revoking from PUBLIC is what actually removes it. Both callers use the service-role client and keep their own explicit grant, so no app access is lost. Verified in prod: anon and authenticated now false on all three, service_role still true. Left alone: ad_account_series / ad_campaign_totals are SECURITY INVOKER and scope every row to owner_id = auth.uid(), so an anon caller gets an empty result, and authenticated needs them. Found by the Supabase security advisor (anon_security_definer_function_executable). Co-Authored-By: Claude Opus 5 (1M context) --- .../20260731160000_ad_rpc_revoke_public.sql | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 supabase/migrations/20260731160000_ad_rpc_revoke_public.sql diff --git a/supabase/migrations/20260731160000_ad_rpc_revoke_public.sql b/supabase/migrations/20260731160000_ad_rpc_revoke_public.sql new file mode 100644 index 00000000..93ed2539 --- /dev/null +++ b/supabase/migrations/20260731160000_ad_rpc_revoke_public.sql @@ -0,0 +1,48 @@ +-- Ad network: actually lock the money RPCs down. +-- +-- Phases 4-6 each ended with: +-- +-- revoke execute on function public.ad_charge_click(...) from anon, authenticated; +-- grant execute on function public.ad_charge_click(...) to service_role; +-- +-- which reads as "only the service role can call this" and is not. Postgres +-- grants EXECUTE to PUBLIC by default on every new function, and anon / +-- authenticated inherit it. Revoking their *explicit* grants leaves the PUBLIC +-- one untouched, so the ACL stayed: +-- +-- =X/postgres | postgres=X/postgres | service_role=X/postgres +-- ^^ the leading "=X" is PUBLIC — has_function_privilege('anon', …) = true +-- +-- ad_charge_click is SECURITY DEFINER, so until this migration anyone holding +-- the publishable anon key could POST /rest/v1/rpc/ad_charge_click and: +-- * forge valid clicks against any funded campaign, draining its credits and +-- its daily budget at will, and +-- * accrue publisher earnings to a slot they own (the self-deal guard only +-- compares slot owner against campaign owner, and here they differ). +-- The ad_payouts solvency trigger capped the cash blast radius at lifetime +-- deposits, and no payout has ever been executed, so nothing was actually +-- withdrawn — but advertiser credits and every delivery metric were forgeable. +-- +-- ad_apply_deposit_bonus was reachable the same way. +-- +-- Revoking from PUBLIC is what actually removes it. Both callers +-- (lib/ads/serve.ts, lib/credits-finalize.ts) use the service-role client and +-- hold their own explicit grant, so nothing in the app loses access. +-- +-- NOT applied to ad_account_series / ad_campaign_totals: those are SECURITY +-- INVOKER and scope every row to `owner_id = auth.uid()`, so an anon caller +-- gets an empty result rather than someone else's data. They need to stay +-- callable by authenticated. +-- +-- Apply via psql over the pooler / MCP (prod history diverged), not `db push`. + +revoke execute on function + public.ad_charge_click(uuid,uuid,uuid,uuid,text,text,text,text,int,numeric) + from public; + +revoke execute on function public.ad_apply_deposit_bonus(text) from public; + +-- Trigger functions have no business being callable over the REST surface at +-- all; this one only ever runs from the ad_payouts trigger. +revoke execute on function public.ad_payout_solvency_guard() + from public, anon, authenticated; From c9cb2aa23faaaebfb2a1b7a36a1b3263a1eebc3e Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 31 Jul 2026 07:53:29 +0000 Subject: [PATCH 2/2] fix: revoke PUBLIC execute on the remaining SECURITY DEFINER functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweeps the same PUBLIC-grant bug as the previous commit across the other 15 SECURITY DEFINER functions in the database. The sharpest is consume_credit(p_owner, p_count): SECURITY DEFINER, and p_count is signed because reconcilePromo calls it with a negative count to refund. Any holder of the publishable anon key could POST consume_credit('', -1000000) and mint themselves credits — and those land in credits_balance without touching promo_credits, so the ad network would treat them as cash-backed and let them be withdrawn as USDC, walking straight through the solvency work. Also locked: credit_purchase_complete, consume/refund_article_*, consume_alert_serp_budget, bump_autoblog_integration, lx_find_internal_links, get_public_audit/get_public_findings, and the handle_new_user / create_default_org_for_profile / rls_auto_enable trigger functions. Every caller was traced before revoking. worker/index.ts builds its client from SUPABASE_SERVICE_ROLE_KEY and the route handlers and server actions all use serviceClient(), so nothing loses access. get_public_audit is called with svc from a server component (never the browser) and get_public_findings has no caller at all, so the public share page is unaffected. Deliberately NOT revoked: the six is_org_*/is_project_*/project_owner_id RLS helpers. They are called from inside 48 policies across 21 tables, and a policy is evaluated as the querying role — revoking would break RLS rather than harden it. All 48 have polroles = '{0}' (PUBLIC), so anon is in scope too. The residual exposure is a boolean membership probe that requires already knowing both uuids and returns no data. Verified in prod: all 15 now anon=false authenticated=false service_role=true, and an authenticated smoke test still resolves RLS correctly (projects=35 campaigns=34 slots=24 audits=2344). Co-Authored-By: Claude Opus 5 (1M context) --- ...260731170000_revoke_public_secdef_rest.sql | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 supabase/migrations/20260731170000_revoke_public_secdef_rest.sql diff --git a/supabase/migrations/20260731170000_revoke_public_secdef_rest.sql b/supabase/migrations/20260731170000_revoke_public_secdef_rest.sql new file mode 100644 index 00000000..60e7d780 --- /dev/null +++ b/supabase/migrations/20260731170000_revoke_public_secdef_rest.sql @@ -0,0 +1,80 @@ +-- Same PUBLIC-grant bug as 20260731160000, swept across the rest of the +-- SECURITY DEFINER functions in this database. +-- +-- Recap: Postgres grants EXECUTE to PUBLIC by default on every new function. +-- "revoke ... from anon, authenticated" removes their explicit grants and +-- leaves PUBLIC's, which anon and authenticated inherit — so functions that +-- read as service-role-only were callable over /rest/v1/rpc/ by anyone +-- holding the publishable anon key. +-- +-- The sharpest one here is consume_credit(p_owner, p_count). It is SECURITY +-- DEFINER and p_count is signed — lib/promote/reconcilePromo.ts calls it with a +-- negative count to refund. So any caller could run +-- consume_credit('', -1000000) and mint themselves credits. Those +-- credits land in credits_balance without touching promo_credits, so the ad +-- network would treat them as CASH-BACKED and let them be withdrawn as USDC — +-- which walks straight through the solvency work in 20260731120000. +-- +-- Every caller of every function below was traced first; all of them use the +-- service-role client, so nothing loses access: +-- * worker/index.ts builds its client from SUPABASE_SERVICE_ROLE_KEY, which +-- covers generateArticle -> consume/refund_article_*, generateGuestPost and +-- processDuePromoteLists/processBrowserPost -> consume_credit, and +-- articleGen -> lx_find_internal_links. +-- * The route handlers and server actions all use serviceClient(): +-- credits-finalize + coinpay webhook, recent-outreach, scheduled-audits, +-- apply-fix, alerts, the outrank/crawlproof webhooks, and app/r/[token]. +-- * get_public_findings has no caller at all; get_public_audit is invoked +-- with svc from a server component, never from the browser, so the public +-- share page keeps working. +-- +-- Trigger and event-trigger functions are revoked outright: they only ever run +-- from their trigger, never over REST. + +-- Privileged RPCs -> service role only. +revoke execute on function public.consume_credit(uuid,integer) from public, anon, authenticated; +grant execute on function public.consume_credit(uuid,integer) to service_role; + +revoke execute on function public.credit_purchase_complete(text,jsonb) from public, anon, authenticated; +grant execute on function public.credit_purchase_complete(text,jsonb) to service_role; + +revoke execute on function public.consume_article_generation(uuid,uuid) from public, anon, authenticated; +grant execute on function public.consume_article_generation(uuid,uuid) to service_role; + +revoke execute on function public.refund_article_entitlement(uuid,uuid) from public, anon, authenticated; +grant execute on function public.refund_article_entitlement(uuid,uuid) to service_role; + +revoke execute on function public.consume_alert_serp_budget(uuid,integer,integer) from public, anon, authenticated; +grant execute on function public.consume_alert_serp_budget(uuid,integer,integer) to service_role; + +revoke execute on function public.bump_autoblog_integration(uuid) from public, anon, authenticated; +grant execute on function public.bump_autoblog_integration(uuid) to service_role; + +revoke execute on function public.lx_find_internal_links(uuid,public.vector,integer,boolean) from public, anon, authenticated; +grant execute on function public.lx_find_internal_links(uuid,public.vector,integer,boolean) to service_role; + +revoke execute on function public.get_public_audit(text) from public, anon, authenticated; +grant execute on function public.get_public_audit(text) to service_role; + +revoke execute on function public.get_public_findings(text) from public, anon, authenticated; +grant execute on function public.get_public_findings(text) to service_role; + +-- Trigger / event-trigger functions: never reachable over REST. +revoke execute on function public.handle_new_user() from public, anon, authenticated; +revoke execute on function public.create_default_org_for_profile() from public, anon, authenticated; +revoke execute on function public.rls_auto_enable() from public, anon, authenticated; + +-- DELIBERATELY NOT REVOKED: is_org_member, is_org_owner, is_org_wide_member, +-- is_project_editor, is_project_member, project_owner_id. +-- +-- These are RLS helpers, called from inside 48 policies across 21 tables. A +-- policy expression is evaluated as the querying role, so that role needs +-- EXECUTE on anything the policy calls — revoking would not harden RLS, it +-- would break it. Every one of those 48 policies has polroles = '{0}' (PUBLIC), +-- so anon is in scope too and can't be revoked either. +-- +-- What that leaves exposed is small and bounded: each takes explicit uuids and +-- returns a boolean, so a caller who already knows both a user id and an org or +-- project id can probe membership. No data is returned. Closing it means +-- rewriting the policies to inline the checks, which is a much larger and +-- riskier change than the leak justifies.