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

Update version comments for SHA-pinned GitHub Actions #5951

Merged

Conversation

jproberts
Copy link
Contributor

GitHub encourages pinning third-party GitHub Actions to a full length commit SHA. It's common for actions pinned by commit SHA to include a comment specifying the version associated with the commit. For example:

- uses: actions/checkout@01aecc # v2.1.0

This change updates the GitHub Actions manager to bump versions in comments that follow SHA-pinned actions, so the comment stays up-to-date with the SHA being updated.

The file_updater now searches the comment string for all references to the previous version and replaces them with the new version. To avoid changing unrelated comments, the comment updater only updates dependencies that pin SHA refs.


Note
I'm not an experienced Ruby developer and I welcome any suggestions to improve the code. My main goal was to make a minimal change and follow the existing code conventions.

Closes #4691

@deivid-rodriguez
Copy link
Contributor

@jproberts Thanks so much for doing this! The PR looks pretty neat and seems to have extensive test coverage 💪. I'll hope to get to do a proper review of this PR soon-ish unless someone else beat me to it, but I wanted to drop a big THANKS for now. Also pretty nice detail to open a PR to your fork first and get a colleague to review it 🥇.

Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

This looks fantastic to me, very well done.

Well done writing this flexibly enough to handle all the permutations people use for specifying version strings.

- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # tag=v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0
- uses: actions/checkout@01aecc # v2.1.0
Copy link
Member

Choose a reason for hiding this comment

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

Major 💖 for phenom set of test cases.

Two more cases:

  1. What about the case where a comment includes the version, but then some text?
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # Versions older than v2.1.0 have a security vulnerability
  1. What if the version string is repeated twice?
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 - Versions older than v2.1.0 have a security vulnerability

I don't want to throw up roadblocks on this awesome PR, so if for simplicity neither case is bumped, that's perfectly acceptable. So one simple heuristic could be if there's any text after the version string, then don't bump the version string... Users that still want it bumped can instead leave their comment as a full-line comment above the actual code line:

# Versions older than v2.1.0 have a security vulnerability
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0

But we do need to ensure we never auto-bump the comment to start incorrectly saying "Versions older than v2.2.0 have a security vulnerability"...

So can you add these two test cases to ensure no future regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I was worried the simple search-and-replace was too broad but I couldn't come up with a sample comment where we wouldn't want to bump the version. The cases you mentioned are exactly what I was looking for!

I've updated the PR with the simple fix to only change comments if there's no text after the version string.

GitHub encourages pinning third-party GitHub Actions to a full
length commit SHA. It's common for actions pinned by commit SHA
to include a comment specifying the version associated with the
commit. For example:

    - uses: actions/checkout@01aecc # v2.1.0

This change updates the GitHub Actions manager to bump versions
in comments that follow SHA-pinned actions, so the comment stays
up-to-date with the SHA being updated.

The file_updater now searches the comment string for all references
to the previous version and replaces them with the new version. To
avoid changing unrelated comments, the comment updater only
updates dependencies that pin SHA refs.
If the version in a comment is followed by additional text, it
could be documentation explaining some behavior, rather than a tag
corresponding to the SHA in the action version. To be safe, don't
update the version unless it's the end of the comment.
@jproberts jproberts force-pushed the github-actions-update-semver-comments branch from 96b107c to a459c88 Compare October 31, 2022 00:53
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

Looks great to me.

Had one minor pedantic suggestion about tweaking the description of intended behavior, but I'm also fine to merge as-is... up to you.

We should be able to get this deployed/merged in the next few days...

@jeffwidman
Copy link
Member

For the CI failure here, it's probably best to just disable the Rubocop line-length check for that block of code:
https://github.com/dependabot/dependabot-core/actions/runs/3357784436/jobs/5564093603

Example of how to do that here:
08c679d#diff-ee98e028c59b193d58fde56ab4daf54d43c486ae674e63d50ddf300b07943e0fR57

Just don't forget to re-enable it after the block as demonstrated in the above example as well.

@jeffwidman
Copy link
Member

FYI @jproberts this got a nice shout out on the blog: https://github.blog/changelog/2022-10-31-dependabot-now-updates-comments-in-github-actions-workflows-referencing-action-versions/

@jproberts
Copy link
Contributor Author

I just saw the post and I'm amazed. Thanks so much for your support @jeffwidman ❤️

@jproberts jproberts deleted the github-actions-update-semver-comments branch November 1, 2022 00:02
@laurentsimon
Copy link

Congrats on the post!

Is this feature compatible with renovatebot's way of handling comments? Renovatebot expects a comment of the form # tag=v1.2.3

It would be useful for the various update tools to be compatible on this point. Wdut?

/cc @rarkins from renovatebot

@rarkins
Copy link

rarkins commented Nov 1, 2022

I have created renovatebot/renovate#18640 recently to align Renovate with this approach. As described in that issue, I have concerns about assuming any comment represents a tag (e.g. # do not change this) so we will make sure it's version-ish before treating it as a tag. I think it would have been better to adopt the explicit # tag= approach for everyone but it's likely too late for that.

@jeffwidman
Copy link
Member

jeffwidman commented Nov 1, 2022

Thanks for swinging by. Agreed that getting the update tools aligned on the implicit format spec would be great.

The # @v2.1.0 format is so intuitive that I personally prefer it, despite the ambiguity compared to # tag=v2.1.0.

Thankfully @jproberts was thorough and already included unit tests for all the formats I've seen in the wild:

- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # 2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # @v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # pin @v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # tag=v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0
# The comment on the next line has a trailing tab. The version should still be updated.
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0
- uses: actions/checkout@01aecc # v2.1.0
integration:
- uses: actions/checkout@v2.1.0 # comments that include the version (v2.1.0) shouldn't be updated for non-SHA refs
- uses: actions/checkout@01aecc#v2.1.0 # this shouldn't be updated, because the version is part of the ref, not a comment.
# The version in the comment for the next action shouldn't be updated
# because it refers to past behavior.
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # Versions older than v2.1.0 have a security vulnerability
# The versions in the comment for the next action won't be updated.
# The first version could be updated, but it's difficult to create
# a heuristic that recognizes the first version as a version alias
# for the SHA commit, and the second version as a concrete version
# that shouldn't change. For simplicity, we don't update either.
- uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 - Versions older than v2.1.0 have a security vulnerability

@rarkins you might find the whole set of examples useful to test against, particularly the negative tests. If you come up with additional edge cases we missed, feel free to circle back and let us know.

leoamigood pushed a commit to leoamigood/dependabot-core that referenced this pull request Nov 2, 2022
GitHub encourages pinning third-party GitHub Actions to a full
length commit SHA. It's common for actions pinned by commit SHA
to include a comment specifying the version associated with the
commit. For example:

    - uses: actions/checkout@01aecc # v2.1.0

This change updates the GitHub Actions manager to bump versions
in comments that follow SHA-pinned actions, so the comment stays
up-to-date with the SHA being updated.

The file_updater now searches the comment string for all references
to the previous version and replaces them with the new version. To
avoid changing unrelated comments, the comment updater only
updates dependencies that pin SHA refs.

If the version in a comment is followed by additional text, it
could be documentation explaining some behavior, rather than a tag
corresponding to the SHA in the action version. To be safe, don't
update the version unless it's the end of the comment.
@kuhnroyal
Copy link

Thanks for this, saves a lot of time!

@crenshaw-dev
Copy link

Do I need to do anything to opt in to this? Not seeing the version update: https://github.com/argoproj/argo-cd/pull/11480/files

sunjayBhatia added a commit to sunjayBhatia/contour that referenced this pull request Jan 3, 2024
Dependabot should be able to update these and preserve the tag in the comment, as per: dependabot/dependabot-core#5951

Workflows updated with: https://app.stepsecurity.io/secureworkflow

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
sunjayBhatia added a commit to sunjayBhatia/contour that referenced this pull request Jan 3, 2024
Dependabot should be able to update these and preserve the tag in the comment, as per: dependabot/dependabot-core#5951

Workflows updated with: https://app.stepsecurity.io/secureworkflow

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
sunjayBhatia added a commit to projectcontour/contour that referenced this pull request Jan 3, 2024
Dependabot should be able to update these and preserve the tag in the comment, as per: dependabot/dependabot-core#5951

Workflows updated with: https://app.stepsecurity.io/secureworkflow

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
huwd added a commit to govuk-one-login/tech-docs that referenced this pull request Feb 7, 2024
Danger of enabling SHA pinning for actions is that they're not
particualrly readable and so we could strand versions and not get
legitimate updates.

DepBot now supports updates for pinned actions where there is the
version number in a comment afterwards (as the tool linked in previous
commits provides automatically).

So let's enable DepBot scans, initially set at weekly.

See:
dependabot/dependabot-core#5951
https://docs.github.com/en/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot
https://github.blog/changelog/2022-10-31-dependabot-now-updates-comments-in-github-actions-workflows-referencing-action-versions/
huwd added a commit to govuk-one-login/tech-docs that referenced this pull request Feb 8, 2024
Danger of enabling SHA pinning for actions is that they're not
particualrly readable and so we could strand versions and not get
legitimate updates.

DepBot now supports updates for pinned actions where there is the
version number in a comment afterwards (as the tool linked in previous
commits provides automatically).

So let's enable DepBot scans, initially set at weekly.

See:
dependabot/dependabot-core#5951
https://docs.github.com/en/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot
https://github.blog/changelog/2022-10-31-dependabot-now-updates-comments-in-github-actions-workflows-referencing-action-versions/
huwd added a commit to alphagov/gds-way that referenced this pull request Feb 8, 2024
SHA pinning is a sensible approach to mitigate potential supply chain
attacks. See some great blog posts here on the approach:

https://blog.rafaelgss.dev/why-you-should-pin-actions-by-commit-hash
also
https://michaelheap.com/improve-your-github-actions-security/#using-pin-github-action

However one down side is that SHA's are not very human readable. It can
be difficult to tell from the SHA if the version we've pinned has an
update, or if that update is a security or important fix.

Best practice therefore is to place a comment after the pinned version
listing the semantic version for a third party github action.

This gets you best of both worlds, maintainability plus certainty.

It might look at bit like this:

```
jobs:
  check-pull-request:
    runs-on: ubuntu-latest
    steps:
      - name: Check out repository code
	uses: actions/checkout@ee066bloop # pin @v2
      - name: Install Ruby uses:
	ruby/setup-ruby@22acsewblah # pin@v1
```

Consistency here also helps us manage this code in line with the GDS Way
requirement to Update dependencies frequently when managing third party
dependencies:
https://gds-way.digital.cabinet-office.gov.uk/standards/tracking-dependencies.html#update-dependencies-frequently

Since October 2022 DependaBot will now look for comments on SHA pinning
and automatically suggest updates. Similar approaches may be possible
for other dependency management tools.

Dependabot currently supports a range of different comment syntaxses
which can be viewed here:
dependabot/dependabot-core#5951 (comment)

I've tried to keep the guidance general and open, leaving detail to this
commit history, given the range of different tools on use across GDS.

The principles are:

- Pin your actions using SHAs
- Ensure human readability by commenting the semver on the line with the
  action
- Explore if your usual dependency management process, especially
  automated ones like DependaBot can help flag and raise visibility on
  new versions.
huwd added a commit to alphagov/gds-way that referenced this pull request Feb 9, 2024
SHA pinning is a sensible approach to mitigate potential supply chain
attacks. See some great blog posts here on the approach:

https://blog.rafaelgss.dev/why-you-should-pin-actions-by-commit-hash
also
https://michaelheap.com/improve-your-github-actions-security/#using-pin-github-action

However one down side is that SHA's are not very human readable. It can
be difficult to tell from the SHA if the version we've pinned has an
update, or if that update is a security or important fix.

Best practice therefore is to place a comment after the pinned version
listing the semantic version for a third party github action.

This gets you best of both worlds, maintainability plus certainty.

It might look at bit like this:

```
jobs:
  check-pull-request:
    runs-on: ubuntu-latest
    steps:
      - name: Check out repository code
	uses: actions/checkout@ee066bloop # pin @v2
      - name: Install Ruby uses:
	ruby/setup-ruby@22acsewblah # pin@v1
```

Consistency here also helps us manage this code in line with the GDS Way
requirement to Update dependencies frequently when managing third party
dependencies:
https://gds-way.digital.cabinet-office.gov.uk/standards/tracking-dependencies.html#update-dependencies-frequently

Since October 2022 DependaBot will now look for comments on SHA pinning
and automatically suggest updates. Similar approaches may be possible
for other dependency management tools.

Dependabot currently supports a range of different comment syntaxses
which can be viewed here:
dependabot/dependabot-core#5951 (comment)

I've tried to keep the guidance general and open, leaving detail to this
commit history, given the range of different tools on use across GDS.

The principles are:

- Pin your actions using SHAs
- Ensure human readability by commenting the semver on the line with the
  action
- Explore if your usual dependency management process, especially
  automated ones like DependaBot can help flag and raise visibility on
  new versions.
huwd added a commit to alphagov/gds-way that referenced this pull request Apr 23, 2024
SHA pinning is a sensible approach to mitigate potential supply chain
attacks. See some great blog posts here on the approach:

https://blog.rafaelgss.dev/why-you-should-pin-actions-by-commit-hash
also
https://michaelheap.com/improve-your-github-actions-security/#using-pin-github-action

However one down side is that SHA's are not very human readable. It can
be difficult to tell from the SHA if the version we've pinned has an
update, or if that update is a security or important fix.

Best practice therefore is to place a comment after the pinned version
listing the semantic version for a third party github action.

This gets you best of both worlds, maintainability plus certainty.

It might look at bit like this:

```
jobs:
  check-pull-request:
    runs-on: ubuntu-latest
    steps:
      - name: Check out repository code
	uses: actions/checkout@ee066bloop # pin @v2
      - name: Install Ruby uses:
	ruby/setup-ruby@22acsewblah # pin@v1
```

Consistency here also helps us manage this code in line with the GDS Way
requirement to Update dependencies frequently when managing third party
dependencies:
https://gds-way.digital.cabinet-office.gov.uk/standards/tracking-dependencies.html#update-dependencies-frequently

Since October 2022 DependaBot will now look for comments on SHA pinning
and automatically suggest updates. Similar approaches may be possible
for other dependency management tools.

Dependabot currently supports a range of different comment syntaxses
which can be viewed here:
dependabot/dependabot-core#5951 (comment)

I've tried to keep the guidance general and open, leaving detail to this
commit history, given the range of different tools on use across GDS.

The principles are:

- Pin your actions using SHAs
- Ensure human readability by commenting the semver on the line with the
  action
- Explore if your usual dependency management process, especially
  automated ones like DependaBot can help flag and raise visibility on
  new versions.
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.

For actions that are pinned-by-hash, bump the human readable version number in the code comment
7 participants