[reyreavman]

Summary

An image's SBOM says a component is there; it does not say which files are that component's. A child image built FROM this one, or importing files out of it, inherits the component list wholesale with nothing to check it against, so a component whose files never arrived stays in the child's SBOM and is reported as its attack surface. Every image whose SBOM werf generates now records, per component, the files that component put into the image, which is the evidence the shipped-scope rule (#452) needs to answer that question on the child.

What

• A component installed by os-pm, pip, npm/yarn/pnpm or luarocks gets evidence.occurrences in the image's SBOM: one entry per file, location = the path in this image, additionalContext = sha256:<hex> for a regular file and symlink for a symlink. Every occurrence werf writes carries one or the other, so a reader can tell them from occurrences that arrived in a merged document.
• The files are the image's, not the manifest's: the digest comes from the image content, and a path a manifest declares but the image no longer holds is left out.
• A component none of those four ecosystems names gets no evidence.occurrences at all — a Go module, a deb/apk/rpm package, an egg-info install, a Yarn PnP cache. Nothing is known about its files and nothing is claimed.
• A component that names no file of its own gets none either: an os-pm metapackage, a distribution whose RECORD is empty, a rock whose manifest lists nothing.
• An occurrence list that is present but empty is the opposite statement — the component's files were looked for and none are in the image. That distinction is what a consumer turns into keep-or-drop.
• An image installed by a pm too old to write /var/lib/pm/files.json gets no os-pm occurrences and one warning line, as before; its os-pm components stay unmarked rather than marked absent.
• A component installed more than once under different paths — the npm case — carries the files of every copy.
• The SBOM artifact format version goes to 11, so an image with an SBOM already attached regenerates instead of serving the occurrence-free one from cache.
• The SBOM grows by one occurrence per recorded file: about 190 B of JSON, 45 B gzipped. VERIFIED: on node:20-slim 203 components and 1949 files take the component list from 26 KB to 400 KB (4 KB to 85 KB gzipped); on python:3.12-slim one component with 442 files adds 79 KB (19 KB gzipped). A project with a 100k-file node_modules would add on the order of 19 MB, 4 MB gzipped. There is no cap and no opt-out flag.
• Generating an SBOM reads the built image out of the container backend it was pulled into — SaveImageToStream, the same read MutateAndPushImage does — not from the registry a second time. The cost is a save of the uncompressed image to a file under the werf tmp dir for the duration of the read (two copies on Buildah, whose SaveImageToStream writes its own), and no network. It happens only for an image that has a component of one of the four ecosystems.
• An image with no component of those four ecosystems — a scratch Go image, a Dockerfile build with only deb packages — is not read at all and costs nothing.
• A failure to save the image or read its file lists warns and leaves the SBOM without these records; it never fails the build. What can fail it is the host — an image that is not there, a files.json that does not parse, a disk that is full — not the network, so the SBOM that gets cached in its place is the one the next build would produce again. The warning names the werf image and the stage image: image app: unable to read the built image <stage ref>; the SBOM records none of the files its components consist of.
• Deliberately unchanged: no component is removed, reordered or re-versioned, and nothing reads the occurrences yet — Match from feat(sbom): decide which components an image actually ships #452 still has no caller. Nothing relocates an inherited occurrence's path through an import's to:; that is the consumer's, as Occurrence's contract states.

Why

The shipped-scope rule landed in #452 as a pure function with no input: it takes components with the files they claim, and werf had nowhere to get those from. os-pm, pip, npm and luarocks each record what they installed, and the image's own layers give the content, so the donor is the only place where the two can be put together — by the time a child is built, the manifests may not have come along at all.

The alternative was to have the child derive the file list from the donor's manifests directly, which fails for exactly the case this is for: a distroless final gets the files and not /var/lib/pm, .dist-info or node_modules/*/package.json.

Occurrences are written after Canonicalize rather than before it. Canonicalization folds duplicate components together, and its evidence merge drops a list that comes out empty — which is precisely the "looked for, none found" signal here, so writing earlier would erase it on every component that appears twice after a merge.

[reyreavman]

Verification

• task test:unit paths="./pkg/sbom/... ./pkg/build/ ./pkg/oci/..." on a Linux VM as well as locally: green apart from two automatic LFS build context specs in pkg/build/image, which fail identically on a clean main checkout here (no git-lfs on the machine) and are untouched by this diff.
• The three recordShippedFiles specs stand up an in-process registry (go-containerregistry/pkg/registry), push a real image carrying /var/lib/pm/files.json, a .dist-info/RECORD, a regular file and a symlink, and assert the occurrences that land on the BOM. That is the only check that exercises OCIStore.ParentImage → CollectFilesets → RecordOccurrences as one path; ConvergeWithMerge itself has no unit caller.
• Mutation, 22 compiling mutants, all killed. The ones worth naming: key without version and key without ecosystem → leaves a component of another version alone / … of another ecosystem alone; PEP 503 folding dropped → matches a PyPI component whose purl keeps a separator the install folds; npm case folding dropped → … spells the name in mixed case; DeclaresFiles forced true in each collector → tells a package that recorded no files from one whose recorded files are gone, yields an empty fileset for a distribution whose record is empty, says a rock whose manifest names no file declares none; symlink marker emptied or replaced by a digest → marks a symlink as one instead of leaving it to be told apart by a missing digest; lo.SomeBy → lo.EveryBy over a component's copies → records a component one of whose copies declares files; the nested-component and metadata-component walks removed → the two walk specs.
• Not run: no mutant kills DeclaresFiles in the JavaScript collector. The declared list there is read out of the image index and a package's own package.json is always in it, so the flag cannot be false for a fileset that exists — the mutant is equivalent, not a gap in the specs.
• Not run: nothing exercises a real build end to end. ConvergeWithMerge needs a backend, a stage and a registry together, which is e2e, and the suite has no SBOM entry to extend.

Review focus

• The nil / empty / present distinction on evidence.occurrences, which is the whole contract: absent means "nothing known, keep", empty means "looked for, none found" and can drop a component downstream. RecordOccurrences skips a component when every fileset of it has DeclaresFiles false, and setOccurrences writes a non-nil empty slice otherwise.
• purlEcosystems in pkg/sbom/shippedscope/occurrences.go: pkg:generic → os-pm rests on werf's own pm emitting generic purls (pkg/sbom/packages/os_pm/os_pm.go), the same assumption declared.purlTypes already makes. deb/apk/rpm are deliberately absent — nothing here reads dpkg/apk/rpm file lists, so a fileset could never exist for them.
• Whether a component can reach ConvergeWithMerge carrying an occurrence written by something other than werf. The symlink marker makes werf's own entries positively identifiable, but a consumer still has to decide what to do with a foreign one.
• OCIStore.ParentImage reads the image by digest from the registry. Confirm the digest a stage converges with can never be an index digest on a multi-platform build — remote.Image on an index would silently pick a child manifest.

Follow-up

• Measure the SBOM artifact size and the build wall-clock on an image with a large node_modules and on a multi-hundred-megabyte image, and decide whether a cap or an opt-out is needed.
• ClassifyBinary in pkg/sbom/shippedscope/binary.go collects info.Deps but not info.Main.Path, so an image whose SBOM carries its own main module reads it as not-linked. One line, belongs to the binary-classification card, not here.

[reyreavman]

Code Review Report

Base: main (69de5842, #452 merged), head fac588fe
Diff: 13 files, +723/-22 — RecordOccurrences, recordShippedFiles wired into ConvergeWithMerge, OCIStore.ParentImage, Fileset.DeclaresFiles, format version 10 → 11.
Context: concept §3.1 (donor side), card F prerequisite. First PR of the series that touches the build pipeline and every user's SBOM.

Verdict

• Technical: RecordOccurrences is sound: keying by purl → ecosystem with canonical names, DeclaresFiles keeping "unknown" apart from "looked for, none found", deterministic output, nested components walked. Three mutations: two fail their spec; the third (symlink overriding a file at the same path) survives because the FileIndex makes the case impossible — dead guard, not a defect. The one real problem is in the wiring: recordShippedFiles turns every failure of a new read of the built image into a failed build, which the concept and card F explicitly say must be fail-open.
• Product: This is the first user-visible change of the series: every SBOM werf writes grows, every cached SBOM regenerates on upgrade (format 11), and SBOM generation now downloads the image a second time. All three are stated in the body as UNVERIFIED in cost. That honesty is good; shipping unmeasured is not — one number on a node_modules-heavy image is cheap to get and belongs in the body before this leaves draft.
• Risk: Medium. A files.json that does not parse, a layer whose digest does not verify, an unsafe tar path — each now fails a build that succeeded yesterday, on an image whose content did not change.

DoD Criteria

Inferred from the body and the concept (card F "Нет данных … fail-open с warning, сборка не падает").

Criteria	Inferred?	Met?	Evidence
occurrences per component for os-pm / pip / npm / luarocks, location + sha256:/symlink	yes	✅	occurrences.go:136-161; spec records the files of a component of each ecosystem
digest from image, missing path omitted	yes	✅	via CollectFilesets (#435)
component no fileset names → untouched; fileset with no declared files → untouched	yes	✅	occurrences.go:55; M1 fails spec :95
present-but-empty list = "looked for, none found"	yes	✅	spec :187
old pm (no files.json) → warning, components unmarked	yes	✅	spec sbom_step_shipped_files_test.go:125
multiple copies → union of files	yes	✅	spec :173
written after Canonicalize	yes	✅	sbom_step.go:204-206
format version bumped	yes	✅	sbom_step.go:493
errors in the new read are fail-open	yes	❌	sbom_step.go:206-208, 237-245 — Major #1
cost on a large image measured	yes	❌	body says UNVERIFIED twice — Major #2
task format/build/lint/test:unit green	no	✅	build OK, lint 0, 135/135 shippedscope; pkg/build red only on the pre-existing LFS specs (same on main)

Issues

Major

1. recordShippedFiles fails the build on any error — sbom_step.go:206-208, :237-245. Inline.
2. Size and traffic cost shipped unmeasured — PR body, two UNVERIFIED paragraphs. Inline on recordShippedFiles.

Minor

3. Whole-image read even when no component can receive occurrences — sbom_step.go:238. Inline.
4. Symlink-vs-file guard in setOccurrences is unreachable — occurrences.go:143-145. Inline.
5. pkg:generic is not only os-pm — occurrences.go:107. syft's binary cataloger emits pkg:generic/python@3.12.3, pkg:generic/node@20.x for Dockerfile images. Collision needs the same (name, version) as a files.json entry, in which case it is the same package and the occurrences are right — so this is a note for the godoc, not a bug.

Mutation log

Mutation	Result
M1: DeclaresFiles ignored	✅ fails leaves a component that declares no files alone…
M2: canonicalName identity	✅ 2 fail — PyPI separator, npm case
M3: symlink overrides file context	green 135/135 — unreachable, Minor #4

Tree restored after each.

Risks

№	Risk	Type	Likelihood	Severity	Location	Circumstances	Consequences	Recommendation
1	Build that succeeded yesterday fails after upgrade	Operational	Possible	High	sbom_step.go:206	Malformed files.json, unsafe tar path, registry blip during the second pull	SBOM step aborts the build; user sees collect component filesets: … with no opt-out	Major #1
2	SBOM artifact size × N on JS/Python images	UX/Product	Likely	Medium	occurrences.go:149-155	node_modules with 100k files → 100k × ~120 B ≈ 12 MB per SBOM	Registry storage, attestation signing time, scanner ingest	Major #2: measure; consider a cap or symlink-only for non-content paths
3	Image downloaded twice per SBOM	Operational	Likely	Low	store.go:52, sbom_step.go:106	Every build with SBOM enabled	Wall-clock + egress; multi-hundred-MB images	Minor #3 + measure
4	All cached SBOMs regenerate on upgrade	Operational	Likely	Low	sbom_step.go:493	Expected; card G	One slow build per image	State in the release notes

Not verified

• e2e sbom suites — not run (Linux + kind). No e2e asserts on evidence.occurrences, so none should break on content; the format bump invalidates their caches, not their assertions.
• Actual size/traffic on a real image — the body says it is unmeasured and I did not measure it either.
• Unscoped task test:unit — pkg/build/image LFS specs are red here as on main.

[reyreavman]

Major. Every error out of recordShippedFiles aborts the SBOM step and with it the build. The new read can fail for reasons that have nothing to do with the user's change:

• CollectFilesets → osPMFilesets returns an error on a files.json that does not parse (os_pm_fileset.go:48);
• IndexImageFiles returns an error on an unsafe tar path or a layer whose digest does not verify (feat(sbom): index the files an image actually ships #433, by design);
• ParentImage is a second registry round-trip that can time out independently of the pull at :106.

Yesterday those images built; after this PR they fail with collect component filesets: … and no flag to turn it off. Card F is explicit — «Нет данных … → fail-open с warning, сборка не падает» and «Fail-open нужен и при Warn индекса, и при его ошибке … причины в логе различать». The same rule applies on the donor side: an SBOM without occurrences is the pre-#453 SBOM, i.e. the consumer keeps everything — the safe direction. Failing the build buys nothing.

Suggest: Warn with the cause and return nil; keep the hard error only for programmer errors (nil BOM). The spec fails when the built image cannot be read then becomes warns and leaves the BOM unmarked when….

[reyreavman]

Fixed in e8bb5cf — you are right, and the argument that convinced me is the one you made: an SBOM without these records is the pre-#453 SBOM, so the consumer keeps everything. Failing bought nothing. Both reads now warn and return: unable to read the built image; the SBOM records none of the files its components consist of and unable to read which files the components of the image consist of; the SBOM records none of them. recordShippedFiles no longer returns an error at all, so the call site cannot reintroduce the abort. The spec became warns and leaves the BOM as it was when the built image cannot be read, and a second one covers the unparseable files.json you named: it pushes an image whose /var/lib/pm/files.json is not json at all and asserts both the warning and that the component stays unmarked.

[reyreavman]

Major (process). The body marks two things UNVERIFIED: artifact size on a node_modules-heavy image, and wall-clock/traffic of the second pull. Both are one werf build away on any image already in the e2e fixtures (test/e2e/sbom/_fixtures/inject/* has npm and pip projects). This is the first PR of the series that every user pays for on every build; the numbers should be in the body before it leaves draft, even if they are fine. If they are not fine, the obvious levers are a cap on occurrences per component (concept §5 measured ncurses at 2917 files / 424 KB alone) and reading the image from the backend the step already pulled into (:106) instead of the registry — though the latter has no Buildah path.

Minor (perf), same place. CollectFilesets flattens and hashes the whole rootfs even when the BOM has no component of the four purl types — a FROM scratch Go image, a Dockerfile image with only deb packages. A pre-check (walkComponents + keyOfComponent, both already here) that skips the read when nothing can receive occurrences costs nothing and saves the full download in exactly the images where it is pure overhead.

[reyreavman]

Measured, numbers are in the body now. Per recorded file it is ~190 B of JSON, ~45 B gzipped. node:20-slim: 203 components, 1949 files, component list 26 KB → 400 KB (4 KB → 85 KB gzipped). python:3.12-slim: one component, 442 files, +79 KB (+19 KB gzipped). Extrapolating, a 100k-file node_modules lands around +19 MB, +4 MB gzipped — close to your estimate. Second read: 65 s on node:20-slim, 9 m 48 s on the ~1 GB node:20, both from Docker Hub over a laptop link, so that figure is layer download and scales with image size, not component count. Measured by driving CollectFilesets + RecordOccurrences over real images rather than a werf build, which isolates the artifact growth from everything else the step does. I have left the cap out of this PR and put it in the follow-up: the lever to pick depends on what the consumer in card F actually needs per component, and I would rather not guess that here.

Minor (perf) — fixed in the same commit. shippedscope.HasFileBackedComponents(bom) walks the BOM first and the image is read only if something could receive occurrences; spec does not read the image when no component could be given files asserts not even a warning is emitted for a BOM of Go modules and deb packages.

[reyreavman]

Minor. Mutation M3 (drop the isFile guard, always write symlinkContext) stays 135/135 green. The guard protects a path that is a regular file in one fileset and a symlink in another fileset of the same component — but a FileIndex holds a path either in Files or in Symlinks, never both (indexed() in file_index.go), and the filesets of one component come from the same index. So the branch cannot fire. Either drop it, or if you want it as a belt-and-braces invariant, say so in a comment so the next reader does not look for the case.

[reyreavman]

Dropped in e8bb5cf. You traced it correctly — a FileIndex holds a path in Files or in Symlinks, never both, and the filesets of one component come from one index, so the branch could not fire. It is a plain assignment now, which is also why your M3 has nothing left to mutate.

[reyreavman]

Verification

Since the first comment, in response to the review:

• e2e sbom suite, run on a Linux VM — task test:e2e paths="./test/e2e/sbom" labelFilter="sbom" parallel=1, 47 minutes: 66 passed, 3 failed, 1 pending, 2 skipped. The three failures are pre-existing: the same three fail identically on a clean 69de5842 worktree (reads go.mod from the werf project directory when it is a git subdirectory, three-image build with mixed PURL resolution outcomes, switching packages type from os-pm to go-mod regenerates SBOM with new scanner output). Two abort in Building stage app/packages with container run failed: exit code 1, before the SBOM step runs at all. The format bump invalidated every SBOM cache in the fixtures and nothing broke on content.
• Size and time measured by driving CollectFilesets + RecordOccurrences over real images, which isolates artifact growth from the rest of the step. Figures in the body. node:20 and node:20-slim yield the same 203 filesets — the extra ~900 MB of the full image carries no further npm or pip packages — so the 9 m 48 s against 65 s is purely layer download.
• Mutation on what the review changed: the pre-check inverted (keyOfComponent → !ok) and its result discarded → the two HasFileBackedComponents specs fail. The two new warn paths are covered by asserting on captured logboek output, not by a return value, since there is no longer one.
• Not run: still nothing exercises a real werf build carrying occurrences end to end. No e2e spec asserts on evidence.occurrences; adding one belongs with the consumer, which has no caller yet.

Review focus

Unchanged from the first comment, minus the fail-open question, which the review settled. One addition:

• HasFileBackedComponents decides whether the image is read at all. If a purl type that can carry a fileset is ever added to purlEcosystems without the BOM reaching this walk — a component under formulation or metadata.tools, neither of which is walked — the image would be skipped and the component silently unmarked rather than marked absent. Safe direction, but it is the one place where the two walks must stay in step.

Follow-up

• Decide on a cap or an opt-out for occurrence volume, once card F shows what the consumer needs per component. The levers are a per-component cap, dropping symlinks, or an opt-out flag.
• ClassifyBinary in pkg/sbom/shippedscope/binary.go collects info.Deps but not info.Main.Path, so an image whose SBOM carries its own main module reads it as not-linked.

[reyreavman]

Code Review — follow-up on e8bb5cfb

Delta since fac588fe: 4 files, +129/-17; PR body now carries measurements.

Previous findings

#	Finding	Status
Major 1	errors fail the build	✅ recordShippedFiles warns and returns (sbom_step.go:236-260); two specs pin it
Major 2	cost unmeasured	✅ measured and in the body — see below
Minor 3	read even with nothing to record	✅ HasFileBackedComponents gate; mutation (gate removed) fails does not read the image when no component could be given files
Minor 4	unreachable symlink guard	✅ removed
Minor 5	pkg:generic note	✅ godoc on purlEcosystems

139/139 shippedscope; pkg/build red only on the pre-existing LFS specs.

Verdict

Code-wise nothing outstanding. The measurements, however, turn Major #2 from "unknown" into a concrete product decision that I do not think should be made implicitly:

65 s on node:20-slim, 9 m 48 s on node:20 (~1 GB) — a second full download of the image per build, for every image with a pip/npm/os-pm/luarocks component, with no opt-out.

That is build time every user pays on every build of a large image, to produce evidence only a child image will ever read. Two observations:

1. The image is already on the machine. ConvergeWithMerge pulls it into the backend at sbom_step.go:106 before anything else. Both backends implement SaveImageToStream (container_backend/interface.go:176; Docker :474, Buildah :1436) which yields a docker-save tar, and gcr's tarball.Image(opener, nil) reads that into a v1.Image — the same type CollectFilesets takes. That replaces a 10-minute network round-trip with a local docker save (disk + CPU), backend-agnostic, no new dependency. OCIStore.ParentImage would then be unused.

2. If (1) is deferred, the body's "no opt-out" should not stand. A WERF_SBOM_* env / flag that disables the record, or at least a size/time threshold, is the minimum for a change that adds minutes to a build. Card F's fail-open principle argues the same way: the feature is optional enrichment, so it should be optional.

I would not approve with the 9m48s figure in the body and neither (1) nor (2) in the PR. Everything else is ready.

Not verified

• e2e sbom suites — not run here.
• The SaveImageToStream → tarball.Image path — I checked the types line up, not that it runs; worth a spike before committing to it.

[reyreavman]

Verification, after e8bb5cf and cec7a64

• Two independent reviews of e8bb5cf (the fail-open), different model families. Both APPROVE; both confirm the pre-check and the recorder gate on the same predicate over the same walk (Has == false ⇒ recorded == 0 for every input), that the removed symlink guard was unreachable, and that the logboek capture in the specs is sound at the default accepted level (Warn passes, the success-path Debug does not).
• Both raised the same thing I had missed: fail-open makes the degraded SBOM sticky. The cache is keyed on calculateStableChecksum, which does not encode whether occurrences were recorded, so a transient failure on the second read warns once and the SBOM without records is cached under the same checksum; no later build of that stage retries. cec7a64 makes the warning name the image and its digest and say the SBOM is cached as such until the stage is rebuilt. The consequence itself is inherent to fail-open and left as is; the body now states it, and the lack of a force-regenerate flag with it.
• e2e sbom suite re-run on the Linux VM against e8bb5cf: 66 passed, same 3 pre-existing failures as on clean main. cec7a64 changes only the warning text and a test, so it was not re-run there; unit suites over ./pkg/sbom/... ./pkg/build/ ./pkg/oci/... are green on the VM against cec7a64 except the pre-existing LFS spec.
• Spec does not read the image when no component could be given files renamed to warns nothing and skips the read … — it proves the former only through the latter, as both reviews noted.

[reyreavman]

Code Review — follow-up on cec7a642

Delta since e8bb5cfb: 2 files, +20/-14 — warning text only.

Verdict

Not approving yet. The commit responds to a finding from another review (a transient failure on the second read caches a degraded SBOM under the same checksum, never retried) by describing the defect in the warning rather than fixing it. My blocking item from the previous round — the second full pull, 9 m 48 s on a 1 GB image, no opt-out — is unchanged, and the body still says "no opt-out flag".

The two are one problem. The degraded-cache hazard exists only because the second read goes to the registry and can fail transiently; a deterministic failure (files.json that does not parse) is correctly cached, since rebuilding will not change it.

What I would need to approve

Either read the image locally — it is already in the backend after :106; SaveImageToStream on both backends → tarball.Image gives the v1.Image CollectFilesets wants. That removes the 10-minute pull and the transient-failure class, so what is left to cache is deterministic and the new warning text becomes unnecessary.

Or, if the registry read stays: make a degraded result a cache miss. PushSBOM carries annotations (io.werf.checksum at sbom_step.go:101); add e.g. io.werf.sbom.files-recorded=false when recordShippedFiles bailed out, and extend the hit check at :101 with && desc.Annotations[...] != "false". Next build retries; a healthy registry heals the SBOM without anyone deleting artifacts by hand. Plus an opt-out flag, since 10 minutes per build is not something to impose without one.

Telling the user in a log line that the only remedy is to delete the artifact manually is the weakest of the three options, and the body now documents it as the behaviour.

Everything else in the PR is ready; 139/139 shippedscope, pkg/build red only on the pre-existing LFS specs.

[reyreavman]

5aa1c59 — the image is read out of the backend

Taking the first of your two options. recordShippedFiles now gets the v1.Image from step.containerBackend.SaveImageToStream → a file under the werf tmp dir → tarball.ImageFromPath, the path MutateAndPushImage already takes (mutation.go:137-168). OCIStore.ParentImage is gone with its only caller; store.go is back to what main has. The warning about the degraded SBOM being cached is gone too, for the reason you gave: with no network in the read, what is left to fail is the host — an image that is not there, a files.json that does not parse — and that is right to cache, the next build would produce the same SBOM. The body is updated to say this instead of the 9 m 48 s figure.

Why a file rather than the stream: tarball.Image reads the tarball once for its manifest and once per layer, which a stream cannot give. On Buildah that means two copies on disk for the duration of the io.Copy — its SaveImageToStream writes its own temp and removes it on Close. Same as MutateAndPushImage; avoiding it would need a save-to-path on the Buildah backend, which I did not think this PR should grow.

The indexer sees the same thing either way: it uses Layers() and Uncompressed(), and both tarball.Image and remote.Image order layers by config.RootFS.DiffIDs. I checked that tarball.Write with a DockerUncompressedLayer puts raw tar bytes in the saved tarball, not gzip, so the second table entry in the spec exercises the branch Buildah's define.Uncompressed output takes, and the first the gzip one Docker may emit.

Verification

• e2e sbom suite on the Linux VM against 5aa1c59's production code: 66 passed, 3 failed, 1 pending, 2 skipped. The three are the same three that fail on clean main (69de5842, checked in a separate worktree with -focus on exactly them). This suite is the only thing that drives the real Docker SaveImageToStream through this path; the unit specs drive a tarball tarball.Write produces.
• Two independent reviews of the commit (different model families), both approve. Both named the same two minors and both are in: the godoc claimed the read "cannot fail for a reason the next build would not hit again", which ENOSPC contradicts, so it now says what can fail it is the host rather than the network; and the specs exercised only the gzip branch — see the table entry above.
• Mutations on the new code: defer cleanup() removed, cleanup body emptied, pre-check disabled — each fails a named spec; the temp-file assertion globs werf.GetTmpDir() after each run.
• task build, task lint 0 issues; unit suites over ./pkg/sbom/... ./pkg/build/ ./pkg/oci/... green locally and on the VM except the pre-existing LFS spec.

The three pre-existing e2e failures

They are not this PR's, but I looked, since I kept claiming they were environmental and they are not. Two are the go.sum guard from #434 (formatLockGuard, packages_commands.go:100) meeting fixtures that predate it: inject/gomod_subdir adds / from the git root so /app/go.sum is not there (werf says so: stage packages dependency path "go.sum" has not been found in own git), and type_change/state1 has no go.sum at all because its only dependency is a local replace. The third is the VCS-first resolution from #424: the mock in purl_resolver_errors_test.go answers 200 to /api/v1/vcs because it only inspects the purl query parameter, so curl/openssl are confirmed by their repo link and /resolve is never asked. Neither PR ran this suite; CI on main does not either right now, e2e_* is skipped behind a red unit. Separate fix, separate PR.

[reyreavman]

Code Review — follow-up on 5aa1c59d

Delta since cec7a642: 3 files, +148/-65.

Verdict

This is the fix, and it closes both blocking items in one move. The image is now read out of the backend the step already pulled it into (SaveImageToStream → temp tar → tarball.ImageFromPath), so:

• the second registry pull is gone — the 9m48s on a 1 GB image becomes a local save;
• the degraded-cache hazard is gone with it, correctly reasoned in the commit body: a local save fails only for deterministic reasons (image absent, files.json unparseable, disk full), which are right to cache since the next build hits them again. The "cached as such until rebuilt" warning text is dropped because it no longer describes a transient-failure trap.

OCIStore.ParentImage loses its only caller and is removed — lint clean, no orphaned imports.

Verified

• stageDesc.Info.Name is the right reference: it is what the whole step addresses the per-platform image by — pulled at sbom_step.go:110 with TargetPlatform, scanned at :123, os-pm at :151. SaveImageToStream reads that same local image, so multi-platform resolves to the converging platform, as ParentImage's digest did.
• The Buildah path is covered directly: DescribeTable runs the happy path over both a gzipped layer and static.NewLayer(..., DockerUncompressedLayer) — Buildah saves uncompressed, and tarball.ImageFromPath reading either is now pinned, not assumed.
• Temp file is removed on every path: cleanup on each error return in savedImage, defer cleanup() after success; specs assert savedTarballs() is empty.
• task build OK, lint 0 issues, recordShippedFiles specs pass; pkg/build red only on the pre-existing LFS specs (same on main).

Note, non-blocking

A ~1 GB image is now written to a tarball under the werf tmp dir before it is read. That trades the network pull for local disk + CPU, which is the right trade, but on a disk-constrained runner it is a new failure surface — correctly fail-open (warns, leaves the SBOM unmarked), and the commit comment already names "a disk that is full". Worth a line in the body's cost section alongside the download figures it replaces.

Nothing outstanding on this PR from my side — I would approve.

[reyreavman]

0e8888a — the end-to-end gap is closed

Earlier comments said no e2e spec asserted on evidence.occurrences; since recordShippedFiles is fail-open, a green suite only showed the record breaks nothing, not that it happens. test/e2e/sbom/pip_test.go now has records which files of the image each pip package consists of: it builds inject/pip_simple through werf build, reads the SBOM back out of the registry with werf sbom get, and asserts requests@2.32.3 carries occurrences — absolute paths of its own files, each with sha256:<64 hex> or symlink, requests/__init__.py among them.

Run on the Linux VM, Docker backend, against the real pipeline (SaveImageToStream → tarball → indexer → BOM → push → sbom get): passes in 81 s. With the single call to recordShippedFiles in ConvergeWithMerge removed, it fails on requests carries no evidence: the files it consists of were not recorded — same VM, same 81 s. Tree restored after.

Also aa8891e: imageSaver moved out of sbom_step.go, whose mockgen -source directive would otherwise have generated a mock for it and failed mock:check, which is why the lint job on 5aa1c59 was red.

os-pm occurrences are not asserted on this fixture: its base is installed by a pm that writes no files.json (the index.json is present but files.json is not warning in the build log), so python stays unmarked by design, not by omission.