diff --git a/packages/cli/src/config/policy.test.ts b/packages/cli/src/config/policy.test.ts index 1a773d56a7..9baccd3359 100644 --- a/packages/cli/src/config/policy.test.ts +++ b/packages/cli/src/config/policy.test.ts @@ -8,7 +8,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; -import { resolveWorkspacePolicyState } from './policy.js'; +import { + resolveWorkspacePolicyState, + autoAcceptWorkspacePolicies, + setAutoAcceptWorkspacePolicies, + disableWorkspacePolicies, + setDisableWorkspacePolicies, +} from './policy.js'; import { writeToStderr } from '@google/gemini-cli-core'; // Mock debugLogger to avoid noise in test output @@ -41,6 +47,9 @@ describe('resolveWorkspacePolicyState', () => { fs.mkdirSync(workspaceDir); policiesDir = path.join(workspaceDir, '.gemini', 'policies'); + // Enable policies for these tests to verify loading logic + setDisableWorkspacePolicies(false); + vi.clearAllMocks(); }); @@ -63,29 +72,30 @@ describe('resolveWorkspacePolicyState', () => { }); }); + it('should have disableWorkspacePolicies set to true by default', () => { + // We explicitly set it to false in beforeEach for other tests, + // so here we test that setting it to true works. + setDisableWorkspacePolicies(true); + expect(disableWorkspacePolicies).toBe(true); + }); + it('should return policy directory if integrity matches', async () => { // Set up policies directory with a file fs.mkdirSync(policiesDir, { recursive: true }); fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); - // First call to establish integrity (interactive accept) + // First call to establish integrity (interactive auto-accept) const firstResult = await resolveWorkspacePolicyState({ cwd: workspaceDir, trustedFolder: true, interactive: true, }); - expect(firstResult.policyUpdateConfirmationRequest).toBeDefined(); - - // Establish integrity manually as if accepted - const { PolicyIntegrityManager } = await import('@google/gemini-cli-core'); - const integrityManager = new PolicyIntegrityManager(); - await integrityManager.acceptIntegrity( - 'workspace', - workspaceDir, - firstResult.policyUpdateConfirmationRequest!.newHash, - ); + expect(firstResult.workspacePoliciesDir).toBe(policiesDir); + expect(firstResult.policyUpdateConfirmationRequest).toBeUndefined(); + expect(writeToStderr).not.toHaveBeenCalled(); // Second call should match + const result = await resolveWorkspacePolicyState({ cwd: workspaceDir, trustedFolder: true, @@ -107,26 +117,33 @@ describe('resolveWorkspacePolicyState', () => { expect(result.policyUpdateConfirmationRequest).toBeUndefined(); }); - it('should return confirmation request if changed in interactive mode', async () => { - fs.mkdirSync(policiesDir, { recursive: true }); - fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + it('should return confirmation request if changed in interactive mode when AUTO_ACCEPT is false', async () => { + const originalValue = autoAcceptWorkspacePolicies; + setAutoAcceptWorkspacePolicies(false); - const result = await resolveWorkspacePolicyState({ - cwd: workspaceDir, - trustedFolder: true, - interactive: true, - }); + try { + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); - expect(result.workspacePoliciesDir).toBeUndefined(); - expect(result.policyUpdateConfirmationRequest).toEqual({ - scope: 'workspace', - identifier: workspaceDir, - policyDir: policiesDir, - newHash: expect.any(String), - }); + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + }); + + expect(result.workspacePoliciesDir).toBeUndefined(); + expect(result.policyUpdateConfirmationRequest).toEqual({ + scope: 'workspace', + identifier: workspaceDir, + policyDir: policiesDir, + newHash: expect.any(String), + }); + } finally { + setAutoAcceptWorkspacePolicies(originalValue); + } }); - it('should warn and auto-accept if changed in non-interactive mode', async () => { + it('should warn and auto-accept if changed in non-interactive mode when AUTO_ACCEPT is true', async () => { fs.mkdirSync(policiesDir, { recursive: true }); fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); @@ -143,6 +160,30 @@ describe('resolveWorkspacePolicyState', () => { ); }); + it('should warn and auto-accept if changed in non-interactive mode when AUTO_ACCEPT is false', async () => { + const originalValue = autoAcceptWorkspacePolicies; + setAutoAcceptWorkspacePolicies(false); + + try { + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: false, + }); + + expect(result.workspacePoliciesDir).toBe(policiesDir); + expect(result.policyUpdateConfirmationRequest).toBeUndefined(); + expect(writeToStderr).toHaveBeenCalledWith( + expect.stringContaining('Automatically accepting and loading'), + ); + } finally { + setAutoAcceptWorkspacePolicies(originalValue); + } + }); + it('should not return workspace policies if cwd is the home directory', async () => { const policiesDir = path.join(tempDir, '.gemini', 'policies'); fs.mkdirSync(policiesDir, { recursive: true }); @@ -159,7 +200,26 @@ describe('resolveWorkspacePolicyState', () => { expect(result.policyUpdateConfirmationRequest).toBeUndefined(); }); - it('should not return workspace policies if cwd is a symlink to the home directory', async () => { + it('should return empty state if disableWorkspacePolicies is true even if folder is trusted', async () => { + setDisableWorkspacePolicies(true); + + // Set up policies directory with a file + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + }); + + expect(result).toEqual({ + workspacePoliciesDir: undefined, + policyUpdateConfirmationRequest: undefined, + }); + }); + + it('should return empty state if cwd is a symlink to the home directory', async () => { const policiesDir = path.join(tempDir, '.gemini', 'policies'); fs.mkdirSync(policiesDir, { recursive: true }); fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); diff --git a/packages/cli/src/config/policy.ts b/packages/cli/src/config/policy.ts index 3b85d0b4b6..bc22c928f8 100644 --- a/packages/cli/src/config/policy.ts +++ b/packages/cli/src/config/policy.ts @@ -17,9 +17,38 @@ import { Storage, type PolicyUpdateConfirmationRequest, writeToStderr, + debugLogger, } from '@google/gemini-cli-core'; import { type Settings } from './settings.js'; +/** + * Temporary flag to automatically accept workspace policies to reduce friction. + * Exported as 'let' to allow monkey patching in tests via the setter. + */ +export let autoAcceptWorkspacePolicies = true; + +/** + * Sets the autoAcceptWorkspacePolicies flag. + * Used primarily for testing purposes. + */ +export function setAutoAcceptWorkspacePolicies(value: boolean) { + autoAcceptWorkspacePolicies = value; +} + +/** + * Temporary flag to disable workspace level policies altogether. + * Exported as 'let' to allow monkey patching in tests via the setter. + */ +export let disableWorkspacePolicies = true; + +/** + * Sets the disableWorkspacePolicies flag. + * Used primarily for testing purposes. + */ +export function setDisableWorkspacePolicies(value: boolean) { + disableWorkspacePolicies = value; +} + export async function createPolicyEngineConfig( settings: Settings, approvalMode: ApprovalMode, @@ -66,7 +95,7 @@ export async function resolveWorkspacePolicyState(options: { | PolicyUpdateConfirmationRequest | undefined; - if (trustedFolder) { + if (trustedFolder && !disableWorkspacePolicies) { const storage = new Storage(cwd); // If we are in the home directory (or rather, our target Gemini dir is the global one), @@ -91,8 +120,8 @@ export async function resolveWorkspacePolicyState(options: { ) { // No workspace policies found workspacePoliciesDir = undefined; - } else if (interactive) { - // Policies changed or are new, and we are in interactive mode + } else if (interactive && !autoAcceptWorkspacePolicies) { + // Policies changed or are new, and we are in interactive mode and auto-accept is disabled policyUpdateConfirmationRequest = { scope: 'workspace', identifier: cwd, @@ -100,17 +129,23 @@ export async function resolveWorkspacePolicyState(options: { newHash: integrityResult.hash, }; } else { - // Non-interactive mode: warn and automatically accept/load + // Non-interactive mode or auto-accept is enabled: automatically accept/load await integrityManager.acceptIntegrity( 'workspace', cwd, integrityResult.hash, ); workspacePoliciesDir = potentialWorkspacePoliciesDir; - // debugLogger.warn here doesn't show up in the terminal. It is showing up only in debug mode on the debug console - writeToStderr( - 'WARNING: Workspace policies changed or are new. Automatically accepting and loading them in non-interactive mode.\n', - ); + + if (!interactive) { + writeToStderr( + 'WARNING: Workspace policies changed or are new. Automatically accepting and loading them.\n', + ); + } else { + debugLogger.warn( + 'Workspace policies changed or are new. Automatically accepting and loading them.', + ); + } } } diff --git a/packages/cli/src/config/workspace-policy-cli.test.ts b/packages/cli/src/config/workspace-policy-cli.test.ts index 98cbe05bce..d0d98a5a31 100644 --- a/packages/cli/src/config/workspace-policy-cli.test.ts +++ b/packages/cli/src/config/workspace-policy-cli.test.ts @@ -10,6 +10,7 @@ import { loadCliConfig, type CliArgs } from './config.js'; import { createTestMergedSettings } from './settings.js'; import * as ServerConfig from '@google/gemini-cli-core'; import { isWorkspaceTrusted } from './trustedFolders.js'; +import * as Policy from './policy.js'; // Mock dependencies vi.mock('./trustedFolders.js', () => ({ @@ -53,6 +54,7 @@ describe('Workspace-Level Policy CLI Integration', () => { beforeEach(() => { vi.clearAllMocks(); + Policy.setDisableWorkspacePolicies(false); // Default to MATCH for existing tests mockCheckIntegrity.mockResolvedValue({ status: 'match', @@ -164,7 +166,7 @@ describe('Workspace-Level Policy CLI Integration', () => { ); }); - it('should set policyUpdateConfirmationRequest if integrity MISMATCH in interactive mode', async () => { + it('should automatically accept and load workspacePoliciesDir if integrity MISMATCH in interactive mode when AUTO_ACCEPT is true', async () => { vi.mocked(isWorkspaceTrusted).mockReturnValue({ isTrusted: true, source: 'file', @@ -186,24 +188,23 @@ describe('Workspace-Level Policy CLI Integration', () => { cwd: MOCK_CWD, }); - expect(config.getPolicyUpdateConfirmationRequest()).toEqual({ - scope: 'workspace', - identifier: MOCK_CWD, - policyDir: expect.stringContaining(path.join('.gemini', 'policies')), - newHash: 'new-hash', - }); - // In interactive mode without accept flag, it waits for user confirmation (handled by UI), - // so it currently DOES NOT pass the directory to createPolicyEngineConfig yet. - // The UI will handle the confirmation and reload/update. + expect(config.getPolicyUpdateConfirmationRequest()).toBeUndefined(); + expect(mockAcceptIntegrity).toHaveBeenCalledWith( + 'workspace', + MOCK_CWD, + 'new-hash', + ); expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith( expect.objectContaining({ - workspacePoliciesDir: undefined, + workspacePoliciesDir: expect.stringContaining( + path.join('.gemini', 'policies'), + ), }), expect.anything(), ); }); - it('should set policyUpdateConfirmationRequest if integrity is NEW with files (first time seen) in interactive mode', async () => { + it('should automatically accept and load workspacePoliciesDir if integrity is NEW in interactive mode when AUTO_ACCEPT is true', async () => { vi.mocked(isWorkspaceTrusted).mockReturnValue({ isTrusted: true, source: 'file', @@ -222,18 +223,65 @@ describe('Workspace-Level Policy CLI Integration', () => { cwd: MOCK_CWD, }); - expect(config.getPolicyUpdateConfirmationRequest()).toEqual({ - scope: 'workspace', - identifier: MOCK_CWD, - policyDir: expect.stringContaining(path.join('.gemini', 'policies')), - newHash: 'new-hash', - }); + expect(config.getPolicyUpdateConfirmationRequest()).toBeUndefined(); + expect(mockAcceptIntegrity).toHaveBeenCalledWith( + 'workspace', + MOCK_CWD, + 'new-hash', + ); expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith( expect.objectContaining({ - workspacePoliciesDir: undefined, + workspacePoliciesDir: expect.stringContaining( + path.join('.gemini', 'policies'), + ), }), expect.anything(), ); }); + + it('should set policyUpdateConfirmationRequest if integrity MISMATCH in interactive mode when AUTO_ACCEPT is false', async () => { + // Monkey patch autoAcceptWorkspacePolicies using setter + const originalValue = Policy.autoAcceptWorkspacePolicies; + Policy.setAutoAcceptWorkspacePolicies(false); + + try { + vi.mocked(isWorkspaceTrusted).mockReturnValue({ + isTrusted: true, + source: 'file', + }); + mockCheckIntegrity.mockResolvedValue({ + status: 'mismatch', + hash: 'new-hash', + fileCount: 1, + }); + vi.mocked(ServerConfig.isHeadlessMode).mockReturnValue(false); // Interactive + + const settings = createTestMergedSettings(); + const argv = { + query: 'test', + promptInteractive: 'test', + } as unknown as CliArgs; + + const config = await loadCliConfig(settings, 'test-session', argv, { + cwd: MOCK_CWD, + }); + + expect(config.getPolicyUpdateConfirmationRequest()).toEqual({ + scope: 'workspace', + identifier: MOCK_CWD, + policyDir: expect.stringContaining(path.join('.gemini', 'policies')), + newHash: 'new-hash', + }); + expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith( + expect.objectContaining({ + workspacePoliciesDir: undefined, + }), + expect.anything(), + ); + } finally { + // Restore for other tests + Policy.setAutoAcceptWorkspacePolicies(originalValue); + } + }); }); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index f66d60ef8b..a15e3264e8 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -169,10 +169,7 @@ export class Storage { } getAutoSavedPolicyPath(): string { - return path.join( - this.getWorkspacePoliciesDir(), - AUTO_SAVED_POLICY_FILENAME, - ); + return path.join(Storage.getUserPoliciesDir(), AUTO_SAVED_POLICY_FILENAME); } ensureProjectTempDirExists(): void { diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 7de415cb37..3e40771a85 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -447,9 +447,8 @@ export function createPolicyUpdater( if (message.persist) { persistenceQueue = persistenceQueue.then(async () => { try { - const workspacePoliciesDir = storage.getWorkspacePoliciesDir(); - await fs.mkdir(workspacePoliciesDir, { recursive: true }); const policyFile = storage.getAutoSavedPolicyPath(); + await fs.mkdir(path.dirname(policyFile), { recursive: true }); // 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 43f52a956d..c5a71fdd93 100644 --- a/packages/core/src/policy/persistence.test.ts +++ b/packages/core/src/policy/persistence.test.ts @@ -48,14 +48,8 @@ describe('createPolicyUpdater', () => { it('should persist policy when persist flag is true', async () => { createPolicyUpdater(policyEngine, messageBus, mockStorage); - const workspacePoliciesDir = '/mock/project/.gemini/policies'; - const policyFile = path.join( - workspacePoliciesDir, - AUTO_SAVED_POLICY_FILENAME, - ); - vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue( - workspacePoliciesDir, - ); + const userPoliciesDir = '/mock/user/.gemini/policies'; + const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME); vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile); (fs.mkdir as unknown as Mock).mockResolvedValue(undefined); (fs.readFile as unknown as Mock).mockRejectedValue( @@ -79,8 +73,7 @@ describe('createPolicyUpdater', () => { // Wait for async operations (microtasks) await new Promise((resolve) => setTimeout(resolve, 0)); - expect(mockStorage.getWorkspacePoliciesDir).toHaveBeenCalled(); - expect(fs.mkdir).toHaveBeenCalledWith(workspacePoliciesDir, { + expect(fs.mkdir).toHaveBeenCalledWith(userPoliciesDir, { recursive: true, }); @@ -115,14 +108,8 @@ describe('createPolicyUpdater', () => { it('should persist policy with commandPrefix when provided', async () => { createPolicyUpdater(policyEngine, messageBus, mockStorage); - const workspacePoliciesDir = '/mock/project/.gemini/policies'; - const policyFile = path.join( - workspacePoliciesDir, - AUTO_SAVED_POLICY_FILENAME, - ); - vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue( - workspacePoliciesDir, - ); + const userPoliciesDir = '/mock/user/.gemini/policies'; + const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME); vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile); (fs.mkdir as unknown as Mock).mockResolvedValue(undefined); (fs.readFile as unknown as Mock).mockRejectedValue( @@ -168,14 +155,8 @@ describe('createPolicyUpdater', () => { it('should persist policy with mcpName and toolName when provided', async () => { createPolicyUpdater(policyEngine, messageBus, mockStorage); - const workspacePoliciesDir = '/mock/project/.gemini/policies'; - const policyFile = path.join( - workspacePoliciesDir, - AUTO_SAVED_POLICY_FILENAME, - ); - vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue( - workspacePoliciesDir, - ); + const userPoliciesDir = '/mock/user/.gemini/policies'; + const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME); vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile); (fs.mkdir as unknown as Mock).mockResolvedValue(undefined); (fs.readFile as unknown as Mock).mockRejectedValue( @@ -214,14 +195,8 @@ describe('createPolicyUpdater', () => { it('should escape special characters in toolName and mcpName', async () => { createPolicyUpdater(policyEngine, messageBus, mockStorage); - const workspacePoliciesDir = '/mock/project/.gemini/policies'; - const policyFile = path.join( - workspacePoliciesDir, - AUTO_SAVED_POLICY_FILENAME, - ); - vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue( - workspacePoliciesDir, - ); + const userPoliciesDir = '/mock/user/.gemini/policies'; + const policyFile = path.join(userPoliciesDir, AUTO_SAVED_POLICY_FILENAME); vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile); (fs.mkdir as unknown as Mock).mockResolvedValue(undefined); (fs.readFile as unknown as Mock).mockRejectedValue( diff --git a/packages/core/src/policy/policy-updater.test.ts b/packages/core/src/policy/policy-updater.test.ts index 40780a1850..3037667949 100644 --- a/packages/core/src/policy/policy-updater.test.ts +++ b/packages/core/src/policy/policy-updater.test.ts @@ -50,8 +50,8 @@ describe('createPolicyUpdater', () => { messageBus = new MessageBus(policyEngine); mockStorage = new Storage('/mock/project'); - vi.spyOn(mockStorage, 'getWorkspacePoliciesDir').mockReturnValue( - '/mock/project/.gemini/policies', + vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue( + '/mock/user/.gemini/policies/auto-saved.toml', ); });