fix(security): pin transaction status and columns for user-scoped writes (#528)

[guillermoscript]

What

transactions RLS now restricts which columns a user-scoped caller may write, not just which rows. The INSERT policy pins status = 'pending', and table-level UPDATE is revoked from authenticated and re-granted on only the three columns a legitimate caller actually writes.

Closes #528

Why

transactions grants table-level INSERT/UPDATE to authenticated (20260126190500_lms_complete.sql), and the policies in 20260313025318_rls_transactions.sql constrain only which rows a user may write:

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 an authenticated student could POST /rest/v1/transactions with any column values they liked, as long as user_id was theirs and tenant_id was their current tenant. Both impacts were reproduced end-to-end against a local database, signed in as alice@student.com with nothing but the anon key:

1. Free enrolment. trigger_manage_transactions (AFTER INSERT/UPDATE) calls enroll_user(NEW.user_id, NEW.product_id) whenever product_id is set and status = 'successful'. A single self-inserted row bought a $49 course for one cent:

POST /rest/v1/transactions
{"product_id":2001,"amount":0.01,"status":"successful", ...}       -> HTTP 201

GET  /rest/v1/entitlements?user_id=eq.<alice>&course_id=eq.2001
[{"course_id":2001,"source_type":"product","source_id":2001,"status":"active"}]

2. Fabricated payout liability. getPayoutsOwed() (app/actions/platform/payouts.ts:52) reads transactions with status IN ('successful','refunded') and a platform-settled payment_provider. A self-inserted row landed in grossOwed, so the platform's own payout dashboard reported it owed a school money for a sale that never happened:

POST /rest/v1/transactions
{"amount":9999,"status":"successful","payment_provider":"paypal"}  -> HTTP 201

#512 (PR #525) closed school_percentage_snapshot specifically, by making the database compute and freeze that one column. The rest of the row was still caller-controlled.

The shape of the fix

Both impacts are gated on the same thing — a row reaching 'successful' (or 'refunded' for the payout half) — and neither is reachable from a pending row. So the invariant enforced is narrow and checkable: an untrusted caller may open a transaction, but may not declare it paid. Only the webhook, verify and reconcile paths, which all run on the service-role client and bypass RLS entirely, may do that.

Two layers, defending against different mechanics:

• INSERT policy pins status = 'pending' — closes the self-insert path. Permissive policies OR together, so this replaces the existing policy rather than sitting alongside it.
• UPDATE is narrowed to three columns (status, provider_subscription_id, stripe_payment_intent_id) via column-level grants, and the policy pins USING (status = 'pending') / WITH CHECK (status IN ('pending','failed')) — closes the self-escalate path. after_transaction_update fires the same trigger, so owning a genuine pending row was a second route to the same place.

The column grant is not redundant with the policy. An RLS WITH CHECK sees only the NEW row and cannot compare it to OLD, so a policy alone cannot make a column immutable. Without the grant, a student could open a legitimate pending transaction for a cheap product, PATCH its product_id to an expensive one, pay the cheap price, and let the provider's webhook flip the row to successful — at which point the trigger enrols them in the expensive course. Column privileges close that at the grant layer, without needing OLD.

Ordering matters: a bare REVOKE ... (column) is a no-op while the table-level grant is held, so table-level UPDATE is revoked first and the allowed columns re-granted after.

Why enrollUser() moved to the admin client

A full inventory of application write sites found 7 user-scoped writes. Five already write 'pending' or 'failed' and are unaffected. The two inserts in enrollUser() (app/[locale]/(public)/checkout/actions.ts, the mock/sandbox checkout) write 'successful' and would have broken, so they move to createAdminClient(). Both already validate tenant ownership before writing — the product/plan lookups are .eq('tenant_id', tenantId) reads on the user-scoped client, and user_id comes from the session — which satisfies the createAdminClient() rule in CLAUDE.md.

Worth stating explicitly, because it bounds how much this matters: enrollUser() is reachable only from the "Pay & Enroll (Test)" sandbox button, which components/public/checkout-form.tsx:678 gates behind process.env.NODE_ENV !== 'production'. In production a Stripe product renders StripePaymentForm (the webhook grants enrollment) and a provider-less product renders a "card unavailable" notice, so neither reaches this action. The move to the admin client therefore keeps local development and the E2E sandbox flow working; it is not covering a production payment path.

Everything on a service-role client (the Stripe webhook, lib/payments/webhook-dispatch.ts, the Solana and Binance reconcilers, both cron routes, payment-requests.ts, admin/binance-personal.ts) bypasses RLS and is untouched. grant_free_subscription() is SECURITY DEFINER and the table is not FORCE ROW LEVEL SECURITY, so it runs as the table owner and still inserts its zero-amount 'successful' row — verified live, not assumed.

How to QA

On a fresh npm run db:reset, signed in as alice@student.com / password123 on code-academy.lvh.me:3000 (Code Academy Pro).

Automated state check — every row should report PASS:

docker exec -i supabase_db_lms-front psql -U postgres -d postgres \
  -f - < supabase/snippets/verify_transactions_column_hardening.sql

This asserts the final grant and policy state rather than the migration text, so it also catches a later migration re-widening things.

Manual — the attacks must fail. Get a student JWT and hit PostgREST directly (this is the part that matters; RLS is invisible to the type checker):

URL=http://127.0.0.1:54321
ANON=<NEXT_PUBLIC_SUPABASE_PUBLISHABLE_OR_ANON_KEY from .env.local>
JWT=$(curl -s -X POST "$URL/auth/v1/token?grant_type=password" \
  -H "apikey: $ANON" -H "Content-Type: application/json" \
  -d '{"email":"alice@student.com","password":"password123"}' | jq -r .access_token)
ALICE=a1000000-0000-0000-0000-000000000004
TENANT=00000000-0000-0000-0000-000000000002

1. Self-insert a successful sale → expect 403, new row violates row-level security policy:

   curl -s -X POST "$URL/rest/v1/transactions" -H "apikey: $ANON" \
     -H "Authorization: Bearer $JWT" -H "Content-Type: application/json" \
     -d "{\"user_id\":\"$ALICE\",\"tenant_id\":\"$TENANT\",\"product_id\":2001,\"amount\":0.01,\"currency\":\"usd\",\"status\":\"successful\"}"

2. Confirm no entitlement was created → expect []:

   curl -s "$URL/rest/v1/entitlements?user_id=eq.$ALICE&course_id=eq.2001&select=course_id,status" \
     -H "apikey: $ANON" -H "Authorization: Bearer $JWT"

3. Self-insert a fake platform-settled sale ("amount":9999,"status":"successful","payment_provider":"paypal") → expect 403.
4. Open a legitimate pending row (same body as step 1 but "product_id":2002,"amount":19,"status":"pending","payment_provider":"stripe") → expect 201. Keep the returned transaction_id.
5. Rewrite financial columns on that pending row (PATCH ...?transaction_id=eq.<id> with {"amount":0.01,"payment_provider":"paypal"}) → expect 403, permission denied for table transactions.
6. Escalate that pending row ({"status":"successful"}) → expect 403, RLS violation.
7. Roll it back to failed ({"status":"failed"}) → expect 200. This is app/api/payments/checkout/route.ts:337 and must keep working.
8. Write the two provider-metadata columns on a fresh pending row ({"provider_subscription_id":"sub_test_123"}, then {"stripe_payment_intent_id":"pi_test_123"}) → both expect 200. These are payments/checkout/route.ts:320 and create-payment-intent/route.ts:241.

Manual — the app must still work. Free-course and free-plan checkout (enrollFree / subscribeFree, both on SECURITY DEFINER RPCs) still enrol, and the inline mock checkout on a paid product still grants access.

Screenshots / GIF

No UI change — this is grant/policy DDL plus a Supabase client swap in a server action. The verification evidence is the before/after PostgREST transcript above: the same 8 requests returned 201/200 for all four attacks before the migration and 403 for all four after, with both legitimate paths unchanged.

Checklist

• npm run typecheck and npm run test:unit pass — typecheck clean; 370 tests across 31 files pass
• npm run build passes
• Every new tenant-scoped query filters by tenant_id (or the table genuinely has no such column) — no new queries; the two enrollUser() inserts already carry tenant_id and keep it on the admin client
• Tested with every relevant role (student / teacher / admin) — exercised as a student (the only role that can reach the vector) and as the service-role/SECURITY DEFINER paths; teacher and admin have SELECT-only policies on this table and are unaffected
• Loading and error states handled — no UI surface; rejected writes surface through the existing txError branches
• New UI strings added to both messages/en.json and messages/es.json — n/a, no new strings
• Migration applies cleanly on npm run db:reset, has RLS policies, and lib/database.types.ts was regenerated — applies clean from scratch; the migration is RLS policies; no regeneration needed, as it changes no columns or types (grants and policies only)

Follow-up (deliberately out of scope)

One adjacent problem surfaced while surveying. It is real, it is not what #528 describes, and folding it in would have made this unreviewable.

Settlement columns are still caller-controlled at INSERT. settlement_base, settlement_currency, settlement_mint and settlement_sol_usd remain writable, and app/api/payments/solana/verify/route.ts prices the on-chain payment against them. A self-inserted pending row carrying settlement_base: 1 is a pay-one-lamport-for-a-$100-course path, and it survives this PR because the row is legitimately pending — the invariant enforced here is "an untrusted caller may not declare a row paid", which does not constrain what a pending row claims it is owed. Closing it properly needs the checkout RPC (direction 3 in the issue), since the SOL/USD rate is fetched off-platform and cannot be recomputed in a trigger. Filed separately.

[guillermoscript]

Merged and shipped as described in the body — no scope changed during review.

What landed:

• supabase/migrations/20260725170000_transactions_column_hardening.sql — the INSERT policy on transactions now pins status = 'pending', and table-level UPDATE is revoked from authenticated in favour of a three-column grant (status, provider_subscription_id, stripe_payment_intent_id), with the UPDATE policy restricted to USING (status = 'pending') / WITH CHECK (status IN ('pending','failed')).
• app/[locale]/(public)/checkout/actions.ts — the two enrollUser() inserts moved to createAdminClient(), the only legitimate user-scoped caller that needed to write 'successful'.
• A rollback migration and supabase/snippets/verify_transactions_column_hardening.sql, which asserts the final grant and policy state rather than the migration text, so a later migration re-widening things is caught.

Verified before merge, against a local database with a real student JWT on the anon key: all four attacks returned 201/200 before the migration (including an entitlements row appearing for a $49 course) and 403 after, with both legitimate user-scoped paths — the 'failed' rollback and the two provider-metadata column writes — still returning 200. grant_free_subscription() was exercised end to end and still inserts its zero-amount 'successful' row, since SECURITY DEFINER runs as the table owner and the table is not FORCE ROW LEVEL SECURITY.

Checks: npm run typecheck clean, 370/370 unit tests, npm run build passes, the migration applies from scratch on npm run db:reset, and the verification snippet reports PASS on all six checks. CI verify passed.

Two notes for whoever picks this up next:

• The migration has only been applied locally. It still needs supabase db push against the cloud project.
• Deferred, and filed as Solana settlement columns are caller-controlled at INSERT — a pending row can under-quote what it owes (#528 follow-up) #538: the Solana settlement columns (settlement_base, settlement_currency, settlement_mint, settlement_sol_usd) are still caller-controlled at INSERT, and lib/payments/solana-reconcile.ts:103-106 verifies the on-chain payment against the row's own settlement_base. That path survives this PR, because the invariant enforced here is "an untrusted caller may not declare a row paid" — it does not constrain what a legitimately pending row claims it is owed.

One correction was made to this PR's body before review: it originally claimed enrollUser() grants free access in production for Stripe and provider-less products. It does not — the button reaching it is gated behind process.env.NODE_ENV !== 'production' (components/public/checkout-form.tsx:678). The call site was checked but not the render gate. The body and the issue were corrected before this was marked ready.