all: add linting test
Lint all reports in main unit test.
Change-Id: Ib605dc109128581f8209a6a2088313bd0dbc4801
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/337851
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
diff --git a/cmd/gendb/main.go b/cmd/gendb/main.go
index 98ab2a5..a38fc56 100644
--- a/cmd/gendb/main.go
+++ b/cmd/gendb/main.go
@@ -64,8 +64,12 @@
if err != nil {
fail(fmt.Sprintf("unable to unmarshal %q: %s", f.Name(), err))
}
- if err = vuln.Lint(); err != nil {
- fail(fmt.Sprintf("invalid vulnerability %q: %s", f.Name(), err))
+ if lints := vuln.Lint(); len(lints) > 0 {
+ fmt.Fprintf(os.Stderr, "invalid vulnerability file %q:\n", os.Args[1])
+ for _, lint := range lints {
+ fmt.Fprintf(os.Stderr, "\t%s\n", lint)
+ }
+ os.Exit(1)
}
name := strings.TrimSuffix(filepath.Base(f.Name()), filepath.Ext(f.Name()))
diff --git a/cmd/genhtml/main.go b/cmd/genhtml/main.go
index bad0bd3..d961fac 100644
--- a/cmd/genhtml/main.go
+++ b/cmd/genhtml/main.go
@@ -198,8 +198,12 @@
if err != nil {
fail(fmt.Sprintf("unable to unmarshal %q: %s", f.Name(), err))
}
- if err = vuln.Lint(); err != nil {
- fail(fmt.Sprintf("invalid vulnerability %q: %s", f.Name(), err))
+ if lints := vuln.Lint(); len(lints) > 0 {
+ fmt.Fprintf(os.Stderr, "invalid vulnerability file %q:\n", os.Args[1])
+ for _, lint := range lints {
+ fmt.Fprintf(os.Stderr, "\t%s\n", lint)
+ }
+ os.Exit(1)
}
name := strings.TrimSuffix(filepath.Base(f.Name()), filepath.Ext(f.Name()))
htmlVulns[name] = vuln
diff --git a/cmd/linter/main.go b/cmd/linter/main.go
index 6312b75..09e019a 100644
--- a/cmd/linter/main.go
+++ b/cmd/linter/main.go
@@ -32,8 +32,11 @@
os.Exit(1)
}
- if err = vuln.Lint(); err != nil {
- fmt.Fprintf(os.Stderr, "invalid vulnerability file %q: %s\n", os.Args[1], err)
+ if lints := vuln.Lint(); len(lints) > 0 {
+ fmt.Fprintf(os.Stderr, "invalid vulnerability file %q:\n", os.Args[1])
+ for _, lint := range lints {
+ fmt.Fprintf(os.Stderr, "\t%s\n", lint)
+ }
os.Exit(1)
}
}
diff --git a/lint_test.go b/lint_test.go
new file mode 100644
index 0000000..7c1f351
--- /dev/null
+++ b/lint_test.go
@@ -0,0 +1,43 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "io/ioutil"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "golang.org/x/vulndb/report"
+ "gopkg.in/yaml.v2"
+)
+
+func TestLintReports(t *testing.T) {
+ reports, err := ioutil.ReadDir("reports")
+ if err != nil {
+ t.Fatalf("unable to read reports/: %s", err)
+ }
+
+ for _, rf := range reports {
+ if rf.IsDir() {
+ continue
+ }
+ t.Run(rf.Name(), func(t *testing.T) {
+ b, err := ioutil.ReadFile(filepath.Join("reports", rf.Name()))
+ if err != nil {
+ t.Fatalf("unable to read %q: %s", rf.Name(), err)
+ }
+
+ var r report.Report
+ if err := yaml.UnmarshalStrict(b, &r); err != nil {
+ t.Fatalf("unable to parse report %q: %s", rf.Name(), err)
+ }
+
+ if lints := r.Lint(); len(lints) > 0 {
+ t.Errorf(strings.Join(lints, "\n"))
+ }
+ })
+ }
+}
diff --git a/report/lint.go b/report/lint.go
index 5cc8ccf..5bfdc08 100644
--- a/report/lint.go
+++ b/report/lint.go
@@ -9,6 +9,7 @@
"fmt"
"io/ioutil"
"net/http"
+ "os"
"regexp"
"strings"
@@ -20,7 +21,13 @@
// TODO: getting things from the proxy should all be cached so we
// aren't re-requesting the same stuff over and over.
-const proxyURL = "https://proxy.golang.org"
+var proxyURL = "https://proxy.golang.org"
+
+func init() {
+ if proxy, ok := os.LookupEnv("GOPROXY"); ok {
+ proxyURL = proxy
+ }
+}
func getModVersions(module string) (map[string]bool, error) {
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/list", proxyURL, module))
@@ -131,44 +138,48 @@
// we aren't fixing one thing at a time. Similarly it might make sense to include
// warnings or informational things alongside errors, especially during for use
// during the triage process.
-func (vuln *Report) Lint() error {
+func (vuln *Report) Lint() []string {
+ var issues []string
+
var importPath string
if !vuln.Stdlib {
if vuln.Module == "" {
- return errors.New("missing module")
+ issues = append(issues, "missing module")
}
- if vuln.Package == vuln.Module {
- return errors.New("package is redundant and can be removed")
+ if vuln.Module != "" && vuln.Package == vuln.Module {
+ issues = append(issues, "package is redundant and can be removed")
}
if vuln.Package != "" && !strings.HasPrefix(vuln.Package, vuln.Module) {
- return errors.New("module must be a prefix of package")
+ issues = append(issues, "module must be a prefix of package")
}
if vuln.Package == "" {
importPath = vuln.Module
} else {
importPath = vuln.Package
}
- if err := checkModVersions(vuln.Module, vuln.Versions); err != nil {
- return err
- }
+ if vuln.Module != "" && importPath != "" {
+ if err := checkModVersions(vuln.Module, vuln.Versions); err != nil {
+ issues = append(issues, err.Error())
+ }
- if err := module.CheckImportPath(importPath); err != nil {
- return err
+ if err := module.CheckImportPath(importPath); err != nil {
+ issues = append(issues, err.Error())
+ }
}
} else if vuln.Package == "" {
- return errors.New("missing package")
+ issues = append(issues, "missing package")
}
for _, additionalPackage := range vuln.AdditionalPackages {
var additionalImportPath string
if additionalPackage.Module == "" {
- return errors.New("missing additional_package.module")
+ issues = append(issues, "missing additional_package.module")
}
if additionalPackage.Package == additionalPackage.Module {
- return errors.New("package is redundant and can be removed")
+ issues = append(issues, "package is redundant and can be removed")
}
if additionalPackage.Package != "" && !strings.HasPrefix(additionalPackage.Package, additionalPackage.Module) {
- return errors.New("additional_package.module must be a prefix of additional_package.package")
+ issues = append(issues, "additional_package.module must be a prefix of additional_package.package")
}
if additionalPackage.Package == "" {
additionalImportPath = additionalPackage.Module
@@ -176,44 +187,44 @@
additionalImportPath = additionalPackage.Package
}
if err := module.CheckImportPath(additionalImportPath); err != nil {
- return err
+ issues = append(issues, err.Error())
}
if !vuln.Stdlib {
if err := checkModVersions(additionalPackage.Module, additionalPackage.Versions); err != nil {
- return err
+ issues = append(issues, err.Error())
}
}
}
if vuln.Description == "" {
- return errors.New("missing description")
+ issues = append(issues, "missing description")
}
if vuln.Published.IsZero() {
- return errors.New("missing published")
+ issues = append(issues, "missing published")
}
if vuln.LastModified != nil && vuln.LastModified.Before(vuln.Published) {
- return errors.New("last_modified is before published")
+ issues = append(issues, "last_modified is before published")
}
if vuln.CVE != "" && vuln.CVEMetadata != nil && vuln.CVEMetadata.ID != "" {
// TODO: may just want to use one of these? :shrug:
- return errors.New("only one of cve and cve_metadata.id should be present")
+ issues = append(issues, "only one of cve and cve_metadata.id should be present")
}
if vuln.CVE != "" && !cveRegex.MatchString(vuln.CVE) {
- return fmt.Errorf("malformed cve: %s", vuln.CVE)
+ issues = append(issues, "malformed cve identifier")
}
if vuln.CVEMetadata != nil {
if vuln.CVEMetadata.ID == "" {
- return errors.New("cve_metadata.id is required")
+ issues = append(issues, "cve_metadata.id is required")
}
if !cveRegex.MatchString(vuln.CVEMetadata.ID) {
- return fmt.Errorf("malformed cve_metadata.id: %s", vuln.CVEMetadata.ID)
+ issues = append(issues, "malformed cve_metadata.id identifier")
}
}
- return nil
+ return issues
}
