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 b123b718ed1..31b52659564 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,9 +1,15 @@ +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 776dac21563..abc044a8f42 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,8 +1,14 @@ +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 afce03131d3..eef51e3377d 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 @@ -126,21 +126,6 @@ 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 9d5b81d2487..5586a708fc8 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 @@ -98,21 +98,6 @@ 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 ded5e9ea82b..69af6161220 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,8 +1,6 @@ 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) */ @@ -19,8 +17,7 @@ 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 68f248576bf..afaeb273a7b 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -457,7 +457,6 @@ export { } from './schemas/eval-insights.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 deleted file mode 100644 index b08af46bf85..00000000000 --- a/packages/@n8n/api-types/src/schemas/__tests__/data-table.schema.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -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 f1d71e875e2..c8c10c9d451 100644 --- a/packages/@n8n/api-types/src/schemas/data-table.schema.ts +++ b/packages/@n8n/api-types/src/schemas/data-table.schema.ts @@ -1,16 +1,10 @@ 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) - .refine(xssCheck, { message: 'Potentially malicious string' }); +export const dataTableNameSchema = z.string().trim().min(1).max(128); 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 deleted file mode 100644 index 4b0124affed..00000000000 --- a/packages/@n8n/api-types/src/utils/__tests__/xss-check.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -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 deleted file mode 100644 index e2b91fb21a2..00000000000 --- a/packages/@n8n/api-types/src/utils/xss-check.ts +++ /dev/null @@ -1,10 +0,0 @@ -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 ba180d668f9..9bf2e89e4fe 100644 --- a/packages/@n8n/db/src/entities/workflow-entity.ts +++ b/packages/@n8n/db/src/entities/workflow-entity.ts @@ -24,6 +24,7 @@ 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.',