feat(release-please): implement PAI 1+6 jobs DAG with binary integrity and tag signature verification#501
feat(release-please): implement PAI 1+6 jobs DAG with binary integrity and tag signature verification#501WilliamBerryiii wants to merge 1 commit into
Conversation
…y and tag signature verification - add check-binary-integrity.yml and verify-tag-signature.yml workflows - restructure release-please.yml with PAI 1+6 jobs DAG (release-please, generate-dependency-sbom, attest-release, sbom-diff, append-verification-notes, publish-release, close-milestone) - adopt actions/create-github-app-token@v3.1.1 with vars.RELEASE_APP_CLIENT_ID and secrets.RELEASE_APP_PRIVATE_KEY - rename .release-please-manifest.json to release-please-manifest.json (no leading dot) - update release-please-config.json, .syft.yaml, pr-validation.yml; remove legacy create-release.yml.disabled
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #501 +/- ##
=======================================
Coverage 32.41% 32.41%
=======================================
Files 40 40
Lines 5902 5902
=======================================
Hits 1913 1913
Misses 3989 3989
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
katriendg
left a comment
katriendg
left a comment
There was a problem hiding this comment.
Review Summary
CI/CD-only change implementing a solid 1+6 DAG for release-please with SLSA provenance, SPDX SBOM, sigstore signing, and a clean aggregator gate. The overall architecture and supply-chain posture improvements are well-designed. There are two security issues that must be resolved before merge, one functional bug that makes the binary integrity check ineffective, and one high-severity reliability gap across six run: blocks.
🔒 Must Fix Before Merge
| # | File | Issue |
|---|---|---|
| 1 | release-please.yml:37, verify-tag-signature.yml:35 |
chainguard-dev/actions/setup-gitsign@v0.13.0 — mutable tag breaks SHA-pin policy |
| 2 | verify-tag-signature.yml:44-47, release-please.yml:191 |
--certificate-identity-regexp '.*' — wildcard accepts any Sigstore identity |
| 3 | check-binary-integrity.yml:54-58 |
gh attestation verify "${bundle}" --bundle "${bundle}" — attestation verified against itself, not against any binary |
| 4 | release-please.yml (6 run: blocks) |
Missing set -euo pipefail — silent failures can cascade through the release pipeline |
💡 Should Fix (Conventions / Quality)
| # | File | Issue |
|---|---|---|
| 5 | release-please.yml:149,156 |
[ ] and $var in sbom-diff — use [[ ]] and "${var}" |
| 6 | pr-validation.yml:373 |
PD-07 Option D plan-phase marker in comment — remove per comment policy |
| 7 | release-please.yml:27-28 |
upload_url and body outputs unused by any downstream job |
| 8 | release-please.yml:58 |
fetch-depth: 0 not needed for SBOM generation — use fetch-depth: 1 |
| 9 | release-please.yml:130-131 |
sbom-diff, append-verification-notes, publish-release missing explicit if: guards |
⚠️ Post-Merge Operational Note
After merge, the branch protection ruleset on main must be updated to require only pr-validation-gate instead of the previous set of individual job names. This is noted in the PR but has no automated enforcement — ensure it is tracked as a follow-up action item.
Inline comments with suggested fixes are attached to each finding above.
| for bundle in "${attestations[@]}"; do | ||
| echo "::group::Verifying ${bundle}" | ||
| gh attestation verify "${bundle}" \ | ||
| --repo "${GITHUB_REPOSITORY}" \ | ||
| --bundle "${bundle}" |
There was a problem hiding this comment.
gh attestation verify is verifying the attestation against itself, not against any release binary
gh attestation verify <subject> expects the subject artifact (the binary) as the positional argument; --bundle supplies the accompanying attestation for that subject. Passing the same .intoto.jsonl file as both the positional subject and --bundle verifies the attestation's internal consistency only — the actual release binaries are never touched. The step name "Verify provenance attestations" is misleading; no artifact provenance is checked.
Correct usage pairs each binary with its attestation bundle:
- name: Verify provenance attestations
run: |
set -euo pipefail
shopt -s nullglob
assets=(release-assets/*)
if [[ ${#assets[@]} -eq 0 ]]; then
echo "No release assets downloaded." >&2
exit 1
fi
verified=0
for asset in "${assets[@]}"; do
[[ "${asset}" == *.intoto.jsonl ]] && continue
bundle="${asset}.intoto.jsonl"
if [[ ! -f "${bundle}" ]]; then
echo "No attestation bundle found for ${asset}." >&2
exit 1
fi
echo "::group::Verifying ${asset}"
gh attestation verify "${asset}" \
--repo "${GITHUB_REPOSITORY}" \
--bundle "${bundle}"
echo "::endgroup::"
(( verified++ ))
done
if [[ ${verified} -eq 0 ]]; then
echo "No non-attestation assets found to verify." >&2
exit 1
fi| gitsign verify-tag \ | ||
| --certificate-identity-regexp '.*' \ | ||
| --certificate-oidc-issuer 'https://token.actions.githubusercontent.com' \ | ||
| "${TAG_NAME}" |
There was a problem hiding this comment.
🔒 Critical — --certificate-identity-regexp '.*' accepts signatures from any Sigstore identity
The wildcard regexp matches every possible Sigstore certificate subject, including personal or attacker-controlled identities. An actor who can push a tag can sign it with their own Sigstore certificate and gitsign verify-tag will pass without error. This verification step provides no meaningful supply-chain guarantee as written.
Scope the identity to the exact workflow URL that performs the signing:
- name: Verify gitsign tag signature
run: |
set -euo pipefail
if ! git rev-parse "refs/tags/${TAG_NAME}" >/dev/null 2>&1; then
echo "Tag ${TAG_NAME} not found in repository." >&2
exit 1
fi
gitsign verify-tag \
--certificate-identity 'https://github.com/microsoft/edge-ai/.github/workflows/release-please.yml@refs/heads/main' \
--certificate-oidc-issuer 'https://token.actions.githubusercontent.com' \
"${TAG_NAME}"Note: The same permissive regexp is also embedded verbatim in the append-verification-notes job in release-please.yml (line 191), instructing downstream consumers to perform the same ineffective check. Both occurrences must be updated together.
| client-id: ${{ vars.RELEASE_APP_CLIENT_ID }} | ||
| private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} | ||
|
|
||
| - uses: chainguard-dev/actions/setup-gitsign@v0.13.0 |
There was a problem hiding this comment.
🔒 Critical — Mutable tag pin violates SHA-pin policy
chainguard-dev/actions/setup-gitsign@v0.13.0 is pinned to a mutable tag. Every other third-party action in this PR is SHA-pinned with a # vX.Y.Z annotation, which is the repository policy. Tags can be force-pushed at any time, enabling a supply-chain attack against the release pipeline itself.
The PR description states this is per upstream guidance but provides no citation. Regardless, the policy applies uniformly — resolve the commit SHA that v0.13.0 currently points to and pin both occurrences (also line 35 in verify-tag-signature.yml):
- uses: chainguard-dev/actions/setup-gitsign@<commit-sha> # v0.13.0| path: test-results/ | ||
| retention-days: 30 | ||
|
|
||
| # Aggregator gate - single required check for branch protection (PD-07 Option D) |
There was a problem hiding this comment.
💡 Medium — Plan-phase marker PD-07 Option D violates the comment policy
The repository comment policy prohibits plan-phase markers, task references, and tracking codes in code files. Remove the reference and keep only the functional description:
# Aggregator gate — single required check for branch protection| run: | | ||
| gh release upload "${{ needs.release-please.outputs.tag_name }}" \ |
There was a problem hiding this comment.
set -euo pipefail missing across six run: blocks
check-binary-integrity.yml (introduced in this same PR) correctly opens every run: block with set -euo pipefail and should be the model. Without it, a failing command in any of these steps is silently ignored and subsequent steps execute on corrupted or missing state — for example, the release could be published even when provenance attachment failed.
Affected steps in this file:
| Job | Step | Line |
|---|---|---|
generate-dependency-sbom |
Attach SBOM to release | 88 |
attest-release |
Stage release source tarball | 109 |
attest-release |
Attach provenance to release | 126 |
append-verification-notes |
Compose and apply verification appendix | 185 |
publish-release |
Flip release draft to published | 216 (single-line — convert to block) |
close-milestone |
Close milestone matching tag | 229 |
Add set -euo pipefail as the first line of each multi-line run: block. For the single-line publish-release step, convert to a multi-line block:
run: |
set -euo pipefail
gh release edit "${TAG}" --draft=falseThe two Extract DSSE envelope steps (single-line jq) should also be converted to blocks or suffixed with || exit 1.
| if [ -n "$prev" ]; then | ||
| gh release download "$prev" --pattern 'dep-sbom.spdx.json' \ | ||
| --dir previous-sbom || true | ||
| fi | ||
|
|
||
| - name: Diff SBOMs | ||
| run: | | ||
| if [ -f previous-sbom/dep-sbom.spdx.json ]; then |
There was a problem hiding this comment.
💡 Medium — POSIX [ ] tests and unbraced $var expansion violate shell conventions
Per the repository shell conventions: use [[ ... ]] instead of [ ... ] or test, and prefer "${var}" over "$var" for variable expansion. Both violations appear in this step (lines 149 and 156):
# Current (violations)
if [ -n "$prev" ]; then
gh release download "$prev" --pattern 'dep-sbom.spdx.json' \
--dir previous-sbom || true
fi
# Corrected
if [[ -n "${prev}" ]]; then
gh release download "${prev}" --pattern 'dep-sbom.spdx.json' \
--dir previous-sbom || true
fi# Current
if [ -f previous-sbom/dep-sbom.spdx.json ]; then
# Corrected
if [[ -f previous-sbom/dep-sbom.spdx.json ]]; then| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| ref: ${{ needs.release-please.outputs.tag_name }} | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
💡 Low — fetch-depth: 0 fetches full repository history unnecessarily
SBOM generation (anchore/sbom-action) analyses the working tree, not git history. Full clone depth is not needed here. check-binary-integrity.yml (also in this PR) uses fetch-depth: 1 for the same operation. Change to fetch-depth: 1 for consistency and faster checkout:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ needs.release-please.outputs.tag_name }}
fetch-depth: 1| tag_name: ${{ steps.release.outputs.tag_name }} | ||
| upload_url: ${{ steps.release.outputs.upload_url }} | ||
| body: ${{ steps.release.outputs.body }} |
There was a problem hiding this comment.
💡 Low — Dead outputs upload_url and body are declared but never consumed
Both outputs are captured from steps.release.outputs but no downstream job references them. Dead outputs increase maintenance overhead and create confusion about the job's contract.
Remove them, or add an inline comment explaining why they are preserved (e.g., reserved for a planned future step):
outputs:
release_created: ${{ steps.release.outputs.release_created }}
tag_name: ${{ steps.release.outputs.tag_name }}
# upload_url and body removed — not consumed by any downstream job| sbom-diff: | ||
| needs: [release-please, generate-dependency-sbom] |
There was a problem hiding this comment.
💡 Low — Three downstream jobs rely on implicit cascading-skip behaviour instead of explicit if: guards
sbom-diff, append-verification-notes, and publish-release have no if: needs.release-please.outputs.release_created == 'true' condition. They rely on GitHub Actions' implicit rule that a skipped dependency propagates skips to dependents. This works today but is fragile if the DAG is restructured, and obscures intent at a glance. attest-release and close-milestone — both in this workflow — already use explicit guards. Apply the same pattern for consistency:
sbom-diff:
needs: [release-please, generate-dependency-sbom]
if: needs.release-please.outputs.release_created == 'true'
...
append-verification-notes:
needs: [release-please, attest-release, sbom-diff]
if: needs.release-please.outputs.release_created == 'true'
...
publish-release:
needs: [release-please, attest-release, sbom-diff, append-verification-notes]
if: needs.release-please.outputs.release_created == 'true'
...
|
|
||
| - name: Recompute and compare digests | ||
| run: | | ||
| set -euo pipefail | ||
| shopt -s nullglob | ||
| assets=(release-assets/*) | ||
| if [[ ${#assets[@]} -eq 0 ]]; then | ||
| echo "No release assets downloaded." >&2 | ||
| exit 1 | ||
| fi | ||
| { | ||
| echo "## Release asset digests (${TAG_NAME})" | ||
| echo | ||
| echo '| Asset | SHA256 |' |
There was a problem hiding this comment.
Medium: "Recompute and compare digests" step performs no actual comparison
This step computes SHA256 digests of downloaded release assets and writes them to the step summary, but never compares them against any reference value. No baseline (from the provenance attestation, a published hash file, or a pinned digest record) is fetched and diffed. The step passes unconditionally regardless of asset content, so a substituted release asset would not be detected here.
This is separate from the attestation verification bug in the "Verify provenance attestations" step above (where gh attestation verify receives the same file as both subject and bundle). That issue is about the attestation check. This issue is about the digest step that follows it, which provides no automated assertion despite its name.
Suggested fix: rename the step to reflect its informational nature ("Log release asset digests"), or add a comparison against digest values extracted from the provenance attestation before logging.
4 participants
Description
Reworked the GitHub release pipeline to a 1+6 jobs DAG anchored on
googleapis/release-please-action@v5.0.0with full SLSA build provenance, SBOM generation and diff, sigstore tag signing, and post-publish verification. The previouscreate-release.ymlworkflow has been retired in favor ofrelease-please.yml, and a new aggregator gate plus two reusable verification workflows have been added so every PR exercises the same integrity checks the release pipeline relies on.The change is CI/CD-only — no Terraform, Bicep, or application source is modified. All third-party actions are pinned to commit SHAs (with
# v…comment annotations) per repository policy, and the release-please configuration was updated to use the conventional manifest filename.Related Issue
Implements release-please parity with the previously disabled
create-release.ymlworkflow. Tracking: internal release-please-parity plan; no GitHub issue link required because this is process plumbing.Type of Change
Implementation Details
release-please.yml(modified, +208/−) — Replaces a single-job workflow with a 1+6 jobs DAG:release-please— runsgoogleapis/release-please-action@v5.0.0with a GitHub App token minted byactions/create-github-app-token@v3.1.1(vars.RELEASE_APP_CLIENT_ID+secrets.RELEASE_APP_PRIVATE_KEY). Outputsrelease_created,tag_name,upload_url, andprs_createdfor downstream gating.generate-dependency-sbom(parallel) — runsanchore/sbom-action@v0.24.0to produce a SPDX-JSON SBOM for the release commit, then attests it withactions/attest@v4.1.0.attest-release(parallel) — generates SLSA build provenance for the release artifacts viaactions/attest-build-provenance@v4.1.0.sbom-diff— diffs the new SBOM against the prior release's SBOM and posts the result to the release notes.append-verification-notes— appends signing/attestation pointers to the release notes so downstream consumers can verify offline.publish-release— finalizes the GitHub Release (drops draft, opens to consumers).close-milestone— closes the matching milestone if one exists.Tag signing uses
chainguard-dev/actions/setup-gitsign@v0.13.0(kept in tag form per the action's published guidance).pr-validation.yml(modified, +43/−) — Added a newpr-validation-gateaggregator job (PD-07 Option D) with 26needs:entries that resolves to a single required check on the branch protection ruleset, eliminating per-job required-check sprawl.check-binary-integrity.yml(new, +82) — Reusable workflow that verifies SHA-256 hashes of release binaries against the published attestation; consumed by both PR validation and release verification.verify-tag-signature.yml(new, +54) — Reusable workflow that verifies the sigstore signature on the release tag usinggitsign verify.release-please-config.json(modified, ±20) — Adjusted release-as / extra-files configuration; behavior-equivalent to prior config aside from the manifest rename..release-please-manifest.json→release-please-manifest.json— Renamed (100% rename, byte-identical content) to use the conventional manifest filename without a leading dot. Pinned to action version2.8.0..syft.yaml(modified, +5) — Tightened SBOM cataloger selection so the SPDX output is deterministic across runs.create-release.yml.disabled(deleted, −131) — Retires the legacy workflow that has been disabled but checked in.Testing Performed
Validation Steps
All workflow and config files in this change passed local validation before commit:
actionlintagainst all 4 modified/added GitHub Actions workflows (release-please.yml,pr-validation.yml,check-binary-integrity.yml,verify-tag-signature.yml) — PASS..syft.yaml— PASS.release-please-config.jsonandrelease-please-manifest.json— PASS.# vX.Y.Zcomment annotation (exception:chainguard-dev/actions/setup-gitsign@v0.13.0which is intentionally kept in tag form per upstream guidance)..release-please-manifest.json→release-please-manifest.jsonis a 100% rename (byte-identical content) confirmed viagit diff -M100% --stat.End-to-end behavior of the release pipeline cannot be tested in a PR (it activates only on merge to
mainwhen release-please opens or merges its release PR); the design has been reviewed against the upstreamgoogleapis/release-please-actionv5 documentation and the existingpr-validation.ymlaggregator pattern in this repo.Checklist
Security Review
The release pipeline tightens the supply-chain posture rather than expanding it: a short-lived GitHub App token replaces the long-lived
GITHUB_TOKEN-only model, every third-party action is SHA-pinned, releases now carry SLSA build provenance and a verifiable SBOM, and tags are sigstore-signed with offline-verifiable signatures.Additional Notes
feat/release-please-parity, head commitdaf7f774, parent39d385a0(microsoft/edge-aimain).githubremote only (microsoft/edge-ai). Theorigin(Azure DevOps) remote was intentionally not updated.pr-validation-gateaggregator (PD-07 Option D) supersedes per-job required-check entries on the branch protection rule. After merge, the branch protection ruleset onmainshould be updated to require onlypr-validation-gateinstead of the individual job names.Screenshots (if applicable)
N/A — CI/CD-only change with no UI surface.