fix(cli): prevent command injection by escaping git branch suggestions

This commit is contained in:
MD. MOHIBUR RAHMAN
2026-02-24 04:25:16 +06:00
parent 5b3819073f
commit 39b393abfb
2 changed files with 45 additions and 6 deletions
@@ -8,9 +8,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { gitProvider } from './gitProvider.js';
import * as childProcess from 'node:child_process';
vi.mock('node:child_process', () => ({
execFile: vi.fn(),
}));
vi.mock('node:child_process', async (importOriginal) => {
const actual = await importOriginal<typeof import('node:child_process')>();
return {
...actual,
execFile: vi.fn(),
};
});
describe('gitProvider', () => {
beforeEach(() => {
@@ -34,9 +38,11 @@ describe('gitProvider', () => {
(_cmd, _args, _opts, cb: unknown) => {
const callback = (typeof _opts === 'function' ? _opts : cb) as (
error: Error | null,
stdout: string,
result: { stdout: string },
) => void;
callback(null, 'main\nfeature-branch\nfix/bug\n');
callback(null, {
stdout: 'main\nfeature-branch\nfix/bug\nbranch(with)special\n',
});
return {} as ReturnType<typeof childProcess.execFile>;
},
);
@@ -49,6 +55,7 @@ describe('gitProvider', () => {
expect(result.exclusive).toBe(true);
expect(result.suggestions).toHaveLength(1);
expect(result.suggestions[0].label).toBe('feature-branch');
expect(result.suggestions[0].value).toBe('feature-branch');
expect(childProcess.execFile).toHaveBeenCalledWith(
'git',
@@ -58,6 +65,37 @@ describe('gitProvider', () => {
);
});
it('escapes branch names with shell metacharacters', async () => {
vi.mocked(childProcess.execFile).mockImplementation(
(_cmd, _args, _opts, cb: unknown) => {
const callback = (typeof _opts === 'function' ? _opts : cb) as (
error: Error | null,
result: { stdout: string },
) => void;
callback(null, { stdout: 'main\nbranch(with)special\n' });
return {} as ReturnType<typeof childProcess.execFile>;
},
);
const result = await gitProvider.getCompletions(
['git', 'checkout', 'branch('],
2,
'/tmp',
);
expect(result.exclusive).toBe(true);
expect(result.suggestions).toHaveLength(1);
expect(result.suggestions[0].label).toBe('branch(with)special');
// On Windows, space escape is not done. But since UNIX_SHELL_SPECIAL_CHARS is mostly tested,
// we can use a matcher that checks if escaping was applied (it differs per platform but that's handled by escapeShellPath).
// Let's match the value against either unescaped (win) or escaped (unix).
const isWin = process.platform === 'win32';
expect(result.suggestions[0].value).toBe(
isWin ? 'branch(with)special' : 'branch\\(with\\)special',
);
});
it('returns empty results if git branch fails', async () => {
vi.mocked(childProcess.execFile).mockImplementation(
(_cmd, _args, _opts, cb: unknown) => {
@@ -7,6 +7,7 @@
import { execFile } from 'node:child_process';
import { promisify } from 'node:util';
import type { ShellCompletionProvider, CompletionResult } from './types.js';
import { escapeShellPath } from '../useShellCompletion.js';
const execFileAsync = promisify(execFile);
@@ -74,7 +75,7 @@ export const gitProvider: ShellCompletionProvider = {
.filter((b) => b.startsWith(partial))
.map((b) => ({
label: b,
value: b,
value: escapeShellPath(b),
description: 'branch',
})),
exclusive: true,