Skip to content

Commit 04a581e

Browse files
Copilotlpcoxpelikhangithub-actions[bot]
authored
Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot) (#38152)
* Initial plan * Initial plan: fix bot actor handling in assertTrustedCheckoutRuntime Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * Fix checkout_pr_branch.cjs failing for bot/app actors (Copilot, dependabot) Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * Harden 404 collaborator fallback and revert unrelated lockfiles Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * chore: outline pr-finisher plan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Revert lockfile churn and tighten trusted actor checks Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Restore workflow lockfiles to main baseline Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 43eee91 commit 04a581e

2 files changed

Lines changed: 107 additions & 11 deletions

File tree

‎actions/setup/js/checkout_pr_branch.cjs‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,19 +129,48 @@ async function assertTrustedCheckoutRuntime() {
129129
throw new Error("Refusing PR checkout: unable to determine triggering actor");
130130
}
131131

132-
const { data: permissionData } = await github.rest.repos.getCollaboratorPermissionLevel({
133-
owner: context.repo.owner,
134-
repo: context.repo.repo,
135-
username: actor,
136-
});
137-
138-
const permission = permissionData?.permission || "none";
139-
const hasWriteOrHigher = TRUSTED_CHECKOUT_PERMISSIONS.includes(permission);
140-
if (!hasWriteOrHigher) {
141-
throw new Error(`Refusing PR checkout: actor '${actor}' has '${permission}' permission (requires write or higher)`);
132+
// Bot and app actors (e.g. Copilot, dependabot[bot]) are not regular GitHub
133+
// users and cannot be resolved via the collaborators API (returns 404).
134+
// Trust them implicitly: the non-fork repository check above already ensures
135+
// the workflow is running in a controlled context.
136+
const senderType = context.payload.sender?.type;
137+
if (senderType === "Bot") {
138+
core.info(`Runtime safety check passed for bot/app actor '${actor}' (sender type: ${senderType})`);
139+
return;
142140
}
143141

144-
core.info(`Runtime safety check passed for actor '${actor}' with '${permission}' permission`);
142+
try {
143+
const { data: permissionData } = await github.rest.repos.getCollaboratorPermissionLevel({
144+
owner: context.repo.owner,
145+
repo: context.repo.repo,
146+
username: actor,
147+
});
148+
149+
const permission = permissionData?.permission || "none";
150+
const hasWriteOrHigher = TRUSTED_CHECKOUT_PERMISSIONS.includes(permission);
151+
if (!hasWriteOrHigher) {
152+
throw new Error(`Refusing PR checkout: actor '${actor}' has '${permission}' permission (requires write or higher)`);
153+
}
154+
155+
core.info(`Runtime safety check passed for actor '${actor}' with '${permission}' permission`);
156+
} catch (err) {
157+
// A 404 here is ambiguous: it can indicate either a non-user app/bot actor
158+
// or a real user that is not a collaborator. Disambiguate via users API.
159+
// Real users resolve via users.getByUsername; app/bot actors return 404.
160+
if (err.status === 404) {
161+
try {
162+
await github.rest.users.getByUsername({ username: actor });
163+
throw new Error(`Refusing PR checkout: actor '${actor}' is not a collaborator (requires write or higher)`);
164+
} catch (userErr) {
165+
if (userErr.status === 404) {
166+
core.info(`Runtime safety check passed for app actor '${actor}' (not a regular user)`);
167+
return;
168+
}
169+
throw userErr;
170+
}
171+
}
172+
throw err;
173+
}
145174
}
146175

147176
async function main() {

‎actions/setup/js/checkout_pr_branch.test.cjs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ describe("checkout_pr_branch.cjs", () => {
8282
},
8383
}),
8484
},
85+
users: {
86+
getByUsername: vi.fn().mockResolvedValue({
87+
data: {
88+
login: "test-actor",
89+
},
90+
}),
91+
},
8592
pulls: {
8693
get: vi.fn().mockResolvedValue({
8794
data: {
@@ -257,6 +264,66 @@ If the pull request is still open, verify that:
257264
expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
258265
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("requires write or higher"));
259266
});
267+
268+
it("should allow checkout for Bot actor without calling the collaborator API", async () => {
269+
mockContext.actor = "Copilot";
270+
mockContext.payload.sender = { login: "Copilot", type: "Bot" };
271+
272+
await runScript();
273+
274+
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
275+
expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for bot/app actor 'Copilot' (sender type: Bot)");
276+
expect(mockCore.setFailed).not.toHaveBeenCalled();
277+
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
278+
expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
279+
});
280+
281+
it("should allow checkout when collaborator API returns 404 (app actor without sender type)", async () => {
282+
mockContext.actor = "Copilot";
283+
// No sender.type set — simulates an event payload without type info
284+
const notAUserError = Object.assign(new Error("Copilot is not a user"), { status: 404 });
285+
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(notAUserError);
286+
mockGithub.rest.users.getByUsername.mockRejectedValue(notAUserError);
287+
288+
await runScript();
289+
290+
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
291+
expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "Copilot" });
292+
expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for app actor 'Copilot' (not a regular user)");
293+
expect(mockCore.setFailed).not.toHaveBeenCalled();
294+
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
295+
expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
296+
});
297+
298+
it("should fail when collaborator API returns 404 for a regular non-collaborator user", async () => {
299+
mockContext.actor = "real-user";
300+
const notCollaboratorError = Object.assign(new Error("Not Found"), { status: 404 });
301+
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(notCollaboratorError);
302+
mockGithub.rest.users.getByUsername.mockResolvedValue({
303+
data: {
304+
login: "real-user",
305+
},
306+
});
307+
308+
await runScript();
309+
310+
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
311+
expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "real-user" });
312+
expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
313+
expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
314+
expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
315+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("is not a collaborator"));
316+
});
317+
318+
it("should fail when collaborator API returns a non-404 error", async () => {
319+
const serverError = Object.assign(new Error("Internal Server Error"), { status: 500 });
320+
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(serverError);
321+
322+
await runScript();
323+
324+
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
325+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Internal Server Error"));
326+
});
260327
});
261328

262329
it("should handle git fetch errors", async () => {

0 commit comments

Comments
 (0)