ci(security): fail the build on a fixable critical in an image (#2018) - #2238
Conversation
Two gates, both on CRITICAL with --ignore-unfixed, matching what the issue asks for. Publish side: a new composite action scans the push-by-digest blobs in each merge job, before docker/metadata-action's tags are applied. A rejected build therefore keeps no build tag and does not move :latest, so no descriptor bump and no chart can reference it. It covers the seven jobs that produce the fourteen images the nightly scans; insight-ui-tests stays out, as it is out of that matrix. Drift side: trivy-images.yml gets a verdict step. It runs last and reads the JSON the scan already wrote, so the job summary and the SARIF upload still happen for a failing image -- putting --exit-code 1 on the scan itself would abort before `trivy convert` and lose both. Gating on Trivy's own severity rather than on the Code Scanning level is deliberate: GitHub derives its level from the CVSS score, so CVE-2026-59873 (#2014) was CRITICAL for Trivy and high in the Security tab. Note that `--severity` is undocumented on `trivy convert` yet honoured, which is one more reason the image stays pinned by digest. Baseline measured before enabling: insight-toolbox 0 critical after #2168, insight-gateway, insight-front and source-salesforce-insight also 0. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesImage CVE enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/actions/image-cve-gate/action.yml (1)
54-57: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the Trivy cache across digest scans.
Each
docker run --rmstarts without a host mount for/root/.cache, so Trivy may redo vulnerability cache setup for every digest image. Mount/tmp/trivy-cache:/root/.cachebefore the digest loop, and create it withmkdir -p /tmp/trivy-cache.🤖 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 @.github/actions/image-cve-gate/action.yml around lines 54 - 57, Update the Trivy digest-scan docker run command to reuse a host cache by creating /tmp/trivy-cache before the digest loop and mounting it at /root/.cache alongside the existing workspace volume. Apply this to the docker invocation using TRIVY_IMAGE without changing the scan behavior.
🤖 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.
Inline comments:
In @.github/workflows/build-images.yml:
- Around line 385-390: Add a pinned actions/checkout step immediately before
each local image-cve-gate invocation in .github/workflows/build-images.yml at
lines 385-390, 496-501, 607-612, 732-737, 866-871, 1042-1047, and 1309-1314,
ensuring the repository is checked out before the local composite action is
loaded.
---
Nitpick comments:
In @.github/actions/image-cve-gate/action.yml:
- Around line 54-57: Update the Trivy digest-scan docker run command to reuse a
host cache by creating /tmp/trivy-cache before the digest loop and mounting it
at /root/.cache alongside the existing workspace volume. Apply this to the
docker invocation using TRIVY_IMAGE without changing the scan behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cd63d666-7efd-4060-acc0-06606491232d
📒 Files selected for processing (3)
.github/actions/image-cve-gate/action.yml.github/workflows/build-images.yml.github/workflows/trivy-images.yml
Two measured tweaks to the image gate. The docker run now mounts the same /tmp/trivy-cache the nightly workflow uses, so the second digest in a job is a cache hit rather than another database download and image analysis: 79s -> 1.6s on the toolbox. And --scanners vuln drops the default secret pass, which the gate never reads, for about 13% off a cold run. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
A `uses: ./…` reference is resolved against the workspace when the runner loads the action, before any of its steps run, so the checkout inside the composite was too late -- the merge jobs have no working tree and every one of them would have failed to find action.yml. The checkout moves to the caller, where it also covers the .trivyignore the scan reads. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
3 participants
Closes the second half of #2018 — "a published image with fixable critical CVEs fails a check". The findings half is already done: #2012 and #2013 are closed and #2014 landed this morning as #2168.
Two gates, both
--severity CRITICAL --ignore-unfixed, matching the issue's acceptance (criticals only, fixable only).Publish gate —
build-images.ymlThe per-arch build pushes by digest with no tag (
push-by-digest=true); tags appear later, inmerge-<service>, viadocker/metadata-action. That gap is where the gate goes: a new composite action scans the digests immediately beforeCreate multi-arch manifest and push.A rejected build therefore gets no build tag and does not move
:latest, sobump-descriptorsandpublish-charthave nothing to reference and the gitops poller never sees a new version. The blob stays in the registry, unreferenced.Covers the seven merge jobs behind the fourteen images the nightly scans (
insight-jira-enrichrides the connector matrix inmerge-image).insight-ui-testsis out, matching its absence from that matrix.Drift gate —
trivy-images.ymlA verdict step, placed last and reading the JSON the scan already produced.
Putting
--exit-code 1on the scan itself does not work:trivy convertruns inside the same step underset -euo pipefail, so a failure aborts before the SARIF exists, the upload skips itself onhashFiles(...) != '', and the summary exits on its own[ -f trivy-image.json ]guard. You would lose the alerts and the summary for exactly the images that failed.Why Trivy's severity and not the alert level
GitHub derives its level from the CVSS score in
security-severity, not from Trivy's label.CVE-2026-59873carriedCRITICALwith a CVSS of 7.5 from all three sources Trivy holds, so it appeared as high in the Security tab. A gate reading Code Scanning severities would have missed it.--severityis undocumented ontrivy convert(only--exit-codeis listed) but is honoured — one more reason the Trivy image stays pinned by digest.Cost
Measured locally. On a cold cache the largest image (
source-gitlab-insight, 209 MB compressed) takes 30 s with Trivy's default scanners and 26 s with--scanners vuln, which is what the gate uses — the default set adds a secret pass the gate never reads.The bigger term is the second digest in the same job. The run mounts the same
/tmp/trivy-cachethe nightly workflow uses, so it is a cache hit instead of another database download and image analysis: measured on the toolbox, 79 s → 1.6 s.That puts a merge job at roughly 30–45 s added — ~25–40 s for the first digest, a couple of seconds for the second. The seven jobs run in parallel and the last full
build-images.ymltook 29 min, so the wall-clock cost is well under a minute. A merge job itself currently takes ~34 s, so in relative terms it roughly doubles the cheapest jobs in the workflow. My link to GHCR is slower than a runner's, so treat these as upper bounds.Test plan
actionlint: 7 distinct findings before and after, identical set — no new lint.CRITICAL→ exit 0; the same image atHIGH(10 present) → exit 1; empty digests directory → exit 1, so a missing artefact fails loudly instead of passing silently.trivy convertverdict verified on real scan output:--severity CRITICAL --exit-code 1gives 0 on the post-fix(images): bump the toolbox to Node 24 and upgrade npm in place (#2166) #2168 toolbox JSON and 1 on the pre-fix(images): bump the toolbox to Node 24 and upgrade npm in place (#2166) #2168 one; without--severitythe same clean JSON gives 1, which proves the flag is applied.insight-toolbox0 critical / 10 high after fix(images): bump the toolbox to Node 24 and upgrade npm in place (#2166) #2168 (was 1 / 24),insight-gateway0 / 0,insight-front0 / 0,source-salesforce-insight0 / 1.workflow_dispatchontrivy-images.ymlafter merge, to confirm the baseline across all fourteen images at once.fail-fast: false, so any red job names its image.CRITICAL→ 0, same image atHIGH→ 1, empty digests directory → 1.mainbuild after merge passes the publish gate.Not addressed here
Recorded so they are not mistaken for done: the nightly still scans
:latestwhile deployments pin the build tag;--ignore-unfixedmeans neither gate can ever fire on a critical without a released fix; the fourteen-image matrix is still hand-maintained; and Trivy's SARIF still collapses findings that share a path across the seven connector images.Summary by CodeRabbit
New Features
Bug Fixes