Skip to content

feat(sbom): add BOMID field to match packages and decoded BOM components #9597

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+23 −8
Draft

feat(sbom): add BOMID field to match packages and decoded BOM components #9597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
200059b
feat(ftypes): add BOMID field
DmitriyLewen Oct 6, 2025
bda62f9
fix(vex): use BOMID to match vulns and components
DmitriyLewen Oct 6, 2025
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
4 changes: 4 additions & 0 deletions pkg/fanal/types/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"golang.org/x/xerrors"

"github.com/aquasecurity/trivy/pkg/digest"
"github.com/aquasecurity/trivy/pkg/uuid"
)

type Relationship int
Expand Down Expand Up @@ -78,6 +79,7 @@ type PkgIdentifier struct {
UID string `json:",omitempty"` // Calculated by the package struct
PURL *packageurl.PackageURL `json:"-"`
BOMRef string `json:",omitempty"` // For CycloneDX
BOMID uuid.UUID `json:"-"` // Used to match a package with a decoded BOM component
}

// MarshalJSON customizes the JSON encoding of PkgIdentifier.
Expand Down Expand Up @@ -136,6 +138,8 @@ func (id *PkgIdentifier) Match(s string) bool {
}

switch {
case id.BOMID.String() == s:
return true
case id.BOMRef == s:
return true
case id.PURL != nil && id.PURL.String() == s:
Expand Down
1 change: 1 addition & 0 deletions pkg/sbom/io/decode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ func (m *Decoder) decodePackage(ctx context.Context, c *core.Component) (*ftypes
}

pkg.Identifier.BOMRef = c.PkgIdentifier.BOMRef
pkg.Identifier.BOMID = c.ID()
pkg.Licenses = c.Licenses

for _, f := range c.Files {
Expand Down
10 changes: 4 additions & 6 deletions pkg/sbom/io/encode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,13 @@
// Create a lookup map from BOM-Ref to component for efficient vulnerability assignment
// BOM-Ref is used as the key because it's the standard identifier in CycloneDX format
// and is guaranteed to be present in components from CycloneDX SBOMs
components := lo.MapKeys(report.BOM.Components(), func(v *core.Component, _ uuid.UUID) string {
return v.PkgIdentifier.BOMRef
})
components := report.BOM.Components()

for _, result := range report.Results {
// Group newly detected vulnerabilities by their component's BOM-Ref
vulns := make(map[string][]core.Vulnerability)
vulns := make(map[uuid.UUID][]core.Vulnerability)
for _, vuln := range result.Vulnerabilities {
vulns[vuln.PkgIdentifier.BOMRef] = append(vulns[vuln.PkgIdentifier.BOMRef], e.vulnerability(vuln))
vulns[vuln.PkgIdentifier.BOMID] = append(vulns[vuln.PkgIdentifier.BOMID], e.vulnerability(vuln))
}

// Associate vulnerabilities with their corresponding components in the SBOM
Expand All @@ -287,7 +285,7 @@
// This should never happen in proper SBOM rescanning because vulnerabilities
// should only be detected for components that exist in the original SBOM
log.Warn("Skipping vulnerabilities for component not found in SBOM",
log.String("bom-ref", bomRef),
//log.String("bom-ref", bomRef), TODO create new log!!!

Check failure on line 288 in pkg/sbom/io/encode.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest) 

commentFormatting: put a space between `//` and comment text (gocritic)
log.Int("vulnerabilities", len(componentVulns)))
continue
}
Expand Down
16 changes: 14 additions & 2 deletions pkg/vex/vex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package vex

import (
"cmp"
"context"
"errors"

Expand Down Expand Up @@ -145,12 +146,23 @@ func (c *Client) NotAffected(vuln types.DetectedVulnerability, product, subCompo
}

func filterVulnerabilities(result *types.Result, bom *core.BOM, fn NotAffected) {
// We use one of two possible options as a key in the map for components:
// 1. UID - Used for all components, except when reusing a scanned SBOM file.
// 2. BOMID - UUID of the decoded `core BOM` component.
//
// Key selection rules:
// - In "sbom" mode (`trivy sbom ...`):
// * Components always contain only BOMID.
// * Vulnerabilities contain both BOMID and UID.
// - In all other modes:
// * Both components and vulnerabilities contain only UID.

components := lo.MapEntries(bom.Components(), func(_ uuid.UUID, component *core.Component) (string, *core.Component) {
return component.PkgIdentifier.UID, component
return cmp.Or(component.ID().String(), component.PkgIdentifier.UID), component
Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main problem in #9593 is that the decoded BOM component (from a reused BOM) doesn’t have a UID field.

I tried to fix this in this package, but we run into the following scenarios:

mode component fields vulnerability fields
fs mode UID + PURL UID + PURL
scan CycloneDX file UID + bom-ref + PURL UID + bom-ref + PURL
scan SPDX file PURL UID + PURL

There are two problems:

  • We can’t define a consistent matching order, so in some cases we have to check the components map up to three times.
  • We have to rely on PURL for SPDX, which isn’t ideal because multiple packages can share the same PURL.

I thought about adding a new field for SPDX-ID, but in that case there would still be a question of how to efficiently check the components map.

So I chose this approach instead.
BOMID is used only for decoded components, which allows us to reliably match components with vulnerabilities/packages.

@knqyf263 let me know what you think. If this looks good, I’ll go ahead and update the tests, comments, logs, etc.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found another approach:
we can calculate the UID after decoding the package and then update the component with this UID.
Please take a look at this branch: https://github.com/DmitriyLewen/trivy/tree/fix/add-uid-for-decoded-bom-components

Also, we can use the UID to match components with vulnerabilities.

Copy link
Collaborator

@knqyf263 knqyf263 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can calculate the UID after decoding the package and then update the component with this UID.

After reading the content of the issue (and before reading this PR), I was just about to propose exactly this change. I think it’s generally good, but wouldn’t it be better to insert the UID inside decodePackage?

pkg.Identifier.UID = dependency.UID(pkg.FilePath, *pkg)
return pkg, nil

https://github.com/DmitriyLewen/trivy/blob/3cf4b7936646db692947085540e9cd65948fa7a4/pkg/sbom/io/decode.go#L251

// Overwrite component with UID for package
c.PkgIdentifier.UID = pkg.Identifier.UID
m.bom.AddComponent(c)

Since the returned package will be stored in m.pkgs, it might be better to save the UID before that point.

Copy link
Collaborator

@knqyf263 knqyf263 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I planned to add BOMID or something similar in the future. It may be a good idea to do that now.

Copy link
Collaborator

@knqyf263 knqyf263 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make a quick PoC—just a second.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that too.
IIUC, we'll need to revert some changes:

  1. aff03eb#diff-bda89d30f6e4b3fac0df2f83407739dc83f36828c830d5860fa5b9cf61083260
  2. aff03eb#diff-bda89d30f6e4b3fac0df2f83407739dc83f36828c830d5860fa5b9cf61083260L288-L291 (i am not sure about this)

This means we’ll need to update the logic for the root component in two places (for the reusable BOM and for the VEX case).
It shouldn’t be difficult, but it’s something we need to keep in mind.

Copy link
Collaborator

@knqyf263 knqyf263 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File selection in GitHub commit URLs doesn’t navigate properly (is it just my browser?), so I couldn’t see which file you were referring to. Since we only need the tree in VEX, I thought it would be fine to regenerate it, but is there any issue with that?

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(is it just my browser?)

same for me...

I wrote about these changes (encode.go file):
изображение
изображение

UPD:
IIUC we need to reuse the root component since there might be a case where the root component (from the scanned sbom) uses a different purl.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to write a test to demonstrate my thoughts, but it looks like this is a very hard-to-reproduce case and probably doesn’t occur in normal usage.
So for now, I’ll apply your suggested changes and create another PR.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created #9604

})

result.Vulnerabilities = lo.Filter(result.Vulnerabilities, func(vuln types.DetectedVulnerability, _ int) bool {
c, ok := components[vuln.PkgIdentifier.UID]
c, ok := components[cmp.Or(vuln.PkgIdentifier.BOMID.String(), vuln.PkgIdentifier.UID)]
if !ok {
log.Error("Component not found", log.String("uid", vuln.PkgIdentifier.UID))
return true // Should never reach here
Expand Down
Loading