Commit Graph
2 Commits
Author SHA1 Message Date
Moheeb Zara bd8f22a3ff 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
Moheeb Zara a2ea7675e7 notifications(P4d): form submission submitter + admin alerts
submitForm now enqueues two kinds of notifications, gated per the design:

  - form_submission_received: ONLY when the submitter is authenticated
    (members or public_auth visibility). Recipient is the verified auth
    email (user.email), never the typed body.email. Anonymous public
    submissions skip the confirmation because confirming to a typed
    address is a victim-spam vector.

  - form_submission_admin: one row per member who holds forms.manage in
    the space (mirrors the forms-guard / forms RLS gate). Dedupe by
    (submission, admin) so a replay is a no-op.

Finding all permission holders without N round-trips needs a new SQL
helper. Migration 047 adds members_with_permission(sid, perm): the
inverted, set-returning form of user_has_permission, with the same
current/late status gate (046) and the same admin shortcut +
space_role_permissions / user_effective_roles fallback. Additive,
introduces no new tables or policies; existing callers unchanged.
Mirrored into schema.sql; types/database.ts updated.

renderFormEmail + formDedupeKey live in notifications-logic; 6 new
render/dedupe unit tests cover both types, fallback labels, escaping.

Suite 565, build clean.
2026-05-19 15:28:21 -07:00