fix: Validate Send and Wait custom forms before sending (#35262)

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<IWebhookFunctions, 'evaluateExpression'>;
+
+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<IWebhookFunctions, 'evaluateExpression' | 'getNode'>;
+
+/**
+ * @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,
+		});
+	}
+}

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',

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') {