From 485f21a98b8be506af8269f2c2289ebeef625fdf Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:07:50 -0700 Subject: [PATCH 01/12] feat(policy): add MCP policy profile Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- Cargo.lock | 13 + Cargo.toml | 3 +- architecture/sandbox.md | 13 +- crates/openshell-policy/src/lib.rs | 479 +++++++++++++----- crates/openshell-policy/src/merge.rs | 25 + .../openshell-supervisor-network/Cargo.toml | 1 + .../data/sandbox-policy.rego | 13 +- .../src/l7/jsonrpc.rs | 103 +++- .../src/l7/mod.rs | 296 ++++++----- .../src/l7/relay.rs | 108 +++- .../openshell-supervisor-network/src/opa.rs | 260 ++++++++++ .../openshell-supervisor-network/src/proxy.rs | 13 +- docs/reference/policy-schema.mdx | 80 ++- docs/sandboxes/policies.mdx | 37 +- e2e/mcp-conformance/README.md | 16 +- .../client-through-openshell.sh | 12 +- e2e/mcp-conformance/expected-failures.yml | 6 +- e2e/mcp-conformance/policy-template.yaml | 4 +- e2e/with-docker-gateway.sh | 62 ++- 19 files changed, 1194 insertions(+), 350 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f693acd66..726f32ca2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3782,6 +3782,7 @@ dependencies = [ "tokio", "tokio-rustls", "tokio-tungstenite 0.26.2", + "tower-mcp-types", "tracing", "uuid", "webpki-roots 1.0.7", @@ -6455,6 +6456,18 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "121c2a6cda46980bb0fcd1647ffaf6cd3fc79a013de288782836f6df9c48780e" +[[package]] +name = "tower-mcp-types" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6511f1f32c7cb7fd4525edc0eb4dcf307db8f7eceb2833ab24a37b4cc10cda61" +dependencies = [ + "base64 0.22.1", + "serde", + "serde_json", + "thiserror 2.0.18", +] + [[package]] name = "tower-service" version = "0.3.3" diff --git a/Cargo.toml b/Cargo.toml index 86025646a..f450cd5c8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ members = ["crates/*"] [workspace.package] version = "0.0.0" edition = "2024" -rust-version = "1.88" +rust-version = "1.90" license = "Apache-2.0" repository = "https://github.com/NVIDIA/OpenShell" @@ -73,6 +73,7 @@ serde_json = "1" serde_yml = "0.0.12" toml = "0.8" apollo-parser = "0.8.5" +tower-mcp-types = "0.12.0" # HTTP client reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls-native-roots"] } diff --git a/architecture/sandbox.md b/architecture/sandbox.md index eb78eb6ad..b3fda501f 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -51,12 +51,13 @@ identifies the calling binary, checks trust-on-first-use binary identity, reject unsafe internal destinations, and evaluates the active policy. For inspected HTTP traffic, the proxy can enforce REST method/path rules, WebSocket upgrade and text-message rules, GraphQL operation rules, and -JSON-RPC method and params rules on sandbox-to-server request bodies. JSON-RPC -request inspection buffers up to the endpoint `json_rpc.max_body_bytes` limit. -Literal dotted keys in JSON-RPC params are rejected before policy evaluation so -they cannot be confused with flattened nested selector paths. -JSON-RPC responses and server-to-client MCP messages on response or SSE streams -are relayed but are not currently parsed for policy enforcement. +MCP or generic JSON-RPC method and params rules on sandbox-to-server request +bodies. MCP and JSON-RPC inspection buffers up to the endpoint +`mcp.max_body_bytes` or `json_rpc.max_body_bytes` limit. Literal dotted keys in +JSON-RPC params are rejected before policy evaluation so they cannot be confused +with flattened nested selector paths. JSON-RPC responses and server-to-client +MCP messages on response or SSE streams are relayed but are not currently +parsed for policy enforcement. `https://inference.local` is special. It bypasses OPA network policy and is handled by the inference interception path: diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index d110dcfc6..4bf010716 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -108,8 +108,8 @@ struct NetworkEndpointDef { enforcement: String, #[serde(default, skip_serializing_if = "String::is_empty")] access: String, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - rules: Vec, + #[serde(default, skip_serializing_if = "RulesDef::is_empty")] + rules: RulesDef, #[serde(default, skip_serializing_if = "Vec::is_empty")] allowed_ips: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -137,6 +137,8 @@ struct NetworkEndpointDef { graphql_max_body_bytes: u32, #[serde(default, skip_serializing_if = "Option::is_none")] json_rpc: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + mcp: Option, } // Signature dictated by serde's `skip_serializing_if`, which requires `&T`. @@ -162,6 +164,17 @@ fn json_rpc_config_from_proto(max_body_bytes: u32) -> Option { (max_body_bytes > 0).then_some(JsonRpcConfigDef { max_body_bytes }) } +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct McpConfigDef { + #[serde(default, skip_serializing_if = "is_zero_u32")] + max_body_bytes: u32, +} + +fn mcp_config_from_proto(max_body_bytes: u32) -> Option { + (max_body_bytes > 0).then_some(McpConfigDef { max_body_bytes }) +} + #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct GraphqlOperationDef { @@ -179,6 +192,51 @@ struct L7RuleDef { allow: L7AllowDef, } +#[derive(Debug, Serialize, Deserialize)] +#[serde(untagged)] +enum RulesDef { + Legacy(Vec), + Grouped(L7RuleGroupsDef), +} + +impl Default for RulesDef { + fn default() -> Self { + Self::Legacy(Vec::new()) + } +} + +impl RulesDef { + fn is_empty(&self) -> bool { + match self { + Self::Legacy(rules) => rules.is_empty(), + Self::Grouped(groups) => groups.allow.is_empty() && groups.deny.is_empty(), + } + } + + fn into_parts(self) -> (Vec, Vec) { + match self { + Self::Legacy(rules) => (rules, Vec::new()), + Self::Grouped(groups) => ( + groups + .allow + .into_iter() + .map(|allow| L7RuleDef { allow }) + .collect(), + groups.deny, + ), + } + } +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct L7RuleGroupsDef { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + allow: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + deny: Vec, +} + #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct L7AllowDef { @@ -196,6 +254,12 @@ struct L7AllowDef { operation_name: String, #[serde(default, skip_serializing_if = "Vec::is_empty")] fields: Vec, + #[serde(default, skip_serializing_if = "String::is_empty")] + rpc_method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + mcp_method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, } @@ -231,6 +295,12 @@ struct L7DenyRuleDef { operation_name: String, #[serde(default, skip_serializing_if = "Vec::is_empty")] fields: Vec, + #[serde(default, skip_serializing_if = "String::is_empty")] + rpc_method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + mcp_method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, } @@ -267,6 +337,165 @@ fn matcher_proto_to_def(matcher: L7QueryMatcher) -> QueryMatcherDef { } } +fn matcher_glob(glob: String) -> QueryMatcherDef { + QueryMatcherDef::Glob(glob) +} + +fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { + if mcp_method.is_empty() { + rpc_method + } else { + mcp_method + } +} + +fn params_with_tool( + mut params: BTreeMap, + tool: String, +) -> BTreeMap { + if !tool.is_empty() { + params + .entry("name".to_string()) + .or_insert_with(|| matcher_glob(tool)); + } + params +} + +fn allow_def_to_proto(allow: L7AllowDef) -> L7Allow { + L7Allow { + method: allow.method, + path: allow.path, + command: allow.command, + operation_type: allow.operation_type, + operation_name: allow.operation_name, + fields: allow.fields, + rpc_method: method_from_aliases(allow.rpc_method, allow.mcp_method), + query: allow + .query + .into_iter() + .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) + .collect(), + params: params_with_tool(allow.params, allow.tool) + .into_iter() + .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) + .collect(), + } +} + +fn deny_def_to_proto(deny: L7DenyRuleDef) -> L7DenyRule { + L7DenyRule { + method: deny.method, + path: deny.path, + command: deny.command, + operation_type: deny.operation_type, + operation_name: deny.operation_name, + fields: deny.fields, + rpc_method: method_from_aliases(deny.rpc_method, deny.mcp_method), + query: deny + .query + .into_iter() + .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) + .collect(), + params: params_with_tool(deny.params, deny.tool) + .into_iter() + .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) + .collect(), + } +} + +fn json_rpc_max_body_bytes(json_rpc: &Option, mcp: &Option) -> u32 { + mcp.as_ref().map_or_else( + || json_rpc.as_ref().map_or(0, |config| config.max_body_bytes), + |config| config.max_body_bytes, + ) +} + +fn is_mcp_protocol(protocol: &str) -> bool { + protocol.eq_ignore_ascii_case("mcp") +} + +fn split_tool_param( + protocol: &str, + params: BTreeMap, +) -> (String, BTreeMap) { + if !is_mcp_protocol(protocol) { + return (String::new(), params); + } + + let mut params = params; + let tool = match params.remove("name") { + Some(QueryMatcherDef::Glob(glob)) => glob, + Some(other) => { + params.insert("name".to_string(), other); + String::new() + } + None => String::new(), + }; + (tool, params) +} + +fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { + let params: BTreeMap = allow + .params + .into_iter() + .map(|(key, matcher)| (key, matcher_proto_to_def(matcher))) + .collect(); + let (tool, params) = split_tool_param(protocol, params); + let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { + (String::new(), allow.rpc_method) + } else { + (allow.rpc_method, String::new()) + }; + L7AllowDef { + method: allow.method, + path: allow.path, + command: allow.command, + query: allow + .query + .into_iter() + .map(|(key, matcher)| (key, matcher_proto_to_def(matcher))) + .collect(), + operation_type: allow.operation_type, + operation_name: allow.operation_name, + fields: allow.fields, + rpc_method, + mcp_method, + tool, + params, + } +} + +fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { + let params: BTreeMap = deny + .params + .iter() + .map(|(key, matcher)| (key.clone(), matcher_proto_to_def(matcher.clone()))) + .collect(); + let (tool, params) = split_tool_param(protocol, params); + let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { + (String::new(), deny.rpc_method.clone()) + } else { + (deny.rpc_method.clone(), String::new()) + }; + L7DenyRuleDef { + method: deny.method.clone(), + path: deny.path.clone(), + command: deny.command.clone(), + query: deny + .query + .iter() + .map(|(key, matcher)| (key.clone(), matcher_proto_to_def(matcher.clone()))) + .collect(), + operation_type: deny.operation_type.clone(), + operation_name: deny.operation_name.clone(), + fields: deny.fields.clone(), + rpc_method, + mcp_method, + tool, + params, + } +} + fn to_proto(raw: PolicyFile) -> SandboxPolicy { let network_policies = raw .network_policies @@ -282,6 +511,9 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { .endpoints .into_iter() .map(|e| { + let (allow_rules, grouped_deny_rules) = e.rules.into_parts(); + let mut deny_rules = grouped_deny_rules; + deny_rules.extend(e.deny_rules); // Normalize port/ports: ports takes precedence, else // single port is promoted to ports array. let normalized_ports: Vec = if !e.ports.is_empty() { @@ -300,59 +532,14 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { tls: e.tls, enforcement: e.enforcement, access: e.access, - rules: e - .rules + rules: allow_rules .into_iter() .map(|r| L7Rule { - allow: Some(L7Allow { - method: r.allow.method, - path: r.allow.path, - command: r.allow.command, - operation_type: r.allow.operation_type, - operation_name: r.allow.operation_name, - fields: r.allow.fields, - query: r - .allow - .query - .into_iter() - .map(|(key, matcher)| { - (key, matcher_def_to_proto(matcher)) - }) - .collect(), - params: r - .allow - .params - .into_iter() - .map(|(key, matcher)| { - (key, matcher_def_to_proto(matcher)) - }) - .collect(), - }), + allow: Some(allow_def_to_proto(r.allow)), }) .collect(), allowed_ips: e.allowed_ips, - deny_rules: e - .deny_rules - .into_iter() - .map(|d| L7DenyRule { - method: d.method, - path: d.path, - command: d.command, - operation_type: d.operation_type, - operation_name: d.operation_name, - fields: d.fields, - query: d - .query - .into_iter() - .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) - .collect(), - params: d - .params - .into_iter() - .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) - .collect(), - }) - .collect(), + deny_rules: deny_rules.into_iter().map(deny_def_to_proto).collect(), allow_encoded_slash: e.allow_encoded_slash, websocket_credential_rewrite: e.websocket_credential_rewrite, request_body_credential_rewrite: e.request_body_credential_rewrite, @@ -375,10 +562,7 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { }) .collect(), graphql_max_body_bytes: e.graphql_max_body_bytes, - json_rpc_max_body_bytes: e - .json_rpc - .as_ref() - .map_or(0, |config| config.max_body_bytes), + json_rpc_max_body_bytes: json_rpc_max_body_bytes(&e.json_rpc, &e.mcp), } }) .collect(), @@ -458,73 +642,57 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { } else { (clamp(e.ports.first().copied().unwrap_or(e.port)), vec![]) }; + let protocol = e.protocol.clone(); + let allow_defs: Vec = e + .rules + .iter() + .map(|r| { + allow_proto_to_def(&protocol, r.allow.clone().unwrap_or_default()) + }) + .collect(); + let deny_defs: Vec = e + .deny_rules + .iter() + .map(|d| deny_proto_to_def(&protocol, d)) + .collect(); + let (rules, deny_rules) = if is_mcp_protocol(&protocol) + && (!allow_defs.is_empty() || !deny_defs.is_empty()) + { + ( + RulesDef::Grouped(L7RuleGroupsDef { + allow: allow_defs, + deny: deny_defs, + }), + Vec::new(), + ) + } else { + ( + RulesDef::Legacy( + allow_defs + .into_iter() + .map(|allow| L7RuleDef { allow }) + .collect(), + ), + deny_defs, + ) + }; + let (json_rpc, mcp) = if is_mcp_protocol(&protocol) { + (None, mcp_config_from_proto(e.json_rpc_max_body_bytes)) + } else { + (json_rpc_config_from_proto(e.json_rpc_max_body_bytes), None) + }; NetworkEndpointDef { host: e.host.clone(), path: e.path.clone(), port, ports, - protocol: e.protocol.clone(), + protocol, tls: e.tls.clone(), enforcement: e.enforcement.clone(), access: e.access.clone(), - rules: e - .rules - .iter() - .map(|r| { - let a = r.allow.clone().unwrap_or_default(); - L7RuleDef { - allow: L7AllowDef { - method: a.method, - path: a.path, - command: a.command, - operation_type: a.operation_type, - operation_name: a.operation_name, - fields: a.fields, - query: a - .query - .into_iter() - .map(|(key, matcher)| { - (key, matcher_proto_to_def(matcher)) - }) - .collect(), - params: a - .params - .into_iter() - .map(|(key, matcher)| { - (key, matcher_proto_to_def(matcher)) - }) - .collect(), - }, - } - }) - .collect(), + rules, allowed_ips: e.allowed_ips.clone(), - deny_rules: e - .deny_rules - .iter() - .map(|d| L7DenyRuleDef { - method: d.method.clone(), - path: d.path.clone(), - command: d.command.clone(), - operation_type: d.operation_type.clone(), - operation_name: d.operation_name.clone(), - fields: d.fields.clone(), - query: d - .query - .iter() - .map(|(key, matcher)| { - (key.clone(), matcher_proto_to_def(matcher.clone())) - }) - .collect(), - params: d - .params - .iter() - .map(|(key, matcher)| { - (key.clone(), matcher_proto_to_def(matcher.clone())) - }) - .collect(), - }) - .collect(), + deny_rules, allow_encoded_slash: e.allow_encoded_slash, websocket_credential_rewrite: e.websocket_credential_rewrite, request_body_credential_rewrite: e.request_body_credential_rewrite, @@ -544,7 +712,8 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { }) .collect(), graphql_max_body_bytes: e.graphql_max_body_bytes, - json_rpc: json_rpc_config_from_proto(e.json_rpc_max_body_bytes), + json_rpc, + mcp, } }) .collect(), @@ -1761,6 +1930,90 @@ network_policies: assert_eq!(ep.json_rpc_max_body_bytes, 131_072); } + #[test] + fn parse_grouped_mcp_rules_to_jsonrpc_runtime_fields() { + let yaml = r" +version: 1 +network_policies: + mcp: + name: mcp + endpoints: + - host: mcp.example.com + port: 443 + path: /mcp + protocol: mcp + enforcement: enforce + mcp: + max_body_bytes: 131072 + rules: + deny: + - mcp_method: tools/call + tool: send_email + allow: + - mcp_method: initialize + - mcp_method: tools/list + - mcp_method: tools/call + tool: search_web + params: + arguments.repository: NVIDIA/OpenShell + binaries: + - path: /usr/bin/curl +"; + let proto = parse_sandbox_policy(yaml).expect("parse failed"); + let ep = &proto.network_policies["mcp"].endpoints[0]; + + assert_eq!(ep.protocol, "mcp"); + assert_eq!(ep.json_rpc_max_body_bytes, 131_072); + assert_eq!(ep.rules.len(), 3); + assert_eq!(ep.rules[2].allow.as_ref().unwrap().rpc_method, "tools/call"); + assert_eq!( + ep.rules[2].allow.as_ref().unwrap().params["name"].glob, + "search_web" + ); + assert_eq!( + ep.rules[2].allow.as_ref().unwrap().params["arguments.repository"].glob, + "NVIDIA/OpenShell" + ); + assert_eq!(ep.deny_rules.len(), 1); + assert_eq!(ep.deny_rules[0].rpc_method, "tools/call"); + assert_eq!(ep.deny_rules[0].params["name"].glob, "send_email"); + } + + #[test] + fn round_trip_mcp_policy_serializes_mcp_expression() { + let yaml = r" +version: 1 +network_policies: + mcp: + name: mcp + endpoints: + - host: mcp.example.com + port: 443 + protocol: mcp + mcp: + max_body_bytes: 131072 + rules: + allow: + - mcp_method: tools/call + tool: search_web + deny: + - mcp_method: tools/call + tool: send_email + binaries: + - path: /usr/bin/curl +"; + let proto1 = parse_sandbox_policy(yaml).expect("parse failed"); + let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed"); + let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); + + assert!(yaml_out.contains("protocol: mcp")); + assert!(yaml_out.contains("mcp_method: tools/call")); + assert!(yaml_out.contains("tool: search_web")); + assert!(yaml_out.contains("tool: send_email")); + assert!(yaml_out.contains("mcp:")); + assert_eq!(proto1, proto2); + } + #[test] fn parse_rejects_unsupported_json_rpc_config_fields() { let yaml = r" diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index ef6566a1f..17abd3ac7 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -726,6 +726,31 @@ fn expand_existing_access( } fn expand_access_preset(protocol: &str, access: &str) -> Option> { + if matches!(protocol, "json-rpc" | "mcp") { + let rpc_methods = match access { + "read-only" | "read-write" | "full" => vec!["*"], + _ => return None, + }; + return Some( + rpc_methods + .into_iter() + .map(|rpc_method| L7Rule { + allow: Some(L7Allow { + method: String::new(), + path: String::new(), + command: String::new(), + query: HashMap::default(), + operation_type: String::new(), + operation_name: String::new(), + fields: Vec::new(), + rpc_method: rpc_method.to_string(), + params: HashMap::default(), + }), + }) + .collect(), + ); + } + let methods = match (protocol, access) { (_, "full") => vec!["*"], ("websocket", "read-only") => vec!["GET"], diff --git a/crates/openshell-supervisor-network/Cargo.toml b/crates/openshell-supervisor-network/Cargo.toml index 71febf0af..e933c1e04 100644 --- a/crates/openshell-supervisor-network/Cargo.toml +++ b/crates/openshell-supervisor-network/Cargo.toml @@ -38,6 +38,7 @@ spiffe = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tokio-rustls = { workspace = true } +tower-mcp-types = { workspace = true } tracing = { workspace = true } uuid = { workspace = true } webpki-roots = { workspace = true } diff --git a/crates/openshell-supervisor-network/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index fd703f841..54b48388c 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -693,11 +693,15 @@ query_value_matches(value, matcher) if { # JSON-RPC method and params matching. The sandbox flattens object params into # dot-separated keys before policy evaluation, e.g. arguments.scope. jsonrpc_rule_matches(request, rule) if { - jsonrpc := object.get(request, "jsonrpc", {}) + jsonrpc := object.get(request, "jsonrpc", null) is_object(jsonrpc) - method := object.get(jsonrpc, "method", null) - method != null - glob.match(rule.method, [], method) + method := object.get(jsonrpc, "method", "") + is_string(method) + method != "" + rpc_method := object.get(rule, "rpc_method", "") + is_string(rpc_method) + rpc_method != "" + glob.match(rpc_method, [], method) jsonrpc_params_match(jsonrpc, rule) } @@ -718,6 +722,7 @@ jsonrpc_no_parse_error(jsonrpc) if { jsonrpc_params_match(jsonrpc, rule) if { is_object(jsonrpc) param_rules := object.get(rule, "params", {}) + is_object(param_rules) not jsonrpc_param_mismatch(jsonrpc, param_rules) } diff --git a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs index f9f4d91c4..eda948819 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -13,6 +13,12 @@ use crate::l7::provider::{L7Provider, L7Request}; pub const DEFAULT_MAX_BODY_BYTES: usize = 64 * 1024; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum JsonRpcInspectionMode { + JsonRpc, + Mcp, +} + pub struct JsonRpcHttpRequest { pub request: L7Request, pub info: JsonRpcRequestInfo, @@ -22,6 +28,7 @@ pub(crate) async fn parse_jsonrpc_http_request Result> { let provider = crate::l7::rest::RestProvider::with_options(canonicalize_options); let Some(mut request) = provider.parse_request(client).await? else { @@ -35,7 +42,7 @@ pub(crate) async fn parse_jsonrpc_http_request, + pub tool: Option, } impl JsonRpcRequestInfo { @@ -120,6 +128,17 @@ fn request_accepts_sse(request: &L7Request) -> bool { /// Returns an info struct with `method` set on success, or `error` set if the /// body is not valid JSON-RPC 2.0. pub fn parse_jsonrpc_body(body: &[u8]) -> JsonRpcRequestInfo { + parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::JsonRpc) +} + +pub fn parse_mcp_body(body: &[u8]) -> JsonRpcRequestInfo { + parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::Mcp) +} + +pub fn parse_jsonrpc_body_with_mode( + body: &[u8], + inspection_mode: JsonRpcInspectionMode, +) -> JsonRpcRequestInfo { let Ok(value) = serde_json::from_slice::(body) else { return JsonRpcRequestInfo { calls: Vec::new(), @@ -143,7 +162,7 @@ pub fn parse_jsonrpc_body(body: &[u8]) -> JsonRpcRequestInfo { let mut calls = Vec::new(); let mut has_response = false; for item in &items { - match parse_jsonrpc_message(item) { + match parse_jsonrpc_message(item, inspection_mode) { Ok(JsonRpcMessageInfo::Call(call)) => calls.push(call), Ok(JsonRpcMessageInfo::Response) => has_response = true, Err(error) => { @@ -166,7 +185,7 @@ pub fn parse_jsonrpc_body(body: &[u8]) -> JsonRpcRequestInfo { }; } - match parse_jsonrpc_message(&value) { + match parse_jsonrpc_message(&value, inspection_mode) { Ok(JsonRpcMessageInfo::Call(call)) => JsonRpcRequestInfo { calls: vec![call], is_batch: false, @@ -198,6 +217,7 @@ enum JsonRpcMessageInfo { fn parse_jsonrpc_message( value: &serde_json::Value, + inspection_mode: JsonRpcInspectionMode, ) -> std::result::Result { let version = value .get("jsonrpc") @@ -219,13 +239,16 @@ fn parse_jsonrpc_message( } if has_method { - return parse_jsonrpc_call(value).map(JsonRpcMessageInfo::Call); + return parse_jsonrpc_call(value, inspection_mode).map(JsonRpcMessageInfo::Call); } Err("missing or non-string 'method' field".to_string()) } -fn parse_jsonrpc_call(value: &serde_json::Value) -> std::result::Result { +fn parse_jsonrpc_call( + value: &serde_json::Value, + inspection_mode: JsonRpcInspectionMode, +) -> std::result::Result { let method = value .get("method") .and_then(|m| m.as_str()) @@ -233,9 +256,14 @@ fn parse_jsonrpc_call(value: &serde_json::Value) -> std::result::Result std::result::Result<(), Ok(()) } +fn validate_mcp_call(value: &serde_json::Value) -> std::result::Result<(), String> { + if value.get("id").is_some() { + let request: tower_mcp_types::protocol::JsonRpcRequest = + serde_json::from_value(value.clone()) + .map_err(|error| format!("invalid MCP request: {error}"))?; + request + .validate() + .map_err(|error| format!("invalid MCP request: {error:?}"))?; + tower_mcp_types::protocol::McpRequest::from_jsonrpc(&request) + .map_err(|error| format!("invalid MCP request params: {error}"))?; + } else { + let notification: tower_mcp_types::protocol::JsonRpcNotification = + serde_json::from_value(value.clone()) + .map_err(|error| format!("invalid MCP notification: {error}"))?; + tower_mcp_types::protocol::McpNotification::from_jsonrpc(¬ification) + .map_err(|error| format!("invalid MCP notification params: {error}"))?; + } + Ok(()) +} + fn flatten_jsonrpc_params( value: &serde_json::Value, ) -> std::result::Result, String> { @@ -390,6 +438,51 @@ mod tests { ); } + #[test] + fn mcp_mode_validates_known_methods_and_extracts_tool() { + let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"search_web","arguments":{"query":"openshell"}}}"#; + let info = parse_mcp_body(body); + + assert!(info.error.is_none(), "expected valid MCP call: {info:?}"); + let call = info.calls.first().expect("single MCP call"); + assert_eq!(call.method, "tools/call"); + assert_eq!(call.tool.as_deref(), Some("search_web")); + assert_eq!( + call.params.get("arguments.query").map(String::as_str), + Some("openshell") + ); + } + + #[test] + fn mcp_mode_rejects_invalid_known_method_params() { + let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments":{"query":"openshell"}}}"#; + let info = parse_mcp_body(body); + + assert!(info.calls.is_empty()); + assert!( + info.error + .as_deref() + .is_some_and(|error| error.contains("invalid MCP request params")), + "expected MCP params validation error, got {info:?}" + ); + } + + #[test] + fn mcp_mode_allows_unknown_extension_methods() { + let body = + br#"{"jsonrpc":"2.0","id":1,"method":"vendor/extension","params":{"name":"custom"}}"#; + let info = parse_mcp_body(body); + + assert!( + info.error.is_none(), + "extension method should remain addressable" + ); + assert_eq!( + info.calls.first().map(|call| call.method.as_str()), + Some("vendor/extension") + ); + } + #[test] fn rejects_literal_dotted_param_keys() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments.scope":"workspace/other","arguments":{"scope":"workspace/main"}}}"#; diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 4aae9d075..fd47a4de8 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -28,6 +28,7 @@ pub enum L7Protocol { Graphql, Sql, JsonRpc, + Mcp, } impl L7Protocol { @@ -38,9 +39,14 @@ impl L7Protocol { "graphql" => Some(Self::Graphql), "sql" => Some(Self::Sql), "json-rpc" => Some(Self::JsonRpc), + "mcp" => Some(Self::Mcp), _ => None, } } + + pub fn is_jsonrpc_family(self) -> bool { + matches!(self, Self::JsonRpc | Self::Mcp) + } } /// TLS handling mode for proxy connections. @@ -483,96 +489,146 @@ fn validate_graphql_rule( validate_graphql_fields(errors, warnings, loc, rule.get("fields")); } -fn validate_l7_matcher_map( +fn validate_matcher_map( errors: &mut Vec, warnings: &mut Vec, loc: &str, - value: &serde_json::Value, - label: &str, + value: Option<&serde_json::Value>, ) { - let Some(matchers) = value.as_object() else { - errors.push(format!("{loc}: expected map of {label} matchers")); + let Some(value) = value.filter(|v| !v.is_null()) else { + return; + }; + let Some(obj) = value.as_object() else { + errors.push(format!("{loc}: expected map of matchers")); return; }; - for (param, matcher) in matchers { - validate_l7_matcher(errors, warnings, &format!("{loc}.{param}"), matcher); + for (key, matcher) in obj { + if let Some(glob_str) = matcher.as_str() { + if let Some(warning) = check_glob_syntax(glob_str) { + warnings.push(format!("{loc}.{key}: {warning}")); + } + continue; + } + + let Some(matcher_obj) = matcher.as_object() else { + errors.push(format!( + "{loc}.{key}: expected string glob or object with `any`" + )); + continue; + }; + + let has_any = matcher_obj.get("any").is_some(); + let has_glob = matcher_obj.get("glob").is_some(); + let has_unknown = matcher_obj.keys().any(|k| k != "any" && k != "glob"); + if has_unknown { + errors.push(format!( + "{loc}.{key}: unknown matcher keys; only `glob` or `any` are supported" + )); + continue; + } + + if has_glob && has_any { + errors.push(format!( + "{loc}.{key}: matcher cannot specify both `glob` and `any`" + )); + continue; + } + + if !has_glob && !has_any { + errors.push(format!( + "{loc}.{key}: object matcher requires `glob` string or non-empty `any` list" + )); + continue; + } + + if has_glob { + match matcher_obj.get("glob").and_then(|v| v.as_str()) { + None => errors.push(format!("{loc}.{key}.glob: expected glob string")), + Some(glob_str) => { + if let Some(warning) = check_glob_syntax(glob_str) { + warnings.push(format!("{loc}.{key}.glob: {warning}")); + } + } + } + continue; + } + + let Some(any) = matcher_obj.get("any").and_then(|v| v.as_array()) else { + errors.push(format!("{loc}.{key}.any: expected array of glob strings")); + continue; + }; + if any.is_empty() { + errors.push(format!("{loc}.{key}.any: list must not be empty")); + continue; + } + if any.iter().any(|v| v.as_str().is_none()) { + errors.push(format!("{loc}.{key}.any: all values must be strings")); + } + for item in any.iter().filter_map(|v| v.as_str()) { + if let Some(warning) = check_glob_syntax(item) { + warnings.push(format!("{loc}.{key}.any: {warning}")); + } + } } } -fn validate_l7_matcher( +fn validate_jsonrpc_rule_fields( errors: &mut Vec, warnings: &mut Vec, loc: &str, - matcher: &serde_json::Value, + rule: &serde_json::Value, + protocol: &str, ) { - if let Some(glob_str) = matcher.as_str() { - if let Some(warning) = check_glob_syntax(glob_str) { - warnings.push(format!("{loc}: {warning}")); - } - return; + if rule.get("mcp_method").is_some() { + errors.push(format!( + "{loc}.mcp_method: use `method` for protocol mcp L7 rules" + )); } - let Some(matcher_obj) = matcher.as_object() else { - errors.push(format!("{loc}: expected string glob or object with `any`")); - return; - }; + let rpc_method = rule + .get("rpc_method") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let has_params = rule.get("params").is_some_and(|v| !v.is_null()); + let jsonrpc_family = protocol == "json-rpc" || protocol == "mcp"; - let has_any = matcher_obj.get("any").is_some(); - let has_glob = matcher_obj.get("glob").is_some(); - let has_unknown = matcher_obj.keys().any(|k| k != "any" && k != "glob"); - if has_unknown { - errors.push(format!( - "{loc}: unknown matcher keys; only `glob` or `any` are supported" - )); + if jsonrpc_family { + let method_field = if protocol == "mcp" { + "method" + } else { + "rpc_method" + }; + if rpc_method.is_empty() { + errors.push(format!( + "{loc}.{method_field}: required for {protocol} L7 rules" + )); + } else if let Some(warning) = check_glob_syntax(rpc_method) { + warnings.push(format!("{loc}.{method_field}: {warning}")); + } + validate_matcher_map( + errors, + warnings, + &format!("{loc}.params"), + rule.get("params"), + ); + if json_rule_has_non_empty_path_or_query(rule) { + errors.push(format!( + "{loc}: {protocol} L7 rules must use {method_field}/params, not path/query" + )); + } return; } - if has_glob && has_any { + if !rpc_method.is_empty() { errors.push(format!( - "{loc}: matcher cannot specify both `glob` and `any`" + "{loc}.rpc_method: JSON-RPC method matching is only valid for protocol json-rpc or mcp" )); - return; } - - if !has_glob && !has_any { + if has_params { errors.push(format!( - "{loc}: object matcher requires `glob` string or non-empty `any` list" + "{loc}.params: JSON-RPC params matching is only valid for protocol json-rpc or mcp" )); - return; - } - - if has_glob { - match matcher_obj.get("glob").and_then(|v| v.as_str()) { - None => errors.push(format!("{loc}.glob: expected glob string")), - Some(g) => { - if let Some(warning) = check_glob_syntax(g) { - warnings.push(format!("{loc}.glob: {warning}")); - } - } - } - return; - } - - let any = matcher_obj.get("any").and_then(|v| v.as_array()); - let Some(any) = any else { - errors.push(format!("{loc}.any: expected array of glob strings")); - return; - }; - - if any.is_empty() { - errors.push(format!("{loc}.any: list must not be empty")); - return; - } - - if any.iter().any(|v| v.as_str().is_none()) { - errors.push(format!("{loc}.any: all values must be strings")); - } - - for item in any.iter().filter_map(|v| v.as_str()) { - if let Some(warning) = check_glob_syntax(item) { - warnings.push(format!("{loc}.any: {warning}")); - } } } @@ -591,11 +647,6 @@ fn json_rule_has_transport_fields(rule: &serde_json::Value) -> bool { rule.get("method").is_some() || rule.get("path").is_some() || rule.get("query").is_some() } -fn json_rule_has_non_empty_jsonrpc_params(rule: &serde_json::Value) -> bool { - rule.get("params") - .is_some_and(|v| !v.is_null() && !v.as_object().is_some_and(serde_json::Map::is_empty)) -} - fn json_rule_has_non_empty_path_or_query(rule: &serde_json::Value) -> bool { rule.get("path") .and_then(|v| v.as_str()) @@ -606,33 +657,6 @@ fn json_rule_has_non_empty_path_or_query(rule: &serde_json::Value) -> bool { .is_some_and(|v| !v.is_empty()) } -fn validate_jsonrpc_rule( - errors: &mut Vec, - warnings: &mut Vec, - loc: &str, - rule: &serde_json::Value, - kind: &str, -) { - let method = rule.get("method").and_then(|v| v.as_str()).unwrap_or(""); - if method.is_empty() { - errors.push(format!( - "{loc}: JSON-RPC {kind} rules must specify method, for example \"*\"" - )); - } else if let Some(warning) = check_glob_syntax(method) { - warnings.push(format!("{loc}.method: {warning}")); - } - - if json_rule_has_non_empty_path_or_query(rule) { - errors.push(format!( - "{loc}: JSON-RPC {kind} rules must use method/params, not path/query" - )); - } - - if let Some(params) = rule.get("params").filter(|v| !v.is_null()) { - validate_l7_matcher_map(errors, warnings, &format!("{loc}.params"), params, "params"); - } -} - fn json_endpoint_has_graphql_policy(ep: &serde_json::Value) -> bool { ep.get("graphql_persisted_queries") .and_then(|v| v.as_object()) @@ -737,15 +761,25 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< errors.push(format!("{loc}: rules and access are mutually exclusive")); } - if protocol == "json-rpc" && !access.is_empty() { + if (protocol == "json-rpc" || protocol == "mcp") && !access.is_empty() { + let method_field = if protocol == "mcp" { + "method" + } else { + "rpc_method" + }; errors.push(format!( - "{loc}: protocol json-rpc does not support access presets; use explicit rules with allow.method such as \"*\"" + "{loc}: protocol {protocol} does not support access presets; use explicit rules with allow.{method_field} such as \"*\"" )); } - if protocol == "json-rpc" && !has_rules { + if (protocol == "json-rpc" || protocol == "mcp") && !has_rules { + let method_field = if protocol == "mcp" { + "method" + } else { + "rpc_method" + }; errors.push(format!( - "{loc}: protocol json-rpc requires explicit rules with allow.method" + "{loc}: protocol {protocol} requires explicit rules with allow.{method_field}" )); } @@ -758,7 +792,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< if !protocol.is_empty() && L7Protocol::parse(protocol).is_none() { errors.push(format!( - "{loc}: unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, or json-rpc)" + "{loc}: unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, json-rpc, or mcp)" )); } @@ -807,9 +841,12 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - if protocol != "json-rpc" && ep.get("json_rpc_max_body_bytes").is_some() { + if protocol != "json-rpc" + && protocol != "mcp" + && ep.get("json_rpc_max_body_bytes").is_some() + { warnings.push(format!( - "{loc}: JSON-RPC-specific endpoint fields are ignored unless protocol is json-rpc" + "{loc}: JSON-RPC-specific endpoint fields are ignored unless protocol is json-rpc or mcp" )); } @@ -932,15 +969,22 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< // Validate query matchers — mirrors allow-side validation exactly if let Some(query) = deny_rule.get("query").filter(|v| !v.is_null()) { - validate_l7_matcher_map( + validate_matcher_map( &mut errors, &mut warnings, &format!("{deny_loc}.query"), - query, - "query", + Some(query), ); } + validate_jsonrpc_rule_fields( + &mut errors, + &mut warnings, + &deny_loc, + deny_rule, + protocol, + ); + // SQL command validation if let Some(command) = deny_rule.get("command").and_then(|c| c.as_str()) && !command.is_empty() @@ -974,19 +1018,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - if protocol == "json-rpc" { - validate_jsonrpc_rule( - &mut errors, - &mut warnings, - &deny_loc, - deny_rule, - "deny", - ); - } else if json_rule_has_non_empty_jsonrpc_params(deny_rule) { - errors.push(format!( - "{deny_loc}: params are only valid on protocol json-rpc endpoints" - )); - } } } } @@ -1028,12 +1059,11 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< continue; }; - validate_l7_matcher_map( + validate_matcher_map( &mut errors, &mut warnings, &format!("{loc}.rules[{rule_idx}].allow.query"), - query, - "query", + Some(query), ); } } @@ -1043,20 +1073,14 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< for (rule_idx, rule) in rules.iter().enumerate() { let allow = rule.get("allow").unwrap_or(rule); let rule_loc = format!("{loc}.rules[{rule_idx}].allow"); + validate_jsonrpc_rule_fields( + &mut errors, + &mut warnings, + &rule_loc, + allow, + protocol, + ); let allow_has_graphql = json_rule_has_graphql_fields(allow); - if protocol == "json-rpc" { - validate_jsonrpc_rule( - &mut errors, - &mut warnings, - &rule_loc, - allow, - "allow", - ); - } else if json_rule_has_non_empty_jsonrpc_params(allow) { - errors.push(format!( - "{rule_loc}: params are only valid on protocol json-rpc endpoints" - )); - } if websocket_has_graphql_policy && allow .get("method") diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index c2be2e4ea..9bc23d946 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -216,7 +216,9 @@ where .into_diagnostic()?; Ok(()) } - L7Protocol::JsonRpc => relay_jsonrpc(config, &engine, client, upstream, ctx).await, + L7Protocol::JsonRpc | L7Protocol::Mcp => { + relay_jsonrpc(config, &engine, client, upstream, ctx).await + } } } @@ -310,6 +312,37 @@ where } else { None }; + let jsonrpc_info = if config.protocol.is_jsonrpc_family() { + match crate::l7::http::read_body_for_inspection( + client, + &mut req, + config.json_rpc_max_body_bytes, + ) + .await + { + Ok(body) => Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( + &body, + jsonrpc_inspection_mode(config.protocol), + )), + Err(e) => { + if is_benign_connection_error(&e) { + debug!( + host = %ctx.host, + port = ctx.port, + error = %e, + "JSON-RPC L7 connection closed" + ); + } else { + let detail = + parse_rejection_detail(&e.to_string(), ParseRejectionMode::L7Endpoint); + emit_parse_rejection(ctx, &detail, "l7-jsonrpc"); + } + return Ok(()); + } + } + } else { + None + }; if close_if_stale(engine.generation_guard(), ctx) { return Ok(()); @@ -340,7 +373,7 @@ where target: redacted_target.clone(), query_params: req.query_params.clone(), graphql: graphql_info.clone(), - jsonrpc: None, + jsonrpc: jsonrpc_info.clone(), }; let websocket_request = crate::l7::rest::request_is_websocket_upgrade(&req.raw_header); if config.protocol == L7Protocol::Websocket && !websocket_request { @@ -364,7 +397,13 @@ where let parse_error_reason = graphql_info .as_ref() .and_then(|info| info.error.as_deref()) - .map(|error| format!("GraphQL request rejected: {error}")); + .map(|error| format!("GraphQL request rejected: {error}")) + .or_else(|| { + jsonrpc_info + .as_ref() + .and_then(|info| info.error.as_deref()) + .map(|error| format!("JSON-RPC request rejected: {error}")) + }); let force_deny = parse_error_reason.is_some(); let (allowed, reason) = if let Some(reason) = parse_error_reason { (false, reason) @@ -385,8 +424,12 @@ where let engine_type = match config.protocol { L7Protocol::Graphql => "l7-graphql", L7Protocol::Websocket => "l7-websocket", - L7Protocol::Rest | L7Protocol::Sql | L7Protocol::JsonRpc => "l7", + L7Protocol::JsonRpc => "l7-jsonrpc", + L7Protocol::Mcp => "l7-mcp", + L7Protocol::Rest | L7Protocol::Sql => "l7", }; + let protocol_summary = + l7_protocol_log_summary(graphql_info.as_ref(), jsonrpc_info.as_ref()); emit_l7_request_log( ctx, &request_info, @@ -394,7 +437,7 @@ where decision_str, engine_type, &reason, - graphql_info.as_ref(), + &protocol_summary, ); let _ = &eval_target; @@ -472,7 +515,7 @@ fn emit_l7_request_log( decision_str: &str, engine_type: &str, reason: &str, - graphql_info: Option<&crate::l7::graphql::GraphqlRequestInfo>, + protocol_summary: &str, ) { let (action_id, disposition_id, severity) = match decision_str { "deny" => (ActionId::Denied, DispositionId::Blocked, SeverityId::Medium), @@ -487,9 +530,6 @@ fn emit_l7_request_log( SeverityId::Informational, ), }; - let summary = graphql_info - .map(|info| format!(" {}", graphql_log_summary(info))) - .unwrap_or_default(); let event = HttpActivityBuilder::new(openshell_ocsf::ctx::ctx()) .activity(ActivityId::Other) .action(action_id) @@ -503,13 +543,33 @@ fn emit_l7_request_log( .firewall_rule(&ctx.policy_name, engine_type) .message(format!( "L7_REQUEST {decision_str} {} {}:{}{}{} reason={}", - request_info.action, ctx.host, ctx.port, redacted_target, summary, reason, + request_info.action, ctx.host, ctx.port, redacted_target, protocol_summary, reason, )) .build(); ocsf_emit!(event); emit_activity(ctx, decision_str == "deny", "l7_policy"); } +fn l7_protocol_log_summary( + graphql_info: Option<&crate::l7::graphql::GraphqlRequestInfo>, + jsonrpc_info: Option<&crate::l7::jsonrpc::JsonRpcRequestInfo>, +) -> String { + if let Some(info) = graphql_info { + return format!(" {}", graphql_log_summary(info)); + } + + if let Some(info) = jsonrpc_info { + return format!( + " rpc_methods={} params_sha256={}", + jsonrpc_methods_for_log(info), + info.params_sha256() + .unwrap_or_else(|| "".to_string()) + ); + } + + String::new() +} + fn emit_activity(ctx: &L7EvalContext, denied: bool, deny_group: &'static str) { if let Some(tx) = &ctx.activity_tx { let _ = try_record_activity(tx, denied, deny_group); @@ -660,6 +720,20 @@ pub(crate) fn websocket_extension_mode(config: &L7EndpointConfig) -> WebSocketEx } } +fn jsonrpc_inspection_mode(protocol: L7Protocol) -> crate::l7::jsonrpc::JsonRpcInspectionMode { + match protocol { + L7Protocol::Mcp => crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp, + _ => crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, + } +} + +fn jsonrpc_engine_type(protocol: L7Protocol) -> &'static str { + match protocol { + L7Protocol::Mcp => "l7-mcp", + _ => "l7-jsonrpc", + } +} + /// REST relay loop: parse request -> evaluate -> allow/deny -> relay response -> repeat. async fn relay_rest( config: &L7EndpointConfig, @@ -957,6 +1031,7 @@ where allow_encoded_slash: config.allow_encoded_slash, ..Default::default() }, + jsonrpc_inspection_mode(config.protocol), ) .await { @@ -973,7 +1048,7 @@ where } else { let detail = parse_rejection_detail(&e.to_string(), ParseRejectionMode::L7Endpoint); - emit_parse_rejection(ctx, &detail, "l7-jsonrpc"); + emit_parse_rejection(ctx, &detail, jsonrpc_engine_type(config.protocol)); } return Ok(()); } @@ -1049,7 +1124,7 @@ where OcsfUrl::new("http", &ctx.host, &redacted_target, ctx.port), )) .dst_endpoint(Endpoint::from_domain(&ctx.host, ctx.port)) - .firewall_rule(&ctx.policy_name, "l7-jsonrpc") + .firewall_rule(&ctx.policy_name, jsonrpc_engine_type(config.protocol)) .message(jsonrpc_log_message( decision_str, &request_info.action, @@ -1357,11 +1432,18 @@ pub(crate) fn rule_method_names_for_log(info: &crate::l7::jsonrpc::JsonRpcReques } info.calls .iter() - .map(|call| call.method.as_str()) + .map(|call| sanitize_log_token(&call.method)) .collect::>() .join(",") } +fn sanitize_log_token(value: &str) -> String { + value + .chars() + .map(|ch| if ch.is_control() { '?' } else { ch }) + .collect() +} + struct JsonRpcEvaluation { allowed: bool, reason: String, diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index d82ad12cc..9b71f9801 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -743,6 +743,7 @@ fn preprocess_yaml_data(yaml_str: &str) -> Result { // Normalize port → ports for all endpoints so Rego always sees "ports" array. normalize_endpoint_ports(&mut data); + normalize_l7_policy_aliases(&mut data); // Validate BEFORE expanding presets (catches user errors like rules+access) let (errors, warnings) = crate::l7::validate_l7_policies(&data); @@ -819,6 +820,143 @@ fn normalize_endpoint_ports(data: &mut serde_json::Value) { } } +fn normalize_l7_policy_aliases(data: &mut serde_json::Value) { + let Some(policies) = data + .get_mut("network_policies") + .and_then(|v| v.as_object_mut()) + else { + return; + }; + + for (_name, policy) in policies.iter_mut() { + let Some(endpoints) = policy.get_mut("endpoints").and_then(|v| v.as_array_mut()) else { + continue; + }; + + for ep in endpoints.iter_mut() { + let Some(ep_obj) = ep.as_object_mut() else { + continue; + }; + normalize_jsonrpc_config_alias(ep_obj, "json_rpc"); + normalize_jsonrpc_config_alias(ep_obj, "mcp"); + normalize_l7_rules_aliases(ep_obj); + } + } +} + +fn normalize_jsonrpc_config_alias(ep: &mut serde_json::Map, key: &str) { + let Some(config) = ep.remove(key) else { + return; + }; + let Some(max_body_bytes) = config + .as_object() + .and_then(|obj| obj.get("max_body_bytes")) + .and_then(serde_json::Value::as_u64) + else { + return; + }; + ep.entry("json_rpc_max_body_bytes".to_string()) + .or_insert_with(|| serde_json::json!(max_body_bytes)); +} + +fn normalize_l7_rules_aliases(ep: &mut serde_json::Map) { + let protocol = ep + .get("protocol") + .and_then(serde_json::Value::as_str) + .unwrap_or("") + .to_string(); + if let Some(rules) = ep.get_mut("rules").and_then(|v| v.as_array_mut()) { + for rule in rules { + if let Some(allow) = rule + .get_mut("allow") + .and_then(serde_json::Value::as_object_mut) + { + normalize_l7_rule_aliases(allow, &protocol); + } else if let Some(allow) = rule.as_object_mut() { + normalize_l7_rule_aliases(allow, &protocol); + } + } + } + + if let Some(denies) = ep.get_mut("deny_rules").and_then(|v| v.as_array_mut()) { + for deny in denies { + if let Some(deny_obj) = deny.as_object_mut() { + normalize_l7_rule_aliases(deny_obj, &protocol); + } + } + } +} + +fn normalize_l7_rule_aliases( + rule: &mut serde_json::Map, + protocol: &str, +) { + if protocol == "mcp" + && let Some(method) = rule.remove("method") + && rule + .get("rpc_method") + .and_then(serde_json::Value::as_str) + .unwrap_or("") + .is_empty() + { + rule.insert("rpc_method".to_string(), method); + } + + if let Some(tool) = rule.remove("tool") + && let Some(tool_name) = tool.as_str().filter(|s| !s.is_empty()) + { + let params = rule + .entry("params".to_string()) + .or_insert_with(|| serde_json::Value::Object(serde_json::Map::new())); + if let Some(params) = params.as_object_mut() { + params + .entry("name".to_string()) + .or_insert_with(|| serde_json::Value::String(tool_name.to_string())); + } + } + + normalize_jsonrpc_params(rule); +} + +fn normalize_jsonrpc_params(rule: &mut serde_json::Map) { + let Some(params) = rule + .get_mut("params") + .and_then(serde_json::Value::as_object_mut) + else { + return; + }; + + let mut flattened = serde_json::Map::new(); + for (key, matcher) in std::mem::take(params) { + flatten_jsonrpc_param_matcher(&key, matcher, &mut flattened); + } + *params = flattened; +} + +fn flatten_jsonrpc_param_matcher( + key: &str, + matcher: serde_json::Value, + out: &mut serde_json::Map, +) { + let serde_json::Value::Object(children) = matcher else { + out.insert(key.to_string(), matcher); + return; + }; + + if is_jsonrpc_matcher_object(&children) || children.is_empty() { + out.insert(key.to_string(), serde_json::Value::Object(children)); + return; + } + + for (child_key, child) in children { + flatten_jsonrpc_param_matcher(&format!("{key}.{child_key}"), child, out); + } +} + +fn is_jsonrpc_matcher_object(obj: &serde_json::Map) -> bool { + obj.contains_key("any") || obj.contains_key("glob") +} + /// Resolve a policy binary path through the container's root filesystem. /// /// On Linux, `/proc//root/` provides access to the container's mount @@ -1061,6 +1199,7 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St "command": a.map_or("", |a| &a.command), "operation_type": a.map_or("", |a| &a.operation_type), "operation_name": a.map_or("", |a| &a.operation_name), + "rpc_method": a.map_or("", |a| &a.rpc_method), }); if let Some(a) = a && !a.fields.is_empty() @@ -1114,6 +1253,9 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St if !d.fields.is_empty() { deny["fields"] = d.fields.clone().into(); } + if !d.rpc_method.is_empty() { + deny["rpc_method"] = d.rpc_method.clone().into(); + } let query = l7_matchers_to_json(&d.query); if !query.is_empty() { deny["query"] = query.into(); @@ -3007,6 +3149,124 @@ network_policies: assert!(!eval_l7(&engine, &blocked_with_args)); } + #[test] + fn l7_mcp_rules_filter_tools_call() { + let data = r#" +network_policies: + mcp_params: + name: mcp_params + endpoints: + - host: mcp.params.test + port: 8000 + path: /mcp + protocol: mcp + enforcement: enforce + mcp: + max_body_bytes: 131072 + rules: + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call + tool: read_status + deny_rules: + - method: tools/call + tool: blocked_action + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml"); + + let read_status = l7_jsonrpc_input_with_params( + "mcp.params.test", + 8000, + "/mcp", + "tools/call", + serde_json::json!({"name": "read_status"}), + ); + assert!(eval_l7(&engine, &read_status)); + + let blocked = l7_jsonrpc_input_with_params( + "mcp.params.test", + 8000, + "/mcp", + "tools/call", + serde_json::json!({"name": "blocked_action"}), + ); + assert!(!eval_l7(&engine, &blocked)); + } + + #[test] + fn l7_jsonrpc_null_metadata_non_matches_without_opa_error() { + let data = r#" +network_policies: + jsonrpc_null: + name: jsonrpc_null + endpoints: + - host: mcp.null.test + port: 8000 + path: /mcp + protocol: json-rpc + enforcement: enforce + rules: + - allow: + rpc_method: tools/list + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml"); + let input = serde_json::json!({ + "network": { "host": "mcp.null.test", "port": 8000 }, + "exec": { + "path": "/usr/bin/curl", + "ancestors": [], + "cmdline_paths": [] + }, + "request": { + "method": "POST", + "path": "/mcp", + "query_params": {}, + "jsonrpc": null + } + }); + + assert!(!eval_l7(&engine, &input)); + } + + #[test] + fn l7_jsonrpc_params_matcher_validation_rejects_invalid_shape() { + let data = r#" +network_policies: + invalid_jsonrpc_params: + name: invalid_jsonrpc_params + endpoints: + - host: mcp.invalid.test + port: 8000 + path: /mcp + protocol: json-rpc + enforcement: enforce + rules: + - allow: + rpc_method: tools/call + params: + name: + any: [] + binaries: + - { path: /usr/bin/curl } +"#; + let Err(err) = OpaEngine::from_strings(TEST_POLICY, data) else { + panic!("invalid params matcher should fail validation"); + }; + + assert!( + err.to_string() + .contains("params.name.any: list must not be empty"), + "unexpected validation error: {err}" + ); + } + #[test] fn l7_no_request_on_l4_only_endpoint() { // L4-only endpoint should not match L7 allow_request diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index f35b1fc8b..988c42e10 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -3585,7 +3585,7 @@ async fn handle_forward_proxy( } else { None }; - let jsonrpc = if l7_config.config.protocol == crate::l7::L7Protocol::JsonRpc { + let jsonrpc = if l7_config.config.protocol.is_jsonrpc_family() { let header_end = forward_request_bytes .windows(4) .position(|w| w == b"\r\n\r\n") @@ -3636,7 +3636,15 @@ async fn handle_forward_proxy( } }; forward_request_bytes = jsonrpc_request.raw_header; - Some(crate::l7::jsonrpc::parse_jsonrpc_body(&body)) + Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( + &body, + match l7_config.config.protocol { + crate::l7::L7Protocol::Mcp => { + crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp + } + _ => crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, + }, + )) } } else { None @@ -3693,6 +3701,7 @@ async fn handle_forward_proxy( let engine_type = match l7_config.config.protocol { crate::l7::L7Protocol::Graphql => "l7-graphql", crate::l7::L7Protocol::JsonRpc => "l7-jsonrpc", + crate::l7::L7Protocol::Mcp => "l7-mcp", _ => "l7", }; let log_message = request_info.jsonrpc.as_ref().map_or_else( diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 8114bc769..7982a8905 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -155,10 +155,10 @@ Each endpoint defines a reachable destination and optional inspection rules. | `host` | string | Yes | Hostname or IP address. Supports a `*` wildcard inside the first DNS label only: `*.example.com`, `**.example.com`, and intra-label patterns like `*-aiplatform.googleapis.com` are accepted; bare `*`/`**`, TLD wildcards (`*.com`), and wildcards outside the first label are rejected at load time. | | `port` | integer | Yes | TCP port number. | | `path` | string | No | Optional HTTP path glob used to select between L7 endpoints that share the same host and port. Empty means all paths. Use this when REST and GraphQL live under the same host, such as `/repos/**` and `/graphql`. | -| `protocol` | string | No | Set to `rest` for HTTP method/path inspection, `websocket` for RFC 6455 upgrade and client text-message inspection, `graphql` for GraphQL-over-HTTP operation inspection, or `json-rpc` for sandbox-to-server JSON-RPC-over-HTTP method and params inspection. WebSocket endpoints can also use GraphQL operation rules for GraphQL-over-WebSocket traffic. Omit for TCP passthrough. | +| `protocol` | string | No | Set to `rest` for HTTP method/path inspection, `websocket` for RFC 6455 upgrade and client text-message inspection, `graphql` for GraphQL-over-HTTP operation inspection, `mcp` for MCP Streamable HTTP request inspection, or `json-rpc` for generic JSON-RPC-over-HTTP compatibility. WebSocket endpoints can also use GraphQL operation rules for GraphQL-over-WebSocket traffic. Omit for TCP passthrough. | | `tls` | string | No | TLS handling mode. The proxy auto-detects TLS by peeking the first bytes of each connection and terminates it for inspected HTTPS traffic, so this field is optional in most cases. Set to `skip` to disable auto-detection for edge cases such as client-certificate mTLS or non-standard protocols. The values `terminate` and `passthrough` are deprecated and log a warning; they are still accepted for backward compatibility but have no effect on behavior. | | `enforcement` | string | No | `enforce` actively blocks disallowed requests. `audit` logs violations but allows traffic through. | -| `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. Not valid on `protocol: json-rpc`; JSON-RPC endpoints must use explicit `rules` with `method`. | +| `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. Not valid on `protocol: mcp` or `protocol: json-rpc`; those endpoints must use explicit rules. | | `rules` | list of rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. | | `deny_rules` | list of deny rule objects | No | L7 deny rules that block specific requests even when allowed by `access` or `rules`. Deny rules take precedence over allow rules. | | `allowed_ips` | list of string | No | CIDR or IP allowlist for SSRF override. Exact user-declared hostname endpoints may resolve to RFC 1918 private addresses without this field, but wildcard, hostless, and policy-advisor-proposed endpoints still require `allowed_ips` for private resolved IPs. Entries overlapping loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), or unspecified (`0.0.0.0`) are rejected at load time. | @@ -168,21 +168,22 @@ Each endpoint defines a reachable destination and optional inspection rules. | `persisted_queries` | string | No | GraphQL hash-only behavior for `protocol: graphql` and GraphQL-over-WebSocket operation policy. Default is `deny`; use `allow_registered` only with `graphql_persisted_queries`. | | `graphql_persisted_queries` | map | No | Trusted GraphQL persisted-query registry keyed by hash or saved-query ID. Values contain `operation_type`, optional `operation_name`, and optional root `fields`. | | `graphql_max_body_bytes` | integer | No | Maximum GraphQL-over-HTTP request body bytes buffered for inspection. Defaults to `65536`. | +| `mcp` | object | No | MCP endpoint options. For `protocol: mcp`, `mcp.max_body_bytes` sets the maximum MCP JSON-RPC-over-HTTP request body bytes buffered for inspection. Defaults to `65536`. | | `json_rpc` | object | No | JSON-RPC endpoint options. For `protocol: json-rpc`, `json_rpc.max_body_bytes` sets the maximum JSON-RPC-over-HTTP request body bytes buffered for inspection. Defaults to `65536`. | Credential rewrite recognizes the canonical `openshell:resolve:env:KEY` placeholder form and whole-token provider-shaped aliases such as `provider-OPENSHELL-RESOLVE-ENV-API_TOKEN` when the referenced environment key exists in the configured provider credentials. #### Access Levels -The `access` field accepts one of the following values on REST, WebSocket, and GraphQL endpoints. JSON-RPC endpoints reject `access` because HTTP method/path presets cannot authorize JSON-RPC safely; use explicit `rules` with `method` instead. +The `access` field accepts one of the following values on REST, WebSocket, and GraphQL endpoints. MCP and JSON-RPC endpoints reject `access` because HTTP method/path presets cannot authorize JSON-RPC safely; use explicit rules instead. -| Value | REST expansion | WebSocket expansion | GraphQL expansion | JSON-RPC behavior | +| Value | REST expansion | WebSocket expansion | GraphQL expansion | MCP / JSON-RPC expansion | |---|---|---|---|---| | `full` | All methods and paths. | WebSocket upgrade and all inspected client text-message paths. | All operation types. | Rejected. | | `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | Rejected. | | `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | Rejected. | -For JSON-RPC endpoints, configure explicit `rules` with `method` and optional `params`. +For MCP endpoints, configure explicit `rules` with `method`, optional `tool`, and optional `params`. For generic JSON-RPC endpoints, use `rpc_method` and optional `params`. #### Allow Rule Objects @@ -277,14 +278,58 @@ rules: Do not combine `method`, `path`, or `query` with `operation_type`, `operation_name`, or `fields` inside the same WebSocket rule. When a WebSocket endpoint has GraphQL operation policy, use GraphQL rules for client messages instead of a raw `WEBSOCKET_TEXT` allow rule. +##### MCP Allow And Deny Rules (`protocol: mcp`) + +MCP rules match sandbox-to-server MCP Streamable HTTP request bodies by MCP method and optional tool or params selectors. OpenShell parses the underlying JSON-RPC 2.0 envelope, validates known MCP request and notification params, and preserves unknown extension methods as policy-addressable method strings. JSON-RPC responses and server-to-client MCP messages on response bodies or SSE streams are relayed but are not currently parsed for policy enforcement. + +Deny rules take precedence over allow rules. In a batch request, one denied call denies the full batch. + +| Field | Type | Required | Description | +|---|---|---|---| +| `method` | string | Yes | MCP method name or OpenShell glob, such as `initialize`, `tools/list`, `tools/call`, or `tools/*`. `*` is OpenShell policy matching syntax, not JSON-RPC method syntax. | +| `tool` | string | No | Convenience matcher for `tools/call` `params.name`. Omit to match every tool for the method. | +| `params` | map | No | Nested params matcher map. Matcher leaves can be a glob string or an object with `any`. Dot-separated keys such as `arguments.repository` remain accepted for compatibility. Requests with literal `.` characters in params object keys are rejected before policy evaluation. | + +Example MCP rules: + +```yaml showLineNumbers={false} +endpoints: + - host: mcp.example.com + port: 443 + path: /mcp + protocol: mcp + enforcement: enforce + mcp: + max_body_bytes: 131072 + rules: + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call + tool: search_web + - allow: + method: tools/call + tool: create_issue + params: + arguments: + repository: NVIDIA/OpenShell + deny_rules: + - method: tools/call + tool: send_email + - method: tools/call + tool: execute_code +``` + ##### JSON-RPC Allow Rule (`protocol: json-rpc`) -JSON-RPC allow rules match sandbox-to-server JSON-RPC-over-HTTP request objects by RPC method and optional params. They apply to single JSON-RPC requests and batch requests. For a batch, OpenShell evaluates each call independently. Client-to-server JSON-RPC response frames in POST bodies are denied. Server-to-client messages on HTTP response bodies or MCP SSE streams are relayed but are not currently parsed for policy enforcement. +JSON-RPC allow rules match sandbox-to-server JSON-RPC-over-HTTP request objects by RPC method and optional params. They apply to single JSON-RPC requests and batch requests. For a batch, OpenShell evaluates each call independently. JSON-RPC responses and server-to-client messages on response bodies or SSE streams are relayed but are not currently parsed for policy enforcement. | Field | Type | Required | Description | |---|---|---|---| -| `method` | string | Yes | JSON-RPC method name such as `initialize`, `tools/list`, `tools/*`, or glob, `*`, which matches any JSON-RPC method. | -| `params` | map | No | Params matchers keyed by flattened object-param path. Use dot-separated keys for nested object params, such as `arguments.scope`. Matcher value can be a glob string or an object with `any`. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. | +| `rpc_method` | string | Yes | JSON-RPC method name or OpenShell glob, such as `initialize`, `tools/list`, or `tools/*`. `*` is OpenShell policy matching syntax, not JSON-RPC method syntax. | +| `params` | map | No | Nested params matcher map. Matcher leaves can be a glob string or an object with `any`. Dot-separated keys such as `arguments.scope` remain accepted for compatibility. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. | Example JSON-RPC allow rules: @@ -299,18 +344,19 @@ endpoints: max_body_bytes: 131072 rules: - allow: - method: initialize + rpc_method: initialize - allow: - method: tools/list + rpc_method: tools/list - allow: - method: tools/call + rpc_method: tools/call params: name: read_status - allow: - method: tools/call + rpc_method: tools/call params: name: submit_report - arguments.scope: workspace/main + arguments: + scope: workspace/main ``` #### Deny Rule Objects @@ -401,8 +447,8 @@ JSON-RPC deny rules use the same field names as JSON-RPC allow rules, but they a | Field | Type | Required | Description | |---|---|---|---| -| `method` | string | Yes | JSON-RPC method name or glob to deny. A value without glob metacharacters matches that exact method name. In glob patterns, `*` matches any character sequence, so `method: "*"` denies any JSON-RPC method rather than a literal method named `*`. To deny a literal method named `*`, escape the glob character as `method: "\\*"`. | -| `params` | map | No | Params matchers keyed by flattened object-param path. Omit to deny every call matching `method`. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. | +| `rpc_method` | string | Yes | JSON-RPC method name or glob to deny. A value without glob metacharacters matches that exact method name. In glob patterns, `*` matches any character sequence, so `rpc_method: "*"` denies any JSON-RPC method rather than a literal method named `*`. To deny a literal method named `*`, escape the glob character as `rpc_method: "\\*"`. | +| `params` | map | No | Params matchers keyed by flattened object-param path. Omit to deny every call matching `rpc_method`. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. | Example JSON-RPC deny rules: @@ -415,9 +461,9 @@ endpoints: enforcement: enforce rules: - allow: - method: tools/* + rpc_method: tools/* deny_rules: - - method: tools/call + - rpc_method: tools/call params: name: delete_resource ``` diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 1f9912441..87fb0bdbc 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -148,7 +148,7 @@ The following steps outline the hot-reload policy update workflow. To inspect a stored sandbox-authored revision instead of the current effective policy, pass `--rev `. -5. Edit the YAML: add or adjust `network_policies` entries, binaries, `access`, `rules`, or protocol-specific matchers such as GraphQL operation fields and JSON-RPC `method` / `params` rules. +5. Edit the YAML: add or adjust `network_policies` entries, binaries, `access`, `rules`, or protocol-specific matchers such as GraphQL operation fields, MCP `method` / `tool` rules, and generic JSON-RPC `rpc_method` / `params` rules. 6. Push the updated policy when you need a full replacement. Exit codes: 0 = loaded, 1 = validation failed, 124 = timeout. @@ -173,7 +173,7 @@ Use `openshell policy update` when you want to merge network policy changes into - remove one endpoint or one named rule without rewriting the rest of the file. - preview a merged result locally with `--dry-run` before you send it to the gateway. -Use `openshell policy set` instead when you want to replace the full policy, update static sections, or make broader edits that are easier to express in YAML. Use full YAML for GraphQL and JSON-RPC rule shapes. +Use `openshell policy set` instead when you want to replace the full policy, update static sections, or make broader edits that are easier to express in YAML. Use full YAML for GraphQL, MCP, and JSON-RPC rule shapes. ### Update Commands @@ -210,7 +210,7 @@ This is the practical difference: Current constraints: - `--add-allow` and `--add-deny` work on `protocol: rest` and `protocol: websocket` endpoints. -- GraphQL and JSON-RPC fine-grained rules require full policy YAML applied with `openshell policy set`. +- GraphQL, MCP, and JSON-RPC fine-grained rules require full policy YAML applied with `openshell policy set`. - `--add-deny` requires the endpoint to already have an allow base, either an `access` preset or explicit allow `rules`. - `protocol: sql` is not a practical incremental workflow today. OpenShell does not do full SQL parsing, and SQL enforcement is not meaningfully supported yet. @@ -549,7 +549,7 @@ For an end-to-end walkthrough that combines this policy with a GitHub credential - { path: /usr/bin/gh } ``` -Endpoints with `protocol: rest` enable HTTP request inspection and can opt in to supported text request body credential rewrite. Endpoints with `protocol: websocket` validate WebSocket upgrades and inspect client text messages on the upgraded request path. WebSocket endpoints can also classify GraphQL-over-WebSocket operation messages with the same operation rules used by GraphQL-over-HTTP. Endpoints with `protocol: graphql` parse GraphQL-over-HTTP payloads before evaluating rules. Endpoints with `protocol: json-rpc` parse JSON-RPC-over-HTTP request bodies and evaluate `method` and optional params rules. The endpoint-level `path` field lets these protocols share `api.github.com:443` without treating GraphQL payloads as plain REST `POST /graphql` requests. +Endpoints with `protocol: rest` enable HTTP request inspection and can opt in to supported text request body credential rewrite. Endpoints with `protocol: websocket` validate WebSocket upgrades and inspect client text messages on the upgraded request path. WebSocket endpoints can also classify GraphQL-over-WebSocket operation messages with the same operation rules used by GraphQL-over-HTTP. Endpoints with `protocol: graphql` parse GraphQL-over-HTTP payloads before evaluating rules. Endpoints with `protocol: mcp` parse MCP Streamable HTTP request bodies and evaluate `method`, optional `tool`, and optional params rules. Endpoints with `protocol: json-rpc` keep generic JSON-RPC-over-HTTP compatibility by evaluating `rpc_method` and optional params rules. The endpoint-level `path` field lets these protocols share `api.github.com:443` without treating GraphQL payloads as plain REST `POST /graphql` requests. @@ -580,13 +580,13 @@ REST rules can also constrain query parameter values: `query` matchers are case-sensitive and run on decoded values. If a request has duplicate keys (for example, `tag=a&tag=b`), every value for that key must match the configured glob(s). -### JSON-RPC matching +### MCP and JSON-RPC matching -JSON-RPC endpoints use `protocol: json-rpc`. The proxy parses sandbox-to-server JSON-RPC-over-HTTP request bodies, evaluates the JSON-RPC `method` field against rule `method`, and can match object params through dot-separated `params` keys. +MCP endpoints use `protocol: mcp`. The proxy parses sandbox-to-server MCP Streamable HTTP request bodies, validates known MCP request and notification params, evaluates the MCP method against rule `method`, and can match tool calls with the `tool` alias. Generic JSON-RPC endpoints use `protocol: json-rpc`, evaluate `rpc_method`, and can match object params. -JSON-RPC policy enforcement is directional. It applies to HTTP request bodies sent by the sandboxed process to the configured endpoint. JSON-RPC responses and server-to-client messages carried on response bodies or MCP SSE streams are relayed but are not currently parsed for policy enforcement. +MCP policy enforcement is directional. It applies to HTTP request bodies sent by the sandboxed process to the configured endpoint. JSON-RPC responses and server-to-client MCP messages carried on response bodies or SSE streams are relayed but are not currently parsed for policy enforcement. -JSON-RPC endpoint policies currently require full policy YAML applied with `openshell policy set`; the incremental `openshell policy update --add-endpoint` parser does not accept `json-rpc` as a protocol. +MCP and JSON-RPC endpoint policies currently require full policy YAML applied with `openshell policy set`; the incremental `openshell policy update --add-endpoint` parser does not accept `mcp` or `json-rpc` as protocols. ```yaml showLineNumbers={false} mcp_server: @@ -595,9 +595,9 @@ JSON-RPC endpoint policies currently require full policy YAML applied with `open - host: mcp.example.com port: 443 path: /mcp - protocol: json-rpc + protocol: mcp enforcement: enforce - json_rpc: + mcp: max_body_bytes: 131072 rules: - allow: @@ -606,24 +606,25 @@ JSON-RPC endpoint policies currently require full policy YAML applied with `open method: tools/list - allow: method: tools/call - params: - name: read_status + tool: read_status - allow: method: tools/call + tool: submit_report params: - name: submit_report - arguments.scope: workspace/main + arguments: + scope: workspace/main deny_rules: - method: tools/call - params: - name: delete_resource + tool: delete_resource binaries: - { path: /usr/bin/python3 } ``` -`json_rpc.max_body_bytes` controls how many JSON-RPC-over-HTTP request body bytes OpenShell buffers for inspection. It defaults to `65536`. +`mcp.max_body_bytes` controls how many MCP-over-HTTP request body bytes OpenShell buffers for inspection. It defaults to `65536`. + +Use `protocol: json-rpc` and `rpc_method` when you need generic JSON-RPC 2.0 matching for a non-MCP server. `json_rpc.max_body_bytes` controls the generic JSON-RPC inspection buffer. -`params` matchers are case-sensitive and use the same string glob or `{ any: [...] }` matcher syntax as REST query parameters. They match scalar leaf values from object params: strings, numbers, and booleans are converted to strings, and nested JSON object params are flattened with dot-separated keys before matching. Arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. This is useful for controls such as matching MCP `tools/call` by `params.name`, but it is not a complete MCP payload policy for rich nested content. For batch requests, OpenShell evaluates each JSON-RPC call independently and denies the whole batch if any call is denied. +`params` matchers are case-sensitive and use the same string glob or `{ any: [...] }` matcher syntax as REST query parameters. They match scalar leaf values from object params: strings, numbers, and booleans are converted to strings, and nested JSON object params are flattened with dot-separated keys before matching. Arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. This is useful for controls such as matching MCP `tools/call` by `tool` or `params.name`, but it is not a complete MCP payload policy for rich nested content. For batch requests, OpenShell evaluates each JSON-RPC call independently and denies the whole batch if any call is denied. ### GraphQL matching diff --git a/e2e/mcp-conformance/README.md b/e2e/mcp-conformance/README.md index 5f0659323..54f808797 100644 --- a/e2e/mcp-conformance/README.md +++ b/e2e/mcp-conformance/README.md @@ -35,11 +35,15 @@ bridge at `host.openshell.internal` (the alias `e2e/with-docker-gateway.sh` attaches to the CI job container on the e2e network), at `host.docker.internal` on local Docker Desktop, or via `--add-host ...:host-gateway` on local Linux. -The generated policy allows valid JSON-RPC requests to the conformance server -with `method: "*"`. That keeps OpenShell deny-by-default at the network -boundary while allowing the upstream scenarios to exercise MCP behavior. The -policy body lives in `policy-template.yaml`; the wrapper renders its host, port, -and path placeholders from the upstream server URL. +The generated policy uses `protocol: mcp` and allows valid MCP requests to the +conformance server with `method: "*"`. That keeps OpenShell deny-by-default +at the network boundary while allowing the upstream scenarios to exercise MCP +behavior. The policy body lives in `policy-template.yaml`; the wrapper renders +its host, port, and path placeholders from the upstream server URL. + +For local runs, build or stage a static supervisor binary and pass it with +`OPENSHELL_DOCKER_SUPERVISOR_BIN` if the default local supervisor build is +linked against a newer glibc than the conformance client image provides. The upstream `everything-client` has a few handler names that do not line up with released-spec scenario names. The wrapper maps those names when forwarding @@ -47,7 +51,7 @@ with released-spec scenario names. The wrapper maps those names when forwarding checkout. When enabling broader upstream suites, add scenarios that OpenShell does not yet -support through the JSON-RPC proxy to `expected-failures.yml`. The upstream +support through the MCP proxy to `expected-failures.yml`. The upstream runner treats listed failures as allowed and treats stale entries as failures. The default run uses a static scenario list in `e2e/mcp-conformance.sh`. To refresh it after changing the pinned upstream ref or default spec, list the diff --git a/e2e/mcp-conformance/client-through-openshell.sh b/e2e/mcp-conformance/client-through-openshell.sh index f236c91a3..4cb6a3465 100755 --- a/e2e/mcp-conformance/client-through-openshell.sh +++ b/e2e/mcp-conformance/client-through-openshell.sh @@ -99,7 +99,17 @@ SANDBOX_COMMAND=( --timeout "${CLIENT_TIMEOUT_SECONDS}" "${ENV_ARGS[@]}" -- - sh -c 'cd /opt/mcp-conformance && exec ./node_modules/.bin/tsx examples/clients/typescript/everything-client.ts "$1"' + sh -c ' + cd /opt/mcp-conformance + # Keep canonical runner scenario names in the environment. The wrapper only + # swaps client entrypoints for upstream reference-client fixture drift. + case "${MCP_CONFORMANCE_SCENARIO:-}" in + tools_call|tools-call) client=examples/clients/typescript/test2.ts ;; + sse-retry) client=examples/clients/typescript/sse-retry-test.ts ;; + *) client=examples/clients/typescript/everything-client.ts ;; + esac + exec ./node_modules/.bin/tsx "$client" "$1" + ' \ sh "${CLIENT_SERVER_URL}" ) diff --git a/e2e/mcp-conformance/expected-failures.yml b/e2e/mcp-conformance/expected-failures.yml index 167c17183..731faad69 100644 --- a/e2e/mcp-conformance/expected-failures.yml +++ b/e2e/mcp-conformance/expected-failures.yml @@ -2,8 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 # Add client scenarios here when enabling broader MCP conformance suites that -# currently fail in the pinned upstream example client or through OpenShell. -client: - - elicitation-sep1034-client-defaults - - sse-retry +# exercise features OpenShell does not yet support through the MCP proxy. +client: [] server: [] diff --git a/e2e/mcp-conformance/policy-template.yaml b/e2e/mcp-conformance/policy-template.yaml index 9554dc5fa..e849cdeb9 100644 --- a/e2e/mcp-conformance/policy-template.yaml +++ b/e2e/mcp-conformance/policy-template.yaml @@ -36,14 +36,14 @@ network_policies: - ${host_spec} ${port_spec} path: ${path} - protocol: json-rpc + protocol: mcp enforcement: enforce allowed_ips: - "10.0.0.0/8" - "172.16.0.0/12" - "192.168.0.0/16" - "fc00::/7" - json_rpc: + mcp: max_body_bytes: 131072 rules: - allow: diff --git a/e2e/with-docker-gateway.sh b/e2e/with-docker-gateway.sh index 5dc61e6c1..36df0c404 100755 --- a/e2e/with-docker-gateway.sh +++ b/e2e/with-docker-gateway.sh @@ -21,6 +21,10 @@ # The default community sandbox image uses :latest. This wrapper refreshes it # before starting the gateway, while the Docker driver defaults to IfNotPresent # so local Dockerfile-built images remain usable. +# +# Set OPENSHELL_DOCKER_SUPERVISOR_BIN to force a specific Linux +# openshell-sandbox binary. This is useful when a staged static supervisor +# binary should be used instead of the host glibc-linked local target build. set -euo pipefail @@ -428,32 +432,46 @@ fi e2e_build_gateway_binaries "${ROOT}" TARGET_DIR GATEWAY_BIN CLI_BIN -SUPERVISOR_IMAGE="$(resolve_docker_supervisor_image)" -if [ -n "${SUPERVISOR_IMAGE}" ]; then - ensure_docker_supervisor_image "${SUPERVISOR_IMAGE}" - echo "Using Docker supervisor image: ${SUPERVISOR_IMAGE}" - DOCKER_SUPERVISOR_ARGS=(--docker-supervisor-image "${SUPERVISOR_IMAGE}") -else - echo "Building openshell-sandbox for ${SUPERVISOR_TARGET}..." - mkdir -p "${SUPERVISOR_OUT_DIR}" - if [ "${HOST_OS}" = "Linux" ] && [ "${HOST_ARCH}" = "${DAEMON_ARCH}" ]; then - rustup target add "${SUPERVISOR_TARGET}" >/dev/null 2>&1 || true - cargo build ${CARGO_BUILD_JOBS_ARG[@]+"${CARGO_BUILD_JOBS_ARG[@]}"} \ - --release -p openshell-sandbox --target "${SUPERVISOR_TARGET}" - cp "${TARGET_DIR}/${SUPERVISOR_TARGET}/release/openshell-sandbox" "${SUPERVISOR_BIN}" - else - CONTAINER_ENGINE=docker \ - DOCKER_PLATFORM="linux/${DAEMON_ARCH}" \ - DOCKER_OUTPUT="type=local,dest=${SUPERVISOR_OUT_DIR}" \ - bash "${ROOT}/tasks/scripts/docker-build-image.sh" supervisor-output - fi - +if [ -n "${OPENSHELL_DOCKER_SUPERVISOR_BIN:-}" ]; then + case "${OPENSHELL_DOCKER_SUPERVISOR_BIN}" in + /*) SUPERVISOR_BIN="${OPENSHELL_DOCKER_SUPERVISOR_BIN}" ;; + *) SUPERVISOR_BIN="${ROOT}/${OPENSHELL_DOCKER_SUPERVISOR_BIN}" ;; + esac if [ ! -f "${SUPERVISOR_BIN}" ]; then - echo "ERROR: expected supervisor binary at ${SUPERVISOR_BIN}" >&2 - exit 1 + echo "ERROR: Docker supervisor binary '${SUPERVISOR_BIN}' does not exist." >&2 + exit 2 fi chmod +x "${SUPERVISOR_BIN}" + echo "Using Docker supervisor binary: ${SUPERVISOR_BIN}" DOCKER_SUPERVISOR_ARGS=(--docker-supervisor-bin "${SUPERVISOR_BIN}") +else + SUPERVISOR_IMAGE="$(resolve_docker_supervisor_image)" + if [ -n "${SUPERVISOR_IMAGE}" ]; then + ensure_docker_supervisor_image "${SUPERVISOR_IMAGE}" + echo "Using Docker supervisor image: ${SUPERVISOR_IMAGE}" + DOCKER_SUPERVISOR_ARGS=(--docker-supervisor-image "${SUPERVISOR_IMAGE}") + else + echo "Building openshell-sandbox for ${SUPERVISOR_TARGET}..." + mkdir -p "${SUPERVISOR_OUT_DIR}" + if [ "${HOST_OS}" = "Linux" ] && [ "${HOST_ARCH}" = "${DAEMON_ARCH}" ]; then + rustup target add "${SUPERVISOR_TARGET}" >/dev/null 2>&1 || true + cargo build ${CARGO_BUILD_JOBS_ARG[@]+"${CARGO_BUILD_JOBS_ARG[@]}"} \ + --release -p openshell-sandbox --target "${SUPERVISOR_TARGET}" + cp "${TARGET_DIR}/${SUPERVISOR_TARGET}/release/openshell-sandbox" "${SUPERVISOR_BIN}" + else + CONTAINER_ENGINE=docker \ + DOCKER_PLATFORM="linux/${DAEMON_ARCH}" \ + DOCKER_OUTPUT="type=local,dest=${SUPERVISOR_OUT_DIR}" \ + bash "${ROOT}/tasks/scripts/docker-build-image.sh" supervisor-output + fi + + if [ ! -f "${SUPERVISOR_BIN}" ]; then + echo "ERROR: expected supervisor binary at ${SUPERVISOR_BIN}" >&2 + exit 1 + fi + chmod +x "${SUPERVISOR_BIN}" + DOCKER_SUPERVISOR_ARGS=(--docker-supervisor-bin "${SUPERVISOR_BIN}") + fi fi DEFAULT_SANDBOX_IMAGE="ghcr.io/nvidia/openshell-community/sandboxes/base:latest" From 636c27a05bce79b647f0db688fc2caead67afcd4 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:02:25 -0700 Subject: [PATCH 02/12] refactor(mcp): parse calls with tower protocol types Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- .../src/l7/jsonrpc.rs | 65 ++++++++++++++----- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs index eda948819..b197471a1 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -8,6 +8,9 @@ use sha2::{Digest, Sha256}; use std::collections::BTreeMap; use std::collections::HashMap; use tokio::io::{AsyncRead, AsyncWrite}; +use tower_mcp_types::protocol::{ + JSONRPC_VERSION, JsonRpcNotification, JsonRpcRequest, McpNotification, McpRequest, +}; use crate::l7::provider::{L7Provider, L7Request}; @@ -223,7 +226,7 @@ fn parse_jsonrpc_message( .get("jsonrpc") .and_then(|v| v.as_str()) .ok_or_else(|| "missing or non-string 'jsonrpc' field".to_string())?; - if version != "2.0" { + if version != JSONRPC_VERSION { return Err(format!("unsupported JSON-RPC version '{version}'")); } @@ -249,17 +252,16 @@ fn parse_jsonrpc_call( value: &serde_json::Value, inspection_mode: JsonRpcInspectionMode, ) -> std::result::Result { + if inspection_mode == JsonRpcInspectionMode::Mcp { + return parse_mcp_call(value); + } + let method = value .get("method") .and_then(|m| m.as_str()) .ok_or_else(|| "missing or non-string 'method' field".to_string())?; - let params = value - .get("params") - .map_or_else(|| Ok(HashMap::new()), flatten_jsonrpc_params)?; + let params = flatten_jsonrpc_params_opt(value.get("params"))?; let tool = params.get("name").cloned(); - if inspection_mode == JsonRpcInspectionMode::Mcp { - validate_mcp_call(value)?; - } Ok(JsonRpcCallInfo { method: method.to_string(), params, @@ -296,24 +298,39 @@ fn parse_jsonrpc_response(value: &serde_json::Value) -> std::result::Result<(), Ok(()) } -fn validate_mcp_call(value: &serde_json::Value) -> std::result::Result<(), String> { +fn parse_mcp_call(value: &serde_json::Value) -> std::result::Result { if value.get("id").is_some() { - let request: tower_mcp_types::protocol::JsonRpcRequest = - serde_json::from_value(value.clone()) - .map_err(|error| format!("invalid MCP request: {error}"))?; + let request: JsonRpcRequest = serde_json::from_value(value.clone()) + .map_err(|error| format!("invalid MCP request: {error}"))?; request .validate() .map_err(|error| format!("invalid MCP request: {error:?}"))?; - tower_mcp_types::protocol::McpRequest::from_jsonrpc(&request) + let mcp_request = McpRequest::from_jsonrpc(&request) .map_err(|error| format!("invalid MCP request params: {error}"))?; + + return Ok(JsonRpcCallInfo { + method: mcp_request.method_name().to_string(), + params: flatten_jsonrpc_params_opt(request.params.as_ref())?, + tool: mcp_tool_name(&mcp_request), + }); } else { - let notification: tower_mcp_types::protocol::JsonRpcNotification = - serde_json::from_value(value.clone()) - .map_err(|error| format!("invalid MCP notification: {error}"))?; - tower_mcp_types::protocol::McpNotification::from_jsonrpc(¬ification) + let notification: JsonRpcNotification = serde_json::from_value(value.clone()) + .map_err(|error| format!("invalid MCP notification: {error}"))?; + if notification.jsonrpc != JSONRPC_VERSION { + return Err(format!( + "unsupported JSON-RPC version '{}'", + notification.jsonrpc + )); + } + McpNotification::from_jsonrpc(¬ification) .map_err(|error| format!("invalid MCP notification params: {error}"))?; + + return Ok(JsonRpcCallInfo { + method: notification.method, + params: flatten_jsonrpc_params_opt(notification.params.as_ref())?, + tool: None, + }); } - Ok(()) } fn flatten_jsonrpc_params( @@ -324,6 +341,20 @@ fn flatten_jsonrpc_params( Ok(params) } +fn flatten_jsonrpc_params_opt( + value: Option<&serde_json::Value>, +) -> std::result::Result, String> { + value.map_or_else(|| Ok(HashMap::new()), flatten_jsonrpc_params) +} + +fn mcp_tool_name(request: &McpRequest) -> Option { + if let McpRequest::CallTool(params) = request { + Some(params.name.clone()) + } else { + None + } +} + fn canonical_params_map(params: &HashMap) -> BTreeMap { params .iter() From 60b33cb2dd7ece5952631f2612135e59e5933da5 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:17:24 -0700 Subject: [PATCH 03/12] feat(policy): support nested MCP params matchers Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 121 ++++++++++++-- .../src/l7/mod.rs | 158 ++++++++++++------ .../openshell-supervisor-network/src/opa.rs | 20 ++- docs/sandboxes/policies.mdx | 2 +- 4 files changed, 235 insertions(+), 66 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 4bf010716..c7fbfbcf6 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -261,17 +261,24 @@ struct L7AllowDef { #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - params: BTreeMap, + params: BTreeMap, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] enum QueryMatcherDef { Glob(String), Any(QueryAnyDef), } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(untagged)] +enum ParamMatcherDef { + Matcher(QueryMatcherDef), + Object(BTreeMap), +} + +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct QueryAnyDef { #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -302,7 +309,7 @@ struct L7DenyRuleDef { #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - params: BTreeMap, + params: BTreeMap, } #[derive(Debug, Serialize, Deserialize)] @@ -341,6 +348,91 @@ fn matcher_glob(glob: String) -> QueryMatcherDef { QueryMatcherDef::Glob(glob) } +fn param_matcher_glob(glob: String) -> ParamMatcherDef { + ParamMatcherDef::Matcher(matcher_glob(glob)) +} + +fn flatten_param_matchers( + params: BTreeMap, +) -> BTreeMap { + let mut flattened = BTreeMap::new(); + for (key, matcher) in params { + flatten_param_matcher(&key, matcher, &mut flattened); + } + flattened +} + +fn flatten_param_matcher( + key: &str, + matcher: ParamMatcherDef, + out: &mut BTreeMap, +) { + match matcher { + ParamMatcherDef::Matcher(matcher) => { + out.insert(key.to_string(), matcher); + } + ParamMatcherDef::Object(children) => { + for (child_key, child) in children { + let nested_key = format!("{key}.{child_key}"); + flatten_param_matcher(&nested_key, child, out); + } + } + } +} + +fn flat_params_to_def( + protocol: &str, + params: BTreeMap, +) -> BTreeMap { + let flat = params.into_iter().collect::>(); + if !is_mcp_protocol(protocol) { + return flat_param_matchers_to_def(flat); + } + + let mut nested = BTreeMap::new(); + for (key, matcher) in &flat { + if insert_nested_param(&mut nested, key, ParamMatcherDef::Matcher(matcher.clone())).is_err() + { + return flat_param_matchers_to_def(flat); + } + } + nested +} + +fn flat_param_matchers_to_def( + params: Vec<(String, QueryMatcherDef)>, +) -> BTreeMap { + params + .into_iter() + .map(|(key, matcher)| (key, ParamMatcherDef::Matcher(matcher))) + .collect() +} + +fn insert_nested_param( + root: &mut BTreeMap, + key: &str, + matcher: ParamMatcherDef, +) -> Result<(), ()> { + let mut parts = key.split('.').peekable(); + let Some(first) = parts.next() else { + return Err(()); + }; + + if parts.peek().is_none() { + root.insert(first.to_string(), matcher); + return Ok(()); + } + + let child = root + .entry(first.to_string()) + .or_insert_with(|| ParamMatcherDef::Object(BTreeMap::new())); + let ParamMatcherDef::Object(children) = child else { + return Err(()); + }; + let remainder = parts.collect::>().join("."); + insert_nested_param(children, &remainder, matcher) +} + fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { if mcp_method.is_empty() { rpc_method @@ -350,13 +442,13 @@ fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { } fn params_with_tool( - mut params: BTreeMap, + mut params: BTreeMap, tool: String, -) -> BTreeMap { +) -> BTreeMap { if !tool.is_empty() { params .entry("name".to_string()) - .or_insert_with(|| matcher_glob(tool)); + .or_insert_with(|| param_matcher_glob(tool)); } params } @@ -375,7 +467,7 @@ fn allow_def_to_proto(allow: L7AllowDef) -> L7Allow { .into_iter() .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) .collect(), - params: params_with_tool(allow.params, allow.tool) + params: flatten_param_matchers(params_with_tool(allow.params, allow.tool)) .into_iter() .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) .collect(), @@ -396,7 +488,7 @@ fn deny_def_to_proto(deny: L7DenyRuleDef) -> L7DenyRule { .into_iter() .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) .collect(), - params: params_with_tool(deny.params, deny.tool) + params: flatten_param_matchers(params_with_tool(deny.params, deny.tool)) .into_iter() .map(|(key, matcher)| (key, matcher_def_to_proto(matcher))) .collect(), @@ -441,6 +533,7 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { .map(|(key, matcher)| (key, matcher_proto_to_def(matcher))) .collect(); let (tool, params) = split_tool_param(protocol, params); + let params = flat_params_to_def(protocol, params); let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { (String::new(), allow.rpc_method) } else { @@ -472,6 +565,7 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { .map(|(key, matcher)| (key.clone(), matcher_proto_to_def(matcher.clone()))) .collect(); let (tool, params) = split_tool_param(protocol, params); + let params = flat_params_to_def(protocol, params); let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { (String::new(), deny.rpc_method.clone()) } else { @@ -1955,7 +2049,8 @@ network_policies: - mcp_method: tools/call tool: search_web params: - arguments.repository: NVIDIA/OpenShell + arguments: + repository: NVIDIA/OpenShell binaries: - path: /usr/bin/curl "; @@ -1996,6 +2091,9 @@ network_policies: allow: - mcp_method: tools/call tool: search_web + params: + arguments: + repository: NVIDIA/OpenShell deny: - mcp_method: tools/call tool: send_email @@ -2010,6 +2108,9 @@ network_policies: assert!(yaml_out.contains("mcp_method: tools/call")); assert!(yaml_out.contains("tool: search_web")); assert!(yaml_out.contains("tool: send_email")); + assert!(yaml_out.contains("arguments:")); + assert!(yaml_out.contains("repository: NVIDIA/OpenShell")); + assert!(!yaml_out.contains("arguments.repository")); assert!(yaml_out.contains("mcp:")); assert_eq!(proto1, proto2); } diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index fd47a4de8..57eb4d305 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -504,71 +504,84 @@ fn validate_matcher_map( }; for (key, matcher) in obj { - if let Some(glob_str) = matcher.as_str() { - if let Some(warning) = check_glob_syntax(glob_str) { - warnings.push(format!("{loc}.{key}: {warning}")); - } - continue; + validate_matcher_value(errors, warnings, &format!("{loc}.{key}"), matcher); + } +} + +fn validate_matcher_value( + errors: &mut Vec, + warnings: &mut Vec, + loc: &str, + matcher: &serde_json::Value, +) { + if let Some(glob_str) = matcher.as_str() { + if let Some(warning) = check_glob_syntax(glob_str) { + warnings.push(format!("{loc}: {warning}")); } + return; + } - let Some(matcher_obj) = matcher.as_object() else { - errors.push(format!( - "{loc}.{key}: expected string glob or object with `any`" - )); - continue; - }; + let Some(matcher_obj) = matcher.as_object() else { + errors.push(format!( + "{loc}: expected string glob, matcher object, or nested matcher map" + )); + return; + }; - let has_any = matcher_obj.get("any").is_some(); - let has_glob = matcher_obj.get("glob").is_some(); - let has_unknown = matcher_obj.keys().any(|k| k != "any" && k != "glob"); - if has_unknown { - errors.push(format!( - "{loc}.{key}: unknown matcher keys; only `glob` or `any` are supported" - )); - continue; + let has_any = matcher_obj.get("any").is_some(); + let has_glob = matcher_obj.get("glob").is_some(); + if !has_any && !has_glob { + if matcher_obj.is_empty() { + errors.push(format!("{loc}: nested matcher map must not be empty")); + return; } - - if has_glob && has_any { - errors.push(format!( - "{loc}.{key}: matcher cannot specify both `glob` and `any`" - )); - continue; + for (key, child) in matcher_obj { + validate_matcher_value(errors, warnings, &format!("{loc}.{key}"), child); } + return; + } - if !has_glob && !has_any { - errors.push(format!( - "{loc}.{key}: object matcher requires `glob` string or non-empty `any` list" - )); - continue; - } + let has_unknown = matcher_obj.keys().any(|k| k != "any" && k != "glob"); + if has_unknown { + errors.push(format!( + "{loc}: unknown matcher keys; only `glob` or `any` are supported" + )); + return; + } - if has_glob { - match matcher_obj.get("glob").and_then(|v| v.as_str()) { - None => errors.push(format!("{loc}.{key}.glob: expected glob string")), - Some(glob_str) => { - if let Some(warning) = check_glob_syntax(glob_str) { - warnings.push(format!("{loc}.{key}.glob: {warning}")); - } + if has_glob && has_any { + errors.push(format!( + "{loc}: matcher cannot specify both `glob` and `any`" + )); + return; + } + + if has_glob { + match matcher_obj.get("glob").and_then(|v| v.as_str()) { + None => errors.push(format!("{loc}.glob: expected glob string")), + Some(glob_str) => { + if let Some(warning) = check_glob_syntax(glob_str) { + warnings.push(format!("{loc}.glob: {warning}")); } } - continue; } + return; + } - let Some(any) = matcher_obj.get("any").and_then(|v| v.as_array()) else { - errors.push(format!("{loc}.{key}.any: expected array of glob strings")); - continue; - }; - if any.is_empty() { - errors.push(format!("{loc}.{key}.any: list must not be empty")); - continue; - } - if any.iter().any(|v| v.as_str().is_none()) { - errors.push(format!("{loc}.{key}.any: all values must be strings")); - } - for item in any.iter().filter_map(|v| v.as_str()) { - if let Some(warning) = check_glob_syntax(item) { - warnings.push(format!("{loc}.{key}.any: {warning}")); - } + let Some(any) = matcher_obj.get("any").and_then(|v| v.as_array()) else { + errors.push(format!("{loc}.any: expected array of glob strings")); + return; + }; + if any.is_empty() { + errors.push(format!("{loc}.any: list must not be empty")); + return; + } + if any.iter().any(|v| v.as_str().is_none()) { + errors.push(format!("{loc}.any: all values must be strings")); + } + for item in any.iter().filter_map(|v| v.as_str()) { + if let Some(warning) = check_glob_syntax(item) { + warnings.push(format!("{loc}.any: {warning}")); } } } @@ -2515,6 +2528,43 @@ mod tests { ); } + #[test] + fn validate_jsonrpc_nested_params_matchers_are_accepted() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "mcp.example.com", + "port": 443, + "protocol": "mcp", + "rules": [{ + "allow": { + "rpc_method": "tools/call", + "params": { + "name": "submit_report", + "arguments": { + "scope": "workspace/main", + "repository": { "any": ["NVIDIA/OpenShell", "NVIDIA/*"] } + } + } + } + }] + }], + "binaries": [] + } + } + }); + let (errors, warnings) = validate_l7_policies(&data); + assert!( + errors.is_empty(), + "valid nested params matchers should not error: {errors:?}" + ); + assert!( + warnings.is_empty(), + "valid nested params matchers should not warn: {warnings:?}" + ); + } + // --- Deny rules validation tests --- #[test] diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 9b71f9801..6d30ff9ce 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -3171,6 +3171,9 @@ network_policies: - allow: method: tools/call tool: read_status + params: + arguments: + scope: workspace/main deny_rules: - method: tools/call tool: blocked_action @@ -3184,10 +3187,25 @@ network_policies: 8000, "/mcp", "tools/call", - serde_json::json!({"name": "read_status"}), + serde_json::json!({ + "name": "read_status", + "arguments.scope": "workspace/main" + }), ); assert!(eval_l7(&engine, &read_status)); + let wrong_scope = l7_jsonrpc_input_with_params( + "mcp.params.test", + 8000, + "/mcp", + "tools/call", + serde_json::json!({ + "name": "read_status", + "arguments.scope": "workspace/other" + }), + ); + assert!(!eval_l7(&engine, &wrong_scope)); + let blocked = l7_jsonrpc_input_with_params( "mcp.params.test", 8000, diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 87fb0bdbc..a1e20239b 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -624,7 +624,7 @@ MCP and JSON-RPC endpoint policies currently require full policy YAML applied wi Use `protocol: json-rpc` and `rpc_method` when you need generic JSON-RPC 2.0 matching for a non-MCP server. `json_rpc.max_body_bytes` controls the generic JSON-RPC inspection buffer. -`params` matchers are case-sensitive and use the same string glob or `{ any: [...] }` matcher syntax as REST query parameters. They match scalar leaf values from object params: strings, numbers, and booleans are converted to strings, and nested JSON object params are flattened with dot-separated keys before matching. Arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. This is useful for controls such as matching MCP `tools/call` by `tool` or `params.name`, but it is not a complete MCP payload policy for rich nested content. For batch requests, OpenShell evaluates each JSON-RPC call independently and denies the whole batch if any call is denied. +`params` matchers are case-sensitive and use the same string glob or `{ any: [...] }` matcher syntax as REST query parameters. Write them as nested maps that mirror MCP params. OpenShell flattens the matcher map internally before evaluating scalar leaf values from object params: strings, numbers, and booleans are converted to strings. Dot-separated matcher keys remain accepted for compatibility. Arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. This is useful for controls such as matching MCP `tools/call` by `tool` or `params.name`, but it is not a complete MCP payload policy for rich nested content. For batch requests, OpenShell evaluates each JSON-RPC call independently and denies the whole batch if any call is denied. ### GraphQL matching From 83e53c5e2d5ba8ce1ebc8d32ff6f0cab7edf1406 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:43:55 -0700 Subject: [PATCH 04/12] docs(policy): comment MCP policy internals Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 23 +++++++++++++++ .../src/l7/jsonrpc.rs | 29 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index c7fbfbcf6..b02856470 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -164,6 +164,9 @@ fn json_rpc_config_from_proto(max_body_bytes: u32) -> Option { (max_body_bytes > 0).then_some(JsonRpcConfigDef { max_body_bytes }) } +// MCP rides the same HTTP/JSON-RPC inspection machinery at runtime, but it +// gets its own policy stanza so user-authored YAML can name the primary +// protocol instead of treating MCP as generic JSON-RPC. #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct McpConfigDef { @@ -192,6 +195,8 @@ struct L7RuleDef { allow: L7AllowDef, } +// Preserve the original `rules: [{ allow: ... }]` shape while accepting the +// newer grouped shape (`rules.allow` / `rules.deny`) used by MCP examples. #[derive(Debug, Serialize, Deserialize)] #[serde(untagged)] enum RulesDef { @@ -206,6 +211,8 @@ impl Default for RulesDef { } impl RulesDef { + // Serde needs this for `skip_serializing_if`; keeping it on the enum keeps + // call sites from having to know which rules shape was parsed. fn is_empty(&self) -> bool { match self { Self::Legacy(rules) => rules.is_empty(), @@ -213,6 +220,8 @@ impl RulesDef { } } + // The proto model still carries allow rules and deny rules separately. This + // folds both YAML spellings back into that stable internal representation. fn into_parts(self) -> (Vec, Vec) { match self { Self::Legacy(rules) => (rules, Vec::new()), @@ -267,10 +276,14 @@ struct L7AllowDef { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] enum QueryMatcherDef { + // Short form: `query: { repo: "NVIDIA/*" }`. Glob(String), + // Expanded form: `query: { repo: { any: ["NVIDIA/*", "openai/*"] } }`. Any(QueryAnyDef), } +// JSON-RPC/MCP params can be authored as nested maps in YAML, but the runtime +// matcher map remains flat so the Rego policy can share query-param matching. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] enum ParamMatcherDef { @@ -385,6 +398,8 @@ fn flat_params_to_def( params: BTreeMap, ) -> BTreeMap { let flat = params.into_iter().collect::>(); + // Generic JSON-RPC keeps the flat form because JSON-RPC does not define a + // conventional params object shape. MCP uses nested YAML for readability. if !is_mcp_protocol(protocol) { return flat_param_matchers_to_def(flat); } @@ -433,6 +448,8 @@ fn insert_nested_param( insert_nested_param(children, &remainder, matcher) } +// `mcp_method` is a YAML alias for `rpc_method`; both compile to the existing +// proto field so older runtime policy evaluation stays compatible. fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { if mcp_method.is_empty() { rpc_method @@ -441,6 +458,8 @@ fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { } } +// MCP `tool` is a policy convenience for the standard `tools/call` params.name +// field. It only fills the matcher when the caller did not set `params.name`. fn params_with_tool( mut params: BTreeMap, tool: String, @@ -496,6 +515,8 @@ fn deny_def_to_proto(deny: L7DenyRuleDef) -> L7DenyRule { } fn json_rpc_max_body_bytes(json_rpc: &Option, mcp: &Option) -> u32 { + // The proto has one JSON-RPC-family body limit. Prefer the MCP stanza when + // present because MCP policies should not need a shadow `json_rpc` block. mcp.as_ref().map_or_else( || json_rpc.as_ref().map_or(0, |config| config.max_body_bytes), |config| config.max_body_bytes, @@ -510,6 +531,8 @@ fn split_tool_param( protocol: &str, params: BTreeMap, ) -> (String, BTreeMap) { + // Only MCP has the tool-name convention. Generic JSON-RPC keeps `name` as a + // normal params matcher so serialization does not invent MCP semantics. if !is_mcp_protocol(protocol) { return (String::new(), params); } diff --git a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs index b197471a1..62327eee6 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -16,12 +16,16 @@ use crate::l7::provider::{L7Provider, L7Request}; pub const DEFAULT_MAX_BODY_BYTES: usize = 64 * 1024; +/// Selects whether the parser should treat a JSON-RPC message as generic +/// JSON-RPC 2.0 or as an MCP message with MCP method/params validation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum JsonRpcInspectionMode { JsonRpc, Mcp, } +/// Parsed HTTP request plus the JSON-RPC-family metadata extracted from the +/// body. The original HTTP request is still forwarded if policy allows it. pub struct JsonRpcHttpRequest { pub request: L7Request, pub info: JsonRpcRequestInfo, @@ -51,6 +55,8 @@ pub(crate) async fn parse_jsonrpc_http_request, pub is_batch: bool, pub receive_stream: bool, @@ -60,12 +66,20 @@ pub struct JsonRpcRequestInfo { #[derive(Debug, Clone, PartialEq, Eq)] pub struct JsonRpcCallInfo { + /// JSON-RPC method, or the MCP method name after typed MCP parsing. pub method: String, + /// Flattened scalar params used by the current Rego matcher path. Strings, + /// numbers, and booleans are represented as strings for compatibility with + /// the existing query matcher implementation. pub params: HashMap, + /// MCP `tools/call` tool name when known. Generic JSON-RPC leaves this as + /// a best-effort projection of `params.name`. pub tool: Option, } impl JsonRpcRequestInfo { + /// MCP streamable HTTP uses an empty GET to receive server messages. It has + /// no request body to inspect, but it must still pass through MCP endpoints. pub(crate) fn receive_stream() -> Self { Self { calls: Vec::new(), @@ -76,6 +90,8 @@ impl JsonRpcRequestInfo { } } + /// Logs store only a digest of params. For batches, hash the per-call + /// canonical maps so denied-call logging cannot leak raw argument values. pub(crate) fn params_sha256(&self) -> Option { if self.is_batch { if self.calls.is_empty() || self.calls.iter().all(|call| call.params.is_empty()) { @@ -134,6 +150,8 @@ pub fn parse_jsonrpc_body(body: &[u8]) -> JsonRpcRequestInfo { parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::JsonRpc) } +/// Parse a JSON-RPC body as MCP, using tower-mcp-types for known MCP request +/// and notification shapes while still allowing extension methods. pub fn parse_mcp_body(body: &[u8]) -> JsonRpcRequestInfo { parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::Mcp) } @@ -218,6 +236,8 @@ enum JsonRpcMessageInfo { Response, } +// Shared framing for JSON-RPC-family messages. MCP-specific validation starts +// only after the common JSON-RPC version/method/response checks. fn parse_jsonrpc_message( value: &serde_json::Value, inspection_mode: JsonRpcInspectionMode, @@ -252,6 +272,8 @@ fn parse_jsonrpc_call( value: &serde_json::Value, inspection_mode: JsonRpcInspectionMode, ) -> std::result::Result { + // MCP mode delegates method-specific validation to tower-mcp-types. The + // generic mode intentionally remains looser for non-MCP JSON-RPC servers. if inspection_mode == JsonRpcInspectionMode::Mcp { return parse_mcp_call(value); } @@ -300,6 +322,9 @@ fn parse_jsonrpc_response(value: &serde_json::Value) -> std::result::Result<(), fn parse_mcp_call(value: &serde_json::Value) -> std::result::Result { if value.get("id").is_some() { + // Requests can be converted into typed MCP variants, which gives us + // method names and tool-call params without maintaining local copies of + // the MCP request schema. let request: JsonRpcRequest = serde_json::from_value(value.clone()) .map_err(|error| format!("invalid MCP request: {error}"))?; request @@ -314,6 +339,8 @@ fn parse_mcp_call(value: &serde_json::Value) -> std::result::Result, ) -> std::result::Result<(), String> { + // Keep the runtime input flat for the existing OPA matcher, while rejecting + // literal dotted keys that would collide with nested object paths. match value { serde_json::Value::Object(map) => { for (key, child) in map { From 431f8e1a4d9f31a5b0f8af4e23f7a0e6a5972b58 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:56:37 -0700 Subject: [PATCH 05/12] fix(policy): satisfy MCP clippy checks Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 2 +- .../src/l7/jsonrpc.rs | 36 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index b02856470..121326bbc 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -288,7 +288,7 @@ enum QueryMatcherDef { #[serde(untagged)] enum ParamMatcherDef { Matcher(QueryMatcherDef), - Object(BTreeMap), + Object(BTreeMap), } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs index 62327eee6..4032e1013 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -338,26 +338,26 @@ fn parse_mcp_call(value: &serde_json::Value) -> std::result::Result Date: Tue, 16 Jun 2026 15:24:02 -0700 Subject: [PATCH 06/12] test(mcp): cover relay deny by tool Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- .../src/l7/relay.rs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 9bc23d946..73e9b4e32 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2436,6 +2436,82 @@ network_policies: assert!(!message.contains("purge_cache")); } + #[test] + fn mcp_tool_deny_rule_blocks_tools_call() { + let data = r#" +network_policies: + mcp_api: + name: mcp_api + endpoints: + - host: api.example.test + port: 443 + path: "/mcp" + protocol: mcp + enforcement: enforce + mcp: + max_body_bytes: 131072 + rules: + deny: + - mcp_method: tools/call + tool: delete_resource + allow: + - mcp_method: initialize + - mcp_method: tools/list + - mcp_method: tools/call + tool: read_status + binaries: + - { path: /usr/bin/node } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap(); + let tunnel_engine = engine + .clone_engine_for_tunnel(engine.current_generation()) + .unwrap(); + let ctx = L7EvalContext { + host: "api.example.test".into(), + port: 443, + policy_name: "mcp_api".into(), + binary_path: "/usr/bin/node".into(), + ancestors: vec![], + cmdline_paths: vec![], + secret_resolver: None, + activity_tx: None, + dynamic_credentials: None, + token_grant_resolver: None, + }; + let mut request = L7RequestInfo { + action: "POST".into(), + target: "/mcp".into(), + query_params: std::collections::HashMap::new(), + graphql: None, + jsonrpc: Some(crate::l7::jsonrpc::parse_mcp_body( + br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"read_status","arguments":{}}}"#, + )), + }; + + let (allowed, reason) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); + assert!(allowed, "{reason}"); + + request.jsonrpc = Some(crate::l7::jsonrpc::parse_mcp_body( + br#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"delete_resource","arguments":{"scope":"workspace/main"}}}"#, + )); + let parsed = request.jsonrpc.as_ref().expect("parsed MCP request"); + assert!( + parsed.error.is_none(), + "MCP request should parse: {parsed:?}" + ); + assert_eq!( + parsed.calls.first().and_then(|call| call.tool.as_deref()), + Some("delete_resource") + ); + + let (allowed, reason) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); + assert!(!allowed, "delete_resource must match the MCP deny rule"); + assert!( + reason.contains("deny rule"), + "deny reason should identify policy denial: {reason}" + ); + } + #[test] fn jsonrpc_log_records_digest_not_args() { let info = crate::l7::jsonrpc::parse_jsonrpc_body( From abee95900b20630bfb7ba0cfe602a6795c749d7f Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 16 Jun 2026 16:00:41 -0700 Subject: [PATCH 07/12] fix(mcp): permit streamable http control traffic Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- .../data/sandbox-policy.rego | 31 +++++--- .../src/l7/relay.rs | 54 +++++++------ .../openshell-supervisor-network/src/opa.rs | 78 +++++++++++++++++-- e2e/mcp-conformance/expected-failures.yml | 22 +++++- 4 files changed, 139 insertions(+), 46 deletions(-) diff --git a/crates/openshell-supervisor-network/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index 54b48388c..f23ef7d7c 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -257,7 +257,7 @@ deny_request if { # --- L7 deny rule matching: REST method + path + query --- request_denied_for_endpoint(request, endpoint) if { - object.get(endpoint, "protocol", "") != "json-rpc" + not jsonrpc_family_endpoint(endpoint) some deny_rule deny_rule := endpoint.deny_rules[_] deny_rule.method @@ -278,16 +278,16 @@ request_denied_for_endpoint(request, endpoint) if { # --- L7 deny rule matching: JSON-RPC method + params --- request_denied_for_endpoint(request, endpoint) if { - endpoint.protocol == "json-rpc" + jsonrpc_family_endpoint(endpoint) request.method == "POST" some deny_rule deny_rule := endpoint.deny_rules[_] - deny_rule.method + deny_rule.rpc_method jsonrpc_rule_matches(request, deny_rule) } request_denied_for_endpoint(request, endpoint) if { - endpoint.protocol == "json-rpc" + jsonrpc_family_endpoint(endpoint) request.method == "POST" jsonrpc_response_frame_present(request) } @@ -426,7 +426,7 @@ request_deny_reason := reason if { # --- L7 rule matching: REST method + path --- request_allowed_for_endpoint(request, endpoint) if { - object.get(endpoint, "protocol", "") != "json-rpc" + not jsonrpc_family_endpoint(endpoint) some rule rule := endpoint.rules[_] rule.allow.method @@ -447,20 +447,29 @@ request_allowed_for_endpoint(request, endpoint) if { # --- L7 rule matching: JSON-RPC method --- request_allowed_for_endpoint(request, endpoint) if { - endpoint.protocol == "json-rpc" + jsonrpc_family_endpoint(endpoint) request.method == "POST" some rule rule := endpoint.rules[_] - rule.allow.method + rule.allow.rpc_method not jsonrpc_response_frame_present(request) jsonrpc_rule_matches(request, rule.allow) } -# MCP Streamable HTTP uses GET on the JSON-RPC endpoint as a receive stream for -# server-to-client messages. The stream itself has no client-to-server JSON-RPC -# request body to inspect; allow it once the endpoint path and binary matched. -request_allowed_for_endpoint(request, endpoint) if { +jsonrpc_family_endpoint(endpoint) if { endpoint.protocol == "json-rpc" +} + +jsonrpc_family_endpoint(endpoint) if { + endpoint.protocol == "mcp" +} + +# MCP Streamable HTTP uses GET on the JSON-RPC-family endpoint as a receive +# stream for server-to-client messages. The stream itself has no +# client-to-server JSON-RPC request body to inspect; allow it once the endpoint +# path and binary matched. +request_allowed_for_endpoint(request, endpoint) if { + jsonrpc_family_endpoint(endpoint) request.method == "GET" is_object(request.jsonrpc) object.get(request.jsonrpc, "receive_stream", false) diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 73e9b4e32..2e6c85f91 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -313,31 +313,37 @@ where None }; let jsonrpc_info = if config.protocol.is_jsonrpc_family() { - match crate::l7::http::read_body_for_inspection( - client, - &mut req, - config.json_rpc_max_body_bytes, - ) - .await - { - Ok(body) => Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( - &body, - jsonrpc_inspection_mode(config.protocol), - )), - Err(e) => { - if is_benign_connection_error(&e) { - debug!( - host = %ctx.host, - port = ctx.port, - error = %e, - "JSON-RPC L7 connection closed" - ); - } else { - let detail = - parse_rejection_detail(&e.to_string(), ParseRejectionMode::L7Endpoint); - emit_parse_rejection(ctx, &detail, "l7-jsonrpc"); + if crate::l7::jsonrpc::jsonrpc_receive_stream_request(&req) { + Some(crate::l7::jsonrpc::JsonRpcRequestInfo::receive_stream()) + } else { + match crate::l7::http::read_body_for_inspection( + client, + &mut req, + config.json_rpc_max_body_bytes, + ) + .await + { + Ok(body) => Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( + &body, + jsonrpc_inspection_mode(config.protocol), + )), + Err(e) => { + if is_benign_connection_error(&e) { + debug!( + host = %ctx.host, + port = ctx.port, + error = %e, + "JSON-RPC L7 connection closed" + ); + } else { + let detail = parse_rejection_detail( + &e.to_string(), + ParseRejectionMode::L7Endpoint, + ); + emit_parse_rejection(ctx, &detail, "l7-jsonrpc"); + } + return Ok(()); } - return Ok(()); } } } else { diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 6d30ff9ce..9e9c92de0 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -2893,7 +2893,7 @@ network_policies: enforcement: enforce rules: - allow: - method: initialize + rpc_method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -2961,6 +2961,68 @@ network_policies: assert!(!eval_l7(&engine, &bodyless_get_without_receive_stream)); } + #[test] + fn l7_mcp_receive_stream_get_is_allowed_for_matching_endpoint() { + let data = r#" +network_policies: + mcp_stream: + name: mcp_stream + endpoints: + - host: mcp.stream.test + port: 8000 + path: /mcp + protocol: mcp + enforcement: enforce + rules: + - allow: + method: initialize + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml"); + let allow_input = serde_json::json!({ + "network": { "host": "mcp.stream.test", "port": 8000 }, + "exec": { + "path": "/usr/bin/curl", + "ancestors": [], + "cmdline_paths": [] + }, + "request": { + "method": "GET", + "path": "/mcp", + "query_params": {}, + "jsonrpc": { + "method": null, + "params": {}, + "receive_stream": true, + "error": null + } + } + }); + assert!(eval_l7(&engine, &allow_input)); + + let deny_input = serde_json::json!({ + "network": { "host": "mcp.stream.test", "port": 8000 }, + "exec": { + "path": "/usr/bin/curl", + "ancestors": [], + "cmdline_paths": [] + }, + "request": { + "method": "GET", + "path": "/other", + "query_params": {}, + "jsonrpc": { + "method": null, + "params": {}, + "receive_stream": true, + "error": null + } + } + }); + assert!(!eval_l7(&engine, &deny_input)); + } + #[test] fn l7_jsonrpc_response_post_is_denied_for_matching_endpoint() { let data = r#" @@ -2975,7 +3037,7 @@ network_policies: enforcement: enforce rules: - allow: - method: initialize + rpc_method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3011,7 +3073,7 @@ network_policies: enforcement: enforce rules: - allow: - method: initialize + rpc_method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3044,9 +3106,9 @@ network_policies: enforcement: enforce rules: - allow: - method: initialize + rpc_method: initialize deny_rules: - - method: tools/delete + - rpc_method: tools/delete binaries: - { path: /usr/bin/curl } "#; @@ -3089,16 +3151,16 @@ network_policies: enforcement: enforce rules: - allow: - method: tools/call + rpc_method: tools/call params: name: read_status - allow: - method: tools/call + rpc_method: tools/call params: name: submit_report arguments.scope: workspace/main deny_rules: - - method: tools/call + - rpc_method: tools/call params: name: blocked_action binaries: diff --git a/e2e/mcp-conformance/expected-failures.yml b/e2e/mcp-conformance/expected-failures.yml index 731faad69..58a4012ff 100644 --- a/e2e/mcp-conformance/expected-failures.yml +++ b/e2e/mcp-conformance/expected-failures.yml @@ -1,7 +1,23 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -# Add client scenarios here when enabling broader MCP conformance suites that -# exercise features OpenShell does not yet support through the MCP proxy. -client: [] +# Add client scenarios here when enabling broader MCP conformance suites. +# +# elicitation-sep1034-client-defaults is allowed because the pinned upstream +# TypeScript conformance client appears to advertise the wrong SDK capability +# shape for default application: it sets elicitation.applyDefaults, while +# @modelcontextprotocol/sdk 1.29.0 applies defaults only when +# elicitation.form.applyDefaults is true. Reproduce without OpenShell in the +# path from .cache/mcp-conformance after npm install/build: +# Remove this entry when OPENSHELL_MCP_CONFORMANCE_REF is bumped to an upstream +# conformance commit where the bundled client advertises +# elicitation.form.applyDefaults. +# +# node dist/index.js client \ +# --command "env MCP_CONFORMANCE_SCENARIO=elicitation-defaults \ +# ./node_modules/.bin/tsx examples/clients/typescript/everything-client.ts" \ +# --scenario elicitation-sep1034-client-defaults \ +# --spec-version 2025-11-25 +client: + - elicitation-sep1034-client-defaults server: [] From f70347cf108e8e0c13d4a0c4c7151495b84394f8 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:46:53 -0700 Subject: [PATCH 08/12] fix(mcp): align conformance fixture workarounds Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- e2e/mcp-conformance.sh | 95 ++++++++++++++++--- e2e/mcp-conformance/README.md | 14 ++- .../client-through-openshell.sh | 13 +-- e2e/mcp-conformance/expected-failures.yml | 24 +---- 4 files changed, 100 insertions(+), 46 deletions(-) diff --git a/e2e/mcp-conformance.sh b/e2e/mcp-conformance.sh index 8224b3502..409b5b9f8 100755 --- a/e2e/mcp-conformance.sh +++ b/e2e/mcp-conformance.sh @@ -8,10 +8,10 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" # shellcheck source=e2e/support/gateway-common.sh disable=SC1091 source "${ROOT}/e2e/support/gateway-common.sh" CONFORMANCE_DIR="${OPENSHELL_MCP_CONFORMANCE_DIR:-${ROOT}/.cache/mcp-conformance}" -# Pinned after v0.1.16 because that tag has an upstream scenario-name mismatch: -# the runner exposes `tools_call`, while the bundled client only accepts -# `tools-call`. This commit registers both names in the client and keeps the -# runner's canonical `tools_call` scenario name. +# Pinned after v0.1.16 for the upstream tools_call fixture fix. The current +# checkout still needs temporary client-fixture patches for +# modelcontextprotocol/conformance#345; remove patch_conformance_clients when +# OPENSHELL_MCP_CONFORMANCE_REF points at a release containing those fixes. CONFORMANCE_REF="${OPENSHELL_MCP_CONFORMANCE_REF:-b9041ea41b0188581803459dbae71bc7e02fd995}" CLIENT_IMAGE="${OPENSHELL_MCP_CONFORMANCE_CLIENT_IMAGE:-openshell-mcp-conformance-client:local}" SCENARIOS="${OPENSHELL_MCP_CONFORMANCE_SCENARIOS:-}" @@ -22,6 +22,7 @@ DOCKER_PULL="${OPENSHELL_MCP_CONFORMANCE_DOCKER_PULL:-0}" CLIENT_IMAGE_REF_LABEL="org.openshell.mcp-conformance.ref" CLIENT_IMAGE_DOCKERFILE_LABEL="org.openshell.mcp-conformance.dockerfile" CLIENT_IMAGE_DOCKERIGNORE_LABEL="org.openshell.mcp-conformance.dockerignore" +CLIENT_IMAGE_FIXTURE_HASH_LABEL="org.openshell.mcp-conformance.fixture-hash" RUN_SCENARIOS_COMMAND="__openshell_mcp_run_scenarios" CLIENT_SANDBOX_MANAGED=0 HOST_BRIDGE_PID="" @@ -109,6 +110,67 @@ openshell_bin() { printf '%s\n' "${target_dir}/debug/openshell" } +patch_conformance_clients() { + node - "${CONFORMANCE_DIR}" <<'NODE' +const fs = require('node:fs'); +const path = require('node:path'); + +const root = process.argv[2]; + +function rewrite(file, rewriter) { + const target = path.join(root, file); + const source = fs.readFileSync(target, 'utf8'); + const next = rewriter(source, file); + + if (next !== source) { + fs.writeFileSync(target, next); + console.error(`Patched upstream MCP conformance fixture: ${file}`); + } +} + +function patchApplyDefaults(source, file) { + if (/elicitation:\s*{\s*form:\s*{\s*applyDefaults:\s*true\s*}\s*}/m.test(source)) { + return source; + } + + const broken = /elicitation:\s*{\s*applyDefaults:\s*true\s*}/m; + if (!broken.test(source)) { + throw new Error(`${file}: could not find the known elicitation defaults fixture`); + } + + return source.replace( + broken, + `elicitation: { + form: { + applyDefaults: true + } + }` + ); +} + +rewrite('examples/clients/typescript/everything-client.ts', (source, file) => { + let next = patchApplyDefaults(source, file); + if (next.includes('elicitation-sep1034-client-defaults')) { + return next; + } + + const oldRegistration = "registerScenario('elicitation-defaults', runElicitationDefaultsClient);"; + const newRegistration = `registerScenarios( + ['elicitation-defaults', 'elicitation-sep1034-client-defaults'], + runElicitationDefaultsClient +);`; + + if (!next.includes(oldRegistration)) { + throw new Error(`${file}: could not find the known elicitation scenario registration`); + } + + return next.replace(oldRegistration, newRegistration); +}); + +rewrite('examples/clients/typescript/elicitation-defaults-test.ts', patchApplyDefaults); +NODE +} + start_host_bridge() { local port=$1 local openshell runner_ip @@ -210,21 +272,29 @@ stop_host_bridge() { } build_client_image() { - local conformance_head dockerfile_hash dockerignore_hash - local image_ref image_dockerfile image_dockerignore + local conformance_head dockerfile_hash dockerignore_hash fixture_hash + local image_ref image_dockerfile image_dockerignore image_fixture_hash local -a pull_args=() conformance_head="$(git -C "${CONFORMANCE_DIR}" rev-parse HEAD)" dockerfile_hash="$(git -C "${ROOT}" hash-object "${ROOT}/e2e/mcp-conformance/Dockerfile.client")" dockerignore_hash="$(git -C "${ROOT}" hash-object "${ROOT}/e2e/mcp-conformance/Dockerfile.client.dockerignore")" + fixture_hash="$( + git -C "${CONFORMANCE_DIR}" diff -- \ + examples/clients/typescript/everything-client.ts \ + examples/clients/typescript/elicitation-defaults-test.ts | + git hash-object --stdin + )" image_ref="$(docker_image_label "${CLIENT_IMAGE}" "${CLIENT_IMAGE_REF_LABEL}")" image_dockerfile="$(docker_image_label "${CLIENT_IMAGE}" "${CLIENT_IMAGE_DOCKERFILE_LABEL}")" image_dockerignore="$(docker_image_label "${CLIENT_IMAGE}" "${CLIENT_IMAGE_DOCKERIGNORE_LABEL}")" + image_fixture_hash="$(docker_image_label "${CLIENT_IMAGE}" "${CLIENT_IMAGE_FIXTURE_HASH_LABEL}")" if [ "${FORCE_REBUILD}" != "1" ] \ && [ "${image_ref}" = "${conformance_head}" ] \ && [ "${image_dockerfile}" = "${dockerfile_hash}" ] \ - && [ "${image_dockerignore}" = "${dockerignore_hash}" ]; then + && [ "${image_dockerignore}" = "${dockerignore_hash}" ] \ + && [ "${image_fixture_hash}" = "${fixture_hash}" ]; then echo "Using cached MCP conformance client image ${CLIENT_IMAGE} (${conformance_head})." >&2 return fi @@ -237,6 +307,7 @@ build_client_image() { --label "${CLIENT_IMAGE_REF_LABEL}=${conformance_head}" \ --label "${CLIENT_IMAGE_DOCKERFILE_LABEL}=${dockerfile_hash}" \ --label "${CLIENT_IMAGE_DOCKERIGNORE_LABEL}=${dockerignore_hash}" \ + --label "${CLIENT_IMAGE_FIXTURE_HASH_LABEL}=${fixture_hash}" \ -f "${ROOT}/e2e/mcp-conformance/Dockerfile.client" \ -t "${CLIENT_IMAGE}" \ "${CONFORMANCE_DIR}" @@ -429,16 +500,18 @@ main() { return fi - # git fetches and pins the upstream conformance repo (and hashes it for image - # caching); docker builds and runs the runner/client containers; python3 runs - # the host bridge and renders policies. The conformance runner itself (node, - # npm, tsx) now runs inside the container image, not on the host. + # git fetches and pins the upstream conformance repo; node patches temporary + # fixture drift before the image build; docker builds and runs the + # runner/client containers; python3 runs the host bridge and renders policies. + # npm and tsx run inside the container image, not on the host. require_command git + require_command node require_command docker require_command python3 echo "MCP conformance spec version: ${SPEC_VERSION}" >&2 checkout_conformance + patch_conformance_clients build_client_image run_scenarios_under_gateway "$@" } diff --git a/e2e/mcp-conformance/README.md b/e2e/mcp-conformance/README.md index 54f808797..9f9f4a45d 100644 --- a/e2e/mcp-conformance/README.md +++ b/e2e/mcp-conformance/README.md @@ -45,10 +45,16 @@ For local runs, build or stage a static supervisor binary and pass it with `OPENSHELL_DOCKER_SUPERVISOR_BIN` if the default local supervisor build is linked against a newer glibc than the conformance client image provides. -The upstream `everything-client` has a few handler names that do not line up -with released-spec scenario names. The wrapper maps those names when forwarding -`MCP_CONFORMANCE_SCENARIO` into the sandbox, but it does not patch the upstream -checkout. +The pinned upstream checkout includes reference-client fixture drift that is +tracked in `modelcontextprotocol/conformance#345`. The wrapper patches the +checkout before building the client image so the bundled TypeScript client +advertises `elicitation.form.applyDefaults` and accepts the canonical +`elicitation-sep1034-client-defaults` scenario. It also routes `sse-retry` to +the upstream standalone `sse-retry-test.ts` client so the reconnect timing path +is exercised instead of aliasing it to another scenario. + +Remove those local workarounds when `OPENSHELL_MCP_CONFORMANCE_REF` points at +an upstream release that includes the `#345` fixes. When enabling broader upstream suites, add scenarios that OpenShell does not yet support through the MCP proxy to `expected-failures.yml`. The upstream diff --git a/e2e/mcp-conformance/client-through-openshell.sh b/e2e/mcp-conformance/client-through-openshell.sh index 4cb6a3465..5dfadb279 100755 --- a/e2e/mcp-conformance/client-through-openshell.sh +++ b/e2e/mcp-conformance/client-through-openshell.sh @@ -60,24 +60,13 @@ trap 'rm -f "${POLICY_FILE}"' EXIT CLIENT_SERVER_URL="$(python3 "${ROOT}/e2e/mcp-conformance/render-policy.py" "${SERVER_URL}" "${POLICY_FILE}" "${POLICY_TEMPLATE}")" ENV_ARGS=() -CLIENT_SCENARIO="${MCP_CONFORMANCE_SCENARIO:-}" -case "${CLIENT_SCENARIO}" in - elicitation-sep1034-client-defaults) - CLIENT_SCENARIO="elicitation-defaults" - ;; - sse-retry) - CLIENT_SCENARIO="tools_call" - ;; -esac # These environment variables are set by the upstream conformance test runner # before it invokes the configured client command. Forward them into the # sandbox because the sandboxed TypeScript client depends on them to select the # scenario and read scenario-specific context. for NAME in MCP_CONFORMANCE_SCENARIO MCP_CONFORMANCE_CONTEXT MCP_CONFORMANCE_PROTOCOL_VERSION; do - if [ "${NAME}" = "MCP_CONFORMANCE_SCENARIO" ] && [ -n "${CLIENT_SCENARIO}" ]; then - ENV_ARGS+=(--env "MCP_CONFORMANCE_SCENARIO=${CLIENT_SCENARIO}") - elif [ -n "${!NAME+x}" ]; then + if [ -n "${!NAME+x}" ]; then ENV_ARGS+=(--env "${NAME}=${!NAME}") fi done diff --git a/e2e/mcp-conformance/expected-failures.yml b/e2e/mcp-conformance/expected-failures.yml index 58a4012ff..05c6f8afd 100644 --- a/e2e/mcp-conformance/expected-failures.yml +++ b/e2e/mcp-conformance/expected-failures.yml @@ -1,23 +1,9 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -# Add client scenarios here when enabling broader MCP conformance suites. -# -# elicitation-sep1034-client-defaults is allowed because the pinned upstream -# TypeScript conformance client appears to advertise the wrong SDK capability -# shape for default application: it sets elicitation.applyDefaults, while -# @modelcontextprotocol/sdk 1.29.0 applies defaults only when -# elicitation.form.applyDefaults is true. Reproduce without OpenShell in the -# path from .cache/mcp-conformance after npm install/build: -# Remove this entry when OPENSHELL_MCP_CONFORMANCE_REF is bumped to an upstream -# conformance commit where the bundled client advertises -# elicitation.form.applyDefaults. -# -# node dist/index.js client \ -# --command "env MCP_CONFORMANCE_SCENARIO=elicitation-defaults \ -# ./node_modules/.bin/tsx examples/clients/typescript/everything-client.ts" \ -# --scenario elicitation-sep1034-client-defaults \ -# --spec-version 2025-11-25 -client: - - elicitation-sep1034-client-defaults +# Add scenarios here only for known OpenShell MCP conformance gaps. +# Upstream reference-client fixture drift is handled by the local wrapper and +# checkout patching in e2e/mcp-conformance.sh so it does not hide OpenShell +# regressions behind expected failures. +client: [] server: [] From 6a15197cc4c3ac9551e6009a52e5e1e92209924a Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Wed, 17 Jun 2026 23:24:16 -0700 Subject: [PATCH 09/12] refactor(policy): align mcp method syntax Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 92 ++++++++++--------- .../src/l7/relay.rs | 8 +- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 121326bbc..bb6662a0e 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -266,8 +266,6 @@ struct L7AllowDef { #[serde(default, skip_serializing_if = "String::is_empty")] rpc_method: String, #[serde(default, skip_serializing_if = "String::is_empty")] - mcp_method: String, - #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, @@ -318,8 +316,6 @@ struct L7DenyRuleDef { #[serde(default, skip_serializing_if = "String::is_empty")] rpc_method: String, #[serde(default, skip_serializing_if = "String::is_empty")] - mcp_method: String, - #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, @@ -448,16 +444,6 @@ fn insert_nested_param( insert_nested_param(children, &remainder, matcher) } -// `mcp_method` is a YAML alias for `rpc_method`; both compile to the existing -// proto field so older runtime policy evaluation stays compatible. -fn method_from_aliases(rpc_method: String, mcp_method: String) -> String { - if mcp_method.is_empty() { - rpc_method - } else { - mcp_method - } -} - // MCP `tool` is a policy convenience for the standard `tools/call` params.name // field. It only fills the matcher when the caller did not set `params.name`. fn params_with_tool( @@ -472,15 +458,26 @@ fn params_with_tool( params } -fn allow_def_to_proto(allow: L7AllowDef) -> L7Allow { +fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { + let (method, rpc_method) = if is_mcp_protocol(protocol) { + let rpc_method = if allow.method.is_empty() { + allow.rpc_method + } else { + allow.method + }; + (String::new(), rpc_method) + } else { + (allow.method, allow.rpc_method) + }; + L7Allow { - method: allow.method, + method, path: allow.path, command: allow.command, operation_type: allow.operation_type, operation_name: allow.operation_name, fields: allow.fields, - rpc_method: method_from_aliases(allow.rpc_method, allow.mcp_method), + rpc_method, query: allow .query .into_iter() @@ -493,15 +490,26 @@ fn allow_def_to_proto(allow: L7AllowDef) -> L7Allow { } } -fn deny_def_to_proto(deny: L7DenyRuleDef) -> L7DenyRule { +fn deny_def_to_proto(protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { + let (method, rpc_method) = if is_mcp_protocol(protocol) { + let rpc_method = if deny.method.is_empty() { + deny.rpc_method + } else { + deny.method + }; + (String::new(), rpc_method) + } else { + (deny.method, deny.rpc_method) + }; + L7DenyRule { - method: deny.method, + method, path: deny.path, command: deny.command, operation_type: deny.operation_type, operation_name: deny.operation_name, fields: deny.fields, - rpc_method: method_from_aliases(deny.rpc_method, deny.mcp_method), + rpc_method, query: deny .query .into_iter() @@ -557,13 +565,13 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { .collect(); let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); - let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { - (String::new(), allow.rpc_method) - } else { + let (method, rpc_method) = if is_mcp_protocol(protocol) { (allow.rpc_method, String::new()) + } else { + (allow.method, allow.rpc_method) }; L7AllowDef { - method: allow.method, + method, path: allow.path, command: allow.command, query: allow @@ -575,7 +583,6 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { operation_name: allow.operation_name, fields: allow.fields, rpc_method, - mcp_method, tool, params, } @@ -589,13 +596,13 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { .collect(); let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); - let (rpc_method, mcp_method) = if is_mcp_protocol(protocol) { - (String::new(), deny.rpc_method.clone()) - } else { + let (method, rpc_method) = if is_mcp_protocol(protocol) { (deny.rpc_method.clone(), String::new()) + } else { + (deny.method.clone(), deny.rpc_method.clone()) }; L7DenyRuleDef { - method: deny.method.clone(), + method, path: deny.path.clone(), command: deny.command.clone(), query: deny @@ -607,7 +614,6 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { operation_name: deny.operation_name.clone(), fields: deny.fields.clone(), rpc_method, - mcp_method, tool, params, } @@ -628,6 +634,7 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { .endpoints .into_iter() .map(|e| { + let protocol = e.protocol; let (allow_rules, grouped_deny_rules) = e.rules.into_parts(); let mut deny_rules = grouped_deny_rules; deny_rules.extend(e.deny_rules); @@ -645,18 +652,21 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { path: e.path, port: normalized_ports.first().copied().unwrap_or(0), ports: normalized_ports, - protocol: e.protocol, + protocol: protocol.clone(), tls: e.tls, enforcement: e.enforcement, access: e.access, rules: allow_rules .into_iter() .map(|r| L7Rule { - allow: Some(allow_def_to_proto(r.allow)), + allow: Some(allow_def_to_proto(&protocol, r.allow)), }) .collect(), allowed_ips: e.allowed_ips, - deny_rules: deny_rules.into_iter().map(deny_def_to_proto).collect(), + deny_rules: deny_rules + .into_iter() + .map(|deny| deny_def_to_proto(&protocol, deny)) + .collect(), allow_encoded_slash: e.allow_encoded_slash, websocket_credential_rewrite: e.websocket_credential_rewrite, request_body_credential_rewrite: e.request_body_credential_rewrite, @@ -2048,7 +2058,7 @@ network_policies: } #[test] - fn parse_grouped_mcp_rules_to_jsonrpc_runtime_fields() { + fn parse_grouped_mcp_rules_to_runtime_fields() { let yaml = r" version: 1 network_policies: @@ -2064,12 +2074,12 @@ network_policies: max_body_bytes: 131072 rules: deny: - - mcp_method: tools/call + - method: tools/call tool: send_email allow: - - mcp_method: initialize - - mcp_method: tools/list - - mcp_method: tools/call + - method: initialize + - method: tools/list + - method: tools/call tool: search_web params: arguments: @@ -2112,13 +2122,13 @@ network_policies: max_body_bytes: 131072 rules: allow: - - mcp_method: tools/call + - method: tools/call tool: search_web params: arguments: repository: NVIDIA/OpenShell deny: - - mcp_method: tools/call + - method: tools/call tool: send_email binaries: - path: /usr/bin/curl @@ -2128,7 +2138,7 @@ network_policies: let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); assert!(yaml_out.contains("protocol: mcp")); - assert!(yaml_out.contains("mcp_method: tools/call")); + assert!(yaml_out.contains("method: tools/call")); assert!(yaml_out.contains("tool: search_web")); assert!(yaml_out.contains("tool: send_email")); assert!(yaml_out.contains("arguments:")); diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 2e6c85f91..039d60988 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2458,12 +2458,12 @@ network_policies: max_body_bytes: 131072 rules: deny: - - mcp_method: tools/call + - method: tools/call tool: delete_resource allow: - - mcp_method: initialize - - mcp_method: tools/list - - mcp_method: tools/call + - method: initialize + - method: tools/list + - method: tools/call tool: read_status binaries: - { path: /usr/bin/node } From f6d11bc8b35f19f4952ef28c0bb71f7f87fafa14 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Thu, 18 Jun 2026 00:10:02 -0700 Subject: [PATCH 10/12] refactor(policy): align mcp deny rules Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 165 ++++++------------ crates/openshell-policy/src/merge.rs | 3 +- .../src/l7/mod.rs | 1 - .../src/l7/relay.rs | 19 +- .../openshell-supervisor-network/src/opa.rs | 15 +- docs/reference/policy-schema.mdx | 4 +- 6 files changed, 76 insertions(+), 131 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index bb6662a0e..d3a713aba 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -108,8 +108,8 @@ struct NetworkEndpointDef { enforcement: String, #[serde(default, skip_serializing_if = "String::is_empty")] access: String, - #[serde(default, skip_serializing_if = "RulesDef::is_empty")] - rules: RulesDef, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + rules: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] allowed_ips: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -195,57 +195,6 @@ struct L7RuleDef { allow: L7AllowDef, } -// Preserve the original `rules: [{ allow: ... }]` shape while accepting the -// newer grouped shape (`rules.allow` / `rules.deny`) used by MCP examples. -#[derive(Debug, Serialize, Deserialize)] -#[serde(untagged)] -enum RulesDef { - Legacy(Vec), - Grouped(L7RuleGroupsDef), -} - -impl Default for RulesDef { - fn default() -> Self { - Self::Legacy(Vec::new()) - } -} - -impl RulesDef { - // Serde needs this for `skip_serializing_if`; keeping it on the enum keeps - // call sites from having to know which rules shape was parsed. - fn is_empty(&self) -> bool { - match self { - Self::Legacy(rules) => rules.is_empty(), - Self::Grouped(groups) => groups.allow.is_empty() && groups.deny.is_empty(), - } - } - - // The proto model still carries allow rules and deny rules separately. This - // folds both YAML spellings back into that stable internal representation. - fn into_parts(self) -> (Vec, Vec) { - match self { - Self::Legacy(rules) => (rules, Vec::new()), - Self::Grouped(groups) => ( - groups - .allow - .into_iter() - .map(|allow| L7RuleDef { allow }) - .collect(), - groups.deny, - ), - } - } -} - -#[derive(Debug, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct L7RuleGroupsDef { - #[serde(default, skip_serializing_if = "Vec::is_empty")] - allow: Vec, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - deny: Vec, -} - #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct L7AllowDef { @@ -459,15 +408,14 @@ fn params_with_tool( } fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { - let (method, rpc_method) = if is_mcp_protocol(protocol) { - let rpc_method = if allow.method.is_empty() { + let method = if is_jsonrpc_family_protocol(protocol) { + if allow.method.is_empty() { allow.rpc_method } else { allow.method - }; - (String::new(), rpc_method) + } } else { - (allow.method, allow.rpc_method) + allow.method }; L7Allow { @@ -477,7 +425,6 @@ fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { operation_type: allow.operation_type, operation_name: allow.operation_name, fields: allow.fields, - rpc_method, query: allow .query .into_iter() @@ -491,15 +438,14 @@ fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { } fn deny_def_to_proto(protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { - let (method, rpc_method) = if is_mcp_protocol(protocol) { - let rpc_method = if deny.method.is_empty() { + let method = if is_jsonrpc_family_protocol(protocol) { + if deny.method.is_empty() { deny.rpc_method } else { deny.method - }; - (String::new(), rpc_method) + } } else { - (deny.method, deny.rpc_method) + deny.method }; L7DenyRule { @@ -509,7 +455,6 @@ fn deny_def_to_proto(protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { operation_type: deny.operation_type, operation_name: deny.operation_name, fields: deny.fields, - rpc_method, query: deny .query .into_iter() @@ -535,6 +480,14 @@ fn is_mcp_protocol(protocol: &str) -> bool { protocol.eq_ignore_ascii_case("mcp") } +fn is_jsonrpc_protocol(protocol: &str) -> bool { + protocol.eq_ignore_ascii_case("json-rpc") +} + +fn is_jsonrpc_family_protocol(protocol: &str) -> bool { + is_mcp_protocol(protocol) || is_jsonrpc_protocol(protocol) +} + fn split_tool_param( protocol: &str, params: BTreeMap, @@ -566,9 +519,11 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); let (method, rpc_method) = if is_mcp_protocol(protocol) { - (allow.rpc_method, String::new()) + (allow.method, String::new()) + } else if is_jsonrpc_protocol(protocol) { + (String::new(), allow.method) } else { - (allow.method, allow.rpc_method) + (allow.method, String::new()) }; L7AllowDef { method, @@ -597,9 +552,11 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); let (method, rpc_method) = if is_mcp_protocol(protocol) { - (deny.rpc_method.clone(), String::new()) + (deny.method.clone(), String::new()) + } else if is_jsonrpc_protocol(protocol) { + (String::new(), deny.method.clone()) } else { - (deny.method.clone(), deny.rpc_method.clone()) + (deny.method.clone(), String::new()) }; L7DenyRuleDef { method, @@ -635,9 +592,8 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { .into_iter() .map(|e| { let protocol = e.protocol; - let (allow_rules, grouped_deny_rules) = e.rules.into_parts(); - let mut deny_rules = grouped_deny_rules; - deny_rules.extend(e.deny_rules); + let allow_rules = e.rules; + let deny_rules = e.deny_rules; // Normalize port/ports: ports takes precedence, else // single port is promoted to ports array. let normalized_ports: Vec = if !e.ports.is_empty() { @@ -770,39 +726,21 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { (clamp(e.ports.first().copied().unwrap_or(e.port)), vec![]) }; let protocol = e.protocol.clone(); - let allow_defs: Vec = e + let rules = e .rules .iter() - .map(|r| { - allow_proto_to_def(&protocol, r.allow.clone().unwrap_or_default()) + .map(|r| L7RuleDef { + allow: allow_proto_to_def( + &protocol, + r.allow.clone().unwrap_or_default(), + ), }) .collect(); - let deny_defs: Vec = e + let deny_rules: Vec = e .deny_rules .iter() .map(|d| deny_proto_to_def(&protocol, d)) .collect(); - let (rules, deny_rules) = if is_mcp_protocol(&protocol) - && (!allow_defs.is_empty() || !deny_defs.is_empty()) - { - ( - RulesDef::Grouped(L7RuleGroupsDef { - allow: allow_defs, - deny: deny_defs, - }), - Vec::new(), - ) - } else { - ( - RulesDef::Legacy( - allow_defs - .into_iter() - .map(|allow| L7RuleDef { allow }) - .collect(), - ), - deny_defs, - ) - }; let (json_rpc, mcp) = if is_mcp_protocol(&protocol) { (None, mcp_config_from_proto(e.json_rpc_max_body_bytes)) } else { @@ -2058,7 +1996,7 @@ network_policies: } #[test] - fn parse_grouped_mcp_rules_to_runtime_fields() { + fn parse_mcp_rules_to_runtime_fields() { let yaml = r" version: 1 network_policies: @@ -2073,17 +2011,19 @@ network_policies: mcp: max_body_bytes: 131072 rules: - deny: - - method: tools/call - tool: send_email - allow: - - method: initialize - - method: tools/list - - method: tools/call + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call tool: search_web params: arguments: repository: NVIDIA/OpenShell + deny_rules: + - method: tools/call + tool: send_email binaries: - path: /usr/bin/curl "; @@ -2093,7 +2033,7 @@ network_policies: assert_eq!(ep.protocol, "mcp"); assert_eq!(ep.json_rpc_max_body_bytes, 131_072); assert_eq!(ep.rules.len(), 3); - assert_eq!(ep.rules[2].allow.as_ref().unwrap().rpc_method, "tools/call"); + assert_eq!(ep.rules[2].allow.as_ref().unwrap().method, "tools/call"); assert_eq!( ep.rules[2].allow.as_ref().unwrap().params["name"].glob, "search_web" @@ -2103,7 +2043,7 @@ network_policies: "NVIDIA/OpenShell" ); assert_eq!(ep.deny_rules.len(), 1); - assert_eq!(ep.deny_rules[0].rpc_method, "tools/call"); + assert_eq!(ep.deny_rules[0].method, "tools/call"); assert_eq!(ep.deny_rules[0].params["name"].glob, "send_email"); } @@ -2121,15 +2061,15 @@ network_policies: mcp: max_body_bytes: 131072 rules: - allow: - - method: tools/call + - allow: + method: tools/call tool: search_web params: arguments: repository: NVIDIA/OpenShell - deny: - - method: tools/call - tool: send_email + deny_rules: + - method: tools/call + tool: send_email binaries: - path: /usr/bin/curl "; @@ -2141,6 +2081,7 @@ network_policies: assert!(yaml_out.contains("method: tools/call")); assert!(yaml_out.contains("tool: search_web")); assert!(yaml_out.contains("tool: send_email")); + assert!(yaml_out.contains("deny_rules:")); assert!(yaml_out.contains("arguments:")); assert!(yaml_out.contains("repository: NVIDIA/OpenShell")); assert!(!yaml_out.contains("arguments.repository")); diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 17abd3ac7..09ddcf998 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -736,14 +736,13 @@ fn expand_access_preset(protocol: &str, access: &str) -> Option> { .into_iter() .map(|rpc_method| L7Rule { allow: Some(L7Allow { - method: String::new(), + method: rpc_method.to_string(), path: String::new(), command: String::new(), query: HashMap::default(), operation_type: String::new(), operation_name: String::new(), fields: Vec::new(), - rpc_method: rpc_method.to_string(), params: HashMap::default(), }), }) diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 57eb4d305..55aa81fcd 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -1030,7 +1030,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< "{deny_loc}: GraphQL rule fields are ignored unless protocol is graphql or websocket" )); } - } } } diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 039d60988..61f5ebe12 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -140,6 +140,7 @@ fn engine_type_for_protocol(protocol: L7Protocol) -> &'static str { match protocol { L7Protocol::Graphql => "l7-graphql", L7Protocol::JsonRpc => "l7-jsonrpc", + L7Protocol::Mcp => "l7-mcp", L7Protocol::Websocket => "l7-websocket", L7Protocol::Rest | L7Protocol::Sql => "l7", } @@ -567,7 +568,7 @@ fn l7_protocol_log_summary( if let Some(info) = jsonrpc_info { return format!( " rpc_methods={} params_sha256={}", - jsonrpc_methods_for_log(info), + rule_method_names_for_log(info), info.params_sha256() .unwrap_or_else(|| "".to_string()) ); @@ -2457,14 +2458,16 @@ network_policies: mcp: max_body_bytes: 131072 rules: - deny: - - method: tools/call - tool: delete_resource - allow: - - method: initialize - - method: tools/list - - method: tools/call + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call tool: read_status + deny_rules: + - method: tools/call + tool: delete_resource binaries: - { path: /usr/bin/node } "#; diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 9e9c92de0..d601b2a80 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -1188,18 +1188,20 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St ep["access"] = e.access.clone().into(); } if !e.rules.is_empty() { + let jsonrpc_family = + matches!(e.protocol.as_str(), "json-rpc" | "mcp"); let rules: Vec = e .rules .iter() .map(|r| { let a = r.allow.as_ref(); let mut allow = serde_json::json!({ - "method": a.map_or("", |a| &a.method), + "method": if jsonrpc_family { "" } else { a.map_or("", |a| &a.method) }, "path": a.map_or("", |a| &a.path), "command": a.map_or("", |a| &a.command), "operation_type": a.map_or("", |a| &a.operation_type), "operation_name": a.map_or("", |a| &a.operation_name), - "rpc_method": a.map_or("", |a| &a.rpc_method), + "rpc_method": if jsonrpc_family { a.map_or("", |a| &a.method) } else { "" }, }); if let Some(a) = a && !a.fields.is_empty() @@ -1230,12 +1232,16 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St ep["advisor_proposed"] = true.into(); } if !e.deny_rules.is_empty() { + let jsonrpc_family = + matches!(e.protocol.as_str(), "json-rpc" | "mcp"); let deny_rules: Vec = e .deny_rules .iter() .map(|d| { let mut deny = serde_json::json!({}); - if !d.method.is_empty() { + if jsonrpc_family && !d.method.is_empty() { + deny["rpc_method"] = d.method.clone().into(); + } else if !d.method.is_empty() { deny["method"] = d.method.clone().into(); } if !d.path.is_empty() { @@ -1253,9 +1259,6 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St if !d.fields.is_empty() { deny["fields"] = d.fields.clone().into(); } - if !d.rpc_method.is_empty() { - deny["rpc_method"] = d.rpc_method.clone().into(); - } let query = l7_matchers_to_json(&d.query); if !query.is_empty() { deny["query"] = query.into(); diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 7982a8905..9d0c7ab63 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -159,7 +159,7 @@ Each endpoint defines a reachable destination and optional inspection rules. | `tls` | string | No | TLS handling mode. The proxy auto-detects TLS by peeking the first bytes of each connection and terminates it for inspected HTTPS traffic, so this field is optional in most cases. Set to `skip` to disable auto-detection for edge cases such as client-certificate mTLS or non-standard protocols. The values `terminate` and `passthrough` are deprecated and log a warning; they are still accepted for backward compatibility but have no effect on behavior. | | `enforcement` | string | No | `enforce` actively blocks disallowed requests. `audit` logs violations but allows traffic through. | | `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. Not valid on `protocol: mcp` or `protocol: json-rpc`; those endpoints must use explicit rules. | -| `rules` | list of rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. | +| `rules` | list of allow rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. | | `deny_rules` | list of deny rule objects | No | L7 deny rules that block specific requests even when allowed by `access` or `rules`. Deny rules take precedence over allow rules. | | `allowed_ips` | list of string | No | CIDR or IP allowlist for SSRF override. Exact user-declared hostname endpoints may resolve to RFC 1918 private addresses without this field, but wildcard, hostless, and policy-advisor-proposed endpoints still require `allowed_ips` for private resolved IPs. Entries overlapping loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), or unspecified (`0.0.0.0`) are rejected at load time. | | `allow_encoded_slash` | bool | No | When `true`, L7 request parsing preserves `%2F` inside path segments instead of rejecting it. Use this for registries and APIs such as npm scoped packages (`/@scope%2Fname`). Defaults to `false`. | @@ -282,7 +282,7 @@ Do not combine `method`, `path`, or `query` with `operation_type`, `operation_na MCP rules match sandbox-to-server MCP Streamable HTTP request bodies by MCP method and optional tool or params selectors. OpenShell parses the underlying JSON-RPC 2.0 envelope, validates known MCP request and notification params, and preserves unknown extension methods as policy-addressable method strings. JSON-RPC responses and server-to-client MCP messages on response bodies or SSE streams are relayed but are not currently parsed for policy enforcement. -Deny rules take precedence over allow rules. In a batch request, one denied call denies the full batch. +Use `rules` for MCP allow rules and `deny_rules` for MCP deny rules. Deny rules take precedence over allow rules. In a batch request, one denied call denies the full batch. | Field | Type | Required | Description | |---|---|---|---| From a761fc8a89dee9f324fbf4bfb5c2415a8bc60733 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:14:26 -0700 Subject: [PATCH 11/12] refactor(policy): use method for json-rpc rules Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 60 ++----------------- crates/openshell-policy/src/merge.rs | 8 +-- .../data/sandbox-policy.rego | 12 ++-- .../src/l7/mod.rs | 49 +++------------ .../src/l7/relay.rs | 2 +- .../openshell-supervisor-network/src/opa.rs | 58 +++++------------- docs/reference/policy-schema.mdx | 20 +++---- docs/sandboxes/policies.mdx | 8 +-- 8 files changed, 56 insertions(+), 161 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index d3a713aba..5fe3edd1f 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -213,8 +213,6 @@ struct L7AllowDef { #[serde(default, skip_serializing_if = "Vec::is_empty")] fields: Vec, #[serde(default, skip_serializing_if = "String::is_empty")] - rpc_method: String, - #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, @@ -263,8 +261,6 @@ struct L7DenyRuleDef { #[serde(default, skip_serializing_if = "Vec::is_empty")] fields: Vec, #[serde(default, skip_serializing_if = "String::is_empty")] - rpc_method: String, - #[serde(default, skip_serializing_if = "String::is_empty")] tool: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] params: BTreeMap, @@ -407,19 +403,9 @@ fn params_with_tool( params } -fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { - let method = if is_jsonrpc_family_protocol(protocol) { - if allow.method.is_empty() { - allow.rpc_method - } else { - allow.method - } - } else { - allow.method - }; - +fn allow_def_to_proto(_protocol: &str, allow: L7AllowDef) -> L7Allow { L7Allow { - method, + method: allow.method, path: allow.path, command: allow.command, operation_type: allow.operation_type, @@ -437,19 +423,9 @@ fn allow_def_to_proto(protocol: &str, allow: L7AllowDef) -> L7Allow { } } -fn deny_def_to_proto(protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { - let method = if is_jsonrpc_family_protocol(protocol) { - if deny.method.is_empty() { - deny.rpc_method - } else { - deny.method - } - } else { - deny.method - }; - +fn deny_def_to_proto(_protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { L7DenyRule { - method, + method: deny.method, path: deny.path, command: deny.command, operation_type: deny.operation_type, @@ -480,14 +456,6 @@ fn is_mcp_protocol(protocol: &str) -> bool { protocol.eq_ignore_ascii_case("mcp") } -fn is_jsonrpc_protocol(protocol: &str) -> bool { - protocol.eq_ignore_ascii_case("json-rpc") -} - -fn is_jsonrpc_family_protocol(protocol: &str) -> bool { - is_mcp_protocol(protocol) || is_jsonrpc_protocol(protocol) -} - fn split_tool_param( protocol: &str, params: BTreeMap, @@ -518,15 +486,8 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { .collect(); let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); - let (method, rpc_method) = if is_mcp_protocol(protocol) { - (allow.method, String::new()) - } else if is_jsonrpc_protocol(protocol) { - (String::new(), allow.method) - } else { - (allow.method, String::new()) - }; L7AllowDef { - method, + method: allow.method, path: allow.path, command: allow.command, query: allow @@ -537,7 +498,6 @@ fn allow_proto_to_def(protocol: &str, allow: L7Allow) -> L7AllowDef { operation_type: allow.operation_type, operation_name: allow.operation_name, fields: allow.fields, - rpc_method, tool, params, } @@ -551,15 +511,8 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { .collect(); let (tool, params) = split_tool_param(protocol, params); let params = flat_params_to_def(protocol, params); - let (method, rpc_method) = if is_mcp_protocol(protocol) { - (deny.method.clone(), String::new()) - } else if is_jsonrpc_protocol(protocol) { - (String::new(), deny.method.clone()) - } else { - (deny.method.clone(), String::new()) - }; L7DenyRuleDef { - method, + method: deny.method.clone(), path: deny.path.clone(), command: deny.command.clone(), query: deny @@ -570,7 +523,6 @@ fn deny_proto_to_def(protocol: &str, deny: &L7DenyRule) -> L7DenyRuleDef { operation_type: deny.operation_type.clone(), operation_name: deny.operation_name.clone(), fields: deny.fields.clone(), - rpc_method, tool, params, } diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 09ddcf998..6050faaa4 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -727,16 +727,16 @@ fn expand_existing_access( fn expand_access_preset(protocol: &str, access: &str) -> Option> { if matches!(protocol, "json-rpc" | "mcp") { - let rpc_methods = match access { + let methods = match access { "read-only" | "read-write" | "full" => vec!["*"], _ => return None, }; return Some( - rpc_methods + methods .into_iter() - .map(|rpc_method| L7Rule { + .map(|method| L7Rule { allow: Some(L7Allow { - method: rpc_method.to_string(), + method: method.to_string(), path: String::new(), command: String::new(), query: HashMap::default(), diff --git a/crates/openshell-supervisor-network/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index f23ef7d7c..661ac9ee4 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -282,7 +282,7 @@ request_denied_for_endpoint(request, endpoint) if { request.method == "POST" some deny_rule deny_rule := endpoint.deny_rules[_] - deny_rule.rpc_method + deny_rule.method jsonrpc_rule_matches(request, deny_rule) } @@ -451,7 +451,7 @@ request_allowed_for_endpoint(request, endpoint) if { request.method == "POST" some rule rule := endpoint.rules[_] - rule.allow.rpc_method + rule.allow.method not jsonrpc_response_frame_present(request) jsonrpc_rule_matches(request, rule.allow) } @@ -707,10 +707,10 @@ jsonrpc_rule_matches(request, rule) if { method := object.get(jsonrpc, "method", "") is_string(method) method != "" - rpc_method := object.get(rule, "rpc_method", "") - is_string(rpc_method) - rpc_method != "" - glob.match(rpc_method, [], method) + rule_method := object.get(rule, "method", "") + is_string(rule_method) + rule_method != "" + glob.match(rule_method, [], method) jsonrpc_params_match(jsonrpc, rule) } diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 55aa81fcd..e29961810 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -593,31 +593,15 @@ fn validate_jsonrpc_rule_fields( rule: &serde_json::Value, protocol: &str, ) { - if rule.get("mcp_method").is_some() { - errors.push(format!( - "{loc}.mcp_method: use `method` for protocol mcp L7 rules" - )); - } - - let rpc_method = rule - .get("rpc_method") - .and_then(|v| v.as_str()) - .unwrap_or(""); + let method = rule.get("method").and_then(|v| v.as_str()).unwrap_or(""); let has_params = rule.get("params").is_some_and(|v| !v.is_null()); let jsonrpc_family = protocol == "json-rpc" || protocol == "mcp"; if jsonrpc_family { - let method_field = if protocol == "mcp" { - "method" - } else { - "rpc_method" - }; - if rpc_method.is_empty() { - errors.push(format!( - "{loc}.{method_field}: required for {protocol} L7 rules" - )); - } else if let Some(warning) = check_glob_syntax(rpc_method) { - warnings.push(format!("{loc}.{method_field}: {warning}")); + if method.is_empty() { + errors.push(format!("{loc}.method: required for {protocol} L7 rules")); + } else if let Some(warning) = check_glob_syntax(method) { + warnings.push(format!("{loc}.method: {warning}")); } validate_matcher_map( errors, @@ -627,17 +611,12 @@ fn validate_jsonrpc_rule_fields( ); if json_rule_has_non_empty_path_or_query(rule) { errors.push(format!( - "{loc}: {protocol} L7 rules must use {method_field}/params, not path/query" + "{loc}: {protocol} L7 rules must use method/params, not path/query" )); } return; } - if !rpc_method.is_empty() { - errors.push(format!( - "{loc}.rpc_method: JSON-RPC method matching is only valid for protocol json-rpc or mcp" - )); - } if has_params { errors.push(format!( "{loc}.params: JSON-RPC params matching is only valid for protocol json-rpc or mcp" @@ -775,24 +754,14 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< } if (protocol == "json-rpc" || protocol == "mcp") && !access.is_empty() { - let method_field = if protocol == "mcp" { - "method" - } else { - "rpc_method" - }; errors.push(format!( - "{loc}: protocol {protocol} does not support access presets; use explicit rules with allow.{method_field} such as \"*\"" + "{loc}: protocol {protocol} does not support access presets; use explicit rules with allow.method such as \"*\"" )); } if (protocol == "json-rpc" || protocol == "mcp") && !has_rules { - let method_field = if protocol == "mcp" { - "method" - } else { - "rpc_method" - }; errors.push(format!( - "{loc}: protocol {protocol} requires explicit rules with allow.{method_field}" + "{loc}: protocol {protocol} requires explicit rules with allow.method" )); } @@ -2538,7 +2507,7 @@ mod tests { "protocol": "mcp", "rules": [{ "allow": { - "rpc_method": "tools/call", + "method": "tools/call", "params": { "name": "submit_report", "arguments": { diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 61f5ebe12..5e3b9854f 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -567,7 +567,7 @@ fn l7_protocol_log_summary( if let Some(info) = jsonrpc_info { return format!( - " rpc_methods={} params_sha256={}", + " rule_methods={} params_sha256={}", rule_method_names_for_log(info), info.params_sha256() .unwrap_or_else(|| "".to_string()) diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index d601b2a80..916a1b35c 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -860,20 +860,15 @@ fn normalize_jsonrpc_config_alias(ep: &mut serde_json::Map) { - let protocol = ep - .get("protocol") - .and_then(serde_json::Value::as_str) - .unwrap_or("") - .to_string(); if let Some(rules) = ep.get_mut("rules").and_then(|v| v.as_array_mut()) { for rule in rules { if let Some(allow) = rule .get_mut("allow") .and_then(serde_json::Value::as_object_mut) { - normalize_l7_rule_aliases(allow, &protocol); + normalize_l7_rule_aliases(allow); } else if let Some(allow) = rule.as_object_mut() { - normalize_l7_rule_aliases(allow, &protocol); + normalize_l7_rule_aliases(allow); } } } @@ -881,27 +876,13 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map, - protocol: &str, -) { - if protocol == "mcp" - && let Some(method) = rule.remove("method") - && rule - .get("rpc_method") - .and_then(serde_json::Value::as_str) - .unwrap_or("") - .is_empty() - { - rule.insert("rpc_method".to_string(), method); - } - +fn normalize_l7_rule_aliases(rule: &mut serde_json::Map) { if let Some(tool) = rule.remove("tool") && let Some(tool_name) = tool.as_str().filter(|s| !s.is_empty()) { @@ -1188,20 +1169,17 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St ep["access"] = e.access.clone().into(); } if !e.rules.is_empty() { - let jsonrpc_family = - matches!(e.protocol.as_str(), "json-rpc" | "mcp"); let rules: Vec = e .rules .iter() .map(|r| { let a = r.allow.as_ref(); let mut allow = serde_json::json!({ - "method": if jsonrpc_family { "" } else { a.map_or("", |a| &a.method) }, + "method": a.map_or("", |a| &a.method), "path": a.map_or("", |a| &a.path), "command": a.map_or("", |a| &a.command), "operation_type": a.map_or("", |a| &a.operation_type), "operation_name": a.map_or("", |a| &a.operation_name), - "rpc_method": if jsonrpc_family { a.map_or("", |a| &a.method) } else { "" }, }); if let Some(a) = a && !a.fields.is_empty() @@ -1232,16 +1210,12 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St ep["advisor_proposed"] = true.into(); } if !e.deny_rules.is_empty() { - let jsonrpc_family = - matches!(e.protocol.as_str(), "json-rpc" | "mcp"); let deny_rules: Vec = e .deny_rules .iter() .map(|d| { let mut deny = serde_json::json!({}); - if jsonrpc_family && !d.method.is_empty() { - deny["rpc_method"] = d.method.clone().into(); - } else if !d.method.is_empty() { + if !d.method.is_empty() { deny["method"] = d.method.clone().into(); } if !d.path.is_empty() { @@ -2896,7 +2870,7 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: initialize + method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3040,7 +3014,7 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: initialize + method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3076,7 +3050,7 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: initialize + method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3109,9 +3083,9 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: initialize + method: initialize deny_rules: - - rpc_method: tools/delete + - method: tools/delete binaries: - { path: /usr/bin/curl } "#; @@ -3154,16 +3128,16 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: tools/call + method: tools/call params: name: read_status - allow: - rpc_method: tools/call + method: tools/call params: name: submit_report arguments.scope: workspace/main deny_rules: - - rpc_method: tools/call + - method: tools/call params: name: blocked_action binaries: @@ -3295,7 +3269,7 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: tools/list + method: tools/list binaries: - { path: /usr/bin/curl } "#; @@ -3332,7 +3306,7 @@ network_policies: enforcement: enforce rules: - allow: - rpc_method: tools/call + method: tools/call params: name: any: [] diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 9d0c7ab63..762d6cc8c 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -183,7 +183,7 @@ The `access` field accepts one of the following values on REST, WebSocket, and G | `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | Rejected. | | `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | Rejected. | -For MCP endpoints, configure explicit `rules` with `method`, optional `tool`, and optional `params`. For generic JSON-RPC endpoints, use `rpc_method` and optional `params`. +For MCP endpoints, configure explicit `rules` with `method`, optional `tool`, and optional `params`. For generic JSON-RPC endpoints, use `method` and optional `params`. #### Allow Rule Objects @@ -328,7 +328,7 @@ JSON-RPC allow rules match sandbox-to-server JSON-RPC-over-HTTP request objects | Field | Type | Required | Description | |---|---|---|---| -| `rpc_method` | string | Yes | JSON-RPC method name or OpenShell glob, such as `initialize`, `tools/list`, or `tools/*`. `*` is OpenShell policy matching syntax, not JSON-RPC method syntax. | +| `method` | string | Yes | JSON-RPC method name or OpenShell glob, such as `initialize`, `tools/list`, or `tools/*`. `*` is OpenShell policy matching syntax, not JSON-RPC method syntax. | | `params` | map | No | Nested params matcher map. Matcher leaves can be a glob string or an object with `any`. Dot-separated keys such as `arguments.scope` remain accepted for compatibility. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. | Example JSON-RPC allow rules: @@ -344,15 +344,15 @@ endpoints: max_body_bytes: 131072 rules: - allow: - rpc_method: initialize + method: initialize - allow: - rpc_method: tools/list + method: tools/list - allow: - rpc_method: tools/call + method: tools/call params: name: read_status - allow: - rpc_method: tools/call + method: tools/call params: name: submit_report arguments: @@ -447,8 +447,8 @@ JSON-RPC deny rules use the same field names as JSON-RPC allow rules, but they a | Field | Type | Required | Description | |---|---|---|---| -| `rpc_method` | string | Yes | JSON-RPC method name or glob to deny. A value without glob metacharacters matches that exact method name. In glob patterns, `*` matches any character sequence, so `rpc_method: "*"` denies any JSON-RPC method rather than a literal method named `*`. To deny a literal method named `*`, escape the glob character as `rpc_method: "\\*"`. | -| `params` | map | No | Params matchers keyed by flattened object-param path. Omit to deny every call matching `rpc_method`. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. | +| `method` | string | Yes | JSON-RPC method name or glob to deny. A value without glob metacharacters matches that exact method name. In glob patterns, `*` matches any character sequence, so `method: "*"` denies any JSON-RPC method rather than a literal method named `*`. To deny a literal method named `*`, escape the glob character as `method: "\\*"`. | +| `params` | map | No | Params matchers keyed by flattened object-param path. Omit to deny every call matching `method`. Strings, numbers, and booleans are converted to strings; arrays, `null`, and non-object top-level params do not produce matcher keys. | Example JSON-RPC deny rules: @@ -461,9 +461,9 @@ endpoints: enforcement: enforce rules: - allow: - rpc_method: tools/* + method: tools/* deny_rules: - - rpc_method: tools/call + - method: tools/call params: name: delete_resource ``` diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index a1e20239b..17eb625c9 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -148,7 +148,7 @@ The following steps outline the hot-reload policy update workflow. To inspect a stored sandbox-authored revision instead of the current effective policy, pass `--rev `. -5. Edit the YAML: add or adjust `network_policies` entries, binaries, `access`, `rules`, or protocol-specific matchers such as GraphQL operation fields, MCP `method` / `tool` rules, and generic JSON-RPC `rpc_method` / `params` rules. +5. Edit the YAML: add or adjust `network_policies` entries, binaries, `access`, `rules`, or protocol-specific matchers such as GraphQL operation fields, MCP `method` / `tool` rules, and generic JSON-RPC `method` / `params` rules. 6. Push the updated policy when you need a full replacement. Exit codes: 0 = loaded, 1 = validation failed, 124 = timeout. @@ -549,7 +549,7 @@ For an end-to-end walkthrough that combines this policy with a GitHub credential - { path: /usr/bin/gh } ``` -Endpoints with `protocol: rest` enable HTTP request inspection and can opt in to supported text request body credential rewrite. Endpoints with `protocol: websocket` validate WebSocket upgrades and inspect client text messages on the upgraded request path. WebSocket endpoints can also classify GraphQL-over-WebSocket operation messages with the same operation rules used by GraphQL-over-HTTP. Endpoints with `protocol: graphql` parse GraphQL-over-HTTP payloads before evaluating rules. Endpoints with `protocol: mcp` parse MCP Streamable HTTP request bodies and evaluate `method`, optional `tool`, and optional params rules. Endpoints with `protocol: json-rpc` keep generic JSON-RPC-over-HTTP compatibility by evaluating `rpc_method` and optional params rules. The endpoint-level `path` field lets these protocols share `api.github.com:443` without treating GraphQL payloads as plain REST `POST /graphql` requests. +Endpoints with `protocol: rest` enable HTTP request inspection and can opt in to supported text request body credential rewrite. Endpoints with `protocol: websocket` validate WebSocket upgrades and inspect client text messages on the upgraded request path. WebSocket endpoints can also classify GraphQL-over-WebSocket operation messages with the same operation rules used by GraphQL-over-HTTP. Endpoints with `protocol: graphql` parse GraphQL-over-HTTP payloads before evaluating rules. Endpoints with `protocol: mcp` parse MCP Streamable HTTP request bodies and evaluate `method`, optional `tool`, and optional params rules. Endpoints with `protocol: json-rpc` keep generic JSON-RPC-over-HTTP compatibility by evaluating `method` and optional params rules. The endpoint-level `path` field lets these protocols share `api.github.com:443` without treating GraphQL payloads as plain REST `POST /graphql` requests. @@ -582,7 +582,7 @@ REST rules can also constrain query parameter values: ### MCP and JSON-RPC matching -MCP endpoints use `protocol: mcp`. The proxy parses sandbox-to-server MCP Streamable HTTP request bodies, validates known MCP request and notification params, evaluates the MCP method against rule `method`, and can match tool calls with the `tool` alias. Generic JSON-RPC endpoints use `protocol: json-rpc`, evaluate `rpc_method`, and can match object params. +MCP endpoints use `protocol: mcp`. The proxy parses sandbox-to-server MCP Streamable HTTP request bodies, validates known MCP request and notification params, evaluates the MCP method against rule `method`, and can match tool calls with the `tool` alias. Generic JSON-RPC endpoints use `protocol: json-rpc`, evaluate `method`, and can match object params. MCP policy enforcement is directional. It applies to HTTP request bodies sent by the sandboxed process to the configured endpoint. JSON-RPC responses and server-to-client MCP messages carried on response bodies or SSE streams are relayed but are not currently parsed for policy enforcement. @@ -622,7 +622,7 @@ MCP and JSON-RPC endpoint policies currently require full policy YAML applied wi `mcp.max_body_bytes` controls how many MCP-over-HTTP request body bytes OpenShell buffers for inspection. It defaults to `65536`. -Use `protocol: json-rpc` and `rpc_method` when you need generic JSON-RPC 2.0 matching for a non-MCP server. `json_rpc.max_body_bytes` controls the generic JSON-RPC inspection buffer. +Use `protocol: json-rpc` and `method` when you need generic JSON-RPC 2.0 matching for a non-MCP server. `json_rpc.max_body_bytes` controls the generic JSON-RPC inspection buffer. `params` matchers are case-sensitive and use the same string glob or `{ any: [...] }` matcher syntax as REST query parameters. Write them as nested maps that mirror MCP params. OpenShell flattens the matcher map internally before evaluating scalar leaf values from object params: strings, numbers, and booleans are converted to strings. Dot-separated matcher keys remain accepted for compatibility. Arrays, `null`, and non-object top-level params do not produce matcher keys. Requests with literal `.` characters in params object keys are rejected before policy evaluation because they are ambiguous with flattened nested paths. This is useful for controls such as matching MCP `tools/call` by `tool` or `params.name`, but it is not a complete MCP payload policy for rich nested content. For batch requests, OpenShell evaluates each JSON-RPC call independently and denies the whole batch if any call is denied. From a71aeff9723257919677ae36b554430ffa5230e1 Mon Sep 17 00:00:00 2001 From: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:58:40 -0700 Subject: [PATCH 12/12] fix(policy): align json-rpc inspection mode Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/merge.rs | 24 ------- .../src/l7/jsonrpc.rs | 67 +++++++++--------- .../src/l7/mod.rs | 68 +++++++++++++------ .../src/l7/relay.rs | 26 +++---- .../openshell-supervisor-network/src/proxy.rs | 11 ++- 5 files changed, 100 insertions(+), 96 deletions(-) diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 6050faaa4..ef6566a1f 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -726,30 +726,6 @@ fn expand_existing_access( } fn expand_access_preset(protocol: &str, access: &str) -> Option> { - if matches!(protocol, "json-rpc" | "mcp") { - let methods = match access { - "read-only" | "read-write" | "full" => vec!["*"], - _ => return None, - }; - return Some( - methods - .into_iter() - .map(|method| L7Rule { - allow: Some(L7Allow { - method: method.to_string(), - path: String::new(), - command: String::new(), - query: HashMap::default(), - operation_type: String::new(), - operation_name: String::new(), - fields: Vec::new(), - params: HashMap::default(), - }), - }) - .collect(), - ); - } - let methods = match (protocol, access) { (_, "full") => vec!["*"], ("websocket", "read-only") => vec!["GET"], diff --git a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs index 4032e1013..1d66395bf 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -24,6 +24,15 @@ pub enum JsonRpcInspectionMode { Mcp, } +impl JsonRpcInspectionMode { + pub(crate) fn for_protocol(protocol: crate::l7::L7Protocol) -> Self { + match protocol { + crate::l7::L7Protocol::Mcp => Self::Mcp, + _ => Self::JsonRpc, + } + } +} + /// Parsed HTTP request plus the JSON-RPC-family metadata extracted from the /// body. The original HTTP request is still forwarded if policy allows it. pub struct JsonRpcHttpRequest { @@ -49,7 +58,7 @@ pub(crate) async fn parse_jsonrpc_http_request bool { }) }) } -/// Parse a JSON-RPC 2.0 request body and extract the `method` field. -/// -/// Returns an info struct with `method` set on success, or `error` set if the -/// body is not valid JSON-RPC 2.0. -pub fn parse_jsonrpc_body(body: &[u8]) -> JsonRpcRequestInfo { - parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::JsonRpc) -} - -/// Parse a JSON-RPC body as MCP, using tower-mcp-types for known MCP request -/// and notification shapes while still allowing extension methods. -pub fn parse_mcp_body(body: &[u8]) -> JsonRpcRequestInfo { - parse_jsonrpc_body_with_mode(body, JsonRpcInspectionMode::Mcp) -} - -pub fn parse_jsonrpc_body_with_mode( +/// Parse a JSON-RPC-family body using the endpoint's inspection mode. +pub fn parse_jsonrpc_body( body: &[u8], inspection_mode: JsonRpcInspectionMode, ) -> JsonRpcRequestInfo { @@ -449,7 +445,7 @@ mod tests { #[test] fn parses_method_from_request_body() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert_eq!( info.calls.first().map(|call| call.method.as_str()), Some("initialize") @@ -463,7 +459,7 @@ mod tests { #[test] fn parses_jsonrpc_response_body_without_method() { let body = br#"{"jsonrpc":"2.0","id":1,"result":{"action":"accept","content":{}}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!(!info.is_batch); @@ -476,7 +472,7 @@ mod tests { fn parses_jsonrpc_error_response_body_without_method() { let body = br#"{"jsonrpc":"2.0","id":"request-1","error":{"code":-32603,"message":"failed"}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!(info.has_response); @@ -486,7 +482,7 @@ mod tests { #[test] fn flattens_object_params_for_policy_matching() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/main"}}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); let params = &info.calls.first().expect("single request call").params; assert_eq!( params.get("name").map(String::as_str), @@ -501,7 +497,7 @@ mod tests { #[test] fn mcp_mode_validates_known_methods_and_extracts_tool() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"search_web","arguments":{"query":"openshell"}}}"#; - let info = parse_mcp_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::Mcp); assert!(info.error.is_none(), "expected valid MCP call: {info:?}"); let call = info.calls.first().expect("single MCP call"); @@ -516,7 +512,7 @@ mod tests { #[test] fn mcp_mode_rejects_invalid_known_method_params() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments":{"query":"openshell"}}}"#; - let info = parse_mcp_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::Mcp); assert!(info.calls.is_empty()); assert!( @@ -531,7 +527,7 @@ mod tests { fn mcp_mode_allows_unknown_extension_methods() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"vendor/extension","params":{"name":"custom"}}"#; - let info = parse_mcp_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::Mcp); assert!( info.error.is_none(), @@ -546,7 +542,7 @@ mod tests { #[test] fn rejects_literal_dotted_param_keys() { let body = br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments.scope":"workspace/other","arguments":{"scope":"workspace/main"}}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!( @@ -593,7 +589,7 @@ mod tests { #[test] fn rejects_requests_missing_jsonrpc_version() { let body = br#"{"id":1,"method":"tools/list"}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert_eq!( @@ -608,7 +604,7 @@ mod tests { {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"id":2,"method":"tools/call","params":{"name":"read_status"}} ]"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!(info.is_batch); @@ -621,7 +617,7 @@ mod tests { #[test] fn rejects_unsupported_jsonrpc_version() { let body = br#"{"jsonrpc":"1.0","id":1,"method":"tools/list"}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert_eq!( @@ -646,7 +642,7 @@ mod tests { {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"read_status"}} ]"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.error.is_none()); assert!(info.is_batch); assert!(!info.has_response); @@ -665,7 +661,7 @@ mod tests { {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"result":{"ok":true}} ]"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.error.is_none()); assert!(info.is_batch); @@ -678,7 +674,7 @@ mod tests { fn rejects_invalid_jsonrpc_response_body() { let body = br#"{"jsonrpc":"2.0","id":1,"result":{},"error":{"code":-32603,"message":"failed"}}"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!(!info.has_response); @@ -691,7 +687,7 @@ mod tests { #[test] fn rejects_message_with_method_and_result_or_error() { let result_body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","result":{}}"#; - let result_info = parse_jsonrpc_body(result_body); + let result_info = parse_jsonrpc_body(result_body, JsonRpcInspectionMode::JsonRpc); assert!(result_info.calls.is_empty()); assert_eq!( result_info.error.as_deref(), @@ -699,7 +695,7 @@ mod tests { ); let error_body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","error":{"code":-32603,"message":"failed"}}"#; - let error_info = parse_jsonrpc_body(error_body); + let error_info = parse_jsonrpc_body(error_body, JsonRpcInspectionMode::JsonRpc); assert!(error_info.calls.is_empty()); assert_eq!( error_info.error.as_deref(), @@ -713,7 +709,7 @@ mod tests { {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"initialize","result":{}} ]"#; - let info = parse_jsonrpc_body(body); + let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::JsonRpc); assert!(info.calls.is_empty()); assert!(info.is_batch); @@ -727,12 +723,15 @@ mod tests { fn params_digest_is_canonical_and_redacted() { let first = parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/main"}}}"#, + JsonRpcInspectionMode::JsonRpc, ); let reordered = parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"arguments":{"scope":"workspace/main"},"name":"submit_report"}}"#, + JsonRpcInspectionMode::JsonRpc, ); let changed = parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"submit_report","arguments":{"scope":"workspace/other"}}}"#, + JsonRpcInspectionMode::JsonRpc, ); let digest = first.params_sha256().expect("params digest"); @@ -751,12 +750,14 @@ mod tests { {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"blocked_action"}} ]"#, + JsonRpcInspectionMode::JsonRpc, ); let empty_batch = parse_jsonrpc_body( br#"[ {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"initialize"} ]"#, + JsonRpcInspectionMode::JsonRpc, ); let digest = batch.params_sha256().expect("batch params digest"); diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index e29961810..e61e80dad 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -489,11 +489,18 @@ fn validate_graphql_rule( validate_graphql_fields(errors, warnings, loc, rule.get("fields")); } +#[derive(Clone, Copy)] +enum MatcherNesting { + Flat, + Nested, +} + fn validate_matcher_map( errors: &mut Vec, warnings: &mut Vec, loc: &str, value: Option<&serde_json::Value>, + nesting: MatcherNesting, ) { let Some(value) = value.filter(|v| !v.is_null()) else { return; @@ -504,7 +511,7 @@ fn validate_matcher_map( }; for (key, matcher) in obj { - validate_matcher_value(errors, warnings, &format!("{loc}.{key}"), matcher); + validate_matcher_value(errors, warnings, &format!("{loc}.{key}"), matcher, nesting); } } @@ -513,6 +520,7 @@ fn validate_matcher_value( warnings: &mut Vec, loc: &str, matcher: &serde_json::Value, + nesting: MatcherNesting, ) { if let Some(glob_str) = matcher.as_str() { if let Some(warning) = check_glob_syntax(glob_str) { @@ -522,21 +530,31 @@ fn validate_matcher_value( } let Some(matcher_obj) = matcher.as_object() else { - errors.push(format!( - "{loc}: expected string glob, matcher object, or nested matcher map" - )); + errors.push(format!("{loc}: {}", matcher_expected_message(nesting))); return; }; let has_any = matcher_obj.get("any").is_some(); let has_glob = matcher_obj.get("glob").is_some(); if !has_any && !has_glob { + if matches!(nesting, MatcherNesting::Flat) { + errors.push(format!( + "{loc}: unknown matcher keys; only `glob` or `any` are supported" + )); + return; + } if matcher_obj.is_empty() { errors.push(format!("{loc}: nested matcher map must not be empty")); return; } for (key, child) in matcher_obj { - validate_matcher_value(errors, warnings, &format!("{loc}.{key}"), child); + validate_matcher_value( + errors, + warnings, + &format!("{loc}.{key}"), + child, + MatcherNesting::Nested, + ); } return; } @@ -586,6 +604,13 @@ fn validate_matcher_value( } } +fn matcher_expected_message(nesting: MatcherNesting) -> &'static str { + match nesting { + MatcherNesting::Flat => "expected string glob or matcher object", + MatcherNesting::Nested => "expected string glob, matcher object, or nested matcher map", + } +} + fn validate_jsonrpc_rule_fields( errors: &mut Vec, warnings: &mut Vec, @@ -595,7 +620,7 @@ fn validate_jsonrpc_rule_fields( ) { let method = rule.get("method").and_then(|v| v.as_str()).unwrap_or(""); let has_params = rule.get("params").is_some_and(|v| !v.is_null()); - let jsonrpc_family = protocol == "json-rpc" || protocol == "mcp"; + let jsonrpc_family = L7Protocol::parse(protocol).is_some_and(L7Protocol::is_jsonrpc_family); if jsonrpc_family { if method.is_empty() { @@ -608,6 +633,7 @@ fn validate_jsonrpc_rule_fields( warnings, &format!("{loc}.params"), rule.get("params"), + MatcherNesting::Nested, ); if json_rule_has_non_empty_path_or_query(rule) { errors.push(format!( @@ -695,6 +721,8 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< for (i, ep) in endpoints.iter().enumerate() { let protocol = ep.get("protocol").and_then(|v| v.as_str()).unwrap_or(""); + let l7_protocol = L7Protocol::parse(protocol); + let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family); let tls = ep.get("tls").and_then(|v| v.as_str()).unwrap_or(""); let enforcement = ep.get("enforcement").and_then(|v| v.as_str()).unwrap_or(""); let access = ep.get("access").and_then(|v| v.as_str()).unwrap_or(""); @@ -753,13 +781,13 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< errors.push(format!("{loc}: rules and access are mutually exclusive")); } - if (protocol == "json-rpc" || protocol == "mcp") && !access.is_empty() { + if jsonrpc_family && !access.is_empty() { errors.push(format!( "{loc}: protocol {protocol} does not support access presets; use explicit rules with allow.method such as \"*\"" )); } - if (protocol == "json-rpc" || protocol == "mcp") && !has_rules { + if jsonrpc_family && !has_rules { errors.push(format!( "{loc}: protocol {protocol} requires explicit rules with allow.method" )); @@ -772,7 +800,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - if !protocol.is_empty() && L7Protocol::parse(protocol).is_none() { + if !protocol.is_empty() && l7_protocol.is_none() { errors.push(format!( "{loc}: unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, json-rpc, or mcp)" )); @@ -823,10 +851,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - if protocol != "json-rpc" - && protocol != "mcp" - && ep.get("json_rpc_max_body_bytes").is_some() - { + if !jsonrpc_family && ep.get("json_rpc_max_body_bytes").is_some() { warnings.push(format!( "{loc}: JSON-RPC-specific endpoint fields are ignored unless protocol is json-rpc or mcp" )); @@ -956,6 +981,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< &mut warnings, &format!("{deny_loc}.query"), Some(query), + MatcherNesting::Flat, ); } @@ -1045,6 +1071,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< &mut warnings, &format!("{loc}.rules[{rule_idx}].allow.query"), Some(query), + MatcherNesting::Flat, ); } } @@ -1585,7 +1612,7 @@ mod tests { assert!( errors .iter() - .any(|e| { e.contains("JSON-RPC allow rules must specify method") }), + .any(|e| { e.contains("rules[0].allow.method") && e.contains("required") }), "JSON-RPC allow rules without method should be rejected: {errors:?}" ); assert!( @@ -1626,13 +1653,13 @@ mod tests { assert!( errors .iter() - .any(|e| { e.contains("rules[0].allow") && e.contains("params are only valid") }), + .any(|e| { e.contains("rules[0].allow.params") && e.contains("only valid") }), "REST allow rules with JSON-RPC fields should be rejected: {errors:?}" ); assert!( errors .iter() - .any(|e| { e.contains("deny_rules[0]") && e.contains("params are only valid") }), + .any(|e| { e.contains("deny_rules[0].params") && e.contains("only valid") }), "REST deny rules with JSON-RPC fields should be rejected: {errors:?}" ); } @@ -1664,7 +1691,7 @@ mod tests { assert!( errors .iter() - .any(|e| e.contains("JSON-RPC deny rules must specify method")), + .any(|e| e.contains("deny_rules[0].method") && e.contains("required")), "JSON-RPC deny rules without method should be rejected: {errors:?}" ); } @@ -1683,7 +1710,7 @@ mod tests { "allow": { "method": "tools/call", "params": { - "name": { "mode": "read-*" }, + "name": { "glob": "read-*", "mode": "strict" }, "scope": { "any": [] }, "count": 1 } @@ -1715,7 +1742,8 @@ mod tests { ); assert!( errors.iter().any(|e| { - e.contains("allow.params.count") && e.contains("expected string glob or object") + e.contains("allow.params.count") + && e.contains("expected string glob, matcher object, or nested matcher map") }), "JSON-RPC params should reject non-string/non-object matchers: {errors:?}" ); @@ -2686,7 +2714,7 @@ mod tests { assert!( errors .iter() - .any(|e| e.contains("expected string glob or object")), + .any(|e| e.contains("expected string glob or matcher object")), "should reject non-string/non-object matcher in deny query: {errors:?}" ); } diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 5e3b9854f..523258ca6 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -324,9 +324,9 @@ where ) .await { - Ok(body) => Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( + Ok(body) => Some(crate::l7::jsonrpc::parse_jsonrpc_body( &body, - jsonrpc_inspection_mode(config.protocol), + crate::l7::jsonrpc::JsonRpcInspectionMode::for_protocol(config.protocol), )), Err(e) => { if is_benign_connection_error(&e) { @@ -727,13 +727,6 @@ pub(crate) fn websocket_extension_mode(config: &L7EndpointConfig) -> WebSocketEx } } -fn jsonrpc_inspection_mode(protocol: L7Protocol) -> crate::l7::jsonrpc::JsonRpcInspectionMode { - match protocol { - L7Protocol::Mcp => crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp, - _ => crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, - } -} - fn jsonrpc_engine_type(protocol: L7Protocol) -> &'static str { match protocol { L7Protocol::Mcp => "l7-mcp", @@ -1038,7 +1031,7 @@ where allow_encoded_slash: config.allow_encoded_slash, ..Default::default() }, - jsonrpc_inspection_mode(config.protocol), + crate::l7::jsonrpc::JsonRpcInspectionMode::for_protocol(config.protocol), ) .await { @@ -2359,6 +2352,7 @@ network_policies: {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"read_status"}} ]"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, )), }; @@ -2370,6 +2364,7 @@ network_policies: {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"result":{"ok":true}} ]"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, )); let (allowed, reason) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); assert!(!allowed); @@ -2387,6 +2382,7 @@ network_policies: request.jsonrpc = Some(crate::l7::jsonrpc::parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":2,"result":{"ok":true}}"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, )); let (allowed, reason) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); assert!(!allowed); @@ -2405,6 +2401,7 @@ network_policies: {"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"blocked_action"}}, {"jsonrpc":"2.0","id":3,"method":"tools/delete","params":{"name":"purge_cache"}} ]"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, )); let (allowed, _) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); assert!(!allowed); @@ -2492,16 +2489,18 @@ network_policies: target: "/mcp".into(), query_params: std::collections::HashMap::new(), graphql: None, - jsonrpc: Some(crate::l7::jsonrpc::parse_mcp_body( + jsonrpc: Some(crate::l7::jsonrpc::parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"read_status","arguments":{}}}"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp, )), }; let (allowed, reason) = evaluate_l7_request(&tunnel_engine, &ctx, &request).unwrap(); assert!(allowed, "{reason}"); - request.jsonrpc = Some(crate::l7::jsonrpc::parse_mcp_body( + request.jsonrpc = Some(crate::l7::jsonrpc::parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"delete_resource","arguments":{"scope":"workspace/main"}}}"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp, )); let parsed = request.jsonrpc.as_ref().expect("parsed MCP request"); assert!( @@ -2525,6 +2524,7 @@ network_policies: fn jsonrpc_log_records_digest_not_args() { let info = crate::l7::jsonrpc::parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"delete_resource","arguments":{"scope":"secret-scope"}}}"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, ); let params_sha256 = info.params_sha256().expect("params digest"); let message = jsonrpc_log_message( @@ -2550,6 +2550,7 @@ network_policies: {"jsonrpc":"2.0","id":1,"method":"tools/list"}, {"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"delete_resource"}} ]"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, ); let batch_params_sha256 = batch.params_sha256().expect("batch params digest"); let batch_message = jsonrpc_log_message( @@ -2571,6 +2572,7 @@ network_policies: let no_params = crate::l7::jsonrpc::parse_jsonrpc_body( br#"{"jsonrpc":"2.0","id":1,"method":"initialize"}"#, + crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, ); let no_params_sha256 = no_params .params_sha256() diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index 988c42e10..89ba0ba39 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -3636,14 +3636,11 @@ async fn handle_forward_proxy( } }; forward_request_bytes = jsonrpc_request.raw_header; - Some(crate::l7::jsonrpc::parse_jsonrpc_body_with_mode( + Some(crate::l7::jsonrpc::parse_jsonrpc_body( &body, - match l7_config.config.protocol { - crate::l7::L7Protocol::Mcp => { - crate::l7::jsonrpc::JsonRpcInspectionMode::Mcp - } - _ => crate::l7::jsonrpc::JsonRpcInspectionMode::JsonRpc, - }, + crate::l7::jsonrpc::JsonRpcInspectionMode::for_protocol( + l7_config.config.protocol, + ), )) } } else {