From a4b1b73527f85a846d35d687a81082ebd492f482 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Tue, 2 Jul 2024 11:24:38 +0600 Subject: [PATCH 1/5] fix(sbom): use logger with `sbom` prefix --- pkg/sbom/io/decode.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/sbom/io/decode.go b/pkg/sbom/io/decode.go index 7544cf215a3e..c754a3acb1a4 100644 --- a/pkg/sbom/io/decode.go +++ b/pkg/sbom/io/decode.go @@ -186,14 +186,14 @@ func (m *Decoder) decodeApplication(c *core.Component) *ftypes.Application { func (m *Decoder) decodeLibrary(c *core.Component) (*ftypes.Package, error) { p := (*purl.PackageURL)(c.PkgIdentifier.PURL) if p == nil { - log.Debug("Skipping a component without PURL", + m.logger.Debug("Skipping a component without PURL", log.String("name", c.Name), log.String("version", c.Version)) return nil, ErrPURLEmpty } pkg := p.Package() if p.Class() == types.ClassUnknown { - log.Debug("Skipping a component with an unsupported type", + m.logger.Debug("Skipping a component with an unsupported type", log.String("name", c.Name), log.String("version", c.Version), log.String("type", p.Type)) return nil, ErrUnsupportedType } @@ -318,7 +318,7 @@ func (m *Decoder) parseSrcVersion(pkg *ftypes.Package, ver string) { case packageurl.TypeDebian: v, err := debver.NewVersion(ver) if err != nil { - log.Debug("Failed to parse Debian version", log.Err(err)) + m.logger.Debug("Failed to parse Debian version", log.Err(err)) return } pkg.SrcEpoch = v.Epoch() @@ -382,7 +382,7 @@ func (m *Decoder) addOrphanPkgs(sbom *types.SBOM) error { // Add OS packages only when OS is detected. for _, pkgs := range osPkgMap { if sbom.Metadata.OS == nil || !sbom.Metadata.OS.Detected() { - log.Warn("Ignore the OS package as no OS is detected.") + m.logger.Warn("Ignore the OS package as no OS is detected.") break } From 47c0e0bd63f65f23bead60700639ee688864bffe Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Tue, 2 Jul 2024 12:59:42 +0600 Subject: [PATCH 2/5] feat: add paths to sbom files using context --- pkg/fanal/analyzer/sbom/sbom.go | 6 ++-- pkg/fanal/artifact/sbom/sbom.go | 5 ++-- pkg/fanal/handler/unpackaged/unpackaged.go | 3 +- pkg/sbom/cyclonedx/unmarshal_test.go | 5 +++- pkg/sbom/io/decode.go | 33 +++++++++++----------- pkg/sbom/sbom.go | 5 ++-- pkg/sbom/spdx/unmarshal_test.go | 5 +++- 7 files changed, 37 insertions(+), 25 deletions(-) diff --git a/pkg/fanal/analyzer/sbom/sbom.go b/pkg/fanal/analyzer/sbom/sbom.go index 6392f0200da3..9bd2d52d85ab 100644 --- a/pkg/fanal/analyzer/sbom/sbom.go +++ b/pkg/fanal/analyzer/sbom/sbom.go @@ -10,6 +10,7 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/analyzer" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/log" "github.com/aquasecurity/trivy/pkg/sbom" "github.com/aquasecurity/trivy/pkg/types" ) @@ -29,14 +30,15 @@ var requiredSuffixes = []string{ type sbomAnalyzer struct{} -func (a sbomAnalyzer) Analyze(_ context.Context, input analyzer.AnalysisInput) (*analyzer.AnalysisResult, error) { +func (a sbomAnalyzer) Analyze(ctx context.Context, input analyzer.AnalysisInput) (*analyzer.AnalysisResult, error) { // Format auto-detection format, err := sbom.DetectFormat(input.Content) if err != nil { return nil, xerrors.Errorf("failed to detect SBOM format: %w", err) } - bom, err := sbom.Decode(input.Content, format) + ctx = log.WithContextAttrs(ctx, log.String("file", input.FilePath)) + bom, err := sbom.Decode(ctx, input.Content, format) if err != nil { return nil, xerrors.Errorf("SBOM decode error: %w", err) } diff --git a/pkg/fanal/artifact/sbom/sbom.go b/pkg/fanal/artifact/sbom/sbom.go index a5b646c18889..2dad9ddf8607 100644 --- a/pkg/fanal/artifact/sbom/sbom.go +++ b/pkg/fanal/artifact/sbom/sbom.go @@ -37,7 +37,7 @@ func NewArtifact(filePath string, c cache.ArtifactCache, opt artifact.Option) (a }, nil } -func (a Artifact) Inspect(_ context.Context) (artifact.Reference, error) { +func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { f, err := os.Open(a.filePath) if err != nil { return artifact.Reference{}, xerrors.Errorf("failed to open sbom file error: %w", err) @@ -51,7 +51,8 @@ func (a Artifact) Inspect(_ context.Context) (artifact.Reference, error) { } log.Info("Detected SBOM format", log.String("format", string(format))) - bom, err := sbom.Decode(f, format) + ctx = log.WithContextAttrs(ctx, log.String("file", a.filePath)) + bom, err := sbom.Decode(ctx, f, format) if err != nil { return artifact.Reference{}, xerrors.Errorf("SBOM decode error: %w", err) } diff --git a/pkg/fanal/handler/unpackaged/unpackaged.go b/pkg/fanal/handler/unpackaged/unpackaged.go index ed380de49cb0..802db8bfb68b 100644 --- a/pkg/fanal/handler/unpackaged/unpackaged.go +++ b/pkg/fanal/handler/unpackaged/unpackaged.go @@ -64,7 +64,8 @@ func (h unpackagedHook) Handle(ctx context.Context, res *analyzer.AnalysisResult } // Parse the fetched SBOM - bom, err := sbom.Decode(bytes.NewReader(raw), format) + ctx = log.WithContextAttrs(ctx, log.String("file", filePath)) + bom, err := sbom.Decode(ctx, bytes.NewReader(raw), format) if err != nil { return err } diff --git a/pkg/sbom/cyclonedx/unmarshal_test.go b/pkg/sbom/cyclonedx/unmarshal_test.go index 834e886c46dd..6a7f16cc6ea4 100644 --- a/pkg/sbom/cyclonedx/unmarshal_test.go +++ b/pkg/sbom/cyclonedx/unmarshal_test.go @@ -1,7 +1,9 @@ package cyclonedx_test import ( + "context" "encoding/json" + "github.com/aquasecurity/trivy/pkg/log" "os" "testing" @@ -757,7 +759,8 @@ func TestUnmarshaler_Unmarshal(t *testing.T) { require.NoError(t, err) var got types.SBOM - err = sbomio.NewDecoder(cdx.BOM).Decode(&got) + ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile)) + err = sbomio.NewDecoder(cdx.BOM).Decode(ctx, &got) require.NoError(t, err) got.BOM = nil diff --git a/pkg/sbom/io/decode.go b/pkg/sbom/io/decode.go index c754a3acb1a4..379e1af32d52 100644 --- a/pkg/sbom/io/decode.go +++ b/pkg/sbom/io/decode.go @@ -1,6 +1,7 @@ package io import ( + "context" "errors" "slices" "sort" @@ -46,14 +47,14 @@ func NewDecoder(bom *core.BOM) *Decoder { } } -func (m *Decoder) Decode(sbom *types.SBOM) error { +func (m *Decoder) Decode(ctx context.Context, sbom *types.SBOM) error { // Parse the root component if err := m.decodeRoot(sbom); err != nil { return xerrors.Errorf("failed to decode root component: %w", err) } // Parse all components - if err := m.decodeComponents(sbom); err != nil { + if err := m.decodeComponents(ctx, sbom); err != nil { return xerrors.Errorf("failed to decode components: %w", err) } @@ -67,7 +68,7 @@ func (m *Decoder) Decode(sbom *types.SBOM) error { m.addLangPkgs(sbom) // Add remaining packages - if err := m.addOrphanPkgs(sbom); err != nil { + if err := m.addOrphanPkgs(ctx, sbom); err != nil { return xerrors.Errorf("failed to aggregate packages: %w", err) } @@ -109,9 +110,9 @@ func (m *Decoder) decodeRoot(s *types.SBOM) error { return nil } -func (m *Decoder) decodeComponents(sbom *types.SBOM) error { +func (m *Decoder) decodeComponents(ctx context.Context, sbom *types.SBOM) error { onceMultiOSWarn := sync.OnceFunc(func() { - m.logger.Warn("Multiple OS components are not supported, taking the first one and ignoring the rest") + m.logger.WarnContext(ctx, "Multiple OS components are not supported, taking the first one and ignoring the rest") }) for id, c := range m.bom.Components() { @@ -136,7 +137,7 @@ func (m *Decoder) decodeComponents(sbom *types.SBOM) error { // Third-party SBOMs may contain packages in types other than "Library" if c.Type == core.TypeLibrary || c.PkgIdentifier.PURL != nil { - pkg, err := m.decodeLibrary(c) + pkg, err := m.decodeLibrary(ctx, c) if errors.Is(err, ErrUnsupportedType) || errors.Is(err, ErrPURLEmpty) { continue } else if err != nil { @@ -183,17 +184,17 @@ func (m *Decoder) decodeApplication(c *core.Component) *ftypes.Application { return &app } -func (m *Decoder) decodeLibrary(c *core.Component) (*ftypes.Package, error) { +func (m *Decoder) decodeLibrary(ctx context.Context, c *core.Component) (*ftypes.Package, error) { p := (*purl.PackageURL)(c.PkgIdentifier.PURL) if p == nil { - m.logger.Debug("Skipping a component without PURL", + m.logger.DebugContext(ctx, "Skipping a component without PURL", log.String("name", c.Name), log.String("version", c.Version)) return nil, ErrPURLEmpty } pkg := p.Package() if p.Class() == types.ClassUnknown { - m.logger.Debug("Skipping a component with an unsupported type", + m.logger.DebugContext(ctx, "Skipping a component with an unsupported type", log.String("name", c.Name), log.String("version", c.Version), log.String("type", p.Type)) return nil, ErrUnsupportedType } @@ -240,7 +241,7 @@ func (m *Decoder) decodeLibrary(c *core.Component) (*ftypes.Package, error) { } if p.Class() == types.ClassOSPkg { - m.fillSrcPkg(c, pkg) + m.fillSrcPkg(ctx, c, pkg) } return pkg, nil @@ -278,11 +279,11 @@ func (m *Decoder) pkgName(pkg *ftypes.Package, c *core.Component) string { return c.Name } -func (m *Decoder) fillSrcPkg(c *core.Component, pkg *ftypes.Package) { +func (m *Decoder) fillSrcPkg(ctx context.Context, c *core.Component, pkg *ftypes.Package) { if c.SrcName != "" && pkg.SrcName == "" { pkg.SrcName = c.SrcName } - m.parseSrcVersion(pkg, c.SrcVersion) + m.parseSrcVersion(ctx, pkg, c.SrcVersion) // Source info was added from component or properties if pkg.SrcName != "" && pkg.SrcVersion != "" { @@ -305,7 +306,7 @@ func (m *Decoder) fillSrcPkg(c *core.Component, pkg *ftypes.Package) { } // parseSrcVersion parses the version of the source package. -func (m *Decoder) parseSrcVersion(pkg *ftypes.Package, ver string) { +func (m *Decoder) parseSrcVersion(ctx context.Context, pkg *ftypes.Package, ver string) { if ver == "" { return } @@ -318,7 +319,7 @@ func (m *Decoder) parseSrcVersion(pkg *ftypes.Package, ver string) { case packageurl.TypeDebian: v, err := debver.NewVersion(ver) if err != nil { - m.logger.Debug("Failed to parse Debian version", log.Err(err)) + m.logger.DebugContext(ctx, "Failed to parse Debian version", log.Err(err)) return } pkg.SrcEpoch = v.Epoch() @@ -361,7 +362,7 @@ func (m *Decoder) addLangPkgs(sbom *types.SBOM) { // addOrphanPkgs adds orphan packages. // Orphan packages are packages that are not related to any components. -func (m *Decoder) addOrphanPkgs(sbom *types.SBOM) error { +func (m *Decoder) addOrphanPkgs(ctx context.Context, sbom *types.SBOM) error { osPkgMap := make(map[string]ftypes.Packages) langPkgMap := make(map[ftypes.LangType]ftypes.Packages) for _, pkg := range m.pkgs { @@ -382,7 +383,7 @@ func (m *Decoder) addOrphanPkgs(sbom *types.SBOM) error { // Add OS packages only when OS is detected. for _, pkgs := range osPkgMap { if sbom.Metadata.OS == nil || !sbom.Metadata.OS.Detected() { - m.logger.Warn("Ignore the OS package as no OS is detected.") + m.logger.WarnContext(ctx, "Ignore the OS package as no OS is detected.") break } diff --git a/pkg/sbom/sbom.go b/pkg/sbom/sbom.go index 26ae9f9158d1..6a943e3f2822 100644 --- a/pkg/sbom/sbom.go +++ b/pkg/sbom/sbom.go @@ -2,6 +2,7 @@ package sbom import ( "bufio" + "context" "encoding/json" "encoding/xml" "io" @@ -180,7 +181,7 @@ func decodeAttestCycloneDXJSONFormat(r io.ReadSeeker) (Format, bool) { return FormatAttestCycloneDXJSON, true } -func Decode(f io.Reader, format Format) (types.SBOM, error) { +func Decode(ctx context.Context, f io.Reader, format Format) (types.SBOM, error) { var ( v any bom = core.NewBOM(core.Options{}) @@ -227,7 +228,7 @@ func Decode(f io.Reader, format Format) (types.SBOM, error) { } var sbom types.SBOM - if err := sbomio.NewDecoder(bom).Decode(&sbom); err != nil { + if err := sbomio.NewDecoder(bom).Decode(ctx, &sbom); err != nil { return types.SBOM{}, xerrors.Errorf("failed to decode: %w", err) } diff --git a/pkg/sbom/spdx/unmarshal_test.go b/pkg/sbom/spdx/unmarshal_test.go index c68d9f32e654..e073721b3b33 100644 --- a/pkg/sbom/spdx/unmarshal_test.go +++ b/pkg/sbom/spdx/unmarshal_test.go @@ -1,6 +1,7 @@ package spdx_test import ( + "context" "encoding/json" "os" "sort" @@ -11,6 +12,7 @@ import ( "github.com/stretchr/testify/require" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/log" sbomio "github.com/aquasecurity/trivy/pkg/sbom/io" "github.com/aquasecurity/trivy/pkg/sbom/spdx" "github.com/aquasecurity/trivy/pkg/types" @@ -357,7 +359,8 @@ func TestUnmarshaler_Unmarshal(t *testing.T) { } var got types.SBOM - err = sbomio.NewDecoder(v.BOM).Decode(&got) + ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile)) + err = sbomio.NewDecoder(v.BOM).Decode(ctx, &got) require.NoError(t, err) // Not compare BOM From 82c2620c7614761d8d7531f6e0be97c6c372dd53 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Tue, 2 Jul 2024 13:22:20 +0600 Subject: [PATCH 3/5] fix linter error --- pkg/sbom/cyclonedx/unmarshal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sbom/cyclonedx/unmarshal_test.go b/pkg/sbom/cyclonedx/unmarshal_test.go index 6a7f16cc6ea4..431f04d22a89 100644 --- a/pkg/sbom/cyclonedx/unmarshal_test.go +++ b/pkg/sbom/cyclonedx/unmarshal_test.go @@ -3,7 +3,6 @@ package cyclonedx_test import ( "context" "encoding/json" - "github.com/aquasecurity/trivy/pkg/log" "os" "testing" @@ -12,6 +11,7 @@ import ( "github.com/stretchr/testify/require" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/log" "github.com/aquasecurity/trivy/pkg/purl" "github.com/aquasecurity/trivy/pkg/sbom/cyclonedx" sbomio "github.com/aquasecurity/trivy/pkg/sbom/io" From e0b36ca7e2e5c5862b1870bf110c33c5fe203369 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Tue, 2 Jul 2024 15:56:09 +0600 Subject: [PATCH 4/5] refactor(sbom): remove context filepath for tests --- pkg/sbom/cyclonedx/unmarshal_test.go | 4 +--- pkg/sbom/spdx/unmarshal_test.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/sbom/cyclonedx/unmarshal_test.go b/pkg/sbom/cyclonedx/unmarshal_test.go index 431f04d22a89..7008efb8e9eb 100644 --- a/pkg/sbom/cyclonedx/unmarshal_test.go +++ b/pkg/sbom/cyclonedx/unmarshal_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" - "github.com/aquasecurity/trivy/pkg/log" "github.com/aquasecurity/trivy/pkg/purl" "github.com/aquasecurity/trivy/pkg/sbom/cyclonedx" sbomio "github.com/aquasecurity/trivy/pkg/sbom/io" @@ -759,8 +758,7 @@ func TestUnmarshaler_Unmarshal(t *testing.T) { require.NoError(t, err) var got types.SBOM - ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile)) - err = sbomio.NewDecoder(cdx.BOM).Decode(ctx, &got) + err = sbomio.NewDecoder(cdx.BOM).Decode(context.Background(), &got) require.NoError(t, err) got.BOM = nil diff --git a/pkg/sbom/spdx/unmarshal_test.go b/pkg/sbom/spdx/unmarshal_test.go index e073721b3b33..7efcaa87a29e 100644 --- a/pkg/sbom/spdx/unmarshal_test.go +++ b/pkg/sbom/spdx/unmarshal_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" - "github.com/aquasecurity/trivy/pkg/log" sbomio "github.com/aquasecurity/trivy/pkg/sbom/io" "github.com/aquasecurity/trivy/pkg/sbom/spdx" "github.com/aquasecurity/trivy/pkg/types" @@ -359,8 +358,7 @@ func TestUnmarshaler_Unmarshal(t *testing.T) { } var got types.SBOM - ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile)) - err = sbomio.NewDecoder(v.BOM).Decode(ctx, &got) + err = sbomio.NewDecoder(v.BOM).Decode(context.Background(), &got) require.NoError(t, err) // Not compare BOM From cf30a86517daf875e2672382d617c05307c195e9 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Tue, 2 Jul 2024 16:13:28 +0600 Subject: [PATCH 5/5] refactor: use `log.FilePath()` --- pkg/fanal/analyzer/sbom/sbom.go | 2 +- pkg/fanal/artifact/sbom/sbom.go | 2 +- pkg/fanal/handler/unpackaged/unpackaged.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/fanal/analyzer/sbom/sbom.go b/pkg/fanal/analyzer/sbom/sbom.go index 9bd2d52d85ab..55768cff3a9b 100644 --- a/pkg/fanal/analyzer/sbom/sbom.go +++ b/pkg/fanal/analyzer/sbom/sbom.go @@ -37,7 +37,7 @@ func (a sbomAnalyzer) Analyze(ctx context.Context, input analyzer.AnalysisInput) return nil, xerrors.Errorf("failed to detect SBOM format: %w", err) } - ctx = log.WithContextAttrs(ctx, log.String("file", input.FilePath)) + ctx = log.WithContextAttrs(ctx, log.FilePath(input.FilePath)) bom, err := sbom.Decode(ctx, input.Content, format) if err != nil { return nil, xerrors.Errorf("SBOM decode error: %w", err) diff --git a/pkg/fanal/artifact/sbom/sbom.go b/pkg/fanal/artifact/sbom/sbom.go index 2dad9ddf8607..782a32d9b1f8 100644 --- a/pkg/fanal/artifact/sbom/sbom.go +++ b/pkg/fanal/artifact/sbom/sbom.go @@ -51,7 +51,7 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { } log.Info("Detected SBOM format", log.String("format", string(format))) - ctx = log.WithContextAttrs(ctx, log.String("file", a.filePath)) + ctx = log.WithContextAttrs(ctx, log.FilePath(a.filePath)) bom, err := sbom.Decode(ctx, f, format) if err != nil { return artifact.Reference{}, xerrors.Errorf("SBOM decode error: %w", err) diff --git a/pkg/fanal/handler/unpackaged/unpackaged.go b/pkg/fanal/handler/unpackaged/unpackaged.go index ee5f176e7224..07925d9596f0 100644 --- a/pkg/fanal/handler/unpackaged/unpackaged.go +++ b/pkg/fanal/handler/unpackaged/unpackaged.go @@ -64,7 +64,7 @@ func (h unpackagedHook) Handle(ctx context.Context, res *analyzer.AnalysisResult } // Parse the fetched SBOM - ctx = log.WithContextAttrs(ctx, log.String("file", filePath)) + ctx = log.WithContextAttrs(ctx, log.FilePath(filePath)) bom, err := sbom.Decode(ctx, bytes.NewReader(raw), format) if err != nil { return err