-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
Security: don't use an install script to download the binary #915
Comments
We've also run into issues with the install script. The binary fails to download sometimes, which either causes our CI to fail during
(It's a bit troubling how many npm packages connect to a non-npm URL directly and download mysterious things during package installation.) |
Another issue, if I'm reading |
@kaiyoma We're having a similar issue with our CI where we get intermittent errors of
It does seem that the ideal would be a prebuilt binary, both from a security and convenience standpoint. In the meantime, how have you worked around the CI error? |
@sehcheese We ended up creating a local binary/library cache on one of our internal file servers and populating it with the versions that we use. Sentry provides a mechanism to specify a custom URL to the sentry-cli binary: https://docs.sentry.io/product/cli/installation/#downloading-from-a-custom-source In our CI, our package installation looks like this now:
(where |
I've been looking into this, and I believe the reason we've gone for the download-in-postinstall-script approach is in order to be able to match the installed binary to the architecture on which it's running. It turns out (That said, in the interests of full disclosure, with the holidays coming up it's unlikely this will come up for consideration before the new year.) |
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This issue should remain open. |
Agree, just added the bot, so it went through everything :) |
I went through all the linked issues of other repositories, and I believe there is indeed not a single perfect solution for this problem. The main issue is that this CLI is used across various other projects indirectly, be it Android, Xcode, Fastlane, Maven, Gradle to name a few. And only recent versions of npm/yarn are doing everything correctly, no noisy logs, no fetching of unnecessary platforms data, etc. Also migrating to For now, I'd opt for adding checksum validation (security measure) and upgrading our install script to allow for a better and faster diagnosis of what went wrong and how to possibly fix the given problem (ux measure). Installing Checksum verification has been already added in the latest version - #1123 I'm as always open for the feedback :) |
After adding #1129 it's now also possible to use the local binary, which is installed in any way users choose to. This, and checksum validation should be enough of an upgrade, for now, so I'm going to close this issue. |
The usage of an install script is a vulnerability issue. It downloads an unsigned binary to the executing machine, which opens the way for potentially malicious code to be unintendedly downloaded. The NPM package should contain the binary such that it's there when installed and the install script can be omitted. This will make sure that an install of a locked version of the package will always result in the same artifact, which also helps with caching.
The text was updated successfully, but these errors were encountered: