[rarkins]

Hi, Renovate author/maintainer here.

The affected repo has now been taken down, so I am writing this partly from memory, but I believe the scenario is:

1. An attacker had write access to the tj-actions/changed-files repo

2. The attacker chose to spoof a Renovate commit, in fact they spoofed the most recent commit in the same repo, which came from Renovate

3. Important: this spoofing of commits wasn't done to "trick" a maintainer into accepting any PR, instead it was just to obfuscate it a little. It was an orphan commit and not on top of main or any other branch

4. As you'd expect, the commit showed up as Unverified, although if we're being realistic, most people don't look at that or enforce signed commits only (the real bot signs its commits)

5. Kind of unrelated, but the "real" Renovate Bot - just like Dependabot presumably - then started proposing PRs to update the action, like it does any other outdated dependency

6. Some people had automerging of such updates enabled, but this is not Renovate's default behavior. Even without automerging, an action like this might be able to achieve its aim only with a PR, if it's run as part of PR builds

7. This incident has reminded that many people mistakenly assume that git tags are immutable, especially if they are in semver format. Although it's rare for such tags to be changed, they are not immutable by design

[srvaroa]

>7. This incident has reminded that many people mistakenly assume that git tags are immutable, especially if they are in semver format. Although it's rare for such tags to be changed, they are not immutable by design

IME, this will be more "learned" than "reminded". Many many people set up pipelines to build artefacts based on tags (e.g. a common practise being "on tag with some pattern, then build artefact:$tag") and are just surprised if you call out the flaws.

It's one of many practises adopted because everyone does it but without basic awareness of the tradeoffs. Semver is another similar case of inherited practise, where surprisingly many people seem to believe that labelling software with a particular string magically translates into hard guarantees about its behaviour.

[junon]

I theorized about this vulnerability a while back when I noticed new commits didn't disable automerging. This is an insane default from GH.

EDIT: seems GitHub has finally noticed (or started to care); just went to test this and auto merge has been seemingly disabled sitewide. Even though the setting is enabled, no option to automerge PRs shows up.

Seems I was right to worry!

EDIT2: We just tested this on GitLab's CI since they also have an auto-merge function and it appears they've done things correctly. Auto-merge enablement is only valid for the commit for which it was enabled; new pushes disable auto-merge. Much more sensible and secure.

[nine_k]

Tags can be signed, and the signature can be verified. It's about as easy as signing / verifying commits. One can even make signing tags as the default option when creating tags.

This won't help in this case though, because a legitimate bot was tricked into working with a rogue commit; a tricked bot could as well sign a tag with a legitimate key.

"Immutable tags" of course exist, they are commit hashes, but they are uninformative :(

[klysm]

How else should we do it?

[diggan]

> 6. Some people had automerging of such updates enabled, but this is not Renovate's default behavior. Even without automerging, an action like this might be able to achieve its aim only with a PR, if it's run as part of PR builds

I'm not sure how this could exploited by just making a PR, unless you for some reason have secrets enabled for builds by unknown contributors, which obviously would be a mistake. Usually, only builds using secrets only run on certain branches which has a known contributor approving the code before it gets there.

> people mistakenly assume that git tags are immutable

If you're distributing a library on GitHub used by many other people/projects, then you really need to setup `protected branches` and `protected tags`, where you can prevent changes somewhat.

[semiquaver]

  > I'm not sure how this could exploited by just making a PR, unless you for some reason have secrets enabled for builds by unknown contributors

In this context the renovate bot would be making the PR to a repo it had been installed on, making it a trusted contributor able to trigger CI builds on its PRs.

[jonenst]

Neither Branch Protection nor the newer Rulesets allow to protect secrets from someone with push acces to the repo. From what I understand, only environment secrets provide this feature (and have the drawback that you can't share them among multiple repos in the same org without copying them everywhere, although you can script the copying with the github api)

[mlor]

Thanks for taking the time to comment. Not that it wasn't there before this, but this incident highlights a lot to take into consideration with respect to securing one's supply chain going forward.

[afitnerd]

Thanks for this writeup! It seems like #1 was the real weakness. Have you identified how the attacker was able to get write access to tj-actions/changed-files? Did this discovery result in any changes to how people can contribute to the project?

[3np]

So I follow steps 1-4 but don't see what triggered step 5, if they indeed just did an orphan commit outside of releases or main branch?