fix(ci): reject a non-tag ref on the manual refresh dispatch - #46
Merged
github-actions[bot] merged 3 commits intoAug 17, 2026
Merged
Conversation
#43 added a required `tag` input to release-cd-refresh-master.yml but left it unvalidated. The reusable resolves from_branch as tag -> origin/<branch> -> raw SHA, in that order, so a mistyped `develop` does not fail: it resolves to refs/remotes/origin/develop and force-pushes unreleased work onto `main`, the branch the README and the docs site link to publicly. Add a validate_ref job the refresh depends on. It accepts only a vN.N.N shape and rejects branch names, raw SHAs, and the empty string. The job carries no `if:` and instead short-circuits on a non-dispatch event, because a skipped job makes its `needs:` dependent skip too — an `if:` here would silently disable the refresh on the release path. The upstream resolution order is deliberate and shared, so it is not patched; this is a caller-side constraint, per spec/project/github-actions-best-practices/ §E which forbids fixing shared logic in a consumer copy. Verified by extracting the step's script and running it against v0.1.5, v1.2.3, develop, main, the empty string, and a raw SHA: the first two are accepted, the rest rejected, and a release-path invocation exits 0 without checking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4KA8swD1ZzDqknDV5TUxx
Review found the case glob `v[0-9]*.[0-9]*.[0-9]*` is not right-anchored: it ends in `*`, so `v2.0.0-rc`, `v1.2.3-hotfix`, `v1x.2y.3z` and a trailing-space `v1.2.3 ` all matched. Those are realistic release-prep branch names, and the reusable resolves a branch when no tag of that name exists — so the guard would have waved through exactly the force-push it was added to prevent. Replace it with an anchored bash regex `^v[0-9]+\.[0-9]+\.[0-9]+$`. Re-verified by extracting the step's script and running it against thirteen inputs: v0.1.5, v1.2.3 and v10.20.30 accepted; v2.0.0-rc, v1.2.3-hotfix, v1x.2y.3z, 'v1.2.3 ', develop, main, release/v1.2.3, the empty string, 57450b5 and v1.2 rejected; the release path still exits 0 without checking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4KA8swD1ZzDqknDV5TUxx
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.
Summary
#43 added a required
taginput torelease-cd-refresh-master.ymlbut left itunvalidated. The reusable resolves
from_branchas tag →origin/<branch>→raw SHA, so a mistyped
developdoes not fail — it resolves and force-pushesunreleased work onto
main, the branch the README and the docs site link topublicly. Add a guard job the refresh depends on.
Changes
validate_refjob that accepts only avN.N.Nshape forinputs.tagand rejects branch names, raw SHAs, and the empty string.refresh_presentation_branchdepend on it vianeeds:.if:and instead short-circuits on a non-dispatchevent, because a skipped job makes its
needs:dependent skip too — anif:here would silently disable the refresh on the
release: publishedpath.permissions: {}on the guard job: it reads an input and needs no token.Linked issues
None
Testing
task --yes lint— all pre-commit hooks pass.python3 -c "yaml.safe_load(...)"— parses;jobsis['validate_ref', 'refresh_presentation_branch']and the latter carriesneeds: ['validate_ref'].run:script from the parsed YAML andexecuted it against six inputs with
EVENT_NAME=workflow_dispatch.v0.1.5andv1.2.3are accepted;develop,main, the empty string, and57450b5are rejected. WithEVENT_NAME=releaseit exits 0 without checking.workflow_dispatchreads the workflow file from the target ref, so that isonly observable after this lands on
develop.Risk / rollout notes
Low, with one deliberate trade-off: the guard adds a runner job to the
release: publishedpath that does nothing but exit 0. The alternative — anif: github.event_name == 'workflow_dispatch'on the guard — would skip thejob, and a skipped
needs:dependency skips the refresh with it. Paying onetrivial job beats silently disabling the automatic path.
The shape check is intentionally structural rather than a lookup against
published tags: it rules out the reported failure mode without a token or an
API call. The first revision used a
caseglob, which review found was notright-anchored —
v2.0.0-rc,v1.2.3-hotfixandv1x.2y.3zall passed.Fixed in 5e45afd with an anchored regex and re-verified against thirteen
inputs. The residual gap is now only a branch named exactly
vN.N.N, which isaccepted as out of scope.
Ordering: this touches the same
jobs:block as #44. Whichever lands secondwill need a trivial rebase.
Originating source:
/code-reviewof PR #43, finding 4 of 4Dispatched specialist: no matching specialist existed — generalist handled