Two findings from an ultrathink audit of the Phase 4 work.
(1) members_with_permission was callable by any PostgREST client. Postgres
grants EXECUTE on new functions to PUBLIC by default, and this codebase
has no existing REVOKE pattern. The function returns a SET of member_ids
for any (space, permission) pair, so without a lockdown ANY authenticated
caller could enumerate "who has permission X in space Y" for spaces they
are not even members of, learning member_ids and the permission
distribution. user_has_permission has a narrower exposure (per-uid
boolean) and is needed by anon/authenticated for RLS-policy evaluation,
so it stays callable; the new function is locked to service_role since
the only legitimate caller (the form-submission fan-out) goes through
the admin client. Migration 047 + schema.sql now REVOKE EXECUTE FROM
PUBLIC and GRANT TO service_role.
(2) The Phase 4 enqueue blocks in equipment.ts (reserve, cancel) and
classes.ts (signUp, cancelMySignup waitlist promotion, updateSession
session-cancellation fan-out) used the shared helpers, but the helpers
themselves were not internally best-effort. The Stripe webhook's
original enqueueDues wrapped its entire body in try/catch so a thrown
member-lookup error could never bubble into the money path; my refactor
removed that inner wrap and relied on enqueueNotification's
best-effort. resolveMemberContact and getSpaceName could still throw on
a transient supabase network error, which would surface to the action
return value after the underlying mutation already committed: the
client would see an error even though their booking, signup, or
session-cancellation succeeded. Fixed at both layers: the helpers now
swallow throws internally (returning null / empty), and each call site
in equipment/classes is wrapped in try/catch around the lookup chain so
secondary action-specific queries (equipment name+location lookup,
promoted-signup session lookup, session-cancellation signup fan-out)
also cannot leak. forms.submitForm already had the wrap from P4d.
2 new helper tests pin the best-effort behavior; suite 567, build
clean.
signUpForClass enqueues class_signup_registered or class_signup_waitlisted
based on the RPC return shape; recipient is the affected member, so a
manager signing someone else up still emails the target. cancelMySignup
enqueues class_signup_promoted to whoever was bumped from the waitlist
into the session (the promoted_id returned by class_cancel_tx).
updateSession, when status flips to 'cancelled', fans out a
class_session_cancelled email to every still-active signup (one row per
member, dedupe by session + member).
No type for plain self-cancel: cancelMySignup is self-only and the rule
is "only when someone other than the affected member cancelled," so it
has no caller.
Adds renderClassEmail + classDedupeKey to notifications-logic; 9 new
render/dedupe unit tests cover each type's subject + body and the
session-cancelled per-member dedupe shape.
Suite 559, build clean.
Deferred deeper-audit P1. signUpForClass (count->decide->insert) and
cancelMySignup (cancel->recount->promote) were non-atomic: concurrent
signups at the capacity boundary over-enrolled and concurrent cancels
double-promoted waitlisters (dynamic capacity can't be a DB constraint).
Migration 045 adds class_signup_tx/class_cancel_tx (SECURITY DEFINER,
pg_advisory_xact_lock per session) doing the decision+write atomically;
actions call the RPCs and keep all pre-checks. Removed the now-unused
effectiveCapacity/computeSignupStatus/pickPromotion imports +
isUniqueViolation (pure rules still live + tested in classes-logic).
528 tests; build clean.
Audit follow-up (RLS already enforced these; adding boundary checks so a
future RLS regression cannot silently allow cross-space writes):
- castVote: verify the proposal is in the caller's space and still open
before the upsert.
- cancelMySignup: scope every admin-client query/update in the waitlist
promotion path by member.space_id (not session_id alone).
- setAreaLead: reject a lead_id that is not a member of the space.
- signUpForClass: comment the space-safety invariant on the count.
No behavior change for valid callers.
Pure signupFormEligibility (manager override bypasses) + 4 tests.
signUpForClass now: classes.manage override + sign-up-on-behalf
(targetMemberId), and a hard form gate -- if the class has a
required_form_id the member must have a form_submissions row on file
(checked via service client, since a class manager need not hold
forms.manage). create/updateClass validate the form is in-space and
published. listUpcomingSessions exposes required_form {title,url,
satisfied}. Manage UI: published-form picker on create/edit + a 'requires
form' badge. Member UI: shows the requirement, links to the form, and
replaces Sign up with 'Complete required form' until it is on file.
Mirrors the equipment cert-gate pattern.
classes.ts: class/session CRUD (classes.manage; delete refused once
sessions/signups exist, archive instead), member signUpForClass /
cancelMySignup as validated service-client writes enforcing eligibility,
capacity vs waitlist, de-dupe and earliest-waitlist promotion on cancel,
listUpcomingSessions (service-client counts so members see spots-left
without seeing who signed up), instructor markAttendance and
completeSession. completeSession awards a class's certification via the
normal grantCertification path, so it only issues when the acting
instructor also holds certifications.grant; otherwise completion still
succeeds and reports certificatesSkipped. classes-guard mirrors the
forms/certifications guards. Barrel + API_REFERENCE updated.
Moheeb Zara