Fix rule index reference into sarif report (#934)

Author: ccojocar

report/sarif/formatter.go

@@ -12,20 +12,13 @@ import (
 	"github.com/securego/gosec/v2/issue"
 )
 
-// GenerateReport Convert a gosec report to a Sarif Report
+// GenerateReport converts a gosec report into a SARIF report
 func GenerateReport(rootPaths []string, data *gosec.ReportInfo) (*Report, error) {
-	type rule struct {
-		index int
-		rule  *ReportingDescriptor
-	}
-
-	rules := make([]*ReportingDescriptor, 0)
-	rulesIndices := make(map[string]rule)
-	lastRuleIndex := -1
+	rules := []*ReportingDescriptor{}
 
 	results := []*Result{}
-	cweTaxa := make([]*ReportingDescriptor, 0)
-	weaknesses := make(map[string]*cwe.Weakness)
+	cweTaxa := []*ReportingDescriptor{}
+	weaknesses := map[string]*cwe.Weakness{}
 
 	for _, issue := range data.Issues {
 		if issue.Cwe != nil {
@@ -38,26 +31,26 @@ func GenerateReport(rootPaths []string, data *gosec.ReportInfo) (*Report, error)
 			}
 		}
 
-		r, ok := rulesIndices[issue.RuleID]
-		if !ok {
-			lastRuleIndex++
-			r = rule{index: lastRuleIndex, rule: parseSarifRule(issue)}
-			rulesIndices[issue.RuleID] = r
-			rules = append(rules, r.rule)
-		}
+		rule := parseSarifRule(issue)
+		ruleIndex := 0
+		rules, ruleIndex = addRuleInOrder(rules, rule)
 
 		location, err := parseSarifLocation(issue, rootPaths)
 		if err != nil {
 			return nil, err
 		}
 
-		result := NewResult(r.rule.ID, r.index, getSarifLevel(issue.Severity.String()), issue.What, buildSarifSuppressions(issue.Suppressions)).
-			WithLocations(location)
+		result := NewResult(
+			issue.RuleID,
+			ruleIndex,
+			getSarifLevel(issue.Severity.String()),
+			issue.What,
+			buildSarifSuppressions(issue.Suppressions),
+		).WithLocations(location)
 
 		results = append(results, result)
 	}
 
-	sort.SliceStable(rules, func(i, j int) bool { return rules[i].ID < rules[j].ID })
 	sort.SliceStable(cweTaxa, func(i, j int) bool { return cweTaxa[i].ID < cweTaxa[j].ID })
 
 	tool := NewTool(buildSarifDriver(rules, data.GosecVersion))
@@ -72,6 +65,26 @@ func GenerateReport(rootPaths []string, data *gosec.ReportInfo) (*Report, error)
 		WithRuns(run), nil
 }
 
+// addRuleInOrder inserts a rule into the rules slice keeping the rules IDs order, it returns the new rules
+// slice and the position where the rule was inserted
+func addRuleInOrder(rules []*ReportingDescriptor, rule *ReportingDescriptor) ([]*ReportingDescriptor, int) {
+	position := 0
+	for i, r := range rules {
+		if r.ID < rule.ID {
+			continue
+		}
+		if r.ID == rule.ID {
+			return rules, i
+		}
+		position = i
+		break
+	}
+	rules = append(rules, nil)
+	copy(rules[position+1:], rules[position:])
+	rules[position] = rule
+	return rules, position
+}
+
 // parseSarifRule return SARIF rule field struct
 func parseSarifRule(i *issue.Issue) *ReportingDescriptor {
 	cwe := issue.GetCweByRule(i.RuleID)

report/sarif/sarif_test.go

@@ -111,5 +111,65 @@ var _ = Describe("Sarif Formatter", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(sarifReport.Runs[0].Results[0].Locations[0].PhysicalLocation.Region.Snippet.Text).Should(Equal(expectedCode))
 		})
+		It("sarif formatted report should have proper rule index", func() {
+			rules := []string{"G404", "G101", "G102", "G103"}
+			issues := []*issue.Issue{}
+			for _, rule := range rules {
+				cwe := issue.GetCweByRule(rule)
+				newissue := issue.Issue{
+					File:       "/home/src/project/test.go",
+					Line:       "69-70",
+					Col:        "14",
+					RuleID:     rule,
+					What:       "test",
+					Confidence: issue.High,
+					Severity:   issue.High,
+					Cwe:        cwe,
+					Suppressions: []issue.SuppressionInfo{
+						{
+							Kind:          "kind",
+							Justification: "justification",
+						},
+					},
+				}
+				issues = append(issues, &newissue)
+
+			}
+			dupRules := []string{"G102", "G404"}
+			for _, rule := range dupRules {
+				cwe := issue.GetCweByRule(rule)
+				newissue := issue.Issue{
+					File:       "/home/src/project/test.go",
+					Line:       "69-70",
+					Col:        "14",
+					RuleID:     rule,
+					What:       "test",
+					Confidence: issue.High,
+					Severity:   issue.High,
+					Cwe:        cwe,
+					Suppressions: []issue.SuppressionInfo{
+						{
+							Kind:          "kind",
+							Justification: "justification",
+						},
+					},
+				}
+				issues = append(issues, &newissue)
+			}
+			reportInfo := gosec.NewReportInfo(issues, &gosec.Metrics{}, map[string][]gosec.Error{}).WithVersion("v2.7.0")
+
+			sarifReport, err := sarif.GenerateReport([]string{}, reportInfo)
+
+			Expect(err).ShouldNot(HaveOccurred())
+			resultRuleIdexes := map[string]int{}
+			for _, result := range sarifReport.Runs[0].Results {
+				resultRuleIdexes[result.RuleID] = result.RuleIndex
+			}
+			driverRuleIndexes := map[string]int{}
+			for ruleIndex, rule := range sarifReport.Runs[0].Tool.Driver.Rules {
+				driverRuleIndexes[rule.ID] = ruleIndex
+			}
+			Expect(resultRuleIdexes).Should(Equal(driverRuleIndexes))
+		})
 	})
 })