Skip to content

Conversation

incertum
Copy link
Contributor

See swiftlang/github-workflows#167 for additional context

This approach aligns with security best practices, as detailed in the following documentation:

The default GITHUB_TOKEN permissions are defined at the repository level. This PR modifies the workflow-level overrides to conform to OpenSSF best practices -> defense in depth.

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

Signed-off-by: Melissa Kilby <mkilby@apple.com>
Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

LGTM—thank you!

@simonjbeaumont simonjbeaumont added the semver/none No version bump required. label Oct 18, 2025
@simonjbeaumont simonjbeaumont merged commit 076b1f7 into apple:main Oct 18, 2025
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/none No version bump required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants