cleancoders · arootroatch · Jul 28, 2026 · Jul 28, 2026
diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml
@@ -351,41 +351,11 @@ jobs:
           fi
           semgrep "${args[@]}"
 
-          if [ ! -f semgrep.sarif ]; then
-            echo "::error::semgrep produced no SARIF; the scan did not complete"
-            exit 1
-          fi
-
-          # Severity lives on the rule, not the result: semgrep leaves
-          # results[].level null and puts it in tool.driver.rules[].
-          # defaultConfiguration.level. Join them so the printout shows severity
-          # and so the exit decision can honour it.
-          jq -r '.runs[] as $r
-                 | ($r.tool.driver.rules
-                    | map({key: .id, value: .defaultConfiguration.level})
-                    | from_entries) as $lv
-                 | $r.results[]
-                 | "  [\($lv[.ruleId] // "unknown")] \(.ruleId | split(".") | last)  "
-                   + "\(.locations[0].physicalLocation.artifactLocation.uri)"
-                   + ":\(.locations[0].physicalLocation.region.startLine // 0)"' \
-            semgrep.sarif
-
-          # Only error-level findings fail the job. cc-path-traversal and
-          # cc-generic-catch are severity: WARNING on purpose — without dataflow
-          # they cannot be precise enough to gate a build, and the README
-          # promises they do not block. Counting every result would break that.
-          blocking=$(jq '[.runs[] as $r
-                          | ($r.tool.driver.rules
-                             | map({key: .id, value: .defaultConfiguration.level})
-                             | from_entries) as $lv
-                          | $r.results[] | select($lv[.ruleId] == "error")] | length' \
-                     semgrep.sarif)
-          total=$(jq '[.runs[].results[]?] | length' semgrep.sarif)
-          echo "semgrep: ${total} finding(s), ${blocking} blocking"
-          if [ "$blocking" -gt 0 ]; then
-            echo "::error::semgrep found $blocking blocking finding(s); see above and the semgrep-sarif artifact"
-            exit 1
-          fi
+          # Rendering and the exit decision live in bin/report-sarif.sh so both
+          # are testable: bin/test-report-sarif.sh runs them against a SARIF
+          # captured from a real consumer scan. The critical case it pins is that
+          # warning-level rules do not block.
+          bash .cc-security-rules/bin/report-sarif.sh semgrep.sarif
       - name: Upload SARIF
         if: always()   # evidence of a FAILING scan is the evidence most worth keeping
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1 (node24)

diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml
@@ -58,6 +58,9 @@ jobs:
         # Replaces the old security-skips job, which duplicated all nine scanner
         # jobs to exercise two conditionals. 17 cases, milliseconds.
         run: bash bin/test-detect.sh
+      - name: Run SARIF reporter tests
+        # Pins the exit-code contract: warning-level rules must not block.
+        run: bash bin/test-report-sarif.sh
       - name: Run rule fixture tests
         run: bash bin/test-rules.sh
       - name: Check coverage table is current

diff --git a/bin/report-sarif.sh b/bin/report-sarif.sh
@@ -0,0 +1,112 @@
+#!/usr/bin/env bash
+# Renders a semgrep SARIF file as a CI-readable table and sets the exit code.
+#
+#   report-sarif.sh <file.sarif>
+#     exit 0  no error-level findings
+#     exit 1  at least one error-level finding
+#
+# Why a table: the previous one-line-per-finding format told you WHERE but not
+# WHAT CLASS, so acting on a finding meant looking the rule up by hand. Semgrep
+# already carries CWE, OWASP category and confidence in
+# tool.driver.rules[].properties.tags — this surfaces them.
+#
+# Severity lives on the rule, not the result: semgrep leaves results[].level null
+# and puts it in tool.driver.rules[].defaultConfiguration.level. Everything below
+# joins through that.
+#
+# Only error-level findings affect the exit code. cc-path-traversal,
+# cc-generic-catch and cc-clojure-xml-xxe are WARNING on purpose — without
+# dataflow they cannot be precise enough to gate a build, and the README promises
+# they do not block.
+#
+# Results carrying a `suppressions` array are excluded from the table and the
+# exit code. semgrep still emits a finding when source has a `nosemgrep`
+# annotation, marking it suppressions:[{kind:"inSource"}]. Counting those means
+# blocking a build on a finding a developer already triaged, and makes nosemgrep
+# look broken. They are reported as a count so the suppression stays visible
+# rather than silent.
+set -euo pipefail
+
+SARIF="${1:?usage: report-sarif.sh <file.sarif>}"
+
+command -v jq >/dev/null || { echo "jq not installed"; exit 1; }
+[ -f "$SARIF" ] || { echo "::error::$SARIF not found; the scan did not complete"; exit 1; }
+
+total="$(jq '[.runs[].results[]? | select(.suppressions == null or (.suppressions | length) == 0)] | length' "$SARIF")"
+suppressed="$(jq '[.runs[].results[]? | select(.suppressions != null and (.suppressions | length) > 0)] | length' "$SARIF")"
+if [ "$total" -eq 0 ]; then
+  echo "semgrep: no findings${suppressed:+ (${suppressed} suppressed in source)}"
+  exit 0
+fi
+
+# One TSV row per finding. Tag shapes semgrep emits:
+#   "CWE-79: Improper Neutralization of Input ... ('Cross-site Scripting')"
+#   "OWASP-A05:2025 - Injection"          (a rule may carry several editions)
+#   "HIGH CONFIDENCE"
+# CWE and OWASP are reduced to identifiers here to keep the table narrow; the
+# legend below prints the full names once each.
+rows="$(jq -r '
+  .runs[] as $r
+  | ($r.tool.driver.rules
+     | map({key: .id, value: .}) | from_entries) as $rules
+  | [$r.results[] | select(.suppressions == null or (.suppressions | length) == 0)]
+    | to_entries[]
+  | .key as $i | .value as $res
+  | ($rules[$res.ruleId] // {}) as $rule
+  | ($rule.properties.tags // []) as $tags
+  | ($rule.defaultConfiguration.level // "unknown") as $lvl
+  | ($tags | map(select(startswith("CWE-")) | split(":")[0]) | join(", ")) as $cwe
+  | ($tags | map(select(startswith("OWASP-")) | sub("^OWASP-";"") | split(" ")[0])
+           | join(", ")) as $owasp
+  | ($tags | map(select(endswith(" CONFIDENCE")) | sub(" CONFIDENCE";""))
+           | first // "-") as $conf
+  | [ ($i + 1 | tostring),
+      $lvl,
+      ($res.locations[0].physicalLocation.artifactLocation.uri
+       + ":" + ($res.locations[0].physicalLocation.region.startLine // 0 | tostring)),
+      ($res.ruleId | split(".") | last),
+      (if $cwe == "" then "-" else $cwe end),
+      (if $owasp == "" then "-" else $owasp end),
+      $conf ]
+  | @tsv' "$SARIF")"
+
+{
+  printf '#\tSEVERITY\tFILE:LINE\tRULE\tCWE\tOWASP\tCONFIDENCE\n'
+  printf '%s\n' "$rows"
+} | { command -v column >/dev/null && column -t -s "$(printf '\t')" || cat; }
+
+# Legend: expand each identifier once rather than repeating long names per row.
+echo
+echo "CWE:"
+jq -r '.runs[] as $r
+       | ([$r.results[] | select(.suppressions == null or (.suppressions | length) == 0)
+           | .ruleId] | unique) as $shown
+       | [$r.tool.driver.rules[] | select(.id as $i | $shown | index($i))
+          | .properties.tags[]? | select(startswith("CWE-"))]
+       | unique | .[] | "  " + .' "$SARIF"
+echo "OWASP:"
+jq -r '.runs[] as $r
+       | ([$r.results[] | select(.suppressions == null or (.suppressions | length) == 0)
+           | .ruleId] | unique) as $shown
+       | [$r.tool.driver.rules[] | select(.id as $i | $shown | index($i))
+          | .properties.tags[]? | select(startswith("OWASP-")) | sub("^OWASP-";"")]
+       | unique | .[] | "  " + .' "$SARIF"
+
+blocking="$(jq '[.runs[] as $r
+                 | ($r.tool.driver.rules
+                    | map({key: .id, value: .defaultConfiguration.level})
+                    | from_entries) as $lv
+                 | $r.results[]
+                 | select(.suppressions == null or (.suppressions | length) == 0)
+                 | select($lv[.ruleId] == "error")] | length' "$SARIF")"
+
+echo
+echo "semgrep: ${total} finding(s), ${blocking} blocking (error-level)"
+if [ "$suppressed" -gt 0 ]; then
+  echo "         ${suppressed} additional finding(s) suppressed in source (nosemgrep)"
+fi
+if [ "$blocking" -gt 0 ]; then
+  echo "::error::semgrep found ${blocking} blocking finding(s); see the table above and the semgrep-sarif artifact"
+  exit 1
+fi
+exit 0
diff --git a/bin/test-report-sarif.sh b/bin/test-report-sarif.sh
@@ -0,0 +1,70 @@
+#!/usr/bin/env bash
+# Tests bin/report-sarif.sh against a real SARIF captured from a consumer run.
+#
+# The exit code here decides whether a build passes, so the warnings-only case is
+# the one that matters: cc-path-traversal, cc-generic-catch and
+# cc-clojure-xml-xxe are WARNING on purpose and the README promises they do not
+# block. A regression that counted every result would quietly start failing
+# builds on low-precision findings.
+set -uo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+REPORT="${ROOT}/bin/report-sarif.sh"
+SAMPLE="${ROOT}/spec-fixtures/sarif/sample.sarif"
+pass=0; fail=0
+
+ok()    { pass=$((pass+1)); }
+bad()   { fail=$((fail+1)); echo "FAIL: $1"; }
+check() { if [ "$2" = "$3" ]; then ok; else bad "$1 — expected '$3', got '$2'"; fi; }
+
+WORK="$(mktemp -d)"
+trap 'rm -rf "${WORK}"' EXIT
+
+# --- the real sample: 7 results, 2 nosemgrep-suppressed, 1 error-level --------
+# The suppressed pair is the important part. cleancoders.com annotated both
+# secret findings with `nosemgrep`; semgrep honours that by setting
+# suppressions:[{kind:"inSource"}] but still emits the result. Counting them
+# blocks a build on findings a developer already accepted.
+out="$(bash "${REPORT}" "${SAMPLE}" 2>&1)"; rc=$?
+check "one error-level finding blocks"        "$rc" "1"
+check "excludes suppressed from the counts"   "$(echo "$out" | grep -c '5 finding(s), 1 blocking')" "1"
+check "reports the suppressed count"          "$(echo "$out" | grep -c '2 additional finding(s) suppressed')" "1"
+check "emits one row per unsuppressed finding" "$(echo "$out" | grep -cE '^[1-5] +(error|warning) ')" "5"
+check "suppressed findings stay out of table" "$(echo "$out" | grep -cE 'detected-jwt-token|detected-generic-secret')" "0"
+check "legend omits suppressed-only CWEs"     "$(echo "$out" | grep -cE 'CWE-321|CWE-798')" "0"
+# cc-generic-catch fires 4x, so its multi-CWE cell appears on all 4 rows.
+check "surfaces multi-CWE cells per row"      "$(echo "$out" | grep -c 'CWE-396, CWE-636')" "4"
+check "reports confidence"                    "$(echo "$out" | grep -cE 'cc-cljs-innerhtml.*HIGH')" "1"
+check "legend expands a CWE id once"          "$(echo "$out" | grep -c "CWE-636: Not Failing Securely")" "1"
+check "legend expands an OWASP category"      "$(echo "$out" | grep -c 'A10:2025 - Mishandling')" "1"
+check "annotates the failure for GitHub"      "$(echo "$out" | grep -c '::error::semgrep found 1')" "1"
+
+# --- warnings only: must NOT block -------------------------------------------
+jq '.runs |= map(
+      (.tool.driver.rules | map(select(.defaultConfiguration.level == "warning") | .id)) as $warn
+      | .results |= map(select(.ruleId as $i | $warn | index($i))))' \
+   "${SAMPLE}" > "${WORK}/warn.sarif"
+out="$(bash "${REPORT}" "${WORK}/warn.sarif" 2>&1)"; rc=$?
+check "warnings alone do not block"      "$rc" "0"
+check "warning count still reported"     "$(echo "$out" | grep -c '4 finding(s), 0 blocking')" "1"
+check "no error annotation on warnings"  "$(echo "$out" | grep -c '::error::')" "0"
+
+# --- suppressed-only: nothing actionable, must pass and say why ---------------
+jq '.runs |= map(.results |= map(select(.suppressions != null)))' "${SAMPLE}" > "${WORK}/supp.sarif"
+out="$(bash "${REPORT}" "${WORK}/supp.sarif" 2>&1)"; rc=$?
+check "suppressed-only passes"           "$rc" "0"
+check "suppressed-only is not silent"    "$(echo "$out" | grep -c '2 suppressed in source')" "1"
+
+# --- no findings --------------------------------------------------------------
+jq '.runs |= map(.results = [])' "${SAMPLE}" > "${WORK}/clean.sarif"
+out="$(bash "${REPORT}" "${WORK}/clean.sarif" 2>&1)"; rc=$?
+check "clean scan passes"        "$rc" "0"
+check "clean scan says so"       "$(echo "$out" | grep -c 'no findings')" "1"
+
+# --- missing file: a scan that never ran must fail loudly --------------------
+out="$(bash "${REPORT}" "${WORK}/absent.sarif" 2>&1)"; rc=$?
+check "missing SARIF fails"           "$rc" "1"
+check "missing SARIF explains why"    "$(echo "$out" | grep -c 'did not complete')" "1"
+
+echo "report-sarif tests: ${pass} passed, ${fail} failed"
+[ "${fail}" -eq 0 ]