Skip to content

fix(ci): scan the image by digest and platform, not by tag - #1205

Open
AlexKantor87 wants to merge 1 commit into
mainfrom
claude/sbom-scan-by-digest
Open

AlexKantor87 wants to merge 1 commit into
mainfrom
claude/sbom-scan-by-digest

Conversation

@AlexKantor87

Copy link
Copy Markdown
Contributor

The image is built for two platforms, merged into one OCI index, and that index digest is what gets attested to Kosli. The SBOM was generated from IMAGE:TAG.

Syft resolves a multi-platform tag to whichever platform the runner is, and names that image. So the SBOM's subject was never the attested index. On the live attestation the subject digest was 02eb3b33…, which returns 404 from ghcr.

The fix

Pin the index digest and name the platform, so the subject is the index's entry for that platform whatever the runner is. Then assert it, so the build fails rather than attesting an SBOM about something else.

anchore/sbom-action has no platform input, so this calls syft directly and keeps the action for the install.

Verified

Ran the step extracted from the YAML against ghcr, from an arm64 host.

run subject
pinned index, --platform linux/amd64 0a222bfc…, the index's amd64 entry. Passes
pinned index, --platform linux/arm64 d7f84a11…, its arm64 entry
the tag, as today d7f84a11… on this host. That is the defect

Mutations, each caught by name: drop --platform, tamper the subject, point DIGEST at a single manifest. Removing the comparison and using the wrong platform exits 0, so the comparison is the only thing catching it.

Full suite green on this tree: 3175 tests, 53 skipped, no failures.

Still one platform

Syft scans one platform, so the SBOM covers amd64 of two. Covering both needs one attestation per architecture, and static template slots. But platforms is a required input with no fixed value, so slots would break when a caller changes the list. The attestation now records sbom_platform instead of leaving the coverage to be inferred. Worth a follow-up.

🤖 Generated with Claude Code

The SBOM was generated from IMAGE:TAG. Syft resolves a multi-platform tag
to whichever platform the runner is, and names that image. The artifact
attested to Kosli is the index digest, so the SBOM's subject was never the
artifact, and on the live attestation it was a digest that does not exist
in the registry at all.

Pinning the index digest and naming the platform makes the subject the
index's entry for that platform, whatever the runner is. The step then
asserts that, so the build fails rather than attesting an SBOM about
something else.

The SBOM still covers one platform of two. That is recorded on the
attestation as sbom_platform rather than left to be inferred.

verified: ran the step extracted from the YAML against ghcr, from an arm64
          host. Pinned to the index with --platform linux/amd64 it produced
          subject 0a222bfc..., the index's amd64 entry, and passed. The
          same index with --platform linux/arm64 produced d7f84a11..., its
          arm64 entry. Scanning the tag on this host produced the arm64
          digest, which is the defect.

mutation: drop --platform -> the subject becomes the host's platform and
          the comparison fails by name. Tamper the subject in the file ->
          fails by name. Point DIGEST at a single manifest -> the index
          check fails by name. Remove the comparison and use the wrong
          platform -> exit 0, so the comparison is the only thing catching
          it.

search: grep -rn container-sbom over the repo finds main-flow-template.yml,
        release-flow-template.yml and this step; no policy reads it, and no
        other step generates an SBOM from a tag. Snyk's container test still
        uses the tag, which is a scan and not an attested document.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @AlexKantor87's task in 4m 1s —— View job


Review: scan the image by digest and platform

The diagnosis is right and the fix is the right shape. Resolving a multi-platform tag through syft gives you whichever platform the runner happens to be, so the SBOM's subject was a manifest that the attestation never mentioned — hence the 404 on 02eb3b33…. Pinning the index digest and naming the platform fixes it, and the assertion is the more valuable half of the change: without it the correct behaviour is unverifiable from the outside, and the mutation results in the description (drop --platform, wrong platform → exit 0) show that comparison is the only thing standing between you and a silently wrong subject again. The empirical table from an arm64 host is exactly the evidence this change needed.

Everything below is refinement — nothing blocks merge.

Checks run

  • go build ./... — clean (no Go changed; go vet / go mod tidy moot for a YAML-only diff)
  • Verified --annotate sbom_platform=… is accepted: attestSbom.go:297 reserves only sbom_format and sbom_sha256, so the new key passes PreRunE and merges in annotate() at attestSbom.go:320
  • Traced the jq platform selection against a buildx-shaped index (incl. the unknown/unknown attestation entries, correctly excluded)
  • Not run: the workflow itself. Its behaviour is only observable on a real ghcr push.

Inline findings

# Line Issue
1 302 No set -o pipefail — a failed imagetools inspect is reported as "not a multi-platform index"
2 212 SBOM_PLATFORM hardcoded while platforms is a required input; nothing ties them together
3 308 Platform match drops variant; the next RUNNER_MAP entry needing one (linux/arm/v7) fails obscurely
4 313 Nits: guard placement, ${DIGEST} inside a shell comment, empty-subject message

All four are fail-closed. The common thread is that a step whose entire purpose is to name a mismatch has three paths that name the wrong cause.

The same defect is still live two steps down

.github/workflows/docker.yml:370:

snyk container test ${{ env.IMAGE }}:${{ inputs.tag }}

This is the bug this PR fixes, unfixed. Snyk resolves the multi-platform tag to the runner's platform, and the result is attested at line 384 against FINGERPRINT — the index digest. So snyk-container-test claims a vulnerability scan of the index while scanning one arch of it, non-deterministically, with no annotation recording which. Same root cause, same class of wrong subject, and it happens to agree with linux/amd64 today only because the merge job runs on ubuntu-latest.

Out of scope for this PR, but it belongs in the same follow-up as the "still one platform" note — the fix is the same two moves (${IMAGE}@${DIGEST}, an explicit platform), and leaving it means half the attestations on this trail are pinned and half are not. Fix this →

One question on the GitHub attestation

Line 327 attests sbom.spdx.json with subject-digest set to the index digest, while the SBOM now provably describes the amd64 manifest. The Kosli side records sbom_platform; the GitHub/sigstore side does not, so a consumer pulling that attestation sees an SBOM presented for the whole index with the narrowing recorded only inside the SPDX root package. That may well be what you want — the index is the thing people look up — but if so it's worth a line of comment saying so, since the step immediately above it just went to some trouble to establish that those two digests are deliberately different.

Worth keeping

  • Dropping to syft directly and retaining anchore/sbom-action/download-syft for the install is the minimal change that gets --platform; resisting the urge to hand-roll the install is the right call.
  • Pinning @${DIGEST} rather than re-resolving the tag also closes a smaller race — the tag could move between imagetools create and the scan.
  • .manifests[]? handling the single-manifest case, and the separate -z branch distinguishing "no such platform" from "wrong subject", are the kind of detail that gets skipped.
  • The PR description documents the residual single-platform coverage instead of letting it be inferred. That, plus the sbom_platform annotation, is the honest version of a partial fix.
    · branch claude/sbom-scan-by-digest

Comment on lines +302 to +304
run: |
syft -q -o spdx-json --platform "${SBOM_PLATFORM}" \
"registry:${IMAGE}@${DIGEST}" > sbom.spdx.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing pipefail turns a registry failure into a misleading error.

The default shell here is bash -e {0}-e but not -o pipefail. In the pipeline on line 308, only jq's status survives:

expected=$(docker buildx imagetools inspect "${IMAGE}@${DIGEST}" --raw | jq -r ...)

If imagetools inspect fails (transient ghcr 5xx, rate limit, token expiry), it writes nothing to the pipe, jq reads empty input and exits 0, so expected is empty and the build fails on line 319 with:

ghcr.io/kosli-dev/cli@sha256:… is not a multi-platform index listing linux/amd64

which is a wrong diagnosis of a network problem, on a release pipeline, where someone will go looking at the manifest rather than at the retry. It fails closed, so nothing gets attested wrongly — it's the diagnosis that's wrong, and this step exists precisely so failures are legible.

Suggested change
run: |
syft -q -o spdx-json --platform "${SBOM_PLATFORM}" \
"registry:${IMAGE}@${DIGEST}" > sbom.spdx.json
run: |
set -o pipefail
syft -q -o spdx-json --platform "${SBOM_PLATFORM}" \
"registry:${IMAGE}@${DIGEST}" > sbom.spdx.json

Comment on lines +210 to +212
# Syft scans one platform. The image is multi-platform, so the SBOM covers
# this one only, and the attestation records which.
SBOM_PLATFORM: linux/amd64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SBOM_PLATFORM is a constant, but platforms is a required caller input.

Both callers pass linux/amd64,linux/arm64 today (main.yml:118, release.yml:121), so this is correct as merged. But the two values are now coupled with nothing tying them together: a caller that builds linux/arm64 only gets a hard failure at line 319 reading "is not a multi-platform index listing linux/amd64" — true, but it points at the registry rather than at the input that actually disagrees.

Since prepare already parses inputs.platforms into a matrix, the cheapest fix is to emit the first entry as a job output and read it here, which also makes the "SBOM covers one of N" caveat self-maintaining as RUNNER_MAP grows. Failing that, a one-line assertion in prepare that the list contains linux/amd64 would name the real cause.

Comment on lines +308 to +312
expected=$(docker buildx imagetools inspect "${IMAGE}@${DIGEST}" --raw \
| jq -r --arg p "${SBOM_PLATFORM}" '
.manifests[]?
| select((.platform.os + "/" + .platform.architecture) == $p)
| .digest')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The platform match drops variant.

.platform.os + "/" + .platform.architecture ignores .platform.variant, which is deliberate and correct for linux/arm64 (the index entry carries variant: "v8", and the input string does not). But line 58 advertises RUNNER_MAP as the extension point — "To support a new platform, add one entry to RUNNER_MAP below" — and the first entry anyone adds that needs its variant, linux/arm/v7, silently never matches here. The failure is the line 319 error claiming the index doesn't list the platform, when it does.

Fail-closed, so no wrong SBOM escapes; it's the same misdirection as above. A comment pinning the assumption next to RUNNER_MAP, or matching on the variant when the input carries one, would keep the two ends in step:

select(([.platform.os, .platform.architecture, .platform.variant]
        | map(select(. != null)) | join("/")) == $p
       or (.platform.os + "/" + .platform.architecture) == $p)

Also worth noting .platform being absent on an entry is safe here — jq gives null.os == null and null + "/" == "/", no error, no match.

Comment on lines +313 to +321
subject=$(jq -r '
(.relationships[] | select(.relationshipType == "DESCRIBES") | .relatedSpdxElement) as $root
| .packages[] | select(.SPDXID == $root)
| .checksums[]? | select(.algorithm == "SHA256") | .checksumValue
' sbom.spdx.json)

if [ -z "${expected}" ]; then
echo "::error::${IMAGE}@${DIGEST} is not a multi-platform index listing ${SBOM_PLATFORM}"; exit 1
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit, ordering: the -z "${expected}" guard sits two statements away from the assignment it guards, with the unrelated subject extraction in between. Moving it directly under line 312 puts the check where a reader looks for it, and means the subject jq never runs in the case where the answer is already known to be a failure.

Two smaller things in the same block:

  • Line 306's comment writes ${DIGEST} inside a # comment, where it reads like an interpolation but is literal text. the index that DIGEST points at avoids the double-take.
  • If the subject query yields nothing (syft changes its SPDX shape, root package loses its checksum), the comparison fails with sbom.spdx.json describes sha256:, not the … — correct outcome, cryptic message. An explicit [ -z "${subject}" ] branch naming "could not find the SPDX root package checksum" would say what actually happened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant