Skip to content

Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot) #38152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 8 commits into from
Jun 9, 2026
+107 −11
Merged

Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot) #38152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2532767
Initial plan
Copilot Jun 9, 2026
810f393
Initial plan: fix bot actor handling in assertTrustedCheckoutRuntime
Copilot Jun 9, 2026
1a4212e
Fix checkout_pr_branch.cjs failing for bot/app actors (Copilot, depen…
Copilot Jun 9, 2026
fa841f0
Harden 404 collaborator fallback and revert unrelated lockfiles
Copilot Jun 9, 2026
d24fe0a
chore: outline pr-finisher plan
Copilot Jun 9, 2026
9fe5bb8
Merge branch 'main' into copilot/fix-checkout-pr-branch
github-actions[bot] Jun 9, 2026
272779d
Revert lockfile churn and tighten trusted actor checks
Copilot Jun 9, 2026
952e91f
Restore workflow lockfiles to main baseline
Copilot Jun 9, 2026
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
51 changes: 40 additions & 11 deletions actions/setup/js/checkout_pr_branch.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,48 @@ async function assertTrustedCheckoutRuntime() {
throw new Error("Refusing PR checkout: unable to determine triggering actor");
}

const { data: permissionData } = await github.rest.repos.getCollaboratorPermissionLevel({
owner: context.repo.owner,
repo: context.repo.repo,
username: actor,
});

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

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice — bypassing the collaborators API for Bot/Mannequin sender types is a clean guard. Consider a brief comment noting these are non-fork-verified contexts.


core.info(`Runtime safety check passed for actor '${actor}' with '${permission}' permission`);
try {
const { data: permissionData } = await github.rest.repos.getCollaboratorPermissionLevel({
owner: context.repo.owner,
repo: context.repo.repo,
username: actor,
});

const permission = permissionData?.permission || "none";
const hasWriteOrHigher = TRUSTED_CHECKOUT_PERMISSIONS.includes(permission);

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good defensive use of try/catch around getCollaboratorPermissionLevel — the 404 disambiguation via the users API is a thoughtful touch.

if (!hasWriteOrHigher) {
throw new Error(`Refusing PR checkout: actor '${actor}' has '${permission}' permission (requires write or higher)`);
}

core.info(`Runtime safety check passed for actor '${actor}' with '${permission}' permission`);
} catch (err) {
// A 404 here is ambiguous: it can indicate either a non-user app/bot actor
// or a real user that is not a collaborator. Disambiguate via users API.
// Real users resolve via users.getByUsername; app/bot actors return 404.
if (err.status === 404) {

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] This catch trusts any 404 from getCollaboratorPermissionLevel, including cases where sender.type is explicitly "User" and the API unexpectedly returns 404.

The security model relies on GitHub never returning 404 for real GitHub users (they receive permission: "none" instead). That assumption holds today but is not stated anywhere — a future API change or an edge case (e.g., a deleted account used as actor) would silently widen the trust surface.

💡 Suggestion: document the API contract assumption 
// GitHub’s collaborators API returns 404 only for non-user accounts (e.g. GitHub App
// actors like Copilot). Real GitHub users always receive a permission response, never
// a 404. Trusting 404 here is therefore safe within the already-verified non-fork context.
// Ref: https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user
if (err.status === 404) {

With this comment, future maintainers know exactly what assumption to revalidate if the API behaviour changes.

try {
await github.rest.users.getByUsername({ username: actor });
throw new Error(`Refusing PR checkout: actor '${actor}' is not a collaborator (requires write or higher)`);
} catch (userErr) {
if (userErr.status === 404) {
core.info(`Runtime safety check passed for app actor '${actor}' (not a regular user)`);
return;
}
throw userErr;
}
}
throw err;
Comment on lines +160 to +172
}
}

async function main() {
Expand Down
67 changes: 67 additions & 0 deletions actions/setup/js/checkout_pr_branch.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ describe("checkout_pr_branch.cjs", () => {
},
}),
},
users: {
getByUsername: vi.fn().mockResolvedValue({
data: {
login: "test-actor",
},
}),
},
pulls: {
get: vi.fn().mockResolvedValue({
data: {
Expand Down Expand Up @@ -257,6 +264,66 @@ If the pull request is still open, verify that:
expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("requires write or higher"));
});

it("should allow checkout for Bot actor without calling the collaborator API", async () => {
mockContext.actor = "Copilot";
mockContext.payload.sender = { login: "Copilot", type: "Bot" };

await runScript();

expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for bot/app actor 'Copilot' (sender type: Bot)");
expect(mockCore.setFailed).not.toHaveBeenCalled();
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
});

it("should allow checkout when collaborator API returns 404 (app actor without sender type)", async () => {
mockContext.actor = "Copilot";
// No sender.type set — simulates an event payload without type info
const notAUserError = Object.assign(new Error("Copilot is not a user"), { status: 404 });
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(notAUserError);
mockGithub.rest.users.getByUsername.mockRejectedValue(notAUserError);

await runScript();

expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "Copilot" });
expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for app actor 'Copilot' (not a regular user)");
expect(mockCore.setFailed).not.toHaveBeenCalled();
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
});

it("should fail when collaborator API returns 404 for a regular non-collaborator user", async () => {
mockContext.actor = "real-user";
const notCollaboratorError = Object.assign(new Error("Not Found"), { status: 404 });
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(notCollaboratorError);
mockGithub.rest.users.getByUsername.mockResolvedValue({
data: {
login: "real-user",
},
});

await runScript();

expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
expect(mockGithub.rest.users.getByUsername).toHaveBeenCalledWith({ username: "real-user" });
expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);
expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "false");
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("is not a collaborator"));
});

it("should fail when collaborator API returns a non-404 error", async () => {
const serverError = Object.assign(new Error("Internal Server Error"), { status: 500 });
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(serverError);

await runScript();

expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Internal Server Error"));

@github-actions github-actions Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The non-404 error test does not assert that getCollaboratorPermissionLevel was actually called.

The default mock context has no sender, so the code correctly falls into the try/catch — but adding an explicit assertion pins the execution path and guards against a future widening of the fast-path bypass (e.g. additional sender.type values) that would short-circuit before the collaborators API is ever called.

💡 Add 
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();

});
});

it("should handle git fetch errors", async () => {
Expand Down
Loading