Skip to content

Commit

Permalink
internal/report: detect unused exclusions
Browse files Browse the repository at this point in the history
  • Loading branch information
seilagamo committed Aug 23, 2024
1 parent f7e76af commit 58948df
Showing 9 changed files with 205 additions and 26 deletions.
1 change: 1 addition & 0 deletions cmd/lava/internal/scan/scan.go
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@ that have been found.
- 1: Command error
- 2: Syntax error
- 3: Check error
- 4: Not matching exclusions
- 100: Informational vulnerabilities found
- 101: Low severity vulnerabilities found
- 102: Medium severity vulnerabilities found
2 changes: 2 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
@@ -189,6 +189,8 @@ type ReportConfig struct {
// instance, accepted risks, false positives, etc.
Exclusions []Exclusion `yaml:"exclusions"`

ErrorOnNotMatchedExclusions bool `yaml:"error_not_matched_exclusions"`

// Metrics is the file where the metrics will be written.
// If Metrics is an empty string or not specified in the yaml file, then
// the metrics report is not saved.
34 changes: 34 additions & 0 deletions internal/report/human.tmpl
Original file line number Diff line number Diff line change
@@ -5,6 +5,9 @@
{{- if .Vulns}}
{{template "vulns" . -}}
{{end -}}
{{- if not .AllExclMatched}}
{{template "exclsNotMatched" . -}}
{{end -}}
{{- end -}}


@@ -162,6 +165,37 @@ Number of excluded vulnerabilities not included in the summary table: {{.Exclude
{{end}}{{end}}
{{- end -}}

{{- /* exclsNotMatched is the template used to render the details of a the not matched exclusions. */ -}}
{{define "exclsNotMatched" -}}
{{"The following exclusions don't match any finding:" | red }}
{{range $excl := .Exclusions}}
{{- if not $excl.Matched -}}
{{template "excl" .}}
{{end -}}
{{end}}
{{- end -}}

{{- /* excl is the template used to render an exclusion. */ -}}
{{- define "excl" -}}
{{- if .Target}}
{{"Target: " | bold}}{{.Target | trim -}}
{{end -}}
{{- if .Description}}
{{"Description: " | bold}}{{.Description | trim -}}
{{end -}}
{{- if .Resource}}
{{"Resource: " | bold}}{{.Resource | trim -}}
{{end -}}
{{- if .Fingerprint}}
{{"Fingerprint: " | bold}}{{.Fingerprint | trim -}}
{{end -}}
{{- if .Summary}}
{{"Summary: " | bold}}{{.Summary | trim -}}
{{end -}}
{{- if not .ExpirationDate.IsZero}}
{{"Expiration Date: " | bold}}{{.ExpirationDate.String | trim -}}
{{end -}}
{{- end -}}

{{- /* Render the report. */ -}}
{{- template "report" . -}}
36 changes: 25 additions & 11 deletions internal/report/humanprinter.go
Original file line number Diff line number Diff line change
@@ -40,7 +40,8 @@ var (
)

// Print renders the scan results in a human-readable format.
func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus) error {
func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary,
status []checkStatus, exclusions []reportExclusion) error {
// count the total non-excluded vulnerabilities found.
var total int
for _, ss := range summ.count {
@@ -52,18 +53,31 @@ func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary,
stats[s.String()] = summ.count[s]
}

// check if there are exclusions not matched.
var allMatched = true
for _, e := range exclusions {
if !e.Matched {
allMatched = false
break
}
}

data := struct {
Stats map[string]int
Total int
Excluded int
Vulns []vulnerability
Status []checkStatus
Stats map[string]int
Total int
Excluded int
Vulns []vulnerability
Status []checkStatus
AllExclMatched bool
Exclusions []reportExclusion
}{
Stats: stats,
Total: total,
Excluded: summ.excluded,
Vulns: vulns,
Status: status,
Stats: stats,
Total: total,
Excluded: summ.excluded,
Vulns: vulns,
Status: status,
AllExclMatched: allMatched,
Exclusions: exclusions,
}

if err := humanTmpl.Execute(w, data); err != nil {
3 changes: 2 additions & 1 deletion internal/report/humanprinter_test.go
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ func TestUserFriendlyPrinter_Print(t *testing.T) {
vulnerabilities []vulnerability
summ summary
status []checkStatus
exclusions []reportExclusion
want []string
}{
{
@@ -193,7 +194,7 @@ func TestUserFriendlyPrinter_Print(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var buf bytes.Buffer
w := humanPrinter{}
if err := w.Print(&buf, tt.vulnerabilities, tt.summ, tt.status); err != nil {
if err := w.Print(&buf, tt.vulnerabilities, tt.summ, tt.status, tt.exclusions); err != nil {
t.Errorf("unexpected error value: %v", err)
}
text := buf.String()
2 changes: 1 addition & 1 deletion internal/report/jsonprinter.go
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ import (
type jsonPrinter struct{}

// Print renders the scan results in JSON format.
func (prn jsonPrinter) Print(w io.Writer, vulns []vulnerability, _ summary, _ []checkStatus) error {
func (prn jsonPrinter) Print(w io.Writer, vulns []vulnerability, _ summary, _ []checkStatus, _ []reportExclusion) error {
enc := json.NewEncoder(w)
enc.SetIndent("", " ")
if err := enc.Encode(vulns); err != nil {
2 changes: 1 addition & 1 deletion internal/report/jsonprinter_test.go
Original file line number Diff line number Diff line change
@@ -163,7 +163,7 @@ func TestJsonPrinter_Print(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var buf bytes.Buffer
w := jsonPrinter{}
err := w.Print(&buf, tt.vulnerabilities, summary{}, nil)
err := w.Print(&buf, tt.vulnerabilities, summary{}, nil, nil)
if (err == nil) != tt.wantNilErr {
t.Errorf("unexpected error value: %v", err)
}
58 changes: 46 additions & 12 deletions internal/report/report.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"os"
"regexp"
"slices"
@@ -28,7 +29,13 @@ type Writer struct {
isStdout bool
minSeverity config.Severity
showSeverity config.Severity
exclusions []config.Exclusion
exclusions []reportExclusion
eome bool
}

type reportExclusion struct {
config.Exclusion
Matched bool
}

// timeNow is set by tests to mock the current time.
@@ -64,13 +71,19 @@ func NewWriter(cfg config.ReportConfig) (Writer, error) {
showSeverity = cfg.Severity
}

var exclusions []reportExclusion
for _, e := range cfg.Exclusions {
exclusions = append(exclusions, reportExclusion{Exclusion: e, Matched: false})
}

return Writer{
prn: prn,
w: w,
isStdout: isStdout,
minSeverity: cfg.Severity,
showSeverity: showSeverity,
exclusions: cfg.Exclusions,
exclusions: exclusions,
eome: cfg.ErrorOnNotMatchedExclusions,
}, nil
}

@@ -96,7 +109,7 @@ func (writer Writer) Write(er engine.Report) (ExitCode, error) {
status := mkStatus(er)
exitCode := writer.calculateExitCode(summ, status)

if err = writer.prn.Print(writer.w, fvulns, summ, status); err != nil {
if err = writer.prn.Print(writer.w, fvulns, summ, status, writer.exclusions); err != nil {
return exitCode, fmt.Errorf("print report: %w", err)
}

@@ -140,8 +153,8 @@ func (writer Writer) parseReport(er engine.Report) ([]vulnerability, error) {

// isExcluded returns whether the provided [report.Vulnerability] is
// excluded based on the [Writer] configuration and the affected target.
func (writer Writer) isExcluded(v report.Vulnerability, target string) (bool, error) {
for _, excl := range writer.exclusions {
func (writer *Writer) isExcluded(v report.Vulnerability, target string) (bool, error) {
for i, excl := range writer.exclusions {
if !excl.ExpirationDate.IsZero() && excl.ExpirationDate.Before(timeNow()) {
continue
}
@@ -183,6 +196,8 @@ func (writer Writer) isExcluded(v report.Vulnerability, target string) (bool, er
continue
}
}

writer.exclusions[i].Matched = true
return true, nil
}
return false, nil
@@ -224,6 +239,24 @@ func (writer Writer) calculateExitCode(summ summary, status []checkStatus) ExitC
}
}

var notMatched bool
// Log all the not-matched exclusions.
for i, e := range writer.exclusions {
if !e.Matched {
notMatched = true
var loggerLevel func(msg string, args ...any)
if writer.eome {
loggerLevel = slog.Error
} else {
loggerLevel = slog.Warn
}
loggerLevel("The exclusion doesn't much with any finding", "index", i)
}
}
if writer.eome && notMatched {
return ExitCodeNotMatchedExclusions
}

for sev := config.SeverityCritical; sev >= writer.minSeverity; sev-- {
if summ.count[sev] > 0 {
diff := sev - config.SeverityInfo
@@ -243,7 +276,7 @@ type vulnerability struct {

// A printer renders a Vulcan report in a specific format.
type printer interface {
Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus) error
Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus, exclusions []reportExclusion) error
}

// scoreToSeverity converts a CVSS score into a [config.Severity].
@@ -324,10 +357,11 @@ type ExitCode int

// Exit codes depending on the maximum severity found.
const (
ExitCodeCheckError ExitCode = 3
ExitCodeInfo ExitCode = 100
ExitCodeLow ExitCode = 101
ExitCodeMedium ExitCode = 102
ExitCodeHigh ExitCode = 103
ExitCodeCritical ExitCode = 104
ExitCodeCheckError ExitCode = 3
ExitCodeNotMatchedExclusions ExitCode = 4
ExitCodeInfo ExitCode = 100
ExitCodeLow ExitCode = 101
ExitCodeMedium ExitCode = 102
ExitCodeHigh ExitCode = 103
ExitCodeCritical ExitCode = 104
)
93 changes: 93 additions & 0 deletions internal/report/report_test.go
Original file line number Diff line number Diff line change
@@ -210,6 +210,63 @@ func TestWriter_calculateExitCode(t *testing.T) {
},
want: ExitCodeCheckError,
},
{
name: "not matched exclusions (warn)",
summ: summary{
count: map[config.Severity]int{
config.SeverityCritical: 0,
config.SeverityHigh: 0,
config.SeverityMedium: 1,
config.SeverityLow: 1,
config.SeverityInfo: 1,
},
},
status: []checkStatus{
{
Checktype: "Checktype1",
Target: "Target1",
Status: "FINISHED",
},
},
rConfig: config.ReportConfig{
Severity: config.SeverityHigh,
Exclusions: []config.Exclusion{
{
Summary: "Unused exclusion",
},
},
},
want: 0,
},
{
name: "not matched exclusions (error)",
summ: summary{
count: map[config.Severity]int{
config.SeverityCritical: 0,
config.SeverityHigh: 0,
config.SeverityMedium: 1,
config.SeverityLow: 1,
config.SeverityInfo: 1,
},
},
status: []checkStatus{
{
Checktype: "Checktype1",
Target: "Target1",
Status: "FINISHED",
},
},
rConfig: config.ReportConfig{
Severity: config.SeverityHigh,
ErrorOnNotMatchedExclusions: true,
Exclusions: []config.Exclusion{
{
Summary: "Unused exclusion",
},
},
},
want: 4,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -694,6 +751,42 @@ func TestWriter_isExcluded(t *testing.T) {
}
}

func TestWriter_isExcluded_exclusionMatched(t *testing.T) {
v := vreport.Vulnerability{
Summary: "Vulnerability Summary 1",
Score: 6.7,
}
target := "."
rConfig := config.ReportConfig{
Exclusions: []config.Exclusion{
{
Summary: "Summary 1",
Description: "Excluded vulnerabilities Summary 1",
},
{
Summary: "Unused exclusion",
},
},
}
wantMatches := []bool{true, false}
w, err := NewWriter(rConfig)
if err != nil {
t.Fatalf("unable to create a report writer: %v", err)
}
got, err := w.isExcluded(v, target)
if err != nil {
t.Errorf("unexpected error value: %v", err)
}
if !got {
t.Errorf("unexpected excluded value: got: %v, want: %v", got, true)
}
for i, want := range wantMatches {
if w.exclusions[i].Matched != want {
t.Errorf("unexpected matched value: got: %v, want: %v", w.exclusions[i].Matched, want)
}
}
}

func TestMkSummary(t *testing.T) {
tests := []struct {
name string

0 comments on commit 58948df

Please sign in to comment.