diff --git a/packages/cli/src/config/policy.ts b/packages/cli/src/config/policy.ts index e689094f947..ef6164efb72 100644 --- a/packages/cli/src/config/policy.ts +++ b/packages/cli/src/config/policy.ts @@ -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 { diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 8dbff891286..412ba01b310 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -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() diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index bd04123c346..3099f39d1e5 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -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 }); } diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 50fbc0ef2a4..7de415cb37e 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -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[] } = {}; diff --git a/packages/core/src/policy/persistence.test.ts b/packages/core/src/policy/persistence.test.ts index 3acf7c714d1..43f52a956dc 100644 --- a/packages/core/src/policy/persistence.test.ts +++ b/packages/core/src/policy/persistence.test.ts @@ -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'), diff --git a/packages/core/src/policy/policy-updater.test.ts b/packages/core/src/policy/policy-updater.test.ts index 928d84408b2..40780a18506 100644 --- a/packages/core/src/policy/policy-updater.test.ts +++ b/packages/core/src/policy/policy-updater.test.ts @@ -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,