display verbiage for auto-executing tools and make sure auto-execute only happens when tools are being sandboxed

This commit is contained in:
A.K.M. Adib
2026-03-30 17:03:02 -04:00
parent 7233000464
commit e5d3235eaf
13 changed files with 294 additions and 14 deletions
+1 -2
View File
@@ -439,8 +439,7 @@ The Gemini CLI ships with a set of default policies to provide a safe
out-of-the-box experience.
- **Read-only tools** (like `read_file`, `glob`) are generally **allowed**.
- **MCP Read-only tools**: MCP tools that explicitly declare themselves as
read-only via the `readOnlyHint` annotation are automatically allowed.
- **MCP Read-only tools**: MCP tools that explicitly declare themselves as read-only via the `readOnlyHint` annotation are automatically allowed, but **only if tool sandboxing is enabled**. If sandboxing is disabled, they default to `ask_user`.
- **Agent delegation** defaults to **`ask_user`** to ensure remote agents can
prompt for confirmation, but local sub-agent actions are executed silently and
checked individually.
+1 -4
View File
@@ -653,10 +653,7 @@ When confirmation is required, users can choose:
#### Read-only bypass
If an MCP tool provides a `readOnlyHint` in its metadata (annotations), the
Gemini CLI will automatically execute it without prompting for confirmation,
regardless of the configured approval mode. This ensures a seamless experience
for safe, data-fetching operations.
If an MCP tool provides a `readOnlyHint` in its metadata (annotations) AND tool sandboxing is enabled, the Gemini CLI will automatically execute it without prompting for confirmation, regardless of the configured approval mode. This ensures a seamless experience for safe, data-fetching operations while maintaining system security.
### 3. Execution
+56
View File
@@ -27,6 +27,33 @@ vi.mock('fs/promises', () => ({
writeFile: vi.fn(),
}));
vi.mock('@modelcontextprotocol/sdk/client/index.js', () => {
class MockClient {
async connect() {}
async listTools() {
return {
tools: [
{ name: 'read_only_tool', annotations: { readOnlyHint: true } },
{ name: 'write_tool', annotations: { readOnlyHint: false } },
],
};
}
async close() {}
}
return {
Client: MockClient,
};
});
vi.mock('@google/gemini-cli-core', async () => {
const actual = await vi.importActual('@google/gemini-cli-core');
return {
...actual,
createTransport: vi.fn().mockResolvedValue({}),
createSandboxManager: vi.fn().mockReturnValue({}),
};
});
vi.mock('os', () => {
const homedir = vi.fn(() => '/home/user');
return {
@@ -68,6 +95,33 @@ describe('mcp add command', () => {
setValue: mockSetValue,
workspace: { path: '/path/to/project' },
user: { path: '/home/user' },
merged: {},
});
});
describe('sandboxing warnings', () => {
it('should discover tools and emit warning when sandboxing is enabled', async () => {
mockedLoadSettings.mockReturnValue({
forScope: () => ({ settings: {} }),
setValue: mockSetValue,
workspace: { path: '/path/to/project' },
user: { path: '/home/user' },
merged: {
tools: {
sandbox: { enabled: true },
},
},
});
const debugLoggerWarnSpy = vi.spyOn(debugLogger, 'warn').mockImplementation(() => {});
await parser.parseAsync('add sandbox-server /path/to/server');
expect(debugLoggerWarnSpy).toHaveBeenCalledWith(
expect.stringContaining(
'Warning: With sandboxing enabled, the following tools are marked as read-only and will AUTO-EXECUTE without confirmation:\n * read_only_tool',
),
);
});
});
@@ -216,6 +270,7 @@ describe('mcp add command', () => {
setValue: mockSetValue,
workspace: { path: workspacePath },
user: { path: '/home/user' },
merged: {},
});
};
@@ -383,6 +438,7 @@ describe('mcp add command', () => {
setValue: mockSetValue,
workspace: { path: '/path/to/project' },
user: { path: '/home/user' },
merged: {},
});
});
+64 -1
View File
@@ -7,7 +7,14 @@
// File for 'gemini mcp add' command
import type { CommandModule } from 'yargs';
import { loadSettings, SettingScope } from '../../config/settings.js';
import { debugLogger, type MCPServerConfig } from '@google/gemini-cli-core';
import {
debugLogger,
type MCPServerConfig,
type McpContext,
createTransport,
createSandboxManager,
} from '@google/gemini-cli-core';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { exitCli } from '../utils.js';
async function addMcpServer(
@@ -135,6 +142,62 @@ async function addMcpServer(
`MCP server "${name}" added to ${scope} settings. (${transport})`,
);
}
const isSandboxEnabled =
typeof settings.merged.tools?.sandbox === 'object'
? settings.merged.tools.sandbox.enabled
: !!settings.merged.tools?.sandbox;
if (isSandboxEnabled) {
const mcpContext: McpContext = {
sanitizationConfig: {
enableEnvironmentVariableRedaction: true,
allowedEnvironmentVariables: [],
blockedEnvironmentVariables: settings.merged.advanced?.excludedEnvVars ?? [],
},
emitMcpDiagnostic: () => {},
isTrustedFolder: () => true,
isSandboxEnabled: () => true,
sandboxManager: createSandboxManager(
{ enabled: true },
{ workspace: process.cwd(), modeConfig: { readonly: true } },
),
};
try {
const serverTransport = await createTransport(
name,
newServer as MCPServerConfig,
false,
mcpContext,
);
const client = new Client(
{ name: 'gemini-cli-discovery', version: '1.0.0' },
{ capabilities: {} },
);
await client.connect(serverTransport);
const result = await client.listTools();
const readOnlyTools = result.tools
.filter(
(t: { name: string; annotations?: { readOnlyHint?: boolean } }) =>
t.annotations?.readOnlyHint === true,
)
.map((t) => t.name);
if (readOnlyTools.length > 0) {
debugLogger.warn(
`Warning: With sandboxing enabled, the following tools are marked as read-only and will AUTO-EXECUTE without confirmation:\n${readOnlyTools
.map((t) => ` * ${t}`)
.join('\n')}`,
);
}
await client.close();
} catch (e) {
debugLogger.warn(
'Warning: With sandboxing enabled, any read-only tools provided by this server will AUTO-EXECUTE without confirmation.',
);
}
}
}
export const addCommand: CommandModule = {
+3
View File
@@ -17,6 +17,7 @@ import {
debugLogger,
applyAdminAllowlist,
getAdminBlockedMcpServersMessage,
NoopSandboxManager,
} from '@google/gemini-cli-core';
import type { MCPServerConfig } from '@google/gemini-cli-core';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
@@ -115,6 +116,8 @@ async function testMCPConnection(
}
},
isTrustedFolder: () => isTrusted,
isSandboxEnabled: () => false,
sandboxManager: new NoopSandboxManager(),
};
let transport;
+4
View File
@@ -2831,6 +2831,10 @@ export class Config implements McpContext, AgentLoopContext {
return this.folderTrust ? (this.trustedFolder ?? false) : true;
}
isSandboxEnabled(): boolean {
return this.sandbox?.enabled ?? false;
}
setIdeMode(value: boolean): void {
this.ideMode = value;
}
+1
View File
@@ -157,6 +157,7 @@ export * from './ide/types.js';
// Export Shell Execution Service
export * from './services/shellExecutionService.js';
export * from './services/sandboxManager.js';
export { createSandboxManager } from './services/sandboxManagerFactory.js';
// Export Execution Lifecycle Service
export * from './services/executionLifecycleService.js';
@@ -61,6 +61,7 @@ describe('McpClientManager', () => {
isInitialized: vi.fn(),
}),
refreshMcpContext: vi.fn(),
isSandboxEnabled: vi.fn().mockReturnValue(false),
} as unknown as Config);
toolRegistry = vi.mockObject({
registerTool: vi.fn(),
@@ -821,4 +822,70 @@ describe('McpClientManager', () => {
expect(coreEventsMock.emitFeedback).toHaveBeenCalledTimes(2); // Now the actual error
});
});
describe('auto-executing tools warning', () => {
let coreEventsMock: typeof import('../utils/events.js').coreEvents;
beforeEach(async () => {
const eventsModule = await import('../utils/events.js');
coreEventsMock = eventsModule.coreEvents;
vi.spyOn(coreEventsMock, 'emitFeedback').mockImplementation(() => {});
mockedMcpClient = vi.mockObject({
connect: vi.fn(),
discoverInto: vi.fn(),
disconnect: vi.fn(),
getStatus: vi.fn().mockReturnValue(MCPServerStatus.DISCONNECTED),
getServerConfig: vi.fn(),
getServerName: vi.fn().mockReturnValue('test-server'),
getAutoExecutingTools: vi.fn().mockReturnValue([]),
} as unknown as McpClient);
vi.mocked(McpClient).mockReturnValue(mockedMcpClient);
});
it('should warn about auto-executing tools if sandboxing is enabled', async () => {
mockConfig.isSandboxEnabled.mockReturnValue(true);
mockConfig.getMcpServers.mockReturnValue({
'test-server': { command: 'node' },
});
mockedMcpClient.getAutoExecutingTools.mockReturnValue([
'[test-server] tool1',
'[test-server] tool2',
]);
const manager = setupManager(new McpClientManager('0.0.1', mockConfig));
await manager.startConfiguredMcpServers();
expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith(
'warning',
expect.stringContaining(
'With sandboxing enabled, the following MCP tools will AUTO-EXECUTE',
),
);
expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith(
'warning',
expect.stringContaining('* [test-server] tool1'),
);
expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith(
'warning',
expect.stringContaining('* [test-server] tool2'),
);
});
it('should not warn if sandboxing is disabled', async () => {
mockConfig.isSandboxEnabled.mockReturnValue(false);
mockConfig.getMcpServers.mockReturnValue({
'test-server': { command: 'node' },
});
mockedMcpClient.getAutoExecutingTools.mockReturnValue(['tool1']);
const manager = setupManager(new McpClientManager('0.0.1', mockConfig));
await manager.startConfiguredMcpServers();
expect(coreEventsMock.emitFeedback).not.toHaveBeenCalledWith(
'warning',
expect.stringContaining('AUTO-EXECUTE'),
);
});
});
});
@@ -63,6 +63,8 @@ export class McpClientManager {
*/
private shownDiagnostics: Map<string, 'silent' | 'verbose'> = new Map();
private warnedAutoExecutingTools = new Set<string>();
/**
* Track whether the MCP "hint" has been shown.
*/
@@ -706,6 +708,7 @@ export class McpClientManager {
this.pendingMcpContextRefresh = false;
debugLogger.log('Executing MCP context refresh...');
await this.cliConfig.refreshMcpContext();
this.warnAboutAutoExecutingTools();
debugLogger.log('MCP context refresh complete.');
// If more refresh requests came in during the execution, wait a bit
@@ -730,6 +733,29 @@ export class McpClientManager {
return this.pendingRefreshPromise;
}
private warnAboutAutoExecutingTools() {
if (!this.cliConfig.isSandboxEnabled()) {
return;
}
const toolsToWarn: string[] = [];
for (const client of this.clients.values()) {
for (const toolName of client.getAutoExecutingTools()) {
if (!this.warnedAutoExecutingTools.has(toolName)) {
toolsToWarn.push(toolName);
this.warnedAutoExecutingTools.add(toolName);
}
}
}
if (toolsToWarn.length > 0) {
coreEvents.emitFeedback(
'warning',
`With sandboxing enabled, the following MCP tools will AUTO-EXECUTE without confirmation:\n${toolsToWarn
.map((t) => ` * ${t}`)
.join('\n')}`,
);
}
}
getMcpServerCount(): number {
return this.clients.size;
}
@@ -28,6 +28,7 @@ import {
import type { DiscoveredMCPTool } from './mcp-tool.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
import { NoopSandboxManager } from '../services/sandboxManager.js';
import {
connectToMcpServer,
createTransport,
@@ -63,6 +64,8 @@ const MOCK_CONTEXT_DEFAULT = {
emitMcpDiagnostic: vi.fn(),
setUserInteractedWithMcp: vi.fn(),
isTrustedFolder: vi.fn().mockReturnValue(true),
isSandboxEnabled: vi.fn().mockReturnValue(false),
sandboxManager: new NoopSandboxManager(),
};
let MOCK_CONTEXT: McpContext = MOCK_CONTEXT_DEFAULT;
@@ -93,6 +96,8 @@ describe('mcp-client', () => {
emitMcpDiagnostic: vi.fn(),
setUserInteractedWithMcp: vi.fn(),
isTrustedFolder: vi.fn().mockReturnValue(true),
isSandboxEnabled: vi.fn().mockReturnValue(false),
sandboxManager: new NoopSandboxManager(),
};
// create a tmp dir for this test
// Create a unique temporary directory for the workspace to avoid conflicts
+36 -4
View File
@@ -80,6 +80,7 @@ import {
type EnvironmentSanitizationConfig,
} from '../services/environmentSanitization.js';
import { expandEnvVars } from '../utils/envExpansion.js';
import type { SandboxManager } from '../services/sandboxManager.js';
import {
GEMINI_CLI_IDENTIFICATION_ENV_VAR,
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
@@ -161,6 +162,8 @@ export class McpClient implements McpProgressReporter {
*/
private readonly progressTokenToCallId = new Map<string | number, string>();
private autoExecutingTools: string[] = [];
constructor(
private readonly serverName: string,
private readonly serverConfig: MCPServerConfig,
@@ -175,6 +178,10 @@ export class McpClient implements McpProgressReporter {
return this.serverName;
}
getAutoExecutingTools(): string[] {
return this.autoExecutingTools;
}
/**
* Connects to the MCP server.
*/
@@ -236,6 +243,10 @@ export class McpClient implements McpProgressReporter {
const resources = await this.discoverResources();
this.updateResourceRegistry(resources, registries.resourceRegistry);
this.autoExecutingTools = tools
.filter((t) => t.isReadOnly)
.map((t) => `[${t.serverName}] ${t.serverToolName}`);
if (prompts.length === 0 && tools.length === 0 && resources.length === 0) {
throw new Error('No prompts, tools, or resources found on the server.');
}
@@ -1754,6 +1765,8 @@ export interface McpContext {
): void;
setUserInteractedWithMcp?(): void;
isTrustedFolder(): boolean;
isSandboxEnabled(): boolean;
sandboxManager?: SandboxManager;
getPolicyEngine?(): {
getRules(): ReadonlyArray<{
toolName: string;
@@ -2275,11 +2288,29 @@ export async function createTransport(
}
}
let preparedProgram = mcpServerConfig.command;
let preparedArgs = mcpServerConfig.args || [];
let preparedEnv = finalEnv;
let preparedCwd = mcpServerConfig.cwd || process.cwd();
if (cliConfig.isSandboxEnabled() && cliConfig.sandboxManager) {
const prepared = await cliConfig.sandboxManager.prepareCommand({
command: preparedProgram,
args: preparedArgs,
cwd: preparedCwd,
env: preparedEnv,
});
preparedProgram = prepared.program;
preparedArgs = prepared.args;
preparedCwd = prepared.cwd || preparedCwd;
preparedEnv = prepared.env as Record<string, string>;
}
let transport: Transport = new StdioClientTransport({
command: mcpServerConfig.command,
args: mcpServerConfig.args || [],
env: finalEnv,
cwd: mcpServerConfig.cwd,
command: preparedProgram,
args: preparedArgs,
env: preparedEnv,
cwd: preparedCwd,
stderr: 'pipe',
});
@@ -2369,3 +2400,4 @@ export function isEnabled(
)
);
}
+28 -1
View File
@@ -1022,7 +1022,7 @@ describe('DiscoveredMCPTool', () => {
});
describe('shouldConfirmExecute with read-only hint', () => {
it('should return false if isReadOnly is true', async () => {
it('should return false if isReadOnly and isSandboxEnabled are true', async () => {
const bus = createMockMessageBus();
getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user';
const readOnlyTool = new DiscoveredMCPTool(
@@ -1034,6 +1034,8 @@ describe('DiscoveredMCPTool', () => {
bus,
false, // trust
true, // isReadOnly
undefined, // nameOverride
{ isSandboxEnabled: () => true, isTrustedFolder: () => true } as any, // cliConfig
);
const invocation = readOnlyTool.build({ param: 'mock' });
expect(
@@ -1041,6 +1043,29 @@ describe('DiscoveredMCPTool', () => {
).toBe(false);
});
it('should return confirmation details if isReadOnly is true but isSandboxEnabled is false', async () => {
const bus = createMockMessageBus();
getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user';
const readOnlyTool = new DiscoveredMCPTool(
mockCallableToolInstance,
serverName,
serverToolName,
baseDescription,
inputSchema,
bus,
false, // trust
true, // isReadOnly
undefined, // nameOverride
{ isSandboxEnabled: () => false, isTrustedFolder: () => true } as any, // cliConfig
);
const invocation = readOnlyTool.build({ param: 'mock' });
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).not.toBe(false);
expect(confirmation).toHaveProperty('type', 'mcp');
});
it('should return confirmation details if isReadOnly is false', async () => {
const bus = createMockMessageBus();
getMockMessageBusInstance(bus).defaultToolDecision = 'ask_user';
@@ -1053,6 +1078,8 @@ describe('DiscoveredMCPTool', () => {
bus,
false, // trust
false, // isReadOnly
undefined, // nameOverride
{ isSandboxEnabled: () => true, isTrustedFolder: () => true } as any, // cliConfig
);
const invocation = readWriteTool.build({ param: 'mock' });
const confirmation = await invocation.shouldConfirmExecute(
+2 -2
View File
@@ -206,8 +206,8 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation<
return false; // server is trusted, no confirmation needed
}
if (this.isReadOnly) {
return false; // read-only tools do not require confirmation
if (this.isReadOnly && this.cliConfig?.isSandboxEnabled()) {
return false; // read-only tools do not require confirmation if sandboxing is enabled
}
if (