Skip to content

fix(ci): reject a non-tag ref on the manual refresh dispatch - #46

Merged
github-actions[bot] merged 3 commits into
developfrom
fix/validate-refresh-master-tag-input
Aug 17, 2026
Merged

fix(ci): reject a non-tag ref on the manual refresh dispatch#46
github-actions[bot] merged 3 commits into
developfrom
fix/validate-refresh-master-tag-input

Conversation

@nolte

@nolte nolte commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Summary

#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, so a mistyped develop does not fail — it resolves and force-pushes
unreleased work onto main, the branch the README and the docs site link to
publicly. Add a guard job the refresh depends on.

Changes

  • Add a validate_ref job that accepts only a vN.N.N shape for
    inputs.tag and rejects branch names, raw SHAs, and the empty string.
  • Make refresh_presentation_branch depend on it via needs:.
  • The guard 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: published path.
  • 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; jobs is
    ['validate_ref', 'refresh_presentation_branch'] and the latter carries
    needs: ['validate_ref'].
  • Behavioural check: extracted the step's run: script from the parsed YAML and
    executed it against six inputs with EVENT_NAME=workflow_dispatch.
    v0.1.5 and v1.2.3 are accepted; develop, main, the empty string, and
    57450b5 are rejected. With EVENT_NAME=release it exits 0 without checking.
  • Not established: an end-to-end dispatch of the guarded workflow.
    workflow_dispatch reads the workflow file from the target ref, so that is
    only observable after this lands on develop.

Risk / rollout notes

Low, with one deliberate trade-off: the guard adds a runner job to the
release: published path that does nothing but exit 0. The alternative — an
if: github.event_name == 'workflow_dispatch' on the guard — would skip the
job, and a skipped needs: dependency skips the refresh with it. Paying one
trivial 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 case glob, which review found was not
right-anchored — v2.0.0-rc, v1.2.3-hotfix and v1x.2y.3z all 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 is
accepted as out of scope.

Ordering: this touches the same jobs: block as #44. Whichever lands second
will need a trivial rebase.

Originating source: /code-review of PR #43, finding 4 of 4
Dispatched specialist: no matching specialist existed — generalist handled

#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
@boring-cyborg boring-cyborg Bot added chore Maintenance cicd ci/cd process functionality. labels Aug 17, 2026
nolte and others added 2 commits August 17, 2026 23:26
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
@nolte
nolte marked this pull request as ready for review August 17, 2026 21:34
@nolte nolte added the automerge Allow automatic Merge. label Aug 17, 2026
@github-actions
github-actions Bot merged commit dfcec41 into develop Aug 17, 2026
9 checks passed
@github-actions
github-actions Bot deleted the fix/validate-refresh-master-tag-input branch August 17, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Allow automatic Merge. chore Maintenance cicd ci/cd process functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant