Skip to content

chore(deps): update tidb-dashboard to eliminate vulnerable JWT dependency#10110

Merged
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
JmPotato:update_dashboard
Jan 21, 2026
Merged

chore(deps): update tidb-dashboard to eliminate vulnerable JWT dependency#10110
ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
JmPotato:update_dashboard

Conversation

@JmPotato
Copy link
Member

@JmPotato JmPotato commented Dec 23, 2025

What problem does this PR solve?

Issue Number: close #10109.

What is changed and how does it work?

Update tidb-dashboard to 805c56a830e0fcda07043e78f1d858ccf499babb to eliminate vulnerable `golang-jwt/jwt` dependency.

Check List

Tests

  • Unit test
  • Integration test

Release note

None.

Summary by CodeRabbit

Release Notes

  • Chores

    • Updated critical dependencies: gRPC to v1.75.1, OpenTelemetry to v1.37.0, Protobuf to v1.36.10, and Testify to v1.11.1 for improved stability and compatibility.
    • Enhanced code quality by adding lint suppressions to address staticcheck warnings across the codebase.
  • Tests

    • Improved test infrastructure with updated dependencies and enhanced compatibility handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. labels Dec 23, 2025
@JmPotato JmPotato changed the title chore(deps): update tidb-dashboard to eliminate vulnerable JWT depend… chore(deps): update tidb-dashboard to eliminate vulnerable JWT dependency Dec 23, 2025
@ti-chi-bot ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 23, 2025
golang.org/x/time v0.5.0
golang.org/x/tools v0.38.0
google.golang.org/grpc v1.62.1
google.golang.org/grpc v1.75.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it expected?

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

The PR adds nolint:staticcheck directives and TODO comments across multiple files to suppress deprecation warnings related to gRPC APIs (Dial, DialContext, WithBlock), updates multiple go.mod files with significant dependency version bumps (grpc v1.75.1, gin v1.11.0, testify v1.11.1, OpenTelemetry v1.37.0), introduces github.com/appleboy/gin-jwt/v2 as a new dependency, and updates goleak ignore patterns in test utilities.

Changes

Cohort / File(s) Summary
gRPC API deprecation lint suppressions
client/pkg/utils/grpcutil/grpcutil.go, pkg/mcs/router/server/sync.go, pkg/mcs/tso/server/testutil.go, pkg/syncer/client.go, pkg/utils/grpcutil/grpcutil.go, pkg/utils/testutil/testutil.go
Added //nolint:staticcheck directives and TODO comments before deprecated grpc.Dial, grpc.DialContext, and grpc.WithBlock() calls; no functional changes.
Test integration lint suppressions
tests/integrations/client/client_tls_test.go, tests/integrations/mcs/keyspace/tso_keyspace_group_test.go, tests/integrations/mcs/scheduling/server_test.go, tests/integrations/mcs/tso/proxy_test.go, tests/integrations/mcs/tso/server_test.go
Added //nolint:staticcheck directives to suppress deprecation warnings on gRPC dial calls in test contexts.
Tool utilities lint suppressions
tools/pd-simulator/simulator/client.go, tools/pd-api-bench/main.go, tools/pd-region-bench/main.go, tools/pd-tso-bench/main.go, client/servicediscovery/service_discovery_test.go
Added //nolint:staticcheck directives and TODO comments before deprecated gRPC function calls in tool utilities and service discovery tests.
Primary go.mod dependency upgrades
go.mod
Updated: grpc v1.62.1→v1.75.1, gin-gonic/gin v1.10.0→v1.11.0, testify v1.9.0→v1.11.1; added github.com/appleboy/gin-jwt/v2 v2.10.3; upgraded OpenTelemetry suite (otel v1.37.0), protobuf, and numerous indirect dependencies.
Test integrations go.mod
tests/integrations/go.mod
Synchronized dependency updates: grpc v1.62.1→v1.75.1, gin v1.10.0→v1.11.0, testify v1.9.0→v1.11.1; added gin-jwt/v2; updated OpenTelemetry, sonic, and cloudwego modules; added testify v1.10.0 replace directive.
Tools go.mod
tools/go.mod
Synchronized dependency updates matching primary go.mod: grpc v1.75.1, gin v1.11.0, testify v1.11.1; added gin-jwt/v2; upgraded OTEL suite, protobuf, and cloudwego/bytedance modules; added testify replacement.
Client go.mod
client/go.mod
Updated grpc v1.62.1→v1.75.1, protobuf v1.34.2→v1.36.6, and synchronized indirect dependencies (golang.org/x/net, golang.org/x/sys, golang.org/x/text, google.golang.org/genproto).
Test leak ignore patterns
client/pkg/utils/testutil/leak.go, pkg/utils/testutil/leak.go
Updated LeakOptions goleak.IgnoreTopFunction from resetTransport to resetTransportAndUnlock to match gRPC v1.75.1 internal function naming.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 Our GRPC calls grow wise with age,
Deprecated APIs turn a new page,
With lint directives cast and tested with care,
And dependencies fresh beyond compare,
We hop toward futures secure and bright!

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive While most changes (dependency updates, nolint directives) relate to the JWT vulnerability fix, the extensive refactoring of gRPC calls with TODO comments and nolint directives across 20+ files appears tangential to the stated objective. Clarify whether the widespread gRPC refactoring (TODO comments for grpc.DialContext deprecation, nolint:staticcheck directives) is directly necessary for eliminating the JWT vulnerability or if it should be separated into a distinct PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating tidb-dashboard dependency to eliminate vulnerable JWT dependency, which aligns with the PR's primary objective.
Description check ✅ Passed The description follows the template with Issue Number linked to #10109, clear explanation of changes via commit message, and appropriate release note. All required sections are present.
Linked Issues check ✅ Passed The PR successfully addresses the requirements in #10109: eliminates the vulnerable golang-jwt/jwt dependency by updating tidb-dashboard to version containing commit 805c56a, and includes dependency updates across multiple go.mod files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @tests/integrations/go.mod:
- Line 21: Run the full Go test suite after the dependency updates (testify ->
v1.11.1 and grpc -> v1.75.1) by running `go test ./...` and `go test -race
./...`; ensure `go mod tidy` is applied so versions are resolved. Pay special
attention to mock behavior changes in github.com/stretchr/testify/mock
(argument-matching behavior) and any grpc-related race or flow-control failures
in packages using google.golang.org/grpc; update affected tests/mocks to the new
matcher semantics or adjust expectations, and fix any flakey tests revealed by
the race detector so CI passes.
🧹 Nitpick comments (8)
tests/integrations/mcs/scheduling/server_test.go (1)

1697-1698: Consider migrating to the newer gRPC dial API.

The //nolint:staticcheck directive suppresses warnings about deprecated grpc.Dial usage. While this works, the gRPC team recommends using grpc.NewClient (introduced in grpc-go v1.48) for new code.

♻️ Optional refactor to use the newer API
-	//nolint:staticcheck
-	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
tests/integrations/mcs/keyspace/tso_keyspace_group_test.go (1)

107-108: Consider migrating to the newer gRPC dial API.

The //nolint:staticcheck directive suppresses warnings about deprecated grpc.Dial usage. Consider using grpc.NewClient (available since grpc-go v1.48) instead.

♻️ Optional refactor to use the newer API
-	//nolint:staticcheck
-	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
pkg/utils/grpcutil/grpcutil.go (1)

172-173: Consider migrating to grpc.NewClient for future-proofing.

The //nolint:staticcheck directive suppresses deprecation warnings for grpc.DialContext. Since GetClientConn is a core utility function, migrating to grpc.NewClient (introduced in grpc-go v1.48, now at v1.75.1) would align with gRPC best practices.

♻️ Recommended migration to the newer API
-	//nolint:staticcheck
-	cc, err := grpc.DialContext(ctx, u.Host, do...)
+	cc, err := grpc.NewClient(u.Host, do...)

Note: grpc.NewClient doesn't require a context parameter as it performs lazy connection initialization.

client/pkg/utils/grpcutil/grpcutil.go (1)

148-149: Consider migrating to grpc.NewClient for future-proofing.

The //nolint:staticcheck directive suppresses deprecation warnings for grpc.DialContext. Since GetClientConn is a client utility function, migrating to grpc.NewClient would align with current gRPC best practices.

♻️ Recommended migration to the newer API
-	//nolint:staticcheck
-	cc, err := grpc.DialContext(ctx, u.Host, do...)
+	cc, err := grpc.NewClient(u.Host, do...)

Note: grpc.NewClient performs lazy connection initialization and doesn't require a context parameter. The interceptors and dial options should remain compatible.

pkg/utils/testutil/testutil.go (1)

84-85: Consider migrating to the newer gRPC dial API.

The //nolint:staticcheck directive suppresses warnings about deprecated grpc.Dial usage. Consider using grpc.NewClient (available since grpc-go v1.48) for consistency with modern gRPC patterns.

♻️ Optional refactor to use the newer API
-	//nolint:staticcheck
-	conn, err := grpc.Dial(strings.TrimPrefix(addr, "http://"), grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.NewClient(strings.TrimPrefix(addr, "http://"), grpc.WithTransportCredentials(insecure.NewCredentials()))
pkg/mcs/router/server/sync.go (1)

241-242: LGTM - Staticcheck suppression correctly applied.

The //nolint:staticcheck directive appropriately silences warnings for the deprecated grpc.WithBlock() dial option. This aligns with the repository-wide pattern of suppressing gRPC staticcheck warnings.

For future consideration: grpc.WithBlock() is deprecated in favor of using context timeouts/cancellation. When refactoring is feasible, consider migrating to the recommended pattern.

pkg/syncer/client.go (1)

134-135: LGTM - Staticcheck suppression correctly applied.

The //nolint:staticcheck directive appropriately silences warnings for the deprecated grpc.WithBlock() dial option. The comment on line 133 documents the intended blocking behavior.

For future consideration: The blocking dial pattern could be replaced with context timeout/cancellation as recommended by the gRPC deprecation notice.

tests/integrations/go.mod (1)

54-228: Optional: Consider verifying transitive dependency updates.

The extensive indirect dependency updates across OpenTelemetry (v1.37.0), protobuf (v1.36.10), and x/* packages align with the gRPC v1.75.1 upgrade. While these are transitive updates, consider running integration tests to ensure compatibility across the dependency graph.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92932ad and df7d0d4.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • client/pkg/utils/grpcutil/grpcutil.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-simulator/simulator/client.go
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integrations/client/client_tls_test.go (1)
client/opt/option.go (1)
  • WithGRPCDialOptions (174-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: tso-function-test
  • GitHub Check: statics
  • GitHub Check: chunks
🔇 Additional comments (10)
tools/pd-simulator/simulator/client.go (1)

110-111: LGTM - Lint suppression correctly applied.

The //nolint:staticcheck directive properly silences warnings for the deprecated blocking behavior of grpc.Dial(). This is consistent with the codebase-wide suppression pattern.

tests/integrations/mcs/tso/server_test.go (1)

115-116: LGTM - Staticcheck suppression correctly applied in test.

The //nolint:staticcheck directive appropriately suppresses warnings for the deprecated blocking dial behavior. This maintains the existing test logic while silencing staticcheck warnings.

pkg/mcs/tso/server/testutil.go (1)

30-31: LGTM - Lint suppression correctly applied in test utility.

The //nolint:staticcheck directive properly suppresses warnings for the deprecated grpc.Dial() blocking behavior in this test utility function.

tests/integrations/client/client_tls_test.go (1)

173-173: LGTM: Appropriate lint suppressions for deprecated grpc API in test code.

The //nolint:staticcheck directives correctly suppress warnings for the deprecated grpc.WithBlock() dial option. While this option is deprecated in gRPC v1.75.1, suppressing the warnings in test code is acceptable and avoids unnecessary refactoring of test utilities.

Also applies to: 206-206, 221-221

tests/integrations/mcs/tso/proxy_test.go (1)

189-190: LGTM: Appropriate lint suppressions for deprecated grpc.Dial in test utilities.

The //nolint:staticcheck directives correctly suppress warnings for the deprecated grpc.Dial function. Suppressing these warnings in test code is reasonable while the codebase transitions to newer gRPC APIs.

Also applies to: 394-395

tests/integrations/go.mod (1)

112-112: **Verify JWT vulnerability fix and tidb-dashboard update.**CVE-2025-30204 affects golang-jwt starting in version 3.2.0 and prior to versions 5.2.2 and 4.5.2, and the issue is fixed in 4.5.2. The upgrade to golang-jwt/jwt/v4 v4.5.2 (line 112) successfully addresses this security vulnerability. The tidb-dashboard update to commit 121f017c2150 (line 156) aligns with the PR objective to eliminate the vulnerable JWT dependency.

Based on web search results.

Also applies to: 156-156

tools/go.mod (2)

19-19: LGTM: Dependency updates consistent with other modules.

The updates to gin (v1.11.0), testify (v1.11.1), and grpc (v1.75.1) mirror those in tests/integrations/go.mod, ensuring consistency across the module graph.

Also applies to: 33-33, 45-45


119-119: Security fix confirmed: JWT vulnerability addressed.

The upgrade to golang-jwt/jwt/v4 v4.5.2 (line 119) and the tidb-dashboard update (line 162) successfully address CVE-2025-30204, fulfilling the primary objective of this PR.

Based on web search results confirming v4.5.2 fixes the vulnerability.

Also applies to: 162-162

go.mod (2)

26-26: LGTM: Core dependency upgrades align with PR objectives.

The coordinated updates to gin (v1.11.0), tidb-dashboard (121f017c2150), testify (v1.11.1), and grpc (v1.75.1) successfully address the PR's security objectives while modernizing the dependency graph.

Also applies to: 42-42, 51-51, 65-65


138-138: Security vulnerability successfully resolved.

The presence of golang-jwt/jwt/v4 v4.5.2 confirms that CVE-2025-30204 has been addressed. This version includes the fix for the denial-of-service vulnerability in the ParseUnverified function.

Based on web search results confirming the fix.

@JmPotato
Copy link
Member Author

JmPotato commented Jan 8, 2026

/retest

@JmPotato JmPotato force-pushed the update_dashboard branch 2 times, most recently from 84e7f4a to a62d612 Compare January 8, 2026 11:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
pkg/utils/grpcutil/grpcutil.go (1)

172-173: Lint suppression in production code - consider adding a comment.

The //nolint:staticcheck directive is applied here in a core utility function. While this is consistent with the broader dependency update pattern, production code suppressions benefit from inline comments explaining why they're necessary.

💡 Optional improvement: Add explanatory comment
+	// nolint:staticcheck - Suppressing deprecation warning for grpc.DialContext.
+	// TODO: Evaluate migration to newer gRPC connection APIs in future updates.
 	//nolint:staticcheck
 	cc, err := grpc.DialContext(ctx, u.Host, do...)

This helps future maintainers understand the suppression context and track technical debt.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df7d0d4 and a62d612.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • client/pkg/utils/grpcutil/grpcutil.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/leak.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-simulator/simulator/client.go
✅ Files skipped from review due to trivial changes (1)
  • tests/integrations/client/client_tls_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
  • tools/pd-simulator/simulator/client.go
  • pkg/mcs/router/server/sync.go
  • client/pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/testutil.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • tests/integrations/mcs/tso/proxy_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: tso-function-test
  • GitHub Check: statics
  • GitHub Check: chunks
🔇 Additional comments (10)
tests/integrations/mcs/scheduling/server_test.go (1)

1697-1698: LGTM - Consistent lint suppression pattern.

The //nolint:staticcheck directive is appropriately applied here, consistent with the pattern used throughout the codebase for gRPC dial calls after the dependency updates.

tests/integrations/mcs/tso/server_test.go (1)

115-116: LGTM - Lint suppression for test gRPC dial call.

The suppression is appropriately applied and consistent with the pattern used across test files.

tests/integrations/mcs/keyspace/tso_keyspace_group_test.go (1)

107-108: LGTM - Lint suppression for deprecated gRPC dial call.

The //nolint:staticcheck directive appropriately suppresses staticcheck warnings for grpc.Dial, which is deprecated in gRPC v1.75.1. This pattern is consistently applied throughout the codebase for all grpc.Dial and grpc.DialContext calls.

pkg/utils/testutil/leak.go (1)

23-23: No changes needed—function name is correct for gRPC v1.75.1.

The update from resetTransport to resetTransportAndUnlock correctly reflects the internal gRPC implementation in v1.75.1. The function name exists and is the proper target for goroutine leak detection in this version.

tests/integrations/go.mod (3)

112-112: Security fix verified: JWT v4.5.2 is in place.

The vulnerable github.com/golang-jwt/jwt v3.2.2+incompatible has been removed, and the secure version github.com/golang-jwt/jwt/v4 v4.5.2 is now present as an indirect dependency. The tidb-dashboard update to commit 121f017c2150... aligns with the security objective. This is consistent across all three go.mod files.

Please verify that the tidb-dashboard commit update actually eliminates the vulnerable v3 JWT dependency from the dependency tree.

Also applies to: 156-156


54-54: New indirect dependency gin-jwt/v2 added; verify it doesn't reintroduce vulnerability.

A new transitive dependency github.com/appleboy/gin-jwt/v2 v2.10.3 appears across all three go.mod files. Since this PR is specifically targeting JWT dependency elimination, please confirm that this gin-jwt package uses the secure v4 JWT implementation and doesn't inadvertently pull in the vulnerable v3 version.

🔍 Verification steps

You can verify this by:

  1. Checking the upstream gin-jwt/v2 repository to confirm it depends on github.com/golang-jwt/jwt/v4
  2. Running go mod graph to inspect the full dependency tree for any v3 JWT references
  3. Confirming that no github.com/golang-jwt/jwt (v3 implicit path) exists in the final build

28-28: Large grpc and protobuf ecosystem upgrades; verify compatibility.

This change upgrades google.golang.org/grpc from v1.62.1 to v1.75.1 and introduces corresponding updates to:

  • google.golang.org/protobuf v1.36.10
  • google.golang.org/genproto/googleapis/{api,rpc} to pre-release versions (v0.0.0-20250707201910-...)
  • go.opentelemetry.io/otel family to v1.37.0

While these upgrades appear intentional and part of broader modernization, such major version jumps can introduce subtle incompatibilities. The pre-release genproto versions are particularly noteworthy.

Please confirm that the integration tests pass with these updated versions to rule out gRPC API or protobuf compatibility issues.

Also applies to: 138-138, 156-156

tools/go.mod (1)

119-119: Consistent security fix across modules.

The JWT v4.5.2 update and tidb-dashboard commit are consistent with tests/integrations/go.mod, confirming a coordinated dependency update across the project's module structure.

Also applies to: 162-162

go.mod (2)

138-138: Root module security fix aligns with transitive modules.

The root go.mod maintains consistency with tests/integrations/go.mod and tools/go.mod, all using JWT v4.5.2 and the same tidb-dashboard commit. This coordinated update strengthens confidence in the security fix.

Also applies to: 42-42


181-182: New QUIC tooling added; verify intent.

The QUIC dependencies (github.com/quic-go/quic-go v0.55.0 and github.com/quic-go/qpack v0.5.1) were added as indirect dependencies. While this is likely driven by gRPC v1.75.1's HTTP/2 and protocol improvements, please confirm this aligns with the project's protocol and feature roadmap and doesn't represent unexpected transitive growth.

@JmPotato
Copy link
Member Author

JmPotato commented Jan 8, 2026

/retest

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @tests/integrations/go.mod:
- Line 28: Tests are using deprecated gRPC APIs (grpc.WithBlock, grpc.Dial,
grpc.DialContext) which are currently silenced with //nolint:staticcheck; update
call sites to use the new grpc.NewClient pattern instead and remove the nolint
suppressions: locate tests and helper functions that call grpc.Dial/
grpc.DialContext/ grpc.WithBlock, replace them with the grpc.NewClient API (or
wrapper that constructs a grpc.Client using the new options and context-based
dial semantics), adapt connection setup and options to the NewClient style, and
add a short migration note in the tests/docs to track remaining deprecated
usages until all are converted.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a62d612 and b2d2bf8.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • client/pkg/utils/grpcutil/grpcutil.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/leak.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-simulator/simulator/client.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • pkg/mcs/tso/server/testutil.go
  • pkg/mcs/router/server/sync.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tools/go.mod
  • pkg/syncer/client.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tools/pd-simulator/simulator/client.go
  • client/pkg/utils/grpcutil/grpcutil.go
  • tests/integrations/client/client_tls_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: tso-function-test
  • GitHub Check: statics
  • GitHub Check: chunks
🔇 Additional comments (10)
pkg/utils/testutil/testutil.go (1)

84-85: LGTM - Consistent lint suppression for test utility.

The nolint directive is correctly placed and consistent with other similar changes in this PR. Test utilities are reasonable candidates for deferred migration to grpc.NewClient.

pkg/utils/grpcutil/grpcutil.go (1)

172-173: LGTM - Lint suppression for core gRPC utility.

The nolint directive appropriately suppresses the deprecation warning. Note that this is production code (not test code), so migration to grpc.NewClient should be prioritized in a follow-up effort. The documented blocking/non-blocking semantics (lines 138-150) will need careful consideration during that migration, as grpc.NewClient has different connection establishment behavior.

tests/integrations/mcs/tso/server_test.go (1)

115-116: LGTM - Lint suppression for test connectivity check.

The nolint directive is correctly placed. This test code simply verifies gRPC connectivity, making it a low-priority candidate for migration to the new API.

tests/integrations/mcs/keyspace/tso_keyspace_group_test.go (1)

107-108: LGTM - Lint suppression for deprecated gRPC API.

The //nolint:staticcheck directive appropriately suppresses the deprecation warning for grpc.Dial, which was superseded by grpc.NewClient in grpc-go v1.63. This is acceptable as a short-term solution when upgrading gRPC dependencies. Consider migrating to grpc.NewClient in a future cleanup effort.

pkg/utils/testutil/leak.go (1)

23-23: Line 23 contains an inconsistency with the codebase.

The gRPC function reference differs between the two leak.go files: pkg/utils/testutil/leak.go uses resetTransportAndUnlock (line 23) while client/pkg/utils/testutil/leak.go uses resetTransport (line 24). Additionally, the git history shows these files were newly created in commit b2d2bf8, not updated in response to a gRPC function rename. Web search results indicate that resetTransport is the current implementation in gRPC, while resetTransportAndUnlock appears in older versions.

Verify that both leak.go files are using the correct function names for gRPC v1.75.1.

Likely an incorrect or invalid review comment.

tests/integrations/go.mod (2)

21-232: Dependency updates aligned with tidb-dashboard update.

The extensive dependency updates across testing frameworks (testify v1.11.1), serialization libraries (sonic v1.14.1, goccy/go-json v0.10.5), validation (validator/v10 v10.28.0), observability (OpenTelemetry v1.37.0), and other indirect dependencies appear to be transitive updates resulting from the tidb-dashboard upgrade. These changes maintain consistency across the dependency graph.


156-156: The tidb-dashboard update to commit 121f017c2150 successfully eliminates CVE-2025-30204.

The indirect dependency github.com/golang-jwt/jwt/v4 v4.5.2 is present in the go.mod, which includes the fix for CVE-2025-30204 (excessive memory allocation during header parsing). The patched version v4.5.2 resolves this vulnerability as expected.

go.mod (3)

42-42: Primary tidb-dashboard update to address security vulnerability.

This direct dependency update to tidb-dashboard commit 121f017c2150 is the core change that should eliminate the vulnerable JWT dependency mentioned in the PR objectives.


138-138: JWT dependency successfully updated to v4.5.2.

The vulnerable github.com/golang-jwt/jwt v3.2.2+incompatible has been replaced with github.com/golang-jwt/jwt/v4 v4.5.2, which should address CVE-2025-30204 as stated in the PR objectives.


26-26: No compatibility concerns with the Gin v1.11.0 update.

The codebase uses standard Gin patterns (engine creation, middleware, handlers with *gin.Context, gin.H responses) that are fully compatible with v1.11.0. The go.mod specifies Go 1.25.5, which exceeds the v1.11.0 minimum of Go 1.21. The only breaking change in v1.11.0 (headers parameter removal from writeHeader) affects only custom renderers, which the codebase does not use. The update proceeds without risk.

go.uber.org/goleak v1.3.0
go.uber.org/zap v1.27.0
google.golang.org/grpc v1.62.1
google.golang.org/grpc v1.75.1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's check the current go.mod file to see versions
echo "=== Current go.mod file (lines around grpc) ==="
cat tests/integrations/go.mod | grep -A2 -B2 "grpc"

echo -e "\n=== Searching for grpc imports ==="
fd -e go -path 'tests/integrations/*' -type f | head -20

echo -e "\n=== Searching for gRPC dial patterns ==="
rg 'grpc\.(Dial|DialContext|WithBlock)' --type=go tests/integrations/ -n

echo -e "\n=== Searching for staticcheck nolint directives ==="
rg 'nolint:staticcheck' --type=go tests/integrations/ -B3 -A3

echo -e "\n=== Checking for any grpc references in test files ==="
rg 'import.*grpc' --type=go tests/integrations/ -n

Repository: tikv/pd

Length of output: 13609


🏁 Script executed:

#!/bin/bash
# Check git history for previous gRPC version
echo "=== Git history of gRPC version in go.mod ==="
git log -p --follow -S 'google.golang.org/grpc' -- tests/integrations/go.mod | head -100

echo -e "\n=== Recent commits modifying tests/integrations/go.mod ==="
git log --oneline tests/integrations/go.mod | head -10

Repository: tikv/pd

Length of output: 229


🏁 Script executed:

#!/bin/bash
# Get detailed commit info
echo "=== Commit details ==="
git show b2d2bf83 --stat

echo -e "\n=== Changes to go.mod in that commit ==="
git show b2d2bf83 -- tests/integrations/go.mod | head -60

Repository: tikv/pd

Length of output: 50363


🏁 Script executed:

#!/bin/bash
# Get the parent commit before the nolint commit
echo "=== Previous commit before nolint commit ==="
git show b2d2bf83^:tests/integrations/go.mod | grep -A2 -B2 "grpc" || echo "File may not exist in parent"

echo -e "\n=== Check if go.mod changed in nolint commit ==="
git diff b2d2bf83^..b2d2bf83 -- tests/integrations/go.mod | head -50

Repository: tikv/pd

Length of output: 273


🌐 Web query:

gRPC v1.75.1 release notes deprecations

💡 Result:

In gRPC v1.75.1 the only explicit deprecation mentioned is for Python asyncio usage: calling grpc.aio async methods outside a running event loop is discouraged and "will be deprecated in future gRPC releases" (use asyncio.run() / asyncio.Runner or the asyncio REPL instead). [1][2]

Sources:

  • gRPC v1.75.1 release notes (GitHub). [1]
  • gRPC v1.75.1 mirror/release notes (SourceForge). [2]

🌐 Web query:

gRPC Go grpc.WithBlock() deprecated when

💡 Result:

grpc.WithBlock was deprecated in the grpc-go 1.64.x series (announced in the v1.64.0 release notes / docs; PR #7097). [1][2]

Sources:

  • pkg.go.dev grpc docs (WithBlock marked Deprecated). [1]
  • grpc-go v1.64.0 release notes / PR mcs: update store stats #7097 (deprecates WithBlock). [2]

🌐 Web query:

gRPC Go grpc.Dial deprecated when changelog

💡 Result:

Short answer: grpc.Dial and grpc.DialContext were marked deprecated around the grpc-go v1.63.0 work that introduced grpc.NewClient (April 2024). The project briefly removed the deprecated tag in v1.63.1/v1.63.2, and the deprecation was planned to be reintroduced in v1.64.0; see the grpc-go release/merge notes and related issues. [1][2][3]

Sources

  • grpc-go v1.63.0 release notes (introduces grpc.NewClient / deprecation work). [1]
  • Merge notes / changelog mentioning "Deprecate Dial and DialContext" and the deprecation removal in v1.63.x (and plan for v1.64). [2]
  • Issues and migration discussions (history and guidance to move to grpc.NewClient). [3]

Address deprecated gRPC APIs with migration plan.

The update to v1.75.1 includes gRPC APIs that have been deprecated since v1.63–v1.64 (grpc.WithBlock, grpc.Dial, grpc.DialContext). The staticcheck suppressions (//nolint:staticcheck) appropriately suppress these deprecation warnings across test files. While the suppressions are a valid short-term workaround, consider planning a migration to the recommended grpc.NewClient API to eliminate the need for these directives and stay aligned with gRPC's deprecation guidance.

🤖 Prompt for AI Agents
In @tests/integrations/go.mod at line 28, Tests are using deprecated gRPC APIs
(grpc.WithBlock, grpc.Dial, grpc.DialContext) which are currently silenced with
//nolint:staticcheck; update call sites to use the new grpc.NewClient pattern
instead and remove the nolint suppressions: locate tests and helper functions
that call grpc.Dial/ grpc.DialContext/ grpc.WithBlock, replace them with the
grpc.NewClient API (or wrapper that constructs a grpc.Client using the new
options and context-based dial semantics), adapt connection setup and options to
the NewClient style, and add a short migration note in the tests/docs to track
remaining deprecated usages until all are converted.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
go.mod (1)

65-65: gRPC upgrade is expected as part of the dependency update.

The upgrade from v1.62.1 to v1.75.1 is part of the broader dependency synchronization. The nolint directives added elsewhere in the PR address the deprecation warnings introduced by this upgrade.

🧹 Nitpick comments (1)
tests/integrations/go.mod (1)

76-78: Consider verifying key indirect dependency updates for compatibility.

Multiple indirect dependencies have been updated, including:

  • gin-gonic/gin (v1.10.0 → v1.11.0)
  • validator/v10 (v10.20.0 → v10.28.0)
  • protobuf (v1.34.2 → v1.36.10)
  • oauth2 (v0.27.0 → v0.30.0)
  • bytedance/sonic (v1.14.1)
  • Various other packages

While these are indirect dependencies, consider running the test suite to ensure compatibility, particularly for larger version jumps like validator/v10.

#!/bin/bash
# Description: Run test suite to verify dependency compatibility

# Run tests with the updated dependencies
echo "=== Running test suite ==="
cd tests/integrations
go test -v ./... -short

# Check for any deprecation warnings
echo -e "\n=== Checking for deprecation warnings ==="
go build ./... 2>&1 | grep -i deprecat || echo "✓ No deprecation warnings"

Also applies to: 84-84, 91-91, 95-97, 106-106, 109-110, 138-138, 150-150, 163-164, 186-186, 192-192, 213-213, 219-219, 226-228

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2d2bf8 and 8fd6721.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • client/pkg/utils/grpcutil/grpcutil.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/leak.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-simulator/simulator/client.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • pkg/mcs/tso/server/testutil.go
  • tools/pd-simulator/simulator/client.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/client/client_tls_test.go
  • client/pkg/utils/grpcutil/grpcutil.go
  • pkg/mcs/router/server/sync.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/syncer/client.go
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: statics
  • GitHub Check: chunks
  • GitHub Check: tso-function-test
🔇 Additional comments (12)
tests/integrations/mcs/tso/server_test.go (1)

115-116: Appropriate lint suppression for deprecated gRPC API.

The //nolint:staticcheck directive correctly suppresses the SA1019 warning for grpc.DialContext, which is deprecated in gRPC v1.75.1. This is a reasonable temporary measure given the PR's focus on addressing the JWT security vulnerability. Consider tracking migration to grpc.NewClient as a follow-up task.

pkg/utils/testutil/testutil.go (1)

84-85: LGTM!

The nolint directive appropriately suppresses the deprecation warning for grpc.Dial in this test utility. Consistent with similar changes across the codebase.

tools/go.mod (2)

119-119: JWT dependency updated to patched version - addresses CVE-2025-30204.

The golang-jwt/jwt/v4 v4.5.2 dependency is the patched version that fixes the high-severity vulnerability CVE-2025-30204. This aligns with the PR objective of eliminating the vulnerable JWT dependency.


162-162: tidb-dashboard updated to the target commit eliminating vulnerable JWT.

The update to commit 121f017c2150 is the core change for this PR, pulling in the tidb-dashboard version that eliminates the vulnerable github.com/golang-jwt/jwt dependency.

go.mod (3)

42-42: Core fix: tidb-dashboard updated to eliminate JWT vulnerability.

The update to commit 121f017c2150ec1037199e18e5f255aac057d48f is the primary change addressing issue #10109 and CVE-2025-30204.


88-88: New indirect dependency: gin-jwt/v2.

This new indirect dependency appears as part of the tidb-dashboard update chain. It uses golang-jwt/jwt/v4 v4.5.2 internally, which is the patched version.


138-138: JWT dependency correctly updated to patched version v4.5.2.

The vulnerable v3 dependency has been removed and replaced with golang-jwt/jwt/v4 v4.5.2, which addresses CVE-2025-30204 (a denial-of-service vulnerability in the ParseUnverified function triggered by crafted tokens with excessive dots). The dependency graph contains no remaining v3 references, and v4.5.2 is confirmed as the fixed version.

pkg/utils/testutil/leak.go (1)

23-23: Correct update for gRPC internal function name change.

The goroutine leak filter update from resetTransport to resetTransportAndUnlock is necessary for gRPC v1.75.1 and prevents false positive leak detections in tests.

tests/integrations/go.mod (4)

112-112: Dependency update resolves CVE-2025-30204.

The updated github.com/golang-jwt/jwt/v4 v4.5.2 correctly addresses CVE-2025-30204 (a resource-consumption bug in parse.ParseUnverified). The vulnerable v3.2.2+incompatible has been removed from the dependency graph, and v4.5.2 is confirmed as the active version.


156-156: This tidb-dashboard update resolves CVE-2025-30204 JWT vulnerability.

The update to commit 121f017c2150ec1037199e18e5f255aac057d48f transitively pulls in github.com/golang-jwt/jwt/v4 v4.5.2 (line 112), addressing the high-severity DoS vulnerability in golang-jwt's parse.ParseUnverified function that affects versions below v4.5.2.


200-208: Resolve version inconsistency in OpenTelemetry packages.

The OpenTelemetry dependencies have mismatched versions: core packages (otel, metric, sdk, trace) are at v1.37.0, but the exporters (otlp/otlptrace and otlptrace/otlptracegrpc) remain at v1.20.0. While these are indirect dependencies with no direct usage in tests, this version mismatch could cause compatibility issues. Verify that the exporters can be upgraded to v1.37.0 or that the core packages should be rolled back to v1.20.0 for consistency.


54-54: Investigate gin-jwt/v2 version and transitive JWT dependencies.

The indirect dependency github.com/appleboy/gin-jwt/v2 v2.10.3 has been introduced. While no direct CVE exists for v2.10.3, this version is outdated—v3.2.0 was released in November 2025. The v3 series uses the newer github.com/golang-jwt/jwt library directly, whereas v2 uses an older JWT dependency. Verify:

  1. What JWT library version gin-jwt/v2 v2.10.3 transitively depends on
  2. Whether tidb-dashboard can be updated to use gin-jwt/v3.2.0 instead

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@JmPotato JmPotato force-pushed the update_dashboard branch 3 times, most recently from f67f5b8 to f051fa4 Compare January 14, 2026 08:18
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@go.mod`:
- Line 42: The go.mod entry uses pseudo-version
v0.0.0-20260112071732-805c56a830e0 which does not match the expected commit
121f017c2150ec1037199e18e5f255aac057d48f; verify the actual commit by running
the provided go mod download/check commands (or inspect go.sum) and if the wrong
commit was used, update the go.mod dependency to the pseudo-version that
corresponds to commit 121f017c2150ec1037199e18e5f255aac057d48f (or replace with
an exact tag/commit), run go mod tidy to refresh go.sum, re-run builds/tests,
and commit the corrected go.mod/go.sum so the JWT fix is truly included.
♻️ Duplicate comments (1)
go.mod (1)

65-65: gRPC upgrade is expected as a transitive dependency update.

This upgrade from v1.62.1 to v1.75.1 is a transitive update pulled in by the tidb-dashboard upgrade. The AI-generated summary notes that nolint:staticcheck annotations were added across the codebase to suppress deprecation warnings for grpc.Dial and grpc.WithBlock APIs, indicating the team is aware of the upgrade and has addressed linter concerns.

Based on the past review comments and the PR context, this is an expected consequence of updating tidb-dashboard.

🧹 Nitpick comments (1)
tools/pd-region-bench/main.go (1)

137-143: Consider adding a TODO comment for migration to grpc.NewClient.

The AI summary mentions a TODO comment about using grpc.NewClient instead of grpc.DialContext, but the actual code only has the nolint:staticcheck directive without the accompanying TODO.

For consistency with other files in this PR (as noted in the summary for grpcutil.go) and to document the technical debt, consider adding:

 	//nolint:staticcheck
+	// TODO: use grpc.NewClient instead of grpc.DialContext
 	opts = append(opts, opt.WithGRPCDialOptions(

This provides context for future maintainers about why the lint is suppressed and what the intended migration path is.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f67f5b8 and f051fa4.

⛔ Files ignored due to path filters (4)
  • client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (22)
  • client/go.mod
  • client/pkg/utils/grpcutil/grpcutil.go
  • client/pkg/utils/testutil/leak.go
  • client/servicediscovery/service_discovery_test.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/leak.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-api-bench/main.go
  • tools/pd-region-bench/main.go
  • tools/pd-simulator/simulator/client.go
  • tools/pd-tso-bench/main.go
✅ Files skipped from review due to trivial changes (1)
  • client/pkg/utils/grpcutil/grpcutil.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • client/servicediscovery/service_discovery_test.go
  • tools/pd-simulator/simulator/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tools/pd-api-bench/main.go
  • pkg/mcs/tso/server/testutil.go
  • tests/integrations/mcs/tso/proxy_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integrations/client/client_tls_test.go (1)
client/opt/option.go (1)
  • WithGRPCDialOptions (174-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: tso-function-test
  • GitHub Check: chunks
  • GitHub Check: statics
🔇 Additional comments (9)
tools/pd-tso-bench/main.go (1)

239-245: Remove the unnecessary //nolint:staticcheck directive.

This code uses only standard, non-deprecated APIs (grpc.WithKeepaliveParams and opt.WithGRPCDialOptions). The nolint directive serves no purpose here and is inconsistent with identical code in tools/pd-region-bench/main.go and tools/pd-api-bench/main.go, which don't include this suppression. Unlike other nolint directives in the codebase that suppress warnings from deprecated APIs like grpc.Dial() or grpc.DialContext(), there is no deprecated API being used at these lines.

Likely an incorrect or invalid review comment.

tests/integrations/client/client_tls_test.go (1)

173-173: LGTM - Lint suppressions for deprecated grpc.WithBlock() are appropriate.

The //nolint:staticcheck directives correctly suppress warnings for the deprecated grpc.WithBlock() dial option. Using blocking dial in tests ensures connections are established before assertions run, which is a valid use case.

Consider adding TODO comments (as done in the production code files pkg/syncer/client.go and pkg/mcs/router/server/sync.go) to track the eventual migration away from grpc.WithBlock(), though this is optional for test code.

Also applies to: 206-206, 221-221, 335-335

pkg/mcs/router/server/sync.go (1)

240-243: LGTM - Well-documented deprecation suppression.

The TODO comment clearly documents the technical debt, and the //nolint:staticcheck directive appropriately suppresses the warning for the deprecated grpc.WithBlock(). The existing comment on line 240 explains why blocking behavior is used here, making the code maintainable.

pkg/syncer/client.go (1)

133-136: LGTM - Consistent with other nolint suppressions in this PR.

The TODO comment and //nolint:staticcheck directive follow the same pattern as pkg/mcs/router/server/sync.go, maintaining consistency across the codebase. The existing comment explains the intentional blocking behavior.

pkg/utils/testutil/leak.go (1)

23-23: LGTM - Consistent with the client package change.

This update mirrors the change in client/pkg/utils/testutil/leak.go, ensuring both testutil packages correctly ignore the renamed gRPC internal goroutine after the version upgrade.

client/pkg/utils/testutil/leak.go (1)

24-24: Goleak ignore rule for gRPC v1.75.1 transport connection management is correct.

The resetTransportAndUnlock function is a real gRPC-Go internal function in the addrConn type responsible for transport connection management. Adding this to the goleak ignore list prevents false positive goroutine leak detection during tests.

tools/go.mod (2)

119-119: Verification confirmed: JWT v4.5.2 addresses CVE-2025-30204.

Version v4.5.2 fully resolves CVE-2025-30204 (excessive memory allocations in parse.ParseUnverified) as confirmed by GitHub security advisory GHSA-mh63-6h87-95cp. No instances of the vulnerable v3.x remain in the dependency graph.


162-162: Verify commit hash alignment with PR objectives.

The pseudo-version v0.0.0-20260112071732-805c56a830e0 in tools/go.mod (line 162) contains the short commit hash 805c56a830e0. Confirm this matches the intended tidb-dashboard commit specified in the PR description, as stated expectations need verification against the actual commit referenced in the version string.

go.mod (1)

69-77: This separate require block is intentionally preserved and requires no changes.

The go mod tidy command preserves this separation of 7 indirect dependencies (lines 69–77), which confirms the separation is intentional rather than accidental. Go's module system would consolidate these into the main indirect block if they were unintentionally separated, but since they remain separate after running go mod tidy, the structure is deliberate. If this separation was introduced as part of the tidb-dashboard update to highlight new transitive dependencies, that organizational choice is valid and should be retained.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

github.com/pingcap/metering_sdk v0.0.0-20250918015914-468cd6feb1dc
github.com/pingcap/sysutil v1.0.1-0.20230407040306-fb007c5aff21
github.com/pingcap/tidb-dashboard v0.0.0-20250928152019-e111aaee93f4
github.com/pingcap/tidb-dashboard v0.0.0-20260112071732-805c56a830e0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Primary change to address CVE-2025-30204 - verify the commit hash.

This tidb-dashboard update is the main change to eliminate the vulnerable JWT dependency. The pseudo-version v0.0.0-20260112071732-805c56a830e0 should correspond to commit 121f017c2150ec1037199e18e5f255aac057d48f as stated in the PR objectives. However, the short hash in the pseudo-version (805c56a830e0) does not match the prefix of the expected commit hash (121f017c). Verify that the correct commit was used.

#!/bin/bash
# Verify the actual commit hash used in the dependency

echo "=== Extracting commit information for tidb-dashboard ==="
go mod download -json github.com/pingcap/[email protected] 2>/dev/null | jq -r '{Version: .Version, Time: .Time, Dir: .Dir}' || echo "Download failed, trying alternate method"

echo -e "\n=== Expected commit from PR objectives ==="
echo "121f017c2150ec1037199e18e5f255aac057d48f"

echo -e "\n=== Checking go.sum for the full commit hash ==="
grep "pingcap/tidb-dashboard.*20260112071732" go.sum | head -1

If the commit hash doesn't match, the wrong version of tidb-dashboard may have been pulled in, which could mean the JWT vulnerability fix is not properly applied.

🤖 Prompt for AI Agents
In `@go.mod` at line 42, The go.mod entry uses pseudo-version
v0.0.0-20260112071732-805c56a830e0 which does not match the expected commit
121f017c2150ec1037199e18e5f255aac057d48f; verify the actual commit by running
the provided go mod download/check commands (or inspect go.sum) and if the wrong
commit was used, update the go.mod dependency to the pseudo-version that
corresponds to commit 121f017c2150ec1037199e18e5f255aac057d48f (or replace with
an exact tag/commit), run go mod tidy to refresh go.sum, re-run builds/tests,
and commit the corrected go.mod/go.sum so the JWT fix is truly included.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@client/go.mod`:
- Line 19: Update the client module metadata to reflect that no functional
change is required: ensure the google.golang.org/grpc version remains v1.75.1 in
client/go.mod, run go mod tidy to confirm no further edits, and remove or revise
the PR claim that this upgrade was required for tidb-dashboard compatibility
(there is no evidence in the repo that tidb-dashboard necessitates this gRPC
bump); reference the literal module line "google.golang.org/grpc v1.75.1" when
making the tidy/commit and update the PR description accordingly.

In `@tools/go.mod`:
- Around line 240-243: The repo pins testify to v1.10.0 to avoid behavior
changes in Eventually/EventuallyWithT; update tests or refactor the wrapper to
remove this dependency: either fix timing assumptions in tests that rely on
delayed evaluation or change testutil.Eventually() to explicitly implement the
older lazy-check semantics (so it behaves the same across testify versions); add
a tracking issue for this technical debt and correct the comment typo from
"flasky tests" to "flaky tests"; after changes run make tidy to update
tools/go.mod and tools/go.sum.
🧹 Nitpick comments (1)
tests/integrations/mcs/keyspace/tso_keyspace_group_test.go (1)

108-108: Consider adding a TODO comment for consistency.

Other files in this PR (e.g., pkg/utils/grpcutil/grpcutil.go, pkg/utils/testutil/testutil.go) include a TODO comment before the nolint directive to track the migration to grpc.NewClient. For consistency and to help track technical debt, consider adding a similar comment:

-	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) //nolint:staticcheck
+	// TODO: use grpc.NewClient instead of grpc.Dial.
+	//nolint:staticcheck
+	conn, err := grpc.Dial(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2bd7b3f and fec1e7f.

⛔ Files ignored due to path filters (4)
  • client/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
  • tests/integrations/go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (22)
  • client/go.mod
  • client/pkg/utils/grpcutil/grpcutil.go
  • client/pkg/utils/testutil/leak.go
  • client/servicediscovery/service_discovery_test.go
  • go.mod
  • pkg/mcs/router/server/sync.go
  • pkg/mcs/tso/server/testutil.go
  • pkg/syncer/client.go
  • pkg/utils/grpcutil/grpcutil.go
  • pkg/utils/testutil/leak.go
  • pkg/utils/testutil/testutil.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/go.mod
  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tests/integrations/mcs/tso/proxy_test.go
  • tests/integrations/mcs/tso/server_test.go
  • tools/go.mod
  • tools/pd-api-bench/main.go
  • tools/pd-region-bench/main.go
  • tools/pd-simulator/simulator/client.go
  • tools/pd-tso-bench/main.go
🚧 Files skipped from review as they are similar to previous changes (11)
  • pkg/mcs/router/server/sync.go
  • tests/integrations/mcs/scheduling/server_test.go
  • tools/pd-simulator/simulator/client.go
  • tools/pd-tso-bench/main.go
  • pkg/mcs/tso/server/testutil.go
  • tests/integrations/mcs/tso/proxy_test.go
  • client/servicediscovery/service_discovery_test.go
  • tests/integrations/client/client_tls_test.go
  • tests/integrations/mcs/tso/server_test.go
  • client/pkg/utils/grpcutil/grpcutil.go
  • pkg/syncer/client.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use gci/goimports ordering for Go imports: stdlib | third-party | pingcap | tikv/pd | blank
Avoid dot-imports in Go; revive rule will warn
Run gofmt/goimports/gci on all touched Go files; apply gofmt rewrite rule to convert interface{} to any
Prefer concrete types in Go; keep structs zero-value friendly; initialize maps and slices before use
Use pointers for large structs; avoid pointer-to-interface and copying mutex holders
Use typed constants with iota for enums in Go; keep error codes in errors.toml and regenerate via make generate-errdoc
First parameter must be context.Context for external effects in Go; never store contexts in structs
Keep Go package directory names in lower_snake case; filenames can be kebab or underscore
Use uppercase acronyms in Go (TSO, API, HTTP)
Exported identifiers in Go must have GoDoc comments starting with the identifier name; avoid stutter (e.g., pd.PDServer should be Server)
Wrap errors in Go using github.com/pingcap/errors (errors.Wrap/Annotate) or fmt.Errorf("...: %w", err)
Do not ignore errors in Go unless allowed in .golangci.yml errcheck exclusions
Error strings in Go must be lowercase with no trailing punctuation
Prefer sentinel errors over panic in Go; only panic on programmer bugs or impossible states
HTTP handlers in Go must use errcode and errorResp; avoid http.Error
Use existing structured logging (zap/log) in Go; avoid fmt.Println
Include useful fields in Go logs (component, store id, region id); never log secrets or PII
Cancel timers and tickers in Go; close resources with defer and error checks
sync.WaitGroup in Go must be a pointer, not a value; revive rule enforces this
Prevent goroutine leaks in Go: pair goroutines with cancellation; consider using errgroup
Guard shared state in Go with mutex or RWMutex; keep lock ordering consistent
Do not capture loop variables by pointer in Go; copy the value inside the loop (v := val)
Preallocate slices and maps in Go when size is kno...

Files:

  • tests/integrations/mcs/keyspace/tso_keyspace_group_test.go
  • pkg/utils/testutil/testutil.go
  • pkg/utils/testutil/leak.go
  • tools/pd-region-bench/main.go
  • client/pkg/utils/testutil/leak.go
  • pkg/utils/grpcutil/grpcutil.go
  • tools/pd-api-bench/main.go
go.mod

📄 CodeRabbit inference engine (AGENTS.md)

Run make tidy after dependency changes; go.mod and go.sum must remain clean with no diff

Files:

  • go.mod
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Ensure depguard bans are respected: no `github.com/pkg/errors`, `go.uber.org/atomic`, or `math/rand`; prefer `math/rand/v2`
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Language is Go modules (root + `client/` submodule); Go version >= 1.25 required
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : gRPC services in Go must use interceptors; add auth and validation in handlers, not in transport layer
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Do not disable lint or security checks in Go without discussion
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Prevent goroutine leaks in Go: pair goroutines with cancellation; consider using errgroup
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Respect the golangci-lint `.golangci.yml` configuration: gofmt, goimports, gci formatting; depguard; goheader copyright; revive and testifylint rules; forbid WaitGroup by value
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Swagger specification: run `make swagger-spec` (with SWAGGER=1) to regenerate; keep annotations current
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : gRPC services in Go must use interceptors; add auth and validation in handlers, not in transport layer

Applied to files:

  • pkg/utils/testutil/testutil.go
  • client/go.mod
  • client/pkg/utils/testutil/leak.go
  • go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Prevent goroutine leaks in Go: pair goroutines with cancellation; consider using errgroup

Applied to files:

  • pkg/utils/testutil/leak.go
  • client/go.mod
  • client/pkg/utils/testutil/leak.go
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Language is Go modules (root + `client/` submodule); Go version >= 1.25 required

Applied to files:

  • client/go.mod
  • go.mod
  • tests/integrations/go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Ensure depguard bans are respected: no `github.com/pkg/errors`, `go.uber.org/atomic`, or `math/rand`; prefer `math/rand/v2`

Applied to files:

  • client/go.mod
  • go.mod
  • tests/integrations/go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Avoid per-request reflection in Go; keep hot paths lean

Applied to files:

  • client/go.mod
  • go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Swagger specification: run `make swagger-spec` (with SWAGGER=1) to regenerate; keep annotations current

Applied to files:

  • go.mod
  • tests/integrations/go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Use gci/goimports ordering for Go imports: stdlib | third-party | pingcap | tikv/pd | blank

Applied to files:

  • go.mod
  • tests/integrations/go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to **/*.go : Add tests for new metrics when feasible; use helpers from `metrics/` package

Applied to files:

  • go.mod
  • tests/integrations/go.mod
  • tools/go.mod
📚 Learning: 2026-01-16T05:51:31.264Z
Learnt from: CR
Repo: tikv/pd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T05:51:31.264Z
Learning: Applies to go.mod : Run `make tidy` after dependency changes; `go.mod` and `go.sum` must remain clean with no diff

Applied to files:

  • tests/integrations/go.mod
  • tools/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: chunks (5, Tests(2))
  • GitHub Check: chunks (7, Client Integration Test)
  • GitHub Check: chunks (9, Microservice Integration(!TSO))
  • GitHub Check: chunks (10, Microservice Integration(TSO))
  • GitHub Check: chunks (8, TSO Integration Test)
  • GitHub Check: chunks (6, Tools Test)
  • GitHub Check: chunks (2, Unit Test(2))
  • GitHub Check: chunks (3, Unit Test(3))
  • GitHub Check: chunks (1, Unit Test(1))
  • GitHub Check: chunks (4, Tests(1))
  • GitHub Check: chunks
  • GitHub Check: statics
  • GitHub Check: tso-function-test
🔇 Additional comments (18)
tools/pd-region-bench/main.go (1)

137-143: LGTM with optional improvement.

The nolint directive correctly suppresses deprecation warnings. For consistency with pkg/utils/grpcutil/grpcutil.go and pkg/utils/testutil/testutil.go, consider adding a TODO comment to track the migration intent.

pkg/utils/grpcutil/grpcutil.go (1)

172-174: LGTM - Well-documented deprecation suppression.

The TODO comment clearly documents the migration intent to grpc.NewClient, and the nolint directive appropriately suppresses the deprecation warning. This is a good pattern for tracking technical debt while maintaining compatibility with the current gRPC version.

pkg/utils/testutil/testutil.go (1)

84-86: LGTM - Consistent with other changes in this PR.

The TODO comment and nolint directive follow the same well-documented pattern used in pkg/utils/grpcutil/grpcutil.go, making it easy to track and address this deprecation when migrating to grpc.NewClient.

tools/pd-api-bench/main.go (1)

388-394: LGTM with optional improvement.

The nolint directive correctly suppresses the deprecation warning. For consistency with pkg/utils/grpcutil/grpcutil.go and pkg/utils/testutil/testutil.go, consider adding a TODO comment to track the migration intent.

pkg/utils/testutil/leak.go (1)

23-23: LGTM!

The goleak ignore pattern correctly updated to match the renamed gRPC internal function (resetTransportAndUnlock) introduced in newer gRPC versions. This ensures leak detection continues to work properly with gRPC v1.75.1.

client/pkg/utils/testutil/leak.go (1)

24-24: LGTM!

Consistent with the same change in pkg/utils/testutil/leak.go, updating the goleak ignore pattern for the renamed gRPC internal function.

go.mod (4)

65-65: gRPC v1.75.1 upgrade is expected.

This gRPC upgrade is part of the tidb-dashboard dependency chain and aligns with the other module updates. The version is consistent across all go.mod files in this PR.


88-88: New indirect dependency: gin-jwt/v2.

This JWT middleware library is pulled in transitively via the tidb-dashboard update. It uses golang-jwt/jwt/v4 internally, which is the patched version.


42-42: CVE-2025-30204 fix confirmed via tidb-dashboard update.

The indirect dependency golang-jwt/jwt/v4 v4.5.2 is now present in go.mod, which addresses the JWT vulnerability. No vulnerable JWT v3 versions remain in the module. The tidb-dashboard update at line 42 successfully incorporates the necessary security patch.


248-252: Testify replace directive with documented TODO is appropriate.

The replace directive pinning testify to v1.10.0 addresses a breaking change in v1.11.x where Eventually/EventuallyWithT now check conditions immediately. With 50+ usages of Eventually across test files, this temporary workaround is necessary to maintain test stability. The go.mod is clean with matching go.sum hashes, indicating proper dependency management. The TODO in the comment provides tracking for future fixes when the flaky tests are addressed.

tests/integrations/go.mod (3)

21-28: Dependency updates consistent with root module.

The testify and gRPC version updates match the root go.mod, ensuring consistency across the repository.


156-156: tidb-dashboard indirect dependency updated.

The integration tests module correctly picks up the updated tidb-dashboard version transitively, ensuring the JWT vulnerability fix propagates to test builds.


237-241: Testify replace directive mirrors root module.

The same workaround is applied here to maintain consistent test behavior across modules. The TODO comment is appropriately documented.

tools/go.mod (5)

19-19: Direct dependency updates look appropriate.

The gin-gonic/gin and stretchr/testify version bumps align with the updated dependency graph from tidb-dashboard. Note that testify v1.11.1 here will be overridden by the replace directive at line 243.

Also applies to: 33-33


119-119: Security fix successfully applied: JWT v4.5.2 addresses CVE-2025-30204.

The vulnerable github.com/golang-jwt/jwt v3.2.2+incompatible has been eliminated and replaced with the patched v4.5.2, meeting the PR objective to address the high-severity vulnerability.


162-162: tidb-dashboard successfully updated to target commit.

The update to commit 805c56a830e0 matches the PR objective and brings in the patched JWT dependency, eliminating the CVE-2025-30204 vulnerability.


89-227: Indirect dependency updates appear consistent.

The transitive dependency updates (Bytedance sonic, OpenTelemetry v1.37.0, protobuf v1.36.10, and genproto packages) are expected cascading effects from the tidb-dashboard update. The version alignment across related packages (e.g., all OpenTelemetry packages at v1.37.0) indicates a clean dependency resolution.


67-67: Dependency verification complete. The new indirect dependency github.com/appleboy/gin-jwt/v2 v2.10.3 correctly depends on github.com/golang-jwt/jwt/v4 v4.5.2, which includes the security fix. No action required.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

go.uber.org/goleak v1.1.11
go.uber.org/zap v1.24.0
google.golang.org/grpc v1.62.1
google.golang.org/grpc v1.75.1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check the client module's go.mod and compare with root
echo "=== Client go.mod grpc version ==="
grep "google.golang.org/grpc" client/go.mod

echo ""
echo "=== Root go.mod grpc version ==="
grep "google.golang.org/grpc" go.mod

echo ""
echo "=== Check if client go.mod needs tidying ==="
cd client && go mod tidy -diff 2>&1 | head -100

Repository: tikv/pd

Length of output: 3763


🏁 Script executed:

# Check for tidb-dashboard references in the codebase
echo "=== Searching for tidb-dashboard references ==="
rg "tidb-dashboard" --type go -i 2>&1 | head -20

Repository: tikv/pd

Length of output: 1583


🌐 Web query:

gRPC v1.75.1 release stability

💡 Result:

Short answer: v1.75.1 is a small patch release (bug-fix / stability fixes) and is considered stable — it fixes several race conditions and Python asyncio shutdown issues; no high‑severity/vetted vulnerabilities are reported for grpcio 1.75.1 in public vulnerability databases. See the upstream release notes for details. [1][2][3]

Sources:

  • gRPC (core) v1.75.1 release notes (changelog / bugfixes for Python, etc.). [1]
  • gRPC-Go v1.75.1 release notes (race/data‑race and flow‑control fixes). [2]
  • Snyk package page: grpcio 1.75.1 — no direct vulnerabilities reported. [3]

References:
[1] GitHub — grpc/grpc releases: v1.75.1.
[2] GitHub — grpc/grpc-go releases: v1.75.1.
[3] Snyk — grpcio 1.75.1 vulnerability summary.


gRPC v1.75.1 upgrade aligns between root and client modules.

Both go.mod files specify v1.75.1, which is a stable patch release containing bug fixes for race conditions and flow control. The client module's go.mod is already tidy with no changes needed.

Note: The claim that this upgrade is necessary for tidb-dashboard compatibility cannot be substantiated from the codebase—tidb-dashboard is a dependency, but there is no evidence of a recent update to it that would require this gRPC version.

🤖 Prompt for AI Agents
In `@client/go.mod` at line 19, Update the client module metadata to reflect that
no functional change is required: ensure the google.golang.org/grpc version
remains v1.75.1 in client/go.mod, run go mod tidy to confirm no further edits,
and remove or revise the PR claim that this upgrade was required for
tidb-dashboard compatibility (there is no evidence in the repo that
tidb-dashboard necessitates this gRPC bump); reference the literal module line
"google.golang.org/grpc v1.75.1" when making the tidy/commit and update the PR
description accordingly.

Comment on lines +240 to +243
// Due to https://github.com/stretchr/testify/pull/1427, `Eventually` and `EventuallyWithT` starts checking the condition immediately,
// which will cause several different tests to fail. So this is a temporary workaround to use the old version of `testify`.
// TODO: fix those flasky tests introduced by the behavior change of `Eventually` and `EventuallyWithT` assertions.
replace github.com/stretchr/testify => github.com/stretchr/testify v1.10.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check the actual versions in tools/go.mod and tools/go.sum
head -n 280 tools/go.mod | tail -n 50

Repository: tikv/pd

Length of output: 2698


🏁 Script executed:

# Find all usages of Eventually and EventuallyWithT in test files
rg -n '(Eventually|EventuallyWithT)' --type=go -g '*_test.go' | head -50

Repository: tikv/pd

Length of output: 5094


🏁 Script executed:

# Check if there's a TODO or issue tracking this testify workaround
rg -n 'testify|Eventually|EventuallyWithT' tools/go.mod -A2 -B2

Repository: tikv/pd

Length of output: 761


🏁 Script executed:

# Check if there are any related issues or PRs mentioning this
rg -rn 'flasky tests|Eventually.*behavior' --type=go --type=md | head -30

Repository: tikv/pd

Length of output: 33


🏁 Script executed:

# Check testutil.Eventually to see if it's a custom wrapper or direct testify usage
rg -n 'func Eventually|import.*testify' --type=go tools/ | head -30

Repository: tikv/pd

Length of output: 33


🏁 Script executed:

# Look at tools/go.sum to see actual pinned testify version
rg 'github.com/stretchr/testify' tools/go.sum

Repository: tikv/pd

Length of output: 225


🏁 Script executed:

# Check if make tidy was run by looking at go.sum structure
wc -l tools/go.sum
stat tools/go.sum

Repository: tikv/pd

Length of output: 129


🏁 Script executed:

# Search for any related issues in the codebase mentioning this pinning
rg -i 'eventually.*condition|testify.*v1.10|pr.*1427' --type=md --type=go

Repository: tikv/pd

Length of output: 510


🏁 Script executed:

# Check the implementation of testutil.Eventually to understand if it depends on testify.Eventually
cat -n pkg/utils/testutil/testutil.go | head -100

Repository: tikv/pd

Length of output: 3648


🏁 Script executed:

# Also check if there's a version in the main repo (not tools)
grep -n "github.com/stretchr/testify" go.mod

Repository: tikv/pd

Length of output: 308


🏁 Script executed:

# Look for any references to the stretchr/testify PR in issues, comments, or documentation
rg -i 'pr.*1427|stretchr/testify.*1427' --type=md --type=txt

Repository: tikv/pd

Length of output: 33


Address technical debt: testify downgrade required to maintain test compatibility.

The replace directive pins testify to v1.10.0 in tools/go.mod because v1.11.1 changed the behavior of Eventually/EventuallyWithT to check conditions immediately, breaking tests that depend on lazy evaluation. The custom testutil.Eventually() wrapper delegates to testify's underlying method, so this behavior change directly impacts test execution.

To resolve this:

  • Fix the timing assumptions in the tests that currently depend on v1.10.0's delayed checking, or refactor testutil.Eventually() to make behavior version-agnostic
  • Create a tracking issue if one doesn't exist
  • After making changes, run make tidy to ensure tools/go.mod and tools/go.sum are properly updated

Note: "flasky tests" in the comment should be "flaky tests".

🤖 Prompt for AI Agents
In `@tools/go.mod` around lines 240 - 243, The repo pins testify to v1.10.0 to
avoid behavior changes in Eventually/EventuallyWithT; update tests or refactor
the wrapper to remove this dependency: either fix timing assumptions in tests
that rely on delayed evaluation or change testutil.Eventually() to explicitly
implement the older lazy-check semantics (so it behaves the same across testify
versions); add a tracking issue for this technical debt and correct the comment
typo from "flasky tests" to "flaky tests"; after changes run make tidy to update
tools/go.mod and tools/go.sum.

@JmPotato
Copy link
Member Author

/retest

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.57%. Comparing base (6dbd9cd) to head (fec1e7f).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10110      +/-   ##
==========================================
+ Coverage   78.56%   78.57%   +0.01%     
==========================================
  Files         520      520              
  Lines       69676    69720      +44     
==========================================
+ Hits        54739    54784      +45     
+ Misses      10990    10985       -5     
- Partials     3947     3951       +4     
Flag Coverage Δ
unittests 78.57% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JmPotato
Copy link
Member Author

/retest

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 16, 2026
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 21, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 21, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: okJiang, rleungx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the approved label Jan 21, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 21, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-01-16 08:19:05.426946461 +0000 UTC m=+136373.040903307: ☑️ agreed by okJiang.
  • 2026-01-21 03:23:58.590280255 +0000 UTC m=+550666.204237111: ☑️ agreed by rleungx.

@JmPotato
Copy link
Member Author

/retest

@ti-chi-bot ti-chi-bot bot merged commit 8fa85b4 into tikv:master Jan 21, 2026
50 of 54 checks passed
@JmPotato JmPotato deleted the update_dashboard branch January 21, 2026 05:15
bufferflies pushed a commit to bufferflies/pd that referenced this pull request Jan 21, 2026
…ency (tikv#10110)

close tikv#10109

Update tidb-dashboard to 805c56a830e0fcda07043e78f1d858ccf499babb to eliminate vulnerable `golang-jwt/jwt` dependency.

Signed-off-by: JmPotato <[email protected]>
bufferflies pushed a commit to bufferflies/pd that referenced this pull request Jan 22, 2026
…ency (tikv#10110)

close tikv#10109

Update tidb-dashboard to 805c56a830e0fcda07043e78f1d858ccf499babb to eliminate vulnerable `golang-jwt/jwt` dependency.

Signed-off-by: JmPotato <[email protected]>
@JmPotato
Copy link
Member Author

/cherry-pick release-8.5

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Jan 23, 2026
@ti-chi-bot
Copy link
Member

@JmPotato: new pull request created to branch release-8.5: #10186.
But this PR has conflicts, please resolve them!

Details

In response to this:

/cherry-pick release-8.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eliminate vulnerable golang-jwt/jwt dependency

4 participants