Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions pkg/fanal/artifact/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,9 @@ func (a Artifact) inspectLayer(ctx context.Context, layer types.Layer, disabled
defer composite.Cleanup()

// Walk a tar layer
var analyzeErr error
opqDirs, whFiles, err := a.walker.Walk(cr, func(filePath string, info os.FileInfo, opener analyzer.Opener) error {
if err = a.analyzer.AnalyzeFile(egCtx, eg, limit, result, "", filePath, info, opener, disabled, opts); err != nil {
if err := a.analyzer.AnalyzeFile(egCtx, eg, limit, result, "", filePath, info, opener, disabled, opts); err != nil {
return xerrors.Errorf("failed to analyze %s: %w", filePath, err)
}

Expand All @@ -488,13 +489,18 @@ func (a Artifact) inspectLayer(ctx context.Context, layer types.Layer, disabled
return nil
})
if err != nil {
return types.BlobInfo{}, xerrors.Errorf("walk error: %w", err)
analyzeErr = xerrors.Errorf("walk error: %w", err)
}

// Wait for all the goroutine to finish and check errors
// errgroup cancels egCtx when an analysis goroutine fails, so the walk above can
// fail with context.Canceled and mask the real cause (e.g. a remote 429).
// Surface eg.Wait()'s error first; fall back to the walk error only when the group is clean.
if err = eg.Wait(); err != nil {
return types.BlobInfo{}, xerrors.Errorf("analyze error: %w", err)
}
if analyzeErr != nil {
return types.BlobInfo{}, analyzeErr
}

// Post-analysis
if err = a.analyzer.PostAnalyze(ctx, composite, result, opts); err != nil {
Expand Down
12 changes: 9 additions & 3 deletions pkg/fanal/artifact/local/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,23 +212,29 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) {

// Use static paths instead of traversing the filesystem when all analyzers implement StaticPathAnalyzer
// so that we can analyze files faster
var analyzeErr error
if paths, canUseStaticPaths := a.analyzer.StaticPaths(a.artifactOption.DisabledAnalyzers); canUseStaticPaths {
// Analyze files in static paths
a.logger.Debug("Analyzing files in static paths")
if err = a.analyzeWithStaticPaths(egCtx, eg, limit, result, composite, opts, paths); err != nil {
return artifact.Reference{}, xerrors.Errorf("analyze with static paths: %w", err)
analyzeErr = xerrors.Errorf("analyze with static paths: %w", err)
}
} else {
// Analyze files by traversing the root directory
if err = a.analyzeWithRootDir(egCtx, eg, limit, result, composite, opts); err != nil {
return artifact.Reference{}, xerrors.Errorf("analyze with traversal: %w", err)
analyzeErr = xerrors.Errorf("analyze with traversal: %w", err)
}
}

// Wait for all the goroutine to finish.
// errgroup cancels egCtx when an analysis goroutine fails, so the walk above can
// fail with context.Canceled and mask the real cause (e.g. a remote 429).
// Surface eg.Wait()'s error first; fall back to the walk error only when the group is clean.
if err = eg.Wait(); err != nil {
return artifact.Reference{}, xerrors.Errorf("analyze error: %w", err)
}
if analyzeErr != nil {
return artifact.Reference{}, analyzeErr
}

// Post-analysis
if err = a.analyzer.PostAnalyze(ctx, composite, result, opts); err != nil {
Expand Down
41 changes: 41 additions & 0 deletions pkg/fanal/artifact/local/fs_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package local

import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -15,6 +18,7 @@
"github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/fanal/walker"
"github.com/aquasecurity/trivy/pkg/misconf"
trivytypes "github.com/aquasecurity/trivy/pkg/types"
"github.com/aquasecurity/trivy/pkg/uuid"

_ "github.com/aquasecurity/trivy/pkg/fanal/analyzer/config/all"
Expand Down Expand Up @@ -2663,3 +2667,40 @@
})
}
}

// userErrorAnalyzer fails every matching file with a *types.UserError,
// emulating a fatal analyzer error such as a remote Maven 429.
type userErrorAnalyzer struct{}

func (userErrorAnalyzer) Type() analyzer.Type { return "user-error-test" }
func (userErrorAnalyzer) Version() int { return 1 }
func (userErrorAnalyzer) Required(filePath string, _ os.FileInfo) bool {
return strings.HasSuffix(filePath, ".usererror")
}

func (userErrorAnalyzer) Analyze(_ context.Context, _ analyzer.AnalysisInput) (*analyzer.AnalysisResult, error) {
return nil, &trivytypes.UserError{Message: "429 Too Many Requests"}
}

// TestArtifact_Inspect_AnalyzeErrorNotMasked is a regression test for #10790:
// a fatal analyzer error (a remote Maven 429, as *types.UserError) must be
// surfaced, not masked by the context.Canceled the walk hits after egCtx cancels.
func TestArtifact_Inspect_AnalyzeErrorNotMasked(t *testing.T) {
dir := t.TempDir()
// Parallel=1 with several files: the first file cancels egCtx, so a later
// file's semaphore acquire fails with context.Canceled.
for i := 0; i < 10; i++ {

Check failure on line 2692 in pkg/fanal/artifact/local/fs_test.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

rangeint: for loop can be modernized using range over int (modernize)
require.NoError(t, os.WriteFile(
filepath.Join(dir, fmt.Sprintf("%d.usererror", i)), []byte("x"), 0o600))
}

analyzer.RegisterAnalyzer(userErrorAnalyzer{})
t.Cleanup(func() { analyzer.DeregisterAnalyzer("user-error-test") })

a, err := NewArtifact(dir, cache.NewMemoryCache(), walker.NewFS(), artifact.Option{Parallel: 1})
require.NoError(t, err)

_, err = a.Inspect(t.Context())
require.Error(t, err)
assert.ErrorContains(t, err, "429 Too Many Requests")
}
12 changes: 9 additions & 3 deletions pkg/fanal/artifact/vm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ func (a *Storage) Analyze(ctx context.Context, r *io.SectionReader) (types.BlobI
defer composite.Cleanup()

// TODO: Always walk from the root directory. Consider whether there is a need to be able to set optional
var analyzeErr error
err = a.walker.Walk(r, "/", a.artifactOption.WalkerOption, func(filePath string, info os.FileInfo, opener analyzer.Opener) error {
path := strings.TrimPrefix(filePath, "/")
if err = a.analyzer.AnalyzeFile(egCtx, eg, limit, result, "/", path, info, opener, nil, opts); err != nil {
if err := a.analyzer.AnalyzeFile(egCtx, eg, limit, result, "/", path, info, opener, nil, opts); err != nil {
return xerrors.Errorf("analyze file (%s): %w", path, err)
}

Expand All @@ -127,13 +128,18 @@ func (a *Storage) Analyze(ctx context.Context, r *io.SectionReader) (types.BlobI
return nil
})
if err != nil {
return types.BlobInfo{}, xerrors.Errorf("walk vm error: %w", err)
analyzeErr = xerrors.Errorf("walk vm error: %w", err)
}

// Wait for all the goroutine to finish.
// errgroup cancels egCtx when an analysis goroutine fails, so the walk above can
// fail with context.Canceled and mask the real cause (e.g. a remote 429).
// Surface eg.Wait()'s error first; fall back to the walk error only when the group is clean.
if err = eg.Wait(); err != nil {
return types.BlobInfo{}, xerrors.Errorf("analyze error: %w", err)
}
if analyzeErr != nil {
return types.BlobInfo{}, analyzeErr
}

// Post-analysis
if err = a.analyzer.PostAnalyze(ctx, composite, result, opts); err != nil {
Expand Down
Loading