fix(sbom): keep the dependency graph intact in container-format product SBOM - #321
Open
reyreavman wants to merge 3 commits into
Open
reyreavman wants to merge 3 commits into
reyreavman wants to merge 3 commits into
Conversation
…ct SBOM The container assembler took components from the namespaced per-image BOMs but dependencies from MergeBOMs, which had already rewritten every ref to a purl+package-id form and mutated the shared slices in place. No dependency subject or target matched a component in the result, so the product graph was unusable while the ISPRAS checker stayed silent. Copy the per-image dependency graphs before the merge and use them as the product graph, so every ref resolves to a namespaced component. Cover it with a unit test on the assembler and assert graph integrity in the multi-image lifecycle e2e, whose fixture now includes a package with real dependencies. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Collaborator
Author
Verification
Review focus
|
Collaborator
Author
CI status
|
GOST upsert and validation walked only metadata.component and the top-level components list, and the container value in the product SBOM was aggregated from the top level alone. A base or imported SBOM with nested components kept its old values, passed validation with missing properties, and could hold a higher value than the container above it, which the ISPRAS checker rejects. Recurse through nested components in Upsert, Validate and aggregateGOST so the whole tree gets the configured values, is checked, and the container reflects the maximum over all descendants. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
…nt too Upsert and Validate descended into bom.Components but not into metadata.component.components, so a BOM whose root component nests its parts — the shape werf itself emits for a container-format product — left those parts untouched and unvalidated. Walk them with the same recursion. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
reyreavman
marked this pull request as ready for review
September 15, 2026 03:58
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
werf sbom merge --ispras-format containerproduced adependenciessection whose refs matched none of the components in the file: components carried the<image>/namespace prefix, dependencies had been rewritten topurl?package-id=…. The graph was present but unusable, and the ISPRAS checker does not flag dangling refs, sowerf sbom validatestill passed. Reproduces from any stapel project withpackages: os-pmthat pulls in a package with dependencies (e.g.curl==8.12.1), merged in container format.What
dependencies[].refand everydependsOnentry resolves to a component in the container tree, using the namespaced<image>/<bom-ref>form.jqandcurlviaos-pm) the merged product has 8 edges, 0 dangling subjects, 0 dangling targets, and the ISPRAS checker reportsфайл корректный; before the change it had 3 dangling subjects and 7 dangling targets on the same input.MergeBOMswith ref rewriting and dedup, and its graph was already consistent.multi_imagee2e fixture'sbackendimage now also installscurl==8.12.1, so the lifecycle test has a real graph to assert on; the sharedjq/yqset still covers cross-image duplicates.Why
ContainerAssembler.Assemblebuilds the result throughMergeBOMs, which runsensureUniqueBOMRefs(rewriting every component ref topurl?package-id=…and remapping dependencies to match) andDedupBOM. It then discards the merged component list and re-nests the original per-image components — still under their<image>/prefixes — inside container nodes, while keeping the rewrittendependencies. On top of that,appendBOMDependenciesappends the per-image[]cdx.Dependencyvalues whoseDependencies *[]stringpointers alias the source slices, so the rewrite also mutated the image BOMs in place; reading them after the merge already returns rewritten targets.Left alone, the product graph is noise in a document customers submit, and any downstream logic that reads in-degree — such as deriving GOST attack-surface roots from the dependency graph, currently under discussion — would see every component as a root.
Alternative considered: skipping
ensureUniqueBOMRefs/DedupBOMfor the container assembler insideMergeBOMs. Rejected: the assembler already replacesComponentswholesale and only needsMergeBOMsfor metadata-level fields, so snapshotting the graph before the merge is the smaller, local change and leaves the merge path used by build-time merging untouched.