Skip to content

ci: auto-tag release commits and fix publish-svn permissions #424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sherwinski merged 1 commit into from
May 21, 2026
Merged

ci: auto-tag release commits and fix publish-svn permissions #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/publish-svn-v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ jobs:
needs: publish
permissions:
contents: write
pull-requests: read
uses: OneSignal/sdk-shared/.github/workflows/github-release.yml@main
with:
version: ${{ needs.publish.outputs.bare_version }}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/publish-svn.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ jobs:
needs: publish
permissions:
contents: write
pull-requests: read
uses: OneSignal/sdk-shared/.github/workflows/github-release.yml@main
with:
version: ${{ needs.publish.outputs.bare_version }}
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/tag-on-release-merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: Tag on Release Merge

on:
push:
branches:
- main
- v2

permissions:
contents: write

jobs:
tag:
runs-on: ubuntu-latest
if: startsWith(github.event.head_commit.message, 'chore: Release ')
steps:
- uses: actions/checkout@v5
with:
fetch-depth: 0
# PAT is required so the pushed tag triggers downstream tag-based
# workflows (publish-svn.yml / publish-svn-v2.yml). Tags pushed with
# the default GITHUB_TOKEN do not trigger other workflows.
token: ${{ secrets.GH_PUSH_TOKEN }}

- uses: onesignal/sdk-shared/.github/actions/setup-git-user@main

- name: Extract version and push tag
env:
MSG: ${{ github.event.head_commit.message }}
run: |
SUBJECT=$(printf '%s\n' "$MSG" | head -n 1)

# Match: "chore: Release X.Y.Z" or "chore: Release X.Y.Z (#NNN)"
VERSION=$(echo "$SUBJECT" | sed -nE 's/^chore: Release ([0-9]+\.[0-9]+\.[0-9]+)( \(#[0-9]+\))?$/\1/p')
if [ -z "$VERSION" ]; then
echo "Commit subject is not a release commit; skipping: $SUBJECT"
exit 0
fi

Check warning on line 38 in .github/workflows/tag-on-release-merge.yml

View check run for this annotation

Claude / Claude Code Review

Missing branch/version-major guard in tag-on-release-merge 

Defense-in-depth gap: the workflow extracts any `X.Y.Z` from `chore: Release X.Y.Z` and pushes `vX.Y.Z` without verifying that the major matches the branch (main → 3.x, v2 → 2.x). Because the `version` input to `release.yml` / `release-v2.yml` is a free-form string with no validation, an operator running the wrong Release workflow (e.g. Release (v2) with `3.9.1`) would land a `chore: Release 3.9.1` commit on `v2`; this job would then push tag `v3.9.1` at a v2 commit, and `publish-svn.yml` (which
Comment on lines +34 to +38

@claude claude Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Defense-in-depth gap: the workflow extracts any X.Y.Z from chore: Release X.Y.Z and pushes vX.Y.Z without verifying that the major matches the branch (main → 3.x, v2 → 2.x). Because the version input to release.yml / release-v2.yml is a free-form string with no validation, an operator running the wrong Release workflow (e.g. Release (v2) with 3.9.1) would land a chore: Release 3.9.1 commit on v2; this job would then push tag v3.9.1 at a v2 commit, and publish-svn.yml (which has no trunk-version guard, unlike its v2 sibling) would silently deploy v2 source to the v3 trunk on wordpress.org. Consider rejecting when the version major doesn't match the branch (fail if [[ $GITHUB_REF_NAME == main && $VERSION != 3.* ]] || [[ $GITHUB_REF_NAME == v2 && $VERSION != 2.* ]]).

Extended reasoning...

What the bug is

tag-on-release-merge.yml triggers on push to both main and v2 and extracts the version with this regex:

VERSION=$(echo "$SUBJECT" | sed -nE 's/^chore: Release ([0-9]+\.[0-9]+\.[0-9]+)( \(#[0-9]+\))?$/\1/p')

It then pushes v$VERSION at $GITHUB_SHA with no cross-check that the major component of VERSION matches the branch the commit landed on. Because publish-svn.yml triggers on v3.* tags and publish-svn-v2.yml on v2.* tags, a tag whose major doesn't match its commit's branch routes the wrong source tree to the wrong WordPress.org slot.

The code path that triggers it

release.yml and release-v2.yml both expose a free-form string version input via workflow_dispatch with no validation:

version:
  description: 'Version to release (e.g., 3.9.0)'
  required: true
  type: string

release.yml hardcodes target_branch: main; release-v2.yml hardcodes target_branch: v2. The branch is locked, but the version isn't validated against it.

Step-by-step proof (mistyped version)

  1. Operator opens the Actions UI and runs Release (v2) with version: 3.9.1 (typo / wrong workflow).
  2. release-v2.yml calls prep-release.yml with version=3.9.1, target_branch=v2, which opens a PR titled chore: Release 3.9.1 (#NNN) against v2. The bump-version step only edits version files on a branch off v2, so the PR diff is small and looks normal.
  3. PR is squash-merged into v2. The merge commit subject is chore: Release 3.9.1 (#NNN).
  4. tag-on-release-merge.yml fires on the push to v2. The regex extracts 3.9.1. The workflow pushes tag v3.9.1 at the v2 head SHA.
  5. publish-svn.yml triggers on the v3.* tag, checks out v3.9.1 (= a v2 commit), rsyncs the v2 source tree into /tmp/release-files, and svn commits it to the v3 trunk on wordpress.org as version 3.9.1.

The mirror case (Release (v3) with version 2.4.6) lands a chore: Release 2.4.6 commit on main, pushes v2.4.6, and triggers publish-svn-v2.yml. In that direction, the Verify trunk is untouched step (publish-svn-v2.yml:110-124) would catch it — but the v3 direction has no equivalent guard.

Why existing code doesn't prevent it

  • The release workflows lock target_branch but not major(version) == branch_major.
  • tag-on-release-merge.yml only checks startsWith(message, 'chore: Release ') and regex shape — not major-vs-branch.
  • publish-svn.yml has no trunk-version sanity check (only its v2 sibling does), so the v3 direction silently publishes.

Addressing the refutation

The refutation argues this requires manually retargeting the auto-PR base branch, which is implausible. That scenario is indeed implausible — but it isn't the only path. The mistyped-version-input path above doesn't require retargeting anything; it only requires running the wrong of two near-identically-named release workflows or making a typo on the major digit. That's a normal-human-error category (the inputs are free-form strings on workflows named Release (v2) and Release (v3) sitting next to each other in the Actions list). The refutation also claims the payload outcome is technically incorrect because the bump-version branch would carry v3 code; that's exactly right and exactly the problem — v3 code (the entire v3/ tree) gets published to wordpress.org under a v2 version number (or vice versa).

The refutation's point that the right place to harden against this is the prep workflow (validate version major against target_branch) is fair; either layer works. A guard here is the simpler and more local change since this PR introduces the new automatic-tag step.

Impact

A successful misroute publishes the wrong major's source tree to the WordPress.org plugin directory under the wrong version number. Reverting an SVN trunk commit and a wordpress.org-visible plugin version is operationally painful and user-visible. The v3 direction is unguarded.

How to fix

Two-line guard before pushing the tag:

case "$GITHUB_REF_NAME" in
  main) [[ "$VERSION" == 3.* ]] || { echo "::error::version $VERSION on main; expected 3.x"; exit 1; } ;;
  v2)   [[ "$VERSION" == 2.* ]] || { echo "::error::version $VERSION on v2; expected 2.x"; exit 1; } ;;
esac

This converts the silent mis-publish into a CI failure on the tag step, before anything reaches SVN.

Severity

Filing as nit: requires operator error, but the fix is trivial and the blast radius (wrong major published to wordpress.org with no v3-side guard) is non-trivial.


TAG="v$VERSION"
if git rev-parse "refs/tags/$TAG" >/dev/null 2>&1; then
echo "Tag $TAG already exists; skipping."
exit 0
fi

echo "Tagging $TAG at $GITHUB_SHA"
git tag "$TAG" "$GITHUB_SHA"
git push origin "$TAG"
Loading