Skip to content

Commit 9eb125a

Browse files
krislavtenclaude
krislavten
and
claude
authored
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 <[email protected]>
1 parent 1450f65 commit 9eb125a

4 files changed

Lines changed: 286 additions & 36 deletions

File tree

‎src/github/validation/actor.ts‎

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,53 +8,75 @@
88
import type { Octokit } from "@octokit/rest";
99
import type { GitHubContext } from "../context";
1010

11+
function isAllowedBot(actor: string, allowedBots: string): boolean {
12+
const trimmed = allowedBots.trim();
13+
if (trimmed === "*") return true;
14+
if (!trimmed) return false;
15+
16+
const allowedList = trimmed
17+
.split(",")
18+
.map((bot) =>
19+
bot
20+
.trim()
21+
.toLowerCase()
22+
.replace(/\[bot\]$/, ""),
23+
)
24+
.filter((bot) => bot.length > 0);
25+
26+
const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
27+
return allowedList.includes(normalizedActor);
28+
}
29+
1130
export async function checkHumanActor(
1231
octokit: Octokit,
1332
githubContext: GitHubContext,
1433
) {
15-
// Fetch user information from GitHub API
16-
const { data: userData } = await octokit.users.getByUsername({
17-
username: githubContext.actor,
18-
});
34+
const allowedBots = githubContext.inputs.allowedBots;
1935

20-
const actorType = userData.type;
21-
22-
console.log(`Actor type: ${actorType}`);
23-
24-
// Check bot permissions if actor is not a User
25-
if (actorType !== "User") {
26-
const allowedBots = githubContext.inputs.allowedBots;
36+
// Check allowed_bots BEFORE calling the GitHub Users API.
37+
// Some bot actors (e.g. GitHub Copilot with GITHUB_ACTOR="Copilot") are
38+
// not resolvable via the Users API and would cause a 404 if we called it
39+
// first. By checking the allow-list early we avoid the unnecessary API
40+
// call and the resulting crash.
41+
if (isAllowedBot(githubContext.actor, allowedBots)) {
42+
console.log(
43+
`Actor ${githubContext.actor} is in allowed_bots list, skipping human actor check`,
44+
);
45+
return;
46+
}
2747

28-
// Check if all bots are allowed
29-
if (allowedBots.trim() === "*") {
30-
console.log(
31-
`All bots are allowed, skipping human actor check for: ${githubContext.actor}`,
48+
// Fetch user information from GitHub API
49+
let actorType: string;
50+
try {
51+
const { data: userData } = await octokit.users.getByUsername({
52+
username: githubContext.actor,
53+
});
54+
actorType = userData.type;
55+
} catch (error) {
56+
// Handle 404 for non-user actors (GitHub Apps whose GITHUB_ACTOR
57+
// doesn't match any user account, e.g. "Copilot").
58+
if (
59+
error instanceof Error &&
60+
(error.message.includes("Not Found") ||
61+
error.message.includes("is not a user"))
62+
) {
63+
const botName = githubContext.actor
64+
.toLowerCase()
65+
.replace(/\[bot\]$/, "");
66+
throw new Error(
67+
`Workflow initiated by non-human actor: ${botName} (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.`,
3268
);
33-
return;
3469
}
70+
throw error;
71+
}
3572

36-
// Parse allowed bots list
37-
const allowedBotsList = allowedBots
38-
.split(",")
39-
.map((bot) =>
40-
bot
41-
.trim()
42-
.toLowerCase()
43-
.replace(/\[bot\]$/, ""),
44-
)
45-
.filter((bot) => bot.length > 0);
73+
console.log(`Actor type: ${actorType}`);
4674

75+
// Check bot permissions if actor is not a User
76+
if (actorType !== "User") {
4777
const botName = githubContext.actor.toLowerCase().replace(/\[bot\]$/, "");
4878

49-
// Check if specific bot is allowed
50-
if (allowedBotsList.includes(botName)) {
51-
console.log(
52-
`Bot ${botName} is in allowed list, skipping human actor check`,
53-
);
54-
return;
55-
}
56-
57-
// Bot not allowed
79+
// Bot not allowed (we already checked allowed_bots above)
5880
throw new Error(
5981
`Workflow initiated by non-human actor: ${botName} (type: ${actorType}). Add bot to allowed_bots list or use '*' to allow all bots.`,
6082
);

‎src/github/validation/permissions.ts‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,28 @@ import * as core from "@actions/core";
22
import type { ParsedGitHubContext } from "../context";
33
import type { Octokit } from "@octokit/rest";
44

5+
/**
6+
* Check if a bot actor is in the allowed bots list.
7+
*/
8+
function isAllowedBot(actor: string, allowedBots: string): boolean {
9+
const trimmed = allowedBots.trim();
10+
if (trimmed === "*") return true;
11+
if (!trimmed) return false;
12+
13+
const allowedList = trimmed
14+
.split(",")
15+
.map((bot) =>
16+
bot
17+
.trim()
18+
.toLowerCase()
19+
.replace(/\[bot\]$/, ""),
20+
)
21+
.filter((bot) => bot.length > 0);
22+
23+
const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
24+
return allowedList.includes(normalizedActor);
25+
}
26+
527
/**
628
* Check if the actor has write permissions to the repository
729
* @param octokit - The Octokit REST client
@@ -17,6 +39,7 @@ export async function checkWritePermissions(
1739
githubTokenProvided?: boolean,
1840
): Promise<boolean> {
1941
const { repository, actor } = context;
42+
const allowedBots = context.inputs.allowedBots ?? "";
2043

2144
try {
2245
core.info(`Checking permissions for actor: ${actor}`);
@@ -43,12 +66,21 @@ export async function checkWritePermissions(
4366
}
4467
}
4568

46-
// Check if the actor is a GitHub App (bot user)
69+
// Check if the actor is a GitHub App (bot user with [bot] suffix)
4770
if (actor.endsWith("[bot]")) {
4871
core.info(`Actor is a GitHub App: ${actor}`);
4972
return true;
5073
}
5174

75+
// Check if the actor is in the allowed bots list (handles non-[bot] actors
76+
// like GitHub Copilot whose GITHUB_ACTOR is "Copilot", not "Copilot[bot]")
77+
if (isAllowedBot(actor, allowedBots)) {
78+
core.info(
79+
`Actor ${actor} is in allowed_bots list, skipping permission check`,
80+
);
81+
return true;
82+
}
83+
5284
// Check permissions directly using the permission endpoint
5385
const response = await octokit.repos.getCollaboratorPermissionLevel({
5486
owner: repository.owner,
@@ -67,6 +99,28 @@ export async function checkWritePermissions(
6799
return false;
68100
}
69101
} catch (error) {
102+
// Handle 404 errors for non-user actors (e.g. GitHub Apps like Copilot
103+
// whose GITHUB_ACTOR doesn't end with [bot]).
104+
// The collaborator permission API only works for user accounts.
105+
if (
106+
error instanceof Error &&
107+
error.message.includes("is not a user")
108+
) {
109+
core.info(
110+
`Actor ${actor} is not a GitHub user (likely a GitHub App). Checking allowed_bots...`,
111+
);
112+
if (isAllowedBot(actor, allowedBots)) {
113+
core.info(
114+
`Non-user actor ${actor} is in allowed_bots list, granting access`,
115+
);
116+
return true;
117+
}
118+
core.warning(
119+
`Non-user actor ${actor} is not in allowed_bots list. Add it to allowed_bots or use '*' to allow all bots.`,
120+
);
121+
return false;
122+
}
123+
70124
core.error(`Failed to check permissions: ${error}`);
71125
throw new Error(`Failed to check permissions for ${actor}: ${error}`);
72126
}

‎test/actor.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,78 @@ describe("checkHumanActor", () => {
9393
"Workflow initiated by non-human actor: other-bot (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.",
9494
);
9595
});
96+
97+
describe("non-[bot] actors (e.g. GitHub Copilot)", () => {
98+
// GitHub Copilot SWE Agent sets GITHUB_ACTOR="Copilot" which is not a
99+
// valid GitHub user and doesn't end with [bot], causing 404 on the
100+
// Users API. These tests verify the fix handles this gracefully.
101+
102+
function createMockOctokitThat404s(): Octokit {
103+
return {
104+
users: {
105+
getByUsername: async () => {
106+
const err = new Error("Not Found");
107+
(err as any).status = 404;
108+
throw err;
109+
},
110+
},
111+
} as unknown as Octokit;
112+
}
113+
114+
test("should pass for non-[bot] actor when in allowed_bots list", async () => {
115+
const mockOctokit = createMockOctokitThat404s();
116+
const context = createMockContext();
117+
context.actor = "Copilot";
118+
context.inputs.allowedBots = "copilot,cursor";
119+
120+
// Should not even call the API — allowed_bots check happens first
121+
await expect(
122+
checkHumanActor(mockOctokit, context),
123+
).resolves.toBeUndefined();
124+
});
125+
126+
test("should pass for non-[bot] actor when all bots are allowed", async () => {
127+
const mockOctokit = createMockOctokitThat404s();
128+
const context = createMockContext();
129+
context.actor = "Copilot";
130+
context.inputs.allowedBots = "*";
131+
132+
await expect(
133+
checkHumanActor(mockOctokit, context),
134+
).resolves.toBeUndefined();
135+
});
136+
137+
test("should throw with clear message for non-[bot] actor that 404s and is not in allowed list", async () => {
138+
const mockOctokit = createMockOctokitThat404s();
139+
const context = createMockContext();
140+
context.actor = "Copilot";
141+
context.inputs.allowedBots = "cursor";
142+
143+
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
144+
"Workflow initiated by non-human actor: copilot (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.",
145+
);
146+
});
147+
148+
test("should throw with clear message for non-[bot] actor that 404s and allowed_bots is empty", async () => {
149+
const mockOctokit = createMockOctokitThat404s();
150+
const context = createMockContext();
151+
context.actor = "Copilot";
152+
context.inputs.allowedBots = "";
153+
154+
await expect(checkHumanActor(mockOctokit, context)).rejects.toThrow(
155+
"Workflow initiated by non-human actor: copilot (actor not found on GitHub). Add bot to allowed_bots list or use '*' to allow all bots.",
156+
);
157+
});
158+
159+
test("should match allowed_bots case-insensitively for non-[bot] actors", async () => {
160+
const mockOctokit = createMockOctokitThat404s();
161+
const context = createMockContext();
162+
context.actor = "Copilot";
163+
context.inputs.allowedBots = "COPILOT";
164+
165+
await expect(
166+
checkHumanActor(mockOctokit, context),
167+
).resolves.toBeUndefined();
168+
});
169+
});
96170
});

‎test/permissions.test.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,4 +303,104 @@ describe("checkWritePermissions", () => {
303303
);
304304
});
305305
});
306+
307+
describe("non-[bot] actors (e.g. GitHub Copilot)", () => {
308+
// GitHub Copilot SWE Agent sets GITHUB_ACTOR="Copilot" which doesn't
309+
// end with [bot] and is not a valid GitHub user, so the collaborator
310+
// permission API returns 404 with "is not a user".
311+
312+
const createMockOctokitThat404s = () => ({
313+
repos: {
314+
getCollaboratorPermissionLevel: async () => {
315+
const err = new Error(
316+
"HttpError: Copilot is not a user - https://docs.github.com/rest/collaborators/collaborators#get-repository-permissions-for-a-user",
317+
);
318+
(err as any).status = 404;
319+
throw err;
320+
},
321+
},
322+
} as any);
323+
324+
test("should return true for non-[bot] actor in allowed_bots (pre-API check)", async () => {
325+
// The allowed_bots check should happen BEFORE calling the API,
326+
// so this should succeed even with a 404-ing mock.
327+
const mockOctokit = createMockOctokitThat404s();
328+
const context = createContext();
329+
context.actor = "Copilot";
330+
context.inputs.allowedBots = "copilot,cursor";
331+
332+
const result = await checkWritePermissions(mockOctokit, context);
333+
334+
expect(result).toBe(true);
335+
expect(coreInfoSpy).toHaveBeenCalledWith(
336+
"Actor Copilot is in allowed_bots list, skipping permission check",
337+
);
338+
});
339+
340+
test("should return true for non-[bot] actor when allowed_bots is '*' (pre-API check)", async () => {
341+
const mockOctokit = createMockOctokitThat404s();
342+
const context = createContext();
343+
context.actor = "Copilot";
344+
context.inputs.allowedBots = "*";
345+
346+
const result = await checkWritePermissions(mockOctokit, context);
347+
348+
expect(result).toBe(true);
349+
});
350+
351+
test("should return true for non-[bot] actor in allowed_bots via 404 fallback", async () => {
352+
// Even if somehow we reach the API call (e.g. race condition or
353+
// future refactor), the 404 catch path should also check allowed_bots.
354+
const mockOctokit = createMockOctokitThat404s();
355+
const context = createContext();
356+
context.actor = "SomeNewBot";
357+
context.inputs.allowedBots = "somenewbot";
358+
359+
const result = await checkWritePermissions(mockOctokit, context);
360+
361+
expect(result).toBe(true);
362+
});
363+
364+
test("should return false for non-[bot] actor that 404s and is not in allowed_bots", async () => {
365+
const mockOctokit = createMockOctokitThat404s();
366+
const context = createContext();
367+
context.actor = "Copilot";
368+
context.inputs.allowedBots = "cursor";
369+
370+
const result = await checkWritePermissions(mockOctokit, context);
371+
372+
expect(result).toBe(false);
373+
expect(coreWarningSpy).toHaveBeenCalledWith(
374+
"Non-user actor Copilot is not in allowed_bots list. Add it to allowed_bots or use '*' to allow all bots.",
375+
);
376+
});
377+
378+
test("should return false for non-[bot] actor that 404s with empty allowed_bots", async () => {
379+
const mockOctokit = createMockOctokitThat404s();
380+
const context = createContext();
381+
context.actor = "Copilot";
382+
context.inputs.allowedBots = "";
383+
384+
const result = await checkWritePermissions(mockOctokit, context);
385+
386+
expect(result).toBe(false);
387+
});
388+
389+
test("should still throw for non-404 API errors", async () => {
390+
const mockOctokit = {
391+
repos: {
392+
getCollaboratorPermissionLevel: async () => {
393+
throw new Error("Internal Server Error");
394+
},
395+
},
396+
} as any;
397+
const context = createContext();
398+
context.actor = "Copilot";
399+
context.inputs.allowedBots = "";
400+
401+
await expect(checkWritePermissions(mockOctokit, context)).rejects.toThrow(
402+
"Failed to check permissions for Copilot",
403+
);
404+
});
405+
});
306406
});

0 commit comments

Comments
 (0)