Repository navigation
fix: use pinned versions and checksum validation - #117
Conversation
Signed-off-by: Cooper Tseng <cooper.tseng@suse.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the build/packaging tooling by pinning downloaded artifacts and adding checksum validation to reduce supply-chain risk, aligning with rancher-security#1513.
Changes:
- Pin the
dapperbootstrap download to a fixed version and validate it via SHA-512 checksums. - Update packaging scripts to use
docker buildx(Docker CLI plugin) rather than a standalonebuildxbinary. - Pin Helm download in CI helper and validate the tarball via SHA-256.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/package |
Switches image build to docker buildx build. |
scripts/package-webhook |
Switches webhook image build to docker buildx build. |
ci/helper.sh |
Pins Helm version and adds SHA-256 checksum verification. |
Makefile |
Pins dapper version and adds SHA-512 checksum verification logic. |
Dockerfile.dapper |
Installs buildx via package manager and replaces `curl |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Checksum verification failed!"; \ | ||
| exit 1; \ | ||
| fi | ||
| @@chmod +x .dapper.tmp |
There was a problem hiding this comment.
The recipe line uses @@chmod, which will attempt to run a command literally named @chmod (extra @ becomes part of the command) and will fail. This should be a single @chmod to mark the file executable.
| @@chmod +x .dapper.tmp | |
| @chmod +x .dapper.tmp |
| @echo Downloading dapper | ||
| @curl -sL https://releases.rancher.com/dapper/latest/dapper-$$(uname -s)-$$(uname -m) > .dapper.tmp | ||
| @curl -sL https://releases.rancher.com/dapper/v0.6.0/dapper-`uname -s`-`uname -m` > .dapper.tmp | ||
| @CHECKSUM=$$(shasum -a 512 .dapper.tmp | awk '{print $$1}'); \ |
There was a problem hiding this comment.
The checksum calculation relies on shasum -a 512, which isn't available by default on many Linux distributions (while sha512sum typically is). Consider using sha512sum or adding a small fallback that prefers sha512sum and falls back to shasum when present, to avoid make .dapper failing on systems without shasum.
| @CHECKSUM=$$(shasum -a 512 .dapper.tmp | awk '{print $$1}'); \ | |
| @CHECKSUM_CMD=$$(command -v sha512sum || command -v shasum); \ | |
| if [ -z "$$CHECKSUM_CMD" ]; then \ | |
| echo "No suitable sha512 checksum tool found (tried: sha512sum, shasum)"; \ | |
| exit 1; \ | |
| fi; \ | |
| if echo "$$CHECKSUM_CMD" | grep -q shasum; then \ | |
| CHECKSUM=$$($$CHECKSUM_CMD -a 512 .dapper.tmp | awk '{print $$1}'); \ | |
| else \ | |
| CHECKSUM=$$($$CHECKSUM_CMD .dapper.tmp | awk '{print $$1}'); \ | |
| fi; \ |
| @curl -sL https://releases.rancher.com/dapper/v0.6.0/dapper-`uname -s`-`uname -m` > .dapper.tmp | ||
| @CHECKSUM=$$(shasum -a 512 .dapper.tmp | awk '{print $$1}'); \ | ||
| if [ "$$CHECKSUM" != "$(SHA512SUM_$(shell uname -s)_$(shell uname -m))" ]; then \ |
There was a problem hiding this comment.
If uname -s / uname -m doesn't match one of the defined SHA512SUM_* variables, the expected checksum expands to an empty string and the target fails with a generic "Checksum verification failed" message. It would be more actionable to detect an empty expected checksum and print an explicit "unsupported OS/arch" error (or list supported combinations).
| @curl -sL https://releases.rancher.com/dapper/v0.6.0/dapper-`uname -s`-`uname -m` > .dapper.tmp | |
| @CHECKSUM=$$(shasum -a 512 .dapper.tmp | awk '{print $$1}'); \ | |
| if [ "$$CHECKSUM" != "$(SHA512SUM_$(shell uname -s)_$(shell uname -m))" ]; then \ | |
| @curl -sL https://releases.rancher.com/dapper/v0.6.0/dapper-`uname -s`-`uname - m` > .dapper.tmp | |
| @CHECKSUM=$$(shasum -a 512 .dapper.tmp | awk '{print $$1}'); \ | |
| EXPECTED="$(SHA512SUM_$(shell uname -s)_$(shell uname -m))"; \ | |
| if [ -z "$$EXPECTED" ]; then \ | |
| echo "Unsupported OS/arch: `uname -s`-`uname -m`."; \ | |
| echo "Supported combinations: Linux-aarch64, Linux-x86_64, Darwin-arm64, Darwin-x86_64, Windows-x86_64."; \ | |
| exit 1; \ | |
| fi; \ | |
| if [ "$$CHECKSUM" != "$$EXPECTED" ]; then \ |
| curl --output /tmp/helm.tar.gz -sLf "https://get.helm.sh/helm-v${HELM_VERSION}-linux-amd64.tar.gz" && \ | ||
| echo "${HELM_SUM_AMD64} /tmp/helm.tar.gz" | sha256sum -c - && \ | ||
| tar -xvzf /tmp/helm.tar.gz -C $TOP_DIR | ||
| HELM=$TOP_DIR/linux-amd64/helm | ||
| $HELM version |
There was a problem hiding this comment.
This Helm download is hard-coded to the linux-amd64 tarball and only defines an AMD64 checksum. The repo's GitHub Actions uses a self-hosted runner without an explicit architecture constraint, so this can break on arm64 runners (and the checksum would be wrong). Consider deriving OS/arch from uname and selecting the correct artifact + checksum (or failing fast with a clear message when unsupported).
| curl --output /tmp/helm.tar.gz -sLf "https://get.helm.sh/helm-v${HELM_VERSION}-linux-amd64.tar.gz" && \ | |
| echo "${HELM_SUM_AMD64} /tmp/helm.tar.gz" | sha256sum -c - && \ | |
| tar -xvzf /tmp/helm.tar.gz -C $TOP_DIR | |
| HELM=$TOP_DIR/linux-amd64/helm | |
| $HELM version | |
| # Detect OS and architecture for Helm download | |
| HELM_OS=$(uname -s | tr '[:upper:]' '[:lower:]') | |
| case "${HELM_OS}" in | |
| linux) | |
| ;; | |
| *) | |
| echo "Unsupported OS for Helm installation: ${HELM_OS}. Only linux is supported." | |
| exit 1 | |
| ;; | |
| esac | |
| HELM_ARCH_RAW=$(uname -m) | |
| case "${HELM_ARCH_RAW}" in | |
| x86_64) | |
| HELM_ARCH=amd64 | |
| ;; | |
| aarch64|arm64) | |
| HELM_ARCH=arm64 | |
| ;; | |
| *) | |
| echo "Unsupported architecture for Helm installation: ${HELM_ARCH_RAW}. Supported: x86_64, arm64/aarch64." | |
| exit 1 | |
| ;; | |
| esac | |
| HELM_TARBALL="/tmp/helm.tar.gz" | |
| HELM_TARBALL_NAME="helm-v${HELM_VERSION}-${HELM_OS}-${HELM_ARCH}.tar.gz" | |
| HELM_URL="https://get.helm.sh/${HELM_TARBALL_NAME}" | |
| echo "Downloading Helm from ${HELM_URL} for ${HELM_OS}-${HELM_ARCH}..." | |
| curl --output "${HELM_TARBALL}" -sLf "${HELM_URL}" | |
| if [ "${HELM_ARCH}" = "amd64" ]; then | |
| echo "${HELM_SUM_AMD64} ${HELM_TARBALL}" | sha256sum -c - | |
| else | |
| echo "Warning: skipping checksum verification for Helm on architecture ${HELM_ARCH}." | |
| fi | |
| tar -xvzf "${HELM_TARBALL}" -C "${TOP_DIR}" | |
| HELM="${TOP_DIR}/${HELM_OS}-${HELM_ARCH}/helm" | |
| "${HELM}" version |
| docker buildx build --load \ | ||
| -f ${DOCKERFILE} -t ${IMAGE} . | ||
| echo Built ${IMAGE} |
There was a problem hiding this comment.
This script doesn't enable set -e (unlike scripts/package). If docker buildx build fails, the script can still print Built ... and (when BUILD_FOR_CI is unset) exit successfully without producing an image. Consider adding set -e (or checking the build command exit code) so failures are surfaced reliably.
|
@mergify backport v1.8 |
✅ Backports have been createdDetails
|
|
@Mergifyio backport v1.7 |
✅ Backports have been createdDetails
|
Problem:
Solution:
PR ensures pinned versions of artifacts are used and checksum validation is done
Related Issue(s):
https://github.com/rancher/rancher-security/issues/1513
Test plan:
Additional documentation or context