fix(cyclonedx): drop dependency edges that reference packages missing from the catalog - #5233
fix(cyclonedx): drop dependency edges that reference packages missing from the catalog#5233pujitha24 wants to merge 2 commits into
Conversation
… from the catalog Motivation: CycloneDX SBOMs generated by syft can contain a "dependencies" entry whose bom-ref points at a component that never appears in the SBOM's "components" list. This happens when an artifact.Relationship survives into formatting even though one of the packages it refers to was filtered out of (or never added to) the final package catalog. CycloneDX validators reject such SBOMs with "Dependency references to Components/Services that are not known in this BOM". Approach: toDependencies() in the CycloneDX encoder now looks up both sides of each relationship in the final package catalog (s.Artifacts.Packages) before emitting a dependency edge for it, dropping any relationship that references a package the catalog doesn't actually contain - the same way relationships with un-encodable types are already dropped. Validation: Added a table test case in Test_relationships that builds an SBOM with only 3 packages in the catalog but a relationship pointing at a 4th package never added to it. Confirmed this test fails on the pre-fix code (the dangling edge is emitted) and passes after the fix (the dangling edge is dropped, the valid edge is kept): go test ./syft/format/common/cyclonedxhelpers/... -run Test_relationships -v go build ./... passes. go test ./syft/format/... passes except for 4 pre-existing test functions that require a local Docker daemon to build image fixtures (unrelated to this change, reproducible identically on the unmodified base commit). make lint (golangci-lint) passes with no findings on the change. Report: anchore#4208 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code)
CAOShurong
left a comment
CAOShurong
left a comment
There was a problem hiding this comment.
Thanks for tightening dependency references against the final package catalog. I found one nil-catalog regression on exact head c6aac5c.
A focused regression with an expressible relationship and no package catalog fails with a nil-pointer panic at this lookup. Adding a nil guard in toDependencies() makes that regression pass 20/20; the complete cyclonedxhelpers package and go vet for the package also pass. Please preserve the existing nil-catalog behavior and add a regression for this boundary.
I used AI assistance to help inspect the change and run the reproducer; I verified the exact diff, failure, and local results before submitting this review.
| // a relationship may reference a package that was filtered out of (or never made it into) the | ||
| // final catalog. Including it here would create a dependency reference to a component that | ||
| // does not exist in the BOM, which fails CycloneDX validation. | ||
| if catalog.Package(fromPkg.ID()) == nil || catalog.Package(toPkg.ID()) == nil { |
There was a problem hiding this comment.
Could we handle a nil catalog before these lookups? Collection.Sorted() is nil-receiver safe, so ToFormatModel() previously tolerated an SBOM with Artifacts.Packages == nil. With any expressible relationship, this new call panics. Returning no dependencies when catalog is nil, with a nil-catalog relationship regression, preserves that boundary.
pkg.Collection.Package() is not nil-receiver safe (unlike Enumerate/Sorted), so the new catalog cross-check introduced in c6aac5c panicked when Artifacts.Packages was nil, a state ToFormatModel previously tolerated. Return no dependencies in that case, matching prior behavior. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
CAOShurong
left a comment
CAOShurong
left a comment
There was a problem hiding this comment.
Follow-up review on exact head 9af36d3: the new entry guard preserves the prior nil-catalog behavior, and the added regression exercises an expressible relationship with Artifacts.Packages == nil. This resolves my blocking finding without weakening the filtered-package check.
Locally, Test_relationships passed 20/20, the complete cyclonedxhelpers package passed, package-scoped go vet passed, and the old-to-new diff check is clean. The public checks for this new head are still being populated, so normal upstream CI and maintainer review remain separate gates.
I used AI assistance to help inspect the changed head and run the verification; I checked the exact diff and results before submitting this approval.
2 participants
Description
Syft's CycloneDX encoder can emit a
dependenciessection that references a component (bom-ref) which does not exist anywhere in the SBOM'scomponentslist. This happens when an internalartifact.Relationshiprefers to a package that was filtered out of (or never made it into) the final package catalog before formatting — the relationship survives, but the component it points to does not. CycloneDX validators (e.g. cyclonedx-python-lib) reject this with:This change makes
toDependencies()in the CycloneDX encoder cross-check both sides of a relationship against the final package catalog (s.Artifacts.Packages) before emitting a dependency edge, dropping any relationship that references a package not present in the catalog — the same way relationships with un-encodable types are already dropped.No CLI output or configuration changes.
Type of change
Checklist
Issue references
Report: #4208
Validation
syft/format/common/cyclonedxhelpers/to_format_model_test.go::Test_relationships/relationships_referencing_a_package_missing_from_the_catalog_are_dropped, which builds an SBOM with only 3 packages in the catalog but a relationship pointing at a 4th package that was never added to it. Verified this test FAILS on the pre-fix code (the dangling dependency edge is emitted) and PASSES after the fix (the dangling edge is dropped, the valid edge is kept).go build ./...passes.go test ./syft/format/...passes except for 4 pre-existing test functions that require a local Docker daemon to build image fixtures (exec: "docker": executable file not found in $PATH) — unrelated to this change and reproducible identically on the unmodified base commit.make lint(golangci-lint) passes with no findings on the change.Note: this fix is scoped to the CycloneDX encoder since that's what the linked issue reports. The SPDX and syft-json encoders have the same unguarded pattern (relationships aren't checked against the final catalog) and may be worth a follow-up, but that's out of scope here to keep this change minimal.
Fixes #4208