feat(presets): pinGitHubActionDigestsToSemver

[HonkingGoose]

@coderabbitai: ignore

Changes

• Create new helpers:pinGitHubActionDigestsToSemver preset

Context

Based on:

• Enable pinDigest to convert a major version to semver #21901
• docs(GHA tags pining): Describe vX -> vX.Y.Z conversion existance #23577

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

• Code inspection only, or
• Newly added/modified unit tests, or
• No unit tests but ran on a real repository, or
• Both unit tests + ran on a real repository

[suzuki-shunsuke]

Thank you for addressing my feature request!

[HonkingGoose]

@TWiStErRob can you give me new suggestions to resolve the comments? I don't know which suggestions I should apply... 🙈

[travi]

originally led to this approach by #23577 (comment), i thought i'd try to provide some thoughts from my experience after adding the work-around to my personal preset.

I think only some readers will want this information.

this is from the comment that i linked to above. while i think this specific quote was specifically about including the reference to the workaround alongside the docs for the pinGitHubActionDigests helper, i'm calling this out because i think i can really get behind the idea of this being closely related to pinning actions digests. i would maybe even go as far as suggesting that this should be default behavior of pinning digests and that the only reason i can think of for not choosing to do this is not knowing about it.

My goals with digests

my goal with pinning digests is to make sure that the version of the action referenced in git for a certain commit remains fully repeatable, so that i avoid the situation where a tag could be moved to a different commit. my assumption was that this would still reference specific releases, but adding this rule has helped me recognize that my expectations might not have aligned with the real behaviors. i value the visibility that the version comment adds so that i can understand the current state of things, but i did not expect that the comment might also be driving some behavior.

Scenarios with pinned digests

i can think of three scenarios that i've experienced since adding the pinning helper further back than this additional rule.

1. digest with a major version comment. often the result of switching to pinning for a version that i referenced the docs of an action to determine the version to reference
2. digest with a full semver comment. the result of the new rule when starting from a digest with a major version
3. digest with no comment. usually an accident. i update my yaml files programmatically often and with an AST tool that doesnt handle comments. i try to revert the dropping of the comments before committing the changes, but sometimes miss catching it

behavior observations

• one of the things that stood out to me the most after adding this rule was that i started getting release notes included in the PRs after getting to state 2. i had grown to assume that those simply werent available for actions updates, but i think it was more because of being in states 1 or 3.
• it seems like at least state 3 would send PRs for digest changes that dont align with released versions. i imagine these are real commits that dont end up triggering releases because they are changes in the repo that dont have consumer impact. if my understanding is correct. those updates are not valuable to me. i noticed this behavior when adding comments back in after noting they'd been lost in some places to get them back to either state 1 or 2. i was surprised that after adding the latest released version as a comment, i would get a PR realigning the digest with the actual version that was released. this surprised me because i thought the latest digest would have already been aligned with the latest released version.
• it became clear to me that renovate was better at getting the version and comment aligned than i was doing manually (part of the reason i saw value in the pin helper and this new rule in the first place), so it made me wish that renovate would add the comment back automatically in cases where i accidentally dropped it. if there was another config option to enable this behavior, i would absolutely enable it

collecting my thoughts

sorry for the lengthy context that somewhat hijacks this thread. happy to be redirected elsewhere if it would be valuable, but seemed related enough to this effort that i thought it would be valuable to start here.

what i'm building to is this. i think the addition of this helper will be a great addition. however, i think there might be something lost in thinking about implementing this that might deserve stepping back and considering from a higher level. my goal is to pin the digest for reproducibility and avoid potential attacks because of a tag being moved to a different commit. i still want to see the corresponding version, so the comment is valuable. i want it there always, even if i accidentally remove it. since i'm already pinning to an exact version through the digest, the version comment might as well also be as specific as possible.

to tie in the comment from above, i want to clarify that i want the pinning that happens to correspond to real releases, whatever that means. being fully semver compliant is less valuable than aligning to real releases. i dont know the details of the underlying implementation, but i'd love to see the updates be driven by the version comment rather than the digest. when a release happens, i want the version comment to be updated to the most specific available and the digest to align to that released version.

i think this proposed helper is a big step in the right direction and i'm eager to add it to some other projects, but i'm hoping that stepping back to consider the goal might help think through how to iterate forward.

[HonkingGoose]

Make pin to SemVer default behavior?

this is from the comment that i linked to above. while i think this specific quote was specifically about including the reference to the workaround alongside the docs for the pinGitHubActionDigests helper, i'm calling this out because i think i can really get behind the idea of this being closely related to pinning actions digests. i would maybe even go as far as suggesting that this should be default behavior of pinning digests and that the only reason i can think of for not choosing to do this is not knowing about it.

@travi Yes, that comment was about putting a load of text into a description field. The preset docs page is meant to only show short descriptions. And text in a description is harder to style properly.

@travi It sounds like you want to change the helpers:pinGitHubActionDigests preset, so it pins digests to their SemVer version, by default? So that users don't have to opt-in to another preset to get the actual best-practice behavior?

Here's the current code for the helpers:pinGitHubActionDigest preset:

{
  "packageRules": [
    {
      "matchDepTypes": [
        "action"
      ],
      "pinDigests": true
    }
  ]
}

About digest pinning

My goals with digests

my goal with pinning digests is to make sure that the version of the action referenced in git for a certain commit remains fully repeatable, so that i avoid the situation where a tag could be moved to a different commit. my assumption was that this would still reference specific releases, but adding this rule has helped me recognize that my expectations might not have aligned with the real behaviors. i value the visibility that the version comment adds so that i can understand the current state of things, but i did not expect that the comment might also be driving some behavior.

GitHub recommend that you pin third-party actions to a specific commit, to prevent bad actors from making a malicious release. As an added bonus, pinning to a specific commit also makes your CI more reproducible.

About the three states in your comment

I'll let one of the maintainer deal with this part, I'm not qualified to talk about the behavior or the code. 😄

About your behavior observations

it became clear to me that renovate was better at getting the version and comment aligned than i was doing manually (part of the reason i saw value in the pin helper and this new rule in the first place), so it made me wish that renovate would add the comment back automatically in cases where i accidentally dropped it. if there was another config option to enable this behavior, i would absolutely enable it

I guess Renovate doesn't have enough information based on just the commit hash what it should do:

1. Add comment to follow full SemVer version
2. Add comment to follow partial version like @v3
3. Do nothing
4. Let user configure "missing comment behavior" with a new config option whatToDoIfNoCommentFoundInGitHubActionFile
5. Something else

I don't know what happens if Renovate sees a commit hash without the tag comment, right now. I found some relevant text in the github-action manager docs ¹. I've bolded the key part:

The github-actions manager extracts dependencies from GitHub Actions workflow and workflow template files.

If you like to use digest pinning but want to follow the action version tag, you can use the following sample:

name: build

on: [push]

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@af513c7a016048ae468971c52ed77d9562c7c819 # v1.0.0

Renovate will update the commit SHA but follow the GitHub tag you specified.
Renovate can update digests that use SHA1 and SHA256 algorithms.

If you want to automatically pin action digests add the helpers:pinGitHubActionDigests preset to the extends array:

{
  "extends": ["helpers:pinGitHubActionDigests"]
}

About collecting my thoughts

my goal is to pin the digest for reproducibility and avoid potential attacks because of a tag being moved to a different commit. i still want to see the corresponding version, so the comment is valuable. i want it there always, even if i accidentally remove it. since i'm already pinning to an exact version through the digest, the version comment might as well also be as specific as possible.

Agreed.

to tie in the #23663 (comment), i want to clarify that i want the pinning that happens to correspond to real releases, whatever that means. being fully semver compliant is less valuable than aligning to real releases. i dont know the details of the underlying implementation, but i'd love to see the updates be driven by the version comment rather than the digest. when a release happens, i want the version comment to be updated to the most specific available and the digest to align to that released version.

I like your point of aliging to "real releases"!

HonkingGoose's summary of your idea

Checking in to see if I understand your comment correctly. I think this is the short summary of what you want?

1. Renovate to replace any missing comments.
2. Renovate to align to "real releases".
3. Renovate to always pin to the full SemVer version.
4. Renovate to keep you safe from malicious "release bad version on same Git tag" stuff

Please correct me if I'm wrong! 😉

Footnotes

1. https://docs.renovatebot.com/modules/manager/github-actions/#additional-information ↩

[rarkins]

What's an example change which this would make? Can someone give me an example of before and after?

[travi]

It sounds like you want to change the helpers:pinGitHubActionDigests preset, so it pins digests to their SemVer version, by default? So that users don't have to opt-in to another preset to get the actual best-practice behavior?

yes, i think there would be value in enabling this behavior by default. it does feel like a best-practice to me.

the one detail that i would clarify here is that i think the semver precision is more of a side-effect than the target behavior. i know this is repetitive with the similar point in the "collecting my thoughts" section, so this is just reinforcing that point. i dont fully understand the relationships that renovate is capable of managing, but it seems to me that aligning the comment to the most specific release version available is the best target regardless of whether that is fully semver or not. it is common for github actions to have to tracked versions, one with just the major (vx) and one that is a full semver (vx.y.z). however, some projects choose other patterns like major (vx) and vx.y. in a case like that, using the vx.y is valuable, even though it isnt a full vx.y.z.

Checking in to see if I understand your comment correctly. I think this is the short summary of what you want?

1. Renovate to replace any missing comments.
2. Renovate to align to "real releases".
3. Renovate to always pin to the full SemVer version.
4. Renovate to keep you safe from malicious "release bad version on same Git tag" stuff

Please correct me if I'm wrong! 😉

#3 is the only one that i'd again possibly question the wording of. like above, i think i'd land somewhere closer to "most specific version available from a real release". i think we are basically saying the same thing, but i think there are real situations where a full semver version doesnt exist in a real release, so i think this rewording mostly helps to define the behavior in the less common scenarios where a full semver release doesnt exist

overall, i think you've captured my point well

[travi]

What's an example change which this would make? Can someone give me an example of before and after?

if i'm understanding your question correctly, these are examples of the proposed preset making the version comment more specific:

• form8ion/project@5e37ee0
• form8ion/project@a36a50c

in case it is helpful, this is an example of manually re-adding an accidentally removed version comment and the steps renovate currently follows to get back to a good state:

• the re-add of the version comment(s)
• get the digest back in sync with the major version tag (i'm fairly confident this was because the digest matched a commit that was ahead of the position of the v3 tag)
• update the version comment to match the more specific version

do these examples clarify what you were asking about?

[rarkins]

Thanks, that helps to clarify. The 3 -> 3.1.4 part is what we'd normally refer to as pinning versions, so the "digests" part in the title here threw me off (and maybe needs reconsideration).

Let's consider two similar scenarios:

1. You have no SHA pinned digest, and use a "range" version like v3
2. Same as (1) but you have a SHA (and the version is in a comment like # v3)

For (1), going from v3 to v3.1.4 does make a difference to future behavior. With v3 alone, you wouldn't get any PR until a v4 is one day released. For (2) you'll get PRs any time the v3 tag is updated to point to a different SHA, which is quite likely to be any time a new semver release comes out (e.g. v3.1.5). So in (2), you are already going to get a Renovate PR approximately every time there's a new semver release anyway and pinning to v3.1.4 does not cause more noise. On the upside, it will allow you to get release notes and more transparency about what version you're using.

In other words:

• If no SHA exists, and pinDigests=false, leave it alone
• If a SHA exists, but no version comment, then add it
• If a SHA exists and version comment, but it's not pinned to semver, then pin it

Is the above aligned with your thinking too?

[HonkingGoose]

What's the next steps here?

Somebody should check if the regex that's currently in my PR will do this:

My preferred functionality (copy pasted from earlier) is this:

* If no SHA exists, and pinDigests=false, leave it alone

* If a SHA exists, but no version comment, then add it

* If a SHA exists and version comment, but it's not pinned to semver, then pin it

I'm surprised and happy if this can already be done using a preset alone

[rarkins]

Can some examples of this in action be shared?

[HonkingGoose]

Can some examples of this in action be shared?

@rarkins I see that @travi tried some code on their own repository:

• ci(renovate): configured renovate to pin actions version comments semantic-release/.github#20

[TWiStErRob]

@rarkins, I think you're looking for these: org:semantic-release is:pr created:2023-10-23 "update actions".
The PR mentioned by @HonkingGoose was merged 23rd 18:46. Major updates (e.g. node v4) seem to contain the version number before the change too, but for example these two look like the consequence of this change:

• ci(action): update actions/setup-node action to v3.8.2 semantic-release/github#726
• ci(action): update actions/checkout action to v4.1.1 semantic-release/github#725

Weird thing is that this is also after the merge:

• ci(action): update actions/setup-node digest to 1a4442c semantic-release/github#724

[rarkins]

I'm not sure exactly how/why this works, but it's opt-in/optional for users so I'm happy to merge it and let people try it

[MPV]

What remains to resolve this?
Looks like "only" an unresolved conversation to me.
@TWiStErRob would you be okay with this PR now (given that you raised the still open discussions)?

[TWiStErRob]

@MPV that mega-thread is probably 3 conversations away from the original which was "resolved" in August: https://github.com/renovatebot/renovate/pull/23663/files#r1299844474, only collaborators and PR author can resolve conversations. I'm lost in all the follow-ups :)

Ignoring all the convos, the code make sense in the PR. I disagree with with only supporting vx and vx.y.z, but that was the maintainer's decision, so I have to accept.

[renovate-release]

🎉 This PR is included in version 37.119.0 🎉

The release is available on:

• GitHub release
• 37.119.0

Your semantic-release bot 📦🚀