diff --git a/README.md b/README.md index 057b34bd3..7a5aca8f9 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,8 @@ jobs: # Optional: grant additional permissions (requires corresponding GitHub token permissions) # additional_permissions: | # actions: read + # Optional: allow specific bots to trigger Claude + # allowed_bots: "dependabot,renovate" ``` ## Inputs @@ -193,6 +195,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) @@ -799,7 +802,7 @@ Both AWS Bedrock and GCP Vertex AI require OIDC authentication. ### 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/ROADMAP.md b/ROADMAP.md index d9fd75797..97f1b60ef 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 708219af6..199210d89 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 model: @@ -144,6 +148,7 @@ runs: DIRECT_PROMPT: ${{ inputs.direct_prompt }} MCP_CONFIG: ${{ inputs.mcp_config }} OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} + ALLOWED_BOTS: ${{ inputs.allowed_bots }} GITHUB_RUN_ID: ${{ github.run_id }} USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }} ACTIONS_TOKEN: ${{ github.token }} diff --git a/src/github/context.ts b/src/github/context.ts index c156b547f..c43f2d62b 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -39,6 +39,7 @@ export type ParsedGitHubContext = { useStickyComment: boolean; additionalPermissions: Map; useCommitSigning: boolean; + allowedBots: string; }; }; @@ -70,6 +71,7 @@ export function parseGitHubContext(): ParsedGitHubContext { process.env.ADDITIONAL_PERMISSIONS ?? "", ), 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 c48764b92..4fe2bcbef 100644 --- a/src/github/validation/actor.ts +++ b/src/github/validation/actor.ts @@ -21,9 +21,35 @@ 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; + + // Parse allowed bots list + const allowedBotsList = allowedBots + .split(",") + .map((bot) => bot.trim().toLowerCase()) + .filter((bot) => bot.length > 0); + + // Check if all bots are allowed + if (allowedBots.trim() === "*") { + console.log( + `All bots are allowed, skipping human actor check for: ${githubContext.actor}`, + ); + return; + } + + // Check if specific bot is allowed + if (allowedBotsList.includes(githubContext.actor.toLowerCase())) { + console.log( + `Bot ${githubContext.actor} 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: ${githubContext.actor} (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 d34e3965c..e571e3a9d 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/test/actor.test.ts b/test/actor.test.ts new file mode 100644 index 000000000..677689d88 --- /dev/null +++ b/test/actor.test.ts @@ -0,0 +1,74 @@ +#!/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"; + 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"; + 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"; + 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"; + 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 3f14a6eab..79ef74b17 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -35,6 +35,7 @@ describe("prepareMcpConfig", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }; diff --git a/test/mockContext.ts b/test/mockContext.ts index d035afc57..76f7127d2 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -23,6 +23,7 @@ const defaultInputs = { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }; const defaultRepository = { diff --git a/test/permissions.test.ts b/test/permissions.test.ts index 7471acbea..c21c654ae 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -124,6 +124,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 eaaf83464..16f6244cc 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -39,6 +39,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -70,6 +71,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false); @@ -285,6 +287,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -317,6 +320,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -349,6 +353,7 @@ describe("checkContainsTrigger", () => { useStickyComment: false, additionalPermissions: new Map(), useCommitSigning: false, + allowedBots: "", }, }); expect(checkContainsTrigger(context)).toBe(false);