diff --git a/analyzer.go b/analyzer.go index 371904b..6e44246 100644 --- a/analyzer.go +++ b/analyzer.go @@ -187,7 +187,6 @@ type Analyzer struct { trackSuppressions bool concurrency int analyzerSet *analyzers.AnalyzerSet - mu sync.Mutex } // NewAnalyzer builds a new analyzer. @@ -251,12 +250,6 @@ func (gosec *Analyzer) LoadAnalyzers(analyzerDefinitions map[string]analyzers.An // Process kicks off the analysis process for a given package func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error { - config := &packages.Config{ - Mode: LoadMode, - BuildFlags: buildTags, - Tests: gosec.tests, - } - type result struct { pkgPath string pkgs []*packages.Package @@ -273,7 +266,7 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error for { select { case s := <-j: - pkgs, err := gosec.load(s, config) + pkgs, err := gosec.load(s, buildTags) select { case r <- result{pkgPath: s, pkgs: pkgs, err: err}: case <-quit: @@ -326,7 +319,7 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error return nil } -func (gosec *Analyzer) load(pkgPath string, conf *packages.Config) ([]*packages.Package, error) { +func (gosec *Analyzer) load(pkgPath string, buildTags []string) ([]*packages.Package, error) { abspath, err := GetPkgAbsPath(pkgPath) if err != nil { gosec.logger.Printf("Skipping: %s. Path doesn't exist.", abspath) @@ -334,12 +327,10 @@ func (gosec *Analyzer) load(pkgPath string, conf *packages.Config) ([]*packages. } gosec.logger.Println("Import directory:", abspath) - // step 1/3 create build context. + + // step 1/2: build context requires the array of build tags. buildD := build.Default - // step 2/3: add build tags to get env dependent files into basePackage. - gosec.mu.Lock() - buildD.BuildTags = conf.BuildFlags - gosec.mu.Unlock() + buildD.BuildTags = buildTags basePackage, err := buildD.ImportDir(pkgPath, build.ImportComment) if err != nil { return []*packages.Package{}, fmt.Errorf("importing dir %q: %w", pkgPath, err) @@ -362,10 +353,12 @@ func (gosec *Analyzer) load(pkgPath string, conf *packages.Config) ([]*packages. } } - // step 3/3 remove build tags from conf to proceed build correctly. - gosec.mu.Lock() - conf.BuildFlags = nil - defer gosec.mu.Unlock() + // step 2/2: pass in cli encoded build flags to build correctly. + conf := &packages.Config{ + Mode: LoadMode, + BuildFlags: CLIBuildTags(buildTags), + Tests: gosec.tests, + } pkgs, err := packages.Load(conf, packageFiles...) if err != nil { return []*packages.Package{}, fmt.Errorf("loading files from package %q: %w", pkgPath, err) diff --git a/analyzer_test.go b/analyzer_test.go index 62a2361..0c80665 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -16,6 +16,7 @@ package gosec_test import ( "errors" + "fmt" "go/build" "log" "regexp" @@ -783,16 +784,79 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) - It("should pass the build tags", func() { + It("should not panic if a file can not compile", func() { + sample := testutils.SampleCodeCompilationFail[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false).RulesInfo()) + pkg := testutils.NewTestPackage() + defer pkg.Close() + + pkg.AddFile("main.go", source) + err := pkg.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, pkg.Path) + Expect(err).ShouldNot(HaveOccurred()) + }) + + It("should exclude a reportable file, if excluded by build tags", func() { + // file has a reportable security issue, but should only be flagged + // to only being compiled in via a build flag. + sample := testutils.SampleCodeG501BuildTag[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false).RulesInfo()) + pkg := testutils.NewTestPackage() + defer pkg.Close() + + pkg.AddFile("main.go", source) + err := pkg.Build() + Expect(err).To(BeEquivalentTo(&build.NoGoError{Dir: pkg.Path})) // no files should be found for scanning. + err = analyzer.Process(buildTags, pkg.Path) + Expect(err).ShouldNot(HaveOccurred()) + + issues, _, _ := analyzer.Report() + Expect(issues).Should(BeEmpty()) + }) + + It("should attempt to analyse a file with build tags", func() { sample := testutils.SampleCodeBuildTag[0] source := sample.Code[0] analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() - pkg.AddFile("tags.go", source) + tags := []string{"tag"} - err := analyzer.Process(tags, pkg.Path) + pkg.AddFile("main.go", source) + err := pkg.Build(testutils.WithBuildTags(tags)) Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(tags, pkg.Path) + Expect(err).ShouldNot(HaveOccurred()) + + issues, _, _ := analyzer.Report() + if len(issues) != sample.Errors { + fmt.Println(sample.Code) + } + Expect(issues).Should(HaveLen(sample.Errors)) + }) + + It("should report issues from a file with build tags", func() { + sample := testutils.SampleCodeG501BuildTag[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false).RulesInfo()) + pkg := testutils.NewTestPackage() + defer pkg.Close() + + tags := []string{"tag"} + pkg.AddFile("main.go", source) + err := pkg.Build(testutils.WithBuildTags(tags)) + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(tags, pkg.Path) + Expect(err).ShouldNot(HaveOccurred()) + + issues, _, _ := analyzer.Report() + if len(issues) != sample.Errors { + fmt.Println(sample.Code) + } + Expect(issues).Should(HaveLen(sample.Errors)) }) It("should process an empty package with test file", func() { diff --git a/helpers.go b/helpers.go index 33e2b4a..7f5724b 100644 --- a/helpers.go +++ b/helpers.go @@ -553,3 +553,24 @@ func parseGoVersion(version string) (int, int, int) { return major, minor, build } + +// CLIBuildTags converts a list of Go build tags into the corresponding CLI +// build flag (-tags=form) by trimming whitespace, removing empty entries, +// and joining them into a comma-separated -tags argument for use with go build +// commands. +func CLIBuildTags(buildTags []string) []string { + var buildFlags []string + if len(buildTags) > 0 { + for _, tag := range buildTags { + // remove empty entries and surrounding whitespace + if t := strings.TrimSpace(tag); t != "" { + buildFlags = append(buildFlags, t) + } + } + if len(buildFlags) > 0 { + buildFlags = []string{"-tags=" + strings.Join(buildFlags, ",")} + } + } + + return buildFlags +} diff --git a/helpers_test.go b/helpers_test.go index 3349057..2d06ae4 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -342,4 +342,26 @@ var _ = Describe("Helpers", func() { Expect(operands).Should(HaveLen(4)) }) }) + + Context("when transforming build tags to cli build flags", func() { + It("should return an empty slice when no tags are provided", func() { + result := gosec.CLIBuildTags([]string{}) + Expect(result).To(BeEmpty()) + }) + + It("should return a single -tags flag when one tag is provided", func() { + result := gosec.CLIBuildTags([]string{"integration"}) + Expect(result).To(Equal([]string{"-tags=integration"})) + }) + + It("should combine multiple tags into a single -tags flag", func() { + result := gosec.CLIBuildTags([]string{"linux", "amd64", "netgo"}) + Expect(result).To(Equal([]string{"-tags=linux,amd64,netgo"})) + }) + + It("should trim and ignore empty tags", func() { + result := gosec.CLIBuildTags([]string{" linux ", "", "amd64"}) + Expect(result).To(Equal([]string{"-tags=linux,amd64"})) + }) + }) }) diff --git a/testutils/build_samples.go b/testutils/build_samples.go new file mode 100644 index 0000000..16b68cf --- /dev/null +++ b/testutils/build_samples.go @@ -0,0 +1,31 @@ +package testutils + +import "github.com/securego/gosec/v2" + +var ( + // SampleCodeCompilationFail provides a file that won't compile. + SampleCodeCompilationFail = []CodeSample{ + {[]string{` +package main + +func main() { + fmt.Println("no package imported error") +} +`}, 1, gosec.NewConfig()}, + } + + // SampleCodeBuildTag provides a small program that should only compile + // provided a build tag. + SampleCodeBuildTag = []CodeSample{ + {[]string{` +// +build tag +package main + +import "fmt" + +func main() { + fmt.Println("Hello world") +} +`}, 0, gosec.NewConfig()}, + } +) diff --git a/testutils/g501_samples.go b/testutils/g501_samples.go index 238dd52..77ac114 100644 --- a/testutils/g501_samples.go +++ b/testutils/g501_samples.go @@ -3,8 +3,9 @@ package testutils import "github.com/securego/gosec/v2" // SampleCodeG501 - Blocklisted import MD5 -var SampleCodeG501 = []CodeSample{ - {[]string{` +var ( + SampleCodeG501 = []CodeSample{ + {[]string{` package main import ( @@ -19,4 +20,27 @@ func main() { } } `}, 1, gosec.NewConfig()}, + } + + // SampleCodeG501BuildTag provides a reportable file if a build tag is + // supplied. + SampleCodeG501BuildTag = []CodeSample{ + {[]string{` +//go:build tag + +package main + +import ( + "crypto/md5" + "fmt" + "os" +) + +func main() { + for _, arg := range os.Args { + fmt.Printf("%x - %s\n", md5.Sum([]byte(arg)), arg) + } } +`}, 2, gosec.NewConfig()}, + } +) diff --git a/testutils/g601_samples.go b/testutils/g601_samples.go index f6dfc02..6db1f8a 100644 --- a/testutils/g601_samples.go +++ b/testutils/g601_samples.go @@ -2,10 +2,9 @@ package testutils import "github.com/securego/gosec/v2" -var ( - // SampleCodeG601 - Implicit aliasing over range statement - SampleCodeG601 = []CodeSample{ - {[]string{` +// SampleCodeG601 - Implicit aliasing over range statement +var SampleCodeG601 = []CodeSample{ + {[]string{` package main import "fmt" @@ -40,7 +39,7 @@ func main() { fmt.Printf("%d %v %s", zero, c_star, c) } `}, 1, gosec.NewConfig()}, - {[]string{` + {[]string{` // see: github.com/securego/gosec/issues/475 package main @@ -56,7 +55,7 @@ func main() { } } `}, 0, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -77,7 +76,7 @@ func main() { } } `}, 0, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -98,7 +97,7 @@ func main() { } } `}, 1, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -119,7 +118,7 @@ func main() { } } `}, 0, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -140,7 +139,7 @@ func main() { } } `}, 1, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -165,7 +164,7 @@ func main() { } } `}, 1, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -190,7 +189,7 @@ func main() { } } `}, 0, gosec.NewConfig()}, - {[]string{` + {[]string{` package main import ( @@ -205,17 +204,4 @@ func main() { } } `}, 1, gosec.NewConfig()}, - } - - // SampleCodeBuildTag - G601 build tags - SampleCodeBuildTag = []CodeSample{ - {[]string{` -// +build tag -package main - -func main() { - fmt.Println("no package imported error") } -`}, 1, gosec.NewConfig()}, - } -) diff --git a/testutils/pkg.go b/testutils/pkg.go index 6dcf79e..79eeced 100644 --- a/testutils/pkg.go +++ b/testutils/pkg.go @@ -27,6 +27,17 @@ type TestPackage struct { build *buildObj } +// Option provides a way to adjust the package config depending on testing +// requirements +type Option func(conf *packages.Config) + +// WithBuildTags enables injecting build tags into the package config on build. +func WithBuildTags(tags []string) Option { + return func(conf *packages.Config) { + conf.BuildFlags = tags + } +} + // NewTestPackage will create a new and empty package. Must call Close() to cleanup // auxiliary files func NewTestPackage() *TestPackage { @@ -62,14 +73,26 @@ func (p *TestPackage) write() error { } // Build ensures all files are persisted to disk and built -func (p *TestPackage) Build() error { +func (p *TestPackage) Build(opts ...Option) error { if p.build != nil { return nil } if err := p.write(); err != nil { return err } - basePackage, err := build.Default.ImportDir(p.Path, build.ImportComment) + + conf := &packages.Config{ + Mode: gosec.LoadMode, + Tests: false, + } + for _, opt := range opts { + opt(conf) + } + + // step 1/2: build context requires the array of build tags. + builder := build.Default + builder.BuildTags = conf.BuildFlags + basePackage, err := builder.ImportDir(p.Path, build.ImportComment) if err != nil { return err } @@ -79,10 +102,8 @@ func (p *TestPackage) Build() error { packageFiles = append(packageFiles, path.Join(p.Path, filename)) } - conf := &packages.Config{ - Mode: gosec.LoadMode, - Tests: false, - } + // step 2/2: normalise to cli build flags for package loading + conf.BuildFlags = gosec.CLIBuildTags(conf.BuildFlags) pkgs, err := packages.Load(conf, packageFiles...) if err != nil { return err @@ -96,8 +117,8 @@ func (p *TestPackage) Build() error { } // CreateContext builds a context out of supplied package context -func (p *TestPackage) CreateContext(filename string) *gosec.Context { - if err := p.Build(); err != nil { +func (p *TestPackage) CreateContext(filename string, opts ...Option) *gosec.Context { + if err := p.Build(opts...); err != nil { log.Fatal(err) return nil }