notifications(P4f): audit fixes, lock down 047 + best-effort enqueue paths

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.
2026-05-19 16:12:03 -07:00
parent 6abac53380
commit bd8f22a3ff
6 changed files with 260 additions and 181 deletions
@@ -311,29 +311,36 @@ export async function reserveEquipment(input: unknown) {

  // Booking confirmation goes to the affected member (target), not the actor.
  // A manager booking on behalf of someone else still emails the booked-for
-  // member. Best-effort: the enqueue helper never throws into this action.
-  const contact = await resolveMemberContact(admin, member.space_id, targetMemberId)
-  if (contact?.email) {
-    const { subject, html, text } = renderBookingEmail({
-      type: 'booking_confirmed',
-      spaceName: await getSpaceName(admin, member.space_id),
-      memberName: contact.displayName,
-      equipmentName: (equip.name as string | null) ?? 'equipment',
-      location: (equip.location as string | null) ?? null,
-      startsAt: r.starts_at,
-      endsAt: r.ends_at,
-      manageUrl: buildManageUrl(null),
-    })
-    await enqueueNotification(admin, {
-      spaceId: member.space_id,
-      memberId: targetMemberId,
-      type: 'booking_confirmed',
-      recipient: contact.email,
-      subject,
-      bodyHtml: html,
-      bodyText: text,
-      dedupeKey: bookingDedupeKey('booking_confirmed', data.id as string),
-    })
+  // member. Wrapped: the reservation row is already committed, so a transient
+  // DB error in the email path must NOT surface as an action error to the
+  // client. enqueueNotification is internally best-effort; the wrap covers
+  // resolveMemberContact, getSpaceName, and any other lookup that could throw.
+  try {
+    const contact = await resolveMemberContact(admin, member.space_id, targetMemberId)
+    if (contact?.email) {
+      const { subject, html, text } = renderBookingEmail({
+        type: 'booking_confirmed',
+        spaceName: await getSpaceName(admin, member.space_id),
+        memberName: contact.displayName,
+        equipmentName: (equip.name as string | null) ?? 'equipment',
+        location: (equip.location as string | null) ?? null,
+        startsAt: r.starts_at,
+        endsAt: r.ends_at,
+        manageUrl: buildManageUrl(null),
+      })
+      await enqueueNotification(admin, {
+        spaceId: member.space_id,
+        memberId: targetMemberId,
+        type: 'booking_confirmed',
+        recipient: contact.email,
+        subject,
+        bodyHtml: html,
+        bodyText: text,
+        dedupeKey: bookingDedupeKey('booking_confirmed', data.id as string),
+      })
+    }
+  } catch (e) {
+    console.error('[reserveEquipment] booking_confirmed enqueue failed:', e instanceof Error ? e.message : e)
  }

  revalidatePath('/equipment')
@@ -382,36 +389,42 @@ export async function cancelReservation(input: unknown) {
  // Cancel emails fire ONLY when someone other than the affected member
  // cancelled (a manager cancelling on the member's behalf). A self-cancel is
  // silent: the actor already saw the UI confirm and an email would be noise.
+  // Wrapped: cancel is committed; transient errors in the email path must not
+  // surface to the client.
  if (!ownIt) {
-    const affectedMemberId = res.member_id as string
-    const contact = await resolveMemberContact(admin, member.space_id, affectedMemberId)
-    if (contact?.email) {
-      const { data: equip } = await admin
-        .from('equipment')
-        .select('name, location')
-        .eq('id', res.equipment_id as string)
-        .eq('space_id', member.space_id)
-        .maybeSingle()
-      const { subject, html, text } = renderBookingEmail({
-        type: 'booking_cancelled',
-        spaceName: await getSpaceName(admin, member.space_id),
-        memberName: contact.displayName,
-        equipmentName: (equip?.name as string | null) ?? 'equipment',
-        location: (equip?.location as string | null) ?? null,
-        startsAt: res.starts_at as string,
-        endsAt: res.ends_at as string,
-        manageUrl: buildManageUrl(null),
-      })
-      await enqueueNotification(admin, {
-        spaceId: member.space_id,
-        memberId: affectedMemberId,
-        type: 'booking_cancelled',
-        recipient: contact.email,
-        subject,
-        bodyHtml: html,
-        bodyText: text,
-        dedupeKey: bookingDedupeKey('booking_cancelled', v.data.reservationId),
-      })
+    try {
+      const affectedMemberId = res.member_id as string
+      const contact = await resolveMemberContact(admin, member.space_id, affectedMemberId)
+      if (contact?.email) {
+        const { data: equip } = await admin
+          .from('equipment')
+          .select('name, location')
+          .eq('id', res.equipment_id as string)
+          .eq('space_id', member.space_id)
+          .maybeSingle()
+        const { subject, html, text } = renderBookingEmail({
+          type: 'booking_cancelled',
+          spaceName: await getSpaceName(admin, member.space_id),
+          memberName: contact.displayName,
+          equipmentName: (equip?.name as string | null) ?? 'equipment',
+          location: (equip?.location as string | null) ?? null,
+          startsAt: res.starts_at as string,
+          endsAt: res.ends_at as string,
+          manageUrl: buildManageUrl(null),
+        })
+        await enqueueNotification(admin, {
+          spaceId: member.space_id,
+          memberId: affectedMemberId,
+          type: 'booking_cancelled',
+          recipient: contact.email,
+          subject,
+          bodyHtml: html,
+          bodyText: text,
+          dedupeKey: bookingDedupeKey('booking_cancelled', v.data.reservationId),
+        })
+      }
+    } catch (e) {
+      console.error('[cancelReservation] booking_cancelled enqueue failed:', e instanceof Error ? e.message : e)
    }
  }