From dba032fdf4c11513044cb2909fb26a3597101b9c Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Tue, 5 Nov 2024 21:05:46 +0000 Subject: [PATCH] internal/sarif: use empty arrays instead of nils Sarif specification requires that some slice elements explicitly exist in the JSON output even if they are empty. For instance, results should be an empty array if the sarif handler finished but found nothing. Another example is tags. Each rule in govulncheck sarif has tags property that can sometimes be empty. If so, JSON should contain an empty slice of tags. Fixes golang/go#70157 Change-Id: I112181e4efa5bc0a1577ff98f1b9eab912ed814c Reviewed-on: https://go-review.googlesource.com/c/vuln/+/625656 LUCI-TryBot-Result: Go LUCI Reviewed-by: Maceo Thompson --- .../source-call/source_call_sarif.ct | 30 +++++++++++++++++++ .../testfiles/source-call/source_call_text.ct | 4 +++ internal/sarif/handler.go | 19 ++++++++---- internal/sarif/sarif.go | 10 +++---- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct index a0c1d91c..ca99f34c 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct @@ -496,3 +496,33 @@ $ govulncheck -C ${moddir}/vuln -format sarif ./... } ] } + +# Test sarif json output for no vulnerabilities +$ govulncheck -C ${moddir}/novuln -format sarif ./... +{ + "version": "2.1.0", + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "tool": { + "driver": { + "name": "govulncheck", + "semanticVersion": "v0.0.0", + "informationUri": "https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck", + "properties": { + "protocol_version": "v1.0.0", + "scanner_name": "govulncheck", + "scanner_version": "v0.0.0-00000000000-20000101010101", + "db": "testdata/vulndb-v1", + "db_last_modified": "2023-04-03T15:57:51Z", + "go_version": "go1.18", + "scan_level": "symbol", + "scan_mode": "source" + }, + "rules": [] + } + }, + "results": [] + } + ] +} diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct index 6f3bc37a..503dc09e 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct @@ -135,3 +135,7 @@ Your code is affected by 2 vulnerabilities from 1 module. This scan also found 1 vulnerability in packages you import and 1 vulnerability in modules you require, but your code doesn't appear to call these vulnerabilities. + +# Test no vulnerabilities in source mode +$ govulncheck -C ${moddir}/novuln ./... +No vulnerabilities found. diff --git a/internal/sarif/handler.go b/internal/sarif/handler.go index 3d6f6894..d9e585b7 100644 --- a/internal/sarif/handler.go +++ b/internal/sarif/handler.go @@ -142,7 +142,7 @@ func toSarif(h *handler) Log { } func rules(h *handler) []Rule { - var rs []Rule + rs := make([]Rule, 0, len(h.findings)) // must not be nil for id := range h.findings { osv := h.osvs[id] // s is either summary if it exists, or details @@ -157,15 +157,24 @@ func rules(h *handler) []Rule { FullDescription: Description{Text: s}, HelpURI: fmt.Sprintf("https://pkg.go.dev/vuln/%s", osv.ID), Help: Description{Text: osv.Details}, - Properties: RuleTags{Tags: osv.Aliases}, + Properties: RuleTags{Tags: tags(osv)}, }) } sort.SliceStable(rs, func(i, j int) bool { return rs[i].ID < rs[j].ID }) return rs } +// tags returns an slice of zero or +// more aliases of o. +func tags(o *osv.Entry) []string { + if len(o.Aliases) > 0 { + return o.Aliases + } + return []string{} // must not be nil +} + func results(h *handler) []Result { - var results []Result + results := make([]Result, 0, len(h.findings)) // must not be nil for osv, fs := range h.findings { var locs []Location if h.cfg.ScanMode != govulncheck.ScanModeBinary { @@ -283,7 +292,7 @@ func stack(h *handler, f *govulncheck.Finding) Stack { trace := f.Trace top := trace[len(trace)-1] // belongs to top level module - var frames []Frame + frames := make([]Frame, 0, len(trace)) // must not be nil for i := len(trace) - 1; i >= 0; i-- { // vulnerable symbol is at the top frame frame := trace[i] pos := govulncheck.Position{Line: 1, Column: 1} @@ -350,7 +359,7 @@ func codeFlows(h *handler, fs []*govulncheck.Finding) []CodeFlow { } func threadFlows(h *handler, fs []*govulncheck.Finding) []ThreadFlow { - var tfs []ThreadFlow + tfs := make([]ThreadFlow, 0, len(fs)) // must not be nil for _, f := range fs { trace := traces.Compact(f) top := trace[len(trace)-1] // belongs to top level module diff --git a/internal/sarif/sarif.go b/internal/sarif/sarif.go index ca076005..934ade8b 100644 --- a/internal/sarif/sarif.go +++ b/internal/sarif/sarif.go @@ -66,7 +66,7 @@ type Run struct { Tool Tool `json:"tool,omitempty"` // Results contain govulncheck findings. There should be exactly one // Result per a detected use of an OSV. - Results []Result `json:"results,omitempty"` + Results []Result `json:"results"` } // Tool captures information about govulncheck analysis that was run. @@ -85,7 +85,7 @@ type Driver struct { // Properties are govulncheck run metadata, such as vuln db, Go version, etc. Properties govulncheck.Config `json:"properties,omitempty"` - Rules []Rule `json:"rules,omitempty"` + Rules []Rule `json:"rules"` } // Rule corresponds to the static analysis rule/analyzer that @@ -105,7 +105,7 @@ type Rule struct { // RuleTags defines properties.tags. type RuleTags struct { - Tags []string `json:"tags,omitempty"` + Tags []string `json:"tags"` } // Description is a text in its raw or markdown form. @@ -143,7 +143,7 @@ type Result struct { // for, say, a particular symbol or package. type CodeFlow struct { // ThreadFlows is effectively a set of related information flows. - ThreadFlows []ThreadFlow `json:"threadFlows,omitempty"` + ThreadFlows []ThreadFlow `json:"threadFlows"` Message Description `json:"message,omitempty"` } @@ -165,7 +165,7 @@ type ThreadFlowLocation struct { // Stack is a sequence of frames and can encode a govulncheck call stack. type Stack struct { Message Description `json:"message,omitempty"` - Frames []Frame `json:"frames,omitempty"` + Frames []Frame `json:"frames"` } // Frame is effectively a module location. It can also contain thread and