Tighten bash shell option handling (#12532)

This commit is contained in:
cornmander
2025-11-04 11:11:29 -05:00
committed by GitHub
parent 96d7eb2966
commit b8b6620365
4 changed files with 108 additions and 10 deletions
@@ -186,7 +186,10 @@ describe('ShellExecutionService', () => {
expect(mockPtySpawn).toHaveBeenCalledWith( expect(mockPtySpawn).toHaveBeenCalledWith(
'bash', 'bash',
['-c', 'ls -l'], [
'-c',
'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l',
],
expect.any(Object), expect.any(Object),
); );
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
@@ -539,7 +542,10 @@ describe('ShellExecutionService', () => {
expect(mockPtySpawn).toHaveBeenCalledWith( expect(mockPtySpawn).toHaveBeenCalledWith(
'bash', 'bash',
['-c', 'ls "foo bar"'], [
'-c',
'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
],
expect.any(Object), expect.any(Object),
); );
}); });
@@ -691,7 +697,10 @@ describe('ShellExecutionService child_process fallback', () => {
expect(mockCpSpawn).toHaveBeenCalledWith( expect(mockCpSpawn).toHaveBeenCalledWith(
'bash', 'bash',
['-c', 'ls -l'], [
'-c',
'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls -l',
],
expect.objectContaining({ shell: false, detached: true }), expect.objectContaining({ shell: false, detached: true }),
); );
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
@@ -981,7 +990,10 @@ describe('ShellExecutionService child_process fallback', () => {
expect(mockCpSpawn).toHaveBeenCalledWith( expect(mockCpSpawn).toHaveBeenCalledWith(
'bash', 'bash',
['-c', 'ls "foo bar"'], [
'-c',
'shopt -u promptvars nullglob extglob nocaseglob dotglob; ls "foo bar"',
],
expect.objectContaining({ expect.objectContaining({
shell: false, shell: false,
detached: true, detached: true,
@@ -12,7 +12,7 @@ import { TextDecoder } from 'node:util';
import os from 'node:os'; import os from 'node:os';
import type { IPty } from '@lydell/node-pty'; import type { IPty } from '@lydell/node-pty';
import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js'; 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 { isBinary } from '../utils/textUtils.js';
import pkg from '@xterm/headless'; import pkg from '@xterm/headless';
import { import {
@@ -24,6 +24,22 @@ const { Terminal } = pkg;
const SIGKILL_TIMEOUT_MS = 200; const SIGKILL_TIMEOUT_MS = 200;
const MAX_CHILD_PROCESS_BUFFER_SIZE = 16 * 1024 * 1024; // 16MB 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. */ /** A structured result from a shell command execution. */
export interface ShellExecutionResult { export interface ShellExecutionResult {
/** The raw, unprocessed output buffer. */ /** The raw, unprocessed output buffer. */
@@ -190,8 +206,9 @@ export class ShellExecutionService {
): ShellExecutionHandle { ): ShellExecutionHandle {
try { try {
const isWindows = os.platform() === 'win32'; const isWindows = os.platform() === 'win32';
const { executable, argsPrefix } = getShellConfiguration(); const { executable, argsPrefix, shell } = getShellConfiguration();
const spawnArgs = [...argsPrefix, commandToExecute]; const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
const spawnArgs = [...argsPrefix, guardedCommand];
const child = cpSpawn(executable, spawnArgs, { const child = cpSpawn(executable, spawnArgs, {
cwd, cwd,
@@ -403,8 +420,9 @@ export class ShellExecutionService {
try { try {
const cols = shellExecutionConfig.terminalWidth ?? 80; const cols = shellExecutionConfig.terminalWidth ?? 80;
const rows = shellExecutionConfig.terminalHeight ?? 30; const rows = shellExecutionConfig.terminalHeight ?? 30;
const { executable, argsPrefix } = getShellConfiguration(); const { executable, argsPrefix, shell } = getShellConfiguration();
const args = [...argsPrefix, commandToExecute]; const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
const args = [...argsPrefix, guardedCommand];
const ptyProcess = ptyInfo.module.spawn(executable, args, { const ptyProcess = ptyInfo.module.spawn(executable, args, {
cwd, cwd,
@@ -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', () => { describe('command substitution', () => {
it('should allow command substitution using `$(...)`', () => { it('should allow command substitution using `$(...)`', () => {
const result = isCommandAllowed('echo $(goodCommand --safe)', config); const result = isCommandAllowed('echo $(goodCommand --safe)', config);
@@ -465,6 +484,18 @@ describe('getCommandRoots', () => {
const result = getCommandRoots('echo `badCommand --danger`'); const result = getCommandRoots('echo `badCommand --danger`');
expect(result).toEqual(['echo', 'badCommand']); 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', () => { describeWindowsOnly('PowerShell integration', () => {
+38 -1
View File
@@ -256,6 +256,40 @@ function collectCommandDetails(
return details; 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 { function parseBashCommandDetails(command: string): CommandParseResult | null {
if (treeSitterInitializationError) { if (treeSitterInitializationError) {
throw treeSitterInitializationError; throw treeSitterInitializationError;
@@ -276,7 +310,10 @@ function parseBashCommandDetails(command: string): CommandParseResult | null {
const details = collectCommandDetails(tree.rootNode, command); const details = collectCommandDetails(tree.rootNode, command);
return { return {
details, details,
hasError: tree.rootNode.hasError || details.length === 0, hasError:
tree.rootNode.hasError ||
details.length === 0 ||
hasPromptCommandTransform(tree.rootNode),
}; };
} }