Skip to content

Allow Copilot coding agents through the bot allowlist without collaborator lookup #67238

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 5 commits into from
Oct 10, 2026
Merged

Allow Copilot coding agents through the bot allowlist without collaborator lookup #67238

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70cdf3b
Initial plan
Copilot Oct 9, 2026
f7d2328
Authorize allowlisted built-in Copilot agents without collaborator lo…
Copilot Oct 9, 2026
6591aad
Merge remote-tracking branch 'origin/main' into copilot/fix-copilot-b…
Copilot Oct 9, 2026
fb33cbd
Authorize canonical Copilot bot aliases safely
Copilot Oct 9, 2026
7118ce0
Merge remote-tracking branch 'origin/main' into copilot/fix-copilot-b…
Copilot Oct 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
8 changes: 5 additions & 3 deletions actions/setup/js/check_membership.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ async function checkBotAllowlistAuthorization(actorToValidate, allowedBots, owne

core.info(`Actor '${actorToValidate}' matched the allowed bots list: ${allowedBots.join(", ")}`);

// Verify the bot is active/installed on the repository
const botStatus = await checkBotStatus(actorToValidate, owner, repo);
// Built-in Copilot agents are not repository collaborators; the explicit allowlist
// is sufficient for their GitHub-provided actor identities.
const isBuiltInCopilot = actorToValidate === "Copilot" || actorToValidate === "copilot-swe-agent" || actorToValidate === "copilot-swe-agent[bot]";

@github-actions github-actions Bot Oct 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.

This bypass only recognizes three hard-coded actor strings, so supported Copilot aliases like copilot and @app/copilot-swe-agent still fall through to checkBotStatus() and get rejected as bot_not_active.

💡 Why this blocks the fix

The compiler already expands on.bots: [copilot] to the full Copilot family (copilot-swe-agent, Copilot, copilot, @app/copilot-swe-agent). With the current check, only Copilot, copilot-swe-agent, and copilot-swe-agent[bot] bypass the collaborator lookup, so two canonical identities that the product explicitly emits remain broken.

Please derive this exception from the same normalized Copilot alias set the compiler uses, or at least include every expanded identifier after stripping an optional [bot] suffix, so the auth path cannot drift again.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fb33cbd: the membership check now recognizes every compiler-supplied Copilot alias after [bot] normalization and only allows a conclusive collaborator 404. Regression coverage exercises the full alias family.

@github-actions github-actions Bot Oct 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.

[/diagnosing-bugs] This hardcodes a third, independent list of "built-in Copilot" identities (Copilot, copilot-swe-agent, copilot-swe-agent[bot]) that must stay in sync with pkg/constants.CopilotBotNames (which also includes copilot and @app/copilot-swe-agent). It also completely skips checkBotStatus instead of reusing the already-tested "conclusively not a collaborator" bypass (installationCheckOptional) used for repository_dispatch — that path addresses the same root cause (GitHub App identities aren't collaborators) without a brand-new, unverified branch.

💡 Why this matters

The root cause of #67157 is "GitHub App/bot identities aren't repository collaborators, so the lookup 404s." That's exactly what installationCheckOptional already solves for repository_dispatch, with real verification (a 404 from the API, not an actor-name string match) and test coverage for the "lookup fails → not authorized" case. This PR instead adds a second, hand-maintained identity list that bypasses the API call entirely:

  • If pkg/constants.CopilotBotNames gains a new alias later (it already has copilot and @app/copilot-swe-agent), this list silently falls out of sync and those aliases regress back to the original 404 bug.
  • Skipping checkBotStatus means zero API verification occurs for Copilot — a typo'd or spoofed actorToValidate string match would be the only protection, whereas the existing installationCheckOptional pattern still requires a conclusive 404 (not a transient error) before authorizing.

Consider either: (a) extracting a shared isBuiltInCopilotIdentity helper backed by the same canonical list the Go compiler uses (or passing it through as a generated constant), or (b) setting installationCheckOptional: true for Copilot's pull_request/pull_request_target paths and reusing the existing conclusively-not-a-collaborator logic instead of a full bypass.

@copilot please address this.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fb33cbd: Copilot aliases now come from constants.CopilotBotNames, are matched using canonicalizeBotIdentifier, and reuse the existing installationCheckOptional path, which still denies transient lookup errors.

@github-actions github-actions Bot Oct 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.

[/codebase-design] The three-way literal string comparison (actorToValidate === "Copilot" || actorToValidate === "copilot-swe-agent" || actorToValidate === "copilot-swe-agent[bot]") duplicates logic that canonicalizeBotIdentifier already exists to solve elsewhere in this same file's import list.

💡 Suggested simplification 
const canonicalActor = canonicalizeBotIdentifier(actorToValidate);
const isBuiltInCopilot = canonicalActor === "Copilot" || canonicalActor === "copilot-swe-agent";

This reduces three branches to two, matches the existing canonicalization convention used by isAllowedBot/checkBotStatus in check_permissions_utils.cjs, and automatically covers any future [bot]-suffixed variant without needing a third literal added by hand.

@copilot please address this.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fb33cbd: actor and configured alias names now use the existing canonicalizeBotIdentifier helper, including optional [bot] suffixes.

@github-actions github-actions Bot Oct 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.

This literal identity list ("Copilot", "copilot-swe-agent", "copilot-swe-agent[bot]") duplicates knowledge already centralized in Go as constants.CopilotBotNames (copilot-swe-agent, Copilot, copilot, @app/copilot-swe-agent) and used for on.bots: [Copilot] expansion. If that canonical list ever changes (e.g. a new runtime alias is added), this JS check silently drifts out of sync and either over- or under-grants the collaborator-lookup bypass.

Consider deriving this list from a shared JS constant (mirroring bot_constants.go) or adding a code comment/test that explicitly cross-references constants.CopilotBotNames so a future update to one is caught by review of the other.

@copilot please address this.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fb33cbd: the compiler passes the canonical constants.CopilotBotNames list to membership validation, so the JavaScript authorization check no longer maintains a separate identity list.

const botStatus = isBuiltInCopilot ? { isBot: true, isActive: true } : await checkBotStatus(actorToValidate, owner, repo);

if (botStatus.isBot && botStatus.isActive) {
core.info(`✅ Bot '${actorToValidate}' is active on the repository and authorized`);
core.info(isBuiltInCopilot ? `✅ Built-in bot '${actorToValidate}' is allowlisted and authorized` : `✅ Bot '${actorToValidate}' is active on the repository and authorized`);
core.setOutput("is_team_member", "true");
core.setOutput("result", "authorized_bot");
core.setOutput("user_permission", "bot");
Expand Down
59 changes: 51 additions & 8 deletions actions/setup/js/check_membership.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,47 @@ describe("check_membership.cjs", () => {
expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "confused_deputy");
});

it("should authorize allowlisted Copilot synchronizing a trusted author's same-repository PR without a bot lookup", async () => {
process.env.GH_AW_ALLOWED_BOTS = "Copilot";
mockContext.actor = "Copilot";
mockContext.eventName = "pull_request";
mockContext.payload = {
action: "synchronize",
pull_request: {
user: { login: "human-author" },
head: { repo: { id: 123, full_name: "testorg/testrepo" } },
base: { repo: { id: 123, full_name: "testorg/testrepo" } },
},
};
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "write" } });

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("result", "authorized_bot");
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledTimes(1);
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ owner: "testorg", repo: "testrepo", username: "human-author" });
});

it("should deny allowlisted Copilot synchronizing a fork PR before bot authorization", async () => {
process.env.GH_AW_ALLOWED_BOTS = "Copilot";
mockContext.actor = "Copilot";
mockContext.eventName = "pull_request";
mockContext.payload = {
action: "synchronize",
pull_request: {
user: { login: "fork-author" },
head: { repo: { id: 456, full_name: "fork/repo" } },
base: { repo: { id: 123, full_name: "testorg/testrepo" } },
},
};

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("result", "confused_deputy");
expect(mockCore.setOutput).not.toHaveBeenCalledWith("result", "authorized_bot");
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
});

it.each(["pull_request", "pull_request_target"])("should deny an active allowlisted bot when synchronizing a cross-repository PR (%s event)", async eventName => {
process.env.GH_AW_ALLOWED_BOTS = "my-fixup-bot[bot]";
mockContext.actor = "my-fixup-bot";
Expand Down Expand Up @@ -890,22 +931,23 @@ describe("check_membership.cjs", () => {
expect(mockCore.setOutput).toHaveBeenCalledWith("result", "insufficient_permissions");
});

it("should authorize a bot in the allowlist when permission check returns an API error (e.g. GitHub App not a user)", async () => {
process.env.GH_AW_ALLOWED_BOTS = "Copilot";
mockContext.actor = "Copilot";

mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValueOnce({ data: { permission: "none" } }); // bot status check (Copilot[bot] form) → active
it.each(["Copilot", "copilot-swe-agent", "copilot-swe-agent[bot]"])("should authorize explicitly allowlisted built-in agent %s without a collaborator lookup on a PR", async actor => {
process.env.GH_AW_ALLOWED_BOTS = actor;
mockContext.actor = actor;
mockContext.eventName = "pull_request";
mockContext.payload = { action: "opened", pull_request: { user: { login: actor } } };

await runScript();

expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "true");
expect(mockCore.setOutput).toHaveBeenCalledWith("result", "authorized_bot");
expect(mockCore.setOutput).toHaveBeenCalledWith("user_permission", "bot");
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
});

it("should return bot_not_active when permission check returns API error and bot is not installed", async () => {
process.env.GH_AW_ALLOWED_BOTS = "Copilot";
mockContext.actor = "Copilot";
it("should still require a collaborator lookup for other Copilot-like actors", async () => {
process.env.GH_AW_ALLOWED_BOTS = "copilot[bot]";
mockContext.actor = "copilot[bot]";

const notFoundError = { status: 404, message: "Not Found" };
mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(notFoundError); // all bot status checks → 404
Expand All @@ -914,6 +956,7 @@ describe("check_membership.cjs", () => {

expect(mockCore.setOutput).toHaveBeenCalledWith("is_team_member", "false");
expect(mockCore.setOutput).toHaveBeenCalledWith("result", "bot_not_active");
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledTimes(2);
});

it("should return api_error when permission check fails and actor is not in allowed bots list", async () => {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/triggers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ on:

The `[bot]` suffix is optional — `github-actions` matches `github-actions[bot]` automatically.

Allowlisted bots must be installed on the repository, which is verified through the repository collaborator API. GitHub App identities are not collaborators, so that lookup finds nothing for them. On `repository_dispatch` the check is therefore relaxed: the lookup is still performed, and an allowlisted App is authorized when it conclusively reports that the actor is not a collaborator, because sending the event already requires `contents: write` access to the repository. A lookup that fails (for example during an API outage or when rate limited) is still denied. This is how an App-triggered `repository_dispatch` is admitted — `on.roles:` is matched against repository collaborator roles and never matches an App sender.
Allowlisted bots must be installed on the repository, which is verified through the repository collaborator API. The built-in Copilot coding agent identities (`Copilot` and `copilot-swe-agent`, including `copilot-swe-agent[bot]`) are an exception: when explicitly allowlisted, they do not require a collaborator lookup. Other GitHub App identities are not collaborators, so that lookup finds nothing for them. On `repository_dispatch` the check is therefore relaxed: the lookup is still performed, and an allowlisted App is authorized when it conclusively reports that the actor is not a collaborator, because sending the event already requires `contents: write` access to the repository. A lookup that fails (for example during an API outage or when rate limited) is still denied. This is how an App-triggered `repository_dispatch` is admitted — `on.roles:` is matched against repository collaborator roles and never matches an App sender.

### Filtering by Author Associations (`on.skip-author-associations`)

Expand Down
Loading