docs: HANDOFF pass-56, record P4g phishing-defense audit fix

Renames the P4f entry to "first-audit fixes", adds a P4g entry for the
third-audit URL-strip defense, and bumps the test-count line to 572
(40 new cases). The commit list at the top of the section now ends at
09f293e.
This commit is contained in:
Moheeb Zara
2026-05-19 17:24:42 -07:00
parent 09f293e3c1
commit a61e34d98d
+5 -3
View File
@@ -14,20 +14,22 @@ Branch `main`. Took item 3 from the pass-55 backlog: booking, class signup, and
- Cancel confirmations: **only when someone other than the affected member cancelled** (self-cancels stay silent; the actor already saw the UI confirm).
- Session-cancellation: **fan out**: when an instructor flips a session to `cancelled`, every still-active signup gets an email (dedupe by `(session, member)`).
### Shipped (commits 84d31ee, 64ffa4e, 105b62f, a2ea767, 6abac53, bd8f22a)
### Shipped (commits 84d31ee, 64ffa4e, 105b62f, a2ea767, 6abac53, bd8f22a, 09f293e)
- **P4a (84d31ee)** Shared best-effort enqueue helper `lib/notifications/enqueue.ts` (`resolveMemberContact`, `enqueueNotification`, `getSpaceName`, `buildManageUrl`). Stripe webhook's `enqueueDues` refactored to compose them; zero behavior change (same recipient resolution, same upsert shape, same idempotency via `(space_id, dedupe_key)`, same best-effort wrap). 12 new helper unit tests.
- **P4b (64ffa4e)** Equipment: `reserveEquipment` → `booking_confirmed` (always, to the booked-for member). `cancelReservation` → `booking_cancelled` only when actor ≠ affected member. `renderBookingEmail` + `bookingDedupeKey` added; existing `renderDuesEmail` quietly refactored onto a shared `renderShell` (one place to fix the HTML wrapper across 6 templates).
- **P4c (105b62f)** Classes: `signUpForClass` → `class_signup_registered` or `class_signup_waitlisted` from the RPC's returned status. `cancelMySignup` → `class_signup_promoted` to whoever `class_cancel_tx` returned as `promoted_id`. `updateSession` setting `status='cancelled'` → `class_session_cancelled` fanned out to every still-active signup. No `class_signup_cancelled` type: `cancelMySignup` is self-only, so under the rule it has no caller.
- **P4d (a2ea767)** Forms: `submitForm` → `form_submission_received` only when authenticated; `form_submission_admin` fanned out to every member with `forms.manage`. Migration 047 adds `members_with_permission(sid, perm)` SECURITY DEFINER (the inverted, set-returning form of `user_has_permission`, same current/late gate as 046) so the fan-out resolves recipients in one query, not N. Additive; existing `user_has_permission` callers unchanged. `types/database.ts` + `schema.sql` + `docs/DATABASE_SCHEMA.md` + `DB_SCHEMA_MAP.md` updated same-change.
- **P4f (bd8f22a) audit fixes**. Ultrathink audit of P4a-P4d before deploy surfaced two issues, both fixed pre-deploy:
- **P4f (bd8f22a) first-audit fixes**. Ultrathink audit of P4a-P4d before deploy surfaced two issues, both fixed pre-deploy:
1. **RPC enumeration vector.** `members_with_permission` was callable by any PostgREST client. Postgres grants EXECUTE on new functions to PUBLIC by default; the 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 in. `user_has_permission` has a narrower exposure (per-uid boolean) and is needed by anon/authenticated for RLS-policy evaluation, so it stays open; the new function is locked to `service_role` (the only legitimate caller goes through the admin client). Migration 047 + schema.sql now `REVOKE EXECUTE FROM PUBLIC` and `GRANT EXECUTE TO service_role`.
2. **Best-effort gap.** P4b/P4c used the helpers but the helpers were not internally best-effort. The Stripe webhook's original `enqueueDues` wrapped its entire body in try/catch so a transient member-lookup error could never bubble into the money path; the P4a refactor removed that inner wrap and relied on `enqueueNotification`'s best-effort. `resolveMemberContact` + `getSpaceName` could still throw on supabase network errors, surfacing into the action's return after the underlying mutation already committed (the client would see an error even though their booking / signup / 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, promoted-signup session lookup, session-cancellation signup fan-out) also cannot leak. forms.submitForm already had the wrap from P4d.
- **P4g (09f293e) third-audit fix, render-layer phishing defense**. renderShell auto-linkifies any `https?://` in body lines so the manageUrl becomes a clickable link. The same regex turns any URL embedded in a user-controlled field into a clickable link. In Phase 2 dues this was self-only (member's own display_name appears in their own email), but Phase 4's `form_submission_admin` is the first email type where one user's display_name lands in another user's inbox: an authenticated member can set display_name to a URL and submit a form, phishing every `forms.manage` holder when their email client renders the linkified URL. `body.email` is Zod-format-validated so the typed-email path is closed, but `display_name` is a free string (max 100). Fix: a new `stripUrls(s)` helper replaces `https?://\S+` with `[link]`, applied to every user-controlled input across all four renderers (member name, equipment/class/form title, location, submitterLabel, space name). `manageUrl` stays untouched and remains the only linkified URL in the rendered email. 5 new tests cover the new attack vector and defense-in-depth across the other renderers.
### Pre-existing exposure flagged for separate triage
`user_has_permission` is also callable by anon + authenticated via PostgREST RPC (same Postgres default; no REVOKE in the codebase). The exposure is narrower than the 047 case (you need to know a target uid, and uids are only learnable for co-members in your own spaces), and locking it down would require careful audit because RLS policies call it from anon/authenticated contexts and would break if EXECUTE were stripped. Out of Phase 4 scope; flagging for the owner to triage.
### Test coverage
35 new `__tests__/` cases (suite 567). Per-render unit tests cover subject lines, body copy, location + range formatting, the "someone" fallback in admin alerts, HTML escaping of injected names/titles. Per-dedupe-key tests pin shape and per-member fan-out distinctness. The shared enqueue helper has hit/miss/error/throw coverage and (post-P4f) the resolveMemberContact / getSpaceName best-effort no-throw paths. Integration suite (31 vs real Postgres) unchanged: the existing Stripe-webhook integration test already exercises the outbox semantics end to end via the same shared upsert; P4 actions enqueue through the same path so the runtime contract is the same.
40 new `__tests__/` cases (suite 572). Per-render unit tests cover subject lines, body copy, location + range formatting, the "someone" fallback in admin alerts, HTML escaping of injected names/titles, and (post-P4g) URL-strip defense across every renderer. Per-dedupe-key tests pin shape and per-member fan-out distinctness. The shared enqueue helper has hit/miss/error/throw coverage and (post-P4f) the resolveMemberContact / getSpaceName best-effort no-throw paths. Integration suite (31 vs real Postgres) unchanged: the existing Stripe-webhook integration test already exercises the outbox semantics end to end via the same shared upsert; P4 actions enqueue through the same path so the runtime contract is the same.
### State
- **Not deployed.** Awaiting one reviewed deploy for the whole set (migration 047 runs as part of `deploy.sh`).