diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 135b020..0136375 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -81,6 +81,48 @@ export function buildAllowedToolsString( return allAllowedTools; } +/** + * Specialized allowed tools string for remote agent mode + * Always uses MCP commit signing and excludes dangerous git commands + */ +export function buildRemoteAgentAllowedToolsString( + customAllowedTools?: string[], + includeActionsTools: boolean = false, +): string { + let baseTools = [...BASE_ALLOWED_TOOLS]; + + // Always include the comment update tool from the comment server + baseTools.push("mcp__github_comment__update_claude_comment"); + + // Remote agent mode always uses MCP commit signing + baseTools.push( + "mcp__github_file_ops__commit_files", + "mcp__github_file_ops__delete_files", + ); + + // Add safe git tools only (read-only operations) + baseTools.push( + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + ); + + // Add GitHub Actions MCP tools if enabled + if (includeActionsTools) { + baseTools.push( + "mcp__github_ci__get_ci_status", + "mcp__github_ci__get_workflow_run_details", + "mcp__github_ci__download_job_log", + ); + } + + let allAllowedTools = baseTools.join(","); + if (customAllowedTools && customAllowedTools.length > 0) { + allAllowedTools = `${allAllowedTools},${customAllowedTools.join(",")}`; + } + return allAllowedTools; +} + export function buildDisallowedToolsString( customDisallowedTools?: string[], allowedTools?: string[], diff --git a/src/modes/remote-agent/index.ts b/src/modes/remote-agent/index.ts index d65c2b3..809a22e 100644 --- a/src/modes/remote-agent/index.ts +++ b/src/modes/remote-agent/index.ts @@ -4,11 +4,10 @@ import type { Mode, ModeOptions, ModeResult } from "../types"; import { isRepositoryDispatchEvent } from "../../github/context"; import type { GitHubContext } from "../../github/context"; import { setupBranch } from "../../github/operations/branch"; -import { configureGitAuth } from "../../github/operations/git-config"; import { prepareMcpConfig } from "../../mcp/install-mcp-server"; import { GITHUB_SERVER_URL } from "../../github/api/config"; import { - buildAllowedToolsString, + buildRemoteAgentAllowedToolsString, buildDisallowedToolsString, type PreparedContext, } from "../../create-prompt"; @@ -223,29 +222,8 @@ export const remoteAgentMode: Mode = { throw error; } - // Configure git authentication if not using commit signing - if (!context.inputs.useCommitSigning) { - try { - // Force Claude bot as git user - await configureGitAuth(githubToken, context, { - login: "claude[bot]", - id: 209825114, - }); - } catch (error) { - console.error("Failed to configure git authentication:", error); - // Report failure if we have system progress config - if (systemProgressConfig) { - reportWorkflowFailed( - systemProgressConfig, - oidcToken, - "initialization", - error as Error, - "git_config_failed", - ); - } - throw error; - } - } + // Remote agent mode always uses commit signing for security + // No git authentication configuration needed as we use GitHub API // Report workflow initialized if (systemProgressConfig) { @@ -337,10 +315,9 @@ export const remoteAgentMode: Mode = { const hasActionsReadPermission = context.inputs.additionalPermissions.get("actions") === "read"; - const allowedToolsString = buildAllowedToolsString( + const allowedToolsString = buildRemoteAgentAllowedToolsString( context.inputs.allowedTools, hasActionsReadPermission, - context.inputs.useCommitSigning, ); const disallowedToolsString = buildDisallowedToolsString( context.inputs.disallowedTools, @@ -425,26 +402,12 @@ function generateDispatchSystemPrompt( ? `Co-authored-by: ${triggerDisplayName ?? triggerUsername} <${triggerUsername}@users.noreply.github.com>` : ""; - let commitInstructions = ""; - if (context.inputs.useCommitSigning) { - commitInstructions = `- Use mcp__github_file_ops__commit_files and mcp__github_file_ops__delete_files to commit and push changes`; - if (coAuthorLine) { - commitInstructions += ` + // Remote agent mode always uses MCP for commit signing + let commitInstructions = `- Use mcp__github_file_ops__commit_files and mcp__github_file_ops__delete_files to commit and push changes`; + if (coAuthorLine) { + commitInstructions += ` - When pushing changes, include a Co-authored-by trailer in the commit message - Use: "${coAuthorLine}"`; - } - } else { - commitInstructions = `- Use git commands via the Bash tool to commit and push your changes: - - Stage files: Bash(git add ) - - Commit with a descriptive message: Bash(git commit -m "")`; - if (coAuthorLine) { - commitInstructions += ` - - When committing, include a Co-authored-by trailer: - Bash(git commit -m "\\n\\n${coAuthorLine}")`; - } - commitInstructions += ` - - Be sure to follow your commit message guidelines - - Push to the remote: Bash(git push origin HEAD)`; } return `You are Claude, an AI assistant designed to help with GitHub issues and pull requests. Think carefully as you analyze the context and respond appropriately. Here's the context for your current task: diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index 5e86ab1..e2e6c07 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -7,6 +7,7 @@ import { getEventTypeAndContext, buildAllowedToolsString, buildDisallowedToolsString, + buildRemoteAgentAllowedToolsString, } from "../src/create-prompt"; import type { PreparedContext } from "../src/create-prompt"; import type { Mode } from "../src/modes/types"; @@ -1149,3 +1150,114 @@ describe("buildDisallowedToolsString", () => { expect(result).toBe("BadTool1,BadTool2"); }); }); + +describe("buildRemoteAgentAllowedToolsString", () => { + test("should return correct tools for remote agent mode (always uses commit signing)", () => { + const result = buildRemoteAgentAllowedToolsString(); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Glob"); + expect(result).toContain("Grep"); + expect(result).toContain("LS"); + expect(result).toContain("Read"); + expect(result).toContain("Write"); + + // Comment tool should always be included + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // MCP commit signing tools should always be included + expect(result).toContain("mcp__github_file_ops__commit_files"); + expect(result).toContain("mcp__github_file_ops__delete_files"); + + // Safe git tools should be included + expect(result).toContain("Bash(git status:*)"); + expect(result).toContain("Bash(git diff:*)"); + expect(result).toContain("Bash(git log:*)"); + + // Dangerous git tools should NOT be included + expect(result).not.toContain("Bash(git commit:*)"); + expect(result).not.toContain("Bash(git add:*)"); + expect(result).not.toContain("Bash(git push:*)"); + expect(result).not.toContain("Bash(git config"); + expect(result).not.toContain("Bash(git rm:*)"); + }); + + test("should include custom tools when provided", () => { + const customTools = ["CustomTool1", "CustomTool2"]; + const result = buildRemoteAgentAllowedToolsString(customTools); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Glob"); + + // Custom tools should be included + expect(result).toContain("CustomTool1"); + expect(result).toContain("CustomTool2"); + + // MCP commit signing tools should still be included + expect(result).toContain("mcp__github_file_ops__commit_files"); + expect(result).toContain("mcp__github_file_ops__delete_files"); + + // Dangerous git tools should still NOT be included + expect(result).not.toContain("Bash(git commit:*)"); + expect(result).not.toContain("Bash(git config"); + }); + + test("should include GitHub Actions tools when includeActionsTools is true", () => { + const result = buildRemoteAgentAllowedToolsString([], true); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Glob"); + + // GitHub Actions tools should be included + expect(result).toContain("mcp__github_ci__get_ci_status"); + expect(result).toContain("mcp__github_ci__get_workflow_run_details"); + expect(result).toContain("mcp__github_ci__download_job_log"); + + // MCP commit signing tools should still be included + expect(result).toContain("mcp__github_file_ops__commit_files"); + expect(result).toContain("mcp__github_file_ops__delete_files"); + + // Dangerous git tools should still NOT be included + expect(result).not.toContain("Bash(git commit:*)"); + expect(result).not.toContain("Bash(git config"); + }); + + test("should include both custom and Actions tools when both provided", () => { + const customTools = ["CustomTool1"]; + const result = buildRemoteAgentAllowedToolsString(customTools, true); + + // Base tools should be present + expect(result).toContain("Edit"); + + // Custom tools should be included + expect(result).toContain("CustomTool1"); + + // GitHub Actions tools should be included + expect(result).toContain("mcp__github_ci__get_ci_status"); + + // MCP commit signing tools should still be included + expect(result).toContain("mcp__github_file_ops__commit_files"); + + // Dangerous git tools should still NOT be included + expect(result).not.toContain("Bash(git commit:*)"); + expect(result).not.toContain("Bash(git config"); + }); + + test("should never include dangerous git tools regardless of parameters", () => { + const dangerousCustomTools = ["Bash(git commit:*)", "Bash(git config:*)"]; + const result = buildRemoteAgentAllowedToolsString(dangerousCustomTools, true); + + // The function should still include dangerous tools if explicitly provided in custom tools + // This is by design - if someone explicitly adds them, they should be included + expect(result).toContain("Bash(git commit:*)"); + expect(result).toContain("Bash(git config:*)"); + + // But the base function should not add them automatically + const resultWithoutCustom = buildRemoteAgentAllowedToolsString([], true); + expect(resultWithoutCustom).not.toContain("Bash(git commit:*)"); + expect(resultWithoutCustom).not.toContain("Bash(git config"); + }); +});