fix(release): refuse to overwrite an unrelated hand-placed v1
release-v1.sh's push_leased() only detected a lost lease after a push
was *rejected* -- but force-with-lease compares the remote ref's raw
value against the caller's expected value, not ancestry. If v1 already
sat on a hand-placed, unrelated commit when push_leased() was first
called (no race, nobody moves it mid-call), the very first push found
the ref exactly where it expected, succeeded outright, and silently
overwrote the unrelated v1 with <sha> -- skipping every ancestry check
the function has, since those only run after a rejection.
Fix: before the first push attempt, check whether the caller's
`expect` is neither an ancestor of `sha` (the ordinary stale-v1 case)
nor already covering it (nothing to do) -- and go red naming both SHAs
if so. `expect` is always a peeled commit (fetch_v1() reads
`refs/tags/v1^{commit}`), so this doesn't add a second failure mode
for an annotated v1; that tag form's existing "not a lost lease"
behavior on the first rejected push is untouched.
Surfaced by PR #28's final review. New selftest scenario 11 in
release-v1-selftest.sh, red-proven against the unfixed script (v1 was
silently moved off the stray commit); green after the fix, with the
full 7-suite gate (shellcheck + selftest.sh) passing.
Ride-alongs from the same review:
- ci.yaml:120-123 claimed the README's Versioning section documented
what's verified about the release token's write access; it said
nothing. Added an accurate sentence there (the grant is unobserved
until the first merge, capped by repo/owner token-permission maxima
and unreadable tag protections) and pointed the comment at it.
- Deleted two comment-as-decision-history paragraphs per
comments-are-not-exposition: ci.yaml's "no job-level concurrency"
rationale (kept one line of intent) and
prune-cache-selftest.sh scenario 9's account of how an
existence-only assertion used to pass with the pass-2 guard removed
(kept a one-line statement of what it checks).
- README's release-v1-selftest.sh table row now names the new
hand-placed-v1 scenario.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkSjXXtU6JYcN2vPntWhfb
This commit is contained in:
@@ -34,15 +34,9 @@
|
||||
# warm start.
|
||||
# 8. SELF-CLEAR REPORTS LOUDLY to the job summary, not just a log warning.
|
||||
# 9. OWN CACHE NEVER EVICTED by a sibling pass, genuinely under pressure —
|
||||
# against a real, shrinking `df` (gitdan-actions#26): the static
|
||||
# CACHE_DF_OVERRIDE every other scenario uses never reflects an
|
||||
# eviction, so pass 3's self-clear (`rm -rf "$OWN_DIR"; mkdir -p
|
||||
# "$OWN_DIR"`) fires regardless and recreates an empty OWN_DIR whether
|
||||
# pass 2 touched it or not — existence survives either way, which is
|
||||
# why an existence-only assertion here passed even with the pass-2
|
||||
# guard removed. This one checks CONTENTS, and sizes the requirement
|
||||
# so it is satisfiable without self-clear at all: only pass 2's guard
|
||||
# decides the outcome.
|
||||
# against a real, shrinking `df` (gitdan-actions#26). Checks CONTENTS,
|
||||
# not just existence, so pass 3's self-clear can't mask a missed
|
||||
# pass-2 guard.
|
||||
# 10. SCOPED TO THE CACHE ROOT — a decoy outside it (standing in for another
|
||||
# project's volume) is never touched.
|
||||
# 11. A LIVE READER MARKER PROTECTS A CACHE the same way a lock file does — a
|
||||
|
||||
@@ -161,5 +161,20 @@ in_ci merge "$tip" >/dev/null
|
||||
[ "$(origin_tip)" = "$tip" ] || fail "main moved"
|
||||
ok "tip == gated sha: released, including onto an absent v1"
|
||||
|
||||
echo
|
||||
echo "=== 11. a v1 hand-placed on an unrelated commit is never silently overwritten ==="
|
||||
# Unlike #7, nothing races here -- v1 already sits on the stray commit before
|
||||
# the very first push attempt, so force-with-lease sees exactly the value it
|
||||
# expects and would otherwise succeed outright.
|
||||
fresh
|
||||
stray=$(git -C "$scratch/w/dev" commit-tree -m stray 'HEAD^{tree}')
|
||||
git -C "$scratch/w/dev" push -q -f origin "$stray:refs/tags/v1"
|
||||
tip=$(commit_to_main)
|
||||
if in_ci merge "$tip" 2>"$scratch/err"; then fail "an unrelated hand-placed v1 was overwritten"; fi
|
||||
[ "$(origin_v1)" = "$stray" ] || fail "v1 moved off the hand-placed $stray"
|
||||
grep -q "$stray" "$scratch/err" || fail "the error did not name the stray v1: $(cat "$scratch/err")"
|
||||
grep -q "$tip" "$scratch/err" || fail "the error did not name the gated sha: $(cat "$scratch/err")"
|
||||
ok "hand-placed v1, unrelated to tip: red on the first push, v1 untouched"
|
||||
|
||||
echo
|
||||
echo "release-v1-selftest: all $pass_count assertions passed"
|
||||
|
||||
@@ -41,6 +41,15 @@ covers() {
|
||||
|
||||
push_leased() {
|
||||
local sha="$1" expect="$2" now attempt
|
||||
|
||||
# force-with-lease only compares the ref's current value, not ancestry, so
|
||||
# an unrelated v1 -- neither behind <sha> nor covering it -- would
|
||||
# otherwise be silently overwritten on the very first push.
|
||||
if [ -n "$expect" ] && ! covers "$sha" "$expect" && ! git merge-base --is-ancestor "$expect" "$sha"; then
|
||||
echo "ERROR: v1 ($expect) is neither an ancestor of $sha nor at/ahead of it -- refusing to overwrite an unrelated v1" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
for ((attempt = 1; attempt <= MAX_ATTEMPTS; attempt++)); do
|
||||
if git push -q --force-with-lease="refs/tags/v1:$expect" origin "$sha:refs/tags/v1"; then
|
||||
echo "v1 moved ${expect:-<absent>} -> $sha"
|
||||
|
||||
Reference in New Issue
Block a user