feat(core): Upgrade to express 5 to address CVE-2024-52798 (#14332)

package.json

@@ -98,7 +98,7 @@
       "bull@4.12.1": "patches/bull@4.12.1.patch",
       "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch",
       "pyodide@0.23.4": "patches/pyodide@0.23.4.patch",
-      "@types/express-serve-static-core@4.17.43": "patches/@types__express-serve-static-core@4.17.43.patch",
+      "@types/express-serve-static-core@5.0.6": "patches/@types__express-serve-static-core@5.0.6.patch",
       "@types/ws@8.5.4": "patches/@types__ws@8.5.4.patch",
       "@types/uuencode@0.0.3": "patches/@types__uuencode@0.0.3.patch",
       "vue-tsc@2.2.8": "patches/vue-tsc@2.2.8.patch",

packages/cli/bin/n8n

@@ -38,7 +38,6 @@ Please use a Node.js version that satisfies the following version range: ${suppo
 const { inspect } = require('util');
 inspect.defaultOptions.customInspect = false;
 
-require('express-async-errors');
 require('source-map-support').install();
 require('reflect-metadata');
 

packages/cli/package.json

@@ -57,9 +57,9 @@
     "@redocly/cli": "^1.28.5",
     "@types/aws4": "^1.5.1",
     "@types/bcryptjs": "^2.4.2",
-    "@types/compression": "1.0.1",
+    "@types/compression": "^1.7.5",
     "@types/convict": "^6.1.1",
-    "@types/cookie-parser": "^1.4.7",
+    "@types/cookie-parser": "^1.4.8",
     "@types/express": "catalog:",
     "@types/flat": "^5.0.5",
     "@types/formidable": "^3.4.5",
@@ -111,23 +111,22 @@
     "change-case": "4.1.2",
     "class-transformer": "0.5.1",
     "class-validator": "0.14.0",
-    "compression": "1.7.4",
+    "compression": "1.8.0",
     "convict": "6.2.4",
     "cookie-parser": "1.4.7",
     "csrf": "3.1.0",
     "dotenv": "8.6.0",
-    "express": "4.21.1",
+    "express": "5.1.0",
-    "express-async-errors": "3.1.1",
+    "express-handlebars": "8.0.1",
-    "express-handlebars": "7.1.2",
     "express-openapi-validator": "5.4.7",
-    "express-prom-bundle": "6.6.0",
+    "express-prom-bundle": "8.0.0",
-    "express-rate-limit": "7.2.0",
+    "express-rate-limit": "7.5.0",
     "fast-glob": "catalog:",
     "flat": "5.0.2",
     "flatted": "catalog:",
     "formidable": "3.5.1",
     "handlebars": "4.7.8",
-    "helmet": "7.1.0",
+    "helmet": "8.1.0",
     "infisical-node": "1.3.0",
     "ioredis": "5.3.2",
     "isbot": "3.6.13",
@@ -153,9 +152,9 @@
     "picocolors": "catalog:",
     "pkce-challenge": "3.0.0",
     "posthog-node": "3.2.1",
-    "prom-client": "13.2.0",
+    "prom-client": "15.1.3",
     "psl": "1.9.0",
-    "raw-body": "2.5.1",
+    "raw-body": "3.0.0",
     "reflect-metadata": "catalog:",
     "replacestream": "4.0.3",
     "samlify": "2.9.0",

packages/cli/src/abstract-server.ts

@@ -114,14 +114,17 @@ export abstract class AbstractServer {
 
 	private async setupHealthCheck() {
 		// main health check should not care about DB connections
-		this.app.get('/healthz', async (_req, res) => {
+		this.app.get('/healthz', (_req, res) => {
 			res.send({ status: 'ok' });
 		});
 
-		this.app.get('/healthz/readiness', async (_req, res) => {
+		this.app.get('/healthz/readiness', (_req, res) => {
-			return Db.connectionState.connected && Db.connectionState.migrated
+			const { connected, migrated } = Db.connectionState;
-				? res.status(200).send({ status: 'ok' })
+			if (connected && migrated) {
-				: res.status(503).send({ status: 'error' });
+				res.status(200).send({ status: 'ok' });
+			} else {
+				res.status(503).send({ status: 'error' });
+			}
 		});
 
 		const { connectionState } = Db;
@@ -183,20 +186,20 @@ export abstract class AbstractServer {
 		if (this.webhooksEnabled) {
 			const liveWebhooksRequestHandler = createWebhookHandlerFor(Container.get(LiveWebhooks));
 			// Register a handler for live forms
-			this.app.all(`/${this.endpointForm}/:path(*)`, liveWebhooksRequestHandler);
+			this.app.all(`/${this.endpointForm}/*path`, liveWebhooksRequestHandler);
 
 			// Register a handler for live webhooks
-			this.app.all(`/${this.endpointWebhook}/:path(*)`, liveWebhooksRequestHandler);
+			this.app.all(`/${this.endpointWebhook}/*path`, liveWebhooksRequestHandler);
 
 			// Register a handler for waiting forms
 			this.app.all(
-				`/${this.endpointFormWaiting}/:path/:suffix?`,
+				`/${this.endpointFormWaiting}/:path/{:suffix}`,
 				createWebhookHandlerFor(Container.get(WaitingForms)),
 			);
 
 			// Register a handler for waiting webhooks
 			this.app.all(
-				`/${this.endpointWebhookWaiting}/:path/:suffix?`,
+				`/${this.endpointWebhookWaiting}/:path/{:suffix}`,
 				createWebhookHandlerFor(Container.get(WaitingWebhooks)),
 			);
 		}
@@ -205,8 +208,8 @@ export abstract class AbstractServer {
 			const testWebhooksRequestHandler = createWebhookHandlerFor(Container.get(TestWebhooks));
 
 			// Register a handler
-			this.app.all(`/${this.endpointFormTest}/:path(*)`, testWebhooksRequestHandler);
+			this.app.all(`/${this.endpointFormTest}/*path`, testWebhooksRequestHandler);
-			this.app.all(`/${this.endpointWebhookTest}/:path(*)`, testWebhooksRequestHandler);
+			this.app.all(`/${this.endpointWebhookTest}/*path`, testWebhooksRequestHandler);
 		}
 
 		// Block bots from scanning the application

packages/cli/src/controllers/annotation-tags.controller.ee.ts

@@ -22,7 +22,7 @@ export class AnnotationTagsController {
 		return await this.annotationTagService.save(tag);
 	}
 
-	@Patch('/:id(\\w+)')
+	@Patch('/:id')
 	@GlobalScope('annotationTag:update')
 	async updateTag(req: AnnotationTagsRequest.Update) {
 		const newTag = this.annotationTagService.toEntity({
@@ -33,7 +33,7 @@ export class AnnotationTagsController {
 		return await this.annotationTagService.save(newTag);
 	}
 
-	@Delete('/:id(\\w+)')
+	@Delete('/:id')
 	@GlobalScope('annotationTag:delete')
 	async deleteTag(req: AnnotationTagsRequest.Delete) {
 		const { id } = req.params;

packages/cli/src/controllers/tags.controller.ts

@@ -38,7 +38,7 @@ export class TagsController {
 		return await this.tagService.save(tag, 'create');
 	}
 
-	@Patch('/:id(\\w+)')
+	@Patch('/:id')
 	@GlobalScope('tag:update')
 	async updateTag(
 		_req: AuthenticatedRequest,
@@ -51,7 +51,7 @@ export class TagsController {
 		return await this.tagService.save(newTag, 'update');
 	}
 
-	@Delete('/:id(\\w+)')
+	@Delete('/:id')
 	@GlobalScope('tag:delete')
 	async deleteTag(_req: AuthenticatedRequest, _res: Response, @Param('id') tagId: string) {
 		await this.tagService.delete(tagId);

packages/cli/src/decorators/controller.registry.ts

@@ -116,7 +116,13 @@ export class ControllerRegistry {
 				...(route.accessScope ? [this.createScopedMiddleware(route.accessScope)] : []),
 				...controllerMiddlewares,
 				...route.middlewares,
-				route.usesTemplates ? handler : send(handler),
+				route.usesTemplates
+					? async (req, res) => {
+							// When using templates, intentionally drop the return value,
+							// since template rendering writes directly to the response.
+							await handler(req, res);
+						}
+					: send(handler),
 			);
 		}
 	}
@@ -133,11 +139,10 @@ export class ControllerRegistry {
 	private createLicenseMiddleware(feature: BooleanLicenseFeature): RequestHandler {
 		return (_req, res, next) => {
 			if (!this.license.isFeatureEnabled(feature)) {
-				return res
+				res.status(403).json({ status: 'error', message: 'Plan lacks license for this feature' });
-					.status(403)
+				return;
-					.json({ status: 'error', message: 'Plan lacks license for this feature' });
 			}
-			return next();
+			next();
 		};
 	}
 
@@ -152,13 +157,14 @@ export class ControllerRegistry {
 			const { scope, globalOnly } = accessScope;
 
 			if (!(await userHasScopes(req.user, [scope], globalOnly, req.params))) {
-				return res.status(403).json({
+				res.status(403).json({
 					status: 'error',
 					message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE,
 				});
+				return;
 			}
 
-			return next();
+			next();
 		};
 	}
 }

packages/cli/src/environments.ee/variables/variables.controller.ee.ts

@@ -76,7 +76,7 @@ export class VariablesController {
 		}
 	}
 
-	@Delete('/:id(\\w+)')
+	@Delete('/:id')
 	@GlobalScope('variable:delete')
 	async deleteVariable(req: VariablesRequest.Delete) {
 		const id = req.params.id;

packages/cli/src/metrics/__tests__/prometheus-metrics.service.test.ts

@@ -23,6 +23,8 @@ jest.mock('prom-client');
 jest.mock('express-prom-bundle', () => jest.fn(() => mockMiddleware));
 
 describe('PrometheusMetricsService', () => {
+	promClient.Counter.prototype.inc = jest.fn();
+
 	const globalConfig = mockInstance(GlobalConfig, {
 		endpoints: {
 			metrics: {

packages/cli/src/metrics/prometheus-metrics.service.ts

@@ -153,11 +153,11 @@ export class PrometheusMetricsService {
 				`/${this.globalConfig.endpoints.formWaiting}/`,
 				`/${this.globalConfig.endpoints.formTest}/`,
 			],
-			(req, res, next) => {
+			async (req, res, next) => {
 				activityGauge.reset();
 				activityGauge.set({ timestamp: new Date().toISOString() }, 1);
 
-				metricsMiddleware(req, res, next);
+				await metricsMiddleware(req, res, next);
 			},
 		);
 	}

packages/cli/src/middlewares/list-query/__tests__/list-query.test.ts

@@ -81,36 +81,36 @@ describe('List query middleware', () => {
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('should parse valid select', () => {
+		test('should parse valid select', async () => {
 			mockReq.query = { select: '["name", "id"]' };
 
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({ select: { name: true, id: true } });
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('ignore invalid select', () => {
+		test('ignore invalid select', async () => {
 			mockReq.query = { select: '["name", "foo"]' };
 
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({ select: { name: true } });
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('throw on invalid JSON', () => {
+		test('throw on invalid JSON', async () => {
 			mockReq.query = { select: '["name"' };
 
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
 
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
 
-		test('throw on non-string-array JSON for select', () => {
+		test('throw on non-string-array JSON for select', async () => {
 			mockReq.query = { select: '"name"' };
 
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
 
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
@@ -126,51 +126,51 @@ describe('List query middleware', () => {
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('should parse valid pagination', () => {
+		test('should parse valid pagination', async () => {
 			mockReq.query = { skip: '1', take: '2' };
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({ skip: 1, take: 2 });
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('should throw on skip without take', () => {
+		test('should throw on skip without take', async () => {
 			mockReq.query = { skip: '1' };
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toBeUndefined();
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
 
-		test('should default skip to 0', () => {
+		test('should default skip to 0', async () => {
 			mockReq.query = { take: '2' };
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({ skip: 0, take: 2 });
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('should cap take at 50', () => {
+		test('should cap take at 50', async () => {
 			mockReq.query = { take: '51' };
 
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({ skip: 0, take: 50 });
 			expect(nextFn).toBeCalledTimes(1);
 		});
 
-		test('should throw on non-numeric-integer take', () => {
+		test('should throw on non-numeric-integer take', async () => {
 			mockReq.query = { take: '3.2' };
 
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
 
-		test('should throw on non-numeric-integer skip', () => {
+		test('should throw on non-numeric-integer skip', async () => {
 			mockReq.query = { take: '3', skip: '3.2' };
 
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
@@ -224,7 +224,7 @@ describe('List query middleware', () => {
 				sortBy: value,
 			};
 
-			sortByQueryMiddleware(...args);
+			await sortByQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toMatchObject(
 				expect.objectContaining({
@@ -239,7 +239,7 @@ describe('List query middleware', () => {
 				sortBy: value as ListQuery.Workflow.SortOrder,
 			};
 
-			sortByQueryMiddleware(...args);
+			await sortByQueryMiddleware(...args);
 
 			expect(sendErrorResponse).toHaveBeenCalledTimes(1);
 		});
@@ -247,7 +247,7 @@ describe('List query middleware', () => {
 		test('should not pass sortBy to listQueryOptions if not provided', async () => {
 			mockReq.query = {};
 
-			sortByQueryMiddleware(...args);
+			await sortByQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toBeUndefined();
 			expect(nextFn).toBeCalledTimes(1);
@@ -259,7 +259,7 @@ describe('List query middleware', () => {
 			mockReq.query = { filter: '{ "name": "My Workflow" }', select: '["name", "id"]' };
 
 			await filterListQueryMiddleware(...args);
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({
 				select: { name: true, id: true },
@@ -273,7 +273,7 @@ describe('List query middleware', () => {
 			mockReq.query = { filter: '{ "name": "My Workflow" }', skip: '1', take: '2' };
 
 			await filterListQueryMiddleware(...args);
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({
 				filter: { name: 'My Workflow' },
@@ -287,8 +287,8 @@ describe('List query middleware', () => {
 		test('should combine select with pagination options', async () => {
 			mockReq.query = { select: '["name", "id"]', skip: '1', take: '2' };
 
-			selectListQueryMiddleware(...args);
+			await selectListQueryMiddleware(...args);
-			paginationListQueryMiddleware(...args);
+			await paginationListQueryMiddleware(...args);
 
 			expect(mockReq.listQueryOptions).toEqual({
 				select: { name: true, id: true },

packages/cli/src/public-api/index.ts

@@ -95,7 +95,7 @@ async function createApiRouter(
 			res: express.Response,
 			_next: express.NextFunction,
 		) => {
-			return res.status(error.status || 400).json({
+			res.status(error.status || 400).json({
 				message: error.message,
 			});
 		},

packages/cli/src/response-helper.ts

@@ -153,7 +153,7 @@ export function send<T, R extends Request, S extends Response>(
 	processFunction: (req: R, res: S) => Promise<T>,
 	raw = false,
 ) {
-	return async (req: R, res: S) => {
+	return async (req: R, res: S): Promise<void> => {
 		try {
 			const data = await processFunction(req, res);
 

packages/cli/src/scaling/worker-server.ts

@@ -100,8 +100,12 @@ export class WorkerServer {
 		const { health, overwrites, metrics } = this.endpointsConfig;
 
 		if (health) {
-			this.app.get('/healthz', async (_, res) => res.send({ status: 'ok' }));
+			this.app.get('/healthz', async (_, res) => {
-			this.app.get('/healthz/readiness', async (_, res) => await this.readiness(_, res));
+				res.send({ status: 'ok' });
+			});
+			this.app.get('/healthz/readiness', async (_, res) => {
+				await this.readiness(_, res);
+			});
 		}
 
 		if (overwrites) {

packages/cli/src/server.ts

@@ -311,7 +311,12 @@ export class Server extends AbstractServer {
 		const cacheOptions = inE2ETests || inDevelopment ? {} : { maxAge };
 		const { staticCacheDir } = Container.get(InstanceSettings);
 		if (frontendService) {
-			const serveIcons: express.RequestHandler = async (req, res) => {
+			this.app.use(
+				[
+					'/icons/{@:scope/}:packageName/*path/*file.svg',
+					'/icons/{@:scope/}:packageName/*path/*file.png',
+				],
+				async (req, res) => {
 					// eslint-disable-next-line prefer-const
 					let { scope, packageName } = req.params;
 					if (scope) packageName = `@${scope}/${packageName}`;
@@ -319,11 +324,12 @@ export class Server extends AbstractServer {
 					if (filePath) {
 						try {
 							await fsAccess(filePath);
-						return res.sendFile(filePath, cacheOptions);
+							return res.sendFile(filePath, { maxAge, dotfiles: 'allow' });
 						} catch {}
 					}
 					res.sendStatus(404);
-			};
+				},
+			);
 
 			const serveSchemas: express.RequestHandler = async (req, res) => {
 				const { node, version, resource, operation } = req.params;
@@ -342,10 +348,7 @@ export class Server extends AbstractServer {
 				}
 				res.sendStatus(404);
 			};
-
+			this.app.use('/schemas/:node/:version/:resource/:operation.json', serveSchemas);
-			this.app.use('/icons/@:scope/:packageName/*/*.(svg|png)', serveIcons);
-			this.app.use('/icons/:packageName/*/*.(svg|png)', serveIcons);
-			this.app.use('/schemas/:node/:version/:resource?/:operation?.json', serveSchemas);
 
 			const isTLSEnabled =
 				this.globalConfig.protocol === 'https' && !!(this.sslKey && this.sslCert);

packages/cli/src/task-runners/task-broker/task-broker-server.ts

@@ -171,7 +171,9 @@ export class TaskBrokerServer {
 			send(async (req) => await this.authController.createGrantToken(req)),
 		);
 
-		this.app.get('/healthz', (_, res) => res.send({ status: 'ok' }));
+		this.app.get('/healthz', (_, res) => {
+			res.send({ status: 'ok' });
+		});
 	}
 
 	private handleUpgradeRequest = (

packages/cli/src/webhooks/webhook-request-handler.ts

@@ -123,6 +123,10 @@ export function createWebhookHandlerFor(webhookManager: IWebhookManager) {
 	const handler = new WebhookRequestHandler(webhookManager);
 
 	return async (req: WebhookRequest | WebhookOptionsRequest, res: express.Response) => {
+		const { params } = req;
+		if (Array.isArray(params.path)) {
+			params.path = params.path.join('/');
+		}
 		await handler.handleRequest(req, res);
 	};
 }

packages/cli/test/integration/middlewares/body-parser.test.ts

@@ -7,8 +7,8 @@ import { rawBodyReader, bodyParser } from '@/middlewares/body-parser';
 
 describe('bodyParser', () => {
 	const server = createServer((req: Request, res: Response) => {
-		rawBodyReader(req, res, async () => {
+		void rawBodyReader(req, res, async () => {
-			bodyParser(req, res, () => res.end(JSON.stringify(req.body)));
+			void bodyParser(req, res, () => res.end(JSON.stringify(req.body)));
 		});
 	});
 

packages/cli/test/integration/variables.test.ts

@@ -177,7 +177,7 @@ describe('POST /variables', () => {
 		expect(byKey).toBeNull();
 	});
 
-	test("POST /variables should not create a new variable and return it if the instance doesn't have a license", async () => {
+	test("should not create a new variable and return it if the instance doesn't have a license", async () => {
 		license.disable('feat:variables');
 		const response = await authOwnerAgent.post('/variables').send(toCreate);
 		expect(response.statusCode).toBe(403);

patches/@types__express-serve-static-core@5.0.6.patch

@@ -1,8 +1,8 @@
 diff --git a/index.d.ts b/index.d.ts
-index 5cc36f5760c806a76ee839bfb67c419c9cb48901..8ef0bf74f0f31741b564fe37f040144526e98eb5 100644
+index fbad2c77669b1effe9d1ca30f518eb5e0058f2e0..6dc02519a6a8dfe1c7dc7d4d600478ba841b172a 100644
 --- a/index.d.ts
 +++ b/index.d.ts
-@@ -646,7 +646,7 @@ export interface Request<
+@@ -641,7 +641,7 @@ export interface Request<
  
      query: ReqQuery;
  

pnpm-workspace.yaml

@@ -8,7 +8,7 @@ packages:
 catalog:
   '@sentry/node': 8.52.1
   '@types/basic-auth': ^1.1.3
-  '@types/express': ^4.17.21
+  '@types/express': ^5.0.1
   '@types/lodash': ^4.14.195
   '@types/uuid': ^10.0.0
   '@types/xml2js': ^0.4.14