Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 57 additions & 35 deletions src/github/validation/actor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,53 +8,75 @@
import type { Octokit } from "@octokit/rest";
import type { GitHubContext } from "../context";

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);
}

export async function checkHumanActor(
octokit: Octokit,
githubContext: GitHubContext,
) {
// Fetch user information from GitHub API
const { data: userData } = await octokit.users.getByUsername({
username: githubContext.actor,
});
const allowedBots = githubContext.inputs.allowedBots;

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 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(
`Actor ${githubContext.actor} is in allowed_bots list, skipping human actor check`,
);
return;
}

// Check if all bots are allowed
if (allowedBots.trim() === "*") {
console.log(
`All bots are allowed, skipping human actor check for: ${githubContext.actor}`,
// 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.`,
);
return;
}
throw error;
}

// Parse allowed bots list
const allowedBotsList = allowedBots
.split(",")
.map((bot) =>
bot
.trim()
.toLowerCase()
.replace(/\[bot\]$/, ""),
)
.filter((bot) => bot.length > 0);
console.log(`Actor type: ${actorType}`);

// Check bot permissions if actor is not a User
if (actorType !== "User") {
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
// 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.`,
);
Expand Down
56 changes: 55 additions & 1 deletion src/github/validation/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}`);
Expand All @@ -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,
Expand All @@ -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}`);
}
Expand Down
74 changes: 74 additions & 0 deletions test/actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
100 changes: 100 additions & 0 deletions test/permissions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
});
});
});