Skip to content

ci: declare workflow-level contents: read on 4 workflows#3838

Open
arpitjain099 wants to merge 1 commit into
eclipse-platform:masterfrom
arpitjain099:chore/declare-workflow-perms
Open

ci: declare workflow-level contents: read on 4 workflows#3838
arpitjain099 wants to merge 1 commit into
eclipse-platform:masterfrom
arpitjain099:chore/declare-workflow-perms

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

Adds workflow-level permissions: contents: read to four workflows: callUpdateTarget, htmlvalidator, pr-checks, version-increments. None call the GitHub API beyond checkout.

Same post-CVE-2025-30066 (tj-actions/changed-files) hardening pattern. YAML validated locally.

Comment on lines +12 to +14
permissions:
contents: read

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it really necessary on jobs that just call other workflows?
Wouldn't it be sufficient to specify the permissions on the called workflows (which most but not all do).

Furthermore in this case, wouldn't this break some of the called workflows? IIRC it isn't possible to elevate the permissions?

@arpitjain099

Copy link
Copy Markdown
Contributor Author

Good catch @HannesWell, thanks. You're right on both counts: pr-checks.yml, callUpdateTarget.yml, and version-increments.yml are all just callers of reusable workflows, and capping permissions at the caller would intersect with whatever the callees need. I reverted those three back to upstream and kept the change only on htmlvalidator.yml which actually runs work itself.

Force-pushed. Let me know if the reduced scope still makes sense, or if you'd rather close.

@arpitjain099 arpitjain099 force-pushed the chore/declare-workflow-perms branch 2 times, most recently from 493c13a to 61273a5 Compare May 26, 2026 06:31
@HannesWell

Copy link
Copy Markdown
Member

Force-pushed. Let me know if the reduced scope still makes sense, or if you'd rather close.

Thanks for the update. Generally it totally makes sense to use only the minimal permissions. I'll have a look at it in detail probably not before the end of the week (we are currently in the final release phase, which is very busy).

Besides this, I think there are a few more 'leave' workflows that don't have their permissions minimized. Could you check and update them too as appropriated?

arpitjain099 added a commit to arpitjain099/eclipse.platform.releng.aggregator that referenced this pull request May 28, 2026
Per maintainer request on eclipse-platform#3838, extends least-privilege permissions to the remaining directly-triggered (leaf) workflows that lacked them:

- htmlvalidator.yml: contents: read (read-only HTML validation on PRs)
- pr-checks.yml: contents: read + issues: read (orchestrator; this is the minimal union its reusable workflows declare - checkVersions {}, verifyFreezePeriod issues: read, checkMergeCommits contents: read - so no write scope is needed)
- licensecheck.yml: top-level contents: read default; the call-license-check job keeps its explicit pull-requests: write

Left the reusable workflows (on: workflow_call) and the bot-PAT writer leaves (callUpdateTarget, version-increments) untouched, since their permissions intersect with caller grants and the maintainers are best placed to set those.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@arpitjain099 arpitjain099 force-pushed the chore/declare-workflow-perms branch from 61273a5 to fb69314 Compare May 28, 2026 06:16
@arpitjain099

Copy link
Copy Markdown
Contributor Author

Thanks, and no rush given the release phase. I went through the other leaf (directly-triggered) workflows and extended the PR to the ones that can be safely minimized:

  • htmlvalidator.yml: contents: read (read-only validation on PRs)
  • pr-checks.yml: contents: read + issues: read. It is purely an orchestrator, and that is the minimal union its reusables declare (checkVersions is {}, verifyFreezePeriod needs issues: read, checkMergeCommits needs contents: read), so no write scope is required.
  • licensecheck.yml: top-level contents: read as the default; the call-license-check job keeps its explicit pull-requests: write.

I deliberately left two leaves alone: callUpdateTarget.yml and version-increments.yml. Both delegate to reusables that create PRs / push via a bot PAT (RELENG_BOT_PAT, PLATFORM_BOT_PAT), so the right minimal GITHUB_TOKEN scope there depends on how much the workflow relies on the PAT vs the default token, which you would know better than me. Happy to take a stab at those too if you would like. The reusable (on: workflow_call) workflows are also untouched on purpose, since their permissions intersect with caller grants.

Per maintainer request on eclipse-platform#3838, extends least-privilege permissions to the remaining directly-triggered (leaf) workflows that lacked them:

- htmlvalidator.yml: contents: read (read-only HTML validation on PRs)
- pr-checks.yml: contents: read + issues: read (orchestrator; this is the minimal union its reusable workflows declare - checkVersions {}, verifyFreezePeriod issues: read, checkMergeCommits contents: read - so no write scope is needed)
- licensecheck.yml: top-level contents: read default; the call-license-check job keeps its explicit pull-requests: write

Left the reusable workflows (on: workflow_call) and the bot-PAT writer leaves (callUpdateTarget, version-increments) untouched, since their permissions intersect with caller grants and the maintainers are best placed to set those.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens GitHub Actions workflows by explicitly declaring workflow-level permissions, aiming to follow a post–CVE hardening pattern and reduce default token privileges while keeping required checkout behavior working.

Changes:

  • Adds a workflow-level permissions block to explicitly set contents: read (and related permissions) for updated workflows.
  • Updates pr-checks.yml to declare required permissions for its reusable-workflow jobs.
  • Updates licensecheck.yml to declare workflow-level permissions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/pr-checks.yml Adds workflow-level permissions for PR checks (notably contents: read, plus additional scope).
.github/workflows/licensecheck.yml Adds workflow-level contents: read permissions for the license check workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +12 to 18
permissions:
contents: read
issues: read

jobs:
check-freeze-period:
uses: ./.github/workflows/verifyFreezePeriod.yml
Comment on lines +17 to +19
permissions:
contents: read

Comment on lines +12 to +14
permissions:
contents: read
issues: read
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants