ci(security): give the image scans their own code-scanning tool (#2266) - #2396
Conversation
📝 WalkthroughWalkthroughThe Trivy image workflow now renames the SARIF tool identifier to ChangesTrivy SARIF normalization
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
Code scanning reports "N configurations not found" per tool, not per category, and the Trivy tool currently owns nineteen: trivy-fs and trivy-helm, which every pull request produces, plus seventeen trivy-image:* that only this schedule-only workflow can produce, because it scans published images and a pull request has none. So the Trivy check carries a permanent list of missing configurations — neutral usually, failure when the change touches files they could have covered — and buries the case it exists for, a scanner that quietly stopped uploading. Semgrep OSS sits next to it green for exactly this reason: its one category is produced everywhere. Renaming the driver in the image SARIF splits the two apart. Trivy keeps only the categories every run produces and its check goes clean; the image findings stay in the Security tab under their own tool. Nothing about the scan, the summary or the critical gate changes. The seventeen categories already stored under Trivy do not move and will keep the old check red until an admin deletes their analyses; a write account gets 403 from that endpoint. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
030afdc to
320935d
Compare
| # can produce these categories — keep them off the Trivy check PRs see (#2266). | ||
| if: ${{ always() && hashFiles('trivy-image.sarif') != '' }} | ||
| run: | | ||
| python3 - <<'PY' |
Same two fields of behaviour, eight lines shorter, and it matches how the rest of the workflows read and reshape JSON. Verified against a real Trivy image SARIF: the jq output parses equal to what the python version produced, only driver.name differs from the input, and the 25 results and 24 rules survive. jq's assignment also creates the intermediate objects, so a SARIF without tool.driver still comes out renamed rather than untouched — that was the reason for setdefault. The output goes to a temporary file and is moved over the original; redirecting onto the input would truncate it before jq reads it. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/trivy-images.yml:
- Around line 141-145: Add id normalize_sarif to the SARIF normalization step
containing jq and mv, then update the upload step’s if condition to require
steps.normalize_sarif.outcome == 'success' in addition to the existing checks.
🪄 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: 7810269f-eebb-4652-821f-ec12de881f30
📒 Files selected for processing (1)
.github/workflows/trivy-images.yml
| if: ${{ always() && hashFiles('trivy-image.sarif') != '' }} | ||
| run: | | ||
| set -euo pipefail | ||
| jq '.runs[].tool.driver.name = "Trivy Images"' trivy-image.sarif > renamed.sarif | ||
| mv renamed.sarif trivy-image.sarif |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
fd -a 'trivy-images.yml|trivy' .github 2>/dev/null || true
echo "== target section =="
if [ -f .github/workflows/trivy-images.yml ]; then
nl -ba .github/workflows/trivy-images.yml | sed -n '120,165p'
fi
echo "== normalization/upload references =="
rg -n "normalize_sarif|trivy-image\.sarif|rename|rm|jq|Upload SARIF|actionlint" .github/workflows/trivy-images.yml || true
echo "== nearby workflow structure =="
rg -n "jobs:|trivy-image-scan|upload|sarif" .github/workflows/trivy-images.yml || trueRepository: constructorfabric/insight
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/trivy-images.yml')
if not p.exists():
raise SystemExit(0)
text = p.read_text()
lines = text.splitlines()
start = next((i for i,l in enumerate(lines,1) if 'Give the image scans their own tool name' in l), None)
upload_start = next((i for i,l in enumerate(lines,1) if 'Upload SARIF to GitHub Code Scanning' in l), None)
for section in [(start, 'normalize section'), (upload_start, 'upload section')]:
idx, name = section
if idx is None:
print(f'{name}: not found')
continue
# print until next indent 8 at same or lower level? rough
end = next((i for i,l in enumerate(lines[idx:], idx+1) if l.startswith(' - name: ') or l.startswith(' - name: ') or i==len(lines)), min(len(lines)+1, idx+25))
print(f"\n== {name} lines {idx}-{end-1} ==")
for n,line in enumerate(lines[idx:min(end, len(lines)+1)], idx):
print(f'{n}:{line}')
PYRepository: constructorfabric/insight
Length of output: 1262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/trivy-images.yml')
text = p.read_text()
lines = text.splitlines()
print("== normalization step context ==")
for i in range(128, 147):
print(f"{i+1}: {lines[i]}")
print("\n== upload step context ==")
for i in range(146, 155):
print(f"{i+1}: {lines[i]}")
print("\n== normalize_sarif id and success guard references ==")
for i,l in enumerate(lines):
if 'normalize_sarif' in l or "steps." in l:
print(f"{i+1}: {l}")
PYRepository: constructorfabric/insight
Length of output: 1978
Gate the upload on successful SARIF normalization.
The normalize step does not set an id, and the upload step still uses hash-only existence checks. If jq or mv fails, the original trivy-image.sarif may remain and still be uploaded. Add id: normalize_sarif to the normalize step and require steps.normalize_sarif.outcome == 'success' in the upload condition.
🤖 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/workflows/trivy-images.yml around lines 141 - 145, Add id
normalize_sarif to the SARIF normalization step containing jq and mv, then
update the upload step’s if condition to require steps.normalize_sarif.outcome
== 'success' in addition to the existing checks.
Splits the noisy half of the
Trivycheck away from the useful half, without giving up the image findings. Follow-up to #2266.The mechanism
Code scanning creates one check per tool, not per category, and reports that tool's missing configurations. Today the
Trivytool owns nineteen categories:trivy-fstrivy.yml(pull_request, push, schedule)trivy-helmtrivy.ymltrivy-image:*× 17trivy-images.yml(schedule only)trivy-images.ymlscans published images, which a pull request does not have, so those seventeen can never be produced anywhere except the nightly cron. TheTrivycheck therefore reports them missing on every analysis this repository uploads — on PRs and on pushes tomainalike:Semgrep OSSsits beside it reportingsuccess — No new alerts in code changed by this pull request, for one reason only: its single category is produced everywhere.The statement is accurate and permanently unactionable, and it costs more than noise — the check is genuinely useful for
trivy-fs,trivy-helmandsemgrep, so if one of those ever stopped uploading, this is what would say so, under seventeen bullet points nobody reads.What this PR does
One step, before the upload, rewrites
runs[].tool.driver.namein the image SARIF fromTrivytoTrivy Imageswith jq. That is the field code scanning keys the tool on, so the two sets separate:Trivykeepstrivy-fsandtrivy-helm— both produced by every PR, so its check goes clean, like Semgrep's.Trivy Imagesowns the seventeen image categories, produced by the nightly.What stays exactly as it is
The scan, the per-image summary table,
Fail on a fixable critical(#2238), the per-imagecategory:, the.trivyignorehandling, and the alerts themselves. The image findings remain in the Security tab — only the tool they are filed under changes.+14lines, nothing removed.What to look at in the nightly
https://github.com/constructorfabric/insight/actions/workflows/trivy-images.yml — filter Event: schedule. One job per image; the run page's Summary block carries a table per image, grouped by package:
Latest run for reference: https://github.com/constructorfabric/insight/actions/runs/31359228269, and its
insight-toolboxjob https://github.com/constructorfabric/insight/actions/runs/31359228269/job/93364623071 — that image is where 10 of the 11 open image alerts sit.A red job in that workflow means a fixable critical appeared in a published image. Everything else is reported and not gated there; the gate that blocks a bad build lives in
.github/actions/image-cve-gate.Alerts, after this lands, filtered by the new tool:
https://github.com/constructorfabric/insight/security/code-scanning?query=is%3Aopen+tool%3A%22Trivy+Images%22
Test plan
driver.nameTrivy→Trivy Images, every other driver field identical, 25 results and 24 rules preserved, still valid JSON.toolat all comes out as{"driver":{"name":"Trivy Images"}}, so a malformed input is renamed rather than silently skipped.mv; redirecting onto the input would truncate it before jq reads it..github/workflows/trivy-images.ymlparses; thescanjob now has six steps in the order Checkout, scan, summary, rename, upload, gate.actionlint: 8 findings onmain, 8 on the branch, identical set; none intrivy-images.yml.Trivy Images, the summary tables are unchanged.Trivycheck reportssuccessinstead of listing configurations.Known unknown
Whether a check appears for
Trivy Imageson pull requests, where it uploads nothing. Both existing tools always upload, so there is no way to observe this before merging. If one does appear and is red, the fallback is to drop the upload entirely — that was this branch's previous revision and it is one revert away.After merge, one admin action
The seventeen categories already recorded under the old tool do not move; the
Trivycheck keeps listing them until their analyses are deleted. That is 261 analyses, andDELETE /repos/.../code-scanning/analyses/{id}answers403 You are not authorized to delete analysesfor awriteaccount, so it needs someone in @constructorfabric/admins. Order matters: this PR first, deletion after, otherwise the nightly re-registers them underTrivythe same night.Summary by CodeRabbit