diff --git a/src/controller/repository.ts b/src/controller/repository.ts index c643ee8..a1ae2be 100644 --- a/src/controller/repository.ts +++ b/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( diff --git a/src/utils/execFile.ts b/src/utils/execFile.ts new file mode 100644 index 0000000..8e8c41e --- /dev/null +++ b/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); diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 601fd6b..7c59294 100644 --- a/src/utils/logger.ts +++ b/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`;