From d1ebbc7b738cecb7839fb228a79135a6edadd9f0 Mon Sep 17 00:00:00 2001 From: Maxwell Calkin Date: Sun, 8 Mar 2026 14:26:17 -0400 Subject: [PATCH] fix: respect --allowedTools when building disallowed tools list In agent mode, WebSearch and WebFetch are now properly disabled by default via --disallowedTools, consistent with tag mode's security posture. When users explicitly allow these tools via --allowedTools in claude_args, they are removed from the disallowed list. Fixes #690 Co-Authored-By: Claude Opus 4.6 --- src/modes/agent/index.ts | 9 ++++ test/modes/agent.test.ts | 108 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index e6047379c..f10ada784 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -1,6 +1,7 @@ import { mkdir, writeFile } from "fs/promises"; import { prepareMcpConfig } from "../../mcp/install-mcp-server"; import { parseAllowedTools } from "./parse-tools"; +import { buildDisallowedToolsString } from "../../create-prompt"; import { configureGitAuth, setupSshSigning, @@ -118,6 +119,14 @@ export async function prepareAgentMode({ claudeArgs = `--mcp-config '${escapedOurConfig}'`; } + // Disable WebSearch and WebFetch by default for security, but respect + // user's --allowedTools from claude_args (e.g., --allowedTools WebSearch + // removes WebSearch from the disallowed list) + const disallowedTools = buildDisallowedToolsString([], allowedTools); + if (disallowedTools) { + claudeArgs = `${claudeArgs} --disallowedTools "${disallowedTools}"`; + } + // Append user's claude_args (which may have more --mcp-config flags) claudeArgs = `${claudeArgs} ${userClaudeArgs}`.trim(); diff --git a/test/modes/agent.test.ts b/test/modes/agent.test.ts index be6f0d2b1..85b65ed1b 100644 --- a/test/modes/agent.test.ts +++ b/test/modes/agent.test.ts @@ -84,8 +84,11 @@ describe("Agent Mode", () => { githubToken: "test-token", }); - // Verify claude_args includes user args (no MCP config in agent mode without allowed tools) - expect(result.claudeArgs).toBe("--model claude-sonnet-4 --max-turns 10"); + // Verify claude_args includes disallowed tools and user args + expect(result.claudeArgs).toContain("--disallowedTools"); + expect(result.claudeArgs).toContain("WebSearch"); + expect(result.claudeArgs).toContain("WebFetch"); + expect(result.claudeArgs).toContain("--model claude-sonnet-4 --max-turns 10"); expect(result.claudeArgs).not.toContain("--mcp-config"); // Verify return structure - should use "main" as fallback when no env vars set @@ -97,7 +100,7 @@ describe("Agent Mode", () => { claudeBranch: undefined, }, mcpConfig: expect.any(String), - claudeArgs: "--model claude-sonnet-4 --max-turns 10", + claudeArgs: expect.stringContaining("--disallowedTools"), }); // Clean up @@ -203,4 +206,103 @@ describe("Agent Mode", () => { // Should be empty or just whitespace when no MCP servers are included expect(result.claudeArgs).not.toContain("--mcp-config"); }); + + + test("--allowedTools WebSearch removes WebSearch from disallowed list", async () => { + const context = createMockAutomationContext({ + eventName: "workflow_dispatch", + }); + + const originalHeadRef = process.env.GITHUB_HEAD_REF; + const originalRefName = process.env.GITHUB_REF_NAME; + delete process.env.GITHUB_HEAD_REF; + delete process.env.GITHUB_REF_NAME; + + // Set CLAUDE_ARGS with --allowedTools WebSearch + process.env.CLAUDE_ARGS = '--allowedTools "WebSearch"'; + + const mockOctokit = { + rest: { + users: { + getAuthenticated: mock(() => + Promise.resolve({ + data: { login: "test-user", id: 12345, type: "User" }, + }), + ), + getByUsername: mock(() => + Promise.resolve({ + data: { login: "test-user", id: 12345, type: "User" }, + }), + ), + }, + }, + } as any; + + const result = await prepareAgentMode({ + context, + octokit: mockOctokit, + githubToken: "test-token", + }); + + // WebSearch should NOT be in disallowed tools since user explicitly allowed it + // WebFetch should still be disallowed + expect(result.claudeArgs).toContain("--disallowedTools"); + expect(result.claudeArgs).not.toMatch(/--disallowedTools[^"]*WebSearch/); + expect(result.claudeArgs).toContain("WebFetch"); + + // Clean up + delete process.env.CLAUDE_ARGS; + if (originalHeadRef !== undefined) + process.env.GITHUB_HEAD_REF = originalHeadRef; + if (originalRefName !== undefined) + process.env.GITHUB_REF_NAME = originalRefName; + }); + + test("--allowedTools WebSearch,WebFetch removes both from disallowed list", async () => { + const context = createMockAutomationContext({ + eventName: "workflow_dispatch", + }); + + const originalHeadRef = process.env.GITHUB_HEAD_REF; + const originalRefName = process.env.GITHUB_REF_NAME; + delete process.env.GITHUB_HEAD_REF; + delete process.env.GITHUB_REF_NAME; + + // Set CLAUDE_ARGS with both tools allowed + process.env.CLAUDE_ARGS = '--allowedTools "WebSearch,WebFetch"'; + + const mockOctokit = { + rest: { + users: { + getAuthenticated: mock(() => + Promise.resolve({ + data: { login: "test-user", id: 12345, type: "User" }, + }), + ), + getByUsername: mock(() => + Promise.resolve({ + data: { login: "test-user", id: 12345, type: "User" }, + }), + ), + }, + }, + } as any; + + const result = await prepareAgentMode({ + context, + octokit: mockOctokit, + githubToken: "test-token", + }); + + // Neither WebSearch nor WebFetch should be in disallowed tools + // So --disallowedTools should not be present at all + expect(result.claudeArgs).not.toContain("--disallowedTools"); + + // Clean up + delete process.env.CLAUDE_ARGS; + if (originalHeadRef !== undefined) + process.env.GITHUB_HEAD_REF = originalHeadRef; + if (originalRefName !== undefined) + process.env.GITHUB_REF_NAME = originalRefName; + }); });