Deny unknown fields on RPC Request and Response#6926
Draft
sudo-shashank wants to merge 5 commits intomainfrom
Draft
Deny unknown fields on RPC Request and Response#6926sudo-shashank wants to merge 5 commits intomainfrom
sudo-shashank wants to merge 5 commits intomainfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 32: Update the CHANGELOG entry that currently references the PR number
[`#6926`] to reference the tracked issue numbers instead; replace the PR link and
label with the issue reference(s) [`#5600`] and/or [`#5635`] and the corresponding
issue URL(s) while keeping the rest of the description unchanged ("Added strict
JSON validation to deny unknown fields in RPC request parameters and response
results when `FOREST_STRICT_JSON` is enabled."); edit the exact line that
contains the current "- [`#6926`](...): ..." entry so the format matches other
changelog entries (use [`#ISSUE_NO`](link-to-issue): <description>).
In `@src/rpc/reflect/mod.rs`:
- Around line 278-285: The current validation only re-serializes Forest's own
LotusJson before returning; to actually reject unknown fields from remote nodes
update RpcMethodExt::call_raw to validate the incoming JSON payload using
crate::rpc::json_validator::from_value_rejecting_unknown_fields (into the
<Self::Ok as HasLotusJson>::LotusJson type) instead of using plain
serde_json::from_value, so the client.call(...) result is passed through
from_value_rejecting_unknown_fields (honoring FOREST_STRICT_JSON behavior) and
any unknown fields cause an error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9e9c365b-6589-49ef-96e3-d17f624df0ce
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.mdCargo.tomldocs/docs/users/reference/env_variables.mdsrc/rpc/json_validator.rssrc/rpc/reflect/mod.rssrc/rpc/reflect/parser.rs
Comment on lines
+278
to
+285
| let result = ok.into_lotus_json(); | ||
| if crate::rpc::json_validator::is_strict_mode() { | ||
| let v = serde_json::to_value(&result).map_err(Error::from)?; | ||
| let _: <Self::Ok as HasLotusJson>::LotusJson = | ||
| crate::rpc::json_validator::from_value_rejecting_unknown_fields(v) | ||
| .map_err(Error::from)?; | ||
| } | ||
| Result::<_, jsonrpsee::types::ErrorObjectOwned>::Ok(result) |
Contributor
There was a problem hiding this comment.
This doesn't validate incoming RPC responses.
The round-trip here only self-checks Forest's own LotusJson before returning it. Line 360 in RpcMethodExt::call_raw still deserializes remote payloads with plain serde_json::from_value(json), so extra fields from Lotus/other nodes will continue to bypass FOREST_STRICT_JSON.
Possible fix
fn call_raw(
client: &crate::rpc::client::Client,
params: Self::Params,
) -> impl Future<Output = Result<<Self::Ok as HasLotusJson>::LotusJson, jsonrpsee::core::ClientError>>
{
async {
let json = client.call(Self::request(params)?.map_ty()).await?;
Ok(crate::rpc::json_validator::from_value_rejecting_unknown_fields(json)?)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rpc/reflect/mod.rs` around lines 278 - 285, The current validation only
re-serializes Forest's own LotusJson before returning; to actually reject
unknown fields from remote nodes update RpcMethodExt::call_raw to validate the
incoming JSON payload using
crate::rpc::json_validator::from_value_rejecting_unknown_fields (into the
<Self::Ok as HasLotusJson>::LotusJson type) instead of using plain
serde_json::from_value, so the client.call(...) result is passed through
from_value_rejecting_unknown_fields (honoring FOREST_STRICT_JSON behavior) and
any unknown fields cause an error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
FOREST_STRICT_JSONis enabled.Reference issue to close (if applicable)
Closes #5600
Closes #5635
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
New Features
FOREST_STRICT_JSONis enabled, RPC request parameters and response results now reject unknown fields in addition to duplicate keys.Documentation