diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index 75f04c5b9c..40d4ab783c 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -908,6 +908,68 @@ Scanned /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 /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 /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 /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] --- diff --git a/cmd/osv-scanner/main.go b/cmd/osv-scanner/main.go index 595b1afe49..3c891f338c 100644 --- a/cmd/osv-scanner/main.go +++ b/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 { diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index a8c543ef1c..58530d87c9 100644 --- a/cmd/osv-scanner/main_test.go +++ b/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, }, diff --git a/cmd/osv-scanner/scan/main.go b/cmd/osv-scanner/scan/main.go index ffa5281205..0e455b886d 100644 --- a/cmd/osv-scanner/scan/main.go +++ b/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"), diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index b4345ed292..68f0394513 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/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...) }