internal/vulncheck: add documentation and propagate errors

This CL is part of a series of changes cleaning up the code a bit.

Change-Id: Ica5890efe0ff46d4afe651355baec147d0557cc1
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/538789
Reviewed-by: Maceo Thompson <maceothompson@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

internal/vulncheck/binary.go

@@ -27,13 +27,12 @@ func Binary(ctx context.Context, handler govulncheck.Handler, exe io.ReaderAt, c
 	if err != nil {
 		return err
 	}
-	callstacks := binaryCallstacks(vr)
-	return emitBinaryResult(handler, vr, callstacks)
+	return emitBinaryResult(handler, vr, binaryCallstacks(vr))
 }
 
 // binary detects presence of vulnerable symbols in exe.
 // The Calls, Imports, and Requires fields on Result will be empty.
-func binary(ctx context.Context, handler govulncheck.Handler, exe io.ReaderAt, cfg *govulncheck.Config, client *client.Client) (_ *Result, err error) {
+func binary(ctx context.Context, handler govulncheck.Handler, exe io.ReaderAt, cfg *govulncheck.Config, client *client.Client) (*Result, error) {
 	mods, packageSymbols, bi, err := buildinfo.ExtractPackagesAndSymbols(exe)
 	if err != nil {
 		return nil, fmt.Errorf("could not parse provided binary: %v", err)
@@ -49,7 +48,9 @@ func binary(ctx context.Context, handler govulncheck.Handler, exe io.ReaderAt, c
 	}
 
 	// Emit OSV entries immediately in their raw unfiltered form.
-	emitOSVs(handler, mv)
+	if err := emitOSVs(handler, mv); err != nil {
+		return nil, err
+	}
 
 	modVulns := moduleVulnerabilities(mv)
 	goos := findSetting("GOOS", bi)
@@ -60,7 +61,6 @@ func binary(ctx context.Context, handler govulncheck.Handler, exe io.ReaderAt, c
 
 	modVulns = modVulns.filter(goos, goarch)
 	result := &Result{}
-
 	if packageSymbols == nil {
 		// The binary exe is stripped. We currently cannot detect inlined
 		// symbols for stripped binaries (see #57764), so we report

internal/vulncheck/emit.go

@@ -13,6 +13,7 @@ import (
 	"golang.org/x/vuln/internal/osv"
 )
 
+// emitOSVs emits all OSV vuln entries in modVulns to handler.
 func emitOSVs(handler govulncheck.Handler, modVulns []*ModVulns) error {
 	for _, mv := range modVulns {
 		for _, v := range mv.Vulns {
@@ -24,9 +25,26 @@ func emitOSVs(handler govulncheck.Handler, modVulns []*ModVulns) error {
 	return nil
 }
 
-func emitCalledVulns(handler govulncheck.Handler, callstacks map[*Vuln]CallStack) error {
-	var vulns []*Vuln
+// emitModuleFindings emits module-level findings for vulnerabilities in modVulns.
+func emitModuleFindings(handler govulncheck.Handler, modVulns moduleVulnerabilities) error {
+	for _, vuln := range modVulns {
+		for _, osv := range vuln.Vulns {
+			if err := handler.Finding(&govulncheck.Finding{
+				OSV:          osv.ID,
+				FixedVersion: FixedVersion(modPath(vuln.Module), modVersion(vuln.Module), osv.Affected),
+				Trace:        []*govulncheck.Frame{frameFromModule(vuln.Module, osv.Affected)},
+			}); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
 
+// emitCallFindings emits call-level findings for vulnerabilities
+// that have a call stack in callstacks.
+func emitCallFindings(handler govulncheck.Handler, callstacks map[*Vuln]CallStack) error {
+	var vulns []*Vuln
 	for v := range callstacks {
 		vulns = append(vulns, v)
 	}
@@ -52,21 +70,6 @@ func emitCalledVulns(handler govulncheck.Handler, callstacks map[*Vuln]CallStack
 	return nil
 }
 
-func emitModuleFindings(handler govulncheck.Handler, modVulns moduleVulnerabilities) error {
-	for _, vuln := range modVulns {
-		for _, osv := range vuln.Vulns {
-			if err := handler.Finding(&govulncheck.Finding{
-				OSV:          osv.ID,
-				FixedVersion: FixedVersion(modPath(vuln.Module), modVersion(vuln.Module), osv.Affected),
-				Trace:        []*govulncheck.Frame{frameFromModule(vuln.Module, osv.Affected)},
-			}); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 func emitPackageFinding(handler govulncheck.Handler, vuln *Vuln) error {
 	return handler.Finding(&govulncheck.Finding{
 		OSV:          vuln.OSV.ID,

internal/vulncheck/source.go

@@ -22,11 +22,15 @@ func Source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
 	if err != nil {
 		return err
 	}
-	callStacks := sourceCallstacks(vr)
-	return emitCalledVulns(handler, callStacks)
+
+	if cfg.ScanLevel.WantSymbols() {
+		return emitCallFindings(handler, sourceCallstacks(vr))
+	}
+	return nil
 }
 
-// source detects vulnerabilities in packages. The result will contain:
+// source detects vulnerabilities in packages. It emits findings to handler
+// and produces a Result that will contain:
 //
 // 1) An ImportGraph related to an import of a package with some known
 // vulnerabilities.
@@ -37,7 +41,7 @@ func Source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
 // 3) A CallGraph leading to the use of a known vulnerable function or method.
 //
 // Assumes that pkgs are non-empty and belong to the same program.
-func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) (_ *Result, err error) {
+func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.Package, cfg *govulncheck.Config, client *client.Client, graph *PackageGraph) (*Result, error) {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
@@ -71,14 +75,17 @@ func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
 	}
 
 	// Emit OSV entries immediately in their raw unfiltered form.
-	emitOSVs(handler, mv)
+	if err := emitOSVs(handler, mv); err != nil {
+		return nil, err
+	}
 
 	modVulns := moduleVulnerabilities(mv)
 	modVulns = modVulns.filter("", "")
-	result := &Result{}
-	// instead of add to result, output using the handler
-	emitModuleFindings(handler, modVulns)
+	if err := emitModuleFindings(handler, modVulns); err != nil {
+		return nil, err
+	}
 
+	result := &Result{}
 	if !cfg.ScanLevel.WantPackages() || len(modVulns) == 0 {
 		return result, nil
 	}
@@ -96,7 +103,6 @@ func source(ctx context.Context, handler govulncheck.Handler, pkgs []*packages.P
 	}
 
 	vulnCallGraphSlice(entries, modVulns, cg, result, graph)
-
 	return result, nil
 }