From a3e37fcd1216231e1d8fb0a39e051f229ca10bc0 Mon Sep 17 00:00:00 2001 From: Thomas Shellberg <6723003+tommyshellberg@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:23:23 +0200 Subject: [PATCH] fix(core): Prevent isUniqueConstraintError false positives (#31284) Co-authored-by: SAAKSHI GUPTA Co-authored-by: Claude Opus 4.7 (1M context) --- .../cli/src/__tests__/response-helper.test.ts | 77 ++++++++++++++++++- packages/cli/src/response-helper.ts | 25 +++++- .../secret-providers-connections.api.test.ts | 2 +- 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/__tests__/response-helper.test.ts b/packages/cli/src/__tests__/response-helper.test.ts index 1981210c178..1b984da076b 100644 --- a/packages/cli/src/__tests__/response-helper.test.ts +++ b/packages/cli/src/__tests__/response-helper.test.ts @@ -1,8 +1,9 @@ +import { QueryFailedError } from '@n8n/db'; import type { Response } from 'express'; import { mock } from 'jest-mock-extended'; import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error'; -import { sendErrorResponse } from '@/response-helper'; +import { isUniqueConstraintError, sendErrorResponse } from '@/response-helper'; describe('sendErrorResponse', () => { let mockResponse: Response; @@ -51,3 +52,77 @@ describe('sendErrorResponse', () => { ); }); }); + +describe('isUniqueConstraintError', () => { + const makeQueryFailedError = ( + message: string, + driverError: { code?: string } = {}, + ): QueryFailedError => { + return new QueryFailedError('Query', [], Object.assign(new Error(message), driverError)); + }; + + describe('returns true for actual database unique-constraint violations', () => { + it('SQLite extended code SQLITE_CONSTRAINT_UNIQUE', () => { + const error = makeQueryFailedError('UNIQUE constraint failed: workflow_entity.name', { + code: 'SQLITE_CONSTRAINT_UNIQUE', + }); + expect(isUniqueConstraintError(error)).toBe(true); + }); + + it('SQLite base code SQLITE_CONSTRAINT when message mentions UNIQUE constraint', () => { + const error = makeQueryFailedError( + 'SQLITE_CONSTRAINT: UNIQUE constraint failed: workflow_entity.name', + { code: 'SQLITE_CONSTRAINT' }, + ); + expect(isUniqueConstraintError(error)).toBe(true); + }); + + it('PostgreSQL unique_violation (code 23505)', () => { + const error = makeQueryFailedError( + 'duplicate key value violates unique constraint "users_email_key"', + { code: '23505' }, + ); + expect(isUniqueConstraintError(error)).toBe(true); + }); + }); + + describe('returns false for errors that previously triggered false positives (#25012)', () => { + it('plain Error whose message mentions a workflow named "Duplicate Detection"', () => { + const error = new Error( + 'Cannot publish workflow: Node "Execute Workflow" references workflow "Duplicate Detection" which is not published', + ); + expect(isUniqueConstraintError(error)).toBe(false); + }); + + it('plain Error whose message mentions "unique"', () => { + const error = new Error('The value must be unique across all entries'); + expect(isUniqueConstraintError(error)).toBe(false); + }); + + it('plain Error mentioning both trigger words', () => { + const error = new Error('Unique identifier duplicate found in configuration'); + expect(isUniqueConstraintError(error)).toBe(false); + }); + }); + + describe('returns false for unrelated database errors', () => { + it('SQLite base code SQLITE_CONSTRAINT for a NOT NULL violation', () => { + const error = makeQueryFailedError('NOT NULL constraint failed: workflow_entity.name', { + code: 'SQLITE_CONSTRAINT', + }); + expect(isUniqueConstraintError(error)).toBe(false); + }); + + it('PostgreSQL not_null_violation (code 23502)', () => { + const error = makeQueryFailedError('null value in column violates not-null constraint', { + code: '23502', + }); + expect(isUniqueConstraintError(error)).toBe(false); + }); + + it('QueryFailedError with no driver code', () => { + const error = makeQueryFailedError('Connection timeout after 30000ms'); + expect(isUniqueConstraintError(error)).toBe(false); + }); + }); +}); diff --git a/packages/cli/src/response-helper.ts b/packages/cli/src/response-helper.ts index 09b3c84c9f6..a1faec367a9 100644 --- a/packages/cli/src/response-helper.ts +++ b/packages/cli/src/response-helper.ts @@ -1,5 +1,5 @@ import { inDevelopment, Logger } from '@n8n/backend-common'; -import type { User } from '@n8n/db'; +import { QueryFailedError, type User } from '@n8n/db'; import { Container } from '@n8n/di'; import type { ReportingOptions } from '@n8n/errors'; import type { Request, Response } from 'express'; @@ -92,8 +92,27 @@ export function sendErrorResponse(res: Response, error: Error) { res.status(status).json(response); } -export const isUniqueConstraintError = (error: Error) => - ['unique', 'duplicate'].some((s) => error.message.toLowerCase().includes(s)); +export const isUniqueConstraintError = (error: Error): boolean => { + if (!(error instanceof QueryFailedError)) return false; + + // TypeORM types `driverError` as `any`; narrow it via `unknown` so the + // property checks below stay type-safe without an `as` cast. + const driverError: unknown = error.driverError; + if (typeof driverError !== 'object' || driverError === null) return false; + + const code = + 'code' in driverError && typeof driverError.code === 'string' ? driverError.code : undefined; + + // PostgreSQL: 23505 = unique_violation + if (code === '23505') return true; + + // SQLite: extended code is unambiguous; the base code covers all constraint + // kinds (NOT NULL, FK, CHECK, UNIQUE), so disambiguate via the message. + if (code === 'SQLITE_CONSTRAINT_UNIQUE') return true; + if (code === 'SQLITE_CONSTRAINT' && /UNIQUE constraint/i.test(error.message)) return true; + + return false; +}; export function reportError(error: Error, options?: ReportingOptions) { if (!(error instanceof ResponseError) || error.httpStatusCode > 404) { diff --git a/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts b/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts index cc7814af7c4..dfdab17a32a 100644 --- a/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts +++ b/packages/cli/test/integration/external-secrets/secret-providers-connections.api.test.ts @@ -204,7 +204,7 @@ describe('Secret Providers Connections API', () => { .send(payload) .expect(400); - expect(response.body.message).toBe('There is already an entry with this name'); + expect(response.body.message).toBe('Connection with key "duplicateTest" already exists'); }); });