Skip to content

Commit

Permalink
Allow excluding analyzers globally
Browse files Browse the repository at this point in the history
* This change does not exclude analyzers for inline comment
* Changed the expected issues count for G103, G109. Previously G115 has been included
* show analyzers IDs(G115, G602) in gosec usage help
* See #1175
  • Loading branch information
Rgvs committed Aug 16, 2024
1 parent 56f943b commit b8bc4f4
Show file tree
Hide file tree
Showing 11 changed files with 281 additions and 26 deletions.
18 changes: 14 additions & 4 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ type Analyzer struct {
showIgnored bool
trackSuppressions bool
concurrency int
analyzerList []*analysis.Analyzer
analyzerSet *AnalyzerSet
mu sync.Mutex
}

Expand Down Expand Up @@ -213,7 +213,7 @@ func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressio
concurrency: concurrency,
excludeGenerated: excludeGenerated,
trackSuppressions: trackSuppressions,
analyzerList: analyzers.BuildDefaultAnalyzers(),
analyzerSet: NewAnalyzerSet(),
}
}

Expand All @@ -236,6 +236,15 @@ func (gosec *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder, ruleSup
}
}

// LoadAnalyzers instantiates all the analyzers to be used when analyzing source
// packages
func (gosec *Analyzer) LoadAnalyzers(analyzerDefinitions map[string]analyzers.AnalyzerDefinition, analyzerSuppressed map[string]bool) {
for id, def := range analyzerDefinitions {
r := def.Create(def.ID, def.Description)
gosec.analyzerSet.Register(r, analyzerSuppressed[id])
}
}

// Process kicks off the analysis process for a given package
func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error {
config := &packages.Config{
Expand Down Expand Up @@ -415,7 +424,7 @@ func (gosec *Analyzer) CheckAnalyzers(pkg *packages.Package) {

generatedFiles := gosec.generatedFiles(pkg)

for _, analyzer := range gosec.analyzerList {
for _, analyzer := range gosec.analyzerSet.Analyzers {
pass := &analysis.Pass{
Analyzer: analyzer,
Fset: pkg.Fset,
Expand Down Expand Up @@ -666,7 +675,7 @@ func (gosec *Analyzer) getSuppressionsAtLineInFile(file string, line string, id
suppressions := append(generalSuppressions, ruleSuppressions...)

// Track external suppressions of this rule.
if gosec.ruleset.IsRuleSuppressed(id) {
if gosec.ruleset.IsRuleSuppressed(id) || gosec.analyzerSet.IsSuppressed(id) {
ignored = true
suppressions = append(suppressions, issue.SuppressionInfo{
Kind: "external",
Expand Down Expand Up @@ -705,4 +714,5 @@ func (gosec *Analyzer) Reset() {
gosec.issues = make([]*issue.Issue, 0, 16)
gosec.stats = &Metrics{}
gosec.ruleset = NewRuleSet()
gosec.analyzerSet = NewAnalyzerSet()
}
41 changes: 41 additions & 0 deletions analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/securego/gosec/v2"
"github.com/securego/gosec/v2/analyzers"
"github.com/securego/gosec/v2/rules"
"github.com/securego/gosec/v2/testutils"
"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -1110,6 +1111,7 @@ var _ = Describe("Analyzer", func() {
It("should be able to scan generated files if NOT excluded when using the analyzes", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
customAnalyzer.LoadAnalyzers(analyzers.Generate(false).AnalyzersInfo())
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("foo.go", `
Expand All @@ -1132,6 +1134,7 @@ var _ = Describe("Analyzer", func() {
It("should be able to skip generated files if excluded when using the analyzes", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, true, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
customAnalyzer.LoadAnalyzers(analyzers.Generate(false).AnalyzersInfo())
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("foo.go", `
Expand Down Expand Up @@ -1499,6 +1502,44 @@ var _ = Describe("Analyzer", func() {
Expect(issues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should not report an error if the analyzer is not included", func() {
sample := testutils.SampleCodeG602[0]
source := sample.Code[0]
analyzer.LoadAnalyzers(analyzers.Generate(true, analyzers.NewAnalyzerFilter(false, "G115")).AnalyzersInfo())

controlPackage := testutils.NewTestPackage()
defer controlPackage.Close()
controlPackage.AddFile("cipher.go", source)
err := controlPackage.Build()
Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(buildTags, controlPackage.Path)
Expect(err).ShouldNot(HaveOccurred())
controlIssues, _, _ := analyzer.Report()
Expect(controlIssues).Should(HaveLen(sample.Errors))
Expect(controlIssues[0].Suppressions).To(HaveLen(1))
Expect(controlIssues[0].Suppressions[0].Kind).To(Equal("external"))
Expect(controlIssues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should not report an error if the analyzer is excluded", func() {
sample := testutils.SampleCodeG602[0]
source := sample.Code[0]
analyzer.LoadAnalyzers(analyzers.Generate(true, analyzers.NewAnalyzerFilter(true, "G602")).AnalyzersInfo())

controlPackage := testutils.NewTestPackage()
defer controlPackage.Close()
controlPackage.AddFile("cipher.go", source)
err := controlPackage.Build()
Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(buildTags, controlPackage.Path)
Expect(err).ShouldNot(HaveOccurred())
issues, _, _ := analyzer.Report()
Expect(issues).Should(HaveLen(sample.Errors))
Expect(issues[0].Suppressions).To(HaveLen(1))
Expect(issues[0].Suppressions[0].Kind).To(Equal("external"))
Expect(issues[0].Suppressions[0].Justification).To(Equal("Globally suppressed."))
})

It("should track multiple suppressions if the violation is multiply suppressed", func() {
sample := testutils.SampleCodeG101[0]
source := sample.Code[0]
Expand Down
62 changes: 62 additions & 0 deletions analyzers/analyzers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package analyzers_test

import (
"fmt"
"log"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/securego/gosec/v2"
"github.com/securego/gosec/v2/analyzers"
"github.com/securego/gosec/v2/testutils"
)

var _ = Describe("gosec analyzers", func() {
var (
logger *log.Logger
config gosec.Config
analyzer *gosec.Analyzer
runner func(string, []testutils.CodeSample)
buildTags []string
tests bool
)

BeforeEach(func() {
logger, _ = testutils.NewLogger()
config = gosec.NewConfig()
analyzer = gosec.NewAnalyzer(config, tests, false, false, 1, logger)
runner = func(analyzerId string, samples []testutils.CodeSample) {
for n, sample := range samples {
analyzer.Reset()
analyzer.SetConfig(sample.Config)
analyzer.LoadAnalyzers(analyzers.Generate(false, analyzers.NewAnalyzerFilter(false, analyzerId)).AnalyzersInfo())
pkg := testutils.NewTestPackage()
defer pkg.Close()
for i, code := range sample.Code {
pkg.AddFile(fmt.Sprintf("sample_%d_%d.go", n, i), code)
}
err := pkg.Build()
Expect(err).ShouldNot(HaveOccurred())
Expect(pkg.PrintErrors()).Should(BeZero())
err = analyzer.Process(buildTags, pkg.Path)
Expect(err).ShouldNot(HaveOccurred())
issues, _, _ := analyzer.Report()
if len(issues) != sample.Errors {
fmt.Println(sample.Code)
}
Expect(issues).Should(HaveLen(sample.Errors))
}
}
})

Context("report correct errors for all samples", func() {
It("should detect integer conversion overflow", func() {
runner("G115", testutils.SampleCodeG115)
})

It("should detect out of bounds slice access", func() {
runner("G602", testutils.SampleCodeG602)
})
})
})
89 changes: 89 additions & 0 deletions analyzers/analyzerslist.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package analyzers

import (
"golang.org/x/tools/go/analysis"
)

// AnalyzerDefinition contains the description of an analyzer and a mechanism to
// create it.
type AnalyzerDefinition struct {
ID string
Description string
Create AnalyzerBuilder
}

type AnalyzerBuilder func(id string, description string) *analysis.Analyzer

// AnalyzerList contains a mapping of analyzer ID's to analyzer definitions and a mapping
// of analyzer ID's to whether analyzers are suppressed.
type AnalyzerList struct {
Analyzers map[string]AnalyzerDefinition
AnalyzerSuppressed map[string]bool
}

// AnalyzersInfo returns all the create methods and the analyzer suppressed map for a
// given list
func (rl AnalyzerList) AnalyzersInfo() (map[string]AnalyzerDefinition, map[string]bool) {
builders := make(map[string]AnalyzerDefinition)
for _, def := range rl.Analyzers {
builders[def.ID] = def
}
return builders, rl.AnalyzerSuppressed
}

// AnalyzerFilter can be used to include or exclude an analyzer depending on the return
// value of the function
type AnalyzerFilter func(string) bool

// NewAnalyzerFilter is a closure that will include/exclude the analyzer ID's based on
// the supplied boolean value.
func NewAnalyzerFilter(action bool, analyzerIDs ...string) AnalyzerFilter {
analyzerlist := make(map[string]bool)
for _, analyzer := range analyzerIDs {
analyzerlist[analyzer] = true
}
return func(analyzer string) bool {
if _, found := analyzerlist[analyzer]; found {
return action
}
return !action
}
}

// Generate the list of analyzers to use
func Generate(trackSuppressions bool, filters ...AnalyzerFilter) AnalyzerList {
analyzers := []AnalyzerDefinition{
{"G115", "Type conversion which leads to integer overflow", newConversionOverflowAnalyzer},
{"G602", "Possible slice bounds out of range", newSliceBoundsAnalyzer},
}

analyzerMap := make(map[string]AnalyzerDefinition)
analyzerSuppressedMap := make(map[string]bool)

ANALYZERS:
for _, analyzer := range analyzers {
analyzerSuppressedMap[analyzer.ID] = false
for _, filter := range filters {
if filter(analyzer.ID) {
analyzerSuppressedMap[analyzer.ID] = true
if !trackSuppressions {
continue ANALYZERS
}
}
}
analyzerMap[analyzer.ID] = analyzer
}
return AnalyzerList{Analyzers: analyzerMap, AnalyzerSuppressed: analyzerSuppressedMap}
}
13 changes: 13 additions & 0 deletions analyzers/anaylzers_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package analyzers_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestAnalyzers(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Analyzers Suite")
}
8 changes: 0 additions & 8 deletions analyzers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ type SSAAnalyzerResult struct {
SSA *buildssa.SSA
}

// BuildDefaultAnalyzers returns the default list of analyzers
func BuildDefaultAnalyzers() []*analysis.Analyzer {
return []*analysis.Analyzer{
newConversionOverflowAnalyzer("G115", "Type conversion which leads to integer overflow"),
newSliceBoundsAnalyzer("G602", "Possible slice bounds out of range"),
}
}

// getSSAResult retrieves the SSA result from analysis pass
func getSSAResult(pass *analysis.Pass) (*SSAAnalyzerResult, error) {
result, ok := pass.ResultOf[buildssa.Analyzer]
Expand Down
39 changes: 36 additions & 3 deletions cmd/gosec/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"

"github.com/securego/gosec/v2"
"github.com/securego/gosec/v2/analyzers"
"github.com/securego/gosec/v2/autofix"
"github.com/securego/gosec/v2/cmd/vflag"
"github.com/securego/gosec/v2/issue"
Expand Down Expand Up @@ -175,14 +176,23 @@ func usage() {

// sorted rule list for ease of reading
rl := rules.Generate(*flagTrackSuppressions)
keys := make([]string, 0, len(rl.Rules))
al := analyzers.Generate(*flagTrackSuppressions)
keys := make([]string, 0, len(rl.Rules)+len(al.Analyzers))
for key := range rl.Rules {
keys = append(keys, key)
}
for key := range al.Analyzers {
keys = append(keys, key)
}
sort.Strings(keys)
for _, k := range keys {
v := rl.Rules[k]
fmt.Fprintf(os.Stderr, "\t%s: %s\n", k, v.Description)
var description string
if rule, ok := rl.Rules[k]; ok {
description = rule.Description
} else if analyzer, ok := al.Analyzers[k]; ok {
description = analyzer.Description
}
fmt.Fprintf(os.Stderr, "\t%s: %s\n", k, description)
}
fmt.Fprint(os.Stderr, "\n")
}
Expand Down Expand Up @@ -243,6 +253,26 @@ func loadRules(include, exclude string) rules.RuleList {
return rules.Generate(*flagTrackSuppressions, filters...)
}

func loadAnalyzers(include, exclude string) analyzers.AnalyzerList {
var filters []analyzers.AnalyzerFilter
if include != "" {
logger.Printf("Including analyzers: %s", include)
including := strings.Split(include, ",")
filters = append(filters, analyzers.NewAnalyzerFilter(false, including...))
} else {
logger.Println("Including analyzers: default")
}

if exclude != "" {
logger.Printf("Excluding analyzers: %s", exclude)
excluding := strings.Split(exclude, ",")
filters = append(filters, analyzers.NewAnalyzerFilter(true, excluding...))
} else {
logger.Println("Excluding analyzers: default")
}
return analyzers.Generate(*flagTrackSuppressions, filters...)
}

func getRootPaths(paths []string) []string {
rootPaths := make([]string, 0)
for _, path := range paths {
Expand Down Expand Up @@ -410,9 +440,12 @@ func main() {
logger.Fatal("No rules are configured")
}

analyzerList := loadAnalyzers(includeRules, excludeRules)

// Create the analyzer
analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, *flagConcurrency, logger)
analyzer.LoadRules(ruleList.RulesInfo())
analyzer.LoadAnalyzers(analyzerList.AnalyzersInfo())

excludedDirs := gosec.ExcludedDirsRegExp(flagDirsExclude)
var packages []string
Expand Down
Loading

0 comments on commit b8bc4f4

Please sign in to comment.