Merge pull request #1653 from irgaly/fix/sarif_range

fix: SARIF parser: parse with no region result. fix originalOutput field

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### :bug: Fixes
 - [#1651](https://github.com/reviewdog/reviewdog/pull/1651) Revert #1576: Support `--filter-mode=file` in `github-pr-review`. Reasons: #1645
+- [#1653](https://github.com/reviewdog/reviewdog/pull/1653) fix: SARIF parser: parse with no region result. fix originalOutput field
+- ...
 
 ### :rotating_light: Breaking changes
 - ...

parser/sarif.go

@@ -2,7 +2,6 @@ package parser
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/url"
 	"os"
@@ -46,6 +45,10 @@ func (p *SarifParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
 			rules[rule.ID] = rule
 		}
 		for _, result := range run.Results {
+			original, err := json.Marshal(result)
+			if err != nil {
+				return nil, err
+			}
 			message := result.Message.GetText()
 			ruleID := result.RuleID
 			rule := rules[ruleID]
@@ -88,10 +91,6 @@ func (p *SarifParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
 				}
 				region := physicalLocation.Region
 				rng := region.GetRdfRange()
-				if rng == nil {
-					// No line information in result
-					continue
-				}
 				var code *rdf.Code
 				if ruleID != "" {
 					code = &rdf.Code{
@@ -110,12 +109,9 @@ func (p *SarifParser) Parse(r io.Reader) ([]*rdf.Diagnostic, error) {
 						Name: name,
 						Url:  informationURI,
 					},
-					Code:        code,
-					Suggestions: suggestionsMap[path],
-					OriginalOutput: fmt.Sprintf(
-						"%v:%d:%d: %v: %v (%v)",
-						path, rng.Start.Line, getActualStartColumn(rng), level, message, ruleID,
-					),
+					Code:           code,
+					Suggestions:    suggestionsMap[path],
+					OriginalOutput: string(original),
 				}
 				ds = append(ds, d)
 			}
@@ -132,27 +128,27 @@ type SarifJson struct {
 	Runs []struct {
 		OriginalURIBaseIds map[string]SarifOriginalURI `json:"originalUriBaseIds"`
 		Results            []struct {
-			Level     string `json:"level"`
+			Level     string `json:"level,omitempty"`
 			Locations []struct {
 				PhysicalLocation struct {
-					ArtifactLocation SarifArtifactLocation `json:"artifactLocation"`
-					Region           SarifRegion           `json:"region"`
-				} `json:"physicalLocation"`
+					ArtifactLocation SarifArtifactLocation `json:"artifactLocation,omitempty"`
+					Region           SarifRegion           `json:"region,omitempty"`
+				} `json:"physicalLocation,omitempty"`
 			} `json:"locations"`
 			Message SarifText `json:"message"`
-			RuleID  string    `json:"ruleId"`
+			RuleID  string    `json:"ruleId,omitempty"`
 			Fixes   []struct {
 				Description     SarifText `json:"description"`
 				ArtifactChanges []struct {
-					ArtifactLocation SarifArtifactLocation `json:"artifactLocation"`
+					ArtifactLocation SarifArtifactLocation `json:"artifactLocation,omitempty"`
 					Replacements     []struct {
 						DeletedRegion   SarifRegion `json:"deletedRegion"`
 						InsertedContent struct {
 							Text string `json:"text"`
-						} `json:"insertedContent"`
+						} `json:"insertedContent,omitempty"`
 					} `json:"replacements"`
 				} `json:"artifactChanges"`
-			} `json:"fixes"`
+			} `json:"fixes,omitempty"`
 		} `json:"results"`
 		Tool struct {
 			Driver struct {
@@ -170,9 +166,9 @@ type SarifOriginalURI struct {
 }
 
 type SarifArtifactLocation struct {
-	URI       string `json:"uri"`
+	URI       string `json:"uri,omitempty"`
 	URIBaseID string `json:"uriBaseId"`
-	Index     int    `json:"index"`
+	Index     int    `json:"index,omitempty"`
 }
 
 func (l *SarifArtifactLocation) GetPath(
@@ -198,8 +194,8 @@ func (l *SarifArtifactLocation) GetPath(
 }
 
 type SarifText struct {
-	Text     string  `json:"text"`
-	Markdown *string `json:"markdown"`
+	Text     string  `json:"text,omitempty"`
+	Markdown *string `json:"markdown,omitempty"`
 }
 
 func (t *SarifText) GetText() string {
@@ -212,9 +208,9 @@ func (t *SarifText) GetText() string {
 
 type SarifRegion struct {
 	StartLine   *int `json:"startLine"`
-	StartColumn *int `json:"startColumn"`
-	EndLine     *int `json:"endLine"`
-	EndColumn   *int `json:"endColumn"`
+	StartColumn *int `json:"startColumn,omitempty"`
+	EndLine     *int `json:"endLine,omitempty"`
+	EndColumn   *int `json:"endColumn,omitempty"`
 }
 
 // convert SARIF Region to RDF Range
@@ -353,12 +349,3 @@ type SarifRule struct {
 		Rank  int    `json:"rank"`
 	} `json:"defaultConfiguration"`
 }
-
-func getActualStartColumn(r *rdf.Range) int32 {
-	startColumn := r.Start.Column
-	if startColumn == 0 {
-		// startColumn's default value means column 1
-		startColumn = 1
-	}
-	return startColumn
-}

parser/sarif_test.go

@@ -1,13 +1,12 @@
 package parser
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
-	"reflect"
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/reviewdog/reviewdog/service/serviceutil"
 	"google.golang.org/protobuf/encoding/protojson"
 )
@@ -19,18 +18,25 @@ func TestExampleSarifParser(t *testing.T) {
 		if err != nil {
 			panic(err)
 		}
+		if len(diagnostics) == 0 {
+			t.Errorf("empty diagnostics")
+		}
 		for _, d := range diagnostics {
-			rdjson, _ := protojson.MarshalOptions{Indent: "  "}.Marshal(d)
-			var actualJson interface{}
-			var expectJson interface{}
-			json.Unmarshal([]byte(rdjson), &actualJson)
-			json.Unmarshal([]byte(fixture[1]), &expectJson)
-			if !reflect.DeepEqual(actualJson, expectJson) {
-				var out bytes.Buffer
-				json.Indent(&out, rdjson, "", "\t")
-				actual := out.String()
-				expect := fixture[1]
-				t.Errorf("actual(%v):\n%v\n---\nexpect(%v):\n%v", i, actual, i, expect)
+			rdjson, err := protojson.MarshalOptions{Indent: "  "}.Marshal(d)
+			if err != nil {
+				t.Fatal(err)
+			}
+			var actualJSON map[string]any
+			var expectJSON map[string]any
+			if err := json.Unmarshal(rdjson, &actualJSON); err != nil {
+				t.Fatal(err)
+			}
+			if err := json.Unmarshal([]byte(fixture[1]), &expectJSON); err != nil {
+				t.Fatal(err)
+			}
+			expectJSON["originalOutput"] = actualJSON["originalOutput"]
+			if diff := cmp.Diff(actualJSON, expectJSON); diff != "" {
+				t.Errorf("fixtures[%d] (-got, +want):\n%s", i, diff)
 			}
 		}
 	}
@@ -121,8 +127,7 @@ var fixtures = [][]string{{
 	"code": {
 		"value": "rule_id",
 		"url": "https://example.com"
-	},
-	"originalOutput": "src/MyClass.kt:10:5: warning: result message (rule_id)"
+	}
 }`},
 	{`{
 	"runs": [
@@ -184,8 +189,7 @@ var fixtures = [][]string{{
 	},
 	"code": {
 		"value": "rule_id"
-	},
-	"originalOutput": "src/MyClass.kt:10:1: error: message (rule_id)"
+	}
 }`},
 	{`{
 	"runs": [
@@ -270,7 +274,85 @@ var fixtures = [][]string{{
 			},
 			"text": "// "
 		}
-	],
-	"originalOutput": "src/MyClass.kt:10:1: : message (rule_id)"
+	]
+}`},
+	{fmt.Sprintf(`{
+	"runs": [
+	  {
+		"originalUriBaseIds": {
+			"ROOTPATH": {
+			  "uri": "%s"
+			}
+		  },
+		"tool": {
+		  "driver": {
+			"name": "Trivy",
+			"informationUri": "https://github.com/aquasecurity/trivy",
+			"fullName": "Trivy Vulnerability Scanner",
+			"version": "0.15.0",
+			"rules": [
+			  {
+				"id": "CVE-2018-14618/curl",
+				"name": "OS Package Vulnerability (Alpine)",
+				"shortDescription": {
+				  "text": "CVE-2018-14618 Package: curl"
+				},
+				"fullDescription": {
+				  "text": "curl: NTLM password overflow via integer overflow."
+				},
+				"defaultConfiguration": {
+				  "level": "error"
+				},
+				"helpUri": "https://avd.aquasec.com/nvd/cve-2018-14618",
+				"help": {
+				  "text": "Vulnerability CVE-2018-14618\nSeverity: CRITICAL\n...",
+				  "markdown": "**Vulnerability CVE-2018-14618**\n| Severity..."
+				},
+				"properties": {
+				  "tags": [
+					"vulnerability",
+					"CRITICAL",
+					"curl"
+				  ],
+				  "precision": "very-high"
+				}
+			  }
+			]
+		  }
+		},
+		"results": [
+		  {
+			"ruleId": "CVE-2018-14618/curl",
+			"ruleIndex": 0,
+			"level": "error",
+			"message": {
+			  "text": "curl before version 7.61.1 is..."
+			},
+			"locations": [{
+			  "physicalLocation": {
+				"artifactLocation": {
+				  "uri": "knqyf263/vuln-image (alpine 3.7.1)",
+				  "uriBaseId": "ROOTPATH"
+				}
+			  }
+			}]
+		  }]
+	  }
+	]
+  }
+`, basedir()), `{
+	"message": "curl before version 7.61.1 is...",
+	"location": {
+		"path": "knqyf263/vuln-image (alpine 3.7.1)"
+	},
+	"severity": "ERROR",
+	"source": {
+		"name": "Trivy",
+		"url": "https://github.com/aquasecurity/trivy"
+	},
+	"code": {
+		"value": "CVE-2018-14618/curl",
+		"url": "https://avd.aquasec.com/nvd/cve-2018-14618"
+	}
 }`},
 }