Skip to content

feat(sbom): track Go and Rust dependencies in shipped binaries - #466

Merged
reyreavman merged 7 commits into
mainfrom
feat/sbom/produced-binaries
Oct 22, 2026
Merged

reyreavman merged 7 commits into
mainfrom
feat/sbom/produced-binaries

Conversation

@reyreavman

@reyreavman reyreavman commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

SBOM-enabled Stapel builds now distinguish Go modules and Rust crates backed by shipped, build-produced binaries from dependencies present only in manifests. Binary evidence survives selective image imports and SBOM regeneration from cached build stages.

What

Evidence and imports

  • Count newly written or changed binary content from install, beforeSetup and setup only when stage ancestry and layer boundaries are verified; package-installation, base-image and import stages are not local production.
  • Record digest-bound binary occurrences in CycloneDX and mark a dependency werf:shipped=false only when complete evidence establishes that its binaries are absent from the final filesystem.
  • Preserve inherited unknown origins beside positive or checked-empty evidence; bind unreadable-binary uncertainty to that binary's content. Selective and repeated imports retain this evidence; a manifest-only import does not prove its dependencies ship.
  • Resolve same-layer hardlink chains against their owning layer, including captured content and historical symlink targets. Unresolved or flattened cross-layer fallback in the final image or a stage boundary keeps binary attribution unknown; it can prevent negative Go/Rust verdicts for the image.

Caches and diagnostics

  • Persist the reserved werf.io/sbom-stage-v1 label for newly built SBOM-enabled Stapel stages, preserving it through label removal and Docker commit quoting. Existing cached image stages remain reusable without that label; their provenance stays unknown until a new build supplies a verifiable chain.
  • Re-read stage provenance before choosing the SBOM cache key. When a failed registry read recovers, the recovered provenance changes that key; SBOM evidence can regenerate without rebuilding image stages.
  • Classify candidate binaries only in eligible layers. Boundary reconstruction still indexes image prefixes; default-level output reports actual filesystem boundary passes, scanned layers and elapsed time, and identifies hardlink fallback when it prevents attribution.
  • Advance the SBOM artifact format to invalidate incompatible SBOM caches without changing image-build cache keys or adding configuration switches. Dockerfile production attribution, other ecosystem matching rules and the public low-level Match API are unchanged.

Why

A manifest says which dependencies were resolved, not which ones were compiled into the binaries that an image ships. Inspecting every executable would also attribute base-image and toolchain binaries to the current build, while keeping evidence only in memory would lose it on cached builds and downstream imports. Verified stage boundaries identify local production; persisted content-bound evidence lets each descendant judge its own filesystem without discarding uncertainty from other origins.

Record digest-bound binary evidence from verified production layers and preserve it through component merges and imports. Keep dependencies when provenance, classification, inherited evidence or hardlink reconstruction is incomplete.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Persist validated stage ancestry and layer boundaries for SBOM-enabled Stapel builds, including exact JSON transport through Docker commits. Regenerate binary evidence without rebuilding cached stages and fail open for older images or invalid ancestry.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Cover selective import from a two-binary donor, a second import and a manifest-only image. Assert dependency verdicts and occurrences remain stable after SBOM cache hits and regeneration from cached stages, while inherited unknown evidence remains conservative.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
@reyreavman

reyreavman commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Verification

  • Review-response commits: da082709d2, 6f49d044f0, b6a9ee3efa, 328ea41d72. Fresh format/build/lint and scoped restored-tree tests passed. Linux VM full unit run passed all 91 suites.
  • Linux VM, Docker backend: task test:e2e paths=./test/e2e/sbom labelFilter='sbom && (produced-binaries || pip)' parallel=1 passed all five selected specs. This includes the full two-binary/selective-import/re-import/manifest-only/cache/regeneration scenario and four Python cases exercising shared filesystem indexing.
  • Eight review-response mutations failed with assertions: removing the flattened-fallback guard; suppressing captured chain bodies; stopping owned-chain traversal; scanning excluded layers; losing the default cost diagnostic; swapping eligible/dependency stage order; omitting provenance from the SBOM cache key; rejecting old cached stages without a provenance label. Each mutation was restored before the green runs.
  • Earlier mutations also cover malformed/inherited evidence, unknown-origin merging, missing ancestors, rewritten layer prefixes and reserved-label removal. The Docker JSON transport regression failed before its fix, and the real Docker scenario passed afterward.
  • The new order test derives all 16 stages from MapStapelConfigToImages, including the four import stages, git, imageSpec, verity and signing. It does not use the incomplete stage.AllStages list or another copied ordering as its input.
  • Legacy integration was rerun: the VM's synthetic repository still lacks v1.0.10 and fails the five setup cases. The preceding local run established that the five checksum expectations also fail on the then-current clean base ac29d1bbf8. No legacy expectations or cleanup code were changed.
  • Not run: real Buildah production-attribution e2e and a Rust build/import e2e. The new end-to-end scenario uses Docker and Go.
  • CI for the review-response head is pending. The previous run 39668573452 failed in all five integration/e2e jobs with repeated shared-cleanup timeouts waiting for docker images; its lint, unit, binary build and Qlty passed. This is not a green-CI claim.

Review focus

  • Independent review of the response delta: APPROVE, no findings. The review specifically checked owning-layer resolution, captured historical bytes, remaining incomplete fallback, eligible-only classification and the mapper-derived order test.
  • The hardlink guard deliberately remains: simply dropping it allows a later replacement of a link target to hide surviving old binary bytes. Same-layer chains are now resolved precisely; cross-layer reconstruction is not expanded in this PR.
  • Prefix reconstruction remains multi-pass. The change removes non-eligible classification scans and makes the remaining work visible; it does not claim a new one-pass overlay engine or a measured whole-image speedup.

Follow-up

  • BLOCKER: Obtain green integration/e2e CI. Diagnose shared project-cleanup timeouts separately if they recur; no timeout increase, cleanup bypass or runner modification is included here.
  • Repair the pre-existing legacy checksum suite separately in deckhouse/delivery-kit (test/legacy_e2e/suites/build/dockerfile_image/context_test.go).

@reyreavman reyreavman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Report — static

Base: main (ac29d1bb, #464 merged), head 4687f022
Diff: 30 files, +2013/-53 — produced-set recovery from cached stage ancestry (sbom_provenance.go), per-layer binary attribution (produced.go), digest-bound binary evidence in CycloneDX (binary_occurrences.go), werf.io/sbom-stage-v1 label through build/commit/image-spec, format 13 → 14, e2e.
Context: cards D→E→F closing the Go/Rust side; concept §1.1 (per-stage produced set), §3.3, §4 pass 1a.
Mode: static, as requested — read every changed file, no build/test/mutation. Line refs are from the head blobs.

Verdict

  • Technical: The design is the concept's: produced set = diff(install) ∪ diff(beforeSetup) ∪ diff(setup) by verified layer boundaries, never packages/dependencies*/base; evidence is digest-bound and survives import by content; every gap (no label, broken ancestry, mismatched DiffIDs, hardlink uncertainty) resolves to unknown → keep. Reading it, I did not find a path to a false drop. I did find one false-positive-prone fail-open that will neuter the feature on common images (Major #1), and one place where recovery cost is unbounded (Major #2). Everything else is Minor.
  • Product: This is the increment where Go modules first get werf:shipped=false in real SBOMs. The eligibility gates are so conservative that on a stock glibc base most images will say werf:binaries=unknown for every module and mark nothing — correct, but the body should say so, or the first measurement will read as "the feature does nothing".
  • Risk: Medium, operational. A new label on every SBOM-enabled stapel stage changes stage content → image-build cache invalidates once for every such project. The body says the opposite.

DoD Criteria

From cards D/E/F "Готово, когда", concept §1.1/§3.3/§4, and the body.

Criteria Inferred? Met? Evidence
produced set per stage: install ∪ beforeSetup ∪ setup; packages/dependencies*/base excluded no ✅ sbom_provenance.go:115-117
stage boundaries verified against DiffIDs (parent is prefix) yes ✅ :110-114; collectShippedScope re-checks saved image DiffIDs
test binary contributes nothing no ✅ produced.go:188, binary_occurrences.go:77
1a: no exclusivity, any binary naming the module holds it no ✅ binary_occurrences.go:86-92
unknown verdict holds everything of that ecosystem no ✅ :93, :99-100; binaryDecision :197-204
produced set unknown → no-data (never "empty") no ✅ binaryDecision :206-207; RecordProducedBinaryOccurrences :72-74
evidence survives import, relocation, re-import by content no ✅ digest-keyed occurrences; NormalizeInheritedBinaryOccurrences before merge
cached SBOM invalidated no ✅ format "14" + provenance in checksum
"without invalidating image-build caches" (body) no ❌ build_phase.go:1832-1843 — Major #3
e2e: selective import, second import, manifest-only, cache hit no ⚠️ produced_binaries_test.go present; not run here
task format/build/lint/test:unit no — not run (static)

Issues

Major

  1. FileIndex.Incomplete is set whenever any hardlink falls to the flattened fallback — file_index.go:132-134. remaining after resolveHardlinksWithinLayers holds every link whose own layer did not answer. #433's own specs show legitimate cases that land there and resolve fine in the fallback: a link chain, a link whose target sits in a layer below. Any such link → Incomplete = true → CollectProducedFiles returns {} (produced.go:50) → werf:binaries=unknown on every module → nothing is ever marked for Go/Rust in that image. busybox/coreutils/glibc bases are full of hardlinks; one chain is enough. The intent ("a hardlink resolved against the final filesystem may hide bytes") is right, but the signal is too coarse: set Incomplete only in resolveHardlinks for the unresolved count (:363, already there), not for everything that reached the fallback. Then write a spec: image with a hardlink chain → Incomplete == false.

  2. Boundary reconstruction re-flattens and re-hashes the image once per distinct boundary — produced.go:70-83. Three eligible stages → up to five IndexImageFiles calls over growing prefixes of the same layer list, each a full mutate.Extract + sha256 of every file. On the node:20-class images #453 measured at minutes for one pass this is 3–5×, on every SBOM regeneration, with no cap and no log line. Two cheap improvements: (a) index boundaries incrementally — one top-down walk of the layer tars with the same whiteout logic, snapshotting the map at each boundary, is one pass; (b) at minimum, log the count and time at Default level so the cost is visible.

  3. The new label invalidates every SBOM-enabled stapel stage once — build_phase.go:1832-1843. serviceLabels become part of the committed image config; a new key with a new value is new stage content. The body claims "without invalidating image-build caches" — it will, exactly once per stage, on the first build after upgrade. That is acceptable (same as any stage-config change) but must be stated, and the sentence in the body removed.

Minor

  1. classifyProducedLayer spools and sha256s every executable in every layer, not only candidates — produced.go:105-113, :143-162. candidates is known before the loop; the digest is only available after reading the whole file, so the spool is unavoidable, but the scan can skip layers outside every range (a base-image layer cannot hold a produced file unless a produced file is byte-identical to a base one, which candidates would then match anyway — but then it is also in a produced layer). Restrict to imgLayers[r.Start:r.End] over the ranges.

  2. Three Decision/Reason paths for binary-backed components now exist — match.go:goVerdict, binary_occurrences.go:binaryDecision, and MarkNotShipped bypassing Match for them (mark.go:55-63). Match still accepts ProducedBinaries and EcosystemGo/Rust components, but nothing passes them any more; MarkNotShipped hands ProducedBinaries{} and routes binary-backed components around it. Either delete the dead path in Match (and its specs) or make MarkNotShipped the only caller and say so in the Match godoc. Two rules for one question is how they drift.

  3. werf:binaries property semantics are undocumented at the type — binary_occurrences.go:14-20. none / unknown / absent, with unknown accumulated (never removed once added, :58, :73, :95) and none removed once a positive occurrence lands (:118-121). That lattice is correct and subtle; it needs a paragraph on PropertyBinaries the way PropertyShipped has one in mark.go:13-20.

  4. sbom_provenance.go:54-59 hardcodes the stage order. stage.AllStages or whatever the conveyor uses would keep this from drifting when a stage is added; if no such list exists, a spec asserting this slice equals the conveyor's order is the next best thing.

  5. ExactValues || k == image.WerfSBOMStageLabel — legacy_stage_image_container_options.go:218, :260. Special-casing one key in the escape path is a smell; the JSON record needs exact quoting, so either all werf.io/* labels go exact (they are all machine-written) or the record is base64'd and the escape path stays uniform. Covered by legacy_sbom_label_test.go, so it works — this is about the next label.

Risks

№ Risk Type Likelihood Severity Location Circumstances Consequences Recommendation
1 Go/Rust marking silently inert on hardlink-rich bases UX/Product Likely Medium file_index.go:132 Any base with a hardlink chain or cross-layer link binaries=unknown everywhere; F produces no data for H Major #1
2 SBOM regeneration time × boundaries Operational Likely Medium produced.go:70-83 Three eligible stages on a large image Minutes → tens of minutes per image Major #2
3 One-time rebuild of every SBOM-enabled stage Operational Certain Low build_phase.go:1832 First build after upgrade Expected, undocumented Major #3: fix the body
4 Provenance read fails on a registry hiccup → unknown for the whole build, cached under a checksum that encodes Known=false Operational Possible Low sbom_step.go:98-105, sbomProvenanceChecksum Transient GetRepoImageConfigFile error SBOM cached without binary evidence until stage digest moves — the #453 trap, one level up Fold a failed provenance read into a cache miss (do not encode Known=false into the checksum; recompute next time)

Not verified

  • Nothing was built, linted or run — static pass by request. Specifically unverified: that produced_test.go / produced_overlay_test.go fail under the mutations I would try (Incomplete set only on unresolved; ranges restricted; none not removed on positive evidence), and that the e2e fixture builds on linux/amd64.
  • #464 (the base) was not reviewed by me; CloneBOM and the import-evidence preservation it added are taken as given.

Comment on lines +132 to 134
index.Incomplete = true
}
resolveHardlinks(ctx, index, remaining)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major. remaining here is every hardlink whose own layer did not answer — that includes links #433 resolves correctly in the flattened fallback (a chain, a target in a lower layer; both have specs). Setting Incomplete on all of them makes CollectProducedFiles return {} (produced.go:50) → werf:binaries=unknown on every module → Go/Rust marking never fires for that image. On a busybox/coreutils/glibc base one chain is enough.

The guard you want is already at :363: set Incomplete only when resolveHardlinks fails to resolve a link (the unresolved counter). Drop this block, and add a spec: image with a→b→origin → Incomplete == false.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in da08270, but not by removing the safety guard. Same-layer chains now resolve through owning-layer maps, propagate captured bodies and remain complete even when the terminal path is later replaced or removed. Regression tests cover regular content, captured historical bytes, symlink targets and produced binaries surviving only through the chain.

The proposed blanket removal is unsafe: layer 0 writes z=old binary, layer 1 creates a -> z, and layer 2 replaces z. Flattened fallback can credit a with the replacement bytes, hiding the surviving old binary. Cross-layer/unverified fallback therefore remains incomplete. The mutation removing that guard failed the overlay assertions; the PR now explicitly describes this remaining limitation.

Comment on lines +70 to +83
for _, boundary := range []int{r.Start, r.End} {
if _, ok := indexes[boundary]; ok {
continue
}
prefix, err := mutate.AppendLayers(empty.Image, imgLayers[:boundary]...)
if err != nil {
return nil, fmt.Errorf("read stage filesystem boundary %d: %w", boundary, err)
}
boundaryIndex, err := IndexImageFiles(ctx, prefix)
if err != nil {
return nil, fmt.Errorf("index stage filesystem boundary %d: %w", boundary, err)
}
indexes[boundary] = boundaryIndex
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major (cost). Each distinct boundary that is not 0 or N triggers mutate.AppendLayers + IndexImageFiles over a prefix of the layer list — a full flatten + sha256 of every file up to that point. With install/beforeSetup/setup eligible and each adding layers, that is up to 5 full passes over growing prefixes of the same image, on top of the one CollectFilesets already did. #453 measured one pass at 65 s on node:20-slim; this is a multiple of that on every SBOM regeneration, and nothing logs it.

One top-down walk with the whiteout bookkeeping from mutate.go:295-397, snapshotting the path→digest map at each boundary, does all boundaries in a single pass. If that is out of scope for this PR, at least log boundary count and elapsed at Default so the number is visible in CI logs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed with the bounded alternative in 6f49d04: default-level output reports actual filesystem boundary passes, scanned layers and elapsed time, and identifies hardlink fallback when it prevents attribution. Classification also skips non-eligible layers.

Prefix reconstruction itself remains multi-pass, explicitly stated in the PR description; this change does not claim a new overlay engine or an unmeasured 3–5x speedup. The new scan-cost regression counts real layer openings: two base-prefix reads are retained, while redundant base/final-layer classification reads are eliminated. Reintroducing excluded-layer scans or removing the diagnostic causes assertion failures.

Comment thread pkg/build/build_phase.go
Comment on lines +1832 to +1843
if phase.Conveyor.EnableSbom() && img.StapelImageConfig != nil {
if _, ok := serviceLabels[imagePkg.WerfParentStageID]; !ok {
serviceLabels[imagePkg.WerfParentStageID] = ""
}
record, err := json.Marshal(sbomStageRecord{
Stage: stg.Name(), StageDigest: stg.GetDigest(), ParentStageID: serviceLabels[imagePkg.WerfParentStageID],
})
if err != nil {
return fmt.Errorf("encode SBOM stage record: %w", err)
}
serviceLabels[imagePkg.WerfSBOMStageLabel] = string(record)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major (claim). A new service label with a per-stage value is new committed config → new stage content → every SBOM-enabled stapel stage rebuilds once after upgrade. The PR body says "without invalidating image-build caches". That sentence is false and should go; the one-time rebuild itself is fine and worth a line in the release notes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cache-invalidation claim does not match the actual lookup order. BuildPhase.onImageStage finds a suitable cached stage and returns before calling prepareStageInstructions; the new service label is written only after a miss. A newly committed image config has different content, but that does not change the recipe/anchor key used to select an existing stage.

b6a9ee3 adds regressions for both image-anchor reuse and an individual cached stage with SBOM enabled and no provenance label. Both pass, leave the old label absent and avoid rebuilding. A mutation rejecting such a cached stage fails the regression. The description now states the precise consequence: old image caches remain reusable, but their binary provenance remains unknown until a newly built chain supplies it. No release-note claim of an unavoidable rebuild was added.

Comment thread pkg/sbom/shippedscope/produced.go Outdated
Comment on lines +105 to +113
for _, layer := range imgLayers {
files, err := classifyProducedLayer(ctx, layer, candidates)
if err != nil {
return nil, err
}
for _, file := range files {
found[file.Digest] = file
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor (cost). Every layer of the image is scanned and every executable spooled + hashed, but a file can only be a candidate if it was written in a produced range (:88-97). A byte-identical copy in a base layer would match the same digest, but then the produced-range copy exists too and is found there. Restrict to the layers inside layers.Ranges; on a 1 GB base with a 10 MB app layer that is the difference between reading 1 GB and 10 MB.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 6f49d04: classification visits only layers in the eligible ranges, deduplicated by layer index. The regression counts actual Uncompressed openings and fails if scanning all layers is restored.

This optimization depends on keeping the completeness guard: admitted hardlink content must come from the owning layer. Same-layer chain resolution preserves that invariant. If precise cross-layer resolution is added later, classification must also track source layers or use an appropriate fallback; that dependency is documented in the collector godoc.

Comment on lines +55 to +63
if isBinaryBacked(key.ecosystem) {
decision := binaryDecision(comp, filesets.Index.Incomplete, byDigest)
if !decision.Shipped {
setProperty(comp, PropertyShipped, propertyShippedFalse)
count++
logboek.Context(ctx).Debug().LogF("not shipped %s %s@%s (%s)\n", key.ecosystem, key.name, key.version, decision.Reason)
}
return
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor. Binary-backed components now bypass Match entirely and go through binaryDecision; Match still has goVerdict/rustVerdict and takes ProducedBinaries, but this is its only caller and passes ProducedBinaries{} with no Go/Rust components. Two implementations of pass 1a is how they drift. Remove the dead path from Match (with its specs) or document that Match is file-backed only.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified in 328ea41 without removing the public low-level API or its tests. Match accepts a normalized evidence set; it cannot represent the completeness of multiple origins in a persisted merged BOM. MarkNotShipped handles that persisted binary evidence and delegates file-backed components to Match.

The Match godoc now directs persisted-BOM callers to MarkNotShipped and explains this boundary. Calling Match “file-backed only” would be inaccurate: its public Go/Rust behavior remains supported and tested.

Comment on lines +14 to +20
const PropertyBinaries = "werf:binaries"

const (
binaryEvidencePrefix = "werf:binary:v1:"
propertyBinariesNone = "none"
propertyBinariesUnknown = "unknown"
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor. The werf:binaries lattice — none means "checked, nothing produced names it", unknown means "some origin could not say", absent means "a positive occurrence exists"; unknown accumulates across merges and is never removed, none is removed on the first positive — is correct and subtle. PropertyShipped has that paragraph in mark.go:13-20; this one needs the same, since a downstream reader (and the next PR) will have to reconstruct it from binaryOccurrences :160-188.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented on PropertyBinaries in 328ea41: checked-empty none, accumulated origin-level unknown, replacement of none by positive or digest-bound-unknown occurrences, and preservation of inherited uncertainty.

One important qualification: absence of the property alone does not mean positive evidence exists. With neither a recognized completeness property nor a valid binary occurrence, the component is unknown. The godoc states that explicitly.

Comment thread pkg/build/sbom_step.go
Comment on lines +98 to +105
var provenance sbomStageProvenance
if isStapel {
var err error
provenance, err = resolveSBOMStageProvenance(ctx, stageDesc, docker_registry.API().GetRepoImageConfigFile)
if err != nil {
logboek.Context(ctx).Debug().LogF("image %s: produced binaries are unknown: %s\n", werfImgName, err)
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk, non-blocking. A transient failure in GetRepoImageConfigFile → provenance.Layers.Known == false → that is encoded into the checksum (sbomProvenanceChecksum) → the SBOM is pushed and cached under a key that says "unknown". The next build recomputes the same key and hits the cache — the degraded SBOM stays until the stage digest moves. Same shape as the trap #453 closed by moving the read local. Here the read is to the registry by nature; the fix is to not let a failed read into the checksum: on error, skip the cache lookup for this build (force a miss) rather than encoding Known=false. A deterministically unknown provenance (old image, no label) is fine to cache.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed sticky-cache failure does not occur with the current ordering: ConvergeWithMerge resolves provenance before calculating the checksum and before querying the SBOM cache on every invocation. Once the registry read recovers, Known, DiffIDs and ranges change the key, so the degraded artifact is not reused under the recovered key.

b6a9ee3 adds a transient-failure → recovery → stable-success regression using the real resolver and full SBOM checksum. Removing provenance from that checksum makes it fail. The existing missing-ancestor checksum regression remains. No forced cache-miss policy was added; the PR description now states the recovery behavior.

Resolve chains and captured content using only their owning layer. Keep flattened fallback incomplete so overwritten or cross-layer targets cannot turn surviving binary content into a false absence.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Limit classification to eligible layers while retaining the same-layer completeness guard. Report filesystem boundary passes, scanned layers, elapsed time and hardlink fallback at the default log level; keep prefix reconstruction unchanged.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Feed all sixteen mapper-generated stages through provenance recovery, including imports and signing. Cover registry recovery changing the SBOM cache key and image-cache hits without the new provenance label.

Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
Signed-off-by: Radmir Khurum <radmir.khurum@flant.com>
@reyreavman

Copy link
Copy Markdown
Collaborator Author

Remaining general-review points:

  • chore(main): release 2.57.1 #7 (stage order): b6a9ee3 adds a real MapStapelConfigToImages fixture covering all 16 stages, including all four import stages, git, imageSpec, verity and signing. Its generated chain must pass the provenance resolver and yield exactly the three eligible ranges. Swapping install/dependency order in the resolver fails the test. stage.AllStages is not substituted because it omits Packages, VerityAnnotation and Sign.
  • refactor(image-spec): adopt image spec impl  #8 (label quoting): retained exact quoting for this reserved JSON key only. Generalizing it to all werf.io/* keys would alter existing legacy interpretation, while base64 would change the agreed persisted format. Neither is needed to fix the observed transport bug. Both exact-value modes and both Docker commit-change paths are covered, and the real Docker e2e passed.

All seven inline threads have individual responses with evidence. The review-response delta has a fresh independent APPROVE. On the Linux VM, the full unit suite passed (91 suites), and the produced-binaries + pip e2e selection passed (5 specs). The PR remains draft; the new CI result is not yet established.

@reyreavman reyreavman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — follow-up on da082709…328ea41d

Delta since 4687f022: 4 commits, 17 files, +381/-16.

Verdict

All findings closed, and one of my Majors was wrong — I retract it below. Static review plus, this round, build + targeted tests + one mutation.

# Finding Status
Major 1 Incomplete set for every flattened-fallback hardlink ✅ fixed — chains resolve within the owning layer; only genuine fallback stays Incomplete. file_index_chains_test.go pins all three cases (same-layer chain → Incomplete=false + captured bytes; intermediate target only in flattened view → Incomplete=true; produced binary surviving through a chain → classified). My mutation (revert chain pre-capture to single-hop) fails the capture spec.
Major 2 boundary reconstruction cost unbounded/invisible ✅ acceptably — classification restricted to eligible layers (produced.go:124-131), and boundary passes / scanned layers / elapsed / hardlink-fallback now logged at Default. The multi-pass prefix reconstruction is kept, which I flagged as the acceptable fallback; the cost is now visible in CI.
Major 3 new label invalidates image-build caches ❌ retracted — I was wrong. introspect_anchor_test.go proves the opposite: an SBOM-enabled image reuses a stage cached before the provenance label existed (AnchorReused=true, Rebuilt=false, reused stage has no WerfSBOMStageLabel, processedStages empty). It is a service label like WerfParentStageID, excluded from the content digest. The body claim is correct and now test-pinned.
Minor 4 every layer scanned, not just produced ranges ✅ eligibleLayers gate
Minor 5 two 1a implementations drift ✅ Match godoc now routes persisted BOMs to MarkNotShipped and declares itself file-backed-only
Minor 6 werf:binaries lattice undocumented ✅ godoc on PropertyBinaries
Minor 7 hardcoded stage order ✅ sbom_stage_order_test.go drives all 16 mapper-generated stages through recovery
Risk 4 transient provenance → degraded SBOM cached, not retried ✅ provenance is in the checksum, so a recovered read computes a different key than the failed one (recovers from a transient registry error before selecting the SBOM cache key): the degraded SBOM self-heals on the next successful build instead of sticking. Cleaner than the cache-miss I suggested.

Verified this round

  • task build OK, lint 0 issues, shippedscope 246/246, the new pkg/build anchor/provenance/stage-order/produced specs pass.
  • Mutation on resolveHardlinksWithinLayers chain pre-capture → file_index_chains_test.go keeps captured historical bytes after the terminal target is replaced fails as expected.

Not verified

  • e2e produced_binaries — compiled, not run (linux/amd64 + kind); the body says it was exercised on a linux host.
  • #464 (base) not reviewed by me.

Nothing outstanding on this PR from my side — I would approve. This closes the Go/Rust side of the concept: #433 → #435 → #442 → #452 → #453 → #459 → #464 → #466.

@reyreavman
reyreavman marked this pull request as ready for review October 22, 2026 09:02
@reyreavman
reyreavman merged commit 359006b into main Oct 22, 2026
8 of 13 checks passed
@reyreavman
reyreavman deleted the feat/sbom/produced-binaries branch October 22, 2026 09:02
reyreavman pushed a commit that referenced this pull request Oct 22, 2026
🤖 I have created a release *beep* *boop*
---


##
[3.12.0-dk.2](v3.12.0-dk.1...v3.12.0-dk.2)
(2026-10-22)


### Features

* **build, stapel:** sign files inside the image with a key the build
container never sees
([#438](#438))
([09f22e7](09f22e7))
* **sbom:** track Go and Rust dependencies in shipped binaries
([#466](#466))
([359006b](359006b))


### Bug Fixes

* **sbom:** preserve shipped-scope evidence across image imports
([#464](#464))
([ac29d1b](ac29d1b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant