From d08bc96fab551f8ddd95e0e8e69e44cce9c75694 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Wed, 18 Feb 2026 15:15:36 -0800 Subject: [PATCH] refactor(cli): abstract workspace policy resolution logic Centralized the workspace policy discovery and integrity verification logic into a new 'resolveWorkspacePolicyState' helper in the policy module. This significantly simplifies 'loadCliConfig' in config.ts, reducing its imperative bloat and removing low-level core dependencies from the main configuration flow. - Moved workspace integrity check and directory discovery to policy.ts - Refactored loadCliConfig to use the new declarative resolver - Added comprehensive unit tests for the resolver using real temp dirs - Cleaned up redundant function arguments in core and CLI calls - Verified project integrity with 'npm run preflight' --- packages/cli/src/config/config.ts | 64 ++----- packages/cli/src/config/policy.test.ts | 158 ++++++++++++++++++ packages/cli/src/config/policy.ts | 75 +++++++++ .../src/config/workspace-policy-cli.test.ts | 7 - 4 files changed, 243 insertions(+), 61 deletions(-) create mode 100644 packages/cli/src/config/policy.test.ts diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 9c67fa87fa..8f4857c83f 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -40,13 +40,9 @@ import { Config, applyAdminAllowlist, getAdminBlockedMcpServersMessage, - Storage, type HookDefinition, type HookEventName, type OutputFormat, - PolicyIntegrityManager, - IntegrityStatus, - type PolicyUpdateConfirmationRequest, } from '@google/gemini-cli-core'; import { type Settings, @@ -60,7 +56,10 @@ import { resolvePath } from '../utils/resolvePath.js'; import { RESUME_LATEST } from '../utils/sessionUtils.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; -import { createPolicyEngineConfig } from './policy.js'; +import { + createPolicyEngineConfig, + resolveWorkspacePolicyState, +} from './policy.js'; import { ExtensionManager } from './extension-manager.js'; import { McpServerEnablementManager } from './mcp/mcpServerEnablement.js'; import type { ExtensionEvents } from '@google/gemini-cli-core/src/utils/extensionLoader.js'; @@ -702,56 +701,13 @@ export async function loadCliConfig( policyPaths: argv.policy, }; - let workspacePoliciesDir: string | undefined; - let policyUpdateConfirmationRequest: - | PolicyUpdateConfirmationRequest - | undefined; - - if (trustedFolder) { - const potentialWorkspacePoliciesDir = new Storage( + const { workspacePoliciesDir, policyUpdateConfirmationRequest } = + await resolveWorkspacePolicyState({ cwd, - ).getWorkspacePoliciesDir(); - const integrityManager = new PolicyIntegrityManager(); - const integrityResult = await integrityManager.checkIntegrity( - 'workspace', - cwd, - potentialWorkspacePoliciesDir, - ); - - if (integrityResult.status === IntegrityStatus.MATCH) { - workspacePoliciesDir = potentialWorkspacePoliciesDir; - } else if ( - integrityResult.status === IntegrityStatus.NEW && - integrityResult.fileCount === 0 - ) { - // No workspace policies found - workspacePoliciesDir = undefined; - } else { - // Policies changed or are new - if (argv.acceptChangedPolicies) { - debugLogger.warn( - 'WARNING: Workspace policies changed or are new. Auto-accepting due to --accept-changed-policies flag.', - ); - await integrityManager.acceptIntegrity( - 'workspace', - cwd, - integrityResult.hash, - ); - workspacePoliciesDir = potentialWorkspacePoliciesDir; - } else if (interactive) { - policyUpdateConfirmationRequest = { - scope: 'workspace', - identifier: cwd, - policyDir: potentialWorkspacePoliciesDir, - newHash: integrityResult.hash, - }; - } else { - debugLogger.warn( - 'WARNING: Workspace policies changed or are new. Loading default policies only. Use --accept-changed-policies to accept.', - ); - } - } - } + trustedFolder, + interactive, + acceptChangedPolicies: argv.acceptChangedPolicies ?? false, + }); const policyEngineConfig = await createPolicyEngineConfig( effectiveSettings, diff --git a/packages/cli/src/config/policy.test.ts b/packages/cli/src/config/policy.test.ts new file mode 100644 index 0000000000..6b7e1021bc --- /dev/null +++ b/packages/cli/src/config/policy.test.ts @@ -0,0 +1,158 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +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 { debugLogger } from '@google/gemini-cli-core'; + +// Mock debugLogger to avoid noise in test output +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + debugLogger: { + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }; +}); + +describe('resolveWorkspacePolicyState', () => { + let tempDir: string; + let workspaceDir: string; + let policiesDir: string; + + beforeEach(() => { + // Create a temporary directory for the test + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-cli-test-')); + // Redirect GEMINI_CLI_HOME to the temp directory to isolate integrity storage + vi.stubEnv('GEMINI_CLI_HOME', tempDir); + + workspaceDir = path.join(tempDir, 'workspace'); + fs.mkdirSync(workspaceDir); + policiesDir = path.join(workspaceDir, '.gemini', 'policies'); + + vi.clearAllMocks(); + }); + + afterEach(() => { + // Clean up temporary directory + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.unstubAllEnvs(); + }); + + it('should return empty state if folder is not trusted', async () => { + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: false, + interactive: true, + acceptChangedPolicies: false, + }); + + expect(result).toEqual({ + workspacePoliciesDir: undefined, + policyUpdateConfirmationRequest: undefined, + }); + }); + + 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 (auto-accept) + await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + acceptChangedPolicies: true, + }); + + // Second call should match + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + acceptChangedPolicies: false, + }); + + expect(result.workspacePoliciesDir).toBe(policiesDir); + expect(result.policyUpdateConfirmationRequest).toBeUndefined(); + }); + + it('should return undefined if integrity is NEW but fileCount is 0', async () => { + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + acceptChangedPolicies: false, + }); + + expect(result.workspacePoliciesDir).toBeUndefined(); + expect(result.policyUpdateConfirmationRequest).toBeUndefined(); + }); + + it('should auto-accept changed policies if acceptChangedPolicies is true', async () => { + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + acceptChangedPolicies: true, + }); + + expect(result.workspacePoliciesDir).toBe(policiesDir); + expect(result.policyUpdateConfirmationRequest).toBeUndefined(); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Auto-accepting'), + ); + }); + + it('should return confirmation request if changed in interactive mode', async () => { + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: true, + acceptChangedPolicies: false, + }); + + expect(result.workspacePoliciesDir).toBeUndefined(); + expect(result.policyUpdateConfirmationRequest).toEqual({ + scope: 'workspace', + identifier: workspaceDir, + policyDir: policiesDir, + newHash: expect.any(String), + }); + }); + + it('should warn and return undefined if changed in non-interactive mode', async () => { + fs.mkdirSync(policiesDir, { recursive: true }); + fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []'); + + const result = await resolveWorkspacePolicyState({ + cwd: workspaceDir, + trustedFolder: true, + interactive: false, + acceptChangedPolicies: false, + }); + + expect(result.workspacePoliciesDir).toBeUndefined(); + expect(result.policyUpdateConfirmationRequest).toBeUndefined(); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Loading default policies only'), + ); + }); +}); diff --git a/packages/cli/src/config/policy.ts b/packages/cli/src/config/policy.ts index 2caa8d71e0..49d906acae 100644 --- a/packages/cli/src/config/policy.ts +++ b/packages/cli/src/config/policy.ts @@ -12,6 +12,11 @@ import { type PolicySettings, createPolicyEngineConfig as createCorePolicyEngineConfig, createPolicyUpdater as createCorePolicyUpdater, + PolicyIntegrityManager, + IntegrityStatus, + Storage, + type PolicyUpdateConfirmationRequest, + debugLogger, } from '@google/gemini-cli-core'; import { type Settings } from './settings.js'; @@ -39,3 +44,73 @@ export function createPolicyUpdater( ) { return createCorePolicyUpdater(policyEngine, messageBus); } + +export interface WorkspacePolicyState { + workspacePoliciesDir?: string; + policyUpdateConfirmationRequest?: PolicyUpdateConfirmationRequest; +} + +/** + * Resolves the workspace policy state by checking folder trust and policy integrity. + */ +export async function resolveWorkspacePolicyState(options: { + cwd: string; + trustedFolder: boolean; + interactive: boolean; + acceptChangedPolicies: boolean; +}): Promise { + const { cwd, trustedFolder, interactive, acceptChangedPolicies } = options; + + let workspacePoliciesDir: string | undefined; + let policyUpdateConfirmationRequest: + | PolicyUpdateConfirmationRequest + | undefined; + + if (trustedFolder) { + const potentialWorkspacePoliciesDir = new Storage( + cwd, + ).getWorkspacePoliciesDir(); + const integrityManager = new PolicyIntegrityManager(); + const integrityResult = await integrityManager.checkIntegrity( + 'workspace', + cwd, + potentialWorkspacePoliciesDir, + ); + + if (integrityResult.status === IntegrityStatus.MATCH) { + workspacePoliciesDir = potentialWorkspacePoliciesDir; + } else if ( + integrityResult.status === IntegrityStatus.NEW && + integrityResult.fileCount === 0 + ) { + // No workspace policies found + workspacePoliciesDir = undefined; + } else { + // Policies changed or are new + if (acceptChangedPolicies) { + debugLogger.warn( + 'WARNING: Workspace policies changed or are new. Auto-accepting due to --accept-changed-policies flag.', + ); + await integrityManager.acceptIntegrity( + 'workspace', + cwd, + integrityResult.hash, + ); + workspacePoliciesDir = potentialWorkspacePoliciesDir; + } else if (interactive) { + policyUpdateConfirmationRequest = { + scope: 'workspace', + identifier: cwd, + policyDir: potentialWorkspacePoliciesDir, + newHash: integrityResult.hash, + }; + } else { + debugLogger.warn( + 'WARNING: Workspace policies changed or are new. Loading default policies only. Use --accept-changed-policies to accept.', + ); + } + } + } + + return { workspacePoliciesDir, policyUpdateConfirmationRequest }; +} diff --git a/packages/cli/src/config/workspace-policy-cli.test.ts b/packages/cli/src/config/workspace-policy-cli.test.ts index 630c7245b8..d39d48c19c 100644 --- a/packages/cli/src/config/workspace-policy-cli.test.ts +++ b/packages/cli/src/config/workspace-policy-cli.test.ts @@ -87,7 +87,6 @@ describe('Workspace-Level Policy CLI Integration', () => { ), }), expect.anything(), - undefined, ); }); @@ -107,7 +106,6 @@ describe('Workspace-Level Policy CLI Integration', () => { workspacePoliciesDir: undefined, }), expect.anything(), - undefined, ); }); @@ -132,7 +130,6 @@ describe('Workspace-Level Policy CLI Integration', () => { workspacePoliciesDir: undefined, }), expect.anything(), - undefined, ); }); @@ -161,7 +158,6 @@ describe('Workspace-Level Policy CLI Integration', () => { workspacePoliciesDir: undefined, }), expect.anything(), - undefined, // Should NOT load policies ); }); @@ -193,7 +189,6 @@ describe('Workspace-Level Policy CLI Integration', () => { ), }), expect.anything(), - undefined, ); }); @@ -233,7 +228,6 @@ describe('Workspace-Level Policy CLI Integration', () => { workspacePoliciesDir: undefined, }), expect.anything(), - undefined, ); }); @@ -268,7 +262,6 @@ describe('Workspace-Level Policy CLI Integration', () => { workspacePoliciesDir: undefined, }), expect.anything(), - undefined, ); }); });