From ecfe10520e9e27abe0d77aac71af2794264bae95 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 2 Sep 2026 17:23:39 +0200 Subject: [PATCH 1/4] feat: add authorize-commenter action --- .../actions/authorize-commenter/action.yml | 60 +++++++++++++++++++ CHANGELOG.md | 4 ++ 2 files changed, 64 insertions(+) create mode 100644 .github/actions/authorize-commenter/action.yml diff --git a/.github/actions/authorize-commenter/action.yml b/.github/actions/authorize-commenter/action.yml new file mode 100644 index 00000000..89cfbac3 --- /dev/null +++ b/.github/actions/authorize-commenter/action.yml @@ -0,0 +1,60 @@ +name: Authorize commenter +description: >- + Check whether the author of a comment has write access to the repository. + + Workflows triggered by `issue_comment` run from the default branch with + access to secrets, so gating them on the comment body alone lets anyone who + can comment start them. Use this action as the first job of such a workflow + and make the privileged jobs depend on it. + + The check fails closed, so if the permission cannot be read then nobody is + authorized. The token must be able to read collaborator permissions, which + the default `GITHUB_TOKEN` may not be. + +inputs: + username: + description: The login to authorize. Defaults to the author of the comment + that triggered the workflow. + required: true + default: ${{ github.event.comment.user.login }} + + github-token: + description: The GitHub token to use for authentication. Defaults to the + standard GITHUB_TOKEN. + required: true + default: ${{ github.token }} + +outputs: + authorized: + value: ${{ steps.authorize.outputs.authorized || 'false' }} + description: Whether the user has write access to the repository. + +runs: + using: composite + steps: + - name: Check the commenter's repository permission + id: authorize + uses: actions/github-script@v9 + env: + USERNAME: ${{ inputs.username }} + with: + github-token: ${{ inputs.github-token }} + script: | + const { USERNAME } = process.env; + + if (!USERNAME) { + return core.setFailed('No username to authorize. Pass `username`, or run this action on an event that has a comment author.'); + } + + // `permission` collapses custom roles onto admin/write/read/none, so + // write and admin are exactly the people who can push to the repo. + // On a public repository everyone else reads back as `read`, so + // there is no "not a collaborator" error case to handle. + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: USERNAME, + }); + + const authorized = ['admin', 'write'].includes(data.permission); + core.info(`${USERNAME} has ${data.permission} permission, ${authorized ? 'authorized' : 'not authorized'}.`); + core.setOutput('authorized', String(authorized)); diff --git a/CHANGELOG.md b/CHANGELOG.md index 3219e3ae..b7d7784d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `authorize-commenter` action, which checks whether the author of a comment has write access to the repository, so that `issue_comment` triggered workflows can refuse to run privileged steps for someone who only has read access ([#PR](https://github.com/MetaMask/github-tools/pull/PR)) + ### Fixed - Count changed lines in `pr-line-check` from the pull request files API, so the count always reflects the pull request's current base branch ([#273](https://github.com/MetaMask/github-tools/pull/273)) From a91e4e9ae206a4ff48d3eba742293d2433f41c05 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 2 Sep 2026 17:25:58 +0200 Subject: [PATCH 2/4] docs: link the changelog entry to the pull request --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d7784d..446eb1ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `authorize-commenter` action, which checks whether the author of a comment has write access to the repository, so that `issue_comment` triggered workflows can refuse to run privileged steps for someone who only has read access ([#PR](https://github.com/MetaMask/github-tools/pull/PR)) +- Add `authorize-commenter` action, which checks whether the author of a comment has write access to the repository, so that `issue_comment` triggered workflows can refuse to run privileged steps for someone who only has read access ([#286](https://github.com/MetaMask/github-tools/pull/286)) ### Fixed From f0291a40f5c3a63e2f88a0217599b7c69d6109af Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 2 Sep 2026 18:00:08 +0200 Subject: [PATCH 3/4] feat: fail on unauthorized instead of returning an output --- .../actions/authorize-commenter/action.yml | 43 +++++++++++-------- .../workflows/test-authorize-commenter.yml | 36 ++++++++++++++++ CHANGELOG.md | 2 +- 3 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/test-authorize-commenter.yml diff --git a/.github/actions/authorize-commenter/action.yml b/.github/actions/authorize-commenter/action.yml index 89cfbac3..38e5d525 100644 --- a/.github/actions/authorize-commenter/action.yml +++ b/.github/actions/authorize-commenter/action.yml @@ -1,15 +1,22 @@ name: Authorize commenter description: >- - Check whether the author of a comment has write access to the repository. + Check that the author of a comment has write access to the repository, and + fail if they do not. Workflows triggered by `issue_comment` run from the default branch with access to secrets, so gating them on the comment body alone lets anyone who - can comment start them. Use this action as the first job of such a workflow - and make the privileged jobs depend on it. + can comment start them. Run this action as the first job of such a workflow + and give every privileged job `needs` on that job. A failed dependency skips + the jobs that need it, so there is no condition to forget. - The check fails closed, so if the permission cannot be read then nobody is - authorized. The token must be able to read collaborator permissions, which - the default `GITHUB_TOKEN` may not be. + This authorizes the person who wrote the comment, not the code they wrote it + on. A workflow that passes this gate and then checks out the pull request + head or merge ref runs untrusted code with the privileges the gate was + protecting. + + The token must be able to read collaborator permissions, which the default + `GITHUB_TOKEN` may not be. If it cannot, this action fails, so an + underpowered token denies everyone rather than quietly authorizing them. inputs: username: @@ -24,16 +31,10 @@ inputs: required: true default: ${{ github.token }} -outputs: - authorized: - value: ${{ steps.authorize.outputs.authorized || 'false' }} - description: Whether the user has write access to the repository. - runs: using: composite steps: - name: Check the commenter's repository permission - id: authorize uses: actions/github-script@v9 env: USERNAME: ${{ inputs.username }} @@ -46,15 +47,19 @@ runs: return core.setFailed('No username to authorize. Pass `username`, or run this action on an event that has a comment author.'); } - // `permission` collapses custom roles onto admin/write/read/none, so - // write and admin are exactly the people who can push to the repo. - // On a public repository everyone else reads back as `read`, so - // there is no "not a collaborator" error case to handle. + // `permission` collapses custom roles onto admin, write, read and + // none, so admin and write are exactly the logins that can push. + // Everyone else reads back as `read` on a public repository and + // `none` on a private one. A login that no longer resolves, such as + // `ghost`, raises instead, which fails this step rather than + // authorizing. const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ ...context.repo, username: USERNAME, }); - const authorized = ['admin', 'write'].includes(data.permission); - core.info(`${USERNAME} has ${data.permission} permission, ${authorized ? 'authorized' : 'not authorized'}.`); - core.setOutput('authorized', String(authorized)); + if (!['admin', 'write'].includes(data.permission)) { + return core.setFailed(`@${USERNAME} has ${data.permission} permission on this repository, which is not enough to run this command.`); + } + + core.info(`@${USERNAME} has ${data.permission} permission.`); diff --git a/.github/workflows/test-authorize-commenter.yml b/.github/workflows/test-authorize-commenter.yml new file mode 100644 index 00000000..d1876ae8 --- /dev/null +++ b/.github/workflows/test-authorize-commenter.yml @@ -0,0 +1,36 @@ +name: Test "Authorize Commenter" Action + +# Temporary. This exists to confirm that the default GITHUB_TOKEN can read +# collaborator permissions. Remove it, or wire it into main.yml as a permanent +# test, once that question is settled. + +on: pull_request + +permissions: + contents: read + +jobs: + test: + name: Test + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: A login with write access is authorized + uses: ./.github/actions/authorize-commenter + with: + username: ${{ github.actor }} + + - name: A login without write access is refused + id: refused + continue-on-error: true + uses: ./.github/actions/authorize-commenter + with: + username: octocat + + - name: Fail if the refusal did not happen + if: steps.refused.outcome != 'failure' + run: | + echo "Expected octocat to be refused, but the action succeeded." + exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 446eb1ec..6d88377e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `authorize-commenter` action, which checks whether the author of a comment has write access to the repository, so that `issue_comment` triggered workflows can refuse to run privileged steps for someone who only has read access ([#286](https://github.com/MetaMask/github-tools/pull/286)) +- Add `authorize-commenter` action, which fails unless the author of a comment has write access to the repository, so that `issue_comment` triggered workflows can refuse to run privileged jobs for someone who only has read access ([#286](https://github.com/MetaMask/github-tools/pull/286)) ### Fixed From d30ca1cd90e70f74eb6a16e987315607b2719562 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 2 Sep 2026 18:01:59 +0200 Subject: [PATCH 4/4] test: keep the action test and record the verified token behavior --- .github/actions/authorize-commenter/action.yml | 4 ++-- .github/workflows/test-authorize-commenter.yml | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/actions/authorize-commenter/action.yml b/.github/actions/authorize-commenter/action.yml index 38e5d525..7acc3a86 100644 --- a/.github/actions/authorize-commenter/action.yml +++ b/.github/actions/authorize-commenter/action.yml @@ -14,8 +14,8 @@ description: >- head or merge ref runs untrusted code with the privileges the gate was protecting. - The token must be able to read collaborator permissions, which the default - `GITHUB_TOKEN` may not be. If it cannot, this action fails, so an + The default `GITHUB_TOKEN` can read collaborator permissions with no more + than `contents: read`. If a token ever cannot, this action fails, so an underpowered token denies everyone rather than quietly authorizing them. inputs: diff --git a/.github/workflows/test-authorize-commenter.yml b/.github/workflows/test-authorize-commenter.yml index d1876ae8..b3e13f7b 100644 --- a/.github/workflows/test-authorize-commenter.yml +++ b/.github/workflows/test-authorize-commenter.yml @@ -1,8 +1,7 @@ name: Test "Authorize Commenter" Action -# Temporary. This exists to confirm that the default GITHUB_TOKEN can read -# collaborator permissions. Remove it, or wire it into main.yml as a permanent -# test, once that question is settled. +# Unlike the other test-*.yml workflows, this one triggers directly rather than +# through workflow_call, so that it actually runs on every pull request. on: pull_request @@ -17,7 +16,9 @@ jobs: - name: Checkout repository uses: actions/checkout@v6 + # Skipped on forks, where the author has read access and would be refused. - name: A login with write access is authorized + if: github.event.pull_request.head.repo.fork == false uses: ./.github/actions/authorize-commenter with: username: ${{ github.actor }}