Skip to content

Commit 31ea2a8

Browse files
fix(patch): cherry-pick 85566a7 to release/v0.43.0-preview.0-pr-27073 [CONFLICTS] (#27256)
Co-authored-by: Keith Schaab <keith.schaab@gmail.com> Co-authored-by: Keith Schaab <keithsc@google.com>
1 parent 20495d6 commit 31ea2a8

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
@@ -20,7 +20,9 @@ import {
2020
isHeadlessMode,
2121
FatalAuthenticationError,
2222
PolicyDecision,
23+
ApprovalMode,
2324
PRIORITY_YOLO_ALLOW_ALL,
25+
createPolicyEngineConfig,
2426
} from '@google/gemini-cli-core';
2527

2628
// Mock dependencies
@@ -60,6 +62,32 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
6062
FileDiscoveryService: vi.fn(),
6163
getCodeAssistServer: vi.fn(),
6264
fetchAdminControlsOnce: vi.fn(),
65+
createPolicyEngineConfig: vi
66+
.fn()
67+
.mockImplementation(
68+
(_settings, mode, _defaultPoliciesDir, _interactive) => ({
69+
rules:
70+
mode === actual.ApprovalMode.YOLO
71+
? [
72+
{
73+
toolName: '*',
74+
decision: actual.PolicyDecision.ALLOW,
75+
priority: actual.PRIORITY_YOLO_ALLOW_ALL,
76+
modes: [actual.ApprovalMode.YOLO],
77+
allowRedirection: true,
78+
},
79+
]
80+
: [
81+
{
82+
toolName: 'read_file',
83+
decision: actual.PolicyDecision.ALLOW,
84+
priority: 1.05,
85+
source: 'Default: read-only.toml',
86+
},
87+
],
88+
checkers: [],
89+
}),
90+
),
6391
coreEvents: {
6492
emitAdminSettingsChanged: vi.fn(),
6593
},
@@ -286,6 +314,85 @@ describe('loadConfig', () => {
286314
});
287315
});
288316

317+
describe('policy engine configuration', () => {
318+
it('should merge V1 and V2 tool settings into policySettings', async () => {
319+
const settings: Settings = {
320+
allowedTools: ['v1-allowed'],
321+
tools: {
322+
allowed: ['v2-allowed'],
323+
exclude: ['v2-exclude'],
324+
core: ['v2-core'],
325+
},
326+
mcpServers: {
327+
test: { command: 'test', args: [] },
328+
},
329+
policyPaths: ['/path/to/policy'],
330+
adminPolicyPaths: ['/path/to/admin/policy'],
331+
};
332+
333+
await loadConfig(settings, mockExtensionLoader, taskId);
334+
335+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
336+
expect.objectContaining({
337+
tools: {
338+
core: ['v2-core'],
339+
exclude: ['v2-exclude'],
340+
allowed: ['v1-allowed'],
341+
},
342+
mcpServers: settings.mcpServers,
343+
policyPaths: settings.policyPaths,
344+
adminPolicyPaths: settings.adminPolicyPaths,
345+
}),
346+
ApprovalMode.DEFAULT,
347+
undefined,
348+
true,
349+
);
350+
});
351+
352+
it('should use V2 tool settings when V1 is missing', async () => {
353+
const settings: Settings = {
354+
tools: {
355+
allowed: ['v2-allowed'],
356+
},
357+
};
358+
359+
await loadConfig(settings, mockExtensionLoader, taskId);
360+
361+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
362+
expect.objectContaining({
363+
tools: expect.objectContaining({
364+
allowed: ['v2-allowed'],
365+
}),
366+
}),
367+
ApprovalMode.DEFAULT,
368+
undefined,
369+
true,
370+
);
371+
});
372+
373+
it('should use V1 tool settings when V2 is also present', async () => {
374+
const settings: Settings = {
375+
allowedTools: ['v1-allowed'],
376+
tools: {
377+
allowed: ['v2-allowed'],
378+
},
379+
};
380+
381+
await loadConfig(settings, mockExtensionLoader, taskId);
382+
383+
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
384+
expect.objectContaining({
385+
tools: expect.objectContaining({
386+
allowed: ['v1-allowed'],
387+
}),
388+
}),
389+
ApprovalMode.DEFAULT,
390+
undefined,
391+
true,
392+
);
393+
});
394+
});
395+
289396
describe('tool configuration', () => {
290397
it('should pass V1 allowedTools to Config properly', async () => {
291398
const settings: Settings = {
@@ -410,14 +517,19 @@ describe('loadConfig', () => {
410517
);
411518
});
412519

413-
it('should use default approval mode and empty rules when GEMINI_YOLO_MODE is not true', async () => {
520+
it('should use default approval mode and load default rules when GEMINI_YOLO_MODE is not true', async () => {
414521
vi.stubEnv('GEMINI_YOLO_MODE', 'false');
415522
await loadConfig(mockSettings, mockExtensionLoader, taskId);
416523
expect(Config).toHaveBeenCalledWith(
417524
expect.objectContaining({
418525
approvalMode: 'default',
419526
policyEngineConfig: expect.objectContaining({
420-
rules: [],
527+
rules: expect.arrayContaining([
528+
expect.objectContaining({
529+
toolName: 'read_file',
530+
decision: PolicyDecision.ALLOW,
531+
}),
532+
]),
421533
}),
422534
}),
423535
);

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

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import {
2525
ExperimentFlags,
2626
isHeadlessMode,
2727
FatalAuthenticationError,
28-
PolicyDecision,
29-
PRIORITY_YOLO_ALLOW_ALL,
28+
createPolicyEngineConfig,
29+
type PolicySettings,
3030
type TelemetryTarget,
3131
type ConfigParameters,
3232
type ExtensionLoader,
@@ -40,6 +40,7 @@ export async function loadConfig(
4040
settings: Settings,
4141
extensionLoader: ExtensionLoader,
4242
taskId: string,
43+
trusted: boolean = false,
4344
): Promise<Config> {
4445
const workspaceDir = process.cwd();
4546

@@ -65,6 +66,24 @@ export async function loadConfig(
6566
? ApprovalMode.YOLO
6667
: ApprovalMode.DEFAULT;
6768

69+
const policySettings: PolicySettings = {
70+
mcpServers: settings.mcpServers,
71+
tools: {
72+
core: settings.coreTools || settings.tools?.core,
73+
exclude: settings.excludeTools || settings.tools?.exclude,
74+
allowed: settings.allowedTools || settings.tools?.allowed,
75+
},
76+
policyPaths: settings.policyPaths,
77+
adminPolicyPaths: settings.adminPolicyPaths,
78+
};
79+
80+
const policyEngineConfig = await createPolicyEngineConfig(
81+
policySettings,
82+
approvalMode,
83+
undefined,
84+
true,
85+
);
86+
6887
const configParams: ConfigParameters = {
6988
sessionId: taskId,
7089
clientName: 'a2a-server',
@@ -80,20 +99,7 @@ export async function loadConfig(
8099
allowedTools: settings.allowedTools || settings.tools?.allowed || undefined,
81100
showMemoryUsage: settings.showMemoryUsage || false,
82101
approvalMode,
83-
policyEngineConfig: {
84-
rules:
85-
approvalMode === ApprovalMode.YOLO
86-
? [
87-
{
88-
toolName: '*',
89-
decision: PolicyDecision.ALLOW,
90-
priority: PRIORITY_YOLO_ALLOW_ALL,
91-
modes: [ApprovalMode.YOLO],
92-
allowRedirection: true,
93-
},
94-
]
95-
: [],
96-
},
102+
policyEngineConfig,
97103
mcpServers: settings.mcpServers,
98104
cwd: workspaceDir,
99105
telemetry: {
@@ -120,7 +126,7 @@ export async function loadConfig(
120126
},
121127
ideMode: false,
122128
folderTrust,
123-
trustedFolder: true,
129+
trustedFolder: trusted,
124130
extensionLoader,
125131
checkpointing,
126132
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)