diff options
| author | Abhi <[email protected]> | 2025-08-17 00:02:54 -0400 |
|---|---|---|
| committer | GitHub <[email protected]> | 2025-08-17 04:02:54 +0000 |
| commit | 33b9bdb11e9371dfa6891ba1be47a12b958f493d (patch) | |
| tree | 46ac192df7dd2aeaf35cb5eef5c121bc5decada9 /packages/core/src | |
| parent | e7dbc607a598c5270507d0ce7d55a3c98dcb0c0c (diff) | |
feat(cli): Introduce arguments for shell execution in custom commands (#5966)
Diffstat (limited to 'packages/core/src')
| -rw-r--r-- | packages/core/src/services/shellExecutionService.test.ts | 19 | ||||
| -rw-r--r-- | packages/core/src/utils/shell-utils.test.ts | 157 | ||||
| -rw-r--r-- | packages/core/src/utils/shell-utils.ts | 96 |
3 files changed, 268 insertions, 4 deletions
diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 2fe51a5e..47df12e8 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -10,6 +10,16 @@ vi.mock('child_process', () => ({ spawn: mockSpawn, })); +const mockGetShellConfiguration = vi.hoisted(() => vi.fn()); +let mockIsWindows = false; + +vi.mock('../utils/shell-utils.js', () => ({ + getShellConfiguration: mockGetShellConfiguration, + get isWindows() { + return mockIsWindows; + }, +})); + import EventEmitter from 'events'; import { Readable } from 'stream'; import { type ChildProcess } from 'child_process'; @@ -43,18 +53,21 @@ describe('ShellExecutionService', () => { vi.clearAllMocks(); mockIsBinary.mockReturnValue(false); - mockPlatform.mockReturnValue('linux'); + + mockGetShellConfiguration.mockReturnValue({ + executable: 'bash', + argsPrefix: ['-c'], + }); + mockIsWindows = false; onOutputEventMock = vi.fn(); mockChildProcess = new EventEmitter() as EventEmitter & Partial<ChildProcess>; - // FIX: Cast simple EventEmitters to the expected stream type. mockChildProcess.stdout = new EventEmitter() as Readable; mockChildProcess.stderr = new EventEmitter() as Readable; mockChildProcess.kill = vi.fn(); - // FIX: Use Object.defineProperty to set the readonly 'pid' property. Object.defineProperty(mockChildProcess, 'pid', { value: 12345, configurable: true, diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 22d316ad..913ec897 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -4,24 +4,47 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { expect, describe, it, beforeEach } from 'vitest'; +import { expect, describe, it, beforeEach, vi, afterEach } from 'vitest'; import { checkCommandPermissions, + escapeShellArg, getCommandRoots, + getShellConfiguration, isCommandAllowed, stripShellWrapper, } from './shell-utils.js'; import { Config } from '../config/config.js'; +const mockPlatform = vi.hoisted(() => vi.fn()); +vi.mock('os', () => ({ + default: { + platform: mockPlatform, + }, + platform: mockPlatform, +})); + +const mockQuote = vi.hoisted(() => vi.fn()); +vi.mock('shell-quote', () => ({ + quote: mockQuote, +})); + let config: Config; beforeEach(() => { + mockPlatform.mockReturnValue('linux'); + mockQuote.mockImplementation((args: string[]) => + args.map((arg) => `'${arg}'`).join(' '), + ); config = { getCoreTools: () => [], getExcludeTools: () => [], } as unknown as Config; }); +afterEach(() => { + vi.clearAllMocks(); +}); + describe('isCommandAllowed', () => { it('should allow a command if no restrictions are provided', () => { const result = isCommandAllowed('ls -l', config); @@ -277,3 +300,135 @@ describe('stripShellWrapper', () => { expect(stripShellWrapper('ls -l')).toEqual('ls -l'); }); }); + +describe('escapeShellArg', () => { + describe('POSIX (bash)', () => { + it('should use shell-quote for escaping', () => { + mockQuote.mockReturnValueOnce("'escaped value'"); + const result = escapeShellArg('raw value', 'bash'); + expect(mockQuote).toHaveBeenCalledWith(['raw value']); + expect(result).toBe("'escaped value'"); + }); + + it('should handle empty strings', () => { + const result = escapeShellArg('', 'bash'); + expect(result).toBe(''); + expect(mockQuote).not.toHaveBeenCalled(); + }); + }); + + describe('Windows', () => { + describe('when shell is cmd.exe', () => { + it('should wrap simple arguments in double quotes', () => { + const result = escapeShellArg('search term', 'cmd'); + expect(result).toBe('"search term"'); + }); + + it('should escape internal double quotes by doubling them', () => { + const result = escapeShellArg('He said "Hello"', 'cmd'); + expect(result).toBe('"He said ""Hello"""'); + }); + + it('should handle empty strings', () => { + const result = escapeShellArg('', 'cmd'); + expect(result).toBe(''); + }); + }); + + describe('when shell is PowerShell', () => { + it('should wrap simple arguments in single quotes', () => { + const result = escapeShellArg('search term', 'powershell'); + expect(result).toBe("'search term'"); + }); + + it('should escape internal single quotes by doubling them', () => { + const result = escapeShellArg("It's a test", 'powershell'); + expect(result).toBe("'It''s a test'"); + }); + + it('should handle double quotes without escaping them', () => { + const result = escapeShellArg('He said "Hello"', 'powershell'); + expect(result).toBe('\'He said "Hello"\''); + }); + + it('should handle empty strings', () => { + const result = escapeShellArg('', 'powershell'); + expect(result).toBe(''); + }); + }); + }); +}); + +describe('getShellConfiguration', () => { + const originalEnv = { ...process.env }; + + afterEach(() => { + process.env = originalEnv; + }); + + it('should return bash configuration on Linux', () => { + mockPlatform.mockReturnValue('linux'); + const config = getShellConfiguration(); + expect(config.executable).toBe('bash'); + expect(config.argsPrefix).toEqual(['-c']); + expect(config.shell).toBe('bash'); + }); + + it('should return bash configuration on macOS (darwin)', () => { + mockPlatform.mockReturnValue('darwin'); + const config = getShellConfiguration(); + expect(config.executable).toBe('bash'); + expect(config.argsPrefix).toEqual(['-c']); + expect(config.shell).toBe('bash'); + }); + + describe('on Windows', () => { + beforeEach(() => { + mockPlatform.mockReturnValue('win32'); + }); + + it('should return cmd.exe configuration by default', () => { + delete process.env.ComSpec; + const config = getShellConfiguration(); + expect(config.executable).toBe('cmd.exe'); + expect(config.argsPrefix).toEqual(['/d', '/s', '/c']); + expect(config.shell).toBe('cmd'); + }); + + it('should respect ComSpec for cmd.exe', () => { + const cmdPath = 'C:\\WINDOWS\\system32\\cmd.exe'; + process.env.ComSpec = cmdPath; + const config = getShellConfiguration(); + expect(config.executable).toBe(cmdPath); + expect(config.argsPrefix).toEqual(['/d', '/s', '/c']); + expect(config.shell).toBe('cmd'); + }); + + it('should return PowerShell configuration if ComSpec points to powershell.exe', () => { + const psPath = + 'C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'; + process.env.ComSpec = psPath; + const config = getShellConfiguration(); + expect(config.executable).toBe(psPath); + expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']); + expect(config.shell).toBe('powershell'); + }); + + it('should return PowerShell configuration if ComSpec points to pwsh.exe', () => { + const pwshPath = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe'; + process.env.ComSpec = pwshPath; + const config = getShellConfiguration(); + expect(config.executable).toBe(pwshPath); + expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']); + expect(config.shell).toBe('powershell'); + }); + + it('should be case-insensitive when checking ComSpec', () => { + process.env.ComSpec = 'C:\\Path\\To\\POWERSHELL.EXE'; + const config = getShellConfiguration(); + expect(config.executable).toBe('C:\\Path\\To\\POWERSHELL.EXE'); + expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']); + expect(config.shell).toBe('powershell'); + }); + }); +}); diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 4164cdca..2c818c8e 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -5,6 +5,102 @@ */ import { Config } from '../config/config.js'; +import os from 'os'; +import { quote } from 'shell-quote'; + +/** + * An identifier for the shell type. + */ +export type ShellType = 'cmd' | 'powershell' | 'bash'; + +/** + * Defines the configuration required to execute a command string within a specific shell. + */ +export interface ShellConfiguration { + /** The path or name of the shell executable (e.g., 'bash', 'cmd.exe'). */ + executable: string; + /** + * The arguments required by the shell to execute a subsequent string argument. + */ + argsPrefix: string[]; + /** An identifier for the shell type. */ + shell: ShellType; +} + +/** + * Determines the appropriate shell configuration for the current platform. + * + * This ensures we can execute command strings predictably and securely across platforms + * using the `spawn(executable, [...argsPrefix, commandString], { shell: false })` pattern. + * + * @returns The ShellConfiguration for the current environment. + */ +export function getShellConfiguration(): ShellConfiguration { + if (isWindows()) { + const comSpec = process.env.ComSpec || 'cmd.exe'; + const executable = comSpec.toLowerCase(); + + if ( + executable.endsWith('powershell.exe') || + executable.endsWith('pwsh.exe') + ) { + // For PowerShell, the arguments are different. + // -NoProfile: Speeds up startup. + // -Command: Executes the following command. + return { + executable: comSpec, + argsPrefix: ['-NoProfile', '-Command'], + shell: 'powershell', + }; + } + + // Default to cmd.exe for anything else on Windows. + // Flags for CMD: + // /d: Skip execution of AutoRun commands. + // /s: Modifies the treatment of the command string (important for quoting). + // /c: Carries out the command specified by the string and then terminates. + return { + executable: comSpec, + argsPrefix: ['/d', '/s', '/c'], + shell: 'cmd', + }; + } + + // Unix-like systems (Linux, macOS) + return { executable: 'bash', argsPrefix: ['-c'], shell: 'bash' }; +} + +/** + * Export the platform detection constant for use in process management (e.g., killing processes). + */ +export const isWindows = () => os.platform() === 'win32'; + +/** + * Escapes a string so that it can be safely used as a single argument + * in a shell command, preventing command injection. + * + * @param arg The argument string to escape. + * @param shell The type of shell the argument is for. + * @returns The shell-escaped string. + */ +export function escapeShellArg(arg: string, shell: ShellType): string { + if (!arg) { + return ''; + } + + switch (shell) { + case 'powershell': + // For PowerShell, wrap in single quotes and escape internal single quotes by doubling them. + return `'${arg.replace(/'/g, "''")}'`; + case 'cmd': + // Simple Windows escaping for cmd.exe: wrap in double quotes and escape inner double quotes. + return `"${arg.replace(/"/g, '""')}"`; + case 'bash': + default: + // POSIX shell escaping using shell-quote. + return quote([arg]); + } +} /** * Splits a shell command into a list of individual commands, respecting quotes. |
