From 6f4821eee284294111a6f9522c15389a8e2ad7bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:25:15 +0000 Subject: [PATCH 1/2] Initial plan From 4d88306baa38c1b78a6f2ea936b382bbf16d1dec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:41:48 +0000 Subject: [PATCH 2/2] fix: canonicalize bot identifiers so slug and slug[bot] match in on.bots Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/check_membership.cjs | 4 +- actions/setup/js/check_permissions_utils.cjs | 45 +++++++-- .../setup/js/check_permissions_utils.test.cjs | 95 ++++++++++++++++++- actions/setup/js/compute_text.cjs | 8 +- 4 files changed, 130 insertions(+), 22 deletions(-) diff --git a/actions/setup/js/check_membership.cjs b/actions/setup/js/check_membership.cjs index d2d67ff2dd8..699d38bb419 100644 --- a/actions/setup/js/check_membership.cjs +++ b/actions/setup/js/check_membership.cjs @@ -1,7 +1,7 @@ // @ts-check /// -const { parseRequiredPermissions, parseAllowedBots, checkRepositoryPermission, checkBotStatus } = require("./check_permissions_utils.cjs"); +const { parseRequiredPermissions, parseAllowedBots, checkRepositoryPermission, checkBotStatus, isAllowedBot } = require("./check_permissions_utils.cjs"); async function main() { const { eventName } = context; @@ -68,7 +68,7 @@ async function main() { if (allowedBots && allowedBots.length > 0) { core.info(`Checking if actor '${actor}' is in allowed bots list: ${allowedBots.join(", ")}`); - if (allowedBots.includes(actor)) { + if (isAllowedBot(actor, allowedBots)) { core.info(`Actor '${actor}' is in the allowed bots list`); // Verify the bot is active/installed on the repository diff --git a/actions/setup/js/check_permissions_utils.cjs b/actions/setup/js/check_permissions_utils.cjs index 9f10e54f222..d78f3674a98 100644 --- a/actions/setup/js/check_permissions_utils.cjs +++ b/actions/setup/js/check_permissions_utils.cjs @@ -25,7 +25,31 @@ function parseAllowedBots() { } /** - * Check if the actor is a bot and if it's active on the repository + * Canonicalize a bot/App identifier by stripping the [bot] suffix. + * Both "my-app" and "my-app[bot]" normalize to "my-app". + * @param {string} name - Bot identifier (with or without [bot] suffix) + * @returns {string} The base slug without [bot] suffix + */ +function canonicalizeBotIdentifier(name) { + return name.endsWith("[bot]") ? name.slice(0, -5) : name; +} + +/** + * Check if an actor matches any entry in the allowed bots list, + * treating and [bot] as equivalent App identities. + * @param {string} actor - The runtime actor name + * @param {string[]} allowedBots - Array of allowed bot identifiers + * @returns {boolean} + */ +function isAllowedBot(actor, allowedBots) { + const canonicalActor = canonicalizeBotIdentifier(actor); + return allowedBots.some(bot => canonicalizeBotIdentifier(bot) === canonicalActor); +} + +/** + * Check if the actor is a bot and if it's active on the repository. + * Accepts both and [bot] actor forms, since GitHub Apps + * may appear either way depending on the event context. * @param {string} actor - GitHub username to check * @param {string} owner - Repository owner * @param {string} repo - Repository name @@ -33,22 +57,21 @@ function parseAllowedBots() { */ async function checkBotStatus(actor, owner, repo) { try { - // Check if the actor looks like a bot (ends with [bot]) - const isBot = actor.endsWith("[bot]"); - - if (!isBot) { - return { isBot: false, isActive: false }; - } + // GitHub Apps can appear as either or [bot]. + // Treat both forms as a bot identity; always query the API with the [bot] form. + const actorHasBotSuffix = actor.endsWith("[bot]"); + const actorForApi = actorHasBotSuffix ? actor : `${actor}[bot]`; core.info(`Checking if bot '${actor}' is active on ${owner}/${repo}`); - // Try to get the bot's permission level to verify it's installed/active on the repo - // GitHub Apps/bots that are installed on a repository show up in the collaborators + // Try to get the bot's permission level to verify it's installed/active on the repo. + // GitHub Apps/bots that are installed on a repository show up in the collaborators. + // Use the [bot]-suffixed form since that is how GitHub App identities are listed. try { const botPermission = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, - username: actor, + username: actorForApi, }); core.info(`Bot '${actor}' is active with permission level: ${botPermission.data.permission}`); @@ -114,6 +137,8 @@ async function checkRepositoryPermission(actor, owner, repo, requiredPermissions module.exports = { parseRequiredPermissions, parseAllowedBots, + canonicalizeBotIdentifier, + isAllowedBot, checkRepositoryPermission, checkBotStatus, }; diff --git a/actions/setup/js/check_permissions_utils.test.cjs b/actions/setup/js/check_permissions_utils.test.cjs index 71251ccdc6c..c745ff0572d 100644 --- a/actions/setup/js/check_permissions_utils.test.cjs +++ b/actions/setup/js/check_permissions_utils.test.cjs @@ -29,6 +29,8 @@ global.github = mockGithub; describe("check_permissions_utils", () => { let parseRequiredPermissions; let parseAllowedBots; + let canonicalizeBotIdentifier; + let isAllowedBot; let checkRepositoryPermission; let checkBotStatus; let originalEnv; @@ -47,6 +49,8 @@ describe("check_permissions_utils", () => { const module = await import("./check_permissions_utils.cjs"); parseRequiredPermissions = module.parseRequiredPermissions; parseAllowedBots = module.parseAllowedBots; + canonicalizeBotIdentifier = module.canonicalizeBotIdentifier; + isAllowedBot = module.isAllowedBot; checkRepositoryPermission = module.checkRepositoryPermission; checkBotStatus = module.checkBotStatus; }); @@ -100,6 +104,54 @@ describe("check_permissions_utils", () => { }); }); + describe("canonicalizeBotIdentifier", () => { + it("should strip [bot] suffix", () => { + expect(canonicalizeBotIdentifier("dependabot[bot]")).toBe("dependabot"); + }); + + it("should return name unchanged when no [bot] suffix", () => { + expect(canonicalizeBotIdentifier("my-pipeline-app")).toBe("my-pipeline-app"); + }); + + it("should handle names with [bot] suffix only once", () => { + expect(canonicalizeBotIdentifier("github-actions[bot]")).toBe("github-actions"); + }); + }); + + describe("isAllowedBot", () => { + it("should match exact slug to slug", () => { + expect(isAllowedBot("my-app", ["my-app"])).toBe(true); + }); + + it("should match slug to slug[bot]", () => { + expect(isAllowedBot("my-app[bot]", ["my-app"])).toBe(true); + }); + + it("should match slug[bot] to slug", () => { + expect(isAllowedBot("my-app", ["my-app[bot]"])).toBe(true); + }); + + it("should match slug[bot] to slug[bot]", () => { + expect(isAllowedBot("my-app[bot]", ["my-app[bot]"])).toBe(true); + }); + + it("should return false when actor is not in the list", () => { + expect(isAllowedBot("other-app", ["my-app"])).toBe(false); + }); + + it("should return false for empty allowed bots list", () => { + expect(isAllowedBot("my-app", [])).toBe(false); + }); + + it("should match against any entry in the list", () => { + expect(isAllowedBot("renovate[bot]", ["dependabot[bot]", "renovate", "github-actions[bot]"])).toBe(true); + }); + + it("should not match partial slug names", () => { + expect(isAllowedBot("my-app-extra[bot]", ["my-app"])).toBe(false); + }); + }); + describe("parseRequiredPermissions", () => { it("should parse comma-separated permissions", () => { process.env.GH_AW_REQUIRED_ROLES = "admin,write,read"; @@ -287,15 +339,48 @@ describe("check_permissions_utils", () => { expect(mockCore.info).toHaveBeenCalledWith("Bot 'dependabot[bot]' is active with permission level: write"); }); - it("should return false for non-bot users", async () => { - const result = await checkBotStatus("regularuser", "testowner", "testrepo"); + it("should identify active bot by slug without [bot] suffix", async () => { + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ + data: { permission: "write" }, + }); + + const result = await checkBotStatus("my-pipeline-app", "testowner", "testrepo"); + + expect(result).toEqual({ + isBot: true, + isActive: true, + }); + + // API should be called with the [bot]-suffixed form + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + username: "my-pipeline-app[bot]", + }); + + expect(mockCore.info).toHaveBeenCalledWith("Checking if bot 'my-pipeline-app' is active on testowner/testrepo"); + expect(mockCore.info).toHaveBeenCalledWith("Bot 'my-pipeline-app' is active with permission level: write"); + }); + + it("should return inactive bot when slug without [bot] suffix is not installed", async () => { + const apiError = { status: 404, message: "Not Found" }; + mockGithub.rest.repos.getCollaboratorPermissionLevel.mockRejectedValue(apiError); + + const result = await checkBotStatus("my-pipeline-app", "testowner", "testrepo"); expect(result).toEqual({ - isBot: false, + isBot: true, isActive: false, }); - expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled(); + // API should still be called with the [bot]-suffixed form + expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + username: "my-pipeline-app[bot]", + }); + + expect(mockCore.warning).toHaveBeenCalledWith("Bot 'my-pipeline-app' is not active/installed on testowner/testrepo"); }); it("should handle 404 error for inactive bot", async () => { @@ -357,7 +442,7 @@ describe("check_permissions_utils", () => { }); }); - it("should verify bot is installed on repository", async () => { + it("should verify bot is installed on repository using [bot] form", async () => { mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ data: { permission: "admin" }, }); diff --git a/actions/setup/js/compute_text.cjs b/actions/setup/js/compute_text.cjs index e7f5f0b754d..c9d24a17001 100644 --- a/actions/setup/js/compute_text.cjs +++ b/actions/setup/js/compute_text.cjs @@ -8,6 +8,7 @@ */ const { sanitizeIncomingText, writeRedactedDomainsLog } = require("./sanitize_incoming_text.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { parseAllowedBots, isAllowedBot } = require("./check_permissions_utils.cjs"); async function main() { let text = ""; @@ -33,11 +34,8 @@ async function main() { } catch (permError) { core.warning(`Permission check failed for actor '${actor}': ${getErrorMessage(permError)}`); // Check if actor is in the allowed bots list (configured via on.bots in frontmatter) - const allowedBots = - process.env.GH_AW_ALLOWED_BOTS?.split(",") - .map(b => b.trim()) - .filter(b => b) ?? []; - if (allowedBots.includes(actor)) { + const allowedBots = parseAllowedBots(); + if (isAllowedBot(actor, allowedBots)) { core.info(`Actor '${actor}' is in the allowed bots list, treating as 'write' access`); permission = "write"; } else {