MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-03-10 14:10:37 -07:00
Code Issues Packages Projects Releases Wiki Activity

feat(policy): repurpose "Always Allow" persistence to workspace level (#19707)

Browse Source
This commit is contained in:
Abhijit Balaji
2026-02-20 14:07:20 -08:00
committed by GitHub
parent b48970da15
commit c5baf39dbd
6 changed files with 113 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
packages/cli/src/config/policy.ts
View File

@@ -41,8 +41,9 @@ export async function createPolicyEngineConfig(
export function createPolicyUpdater(
policyEngine: PolicyEngine,
messageBus: MessageBus,
storage: Storage,
) {
return createCorePolicyUpdater(policyEngine, messageBus);
return createCorePolicyUpdater(policyEngine, messageBus, storage);
}
export interface WorkspacePolicyState {

2
packages/cli/src/gemini.tsx
View File

@@ -583,7 +583,7 @@ export async function main() {
const policyEngine = config.getPolicyEngine();
const messageBus = config.getMessageBus();
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, config.storage);
// Register SessionEnd hook to fire on graceful exit
// This runs before telemetry shutdown in runExitCleanup()

9
packages/core/src/config/storage.ts
View File

@@ -23,6 +23,8 @@ const TMP_DIR_NAME = 'tmp';
const BIN_DIR_NAME = 'bin';
const AGENTS_DIR_NAME = '.agents';
export const AUTO_SAVED_POLICY_FILENAME = 'auto-saved.toml';
export class Storage {
private readonly targetDir: string;
private readonly sessionId: string | undefined;
@@ -154,6 +156,13 @@ export class Storage {
return path.join(this.getGeminiDir(), 'policies');
}
getAutoSavedPolicyPath(): string {
return path.join(
this.getWorkspacePoliciesDir(),
AUTO_SAVED_POLICY_FILENAME,
);
}
ensureProjectTempDirExists(): void {
fs.mkdirSync(this.getProjectTempDir(), { recursive: true });
}

70
packages/core/src/policy/config.ts
View File

@@ -43,6 +43,20 @@ export const WORKSPACE_POLICY_TIER = 2;
export const USER_POLICY_TIER = 3;
export const ADMIN_POLICY_TIER = 4;
// Specific priority offsets and derived priorities for dynamic/settings rules.
// These are added to the tier base (e.g., USER_POLICY_TIER).
// Workspace tier (2) + high priority (950/1000) = ALWAYS_ALLOW_PRIORITY
// This ensures user "always allow" selections are high priority
// within the workspace tier but still lose to user/admin policies.
export const ALWAYS_ALLOW_PRIORITY = WORKSPACE_POLICY_TIER + 0.95;
export const MCP_EXCLUDED_PRIORITY = USER_POLICY_TIER + 0.9;
export const EXCLUDE_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.4;
export const ALLOWED_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.3;
export const TRUSTED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.2;
export const ALLOWED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.1;
/**
* Gets the list of directories to search for policy files, in order of increasing priority
* (Default -> User -> Project -> Admin).
@@ -233,13 +247,14 @@ export async function createPolicyEngineConfig(
// This ensures Admin > User > Workspace > Default hierarchy is always preserved,
// while allowing user-specified priorities to work within each tier.
//
// Settings-based and dynamic rules (all in user tier 3.x):
// 3.95: Tools that the user has selected as "Always Allow" in the interactive UI
// 3.9: MCP servers excluded list (security: persistent server blocks)
// 3.4: Command line flag --exclude-tools (explicit temporary blocks)
// 3.3: Command line flag --allowed-tools (explicit temporary allows)
// 3.2: MCP servers with trust=true (persistent trusted servers)
// 3.1: MCP servers allowed list (persistent general server allows)
// Settings-based and dynamic rules (mixed tiers):
// MCP_EXCLUDED_PRIORITY: MCP servers excluded list (security: persistent server blocks)
// EXCLUDE_TOOLS_FLAG_PRIORITY: Command line flag --exclude-tools (explicit temporary blocks)
// ALLOWED_TOOLS_FLAG_PRIORITY: Command line flag --allowed-tools (explicit temporary allows)
// TRUSTED_MCP_SERVER_PRIORITY: MCP servers with trust=true (persistent trusted servers)
// ALLOWED_MCP_SERVER_PRIORITY: MCP servers allowed list (persistent general server allows)
// ALWAYS_ALLOW_PRIORITY: Tools that the user has selected as "Always Allow" in the interactive UI
// (Workspace tier 2.x - scoped to the project)
//
// TOML policy priorities (before transformation):
// 10: Write tools default to ASK_USER (becomes 1.010 in default tier)
@@ -250,33 +265,33 @@ export async function createPolicyEngineConfig(
// 999: YOLO mode allow-all (becomes 1.999 in default tier)
// MCP servers that are explicitly excluded in settings.mcp.excluded
// Priority: 3.9 (highest in user tier for security - persistent server blocks)
// Priority: MCP_EXCLUDED_PRIORITY (highest in user tier for security - persistent server blocks)
if (settings.mcp?.excluded) {
for (const serverName of settings.mcp.excluded) {
rules.push({
toolName: `${serverName}__*`,
decision: PolicyDecision.DENY,
priority: 3.9,
priority: MCP_EXCLUDED_PRIORITY,
source: 'Settings (MCP Excluded)',
});
}
}
// Tools that are explicitly excluded in the settings.
// Priority: 3.4 (user tier - explicit temporary blocks)
// Priority: EXCLUDE_TOOLS_FLAG_PRIORITY (user tier - explicit temporary blocks)
if (settings.tools?.exclude) {
for (const tool of settings.tools.exclude) {
rules.push({
toolName: tool,
decision: PolicyDecision.DENY,
priority: 3.4,
priority: EXCLUDE_TOOLS_FLAG_PRIORITY,
source: 'Settings (Tools Excluded)',
});
}
}
// Tools that are explicitly allowed in the settings.
// Priority: 3.3 (user tier - explicit temporary allows)
// Priority: ALLOWED_TOOLS_FLAG_PRIORITY (user tier - explicit temporary allows)
if (settings.tools?.allowed) {
for (const tool of settings.tools.allowed) {
// Check for legacy format: toolName(args)
@@ -296,7 +311,7 @@ export async function createPolicyEngineConfig(
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: 3.3,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
argsPattern: new RegExp(pattern),
source: 'Settings (Tools Allowed)',
});
@@ -308,7 +323,7 @@ export async function createPolicyEngineConfig(
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: 3.3,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
source: 'Settings (Tools Allowed)',
});
}
@@ -320,7 +335,7 @@ export async function createPolicyEngineConfig(
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: 3.3,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
source: 'Settings (Tools Allowed)',
});
}
@@ -328,7 +343,7 @@ export async function createPolicyEngineConfig(
}
// MCP servers that are trusted in the settings.
// Priority: 3.2 (user tier - persistent trusted servers)
// Priority: TRUSTED_MCP_SERVER_PRIORITY (user tier - persistent trusted servers)
if (settings.mcpServers) {
for (const [serverName, serverConfig] of Object.entries(
settings.mcpServers,
@@ -339,7 +354,7 @@ export async function createPolicyEngineConfig(
rules.push({
toolName: `${serverName}__*`,
decision: PolicyDecision.ALLOW,
priority: 3.2,
priority: TRUSTED_MCP_SERVER_PRIORITY,
source: 'Settings (MCP Trusted)',
});
}
@@ -347,13 +362,13 @@ export async function createPolicyEngineConfig(
}
// MCP servers that are explicitly allowed in settings.mcp.allowed
// Priority: 3.1 (user tier - persistent general server allows)
// Priority: ALLOWED_MCP_SERVER_PRIORITY (user tier - persistent general server allows)
if (settings.mcp?.allowed) {
for (const serverName of settings.mcp.allowed) {
rules.push({
toolName: `${serverName}__*`,
decision: PolicyDecision.ALLOW,
priority: 3.1,
priority: ALLOWED_MCP_SERVER_PRIORITY,
source: 'Settings (MCP Allowed)',
});
}
@@ -381,6 +396,7 @@ interface TomlRule {
export function createPolicyUpdater(
policyEngine: PolicyEngine,
messageBus: MessageBus,
storage: Storage,
) {
// Use a sequential queue for persistence to avoid lost updates from concurrent events.
let persistenceQueue = Promise.resolve();
@@ -400,10 +416,7 @@ export function createPolicyUpdater(
policyEngine.addRule({
toolName,
decision: PolicyDecision.ALLOW,
// User tier (3) + high priority (950/1000) = 3.95
// This ensures user "always allow" selections are high priority
// but still lose to admin policies (4.xxx) and settings excludes (300)
priority: 3.95,
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp(pattern),
source: 'Dynamic (Confirmed)',
});
@@ -425,10 +438,7 @@ export function createPolicyUpdater(
policyEngine.addRule({
toolName,
decision: PolicyDecision.ALLOW,
// User tier (3) + high priority (950/1000) = 3.95
// This ensures user "always allow" selections are high priority
// but still lose to admin policies (4.xxx) and settings excludes (300)
priority: 3.95,
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern,
source: 'Dynamic (Confirmed)',
});
@@ -437,9 +447,9 @@ export function createPolicyUpdater(
if (message.persist) {
persistenceQueue = persistenceQueue.then(async () => {
try {
const userPoliciesDir = Storage.getUserPoliciesDir();
await fs.mkdir(userPoliciesDir, { recursive: true });
const policyFile = path.join(userPoliciesDir, 'auto-saved.toml');
const workspacePoliciesDir = storage.getWorkspacePoliciesDir();
await fs.mkdir(workspacePoliciesDir, { recursive: true });
const policyFile = storage.getAutoSavedPolicyPath();
// Read existing file
let existingData: { rule?: TomlRule[] } = {};

68
packages/core/src/policy/persistence.test.ts
View File

@@ -15,11 +15,11 @@ import {
} from 'vitest';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { createPolicyUpdater } from './config.js';
import { createPolicyUpdater, ALWAYS_ALLOW_PRIORITY } from './config.js';
import { PolicyEngine } from './policy-engine.js';
import { MessageBus } from '../confirmation-bus/message-bus.js';
import { MessageBusType } from '../confirmation-bus/types.js';
import { Storage } from '../config/storage.js';
import { Storage, AUTO_SAVED_POLICY_FILENAME } from '../config/storage.js';
import { ApprovalMode } from './types.js';
vi.mock('node:fs/promises');
@@ -28,6 +28,7 @@ vi.mock('../config/storage.js');
describe('createPolicyUpdater', () => {
let policyEngine: PolicyEngine;
let messageBus: MessageBus;
let mockStorage: Storage;
beforeEach(() => {
policyEngine = new PolicyEngine({
@@ -36,6 +37,7 @@ describe('createPolicyUpdater', () => {
approvalMode: ApprovalMode.DEFAULT,
});
messageBus = new MessageBus(policyEngine);
mockStorage = new Storage('/mock/project');
vi.clearAllMocks();
});
@@ -44,10 +46,17 @@ describe('createPolicyUpdater', () => {
});
it('should persist policy when persist flag is true', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/policies';
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(userPoliciesDir);
const workspacePoliciesDir = '/mock/project/.gemini/policies';
const policyFile = path.join(
workspacePoliciesDir,
AUTO_SAVED_POLICY_FILENAME,
);
vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue(
workspacePoliciesDir,
);
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
@@ -70,8 +79,8 @@ describe('createPolicyUpdater', () => {
// Wait for async operations (microtasks)
await new Promise((resolve) => setTimeout(resolve, 0));
expect(Storage.getUserPoliciesDir).toHaveBeenCalled();
expect(fs.mkdir).toHaveBeenCalledWith(userPoliciesDir, {
expect(mockStorage.getWorkspacePoliciesDir).toHaveBeenCalled();
expect(fs.mkdir).toHaveBeenCalledWith(workspacePoliciesDir, {
recursive: true,
});
@@ -85,12 +94,12 @@ describe('createPolicyUpdater', () => {
);
expect(fs.rename).toHaveBeenCalledWith(
expect.stringMatching(/\.tmp$/),
path.join(userPoliciesDir, 'auto-saved.toml'),
policyFile,
);
});
it('should not persist policy when persist flag is false or undefined', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
@@ -104,10 +113,17 @@ describe('createPolicyUpdater', () => {
});
it('should persist policy with commandPrefix when provided', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/policies';
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(userPoliciesDir);
const workspacePoliciesDir = '/mock/project/.gemini/policies';
const policyFile = path.join(
workspacePoliciesDir,
AUTO_SAVED_POLICY_FILENAME,
);
vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue(
workspacePoliciesDir,
);
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
@@ -136,7 +152,7 @@ describe('createPolicyUpdater', () => {
const rules = policyEngine.getRules();
const addedRule = rules.find((r) => r.toolName === toolName);
expect(addedRule).toBeDefined();
expect(addedRule?.priority).toBe(3.95);
expect(addedRule?.priority).toBe(ALWAYS_ALLOW_PRIORITY);
expect(addedRule?.argsPattern).toEqual(
new RegExp(`"command":"git\\ status(?:[\\s"]|\\\\")`),
);
@@ -150,10 +166,17 @@ describe('createPolicyUpdater', () => {
});
it('should persist policy with mcpName and toolName when provided', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/policies';
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(userPoliciesDir);
const workspacePoliciesDir = '/mock/project/.gemini/policies';
const policyFile = path.join(
workspacePoliciesDir,
AUTO_SAVED_POLICY_FILENAME,
);
vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue(
workspacePoliciesDir,
);
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),
@@ -189,10 +212,17 @@ describe('createPolicyUpdater', () => {
});
it('should escape special characters in toolName and mcpName', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
const userPoliciesDir = '/mock/user/policies';
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(userPoliciesDir);
const workspacePoliciesDir = '/mock/project/.gemini/policies';
const policyFile = path.join(
workspacePoliciesDir,
AUTO_SAVED_POLICY_FILENAME,
);
vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue(
workspacePoliciesDir,
);
vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile);
(fs.mkdir as unknown as Mock).mockResolvedValue(undefined);
(fs.readFile as unknown as Mock).mockRejectedValue(
new Error('File not found'),

19
packages/core/src/policy/policy-updater.test.ts
View File

@@ -6,7 +6,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs/promises';
import { createPolicyUpdater } from './config.js';
import { createPolicyUpdater, ALWAYS_ALLOW_PRIORITY } from './config.js';
import { PolicyEngine } from './policy-engine.js';
import { MessageBus } from '../confirmation-bus/message-bus.js';
import { MessageBusType } from '../confirmation-bus/types.js';
@@ -41,6 +41,7 @@ interface TestableShellToolInvocation {
describe('createPolicyUpdater', () => {
let policyEngine: PolicyEngine;
let messageBus: MessageBus;
let mockStorage: Storage;
beforeEach(() => {
vi.resetAllMocks();
@@ -48,8 +49,9 @@ describe('createPolicyUpdater', () => {
vi.spyOn(policyEngine, 'addRule');
messageBus = new MessageBus(policyEngine);
vi.spyOn(Storage, 'getUserPoliciesDir').mockReturnValue(
'/mock/user/policies',
mockStorage = new Storage('/mock/project');
vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue(
'/mock/project/.gemini/policies',
);
});
@@ -58,7 +60,7 @@ describe('createPolicyUpdater', () => {
});
it('should add multiple rules when commandPrefix is an array', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
@@ -72,6 +74,7 @@ describe('createPolicyUpdater', () => {
1,
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"echo(?:[\\s"]|\\\\")'),
}),
);
@@ -79,13 +82,14 @@ describe('createPolicyUpdater', () => {
2,
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"ls(?:[\\s"]|\\\\")'),
}),
);
});
it('should add a single rule when commandPrefix is a string', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,
@@ -98,13 +102,14 @@ describe('createPolicyUpdater', () => {
expect(policyEngine.addRule).toHaveBeenCalledWith(
expect.objectContaining({
toolName: 'run_shell_command',
priority: ALWAYS_ALLOW_PRIORITY,
argsPattern: new RegExp('"command":"git(?:[\\s"]|\\\\")'),
}),
);
});
it('should persist multiple rules correctly to TOML', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
vi.mocked(fs.readFile).mockRejectedValue({ code: 'ENOENT' });
vi.mocked(fs.mkdir).mockResolvedValue(undefined);
@@ -139,7 +144,7 @@ describe('createPolicyUpdater', () => {
});
it('should reject unsafe regex patterns', async () => {
createPolicyUpdater(policyEngine, messageBus);
createPolicyUpdater(policyEngine, messageBus, mockStorage);
await messageBus.publish({
type: MessageBusType.UPDATE_POLICY,