From 071f3447fd4239438174d10b1f1d38181f408b8e Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 10 Aug 2026 08:37:42 -0700 Subject: [PATCH 1/3] Add static security advisory to HTML block style panel Custom HTML block renders arbitrary markup on the published site. Scripts and event handlers are stripped, but forms, iframes, and images survive and can send data elsewhere. Add a plain, always-shown advisory (no content detection) alongside the existing wrapper-styling note. Co-Authored-By: Claude Opus 5 (1M context) --- craft/src/panels/right/styles/HtmlStylePanel.test.tsx | 6 ++++++ craft/src/panels/right/styles/HtmlStylePanel.tsx | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/craft/src/panels/right/styles/HtmlStylePanel.test.tsx b/craft/src/panels/right/styles/HtmlStylePanel.test.tsx index 891d2cd..f065fff 100644 --- a/craft/src/panels/right/styles/HtmlStylePanel.test.tsx +++ b/craft/src/panels/right/styles/HtmlStylePanel.test.tsx @@ -35,4 +35,10 @@ describe('HtmlStylePanel', () => { expect(container.textContent).not.toContain('Padding'); expect(container.textContent).not.toContain('Border Radius'); }); + + test('renders the security advisory', () => { + render(x

', style: {} }} />); + expect(container.textContent).toContain('Use this block with care.'); + expect(container.textContent).toContain('Scripts and event handlers are stripped'); + }); }); diff --git a/craft/src/panels/right/styles/HtmlStylePanel.tsx b/craft/src/panels/right/styles/HtmlStylePanel.tsx index 224435f..691857e 100644 --- a/craft/src/panels/right/styles/HtmlStylePanel.tsx +++ b/craft/src/panels/right/styles/HtmlStylePanel.tsx @@ -25,6 +25,13 @@ export const HtmlStylePanel: React.FC<{ selectedId: string; nodeProps: Record +

+ Use this block with care. It renders your markup as-is + on the published site. Scripts and event handlers are stripped + automatically, but anything that survives — forms, iframes, images — + can still send data to wherever it points. Only paste code you + understand or trust. +

); }; -- 2.52.0 From d1c57db967e9fa90ed174c562a5c7fa248b18c3f Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 10 Aug 2026 15:45:05 -0700 Subject: [PATCH 2/3] feat(site-builder): webhook destination controls on the contact form ContactForm gains four craft props (destinationType, webhookUrl, webhookSecretId, webhookAuthMode), all present in craft.props defaults so FormStylePanel's `nodeProps.X !== undefined` gates actually render their controls. The controls live in FormStylePanel (RightPanel renders only GuidedStyles, so related.settings would be dead UI). relayFormWiring widens the marker to optionally carry type/url/secret/ authmode BETWEEN `id` and `recipient`, which is where FormRelayRewrite.php's parser looks. A marker with no type is byte-identical to what shipped before -- pinned by a test that diffs an explicit-email form against one with no destination props at all, since every already-published site depends on that shape continuing to provision an email endpoint. Every optional attribute value goes through one escaping site (markerAttr -> escapeAttr); type and authmode are additionally allowlisted, so a case-drifted "Bearer" reaches the relay as the exact literal it compares against instead of being silently downgraded to unsigned. The raw shared secret is never a prop: it is held in WebhookSecretField's local state, POSTed to /api/form-webhook-secret.php on blur, and only the returned opaque id is persisted. The field is write-only (set / replace / remove, never view) because the endpoint has no read route, and the endpoint's 429 cap message is surfaced verbatim so a customer can act on it. Co-Authored-By: Claude Opus 5 (1M context) --- .../forms/ContactForm.toHtml.test.ts | 80 +++++++ craft/src/components/forms/ContactForm.tsx | 28 ++- .../panels/right/styles/FormStylePanel.tsx | 149 +++++++++++- .../styles/FormStylePanel.webhook.test.tsx | 222 ++++++++++++++++++ craft/src/utils/form-relay-wiring.test.ts | 117 +++++++++ craft/src/utils/form-relay-wiring.ts | 101 +++++++- craft/src/utils/form-webhook-secret.test.ts | 79 +++++++ craft/src/utils/form-webhook-secret.ts | 64 +++++ 8 files changed, 831 insertions(+), 9 deletions(-) create mode 100644 craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx create mode 100644 craft/src/utils/form-webhook-secret.test.ts create mode 100644 craft/src/utils/form-webhook-secret.ts diff --git a/craft/src/components/forms/ContactForm.toHtml.test.ts b/craft/src/components/forms/ContactForm.toHtml.test.ts index d29b243..e72c4aa 100644 --- a/craft/src/components/forms/ContactForm.toHtml.test.ts +++ b/craft/src/components/forms/ContactForm.toHtml.test.ts @@ -196,3 +196,83 @@ describe('ContactForm.craft.props includes animation/visibility defaults', () => }); }); }); + +/* ---------- Webhook destination (Task 10) ---------- + This file builds props inline rather than spreading a shared object, so + `defaultProps` is introduced here for the destination cases only; every + pre-existing test above is untouched. */ +const defaultProps = { fields: [] as any[], formAction: '#' }; + +describe('ContactForm.craft.props includes the destination defaults', () => { + // The trap this pins: FormStylePanel renders each destination control behind + // `nodeProps.X !== undefined`, so a prop omitted from these defaults yields an + // invisible control and the whole feature looks like it does nothing. + test('destinationType/webhookUrl/webhookSecretId/webhookAuthMode are all present', () => { + expect(ContactForm.craft!.props).toMatchObject({ + destinationType: 'email', + webhookUrl: '', + webhookSecretId: '', + webhookAuthMode: 'signature', + }); + }); + + test('no craft prop holds a raw secret -- only an id', () => { + const keys = Object.keys(ContactForm.craft!.props as object); + expect(keys).toContain('webhookSecretId'); + expect(keys.filter((k) => /secret/i.test(k))).toEqual(['webhookSecretId']); + }); +}); + +describe('ContactForm.toHtml destination marker', () => { + test('email destination emits the legacy marker unchanged', () => { + const out = toHtml( + { ...defaultProps, destinationType: 'email', recipientEmail: 'a@example.com', thankYouUrl: '' }, ''); + expect(out.html).toContain('
{ + const out = toHtml( + { ...defaultProps, destinationType: 'webhook', webhookUrl: 'https://hooks.example.com/x', + webhookSecretId: 'sec-1', webhookAuthMode: 'bearer', recipientEmail: 'fb@example.com' }, ''); + expect(out.html).toContain('type="webhook"'); + expect(out.html).toContain('url="https://hooks.example.com/x"'); + expect(out.html).toContain('secret="sec-1"'); + expect(out.html).toContain('authmode="bearer"'); + expect(out.html).toContain('recipient="fb@example.com"'); + }); + + test('a raw secret value is never emitted, only its id', () => { + const out = toHtml( + { ...defaultProps, destinationType: 'webhook', webhookUrl: 'https://hooks.example.com/x', + webhookSecretId: 'sec-1', webhookSecret: 'SUPERSECRET' } as any, ''); + // Non-vacuous: the marker IS emitted (so there is something that could have + // carried the secret) and carries the id, but not the value. + expect(out.html).toContain('secret="sec-1"'); + expect(out.html).not.toContain('SUPERSECRET'); + }); + + test('a webhook form with no fallback email still emits a marker (never a bare formAction)', () => { + const out = toHtml( + { ...defaultProps, destinationType: 'webhook', webhookUrl: 'https://hooks.example.com/x' }, ''); + expect(out.html).toContain('type="webhook"'); + expect(out.html).toContain('recipient=""'); + expect(out.html).toMatch(/action="__WHP_FORM_ACTION__F_[0-9a-z]+__"/); + expect(out.html).toContain('name="_gotcha"'); + }); +}); diff --git a/craft/src/components/forms/ContactForm.tsx b/craft/src/components/forms/ContactForm.tsx index 2e410f6..f525efa 100644 --- a/craft/src/components/forms/ContactForm.tsx +++ b/craft/src/components/forms/ContactForm.tsx @@ -34,6 +34,16 @@ interface ContactFormProps { inputBorder?: string; recipientEmail?: string; thankYouUrl?: string; + /* ---- Submission destination (see utils/form-relay-wiring.ts) ---- + 'email' (default) reproduces the legacy marker exactly. 'webhook' widens it + with the url / secret id / auth mode below. + There is deliberately NO raw-secret prop: craft props are serialised into + the saved project and into published output, so the secret is POSTed to + /api/form-webhook-secret.php and only the returned opaque id is kept. */ + destinationType?: 'email' | 'webhook'; + webhookUrl?: string; + webhookSecretId?: string; + webhookAuthMode?: 'signature' | 'bearer'; animation?: string; animationDelay?: string; hideOnDesktop?: boolean; @@ -171,6 +181,12 @@ ContactForm.craft = { inputBorder: '#d1d5db', recipientEmail: '', thankYouUrl: '', + // Present (not omitted) so FormStylePanel's `nodeProps.X !== undefined` + // gates actually render the destination controls. + destinationType: 'email', + webhookUrl: '', + webhookSecretId: '', + webhookAuthMode: 'signature', animation: '', animationDelay: '', hideOnDesktop: false, @@ -233,7 +249,17 @@ ContactForm.craft = { alignSelf: 'flex-start', }); - const { marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.formAction, nodeId); + // Only the webhook SECRET ID travels here -- there is no prop holding the raw + // secret, by construction (see ContactFormProps). + const { marker, actionAttr, honeypot } = relayFormWiring( + props.recipientEmail, props.thankYouUrl, props.formAction, nodeId, + { + type: props.destinationType, + url: props.webhookUrl, + secretId: props.webhookSecretId, + authMode: props.webhookAuthMode, + }, + ); // The form-sender relay delivers success via a full-page 303 redirect // (to thankYouUrl or a hosted thanks.php page) -- there is no in-page JS diff --git a/craft/src/panels/right/styles/FormStylePanel.tsx b/craft/src/panels/right/styles/FormStylePanel.tsx index bafa26c..8122c86 100644 --- a/craft/src/panels/right/styles/FormStylePanel.tsx +++ b/craft/src/panels/right/styles/FormStylePanel.tsx @@ -1,5 +1,6 @@ -import React from 'react'; +import React, { useState } from 'react'; import { useEditor } from '@craftjs/core'; +import { storeWebhookSecret } from '../../../utils/form-webhook-secret'; import { BG_COLORS, SPACING_PRESETS, @@ -58,12 +59,87 @@ const SPACING_SIDE_KEYS: { side: 'top' | 'right' | 'bottom' | 'left'; suffix: 'T { side: 'left', suffix: 'Left' }, ]; +const hintStyle: React.CSSProperties = { fontSize: 10, color: '#71717a', margin: '4px 0 0' }; + +/* ---------- Webhook shared secret: WRITE-ONLY field ---------- + The raw secret lives in this component's local state and nowhere else. On + blur it is POSTed to the panel endpoint, which returns an opaque id; only + that id is handed to `onStored` (and thence to a craft prop). The field is + then cleared, because there is no read route and nothing to show back -- + the UI offers set / replace / remove, never "view". */ +export const WebhookSecretField: React.FC<{ + secretId: string; + onStored: (secretId: string) => void; + onRemoved: () => void; +}> = ({ secretId, onStored, onRemoved }) => { + const [draft, setDraft] = useState(''); + const [status, setStatus] = useState<'idle' | 'saving' | 'saved' | 'error'>('idle'); + const [error, setError] = useState(''); + + const save = async () => { + const raw = draft.trim(); + if (raw === '' || status === 'saving') return; + setStatus('saving'); + setError(''); + const result = await storeWebhookSecret(raw); + if (result.ok && result.secretId) { + // Only the id crosses this line. The raw value is dropped here and is + // never written to a prop, to storage, or back into the input. + onStored(result.secretId); + setDraft(''); + setStatus('saved'); + } else { + setStatus('error'); + setError(result.error || 'Could not store the secret.'); + } + }; + + return ( +
+ + { setDraft(e.target.value); if (status !== 'idle') { setStatus('idle'); setError(''); } }} + onBlur={() => { void save(); }} + placeholder={secretId ? 'Paste a new secret to replace' : 'Paste the secret from your receiver'} + style={inputStyle} + /> + {secretId && ( +
+ Secret stored + +
+ )} + {status === 'saving' &&

Storing…

} + {status === 'saved' &&

Secret stored.

} + {status === 'error' &&

{error}

} +

+ Stored on the server and never shown again — paste a new one to replace it. Used to sign + (or authorise) each delivery so your receiver can verify it came from this site. +

+
+ ); +}; + /* ---------- FORM ---------- */ export const FormStylePanel: React.FC = ({ selectedId, nodeProps }) => { const { actions } = useEditor(); const { setProp, setPropStyle } = useNodeProp(selectedId); const style = nodeProps.style || {}; + // ContactForm only: FormContainer/SubscribeForm have no destinationType prop, + // so their relay controls stay exactly as they were. + const isWebhook = nodeProps.destinationType === 'webhook'; const updateField = (index: number, patch: Record) => { actions.setProp(selectedId, (props: any) => { @@ -158,14 +234,77 @@ export const FormStylePanel: React.FC = ({ selectedId, nodeProp )} + {/* Destination: email (default, unchanged behaviour) or webhook. Gated on + the `destinationType` default in ContactForm.craft.props -- a prop + missing from those defaults is `undefined` here and the control would + simply never render. */} + {nodeProps.destinationType !== undefined && ( +
+ +
+ {[{ v: 'email', l: 'Email' }, { v: 'webhook', l: 'Webhook' }].map((o) => ( + + ))} +
+
+ )} + + {nodeProps.destinationType !== undefined && isWebhook && ( + <> +
+ + setProp('webhookUrl', e.target.value)} + placeholder="https://hooks.example.com/..." + style={inputStyle} + /> +

+ Must be an absolute https:// URL. Each submission is POSTed as JSON; + failures are retried, then emailed to the fallback address below. +

+
+
+ + +
+ setProp('webhookSecretId', id)} + onRemoved={() => setProp('webhookSecretId', '')} + /> + + )} + {/* Contact-form relay: where submissions are emailed. Present on ContactForm - and FormContainer (both have recipientEmail/thankYouUrl props). */} + and FormContainer (both have recipientEmail/thankYouUrl props). With a + webhook destination this same address is the FALLBACK the relay uses + when delivery is exhausted. */} {nodeProps.recipientEmail !== undefined && (
- + setProp('recipientEmail', e.target.value)} placeholder="you@example.com" style={inputStyle} /> -

- Emailed via the site's contact-form relay (an admin must enable it in Server Settings). Leave blank to use the Form Action URL instead. +

+ {isWebhook + ? 'Emailed here if the webhook keeps failing after retries. Leave blank to skip the fallback.' + : "Emailed via the site's contact-form relay (an admin must enable it in Server Settings). Leave blank to use the Form Action URL instead."}

)} diff --git a/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx b/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx new file mode 100644 index 0000000..c3fde75 --- /dev/null +++ b/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx @@ -0,0 +1,222 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; + +/* Same DOM harness + craftjs mock as NavStylePanel.test.tsx (no + @testing-library/react in this repo). `lastProps` IS the node's prop bag: the + mocked setProp mutates it exactly as Craft.js would, which is what lets the + "no raw secret ever reaches a prop" assertion below be a real check on + everything the panel writes rather than on a hand-picked key. */ +const setPropSpy = vi.fn((_id: string, updater: (p: any) => void) => { + updater(lastProps); +}); +let lastProps: any; + +vi.mock('@craftjs/core', () => ({ + useEditor: () => ({ actions: { setProp: setPropSpy } }), +})); + +import { FormStylePanel } from './FormStylePanel'; + +const RAW_SECRET = 'hunter2-SUPER-SECRET-VALUE'; + +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function rerender(ui: React.ReactElement) { + act(() => { root.render(ui); }); +} + +function unmount() { + act(() => { root.unmount(); }); + container.remove(); +} + +function setValue(el: HTMLInputElement | HTMLSelectElement, value: string) { + const proto = el instanceof HTMLSelectElement ? window.HTMLSelectElement.prototype : window.HTMLInputElement.prototype; + const setter = Object.getOwnPropertyDescriptor(proto, 'value')!.set!; + act(() => { + setter.call(el, value); + el.dispatchEvent(new Event('input', { bubbles: true })); + el.dispatchEvent(new Event('change', { bubbles: true })); + }); +} + +function click(el: Element | null) { + if (!el) throw new Error('element not found'); + act(() => { (el as HTMLElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); }); +} + +/* React 17+ implements onBlur with the native `focusout` event (which bubbles), + not `blur`. */ +async function blur(el: Element) { + await act(async () => { + el.dispatchEvent(new FocusEvent('focusout', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +const testid = (id: string) => container.querySelector(`[data-testid="${id}"]`); + +function contactFormProps(over: Record = {}) { + return { + fields: [], + style: {}, + recipientEmail: '', + thankYouUrl: '', + destinationType: 'email', + webhookUrl: '', + webhookSecretId: '', + webhookAuthMode: 'signature', + ...over, + }; +} + +beforeEach(() => { + setPropSpy.mockClear(); + (window as any).WHP_CONFIG = { apiUrl: '/api/site-builder.php', csrfToken: 'tok', siteId: 42 }; +}); + +afterEach(() => { + if (container) unmount(); + delete (window as any).WHP_CONFIG; + vi.unstubAllGlobals(); +}); + +describe('FormStylePanel destination selector', () => { + test('choosing Webhook writes destinationType and reveals the webhook fields', () => { + lastProps = contactFormProps(); + render(); + expect(testid('webhook-url')).toBeNull(); + + click(testid('destination-webhook')); + expect(lastProps.destinationType).toBe('webhook'); + + rerender(); + expect(testid('webhook-url')).toBeTruthy(); + expect(testid('webhook-authmode')).toBeTruthy(); + expect(testid('webhook-secret-input')).toBeTruthy(); + }); + + test('the URL and auth-mode controls write their props', () => { + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + setValue(testid('webhook-url') as HTMLInputElement, 'https://hooks.example.com/x'); + expect(lastProps.webhookUrl).toBe('https://hooks.example.com/x'); + setValue(testid('webhook-authmode') as HTMLSelectElement, 'bearer'); + expect(lastProps.webhookAuthMode).toBe('bearer'); + }); + + test('the recipient field is relabelled as the fallback address for a webhook', () => { + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + const labels = Array.from(container.querySelectorAll('label')).map((l) => l.textContent); + expect(labels.some((t) => t?.includes('Fallback email'))).toBe(true); + expect(labels.some((t) => t === 'Send submissions to (email)')).toBe(false); + }); + + test('a component without destinationType (FormContainer) shows neither the selector nor the webhook fields', () => { + lastProps = { recipientEmail: '', thankYouUrl: '', style: {} }; + render(); + expect(testid('destination-webhook')).toBeNull(); + expect(testid('webhook-url')).toBeNull(); + const labels = Array.from(container.querySelectorAll('label')).map((l) => l.textContent); + expect(labels).toContain('Send submissions to (email)'); + }); +}); + +describe('FormStylePanel webhook secret field is WRITE-ONLY', () => { + test('blurring the secret POSTs it and persists ONLY the returned id', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, status: 200, + json: async () => ({ success: true, secret_id: 'whs_42_deadbeefdeadbeef' }), + }); + vi.stubGlobal('fetch', fetchMock); + + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + const input = testid('webhook-secret-input') as HTMLInputElement; + setValue(input, RAW_SECRET); + await blur(input); + + // The flow really ran (otherwise the assertions below would pass vacuously). + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(JSON.parse(fetchMock.mock.calls[0][1].body).secret).toBe(RAW_SECRET); + expect(lastProps.webhookSecretId).toBe('whs_42_deadbeefdeadbeef'); + + // THE PROPERTY: nothing the panel wrote to the node's props contains the raw + // secret, under any key -- craft props are serialised into the saved project + // and into published output. + expect(JSON.stringify(lastProps)).not.toContain(RAW_SECRET); + for (const [, updater] of setPropSpy.mock.calls) { + const probe: any = {}; + (updater as (p: any) => void)(probe); + expect(JSON.stringify(probe)).not.toContain(RAW_SECRET); + } + + // ...and the input is cleared, so it isn't sitting in the DOM either. + expect((testid('webhook-secret-input') as HTMLInputElement).value).toBe(''); + expect(testid('webhook-secret-saved')).toBeTruthy(); + }); + + test('typing a secret without blurring writes nothing at all', () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + setValue(testid('webhook-secret-input') as HTMLInputElement, RAW_SECRET); + expect(fetchMock).not.toHaveBeenCalled(); + expect(JSON.stringify(lastProps)).not.toContain(RAW_SECRET); + }); + + test('a failed store surfaces the endpoint message and persists no id', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, status: 429, + json: async () => ({ success: false, error: 'Too many webhook secrets stored for this site — please contact support.' }), + })); + + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + const input = testid('webhook-secret-input') as HTMLInputElement; + setValue(input, RAW_SECRET); + await blur(input); + + expect(testid('webhook-secret-error')!.textContent).toContain('Too many webhook secrets stored for this site'); + expect(lastProps.webhookSecretId).toBe(''); + expect(JSON.stringify(lastProps)).not.toContain(RAW_SECRET); + }); + + test('the stored secret is never shown -- only its state, with a Remove action', () => { + lastProps = contactFormProps({ destinationType: 'webhook', webhookSecretId: 'whs_42_deadbeefdeadbeef' }); + render(); + expect(testid('webhook-secret-status')!.textContent).toContain('Secret stored'); + // The field is a password input that starts empty: there is no read route to + // populate it from, and nothing anywhere renders the value. + const input = testid('webhook-secret-input') as HTMLInputElement; + expect(input.type).toBe('password'); + expect(input.value).toBe(''); + + click(testid('webhook-secret-remove')); + expect(lastProps.webhookSecretId).toBe(''); + }); + + test('blurring an empty secret field posts nothing', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + await blur(testid('webhook-secret-input')!); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/craft/src/utils/form-relay-wiring.test.ts b/craft/src/utils/form-relay-wiring.test.ts index eea770c..91b8516 100644 --- a/craft/src/utils/form-relay-wiring.test.ts +++ b/craft/src/utils/form-relay-wiring.test.ts @@ -36,3 +36,120 @@ describe('relayFormWiring deterministic + unique fid (thread node id, no Math.ra expect(w1.actionAttr).toBe(w2.actionAttr); }); }); + +/* ---------- Webhook destination (Task 10) ---------- */ + +/** The publish-side parser, verbatim from web-files/libs/FormRelayRewrite.php's + * fs_rewrite_contact_forms() -- ported to JS so a drift in the emitter's + * attribute ORDER, NAMES or QUOTING fails here rather than at publish time, + * where it either leaks the recipient address or refuses the publish. */ +const PUBLISH_MARKER_RE = + /^$/; + +describe('relayFormWiring email destination stays byte-identical to the legacy shape', () => { + test('no destination arg, type undefined, and type "email" all produce the same marker', () => { + const legacy = relayFormWiring('a@b.com', '/thx', '/act', 'n1'); + const undef = relayFormWiring('a@b.com', '/thx', '/act', 'n1', {}); + const email = relayFormWiring('a@b.com', '/thx', '/act', 'n1', { type: 'email', url: 'https://x.example/y', secretId: 'whs_1_abc', authMode: 'bearer' }); + expect(undef.marker).toBe(legacy.marker); + expect(email.marker).toBe(legacy.marker); + expect(email.actionAttr).toBe(legacy.actionAttr); + expect(legacy.marker).toMatch(/^$/); + }); + + test('an email form with no recipient is still not a relay at all', () => { + const w = relayFormWiring('', '/thx', '/legacy', 'n1', { type: 'email' }); + expect(w.useRelay).toBe(false); + expect(w.marker).toBe(''); + }); +}); + +describe('relayFormWiring webhook marker matches the publish-side parser exactly', () => { + const w = relayFormWiring('fb@b.com', '/thx', '#', 'n1', { + type: 'webhook', url: 'https://hooks.example.com/x', secretId: 'whs_7_abc123', authMode: 'bearer', + }); + + test('the whole marker parses with FormRelayRewrite.php\'s pattern', () => { + const m = w.marker.match(PUBLISH_MARKER_RE); + expect(m).not.toBeNull(); + expect(m![3]).toBe('fb@b.com'); + expect(m![4]).toBe('/thx'); + }); + + test('the optional attributes sit BETWEEN id and recipient, lowercase-named', () => { + const attrs = w.marker.match(PUBLISH_MARKER_RE)![2]; + expect(attrs).toBe(' type="webhook" url="https://hooks.example.com/x" secret="whs_7_abc123" authmode="bearer"'); + }); + + test('a webhook with no recipient still emits a relay marker', () => { + const bare = relayFormWiring('', '', '#', 'n1', { type: 'webhook', url: 'https://hooks.example.com/x' }); + expect(bare.useRelay).toBe(true); + expect(bare.marker).toMatch(PUBLISH_MARKER_RE); + expect(bare.marker).toContain('authmode="signature"'); + }); + + test('two webhook forms with no node id and different urls get different fids', () => { + const a = relayFormWiring('', '', '#', undefined, { type: 'webhook', url: 'https://a.example/x' }); + const b = relayFormWiring('', '', '#', undefined, { type: 'webhook', url: 'https://b.example/x' }); + expect(a.marker).not.toBe(b.marker); + }); +}); + +describe('relayFormWiring escapes EVERY marker attribute value', () => { + /* The property the publish-time strip is built on: no raw `"`, `<`, `>` or a + literal `-->` may reach a marker attribute value. An unescaped one truncates + the strip mid-marker and ships the recipient address in the page source, or + trips the post-condition and refuses the publish outright. */ + const hostile = 'a"bd\'e-->f'; + const w = relayFormWiring(`${hostile}@x.com`, hostile, '#', 'n1', { + type: 'webhook', url: `https://x/${hostile}`, secretId: hostile, authMode: 'bearer', + }); + + test('the marker still parses as ONE marker (nothing escaped out of a value)', () => { + expect(w.marker).toMatch(PUBLISH_MARKER_RE); + }); + + test.each([ + ['url', `https://x/${hostile}`], + ['secret', hostile], + ['recipient', `${hostile}@x.com`], + ['thankyou', hostile], + ])('%s carries no raw ", <, > or -->', (name) => { + const value = w.marker.match(new RegExp(` ${name}="([^"]*)"`))![1]; + expect(value).not.toMatch(/["<>]/); + expect(value).not.toContain('-->'); + expect(value).toContain('"'); + expect(value).toContain('<'); + expect(value).toContain('>'); + }); + + test('the raw hostile string never appears anywhere in the marker', () => { + expect(w.marker).not.toContain(hostile); + }); +}); + +describe('relayFormWiring allowlists type and authmode instead of trusting them', () => { + test('type is matched case-insensitively -- "Webhook" is a webhook, not a silent email', () => { + const w = relayFormWiring('a@b.com', '', '#', 'n1', { type: 'WebHook', url: 'https://x/y' }); + expect(w.marker).toContain('type="webhook"'); + }); + + test('an unknown type falls back to the legacy email marker', () => { + const w = relayFormWiring('a@b.com', '', '#', 'n1', { type: 'slack', url: 'https://x/y' } as any); + expect(w.marker).not.toContain('type='); + expect(w.marker).toBe(relayFormWiring('a@b.com', '', '#', 'n1').marker); + }); + + test('a hostile authMode collapses to signature and cannot break out of the attribute', () => { + const w = relayFormWiring('a@b.com', '', '#', 'n1', { + type: 'webhook', url: 'https://x/y', authMode: 'bearer" onx="1', + }); + expect(w.marker).toContain('authmode="signature"'); + expect(w.marker).toMatch(PUBLISH_MARKER_RE); + }); + + test('"Bearer" is promoted to the exact literal the relay compares against', () => { + const w = relayFormWiring('a@b.com', '', '#', 'n1', { type: 'webhook', url: 'https://x/y', authMode: ' Bearer ' }); + expect(w.marker).toContain('authmode="bearer"'); + }); +}); diff --git a/craft/src/utils/form-relay-wiring.ts b/craft/src/utils/form-relay-wiring.ts index d37ea3f..98c706f 100644 --- a/craft/src/utils/form-relay-wiring.ts +++ b/craft/src/utils/form-relay-wiring.ts @@ -11,6 +11,58 @@ import { escapeAttr, safeUrl, scopeId } from './escape'; +/** + * Where a form's submissions go. Optional on every call site: a form that + * passes nothing here (or `type: 'email'`) emits the LEGACY marker, byte for + * byte -- see `relayFormWiring` below. + */ +export interface FormDestination { + /** 'webhook' (case-insensitive) selects the webhook path; anything else = email. */ + type?: string; + /** Absolute https URL the relay POSTs to. Validated at publish time. */ + url?: string; + /** Opaque id minted by /api/form-webhook-secret.php. NEVER the raw secret. */ + secretId?: string; + /** 'bearer' or 'signature' (HMAC, the default). */ + authMode?: string; +} + +/** The two destination types this emitter knows how to describe. */ +const DESTINATION_TYPES = ['email', 'webhook'] as const; +/** The two auth modes the relay implements (FormRelayProvisioner::upsertWebhookToken). */ +const AUTH_MODES = ['signature', 'bearer'] as const; + +/** + * Allowlist a destination type / auth mode rather than escaping it. + * + * Same reasoning as `sanitizeInputType`/`sanitizeFormMethod` in ./escape: these + * props are declared as unions in TS but arrive raw from a deserialized saved + * state or the AI `update_props` path, and the only legitimate values are a + * fixed pair. Narrowing here also stops a case-drifted `"Bearer"` from being + * silently downgraded to `signature` by the publish step (which compares + * `=== 'bearer'` exactly) -- the customer would see unsigned deliveries with + * nothing in the UI to explain it. + */ +function allowlist(value: unknown, allowed: readonly T[], fallback: T): T { + const v = (typeof value === 'string' ? value : '').trim().toLowerCase(); + return (allowed as readonly string[]).includes(v) ? (v as T) : fallback; +} + +/** + * Emit one ` name="value"` marker attribute. + * + * THE SINGLE ESCAPING SITE for every optional marker attribute. The publish-time + * strip (whp: web-files/libs/FormRelayRewrite.php) has been hardened six times + * over exactly this: an unescaped `<`, `>` or a literal `-->` inside a marker + * attribute value truncates the strip mid-marker and leaks the customer's + * recipient address into their public page source, or trips the post-condition + * and refuses the publish outright. `escapeAttr` removing `"` is also what keeps + * each value inside the `[^"]*` the publish-side parser expects. + */ +function markerAttr(name: string, value: string): string { + return ` ${name}="${escapeAttr(value)}"`; +} + export interface RelayWiring { /** true when a recipient is set (relay path); false = legacy formAction fallback */ useRelay: boolean; @@ -30,20 +82,63 @@ export interface RelayWiring { * the marker/placeholder id deterministically and uniquely -- * see `scopeId` in ./escape. Falls back to a stable hash of the * recipient/thankYouUrl/fallbackAction when omitted (never random). + * @param destination optional destination descriptor. Omitted, or `type: 'email'`, + * yields the LEGACY narrow marker byte for byte -- every + * already-published site depends on that shape continuing to + * provision an email endpoint. + * + * The wide (webhook) marker keeps the optional attributes BETWEEN `id` and + * `recipient`, which is where the publish-side parser looks for them: + * + * // + * + * (FormRelayRewrite.php). Attribute NAMES must therefore be lowercase, and every + * VALUE must be free of `"` -- both guaranteed here, the latter by `markerAttr`. + * + * A webhook marker is emitted whenever the customer selected webhook, even with a + * blank URL: the publish step then refuses that one endpoint and logs it (the form + * publishes inert). Falling back to the email path instead would deliver mail to a + * customer who configured a webhook, with nothing anywhere to explain it -- the + * exact silent degradation the publish-side `type` normalisation exists to stop. */ export function relayFormWiring( recipientEmail: string | undefined, thankYouUrl: string | undefined, fallbackAction: string | undefined, nodeId?: string, + destination?: FormDestination, ): RelayWiring { - if (!recipientEmail) { + const destType = allowlist(destination?.type, DESTINATION_TYPES, 'email'); + const isWebhook = destType === 'webhook'; + + // No destination at all: nothing to deliver to, so no relay (unchanged). + if (!recipientEmail && !isWebhook) { return { useRelay: false, marker: '', actionAttr: escapeAttr(safeUrl(fallbackAction || '#')), honeypot: '' }; } - const fid = scopeId(nodeId, `${recipientEmail}::${thankYouUrl || ''}::${fallbackAction || ''}`, 'F'); + + // Webhook config participates in the fallback seed so two webhook forms with + // no node id and no recipient don't collide on one fid. Appended only in the + // webhook branch, so the legacy seed -- and therefore every legacy fid -- is + // unchanged. + const seed = `${recipientEmail || ''}::${thankYouUrl || ''}::${fallbackAction || ''}` + + (isWebhook ? `::webhook::${destination?.url || ''}::${destination?.secretId || ''}` : ''); + const fid = scopeId(nodeId, seed, 'F'); + + // The `url` value is NOT routed through `safeUrl`: it is never a live sink + // (it lands in an HTML comment that the publish step strips), and blanking it + // here would silently turn a mistyped destination into an inert form with no + // log line. The publish step validates it properly -- absolute https, no + // control characters -- and refuses loudly when it doesn't hold. + const extraAttrs = isWebhook + ? markerAttr('type', 'webhook') + + markerAttr('url', destination?.url || '') + + markerAttr('secret', destination?.secretId || '') + + markerAttr('authmode', allowlist(destination?.authMode, AUTH_MODES, 'signature')) + : ''; + return { useRelay: true, - marker: ``, + marker: ``, actionAttr: `__WHP_FORM_ACTION__${fid}__`, honeypot: ``, }; diff --git a/craft/src/utils/form-webhook-secret.test.ts b/craft/src/utils/form-webhook-secret.test.ts new file mode 100644 index 0000000..ec4ae08 --- /dev/null +++ b/craft/src/utils/form-webhook-secret.test.ts @@ -0,0 +1,79 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import { storeWebhookSecret, webhookSecretEndpoint } from './form-webhook-secret'; + +const CFG = { apiUrl: '/api/site-builder.php', csrfToken: 'tok-123', siteId: 42 }; + +beforeEach(() => { + (window as any).WHP_CONFIG = { ...CFG }; +}); + +afterEach(() => { + delete (window as any).WHP_CONFIG; + vi.unstubAllGlobals(); +}); + +describe('webhookSecretEndpoint', () => { + test('derives the sibling endpoint from the configured API url', () => { + expect(webhookSecretEndpoint('/api/site-builder.php')).toBe('/api/form-webhook-secret.php'); + expect(webhookSecretEndpoint('https://panel.example.com/api/site-builder')) + .toBe('https://panel.example.com/api/form-webhook-secret.php'); + }); + + test('falls back to the absolute path when there is no configured url', () => { + expect(webhookSecretEndpoint(undefined)).toBe('/api/form-webhook-secret.php'); + expect(webhookSecretEndpoint('')).toBe('/api/form-webhook-secret.php'); + }); +}); + +describe('storeWebhookSecret', () => { + test('POSTs the secret with the CSRF header and returns only the id', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, status: 200, + json: async () => ({ success: true, secret_id: 'whs_42_abcdef0123456789' }), + }); + vi.stubGlobal('fetch', fetchMock); + + const result = await storeWebhookSecret('SUPERSECRET'); + + expect(result).toEqual({ ok: true, secretId: 'whs_42_abcdef0123456789' }); + const [url, opts] = fetchMock.mock.calls[0]; + expect(url).toBe('/api/form-webhook-secret.php'); + // POST only -- the endpoint has no read route by design. + expect(opts.method).toBe('POST'); + expect(opts.headers['X-CSRF-Token']).toBe('tok-123'); + expect(JSON.parse(opts.body)).toEqual({ site_id: 42, secret: 'SUPERSECRET' }); + }); + + test('passes the 429 cap message through so a customer can act on it', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, status: 429, + json: async () => ({ success: false, error: 'Too many webhook secrets stored for this site — please contact support.' }), + })); + const result = await storeWebhookSecret('s'); + expect(result.ok).toBe(false); + expect(result.error).toContain('Too many webhook secrets stored for this site'); + }); + + test('a success:false body is a failure even with HTTP 200', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, status: 200, json: async () => ({ success: false, error: 'Invalid CSRF token' }), + })); + expect(await storeWebhookSecret('s')).toEqual({ ok: false, error: 'Invalid CSRF token' }); + }); + + test('a network failure resolves with an error rather than throwing', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('offline'))); + const result = await storeWebhookSecret('s'); + expect(result.ok).toBe(false); + expect(result.secretId).toBeUndefined(); + }); + + test('standalone mode (no WHP_CONFIG) never posts anywhere', async () => { + delete (window as any).WHP_CONFIG; + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const result = await storeWebhookSecret('s'); + expect(result.ok).toBe(false); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/craft/src/utils/form-webhook-secret.ts b/craft/src/utils/form-webhook-secret.ts new file mode 100644 index 0000000..2de007f --- /dev/null +++ b/craft/src/utils/form-webhook-secret.ts @@ -0,0 +1,64 @@ +/* ---------- Contact-form webhook secret: WRITE-ONLY client ---------- + Posts a raw shared secret to the panel endpoint and gets back an opaque id. + + The raw secret is never stored anywhere on the client: it is passed to + `storeWebhookSecret` from local component state, and only the returned + `secret_id` is ever written to a craft prop. Craft props are serialised into + the saved project and into published output, which is the wrong tier for a + credential -- that is the whole reason this endpoint exists. + + THERE IS NO READ. The panel endpoint (web-files/api/form-webhook-secret.php) + is POST-only by design: a "show me my secret" route would re-open the exact + problem this closes. Rotation is another POST, which mints a NEW id. So the + UI can offer set / replace / remove, and never "view". */ + +export interface StoreSecretResult { + ok: boolean; + /** Present only on success -- `whs__`. */ + secretId?: string; + /** Customer-facing message on failure (the endpoint's own, when it sent one). */ + error?: string; +} + +/** + * Derive the secret endpoint from the configured API url, so a deployment that + * moves the panel API (or the vite dev proxy) doesn't need a second constant + * kept in sync: `/api/site-builder.php` -> `/api/form-webhook-secret.php`. + */ +export function webhookSecretEndpoint(apiUrl?: string): string { + const base = typeof apiUrl === 'string' ? apiUrl.trim() : ''; + if (base.includes('/')) return base.replace(/[^/]*$/, 'form-webhook-secret.php'); + return '/api/form-webhook-secret.php'; +} + +/** + * Store a raw webhook secret for the current site; resolves with its opaque id. + * + * Never throws and never returns the secret. Errors are returned as text fit to + * show a customer -- including the endpoint's 429 ("Too many webhook secrets + * stored for this site"), which is actionable (contact support) and so is + * passed through rather than flattened into a generic failure. + */ +export async function storeWebhookSecret(secret: string): Promise { + const cfg = (window as any).WHP_CONFIG; + if (!cfg) { + return { ok: false, error: 'Saving a webhook secret needs the builder to be open inside the control panel.' }; + } + try { + const resp = await fetch(webhookSecretEndpoint(cfg.apiUrl), { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': cfg.csrfToken }, + body: JSON.stringify({ site_id: cfg.siteId, secret }), + }); + const data = await resp.json().catch(() => null); + if (resp.ok && data && data.success === true && typeof data.secret_id === 'string' && data.secret_id !== '') { + return { ok: true, secretId: data.secret_id }; + } + const message = data && typeof data.error === 'string' && data.error !== '' + ? data.error + : `Could not store the secret (HTTP ${resp.status}).`; + return { ok: false, error: message }; + } catch { + return { ok: false, error: 'Could not reach the control panel to store the secret.' }; + } +} -- 2.52.0 From 422697acec98df35c103e45f428b913cc7ae2cfe Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 10 Aug 2026 15:57:38 -0700 Subject: [PATCH 3/3] fix(site-builder): review findings on the webhook destination controls I-1: WebhookSecretField now mounts with key={selectedId}. GuidedStyles renders FormStylePanel with no key, so a selection change re-rendered rather than remounted it and React kept `draft`/`status`. Since a failed store deliberately retains the draft, clicking a second contact form showed node A's raw secret in node B's field, and the next blur assigned the returned secret_id to the wrong form (burning a slot against the per-site cap). The status banner leaked the same way. Both pinned by the reviewer's repro sequence. I-2: the button is "Clear", not "Remove", and says so -- nothing deletes a stored key file, so the 50-per-site cap counts stores-ever. Labelling it Remove told the customer they had reclaimed a slot right up until the 429 that said otherwise. Actually deleting the file is Task 8's territory. M-1: inline warning when the webhook URL is blank or not absolute https. The publish step does refuse these, but into an error_log the customer never reads. Warning only -- isHttpsWebhookUrl never edits the value or blocks the publish, since either would trade a loud server-side refusal for a silently inert form. M-2: the BYTE-IDENTITY test now compares against a literal marker captured by executing the emitter at 071f3447, not against another head-revision output. The self-comparison could only catch a drift affecting one side; a uniform one passed it. Verified: a uniform `'; + const legacy = toHtml({ ...defaultProps, recipientEmail: 'a@example.com', thankYouUrl: '/thx' }, '', 'n1'); + expect(legacy.html.startsWith(`${FROZEN_LEGACY_MARKER} { diff --git a/craft/src/panels/right/styles/FormStylePanel.tsx b/craft/src/panels/right/styles/FormStylePanel.tsx index 8122c86..77e5a47 100644 --- a/craft/src/panels/right/styles/FormStylePanel.tsx +++ b/craft/src/panels/right/styles/FormStylePanel.tsx @@ -61,12 +61,40 @@ const SPACING_SIDE_KEYS: { side: 'top' | 'right' | 'bottom' | 'left'; suffix: 'T const hintStyle: React.CSSProperties = { fontSize: 10, color: '#71717a', margin: '4px 0 0' }; +/** + * Does this look like a webhook URL the publish step will accept? + * + * Mirrors the shape `FormRelayProvisioner::upsertWebhookToken()` enforces + * (absolute https, a host, no whitespace/control characters) closely enough to + * warn in the panel. It is a HINT, not a gate -- the server-side check is the + * real one, and this deliberately never edits or blocks the value. + */ +export function isHttpsWebhookUrl(value: unknown): boolean { + const v = typeof value === 'string' ? value.trim() : ''; + if (v === '' || /[\s\x00-\x1F\x7F]/.test(v)) return false; + try { + const u = new URL(v); + return u.protocol === 'https:' && u.hostname !== ''; + } catch { + return false; + } +} + /* ---------- Webhook shared secret: WRITE-ONLY field ---------- The raw secret lives in this component's local state and nowhere else. On blur it is POSTed to the panel endpoint, which returns an opaque id; only that id is handed to `onStored` (and thence to a craft prop). The field is then cleared, because there is no read route and nothing to show back -- - the UI offers set / replace / remove, never "view". */ + the UI offers set / replace / clear, never "view". + + MOUNT THIS WITH `key={selectedId}`. GuidedStyles renders + with no key, so a selection change re-renders this component rather than + remounting it, and React keeps `draft`/`status`. A failed store deliberately + RETAINS the draft (so a 429 or a network blip doesn't make the customer + retype a pasted key) -- which means without the key, clicking a second + contact form shows node A's raw secret in node B's field, and the next blur + assigns the returned id to the wrong form and burns a slot against the + per-site cap. The `status` banner leaks the same way. */ export const WebhookSecretField: React.FC<{ secretId: string; onStored: (secretId: string) => void; @@ -109,16 +137,26 @@ export const WebhookSecretField: React.FC<{ style={inputStyle} /> {secretId && ( -
- Secret stored - -
+ <> +
+ Secret stored + {/* "Clear", not "Remove": this only drops the form's reference to the + key. The stored key file stays on the server and still counts + toward the per-site cap -- nothing deletes one, so a button + labelled Remove would be telling the customer they had reclaimed + a slot right up until the 429 that says otherwise. */} + +
+

+ Clearing stops this form using the secret; the stored key stays on the server. +

+ )} {status === 'saving' &&

Storing…

} {status === 'saved' &&

Secret stored.

} @@ -268,6 +306,19 @@ export const FormStylePanel: React.FC = ({ selectedId, nodeProp placeholder="https://hooks.example.com/..." style={inputStyle} /> + {/* The publish step DOES refuse a blank/non-https URL -- loudly, but + into an error_log the customer never reads, leaving them with a + form that just doesn't work. Warn here instead. Deliberately a + warning only: not blanking the value and not blocking the + publish, since either would trade a loud server-side refusal for + a silently inert form. */} + {!isHttpsWebhookUrl(nodeProps.webhookUrl) && ( +

+ {nodeProps.webhookUrl + ? 'This must be an absolute https:// URL — submissions to this form won\'t be delivered until it is.' + : 'Enter the https:// URL to POST submissions to — this form won\'t deliver anything until you do.'} +

+ )}

Must be an absolute https:// URL. Each submission is POSTed as JSON; failures are retried, then emailed to the fallback address below. @@ -286,6 +337,10 @@ export const FormStylePanel: React.FC = ({ selectedId, nodeProp setProp('webhookSecretId', id)} onRemoved={() => setProp('webhookSecretId', '')} diff --git a/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx b/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx index c3fde75..f04105d 100644 --- a/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx +++ b/craft/src/panels/right/styles/FormStylePanel.webhook.test.tsx @@ -17,7 +17,7 @@ vi.mock('@craftjs/core', () => ({ useEditor: () => ({ actions: { setProp: setPropSpy } }), })); -import { FormStylePanel } from './FormStylePanel'; +import { FormStylePanel, isHttpsWebhookUrl } from './FormStylePanel'; const RAW_SECRET = 'hunter2-SUPER-SECRET-VALUE'; @@ -197,7 +197,7 @@ describe('FormStylePanel webhook secret field is WRITE-ONLY', () => { expect(JSON.stringify(lastProps)).not.toContain(RAW_SECRET); }); - test('the stored secret is never shown -- only its state, with a Remove action', () => { + test('the stored secret is never shown -- only its state, with a Clear action', () => { lastProps = contactFormProps({ destinationType: 'webhook', webhookSecretId: 'whs_42_deadbeefdeadbeef' }); render(); expect(testid('webhook-secret-status')!.textContent).toContain('Secret stored'); @@ -219,4 +219,105 @@ describe('FormStylePanel webhook secret field is WRITE-ONLY', () => { await blur(testid('webhook-secret-input')!); expect(fetchMock).not.toHaveBeenCalled(); }); + + test('the Clear action says what it does: the stored key stays on the server', () => { + lastProps = contactFormProps({ destinationType: 'webhook', webhookSecretId: 'whs_42_deadbeefdeadbeef' }); + render(); + // "Remove" would be a lie: nothing deletes the key file, and it keeps + // counting toward the per-site cap the customer eventually 429s against. + expect(testid('webhook-secret-remove')!.textContent).toBe('Clear'); + expect(container.textContent).toContain('the stored key stays on the server'); + }); +}); + +describe('FormStylePanel: a retained secret draft never follows the selection to another form', () => { + /* GuidedStyles renders with no key, so a selection change + re-renders rather than remounts. A failed store deliberately KEEPS the + draft, so without a remount the next form's field would open holding the + previous form's raw secret -- and the next blur would POST it and assign + the returned id to the wrong node. */ + test('after a failed store on node A, selecting node B shows an empty field and no status', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, status: 429, + json: async () => ({ success: false, error: 'Too many webhook secrets stored for this site — please contact support.' }), + })); + + const nodeA = contactFormProps({ destinationType: 'webhook', webhookUrl: 'https://a.example/x' }); + lastProps = nodeA; + render(); + const input = testid('webhook-secret-input') as HTMLInputElement; + setValue(input, RAW_SECRET); + await blur(input); + + // Precondition: the draft really was retained on node A (otherwise this + // test would pass for the wrong reason). + expect((testid('webhook-secret-input') as HTMLInputElement).value).toBe(RAW_SECRET); + expect(testid('webhook-secret-error')).toBeTruthy(); + + const nodeB = contactFormProps({ destinationType: 'webhook', webhookUrl: 'https://b.example/y' }); + lastProps = nodeB; + rerender(); + + expect((testid('webhook-secret-input') as HTMLInputElement).value).toBe(''); + expect(container.textContent).not.toContain(RAW_SECRET); + expect(testid('webhook-secret-error')).toBeNull(); + }); + + test('a "Secret stored." banner does not follow the selection either', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, status: 200, + json: async () => ({ success: true, secret_id: 'whs_42_deadbeefdeadbeef' }), + })); + + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + const input = testid('webhook-secret-input') as HTMLInputElement; + setValue(input, RAW_SECRET); + await blur(input); + expect(testid('webhook-secret-saved')).toBeTruthy(); + + const nodeB = contactFormProps({ destinationType: 'webhook' }); + lastProps = nodeB; + rerender(); + expect(testid('webhook-secret-saved')).toBeNull(); + expect(testid('webhook-secret-status')).toBeNull(); + }); +}); + +describe('isHttpsWebhookUrl / the inline URL warning', () => { + test.each([ + ['https://hooks.example.com/x', true], + ['https://hooks.example.com/x?a=1&b=2', true], + ['http://hooks.example.com/x', false], + ['hooks.example.com/x', false], + ['/relative/path', false], + ['', false], + [' ', false], + ['https://hooks.example.com/x\nHost: evil', false], + ['javascript:alert(1)', false], + ])('%s -> %s', (value, expected) => { + expect(isHttpsWebhookUrl(value)).toBe(expected); + }); + + test('a blank URL warns that nothing will be delivered', () => { + lastProps = contactFormProps({ destinationType: 'webhook' }); + render(); + expect(testid('webhook-url-warning')!.textContent).toContain("won't deliver anything"); + }); + + test('a non-https URL warns without altering the value', () => { + lastProps = contactFormProps({ destinationType: 'webhook', webhookUrl: 'http://hooks.example.com/x' }); + render(); + expect(testid('webhook-url-warning')!.textContent).toContain('absolute https:// URL'); + // Warning only -- the panel must not blank or rewrite the prop, which would + // turn the publish step's loud refusal into a silently inert form. + expect(lastProps.webhookUrl).toBe('http://hooks.example.com/x'); + expect((testid('webhook-url') as HTMLInputElement).value).toBe('http://hooks.example.com/x'); + }); + + test('a valid https URL shows no warning', () => { + lastProps = contactFormProps({ destinationType: 'webhook', webhookUrl: 'https://hooks.example.com/x' }); + render(); + expect(testid('webhook-url-warning')).toBeNull(); + }); }); -- 2.52.0