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) <noreply@anthropic.com>
This commit is contained in:
@@ -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 =
|
||||
/^<!--WHP-FORM id="([^"]+)"((?: [a-z]+="[^"]*")*) recipient="([^"]*)" thankyou="([^"]*)"-->$/;
|
||||
|
||||
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(/^<!--WHP-FORM id="F_[0-9a-z]+" recipient="a@b\.com" thankyou="\/thx"-->$/);
|
||||
});
|
||||
|
||||
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"b<c>d\'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"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<T extends string>(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:
|
||||
*
|
||||
* /<!--WHP-FORM id="([^"]+)"((?: [a-z]+="[^"]*")*) recipient="([^"]*)" thankyou="([^"]*)"-->/
|
||||
*
|
||||
* (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: `<!--WHP-FORM id="${fid}" recipient="${escapeAttr(recipientEmail)}" thankyou="${escapeAttr(thankYouUrl || '')}"-->`,
|
||||
marker: `<!--WHP-FORM id="${fid}"${extraAttrs} recipient="${escapeAttr(recipientEmail || '')}" thankyou="${escapeAttr(thankYouUrl || '')}"-->`,
|
||||
actionAttr: `__WHP_FORM_ACTION__${fid}__`,
|
||||
honeypot: `<input type="text" name="_gotcha" tabindex="-1" autocomplete="off" style="position:absolute;left:-9999px" aria-hidden="true">`,
|
||||
};
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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_<siteId>_<hex>`. */
|
||||
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<StoreSecretResult> {
|
||||
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.' };
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user