-
Notifications
You must be signed in to change notification settings - Fork 1
fix: verify immutable release asset reruns #371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,12 +14,15 @@ on: | |
| - "scripts/validate_examples.py" | ||
| - "scripts/generate_release_metadata.py" | ||
| - "scripts/validate_release_metadata.py" | ||
| - "scripts/verify_release_assets.py" | ||
| - "scripts/validate_changelog.py" | ||
| - "scripts/validate_release_ref.py" | ||
| - "CHANGELOG.md" | ||
| - "examples/**" | ||
| - "compatibility/**" | ||
| - "docs/releasing.md" | ||
| - "tests/test_package_workflow.py" | ||
| - "tests/test_verify_release_assets.py" | ||
| - "requirements/release.in" | ||
| - "requirements/release.txt" | ||
| - ".github/workflows/package.yml" | ||
|
|
@@ -312,7 +315,16 @@ jobs: | |
| timeout-minutes: 10 | ||
| permissions: | ||
| contents: write | ||
| attestations: read | ||
| steps: | ||
| - name: Check out the tag source | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.13" | ||
|
|
||
| - name: Download reviewed distributions | ||
| uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
|
|
@@ -325,18 +337,91 @@ jobs: | |
| name: base-cli-release-metadata-${{ github.run_id }} | ||
| path: dist | ||
|
|
||
| - name: Create GitHub Release | ||
| - name: Verify and create immutable GitHub Release | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| set -euo pipefail | ||
| tag="$GITHUB_REF_NAME" | ||
| assets=(dist/*.whl dist/*.tar.gz dist/SHA256SUMS dist/SBOM.spdx.json dist/RELEASE-BOM-ROW.json) | ||
| if gh release view "$tag" --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then | ||
| echo "Published release $tag already exists; refusing to replace immutable release assets." >&2 | ||
| tag_commit="$(gh api "repos/$GITHUB_REPOSITORY/commits/$tag" --jq .sha)" | ||
| if [[ "$tag_commit" != "$GITHUB_SHA" ]]; then | ||
| echo "Release tag $tag resolves to $tag_commit, not reviewed commit $GITHUB_SHA." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| python scripts/validate_release_metadata.py dist | ||
| for asset in dist/*.whl dist/*.tar.gz; do | ||
| gh attestation verify "$asset" \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --signer-workflow "github.com/$GITHUB_REPOSITORY/.github/workflows/package.yml" \ | ||
| --source-digest "$GITHUB_SHA" \ | ||
| --source-ref "$GITHUB_REF" | ||
| gh attestation verify "$asset" \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --signer-workflow "github.com/$GITHUB_REPOSITORY/.github/workflows/package.yml" \ | ||
| --source-digest "$GITHUB_SHA" \ | ||
| --source-ref "$GITHUB_REF" \ | ||
| --predicate-type "https://spdx.dev/Document" | ||
| done | ||
|
|
||
| release_tmp="$(mktemp -d "$RUNNER_TEMP/base-cli-release.XXXXXX")" | ||
| trap 'rm -rf "$release_tmp"' EXIT | ||
| release_json="$release_tmp/release.json" | ||
| release_error="$release_tmp/release-error.txt" | ||
|
|
||
| read_release_metadata() { | ||
| 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 | ||
| return 1 | ||
| fi | ||
| cat "$release_error" >&2 | ||
| return 2 | ||
| } | ||
|
|
||
| verify_existing_release() { | ||
| local existing_assets="$release_tmp/existing-assets" | ||
| mkdir -p "$existing_assets" | ||
| gh release download "$tag" --repo "$GITHUB_REPOSITORY" --dir "$existing_assets" | ||
| python scripts/verify_release_assets.py \ | ||
| --expected-dir dist \ | ||
| --existing-dir "$existing_assets" \ | ||
| --release-json "$release_json" \ | ||
| --version-file VERSION \ | ||
| --tag "$tag" \ | ||
| --source-commit "$GITHUB_SHA" \ | ||
| --resolved-tag-commit "$tag_commit" | ||
| } | ||
|
|
||
| if read_release_metadata; then | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| verify_existing_release | ||
| echo "Existing release $tag is byte-for-byte identical; leaving the immutable release unchanged." | ||
| exit 0 | ||
| else | ||
| release_status=$? | ||
| if [[ "$release_status" -ne 1 ]]; then | ||
| exit "$release_status" | ||
| fi | ||
| fi | ||
|
|
||
| create_status=0 | ||
| gh release create "$tag" "${assets[@]}" \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --title "$tag" \ | ||
| --generate-notes \ | ||
| --notes "Published distributions and release metadata for $tag. See CHANGELOG.md for the reviewed release notes." | ||
| --notes "Published distributions and release metadata for $tag. See CHANGELOG.md for the reviewed release notes." \ | ||
| || create_status=$? | ||
|
|
||
| if read_release_metadata; then | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correctness: success is inferred solely from the post-create read-back matching bytes, so a genuine failure reported by |
||
| verify_existing_release | ||
| echo "Verified immutable release $tag after publication." | ||
| else | ||
| release_status=$? | ||
| if [[ "$create_status" -ne 0 ]]; then | ||
| exit "$create_status" | ||
| fi | ||
| echo "Release $tag was created but could not be read back (HTTP 404)." >&2 | ||
| exit "$release_status" | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| #!/usr/bin/env python3 | ||
| """Verify that an existing GitHub Release is an exact replay of reviewed assets.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import hashlib | ||
| import json | ||
| import re | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| SBOM_NAME = "SBOM.spdx.json" | ||
| CHECKSUMS_NAME = "SHA256SUMS" | ||
| BOM_ROW_NAME = "RELEASE-BOM-ROW.json" | ||
| SHA_RE = re.compile(r"^[0-9a-f]{40}$") | ||
| HEX_SHA256_RE = re.compile(r"^[0-9a-f]{64}$") | ||
|
|
||
|
|
||
| def _sha256(path: Path) -> str: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reuse: the |
||
| digest = hashlib.sha256() | ||
| with path.open("rb") as stream: | ||
| for chunk in iter(lambda: stream.read(1024 * 1024), b""): | ||
| digest.update(chunk) | ||
| return digest.hexdigest() | ||
|
|
||
|
|
||
| def _files(directory: Path, label: str, errors: list[str]) -> dict[str, Path]: | ||
| if not directory.is_dir(): | ||
| errors.append(f"{label} directory does not exist: {directory}") | ||
| return {} | ||
| result: dict[str, Path] = {} | ||
| for path in directory.iterdir(): | ||
| if path.is_symlink() or not path.is_file(): | ||
| errors.append(f"{label} contains a non-regular asset: {path.name}") | ||
| continue | ||
| result[path.name] = path | ||
| return result | ||
|
|
||
|
|
||
| def _read_json(path: Path, label: str, errors: list[str]) -> dict[str, Any]: | ||
| try: | ||
| value = json.loads(path.read_text(encoding="utf-8")) | ||
| except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: | ||
| errors.append(f"{label} is not valid JSON: {exc}") | ||
| return {} | ||
| if not isinstance(value, dict): | ||
| errors.append(f"{label} must be a JSON object") | ||
| return {} | ||
| return value | ||
|
|
||
|
|
||
| def validate_release_assets( | ||
| expected_dir: Path, | ||
| existing_dir: Path, | ||
| release_json: Path, | ||
| version_file: Path, | ||
| *, | ||
| tag: str, | ||
| source_commit: str, | ||
| resolved_tag_commit: str, | ||
| ) -> list[str]: | ||
| """Return violations between reviewed files and an existing immutable release.""" | ||
|
|
||
| errors: list[str] = [] | ||
| expected = _files(expected_dir, "reviewed artifact", errors) | ||
| existing = _files(existing_dir, "existing release", errors) | ||
|
|
||
| required_names = {SBOM_NAME, CHECKSUMS_NAME, BOM_ROW_NAME} | ||
| wheels = {name for name in expected if name.endswith(".whl")} | ||
| sdists = {name for name in expected if name.endswith(".tar.gz")} | ||
| if len(wheels) != 1 or len(sdists) != 1 or not required_names.issubset(expected): | ||
| errors.append("reviewed artifacts must contain one wheel, one sdist, SHA256SUMS, SBOM, and release BOM row") | ||
|
|
||
| try: | ||
| version = version_file.read_text(encoding="utf-8").strip() | ||
| except (OSError, UnicodeDecodeError) as exc: | ||
| errors.append(f"could not read package VERSION: {exc}") | ||
| version = "" | ||
| if tag != f"v{version}" or not version: | ||
| errors.append(f"release tag {tag!r} does not match package version {version!r}") | ||
| if not SHA_RE.fullmatch(source_commit): | ||
| errors.append("source commit must be a lowercase full 40-character SHA") | ||
| if resolved_tag_commit != source_commit: | ||
| errors.append( | ||
| f"release tag resolves to {resolved_tag_commit!r}, not the reviewed source commit {source_commit!r}" | ||
| ) | ||
|
|
||
| release = _read_json(release_json, "existing release metadata", errors) | ||
| if release.get("tag_name") != tag: | ||
| errors.append(f"existing release tag {release.get('tag_name')!r} does not match {tag!r}") | ||
| if release.get("draft") is not False or release.get("prerelease") is not False: | ||
| errors.append("an existing release must be published and non-prerelease") | ||
|
|
||
| if SBOM_NAME in expected: | ||
| sbom = _read_json(expected[SBOM_NAME], SBOM_NAME, errors) | ||
| namespace = str(sbom.get("documentNamespace", "")) | ||
| if sbom.get("name") != f"base-cli-{version}": | ||
| errors.append(f"{SBOM_NAME} does not identify base-cli version {version}") | ||
| if f"/sbom/{version}/{source_commit}" not in namespace: | ||
| errors.append(f"{SBOM_NAME} namespace is not bound to tag {tag} and commit {source_commit}") | ||
| if source_commit not in str(sbom.get("documentComment", "")): | ||
| errors.append(f"{SBOM_NAME} comment is not bound to source commit {source_commit}") | ||
|
|
||
| if BOM_ROW_NAME in expected: | ||
| bom_row = _read_json(expected[BOM_ROW_NAME], BOM_ROW_NAME, errors) | ||
| if bom_row.get("repository") != "basefoundry/base-cli": | ||
| errors.append(f"{BOM_ROW_NAME} identifies the wrong repository") | ||
| if bom_row.get("version") != version or bom_row.get("tag") != tag: | ||
| errors.append(f"{BOM_ROW_NAME} version/tag does not match {tag}") | ||
| if bom_row.get("commit") != source_commit: | ||
| errors.append(f"{BOM_ROW_NAME} is not bound to source commit {source_commit}") | ||
|
|
||
| if CHECKSUMS_NAME in expected: | ||
| checksum_rows: dict[str, str] = {} | ||
| try: | ||
| lines = expected[CHECKSUMS_NAME].read_text(encoding="utf-8").splitlines() | ||
| except (OSError, UnicodeDecodeError) as exc: | ||
| errors.append(f"{CHECKSUMS_NAME} could not be read: {exc}") | ||
| lines = [] | ||
| for line in lines: | ||
| fields = line.split(maxsplit=1) | ||
| if len(fields) != 2 or not HEX_SHA256_RE.fullmatch(fields[0]): | ||
| errors.append(f"invalid checksum row in {CHECKSUMS_NAME}: {line!r}") | ||
| continue | ||
| checksum_rows[fields[1]] = fields[0] | ||
| binary_names = wheels | sdists | ||
| if set(checksum_rows) != binary_names: | ||
| errors.append(f"{CHECKSUMS_NAME} must cover exactly the reviewed wheel and sdist") | ||
| for name in binary_names & set(expected): | ||
| actual = _sha256(expected[name]) | ||
| if checksum_rows.get(name) != actual: | ||
| errors.append(f"{CHECKSUMS_NAME} digest for {name} does not match the reviewed artifact") | ||
|
|
||
| expected_names = set(expected) | ||
| existing_names = set(existing) | ||
| if expected_names != existing_names: | ||
| errors.append( | ||
| "release asset filenames differ: " | ||
| f"expected {sorted(expected_names)!r}, observed {sorted(existing_names)!r}; " | ||
| "refusing to replace immutable release assets" | ||
| ) | ||
| for name in sorted(expected_names & existing_names): | ||
| expected_digest = _sha256(expected[name]) | ||
| existing_digest = _sha256(existing[name]) | ||
| if expected_digest != existing_digest: | ||
| errors.append( | ||
| f"release asset {name} differs: expected SHA-256 {expected_digest}, " | ||
| f"observed SHA-256 {existing_digest}; refusing to replace immutable release assets" | ||
| ) | ||
| return errors | ||
|
|
||
|
|
||
| def main() -> None: | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--expected-dir", type=Path, required=True) | ||
| parser.add_argument("--existing-dir", type=Path, required=True) | ||
| parser.add_argument("--release-json", type=Path, required=True) | ||
| parser.add_argument("--version-file", type=Path, default=Path("VERSION")) | ||
| parser.add_argument("--tag", required=True) | ||
| parser.add_argument("--source-commit", required=True) | ||
| parser.add_argument("--resolved-tag-commit", required=True) | ||
| args = parser.parse_args() | ||
| errors = validate_release_assets( | ||
| args.expected_dir, | ||
| args.existing_dir, | ||
| args.release_json, | ||
| args.version_file, | ||
| tag=args.tag, | ||
| source_commit=args.source_commit, | ||
| resolved_tag_commit=args.resolved_tag_commit, | ||
| ) | ||
| if errors: | ||
| for error in errors: | ||
| print(f"release asset verification failed: {error}", file=sys.stderr) | ||
| raise SystemExit(1) | ||
| print(f"Verified existing release {args.tag}: all assets and source identities are unchanged.") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,13 @@ def test_package_workflow_uses_numeric_reproducibility_epoch() -> None: | |
|
|
||
| def test_package_workflow_does_not_replace_published_release_assets() -> None: | ||
| 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 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test gap: this test still asserts |
||
| assert "refusing to replace immutable release assets" in workflow | ||
| assert "verify_release_assets.py" in workflow | ||
| assert "refusing to replace immutable release assets" in verifier | ||
| assert "gh release download" in workflow | ||
| assert "gh attestation verify" in workflow | ||
| assert '--source-digest "$GITHUB_SHA"' in workflow | ||
| assert '--source-ref "$GITHUB_REF"' in workflow | ||
| assert "--clobber" not in workflow | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theghCLI's error text. If a futureghversion 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.