diff --git a/docs/cli/commands.md b/docs/cli/commands.md index b5b25a682a7..01eafb77b85 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -292,8 +292,9 @@ Gemini CLI. - **`!`** - **Description:** Execute the given `` using `bash` on - Linux/macOS or `cmd.exe` on Windows. Any output or errors from the command - are displayed in the terminal. + Linux/macOS or `powershell.exe -NoProfile -Command` on Windows (unless you + override `ComSpec`). Any output or errors from the command are displayed in + the terminal. - **Examples:** - `!ls -la` (executes `ls -la` and returns to Gemini CLI) - `!git status` (executes `git status` and returns to Gemini CLI) diff --git a/docs/tools/shell.md b/docs/tools/shell.md index f5ef32ba417..3802c10d3f0 100644 --- a/docs/tools/shell.md +++ b/docs/tools/shell.md @@ -10,8 +10,9 @@ command, including interactive commands that require user input (e.g., `vim`, `git rebase -i`) if the `tools.shell.enableInteractiveShell` setting is set to `true`. -On Windows, commands are executed with `cmd.exe /c`. On other platforms, they -are executed with `bash -c`. +On Windows, commands are executed with `powershell.exe -NoProfile -Command` +(unless you explicitly point `ComSpec` at another shell). On other platforms, +they are executed with `bash -c`. ### Arguments diff --git a/esbuild.config.js b/esbuild.config.js index d5e10e29692..2b13adcbbb6 100644 --- a/esbuild.config.js +++ b/esbuild.config.js @@ -8,6 +8,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { createRequire } from 'node:module'; import { writeFileSync } from 'node:fs'; +import { wasmLoader } from 'esbuild-plugin-wasm'; let esbuild; try { @@ -22,6 +23,37 @@ const __dirname = path.dirname(__filename); const require = createRequire(import.meta.url); const pkg = require(path.resolve(__dirname, 'package.json')); +function createWasmPlugins() { + const wasmBinaryPlugin = { + name: 'wasm-binary', + setup(build) { + build.onResolve({ filter: /\.wasm\?binary$/ }, (args) => { + const specifier = args.path.replace(/\?binary$/, ''); + const resolveDir = args.resolveDir || ''; + const isBareSpecifier = + !path.isAbsolute(specifier) && + !specifier.startsWith('./') && + !specifier.startsWith('../'); + + let resolvedPath; + if (isBareSpecifier) { + resolvedPath = require.resolve(specifier, { + paths: resolveDir ? [resolveDir, __dirname] : [__dirname], + }); + } else { + resolvedPath = path.isAbsolute(specifier) + ? specifier + : path.join(resolveDir, specifier); + } + + return { path: resolvedPath, namespace: 'wasm-embedded' }; + }); + }, + }; + + return [wasmBinaryPlugin, wasmLoader({ mode: 'embedded' })]; +} + const external = [ '@lydell/node-pty', 'node-pty', @@ -51,6 +83,7 @@ const cliConfig = { define: { 'process.env.CLI_VERSION': JSON.stringify(pkg.version), }, + plugins: createWasmPlugins(), alias: { 'is-in-ci': path.resolve(__dirname, 'packages/cli/src/patches/is-in-ci.ts'), }, @@ -67,6 +100,7 @@ const a2aServerConfig = { define: { 'process.env.CLI_VERSION': JSON.stringify(pkg.version), }, + plugins: createWasmPlugins(), }; Promise.allSettled([ diff --git a/integration-tests/flicker.test.ts b/integration-tests/flicker.test.ts index 730cde86f3b..a7ed9fcf2e9 100644 --- a/integration-tests/flicker.test.ts +++ b/integration-tests/flicker.test.ts @@ -8,7 +8,8 @@ import { describe, it, expect } from 'vitest'; import { TestRig } from './test-helper.js'; describe('Flicker Detector', () => { - it('should not detect a flicker under the max height budget', async () => { + // TODO: https://github.com/google-gemini/gemini-cli/issues/11170 + it.skip('should not detect a flicker under the max height budget', async () => { const rig = new TestRig(); await rig.setup('flicker-detector-test'); diff --git a/integration-tests/run_shell_command.test.ts b/integration-tests/run_shell_command.test.ts index a0aab141939..06d94f7113e 100644 --- a/integration-tests/run_shell_command.test.ts +++ b/integration-tests/run_shell_command.test.ts @@ -21,6 +21,46 @@ function getLineCountCommand(): { command: string; tool: string } { } } +function getInvalidCommand(): string { + switch (shell) { + case 'powershell': + return `Get-ChildItem | | Select-Object`; + case 'cmd': + return `dir | | findstr foo`; + case 'bash': + default: + return `echo "hello" > > file`; + } +} + +function getAllowedListCommand(): string { + switch (shell) { + case 'powershell': + return 'Get-ChildItem'; + case 'cmd': + return 'dir'; + case 'bash': + default: + return 'ls'; + } +} + +function getDisallowedFileReadCommand(testFile: string): { + command: string; + tool: string; +} { + const quotedPath = `"${testFile}"`; + switch (shell) { + case 'powershell': + return { command: `Get-Content ${quotedPath}`, tool: 'Get-Content' }; + case 'cmd': + return { command: `type ${quotedPath}`, tool: 'type' }; + case 'bash': + default: + return { command: `cat ${quotedPath}`, tool: 'cat' }; + } +} + describe('run_shell_command', () => { it('should be able to run a shell command', async () => { const rig = new TestRig(); @@ -102,8 +142,17 @@ describe('run_shell_command', () => { const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000); if (!foundToolCall) { + const toolLogs = rig.readToolLogs().map(({ toolRequest }) => ({ + name: toolRequest.name, + success: toolRequest.success, + args: toolRequest.args, + })); printDebugInfo(rig, result, { 'Found tool call': foundToolCall, + 'Allowed tools flag': `run_shell_command(${tool})`, + Prompt: prompt, + 'Tool logs': toolLogs, + Result: result, }); } @@ -210,8 +259,17 @@ describe('run_shell_command', () => { const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000); if (!foundToolCall) { + const toolLogs = rig.readToolLogs().map(({ toolRequest }) => ({ + name: toolRequest.name, + success: toolRequest.success, + args: toolRequest.args, + })); printDebugInfo(rig, result, { 'Found tool call': foundToolCall, + 'Allowed tools flag': `ShellTool(${tool})`, + Prompt: prompt, + 'Tool logs': toolLogs, + Result: result, }); } @@ -280,6 +338,73 @@ describe('run_shell_command', () => { } }); + it('should reject commands not on the allowlist', async () => { + const rig = new TestRig(); + await rig.setup('should reject commands not on the allowlist'); + + const testFile = rig.createFile('test.txt', 'Disallowed command check\n'); + const allowedCommand = getAllowedListCommand(); + const disallowed = getDisallowedFileReadCommand(testFile); + const prompt = + `I am testing the allowed tools configuration. ` + + `Attempt to run "${disallowed.command}" to read the contents of ${testFile}. ` + + `If the command fails because it is not permitted, respond with the single word FAIL. ` + + `If it succeeds, respond with SUCCESS.`; + + const result = await rig.run( + { + stdin: prompt, + yolo: false, + }, + `--allowed-tools=run_shell_command(${allowedCommand})`, + ); + + if (!result.toLowerCase().includes('fail')) { + printDebugInfo(rig, result, { + Result: result, + AllowedCommand: allowedCommand, + DisallowedCommand: disallowed.command, + }); + } + expect(result).toContain('FAIL'); + + const foundToolCall = await rig.waitForToolCall( + 'run_shell_command', + 15000, + (args) => args.toLowerCase().includes(disallowed.tool.toLowerCase()), + ); + + if (!foundToolCall) { + printDebugInfo(rig, result, { + 'Found tool call': foundToolCall, + ToolLogs: rig.readToolLogs(), + }); + } + expect(foundToolCall).toBe(true); + + const toolLogs = rig + .readToolLogs() + .filter((toolLog) => toolLog.toolRequest.name === 'run_shell_command'); + const failureLog = toolLogs.find((toolLog) => + toolLog.toolRequest.args + .toLowerCase() + .includes(disallowed.tool.toLowerCase()), + ); + + if (!failureLog || failureLog.toolRequest.success) { + printDebugInfo(rig, result, { + ToolLogs: toolLogs, + DisallowedTool: disallowed.tool, + }); + } + + expect( + failureLog, + 'Expected failing run_shell_command invocation', + ).toBeTruthy(); + expect(failureLog!.toolRequest.success).toBe(false); + }); + it('should allow all with "ShellTool" and other specific tools', async () => { const rig = new TestRig(); await rig.setup( @@ -386,4 +511,53 @@ describe('run_shell_command', () => { validateModelOutput(result, fileName, 'Platform-specific listing test'); expect(result).toContain(fileName); }); + + it('rejects invalid shell expressions', async () => { + const rig = new TestRig(); + await rig.setup('rejects invalid shell expressions'); + const invalidCommand = getInvalidCommand(); + const result = await rig.run( + `I am testing the error handling of the run_shell_command tool. Please attempt to run the following command, which I know has invalid syntax: \`${invalidCommand}\`. If the command fails as expected, please return the word FAIL, otherwise return the word SUCCESS.`, + ); + expect(result).toContain('FAIL'); + + const escapedInvalidCommand = JSON.stringify(invalidCommand).slice(1, -1); + const foundToolCall = await rig.waitForToolCall( + 'run_shell_command', + 15000, + (args) => + args.toLowerCase().includes(escapedInvalidCommand.toLowerCase()), + ); + + if (!foundToolCall) { + printDebugInfo(rig, result, { + 'Found tool call': foundToolCall, + EscapedCommand: escapedInvalidCommand, + ToolLogs: rig.readToolLogs(), + }); + } + expect(foundToolCall).toBe(true); + + const toolLogs = rig + .readToolLogs() + .filter((toolLog) => toolLog.toolRequest.name === 'run_shell_command'); + const failureLog = toolLogs.find((toolLog) => + toolLog.toolRequest.args + .toLowerCase() + .includes(escapedInvalidCommand.toLowerCase()), + ); + + if (!failureLog || failureLog.toolRequest.success) { + printDebugInfo(rig, result, { + ToolLogs: toolLogs, + EscapedCommand: escapedInvalidCommand, + }); + } + + expect( + failureLog, + 'Expected failing run_shell_command invocation for invalid syntax', + ).toBeTruthy(); + expect(failureLog!.toolRequest.success).toBe(false); + }); }); diff --git a/package-lock.json b/package-lock.json index 024718f647d..2c3edcc5616 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,6 +28,7 @@ "@vitest/eslint-plugin": "^1.3.4", "cross-env": "^7.0.3", "esbuild": "^0.25.0", + "esbuild-plugin-wasm": "^1.1.0", "eslint": "^9.24.0", "eslint-config-prettier": "^10.1.2", "eslint-plugin-import": "^2.31.0", @@ -8040,6 +8041,20 @@ "@esbuild/win32-x64": "0.25.6" } }, + "node_modules/esbuild-plugin-wasm": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/esbuild-plugin-wasm/-/esbuild-plugin-wasm-1.1.0.tgz", + "integrity": "sha512-0bQ6+1tUbySSnxzn5jnXHMDvYnT0cN/Wd4Syk8g/sqAIJUg7buTIi22svS3Qz6ssx895NT+TgLPb33xi1OkZig==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.10.0" + }, + "funding": { + "type": "individual", + "url": "https://ko-fi.com/tschrock" + } + }, "node_modules/escalade": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/escalade/-/escalade-3.2.0.tgz", @@ -12438,6 +12453,17 @@ "webidl-conversions": "^3.0.0" } }, + "node_modules/node-gyp-build": { + "version": "4.8.4", + "resolved": "https://registry.npmjs.org/node-gyp-build/-/node-gyp-build-4.8.4.tgz", + "integrity": "sha512-LA4ZjwlnUblHVgq0oBF3Jl/6h/Nvs5fzBLwdEF4nuxnFdsfajde4WfxtJr3CaiH+F6ewcIB/q4jQ4UzPyid+CQ==", + "license": "MIT", + "bin": { + "node-gyp-build": "bin.js", + "node-gyp-build-optional": "optional.js", + "node-gyp-build-test": "build-test.js" + } + }, "node_modules/node-pty": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/node-pty/-/node-pty-1.0.0.tgz", @@ -16154,6 +16180,34 @@ "tslib": "2" } }, + "node_modules/tree-sitter-bash": { + "version": "0.25.0", + "resolved": "https://registry.npmjs.org/tree-sitter-bash/-/tree-sitter-bash-0.25.0.tgz", + "integrity": "sha512-gZtlj9+qFS81qKxpLfD6H0UssQ3QBc/F0nKkPsiFDyfQF2YBqYvglFJUzchrPpVhZe9kLZTrJ9n2J6lmka69Vg==", + "hasInstallScript": true, + "license": "MIT", + "dependencies": { + "node-addon-api": "^8.2.1", + "node-gyp-build": "^4.8.2" + }, + "peerDependencies": { + "tree-sitter": "^0.25.0" + }, + "peerDependenciesMeta": { + "tree-sitter": { + "optional": true + } + } + }, + "node_modules/tree-sitter-bash/node_modules/node-addon-api": { + "version": "8.5.0", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.5.0.tgz", + "integrity": "sha512-/bRZty2mXUIFY/xU5HLvveNHlswNJej+RnxBjOMkidWfwZzgTbPG1E3K5TOxRLOR+5hX7bSofy8yf1hZevMS8A==", + "license": "MIT", + "engines": { + "node": "^18 || ^20 || >= 21" + } + }, "node_modules/triple-beam": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/triple-beam/-/triple-beam-1.4.1.tgz", @@ -16950,6 +17004,20 @@ "node": ">=18" } }, + "node_modules/web-tree-sitter": { + "version": "0.25.10", + "resolved": "https://registry.npmjs.org/web-tree-sitter/-/web-tree-sitter-0.25.10.tgz", + "integrity": "sha512-Y09sF44/13XvgVKgO2cNDw5rGk6s26MgoZPXLESvMXeefBf7i6/73eFurre0IsTW6E14Y0ArIzhUMmjoc7xyzA==", + "license": "MIT", + "peerDependencies": { + "@types/emscripten": "^1.40.0" + }, + "peerDependenciesMeta": { + "@types/emscripten": { + "optional": true + } + } + }, "node_modules/webidl-conversions": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-7.0.0.tgz", @@ -18046,7 +18114,9 @@ "shell-quote": "^1.8.3", "simple-git": "^3.28.0", "strip-ansi": "^7.1.0", + "tree-sitter-bash": "^0.25.0", "undici": "^7.10.0", + "web-tree-sitter": "^0.25.10", "ws": "^8.18.0" }, "devDependencies": { diff --git a/package.json b/package.json index 1b4efaf47e8..9d13f5dbd4c 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "@vitest/eslint-plugin": "^1.3.4", "cross-env": "^7.0.3", "esbuild": "^0.25.0", + "esbuild-plugin-wasm": "^1.1.0", "eslint": "^9.24.0", "eslint-config-prettier": "^10.1.2", "eslint-plugin-import": "^2.31.0", diff --git a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts index e4021b54db9..2c93ecf8c0d 100644 --- a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts +++ b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts @@ -9,8 +9,7 @@ import { ConfirmationRequiredError, ShellProcessor } from './shellProcessor.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; import type { CommandContext } from '../../ui/commands/types.js'; import type { Config } from '@google/gemini-cli-core'; -import { ApprovalMode } from '@google/gemini-cli-core'; -import os from 'node:os'; +import { ApprovalMode, getShellConfiguration } from '@google/gemini-cli-core'; import { quote } from 'shell-quote'; import { createPartFromText } from '@google/genai'; import type { PromptPipelineContent } from './types.js'; @@ -18,18 +17,16 @@ import type { PromptPipelineContent } from './types.js'; // Helper function to determine the expected escaped string based on the current OS, // mirroring the logic in the actual `escapeShellArg` implementation. function getExpectedEscapedArgForPlatform(arg: string): string { - if (os.platform() === 'win32') { - const comSpec = (process.env['ComSpec'] || 'cmd.exe').toLowerCase(); - const isPowerShell = - comSpec.endsWith('powershell.exe') || comSpec.endsWith('pwsh.exe'); + const { shell } = getShellConfiguration(); - if (isPowerShell) { + switch (shell) { + case 'powershell': return `'${arg.replace(/'/g, "''")}'`; - } else { + case 'cmd': return `"${arg.replace(/"/g, '""')}"`; - } - } else { - return quote([arg]); + case 'bash': + default: + return quote([arg]); } } diff --git a/packages/core/package.json b/packages/core/package.json index 1052c1648b9..e3d73e2ddf7 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -20,10 +20,10 @@ "dist" ], "dependencies": { - "@google/genai": "1.16.0", + "@google-cloud/logging": "^11.2.1", "@google-cloud/opentelemetry-cloud-monitoring-exporter": "^0.21.0", "@google-cloud/opentelemetry-cloud-trace-exporter": "^3.0.0", - "@google-cloud/logging": "^11.2.1", + "@google/genai": "1.16.0", "@joshua.litt/get-ripgrep": "^0.0.2", "@modelcontextprotocol/sdk": "^1.11.0", "@opentelemetry/api": "^1.9.0", @@ -61,7 +61,9 @@ "shell-quote": "^1.8.3", "simple-git": "^3.28.0", "strip-ansi": "^7.1.0", + "tree-sitter-bash": "^0.25.0", "undici": "^7.10.0", + "web-tree-sitter": "^0.25.10", "ws": "^8.18.0" }, "optionalDependencies": { diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index b8457e40f82..45aa9fce97c 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -24,9 +24,14 @@ const mockSerializeTerminalToObject = vi.hoisted(() => vi.fn()); vi.mock('@lydell/node-pty', () => ({ spawn: mockPtySpawn, })); -vi.mock('child_process', () => ({ - spawn: mockCpSpawn, -})); +vi.mock('node:child_process', async (importOriginal) => { + const actual = + (await importOriginal()) as typeof import('node:child_process'); + return { + ...actual, + spawn: mockCpSpawn, + }; +}); vi.mock('../utils/textUtils.js', () => ({ isBinary: mockIsBinary, })); @@ -465,15 +470,15 @@ describe('ShellExecutionService', () => { }); describe('Platform-Specific Behavior', () => { - it('should use cmd.exe on Windows', async () => { + it('should use powershell.exe on Windows', async () => { mockPlatform.mockReturnValue('win32'); await simulateExecution('dir "foo bar"', (pty) => pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }), ); expect(mockPtySpawn).toHaveBeenCalledWith( - 'cmd.exe', - '/c dir "foo bar"', + 'powershell.exe', + ['-NoProfile', '-Command', 'dir "foo bar"'], expect.any(Object), ); }); @@ -637,9 +642,9 @@ describe('ShellExecutionService child_process fallback', () => { }); expect(mockCpSpawn).toHaveBeenCalledWith( - 'ls -l', - [], - expect.objectContaining({ shell: 'bash' }), + 'bash', + ['-c', 'ls -l'], + expect.objectContaining({ shell: false, detached: true }), ); expect(result.exitCode).toBe(0); expect(result.signal).toBeNull(); @@ -905,18 +910,19 @@ describe('ShellExecutionService child_process fallback', () => { }); describe('Platform-Specific Behavior', () => { - it('should use cmd.exe on Windows', async () => { + it('should use powershell.exe on Windows', async () => { mockPlatform.mockReturnValue('win32'); await simulateExecution('dir "foo bar"', (cp) => cp.emit('exit', 0, null), ); expect(mockCpSpawn).toHaveBeenCalledWith( - 'dir "foo bar"', - [], + 'powershell.exe', + ['-NoProfile', '-Command', 'dir "foo bar"'], expect.objectContaining({ - shell: true, + shell: false, detached: false, + windowsVerbatimArguments: false, }), ); }); @@ -926,10 +932,10 @@ describe('ShellExecutionService child_process fallback', () => { await simulateExecution('ls "foo bar"', (cp) => cp.emit('exit', 0, null)); expect(mockCpSpawn).toHaveBeenCalledWith( - 'ls "foo bar"', - [], + 'bash', + ['-c', 'ls "foo bar"'], expect.objectContaining({ - shell: 'bash', + shell: false, detached: true, }), ); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index d76fc56ee20..f3bdddbc55b 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -12,6 +12,7 @@ import { TextDecoder } from 'node:util'; import os from 'node:os'; import type { IPty } from '@lydell/node-pty'; import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js'; +import { getShellConfiguration } from '../utils/shell-utils.js'; import { isBinary } from '../utils/textUtils.js'; import pkg from '@xterm/headless'; import { @@ -189,12 +190,14 @@ export class ShellExecutionService { ): ShellExecutionHandle { try { const isWindows = os.platform() === 'win32'; + const { executable, argsPrefix } = getShellConfiguration(); + const spawnArgs = [...argsPrefix, commandToExecute]; - const child = cpSpawn(commandToExecute, [], { + const child = cpSpawn(executable, spawnArgs, { cwd, stdio: ['ignore', 'pipe', 'pipe'], - windowsVerbatimArguments: true, - shell: isWindows ? true : 'bash', + windowsVerbatimArguments: isWindows ? false : undefined, + shell: false, detached: !isWindows, env: { ...process.env, @@ -400,13 +403,10 @@ export class ShellExecutionService { try { const cols = shellExecutionConfig.terminalWidth ?? 80; const rows = shellExecutionConfig.terminalHeight ?? 30; - const isWindows = os.platform() === 'win32'; - const shell = isWindows ? 'cmd.exe' : 'bash'; - const args = isWindows - ? `/c ${commandToExecute}` - : ['-c', commandToExecute]; + const { executable, argsPrefix } = getShellConfiguration(); + const args = [...argsPrefix, commandToExecute]; - const ptyProcess = ptyInfo.module.spawn(shell, args, { + const ptyProcess = ptyInfo.module.spawn(executable, args, { cwd, name: 'xterm', cols, diff --git a/packages/core/src/tools/__snapshots__/shell.test.ts.snap b/packages/core/src/tools/__snapshots__/shell.test.ts.snap index 1579cb97169..76a5ded3ef4 100644 --- a/packages/core/src/tools/__snapshots__/shell.test.ts.snap +++ b/packages/core/src/tools/__snapshots__/shell.test.ts.snap @@ -17,7 +17,7 @@ exports[`ShellTool > getDescription > should return the non-windows description `; exports[`ShellTool > getDescription > should return the windows description when on windows 1`] = ` -"This tool executes a given shell command as \`cmd.exe /c \`. Command can start background processes using \`start /b\`. +"This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`. The following information is returned: diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 7c861eeac46..a45183af7a7 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect, + beforeAll, beforeEach, afterEach, type Mock, @@ -23,7 +24,10 @@ vi.mock('os'); vi.mock('crypto'); vi.mock('../utils/summarizer.js'); -import { isCommandAllowed } from '../utils/shell-utils.js'; +import { + initializeShellParsers, + isCommandAllowed, +} from '../utils/shell-utils.js'; import { ShellTool } from './shell.js'; import { type Config } from '../config/config.js'; import { @@ -41,6 +45,9 @@ import { ToolConfirmationOutcome } from './tools.js'; import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; +const originalComSpec = process.env['ComSpec']; +const itWindowsOnly = process.platform === 'win32' ? it : it.skip; + describe('ShellTool', () => { let shellTool: ShellTool; let mockConfig: Config; @@ -73,6 +80,8 @@ describe('ShellTool', () => { (vi.mocked(crypto.randomBytes) as Mock).mockReturnValue( Buffer.from('abcdef', 'hex'), ); + process.env['ComSpec'] = + 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'; // Capture the output callback to simulate streaming events from the service mockShellExecutionService.mockImplementation((_cmd, _cwd, callback) => { @@ -86,23 +95,36 @@ describe('ShellTool', () => { }); }); + afterEach(() => { + if (originalComSpec === undefined) { + delete process.env['ComSpec']; + } else { + process.env['ComSpec'] = originalComSpec; + } + }); + describe('isCommandAllowed', () => { it('should allow a command if no restrictions are provided', () => { (mockConfig.getCoreTools as Mock).mockReturnValue(undefined); (mockConfig.getExcludeTools as Mock).mockReturnValue(undefined); - expect(isCommandAllowed('ls -l', mockConfig).allowed).toBe(true); + expect(isCommandAllowed('goodCommand --safe', mockConfig).allowed).toBe( + true, + ); }); - it('should block a command with command substitution using $()', () => { - expect(isCommandAllowed('echo $(rm -rf /)', mockConfig).allowed).toBe( - false, + it('should allow a command with command substitution using $()', () => { + const evaluation = isCommandAllowed( + 'echo $(goodCommand --safe)', + mockConfig, ); + expect(evaluation.allowed).toBe(true); + expect(evaluation.reason).toBeUndefined(); }); }); describe('build', () => { it('should return an invocation for a valid command', () => { - const invocation = shellTool.build({ command: 'ls -l' }); + const invocation = shellTool.build({ command: 'goodCommand --safe' }); expect(invocation).toBeDefined(); }); @@ -209,7 +231,7 @@ describe('ShellTool', () => { ); }); - it('should not wrap command on windows', async () => { + itWindowsOnly('should not wrap command on windows', async () => { vi.mocked(os.platform).mockReturnValue('win32'); const invocation = shellTool.build({ command: 'dir' }); const promise = invocation.execute(mockAbortSignal); @@ -474,3 +496,6 @@ describe('ShellTool', () => { }); }); }); +beforeAll(async () => { + await initializeShellParsers(); +}); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 29a75b024b1..df1d8283068 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -34,6 +34,7 @@ import { formatMemoryUsage } from '../utils/formatters.js'; import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { getCommandRoots, + initializeShellParsers, isCommandAllowed, SHELL_TOOL_NAMES, stripShellWrapper, @@ -340,25 +341,17 @@ function getShellToolDescription(): string { Process Group PGID: Process group started or \`(none)\``; if (os.platform() === 'win32') { - return `This tool executes a given shell command as \`cmd.exe /c \`. Command can start background processes using \`start /b\`.${returnedInfo}`; + return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`.${returnedInfo}`; } else { return `This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${returnedInfo}`; } } function getCommandDescription(): string { - const cmd_substitution_warning = - '\n*** WARNING: Command substitution using $(), `` ` ``, <(), or >() is not allowed for security reasons.'; if (os.platform() === 'win32') { - return ( - 'Exact command to execute as `cmd.exe /c `' + - cmd_substitution_warning - ); + return 'Exact command to execute as `powershell.exe -NoProfile -Command `'; } else { - return ( - 'Exact bash command to execute as `bash -c `' + - cmd_substitution_warning - ); + return 'Exact bash command to execute as `bash -c `'; } } @@ -370,6 +363,9 @@ export class ShellTool extends BaseDeclarativeTool< private allowlist: Set = new Set(); constructor(private readonly config: Config) { + void initializeShellParsers().catch(() => { + // Errors are surfaced when parsing commands. + }); super( ShellTool.Name, 'Shell', @@ -403,6 +399,10 @@ export class ShellTool extends BaseDeclarativeTool< protected override validateToolParamValues( params: ShellToolParams, ): string | null { + if (!params.command.trim()) { + return 'Command cannot be empty.'; + } + const commandCheck = isCommandAllowed(params.command, this.config); if (!commandCheck.allowed) { if (!commandCheck.reason) { @@ -413,9 +413,6 @@ export class ShellTool extends BaseDeclarativeTool< } return commandCheck.reason; } - if (!params.command.trim()) { - return 'Command cannot be empty.'; - } if (getCommandRoots(params.command).length === 0) { return 'Could not identify command root to obtain permission from user.'; } diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 8525c3b913c..63c6cca48c2 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -13,6 +13,43 @@ import mime from 'mime/lite'; import type { FileSystemService } from '../services/fileSystemService.js'; import { ToolErrorType } from '../tools/tool-error.js'; import { BINARY_EXTENSIONS } from './ignorePatterns.js'; +import { createRequire as createModuleRequire } from 'node:module'; + +const requireModule = createModuleRequire(import.meta.url); + +export async function readWasmBinaryFromDisk( + specifier: string, +): Promise { + const resolvedPath = requireModule.resolve(specifier); + const buffer = await fsPromises.readFile(resolvedPath); + return new Uint8Array(buffer); +} + +export async function loadWasmBinary( + dynamicImport: () => Promise<{ default: Uint8Array }>, + fallbackSpecifier: string, +): Promise { + try { + const module = await dynamicImport(); + if (module?.default instanceof Uint8Array) { + return module.default; + } + } catch (error) { + try { + return await readWasmBinaryFromDisk(fallbackSpecifier); + } catch { + throw error; + } + } + + try { + return await readWasmBinaryFromDisk(fallbackSpecifier); + } catch (error) { + throw new Error('WASM binary module did not provide a Uint8Array export', { + cause: error, + }); + } +} // Constants for text file processing const DEFAULT_MAX_LINES_TEXT_FILE = 2000; diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 9ac6b207ad0..14fa1c0c374 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -4,13 +4,22 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { expect, describe, it, beforeEach, vi, afterEach } from 'vitest'; +import { + expect, + describe, + it, + beforeEach, + beforeAll, + vi, + afterEach, +} from 'vitest'; import { checkCommandPermissions, escapeShellArg, getCommandRoots, getShellConfiguration, isCommandAllowed, + initializeShellParsers, stripShellWrapper, } from './shell-utils.js'; import type { Config } from '../config/config.js'; @@ -32,6 +41,13 @@ vi.mock('shell-quote', () => ({ })); let config: Config; +const isWindowsRuntime = process.platform === 'win32'; +const describeWindowsOnly = isWindowsRuntime ? describe : describe.skip; + +beforeAll(async () => { + mockPlatform.mockReturnValue('linux'); + await initializeShellParsers(); +}); beforeEach(() => { mockPlatform.mockReturnValue('linux'); @@ -51,41 +67,41 @@ afterEach(() => { describe('isCommandAllowed', () => { it('should allow a command if no restrictions are provided', () => { - const result = isCommandAllowed('ls -l', config); + const result = isCommandAllowed('goodCommand --safe', config); expect(result.allowed).toBe(true); }); it('should allow a command if it is in the global allowlist', () => { - config.getCoreTools = () => ['ShellTool(ls)']; - const result = isCommandAllowed('ls -l', config); + config.getCoreTools = () => ['ShellTool(goodCommand)']; + const result = isCommandAllowed('goodCommand --safe', config); expect(result.allowed).toBe(true); }); it('should block a command if it is not in a strict global allowlist', () => { - config.getCoreTools = () => ['ShellTool(ls -l)']; - const result = isCommandAllowed('rm -rf /', config); + config.getCoreTools = () => ['ShellTool(goodCommand --safe)']; + const result = isCommandAllowed('badCommand --danger', config); expect(result.allowed).toBe(false); expect(result.reason).toBe( - `Command(s) not in the allowed commands list. Disallowed commands: "rm -rf /"`, + `Command(s) not in the allowed commands list. Disallowed commands: "badCommand --danger"`, ); }); it('should block a command if it is in the blocked list', () => { - config.getExcludeTools = () => ['ShellTool(rm -rf /)']; - const result = isCommandAllowed('rm -rf /', config); + config.getExcludeTools = () => ['ShellTool(badCommand --danger)']; + const result = isCommandAllowed('badCommand --danger', config); expect(result.allowed).toBe(false); expect(result.reason).toBe( - `Command 'rm -rf /' is blocked by configuration`, + `Command 'badCommand --danger' is blocked by configuration`, ); }); it('should prioritize the blocklist over the allowlist', () => { - config.getCoreTools = () => ['ShellTool(rm -rf /)']; - config.getExcludeTools = () => ['ShellTool(rm -rf /)']; - const result = isCommandAllowed('rm -rf /', config); + config.getCoreTools = () => ['ShellTool(badCommand --danger)']; + config.getExcludeTools = () => ['ShellTool(badCommand --danger)']; + const result = isCommandAllowed('badCommand --danger', config); expect(result.allowed).toBe(false); expect(result.reason).toBe( - `Command 'rm -rf /' is blocked by configuration`, + `Command 'badCommand --danger' is blocked by configuration`, ); }); @@ -106,58 +122,64 @@ describe('isCommandAllowed', () => { it('should block a command on the blocklist even with a wildcard allow', () => { config.getCoreTools = () => ['ShellTool']; - config.getExcludeTools = () => ['ShellTool(rm -rf /)']; - const result = isCommandAllowed('rm -rf /', config); + config.getExcludeTools = () => ['ShellTool(badCommand --danger)']; + const result = isCommandAllowed('badCommand --danger', config); expect(result.allowed).toBe(false); expect(result.reason).toBe( - `Command 'rm -rf /' is blocked by configuration`, + `Command 'badCommand --danger' is blocked by configuration`, ); }); it('should allow a chained command if all parts are on the global allowlist', () => { config.getCoreTools = () => [ 'run_shell_command(echo)', - 'run_shell_command(ls)', + 'run_shell_command(goodCommand)', ]; - const result = isCommandAllowed('echo "hello" && ls -l', config); + const result = isCommandAllowed( + 'echo "hello" && goodCommand --safe', + config, + ); expect(result.allowed).toBe(true); }); it('should block a chained command if any part is blocked', () => { - config.getExcludeTools = () => ['run_shell_command(rm)']; - const result = isCommandAllowed('echo "hello" && rm -rf /', config); + config.getExcludeTools = () => ['run_shell_command(badCommand)']; + const result = isCommandAllowed( + 'echo "hello" && badCommand --danger', + config, + ); expect(result.allowed).toBe(false); expect(result.reason).toBe( - `Command 'rm -rf /' is blocked by configuration`, + `Command 'badCommand --danger' is blocked by configuration`, ); }); describe('command substitution', () => { - it('should block command substitution using `$(...)`', () => { - const result = isCommandAllowed('echo $(rm -rf /)', config); - expect(result.allowed).toBe(false); - expect(result.reason).toContain('Command substitution'); + it('should allow command substitution using `$(...)`', () => { + const result = isCommandAllowed('echo $(goodCommand --safe)', config); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); }); - it('should block command substitution using `<(...)`', () => { + it('should allow command substitution using `<(...)`', () => { const result = isCommandAllowed('diff <(ls) <(ls -a)', config); - expect(result.allowed).toBe(false); - expect(result.reason).toContain('Command substitution'); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); }); - it('should block command substitution using `>(...)`', () => { + it('should allow command substitution using `>(...)`', () => { const result = isCommandAllowed( 'echo "Log message" > >(tee log.txt)', config, ); - expect(result.allowed).toBe(false); - expect(result.reason).toContain('Command substitution'); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); }); - it('should block command substitution using backticks', () => { - const result = isCommandAllowed('echo `rm -rf /`', config); - expect(result.allowed).toBe(false); - expect(result.reason).toContain('Command substitution'); + it('should allow command substitution using backticks', () => { + const result = isCommandAllowed('echo `goodCommand --safe`', config); + expect(result.allowed).toBe(true); + expect(result.reason).toBeUndefined(); }); it('should allow substitution-like patterns inside single quotes', () => { @@ -165,33 +187,54 @@ describe('isCommandAllowed', () => { const result = isCommandAllowed("echo '$(pwd)'", config); expect(result.allowed).toBe(true); }); + + it('should block a command when parsing fails', () => { + const result = isCommandAllowed('ls &&', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); }); }); describe('checkCommandPermissions', () => { describe('in "Default Allow" mode (no sessionAllowlist)', () => { it('should return a detailed success object for an allowed command', () => { - const result = checkCommandPermissions('ls -l', config); + const result = checkCommandPermissions('goodCommand --safe', config); expect(result).toEqual({ allAllowed: true, disallowedCommands: [], }); }); + it('should block commands that cannot be parsed safely', () => { + const result = checkCommandPermissions('ls &&', config); + expect(result).toEqual({ + allAllowed: false, + disallowedCommands: ['ls &&'], + blockReason: 'Command rejected because it could not be parsed safely', + isHardDenial: true, + }); + }); + it('should return a detailed failure object for a blocked command', () => { - config.getExcludeTools = () => ['ShellTool(rm)']; - const result = checkCommandPermissions('rm -rf /', config); + config.getExcludeTools = () => ['ShellTool(badCommand)']; + const result = checkCommandPermissions('badCommand --danger', config); expect(result).toEqual({ allAllowed: false, - disallowedCommands: ['rm -rf /'], - blockReason: `Command 'rm -rf /' is blocked by configuration`, + disallowedCommands: ['badCommand --danger'], + blockReason: `Command 'badCommand --danger' is blocked by configuration`, isHardDenial: true, }); }); it('should return a detailed failure object for a command not on a strict allowlist', () => { - config.getCoreTools = () => ['ShellTool(ls)']; - const result = checkCommandPermissions('git status && ls', config); + config.getCoreTools = () => ['ShellTool(goodCommand)']; + const result = checkCommandPermissions( + 'git status && goodCommand', + config, + ); expect(result).toEqual({ allAllowed: false, disallowedCommands: ['git status'], @@ -204,24 +247,24 @@ describe('checkCommandPermissions', () => { describe('in "Default Deny" mode (with sessionAllowlist)', () => { it('should allow a command on the sessionAllowlist', () => { const result = checkCommandPermissions( - 'ls -l', + 'goodCommand --safe', config, - new Set(['ls -l']), + new Set(['goodCommand --safe']), ); expect(result.allAllowed).toBe(true); }); it('should block a command not on the sessionAllowlist or global allowlist', () => { const result = checkCommandPermissions( - 'rm -rf /', + 'badCommand --danger', config, - new Set(['ls -l']), + new Set(['goodCommand --safe']), ); expect(result.allAllowed).toBe(false); expect(result.blockReason).toContain( 'not on the global or session allowlist', ); - expect(result.disallowedCommands).toEqual(['rm -rf /']); + expect(result.disallowedCommands).toEqual(['badCommand --danger']); }); it('should allow a command on the global allowlist even if not on the session allowlist', () => { @@ -229,7 +272,7 @@ describe('checkCommandPermissions', () => { const result = checkCommandPermissions( 'git status', config, - new Set(['ls -l']), + new Set(['goodCommand --safe']), ); expect(result.allAllowed).toBe(true); }); @@ -245,11 +288,11 @@ describe('checkCommandPermissions', () => { }); it('should block a command on the sessionAllowlist if it is also globally blocked', () => { - config.getExcludeTools = () => ['run_shell_command(rm)']; + config.getExcludeTools = () => ['run_shell_command(badCommand)']; const result = checkCommandPermissions( - 'rm -rf /', + 'badCommand --danger', config, - new Set(['rm -rf /']), + new Set(['badCommand --danger']), ); expect(result.allAllowed).toBe(false); expect(result.blockReason).toContain('is blocked by configuration'); @@ -258,12 +301,12 @@ describe('checkCommandPermissions', () => { it('should block a chained command if one part is not on any allowlist', () => { config.getCoreTools = () => ['run_shell_command(echo)']; const result = checkCommandPermissions( - 'echo "hello" && rm -rf /', + 'echo "hello" && badCommand --danger', config, new Set(['echo']), ); expect(result.allAllowed).toBe(false); - expect(result.disallowedCommands).toEqual(['rm -rf /']); + expect(result.disallowedCommands).toEqual(['badCommand --danger']); }); }); }); @@ -290,6 +333,54 @@ describe('getCommandRoots', () => { const result = getCommandRoots('echo "hello" && git commit -m "feat"'); expect(result).toEqual(['echo', 'git']); }); + + it('should include nested command substitutions', () => { + const result = getCommandRoots('echo $(badCommand --danger)'); + expect(result).toEqual(['echo', 'badCommand']); + }); + + it('should include process substitutions', () => { + const result = getCommandRoots('diff <(ls) <(ls -a)'); + expect(result).toEqual(['diff', 'ls', 'ls']); + }); + + it('should include backtick substitutions', () => { + const result = getCommandRoots('echo `badCommand --danger`'); + expect(result).toEqual(['echo', 'badCommand']); + }); +}); + +describeWindowsOnly('PowerShell integration', () => { + const originalComSpec = process.env['ComSpec']; + + beforeEach(() => { + mockPlatform.mockReturnValue('win32'); + const systemRoot = process.env['SystemRoot'] || 'C:\\\\Windows'; + process.env['ComSpec'] = + `${systemRoot}\\\\System32\\\\WindowsPowerShell\\\\v1.0\\\\powershell.exe`; + }); + + afterEach(() => { + if (originalComSpec === undefined) { + delete process.env['ComSpec']; + } else { + process.env['ComSpec'] = originalComSpec; + } + }); + + it('should return command roots using PowerShell AST output', () => { + const roots = getCommandRoots('Get-ChildItem | Select-Object Name'); + expect(roots.length).toBeGreaterThan(0); + expect(roots).toContain('Get-ChildItem'); + }); + + it('should block commands when PowerShell parser reports errors', () => { + const { allowed, reason } = isCommandAllowed('Get-ChildItem |', config); + expect(allowed).toBe(false); + expect(reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); }); describe('stripShellWrapper', () => { @@ -309,6 +400,21 @@ describe('stripShellWrapper', () => { expect(stripShellWrapper('cmd.exe /c "dir"')).toEqual('dir'); }); + it('should strip powershell.exe -Command with optional -NoProfile', () => { + expect( + stripShellWrapper('powershell.exe -NoProfile -Command "Get-ChildItem"'), + ).toEqual('Get-ChildItem'); + expect( + stripShellWrapper('powershell.exe -Command "Get-ChildItem"'), + ).toEqual('Get-ChildItem'); + }); + + it('should strip pwsh -Command wrapper', () => { + expect( + stripShellWrapper('pwsh -NoProfile -Command "Get-ChildItem"'), + ).toEqual('Get-ChildItem'); + }); + it('should not strip anything if no wrapper is present', () => { expect(stripShellWrapper('ls -l')).toEqual('ls -l'); }); @@ -400,21 +506,21 @@ describe('getShellConfiguration', () => { mockPlatform.mockReturnValue('win32'); }); - it('should return cmd.exe configuration by default', () => { + it('should return PowerShell 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'); + expect(config.executable).toBe('powershell.exe'); + expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']); + expect(config.shell).toBe('powershell'); }); - it('should respect ComSpec for cmd.exe', () => { + it('should ignore ComSpec when pointing to 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'); + expect(config.executable).toBe('powershell.exe'); + expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']); + expect(config.shell).toBe('powershell'); }); it('should return PowerShell configuration if ComSpec points to powershell.exe', () => { diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index e038e7cf2d4..20611bf413f 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -9,7 +9,14 @@ import type { Config } from '../config/config.js'; import os from 'node:os'; import { quote } from 'shell-quote'; import { doesToolInvocationMatch } from './tool-utils.js'; -import { spawn, type SpawnOptionsWithoutStdio } from 'node:child_process'; +import { + spawn, + spawnSync, + type SpawnOptionsWithoutStdio, +} from 'node:child_process'; +import type { Node } from 'web-tree-sitter'; +import { Language, Parser } from 'web-tree-sitter'; +import { loadWasmBinary } from './fileUtils.js'; export const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool']; @@ -22,7 +29,7 @@ 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'). */ + /** The path or name of the shell executable (e.g., 'bash', 'powershell.exe'). */ executable: string; /** * The arguments required by the shell to execute a subsequent string argument. @@ -32,6 +39,343 @@ export interface ShellConfiguration { shell: ShellType; } +let bashLanguage: Language | null = null; +let treeSitterInitialization: Promise | null = null; +let treeSitterInitializationError: Error | null = null; + +class ShellParserInitializationError extends Error { + constructor(cause: Error) { + super(`Failed to initialize bash parser: ${cause.message}`, { cause }); + this.name = 'ShellParserInitializationError'; + } +} + +function toError(value: unknown): Error { + if (value instanceof Error) { + return value; + } + if (typeof value === 'string') { + return new Error(value); + } + return new Error('Unknown tree-sitter initialization error', { + cause: value, + }); +} + +async function loadBashLanguage(): Promise { + try { + treeSitterInitializationError = null; + const [treeSitterBinary, bashBinary] = await Promise.all([ + loadWasmBinary( + () => + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore resolved by esbuild-plugin-wasm during bundling + import('web-tree-sitter/tree-sitter.wasm?binary'), + 'web-tree-sitter/tree-sitter.wasm', + ), + loadWasmBinary( + () => + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore resolved by esbuild-plugin-wasm during bundling + import('tree-sitter-bash/tree-sitter-bash.wasm?binary'), + 'tree-sitter-bash/tree-sitter-bash.wasm', + ), + ]); + + await Parser.init({ wasmBinary: treeSitterBinary }); + bashLanguage = await Language.load(bashBinary); + } catch (error) { + bashLanguage = null; + const normalized = toError(error); + const initializationError = + normalized instanceof ShellParserInitializationError + ? normalized + : new ShellParserInitializationError(normalized); + treeSitterInitializationError = initializationError; + throw initializationError; + } +} + +export async function initializeShellParsers(): Promise { + if (!treeSitterInitialization) { + treeSitterInitialization = loadBashLanguage().catch((error) => { + treeSitterInitialization = null; + throw error; + }); + } + + await treeSitterInitialization; +} + +interface ParsedCommandDetail { + name: string; + text: string; +} + +interface CommandParseResult { + details: ParsedCommandDetail[]; + hasError: boolean; +} + +const POWERSHELL_COMMAND_ENV = '__GCLI_POWERSHELL_COMMAND__'; + +// Encode the parser script as UTF-16LE base64 so we can pass it via PowerShell's -EncodedCommand flag; +// this avoids brittle quoting/escaping when spawning PowerShell and ensures the script is received byte-for-byte. +const POWERSHELL_PARSER_SCRIPT = Buffer.from( + ` +$ErrorActionPreference = 'Stop' +$commandText = $env:${POWERSHELL_COMMAND_ENV} +if ([string]::IsNullOrEmpty($commandText)) { + Write-Output '{"success":false}' + exit 0 +} +$tokens = $null +$errors = $null +$ast = [System.Management.Automation.Language.Parser]::ParseInput($commandText, [ref]$tokens, [ref]$errors) +if ($errors -and $errors.Count -gt 0) { + Write-Output '{"success":false}' + exit 0 +} +$commandAsts = $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.CommandAst] }, $true) +$commandObjects = @() +foreach ($commandAst in $commandAsts) { + $name = $commandAst.GetCommandName() + if ([string]::IsNullOrWhiteSpace($name)) { + continue + } + $commandObjects += [PSCustomObject]@{ + name = $name + text = $commandAst.Extent.Text.Trim() + } +} +[PSCustomObject]@{ + success = $true + commands = $commandObjects +} | ConvertTo-Json -Compress +`, + 'utf16le', +).toString('base64'); + +function createParser(): Parser | null { + if (!bashLanguage) { + if (treeSitterInitializationError) { + throw treeSitterInitializationError; + } + return null; + } + + try { + const parser = new Parser(); + parser.setLanguage(bashLanguage); + return parser; + } catch { + return null; + } +} + +function parseCommandTree(command: string) { + const parser = createParser(); + if (!parser || !command.trim()) { + return null; + } + + try { + return parser.parse(command); + } catch { + return null; + } +} + +function normalizeCommandName(raw: string): string { + if (raw.length >= 2) { + const first = raw[0]; + const last = raw[raw.length - 1]; + if ((first === '"' && last === '"') || (first === "'" && last === "'")) { + return raw.slice(1, -1); + } + } + const trimmed = raw.trim(); + if (!trimmed) { + return trimmed; + } + return trimmed.split(/[\\/]/).pop() ?? trimmed; +} + +function extractNameFromNode(node: Node): string | null { + switch (node.type) { + case 'command': { + const nameNode = node.childForFieldName('name'); + if (!nameNode) { + return null; + } + return normalizeCommandName(nameNode.text); + } + case 'declaration_command': + case 'unset_command': + case 'test_command': { + const firstChild = node.child(0); + if (!firstChild) { + return null; + } + return normalizeCommandName(firstChild.text); + } + default: + return null; + } +} + +function collectCommandDetails( + root: Node, + source: string, +): ParsedCommandDetail[] { + const stack: Node[] = [root]; + const details: ParsedCommandDetail[] = []; + + while (stack.length > 0) { + const current = stack.pop(); + if (!current) { + continue; + } + + const commandName = extractNameFromNode(current); + if (commandName) { + details.push({ + name: commandName, + text: source.slice(current.startIndex, current.endIndex).trim(), + }); + } + + for (let i = current.namedChildCount - 1; i >= 0; i -= 1) { + const child = current.namedChild(i); + if (child) { + stack.push(child); + } + } + } + + return details; +} + +function parseBashCommandDetails(command: string): CommandParseResult | null { + if (treeSitterInitializationError) { + throw treeSitterInitializationError; + } + + if (!bashLanguage) { + initializeShellParsers().catch(() => { + // The failure path is surfaced via treeSitterInitializationError. + }); + return null; + } + + const tree = parseCommandTree(command); + if (!tree) { + return null; + } + + const details = collectCommandDetails(tree.rootNode, command); + return { + details, + hasError: tree.rootNode.hasError || details.length === 0, + }; +} + +function parsePowerShellCommandDetails( + command: string, + executable: string, +): CommandParseResult | null { + const trimmed = command.trim(); + if (!trimmed) { + return { + details: [], + hasError: true, + }; + } + + try { + const result = spawnSync( + executable, + [ + '-NoLogo', + '-NoProfile', + '-NonInteractive', + '-EncodedCommand', + POWERSHELL_PARSER_SCRIPT, + ], + { + env: { + ...process.env, + [POWERSHELL_COMMAND_ENV]: command, + }, + encoding: 'utf-8', + }, + ); + + if (result.error || result.status !== 0) { + return null; + } + + const output = (result.stdout ?? '').toString().trim(); + if (!output) { + return { details: [], hasError: true }; + } + + let parsed: { + success?: boolean; + commands?: Array<{ name?: string; text?: string }>; + } | null = null; + try { + parsed = JSON.parse(output); + } catch { + return { details: [], hasError: true }; + } + + if (!parsed?.success) { + return { details: [], hasError: true }; + } + + const details = (parsed.commands ?? []) + .map((commandDetail) => { + if (!commandDetail || typeof commandDetail.name !== 'string') { + return null; + } + + const name = normalizeCommandName(commandDetail.name); + const text = + typeof commandDetail.text === 'string' + ? commandDetail.text.trim() + : command; + + return { + name, + text, + }; + }) + .filter((detail): detail is ParsedCommandDetail => detail !== null); + + return { + details, + hasError: details.length === 0, + }; + } catch { + return null; + } +} + +function parseCommandDetails(command: string): CommandParseResult | null { + const configuration = getShellConfiguration(); + + if (configuration.shell === 'powershell') { + return parsePowerShellCommandDetails(command, configuration.executable); + } + + if (configuration.shell === 'bash') { + return parseBashCommandDetails(command); + } + + return null; +} + /** * Determines the appropriate shell configuration for the current platform. * @@ -42,32 +386,26 @@ export interface ShellConfiguration { */ 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', - }; + const comSpec = process.env['ComSpec']; + if (comSpec) { + const executable = comSpec.toLowerCase(); + if ( + executable.endsWith('powershell.exe') || + executable.endsWith('pwsh.exe') + ) { + 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. + // Default to PowerShell for all other Windows configurations. return { - executable: comSpec, - argsPrefix: ['/d', '/s', '/c'], - shell: 'cmd', + executable: 'powershell.exe', + argsPrefix: ['-NoProfile', '-Command'], + shell: 'powershell', }; } @@ -114,53 +452,12 @@ export function escapeShellArg(arg: string, shell: ShellType): string { * @returns An array of individual command strings */ export function splitCommands(command: string): string[] { - const commands: string[] = []; - let currentCommand = ''; - let inSingleQuotes = false; - let inDoubleQuotes = false; - let i = 0; - - while (i < command.length) { - const char = command[i]; - const nextChar = command[i + 1]; - - if (char === '\\' && i < command.length - 1) { - currentCommand += char + command[i + 1]; - i += 2; - continue; - } - - if (char === "'" && !inDoubleQuotes) { - inSingleQuotes = !inSingleQuotes; - } else if (char === '"' && !inSingleQuotes) { - inDoubleQuotes = !inDoubleQuotes; - } - - if (!inSingleQuotes && !inDoubleQuotes) { - if ( - (char === '&' && nextChar === '&') || - (char === '|' && nextChar === '|') - ) { - commands.push(currentCommand.trim()); - currentCommand = ''; - i++; // Skip the next character - } else if (char === ';' || char === '&' || char === '|') { - commands.push(currentCommand.trim()); - currentCommand = ''; - } else { - currentCommand += char; - } - } else { - currentCommand += char; - } - i++; - } - - if (currentCommand.trim()) { - commands.push(currentCommand.trim()); + const parsed = parseCommandDetails(command); + if (!parsed || parsed.hasError) { + return []; } - return commands.filter(Boolean); // Filter out any empty strings + return parsed.details.map((detail) => detail.text).filter(Boolean); } /** @@ -172,40 +469,30 @@ export function splitCommands(command: string): string[] { * @example getCommandRoot("git status && npm test") returns "git" */ export function getCommandRoot(command: string): string | undefined { - const trimmedCommand = command.trim(); - if (!trimmedCommand) { + const parsed = parseCommandDetails(command); + if (!parsed || parsed.hasError || parsed.details.length === 0) { return undefined; } - // This regex is designed to find the first "word" of a command, - // while respecting quotes. It looks for a sequence of non-whitespace - // characters that are not inside quotes. - const match = trimmedCommand.match(/^"([^"]+)"|^'([^']+)'|^(\S+)/); - if (match) { - // The first element in the match array is the full match. - // The subsequent elements are the capture groups. - // We prefer a captured group because it will be unquoted. - const commandRoot = match[1] || match[2] || match[3]; - if (commandRoot) { - // If the command is a path, return the last component. - return commandRoot.split(/[\\/]/).pop(); - } - } - - return undefined; + return parsed.details[0]?.name; } export function getCommandRoots(command: string): string[] { if (!command) { return []; } - return splitCommands(command) - .map((c) => getCommandRoot(c)) - .filter((c): c is string => !!c); + + const parsed = parseCommandDetails(command); + if (!parsed || parsed.hasError) { + return []; + } + + return parsed.details.map((detail) => detail.name).filter(Boolean); } export function stripShellWrapper(command: string): string { - const pattern = /^\s*(?:sh|bash|zsh|cmd.exe)\s+(?:\/c|-c)\s+/; + const pattern = + /^\s*(?:(?:sh|bash|zsh)\s+-c|cmd\.exe\s+\/c|powershell(?:\.exe)?\s+(?:-NoProfile\s+)?-Command|pwsh(?:\.exe)?\s+(?:-NoProfile\s+)?-Command)\s+/i; const match = command.match(pattern); if (match) { let newCommand = command.substring(match[0].length).trim(); @@ -228,62 +515,6 @@ export function stripShellWrapper(command: string): string { * @param command The shell command string to check * @returns true if command substitution would be executed by bash */ -export function detectCommandSubstitution(command: string): boolean { - let inSingleQuotes = false; - let inDoubleQuotes = false; - let inBackticks = false; - let i = 0; - - while (i < command.length) { - const char = command[i]; - const nextChar = command[i + 1]; - - // Handle escaping - only works outside single quotes - if (char === '\\' && !inSingleQuotes) { - i += 2; // Skip the escaped character - continue; - } - - // Handle quote state changes - if (char === "'" && !inDoubleQuotes && !inBackticks) { - inSingleQuotes = !inSingleQuotes; - } else if (char === '"' && !inSingleQuotes && !inBackticks) { - inDoubleQuotes = !inDoubleQuotes; - } else if (char === '`' && !inSingleQuotes) { - // Backticks work outside single quotes (including in double quotes) - inBackticks = !inBackticks; - } - - // Check for command substitution patterns that would be executed - if (!inSingleQuotes) { - // $(...) command substitution - works in double quotes and unquoted - if (char === '$' && nextChar === '(') { - return true; - } - - // <(...) process substitution - works unquoted only (not in double quotes) - if (char === '<' && nextChar === '(' && !inDoubleQuotes && !inBackticks) { - return true; - } - - // >(...) process substitution - works unquoted only (not in double quotes) - if (char === '>' && nextChar === '(' && !inDoubleQuotes && !inBackticks) { - return true; - } - - // Backtick command substitution - check for opening backtick - // (We track the state above, so this catches the start of backtick substitution) - if (char === '`' && !inBackticks) { - return true; - } - } - - i++; - } - - return false; -} - /** * Checks a shell command against security policies and allowlists. * @@ -318,19 +549,20 @@ export function checkCommandPermissions( blockReason?: string; isHardDenial?: boolean; } { - // Disallow command substitution for security. - if (detectCommandSubstitution(command)) { + const parseResult = parseCommandDetails(command); + if (!parseResult || parseResult.hasError) { return { allAllowed: false, disallowedCommands: [command], - blockReason: - 'Command substitution using $(), `` ` ``, <(), or >() is not allowed for security reasons', + blockReason: 'Command rejected because it could not be parsed safely', isHardDenial: true, }; } const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' '); - const commandsToValidate = splitCommands(command).map(normalize); + const commandsToValidate = parseResult.details + .map((detail) => normalize(detail.text)) + .filter(Boolean); const invocation: AnyToolInvocation & { params: { command: string } } = { params: { command: '' }, } as AnyToolInvocation & { params: { command: string } }; diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json index 06e3256b971..fac510b7299 100644 --- a/packages/core/tsconfig.json +++ b/packages/core/tsconfig.json @@ -6,6 +6,6 @@ "composite": true, "types": ["node", "vitest/globals"] }, - "include": ["index.ts", "src/**/*.ts", "src/**/*.json"], + "include": ["index.ts", "src/**/*.ts", "src/**/*.d.ts", "src/**/*.json"], "exclude": ["node_modules", "dist"] }