Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request comprehensively updates the project's Go dependencies to their latest stable versions, primarily driven by the need to address known security vulnerabilities and improve overall stability. A critical CVE in the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Updates vendored Go dependencies to address reported vulnerabilities (notably upgrading filippo.io/edwards25519) and refreshes CI to add caching, multi-version testing, linting, and vulnerability scanning.
Changes:
- Bump direct/indirect module versions and refresh
vendor/(includingx/sys,x/crypto,stats,go-sqlite3,color,edwards25519). - Update GitHub Actions workflows to enable Go build caching and add build matrix, lint, and govulncheck jobs.
- Sync
go.mod/go.sumandvendor/modules.txtwith the new dependency set.
Reviewed changes
Copilot reviewed 4 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/modules.txt | Reflect updated vendored module versions and their minimum Go versions. |
| vendor/golang.org/x/sys/windows/syscall_windows.go | Vendor update: removes local Signal implementation in favor of aliasing. |
| vendor/golang.org/x/sys/windows/aliases.go | Vendor update: adds type Signal = syscall.Signal alias. |
| vendor/golang.org/x/sys/unix/ztypes_linux.go | Vendor update: updates Linux types/constants and adds prefix-related structs. |
| vendor/golang.org/x/sys/plan9/syscall_plan9.go | Vendor update: aliases Note to syscall.Note. |
| vendor/github.com/montanaflynn/stats/percentile.go | Vendor update: changes Percentile implementation to NIST linear interpolation method. |
| vendor/github.com/montanaflynn/stats/doc.go | Vendor update: updates copyright year range. |
| vendor/github.com/montanaflynn/stats/README.md | Vendor update: updates copyright year range. |
| vendor/github.com/montanaflynn/stats/LICENSE | Vendor update: updates copyright year range. |
| vendor/github.com/montanaflynn/stats/DOCUMENTATION.md | Vendor update: updates copyright year range. |
| vendor/github.com/montanaflynn/stats/CHANGELOG.md | Vendor update: adds v0.8.0 changelog + link updates. |
| vendor/github.com/montanaflynn/stats/.goreleaser.yml | Vendor update: adds goreleaser config file. |
| vendor/github.com/montanaflynn/stats/.github-write-test | Vendor update: adds upstream file present in new vendored version. |
| vendor/github.com/mattn/go-sqlite3/sqlite3_opt_serialize.go | Vendor update: switches unsafe slice handling to unsafe.Slice. |
| vendor/github.com/mattn/go-sqlite3/sqlite3_opt_percentile.go | Vendor update: adds build tag to enable SQLite percentile feature. |
| vendor/github.com/mattn/go-sqlite3/sqlite3.go | Vendor update: refactors Close/Stmt Close and clears bindings before rebinding. |
| vendor/github.com/mattn/go-sqlite3/sqlite3-binding.h | Vendor update: bumps bundled SQLite version metadata. |
| vendor/github.com/mattn/go-sqlite3/sqlite3-binding.c | Vendor update: bumps bundled SQLite amalgamation and related logic changes. |
| vendor/github.com/fatih/color/color_windows.go | Vendor update: guard against nil os.Stdout on Windows. |
| vendor/github.com/fatih/color/color.go | Vendor update: centralizes stdout/stderr handling and improves writer set/unset accounting. |
| vendor/filippo.io/edwards25519/tables.go | Vendor update: import formatting simplification. |
| vendor/filippo.io/edwards25519/scalar.go | Vendor update: constant-time reduced check using math/bits. |
| vendor/filippo.io/edwards25519/pull.sh | Vendor update: adds upstream helper script for syncing from Go repo history. |
| vendor/filippo.io/edwards25519/field/fe_generic.go | Vendor update: arithmetic refactor and carry propagation changes. |
| vendor/filippo.io/edwards25519/field/fe_arm64_noasm.go | Vendor update: removes arm64-specific noasm shim. |
| vendor/filippo.io/edwards25519/field/fe_arm64.s | Vendor update: removes arm64 assembly implementation. |
| vendor/filippo.io/edwards25519/field/fe_arm64.go | Vendor update: removes arm64 assembly binding wrapper. |
| vendor/filippo.io/edwards25519/field/fe_amd64_noasm.go | Vendor update: adjusts build tags for amd64 no-asm fallback. |
| vendor/filippo.io/edwards25519/field/fe_amd64.s | Vendor update: adjusts build tags and updates asm sequences. |
| vendor/filippo.io/edwards25519/field/fe_amd64.go | Vendor update: adjusts build tags for amd64 stub. |
| vendor/filippo.io/edwards25519/field/fe.go | Vendor update: changes Add() to use updated carry propagation and updates byte packing. |
| vendor/filippo.io/edwards25519/extra.go | Vendor update: uses slices.Grow, tweaks Montgomery conversion, and adds new Point APIs. |
| vendor/filippo.io/edwards25519/doc.go | Vendor update: docs now reference crypto/ecdh and fips140 upstream package path. |
| vendor/filippo.io/edwards25519/README.md | Vendor update: documentation refresh re upstream tracking and recommended alternatives. |
| go.sum | Updates checksums for bumped dependencies. |
| go.mod | Bumps dependency versions and keeps raft replace; Go version remains 1.25.7. |
| .github/workflows/upgrade.yml | Enables Go cache in setup-go for the upgrade workflow. |
| .github/workflows/system.yml | Enables Go cache in setup-go for system tests workflow. |
| .github/workflows/main.yml | Adds Go version matrix, enables caching, and introduces lint + govulncheck jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| go-version: ['1.24', '1.25.7'] |
There was a problem hiding this comment.
The build matrix includes Go 1.24, but go.mod declares go 1.25.7 and multiple vendored modules require Go 1.25.x. Running CI with 1.24 will fail early with a minimum Go version error. Either drop Go 1.24 from the matrix, or lower the module go directive / dependency minimum versions to restore 1.24 compatibility.
| go-version: ['1.24', '1.25.7'] | |
| go-version: ['1.25.7'] |
| - name: Run govulncheck | ||
| run: govulncheck ./... |
There was a problem hiding this comment.
The govulncheck ./... step will exit non-zero when vulnerabilities are found, and the PR description indicates there are still known stdlib vulnerabilities (fixed only in Go 1.25.8). As written, the new security job is expected to fail on every run until the toolchain is upgraded. Consider gating this job on a Go version that includes the fixes, or allowing only the known GO-2026-460{1,2,3} findings (e.g., parse JSON output and fail only on unexpected IDs) so CI remains actionable.
| - name: Run govulncheck | |
| run: govulncheck ./... | |
| - name: Install jq | |
| run: sudo apt-get update && sudo apt-get install -y jq | |
| - name: Run govulncheck (fail only on unexpected vulnerabilities) | |
| run: | | |
| # Run govulncheck with JSON output; do not fail the step on its exit code alone. | |
| govulncheck -json ./... > govulncheck.json || true | |
| # Extract any vulnerability IDs that are not the known stdlib vulnerabilities. | |
| DISALLOWED_VULNS=$(jq -r ' | |
| .vulns[]?.osv.id | |
| | select(. != "GO-2026-4601" and . != "GO-2026-4602" and . != "GO-2026-4603") | |
| ' govulncheck.json || true) | |
| if [ -n "$DISALLOWED_VULNS" ]; then | |
| echo "Unexpected vulnerabilities found by govulncheck:" | |
| echo "$DISALLOWED_VULNS" | |
| exit 1 | |
| fi | |
| echo "govulncheck completed with only expected vulnerabilities (GO-2026-4601/4602/4603) or none." |
| - name: Run golangci-lint | ||
| uses: golangci/golangci-lint-action@v6 | ||
| with: | ||
| version: latest |
There was a problem hiding this comment.
golangci-lint-action is configured with version: latest, which makes CI non-deterministic (a new golangci-lint release can break linting without any repo changes). Pin to a specific golangci-lint version (and update it intentionally) to keep the workflow reproducible.
| version: latest | |
| version: v1.61.0 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several Go dependencies, notably fixing CVE-2026-4503 in filippo.io/edwards25519. The changes across the vendored libraries include performance optimizations, security enhancements (like using constant-time operations), and correctness fixes. The PR description is thorough and provides good context.
I've identified a couple of minor regressions in the updated fatih/color library where error handling could lead to the terminal's color not being reset. I've provided suggestions to address these issues. Overall, the dependency updates are beneficial and well-executed.
| n, err = c.setWriter(w) | ||
| if err != nil { | ||
| return n, err | ||
| } | ||
|
|
||
| nn, err := fmt.Fprint(w, a...) | ||
| n += nn | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| return fmt.Fprint(w, a...) | ||
| nn, err = c.unsetWriter(w) | ||
| n += nn | ||
| return n, err |
There was a problem hiding this comment.
There's a potential issue in the refactored Fprint function. If fmt.Fprint returns an error, the function will return early without calling c.unsetWriter(w). This could leave the terminal with the color attribute set, which is likely not the intended behavior. The previous implementation using defer did not have this issue.
To ensure the color is always unset, c.unsetWriter(w) should be called regardless of whether fmt.Fprint succeeds or fails. Here's a suggested implementation that addresses this while also correctly handling errors and byte counts from all operations.
n, err = c.setWriter(w)
if err != nil {
return n, err
}
nn, printErr := fmt.Fprint(w, a...)
n += nn
nn, unsetErr := c.unsetWriter(w)
n += nn
if printErr != nil {
return n, printErr
}
return n, unsetErr| n, err = c.setWriter(w) | ||
| if err != nil { | ||
| return n, err | ||
| } | ||
|
|
||
| nn, err := fmt.Fprintf(w, format, a...) | ||
| n += nn | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| return fmt.Fprintf(w, format, a...) | ||
| nn, err = c.unsetWriter(w) | ||
| n += nn | ||
| return n, err |
There was a problem hiding this comment.
Similar to the Fprint function, this implementation of Fprintf has a potential issue where an error in fmt.Fprintf will cause an early return, skipping the call to c.unsetWriter(w). This can leave the terminal's color state modified.
To fix this, c.unsetWriter(w) should be called even if fmt.Fprintf fails. The suggested change below ensures the color is always reset.
n, err = c.setWriter(w)
if err != nil {
return n, err
}
nn, printErr := fmt.Fprintf(w, format, a...)
n += nn
nn, unsetErr := c.unsetWriter(w)
n += nn
if printErr != nil {
return n, printErr
}
return n, unsetErr- filippo.io/edwards25519: v1.1.0 -> v1.2.0 (fixes GO-2026-4503) - github.com/hashicorp/consul/api: v1.33.2 -> v1.33.4 - github.com/mattn/go-sqlite3: v1.14.34 -> v1.14.37 - github.com/montanaflynn/stats: v0.7.1 -> v0.8.2 - github.com/fatih/color: v1.18.0 -> v1.19.0 - golang.org/x/crypto: v0.48.0 -> v0.49.0 - golang.org/x/exp: updated to latest - golang.org/x/sys: v0.41.0 -> v0.42.0 - golang.org/x/term: v0.40.0 -> v0.41.0 Closes #18
renecannao
force-pushed
the
issue18-dependency-updates
branch
from
6da2d29 to
45bcbb3
Compare
Summary
filippo.io/edwards25519Closes #18
govulncheck results — BEFORE updates
govulncheck results — AFTER updates
GO-2026-4503 (edwards25519) is resolved by upgrading to v1.2.0. The remaining 3 vulnerabilities are in the Go standard library and require upgrading to Go 1.25.8 when available.
Dependency changes
Note:
hashicorp/consul/apicould not be updated beyond v1.33.4 because v1.33.5 requires Go 1.25.8.openark/raft fork assessment
The
go.modreplacesgithub.com/hashicorp/raftwithgithub.com/openark/raft v0.0.0-20170918052300-fba9f909f7fe— a 2017-era fork with ~9 years of divergence from upstream hashicorp/raft (now at v1.7.x).The codebase uses APIs from the old raft interface that no longer exist in modern hashicorp/raft:
raft.PeerStore/raft.StaticPeers(replaced byraft.Configurationin upstream)raft.AddUniquePeer()(removed in upstream)config.EnableSingleNode/config.DisableBootstrapAfterElect(replaced by bootstrap API)raft.NewRaft()with 7 arguments including a PeerStore (upstream uses different signature)raft.AddPeer()/raft.RemovePeer()(replaced byAddVoter()/RemoveServer())raft.Leader()returns string (upstream returnsraft.ServerAddress)raft.Yield()/raft.StepDown()(non-standard extensions)Conclusion: Migrating to upstream hashicorp/raft would require significant refactoring of
go/raft/. This should be deferred to Phase 3 as recommended in the design spec.Test plan
go buildsucceedsgo test ./go/...— same results as master (pre-existing vet failures unrelated to dependency changes)