fix: Bump Go 1.26.1 to 1.26.2 for 6 stdlib CVEs#2170
Conversation
…ities Fixes GO-2026-4865 (html/template), GO-2026-4866 (crypto/x509), GO-2026-4869 (archive/tar), GO-2026-4870 (crypto/tls), GO-2026-4946 (crypto/x509), GO-2026-4947 (crypto/x509). Updates go.mod, all Dockerfiles, and all CI workflow files.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBump Go toolchain from 1.26.1 → 1.26.2 across CI workflows, Docker builder images, Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Claude Code ReviewCommit: SummaryClean security bump from Go 1.26.1 to 1.26.2, resolving 6 stdlib CVEs (html/template injection, crypto/x509 chain building, archive/tar path traversal, crypto/tls handshake, 2x crypto/x509 policy validation). Also bumps golangci-lint v2.9.0 to v2.11.4 and fixes lint issues surfaced by the upgrade. Version updates are comprehensive: go.mod, 19 Dockerfiles, 16 CI workflows, 2 docs. No stale 1.26.1 references remain. The most significant non-mechanical change is Existing test coverage ( Risk Assessment
FindingsNo open findings. Bot Review Notes
Previously Flagged
|
Addressed - all 16 service Dockerfiles updated to Go 1.26.2
Aligns CI with Go 1.26.2 contextcheck behavior. v2.9.0 had stale contextcheck heuristics that flagged existing nolint directives as unused, while v2.11.4 correctly identifies them as necessary.
There was a problem hiding this comment.
Clean patch-version bump. Go 1.26.1 to 1.26.2 resolving 6 stdlib CVEs, plus golangci-lint v2.9.0 to v2.11.4. All version-pinning locations updated comprehensively (go.mod, 19 Dockerfiles, 16 CI workflows, 2 docs). No code changes, safe to rollback. See summary comment for full details.
- Migrate proxy.Director to proxy.Rewrite (deprecated since Go 1.26) - Use fmt.Fprintf instead of WriteString(fmt.Sprintf(...)) - Exclude noctx linter for test files (httptest.NewRequest creates a valid context; requiring WithContext in tests is noise) - Remove now-unused nolint:noctx directive in bench test - Add nolint:prealloc for defensive-copy test
Accidentally replaced the em dash with a hyphen during the fmt.Fprintf refactor, breaking TestBuildTopicList_TopicsHaveDescriptions.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/api-gateway/proxy.go`:
- Around line 68-71: The proxy returned by httputil.NewSingleHostReverseProxy
(created as proxy) still has its default Director set, causing a runtime
conflict when you assign proxy.Rewrite; clear the Director field before setting
Rewrite to satisfy ReverseProxy's mutual exclusivity. Locate where proxy is
created with NewSingleHostReverseProxy and set proxy.Director = nil (or
remove/reset it) prior to assigning proxy.Rewrite in the same scope so only
Rewrite is populated.
- Around line 72-73: The rewrite currently calls r.SetURL(target) which changes
the outbound Host header and breaks parity with NewSingleHostReverseProxy; fix
by preserving the original inbound Host: capture the incoming host before
calling r.SetURL (e.g., originalHost := r.Host or r.Header.Get("Host")), call
r.SetURL(target) and r.SetXForwarded() as before, then explicitly restore the
request Host for the outbound request by setting r.Host = originalHost (and if
needed r.Header.Set("Host", originalHost)) so backends see the original
authority while still setting X-Forwarded-Host.
In `@services/control-plane/internal/generator/topic_list.go`:
- Line 41: The test TestBuildTopicList_TopicsHaveDescriptions expects an em dash
separator but the implementation uses a hyphen; update the formatting in
topic_list.go where fmt.Fprintf(&sb, "- `%s` - %s\n", topic, desc) is used to
restore the em dash (use " — " between the topic and description) so the
generated line matches the test expectation, or alternatively update the test
assertion to look for " - " if you intentionally want a hyphen (referenced
symbols: fmt.Fprintf call in topic_list.go and
TestBuildTopicList_TopicsHaveDescriptions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5b99cd8-9ca1-4b7f-8189-c9f0800e8440
📒 Files selected for processing (7)
.golangci.ymlservices/api-gateway/proxy.goservices/api-gateway/transcoding_bench_test.goservices/control-plane/internal/generator/handler_reference.goservices/control-plane/internal/generator/llm_client.goservices/control-plane/internal/generator/topic_list.goservices/position-keeping/domain/transaction_lineage_test.go
✅ Files skipped from review due to trivial changes (4)
- services/api-gateway/transcoding_bench_test.go
- services/control-plane/internal/generator/handler_reference.go
- services/position-keeping/domain/transaction_lineage_test.go
- services/control-plane/internal/generator/llm_client.go
- Use fmt.Fprintf in saga validation report (4 instances) - Preserve Host header in proxy.Rewrite (SetURL overwrites it)
Addressed - Host header preserved, em dash restored, lint fixes applied
…Director Replace NewSingleHostReverseProxy + Director=nil with a direct ReverseProxy struct literal using only the Rewrite field. This avoids both the deprecated Director API and the mutual exclusivity error. Preserves existing X-Forwarded-Host from incoming requests.
There was a problem hiding this comment.
Comprehensive and correct Go 1.26.2 security bump. All 46 version-pinned files updated, no stale references. The proxy.go Director-to-Rewrite migration is behaviorally equivalent with correct Host and X-Forwarded-Host handling. All previous review findings resolved. See summary comment for full details.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Verified locally:
govulncheck ./...now reports only the 3 pre-existing allowlisted vulns (Dex, Docker/Moby).Test plan