Skip to content
18 changes: 15 additions & 3 deletions packages/core/src/utils/shell-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,15 @@ vi.mock('node:child_process', () => ({
}));

const mockQuote = vi.hoisted(() => vi.fn());
vi.mock('shell-quote', () => ({
quote: mockQuote,
}));
const mockParse = vi.hoisted(() => vi.fn());
vi.mock('shell-quote', async (importOriginal) => {
const actual = await importOriginal<typeof import('shell-quote')>();
return {
...actual,
quote: mockQuote.mockImplementation(actual.quote),
parse: mockParse.mockImplementation(actual.parse),
};
});
Comment thread
sanchezcoraspe marked this conversation as resolved.

const mockDebugLogger = vi.hoisted(() => ({
error: vi.fn(),
Expand Down Expand Up @@ -388,6 +394,12 @@ describe('stripShellWrapper', () => {
it('should not strip anything if no wrapper is present', () => {
expect(stripShellWrapper('ls -l')).toEqual('ls -l');
});

it('should handle multi-line escaped double quotes correctly', () => {
const multiLine = 'bash -c "hg commit -m \\"title\n\nbody\\""';
const expected = 'hg commit -m "title\n\nbody"';
expect(stripShellWrapper(multiLine)).toEqual(expected);
});
});

describe('escapeShellArg', () => {
Expand Down
37 changes: 27 additions & 10 deletions packages/core/src/utils/shell-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os from 'node:os';
import fs from 'node:fs';
import path from 'node:path';
import { quote, type ParseEntry } from 'shell-quote';
import { quote, parse, type ParseEntry } from 'shell-quote';
Comment thread
sanchezcoraspe marked this conversation as resolved.
import {
spawn,
spawnSync,
Expand Down Expand Up @@ -62,6 +62,10 @@ export const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
*/
export type ShellType = 'cmd' | 'powershell' | 'bash';

function isString(value: unknown): value is string {
return typeof value === 'string';
}

/**
* Defines the configuration required to execute a command string within a specific shell.
*/
Expand Down Expand Up @@ -580,15 +584,14 @@ function parsePowerShellCommandDetails(

const details = (parsed.commands ?? [])
.map((commandDetail): ParsedCommandDetail | null => {
if (!commandDetail || typeof commandDetail.name !== 'string') {
if (!commandDetail || !isString(commandDetail.name)) {
return null;
}

const name = normalizeCommandName(commandDetail.name);
const text =
typeof commandDetail.text === 'string'
? commandDetail.text.trim()
: command;
const text = isString(commandDetail.text)
? commandDetail.text.trim()
: command;

return {
name,
Expand Down Expand Up @@ -846,10 +849,24 @@ export function stripShellWrapper(command: string): string {
if (match) {
let newCommand = command.substring(match[0].length).trim();
if (
(newCommand.startsWith('"') && newCommand.endsWith('"')) ||
(newCommand.startsWith("'") && newCommand.endsWith("'"))
newCommand.length >= 2 &&
((newCommand.startsWith('"') && newCommand.endsWith('"')) ||
(newCommand.startsWith("'") && newCommand.endsWith("'")))
) {
newCommand = newCommand.substring(1, newCommand.length - 1);
const isWindowsShell = /cmd(?:\.exe)?|powershell|pwsh/i.test(match[0]);
if (!isWindowsShell) {
try {
const parsed = parse(newCommand, (key) => '$' + key);
const firstEntry = parsed[0];
if (parsed.length === 1 && isString(firstEntry)) {
newCommand = firstEntry;
}
} catch {
// If parsing fails, we leave the command as is to avoid corruption
}
} else {
newCommand = newCommand.substring(1, newCommand.length - 1);
}
Comment thread
sanchezcoraspe marked this conversation as resolved.
Outdated
}
return newCommand;
}
Expand Down Expand Up @@ -1067,7 +1084,7 @@ export async function* execStreaming(
export function detectCommandSubstitution(command: string): boolean {
const shell = getShellConfiguration().shell;
const isPowerShell =
typeof shell === 'string' &&
isString(shell) &&
(shell.toLowerCase().includes('powershell') ||
shell.toLowerCase().includes('pwsh'));
if (isPowerShell) {
Expand Down