18 ms·

Git archive checksums may change

245 points by mcovalt 4y ago

rektide 4y ago
Now please give us compression options beyond gzip? :) Some zstd & lz4 please?
wildfire 4y ago
See https://github.com/orgs/community/discussions/45830 https://github.com/orgs/community/discussions/45830 for the fallout.
doubleunplussed 4y ago
Ah, this will presumably break some Arch Linux AUR packages. Preparing for bug reports.
- frankjr 4y ago
  Yep, it has already broken labwc for me. ==> Validating source files with b2sums... labwc-0.6.1.tar.gz ... FAILED ==> ERROR: One or more files did not pass the validity check!
- elesiuta 4y ago
  I always anticipated something like this could happen and it bothered me enough to create my own workflow [1] to archive, hash, and attach it to each release automatically for my AUR package. I can see how most people wouldn't notice/bother with such a small detail though, so I am not at all surprised by the fallout this caused. [1] https://github.com/elesiuta/picosnitch/blob/master/.github/workflows/tar-publish.yml https://github.com/elesiuta/picosnitch/blob/master/.github/w...
swarfield 4y ago
They have broken almost every open source project that builds external deps. Also broke homebrew apparently.
- groestl 4y ago
  > every open source project that builds external deps and relies on checksumming ephemeral artefacts for integrity.
  - pxc 4y ago
    Such tools should definitely checksum package sources lol
  - catiopatio 4y ago
    Source archives have never, in the entire history of open source, been considered ephemeral. GitHub unilaterally made that decision for their own convenience, and violated a decades-long universal community norm in the process.
    - mardifoufs 4y ago
      I think this change only affects automatically (and dynamically) generated source archives, not those that are actually pushed to Github Releases beforehand.
    - Denvercoder9 4y ago
      You could also say that some maintainers made that decision for their convenience of not having to build and upload source archives. It is possible to upload your own artifacts to a release on GitHub, and lots of projects do. Those are correctly treated as immutable by GitHub.
      - bentley 4y ago
        GitHub had no releases feature for many years. Most maintainers aren’t aware of the option, and of those who are, I doubt many are even aware that GitHub’s autogenerated tarballs are not stable or that they don’t include submodules.
      - ecnahc515 4y ago
        I imagine GitHub would be equally upset if every project every started uploading their source tarballs themselves, as presumably the point of their source tarball feature was not needing to store the source tarball for every repository ever, but generating it on demand.
gray_-_wolf 4y ago
Did people not know this? Honest question. I did run into this few times already before this change, so I assumed this would be wide-spread knowledge and mirrored everything.
- skobovm 4y ago
  How would anyone (outside of GH) have known this? The checksums have been stable for years, and this issue resulted from an internal update to the version of Git being used. It also was not publicized, until this ex post facto blog post
  - deleted 4y ago
    [deleted]
  - anecdotal1 4y ago
    They have not been stable https://github.com/freebsd/freebsd-ports/commit/a43ec88422ee8f1de13c5a0b5895639d4ee0fb43 https://github.com/freebsd/freebsd-ports/commit/a43ec88422ee...
- mhitza 4y ago
  https://xkcd.com/1053/ https://xkcd.com/1053/
frankjr 4y ago
GitHub will need to revert this change. They've just crippled pretty much every "from source" package manager out there.
- nick__m 4y ago
  I prefer that tool be adapted to be more resilient and not depend on github particular implementation.
  - swarfield 4y ago
    Using SHA hashes when building guarantees that the code that you are building is what you think it is. How else would you verify dependencies like this, GPG signatures would have the same issue if you change the underlying bits.
    - ArchOversight 4y ago
      a git checkout of the code at that particular tag hasn't changed. Just the tarball that git archive generates has.
      - vlovich123 4y ago
        The two main problems are: A) How do you catch tarballs that have extra files injected that aren't part of your manifest B) What does the performance of this look like? Certainly for traditional HDDs this is going to kill performance, but even for SSDs I think verifying a bunch of small files is going to be less efficient than verifying the tarball.
        ArchOversight 4y ago
        A wouldn't be an issue since you are checking out a git tag. B would just be a normal git checkout, which already validates that all the objects are reachable and git tags (and commits for that matter) can be signed, and since the sha1 hash is signed as well it validates that the entire tree of commits has not been tampered with. So as long you trust git to not lie about what it is writing to disk, you have a valid checkout of that tag. And if you do expect it to lie, why do you expect tar to not lie about what it is unpacking?
swarfield 4y ago
https://github.com/bazel-contrib/SIG-rules-authors/issues/11#issuecomment-1409404725 https://github.com/bazel-contrib/SIG-rules-authors/issues/11...
ArchOversight 4y ago
I remember a similar breakage happening before due to internal git changes, and thought it was common knowledge to upload your own signed tarballs for releases.
medellin 4y ago
Im thinking of all the bazel build rules that are about to break from my last company. Someone will have a fun day updating hundreds of hashes.
- jart 4y ago
  If they're using multiple URLs like a good Bazel user then they shouldn't be impacted.
  - medellin 4y ago
    They did where applicable but i know that not all of them had multiple
    - jart 4y ago
      Well now they know why it's so important. https://github.com/bazelbuild/bazel/commit/ed7ced0018dc5c5ebd6fc8afc7158037ac1df00d https://github.com/bazelbuild/bazel/commit/ed7ced0018dc5c5eb...
  - thirtyseven 4y ago
    The setup instructions for almost [1] every [2] major [3] rule set [4] only provide one (GitHub) url in the Starlark blob you're supposed to copy and paste, so hard to blame users here. [1] https://github.com/bazelbuild/rules_jvm_external/releases/tag/4.5 https://github.com/bazelbuild/rules_jvm_external/releases/ta... [2] https://github.com/bazelbuild/rules_python/releases/tag/0.17.3 https://github.com/bazelbuild/rules_python/releases/tag/0.17... [3] https://github.com/bazelbuild/rules_java/releases/tag/5.4.0 https://github.com/bazelbuild/rules_java/releases/tag/5.4.0 [4] https://github.com/bazelbuild/rules_scala https://github.com/bazelbuild/rules_scala
    - jart 4y ago
      I agree. The Bazel developers failed in their leadership.
UncleOxidant 4y ago
Lol... I was being burned by this just about an hour ago. Cloned a repo, did a build of the project (which uses bezel to fetch dependencies) and it reported errors due to mismatch in expected checksums.
vlovich123 4y ago
Hyrum's Law strikes again. It kind of doesn't matter what you document. If you weren't randomizing your checksum previously [1], you can't just spring this on the community and blame it for the fallout. I'm more shocked that there's resistance from the GitHub team saying "but we documented this isn't stable". Default stance for the team should be rollback & reevaluate an alternate path forward when the scope is this wide (e.g. only generating the new tarballs for future commits going forward). [1] Apparently googlesource did do this and just had people shift to using GitHub mirrors to avoid this problem.
- blueflow 4y ago
  But look at it from the other side. Users that don't read your documentation and expect your software to work like they imagined are just a huge pain in the ass.
  - mr_toad 4y ago
    Give a man a fish and he’ll assume he’s entitled to a lifetime supply of free fish.
    - dataflow 4y ago
      This has nothing to do with free vs. paid? The question is whether giving someone 99 of the same fish entitles them to expect the 100th one you throw in to be the same kind of fish, whether they paid for it or not.
  - kkirsche 4y ago
    This. You have to draw the line somewhere. Was this specific choice that line? Maybe not, but sometimes users aren’t right and changes just need to occur to ensure other asks from the same users can be delivered.
  - vlovich123 4y ago
    Fact of life: the vast majority of your users do not read your documentation (or do not do so carefully enough that what you put in your docs is an ironclad proof that all users adhere to). That's literally what Hyrum's law is about. Of course, you can choose to do whatever you want. It's valuable to recognize of course that you're trading off good will from your users with whatever technical improvement is getting made. Sometimes it's appropriate and inevitable (e.g. old behavior is just wrong or harmful and better to cut off). In the vast majority of cases though it's better to just have a better process in place to manage this with minimal disruption, identifying and communicating with broken users, and only then making that change.
deleted 4y ago
[deleted]
vtbassmatt 4y ago
Hey folks. I'm the product manager for Git at GitHub. We're sorry for the breakage, we're reverting the change, and we'll communicate better about such changes in the future (including timelines). Also posted here: https://github.com/bazel-contrib/SIG-rules-authors/issues/11#issuecomment-1409438954 https://github.com/bazel-contrib/SIG-rules-authors/issues/11...
- deleted 4y ago
  [deleted]
- vtbassmatt 4y ago
  We updated our Git version which made this change for the reasons explained. At the time we didn't foresee the impact. We're quickly rolling back the change now, as it's clear we need to look at this more closely to see if we can make the changes in a less disruptive way. Thanks for letting us know.
  - phphphphp 4y ago
    Consumers often mistake hasn’t changed for a commitment to never change: any sufficiently large product will be littered with these kind of implicit commitments made by the product to consumers that nobody has visibility into. You’re unfortunate that we were all relying on this commitment you’ve never made, but the quick reversion is the best we can hope for. People will theorise how this could have been avoided but c’est la vie — easy mistake that you’ve responded well to.
    - VWWHFSfQ 4y ago
      At this point they'll be stuck on old git for all of eternity unless they just roll their own archive/compress step out of band so the old hashes still work. Yikes.
      - groestl 4y ago
        They could also brownout the implied contract over a longer timespan.
WayToDoor 4y ago
https://github.com/orgs/community/discussions/45830#discussioncomment-4823799 https://github.com/orgs/community/discussions/45830#discussi... > Hey folks. I'm the product manager for Git at GitHub. We're sorry for the breakage, we're reverting the change, and we'll communicate better about such changes in the future (including timelines).
forgotpwd16 4y ago
Can anyone explain what happened? Thing changed, things broke, and things changed back in less than an hour.
deleted 4y ago
[deleted]
deleted 4y ago
[deleted]
jzelinskie 4y ago
Does anyone have the motivation for why the git project wants to use their own implementation of gzip? Did this implementation already exist and was being used for something else? I understand wanting fewer dependencies, but gut-reaction is that it's a bad move in the unsafe world of C to rewrite something that already has a far more audited, ubiquitous implementation.
- groestl 4y ago
  I think "Drop the dependency on gzip" for something like Git trumps a bit more exposure (which can be mitigated with thorough reviews).
- nemetroid 4y ago
  They're still using zlib to do the heavy lifting. It's not a large patch. https://public-inbox.org/git/1328fe72-1a27-b214-c226-d239099be673@web.de/ https://public-inbox.org/git/1328fe72-1a27-b214-c226-d239099...
  - capableweb 4y ago
    > So the internal implementation takes 17% longer on the Linux repo, but > uses 2% less CPU time. That's because the external gzip can run in > parallel on its own processor, while the internal one works sequentially > and avoids the inter-process communication overhead. > What are the benefits? Only an internal sequential implementation can > offer this eco mode, and it allows avoiding the gzip(1) requirement. It seems like they changed it because it uses less CPU, which makes sense in a "we're a global git hosting company" perspective, but less so for users who run the command themselves. They intentionally made it 17% slower to save 2% of CPU time, which probably makes sense at their scale, but for every user who run the command locally to lose 17% more of time?
    - pixl97 4y ago
      Because they pay for the 2% CPU time, not for the 17% local time. In theory the user also pays for 2% less CPU time, but they are much less likely to be CPU limited in their build processes. Of course 17% more time may not really be that much for most processes. Are we talking about 17% more of a second or of an hour?
daniealapt 4y ago
Any change breaks a workflow - https://xkcd.com/1172/ https://xkcd.com/1172/
- capableweb 4y ago
  True, small percentage will always be impacted by even the tiniest of change. But this was not that, checksums all over the place started breaking, as lots of FOSS is hosted on GitHub and lots of infrastructure depends on checksums remaining the same, otherwise they error out (correctly).
robomc 4y ago
Think this also broke github codespaces (the downloading of devcontainer "features").
skobovm 4y ago
I wonder what monetary loss in productivity was due to this change. We noticed this issue a bit before noon, tracked it down to GH, sent out company-wide comms notifying others of the problem, filed tickets with GH, had to modify numerous repos across multiple teams, and now it's 3pm and I'm here reading about it. It's crazy how such a seemingly innocuous change, like this, could lead to such widespread loss in productivity across the globe.
- misnome 4y ago
  Our conda-forge package builds broke. We had someone declare to us that tag downloads were never stable, just releases. This seems to be the opposite of the known truth about the previous status quo - but does go some way to demonstrating how little the state of the actual guarantees for this system were understood.
lucb1e 4y ago
I didn't even know I should be depending on compression, file ordering, created-at file metadata, etc. being stable when pressing 'download repository as zip' (if I understand correctly what this is about, since the article doesn't really say). Perhaps it could be stable due to caching for a while after you first press it, but when it gets re-generated? I'm very surprised this was reproducible to begin with, given how much trouble other projects have with that. For projects where I verify the download, gpg seems to be what all of them use (thinking of projects like etesync and restic here). Interesting that so many people relied on a zip being generated byte-for-byte identically every time.
- slaymaker1907 4y ago
  I once had a small issue with a deployment at work because of ordering issues within a zip file. That order is important with Spring since that determines which classes are initialized first.
  - groestl 4y ago
    One of the first things I check with every jvm packaging/deployment tool I investigate: does it preserve classpath ordering. Some offenders think -jar * is enough.
- leoh 4y ago
  Many tools set mtime to zero to avoid checksum drift
- philipwhiuk 4y ago
  There are lots of methods to solve this problem - I imagine this was just easiest at the time given it appeared to work. Bazel devs on the list are discussing the best approach going forward - a simple change is to upload a fixed copy as a release artifact.
- rfoo 4y ago
  > gpg seems to be what all of them use GPG signs a hash of the message with the private key, and you verify that the signature matches the file hash. Oh wait, what hash? :clown:
philipwhiuk 4y ago
Yet another reason why GitHub is not a good Artifactory/Nexus replacement. Anyone remember the crazyness when Homebrew had problems with using GitHub for the same thing?
- naikrovek 4y ago
  this is a git behavior, not a GitHub behavior. files uploaded to GH Packages are not modified by GitHub. only the "Source Code (.zip)" and "Source Code (.tgz)" files that are part of releases and tags are affected because git generates them on demand, and git does not guarantee hash stability. if you upload a package to GH Packages or upload a release asset to a GitHub releases those are never modified, and you can rely on those hashes.
  - philipwhiuk 4y ago
    No, it's not. GitHub chooses to do this. It's GitHub's choice to generate Source Code files on demand rather than when the release is made. It's a way of reducing their disk usage at the cost of this kind of potential problem. The problem is they also presented it as if it was a stable reference. If people knew it was not stable they would have done what the Bazel devs are now talking about doing, which is also uploading the source code at release time, as an artifact (which is how it works on Nexus).
    - naikrovek 4y ago
      > The problem is they also presented it as if it was a stable reference. how? the docs state that the hashes of these files are not guaranteed to be stable. the decision to generate those files on demand is a good one, provided that the behavior is documented, and it is. others in this thread figured it out before this particular issue arose and made the necessary changes to their workflows so that their downloads would have stable hashes.
yakubin 4y ago
Now I’m having a laugh at all those times someone tried to explain to me that vendoring dependencies doesn’t make sense, when you have package managers which verify checksums of the things downloaded from GitHub/wherever. A good laugh. Keep it simple, just vendor your deps.
- DoctorNick 4y ago
  With what? The abomination that is `git submodules`?
  - yakubin 4y ago
    No. Just copy files into the repo. Any way you like. In a GUI, in a terminal — it doesn’t require a dedicated tool. Although cargo in Rust e.g. provides a dubcommand for it (cargo vendor). Alternatively you can host the tarballs somewhere you control in static storage — be it a static web server, object storage or whatever. How it’s done in Chromium: <https://source.chromium.org/chromium/chromium/src/+/main:third_party/ https://source.chromium.org/chromium/chromium/src/+/main:thi...>.
    - skobovm 4y ago
      Woof. At the rate packages get updated these days, and the amount of dependencies between them, that just isn't sustainable for any reasonably-sized project in server and -- especially -- frontend land.
      - DoctorNick 4y ago
        Exactly. Unless the package manager has a mechanism for doing that, good fucking luck updating any of your packages ever again.
        viraptor 4y ago
        It is implemented pretty well in a few languages. For ruby for example it's almost trivial to maintain a `vendor` directory that matches the current `Gemfile` and `Gemfile.lock`. The size changes without LFS mean that's a bad idea, but... you can do it.
metrognome 4y ago
I wonder if this incident will encourage our industry to build more robust forms of artifact integrity verification, or if we will instead codify the status quo of "we guarantee repos to be archived deterministically." To me, the latter seems like a more troubling precedent.
- bentley 4y ago
  We’ve regressed from the previous norm of open source projects providing stable source tarballs with fixed checksums, sometimes even with cryptographic signatures.
  - reindeerer 4y ago
    That norm still exists, and it's offered by Github in form of Github Releases feature as well. It's the downstream tooling ( i.e. all the builds and package managers ) that need to clean their act up.
    - JonChesterfield 4y ago
      If the source tar changes, how do you propose the downstream tooling distinguishes between data corruption, MITM attack and upstream deciding to change the number without notifying anyone?
      - reindeerer 4y ago
        That's the whole point, source tars when properly versioned don't change. And you can get unchanged versions from any mirror in the world. sha256 of linux-2.6.10 release is 404e33da7c1bf271e0791cd771d065e19a2b1401ef8ebb481a60ce8ddc73e131, it wont change
- rswail 4y ago
  This is being driven in industry by the push by US FedGov (via NIST) to have supply chain verification after the recent hacks. POTUS issued an EO and NIST have been following up, leading to the promotion of schemes such as spdx https://tools.spdx.org/app/about/ https://tools.spdx.org/app/about/ Where I work is also required to start documenting our supply chain as part of the (new, replacing PCI-DSS) PCI-SFF certification requirements, which requires end-to-end verification of artifacts that are deployed within PCI scope. So really, the arguments about CPU time etc are basically silly. The use of SHA hashes for artifacts that don't change will be a requirement for anyone building industrial software, or supplying to government, or in the money transacting business.
stbenjam 4y ago
Oh god I spent like an hour debugging why gpg wouldn’t recognize the signature of RVM (Ruby version manager)
fomine3 4y ago
I haven't aware that git archive is reproducible
lopkeny12ko 4y ago
I can't fathom how no one internally at Microsoft-Github realized how widespread the breakage would be before rolling this out to all public users. Surely, Microsoft-Github's own internal builds would have started failing as a result of this change? Or do they not even canary releases internally at all?
- ilyt 4y ago
  I can "didn't read every commit in new version of git, realized after the fact"
1letterunixname 4y ago
Forever problem 0: Tar/zipball archives on the same ref never have a stable hash. Forever problem 1: No sha256/512/3 hashes of said tar/zipballs. Forever problem 2: No metalinks for those. Forever problem 3: Not IPv6. Some of our network is IPv6 only. Forever problem 4: Hitting secondary rate limiting because I can browse fast.
pabs3 4y ago
I note that diffoscope is useful for verifying which parts of git/other archives have changed: https://diffoscope.org/ https://diffoscope.org/ You can try it online here: https://try.diffoscope.org/ https://try.diffoscope.org/
hamandcheese 4y ago
The fact that this is causing problems seems like a flaw in Bazel, imo. Nix, for example, calculates a hash of the contents of a tarball, rather than a hash of the tarball itself.
- rfoo 4y ago
  Yep, Nix not affected at all is pretty impressive. On the other hand this goes against the "verify before parse" principle so I have mixed feelings on Nix's approach.
  - Foxboron 4y ago
    They don't really do any source authentication at all. There is no strategy for checking gpg/minisign/whatever signatures and fetching keys to validate these things.
kelnos 4y ago
The thing I don't get is how this ever worked. The change was upstream from git itself, and it was to use the builtin (zlib-based) compression code in git, rather than shelling out to gzip. But would the gzip binary itself give reproducible results across versions of gzip (and zlib)? Intuition seems to suggest it wouldn't, at least not always. And if not, was the "strategy" just to never update gzip or zlib on GitHub's servers? That seems like a non-starter...
- FeepingCreature 4y ago
  gzip is 28 years old. I don't think the output changes anymore.
  - account42 4y ago
    There is no reason to believe that it won't. Even after 28 years, there could be improvements merged for the compressor. Or perhaps especially after 28 years - we have a lot more memory now but it is slower when compared to our CPUs than it used to be so there is most likely room for tuning. Similar for patches that make use of newer CPU instructions - why would you expect them to take care to produce the exact same output rather than just the best compression possible for a perf budget.
    - ihattendorf 4y ago
      That's the whole point, it wasn't an enforced contract but just happened to not change in a long time so it was assumed to be part of the contract. The majority of users don't know how exactly GitHub is serving these archives, they just assume (incorrectly, but reasonably) if they download from this URL they'll always get the same archive bit for bit. That assumption has grown stronger and stronger over time the longer they remained the same, until today.
SuperSandro2000 4y ago
Thats why nix unpacks the archives first and then hashes them.
Aissen 4y ago
It was publicly known that Github was breaking automatic git archives consistency for many years. Here is a bug on a project to stop relying on fake github archives (as opposed to stable git-archive(1)): https://bugzilla.tianocore.org/show_bug.cgi?id=3099 https://bugzilla.tianocore.org/show_bug.cgi?id=3099 At some point it was impossible to go a few weeks (or even days) without a github archive change (depending on which part of the "CDN" you hit), I guess they must have stabilized it at some point. Here is an old issue before GitHub had a community issue tracker: https://github.com/isaacs/github/issues/1483 https://github.com/isaacs/github/issues/1483 I am glad this is getting more attention, maybe now github will finally have a stable endpoint for archives.
zoobab 4y ago
Github devs cannot point to their git commit, because Github is not open source.
jakeogh 4y ago
Github support, please checkout: https://news.ycombinator.com/item?id=34606345 https://news.ycombinator.com/item?id=34606345

245 points by mcovalt 4y ago

Now please give us compression options beyond gzip? :) Some zstd & lz4 please?

See https://github.com/orgs/community/discussions/45830 https://github.com/orgs/community/discussions/45830 for the fallout.

doubleunplussed 4y ago

Ah, this will presumably break some Arch Linux AUR packages. Preparing for bug reports.

frankjr 4y ago
Yep, it has already broken labwc for me. ==> Validating source files with b2sums... labwc-0.6.1.tar.gz ... FAILED ==> ERROR: One or more files did not pass the validity check!
elesiuta 4y ago
I always anticipated something like this could happen and it bothered me enough to create my own workflow [1] to archive, hash, and attach it to each release automatically for my AUR package. I can see how most people wouldn't notice/bother with such a small detail though, so I am not at all surprised by the fallout this caused. [1] https://github.com/elesiuta/picosnitch/blob/master/.github/workflows/tar-publish.yml https://github.com/elesiuta/picosnitch/blob/master/.github/w...

swarfield 4y ago

They have broken almost every open source project that builds external deps. Also broke homebrew apparently.

groestl 4y ago
> every open source project that builds external deps and relies on checksumming ephemeral artefacts for integrity.
- pxc 4y ago
  Such tools should definitely checksum package sources lol
- catiopatio 4y ago
  Source archives have never, in the entire history of open source, been considered ephemeral. GitHub unilaterally made that decision for their own convenience, and violated a decades-long universal community norm in the process.
  - mardifoufs 4y ago
    I think this change only affects automatically (and dynamically) generated source archives, not those that are actually pushed to Github Releases beforehand.
  - Denvercoder9 4y ago
    You could also say that some maintainers made that decision for their convenience of not having to build and upload source archives. It is possible to upload your own artifacts to a release on GitHub, and lots of projects do. Those are correctly treated as immutable by GitHub.
    - bentley 4y ago
      GitHub had no releases feature for many years. Most maintainers aren’t aware of the option, and of those who are, I doubt many are even aware that GitHub’s autogenerated tarballs are not stable or that they don’t include submodules.
    - ecnahc515 4y ago
      I imagine GitHub would be equally upset if every project every started uploading their source tarballs themselves, as presumably the point of their source tarball feature was not needing to store the source tarball for every repository ever, but generating it on demand.

gray_-_wolf 4y ago

Did people not know this? Honest question. I did run into this few times already before this change, so I assumed this would be wide-spread knowledge and mirrored everything.

skobovm 4y ago
How would anyone (outside of GH) have known this? The checksums have been stable for years, and this issue resulted from an internal update to the version of Git being used. It also was not publicized, until this ex post facto blog post
- deleted 4y ago
  [deleted]
- anecdotal1 4y ago
  They have not been stable https://github.com/freebsd/freebsd-ports/commit/a43ec88422ee8f1de13c5a0b5895639d4ee0fb43 https://github.com/freebsd/freebsd-ports/commit/a43ec88422ee...
mhitza 4y ago
https://xkcd.com/1053/ https://xkcd.com/1053/

frankjr 4y ago

GitHub will need to revert this change. They've just crippled pretty much every "from source" package manager out there.

nick__m 4y ago
I prefer that tool be adapted to be more resilient and not depend on github particular implementation.
- swarfield 4y ago
  Using SHA hashes when building guarantees that the code that you are building is what you think it is. How else would you verify dependencies like this, GPG signatures would have the same issue if you change the underlying bits.
  - ArchOversight 4y ago
    a git checkout of the code at that particular tag hasn't changed. Just the tarball that git archive generates has.
    - vlovich123 4y ago
      The two main problems are: A) How do you catch tarballs that have extra files injected that aren't part of your manifest B) What does the performance of this look like? Certainly for traditional HDDs this is going to kill performance, but even for SSDs I think verifying a bunch of small files is going to be less efficient than verifying the tarball.
      - ArchOversight 4y ago
        A wouldn't be an issue since you are checking out a git tag. B would just be a normal git checkout, which already validates that all the objects are reachable and git tags (and commits for that matter) can be signed, and since the sha1 hash is signed as well it validates that the entire tree of commits has not been tampered with. So as long you trust git to not lie about what it is writing to disk, you have a valid checkout of that tag. And if you do expect it to lie, why do you expect tar to not lie about what it is unpacking?

swarfield 4y ago

https://github.com/bazel-contrib/SIG-rules-authors/issues/11#issuecomment-1409404725 https://github.com/bazel-contrib/SIG-rules-authors/issues/11...

ArchOversight 4y ago

I remember a similar breakage happening before due to internal git changes, and thought it was common knowledge to upload your own signed tarballs for releases.

medellin 4y ago

Im thinking of all the bazel build rules that are about to break from my last company. Someone will have a fun day updating hundreds of hashes.

jart 4y ago
If they're using multiple URLs like a good Bazel user then they shouldn't be impacted.
- medellin 4y ago
  They did where applicable but i know that not all of them had multiple
  - jart 4y ago
    Well now they know why it's so important. https://github.com/bazelbuild/bazel/commit/ed7ced0018dc5c5ebd6fc8afc7158037ac1df00d https://github.com/bazelbuild/bazel/commit/ed7ced0018dc5c5eb...
- thirtyseven 4y ago
  The setup instructions for almost [1] every [2] major [3] rule set [4] only provide one (GitHub) url in the Starlark blob you're supposed to copy and paste, so hard to blame users here. [1] https://github.com/bazelbuild/rules_jvm_external/releases/tag/4.5 https://github.com/bazelbuild/rules_jvm_external/releases/ta... [2] https://github.com/bazelbuild/rules_python/releases/tag/0.17.3 https://github.com/bazelbuild/rules_python/releases/tag/0.17... [3] https://github.com/bazelbuild/rules_java/releases/tag/5.4.0 https://github.com/bazelbuild/rules_java/releases/tag/5.4.0 [4] https://github.com/bazelbuild/rules_scala https://github.com/bazelbuild/rules_scala
  - jart 4y ago
    I agree. The Bazel developers failed in their leadership.

UncleOxidant 4y ago

Lol... I was being burned by this just about an hour ago. Cloned a repo, did a build of the project (which uses bezel to fetch dependencies) and it reported errors due to mismatch in expected checksums.

vlovich123 4y ago

Hyrum's Law strikes again. It kind of doesn't matter what you document. If you weren't randomizing your checksum previously [1], you can't just spring this on the community and blame it for the fallout. I'm more shocked that there's resistance from the GitHub team saying "but we documented this isn't stable". Default stance for the team should be rollback & reevaluate an alternate path forward when the scope is this wide (e.g. only generating the new tarballs for future commits going forward). [1] Apparently googlesource did do this and just had people shift to using GitHub mirrors to avoid this problem.

blueflow 4y ago
But look at it from the other side. Users that don't read your documentation and expect your software to work like they imagined are just a huge pain in the ass.
- mr_toad 4y ago
  Give a man a fish and he’ll assume he’s entitled to a lifetime supply of free fish.
  - dataflow 4y ago
    This has nothing to do with free vs. paid? The question is whether giving someone 99 of the same fish entitles them to expect the 100th one you throw in to be the same kind of fish, whether they paid for it or not.
- kkirsche 4y ago
  This. You have to draw the line somewhere. Was this specific choice that line? Maybe not, but sometimes users aren’t right and changes just need to occur to ensure other asks from the same users can be delivered.
- vlovich123 4y ago
  Fact of life: the vast majority of your users do not read your documentation (or do not do so carefully enough that what you put in your docs is an ironclad proof that all users adhere to). That's literally what Hyrum's law is about. Of course, you can choose to do whatever you want. It's valuable to recognize of course that you're trading off good will from your users with whatever technical improvement is getting made. Sometimes it's appropriate and inevitable (e.g. old behavior is just wrong or harmful and better to cut off). In the vast majority of cases though it's better to just have a better process in place to manage this with minimal disruption, identifying and communicating with broken users, and only then making that change.

deleted 4y ago

[deleted]

vtbassmatt 4y ago

Hey folks. I'm the product manager for Git at GitHub. We're sorry for the breakage, we're reverting the change, and we'll communicate better about such changes in the future (including timelines). Also posted here: https://github.com/bazel-contrib/SIG-rules-authors/issues/11#issuecomment-1409438954 https://github.com/bazel-contrib/SIG-rules-authors/issues/11...

deleted 4y ago
[deleted]
vtbassmatt 4y ago
We updated our Git version which made this change for the reasons explained. At the time we didn't foresee the impact. We're quickly rolling back the change now, as it's clear we need to look at this more closely to see if we can make the changes in a less disruptive way. Thanks for letting us know.
- phphphphp 4y ago
  Consumers often mistake hasn’t changed for a commitment to never change: any sufficiently large product will be littered with these kind of implicit commitments made by the product to consumers that nobody has visibility into. You’re unfortunate that we were all relying on this commitment you’ve never made, but the quick reversion is the best we can hope for. People will theorise how this could have been avoided but c’est la vie — easy mistake that you’ve responded well to.
  - VWWHFSfQ 4y ago
    At this point they'll be stuck on old git for all of eternity unless they just roll their own archive/compress step out of band so the old hashes still work. Yikes.
    - groestl 4y ago
      They could also brownout the implied contract over a longer timespan.

WayToDoor 4y ago

https://github.com/orgs/community/discussions/45830#discussioncomment-4823799 https://github.com/orgs/community/discussions/45830#discussi... > Hey folks. I'm the product manager for Git at GitHub. We're sorry for the breakage, we're reverting the change, and we'll communicate better about such changes in the future (including timelines).

forgotpwd16 4y ago

Can anyone explain what happened? Thing changed, things broke, and things changed back in less than an hour.

deleted 4y ago

[deleted]

deleted 4y ago

[deleted]

jzelinskie 4y ago

Does anyone have the motivation for why the git project wants to use their own implementation of gzip? Did this implementation already exist and was being used for something else? I understand wanting fewer dependencies, but gut-reaction is that it's a bad move in the unsafe world of C to rewrite something that already has a far more audited, ubiquitous implementation.

groestl 4y ago
I think "Drop the dependency on gzip" for something like Git trumps a bit more exposure (which can be mitigated with thorough reviews).
nemetroid 4y ago
They're still using zlib to do the heavy lifting. It's not a large patch. https://public-inbox.org/git/1328fe72-1a27-b214-c226-d239099be673@web.de/ https://public-inbox.org/git/1328fe72-1a27-b214-c226-d239099...
- capableweb 4y ago
  > So the internal implementation takes 17% longer on the Linux repo, but > uses 2% less CPU time. That's because the external gzip can run in > parallel on its own processor, while the internal one works sequentially > and avoids the inter-process communication overhead. > What are the benefits? Only an internal sequential implementation can > offer this eco mode, and it allows avoiding the gzip(1) requirement. It seems like they changed it because it uses less CPU, which makes sense in a "we're a global git hosting company" perspective, but less so for users who run the command themselves. They intentionally made it 17% slower to save 2% of CPU time, which probably makes sense at their scale, but for every user who run the command locally to lose 17% more of time?
  - pixl97 4y ago
    Because they pay for the 2% CPU time, not for the 17% local time. In theory the user also pays for 2% less CPU time, but they are much less likely to be CPU limited in their build processes. Of course 17% more time may not really be that much for most processes. Are we talking about 17% more of a second or of an hour?

daniealapt 4y ago

Any change breaks a workflow - https://xkcd.com/1172/ https://xkcd.com/1172/

capableweb 4y ago
True, small percentage will always be impacted by even the tiniest of change. But this was not that, checksums all over the place started breaking, as lots of FOSS is hosted on GitHub and lots of infrastructure depends on checksums remaining the same, otherwise they error out (correctly).

robomc 4y ago

Think this also broke github codespaces (the downloading of devcontainer "features").

skobovm 4y ago

I wonder what monetary loss in productivity was due to this change. We noticed this issue a bit before noon, tracked it down to GH, sent out company-wide comms notifying others of the problem, filed tickets with GH, had to modify numerous repos across multiple teams, and now it's 3pm and I'm here reading about it. It's crazy how such a seemingly innocuous change, like this, could lead to such widespread loss in productivity across the globe.

misnome 4y ago
Our conda-forge package builds broke. We had someone declare to us that tag downloads were never stable, just releases. This seems to be the opposite of the known truth about the previous status quo - but does go some way to demonstrating how little the state of the actual guarantees for this system were understood.

lucb1e 4y ago

I didn't even know I should be depending on compression, file ordering, created-at file metadata, etc. being stable when pressing 'download repository as zip' (if I understand correctly what this is about, since the article doesn't really say). Perhaps it could be stable due to caching for a while after you first press it, but when it gets re-generated? I'm very surprised this was reproducible to begin with, given how much trouble other projects have with that. For projects where I verify the download, gpg seems to be what all of them use (thinking of projects like etesync and restic here). Interesting that so many people relied on a zip being generated byte-for-byte identically every time.

slaymaker1907 4y ago
I once had a small issue with a deployment at work because of ordering issues within a zip file. That order is important with Spring since that determines which classes are initialized first.
- groestl 4y ago
  One of the first things I check with every jvm packaging/deployment tool I investigate: does it preserve classpath ordering. Some offenders think -jar * is enough.
leoh 4y ago
Many tools set mtime to zero to avoid checksum drift
philipwhiuk 4y ago
There are lots of methods to solve this problem - I imagine this was just easiest at the time given it appeared to work. Bazel devs on the list are discussing the best approach going forward - a simple change is to upload a fixed copy as a release artifact.
rfoo 4y ago
> gpg seems to be what all of them use GPG signs a hash of the message with the private key, and you verify that the signature matches the file hash. Oh wait, what hash? :clown:

philipwhiuk 4y ago

Yet another reason why GitHub is not a good Artifactory/Nexus replacement. Anyone remember the crazyness when Homebrew had problems with using GitHub for the same thing?

naikrovek 4y ago
this is a git behavior, not a GitHub behavior. files uploaded to GH Packages are not modified by GitHub. only the "Source Code (.zip)" and "Source Code (.tgz)" files that are part of releases and tags are affected because git generates them on demand, and git does not guarantee hash stability. if you upload a package to GH Packages or upload a release asset to a GitHub releases those are never modified, and you can rely on those hashes.
- philipwhiuk 4y ago
  No, it's not. GitHub chooses to do this. It's GitHub's choice to generate Source Code files on demand rather than when the release is made. It's a way of reducing their disk usage at the cost of this kind of potential problem. The problem is they also presented it as if it was a stable reference. If people knew it was not stable they would have done what the Bazel devs are now talking about doing, which is also uploading the source code at release time, as an artifact (which is how it works on Nexus).
  - naikrovek 4y ago
    > The problem is they also presented it as if it was a stable reference. how? the docs state that the hashes of these files are not guaranteed to be stable. the decision to generate those files on demand is a good one, provided that the behavior is documented, and it is. others in this thread figured it out before this particular issue arose and made the necessary changes to their workflows so that their downloads would have stable hashes.

yakubin 4y ago

Now I’m having a laugh at all those times someone tried to explain to me that vendoring dependencies doesn’t make sense, when you have package managers which verify checksums of the things downloaded from GitHub/wherever. A good laugh. Keep it simple, just vendor your deps.

DoctorNick 4y ago
With what? The abomination that is `git submodules`?
- yakubin 4y ago
  No. Just copy files into the repo. Any way you like. In a GUI, in a terminal — it doesn’t require a dedicated tool. Although cargo in Rust e.g. provides a dubcommand for it (cargo vendor). Alternatively you can host the tarballs somewhere you control in static storage — be it a static web server, object storage or whatever. How it’s done in Chromium: <https://source.chromium.org/chromium/chromium/src/+/main:third_party/ https://source.chromium.org/chromium/chromium/src/+/main:thi...>.
  - skobovm 4y ago
    Woof. At the rate packages get updated these days, and the amount of dependencies between them, that just isn't sustainable for any reasonably-sized project in server and -- especially -- frontend land.
    - DoctorNick 4y ago
      Exactly. Unless the package manager has a mechanism for doing that, good fucking luck updating any of your packages ever again.
      - viraptor 4y ago
        It is implemented pretty well in a few languages. For ruby for example it's almost trivial to maintain a `vendor` directory that matches the current `Gemfile` and `Gemfile.lock`. The size changes without LFS mean that's a bad idea, but... you can do it.

metrognome 4y ago

I wonder if this incident will encourage our industry to build more robust forms of artifact integrity verification, or if we will instead codify the status quo of "we guarantee repos to be archived deterministically." To me, the latter seems like a more troubling precedent.

bentley 4y ago
We’ve regressed from the previous norm of open source projects providing stable source tarballs with fixed checksums, sometimes even with cryptographic signatures.
- reindeerer 4y ago
  That norm still exists, and it's offered by Github in form of Github Releases feature as well. It's the downstream tooling ( i.e. all the builds and package managers ) that need to clean their act up.
  - JonChesterfield 4y ago
    If the source tar changes, how do you propose the downstream tooling distinguishes between data corruption, MITM attack and upstream deciding to change the number without notifying anyone?
    - reindeerer 4y ago
      That's the whole point, source tars when properly versioned don't change. And you can get unchanged versions from any mirror in the world. sha256 of linux-2.6.10 release is 404e33da7c1bf271e0791cd771d065e19a2b1401ef8ebb481a60ce8ddc73e131, it wont change
rswail 4y ago
This is being driven in industry by the push by US FedGov (via NIST) to have supply chain verification after the recent hacks. POTUS issued an EO and NIST have been following up, leading to the promotion of schemes such as spdx https://tools.spdx.org/app/about/ https://tools.spdx.org/app/about/ Where I work is also required to start documenting our supply chain as part of the (new, replacing PCI-DSS) PCI-SFF certification requirements, which requires end-to-end verification of artifacts that are deployed within PCI scope. So really, the arguments about CPU time etc are basically silly. The use of SHA hashes for artifacts that don't change will be a requirement for anyone building industrial software, or supplying to government, or in the money transacting business.

stbenjam 4y ago

Oh god I spent like an hour debugging why gpg wouldn’t recognize the signature of RVM (Ruby version manager)

fomine3 4y ago

I haven't aware that git archive is reproducible

lopkeny12ko 4y ago

I can't fathom how no one internally at Microsoft-Github realized how widespread the breakage would be before rolling this out to all public users. Surely, Microsoft-Github's own internal builds would have started failing as a result of this change? Or do they not even canary releases internally at all?

ilyt 4y ago
I can "didn't read every commit in new version of git, realized after the fact"

1letterunixname 4y ago

Forever problem 0: Tar/zipball archives on the same ref never have a stable hash. Forever problem 1: No sha256/512/3 hashes of said tar/zipballs. Forever problem 2: No metalinks for those. Forever problem 3: Not IPv6. Some of our network is IPv6 only. Forever problem 4: Hitting secondary rate limiting because I can browse fast.

pabs3 4y ago

I note that diffoscope is useful for verifying which parts of git/other archives have changed: https://diffoscope.org/ https://diffoscope.org/ You can try it online here: https://try.diffoscope.org/ https://try.diffoscope.org/

hamandcheese 4y ago

The fact that this is causing problems seems like a flaw in Bazel, imo. Nix, for example, calculates a hash of the contents of a tarball, rather than a hash of the tarball itself.

rfoo 4y ago
Yep, Nix not affected at all is pretty impressive. On the other hand this goes against the "verify before parse" principle so I have mixed feelings on Nix's approach.
- Foxboron 4y ago
  They don't really do any source authentication at all. There is no strategy for checking gpg/minisign/whatever signatures and fetching keys to validate these things.

kelnos 4y ago

The thing I don't get is how this ever worked. The change was upstream from git itself, and it was to use the builtin (zlib-based) compression code in git, rather than shelling out to gzip. But would the gzip binary itself give reproducible results across versions of gzip (and zlib)? Intuition seems to suggest it wouldn't, at least not always. And if not, was the "strategy" just to never update gzip or zlib on GitHub's servers? That seems like a non-starter...

FeepingCreature 4y ago
gzip is 28 years old. I don't think the output changes anymore.
- account42 4y ago
  There is no reason to believe that it won't. Even after 28 years, there could be improvements merged for the compressor. Or perhaps especially after 28 years - we have a lot more memory now but it is slower when compared to our CPUs than it used to be so there is most likely room for tuning. Similar for patches that make use of newer CPU instructions - why would you expect them to take care to produce the exact same output rather than just the best compression possible for a perf budget.
  - ihattendorf 4y ago
    That's the whole point, it wasn't an enforced contract but just happened to not change in a long time so it was assumed to be part of the contract. The majority of users don't know how exactly GitHub is serving these archives, they just assume (incorrectly, but reasonably) if they download from this URL they'll always get the same archive bit for bit. That assumption has grown stronger and stronger over time the longer they remained the same, until today.

SuperSandro2000 4y ago

Thats why nix unpacks the archives first and then hashes them.

Aissen 4y ago

It was publicly known that Github was breaking automatic git archives consistency for many years. Here is a bug on a project to stop relying on fake github archives (as opposed to stable git-archive(1)): https://bugzilla.tianocore.org/show_bug.cgi?id=3099 https://bugzilla.tianocore.org/show_bug.cgi?id=3099 At some point it was impossible to go a few weeks (or even days) without a github archive change (depending on which part of the "CDN" you hit), I guess they must have stabilized it at some point. Here is an old issue before GitHub had a community issue tracker: https://github.com/isaacs/github/issues/1483 https://github.com/isaacs/github/issues/1483 I am glad this is getting more attention, maybe now github will finally have a stable endpoint for archives.

zoobab 4y ago

Github devs cannot point to their git commit, because Github is not open source.

jakeogh 4y ago

Github support, please checkout: https://news.ycombinator.com/item?id=34606345 https://news.ycombinator.com/item?id=34606345