Repository navigation
ci: build and release with Go 1.27.2 - #56
k4rthiknerella wants to merge 2 commits into
Conversation
b641180 to
f6a5113
Compare
0e4fbad to
929ccf1
Compare
lutic1
left a comment
There was a problem hiding this comment.
@k4rthiknerella, please fix these before this merges.
Blocking
.github/workflows/release.yaml:82: under go1.27.2,govulncheck@v1.1.4panics (unexpected expr: *ast.KeyValueExprin x/tools v0.29.0go/ssa) and exits non-zero. Every tag push would stop before GoReleaser runs. The description says this command exits 0 under 1.27.2; it reproduced as a crash twice. Bump it togovulncheck@v1.9.0, which exits 0 with "No vulnerabilities found" on 1.27.2. Please also run the same govulncheck line inci.yaml, so a toolchain bump tests it before a tag does.
Should fix
CONTRIBUTING.md:28still says "Go 1.25 or later".go.modkeepsgo 1.25.0, but no CI job builds with 1.25 any more, so a 1.25-only break would go unseen. Either say CI covers only the pinned toolchain, or add a cheap build job on the minimum version.
All four 1.27.2 pins agree, action SHAs are unchanged, build/vet/test pass on 1.25.0 and 1.27.2, and the DEL-17/DEL-21 mutants go red.
Generated by Claude Code
lutic1
left a comment
There was a problem hiding this comment.
@k4rthiknerella, this is a follow-up to the earlier review: an adversarial pass over this PR. Please fix each item, or reply on the PR saying why it doesn't apply. The earlier govulncheck blocker still stands.
Vulnerability gate
release.yaml:81-82: the comment justifies the bump with govulncheck v1.9.0 measurements, but the gate still runs v1.1.4. Default mode fails only on reachable vulnerabilities, and that blind spot is what let go1.25.8 through. Pin v1.9.0, and if the policy is "no known stdlib advisories", enforce it (-scan module, or fail on any stdlib finding).- No job in
ci.yamlruns govulncheck, so PRs and pushes to main never check for vulnerabilities. Add the pinned step to the test job, plus a scheduled run so new advisories surface between releases. - Nothing reminds anyone when the pinned Go major leaves support: Dependabot doesn't update
go-versionstrings, and DEL-21 checks only that pins agree. Add a scheduled check againstgo.dev/dl/?mode=json, or Renovate's golang-version manager.
Toolchain floor
go.modkeepsgo 1.25.0and CONTRIBUTING says "1.25 or later". The PR itself calls 1.25 unsupported, and no job builds with it. Add a 1.25.x build/test job, or raise the floor and explain why.- Release binaries jump from 1.25 to 1.27. Check the 1.26/1.27 "Ports" notes (minimum macOS, Windows and kernel versions) against the six release targets, and record the result.
DEL-21 / DEL-17 parser (test/mutation/sweep_test.go)
-
goVersionPinsmatches pins to setup-go steps by count only. A setup-go step with no pin plus a straygo-version:line elsewhere passes. Tie each pin to its own step'swith:block. -
The setup-go regex misses quoted
uses: "actions/setup-go@…", and it never scans.github/actions/**/action.yml. -
Any quoted string counts as a pin, including
"stable","1.27.x"and">=1.27". Require exactly^\d+\.\d+\.\d+$, and add fixtures for those cases. -
Pins are never compared with
go.mod'sgodirective. Assert every pin is at least that version. -
jobsAndMissingTimeouts:- It still skips lines at an unexpected indent (3 spaces).
- It credits a 4-space
timeout-minutesline to the previous job. - It enters the jobs block only on an exact
jobs:line, so a trailing comment or CRLF breaks it.
Normalise the line endings and add fixtures for each case.
-
DEL-21_go-version-pins.py: no mutant makesrelease.yamldisagree withci.yaml, or disables the quoted-pin orgo-version-filechecks. The anchor also relies on the prose comment# Same as in Test. -
The file header still lists only DEL-1/2/17, and no spec row exists anywhere for DEL-21.
Process
- The same long rationale, with hard-coded figures, is copied word for word into
ci.yamlandrelease.yaml. Keep one copy, have the other point to it, and mark the old go1.25.8 paragraph as history. - There's no evidence the mutation sweep ran under 1.27.2. Run it on this branch by hand and link the run.
Generated by Claude Code
The workflows pinned the build toolchain at 1.25.8, chosen in September because 1.25.6 left two reachable standard-library vulnerabilities in the binary and 1.25.8 cleared them. Go fixes security issues in the two most recent majors only, and with 1.27 out that is 1.26 and 1.27: 1.25 gets nothing more, however new its patch. Measured with govulncheck (v1.9.0, the same scanner for every run, on the 2026-10-08 database): under go1.25.8 the module's dependency graph carries 39 standard-library advisories, none reachable from its own code; the newest 1.25 patch, 1.25.14, would clear 26 of them and leave the 13 issued since 1.25 left support, every one fixed only in 1.26.9 or later; under go1.27.2 there are none. The pin now follows the newest stable release of a supported major. Four pins move: the test, cross-build and on-demand sweep jobs in ci.yaml, and release.yaml. The build toolchain is pinned by hand in each job, so a bump that misses one leaves that job building with a toolchain nobody chose; TestDEL21_GoVersionPinsAgree now reads every setup-go step across the workflows and fails unless each has a go-version and all agree, and a sweep mutant drifts one pin to show it bites. The guard's first parsers failed open twice, found in review: the pin parser opened a setup-go step only on `- uses:` and never saw the name-first form, and the timeout parser did not take a job key with a trailing comment for a job, so its bound was credited to the job above. Both now count what they see in any form and refuse what they do not recognise, with fixtures for each, because a real-tree test cannot go red on a well-formed tree, and a mutant for each refusal. go.mod's `go 1.25.0` is the language minimum and is unchanged; there is still no toolchain directive. Under 1.27.2 gofmt lists nothing, vet passes natively, under -tags rashomonfault and for GOOS=windows, the suite passes with the race detector, and `go mod tidy -diff` is empty. Both workflow comments also said go.mod read `go 1.24.0`; it has read 1.25.0 since the module took its dependency.
Under go1.27.2, govulncheck v1.1.4 panics in go/ssa
("unexpected expr: *ast.KeyValueExpr", exit 2) when it analyses
linux/amd64, so every tag push would have stopped before GoReleaser. On
darwin it never reaches that pass and exits 0. v1.9.0 exits 0 with "No
vulnerabilities found." on the same tree for linux/amd64.
The release job moves to v1.9.0, and the test job now runs the same
line, so a toolchain bump that breaks the scanner fails on a pull
request instead of on a tag. TestDEL21c requires exactly one govulncheck
step in each of ci.yaml and release.yaml and one version across every
workflow; a sweep mutant drifts the test job's copy to show it bites.
f6a5113 to
a4f5d75
Compare
|
Thanks for the careful pass. Point by point: Blocking 1. Confirmed. Under go1.27.2, v1.1.4 panics with Blocking 1 (ci.yaml). Done in the same commit, "ci: run govulncheck v1.9.0 in the test job and the release job". The Test job runs the same govulncheck@v1.9.0 line. TestDEL21c_GovulncheckRunsInCIAtTheReleaseVersion requires one govulncheck step in each workflow at a single version, so the next toolchain bump fails on a PR, not on a tag. Should fix 1. Fair. I'll add a line to CONTRIBUTING saying go.mod's Summary. Thanks for checking the pins and the mutants. The DEL-21 file has a third mutant now (the test job's govulncheck drifting), and it goes red as well. |
lutic1
left a comment
There was a problem hiding this comment.
@k4rthiknerella, I re-reviewed the PR at a4f5d75. The blocker is fixed: govulncheck@v1.9.0 exits 0 with "No vulnerabilities found" under go1.27.2. The same step now runs in the Test job, and TestDEL21c holds the two workflows to one version. The DEL-21 sweep turns all 3 mutants red.
Still open
- The CONTRIBUTING change was never pushed. Your reply says "I'll add a line", but
CONTRIBUTING.md:28still says "Go 1.25 or later", and no job builds with 1.25. - The description is stale. It says "1098 mutants in 55 files", but the rebased tree has 1111 mutants in 69 files. The commit message and the description also say every review point is handled, but the adversarial pass hasn't been answered.
Adversarial pass: no reply yet. Please fix or answer each item. The ones that matter most:
- govulncheck runs in default mode only, so the "no known stdlib advisories" policy isn't enforced (
-scan module). - There's no scheduled vulnerability run, and nothing warns when the pinned Go major leaves support.
- The Go 1.26/1.27 Ports notes haven't been checked against the six release targets.
goVersionPins:- It matches pins to steps by count only (
sweep_test.go:798). The description now admits this, but it isn't fixed. - It misses quoted
uses:lines. - It accepts
"stable"and"1.27.x". - It never compares pins to
go.mod.
- It matches pins to steps by count only (
jobsAndMissingTimeouts:- It skips 3-space lines.
- It credits a 4-space timeout to the previous job.
- It requires
jobs:exactly, so CRLF breaks it.
- DEL-21 has no mutant for release.yaml disagreeing with ci.yaml.
- The file header and the spec row for DEL-21 are missing.
- The rationale is copied word for word between
ci.yamlandrelease.yaml.
Generated by Claude Code
Stacked on #55 (mutation: one file per change, every anchor exactly once); review and merge after it.
The workflows pinned the build toolchain at 1.25.8, chosen in September because 1.25.6 left two reachable standard-library vulnerabilities in the binary and 1.25.8 cleared them. Go fixes security issues in the two most recent majors only, and with 1.27 out that is 1.26 and 1.27: 1.25 gets nothing more, however new its patch. The pin now follows the newest stable release of a supported major, and the workflow comment gives the reason in numbers instead of "bump this when an advisory lands".
go.mod'sgo 1.25.0is the language minimum and is unchanged; there is still notoolchaindirective. Both workflow comments also said go.mod readgo 1.24.0; it has read 1.25.0 since the module took its dependency.What changed
.github/workflows/ci.yamlgo-version: "1.27.2"intest,cross-buildand the on-demandsweepjob; the comment's minimum corrected; the measurement below.github/workflows/release.yamlgo-version: "1.27.2"; the same comment; govulncheck moves from v1.1.4 to v1.9.0.github/workflows/ci.yaml(test job)govulncheckstep, the samegovulncheck@v1.9.0 ./...line the release job runstest/mutation/sweep_test.goTestDEL21_GoVersionPinsAgree: everysetup-gostep carries ago-version, and every pin across the workflows is one version.TestDEL21b_PinParserFailsClosedandTestDEL17b_TimeoutParserFailsClosed: fixtures for the two workflow parsers.TestDEL21c_GovulncheckRunsInCIAtTheReleaseVersion: ci.yaml and release.yaml each run govulncheck once, at one versiontest/mutation/mutants/DEL-21_go-version-pins.pytest/mutation/mutants/DEL-17_job-timeouts.pyEach job pins the build toolchain by hand, so a bump that misses one job leaves that job building with a toolchain nobody chose. That already happened once between two branches, which is why the test exists. No other Go source changes: under 1.27.2 gofmt lists nothing, and vet passes natively, under
-tags rashomonfaultand forGOOS=windows.This branch is stacked on the mutation per-change-files change, whose sweep job is the fourth pin; merge that first, then this. Later changes in the stack add perf jobs that pin 1.25.8; when they merge, those pins move to 1.27.2, and
TestDEL21_GoVersionPinsAgreeholds them to it.Rows: DEL-21 is new (
TestDEL21_GoVersionPinsAgree,TestDEL21b_PinParserFailsClosed,TestDEL21c_GovulncheckRunsInCIAtTheReleaseVersion, three mutants); DEL-17 gainsTestDEL17b_TimeoutParserFailsClosedand a mutant.Evidence
govulncheck, one scanner, three toolchains
govulncheck v1.9.0 built with go1.27.2, database of 2026-10-08,
-show verbose, run from the module root withGOTOOLCHAINset to each toolchain. Each run scans the same 11 modules and that toolchain's standard library. All three exit 0, and none finds a vulnerability reachable from the module's code.The 13 that no 1.25 patch fixes
Each is "Found in" the go1.25.14 standard library and "Fixed in" go1.26.9 or later:
release.yamlused to rungovulncheck@v1.1.4at the tag. I first measured that under go1.27.2 on darwin, where it exits 0, but darwin never reaches its SSA pass. Analysing linux/amd64, which is what the release runner does, v1.1.4 panics in x/tools v0.29.0go/ssaand exits 2:So every tag push would have stopped before GoReleaser. v1.9.0 on the same tree for linux/amd64 prints
No vulnerabilities found.and exits 0. The release job now runs v1.9.0, and the CI test job runs the same line, so the next toolchain bump that breaks the scanner fails on a pull request. A tag does not run CI, which is how this one got through.A govulncheck built with Go 1.25 cannot load a 1.27 standard library: its type checker rejects source that 1.27 permits (
math/rand/v2/rand.go:213:17: method must have no type parameters,net/url/url.go:1348:13: u is not a type), and the scan fails before it starts. Build the scanner with the toolchain it scans:The parsers fail closed
Both workflow parsers in
sweep_test.gofirst failed open. The pin parser opened a step only on- uses:and never saw the name-first form, so a step written as- name:thenuses:could carry an old pin unseen. The timeout parser did not treat a job key with a trailing comment as a job, so that job's bound was credited to the job above. Both now count what they see in any form and refuse what they do not recognise. A real-tree test cannot go red on a well-formed tree, so each parser has fixtures and a mutant. A name-first step at 1.25.8 beside a- uses:step at 1.27.2 now fails:The guard is green on the branch. A hand-made drift of
release.yamlto"1.27.1"turns it red withgo-version pins disagree: ci.yaml: 1.27.2 vs release.yaml: 1.27.1.Mutation sweep over the two mutant files this PR adds or changes.
python3 test/mutation/sweep.py --files 'mutants/DEL-17*': baseline green, both DEL-17 mutants red, undetected: none. All three DEL-21 mutants also go red, each applied alone with its named test run.python3 test/mutation/sweep.py --check: 1098 mutants in 55 files; every anchor occurs exactly once.The same tree under go1.27.2
go mod tidy -diff: prints nothing.gofmt -l .: lists nothing.go build ./...,go vet ./...,go vet -tags rashomonfault ./...,GOOS=windows go vet ./...: pass.go test -race ./...: every package ok,test/acceptance47.137s,test/mutation3.689s. The same suite on the untouched base tree under go1.27.2 also passes (test/acceptance64.509s).Race suite output under go1.27.2
What it does not do, and what it costs
Runtime defaults do not change.
go list -f '{{.DefaultGODEBUG}}' ./cmd/rashomonunder 1.27.2 printscryptocustomrand=1,httpservecontentmaxranges=1,tlssecpmlkem=0,tracebacklabels=0,urlstrictcolons=0,x509sslcertoverrideplatform=0. The six settings that 1.26 and 1.27 changed stay at their 1.25 values, because go.mod'sgo 1.25.0fixes them. The security fixes ship as code, not behind GODEBUG, so all 39 clear while no default behaviour changes (url.Parseininternal/hoststays lenient, as today). The 1.26 and 1.27 defaults arrive only when go.mod's go line moves, which needs its own review.Known limit of the pin guard. The count balances per file, not per step: a pinless
setup-gostep beside a quotedgo-version:elsewhere in the same file (a matrix key, or another action's input) is not caught. A real matrix pin is${{ matrix.go-version }}, which is unquoted and already refused. Closing the gap means counting a pin only inside a step whose most recentuses:issetup-go, with the step reset at each-line. This PR leaves the gap open.Cost. Peak memory of one
rashomon hookcall (short Bash PreToolUse, release flags, darwin/arm64, 40 interleaved runs each): go1.25.8 median 10.70 MiB (p10 10.56, p90 10.86); go1.27.2 median 11.17 MiB (p10 10.97, p90 11.28). That is about 0.47 MiB more per process, and the two spreads do not overlap. Binary size goes from 7,799,090 to 8,188,738 bytes (+5.0%). The new garbage collector is not the cause: withGOEXPERIMENT=nogreenteagcunder 1.27.2 the median is 11.17 MiB, against 11.14 with it on (n=30 each). The cause is not identified. The memory budgets that the next changes in the stack set are to be measured under this pin.