Skip to content

Commit af7bc1f

Browse files
leodidoona-agent
andcommitted
fix: skip vulnerability scanning for packages that failed to build
Root cause: When multiple packages depend on the same package (e.g., api/go:lib), and that package's build fails, the build lock mechanism causes a race condition: 1. Goroutine A obtains the lock and starts building the shared dependency 2. Goroutine B waits on the lock 3. If Goroutine A's build fails, it releases the lock and returns an error 4. Goroutine B wakes up, sees the lock is released, and returns nil (success) 5. The error propagates up through Goroutine A's dependency chain only 6. If the target package's dependency chain goes through Goroutine B, the main build 'succeeds' even though the dependency failed 7. Vulnerability scanning runs and fails because the package isn't in cache This fix: - Passes the package build status map to vulnerability scanning - Only scans packages with status PackageBuilt or PackageDownloaded - Skips packages that failed verification, download, or build - Logs which packages are skipped and why This prevents fatal errors in vulnerability scanning when a dependency build fails in a parallel goroutine. The build will still fail if the dependency error propagates through the main dependency chain, but vulnerability scanning won't cause an additional confusing error. Fixes the issue in gitpod-next PR #11869 where api/go:lib build failed in a parallel goroutine, but the main build continued and vulnerability scanning crashed trying to scan a package that wasn't in cache. Co-authored-by: Ona <no-reply@ona.com>
1 parent 75fa386 commit af7bc1f

File tree

2 files changed

+31
-6
lines changed

2 files changed

+31
-6
lines changed

pkg/leeway/build.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,8 +748,10 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
748748

749749
// Scan all packages for vulnerabilities after the build completes
750750
// This ensures we scan even cached packages that weren't rebuilt
751+
// Only scan packages that were successfully built or downloaded to avoid
752+
// errors when a dependency build failed in a parallel goroutine
751753
if pkg.C.W.SBOM.Enabled && pkg.C.W.SBOM.ScanVulnerabilities {
752-
if err := scanAllPackagesForVulnerabilities(ctx, allpkg); err != nil {
754+
if err := scanAllPackagesForVulnerabilities(ctx, allpkg, pkgstatus); err != nil {
753755
return err
754756
}
755757
}

pkg/leeway/sbom-scan.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ type PackageVulnerabilityStats struct {
4949
// This function is called after the build process completes to identify security issues
5050
// in all built packages, including those loaded from cache. It generates comprehensive
5151
// vulnerability reports in multiple formats and collects statistics across all packages.
52-
func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Package, customOutputDir ...string) error {
52+
//
53+
// Only packages with successful build status (PackageBuilt or PackageDownloaded) are scanned.
54+
// This prevents errors when a dependency build fails in a parallel goroutine but the main
55+
// build continues (due to the build lock mechanism allowing other goroutines to proceed).
56+
func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Package, pkgstatus map[*Package]PackageBuildStatus, customOutputDir ...string) error {
5357
if len(packages) == 0 {
5458
return nil
5559
}
@@ -74,6 +78,15 @@ func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Packa
7478

7579
// Process each package
7680
for _, p := range packages {
81+
// Skip packages that were not successfully built or downloaded
82+
// This can happen when a dependency build fails in a parallel goroutine
83+
// but other goroutines continue due to the build lock mechanism
84+
status := pkgstatus[p]
85+
if status != PackageBuilt && status != PackageDownloaded {
86+
buildctx.Reporter.PackageBuildLog(p, false, []byte(fmt.Sprintf("Skipping vulnerability scan for package %s (status: %s)\n", p.FullName(), status)))
87+
continue
88+
}
89+
7790
if !p.C.W.SBOM.Enabled {
7891
errMsg := fmt.Append(nil, "SBOM feature is disabled, cannot scan for vulnerabilities")
7992
buildctx.Reporter.PackageBuildLog(p, false, errMsg)
@@ -88,9 +101,10 @@ func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Packa
88101

89102
location, exists := buildctx.LocalCache.Location(p)
90103
if !exists {
91-
errMsg := fmt.Appendf(nil, "Package %s not found in local cache, cannot scan for vulnerabilities\n", p.FullName())
92-
buildctx.Reporter.PackageBuildLog(p, false, errMsg)
93-
return xerrors.Errorf(string(errMsg))
104+
// This should not happen since we already filtered by build status,
105+
// but handle it gracefully just in case
106+
buildctx.Reporter.PackageBuildLog(p, false, []byte(fmt.Sprintf("Package %s not found in local cache, skipping vulnerability scan\n", p.FullName())))
107+
continue
94108
}
95109

96110
// Create temporary file for SBOM content
@@ -194,6 +208,8 @@ func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Packa
194208
// ScanAllPackagesForVulnerabilities provides a public API for scanning packages for vulnerabilities.
195209
// It creates a build context with the provided local cache and reporter, then calls the internal
196210
// scanAllPackagesForVulnerabilities function to perform the actual scanning.
211+
//
212+
// This function assumes all provided packages are already built and available in the local cache.
197213
func ScanAllPackagesForVulnerabilities(localCache cache.LocalCache, packages []*Package, customOutputDir ...string) error {
198214
buildctx := &buildContext{
199215
buildOptions: buildOptions{
@@ -202,7 +218,14 @@ func ScanAllPackagesForVulnerabilities(localCache cache.LocalCache, packages []*
202218
},
203219
}
204220

205-
return scanAllPackagesForVulnerabilities(buildctx, packages, customOutputDir...)
221+
// Create a status map marking all packages as built (since this is a public API
222+
// that expects packages to already be in cache)
223+
pkgstatus := make(map[*Package]PackageBuildStatus)
224+
for _, p := range packages {
225+
pkgstatus[p] = PackageBuilt
226+
}
227+
228+
return scanAllPackagesForVulnerabilities(buildctx, packages, pkgstatus, customOutputDir...)
206229
}
207230

208231
// scanSBOMForVulnerabilities scans an SBOM file for vulnerabilities and generates reports.

0 commit comments

Comments
 (0)