Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ jobs:
artifact-metadata: write
env:
RESULTS_FILE: smoke-test-results.json
# 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
Comment on lines +210 to +212

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.


steps:
- name: Harden Runner
Expand Down Expand Up @@ -289,13 +292,37 @@ jobs:
subject-digest: ${{ steps.docker_build.outputs.digest }}
push-to-registry: true

- name: Install syft
uses: anchore/sbom-action/download-syft@v0

- name: Generate SBOM for the docker image
uses: anchore/sbom-action@v0
with:
image: ${{ env.IMAGE }}:${{ inputs.tag }}
format: 'spdx-json'
output-file: 'sbom.spdx.json'
upload-artifact: false
env:
IMAGE: ${{ env.IMAGE }}
DIGEST: ${{ steps.docker_build.outputs.digest }}
run: |
syft -q -o spdx-json --platform "${SBOM_PLATFORM}" \
"registry:${IMAGE}@${DIGEST}" > sbom.spdx.json
Comment on lines +302 to +304

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


# Syft names the platform image it scanned, never the index ${DIGEST}
# points at, so the subject has to be that platform's entry in it.
expected=$(docker buildx imagetools inspect "${IMAGE}@${DIGEST}" --raw \
| jq -r --arg p "${SBOM_PLATFORM}" '
.manifests[]?
| select((.platform.os + "/" + .platform.architecture) == $p)
| .digest')
Comment on lines +308 to +312

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.

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
Comment on lines +313 to +321

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.

if [ "sha256:${subject}" != "${expected}" ]; then
echo "::error::sbom.spdx.json describes sha256:${subject}, not the ${SBOM_PLATFORM} image ${expected}"
exit 1
fi

- name: Attest SBOM to Github
uses: actions/attest@v4.2.2
Expand Down Expand Up @@ -333,6 +360,7 @@ jobs:
--name container-sbom
--fingerprint ${{ env.FINGERPRINT }}
--sbom-file sbom.spdx.json
--annotate sbom_platform=${{ env.SBOM_PLATFORM }}
--org ${{ inputs.kosli_org }}

- name: Run Snyk Container Test to scan the Docker image for vulnerabilities
Expand Down
Loading