Make --allowed-tools work in non-interactive mode (#9114)

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This commit is contained in:
mistergarrison
2025-09-26 10:03:35 -07:00
committed by GitHub
parent 8a16165a9b
commit e8a065cb9f
9 changed files with 388 additions and 7 deletions

View File

@@ -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');

View File

@@ -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', () => {

View File

@@ -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,

View File

@@ -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');
});
});

View File

@@ -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);

View File

@@ -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),
);

View File

@@ -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.

View File

@@ -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' },

View File

@@ -6,8 +6,7 @@
import type { AnyDeclarativeTool, AnyToolInvocation } from '../index.js';
import { isTool } from '../index.js';
const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool'];
import { SHELL_TOOL_NAMES } from './shell-utils.js';
/**
* 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,