docs(ci): correct the job-level permissions comment - #3591
Conversation
The comment above the top-level `permissions` block claimed that jobs request the minimum extra scopes on top of it. A job-level block does not add to the workflow-level one: it replaces it, and every scope the job does not name is set to none. A job needing one extra scope has to restate `contents: read` alongside it or lose read access to the repository. The corrected wording states the replace semantics, so the next job block written against this comment grants the whole set it needs rather than only the delta. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
📝 WalkthroughWalkthroughThe pull request updates permission comments in ten GitHub Actions workflows. The comments state that top-level tokens are read-only and that job-level permission blocks replace top-level defaults. ChangesWorkflow permission documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM
The comment now matches GitHub Actions' documented behavior (a job-level permissions block replaces the workflow-level one rather than merging, and every scope left unnamed drops to none); the change is comment-only across all ten workflows with no permissions: value touched.
Caveats
- Verified comment-only: every hunk changes solely the narrative line above
permissions:; thecontents: readblock itself is unchanged in all ten files, so there is no token-scope behavior change to regress. - The corrected wording fixes a genuinely misleading prior comment ("jobs request the minimum extra scopes" implied additive semantics); the replace-not-merge phrasing is accurate. A reader should still note the unstated corollary that a job with no
permissions:block continues to inherit the workflow-level defaults, but the sentence as written ("A job-level block replaces this one") is correct and not obligated to enumerate that case.
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM
Comment-only change; the corrected semantics are verified true against GitHub Actions documentation, applied identically across all 10 touched files, and the diff contains no non-comment line.
Caveats
- Phases 5b/5c/5d (upgrade/fresh-install impact, config-combination matrix, test-adequacy) are N/A for this PR: it touches only
.github/workflows/*.yaml/.ymlcomment lines, nopackages/, CRD, RBAC value, chart, or migration. Verified viagit diff | grep -vE '^[+-]#'that zero non-comment lines changed in any of the 10 files. - The GitHub Actions semantics the corrected comment now states were independently verified (not taken from the PR body) against
docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissionsvia two independent lookups: a job-levelpermissions:block replaces the workflow-level one for that job (does not merge/add), and any permission not named in whichever block applies is set tonone. - A repo-wide audit of every job-level
permissions:block in the 10 touched files found none currently exhibiting the footgun the corrected comment warns about (a job withpackages: writealone that also callsactions/checkout): every job either omitscontents:and never checks out code, or restatescontents: readalongside its other scopes. The fix is preventative documentation correctness, not a fix for a currently-broken job. - Second independent naive-reviewer pass (novelty pass) was not run: the diff is verified byte-for-byte comment-only across all 10 files with zero non-comment lines changed, leaving no semantic surface for an independent derivation beyond the taxonomy-blind hypotheses already executed directly (Phase 3c: 3 derived, 0 survived).
Recommended follow-ups
.github/workflows/build-main.yamlcarries a different, bespoke top-level comment ("the warm-cache job below narrows to the packages:write scope it actually needs") rather than this PR's newly standardized wording. It was not touched here and itswarm-cachejob already restates bothcontents: readandpackages: writecorrectly, so there is no defect, only a wording inconsistency; optional alignment in a separate PR.
What this PR does
The comment above the top-level
permissions:block said jobs request the minimum extra scopes on top of it. That's backwards. A job-level block replaces the workflow-level one, and every scope it does not name is set tonone. Writepackages: writealone in a job and it losescontents: readand can't check the repo out.Same wording in every file that carried the old line. It matches what #3574 uses for the workflow it adds.
backport.yamlandpull-requests.yamlalso appear in open #3569. Its changed lines there are thecancel-in-progressexpressions two lines below the comment, and a three-way merge is clean in either order.Comment-only: every changed line starts with
#, the files still parse, and actionlint reports the same 45 pre-existing findings before and after.Screenshots
Not a UI change.
Downstream repositories
Walked the trigger map against the diff. The only row that names a file here is the ccp one on
.github/workflows/tags.yaml, and it triggers on release-prep behaviour, which this doesn't touch.Release note
Summary by CodeRabbit