Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 116 additions & 0 deletions packages/cli/src/acp/acpSession.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import {
InvalidStreamError,
GeminiEventType,
type ServerGeminiStreamEvent,
PolicyDecision,
MessageBusType,
type ToolConfirmationRequest,
} from '@google/gemini-cli-core';
import type { LoadedSettings } from '../config/settings.js';
import { type Part, FinishReason } from '@google/genai';
Expand Down Expand Up @@ -139,6 +142,9 @@ describe('Session', () => {
isPlanEnabled: vi.fn().mockReturnValue(true),
getCheckpointingEnabled: vi.fn().mockReturnValue(false),
getGitService: vi.fn().mockResolvedValue({} as GitService),
getPolicyEngine: vi.fn().mockReturnValue({
check: vi.fn(),
}),
validatePathAccess: vi.fn().mockReturnValue(null),
getWorkspaceContext: vi.fn().mockReturnValue({
addReadOnlyPath: vi.fn(),
Expand Down Expand Up @@ -707,4 +713,114 @@ describe('Session', () => {
}),
);
});

describe('Policy Handling', () => {
it('should auto-approve tool calls when PolicyEngine returns ALLOW', async () => {
const mockPolicyEngine = mockConfig.getPolicyEngine() as unknown as {
check: Mock<
(
toolCall: { name: string; args: Record<string, unknown> },
serverName?: string,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
) => Promise<{ decision: PolicyDecision }>
>;
};
mockPolicyEngine.check.mockResolvedValue({
decision: PolicyDecision.ALLOW,
});

// Trigger the subscription handler
const handler = mockMessageBus.subscribe.mock.calls.find(
(call) => call[0] === MessageBusType.TOOL_CONFIRMATION_REQUEST,
)?.[1] as (request: ToolConfirmationRequest) => Promise<void>;

expect(handler).toBeDefined();

await handler({
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
correlationId: 'test-id',
toolCall: { name: 'ls', args: {} },
});

expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: 'test-id',
confirmed: true,
requiresUserConfirmation: false,
}),
);
});

it('should request user confirmation when PolicyEngine returns ASK_USER', async () => {
const mockPolicyEngine = mockConfig.getPolicyEngine() as unknown as {
check: Mock<
(
toolCall: { name: string; args: Record<string, unknown> },
serverName?: string,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
) => Promise<{ decision: PolicyDecision }>
>;
};
mockPolicyEngine.check.mockResolvedValue({
decision: PolicyDecision.ASK_USER,
});

const handler = mockMessageBus.subscribe.mock.calls.find(
(call) => call[0] === MessageBusType.TOOL_CONFIRMATION_REQUEST,
)?.[1] as (request: ToolConfirmationRequest) => Promise<void>;

await handler({
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
correlationId: 'test-id-2',
toolCall: { name: 'rm', args: { path: '/' } },
});

expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: 'test-id-2',
confirmed: false,
requiresUserConfirmation: true,
}),
);
});

it('should deny tool calls when PolicyEngine returns DENY', async () => {
const mockPolicyEngine = mockConfig.getPolicyEngine() as unknown as {
check: Mock<
(
toolCall: { name: string; args: Record<string, unknown> },
serverName?: string,
toolAnnotations?: Record<string, unknown>,
subagent?: string,
) => Promise<{ decision: PolicyDecision }>
>;
};
mockPolicyEngine.check.mockResolvedValue({
decision: PolicyDecision.DENY,
});

const handler = mockMessageBus.subscribe.mock.calls.find(
(call) => call[0] === MessageBusType.TOOL_CONFIRMATION_REQUEST,
)?.[1] as (request: ToolConfirmationRequest) => Promise<void>;

await handler({
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
correlationId: 'test-id-3',
toolCall: { name: 'forbidden', args: {} },
});

expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: 'test-id-3',
confirmed: false,
requiresUserConfirmation: false,
}),
);
});
});
});
37 changes: 37 additions & 0 deletions packages/cli/src/acp/acpSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ import {
isNodeError,
REFERENCE_CONTENT_START,
InvalidStreamError,
MessageBusType,
PolicyDecision,
type ToolConfirmationRequest,
} from '@google/gemini-cli-core';
import * as acp from '@agentclientprotocol/sdk';
import type { Part, FunctionCall } from '@google/genai';
Expand Down Expand Up @@ -61,6 +64,7 @@ export class Session {
private pendingPrompt: AbortController | null = null;
private commandHandler = new CommandHandler();
private callIdCounter = 0;
private readonly disposeController = new AbortController();

private generateCallId(name: string): string {
return `${name}-${Date.now()}-${++this.callIdCounter}`;
Expand All @@ -77,8 +81,40 @@ export class Session {
CoreEvent.ApprovalModeChanged,
this.handleApprovalModeChanged,
);

// Subscribe to tool confirmation requests to handle policy checks (e.g. auto-allowing safe shell commands)
this.context.config
.getMessageBus()
?.subscribe(
MessageBusType.TOOL_CONFIRMATION_REQUEST,
this.handleToolConfirmationRequest,
{ signal: this.disposeController.signal },
);
Comment thread
cocosheng-g marked this conversation as resolved.
}

private handleToolConfirmationRequest = async (
request: ToolConfirmationRequest,
) => {
const policyEngine = this.context.config.getPolicyEngine?.();
if (!policyEngine) {
return;
}

const result = await policyEngine.check(
request.toolCall,
request.serverName,
request.toolAnnotations,
undefined, // subagent
);
Comment thread
cocosheng-g marked this conversation as resolved.
Outdated

await this.context.config.getMessageBus()?.publish({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
correlationId: request.correlationId,
confirmed: result.decision === PolicyDecision.ALLOW,
requiresUserConfirmation: result.decision === PolicyDecision.ASK_USER,
});
};

private handleApprovalModeChanged = (payload: ApprovalModeChangedPayload) => {
if (payload.sessionId === this.id) {
void this.sendUpdate({
Expand All @@ -96,6 +132,7 @@ export class Session {
CoreEvent.ApprovalModeChanged,
this.handleApprovalModeChanged,
);
this.disposeController.abort();
}

async cancelPendingPrompt(): Promise<void> {
Expand Down
Loading