diff --git a/actions/start.action.ts b/actions/start.action.ts index 00081a505..8c1a2ca35 100644 --- a/actions/start.action.ts +++ b/actions/start.action.ts @@ -2,6 +2,7 @@ import { red } from 'ansis'; import { spawn } from 'child_process'; import * as fs from 'fs'; import { join } from 'path'; +import { platform } from 'os'; import * as killProcess from 'tree-kill'; import { Input } from '../commands'; import { getTscConfigPath } from '../lib/compiler/helpers/get-tsc-config.path'; @@ -79,7 +80,8 @@ export class StartAction extends BuildAction { const shellOption = commandOptions.find( (option) => option.name === 'shell', ); - const useShell = !!shellOption?.value; + const isWindows = platform() === 'win32'; + const useShell = shellOption?.value !== false && isWindows; const envFileOption = commandOptions.find( (option) => option.name === 'envFile', diff --git a/lib/runners/abstract.runner.ts b/lib/runners/abstract.runner.ts index d4c9747f2..5d4b64c70 100644 --- a/lib/runners/abstract.runner.ts +++ b/lib/runners/abstract.runner.ts @@ -1,5 +1,6 @@ import { red } from 'ansis'; import { ChildProcess, spawn, SpawnOptions } from 'child_process'; +import { platform } from 'os'; import { MESSAGES } from '../ui'; export class AbstractRunner { @@ -14,14 +15,15 @@ export class AbstractRunner { cwd: string = process.cwd(), ): Promise { const args: string[] = [command]; + const isWindows = platform() === 'win32'; const options: SpawnOptions = { cwd, stdio: collect ? 'pipe' : 'inherit', - shell: true, + shell: isWindows, }; return new Promise((resolve, reject) => { const child: ChildProcess = spawn( - `${this.binary}`, + this.binary, [...this.args, ...args], options, ); diff --git a/test/lib/runners/abstract.runner.security.spec.ts b/test/lib/runners/abstract.runner.security.spec.ts new file mode 100644 index 000000000..71c5cfe8f --- /dev/null +++ b/test/lib/runners/abstract.runner.security.spec.ts @@ -0,0 +1,95 @@ +import { spawn } from 'child_process'; +import { platform } from 'os'; +import { AbstractRunner } from '../../../lib/runners/abstract.runner'; + +jest.mock('child_process', () => ({ + spawn: jest.fn(), +})); + +jest.mock('os', () => ({ + platform: jest.fn(), +})); + +describe('AbstractRunner Security Tests', () => { + let mockSpawn: jest.MockedFunction; + let mockPlatform: jest.MockedFunction; + + beforeEach(() => { + mockSpawn = spawn as jest.MockedFunction; + mockPlatform = platform as jest.MockedFunction; + + const mockChild = { + on: jest.fn((event: string, callback: (code: number) => void) => { + if (event === 'close') { + setTimeout(() => callback(0), 0); + } + }), + } as any; + + mockSpawn.mockReturnValue(mockChild); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('Shell injection protection', () => { + it('should not use shell on macOS/Linux to prevent command injection', async () => { + mockPlatform.mockReturnValue('darwin'); + + const runner = new AbstractRunner('node'); + await runner.run('$(malicious-command)'); + + expect(mockSpawn).toHaveBeenCalledWith( + 'node', + ['$(malicious-command)'], + expect.objectContaining({ + shell: false, + }), + ); + }); + + it('should not use shell on Linux to prevent variable expansion', async () => { + mockPlatform.mockReturnValue('linux'); + + const runner = new AbstractRunner('node'); + await runner.run('$USER'); + + expect(mockSpawn).toHaveBeenCalledWith( + 'node', + ['$USER'], + expect.objectContaining({ + shell: false, + }), + ); + }); + + it('should use shell only on Windows for compatibility', async () => { + mockPlatform.mockReturnValue('win32'); + + const runner = new AbstractRunner('node'); + await runner.run('test-command'); + + expect(mockSpawn).toHaveBeenCalledWith( + 'node', + ['test-command'], + expect.objectContaining({ + shell: true, + }), + ); + }); + + it('should pass commands as separate arguments, not concatenated strings', async () => { + mockPlatform.mockReturnValue('darwin'); + + const runner = new AbstractRunner('node', ['--enable-source-maps']); + await runner.run('dist/main.js'); + + expect(mockSpawn).toHaveBeenCalledWith( + 'node', + ['--enable-source-maps', 'dist/main.js'], + expect.any(Object), + ); + }); + }); +});