Skip to content

Commit

Permalink
internal/sarif: use empty arrays instead of nils
Browse files Browse the repository at this point in the history
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Maceo Thompson <maceothompson@google.com>
  • Loading branch information
zpavlinovic committed Nov 8, 2024
1 parent 47cd072 commit dba032f
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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": []
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
19 changes: 14 additions & 5 deletions internal/sarif/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions internal/sarif/sarif.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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"`
}

Expand All @@ -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
Expand Down

0 comments on commit dba032f

Please sign in to comment.