fix: build tag parsing. (#1413)

* fix: build tag parsing.

* chore: lint fixes.

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)

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() {

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
+}

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"}))
+		})
+	})
 })

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()},
+	}
+)

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()},
+	}
+)

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()},
-	}
-)

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
 	}