-
Notifications
You must be signed in to change notification settings - Fork 284
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
chore: Add fetch-sha256 script to update bootstrap node hash. #2513
Conversation
cc88306
to
bf8632f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2513 +/- ##
==========================================
- Coverage 68.99% 68.96% -0.03%
==========================================
Files 89 89
Lines 27781 27781
==========================================
- Hits 19168 19160 -8
- Misses 8613 8621 +8 ☔ View full report in Codecov by Sentry. |
Heh, the next step would be having the CI update the hash for you. CI using the github cli tool to push a commit into the PR branch updating the hash, dropping all the previous such commits it has done earlier in the PR.
That's not verifiable nor enforceable. You can't tell if the dev built it locally of used the fetch-sha256 script. Actually, does it even matter if the dev builds it locally or if they grab the hash off the CI? What does the hash check protect against? It's clearly not a rouge CI, as we trust the CI with dockerhub credentials, we trust it to publish the container to the DockerHub, and we trust it to run the check (a rouge CI running a check against a rouge CI would make no sense). That check is also run when users use the Dockerfile directly, without any CI involved. So it looks like the check's purpose is not to protect against a rouge CI but to protect against the base images the tox-bootstrapd container uses from being compromised on DockerHub. Primarily the Alpine image, as it's the one affecting the hash of the tox-bootstrapd binary. So it's probably okay to grab the hash off the CI since it's trusted anyway and there is no real need to have the developer update the hash locally? Correct me if my reasoning is wrong! |
98d3727
to
6c8e494
Compare
Yes, that reasoning makes sense. |
This fetches it from github, so we don't need to build it locally. Not super ideal, because devs are supposed to build it locally to prove reproducibility, but we can keep that diligence on the dev to do once when actually merging the PR.
This fetches it from github, so we don't need to build it locally.
Not super ideal, because devs are supposed to build it locally to prove reproducibility, but we can keep that diligence on the dev to do once when actually merging the PR.
This change is