Skip to content

Commit 85566a7

Browse files
authored
fix(a2a-server): Implement default policy loading for parity with CLI (google-gemini#27073)
1 parent 7478859 commit 85566a7

8 files changed

Lines changed: 294 additions & 41 deletions

File tree

packages/a2a-server/src/agent/executor.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,16 @@ export class CoderAgentExecutor implements AgentExecutor {
9393
taskId: string,
9494
): Promise<Config> {
9595
const workspaceRoot = setTargetDir(agentSettings);
96+
const isTrusted = agentSettings.isTrusted ?? false;
9697
loadEnvironment(); // Will override any global env with workspace envs
97-
const settings = loadSettings(workspaceRoot);
98+
const settings = loadSettings(workspaceRoot, isTrusted);
9899
const extensions = loadExtensions(workspaceRoot);
99-
return loadConfig(settings, new SimpleExtensionLoader(extensions), taskId);
100+
return loadConfig(
101+
settings,
102+
new SimpleExtensionLoader(extensions),
103+
taskId,
104+
isTrusted,
105+
);
100106
}
101107

102108
/**

packages/a2a-server/src/config/config.test.ts

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import {
1919
isHeadlessMode,
2020
FatalAuthenticationError,
2121
PolicyDecision,
22+
ApprovalMode,
2223
PRIORITY_YOLO_ALLOW_ALL,
24+
createPolicyEngineConfig,
2325
} from '@google/gemini-cli-core';
2426

2527
// Mock dependencies
@@ -53,6 +55,32 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
5355
isHeadlessMode: vi.fn().mockReturnValue(false),
5456
getCodeAssistServer: vi.fn(),
5557
fetchAdminControlsOnce: vi.fn(),
58+
createPolicyEngineConfig: vi
59+
.fn()
60+
.mockImplementation(
61+
(_settings, mode, _defaultPoliciesDir, _interactive) => ({
62+
rules:
63+
mode === actual.ApprovalMode.YOLO
64+
? [
65+
{
66+
toolName: '*',
67+
decision: actual.PolicyDecision.ALLOW,
68+
priority: actual.PRIORITY_YOLO_ALLOW_ALL,
69+
modes: [actual.ApprovalMode.YOLO],
70+
allowRedirection: true,
71+
},
72+
]
73+
: [
74+
{
75+
toolName: 'read_file',
76+
decision: actual.PolicyDecision.ALLOW,
77+
priority: 1.05,
78+
source: 'Default: read-only.toml',
79+
},
80+
],
81+
checkers: [],
82+
}),
83+
),
5684
coreEvents: {
5785
emitAdminSettingsChanged: vi.fn(),
5886
},
@@ -261,6 +289,85 @@ describe('loadConfig', () => {
261289
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]);
262290
});
263291

292+
describe('policy engine configuration', () => {
293+
it('should merge V1 and V2 tool settings into policySettings', async () => {
294+
const settings: Settings = {
295+
allowedTools: ['v1-allowed'],
296+
tools: {
297+
allowed: ['v2-allowed'],
298+
exclude: ['v2-exclude'],
299+
core: ['v2-core'],
300+
},
301+
mcpServers: {
302+
test: { command: 'test', args: [] },
303+
},
304+
policyPaths: ['/path/to/policy'],
305+
adminPolicyPaths: ['/path/to/admin/policy'],
306+
};
307+
308+
await loadConfig(settings, mockExtensionLoader, taskId);
309+
310+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
311+
expect.objectContaining({
312+
tools: {
313+
core: ['v2-core'],
314+
exclude: ['v2-exclude'],
315+
allowed: ['v1-allowed'],
316+
},
317+
mcpServers: settings.mcpServers,
318+
policyPaths: settings.policyPaths,
319+
adminPolicyPaths: settings.adminPolicyPaths,
320+
}),
321+
ApprovalMode.DEFAULT,
322+
undefined,
323+
true,
324+
);
325+
});
326+
327+
it('should use V2 tool settings when V1 is missing', async () => {
328+
const settings: Settings = {
329+
tools: {
330+
allowed: ['v2-allowed'],
331+
},
332+
};
333+
334+
await loadConfig(settings, mockExtensionLoader, taskId);
335+
336+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
337+
expect.objectContaining({
338+
tools: expect.objectContaining({
339+
allowed: ['v2-allowed'],
340+
}),
341+
}),
342+
ApprovalMode.DEFAULT,
343+
undefined,
344+
true,
345+
);
346+
});
347+
348+
it('should use V1 tool settings when V2 is also present', async () => {
349+
const settings: Settings = {
350+
allowedTools: ['v1-allowed'],
351+
tools: {
352+
allowed: ['v2-allowed'],
353+
},
354+
};
355+
356+
await loadConfig(settings, mockExtensionLoader, taskId);
357+
358+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
359+
expect.objectContaining({
360+
tools: expect.objectContaining({
361+
allowed: ['v1-allowed'],
362+
}),
363+
}),
364+
ApprovalMode.DEFAULT,
365+
undefined,
366+
true,
367+
);
368+
});
369+
});
370+
264371
describe('tool configuration', () => {
265372
it('should pass V1 allowedTools to Config properly', async () => {
266373
const settings: Settings = {
@@ -385,14 +492,19 @@ describe('loadConfig', () => {
385492
);
386493
});
387494

388-
it('should use default approval mode and empty rules when GEMINI_YOLO_MODE is not true', async () => {
495+
it('should use default approval mode and load default rules when GEMINI_YOLO_MODE is not true', async () => {
389496
vi.stubEnv('GEMINI_YOLO_MODE', 'false');
390497
await loadConfig(mockSettings, mockExtensionLoader, taskId);
391498
expect(Config).toHaveBeenCalledWith(
392499
expect.objectContaining({
393500
approvalMode: 'default',
394501
policyEngineConfig: expect.objectContaining({
395-
rules: [],
502+
rules: expect.arrayContaining([
503+
expect.objectContaining({
504+
toolName: 'read_file',
505+
decision: PolicyDecision.ALLOW,
506+
}),
507+
]),
396508
}),
397509
}),
398510
);

packages/a2a-server/src/config/config.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import {
2323
ExperimentFlags,
2424
isHeadlessMode,
2525
FatalAuthenticationError,
26-
PolicyDecision,
27-
PRIORITY_YOLO_ALLOW_ALL,
26+
createPolicyEngineConfig,
27+
type PolicySettings,
2828
type TelemetryTarget,
2929
type ConfigParameters,
3030
type ExtensionLoader,
@@ -38,6 +38,7 @@ export async function loadConfig(
3838
settings: Settings,
3939
extensionLoader: ExtensionLoader,
4040
taskId: string,
41+
trusted: boolean = false,
4142
): Promise<Config> {
4243
const workspaceDir = process.cwd();
4344

@@ -63,6 +64,24 @@ export async function loadConfig(
6364
? ApprovalMode.YOLO
6465
: ApprovalMode.DEFAULT;
6566

67+
const policySettings: PolicySettings = {
68+
mcpServers: settings.mcpServers,
69+
tools: {
70+
core: settings.coreTools || settings.tools?.core,
71+
exclude: settings.excludeTools || settings.tools?.exclude,
72+
allowed: settings.allowedTools || settings.tools?.allowed,
73+
},
74+
policyPaths: settings.policyPaths,
75+
adminPolicyPaths: settings.adminPolicyPaths,
76+
};
77+
78+
const policyEngineConfig = await createPolicyEngineConfig(
79+
policySettings,
80+
approvalMode,
81+
undefined,
82+
true,
83+
);
84+
6685
const configParams: ConfigParameters = {
6786
sessionId: taskId,
6887
clientName: 'a2a-server',
@@ -78,20 +97,7 @@ export async function loadConfig(
7897
allowedTools: settings.allowedTools || settings.tools?.allowed || undefined,
7998
showMemoryUsage: settings.showMemoryUsage || false,
8099
approvalMode,
81-
policyEngineConfig: {
82-
rules:
83-
approvalMode === ApprovalMode.YOLO
84-
? [
85-
{
86-
toolName: '*',
87-
decision: PolicyDecision.ALLOW,
88-
priority: PRIORITY_YOLO_ALLOW_ALL,
89-
modes: [ApprovalMode.YOLO],
90-
allowRedirection: true,
91-
},
92-
]
93-
: [],
94-
},
100+
policyEngineConfig,
95101
mcpServers: settings.mcpServers,
96102
cwd: workspaceDir,
97103
telemetry: {
@@ -118,7 +124,7 @@ export async function loadConfig(
118124
},
119125
ideMode: false,
120126
folderTrust,
121-
trustedFolder: true,
127+
trustedFolder: trusted,
122128
extensionLoader,
123129
checkpointing,
124130
interactive: true,

packages/a2a-server/src/config/settings.test.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
99
import * as path from 'node:path';
1010
import * as os from 'node:os';
1111
import { loadSettings, USER_SETTINGS_PATH } from './settings.js';
12-
import { debugLogger } from '@google/gemini-cli-core';
12+
import { debugLogger, checkPathTrust } from '@google/gemini-cli-core';
1313

1414
const mocks = vi.hoisted(() => {
1515
const suffix = Math.random().toString(36).slice(2);
@@ -40,6 +40,8 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
4040
},
4141
getErrorMessage: (error: unknown) => String(error),
4242
homedir: () => path.join(os.tmpdir(), `gemini-home-${mocks.suffix}`),
43+
checkPathTrust: vi.fn(() => ({ isTrusted: false })),
44+
isHeadlessMode: vi.fn(() => true),
4345
};
4446
});
4547

@@ -146,12 +148,86 @@ describe('loadSettings', () => {
146148
);
147149
fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings));
148150

149-
const result = loadSettings(mockWorkspaceDir);
151+
const result = loadSettings(mockWorkspaceDir, true);
150152
// Primitive value overwritten
151153
expect(result.showMemoryUsage).toBe(true);
152154

153155
// Object value completely replaced (shallow merge behavior)
154156
expect(result.fileFiltering?.respectGitIgnore).toBe(false);
155157
expect(result.fileFiltering?.enableRecursiveFileSearch).toBeUndefined();
156158
});
159+
160+
describe('security', () => {
161+
it('should NOT load workspace settings if workspace is NOT trusted', () => {
162+
const userSettings = { showMemoryUsage: false };
163+
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
164+
165+
const workspaceSettings = { showMemoryUsage: true };
166+
const workspaceSettingsPath = path.join(
167+
mockGeminiWorkspaceDir,
168+
'settings.json',
169+
);
170+
fs.writeFileSync(
171+
workspaceSettingsPath,
172+
JSON.stringify(workspaceSettings),
173+
);
174+
175+
// checkPathTrust is mocked to return isTrusted: false by default
176+
const result = loadSettings(mockWorkspaceDir);
177+
expect(result.showMemoryUsage).toBe(false);
178+
});
179+
180+
it('should load workspace settings if workspace IS trusted', () => {
181+
vi.mocked(checkPathTrust).mockReturnValueOnce({
182+
isTrusted: true,
183+
source: 'file',
184+
});
185+
const userSettings = { showMemoryUsage: false };
186+
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
187+
188+
const workspaceSettings = { showMemoryUsage: true };
189+
const workspaceSettingsPath = path.join(
190+
mockGeminiWorkspaceDir,
191+
'settings.json',
192+
);
193+
fs.writeFileSync(
194+
workspaceSettingsPath,
195+
JSON.stringify(workspaceSettings),
196+
);
197+
198+
const result = loadSettings(mockWorkspaceDir);
199+
expect(result.showMemoryUsage).toBe(true);
200+
});
201+
202+
it('should NOT allow workspace settings to override adminPolicyPaths or policyPaths even if trusted', () => {
203+
vi.mocked(checkPathTrust).mockReturnValueOnce({
204+
isTrusted: true,
205+
source: 'file',
206+
});
207+
const userSettings = {
208+
adminPolicyPaths: ['/trusted/admin'],
209+
policyPaths: ['/trusted/user'],
210+
};
211+
fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings));
212+
213+
const workspaceSettings = {
214+
adminPolicyPaths: ['./malicious/admin'],
215+
policyPaths: ['./malicious/user'],
216+
showMemoryUsage: true,
217+
};
218+
const workspaceSettingsPath = path.join(
219+
mockGeminiWorkspaceDir,
220+
'settings.json',
221+
);
222+
fs.writeFileSync(
223+
workspaceSettingsPath,
224+
JSON.stringify(workspaceSettings),
225+
);
226+
227+
const result = loadSettings(mockWorkspaceDir);
228+
expect(result.showMemoryUsage).toBe(true);
229+
expect(result.adminPolicyPaths).toEqual(['/trusted/admin']);
230+
expect(result.policyPaths).toEqual(['/trusted/user']);
231+
});
232+
});
157233
});

0 commit comments

Comments
 (0)