Fix shell auto-approval parsing for chained commands (#11527)

packages/core/src/core/coreToolScheduler.test.ts

@@ -19,6 +19,7 @@ import type {
   ToolResult,
   Config,
   ToolRegistry,
+  AnyToolInvocation,
 } from '../index.js';
 import {
   DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
@@ -37,6 +38,7 @@ import {
 } from '../test-utils/mock-tool.js';
 import * as fs from 'node:fs/promises';
 import * as path from 'node:path';
+import { isShellInvocationAllowlisted } from '../utils/shell-utils.js';
 
 vi.mock('fs/promises', () => ({
   writeFile: vi.fn(),
@@ -1192,6 +1194,110 @@ describe('CoreToolScheduler request queueing', () => {
     }
   });
 
+  it('should require approval for a chained shell command even when prefix is allowlisted', async () => {
+    expect(
+      isShellInvocationAllowlisted(
+        {
+          params: { command: 'git status && rm -rf /tmp/should-not-run' },
+        } as unknown as AnyToolInvocation,
+        ['run_shell_command(git)'],
+      ),
+    ).toBe(false);
+
+    const executeFn = vi.fn().mockResolvedValue({
+      llmContent: 'Shell command executed',
+      returnDisplay: 'Shell command executed',
+    });
+
+    const mockShellTool = new MockTool({
+      name: 'run_shell_command',
+      shouldConfirmExecute: (params) =>
+        Promise.resolve({
+          type: 'exec',
+          title: 'Confirm Shell Command',
+          command: String(params['command'] ?? ''),
+          rootCommand: 'git',
+          onConfirm: async () => {},
+        }),
+      execute: () => executeFn({}),
+    });
+
+    const toolRegistry = {
+      getTool: () => mockShellTool,
+      getToolByName: () => mockShellTool,
+      getFunctionDeclarations: () => [],
+      tools: new Map(),
+      discovery: {},
+      registerTool: () => {},
+      getToolByDisplayName: () => mockShellTool,
+      getTools: () => [],
+      discoverTools: async () => {},
+      getAllTools: () => [],
+      getToolsByServer: () => [],
+    };
+
+    const onAllToolCallsComplete = vi.fn();
+    const onToolCallsUpdate = vi.fn();
+
+    const mockConfig = {
+      getSessionId: () => 'test-session-id',
+      getUsageStatisticsEnabled: () => true,
+      getDebugMode: () => false,
+      getApprovalMode: () => ApprovalMode.DEFAULT,
+      getAllowedTools: () => ['run_shell_command(git)'],
+      getContentGeneratorConfig: () => ({
+        model: 'test-model',
+        authType: 'oauth-personal',
+      }),
+      getShellExecutionConfig: () => ({
+        terminalWidth: 80,
+        terminalHeight: 24,
+      }),
+      getTerminalWidth: vi.fn(() => 80),
+      getTerminalHeight: vi.fn(() => 24),
+      storage: {
+        getProjectTempDir: () => '/tmp',
+      },
+      getTruncateToolOutputThreshold: () =>
+        DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD,
+      getTruncateToolOutputLines: () => DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
+      getToolRegistry: () => toolRegistry,
+      getUseSmartEdit: () => false,
+      getUseModelRouter: () => false,
+      getGeminiClient: () => null,
+      getEnableMessageBusIntegration: () => false,
+      getMessageBus: () => null,
+      getPolicyEngine: () => null,
+    } as unknown as Config;
+
+    const scheduler = new CoreToolScheduler({
+      config: mockConfig,
+      onAllToolCallsComplete,
+      onToolCallsUpdate,
+      getPreferredEditor: () => 'vscode',
+      onEditorClose: vi.fn(),
+    });
+
+    const abortController = new AbortController();
+    const request = {
+      callId: 'shell-1',
+      name: 'run_shell_command',
+      args: { command: 'git status && rm -rf /tmp/should-not-run' },
+      isClientInitiated: false,
+      prompt_id: 'prompt-shell-auto-approved',
+    };
+
+    await scheduler.schedule([request], abortController.signal);
+
+    const statusUpdates = onToolCallsUpdate.mock.calls
+      .map((call) => (call[0][0] as ToolCall)?.status)
+      .filter(Boolean);
+
+    expect(statusUpdates).toContain('awaiting_approval');
+    expect(executeFn).not.toHaveBeenCalled();
+    expect(onAllToolCallsComplete).not.toHaveBeenCalled();
+  });
+
   it('should handle two synchronous calls to schedule', async () => {
     const executeFn = vi.fn().mockResolvedValue({
       llmContent: 'Tool executed',

packages/core/src/core/coreToolScheduler.ts

@@ -38,6 +38,10 @@ import {
 import * as Diff from 'diff';
 import * as fs from 'node:fs/promises';
 import * as path from 'node:path';
+import {
+  isShellInvocationAllowlisted,
+  SHELL_TOOL_NAMES,
+} from '../utils/shell-utils.js';
 import { doesToolInvocationMatch } from '../utils/tool-utils.js';
 import levenshtein from 'fast-levenshtein';
 import { ShellToolInvocation } from '../tools/shell.js';
@@ -728,7 +732,8 @@ export class CoreToolScheduler {
           continue;
         }
 
-        const { request: reqInfo, invocation } = toolCall;
+        const validatingCall = toolCall as ValidatingToolCall;
+        const { request: reqInfo, invocation } = validatingCall;
 
         try {
           if (signal.aborted) {
@@ -752,11 +757,7 @@ export class CoreToolScheduler {
             continue;
           }
 
-          const allowedTools = this.config.getAllowedTools() || [];
-          if (
-            this.config.getApprovalMode() === ApprovalMode.YOLO ||
-            doesToolInvocationMatch(toolCall.tool, invocation, allowedTools)
-          ) {
+          if (this.isAutoApproved(validatingCall)) {
             this.setToolCallOutcome(
               reqInfo.callId,
               ToolConfirmationOutcome.ProceedAlways,
@@ -1190,6 +1191,22 @@ export class CoreToolScheduler {
     });
   }
 
+  private isAutoApproved(toolCall: ValidatingToolCall): boolean {
+    if (this.config.getApprovalMode() === ApprovalMode.YOLO) {
+      return true;
+    }
+
+    const allowedTools = this.config.getAllowedTools() || [];
+    const { tool, invocation } = toolCall;
+    const toolName = typeof tool === 'string' ? tool : tool.name;
+
+    if (SHELL_TOOL_NAMES.includes(toolName)) {
+      return isShellInvocationAllowlisted(invocation, allowedTools);
+    }
+
+    return doesToolInvocationMatch(tool, invocation, allowedTools);
+  }
+
   private async autoApproveCompatiblePendingTools(
     signal: AbortSignal,
     triggeringCallId: string,

packages/core/src/utils/shell-utils.test.ts

@@ -21,8 +21,10 @@ import {
   isCommandAllowed,
   initializeShellParsers,
   stripShellWrapper,
+  isShellInvocationAllowlisted,
 } from './shell-utils.js';
 import type { Config } from '../config/config.js';
+import type { AnyToolInvocation } from '../index.js';
 
 const mockPlatform = vi.hoisted(() => vi.fn());
 const mockHomedir = vi.hoisted(() => vi.fn());
@@ -420,6 +422,53 @@ describe('stripShellWrapper', () => {
   });
 });
 
+describe('isShellInvocationAllowlisted', () => {
+  function createInvocation(command: string): AnyToolInvocation {
+    return { params: { command } } as unknown as AnyToolInvocation;
+  }
+
+  it('should return false when any chained command segment is not allowlisted', () => {
+    const invocation = createInvocation(
+      'git status && rm -rf /tmp/should-not-run',
+    );
+    expect(
+      isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']),
+    ).toBe(false);
+  });
+
+  it('should return true when every segment is explicitly allowlisted', () => {
+    const invocation = createInvocation(
+      'git status && rm -rf /tmp/should-run && git diff',
+    );
+    expect(
+      isShellInvocationAllowlisted(invocation, [
+        'run_shell_command(git)',
+        'run_shell_command(rm -rf)',
+      ]),
+    ).toBe(true);
+  });
+
+  it('should return true when the allowlist contains a wildcard shell entry', () => {
+    const invocation = createInvocation('git status && rm -rf /tmp/should-run');
+    expect(
+      isShellInvocationAllowlisted(invocation, ['run_shell_command']),
+    ).toBe(true);
+  });
+
+  it('should treat piped commands as separate segments that must be allowlisted', () => {
+    const invocation = createInvocation('git status | tail -n 1');
+    expect(
+      isShellInvocationAllowlisted(invocation, ['run_shell_command(git)']),
+    ).toBe(false);
+    expect(
+      isShellInvocationAllowlisted(invocation, [
+        'run_shell_command(git)',
+        'run_shell_command(tail)',
+      ]),
+    ).toBe(true);
+  });
+});
+
 describe('escapeShellArg', () => {
   describe('POSIX (bash)', () => {
     it('should use shell-quote for escaping', () => {

packages/core/src/utils/shell-utils.ts

@@ -738,3 +738,75 @@ export function isCommandAllowed(
   }
   return { allowed: false, reason: blockReason };
 }
+
+/**
+ * Determines whether a shell invocation should be auto-approved based on an allowlist.
+ *
+ * This reuses the same parsing logic as command-permission enforcement so that
+ * chained commands must be individually covered by the allowlist.
+ *
+ * @param invocation The shell tool invocation being evaluated.
+ * @param allowedPatterns The configured allowlist patterns (e.g. `run_shell_command(git)`).
+ * @returns True if every parsed command segment is allowed by the patterns; false otherwise.
+ */
+export function isShellInvocationAllowlisted(
+  invocation: AnyToolInvocation,
+  allowedPatterns: string[],
+): boolean {
+  if (!allowedPatterns.length) {
+    return false;
+  }
+
+  const hasShellWildcard = allowedPatterns.some((pattern) =>
+    SHELL_TOOL_NAMES.includes(pattern),
+  );
+  const hasShellSpecificPattern = allowedPatterns.some((pattern) =>
+    SHELL_TOOL_NAMES.some((name) => pattern.startsWith(`${name}(`)),
+  );
+
+  if (!hasShellWildcard && !hasShellSpecificPattern) {
+    return false;
+  }
+
+  if (hasShellWildcard) {
+    return true;
+  }
+
+  if (
+    !('params' in invocation) ||
+    typeof invocation.params !== 'object' ||
+    invocation.params === null ||
+    !('command' in invocation.params)
+  ) {
+    return false;
+  }
+
+  const commandValue = (invocation.params as { command?: unknown }).command;
+  if (typeof commandValue !== 'string' || !commandValue.trim()) {
+    return false;
+  }
+
+  const command = commandValue.trim();
+
+  const parseResult = parseCommandDetails(command);
+  if (!parseResult || parseResult.hasError) {
+    return false;
+  }
+
+  const normalize = (cmd: string): string => cmd.trim().replace(/\s+/g, ' ');
+  const commandsToValidate = parseResult.details
+    .map((detail) => normalize(detail.text))
+    .filter(Boolean);
+
+  if (commandsToValidate.length === 0) {
+    return false;
+  }
+
+  return commandsToValidate.every((commandSegment) =>
+    doesToolInvocationMatch(
+      SHELL_TOOL_NAMES[0],
+      { params: { command: commandSegment } } as AnyToolInvocation,
+      allowedPatterns,
+    ),
+  );
+}