From b8b6620365ba494780c4172fcd21782e25796d77 Mon Sep 17 00:00:00 2001 From: cornmander Date: Tue, 4 Nov 2025 11:11:29 -0500 Subject: [PATCH] Tighten bash shell option handling (#12532) --- .../services/shellExecutionService.test.ts | 20 ++++++++-- .../src/services/shellExecutionService.ts | 28 ++++++++++--- packages/core/src/utils/shell-utils.test.ts | 31 +++++++++++++++ packages/core/src/utils/shell-utils.ts | 39 ++++++++++++++++++- 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 372077139e..3e2fdc889e 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -186,7 +186,10 @@ describe('ShellExecutionService', () => { expect(mockPtySpawn).toHaveBeenCalledWith( 'bash', - ['-c', 'ls -l'], + [ + '-c', + 'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l', + ], expect.any(Object), ); expect(result.exitCode).toBe(0); @@ -539,7 +542,10 @@ describe('ShellExecutionService', () => { expect(mockPtySpawn).toHaveBeenCalledWith( 'bash', - ['-c', 'ls "foo bar"'], + [ + '-c', + 'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"', + ], expect.any(Object), ); }); @@ -691,7 +697,10 @@ describe('ShellExecutionService child_process fallback', () => { expect(mockCpSpawn).toHaveBeenCalledWith( 'bash', - ['-c', 'ls -l'], + [ + '-c', + 'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l', + ], expect.objectContaining({ shell: false, detached: true }), ); expect(result.exitCode).toBe(0); @@ -981,7 +990,10 @@ describe('ShellExecutionService child_process fallback', () => { expect(mockCpSpawn).toHaveBeenCalledWith( 'bash', - ['-c', 'ls "foo bar"'], + [ + '-c', + 'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"', + ], expect.objectContaining({ shell: false, detached: true, diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index eeec882ca8..66952afc03 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -12,7 +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 { getShellConfiguration, type ShellType } from '../utils/shell-utils.js'; import { isBinary } from '../utils/textUtils.js'; import pkg from '@xterm/headless'; import { @@ -24,6 +24,22 @@ const { Terminal } = pkg; const SIGKILL_TIMEOUT_MS = 200; const MAX_CHILD_PROCESS_BUFFER_SIZE = 16 * 1024 * 1024; // 16MB +const BASH_SHOPT_OPTIONS = 'promptvars nullglob extglob nocaseglob dotglob'; +const BASH_SHOPT_GUARD = `shopt -u ${BASH_SHOPT_OPTIONS};`; + +function ensurePromptvarsDisabled(command: string, shell: ShellType): string { + if (shell !== 'bash') { + return command; + } + + const trimmed = command.trimStart(); + if (trimmed.startsWith(BASH_SHOPT_GUARD)) { + return command; + } + + return `${BASH_SHOPT_GUARD} ${command}`; +} + /** A structured result from a shell command execution. */ export interface ShellExecutionResult { /** The raw, unprocessed output buffer. */ @@ -190,8 +206,9 @@ export class ShellExecutionService { ): ShellExecutionHandle { try { const isWindows = os.platform() === 'win32'; - const { executable, argsPrefix } = getShellConfiguration(); - const spawnArgs = [...argsPrefix, commandToExecute]; + const { executable, argsPrefix, shell } = getShellConfiguration(); + const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell); + const spawnArgs = [...argsPrefix, guardedCommand]; const child = cpSpawn(executable, spawnArgs, { cwd, @@ -403,8 +420,9 @@ export class ShellExecutionService { try { const cols = shellExecutionConfig.terminalWidth ?? 80; const rows = shellExecutionConfig.terminalHeight ?? 30; - const { executable, argsPrefix } = getShellConfiguration(); - const args = [...argsPrefix, commandToExecute]; + const { executable, argsPrefix, shell } = getShellConfiguration(); + const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell); + const args = [...argsPrefix, guardedCommand]; const ptyProcess = ptyInfo.module.spawn(executable, args, { cwd, diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index c178d20d6a..1ae4049f2b 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -271,6 +271,25 @@ EOF`, ); }); + it('should block commands containing prompt transformations', () => { + const result = isCommandAllowed( + 'echo "${var1=aa\\140 env| ls -l\\140}${var1@P}"', + config, + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); + + it('should block simple prompt transformation expansions', () => { + const result = isCommandAllowed('echo ${foo@P}', config); + expect(result.allowed).toBe(false); + expect(result.reason).toBe( + 'Command rejected because it could not be parsed safely', + ); + }); + describe('command substitution', () => { it('should allow command substitution using `$(...)`', () => { const result = isCommandAllowed('echo $(goodCommand --safe)', config); @@ -465,6 +484,18 @@ describe('getCommandRoots', () => { const result = getCommandRoots('echo `badCommand --danger`'); expect(result).toEqual(['echo', 'badCommand']); }); + + it('should treat parameter expansions with prompt transformations as unsafe', () => { + const roots = getCommandRoots( + 'echo "${var1=aa\\140 env| ls -l\\140}${var1@P}"', + ); + expect(roots).toEqual([]); + }); + + it('should not return roots for prompt transformation expansions', () => { + const roots = getCommandRoots('echo ${foo@P}'); + expect(roots).toEqual([]); + }); }); describeWindowsOnly('PowerShell integration', () => { diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 1130deaa47..ea136492cc 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -256,6 +256,40 @@ function collectCommandDetails( return details; } +function hasPromptCommandTransform(root: Node): boolean { + const stack: Node[] = [root]; + + while (stack.length > 0) { + const current = stack.pop(); + if (!current) { + continue; + } + + if (current.type === 'expansion') { + for (let i = 0; i < current.childCount - 1; i += 1) { + const operatorNode = current.child(i); + const transformNode = current.child(i + 1); + + if ( + operatorNode?.type === '@' && + transformNode?.text?.toLowerCase() === 'p' + ) { + return true; + } + } + } + + for (let i = current.namedChildCount - 1; i >= 0; i -= 1) { + const child = current.namedChild(i); + if (child) { + stack.push(child); + } + } + } + + return false; +} + function parseBashCommandDetails(command: string): CommandParseResult | null { if (treeSitterInitializationError) { throw treeSitterInitializationError; @@ -276,7 +310,10 @@ function parseBashCommandDetails(command: string): CommandParseResult | null { const details = collectCommandDetails(tree.rootNode, command); return { details, - hasError: tree.rootNode.hasError || details.length === 0, + hasError: + tree.rootNode.hasError || + details.length === 0 || + hasPromptCommandTransform(tree.rootNode), }; }