fix(security): confirm a Rekor 409 by verifying it, sign before the rollout, publish release artifacts only after the attestation gate - #390
Open
Cre-eD wants to merge 6 commits into
Conversation
…ot a failed deploy
Redeploying an unchanged image digest regenerates a byte-identical SBOM or
provenance predicate. The transparency log already holds that exact entry, so
it answers 409 createLogEntryConflict, cosign exits non-zero and the deploy
fails on an artifact that is already in the state the deploy wanted:
cosign attest failed: signing bundle: [POST /api/v1/log/entries][409]
createLogEntryConflict {"code":409,"message":"an equivalent entry already
exists in the transparency log with UUID ..."}
Error: failed to attach SBOM: cosign attest failed: exit status 1
Retrying does not help here. Under key-based signing the body is deterministic,
so every attempt reproduces the same conflict and the loop only delays the
failure.
The existing code deliberately refused to read a 409 as success, on the sound
grounds that cosign uploads to Rekor before it pushes to the registry, so a
tlog entry is not proof the artifact was attached. That reasoning is kept and
the gap is closed instead of ignored: on a conflict the retry loop now asks the
registry directly, with `cosign download signature` or `cosign download
attestation --predicate-type <type>`. Both are read-only, need no verification
key, and work for keyless and key-based signing alike. If the artifact is
there, the run is already done and returns success without burning the rest of
the retry budget. If it is not, the previous retry-then-report behaviour is
unchanged.
The confirmation is fail-closed: only a clean exit AND non-empty output count
as present, so a probe that errors, times out or prints nothing can never turn
a genuine signing failure into a green deploy. The type is carried over from
the attest invocation, otherwise an existing SBOM attestation would stand in
for a missing provenance one.
Also handles the sibling symptom seen when several deploys race on one digest:
signing bundle: Post "https://rekor.sigstore.dev/api/v1/log/entries": ...
giving up after 2 attempt(s)
That is cosign exhausting its own HTTP retries against the log, not a conflict,
and it used to fall straight through to the fail-fast branch and break the
deploy on the first hiccup. It is now a second retryable class, matched only on
a log-entries POST so an unrelated give-up from a registry or from Fulcio still
fails fast. With two retryable classes the bound goes from 3 attempts to 5;
the jittered backoff schedule is unchanged.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
… workload updated BuildAndPushImage models sign, verify, SBOM attestation and provenance attestation as resources hanging off the image push and returns their fan-in on ImageOut.AddOpts. The documented contract is that the workload update depends on them. The ECS path implements that. The Kubernetes path did not: BuildAndPushImages copied AddOpts onto ContainerImage and nothing ever read it again, so DeploySimpleContainer created the Deployment with no dependency edge to any security operation. Pulumi was therefore free to update the workload first. Observed ordering in a failing run: the rollout completed at 06:42:04 and signing failed at 06:42:26, after which the run reported Image Signing SKIPPED Provenance Generation SKIPPED Report Uploads SKIPPED error: update failed The practical result is the worst of both: an unsigned, unattested image with no SBOM is already serving traffic, and the pipeline is red so the next deploy looks like the problem rather than the symptom. Fold the per-image options into the resource options DeploySimpleContainer builds, so signing becomes a precondition of the rollout in the Kubernetes path too. Images SC did not build, and stacks with security disabled, contribute no options and deploy exactly as before. The regression test drives DeploySimpleContainer under Pulumi mocks with a stand-in gate resource on AddOpts and asserts the registered Deployment lists it as a dependency. Reverting the one-line wiring fails it with the gate URN absent from the dependency list. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
The whole attestation chain in push.yaml is continue-on-error and the step that collects the outcomes only emits ::warning title=Attestation incomplete::... so a release can publish a tarball with no SBOM, no signature and no provenance on a fully green workflow. SECURITY.md tells consumers the opposite: "Every release produces signed, attested artifacts". Nothing downstream closes the gap either. verify-attestations runs after publish and its own header says it surfaces the regression "without blocking the release that already shipped". The soft-fail was scoped as a 14-day bake-in. The comments point at a HARDENING.md Phase 2 plan that is not in the repository and never has been, there is no open issue tracking the flip, and the bake-in window closed months ago. Treating it as expired rather than as a live decision. Turn the three collectors in push.yaml into gates that exit 1: the per-platform tarball chain, the per-image chain, and the sidecar count assertion that guards the bundle actually uploaded to the CDN. What the chain does is unchanged. Every sub-step stays soft on purpose, so one failure does not hide the others and the gate can report the whole picture at once instead of aborting at the first missing piece. Break glass: the repository variable ATTESTATION_SOFT_FAIL=true degrades all three back to warnings for the duration of a Sigstore or Rekor outage. It is unset by default, so the enforcing path is the one that runs. branch-preview.yaml and build-staging.yml stay warn-only and gain a comment saying why: neither is a release, no published contract promises their artifacts carry a complete chain, and blocking them would stall iteration on an upstream hiccup. Matrix values in the rewritten gate scripts are read through env instead of being interpolated into the shell. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Cre-eD
requested review from
Laboratory,
smecsia and
universe-ops
as code owners
August 22, 2026 20:55
Semgrep Scan ResultsRepository:
Scanned at 2026-08-23 11:25 UTC |
Security Scan ResultsRepository:
Scanned at 2026-08-23 11:26 UTC |
📊 Statement coverageMeasured on the documented included set (see
Baseline: |
…n download The idempotency fix earlier in this branch never fires on current cosign. cosign v3 defaults to the new bundle format and writes nothing to the legacy sha256-<digest>.sig tag, so `cosign download signature` answers "no signatures associated" and `cosign download attestation --predicate-type ...` answers "found no attestations" for an image that `cosign verify` and `cosign verify-attestation` both accept. Reproduced against a local registry with cosign v3.0.2, same image, same run. The consequence was worse than the bug being fixed. Every conflict ran the retry loop to exhaustion and the sign or attest step failed; with the rollout now gated on the sign/attest fan-in, a redeploy of an unchanged digest stopped rolling out at all instead of rolling out with a red run. Probe with verify instead. That also closes presence versus identity: the old probe accepted any entry under the legacy tag, so a signature made under a rotated key, or an attestation of the same predicate family written by an unrelated workflow, read as proof. verify checks the identity, and the probe's exit code is the whole signal, because cosign prints its banner to stderr and nothing to stdout. A probe is built only when the config can name an identity to verify against. Without one the caller keeps the retry-then-report path rather than accept an artifact it cannot attribute. Config.CreateSigner now plumbs the certificate identity into KeylessSigner so the path is live in production keyless runs. Also here: - Bound the probe at 30s instead of reusing the sign/attest timeout. Worst case per artifact was 5 x (timeout + timeout) plus backoff, roughly 20 minutes for the SBOM attacher against 6 minutes before confirmation existed, and both attachers run per image. - Filter the probe environment to registry-relevant variables. It mints no certificate, so it had no business holding SIGSTORE_ID_TOKEN. - Tighten rekorGiveUpRe. (?s) with a 400-character gap only required the two halves to appear near each other in one stream, so a log-entries POST that returned 201 followed by a registry, Fulcio or OIDC give-up was classified as a transparency-log hiccup, retried five times, and sent the operator to the wrong runbook. All three cases are now regression tests. - Report a confirmed conflict on SignResult.Confirmed and say in the log that no transparency-log entry URL exists, instead of returning a result that claims success with an empty RekorEntry. - Teach the cosign fake to validate the probe argv the way real cosign does, answer `download` the way cosign v3 does, and model probe exit status and stdout separately. Answering on $1 alone is why the breakage above was invisible to the suite. The probe branch now sits below the delay scripting, so probes cost simulated time like every other invocation. - Add an integration test that pushes to a real local registry, signs for real, and drives the confirmation path with genuine cosign, including the negative half over an unsigned image. It records what the legacy download path returns on the installed version. Build-tagged and skipped under -short. - Fix GenerateKeyPair, found by that test: without --output-key-prefix cosign writes the pair into the process working directory rather than outputDir, so every caller passing a directory failed on the chmod that follows, after dropping an encrypted private key into the source tree. Ignore those two filenames as a second line of defence. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
docker/build-push-action pushed :latest and :<version> with push: true, so both were live before Sign + attest and before the attestation gate. A failing gate only failed the job and skipped docker-finalize, which left a mutable :latest pointing at an unsigned, un-SBOM'd image while the dist site still served the previous version. The tarball path already gates before upload, and SECURITY.md already told consumers "the release build fails and nothing is published", which was false for images. Build to a throwaway :ci-<run>-<attempt> tag, because cosign signs by digest and needs the manifest in the registry first, then promote the release tags after the gate and delete the throwaway tag. Promotion re-PUTs the signed manifest byte for byte, which is what `crane tag` does. `docker buildx imagetools create` is not usable here: given a single platform-specific manifest it wraps it in a NEW index, so the tag resolves to an index digest that was never signed and `cosign verify <repo>:latest` fails. Measured against a local registry, a source manifest came back under a different digest. The step compares the registry's Docker-Content-Digest with the signed digest and fails the release if they ever diverge, so a promotion that stops preserving the digest cannot ship silently. ATTESTATION_SOFT_FAIL now carries an expiry. It was an unbounded, unattributed and invisible bypass: vars.* also resolves organization variables, so one set on the org silently disabled the gate in every repository that read it, nothing printed which scope supplied it, and a break-glass release was byte-indistinguishable from a clean one. The value must now be an ISO-8601 UTC date, anything else including the historical 'true' is rejected and does not bypass, every gate prints the value on every run whether it passes or fails, and a bypassed run writes attestation-breakglass=<date> to the job summary. The sidecar assertion pairs each tarball with its own sidecars instead of comparing totals. Totals passed a bundle in which one tarball carried two of a kind and another none, which is exactly what a half-failed matrix leg produces. An empty sidecar now counts as missing. Documentation caught up with the workflow. SECURITY.md describes the promote step and the new break-glass contract. The preview and staging comments no longer claim to mirror push.yaml or to offer the same guarantees as a release, because push.yaml has neither a bake-in nor a warn-only aggregator any more, and verify-attestations no longer describes itself as the gate that a release gets past. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
The Kubernetes path's imageSecurityOpts skips nil entries and the ECS export loop further down guards for them, but the ECS options loop did not, so the two loops disagreed about whether ref.Images can hold a nil and one of them would panic if it ever does. Also drop the suffix arm from containsURN in the Kubernetes gate test. want is always a full URN, so any URN merely ending in the wanted one satisfied the assertion and it was weaker than it reads. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four related supply-chain fixes. The first two break deploys today; the third
is why nobody notices when they do; the fourth is that "nothing is published"
was not true for images.
A review pass over the first three reproduced two of its findings
empirically. Everything it raised is addressed here; the mechanism in
section 1 changed substantially as a result. See Review round 2 at the end
for the finding-by-finding list.
1. A Rekor 409 on an already-attested image now succeeds
Redeploying an unchanged image digest regenerates a byte-identical SBOM or
provenance predicate. The transparency log already holds that exact entry, so
it answers 409 and the deploy fails on an artifact that is already in the state
the deploy wanted:
Retrying cannot clear it. Under key-based signing the body is deterministic, so
every attempt reproduces the same conflict and the loop only delays the failure.
RunCosignWithRetryalready refused to read a 409 as success, and its reasoningwas right: cosign uploads to Rekor before it pushes to the registry, so a tlog
entry is not proof the artifact was attached. Rather than drop that guarantee,
this closes the gap. On a conflict the loop asks the registry directly, with
cosign verify --key/--certificate-identity-regexpfor a signature andcosign verify-attestation --type <type> ...for an attestation. Artifactpresent and verifying means the run is already done, so it returns success
immediately without burning the rest of the retry budget. Otherwise the
previous retry-then-report behaviour is untouched.
Verify, not download. The first draft of this probe used
cosign download signature/cosign download attestation. Two problems, both raised in reviewand one reproduced against a local registry with cosign v3.0.2:
sha256-<digest>.sigtag. On a signed image,cosign download signaturereturns
no signatures associatedwhilecosign verify --keysucceeds. Thedownload probe is therefore a permanent false negative on current cosign, so
every conflict would run to exhaustion and fail. Combined with fix 2 below,
which makes signing a hard predecessor of the Deployment, the exact case this
PR exists to fix would have degraded from "rolls out, run is red" to "does not
roll out at all".
downloadanswers presence, not identity. Any entry under the legacy tagcounted, so a signature made under a rotated key, or an attestation of the
same predicate family written by an unrelated workflow, read as proof.
security.Signing.Verifyis opt-in, so nothing downstream re-checked.verifyreads both formats and checks the identity, so a confirmation now meansthe artifact is present and ours. The probe's exit code is the whole signal:
cosign prints its verification banner to stderr and nothing to stdout, so the
earlier "clean exit and non-empty stdout" rule was itself a way never to
confirm.
The confirmation stays fail-closed. A probe that errors, times out, or cannot be
built at all leaves the caller on retry-then-report, so it can never convert a
genuine signing failure into a green deploy. It is only built when the config
can name an identity to verify against; without one the caller keeps the plain
retry path rather than accept an artifact it cannot attribute. The predicate
type carries through from the attest invocation, otherwise an existing SBOM
attestation would stand in for a missing provenance one.
The probe is bounded at 30s rather than inheriting the sign/attest budget.
Reusing it made the worst case per artifact 5 x (timeout + timeout) plus
backoff, roughly 20 minutes for the SBOM attacher against 6 minutes before
confirmation existed, and both attachers run per image. It also no longer
inherits
SIGSTORE_ID_TOKEN: it mints no certificate, so its environment isfiltered to registry-relevant variables.
Proving the probe actually fires. A fake cosign cannot certify this: the
fake decides what the probe means, which is exactly why the v3 breakage was
invisible to a green suite. So the suite now has an integration test that starts
a real
registry:2, pushes an image, signs it for real, and drives theconfirmation path with the genuine cosign binary, plus the negative half over an
unsigned image. The synthetic part is only the 409 itself, because reaching a
real one would mean uploading to a public transparency log. On the run used to
develop this, that test logged:
while the verify probe confirmed. The fake was hardened to match: it validates
the probe argv the way real cosign does (identity flag required,
--typerequired for
verify-attestation, image reference must be the one under test),answers
downloadthe way cosign v3 does, and models probe exit status andstdout separately so exit-0-empty, auth failure and false-negative are all
expressible. Its probe branch also moved below the delay scripting, so probes
cost simulated time like every other invocation.
Sibling symptom. Concurrent deploys of one digest also hit
That is cosign exhausting its own HTTP retries against the log, not a conflict.
It used to fall straight through to the fail-fast branch and break the deploy on
the first hiccup. It is now a second retryable class, matched only on a
log-entries POST so an unrelated give-up from a registry or from Fulcio still
fails fast.
The first draft matched the two halves with
(?s)and a 400-character gap,which only required them to appear near each other in the same stream. Review
confirmed all three of these matched: a log-entries POST returning
201 Createdfollowed by a registry
unauthorized, giving up after 3 attempt(s), the samefollowed by a Fulcio
signingCertgive-up, and the same followed by an OIDCtoken give-up. None of them falsely succeeded, but a permanent registry-auth
failure was retried five times and the operator was pointed at the
transparency-log runbook. The gap now excludes quotes and newlines, so the match
stays inside one message; every competing give-up names its own quoted URL
first. All three are regression tests.
Retry budget. With two retryable classes the bound goes from 3 attempts to
5. The jittered full-backoff schedule is unchanged (1s base doubling to an 8s
cap), so the worst case grows from about 3s to about 15s of waiting. That only
applies to the genuinely-contended path now: a conflict on an artifact that is
already attached short-circuits on the first attempt and never sleeps at all.
2. Signing no longer runs after the workload has rolled out
BuildAndPushImagemodels sign, verify, SBOM attestation and provenanceattestation as resources hanging off the image push and returns their fan-in on
ImageOut.AddOpts. The documented contract is that the workload update dependson them. The ECS path implements it (
aws/ecs_fargate.gofoldsimg.AddOptsinto its options). The Kubernetes path did not:
BuildAndPushImagescopiedAddOptsontoContainerImageand nothing read it again, soDeploySimpleContainercreated the Deployment with no dependency edge to anysecurity operation.
Pulumi was therefore free to update the workload first. Observed ordering in a
failing run: rollout completed at 06:42:04, signing failed at 06:42:26, then
Worst of both outcomes: an unsigned, unattested image with no SBOM is already
serving traffic, and CI is red so the next deploy looks like the problem rather
than the symptom.
This is the full fix, not a guard. Folding the per-image options into the
options
DeploySimpleContainerbuilds makes signing a precondition of therollout, the same way the ECS path already worked. Images SC did not build, and
stacks with security disabled, contribute no options and deploy exactly as
before. Nothing about the security resources themselves changed.
The regression test drives
DeploySimpleContainerunder Pulumi mocks with astand-in gate resource on
AddOptsand asserts the registered Deployment listsit as a dependency. Reverting the one-line wiring fails it with the gate URN
absent from the dependency list.
3. The attestation chain now blocks a release
In
push.yamlevery step of the chain iscontinue-on-error: trueand thecollector only emits
::warning title=Attestation incomplete::.... A releasecan therefore publish a tarball with no SBOM, no signature and no provenance on
a fully green workflow.
docs/SECURITY.mdtells consumers the opposite: "Everyrelease produces signed, attested artifacts". Nothing downstream closes the gap
either, since
verify-attestationsruns after publish and its own header saysit surfaces the regression "without blocking the release that already shipped".
Which path I took, and why. The soft-fail is scoped in the comments as a
14-day bake-in tracked in a
HARDENING.mdPhase 2 plan. I looked for evidencethat this is still a live decision and found the opposite:
HARDENING.mddoes not exist in the repository and never has(
git log --diff-filter=A -- '**HARDENING.md'is empty)closed roughly three months ago
So this enforces rather than hiding behind a flag defaulting to today's
behaviour. The three collectors in
push.yamlnowexit 1: the per-platformtarball chain, the per-image chain, and the sidecar count assertion guarding the
bundle actually uploaded to the CDN (it runs before publish, so a gap there
blocks the upload).
What the chain does is unchanged. Every sub-step deliberately stays soft, so one
failure does not mask the others and the gate reports the whole picture at once
instead of aborting at the first missing piece.
Break glass, with an expiry.
ATTESTATION_SOFT_FAILdegrades all three backto warnings for the duration of a Sigstore or Rekor outage. It is unset by
default, so the enforcing path is what runs.
Review pointed out that the first draft was an unbounded, unattributed and
invisible bypass:
vars.*also resolves ORGANIZATION variables, so one set onthe org silently disabled the gate in every repository that reads it, nothing
printed which scope supplied it, there was no expiry or recorded reason, and a
break-glass release was byte-indistinguishable from a clean one at the consumer
end. So the value is now an ISO-8601 UTC expiry date (
YYYY-MM-DD) and:true, is rejected withan
::errorand does not bypassup in green runs
attestation-breakglass=<date>to the job summaryBreak-glass expiredwarning before blockingdocs/SECURITY.mddocuments the format, the organization-scope caveat and whomay set it.
Sidecar pairing. The bundle assertion compared totals per sidecar kind
against the tarball count, so it passed a bundle in which one tarball carried
two of a kind and another none, which is exactly the shape a half-failed matrix
leg produces. It now pairs each tarball with its own four sidecars, and an empty
sidecar counts as missing.
branch-preview.yamlandbuild-staging.ymlstay warn-only and gain a commentsaying why: neither is a release, no published contract promises their artifacts
carry a complete chain, and blocking them would stall iteration on an upstream
hiccup.
Matrix values in the rewritten gate scripts are read through
envrather thaninterpolated into the shell.
4. Release image tags are published only after the gate
Fix 3 made the gate block, but for images it blocked too late. The build step
ran
docker/build-push-actionwithpush: trueandtags:containing both<img>:latestand:<version>, so the release tags were live beforeSign + attestand before the gate. A failing gate only failed the job andskipped
docker-finalize, which left a mutable:latestpointing at anunsigned, un-SBOM'd image while
dist.simple-container.comstill served theprevious version. The tarball path already gates before upload, so this was an
oversight rather than a design choice, and
docs/SECURITY.mdwas left assertingin writing something that was false for images.
The image now builds to a throwaway
:ci-<run>-<attempt>tag, because cosignsigns by digest and needs the manifest in the registry before it can sign
anything. The release tags are applied by a promote step after the gate, and the
throwaway tag is deleted afterwards, including when the gate blocked, since that
is precisely when an unsigned image would otherwise stay reachable under a
predictable name.
Promotion has to preserve the digest, and the obvious tool does not. The
natural shape is
docker buildx imagetools create -t <tag> <repo>@<digest>.Measured against a local registry, given a single platform-specific manifest it
wraps it in a new index: source
sha256:dd54c6...came back assha256:45b9e3.... The tag would then resolve to a digest that was neversigned, and
cosign verify <repo>:latestwould fail for every consumerfollowing the documented verification path. Promotion instead re-PUTs the signed
manifest byte for byte under each release tag, which is what
crane tagdoesand what the registry v2 API is for, needing nothing on the runner beyond curl
and jq. The step then compares the registry's
Docker-Content-Digestagainstthe signed digest and fails the release if they ever diverge, so a promotion
that stops preserving the digest cannot ship silently.
With this, "the release build fails and nothing is published" is literally true
on both paths, and
docs/SECURITY.mdsays how.Validation
go build ./...clean,go vet ./...cleango test ./...: 48 packages, all pass, 0 failures. Thepkg/api/gitenvironment failure noted in the first round no longer reproduces (this run
used a worktree outside
/tmp).pkg/security/signingwas run for real against alocal
registry:2with cosign v3.0.2: both halves pass, and it is the sourceof the download-versus-verify evidence quoted in section 1. Build-tagged
integrationand skipped under-short, so it does not run in normal CI.actionlintreports the same finding setbefore and after (6
runner-labelfor the self-hosted runners, 1 pre-existingstack-nameexpression warning)against a local registry / shell before being written into the workflow: the
promotion preserves the digest across two tags, and
true, a malformed value,an expired date, today's date and a future date all behave as documented
gofmt -lclean on every file in the diffnothing written to a transparency log
Review round 2
Every finding from that review, and what happened to it.
rekorGiveUpReover-matcheswant: falseATTESTATION_SOFT_FAILunbounded and invisibleProbeExit/ProbeStdout, probe below the delay, plus the real-registry integration testRekorEntrywhile reporting successSignResult.Confirmedplus an explicit log lineSIGSTORE_ID_TOKENcontainsURNaccepts a suffixecs_fargate.gomissing the nil guardimageSecurityOptshasSECURITY.md"nothing is published", preview/staging "mirrors push.yaml", preview same-guarantees contradiction,verify-attestationsheaderTwo things worth flagging to a human:
GenerateKeyPairnever honoured itsoutputDir. Without--output-key-prefix, cosign writes the pair into theprocess working directory, so every caller passing a directory failed on the
chmod that follows, after dropping an encrypted private key into the source
tree. Fixed, and both filenames are now git-ignored as a second line of
defence.
pkg/security/signing/e2e_test.gohas been calling it that way.private-organisation references on
mainof this public repository,including a real cloud account id in a test fixture. They are untouched here
and need their own change plus an owner decision.
Note on #383
#383 is red only because its base predates the
godirective move to 1.26.6. Arebase clears it. Not touched here.