fix(report): sort sarif driver rules and strip crlf from markdown headers (#337)

* fix(report): sort sarif driver rules for deterministic output

driver.rules was built by ranging a go map, so the emitted order
varied between runs over the same input, producing byte-unstable
sarif for identical scans. collect the rule ids into a slice and
sort them before building the rules list; ruleId still references
by id so no result is affected.

* fix(report): strip crlf from markdown target headers

target and module id are operator-supplied and were written verbatim
into "## " / "### " heading lines, so a target containing an
embedded newline followed by "## " text could inject a fake
standalone heading into the markdown report. sanitizeHeading
collapses cr/lf in both before they're written; the finding data
block itself was already unaffected.
This commit is contained in:
Tigah
2026-07-22 12:53:56 -07:00
committed by GitHub
parent 788d20c18e
commit d8ca2e96b1
3 changed files with 85 additions and 4 deletions
+13 -2
View File
@@ -40,7 +40,7 @@ func Markdown(results []Result) []byte {
for i := 0; i < len(order); i++ {
target := order[i]
b.WriteString("## ")
b.WriteString(target)
b.WriteString(sanitizeHeading(target))
b.WriteString("\n\n")
mods := byTarget[target]
@@ -49,7 +49,7 @@ func Markdown(results []Result) []byte {
for j := 0; j < len(mods); j++ {
b.WriteString("### ")
b.WriteString(mods[j].Module)
b.WriteString(sanitizeHeading(mods[j].Module))
b.WriteString("\n\n")
b.WriteString("```json\n")
b.WriteString(prettyJSON(mods[j].Data))
@@ -60,6 +60,17 @@ func Markdown(results []Result) []byte {
return []byte(b.String())
}
// sanitizeHeading strips CR/LF from operator-supplied text (targets, module
// ids) before it's written into a markdown heading line. both target and
// module id can come from scan input, so an embedded newline followed by
// "## ..." would otherwise render as a standalone, injected heading.
func sanitizeHeading(s string) string {
s = strings.ReplaceAll(s, "\r\n", " ")
s = strings.ReplaceAll(s, "\r", " ")
s = strings.ReplaceAll(s, "\n", " ")
return s
}
// prettyJSON re-indents the raw finding for readability; if it doesn't parse as
// json (shouldn't happen, but never trust it) the raw bytes are returned as-is.
func prettyJSON(raw json.RawMessage) string {
+60
View File
@@ -14,6 +14,8 @@ package report
import (
"encoding/json"
"reflect"
"sort"
"strings"
"testing"
)
@@ -75,6 +77,46 @@ func TestSARIF_ValidAndContainsFindings(t *testing.T) {
}
}
func TestSARIF_RulesAreSorted(t *testing.T) {
// rules are collected from a map internally, so without an explicit sort
// the emitted order is whatever the map iteration happened to produce.
// several distinct module ids make an accidentally-sorted iteration
// vanishingly unlikely, so this pins the fix rather than relying on luck.
results := []Result{
{Target: "https://t", Module: "zeta", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "mike", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "alpha", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "yankee", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "bravo", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "delta", Data: json.RawMessage(`{}`)},
{Target: "https://t", Module: "kilo", Data: json.RawMessage(`{}`)},
}
var firstIDs []string
for run := 0; run < 5; run++ {
out, err := SARIF(results)
if err != nil {
t.Fatalf("SARIF: %v", err)
}
var doc sarifLog
if err := json.Unmarshal(out, &doc); err != nil {
t.Fatalf("invalid json: %v", err)
}
ids := make([]string, len(doc.Runs[0].Tool.Driver.Rules))
for i, r := range doc.Runs[0].Tool.Driver.Rules {
ids[i] = r.ID
}
if !sort.StringsAreSorted(ids) {
t.Fatalf("run %d: driver.rules not sorted: %v", run, ids)
}
if run == 0 {
firstIDs = ids
} else if !reflect.DeepEqual(firstIDs, ids) {
t.Fatalf("driver.rules order changed across runs: %v vs %v", firstIDs, ids)
}
}
}
func TestSARIF_DedupesRulesAcrossTargets(t *testing.T) {
// the same module on two targets must yield one rule but two results.
results := []Result{
@@ -193,6 +235,24 @@ func TestMarkdown_GroupsByTarget(t *testing.T) {
}
}
func TestMarkdown_StripsNewlinesFromTargetHeader(t *testing.T) {
// same newline-injection guard as sanitizeHeading (markdown.go); this exercises the malicious-input case.
results := []Result{
{Target: "https://evil.example.com\n## injected", Module: "probe", Data: json.RawMessage(`{"status_code":200}`)},
}
out := string(Markdown(results))
lines := strings.Split(out, "\n")
for _, line := range lines {
if line == "## injected" {
t.Errorf("target newline produced a standalone injected heading:\n%s", out)
}
}
if strings.Contains(out, "\r") {
t.Errorf("markdown output must not contain carriage returns:\n%q", out)
}
}
// sarifHasResult reports whether any result carries the given rule id and target
// uri, the pairing that proves a finding survived serialization.
func sarifHasResult(results []sarifResult, ruleID, target string) bool {
+12 -2
View File
@@ -15,6 +15,7 @@ package report
import (
"encoding/json"
"fmt"
"sort"
)
// sarif format/version constants pinned to the 2.1.0 schema so the output is
@@ -121,9 +122,18 @@ func SARIF(results []Result) ([]byte, error) {
}
// rules must list each id exactly once; build it from the set so duplicate
// modules across targets don't duplicate the rule.
rules := make([]sarifRule, 0, len(ruleSet))
// modules across targets don't duplicate the rule. the set itself ranges
// in random map order, so sort the ids first: otherwise driver.rules
// would come out in a different order on every run, making the sarif
// output byte-unstable across identical scans.
ruleIDs := make([]string, 0, len(ruleSet))
for id := range ruleSet {
ruleIDs = append(ruleIDs, id)
}
sort.Strings(ruleIDs)
rules := make([]sarifRule, 0, len(ruleIDs))
for _, id := range ruleIDs {
rules = append(rules, sarifRule{ID: id})
}