audit: remove duplicate Stripe integration card + tighten input validation
Deep audit of the recent change set (passes 56-60): - Remove the generic Integrations-tab Stripe card. saveIntegration overwrites integrations.config wholesale, so configuring Stripe there clobbered the dues config (mode/prices/grace_days) that the Dues tab manages, and couldn't actually set up dues. Stripe is now configured in one place (the Dues tab). PayPal/Zeffy/Venmo sync cards unchanged. - deleteDuesPaymentMethod validates platform against DUES_LINK_PLATFORMS instead of passing a raw client string into the enum column. - markNotificationsRead uses a strict uuid regex for the id filter. PGRST201 class characterized + confirmed contained: PostgREST treats a 2-FK table as an ambiguous junction only when its PK covers both FKs; the only such table now is space_member_custom_roles (a legitimate M2M). Guarded by integration/auth-embed.test.ts.
This commit is contained in:
@@ -71,22 +71,11 @@ const INTEGRATIONS_CONFIG = [
|
|||||||
],
|
],
|
||||||
docs: 'https://developer.venmo.com/',
|
docs: 'https://developer.venmo.com/',
|
||||||
},
|
},
|
||||||
{
|
// Stripe is intentionally NOT listed here. Stripe recurring dues is
|
||||||
platform: 'stripe',
|
// configured in the dedicated Dues tab (saveStripeSettings: mode, tier->price
|
||||||
name: 'Stripe',
|
// map, grace days, vaulted keys). The generic saveIntegration path OVERWRITES
|
||||||
description: 'Accept card payments and subscriptions',
|
// integrations.config with a different shape, which would clobber the dues
|
||||||
icon: (
|
// config (drop prices/mode/grace_days). One Stripe config UI, in Dues.
|
||||||
<svg className="w-6 h-6 text-indigo-600" viewBox="0 0 24 24" fill="currentColor">
|
|
||||||
<path d="M13.976 9.15c-2.172-.806-3.356-1.426-3.356-2.409 0-.831.683-1.305 1.901-1.305 2.227 0 4.515.858 6.09 1.631l.89-5.494C18.252.975 15.697 0 12.165 0 9.667 0 7.589.654 6.104 1.872 4.56 3.147 3.757 4.992 3.757 7.218c0 4.039 2.467 5.76 6.476 7.219 2.585.92 3.445 1.574 3.445 2.583 0 .98-.84 1.545-2.354 1.545-1.875 0-4.965-.921-6.99-2.109l-.9 5.555C5.175 22.99 8.385 24 11.714 24c2.641 0 4.843-.624 6.328-1.813 1.664-1.305 2.525-3.236 2.525-5.732 0-4.128-2.524-5.851-6.591-7.305z"/>
|
|
||||||
</svg>
|
|
||||||
),
|
|
||||||
fields: [
|
|
||||||
{ key: 'publishable_key', label: 'Publishable Key', type: 'text', placeholder: 'pk_test_...' },
|
|
||||||
{ key: 'secret_key', label: 'Secret Key', type: 'password', placeholder: 'sk_test_...' },
|
|
||||||
{ key: 'webhook_secret', label: 'Webhook Secret', type: 'password', placeholder: 'whsec_...' },
|
|
||||||
],
|
|
||||||
docs: 'https://stripe.com/docs/api',
|
|
||||||
},
|
|
||||||
]
|
]
|
||||||
|
|
||||||
export default function SettingsClient({ space, isAdmin, integrations, currentRole }: Props) {
|
export default function SettingsClient({ space, isAdmin, integrations, currentRole }: Props) {
|
||||||
|
|||||||
@@ -4,6 +4,26 @@ Append-only. Newest entries on top. Keep each entry to one screen.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 2026-05-20 (pass 61): Deep audit + fixes (LOCAL, NOT deployed)
|
||||||
|
|
||||||
|
Branch `main`. Deep audit of the app + the recent change set (passes 56-60), an independent subagent review of the new code, and fixes for what was real. Suite 594 unit / 40 integration / build green.
|
||||||
|
|
||||||
|
### Systemic PGRST201 finding (root cause fully characterized, class contained)
|
||||||
|
The outage class is now precisely understood. PostgREST treats a 2-foreign-key table as an ambiguous many-to-many **junction only when its PRIMARY KEY covers both FK columns** (the classic join-table shape). That is why `notification_preferences` (PK `space_id, member_id, category`) broke the `space_members <-> spaces` embed, while the **23 other tables that FK to both** `space_members` and `spaces` (notifications, payments, member_billing, classes, equipment, ...) do **not** — they have surrogate `id` PKs. Verified by querying `pg_constraint`/`pg_index`: the only remaining composite-PK 2-FK junction is `space_member_custom_roles` (space_members <-> space_custom_roles), which is a **legitimate** M2M (no competing direct relationship, so no ambiguity). The auth embed is guarded by `integration/auth-embed.test.ts`. Conclusion: the class is contained; the rule to remember is in DATABASE_SCHEMA 052 and the ARCHITECTURE gotcha note.
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
- **Stripe config footgun (real bug).** The Integrations tab had a generic "Stripe" card writing via `saveIntegration`, which **overwrites** `integrations.config` wholesale -- it would clobber the dues config's `mode`/`prices`/`grace_days` (managed by the Dues-tab `saveStripeSettings`) and could not actually configure dues. Removed the Stripe entry from `INTEGRATIONS_CONFIG`; Stripe is now configured in exactly one place (the Dues tab). Integrations keeps PayPal/Zeffy/Venmo (separate sync surface).
|
||||||
|
- **Boundary validation** (subagent P1/P2): `deleteDuesPaymentMethod` now validates `platform` against `DUES_LINK_PLATFORMS` (was passing a raw client string into the enum column); `markNotificationsRead` uses a strict uuid regex for the optional id filter.
|
||||||
|
|
||||||
|
### Reviewed, no change needed
|
||||||
|
- Independent subagent review of notifications/dues/inbox/me/settings: **no P0**. Self-views correctly scoped to the caller's own `member_id`; no cross-member/cross-space path; inbox renders `body_text` (never `body_html`) so no XSS; dues links double-validated https before render; cron auth constant-time; cron prefs fail open (correct for a billing-critical mailer).
|
||||||
|
- Webhook signing secret is shown pre-populated to admins on `/settings` (subagent P2): left as-is -- webhook secrets are meant to be retrievable, the page is admin-only.
|
||||||
|
|
||||||
|
### Open / next
|
||||||
|
- NOT deployed: the Stripe-card removal + the two validation fixes are prod code, committed locally. Awaiting user go. (No migration.)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## 2026-05-20 (pass 60): /settings "Dues" tab (LOCAL, NOT deployed)
|
## 2026-05-20 (pass 60): /settings "Dues" tab (LOCAL, NOT deployed)
|
||||||
|
|
||||||
Branch `main`. The Stripe-dues config + external-payment-links panels were floating below the settings tab bar (rendered by page.tsx outside SettingsClient, shown regardless of the active tab). Moved both into a new **Dues** tab inside SettingsClient (tabs are now Space / Integrations / Dues / Webhooks); the panels render as standard `bg-card rounded border` tab cards instead of `border-t` page sections. page.tsx now renders only SettingsClient. UI-only, no schema/actions change. Build + 594 unit green. NOT deployed.
|
Branch `main`. The Stripe-dues config + external-payment-links panels were floating below the settings tab bar (rendered by page.tsx outside SettingsClient, shown regardless of the active tab). Moved both into a new **Dues** tab inside SettingsClient (tabs are now Space / Integrations / Dues / Webhooks); the panels render as standard `bg-card rounded border` tab cards instead of `border-t` page sections. page.tsx now renders only SettingsClient. UI-only, no schema/actions change. Build + 594 unit green. NOT deployed.
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import { createClient } from '@/lib/supabase/server'
|
|||||||
import { requireMember, requireMemberWithRole, parseInput } from '@/lib/auth-helpers'
|
import { requireMember, requireMemberWithRole, parseInput } from '@/lib/auth-helpers'
|
||||||
import { ADMIN_ROLES } from '@/lib/permissions'
|
import { ADMIN_ROLES } from '@/lib/permissions'
|
||||||
import { duesPaymentMethodSchema } from '@/lib/validations'
|
import { duesPaymentMethodSchema } from '@/lib/validations'
|
||||||
import { isSafeDuesUrl } from '@/lib/dues-payments-logic'
|
import { isSafeDuesUrl, isDuesLinkPlatform } from '@/lib/dues-payments-logic'
|
||||||
|
|
||||||
export type DuesPaymentMethod = {
|
export type DuesPaymentMethod = {
|
||||||
platform: string
|
platform: string
|
||||||
@@ -100,6 +100,10 @@ export async function deleteDuesPaymentMethod(input: {
|
|||||||
if (!auth.ok) return { error: auth.error }
|
if (!auth.ok) return { error: auth.error }
|
||||||
const { member } = auth
|
const { member } = auth
|
||||||
|
|
||||||
|
// Validate at the boundary (same as the upsert path) instead of passing a
|
||||||
|
// raw client string into the enum column.
|
||||||
|
if (!isDuesLinkPlatform(input.platform)) return { error: 'Invalid platform' }
|
||||||
|
|
||||||
const { error } = await supabase
|
const { error } = await supabase
|
||||||
.from('dues_payment_methods')
|
.from('dues_payment_methods')
|
||||||
.delete()
|
.delete()
|
||||||
|
|||||||
@@ -77,7 +77,9 @@ export async function markNotificationsRead(
|
|||||||
if (input.ids && input.ids.length > 0) {
|
if (input.ids && input.ids.length > 0) {
|
||||||
// Defend the uuid column against a malformed client id (a non-uuid would
|
// Defend the uuid column against a malformed client id (a non-uuid would
|
||||||
// error the cast). Only well-formed ids pass; an empty result is a no-op.
|
// error the cast). Only well-formed ids pass; an empty result is a no-op.
|
||||||
const ids = input.ids.filter(id => /^[0-9a-f-]{36}$/i.test(id))
|
const ids = input.ids.filter(id =>
|
||||||
|
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(id),
|
||||||
|
)
|
||||||
if (ids.length === 0) return { ok: true }
|
if (ids.length === 0) return { ok: true }
|
||||||
q = q.in('id', ids)
|
q = q.in('id', ids)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user