-
-
Notifications
You must be signed in to change notification settings - Fork 620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support multi module project #1100
Changes from 2 commits
8bf5661
19cd336
588d189
1bba3a6
2f3e8c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -333,12 +333,28 @@ func (gosec *Analyzer) load(pkgPath string, conf *packages.Config) ([]*packages. | |
return []*packages.Package{}, fmt.Errorf("importing dir %q: %w", pkgPath, err) | ||
} | ||
|
||
var packageFiles []string | ||
for _, filename := range basePackage.GoFiles { | ||
packageFiles = append(packageFiles, path.Join(pkgPath, filename)) | ||
var goModFile string | ||
goModDir := abspath[0 : len(abspath)-len(pkgPath)] // get root dir | ||
if os.Getenv("DISABLE_MULTI_MODULE_MODE") != "true" { | ||
if modPkgs, err := packages.Load(&packages.Config{Mode: packages.NeedModule, Dir: abspath}, abspath); err == nil && len(modPkgs) == 1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you run the scan in the workspace folder? There are multiple modules, each module with multiple packages. As I understand here, there will be loaded just one of this modules? Is the right module of the package? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added a message that if run in a |
||
goModFile = modPkgs[0].Module.GoMod | ||
goModDir = path.Dir(goModFile) | ||
} | ||
} | ||
for _, filename := range basePackage.CgoFiles { | ||
packageFiles = append(packageFiles, path.Join(pkgPath, filename)) | ||
|
||
var packageFiles []string | ||
for _, filename := range append(basePackage.GoFiles, basePackage.CgoFiles...) { | ||
if goModDir == "" { | ||
Emptyless marked this conversation as resolved.
Show resolved
Hide resolved
|
||
packageFiles = append(packageFiles, path.Join(pkgPath, filename)) | ||
} else { | ||
filePath := path.Join(abspath, filename) | ||
relPath, err := filepath.Rel(goModDir, filePath) | ||
if err != nil { | ||
gosec.logger.Printf("Skipping: %s. Cannot get relative path between %q and %q: %s", filename, goModDir, filePath, err) | ||
continue | ||
} | ||
packageFiles = append(packageFiles, relPath) | ||
} | ||
} | ||
|
||
if gosec.tests { | ||
|
@@ -354,6 +370,7 @@ func (gosec *Analyzer) load(pkgPath string, conf *packages.Config) ([]*packages. | |
gosec.mu.Lock() | ||
conf.BuildFlags = nil | ||
defer gosec.mu.Unlock() | ||
conf.Dir = goModDir | ||
pkgs, err := packages.Load(conf, packageFiles...) | ||
if err != nil { | ||
return []*packages.Package{}, fmt.Errorf("loading files from package %q: %w", pkgPath, err) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,12 @@ | |
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"log" | ||
"os" | ||
"regexp" | ||
"strings" | ||
"testing" | ||
|
||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
|
@@ -124,6 +126,36 @@ | |
Expect(metrics.NumFiles).To(Equal(2)) | ||
}) | ||
|
||
It("should be able to analyze multiple Go modules", func() { | ||
analyzer.LoadRules(rules.Generate(false).RulesInfo()) | ||
pkg1 := testutils.NewTestPackage() | ||
pkg2 := testutils.NewTestPackage() | ||
defer pkg1.Close() | ||
defer pkg2.Close() | ||
pkg1.AddFile("go.mod", ` | ||
module github.com/securego/gosec/v2/test1 | ||
`) | ||
pkg1.AddFile("foo.go", ` | ||
package main | ||
func main(){ | ||
}`) | ||
pkg2.AddFile("go.mod", ` | ||
module github.com/securego/gosec/v2/test2 | ||
`) | ||
pkg2.AddFile("bar.go", ` | ||
package main | ||
func bar(){ | ||
}`) | ||
err := pkg1.Build() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look like a multi-module project. You are missing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the test was intended to mock a multi-module project without a go.work file, this relies on the published version of a module rather than local filesystem version. This could be the case if within a project it is decided not to commit the |
||
Expect(err).ShouldNot(HaveOccurred()) | ||
err = pkg2.Build() | ||
Expect(err).ShouldNot(HaveOccurred()) | ||
err = analyzer.Process(buildTags, pkg1.Path, pkg2.Path) | ||
Expect(err).ShouldNot(HaveOccurred()) | ||
_, metrics, _ := analyzer.Report() | ||
Expect(metrics.NumFiles).To(Equal(2)) | ||
}) | ||
|
||
It("should find errors when nosec is not in use", func() { | ||
sample := testutils.SampleCodeG401[0] | ||
source := sample.Code[0] | ||
|
@@ -896,3 +928,63 @@ | |
}) | ||
}) | ||
}) | ||
|
||
func BenchmarkWithMultiModuleDisabled(b *testing.B) { | ||
b.Setenv("DISABLE_MULTI_MODULE_MODE", "true") | ||
for _, numFiles := range []int{1, 10, 100, 1000, 5000} { | ||
b.Run(fmt.Sprintf("_%d", numFiles), func(b *testing.B) { | ||
for i := 0; i < b.N; i++ { | ||
benchutil(numFiles, b) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func BenchmarkWithMultiModuleEnabled(b *testing.B) { | ||
for _, numFiles := range []int{1, 10, 100, 1000, 5000} { | ||
b.Run(fmt.Sprintf("_%d", numFiles), func(b *testing.B) { | ||
for i := 0; i < b.N; i++ { | ||
benchutil(numFiles, b) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func benchutil(numFiles int, t testing.TB) { | ||
logger, _ := testutils.NewLogger() | ||
analyzer := gosec.NewAnalyzer(nil, false, false, false, 1, logger) | ||
analyzer.LoadRules(rules.Generate(false).RulesInfo()) | ||
pkg1 := testutils.NewTestPackage() | ||
defer pkg1.Close() | ||
pkg1.AddFile("go.mod", ` | ||
module github.com/securego/gosec/v2/test2 | ||
`) | ||
|
||
pkg1.AddFile("foo.go", ` | ||
package main | ||
func main(){} | ||
`) | ||
|
||
for i := 0; i < numFiles; i++ { | ||
filename := fmt.Sprintf("foo_%d.go", i) | ||
pkg1.AddFile(filename, fmt.Sprintf(` | ||
package main | ||
func SomeFunc%d(){} | ||
`, i)) | ||
} | ||
|
||
err := pkg1.Build() | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
err = analyzer.Process([]string{}, pkg1.Path) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
_, _, errs := analyzer.Report() | ||
if errs != nil && len(errs) != 0 { | ||
Check failure on line 987 in analyzer_test.go GitHub Actions / test (1.21.7, latest)
|
||
t.Fatal(errs) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the multi-module project requires a workspace file? https://go.dev/ref/mod#workspaces
The switch from single module to multi module project is controlled by GOWORK environment variable and this should be already supported inside of Load function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to use the replace directive in a
go.mod
. See: https://go.dev/ref/mod#go-mod-file-replacego.work
was introduced later in go1.18 to simplify multi-module projects but is not mandated iirc