diff --git a/docs/security.md b/docs/security.md index 7a07dea6..47327fa0 100644 --- a/docs/security.md +++ b/docs/security.md @@ -2,7 +2,7 @@ ## Access Control -- **Repository Access**: The action can only be triggered by users with write access to the repository +- **Repository Access**: The action can only be triggered by users with write access to the repository. This is checked for issue, pull request, comment, and review events, and for `workflow_run` events, where both the workflow actor and the actor that started the upstream run are checked. `workflow_dispatch`, `repository_dispatch`, and `schedule` events are not checked separately — GitHub itself requires write access to dispatch a workflow, and scheduled runs have no external actor. - **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 - **⚠️ Allowed bots are not checked for repository permissions.** A bot that matches an entry does **not** need to be installed on your repository or have write access. On a **public repository**, external parties — including GitHub Apps created by anyone — may be able to trigger workflow events such as opening issues, commenting, or reviewing pull requests. If your workflow listens on those events and `allowed_bots` is set to `'*'`, any such App can invoke this action with a prompt it controls. - Prefer an explicit list over `'*'` @@ -22,6 +22,8 @@ ## Using this action with `pull_request_target` or `workflow_run` +For `workflow_run` events, the action checks the repository access of the actor that started the upstream run (for example, the author of the fork pull request that triggered your CI workflow) in addition to the workflow actor. If that actor does not have write access, the action stops before running Claude. To run on `workflow_run` events downstream of pull requests from contributors without write access, add those users to `allowed_non_write_users` and pass `github_token: ${{ secrets.GITHUB_TOKEN }}` — see the notes on that input above and keep the workflow's permissions minimal. + `pull_request_target` and `workflow_run` execute with the **base repository's secrets**. If your workflow checks out the PR head (`ref: ${{ github.event.pull_request.head.sha }}` for `pull_request_target`, `ref: ${{ github.event.workflow_run.head_sha }}` for `workflow_run`) into `$GITHUB_WORKSPACE` before this action, the action and Claude run with that checkout as the working directory. **Do not check out an untrusted ref into the workspace root before this action.** Use one of these patterns instead: diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index a0b0aad6..553bdb66 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -9,7 +9,11 @@ import * as core from "@actions/core"; import { setupGitHubToken } from "../github/token"; import { checkWritePermissions } from "../github/validation/permissions"; import { createOctokit } from "../github/api/client"; -import { parseGitHubContext, isEntityContext } from "../github/context"; +import { + parseGitHubContext, + isEntityContext, + isWorkflowRunEvent, +} from "../github/context"; import { detectMode } from "../modes/detector"; import { prepareTagMode } from "../modes/tag"; import { prepareAgentMode } from "../modes/agent"; @@ -33,8 +37,8 @@ async function run() { const githubToken = await setupGitHubToken(); const octokit = createOctokit(githubToken); - // Step 3: Check write permissions (only for entity contexts) - if (isEntityContext(context)) { + // Step 3: Check write permissions (entity contexts and workflow_run) + if (isEntityContext(context) || isWorkflowRunEvent(context)) { // Check if github_token was provided as input (not from app) const githubTokenProvided = !!process.env.OVERRIDE_GITHUB_TOKEN; const hasWritePermissions = await checkWritePermissions( diff --git a/src/entrypoints/run.ts b/src/entrypoints/run.ts index e28e8778..179df917 100644 --- a/src/entrypoints/run.ts +++ b/src/entrypoints/run.ts @@ -21,6 +21,7 @@ import { isPullRequestEvent, isPullRequestReviewEvent, isPullRequestReviewCommentEvent, + isWorkflowRunEvent, } from "../github/context"; import type { GitHubContext } from "../github/context"; import { detectMode } from "../modes/detector"; @@ -185,8 +186,10 @@ async function run() { process.env.GITHUB_TOKEN = githubToken; process.env.GH_TOKEN = githubToken; - // Check write permissions (only for entity contexts) - if (isEntityContext(context)) { + // Check write permissions for entity contexts, and for workflow_run + // events, whose upstream run may have been started by an actor without + // write access (e.g. the author of a fork pull request) + if (isEntityContext(context) || isWorkflowRunEvent(context)) { const hasWritePermissions = await checkWritePermissions( octokit.rest, context, diff --git a/src/github/context.ts b/src/github/context.ts index eeefb998..9826b0cc 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -282,6 +282,12 @@ export function isPullRequestReviewCommentEvent( return context.eventName === "pull_request_review_comment"; } +export function isWorkflowRunEvent( + context: GitHubContext, +): context is AutomationContext & { payload: WorkflowRunEvent } { + return context.eventName === "workflow_run"; +} + export function isIssuesAssignedEvent( context: GitHubContext, ): context is ParsedGitHubContext & { payload: IssuesAssignedEvent } { diff --git a/src/github/validation/permissions.ts b/src/github/validation/permissions.ts index 9b6600a3..6fd85515 100644 --- a/src/github/validation/permissions.ts +++ b/src/github/validation/permissions.ts @@ -1,5 +1,5 @@ import * as core from "@actions/core"; -import type { ParsedGitHubContext } from "../context"; +import { isWorkflowRunEvent, type GitHubContext } from "../context"; import type { Octokit } from "@octokit/rest"; /** @@ -24,6 +24,28 @@ function isAllowedBot(actor: string, allowedBots: string): boolean { return allowedList.includes(normalizedActor); } +/** + * Collect the actors whose repository access should be checked. This is + * normally just the workflow actor (GITHUB_ACTOR). For workflow_run events + * the actor that started the upstream run is checked as well when it + * differs, since that is the account the run originates from. + */ +function getActorsToCheck(context: GitHubContext): string[] { + const actors = [context.actor]; + + if (isWorkflowRunEvent(context)) { + const runActor = context.payload.workflow_run?.actor?.login; + if (runActor && !actors.includes(runActor)) { + core.info( + `workflow_run was started by ${runActor}; checking permissions for that actor as well`, + ); + actors.push(runActor); + } + } + + return actors; +} + /** * Check if the actor has write permissions to the repository * @param octokit - The Octokit REST client @@ -34,11 +56,31 @@ function isAllowedBot(actor: string, allowedBots: string): boolean { */ export async function checkWritePermissions( octokit: Octokit, - context: ParsedGitHubContext, + context: GitHubContext, allowedNonWriteUsers?: string, githubTokenProvided?: boolean, ): Promise { - const { repository, actor } = context; + for (const actor of getActorsToCheck(context)) { + const allowed = await checkActorWritePermissions( + octokit, + context, + actor, + allowedNonWriteUsers, + githubTokenProvided, + ); + if (!allowed) return false; + } + return true; +} + +async function checkActorWritePermissions( + octokit: Octokit, + context: GitHubContext, + actor: string, + allowedNonWriteUsers?: string, + githubTokenProvided?: boolean, +): Promise { + const { repository } = context; const allowedBots = context.inputs.allowedBots ?? ""; try { diff --git a/test/github-context.test.ts b/test/github-context.test.ts index 40870f8f..caf12d97 100644 --- a/test/github-context.test.ts +++ b/test/github-context.test.ts @@ -33,6 +33,7 @@ import { isIssuesAssignedEvent, isEntityContext, isAutomationContext, + isWorkflowRunEvent, } from "../src/github/context"; import { CLAUDE_APP_BOT_ID, CLAUDE_BOT_LOGIN } from "../src/github/constants"; import { createMockContext, createMockAutomationContext } from "./mockContext"; @@ -517,4 +518,14 @@ describe("type guards", () => { ).toBe(true); expect(isAutomationContext(issuesContext)).toBe(false); }); + + test("isWorkflowRunEvent accepts only workflow_run", () => { + expect( + isWorkflowRunEvent( + createMockAutomationContext({ eventName: "workflow_run" }), + ), + ).toBe(true); + expect(isWorkflowRunEvent(workflowDispatchContext)).toBe(false); + expect(isWorkflowRunEvent(issuesContext)).toBe(false); + }); }); diff --git a/test/permissions.test.ts b/test/permissions.test.ts index f89a7716..95a1ef4c 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -3,6 +3,7 @@ import * as core from "@actions/core"; import { checkWritePermissions } from "../src/github/validation/permissions"; import type { ParsedGitHubContext } from "../src/github/context"; import { CLAUDE_APP_BOT_ID, CLAUDE_BOT_LOGIN } from "../src/github/constants"; +import { createMockAutomationContext } from "./mockContext"; describe("checkWritePermissions", () => { let coreInfoSpy: any; @@ -455,4 +456,159 @@ describe("checkWritePermissions", () => { expect(result).toBe(true); }); }); + + describe("workflow_run contexts", () => { + const createWorkflowRunContext = ( + actor: string, + runActor: string = actor, + ) => + createMockAutomationContext({ + eventName: "workflow_run", + eventAction: "completed", + actor, + payload: { + action: "completed", + workflow_run: { + id: 123, + event: "pull_request", + actor: { login: runActor }, + head_repository: { full_name: "fork-owner/test-repo" }, + }, + } as any, + }); + + const createMockOctokitWithLevels = (levels: Record) => + ({ + repos: { + getCollaboratorPermissionLevel: async (params: { + username: string; + }) => ({ + data: { permission: levels[params.username] ?? "none" }, + }), + }, + }) as any; + + test("should return false when the run actor lacks write access", async () => { + const mockOctokit = createMockOctokit("read"); + const context = createWorkflowRunContext("fork-contributor"); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(false); + expect(coreWarningSpy).toHaveBeenCalledWith( + "Actor has insufficient permissions: read", + ); + }); + + test("should return true when the run actor has write access", async () => { + const mockOctokit = createMockOctokit("write"); + const context = createWorkflowRunContext("maintainer"); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + }); + + test("should return true when the run actor has admin access", async () => { + const mockOctokit = createMockOctokit("admin"); + const context = createWorkflowRunContext("maintainer"); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + }); + + test("should also check the payload run actor when it differs from the workflow actor", async () => { + const mockOctokit = createMockOctokitWithLevels({ + maintainer: "write", + "fork-contributor": "read", + }); + const context = createWorkflowRunContext( + "maintainer", + "fork-contributor", + ); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(false); + expect(coreInfoSpy).toHaveBeenCalledWith( + "workflow_run was started by fork-contributor; checking permissions for that actor as well", + ); + }); + + test("should return true when both the workflow actor and run actor have write access", async () => { + const mockOctokit = createMockOctokitWithLevels({ + maintainer: "write", + "other-maintainer": "admin", + }); + const context = createWorkflowRunContext( + "maintainer", + "other-maintainer", + ); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + }); + + test("should allow a run actor listed in allowed_non_write_users when github_token is provided", async () => { + const mockOctokit = createMockOctokit("read"); + const context = createWorkflowRunContext("fork-contributor"); + + const result = await checkWritePermissions( + mockOctokit, + context, + "fork-contributor,other-user", + true, + ); + + expect(result).toBe(true); + expect(coreWarningSpy).toHaveBeenCalledWith( + "⚠️ SECURITY WARNING: Bypassing write permission check for fork-contributor due to allowed_non_write_users configuration. This should only be used for workflows with very limited permissions.", + ); + }); + + test("should NOT bypass for a run actor in allowed_non_write_users when github_token is not provided", async () => { + const mockOctokit = createMockOctokit("read"); + const context = createWorkflowRunContext("fork-contributor"); + + const result = await checkWritePermissions( + mockOctokit, + context, + "fork-contributor", + false, + ); + + expect(result).toBe(false); + expect(coreWarningSpy).toHaveBeenCalledWith( + "Actor has insufficient permissions: read", + ); + }); + + test("should require the payload run actor to also be in allowed_non_write_users", async () => { + const mockOctokit = createMockOctokit("read"); + const context = createWorkflowRunContext( + "maintainer", + "fork-contributor", + ); + + const result = await checkWritePermissions( + mockOctokit, + context, + "maintainer", + true, + ); + + expect(result).toBe(false); + }); + + test("should return true for [bot] run actors", async () => { + const mockOctokit = createMockOctokit("none"); + const context = createWorkflowRunContext("dependabot[bot]"); + + const result = await checkWritePermissions(mockOctokit, context); + + expect(result).toBe(true); + }); + }); });