First of all, it's great to see that the longstanding issue of trusting a remote SHASUMS256.txt file has been fixed. Huge thanks for that!
There is a bug in the impl though: it doesn't recheck that shasums match when it pulls a file out of cache.
That happens in https://github.com/electron/get/blob/main/src/index.ts#L72-L82 -- that codepath returns the file from cache without any shasum verification, regardless of how it got into the cache.
Steps to reproduce, variant 1
npm i electron
replace the cache file (look in ~/.cache/electron) with something else
npm i electron in another dir -- the tampered file will be used without any shasum check
Steps to reproduce, variant 2
npm i electron to get npm packages
rm -rf ~/.cache/electron to clear cache
Replace the hash in node_modules/electron/checksums.json to simulate a hash mismatch
rm -rf ~/node_modules/electron/dist to trigger reinstall
node ./node_modules/electron/install.js now fails, as it should
rm -rf ~/node_modules/electron/dist to trigger reinstall
node ./node_modules/electron/install.js succeeds, despite the hash mismatching the expected one in checksums.json.
I.e. if the first call to whatever put the package in cache has been done e.g. with electron_use_remote_checksums env var, or without checksums option, or with unsafelyDisableChecksums option, the follow up call which has checksums option set and expects them to be validated against those supplied ones won't get them validated.
Same as when the cache has been corrupted or tampered, allowing e.g. a long-term CI poisoning via cache artifacts.
First of all, it's great to see that the longstanding issue of trusting a remote SHASUMS256.txt file has been fixed. Huge thanks for that!
There is a bug in the impl though: it doesn't recheck that shasums match when it pulls a file out of cache. That happens in https://github.com/electron/get/blob/main/src/index.ts#L72-L82 -- that codepath returns the file from cache without any shasum verification, regardless of how it got into the cache.
Steps to reproduce, variant 1
npm i electron
~/.cache/electron
) with something elsenpm i electron
in another dir -- the tampered file will be used without any shasum checkSteps to reproduce, variant 2
npm i electron
to get npm packagesrm -rf ~/.cache/electron
to clear cachenode_modules/electron/checksums.json
to simulate a hash mismatchrm -rf ~/node_modules/electron/dist
to trigger reinstallnode ./node_modules/electron/install.js
now fails, as it shouldelectron_use_remote_checksums=1 node ./node_modules/electron/install.js
succeeds (expected)rm -rf ~/node_modules/electron/dist
to trigger reinstallnode ./node_modules/electron/install.js
succeeds, despite the hash mismatching the expected one inchecksums.json
.I.e. if the first call to whatever put the package in cache has been done e.g. with
electron_use_remote_checksums
env var, or withoutchecksums
option, or withunsafelyDisableChecksums
option, the follow up call which haschecksums
option set and expects them to be validated against those supplied ones won't get them validated.Same as when the cache has been corrupted or tampered, allowing e.g. a long-term CI poisoning via cache artifacts.