commands/sbom: compose per-runtime and per-extension SBOMs - #238
commands/sbom: compose per-runtime and per-extension SBOMs#238hiagofranco wants to merge 1 commit into
Conversation
ENG-2405 emitted one `software_Sbom` covering every sysroot at once, so no runtime or extension had an artifact of its own. Add one per runtime, per extension, and — with --include-sdk — one for the build host, and attach the runtime's own document to `connect upload`. More elements in the one graph rather than one file each. A package in both rootfs and a runtime is one element with two `contains` edges; across files it becomes two, and a scanner counting packages or matching CVEs counts the same exposure twice. A standalone extension file also could not name its parent runtime without an SPDX `imports` map nothing here emits. `build_document` was already a pure function of the scope slice handed to it, so upload builds a runtime-scoped document by filtering rather than by any new CLI surface. `in_runtime` is the one predicate both that filter and the grouping use. A group is skipped when its own defining scope is empty, not when every member is: an empty scope gets no element, so such a group would be named for something the graph never names. The attach never fails an upload. A build error warns and sends nothing, the field is skipped when absent, and any 4xx retries once without it — gated on the status via a new `HttpStatus` error rather than on message text. Any 4xx and not an enumerated list, because the likeliest refusal is 413: ~400 packages of SPDX is megabytes into a body that is otherwise kilobytes. A 4xx also means the first attempt created nothing, so the retry leaves no duplicate runtime. Both notices use `print_warning_above`, since `print_warning` is suppressed under --output json — the one path where a silently SBOM-less upload would go unnoticed. `AVOCADO_UPLOAD_NO_SBOM=1` skips the build. An `includes:<n>` scope is a remote extension, not a project include, so it gets a document too. It and a legacy `ext:<n>` name no runtime, so no runtime group claims them. The device SBOM stays first in the graph — existing tests take the first `software_Sbom` they find — and now states in prose, not only in `software_sbomType`, what it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f79e9f0 to
658627c
Compare
jetm
left a comment
jetm
left a comment
There was a problem hiding this comment.
Read this with the artifact's consumer in mind rather than the code alone: an SBOM that is quietly incomplete is worse than one that fails to build, because nothing downstream can tell the difference. Four inline comments, and three of them are that shape - a document that looks whole and is not.
The composition mechanics themselves hold up. A package in two extensions collapses to one entry, conflicting versions stay two distinct elements, the slug-collision path is covered by the SHA-256 suffix, and is_client_error survives two .context() layers - I compiled a probe for that last one rather than reading it off the types.
cargo test --all-targets passes, 3277 tests across 15 suites, and cargo fmt --check is clean. cargo clippy -D warnings exits 101 on result_large_err at src/utils/container_dev/registry.rs:424, a file this PR does not touch and which last changed in #184 - pre-existing, not yours.
One thing I did not put inline because it is a judgement rather than a defect: CHANGELOG.md:96-99, already shipped, says of this same document that "the command never transmits it... the operator's call, on a document they can read first." This PR transmits it by default. That may well be the intended change, but the shipped promise now contradicts the behaviour and one of them should move.
Ten smaller notes held back - a doubled HTTP status in one error render, retry firing on any 4xx, and some test gaps around the new grouping. Say the word and I'll add them.
| .into_iter() | ||
| .filter(|s| in_runtime(&s.name, &self.runtime)) | ||
| .collect(); | ||
| Ok(cmd.build_document(&kept, &target, snapshot.as_ref())) |
There was a problem hiding this comment.
uploaded runtime SBOM is named "device SBOM"
scan_runtime_sbom filters scopes with in_runtime and then calls the same build_document, which unconditionally emits "name": "avocado {target} device SBOM" with the comment "Describes what this project installs, and so what runs on the device" (generate.rs:1081-1090), and makes it the sole SpdxDocument.rootElement (generate.rs:1235).
Failure: a project with runtimes dev and prod. avocado connect upload dev attaches a document whose SPDX entry point asserts device-wide coverage while holding only rootfs, initramfs, runtime:dev and ext:dev/*. A consumer traversing rootElement - or taking the first software_Sbom, which is the pattern this PR's own the_device_sbom_is_the_first_one_in_the_graph test pins - CVE-matches a one-runtime inventory as the device inventory, under-reporting every package unique to prod.
The artifact is what a customer consumes, so a name that overstates its scope is worse than a smaller one that is honest about it.
|
|
||
| for scope in scopes { | ||
| // `includes:<n>` is a remote extension, not a project include. | ||
| if (scope.name.starts_with("ext:") || scope.name.starts_with("includes:")) |
There was a problem hiding this comment.
nested-layout remote extensions are dropped from every group
sysroot_scan.rs:85 emits a scope named plainly includes - the shared installroot an avocado-ext-layout(nested) extension installs into (ext_fetch.rs:190,207). But groups() keys on includes: and ext:, and in_runtime matches rootfs, initramfs, runtime: and ext:<rt>/. A bare includes matches neither.
Failure: a project whose remote extensions are nested-layout. Their rpmdb lives only in the shared root, so query_root "includes:<n>" finds no database and drops out (sysroot_scan.rs:50,98-101). Those packages then get no per-extension SBOM, appear in no runtime SBOM, and are filtered out of the uploaded runtime.sbom entirely - a shipped extension missing from the artifact, with nothing on the console to say so.
| let (scopes, target, snapshot) = cmd.scan().await?; | ||
| let kept: Vec<_> = scopes | ||
| .into_iter() | ||
| .filter(|s| in_runtime(&s.name, &self.runtime)) |
There was a problem hiding this comment.
nothing checks the runtime's own scope survived the filter
in_runtime returns true for rootfs and initramfs unconditionally, so kept is non-empty even when no runtime:<name> scope was scanned, or its packages were fully subtracted. build_document then succeeds and build_sbom returns Some with no warning.
Failure: the runtime installroot holds no rpmdb, or its transaction was pure seed - exactly the case a_group_whose_own_scope_is_empty_gets_no_document already pins. groups() emits no Group::Runtime, so Connect stores a document containing not one package that runtime installs, and the operator sees a green upload.
| } | ||
|
|
||
| Ok(()) | ||
| Ok((scopes, target, snapshot)) |
There was a problem hiding this comment.
incomplete-scan warnings are invisible on the JSON path this diff adds
(Anchored at scan()'s return; the two sites are :676 and :684 above.)
scan()'s unreadable-row warning at :676 and seeding_warnings at :684 go out as bare eprintln!, while build_sbom (upload.rs:462) deliberately uses print_warning_above because it also emits {"event":"warning"} on the NDJSON stream.
Failure: avocado connect upload --output json, driven by Avocado Desktop, on a project where three rpm rows carry an embedded tab. An SBOM three packages short is uploaded while the NDJSON stream shows create-runtime: running -> success with no warning event at all.
Worth noting the constraint rather than a straight swap: print_warning_above uses println!, so moving these onto it would corrupt avocado sbom > sbom.json.
2 participants
Summary
ENG-2405 emitted one
software_Sbomcovering every sysroot at once, so noruntime or extension had an artifact of its own. This adds one per runtime,
per extension, and — with
--include-sdk— one for the build host, andattaches the runtime's own document to
connect upload.software_Sbomelements in the one graph, not one file per extension.A package shared between
rootfsand a runtime stays one element with twocontainsedges; split across files it becomes two, and a scanner countingpackages or matching CVEs counts the same exposure twice. A standalone
extension file also could not name its parent runtime without SPDX
importsmachinery nothing here emits.
build_documentwas already a pure function of the scope slice handed toit, so
connect uploadbuilds a runtime-scoped document by filteringrather than through any new CLI surface.
the
sbomfield is skipped when absent (so a server that doesn't read ityet, ENG-2284, sees today's request unchanged), and a 4xx retries once
without it — likeliest cause is 413, since ~400 packages of SPDX is
megabytes into a body that's otherwise kilobytes.
AVOCADO_UPLOAD_NO_SBOM=1skips the build entirely.
includes:<n>scope is a remote extension, not a project include, so itgets its own document too.
first
software_Sbommatch) and now states in prose, not only viasoftware_sbomType, what it describes.Closes ENG-2219.
Test plan
cargo testgreen (1517+9+2+6+8+135 passed; the onecleantestfailure locally is an unrelated podman volume-cleanup race, not
touched by this change)
software_Sbomelement in the graph