From a7e7f1ae557e0414413d78844b6e7621e4dd4edb Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Thu, 5 Nov 2026 22:49:39 +0800 Subject: [PATCH 1/2] fix(release): verify archives before signing Compare downloaded archives with the RC commit before signing. --- dev/release/README.md | 13 +++ dev/release/release_rc.sh | 46 +++++++- dev/release/test_release_rc.py | 202 +++++++++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 5 deletions(-) create mode 100644 dev/release/test_release_rc.py diff --git a/dev/release/README.md b/dev/release/README.md index 13b63e605..6df1e20cf 100644 --- a/dev/release/README.md +++ b/dev/release/README.md @@ -50,6 +50,12 @@ Requirements to run `release_rc.sh`: * You must be an Apache Iceberg committer or PMC member * You must prepare your PGP key for signing +The signing step also requires `gzip`, `cmp`, `mktemp`, and either `shasum` or +`sha512sum`. It verifies the downloaded checksum and compares the uncompressed +archive against `git archive` of the RC commit before signing. Differences in +Git's tar output or local archive settings can cause verification to fail. +An existing RC output directory must be moved aside before retrying. + If you don't have a PGP key, https://infra.apache.org/release-signing.html#generate may be helpful. @@ -108,3 +114,10 @@ $ dev/release/verify_rc.sh ${VERSION} ${RC} ``` If the verification is successful, the message `RC looks good!` is shown. + +The release script's regression tests run locally with simulated signing and +upload commands: + +```console +$ python3 -B -m unittest discover -s dev/release -p 'test_release_rc.py' -v +``` diff --git a/dev/release/release_rc.sh b/dev/release/release_rc.sh index 5f4595f21..319191bc1 100755 --- a/dev/release/release_rc.sh +++ b/dev/release/release_rc.sh @@ -19,13 +19,22 @@ set -eu -for cmd in git gh gpg svn; do +for cmd in git gh gpg svn gzip cmp mktemp; do if ! command -v ${cmd} &> /dev/null; then echo "This script requires '${cmd}' but it's not installed. Aborting." exit 1 fi done +if command -v shasum > /dev/null 2>&1; then + sha512_verify=(shasum -a 512 -c) +elif command -v sha512sum > /dev/null 2>&1; then + sha512_verify=(sha512sum -c) +else + echo "This script requires 'shasum' or 'sha512sum'. Aborting." + exit 1 +fi + SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SOURCE_TOP_DIR="$(cd "${SOURCE_DIR}/../../" && pwd)" @@ -78,6 +87,11 @@ rc_id="${id}-rc${rc}" tar_gz="${id}.tar.gz" if [ "${RELEASE_SIGN}" -gt 0 ]; then + if [ -e "${rc_id}" ] || [ -L "${rc_id}" ]; then + echo "Release directory ${rc_id} already exists. Move it aside before retrying." + exit 1 + fi + git_origin_url="$(git remote get-url origin)" repository="${git_origin_url#*github.com?}" repository="${repository%.git}" @@ -126,15 +140,37 @@ if [ "${RELEASE_SIGN}" -gt 0 ]; then done fi - mkdir -p "${rc_id}" + verify_dir="$(mktemp -d "${TMPDIR:-/tmp}/iceberg-cpp-rc.XXXXXX")" + trap 'rm -rf "${verify_dir}"' EXIT + mkdir "${verify_dir}/artifacts" echo "Downloading .tar.gz and .sha512 from GitHub Releases" gh release download "${rc_tag}" \ - --dir "${rc_id}" \ + --dir "${verify_dir}/artifacts" \ --pattern "${tar_gz}" \ --pattern "${tar_gz}.sha512" \ - --repo "${repository}" \ - --skip-existing + --repo "${repository}" + + echo "Verifying downloaded tar.gz before signing" + if ! (cd "${verify_dir}/artifacts" && "${sha512_verify[@]}" "${tar_gz}.sha512"); then + echo "Downloaded ${tar_gz} failed SHA-512 verification. Not signing." + exit 1 + fi + if ! gzip -dc "${verify_dir}/artifacts/${tar_gz}" > "${verify_dir}/downloaded.tar"; then + echo "Failed to decompress ${tar_gz}. Not signing." + exit 1 + fi + if ! git archive "${rc_hash}" --format=tar --prefix "${id}/" > "${verify_dir}/expected.tar"; then + echo "Failed to archive RC commit ${rc_hash}. Not signing." + exit 1 + fi + # The checksum can be replaced along with the asset; anchor it to the RC commit. + if ! cmp -s "${verify_dir}/expected.tar" "${verify_dir}/downloaded.tar"; then + echo "Downloaded ${tar_gz} does not match RC commit ${rc_hash}. Not signing." + exit 1 + fi + + mv "${verify_dir}/artifacts" "${rc_id}" echo "Signing tar.gz" cd "${rc_id}" diff --git a/dev/release/test_release_rc.py b/dev/release/test_release_rc.py new file mode 100644 index 000000000..c006e6349 --- /dev/null +++ b/dev/release/test_release_rc.py @@ -0,0 +1,202 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import gzip +import hashlib +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import unittest + + +MOCK_TOOL = r''' +import os +from pathlib import Path +import shutil +import subprocess +import sys + +tool = Path(sys.argv[0]).name +args = sys.argv[1:] +if tool in ("git", "gzip"): + result = subprocess.run([os.environ["REAL_" + tool.upper()], *args]) + if tool == "git" and "archive" in args and os.environ.get("FAIL_ARCHIVE"): + sys.exit(1) + sys.exit(result.returncode) +elif tool == "gh" and args[:2] == ["run", "watch"]: + pass +elif tool == "gh" and args[:2] == ["release", "download"]: + destination = Path(args[args.index("--dir") + 1]) + for index, arg in enumerate(args): + if arg == "--pattern": + filename = args[index + 1] + target = destination / filename + if target.exists() and "--skip-existing" in args: + continue + shutil.copyfile(Path(os.environ["ASSETS"]) / filename, target) +elif tool == "gpg": + Path(args[args.index("--output") + 1]).write_text("test signature") +elif tool == "svn" and args[0] == "import": + pass +elif tool != "gh" or args[:2] != ["release", "upload"]: + sys.exit("Unexpected command: " + tool + " " + " ".join(args)) + +if tool in ("gpg", "svn") or (tool == "gh" and args[:2] == ["release", "upload"]): + with open(os.environ["CALL_LOG"], "a") as log: + log.write(tool + " " + " ".join(args) + "\n") +''' + + +class ReleaseRcTest(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="iceberg-release-test-") + self.addCleanup(temporary.cleanup) + self.directory = Path(temporary.name) + self.repo = self.directory / "repo" + self.repo.mkdir() + self.assets = self.directory / "assets" + self.assets.mkdir() + self.calls = self.directory / "calls.log" + self.scratch = self.directory / "scratch" + self.scratch.mkdir() + self.name = "apache-iceberg-cpp-1.2.3" + self.tar_gz = self.name + ".tar.gz" + self.output = self.repo / (self.name + "-rc0") + self.real_git = shutil.which("git") + self.env = os.environ.copy() + for key in list(self.env): + if key.startswith(("RELEASE_", "GIT_")): + del self.env[key] + self.env.update({ + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_AUTHOR_NAME": "Release Test", + "GIT_AUTHOR_EMAIL": "test@example.invalid", + "GIT_COMMITTER_NAME": "Release Test", + "GIT_COMMITTER_EMAIL": "test@example.invalid", + "RELEASE_DEFAULT": "0", + "RELEASE_SIGN": "1", + "RELEASE_UPLOAD": "1", + "RELEASE_WATCH": "1", + "RELEASE_RUN_ID": "123", + "ASSETS": str(self.assets), + "CALL_LOG": str(self.calls), + "REAL_GIT": self.real_git, + "REAL_GZIP": shutil.which("gzip"), + "TMPDIR": str(self.scratch), + }) + self.git("init", "--quiet") + self.git("remote", "add", "origin", "https://github.com/example/iceberg-cpp.git") + (self.repo / "source.txt").write_text("original source\n") + self.git("add", "source.txt") + self.git("commit", "--quiet", "-m", "Initial source") + self.git("tag", "-a", "v1.2.3-rc0", "-m", "Release candidate") + self.archive = self.git("archive", "HEAD", "--prefix", self.name + "/") + self.publish(gzip.compress(self.archive, mtime=123)) + + script = Path(__file__).with_name("release_rc.sh") + self.script = self.repo / "dev" / "release" / script.name + self.script.parent.mkdir(parents=True) + shutil.copyfile(script, self.script) + commands = self.directory / "bin" + commands.mkdir() + for tool in ("gh", "gpg", "svn", "git", "gzip"): + executable = commands / tool + executable.write_text("#!" + sys.executable + "\n" + MOCK_TOOL) + executable.chmod(0o755) + self.env["PATH"] = str(commands) + os.pathsep + self.env["PATH"] + + def git(self, *args): + return subprocess.check_output( + [self.real_git, *args], cwd=self.repo, env=self.env, + stderr=subprocess.STDOUT, + ) + + def publish(self, archive): + (self.assets / self.tar_gz).write_bytes(archive) + checksum = hashlib.sha512(archive).hexdigest() + (self.assets / (self.tar_gz + ".sha512")).write_text( + checksum + " " + self.tar_gz + "\n" + ) + + def release(self): + return subprocess.run( + ["bash", str(self.script), "1.2.3", "0"], + cwd=self.repo, env=self.env, capture_output=True, text=True, + timeout=30, + ) + + def assert_rejected(self, result, message): + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn(message, result.stdout + result.stderr) + self.assertFalse(self.calls.exists(), "Signing or upload was attempted") + self.assertEqual(list(self.scratch.iterdir()), []) + + def test_valid_archive_with_different_gzip_metadata(self): + result = self.release() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual( + (self.output / self.tar_gz).read_bytes(), + (self.assets / self.tar_gz).read_bytes(), + ) + self.assertEqual( + {path.name for path in self.output.iterdir()}, + {self.tar_gz, self.tar_gz + ".sha512", self.tar_gz + ".asc"}, + ) + calls = self.calls.read_text().splitlines() + self.assertEqual([line.split()[0] for line in calls], ["gpg", "gh", "svn"]) + self.assertEqual(list(self.scratch.iterdir()), []) + + def test_replaced_archive_and_checksum(self): + (self.repo / "source.txt").write_text("modified source\n") + self.git("add", "source.txt") + self.git("commit", "--quiet", "-m", "Unreleased changes") + modified = self.git("archive", "HEAD", "--prefix", self.name + "/") + self.publish(gzip.compress(modified)) + self.assert_rejected(self.release(), "does not match RC commit") + + def test_incorrect_checksum(self): + (self.assets / (self.tar_gz + ".sha512")).write_text( + "0" * 128 + " " + self.tar_gz + "\n" + ) + self.assert_rejected(self.release(), "failed SHA-512 verification") + + def test_corrupt_gzip_trailer_with_matching_checksum(self): + archive = bytearray((self.assets / self.tar_gz).read_bytes()) + archive[-8] ^= 1 + self.publish(archive) + self.assert_rejected(self.release(), "Failed to decompress") + + def test_existing_release_directory_is_preserved(self): + self.output.mkdir() + stale = self.output / self.tar_gz + stale.write_bytes(b"stale archive") + self.assert_rejected(self.release(), "already exists") + self.assertEqual(stale.read_bytes(), b"stale archive") + + def test_failed_git_archive_after_writing_complete_output(self): + self.env["FAIL_ARCHIVE"] = "1" + self.assert_rejected(self.release(), "Failed to archive RC commit") + + +if __name__ == "__main__": + unittest.main() From 41b9befd7d7624b1d9911da56697766a4a873aa1 Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Mon, 9 Nov 2026 20:11:52 +0800 Subject: [PATCH 2/2] fix(release): pin archive settings and run tests in CI --- .github/workflows/rc.yml | 3 +- .github/workflows/release_test.yml | 61 ++++++++++++++++++++++++++++++ dev/release/README.md | 9 +++-- dev/release/release_rc.sh | 3 +- dev/release/test_release_rc.py | 20 +++++++++- 5 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/release_test.yml diff --git a/.github/workflows/rc.yml b/.github/workflows/rc.yml index 1af2b9b7d..0dcbc77ef 100644 --- a/.github/workflows/rc.yml +++ b/.github/workflows/rc.yml @@ -55,7 +55,8 @@ jobs: ARCHIVE_FILE_NAME="apache-iceberg-cpp-${VERSION}" tar_gz="${ARCHIVE_FILE_NAME}.tar.gz" echo "TAR_GZ=${tar_gz}" >> ${GITHUB_ENV} - git archive HEAD --prefix "${ARCHIVE_FILE_NAME}/" --output "${tar_gz}" + git -c core.autocrlf=false -c core.eol=lf -c tar.umask=0002 \ + archive HEAD --prefix "${ARCHIVE_FILE_NAME}/" --output "${tar_gz}" sha512sum "${tar_gz}" > "${tar_gz}.sha512" - name: Audit diff --git a/.github/workflows/release_test.yml b/.github/workflows/release_test.yml new file mode 100644 index 000000000..39632890f --- /dev/null +++ b/.github/workflows/release_test.yml @@ -0,0 +1,61 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Release Script Tests + +on: + push: + branches: + - main + paths: + - 'dev/release/**' + - '.github/workflows/rc.yml' + - '.github/workflows/release_test.yml' + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - 'dev/release/**' + - '.github/workflows/rc.yml' + - '.github/workflows/release_test.yml' + +concurrency: + group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + release-tests: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} + runs-on: ${{ matrix.os }} + timeout-minutes: 5 + strategy: + fail-fast: false + matrix: + os: [ubuntu-26.04, macos-26] + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 + with: + python-version: '3.13' + - name: Test release script + run: python3 -B -m unittest discover -s dev/release -p 'test_release_rc.py' -v diff --git a/dev/release/README.md b/dev/release/README.md index 6df1e20cf..596ff2f63 100644 --- a/dev/release/README.md +++ b/dev/release/README.md @@ -52,8 +52,9 @@ Requirements to run `release_rc.sh`: The signing step also requires `gzip`, `cmp`, `mktemp`, and either `shasum` or `sha512sum`. It verifies the downloaded checksum and compares the uncompressed -archive against `git archive` of the RC commit before signing. Differences in -Git's tar output or local archive settings can cause verification to fail. +archive against `git archive` of the RC commit before signing. Both CI and local +verification pin line endings and archive permissions. Differences in Git's tar +output across versions can still cause verification to fail. An existing RC output directory must be moved aside before retrying. If you don't have a PGP key, https://infra.apache.org/release-signing.html#generate @@ -115,8 +116,8 @@ $ dev/release/verify_rc.sh ${VERSION} ${RC} If the verification is successful, the message `RC looks good!` is shown. -The release script's regression tests run locally with simulated signing and -upload commands: +The release script's regression tests run in CI on Ubuntu and macOS with +simulated signing and upload commands. To run them locally: ```console $ python3 -B -m unittest discover -s dev/release -p 'test_release_rc.py' -v diff --git a/dev/release/release_rc.sh b/dev/release/release_rc.sh index 319191bc1..680142a8d 100755 --- a/dev/release/release_rc.sh +++ b/dev/release/release_rc.sh @@ -160,7 +160,8 @@ if [ "${RELEASE_SIGN}" -gt 0 ]; then echo "Failed to decompress ${tar_gz}. Not signing." exit 1 fi - if ! git archive "${rc_hash}" --format=tar --prefix "${id}/" > "${verify_dir}/expected.tar"; then + if ! git -c core.autocrlf=false -c core.eol=lf -c tar.umask=0002 \ + archive "${rc_hash}" --format=tar --prefix "${id}/" > "${verify_dir}/expected.tar"; then echo "Failed to archive RC commit ${rc_hash}. Not signing." exit 1 fi diff --git a/dev/release/test_release_rc.py b/dev/release/test_release_rc.py index c006e6349..204a7d514 100644 --- a/dev/release/test_release_rc.py +++ b/dev/release/test_release_rc.py @@ -107,7 +107,8 @@ def setUp(self): self.git("init", "--quiet") self.git("remote", "add", "origin", "https://github.com/example/iceberg-cpp.git") (self.repo / "source.txt").write_text("original source\n") - self.git("add", "source.txt") + (self.repo / ".gitattributes").write_text("source.txt text\n") + self.git("add", "source.txt", ".gitattributes") self.git("commit", "--quiet", "-m", "Initial source") self.git("tag", "-a", "v1.2.3-rc0", "-m", "Release candidate") self.archive = self.git("archive", "HEAD", "--prefix", self.name + "/") @@ -151,7 +152,7 @@ def assert_rejected(self, result, message): self.assertFalse(self.calls.exists(), "Signing or upload was attempted") self.assertEqual(list(self.scratch.iterdir()), []) - def test_valid_archive_with_different_gzip_metadata(self): + def assert_release_succeeds(self): result = self.release() self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertEqual( @@ -166,6 +167,21 @@ def test_valid_archive_with_different_gzip_metadata(self): self.assertEqual([line.split()[0] for line in calls], ["gpg", "gh", "svn"]) self.assertEqual(list(self.scratch.iterdir()), []) + def test_valid_archive_with_different_gzip_metadata(self): + self.assert_release_succeeds() + + def test_local_autocrlf_does_not_change_archive(self): + self.git("config", "core.autocrlf", "true") + self.assert_release_succeeds() + + def test_local_eol_does_not_change_archive(self): + self.git("config", "core.eol", "crlf") + self.assert_release_succeeds() + + def test_local_tar_umask_does_not_change_archive(self): + self.git("config", "tar.umask", "022") + self.assert_release_succeeds() + def test_replaced_archive_and_checksum(self): (self.repo / "source.txt").write_text("modified source\n") self.git("add", "source.txt")