DAT-21268: Persist vulnerability scan results to scan-results branch#514
DAT-21268: Persist vulnerability scan results to scan-results branch#514
Conversation
After vulnerability scans complete, a new persist-results job downloads all scan artifacts and commits them to a scan-results branch. This gives the Liquibase Security dashboard persistent access to historical scan data via GitHub Contents API, replacing the 7-day artifact retention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds automation to persist published-image vulnerability scan artifacts into a long-lived scan-results branch (for consumption via GitHub Contents API), extending access beyond the default artifact retention period.
Changes:
- Introduces
scripts/persist-scan-results.shto collect scan artifacts, update amanifest.json, and push results toscan-results. - Updates the published-images scanning workflow to add a
persist-resultsjob that runs after scans and pushes results to the branch. - Adjusts GitHub Actions
contentspermissions to enable branch pushes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/persist-scan-results.sh | New script to assemble artifact outputs, write per-tag metadata, maintain a manifest, and commit/push to scan-results. |
| .github/workflows/trivy-scan-published-images.yml | Adds the persistence job and elevates contents permission to support pushing scan results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The artifacts directory was passed as a relative path but the script cd's into a temporary git worktree, breaking the glob. Convert to absolute path at the start. Also fix grep -c exit code handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add parentheses around `sort_by(...) | reverse` so that `reverse` applies to the sorted array inside each image entry, not to the outer `with_entries` object — which caused "Cannot index object with number" at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Data-driven image name parsing instead of hardcoded prefixes - Proper semver-aware version sorting (non-semver tags appended) - Least-privilege: contents:write only on persist-results job - Fix grep -c fragility under set -euo pipefail
|
Claude finished @jandroav's task in 1m 4s —— View job Code Review: DAT-21268 - Persist vulnerability scan results
All previous issues resolved ✅Hardcoded image prefixes ( Workflow-level Non-semver version sort ( LGTM — no significant issues remaining. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a GitHub Actions job and a new script to persist Trivy/Grype vulnerability scan artifacts into a dedicated Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant Artifacts as Artifact Storage
participant Checkout as Repo Checkout
participant Worktree as Git Worktree
participant FS as File System
GHA->>Checkout: actions/checkout
GHA->>Artifacts: download vulnerability-report-* artifacts
Artifacts-->>GHA: artifacts downloaded
GHA->>FS: extract scan-artifacts/
GHA->>Worktree: check or create scan-results branch (worktree)
alt branch exists
Worktree->>Checkout: fetch & checkout branch
else branch missing
Worktree->>Worktree: create orphan branch + init manifest
end
loop per artifact (vulnerability-report-*)
FS->>FS: parse org/image:tag from dir name
FS->>FS: copy trivy/grype results into org/image/tag
FS->>FS: write metadata.json and update manifest in-memory
end
FS->>FS: mark missing expected scans (EXPECTED_MATRIX) as failed
FS->>FS: sort tags (semver desc), update manifest.lastUpdated
FS->>Worktree: stage changes
Worktree->>Checkout: commit & push to scan-results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/trivy-scan-published-images.yml:
- Around line 60-63: The persist-results job currently runs on failure and
skipped legs because its if condition uses always() &&
needs.vulnerability-scan.result != 'cancelled'; change this to gate on full
success by replacing the condition with needs.vulnerability-scan.result ==
'success' (or alternatively add explicit handling to record failures for each
matrix leg), so the "persist-results" job (job id persist-results / name Persist
Scan Results) only runs when the entire vulnerability-scan job set succeeded and
does not publish a fresh manifest after a failed scan matrix.
In `@scripts/persist-scan-results.sh`:
- Around line 207-213: The current push to origin "$BRANCH" can fail with a
non-fast-forward when concurrent runs race; add a retry path around the git push
that fetches and rebases the latest origin branch and retries the push (e.g.
loop up to N attempts: git fetch origin "$BRANCH", git rebase origin/"$BRANCH"
(or git pull --rebase origin "$BRANCH"), resolve/retry the commit/push, with
short backoff between attempts) and preserve CHANGED_FILES/CHANGED_COUNT logic;
reference the variables CHANGED_FILES, CHANGED_COUNT and BRANCH and ensure the
final push only exits with failure after retries are exhausted.
- Around line 145-169: The script currently copies available files in the FILE
loop but proceeds to write metadata.json, git add DEST_DIR and update MANIFEST
even if some scanner JSONs are missing, producing mixed old/new results; change
the logic to first verify presence of all required JSON artifacts (e.g.,
trivy-surface.json, trivy-deep.json, grype-results.json or a JSON variant of
grype) under ARTIFACT_PATH and abort with a non-zero exit (and no git/manifest
changes) if any are missing, or alternatively stage copies into a temporary
directory and only move/commit DEST_DIR, write metadata.json and update MANIFEST
(using SCANNED_AT, IMAGE_PATH, TAG, GITHUB_RUN_ID and MANIFEST) after the
complete set has been validated; ensure no partial writes occur when exiting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e06059e2-1e51-416d-9e87-a65bca435668
📒 Files selected for processing (2)
.github/workflows/trivy-scan-published-images.ymlscripts/persist-scan-results.sh
| persist-results: | ||
| name: Persist Scan Results | ||
| needs: vulnerability-scan | ||
| if: always() && needs.vulnerability-scan.result != 'cancelled' |
There was a problem hiding this comment.
Don't publish a fresh manifest after a failed scan matrix.
This condition still runs on failure and skipped. Because the script rewrites lastUpdated and only updates the artifacts it sees, a broken scan leg leaves stale data in scan-results while making the manifest look current. If partial persistence is intentional, record failures explicitly; otherwise gate this job on full success.
Suggested fix
persist-results:
name: Persist Scan Results
needs: vulnerability-scan
- if: always() && needs.vulnerability-scan.result != 'cancelled'
+ if: needs.vulnerability-scan.result == 'success'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| persist-results: | |
| name: Persist Scan Results | |
| needs: vulnerability-scan | |
| if: always() && needs.vulnerability-scan.result != 'cancelled' | |
| persist-results: | |
| name: Persist Scan Results | |
| needs: vulnerability-scan | |
| if: needs.vulnerability-scan.result == 'success' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/trivy-scan-published-images.yml around lines 60 - 63, The
persist-results job currently runs on failure and skipped legs because its if
condition uses always() && needs.vulnerability-scan.result != 'cancelled';
change this to gate on full success by replacing the condition with
needs.vulnerability-scan.result == 'success' (or alternatively add explicit
handling to record failures for each matrix leg), so the "persist-results" job
(job id persist-results / name Persist Scan Results) only runs when the entire
vulnerability-scan job set succeeded and does not publish a fresh manifest after
a failed scan matrix.
| for FILE in trivy-surface.json trivy-deep.json grype-results.json; do | ||
| if [ -f "$ARTIFACT_PATH/$FILE" ]; then | ||
| cp "$ARTIFACT_PATH/$FILE" "$DEST_DIR/$FILE" | ||
| fi | ||
| done | ||
|
|
||
| # Also check for grype-results.json variants (some workflows output grype-results.sarif too) | ||
| # We only need the JSON | ||
|
|
||
| # Create metadata.json | ||
| cat > "$DEST_DIR/metadata.json" <<EOF | ||
| { | ||
| "scannedAt": "$SCANNED_AT", | ||
| "image": "$IMAGE_PATH", | ||
| "tag": "$TAG", | ||
| "workflowRunId": "${GITHUB_RUN_ID:-}" | ||
| } | ||
| EOF | ||
|
|
||
| git add "$DEST_DIR/" | ||
|
|
||
| # Update manifest in memory — add tag to image list if not already present | ||
| MANIFEST=$(echo "$MANIFEST" | jq --arg img "$IMAGE_PATH" --arg tag "$TAG" ' | ||
| .images[$img] = ((.images[$img] // []) | if index($tag) then . else . + [$tag] end) | ||
| ') |
There was a problem hiding this comment.
Require a complete scan artifact before publishing a version.
This block silently skips missing JSON files, then still writes fresh metadata.json and updates manifest.json. On reruns that also means an old scanner output can remain in scan-results beside new files, so the dashboard sees a mixed old/new result as if it were complete.
Suggested fix
- for FILE in trivy-surface.json trivy-deep.json grype-results.json; do
- if [ -f "$ARTIFACT_PATH/$FILE" ]; then
- cp "$ARTIFACT_PATH/$FILE" "$DEST_DIR/$FILE"
- fi
- done
+ MISSING_FILES=()
+ for FILE in trivy-surface.json trivy-deep.json grype-results.json; do
+ if [ -f "$ARTIFACT_PATH/$FILE" ]; then
+ cp "$ARTIFACT_PATH/$FILE" "$DEST_DIR/$FILE"
+ else
+ MISSING_FILES+=("$FILE")
+ fi
+ done
+
+ if [ "${`#MISSING_FILES`[@]}" -gt 0 ]; then
+ echo "Error: artifact $ARTIFACT_NAME is missing required files: ${MISSING_FILES[*]}" >&2
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for FILE in trivy-surface.json trivy-deep.json grype-results.json; do | |
| if [ -f "$ARTIFACT_PATH/$FILE" ]; then | |
| cp "$ARTIFACT_PATH/$FILE" "$DEST_DIR/$FILE" | |
| fi | |
| done | |
| # Also check for grype-results.json variants (some workflows output grype-results.sarif too) | |
| # We only need the JSON | |
| # Create metadata.json | |
| cat > "$DEST_DIR/metadata.json" <<EOF | |
| { | |
| "scannedAt": "$SCANNED_AT", | |
| "image": "$IMAGE_PATH", | |
| "tag": "$TAG", | |
| "workflowRunId": "${GITHUB_RUN_ID:-}" | |
| } | |
| EOF | |
| git add "$DEST_DIR/" | |
| # Update manifest in memory — add tag to image list if not already present | |
| MANIFEST=$(echo "$MANIFEST" | jq --arg img "$IMAGE_PATH" --arg tag "$TAG" ' | |
| .images[$img] = ((.images[$img] // []) | if index($tag) then . else . + [$tag] end) | |
| ') | |
| MISSING_FILES=() | |
| for FILE in trivy-surface.json trivy-deep.json grype-results.json; do | |
| if [ -f "$ARTIFACT_PATH/$FILE" ]; then | |
| cp "$ARTIFACT_PATH/$FILE" "$DEST_DIR/$FILE" | |
| else | |
| MISSING_FILES+=("$FILE") | |
| fi | |
| done | |
| if [ "${`#MISSING_FILES`[@]}" -gt 0 ]; then | |
| echo "Error: artifact $ARTIFACT_NAME is missing required files: ${MISSING_FILES[*]}" >&2 | |
| exit 1 | |
| fi | |
| # Also check for grype-results.json variants (some workflows output grype-results.sarif too) | |
| # We only need the JSON | |
| # Create metadata.json | |
| cat > "$DEST_DIR/metadata.json" <<EOF | |
| { | |
| "scannedAt": "$SCANNED_AT", | |
| "image": "$IMAGE_PATH", | |
| "tag": "$TAG", | |
| "workflowRunId": "${GITHUB_RUN_ID:-}" | |
| } | |
| EOF | |
| git add "$DEST_DIR/" | |
| # Update manifest in memory — add tag to image list if not already present | |
| MANIFEST=$(echo "$MANIFEST" | jq --arg img "$IMAGE_PATH" --arg tag "$TAG" ' | |
| .images[$img] = ((.images[$img] // []) | if index($tag) then . else . + [$tag] end) | |
| ') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/persist-scan-results.sh` around lines 145 - 169, The script currently
copies available files in the FILE loop but proceeds to write metadata.json, git
add DEST_DIR and update MANIFEST even if some scanner JSONs are missing,
producing mixed old/new results; change the logic to first verify presence of
all required JSON artifacts (e.g., trivy-surface.json, trivy-deep.json,
grype-results.json or a JSON variant of grype) under ARTIFACT_PATH and abort
with a non-zero exit (and no git/manifest changes) if any are missing, or
alternatively stage copies into a temporary directory and only move/commit
DEST_DIR, write metadata.json and update MANIFEST (using SCANNED_AT, IMAGE_PATH,
TAG, GITHUB_RUN_ID and MANIFEST) after the complete set has been validated;
ensure no partial writes occur when exiting.
| CHANGED_FILES=$(git diff --cached --name-only) | ||
| CHANGED_COUNT=0 | ||
| if [ -n "$CHANGED_FILES" ]; then | ||
| CHANGED_COUNT=$(echo "$CHANGED_FILES" | grep -c "metadata.json" || true) | ||
| fi | ||
| git commit -m "Update scan results ($CHANGED_COUNT version(s)) — $SCANNED_AT" | ||
| git push origin "$BRANCH" |
There was a problem hiding this comment.
This push path still races across workflow runs.
Every run commits locally and pushes straight to the same scan-results branch. If two runs reach this block together, one of them can lose the update on a plain non-fast-forward rejection, because git push refuses to update a branch that is no longer an ancestor of the commit being pushed. Either serialize the writer job or add a retry path that refreshes from origin/$BRANCH before pushing. (git-scm.com)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/persist-scan-results.sh` around lines 207 - 213, The current push to
origin "$BRANCH" can fail with a non-fast-forward when concurrent runs race; add
a retry path around the git push that fetches and rebases the latest origin
branch and retries the push (e.g. loop up to N attempts: git fetch origin
"$BRANCH", git rebase origin/"$BRANCH" (or git pull --rebase origin "$BRANCH"),
resolve/retry the commit/push, with short backoff between attempts) and preserve
CHANGED_FILES/CHANGED_COUNT logic; reference the variables CHANGED_FILES,
CHANGED_COUNT and BRANCH and ensure the final push only exits with failure after
retries are exhausted.
When some vulnerability scan matrix jobs fail, the persist step now compares expected matrix entries against actual artifacts and writes status "failed" metadata for missing scans instead of silently omitting them. Adds scan_status to manifest.json for dashboard visibility.
|
Claude finished @jandroav's task in 1m 57s —— View job Review: DAT-21268 latest changes
The
|
Checks for all three required scan files (trivy-surface.json, trivy-deep.json, grype-results.json) before persisting. Missing files are logged as warnings and recorded in metadata.json's missing_files array so the dashboard can flag incomplete scan data.
|
Claude finished @jandroav's task in 2m 8s —— View job Review: latest commits (3b74b31, bed7a6f)
Fixes verified ✅
One remaining issue
|
|
Claude finished @jandroav's task in 1m 52s —— View job Review: DAT-21268 latest changes
Two remaining issues from prior review rounds have been fixed in commit
No other significant issues found. |
Summary
scripts/persist-scan-results.shto download scan artifacts and commit them to a persistentscan-resultsbranchpersist-resultsjob to the published images scanning workflow that runs after all scans completecontentspermission fromreadtowriteto allow pushing to thescan-resultsbranchThis enables the Liquibase Security dashboard to read historical scan data via GitHub Contents API, replacing the 7-day artifact retention window.
Branch structure
Test plan
gh workflow run "Published Images Vulnerability Scanning"scan-resultsbranch is created withmanifest.jsonand versioned scan datamanifest.jsonlists all scanned images and tagstrivy-surface.json,trivy-deep.json,grype-results.json,metadata.json🤖 Generated with Claude Code