🌱 Pin actions to a full length commit SHA #6341

naveensrinivasan · 2022-03-27T21:55:28Z

What this PR does / why we need it:

Only pinned the non-github actions

Pinned actions by SHA https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies
Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions

Pin actions to a full length commit SHA

Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.

https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions

Also, dependabot supports upgrading based on SHA.

Signed-off-by: naveensrinivasan [email protected]

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

k8s-ci-robot · 2022-03-27T21:55:36Z

Hi @naveensrinivasan. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2022-03-27T21:55:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign vincepri after the PR has been reviewed.
You can assign the PR to them by writing /assign @vincepri in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sbueringer

I assume this was not done?

Included permissions for the action.

(just because it was mentioned in the PR description)

.github/workflows/dependabot.yml

sbueringer · 2022-03-28T09:42:40Z

/ok-to-test

naveensrinivasan · 2022-03-28T11:04:46Z

I assume this was not done?

Included permissions for the action.

(just because it was mentioned in the PR description)

Yes. I will do that as a separate PR. My apologies for the wrong message.

- Pinned actions by SHA https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies - Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions >Pin actions to a full length commit SHA >Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions Also, dependabot supports upgrading based on SHA. Signed-off-by: naveensrinivasan <[email protected]>

sbueringer · 2022-03-29T05:57:22Z

.github/workflows/dependabot.yml

@@ -34,7 +34,7 @@ jobs:
      run: make modules
    - name: Update generated code
      run: make generate
-    - uses: EndBug/add-and-commit@v7
+    - uses: EndBug/add-and-commit@8c12ff729a98cfbcd3fe38b49f55eceb98a5ec02 # v7 https://api.github.com/repos/EndBug/add-and-commit/git/commits/8c12ff729a98cfbcd3fe38b49f55eceb98a5ec02


@naveensrinivasan My bad. I missed that they actually have a v7 and a v7.0.0 tag in this repo. That's why I though it is not just the commit id of the repo.

Do you know if dependabot will update the comment including the tag and the URL automatically?

Anyway for me it would be fine to drop the URL. I just didn't know / misread that it's directly a commit ID on the repo. I thought it might be a sha of a published "action artifact" or something.

If we keep the URL, can you please link to those URLs instead: https://github.com/EndBug/add-and-commit/releases/tag/v7

This way the tag can be directly mapped to the commit id

TBH I use the API to get the URL. Sorry, As of now I can do this.

sbueringer · 2022-03-29T06:01:54Z

.github/workflows/release.yml

@@ -28,7 +28,7 @@ jobs:
        run: |
          make release-notes
      - name: Release
-        uses: softprops/action-gh-release@v1
+        uses: softprops/action-gh-release@4716bde57e0fcda6ce83002e2469b8a90e560573 # v1 https://api.github.com/repos/softprops/action-gh-release/git/tags/4716bde57e0fcda6ce83002e2469b8a90e560573


v1 seems to be:
https://github.com/softprops/action-gh-release/releases/tag/v1 => 1e07f4398721186383de40550babbdf2b84acfc5

I couldn't find 4716bde57e0fcda6ce83002e2469b8a90e560573

API endpoint (from above) seems to point to the same release commit commit if I'm reading the api endpoint right:
softprops/action-gh-release@1e07f43

I'm not sure why the shas are different though, but I guess it's the tag i.e. https://docs.github.com/en/rest/reference/git#tags

I couldn't find 4716bde57e0fcda6ce83002e2469b8a90e560573

Have you had a chance to look at this URL https://api.github.com/repos/softprops/action-gh-release/git/tags/4716bde57e0fcda6ce83002e2469b8a90e560573 ?

Hm maybe some GitHub issue. The URL didn't work for me a few hours before.

But v1 is definitely not pointing to this commit: https://github.com/softprops/action-gh-release/releases/tag/v1

I wrote something up https://gist.github.com/naveensrinivasan/ca008c07279176acce28969fb77d056f hope this helps.

Okay got the API query issue. But couldn't we just use the commit id linked at the release in the GitHub UI?
=> https://github.com/softprops/action-gh-release/releases/tag/v1 (softprops/action-gh-release@1e07f43)

But I think the most important question is, what would dependabot do?

Does it update to the latest release or the latest tag and which sha would it use then?

And also would it update the URL comment, because I prefer not adding the URL if we would have to update it manually after each dependabot PR to bump actions

It wouldn't add the URL. The dependabot would update to the latest tag instead of the version.

It wouldn't add the URL

Okay then let's drop the URLs

Would it update the comment with the tag behind the sha?

update to the latest tag instead of the version

To clarify, the latest tag instead of the latest release? (or what do you mean with version?)

k8s-ci-robot · 2022-04-04T18:35:57Z

@naveensrinivasan: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

vincepri · 2022-06-07T17:15:02Z

Any updates on this PR?

sbueringer · 2022-06-07T17:18:44Z

I think in general we are basically happy with the way actions are automatically bumped by dependabot right now.

The only gap is that we are using tags so we're not safe against folks moving tags around.

If we have a good way to automatically bump GitHub actions based on sha's while still preserving the info which corresponding tag it is we can move forward.

naveensrinivasan · 2022-06-07T17:35:08Z

Any updates on this PR?

Sorry I need to recall what needs to be done for this. Should I just drop the URL's would that suffice to merge this in. Please let me know. I can rebase and make that change.

Just an FYI dependabot doesn't update the comment on the version so it becomes obsolete.

sbueringer · 2022-06-07T17:38:41Z

I can't really decide it on my own, but I'm not sure if we want to have the sha instead of the tag.

It's more secure, but for humans it's hard to know which version of an action we are using.

@vincepri WDYT?

vincepri · 2022-06-09T16:06:56Z

Unless dependabot can update sha's automatically based on tags, I'd leave it as is and use tags for now

naveensrinivasan · 2022-06-09T16:33:34Z

Unless dependabot can update sha's automatically based on tags, I'd leave it as is and use tags for now

Dependabot can update sha's automatically based on tags.

sbueringer · 2022-06-09T16:41:10Z

I think the only problem is that we won't have the tags in the workflow YAMLs. So dependabot will bump the same way but if we want to know what we're using we have to lookup sha => tag manually.

naveensrinivasan · 2022-06-11T14:47:09Z

I think the only problem is that we won't have the tags in the workflow YAMLs. So dependabot will bump the same way but if we want to know what we're using we have to lookup sha => tag manually.

Yes and I wrote how it can be done here https://gist.github.com/naveensrinivasan/ca008c07279176acce28969fb77d056f

fabriziopandini · 2022-06-12T13:33:06Z

I understand security concerns, but at least from my point of view what mostly impacts us is maintainability and human-readable tags make it easy.

I saw this PR adds a comment after SHA to find a middle ground; is dependabot doing the same when it bumps a new release? I there an example of those bumps in other projects?
If we can't find a middle ground that makes the maintainers comfortable, I'm -1 to this change

naveensrinivasan · 2022-06-12T15:12:02Z

I understand security concerns, but at least from my point of view what mostly impacts us is maintainability and human-readable tags make it easy.

I saw this PR adds a comment after SHA to find a middle ground; is dependabot doing the same when it bumps a new release? I there an example of those bumps in other projects? If we can't find a middle ground that makes the maintainers comfortable, I'm -1 to this change

Dependabot doesn't add comments to the version of the SHA that is pinned. That is an issue.

chrischdi · 2022-06-12T16:28:38Z

For dependabot: there is an issue which would help to have the best of both (pin by hash and have the tag in a comment): dependabot/dependabot-core#4691

However, this did not get implemented as of today.

sbueringer · 2022-06-13T04:51:02Z

Thx for the links. I subscribed to those issues. I would personally prefer waiting with sha pinning until updating the comment is implemented.

fabriziopandini · 2022-06-13T09:45:45Z

+1 to wait for a version of dependabot that adds the comment

naveensrinivasan · 2022-06-13T10:14:25Z

Sounds good.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 27, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 27, 2022

k8s-ci-robot requested review from CecileRobertMichon and JoelSpeed March 27, 2022 21:55

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 27, 2022

naveensrinivasan changed the title ~~Pin actions to a full length commit SHA~~ 🌱 Pin actions to a full length commit SHA Mar 27, 2022

sbueringer reviewed Mar 28, 2022

View reviewed changes

.github/workflows/dependabot.yml Outdated Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 28, 2022

naveensrinivasan added 2 commits March 28, 2022 21:24

Included comments

be15876

naveensrinivasan force-pushed the naveen/feat/pin-deps branch from bb35e3b to 2f32715 Compare March 28, 2022 21:27

naveensrinivasan requested a review from sbueringer March 28, 2022 21:28

sbueringer reviewed Mar 29, 2022

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2022

Merge branch 'main' into naveen/feat/pin-deps

36ab94f

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 3, 2022

naveensrinivasan closed this Jun 13, 2022

sbueringer mentioned this pull request Nov 7, 2022

Pin GitHub actions to commit SHAs #7466

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Pin actions to a full length commit SHA #6341

🌱 Pin actions to a full length commit SHA #6341

naveensrinivasan commented Mar 27, 2022

k8s-ci-robot commented Mar 27, 2022

k8s-ci-robot commented Mar 27, 2022

sbueringer left a comment

sbueringer commented Mar 28, 2022

naveensrinivasan commented Mar 28, 2022

sbueringer Mar 29, 2022 •

edited

Loading

sbueringer Mar 29, 2022 •

edited

Loading

naveensrinivasan Mar 29, 2022

sbueringer Mar 29, 2022

sbueringer Mar 29, 2022

killianmuldoon Mar 29, 2022 •

edited

Loading

naveensrinivasan Mar 29, 2022

sbueringer Mar 29, 2022 •

edited

Loading

naveensrinivasan Apr 3, 2022

sbueringer Apr 4, 2022 •

edited

Loading

sbueringer Apr 4, 2022 •

edited

Loading

naveensrinivasan Apr 4, 2022

sbueringer Apr 4, 2022 •

edited

Loading

k8s-ci-robot commented Apr 4, 2022

vincepri commented Jun 7, 2022

sbueringer commented Jun 7, 2022

naveensrinivasan commented Jun 7, 2022

sbueringer commented Jun 7, 2022 •

edited

Loading

vincepri commented Jun 9, 2022 •

edited

Loading

naveensrinivasan commented Jun 9, 2022

sbueringer commented Jun 9, 2022 •

edited

Loading

naveensrinivasan commented Jun 11, 2022

fabriziopandini commented Jun 12, 2022

naveensrinivasan commented Jun 12, 2022

chrischdi commented Jun 12, 2022

sbueringer commented Jun 13, 2022 •

edited

Loading

fabriziopandini commented Jun 13, 2022

naveensrinivasan commented Jun 13, 2022

🌱 Pin actions to a full length commit SHA #6341

🌱 Pin actions to a full length commit SHA #6341

Conversation

naveensrinivasan commented Mar 27, 2022

k8s-ci-robot commented Mar 27, 2022

k8s-ci-robot commented Mar 27, 2022

sbueringer left a comment

Choose a reason for hiding this comment

sbueringer commented Mar 28, 2022

naveensrinivasan commented Mar 28, 2022

sbueringer Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

sbueringer Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

naveensrinivasan Mar 29, 2022

Choose a reason for hiding this comment

sbueringer Mar 29, 2022

Choose a reason for hiding this comment

sbueringer Mar 29, 2022

Choose a reason for hiding this comment

killianmuldoon Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

naveensrinivasan Mar 29, 2022

Choose a reason for hiding this comment

sbueringer Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

naveensrinivasan Apr 3, 2022

Choose a reason for hiding this comment

sbueringer Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

sbueringer Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

naveensrinivasan Apr 4, 2022

Choose a reason for hiding this comment

sbueringer Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 4, 2022

vincepri commented Jun 7, 2022

sbueringer commented Jun 7, 2022

naveensrinivasan commented Jun 7, 2022

sbueringer commented Jun 7, 2022 • edited Loading

vincepri commented Jun 9, 2022 • edited Loading

naveensrinivasan commented Jun 9, 2022

sbueringer commented Jun 9, 2022 • edited Loading

naveensrinivasan commented Jun 11, 2022

fabriziopandini commented Jun 12, 2022

naveensrinivasan commented Jun 12, 2022

chrischdi commented Jun 12, 2022

sbueringer commented Jun 13, 2022 • edited Loading

fabriziopandini commented Jun 13, 2022

naveensrinivasan commented Jun 13, 2022

sbueringer Mar 29, 2022 •

edited

Loading

sbueringer Mar 29, 2022 •

edited

Loading

killianmuldoon Mar 29, 2022 •

edited

Loading

sbueringer Mar 29, 2022 •

edited

Loading

sbueringer Apr 4, 2022 •

edited

Loading

sbueringer Apr 4, 2022 •

edited

Loading

sbueringer Apr 4, 2022 •

edited

Loading

sbueringer commented Jun 7, 2022 •

edited

Loading

vincepri commented Jun 9, 2022 •

edited

Loading

sbueringer commented Jun 9, 2022 •

edited

Loading

sbueringer commented Jun 13, 2022 •

edited

Loading