Attach a static linux/arm64 seid binary to releases - #4004
Conversation
PR SummaryMedium Risk Overview GoReleaser registers binfmt for cross-building arm64 on the amd64 release runner, runs
CI adds a Linux ARM64 (static) job on native
Reviewed by Cursor Bugbot for commit a48c675. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cleanly executed arch-split of the static build: the per-arch libgcc pin, checksums, output naming, and the ELF-machine guards in build-static.sh / boot-smoke.sh / goreleaser-shim.sh all line up and fail closed. Two things to address: the new binfmt hook runs an unpinned third-party image with --privileged on the release runner, and the b-tree symbol assertion still has no positive control.
Findings: 1 blocking | 2 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- [suggestion] The release-built
linux/arm64binary is never booted; the safety argument rests on it being byte-identical to the natively-built binary that the newLinux ARM64 (static)job runs the 8-boot gauntlet against. Nothing enforces that.scripts/build-static.shrunsapk add --no-cache build-baseinside the pinned golang:1.25.6-alpine image, which resolves Alpine package versions from the live index at build time, so the CI build and a later release build can pick up different gcc/binutils and diverge. Consider printingsha256sumof the produced binary in both the CI job and the goreleaser hook (or pinning the apk package versions) so a divergence is visible rather than silent — otherwise the gate can be green for a binary that isn't the one shipped. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4004 +/- ##
==========================================
- Coverage 61.24% 60.21% -1.04%
==========================================
Files 2153 2054 -99
Lines 188393 176647 -11746
==========================================
- Hits 115387 106360 -9027
+ Misses 62274 60516 -1758
+ Partials 10732 9771 -961
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@seidroid please re-review, all three findings are addressed in the last two commits.
|
|
Claude finished @monty-sei's task in 3m 14s —— View job Re-review — 2 of 3 findings closed, one new blocker from the fix
Findings: 2 blocking | 2 non-blocking Blockers1. The emulated boot hook still uses the native 25 s per-boot budget — The new hook runs When it doesn't fit, Two parts to the fix: # scripts/boot-smoke.sh — make the budget a parameter
BOOT_TIMEOUT=${BOOT_TIMEOUT:-25}
...
env $RAYON timeout -k 5 "$BOOT_TIMEOUT" "$BIN" start --home "$H" >"$LOG" 2>&1 || status=$?# .goreleaser.yaml — emulated boots get a generous budget
- BOOT_TIMEOUT=300 bash scripts/boot-smoke.sh build/seid-arm64 4And distinguish a timeout from a failure: 2. The exec probe traded a precise diagnosis for a broad one —
The last row is the one that costs you: a [ -x "$BIN" ] || { echo "boot-smoke: ERROR: $BIN is missing or not executable." >&2; exit 1; }
elf_machine=$(od -An -tx1 -j18 -N2 "$BIN" | tr -d ' \n')
case "$(uname -m)" in
x86_64) host_machine=3e00 ;;
aarch64|arm64) host_machine=b700 ;;
*) echo "boot-smoke: unsupported host architecture $(uname -m)" >&2; exit 1 ;;
esac
# A foreign binary is allowed, but only if binfmt can actually run it — that is how the
# release hook boots the arm64 binary on an amd64 runner.
if [ "$elf_machine" != "$host_machine" ] && ! "$BIN" version >/dev/null 2>&1; then
echo "boot-smoke: ERROR: $BIN is a foreign-architecture binary (e_machine $elf_machine) that this host cannot run; register binfmt first." >&2
exit 1
fiConfirmed fixed
Non-blockingThree comments now assert the opposite of what the code does. The last commit added the arm64 hook without updating the prose that existed to explain its absence:
Release wall-clock. No prompt-injection or instruction-shaped content found in the diff or comments. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d229059. Configure here.
f138c3b to
5eba408
Compare
|
@seidroid please re-review. Rebased onto current main (that also clears the stale Two things worth calling out, since both are corrections to fixes made during review rather than to the original PR: The arm64 boot hook could never have executed. goreleaser execs a before-hook directly rather than through a shell, so the BOOT_TIMEOUT was the cost of every boot, not a ceiling, since On the native-broken-binary case you flagged: the probe now runs for every binary and only the message branches on architecture. Verified each cause reports itself: missing path, non-executable, non-ELF, truncated header, unrunnable foreign binary, and a native binary whose Also re-ran the controls after the early-exit change, since it could have weakened the gate: the preserved b-tree binary is still caught on boot 1, the pinned binary still passes. |
|
Claude finished @monty-sei's task in 5m 5s —— View job Re-review — both blockers closed, no blockers remain
Findings: 0 blocking | 5 non-blocking Confirmed fixedBlocker 1 — Your correction here is the better one and it is worth stating plainly: Blocker 2 — classification before the probe.
Stale comments from last round — all three corrected: Non-blocking1. The comment justifies two boots; the hook runs four.
The hook passes 2. Nothing executes the A full 3. The crash grep now reads the log after a
I have not observed this — 8/8 on both architectures is real evidence it does not flake today — so this is hardening, not a defect. But the release hook is where a false positive costs a tag with no artefacts, and the fix is small: snapshot the log at the kill and grep the snapshot, keeping the full log for crash_log="$LOG"
...
# Snapshot before the TERM: shutdown noise from a signal delivered mid-startup
# is not the crash this gate looks for.
cp "$LOG" "$LOG.run"; crash_log="$LOG.run"
kill "$boot_pid" 2>/dev/null || true
...
if grep -qE "SIGSEGV|SIGILL|SIGBUS|panic:" "$crash_log"; then4. The six setup invocations per boot are silent and unbudgeted.
5. Release wall-clock — still worth confirming before merge. Raised last round, not addressed. Nits. goreleaser hooks accept a structured form ( No prompt-injection or instruction-shaped content in the diff or comments. |
|
@seidroid please re-review Finding 2 is the one worth looking at. Added Writing it turned up a bug in the check itself, worth mentioning since it is the same class: I used Finding 1 — you were right that the argument supports changing the code, not the comment. Hook is now 2 boots. Finding 3 — the crash grep reads a snapshot taken before the TERM. I re-ran the controls afterwards since the change could have weakened discrimination: the preserved b-tree binary is still caught on boot 1, the pinned binary still passes, two boots in 10s. Finding 5 — checked and it is a non-issue: Finding 4 left alone as pre-existing, happy to take it if you would rather it landed here. |
|
Claude finished @monty-sei's task in 4m 8s —— View job Re-review of
|
Already addressed, re-review didn't dismiss
Releases carried linux/amd64 only. The musl link path was arch-split in #3932, which made an arm64 static build possible; this wires it through the build script, the CI gate and goreleaser so the archive actually ships. Vendor the aarch64 Alpine 3.15 gcc 10.3.1 libgcc alongside the x86_64 one, each in its own subdirectory. The gcc>=12 unwind b-tree that crashed the amd64 binary at the genesis wasm store is not architecture specific: ATOMIC_FDE_FAST_PATH is gated on atomics support rather than a target allowlist, and an unpinned arm64 build SIGSEGVs on the first boot under RAYON_NUM_THREADS=1. Both targets need the pin. build-static.sh takes a target architecture and writes build/seid-<arch>, so the two builds no longer overwrite each other on one runner. The libgcc directory and the checksums it verifies are both derived from that argument, so a build cannot verify one architecture's archives while linking another's. It also asserts the ELF machine of the output matches what was asked for. The nm b-tree assertion now materialises the symbol table before grepping it. Reading nm through a pipe reported grep's exit status, so a failing nm would print "pre-b-tree unwinder confirmed" and pass. boot-smoke.sh refuses a binary built for another architecture. Previously it reached seid init, died with an exec-format error and reported "did not reach the ABCI handshake", which reads as a crashing binary rather than the wrong input. The release runner is amd64 and cannot execute the arm64 binary, so the goreleaser hook boots only the amd64 one. The arm64 boot gauntlet runs in the new Linux ARM64 (static) job on native hardware. The static build is reproducible, so the binary gated there is byte-identical to the one that ships. Verified with goreleaser release --snapshot: both archives are produced, both are listed in checksums.txt, sha256sum -c --ignore-missing reports OK for both, and each archive contains a binary of the matching architecture.
…b-tree check The binfmt hook ran a mutable tag of a third-party image with --privileged on the release runner, which holds the credentials that publish the binaries. Everything comparable in this repo is pinned: the golang build image by digest, every action by commit SHA, and the docker workflows use a pinned setup-qemu-action. Pin the image by digest so this hook is not the exception. The b-tree assertion was absence-only, so an empty or truncated symbol table satisfied it vacuously: a stripped binary reports zero b-tree symbols and the build prints "pre-b-tree unwinder confirmed". Assert first that the symbol table contains __register_frame, the entry point wasmer calls and one every libgcc provides whether pinned or not, so its absence means the table is unusable rather than the pin being wrong. Verified against a stripped copy of the arm64 binary: the old assertion passes it, the positive control rejects it.
…uild digests The arm64 binary that ships was never booted. Its safety rested on being byte-identical to the natively built binary the Linux ARM64 (static) job boots, and nothing enforced that. build-static.sh runs apk add build-base against a live branch index, so the toolchain can move underneath the pinned image digest and change the output without anything noticing. Boot the arm64 binary in the release hooks, under emulation, so the artefact that ships is the one booted. Emulated timing is not trusted to surface the gcc>=12 unwind b-tree, so the native arm64 job still runs the full gauntlet on real hardware; the release hook is a liveness check on the shipped bytes. boot-smoke.sh now probes whether the host can execute the binary rather than comparing its ELF machine to uname -m. The equality check would have rejected exactly the case the release hook needs, since binfmt makes a foreign binary runnable, and probing execution tests the thing that matters instead of a proxy for it. It still fails closed, with a message naming binfmt. build-static.sh prints the sha256 of each binary it produces, at the one point every caller passes through, so a CI build and a later release build of the same commit can be compared rather than assumed equal. Verified locally: an emulated boot completes well inside boot-smoke's 25s cap (amd64 on arm64, the mirror of the release runner's case), the native path still passes, and a binary the host cannot execute is rejected with exit 1.
boot-smoke bounded every boot at a fixed 25 seconds, sized for a native boot where the handshake lands about a second in. The release hook now boots the arm64 binary under emulation on an amd64 runner, where wasmer's JIT compile at the genesis wasm store is the slow part and emulation multiplies it. A healthy binary could be killed before reaching the handshake and fail the whole release. Introduce BOOT_TIMEOUT, defaulting to 25 so every existing caller is unchanged, and give the emulated arm64 hook 120 seconds. The real case cannot be measured on an arm64 workstation: only amd64 can be emulated here, possibly through Rosetta, while the release runner emulates arm64 under QEMU. The window is therefore set for headroom rather than tuned to a number that would not transfer.
…lf-describing The arm64 boot hook could never have run. GoReleaser execs a before-hook directly rather than through a shell, so the BOOT_TIMEOUT=300 prefix was taken as the program name: "exec: BOOT_TIMEOUT=300: executable file not found in $PATH". The before pipe aborts on that, so a release would have published a tag with no artefacts at all, including the amd64 tarball that ships today. Reproduced on goreleaser 2.15.4 and fixed by invoking through env. Gating the execution probe behind the foreign-architecture test left the case the gate exists to catch with no diagnostic: a native binary that cannot run its own subcommands skipped the probe, reached the unchecked seid init, and errexit killed the script with zero output. Run the probe for every binary and branch only the message, so a foreign binary is told to register binfmt and a native one reports that seid version failed, with its output. BOOT_TIMEOUT was the guaranteed cost of every boot rather than a ceiling, because seid start never exits on its own. A budget large enough to be safe under emulation was therefore also the price paid on every healthy boot. End a boot shortly after the handshake appears, keeping a brief dwell so a crash just after it still lands in the log. Two boots of the release binary now take 11s where they took ~50s, which is what makes 4 boots at a 300s ceiling affordable. Read the ELF header once and length-check it, so a truncated file reports itself instead of dying inside od. Verified: the preserved b-tree binary is still caught on boot 1, the pinned binary still passes, and each of missing path, non-executable, non-ELF, truncated header, unrunnable foreign binary and broken native binary reports its own cause.
…ot gate Nothing in CI runs goreleaser, so every entry in before.hooks is an unexecuted string until a tag is pushed, and this change grew that list from two entries to six. The failure that reached review was argv[0] not resolving, which is checkable in milliseconds: shell-split each entry and assert command -v finds the program. Runs in the amd64 static job, which the .goreleaser.yaml path filter already routes. Verified by reintroducing the original defect, which the check rejects and names the fix for. The arm64 hook ran four boots while the comment beside it argued for two. The argument is the correct one: boot 1 is the single-threaded repro and the rest are identical default-threaded repeats of what the native arm64 job already does, so the code follows the comment rather than the other way round. The crash grep now reads a snapshot taken before the boot is terminated. The signal arrives seconds after the handshake now rather than at a settled node, so shutdown output from a half-started node would otherwise be read as a crash. Not observed, but a false positive here costs a tag with no artefacts. Restores the "To re-derive:" lead-in lost from the libgcc provenance sections. Verified after the change: the preserved b-tree binary is still caught on boot 1, the pinned binary still passes, two boots in 10s.
The check resolved argv[0] and stopped, so `env BOOT_TIMEOUT=300 bash ...` passed on `env` being present and the real program was never looked at. The one hook that needed `env`, and the one the check was written for, was the one hook exempt from it: `env BOOT_TIMEOUT=300 bahs scripts/boot-smoke.sh` would have passed and still aborted the release. Resolve past `env` and the VAR=value operands it consumes. Four of the six hooks only proved that bash exists. When argv[0] is an interpreter, check that the script it runs is readable, so a renamed path fails here rather than at tag time. builds[].tool is exec'd the same way and is now covered too, taking the config from one hook meaningfully checked to seven. The script dropped mapfile for the bash macOS ships, then depended on python3 with PyYAML, which stock macOS python3 does not have: on the machine the comment named, it was more likely to fail than what it replaced, as a bare traceback. Check for the import and say what is missing. Globbing is disabled around the split so a hook containing * is not expanded against the working directory. The six per-boot setup invocations discarded both streams and run outside timeout, so a failure aborted under set -e with no output, which from a goreleaser hook is an unexplained non-zero exit. Each step now names itself and tails its log. This PR is what first runs them under emulation, where they are the same slow path as the boot. Verified: dropping env, a typo after env, a renamed script, and env with no command are each rejected with their own message; a failing setup step reports which one and why; the preserved b-tree binary is still caught on boot 1 and the pinned binary still passes.
45ca1ee to
545d44e
Compare
|
@seidroid review |
There was a problem hiding this comment.
Well-executed arm64 arch-split: the per-arch libgcc pin (checksums verified against the vendored files), the ELF-machine guards in build-static.sh / boot-smoke.sh / goreleaser-shim.sh, and the new hook-executability check all fail closed, and both prior review findings are genuinely fixed. Two non-blocking notes: the newly-added digest print can't support the cross-run comparison its comment claims, and the fail-fast hook check runs after the slow build.
Findings: 0 blocking | 2 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| *) echo "build-static: ERROR: $OUT is not a linux/$ARCH binary" >&2; exit 1 ;; | ||
| esac | ||
|
|
||
| # Record the digest so a CI build and a later release build of the same commit can be |
There was a problem hiding this comment.
[suggestion] The stated purpose — comparing "a CI build and a later release build of the same commit" — can't be met, because the digests differ for reasons unrelated to toolchain drift.
make build bakes version.Version=$(VERSION) and version.Commit=$(COMMIT) into the ldflags (Makefile:70-71). VERSION derives from the branch name and git describe --tags; under actions/checkout the workspace is a detached HEAD at fetch-depth 1, so BRANCH_NAME is HEAD and git describe --tags finds nothing, leaving VERSION empty in every CI static job. At release time goreleaser runs on a tag, so VERSION is the tag string. The two binaries therefore never match byte-for-byte, even on the same commit, and a mismatch tells an operator nothing about whether apk add build-base picked up a different gcc.
The line still has value for run-to-run comparison within one context (two CI runs on the same PR head, or a release rerun on the same tag), which is where apk drift would actually show. Suggest narrowing the comment to that, or — if the cross-context comparison is the goal — pinning the apk package versions instead, which is the only thing that makes the CI-vs-release link enforceable rather than observed.
|
|
||
| # goreleaser is never run by CI, so its hooks are unexecuted strings until a tag is | ||
| # pushed. This catches an unrunnable hook in milliseconds instead. | ||
| - name: Verify goreleaser hooks are executable |
There was a problem hiding this comment.
[suggestion] The step's own comment says it "catches an unrunnable hook in milliseconds instead", but it is placed after Build statically (Alpine / musl), which is a full Alpine apk add build-base plus a cgo make build — roughly ten minutes. A .goreleaser.yaml-only change (the exact case this gate exists for, and one the static-paths pattern deliberately matches) pays that cost before hearing about a typo'd hook.
Moving the step directly after Checkout code gets the advertised latency and costs nothing: check-goreleaser-hooks.sh only reads .goreleaser.yaml and probes PATH, with no dependency on the build output.
|
Successfully created backport PR for |
2 participants
Releases carry
linux/amd64only. #3932 arch-split the musl link path, which made an arm64 static build possible; this wires it through the build script, the CI gate and goreleaser so the archive actually ships.Verified end to end
goreleaser release --snapshoton this branch:Both archives are listed in
checksums.txt,sha256sum -c --ignore-missingreports OK for both, and each archive contains a binary of the matching architecture. That last check matters because--ignore-missingexits 0 silently for a file that is present but unlisted, so an out of band upload would give operators a verification step that verifies nothing.The toolchain pin is needed on both architectures
The gcc>=12 unwind b-tree that crashed the amd64 binary at the genesis wasm store is not amd64 specific.
ATOMIC_FDE_FAST_PATHis gated on atomics support rather than a target allowlist, and an unpinned arm64 build SIGSEGVs on the first boot underRAYON_NUM_THREADS=1, reproduced on native arm64 hardware. So the Alpine 3.15 gcc 10.3.1 libgcc is now vendored for both, each in its own subdirectory, with provenance and checksums in the README.With the pin applied, the arm64 binary carries zero b-tree symbols and boots 8/8 clean at 4 CPUs.
Build script
build-static.shtakes a target architecture and writesbuild/seid-<arch>, so the two builds do not overwrite each other on one runner. The libgcc directory and the checksums it verifies are both derived from that one argument, so a build cannot verify one architecture's archives while linking another's. It also asserts the ELF machine of the output matches what was requested.Two guards that were failing open are now closed:
nmb-tree assertion materialises the symbol table before grepping. Readingnmthrough a pipe reported grep's exit status, so a failingnmprinted "pre-b-tree unwinder confirmed" and passed.boot-smoke.shrefuses a binary built for another architecture. It previously reachedseid init, died with an exec-format error, and reported "did not reach the ABCI handshake", which reads as a crashing binary rather than the wrong file being passed in.Where the arm64 binary gets booted
The release runner is amd64 and cannot execute the arm64 binary, so the goreleaser hook boots only the amd64 one. The arm64 8-boot gauntlet runs in the new
Linux ARM64 (static)job on nativeubuntu-24.04-armhardware.The link between them is reproducibility: the static build produces a byte-identical binary for a given commit (measured, identical
sha256across independent runs), so the binary CI boots is the one that ships. The release job builds arm64 under emulation via binfmt, which costs roughly 45 minutes and is why the boot gate lives on native hardware instead.Notes for review
ucichange needed. Docker and privileged runs already work on the release runner, so binfmt registration is just anotherbefore:hook.