DEP-91 (5/5): harden image release integrity - #1689
Open
racerxdl wants to merge 5 commits into
Open
Conversation
racerxdl
added
the
skip-release
racerxdl
changed the title
This was referenced Aug 6, 2026
racerxdl
marked this pull request as ready for review
August 6, 2026 21:18
PR Summary by QodoDEP-91 (7/7): Harden image release integrity end-to-end
AI Description
Diagram
High-Level Assessment
Files changed (16)
|
Code Review by Qodo
1. No retries on downloads
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+129
to
+132
| && curl --fail --location --silent --show-error \ | ||
| --proto '=https' --tlsv1.2 \ | ||
| "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/${dpkgArch}/kubectl" \ | ||
| --output "$artifact" \ |
There was a problem hiding this comment.
1. No retries on downloads 🐞 Bug ☼ Reliability
Dockerfile.tools switched kubectl/aws/sqlcmd downloads to fail-closed curl --fail, but the new download commands have no bounded retry policy, so transient CDN/network failures will now fail the Docker build immediately and can cause avoidable CI/release flakiness.
Agent Prompt
### Issue description
`Dockerfile.tools` now downloads kubectl/AWS CLI/sqlcmd using `curl --fail --location --silent --show-error` (good: fail-closed), but without `--retry` this makes the build brittle to transient network/CDN issues.
### Issue Context
This PR hardened integrity checks for manually downloaded binaries. To keep builds reliable, the download step should also be resilient. Other integrity-critical download paths in CI already use bounded retries.
### Fix Focus Areas
- Dockerfile.tools[127-155]
- Dockerfile.tools[205-213]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
racerxdl
force-pushed
the
dep-91-integrity
branch
from
August 10, 2026 14:17
88f06bb to
f73d1b1
Compare
Contributor
Migration Safety AnalysisNo database migrations were changed in this PR. Safe to deploy to sandbox. |
racerxdl
changed the title
Contributor
|
✅ Build Completed with Success, Version=1689.0.0-gf73d1b1 |
Verify downloaded tool archives and release rebuild inputs before publishing. Closes #DEP-91 Closes #1643 🤖 Generated with Mister Maluco Co-Authored-By: MisterMal <teskeslab@lucasteske.dev>
Allow Trivy invocations to fail closed without duplicate check arguments, and exercise both runner modes in pull request CI. Fixes #DEP-91 🤖 Generated with Mister Maluco Co-Authored-By: MisterMal <teskeslab@lucasteske.dev>
Four fixes to the release path, all cases where a control reported more assurance than it delivered: - The slim-agent CVE gate scanned amd64 candidates and then rebuilt before pushing, so arm64 was never scanned and the published bytes were not the scanned ones. Build each flavour/arch once, gate all four, and assemble the release tags from those exact images. - The SSM plugin (from a mutable latest/ path), the legacy MongoDB archive and the Oracle client were installed or executed unverified. Pin and verify them, and add a static gate so a future unverified download fails the PR that adds it. Oracle is pinned per arch: arm64 tracked a moving pointer, and converging it on the amd64 version would have downgraded arm64 agents. - The MongoDB and Google apt signing keys were trusted as downloaded. Assert their fingerprints and scope them with signed-by. The Microsoft key is dropped: no Microsoft repository is configured, so it granted archive-wide signing authority for nothing. - SBOM generation ran after the release was already public, so a failure left the release with none. Generate it first and make publishing depend on it. Also: a missing hoop_rs is the expected state in the slim images, so log it as info instead of an error, and refuse to render a defaultAgent sidecar pointed at a slim flavour, which cannot run its startup script. 🤖 Generated with Mister Maluco Co-Authored-By: MisterMal <teskeslab@lucasteske.dev>
The gate matched curl and its URL on one physical line and ended an instruction at any comment, so it saw 1 of the 8 download instructions in Dockerfile.tools — including none of the ones it was written to protect. Removing a verifier from the real file did not fail it. Flatten continuations into one logical instruction, skip comments without closing it, and judge exemptions over every URL the instruction names rather than per line, since the signing-key block passes its URLs as shell-function arguments. apt source declarations are configuration, not fetches, so they are excluded. Tests now assert against the real file's layout: that every download is seen, and that deleting a verifier from it is caught. Also distinguish a missing hoop_rs from an unusable one: only ErrNotFound is the expected slim-image case, so a configured but non-executable binary is reported as the error it is. 🤖 Generated with Mister Maluco Co-Authored-By: MisterMal <teskeslab@lucasteske.dev>
Independent review found the download gate certified recipes that install before they verify. It only checked that the verifier's name appeared somewhere in the RUN, so `curl x && dpkg -i x && verify x` and `curl x && verify y && dpkg -i x` both passed, and an exemption could be claimed by putting a trusted host's name in an attacker URL's path. Walk the commands in execution order, track which downloaded file each verifier actually covers, and fail when a consumer runs first. Match exemptions on parsed host and path, not substring. Two more from the same review: - The Helm defaultAgent guard matched a repository suffix, so it rejected a working custom sidecar at <registry>/hoophq/hoopagent while letting a digest-pinned official slim image through. Match the official repository spellings exactly and read the tag with any @sha256 pin removed. - fetch_key accepted the first fingerprint in the file, so a response appending an attacker key passed and apt trusted both. Require exactly one primary key. Also stop the SBOM comment claiming more than it delivers: it gates the GitHub Release object, not the registry, because the image tags are pushed by earlier jobs. The optional-image branches it describes are gone, so the code and comments now say the same thing. 🤖 Generated with Mister Maluco Co-Authored-By: MisterMal <teskeslab@lucasteske.dev>
racerxdl
force-pushed
the
dep-91-integrity
branch
from
August 12, 2026 17:54
f73d1b1 to
42c1f19
Compare
Contributor
|
✅ Build Completed with Success, Version=1689.0.0-g42c1f19 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack (DEP-91)
Split from #1662 for reviewability, then rescoped:
mainhas since landed #1681 (dependency/toolchain CVEs) and #1693 (hoophq/hoopagent), which superseded two PRs of the original stack.DEP-91 (1/7): bump Go dependencies and toolchain for CVE fixes #1683 — Go dependency + toolchain CVE bumps— closed, superseded by fix(deps): patch govulncheck vulnerabilities and align CI toolchain to go1.26.5 #1681DEP-91 (3/7): add opt-in hoophq/hoopdev-minimal agent image #1685 — opt-in— closed, superseded by feat(agent): add hoophq/hoopagent image with minimal and distroless flavours #1693hoophq/hoopdev-minimalDEP-91 (1/5): agent-tools: drop kernel-header CVEs and modernize clean train #1684 — 1. agent-tools: drop kernel-header CVEs, modernize clean train
DEP-91 (2/5): gate hoopagent vulnerabilities and add govulncheck #1686 — 2. Vulnerability gates for hoopagent + govulncheck
DEP-91 (3/5): attach SBOM and bundled-tool manifest to releases #1687 — 3. SBOM + bundled-tool manifest on releases
DEP-91 (4/5): weekly base-OS rebuild of hoophq/hoopagent #1688 — 4. Weekly base-OS rebuild of hoopagent
DEP-91 (5/5): harden image release integrity #1689 — 5. Image release integrity hardening ← you are here
Merge in order, bottom-up. Each PR targets the one below it.
The remaining work is what
mainstill lacks: the fat image's kernel-header CVEs, and any vulnerability gate, SBOM, or scheduled OS patching forhoophq/hoopagent.What
Hardens how images and release artifacts are published:
govulncheckjob forhsh-tunneld, which ships from its own Go module and is not reachable from the client module's package graphWhy
Final layer: the supply chain should fail closed rather than publish something unverified.
Risk
Highest-surface PR in the stack (release workflow), which is why it merges last — everything below is green by then.
How to test
python3 -m unittest scripts/ci/test_generate_sbom_manifest.py nix-shell -p actionlint --run 'actionlint .github/workflows/release.yml'Expected: tests pass. Remaining actionlint findings (
GitHub-Linux-Arm-Runnerlabel,merge-archive) are pre-existing onmainon lines this PR does not touch.Part of DEP-91.
Automated by MisterMal