-
Notifications
You must be signed in to change notification settings - Fork 252
chore: restrict GitHub workflow permissions - future-proof #1073
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.
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
Conversation
Signed-off-by: Melissa Kilby <[email protected]>
May I kindly check in on the status of this PR? Thank you. |
name: Create PR to merge main into release branch | ||
# In the first period after branching the release branch, we typically want to include all changes from `main` also in the release branch. This workflow automatically creates a PR every Monday to merge main into the release branch. | ||
# Later in the release cycle we should stop this practice to avoid landing risky changes by disabling this workflow. To do so, disable the workflow as described in https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/disabling-and-enabling-a-workflow | ||
permissions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am assuming its ok for the job to overwrite permission
permissions:
contents: write
pull-requests: write
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is ok.
Allow me to also share the updated PR text from last weeks' PRs, here: swiftlang/github-workflows#167 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs updating to write
then - it pushes a change and puts up a PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The job-level permissions have not been changed.
This PR added the OpenSSF recommended top-level workflow permission.
name: Create PR to merge main into release branch
# In the first period after branching the release branch, we typically want to include all changes from `main` also in the release branch. This workflow automatically creates a PR every Monday to merge main into the release branch.
# Later in the release cycle we should stop this practice to avoid landing risky changes by disabling this workflow. To do so, disable the workflow as described in https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/disabling-and-enabling-a-workflow
permissions:
contents: read
on:
schedule:
- cron: '0 9 * * MON'
workflow_dispatch:
jobs:
create_merge_pr:
name: Create PR to merge main into release branch
uses: swiftlang/github-workflows/.github/workflows/create_automerge_pr.yml@main
with:
base_branch: release/6.2
permissions:
contents: write
pull-requests: write
if: (github.event_name == 'schedule' && github.repository == 'swiftlang/swift-format') || (github.event_name != 'schedule') # Ensure that we don't run this on a schedule in a fork
Allow me to quote OpenSSF:
https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions
The highest score is awarded when the permissions definitions in each workflow's yaml file are set as read-only at the top level and the required write permissions are declared at the run-level.
Remediation steps
- Set top-level permissions as read-all or contents: read as described in GitHub's documentation.
- Set any required write permissions at the job-level. Only set the permissions required for that job; do not set permissions: write-all at the job level.
Compare to the LLVM project:
Top-level: contents read, e.g. https://github.com/swiftlang/llvm-project/blob/next/.github/workflows/build-ci-container-windows.yml#L3-L4 -> this makes it future-proof
Job-level: Allow write permissions as needed, e.g. https://github.com/swiftlang/llvm-project/blob/next/.github/workflows/build-ci-container-windows.yml#L53-L58
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, I missed the later write 👍
See swiftlang/github-workflows#167 for additional context
This approach aligns with security best practices, as detailed in the following documentation: