diff --git a/.github/workflows/scan.yml b/.github/workflows/scan.yml index 8a86d9b..4d1cf1a 100644 --- a/.github/workflows/scan.yml +++ b/.github/workflows/scan.yml @@ -22,6 +22,7 @@ name: Cerberus permissions: contents: read packages: read + pull-requests: write # the scan report is posted on the pull request on: workflow_call: @@ -69,6 +70,7 @@ jobs: permissions: contents: read packages: read + pull-requests: write steps: - uses: actions/checkout@v4 with: diff --git a/README.md b/README.md index 67ec5b5..f568f29 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ uploads nothing unless you pass `--upload`. Exit codes: 0 clean, 1 gate failed, - **Stateless.** Cerberus scans everything and sends everything. History, dedup, baselines and suppression live in the backend. - **Delta gate.** Pipelines fail only on findings introduced by the change. A pre-existing backlog never blocks a merge. +- **Says what it found.** Every run writes a report to the job summary, and on a pull request posts it as a comment (replacing its own previous one): each new finding with severity, file:line and a link to the task it created. - **No second UI.** Triage happens in your tracker: close a task as *declined* and the finding is suppressed forever; close it as *done* and the next scan verifies the fix. - **Bring your own backend.** The upload contract is a documented JSON envelope around SARIF; any backend implementing it works. @@ -112,7 +113,7 @@ uploads nothing unless you pass `--upload`. Exit codes: 0 clean, 1 gate failed, - [x] `check` mode for merge requests (classify against baseline, no writes) - [x] GitLab CI template and GitHub composite action - [x] Image published to ghcr.io on every push to main -- [ ] MR/PR comments with the delta table +- [x] Pull-request comment + job summary with the delta table and links to the tasks - [ ] More heads: Hadolint, Checkov, license audit, Zizmor Scanner rules (e.g. the Semgrep registry) are fetched at runtime and are licensed by their respective owners; Cerberus does not bundle them. diff --git a/action.yml b/action.yml index 6be4b50..bf1f010 100644 --- a/action.yml +++ b/action.yml @@ -70,6 +70,9 @@ runs: env: K_SARIF_URL: ${{ inputs.url }} K_SARIF_SECRET: ${{ inputs.secret }} + # Used to post the scan report on the pull request. Read-only without + # `pull-requests: write`, in which case the report stays in the summary. + GITHUB_TOKEN: ${{ inputs.registry-token || github.token }} run: | dns_args="" if [ -n "${{ inputs.dns }}" ]; then @@ -78,8 +81,11 @@ runs: fi docker run --rm $dns_args \ -v "${{ github.workspace }}:/src" \ + -v "${GITHUB_STEP_SUMMARY}:/tmp/summary.md" \ -w "/src/${{ inputs.path }}" \ -e K_SARIF_URL -e K_SARIF_SECRET \ + -e GITHUB_TOKEN -e GITHUB_API_URL -e GITHUB_REF \ + -e GITHUB_STEP_SUMMARY=/tmp/summary.md \ -e GITHUB_ACTIONS -e GITHUB_REPOSITORY -e GITHUB_REF_NAME -e GITHUB_HEAD_REF \ -e GITHUB_SHA -e GITHUB_ACTOR -e GITHUB_DEFAULT_BRANCH="${{ github.event.repository.default_branch }}" \ "${{ inputs.image }}" scan --mode "${{ inputs.mode }}" diff --git a/src/cerberus.test.ts b/src/cerberus.test.ts index 4f70ae6..1c1bbe2 100644 --- a/src/cerberus.test.ts +++ b/src/cerberus.test.ts @@ -6,6 +6,8 @@ import { evaluateGate } from "./gate.js"; import { buildEnvelope, targetFromEnv } from "./upload.js"; import { buildInvocations } from "./scanners.js"; import { flattenFindings } from "./table.js"; +import { buildReport, COMMENT_MARKER } from "./report.js"; +import { prTargetFromEnv } from "./publish.js"; import type { UploadResponse } from "./types.js"; describe("parseConfig", () => { @@ -220,3 +222,64 @@ describe("flattenFindings", () => { expect(findings[1]!.severity).toBe("info"); }); }); + +describe("buildReport", () => { + const ctx = { provider: "github" as const, repo: "web", branch: "f", defaultBranch: "main", changedFiles: [] }; + const base = (over: Partial): UploadResponse => ({ + ok: true, + summary: { total: 10, new: 0, known: 10, suppressed: 0, fixed: 0, reopened: 0, tasksCreated: 0 }, + new: [], + ...over, + }); + + it("frames a baseline as recorded, not as work", () => { + const md = buildReport(ctx, base({ baseline: true }), { failed: false }); + expect(md).toContain(COMMENT_MARKER); + expect(md).toContain("baseline"); + expect(md).not.toContain("Gate failed"); + }); + + it("tables the new findings with location and a link to the task", () => { + const response = base({ + summary: { total: 12, new: 2, known: 10, suppressed: 0, fixed: 1, reopened: 0, tasksCreated: 1 }, + new: [ + { title: "SQL injection", severity: "critical", file: "src/db.ts", line: 42, taskId: "t1", taskKey: "SID/T/12", taskUrl: "https://k.example/SMK/SID/T/12" }, + { title: "Weak hash", severity: "low", file: null, line: null, taskId: null }, + ], + }); + const md = buildReport(ctx, response, { failed: true, reason: "1 new critical finding(s)" }); + + expect(md).toContain("Gate failed"); + expect(md).toContain("[SID/T/12](https://k.example/SMK/SID/T/12)"); + expect(md).toContain("`src/db.ts:42`"); + // Most severe first. + expect(md.indexOf("SQL injection")).toBeLessThan(md.indexOf("Weak hash")); + }); + + it("says so when nothing is new", () => { + expect(buildReport(ctx, base({}), { failed: false })).toContain("No new findings"); + }); + + it("escapes a pipe so one finding cannot break the table", () => { + const response = base({ + summary: { total: 1, new: 1, known: 0, suppressed: 0, fixed: 0, reopened: 0, tasksCreated: 0 }, + new: [{ title: "a | b", severity: "high", file: null, line: null, taskId: null }], + }); + expect(buildReport(ctx, response, { failed: false })).toContain("a \\| b"); + }); +}); + +describe("prTargetFromEnv", () => { + it("recognises a pull request run", () => { + const t = prTargetFromEnv({ + GITHUB_ACTIONS: "true", GITHUB_TOKEN: "x", GITHUB_REPOSITORY: "org/repo", GITHUB_REF: "refs/pull/42/merge", + }); + expect(t).toMatchObject({ repo: "org/repo", prNumber: 42, api: "https://api.github.com" }); + }); + + it("is null off a pull request, or without a token", () => { + expect(prTargetFromEnv({ GITHUB_ACTIONS: "true", GITHUB_TOKEN: "x", GITHUB_REPOSITORY: "o/r", GITHUB_REF: "refs/heads/main" })).toBeNull(); + expect(prTargetFromEnv({ GITHUB_ACTIONS: "true", GITHUB_REPOSITORY: "o/r", GITHUB_REF: "refs/pull/1/merge" })).toBeNull(); + expect(prTargetFromEnv({})).toBeNull(); + }); +}); diff --git a/src/gate.ts b/src/gate.ts index a32fde6..77e24b2 100644 --- a/src/gate.ts +++ b/src/gate.ts @@ -1,11 +1,8 @@ /** The merge gate: fail the pipeline only on NEW findings, by policy. */ -import type { GatePolicy, UploadResponse } from "./types.js"; +import type { GatePolicy, GateResult, UploadResponse } from "./types.js"; -export interface GateResult { - failed: boolean; - reason?: string; -} +export type { GateResult }; const FAIL_SEVERITIES: Record, Set> = { "new-critical": new Set(["critical"]), diff --git a/src/index.ts b/src/index.ts index 03efe40..22d9764 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,6 +16,8 @@ import { mergeSarif } from "./merge.js"; import { targetFromEnv, buildEnvelope, upload } from "./upload.js"; import { evaluateGate } from "./gate.js"; import { flattenFindings, renderTable } from "./table.js"; +import { buildReport } from "./report.js"; +import { writeJobSummary, prTargetFromEnv, upsertPrComment } from "./publish.js"; const HELP = `cerberus — security scan orchestrator and merge gate @@ -123,7 +125,9 @@ async function main() { (s.tasksCreated ? ` · ${s.tasksCreated} task(s) created` : ""), ); for (const f of response.new ?? []) { - console.error(` NEW [${f.severity}] ${f.title}${f.file ? ` (${f.file}${f.line != null ? `:${f.line}` : ""})` : ""}${f.taskId ? ` → task ${f.taskId}` : ""}`); + const where = f.file ? ` (${f.file}${f.line != null ? `:${f.line}` : ""})` : ""; + const task = f.taskUrl ? ` → ${f.taskKey ?? "task"} ${f.taskUrl}` : f.taskId ? ` → task ${f.taskId}` : ""; + console.error(` NEW [${f.severity}] ${f.title}${where}${task}`); } // Findings were accepted but never became work — the run would otherwise look // clean while nothing lands in anyone's queue. @@ -132,6 +136,21 @@ async function main() { } const gate = evaluateGate(config.gate.failOn, response); + + // Publish before exiting: a failed gate is exactly when the reader needs the + // detail, and process.exit() below would skip anything after it. + const report = buildReport(ctx, response, gate); + if (writeJobSummary(report)) console.error("cerberus: report written to the job summary"); + const pr = prTargetFromEnv(); + if (pr) { + const outcome = await upsertPrComment(pr, report); + console.error( + outcome === "failed" + ? "cerberus: could not comment on the pull request (needs pull-requests: write)" + : `cerberus: report ${outcome} on PR #${pr.prNumber}`, + ); + } + if (gate.failed) { console.error(`cerberus: GATE FAILED (${config.gate.failOn}): ${gate.reason}`); process.exit(1); diff --git a/src/publish.ts b/src/publish.ts new file mode 100644 index 0000000..95732d3 --- /dev/null +++ b/src/publish.ts @@ -0,0 +1,84 @@ +/** Where the report goes: the job summary (always) and the pull request + * (when we are running on one and have a token). */ + +import { appendFileSync } from "node:fs"; +import { COMMENT_MARKER } from "./report.js"; + +type Env = Record; + +/** GitHub renders this file on the run's summary page. Free, no token. */ +export function writeJobSummary(markdown: string, env: Env = process.env): boolean { + const path = env.GITHUB_STEP_SUMMARY; + if (!path) return false; + try { + appendFileSync(path, `${markdown}\n`); + return true; + } catch { + return false; + } +} + +interface PrTarget { + api: string; + repo: string; + prNumber: number; + token: string; +} + +/** The pull request this run belongs to, if any. Needs a token with + * `pull-requests: write` — absent on forks, which is fine: the summary still + * carries the report. */ +export function prTargetFromEnv(env: Env = process.env): PrTarget | null { + if (env.GITHUB_ACTIONS !== "true") return null; + const token = env.GITHUB_TOKEN?.trim(); + const repo = env.GITHUB_REPOSITORY?.trim(); + const ref = env.GITHUB_REF ?? ""; // refs/pull/123/merge + const match = /^refs\/pull\/(\d+)\//.exec(ref); + if (!token || !repo || !match) return null; + return { + api: env.GITHUB_API_URL?.trim() || "https://api.github.com", + repo, + prNumber: Number(match[1]), + token, + }; +} + +async function gh(target: PrTarget, path: string, init?: RequestInit): Promise { + return fetch(`${target.api}${path}`, { + ...init, + headers: { + Authorization: `Bearer ${target.token}`, + Accept: "application/vnd.github+json", + "Content-Type": "application/json", + ...(init?.headers ?? {}), + }, + }); +} + +/** + * Post the report on the pull request, replacing our previous one — a scan runs + * on every push, and a thread of stale reports is worse than none. + */ +export async function upsertPrComment(target: PrTarget, markdown: string): Promise<"created" | "updated" | "failed"> { + try { + const listRes = await gh(target, `/repos/${target.repo}/issues/${target.prNumber}/comments?per_page=100`); + if (!listRes.ok) return "failed"; + const comments = (await listRes.json()) as Array<{ id: number; body?: string }>; + const mine = comments.find((c) => c.body?.includes(COMMENT_MARKER)); + + const res = mine + ? await gh(target, `/repos/${target.repo}/issues/comments/${mine.id}`, { + method: "PATCH", + body: JSON.stringify({ body: markdown }), + }) + : await gh(target, `/repos/${target.repo}/issues/${target.prNumber}/comments`, { + method: "POST", + body: JSON.stringify({ body: markdown }), + }); + + if (!res.ok) return "failed"; + return mine ? "updated" : "created"; + } catch { + return "failed"; + } +} diff --git a/src/report.ts b/src/report.ts new file mode 100644 index 0000000..5e56a29 --- /dev/null +++ b/src/report.ts @@ -0,0 +1,80 @@ +/** Human-facing scan report: a markdown summary for the CI job page and the + * pull request, so a reader sees what changed without opening the raw log. */ + +import type { CiContext, GateResult, UploadResponse } from "./types.js"; + +const SEVERITY_ORDER = ["critical", "high", "medium", "low", "info"]; +const SEVERITY_MARK: Record = { + critical: "🔴", high: "🟠", medium: "🟡", low: "🔵", info: "⚪", +}; + +/** Marker so a re-run replaces its own comment instead of stacking a new one. */ +export const COMMENT_MARKER = ""; + +function location(f: { file: string | null; line: number | null }): string { + if (!f.file) return "—"; + return f.line != null ? `\`${f.file}:${f.line}\`` : `\`${f.file}\``; +} + +function taskCell(f: { taskKey?: string | null; taskUrl?: string | null }): string { + if (f.taskUrl && f.taskKey) return `[${f.taskKey}](${f.taskUrl})`; + if (f.taskKey) return f.taskKey; + return "—"; +} + +/** + * The report. `mode` decides the framing: a check gates a change (these are the + * findings *you* are adding), a report describes the default branch. + */ +export function buildReport(ctx: CiContext, response: UploadResponse, gate: GateResult): string { + const s = response.summary; + if (!s) return `${COMMENT_MARKER}\n### Cerberus\n\nNo scan summary returned.`; + + const lines: string[] = [COMMENT_MARKER, "### Cerberus — security scan"]; + + if (response.baseline) { + lines.push( + "", + `Recorded **${s.total}** findings as the baseline for \`${ctx.repo}\`. No tasks were created — from now on only *new* findings become work and can fail this pipeline.`, + ); + return lines.join("\n"); + } + + const findings = [...(response.new ?? [])].sort( + (a, b) => SEVERITY_ORDER.indexOf(a.severity) - SEVERITY_ORDER.indexOf(b.severity), + ); + + lines.push( + "", + gate.failed + ? `❌ **Gate failed** — ${gate.reason}` + : findings.length + ? `⚠️ **${findings.length} new finding(s)** — below the gate threshold, so this pipeline passes.` + : "✅ **No new findings.**", + "", + `\`${s.new}\` new · \`${s.known}\` already known · \`${s.fixed}\` fixed · \`${s.reopened}\` reopened · \`${s.suppressed}\` suppressed`, + ); + + if (findings.length) { + lines.push( + "", + "| | Severity | Finding | Location | Task |", + "|---|---|---|---|---|", + ...findings.map((f) => { + const title = f.title.replace(/\|/g, "\\|").slice(0, 120); + return `| ${SEVERITY_MARK[f.severity] ?? ""} | ${f.severity} | ${title} | ${location(f)} | ${taskCell(f)} |`; + }), + ); + if (s.new > findings.length) lines.push("", `…and ${s.new - findings.length} more.`); + } + + if (s.taskFailures) { + lines.push("", `⚠️ ${s.taskFailures} finding(s) could not be turned into tasks: ${response.taskError ?? "unknown error"}`); + } + + lines.push( + "", + "Findings already on the default branch never fail this check. Close a task as *declined* to suppress its finding for good.", + ); + return lines.join("\n"); +} diff --git a/src/types.ts b/src/types.ts index 06b00e5..07976f8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -75,6 +75,9 @@ export interface UploadResponse { file: string | null; line: number | null; taskId: string | null; + /** Human-facing task id (e.g. SID/T/12) and a link straight to it. */ + taskKey?: string | null; + taskUrl?: string | null; }>; } @@ -86,3 +89,8 @@ export interface ScannerRun { error?: string; durationMs: number; } + +export interface GateResult { + failed: boolean; + reason?: string; +}