fix: #30 call python3 with execFileSync to block shell injection in path

Replace the /bin/bash -c execSync invocation in src/aibolit.ts with execFileSync('python3', [...args, path]) so the path argument is passed as a single argv element and shell metacharacters in user-supplied paths can no longer execute. The catch branch keeps the previous tolerance of a non-zero exit by parsing whatever stdout the failing call already produced. Tests mock execFileSync alongside execSync, add a regression case asserting the argv shape, and cover both arms of the catch fallback.

Author: kreinba

src/aibolit.ts

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025 Yegor Bugayenko
 // SPDX-License-Identifier: MIT
 
-import { execSync } from 'child_process';
+import { execFileSync, execSync } from 'child_process';
 import fs from 'fs';
 import semver from 'semver';
 import { to_gpt } from './to_gpt';
@@ -32,11 +32,16 @@ export const aibolit = async function(path: string): Promise<string> {
   if (!fs.existsSync(path)) {
     return `File does not exist: ${path}`;
   }
-  const warns = execSync(
-    `
-    /bin/bash -c "set -o pipefail; (python3 -m aibolit check --full --filenames ${path} || true)"
-    `
-  ).toString();
+  let warns: string;
+  try {
+    warns = execFileSync(
+      'python3',
+      ['-m', 'aibolit', 'check', '--full', '--filenames', path]
+    ).toString();
+  } catch (e) {
+    const err = e as { stdout?: Buffer | string };
+    warns = err.stdout == null ? '' : err.stdout.toString();
+  }
   const lines = warns.trim().split('\n').filter(line => line.trim());
   const parsed = lines.map(line => {
     const match = line.match(/^.+?\[(\d+)\]:\s(.+?)\s\(P\d+:\s(\d+(?:\.\d+)?)\)$/);

test/aibolit.test.ts

@@ -8,11 +8,13 @@ import { join } from 'path';
 import { tmpdir } from 'os';
 
 jest.mock('child_process', () => ({
+  execFileSync: jest.fn(),
   execSync: jest.fn()
 }));
 
-import { execSync } from 'child_process';
+import { execFileSync, execSync } from 'child_process';
 const mockExecSync = execSync as jest.MockedFunction<typeof execSync>;
+const mockExecFileSync = execFileSync as jest.MockedFunction<typeof execFileSync>;
 
 describe('aibolit', () => {
   beforeEach(() => {
@@ -35,7 +37,7 @@ describe('aibolit', () => {
       `
     );
     mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
-    mockExecSync.mockReturnValueOnce(Buffer.from(''));
+    mockExecFileSync.mockReturnValueOnce(Buffer.from(''));
     try {
       const issue = await aibolit(path);
       expect(issue).toContain('Your code is perfect');
@@ -60,7 +62,7 @@ describe('aibolit', () => {
       `
     );
     mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
-    mockExecSync.mockReturnValueOnce(
+    mockExecFileSync.mockReturnValueOnce(
       Buffer.from('Test.java[44]: Missing final keyword (P13: 5.0)\n'));
     try {
       const issue = await aibolit(path);
@@ -116,7 +118,7 @@ describe('aibolit', () => {
 
   test('executes normally when version is exactly required', async () => {
     mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
-    mockExecSync.mockReturnValueOnce(Buffer.from(''));
+    mockExecFileSync.mockReturnValueOnce(Buffer.from(''));
     const path = '/tmp/test.java';
     writeFileSync(path, 'public class Test {}');
     const result = await aibolit(path);
@@ -125,7 +127,7 @@ describe('aibolit', () => {
 
   test('executes normally when version is newer than required', async () => {
     mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.4.0\n'));
-    mockExecSync.mockReturnValueOnce(Buffer.from(''));
+    mockExecFileSync.mockReturnValueOnce(Buffer.from(''));
     const path = '/tmp/test.java';
     writeFileSync(path, 'public class Test {}');
     const result = await aibolit(path);
@@ -173,11 +175,53 @@ describe('aibolit', () => {
 
   test('handles aibolit output with invalid warning format', async () => {
     mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
-    mockExecSync.mockReturnValueOnce(
+    mockExecFileSync.mockReturnValueOnce(
       Buffer.from('Invalid warning format\nAnother invalid line\n'));
     const path = '/tmp/test.java';
     writeFileSync(path, 'public class Test {}');
     const result = await aibolit(path);
     expect(result).toBe('Your code is perfect');
   });
+
+  test('parses warnings captured on stdout when aibolit exits non-zero', async () => {
+    mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
+    mockExecFileSync.mockImplementationOnce(() => {
+      const err = new Error('Command failed') as Error & { stdout: Buffer };
+      err.stdout = Buffer.from('Test.java[44]: Missing final keyword (P13: 5.0)\n');
+      throw err;
+    });
+    const path = '/tmp/test.java';
+    writeFileSync(path, 'public class Test {}');
+    const result = await aibolit(path);
+    expect(result).not.toContain('Your code is perfect');
+    expect(result).toContain('Missing final keyword');
+  });
+
+  test('returns perfect code when aibolit throws without stdout', async () => {
+    mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
+    mockExecFileSync.mockImplementationOnce(() => {
+      throw new Error('Command failed');
+    });
+    const path = '/tmp/test.java';
+    writeFileSync(path, 'public class Test {}');
+    const result = await aibolit(path);
+    expect(result).toBe('Your code is perfect');
+  });
+
+  test('passes path as separate argv element so shell metacharacters do not execute', async () => {
+    const tmp = mkdtempSync(join(tmpdir(), 'aibolit-test-'));
+    const path = join(tmp, 'Test.java; touch pwned.java');
+    writeFileSync(path, 'public class Test {}');
+    mockExecSync.mockReturnValueOnce(Buffer.from('aibolit 1.3.0\n'));
+    mockExecFileSync.mockReturnValueOnce(Buffer.from(''));
+    try {
+      await aibolit(path);
+      expect(mockExecFileSync).toHaveBeenCalledWith(
+        'python3',
+        ['-m', 'aibolit', 'check', '--full', '--filenames', path]
+      );
+    } finally {
+      rmSync(tmp, { recursive: true, force: true });
+    }
+  });
 });