Skip to content
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

Dependency pinning #542

Merged
merged 3 commits into from
Oct 3, 2024
Merged

Conversation

carlewis
Copy link
Contributor

@carlewis carlewis commented Oct 1, 2024

Overview

Pin external workflow actions to the commit hash. Also fix Scorecard badge.

Reason for change

Pinning external workflow actions to the hashed version is a good practice to help against supply chain attacks. This change will raise the OpenSSF score too.

Description of change

External workflow actions have been pinned to the commit hash. The only actions not pinned are llvm/actions/install-ninja and llvm/actions/setup-windows as they don't have releases.

Dependabot will open PRs in the future, whenever pinned actions need update.

Anything else we should know?

By default, PRs open by dependabot to update dependencies will be tagged to be reviewed by @codeplaysoftware/security-managers and @codeplaysoftware/ock-workflow-reviewers as the code owners.

Checklist

  • Read and follow the project Code of Conduct.
  • Make sure the project builds successfully with your changes.
  • Run relevant testing locally to avoid regressions.
  • Run clang-format-17 on all modified code.

@carlewis carlewis requested a review from a team as a code owner October 1, 2024 12:35
@hvdijk
Copy link
Collaborator

hvdijk commented Oct 1, 2024

Dependency pinning is mentioned at https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions for third-party actions and makes sense in that context, so I'd be happy with the change to hendrikmuhs/ccache-action, no question there. When we do it for GitHub's own actions though, the attack we are trying to protect ourselves against, isn't it GitHub itself being compromised? GitHub's documentation for GitHub's actions does not suggest doing this, so if we deviate from their suggestions, I'd like to be sure we're doing this for good reasons, since it means we no longer get any bugfixes either.

@carlewis
Copy link
Contributor Author

carlewis commented Oct 2, 2024

Dependency pinning is mentioned at https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions for third-party actions and makes sense in that context, so I'd be happy with the change to hendrikmuhs/ccache-action, no question there. When we do it for GitHub's own actions though, the attack we are trying to protect ourselves against, isn't it GitHub itself being compromised? GitHub's documentation for GitHub's actions does not suggest doing this, so if we deviate from their suggestions, I'd like to be sure we're doing this for good reasons, since it means we no longer get any bugfixes either.

Other than GitHub's security guide, we also follow OpenSSF scorecard recommendations. Dependency pinning is a medium level check. It doesn't only apply to GitHub actions, but to Python pip or other package managers dependencies, so this PR won't get us the maximum score, but will improve the value.

@hvdijk
Copy link
Collaborator

hvdijk commented Oct 2, 2024

Other than GitHub's security guide, we also follow OpenSSF scorecard recommendations. Dependency pinning is a medium level check. It doesn't only apply to GitHub actions, but to Python pip or other package managers dependencies, so this PR won't get us the maximum score, but will improve the value.

I know this is in no way your fault, but I am not impressed with the referenced https://github.com/step-security/secure-repo?tab=readme-ov-file#3-pin-actions-to-a-full-length-commit-sha which specifically says (emphasis mine)

GitHub's Security Hardening for GitHub Actions guide recommends pinning actions to full length commit for third party actions.

and then gives an example

-      uses: actions/checkout@v3
+      uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # tag: v3

If they want to deviate from GitHub's recommendations, they should not refer to GitHub's recommendations as a justification.

@hvdijk
Copy link
Collaborator

hvdijk commented Oct 2, 2024

They have an open issue for addressing that because they have received similar feedback before, step-security/secure-repo#1606.

@carlewis
Copy link
Contributor Author

carlewis commented Oct 3, 2024

@hvdijk I just updated the branch focusing the version pinning on third party actions.

@hvdijk
Copy link
Collaborator

hvdijk commented Oct 3, 2024

Thanks, I see there were already GitHub actions pinned to specific commits in .github/workflows/scorecard.yml so it makes sense that you just kept that as is. Looks good to me, then, but will wait for @coldav to comment since this doesn't just pin actions, it also updates them, and he will know better whether there's any actions there where we need to check that that's okay.

@carlewis carlewis merged commit b6187bc into uxlfoundation:main Oct 3, 2024
10 checks passed
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