fix(security): replace exec with execFile for git commands

src/controller/repository.ts

@@ -4,7 +4,7 @@ import path, { join } from 'path';
 import { Application, Applications, Resource } from '../app';
 import AppError from '../utils/appError';
 import { appsDirectory } from '../utils/config';
-import { exec } from '../utils/exec';
+import { execFile } from '../utils/execFile';
 import { findRunners } from '../utils/install';
 import { catchAsync } from './catch';
 
@@ -68,8 +68,12 @@ export const repositoryBranchList = catchAsync(
 		try {
 			const { url } = req.body;
 
-			// list remote branches for the repository
-			const { stdout } = await exec(`git ls-remote --heads ${url}`);
+			// list remote branches for the repository using execFile (no shell)
+			const { stdout } = await execFile('git', [
+				'ls-remote',
+				'--heads',
+				url
+			]);
 
 			// Parse branches from the command output
 			const branches = stdout
@@ -101,16 +105,21 @@ export const repositoryFileList = catchAsync(
 			await repositoryDelete(appsDirectory, url);
 
 			// Clone the repository with the requested branch so ls-tree can resolve it
-			await exec(
-				`git clone --depth=1 --no-checkout --branch ${branch} ${url} ${repoPath}`
-			);
+			await execFile('git', [
+				'clone',
+				'--depth=1',
+				'--no-checkout',
+				'--branch',
+				branch,
+				url,
+				repoPath
+			]);
 
 			// List files in the specified branch
-			const { stdout } = await exec(
-				`git ls-tree -r ${branch} --name-only`,
-				{
-					cwd: repoPath
-				}
+			const { stdout } = await execFile(
+				'git',
+				['ls-tree', '-r', branch, '--name-only'],
+				{ cwd: repoPath }
 			);
 
 			const files = stdout.trim().split('\n').filter(Boolean);
@@ -159,12 +168,15 @@ export const repositoryClone = catchAsync(
 
 		try {
 			// Clone the repository into the specified directory
-			await exec(
-				`git clone --single-branch --depth=1 --branch ${branch} ${url} ${join(
-					appsDirectory,
-					repositoryName(url)
-				)}`
-			);
+			await execFile('git', [
+				'clone',
+				'--single-branch',
+				'--depth=1',
+				'--branch',
+				branch,
+				url,
+				join(appsDirectory, repositoryName(url))
+			]);
 		} catch (err) {
 			const message = err instanceof Error ? err.message : String(err);
 			return next(

src/utils/execFile.ts

@@ -0,0 +1,9 @@
+import { execFile as syncExecFile } from 'child_process';
+import { promisify } from 'util';
+
+/**
+ * Safe alternative to exec that doesn't use a shell.
+ * Uses execFile with argument arrays instead of shell string interpolation.
+ * Prevents command injection attacks.
+ */
+export const execFile = promisify(syncExecFile);

src/utils/logger.ts

@@ -16,46 +16,25 @@ interface PIDToColorCodeMapType {
 	[key: string]: number;
 }
 
-interface AssignedColorCodesType {
-	[key: string]: boolean;
-}
-
 // Maps a PID to a color code
 const PIDToColorCodeMap: PIDToColorCodeMapType = {};
 
-// Tracks whether a color code is assigned
-const assignedColorCodes: AssignedColorCodesType = {};
+// Round-robin counter for color assignment
+let colorIndex = 0;
 
 const logFilePath = path.join(__dirname, '../../logs/');
 const logFileName = 'app.log';
 const logFileFullPath = path.resolve(path.join(logFilePath, logFileName));
 
-// TODO: Implement this properly?
-// const maxWorkerWidth = (maxIndexWidth = 3): number => {
-// 	const workerLengths = Object.keys(Applications).map(
-// 		worker => worker.length
-// 	);
-// 	return Math.max(...workerLengths) + maxIndexWidth;
-// };
-
-// TODO: There is a problem with this code, looking randomly for an unique code
-// will end in an endless loop whenever all color codes are allocated, we should
-// use a better way of managing this
 const assignColorToWorker = (
 	deploymentName: string,
 	workerPID: number
 ): string => {
 	if (!PIDToColorCodeMap[workerPID]) {
-		let colorCode: number;
-
-		// Keep looking for unique code
-		do {
-			colorCode = ANSICode[Math.floor(Math.random() * ANSICode.length)];
-		} while (assignedColorCodes[colorCode]);
-
-		// Assign the unique code and mark it as used
+		// Use round-robin to cycle through colors safely
+		const colorCode = ANSICode[colorIndex % ANSICode.length];
+		colorIndex++;
 		PIDToColorCodeMap[workerPID] = colorCode;
-		assignedColorCodes[colorCode] = true;
 	}
 	const assignColorCode = PIDToColorCodeMap[workerPID];
 	return `\x1b[38;5;${assignColorCode}m${deploymentName}\x1b[0m`;