From ce054792fab7c29437cce687cf2a3fccc333fdeb Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 21 Nov 2024 15:47:54 +0600 Subject: [PATCH 1/6] feat(gomod): construct deps for root pkg in parser --- pkg/dependency/parser/golang/mod/parse.go | 45 ++++++-- .../parser/golang/mod/parse_test.go | 105 ++++++++++-------- .../parser/golang/mod/parse_testcase.go | 66 ++++++++++- 3 files changed, 150 insertions(+), 66 deletions(-) diff --git a/pkg/dependency/parser/golang/mod/parse.go b/pkg/dependency/parser/golang/mod/parse.go index ddcd2ccc880e..e22f12fd957d 100644 --- a/pkg/dependency/parser/golang/mod/parse.go +++ b/pkg/dependency/parser/golang/mod/parse.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "regexp" + "sort" "strconv" "strings" @@ -101,17 +102,6 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc } } - // Main module - if m := modFileParsed.Module; m != nil { - pkgs[m.Mod.Path] = ftypes.Package{ - ID: packageID(m.Mod.Path, m.Mod.Version), - Name: m.Mod.Path, - Version: m.Mod.Version, - ExternalReferences: p.GetExternalRefs(m.Mod.Path), - Relationship: ftypes.RelationshipRoot, - } - } - // Required modules for _, require := range modFileParsed.Require { // Skip indirect dependencies less than Go 1.17 @@ -163,7 +153,38 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc } } - return lo.Values(pkgs), nil, nil + var deps ftypes.Dependencies + // Main module + if m := modFileParsed.Module; m != nil { + root := ftypes.Package{ + ID: packageID(m.Mod.Path, m.Mod.Version), + Name: m.Mod.Path, + Version: m.Mod.Version, + ExternalReferences: p.GetExternalRefs(m.Mod.Path), + Relationship: ftypes.RelationshipRoot, + } + + // Store child dependencies for the root package (main module). + // We will build a dependency graph for Direct/Indirect in `fanal` using additional files. + dependsOn := lo.FilterMap(lo.Values(pkgs), func(pkg ftypes.Package, _ int) (string, bool) { + return pkg.ID, pkg.Relationship == ftypes.RelationshipDirect + }) + + if len(dependsOn) > 0 { + sort.Strings(dependsOn) + deps = append(deps, ftypes.Dependency{ + ID: root.ID, + DependsOn: dependsOn, + }) + } + + pkgs[root.Name] = root + } + + pkgSlice := lo.Values(pkgs) + sort.Sort(ftypes.Packages(pkgSlice)) + + return pkgSlice, deps, nil } // lessThan checks if the Go version is less than `.` diff --git a/pkg/dependency/parser/golang/mod/parse_test.go b/pkg/dependency/parser/golang/mod/parse_test.go index 10bda7f01144..5d407f57081f 100644 --- a/pkg/dependency/parser/golang/mod/parse_test.go +++ b/pkg/dependency/parser/golang/mod/parse_test.go @@ -2,7 +2,6 @@ package mod import ( "os" - "sort" "testing" "github.com/stretchr/testify/assert" @@ -18,74 +17,86 @@ func TestParse(t *testing.T) { file string replace bool useMinVersion bool - want []ftypes.Package + wantPkgs []ftypes.Package + wantDeps []ftypes.Dependency }{ { name: "normal with stdlib", file: "testdata/normal/go.mod", replace: true, useMinVersion: true, - want: GoModNormal, + wantPkgs: GoModNormal, + wantDeps: GoModNormalDeps, }, { - name: "normal", - file: "testdata/normal/go.mod", - replace: true, - want: GoModNormalWithoutStdlib, + name: "normal", + file: "testdata/normal/go.mod", + replace: true, + wantPkgs: GoModNormalWithoutStdlib, + wantDeps: GoModNormalWithoutStdlibDeps, }, { - name: "without go version", - file: "testdata/no-go-version/gomod", - replace: true, - want: GoModNoGoVersion, + name: "without go version", + file: "testdata/no-go-version/gomod", + replace: true, + wantPkgs: GoModNoGoVersion, + wantDeps: defaultGoDepParserDeps, }, { - name: "replace", - file: "testdata/replaced/go.mod", - replace: true, - want: GoModReplaced, + name: "replace", + file: "testdata/replaced/go.mod", + replace: true, + wantPkgs: GoModReplaced, + wantDeps: GoModReplacedDeps, }, { - name: "no replace", - file: "testdata/replaced/go.mod", - replace: false, - want: GoModUnreplaced, + name: "no replace", + file: "testdata/replaced/go.mod", + replace: false, + wantPkgs: GoModUnreplaced, + wantDeps: GoModUnreplacedDeps, }, { - name: "replace with version", - file: "testdata/replaced-with-version/go.mod", - replace: true, - want: GoModReplacedWithVersion, + name: "replace with version", + file: "testdata/replaced-with-version/go.mod", + replace: true, + wantPkgs: GoModReplacedWithVersion, + wantDeps: GoModReplacedWithVersionDeps, }, { - name: "replaced with version mismatch", - file: "testdata/replaced-with-version-mismatch/go.mod", - replace: true, - want: GoModReplacedWithVersionMismatch, + name: "replaced with version mismatch", + file: "testdata/replaced-with-version-mismatch/go.mod", + replace: true, + wantPkgs: GoModReplacedWithVersionMismatch, + wantDeps: defaultGoDepParserDeps, }, { - name: "replaced with local path", - file: "testdata/replaced-with-local-path/go.mod", - replace: true, - want: GoModReplacedWithLocalPath, + name: "replaced with local path", + file: "testdata/replaced-with-local-path/go.mod", + replace: true, + wantPkgs: GoModReplacedWithLocalPath, + wantDeps: defaultGoDepParserDeps, }, { - name: "replaced with local path and version", - file: "testdata/replaced-with-local-path-and-version/go.mod", - replace: true, - want: GoModReplacedWithLocalPathAndVersion, + name: "replaced with local path and version", + file: "testdata/replaced-with-local-path-and-version/go.mod", + replace: true, + wantPkgs: GoModReplacedWithLocalPathAndVersion, + wantDeps: defaultGoDepParserDeps, }, { - name: "replaced with local path and version, mismatch", - file: "testdata/replaced-with-local-path-and-version-mismatch/go.mod", - replace: true, - want: GoModReplacedWithLocalPathAndVersionMismatch, + name: "replaced with local path and version, mismatch", + file: "testdata/replaced-with-local-path-and-version-mismatch/go.mod", + replace: true, + wantPkgs: GoModReplacedWithLocalPathAndVersionMismatch, + wantDeps: defaultGoDepParserDeps, }, { - name: "go 1.16", - file: "testdata/go116/go.mod", - replace: true, - want: GoMod116, + name: "go 1.16", + file: "testdata/go116/go.mod", + replace: true, + wantPkgs: GoMod116, + wantDeps: defaultGoDepParserDeps, }, } @@ -94,13 +105,11 @@ func TestParse(t *testing.T) { f, err := os.Open(tt.file) require.NoError(t, err) - got, _, err := NewParser(tt.replace, tt.useMinVersion).Parse(f) + gotPkgs, gotDeps, err := NewParser(tt.replace, tt.useMinVersion).Parse(f) require.NoError(t, err) - sort.Sort(ftypes.Packages(got)) - sort.Sort(ftypes.Packages(tt.want)) - - assert.Equal(t, tt.want, got) + assert.Equal(t, tt.wantPkgs, gotPkgs) + assert.Equal(t, tt.wantDeps, gotDeps) }) } } diff --git a/pkg/dependency/parser/golang/mod/parse_testcase.go b/pkg/dependency/parser/golang/mod/parse_testcase.go index b8ed49008926..70a163c50516 100644 --- a/pkg/dependency/parser/golang/mod/parse_testcase.go +++ b/pkg/dependency/parser/golang/mod/parse_testcase.go @@ -20,12 +20,6 @@ var ( }, }, }, - { - ID: "stdlib@v1.22.5", - Name: "stdlib", - Version: "v1.22.5", - Relationship: ftypes.RelationshipDirect, - }, { ID: "github.com/aquasecurity/go-version@v0.0.0-20240603093900-cf8a8d29271d", Name: "github.com/aquasecurity/go-version", @@ -38,6 +32,12 @@ var ( }, }, }, + { + ID: "stdlib@v1.22.5", + Name: "stdlib", + Version: "v1.22.5", + Relationship: ftypes.RelationshipDirect, + }, { ID: "github.com/davecgh/go-spew@v1.1.2-0.20180830191138-d8f796af33cc", Name: "github.com/davecgh/go-spew", @@ -82,10 +82,29 @@ var ( }, } + GoModNormalDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-version@v0.0.0-20240603093900-cf8a8d29271d", + "stdlib@v1.22.5", + }, + }, + } + GoModNormalWithoutStdlib = slices.DeleteFunc(slices.Clone(GoModNormal), func(f ftypes.Package) bool { return f.Name == "stdlib" }) + GoModNormalWithoutStdlibDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-version@v0.0.0-20240603093900-cf8a8d29271d", + }, + }, + } + // execute go mod tidy in replaced folder GoModReplaced = []ftypes.Package{ { @@ -118,6 +137,14 @@ var ( Relationship: ftypes.RelationshipIndirect, }, } + GoModReplacedDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20220406074731-71021a481237", + }, + }, + } // execute go mod tidy in replaced folder GoModUnreplaced = []ftypes.Package{ @@ -152,6 +179,15 @@ var ( }, } + GoModUnreplacedDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20211110174639-8257534ffed3", + }, + }, + } + // execute go mod tidy in replaced-with-version folder GoModReplacedWithVersion = []ftypes.Package{ { @@ -185,6 +221,15 @@ var ( }, } + GoModReplacedWithVersionDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20220406074731-71021a481237", + }, + }, + } + // execute go mod tidy in replaced-with-version-mismatch folder GoModReplacedWithVersionMismatch = []ftypes.Package{ { @@ -230,6 +275,15 @@ var ( }, } + defaultGoDepParserDeps = ftypes.Dependencies{ + { + ID: "github.com/org/repo", + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20211224170007-df43bca6b6ff", + }, + }, + } + // execute go mod tidy in replaced-with-local-path folder GoModReplacedWithLocalPath = []ftypes.Package{ { From 2334f56a80fb2368aa8668d78625172083eccab5 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 21 Nov 2024 15:57:55 +0600 Subject: [PATCH 2/6] test: update broken tests --- pkg/fanal/analyzer/language/golang/mod/mod_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/fanal/analyzer/language/golang/mod/mod_test.go b/pkg/fanal/analyzer/language/golang/mod/mod_test.go index 3963bcebbad9..d0608c3a1130 100644 --- a/pkg/fanal/analyzer/language/golang/mod/mod_test.go +++ b/pkg/fanal/analyzer/language/golang/mod/mod_test.go @@ -36,6 +36,9 @@ func Test_gomodAnalyzer_Analyze(t *testing.T) { ID: "github.com/org/repo", Name: "github.com/org/repo", Relationship: types.RelationshipRoot, + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20220406074731-71021a481237", + }, ExternalReferences: []types.ExternalRef{ { Type: types.RefVCS, @@ -86,6 +89,9 @@ func Test_gomodAnalyzer_Analyze(t *testing.T) { ID: "github.com/org/repo", Name: "github.com/org/repo", Relationship: types.RelationshipRoot, + DependsOn: []string{ + "github.com/sad/sad@v0.0.1", + }, ExternalReferences: []types.ExternalRef{ { Type: types.RefVCS, @@ -126,6 +132,9 @@ func Test_gomodAnalyzer_Analyze(t *testing.T) { ID: "github.com/org/repo", Name: "github.com/org/repo", Relationship: types.RelationshipRoot, + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20230219131432-590b1dfb6edd", + }, ExternalReferences: []types.ExternalRef{ { Type: types.RefVCS, @@ -178,6 +187,9 @@ func Test_gomodAnalyzer_Analyze(t *testing.T) { ID: "github.com/org/repo", Name: "github.com/org/repo", Relationship: types.RelationshipRoot, + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v0.0.0-20230219131432-590b1dfb6edd", + }, ExternalReferences: []types.ExternalRef{ { Type: types.RefVCS, From 29d73fba13785a826e477e33c64e08c450073425 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Thu, 21 Nov 2024 17:13:48 +0600 Subject: [PATCH 3/6] refactor: don't check len(dependsOn) --- pkg/dependency/parser/golang/mod/parse.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/dependency/parser/golang/mod/parse.go b/pkg/dependency/parser/golang/mod/parse.go index e22f12fd957d..f108b2101890 100644 --- a/pkg/dependency/parser/golang/mod/parse.go +++ b/pkg/dependency/parser/golang/mod/parse.go @@ -170,13 +170,11 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc return pkg.ID, pkg.Relationship == ftypes.RelationshipDirect }) - if len(dependsOn) > 0 { - sort.Strings(dependsOn) - deps = append(deps, ftypes.Dependency{ - ID: root.ID, - DependsOn: dependsOn, - }) - } + sort.Strings(dependsOn) + deps = append(deps, ftypes.Dependency{ + ID: root.ID, + DependsOn: dependsOn, + }) pkgs[root.Name] = root } From 795f9139f4f0a84c4b21a342a3d173eadead7915 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Fri, 22 Nov 2024 14:08:50 +0400 Subject: [PATCH 4/6] feat: handle orphan indirect depenencies Signed-off-by: knqyf263 --- pkg/fanal/analyzer/language/golang/mod/mod.go | 37 +++++++++++ .../analyzer/language/golang/mod/mod_test.go | 63 +++++++++++++++++++ .../golang/mod/testdata/no-pkg-found/mod | 10 +++ 3 files changed, 110 insertions(+) create mode 100644 pkg/fanal/analyzer/language/golang/mod/testdata/no-pkg-found/mod diff --git a/pkg/fanal/analyzer/language/golang/mod/mod.go b/pkg/fanal/analyzer/language/golang/mod/mod.go index 52d7b32f3bee..bb6117f3a100 100644 --- a/pkg/fanal/analyzer/language/golang/mod/mod.go +++ b/pkg/fanal/analyzer/language/golang/mod/mod.go @@ -101,6 +101,9 @@ func (a *gomodAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalys a.logger.Warn("Unable to collect additional info", log.Err(err)) } + // Add orphan indirect dependencies under the main module + a.addOrphanIndirectDepsUnderRoot(apps) + return &analyzer.AnalysisResult{ Applications: apps, }, nil @@ -212,6 +215,40 @@ func (a *gomodAnalyzer) collectDeps(modDir, pkgID string) (types.Dependency, err }, nil } +// addOrphanIndirectDepsUnderRoot handles indirect dependencies that have no identifiable parent packages in the dependency tree. +// This situation can occur when: +// - $GOPATH/pkg directory doesn't exist +// - Module cache is incomplete +// - etc. +// +// In such cases, indirect packages become "orphaned" - they exist in the dependency list +// but have no connection to the dependency tree. This function resolves this issue by: +// 1. Finding the root (main) module +// 2. Identifying all indirect dependencies that have no parent packages +// 3. Adding these orphaned indirect dependencies under the main module +// +// This ensures that all packages remain visible in the dependency tree, even when the complete +// dependency chain cannot be determined. +func (a *gomodAnalyzer) addOrphanIndirectDepsUnderRoot(apps []types.Application) { + for _, app := range apps { + // Find the main module + _, rootIdx, found := lo.FindIndexOf(app.Packages, func(pkg types.Package) bool { + return pkg.Relationship == types.RelationshipRoot + }) + if !found { + continue + } + + // Collect all orphan indirect dependencies that are unable to identify parents + parents := app.Packages.ParentDeps() + orphanDeps := lo.FilterMap(app.Packages, func(pkg types.Package, _ int) (string, bool) { + return pkg.ID, pkg.Relationship == types.RelationshipIndirect && len(parents[pkg.ID]) == 0 + }) + // Add orphan indirect dependencies under the main module + app.Packages[rootIdx].DependsOn = append(app.Packages[rootIdx].DependsOn, orphanDeps...) + } +} + func parse(fsys fs.FS, path string, parser language.Parser) (*types.Application, error) { f, err := fsys.Open(path) if err != nil { diff --git a/pkg/fanal/analyzer/language/golang/mod/mod_test.go b/pkg/fanal/analyzer/language/golang/mod/mod_test.go index d0608c3a1130..c2e0370d172a 100644 --- a/pkg/fanal/analyzer/language/golang/mod/mod_test.go +++ b/pkg/fanal/analyzer/language/golang/mod/mod_test.go @@ -116,6 +116,69 @@ func Test_gomodAnalyzer_Analyze(t *testing.T) { }, }, }, + { + name: "no pkg dir found", + files: []string{ + "testdata/no-pkg-found/mod", + }, + want: &analyzer.AnalysisResult{ + Applications: []types.Application{ + { + Type: types.GoModule, + FilePath: "go.mod", + Packages: types.Packages{ + { + ID: "github.com/org/repo", + Name: "github.com/org/repo", + Relationship: types.RelationshipRoot, + DependsOn: []string{ + "github.com/aquasecurity/go-dep-parser@v1.0.0", + "github.com/aquasecurity/go-version@v1.0.1", + "golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1", // No parent found, so it's added here. + }, + ExternalReferences: []types.ExternalRef{ + { + Type: types.RefVCS, + URL: "https://github.com/org/repo", + }, + }, + }, + { + ID: "github.com/aquasecurity/go-dep-parser@v1.0.0", + Name: "github.com/aquasecurity/go-dep-parser", + Version: "v1.0.0", + Relationship: types.RelationshipDirect, + ExternalReferences: []types.ExternalRef{ + { + Type: types.RefVCS, + URL: "https://github.com/aquasecurity/go-dep-parser", + }, + }, + }, + { + ID: "github.com/aquasecurity/go-version@v1.0.1", + Name: "github.com/aquasecurity/go-version", + Version: "v1.0.1", + Relationship: types.RelationshipDirect, + ExternalReferences: []types.ExternalRef{ + { + Type: types.RefVCS, + URL: "https://github.com/aquasecurity/go-version", + }, + }, + }, + { + ID: "golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1", + Name: "golang.org/x/xerrors", + Version: "v0.0.0-20200804184101-5ec99f83aff1", + Relationship: types.RelationshipIndirect, + Indirect: true, + }, + }, + }, + }, + }, + }, { name: "less than 1.17", files: []string{ diff --git a/pkg/fanal/analyzer/language/golang/mod/testdata/no-pkg-found/mod b/pkg/fanal/analyzer/language/golang/mod/testdata/no-pkg-found/mod new file mode 100644 index 000000000000..2f64bb82f7a8 --- /dev/null +++ b/pkg/fanal/analyzer/language/golang/mod/testdata/no-pkg-found/mod @@ -0,0 +1,10 @@ +module github.com/org/repo + +go 1.23 + +require ( + github.com/aquasecurity/go-dep-parser v1.0.0 + github.com/aquasecurity/go-version v1.0.1 +) + +require golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect \ No newline at end of file From ef467df511b25b8fdd4d633c43f678bd5d638d57 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Fri, 22 Nov 2024 15:06:42 +0400 Subject: [PATCH 5/6] fix(vex): add fail-safe Signed-off-by: knqyf263 --- pkg/vex/vex.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/vex/vex.go b/pkg/vex/vex.go index e9ad15233b04..827285295c6b 100644 --- a/pkg/vex/vex.go +++ b/pkg/vex/vex.go @@ -181,6 +181,11 @@ func reachRoot(leaf *core.Component, components map[uuid.UUID]*core.Component, p return false } else if c.Root { return true + } else if len(parents[c.ID()]) == 0 { + // Should never reach here as all components other than the root should have at least one parent. + // If it does, it means the component tree is not connected due to a bug in the SBOM generation. + // In this case, so as not to filter out all the vulnerabilities accidentally, return true for fail-safe. + return true } visited[c.ID()] = true From af615d1c127fda1dfbd4289f9a7304d991590c26 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Fri, 22 Nov 2024 15:45:10 +0400 Subject: [PATCH 6/6] fix: lint issues Signed-off-by: knqyf263 --- pkg/vex/vex.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/vex/vex.go b/pkg/vex/vex.go index 827285295c6b..fa3f3151340e 100644 --- a/pkg/vex/vex.go +++ b/pkg/vex/vex.go @@ -177,11 +177,12 @@ func reachRoot(leaf *core.Component, components map[uuid.UUID]*core.Component, p var dfs func(c *core.Component) bool dfs = func(c *core.Component) bool { // Call the function with the current component and the leaf component - if notAffected(c, leaf) { + switch { + case notAffected(c, leaf): return false - } else if c.Root { + case c.Root: return true - } else if len(parents[c.ID()]) == 0 { + case len(parents[c.ID()]) == 0: // Should never reach here as all components other than the root should have at least one parent. // If it does, it means the component tree is not connected due to a bug in the SBOM generation. // In this case, so as not to filter out all the vulnerabilities accidentally, return true for fail-safe.