Description
The handle-confirmation job in .github/workflows/readme-pr-check.yml
(line 63) swaps the gating label from readme: pending to
readme: ready for review whenever any comment on the PR contains the magic
string, without checking who wrote the comment:
handle-confirmation:
if: github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '/i-promise-this-is-not-a-new-server')
The job verifies neither github.event.comment.user.login /
author_association, nor that the commenter is the PR author or a maintainer.
The gate exists to enforce the policy "we are no longer accepting PRs to add
new servers to the README" (see the bot's comment posted by
check-readme-only). Any third party — not just the PR author — can comment
/i-promise-this-is-not-a-new-server and move the PR past the gate, since the
magic string is written out verbatim in the bot's own public comment.
Trigger scenario
- A fork PR modifies only
README.md (e.g. to add a new server), so
check-readme-only applies the readme: pending label.
- The PR author — or any unrelated user — comments
/i-promise-this-is-not-a-new-server.
handle-confirmation removes readme: pending and adds
readme: ready for review, silently bypassing the maintainers' gate.
Impact
A review-process bypass: new-server PRs that the maintainers intended to block
can be marked ready for review without maintainer involvement. No secrets or
OIDC capabilities are exposed (the job only has pull-requests: write), so
the impact is limited to workflow integrity.
Suggested fix
Restrict the confirmation to trusted actors, e.g.:
if: github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '/i-promise-this-is-not-a-new-server') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)
(or require the commenter to equal github.event.issue.user.login, if the
intent is that only the PR author may confirm).
I'd be happy to open a PR with the author_association check. Thanks!
Description
The
handle-confirmationjob in.github/workflows/readme-pr-check.yml(line 63) swaps the gating label from
readme: pendingtoreadme: ready for reviewwhenever any comment on the PR contains the magicstring, without checking who wrote the comment:
The job verifies neither
github.event.comment.user.login/author_association, nor that the commenter is the PR author or a maintainer.The gate exists to enforce the policy "we are no longer accepting PRs to add
new servers to the README" (see the bot's comment posted by
check-readme-only). Any third party — not just the PR author — can comment/i-promise-this-is-not-a-new-serverand move the PR past the gate, since themagic string is written out verbatim in the bot's own public comment.
Trigger scenario
README.md(e.g. to add a new server), socheck-readme-onlyapplies thereadme: pendinglabel./i-promise-this-is-not-a-new-server.handle-confirmationremovesreadme: pendingand addsreadme: ready for review, silently bypassing the maintainers' gate.Impact
A review-process bypass: new-server PRs that the maintainers intended to block
can be marked ready for review without maintainer involvement. No secrets or
OIDC capabilities are exposed (the job only has
pull-requests: write), sothe impact is limited to workflow integrity.
Suggested fix
Restrict the confirmation to trusted actors, e.g.:
(or require the commenter to equal
github.event.issue.user.login, if theintent is that only the PR author may confirm).
I'd be happy to open a PR with the
author_associationcheck. Thanks!