feat: error if configuration file has unknown properties (#1249)

Currently the scanner does not check if there were unrecognized
properties when loading configuration files, meaning that typos can
easily slip through; to help avoid this, I've modified `tryLoadConfig`
to return an error if there are any undecoded keys in the metadata.

While this could cause existing configs to start erroring, I don't think
this should be considered a breaking change as such configurations would
inherently causing different behaviour than what was desired and thus be
a downstream bug themselves.

This will also mean that old versions of the scanner going forward would
not be compatible with configs for even newer versions of the scanner
that introduce new config properties, which is could be a little
annoying but I don't think outweighs the benefit of this validation
especially given such configurations likewise would result in different
behaviour since the old scanner version would not know what to do with
the new configuration option.

Resolves #1098

cmd/osv-scanner/__snapshots__/main_test.snap

@@ -401,6 +401,16 @@ Ignored invalid config file at: <rootdir>/fixtures/config-invalid/osv-scanner.to
 
 ---
 
+[TestRun/config_files_cannot_have_unknown_keys - 1]
+
+---
+
+[TestRun/config_files_cannot_have_unknown_keys - 2]
+Failed to read config file: unknown keys in config file: RustVersionOverride, PackageOverrides.skip, PackageOverrides.license.skip
+unknown keys in config file: RustVersionOverride, PackageOverrides.skip, PackageOverrides.license.skip
+
+---
+
 [TestRun/cyclonedx_1.4_output - 1]
 {
   "$schema": "http://cyclonedx.org/schema/bom-1.4.schema.json",

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

@@ -0,0 +1,24 @@
+RustVersionOverride = "1.2.3"
+
+[[PackageOverrides]]
+ecosystem = "npm"
+skip = true
+license.override = ["0BSD"]
+
+[[PackageOverrides]]
+ecosystem = "Packagist"
+license.override = ["0BSD"]
+
+[[PackageOverrides]]
+ecosystem = "Alpine"
+Name = "musl"
+license.override = ["UNKNOWN"]
+
+[[PackageOverrides]]
+ecosystem = "Alpine"
+name = "musl-utils"
+license.skip = true
+
+[[IgnoredVulns]]
+id = "GO-2022-0274"
+ignoreuntil = 2020-01-01

cmd/osv-scanner/main_test.go

@@ -335,6 +335,12 @@ func TestRun(t *testing.T) {
 			args: []string{"", "--verbosity", "verbose", "./fixtures/config-invalid"},
 			exit: 127,
 		},
+		// config file with unknown keys
+		{
+			name: "config files cannot have unknown keys",
+			args: []string{"", "--config=./fixtures/osv-scanner-unknown-config.toml", "./fixtures/locks-many"},
+			exit: 127,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/config/config.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"strings"
 	"time"
 
 	"github.com/BurntSushi/toml"
@@ -215,8 +216,20 @@ func normalizeConfigLoadPath(target string) (string, error) {
 // returning the Config object if successful or otherwise the error
 func tryLoadConfig(configPath string) (Config, error) {
 	config := Config{}
-	_, err := toml.DecodeFile(configPath, &config)
+	m, err := toml.DecodeFile(configPath, &config)
 	if err == nil {
+		unknownKeys := m.Undecoded()
+
+		if len(unknownKeys) > 0 {
+			keys := make([]string, 0, len(unknownKeys))
+
+			for _, key := range unknownKeys {
+				keys = append(keys, key.String())
+			}
+
+			return Config{}, fmt.Errorf("unknown keys in config file: %s", strings.Join(keys, ", "))
+		}
+
 		config.LoadPath = configPath
 	}
 

pkg/config/config_internal_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -183,6 +184,62 @@ func Test_tryLoadConfig(t *testing.T) {
 	}
 }
 
+func TestTryLoadConfig_UnknownKeys(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		configPath string
+		unknownMsg string
+	}{
+		{
+			configPath: "./fixtures/unknown-key-1.toml",
+			unknownMsg: "IgnoredVulns.ignoreUntilTime",
+		},
+		{
+			configPath: "./fixtures/unknown-key-2.toml",
+			unknownMsg: "IgnoredVulns.ignoreUntiI",
+		},
+		{
+			configPath: "./fixtures/unknown-key-3.toml",
+			unknownMsg: "IgnoredVulns.reasoning",
+		},
+		{
+			configPath: "./fixtures/unknown-key-4.toml",
+			unknownMsg: "PackageOverrides.skip",
+		},
+		{
+			configPath: "./fixtures/unknown-key-5.toml",
+			unknownMsg: "PackageOverrides.license.skip",
+		},
+		{
+			configPath: "./fixtures/unknown-key-6.toml",
+			unknownMsg: "RustVersionOverride",
+		},
+		{
+			configPath: "./fixtures/unknown-key-7.toml",
+			unknownMsg: "RustVersionOverride, PackageOverrides.skip",
+		},
+	}
+
+	for _, testData := range tests {
+		c, err := tryLoadConfig(testData.configPath)
+
+		// we should always be returning an empty config on error
+		if diff := cmp.Diff(Config{}, c); diff != "" {
+			t.Errorf("tryLoadConfig() mismatch (-want +got):\n%s", diff)
+		}
+		if err == nil {
+			t.Fatal("tryLoadConfig() did not return an error")
+		}
+
+		wantMsg := fmt.Sprintf("unknown keys in config file: %v", testData.unknownMsg)
+
+		if err.Error() != wantMsg {
+			t.Errorf("tryLoadConfig() error = '%v', want '%s'", err, wantMsg)
+		}
+	}
+}
+
 func TestConfig_ShouldIgnore(t *testing.T) {
 	t.Parallel()
 

pkg/config/fixtures/unknown-key-1.toml

@@ -0,0 +1,4 @@
+[[IgnoredVulns]]
+id = "GHSA-jgvc-jfgh-rjvv"
+ignoreUntilTime = 2024-08-02 # whoops, should be "ignoreUntil"
+reason = "..."

pkg/config/fixtures/unknown-key-2.toml

@@ -0,0 +1,4 @@
+[[IgnoredVulns]]
+id = "GHSA-jgvc-jfgh-rjvv"
+ignoreUntiI = 2024-08-02 # whoops, should be "ignoreUntil"
+reason = "..."

pkg/config/fixtures/unknown-key-3.toml

@@ -0,0 +1,4 @@
+[[IgnoredVulns]]
+id = "GHSA-jgvc-jfgh-rjvv"
+ignoreUntil = 2024-08-02
+reasoning = "..." # whoops, should be "reason"

pkg/config/fixtures/unknown-key-4.toml

@@ -0,0 +1,4 @@
+[[PackageOverrides]]
+ecosystem = "npm"
+skip = true # whoops, should be "ignore"
+license.override = ["0BSD"]

pkg/config/fixtures/unknown-key-5.toml

@@ -0,0 +1,3 @@
+[[PackageOverrides]]
+ecosystem = "npm"
+license.skip = false # whoops, should be "license.ignore"

pkg/config/fixtures/unknown-key-6.toml

@@ -0,0 +1 @@
+RustVersionOverride = "1.2.3" # whoops, not supported

pkg/config/fixtures/unknown-key-7.toml

@@ -0,0 +1,5 @@
+RustVersionOverride = "1.2.3" # whoops, not supported
+
+[[PackageOverrides]]
+ecosystem = "npm"
+skip = true # whoops, should be "ignore"