-
Notifications
You must be signed in to change notification settings - Fork 13
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
Install scripts should always fetch and verify checksums. #2824
Conversation
mitchell-as
commented
Oct 17, 2023
•
edited by github-actions
bot
Loading
edited by github-actions
bot
DX-2266 Install.ps1 and install.sh should ALWAYS verify the checksum |
9a17939
to
2826ad1
Compare
2826ad1
to
697cfa9
Compare
Test failures are not due to this PR. They are due to existing, known issues and the usual timeouts. |
# Fetch version info. | ||
try { | ||
$infoJson = ConvertFrom-Json -InputObject (download $jsonURL) | ||
} catch [System.Exception] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should report what the exception was, otherwise debugging will be painful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exceptions are output to stdout, so there's no need for additional work. I tested this and it's pretty clear what the issue is (e.g. network unreachable). Or do you mean reported to us via rollbar or something?
installers/install.sh
Outdated
RELURL="$CHANNEL/$VERSIONNOSHA/$OS-amd64/state-$OS-amd64-$VERSION$DOWNLOADEXT" | ||
fi | ||
|
||
rm $INSTALLERTMPDIR/info.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VERSIONNOSHA="`echo $VERSION | sed 's/-SHA.*$//'`" | ||
JSONURL="$BASE_INFO_URL?channel=$CHANNEL&source=install&platform=$OS&target-version=$VERSIONNOSHA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that we basically ignore the SHA part. I think we should at least verify that the returned info matches the SHA in this scenario.
$versionNoSHA = $version -replace "-SHA.*", "" | ||
$jsonURL = "$script:BASEINFOURL/?channel=$script:CHANNEL&platform=windows&source=install&target-version=$versionNoSHA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that we basically ignore the SHA part. I think we should at least verify that the returned info matches the SHA in this scenario.