mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-13 23:51:16 -07:00
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'
This commit is contained in:
@@ -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,
|
||||
|
||||
158
packages/cli/src/config/policy.test.ts
Normal file
158
packages/cli/src/config/policy.test.ts
Normal file
@@ -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<typeof import('@google/gemini-cli-core')>();
|
||||
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'),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<WorkspacePolicyState> {
|
||||
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 };
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user