From 57d757192e9142542f654f83425ba18fccabfa19 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Tue, 1 Sep 2026 14:00:36 +0200 Subject: [PATCH] fix(security): revoke anon EXECUTE on SECURITY DEFINER write RPCs (#2106) * fix(security): revoke anon EXECUTE on SECURITY DEFINER write RPCs Supabase's bootstrap ALTER DEFAULT PRIVILEGES grants EXECUTE on every public-schema function to PUBLIC, anon, authenticated and service_role, and PostgREST publishes each one at /rest/v1/rpc/. Seven SECURITY DEFINER functions were therefore unauthenticated cross-tenant primitives that bypass RLS for anyone holding the public anon key: sync_team_to_company INSERTs into company_members, the table user_company_ids() and every RLS policy read claim_due_webhook_deliveries returns every tenant's webhook payloads generate_delivery_note_number UPDATEs company_settings, needs only a company id generate_article_number UPDATEs company_settings and articles generate_invoice_number UPDATEs company_settings and invoices peek_next_invoice_number leaks another tenant's prefix and next number get_next_arrival_number leaks another tenant's ankomstnummer series The last three carried a guard, and it did not hold. Its shape is "IF auth.uid() IS NOT NULL AND NOT EXISTS (membership) THEN RAISE", with a comment explaining that a NULL auth.uid() means service role or cron and is trusted. The anon key's JWT carries no sub claim, so auth.uid() is NULL for role anon as well, and the guard short-circuits straight into the trusted branch. Revokes EXECUTE from PUBLIC and anon on all seven, and from authenticated on the two with no user-session caller. PUBLIC is mandatory: proacl carries "=X/postgres", so revoking from anon alone leaves has_function_privilege true. The five numbering RPCs the app calls on the user's session client are re-granted to authenticated only after their guard was replaced with a fail-closed one. A sweep then revokes PUBLIC and anon from every remaining definer writer in public; all 20 carry explicit authenticated and service_role grants, verified against prod, so no signed-in or service path changes. tests/pg/definer-function-grants.pg.test.ts generates its assertion from that same sweep rather than a hand list, because hand-listing is exactly how the three guarded numbering RPCs came to be declared safe. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016ifKg6Ec67A39oxfGPU1yc * fix(security): keep procedures out of the definer-writer sweep Review finding (CodeRabbit, Major): the sweep predicates excluded only trigger return types, so a SECURITY DEFINER PROCEDURE would match and the migration would then run REVOKE ... ON FUNCTION against it, which Postgres rejects, aborting the whole migration. The pg test's offender query had the same gap and would have reported a procedure the migration could not fix. public holds no procedures today (verified against prod 2026-09-01), so this is a guard against the first one anyone adds rather than a live bug. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016ifKg6Ec67A39oxfGPU1yc --------- Co-authored-by: Claude Opus 5 (1M context) --- DECISIONS.md | 2 + ..._revoke_anon_execute_on_definer_writes.sql | 639 ++++++++++++++++++ tests/pg/definer-function-grants.pg.test.ts | 363 ++++++++++ 3 files changed, 1004 insertions(+) create mode 100644 supabase/migrations/20260901100000_revoke_anon_execute_on_definer_writes.sql create mode 100644 tests/pg/definer-function-grants.pg.test.ts diff --git a/DECISIONS.md b/DECISIONS.md index 93674dc8..4974e56e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1427,4 +1427,6 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-01] multi_user skeptic fixes: Stripe cancel EXPIRES the multi_user stripe grant instead of deleting it (grace anchor; other grants still deleted per freeze-and-retain); app-side state checks go RPC-first via SECURITY DEFINER company_multi_user_state (capability_grants RLS hides team rows from non-team users, byrå clients would misread as frozen); byra-kind teams get a standing team-scoped multi_user grant via backfill + teams trigger (WL-10 assumption made real; partner billing is out-of-band); PGRST202 on resolution fails OPEN (pre-migration DB has no multi_user rows: gated fallback would freeze all non-owners); /api/v1 got the same dormancy gate as MCP. RLS-level enforcement and the mid-session API fallback write-back window stay v2 follow-ups (documented, same class as pre-existing stale-preference fallback). [2026-09-01] Declined CodeRabbit's UpgradeNote suggestion (PR #1758 follow-up) to append the self-host connector sentence to children instead of replacing them: every caller's children is hosted subscription copy ("... kräver ett abonnemang"), so appending would show subscription wording on a self-host, the exact thing the branch exists to avoid; the "CSV/SIE import stays free" text it cited is a code comment in BankSyncNowButton, not children. Replace-on-self-host stays; a dedicated selfHosted children prop can come when a caller actually needs per-panel reassurance there. [2026-09-01] getConnectorConfig() rebuilds baseUrl as origin + path (userinfo/query/fragment stripped, warn-logged without the raw value): /api/connector/status echoes baseUrl to the operator and the sync/proxy URLs get paths appended, so nothing secret-shaped pasted into GNUBOK_CONNECT_URL may survive; the stripped parts were never meaningful in a base URL. The status route is also Cache-Control: no-store (key prefix + wiring layout out of shared browser caches). +[2026-09-01] Anon-callable SECURITY DEFINER writes: the guard shape `IF auth.uid() IS NOT NULL AND NOT EXISTS (membership)` is unsafe on its own. The anon JWT carries no `sub` claim, so auth.uid() is NULL for role anon too and the guard short-circuits into the trusted branch. It is defense in depth behind a REVOKE FROM PUBLIC, anon, never a substitute for one. Every new SECURITY DEFINER function ships with that REVOKE; tests/pg/definer-function-grants.pg.test.ts enforces it from a sweep rather than a hand list, because hand-listing is exactly how three guarded numbering RPCs were wrongly declared safe. +[2026-09-01] public.create_invoice_with_items(jsonb,jsonb) is revoked, not dropped. It is prod-only, uncalled and already non-functional, so removing it is cleanup rather than security, and the revoke closes the hole in full. An irreversible schema deletion against production belongs in its own reviewable migration. [2026-09-01] SKV connector refresh classification fixed broker-side (skeptic refutation on PR #2103, found independently by two skeptics): the broker's /oauth/token catch-all had collapsed SKV's terminal dead-refresh-token dialects (404 id_not_found, 400 invalid_grant, "Refresh Token status is expired": the DOMINANT refresh outcome, per-flow tokens live 65 min) into the generic 502, so a connector instance could never classify ordinary session expiry: raw English 500s instead of the reconnect flow, staged filing ops consumed as non-recoverable, crons retrying raw forever. The broker now re-codes those dialects (refresh grant only, never the code exchange where invalid_grant means an expired one-shot code) as 401 CONNECTOR_SKV_REFRESH_DEAD, which the instance maps to SESSION_EXPIRED; the generic 502 remains raw so a transient SKV outage still never re-arms the reconnect banner (#1155). Same pass: the data proxy now forwards WWW-Authenticate + x-skv-*/x-amzn-*/x-api-* response headers (the instance's MISSING_SCOPE classification reads them; nothing secret rides in them), and the instance's gateway-refusal guidance is connector-aware (a self-host has no SKATTEVERKET_APIGW_CLIENT_ID or Utvecklarportalen access: point at /api/connector/status + support instead). diff --git a/supabase/migrations/20260901100000_revoke_anon_execute_on_definer_writes.sql b/supabase/migrations/20260901100000_revoke_anon_execute_on_definer_writes.sql new file mode 100644 index 00000000..1c399d6b --- /dev/null +++ b/supabase/migrations/20260901100000_revoke_anon_execute_on_definer_writes.sql @@ -0,0 +1,639 @@ +-- P0 security fix: SECURITY DEFINER write RPCs were EXECUTE-able by `anon`. +-- +-- State before this migration (verified against production 2026-09-01): +-- +-- * Supabase's bootstrap runs +-- ALTER DEFAULT PRIVILEGES IN SCHEMA public +-- GRANT ALL ON FUNCTIONS TO postgres, anon, authenticated, service_role; +-- so every function this repo has ever shipped came out of CREATE with +-- proacl {=X/postgres, postgres=X, anon=X, authenticated=X, service_role=X}. +-- The leading `=X/postgres` is the grant to PUBLIC, and anon is a member of +-- PUBLIC: revoking from anon alone leaves has_function_privilege('anon', ...) +-- TRUE. Every REVOKE below therefore names PUBLIC first. Same trap as +-- 20260727120000 (replace_sie_import), which is the precedent for this file. +-- +-- * PostgREST exposes every public function as POST /rest/v1/rpc/, and +-- the anon key is public by design, so these eight were unauthenticated +-- cross-tenant primitives: +-- +-- sync_team_to_company(uuid,uuid) INSERTs into company_members, +-- the table user_company_ids() +-- and every RLS policy read. No +-- auth check of any kind. +-- claim_due_webhook_deliveries(int,tstz) RETURNS every tenant's webhook +-- payload bodies and flips the +-- rows to in_flight. No auth +-- check; p_now is caller-supplied +-- so backoff does not limit it. +-- generate_delivery_note_number(uuid) UPDATEs company_settings for an +-- arbitrary company. No auth check. +-- generate_article_number(uuid,uuid) UPDATEs company_settings and +-- articles. No auth check. +-- check_and_increment_inbox_quota(uuid,integer,integer) +-- INSERTs and UPDATEs +-- inbox_rate_counters for an +-- arbitrary company. No auth +-- check, so any caller can run +-- another tenant's minute and +-- day intake quota to the cap +-- and stall their receipt +-- intake until the window +-- rolls. +-- generate_invoice_number(uuid,uuid,text) UPDATEs company_settings and +-- invoices (guard bypassed, below). +-- peek_next_invoice_number(uuid,text) Leaks another tenant's +-- invoice_prefix and next number. +-- get_next_arrival_number(uuid) Leaks another tenant's +-- ankomstnummer series. +-- +-- * The last three DID carry a guard, and it did not hold: +-- +-- IF auth.uid() IS NOT NULL AND NOT EXISTS (membership) THEN RAISE +-- +-- auth.uid() resolves the JWT `sub` claim. The anon key's JWT carries no +-- `sub`, so auth.uid() is NULL for role anon and the guard short-circuits +-- straight into the trusted branch. A NULL-trusted guard is safe ONLY once +-- anon has lost EXECUTE: it is the second layer, never the first, and it +-- was never the reason those functions were safe. +-- +-- What this migration does: +-- +-- 1. Revokes EXECUTE from PUBLIC and anon on all eight, plus from +-- authenticated on the two that have no user-session caller. +-- 2. Re-grants EXECUTE to authenticated on the six the app genuinely calls +-- on the user's session client (the five numbering RPCs plus the inbox +-- intake limiter), after replacing the NULL-trusted guard with a +-- fail-closed one, or adding one where there was none (see the guard +-- comment in generate_invoice_number below for the exact trust rule). +-- 3. Revokes every grant on public.create_invoice_with_items(jsonb,jsonb), +-- a prod-only leftover that no migration in this repo ever created, +-- nothing calls, and that raises 23502 on every call anyway. It is +-- revoked rather than dropped: see section 3. +-- 4. Sweeps the rest: every remaining SECURITY DEFINER, non-trigger function +-- in public whose body writes loses EXECUTE from PUBLIC and anon. +-- authenticated and service_role keep the explicit grants they already +-- hold (verified: all 20 anon-callable definer writers carry an explicit +-- `authenticated=X` entry, so revoking PUBLIC takes nothing away from a +-- signed-in caller). The sweep is what stops this class from recurring on +-- the next function that ships, and it is the same predicate +-- tests/pg/definer-function-grants.pg.test.ts asserts on. +-- +-- service_role keeps EXECUTE throughout: the API-key, MCP and cron paths run on +-- createServiceClientNoCookies(), and pg_cron runs as postgres. +-- +-- Callers verified by grep before revoking (repo at 5f81c0638): +-- * sync_team_to_company: no TypeScript caller at all. The only callers are +-- create_company_with_owner, create_company_for_user and +-- create_company_for_brand_signup, all SECURITY DEFINER owned by postgres, +-- so the nested EXECUTE check passes as the owner. No re-grant. +-- * claim_due_webhook_deliveries: lib/webhooks/dispatcher.ts is the sole +-- caller, reached only from app/api/webhooks/dispatch/cron/route.ts and +-- lib/webhooks/dispatch-kick.ts, both on createServiceClientNoCookies(). +-- No re-grant. +-- * generate_invoice_number: lib/invoices/ensure-invoice-number.ts, on the +-- route's session client (app/api/invoices/route.ts and the finalize/send/ +-- convert/mark-sent routes). Re-granted. +-- * peek_next_invoice_number: app/api/invoices/next-number/route.ts, session +-- client. Re-granted. +-- * get_next_arrival_number: app/api/supplier-invoices/route.ts, the credit +-- routes, lib/pending-operations/commit.ts and the invoice-inbox extension; +-- session client on the app routes, service client on the /api/v1 and MCP +-- paths. Re-granted. +-- * generate_delivery_note_number: app/api/invoices/route.ts (session client) +-- and app/api/v1/companies/[companyId]/invoices/route.ts (service client). +-- Re-granted. +-- * generate_article_number: lib/articles/ensure-article-number.ts, from +-- app/api/articles/route.ts, app/api/import/articles/execute/route.ts and +-- lib/pending-operations/commit.ts. Re-granted. +-- * check_and_increment_inbox_quota: lib/rate-limits/inbox.ts, called from +-- extensions/general/invoice-inbox/index.ts on the user's session client +-- (ctx.supabase) at /upload, /upload/create, /items/:id/extracted-data, +-- /items/:id/retry-extraction, /inbox/domain and /inbox/domain/verify, and +-- on a service-role client at /inbound (createServiceRoleClient built with +-- SUPABASE_SERVICE_ROLE_KEY); whatsapp-inbox's lib/process-inbound.ts +-- forwards the createServiceClientNoCookies() client its webhook-kick and +-- sweep callers create. Both shapes are live, so this one keeps +-- authenticated EXECUTE and gets the in-body guard rather than a blanket +-- revoke. Re-granted. The TypeScript helper fails open on RPC error, so a +-- refusal degrades to "this request was not counted", never a 500 for a +-- real user; the cross-tenant write is what the guard stops. +-- +-- Not touched on purpose: public._backfill_remaining_20260817. It is 337 rows +-- of pre-backfill invoice remaining/paid/deduction state, i.e. financial +-- rollback data, already locked down by 20260825170000. Dropping it is a +-- separate founder decision, not part of a security revoke. +-- +-- pg-test: tests/pg/definer-function-grants.pg.test.ts + +-- --------------------------------------------------------------------------- +-- 1. No user-session caller: revoke outright, service_role only. +-- --------------------------------------------------------------------------- + +REVOKE EXECUTE ON FUNCTION public.sync_team_to_company(uuid, uuid) + FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.sync_team_to_company(uuid, uuid) TO service_role; + +COMMENT ON FUNCTION public.sync_team_to_company(uuid, uuid) IS + 'Copies team_members into company_members for a newly created company. Has no authorization check of its own, so it is callable only from the SECURITY DEFINER company-creation RPCs (which do check) and from service_role. Not callable by anon or authenticated.'; + +REVOKE EXECUTE ON FUNCTION public.claim_due_webhook_deliveries(integer, timestamptz) + FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.claim_due_webhook_deliveries(integer, timestamptz) TO service_role; + +COMMENT ON FUNCTION public.claim_due_webhook_deliveries(integer, timestamptz) IS + 'Claims due automation_webhooks deliveries across all tenants for the cron sender (FOR UPDATE SKIP LOCKED). Returns raw payload bodies, so it is service_role only. Not callable by anon or authenticated.'; + +-- --------------------------------------------------------------------------- +-- 2. The six RPCs the app calls on the user's session client: the five +-- numbering ones and the inbox intake limiter. +-- Fail-closed guard first, then least-privilege grants. +-- +-- Bodies below are the production definitions verbatim (pg_get_functiondef, +-- 2026-09-01) with the guard block replaced or added. search_path is +-- restated per function because CREATE OR REPLACE drops every setting +-- attached via ALTER FUNCTION ... SET, and peek_next_invoice_number restates +-- STABLE for the same reason. +-- --------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION public.generate_invoice_number( + p_company_id uuid, + p_invoice_id uuid, + p_document_type text DEFAULT 'invoice' +) +RETURNS text +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $function$ +DECLARE + v_existing text; + v_prefix text; + v_number integer; + v_final text; + v_trusted boolean; +BEGIN + -- Fail-closed authorization gate. A caller is trusted without a membership + -- row only when it has no auth.uid() AND is one of: + -- * service_role: the cookieless server client (API-key, MCP, cron routes) + -- and, in tests, runAsServiceRole(). + -- * a direct database connection that is not PostgREST: psql, pg_cron and + -- the pg-real harness, all of which could bypass the function anyway. + -- Everything reaching this function over PostgREST presents a role claim, so + -- an anon-key caller lands in the membership check with a NULL auth.uid(), + -- finds no row and is refused. The old shape (`IF auth.uid() IS NOT NULL AND + -- NOT EXISTS ...`) trusted exactly that caller. This is defense in depth on + -- top of the REVOKE below, not a replacement for it. + -- + -- COALESCE, not a bare `auth.role() = 'service_role'`: with no role claim + -- that comparison is NULL, the OR keeps it NULL, and `IF NOT NULL AND ...` + -- is not TRUE, so the RAISE would be skipped. The gate has to stay strictly + -- two-valued or it fails open on the caller it exists to stop. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + SELECT invoice_number INTO v_existing + FROM public.invoices + WHERE id = p_invoice_id AND company_id = p_company_id + FOR UPDATE; + + IF NOT FOUND THEN + RAISE EXCEPTION 'Invoice % not found in company %', p_invoice_id, p_company_id; + END IF; + + IF v_existing IS NOT NULL THEN + RETURN v_existing; + END IF; + + UPDATE public.company_settings + SET next_invoice_number = next_invoice_number + 1, + updated_at = now() + WHERE company_id = p_company_id + RETURNING invoice_prefix, next_invoice_number - 1 + INTO v_prefix, v_number; + + IF v_number IS NULL THEN + RAISE EXCEPTION 'Company settings not found for company %', p_company_id; + END IF; + + v_final := CASE + WHEN p_document_type = 'proforma' THEN 'PF-' + ELSE COALESCE(v_prefix, '') + END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0'); + + UPDATE public.invoices + SET invoice_number = v_final + WHERE id = p_invoice_id AND company_id = p_company_id; + + RETURN v_final; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.peek_next_invoice_number( + p_company_id uuid, + p_document_type text DEFAULT 'invoice' +) +RETURNS text +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $function$ +DECLARE + v_prefix text; + v_number integer; + v_trusted boolean; +BEGIN + -- Same fail-closed rule as generate_invoice_number; see the comment there. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + SELECT invoice_prefix, next_invoice_number INTO v_prefix, v_number + FROM public.company_settings + WHERE company_id = p_company_id; + + IF v_number IS NULL THEN + RETURN NULL; + END IF; + + RETURN CASE + WHEN p_document_type = 'proforma' THEN 'PF-' + ELSE COALESCE(v_prefix, '') + END || LPAD(v_number::text, GREATEST(3, length(v_number::text)), '0'); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.get_next_arrival_number(p_company_id uuid) +RETURNS integer +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $function$ +DECLARE + v_floor integer; + v_next integer; + v_trusted boolean; +BEGIN + -- Same fail-closed rule as generate_invoice_number; see the comment there. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + -- Configured start floor (defaults to 1 for every company; NULL only if the + -- settings row is missing, in which case COALESCE keeps the old behavior). + SELECT COALESCE(next_arrival_number, 1) INTO v_floor + FROM public.company_settings + WHERE company_id = p_company_id; + + SELECT GREATEST(COALESCE(MAX(arrival_number), 0) + 1, COALESCE(v_floor, 1)) + INTO v_next + FROM public.supplier_invoices + WHERE company_id = p_company_id; + + RETURN v_next; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.generate_delivery_note_number(p_company_id uuid) +RETURNS text +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = 'public' +AS $function$ +DECLARE + v_number INTEGER; + v_year TEXT; + v_trusted boolean; +BEGIN + -- New gate: this function had no authorization check at all, so a caller + -- holding only the anon key could burn another tenant's delivery-note + -- series. Same fail-closed rule as generate_invoice_number. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + UPDATE public.company_settings + SET next_delivery_note_number = next_delivery_note_number + 1, + updated_at = now() + WHERE company_id = p_company_id + RETURNING next_delivery_note_number - 1 + INTO v_number; + + IF v_number IS NULL THEN + RAISE EXCEPTION 'Company settings not found for company %', p_company_id; + END IF; + + v_year := EXTRACT(YEAR FROM CURRENT_DATE)::TEXT; + RETURN 'FS-' || v_year || LPAD(v_number::TEXT, 3, '0'); +END; +$function$; + +CREATE OR REPLACE FUNCTION public.generate_article_number( + p_company_id uuid, + p_article_id uuid +) +RETURNS text +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = 'public' +AS $function$ +DECLARE + v_existing text; + v_number integer; + v_final text; + v_trusted boolean; +BEGIN + -- New gate: this function had no authorization check at all. Same + -- fail-closed rule as generate_invoice_number. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + SELECT article_number INTO v_existing + FROM public.articles + WHERE id = p_article_id AND company_id = p_company_id + FOR UPDATE; + + IF NOT FOUND THEN + RAISE EXCEPTION 'Article % not found in company %', p_article_id, p_company_id; + END IF; + + IF v_existing IS NOT NULL THEN + RETURN v_existing; + END IF; + + UPDATE public.company_settings + SET next_article_number = next_article_number + 1, + updated_at = now() + WHERE company_id = p_company_id + RETURNING next_article_number - 1 + INTO v_number; + + IF v_number IS NULL THEN + RAISE EXCEPTION 'Company settings not found for company %', p_company_id; + END IF; + + v_final := v_number::text; + + UPDATE public.articles + SET article_number = v_final + WHERE id = p_article_id AND company_id = p_company_id; + + RETURN v_final; +END; +$function$; + +CREATE OR REPLACE FUNCTION public.check_and_increment_inbox_quota( + p_company_id uuid, + p_minute_max integer, + p_day_max integer +) +RETURNS jsonb +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = 'public' +AS $function$ +DECLARE + v_minute_key text := to_char(now() AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI'); + v_day_key text := to_char(now() AT TIME ZONE 'Europe/Stockholm', 'YYYY-MM-DD'); + v_minute_count integer; + v_day_count integer; + v_trusted boolean; +BEGIN + -- New gate: this function had no authorization check at all, and it keeps + -- authenticated EXECUTE because both a session client and a service client + -- call it (see the caller list at the top of this file). The REVOKE alone + -- therefore stops only the anon key: without this guard any signed-in user + -- could still run any other tenant's intake quota to the cap. Same + -- fail-closed rule as generate_invoice_number. + v_trusted := auth.uid() IS NULL + AND ( + COALESCE(auth.role(), '') = 'service_role' + OR (auth.role() IS NULL AND session_user <> 'authenticator') + ); + + IF NOT v_trusted AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE user_id = auth.uid() AND company_id = p_company_id + ) THEN + RAISE EXCEPTION 'unauthorized: caller is not a member of company %', p_company_id + USING ERRCODE = '42501'; + END IF; + + INSERT INTO public.inbox_rate_counters (company_id, window_kind, window_key, count) + VALUES (p_company_id, 'minute', v_minute_key, 1) + ON CONFLICT (company_id, window_kind, window_key) + DO UPDATE SET count = inbox_rate_counters.count + 1, updated_at = now() + RETURNING count INTO v_minute_count; + + IF v_minute_count > p_minute_max THEN + UPDATE public.inbox_rate_counters + SET count = count - 1 + WHERE company_id = p_company_id + AND window_kind = 'minute' + AND window_key = v_minute_key; + RETURN jsonb_build_object('ok', false, 'scope', 'minute', 'retry_after_sec', 60); + END IF; + + INSERT INTO public.inbox_rate_counters (company_id, window_kind, window_key, count) + VALUES (p_company_id, 'day', v_day_key, 1) + ON CONFLICT (company_id, window_kind, window_key) + DO UPDATE SET count = inbox_rate_counters.count + 1, updated_at = now() + RETURNING count INTO v_day_count; + + IF v_day_count > p_day_max THEN + UPDATE public.inbox_rate_counters + SET count = count - 1 + WHERE company_id = p_company_id + AND window_kind = 'day' + AND window_key = v_day_key; + UPDATE public.inbox_rate_counters + SET count = count - 1 + WHERE company_id = p_company_id + AND window_kind = 'minute' + AND window_key = v_minute_key; + RETURN jsonb_build_object('ok', false, 'scope', 'day', 'retry_after_sec', 3600); + END IF; + + RETURN jsonb_build_object('ok', true); +END; +$function$; + +REVOKE EXECUTE ON FUNCTION public.generate_invoice_number(uuid, uuid, text) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.generate_invoice_number(uuid, uuid, text) + TO authenticated, service_role; + +REVOKE EXECUTE ON FUNCTION public.peek_next_invoice_number(uuid, text) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.peek_next_invoice_number(uuid, text) + TO authenticated, service_role; + +REVOKE EXECUTE ON FUNCTION public.get_next_arrival_number(uuid) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.get_next_arrival_number(uuid) + TO authenticated, service_role; + +REVOKE EXECUTE ON FUNCTION public.generate_delivery_note_number(uuid) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.generate_delivery_note_number(uuid) + TO authenticated, service_role; + +REVOKE EXECUTE ON FUNCTION public.generate_article_number(uuid, uuid) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.generate_article_number(uuid, uuid) + TO authenticated, service_role; + +REVOKE EXECUTE ON FUNCTION public.check_and_increment_inbox_quota(uuid, integer, integer) + FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.check_and_increment_inbox_quota(uuid, integer, integer) + TO authenticated, service_role; + +COMMENT ON FUNCTION public.generate_invoice_number(uuid, uuid, text) IS + 'Allocates and persists the next invoice number for p_company_id. Requires the caller to be a member of the company; only a service_role or direct database connection with no auth.uid() is trusted without one. Raises 42501 otherwise. Not callable by anon.'; + +COMMENT ON FUNCTION public.peek_next_invoice_number(uuid, text) IS + 'Previews the next invoice number without consuming it. Requires the caller to be a member of p_company_id; only a service_role or direct database connection with no auth.uid() is trusted without one. Raises 42501 otherwise. Not callable by anon.'; + +COMMENT ON FUNCTION public.get_next_arrival_number(uuid) IS + 'Returns the next ankomstnummer for p_company_id. Requires the caller to be a member of the company; only a service_role or direct database connection with no auth.uid() is trusted without one. Raises 42501 otherwise. Not callable by anon.'; + +COMMENT ON FUNCTION public.generate_delivery_note_number(uuid) IS + 'Allocates the next delivery-note number for p_company_id. Requires the caller to be a member of the company; only a service_role or direct database connection with no auth.uid() is trusted without one. Raises 42501 otherwise. Not callable by anon.'; + +COMMENT ON FUNCTION public.generate_article_number(uuid, uuid) IS + 'Assigns the next article number to p_article_id. Requires the caller to be a member of p_company_id; only a service_role or direct database connection with no auth.uid() is trusted without one. Raises 42501 otherwise. Not callable by anon.'; + +COMMENT ON FUNCTION public.check_and_increment_inbox_quota(uuid, integer, integer) IS + 'Atomic per-company minute and day quota for document-inbox intake. Requires the caller to be a member of p_company_id; only a service_role or direct database connection with no auth.uid() is trusted without one (the inbound email and WhatsApp paths). Raises 42501 otherwise. Not callable by anon.'; + +-- --------------------------------------------------------------------------- +-- 3. Lock down public.create_invoice_with_items(jsonb, jsonb): dead on arrival. +-- +-- Prod-only leftover. No migration in this repo ever created it (see the +-- note at 20260304191528_set_search_path_on_functions.sql:9), and nothing +-- reaches it: no TypeScript caller, and no other function body, view, +-- policy, column default or pg_depend entry references it (read-only sweep +-- of pg_proc.prosrc, pg_views, pg_policies, information_schema.columns and +-- pg_depend, 2026-09-01). pg_stat_statements has no record of a call since +-- its last reset. +-- +-- It is also non-functional, and has been since the multi-tenant refactor: +-- it INSERTs into public.invoices without company_id, which is NOT NULL, so +-- every call raises 23502 before a row is written. +-- +-- We REVOKE rather than DROP. The revoke closes the security hole in full +-- (it is the same treatment section 4 gives every other definer writer), +-- and a DROP would be an irreversible schema deletion against production +-- for a function that is already inert. Removing it is cleanup, not +-- security, so it is left as a separate decision. If that decision is +-- taken, the DROP belongs in its own migration where it can be reviewed +-- and reverted on its own terms. +-- --------------------------------------------------------------------------- + +DO $$ +BEGIN + -- to_regprocedure returns NULL instead of raising when the signature does not + -- exist, which is the case for any database built from supabase/migrations/ + -- alone: locally and in CI this block is a no-op, and only prod has the + -- function to lock down. + IF to_regprocedure('public.create_invoice_with_items(jsonb, jsonb)') IS NOT NULL THEN + REVOKE ALL ON FUNCTION public.create_invoice_with_items(jsonb, jsonb) + FROM PUBLIC, anon, authenticated; + END IF; +END +$$; + +-- --------------------------------------------------------------------------- +-- 4. Sweep every other SECURITY DEFINER writer in public. +-- +-- These are functions with legitimate authenticated or service_role callers +-- (create_company_with_owner, delete_last_voucher, seed_chart_of_accounts, +-- validate_and_increment_api_key, ...). None of them has an anon-key call +-- path: every call site resolves either the user's session client or +-- createServiceClientNoCookies(). They lose PUBLIC and anon only; the +-- explicit authenticated and service_role grants they already carry are +-- untouched, so no application path changes. +-- +-- Done as a loop rather than a hand list because a hand list is exactly how +-- the three guarded numbering RPCs were wrongly declared safe. The predicate +-- is duplicated in tests/pg/definer-function-grants.pg.test.ts, which fails +-- CI when a future migration ships another anon-callable definer writer. +-- --------------------------------------------------------------------------- + +DO $$ +DECLARE + r record; +BEGIN + FOR r IN + SELECT p.oid::regprocedure AS sig + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'public' + AND p.prosecdef + AND p.prokind = 'f' + -- prokind = 'f' only. A SECURITY DEFINER PROCEDURE would satisfy the + -- other predicates, and REVOKE ... ON FUNCTION rejects a procedure, + -- which would abort this migration. public holds no procedures today + -- (verified against prod 2026-09-01), so this is a guard against the + -- first one anyone adds, not a live bug. + AND p.prorettype <> 'trigger'::regtype + AND (p.prosrc ~* '\minsert\s+into\s' + OR p.prosrc ~* '\mupdate\s+[a-z_"]' + OR p.prosrc ~* '\mdelete\s+from\s') + AND has_function_privilege('anon', p.oid, 'EXECUTE') + ORDER BY 1 + LOOP + EXECUTE format('REVOKE EXECUTE ON FUNCTION %s FROM PUBLIC, anon', r.sig); + RAISE NOTICE 'revoked PUBLIC/anon EXECUTE on %', r.sig; + END LOOP; +END; +$$; + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/definer-function-grants.pg.test.ts b/tests/pg/definer-function-grants.pg.test.ts new file mode 100644 index 00000000..46ead7e3 --- /dev/null +++ b/tests/pg/definer-function-grants.pg.test.ts @@ -0,0 +1,363 @@ +import { randomUUID } from 'node:crypto' +import type { PoolClient } from 'pg' +import { describe, expect, it } from 'vitest' +import { getClient, getPool, runAsServiceRole, withUserContext } from '@/tests/pg/setup' +import { seedCompany } from '@/tests/pg/fixtures' + +// Ratchet for 20260901100000_revoke_anon_execute_on_definer_writes.sql. +// +// Supabase's bootstrap grants EXECUTE on every new public function to PUBLIC, +// anon, authenticated and service_role, and PostgREST publishes each of them as +// POST /rest/v1/rpc/. A SECURITY DEFINER function that writes and that +// anon can execute is therefore an unauthenticated cross-tenant write, whether +// or not it carries an in-body guard: the anon key's JWT has no `sub` claim, so +// auth.uid() is NULL and any "NULL is trusted" guard waves it straight through. +// That is exactly how generate_invoice_number, peek_next_invoice_number and +// get_next_arrival_number were wrongly classified as safe. +// +// The first test is generated from a sweep rather than a hand list, so the next +// unguarded function that ships fails CI instead of being declared safe by +// whoever writes the list. +// +// prokind = 'f' keeps procedures out of scope: the migration revokes with +// REVOKE ... ON FUNCTION, which rejects a procedure, so a SECURITY DEFINER +// procedure must not be reported here as an offender the migration can fix. +const DEFINER_WRITERS_SQL = ` + SELECT p.oid::regprocedure::text AS fn, + has_function_privilege('anon', p.oid, 'EXECUTE') AS anon_can + FROM pg_proc p + JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'public' + AND p.prosecdef + AND p.prokind = 'f' + AND p.prorettype <> 'trigger'::regtype + AND (p.prosrc ~* '\\minsert\\s+into\\s' + OR p.prosrc ~* '\\mupdate\\s+[a-z_"]' + OR p.prosrc ~* '\\mdelete\\s+from\\s') + ORDER BY 1 +` + +// SECURITY DEFINER writers deliberately left callable by anon. There are none, +// and there is no good reason for one to exist: an unauthenticated caller has +// no tenant, so a definer write on its behalf is a cross-tenant write. If a +// function ever genuinely belongs here, add it with the call site that needs it +// and the reason the write cannot be attributed to a session. +const ANON_CALLABLE_ALLOWLIST: string[] = [] + +// No user-session caller: revoked from authenticated as well. A forgotten +// PUBLIC in the REVOKE is the standard failure mode (anon is a member of +// PUBLIC), so both roles are asserted explicitly. +const SERVICE_ROLE_ONLY = [ + 'sync_team_to_company(uuid,uuid)', + 'claim_due_webhook_deliveries(integer,timestamp with time zone)', +] + +// Called on the user's session client by the app, so authenticated keeps +// EXECUTE and the in-body membership guard carries the tenant check. For these +// the REVOKE stops the anon key only: the guard is what stops a signed-in +// caller from reaching another tenant, which is why every one of them is +// exercised below with a real non-member session. +const AUTHENTICATED_GUARDED_WRITERS = [ + 'generate_invoice_number(uuid,uuid,text)', + 'peek_next_invoice_number(uuid,text)', + 'get_next_arrival_number(uuid)', + 'generate_delivery_note_number(uuid)', + 'generate_article_number(uuid,uuid)', + 'check_and_increment_inbox_quota(uuid,integer,integer)', +] + +interface GrantRow { + exists: boolean + anon_can: boolean | null + auth_can: boolean | null + service_can: boolean | null +} + +// Resolve through to_regprocedure() so a signature that drifted out of the +// schema fails on `exists` instead of silently returning NULL privileges. +async function grantsFor(signature: string): Promise { + const { rows } = await getPool().query( + `SELECT to_regprocedure($1) IS NOT NULL AS exists, + has_function_privilege('anon', to_regprocedure($1)::oid, 'EXECUTE') AS anon_can, + has_function_privilege('authenticated', to_regprocedure($1)::oid, 'EXECUTE') AS auth_can, + has_function_privilege('service_role', to_regprocedure($1)::oid, 'EXECUTE') AS service_can`, + [`public.${signature}`], + ) + return rows[0]! +} + +// Run `fn` with the presentation an anon-key request gets from PostgREST: a +// role claim of 'anon' and NO `sub` claim, so auth.uid() is NULL. Both GUC +// styles are set for the reason runAsServiceRole() sets both (the CI image +// ships the legacy auth shim, which reads only request.jwt.claim.*). +// +// The connection stays superuser, which is the point: EXECUTE is already +// revoked, so this isolates the in-body guard and proves it fails closed on +// its own rather than leaning on the grant. +async function withAnonClaims(fn: (client: PoolClient) => Promise): Promise { + const client = await getClient() + try { + await client.query('BEGIN') + await client.query(`SELECT set_config('request.jwt.claims', '{"role":"anon"}', true)`) + await client.query(`SELECT set_config('request.jwt.claim.role', 'anon', true)`) + const check = await client.query<{ uid: string | null; role: string | null }>( + `SELECT auth.uid()::text AS uid, auth.role()::text AS role`, + ) + if (check.rows[0]?.uid !== null || check.rows[0]?.role !== 'anon') { + throw new Error( + `withAnonClaims: auth.uid()=${check.rows[0]?.uid ?? 'NULL'}, ` + + `auth.role()=${check.rows[0]?.role ?? 'NULL'}; expected NULL/anon.`, + ) + } + return await fn(client) + } finally { + await client.query('ROLLBACK').catch(() => {}) + client.release() + } +} + +describe('SECURITY DEFINER function grants.pg', () => { + it('leaves no SECURITY DEFINER writer in public executable by anon', async () => { + const { rows } = await getPool().query<{ fn: string; anon_can: boolean }>( + DEFINER_WRITERS_SQL, + ) + + // Sanity: the sweep has to actually match functions, otherwise an empty + // offender list proves nothing about the schema. + expect(rows.length).toBeGreaterThan(10) + + const offenders = rows + .filter((r) => r.anon_can) + .map((r) => r.fn) + .filter((fn) => !ANON_CALLABLE_ALLOWLIST.includes(fn)) + + // If this fails, the named function was created without a REVOKE. Add + // `REVOKE EXECUTE ON FUNCTION FROM PUBLIC, anon;` to the migration + // that created it (PUBLIC included: revoking anon alone is a no-op). + expect(offenders).toEqual([]) + }) + + it.each(SERVICE_ROLE_ONLY)('%s is unreachable for anon and authenticated', async (sig) => { + const grants = await grantsFor(sig) + expect(grants.exists).toBe(true) + expect(grants.anon_can).toBe(false) + expect(grants.auth_can).toBe(false) + expect(grants.service_can).toBe(true) + }) + + it.each(AUTHENTICATED_GUARDED_WRITERS)( + '%s keeps authenticated and service_role but not anon', + async (sig) => { + const grants = await grantsFor(sig) + expect(grants.exists).toBe(true) + expect(grants.anon_can).toBe(false) + expect(grants.auth_can).toBe(true) + expect(grants.service_can).toBe(true) + }, + ) +}) + +describe('numbering RPC guards: fail closed for an anon-shaped caller.pg', () => { + it('refuses every numbering RPC when the JWT has a role but no sub', async () => { + const { userId, companyId } = await seedCompany() + await getPool().query( + `INSERT INTO public.company_settings (user_id, company_id, invoice_prefix, next_invoice_number) + VALUES ($1, $2, 'F', 1) + ON CONFLICT (company_id) DO NOTHING`, + [userId, companyId], + ) + const articleId = randomUUID() + await getPool().query( + `INSERT INTO public.articles (id, company_id, user_id, name) VALUES ($1, $2, $3, 'Konsulttimme')`, + [articleId, companyId, userId], + ) + const customerId = randomUUID() + await getPool().query( + `INSERT INTO public.customers (id, user_id, company_id, name) VALUES ($1, $2, $3, 'Test Customer')`, + [customerId, userId, companyId], + ) + const invoiceId = randomUUID() + await getPool().query( + `INSERT INTO public.invoices + (id, user_id, company_id, customer_id, invoice_number, document_type, + invoice_date, due_date, currency, subtotal, vat_amount, total, + vat_treatment, vat_rate, moms_ruta, status) + VALUES ($1, $2, $3, $4, NULL, 'invoice', + '2026-09-01', '2026-10-01', 'SEK', 1000, 250, 1250, + 'standard_25', 25, '10', 'draft')`, + [invoiceId, userId, companyId, customerId], + ) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.peek_next_invoice_number($1, $2)', [companyId, 'invoice']), + ).rejects.toThrow(/unauthorized/i) + }) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.generate_invoice_number($1, $2, $3)', [ + companyId, + invoiceId, + 'invoice', + ]), + ).rejects.toThrow(/unauthorized/i) + }) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.get_next_arrival_number($1)', [companyId]), + ).rejects.toThrow(/unauthorized/i) + }) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.generate_delivery_note_number($1)', [companyId]), + ).rejects.toThrow(/unauthorized/i) + }) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.generate_article_number($1, $2)', [companyId, articleId]), + ).rejects.toThrow(/unauthorized/i) + }) + + // The invoice must come back unnumbered: the refusal has to land before + // the two UPDATEs, not after one of them. + const { rows } = await getPool().query<{ invoice_number: string | null }>( + `SELECT invoice_number FROM public.invoices WHERE id = $1`, + [invoiceId], + ) + expect(rows[0]!.invoice_number).toBeNull() + }) + + it('leaves the delivery-note and article counters untouched after a refusal', async () => { + const { userId, companyId } = await seedCompany() + await getPool().query( + `INSERT INTO public.company_settings (user_id, company_id) VALUES ($1, $2) + ON CONFLICT (company_id) DO NOTHING`, + [userId, companyId], + ) + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.generate_delivery_note_number($1)', [companyId]), + ).rejects.toThrow(/unauthorized/i) + }) + + const { rows } = await getPool().query<{ + next_delivery_note_number: number + next_article_number: number + }>( + `SELECT next_delivery_note_number, next_article_number + FROM public.company_settings WHERE company_id = $1`, + [companyId], + ) + expect(rows[0]!.next_delivery_note_number).toBe(1) + expect(rows[0]!.next_article_number).toBe(1) + }) + + it('still trusts a direct database connection with no JWT at all', async () => { + // Service-role, cron and pg-real seed paths reach these functions without a + // JWT. They stay trusted: tightening the guard must not break the numbering + // that runs from the API-key and MCP surfaces. + const { userId, companyId } = await seedCompany() + await getPool().query( + `INSERT INTO public.company_settings (user_id, company_id, invoice_prefix, next_invoice_number) + VALUES ($1, $2, 'F', 1) + ON CONFLICT (company_id) DO NOTHING`, + [userId, companyId], + ) + + const peek = await getPool().query<{ peek_next_invoice_number: string }>( + 'SELECT public.peek_next_invoice_number($1, $2)', + [companyId, 'invoice'], + ) + expect(peek.rows[0]!.peek_next_invoice_number).toBe('F001') + + const arrival = await getPool().query<{ n: number }>( + 'SELECT public.get_next_arrival_number($1) AS n', + [companyId], + ) + expect(arrival.rows[0]!.n).toBe(1) + + const deliveryNote = await getPool().query<{ generate_delivery_note_number: string }>( + 'SELECT public.generate_delivery_note_number($1)', + [companyId], + ) + expect(deliveryNote.rows[0]!.generate_delivery_note_number).toMatch(/^FS-\d{4}001$/) + }) +}) + +describe('inbox quota guard: membership required.pg', () => { + const MINUTE_MAX = 30 + const DAY_MAX = 500 + + it('refuses an authenticated caller who is not a member of the target company', async () => { + const victim = await seedCompany() + const outsider = await seedCompany() + + // The shape the REVOKE cannot reach: a real signed-in user of their own + // company, calling with someone else's company id. authenticated keeps + // EXECUTE here because the upload and retry routes run this on the user's + // session client, so the in-body guard is the only thing in the way. + await expect( + withUserContext(outsider.userId, (client) => + client.query('SELECT public.check_and_increment_inbox_quota($1, $2, $3)', [ + victim.companyId, + MINUTE_MAX, + DAY_MAX, + ]), + ), + ).rejects.toThrow(/unauthorized/i) + }) + + it('refuses an anon-shaped caller with a role claim but no sub', async () => { + const { companyId } = await seedCompany() + + await withAnonClaims(async (client) => { + await expect( + client.query('SELECT public.check_and_increment_inbox_quota($1, $2, $3)', [ + companyId, + MINUTE_MAX, + DAY_MAX, + ]), + ).rejects.toThrow(/unauthorized/i) + }) + }) + + it('still counts for a member and for the service path', async () => { + const { userId, companyId } = await seedCompany() + + const member = await withUserContext(userId, async (client) => { + const res = await client.query<{ result: { ok: boolean } }>( + 'SELECT public.check_and_increment_inbox_quota($1, $2, $3) AS result', + [companyId, MINUTE_MAX, DAY_MAX], + ) + return res.rows[0]!.result + }) + expect(member.ok).toBe(true) + + // The inbound-email and WhatsApp paths arrive on + // createServiceClientNoCookies(): no auth.uid() and no membership row to + // find, so the trusted branch has to keep letting them through. + const service = await runAsServiceRole(async (client) => { + const res = await client.query<{ result: { ok: boolean } }>( + 'SELECT public.check_and_increment_inbox_quota($1, $2, $3) AS result', + [companyId, MINUTE_MAX, DAY_MAX], + ) + return res.rows[0]!.result + }) + expect(service.ok).toBe(true) + + // runAsServiceRole commits while withUserContext rolls back, so the one + // surviving counter row is the service call's: proof it went through the + // upsert rather than just returning ok. + const { rows } = await getPool().query<{ count: number }>( + `SELECT count FROM public.inbox_rate_counters + WHERE company_id = $1 AND window_kind = 'minute'`, + [companyId], + ) + expect(rows[0]!.count).toBe(1) + }) +})