Merge remote-tracking branch 'origin/main' into robot-commits

integration-tests/run_shell_command.test.ts

@@ -68,6 +68,166 @@ describe('run_shell_command', () => {
     validateModelOutput(result, 'test-stdin', 'Shell command stdin test');
   });
 
+  it('should run allowed sub-command in non-interactive mode', async () => {
+    const rig = new TestRig();
+    await rig.setup('should run allowed sub-command in non-interactive mode');
+
+    const prompt = `use wc to tell me how many lines there are in /proc/meminfo`;
+
+    // Provide the prompt via stdin to simulate non-interactive mode
+    const result = await rig.run({
+      stdin: prompt,
+      args: ['--allowed-tools=run_shell_command(wc)'],
+    });
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+  });
+
+  it('should succeed with no parens in non-interactive mode', async () => {
+    const rig = new TestRig();
+    await rig.setup('should succeed with no parens in non-interactive mode');
+
+    const prompt = `use wc to tell me how many lines there are in /proc/meminfo`;
+
+    const result = await rig.run({
+      stdin: prompt,
+      args: ['--allowed-tools=run_shell_command'],
+    });
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+  });
+
+  it('should succeed with --yolo mode', async () => {
+    const rig = new TestRig();
+    await rig.setup('should succeed with --yolo mode');
+
+    const prompt = `use wc to tell me how many lines there are in /proc/meminfo`;
+
+    const result = await rig.run(
+      {
+        prompt: prompt,
+      },
+      '--yolo',
+    );
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+    expect(result).toContain('lines in /proc/meminfo');
+  });
+
+  it('should work with ShellTool alias', async () => {
+    const rig = new TestRig();
+    await rig.setup('should work with ShellTool alias');
+
+    const prompt = `use wc to tell me how many lines there are in /proc/meminfo`;
+
+    const result = await rig.run({
+      stdin: prompt,
+      args: ['--allowed-tools=ShellTool(wc)'],
+    });
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+  });
+
+  it('should combine multiple --allowed-tools flags', async () => {
+    const rig = new TestRig();
+    await rig.setup('should combine multiple --allowed-tools flags');
+
+    const prompt = `use wc and ls`;
+
+    const result = await rig.run({
+      stdin: prompt,
+      args: [
+        '--allowed-tools=run_shell_command(wc)',
+        '--allowed-tools=run_shell_command(ls)',
+      ],
+    });
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+  });
+
+  it('should allow all with "ShellTool" and other specifics', async () => {
+    const rig = new TestRig();
+    await rig.setup('should allow all with "ShellTool" and other specifics');
+
+    const prompt = `use date`;
+
+    const result = await rig.run({
+      stdin: prompt,
+      args: [
+        '--allowed-tools=run_shell_command(wc)',
+        '--allowed-tools=run_shell_command',
+      ],
+    });
+
+    const foundToolCall = await rig.waitForToolCall('run_shell_command', 15000);
+
+    if (!foundToolCall) {
+      printDebugInfo(rig, result, {
+        'Found tool call': foundToolCall,
+      });
+    }
+
+    expect(
+      foundToolCall,
+      'Expected to find a run_shell_command tool call',
+    ).toBeTruthy();
+  });
+
   it('should propagate environment variables to the child process', async () => {
     const rig = new TestRig();
     await rig.setup('should propagate environment variables');

packages/cli/src/config/config.test.ts

@@ -1936,6 +1936,51 @@ describe('loadCliConfig tool exclusions', () => {
     expect(config.getExcludeTools()).not.toContain('replace');
     expect(config.getExcludeTools()).not.toContain('write_file');
   });
+
+  it('should not exclude shell tool in non-interactive mode when --allowed-tools="ShellTool" is set', async () => {
+    process.stdin.isTTY = false;
+    process.argv = [
+      'node',
+      'script.js',
+      '-p',
+      'test',
+      '--allowed-tools',
+      'ShellTool',
+    ];
+    const argv = await parseArguments({} as Settings);
+    const config = await loadCliConfig({}, [], 'test-session', argv);
+    expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
+  });
+
+  it('should not exclude shell tool in non-interactive mode when --allowed-tools="run_shell_command" is set', async () => {
+    process.stdin.isTTY = false;
+    process.argv = [
+      'node',
+      'script.js',
+      '-p',
+      'test',
+      '--allowed-tools',
+      'run_shell_command',
+    ];
+    const argv = await parseArguments({} as Settings);
+    const config = await loadCliConfig({}, [], 'test-session', argv);
+    expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
+  });
+
+  it('should not exclude shell tool in non-interactive mode when --allowed-tools="ShellTool(wc)" is set', async () => {
+    process.stdin.isTTY = false;
+    process.argv = [
+      'node',
+      'script.js',
+      '-p',
+      'test',
+      '--allowed-tools',
+      'ShellTool(wc)',
+    ];
+    const argv = await parseArguments({} as Settings);
+    const config = await loadCliConfig({}, [], 'test-session', argv);
+    expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
+  });
 });
 
 describe('loadCliConfig interactive', () => {

packages/cli/src/config/config.ts

@@ -31,6 +31,7 @@ import {
   ShellTool,
   EditTool,
   WriteFileTool,
+  SHELL_TOOL_NAMES,
   resolveTelemetrySettings,
   FatalConfigError,
 } from '@google/gemini-cli-core';
@@ -396,6 +397,36 @@ export async function loadHierarchicalGeminiMemory(
   );
 }
 
+/**
+ * Creates a filter function to determine if a tool should be excluded.
+ *
+ * In non-interactive mode, we want to disable tools that require user
+ * interaction to prevent the CLI from hanging. This function creates a predicate
+ * that returns `true` if a tool should be excluded.
+ *
+ * A tool is excluded if it's not in the `allowedToolsSet`. The shell tool
+ * has a special case: it's not excluded if any of its subcommands
+ * are in the `allowedTools` list.
+ *
+ * @param allowedTools A list of explicitly allowed tool names.
+ * @param allowedToolsSet A set of explicitly allowed tool names for quick lookups.
+ * @returns A function that takes a tool name and returns `true` if it should be excluded.
+ */
+function createToolExclusionFilter(
+  allowedTools: string[],
+  allowedToolsSet: Set<string>,
+) {
+  return (tool: string): boolean => {
+    if (tool === ShellTool.Name) {
+      // If any of the allowed tools is ShellTool (even with subcommands), don't exclude it.
+      return !allowedTools.some((allowed) =>
+        SHELL_TOOL_NAMES.some((shellName) => allowed.startsWith(shellName)),
+      );
+    }
+    return !allowedToolsSet.has(tool);
+  };
+}
+
 export function isDebugMode(argv: CliArgs): boolean {
   return (
     argv.debug ||
@@ -527,6 +558,9 @@ export async function loadCliConfig(
 
   const policyEngineConfig = createPolicyEngineConfig(settings, approvalMode);
 
+  const allowedTools = argv.allowedTools || settings.tools?.allowed || [];
+  const allowedToolsSet = new Set(allowedTools);
+
   // Fix: If promptWords are provided, always use non-interactive mode
   const hasPromptWords = argv.promptWords && argv.promptWords.length > 0;
   const interactive =
@@ -535,14 +569,22 @@ export async function loadCliConfig(
   // In non-interactive mode, exclude tools that require a prompt.
   const extraExcludes: string[] = [];
   if (!interactive && !argv.experimentalAcp) {
+    const defaultExcludes = [ShellTool.Name, EditTool.Name, WriteFileTool.Name];
+    const autoEditExcludes = [ShellTool.Name];
+
+    const toolExclusionFilter = createToolExclusionFilter(
+      allowedTools,
+      allowedToolsSet,
+    );
+
     switch (approvalMode) {
       case ApprovalMode.DEFAULT:
         // In default non-interactive mode, all tools that require approval are excluded.
-        extraExcludes.push(ShellTool.Name, EditTool.Name, WriteFileTool.Name);
+        extraExcludes.push(...defaultExcludes.filter(toolExclusionFilter));
         break;
       case ApprovalMode.AUTO_EDIT:
         // In auto-edit non-interactive mode, only tools that still require a prompt are excluded.
-        extraExcludes.push(ShellTool.Name);
+        extraExcludes.push(...autoEditExcludes.filter(toolExclusionFilter));
         break;
       case ApprovalMode.YOLO:
         // No extra excludes for YOLO mode.
@@ -614,7 +656,7 @@ export async function loadCliConfig(
     question,
     fullContext: argv.allFiles || false,
     coreTools: settings.tools?.core || undefined,
-    allowedTools: argv.allowedTools || settings.tools?.allowed || undefined,
+    allowedTools: allowedTools.length > 0 ? allowedTools : undefined,
     policyEngineConfig,
     excludeTools,
     toolDiscoveryCommand: settings.tools?.discoveryCommand,

packages/cli/src/nonInteractiveCli.test.ts

@@ -873,4 +873,57 @@ describe('runNonInteractive', () => {
 
     expect(processStdoutSpy).toHaveBeenCalledWith('Acknowledged');
   });
+
+  it('should allow a normally-excluded tool when --allowed-tools is set', async () => {
+    // By default, ShellTool is excluded in non-interactive mode.
+    // This test ensures that --allowed-tools overrides this exclusion.
+    vi.mocked(mockConfig.getToolRegistry).mockReturnValue({
+      getTool: vi.fn().mockReturnValue({
+        name: 'ShellTool',
+        description: 'A shell tool',
+        run: vi.fn(),
+      }),
+      getFunctionDeclarations: vi.fn().mockReturnValue([{ name: 'ShellTool' }]),
+    } as unknown as ToolRegistry);
+
+    const toolCallEvent: ServerGeminiStreamEvent = {
+      type: GeminiEventType.ToolCallRequest,
+      value: {
+        callId: 'tool-shell-1',
+        name: 'ShellTool',
+        args: { command: 'ls' },
+        isClientInitiated: false,
+        prompt_id: 'prompt-id-allowed',
+      },
+    };
+    const toolResponse: Part[] = [{ text: 'file.txt' }];
+    mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse });
+
+    const firstCallEvents: ServerGeminiStreamEvent[] = [toolCallEvent];
+    const secondCallEvents: ServerGeminiStreamEvent[] = [
+      { type: GeminiEventType.Content, value: 'file.txt' },
+      {
+        type: GeminiEventType.Finished,
+        value: { reason: undefined, usageMetadata: { totalTokenCount: 10 } },
+      },
+    ];
+
+    mockGeminiClient.sendMessageStream
+      .mockReturnValueOnce(createStreamFromEvents(firstCallEvents))
+      .mockReturnValueOnce(createStreamFromEvents(secondCallEvents));
+
+    await runNonInteractive(
+      mockConfig,
+      mockSettings,
+      'List the files',
+      'prompt-id-allowed',
+    );
+
+    expect(mockCoreExecuteToolCall).toHaveBeenCalledWith(
+      mockConfig,
+      expect.objectContaining({ name: 'ShellTool' }),
+      expect.any(AbortSignal),
+    );
+    expect(processStdoutSpy).toHaveBeenCalledWith('file.txt');
+  });
 });

packages/core/src/tools/shell.test.ts

@@ -61,6 +61,7 @@ describe('ShellTool', () => {
         .mockReturnValue(createMockWorkspaceContext('/test/dir')),
       getGeminiClient: vi.fn(),
       getShouldUseNodePtyShell: vi.fn().mockReturnValue(false),
+      isInteractive: vi.fn().mockReturnValue(true),
     } as unknown as Config;
 
     shellTool = new ShellTool(mockConfig);

packages/core/src/tools/shell.ts

@@ -22,6 +22,7 @@ import {
   ToolConfirmationOutcome,
   Kind,
 } from './tools.js';
+import { ApprovalMode } from '../config/config.js';
 import { getErrorMessage } from '../utils/errors.js';
 import { summarizeToolOutput } from '../utils/summarizer.js';
 import type {
@@ -34,11 +35,58 @@ import type { AnsiOutput } from '../utils/terminalSerializer.js';
 import {
   getCommandRoots,
   isCommandAllowed,
+  SHELL_TOOL_NAMES,
   stripShellWrapper,
 } from '../utils/shell-utils.js';
 
 export const OUTPUT_UPDATE_INTERVAL_MS = 1000;
 
+/**
+ * Parses the `--allowed-tools` flag to determine which sub-commands of the
+ * ShellTool are allowed. The flag can be provided multiple times.
+ *
+ * @param allowedTools The list of allowed tools from the config.
+ * @returns A Set of allowed sub-commands, or null if all commands are allowed.
+ *  - `null`: All sub-commands are allowed (e.g., --allowed-tools="ShellTool").
+ *  - `Set<string>`: A set of specifically allowed sub-commands (e.g., --allowed-tools="ShellTool(wc)" --allowed-tools="ShellTool(ls)").
+ *  - `Set<>` (empty): No sub-commands are allowed (e.g., --allowed-tools="ShellTool()").
+ */
+function parseAllowedSubcommands(
+  allowedTools: readonly string[],
+): Set<string> | null {
+  const shellToolEntries = allowedTools.filter((tool) =>
+    SHELL_TOOL_NAMES.some((name) => tool.startsWith(name)),
+  );
+
+  if (shellToolEntries.length === 0) {
+    return new Set(); // ShellTool not mentioned, so no subcommands are allowed.
+  }
+
+  // If any entry is just "run_shell_command" or "ShellTool", all subcommands are allowed.
+  if (shellToolEntries.some((entry) => SHELL_TOOL_NAMES.includes(entry))) {
+    return null;
+  }
+
+  const allSubcommands = new Set<string>();
+  const toolNamePattern = SHELL_TOOL_NAMES.join('|');
+  const regex = new RegExp(`^(${toolNamePattern})\\((.*)\\)$`);
+
+  for (const entry of shellToolEntries) {
+    const match = entry.match(regex);
+    if (match) {
+      const subcommands = match[2];
+      if (subcommands) {
+        subcommands
+          .split(',')
+          .map((s) => s.trim())
+          .forEach((s) => allSubcommands.add(s));
+      }
+    }
+  }
+
+  return allSubcommands;
+}
+
 export interface ShellToolParams {
   command: string;
   description?: string;
@@ -76,6 +124,30 @@ export class ShellToolInvocation extends BaseToolInvocation<
   ): Promise<ToolCallConfirmationDetails | false> {
     const command = stripShellWrapper(this.params.command);
     const rootCommands = [...new Set(getCommandRoots(command))];
+
+    // In non-interactive mode, we need to prevent the tool from hanging while
+    // waiting for user input. If a tool is not fully allowed (e.g. via
+    // --allowed-tools="ShellTool(wc)"), we should throw an error instead of
+    // prompting for confirmation. This check is skipped in YOLO mode.
+    if (
+      !this.config.isInteractive() &&
+      this.config.getApprovalMode() !== ApprovalMode.YOLO
+    ) {
+      const allowed = this.config.getAllowedTools() || [];
+      const allowedSubcommands = parseAllowedSubcommands(allowed);
+      if (allowedSubcommands !== null) {
+        // Not all commands are allowed, so we need to check.
+        const allCommandsAllowed = rootCommands.every((cmd) =>
+          allowedSubcommands.has(cmd),
+        );
+        if (!allCommandsAllowed) {
+          throw new Error(
+            `Command "${command}" is not in the list of allowed tools for non-interactive mode.`,
+          );
+        }
+      }
+    }
+
     const commandsToConfirm = rootCommands.filter(
       (command) => !this.allowlist.has(command),
     );

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

@@ -11,7 +11,7 @@ import { quote } from 'shell-quote';
 import { doesToolInvocationMatch } from './tool-utils.js';
 import { spawn, type SpawnOptionsWithoutStdio } from 'node:child_process';
 
-const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
+export const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
 
 /**
  * An identifier for the shell type.

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

@@ -36,6 +36,15 @@ describe('doesToolInvocationMatch', () => {
     expect(result).toBe(true);
   });
 
+  it('should match a command with an alias', () => {
+    const invocation = {
+      params: { command: 'wc -l' },
+    } as AnyToolInvocation;
+    const patterns = ['ShellTool(wc)'];
+    const result = doesToolInvocationMatch('ShellTool', invocation, patterns);
+    expect(result).toBe(true);
+  });
+
   it('should match a command that is a prefix', () => {
     const invocation = {
       params: { command: 'git status -v' },

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

@@ -6,8 +6,7 @@
 
 import type { AnyDeclarativeTool, AnyToolInvocation } from '../index.js';
 import { isTool } from '../index.js';
-
+import { SHELL_TOOL_NAMES } from './shell-utils.js';
-const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
 
 /**
  * Checks if a tool invocation matches any of a list of patterns.
@@ -61,7 +60,7 @@ export function doesToolInvocationMatch(
 
     if (
       'command' in invocation.params &&
-      toolNames.includes('run_shell_command')
+      toolNames.some((name) => SHELL_TOOL_NAMES.includes(name))
     ) {
       const argValue = String(
         (invocation.params as { command: string }).command,