TwoMore

Appearance

B3 — Payments (Backend) Adversarial Audit ​

Status: Archived Date: 2026-07-11 Scope: settle-before-play canon (migrations 00298–00303, 00307 refunds, 00313 audit-batch-3 fixes, 00384 settle-before-play, 00385 signal emitters). session_payments one-hold-row dual-rail model, reserve-then-pay trigger, attest_session_payment, mark-paid/waived authority, expire_unpaid_session_holds cron, enforce_settled_match_participants gate, PortOne rail scaffold, refund acknowledgment, cost_total legacy split.

Method: full read of all 9 migrations in scope + 00381/session-payment adapter/mapper/edge-functions + cross-file grep for every INSERT/UPDATE path touching session_payments/matches.participant_ids. Two findings were empirically verified against a live local Postgres (supabase_db_twomore-v2, migrations through 00385 applied) via constructed SQL probes inside rolled-back transactions — not just static reading. Cross-referenced docs/audits/blocks/U3-sessions.md (client-side payment UI) and docs/audits/blocks/U8-dues.md (sibling money system, dues — confirmed structurally separate table/RPC family, no shared code path).

Summary table ​

#FindingSeverityVerified
1Settle-before-play match gate (enforce_settled_match_participants) is bypassed by the exact team-slot-only UPDATE the app's updateMatchTeams/SwapSheet issues — an unsettled participant can be swapped into a fee-session match with zero server-side rejectionCRITICALEmpirically reproduced on live DB
200385_alert_model_wiring.sql silently reverts the 00313 TOCTOU fix in expire_unpaid_session_holds — the attest-vs-expiry race it closed is reopened in the migration that hasn't shipped to prod yetCRITICAL (pre-ship, easy fix)Empirically confirmed on live DB (\sf)
3session_payments UPDATE RLS policies (host 00303, admin 00145) have no WITH CHECK — Postgres reuses USING, which only gates session_id, so an authorized host/admin can tamper with amount, user_id, method, etc. via a raw update, not just statusMEDIUM (defense-in-depth gap; app itself never sends these fields)Confirmed by migration read
4No CHECK constraint on session_payments.amount >= 0 (only NOT NULL)LOWConfirmed by migration read
5Fee-change-after-holds-exist has no reconciliation path; session_payments.amount is frozen at hold-creation and never re-synced to a later sessions.participation_fee editMEDIUM (latent — no UI currently edits fee post-creation, but the adapter/mapper/mutation path exists)Confirmed by code read
6Documented gap 4a (00384 inline hold-cancel-at-completion emits no session_payment_hold_expired signal)MEDIUM — confirmed realConfirmed by code read
7Documented gap 4b (payer-cancelled hold doesn't resolve its hold_created signal)LOW–MEDIUM — confirmed real, but self-heals within payment_hold_minutes (≤10 min) via expires_atConfirmed by code read
8session_payments_writable_by_admins legacy createBatch INSERT has no server-side guard against colliding with an auto-generated fee hold (UNIQUE(session_id,user_id)) — fails loudly, not silently, and the UI already gates isFeeSession, so practical risk is lowLOWConfirmed by code read
9PortOne rail: edge functions well-built (signature verify, replay guard, re-fetch-truth, idempotent RPC) but genuinely unvalidated against a live PortOne channel; native adapter not startedINFO (matches docs, no drift)Confirmed by code read
10SECDEF hygiene: all payment RPCs/triggers have search_path=''; mark_session_payment_paid_via_portone correctly locked to service_role only; enforce_settled_match_participants has REVOKE ALL FROM PUBLIC but no explicit GRANT (harmless — matches the codebase's own precedent for trigger-only functions, e.g. sync_match_participant_ids)Non-issue (verified, noted for completeness)Confirmed by code read

1. CRITICAL — Settle-before-play match gate is bypassed by team-slot swaps ​

The claim (00384's own comment): "MATCH GATE (server backstop; the match board also gates client-side): on a fee session, a match may not include a participant whose session_payment is not settled (paid/waived)."

The mechanism:

  • private.enforce_settled_match_participants() (migration 00384_settle_before_play.sql:38-84) is wired as:
    sql
    CREATE TRIGGER trg_matches_settled_participants
  BEFORE INSERT OR UPDATE OF participant_ids ON public.matches
  FOR EACH ROW EXECUTE FUNCTION private.enforce_settled_match_participants();
    This is a column-list trigger — Postgres only fires it when participant_ids is literally in the SET clause of the executing UPDATE statement, regardless of whether the value is later changed by another trigger in the same statement.
  • private.sync_match_participant_ids() (migration 00313_audit_batch3_participant_ids_and_payment_safety.sql:32-62) is wired as BEFORE INSERT OR UPDATE OF team1_player1_id, team1_player2_id, team2_player1_id, team2_player2_id — it derives NEW.participant_ids from the four team-slot columns as a side effect.
  • The app's swap path — updateMatchTeams(), packages/app/src/adapters/supabase/match.supabase.ts:191-215 (backs SwapSheet / LiveRoundEditorSheet, confirmed by U4 audit) — issues:
    ts
    supabase
  .from('matches')
  .update({
    team1_player1_id,
    team1_player2_id,
    team2_player1_id,
    team2_player2_id,
  })
  .eq('id', matchId);
    participant_ids is never in that SET list. So on a swap: sync_match_participant_ids fires (team columns are targeted) and quietly recomputes participant_ids to include the swapped-in player — enforce_settled_match_participants does not fire, because the triggering statement never targeted participant_ids directly. The settle-before-play backstop is architecturally a no-op on the one write path that most directly needs it: adding a player to an existing match post-generation.
  • The client-side gate the 00384 comment leans on (match-board-screen.tsx:138-167, eligiblePlayerIds/unsettledPlayerIds via partitionSettledMatchPool) is real and does exclude unsettled players from round generation and the swap candidate pool in the sanctioned UI flow (U3 audit confirms this is correctly wired). But it is UI-only — there is no server enforcement backing it on this specific write shape, so any bypass of the client gate (a bug in a future refactor, a raw API call, a different client) writes an unsettled participant into a fee match with zero rejection.

Empirical reproduction (rolled-back transaction on the local Postgres, migrations through 00385 applied):

  1. Created a club, a fee session (participation_fee = 10000), two confirmed RSVPs (p1, p2) — both auto-get pending holds via 00299/00313's sync_session_payment_on_rsvp.
  2. Marked p1's hold paid (settled). Left p2's hold pending (unsettled — confirmed via SELECT status immediately before the probe: pending).
  3. Created a match with host+p1 only (both exempt/settled).
  4. Ran the exact statement shape updateMatchTeams issues: UPDATE matches SET team2_player2_id = p2 WHERE id = match_id;
  5. Result: UPDATE 1 — no exception. participant_ids was silently recomputed to {p1, p2, host}, i.e. p2 (unsettled) is now a match participant of a fee session with no settled payment. The gate never fired.

Fix shape: add participant_ids (or the four team-slot columns) to the enforce_settled_match_participants trigger's UPDATE OF list — the simplest correct fix is BEFORE INSERT OR UPDATE OF team1_player1_id, team1_player2_id, team2_player1_id, team2_player2_id, participant_ids ON public.matches, mirroring sync_match_participant_ids's own column list, and — because trigger firing order matters here — name it to sort after sync_match_participant_ids alphabetically (it already does: sync_... < trg_matches_settled_participants) so it evaluates the final, recomputed NEW.participant_ids. This is the same class of bug flagged independently in U4 ("team-slot swap has no server-side status gate — RLS/trigger both miss it") — both are instances of a single systemic pattern: any "server backstop" keyed to UPDATE OF participant_ids is dead code on the swap path, because nothing ever puts participant_ids in that statement's SET list. Worth fixing both gates in the same pass.

2. CRITICAL (pre-ship) — 00385 silently reverts the 00313 TOCTOU fix in the expiry cron ​

Background: 00313_audit_batch3_participant_ids_and_payment_safety.sql:125-210 explicitly closed an attest-vs-expiry race: the idle-hold expiry cron's FOR v_rec IN SELECT ... cursor can go stale mid-sweep, so a member who taps 송금 완료 after the cursor snapshot but before the cron reaches their row could be evicted despite having just paid. The fix: PERFORM 1 FROM public.rsvps ... FOR UPDATE (lock, skip if no longer confirmed) followed by a second PERFORM 1 FROM public.session_payments ... AND submitted_at IS NULL FOR UPDATE (re-check idle, skip if the member attested in the interim) — documented in-file as fixing exactly this TOCTOU.

The regression: 00385_alert_model_wiring.sql:236-315 re-declares private.expire_unpaid_session_holds() (CREATE OR REPLACE) to append a signals_emit call for the new session_payment_hold_expired type. Its own comment says: "Whole function reproduced verbatim from 00301 (its latest definition) with ONE addition." That premise is false — 00313, not 00301, was the function's actual latest definition at the time 00385 was authored, and 00313's locking/re-check block is absent from 00385's body. Since migrations apply CREATE OR REPLACE in file order, 00385 (numerically after 00313) becomes the live function, overwriting the race-safe version with a body that predates the race fix by two migrations.

Empirical confirmation: ran \sf private.expire_unpaid_session_holds on the live local DB (migrations through 00385 applied) — the returned body is exactly 00385's, with no FOR UPDATE and no idle re-check. The attest-vs-expiry race 00313 closed is live again.

Severity context: per the audit-plan ledger, 00385 is committed (5dec3455) but not yet pushed to prod ("BLOCKED (owner action): npx supabase db push ... denied by the permission classifier"). This means the regression has not shipped — this is the ideal moment to catch it, before the push the owner is about to unblock. Recommend patching 00385's expire_unpaid_session_holds body in place (re-add the two FOR UPDATE blocks from 00313, keep the new signals_emit call) rather than shipping then fixing forward.

3. MEDIUM — session_payments UPDATE RLS has no WITH CHECK; column tampering possible by an authorized host/admin ​

  • session_payments_updatable_by_admins (00145_rls_polish_round_2.sql:89-98) and session_payments_updatable_by_host (00303_session_payment_host_authority.sql:45-54) both declare FOR UPDATE ... USING (...) with no WITH CHECK clause. Per Postgres RLS semantics, when WITH CHECK is omitted on an UPDATE policy, the USING expression is reused as the check on the new row. Both USING expressions only assert session_payments.session_id belongs to a session the caller administers (or hosts) — they say nothing about which columns may change.
  • Net effect: a legitimate session host or club admin (not a random member — RLS still gates row visibility correctly) can, via a raw Supabase client call outside the app's own markPaid/markWaived (which only ever send status/paid_at/method/recorded_by, session-payment.supabase.ts:80-112), rewrite amount, user_id, portone_payment_id, refund_state, etc. on any row they administer, with no DB-level guard.
  • This is a defense-in-depth gap, not an exploitable-by-anyone hole — the app never sends these fields, and only an authorized (if malicious/compromised) host/admin account could exploit it. Still worth closing per the "money integrity" mandate: a WITH CHECK that pins amount/user_id to OLD.amount/OLD.user_id (only status/paid_at/method/ refund_state/recorded_by/timestamps may move) would close it without touching the app.

4. LOW — no CHECK (amount >= 0) on session_payments ​

00059_session_payments.sql:5 declares amount INTEGER NOT NULL with no lower-bound CHECK. In practice amount is always populated server-side from sessions.participation_fee (which does have CHECK (participation_fee >= 0), 00298), so this is unreachable through the reserve-then-pay path — but combined with finding #3 (no WITH CHECK on UPDATE), a negative amount is not structurally impossible, only practically unlikely.

5. MEDIUM — fee change after holds exist has no reconciliation ​

  • sessions.participation_fee has no server-side edit lock. The generic session update() adapter (session.supabase.ts:258-267) and session.mapper.ts:202-216's toUpdateRow both pass participation_fee through when present in Partial<CreateSessionInput>, and use-update-session.ts exposes a useUpdateSession mutation. No AFTER UPDATE OF participation_fee trigger exists to reconcile already-created session_payments.amount rows (verified: no such trigger anywhere in supabase/migrations/*.sql).
  • Practical risk today is low: edit-session-screen.tsx (grepped for fee/참가비, case-insensitive) does not expose a fee field, so no shipped UI can trigger this. But the write path exists end-to-end (adapter → mapper → mutation hook), so a future edit-session feature that adds a fee field would silently create drift: existing holds keep their frozen amount, new holds get the new fee, and — sharper edge — mark_session_payment_paid_via_portone (00302) validates p_paid_amount = sessions.participation_fee (the current, possibly post-edit fee), not the hold's own frozen amount column, so a successfully-verified PortOne payment can land on a session_payments row whose amount column now permanently disagrees with what was actually collected. This is an audit-trail correctness gap, not a money-safety gap (no double-charge, no under-charge slips through the RPC's own check) — recommend either locking participation_fee once any session_payments row exists, or reconciling amount on fee edit.

6. MEDIUM — confirmed: 00384's inline hold-cancel-at-completion emits no signal ​

Per the audit-plan's flagged gap: private.session_completed_record_attendance (00384_settle_before_play.sql:104-138, step "(a)") cancels any still-pending hold at session completion (excused, strike 0) but never calls signals_emit. Contrast with the cron path (00385's expire_unpaid_session_holds) which does emit session_payment_hold_expired for the idle-timeout case. A member whose hold survived to session completion (e.g. they attested/submitted late, past the idle window but the host never confirmed) gets silently RSVP-cancelled with zero notification of why — same downstream state (excused, hold cancelled) as the cron path, but silent. Confirmed real; not previously mis-stated in the plan.

7. LOW–MEDIUM — confirmed: payer-cancel doesn't resolve hold_created signal ​

sync_session_payment_on_rsvp (00313's version, still current) deletes an idle (submitted_at IS NULL) hold on RSVP-cancel but never calls signals_resolve_by_dedup_key('session_payment_hold_created:' || id). Grepped 00385_alert_model_wiring.sql for signals_resolve_by_dedup_key — the only resolve calls in the file are for dues_* types (lines 864-867); none for session_payment_hold_created. Bounded severity: the emitted signal carries p_expires_at := LEAST(created_at + payment_hold_minutes, session start) (00385:77-81), so the stale notification self-clears within at most payment_hold_minutes (default 10 min) of the cancel — a brief dangling alert, not a permanent stuck one (contrast U8's CRITICAL dues-waive finding, which never resolves).

8. LOW — legacy cost_total split (createBatch) can collide with an auto-generated fee hold ​

createBatch (session-payment.supabase.ts:54-78) does a plain multi-row .insert() with no ON CONFLICT handling, into a table with UNIQUE(session_id, user_id) (00059:12). If invoked for a user who already has a fee-gated hold (participation_fee > 0 session), the whole batch insert fails (Postgres batch INSERT is atomic — one conflicting row aborts all). session-payments-screen.tsx:237,382-390 gates the legacy "정산 내역 만들기" CTA behind !isFeeSession, so the sanctioned UI never reaches this collision. No server-side CHECK backs that UI gate, though — the RPC/table layer alone does not prevent it, it just fails loudly (a 500, not silent corruption) if someone gets there another way.

9. INFO — PortOne rail readiness (no drift found vs docs) ​

Read supabase/functions/verify-portone-payment/index.ts and supabase/functions/portone-webhook/index.ts end to end:

  • verify-portone-payment: authenticates caller via JWT, independently re-fetches the payment from PortOne (GET /payments/{id}), checks status==='PAID', cross-checks customData.{sessionId,userId} against the caller (custom.userId !== callerId → 409 payment_mismatch — a member cannot confirm someone else's hold), then calls the service-role RPC. Never trusts client-asserted amount/status.
  • portone-webhook: Standard Webhooks HMAC-SHA256 signature verification (whsec_...) + 5-minute replay-tolerance window, re-fetches payment truth from PortOne rather than trusting the webhook body, deployed --no-verify-jwt (correct — PortOne doesn't send a Supabase JWT).
  • Both funnel into mark_session_payment_paid_via_portone (00302), which is idempotent (WHERE status = 'pending', returns FOUND boolean) — a webhook racing the client-side verify call lands the same single paid row, confirmed by reading the RPC body directly.
  • docs/guides/portone-payments.md's own status table ("Scaffolded — deploy
    • validate against a PortOne TEST channel"; native adapter "Pending the EAS build") matches what's in the repo — no documentation drift. Could not confirm from this environment whether the edge functions are currently deployed (supabase functions list returned empty in this session); not a code-quality finding either way.

10. Non-issue — SECDEF hygiene sweep ​

Checked every payment-related function for SET search_path = '' + REVOKE ... FROM PUBLIC + explicit GRANT:

Functionsearch_path=''REVOKE PUBLICGRANT
private.sync_session_payment_on_rsvp✓✓service_role
private.expire_unpaid_session_holds✓✓postgres (preserved via CREATE OR REPLACE across 00300/01/13/85)
public.attest_session_payment✓✓authenticated
public.dev_mock_pay_session✓✓authenticated (internally gated to one hardcoded dev-account UUID)
public.mark_session_payment_paid_via_portone✓✓service_role only — confirmed correctly locked down, no authenticated grant anywhere
private.session_cancel_open_refunds✓✓postgres
public.cancel_session_as_host✓✓authenticated
public.acknowledge_session_refund✓✓authenticated
private.tick_session_refund_obligations✓✓postgres
private.block_delete_session_with_pending_refunds✓✓postgres
private.enforce_settled_match_participants✓✓none — but this matches the codebase's own established precedent for pure-trigger functions (e.g. private.sync_match_participant_ids, 00313, also has no explicit GRANT); Postgres trigger firing doesn't require the firing role to hold EXECUTE on the trigger function, so this is stylistically inconsistent but not a security gap

No missing search_path='', no missing REVOKE FROM PUBLIC, no accidentally-authenticated-callable service-role-intended function found. This block is clean on SECDEF hygiene.

Other adversarial checks that came back clean ​

  • Double-pay via re-invoking markPaid: markPaid/markWaived do not filter on status='pending' before updating (.update(...).eq('id', paymentId) only) — calling either twice just re-stamps status/paid_at, no double-charge (nothing increments; it's a flag flip, not a ledger entry). Confirmed harmless.
  • Payer self-marking paid: no RLS UPDATE policy grants the payer (user_id = auth.uid()) write access to their own session_payments row — only host/admin UPDATE policies exist, plus the two SECURITY DEFINER RPCs (attest_session_payment only sets submitted_at, never status; dev_mock_pay_session hardcoded to one dev UUID). A payer cannot self-mark paid through any path.
  • Random member marking someone paid: RLS UPDATE policies require EXISTS against sessions.created_by = auth.uid() (host) or club owner/admin membership (admin) — a random confirmed participant has no UPDATE policy match. Confirmed closed.
  • Hold for a 0-fee session: sync_session_payment_on_rsvp's first line is IF COALESCE(v_fee, 0) <= 0 THEN RETURN NEW; — no hold row is ever created for a free session. Confirmed.
  • Waitlist promotion double-hold: UNIQUE(session_id, user_id) + ON CONFLICT DO NOTHING on the reserve trigger makes a second hold for the same user structurally impossible; a promoted waitlister has no prior hold (they were never confirmed before), so they cleanly get exactly one new hold. Confirmed.
  • Refund flow completeness: 00307 — cancel-with-paid-holds → refund_state='pending' + session_refund_notice signal → member acknowledge_session_refund → refund_state='acknowledged', resolves the host's host_reliability_events row + host_refund_unsettled signal when the last pending refund clears; hard-DELETE blocked while any refund is pending (block_delete_session_with_pending_refunds trigger); 72h grace-window cron (tick_session_refund_obligations) dings the host and escalates a persistent signal. This is a complete, well-closed lifecycle — no gaps found.
  • Paid-no-show: confirmed structurally in session_completed_record_attendance step (b): a confirmed RSVP with a settled payment who never appears in any match's participant_ids gets final_status='no_show', strike_value=1.0; nothing in that code path touches session_payments.status, so the fee correctly stays paid while the no-show strike fires independently, matching the documented "the fee stays paid, the 정산 roster flags the row" behavior.
  • Idle-expiry vs attest TOCTOU (the pre-00385 state): 00313's fix (lock rsvps row, re-check session_payments idle before evicting) is correct in isolation — the bug is purely that 00385 clobbered it (see finding #2).

Cross-reference: U3 / U8 ​

  • U3 (docs/audits/blocks/U3-sessions.md) independently confirmed the client-side match-board correctly derives its eligible/excluded player pools from the server settle-before-play model (UnsettledPoolNotice/partitionSettledMatchPool, U3 finding context lines 179-181) and that the 정산 screen's confirmation-authority gate matches 00303 exactly (!canManageDues && !isSessionHost, U3 lines 176-178) — both independently corroborate this audit's reads. U3 also flagged (its own HIGH #1) that the payment-hold UI panel can go stale up to 5 minutes after the backing hold expires because sessionPaymentKeys isn't in the live 10s-poll invalidation set — a client staleness bug, not a server-integrity one; complements but doesn't overlap this audit's findings.
  • U8 (docs/audits/blocks/U8-dues.md) covers the separate dues table/RPC family (monthly club dues), confirmed structurally independent from session_payments (different table, different RPCs, different screens) — no shared code path, so U8's CRITICAL findings (waive with no audit trail / no signal resolution, partial-payment invisibility) do not apply here. Notably, session-payment waive (via markWaived) does stamp recorded_by and does fire the session_payment_confirmed signal (00385 §1c) even for status='waived' — this subsystem does not repeat U8's dues-waive gap.
  1. Fix #1 (match-gate column-list bypass) and #2 (TOCTOU regression) — both are small, surgical SQL migrations; #2 in particular should land before the blocked 00385 prod push goes out, since it's currently uncommitted-to-prod and trivially patchable in place.
  2. Add WITH CHECK to the two session_payments UPDATE RLS policies (#3) — one migration, no app changes required.
  3. #4/#5/#8 are latent/low-practical-risk — bundle into the same hardening migration as #3 (amount CHECK, pin amount/user_id in WITH CHECK covers most of it) rather than a dedicated pass.
  4. #6/#7 (missing signals) — small additions to existing trigger bodies, can ride along with unrelated future signal-model work.

Markdown remains the source of truth. Run yarn docs:check before handoff.