Skip to content

Commit 156c26b

Browse files
committed
fix(jsonrpc): reject policy params matchers
Preserve params for MCP and request parsing, but make generic JSON-RPC policy matching method-only. Signed-off-by: Kris Hicks <khicks@nvidia.com>
1 parent 014fd35 commit 156c26b

11 files changed

Lines changed: 165 additions & 416 deletions

File tree

architecture/sandbox.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,14 @@ identifies the calling binary, checks trust-on-first-use binary identity, reject
5151
unsafe internal destinations, and evaluates the active policy.
5252
For inspected HTTP traffic, the proxy can enforce REST method/path rules,
5353
WebSocket upgrade and text-message rules, GraphQL operation rules, and
54-
MCP or generic JSON-RPC method and params rules on sandbox-to-server request
55-
bodies. MCP and JSON-RPC inspection buffers up to the endpoint
56-
`mcp.max_body_bytes` or `json_rpc.max_body_bytes` limit. MCP `tools/call`
57-
tool names are checked against the spec-recommended syntax by default before
58-
policy evaluation, with a per-endpoint `mcp.strict_tool_names` compatibility
59-
opt-out. Literal dotted keys in generic JSON-RPC params are accepted. If a
60-
literal key and a flattened nested selector path produce the same matcher key,
61-
the literal key takes precedence.
54+
MCP method, tool, and supported params rules or generic JSON-RPC method rules
55+
on sandbox-to-server request bodies. MCP and JSON-RPC inspection buffers up to
56+
the endpoint `mcp.max_body_bytes` or `json_rpc.max_body_bytes` limit. MCP
57+
`tools/call` tool names are checked against the spec-recommended syntax by
58+
default before policy evaluation, with a per-endpoint `mcp.strict_tool_names`
59+
compatibility opt-out. Generic JSON-RPC request `params` are valid payload data
60+
but are not presently supported as policy matchers while we settle the user
61+
experience for nested payload rules.
6262
JSON-RPC responses and server-to-client MCP messages on response or SSE streams
6363
are relayed but are not currently parsed for policy enforcement.
6464

crates/openshell-policy/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ enum QueryMatcherDef {
241241
Any(QueryAnyDef),
242242
}
243243

244-
// JSON-RPC/MCP params can be authored as nested maps in YAML, but the runtime
245-
// matcher map remains flat so the Rego policy can share query-param matching.
244+
// MCP params can be authored as nested maps in YAML, but the runtime matcher
245+
// map remains flat so the Rego policy can share query-param matching.
246246
#[derive(Debug, Clone, Serialize, Deserialize)]
247247
#[serde(untagged)]
248248
enum ParamMatcherDef {
@@ -497,8 +497,8 @@ fn split_tool_param(
497497
protocol: &str,
498498
params: BTreeMap<String, QueryMatcherDef>,
499499
) -> (Option<QueryMatcherDef>, BTreeMap<String, QueryMatcherDef>) {
500-
// Only MCP has the tool-name convention. Generic JSON-RPC keeps `name` as a
501-
// normal params matcher so serialization does not invent MCP semantics.
500+
// Only MCP has the tool-name convention. Generic JSON-RPC preserves proto
501+
// params on round-trip without inventing MCP semantics.
502502
if !is_mcp_protocol(protocol) {
503503
return (None, params);
504504
}

crates/openshell-supervisor-network/data/sandbox-policy.rego

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -275,15 +275,15 @@ request_denied_for_endpoint(request, endpoint) if {
275275
command_matches(request.command, deny_rule.command)
276276
}
277277

278-
# --- L7 deny rule matching: JSON-RPC method + params ---
278+
# --- L7 deny rule matching: JSON-RPC method ---
279279

280280
request_denied_for_endpoint(request, endpoint) if {
281281
jsonrpc_family_endpoint(endpoint)
282282
request.method == "POST"
283283
some deny_rule
284284
deny_rule := endpoint.deny_rules[_]
285285
deny_rule.method
286-
jsonrpc_rule_matches(request, deny_rule)
286+
jsonrpc_rule_matches(request, endpoint, deny_rule)
287287
}
288288

289289
request_denied_for_endpoint(request, endpoint) if {
@@ -454,7 +454,7 @@ request_allowed_for_endpoint(request, endpoint) if {
454454
rule := endpoint.rules[_]
455455
rule.allow.method
456456
not jsonrpc_response_frame_present(request)
457-
jsonrpc_rule_matches(request, rule.allow)
457+
jsonrpc_rule_matches(request, endpoint, rule.allow)
458458
}
459459

460460
# MCP can allow the method layer by endpoint option while still using
@@ -742,10 +742,9 @@ query_value_matches(value, matcher) if {
742742
glob.match(any_patterns[i], [], value)
743743
}
744744

745-
# JSON-RPC method and params matching. The sandbox presents scalar object params
746-
# as dot-separated matcher keys, e.g. arguments.scope. Literal dotted param keys
747-
# are preserved by Rust flattening and take precedence over deeper nested paths.
748-
jsonrpc_rule_matches(request, rule) if {
745+
# JSON-RPC-family method matching. Generic JSON-RPC policies match only method;
746+
# MCP policies also match scalar params, including params.name from tool aliases.
747+
jsonrpc_rule_matches(request, endpoint, rule) if {
749748
jsonrpc := object.get(request, "jsonrpc", null)
750749
is_object(jsonrpc)
751750
method := object.get(jsonrpc, "method", "")
@@ -755,6 +754,15 @@ jsonrpc_rule_matches(request, rule) if {
755754
is_string(rule_method)
756755
rule_method != ""
757756
glob.match(rule_method, [], method)
757+
jsonrpc_rule_params_match_for_protocol(jsonrpc, endpoint, rule)
758+
}
759+
760+
jsonrpc_rule_params_match_for_protocol(_, endpoint, _) if {
761+
endpoint.protocol == "json-rpc"
762+
}
763+
764+
jsonrpc_rule_params_match_for_protocol(jsonrpc, endpoint, rule) if {
765+
endpoint.protocol == "mcp"
758766
jsonrpc_params_match(jsonrpc, rule)
759767
}
760768

crates/openshell-supervisor-network/src/l7/jsonrpc.rs

Lines changed: 22 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33

44
//! JSON-RPC 2.0 over HTTP L7 inspection.
55
6-
use miette::Result;
7-
use sha2::{Digest, Sha256};
8-
use std::collections::BTreeMap;
96
use std::collections::HashMap;
7+
8+
use miette::Result;
109
use tokio::io::{AsyncRead, AsyncWrite};
1110
use tower_mcp_types::protocol::{
1211
JSONRPC_VERSION, JsonRpcNotification, JsonRpcRequest, McpNotification, McpRequest,
@@ -123,28 +122,6 @@ impl JsonRpcRequestInfo {
123122
error: None,
124123
}
125124
}
126-
127-
/// Logs store only a digest of params. For batches, hash the per-call
128-
/// canonical maps so denied-call logging cannot leak raw argument values.
129-
pub(crate) fn params_sha256(&self) -> Option<String> {
130-
if self.is_batch {
131-
if self.calls.is_empty() || self.calls.iter().all(|call| call.params.is_empty()) {
132-
return None;
133-
}
134-
let canonical_params = self
135-
.calls
136-
.iter()
137-
.map(|call| canonical_params_map(&call.params))
138-
.collect::<Vec<_>>();
139-
return Some(sha256_json(&canonical_params));
140-
}
141-
142-
let call = self.calls.first()?;
143-
if call.params.is_empty() {
144-
return None;
145-
}
146-
Some(sha256_json(&canonical_params_map(&call.params)))
147-
}
148125
}
149126

150127
pub(crate) fn jsonrpc_receive_stream_request(request: &L7Request) -> bool {
@@ -437,18 +414,6 @@ fn validate_mcp_tool_name(name: &str) -> std::result::Result<(), String> {
437414
Ok(())
438415
}
439416

440-
fn canonical_params_map(params: &HashMap<String, String>) -> BTreeMap<String, String> {
441-
params
442-
.iter()
443-
.map(|(key, value)| (key.clone(), value.clone()))
444-
.collect()
445-
}
446-
447-
fn sha256_json(value: &impl serde::Serialize) -> String {
448-
let encoded = serde_json::to_vec(value).expect("canonical JSON-RPC params should serialize");
449-
hex::encode(Sha256::digest(&encoded))
450-
}
451-
452417
fn flatten_json_value(
453418
prefix: &str,
454419
path_segments: usize,
@@ -511,10 +476,10 @@ fn insert_flattened_param(
511476
out.insert(key.to_string(), param);
512477
Ok(())
513478
}
514-
515479
#[cfg(test)]
516480
mod tests {
517481
use super::*;
482+
use std::collections::HashMap;
518483

519484
#[test]
520485
fn parses_method_from_request_body() {
@@ -539,7 +504,6 @@ mod tests {
539504
assert!(!info.is_batch);
540505
assert!(info.has_response);
541506
assert!(info.error.is_none());
542-
assert!(info.params_sha256().is_none());
543507
}
544508

545509
#[test]
@@ -554,9 +518,14 @@ mod tests {
554518
}
555519

556520
#[test]
557-
fn flattens_object_params_for_policy_matching() {
521+
fn ignores_params_when_extracting_method() {
558522
let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/main"}}}"#;
559523
let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc);
524+
assert!(info.error.is_none());
525+
assert_eq!(
526+
info.calls.first().map(|call| call.method.as_str()),
527+
Some("tools/call")
528+
);
560529
let params = &info.calls.first().expect("single request call").params;
561530
assert_eq!(
562531
params.get("name").map(String::as_str),
@@ -659,6 +628,19 @@ mod tests {
659628
);
660629
}
661630

631+
#[test]
632+
fn accepts_any_valid_jsonrpc_params_shape() {
633+
let body =
634+
br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":["ignored",{"nested":true}]}"#;
635+
let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc);
636+
637+
assert!(info.error.is_none());
638+
assert_eq!(
639+
info.calls.first().map(|call| call.method.as_str()),
640+
Some("tools/call")
641+
);
642+
}
643+
662644
#[test]
663645
fn recognizes_streamable_http_get_receive_streams() {
664646
let request = L7Request {
@@ -675,7 +657,6 @@ mod tests {
675657
assert!(info.receive_stream);
676658
assert!(info.error.is_none());
677659
assert!(info.calls.is_empty());
678-
assert!(info.params_sha256().is_none());
679660
}
680661

681662
#[test]
@@ -762,10 +743,6 @@ mod tests {
762743
assert_eq!(info.calls.len(), 2);
763744
assert_eq!(info.calls[0].method, "tools/list");
764745
assert_eq!(info.calls[1].method, "tools/call");
765-
assert_eq!(
766-
info.calls[1].params.get("name").map(String::as_str),
767-
Some("read_status")
768-
);
769746
}
770747

771748
#[test]
@@ -831,52 +808,4 @@ mod tests {
831808
Some("batch item invalid: JSON-RPC message includes both method and result/error")
832809
);
833810
}
834-
835-
#[test]
836-
fn params_digest_is_canonical_and_redacted() {
837-
let first = parse_jsonrpc_body(
838-
br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/main"}}}"#,
839-
JsonRpcInspectionMode::JsonRpc,
840-
);
841-
let reordered = parse_jsonrpc_body(
842-
br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments":{"scope":"workspace/main"},"name":"submit_report"}}"#,
843-
JsonRpcInspectionMode::JsonRpc,
844-
);
845-
let changed = parse_jsonrpc_body(
846-
br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/other"}}}"#,
847-
JsonRpcInspectionMode::JsonRpc,
848-
);
849-
850-
let digest = first.params_sha256().expect("params digest");
851-
assert_eq!(Some(digest.as_str()), reordered.params_sha256().as_deref());
852-
assert_ne!(Some(digest.as_str()), changed.params_sha256().as_deref());
853-
assert_eq!(digest.len(), 64);
854-
assert!(digest.chars().all(|c| c.is_ascii_hexdigit()));
855-
assert!(!digest.contains("workspace/main"));
856-
assert!(!digest.contains("submit_report"));
857-
}
858-
859-
#[test]
860-
fn batch_params_digest_covers_call_params_without_raw_values() {
861-
let batch = parse_jsonrpc_body(
862-
br#"[
863-
{"jsonrpc":"2.0","id":1,"method":"tools/list"},
864-
{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"blocked_action"}}
865-
]"#,
866-
JsonRpcInspectionMode::JsonRpc,
867-
);
868-
let empty_batch = parse_jsonrpc_body(
869-
br#"[
870-
{"jsonrpc":"2.0","id":1,"method":"tools/list"},
871-
{"jsonrpc":"2.0","id":2,"method":"initialize"}
872-
]"#,
873-
JsonRpcInspectionMode::JsonRpc,
874-
);
875-
876-
let digest = batch.params_sha256().expect("batch params digest");
877-
assert_eq!(digest.len(), 64);
878-
assert!(digest.chars().all(|c| c.is_ascii_hexdigit()));
879-
assert!(!digest.contains("blocked_action"));
880-
assert!(empty_batch.params_sha256().is_none());
881-
}
882811
}

0 commit comments

Comments
 (0)