ci: gate on real write access, not author_association - #19
Merged
Conversation
Three issues from the repository owner were declined with author_association=CONTRIBUTOR. The REST API says MEMBER; the webhook says CONTRIBUTOR — because that field only reports MEMBER when an organization membership is PUBLIC. With private membership an owner arrives looking like a stranger. Adding CONTRIBUTOR to the allowlist would have fixed the symptom and broken the gate: it is not a permission, it is a fact about the past. Anyone with one merged pull request keeps it forever. The gate now asks GitHub what the account may actually do — collaborators/:user/permission — and requires write, maintain or admin. author_association is kept only for when that call cannot answer, so a token without the scope fails closed rather than open. The notice prints both, so the next surprise is one log line rather than an afternoon.
🔍 ShipIT Forge reviewed this PR — 💬 commented (no blocking issues)0 finding(s) (0 security). See the review above for inline details and suggested fixes. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
ShipIT Forge review
✅ No blocking issues found. I reviewed the changed files and ran:
- 🛡️ Security checks — SSRF, injection (SQL/command/template), broken auth/authz, hardcoded secrets, unsafe deserialization, path traversal, weak crypto.
- 🔧 Code review — correctness, error handling, missing tests, clarity.
Nothing to flag. This is a comment, not an approval — ShipIT Forge never approves PRs; a human reviewer should approve and merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Three issues opened by the repository owner were declined:
The REST API says
MEMBER. The webhook saysCONTRIBUTOR. Both are correct —author_associationreportsMEMBERonly when someone's organization membership is public. With private membership, an owner arrives looking like a stranger.The fix that would have been wrong
Adding
CONTRIBUTORto the allowlist. It fixes the symptom and breaks the gate:CONTRIBUTORis not a permission, it is a fact about the past — anyone with a single merged PR keeps it forever, including on a repo they were never trusted with.The fix
Ask GitHub what the account may actually do:
Require
write,maintainoradmin. Verified against this repo:permission=admin role=admin.author_associationis kept only as a fallback for when that call cannot answer, so a token without the scope fails closed rather than open. The notice prints both values, so the next surprise of this kind is one log line rather than an afternoon of guessing.Why the observable gate earned its keep immediately
The previous PR made the gate a job that prints its decision. It found this on the first run. As a one-line
if:, this was invisible: three skipped runs, empty logs, and no way to tell a correct refusal from a broken expression.