fix(release): repair protocol asset upload#5535
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Idempotent repair is the right shape — skip complete releases, repair incomplete ones, fail-closed when the committed tarball is genuinely absent. The signing fallback is a faithful copy of scripts/sign-protocol-tarball.sh, base64→PEM .crt normalization (adcp#2900) included.
Things I checked
- Unguarded step is safe on non-release pushes. Dropping the
published == 'true'gate means the step now runs on every push to main/3.0.x/2.6.x, but the earlygh release viewexistence check exits 0 cleanly when no release matches the current package.json version. Happy path for ordinary pushes is unchanged. - bash correctness under set -euo pipefail. ubuntu-latest is bash 5.x: empty array expansion under nounset is safe (>=4.4).
grep -Fxqanchors whole-line, so3.0.16.tgzcannot false-match3.0.16.tgz.sha256. The missing-assets / published interaction is correct: publish then upload all; not-published plus nothing-missing then skip; not-published plus missing then repair. - Files exist after checkout. The protocol tarballs and their .sha256/.sig/.crt sidecars are committed to the repo tree (v3.0.0–3.0.15 all carry full sidecars), so the repair path uploads tree bytes; the
cosign sign-blobbranch only fires when sidecars are absent. Theexit 1on a missing tarball is fail-closed. - Signing identity stays bound. cosign is installed at the job level before this step;
id-token: writeis job-scoped (release.yml:22). The keyless cert still namesrelease.yml. security-reviewer: no High findings, no external forge/overwrite path — every new surface requires repo write access. - Publish-path --clobber is not new. Before this PR the step already ran
gh release upload ... --clobberon every publish run; the PR adds the repair path on top. The clobber-on-publish behavior is unchanged.
Follow-ups (non-blocking — file as issues)
- Add a changeset. No
.changeset/*.mdin the diff. CLAUDE.md mandates one on every PR —npx changeset --empty, renamed to e.g.repair-protocol-asset-upload.md. Non-wire change, so empty/no-package category. Not a block (release infra, not schema/reference/build-script), but it is repo convention and trivial. - mapfile from process substitution swallows a failed lookup. A transient
gh release viewAPI failure inside the process-substitution input does not propagate under set -e/pipefail; the empty array reads as "release has zero assets" and the step proceeds to sign + clobber. Capture into a variable first via a plain command substitution so a nonzero exit aborts, then run mapfile on that. (code-reviewer Major.) - Upload only the missing assets; do not --clobber all four. You already compute the missing set; uploading just those (and never clobbering an existing tarball) avoids re-signing/replacing an already-published, already-signed asset with a differently-bound Sigstore cert. Sigstore signatures are not deterministic, so a re-sign churns provenance for identical bytes. (security-reviewer Finding 1, Medium.)
- Repaired sidecars are upload-only. The inline fallback uploads .sig/.crt but, unlike sign-protocol-tarball.sh:76-78, never git-adds them back, so the repo tree at that tag can diverge from the release assets. Log loudly when the fallback fires so repair runs stay auditable.
LGTM. Follow-ups noted below — none block, but get the changeset in before you push.
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.
Summary\n- make protocol release asset upload idempotent\n- sign missing protocol tarball sidecars in GitHub Actions before upload\n- repair existing releases that have a tag/release but missing assets\n\n## Validation\n- git diff --check\n- ruby -e "require 'yaml'; YAML.load_file('.github/workflows/release.yml'); puts 'release.yml parses'"\n- precommit: npm run test:unit, npm run test:test-dynamic-imports, npm run typecheck