Skip to content

fix(sbom): keep the dependency graph intact in container-format product SBOM - #321

Open
reyreavman wants to merge 3 commits into
mainfrom
fix/sbom/product-deps-refs
Open

reyreavman wants to merge 3 commits into
mainfrom
fix/sbom/product-deps-refs

Conversation

@reyreavman

Copy link
Copy Markdown
Collaborator

Summary

werf sbom merge --ispras-format container produced a dependencies section whose refs matched none of the components in the file: components carried the <image>/ namespace prefix, dependencies had been rewritten to purl?package-id=…. The graph was present but unusable, and the ISPRAS checker does not flag dangling refs, so werf sbom validate still passed. Reproduces from any stapel project with packages: os-pm that pulls in a package with dependencies (e.g. curl==8.12.1), merged in container format.

What

  • In a container-format product SBOM every dependencies[].ref and every dependsOn entry resolves to a component in the container tree, using the namespaced <image>/<bom-ref> form.
  • The per-image dependency graphs are concatenated as-is; each container keeps the edges of its own image. Two images with the same package get two distinct subgraphs, matching the two distinct component entries.
  • VERIFIED: on a two-image stapel project (jq and curl via os-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.
  • oss-format output does not change: it still goes through MergeBOMs with ref rewriting and dedup, and its graph was already consistent.
  • The multi_image e2e fixture's backend image now also installs curl==8.12.1, so the lifecycle test has a real graph to assert on; the shared jq/yq set still covers cross-image duplicates.

Why

ContainerAssembler.Assemble builds the result through MergeBOMs, which runs ensureUniqueBOMRefs (rewriting every component ref to purl?package-id=… and remapping dependencies to match) and DedupBOM. 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 rewritten dependencies. On top of that, appendBOMDependencies appends the per-image []cdx.Dependency values whose Dependencies *[]string pointers 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/DedupBOM for the container assembler inside MergeBOMs. Rejected: the assembler already replaces Components wholesale and only needs MergeBOMs for 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.

…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>
@reyreavman

Copy link
Copy Markdown
Collaborator Author

Verification

  • Hand-run on a local registry: two-image stapel project (jq==1.8.1 in a base image, curl==8.12.1 in a child image built fromImage it), werf sbom merge --ispras-format container, then a script over the JSON counting dangling subjects/targets and in-degree, then 3p-ispras-sbom-checker --format container. Before: 3/7 dangling, in-degree 0 everywhere. After: 0/0 dangling, 7 components with in-degree > 0, checker файл корректный.
  • Mutation: imageDependencies snapshot removed (dependencies taken from MergeBOMs again) → ContainerAssembler keeps every dependency ref resolvable to a component fails with dependency target "pkg:deb/debian/openssl@3.0.15?package-id=…" of "app/pkg:deb/debian/curl@8.12.1" has no component.
  • Not run: the e2e annotation-consistency entries locally; relies on CI.

Review focus

  • AssertDependsOn normalises refs by stripping package-id, so in oss format the assertion matches the rewritten ref; in container format the ref carries the backend/ prefix. Confirm the prefix in lifecycle_test.go follows the image name used in the fixture if the fixture is ever renamed.

@reyreavman

Copy link
Copy Markdown
Collaborator Author

CI status

unit, lint, e2e_simple (SBOM suite 109/180 specs, incl. the changed multi-image lifecycle entries) and e2e_complex are green.

e2e_extra fails on three attempts, on runners 1, 2 and 3, with different spec sets each time: staged_dockerfile_run_mount, CMD and ENTRYPOINT combinations (Buildah), Import system dirs, and — once the local registry on the runner went away — every remaining spec with dial tcp 127.0.0.1:32770: connect: connection refused. None of those touch pkg/sbom/ispras. The same job fails today on fix/sbom/explicit-pm-secret-refs, feat/sbom/targeted-dir-scan and on main (run 34590330771), so it is the runner, not this diff. Not retrying further; leaving it for whoever looks at the runners.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant