Third-pass audit finding. renderShell auto-linkifies any https?:// in
body lines so the manageUrl line becomes a clickable link in the HTML
email. The same regex would happily turn a malicious URL injected into
a user-controlled field (member display_name, equipment / class / form
title, submitter label, space name) into a clickable link.
In Phase 2 dues emails this was self-only: the recipient is the same
member whose display_name appears, so self-phishing is silly. Phase 4
introduces the first email type where one user's display_name lands in
a different user's inbox: form_submission_admin embeds the submitter's
display_name (or, when the form was submitted anonymously and an email
match attributed it to a member, that member's display_name) in an
email read by every forms.manage holder. An authenticated member can
set display_name to "Click https://evil.com for free stuff", submit a
form, and the admin's inbox now has a clickable phishing URL.
body.email is Zod-validated against email format so the typed-email
path is closed, but display_name (z.string().min(1).max(100)) is a
free string. No format restriction will catch a determined attacker.
Mitigation: stripUrls(s) replaces https?://\S+ with [link]. Applied to
every user-controlled input across all four renderers (dues, booking,
class, form) including spaceName, memberName / recipientName, equipment
/ class / form title, location, and submitterLabel. manageUrl, which
buildManageUrl constructs from NEXT_PUBLIC_APP_URL or a Stripe-
authenticated webhook host, stays untouched and remains the only
linkified URL in the rendered email.
5 new tests cover the new attack vector (form_submission_admin) and
defense-in-depth across booking, class, and dues renderers. Suite 572,
build clean.
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.
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.
reserveEquipment now enqueues a booking_confirmed email to the booked-for
member (always; a manager booking on someone else's behalf still emails
the target). cancelReservation enqueues a booking_cancelled email ONLY
when the actor is not the affected member, so self-cancels stay silent.
Adds renderBookingEmail + bookingDedupeKey to lib/notifications-logic.ts
(extracts a shared renderShell at the same time so the 6 upcoming
templates do not duplicate the HTML wrapper). 6 new render/dedupe unit
tests cover subject lines, range + location lines, fallback copy when
names are missing, and HTML escaping of injected names.
Suite 550, build clean. No schema change (the notifications.type column
is text), no dispatcher change.
Stripe sends zero-decimal currencies (JPY/KRW/VND/...) already in the
major unit; the unconditional /100 recorded a JPY dues charge 100x
understated in the ledger + receipt. minorToMajor is now currency-aware
(shared ZERO_DECIMAL_CURRENCIES/isZeroDecimalCurrency in
notifications-logic); formatMoney renders no fractional part for those.
Never fires for USD; closes silent ledger corruption for such tenants.
Tests added; suite 526+.
Independent audit verdict was NOT SAFE; all findings fixed:
- dispatcher Idempotency-Key is now per-attempt (${id}:${attempts}),
not row id alone, so a transient-failure response cached by Resend
cannot permanently suppress every retry and silently lose an email.
- enqueueDues is fully try/catch wrapped so a notifications-table or
render failure can never throw into / wedge the Stripe money path.
- dues_lapsed dedupe falls back to the subscription id when periodEnd
is null (canceled/deleted subs), so future lapses no longer collapse
to one key and get suppressed. New test; suite 499.
- documented the dispatcher's once/min concurrency assumption.
Migration 041 + schema.sql section 26: notifications outbox (service-only
writer, admin/board/treasurer SELECT, member self-view via validated
service action like member_billing; (space_id, dedupe_key) unique makes
the webhook enqueue idempotent). lib/notifications-logic.ts is pure:
deterministic dedupe keys, dues email rendering (HTML-escaped, brand-
neutral), terminal-attempt rule. 13 tests. Inert until P2c wires it.
Moheeb Zara