Skip to content

Commit 2481969

Browse files
n8n-assistant[bot]DawidMyslak
n8n-assistant[bot]
and
DawidMyslak
authored
fix(Form Node): Improve form rendering consistency (backport to release-candidate/2.11.x) (#26652)
Co-authored-by: Dawid Myslak <dawid.myslak@gmail.com>
1 parent 2034f5f commit 2481969

File tree

9 files changed

+93
-47
lines changed

9 files changed

+93
-47
lines changed

‎packages/core/src/__tests__/html-sandbox.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('getWebhookSandboxCSP', () => {
2929
it('should return correct CSP sandbox directive', () => {
3030
const csp = getWebhookSandboxCSP();
3131
expect(csp).toBe(
32-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
32+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
3333
);
3434
});
3535

‎packages/core/src/html-sandbox.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ export const isWebhookHtmlSandboxingDisabled = () => {
99
* Returns the CSP header value that sandboxes the HTML page into a separate origin.
1010
*/
1111
export const getWebhookSandboxCSP = (): string => {
12-
return 'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
12+
return 'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
1313
};

‎packages/nodes-base/nodes/Form/test/formCompletionUtils.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ describe('formCompletionUtils', () => {
329329

330330
expect(mockResponse.setHeader).toHaveBeenCalledWith(
331331
'Content-Security-Policy',
332-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
332+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
333333
);
334334
expect(mockResponse.render).toHaveBeenCalled();
335335
});

‎packages/nodes-base/nodes/Form/test/formNodeUtils.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,50 @@ describe('formNodeUtils', () => {
122122
});
123123
});
124124

125+
it('should sanitize formDescription', async () => {
126+
webhookFunctions.getNode.mockReturnValue({ typeVersion: 2.1 } as any);
127+
128+
const testCases = [
129+
{
130+
description: '<script>alert("hello world")</script>',
131+
expected: '',
132+
},
133+
{
134+
description: '<i>hello</i>',
135+
expected: '<i>hello</i>',
136+
},
137+
{
138+
description: 'Plain text description',
139+
expected: 'Plain text description',
140+
},
141+
{
142+
description: '<style>body { display: none; }</style><b>visible</b>',
143+
expected: '<b>visible</b>',
144+
},
145+
];
146+
147+
const formFields: FormFieldsParameter = [];
148+
const triggerMock = mock<NodeTypeAndVersion>({ name: 'triggerName' } as any);
149+
150+
for (const { description, expected } of testCases) {
151+
webhookFunctions.getNodeParameter.calledWith('options').mockReturnValue({
152+
formTitle: 'Test Title',
153+
formDescription: description,
154+
buttonLabel: 'Submit',
155+
});
156+
157+
const mockRender = jest.fn();
158+
const res = mock<Response>({ render: mockRender } as any);
159+
160+
await renderFormNode(webhookFunctions, res, triggerMock, formFields, 'test');
161+
162+
expect(mockRender).toHaveBeenCalledWith(
163+
'form-trigger',
164+
expect.objectContaining({ formDescription: expected }),
165+
);
166+
}
167+
});
168+
125169
describe('getFormTriggerNode', () => {
126170
const mockCurrentNode = { name: 'currentNode' };
127171

‎packages/nodes-base/nodes/Form/test/utils.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ describe('FormTrigger, formWebhook', () => {
703703

704704
expect(mockSetHeader).toHaveBeenCalledWith(
705705
'Content-Security-Policy',
706-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
706+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
707707
);
708708
});
709709

‎packages/nodes-base/nodes/Form/utils/formCompletionUtils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { handleNewlines, sanitizeCustomCss, sanitizeHtml, validateSafeRedirectUrl } from './utils';
1212

1313
const SANDBOX_CSP =
14-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
14+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
1515

1616
const getBinaryDataFromNode = (context: IWebhookFunctions, nodeName: string): IDataObject => {
1717
return context.evaluateExpression(`{{ $('${nodeName}').first().binary }}`) as IDataObject;

‎packages/nodes-base/nodes/Form/utils/formNodeUtils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
FORM_TRIGGER_NODE_TYPE,
99
} from 'n8n-workflow';
1010

11-
import { renderForm } from './utils';
11+
import { handleNewlines, renderForm, sanitizeHtml } from './utils';
1212

1313
export const renderFormNode = async (
1414
context: IWebhookFunctions,
@@ -29,7 +29,7 @@ export const renderFormNode = async (
2929
title = context.evaluateExpression(`{{ $('${trigger?.name}').params.formTitle }}`) as string;
3030
}
3131

32-
const description = options.formDescription ?? '';
32+
const description = handleNewlines(sanitizeHtml(options.formDescription ?? ''));
3333

3434
let buttonLabel = options.buttonLabel;
3535
if (!buttonLabel) {

‎packages/nodes-base/utils/sendAndWait/test/util.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ describe('Send and Wait utils tests', () => {
278278

279279
expect(mockSetHeader).toHaveBeenCalledWith(
280280
'Content-Security-Policy',
281-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
281+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
282282
);
283283

284284
expect(mockRender).toHaveBeenCalledWith('form-trigger', {
@@ -364,7 +364,7 @@ describe('Send and Wait utils tests', () => {
364364

365365
expect(mockSetHeader).toHaveBeenCalledWith(
366366
'Content-Security-Policy',
367-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
367+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols',
368368
);
369369

370370
expect(mockRender).toHaveBeenCalledWith('form-trigger', {

‎packages/testing/playwright/tests/e2e/api/webhook-isolation.spec.ts‎

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,47 @@
11
import { test, expect } from '../../../fixtures/base';
22

3-
test.describe('Webhook Origin Isolation', {
4-
annotation: [
5-
{ type: 'owner', description: 'Catalysts' },
6-
],
7-
}, () => {
8-
test.beforeAll(async ({ api }) => {
9-
await api.workflows.importWorkflowFromFile('webhook-origin-isolation.json', {
10-
makeUnique: false,
3+
test.describe(
4+
'Webhook Origin Isolation',
5+
{
6+
annotation: [{ type: 'owner', description: 'Catalysts' }],
7+
},
8+
() => {
9+
test.beforeAll(async ({ api }) => {
10+
await api.workflows.importWorkflowFromFile('webhook-origin-isolation.json', {
11+
makeUnique: false,
12+
});
1113
});
12-
});
1314

14-
const webhookPaths = [
15-
'webhook-response-data-text-html',
16-
'webhook-response-data-wo-content-type',
17-
'webhook-last-node-no-content-type-header',
18-
'webhook-last-node-text-html-header',
19-
'webhook-last-node-text-html-content-type',
20-
'webhook-response-data-csp-header',
21-
'webhook-last-node-csp-header',
22-
'webhook-last-node-binary-text-html',
23-
'webhook-last-node-binary-no-content-type',
24-
'webhook-last-node-binary-csp-header',
25-
'respond-to-webhook-text-no-content-type',
26-
'respond-to-webhook-text-content-type-text-html',
27-
'respond-to-webhook-text-csp-header',
28-
'respond-to-webhook-json-as-text-html',
29-
];
15+
const webhookPaths = [
16+
'webhook-response-data-text-html',
17+
'webhook-response-data-wo-content-type',
18+
'webhook-last-node-no-content-type-header',
19+
'webhook-last-node-text-html-header',
20+
'webhook-last-node-text-html-content-type',
21+
'webhook-response-data-csp-header',
22+
'webhook-last-node-csp-header',
23+
'webhook-last-node-binary-text-html',
24+
'webhook-last-node-binary-no-content-type',
25+
'webhook-last-node-binary-csp-header',
26+
'respond-to-webhook-text-no-content-type',
27+
'respond-to-webhook-text-content-type-text-html',
28+
'respond-to-webhook-text-csp-header',
29+
'respond-to-webhook-json-as-text-html',
30+
];
3031

31-
const expectedCSP =
32-
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
32+
const expectedCSP =
33+
'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols';
3334

34-
for (const webhookPath of webhookPaths) {
35-
test(`Webhook responses should include the correct response headers for ${webhookPath}`, async ({
36-
api,
37-
}) => {
38-
const webhookResponse = await api.webhooks.trigger(`/webhook/${webhookPath}`);
39-
expect(webhookResponse.ok()).toBe(true);
35+
for (const webhookPath of webhookPaths) {
36+
test(`Webhook responses should include the correct response headers for ${webhookPath}`, async ({
37+
api,
38+
}) => {
39+
const webhookResponse = await api.webhooks.trigger(`/webhook/${webhookPath}`);
40+
expect(webhookResponse.ok()).toBe(true);
4041

41-
const headers = webhookResponse.headers();
42-
expect(headers['content-security-policy']).toBe(expectedCSP);
43-
});
44-
}
45-
});
42+
const headers = webhookResponse.headers();
43+
expect(headers['content-security-policy']).toBe(expectedCSP);
44+
});
45+
}
46+
},
47+
);

0 commit comments

Comments
 (0)