mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-21 10:34:35 -07:00
feat(policy): support auto-add to policy by default and scoped persistence
This commit is contained in:
@@ -548,6 +548,7 @@ export interface ConfigParameters {
|
||||
truncateToolOutputThreshold?: number;
|
||||
eventEmitter?: EventEmitter;
|
||||
useWriteTodos?: boolean;
|
||||
workspacePoliciesDir?: string;
|
||||
policyEngineConfig?: PolicyEngineConfig;
|
||||
directWebFetch?: boolean;
|
||||
policyUpdateConfirmationRequest?: PolicyUpdateConfirmationRequest;
|
||||
@@ -740,6 +741,7 @@ export class Config implements McpContext {
|
||||
private readonly fileExclusions: FileExclusions;
|
||||
private readonly eventEmitter?: EventEmitter;
|
||||
private readonly useWriteTodos: boolean;
|
||||
private readonly workspacePoliciesDir: string | undefined;
|
||||
private readonly messageBus: MessageBus;
|
||||
private readonly policyEngine: PolicyEngine;
|
||||
private policyUpdateConfirmationRequest:
|
||||
@@ -951,6 +953,7 @@ export class Config implements McpContext {
|
||||
this.useWriteTodos = isPreviewModel(this.model)
|
||||
? false
|
||||
: (params.useWriteTodos ?? true);
|
||||
this.workspacePoliciesDir = params.workspacePoliciesDir;
|
||||
this.enableHooksUI = params.enableHooksUI ?? true;
|
||||
this.enableHooks = params.enableHooks ?? true;
|
||||
this.disabledHooks = params.disabledHooks ?? [];
|
||||
@@ -1956,6 +1959,10 @@ export class Config implements McpContext {
|
||||
return this.geminiMdFilePaths;
|
||||
}
|
||||
|
||||
getWorkspacePoliciesDir(): string | undefined {
|
||||
return this.workspacePoliciesDir;
|
||||
}
|
||||
|
||||
setGeminiMdFilePaths(paths: string[]): void {
|
||||
this.geminiMdFilePaths = paths;
|
||||
}
|
||||
|
||||
@@ -168,6 +168,13 @@ export class Storage {
|
||||
return path.join(this.getGeminiDir(), 'policies');
|
||||
}
|
||||
|
||||
getWorkspaceAutoSavedPolicyPath(): string {
|
||||
return path.join(
|
||||
this.getWorkspacePoliciesDir(),
|
||||
AUTO_SAVED_POLICY_FILENAME,
|
||||
);
|
||||
}
|
||||
|
||||
getAutoSavedPolicyPath(): string {
|
||||
return path.join(Storage.getUserPoliciesDir(), AUTO_SAVED_POLICY_FILENAME);
|
||||
}
|
||||
|
||||
@@ -118,6 +118,7 @@ export interface UpdatePolicy {
|
||||
type: MessageBusType.UPDATE_POLICY;
|
||||
toolName: string;
|
||||
persist?: boolean;
|
||||
persistScope?: 'workspace' | 'user';
|
||||
argsPattern?: string;
|
||||
commandPrefix?: string | string[];
|
||||
mcpName?: string;
|
||||
|
||||
@@ -520,9 +520,21 @@ export function createPolicyUpdater(
|
||||
if (message.persist) {
|
||||
persistenceQueue = persistenceQueue.then(async () => {
|
||||
try {
|
||||
const policyFile = storage.getAutoSavedPolicyPath();
|
||||
const policyFile =
|
||||
message.persistScope === 'workspace'
|
||||
? storage.getWorkspaceAutoSavedPolicyPath()
|
||||
: storage.getAutoSavedPolicyPath();
|
||||
await fs.mkdir(path.dirname(policyFile), { recursive: true });
|
||||
|
||||
// Backup existing file if it exists
|
||||
try {
|
||||
await fs.copyFile(policyFile, `${policyFile}.bak`);
|
||||
} catch (error) {
|
||||
if (!isNodeError(error) || error.code !== 'ENOENT') {
|
||||
debugLogger.warn(`Failed to backup ${policyFile}`, error);
|
||||
}
|
||||
}
|
||||
|
||||
// Read existing file
|
||||
let existingData: { rule?: TomlRule[] } = {};
|
||||
try {
|
||||
|
||||
@@ -230,15 +230,87 @@ describe('createPolicyUpdater', () => {
|
||||
// Note: @iarna/toml optimizes for shortest representation, so it may use single quotes 'foo"bar'
|
||||
// instead of "foo\"bar\"" if there are no single quotes in the string.
|
||||
try {
|
||||
expect(writtenContent).toContain(`mcpName = "my\\"jira\\"server"`);
|
||||
expect(writtenContent).toContain('mcpName = "my\\"jira\\"server"');
|
||||
} catch {
|
||||
expect(writtenContent).toContain(`mcpName = 'my"jira"server'`);
|
||||
expect(writtenContent).toContain('mcpName = \'my"jira"server\'');
|
||||
}
|
||||
|
||||
try {
|
||||
expect(writtenContent).toContain(`toolName = "search\\"tool\\""`);
|
||||
expect(writtenContent).toContain('toolName = "search\\"tool\\""');
|
||||
} catch {
|
||||
expect(writtenContent).toContain(`toolName = 'search"tool"'`);
|
||||
expect(writtenContent).toContain('toolName = \'search"tool"\'');
|
||||
}
|
||||
});
|
||||
|
||||
it('should persist to workspace when persistScope is workspace', async () => {
|
||||
createPolicyUpdater(policyEngine, messageBus, mockStorage);
|
||||
|
||||
const workspacePoliciesDir = '/mock/project/.gemini/policies';
|
||||
const policyFile = path.join(
|
||||
workspacePoliciesDir,
|
||||
AUTO_SAVED_POLICY_FILENAME,
|
||||
);
|
||||
vi.spyOn(mockStorage, 'getWorkspaceAutoSavedPolicyPath').mockReturnValue(
|
||||
policyFile,
|
||||
);
|
||||
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
|
||||
(fs.readFile as unknown as Mock).mockRejectedValue(
|
||||
new Error('File not found'),
|
||||
);
|
||||
(fs.copyFile as unknown as Mock).mockResolvedValue(undefined);
|
||||
|
||||
const mockFileHandle = {
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
|
||||
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
|
||||
|
||||
await messageBus.publish({
|
||||
type: MessageBusType.UPDATE_POLICY,
|
||||
toolName: 'test_tool',
|
||||
persist: true,
|
||||
persistScope: 'workspace',
|
||||
});
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(mockStorage.getWorkspaceAutoSavedPolicyPath).toHaveBeenCalled();
|
||||
expect(fs.mkdir).toHaveBeenCalledWith(workspacePoliciesDir, {
|
||||
recursive: true,
|
||||
});
|
||||
expect(fs.rename).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/\.tmp$/),
|
||||
policyFile,
|
||||
);
|
||||
});
|
||||
|
||||
it('should backup existing policy file before writing', async () => {
|
||||
createPolicyUpdater(policyEngine, messageBus, mockStorage);
|
||||
|
||||
const policyFile = '/mock/user/.gemini/policies/auto-saved.toml';
|
||||
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
|
||||
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
|
||||
(fs.readFile as unknown as Mock).mockResolvedValue(
|
||||
'[[rule]]\ntoolName = "existing"',
|
||||
);
|
||||
(fs.copyFile as unknown as Mock).mockResolvedValue(undefined);
|
||||
|
||||
const mockFileHandle = {
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
(fs.open as unknown as Mock).mockResolvedValue(mockFileHandle);
|
||||
(fs.rename as unknown as Mock).mockResolvedValue(undefined);
|
||||
|
||||
await messageBus.publish({
|
||||
type: MessageBusType.UPDATE_POLICY,
|
||||
toolName: 'new_tool',
|
||||
persist: true,
|
||||
});
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(fs.copyFile).toHaveBeenCalledWith(policyFile, `${policyFile}.bak`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -82,3 +82,15 @@ export function buildArgsPatterns(
|
||||
|
||||
return [argsPattern];
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a regex pattern to match a specific file path in tool arguments.
|
||||
* This is used to narrow tool approvals for edit tools to specific files.
|
||||
*
|
||||
* @param filePath The relative path to the file.
|
||||
* @returns A regex string that matches "file_path":"<path>" in a JSON string.
|
||||
*/
|
||||
export function buildFilePathArgsPattern(filePath: string): string {
|
||||
const jsonPath = JSON.stringify(filePath).slice(1, -1);
|
||||
return `"file_path":"${escapeRegex(jsonPath)}"`;
|
||||
}
|
||||
|
||||
@@ -16,7 +16,10 @@ import {
|
||||
import { checkPolicy, updatePolicy, getPolicyDenialError } from './policy.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import { MessageBusType } from '../confirmation-bus/types.js';
|
||||
import {
|
||||
MessageBusType,
|
||||
type SerializableConfirmationDetails,
|
||||
} from '../confirmation-bus/types.js';
|
||||
import { ApprovalMode, PolicyDecision } from '../policy/types.js';
|
||||
import {
|
||||
ToolConfirmationOutcome,
|
||||
@@ -198,6 +201,8 @@ describe('policy.ts', () => {
|
||||
|
||||
it('should handle standard policy updates with persistence', async () => {
|
||||
const mockConfig = {
|
||||
isTrustedFolder: vi.fn().mockReturnValue(false),
|
||||
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
|
||||
setApprovalMode: vi.fn(),
|
||||
} as unknown as Mocked<Config>;
|
||||
const mockMessageBus = {
|
||||
@@ -408,6 +413,8 @@ describe('policy.ts', () => {
|
||||
|
||||
it('should handle MCP ProceedAlwaysAndSave (persist: true)', async () => {
|
||||
const mockConfig = {
|
||||
isTrustedFolder: vi.fn().mockReturnValue(false),
|
||||
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
|
||||
setApprovalMode: vi.fn(),
|
||||
} as unknown as Mocked<Config>;
|
||||
const mockMessageBus = {
|
||||
@@ -439,6 +446,92 @@ describe('policy.ts', () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should determine persistScope: workspace in trusted folders', async () => {
|
||||
const mockConfig = {
|
||||
isTrustedFolder: vi.fn().mockReturnValue(true),
|
||||
getWorkspacePoliciesDir: vi
|
||||
.fn()
|
||||
.mockReturnValue('/mock/project/policies'),
|
||||
setApprovalMode: vi.fn(),
|
||||
} as unknown as Mocked<Config>;
|
||||
const mockMessageBus = {
|
||||
publish: vi.fn(),
|
||||
} as unknown as Mocked<MessageBus>;
|
||||
const tool = { name: 'test-tool' } as AnyDeclarativeTool;
|
||||
|
||||
await updatePolicy(
|
||||
tool,
|
||||
ToolConfirmationOutcome.ProceedAlwaysAndSave,
|
||||
undefined,
|
||||
{ config: mockConfig, messageBus: mockMessageBus },
|
||||
);
|
||||
|
||||
expect(mockMessageBus.publish).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
persistScope: 'workspace',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should determine persistScope: user in untrusted folders', async () => {
|
||||
const mockConfig = {
|
||||
isTrustedFolder: vi.fn().mockReturnValue(false),
|
||||
getWorkspacePoliciesDir: vi
|
||||
.fn()
|
||||
.mockReturnValue('/mock/project/policies'),
|
||||
setApprovalMode: vi.fn(),
|
||||
} as unknown as Mocked<Config>;
|
||||
const mockMessageBus = {
|
||||
publish: vi.fn(),
|
||||
} as unknown as Mocked<MessageBus>;
|
||||
const tool = { name: 'test-tool' } as AnyDeclarativeTool;
|
||||
|
||||
await updatePolicy(
|
||||
tool,
|
||||
ToolConfirmationOutcome.ProceedAlwaysAndSave,
|
||||
undefined,
|
||||
{ config: mockConfig, messageBus: mockMessageBus },
|
||||
);
|
||||
|
||||
expect(mockMessageBus.publish).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
persistScope: 'user',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should narrow edit tools with argsPattern', async () => {
|
||||
const mockConfig = {
|
||||
isTrustedFolder: vi.fn().mockReturnValue(false),
|
||||
getWorkspacePoliciesDir: vi.fn().mockReturnValue(undefined),
|
||||
setApprovalMode: vi.fn(),
|
||||
} as unknown as Mocked<Config>;
|
||||
const mockMessageBus = {
|
||||
publish: vi.fn(),
|
||||
} as unknown as Mocked<MessageBus>;
|
||||
const tool = { name: 'write_file' } as AnyDeclarativeTool;
|
||||
const details = {
|
||||
type: 'edit',
|
||||
filePath: 'src/foo.ts',
|
||||
title: 'Edit',
|
||||
onConfirm: vi.fn(),
|
||||
} as unknown as SerializableConfirmationDetails;
|
||||
|
||||
await updatePolicy(
|
||||
tool,
|
||||
ToolConfirmationOutcome.ProceedAlwaysAndSave,
|
||||
details,
|
||||
{ config: mockConfig, messageBus: mockMessageBus },
|
||||
);
|
||||
|
||||
expect(mockMessageBus.publish).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
toolName: 'write_file',
|
||||
argsPattern: '"file_path":"src/foo\\.ts"',
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getPolicyDenialError', () => {
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
type AnyDeclarativeTool,
|
||||
type PolicyUpdateOptions,
|
||||
} from '../tools/tools.js';
|
||||
import { buildFilePathArgsPattern } from '../policy/utils.js';
|
||||
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
|
||||
import { EDIT_TOOL_NAMES } from '../tools/tool-names.js';
|
||||
import type { ValidatingToolCall } from './types.js';
|
||||
@@ -102,6 +103,20 @@ export async function updatePolicy(
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine persist scope if we are persisting.
|
||||
let persistScope: 'workspace' | 'user' | undefined;
|
||||
if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) {
|
||||
// If folder is trusted and workspace policies are enabled, we prefer workspace scope.
|
||||
if (
|
||||
deps.config.isTrustedFolder() &&
|
||||
deps.config.getWorkspacePoliciesDir()
|
||||
) {
|
||||
persistScope = 'workspace';
|
||||
} else {
|
||||
persistScope = 'user';
|
||||
}
|
||||
}
|
||||
|
||||
// Specialized Tools (MCP)
|
||||
if (confirmationDetails?.type === 'mcp') {
|
||||
await handleMcpPolicyUpdate(
|
||||
@@ -109,6 +124,7 @@ export async function updatePolicy(
|
||||
outcome,
|
||||
confirmationDetails,
|
||||
deps.messageBus,
|
||||
persistScope,
|
||||
);
|
||||
return;
|
||||
}
|
||||
@@ -119,6 +135,7 @@ export async function updatePolicy(
|
||||
outcome,
|
||||
confirmationDetails,
|
||||
deps.messageBus,
|
||||
persistScope,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -148,6 +165,7 @@ async function handleStandardPolicyUpdate(
|
||||
outcome: ToolConfirmationOutcome,
|
||||
confirmationDetails: SerializableConfirmationDetails | undefined,
|
||||
messageBus: MessageBus,
|
||||
persistScope?: 'workspace' | 'user',
|
||||
): Promise<void> {
|
||||
if (
|
||||
outcome === ToolConfirmationOutcome.ProceedAlways ||
|
||||
@@ -157,12 +175,17 @@ async function handleStandardPolicyUpdate(
|
||||
|
||||
if (confirmationDetails?.type === 'exec') {
|
||||
options.commandPrefix = confirmationDetails.rootCommands;
|
||||
} else if (confirmationDetails?.type === 'edit') {
|
||||
options.argsPattern = buildFilePathArgsPattern(
|
||||
confirmationDetails.filePath,
|
||||
);
|
||||
}
|
||||
|
||||
await messageBus.publish({
|
||||
type: MessageBusType.UPDATE_POLICY,
|
||||
toolName: tool.name,
|
||||
persist: outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave,
|
||||
persistScope,
|
||||
...options,
|
||||
});
|
||||
}
|
||||
@@ -180,6 +203,7 @@ async function handleMcpPolicyUpdate(
|
||||
{ type: 'mcp' }
|
||||
>,
|
||||
messageBus: MessageBus,
|
||||
persistScope?: 'workspace' | 'user',
|
||||
): Promise<void> {
|
||||
const isMcpAlways =
|
||||
outcome === ToolConfirmationOutcome.ProceedAlways ||
|
||||
@@ -204,5 +228,6 @@ async function handleMcpPolicyUpdate(
|
||||
toolName,
|
||||
mcpName: confirmationDetails.serverName,
|
||||
persist,
|
||||
persistScope,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -20,7 +20,9 @@ import {
|
||||
type ToolLocation,
|
||||
type ToolResult,
|
||||
type ToolResultDisplay,
|
||||
type PolicyUpdateOptions,
|
||||
} from './tools.js';
|
||||
import { buildFilePathArgsPattern } from '../policy/utils.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import { makeRelative, shortenPath } from '../utils/paths.js';
|
||||
@@ -442,6 +444,14 @@ class EditToolInvocation
|
||||
return [{ path: this.params.file_path }];
|
||||
}
|
||||
|
||||
protected override getPolicyUpdateOptions(
|
||||
_outcome: ToolConfirmationOutcome,
|
||||
): PolicyUpdateOptions | undefined {
|
||||
return {
|
||||
argsPattern: buildFilePathArgsPattern(this.params.file_path),
|
||||
};
|
||||
}
|
||||
|
||||
private async attemptSelfCorrection(
|
||||
params: EditToolParams,
|
||||
currentContent: string,
|
||||
|
||||
@@ -74,6 +74,7 @@ export interface ToolInvocation<
|
||||
* Options for policy updates that can be customized by tool invocations.
|
||||
*/
|
||||
export interface PolicyUpdateOptions {
|
||||
argsPattern?: string;
|
||||
commandPrefix?: string | string[];
|
||||
mcpName?: string;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,9 @@ import {
|
||||
type ToolLocation,
|
||||
type ToolResult,
|
||||
type ToolConfirmationOutcome,
|
||||
type PolicyUpdateOptions,
|
||||
} from './tools.js';
|
||||
import { buildFilePathArgsPattern } from '../policy/utils.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import { makeRelative, shortenPath } from '../utils/paths.js';
|
||||
import { getErrorMessage, isNodeError } from '../utils/errors.js';
|
||||
@@ -150,6 +152,14 @@ class WriteFileToolInvocation extends BaseToolInvocation<
|
||||
return [{ path: this.resolvedPath }];
|
||||
}
|
||||
|
||||
protected override getPolicyUpdateOptions(
|
||||
_outcome: ToolConfirmationOutcome,
|
||||
): PolicyUpdateOptions | undefined {
|
||||
return {
|
||||
argsPattern: buildFilePathArgsPattern(this.params.file_path),
|
||||
};
|
||||
}
|
||||
|
||||
override getDescription(): string {
|
||||
const relativePath = makeRelative(
|
||||
this.resolvedPath,
|
||||
|
||||
Reference in New Issue
Block a user