From c2a9b8d20095259e75017c199aeea90c30cb4444 Mon Sep 17 00:00:00 2001 From: hbc Date: Sat, 15 Apr 2023 13:42:52 +0800 Subject: [PATCH 1/6] fix: convert gosec global settings as map --- pkg/golinters/gosec.go | 64 +++++++++++++++++++++++++++------- pkg/golinters/gosec_test.go | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 pkg/golinters/gosec_test.go diff --git a/pkg/golinters/gosec.go b/pkg/golinters/gosec.go index 5441c40fae5d..b16db4c3bc08 100644 --- a/pkg/golinters/gosec.go +++ b/pkg/golinters/gosec.go @@ -22,24 +22,62 @@ import ( const gosecName = "gosec" -func NewGosec(settings *config.GoSecSettings) *goanalysis.Linter { - var mu sync.Mutex - var resIssues []goanalysis.Issue +// see gosec#Config.convertGlobals +func convertGosecGlobals(settings *config.GoSecSettings, conf gosec.Config) { + if len(settings.Config) < 1 { + return + } + + globalOptionFromConfig, exists := settings.Config[gosec.Globals] + if !exists { + return + } + + globalOptionMap, ok := globalOptionFromConfig.(map[string]any) + if !ok { + return + } + for k, v := range globalOptionMap { + conf.SetGlobal(gosec.GlobalOption(k), fmt.Sprintf("%v", v)) + } +} +func toGosecConfig(settings *config.GoSecSettings) gosec.Config { conf := gosec.NewConfig() - var filters []rules.RuleFilter - if settings != nil { - filters = gosecRuleFilters(settings.Includes, settings.Excludes) + if len(settings.Config) < 1 { + return conf + } - for k, v := range settings.Config { - if k != gosec.Globals { - // Uses ToUpper because the parsing of the map's key change the key to lowercase. - // The value is not impacted by that: the case is respected. - k = strings.ToUpper(k) - } - conf.Set(k, v) + // global section + convertGosecGlobals(settings, conf) + + // non-global section + for k, v := range settings.Config { + if k == gosec.Globals { + continue } + + // Uses ToUpper because the parsing of the map's key change the key to lowercase. + // The value is not impacted by that: the case is respected. + k = strings.ToUpper(k) + conf.Set(k, v) + } + + return conf +} + +func NewGosec(settings *config.GoSecSettings) *goanalysis.Linter { + var ( + mu sync.Mutex + resIssues []goanalysis.Issue + filters []rules.RuleFilter + conf gosec.Config + ) + + if settings != nil { + filters = gosecRuleFilters(settings.Includes, settings.Excludes) + conf = toGosecConfig(settings) } logger := log.New(io.Discard, "", 0) diff --git a/pkg/golinters/gosec_test.go b/pkg/golinters/gosec_test.go new file mode 100644 index 000000000000..f3836be7cd8b --- /dev/null +++ b/pkg/golinters/gosec_test.go @@ -0,0 +1,69 @@ +package golinters + +import ( + "fmt" + "testing" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/securego/gosec/v2" + "github.com/stretchr/testify/assert" +) + +func TestToGosecConfig(t *testing.T) { + t.Run("empty config map", func(t *testing.T) { + settings := &config.GoSecSettings{} + + gosecConfig := toGosecConfig(settings) + assert.Len(t, gosecConfig, 1) + assert.Contains(t, gosecConfig, gosec.Globals) + }) + + t.Run("with global settings", func(t *testing.T) { + globalsSettings := map[string]any{ + string(gosec.Nosec): true, + string(gosec.Audit): "true", + } + settings := &config.GoSecSettings{ + Config: map[string]any{ + gosec.Globals: globalsSettings, + }, + } + + gosecConfig := toGosecConfig(settings) + assert.Len(t, gosecConfig, 1) + assert.Contains(t, gosecConfig, gosec.Globals) + + for _, k := range []gosec.GlobalOption{gosec.Nosec, gosec.Audit} { + v, err := gosecConfig.GetGlobal(k) + assert.NoError(t, err, "error getting global option %s", k) + assert.Equal( + t, + fmt.Sprintf("%v", globalsSettings[string(k)]), + v, + "global option %s is not set to expected value", k, + ) + } + + for _, k := range []gosec.GlobalOption{gosec.NoSecAlternative} { + _, err := gosecConfig.GetGlobal(k) + assert.Error(t, err, "should not set global option %s", k) + } + }) + + t.Run("rule specified settings", func(t *testing.T) { + settings := &config.GoSecSettings{ + Config: map[string]any{ + "g101": map[string]any{ + "pattern": "(?i)example", + }, + "G301": "0750", + }, + } + + gosecConfig := toGosecConfig(settings) + assert.Len(t, gosecConfig, 3) + assert.Contains(t, gosecConfig, gosec.Globals) + assert.Contains(t, gosecConfig, "G101") + assert.Contains(t, gosecConfig, "G301") + }) +} From 10ae46e3c6798780a9ca92de3b70efe8a60209c8 Mon Sep 17 00:00:00 2001 From: hbc Date: Sat, 15 Apr 2023 13:58:09 +0800 Subject: [PATCH 2/6] test: add test case for gosec global option --- test/testdata/configs/gosec_global_option.yml | 5 +++++ test/testdata/gosec_global_option.go | 14 ++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 test/testdata/configs/gosec_global_option.yml create mode 100644 test/testdata/gosec_global_option.go diff --git a/test/testdata/configs/gosec_global_option.yml b/test/testdata/configs/gosec_global_option.yml new file mode 100644 index 000000000000..fc0c3e27ac6f --- /dev/null +++ b/test/testdata/configs/gosec_global_option.yml @@ -0,0 +1,5 @@ +linters-settings: + gosec: + config: + global: + nosec: true diff --git a/test/testdata/gosec_global_option.go b/test/testdata/gosec_global_option.go new file mode 100644 index 000000000000..0a9407ea75de --- /dev/null +++ b/test/testdata/gosec_global_option.go @@ -0,0 +1,14 @@ +//golangcitest:args -Egosec +//golangcitest:config_path testdata/configs/gosec_global_option.yml +package testdata + +import ( + "crypto/md5" // want "G501: Blocklisted import crypto/md5: weak cryptographic primitive" + "log" +) + +func Gosec() { + // #nosec G401 + h := md5.New() // want "G401: Use of weak cryptographic primitive" + log.Print(h) +} From 4384f7af92b200915b72db440a9b4b5dd992fc49 Mon Sep 17 00:00:00 2001 From: hbc Date: Sat, 15 Apr 2023 19:49:39 +0800 Subject: [PATCH 3/6] Update pkg/golinters/gosec_test.go Co-authored-by: Oleksandr Redko --- pkg/golinters/gosec_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/golinters/gosec_test.go b/pkg/golinters/gosec_test.go index f3836be7cd8b..59a3df49e640 100644 --- a/pkg/golinters/gosec_test.go +++ b/pkg/golinters/gosec_test.go @@ -4,9 +4,10 @@ import ( "fmt" "testing" - "github.com/golangci/golangci-lint/pkg/config" "github.com/securego/gosec/v2" "github.com/stretchr/testify/assert" + + "github.com/golangci/golangci-lint/pkg/config" ) func TestToGosecConfig(t *testing.T) { From 3acea4d9f09d1e84f02485e23194405c229367ec Mon Sep 17 00:00:00 2001 From: hbc Date: Sat, 15 Apr 2023 19:49:46 +0800 Subject: [PATCH 4/6] Update pkg/golinters/gosec_test.go Co-authored-by: Oleksandr Redko --- pkg/golinters/gosec_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/golinters/gosec_test.go b/pkg/golinters/gosec_test.go index 59a3df49e640..5564593da614 100644 --- a/pkg/golinters/gosec_test.go +++ b/pkg/golinters/gosec_test.go @@ -62,9 +62,11 @@ func TestToGosecConfig(t *testing.T) { } gosecConfig := toGosecConfig(settings) - assert.Len(t, gosecConfig, 3) - assert.Contains(t, gosecConfig, gosec.Globals) - assert.Contains(t, gosecConfig, "G101") - assert.Contains(t, gosecConfig, "G301") + assert.Equal(t, + gosec.Config{ + "G101": map[string]any{"pattern": "(?i)example"}, + "G301": "0750", + "global": map[gosec.GlobalOption]string{}}, + gosecConfig) }) } From 8757472c1a65a1703b5abf4eadf0a71304782310 Mon Sep 17 00:00:00 2001 From: hbc Date: Sat, 15 Apr 2023 19:51:50 +0800 Subject: [PATCH 5/6] style: move private functions to the end --- pkg/golinters/gosec.go | 90 +++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/pkg/golinters/gosec.go b/pkg/golinters/gosec.go index b16db4c3bc08..fbf418bf02fd 100644 --- a/pkg/golinters/gosec.go +++ b/pkg/golinters/gosec.go @@ -22,51 +22,6 @@ import ( const gosecName = "gosec" -// see gosec#Config.convertGlobals -func convertGosecGlobals(settings *config.GoSecSettings, conf gosec.Config) { - if len(settings.Config) < 1 { - return - } - - globalOptionFromConfig, exists := settings.Config[gosec.Globals] - if !exists { - return - } - - globalOptionMap, ok := globalOptionFromConfig.(map[string]any) - if !ok { - return - } - for k, v := range globalOptionMap { - conf.SetGlobal(gosec.GlobalOption(k), fmt.Sprintf("%v", v)) - } -} - -func toGosecConfig(settings *config.GoSecSettings) gosec.Config { - conf := gosec.NewConfig() - - if len(settings.Config) < 1 { - return conf - } - - // global section - convertGosecGlobals(settings, conf) - - // non-global section - for k, v := range settings.Config { - if k == gosec.Globals { - continue - } - - // Uses ToUpper because the parsing of the map's key change the key to lowercase. - // The value is not impacted by that: the case is respected. - k = strings.ToUpper(k) - conf.Set(k, v) - } - - return conf -} - func NewGosec(settings *config.GoSecSettings) *goanalysis.Linter { var ( mu sync.Mutex @@ -218,3 +173,48 @@ func filterIssues(issues []*gosec.Issue, severity, confidence gosec.Score) []*go } return res } + +// see gosec#Config.convertGlobals +func convertGosecGlobals(settings *config.GoSecSettings, conf gosec.Config) { + if len(settings.Config) < 1 { + return + } + + globalOptionFromConfig, exists := settings.Config[gosec.Globals] + if !exists { + return + } + + globalOptionMap, ok := globalOptionFromConfig.(map[string]any) + if !ok { + return + } + for k, v := range globalOptionMap { + conf.SetGlobal(gosec.GlobalOption(k), fmt.Sprintf("%v", v)) + } +} + +func toGosecConfig(settings *config.GoSecSettings) gosec.Config { + conf := gosec.NewConfig() + + if len(settings.Config) < 1 { + return conf + } + + // global section + convertGosecGlobals(settings, conf) + + // non-global section + for k, v := range settings.Config { + if k == gosec.Globals { + continue + } + + // Uses ToUpper because the parsing of the map's key change the key to lowercase. + // The value is not impacted by that: the case is respected. + k = strings.ToUpper(k) + conf.Set(k, v) + } + + return conf +} From 5ffee18f757b47331c0098617bbb1219b17a351d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 21 Apr 2023 22:03:00 +0200 Subject: [PATCH 6/6] review: remove useless code --- pkg/golinters/gosec.go | 86 ++++++++++++---------------- pkg/golinters/gosec_test.go | 108 ++++++++++++++++++------------------ 2 files changed, 88 insertions(+), 106 deletions(-) diff --git a/pkg/golinters/gosec.go b/pkg/golinters/gosec.go index fbf418bf02fd..a0ff9a90b408 100644 --- a/pkg/golinters/gosec.go +++ b/pkg/golinters/gosec.go @@ -23,13 +23,11 @@ import ( const gosecName = "gosec" func NewGosec(settings *config.GoSecSettings) *goanalysis.Linter { - var ( - mu sync.Mutex - resIssues []goanalysis.Issue - filters []rules.RuleFilter - conf gosec.Config - ) + var mu sync.Mutex + var resIssues []goanalysis.Issue + var filters []rules.RuleFilter + conf := gosec.NewConfig() if settings != nil { filters = gosecRuleFilters(settings.Includes, settings.Excludes) conf = toGosecConfig(settings) @@ -133,6 +131,35 @@ func runGoSec(lintCtx *linter.Context, pass *analysis.Pass, settings *config.GoS return issues } +func toGosecConfig(settings *config.GoSecSettings) gosec.Config { + conf := gosec.NewConfig() + + for k, v := range settings.Config { + if k == gosec.Globals { + convertGosecGlobals(v, conf) + continue + } + + // Uses ToUpper because the parsing of the map's key change the key to lowercase. + // The value is not impacted by that: the case is respected. + conf.Set(strings.ToUpper(k), v) + } + + return conf +} + +// based on https://github.com/securego/gosec/blob/47bfd4eb6fc7395940933388550b547538b4c946/config.go#L52-L62 +func convertGosecGlobals(globalOptionFromConfig any, conf gosec.Config) { + globalOptionMap, ok := globalOptionFromConfig.(map[string]any) + if !ok { + return + } + + for k, v := range globalOptionMap { + conf.SetGlobal(gosec.GlobalOption(k), fmt.Sprintf("%v", v)) + } +} + // based on https://github.com/securego/gosec/blob/569328eade2ccbad4ce2d0f21ee158ab5356a5cf/cmd/gosec/main.go#L170-L188 func gosecRuleFilters(includes, excludes []string) []rules.RuleFilter { var filters []rules.RuleFilter @@ -166,55 +193,12 @@ func convertToScore(str string) (gosec.Score, error) { // code borrowed from https://github.com/securego/gosec/blob/69213955dacfd560562e780f723486ef1ca6d486/cmd/gosec/main.go#L264-L276 func filterIssues(issues []*gosec.Issue, severity, confidence gosec.Score) []*gosec.Issue { res := make([]*gosec.Issue, 0) + for _, issue := range issues { if issue.Severity >= severity && issue.Confidence >= confidence { res = append(res, issue) } } - return res -} - -// see gosec#Config.convertGlobals -func convertGosecGlobals(settings *config.GoSecSettings, conf gosec.Config) { - if len(settings.Config) < 1 { - return - } - - globalOptionFromConfig, exists := settings.Config[gosec.Globals] - if !exists { - return - } - - globalOptionMap, ok := globalOptionFromConfig.(map[string]any) - if !ok { - return - } - for k, v := range globalOptionMap { - conf.SetGlobal(gosec.GlobalOption(k), fmt.Sprintf("%v", v)) - } -} - -func toGosecConfig(settings *config.GoSecSettings) gosec.Config { - conf := gosec.NewConfig() - - if len(settings.Config) < 1 { - return conf - } - - // global section - convertGosecGlobals(settings, conf) - - // non-global section - for k, v := range settings.Config { - if k == gosec.Globals { - continue - } - - // Uses ToUpper because the parsing of the map's key change the key to lowercase. - // The value is not impacted by that: the case is respected. - k = strings.ToUpper(k) - conf.Set(k, v) - } - return conf + return res } diff --git a/pkg/golinters/gosec_test.go b/pkg/golinters/gosec_test.go index 5564593da614..9b1b5faeff96 100644 --- a/pkg/golinters/gosec_test.go +++ b/pkg/golinters/gosec_test.go @@ -1,7 +1,6 @@ package golinters import ( - "fmt" "testing" "github.com/securego/gosec/v2" @@ -10,63 +9,62 @@ import ( "github.com/golangci/golangci-lint/pkg/config" ) -func TestToGosecConfig(t *testing.T) { - t.Run("empty config map", func(t *testing.T) { - settings := &config.GoSecSettings{} - - gosecConfig := toGosecConfig(settings) - assert.Len(t, gosecConfig, 1) - assert.Contains(t, gosecConfig, gosec.Globals) - }) - - t.Run("with global settings", func(t *testing.T) { - globalsSettings := map[string]any{ - string(gosec.Nosec): true, - string(gosec.Audit): "true", - } - settings := &config.GoSecSettings{ - Config: map[string]any{ - gosec.Globals: globalsSettings, +func Test_toGosecConfig(t *testing.T) { + testCases := []struct { + desc string + settings *config.GoSecSettings + expected gosec.Config + }{ + { + desc: "empty config map", + settings: &config.GoSecSettings{}, + expected: gosec.Config{ + "global": map[gosec.GlobalOption]string{}, }, - } - - gosecConfig := toGosecConfig(settings) - assert.Len(t, gosecConfig, 1) - assert.Contains(t, gosecConfig, gosec.Globals) - - for _, k := range []gosec.GlobalOption{gosec.Nosec, gosec.Audit} { - v, err := gosecConfig.GetGlobal(k) - assert.NoError(t, err, "error getting global option %s", k) - assert.Equal( - t, - fmt.Sprintf("%v", globalsSettings[string(k)]), - v, - "global option %s is not set to expected value", k, - ) - } - - for _, k := range []gosec.GlobalOption{gosec.NoSecAlternative} { - _, err := gosecConfig.GetGlobal(k) - assert.Error(t, err, "should not set global option %s", k) - } - }) - - t.Run("rule specified settings", func(t *testing.T) { - settings := &config.GoSecSettings{ - Config: map[string]any{ - "g101": map[string]any{ - "pattern": "(?i)example", + }, + { + desc: "with global settings", + settings: &config.GoSecSettings{ + Config: map[string]any{ + gosec.Globals: map[string]any{ + string(gosec.Nosec): true, + string(gosec.Audit): "true", + }, }, - "G301": "0750", }, - } - - gosecConfig := toGosecConfig(settings) - assert.Equal(t, - gosec.Config{ + expected: gosec.Config{ + "global": map[gosec.GlobalOption]string{ + "audit": "true", + "nosec": "true", + }, + }, + }, + { + desc: "rule specified setting", + settings: &config.GoSecSettings{ + Config: map[string]any{ + "g101": map[string]any{ + "pattern": "(?i)example", + }, + "G301": "0750", + }, + }, + expected: gosec.Config{ "G101": map[string]any{"pattern": "(?i)example"}, "G301": "0750", - "global": map[gosec.GlobalOption]string{}}, - gosecConfig) - }) + "global": map[gosec.GlobalOption]string{}, + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + conf := toGosecConfig(test.settings) + + assert.Equal(t, test.expected, conf) + }) + } }