From d4afb0f38b483b4b3dda0e4ca79c98bcd0acd62d Mon Sep 17 00:00:00 2001
From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com>
Date: Tue, 8 Oct 2024 17:04:45 +0300
Subject: [PATCH] refactor: Simplify resolveIcon checks and add tests (#11165)

---
 .../load-nodes-and-credentials.test.ts        | 37 +++++++++++++++++++
 .../cli/src/load-nodes-and-credentials.ts     | 14 +++----
 2 files changed, 44 insertions(+), 7 deletions(-)
 create mode 100644 packages/cli/src/__tests__/load-nodes-and-credentials.test.ts

diff --git a/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts
new file mode 100644
index 0000000000..bcf485445f
--- /dev/null
+++ b/packages/cli/src/__tests__/load-nodes-and-credentials.test.ts
@@ -0,0 +1,37 @@
+import { mock } from 'jest-mock-extended';
+import type { DirectoryLoader } from 'n8n-core';
+
+import { LoadNodesAndCredentials } from '../load-nodes-and-credentials';
+
+describe('LoadNodesAndCredentials', () => {
+	describe('resolveIcon', () => {
+		let instance: LoadNodesAndCredentials;
+
+		beforeEach(() => {
+			instance = new LoadNodesAndCredentials(mock(), mock(), mock());
+			instance.loaders.package1 = mock<DirectoryLoader>({
+				directory: '/icons/package1',
+			});
+		});
+
+		it('should return undefined if the loader for the package is not found', () => {
+			const result = instance.resolveIcon('unknownPackage', '/icons/unknownPackage/icon.png');
+			expect(result).toBeUndefined();
+		});
+
+		it('should return undefined if the resolved file path is outside the loader directory', () => {
+			const result = instance.resolveIcon('package1', '/some/other/path/icon.png');
+			expect(result).toBeUndefined();
+		});
+
+		it('should return the file path if the file is within the loader directory', () => {
+			const result = instance.resolveIcon('package1', '/icons/package1/icon.png');
+			expect(result).toBe('/icons/package1/icon.png');
+		});
+
+		it('should return undefined if the URL is outside the package directory', () => {
+			const result = instance.resolveIcon('package1', '/icons/package1/../../../etc/passwd');
+			expect(result).toBeUndefined();
+		});
+	});
+});
diff --git a/packages/cli/src/load-nodes-and-credentials.ts b/packages/cli/src/load-nodes-and-credentials.ts
index 4e33d495c8..a5cff3764f 100644
--- a/packages/cli/src/load-nodes-and-credentials.ts
+++ b/packages/cli/src/load-nodes-and-credentials.ts
@@ -29,6 +29,7 @@ import {
 	inE2ETests,
 } from '@/constants';
 import { Logger } from '@/logging/logger.service';
+import { isContainedWithin } from '@/utils/path-util';
 
 interface LoadedNodesAndCredentials {
 	nodes: INodeTypeData;
@@ -155,14 +156,13 @@ export class LoadNodesAndCredentials {
 
 	resolveIcon(packageName: string, url: string): string | undefined {
 		const loader = this.loaders[packageName];
-		if (loader) {
-			const pathPrefix = `/icons/${packageName}/`;
-			const filePath = path.resolve(loader.directory, url.substring(pathPrefix.length));
-			if (!path.relative(loader.directory, filePath).includes('..')) {
-				return filePath;
-			}
+		if (!loader) {
+			return undefined;
 		}
-		return undefined;
+		const pathPrefix = `/icons/${packageName}/`;
+		const filePath = path.resolve(loader.directory, url.substring(pathPrefix.length));
+
+		return isContainedWithin(loader.directory, filePath) ? filePath : undefined;
 	}
 
 	getCustomDirectories(): string[] {