refactor(osv-linter): tidy up human output

Add a `--verbose` flag to tone down the output Clean up how the findings are presented Discovered the Alpine ecosystem was being overlooked in error Signed-off-by: Andrew Pollock <[email protected]>
ossf · Sep 4, 2024 · fb09e0e · fb09e0e
1 parent 285be57
commit fb09e0e
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 20 deletions.
diff --git a/tools/osv-linter/cmd/osv/main.go b/tools/osv-linter/cmd/osv/main.go
@@ -20,6 +20,10 @@ func main() {
 					{
 						Name: "lint",
 						Flags: []cli.Flag{
+							&cli.BoolFlag{
+								Name: "verbose",
+								Usage: "verbose output",
+							},
 							&cli.StringFlag{
 								Name:  "collection",
 								Value: "ALL",

diff --git a/tools/osv-linter/internal/checks/checks.go b/tools/osv-linter/internal/checks/checks.go
@@ -23,7 +23,10 @@ type CheckError struct {
 
 // Error returns the error message, including the code.
 func (ce *CheckError) Error() string {
-	return fmt.Sprintf("%s: %s", ce.Code, ce.Message)
+	if ce.Code == "" {
+		return fmt.Sprintf("%s", ce.Message)
+	}
+	return fmt.Sprintf("[%s]: %s", ce.Code, ce.Message)
 }
 
 // CheckDef defines a single check.
@@ -34,14 +37,19 @@ type CheckDef struct {
 	Check       Check
 }
 
+// CheckConfig defines the configuration for a check.
+type CheckConfig struct {
+	Verbose bool
+}
+
 // Check defines how to run the check.
-type Check func(*gjson.Result) []CheckError
+type Check func(*gjson.Result, *CheckConfig) []CheckError
 
 // Run runs the check, returning any findings.
 // The check has no awareness of the check's Code,
 // this merges that with the check's findings.
-func (c *CheckDef) Run(json *gjson.Result) (findings []CheckError) {
-	for _, finding := range c.Check(json) {
+func (c *CheckDef) Run(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
+	for _, finding := range c.Check(json, config) {
 		findings = append(findings, CheckError{
 			Code:    c.Code,
 			Message: finding.Error(),

diff --git a/tools/osv-linter/internal/checks/packages.go b/tools/osv-linter/internal/checks/packages.go
@@ -22,7 +22,7 @@ type Package struct {
 }
 
 // PackageExists checks the package exists in the registry for that ecosystem.
-func PackageExists(json *gjson.Result) (findings []CheckError) {
+func PackageExists(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
 	affectedEntries := json.Get("affected")
 
 	knownExistent := make(map[Package]bool)
@@ -68,7 +68,7 @@ var CheckPackageVersionsExist = &CheckDef{
 }
 
 // PackageVersionsExist checks the package versions exist in the registry for that ecosystem.
-func PackageVersionsExist(json *gjson.Result) (findings []CheckError) {
+func PackageVersionsExist(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
 	affectedEntries := json.Get("affected")
 
 	// Examine each affected entry:
@@ -135,7 +135,7 @@ var CheckPackagePurlValid = &CheckDef{
 }
 
 // PackagePurlValid checks the package purls validate.
-func PackagePurlValid(json *gjson.Result) (findings []CheckError) {
+func PackagePurlValid(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
 	affectedEntries := json.Get("affected")
 
 	// Examine each affected entry:

diff --git a/tools/osv-linter/internal/checks/ranges.go b/tools/osv-linter/internal/checks/ranges.go
@@ -15,7 +15,7 @@ var CheckRangeHasIntroducedEvent = &CheckDef{
 }
 
 // RangeHasIntroducedEvent checks for missing 'introduced' objects in events.
-func RangeHasIntroducedEvent(json *gjson.Result) (findings []CheckError) {
+func RangeHasIntroducedEvent(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
 	// It is valid to not have any ranges.
 	ranges := json.Get("affected.#(ranges)")
 	if !ranges.Exists() {
@@ -41,7 +41,7 @@ var CheckRangeIsDistinct = &CheckDef{
 
 // RangeIsDistinct checks that the introduced and fixed (or last_affected) values differ.
 // (on a per-repo basis for GIT ranges, and on a per-package basis otherwise)
-func RangeIsDistinct(json *gjson.Result) (findings []CheckError) {
+func RangeIsDistinct(json *gjson.Result, config *CheckConfig) (findings []CheckError) {
 	affectedEntries := json.Get("affected")
 
 	// Examine each entry:

diff --git a/tools/osv-linter/internal/linter.go b/tools/osv-linter/internal/linter.go
@@ -24,18 +24,25 @@ type Content struct {
 	bytes    []byte
 }
 
-func lint(content *Content, checks []*checks.CheckDef) (findings []checks.CheckError) {
+type LintConfig struct {
+	verbose bool
+}
+
+func lint(content *Content, checksDefined []*checks.CheckDef, config *LintConfig) (findings []checks.CheckError) {
 	// Parse file into JSON
 	if !gjson.ValidBytes(content.bytes) {
 		log.Printf("%q: invalid JSON", content.filename)
 	}
 
 	record := gjson.ParseBytes(content.bytes)
 
-	for _, check := range checks {
-		fmt.Printf("Running %q check on %q\n", check.Name, content.filename)
-		checkFindings := check.Run(&record)
-		if checkFindings != nil {
+	for _, check := range checksDefined {
+		if config.verbose {
+			fmt.Printf("Running %q check on %q\n", check.Name, content.filename)
+		}
+		checkConfig := checks.CheckConfig{Verbose: config.verbose}
+		checkFindings := check.Run(&record, &checkConfig)
+		if checkFindings != nil && config.verbose {
 			log.Printf("%q: %q: %#v", content.filename, check.Name, checkFindings)
 		}
 		findings = append(findings, checkFindings...)
@@ -84,10 +91,12 @@ func LintCommand(cCtx *cli.Context) error {
 
 	// Run all the checks in a collection, if no specific checks requested.
 	if checksToBeRun == nil && cCtx.String("collection") != "" {
-		if cCtx.Args().Present() {
-			fmt.Printf("Running %q check collection on %q\n", cCtx.String("collection"), cCtx.Args())
-		} else {
-			fmt.Printf("Running %q check collection on <stdin>\n", cCtx.String("collection"))
+		if cCtx.Bool("verbose") {
+			if cCtx.Args().Present() {
+				fmt.Printf("Running %q check collection on %q\n", cCtx.String("collection"), cCtx.Args())
+			} else {
+				fmt.Printf("Running %q check collection on <stdin>\n", cCtx.String("collection"))
+			}
 		}
 		// Check the requested check collection exists.
 		collection := checks.CollectionFromName(cCtx.String("collection"))
@@ -135,7 +144,9 @@ func LintCommand(cCtx *cli.Context) error {
 				log.Printf("%v, skipping", err)
 				continue
 			}
-			log.Printf("Found %d files in %q", len(filesToCheck), thingToCheck)
+			if cCtx.Bool("verbose") {
+				log.Printf("Found %d files in %q", len(filesToCheck), thingToCheck)
+			}
 		} else {
 			filesToCheck = append(filesToCheck, thingToCheck)
 		}
@@ -160,13 +171,19 @@ func LintCommand(cCtx *cli.Context) error {
 			log.Printf("%v, skipping", err)
 			continue
 		}
-		findings := lint(&Content{filename: fileToCheck, bytes: recordBytes}, checksToBeRun)
+		findings := lint(&Content{filename: fileToCheck, bytes: recordBytes}, checksToBeRun, &LintConfig{verbose: cCtx.Bool("verbose")})
 		if findings != nil {
 			perFileFindings[fileToCheck] = findings
 		}
 	}
 
 	if len(perFileFindings) > 0 {
+		for filename, findings := range perFileFindings {
+			fmt.Printf("%s:\n", filename)
+			for _, finding := range findings {
+				fmt.Printf("\t * %s\n", finding.Error())
+			}
+		}
 		return errors.New("found errors")
 	}
 	return nil

diff --git a/tools/osv-linter/internal/pkgchecker/ecosystems.go b/tools/osv-linter/internal/pkgchecker/ecosystems.go
@@ -21,6 +21,8 @@ import (
 // Dispatcher for ecosystem-specific package existence checking.
 func ExistsInEcosystem(pkg string, ecosystem string) bool {
 	switch ecosystem {
+	case "Alpine":
+		return true
 	case "AlmaLinux":
 		return true
 	case "Android":
@@ -82,6 +84,8 @@ func ExistsInEcosystem(pkg string, ecosystem string) bool {
 // Dispatcher for ecosystem-specific package version existence checking.
 func VersionsExistInEcosystem(pkg string, versions []string, ecosystem string) error {
 	switch ecosystem {
+	case "Alpine":
+		return nil
 	case "AlmaLinux":
 		return nil
 	case "Android":