feat(sbom): generate file-based package SBOMs without docker.sock - #307
reyreavman wants to merge 15 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.
Third review pass on the directory-scan SBOM path. An optional lock file was skipped on any ReadFileFromImage error, but that call also fails on container creation, docker copy and tar/content reads. A lock that exists but could not be read was therefore dropped from a successfully published and cached SBOM, and the warning reported it as "not found". Normalize genuine absence in the docker backend to fs.ErrNotExist (both the copy 404 and the no-regular-file case; the buildah backend already wraps os.ReadFile errors), skip only that, and abort with the cataloger/path context on everything else. A test with io.ErrUnexpectedEOF now requires failure. makeTreeWorldReadable also relaxed the MkdirTemp root to 0755, so on a shared host any user could enumerate sbom-dirscan-* under /tmp and read the extracted manifests (including authenticated dependency URLs in locks) while the scan ran. Keep a 0700 parent owned by the invoking user and materialize into a readable scan/ subtree beneath it; only that subtree is returned for bind-mounting, and cleanup removes the parent. Tests pin the parent mode under a permissive umask and its removal on cleanup. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
The targeted directory scan lost licenses for JavaScript packages that the previous full-image scan retained. A lock file carries no license; syft's javascript-lock-cataloger reads it from the installed package's own manifest, node_modules/<pkg>/package.json, next to the lock. With only the spec and lock materialized there was nothing to enrich from, so every npm/yarn/pnpm component was published without a license. Reproduced on syft v1.45.1: the same pkg:npm/is-number@7.0.0 scanned from docker: carried MIT and from dir: carried nothing; with the manifest placed next to the lock the dir: scan carries MIT too. Give the cataloger the data it needs instead of second-guessing it: declare node_modules as the enrichment dir of the three JavaScript ecosystems, and materialize its package.json files at their in-image path alongside the lock. The cataloger then enriches exactly the packages it found in the lock, as it did on the full image, so the licenses come from syft and cannot drift from its own parsing. Only the manifests are copied, not the installed code, and the directory is read in a single copy rather than one round-trip per package. Add ReadDirFromImage to the container backend for that: one docker cp of the directory streamed as tar, regular files only, optional base-name filter, entries rebased under the destination so a crafted path cannot escape it. A missing directory is reported as fs.ErrNotExist and skipped with a warning, like a missing lock; any other read error aborts. Enrichment dirs join the scan cache key. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
…es in e2e pnpm does not install packages flat: node_modules/<pkg> is a symlink into node_modules/.pnpm/<pkg>@<ver>/node_modules/<pkg>. syft's javascript-lock-cataloger reads node_modules/<pkg>/package.json, which the old full-image scan reached through the link, but docker cp delivers a symlink as a bare tar entry, so the targeted scan still published pnpm components without licenses. Verified on syft v1.45.1 against a real pnpm image: docker: carried MIT for lodash@4.17.21, dir: with only the .pnpm copy carried nothing, dir: with the flat path present carried MIT. Record symlink entries while extracting the directory and, once the stream ends, materialize each link that points at an extracted directory by copying the target's files under the link path, so the flat path the cataloger expects exists. Links to files, to missing targets, or escaping the tree are left out. Add a license assertion to the yarn, npm and pnpm e2e suites: they compared only name, version and PURL, which is why the license regression passed unnoticed. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Go was the second ecosystem, after JavaScript, whose licenses the targeted scan lost. go.mod and go.sum carry no license; syft's go-module-file-cataloger reads it from the module's LICENSE file in the module cache, $GOPATH/pkg/mod/<module>@<ver>, of the scanned filesystem. Verified on syft v1.45.1: docker: carried MIT for github.com/samber/lo, dir: with only go.mod/go.sum carried nothing, dir: with the module's cache entry placed at its in-image path carried MIT again. The cataloger locates the cache from the filesystem it scans, not from its own config knobs, so the fix is to put the cache entries where it looks. Generalize enrichment instead of adding a second special case. An ecosystem now declares an EnrichmentSource — where its cataloger reads installed-package metadata and which files matter — and one materialization loop serves every ecosystem: JavaScript copies node_modules (package.json), Go copies the module cache (license files). The cache root is resolved from the image environment the way the go tool does ($GOMODCACHE, else $GOPATH/pkg/mod, else $HOME/go/pkg/mod) and is not part of the scan cache key, since it is image-specific. Copying the whole module cache would mean the image's entire dependency source tree, so only the modules listed in go.sum are copied, one directory each, with paths escaped as the go tool stores them (github.com/Azure -> github.com/!azure); only files matching the license-file names syft accepts are written. Python, Cargo and Lua were checked the same way and do not enrich from the filesystem, so they declare no source. Reading many paths from one image now goes through an ImageReader session that creates the throwaway container once per directive instead of once per file; this is what keeps per-module copying cheap. ReadDir takes case-insensitive file name patterns rather than an exact list, and the buildah reader follows directory symlinks like the docker one resolves them. An e2e suite builds a go1.12 image with a registry module and asserts its BSD-2-Clause license survives the targeted scan. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Fral738
left a comment
Fral738
left a comment
There was a problem hiding this comment.
License preservation is still incomplete in two supported layouts: JavaScript packages reached through absolute directory symlinks inside node_modules, and Go modules installed with a packages.env cache-path override. Both retain their component identity but lose licenses that the previous full-image scan retained. The inline comments include reproductions and the required corrections. These cases also qualify the description's license-preservation claim and need regression coverage before treating enrichment as equivalent.
| target := filepath.Join(filepath.Dir(rel), hdr.Linkname) | ||
| if filepath.IsAbs(hdr.Linkname) { | ||
| target = hdr.Linkname | ||
| } |
There was a problem hiding this comment.
Absolute symlinks inside the copied image directory are rebased against the wrong root. For ReadDir("/abs/node_modules", dest), is-number -> /abs/node_modules/real is recorded as dest/abs/node_modules/real, but real/package.json was extracted at dest/real/package.json. resolveSymlinks then silently skips the link.
A real docker cp archive containing this link reproduces the missing is-number/package.json. With Syft v1.45.1 and javascript-lock-cataloger, the image scan gives pkg:npm/is-number@7.0.0 license MIT at /abs/yarn.lock; scanning the directory produced by this extractor gives the same component without licenses.
Retain the original image source directory in the extractor and relativize absolute targets against it before mapping them into destDir, rejecting targets outside that source subtree:
relativeTarget, err := filepath.Rel(e.srcDir, hdr.Linkname)
if err != nil {
return fmt.Errorf("relativize symlink target %q: %w", hdr.Linkname, err)
}
if relativeTarget == ".." || strings.HasPrefix(relativeTarget, ".."+string(filepath.Separator)) {
return nil
}
target = relativeTargetAdd an absolute-link archive case alongside the relative pnpm-link test.
|
|
||
| *bom.Components = filtered | ||
| } | ||
| cataloger.Enrichment = toEnrichment(res.enrichment, workdir, lockPath) |
There was a problem hiding this comment.
The enrichment plan drops directive.Env, although GeneratePackagesCommands passes it to the Go install command. With image GOPATH=/go and packages.env.GOPATH=/opt/build/go, installation uses /opt/build/go/pkg/mod but ResolveEnrichmentRoot selects /go/pkg/mod. The missing directory is skipped, so the resulting SBOM loses available module licenses.
A focused mapping test confirms that the generated install command contains GOPATH="/opt/build/go" while the resolved enrichment root is /go/pkg/mod. On Syft v1.45.1, an image containing github.com/pkg/errors@v0.9.1's LICENSE under /opt/build/go/pkg/mod yields BSD-2-Clause in the full-image scan; the spec/lock-only targeted scan loses it. Syft's in-source lookup uses **/go/pkg/mod, so this override is discoverable by the previous scan.
Carry the directive environment into the enrichment plan and overlay it on the image environment before resolving GOMODCACHE, GOPATH and HOME, preserving Go's precedence rules:
vars := envMap(imageEnv)
for name, value := range directiveEnv {
vars[name] = value
}Add a regression where the directive GOPATH differs from the image GOPATH and assert both the extraction path and the resulting license.
…dden GOPATH
Two supported layouts still lost licenses the full-image scan retained, both
keeping component identity while dropping the license.
An absolute symlink inside a copied image directory was rebased against the
destination root instead of the source: for ReadDir("/abs/node_modules", dest),
is-number -> /abs/node_modules/real was recorded at dest/abs/node_modules/real,
but real/package.json extracted at dest/real, so the link resolved to nothing and
the manifest was skipped. Relativize an absolute link target against the in-image
source directory before mapping it into dest, and drop a target that points outside
that directory. Relative links (pnpm) are unchanged.
The Go module cache root was resolved from the image environment alone, ignoring
the packages directive env that the install command runs under. With image
GOPATH=/go and packages.env.GOPATH=/opt/build/go, `go mod download` writes the
cache to /opt/build/go/pkg/mod while enrichment read /go/pkg/mod, found nothing and
skipped it. Carry the directive env on the enrichment plan and overlay it on the
image environment before resolving GOMODCACHE/GOPATH/HOME, matching the install
command's precedence.
Verified on syft v1.45.1: an image with GOPATH=/opt/build/go carries
github.com/pkg/errors BSD-2-Clause in the full-image scan and, with the module dir
placed at /opt/build/go/pkg/mod, in the targeted scan too. Unit tests cover the
absolute-link archive and the directive-GOPATH override extraction path; an e2e
fixture asserts the license survives a packages.env GOPATH override.
Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
The SBOM directory-scan flow is the only caller of the image-reader abstraction, and it rejects the Buildah backend upfront, so the Buildah ImageReader implementation (OpenImageReader, the mounted-rootfs ReadFile and ReadDir, and the symlink-following directory walk) never ran. Remove that dead implementation, restore the standalone ReadFileFromImage that existed before the abstraction was introduced, and make Buildah's OpenImageReader an explicit "not supported" stub. This also drops the latent symlink-escape and error-to-fs.ErrNotExist mapping concerns from code that could never execute. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
…g it The materializer held a context.Context in its struct and read it back in every method, against the convention that a method takes its context as the first argument. Pass ctx explicitly through materialize, enrich and copyEnrichmentDir and drop the field. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
The scan-command checksum deliberately excludes Enrichment.DirectiveEnv and treats a not-yet-resolved Go module cache root as empty, which reads like a bug at a glance. Document that the directive env only shapes the package set already captured by the Packages stage (and thus image) digest the bill of materials is stored under, so hashing it here would add nothing. Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
…r-scan Signed-off-by: Radmir Khurum <radmir.khurum@flant.com> # Conflicts: # pkg/container_backend/docker_server_backend.go
2 participants
Summary
For a stapel image with file-based
packages, werf now generates the SBOM by scanning only the declared spec/lock files — and the installed-package files the cataloger enriches from — extracted from the built image, instead of running syft against the whole image filesystem via adocker:source. The scanner container no longer gets a/var/run/docker.sockmount, so SBOM generation works where the socket is unavailable or forbidden (DOCKER_HOST, DinD, rootless).What
Scan
packages, syft runs once per directive against adir:source built from that directive's declared spec/lock files read out of the built image; the scanner container receives no/var/run/docker.sockmount./app/api/go.mod), so recorded componentsyft:locationpaths match the full-image scan; a crafted path or a..cannot escape the scan directory.FilterBOMBySourcePathsand its cataloger filter modes are removed as redundant.metadata.toolsandmetadata.timestamp; onlymetadata.componentis restored to the image. The skip-scan branch (a stapel image without file-based packages) gets a timestamp too, so a per-image SBOM piped into a timestamp-requiring validator no longer fails.Licenses
node_modules/<pkg>/package.json, for Go the license files of thego.summodules under the module cache ($GOMODCACHE, else$GOPATH/pkg/mod, else$HOME/go/pkg/mod, resolved from the image environment). Only the manifests / license files are copied, not the installed code, and for Go only the modules listed ingo.sum, one directory each, with paths escaped as the go tool stores them (github.com/Azure->github.com/!azure).node_modules/<pkg>symlinks into its.pnpmstore; the extractor resolves those links so the flat path the cataloger reads exists.docker:vsdir:comparison reproduced the loss and the fix —is-number(yarn/pnpm) andgithub.com/pkg/errors@v0.9.1(go) carried their license underdocker:, carried none underdir:with only the spec/lock, and carried it again once the enrichment files were placed at their in-image path.Failure modes
go.mod,requirements.txt,package.json) fails the build with an error naming the directive and path; a declared lock or enrichment source that is genuinely absent (fs.ErrNotExist) is skipped with a warning, matching the old full-image scan which simply did not catalog a file that was not there.docker cp, a container/copy error) aborts, so transport corruption cannot silently publish and cache an incomplete SBOM.0700parent owned by the invoking user, with a world-readable scan subtree; only the subtree is bind-mounted, so on a shared host other users cannot enumerate or read the extracted files (which may hold authenticated dependency URLs).Unchanged
build.sbom.enablebyvalidateSbomOnlyWithStapelImages, so the full-imagedocker:path is unreachable and not exercised; Buildah is rejected too.task test:integration— the e2e sbom suites (yarn/npm/pnpm/gomod now assert licenses) run in CI, not locally.Why
The
docker:source forced the docker.sock mount and a whole-image filesystem walk, then threw away everything outside the declared spec/lock with a post-scan filter — costly, and impossible where the socket is forbidden. Byte-identical spec/lock files yield equivalent name/version/PURL but not equivalent metadata: a lock carries no license, and syft fills it in by reading the installed package's own files (node_modules/<pkg>/package.json, the module cacheLICENSE), which the manifest lacks. So the targeted scan copies exactly the files each cataloger reads and lets syft enrich from them, rather than parsing licenses itself and drifting from syft's own output. 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 mechanism.