fix(provenance): fail closed + explicit attestation; wire verify-image as a gate (SP-GATE-002) - #312
Merged
Merged
Conversation
…y-image as a gate (SP-GATE-002)
The signer was fail-open ('never fails — always exit 0'): an unsigned artifact shipped
provenance a consumer could mistake for attested, and the correct verify-image.sh was
referenced by ZERO workflows.
sign-and-provenance.sh:
- ALWAYS records explicit attestation state in the OSImage provenance
(provenance.attestation.{signed,reason}) so 'unsigned' is loud, not inferred from a
missing signatureRef (feeds the downstream epistemicLevel, SP-GATE-003).
- Fail closed: a signing key present but no artifact signed -> exit 4; and
SOURCEOS_REQUIRE_SIGNATURE=1 + unsigned -> exit 4. No key + not required stays exit 0
(graceful dev), but with attestation.signed=false explicit. Teeth-verified all 3 modes.
build-custom.yml:
- The orphaned verify-image.sh is now a GATE: when a signing key is configured
(vars.SOURCEOS_SIGN_PUBKEY set), every produced image MUST verify against the public key
+ in-toto subject digest or the build fails. Unsigned dev builds skip it.
There was a problem hiding this comment.
🟡 Not ready to approve
The new verification gate has a concrete artifact-discovery bug and, more importantly, it can still allow unsigned artifacts to be uploaded before verification fails, weakening the intended fail-closed behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR closes the SP-GATE-002 provenance “fail-open” gap by making attestation state explicit in emitted OSImage provenance and by introducing a signature/attestation verification gate in the build-custom GitHub Actions workflow when a signing public key is configured.
Changes:
- Update
scripts/sign-and-provenance.shto always include explicitprovenance.attestation.{signed,reason}in OSImage provenance and to exit non-zero in fail-closed scenarios. - Add a verification step to
.github/workflows/build-custom.ymlthat runsscripts/verify-image.shas a gating check whenvars.SOURCEOS_SIGN_PUBKEYis set.
File summaries
| File | Description |
|---|---|
| scripts/sign-and-provenance.sh | Adds explicit attestation state into OSImage provenance and introduces a fail-closed exit path for unsigned outputs in key/required-signature scenarios. |
| .github/workflows/build-custom.yml | Wires verify-image.sh into the build-custom workflow as a gate when a signing public key is configured. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+237
to
+244
| # ── Fail-closed gate (SP-GATE-002) ───────────────────────────────────────────── | ||
| # The old behaviour ("never fails — always exit 0") let an unsigned artifact ship | ||
| # provenance that a downstream consumer could mistake for attested. Refuse that: | ||
| if [[ "$SIGNED" -ne 1 ]]; then | ||
| if [[ -n "${SOURCEOS_SIGN_SECRET_KEY:-}" ]]; then | ||
| log "FAIL-CLOSED: a signing key was provided but no artifact was signed (minisign failed)" | ||
| exit 4 | ||
| fi |
Comment on lines
+87
to
+90
| shopt -s nullglob | ||
| imgs=(out/*.iso out/kernel out/initrd) | ||
| [ ${#imgs[@]} -gt 0 ] || { echo "SP-GATE-002: a signing key is configured but no image artifacts to verify in out/" >&2; exit 1; } | ||
| for f in "${imgs[@]}"; do |
Comment on lines
+82
to
+94
| - name: Verify image signature + attestation | ||
| if: ${{ vars.SOURCEOS_SIGN_PUBKEY != '' }} | ||
| env: | ||
| SOURCEOS_SIGN_PUBKEY: ${{ vars.SOURCEOS_SIGN_PUBKEY }} | ||
| run: | | ||
| shopt -s nullglob | ||
| imgs=(out/*.iso out/kernel out/initrd) | ||
| [ ${#imgs[@]} -gt 0 ] || { echo "SP-GATE-002: a signing key is configured but no image artifacts to verify in out/" >&2; exit 1; } | ||
| for f in "${imgs[@]}"; do | ||
| [ -f "$f" ] || continue | ||
| echo "SP-GATE-002: verifying $(basename "$f")" | ||
| bash scripts/verify-image.sh "$f" "$SOURCEOS_SIGN_PUBKEY" | ||
| done |
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.
The hole (SP-GATE-002, from the SourceOS gate-gap audit)
scripts/sign-and-provenance.shwas fail-open — its header literally said "never fails the build — it always exits 0" — so an unsigned artifact shipped provenance a consumer could mistake for attested (it only omittedsignatureRef). And the correct verifierscripts/verify-image.shwas referenced by 0 of the workflows. Same bug class asray_runnersynthetic metrics.Fix
sign-and-provenance.sh— explicit + fail-closed:provenance.attestation.{signed,reason}, so 'unsigned' is loud, not inferred from a missing field (feeds the downstreamepistemicLevel, SP-GATE-003).SOURCEOS_REQUIRE_SIGNATURE=1+ unsigned → exit 4. No key + not required stays exit 0 (graceful dev) but withattestation.signed=falseexplicit. Teeth-verified all three modes locally.build-custom.yml— the orphaned verifier is now a GATE:vars.SOURCEOS_SIGN_PUBKEYset), every produced image must verify against the public key + in-toto subject digest viaverify-image.sh, or the build fails. Unsigned dev builds (no pubkey) skip it.Verification
YAML valid;
bash -nclean on both scripts; the three fail-closed modes tested (no-key→exit0+marker, REQUIRE_SIGNATURE→exit4, key-present-but-unsigned→exit4).Note: the signing actually happens in build-custom.yml → build-custom-image.sh → sign-and-provenance.sh; release-images/nix-build-images don't sign, so the verify gate is wired at the real signing point. Not auto-merging — for review.