base: Verify GitHub CLI gh download via checksums file#257
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jnohlgard The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughBoth UBI10 and UBI9 base image Dockerfiles now verify GitHub CLI tarballs using upstream SHA-256 checksums. After downloading the gh archive, each build downloads the corresponding checksums file, extracts the expected hash, and validates the tarball before extraction. Changesgh-cli checksum verification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
base/ubi9/Dockerfile (1)
56-58: ⚡ Quick winSame error handling gap as in UBI10 Dockerfile.
The checksum download (line 56) and grep lookup (line 57) failures result in silent skipping of gh-cli installation. Consider applying the same improvement suggested for
base/ubi10/Dockerfileto provide diagnostic messages when these intermediate steps fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/ubi9/Dockerfile` around lines 56 - 58, The checksum download and grep steps silently allow skipping gh-cli installation when they fail; modify the Dockerfile's block that uses GH_CHECKSUMS_URL, GH_CHECKSUMS, GH_TGZ and creates "${GH_TGZ}.sha256" so that failures of curl or grep are detected and logged: after the curl and grep commands, add conditional checks that print an error message (including the failed URL or filename) and exit non‑zero if either step fails before reaching sha256sum -c, and ensure the sha256sum -c path also logs a clear diagnostic on failure; keep the same variables (GH_CHECKSUMS_URL, GH_CHECKSUMS, GH_TGZ, "${GH_TGZ}.sha256") so the updated checks integrate with the existing install flow.base/ubi10/Dockerfile (1)
73-75: ⚡ Quick winMissing error message when checksum download or lookup fails.
If the checksums file download fails (line 73) or grep doesn't find a matching entry (line 74), the
&&chain breaks and gh-cli is silently not installed. Unlike thesha256sum -cfailure case on line 80, there's no diagnostic message explaining why the installation was skipped.Consider adding explicit error handling for these intermediate steps to aid debugging build failures.
Suggested improvement for clearer error reporting
if curl -fsSL "${GH_TGZ_URL}" -o "${GH_TGZ}"; then \ - curl -fsSL "${GH_CHECKSUMS_URL}" -o "${GH_CHECKSUMS}" && \ - grep -F -e " ${GH_TGZ}" "${GH_CHECKSUMS}" > "${GH_TGZ}.sha256" && \ + if ! curl -fsSL "${GH_CHECKSUMS_URL}" -o "${GH_CHECKSUMS}"; then \ + echo "Failed to download checksums file — skipping gh installation."; \ + elif ! grep -F -e " ${GH_TGZ}" "${GH_CHECKSUMS}" > "${GH_TGZ}.sha256"; then \ + echo "Checksum not found for ${GH_TGZ} — skipping gh installation."; \ + elif sha256sum -c "${GH_TGZ}.sha256"; then \ - if sha256sum -c "${GH_TGZ}.sha256"; then \ tar -zxv --no-same-owner -f "${GH_TGZ}" && \ mv "gh_${GH_VERSION}_${GH_ARCH}"/bin/gh /usr/local/bin/ && \ mv "gh_${GH_VERSION}_${GH_ARCH}"/share/man/man1/* /usr/local/share/man/man1; \ else \ echo "Downloaded gh archive invalid — skipping."; \ fi; \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/ubi10/Dockerfile` around lines 73 - 75, The Dockerfile silently skips gh-cli installation when the checksum download or grep lookup fails; update the Dockerfile around the GH_CHECKSUMS_URL/GH_CHECKSUMS/GH_TGZ steps to add explicit failure handling and messages: after the curl of "${GH_CHECKSUMS_URL}" to "${GH_CHECKSUMS}" and after the grep that writes "${GH_TGZ}.sha256", ensure each command detects failure and emits a clear diagnostic (including the variable names GH_CHECKSUMS_URL, GH_CHECKSUMS, GH_TGZ) and exits the build (for example via a conditional || echo ...; exit 1 or similar) before proceeding to the sha256sum -c "${GH_TGZ}.sha256" check so failed downloads or missing grep entries are logged rather than silently skipping installation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@base/ubi10/Dockerfile`:
- Around line 73-75: The Dockerfile silently skips gh-cli installation when the
checksum download or grep lookup fails; update the Dockerfile around the
GH_CHECKSUMS_URL/GH_CHECKSUMS/GH_TGZ steps to add explicit failure handling and
messages: after the curl of "${GH_CHECKSUMS_URL}" to "${GH_CHECKSUMS}" and after
the grep that writes "${GH_TGZ}.sha256", ensure each command detects failure and
emits a clear diagnostic (including the variable names GH_CHECKSUMS_URL,
GH_CHECKSUMS, GH_TGZ) and exits the build (for example via a conditional || echo
...; exit 1 or similar) before proceeding to the sha256sum -c "${GH_TGZ}.sha256"
check so failed downloads or missing grep entries are logged rather than
silently skipping installation.
In `@base/ubi9/Dockerfile`:
- Around line 56-58: The checksum download and grep steps silently allow
skipping gh-cli installation when they fail; modify the Dockerfile's block that
uses GH_CHECKSUMS_URL, GH_CHECKSUMS, GH_TGZ and creates "${GH_TGZ}.sha256" so
that failures of curl or grep are detected and logged: after the curl and grep
commands, add conditional checks that print an error message (including the
failed URL or filename) and exit non‑zero if either step fails before reaching
sha256sum -c, and ensure the sha256sum -c path also logs a clear diagnostic on
failure; keep the same variables (GH_CHECKSUMS_URL, GH_CHECKSUMS, GH_TGZ,
"${GH_TGZ}.sha256") so the updated checks integrate with the existing install
flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd04023f-121c-43ec-8760-305459edccbc
📒 Files selected for processing (2)
base/ubi10/Dockerfilebase/ubi9/Dockerfile
1 participant
Current build skips over the gh CLI package because of missing
filecommand. Better to just verify the sha256 of the archive.Summary by CodeRabbit
Release Notes
Bug Fixes