Skip to content

fix: bot allowlist check short-circuits on permission API error in check_membership.cjs#21109

Merged
pelikhan merged 3 commits intomainfrom
copilot/fix-check-membership-error
Mar 15, 2026
Merged

fix: bot allowlist check short-circuits on permission API error in check_membership.cjs#21109
pelikhan merged 3 commits intomainfrom
copilot/fix-check-membership-error

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

When checkRepositoryPermission() returns {authorized: false, error: "..."} — e.g. when the actor is a GitHub App (Copilot) that isn't a valid GitHub user and causes a 404 — the code bailed out immediately, never reaching the GH_AW_ALLOWED_BOTS fallback. Bots explicitly listed under bots: in workflow frontmatter were silently denied.

Changes

  • check_membership.cjs: Remove the early-return on result.error. Merge the error and insufficient-permissions cases into a unified else block so the bot allowlist is always evaluated first:
checkRepositoryPermission()
  ├── authorized                          → authorized ✓
  └── not authorized (error OR no perms)
        ├── actor in GH_AW_ALLOWED_BOTS?
        │     ├── bot active on repo      → authorized_bot ✓
        │     ├── bot not installed       → bot_not_active ✓
        │     └── bot status check failed → fall through
        └── result.error → api_error
            else         → insufficient_permissions
  • check_membership.test.cjs: Three new test cases covering the previously unreachable paths: permission API error + bot in allowlist (active), permission API error + bot in allowlist (not installed), and permission API error + actor not in allowlist.

Copilot AI and others added 2 commits March 15, 2026 19:07
…I error

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review March 15, 2026 19:32
Copilot AI review requested due to automatic review settings March 15, 2026 19:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes bot allowlist authorization so it still runs when the repository permission API check fails (e.g., actor is a GitHub App that triggers a 404), preventing allowlisted bots from being incorrectly denied.

Changes:

  • Adjusts check_membership.cjs control flow to evaluate the bot allowlist fallback even when checkRepositoryPermission() returns an error.
  • Adds three test cases for permission-check error scenarios combined with bot allowlist behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
actions/setup/js/check_membership.cjs Removes the early return on permission API error so allowlisted bots can still be authorized via bot-status fallback.
actions/setup/js/check_membership.test.cjs Adds test coverage for permission API error paths involving allowlisted and non-allowlisted bots.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

// User doesn't have required permissions, check if they're an allowed bot
// User doesn't have required permissions (or the permission check failed with an error).
// Always attempt the bot allowlist fallback before giving up, so that GitHub Apps whose
// actor is not a recognised GitHub user (e.g. "Copilot") are not silently denied.
Comment on lines +60 to 64
// User doesn't have required permissions (or the permission check failed with an error).
// Always attempt the bot allowlist fallback before giving up, so that GitHub Apps whose
// actor is not a recognised GitHub user (e.g. "Copilot") are not silently denied.
if (allowedBots && allowedBots.length > 0) {
core.info(`Checking if actor '${actor}' is in allowed bots list: ${allowedBots.join(", ")}`);
@pelikhan
Copy link
Contributor

@copilot review comments

…n bot_not_active path

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot review comments

Addressed both review comments in 87fc507:

  1. Spelling: "recognised" → "recognized" in the comment on line 61.
  2. result.permission when result.error is set: Changed core.setOutput("user_permission", result.permission) in the bot_not_active path to core.setOutput("user_permission", result.permission ?? "bot") so the output is always a stable string even when the permission check failed with an error and result.permission is undefined.

Copilot AI requested a review from pelikhan March 15, 2026 19:51
@pelikhan pelikhan merged commit ce3b8a5 into main Mar 15, 2026
51 checks passed
@pelikhan pelikhan deleted the copilot/fix-check-membership-error branch March 15, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check_membership.cjs error branch short-circuits before bot allowlist fallback

3 participants