Post CI security scanner findings as sticky PR comments #68

New issue

Closed

opened 2026-06-15 13:51:55 +00:00 by james · 0 comments

james commented

2026-06-15 13:51:55 +00:00

Owner

Today the security scanners in .forgejo/workflows/pr.yml (npm_audit, osv_scan, image_scan from ADR-0005, plus gitleaks from ADR-0011) all write their findings to $GITHUB_STEP_SUMMARY via scripts/ci/security-summary.mjs. Reviewers only see the findings if they click into the run details — they're not visible from the PR conversation itself.

A sticky PR comment per scanner would surface findings where reviewers actually look: in the PR. The step summary stays as the drill-down detail.

Scope

Extend scripts/ci/security-summary.mjs with a --post-pr-comment mode (or a sibling script that reuses its findings table generator). It takes a marker (e.g. ), the PR number, and a Forgejo API token from env vars.
Sticky upsert, not append. Find the existing comment via GET /api/v1/repos/{owner}/{repo}/issues/{index}/comments filtered by the marker; PATCH it if found, POST a new one if not. Avoids every push spamming a fresh comment.
Findings-only. If a scan reports zero findings at or above the configured threshold, do not post a comment. The green CI status check already conveys "all green." Optionally: if a previous run had findings and the current run is clean, DELETE the stale comment so reviewers don't see lingering warnings.
Keep the step summary as-is. It persists with the run forever and is what you click into for verbose detail. The PR comment is the dashboard; the step summary is the drill-down.
Each scanner posts its own comment with its own marker. Don't try to coordinate one mega-comment across the parallel jobs — coordination overhead for a marginal UX win.
Add the token to the workflow env (Forgejo's ${{ secrets.GITHUB_TOKEN }} or the equivalent shim) with the minimum scope needed to comment on issues / PRs.

Out of scope

Aggregating all scanners into a single comment. Future ticket if visual noise is a real problem.
Threading replies into existing comments / inline-PR-review-comment style. The sticky upsert is enough.
Refactoring scripts/ci/security-summary.mjs beyond what the new mode requires.

Acceptance criteria

A PR run that produces a finding at-or-above threshold from any scanner adds (or updates) a comment to the PR, including the Markdown table that's currently in the step summary.
A subsequent push that resolves the finding either updates the comment to reflect the new clean state or deletes the stale comment — pick one and document the choice in a comment in the script.
A PR run with all scanners clean from the start does NOT post any comment.
The step summary still gets the same Markdown it gets today.
A throwaway branch with a synthetic finding (a deliberately vulnerable dep, or a synthetic ghp_… PAT for gitleaks) end-to-end produces the comment on the PR.
The required token scope is documented in docs/ci.md so a self-hoster forking Carol's workflow knows what to grant.

Part of epic #2. References ADR-0005 (docs/adr/0005-ci-security-scanning.md) and ADR-0011 (docs/adr/0011-gitleaks-ci.md).

Today the security scanners in `.forgejo/workflows/pr.yml` (`npm_audit`, `osv_scan`, `image_scan` from ADR-0005, plus `gitleaks` from ADR-0011) all write their findings to `$GITHUB_STEP_SUMMARY` via `scripts/ci/security-summary.mjs`. Reviewers only see the findings if they click into the run details — they're not visible from the PR conversation itself. A sticky PR comment per scanner would surface findings where reviewers actually look: in the PR. The step summary stays as the drill-down detail. ## Scope - Extend `scripts/ci/security-summary.mjs` with a `--post-pr-comment` mode (or a sibling script that reuses its findings table generator). It takes a marker (e.g. ``), the PR number, and a Forgejo API token from env vars. - **Sticky upsert**, not append. Find the existing comment via `GET /api/v1/repos/{owner}/{repo}/issues/{index}/comments` filtered by the marker; `PATCH` it if found, `POST` a new one if not. Avoids every push spamming a fresh comment. - **Findings-only**. If a scan reports zero findings at or above the configured threshold, do not post a comment. The green CI status check already conveys "all green." Optionally: if a previous run had findings and the current run is clean, `DELETE` the stale comment so reviewers don't see lingering warnings. - **Keep the step summary as-is.** It persists with the run forever and is what you click into for verbose detail. The PR comment is the dashboard; the step summary is the drill-down. - Each scanner posts its own comment with its own marker. Don't try to coordinate one mega-comment across the parallel jobs — coordination overhead for a marginal UX win. - Add the token to the workflow env (Forgejo's `${{ secrets.GITHUB_TOKEN }}` or the equivalent shim) with the minimum scope needed to comment on issues / PRs. ## Out of scope - Aggregating all scanners into a single comment. Future ticket if visual noise is a real problem. - Threading replies into existing comments / inline-PR-review-comment style. The sticky upsert is enough. - Refactoring `scripts/ci/security-summary.mjs` beyond what the new mode requires. ## Acceptance criteria - [ ] A PR run that produces a finding at-or-above threshold from any scanner adds (or updates) a comment to the PR, including the Markdown table that's currently in the step summary. - [ ] A subsequent push that resolves the finding either updates the comment to reflect the new clean state or deletes the stale comment — pick one and document the choice in a comment in the script. - [ ] A PR run with all scanners clean from the start does NOT post any comment. - [ ] The step summary still gets the same Markdown it gets today. - [ ] A throwaway branch with a synthetic finding (a deliberately vulnerable dep, or a synthetic `ghp_…` PAT for gitleaks) end-to-end produces the comment on the PR. - [ ] The required token scope is documented in `docs/ci.md` so a self-hoster forking Carol's workflow knows what to grant. Part of epic #2. References ADR-0005 (`docs/adr/0005-ci-security-scanning.md`) and ADR-0011 (`docs/adr/0011-gitleaks-ci.md`).

james added the

area:ci

label

2026-06-15 13:51:59 +00:00

james referenced this issue from a commit

2026-06-18 02:13:10 +00:00

ci(security): post scanner findings as sticky PR comments (#68)

james referenced this issue from a pull request that will close it,

2026-06-18 02:14:16 +00:00

ci(security): post scanner findings as sticky PR comments (#68) #95

james referenced this issue from a commit

2026-06-18 02:23:03 +00:00