Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

### Added

- [#6926](https://github.com/ChainSafe/forest/pull/6926): Added strict JSON validation to deny unknown fields in RPC request parameters and response results when `FOREST_STRICT_JSON` is enabled.
Comment thread
sudo-shashank marked this conversation as resolved.

### Changed

### Removed
Expand Down
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ schemars = { version = "1", features = ["chrono04", "uuid1"] }
scopeguard = "1"
semver = "1"
serde = { workspace = true }
serde_ignored = "0.1"
serde_ipld_dagcbor = "0.6"
serde_json = { version = "1", features = ["raw_value"] }
serde_with = { version = "3", features = ["chrono_0_4"] }
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/users/reference/env_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ process.
| `FOREST_JWT_DISABLE_EXP_VALIDATION` | 1 or true | empty | 1 | Whether or not to disable JWT expiration validation |
| `FOREST_ETH_BLOCK_CACHE_SIZE` | positive integer | 500 | 1 | The size of Eth block cache |
| `FOREST_RPC_BACKFILL_FULL_TIPSET_FROM_NETWORK` | 1 or true | false | 1 | Whether or not to backfill full tipsets from the p2p network |
| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests |
| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys and reject unknown fields in RPC requests and responses |
| `FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH` | URL or file path | empty | `/var/tmp/forest_snapshot_calibnet.forest.car.zst` | Override snapshot path for `--auto-download-snapshot` |
| `FOREST_DOWNLOAD_CONNECTIONS` | positive integer | 5 | 10 | Number of parallel HTTP connections for downloading snapshots |
| `FOREST_ETH_V1_DISABLE_F3_FINALITY_RESOLUTION` | 1 or true | empty | 1 | Whether or not to disable F3 finality resolution in Eth `v1` RPC methods |
Expand Down
86 changes: 83 additions & 3 deletions src/rpc/json_validator.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
// Copyright 2019-2026 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

//! JSON validation utilities for detecting duplicate keys before serde_json processing.
//! JSON validation utilities for RPC requests and responses processing.
//!
//! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy
//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behavior in RPC calls.
//! - **Duplicate key detection**: `serde_json` automatically deduplicates keys at parse time
//! using a "last-write-wins" strategy. This means JSON like `{"/":"cid1", "/":"cid2"}` will
//! keep only the last value, which can lead to unexpected behavior in RPC calls.
//! - **Unknown field detection**: `serde_json` silently ignores unknown fields by default.
//! In strict mode, [`from_value_rejecting_unknown_fields`] applies to rpc request and
//! responses.
//!
//! All of this is gated behind the `FOREST_STRICT_JSON` environment variable.

use ahash::HashSet;
use serde::de::DeserializeOwned;

pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON";

Expand Down Expand Up @@ -50,9 +57,32 @@ pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> {
check_value(&value)
}

/// De-serializes a [`serde_json::Value`] into `T`, rejecting unknown fields when strict mode is
/// enabled. When strict mode is off, this is equivalent to [`serde_json::from_value`].
pub fn from_value_rejecting_unknown_fields<T: DeserializeOwned>(
value: serde_json::Value,
) -> Result<T, serde_json::Error> {
if !is_strict_mode() {
return serde_json::from_value(value);
}
let mut unknown = Vec::new();
let result: T = serde_ignored::deserialize(value, |path| {
unknown.push(path.to_string());
})?;
if !unknown.is_empty() {
return Err(serde::de::Error::custom(format!(
"unknown field(s): {}. Set {STRICT_JSON_ENV}=0 to disable this check",
unknown.join(", ")
)));
}
Ok(result)
}

#[cfg(test)]
mod tests {
use super::*;
use serde::Deserialize;
use serde_json::json;
use serial_test::serial;

fn with_strict_mode<F>(enabled: bool, f: F)
Expand Down Expand Up @@ -131,4 +161,54 @@ mod tests {
assert!(result.unwrap_err().contains("duplicate key '/'"));
});
}

#[derive(Debug, Deserialize, PartialEq)]
struct RpcTestReq {
name: String,
value: i32,
}

#[test]
#[serial]
fn test_unknown_fields_known_only() {
with_strict_mode(true, || {
let val = json!({"name": "alice", "value": 42});
let result = from_value_rejecting_unknown_fields::<RpcTestReq>(val);
assert_eq!(
result.unwrap(),
RpcTestReq {
name: "alice".into(),
value: 42
}
);
});
}

#[test]
#[serial]
fn test_unknown_fields_detected() {
with_strict_mode(true, || {
let val = json!({"name": "alice", "value": 42, "extra": true});
let err = from_value_rejecting_unknown_fields::<RpcTestReq>(val)
.expect_err("expected Err when unknown JSON field is present under strict mode");
let msg = err.to_string();
assert!(
msg.contains("unknown field(s)") && msg.contains("extra"),
"got: {msg}"
);
});
}

#[test]
#[serial]
fn test_unknown_fields_strict_mode_off() {
with_strict_mode(false, || {
let val = json!({"name": "alice", "value": 42, "extra": true});
let result = from_value_rejecting_unknown_fields::<RpcTestReq>(val);
assert!(
result.is_ok(),
"unknown fields should be allowed when strict mode is off"
);
});
}
}
9 changes: 8 additions & 1 deletion src/rpc/reflect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,14 @@ pub trait RpcMethodExt<const ARITY: usize>: RpcMethod<ARITY> {
let params = Self::parse_params(params.as_str(), calling_convention)
.map_err(|e| Error::invalid_params(e, None))?;
let ok = Self::handle(ctx, params, &extensions).await?;
Result::<_, jsonrpsee::types::ErrorObjectOwned>::Ok(ok.into_lotus_json())
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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
},
)?;
if let Some(alias) = Self::NAME_ALIAS {
Expand Down
6 changes: 3 additions & 3 deletions src/rpc/reflect/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::Deserialize;
use serde_json::{Value, json};

use super::{jsonrpc_types::RequestParameters, util::Optional as _};
use crate::rpc::error::ServerError;
use crate::rpc::{error::ServerError, json_validator};

/// Parser for JSON-RPC parameters.
/// Abstracts calling convention, checks for unexpected params etc, so that
Expand Down Expand Up @@ -142,7 +142,7 @@ impl<'a> Parser<'a> {
false => self.error(missing_parameter)?,
},
Some(ParserInner::ByName(it)) => match it.remove(name) {
Some(it) => match serde_json::from_value::<T>(it) {
Some(it) => match json_validator::from_value_rejecting_unknown_fields::<T>(it) {
Ok(it) => it,
Err(e) => self.error(deserialize_error(e))?,
},
Expand All @@ -152,7 +152,7 @@ impl<'a> Parser<'a> {
},
},
Some(ParserInner::ByPosition(it)) => match it.pop_front() {
Some(it) => match serde_json::from_value::<T>(it) {
Some(it) => match json_validator::from_value_rejecting_unknown_fields::<T>(it) {
Ok(it) => it,
Err(e) => self.error(deserialize_error(e))?,
},
Expand Down
Loading