From 47d85e5235f4f547f9239ff8ea36acc7f389cf7d Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Wed, 27 May 2026 12:37:09 -0400 Subject: [PATCH] fix(marketing/forms): resolve CRM config server-side, not from client (#46239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Bug fix (security hardening). ## What is the current behavior? [PRODSEC-120](https://linear.app/supabase/issue/PRODSEC-120/mythos-ant-2026-btrnt5a3-server-action-accepts-client-controlled-crm) — the marketing form server action accepts the full \`crm\` config (Notion \`database_id\`, HubSpot \`formGuid\`, Customer.io \`event\`, \`staticProperties\`, etc.) from the client, so a crafted submission can write to any Notion database the integration token reaches, post to any HubSpot form in the portal, or trigger arbitrary Customer.io events. ## What is the new behavior? The client now posts only \`{ slug, formId }\` plus the field values; \`submitFormAction\` validates the ref with Zod, looks the trusted CRM config up from the in-process \`_go/**\` page registry via a resolver wired up in \`instrumentation.ts\`, and fails closed if the form isn't found. \`SectionRenderer\` also strips \`crm\` from the section before it crosses into the client bundle (so \`database_id\` / \`formGuid\` no longer ship in page HTML), \`getAllGoPages\` rejects any form section with \`crm\` but no stable \`id\`, and per-submission size/character limits were tightened. ## Additional context Separate follow-ups (not in this PR): confirm \`NOTION_FORMS_API_KEY\` is write-only and scoped to the forms subtree, and chase down the \`NOTION_EVENTS_API_KEY\` validity issue raised on the Linear ticket. ## Summary by CodeRabbit * **New Features** - Forms now support unique identifiers for enhanced tracking and management - Server-side form configuration management for improved reliability * **Improvements** - Enhanced form validation during page initialization to catch configuration issues - Improved form submission handling with better error detection and reporting - Strengthened form operations with fail-safe configuration resolution [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/supabase/supabase/pull/46239?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) --- apps/www/_go/lead-gen/example-lead-gen.tsx | 1 + apps/www/instrumentation.ts | 2 + apps/www/lib/go.ts | 9 ++ apps/www/lib/registerFormCrm.ts | 23 +++++ .../marketing/src/forms/MarketingForm.tsx | 39 +++++--- packages/marketing/src/forms/index.ts | 6 +- .../src/go/actions/formCrmResolver.ts | 32 ++++++ .../marketing/src/go/actions/submitForm.ts | 99 +++++++++++++++++-- packages/marketing/src/go/index.ts | 1 + packages/marketing/src/go/schemas.ts | 56 +++++++++++ .../marketing/src/go/sections/FormSection.tsx | 20 +++- .../src/go/sections/SectionRenderer.tsx | 12 ++- .../src/go/templates/LeadGenTemplate.tsx | 7 +- .../src/go/templates/LegalTemplate.tsx | 7 +- .../src/go/templates/ThankYouTemplate.tsx | 7 +- 15 files changed, 288 insertions(+), 33 deletions(-) create mode 100644 apps/www/lib/registerFormCrm.ts create mode 100644 packages/marketing/src/go/actions/formCrmResolver.ts diff --git a/apps/www/_go/lead-gen/example-lead-gen.tsx b/apps/www/_go/lead-gen/example-lead-gen.tsx index 81c1187744b..8c25c972e7f 100644 --- a/apps/www/_go/lead-gen/example-lead-gen.tsx +++ b/apps/www/_go/lead-gen/example-lead-gen.tsx @@ -224,6 +224,7 @@ alter table posts enable row level security;`, }, { type: 'form', + id: 'form', title: 'Get in touch', description: 'Fill out the form below and our team will get back to you shortly.', fields: [ diff --git a/apps/www/instrumentation.ts b/apps/www/instrumentation.ts index 3063091e693..ce4795aa998 100644 --- a/apps/www/instrumentation.ts +++ b/apps/www/instrumentation.ts @@ -3,6 +3,8 @@ import * as Sentry from '@sentry/nextjs' export async function register() { if (process.env.NEXT_RUNTIME === 'nodejs') { await import('./sentry.server.config') + const { registerFormCrmResolver } = await import('./lib/registerFormCrm') + registerFormCrmResolver() } if (process.env.NEXT_RUNTIME === 'edge') { diff --git a/apps/www/lib/go.ts b/apps/www/lib/go.ts index 3876928737c..2e0ac425ce4 100644 --- a/apps/www/lib/go.ts +++ b/apps/www/lib/go.ts @@ -1,3 +1,5 @@ +import { validateGoPageInvariants } from 'marketing' + import rawPages from '@/_go' import { goPageSchema, type GoPage } from '@/types/go' @@ -14,6 +16,13 @@ export function getAllGoPages(): GoPage[] { ) } + const invariantErrors = validateGoPageInvariants(result.data) + if (invariantErrors.length > 0) { + throw new Error( + `Invalid go page definition (slug: "${result.data.slug}"):\n${invariantErrors.map((m) => ` - ${m}`).join('\n')}` + ) + } + if (seenSlugs.has(result.data.slug)) { throw new Error(`Duplicate slug "${result.data.slug}" in _go registry`) } diff --git a/apps/www/lib/registerFormCrm.ts b/apps/www/lib/registerFormCrm.ts new file mode 100644 index 00000000000..d18d8a097b4 --- /dev/null +++ b/apps/www/lib/registerFormCrm.ts @@ -0,0 +1,23 @@ +import 'server-only' + +import { setFormCrmResolver } from 'marketing' + +import { getGoPageBySlug } from './go' + +/** + * Wire the marketing form server action to look up the trusted CRM config for + * a `{ slug, formId }` pair from the in-process page registry. Without this, + * `submitFormAction` fails closed and rejects every submission. See + * PRODSEC-120 for why the CRM config must never come from the client. + */ +export function registerFormCrmResolver() { + setFormCrmResolver(({ slug, formId }) => { + const page = getGoPageBySlug(slug) + if (!page || !('sections' in page) || !page.sections) return undefined + + const section = page.sections.find((s) => s.type === 'form' && s.id === formId) + if (!section || section.type !== 'form') return undefined + + return section.crm + }) +} diff --git a/packages/marketing/src/forms/MarketingForm.tsx b/packages/marketing/src/forms/MarketingForm.tsx index 10022734a36..82e77495a51 100644 --- a/packages/marketing/src/forms/MarketingForm.tsx +++ b/packages/marketing/src/forms/MarketingForm.tsx @@ -16,11 +16,20 @@ import { import type { z } from 'zod' import { submitFormAction } from '../go/actions/submitForm' -import { formCrmConfigSchema, formFieldSchema, type GoFormFieldShowWhen } from '../go/schemas' +import { formFieldSchema, type GoFormFieldShowWhen } from '../go/schemas' /** Input-shape field type — fields with Zod defaults (`half`, `required`) are optional here. */ export type MarketingFormField = z.input -export type MarketingFormCrmConfig = z.input + +/** + * Opaque reference the client posts back to the server action. The server + * resolves this to the trusted CRM config from the page registry; the client + * never sees or controls the actual CRM target (database id, form GUID, etc.). + */ +export interface MarketingFormRef { + slug: string + formId: string +} /** * Evaluate a `showWhen` rule against the current form values. All supplied @@ -52,8 +61,13 @@ export interface MarketingFormProps { successMessage?: string /** URL to redirect the user to after a successful submission. Overrides `successMessage`. */ successRedirect?: string - /** CRM fan-out config — submits to HubSpot, Customer.io, and/or Notion in parallel. */ - crm?: MarketingFormCrmConfig + /** + * Server-side form reference. When set, submissions are posted to + * `submitFormAction` with this ref; the server looks up the trusted CRM + * config from the page registry. When omitted, the form logs values in dev + * and does nothing in production (useful for previews). + */ + formRef?: MarketingFormRef /** Wraps the form in a styled card (border + padding). Defaults to `true`. */ card?: boolean /** Extra class names applied to the outer wrapper. */ @@ -63,10 +77,9 @@ export interface MarketingFormProps { type SubmitState = 'idle' | 'loading' | 'success' | 'error' /** Build the sessionStorage key used to block double-submits of the same email to the same form. */ -function dedupeKey(crm: MarketingFormCrmConfig | undefined, email: string): string | null { - const formId = crm?.hubspot?.formGuid ?? crm?.notion?.database_id - if (!formId || !email) return null - return `marketing-form-submitted:${formId}:${email.trim().toLowerCase()}` +function dedupeKey(formRef: MarketingFormRef | undefined, email: string): string | null { + if (!formRef || !email) return null + return `marketing-form-submitted:${formRef.slug}:${formRef.formId}:${email.trim().toLowerCase()}` } function FieldInput({ @@ -166,7 +179,7 @@ export default function MarketingForm({ disclaimer, successMessage, successRedirect, - crm, + formRef, card = true, className, }: MarketingFormProps) { @@ -210,9 +223,9 @@ export default function MarketingForm({ Object.entries(values).filter(([name]) => visibleFieldNames.has(name)) ) - if (!crm) { + if (!formRef) { if (process.env.NODE_ENV === 'development') { - console.log('[marketing/form] No CRM configured — form values:', submittedValues) + console.log('[marketing/form] No formRef configured — form values:', submittedValues) } return } @@ -226,7 +239,7 @@ export default function MarketingForm({ submittedValues['emailAddress'] ?? submittedValues['email_address'] ?? '' - const sessionKey = dedupeKey(crm, emailValue) + const sessionKey = dedupeKey(formRef, emailValue) if (sessionKey && typeof window !== 'undefined') { try { if (window.sessionStorage.getItem(sessionKey)) { @@ -250,7 +263,7 @@ export default function MarketingForm({ const honeypot = honeypotRef.current?.value ?? '' try { - const result = await submitFormAction(crm, submittedValues, { + const result = await submitFormAction(formRef, submittedValues, { pageUri, pageName, honeypot, diff --git a/packages/marketing/src/forms/index.ts b/packages/marketing/src/forms/index.ts index 974e7091b4b..a7de322a308 100644 --- a/packages/marketing/src/forms/index.ts +++ b/packages/marketing/src/forms/index.ts @@ -1,9 +1,5 @@ export { default as MarketingForm } from './MarketingForm' -export type { - MarketingFormCrmConfig, - MarketingFormField, - MarketingFormProps, -} from './MarketingForm' +export type { MarketingFormField, MarketingFormProps, MarketingFormRef } from './MarketingForm' export { default as HubSpotFormEmbed } from './HubSpotFormEmbed' export type { HubSpotFormEmbedProps } from './HubSpotFormEmbed' diff --git a/packages/marketing/src/go/actions/formCrmResolver.ts b/packages/marketing/src/go/actions/formCrmResolver.ts new file mode 100644 index 00000000000..994040a2b46 --- /dev/null +++ b/packages/marketing/src/go/actions/formCrmResolver.ts @@ -0,0 +1,32 @@ +import 'server-only' + +import type { GoFormCrmConfig } from '../schemas' + +export interface FormRef { + slug: string + formId: string +} + +export type FormCrmResolver = ( + ref: FormRef +) => GoFormCrmConfig | undefined | Promise + +let resolver: FormCrmResolver | null = null + +/** + * Register the function used by `submitFormAction` to look up the trusted CRM + * config for a form. The consuming app must call this at server startup so the + * server action can resolve a `{ slug, formId }` posted from the client back to + * the same config that lives in the page registry. + * + * The CRM config must never be sourced from the client — see + * `submitFormAction` for the security rationale. + */ +export function setFormCrmResolver(fn: FormCrmResolver): void { + resolver = fn +} + +export async function resolveFormCrmConfig(ref: FormRef): Promise { + if (!resolver) return undefined + return await resolver(ref) +} diff --git a/packages/marketing/src/go/actions/submitForm.ts b/packages/marketing/src/go/actions/submitForm.ts index c1770bb73ec..dddbc498b48 100644 --- a/packages/marketing/src/go/actions/submitForm.ts +++ b/packages/marketing/src/go/actions/submitForm.ts @@ -1,7 +1,10 @@ 'use server' +import { z } from 'zod' + import { CRMClient, type CRMConfig } from '../../crm' import type { GoFormCrmConfig } from '../schemas' +import { resolveFormCrmConfig } from './formCrmResolver' export interface FormSubmitResult { success: boolean @@ -14,6 +17,11 @@ const HONEYPOT_FIELD = 'website' /** Minimum milliseconds a form must be on the page before a submission is accepted. */ const MIN_FORM_RENDER_MS = 3000 +/** Per-submission limits. Keeps any single payload from blowing through CRM rate limits. */ +const MAX_FIELD_NAME_LENGTH = 200 +const MAX_FIELD_VALUE_LENGTH = 10_000 +const MAX_FIELDS_PER_SUBMISSION = 100 + // Enable debug logging in local dev and on Vercel preview/development deployments const isDebug = process.env.NODE_ENV === 'development' || @@ -29,6 +37,39 @@ function debug(message: string, data?: unknown) { } } +/** + * Restricted character set for the form reference. Slugs follow the page + * registry shape (`a/b-c`), formIds match `formIdSchema` in schemas.ts. We + * keep these strict because they flow straight into a registry lookup. + */ +const formRefSchema = z.object({ + slug: z + .string() + .min(1) + .max(200) + .regex(/^[a-z0-9][a-z0-9/_-]*$/i, 'Invalid slug'), + formId: z + .string() + .min(1) + .max(120) + .regex(/^[a-z0-9][a-z0-9_-]*$/i, 'Invalid formId'), +}) + +const valuesSchema = z + .record(z.string().min(1).max(MAX_FIELD_NAME_LENGTH), z.string().max(MAX_FIELD_VALUE_LENGTH)) + .refine((v) => Object.keys(v).length <= MAX_FIELDS_PER_SUBMISSION, { + message: 'Too many fields', + }) + +const contextSchema = z + .object({ + pageUri: z.string().max(2048).optional(), + pageName: z.string().max(500).optional(), + honeypot: z.string().max(MAX_FIELD_VALUE_LENGTH).optional(), + formMountedAt: z.number().int().nonnegative().optional(), + }) + .optional() + function buildCrmConfig(crm: GoFormCrmConfig): CRMConfig { const config: CRMConfig = {} @@ -56,21 +97,49 @@ function buildCrmConfig(crm: GoFormCrmConfig): CRMConfig { } /** - * Submit form values to the configured CRM providers (HubSpot, Customer.io, Notion). + * Submit a form to its configured CRM providers (HubSpot, Customer.io, Notion). + * + * SECURITY: the client only sends `formRef = { slug, formId }` and the field + * values. The trusted CRM config (which database, which form GUID, which + * event, which static properties) is resolved on the server from the page + * registry via the resolver wired up in `setFormCrmResolver`. Never accept + * the CRM config from the wire — a client that controls it can write to any + * Notion database the integration token can reach, submit to any HubSpot form + * on the portal, or trigger arbitrary Customer.io events. See PRODSEC-120. * * Credentials are read from environment variables: * - HubSpot: HUBSPOT_PORTAL_ID * - Customer.io: CUSTOMERIO_SITE_ID, CUSTOMERIO_API_KEY * - Notion: NOTION_FORMS_API_KEY - * - * Per-form config (formGuid, event name, database_id, field mappings) lives in the page definition. */ export async function submitFormAction( - crm: GoFormCrmConfig, - values: Record, - context?: { pageUri?: string; pageName?: string; honeypot?: string; formMountedAt?: number } + rawFormRef: unknown, + rawValues: unknown, + rawContext?: unknown ): Promise { - debug('Form submission received', { crm, values, context }) + const parsedRef = formRefSchema.safeParse(rawFormRef) + if (!parsedRef.success) { + debug('Submission rejected: invalid formRef', parsedRef.error.issues) + return { success: false, errors: ['Invalid form reference.'] } + } + + const parsedValues = valuesSchema.safeParse(rawValues) + if (!parsedValues.success) { + debug('Submission rejected: invalid values', parsedValues.error.issues) + return { success: false, errors: ['Invalid form values.'] } + } + + const parsedContext = contextSchema.safeParse(rawContext) + if (!parsedContext.success) { + debug('Submission rejected: invalid context', parsedContext.error.issues) + return { success: false, errors: ['Invalid submission context.'] } + } + + const formRef = parsedRef.data + const values = parsedValues.data + const context = parsedContext.data + + debug('Form submission received', { formRef, values, context }) // Anti-spam: honeypot tripped or form submitted suspiciously fast. Return a // fake success so bots think they got through and don't retry with variations. @@ -93,6 +162,22 @@ export async function submitFormAction( return { success: true, errors: [] } } + // Trusted CRM config — sourced from the server-side page registry, NOT the + // client. If the resolver isn't registered or the form isn't found, fail + // closed. + let crm: GoFormCrmConfig | undefined + try { + crm = await resolveFormCrmConfig(formRef) + } catch (err: any) { + console.error('[go/form] CRM resolver threw', err) + return { success: false, errors: ['Form configuration unavailable.'] } + } + + if (!crm) { + console.warn('[go/form] Rejected submission: form not found in registry', formRef) + return { success: false, errors: ['Form not found.'] } + } + try { // The honeypot field is never part of the real payload, even if a real // form happens to include a `website` field name. diff --git a/packages/marketing/src/go/index.ts b/packages/marketing/src/go/index.ts index 24186b54d83..beeadf6cde8 100644 --- a/packages/marketing/src/go/index.ts +++ b/packages/marketing/src/go/index.ts @@ -2,3 +2,4 @@ export * from './schemas' export * from './sections' export * from './templates' export * from './actions/submitForm' +export * from './actions/formCrmResolver' diff --git a/packages/marketing/src/go/schemas.ts b/packages/marketing/src/go/schemas.ts index d9bec8effed..77bfc038407 100644 --- a/packages/marketing/src/go/schemas.ts +++ b/packages/marketing/src/go/schemas.ts @@ -256,6 +256,19 @@ export const formCrmConfigSchema = z message: 'At least one CRM provider (hubspot, customerio, or notion) must be configured', }) +/** + * Form `id` doubles as the lookup key the server uses to resolve the trusted + * CRM config at submit time. The client only posts `{ slug, formId }`; the + * server pulls `crm` from the in-process page registry. Keep this restricted + * to a plain identifier — it is round-tripped through the action payload. + * + * Enforcement happens post-parse in `validateGoPage` rather than via a Zod + * `.superRefine` so `formSectionSchema` stays a plain `ZodObject` and remains + * usable as a member of the section `discriminatedUnion`. + */ +export const FORM_ID_PATTERN = /^[a-z0-9][a-z0-9_-]*$/i +export const FORM_ID_MAX_LENGTH = 120 + export const formSectionSchema = z.object({ ...sectionBase, type: z.literal('form'), @@ -430,6 +443,49 @@ export const goPageSchema = z.discriminatedUnion('template', [ legalPageSchema, ]) +/** + * Post-parse checks that can't be expressed in the Zod schema without breaking + * the section `discriminatedUnion`. Returns an array of human-readable errors + * (empty if the page is valid). Intended to be called by the page-registry + * loader alongside `goPageSchema.safeParse`. + * + * Currently checks: + * - Every form section that configures `crm` has a stable `id` (used as the + * server-side lookup key — see `submitFormAction`). + * - Form ids match `FORM_ID_PATTERN` so they're safe to round-trip through + * the action payload. + * - Form ids are unique within a page so resolution is deterministic. + */ +export function validateGoPageInvariants(page: z.infer): string[] { + const errors: string[] = [] + const sections = 'sections' in page ? (page.sections ?? []) : [] + const formIds = new Set() + + sections.forEach((section, index) => { + if (section.type !== 'form') return + + if (section.crm && !section.id) { + errors.push( + `sections[${index}]: form sections that configure \`crm\` must declare an \`id\` — the server uses it to look up the trusted CRM config.` + ) + } + + if (section.id) { + if (section.id.length > FORM_ID_MAX_LENGTH || !FORM_ID_PATTERN.test(section.id)) { + errors.push( + `sections[${index}].id: "${section.id}" is not a valid form id (must be ≤${FORM_ID_MAX_LENGTH} chars, alphanumeric/underscore/dash, starting with alphanumeric).` + ) + } + if (formIds.has(section.id)) { + errors.push(`sections[${index}].id: duplicate form id "${section.id}" on this page.`) + } + formIds.add(section.id) + } + }) + + return errors +} + // ----- Inferred types ----- export type GoImage = z.infer diff --git a/packages/marketing/src/go/sections/FormSection.tsx b/packages/marketing/src/go/sections/FormSection.tsx index c4147eb1251..d34c9765234 100644 --- a/packages/marketing/src/go/sections/FormSection.tsx +++ b/packages/marketing/src/go/sections/FormSection.tsx @@ -3,7 +3,23 @@ import MarketingForm from '../../forms/MarketingForm' import type { GoFormSection } from '../schemas' -export default function FormSection({ section }: { section: GoFormSection }) { +/** + * Form props that are safe to ship to the client. The `crm` config from the + * page registry stays on the server — the client only learns the `{ slug, + * formId }` it needs to post back. The action then re-resolves `crm` from the + * trusted registry. See `submitFormAction` and PRODSEC-120. + */ +export type ClientFormSection = Omit + +export default function FormSection({ + section, + slug, +}: { + section: ClientFormSection + slug: string +}) { + const formRef = section.id ? { slug, formId: section.id } : undefined + return (
@@ -15,7 +31,7 @@ export default function FormSection({ section }: { section: GoFormSection }) { disclaimer={section.disclaimer} successMessage={section.successMessage} successRedirect={section.successRedirect} - crm={section.crm} + formRef={formRef} />
diff --git a/packages/marketing/src/go/sections/SectionRenderer.tsx b/packages/marketing/src/go/sections/SectionRenderer.tsx index 7fb6b0b5dc6..905810be084 100644 --- a/packages/marketing/src/go/sections/SectionRenderer.tsx +++ b/packages/marketing/src/go/sections/SectionRenderer.tsx @@ -21,10 +21,12 @@ export type CustomSectionRenderers = { interface SectionRendererProps { section: GoSection + /** Page slug — threaded through so form sections can build a server-resolvable formRef. */ + slug: string customRenderers?: CustomSectionRenderers } -export default function SectionRenderer({ section, customRenderers }: SectionRendererProps) { +export default function SectionRenderer({ section, slug, customRenderers }: SectionRendererProps) { // Check for a custom renderer first const CustomRenderer = customRenderers?.[section.type] as | React.ComponentType<{ section: typeof section }> @@ -45,9 +47,13 @@ export default function SectionRenderer({ section, customRenderers }: SectionRen case 'three-column': content = break - case 'form': - content = + case 'form': { + // Drop CRM config before the section crosses into the client bundle — + // the server action re-resolves it from the registry by formRef. + const { crm: _crm, ...clientSection } = section + content = break + } case 'feature-grid': content = break diff --git a/packages/marketing/src/go/templates/LeadGenTemplate.tsx b/packages/marketing/src/go/templates/LeadGenTemplate.tsx index 125676c578b..7af08c4186d 100644 --- a/packages/marketing/src/go/templates/LeadGenTemplate.tsx +++ b/packages/marketing/src/go/templates/LeadGenTemplate.tsx @@ -13,7 +13,12 @@ export default function LeadGenTemplate({
{page.sections?.map((section, i) => ( - + ))}
) diff --git a/packages/marketing/src/go/templates/LegalTemplate.tsx b/packages/marketing/src/go/templates/LegalTemplate.tsx index 66637efc98d..d0095c3839d 100644 --- a/packages/marketing/src/go/templates/LegalTemplate.tsx +++ b/packages/marketing/src/go/templates/LegalTemplate.tsx @@ -25,7 +25,12 @@ export default function LegalTemplate({ {page.sections?.map((section, i) => ( - + ))} ) diff --git a/packages/marketing/src/go/templates/ThankYouTemplate.tsx b/packages/marketing/src/go/templates/ThankYouTemplate.tsx index 35cbce99583..02dc573be5a 100644 --- a/packages/marketing/src/go/templates/ThankYouTemplate.tsx +++ b/packages/marketing/src/go/templates/ThankYouTemplate.tsx @@ -17,7 +17,12 @@ export default function ThankYouTemplate({ {page.sections?.map((section, i) => ( - + ))} )