ci(release): gate publishing to master merges only#81
Conversation
The Release step had no branch guard, so any push to any branch whose head commit contained `[release]` ran publish.sh. Verified in run history that releases were firing from feature branches before merge (e.g. lib-cloudinfo v1.2.1 published from feature/NOTASK-lib-cloudinfo-gpu-features), bypassing PR review entirely. Gate the Release step to `github.ref == 'refs/heads/master'`. master is protected and updated only via merged PRs, so releases now require an approved, merged PR — closing the audit hole where anyone with push access could cut a release without review. Drop the `[release]` commit-message marker: publish.sh is already idempotent (skips versions already in the repo, publishes only new ones), so bumping a module's VERSION file is the sole release signal. The marker was redundant ceremony. Docs updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| GITHUB_TOKEN: ${{ secrets.GH_SEQERA_TOKEN }} | ||
|
|
||
| - name: Release | ||
| if: "contains(github.event.head_commit.message, '[release]')" |
There was a problem hiding this comment.
what's up with the [release] tag?
There was a problem hiding this comment.
The script in place already check if a VERSION was incremented. Do we want to force the release with the tag or just look at the VERSION file? I can bring it back without problem, my main concern is to only execute it on master.
Nevertheless, since the current script is already doing this check, I thought we can just always release if VERSION changed. What do you think?
There was a problem hiding this comment.
For me it's a problem that I can just push [release] and publish an artifact without any kind of approval. Claude did it a couple of times and it's:
- Annoying
- Not compliant at all (SOC2, looking at you)
There was a problem hiding this comment.
Basically, if you want to keep the release tag, we can have the line like this:
if: "github.ref == 'refs/heads/master' && contains(github.event.head_commit.message, '[release]')"
Nevertheless, since the publish.sh script already checks for the VERSION files to only release the ones that have been increased, I'd just assume a new version will be released if VERSION gets incremented in order to simplify the process. Your call.
There was a problem hiding this comment.
I think it's preferable to keep align across all repos (explicit [release]) so it's a no brainer choice
There was a problem hiding this comment.
I mean, it's more brainer that having it automatic without any message.
Or you mean to just leave it as it is now and allow to make releases without approval?
I modified it so we do not release if [release] is not present, even if we increase the VERSION
There was a problem hiding this comment.
The same rule for all repos
There was a problem hiding this comment.
This is from platform:
No release in cloudinfo:
https://github.com/seqeralabs/cloudinfo/blob/master/.github/workflows/ci.yaml
Sched:
So same rule for all repos include checking that the branch is master.
In any case, allowing every user with the capacity to push a branch into the repository to trigger a release with random code without any review should not be OK even if done in other repositories. Let's imagine this was the general approach. I find hard to understand that we allow this and, under my opinion, the approach should be to change it everywhere instead of keeping it this way because it's like this everywhere.
As far as I know, we are trying to be SOC2 compliant. And as far as I know, any changes in the repositories should be approved (this is why we have protected branches and require pull requests) on that framework.
Keep the [release] commit-message marker as an explicit per-merge opt-in, in addition to the master branch gate. A release now fires only when the push is to master AND the merge commit message contains [release]; publish.sh remains idempotent, so a bumped VERSION file still gates what actually publishes. This preserves the ability to merge a VERSION bump without publishing it, while keeping releases restricted to approved, merged PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problems
The
Releasestep in.github/workflows/build.ymlhad no branch guard — its only condition wascontains(github.event.head_commit.message, '[release]'). Combined with theon: push: branches: ['**']trigger, any push to any branch with[release]in the head commit ranpublish.shand could publish artifacts. This creates two distinct problems.1. Audit gap — releases without approval
Anyone with push access could cut a release from a feature branch, with no PR review. Verified in run history — releases were in fact firing from feature branches before merge:
28860427327feature/NOTASK-lib-cloudinfo-gpu-features28858278102fix/NOTASK-lib-cloudinfo-sched-passthrough-test28869154215bump-cloudinfo-version2. Error-prone — publishing before review completes
Because the release fires on the pre-merge push rather than on merge, a version can be published from code that review then changes. Review comments refine the code, but the artifact for that version was already burned on the earlier push. And since
publish.shis idempotent (it skips anyname-version.pomalready in the repo), when the refined code finally merges tomasterit finds the version already exists and skips it — so the reviewed, merged code is never published under that version. The published artifact silently diverges from what was reviewed and merged.Circumstantial evidence this already happened:
lib-cloudinfov1.2.1ran as[release] … GpuFeatures … v1.2.1on the feature branch, but the code that merged tomasterwasAcceleratorFeatures … v1.2.1— same version, renamed API. Ifv1.2.1was published from the earlier feature-branch run, the master merge would have skipped it, leaving the pre-renameGpuFeaturesbuild published asv1.2.1. (Not log-confirmed — the feature-branch run logs weren't retrievable — but consistent with the run ordering andpublish.sh's skip-if-exists logic.)Change
Add a
masterbranch guard to the existing[release]marker check — both must hold:mastergate:masteris protected and updated exclusively via merged PRs, so releases now require an approved, merged PR, and the publish runs against the final, reviewed code — not a mid-review snapshot.[release]marker kept: an explicit per-merge opt-in. The marker must be present in the merge commit message (i.e. the squash/PR title, which today already carries it — e.g.[release] Bump cloudinfo library (#80)).A release therefore requires three things to align: push to
master+[release]in the merge commit + a bumpedVERSION(only new versions publish, sincepublish.shskips ones already in the repo).Behavior after this change
master, the merge commit contains[release], and a module'sVERSIONwas increased.VERSIONbump can still be merged without publishing by omitting[release]from the merge commit.Docs
CLAUDE.mdRelease Management section updated to describe the new flow (bumpVERSION+[release]in the PR title → merge tomaster).Note
Opened as draft for review. No Jira ticket (NOTASK).
🤖 Generated with Claude Code