diff --git a/.github/workflows/community-report.yml b/.github/workflows/community-report.yml index da2c4c2df9..86ac45aa94 100644 --- a/.github/workflows/community-report.yml +++ b/.github/workflows/community-report.yml @@ -183,12 +183,14 @@ jobs: use_gemini_code_assist: '${{ vars.GOOGLE_GENAI_USE_GCA }}' settings: |- { - "coreTools": [ - "run_shell_command(gh issue list)", - "run_shell_command(gh pr list)", - "run_shell_command(gh search issues)", - "run_shell_command(gh search prs)" - ] + "tools": { + "core": [ + "run_shell_command(gh issue list)", + "run_shell_command(gh pr list)", + "run_shell_command(gh search issues)", + "run_shell_command(gh search prs)" + ] + } } prompt: |- You are a helpful assistant that analyzes community contribution reports. diff --git a/.github/workflows/gemini-automated-issue-dedup.yml b/.github/workflows/gemini-automated-issue-dedup.yml index d837cd8836..ff64f76897 100644 --- a/.github/workflows/gemini-automated-issue-dedup.yml +++ b/.github/workflows/gemini-automated-issue-dedup.yml @@ -109,10 +109,12 @@ jobs: } }, "maxSessionTurns": 25, - "coreTools": [ - "run_shell_command(echo)", - "run_shell_command(gh issue view)" - ], + "tools": { + "core": [ + "run_shell_command(echo)", + "run_shell_command(gh issue view)" + ] + }, "telemetry": { "enabled": true, "target": "gcp" diff --git a/.github/workflows/gemini-automated-issue-triage.yml b/.github/workflows/gemini-automated-issue-triage.yml index bd9435d7bc..631c6498ee 100644 --- a/.github/workflows/gemini-automated-issue-triage.yml +++ b/.github/workflows/gemini-automated-issue-triage.yml @@ -170,10 +170,12 @@ jobs: "enabled": true, "target": "gcp" }, - "coreTools": [ - "run_shell_command(echo)", - "read_file" - ] + "tools": { + "core": [ + "run_shell_command(echo)", + "read_file" + ] + } } prompt: |- ## Role diff --git a/.github/workflows/gemini-scheduled-issue-dedup.yml b/.github/workflows/gemini-scheduled-issue-dedup.yml index 78601b6495..ef52be84fc 100644 --- a/.github/workflows/gemini-scheduled-issue-dedup.yml +++ b/.github/workflows/gemini-scheduled-issue-dedup.yml @@ -88,9 +88,11 @@ jobs: } }, "maxSessionTurns": 25, - "coreTools": [ - "run_shell_command(echo)" - ], + "tools": { + "core": [ + "run_shell_command(echo)" + ] + }, "telemetry": { "enabled": true, "target": "gcp" diff --git a/.github/workflows/gemini-scheduled-issue-triage.yml b/.github/workflows/gemini-scheduled-issue-triage.yml index 1aae5c8445..c65d0632b0 100644 --- a/.github/workflows/gemini-scheduled-issue-triage.yml +++ b/.github/workflows/gemini-scheduled-issue-triage.yml @@ -192,10 +192,12 @@ jobs: settings: |- { "maxSessionTurns": 25, - "coreTools": [ - "run_shell_command(echo)", - "read_file" - ], + "tools": { + "core": [ + "run_shell_command(echo)", + "read_file" + ] + }, "telemetry": { "enabled": false, "target": "gcp" @@ -315,12 +317,14 @@ jobs: settings: |- { "maxSessionTurns": 30, - "coreTools": [ - "run_shell_command(echo)", - "grep_search", - "glob", - "read_file" - ], + "tools": { + "core": [ + "run_shell_command(echo)", + "grep_search", + "glob", + "read_file" + ] + }, "telemetry": { "enabled": false, "target": "gcp" diff --git a/docs/cli/enterprise.md b/docs/cli/enterprise.md index 1222591fc9..215d2ee557 100644 --- a/docs/cli/enterprise.md +++ b/docs/cli/enterprise.md @@ -285,7 +285,7 @@ environment to a blocklist. > [!WARNING] > Blocklisting with `excludeTools` is less secure than -> allowlisting with `coreTools`, as it relies on blocking known-bad commands, +> allowlisting with `tools.core`, as it relies on blocking known-bad commands, > and clever users may find ways to bypass simple string-based blocks. > **Allowlisting is the recommended approach.** diff --git a/packages/a2a-server/src/config/config.test.ts b/packages/a2a-server/src/config/config.test.ts index d12bf534fb..69f8a8c00a 100644 --- a/packages/a2a-server/src/config/config.test.ts +++ b/packages/a2a-server/src/config/config.test.ts @@ -291,9 +291,8 @@ describe('loadConfig', () => { }); describe('policy engine configuration', () => { - it('should merge V1 and V2 tool settings into policySettings', async () => { + it('should map tool settings into policySettings', async () => { const settings: Settings = { - allowedTools: ['v1-allowed'], tools: { allowed: ['v2-allowed'], exclude: ['v2-exclude'], @@ -313,7 +312,7 @@ describe('loadConfig', () => { tools: { core: ['v2-core'], exclude: ['v2-exclude'], - allowed: ['v1-allowed'], + allowed: ['v2-allowed'], }, mcpServers: settings.mcpServers, policyPaths: settings.policyPaths, @@ -324,64 +323,9 @@ describe('loadConfig', () => { 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', () => { - it('should pass V1 allowedTools to Config properly', async () => { - const settings: Settings = { - allowedTools: ['shell', 'edit'], - }; - await loadConfig(settings, mockExtensionLoader, taskId); - expect(Config).toHaveBeenCalledWith( - expect.objectContaining({ - allowedTools: ['shell', 'edit'], - }), - ); - }); - it('should pass V2 tools.allowed to Config properly', async () => { const settings: Settings = { tools: { @@ -396,21 +340,6 @@ 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'], - }, - }; - await loadConfig(settings, mockExtensionLoader, taskId); - expect(Config).toHaveBeenCalledWith( - expect.objectContaining({ - allowedTools: ['v1-tool'], - }), - ); - }); - it('should pass enableAgents to Config constructor', async () => { const settings: Settings = { experimental: { diff --git a/packages/a2a-server/src/config/config.ts b/packages/a2a-server/src/config/config.ts index 3f937f0a5c..f4c0aa9b28 100644 --- a/packages/a2a-server/src/config/config.ts +++ b/packages/a2a-server/src/config/config.ts @@ -69,9 +69,9 @@ export async function loadConfig( 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, + core: settings.tools?.core, + exclude: settings.tools?.exclude, + allowed: settings.tools?.allowed, }, policyPaths: settings.policyPaths, adminPolicyPaths: settings.adminPolicyPaths, @@ -94,9 +94,9 @@ export async function loadConfig( debugMode: process.env['DEBUG'] === 'true' || false, question: '', // Not used in server mode directly like CLI - coreTools: settings.coreTools || settings.tools?.core || undefined, - excludeTools: settings.excludeTools || settings.tools?.exclude || undefined, - allowedTools: settings.allowedTools || settings.tools?.allowed || undefined, + coreTools: settings.tools?.core || undefined, + excludeTools: settings.tools?.exclude || undefined, + allowedTools: settings.tools?.allowed || undefined, showMemoryUsage: settings.showMemoryUsage || false, approvalMode, policyEngineConfig, diff --git a/packages/a2a-server/src/config/settings.test.ts b/packages/a2a-server/src/config/settings.test.ts index a51d39f328..25f058fd8d 100644 --- a/packages/a2a-server/src/config/settings.test.ts +++ b/packages/a2a-server/src/config/settings.test.ts @@ -94,7 +94,9 @@ describe('loadSettings', () => { it('should load other top-level settings correctly', () => { const settings = { showMemoryUsage: true, - coreTools: ['tool1', 'tool2'], + tools: { + core: ['tool1', 'tool2'], + }, mcpServers: { server1: { command: 'cmd', @@ -109,7 +111,7 @@ describe('loadSettings', () => { const result = loadSettings(mockWorkspaceDir); expect(result.showMemoryUsage).toBe(true); - expect(result.coreTools).toEqual(['tool1', 'tool2']); + expect(result.tools?.core).toEqual(['tool1', 'tool2']); expect(result.mcpServers).toHaveProperty('server1'); expect(result.fileFiltering?.respectGitIgnore).toBe(true); }); diff --git a/packages/a2a-server/src/config/settings.ts b/packages/a2a-server/src/config/settings.ts index 8bb44c56ca..b34b4566ac 100644 --- a/packages/a2a-server/src/config/settings.ts +++ b/packages/a2a-server/src/config/settings.ts @@ -27,9 +27,6 @@ export const USER_SETTINGS_PATH = path.join(USER_SETTINGS_DIR, 'settings.json'); // similar to how packages/cli/src/config/settings.ts handles it. export interface Settings { mcpServers?: Record; - coreTools?: string[]; - excludeTools?: string[]; - allowedTools?: string[]; tools?: { allowed?: string[]; exclude?: string[];