From 0ce48a584fefdbda343082381fff4f3066d6a55f Mon Sep 17 00:00:00 2001
From: xpivarc <41989919+xpivarc@users.noreply.github.com>
Date: Tue, 29 Sep 2020 19:17:38 +0200
Subject: [PATCH] Reproducible junit report (#529)

* Fix junit format ordering

Signed-off-by: L. Pivarc <lpivarc@redhat.com>

* Make ordering stable

Signed-off-by: L. Pivarc <lpivarc@redhat.com>

* Test ordering

Signed-off-by: L. Pivarc <lpivarc@redhat.com>
---
 cmd/gosec/sort_issues.go      | 22 +++++++++-
 cmd/gosec/sort_issues_test.go | 80 +++++++++++++++++++++++++++++++++++
 go.mod                        |  1 -
 go.sum                        |  6 +++
 output/formatter.go           |  4 +-
 output/formatter_test.go      | 24 +++++++++++
 output/junit_xml_format.go    | 47 +++++++++-----------
 7 files changed, 153 insertions(+), 31 deletions(-)
 create mode 100644 cmd/gosec/sort_issues_test.go

diff --git a/cmd/gosec/sort_issues.go b/cmd/gosec/sort_issues.go
index 35bff60630..ef27425b93 100644
--- a/cmd/gosec/sort_issues.go
+++ b/cmd/gosec/sort_issues.go
@@ -2,15 +2,35 @@ package main
 
 import (
 	"sort"
+	"strconv"
+	"strings"
 
 	"github.com/securego/gosec/v2"
 )
 
+// handle ranges
+func extractLineNumber(s string) int {
+	lineNumber, _ := strconv.Atoi(strings.Split(s, "-")[0])
+	return lineNumber
+
+}
+
 type sortBySeverity []*gosec.Issue
 
 func (s sortBySeverity) Len() int { return len(s) }
 
-func (s sortBySeverity) Less(i, j int) bool { return s[i].Severity > s[j].Severity }
+func (s sortBySeverity) Less(i, j int) bool {
+	if s[i].Severity == s[j].Severity {
+		if s[i].What == s[j].What {
+			if s[i].File == s[j].File {
+				return extractLineNumber(s[i].Line) > extractLineNumber(s[j].Line)
+			}
+			return s[i].File > s[j].File
+		}
+		return s[i].What > s[j].What
+	}
+	return s[i].Severity > s[j].Severity
+}
 
 func (s sortBySeverity) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 
diff --git a/cmd/gosec/sort_issues_test.go b/cmd/gosec/sort_issues_test.go
new file mode 100644
index 0000000000..2b06689743
--- /dev/null
+++ b/cmd/gosec/sort_issues_test.go
@@ -0,0 +1,80 @@
+package main
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/securego/gosec/v2"
+)
+
+var defaultIssue = gosec.Issue{
+	File:       "/home/src/project/test.go",
+	Line:       "1",
+	Col:        "1",
+	RuleID:     "ruleID",
+	What:       "test",
+	Confidence: gosec.High,
+	Severity:   gosec.High,
+	Code:       "1: testcode",
+	Cwe:        gosec.GetCwe("G101"),
+}
+
+func createIssue() gosec.Issue {
+	return defaultIssue
+}
+
+func TestRules(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Sort issues Suite")
+}
+
+func firstIsGreater(less, greater *gosec.Issue) {
+	slice := []*gosec.Issue{less, greater}
+
+	sortIssues(slice)
+
+	ExpectWithOffset(0, slice[0]).To(Equal(greater))
+}
+
+var _ = Describe("Sorting by Severity", func() {
+	It("sortes by severity", func() {
+		less := createIssue()
+		less.Severity = gosec.Low
+		greater := createIssue()
+		less.Severity = gosec.High
+		firstIsGreater(&less, &greater)
+	})
+
+	Context("Serverity is same", func() {
+		It("sortes by What", func() {
+			less := createIssue()
+			less.What = "test1"
+			greater := createIssue()
+			greater.What = "test2"
+			firstIsGreater(&less, &greater)
+		})
+	})
+
+	Context("Serverity and What is same", func() {
+		It("sortes by File", func() {
+			less := createIssue()
+			less.File = "test1"
+			greater := createIssue()
+			greater.File = "test2"
+
+			firstIsGreater(&less, &greater)
+		})
+	})
+
+	Context("Serverity, What and File is same", func() {
+		It("sortes by line number", func() {
+			less := createIssue()
+			less.Line = "1"
+			greater := createIssue()
+			greater.Line = "2"
+
+			firstIsGreater(&less, &greater)
+		})
+	})
+})
diff --git a/go.mod b/go.mod
index 350c3c224f..5a37c64537 100644
--- a/go.mod
+++ b/go.mod
@@ -9,7 +9,6 @@ require (
 	github.com/onsi/ginkgo v1.14.0
 	github.com/onsi/gomega v1.10.1
 	github.com/stretchr/testify v1.4.0 // indirect
-	golang.org/x/text v0.3.2 // indirect
 	golang.org/x/tools v0.0.0-20200831203904-5a2aa26beb65
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 	gopkg.in/yaml.v2 v2.3.0
diff --git a/go.sum b/go.sum
index cee1839b45..0271f9c586 100644
--- a/go.sum
+++ b/go.sum
@@ -3,6 +3,7 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
+github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -36,6 +37,7 @@ github.com/mozilla/tls-observatory v0.0.0-20200317151703-4fa42e1c2dee h1:1xJ+Xi9
 github.com/mozilla/tls-observatory v0.0.0-20200317151703-4fa42e1c2dee/go.mod h1:SrKMQvPiws7F7iqYp8/TX+IhxCYhzr6N/1yb8cwHsGk=
 github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d h1:AREM5mwr4u1ORQBMvzfzBgpsctsbQikCVpvC+tX285E=
 github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d/go.mod h1:o96djdrsSGy3AWPyBgZMAGfxZNfgntdJG+11KU4QvbU=
+github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.12.0 h1:Iw5WCbBcaAAd0fpRb1c9r5YCylv4XDoCSigm1zLevwU=
@@ -43,11 +45,13 @@ github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
 github.com/onsi/ginkgo v1.12.3/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY=
 github.com/onsi/ginkgo v1.13.0/go.mod h1:+REjRxOmWfHCjfv9TTWB1jD1Frx4XydAD3zm1lskyM0=
+github.com/onsi/ginkgo v1.14.0 h1:2mOpI4JVVPBN+WQRa0WKH2eXR+Ey+uK4n7Zj0aYpIQA=
 github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY=
 github.com/onsi/gomega v1.7.1 h1:K0jcRCwNQM3vFGh1ppMtDh/+7ApJrjldlX8fA0jDTLQ=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
 github.com/onsi/gomega v1.9.0 h1:R1uwffexN6Pr340GtYRIdZmAiN4J+iw6WG4wog1DUXg=
 github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
+github.com/onsi/gomega v1.10.1 h1:o0+MgICZLuZ7xjH7Vx6zS/zcu93/BEp1VwkIW1mEXCE=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
@@ -75,6 +79,7 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZ
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
+golang.org/x/net v0.0.0-20200822124328-c89045814202 h1:VvcQYSHwXgi7W+TpUR6A9g6Up98WAHf3f/ulnJ62IyA=
 golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f h1:wMNYb4v58l5UBM7MYRLPG6ZhfOqbKu7X5eyFl8ZhKvA=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -89,6 +94,7 @@ golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e h1:N7DeIrjYszNmSW409R3frPPwglRwMkXSBzwVbkOjLLA=
 golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200519105757-fe76b779f299 h1:DYfZAGf2WMFjMxbgTjaC+2HC7NkNAQs+6Q8b9WEB/F4=
 golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
diff --git a/output/formatter.go b/output/formatter.go
index c3616c3b6a..61d14719e0 100644
--- a/output/formatter.go
+++ b/output/formatter.go
@@ -242,9 +242,7 @@ func reportGolint(w io.Writer, data *reportInfo) error {
 }
 
 func reportJUnitXML(w io.Writer, data *reportInfo) error {
-	groupedData := groupDataByRules(data)
-	junitXMLStruct := createJUnitXMLStruct(groupedData)
-
+	junitXMLStruct := createJUnitXMLStruct(data)
 	raw, err := xml.MarshalIndent(junitXMLStruct, "", "\t")
 	if err != nil {
 		return err
diff --git a/output/formatter_test.go b/output/formatter_test.go
index 9ea1e0377c..985ea295b0 100644
--- a/output/formatter_test.go
+++ b/output/formatter_test.go
@@ -12,6 +12,13 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
+func createIssueWithFileWhat(file, what string) *gosec.Issue {
+	issue := createIssue("i1", gosec.GetCwe("G101"))
+	issue.File = file
+	issue.What = what
+	return &issue
+}
+
 func createIssue(ruleID string, cwe gosec.Cwe) gosec.Issue {
 	return gosec.Issue{
 		File:       "/home/src/project/test.go",
@@ -248,6 +255,23 @@ var _ = Describe("Formatter", func() {
 			Expect(*issues).To(Equal(*want))
 		})
 	})
+
+	Context("When using junit", func() {
+		It("preserves order of issues", func() {
+			issues := []*gosec.Issue{createIssueWithFileWhat("i1", "1"), createIssueWithFileWhat("i2", "2"), createIssueWithFileWhat("i3", "1")}
+
+			junitReport := createJUnitXMLStruct(&reportInfo{Issues: issues})
+
+			testSuite := junitReport.Testsuites[0]
+
+			Expect(testSuite.Testcases[0].Name).To(Equal(issues[0].File))
+			Expect(testSuite.Testcases[1].Name).To(Equal(issues[2].File))
+
+			testSuite = junitReport.Testsuites[1]
+			Expect(testSuite.Testcases[0].Name).To(Equal(issues[1].File))
+
+		})
+	})
 	Context("When using different report formats", func() {
 
 		grules := []string{"G101", "G102", "G103", "G104", "G106",
diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go
index d2e6523d5f..de11e49532 100644
--- a/output/junit_xml_format.go
+++ b/output/junit_xml_format.go
@@ -40,35 +40,30 @@ func generatePlaintext(issue *gosec.Issue) string {
 		", CWE: " + issue.Cwe.ID + ")\n" + "> " + htmlLib.EscapeString(issue.Code)
 }
 
-func groupDataByRules(data *reportInfo) map[string][]*gosec.Issue {
-	groupedData := make(map[string][]*gosec.Issue)
-	for _, issue := range data.Issues {
-		if _, ok := groupedData[issue.What]; !ok {
-			groupedData[issue.What] = []*gosec.Issue{}
-		}
-		groupedData[issue.What] = append(groupedData[issue.What], issue)
-	}
-	return groupedData
-}
-
-func createJUnitXMLStruct(groupedData map[string][]*gosec.Issue) junitXMLReport {
+func createJUnitXMLStruct(data *reportInfo) junitXMLReport {
 	var xmlReport junitXMLReport
-	for what, issues := range groupedData {
-		testsuite := testsuite{
-			Name:  what,
-			Tests: len(issues),
+	testsuites := map[string]int{}
+
+	for _, issue := range data.Issues {
+		index, ok := testsuites[issue.What]
+		if !ok {
+			xmlReport.Testsuites = append(xmlReport.Testsuites, testsuite{
+				Name: issue.What,
+			})
+			index = len(xmlReport.Testsuites) - 1
+			testsuites[issue.What] = index
 		}
-		for _, issue := range issues {
-			testcase := testcase{
-				Name: issue.File,
-				Failure: failure{
-					Message: "Found 1 vulnerability. See stacktrace for details.",
-					Text:    generatePlaintext(issue),
-				},
-			}
-			testsuite.Testcases = append(testsuite.Testcases, testcase)
+		testcase := testcase{
+			Name: issue.File,
+			Failure: failure{
+				Message: "Found 1 vulnerability. See stacktrace for details.",
+				Text:    generatePlaintext(issue),
+			},
 		}
-		xmlReport.Testsuites = append(xmlReport.Testsuites, testsuite)
+
+		xmlReport.Testsuites[index].Testcases = append(xmlReport.Testsuites[index].Testcases, testcase)
+		xmlReport.Testsuites[index].Tests++
 	}
+
 	return xmlReport
 }