You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
cmd_publish in scripts/release.py returns success on two paths without ever re-reading the
remote, so neither confirms that the tag it is standing down for is our release.
The two paths
The local pre-check (scripts/release.py:518-520, pre-existing):
iftag_exists(tag):
print(f"{tag} already exists — nothing to publish")
return0
tag_exists is documented as deliberately local — it reads this checkout's refs.
if_already_exists(proc.stderr):
print(f"{tag} was created concurrently — nothing to publish")
return0
_already_exists is "already exists" in stderr.lower() — it matches the phrase GitHub
returns in its 422 body and nothing else.
Neither path checks that the remote tag v<version> points at sha (the commit this run
intended to release, resolved at :526), nor that the published notes came from this
checkout's ## [<version>] CHANGELOG section.
Consequence
A release hand-cut under the same tag from a different commit — or a release.py publish
run from a stale checkout — is indistinguishable from the benign same-flow race, and cmd_publish exits 0. The version is then tagged, the workflow reports success, and nothing
records that the tag does not name the commit we released.
Why it is bounded (and why it was not fixed in #703)
For the flow this script drives, the risk is low: every publisher running release.py publish
derives both the tag name and the notes from the checkout's canonical version
(sync_version.read_canonical() → extract_section(CHANGELOG, version)), so a competing
same-flow publisher produces the release we would have produced. The gap is publishers outside that flow.
Surfaced by CodeRabbit on PR #703 (Major, scripts/release.py:550-552). Triaged as out of
scope there: #703 is #468's three residuals and rewrites that comment only. Adding remote
verification is a behavior change to the release path with a new failure mode and new tests,
and it would have to cover path 1 as well as path 2 to be coherent — path 1 is untouched
pre-existing code. #703 instead narrowed the comment so it no longer claims the desired end
state is proven, and names this gap.
Fix sketch
After a swallowed race (and, for coherence, in place of the bare local pre-check), query the
remote and compare before returning 0 — e.g. gh release view <tag> --json tagName,targetCommitish
or git ls-remote --tags origin <tag> — and _die loudly on a target mismatch. Decide
deliberately whether a notes mismatch is also fatal or merely warned.
Testing hook already exists: tests/test_release.py::_publish_with_gh drives cmd_publish with
a subprocess.run spy (it records kwargs via the seen dict added in #703), so a new remote
probe can be stubbed the same way.
cmd_publishinscripts/release.pyreturns success on two paths without ever re-reading theremote, so neither confirms that the tag it is standing down for is our release.
The two paths
The local pre-check (
scripts/release.py:518-520, pre-existing):tag_existsis documented as deliberately local — it reads this checkout's refs.The lost-race swallow (
scripts/release.py:554-556, landed in9a1dfb8/ release.py publish: tag_exists is a TOCTOU guard, so two branches publishing the same version race #431):_already_existsis"already exists" in stderr.lower()— it matches the phrase GitHubreturns in its 422 body and nothing else.
Neither path checks that the remote tag
v<version>points atsha(the commit this runintended to release, resolved at
:526), nor that the published notes came from thischeckout's
## [<version>]CHANGELOG section.Consequence
A release hand-cut under the same tag from a different commit — or a
release.py publishrun from a stale checkout — is indistinguishable from the benign same-flow race, and
cmd_publishexits 0. The version is then tagged, the workflow reports success, and nothingrecords that the tag does not name the commit we released.
Why it is bounded (and why it was not fixed in #703)
For the flow this script drives, the risk is low: every publisher running
release.py publishderives both the tag name and the notes from the checkout's canonical version
(
sync_version.read_canonical()→extract_section(CHANGELOG, version)), so a competingsame-flow publisher produces the release we would have produced. The gap is publishers
outside that flow.
Surfaced by CodeRabbit on PR #703 (Major,
scripts/release.py:550-552). Triaged as out ofscope there: #703 is #468's three residuals and rewrites that comment only. Adding remote
verification is a behavior change to the release path with a new failure mode and new tests,
and it would have to cover path 1 as well as path 2 to be coherent — path 1 is untouched
pre-existing code. #703 instead narrowed the comment so it no longer claims the desired end
state is proven, and names this gap.
Fix sketch
After a swallowed race (and, for coherence, in place of the bare local pre-check), query the
remote and compare before returning 0 — e.g.
gh release view <tag> --json tagName,targetCommitishor
git ls-remote --tags origin <tag>— and_dieloudly on a target mismatch. Decidedeliberately whether a notes mismatch is also fatal or merely warned.
Testing hook already exists:
tests/test_release.py::_publish_with_ghdrivescmd_publishwitha
subprocess.runspy (it records kwargs via theseendict added in #703), so a new remoteprobe can be stubbed the same way.