From 4acd34e7bf705125441edfa89bbc97740dce2abd Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Sep 2024 07:45:42 +1200 Subject: [PATCH 1/5] fix: warn when a config file is invalid --- cmd/osv-scanner/__snapshots__/main_test.snap | 12 +++++ .../fixtures/config-invalid/composer.lock | 51 +++++++++++++++++++ .../fixtures/config-invalid/osv-scanner.toml | 1 + cmd/osv-scanner/main_test.go | 6 +++ pkg/config/config.go | 11 +++- 5 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 cmd/osv-scanner/fixtures/config-invalid/composer.lock create mode 100644 cmd/osv-scanner/fixtures/config-invalid/osv-scanner.toml diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index 77f42c52b3f..00d4ce6cc26 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -373,6 +373,18 @@ overriding license for package Packagist/league/flysystem/1.0.8 with 0BSD --- +[TestRun/config_file_is_invalid - 1] +Scanning dir ./fixtures/config-invalid +Scanned /fixtures/config-invalid/composer.lock file and found 1 package +Ignored invalid config file at: /fixtures/config-invalid/osv-scanner.toml +No issues found + +--- + +[TestRun/config_file_is_invalid - 2] + +--- + [TestRun/cyclonedx_1.4_output - 1] { "$schema": "http://cyclonedx.org/schema/bom-1.4.schema.json", diff --git a/cmd/osv-scanner/fixtures/config-invalid/composer.lock b/cmd/osv-scanner/fixtures/config-invalid/composer.lock new file mode 100644 index 00000000000..3cfadf73cb4 --- /dev/null +++ b/cmd/osv-scanner/fixtures/config-invalid/composer.lock @@ -0,0 +1,51 @@ +{ + "_readme": [ + "This file locks the dependencies of your project to a known state", + "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", + "This file is @generated automatically" + ], + "content-hash": "439b16dd5df2e0730bd1cc4352654d09", + "packages": [ + { + "name": "sentry/sdk", + "version": "2.0.4", + "source": { + "type": "git", + "url": "https://github.com/getsentry/sentry-php-sdk.git", + "reference": "4c115873c86ad5bd0ac6d962db70ca53bf8fb874" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/getsentry/sentry-php-sdk/zipball/4c115873c86ad5bd0ac6d962db70ca53bf8fb874", + "reference": "4c115873c86ad5bd0ac6d962db70ca53bf8fb874", + "shasum": "" + }, + "require": { + "http-interop/http-factory-guzzle": "^1.0", + "php-http/curl-client": "^1.0|^2.0", + "sentry/sentry": "^2.1.3" + }, + "type": "metapackage", + "notification-url": "https://packagist.org/downloads/", + "license": ["MIT"], + "authors": [ + { + "name": "Sentry", + "email": "accounts@sentry.io" + } + ], + "description": "This is a metapackage shipping sentry/sentry with a recommended http client.", + "time": "2019-09-09T19:54:44+00:00" + } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "dev", + "stability-flags": [], + "prefer-stable": true, + "prefer-lowest": false, + "platform": { + "php": "^7.1.3" + }, + "platform-dev": [] +} diff --git a/cmd/osv-scanner/fixtures/config-invalid/osv-scanner.toml b/cmd/osv-scanner/fixtures/config-invalid/osv-scanner.toml new file mode 100644 index 00000000000..cdf4cb4feba --- /dev/null +++ b/cmd/osv-scanner/fixtures/config-invalid/osv-scanner.toml @@ -0,0 +1 @@ +! diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index b33522bfe13..51d291dc57f 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -318,6 +318,12 @@ func TestRun(t *testing.T) { args: []string{"", "--config=./fixtures/osv-scanner-composite-config.toml", "--experimental-licenses", "MIT", "./fixtures/locks-many", "./fixtures/locks-insecure"}, exit: 1, }, + // invalid config file + { + name: "config file is invalid", + args: []string{"", "./fixtures/config-invalid"}, + exit: 0, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/config/config.go b/pkg/config/config.go index e95bee3918e..084fb3db1bb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "os" "path/filepath" @@ -175,6 +176,9 @@ func (c *ConfigManager) Get(r reporter.Reporter, targetPath string) Config { if configErr == nil { r.Infof("Loaded filter from: %s\n", config.LoadPath) } else { + if !errors.Is(configErr, errConfigFileNotFound) { + r.Warnf("Ignored invalid config file at: %s\n", configPath) + } // If config doesn't exist, use the default config config = c.DefaultConfig } @@ -201,6 +205,9 @@ func normalizeConfigLoadPath(target string) (string, error) { return configPath, nil } +var errConfigFileInvalid = errors.New("failed to parse config file") +var errConfigFileNotFound = errors.New("no config file found") + // tryLoadConfig tries to load config in `target` (or it's containing directory) // `target` will be the key for the entry in configMap func tryLoadConfig(configPath string) (Config, error) { @@ -211,12 +218,12 @@ func tryLoadConfig(configPath string) (Config, error) { _, err := toml.NewDecoder(file).Decode(&config) if err != nil { - return Config{}, fmt.Errorf("failed to parse config file: %w", err) + return Config{}, fmt.Errorf("%w: %s", errConfigFileInvalid, err) } config.LoadPath = configPath return config, nil } - return Config{}, fmt.Errorf("no config file found on this path: %s", configPath) + return Config{}, fmt.Errorf("%w: %s", errConfigFileNotFound, configPath) } From da53501d034690bf2e9290c2d85a3f0d1c1433c0 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Sep 2024 07:50:38 +1200 Subject: [PATCH 2/5] fix: error instead of warn --- cmd/osv-scanner/__snapshots__/main_test.snap | 3 +-- cmd/osv-scanner/main_test.go | 2 +- pkg/config/config.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index 00d4ce6cc26..f4dab0336cd 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -376,12 +376,11 @@ overriding license for package Packagist/league/flysystem/1.0.8 with 0BSD [TestRun/config_file_is_invalid - 1] Scanning dir ./fixtures/config-invalid Scanned /fixtures/config-invalid/composer.lock file and found 1 package -Ignored invalid config file at: /fixtures/config-invalid/osv-scanner.toml -No issues found --- [TestRun/config_file_is_invalid - 2] +Ignored invalid config file at: /fixtures/config-invalid/osv-scanner.toml --- diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index 51d291dc57f..c593a91a148 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -322,7 +322,7 @@ func TestRun(t *testing.T) { { name: "config file is invalid", args: []string{"", "./fixtures/config-invalid"}, - exit: 0, + exit: 127, }, } for _, tt := range tests { diff --git a/pkg/config/config.go b/pkg/config/config.go index 084fb3db1bb..4eb65b448a0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -177,7 +177,7 @@ func (c *ConfigManager) Get(r reporter.Reporter, targetPath string) Config { r.Infof("Loaded filter from: %s\n", config.LoadPath) } else { if !errors.Is(configErr, errConfigFileNotFound) { - r.Warnf("Ignored invalid config file at: %s\n", configPath) + r.Errorf("Ignored invalid config file at: %s\n", configPath) } // If config doesn't exist, use the default config config = c.DefaultConfig From d39849d499dd8f65ab418f6dee69bcb0503e54be Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Sep 2024 07:52:14 +1200 Subject: [PATCH 3/5] refactor: use `%w` --- pkg/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 4eb65b448a0..735f18af2c3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -218,7 +218,7 @@ func tryLoadConfig(configPath string) (Config, error) { _, err := toml.NewDecoder(file).Decode(&config) if err != nil { - return Config{}, fmt.Errorf("%w: %s", errConfigFileInvalid, err) + return Config{}, fmt.Errorf("%w: %w", errConfigFileInvalid, err) } config.LoadPath = configPath From eb060e315419e9739b658937fd85500f88bc5481 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Sep 2024 14:06:37 +1200 Subject: [PATCH 4/5] refactor: check for `os.ErrNotExist` specifically --- pkg/config/config.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 735f18af2c3..74a15c5327a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -176,7 +176,8 @@ func (c *ConfigManager) Get(r reporter.Reporter, targetPath string) Config { if configErr == nil { r.Infof("Loaded filter from: %s\n", config.LoadPath) } else { - if !errors.Is(configErr, errConfigFileNotFound) { + // anything other than the config file not existing is most likely due to an invalid config file + if !errors.Is(configErr, os.ErrNotExist) { r.Errorf("Ignored invalid config file at: %s\n", configPath) } // If config doesn't exist, use the default config @@ -205,9 +206,6 @@ func normalizeConfigLoadPath(target string) (string, error) { return configPath, nil } -var errConfigFileInvalid = errors.New("failed to parse config file") -var errConfigFileNotFound = errors.New("no config file found") - // tryLoadConfig tries to load config in `target` (or it's containing directory) // `target` will be the key for the entry in configMap func tryLoadConfig(configPath string) (Config, error) { @@ -218,12 +216,12 @@ func tryLoadConfig(configPath string) (Config, error) { _, err := toml.NewDecoder(file).Decode(&config) if err != nil { - return Config{}, fmt.Errorf("%w: %w", errConfigFileInvalid, err) + return Config{}, err } config.LoadPath = configPath return config, nil } - return Config{}, fmt.Errorf("%w: %s", errConfigFileNotFound, configPath) + return Config{}, err } From 2236e21a96f17837f3813f95000390169364eb63 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Thu, 12 Sep 2024 15:43:41 +1200 Subject: [PATCH 5/5] feat: include the reason why the config was considered invalid in verbose mode --- cmd/osv-scanner/__snapshots__/main_test.snap | 12 ++++++++++++ cmd/osv-scanner/main_test.go | 5 +++++ pkg/config/config.go | 1 + 3 files changed, 18 insertions(+) diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index f4dab0336cd..3b98ecc95a2 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -384,6 +384,18 @@ Ignored invalid config file at: /fixtures/config-invalid/osv-scanner.to --- +[TestRun/config_file_is_invalid#01 - 1] +Scanning dir ./fixtures/config-invalid +Scanned /fixtures/config-invalid/composer.lock file and found 1 package +Config file /fixtures/config-invalid/osv-scanner.toml is invalid because: toml: line 1: expected '.' or '=', but got '!' instead + +--- + +[TestRun/config_file_is_invalid#01 - 2] +Ignored invalid config file at: /fixtures/config-invalid/osv-scanner.toml + +--- + [TestRun/cyclonedx_1.4_output - 1] { "$schema": "http://cyclonedx.org/schema/bom-1.4.schema.json", diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index c593a91a148..899150473cc 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -324,6 +324,11 @@ func TestRun(t *testing.T) { args: []string{"", "./fixtures/config-invalid"}, exit: 127, }, + { + name: "config file is invalid", + args: []string{"", "--verbosity", "verbose", "./fixtures/config-invalid"}, + exit: 127, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/config/config.go b/pkg/config/config.go index 74a15c5327a..05c23b5e9bf 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -179,6 +179,7 @@ func (c *ConfigManager) Get(r reporter.Reporter, targetPath string) Config { // anything other than the config file not existing is most likely due to an invalid config file if !errors.Is(configErr, os.ErrNotExist) { r.Errorf("Ignored invalid config file at: %s\n", configPath) + r.Verbosef("Config file %s is invalid because: %v\n", configPath, configErr) } // If config doesn't exist, use the default config config = c.DefaultConfig