From ca54f325bd0954ea41640b241da256c63546d3ee Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:03:19 +0300 Subject: [PATCH] fix: Validate Send and Wait custom forms before sending (#35262) --- packages/nodes-base/nodes/Form/utils/utils.ts | 41 ++++-- .../utils/sendAndWait/test/util.test.ts | 129 ++++++++++++++++++ .../nodes-base/utils/sendAndWait/utils.ts | 21 +++ 3 files changed, 181 insertions(+), 10 deletions(-) diff --git a/packages/nodes-base/nodes/Form/utils/utils.ts b/packages/nodes-base/nodes/Form/utils/utils.ts index da53b6819e0..774c42575f3 100644 --- a/packages/nodes-base/nodes/Form/utils/utils.ts +++ b/packages/nodes-base/nodes/Form/utils/utils.ts @@ -1,4 +1,5 @@ import { Container } from '@n8n/di'; +import { ensureError } from '@n8n/utils/errors/ensure-error'; import type { Request, Response } from 'express'; import { rm } from 'fs/promises'; import isbot from 'isbot'; @@ -1105,7 +1106,9 @@ export async function formWebhook( }; } -export function resolveRawData(context: IWebhookFunctions, rawData: string) { +type ExpressionResolutionContext = Pick; + +export function resolveRawData(context: ExpressionResolutionContext, rawData: string) { if (!rawData) return rawData; const resolvables = getResolvables(rawData); @@ -1136,21 +1139,16 @@ type ParseFormFieldsOptions = { fieldsParameterName: string; mode?: 'test' | 'production'; }; + export function parseFormFields(context: IWebhookFunctions, options: ParseFormFieldsOptions) { let fields: FormFieldsParameter = []; if (options.defineForm === 'json') { - try { - const jsonOutput = context.getNodeParameter(options.fieldsParameterName, '', { + const getJsonOutput = () => + context.getNodeParameter(options.fieldsParameterName, '', { rawExpressions: true, }) as string; - fields = tryToParseJsonToFormFields(resolveRawData(context, jsonOutput)); - } catch (error) { - throw new NodeOperationError(context.getNode(), error.message, { - description: error.message, - type: options.mode === 'test' ? 'manual-form-test' : undefined, - }); - } + fields = parseJsonFormFields(context, getJsonOutput, options.mode); } else { fields = context.getNodeParameter(options.fieldsParameterName, []) as FormFieldsParameter; for (const field of fields) { @@ -1165,3 +1163,26 @@ export function parseFormFields(context: IWebhookFunctions, options: ParseFormFi } return fields; } + +type ParseJsonFormFieldsCtx = Pick; + +/** + * @throws {NodeOperationError} if the JSON is invalid or cannot be parsed into form fields + */ +export function parseJsonFormFields( + context: ParseJsonFormFieldsCtx, + getJsonOutput: () => string, + mode?: 'test' | 'production', +) { + try { + const jsonOutput = getJsonOutput(); + + return tryToParseJsonToFormFields(resolveRawData(context, jsonOutput)); + } catch (e) { + const error = ensureError(e); + throw new NodeOperationError(context.getNode(), error.message, { + description: error.message, + type: mode === 'test' ? 'manual-form-test' : undefined, + }); + } +} diff --git a/packages/nodes-base/utils/sendAndWait/test/util.test.ts b/packages/nodes-base/utils/sendAndWait/test/util.test.ts index 5e7f5fea73d..774c1c8a139 100644 --- a/packages/nodes-base/utils/sendAndWait/test/util.test.ts +++ b/packages/nodes-base/utils/sendAndWait/test/util.test.ts @@ -176,6 +176,91 @@ describe('Send and Wait utils tests', () => { ]), ); }); + + describe('customForm validation', () => { + const mockCustomFormParameters = (params: { [key: string]: any }) => + mockExecuteFunctions.getNodeParameter.mockImplementation( + (parameterName: string) => + ({ + message: 'Pick a customer', + responseType: 'customForm', + ...params, + })[parameterName], + ); + + it('should throw when the form JSON resolves to invalid form fields', () => { + mockExecuteFunctions.getNode.mockReturnValue({ name: 'Send Email' } as any); + mockCustomFormParameters({ + defineForm: 'json', + jsonOutput: '={{ JSON.stringify($json.formFields) }}', + }); + // A customer row without a name resolves to `{ option: null }` + mockExecuteFunctions.evaluateExpression.mockReturnValue([ + { + fieldLabel: 'Customer', + fieldType: 'dropdown', + fieldOptions: { values: [{ option: null }] }, + }, + ] as any); + + expect(() => getSendAndWaitConfig(mockExecuteFunctions)).toThrow( + 'Field dropdown in field 0 has an invalid option 0', + ); + }); + + it('should return the config when the form JSON resolves to valid form fields', () => { + mockCustomFormParameters({ + defineForm: 'json', + jsonOutput: '={{ JSON.stringify($json.formFields) }}', + 'options.messageButtonLabel': 'Respond', + }); + mockExecuteFunctions.evaluateExpression.mockReturnValue([ + { + fieldLabel: 'Customer', + fieldType: 'dropdown', + fieldOptions: { values: [{ option: 'Acme Corp' }] }, + }, + ] as any); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); + + const config = getSendAndWaitConfig(mockExecuteFunctions); + + expect(config.options).toEqual([ + { + label: 'Respond', + style: 'primary', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + approved: true, + }, + ]); + }); + + const readParameterNames = () => + mockExecuteFunctions.getNodeParameter.mock.calls.map(([name]) => name); + + it('should not read the form JSON when the form is defined with fields', () => { + mockCustomFormParameters({ defineForm: 'fields' }); + + getSendAndWaitConfig(mockExecuteFunctions); + + expect(readParameterNames()).not.toContain('jsonOutput'); + }); + + it('should not read the form JSON for approval response types', () => { + mockCustomFormParameters({ + responseType: 'approval', + 'approvalOptions.values': { approvalType: 'single' }, + defineForm: 'json', + jsonOutput: '={{ JSON.stringify($json.formFields) }}', + }); + + getSendAndWaitConfig(mockExecuteFunctions); + + expect(readParameterNames()).not.toContain('jsonOutput'); + }); + }); }); describe('createEmail', () => { @@ -450,6 +535,50 @@ describe('Send and Wait utils tests', () => { ); }); + // Form fields are re-resolved from upstream data when the form is rendered, so an execution + // that is already waiting with bad data still breaks the read-only GET. Pins the current + // behaviour; unmasking the reason at render time is a separate follow-up. + it('should throw when a data-driven customForm resolves to invalid form fields', async () => { + mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'GET' } as any); + mockWebhookFunctions.getResponseObject.mockReturnValue({ + render: vi.fn(), + setHeader: vi.fn(), + } as any); + mockWebhookFunctions.getNode.mockReturnValue({ name: 'Dropdown' } as any); + + // A customer row without a name resolves to `{ option: null }` + mockWebhookFunctions.evaluateExpression.mockReturnValue([ + { + fieldLabel: 'Customer', + fieldType: 'dropdown', + fieldOptions: { values: [{ option: 'Acme Corp' }, { option: null }] }, + }, + ] as any); + + mockWebhookFunctions.getNodeParameter.mockImplementation((parameterName: string) => { + const params: { [key: string]: any } = { + responseType: 'customForm', + message: 'Pick a customer', + defineForm: 'json', + jsonOutput: '={{ JSON.stringify($json.formFields) }}', + options: {}, + }; + return params[parameterName]; + }); + + const error = await sendAndWaitWebhook + .call(mockWebhookFunctions) + .catch((e: NodeOperationError) => e); + + expect(error).toBeInstanceOf(NodeOperationError); + expect((error as NodeOperationError).message).toBe( + 'Field dropdown in field 0 has an invalid option 1', + ); + // Without `type: 'manual-form-test'` the reason is masked as + // "Workflow Webhook Error: Workflow could not be started!" by webhook-helpers + expect((error as NodeOperationError).type).toBeUndefined(); + }); + it('should handle customForm POST webhook', async () => { mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'POST', diff --git a/packages/nodes-base/utils/sendAndWait/utils.ts b/packages/nodes-base/utils/sendAndWait/utils.ts index 8c559395a1e..96de977a496 100644 --- a/packages/nodes-base/utils/sendAndWait/utils.ts +++ b/packages/nodes-base/utils/sendAndWait/utils.ts @@ -13,6 +13,7 @@ import { cssVariables } from '../../nodes/Form/cssVariables'; import { formFieldsProperties } from '../../nodes/Form/Form.node'; import { parseFormFields, + parseJsonFormFields, prepareFormData, prepareFormFields, prepareFormReturnItem, @@ -493,6 +494,22 @@ export async function sendAndWaitWebhook(this: IWebhookFunctions) { } // Send and Wait Config ----------------------------------------------------------- + +// The response form is only built when it is requested, from data that may no longer resolve +// by then. Parse it here, exactly as the webhook will, so a form that cannot be built fails +// the node instead of sending a message with a link that can never render. +function validateCustomFormFields(context: IExecuteFunctions) { + const defineForm = context.getNodeParameter('defineForm', 0, 'fields') as 'fields' | 'json'; + // The 'fields' branch has nothing that needs to be validated + if (defineForm !== 'json') return; + + const getJsonOutput = () => + context.getNodeParameter('jsonOutput', 0, '', { + rawExpressions: true, + }) as string; + parseJsonFormFields(context, getJsonOutput); +} + export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitConfig { const message = escapeHtml((context.getNodeParameter('message', 0, '') as string).trim()) .replace(/\\n/g, '\n') @@ -517,6 +534,10 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon const responseType = context.getNodeParameter('responseType', 0, 'approval') as string; + if (responseType === 'customForm') { + validateCustomFormFields(context); + } + const approvedSignedResumeUrl = context.getSignedResumeUrl({ approved: 'true' }); if (responseType === 'freeText' || responseType === 'customForm') {