Merge pull request #414 from anchore/fix-leaky-fds

Close all ReadClosers explicitly retrieved from resolvers

.github/workflows/release.yaml

@@ -31,7 +31,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Static-Analysis"
+          checkName: "Static analysis"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Check unit test results
@@ -40,7 +40,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Unit-Test"
+          checkName: "Unit tests"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Check integration test results
@@ -49,7 +49,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Integration-Test"
+          checkName: "Integration tests"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Check acceptance test results (linux)
@@ -58,7 +58,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Acceptance-Linux"
+          checkName: "Acceptance tests (Linux)"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Check acceptance test results (mac)
@@ -67,7 +67,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Acceptance-Mac"
+          checkName: "Acceptance tests (Mac)"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Check cli test results (linux)
@@ -76,7 +76,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           # This check name is defined as the github action job name (in .github/workflows/testing.yaml)
-          checkName: "Cli-Linux"
+          checkName: "CLI tests (Linux)"
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Quality gate

.github/workflows/validations.yaml

@@ -9,8 +9,8 @@ env:
 
 jobs:
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Static-Analysis:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "Static analysis"
     runs-on: ubuntu-20.04
     # run only on push event (internal PRs) or on a pull_request event that is from a fork (external PR)
@@ -49,8 +49,8 @@ jobs:
       - name: Run static analysis
         run: make static-analysis
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Unit-Test:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "Unit tests"
     runs-on: ubuntu-20.04
     # run only on push event (internal PRs) or on a pull_request event that is from a fork (external PR)
@@ -104,8 +104,8 @@ jobs:
           name: unit-test-results
           path: test/results/**/*
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Integration-Test:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "Integration tests"
     runs-on: ubuntu-20.04
     # run only on push event (internal PRs) or on a pull_request event that is from a fork (external PR)
@@ -272,8 +272,8 @@ jobs:
           name: artifacts
           path: snapshot/**/*
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Acceptance-Linux:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "Acceptance tests (Linux)"
     needs: [Build-Snapshot-Artifacts]
     runs-on: ubuntu-20.04
@@ -291,8 +291,8 @@ jobs:
       - name: Run Acceptance Tests (Linux)
         run: make acceptance-linux
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Acceptance-Mac:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "Acceptance tests (Mac)"
     needs: [Build-Snapshot-Artifacts]
     runs-on: macos-latest
@@ -317,8 +317,8 @@ jobs:
       - name: Run Acceptance Tests (Mac)
         run: make acceptance-mac
 
-  # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
   Cli-Linux:
+    # Note: changing this job name requires making the same update in the .github/workflows/release.yaml pipeline
     name: "CLI tests (Linux)"
     needs: [Build-Snapshot-Artifacts]
     runs-on: ubuntu-20.04

README.md

@@ -6,7 +6,7 @@
 [![License: Apache-2.0](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://github.com/anchore/syft/blob/main/LICENSE)
 [![GitHub go.mod Go version](https://img.shields.io/github/go-mod/go-version/anchore/syft.svg)](https://github.com/anchore/syft)
 
-A CLI tool and go library for generating a Software Bill of Materials (SBOM) from container images and filesystems.
+A CLI tool and go library for generating a Software Bill of Materials (SBOM) from container images and filesystems. Exceptional for vulnerability detection when used with a scanner tool like [Grype](https://github.com/anchore/grype).
 
 ![syft-demo](https://user-images.githubusercontent.com/590471/90277200-2a253000-de33-11ea-893f-32c219eea11a.gif)
 
@@ -15,6 +15,8 @@ A CLI tool and go library for generating a Software Bill of Materials (SBOM) fro
 - Supports packages and libraries from various ecosystems (APK, DEB, RPM, Ruby Bundles, Python Wheel/Egg/requirements.txt, JavaScript NPM/Yarn, Java JAR/EAR/WAR, Jenkins plugins JPI/HPI, Go modules)
 - Linux distribution identification (supports Alpine, BusyBox, CentOS/RedHat, Debian/Ubuntu flavored distributions)
 - Supports Docker and OCI image formats
+- Direct support for [Grype](https://github.com/anchore/grype), a fast and powerful vulnerability matcher.
+
 
 If you encounter an issue, please [let us know using the issue tracker](https://github.com/anchore/syft/issues).
 
@@ -114,7 +116,7 @@ package:
     # enable/disable cataloging of packages
     # SYFT_PACKAGE_CATALOGER_ENABLED env var
     enabled: true
-    
+
     # the search space to look for packages (options: all-layers, squashed)
     # same as -s ; SYFT_PACKAGE_CATALOGER_SCOPE env var
     scope: "squashed"
@@ -155,11 +157,11 @@ file-metadata:
     # enable/disable cataloging of file metadata
     # SYFT_FILE_METADATA_CATALOGER_ENABLED env var
     enabled: true
-    
+
     # the search space to look for file metadata (options: all-layers, squashed)
     # SYFT_FILE_METADATA_CATALOGER_SCOPE env var
     scope: "squashed"
-  
+
   # the file digest algorithms to use when cataloging files (options: "sha256", "md5", "sha1")
   # SYFT_FILE_METADATA_DIGESTS env var
   digests: ["sha256"]
@@ -185,12 +187,12 @@ secrets:
 
   # name-regex pairs to consider when searching files for secrets. Note: the regex must match single line patterns
   # but may also have OPTIONAL multiline capture groups. Regexes with a named capture group of "value" will
-  # use the entire regex to match, but the secret value will be assumed to be entirely contained within the 
+  # use the entire regex to match, but the secret value will be assumed to be entirely contained within the
   # "value" named capture group.
   additional-patterns: {}
 
-  # names to exclude from the secrets search, valid values are: "aws-access-key", "aws-secret-key", "pem-private-key", 
-  # "docker-config-auth", and "generic-api-key". Note: this does not consider any names introduced in the 
+  # names to exclude from the secrets search, valid values are: "aws-access-key", "aws-secret-key", "pem-private-key",
+  # "docker-config-auth", and "generic-api-key". Note: this does not consider any names introduced in the
   # "secrets.additional-patterns" config option.
   # SYFT_SECRETS_EXCLUDE_PATTERN_NAMES env var
   exclude-pattern-names: []
@@ -250,4 +252,4 @@ anchore:
   # same as -d ; SYFT_ANCHORE_DOCKERFILE env var
   dockerfile: ""
 
-```
+```

internal/err_helper.go

@@ -0,0 +1,14 @@
+package internal
+
+import (
+	"io"
+
+	"github.com/anchore/syft/internal/log"
+)
+
+// CloseAndLogError closes the given io.Closer and reports any errors found as a warning in the log
+func CloseAndLogError(closer io.Closer, location string) {
+	if err := closer.Close(); err != nil {
+		log.Warnf("unable to close file for location=%q: %+v", location, err)
+	}
+}

syft/distro/identify.go

@@ -5,6 +5,8 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/source"
 )
@@ -54,13 +56,13 @@ identifyLoop:
 
 		for _, location := range locations {
 			contentReader, err := resolver.FileContentsByLocation(location)
-
 			if err != nil {
 				log.Debugf("unable to get contents from %s: %s", entry.path, err)
 				continue
 			}
 
 			content, err := ioutil.ReadAll(contentReader)
+			internal.CloseAndLogError(contentReader, location.VirtualPath)
 			if err != nil {
 				log.Errorf("unable to read %q: %+v", location.RealPath, err)
 				break

syft/distro/identify_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/anchore/syft/internal"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/anchore/syft/syft/source"
 )
@@ -88,10 +89,15 @@ func TestIdentifyDistro(t *testing.T) {
 
 	observedDistros := internal.NewStringSet()
 	definedDistros := internal.NewStringSet()
+
 	for _, distroType := range All {
 		definedDistros.Add(string(distroType))
 	}
 
+	// Somewhat cheating with Windows. There is no support for detecting/parsing a Windows OS, so it is not
+	// possible to comply with this test unless it is added manually to the "observed distros"
+	definedDistros.Remove(string(Windows))
+
 	for _, test := range tests {
 		t.Run(test.fixture, func(t *testing.T) {
 			s, err := source.NewFromDirectory(test.fixture)
@@ -129,9 +135,7 @@ func TestIdentifyDistro(t *testing.T) {
 				return
 			}
 
-			if d.Version.String() != test.Version {
-				t.Errorf("expected distro version doesn't match: %v != %v", d.Version.String(), test.Version)
-			}
+			assert.Equal(t, d.Version.String(), test.Version)
 		})
 	}
 
@@ -145,7 +149,6 @@ func TestIdentifyDistro(t *testing.T) {
 		}
 		t.Errorf("distro coverage incomplete (defined=%d, coverage=%d)", len(definedDistros), len(observedDistros))
 	}
-
 }
 
 func TestParseOsRelease(t *testing.T) {

syft/distro/type.go

@@ -18,6 +18,7 @@ const (
 	ArchLinux         Type = "archlinux"
 	OpenSuseLeap      Type = "opensuseleap"
 	Photon            Type = "photon"
+	Windows           Type = "windows"
 )
 
 // All contains all Linux distribution options
@@ -34,6 +35,7 @@ var All = []Type{
 	ArchLinux,
 	OpenSuseLeap,
 	Photon,
+	Windows,
 }
 
 // IDMapping connects a distro ID like "ubuntu" to a Distro type
@@ -50,6 +52,7 @@ var IDMapping = map[string]Type{
 	"arch":          ArchLinux,
 	"opensuse-leap": OpenSuseLeap,
 	"photon":        Photon,
+	"windows":       Windows,
 }
 
 // String returns the string representation of the given Linux distribution.

syft/file/classifier.go

@@ -81,7 +81,7 @@ func (c Classifier) Classify(resolver source.FileResolver, location source.Locat
 	if err != nil {
 		return nil, err
 	}
-	defer contentReader.Close()
+	defer internal.CloseAndLogError(contentReader, location.VirtualPath)
 
 	// TODO: there is room for improvement here, as this may use an excessive amount of memory. Alternate approach is to leverage a RuneReader.
 	contents, err := ioutil.ReadAll(contentReader)

syft/file/contents_cataloger.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/source"
 )
@@ -57,7 +59,7 @@ func (i *ContentsCataloger) catalogLocation(resolver source.FileResolver, locati
 	if err != nil {
 		return "", err
 	}
-	defer contentReader.Close()
+	defer internal.CloseAndLogError(contentReader, location.VirtualPath)
 
 	buf := &bytes.Buffer{}
 	if _, err = io.Copy(base64.NewEncoder(base64.StdEncoding, buf), contentReader); err != nil {

syft/file/digest_cataloger.go

@@ -7,6 +7,8 @@ import (
 	"io"
 	"strings"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/internal/log"
 
 	"github.com/anchore/syft/internal/bus"
@@ -53,7 +55,7 @@ func (i *DigestsCataloger) catalogLocation(resolver source.FileResolver, locatio
 	if err != nil {
 		return nil, err
 	}
-	defer contentReader.Close()
+	defer internal.CloseAndLogError(contentReader, location.VirtualPath)
 
 	// create a set of hasher objects tied together with a single writer to feed content into
 	hashers := make([]hash.Hash, len(i.hashes))

syft/file/secrets_cataloger.go

@@ -8,6 +8,8 @@ import (
 	"regexp"
 	"sort"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/internal/bus"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/event"
@@ -101,7 +103,7 @@ func extractValue(resolver source.FileResolver, location source.Location, start,
 	if err != nil {
 		return "", fmt.Errorf("unable to fetch reader for location=%q : %w", location, err)
 	}
-	defer readCloser.Close()
+	defer internal.CloseAndLogError(readCloser, location.VirtualPath)
 
 	n, err := io.CopyN(ioutil.Discard, readCloser, start)
 	if err != nil {

syft/file/secrets_search_by_line_strategy.go

@@ -8,6 +8,8 @@ import (
 	"io/ioutil"
 	"regexp"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/syft/source"
 )
 
@@ -16,7 +18,7 @@ func catalogLocationByLine(resolver source.FileResolver, location source.Locatio
 	if err != nil {
 		return nil, fmt.Errorf("unable to fetch reader for location=%q : %w", location, err)
 	}
-	defer readCloser.Close()
+	defer internal.CloseAndLogError(readCloser, location.VirtualPath)
 
 	var scanner = bufio.NewReader(readCloser)
 	var position int64
@@ -65,6 +67,7 @@ func searchForSecretsWithinLine(resolver source.FileResolver, location source.Lo
 			if secret != nil {
 				secrets = append(secrets, *secret)
 			}
+			internal.CloseAndLogError(reader, location.VirtualPath)
 		}
 	}
 

syft/pkg/cataloger/common/generic_cataloger.go

@@ -6,6 +6,7 @@ package common
 import (
 	"fmt"
 
+	"github.com/anchore/syft/internal"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/pkg"
 	"github.com/anchore/syft/syft/source"
@@ -39,13 +40,14 @@ func (c *GenericCataloger) Catalog(resolver source.FileResolver) ([]pkg.Package,
 	parserByLocation := c.selectFiles(resolver)
 
 	for location, parser := range parserByLocation {
-		content, err := resolver.FileContentsByLocation(location)
+		contentReader, err := resolver.FileContentsByLocation(location)
 		if err != nil {
 			// TODO: fail or log?
 			return nil, fmt.Errorf("unable to fetch contents for location=%v : %w", location, err)
 		}
 
-		entries, err := parser(location.RealPath, content)
+		entries, err := parser(location.RealPath, contentReader)
+		internal.CloseAndLogError(contentReader, location.VirtualPath)
 		if err != nil {
 			// TODO: should we fail? or only log?
 			log.Warnf("cataloger '%s' failed to parse entries (location=%+v): %+v", c.upstreamCataloger, location, err)

syft/pkg/cataloger/deb/cataloger.go

@@ -10,6 +10,8 @@ import (
 	"path/filepath"
 	"sort"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/pkg"
 	"github.com/anchore/syft/syft/source"
@@ -50,6 +52,7 @@ func (c *Cataloger) Catalog(resolver source.FileResolver) ([]pkg.Package, error)
 		}
 
 		pkgs, err = parseDpkgStatus(dbContents)
+		internal.CloseAndLogError(dbContents, dbLocation.VirtualPath)
 		if err != nil {
 			return nil, fmt.Errorf("unable to catalog dpkg package=%+v: %w", dbLocation.RealPath, err)
 		}
@@ -78,6 +81,7 @@ func addLicenses(resolver source.FileResolver, dbLocation source.Location, p *pk
 	copyrightReader, copyrightLocation := fetchCopyrightContents(resolver, dbLocation, p)
 
 	if copyrightReader != nil {
+		defer internal.CloseAndLogError(copyrightReader, copyrightLocation.VirtualPath)
 		// attach the licenses
 		p.Licenses = parseLicensesFromCopyright(copyrightReader)
 
@@ -124,6 +128,7 @@ func getAdditionalFileListing(resolver source.FileResolver, dbLocation source.Lo
 	md5Reader, md5Location := fetchMd5Contents(resolver, dbLocation, p)
 
 	if md5Reader != nil {
+		defer internal.CloseAndLogError(md5Reader, md5Location.VirtualPath)
 		// attach the file list
 		files = append(files, parseDpkgMD5Info(md5Reader)...)
 
@@ -136,6 +141,7 @@ func getAdditionalFileListing(resolver source.FileResolver, dbLocation source.Lo
 	conffilesReader, conffilesLocation := fetchConffileContents(resolver, dbLocation, p)
 
 	if conffilesReader != nil {
+		defer internal.CloseAndLogError(conffilesReader, conffilesLocation.VirtualPath)
 		// attach the file list
 		files = append(files, parseDpkgConffileInfo(md5Reader)...)
 

syft/pkg/cataloger/javascript/parse_package_json.go

@@ -18,7 +18,7 @@ import (
 )
 
 // integrity check
-var _ common.ParserFn = parsePackageLock
+var _ common.ParserFn = parsePackageJSON
 
 // PackageJSON represents a JavaScript package.json file
 type PackageJSON struct {

syft/pkg/cataloger/python/package_cataloger.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/syft/pkg"
 
 	"github.com/anchore/syft/syft/source"
@@ -101,6 +103,7 @@ func (c *PackageCataloger) fetchRecordFiles(resolver source.FileResolver, metada
 		if err != nil {
 			return nil, nil, err
 		}
+		defer internal.CloseAndLogError(recordContents, recordPath)
 
 		// parse the record contents
 		records, err := parseWheelOrEggRecord(recordContents)
@@ -130,6 +133,7 @@ func (c *PackageCataloger) fetchTopLevelPackages(resolver source.FileResolver, m
 	if err != nil {
 		return nil, nil, err
 	}
+	defer internal.CloseAndLogError(topLevelContents, topLevelLocation.VirtualPath)
 
 	scanner := bufio.NewScanner(topLevelContents)
 	for scanner.Scan() {
@@ -151,6 +155,7 @@ func (c *PackageCataloger) assembleEggOrWheelMetadata(resolver source.FileResolv
 	if err != nil {
 		return nil, nil, err
 	}
+	defer internal.CloseAndLogError(metadataContents, metadataLocation.VirtualPath)
 
 	metadata, err := parseWheelOrEggMetadata(metadataLocation.RealPath, metadataContents)
 	if err != nil {

syft/pkg/cataloger/rpmdb/cataloger.go

@@ -6,6 +6,8 @@ package rpmdb
 import (
 	"fmt"
 
+	"github.com/anchore/syft/internal"
+
 	"github.com/anchore/syft/syft/pkg"
 	"github.com/anchore/syft/syft/source"
 )
@@ -39,6 +41,7 @@ func (c *Cataloger) Catalog(resolver source.FileResolver) ([]pkg.Package, error)
 		}
 
 		pkgs, err = parseRpmDB(resolver, location, dbContentReader)
+		internal.CloseAndLogError(dbContentReader, location.VirtualPath)
 		if err != nil {
 			return nil, fmt.Errorf("unable to catalog rpmdb package=%+v: %w", location.RealPath, err)
 		}

syft/pkg/kb_package_metadata.go

@@ -0,0 +1,11 @@
+package pkg
+
+// KbPackageMetadata is slightly odd in how it is expected to map onto data.
+// This is critical to grasp because there is no MSRC cataloger. The `ProductID`
+// field is expected to be the MSRC Product ID, for example:
+// "Windows 10 Version 1703 for 32-bit Systems".
+// `Kb` is expected to be the actual KB number, for example "5001028"
+type KbPackageMetadata struct {
+	ProductID string `toml:"product_id" json:"product_id"`
+	Kb        string `toml:"kb" json:"kb"`
+}

syft/pkg/metadata.go

@@ -14,4 +14,5 @@ const (
 	RpmdbMetadataType            MetadataType = "RpmdbMetadata"
 	PythonPackageMetadataType    MetadataType = "PythonPackageMetadata"
 	RustCargoPackageMetadataType MetadataType = "RustCargoPackageMetadata"
+	KbPackageMetadataType        MetadataType = "KbPackageMetadata"
 )

syft/pkg/type.go

@@ -18,6 +18,7 @@ const (
 	JenkinsPluginPkg Type = "jenkins-plugin"
 	GoModulePkg      Type = "go-module"
 	RustPkg          Type = "rust-crate"
+	KbPkg            Type = "msrc-kb"
 )
 
 // AllPkgs represents all supported package types
@@ -32,6 +33,7 @@ var AllPkgs = []Type{
 	JenkinsPluginPkg,
 	GoModulePkg,
 	RustPkg,
+	KbPkg,
 }
 
 // PackageURLType returns the PURL package type for the current package.

test/integration/catalog_packages_test.go

@@ -63,6 +63,7 @@ func TestPkgCoverageImage(t *testing.T) {
 	for _, p := range pkg.AllPkgs {
 		definedPkgs.Add(string(p))
 	}
+	definedPkgs.Remove(string(pkg.KbPkg))
 
 	var cases []testCase
 	cases = append(cases, commonTestCases...)
@@ -189,6 +190,7 @@ func TestPkgCoverageDirectory(t *testing.T) {
 	definedLanguages.Remove(pkg.UnknownLanguage.String())
 	observedPkgs.Remove(string(pkg.UnknownPkg))
 	definedPkgs.Remove(string(pkg.UnknownPkg))
+	definedPkgs.Remove(string(pkg.KbPkg))
 
 	// ensure that integration test commonTestCases stay in sync with the available catalogers
 	if len(observedLanguages) < len(definedLanguages) {