chore: harden the release publish path - #35
Conversation
Three follow-ups from the v0.0.2 release, plus one hazard they exposed. Preview snapshot tags (v0.0.1-20260728 and friends) shared the `v*` trigger with real releases and failed the ancestry or version gate, leaving five red runs with more to come. The publish job now skips any tag containing `-`. The trigger stays broad on purpose: a narrow tag filter would also silence a malformed release tag like `vfoo`, making a botched release indistinguishable from no release. SemVer uses `-` for pre-release identifiers, and this workflow refuses pre-releases anyway since it always moves `:latest`, so the guard is strictly quieter and never more permissive. The Makefile pointed at `docker.io/hathor`, which does not exist on DockerHub. Correcting it to the real namespace would have turned a target that failed at auth into a one-word way to clobber production: `make push` builds single-arch, always moved `:latest`, and bypasses the tag/manifest match and the immutability check the workflow enforces. It now has no default registry and refuses to run without an explicit DOCKER_REGISTRY, never pushes `:latest`, and checks before building rather than after. `docker push` and `make push` join the deny list. Documents that a publish failing after the push is not retryable, and names the one unrecoverable state: `:<version>` lands, `:latest` does not, and bump-and-retag leaves that version permanently non-latest. Closes #28 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe release pipeline now skips pre-release tags, verifies signed tag and commit provenance, and documents recovery rules. Docker publishing requires an explicit registry and versioned tag. Claude permissions deny destructive or unapproved push commands. ChangesRelease pipeline hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubTagEvent
participant docker_yml
participant GitHubAPI
participant DockerRegistry
GitHubTagEvent->>docker_yml: start release workflow
docker_yml->>GitHubAPI: verify tag and commit signatures
GitHubAPI-->>docker_yml: return provenance status
docker_yml->>DockerRegistry: publish verified release image
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Hardens the release/publish workflow and local tooling to reduce the risk of accidental or unsafe Docker image publishing, especially around snapshot/pre-release tags and local make push behavior.
Changes:
- Skip the Docker publish job entirely for tags containing
-(snapshot/pre-release style), avoiding noisy failed runs. - Remove the Makefile’s default Docker registry and add an early guard so
make pushrequires explicitDOCKER_REGISTRY(and avoids pushing:latestvia explicit tagging). - Extend the agent deny list and document the non-retryable publish failure mode and the sanctioned release path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Makefile | Removes default registry, adds push-time guard, and clarifies push semantics. |
| CLAUDE.md | Documents publish irreversibility and reinforces the workflow-only publish path. |
| .github/workflows/docker.yml | Skips publish job for tags containing - to avoid failed runs for snapshot tags. |
| .claude/settings.json | Adds additional deny-list entries to prevent risky publish commands. |
Suppressed comments (1)
Makefile:50
- Even though
make pushno longer tags/pushes an explicit:latest, it will still push...:latestifTAGfalls back tolatest(e.g., when bun/package.json version resolution fails). That contradicts the safety comment above and can unintentionally clobber a scratch registry’s latest tag. Consider refusing to push whenTAG=latest.
$(MAKE) build
docker tag $(IMAGE_NAME):$(TAG) $(REGISTRY)/$(IMAGE_NAME):$(TAG)
docker push $(REGISTRY)/$(IMAGE_NAME):$(TAG)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @test -n "$(REGISTRY)" || { \ | ||
| echo "DOCKER_REGISTRY is unset."; \ | ||
| echo "Releases are published by .github/workflows/docker.yml from a"; \ | ||
| echo "signed tag — see the Releases section of CLAUDE.md."; \ |
There was a problem hiding this comment.
Good catch — the message was promising something the pipeline did not enforce. Fixed by making it true rather than by softening the wording.
298bd9d adds a Require a signed tag on a signed commit step that reads verification state from GitHub's API and refuses to publish unless both the tag object and the commit it points at are verified signatures. Lightweight tags are rejected outright, since they carry no signature at all.
Both objects are checked deliberately: a signed tag on an unsigned commit still leaves the artifact that actually ships without provenance. Verification comes from the API rather than git tag -v so trust stays anchored to the keys registered to the org's accounts, instead of a keyring the runner imports — which a workflow edit could quietly widen.
Simulated against real history before committing: v0.0.1 and v0.0.2 both pass, and the unsigned v0.0.1-20260728 preview snapshot is rejected as lightweight — so this is also defence-in-depth behind the pre-release skip added in this PR.
Your suppressed comment on line 50 was right too, and is fixed in the same commit: TAG falls back to latest when DOCKER_TAG is unset and the version cannot be read from package.json, so make push could still move a floating tag despite the comment above it saying otherwise. It now refuses when TAG resolves to latest.
The publish path checked that a tag was well-formed and pointed at a reviewed commit, but never that anyone authorised the release. Anyone able to push a tag could ship a public image from any commit on main. The push guard already claimed releases came from a signed tag, which made the message a promise the pipeline did not keep. The workflow now reads verification state from GitHub's API and refuses unless both the tag object and the commit it points at are verified signatures. Reading it from the API rather than `git tag -v` keeps trust anchored to the keys registered to the org's accounts, instead of a keyring the runner imports and a workflow edit could quietly widen. Both objects are checked deliberately: a signed tag on an unsigned commit still leaves the thing actually shipping without provenance. Lightweight tags are rejected outright since they carry no signature at all — a release tag must be `git tag -s`. Verified against real history: v0.0.1 and v0.0.2 both pass, and the unsigned v0.0.1-20260728 preview snapshot is rejected as lightweight, which makes this defence-in-depth behind the pre-release skip. Also refuses `make push` when TAG resolves to `latest`. That happens when DOCKER_TAG is unset and the version cannot be read from package.json, and pushing then moves a floating tag rather than an identifiable build — contradicting the comment directly above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three follow-ups from the v0.0.2 release (#24), plus one hazard they exposed.
How it works
-. Preview snapshot tags (v0.0.1-20260728and friends) shared thev*trigger with real releases and failed the ancestry or version gate, leaving five red runs with more accruing.docker.io/hathor, which does not exist on DockerHub. It now has no default andmake pushrefuses without an explicitDOCKER_REGISTRY;:latestis never pushed from here, and the guard runs before the image build rather than after.docker pushandmake pushjoin the tagging commands on the deny list.The trigger stays broad on purpose. A narrow tag filter would also silence a malformed release tag like
vfoo, making a botched release indistinguishable from no release. SemVer uses-for pre-release identifiers and this workflow refuses pre-releases anyway (it always moves:latest), so the guard is strictly quieter and never more permissive. The version regex remains the authoritative check.Behavioral change
make pushno longer works without configuration, and that is deliberate. Correcting the registry to the real namespace would have converted a target that failed at auth into a one-word way to clobber production: it builds single-arch, so it would replace the published multi-arch index and break arm64 consumers; it always moved:latest; and it bypasses the tag/manifest match and the immutability check the workflow enforces.This deviates from the issue's acceptance criterion, which asked for
make pushto target the same registry as the workflow. Satisfying that as written would have made the repository less safe.Acceptance criteria
make pushwith noDOCKER_REGISTRYfails immediately with an explanatory message and builds nothing;make buildandmake helpare unaffected.CLAUDE.md.Closes #28
Summary by CodeRabbit
latesttag.