feat(openbloxd): publish the daemon binary, add --version, and genericize the docs - #26
Merged
Merged
Conversation
openbloxd is installed on a host, not pulled as an image: it holds the Docker socket so containerized callers do not have to, and running it in a container with that socket mounted would put the privilege straight back where this daemon exists to take it from. So the release artifact is a binary, which is what deploy/openbloxd.service already expects at /usr/local/bin/openbloxd. Nothing published one. release.yml still carried "there are no binaries to ship until cmd/openbloxd exists" — true when it was written, stale since the daemon landed, and v0.3.0 shipped with zero assets as a result. Tag-triggered, matching publish-image.yml: release.yml cuts the tag from conventional commits once CI is green on main, and both publish jobs hang off the tag rather than off each other. Also adds --version, which the install runbook needs and which did not exist: flag.Parse defined only -config, so `openbloxd --version` printed usage and exited 2. The client and daemon must agree on the wire format, so "what is running on this host" has to be answerable. It is stamped via ldflags at release and reports "dev" otherwise, because an unstamped binary is someone's local build and should not claim to be a release. The workflow_dispatch tag input is validated against a semver pattern before it reaches actions/checkout's ref. That input is controlled by anyone who can trigger the workflow, and this job holds contents: write — a crafted value would choose what code gets built and attached to a release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This repository is public. The design doc, the plan and one workflow comment named a specific private service, host and deployment, linked issues in a private repository, and recorded the security posture of a named box — including a socket mount accepted as a known risk on it. None of that is openblox's to publish, and none of it makes the documents better: the design is about what a broker owes *any* caller. So the caller is now "a caller", the rollout describes the shape a deployment follows rather than one deployment's steps, and the decision record keeps the argument while dropping whose argument it was. The example image references stay. They name public images in this repository's own organization, which is what an example config should do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review found that the previous commit asserted two things that are not true, and that this workflow could still publish a wrong or half-finished release. **The kept image references are not public.** That commit justified keeping `blox-sandbox` and `blox-browser` on the grounds that they "name public images in this repository's own organization". They do not: an anonymous ghcr token request is refused for both, while `openblox-sandbox` — the image this repo actually builds and publishes — is granted. So the de-identification pass kept two private references on a rationale that a one-command check refutes. The sandbox examples now use the real public image; the browser example uses a neutral placeholder, because no reference browser image is published. **The release could still be partial.** Three ways, all now closed: - `fail-fast` was left at its default, so a failing amd64 leg cancels arm64 and the release ends up holding one architecture. That is the same partially-published failure this workflow exists to end, only harder to notice than shipping nothing at all. - `gh release upload` needs the release object, which it does not create. A hand-pushed tag has none, and on the normal path go-semantic-release creates the tag ref and the release in separate calls — this job's trigger fires on the first. The build usually outlasts that gap; "usually" is the wrong property here. It now waits for the object and fails loudly if it never appears, rather than creating one and racing the sibling leg. - The verify step ran `--version` and threw the output away, so it proved the flag exists and nothing about the stamp. If `-X main.version` ever stops landing, the binary prints `dev` and the step still passes — publishing an asset that claims to be someone's local build, which is exactly what the `dev` default is for. It now asserts the value equals the tag. Also: attach the systemd unit and example config, since the workflow header claimed to ship a unit and did not; key the concurrency group on the tag rather than the triggering ref, which had it backwards (dispatches for different tags serialized, while a dispatch and a tag push for the same tag did not); document the install in docs/security.md, which described deploying a daemon whose binary had no stated source; and drop the comment in release.yml claiming there are no binaries to ship, which the previous commit diagnosed as the cause of the assetless release and then left in place. `TestVersionDefaultsToDev` pins the invariant the `version` doc comment argues for, so stamping in-tree fails a test instead of shipping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks
|
Warning Review limit reached
Next review available in: 19 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
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 |
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
1 participant
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.
What this changes
Publishes the
openbloxdbinary as a release asset, gives it a--versionflag, and stops the docs from describing one deployment as though it were the only one.openbloxdis installed on a host, not pulled as an image — it holds the Docker socket so containerized callers do not have to, and running it in a container with that socket mounted would put the privilege straight back where the daemon exists to take it from.deploy/openbloxd.servicealready expects/usr/local/bin/openbloxd. Nothing produced one.release.ymlstill carried the comment "there are no binaries to ship until cmd/openbloxd exists" — accurate when written, stale since #22 landed. Sov0.3.0is tagged and ships no assets: an install guide can tell an operator to download a file that was never built.--versiondid not exist either.flag.Parsedefined only-config, soopenbloxd --versionprinted usage and exited 2. That would have failed this workflow's own verification step on every run, and any post-install check that asks a host what it is running. The client and daemon must agree on the wire format, so that question needs an answer. Stamped via-ldflagsat release; reportsdevotherwise, because an unstamped binary is someone's local build and should not claim to be a release.Docs describe a caller, not a customer. This repository is public, and
specs/,plans/and one workflow comment named a specific private service, host and deployment, linked issues in a private repository, and recorded the security posture of a named box — including a socket mount accepted there as a known risk. None of that is openblox's to publish, and none of it made the documents better: the design is about what a broker owes any caller, so the rollout section now describes the shape a deployment follows rather than one deployment's steps, and the decision record keeps the argument without whose argument it was.History was rewritten to remove it. The text had been live on public
mainand inside thev0.3.0tag since4ecc1bc, so a forward fix alone would have left it served. Every reachable commit was filtered (git filter-repo),mainmoved4ecc1bc → 59abbb4, and all fourv*tags were re-pointed. Branch protection and the tag-immutability ruleset were disabled for the push and restored immediately afterwards, with their rules verified intact.What that does not reach, stated plainly rather than left implied:
proxy.golang.orghas an immutable cached zip ofv0.3.0that still contains the original text. Go module zips include every tracked file, sospecs/andplans/went to the mirror the moment anything fetched the module. No rewrite touches this.refs/pull/*keeps the pre-squash commits reachable by SHA, and PR diffs keep their original content. Only GitHub Support can purge those.GOPROXY=directwill see a checksum mismatch againstsum.golang.orgfor the re-pointed tags, since that database is append-only. Anyone on the default proxy is unaffected.The practical consequence: a new version cut from this clean tree is what actually gets consumed going forward.
The private image references that the first commit kept — on the stated grounds that they were public — are fixed in the second commit: they were not public. An anonymous ghcr token request is refused for both, while
openblox-sandbox, which this repo actually publishes, is granted.Shape
Tag-triggered (
v*), matchingpublish-image.yml.release.ymlcuts the tag from conventional commits once CI is green on main, and both publish jobs hang off that tag rather than off each other. linux/amd64 and linux/arm64,CGO_ENABLED=0,-trimpath, with a.sha256beside each.The
workflow_dispatchtaginput exists so an already-tagged release whose assets are missing can be backfilled without cutting a throwaway tag.Checklist
feat:,fix:,docs:, …)makepasses locally (vet, lint, race tests)Verified by hand rather than assumed: the arm64 cross-build produces
ELF 64-bit LSB executable, ARM aarch64, statically linked; a stamped build reports its tag and an unstamped one reportsdev; and the tag regex acceptsv0.3.0/v1.2.3-rc.1while rejectingmain,../../evil,refs/heads/main, andv0.3.0;rm -rf /.Security defaults
Worth a reviewer's attention
The
workflow_dispatchtaginput is validated against a semver pattern before it reachesactions/checkout'sref:. That input is controlled by anyone who can trigger the workflow, and this job holdscontents: write— unvalidated, a crafted value chooses what code gets built and attached to a release. The validation step runs before checkout for that reason. Every other interpolation goes throughenv:rather than into arun:script, matching the disciplinepublish-image.ymlalready documents.contents: writeis scoped to the job, not the workflow, and the built-inGITHUB_TOKENis enough — unlikerelease.yml, nothing downstream needs to be triggered by this job's writes, so no PAT is involved.The second commit closes three ways this workflow could still publish a wrong or half-finished release:
fail-fastdefaulted to cancelling the sibling architecture (so a failing amd64 leg leaves the release holding one arch — the same partial-publish failure this exists to end, only harder to spot);gh release uploadneeds a release object it does not create, which a hand-pushed tag lacks and whichgo-semantic-releasecreates in a separate call from the tag ref this job triggers on; and the verify step discarded--version's output, so it proved the flag exists and nothing about the stamp.Filed separately rather than folded in: #24 (a hung dependency install in CI can silently skip a release) and #25 (
brokerclientcannot detect a policy option set to its library default). Structural, and the root cause of the leak class: a 2,600-line internal implementation plan lives in this public tree — worth deciding whether it belongs here at all.🤖 Generated with Claude Code