3 ms·

Accidentally breaking GitHub Actions with a shorthash collision (2019)

26 points by fanf2 2y ago

thangngoc89 2y ago
Thank you for a very detailed write up. Here I am using shorthash for Docker image tags. I guess I will fix this asap.
- demurgos 2y ago
  Ideally Docker would support a lockfile format so you don't have to manually manage the hashes. I'm still surprised how commonly people use named Docker tags when Docker Hub let's you mutate the corresponding image. In the meantime, the article is right that you should use the full-length digests for situations where you care about reproducibility.
  - thangngoc89 2y ago
    Anytime you run apt install/apk install, your Docker image became unreproducible. I mainly use shorthash for easy manual inspection. The Dockerhub is set to clean up images older than 2 months so the risk if shorthash collision is very low (but not impossible)
mook 2y ago
(2019), while Actions was in beta using a different syntax. Might still be interesting to read though.
mdaniel 2y ago
(2019)
waiwai933 2y ago
I'd simplify the guidance on using full/short hashes to: * Automated use? Always use full hashes * Human use? Short hashes are usually fine, but a full hash doesn't hurt (and please make it available in a link/hovertext)
- cesarb 2y ago
  I'd simplify the guidance even more: short hashes are fine if and only if the repository hasn't been modified since they were generated (AFAIK, git automatically increases the length of the short hash it displays whenever it would otherwise cause a collision).
  - waiwai933 2y ago
    I'm not sure that's necessarily practical though - if I'm dropping a link in Slack to a colleague > You want to look at [a123456](some.git.url/a12345678...) to see where we've done this before I can't promise that the repo won't have changed by the time they look at it (but because the full hash is linked, it's not a problem in practice).
TheRealPomax 2y ago
This needs a [2019] in the title.
cesarb 2y ago
It doesn't seem to have been mentioned in this article, but AFAIK git detects when a collision happens, and automatically generates a longer shorthash for that commit. Of course, that doesn't fix shorthashes generated before the colliding object was added.
ackfoobar 2y ago
From the same author: https://github.com/not-an-aardvark/lucky-commit https://github.com/not-an-aardvark/lucky-commit a tool for generating commits with specific shorthashes.
resource_waste 2y ago
Is anyone actually building more on Microsoft services? I have been withdrawing as much stuff as possible from anything Microsoft owns. The writing is on the wall with their abuse of customers on Windows 11. Microsoft gives me AOL vibes, where the ancient customers will be squeezed until the end.
deleted 2y ago
[deleted]
orf 2y ago
I remember when GitHub actions was based on HCL. They switched to YAML pretty quickly and have never talked much about why. I always thought HCL was a fairly smart/interesting choice, and I’ve always wondered if they backed out because users were unfamiliar with HCL, the ecosystem wasn’t ready for HCL, or something else.
deleted 2y ago
[deleted]
twic 2y ago
> To get the collision to affect anything on GitHub, I needed to push it to the actions/docker repo. This posed a problem, because I didn’t have write access to the actions/docker repo. However, I realized I could get around that issue by forking the actions/docker repo and pushing a commit to my fork (since GitHub shares commits between forks and parent repositories). It does what?!?! So if i have a repo which does not contain commit abcdef, and some random evil bastard forks it, and adds a commit abcdef, then users can ask for commit abcdef from my repository, and get the commit the evil guy added? That seems surprising and bad!
- ackfoobar 2y ago
  I tried it (with short/full commit hash). GitHub gives a warning: > This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

26 points by fanf2 2y ago

thangngoc89 2y ago

Thank you for a very detailed write up. Here I am using shorthash for Docker image tags. I guess I will fix this asap.

demurgos 2y ago
Ideally Docker would support a lockfile format so you don't have to manually manage the hashes. I'm still surprised how commonly people use named Docker tags when Docker Hub let's you mutate the corresponding image. In the meantime, the article is right that you should use the full-length digests for situations where you care about reproducibility.
- thangngoc89 2y ago
  Anytime you run apt install/apk install, your Docker image became unreproducible. I mainly use shorthash for easy manual inspection. The Dockerhub is set to clean up images older than 2 months so the risk if shorthash collision is very low (but not impossible)

mook 2y ago

(2019), while Actions was in beta using a different syntax. Might still be interesting to read though.

mdaniel 2y ago

(2019)

waiwai933 2y ago

I'd simplify the guidance on using full/short hashes to: * Automated use? Always use full hashes * Human use? Short hashes are usually fine, but a full hash doesn't hurt (and please make it available in a link/hovertext)

cesarb 2y ago
I'd simplify the guidance even more: short hashes are fine if and only if the repository hasn't been modified since they were generated (AFAIK, git automatically increases the length of the short hash it displays whenever it would otherwise cause a collision).
- waiwai933 2y ago
  I'm not sure that's necessarily practical though - if I'm dropping a link in Slack to a colleague > You want to look at [a123456](some.git.url/a12345678...) to see where we've done this before I can't promise that the repo won't have changed by the time they look at it (but because the full hash is linked, it's not a problem in practice).

TheRealPomax 2y ago

This needs a [2019] in the title.

cesarb 2y ago

It doesn't seem to have been mentioned in this article, but AFAIK git detects when a collision happens, and automatically generates a longer shorthash for that commit. Of course, that doesn't fix shorthashes generated before the colliding object was added.

ackfoobar 2y ago

From the same author: https://github.com/not-an-aardvark/lucky-commit https://github.com/not-an-aardvark/lucky-commit a tool for generating commits with specific shorthashes.

resource_waste 2y ago

Is anyone actually building more on Microsoft services? I have been withdrawing as much stuff as possible from anything Microsoft owns. The writing is on the wall with their abuse of customers on Windows 11. Microsoft gives me AOL vibes, where the ancient customers will be squeezed until the end.

deleted 2y ago

[deleted]

orf 2y ago

I remember when GitHub actions was based on HCL. They switched to YAML pretty quickly and have never talked much about why. I always thought HCL was a fairly smart/interesting choice, and I’ve always wondered if they backed out because users were unfamiliar with HCL, the ecosystem wasn’t ready for HCL, or something else.

deleted 2y ago

[deleted]

twic 2y ago

> To get the collision to affect anything on GitHub, I needed to push it to the actions/docker repo. This posed a problem, because I didn’t have write access to the actions/docker repo. However, I realized I could get around that issue by forking the actions/docker repo and pushing a commit to my fork (since GitHub shares commits between forks and parent repositories). It does what?!?! So if i have a repo which does not contain commit abcdef, and some random evil bastard forks it, and adds a commit abcdef, then users can ask for commit abcdef from my repository, and get the commit the evil guy added? That seems surprising and bad!

ackfoobar 2y ago
I tried it (with short/full commit hash). GitHub gives a warning: > This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.