mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-07-27 22:38:30 +08:00
fix: handle non-user actors (e.g. Copilot) in permission and actor checks (#1144)
GitHub Apps like Copilot SWE Agent set GITHUB_ACTOR to a value (e.g. "Copilot") that is neither a valid GitHub user nor ends with "[bot]". This caused two independent crashes: 1. checkWritePermissions (permissions.ts): called the collaborator permission API which returns 404 "is not a user" for non-user actors. 2. checkHumanActor (actor.ts): called the Users API first, which 404s, before ever reaching the allowed_bots check. Fix both by: - Checking allowed_bots BEFORE making API calls, so known bots skip the API entirely. - In permissions.ts, catching "is not a user" 404 errors and falling back to the allowed_bots list instead of crashing. - In actor.ts, catching 404 errors and providing a clear error message telling the user to add the bot to allowed_bots. Closes #900, #903, #1018, #1133 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
1450f658d3
commit
9eb125afe3
@ -8,33 +8,12 @@
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
import type { GitHubContext } from "../context";
|
||||
|
||||
export async function checkHumanActor(
|
||||
octokit: Octokit,
|
||||
githubContext: GitHubContext,
|
||||
) {
|
||||
// Fetch user information from GitHub API
|
||||
const { data: userData } = await octokit.users.getByUsername({
|
||||
username: githubContext.actor,
|
||||
});
|
||||
function isAllowedBot(actor: string, allowedBots: string): boolean {
|
||||
const trimmed = allowedBots.trim();
|
||||
if (trimmed === "*") return true;
|
||||
if (!trimmed) return false;
|
||||
|
||||
const actorType = userData.type;
|
||||
|
||||
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
|
||||
const allowedList = trimmed
|
||||
.split(",")
|
||||
.map((bot) =>
|
||||
bot
|
||||
@ -44,17 +23,60 @@ export async function checkHumanActor(
|
||||
)
|
||||
.filter((bot) => bot.length > 0);
|
||||
|
||||
const botName = githubContext.actor.toLowerCase().replace(/\[bot\]$/, "");
|
||||
const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
|
||||
return allowedList.includes(normalizedActor);
|
||||
}
|
||||
|
||||
// Check if specific bot is allowed
|
||||
if (allowedBotsList.includes(botName)) {
|
||||
export async function checkHumanActor(
|
||||
octokit: Octokit,
|
||||
githubContext: GitHubContext,
|
||||
) {
|
||||
const allowedBots = githubContext.inputs.allowedBots;
|
||||
|
||||
// Check allowed_bots BEFORE calling the GitHub Users API.
|
||||
// Some bot actors (e.g. GitHub Copilot with GITHUB_ACTOR="Copilot") are
|
||||
// not resolvable via the Users API and would cause a 404 if we called it
|
||||
// first. By checking the allow-list early we avoid the unnecessary API
|
||||
// call and the resulting crash.
|
||||
if (isAllowedBot(githubContext.actor, allowedBots)) {
|
||||
console.log(
|
||||
`Bot ${botName} is in allowed list, skipping human actor check`,
|
||||
`Actor ${githubContext.actor} is in allowed_bots list, skipping human actor check`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Bot not allowed
|
||||
// Fetch user information from GitHub API
|
||||
let actorType: string;
|
||||
try {
|
||||
const { data: userData } = await octokit.users.getByUsername({
|
||||
username: githubContext.actor,
|
||||
});
|
||||
actorType = userData.type;
|
||||
} catch (error) {
|
||||
// Handle 404 for non-user actors (GitHub Apps whose GITHUB_ACTOR
|
||||
// doesn't match any user account, e.g. "Copilot").
|
||||
if (
|
||||
error instanceof Error &&
|
||||
(error.message.includes("Not Found") ||
|
||||
error.message.includes("is not a user"))
|
||||
) {
|
||||
const botName = githubContext.actor
|
||||
.toLowerCase()
|
||||
.replace(/\[bot\]$/, "");
|
||||
throw new Error(
|
||||
`Workflow initiated by non-human actor: ${botName} (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.`,
|
||||
);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
console.log(`Actor type: ${actorType}`);
|
||||
|
||||
// Check bot permissions if actor is not a User
|
||||
if (actorType !== "User") {
|
||||
const botName = githubContext.actor.toLowerCase().replace(/\[bot\]$/, "");
|
||||
|
||||
// Bot not allowed (we already checked allowed_bots above)
|
||||
throw new Error(
|
||||
`Workflow initiated by non-human actor: ${botName} (type: ${actorType}). Add bot to allowed_bots list or use '*' to allow all bots.`,
|
||||
);
|
||||
|
||||
@ -2,6 +2,28 @@ import * as core from "@actions/core";
|
||||
import type { ParsedGitHubContext } from "../context";
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
|
||||
/**
|
||||
* Check if a bot actor is in the allowed bots list.
|
||||
*/
|
||||
function isAllowedBot(actor: string, allowedBots: string): boolean {
|
||||
const trimmed = allowedBots.trim();
|
||||
if (trimmed === "*") return true;
|
||||
if (!trimmed) return false;
|
||||
|
||||
const allowedList = trimmed
|
||||
.split(",")
|
||||
.map((bot) =>
|
||||
bot
|
||||
.trim()
|
||||
.toLowerCase()
|
||||
.replace(/\[bot\]$/, ""),
|
||||
)
|
||||
.filter((bot) => bot.length > 0);
|
||||
|
||||
const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
|
||||
return allowedList.includes(normalizedActor);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the actor has write permissions to the repository
|
||||
* @param octokit - The Octokit REST client
|
||||
@ -17,6 +39,7 @@ export async function checkWritePermissions(
|
||||
githubTokenProvided?: boolean,
|
||||
): Promise<boolean> {
|
||||
const { repository, actor } = context;
|
||||
const allowedBots = context.inputs.allowedBots ?? "";
|
||||
|
||||
try {
|
||||
core.info(`Checking permissions for actor: ${actor}`);
|
||||
@ -43,12 +66,21 @@ export async function checkWritePermissions(
|
||||
}
|
||||
}
|
||||
|
||||
// Check if the actor is a GitHub App (bot user)
|
||||
// Check if the actor is a GitHub App (bot user with [bot] suffix)
|
||||
if (actor.endsWith("[bot]")) {
|
||||
core.info(`Actor is a GitHub App: ${actor}`);
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check if the actor is in the allowed bots list (handles non-[bot] actors
|
||||
// like GitHub Copilot whose GITHUB_ACTOR is "Copilot", not "Copilot[bot]")
|
||||
if (isAllowedBot(actor, allowedBots)) {
|
||||
core.info(
|
||||
`Actor ${actor} is in allowed_bots list, skipping permission check`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check permissions directly using the permission endpoint
|
||||
const response = await octokit.repos.getCollaboratorPermissionLevel({
|
||||
owner: repository.owner,
|
||||
@ -67,6 +99,28 @@ export async function checkWritePermissions(
|
||||
return false;
|
||||
}
|
||||
} catch (error) {
|
||||
// Handle 404 errors for non-user actors (e.g. GitHub Apps like Copilot
|
||||
// whose GITHUB_ACTOR doesn't end with [bot]).
|
||||
// The collaborator permission API only works for user accounts.
|
||||
if (
|
||||
error instanceof Error &&
|
||||
error.message.includes("is not a user")
|
||||
) {
|
||||
core.info(
|
||||
`Actor ${actor} is not a GitHub user (likely a GitHub App). Checking allowed_bots...`,
|
||||
);
|
||||
if (isAllowedBot(actor, allowedBots)) {
|
||||
core.info(
|
||||
`Non-user actor ${actor} is in allowed_bots list, granting access`,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
core.warning(
|
||||
`Non-user actor ${actor} is not in allowed_bots list. Add it to allowed_bots or use '*' to allow all bots.`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
core.error(`Failed to check permissions: ${error}`);
|
||||
throw new Error(`Failed to check permissions for ${actor}: ${error}`);
|
||||
}
|
||||
|
||||
@ -93,4 +93,78 @@ describe("checkHumanActor", () => {
|
||||
"Workflow initiated by non-human actor: other-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.",
|
||||
);
|
||||
});
|
||||
|
||||
describe("non-[bot] actors (e.g. GitHub Copilot)", () => {
|
||||
// GitHub Copilot SWE Agent sets GITHUB_ACTOR="Copilot" which is not a
|
||||
// valid GitHub user and doesn't end with [bot], causing 404 on the
|
||||
// Users API. These tests verify the fix handles this gracefully.
|
||||
|
||||
function createMockOctokitThat404s(): Octokit {
|
||||
return {
|
||||
users: {
|
||||
getByUsername: async () => {
|
||||
const err = new Error("Not Found");
|
||||
(err as any).status = 404;
|
||||
throw err;
|
||||
},
|
||||
},
|
||||
} as unknown as Octokit;
|
||||
}
|
||||
|
||||
test("should pass for non-[bot] actor when in allowed_bots list", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createMockContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "copilot,cursor";
|
||||
|
||||
// Should not even call the API — allowed_bots check happens first
|
||||
await expect(
|
||||
checkHumanActor(mockOctokit, context),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
test("should pass for non-[bot] actor when all bots are allowed", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createMockContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "*";
|
||||
|
||||
await expect(
|
||||
checkHumanActor(mockOctokit, context),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
test("should throw with clear message for non-[bot] actor that 404s and is not in allowed list", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createMockContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "cursor";
|
||||
|
||||
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
|
||||
"Workflow initiated by non-human actor: copilot (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.",
|
||||
);
|
||||
});
|
||||
|
||||
test("should throw with clear message for non-[bot] actor that 404s and allowed_bots is empty", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createMockContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "";
|
||||
|
||||
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
|
||||
"Workflow initiated by non-human actor: copilot (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.",
|
||||
);
|
||||
});
|
||||
|
||||
test("should match allowed_bots case-insensitively for non-[bot] actors", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createMockContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "COPILOT";
|
||||
|
||||
await expect(
|
||||
checkHumanActor(mockOctokit, context),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -303,4 +303,104 @@ describe("checkWritePermissions", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("non-[bot] actors (e.g. GitHub Copilot)", () => {
|
||||
// GitHub Copilot SWE Agent sets GITHUB_ACTOR="Copilot" which doesn't
|
||||
// end with [bot] and is not a valid GitHub user, so the collaborator
|
||||
// permission API returns 404 with "is not a user".
|
||||
|
||||
const createMockOctokitThat404s = () => ({
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
const err = new Error(
|
||||
"HttpError: Copilot is not a user - https://docs.github.com/rest/collaborators/collaborators#get-repository-permissions-for-a-user",
|
||||
);
|
||||
(err as any).status = 404;
|
||||
throw err;
|
||||
},
|
||||
},
|
||||
} as any);
|
||||
|
||||
test("should return true for non-[bot] actor in allowed_bots (pre-API check)", async () => {
|
||||
// The allowed_bots check should happen BEFORE calling the API,
|
||||
// so this should succeed even with a 404-ing mock.
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "copilot,cursor";
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(coreInfoSpy).toHaveBeenCalledWith(
|
||||
"Actor Copilot is in allowed_bots list, skipping permission check",
|
||||
);
|
||||
});
|
||||
|
||||
test("should return true for non-[bot] actor when allowed_bots is '*' (pre-API check)", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "*";
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return true for non-[bot] actor in allowed_bots via 404 fallback", async () => {
|
||||
// Even if somehow we reach the API call (e.g. race condition or
|
||||
// future refactor), the 404 catch path should also check allowed_bots.
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createContext();
|
||||
context.actor = "SomeNewBot";
|
||||
context.inputs.allowedBots = "somenewbot";
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return false for non-[bot] actor that 404s and is not in allowed_bots", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "cursor";
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(coreWarningSpy).toHaveBeenCalledWith(
|
||||
"Non-user actor Copilot is not in allowed_bots list. Add it to allowed_bots or use '*' to allow all bots.",
|
||||
);
|
||||
});
|
||||
|
||||
test("should return false for non-[bot] actor that 404s with empty allowed_bots", async () => {
|
||||
const mockOctokit = createMockOctokitThat404s();
|
||||
const context = createContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "";
|
||||
|
||||
const result = await checkWritePermissions(mockOctokit, context);
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test("should still throw for non-404 API errors", async () => {
|
||||
const mockOctokit = {
|
||||
repos: {
|
||||
getCollaboratorPermissionLevel: async () => {
|
||||
throw new Error("Internal Server Error");
|
||||
},
|
||||
},
|
||||
} as any;
|
||||
const context = createContext();
|
||||
context.actor = "Copilot";
|
||||
context.inputs.allowedBots = "";
|
||||
|
||||
await expect(checkWritePermissions(mockOctokit, context)).rejects.toThrow(
|
||||
"Failed to check permissions for Copilot",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user