diff --git a/packages/@n8n/api-types/src/dto/api-keys/update-api-key-request.dto.ts b/packages/@n8n/api-types/src/dto/api-keys/update-api-key-request.dto.ts index 31b52659564..b123b718ed1 100644 --- a/packages/@n8n/api-types/src/dto/api-keys/update-api-key-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/api-keys/update-api-key-request.dto.ts @@ -1,15 +1,9 @@ -import xss from 'xss'; import { z } from 'zod'; import { scopesSchema } from '../../schemas/scopes.schema'; +import { xssCheck } from '../../utils/xss-check'; import { Z } from '../../zod-class'; -const xssCheck = (value: string) => - value === - xss(value, { - whiteList: {}, - }); - export class UpdateApiKeyRequestDto extends Z.class({ label: z.string().max(50).min(1).refine(xssCheck), scopes: scopesSchema, diff --git a/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts b/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts index abc044a8f42..776dac21563 100644 --- a/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/user/user-update-request.dto.ts @@ -1,14 +1,8 @@ -import xss from 'xss'; import { z } from 'zod'; +import { xssCheck } from '../../utils/xss-check'; import { Z } from '../../zod-class'; -const xssCheck = (value: string) => - value === - xss(value, { - whiteList: {}, // no tags are allowed - }); - const URL_REGEX = /^(https?:\/\/|www\.)|(\.[\p{L}\d-]+)/iu; const urlCheck = (value: string) => !URL_REGEX.test(value); diff --git a/packages/@n8n/api-types/src/dto/workflows/__tests__/create-workflow.dto.test.ts b/packages/@n8n/api-types/src/dto/workflows/__tests__/create-workflow.dto.test.ts index da04c53152f..52dfa0368d9 100644 --- a/packages/@n8n/api-types/src/dto/workflows/__tests__/create-workflow.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/workflows/__tests__/create-workflow.dto.test.ts @@ -108,6 +108,21 @@ describe('CreateWorkflowDto', () => { request: { name: 'a'.repeat(129), nodes: [], connections: {} }, expectedErrorPath: ['name'], }, + { + name: 'name containing a script tag', + request: { name: '', nodes: [], connections: {} }, + expectedErrorPath: ['name'], + }, + { + name: 'name containing an img onerror payload', + request: { name: '', nodes: [], connections: {} }, + expectedErrorPath: ['name'], + }, + { + name: 'name containing inline HTML markup', + request: { name: 'Report bold', nodes: [], connections: {} }, + expectedErrorPath: ['name'], + }, { name: 'missing nodes', request: { name: 'Test', connections: {} }, diff --git a/packages/@n8n/api-types/src/dto/workflows/__tests__/update-workflow.dto.test.ts b/packages/@n8n/api-types/src/dto/workflows/__tests__/update-workflow.dto.test.ts index 50fcaf0c57b..cf3b20027b8 100644 --- a/packages/@n8n/api-types/src/dto/workflows/__tests__/update-workflow.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/workflows/__tests__/update-workflow.dto.test.ts @@ -88,6 +88,21 @@ describe('UpdateWorkflowDto', () => { request: { name: 'a'.repeat(129) }, expectedErrorPath: ['name'], }, + { + name: 'name containing a script tag', + request: { name: '' }, + expectedErrorPath: ['name'], + }, + { + name: 'name containing an img onerror payload', + request: { name: '' }, + expectedErrorPath: ['name'], + }, + { + name: 'name containing inline HTML markup', + request: { name: 'Report bold' }, + expectedErrorPath: ['name'], + }, { name: 'invalid nodes type', request: { nodes: 'not-an-array' }, diff --git a/packages/@n8n/api-types/src/dto/workflows/base-workflow.dto.ts b/packages/@n8n/api-types/src/dto/workflows/base-workflow.dto.ts index 71145354258..9c32f1ff079 100644 --- a/packages/@n8n/api-types/src/dto/workflows/base-workflow.dto.ts +++ b/packages/@n8n/api-types/src/dto/workflows/base-workflow.dto.ts @@ -1,6 +1,8 @@ import type { IPinData, IConnections, IDataObject, INode, IWorkflowSettings } from 'n8n-workflow'; import { z } from 'zod'; +import { xssCheck } from '../../utils/xss-check'; + export const WORKFLOW_NAME_MAX_LENGTH = 128; /** Maximum allowed size for pinned data in bytes (12 MB) */ @@ -17,7 +19,8 @@ export const workflowNameSchema = z .min(1, { message: 'Workflow name is required' }) .max(WORKFLOW_NAME_MAX_LENGTH, { message: `Workflow name must be ${WORKFLOW_NAME_MAX_LENGTH} characters or less`, - }); + }) + .refine(xssCheck, { message: 'Potentially malicious string' }); export const workflowDescriptionSchema = z.string().nullable(); diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 9c43bb4431f..882311e10d7 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -438,6 +438,7 @@ export { } from './schemas/eval-collections.schema'; export { ALLOWED_DOMAINS, isAllowedDomain } from './utils/allowed-domains'; +export { xssCheck } from './utils/xss-check'; export type { PublishTimelineEvent } from './schemas/workflow-publish-timeline.schema'; export { diff --git a/packages/@n8n/api-types/src/schemas/__tests__/data-table.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/data-table.schema.test.ts new file mode 100644 index 00000000000..b08af46bf85 --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/__tests__/data-table.schema.test.ts @@ -0,0 +1,36 @@ +import { dataTableNameSchema } from '../data-table.schema'; + +describe('dataTableNameSchema', () => { + describe('Valid names', () => { + test.each([ + 'Customers', + 'Customer orders 2024', + 'orders-q1', + "Q1 'Quarterly' Report", + 'orders & invoices', + 'a', + ])('accepts %p', (value) => { + expect(dataTableNameSchema.safeParse(value).success).toBe(true); + }); + + test('trims surrounding whitespace', () => { + const result = dataTableNameSchema.safeParse(' Customers '); + expect(result.success).toBe(true); + expect(result.data).toBe('Customers'); + }); + }); + + describe('Invalid names', () => { + test.each([ + ['empty string', ''], + ['only whitespace', ' '], + ['too long', 'a'.repeat(129)], + ['contains a script tag', ''], + ['contains an img onerror payload', ''], + ['contains inline HTML markup', 'Customers bold'], + ['contains an svg onload payload', ''], + ])('rejects %s', (_label, value) => { + expect(dataTableNameSchema.safeParse(value).success).toBe(false); + }); + }); +}); diff --git a/packages/@n8n/api-types/src/schemas/data-table.schema.ts b/packages/@n8n/api-types/src/schemas/data-table.schema.ts index c8c10c9d451..f1d71e875e2 100644 --- a/packages/@n8n/api-types/src/schemas/data-table.schema.ts +++ b/packages/@n8n/api-types/src/schemas/data-table.schema.ts @@ -1,10 +1,16 @@ import { z } from 'zod'; import type { ListDataTableQueryDto } from '../dto'; +import { xssCheck } from '../utils/xss-check'; export const insertRowReturnType = z.union([z.literal('all'), z.literal('count'), z.literal('id')]); -export const dataTableNameSchema = z.string().trim().min(1).max(128); +export const dataTableNameSchema = z + .string() + .trim() + .min(1) + .max(128) + .refine(xssCheck, { message: 'Potentially malicious string' }); export const dataTableIdSchema = z .string() .max(36) diff --git a/packages/@n8n/api-types/src/utils/__tests__/xss-check.test.ts b/packages/@n8n/api-types/src/utils/__tests__/xss-check.test.ts new file mode 100644 index 00000000000..4b0124affed --- /dev/null +++ b/packages/@n8n/api-types/src/utils/__tests__/xss-check.test.ts @@ -0,0 +1,37 @@ +import { xssCheck } from '../xss-check'; + +describe('xssCheck', () => { + test.each([ + 'My Workflow', + 'My Workflow 2024', + 'Workflow with spaces and 123 numbers', + "O'Brien's workflow", + 'workflow & report', + 'a', + 'name-with-dashes_and.dots', + 'name/with/slashes', + 'name (with) (parens)', + ])('returns true for plain string %p', (value) => { + expect(xssCheck(value)).toBe(true); + }); + + test.each([ + '', + '', + '', + 'click', + '', + 'Name with bold', + '', + '', + '', + '7 > 3 is true', + '< not really a tag', + ])('returns false for value containing HTML-significant characters %p', (value) => { + expect(xssCheck(value)).toBe(false); + }); + + test('returns true for empty string', () => { + expect(xssCheck('')).toBe(true); + }); +}); diff --git a/packages/@n8n/api-types/src/utils/xss-check.ts b/packages/@n8n/api-types/src/utils/xss-check.ts new file mode 100644 index 00000000000..e2b91fb21a2 --- /dev/null +++ b/packages/@n8n/api-types/src/utils/xss-check.ts @@ -0,0 +1,10 @@ +import xss from 'xss'; + +/** + * Returns `true` when the value is preserved by `xss({ whiteList: {} })`, + * i.e. contains no HTML-significant characters. + * + * Use as a zod refine guard for user-supplied names (workflows, data tables, + * user names, API keys, etc.). + */ +export const xssCheck = (value: string): boolean => value === xss(value, { whiteList: {} }); diff --git a/packages/@n8n/db/src/entities/workflow-entity.ts b/packages/@n8n/db/src/entities/workflow-entity.ts index 619f04db3c4..6b0a7ef0791 100644 --- a/packages/@n8n/db/src/entities/workflow-entity.ts +++ b/packages/@n8n/db/src/entities/workflow-entity.ts @@ -24,7 +24,6 @@ import { objectRetriever, sqlite } from '../utils/transformers'; @Entity() export class WorkflowEntity extends WithTimestampsAndStringId implements IWorkflowDb { - // TODO: Add XSS check @Index({ unique: true }) @Length(1, 128, { message: 'Workflow name must be $constraint1 to $constraint2 characters long.',