diff --git a/ROADMAP.md b/ROADMAP.md index d9fd757..97f1b60 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -10,7 +10,7 @@ Thank you for trying out the beta of our GitHub Action! This document outlines o - **Support for workflow_dispatch and repository_dispatch events** - Dispatch Claude on events triggered via API from other workflows or from other services - **Ability to disable commit signing** - Option to turn off GPG signing for environments where it's not required. This will enable Claude to use normal `git` bash commands for committing. This will likely become the default behavior once added. - **Better code review behavior** - Support inline comments on specific lines, provide higher quality reviews with more actionable feedback -- **Support triggering @claude from bot users** - Allow automation and bot accounts to invoke Claude +- ~**Support triggering @claude from bot users** - Allow automation and bot accounts to invoke Claude~ - **Customizable base prompts** - Full control over Claude's initial context with template variables like `$PR_COMMENTS`, `$PR_FILES`, etc. Users can replace our default prompt entirely while still accessing key contextual data --- diff --git a/action.yml b/action.yml index 37db3b8..2b1dfdf 100644 --- a/action.yml +++ b/action.yml @@ -23,6 +23,10 @@ inputs: description: "The prefix to use for Claude branches (defaults to 'claude/', use 'claude-' for dash format)" required: false default: "claude/" + allowed_bots: + description: "Comma-separated list of allowed bot usernames, or '*' to allow all bots. Empty string (default) allows no bots." + required: false + default: "" # Claude Code configuration prompt: @@ -110,6 +114,7 @@ runs: BASE_BRANCH: ${{ inputs.base_branch }} BRANCH_PREFIX: ${{ inputs.branch_prefix }} OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} + ALLOWED_BOTS: ${{ inputs.allowed_bots }} GITHUB_RUN_ID: ${{ github.run_id }} USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }} DEFAULT_WORKFLOW_TOKEN: ${{ github.token }} @@ -125,7 +130,7 @@ runs: echo "Base-action dependencies installed" cd - # Install Claude Code globally - bun install -g @anthropic-ai/claude-code@1.0.70 + bun install -g @anthropic-ai/claude-code@1.0.71 - name: Setup Network Restrictions if: steps.prepare.outputs.contains_trigger == 'true' && inputs.experimental_allowed_domains != '' diff --git a/base-action/action.yml b/base-action/action.yml index 45e0051..9f8e927 100644 --- a/base-action/action.yml +++ b/base-action/action.yml @@ -85,7 +85,7 @@ runs: - name: Install Claude Code shell: bash - run: bun install -g @anthropic-ai/claude-code@1.0.70 + run: bun install -g @anthropic-ai/claude-code@1.0.71 - name: Run Claude Code Action shell: bash diff --git a/docs/security.md b/docs/security.md index e8e1b52..45ea4f2 100644 --- a/docs/security.md +++ b/docs/security.md @@ -3,7 +3,7 @@ ## Access Control - **Repository Access**: The action can only be triggered by users with write access to the repository -- **No Bot Triggers**: GitHub Apps and bots cannot trigger this action +- **Bot User Control**: By default, GitHub Apps and bots cannot trigger this action for security reasons. Use the `allowed_bots` parameter to enable specific bots or all bots - **Token Permissions**: The GitHub app receives only a short-lived token scoped specifically to the repository it's operating in - **No Cross-Repository Access**: Each action invocation is limited to the repository where it was triggered - **Limited Scope**: The token cannot access other repositories or perform actions beyond the configured permissions diff --git a/docs/usage.md b/docs/usage.md index 0d8ed42..7e77080 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -42,6 +42,8 @@ jobs: # Optional: grant additional permissions (requires corresponding GitHub token permissions) # additional_permissions: | # actions: read + # Optional: allow bot users to trigger the action + # allowed_bots: "dependabot[bot],renovate[bot]" ``` ## Inputs @@ -76,6 +78,7 @@ jobs: | `additional_permissions` | Additional permissions to enable. Currently supports 'actions: read' for viewing workflow results | No | "" | | `experimental_allowed_domains` | Restrict network access to these domains only (newline-separated). | No | "" | | `use_commit_signing` | Enable commit signing using GitHub's commit signature verification. When false, Claude uses standard git commands | No | `false` | +| `allowed_bots` | Comma-separated list of allowed bot usernames, or '\*' to allow all bots. Empty string (default) allows no bots | No | "" | \*Required when using direct Anthropic API (default and when not using Bedrock or Vertex) diff --git a/src/github/context.ts b/src/github/context.ts index 4ea8db5..3f0d876 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -69,6 +69,7 @@ type BaseContext = { branchPrefix: string; useStickyComment: boolean; useCommitSigning: boolean; + allowedBots: string; }; }; @@ -115,6 +116,7 @@ export function parseGitHubContext(): GitHubContext { branchPrefix: process.env.BRANCH_PREFIX ?? "claude/", useStickyComment: process.env.USE_STICKY_COMMENT === "true", useCommitSigning: process.env.USE_COMMIT_SIGNING === "true", + allowedBots: process.env.ALLOWED_BOTS ?? "", }, }; diff --git a/src/github/validation/actor.ts b/src/github/validation/actor.ts index c48764b..2599254 100644 --- a/src/github/validation/actor.ts +++ b/src/github/validation/actor.ts @@ -21,9 +21,42 @@ export async function checkHumanActor( console.log(`Actor type: ${actorType}`); + // Check bot permissions if actor is not a User if (actorType !== "User") { + const allowedBots = githubContext.inputs.allowedBots; + + // Check if all bots are allowed + if (allowedBots.trim() === "*") { + console.log( + `All bots are allowed, skipping human actor check for: ${githubContext.actor}`, + ); + return; + } + + // Parse allowed bots list + const allowedBotsList = allowedBots + .split(",") + .map((bot) => + bot + .trim() + .toLowerCase() + .replace(/\[bot\]$/, ""), + ) + .filter((bot) => bot.length > 0); + + const botName = githubContext.actor.toLowerCase().replace(/\[bot\]$/, ""); + + // Check if specific bot is allowed + if (allowedBotsList.includes(botName)) { + console.log( + `Bot ${botName} is in allowed list, skipping human actor check`, + ); + return; + } + + // Bot not allowed throw new Error( - `Workflow initiated by non-human actor: ${githubContext.actor} (type: ${actorType}).`, + `Workflow initiated by non-human actor: ${botName} (type: ${actorType}). Add bot to allowed_bots list or use '*' to allow all bots.`, ); } diff --git a/src/github/validation/permissions.ts b/src/github/validation/permissions.ts index d34e396..e571e3a 100644 --- a/src/github/validation/permissions.ts +++ b/src/github/validation/permissions.ts @@ -17,6 +17,12 @@ export async function checkWritePermissions( try { core.info(`Checking permissions for actor: ${actor}`); + // Check if the actor is a GitHub App (bot user) + if (actor.endsWith("[bot]")) { + core.info(`Actor is a GitHub App: ${actor}`); + return true; + } + // Check permissions directly using the permission endpoint const response = await octokit.repos.getCollaboratorPermissionLevel({ owner: repository.owner, diff --git a/src/mcp/github-inline-comment-server.ts b/src/mcp/github-inline-comment-server.ts index 28a8658..a432466 100644 --- a/src/mcp/github-inline-comment-server.ts +++ b/src/mcp/github-inline-comment-server.ts @@ -41,12 +41,14 @@ server.tool( ), line: z .number() + .nonnegative() .optional() .describe( "Line number for single-line comments (required if startLine is not provided)", ), startLine: z .number() + .nonnegative() .optional() .describe( "Start line for multi-line comments (use with line parameter for the end line)", diff --git a/test/actor.test.ts b/test/actor.test.ts new file mode 100644 index 0000000..4c9d206 --- /dev/null +++ b/test/actor.test.ts @@ -0,0 +1,96 @@ +#!/usr/bin/env bun + +import { describe, test, expect } from "bun:test"; +import { checkHumanActor } from "../src/github/validation/actor"; +import type { Octokit } from "@octokit/rest"; +import { createMockContext } from "./mockContext"; + +function createMockOctokit(userType: string): Octokit { + return { + users: { + getByUsername: async () => ({ + data: { + type: userType, + }, + }), + }, + } as unknown as Octokit; +} + +describe("checkHumanActor", () => { + test("should pass for human actor", async () => { + const mockOctokit = createMockOctokit("User"); + const context = createMockContext(); + context.actor = "human-user"; + + await expect( + checkHumanActor(mockOctokit, context), + ).resolves.toBeUndefined(); + }); + + test("should throw error for bot actor when not allowed", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "test-bot[bot]"; + context.inputs.allowedBots = ""; + + await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow( + "Workflow initiated by non-human actor: test-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.", + ); + }); + + test("should pass for bot actor when all bots allowed", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "test-bot[bot]"; + context.inputs.allowedBots = "*"; + + await expect( + checkHumanActor(mockOctokit, context), + ).resolves.toBeUndefined(); + }); + + test("should pass for specific bot when in allowed list", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "dependabot[bot]"; + context.inputs.allowedBots = "dependabot[bot],renovate[bot]"; + + await expect( + checkHumanActor(mockOctokit, context), + ).resolves.toBeUndefined(); + }); + + test("should pass for specific bot when in allowed list (without [bot])", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "dependabot[bot]"; + context.inputs.allowedBots = "dependabot,renovate"; + + await expect( + checkHumanActor(mockOctokit, context), + ).resolves.toBeUndefined(); + }); + + test("should throw error for bot not in allowed list", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "other-bot[bot]"; + context.inputs.allowedBots = "dependabot[bot],renovate[bot]"; + + await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow( + "Workflow initiated by non-human actor: other-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.", + ); + }); + + test("should throw error for bot not in allowed list (without [bot])", async () => { + const mockOctokit = createMockOctokit("Bot"); + const context = createMockContext(); + context.actor = "other-bot[bot]"; + context.inputs.allowedBots = "dependabot,renovate"; + + await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow( + "Workflow initiated by non-human actor: other-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.", + ); + }); +}); diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index e95ef51..2ec94f6 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -31,6 +31,7 @@ describe("prepareMcpConfig", () => { branchPrefix: "", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }; diff --git a/test/mockContext.ts b/test/mockContext.ts index 098c66d..d664aae 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -18,6 +18,7 @@ const defaultInputs = { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }; const defaultRepository = { diff --git a/test/permissions.test.ts b/test/permissions.test.ts index 4f2f9d0..67c53d3 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -67,6 +67,7 @@ describe("checkWritePermissions", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); @@ -120,6 +121,16 @@ describe("checkWritePermissions", () => { ); }); + test("should return true for bot user", async () => { + const mockOctokit = createMockOctokit("none"); + const context = createContext(); + context.actor = "test-bot[bot]"; + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + }); + test("should throw error when permission check fails", async () => { const error = new Error("API error"); const mockOctokit = { diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index 3bf00b2..36c41f2 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -35,6 +35,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -62,6 +63,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false); @@ -273,6 +275,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -301,6 +304,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -329,6 +333,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false);