feat: warn if a vulnerability is ignored multiple times in the same c…

…onfig (#1377)

Currently if you have multiple ignores for a vulnerability, we just
silently use the first one; this has us instead print a warning to make
it more obvious when that occurs.

I've not had it error as it doesn't feel like a big enough deal to be
worth erroring, though maybe that would be better as I can't think of an
actual reason to have duplicate entries 🤔

Resolves #1367

cmd/osv-scanner/__snapshots__/main_test.snap

@@ -395,6 +395,24 @@ unknown keys in config file: RustVersionOverride, PackageOverrides.skip, Package
 
 ---
 
+[TestRun/config_files_should_not_have_multiple_ignores_with_the_same_id - 1]
+warning: ./fixtures/osv-scanner-duplicate-config.toml has multiple ignores for GO-2022-0274 - only the first will be used!
+Scanning dir ./fixtures/locks-many
+Scanned <rootdir>/fixtures/locks-many/Gemfile.lock file and found 1 package
+Scanned <rootdir>/fixtures/locks-many/alpine.cdx.xml as CycloneDX SBOM and found 14 packages
+Scanned <rootdir>/fixtures/locks-many/composer.lock file and found 1 package
+Scanned <rootdir>/fixtures/locks-many/package-lock.json file and found 1 package
+Scanned <rootdir>/fixtures/locks-many/yarn.lock file and found 1 package
+GHSA-whgm-jr23-g3j9 and 1 alias have been filtered out because: (no reason given)
+Filtered 1 vulnerability from output
+No issues found
+
+---
+
+[TestRun/config_files_should_not_have_multiple_ignores_with_the_same_id - 2]
+
+---
+
 [TestRun/cyclonedx_1.4_output - 1]
 {
   "$schema": "http://cyclonedx.org/schema/bom-1.4.schema.json",

cmd/osv-scanner/fixtures/osv-scanner-duplicate-config.toml

@@ -0,0 +1,10 @@
+[[IgnoredVulns]]
+id = "GO-2022-0274"
+ignoreuntil = 2020-01-01
+
+[[IgnoredVulns]]
+id = "GO-2022-0274"
+ignoreuntil = 2022-01-01
+
+[[IgnoredVulns]]
+id = "GHSA-whgm-jr23-g3j9"

cmd/osv-scanner/main_test.go

@@ -359,6 +359,12 @@ func TestRun(t *testing.T) {
 			args: []string{"", "--config=./fixtures/osv-scanner-unknown-config.toml", "./fixtures/locks-many"},
 			exit: 127,
 		},
+		// config file with multiple ignores with the same id
+		{
+			name: "config files should not have multiple ignores with the same id",
+			args: []string{"", "--config=./fixtures/osv-scanner-duplicate-config.toml", "./fixtures/locks-many"},
+			exit: 0,
+		},
 		// a bunch of requirements.txt files with different names
 		{
 			name: "requirements.txt can have all kinds of names",

internal/config/config.go

@@ -136,8 +136,8 @@ func shouldIgnoreTimestamp(ignoreUntil time.Time) bool {
 
 // Sets the override config by reading the config file at configPath.
 // Will return an error if loading the config file fails
-func (c *Manager) UseOverride(configPath string) error {
-	config, configErr := tryLoadConfig(configPath)
+func (c *Manager) UseOverride(r reporter.Reporter, configPath string) error {
+	config, configErr := tryLoadConfig(r, configPath)
 	if configErr != nil {
 		return configErr
 	}
@@ -165,7 +165,7 @@ func (c *Manager) Get(r reporter.Reporter, targetPath string) Config {
 		return config
 	}
 
-	config, configErr := tryLoadConfig(configPath)
+	config, configErr := tryLoadConfig(r, configPath)
 	if configErr == nil {
 		r.Infof("Loaded filter from: %s\n", config.LoadPath)
 	} else {
@@ -202,7 +202,7 @@ func normalizeConfigLoadPath(target string) (string, error) {
 
 // tryLoadConfig attempts to parse the config file at the given path as TOML,
 // returning the Config object if successful or otherwise the error
-func tryLoadConfig(configPath string) (Config, error) {
+func tryLoadConfig(r reporter.Reporter, configPath string) (Config, error) {
 	config := Config{}
 	m, err := toml.DecodeFile(configPath, &config)
 	if err == nil {
@@ -219,7 +219,19 @@ func tryLoadConfig(configPath string) (Config, error) {
 		}
 
 		config.LoadPath = configPath
+		config.warnAboutDuplicates(r)
 	}
 
 	return config, err
 }
+
+func (c *Config) warnAboutDuplicates(r reporter.Reporter) {
+	seen := make(map[string]struct{})
+
+	for _, vuln := range c.IgnoredVulns {
+		if _, ok := seen[vuln.ID]; ok {
+			r.Warnf("warning: %s has multiple ignores for %s - only the first will be used!\n", c.LoadPath, vuln.ID)
+		}
+		seen[vuln.ID] = struct{}{}
+	}
+}

internal/config/config_internal_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/osv-scanner/pkg/models"
+	"github.com/google/osv-scanner/pkg/reporter"
 )
 
 // Attempts to normalize any file paths in the given `output` so that they can
@@ -172,7 +173,7 @@ func Test_tryLoadConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := tryLoadConfig(tt.args.configPath)
+			got, err := tryLoadConfig(&reporter.VoidReporter{}, tt.args.configPath)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("tryLoadConfig() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -222,7 +223,7 @@ func TestTryLoadConfig_UnknownKeys(t *testing.T) {
 	}
 
 	for _, testData := range tests {
-		c, err := tryLoadConfig(testData.configPath)
+		c, err := tryLoadConfig(&reporter.VoidReporter{}, testData.configPath)
 
 		// we should always be returning an empty config on error
 		if diff := cmp.Diff(Config{}, c); diff != "" {

pkg/osvscanner/osvscanner.go

@@ -842,7 +842,7 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe
 	var scannedPackages []scannedPackage
 
 	if actions.ConfigOverridePath != "" {
-		err := configManager.UseOverride(actions.ConfigOverridePath)
+		err := configManager.UseOverride(r, actions.ConfigOverridePath)
 		if err != nil {
 			r.Errorf("Failed to read config file: %s\n", err)
 			return models.VulnerabilityResults{}, err