Skip to content

fix(ci): let the PR labeler actually write labels #566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidge-jun merged 2 commits into from
Jul 27, 2026
Merged

fix(ci): let the PR labeler actually write labels #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9933017
fix(ci): let the PR labeler actually write labels
lidge-jun Jul 27, 2026
8efa677
test(ci): pin the labeler to the permissions the API demands
lidge-jun Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/scripts/pr-labeler.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,15 @@ describe("pr-labeler workflow", () => {
it("keeps trusted default-branch checkout, concurrency cancel, and minimal permissions", () => {
assert.match(workflow, /ref:\s*\$\{\{\s*github\.event\.repository\.default_branch\s*\}\}/);
assert.match(workflow, /cancel-in-progress:\s*true/);
assert.match(workflow, /pull-requests:\s*read/);
assert.match(workflow, /issues:\s*write/);
assert.doesNotMatch(workflow, /pull-requests:\s*write/);
// The issues label endpoints are shared with pull requests: writing a label
// onto a PR number needs pull_requests=write alongside issues=write, which
// GitHub reports as `issues=write; pull_requests=write`. Pinning this to
// read made the workflow fail closed on the first PR that actually needed a
// label applied (#565), so write is the minimum here, not an escalation.
assert.match(workflow, /pull-requests:\s*write/);
// contents stays read — the labeler never pushes.
assert.match(workflow, /contents:\s*read/);
assert.doesNotMatch(workflow, /contents:\s*write/);
Comment on lines +174 to +177

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Assert the effective top-level permissions map, not arbitrary workflow text.

The unanchored regexes search the entire YAML document, so a comment or unrelated job-level block could satisfy pull-requests: write and contents: read without proving the labeler job receives those permissions. Conversely, an unrelated contents: write occurrence could fail the test. Parse the workflow or extract its top-level permissions block and assert the exact contract: contents: read, pull-requests: write, and issues: write, with no contents: write.

As per path instructions, .github/** is a security boundary and workflow permission changes require explicit security review.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/pr-labeler.test.cjs around lines 174 - 177, Update the
workflow permission assertions in the relevant test to inspect the effective
top-level permissions map rather than matching arbitrary YAML text. Parse the
workflow or isolate its top-level permissions block, then assert exactly
contents: read, pull-requests: write, and issues: write, while rejecting
contents: write; do not rely on comments or job-level permissions.

Source: Path instructions

});
});
10 changes: 8 additions & 2 deletions .github/workflows/pr-labeler.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ concurrency:

permissions:
contents: read
# pulls.get only needs read; label mutations use the issues API.
pull-requests: read
# pulls.get only needs read, but the issues label endpoints are shared with
# pull requests: adding or removing a label on a PR number is rejected with
# "Resource not accessible by integration" unless the token also carries
# pull_requests=write (the API reports `issues=write; pull_requests=write`
# in x-accepted-github-permissions). Read-only here silently worked while
# every run happened to be a no-op sync, and failed on the first PR that
# actually needed a label written.
pull-requests: write
Comment on lines +20 to +27

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Scope pull-requests: write to the labeler job.

The write permission is required for the label mutations, but granting it at workflow scope gives every job and future step a token capable of modifying pull requests. Move this permission under only the job that calls addLabels, removeLabel, and createLabel, while keeping unrelated jobs read-only or without the permission.

Proposed fix 
 permissions:
-  pull-requests: write

 jobs:
   <labeler-job>:
+    permissions:
+      pull-requests: write
🧰 Tools
🪛 zizmor (1.26.1)

[error] 27-27: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level

(excessive-permissions)

[warning] 27-27: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/pr-labeler.yml around lines 20 - 27, Move the
pull-requests: write permission from workflow-level permissions into only the
job invoking addLabels, removeLabel, and createLabel. Keep unrelated jobs
read-only or without this permission, preserving the labeler job’s ability to
mutate labels.

Sources: Path instructions, Linters/SAST tools

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the minimal-permissions regression test

Changing this permission to write leaves .github/scripts/pr-labeler.test.cjs:165-170 explicitly requiring pull-requests: read and rejecting pull-requests: write. The checked .github/workflows/issue-quality-tests.yml is path-triggered by this workflow and runs that test, so node --test .github/scripts/pr-labeler.test.cjs now fails. Update the regression test to expect the intentional write permission; otherwise this change cannot get green CI.

Useful? React with 👍 / 👎.

issues: write

jobs:
Expand Down
Loading