Make commit hashes account for versions in comments

[jauderho]

Dependabot Core just added support for this. See dependabot/dependabot-core#4691

This is more to make sure that secure-workflows tracks this accordingly when doing the initial substitution to a hash. Ideally, the initial substitution will include the version comment for Dependabot to update downstream.

[varunsh-coder]

Thanks @jauderho for informing that dependabot implemented this. Will test it out and add support for this ASAP.

[lucacome]

I believe there's already an issue about this #1087.
Waiting for this feature to come back before updating all of my workflows 🙂

[varunsh-coder]

Sorry, I had forgotten that there was already an issue tracking this. No problem. We can close both once this is done.

@Devils-Knight, can you please take this up? You can do a quick POC to see how dependabot updates the version/ tag in the comments, and then bring back the earlier code that was adding the version/ tag in comment as part of pinning.

[varunsh-coder]

Thanks @Devils-Knight! This is now deployed to INT environment.

@jauderho and @lucacome can you please test it here (https://int1.stepsecurity.io) and let me know if it looks ok? There is a single space between commit SHA and # and then a single space between # and the tag.

Also tagging @n-th, @ericcornelissen, @tellison, and @jkremser from issue #1087 in case you want to share feedback on the change.

Thanks!

[ericcornelissen]

Two quick thoughts:

1. Has this been tested with Renovate (either now or in the past)? I'm asking because workflows in repositories where I use Renovate have a comment that looks like {SHA} # tag=vX.Y.Z and in repositories where I use Dependabot {SHA} # vX.Y.Z - so with or without tag=. I'm not actually sure if this is necessary for Renovate to work.
2. This is probably more complicated to implement, but I think in an ideal scenario something like uses: action@v2 is replaced by uses: action@{SHA} # v2.Y.Z (with minor and patch numbers).

[tellison]

2. This is probably more complicated to implement, but I think in an ideal scenario something like uses: action@v2 is replaced by uses: action@{SHA} # v2.Y.Z (with minor and patch numbers).

+1, the name of the specific release would be preferable.

[lucacome]

I agree, minor and patch numbers would be nice 👍

[varunsh-coder]

Thanks for the feedback!

1. Looks like renovate bot supports this format: # tag=vX.Y.Z and dependabot also supports it. So, should we use this format? That way it will work for both renovatebot and dependabot. In cases where one submits a workflow file, just based on that file, we don't know whether renovatebot or dependabot is being used.

2. I think we should be able to get from vX -> (current) vX.Y.Z. So this should be doable.

[lucacome]

I keep going back and forth abouttag=, it feels unnecessary to me, but if it’s the only format recognized by both bots then I guess we should go for it.

[ericcornelissen]

Looks like renovate bot supports this format: # tag=vX.Y.Z and dependabot also supports it. So, should we use this format? [...]

I received a Pull Request from Renovate on one of my repositories today where it actually remove the "tag=" prefix upon updating an action 😅

The diff looks like:

-        uses: gitleaks/gitleaks-action@1938557f6a58837331b99822ab17b8e536e7bef9 # tag=v2.3.0
+        uses: gitleaks/gitleaks-action@e7168103501562d92f3f52e2c69c253cff74438d # v2.3.1

[varunsh-coder]

Thanks @ericcornelissen for the info!

I think this is the issue renovatebot/renovate#18640

We can do the short tag format as earlier then.

@Devils-Knight is working on converting vX -> (current) vX.Y.Z. I will update this thread when it is deployed to INT environment.

[varunsh-coder]

The change for converting vX -> (current) vX.Y.Z is now in INT.

Please check it out here: https://int1.stepsecurity.io
Here is an example:
https://int1.stepsecurity.io/secureworkflow/adoptium/temurin-build/build.yml/master?enable=pin

Please let me know if it looks ok.

[lucacome]

Looks good @varunsh-coder! Can’t wait to try it on my workflows.

[tellison]

@varunsh-coder thanks for adding this new functionality. I've had a play with it on our actions.

When I run on this repo:

https://int1.stepsecurity.io/secureworkflow/adoptium/temurin-build/ca-cert-updater.yml/master?enable=pin

The line 21 diff suggests:
- uses: gr2m/create-or-update-pull-request-action@dc1726cbf4dd3ce766af4ec29cfb660e0125e8ee # v1

Which "v1" hash has been selected from the g2rm/create-or-update-pull-request-action releases?

Justs eyeballing those releases I don't see one starting dc1726c. I'm sure the tool didn't just make that up so I'm curious which version the action is using! As I review the hash replacements I'm also checking our actual version usage.

[varunsh-coder]

@tellison this is an interesting case. Looks like gr2m/create-or-update-pull-request-action has a v1 branch, but no v1 tag. So gr2m/create-or-update-pull-request-action@v1 converts to gr2m/create-or-update-pull-request-action@dc1726cbf4dd3ce766af4ec29cfb660e0125e8ee # v1 where the v1 refers to the branch and the dc1726cbf4dd3ce766af4ec29cfb660e0125e8ee is the commit in that branch.

For reference, this code is used to get the commit:
https://github.com/step-security/secure-workflows/blob/267e16e2441ba3a584739c34bc1063846eb261a9/remediation/workflow/pin/pinactions.go#L66

[varunsh-coder]

This change has now been merged into main and released to PROD. You can check it here: https://app.stepsecurity.io

Please let me know if you find any issues.

[varunsh-coder]

Thanks everyone for your inputs the feedback on this issue!
Since this is done, I will go ahead and close this and #1087.
Feel free to re-open if something is not right, or create a new issue.

We also recently added support for creating or updating dependabot config and adding CodeQL (if missing). They show up as options at https://app.stepsecurity.io/securerepo
Please do create issues if something is not working as expected or if you have other ideas to reduce steps to automate remediation for Scorecard checks.