Skip to content

Commit

Permalink
Allow excluding analyzers globally (#1180)
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 samples for test. Previously G115 has been included in the issue count
* Show analyzers IDs(G115, G602) in gosec usage help
* See #1175
  • Loading branch information
Rgvs authored Aug 20, 2024
1 parent 18135b4 commit 81cda2f
Show file tree
Hide file tree
Showing 11 changed files with 301 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 *analyzers.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: analyzers.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 = analyzers.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
38 changes: 38 additions & 0 deletions analyzers/analyzers_set.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// (c) Copyright gosec's authors
//
// 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"

type AnalyzerSet struct {
Analyzers []*analysis.Analyzer
AnalyzerSuppressedMap map[string]bool
}

// NewAnalyzerSet constructs a new AnalyzerSet
func NewAnalyzerSet() *AnalyzerSet {
return &AnalyzerSet{nil, make(map[string]bool)}
}

// Register adds a trigger for the supplied analyzer
func (a *AnalyzerSet) Register(analyzer *analysis.Analyzer, isSuppressed bool) {
a.Analyzers = append(a.Analyzers, analyzer)
a.AnalyzerSuppressedMap[analyzer.Name] = isSuppressed
}

// IsSuppressed will return whether the Analyzer is suppressed.
func (a *AnalyzerSet) IsSuppressed(ruleID string) bool {
return a.AnalyzerSuppressedMap[ruleID]
}
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)
})
})
})
94 changes: 94 additions & 0 deletions analyzers/analyzerslist.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// (c) Copyright gosec's authors
//
// 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
}

// AnalyzerBuilder is used to register an analyzer definition with the analyzer
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 (al *AnalyzerList) AnalyzersInfo() (map[string]AnalyzerDefinition, map[string]bool) {
builders := make(map[string]AnalyzerDefinition)
for _, def := range al.Analyzers {
builders[def.ID] = def
}
return builders, al.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
}
}

var defaultAnalyzers = []AnalyzerDefinition{
{"G115", "Type conversion which leads to integer overflow", newConversionOverflowAnalyzer},
{"G602", "Possible slice bounds out of range", newSliceBoundsAnalyzer},
}

// Generate the list of analyzers to use
func Generate(trackSuppressions bool, filters ...AnalyzerFilter) *AnalyzerList {
analyzerMap := make(map[string]AnalyzerDefinition)
analyzerSuppressedMap := make(map[string]bool)

for _, analyzer := range defaultAnalyzers {
analyzerSuppressedMap[analyzer.ID] = false
addToAnalyzerList := true
for _, filter := range filters {
if filter(analyzer.ID) {
analyzerSuppressedMap[analyzer.ID] = true
if !trackSuppressions {
addToAnalyzerList = false
}
}
}
if addToAnalyzerList {
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
Loading

0 comments on commit 81cda2f

Please sign in to comment.