diff --git a/app/onboarding/onboarding-flow.tsx b/app/onboarding/onboarding-flow.tsx index faa6f52..4d2e302 100644 --- a/app/onboarding/onboarding-flow.tsx +++ b/app/onboarding/onboarding-flow.tsx @@ -5,6 +5,7 @@ import { useRouter } from 'next/navigation' import { toast } from 'sonner' import { BrandMark } from '@/components/brand-mark' import { SafeMarkdown } from '@/components/safe-markdown' +import { sanitizeUrl } from '@/lib/security' import { FormRenderer } from '@/components/forms/form-renderer' import type { FormField } from '@/lib/forms-schema' import { @@ -231,16 +232,22 @@ export function OnboardingFlow({ spaceName, steps, canSkip, profile: initialProf ) : step.step_type === 'payment' ? (
{step.body && {step.body}} - {typeof step.config?.payment_url === 'string' && step.config.payment_url && ( - - Set up payment - - )} + {typeof step.config?.payment_url === 'string' && + (() => { + // Admin-entered; sanitize to http(s) so a stored + // javascript:/data: URL can't XSS members in onboarding. + const url = sanitizeUrl(step.config.payment_url as string) + return url ? ( + + Set up payment + + ) : null + })()}
) : step.step_type === 'form' ? (
diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index d92642f..62b9128 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -4,6 +4,25 @@ Append-only. Newest entries on top. Keep each entry to one screen. --- +## 2026-05-20 (pass 62): Diversified surface audit + fixes (LOCAL, NOT deployed) + +Branch `main`. Audited surfaces I had NOT recently touched, via three parallel independent reviews: (1) anonymous/public + all `app/api/**` route handlers, (2) XSS / unsafe rendering across the whole app, (3) authorization + tenant-isolation in legacy server actions (tasks/projects/members/kb/secrets/contacts/comms/classes/equipment/door/presence/certs/invites/roles/permissions/onboarding/forms/governance). **No P0 on any surface.** Suite 594 / build green. + +### Fixed +- **P1 stored XSS (onboarding `payment_url`).** `app/onboarding/onboarding-flow.tsx` rendered the admin-entered `payment_url` into an `href` with no protocol check (onboarding step `config` is `z.record(z.unknown())`). Same class as the dues-url bug: an admin could store `javascript:...` and XSS every new member in onboarding. Fixed at the sink with `sanitizeUrl` (http(s)-only; renders nothing otherwise). +- **P2 forum comment tenant check.** `lib/actions/forum.ts addComment` only space-verified `forum_thread` targets; `proposal`/`incident`/`policy` `entity_id`s were unchecked, so a member could attach a comment to another space's entity (stamped with their own `space_id`; low impact since reads filter by space, but a real gap). Now verifies the target row exists in the caller's space (RLS-scoped read) for every entity type before insert. +- **P2 joinSpace rate limit.** Added `checkRateLimit('joinspace:', 10/hr)` for parity with `createSpace`/`signIn`, bounding invite-code guessing (codes are ~40-bit so not practically guessable, but it was the one unthrottled auth action). + +### Clean (verified by the reviews) +- Markdown rendering is centralized + hardened: `components/markdown.tsx` (no `rehypeRaw`), `components/safe-markdown.tsx` (`rehypeRaw` THEN `rehypeSanitize` with a tightened schema, href protocols allowlisted). Comments/forum/proposals/incidents/policies render inert. Member profile fields render as escaped JSX text. The one `dangerouslySetInnerHTML` (chart.tsx) is developer-config, not user data. +- Public/API: `proxy.ts` whitelist is narrow + exact; `submitForm` resolves space server-side + projects answers through `validateAnswers` + rate-limited; `getPublicForm` never serves members-only; `/auth/callback` open-redirect guard + `/auth/confirm` OTP-type pin intact; `paypal/sync` role-gated + constant fetch URLs (no SSRF). No `fetch()` to a user-controlled URL anywhere. +- AuthZ: every mutating action has an auth gate; every `createAdminClient()` query is scoped to `member.space_id`. (Defense-in-depth noted, not fixed: equipment cancel / door slot writes filter by space-unique id after a space-checked parent load; adding `.eq('space_id')` would be belt-and-suspenders.) + +### Open / next +- NOT deployed: this batch + pass 60 (Dues tab) + pass 61 (audit fixes) are committed locally, no migration. Deploying together. + +--- + ## 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. diff --git a/lib/actions/forum.ts b/lib/actions/forum.ts index f54b77c..214efee 100644 --- a/lib/actions/forum.ts +++ b/lib/actions/forum.ts @@ -118,15 +118,30 @@ export async function addComment(input: { if (!auth.ok) return { error: auth.error } const { member } = auth - // Reject comments on locked forum threads up front. + // Verify the target entity exists IN THE CALLER'S SPACE before commenting. + // The RLS client scopes these reads to the caller's space, so a missing row + // means the entity is not in their space (or does not exist). Without this, + // a member could attach a comment to another space's proposal/incident/policy + // (the row would be stamped with the caller's own space_id). Also gates + // locked forum threads. + let entityInSpace = false + let threadLocked = false if (v.data.entity_type === 'forum_thread') { - const { data: thread } = await supabase - .from('forum_threads') - .select('locked') - .eq('id', v.data.entity_id) - .single() - if (thread?.locked) return { error: 'Thread is locked' } + const { data } = await supabase.from('forum_threads').select('id, locked').eq('id', v.data.entity_id).maybeSingle() + entityInSpace = !!data + threadLocked = !!data?.locked + } else if (v.data.entity_type === 'proposal') { + const { data } = await supabase.from('proposals').select('id').eq('id', v.data.entity_id).maybeSingle() + entityInSpace = !!data + } else if (v.data.entity_type === 'incident') { + const { data } = await supabase.from('incidents').select('id').eq('id', v.data.entity_id).maybeSingle() + entityInSpace = !!data + } else if (v.data.entity_type === 'policy') { + const { data } = await supabase.from('policies').select('id').eq('id', v.data.entity_id).maybeSingle() + entityInSpace = !!data } + if (!entityInSpace) return { error: 'Not found' } + if (threadLocked) return { error: 'Thread is locked' } const { data, error } = await supabase .from('comments') diff --git a/lib/auth-actions.ts b/lib/auth-actions.ts index 1d18cc9..a910bf6 100644 --- a/lib/auth-actions.ts +++ b/lib/auth-actions.ts @@ -111,6 +111,12 @@ export async function joinSpace(formData: { const { data: { user }, error: userErr } = await supabase.auth.getUser() if (userErr || !user) return { error: 'Not authenticated' } + // Throttle to bound invite-code guessing (parity with createSpace/signIn). + const rateLimit = checkRateLimit(`joinspace:${user.id}`, 10, 3600000) // 10 per hour + if (!rateLimit.allowed) { + return { error: 'Too many join attempts. Please try again later.' } + } + // Use admin client for operations const admin = createAdminClient()