diff --git a/lib/dependencies/sub-process.ts b/lib/dependencies/sub-process.ts index 0e12e75..fafe879 100644 --- a/lib/dependencies/sub-process.ts +++ b/lib/dependencies/sub-process.ts @@ -1,5 +1,4 @@ import { spawn, SpawnOptions, spawnSync } from 'child_process'; -import { quoteAll } from 'shescape/stateless'; interface ProcessOptions { cwd?: string; @@ -8,7 +7,7 @@ interface ProcessOptions { function makeSpawnOptions(options?: ProcessOptions) { const spawnOptions: SpawnOptions = { - shell: true, + shell: false, env: { ...process.env }, }; if (options && options.cwd) { @@ -39,12 +38,16 @@ export function execute( options?: ProcessOptions ): Promise { const spawnOptions = makeSpawnOptions(options); - args = quoteAll(args, { flagProtection: false }); return new Promise((resolve, reject) => { let stdout = ''; let stderr = ''; const proc = spawn(command, args, spawnOptions); + + proc.on('error', (error) => { + reject(error); + }); + proc.stdout.on('data', (data) => { stdout = stdout + data; }); @@ -67,7 +70,6 @@ export function executeSync( options?: ProcessOptions ) { const spawnOptions = makeSpawnOptions(options); - args = quoteAll(args, { flagProtection: false }); return spawnSync(command, args, spawnOptions); } diff --git a/test/test-utils.ts b/test/test-utils.ts index c013e38..49082ec 100644 --- a/test/test-utils.ts +++ b/test/test-utils.ts @@ -103,7 +103,7 @@ function createVenv(venvDir: string) { revert = deactivateVirtualenv(); } try { - let proc = subProcess.executeSync('python3 -m venv', [venvDir]); + let proc = subProcess.executeSync('python3', ['-m', 'venv', venvDir]); if (proc.status !== 0) { console.error(proc.stdout.toString() + '\n' + proc.stderr.toString()); throw new Error('Failed to create virtualenv in ' + venvDir); diff --git a/test/unit/sub-process.spec.ts b/test/unit/sub-process.spec.ts index 50f6d09..088ed5f 100644 --- a/test/unit/sub-process.spec.ts +++ b/test/unit/sub-process.spec.ts @@ -1,4 +1,4 @@ -import { executeSync } from '../../lib/dependencies/sub-process'; +import { executeSync, execute } from '../../lib/dependencies/sub-process'; describe('Test sub-process.ts', () => { it('test restoring proxy setting in executeSync()', async () => { @@ -89,4 +89,44 @@ describe('Test sub-process.ts', () => { ); expect(output.stdout.toString().trim()).toEqual(expectedNoProxy); }); + + describe('Security: Command injection protection', () => { + it('should prevent command injection in executeSync()', () => { + // Test that malicious command strings are treated as literal filenames (not executed) + const maliciousCommand = 'python3; echo injected'; + const result = executeSync(maliciousCommand, ['--version']); + + // Should fail with ENOENT because 'python3; echo injected' is not a valid executable + expect(result.status).not.toBe(0); + expect((result.error as any)?.code).toBe('ENOENT'); + }); + + it('should prevent command injection in execute()', async () => { + // Test that malicious command strings are treated as literal filenames (not executed) + const maliciousCommand = 'python3; whoami; echo injected'; + + try { + await execute(maliciousCommand, ['--version']); + fail('Expected execute() to reject with an error'); + } catch (error: any) { + // Should fail with ENOENT because the malicious command is treated as a literal filename + expect(error.code).toBe('ENOENT'); + expect(error.syscall).toBe(`spawn ${maliciousCommand}`); + } + }); + + it('should execute legitimate commands normally', async () => { + // Verify that normal commands still work correctly + const result = await execute('python3', ['--version']); + expect(result).toMatch(/Python \d+\.\d+\.\d+/); + }); + + it('should handle arguments with special characters safely', async () => { + // Verify that special characters in arguments don't enable injection + const result = await execute('python3', ['--version', '; echo injected']); + // Should only show Python version, not execute the injected command + expect(result).toMatch(/Python \d+\.\d+\.\d+/); + expect(result).not.toMatch(/injected/); + }); + }); });