x/vulndb: add tests for all top-level lint errors
Adds tests (and makes minor behavior fixes) for all lint errors emitted
directly by the Lint function.
Behavior fixes:
- getModVersions and getCanonicalModName now check status of http.Get
call
- lint check on CVE metadata now only checks for malformed ID if no ID is present
For #52945
Change-Id: Idd291d86beda8691dd7b8404c783364e839109fb
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/406197
Run-TryBot: Tatiana Bradley <tatiana@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/report/lint.go b/internal/report/lint.go
index cc37654..a609dbf 100644
--- a/internal/report/lint.go
+++ b/internal/report/lint.go
@@ -40,6 +40,8 @@
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/list", proxyURL, escaped))
if err != nil {
return nil, err
+ } else if resp.StatusCode != 200 {
+ return nil, fmt.Errorf("http.Get returned status %v", resp.Status)
}
defer resp.Body.Close()
b, err := io.ReadAll(resp.Body)
@@ -66,6 +68,8 @@
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/%s.mod", proxyURL, escapedPath, escapedVersion))
if err != nil {
return "", err
+ } else if resp.StatusCode != 200 {
+ return "", fmt.Errorf("http.Get returned status %v", resp.Status)
}
defer resp.Body.Close()
b, err := io.ReadAll(resp.Body)
@@ -254,8 +258,7 @@
if r.CVEMetadata != nil {
if r.CVEMetadata.ID == "" {
addIssue("cve_metadata.id is required")
- }
- if !cveRegex.MatchString(r.CVEMetadata.ID) {
+ } else if !cveRegex.MatchString(r.CVEMetadata.ID) {
addIssue("malformed cve_metadata.id identifier")
}
}
diff --git a/internal/report/lint_test.go b/internal/report/lint_test.go
index 43a858d..24f714e 100644
--- a/internal/report/lint_test.go
+++ b/internal/report/lint_test.go
@@ -8,39 +8,233 @@
"bytes"
"strings"
"testing"
+ "time"
)
+// TODO: Add tests for helper functions that call the proxy.
+
func TestLint(t *testing.T) {
+ jan2000 := time.Date(2000, time.January, 0, 0, 0, 0, 0, time.UTC)
+ jan2022 := time.Date(2022, time.January, 0, 0, 0, 0, 0, time.UTC)
for _, test := range []struct {
+ desc string
report Report
want []string
- }{{
- report: Report{
- Packages: []Package{{
- Module: "std",
- Package: "time",
- Versions: []VersionRange{{
- Fixed: "1.2.1",
- }, {
- Fixed: "1.3.2",
- }},
- }},
+ }{
+ {
+ desc: "no packages",
+ report: Report{
+ Description: "description",
+ },
+ want: []string{"no packages"},
},
- want: []string{"version ranges overlap"},
- }, {
- report: Report{
- Packages: []Package{{
- Module: "std",
- Package: "time",
- Versions: []VersionRange{{
- Introduced: "1.3",
- Fixed: "1.2.1",
+ {
+ desc: "missing module",
+ report: Report{
+ Packages: []Package{{
+ // no module
+ Package: "test.com/a/package",
}},
- }},
+ Description: "description",
+ },
+ want: []string{"missing module"},
},
- want: []string{`version "1.3" >= "1.2.1"`},
- }} {
+ {
+ desc: "missing description",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ // no description
+ },
+ want: []string{"missing description"},
+ },
+ {
+ desc: "third party: redundant module and package",
+ report: Report{
+ Packages: []Package{{
+ Module: "github.com/golang/vulndb",
+ Package: "github.com/golang/vulndb",
+ }},
+ Description: "description",
+ },
+ want: []string{"package is redundant and can be removed"},
+ },
+ {
+ desc: "third party: module is not a prefix of package",
+ report: Report{
+ Packages: []Package{{
+ Module: "github.com/golang/vulndb",
+ Package: "github.com/golang/crypto",
+ }},
+ Description: "description",
+ },
+ want: []string{"module must be a prefix of package"},
+ },
+ {
+ desc: "third party: invalid import path",
+ report: Report{
+ Packages: []Package{{
+ Module: "invalid.",
+ Versions: []VersionRange{{
+ Fixed: "1.2.1",
+ }},
+ }},
+ Description: "description",
+ },
+ want: []string{"malformed import path",
+ "unable to retrieve module versions from proxy"},
+ },
+ {
+ desc: "standard library: missing package",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ // no package
+ }},
+ Description: "description",
+ },
+ want: []string{"missing package"},
+ },
+ {
+ desc: "overlapping version ranges",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ Versions: []VersionRange{{
+ Fixed: "1.2.1",
+ }, {
+ Fixed: "1.3.2",
+ }},
+ }},
+ Description: "description",
+ },
+ want: []string{"version ranges overlap"},
+ },
+ {
+ desc: "fixed before introduced",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ Versions: []VersionRange{{
+ Introduced: "1.3",
+ Fixed: "1.2.1",
+ }},
+ }},
+ Description: "description",
+ },
+ want: []string{`version "1.3" >= "1.2.1"`},
+ },
+ {
+ desc: "invalid semantic version",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ Versions: []VersionRange{{
+ Introduced: "1.3.X",
+ }},
+ }},
+ Description: "description",
+ },
+ want: []string{`invalid semantic version: "1.3.X"`},
+ },
+ {
+ desc: "last modified before published",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ LastModified: &jan2000,
+ Published: jan2022,
+ },
+ want: []string{"last_modified is before published"},
+ },
+ {
+ desc: "bad cve identifier",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ CVEs: []string{"CVE.123.456"},
+ },
+ want: []string{"malformed cve identifier"},
+ },
+ {
+ desc: "cve and cve metadata both present",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ CVEs: []string{"CVE-2022-12345"},
+ CVEMetadata: &CVEMeta{
+ ID: "CVE-2022-23456",
+ },
+ },
+ want: []string{"only one of cve and cve_metadata.id should be present"},
+ },
+ {
+ desc: "missing cve metadata id",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ CVEMetadata: &CVEMeta{
+ // no id
+ },
+ },
+ want: []string{"cve_metadata.id is required"},
+ },
+ {
+ desc: "bad cve metadata id",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ CVEMetadata: &CVEMeta{
+ ID: "CVE.2022.00000",
+ },
+ },
+ want: []string{"malformed cve_metadata.id identifier"},
+ },
+ {
+ desc: "unfixed links",
+ report: Report{
+ Packages: []Package{{
+ Module: "std",
+ Package: "time",
+ }},
+ Description: "description",
+ Links: Links{
+ Commit: "https://github.com/golang/go/commit/123",
+ Context: []string{
+ "https://github.com/golang/go/issues/123",
+ "https://golang.org/xxx",
+ "https://groups.google.com/forum/#!/golang-announce/123/1/"},
+ },
+ },
+ want: []string{
+ `"https://github.com/golang/go/issues/123" should be "https://go.dev/issue/123"`,
+ `"https://golang.org/xxx" should be "https://go.dev/xxx"`,
+ `"https://github.com/golang/go/commit/123" should be "https://go.googlesource.com/+/123"`,
+ `"https://groups.google.com/forum/#!/golang-announce/123/1/" should be "https://groups.google.com/g/golang-announce/c/123/m/1/"`},
+ },
+ } {
got := test.report.Lint()
+
var missing []string
for _, w := range test.want {
found := false
@@ -59,13 +253,49 @@
if err := test.report.encode(&buf); err != nil {
t.Error(err)
}
- t.Errorf("missing expected lint warnings in report:\n%v", buf.String())
+ t.Errorf("TestLint(%q): missing expected lint errors in report:\n"+
+ "%v\n"+
+ "got: %q\n"+
+ "want: %q\n", test.desc, buf.String(), got, missing)
+ }
+
+ // Check for unexpected lint errors if there are no missing ones.
+ if len(missing) == 0 {
+ var unexpected []string
for _, g := range got {
- t.Errorf(" got: %v", g)
+ found := false
+ for _, w := range test.want {
+ if strings.Contains(g, w) {
+ found = true
+ continue
+ }
+ }
+ if !found {
+ unexpected = append(unexpected, g)
+ }
}
- for _, w := range missing {
- t.Errorf(" want: %v", w)
+ if len(unexpected) > 0 {
+ var buf bytes.Buffer
+ if err := test.report.encode(&buf); err != nil {
+ t.Error(err)
+ }
+ t.Errorf("TestLint(%q): unexpected lint errors in report:\n"+
+ "%v\n"+
+ "got: %q\n", test.desc, buf.String(), unexpected)
}
}
}
}
+
+func TestLintFile(t *testing.T) {
+ f := "testdata/report.yaml"
+ lintErrs, err := LintFile(f)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(lintErrs) > 0 {
+ t.Errorf("unexpected lint errors for %q:\n"+
+ "got: %q\n"+
+ "want: []", f, lintErrs)
+ }
+}
diff --git a/internal/report/testdata/report.yaml b/internal/report/testdata/report.yaml
index 977bc36..122b6b1 100644
--- a/internal/report/testdata/report.yaml
+++ b/internal/report/testdata/report.yaml
@@ -3,7 +3,7 @@
symbols:
- defaultLogFormatter
versions:
- - fixed: v1.6.0
+ - fixed: 1.6.0
description: |
The default Formatter for the Logger middleware (LoggerConfig.Formatter),
which is included in the Default engine, allows attackers to inject arbitrary
