diff --git a/packages/a2a-server/src/agent/executor.ts b/packages/a2a-server/src/agent/executor.ts index dbb8269376..db2043c4a0 100644 --- a/packages/a2a-server/src/agent/executor.ts +++ b/packages/a2a-server/src/agent/executor.ts @@ -26,7 +26,7 @@ import { getPersistedState, setPersistedState, type StateChange, - type AgentSettings, + type AgentSettings as CoderAgentSettings, type PersistedStateMetadata, getContextIdFromMetadata, getAgentSettingsFromMetadata, @@ -44,9 +44,9 @@ import { pushTaskStateFailed } from '../utils/executor_utils.js'; */ class TaskWrapper { task: Task; - agentSettings: AgentSettings; + agentSettings: CoderAgentSettings; - constructor(task: Task, agentSettings: AgentSettings) { + constructor(task: Task, agentSettings: CoderAgentSettings) { this.task = task; this.agentSettings = agentSettings; } @@ -89,14 +89,18 @@ export class CoderAgentExecutor implements AgentExecutor { constructor(private taskStore?: TaskStore) {} private async getConfig( - agentSettings: AgentSettings, + agentSettings: CoderAgentSettings, taskId: string, ): Promise { const workspaceRoot = setTargetDir(agentSettings); loadEnvironment(); // Will override any global env with workspace envs - const settings = loadSettings(workspaceRoot); + const loadedSettings = loadSettings(workspaceRoot); const extensions = loadExtensions(workspaceRoot); - return loadConfig(settings, new SimpleExtensionLoader(extensions), taskId); + return loadConfig( + loadedSettings, + new SimpleExtensionLoader(extensions), + taskId, + ); } /** @@ -138,10 +142,10 @@ export class CoderAgentExecutor implements AgentExecutor { async createTask( taskId: string, contextId: string, - agentSettingsInput?: AgentSettings, + agentSettingsInput?: CoderAgentSettings, eventBus?: ExecutionEventBus, ): Promise { - const agentSettings: AgentSettings = agentSettingsInput || { + const agentSettings: CoderAgentSettings = agentSettingsInput || { kind: CoderAgentEvent.StateAgentSettingsEvent, workspacePath: process.cwd(), }; diff --git a/packages/a2a-server/src/config/config.test.ts b/packages/a2a-server/src/config/config.test.ts index ee63df36f7..ca646f980d 100644 --- a/packages/a2a-server/src/config/config.test.ts +++ b/packages/a2a-server/src/config/config.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as path from 'node:path'; import { loadConfig } from './config.js'; -import type { Settings } from './settings.js'; +import type { LoadedSettings } from './settings.js'; import { type ExtensionLoader, FileDiscoveryService, @@ -40,6 +40,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { }, }, }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }; @@ -72,13 +73,35 @@ vi.mock('../utils/logger.js', () => ({ })); describe('loadConfig', () => { - const mockSettings = {} as Settings; + const mockLoadedSettings: LoadedSettings = { + userSettings: {}, + workspaceSettings: {}, + }; const mockExtensionLoader = {} as ExtensionLoader; const taskId = 'test-task-id'; beforeEach(() => { vi.clearAllMocks(); vi.stubEnv('GEMINI_API_KEY', 'test-key'); + + // Default mock implementation for Config to include isTrustedFolder + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (Config as any).mockImplementation((params: any) => ({ + ...params, + initialize: vi.fn(), + waitForMcpInit: vi.fn(), + refreshAuth: vi.fn(), + getExperiments: vi.fn().mockReturnValue({ + flags: { + [ExperimentFlags.ENABLE_ADMIN_CONTROLS]: { + boolValue: false, + }, + }, + }), + isTrustedFolder: vi.fn().mockReturnValue(true), + getRemoteAdminSettings: vi.fn(), + setRemoteAdminSettings: vi.fn(), + })); }); afterEach(() => { @@ -87,7 +110,7 @@ describe('loadConfig', () => { describe('admin settings overrides', () => { it('should not fetch admin controls if experiment is disabled', async () => { - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(fetchAdminControlsOnce).not.toHaveBeenCalled(); }); @@ -108,6 +131,7 @@ describe('loadConfig', () => { }, }, }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn().mockReturnValue({}), setRemoteAdminSettings: vi.fn(), }; @@ -129,7 +153,7 @@ describe('loadConfig', () => { }; vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenLastCalledWith( expect.objectContaining({ @@ -150,7 +174,7 @@ describe('loadConfig', () => { }; vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenLastCalledWith( expect.objectContaining({ @@ -165,7 +189,7 @@ describe('loadConfig', () => { const mockAdminSettings: FetchAdminControlsResponse = {}; vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenLastCalledWith(expect.objectContaining({})); }); @@ -184,7 +208,7 @@ describe('loadConfig', () => { ); vi.mocked(fetchAdminControlsOnce).mockResolvedValue(mockAdminSettings); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(fetchAdminControlsOnce).toHaveBeenCalledWith( mockCodeAssistServer, @@ -204,7 +228,11 @@ describe('loadConfig', () => { it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => { const testPath = '/tmp/ignore'; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath); - const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); + const config = await loadConfig( + mockLoadedSettings, + mockExtensionLoader, + taskId, + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ testPath, @@ -213,12 +241,19 @@ describe('loadConfig', () => { it('should set customIgnoreFilePaths when settings.fileFiltering.customIgnoreFilePaths is present', async () => { const testPath = '/settings/ignore'; - const settings: Settings = { - fileFiltering: { - customIgnoreFilePaths: [testPath], + const loadedSettings: LoadedSettings = { + userSettings: { + fileFiltering: { + customIgnoreFilePaths: [testPath], + }, }, + workspaceSettings: {}, }; - const config = await loadConfig(settings, mockExtensionLoader, taskId); + const config = await loadConfig( + loadedSettings, + mockExtensionLoader, + taskId, + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ testPath, @@ -229,12 +264,19 @@ describe('loadConfig', () => { const envPath = '/env/ignore'; const settingsPath = '/settings/ignore'; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', envPath); - const settings: Settings = { - fileFiltering: { - customIgnoreFilePaths: [settingsPath], + const loadedSettings: LoadedSettings = { + userSettings: { + fileFiltering: { + customIgnoreFilePaths: [settingsPath], + }, }, + workspaceSettings: {}, }; - const config = await loadConfig(settings, mockExtensionLoader, taskId); + const config = await loadConfig( + loadedSettings, + mockExtensionLoader, + taskId, + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ settingsPath, @@ -245,13 +287,21 @@ describe('loadConfig', () => { it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => { const paths = ['/path/one', '/path/two']; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', paths.join(path.delimiter)); - const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); + const config = await loadConfig( + mockLoadedSettings, + mockExtensionLoader, + taskId, + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths); }); it('should have empty customIgnoreFilePaths when both are missing', async () => { - const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); + const config = await loadConfig( + mockLoadedSettings, + mockExtensionLoader, + taskId, + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([]); }); @@ -259,13 +309,16 @@ describe('loadConfig', () => { it('should initialize FileDiscoveryService with correct options', async () => { const testPath = '/tmp/ignore'; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath); - const settings: Settings = { - fileFiltering: { - respectGitIgnore: false, + const loadedSettings: LoadedSettings = { + userSettings: { + fileFiltering: { + respectGitIgnore: false, + }, }, + workspaceSettings: {}, }; - await loadConfig(settings, mockExtensionLoader, taskId); + await loadConfig(loadedSettings, mockExtensionLoader, taskId); expect(FileDiscoveryService).toHaveBeenCalledWith(expect.any(String), { respectGitIgnore: false, @@ -276,10 +329,13 @@ describe('loadConfig', () => { describe('tool configuration', () => { it('should pass V1 allowedTools to Config properly', async () => { - const settings: Settings = { - allowedTools: ['shell', 'edit'], + const loadedSettings: LoadedSettings = { + userSettings: { + allowedTools: ['shell', 'edit'], + }, + workspaceSettings: {}, }; - await loadConfig(settings, mockExtensionLoader, taskId); + await loadConfig(loadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenCalledWith( expect.objectContaining({ allowedTools: ['shell', 'edit'], @@ -288,12 +344,15 @@ describe('loadConfig', () => { }); it('should pass V2 tools.allowed to Config properly', async () => { - const settings: Settings = { - tools: { - allowed: ['shell', 'fetch'], + const loadedSettings: LoadedSettings = { + userSettings: { + tools: { + allowed: ['shell', 'fetch'], + }, }, + workspaceSettings: {}, }; - await loadConfig(settings, mockExtensionLoader, taskId); + await loadConfig(loadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenCalledWith( expect.objectContaining({ allowedTools: ['shell', 'fetch'], @@ -302,13 +361,16 @@ describe('loadConfig', () => { }); it('should prefer V1 allowedTools over V2 tools.allowed if both present', async () => { - const settings: Settings = { - allowedTools: ['v1-tool'], - tools: { - allowed: ['v2-tool'], + const loadedSettings: LoadedSettings = { + userSettings: { + allowedTools: ['v1-tool'], + tools: { + allowed: ['v2-tool'], + }, }, + workspaceSettings: {}, }; - await loadConfig(settings, mockExtensionLoader, taskId); + await loadConfig(loadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenCalledWith( expect.objectContaining({ allowedTools: ['v1-tool'], @@ -316,10 +378,42 @@ describe('loadConfig', () => { ); }); + it('should pass agent settings to Config', async () => { + const loadedSettings: LoadedSettings = { + userSettings: { + experimental: { + enableAgents: false, + }, + agents: { + overrides: { + test_agent: { enabled: true }, + }, + }, + }, + workspaceSettings: {}, + }; + await loadConfig(loadedSettings, mockExtensionLoader, taskId); + expect(Config).toHaveBeenCalledWith( + expect.objectContaining({ + enableAgents: false, + agents: loadedSettings.userSettings.agents, + }), + ); + }); + + it('should default enableAgents to false if not specified (secure default)', async () => { + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); + expect(Config).toHaveBeenCalledWith( + expect.objectContaining({ + enableAgents: false, + }), + ); + }); + describe('interactivity', () => { it('should set interactive true when not headless', async () => { vi.mocked(isHeadlessMode).mockReturnValue(false); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenCalledWith( expect.objectContaining({ interactive: true, @@ -330,7 +424,7 @@ describe('loadConfig', () => { it('should set interactive false when headless', async () => { vi.mocked(isHeadlessMode).mockReturnValue(true); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(Config).toHaveBeenCalledWith( expect.objectContaining({ interactive: false, @@ -369,12 +463,13 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(refreshAuthMock).toHaveBeenCalledWith( AuthType.LOGIN_WITH_GOOGLE, @@ -399,13 +494,14 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); await expect( - loadConfig(mockSettings, mockExtensionLoader, taskId), + loadConfig(mockLoadedSettings, mockExtensionLoader, taskId), ).rejects.toThrow('Non-interactive session'); expect(refreshAuthMock).toHaveBeenCalledWith( @@ -428,12 +524,13 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(refreshAuthMock).not.toHaveBeenCalledWith( AuthType.LOGIN_WITH_GOOGLE, @@ -455,12 +552,13 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); - await loadConfig(mockSettings, mockExtensionLoader, taskId); + await loadConfig(mockLoadedSettings, mockExtensionLoader, taskId); expect(refreshAuthMock).not.toHaveBeenCalledWith( AuthType.LOGIN_WITH_GOOGLE, @@ -481,13 +579,14 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); await expect( - loadConfig(mockSettings, mockExtensionLoader, taskId), + loadConfig(mockLoadedSettings, mockExtensionLoader, taskId), ).rejects.toThrow( 'Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.', ); @@ -517,13 +616,14 @@ describe('loadConfig', () => { waitForMcpInit: vi.fn(), refreshAuth: refreshAuthMock, getExperiments: vi.fn().mockReturnValue({ flags: {} }), + isTrustedFolder: vi.fn().mockReturnValue(true), getRemoteAdminSettings: vi.fn(), setRemoteAdminSettings: vi.fn(), }) as unknown as Config, ); await expect( - loadConfig(mockSettings, mockExtensionLoader, taskId), + loadConfig(mockLoadedSettings, mockExtensionLoader, taskId), ).rejects.toThrow( 'OAuth failed. Fallback to COMPUTE_ADC also failed: ADC failed', ); diff --git a/packages/a2a-server/src/config/config.ts b/packages/a2a-server/src/config/config.ts index 5b6757701d..5815d4af01 100644 --- a/packages/a2a-server/src/config/config.ts +++ b/packages/a2a-server/src/config/config.ts @@ -32,17 +32,49 @@ import { } from '@google/gemini-cli-core'; import { logger } from '../utils/logger.js'; -import type { Settings } from './settings.js'; -import { type AgentSettings, CoderAgentEvent } from '../types.js'; +import { + type Settings, + type LoadedSettings, + resolveEnvVarsInObject, +} from './settings.js'; +import { + type AgentSettings as CoderAgentSettings, + CoderAgentEvent, +} from '../types.js'; export async function loadConfig( - settings: Settings, + loadedSettings: LoadedSettings, extensionLoader: ExtensionLoader, taskId: string, ): Promise { const workspaceDir = process.cwd(); const adcFilePath = process.env['GOOGLE_APPLICATION_CREDENTIALS']; + const { userSettings, workspaceSettings } = loadedSettings; + + // Set an initial config to use to determine trust and get a code assist server. + // This is needed to fetch admin controls and safely resolve environment variables. + const initialConfigParams: ConfigParameters = { + sessionId: taskId, + targetDir: workspaceDir, + cwd: workspaceDir, + extensionLoader, + debugMode: process.env['DEBUG'] === 'true' || false, + model: PREVIEW_GEMINI_MODEL, + }; + const tempConfig = new Config(initialConfigParams); + + // Securely merge settings: only expand workspace variables if the user trusts this folder. + // If there are overlapping keys, the values of workspaceSettings will + // override values from userSettings. + const isTrusted = tempConfig.isTrustedFolder(); + const settings: Settings = { + ...userSettings, + ...(isTrusted + ? resolveEnvVarsInObject(workspaceSettings) + : workspaceSettings), + }; + const folderTrust = settings.folderTrust === true || process.env['GEMINI_FOLDER_TRUST'] === 'true'; @@ -109,6 +141,8 @@ export async function loadConfig( interactive: !isHeadlessMode(), enableInteractiveShell: !isHeadlessMode(), ptyInfo: 'auto', + enableAgents: settings.experimental?.enableAgents ?? false, + agents: settings.agents, }; const fileService = new FileDiscoveryService(workspaceDir, { @@ -128,16 +162,11 @@ export async function loadConfig( configParams.geminiMdFileCount = fileCount; configParams.geminiMdFilePaths = filePaths; - // Set an initial config to use to get a code assist server. - // This is needed to fetch admin controls. - const initialConfig = new Config({ - ...configParams, - }); - - const codeAssistServer = getCodeAssistServer(initialConfig); + // Use initialConfig to fetch admin controls. + const codeAssistServer = getCodeAssistServer(tempConfig); const adminControlsEnabled = - initialConfig.getExperiments()?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS] + tempConfig.getExperiments()?.flags[ExperimentFlags.ENABLE_ADMIN_CONTROLS] ?.boolValue ?? false; // Initialize final config parameters to the previous parameters. @@ -177,7 +206,9 @@ export async function loadConfig( return config; } -export function setTargetDir(agentSettings: AgentSettings | undefined): string { +export function setTargetDir( + agentSettings: CoderAgentSettings | undefined, +): string { const originalCWD = process.cwd(); const targetDir = process.env['CODER_AGENT_WORKSPACE_PATH'] ?? diff --git a/packages/a2a-server/src/config/settings.test.ts b/packages/a2a-server/src/config/settings.test.ts index 7c51950535..db7ba78719 100644 --- a/packages/a2a-server/src/config/settings.test.ts +++ b/packages/a2a-server/src/config/settings.test.ts @@ -106,27 +106,41 @@ describe('loadSettings', () => { fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(settings)); const result = loadSettings(mockWorkspaceDir); - expect(result.showMemoryUsage).toBe(true); - expect(result.coreTools).toEqual(['tool1', 'tool2']); - expect(result.mcpServers).toHaveProperty('server1'); - expect(result.fileFiltering?.respectGitIgnore).toBe(true); + expect(result.userSettings.showMemoryUsage).toBe(true); + expect(result.userSettings.coreTools).toEqual(['tool1', 'tool2']); + expect(result.userSettings.mcpServers).toHaveProperty('server1'); + expect(result.userSettings.fileFiltering?.respectGitIgnore).toBe(true); }); - it('should overwrite top-level settings from workspace (shallow merge)', () => { + it('should load experimental and agents settings correctly', () => { + const settings = { + experimental: { + enableAgents: true, + }, + agents: { + overrides: { + test_agent: { enabled: false }, + }, + }, + }; + fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(settings)); + + const result = loadSettings(mockWorkspaceDir); + expect(result.userSettings.experimental?.enableAgents).toBe(true); + expect(result.userSettings.agents?.overrides?.['test_agent']?.enabled).toBe( + false, + ); + }); + + it('should return separate user and raw workspace settings', () => { const userSettings = { showMemoryUsage: false, - fileFiltering: { - respectGitIgnore: true, - enableRecursiveFileSearch: true, - }, }; fs.writeFileSync(USER_SETTINGS_PATH, JSON.stringify(userSettings)); const workspaceSettings = { showMemoryUsage: true, - fileFiltering: { - respectGitIgnore: false, - }, + coreTools: ['${VAR}'], }; const workspaceSettingsPath = path.join( mockGeminiWorkspaceDir, @@ -135,11 +149,9 @@ describe('loadSettings', () => { fs.writeFileSync(workspaceSettingsPath, JSON.stringify(workspaceSettings)); const result = loadSettings(mockWorkspaceDir); - // Primitive value overwritten - expect(result.showMemoryUsage).toBe(true); - - // Object value completely replaced (shallow merge behavior) - expect(result.fileFiltering?.respectGitIgnore).toBe(false); - expect(result.fileFiltering?.enableRecursiveFileSearch).toBeUndefined(); + expect(result.userSettings.showMemoryUsage).toBe(false); + expect(result.workspaceSettings.showMemoryUsage).toBe(true); + // Workspace settings must be RAW (no expansion) + expect(result.workspaceSettings.coreTools).toEqual(['${VAR}']); }); }); diff --git a/packages/a2a-server/src/config/settings.ts b/packages/a2a-server/src/config/settings.ts index da9db4e069..dfa54fe37d 100644 --- a/packages/a2a-server/src/config/settings.ts +++ b/packages/a2a-server/src/config/settings.ts @@ -14,6 +14,7 @@ import { getErrorMessage, type TelemetrySettings, homedir, + type AgentSettings, } from '@google/gemini-cli-core'; import stripJsonComments from 'strip-json-comments'; @@ -48,6 +49,11 @@ export interface Settings { enableRecursiveFileSearch?: boolean; customIgnoreFilePaths?: string[]; }; + experimental?: { + enableEventDrivenScheduler?: boolean; + enableAgents?: boolean; + }; + agents?: AgentSettings; } export interface SettingsError { @@ -60,19 +66,25 @@ export interface CheckpointingSettings { } /** - * Loads settings from user and workspace directories. - * Project settings override user settings. - * - * How is it different to gemini-cli/cli: Returns already merged settings rather - * than `LoadedSettings` (unnecessary since we are not modifying users - * settings.json). + * The result of loading settings, containing both user-level (expanded) + * and workspace-level (raw) configurations. */ -export function loadSettings(workspaceDir: string): Settings { +export interface LoadedSettings { + userSettings: Settings; + workspaceSettings: Settings; +} + +/** + * Loads settings from user and workspace directories. + * Workspace settings are returned RAW (without env var expansion) to prevent + * secret leakage from untrusted repositories. + */ +export function loadSettings(workspaceDir: string): LoadedSettings { let userSettings: Settings = {}; let workspaceSettings: Settings = {}; const settingsErrors: SettingsError[] = []; - // Load user settings + // Load user settings (Safe to expand) try { if (fs.existsSync(USER_SETTINGS_PATH)) { const userContent = fs.readFileSync(USER_SETTINGS_PATH, 'utf-8'); @@ -95,15 +107,14 @@ export function loadSettings(workspaceDir: string): Settings { 'settings.json', ); - // Load workspace settings + // Load workspace settings (RAW - NO EXPANSION) try { if (fs.existsSync(workspaceSettingsPath)) { const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8'); // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - const parsedWorkspaceSettings = JSON.parse( + workspaceSettings = JSON.parse( stripJsonComments(projectContent), ) as Settings; - workspaceSettings = resolveEnvVarsInObject(parsedWorkspaceSettings); } } catch (error: unknown) { settingsErrors.push({ @@ -120,11 +131,9 @@ export function loadSettings(workspaceDir: string): Settings { } } - // If there are overlapping keys, the values of workspaceSettings will - // override values from userSettings return { - ...userSettings, - ...workspaceSettings, + userSettings, + workspaceSettings, }; } @@ -140,7 +149,7 @@ function resolveEnvVarsInString(value: string): string { }); } -function resolveEnvVarsInObject(obj: T): T { +export function resolveEnvVarsInObject(obj: T): T { if ( obj === null || obj === undefined || diff --git a/packages/a2a-server/src/http/app.ts b/packages/a2a-server/src/http/app.ts index 35ca48949f..d2a9cbf219 100644 --- a/packages/a2a-server/src/http/app.ts +++ b/packages/a2a-server/src/http/app.ts @@ -18,7 +18,7 @@ import { import { A2AExpressApp, type UserBuilder } from '@a2a-js/sdk/server/express'; // Import server components import { v4 as uuidv4 } from 'uuid'; import { logger } from '../utils/logger.js'; -import type { AgentSettings } from '../types.js'; +import { type AgentSettings as CoderAgentSettings } from '../types.js'; import { GCSTaskStore, NoOpTaskStore } from '../persistence/gcs.js'; import { CoderAgentExecutor } from '../agent/executor.js'; import { requestStorage } from './requestStorage.js'; @@ -197,10 +197,10 @@ export async function createApp() { // Load the server configuration once on startup. const workspaceRoot = setTargetDir(undefined); loadEnvironment(); - const settings = loadSettings(workspaceRoot); + const loadedSettings = loadSettings(workspaceRoot); const extensions = loadExtensions(workspaceRoot); const config = await loadConfig( - settings, + loadedSettings, new SimpleExtensionLoader(extensions), 'a2a-server', ); @@ -252,7 +252,7 @@ export async function createApp() { const taskId = uuidv4(); // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const agentSettings = req.body.agentSettings as - | AgentSettings + | CoderAgentSettings | undefined; // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const contextId = req.body.contextId || uuidv4(); diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 9ac2ec0cf9..b711ae1e17 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -29,7 +29,7 @@ import { SimpleExtensionLoader } from '../utils/extensionLoader.js'; import type { ToolRegistry } from '../tools/tool-registry.js'; import { ThinkingLevel } from '@google/genai'; import type { AcknowledgedAgentsService } from './acknowledgedAgents.js'; -import { PolicyDecision } from '../policy/types.js'; +import { PolicyDecision, ApprovalMode } from '../policy/types.js'; import { A2AAuthProviderFactory } from './auth-provider/factory.js'; import type { A2AAuthProvider } from './auth-provider/types.js'; @@ -1170,6 +1170,37 @@ describe('AgentRegistry', () => { }), ); }); + + it('should register remote agents with ALLOW decision in YOLO mode', async () => { + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'YoloAgent', + description: 'A remote agent in YOLO mode', + agentCardUrl: 'https://example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + }; + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockResolvedValue({ name: 'YoloAgent' }), + } as unknown as A2AClientManager); + + const policyEngine = mockConfig.getPolicyEngine(); + vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue( + ApprovalMode.YOLO, + ); + const addRuleSpy = vi.spyOn(policyEngine, 'addRule'); + + await registry.testRegisterAgent(remoteAgent); + + // In YOLO mode, even remote agents should be registered with ALLOW. + expect(addRuleSpy).toHaveBeenLastCalledWith( + expect.objectContaining({ + toolName: 'YoloAgent', + decision: PolicyDecision.ALLOW, + source: 'AgentRegistry (Dynamic)', + }), + ); + }); }); describe('reload', () => { diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index c4b08eba22..b932e33587 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -23,7 +23,11 @@ import { type ModelConfig, ModelConfigService, } from '../services/modelConfigService.js'; -import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../policy/types.js'; +import { + PolicyDecision, + PRIORITY_SUBAGENT_TOOL, + ApprovalMode, +} from '../policy/types.js'; import { A2AAgentError, AgentAuthConfigMissingError } from './a2a-errors.js'; /** @@ -176,9 +180,8 @@ export class AgentRegistry { agent.metadata.hash, ); - if (isAcknowledged) { - agentsToRegister.push(agent); - } else { + agentsToRegister.push(agent); + if (!isAcknowledged) { unacknowledgedAgents.push(agent); } } @@ -340,10 +343,23 @@ export class AgentRegistry { policyEngine.removeRulesForTool(definition.name, 'AgentRegistry (Dynamic)'); // Add the new dynamic policy + const isYolo = this.config.getApprovalMode() === ApprovalMode.YOLO; + const isAcknowledged = + definition.kind === 'local' && + (!definition.metadata?.hash || + (this.config.getProjectRoot() && + this.config + .getAcknowledgedAgentsService() + ?.isAcknowledgedSync?.( + this.config.getProjectRoot(), + definition.name, + definition.metadata.hash, + ))); + policyEngine.addRule({ toolName: definition.name, decision: - definition.kind === 'local' + isAcknowledged || isYolo ? PolicyDecision.ALLOW : PolicyDecision.ASK_USER, priority: PRIORITY_SUBAGENT_TOOL,