fix: verify immutable release asset reruns - #371
codeforester wants to merge 1 commit into
Conversation
| --notes "Published distributions and release metadata for $tag. See CHANGELOG.md for the reviewed release notes." \ | ||
| || create_status=$? | ||
|
|
||
| if read_release_metadata; then |
There was a problem hiding this comment.
Correctness: success is inferred solely from the post-create read-back matching bytes, so a genuine failure reported by gh release create (non-zero create_status) is silently discarded whenever the release object still reads back and its assets happen to match. If gh release create fails on a late step (e.g. --generate-notes API hiccup) after uploading all assets, the script would print 'Verified immutable release ... after publication.' and exit 0 — the real failure signal from gh never surfaces.
| --resolved-tag-commit "$tag_commit" | ||
| } | ||
|
|
||
| if read_release_metadata; then |
There was a problem hiding this comment.
Correctness: the post-create read-back has no retry/backoff, so GitHub API read-after-write lag can turn a successful release creation into a spurious job failure — e.g. gh release create succeeds but the immediately-following gh api repos/.../releases/tags/$tag hits a stale replica and returns 404 before the write propagates, causing the job to exit 1 even though the release was created correctly.
| if gh api "repos/$GITHUB_REPOSITORY/releases/tags/$tag" >"$release_json" 2>"$release_error"; then | ||
| return 0 | ||
| fi | ||
| if grep -Fq "(HTTP 404)" "$release_error"; then |
There was a problem hiding this comment.
Robustness: read_release_metadata() distinguishes "release not found" from "real API error" only by grepping stderr for the literal substring (HTTP 404), an undocumented, version-fragile detail of the gh CLI's error text. If a future gh version changes its 404 error format, this falls through to a hard error and would permanently block the first release of a new tag until patched.
| HEX_SHA256_RE = re.compile(r"^[0-9a-f]{64}$") | ||
|
|
||
|
|
||
| def _sha256(path: Path) -> str: |
There was a problem hiding this comment.
Reuse: the _sha256 helper and SBOM/BOM/checksum validation logic (lines 96-113) duplicate what already exists in scripts/validate_release_metadata.py (and generate_release_metadata.py), which this same workflow step already invokes on the identical dist/ files. A future rule change made in one script but not the other would let the two silently drift — a release passing pre-creation validation could fail post-creation verification, or vice versa.
| workflow = (Path(__file__).resolve().parents[1] / ".github/workflows/package.yml").read_text(encoding="utf-8") | ||
| verifier = (Path(__file__).resolve().parents[1] / "scripts/verify_release_assets.py").read_text(encoding="utf-8") | ||
|
|
||
| assert "Create GitHub Release" in workflow |
There was a problem hiding this comment.
Test gap: this test still asserts "Create GitHub Release" in workflow, but the diff renamed the actual release step to "Verify and create immutable GitHub Release". It only still passes because of the unrelated job-level name: Create GitHub Release field — if the release-creation step is later removed, this assertion won't catch it.
1 participant
Summary
Fixes #333.
Validation
./tests/full_validate.shpassed: 533 tests, 193 subtests; 87.22% coverage, Ruff, mypy, docs, schemas, security, and benchmark gates.bash -non the release job.