ci(security): post scanner findings as sticky PR comments (#68) #95
No reviewers
Labels
No labels
area:auth
area:ci
area:db
area:infra
area:native
area:pwa
area:service
epic
feature
foundation
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
james/carol!95
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "68-sticky-scanner-comments"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #68.
Summary
Each security scanner —
npm_audit,osv_scan,image_scan,gitleaks— now posts its qualifying findings as a sticky comment on the PR in addition to writing them to$GITHUB_STEP_SUMMARY. Same Markdown, visible from the conversation view without clicking into run details.Structure
scripts/ci/security-findings.mjs(new) — shared parse + render module. The four scanner parsers and the two Markdown renderers (bucketed for npm/osv/trivy, list-shape for gitleaks) extracted fromsecurity-summary.mjsverbatim. Both CLI scripts import from here.scripts/ci/security-summary.mjs(refactored) — slimmed down to the step-summary CLI it always was. Behavior unchanged from a workflow caller's perspective (same args, same exit code, samequalifying=<n>last line, same Markdown).scripts/ci/post-pr-comment.mjs(new) — sticky upsert via Forgejo's/api/v1/repos/.../issues/{n}/comments. Same args as the summary script; reads token + repo + PR number from env. Finds existing comment by HTML marker (<!-- scanner-comment: <name> -->), thenPOST/PATCH/DELETEper the matrix below.Sticky semantics
POSTnew commentPATCHin placeDELETEDelete-on-clean (rather than update-to-"no longer any findings") keeps the invariant "if a scanner comment exists on the PR, the finding is current" — reviewers don't have to mentally diff comment state against run state. The green CI badge handles the all-clean signalling.
Each scanner has its own marker, so the four parallel jobs don't coordinate writes.
Failure mode
The new step runs
if: always() && github.event_name == 'pull_request'. A flagged gate (the case where reviewers most need the findings surfaced) doesn't skip the comment. A failure inside the comment step does NOT mask a red gate —security-summary.mjshas already run + failed the gating step beforepost-pr-comment.mjsgets called.Files
scripts/ci/security-findings.mjsscripts/ci/security-summary.mjsscripts/ci/post-pr-comment.mjs.forgejo/workflows/pr.ymlpermissions:block (issues: write,pull-requests: write).forgejo/workflows/secrets.ymlpull_requestbranch of the workflow) +permissions:blockdocs/ci.mddocs/adr/0016-sticky-scanner-pr-comments.mdTest plan
actionlint .forgejo/workflows/*.yml— clean.security-summary.mjsagainst synthetic gitleaks SARIF (empty + 1-leak), synthetic npm audit JSON, synthetic Trivy JSON. Output identical in shape to the existing behavior;qualifying=<n>last-line preserved.ghp_…for gitleaks); confirm the scanner posts a comment, then push a follow-up that fixes the finding and confirm the comment is deleted.Note: leftover untracked private key
Still in the repo root:
forgejo-signing-key(private) +.pub. Not staged here, butshred -uafter you've copied it to the Forgejo host. (Repeating from #91 since they survived two more PR cycles.)Reviewers approve PRs from the conversation view, where the only scanner signal today is the red/green job badge. The Markdown findings tables that security-summary.mjs writes are one click into the run details — useful but rarely opened. Each scanner now posts its qualifying findings as a sticky PR comment in addition to the existing $GITHUB_STEP_SUMMARY output. Same Markdown, visible without leaving the PR. Structure: - Extracted parse/render path into scripts/ci/security-findings.mjs so both summary + comment generators share it byte-for-byte. - scripts/ci/security-summary.mjs slims to the step-summary CLI it always was, now an import of the shared module. - scripts/ci/post-pr-comment.mjs is new — sticky upsert via the Forgejo `/issues/{n}/comments` API, marker-matched per scanner. Sticky semantics: - Findings exist + no prior comment → POST. - Findings exist + prior comment → PATCH (in place; no spam on force-push). - Findings gone + prior comment → DELETE (green CI badge already says all-clean). - Findings gone + no prior comment → no-op. Each scanner has its own marker (<!-- scanner-comment: <name> -->), so the four parallel jobs don't have to coordinate writes. The new step runs `if: always() && github.event_name == 'pull_request'` so a failed gate (which is exactly when reviewers need findings surfaced) doesn't skip it. A failure inside the post step itself does NOT mask a red gate — security-summary.mjs has already run and exited the gating step before post-pr-comment.mjs gets a chance. pr.yml + secrets.yml gain top-level `permissions:` blocks declaring `issues: write` + `pull-requests: write`. Documented in docs/ci.md "Sticky PR comments from security scanners". ADR-0016 captures the trade-offs (sticky vs append, per-scanner vs aggregated, delete-on-clean vs update-to-clean, conversation vs inline comments). Closes #68. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>adf4ad233f1d0d83bb7c