Clear what CI found the first time it ran - #3
Merged
Conversation
Three things, all surfaced by gates added last sprint and none of them visible before. govulncheck reported 22 reachable vulnerabilities across 8 modules — reachable meaning it found a call path from this code, not merely a version in go.mod. Ten were in x/crypto, which the server uses for password hashing and SSH. Upgrading x/crypto, x/text, go-git, go-billy, lego, goldmark, circl and grpc takes it to zero. Two remain in modules that are required but never called, which govulncheck reports separately and does not fail on. The gate found these on its first run. They were there the whole time. Two Windows failures, both in tests I wrote rather than in the code: TestTheFirstRunIsSilent left the background refresh running and returned, so the goroutine wrote its state file into a directory the test framework was already removing. Windows reports that as "the directory is not empty"; Unix hides it. The same mistake was fixed in the neighbouring test last sprint and missed here. TestAKeyRoundTripsBackToItsPath asserted that a backslash survives a round trip through an S3 key. It does not, and should not: CleanPath treats a backslash as a separator on every OS so a path is judged identically wherever a server, a client or a shared backend sees it, and sanitizePath reaches that through filepath.ToSlash — which only rewrites on Windows. The case encoded a Unix-only assumption. Removed, with the reasoning, rather than made conditional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two CI failures, neither about the code. golangci-lint was pinned to v1.62, whose binary is built with Go 1.23. It refuses to load a module targeting 1.25 and says so plainly — "the Go language version used to build golangci-lint is lower than the targeted Go version" — so the gate added last sprint could never have passed, and the local run that convinced me otherwise was a newer binary than the one CI installs. v2 with the v2 config schema. The migration is not a rename. In v2 staticcheck absorbs stylecheck, so checks: ["all"] would pull in naming and comment-formatting opinions; the setting is dropped and the defaults used, because a gate that argues about taste is a gate somebody turns off. The five findings the new default reports are fixed rather than excluded: De Morgan on a character-class test, four WriteString(Sprintf) that are Fprintf, and a bare 502 that is http.StatusBadGateway. The control plane pinned Go with go-version-file: go.mod, which setup-go honours literally — go.mod says 1.25.0, and the standard-library CVEs its vulnerability check reports are fixed in 1.25.10 through 1.25.12. The gate was telling the truth about a toolchain nobody meant to pin. It now takes the newest patch of that line, the way contextd already did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The gates added in sprint 4 ran for the first time on the sprint-5 merge and
failed. Everything here is what they found, plus the two ways the gates
themselves were wrong.
Dependencies
govulncheck reported 22 reachable vulnerabilities across 8 modules —
reachable meaning it found a call path from this code, not merely a version in
go.mod. Ten were inx/crypto, which the server uses for password hashing andSSH. Upgrading
x/crypto,x/text,go-git,go-billy,lego,goldmark,circlandgrpctakes it to zero.Two Windows failures, both in tests rather than code
TestTheFirstRunIsSilentleft its background refresh running and returned, sothe goroutine wrote into a directory the test framework was removing. Windows
reports that; Unix hides it. The same mistake was fixed in the neighbouring
test last sprint and missed here.
TestAKeyRoundTripsBackToItsPathasserted a backslash survives an S3 keyround trip. It does not and should not:
CleanPathtreats a backslash as aseparator on every OS, reached through
filepath.ToSlash, which only rewriteson Windows. The case encoded a Unix-only assumption.
The gates themselves
golangci-lintwas pinned to v1.62, whose binary is built with Go 1.23 andrefuses to load a module targeting 1.25. It could never have passed — the local
run that suggested otherwise used a newer binary than CI installs. Migrated to
v2 and the v2 config schema.
That migration is not a rename: in v2
staticcheckabsorbsstylecheck, sochecks: ["all"]would pull in naming opinions. The setting is dropped and thedefaults used. The five findings the defaults report are fixed rather than
excluded — De Morgan on a character-class test, four
WriteString(Sprintf)that are
Fprintf, and a bare502that ishttp.StatusBadGateway.Verified
All three OS matrices, integration, analysis and lint green on this branch.