feat(cve-scan): preview findings in Slack

[vcauesantos]

Summary

A CVE notification that carried only per-severity counts made every alert look alike, so triage always started by opening the workflow run. This puts the findings themselves in the Slack message.

• cve-scan emits a slack-details preview of the findings at or above severity-threshold, grouped by package, plus a one-line tally of what fell below it.
• ci-test-notify gains an advisory layout (warning status, and run-link-position: bottom so the findings lead and the run link trails). Scanner, configuration and cancellation alerts keep the failure layout, since those really are failures.
• The preview is built in the adapter-neutral layer, so swapping scanner never changes what a notification consumer receives.

What a release scan looks like in Slack:

⚠️ CVE scan Warning

Image: ghcr.io/loft-sh/vcluster-pro:v0.38.0-rc.1
Threshold: high · Advisory only — release was not blocked.

Findings at or above threshold

• golang.org/x/net · 1 finding
   Invalid array index

• stdlib · 3 findings
   Out-of-bounds read (2), Integer overflow

Also detected: 1 medium finding, 1 low finding below the notification threshold.

Workflow: View workflow run

Things worth a reviewer's attention

Scanner strings reach a webhook. Package names and titles are influenced by an outside source, and Slack renders the section as mrkdwn, so each field has its control characters stripped, its &, < and > encoded, and its length bounded before it can be sent. A literal channel mention in a CVE title cannot become a real ping, and there is a test asserting exactly that.

Caps are ordered by weight, not by name. Three package groups, four titles per group. Both are sorted so the cap keeps what dominates the group. Otherwise a package with forty findings could be dropped in favour of one that happens to sort earlier alphabetically.

A marker that does not overstate severity. Groups are prefixed with a plain bullet rather than a red circle, which reads as "critical" in a CVE alert. The header icon carries the overall severity and each group states its own count.

The default posture is unchanged. Still advisory, notify-events still defaults to schedule,release, and no new inputs are required of existing callers.

Slack is not a gate. Both cve-scan notify steps are continue-on-error, so an outage, a rate limit or a rejected payload cannot fail the caller. slackapi/slack-github-action runs with errors: true, so without this a notification system could block a release, which contradicts the README's "a flaky scanner must never block a release".

Backwards compatibility

ci-test-notify has roughly 30 call sites across nine repositories, so the default path had to stay exactly as it was. Verified by differential testing the old and new scripts against each other rather than by inspection:

• build-payload.sh, 11 scenarios per locale (every status including empty and unrecognised, empty / multiline / UTF-8 details, the section and header truncation paths): byte-identical payloads under a UTF-8 locale, which is what the runners use.
• should-notify.sh, 12 scenarios (every status crossed with set and empty webhook): gate decision unchanged. warning already notified before this PR, because the gate only ever silenced cancelled and skipped. The old comment claimed otherwise, so this corrects the comment rather than the behaviour.

The one deliberate difference is under a POSIX locale, where the old code measured Slack's character limits in bytes: a 2000-character preview was cut to 1036 characters with a U+FFFD left at the seam. That case now renders in full. It only affects messages that were already being truncated.

run-link-position has a default and warning is additive, and nothing changes the caller's job environment, so this is a tag advance rather than a new major.

Test plan

Check	Result
make test-cve-scan	114 tests, covering the escaping, ordering, bounding and grouping of the preview
make test-ci-test-notify	36 tests, covering both run-link positions and the truncation path that has to keep the link attached
make lint (actionlint + zizmor)	clean
make check-docs	clean
End-to-end render	Piped a findings fixture through process-findings.sh into build-payload.sh and inspected the resulting Slack blocks, which is how the output above was produced

Two defects were found by that last check rather than by the suite. Both are fixed, each with a regression test:

1. Preview titles were sorted alphabetically ahead of a four-title cap, so the cap dropped titles by name instead of by weight.
2. In bash, "${array[*]}" joins on the first character of IFS only, so IFS=', ' rendered the below-threshold tally as 1 medium finding,1 low finding. The original test left exactly one severity below the threshold, which is the one case a broken separator still renders correctly.
3. The bottom-position truncation budget, 3000 - len(run link) - 2, could go negative, and a negative slice length in bash reads as "all but the last n". That would have overshot Slack's limit and lost the whole message to a rejection.

Two notes for anyone re-running this locally:

• On macOS, bash 3.2 does not fire errexit for a bare [[ ]], so a mid-test assertion that fails is still reported as ok and only the Ubuntu runner catches it. Trust CI over a local green here.
• make test is red on an unrelated, pre-existing cleanup-head-charts assertion that is already failing on main.

Closes DEVOPS-1414

[vcauesantos]

/vcluster-review

[vcluster-pr-approver]

🔍 The agent swarm is reviewing PR #240. I'll post the results here when it's done.

Admins can watch the run live

[loft-bot]

Panel review: 3 blocking, 6 quality notes, 10 lanes.

Blocking concerns

• .github/actions/cve-scan/action.yml:170 — a block-on-findings: true run that fails the job alerts as an amber warning and gets no failure notification at all.
• .github/actions/cve-scan/src/process-findings.sh:106 — the three-package preview cap keeps the alphabetically first packages, silently dropping the most-affected one. Reproduced.
• .github/actions/cve-scan/test/process_findings.bats:229 — the BLOCK_ON_FINDINGS branch is never exercised; swapping its two strings keeps the suite green.

The escaping work holds up: the security lane confirmed control-char stripping plus &/</> encoding closes both the mrkdwn mention-injection and the GITHUB_OUTPUT newline-forgery vectors, and write_multiline_output's random delimiter is the right shape for this repo.

What was checked

Checked: correctness, security, test-quality, operability, reuse, architecture, dead-code, typos, infra, PR metadata. Skipped: the four e2e lanes, gap analysis and entry-point fidelity (this repo has no e2e suite, and DEVOPS-1414 is an internal request rather than a customer bug), and the layout-coherence pass (not a restructure-shaped diff).

Adjacent / out-of-diff

• Open PR #160 adds an info status to the same case "$STATUS" block in build-payload.sh and rewrites the same HEADER= line to suppress an empty status suffix. Whichever merges second will conflict, and the two statuses are solving adjacent problems — worth coordinating before either lands.

Quality notes (non-blocking)

• consider — .github/actions/cve-scan/src/process-findings.sh:144 — slack-details is composed on every completed scan, but only reads correctly when a threshold finding exists, and that condition lives in action.yml's step if:, not here. With one low finding at severity-threshold: high the output is "Findings at or above threshold" followed by nothing. Gate the heading on THRESHOLD_COUNT > 0 and derive BLOCKING_CONTEXT from the same pair run.sh gates on. This becomes blocking if a future caller reads slack-details outside that if:.
• consider — .github/actions/ci-test-notify/README.md — the prose is now stale against the action's own behaviour: "Notification gating" still says only success and failure notify (the diff adds warning), and the "Message format" block only shows the top layout, so run-link-position: bottom is documented in the generated table and nowhere else.
• consider — .github/actions/cve-scan/action.yml:203 — the has-vulnerabilities == 'true' && 'FINDINGS…' arm of the details: ternary is unreachable now that this step requires has-vulnerabilities != 'true'. Dropping it would make the ternary match the three cases the comment above it describes.
• consider — .github/actions/ci-test-notify/build-payload.sh:39 — bottom does not only move the link, it re-renders it: top emits Build URL: <raw url> while bottom emits Workflow: <url|View workflow run>. The input is documented as choosing placement only, so a caller opting into bottom silently gets different wording and markup.
• nit — .github/actions/ci-test-notify/build-payload.sh:53 — the *) fallback duplicates the top) arm verbatim; top|*) with the warning emitted before the shared body would remove the copy.

(+1 more: a nit on process-findings.sh:89, where SLACK_IMAGE's inline if length > 200 … re-derives the clip($limit) helper defined a few lines below.)

[vcluster-pr-approver]

@vcauesantos ✅ Review finished for #240

View the session

[Piotr1215]

LGTM overall. I left one comment.