diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 9f83b00bb6..dd49a9c800 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -210,7 +210,7 @@ describe('ShellTool', () => { mockShellOutputCallback = callback; const match = cmd.match(/pgrep -g 0 >([^ ]+)/); if (match) { - extractedTmpFile = match[1].replace(/['"]/g, ''); // remove any quotes if present + extractedTmpFile = match[1].replace(/['"]/g, ''); } return { pid: 12345, @@ -994,7 +994,6 @@ EOF`; const result = await promise; expect(result.llmContent).not.toContain('Process Group PGID:'); }); - it('should have minimal output for successful command', async () => { const invocation = shellTool.build({ command: 'echo hello' }); const promise = invocation.execute({ abortSignal: mockAbortSignal }); @@ -1222,4 +1221,399 @@ EOF`; expect(schema.description).toMatchSnapshot(); }); }); + + describe('command injection detection', () => { + it('should block $() command substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo $(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block backtick command substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo `whoami`' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow normal commands without substitution', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'hello', + rawOutput: Buffer.from('hello'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo hello' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow single quoted strings with special chars', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(not substituted)', + rawOutput: Buffer.from('$(not substituted)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(not substituted)'", + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow escaped backtick outside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'hello', + rawOutput: Buffer.from('hello'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo \\`hello\\`' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should block $() inside double quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "$(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block >() process substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo >(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow $() inside single quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(whoami)'", + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + it('should block PowerShell @() array subexpression', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo @(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block PowerShell $() subexpression', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo $(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow PowerShell single quoted strings', async () => { + mockPlatform.mockReturnValue('win32'); + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(whoami)'", + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + it('should allow escaped substitution outside quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo \\$(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow process substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '<(whoami)', + rawOutput: Buffer.from('<(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "<(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should block process substitution without quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo <(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow escaped $() outside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo \\$(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow output process substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '<(whoami)', + rawOutput: Buffer.from('<(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "<(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should block <() process substitution without quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo <(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + it('should block PowerShell bare () grouping operator', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo (whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow escaped $() inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "\\$(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow escaped substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "\\$(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow PowerShell keyword with flag e.g. switch -regex ($x)', async () => { + mockPlatform.mockReturnValue('win32'); + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'result', + rawOutput: Buffer.from('result'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: 'switch -regex ($x) { "a" { 1 } }', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow PowerShell nested parentheses e.g. if ((condition))', async () => { + mockPlatform.mockReturnValue('win32'); + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'result', + rawOutput: Buffer.from('result'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: 'if ((condition)) { Write-Host ok }', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index a2cb44aba0..7be9a4f26f 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -40,6 +40,7 @@ import { stripShellWrapper, parseCommandDetails, hasRedirection, + detectCommandSubstitution, normalizeCommand, escapeShellArg, } from '../utils/shell-utils.js'; @@ -443,6 +444,18 @@ export class ShellToolInvocation extends BaseToolInvocation< } = options; const strippedCommand = stripShellWrapper(this.params.command); + if (detectCommandSubstitution(strippedCommand)) { + return { + llmContent: + 'Command injection detected: command substitution syntax ' + + '($(), backticks, <() or >()) found in command arguments. ' + + 'On PowerShell, @() array subexpressions and $() subexpressions are also blocked. ' + + 'This is a security risk and the command was blocked.', + returnDisplay: + 'Blocked: command substitution detected in shell command.', + }; + } + if (signal.aborted) { return { llmContent: 'Command was cancelled by user before it could start.', diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 46cffa1d35..6f02579df9 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -1020,3 +1020,119 @@ export async function* execStreaming( prepared.cleanup?.(); } } + +export function detectCommandSubstitution(command: string): boolean { + const shell = getShellConfiguration().shell; + const isPowerShell = + typeof shell === 'string' && + (shell.toLowerCase().includes('powershell') || + shell.toLowerCase().includes('pwsh')); + if (isPowerShell) { + return detectPowerShellSubstitution(command); + } + return detectBashSubstitution(command); +} + +function detectBashSubstitution(command: string): boolean { + let inSingleQuote = false; + let inDoubleQuote = false; + let i = 0; + while (i < command.length) { + const char = command[i]; + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + i++; + continue; + } + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + i++; + continue; + } + if (inSingleQuote) { + i++; + continue; + } + if (char === '\\' && i + 1 < command.length) { + if (inDoubleQuote) { + const next = command[i + 1]; + if (['$', '`', '"', '\\', '\n'].includes(next)) { + i += 2; + continue; + } + } else { + i += 2; + continue; + } + } + if (char === '$' && command[i + 1] === '(') { + return true; + } + if ( + !inDoubleQuote && + (char === '<' || char === '>') && + command[i + 1] === '(' + ) { + return true; + } + if (char === '`') { + return true; + } + i++; + } + return false; +} + +const POWERSHELL_KEYWORD_RE = + /\b(if|elseif|else|foreach|for|while|do|switch|try|catch|finally|until|trap|function|filter)(\s+[-\w]+)*\s*$/i; + +function detectPowerShellSubstitution(command: string): boolean { + let inSingleQuote = false; + let inDoubleQuote = false; + let i = 0; + while (i < command.length) { + const char = command[i]; + + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + i++; + continue; + } + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + i++; + continue; + } + + if (inSingleQuote) { + i++; + continue; + } + if (char === '`' && i + 1 < command.length) { + i += 2; + continue; + } + if (char === '$' && command[i + 1] === '(') { + return true; + } + if (!inDoubleQuote && char === '@' && command[i + 1] === '(') { + return true; + } + if (!inDoubleQuote && char === '(') { + const before = command.slice(0, i).trimEnd(); + const prevChar = before[before.length - 1]; + if (prevChar === '(') { + i++; + continue; + } + if (POWERSHELL_KEYWORD_RE.test(before)) { + i++; + continue; + } + return true; + } + + i++; + } + return false; +}