Skip to content

Commit b92cd00

Browse files
committed
fix(l7): harden json-rpc frame handling
Signed-off-by: Kris Hicks <khicks@nvidia.com>
1 parent c68172e commit b92cd00

3 files changed

Lines changed: 95 additions & 3 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ request_denied_for_endpoint(request, endpoint) if {
278278
# --- L7 deny rule matching: JSON-RPC method + params ---
279279

280280
request_denied_for_endpoint(request, endpoint) if {
281+
endpoint.protocol == "json-rpc"
282+
request.method == "POST"
281283
some deny_rule
282284
deny_rule := endpoint.deny_rules[_]
283285
deny_rule.rpc_method
@@ -445,6 +447,8 @@ request_allowed_for_endpoint(request, endpoint) if {
445447
# --- L7 rule matching: JSON-RPC method ---
446448

447449
request_allowed_for_endpoint(request, endpoint) if {
450+
endpoint.protocol == "json-rpc"
451+
request.method == "POST"
448452
some rule
449453
rule := endpoint.rules[_]
450454
rule.allow.rpc_method
@@ -460,6 +464,8 @@ request_allowed_for_endpoint(request, endpoint) if {
460464
request.method == "GET"
461465
is_object(request.jsonrpc)
462466
jsonrpc_no_parse_error(request.jsonrpc)
467+
object.get(request.jsonrpc, "method", null) == null
468+
not object.get(request.jsonrpc, "has_response", false)
463469
}
464470

465471
# --- L7 rule matching: GraphQL operation ---

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,21 @@ fn parse_jsonrpc_message(
177177
return Err(format!("unsupported JSON-RPC version '{version}'"));
178178
}
179179

180-
if value.get("method").is_some() {
181-
return parse_jsonrpc_call(value).map(JsonRpcMessageInfo::Call);
180+
let has_method = value.get("method").is_some();
181+
let has_response_payload = jsonrpc_response_payload_present(value);
182+
if has_method && has_response_payload {
183+
return Err("JSON-RPC message includes both method and result/error".to_string());
182184
}
183185

184-
if jsonrpc_response_payload_present(value) {
186+
if has_response_payload {
185187
parse_jsonrpc_response(value)?;
186188
return Ok(JsonRpcMessageInfo::Response);
187189
}
188190

191+
if has_method {
192+
return parse_jsonrpc_call(value).map(JsonRpcMessageInfo::Call);
193+
}
194+
189195
Err("missing or non-string 'method' field".to_string())
190196
}
191197

@@ -484,6 +490,41 @@ mod tests {
484490
);
485491
}
486492

493+
#[test]
494+
fn rejects_message_with_method_and_result_or_error() {
495+
let result_body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","result":{}}"#;
496+
let result_info = parse_jsonrpc_body(result_body);
497+
assert!(result_info.calls.is_empty());
498+
assert_eq!(
499+
result_info.error.as_deref(),
500+
Some("JSON-RPC message includes both method and result/error")
501+
);
502+
503+
let error_body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","error":{"code":-32603,"message":"failed"}}"#;
504+
let error_info = parse_jsonrpc_body(error_body);
505+
assert!(error_info.calls.is_empty());
506+
assert_eq!(
507+
error_info.error.as_deref(),
508+
Some("JSON-RPC message includes both method and result/error")
509+
);
510+
}
511+
512+
#[test]
513+
fn rejects_batch_item_with_method_and_result() {
514+
let body = br#"[
515+
{"jsonrpc":"2.0","id":1,"method":"tools/list"},
516+
{"jsonrpc":"2.0","id":2,"method":"initialize","result":{}}
517+
]"#;
518+
let info = parse_jsonrpc_body(body);
519+
520+
assert!(info.calls.is_empty());
521+
assert!(info.is_batch);
522+
assert_eq!(
523+
info.error.as_deref(),
524+
Some("batch item invalid: JSON-RPC message includes both method and result/error")
525+
);
526+
}
527+
487528
#[test]
488529
fn params_digest_is_canonical_and_redacted() {
489530
let first = parse_jsonrpc_body(

‎crates/openshell-supervisor-network/src/opa.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,6 +2852,51 @@ network_policies:
28522852
assert!(!eval_l7(&engine, &progress_input));
28532853
}
28542854

2855+
#[test]
2856+
fn l7_jsonrpc_method_rules_require_post() {
2857+
let data = r#"
2858+
network_policies:
2859+
jsonrpc_post:
2860+
name: jsonrpc_post
2861+
endpoints:
2862+
- host: mcp.post.test
2863+
port: 8000
2864+
path: /mcp
2865+
protocol: json-rpc
2866+
enforcement: enforce
2867+
rules:
2868+
- allow:
2869+
rpc_method: initialize
2870+
deny_rules:
2871+
- rpc_method: tools/delete
2872+
binaries:
2873+
- { path: /usr/bin/curl }
2874+
"#;
2875+
let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml");
2876+
2877+
let mut post_input = l7_jsonrpc_input_with_params(
2878+
"mcp.post.test",
2879+
8000,
2880+
"/mcp",
2881+
"initialize",
2882+
serde_json::json!({}),
2883+
);
2884+
assert!(eval_l7(&engine, &post_input));
2885+
2886+
post_input["request"]["method"] = serde_json::json!("PUT");
2887+
assert!(!eval_l7(&engine, &post_input));
2888+
2889+
let mut get_with_method = l7_jsonrpc_input_with_params(
2890+
"mcp.post.test",
2891+
8000,
2892+
"/mcp",
2893+
"initialize",
2894+
serde_json::json!({}),
2895+
);
2896+
get_with_method["request"]["method"] = serde_json::json!("GET");
2897+
assert!(!eval_l7(&engine, &get_with_method));
2898+
}
2899+
28552900
#[test]
28562901
fn l7_jsonrpc_params_rules_filter_tools_call() {
28572902
let data = r#"

0 commit comments

Comments
 (0)