fix(release): close the residual native release admission gaps - #122
Merged
Conversation
Three gaps that PR #115 deliberately deferred, plus three adjacent ones. Branch-name bypass of the version advance guard. The guard gated its only job on `startsWith(github.head_ref, 'native-version/v')`. The head branch name is author controlled and GitHub counts a skipped job as a satisfied required status check, so a version bump from any other branch skipped the guard entirely. The job now always runs for the five paths. It cannot simply demand the reserved branch, because Dependabot npm/cargo updates and ordinary feature work edit the same files for dependency reasons. It decides on content instead: when the pull request leaves the native version alone it passes; when the pull request moves the version, only the reserved `native-version/v*` branch of this repository may do it, from the current protected base, as the exact strict advance. The job also checks out the base rather than the head, so a pull request cannot rewrite the guard that judges it. Fail-open monotonicity check. `write_verified_release_index` and `write_verified_release_channel` skipped the strict-advance comparison when `--existing` named an absent file, and the release workflow made that reachable by ending its download with `|| true`. A passed but missing `--existing` is now an error, and the workflow decides from the release's authenticated asset list instead of from the absence of a downloaded file. Release-order comparison set filtered by mutable metadata. The set admitted a release only when `draft` was false, `prerelease` was true, and the body carried the installer marker. All three are mutable, and this repository rewrites release notes itself, so a release that lost the marker dropped out and a lower tag became acceptable. The comparison set is now the immutable `desktop-v*` Git tag namespace. That set cannot observe draft state, which the old set refused as a side effect, so `publish-draft` now refuses explicitly to return an already public release to draft. Also: - `verify()` proved each member regular and unlinked, then hashed `(directory / name).read_bytes()`, which resolved the name again and followed a link created in between. It now hashes the validated path objects, and every read in the standalone verifier opens with `O_NOFOLLOW` where the platform has it. - `_git_bytes` and the `git diff` call interpolated a caller ref into an argv element, so a ref beginning with `-` reached Git as an option. Every ref now resolves through `git rev-parse --verify` to a 40-character object id first, and the diff argument list ends with `--`. - `expected_asset_names` in the standalone verifier duplicates `expected_release_asset_names`. The verifier ships beside the installers and may not import the module, so a new contract test fails when the two definitions drift apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #115 (
6f43a97). It closes the three residual gaps that PRdeliberately left for a separate change, plus the three adjacent items raised in
the same review.
1. Branch-name bypass of the version advance guard
native-version-guard.ymlgated its only job onstartsWith(github.head_ref, 'native-version/v'). The head branch name isauthor controlled and GitHub counts a skipped job as a satisfied required
status check, so a version bump from any other branch skipped the guard.
Severity, after confirming the stated mitigation.
native-freshness.yml'spublish-native-tagjob does re-validate the exact five-file transform againstthe engine commit on every push to main, so a bogus bump cannot reach a
desktop-v*tag. The bypass therefore lands a bad commit on main and jams thefreshness lane; it does not produce a signed release. Medium, and cheap to
close.
The fix is not a blanket branch requirement. The five paths also carry
ordinary npm and Cargo dependency state. Dependabot is configured for both
ecosystems, and history shows non-version commits touching
package.json(#104, #96, #48). Refusing every unreserved branch would break all of them.
The job now always runs and decides on content:
native-version/v*branch of thisrepository (a fork can mint any branch name), the current protected base,
and the exact strict advance.
It also checks out the base, not the head, so a pull request cannot rewrite
the guard that judges it. The head is fetched through
refs/pull/N/headandverified against the event SHA.
Simulated against six pull-request shapes, running the step's exact logic:
package.jsonscript2. Fail-open monotonicity check
write_verified_release_indexandwrite_verified_release_channelreadif existing is not None and existing.is_file():, so a passed-but-absent--existingsilently dropped the strict-advance comparison. The caller made itreachable: the download step ended with
|| true.A passed but missing
--existingis now an error. The workflow decides from therelease's authenticated asset list rather than from the absence of a downloaded
file. Simulated with a stubbed
gh:--existing--existing--existingpassed--existingpassed3. Release-order comparison set filtered by mutable metadata
_published_native_releasesadmitted a release only whendraftwas false,prereleasewas true, andINSTALLER_RELEASE_MARKERappeared in the body. Allthree are mutable, and this repository rewrites release notes itself with
gh release edit. A release that lost the marker dropped out of the set and alower tag became acceptable.
The comparison set is now the immutable
desktop-v*Git tag namespace, readwith
git ls-remote --tags. Verified against this repository's real 66-linels-remoteoutput: 17 native tags parsed,desktop-v0.9.0anddesktop-v0.14.0refused againstdesktop-v0.15.0,desktop-v0.16.0accepted.Two consequences, both handled:
desktop-v*tag exists before both call sites read it, and the tag write isidempotent. Equality is still monotonic, and both call sites separately pin
the tag to an exact commit.
refused as a side effect. Without that, re-running the release workflow for an
already published tag would let
gh release edit --draftreturn a live releaseto draft and
--clobberits assets.publish-draftnow refuses thatexplicitly, at the step that would do the damage.
select_latest_native_releasestill reads release objects. It answers adifferent question - which published release the event refers to - and its
failure mode is already closed (a release that loses its marker makes
event_is_selectedfalse, and the job refuses).Also in this change
verify()symlink re-resolution. It proved each member regular andunlinked, then hashed
(directory / name).read_bytes(), resolving the name asecond time. It now hashes the validated
Pathobjects, and every read in thestandalone verifier opens with
O_NOFOLLOWwhere the platform has it(Windows has no equivalent and keeps the explicit
is_symlinkcheck)._git_bytesand thegit diffcallinterpolated a caller ref into an argv element, so a ref beginning with
-reached Git as an option. Every ref now resolves through
git rev-parse --verifyto a 40-character object id before any other Gitcall, and the diff argument list ends with
--. Verified that--output=<path>,-h, and--upload-pack=/bin/falseare all refused andwrite no file.
expected_asset_namesin the standaloneverifier restates
expected_release_asset_names. The verifier ships beside theinstallers and must stay stdlib-only, so the duplication stays and a contract
test fails when the two definitions drift apart.
Verification
uv sync --locked --extra devuv run pytest tests/ -q-> 998 passed, 6 skippeduv run ruff check engine/ tests/ scripts/-> cleanactionlint(runs shellcheck over everyrun:block) -> cleanNo release is cut and nothing is published. Desktop still pins openadapt-flow
1.31.0, which
RELEASES.mdrecords as a non-releasable interim pin.The native installer matrix in
build.ymland the full Linux/macOS/Windows testmatrix run only on pushes to main, so the post-merge run needs a check too.
🤖 Generated with Claude Code