fix(#239,#240,#241,#242): close the security-critical auth/payments bug cluster#276
Merged
Merged
Conversation
…ug cluster Four confirmed Gap-Audit bugs, each verified against the real code + live DB before fixing, then re-verified live. Root-cause fixes only — no symptom patches. #240 Admin dual source of truth (hollow / lingering admins): admin status had two authorities that never synced — the user_profiles.is_admin column (client UI gate) and the JWT app_metadata.is_admin claim (every RLS policy + admin RPC). Make the column the ONE authority: custom_access_token_hook() derives the claim from it at token mint; is_admin() (SECURITY DEFINER) re-reads it live; all 19 admin RLS/RPC guards + the client checkIsAdmin() now call is_admin(), so UI and data can't disagree and revocation is immediate (no waiting for token refresh). #241 Auth audit log empty for real activity (3 defects): (1) the only INSERT policy on auth_audit_logs is service_role, so client-side logAuthEvent() writes were silently RLS-rejected — route them through a SECURITY DEFINER log_auth_event RPC (anti-forgery: caller may only log for itself or anonymously). (2) writer emitted 'sign_in'+success:false while every stat/detector reads 'sign_in_failed' — unify SignInForm on 'sign_in_success'/'sign_in_failed'. (3) delete the dead services/auth/audit-logger module (correct vocab, never wired) + its two tests. #239 PayPal payments double-counted: the redirect capture UPDATEs the pending payment_results row (keyed on the ORDER id) while the webhook blind-INSERTed a new 'succeeded' row (keyed on the CAPTURE id — a different value), so one payment produced two succeeded rows. paypal-webhook now reconciles onto the existing row by intent_id; a partial unique index (one succeeded row per intent) makes a regression impossible. #242 Cancel lets a user start a second subscription before period end: Stripe cancel-at-period-end keeps the sub live at the provider, but our row was set to 'canceled' immediately, dropping it out of the one-live-per-user partial index. Add a 'canceling' status that still counts as live; cancel-subscription and the stripe-webhook mapping write it; the terminal deleted webhook flips it to 'canceled'; resume-subscription handles it. Second sub now correctly rejected. Verification: all four proven live via DB simulation (immediate revocation, audit round-trip + stat, one succeeded row + dup blocked, canceling blocks a 2nd sub). New regression tests: 2 Vitest (admin-auth-service, audit-logger) + 5 RLS DB-contract (tests/rls/security-cluster). Full 65-test RLS suite still green; type-check + lint clean. Schema applied live via the Management API (idempotent, in the monolithic migration). Edge-function redeploy is a merge-time step. Deferred (designed, later sessions): GDPR trio #243/#247/#248 and messaging reliability #244/#245/#246. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e main)
twins.spec.ts:77 ("Top-down compare mode") flaked on CI (failed on main's #274
merge run and the iter6/iter7 runs; passed on other runs) with three 30s-wall
timeouts. The ARIA snapshots from the failed run show the UI rendered correctly —
the ⋯ overflow was [expanded] and the Top-down button was visible — so it was
never a logic regression: the test simply exceeded Playwright's default 30s TEST
cap. The route is the heaviest in the suite (the ?ortho WebGL compare view +
drape + the full baked city) driven through a seven-step overflow round-trip,
and the config already allows a single navigation up to 60s (navigationTimeout) —
more than the whole test was budgeted. Mark it test.slow() (→90s), matching the
rest of the suite. Root-cause fix for the chromium-gen 6/6 flake; no product code
change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TortoiseWolfe
added a commit
that referenced
this pull request
Jul 13, 2026
… auth config (#277) The #240 fix (PR #276) added public.custom_access_token_hook, which derives the JWT app_metadata.is_admin claim from the user_profiles.is_admin column. Enabling it is a project Auth-config setting, not schema — done via `pnpm supabase:auth-config --apply` and now recorded here as desired state so the hook survives any future config reconciliation. Applied + verified live (hook_custom_access_token_enabled=true, uri=pg-functions://postgres/public/custom_access_token_hook). Co-authored-by: TurtleWolfe <TurtleWolfe@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the security-critical / money-relevant subset of the Gap-Audit bug cluster: #240, #241, #239, #242. Every one was verified against the real code + live DB before fixing, then re-verified live — no symptom patches, root causes only.
The four fixes
#240 — Admin dual source of truth (hollow / lingering admins)
Admin status had two authorities that never synced: the
user_profiles.is_admincolumn (client UI gate) vs the JWTapp_metadata.is_adminclaim (every RLS policy + admin RPC). Flipping the column gave a "hollow admin" (UI yes, data empty); revoking the claim left a "lingering admin" until re-login.custom_access_token_hook()derives the claim from it at token mint;is_admin()(SECURITY DEFINER) re-reads it live; all 19 admin RLS/RPC guards and the clientcheckIsAdmin()callis_admin(). UI and data can't disagree, and revocation is immediate (no waiting for token refresh).custom_access_token_hookas the project's Custom Access Token hook (Dashboard → Auth → Hooks / Management API).is_admin()makes the guards correct even before this is done.#241 — Auth audit log empty for real activity (3 defects)
auth_audit_logsisservice_role, so clientlogAuthEvent()writes were silently rejected → route them through a SECURITY DEFINERlog_auth_eventRPC (anti-forgery: caller may only log for itself or anonymously).'sign_in'+success:falsewhile every stat/detector reads'sign_in_failed'→SignInFormnow emits'sign_in_success'/'sign_in_failed'.services/auth/audit-logger.tsmodule was never wired → deleted it + its two duplicate tests;lib/auth/audit-logger.tsis canonical.#239 — PayPal payments double-counted in revenue
The redirect capture UPDATEs the pending
payment_resultsrow (keyed on the ORDER id); the webhook blind-INSERTed a newsucceededrow (keyed on the CAPTURE id — a different value) → two succeeded rows per payment.paypal-webhooknow reconciles onto the existing row byintent_id; a partial unique index (one succeeded row per intent) makes a regression impossible.#242 — Cancel lets a user start a second subscription before period end
Stripe
cancel_at_period_endkeeps the sub live at the provider, but our row was set to'canceled'immediately, dropping it out of the one-live-per-user partial index.'canceling'status still counts as live;cancel-subscription+ thestripe-webhookmapping write it; the terminalcustomer.subscription.deletedwebhook flips it to'canceled';resume-subscriptionhandles it. Second sub now correctly rejected.Verification
All four proven live via DB simulation:
is_admin()true; flip false →is_admin()false on the same call (immediate revocation).log_auth_eventwrites asign_in_failedrow thatadmin_auth_stats.failed_this_weekcounts; anti-forgery guard blocks cross-user logging.canceling; a second sub during that window is rejected; resume →active.Tests: 2 new Vitest (
admin-auth-service,audit-logger) + 5 new RLS DB-contract (tests/rls/security-cluster.test.ts). The full 65-test RLS suite still passes — the guard repointing broke nothing.type-check+lintclean; pre-push production build passed.Schema applied live via the Management API (idempotent, in the monolithic migration). Edge-function redeploy (paypal-webhook, cancel/resume/stripe) is a merge-time step.
🤖 Generated with Claude Code