feat(install.sh): verify the archive checksum #988
Conversation
EtienneM
changed the title
EtienneM
force-pushed
the
fix/422/cli_checksums
branch
2 times, most recently
from
426ae71
to
effe653
Compare
| @@ -112,30 +112,44 @@ main() { | |||
|
|
|||
| version=$(curl --silent https://cli-dl.scalingo.com/version | tr -d ' \t\n') | |||
| if [ -z "$version" ]; then | |||
| echo "-----> Fail to get the version of the CLI" >&2 | |||
| echo "You probably have an old version of curl. Please check your curl version and update accordingly." >&2 | |||
| error "Fail to get the version of the CLI\n" | |||
There was a problem hiding this comment.
I also updated a few lines to use the error function when we want to display error messages.
EtienneM
force-pushed
the
fix/422/cli_checksums
branch
from
8b21f01
to
5cde48d
Compare
EtienneM
force-pushed
the
fix/422/cli_checksums
branch
from
5cde48d
to
b784803
Compare
dists/install.sh
Outdated
| checksum_expected=$(wget -q --output-document - $checksums_url | grep $archive_name | cut -d " " -f 1) | ||
| if [[ "$checksum_computed" != "$checksum_expected" ]]; then | ||
| echo "INVALID" | ||
| error "Checksums don't match ('$checksum_computed' != '$checksum_expected').\n" |
There was a problem hiding this comment.
suggestion(non-blocking): As you said in the PR description, I'm not sure either to keep displaying the checksums.
Could it be possible to only display "Checksums don't match" but when adding DEBUG=1, then display the checksums?
There was a problem hiding this comment.
There is already a DEBUG=true which defines set -x, which would display the variables content. Hence I removed my line.
dists/install.sh
Outdated
| status "Verifying the checksum... " | ||
| checksums_url="https://github.com/Scalingo/cli/releases/download/${version}/checksums.txt" | ||
| checksum_computed=$(sha256sum ${tmpdir}/${archive_name} | cut -d " " -f1) | ||
| checksum_expected=$(wget -q --output-document - $checksums_url | grep $archive_name | cut -d " " -f 1) |
There was a problem hiding this comment.
nitpick(non-blocking): Can you use the full arguments rather than the shortcut ones?
There was a problem hiding this comment.
This should be a blocking comment, you are right ;)
If valid, output is:
If invalid, output is:
I'm not sure we want to keep displaying the checksums in the error message, what do you think?
Fix #422