Verify the tool cache against reviewed hashes at link time

[wallrj]

Motivation

$(DOWNLOAD_DIR) (the tool cache) is persisted between CI runs, and for some jobs it is a node-local directory shared with less-trusted jobs that can overwrite a cached binary in place. A tool's SHA-256 is currently only checked when it is first downloaded, so a binary swapped in the cache afterwards is symlinked onto PATH and executed unverified. This is the tool-cache half of the cache-poisoning problem that #625 and cert-manager/cert-manager#8833 both set out to fix.

Approach

The reviewed SHA-256 already in this file is the trust anchor — no signing or fork-write-scoping needed. This PR makes the per-tool symlink a .PHONY target and, on every build, re-hashes the cached binary against that reviewed value before linking it. A mismatch deletes the binary, re-downloads it, and re-verifies the replacement, failing the build with an actionable message if it still does not match.

For that to work the reviewed hash must be the hash of the extracted binary, not the downloaded archive:

• the archive recipes now hash the binary after extraction (the download still fails closed via lock.sh if the hash is wrong);
• etcd and kube-apiserver gain their own binary hashes (they are extracted from the kubebuilder tarball and cached individually). If an extracted binary fails its check, the cached tarball is deleted along with it, so the next run re-downloads instead of re-extracting the same bad bytes forever;
• the SHAs for the affected tools were relearned with make learn-tools-shas. Bare-binary tools (kubectl, kind, …) were already hashed as binaries and are unchanged.

Consumers adding tools through ADDITIONAL_TOOLS must follow the same convention: the *_SHA256SUM variables hold the hash of the stored binary. An entry still using the old archive-hash convention now fails the build with a message saying exactly that, rather than silently re-downloading on every run.

The vendored Go toolchain is covered too: the cached go tarball is re-verified against its reviewed hash before extraction, healing a mismatch the same way. A poisoned Go toolchain would otherwise undermine the go.sum/GOSUMDB verification that the go-installed tools rely on. Those go-installed tools have no reviewed hash here — they are anchored by go.sum when built, and their staleness is already handled by #708 keying the download path on the Go toolchain version — so the link-time check skips them (empty hash variables are defined to keep --warn-undefined-variables quiet).

Dry runs are safe: GNU make executes recipe lines containing $(MAKE) even under -n/-q/-t, so the check guards on a dry_run variable derived from MAKEFLAGS (filtering out long options such as --warn-undefined-variables, which contains an n).

Relationship to the other PRs

• Stacked on Key go-installed tool binaries on the Go toolchain that builds them #708 — it refactors tool_defs into the $(XXX_DOWNLOAD_PATH) / tool_link_defs seam this change hooks into. The first two commits here are Key go-installed tool binaries on the Go toolchain that builds them #708's; review the commits after those. Rebase onto main once Key go-installed tool binaries on the Go toolchain that builds them #708 merges.
• Supersedes Add cache validation #625 — same reviewed-hash anchor and same "purge poisoned bytes" outcome, but automatic: Add cache validation #625 is a separate verify-cache target a job has to remember to run after restore and before any tool executes; here the check is part of building the symlink, so nothing can forget it.
• Supersedes make: check the hash at runtime instead of after downloading cert-manager#8833 — same "verify at point of use, not just at download" insight, rebased onto Key go-installed tool binaries on the Go toolchain that builds them #708's structure so it lives in makefile-modules and benefits every downstream repo. Credit to @maelvls (verify-at-use, binary hashes) and @inteon (whole-cache purge) — this folds both into one automatic check.

Testing

Locally, host platform (linux/amd64), against a consumer harness replicating repository-base:

• poisoned the cached helm binary and rebuilt: detected, deleted, re-downloaded, re-verified, linked; a further rebuild is a silent no-op;
• with a deliberately wrong reviewed hash, the build fails loudly ([verify] helm still does not match its reviewed hash after re-download; helm_linux_amd64_SHA256SUM must be the hash of the stored binary, not the archive) instead of looping;
• make -n with a poisoned cache prints the recipe but mutates nothing (previously it hashed and deleted for real); dry_run stays empty on normal runs despite MAKEFLAGS containing --warn-undefined-variables;
• corrupted the cached kubebuilder tarball: extraction fails, the tarball is deleted with a clear message, and the next run heals fully — including the binary's executable bit, which a masked tar failure could previously drop;
• replaced the cached Go tarball with garbage and removed the extracted goroot: make vendor-go detects the mismatch, re-downloads, re-verifies, and extracts in the same run;
• learn mode (what Renovate's postUpgradeTasks runs): with a stale hash and LEARN_FILE set, the build succeeds and the learn file records the correct s/old/new/g replacement;
• with Go vendoring enabled (this file appends the vendor-go goal to MAKE), both heal paths re-download via $(firstword $(MAKE)) without dragging vendor-go into the sub-make;
• a failed re-download (forced with CURL=/bin/false) fails the build with the download error, not a misleading hash message;
• steady-state runs no longer write to the download cache at all: the symlink recipe's touch followed the symlink and updated the cached binary's mtime on every run; the target is .PHONY now, so the touch is dropped;
• the relearned etcd linux/amd64 binary hash matches the value independently computed in make: check the hash at runtime instead of after downloading cert-manager#8833.

Known limitations

• Archives are parsed before they are hashed. Previously the archive hash gated tar/unzip; now the binary hash is checked after extraction, so an archive-parser vulnerability is reachable from a MITM'd or compromised download (the result still fails closed — the bad binary is never linked). Restoring the parse gate would mean keeping a second, archive-level hash per tool per platform plus learn-script support for both. I have left that as a maintainer call rather than doubling the hash tables preemptively; downloads come over TLS from the tools' release hosts.
• The window is narrowed, not closed. Verification happens at link time, not exec time, and outside lock.sh, so a writer to a shared cache could still swap a binary between the hash check and its use. Fully closing that needs verification at exec time (or a cache that less-trusted jobs cannot write to at all).
• The extracted goroot tree is not re-verified. In CI the goroot directory is not cached, so every run re-extracts from the (now verified) tarball; on a developer machine the extracted tree in ~/.cache is trusted once extracted. Re-verifying it would need a tree hash.
• PATH is process-wide (_bin/tools is prepended once for the whole make run), so this closes cache poisoning — every cache-backed tool on PATH is verified this run — but not the separate hygiene problem of a target using a tool it did not declare as $(NEEDS_*). That would need per-target PATH scoping and is worth its own issue.

Opening as a draft pending #708 and maintainer review.

with claude opus-4.8 and claude fable-5

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joshvanl for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

This PR hardens modules/tools/00_mod.mk against tool-cache poisoning by re-verifying cached artifacts against the reviewed SHA-256 values at link time (before placing tools on PATH), and by shifting archive-based tools to verify the extracted binary rather than the downloaded archive.

Changes:

• Adds a link-time cache integrity check for tools (with automatic purge + re-download on mismatch), and introduces a dry_run guard to avoid mutations during make -n/-q/-t.
• Re-verifies the cached vendored Go tarball immediately before extraction to prevent a poisoned Go toolchain from undermining downstream verification.
• Updates tool SHA-256 values/recipes so *_SHA256SUM represents the stored binary (post-extraction), and adds per-binary hashes for etcd and kube-apiserver extracted from the kubebuilder tarball.

Suppressed comments (1)

modules/tools/00_mod.mk:563

• Because $(bin_dir)/tools/$1 is now a .PHONY target, this rule runs on every make invocation that needs the tool. touch $@ will therefore update the mtime of the cached binary on every run (touch follows symlinks), causing unnecessary filesystem writes and potentially interfering with any consumers that key off mtimes in $(DOWNLOAD_DIR). Since the target is .PHONY, the touch is no longer needed.

	@# cd into tools dir and create relative symlink (e.g., ../downloaded/tools/helm@v4.0.1_darwin_arm64)
	@# patsubst converts absolute path to relative by replacing $(bin_dir) with ..
	@cd $$(dir $$@) && $$(LN) $$(patsubst $$(bin_dir)/%,../%,$$($(call uc,$1)_DOWNLOAD_PATH)) $$(notdir $$@)
	@touch $$@ # making sure the target of the symlink is newer than *_VERSION

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wallrj]

False positive: the pattern rule at $(bin_dir)/scratch/%_VERSION matches GO_TOOLCHAIN_VERSION, and the variable it stamps is defined in both vendoring branches (L351/L358). Verified empirically: make _bin/scratch/GO_TOOLCHAIN_VERSION writes go1.27.0 to the stamp file, and go-installed tool targets resolve with no missing-rule error.

with claude fable-5

[wallrj]

Good catch — fixed in 7581e90 with $(firstword $(MAKE)) rather than MAKE_COMMAND: keeping the literal $(MAKE) text on the recipe line means make still marks it as recursive, so the dry-run guard keeps working. Tested with vendoring enabled (MAKE = make vendor-go): a poisoned helm binary and a poisoned Go tarball both heal without dragging the vendor-go goal into the sub-make.

with claude fable-5

[wallrj]

Fixed in 7581e90: the sub-make now short-circuits with || exit 1, so a failed download surfaces its own error instead of the misleading hash message. Tested by forcing the download to fail with CURL=/bin/false.

with claude fable-5

[wallrj]

Demonstrated end to end in two consumer repos, with klone.yaml pointing the tools module at this branch (7581e90) and make/_shared/tools vendored via make generate-klone:

• DO NOT MERGE: Test tools-cache verify-at-use (makefile-modules#710) cert-manager#9187 — all presubmits green (make-verify, make-test, e2e-v1-36, e2e-v1-36-upgrade)
• DO NOT MERGE: Test tools-cache verify-at-use (makefile-modules#710) istio-csr#896 — all eleven presubmits green (verify, unit, the istio e2e matrix, ca-rotation, ambient; one istio-v1-29 flake — a pod-Pending port-forward in the runtime-configuration test, no relation to the tools module — passed on retest)

Both demo PRs are draft + /hold and will be closed once this merges; the module change reaches consumers through the normal klone bump.

with claude fable-5