fix(db): make the split snapshot database-owned, not caller-supplied (#512)

[guillermoscript]

Closes #512. Builds on @DPS0340's #525 — their commit is preserved as the first commit here, and the second is the hardening that came out of reviewing it against a live database. If this lands, #525 should be closed as superseded (the credit for the core insight is theirs).

What #525 got right, and why it needed a second pass

The central finding in #525 is correct and worth restating: payment_provider — the field getPayoutsOwed() filters on — is written by UPDATE, after the row exists (lib/payments/webhook-dispatch.ts:205, app/api/stripe/webhook/route.ts:406). A BEFORE INSERT trigger, which is what the issue asked for, never sees that path. Covering UPDATE is right.

Reviewing it against a local database turned up two problems with the rule the trigger applied.

1. The column is client-writable, and "an explicit value always wins" preserves the one write we need to override

transactions grants ALL to authenticated (20260126190500_lms_complete.sql:3692), and the RLS policies restrict which rows a user may write, never which columns (20260313025318_rls_transactions.sql):

CREATE POLICY "Users can create own transactions"
  ON public.transactions FOR INSERT TO authenticated
  WITH CHECK (auth.uid() = user_id AND tenant_id = get_tenant_id());

So school_percentage_snapshot can be supplied by a student, and it flows straight into computeOwedBalances()'s grossOwed. Measured against #525 as submitted, on a local DB where the tenant's real split is 70:

INSERT tamper -> snapshot = 100  (real split is 70)
UPDATE tamper -> snapshot = 100  (was 70)

Both survive, because the trigger's first rule is to return early when the value is non-NULL. Same two statements after this PR:

INSERT tamper -> snapshot = 70.00  (real split is 70)
UPDATE tamper -> snapshot = 70.00  (was 70)

The column is now database-owned: computed from revenue_splits on INSERT (no WHEN guard — the point is to override what the caller sent), and frozen on UPDATE (the previous value is restored verbatim rather than raising, so a tampering UPDATE is neutralised instead of failing the webhook that carried it).

2. The UPDATE branch was a lazy, non-deterministic backfill

Once the trigger is installed, every new row is snapshotted at INSERT — so the UPDATE branch can only ever fire on legacy NULL rows. What #525 did to them was stamp today's split, permanently, at whatever arbitrary moment something incidental first touched the row. A May sale refunded next week would be frozen at next week's split.

That is the retroactive repricing #496 removed; it does not stop being that because it happens one row at a time. #525's own description argues against backfilling for exactly this reason, so this is a case of the implementation not matching its stated intent rather than a disagreement about the goal.

A legacy NULL is now filled in exactly one case: payment_provider is changing, i.e. a provider is activating the row right now — the one moment at which today's split is the honest answer for it. This needs two triggers rather than one, because OLD cannot be referenced in a WHEN clause on a combined INSERT OR UPDATE trigger.

Correction: revenue_splits is not super-admin-only

The migration, the checkout route's comment, and #525's description all repeat that revenue_splits is super-admin-only under RLS. It isn't. Verified on a local DB:

policyname                          | qual
------------------------------------+---------------------------------------------------------
 Admins can view own revenue split  | tenant_id = get_tenant_id() OR is_super_admin()

Despite the policy's name, every member of the tenant — students included — can read their school's split. Two consequences:

• SECURITY DEFINER is kept, but for the opposite reason to the one given. An INVOKER function would work today; it would also make this trigger's correctness depend on a permissive policy whose own name contradicts it. The day someone tightens that policy to match its name, an INVOKER trigger stops seeing the row and silently stamps the 80 fallback onto every checkout, for every tenant — wrong payout numbers, no error, no failing test.
• The comment in app/api/payments/checkout/route.ts claiming the admin client is required there is corrected. It's belt-and-braces, not a requirement.

Unfiled follow-up, not touched here: that policy's name and its predicate disagree. The predicate looks deliberate (dashboard/teacher/revenue reads the table on the user-scoped client), so the name is what's wrong.

Also in this PR

• The verify snippet is relabelled LOCAL ONLY. fix(db): back the revenue-split snapshot with a trigger, not one call site (#512) #525 described it as "safe against a populated database" because it rolls back. The rollback protects the data, not availability: it needs ALTER TABLE transactions DISABLE TRIGGER, which locks the live payments table against INSERT/UPDATE for as long as the script holds it, requires table ownership (it won't run as service_role), and leaves the trigger disabled for other sessions if the script dies with the transaction open. It also grew from 5 cases to 7 — both tamper paths and the no-lazy-backfill rule.
• The unit tests are relabelled as static drift guards. They are regexes over the migration's text; they pass identically whether the trigger works, is broken, or was never applied. The DEFAULT_SCHOOL_PERCENTAGE drift guard earns its place (that constant is genuinely duplicated into the plpgsql); the tautological not.toMatch(/UPDATE\s+transactions\s+SET/i) is dropped, and a guard that the INSERT trigger stays unconditional is added. A green run of that file is not evidence the migration works — the snippet is.
• The rollback script drops both triggers (and the old single-trigger name), and now says plainly that rolling back re-opens the client-writable column, not just the Revenue split isn't time-sliced — plan changes retroactively reprice historical payouts #496 fallback.

Blast radius

transactions INSERT now always does one index lookup on revenue_splits (UNIQUE (tenant_id)), on a table written only on payment events. UPDATE calls the function only when the snapshot or payment_provider is actually changing — ordinary status-only updates don't fire it at all. The pre-existing after_transaction_insert / after_transaction_update triggers are AFTER, so ordering is unaffected. No app behaviour changes: the checkout route's own write is kept (it computes the identical number and keeps the rollback a safe lever), and every other insert path simply stops being wrong.

Testing

Run against a local DB, not just statically:

npm run db:reset
docker exec -i supabase_db_lms-front psql -U postgres -d postgres \
  < supabase/snippets/verify_transaction_split_snapshot_backstop.sql

Expected, and what I got:

PASS 1: insert without snapshot filled from revenue_splits (70)
PASS 2: client-supplied snapshot (100) overridden with the real split (70)
PASS 3: missing revenue_splits row falls back to 80
PASS 4: provider activation filled the snapshot (70) — the BEFORE INSERT gap
PASS 5: existing snapshot untouched by a later update (still 70, split now 90)
PASS 6: update tampering with an existing snapshot reverted to 70
PASS 7: legacy NULL row still NULL after an incidental update and a tamper attempt

Also verified on the local DB:

• The rollback script drops both triggers and the function cleanly; the migration then re-applies and all 7 cases pass again.
• SECURITY DEFINER works from the real checkout shape — SET LOCAL ROLE authenticated with a tenant JWT claim, inserting with no snapshot supplied, yields the tenant's real split (65 in the fixture), not the 80 fallback.

Suite level: npm run test:unit 321 passed (28 files) · npm run typecheck clean · npx eslint clean on changed files.

Not yet deployed to cloud — supabase db push is still to run.

Reviewer notes

• The verification snippet stays in supabase/snippets/ despite that folder otherwise holding Studio dumps. supabase/migrations/ would auto-apply it and supabase/tests/ is reserved for pgTAP (supabase test db), so this is the least-bad existing home. Happy to move it if there's a preferred spot.
• Adjacent and not fixed here, because it's much wider than this issue: the same RLS gap lets a user insert status: 'successful', payment_provider: 'paypal' on their own row, which both self-enrols them via trigger_manage_transactions and lands a fabricated sale in the payout math. Filed separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_013NffLeUiW78cokcUXrt68n

[guillermoscript]

Superseded — the same commit is now on #525 itself (pushed via "Allow edits by maintainers"), so the work lands there with @DPS0340's commit as its base. Closing this to keep one PR for #512.