fix(a2a-server): Implement default policy loading for parity with CLI (#27073)

This commit is contained in:
Keith Schaab
2026-05-19 14:09:57 +00:00
committed by GitHub
parent 7478859502
commit 85566a73f6
8 changed files with 294 additions and 41 deletions
+8 -2
View File
@@ -93,10 +93,16 @@ export class CoderAgentExecutor implements AgentExecutor {
taskId: string, taskId: string,
): Promise<Config> { ): Promise<Config> {
const workspaceRoot = setTargetDir(agentSettings); const workspaceRoot = setTargetDir(agentSettings);
const isTrusted = agentSettings.isTrusted ?? false;
loadEnvironment(); // Will override any global env with workspace envs loadEnvironment(); // Will override any global env with workspace envs
const settings = loadSettings(workspaceRoot); const settings = loadSettings(workspaceRoot, isTrusted);
const extensions = loadExtensions(workspaceRoot); const extensions = loadExtensions(workspaceRoot);
return loadConfig(settings, new SimpleExtensionLoader(extensions), taskId); return loadConfig(
settings,
new SimpleExtensionLoader(extensions),
taskId,
isTrusted,
);
} }
/** /**
+114 -2
View File
@@ -19,7 +19,9 @@ import {
isHeadlessMode, isHeadlessMode,
FatalAuthenticationError, FatalAuthenticationError,
PolicyDecision, PolicyDecision,
ApprovalMode,
PRIORITY_YOLO_ALLOW_ALL, PRIORITY_YOLO_ALLOW_ALL,
createPolicyEngineConfig,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
// Mock dependencies // Mock dependencies
@@ -53,6 +55,32 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
isHeadlessMode: vi.fn().mockReturnValue(false), isHeadlessMode: vi.fn().mockReturnValue(false),
getCodeAssistServer: vi.fn(), getCodeAssistServer: vi.fn(),
fetchAdminControlsOnce: vi.fn(), fetchAdminControlsOnce: vi.fn(),
createPolicyEngineConfig: vi
.fn()
.mockImplementation(
(_settings, mode, _defaultPoliciesDir, _interactive) => ({
rules:
mode === actual.ApprovalMode.YOLO
? [
{
toolName: '*',
decision: actual.PolicyDecision.ALLOW,
priority: actual.PRIORITY_YOLO_ALLOW_ALL,
modes: [actual.ApprovalMode.YOLO],
allowRedirection: true,
},
]
: [
{
toolName: 'read_file',
decision: actual.PolicyDecision.ALLOW,
priority: 1.05,
source: 'Default: read-only.toml',
},
],
checkers: [],
}),
),
coreEvents: { coreEvents: {
emitAdminSettingsChanged: vi.fn(), emitAdminSettingsChanged: vi.fn(),
}, },
@@ -261,6 +289,85 @@ describe('loadConfig', () => {
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]); expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]);
}); });
describe('policy engine configuration', () => {
it('should merge V1 and V2 tool settings into policySettings', async () => {
const settings: Settings = {
allowedTools: ['v1-allowed'],
tools: {
allowed: ['v2-allowed'],
exclude: ['v2-exclude'],
core: ['v2-core'],
},
mcpServers: {
test: { command: 'test', args: [] },
},
policyPaths: ['/path/to/policy'],
adminPolicyPaths: ['/path/to/admin/policy'],
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
tools: {
core: ['v2-core'],
exclude: ['v2-exclude'],
allowed: ['v1-allowed'],
},
mcpServers: settings.mcpServers,
policyPaths: settings.policyPaths,
adminPolicyPaths: settings.adminPolicyPaths,
}),
ApprovalMode.DEFAULT,
undefined,
true,
);
});
it('should use V2 tool settings when V1 is missing', async () => {
const settings: Settings = {
tools: {
allowed: ['v2-allowed'],
},
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.objectContaining({
allowed: ['v2-allowed'],
}),
}),
ApprovalMode.DEFAULT,
undefined,
true,
);
});
it('should use V1 tool settings when V2 is also present', async () => {
const settings: Settings = {
allowedTools: ['v1-allowed'],
tools: {
allowed: ['v2-allowed'],
},
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.objectContaining({
allowed: ['v1-allowed'],
}),
}),
ApprovalMode.DEFAULT,
undefined,
true,
);
});
});
describe('tool configuration', () => { describe('tool configuration', () => {
it('should pass V1 allowedTools to Config properly', async () => { it('should pass V1 allowedTools to Config properly', async () => {
const settings: Settings = { const settings: Settings = {
@@ -385,14 +492,19 @@ describe('loadConfig', () => {
); );
}); });
it('should use default approval mode and empty rules when GEMINI_YOLO_MODE is not true', async () => { it('should use default approval mode and load default rules when GEMINI_YOLO_MODE is not true', async () => {
vi.stubEnv('GEMINI_YOLO_MODE', 'false'); vi.stubEnv('GEMINI_YOLO_MODE', 'false');
await loadConfig(mockSettings, mockExtensionLoader, taskId); await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith( expect(Config).toHaveBeenCalledWith(
expect.objectContaining({ expect.objectContaining({
approvalMode: 'default', approvalMode: 'default',
policyEngineConfig: expect.objectContaining({ policyEngineConfig: expect.objectContaining({
rules: [], rules: expect.arrayContaining([
expect.objectContaining({
toolName: 'read_file',
decision: PolicyDecision.ALLOW,
}),
]),
}), }),
}), }),
); );
+23 -17
View File
@@ -23,8 +23,8 @@ import {
ExperimentFlags, ExperimentFlags,
isHeadlessMode, isHeadlessMode,
FatalAuthenticationError, FatalAuthenticationError,
PolicyDecision, createPolicyEngineConfig,
PRIORITY_YOLO_ALLOW_ALL, type PolicySettings,
type TelemetryTarget, type TelemetryTarget,
type ConfigParameters, type ConfigParameters,
type ExtensionLoader, type ExtensionLoader,
@@ -38,6 +38,7 @@ export async function loadConfig(
settings: Settings, settings: Settings,
extensionLoader: ExtensionLoader, extensionLoader: ExtensionLoader,
taskId: string, taskId: string,
trusted: boolean = false,
): Promise<Config> { ): Promise<Config> {
const workspaceDir = process.cwd(); const workspaceDir = process.cwd();
@@ -63,6 +64,24 @@ export async function loadConfig(
? ApprovalMode.YOLO ? ApprovalMode.YOLO
: ApprovalMode.DEFAULT; : ApprovalMode.DEFAULT;
const policySettings: PolicySettings = {
mcpServers: settings.mcpServers,
tools: {
core: settings.coreTools || settings.tools?.core,
exclude: settings.excludeTools || settings.tools?.exclude,
allowed: settings.allowedTools || settings.tools?.allowed,
},
policyPaths: settings.policyPaths,
adminPolicyPaths: settings.adminPolicyPaths,
};
const policyEngineConfig = await createPolicyEngineConfig(
policySettings,
approvalMode,
undefined,
true,
);
const configParams: ConfigParameters = { const configParams: ConfigParameters = {
sessionId: taskId, sessionId: taskId,
clientName: 'a2a-server', clientName: 'a2a-server',
@@ -78,20 +97,7 @@ export async function loadConfig(
allowedTools: settings.allowedTools || settings.tools?.allowed || undefined, allowedTools: settings.allowedTools || settings.tools?.allowed || undefined,
showMemoryUsage: settings.showMemoryUsage || false, showMemoryUsage: settings.showMemoryUsage || false,
approvalMode, approvalMode,
policyEngineConfig: { policyEngineConfig,
rules:
approvalMode === ApprovalMode.YOLO
? [
{
toolName: '*',
decision: PolicyDecision.ALLOW,
priority: PRIORITY_YOLO_ALLOW_ALL,
modes: [ApprovalMode.YOLO],
allowRedirection: true,
},
]
: [],
},
mcpServers: settings.mcpServers, mcpServers: settings.mcpServers,
cwd: workspaceDir, cwd: workspaceDir,
telemetry: { telemetry: {
@@ -118,7 +124,7 @@ export async function loadConfig(
}, },
ideMode: false, ideMode: false,
folderTrust, folderTrust,
trustedFolder: true, trustedFolder: trusted,
extensionLoader, extensionLoader,
checkpointing, checkpointing,
interactive: true, interactive: true,
@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
import * as path from 'node:path'; import * as path from 'node:path';
import * as os from 'node:os'; import * as os from 'node:os';
import { loadSettings, USER_SETTINGS_PATH } from './settings.js'; import { loadSettings, USER_SETTINGS_PATH } from './settings.js';
import { debugLogger } from '@google/gemini-cli-core'; import { debugLogger, checkPathTrust } from '@google/gemini-cli-core';
const mocks = vi.hoisted(() => { const mocks = vi.hoisted(() => {
const suffix = Math.random().toString(36).slice(2); const suffix = Math.random().toString(36).slice(2);
@@ -40,6 +40,8 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
}, },
getErrorMessage: (error: unknown) => String(error), getErrorMessage: (error: unknown) => String(error),
homedir: () => path.join(os.tmpdir(), `gemini-home-${mocks.suffix}`), homedir: () => path.join(os.tmpdir(), `gemini-home-${mocks.suffix}`),
checkPathTrust: vi.fn(() => ({ isTrusted: false })),
isHeadlessMode: vi.fn(() => true),
}; };
}); });
@@ -146,7 +148,7 @@ describe('loadSettings', () => {
); );
fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings)); fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings));
const result = loadSettings(mockWorkspaceDir); const result = loadSettings(mockWorkspaceDir, true);
// Primitive value overwritten // Primitive value overwritten
expect(result.showMemoryUsage).toBe(true); expect(result.showMemoryUsage).toBe(true);
@@ -154,4 +156,78 @@ describe('loadSettings', () => {
expect(result.fileFiltering?.respectGitIgnore).toBe(false); expect(result.fileFiltering?.respectGitIgnore).toBe(false);
expect(result.fileFiltering?.enableRecursiveFileSearch).toBeUndefined(); expect(result.fileFiltering?.enableRecursiveFileSearch).toBeUndefined();
}); });
describe('security', () => {
it('should NOT load workspace settings if workspace is NOT trusted', () => {
const userSettings = { showMemoryUsage: false };
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
const workspaceSettings = { showMemoryUsage: true };
const workspaceSettingsPath = path.join(
mockGeminiWorkspaceDir,
'settings.json',
);
fs.writeFileSync(
workspaceSettingsPath,
JSON.stringify(workspaceSettings),
);
// checkPathTrust is mocked to return isTrusted: false by default
const result = loadSettings(mockWorkspaceDir);
expect(result.showMemoryUsage).toBe(false);
});
it('should load workspace settings if workspace IS trusted', () => {
vi.mocked(checkPathTrust).mockReturnValueOnce({
isTrusted: true,
source: 'file',
});
const userSettings = { showMemoryUsage: false };
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
const workspaceSettings = { showMemoryUsage: true };
const workspaceSettingsPath = path.join(
mockGeminiWorkspaceDir,
'settings.json',
);
fs.writeFileSync(
workspaceSettingsPath,
JSON.stringify(workspaceSettings),
);
const result = loadSettings(mockWorkspaceDir);
expect(result.showMemoryUsage).toBe(true);
});
it('should NOT allow workspace settings to override adminPolicyPaths or policyPaths even if trusted', () => {
vi.mocked(checkPathTrust).mockReturnValueOnce({
isTrusted: true,
source: 'file',
});
const userSettings = {
adminPolicyPaths: ['/trusted/admin'],
policyPaths: ['/trusted/user'],
};
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
const workspaceSettings = {
adminPolicyPaths: ['./malicious/admin'],
policyPaths: ['./malicious/user'],
showMemoryUsage: true,
};
const workspaceSettingsPath = path.join(
mockGeminiWorkspaceDir,
'settings.json',
);
fs.writeFileSync(
workspaceSettingsPath,
JSON.stringify(workspaceSettings),
);
const result = loadSettings(mockWorkspaceDir);
expect(result.showMemoryUsage).toBe(true);
expect(result.adminPolicyPaths).toEqual(['/trusted/admin']);
expect(result.policyPaths).toEqual(['/trusted/user']);
});
});
}); });
+31 -4
View File
@@ -14,6 +14,8 @@ import {
getErrorMessage, getErrorMessage,
type TelemetrySettings, type TelemetrySettings,
homedir, homedir,
checkPathTrust,
isHeadlessMode,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import stripJsonComments from 'strip-json-comments'; import stripJsonComments from 'strip-json-comments';
@@ -51,6 +53,8 @@ export interface Settings {
experimental?: { experimental?: {
enableAgents?: boolean; enableAgents?: boolean;
}; };
policyPaths?: string[];
adminPolicyPaths?: string[];
} }
export interface SettingsError { export interface SettingsError {
@@ -64,13 +68,16 @@ export interface CheckpointingSettings {
/** /**
* Loads settings from user and workspace directories. * Loads settings from user and workspace directories.
* Project settings override user settings. * Project settings override user settings if the workspace is trusted.
* *
* How is it different to gemini-cli/cli: Returns already merged settings rather * How is it different to gemini-cli/cli: Returns already merged settings rather
* than `LoadedSettings` (unnecessary since we are not modifying users * than `LoadedSettings` (unnecessary since we are not modifying users
* settings.json). * settings.json).
*/ */
export function loadSettings(workspaceDir: string): Settings { export function loadSettings(
workspaceDir: string,
isTrustedOverride?: boolean,
): Settings {
let userSettings: Settings = {}; let userSettings: Settings = {};
let workspaceSettings: Settings = {}; let workspaceSettings: Settings = {};
const settingsErrors: SettingsError[] = []; const settingsErrors: SettingsError[] = [];
@@ -92,13 +99,24 @@ export function loadSettings(workspaceDir: string): Settings {
}); });
} }
let isTrusted = isTrustedOverride;
if (isTrusted === undefined) {
const { isTrusted: trustResult } = checkPathTrust({
path: workspaceDir,
isFolderTrustEnabled: userSettings.folderTrust ?? true,
isHeadless: isHeadlessMode(),
});
isTrusted = trustResult ?? false;
}
const workspaceSettingsPath = path.join( const workspaceSettingsPath = path.join(
workspaceDir, workspaceDir,
GEMINI_DIR, GEMINI_DIR,
'settings.json', 'settings.json',
); );
// Load workspace settings // Load workspace settings only if trusted
if (isTrusted) {
try { try {
if (fs.existsSync(workspaceSettingsPath)) { if (fs.existsSync(workspaceSettingsPath)) {
const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8'); const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8');
@@ -114,6 +132,7 @@ export function loadSettings(workspaceDir: string): Settings {
path: workspaceSettingsPath, path: workspaceSettingsPath,
}); });
} }
}
if (settingsErrors.length > 0) { if (settingsErrors.length > 0) {
debugLogger.error('Errors loading settings:'); debugLogger.error('Errors loading settings:');
@@ -125,10 +144,18 @@ export function loadSettings(workspaceDir: string): Settings {
// If there are overlapping keys, the values of workspaceSettings will // If there are overlapping keys, the values of workspaceSettings will
// override values from userSettings // override values from userSettings
return { const mergedSettings = {
...userSettings, ...userSettings,
...workspaceSettings, ...workspaceSettings,
}; };
// Security: ensure policyPaths and adminPolicyPaths are only loaded from trusted, user-level
// configuration and cannot be overridden by workspace-level settings, even if the
// workspace is trusted.
mergedSettings.policyPaths = userSettings.policyPaths;
mergedSettings.adminPolicyPaths = userSettings.adminPolicyPaths;
return mergedSettings;
} }
function resolveEnvVarsInString(value: string): string { function resolveEnvVarsInString(value: string): string {
+14 -1
View File
@@ -30,6 +30,8 @@ import {
debugLogger, debugLogger,
SimpleExtensionLoader, SimpleExtensionLoader,
GitService, GitService,
checkPathTrust,
isHeadlessMode,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import type { Command, CommandArgument } from '../commands/types.js'; import type { Command, CommandArgument } from '../commands/types.js';
@@ -197,12 +199,23 @@ export async function createApp() {
// Load the server configuration once on startup. // Load the server configuration once on startup.
const workspaceRoot = setTargetDir(undefined); const workspaceRoot = setTargetDir(undefined);
loadEnvironment(); loadEnvironment();
const settings = loadSettings(workspaceRoot);
// Use a temporary settings load to check if folder trust is enabled.
// This is similar to how the CLI handles the initial trust check.
const initialSettings = loadSettings(workspaceRoot, false);
const { isTrusted } = checkPathTrust({
path: workspaceRoot,
isFolderTrustEnabled: initialSettings.folderTrust ?? true,
isHeadless: isHeadlessMode(),
});
const settings = loadSettings(workspaceRoot, isTrusted ?? false);
const extensions = loadExtensions(workspaceRoot); const extensions = loadExtensions(workspaceRoot);
const config = await loadConfig( const config = await loadConfig(
settings, settings,
new SimpleExtensionLoader(extensions), new SimpleExtensionLoader(extensions),
'a2a-server', 'a2a-server',
isTrusted ?? false,
); );
let git: GitService | undefined; let git: GitService | undefined;
+1
View File
@@ -47,6 +47,7 @@ export interface AgentSettings {
kind: CoderAgentEvent.StateAgentSettingsEvent; kind: CoderAgentEvent.StateAgentSettingsEvent;
workspacePath: string; workspacePath: string;
autoExecute?: boolean; autoExecute?: boolean;
isTrusted?: boolean;
} }
export interface ToolCallConfirmation { export interface ToolCallConfirmation {
+12
View File
@@ -54,6 +54,18 @@ for (const file of policyFiles) {
console.log(`Copied ${policyFiles.length} policy files to bundle/policies/`); console.log(`Copied ${policyFiles.length} policy files to bundle/policies/`);
// Also copy policies to a2a-server dist directory for bundled execution
const a2aPolicyDir = join(root, 'packages/a2a-server/dist/policies');
if (!existsSync(a2aPolicyDir)) {
mkdirSync(a2aPolicyDir, { recursive: true });
}
for (const file of policyFiles) {
copyFileSync(join(root, file), join(a2aPolicyDir, basename(file)));
}
console.log(
`Copied ${policyFiles.length} policy files to packages/a2a-server/dist/policies/`,
);
// 3. Copy Documentation (docs/) // 3. Copy Documentation (docs/)
const docsSrc = join(root, 'docs'); const docsSrc = join(root, 'docs');
const docsDest = join(bundleDir, 'docs'); const docsDest = join(bundleDir, 'docs');