fix(sarif): avoid invalid null relationships in SARIF output (#1569)

* fix(sarif): avoid null relationships for rules without CWE

* test(sarif): add offline schema validation and self-scan coverage

* test(sarif): fix lint in self-scan helper

Author: ccojocar

report/sarif/common_test.go

@@ -3,11 +3,10 @@ package sarif_test
 import (
 	"bufio"
 	"bytes"
+	_ "embed"
 	"encoding/json"
 	"fmt"
-	"net/http"
 	"sync"
-	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	"github.com/santhosh-tekuri/jsonschema/v6"
@@ -16,30 +15,20 @@ import (
 )
 
 var (
-	sarifSchemaOnce   sync.Once
-	sarifSchema       *jsonschema.Schema
-	sarifSchemaErr    error
-	sarifSchemaClient = &http.Client{Timeout: 30 * time.Second}
+	sarifSchemaOnce sync.Once
+	sarifSchema     *jsonschema.Schema
+	sarifSchemaErr  error
 )
 
+//go:embed testdata/sarif-schema-2.1.0.json
+var sarifSchemaJSON []byte
+
 func validateSarifSchema(report *sarif.Report) error {
 	GinkgoHelper()
 	sarifSchemaOnce.Do(func() {
-		resp, err := sarifSchemaClient.Get(sarif.Schema)
-		if err != nil {
-			sarifSchemaErr = fmt.Errorf("fetch sarif schema: %w", err)
-			return
-		}
-		defer resp.Body.Close()
-
-		if resp.StatusCode != http.StatusOK {
-			sarifSchemaErr = fmt.Errorf("fetch sarif schema: unexpected status %s", resp.Status)
-			return
-		}
-
-		schema, err := jsonschema.UnmarshalJSON(resp.Body)
+		schema, err := jsonschema.UnmarshalJSON(bytes.NewReader(sarifSchemaJSON))
 		if err != nil {
-			sarifSchemaErr = fmt.Errorf("error unmarshaling schema: %w", err)
+			sarifSchemaErr = fmt.Errorf("unmarshal local sarif schema: %w", err)
 			return
 		}
 

report/sarif/formatter.go

@@ -94,7 +94,8 @@ func parseSarifRule(i *issue.Issue) *ReportingDescriptor {
 	if cwe != nil {
 		name = cwe.Name
 	}
-	return &ReportingDescriptor{
+	relationship := buildSarifReportingDescriptorRelationship(i.Cwe)
+	rule := &ReportingDescriptor{
 		ID:               i.RuleID,
 		Name:             name,
 		ShortDescription: NewMultiformatMessageString(i.What),
@@ -108,10 +109,11 @@ func parseSarifRule(i *issue.Issue) *ReportingDescriptor {
 		DefaultConfiguration: &ReportingConfiguration{
 			Level: getSarifLevel(i.Severity.String()),
 		},
-		Relationships: []*ReportingDescriptorRelationship{
-			buildSarifReportingDescriptorRelationship(i.Cwe),
-		},
 	}
+	if relationship != nil {
+		rule.Relationships = []*ReportingDescriptorRelationship{relationship}
+	}
+	return rule
 }
 
 func buildSarifReportingDescriptorRelationship(weakness *cwe.Weakness) *ReportingDescriptorRelationship {

report/sarif/sarif_test.go

@@ -62,6 +62,38 @@ var _ = Describe("Sarif Formatter", func() {
 			Expect(validateSarifSchema(sarifReport)).To(Succeed())
 		})
 
+		It("sarif formatted report should not include null relationships when CWE is missing (issue #1568)", func() {
+			issueWithoutCWE := []*issue.Issue{
+				{
+					File:       "/home/src/project/test.go",
+					Line:       "1",
+					Col:        "1",
+					RuleID:     "G706",
+					What:       "Log injection via taint analysis",
+					Confidence: issue.High,
+					Severity:   issue.Low,
+					Code:       "1: testcode",
+					Cwe:        nil,
+				},
+			}
+
+			reportInfo := gosec.NewReportInfo(issueWithoutCWE, &gosec.Metrics{}, map[string][]gosec.Error{}).WithVersion("v2.24.0")
+
+			sarifReport, err := sarif.GenerateReport([]string{}, reportInfo)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(validateSarifSchema(sarifReport)).To(Succeed())
+
+			Expect(sarifReport.Runs[0].Tool.Driver.Rules).To(HaveLen(1))
+			Expect(sarifReport.Runs[0].Tool.Driver.Rules[0].Relationships).To(BeNil())
+
+			buf := new(bytes.Buffer)
+			err = sarif.WriteReport(buf, reportInfo, []string{})
+			Expect(err).ShouldNot(HaveOccurred())
+			output := buf.String()
+			Expect(output).NotTo(ContainSubstring(`"relationships":[null]`))
+			Expect(output).NotTo(ContainSubstring(`"relationships": [null]`))
+		})
+
 		It("sarif formatted report should not include fixes when autofix is empty (issue #1482)", func() {
 			ruleID := "G304"
 			cwe := issue.GetCweByRule(ruleID)

report/sarif/self_scan_test.go

@@ -0,0 +1,59 @@
+package sarif_test
+
+import (
+	"encoding/json"
+	"io"
+	"log"
+	"path/filepath"
+	"runtime"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/securego/gosec/v2"
+	"github.com/securego/gosec/v2/report/sarif"
+	"github.com/securego/gosec/v2/rules"
+)
+
+var _ = Describe("Sarif Self Scan", func() {
+	It("produces locally valid sarif without null relationships when scanning gosec source", func() {
+		repoRoot := currentRepoRoot()
+
+		config := gosec.NewConfig()
+		logger := log.New(io.Discard, "", 0)
+		analyzer := gosec.NewAnalyzer(config, false, true, false, 4, logger)
+
+		ruleList := rules.Generate(false, rules.NewRuleFilter(false, "G401"))
+		analyzer.LoadRules(ruleList.RulesInfo())
+
+		excludedDirs := gosec.ExcludedDirsRegExp([]string{"vendor", ".git"})
+		packagePaths, err := gosec.PackagePaths(filepath.Join(repoRoot, "..."), excludedDirs)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(packagePaths).ShouldNot(BeEmpty())
+
+		err = analyzer.Process(nil, packagePaths...)
+		Expect(err).ShouldNot(HaveOccurred())
+
+		issues, metrics, errors := analyzer.Report()
+		Expect(issues).ShouldNot(BeEmpty())
+		reportInfo := gosec.NewReportInfo(issues, metrics, errors).WithVersion("test")
+
+		sarifReport, err := sarif.GenerateReport([]string{repoRoot}, reportInfo)
+		Expect(err).ShouldNot(HaveOccurred())
+
+		Expect(validateSarifSchema(sarifReport)).To(Succeed())
+
+		encoded, err := json.Marshal(sarifReport)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(encoded).NotTo(ContainSubstring(`"relationships":[null]`))
+		Expect(encoded).NotTo(ContainSubstring(`"relationships": [null]`))
+	})
+})
+
+func currentRepoRoot() string {
+	programCounter, currentFile, line, ok := runtime.Caller(0)
+	if !ok || programCounter == 0 || line <= 0 {
+		return filepath.Clean(".")
+	}
+	return filepath.Clean(filepath.Join(filepath.Dir(currentFile), "..", ".."))
+}