build: explicitly require tonic v1.12.3 #2357
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/dapi-grpc/Cargo.toml (1)
Line range hint
41-42: Consider separating unrelated dependency updates.The updates to
serde(1.0.197) andserde_bytes(0.11.12) appear unrelated to the security fix for tonic. Consider handling these updates in a separate PR to maintain a focused scope on the security vulnerability fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/dapi-grpc/Cargo.toml(2 hunks)
🔇 Additional comments (3)
packages/dapi-grpc/Cargo.toml (3)
52-52: LGTM! Correct synchronization with tonic version.
The tonic-build version is properly synchronized with the tonic dependency version.
38-41: LGTM! Explicit version requirement addresses security vulnerability.
The update to tonic 0.12.3 properly addresses RUSTSEC-2024-0376 (DoS vulnerability in tonic::transport::Server).
Let's verify there are no other dependencies that might affect tonic's version resolution:
✅ Verification successful
Tonic version update is correctly synchronized across dependencies
The verification confirms that both tonic and tonic-build dependencies are consistently set to version 0.12.3, with no other tonic-related dependencies in the workspace that could affect version resolution. This ensures the security fix for RUSTSEC-2024-0376 is properly applied.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other dependencies that might affect tonic's version
# Look for any other potential tonic dependencies in the workspace
rg --type toml 'tonic.*=.*'
Length of output: 169
Line range hint 19-27: Verify server feature configuration for security fix.
The security fix in tonic 0.12.3 specifically addresses issues in tonic::transport::Server. The 'server' feature correctly includes the transport module, ensuring the fix will be active when this feature is used.
Let's verify the server feature usage:
✅ Verification successful
Let me verify the server feature configuration by checking the Cargo.toml files and feature dependencies.
Server feature configuration is properly secured
The verification confirms that:
- The
serverfeature indapi-grpccorrectly includestonic/transportand is properly configured - The implementation in
rs-drive-abci/src/server.rsusestonic::transport::Serverthrough thedapi-grpccrate - The feature dependencies are correctly set up to ensure the security fix from tonic 0.12.3 is active
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for server feature usage in the codebase
# Look for potential server implementations using tonic
rg --type rust "tonic::transport::Server"
Length of output: 147
Script:
#!/bin/bash
# Check Cargo.toml files for server feature and tonic dependencies
rg -A 5 'server.*=.*\[' packages/*/Cargo.toml
# Check direct usage of tonic transport in rs-drive-abci
cat packages/rs-drive-abci/src/server.rs
Length of output: 4509
lklimek
changed the title
lklimek
changed the title
| @@ -35,7 +35,7 @@ mocks = ["serde", "dep:serde_json"] | |||
| [dependencies] | |||
| prost = { version = "0.13" } | |||
| futures-core = "0.3.30" | |||
| tonic = { version = "0.12", features = [ | |||
| tonic = { version = "0.12.3", features = [ | |||
There was a problem hiding this comment.
No, we allow upgrades, Patched >=0.12.3 https://rustsec.org/advisories/RUSTSEC-2024-0376.html
lklimek
changed the title
Issue being fixed or feature implemented
Tonic < 0.12.3 has security vulnerability
Closes #2199
What was done?
Explicitly require tonic >= 0.12.3.
This PR does not change anything, as we already build against 0.12.3 (at least since v1.4.0). It just adds formal requirement.
How Has This Been Tested?
GHA
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit