feat(sbom): generate file-based package SBOMs without docker.sock - #307
reyreavman wants to merge 6 commits into
Conversation
Verification
Review focus
Follow-up
|
Previously the SBOM of a stapel image with file-based packages was produced by running syft against the whole image filesystem via a docker: source. That required mounting /var/run/docker.sock into the scanner container and then post-filtering the resulting BOM down to the declared spec/lock files, so SBOM generation could not run where the docker socket is unavailable or forbidden. Now every packages directive is scanned on its own: its declared spec and lock files are read from the built image (the same image-read mechanism os-pm uses), materialized into a temporary directory with their workdir-relative layout, and scanned with a dir: source and the directive's cataloger — one scanner run per directive, with no docker.sock mount. The per-directive BOMs are then unioned. Because the targeted scan only ever sees the declared files, the redundant post-scan FilterBOMBySourcePaths pass and its cataloger filter modes are removed. Dockerfile images keep the full-image docker: scan. The SBOM artifact format version is bumped 3 -> 4 so images carrying a legacy full-scan SBOM regenerate it, while unchanged rebuilds still reuse the cache. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Addresses review findings on the targeted directory-scan SBOM path. Materialize each declared spec/lock under its full in-image path instead of a workdir-relative one, so a dir source records the real location (/app/api/go.mod) rather than a path that never existed in the image (/go.mod). The rebase is anchored and cleaned so a "..", a leading slash or a relative source path cannot escape the scan directory. The scan layout now depends only on the source paths, so Cataloger.Workdir — and the cache-key ambiguity of it being absent from the scan-command checksum — is removed. Restore only the container component of the scanned BOM's metadata and keep syft's own tools and timestamp; replacing the whole metadata dropped the timestamp, and a per-image SBOM without one is rejected by the ISPRAS validator. Stamp the skip-scan branch with a timestamp too. A directory source makes syft emit a PURL-less type=file component for each scanned manifest, which dedup never removes; drop these after every per-directive scan so only real packages remain, which is what keeps the post-scan source-path filter safely removed. Record that SYFT_FILE_METADATA_SELECTION=none is required for this — the scanner honors it, contrary to the task note. Wrap the materialize error, guard the per-directive union against an empty slice, and document its first-directive-wins dedup order. Add a unit test for scanFileBasedPackages, and document the directory scan and the build-fails-on-a -missing-declared-file behavior in the SBOM docs (en/ru). Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
d0bc2a8 to
82ea3f9
Compare
Second review pass follow-ups on the directory-scan SBOM path. MkdirTemp, MkdirAll and WriteFile are all umask-subject, so under a restrictive umask (e.g. 077) the scan root and the nested per-path directories materialized for every scan were 0700 — not traversable by a non-root scanner container, contradicting the function's own world-readable contract. Force the whole materialized tree world-readable (directories also executable) in one pass after writing, instead of the piecemeal chmods that missed the MkdirAll directories. A test under umask 077 pins the intermediate app/, app/api/ directories, which the previous permission assertions (scan root and file only) passed for the wrong reason — a default 022 umask already yields 0755. Extract the image-metadata restoration into restoreImageMetadata and cover it with a unit test: replacing only the container component while keeping syft's tools and timestamp, and stamping a timestamp when the BOM has none. The previous test asserted timestamp survival only through the per-directive union, leaving the actual restoration in ConvergeWithMerge unpinned; reverting it to replace the whole metadata now fails a test. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
A stapel image with file-based `packages` failed the build when a declared lock file was not present in the image. go-mod always declares go.sum (the ecosystem's DefaultLockFile), but a module with no dependencies produces none, so the targeted directory scan aborted with: materialize inputs for cataloger "go-module-file-cataloger": read /app/go.sum ...: Could not find the file /app/go.sum in container ... The old full-image scan tolerated this — it simply did not catalog a file that was not there. The targeted scan read every declared path and hard-failed on absence. Split a cataloger's inputs into required and optional: the spec (e.g. go.mod) must be present and still fails the build with a directive-and-path error when missing, while the lock (e.g. go.sum) is best-effort — an absent one is skipped, restoring the previous behavior. Optional paths join the scan cache key so a declared lock still contributes to cache identity. A unit test covers a go-mod directive whose image has go.mod but no go.sum: materialization succeeds and skips the lock. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
The "Data Source Connection Method" and "Path in Source Image" rows described a Dockerfile full-image scan (daemon + socket via volume, OS root). SBOM is not supported for Dockerfile images — validateSbomOnlyWithStapelImages rejects them — so that path never runs. Describe what actually happens: for stapel images with file-based packages, a directory scan of the spec/lock files extracted from the built image, without the Docker socket. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
An absent lock file is tolerated so that a module with no dependencies builds, but the same skip also fires when a lock that should exist is gone — removed by a later stage, or present only as a symlink. In that case the scan sees the spec alone and transitive dependencies silently drop out of the SBOM, with the only trace at debug level. Surface the skip as a warning naming the lock path, image and cataloger, so a missing lock is visible in the build output. The message states that the absence is expected for a project without dependencies, since for Go that is the normal state (go.sum is only written when there are modules to verify), so the warning stays truthful for both cases. A unit test pins the warning. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Fral738
left a comment
Fral738
left a comment
There was a problem hiding this comment.
Three issues remain: targeted JavaScript scans lose license metadata previously retained by the source-path filter; optional lock extraction errors are accepted as absence and can publish an incomplete SBOM; and extracted manifests become readable to unrelated users on shared Linux hosts. Details and corrections are inline.
The description's equivalence rationale needs narrowing: byte-identical spec/lock files do not imply equivalent SBOM metadata, since Syft enriches lock-derived packages from installed files. The name/version/PURL comparison explicitly listed as UNVERIFIED would not detect the reproduced license loss. Also update the workdir-relative-layout bullet: the current implementation preserves full in-image paths.
Verification at c8be001: task build and task test:unit passed. Eight focused implementation mutations were rejected by relevant assertions, and the same checks passed after restoration. Replacing the optional-lock test's not-found error with io.ErrUnexpectedEOF still passed, confirming the error-classification gap. A real Syft v1.45.1 docker-versus-dir comparison reproduced MIT becoming absent for pkg:npm/is-number@7.0.0 with the same /app/yarn.lock location. The scoped Yarn Vanilla Docker e2e was attempted but stopped before SBOM generation: pushing its builder image to the local registry timed out. Full ecosystem e2e equivalence and task test:integration remain unverified. Previously discussed metadata/path/umask fixes were accounted for; the resolved two-mount concern is not being reopened.
| func (step *sbomStep) scanFileBasedPackages(ctx context.Context, imageRef string, scanOpts scanner.ScanOptions, catalogers []scanner.Cataloger, targetPlatform string) (*cdx.BOM, error) { | ||
| scannedBOMs := make([]*cdx.BOM, 0, len(catalogers)) | ||
| for _, cataloger := range catalogers { | ||
| dir, cleanup, err := managedinput.MaterializeCatalogerInputs(ctx, step.containerBackend, imageRef, cataloger, targetPlatform) |
There was a problem hiding this comment.
Scanning only spec/lock files removes JavaScript license metadata that the old image scan retained. Syft v1.45.1's javascript-lock-cataloger enriches lock-derived components from node_modules//package.json, while remote license lookup is disabled by default.
A real scan of an image containing /app/yarn.lock and /app/node_modules/is-number/package.json with license=MIT, compared with the materialized spec/lock-only directory using the same cataloger and scanner environment, yields:
docker: pkg:npm/is-number@7.0.0 licenses=[MIT] location=/app/yarn.lock
dir: pkg:npm/is-number@7.0.0 licenses absent location=/app/yarn.lock
The old exact-path filter keeps that component because its recorded source is yarn.lock, not node_modules. Preserve local license enrichment for the selected components, for example by reading the relevant installed package.json files separately and enriching the targeted BOM, and add a real-scanner regression assertion for licenses rather than only name/version/PURL.
|
|
||
| for _, sourcePath := range cataloger.OptionalSourcePaths { | ||
| data, err := backend.ReadFileFromImage(ctx, imageRef, sourcePath, opts) | ||
| if err != nil { |
There was a problem hiding this comment.
Every optional-lock extraction error is classified as a missing file here. ReadFileFromImage also returns container-creation, Docker-copy and tar/content-read errors, so an existing lock whose read fails can be omitted from a successfully published and cached SBOM. The warning additionally hides the actual failure.
Replacing the existing optional-lock test's not-found error with io.ErrUnexpectedEOF, leaving all assertions unchanged, still passes: the result is successful spec-only materialization and a misleading 'not found' warning.
Normalize genuine absence in the image reader to a recognizable sentinel such as fs.ErrNotExist, skip only that case, and clean up and return every other error with the cataloger/path context. Add a separate unexpected-EOF case requiring failure so transport corruption cannot take the absent-lock path.
| // MkdirTemp, MkdirAll and WriteFile are all umask-subject, so under a restrictive umask | ||
| // the scan root and its nested directories would not be traversable by the scanner | ||
| // container's user. Force the whole tree world-readable (dirs also executable). | ||
| if err := makeTreeWorldReadable(dir); err != nil { |
There was a problem hiding this comment.
This removes the privacy boundary around files extracted from the image on shared Linux hosts. MkdirTemp normally creates a 0700 directory, but makeTreeWorldReadable changes that root and every descendant directory to 0755 and files to 0644, even under umask 077. With the usual shared /tmp, another host user without Docker/image access can enumerate sbom-dirscan-* and read private manifests while the scan runs, including authenticated dependency URLs when present.
Keep a 0700 temporary parent owned by the invoking user, create a readable scan subtree beneath it, and bind-mount only that subtree. Apply makeTreeWorldReadable to the subtree, not the private parent, and have cleanup remove the parent. This preserves scanner access without granting unrelated host users access; cover parent permissions alongside the existing scanner-readability assertions.
2 participants
Summary
SBOM generation for a stapel image with file-based
packagesno longer mounts/var/run/docker.sockinto the scanner container, so it now runs in environmentswhere the docker socket is unavailable or forbidden. syft is invoked per packages
directive against a directory source built from only the declared spec/lock files,
instead of walking the whole image filesystem and post-filtering the result.
What
packages, syft runs once per directive against adir:source containing only that directive's declared spec/lock files; the scanner container receives no/var/run/docker.sockmount.dir:source.docker:scan, docker.sock still mounted.FilterBOMBySourcePathsand the cataloger filter modes (CatalogerFilterMode): a targeted directory scan surfaces no foreign components, so the post-scan filter is a no-op.dir:source; the e2e sbom suites are the oracle and did not run on the macOS dev host.Why
The
docker:source forced a docker.sock mount and a whole-image filesystem walk,then discarded everything outside the declared spec/lock with a post-filter — costly,
and impossible where the socket is forbidden. Because network is disabled for shell
stages under
build.sbom.enable=trueand installs are frozen/locked, the spec/lock inthe image are byte-identical to source, so scanning just those files is equivalent to
the old scan-then-filter. The rejected alternative was syft-as-a-library, which would
add an external dependency; this keeps the same pinned scanner image and the existing
image-read path, adding no new dependency.