From ac4a2125643d693e07a77358d8497046eedfdfab Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:18:43 -0700 Subject: [PATCH 1/6] chore(release): add one-command approval tooling A merged release PR does not ship on its own: release-tag.yml is gated on the protected `release` environment and sits in `waiting` until a maintainer approves it. Nothing times out and nothing sends a reminder, so a prepared release can sit indefinitely while Release PR's own guard refuses to prepare the next one. v0.1.2 waited an hour for exactly this. Finding that run in the Actions UI was the only step between a merged PR and a signed tag. `make release-approve` finds it, prints the tag it would mint and the commit that tag would point at, and asks before POSTing the approval; `make release-status` reports where a release stands without touching anything. The gate itself is unchanged. It is what keeps the CI release key from being usable by anyone who can merge a `release/*` PR, so this makes the approval easy to find and press rather than automatic. The branch name is parsed through the shared parse-version.sh for the same reason the workflows use it: it decides what gets tagged, so `release/v1.0.0; rm -rf /` has to be rejected rather than interpolated. Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 12 ++ scripts/release/approve.sh | 149 +++++++++++++++++++++++++ tests/test-release-approve.sh | 200 ++++++++++++++++++++++++++++++++++ tests/test-unit.sh | 1 + 4 files changed, 362 insertions(+) create mode 100755 scripts/release/approve.sh create mode 100755 tests/test-release-approve.sh diff --git a/Makefile b/Makefile index a382e33..065dd11 100644 --- a/Makefile +++ b/Makefile @@ -122,6 +122,18 @@ release-pr: ## Trigger Release PR workflow (usage: make release-pr [VERSION=1.2. release-pr-watch: ## Watch the latest workflow run (run after make release-pr) @gh run watch +.PHONY: release-status +release-status: ## Show latest release, any open release PR, and any run awaiting approval + @sh scripts/release/approve.sh --status + +.PHONY: release-approve +release-approve: ## Approve the waiting release (usage: make release-approve [YES=1] to skip the prompt) + @if [ -n "$(YES)" ]; then \ + sh scripts/release/approve.sh --yes; \ + else \ + sh scripts/release/approve.sh; \ + fi + .PHONY: enable-pre-commit enable-pre-commit: ## Enable pre-commit hooks (along with commit-msg and pre-push hooks) @if command -v pre-commit >/dev/null 2>&1; then \ diff --git a/scripts/release/approve.sh b/scripts/release/approve.sh new file mode 100755 index 0000000..d178c14 --- /dev/null +++ b/scripts/release/approve.sh @@ -0,0 +1,149 @@ +#!/bin/sh +# Approve the release waiting on the protected `release` environment. +# +# Usage: approve.sh [--status] [--yes] +# +# A merged release PR does not ship on its own. release-tag.yml is gated on the +# `release` environment, so it sits in `waiting` until a maintainer approves it. +# Nothing times out and nothing sends a reminder, so a prepared release can sit +# indefinitely while Release PR's own guard refuses to prepare the next one. +# Finding that run in the Actions UI is the only step between a merged PR and a +# signed tag; this turns it into one command. +# +# --status reports where a release stands and changes nothing. +# +# Approving mints a GPG-signed tag, so a bare run prints the tag it would create +# and the commit that tag would point at, then asks for confirmation. --yes +# skips the prompt and is required when stdin is not a terminal. +# +# The branch name is parsed through parse-version.sh for the same reason the +# workflows do it: it decides what gets tagged, so a `case` glob that also +# matches `v1.0.0; rm -rf /` is not good enough. +# +# Requires an authenticated `gh` and `jq`. Exits 1 when nothing is waiting. + +set -eu + +SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)" +WORKFLOW="release-tag.yml" + +usage() { + echo "Usage: $0 [--status] [--yes]" >&2 + exit 2 +} + +die() { + echo "Error: $1" >&2 + exit 1 +} + +mode="approve" +assume_yes=0 + +while [ "$#" -gt 0 ]; do + case "$1" in + --status) mode="status" ;; + --yes | -y) assume_yes=1 ;; + -h | --help) usage ;; + *) usage ;; + esac + shift +done + +for tool in gh jq; do + command -v "$tool" >/dev/null 2>&1 || + die "$tool is required but not installed." +done + +repo="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" || + die "Could not resolve the repository. Is gh authenticated?" + +# --limit 1 is safe because release-tag.yml sets `concurrency: release-tag` +# without cancel-in-progress, so at most one of its runs is ever awaiting +# approval. +waiting="$(gh run list --workflow="$WORKFLOW" --status waiting --limit 1 \ + --json databaseId,headBranch,headSha,url)" || + die "Could not list $WORKFLOW runs." + +run_id="$(printf '%s' "$waiting" | jq -r '.[0].databaseId // empty')" + +if [ "$mode" = "status" ]; then + latest="$(gh release view --json tagName --jq .tagName 2>/dev/null || echo "none")" + echo "Latest release: $latest" + + open_pr="$(gh pr list --state open --json number,title,headRefName \ + --jq '[.[] | select(.headRefName | startswith("release/"))] + | map("#\(.number) \(.title)") | join(", ") // empty')" + echo "Open release PR: ${open_pr:-none}" + + if [ -n "$run_id" ]; then + branch="$(printf '%s' "$waiting" | jq -r '.[0].headBranch')" + url="$(printf '%s' "$waiting" | jq -r '.[0].url')" + echo "Awaiting approval: ${branch#release/} — $url" + echo + echo "Run 'make release-approve' to approve it." + else + echo "Awaiting approval: none" + fi + + exit 0 +fi + +[ -n "$run_id" ] || + die "No $WORKFLOW run is awaiting approval. Nothing to release." + +branch="$(printf '%s' "$waiting" | jq -r '.[0].headBranch')" +head_sha="$(printf '%s' "$waiting" | jq -r '.[0].headSha')" +run_url="$(printf '%s' "$waiting" | jq -r '.[0].url')" + +tag="$("$SCRIPT_DIR/parse-version.sh" "${branch#release/}" --require-v)" || + die "Branch '$branch' does not encode a vX.Y.Z tag." + +pending="$(gh api "repos/$repo/actions/runs/$run_id/pending_deployments")" || + die "Could not read pending deployments for run $run_id." + +env_count="$(printf '%s' "$pending" | jq 'length')" +[ "$env_count" -gt 0 ] || + die "Run $run_id is waiting, but no deployment is pending your approval." + +can_approve="$(printf '%s' "$pending" | jq -r 'all(.current_user_can_approve)')" +[ "$can_approve" = "true" ] || + die "You are not an approver for the environment gating run $run_id." + +env_ids="$(printf '%s' "$pending" | jq -c '[.[].environment.id]')" +env_names="$(printf '%s' "$pending" | jq -r '[.[].environment.name] | join(", ")')" + +cat <&2 + exit 0 + ;; + esac +fi + +jq -nc --argjson ids "$env_ids" \ + '{environment_ids: $ids, state: "approved", + comment: "Approved via scripts/release/approve.sh"}' | + gh api "repos/$repo/actions/runs/$run_id/pending_deployments" \ + --method POST --input - >/dev/null || + die "Approval failed for run $run_id." + +echo "Approved. $tag will be tagged and published." +echo "Watch it: gh run watch $run_id" diff --git a/tests/test-release-approve.sh b/tests/test-release-approve.sh new file mode 100755 index 0000000..30490c5 --- /dev/null +++ b/tests/test-release-approve.sh @@ -0,0 +1,200 @@ +#!/usr/bin/env bash +# Test script for scripts/release/approve.sh +# +# `gh` is stubbed on PATH so nothing here touches the network or the real +# repository. The stub is driven by GH_STUB_* variables and records any POST +# body it is handed, which is how the tests below tell "would have approved" +# apart from "approved nothing". + +set -uo pipefail +TEST_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# Load shared configurations +# shellcheck disable=SC1091 # Dynamic path via $TEST_SCRIPT_DIR +. "$TEST_SCRIPT_DIR/colors.sh" + +APPROVE="$TEST_SCRIPT_DIR/../scripts/release/approve.sh" + +PASSED=0 +FAILED=0 + +pass() { + echo -e "${GREEN}✓${NC} $1" + ((PASSED++)) +} + +fail() { + echo -e "${RED}✗${NC} $1" + shift + for msg in "$@"; do + echo -e " ${YELLOW}$msg${NC}" + done + ((FAILED++)) +} + +STUB_DIR="$(mktemp -d)" +trap 'rm -rf "$STUB_DIR"' EXIT + +cat >"$STUB_DIR/gh" <<'STUB' +#!/usr/bin/env bash +case "$1" in +run) printf '%s\n' "${GH_STUB_WAITING:-[]}" ;; +repo) printf '%s\n' "${GH_STUB_REPO:-michen00/custom-commit-hooks}" ;; +release) printf '%s\n' "${GH_STUB_LATEST:-v0.1.2}" ;; +pr) printf '%s\n' "${GH_STUB_OPENPR:-}" ;; +api) + for arg in "$@"; do + if [ "$arg" = "POST" ]; then + cat >>"${GH_STUB_POST_LOG:-/dev/null}" + echo '{}' + exit 0 + fi + done + printf '%s\n' "${GH_STUB_PENDING:-[]}" + ;; +esac +exit 0 +STUB +chmod +x "$STUB_DIR/gh" + +WAITING_RUN='[{"databaseId":42,"headBranch":"release/v9.9.9","headSha":"abc123","url":"https://example.invalid/run/42"}]' +PENDING_OK='[{"environment":{"id":7,"name":"release"},"current_user_can_approve":true}]' +PENDING_DENIED='[{"environment":{"id":7,"name":"release"},"current_user_can_approve":false}]' + +# run_approve [args...] -- runs approve.sh with the stub on PATH. +# Stdin is /dev/null so the confirmation prompt's terminal check is deterministic. +run_approve() { + local log="$1" + shift + GH_STUB_POST_LOG="$log" PATH="$STUB_DIR:$PATH" \ + sh "$APPROVE" "$@" &1 +} + +# --- argument handling ------------------------------------------------------- + +PATH="$STUB_DIR:$PATH" sh "$APPROVE" --bogus >/dev/null 2>&1 +if [ "$?" -eq 2 ]; then + pass "unknown flag exits 2" +else + fail "unknown flag" "Expected exit 2" +fi + +# --- nothing waiting --------------------------------------------------------- + +log="$STUB_DIR/post1.log" +: >"$log" +out="$(GH_STUB_WAITING='[]' run_approve "$log")" +status=$? +if [ "$status" -ne 1 ]; then + fail "no waiting run exits 1" "Exited $status, expected 1" +elif [[ "$out" != *"Nothing to release"* ]]; then + fail "no waiting run explains itself" "Got: $out" +elif [ -s "$log" ]; then + fail "no waiting run approves nothing" "A POST was sent" +else + pass "no waiting run exits 1 and approves nothing" +fi + +out="$(GH_STUB_WAITING='[]' run_approve "$log" --status)" +status=$? +if [ "$status" -ne 0 ]; then + fail "--status exits 0 when idle" "Exited $status, expected 0" +elif [[ "$out" != *"Awaiting approval: none"* ]]; then + fail "--status reports an idle repository" "Got: $out" +else + pass "--status exits 0 and reports nothing awaiting approval" +fi + +# --- status with a run in flight --------------------------------------------- + +out="$(GH_STUB_WAITING="$WAITING_RUN" run_approve "$log" --status)" +if [[ "$out" == *"v9.9.9"* && "$out" == *"https://example.invalid/run/42"* ]]; then + pass "--status names the pending version and links the run" +else + fail "--status names the pending version" "Got: $out" +fi + +if [ -s "$log" ]; then + fail "--status is read-only" "A POST was sent" +else + pass "--status never approves anything" +fi + +# --- refusing to approve ----------------------------------------------------- + +: >"$log" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_OK" \ + run_approve "$log")" +status=$? +if [ "$status" -ne 1 ]; then + fail "non-tty without --yes exits 1" "Exited $status, expected 1" +elif [[ "$out" != *"not a terminal"* ]]; then + fail "non-tty without --yes explains itself" "Got: $out" +elif [ -s "$log" ]; then + fail "non-tty without --yes approves nothing" "A POST was sent" +else + pass "refuses to approve unprompted without --yes" +fi + +: >"$log" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_DENIED" \ + run_approve "$log" --yes)" +status=$? +if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then + pass "a non-approver is rejected without a POST" +else + fail "a non-approver is rejected" "Exited $status; log $([ -s "$log" ] && echo "non-empty" || echo "empty")" +fi + +: >"$log" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING='[]' \ + run_approve "$log" --yes)" +status=$? +if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then + pass "a waiting run with no pending deployment is rejected" +else + fail "no pending deployment is rejected" "Exited $status" +fi + +# A branch name reaches this script from a merged pull request, so it is +# attacker-influenced in the same way the workflows' inputs are. +: >"$log" +INJECT='[{"databaseId":42,"headBranch":"release/v1.0.0; rm -rf /","headSha":"abc","url":"u"}]' +out="$(GH_STUB_WAITING="$INJECT" GH_STUB_PENDING="$PENDING_OK" \ + run_approve "$log" --yes)" +status=$? +if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then + pass "a branch that is not a clean vX.Y.Z is rejected" +else + fail "malformed branch is rejected" "Exited $status" +fi + +# --- the approving path ------------------------------------------------------ + +: >"$log" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_OK" \ + run_approve "$log" --yes)" +status=$? +if [ "$status" -ne 0 ]; then + fail "--yes approves" "Exited $status, expected 0. Output: $out" +elif [ ! -s "$log" ]; then + fail "--yes sends an approval" "No POST was recorded" +else + body="$(cat "$log")" + if [[ "$(jq -r '.state' <<<"$body")" == "approved" ]] && + [[ "$(jq -c '.environment_ids' <<<"$body")" == "[7]" ]]; then + pass "--yes POSTs state=approved for the pending environment" + else + fail "--yes POSTs the right body" "Got: $body" + fi +fi + +if [[ "$out" == *"v9.9.9"* ]]; then + pass "the approval summary names the tag being minted" +else + fail "the approval summary names the tag" "Got: $out" +fi + +printf "\nResults: ${GREEN}%d passed${NC}, ${RED}%d failed${NC}\n" "$PASSED" "$FAILED" + +[ "$FAILED" -eq 0 ] diff --git a/tests/test-unit.sh b/tests/test-unit.sh index 05e8660..e2e3619 100755 --- a/tests/test-unit.sh +++ b/tests/test-unit.sh @@ -43,6 +43,7 @@ run_test "$TEST_SCRIPT_DIR/test-conventional-merge-commit.sh" "conventional-merg run_test "$TEST_SCRIPT_DIR/test-parse-version.sh" "release/parse-version.sh tests" run_test "$TEST_SCRIPT_DIR/test-bump-pins.sh" "release/bump-pins.sh tests" run_test "$TEST_SCRIPT_DIR/test-stamp-changelog.sh" "release/stamp-changelog.sh tests" +run_test "$TEST_SCRIPT_DIR/test-release-approve.sh" "release/approve.sh tests" run_test "$TEST_SCRIPT_DIR/test-cliff-header.sh" "cliff.toml changelog header tests" # Summary From 08f356acfdaa8c1024e78244e9ca3b34d4d1bb43 Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:19:02 -0700 Subject: [PATCH 2/6] docs: extract RELEASING.md from CONTRIBUTING The release material was lines 70-153 of CONTRIBUTING, about 45% of a contributor-facing document given over to a process only maintainers can run. It moves out largely verbatim; every claim in it was checked against the workflows and against the v0.1.0-v0.1.2 run history before the move, and all of them held. Two changes while moving it. The approval becomes its own numbered step rather than a consequence explained in prose after the list, because that is the one step a human has to take and burying it is how v0.1.2 came to wait an hour. And the flow now names `make release-status` and `make release-approve`. CONTRIBUTING keeps a `#### Creating a release` stub pointing here, so the `#creating-a-release` anchor still resolves for any link not updated below, and the heading is added to a table of contents that had been missing it. README, AGENTS.md, CLAUDE.md and copilot-instructions.md all now point at RELEASING; the copilot one had been linking `CONTRIBUTING.md` from inside `.github/`, which never resolved, and described the flow as Release PR then Release Publish with Release Tag left out entirely. Co-Authored-By: Claude Opus 5 (1M context) --- .github/copilot-instructions.md | 4 +- AGENTS.md | 2 +- CLAUDE.md | 2 +- CONTRIBUTING.md | 84 +---------------------- README.md | 2 +- RELEASING.md | 116 ++++++++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 87 deletions(-) create mode 100644 RELEASING.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 65c5db5..49ccad5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -87,8 +87,8 @@ git merge --no-ff feature-branch ## Release -- Releases use **Release PR** then **Release Publish** workflows; artifacts are signed with Sigstore and GPG. -- See [CONTRIBUTING](CONTRIBUTING.md#creating-a-release) for steps and verification. Do not change `scripts/release/` or release workflows without checking CONTRIBUTING. +- Releases run **Release PR** -> **Release Tag** -> **Release Publish**; **Release Tag** waits on the protected `release` environment for maintainer approval. Artifacts are signed with Sigstore and GPG. +- See [RELEASING](../RELEASING.md) for steps and verification. Do not change `scripts/release/` or release workflows without checking it. ## Boundaries diff --git a/AGENTS.md b/AGENTS.md index 18a10d2..f1f6076 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ git cliff --tag v1.0.0 --output CHANGELOG.md - Scripts: `scripts/release/build-artifacts.sh`, `scripts/release/sign-artifacts.sh`, `scripts/release/parse-version.sh`, `scripts/release/stamp-changelog.sh`, `scripts/update-unreleased.sh`. - Two changelog scripts, and they are not interchangeable. `update-unreleased.sh` refreshes the Unreleased section and belongs to the weekly autoupdate. `release/stamp-changelog.sh` writes the pending version as its own `## [X.Y.Z]` section and is what a release must use: once the tag exists, `git cliff --unreleased` no longer reports the commits it covers, so anything that only ever lived under Unreleased is dropped by the next refresh. That is how v0.1.0 shipped without a changelog section and nearly took 134 lines of history with it. - All three workflows validate versions through `scripts/release/parse-version.sh`; it is covered by `tests/test-parse-version.sh`. Do not replace it with a `case` glob such as `v[0-9]*.[0-9]*.[0-9]*`, which also matches `v1.0.0; rm -rf /`. -- Full steps and verification commands: see [CONTRIBUTING](CONTRIBUTING.md#creating-a-release). +- Full steps and verification commands: see [RELEASING](RELEASING.md). ## Testing diff --git a/CLAUDE.md b/CLAUDE.md index 9781d22..28aa55f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ See @README.md for project overview and features. - `.pre-commit-hooks.yaml` - Hook definitions for pre-commit framework - `cliff.toml` - Configuration for git-cliff changelog generation - `tests/` - Hook tests and test utilities (including `tests/test-unit.sh`) -- `scripts/release/` - Release artifact build and signing, plus `parse-version.sh` (shared strict `vX.Y.Z` validation used by all release workflows); `stamp-changelog.sh` writes the pending version as its own changelog section during a release, while `scripts/update-unreleased.sh` only refreshes Unreleased for the weekly autoupdate — a release must use the former, or entries that never got a versioned section are dropped by the next refresh. Release process and verification: see CONTRIBUTING. +- `scripts/release/` - Release artifact build and signing, plus `parse-version.sh` (shared strict `vX.Y.Z` validation used by all release workflows); `stamp-changelog.sh` writes the pending version as its own changelog section during a release, while `scripts/update-unreleased.sh` only refreshes Unreleased for the weekly autoupdate — a release must use the former, or entries that never got a versioned section are dropped by the next refresh. Release process and verification: see RELEASING.md. ## Code Style diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 48ec0a7..1e0e8c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,6 +18,7 @@ The project has defined a [code of conduct](CODE_OF_CONDUCT.md) to ensure a welc - [How do I submit a good enhancement suggestion?](#how-do-i-submit-a-good-enhancement-suggestion) - [Your first code contribution](#your-first-code-contribution) - [Contribution workflow](#contribution-workflow) + - [Creating a release](#creating-a-release) - [Recommended VSCode extensions](#recommended-vscode-extensions) ## I want to contribute @@ -69,88 +70,7 @@ Using the web-based interface to make changes is fine too, and will help you by #### Creating a release -Default flow (automated): - -1. **Release PR** (`.github/workflows/release-pr.yml`) opens the release PR by itself when a commit worth releasing lands on `main`. Every conventional type bumps at least the patch version, so the version cannot decide that on its own; the workflow gates on the group `cliff.toml` parsed each commit into — features, fixes, performance and reverts, plus anything marked breaking. A `chore`, `docs`, `build`, `ci`, `test`, `refactor` or `style` merge — the weekly hook autoupdate and Dependabot among them — rides along in the next release without proposing one. Run the workflow by hand, or `make release-pr`, to pin the version or to release a batch containing none of those types; a manual run skips the worthiness gate. Leave `version` empty to derive it via `git cliff --bumped-version`, or pass `X.Y.Z` / `vX.Y.Z`. -1. Review and merge the generated PR (`chore(release): prepare vX.Y.Z`). -1. **Release Tag** workflow (`.github/workflows/release-tag.yml`) runs on merge of a `release/*` branch. It creates a GPG-signed annotated tag, pushes it, and dispatches **Release Publish**. -1. **Release Publish** workflow (`.github/workflows/release-publish.yml`) builds and uploads signed artifacts to the GitHub release. - -No local step is required after the release PR merges, but the release is not automatic: **Release Tag** runs in the protected `release` environment and waits for a maintainer to approve the run. Approve from the run page, or from the PR's checks tab, to mint the tag and publish. Rejecting the approval leaves no tag behind. - -That is the only approval a normal release needs. **Release Publish** also declares the `release` environment, but when **Release Tag** dispatches it the deployment is created by `github-actions[bot]` and the reviewer rule is skipped, so it proceeds without a second prompt. The declaration still gates a Release Publish run dispatched by hand, which is the manual fallback path. Treat the Release Tag approval as the release decision — no tag means no publish. - -Two guards follow from that. Because the tag is what marks a release finished, **Release PR** refuses to prepare a second one while the last prepared version is still untagged — on a push it says so and stops, and a manual run fails. That covers the approval window: a `fix` merged while **Release Tag** waits would otherwise propose a duplicate PR for the version already on its way out. It also latches when an approval is _rejected_, since that leaves a prepared version that never gets a tag; clear it with the manual fallback below, which both publishes that release and satisfies the check. - -And if the version moves while a release PR is open — a `feat` landing on top of a pending patch — the next run opens a PR for the new version and closes the superseded one. **Release Tag** reads the version it mints from the `release/*` branch name, so leaving the stale PR open would leave a merge path that tags the wrong version. - -Manual fallback: - -1. Tag and push by hand from `main`: - - `git switch main && git pull` - - `git tag -a vX.Y.Z -m vX.Y.Z -s` - - `git push origin vX.Y.Z` A tag pushed this way triggers **Release Publish** directly on tag push. -1. If needed, run **Release Publish** via `workflow_dispatch` with an existing `tag`. - -> **Note:** Release Tag dispatches Release Publish explicitly rather than relying on the tag push, because a tag pushed with `GITHUB_TOKEN` does not trigger `on: push: tags`. - -Signing model: - -- Sigstore keyless signatures are generated in CI for every release artifact. -- GPG detached signatures are also generated for compatibility. -- Release tags are annotated and GPG-signed. When **Release Tag** creates the tag, it is signed with the CI release key rather than a maintainer's personal key. The protected `release` environment is what keeps that key from being usable by anyone who merges a `release/*` PR: the tagging job waits for maintainer approval before it runs. -- Required repository secrets for GPG signing in CI: - - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) - - `RELEASE_GPG_PASSPHRASE` (passphrase for the private key) - -Without both secrets, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for them: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. - -##### One-time release key setup - -Run these locally as a maintainer; never paste private key material into an issue, a PR, or a chat transcript. - -```bash -# 1. Pick a passphrase and generate a dedicated release key (not your personal key). -PASSPHRASE='' -gpg --batch --passphrase "$PASSPHRASE" \ - --quick-generate-key 'custom-commit-hooks release ' rsa4096 sign 2y - -# 2. Note the fingerprint of the key you just made. -gpg --list-secret-keys --keyid-format=long - -# 3. Export the private key, ASCII-armored. -gpg --armor --export-secret-keys >release-key.asc - -# 4. Store both secrets on the repository. -gh secret set RELEASE_GPG_PRIVATE_KEY /dev/null 2>&1; then - shred -u release-key.asc -else - rm -f release-key.asc -fi - -# 6. Optional: register the public key so signed tags display as Verified on GitHub. -# Paste the output at https://github.com/settings/gpg/new -gpg --armor --export -``` - -Confirm both secrets landed with `gh secret list`. The key expires in two years; rotate by repeating these steps. - -Neither removal above guarantees the bytes are gone: copy-on-write filesystems and SSD wear levelling can leave the export recoverable. Treat the passphrase as the real protection for that file, and prefer a passphrase over an empty one for exactly this reason. - -Verification examples: - -```bash -# Sigstore -cosign verify-blob --signature artifact.sig --certificate artifact.pem --certificate-oidc-issuer https://token.actions.githubusercontent.com --certificate-identity-regexp 'https://github.com/.+' artifact - -# GPG -gpg --verify artifact.asc artifact -``` +Releases are cut by maintainers and are automated end to end except for a single approval step. See [RELEASING](RELEASING.md) for the release flow, the signing model, one-time release key setup, and verification commands. ### Recommended VSCode extensions diff --git a/README.md b/README.md index 2f26f8f..8d1e7bc 100644 --- a/README.md +++ b/README.md @@ -92,4 +92,4 @@ chore: merge branch 'feature/new-api' into main ## Documentation: [![Ask DeepWiki](https://deepwiki.com/badge.svg)](https://deepwiki.com/michen00/custom-commit-hooks) -Releases are signed (Sigstore + GPG). For verification commands and release process, see [CONTRIBUTING](CONTRIBUTING.md#creating-a-release). +Releases are signed (Sigstore + GPG). For verification commands and release process, see [RELEASING](RELEASING.md). diff --git a/RELEASING.md b/RELEASING.md new file mode 100644 index 0000000..060660c --- /dev/null +++ b/RELEASING.md @@ -0,0 +1,116 @@ + + +# Releasing + +Releases are cut by maintainers. Contributors do not need anything here — see [CONTRIBUTING](CONTRIBUTING.md) instead. + +The flow is automated end to end except for a single approval, which is deliberate: that approval is the only thing standing between a merged pull request and a GPG-signed tag minted with the CI release key. + + + +## Table of Contents + +- [Default flow](#default-flow) +- [Checking release status](#checking-release-status) +- [Why Release PR sometimes does nothing](#why-release-pr-sometimes-does-nothing) +- [Manual fallback](#manual-fallback) +- [Signing model](#signing-model) + - [One-time release key setup](#one-time-release-key-setup) +- [Verifying a release](#verifying-a-release) + +## Default flow + +1. **Release PR** (`.github/workflows/release-pr.yml`) opens the release PR by itself when a commit worth releasing lands on `main`. Every conventional type bumps at least the patch version, so the version cannot decide that on its own; the workflow gates on the group `cliff.toml` parsed each commit into — features, fixes, performance and reverts, plus anything marked breaking. A `chore`, `docs`, `build`, `ci`, `test`, `refactor` or `style` merge — the weekly hook autoupdate and Dependabot among them — rides along in the next release without proposing one. Run the workflow by hand, or `make release-pr`, to pin the version or to release a batch containing none of those types; a manual run skips the worthiness gate. Leave `version` empty to derive it via `git cliff --bumped-version`, or pass `X.Y.Z` / `vX.Y.Z`. +1. Review and merge the generated PR (`chore(release): prepare vX.Y.Z`). +1. **Approve the release.** This is the only manual step, and nothing ships until it happens. Run `make release-approve`, which finds the waiting run, shows the tag it will mint and the commit that tag will point at, and asks you to confirm. You can also approve from the run page on GitHub, or from the PR's checks tab. Rejecting the approval leaves no tag behind. +1. **Release Tag** (`.github/workflows/release-tag.yml`) runs on merge of a `release/*` branch, gated on the protected `release` environment. Once approved it creates a GPG-signed annotated tag, pushes it, and dispatches **Release Publish**. +1. **Release Publish** (`.github/workflows/release-publish.yml`) builds and uploads signed artifacts to the GitHub release. + +A release that is never approved simply waits. There is no timeout and no reminder, so a merged release PR can sit indefinitely — `make release-status` is the fastest way to find out whether that is what has happened. + +That approval is the only one a normal release needs. **Release Publish** also declares the `release` environment, but when **Release Tag** dispatches it the deployment is created by `github-actions[bot]` and the reviewer rule is skipped, so it proceeds without a second prompt. The declaration still gates a Release Publish run dispatched by hand, which is the manual fallback path. Treat the Release Tag approval as the release decision — no tag means no publish. + +## Checking release status + +```bash +make release-status # latest tag, any open release PR, any run awaiting approval +make release-approve # approve the waiting run after confirming what it will tag +``` + +Both read the repository through `gh`, so they need an authenticated GitHub CLI and no other local state. `make release-approve` refuses to do anything when no run is waiting, rather than guessing at which run you meant. + +## Why Release PR sometimes does nothing + +Two guards follow from the tag being what marks a release finished. + +**Release PR** refuses to prepare a second release while the last prepared version is still untagged — on a push it says so and stops, and a manual run fails. That covers the approval window: a `fix` merged while **Release Tag** waits would otherwise propose a duplicate PR for the version already on its way out. It also latches when an approval is _rejected_, since that leaves a prepared version that never gets a tag; clear it with the manual fallback below, which both publishes that release and satisfies the check. + +And if the version moves while a release PR is open — a `feat` landing on top of a pending patch — the next run opens a PR for the new version and closes the superseded one. **Release Tag** reads the version it mints from the `release/*` branch name, so leaving the stale PR open would leave a merge path that tags the wrong version. + +## Manual fallback + +1. Tag and push by hand from `main`: + - `git switch main && git pull` + - `git tag -a vX.Y.Z -m vX.Y.Z -s` + - `git push origin vX.Y.Z` A tag pushed this way triggers **Release Publish** directly on tag push. +1. If needed, run **Release Publish** via `workflow_dispatch` with an existing `tag`. + +> **Note:** Release Tag dispatches Release Publish explicitly rather than relying on the tag push, because a tag pushed with `GITHUB_TOKEN` does not trigger `on: push: tags`. + +## Signing model + +- Sigstore keyless signatures are generated in CI for every release artifact. +- GPG detached signatures are also generated for compatibility. +- Release tags are annotated and GPG-signed. When **Release Tag** creates the tag, it is signed with the CI release key rather than a maintainer's personal key. The protected `release` environment is what keeps that key from being usable by anyone who merges a `release/*` PR: the tagging job waits for maintainer approval before it runs. +- Required repository secrets for GPG signing in CI: + - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) + - `RELEASE_GPG_PASSPHRASE` (passphrase for the private key) + +Without both secrets, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for them: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. + +### One-time release key setup + +Run these locally as a maintainer; never paste private key material into an issue, a PR, or a chat transcript. + +```bash +# 1. Pick a passphrase and generate a dedicated release key (not your personal key). +PASSPHRASE='' +gpg --batch --passphrase "$PASSPHRASE" \ + --quick-generate-key 'custom-commit-hooks release ' rsa4096 sign 2y + +# 2. Note the fingerprint of the key you just made. +gpg --list-secret-keys --keyid-format=long + +# 3. Export the private key, ASCII-armored. +gpg --armor --export-secret-keys >release-key.asc + +# 4. Store both secrets on the repository. +gh secret set RELEASE_GPG_PRIVATE_KEY /dev/null 2>&1; then + shred -u release-key.asc +else + rm -f release-key.asc +fi + +# 6. Optional: register the public key so signed tags display as Verified on GitHub. +# Paste the output at https://github.com/settings/gpg/new +gpg --armor --export +``` + +Confirm both secrets landed with `gh secret list`. The key expires in two years; rotate by repeating these steps. + +Neither removal above guarantees the bytes are gone: copy-on-write filesystems and SSD wear levelling can leave the export recoverable. Treat the passphrase as the real protection for that file, and prefer a passphrase over an empty one for exactly this reason. + +## Verifying a release + +```bash +# Sigstore +cosign verify-blob --signature artifact.sig --certificate artifact.pem --certificate-oidc-issuer https://token.actions.githubusercontent.com --certificate-identity-regexp 'https://github.com/.+' artifact + +# GPG +gpg --verify artifact.asc artifact +``` From 7bc2f1cc3c9475abd27f05747b1952b59e1686c7 Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:19:16 -0700 Subject: [PATCH 3/6] docs(CONTRIBUTING.md): fix code of conduct link The link was relative, so it resolved to a file in this repository that does not exist. The code of conduct is an account-level community health file in michen00/.github: GitHub surfaces it on the community profile, but a relative link from here still 404s, so point at where it lives. Co-Authored-By: Claude Opus 5 (1M context) --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e0e8c9..48c49f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,7 @@ Welcome! We're happy to have you here. All types of contributions are encouraged See the [Table of Contents](#table-of-contents) for different ways to help and details about how this project handles them. Please make sure to read the relevant section before making your contribution. It will make it a lot easier for us maintainers and smooth out the experience for all involved. We look forward to your contributions! -The project has defined a [code of conduct](CODE_OF_CONDUCT.md) to ensure a welcoming and friendly environment. Please adhere to it in all interactions. +The project has defined a [code of conduct](https://github.com/michen00/.github/blob/main/CODE_OF_CONDUCT.md) to ensure a welcoming and friendly environment. Please adhere to it in all interactions. From cfa626095e763359937add24609a1a6b8743d5be Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:21:39 -0700 Subject: [PATCH 4/6] docs: correct the GPG secret requirements RELEASING said both RELEASE_GPG_PRIVATE_KEY and RELEASE_GPG_PASSPHRASE were required, and that Release Tag and Release Publish fail at the GPG import step without both. `gh secret list` shows only the private key, and v0.1.2 published forty minutes ago, so that was wrong in the direction that costs the most: it invites someone to "fix" a missing secret by setting it to an empty string. The release key has no passphrase. sign-artifacts.sh already branches on `[ -n "${GPG_PASSPHRASE-}" ]` and the import action accepts an empty passphrase, so omitting the secret is the supported path, not an oversight. Say which secret is actually required, say why the other is absent, and keep the setup snippet's passphrase advice for anyone generating a new key. Co-Authored-By: Claude Opus 5 (1M context) --- RELEASING.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index 060660c..207183a 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -62,11 +62,13 @@ And if the version moves while a release PR is open — a `feat` landing on top - Sigstore keyless signatures are generated in CI for every release artifact. - GPG detached signatures are also generated for compatibility. - Release tags are annotated and GPG-signed. When **Release Tag** creates the tag, it is signed with the CI release key rather than a maintainer's personal key. The protected `release` environment is what keeps that key from being usable by anyone who merges a `release/*` PR: the tagging job waits for maintainer approval before it runs. -- Required repository secrets for GPG signing in CI: - - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) - - `RELEASE_GPG_PASSPHRASE` (passphrase for the private key) +- Repository secrets for GPG signing in CI: + - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) — required. + - `RELEASE_GPG_PASSPHRASE` (passphrase for that key) — only when the key has one. -Without both secrets, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for them: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. +This repository's release key has no passphrase, so `RELEASE_GPG_PASSPHRASE` is deliberately absent and `gh secret list` shows only the private key. `scripts/release/sign-artifacts.sh` branches on whether the passphrase is set, and the import action accepts an empty one, so this is a supported path rather than a misconfiguration — v0.1.0 through v0.1.2 were all signed this way. Do not "fix" the missing secret by setting it to an empty string; omitting it is what the branch tests for. + +Without the private key, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for it: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. ### One-time release key setup @@ -84,7 +86,8 @@ gpg --list-secret-keys --keyid-format=long # 3. Export the private key, ASCII-armored. gpg --armor --export-secret-keys >release-key.asc -# 4. Store both secrets on the repository. +# 4. Store the key on the repository. Set the passphrase secret only if the +# key has a passphrase -- leave it unset otherwise, rather than empty. gh secret set RELEASE_GPG_PRIVATE_KEY ``` -Confirm both secrets landed with `gh secret list`. The key expires in two years; rotate by repeating these steps. +Confirm the secrets landed with `gh secret list`. The key expires in two years; rotate by repeating these steps. Neither removal above guarantees the bytes are gone: copy-on-write filesystems and SSD wear levelling can leave the export recoverable. Treat the passphrase as the real protection for that file, and prefer a passphrase over an empty one for exactly this reason. From 4d8edb7a27bbb9fa04167292d4a2175e1ab25f19 Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:34:13 -0700 Subject: [PATCH 5/6] fix(release): show the commit that gets tagged Qodo reviewed 7bc2f1c and raised seven findings. Four are real. The high one is a genuine bug. approve.sh printed the waiting run's headSha as the commit to be tagged, but release-tag.yml checks out `pull_request.merge_commit_sha`, and this repo squash-merges, so the two always differ. Verified against the release that just shipped: the run reported 3179e83 while v0.1.2 landed on 62c098a. A prompt whose whole job is showing what you are approving was naming a commit the tag never points at. It now resolves the merged PR for the release branch and shows that merge commit, refusing to approve when it cannot be resolved rather than falling back to the branch tip. The test could reach the real GitHub CLI. Setup was unchecked, so a failed mktemp or chmod would leave `gh` resolving to the caller's authenticated binary and the --yes cases would POST an approval against whatever release was actually waiting. Setup is now fail-fast and ends by asserting the stub is what `gh` resolves to. `gh release view` exits non-zero both when no release exists and when the call fails, so `|| echo none` reported an outage as an empty repository; `release list` separates them. `gh pr list` used the default 30-item page, so a release PR behind thirty open PRs read as absent. The approval path now exits 0 explicitly. Three findings are declined, all style claims contradicted by the repo's own conventions: the bash shebang, BASH_SOURCE, `local` and `[[ ]]` match every other file in tests/; PASSED/FAILED match tests/test-parse-version.sh; and that same file ends on a bare `[ "$FAILED" -eq 0 ]`. CLAUDE.md's POSIX rule covers scripts/, which shellcheck --shell=sh enforces separately. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/release/approve.sh | 37 +++++++++-- tests/test-release-approve.sh | 122 ++++++++++++++++++++++++++++------ 2 files changed, 133 insertions(+), 26 deletions(-) diff --git a/scripts/release/approve.sh b/scripts/release/approve.sh index d178c14..38a0785 100755 --- a/scripts/release/approve.sh +++ b/scripts/release/approve.sh @@ -68,12 +68,22 @@ waiting="$(gh run list --workflow="$WORKFLOW" --status waiting --limit 1 \ run_id="$(printf '%s' "$waiting" | jq -r '.[0].databaseId // empty')" if [ "$mode" = "status" ]; then - latest="$(gh release view --json tagName --jq .tagName 2>/dev/null || echo "none")" + # `gh release view` exits non-zero both when no release exists and when the + # call fails, so a bare `|| echo none` reports an outage as an empty + # repository. `list` returns `[]` for the first and still fails for the + # second, which is the distinction a status command owes the reader. + releases="$(gh release list --limit 1 --json tagName)" || + die "Could not read releases." + latest="$(printf '%s' "$releases" | jq -r '.[0].tagName // "none"')" echo "Latest release: $latest" - open_pr="$(gh pr list --state open --json number,title,headRefName \ + # --limit because the default page is 30: a release PR sitting behind thirty + # other open PRs would otherwise be reported as absent. + open_pr="$(gh pr list --state open --limit 100 \ + --json number,title,headRefName \ --jq '[.[] | select(.headRefName | startswith("release/"))] - | map("#\(.number) \(.title)") | join(", ") // empty')" + | map("#\(.number) \(.title)") | join(", ") // empty')" || + die "Could not list open pull requests." echo "Open release PR: ${open_pr:-none}" if [ -n "$run_id" ]; then @@ -93,12 +103,27 @@ fi die "No $WORKFLOW run is awaiting approval. Nothing to release." branch="$(printf '%s' "$waiting" | jq -r '.[0].headBranch')" -head_sha="$(printf '%s' "$waiting" | jq -r '.[0].headSha')" run_url="$(printf '%s' "$waiting" | jq -r '.[0].url')" tag="$("$SCRIPT_DIR/parse-version.sh" "${branch#release/}" --require-v)" || die "Branch '$branch' does not encode a vX.Y.Z tag." +# Deliberately not the run's headSha. That is the release branch tip, but +# release-tag.yml checks out `pull_request.merge_commit_sha` and tags whatever +# it finds there -- and this repository squash-merges, so the two always +# differ. v0.1.2's run reported 3179e83 while the tag landed on 62c098a. +# Naming a commit the tag will never point at is the one thing a confirmation +# prompt must not do. +pr_json="$(gh pr list --state merged --head "$branch" --limit 1 \ + --json number,mergeCommit)" || + die "Could not look up the merged pull request for '$branch'." + +pr_number="$(printf '%s' "$pr_json" | jq -r '.[0].number // empty')" +merge_sha="$(printf '%s' "$pr_json" | jq -r '.[0].mergeCommit.oid // empty')" + +[ -n "$merge_sha" ] || + die "No merged pull request with a merge commit found for '$branch'." + pending="$(gh api "repos/$repo/actions/runs/$run_id/pending_deployments")" || die "Could not read pending deployments for run $run_id." @@ -116,7 +141,8 @@ env_names="$(printf '%s' "$pending" | jq -r '[.[].environment.name] | join(", ") cat <&2 + exit 1 +} + +STUB_DIR="$(mktemp -d)" || setup_failed "mktemp -d" trap 'rm -rf "$STUB_DIR"' EXIT -cat >"$STUB_DIR/gh" <<'STUB' +cat >"$STUB_DIR/gh" <<'STUB' || setup_failed "writing the gh stub" #!/usr/bin/env bash case "$1" in run) printf '%s\n' "${GH_STUB_WAITING:-[]}" ;; repo) printf '%s\n' "${GH_STUB_REPO:-michen00/custom-commit-hooks}" ;; -release) printf '%s\n' "${GH_STUB_LATEST:-v0.1.2}" ;; -pr) printf '%s\n' "${GH_STUB_OPENPR:-}" ;; +release) + [ -n "${GH_STUB_RELEASE_FAIL:-}" ] && exit 1 + printf '%s\n' "${GH_STUB_RELEASES:-[]}" + ;; +pr) + for arg in "$@"; do + if [ "$arg" = "merged" ]; then + printf '%s\n' "${GH_STUB_MERGEDPR:-[]}" + exit 0 + fi + done + [ -n "${GH_STUB_OPENPR_FAIL:-}" ] && exit 1 + printf '%s\n' "${GH_STUB_OPENPR:-}" + ;; api) for arg in "$@"; do if [ "$arg" = "POST" ]; then @@ -55,9 +77,23 @@ api) esac exit 0 STUB -chmod +x "$STUB_DIR/gh" -WAITING_RUN='[{"databaseId":42,"headBranch":"release/v9.9.9","headSha":"abc123","url":"https://example.invalid/run/42"}]' +[ -s "$STUB_DIR/gh" ] || setup_failed "gh stub is empty" +chmod +x "$STUB_DIR/gh" || setup_failed "chmod +x on the gh stub" + +# The load-bearing guard. Anything short of the stub resolving first means the +# suite would be driving the real GitHub CLI. +resolved="$(PATH="$STUB_DIR:$PATH" command -v gh)" +[ "$resolved" = "$STUB_DIR/gh" ] || + setup_failed "gh resolves to '$resolved', not the stub" + +# Distinct sentinels: the run reports the release branch tip, but release-tag.yml +# tags the squash merge commit. The summary must name the second, never the first. +BRANCH_TIP="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +MERGE_SHA="bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + +WAITING_RUN="[{\"databaseId\":42,\"headBranch\":\"release/v9.9.9\",\"headSha\":\"$BRANCH_TIP\",\"url\":\"https://example.invalid/run/42\"}]" +MERGED_PR="[{\"number\":83,\"mergeCommit\":{\"oid\":\"$MERGE_SHA\"}}]" PENDING_OK='[{"environment":{"id":7,"name":"release"},"current_user_can_approve":true}]' PENDING_DENIED='[{"environment":{"id":7,"name":"release"},"current_user_can_approve":false}]' @@ -81,7 +117,7 @@ fi # --- nothing waiting --------------------------------------------------------- -log="$STUB_DIR/post1.log" +log="$STUB_DIR/post.log" : >"$log" out="$(GH_STUB_WAITING='[]' run_approve "$log")" status=$? @@ -95,19 +131,42 @@ else pass "no waiting run exits 1 and approves nothing" fi -out="$(GH_STUB_WAITING='[]' run_approve "$log" --status)" +# --- status mode ------------------------------------------------------------- + +out="$(GH_STUB_WAITING='[]' GH_STUB_RELEASES='[{"tagName":"v0.1.2"}]' \ + run_approve "$log" --status)" status=$? if [ "$status" -ne 0 ]; then fail "--status exits 0 when idle" "Exited $status, expected 0" elif [[ "$out" != *"Awaiting approval: none"* ]]; then fail "--status reports an idle repository" "Got: $out" +elif [[ "$out" != *"v0.1.2"* ]]; then + fail "--status reports the latest release" "Got: $out" else pass "--status exits 0 and reports nothing awaiting approval" fi -# --- status with a run in flight --------------------------------------------- +# An empty release list is a fact about the repository, not a failure. +out="$(GH_STUB_WAITING='[]' GH_STUB_RELEASES='[]' run_approve "$log" --status)" +status=$? +if [ "$status" -eq 0 ] && [[ "$out" == *"Latest release: none"* ]]; then + pass "--status reports 'none' for a repository with no releases" +else + fail "--status handles an empty release list" "Exited $status. Got: $out" +fi -out="$(GH_STUB_WAITING="$WAITING_RUN" run_approve "$log" --status)" +# A failed lookup is not the same fact, and must not read as one. +out="$(GH_STUB_WAITING='[]' GH_STUB_RELEASE_FAIL=1 run_approve "$log" --status)" +status=$? +if [ "$status" -eq 1 ] && [[ "$out" != *"Latest release: none"* ]]; then + pass "--status fails loudly when the release lookup errors" +else + fail "--status distinguishes lookup failure from no releases" \ + "Exited $status. Got: $out" +fi + +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_RELEASES='[]' \ + run_approve "$log" --status)" if [[ "$out" == *"v9.9.9"* && "$out" == *"https://example.invalid/run/42"* ]]; then pass "--status names the pending version and links the run" else @@ -123,8 +182,8 @@ fi # --- refusing to approve ----------------------------------------------------- : >"$log" -out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_OK" \ - run_approve "$log")" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_MERGEDPR="$MERGED_PR" \ + GH_STUB_PENDING="$PENDING_OK" run_approve "$log")" status=$? if [ "$status" -ne 1 ]; then fail "non-tty without --yes exits 1" "Exited $status, expected 1" @@ -137,18 +196,18 @@ else fi : >"$log" -out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_DENIED" \ - run_approve "$log" --yes)" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_MERGEDPR="$MERGED_PR" \ + GH_STUB_PENDING="$PENDING_DENIED" run_approve "$log" --yes)" status=$? if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then pass "a non-approver is rejected without a POST" else - fail "a non-approver is rejected" "Exited $status; log $([ -s "$log" ] && echo "non-empty" || echo "empty")" + fail "a non-approver is rejected" "Exited $status" fi : >"$log" -out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING='[]' \ - run_approve "$log" --yes)" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_MERGEDPR="$MERGED_PR" \ + GH_STUB_PENDING='[]' run_approve "$log" --yes)" status=$? if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then pass "a waiting run with no pending deployment is rejected" @@ -156,12 +215,24 @@ else fail "no pending deployment is rejected" "Exited $status" fi +# Without a resolvable merge commit there is nothing honest to show, so the +# prompt must not fall back to the branch tip. +: >"$log" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_MERGEDPR='[]' \ + GH_STUB_PENDING="$PENDING_OK" run_approve "$log" --yes)" +status=$? +if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then + pass "an unresolvable merge commit is rejected without a POST" +else + fail "unresolvable merge commit is rejected" "Exited $status. Got: $out" +fi + # A branch name reaches this script from a merged pull request, so it is # attacker-influenced in the same way the workflows' inputs are. : >"$log" INJECT='[{"databaseId":42,"headBranch":"release/v1.0.0; rm -rf /","headSha":"abc","url":"u"}]' -out="$(GH_STUB_WAITING="$INJECT" GH_STUB_PENDING="$PENDING_OK" \ - run_approve "$log" --yes)" +out="$(GH_STUB_WAITING="$INJECT" GH_STUB_MERGEDPR="$MERGED_PR" \ + GH_STUB_PENDING="$PENDING_OK" run_approve "$log" --yes)" status=$? if [ "$status" -eq 1 ] && [ ! -s "$log" ]; then pass "a branch that is not a clean vX.Y.Z is rejected" @@ -172,8 +243,8 @@ fi # --- the approving path ------------------------------------------------------ : >"$log" -out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_PENDING="$PENDING_OK" \ - run_approve "$log" --yes)" +out="$(GH_STUB_WAITING="$WAITING_RUN" GH_STUB_MERGEDPR="$MERGED_PR" \ + GH_STUB_PENDING="$PENDING_OK" run_approve "$log" --yes)" status=$? if [ "$status" -ne 0 ]; then fail "--yes approves" "Exited $status, expected 0. Output: $out" @@ -195,6 +266,15 @@ else fail "the approval summary names the tag" "Got: $out" fi +# The regression that matters: release-tag.yml tags merge_commit_sha, so showing +# the run's headSha would name a commit the tag never points at. +if [[ "$out" == *"$MERGE_SHA"* && "$out" != *"$BRANCH_TIP"* ]]; then + pass "the summary names the merge commit, not the branch tip" +else + fail "the summary names the merge commit" \ + "Expected $MERGE_SHA and not $BRANCH_TIP. Got: $out" +fi + printf "\nResults: ${GREEN}%d passed${NC}, ${RED}%d failed${NC}\n" "$PASSED" "$FAILED" [ "$FAILED" -eq 0 ] From 3a1cc5cfec56ac65db2aa147e30bc926065fb80a Mon Sep 17 00:00:00 2001 From: Michael I Chen Date: Tue, 15 Sep 2026 21:42:36 -0700 Subject: [PATCH 6/6] fix: correct the passphrase-secret mechanism MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo is right, and this is a claim I introduced one commit ago while fixing a different wrong claim about the same secret. RELEASING said omitting RELEASE_GPG_PASSPHRASE "is what the branch tests for". It is not. Both workflows pass the secret through unconditionally, an absent secret expands to an empty string, and sign-artifacts.sh branches on `[ -n "${GPG_PASSPHRASE-}" ]` — empty versus nonempty, never absent versus present. An empty secret and no secret sign identically. The advice to omit it still holds, but the reason had to change: not that signing can tell the difference, but that a secret which exists and means nothing invites someone to later fill it with a passphrase the key does not have. The setup snippet now guards the `gh secret set` on a non-empty passphrase instead of telling the reader to set it conditionally while unconditionally setting it. Also captured the exit status in the status-mode test that asserted on output alone, so a regression that prints the right text and then fails cannot pass. Co-Authored-By: Claude Opus 5 (1M context) --- RELEASING.md | 6 ++++-- tests/test-release-approve.sh | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index 207183a..97825d7 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -66,7 +66,7 @@ And if the version moves while a release PR is open — a `feat` landing on top - `RELEASE_GPG_PRIVATE_KEY` (ASCII-armored private key) — required. - `RELEASE_GPG_PASSPHRASE` (passphrase for that key) — only when the key has one. -This repository's release key has no passphrase, so `RELEASE_GPG_PASSPHRASE` is deliberately absent and `gh secret list` shows only the private key. `scripts/release/sign-artifacts.sh` branches on whether the passphrase is set, and the import action accepts an empty one, so this is a supported path rather than a misconfiguration — v0.1.0 through v0.1.2 were all signed this way. Do not "fix" the missing secret by setting it to an empty string; omitting it is what the branch tests for. +This repository's release key has no passphrase, so `RELEASE_GPG_PASSPHRASE` is deliberately absent and `gh secret list` shows only the private key. Both workflows pass the secret through unconditionally and an absent secret expands to an empty string, so `scripts/release/sign-artifacts.sh` — which branches on `[ -n "${GPG_PASSPHRASE-}" ]` — takes its no-passphrase path either way, and the import action accepts an empty passphrase. v0.1.0 through v0.1.2 were all signed that way. Setting the secret to an empty string would sign identically rather than fix anything; it is still not worth creating, because a secret that exists and means nothing invites someone to later fill it with a passphrase the key does not have. Without the private key, **Release Tag** and **Release Publish** fail at the GPG import step, so no tag is created and no artifacts are published. A GitHub App token does not substitute for it: a token authenticates git and API calls but cannot produce a GPG signature, and GitHub signs only commits it creates via the API, never annotated tag objects. @@ -89,7 +89,9 @@ gpg --armor --export-secret-keys >release-key.asc # 4. Store the key on the repository. Set the passphrase secret only if the # key has a passphrase -- leave it unset otherwise, rather than empty. gh secret set RELEASE_GPG_PRIVATE_KEY