Board reconciler: trust Triage blocks only from collaborators - #1823
Merged
Merged
Conversation
…t an empty one Anyone can edit the body of an issue they filed, so a `### Triage` block was a way for any reporter to set their own Priority on the board within minutes of the `issues: edited` trigger firing. A block now counts only when the author's association is OWNER, MEMBER or COLLABORATOR, or the login is in TRUSTED_TRIAGE_LOGINS, which the intake bot needs because it writes under its own identity. An ignored block is reported as drift rather than dropped. Separately, the heading regex ended with `\s*\n`, which swallowed the blank line after an empty `### Triage` and let the next section's lines be read as triage values. It now stops at the heading's own line end, and tolerates the CRLF bodies the web form submits — the existing tests only ever used LF.
…egrade The reconciler reads the board as `user(login: <repo owner>)` but writes with this repository's hardcoded project id, so a fork carrying the file would read a different board and, lacking PROJECT_TOKEN, fail red on every issue event. The job is now gated on the repository name. theme-label.yml imports the shared classifier at runtime. If that import ever fails — the script renamed, or a syntax error committed straight to master — a new issue previously got no label at all. It now falls back to `needs-theme` with a warning, so the failure is visible and the issue still surfaces for a human. TRUSTED_TRIAGE_LOGINS is passed from a repository variable so the intake bot's login can be allowlisted without a code change.
✅ Deploy Preview for afmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
2 participants
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.
Follow-up to #1818, from a review of the code as it runs. One hole that should not have shipped, and three smaller things.
The hole: any reporter could set their own Priority
The reconciler reads a
### Triageblock from the issue body to fill Priority and Size. Nothing checked who wrote it. Since anyone can edit the body of an issue they filed, and the workflow runs onissues: edited, a reporter could add three lines to their own bug and have it show asP0 – Urgenton your board within minutes — looking exactly like a moderator had triaged it.Nothing exploited it, and fill-only meant you could always override, but the field was supposed to mean "someone you trust judged this", and it didn't.
Now a block counts only when the issue author's association is
OWNER,MEMBERorCOLLABORATOR, or their login is in a repository variableTRUSTED_TRIAGE_LOGINS(comma-separated). The variable is for the Discord intake bot once it exists, since it will write under its own identity — it can stay unset until then, which means "collaborators only". An ignored block is listed in the run summary rather than silently dropped, so a reporter who tries it is visible.The three smaller things
An empty Triage block read values from the next section. The heading regex swallowed the blank line after
### Triage, so a body with an empty block followed by, say,### Additional contextcontaining aPriority:line would write that value. Contrived, but it is a value nobody put in the block. Fixed, and the parser now explicitly handles the\r\nline endings the web form submits — the tests had only ever used\n.The workflow would go red on every fork. It reads the board by repository owner but writes with this repository's hardcoded project id. A fork carrying the file — mine, at the next sync — would fail with
PROJECT_TOKEN is not seton every issue event. The job is now gated ongithub.repository.The labeler had no fallback.
theme-label.ymlimports the shared classifier at runtime. If that ever failed, a new issue got no label at all. It now falls back toneeds-themewith a warning. Worth noting: that import has not yet run in production — no issue has been filed since #1818 merged — so the first new issue is its first real execution, and this makes the failure mode visible rather than silent.Nothing to do on your side
Dry run against the board at this commit is identical to the last scheduled run: 0 field writes, 0 label writes, 9 items reported. 39 script tests pass, 8 of them new. No behaviour changes for anything already on the board.