Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check-file format for release checksums #901

Closed
Rudxain opened this issue Jul 10, 2023 · 6 comments
Closed

check-file format for release checksums #901

Rudxain opened this issue Jul 10, 2023 · 6 comments
Assignees
Labels

Comments

@Rudxain
Copy link

Rudxain commented Jul 10, 2023

The feature request

Please reformat the checksum list to be a valid *sum check-file.

Examples:

this

3bba4a245869304324647f3f406f7c500aeb6324b3c3d347ee867d0b93c49399  GitHubDesktop-linux-3.2.5-linux1.AppImage
a7eaabf7f68441c7e4280ac2de84f22228c791a3732ee9b7c577929cd7b181b5  GitHubDesktop-linux-3.2.5-linux1.deb
527ec0bc199c7db2ebdbda09cd657786c7e5a42a554e71d4619c45f3183639f2  GitHubDesktop-linux-3.2.5-linux1.rpm

not this

  • GitHubDesktop-linux-3.2.5-linux1.AppImage - 3bba4a245869304324647f3f406f7c500aeb6324b3c3d347ee867d0b93c49399
  • GitHubDesktop-linux-3.2.5-linux1.deb - a7eaabf7f68441c7e4280ac2de84f22228c791a3732ee9b7c577929cd7b181b5
  • GitHubDesktop-linux-3.2.5-linux1.rpm - 527ec0bc199c7db2ebdbda09cd657786c7e5a42a554e71d4619c45f3183639f2

Proposed solution

This will make it faster+easier to verify downloads, like so:

# assuming checktext was copied to X clipboard
xclip -out | sha256sum -c

# assuming checktext was written to "ghd.sha256"
sha256sum -c ghd.sha256

Rather than having to remove the "-" and swap strings, or comparing the output of sha256sum GitHubDesktop*

Additional context

I just found where's the src for the auto-release-notes

@shiftkey
Copy link
Owner

@Rudxain thanks for the note on this. I haven't yet kicked off a new release using those generated notes but I can do that some time this week.

@shiftkey shiftkey added the meta label Jul 11, 2023
@shiftkey shiftkey self-assigned this Jul 11, 2023
@shiftkey
Copy link
Owner

I took a shot at formatting the checksums in the way requested @Rudxain, but I wanted to organize the packages by architecture to avoid confusion - please let me know if you have any additional feedback:

https://github.com/shiftkey/desktop/releases/tag/release-3.2.7-test7

Alternatively, if it's easier - we can just include the *.sha256 in the release and omit these entries from the release notes.

@theofficialgman
Copy link

theofficialgman commented Jul 11, 2023

@Rudxain if I might ask. what is the purpose you are using to verify downloads? if it is just to verify that the file you downloaded was not corrupted during download, wouldn't it be better to simply include the .sha256 files that are already generated in the artifacts as assets in the github release (just noticed that shiftkey also asked you that)? that way you can just download the single .sha256 file of the same name as the deb/rpm/AppImage that you downloaded

for reference, these are the files that are generated as part of the artifacts themselves https://github.com/shiftkey/desktop/actions/runs/5524699326/jobs/10077640875#step:4:10 . the actual .sha1sum files are not currently published as an asset in a github release

if instead you are concerned about your connection being hijacked and downloads not being what you expect them to be (ie: malware in the binary), then I don't think you can trust the github website releases page itself to show the correct sha256 (if whatever bad actor is smart enough to redirect your downloads, they are smart enough to also redirect the webpage to show a sha256 that matches their malware binary)

@Rudxain
Copy link
Author

Rudxain commented Jul 12, 2023

@shiftkey

I took a shot at formatting the checksums in the way requested @Rudxain, but I wanted to organize the packages by architecture to avoid confusion - please let me know if you have any additional feedback

Thanks! I expected MD code-blocks, but the plaintext is fine too. Having different sections for different archs seems good enough.

Alternatively, if it's easier - we can just include the *.sha256 in the release and omit these entries from the release notes.

I agree, that would be better!

@theofficialgman

what is the purpose you are using to verify downloads?

I usually verify them manually, since it's not a frequent task. However, releases are regular enough that I attempted to "automate" this process:

#!/usr/bin/env bash
set -eu

file=(GitHubDesktop-linux-*.AppImage)
printf '%s\n' "$1  ${file[0]}" | sha256sum -c 

The time saved is likely negligible haha, but I'll probably continue using it.

wouldn't it be better to simply include the .sha256 files that are already generated in the artifacts [...]?

Yes. I didn't know it was included in the artifacts until you and @shiftkey mentioned it. I apologize for my ignorance 😅

I don't think you can trust the github website releases page itself to show the correct sha256

Correct. That's why #128 was opened

@sarim
Copy link

sarim commented Jul 19, 2023

I also think .sha256 files are better than including hashes in the release note body text.

@shiftkey shiftkey changed the title [META] check-file format for release checksums check-file format for release checksums Jul 29, 2023
@shiftkey
Copy link
Owner

Closing this out as resolved. Please open fresh issues for anything further...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

4 participants