From be62a916a2c0147666648a7e456e81c4224e96ed Mon Sep 17 00:00:00 2001 From: Guillaume Dequenne Date: Tue, 13 Oct 2026 10:08:47 +0200 Subject: [PATCH] Fix build-number ref creation for PR merge commits --- README.md | 4 +- get-build-number/get_build_number.sh | 22 +++++++-- spec/get_build_number_spec.sh | 73 +++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 39621176..3d7db2a5 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,9 @@ No inputs are required for this action. ### Git References This action coordinates purely through Git references on the repository - no external state, cache, or database. Each reference is created -pointing at `$GITHUB_SHA` (the commit that triggered the claim); that target is never read back - only the reference's existence matters +pointing at the repository's default branch HEAD, resolved once before acquiring the lock. Using the existing default branch head avoids +GitHub's additional workflow-write restrictions on some PR merge commits; `contents: write` remains sufficient. The target is never read +back - only the reference's existence matters (for `build-number` and `build-number-lock`), or its name, which encodes the claimed number (for `build-runs`). | Reference | Lifetime | Purpose | diff --git a/get-build-number/get_build_number.sh b/get-build-number/get_build_number.sh index 13155b71..eedb57e9 100755 --- a/get-build-number/get_build_number.sh +++ b/get-build-number/get_build_number.sh @@ -8,13 +8,15 @@ # refs/build-number-lock: exclusive, transient, repository-wide lock serializing every new claim, not just those within one run. # Released immediately after use (success or failure) via the trap below, not held for a run's lifetime. # -# Every ref points at $GITHUB_SHA purely because creation requires some valid target; that target is never read back. +# Every ref points at the default branch HEAD purely because creation requires some valid target; that target is never read back. +# PR merge commits can require workflows:write when used as ref targets, even with contents:write. The existing default branch +# head avoids that restriction without changing the claim/lock protocol or requiring a more privileged token. # All ref reads/writes use the ambient GITHUB_TOKEN. LEGACY_PROPERTY_TOKEN (from Vault) is only used to read the legacy # build_number property during migration - see README. set -euo pipefail -: "${GITHUB_REPOSITORY:?}" "${GITHUB_SHA:?}" "${GITHUB_RUN_ID:?}" +: "${GITHUB_REPOSITORY:?}" "${GITHUB_RUN_ID:?}" GH_API_VERSION_HEADER="X-GitHub-Api-Version: 2022-11-28" BUILD_NUMBER_FILE="${BUILD_NUMBER_FILE:-.build_number.txt}" @@ -31,7 +33,7 @@ RETRY_INTERVAL_SECONDS="${RETRY_INTERVAL_SECONDS:-1}" # between internal retries create_ref() { local ref="$1" - gh api --method POST -H "$GH_API_VERSION_HEADER" "$REFS_API_URL" -f "ref=refs/${ref}" -f "sha=${GITHUB_SHA}" 2>&1 + gh api --method POST -H "$GH_API_VERSION_HEADER" "$REFS_API_URL" -f "ref=refs/${ref}" -f "sha=${REF_TARGET_SHA}" 2>&1 } delete_ref() { @@ -93,9 +95,23 @@ trap release_lock EXIT echo "::group::Get build number" attempt=1 +REF_TARGET_SHA="" while true; do try_reuse_existing_claim && { echo "::endgroup::" && exit 0; } + # Resolve once, before acquiring the lock. Existing run markers remain a read-only fast path. + # HEAD resolves to the repository's default branch, independently of the workflow's trigger/ref. + if [[ -z "$REF_TARGET_SHA" ]]; then + if ! REF_TARGET_SHA=$(gh api -H "$GH_API_VERSION_HEADER" "repos/${GITHUB_REPOSITORY}/commits/HEAD" --jq '.sha // empty'); then + echo "::error title=Build number claim failed::Could not resolve the repository's default branch HEAD." >&2 + exit 1 + fi + if [[ -z "$REF_TARGET_SHA" ]]; then + echo "::error title=Build number claim failed::The repository's default branch HEAD has no commit SHA." >&2 + exit 1 + fi + fi + RESPONSE=$(create_ref "$NUMBER_LOCK_REF") && LOCK_STATUS=0 || LOCK_STATUS=$? if [[ "$LOCK_STATUS" -eq 0 ]]; then HELD_LOCK=1 diff --git a/spec/get_build_number_spec.sh b/spec/get_build_number_spec.sh index 86e70bfc..75136975 100755 --- a/spec/get_build_number_spec.sh +++ b/spec/get_build_number_spec.sh @@ -27,7 +27,7 @@ Describe 'get_build_number.sh' Mock gh if [[ "$*" == *"matching-refs/build-runs/"* ]]; then echo "refs/build-runs/${GITHUB_RUN_ID}/5" - elif [[ "$*" == *"build-number-lock"* ]]; then + elif [[ "$*" == *"build-number-lock"* || "$*" == *"commits/HEAD"* ]]; then echo "1" >>"$GH_LOCK_CALLS_FILE" echo "gh $*" else @@ -41,6 +41,77 @@ Describe 'get_build_number.sh' The path "$GH_LOCK_CALLS_FILE" should not be file End + It 'should anchor the lock, claim and run marker to the default branch head rather than the PR merge SHA' + export GH_POST_CALLS_FILE="${TEMP_DIR}/gh_post_calls_default_head.txt" + export DEFAULT_HEAD_SHA="0123456789abcdef0123456789abcdef01234567" + rm -f "$GH_POST_CALLS_FILE" + Mock gh + if [[ "$*" == *"commits/HEAD"* ]]; then + echo "$DEFAULT_HEAD_SHA" + elif [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"matching-refs/build-number/"* ]]; then + echo 'refs/build-number/5' + elif [[ "$*" == *"--method POST"* ]]; then + echo "$*" >>"$GH_POST_CALLS_FILE" + if [[ "$*" != *"sha=${DEFAULT_HEAD_SHA}"* || "$*" == *"sha=${GITHUB_SHA}"* ]]; then + echo '{"message":"Resource not accessible by integration"}' >&2 + exit 1 + fi + echo "gh $*" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be success + The output should include "Claimed build number 6" + The contents of file "$BUILD_NUMBER_FILE" should equal "6" + The contents of file "$GH_POST_CALLS_FILE" should include "ref=refs/build-number-lock/global" + The contents of file "$GH_POST_CALLS_FILE" should include "ref=refs/build-number/6" + The contents of file "$GH_POST_CALLS_FILE" should include "ref=refs/build-runs/${GITHUB_RUN_ID}/6" + End + + It 'should fail before writing refs if the default branch head cannot be resolved' + export GH_POST_CALLS_FILE="${TEMP_DIR}/gh_post_calls_lookup_failure.txt" + rm -f "$GH_POST_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* ]]; then + echo '' + elif [[ "$*" == *"commits/HEAD"* ]]; then + exit 1 + elif [[ "$*" == *"--method POST"* ]]; then + echo "$*" >>"$GH_POST_CALLS_FILE" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "Could not resolve the repository's default branch HEAD" + The path "$GH_POST_CALLS_FILE" should not be file + End + + It 'should fail before writing refs if the default branch has no commit SHA' + export GH_POST_CALLS_FILE="${TEMP_DIR}/gh_post_calls_empty_head.txt" + rm -f "$GH_POST_CALLS_FILE" + Mock gh + if [[ "$*" == *"matching-refs/build-runs/"* || "$*" == *"commits/HEAD"* ]]; then + echo '' + elif [[ "$*" == *"--method POST"* ]]; then + echo "$*" >>"$GH_POST_CALLS_FILE" + else + echo "gh $*" + fi + End + When run script get-build-number/get_build_number.sh + The status should be failure + The output should include "::group::Get build number" + The stderr should include "default branch HEAD has no commit SHA" + The path "$GH_POST_CALLS_FILE" should not be file + End + It 'should claim build number 1 and warn when there are no build-number refs and no migration token' Mock gh if [[ "$*" == *"matching-refs/build-runs/"* ]]; then