DLPX-98872 linux-pkg: generate per-package CycloneDX SBOM via Syft deb scan and publish S3 sidecar - #418
Draft
justsanjeev wants to merge 8 commits into
Draft
Conversation
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/721d0991-92c3-46ab-a792-d34028a44ae9
branch
from
September 9, 2026 08:07
d617ba0 to
bbde65e
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/721d0991-92c3-46ab-a792-d34028a44ae9
branch
2 times, most recently
from
September 9, 2026 09:55
4153069 to
2623916
Compare
…b scan and publish S3 sidecar PR URL: https://www.github.com/delphix/linux-pkg/pull/418
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/721d0991-92c3-46ab-a792-d34028a44ae9
branch
from
September 9, 2026 10:03
2623916 to
ee451b2
Compare
There was a problem hiding this comment.
🟡 Changes recommended
packages/docker-python-image/config.sh sets PACKAGE_NEEDS_DOCKER="false" while its own comment indicates the build runs docker pull, which will likely fail without the docker socket mount.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds per-package, opt-in CycloneDX SBOM “sidecars” generated from built .deb artifacts (via Syft), plus CI enforcement that every package is explicitly classified for deep scanning.
Changes:
- Introduces
SBOM_DEEP_SCANas a per-package classification flag and exposes it viaquery-packages.sh. - Adds
generate_sbom()stage to build flow to scan.deboutputs with Syft and (if needed) merge/validate viacyclonedx-cli. - Adds a GitHub Actions lint to fail CI if any package lacks an explicit
SBOM_DEEP_SCANclassification.
File summaries
| File | Description |
|---|---|
| query-packages.sh | Adds sbom-deep-scan output field to surface SBOM_DEEP_SCAN values. |
| buildpkg.sh | Wires new generate_sbom stage into the package build pipeline. |
| lib/common.sh | Implements generate_sbom() (Syft scan, optional merge, schema validation). |
| docs/specs/2026-09-08-sbom-per-package-sidecar-design.md | Adds design/spec documenting per-package sidecar generation approach. |
| .github/workflows/main.yml | Adds CI job to enforce SBOM_DEEP_SCAN classification. |
| .github/scripts/verify-sbom-scan-flag.sh | New lint script ensuring all packages set SBOM_DEEP_SCAN to true/false. |
| packages/zfs/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/windows-connector/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/virtualization/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/targetcli-fb/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/syft/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/sdb/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/savedump/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/python-rtslib-fb/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/ptools/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/performance-diagnostics/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/nfs-utils/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/misc-debs/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/masking/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/makedumpfile/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/linux-kernel-oracle/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/linux-kernel-generic/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/linux-kernel-gcp/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/linux-kernel-azure/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/linux-kernel-aws/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/libkdumpfile/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/host-jdks/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/grub2/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/gdb-python/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/fluentd-gems/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/dwarves/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/drgn/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/docker-python-image/config.sh | Adds SBOM classification and changes docker-socket requirement flag. |
| packages/delphix-sso-app/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/delphix-rust/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/delphix-platform/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/delphix-kernel/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/delphix-go/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/cyclonedx-cli/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/crypt-blowfish/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/crash-python/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/containerized-masking/config.sh | Classifies package for deep SBOM scan (SBOM_DEEP_SCAN="true"). |
| packages/connstat/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/cloud-init/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/challenge-response/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
| packages/bcc/config.sh | Explicitly classifies as not requiring deep SBOM scan. |
Review details
- Files reviewed: 46/46 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…version generate_sbom() assumed syft/cyclonedx-cli were already on PATH, but nothing installed them into the linux-pkg build container -- Phase 1 only installs them onto the appliance-build host. Confirmed via a real pre-push run: every SBOM_DEEP_SCAN package failed with "syft: command not found". Fixed by declaring syft/cyclonedx-cli as PACKAGE_DEPENDENCIES on all 8 flagged packages (masking, virtualization, delphix-sso-app, containerized-masking, windows-connector, zfs, ptools, delphix-rust), and having generate_sbom() install them from $DEPDIR before scanning -- the same pattern zfs already uses for delphix-rust. Also fixes --source-version showing up blank for packages (like masking) that don't set $PACKAGE_VERSION themselves: read the version back out of the built .deb via dpkg-deb instead of relying on a shell variable that doesn't reliably survive to this stage. verify-query-packages.sh's zfs dependency-list assertion updated to match the new PACKAGE_DEPENDENCIES.
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/721d0991-92c3-46ab-a792-d34028a44ae9
branch
from
September 10, 2026 07:44
48d6823 to
0b4e66b
Compare
generate_sbom() didn't set SYFT_FILE_METADATA_SELECTION=none, so every sidecar carried an extra CycloneDX "file" component for the .deb itself (SHA-1/SHA-256 hashes plus the absolute build-workspace path) alongside the real pkg:deb component -- confirmed in delphix-sso-app.cdx.json from a real pre-push build. Same fix appliance-build's 95-generate-sbom.binary hook already applies, for the same reason: a flat, per-package pkg:deb document shouldn't carry per-file noise or leak the build machine's local path.
Multi-.deb packages (delphix-rust: delphix-rust + delphix-rust-src; zfs similarly) go through the cyclonedx-cli merge path in generate_sbom(), which had no --output-version pinned. cyclonedx-cli merge defaults to the newest spec version it supports (1.7), not the 1.6 Syft emitted, so the merged document then failed the very next validate step: Incorrect schema version: expected 1.6 actual 1.7 Confirmed via a real pre-push delphix-rust build. Single-.deb packages never hit this, since the merge step is skipped entirely for them (a plain cp). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviewer feedback: each .deb should have its own distinct BOM file
with a common filename prefix, not one merged SBOM per source package.
The prior approach scanned each .deb separately then merged them via
cyclonedx-cli merge into a single <package>.cdx.json -- correct per
the top-level design doc's stated intent ("one package-level SBOM...
associated with all of that package's debs"), but that assumption
didn't survive review.
generate_sbom() now writes one <deb-filename>.deb.cdx.json per .deb,
independently scanned and validated, with no merge step at all. This
also removes the cyclonedx-cli merge --output-version bug entirely,
since there's no merge left to have a version mismatch in.
Also reconfirms compliance with the reviewer's other point: syft,
cyclonedx-cli, and all linux-kernel-* packages are SBOM_DEEP_SCAN=false
(unaffected by this change, verified separately) -- no SBOM is
generated for build-host tooling or 3rd-party kernel forks.
Design doc updated to match: architecture diagram, the "multiple .debs
per package" resolution, and the implementation-status/follow-ups
sections now describe the 1:1 mapping and the real-build testing
already done, instead of the merge approach and its since-superseded
open question about cyclonedx-cli's --output-version default.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sebroy
requested changes
Sep 10, 2026
| # (lib/common.sh) to scan this package's own .deb -- never shipped in the | ||
| # built package itself. | ||
| # | ||
| PACKAGE_DEPENDENCIES="syft cyclonedx-cli" |
Contributor
There was a problem hiding this comment.
If the default hook is used, then the package's config.sh should not have to declare any such dependencies. Those should be handled by linux-pkg's generic build dependencies that are installed prior.
| awk -F'\t' '$2 == "none" { print $1 }') | ||
|
|
||
| if [[ -n "$unclassified" ]]; then | ||
| echo "The following packages have not set SBOM_DEEP_SCAN (\"true\" or" \ |
Contributor
There was a problem hiding this comment.
We should add some context in this error that generally, 3rd-party forks of debian packages, or packages that are not included in a shipping product should set this to false, while 1st-party packages should set this to true so that their internal packaged components are included in the product's aggregate sbom.
Review feedback: generate_sbom() is a default hook, inherited unmodified by every SBOM_DEEP_SCAN package, so the tooling it runs is the hook's concern -- a package's config.sh should not have to declare it. Those tools belong with the rest of linux-pkg's generic build dependencies, installed prior to any package build. All 8 flagged packages therefore drop PACKAGE_DEPENDENCIES="syft cyclonedx-cli" (virtualization and zfs are restored to their original dependency lists), and setup.sh gains install_sbom_tools() alongside install_awscli/install_shfmt. Unlike everything else installed there, delphix-syft/delphix-cyclonedx-cli are Delphix-built .debs with no apt source -- the container's apt sources are only the Ubuntu primary mirror and the PPA secondary mirror -- so they are fetched from the same S3 location fetch_dependencies() uses and installed by path. apt rather than dpkg, since delphix-cyclonedx-cli has real libicu dependencies. The install is best-effort: setup.sh is package-agnostic, so it cannot skip itself when the package being built is syft or cyclonedx-cli. A hard failure on a missing artifact would break every build on a branch where neither has been published yet, including their own, so a missing artifact warns and continues. generate_sbom() checks for the tools itself and fails loudly, affecting only the builds that need them. Trade-off worth noting: with no PACKAGE_DEPENDENCIES entry, Jenkins's static dependency graph no longer knows these packages relate to syft/cyclonedx-cli, so build-order batching and the rebuild-dependents- on-syft-change cascade no longer apply to them. Documented in the spec, along with the rejected alternative of deriving the dependency from SBOM_DEEP_SCAN in load_package_config(). verify-query-packages.sh's zfs assertion is restored accordingly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback: the lint told you that a package was unclassified but not how to classify it, leaving whoever adds the next package to go read lib/common.sh or the spec to find out. State the rule where it is actually needed -- 1st-party packages set "true" so the third-party components they package internally land in the product's aggregate SBOM; 3rd-party forks of Debian packages and anything not shipped in a product set "false", since appliance-build's image-level scan already covers those as a flat pkg:deb component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
Phase 1 (#414 + appliance-build#892, now merged) gives every image a flat, dpkg-only
CycloneDX base scan. That's correct for 3rd-party debs, but wrong for Delphix's own
first-party packages —
masking,virtualization,delphix-sso-app,containerized-masking,windows-connector,zfs,ptools,delphix-rust— whichbundle third-party components (jars, npm, wheels, Rust crates) that dpkg can't see.
Solution
SBOM_DEEP_SCAN— new per-package opt-in flag inconfig.sh, mirroring theexisting
MEND_SCAN_APPLICABLEpattern. Set"true"on the 8 packages above,"false"on the other 32.verify-sbom-scan-flag.sh) — fails if any package hasn't explicitly setthe flag either way. No silent gaps.
generate_sbom()— new stage inbuildpkg.sh/lib/common.sh. For a flaggedpackage: installs
syft/cyclonedx-cli(declared asPACKAGE_DEPENDENCIES), scansthe built
.deb(s), merges viacyclonedx-cliwhen a package emits more than one.deb(e.g.zfs), and writes<package>.cdx.jsonnext to the.debin$WORKDIR/artifacts/.artifacts/→ S3 sync already picks up thenew file, same as every other build artifact.
This is Phase 2 (CP-13465, Jira DLPX-98872) of the CycloneDX SBOM effort (Epic
CP-13455). Full design in
docs/specs/2026-09-08-sbom-per-package-sidecar-design.md.Testing done
Ran a real pre-push build of
delphix-sso-append-to-end and inspected the actualoutput — not just checked that the build passed:
Out of scope (by design)
cyclonedx-gradle-pluginfor Java,cargo-cyclonedxforRust) — this is intentionally baseline Syft-on-deb only. Whether that's good enough is
Phase 4's evaluation.
appliance-buildfetching/merging these sidecars into the per-image SBOM — Phase 3(CP-13466), not started.