feat: Update docker container scanning flag (#1350)

Resolves #1316 

Update the --docker flag to:
- Only accept one image to scan at a time (to make displaying results
easier)
- Call new image scanning function internally.
- Acts like a convenience function for 
```
docker save <image-name> > img-name.tar && osv-scanner --experimental-oci-image=img.name.tar
```

TODO: 
- [x] Add an ACCEPTANCE test which uses docker to pull down a stable
image.
- [x] Include a docker pull first, as docker save only saves images
already on device and does not pull images online.

cmd/osv-scanner/__snapshots__/main_test.snap

@@ -908,6 +908,68 @@ Scanned <rootdir>/fixtures/call-analysis-go-project/go.mod file and found 4 pack
 
 ---
 
+[TestRun_Docker/Fake_alpine_image - 1]
+Pulling docker image ("alpine:non-existent-tag")...
+
+---
+
+[TestRun_Docker/Fake_alpine_image - 2]
+Docker command exited with code ("/usr/bin/docker pull -q alpine:non-existent-tag"): 1
+STDERR:
+> Error response from daemon: manifest for alpine:non-existent-tag not found: manifest unknown: manifest unknown
+failed to run docker command
+
+---
+
+[TestRun_Docker/Fake_image_entirely - 1]
+Pulling docker image ("this-image-definitely-does-not-exist-abcde")...
+
+---
+
+[TestRun_Docker/Fake_image_entirely - 2]
+Docker command exited with code ("/usr/bin/docker pull -q this-image-definitely-does-not-exist-abcde"): 1
+STDERR:
+> Error response from daemon: pull access denied for this-image-definitely-does-not-exist-abcde, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
+failed to run docker command
+
+---
+
+[TestRun_Docker/Real_Alpine_image - 1]
+Pulling docker image ("alpine:3.18.9")...
+Saving docker image ("alpine:3.18.9") to temporary file...
+Scanning image...
+No issues found
+
+---
+
+[TestRun_Docker/Real_Alpine_image - 2]
+
+---
+
+[TestRun_Docker/Real_empty_image - 1]
+Pulling docker image ("hello-world")...
+Saving docker image ("hello-world") to temporary file...
+Scanning image...
+
+---
+
+[TestRun_Docker/Real_empty_image - 2]
+No package sources found, --help for usage information.
+
+---
+
+[TestRun_Docker/Real_empty_image_with_tag - 1]
+Pulling docker image ("hello-world:linux")...
+Saving docker image ("hello-world:linux") to temporary file...
+Scanning image...
+
+---
+
+[TestRun_Docker/Real_empty_image_with_tag - 2]
+No package sources found, --help for usage information.
+
+---
+
 [TestRun_GithubActions/scanning_osv-scanner_custom_format - 1]
 Scanned <rootdir>/fixtures/locks-insecure/osv-scanner-flutter-deps.json file as a osv-scanner and found 3 packages
 +--------------------------------+------+-----------+----------------------------+----------------------------+-------------------------------------------------------+
@@ -2395,7 +2457,7 @@ No issues found
 
 ---
 
-[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_datda_source - 1]
+[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_data_source - 1]
 Scanned <rootdir>/fixtures/maven-transitive/registry.xml file as a pom.xml and found 59 packages
 +-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+
 | OSV URL                             | CVSS | ECOSYSTEM | PACKAGE                                       | VERSION | SOURCE                                 |
@@ -2409,7 +2471,7 @@ Scanned <rootdir>/fixtures/maven-transitive/registry.xml file as a pom.xml and f
 
 ---
 
-[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_datda_source - 2]
+[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_data_source - 2]
 
 ---
 

cmd/osv-scanner/main.go

@@ -47,6 +47,18 @@ func run(args []string, stdout, stderr io.Writer) int {
 		},
 	}
 
+	// If ExitErrHandler is not set, cli will use the default cli.HandleExitCoder.
+	// This is not ideal as cli.HandleExitCoder checks if the error implements cli.ExitCode interface.
+	//
+	// 99% of the time, this is fine, as we do not implement cli.ExitCode in our errors, so errors pass through
+	// that handler untouched.
+	// However, because of Go's duck typing, any error that happens to have a ExitCode() function
+	// (e.g. *exec.ExitError) will be assumed to implement cli.ExitCode interface and cause the program to exit
+	// early without proper error handling.
+	//
+	// This removes the handler entirely so that behavior will not unexpectedly happen.
+	app.ExitErrHandler = func(_ *cli.Context, _ error) {}
+
 	args = insertDefaultCommand(args, app.Commands, app.DefaultCommand, stdout, stderr)
 
 	if err := app.Run(args); err != nil {

cmd/osv-scanner/main_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -728,6 +729,51 @@ func TestRun_Licenses(t *testing.T) {
 	}
 }
 
+func TestRun_Docker(t *testing.T) {
+	t.Parallel()
+
+	testutility.SkipIfNotAcceptanceTesting(t, "Takes a long time to pull down images")
+
+	tests := []cliTestCase{
+		{
+			name: "Fake alpine image",
+			args: []string{"", "--docker", "alpine:non-existent-tag"},
+			exit: 127,
+		},
+		{
+			name: "Fake image entirely",
+			args: []string{"", "--docker", "this-image-definitely-does-not-exist-abcde"},
+			exit: 127,
+		},
+		// TODO: How to prevent these snapshots from changing constantly
+		{
+			name: "Real empty image",
+			args: []string{"", "--docker", "hello-world"},
+			exit: 128, // No packages found
+		},
+		{
+			name: "Real empty image with tag",
+			args: []string{"", "--docker", "hello-world:linux"},
+			exit: 128, // No package found
+		},
+		{
+			name: "Real Alpine image",
+			args: []string{"", "--docker", "alpine:3.18.9"},
+			exit: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Only test on linux, and mac/windows CI/CD does not come with docker preinstalled
+			if runtime.GOOS == "linux" {
+				testCli(t, tt)
+			}
+		})
+	}
+}
+
 func TestRun_OCIImage(t *testing.T) {
 	t.Parallel()
 
@@ -923,7 +969,7 @@ func TestRun_MavenTransitive(t *testing.T) {
 			exit: 1,
 		},
 		{
-			name: "resolve transitive dependencies with native datda source",
+			name: "resolve transitive dependencies with native data source",
 			args: []string{"", "--config=./fixtures/osv-scanner-empty-config.toml", "--experimental-resolution-data-source=native", "-L", "pom.xml:./fixtures/maven-transitive/registry.xml"},
 			exit: 1,
 		},

cmd/osv-scanner/scan/main.go

@@ -31,10 +31,10 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
 		Usage:       "scans various mediums for dependencies and matches it against the OSV database",
 		Description: "scans various mediums for dependencies and matches it against the OSV database",
 		Flags: []cli.Flag{
-			&cli.StringSliceFlag{
+			&cli.StringFlag{
 				Name:      "docker",
 				Aliases:   []string{"D"},
-				Usage:     "scan docker image with this name. Warning: Only run this on a trusted container image, as it runs the container image to retrieve the package versions",
+				Usage:     "scan docker image with this name. This is a convenience function which runs `docker save` before scanning the saved image using --oci-image",
 				TakesFile: false,
 			},
 			&cli.StringSliceFlag{
@@ -258,15 +258,15 @@ func action(context *cli.Context, stdout, stderr io.Writer) (reporter.Reporter,
 	}
 
 	vulnResult, err := osvscanner.DoScan(osvscanner.ScannerActions{
-		LockfilePaths:        context.StringSlice("lockfile"),
-		SBOMPaths:            context.StringSlice("sbom"),
-		DockerContainerNames: context.StringSlice("docker"),
-		Recursive:            context.Bool("recursive"),
-		SkipGit:              context.Bool("skip-git"),
-		NoIgnore:             context.Bool("no-ignore"),
-		ConfigOverridePath:   context.String("config"),
-		DirectoryPaths:       context.Args().Slice(),
-		CallAnalysisStates:   callAnalysisStates,
+		LockfilePaths:      context.StringSlice("lockfile"),
+		SBOMPaths:          context.StringSlice("sbom"),
+		DockerImageName:    context.String("docker"),
+		Recursive:          context.Bool("recursive"),
+		SkipGit:            context.Bool("skip-git"),
+		NoIgnore:           context.Bool("no-ignore"),
+		ConfigOverridePath: context.String("config"),
+		DirectoryPaths:     context.Args().Slice(),
+		CallAnalysisStates: callAnalysisStates,
 		ExperimentalScannerActions: osvscanner.ExperimentalScannerActions{
 			LocalDBPath:       context.String("experimental-local-db-path"),
 			DownloadDatabases: context.Bool("experimental-download-offline-databases"),

pkg/osvscanner/osvscanner.go

@@ -37,16 +37,16 @@ import (
 )
 
 type ScannerActions struct {
-	LockfilePaths        []string
-	SBOMPaths            []string
-	DirectoryPaths       []string
-	GitCommits           []string
-	Recursive            bool
-	SkipGit              bool
-	NoIgnore             bool
-	DockerContainerNames []string
-	ConfigOverridePath   string
-	CallAnalysisStates   map[string]bool
+	LockfilePaths      []string
+	SBOMPaths          []string
+	DirectoryPaths     []string
+	GitCommits         []string
+	Recursive          bool
+	SkipGit            bool
+	NoIgnore           bool
+	DockerImageName    string
+	ConfigOverridePath string
+	CallAnalysisStates map[string]bool
 
 	ExperimentalScannerActions
 }
@@ -644,72 +644,77 @@ func createCommitQueryPackage(commit string, source string) scannedPackage {
 	}
 }
 
-func scanDebianDocker(r reporter.Reporter, dockerImageName string) ([]scannedPackage, error) {
-	cmd := exec.Command("docker", "run", "--rm", "--entrypoint", "/usr/bin/dpkg-query", dockerImageName, "-f", "${Package}###${Version}\\n", "-W")
-	stdout, err := cmd.StdoutPipe()
+func runCommandLogError(r reporter.Reporter, name string, args ...string) error {
+	cmd := exec.Command(name, args...)
 
+	// Get stderr for debugging when docker fails
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		r.Errorf("Failed to get stderr: %s\n", err)
+		return err
+	}
+
+	err = cmd.Start()
+	if err != nil {
+		r.Errorf("Failed to run docker command (%q): %s\n", cmd.String(), err)
+		return err
+	}
+	// This has to be captured before cmd.Wait() is called, as cmd.Wait() closes the stderr pipe.
+	var stderrLines []string
+	scanner := bufio.NewScanner(stderr)
+	for scanner.Scan() {
+		stderrLines = append(stderrLines, scanner.Text())
+	}
+
+	err = cmd.Wait()
 	if err != nil {
-		r.Errorf("Failed to get stdout: %s\n", err)
+		r.Errorf("Docker command exited with code (%q): %d\nSTDERR:\n", cmd.String(), cmd.ProcessState.ExitCode())
+		for _, line := range stderrLines {
+			r.Errorf("> %s\n", line)
+		}
+
+		return errors.New("failed to run docker command")
+	}
+
+	return nil
+}
+
+func scanDockerImage(r reporter.Reporter, dockerImageName string) ([]scannedPackage, error) {
+	tempImageFile, err := os.CreateTemp("", "docker-image-*.tar")
+	if err != nil {
+		r.Errorf("Failed to create temporary file: %s\n", err)
 		return nil, err
 	}
-	stderr, err := cmd.StderrPipe()
 
+	err = tempImageFile.Close()
 	if err != nil {
-		r.Errorf("Failed to get stderr: %s\n", err)
 		return nil, err
 	}
+	defer os.Remove(tempImageFile.Name())
 
-	err = cmd.Start()
+	r.Infof("Pulling docker image (%q)...\n", dockerImageName)
+	err = runCommandLogError(r, "docker", "pull", "-q", dockerImageName)
 	if err != nil {
-		r.Errorf("Failed to start docker image: %s\n", err)
 		return nil, err
 	}
-	defer func() {
-		var stderrlines []string
 
-		scanner := bufio.NewScanner(stderr)
-		for scanner.Scan() {
-			stderrlines = append(stderrlines, scanner.Text())
-		}
+	r.Infof("Saving docker image (%q) to temporary file...\n", dockerImageName)
+	err = runCommandLogError(r, "docker", "save", "-o", tempImageFile.Name(), dockerImageName)
+	if err != nil {
+		return nil, err
+	}
 
-		err := cmd.Wait()
-		if err != nil {
-			r.Errorf("Docker command exited with code %d\n", cmd.ProcessState.ExitCode())
-			for _, line := range stderrlines {
-				r.Errorf("> %s\n", line)
-			}
-		}
-	}()
+	r.Infof("Scanning image...\n")
+	packages, err := scanImage(r, tempImageFile.Name())
+	if err != nil {
+		return nil, err
+	}
 
-	scanner := bufio.NewScanner(stdout)
-	var packages []scannedPackage
-	for scanner.Scan() {
-		text := scanner.Text()
-		text = strings.TrimSpace(text)
-		if len(text) == 0 {
-			continue
-		}
-		splitText := strings.Split(text, "###")
-		if len(splitText) != 2 {
-			r.Errorf("Unexpected output from Debian container: \n\n%s\n", text)
-			return nil, fmt.Errorf("unexpected output from Debian container: \n\n%s", text)
-		}
-		// TODO(rexpan): Get and specify exact debian release version
-		packages = append(packages, scannedPackage{
-			Name:      splitText[0],
-			Version:   splitText[1],
-			Ecosystem: "Debian",
-			Source: models.SourceInfo{
-				Path: dockerImageName,
-				Type: "docker",
-			},
-		})
+	// Modify the image path to be the image name, rather than the temporary file name
+	for i := range packages {
+		_, internalPath, _ := strings.Cut(packages[i].Source.Path, ":")
+		packages[i].Source.Path = dockerImageName + ":" + internalPath
 	}
-	r.Infof(
-		"Scanned docker image with %d %s\n",
-		len(packages),
-		output.Form(len(packages), "package", "packages"),
-	)
 
 	return packages, nil
 }
@@ -895,9 +900,11 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe
 		scannedPackages = append(scannedPackages, pkgs...)
 	}
 
-	// TODO: Deprecated
-	for _, container := range actions.DockerContainerNames {
-		pkgs, _ := scanDebianDocker(r, container)
+	if actions.DockerImageName != "" {
+		pkgs, err := scanDockerImage(r, actions.DockerImageName)
+		if err != nil {
+			return models.VulnerabilityResults{}, err
+		}
 		scannedPackages = append(scannedPackages, pkgs...)
 	}