Skip to content

Commit 9be954b

Browse files
committed
fix(uat): bind helm-diff provenance to the requested version
Address review on #2267. Gates 1-3 all reduce to "these bytes match the pin", so none of them can detect a pin that was itself poisoned. tools/update-helm-diff-checksums derives the pin from an unsigned checksums.txt, and the release asset filename carries no version, so a compromised release or CDN can seed the pin with an older release's hashes during a Renovate bump and then serve that older tarball at install time. The older tarball is genuinely signed by the maintainer, so gate 2 passes, and its .prov keys the same helm-diff-<os>-<arch>.tgz filename with the hash now in the pin, so gate 3 passes too. Result is a downgrade despite the requested version. Add gate 4: extract the version from the already-verified provenance body and fail unless it equals the requested version. Confirmed against real artifacts - v3.15.10's .prov yields VALIDSIG for the pinned fingerprint and carries the identical files key, so only the signed version field separates it from v3.15.11. Also from the same review: - Accept the pinned fingerprint in either VALIDSIG position, so signing with a future subkey does not false-reject a legitimate signature. - Preflight gpg alongside helmfile/helm/curl. - Read testing_tools.helm_diff with a // "" fallback and an explicit empty check, matching the checksum read, so an unpinned version fails with a clear message instead of a 404 on a .../null/... URL. - Note that --verify=false skips helm's provenance check but not helm-diff's install hook, whose no-refetch behavior depends on upstream continuing to bundle bin/diff. - Correct the gate rationale: gate 1 is the control that blocks a replayed older release, not gate 3. - Record that a stale helm_diff pin has no PR-time gate, unlike its helmfile/chainsaw siblings, since helm_diff_checksums is read only by the nightly UAT. Signed-off-by: Brian Lockwood <lockwobr@gmail.com>
1 parent a8beebd commit 9be954b

2 files changed

Lines changed: 61 additions & 12 deletions

File tree

.github/workflows/renovate.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,16 @@ jobs:
124124
# not), which then breaks E2E/CLI E2E at the sha256 verify step.
125125
# `[a-z-]+` (not `[a-z]+`) so hyphenated tool names match too, e.g.
126126
# tools/update-helm-diff-checksums.
127+
#
128+
# The stale-checksum symptom above is not uniform across the three
129+
# tools. A hook that runs but *fails* (renamed asset, egress blip)
130+
# leaves the new version pinned against the old checksums, and only
131+
# helmfile/chainsaw fail a PR-gating job for it — qualification.yaml
132+
# feeds their sha256 into setup-build-tools, which verifies it.
133+
# helm_diff_checksums is read solely by tests/uat/lib/phases.sh, so a
134+
# stale helm-diff pin stays green on every PR check and first surfaces
135+
# in nightly UAT, across all clouds. Renovate's "Artifact update
136+
# problem" warning in the PR body is the signal to heed; helm-diff is
137+
# deliberately not auto-merged, so a human sees it.
127138
RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS: '["^\\./tools/update-[a-z-]+-checksums "]'
128139
LOG_LEVEL: ${{ inputs.logLevel || 'info' }}

tests/uat/lib/phases.sh

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -437,13 +437,18 @@ phase_install() {
437437
command -v helmfile >/dev/null || { echo "helmfile not on PATH" >&2; exit 1; }
438438
command -v helm >/dev/null || { echo "helm not on PATH" >&2; exit 1; }
439439
command -v curl >/dev/null || { echo "curl not on PATH" >&2; exit 1; }
440+
command -v gpg >/dev/null || { echo "gpg not on PATH" >&2; exit 1; }
440441

441442
# Read helm-diff version from the single source of truth (.settings.yaml).
442443
local SCRIPT_DIR
443444
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
444445
local REPO_ROOT="${SCRIPT_DIR}/../../.."
445446
local HELM_DIFF_VERSION
446-
HELM_DIFF_VERSION="$(yq -r '.testing_tools.helm_diff' "${REPO_ROOT}/.settings.yaml")"
447+
HELM_DIFF_VERSION="$(yq -r '.testing_tools.helm_diff // ""' "${REPO_ROOT}/.settings.yaml")"
448+
if [[ -z "${HELM_DIFF_VERSION}" ]]; then
449+
echo "::error::testing_tools.helm_diff is not pinned in .settings.yaml" >&2
450+
exit 1
451+
fi
447452

448453
# Declared separately from the assignment: `local x="$(...)"` would mask the
449454
# substitution's exit status behind local's own success.
@@ -471,21 +476,27 @@ phase_install() {
471476
echo "helm-diff ${installed_ver} installed; removing to pin ${HELM_DIFF_VERSION}"
472477
helm plugin remove diff
473478
fi
474-
# Install the exact bytes we verified. Three independent gates, all on one
479+
# Install the exact bytes we verified. Four independent gates, all on one
475480
# downloaded copy — helm never re-fetches, so there is no window in which a
476481
# hostile cache can substitute different bytes between check and install:
477482
#
478-
# 1. SHA256 matches the .settings.yaml pin — fixes *which* release.
483+
# 1. SHA256 matches the .settings.yaml pin — fixes *which* bytes.
479484
# 2. The .prov clearsign carries VALIDSIG for the pinned maintainer
480485
# fingerprint — proves upstream authorship.
481486
# 3. The SHA256 recorded inside that signed .prov equals the pin
482487
# — binds (2) to (1), so the
483-
# signature covers our bytes rather than some other signed release.
488+
# signature covers our bytes, not some other signed release.
489+
# 4. The version inside that signed .prov equals the requested version
490+
# — the only gate that survives
491+
# a poisoned pin; see its comment below.
484492
#
485-
# Gate 3 is what lets us pass --verify=false safely. Helm's own --verify
486-
# only establishes (2): it would accept any validly signed helm-diff
487-
# release, including an older one replayed by a compromised cache, which
488-
# is a downgrade past the pin. Doing the checks here is strictly stronger.
493+
# Gate 1 is the load-bearing control against a *replayed older release*: a
494+
# stale tarball carries a genuine upstream signature but the wrong hash, so
495+
# it is rejected before any signature is examined. That is what helm's own
496+
# --verify cannot do — it establishes only (2) and would accept any validly
497+
# signed release — and it is why passing --verify=false here is strictly
498+
# stronger rather than weaker. Gates 3 and 4 cover what Gate 1 cannot: a pin
499+
# that was poisoned at refresh time, when the hash alone proves nothing.
489500
# The cost is a cosmetic `PROVENANCE: unsigned` in `helm plugin list`.
490501
#
491502
# The tarball is staged as diff.tgz because helm's local-tarball installer
@@ -535,22 +546,49 @@ phase_install() {
535546
echo "::error::helm-diff release key fingerprint mismatch (expected ${HELM_DIFF_KEY_FPR})" >&2
536547
exit 1
537548
fi
549+
# VALIDSIG's first field is the fingerprint of the key that actually made
550+
# the signature and its last field is the primary key's, which differ once
551+
# a maintainer signs with a dedicated subkey. Accept the pin in either
552+
# position so a legitimate future subkey does not false-reject.
538553
if ! gpg --verify --status-fd=1 "${tarball}.prov" 2>/dev/null \
539-
| grep -q "^\[GNUPG:\] VALIDSIG ${HELM_DIFF_KEY_FPR} "; then
554+
| awk -v fpr="${HELM_DIFF_KEY_FPR}" \
555+
'$1=="[GNUPG:]" && $2=="VALIDSIG" && ($3==fpr || $NF==fpr) {found=1} END{exit !found}'; then
540556
echo "::error::helm-diff provenance is not validly signed by ${HELM_DIFF_KEY_FPR}" >&2
541557
exit 1
542558
fi
543559

560+
# Decode once: gates 3 and 4 both read the signed body.
561+
prov_body="$(gpg --decrypt "${tarball}.prov" 2>/dev/null)"
562+
544563
# Gate 3: that signature covers the bytes we just pinned. The .prov keys
545564
# its files map by the upstream asset name, not our staged filename.
546-
prov_sha="$(gpg --decrypt "${tarball}.prov" 2>/dev/null \
547-
| awk -v a="${HELM_DIFF_FILE}:" '$1==a {gsub(/"|sha256:/,"",$2); print $2}')"
565+
prov_sha="$(awk -v a="${HELM_DIFF_FILE}:" \
566+
'$1==a {gsub(/"|sha256:/,"",$2); print $2}' <<<"${prov_body}")"
548567
if [[ "${prov_sha}" != "${HELM_DIFF_SHA256}" ]]; then
549568
echo "::error::helm-diff provenance records sha256 ${prov_sha:-<none>} for ${HELM_DIFF_FILE}, pinned ${HELM_DIFF_SHA256}" >&2
550569
exit 1
551570
fi
552-
echo "helm-diff verified (${HELM_DIFF_SHA_KEY}: ${HELM_DIFF_SHA256}, signed by ${HELM_DIFF_KEY_FPR})"
553571

572+
# Gate 4: the signed body names the release we asked for. Gates 1-3 all
573+
# reduce to "these bytes match the pin", so they cannot detect a pin that
574+
# was itself poisoned: tools/update-helm-diff-checksums derives the pin
575+
# from an *unsigned* checksums.txt, and the asset filename carries no
576+
# version, so a compromised release/CDN can seed the pin with an older
577+
# release's hashes and then serve that older, genuinely signed tarball
578+
# here. Only the version inside the signature distinguishes them.
579+
prov_ver="$(awk '$1=="version:" {gsub(/"/,"",$2); print $2; exit}' <<<"${prov_body}")"
580+
if [[ "${prov_ver}" != "${HELM_DIFF_VERSION#v}" ]]; then
581+
echo "::error::helm-diff provenance is for version ${prov_ver:-<none>}, requested ${HELM_DIFF_VERSION#v} — possible downgrade" >&2
582+
exit 1
583+
fi
584+
echo "helm-diff verified (${HELM_DIFF_SHA_KEY}: ${HELM_DIFF_SHA256}, version ${prov_ver} signed by ${HELM_DIFF_KEY_FPR})"
585+
586+
# --verify=false skips helm's provenance check, not helm-diff's install
587+
# hook, which still runs. The gates above constrain the .tgz bytes, not
588+
# what the hook does: today it finds the bundled diff/bin/diff and skips
589+
# downloading, so nothing unverified is fetched. A bump that stops
590+
# shipping that prebuilt binary would silently reintroduce a fetch and
591+
# must be caught in review.
554592
helm plugin install "${tarball}" --verify=false
555593
) || exit 1
556594
fi

0 commit comments

Comments
 (0)