From c6423ce46a765009f7543948e36fa366dee551cc 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/10] 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 | 170 ++++++- .../src/l7/relay.rs | 108 +++- .../openshell-supervisor-network/src/opa.rs | 259 ++++++++++ .../openshell-supervisor-network/src/proxy.rs | 13 +- docs/reference/policy-schema.mdx | 61 ++- docs/sandboxes/policies.mdx | 51 +- e2e/mcp-conformance/README.md | 16 +- .../client-through-openshell.sh | 11 +- e2e/mcp-conformance/expected-failures.yml | 6 +- e2e/mcp-conformance/policy-template.yaml | 8 +- e2e/with-docker-gateway.sh | 62 ++- 19 files changed, 1190 insertions(+), 225 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 005a1c54b..4c2ec2be3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3704,6 +3704,7 @@ dependencies = [ "tokio", "tokio-rustls", "tokio-tungstenite 0.26.2", + "tower-mcp-types", "tracing", "uuid", "webpki-roots 1.0.7", @@ -6377,6 +6378,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 7a380eb9d..1afe670e2 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 { @@ -198,6 +256,10 @@ struct L7AllowDef { 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, } @@ -235,6 +297,10 @@ struct L7DenyRuleDef { 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, } @@ -271,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 @@ -286,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() { @@ -304,61 +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, - rpc_method: r.allow.rpc_method, - 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, - rpc_method: d.rpc_method, - 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, @@ -381,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(), @@ -464,75 +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, - rpc_method: a.rpc_method, - 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(), - rpc_method: d.rpc_method.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, @@ -552,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(), @@ -1769,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 6be185ca0..f8c2fe808 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 b16fe8b70..3d9ee14a7 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -681,11 +681,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.rpc_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) } @@ -700,6 +704,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 2f907f515..b937e87d2 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 { @@ -97,6 +105,17 @@ pub(crate) fn jsonrpc_receive_stream_request(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(), @@ -118,7 +137,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) => { @@ -139,7 +158,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, @@ -168,6 +187,7 @@ enum JsonRpcMessageInfo { fn parse_jsonrpc_message( value: &serde_json::Value, + inspection_mode: JsonRpcInspectionMode, ) -> std::result::Result { let version = value .get("jsonrpc") @@ -178,7 +198,7 @@ fn parse_jsonrpc_message( } if value.get("method").is_some() { - return parse_jsonrpc_call(value).map(JsonRpcMessageInfo::Call); + return parse_jsonrpc_call(value, inspection_mode).map(JsonRpcMessageInfo::Call); } if jsonrpc_response_payload_present(value) { @@ -189,7 +209,10 @@ fn parse_jsonrpc_message( 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()) @@ -197,9 +220,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> { @@ -354,6 +402,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 563094dd0..c43734d37 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,6 +489,133 @@ fn validate_graphql_rule( validate_graphql_fields(errors, warnings, loc, rule.get("fields")); } +fn validate_matcher_map( + errors: &mut Vec, + warnings: &mut Vec, + loc: &str, + value: Option<&serde_json::Value>, +) { + 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 (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_jsonrpc_rule_fields( + errors: &mut Vec, + warnings: &mut Vec, + loc: &str, + rule: &serde_json::Value, + protocol: &str, +) { + 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"; + + if jsonrpc_family { + if rpc_method.is_empty() { + errors.push(format!( + "{loc}.rpc_method: required for {protocol} L7 rules" + )); + } else if let Some(warning) = check_glob_syntax(rpc_method) { + warnings.push(format!("{loc}.rpc_method: {warning}")); + } + validate_matcher_map( + errors, + warnings, + &format!("{loc}.params"), + rule.get("params"), + ); + 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" + )); + } +} + fn json_rule_has_graphql_fields(rule: &serde_json::Value) -> bool { rule.get("operation_type") .and_then(|v| v.as_str()) @@ -611,7 +744,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)" )); } @@ -660,9 +793,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" )); } @@ -882,6 +1018,14 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< } } + 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() @@ -1058,6 +1202,13 @@ 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 websocket_has_graphql_policy && allow @@ -1139,6 +1290,11 @@ pub fn expand_access_presets(data: &mut serde_json::Value) { "full" => vec![graphql_rule_json("*")], _ => continue, } + } else if protocol == "json-rpc" || protocol == "mcp" { + match access.as_str() { + "read-only" | "read-write" | "full" => vec![jsonrpc_rule_json("*")], + _ => continue, + } } else if protocol == "websocket" { match access.as_str() { "read-only" => vec![rule_json("GET", "**")], @@ -1199,6 +1355,14 @@ fn graphql_rule_json(operation_type: &str) -> serde_json::Value { }) } +fn jsonrpc_rule_json(rpc_method: &str) -> serde_json::Value { + serde_json::json!({ + "allow": { + "rpc_method": rpc_method + } + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 6aa3482c7..f781c8dfe 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -178,7 +178,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 + } } } @@ -268,6 +270,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(()); @@ -298,7 +331,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 { @@ -322,7 +355,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) @@ -343,8 +382,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, @@ -352,7 +395,7 @@ where decision_str, engine_type, &reason, - graphql_info.as_ref(), + &protocol_summary, ); let _ = &eval_target; @@ -430,7 +473,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), @@ -445,9 +488,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) @@ -461,13 +501,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); @@ -618,6 +678,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, @@ -911,6 +985,7 @@ where allow_encoded_slash: config.allow_encoded_slash, ..Default::default() }, + jsonrpc_inspection_mode(config.protocol), ) .await { @@ -927,7 +1002,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(()); } @@ -998,7 +1073,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, @@ -1302,11 +1377,18 @@ pub(crate) fn jsonrpc_methods_for_log(info: &crate::l7::jsonrpc::JsonRpcRequestI } 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 3d4fa188c..4af86f972 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -723,6 +723,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); @@ -799,6 +800,148 @@ 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 mut grouped_denies = Vec::new(); + if let Some(rules) = ep.get_mut("rules") { + match rules { + serde_json::Value::Array(items) => { + for rule in items { + if let Some(allow) = rule + .get_mut("allow") + .and_then(serde_json::Value::as_object_mut) + { + normalize_mcp_rule_aliases(allow); + } else if let Some(allow) = rule.as_object_mut() { + normalize_mcp_rule_aliases(allow); + } + } + } + serde_json::Value::Object(groups) => { + let allow_rules = groups + .remove("allow") + .and_then(|v| v.as_array().cloned()) + .unwrap_or_default() + .into_iter() + .map(|mut allow| { + if let Some(allow_obj) = allow.as_object_mut() { + normalize_mcp_rule_aliases(allow_obj); + } + serde_json::json!({ "allow": allow }) + }) + .collect::>(); + let deny_rules = groups + .remove("deny") + .and_then(|v| v.as_array().cloned()) + .unwrap_or_default() + .into_iter() + .map(|mut deny| { + if let Some(deny_obj) = deny.as_object_mut() { + normalize_mcp_rule_aliases(deny_obj); + } + deny + }) + .collect::>(); + *rules = serde_json::Value::Array(allow_rules); + grouped_denies = deny_rules; + } + _ => {} + } + } + + if !grouped_denies.is_empty() { + append_denies(ep, grouped_denies); + } + + 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_mcp_rule_aliases(deny_obj); + } + } + } +} + +fn append_denies( + ep: &mut serde_json::Map, + mut deny_rules: Vec, +) { + match ep.get_mut("deny_rules") { + Some(serde_json::Value::Array(existing)) => existing.append(&mut deny_rules), + Some(_) | None => { + ep.insert( + "deny_rules".to_string(), + serde_json::Value::Array(deny_rules), + ); + } + } +} + +fn normalize_mcp_rule_aliases(rule: &mut serde_json::Map) { + if let Some(mcp_method) = rule.remove("mcp_method") + && rule + .get("rpc_method") + .and_then(serde_json::Value::as_str) + .unwrap_or("") + .is_empty() + { + rule.insert("rpc_method".to_string(), mcp_method); + } + + let Some(tool) = rule.remove("tool") else { + return; + }; + let Some(tool_name) = tool.as_str().filter(|s| !s.is_empty()) else { + return; + }; + 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())); + } +} + /// Resolve a policy binary path through the container's root filesystem. /// /// On Linux, `/proc//root/` provides access to the container's mount @@ -2839,6 +2982,122 @@ network_policies: assert!(!eval_l7(&engine, &blocked_with_args)); } + #[test] + fn l7_mcp_grouped_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: + deny: + - mcp_method: tools/call + tool: blocked_action + allow: + - mcp_method: initialize + - mcp_method: tools/list + - mcp_method: tools/call + tool: read_status + 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 132585462..e837aff12 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -3409,7 +3409,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") @@ -3460,7 +3460,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 @@ -3517,6 +3525,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 1a0705cda..03c9d372d 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -155,12 +155,12 @@ 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`. | -| `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. | +| `rules` | list or grouped object | No | Fine-grained protocol-specific allow rules. For MCP, prefer grouped `rules.allow` and `rules.deny`. 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. Existing policies can keep this shape; new MCP policies should prefer grouped `rules.deny`. | | `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`. | | `websocket_credential_rewrite` | bool | No | When `true` on a `protocol: rest` or `protocol: websocket` endpoint, OpenShell rewrites credential placeholders in client-to-server WebSocket text messages after an allowed HTTP `101` upgrade. Binary frames are relayed but not rewritten. Defaults to `false`. | @@ -168,6 +168,7 @@ 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. @@ -176,13 +177,13 @@ Credential rewrite recognizes the canonical `openshell:resolve:env:KEY` placehol The `access` field accepts one of the following values: -| 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. | Allows matching HTTP requests without constraining JSON-RPC methods. | -| `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | Expands to HTTP read methods and does not allow typical JSON-RPC `POST` calls. | -| `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | Allows matching HTTP write requests without constraining JSON-RPC methods. | +| `full` | All methods and paths. | WebSocket upgrade and all inspected client text-message paths. | All operation types. | `rpc_method: "*"` | +| `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | `rpc_method: "*"` | +| `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | `rpc_method: "*"` | -For JSON-RPC endpoints, prefer explicit `rules` with `rpc_method` and optional `params` when you need method-level control. +For MCP and JSON-RPC endpoints, prefer explicit rules when you need method-level or tool-level control. #### Allow Rule Objects @@ -277,13 +278,53 @@ 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. + +Use grouped `rules.allow` and `rules.deny` for MCP policies. Deny rules take precedence over allow rules. In a batch request, one denied call denies the full batch. + +| Field | Type | Required | Description | +|---|---|---|---| +| `mcp_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 | Params matchers keyed by flattened object-param path. Use dot-separated keys for nested object params, such as `arguments.repository`. Matcher value can be a glob string or an object with `any`. 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: + deny: + - mcp_method: tools/call + tool: send_email + - mcp_method: tools/call + tool: execute_code + allow: + - mcp_method: initialize + - mcp_method: tools/list + - mcp_method: tools/call + tool: search_web + - mcp_method: tools/call + tool: create_issue + params: + arguments.repository: NVIDIA/OpenShell +``` + ##### 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. JSON-RPC responses and server-to-client messages on 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 | |---|---|---|---| -| `rpc_method` | string | Yes | JSON-RPC method name or glob, such as `initialize`, `tools/list`, or `tools/*`. | +| `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 | 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. | Example JSON-RPC allow rules: diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index f0a3464b3..f4c8c905d 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 `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 `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 `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 `mcp_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 `method` field against `rpc_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 `mcp_method`, and can match tool calls with the `tool` alias. -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,35 +595,32 @@ 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: - rpc_method: initialize - - allow: - rpc_method: tools/list - - allow: - rpc_method: tools/call + allow: + - mcp_method: initialize + - mcp_method: tools/list + - mcp_method: tools/call + tool: read_status + - mcp_method: tools/call + tool: submit_report params: - name: read_status - - allow: - rpc_method: tools/call - params: - name: submit_report arguments.scope: workspace/main - deny_rules: - - rpc_method: tools/call - params: - name: delete_resource + deny: + - mcp_method: tools/call + 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 dedd63f20..94d5b7d02 100644 --- a/e2e/mcp-conformance/README.md +++ b/e2e/mcp-conformance/README.md @@ -16,11 +16,15 @@ wrapper rewrites local URLs to `host.openshell.internal`, the alias that `e2e/with-docker-gateway.sh` attaches to the job container on the e2e Docker network. -The generated policy allows valid JSON-RPC requests to the conformance server -with `rpc_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 `mcp_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 @@ -28,5 +32,5 @@ 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. diff --git a/e2e/mcp-conformance/client-through-openshell.sh b/e2e/mcp-conformance/client-through-openshell.sh index ce8d1c180..6b0fe7fd4 100755 --- a/e2e/mcp-conformance/client-through-openshell.sh +++ b/e2e/mcp-conformance/client-through-openshell.sh @@ -116,5 +116,14 @@ export OPENSHELL_E2E_DOCKER_SANDBOX_IMAGE="${OPENSHELL_E2E_DOCKER_SANDBOX_IMAGE: --policy "${POLICY_FILE}" \ "${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 + # The v0.1.16 everything client only lists tools for this scenario; + # test2.ts is the bundled client that calls add_numbers. + case "${MCP_CONFORMANCE_SCENARIO:-}" in + tools_call|tools-call) client=examples/clients/typescript/test2.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 2a02f6374..5104522e1 100644 --- a/e2e/mcp-conformance/policy-template.yaml +++ b/e2e/mcp-conformance/policy-template.yaml @@ -36,18 +36,18 @@ network_policies: - host: ${host} port: ${port} path: ${path} - protocol: json-rpc + protocol: mcp enforcement: enforce allowed_ips: - "10.0.0.0/8" - "172.0.0.0/8" - "192.168.0.0/16" - "fc00::/7" - json_rpc: + mcp: max_body_bytes: 131072 rules: - - allow: - rpc_method: "*" + allow: + - mcp_method: "*" binaries: - path: /bin/sh - path: /usr/bin/env diff --git a/e2e/with-docker-gateway.sh b/e2e/with-docker-gateway.sh index 4c7ccd9ff..4d014a108 100755 --- a/e2e/with-docker-gateway.sh +++ b/e2e/with-docker-gateway.sh @@ -13,6 +13,10 @@ # # HTTPS endpoint-only mode is intentionally unsupported here. Use a named # gateway config when mTLS materials are needed. +# +# 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 @@ -356,32 +360,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 b9862fa18659eec2afe6af1ce986cc11855f04d2 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/10] 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 b937e87d2..434c7764b 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}; @@ -193,7 +196,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}'")); } @@ -213,17 +216,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, @@ -260,24 +262,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( @@ -288,6 +305,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 640c0cb41d4e48021dd657703ba8a4d5429a49b4 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/10] 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 | 77 +++++++-- docs/reference/policy-schema.mdx | 10 +- docs/sandboxes/policies.mdx | 5 +- 5 files changed, 291 insertions(+), 80 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 1afe670e2..a6184357a 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 c43734d37..138281e53 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}")); } } } @@ -2422,6 +2435,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 4af86f972..5c5cc91f7 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -926,22 +926,61 @@ fn normalize_mcp_rule_aliases(rule: &mut serde_json::Map) { + let Some(params) = rule + .get_mut("params") + .and_then(serde_json::Value::as_object_mut) + else { return; }; - let Some(tool_name) = tool.as_str().filter(|s| !s.is_empty()) else { + + 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; }; - 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())); + + 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 @@ -3005,6 +3044,9 @@ network_policies: - mcp_method: tools/list - mcp_method: tools/call tool: read_status + params: + arguments: + scope: workspace/main binaries: - { path: /usr/bin/curl } "#; @@ -3015,10 +3057,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/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 03c9d372d..c90175cbc 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -288,7 +288,7 @@ Use grouped `rules.allow` and `rules.deny` for MCP policies. Deny rules take pre |---|---|---|---| | `mcp_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 | Params matchers keyed by flattened object-param path. Use dot-separated keys for nested object params, such as `arguments.repository`. Matcher value can be a glob string or an object with `any`. Requests with literal `.` characters in params object keys are rejected before policy evaluation. | +| `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: @@ -315,7 +315,8 @@ endpoints: - mcp_method: tools/call tool: create_issue params: - arguments.repository: NVIDIA/OpenShell + arguments: + repository: NVIDIA/OpenShell ``` ##### JSON-RPC Allow Rule (`protocol: json-rpc`) @@ -325,7 +326,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. | -| `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. | +| `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: @@ -351,7 +352,8 @@ endpoints: rpc_method: tools/call params: name: submit_report - arguments.scope: workspace/main + arguments: + scope: workspace/main ``` #### Deny Rule Objects diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index f4c8c905d..9d5ad9139 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -608,7 +608,8 @@ MCP and JSON-RPC endpoint policies currently require full policy YAML applied wi - mcp_method: tools/call tool: submit_report params: - arguments.scope: workspace/main + arguments: + scope: workspace/main deny: - mcp_method: tools/call tool: delete_resource @@ -620,7 +621,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 cb1bd0761e8a1730e4c3a04df1d90ac907e7af8e 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/10] 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 a6184357a..505f32afb 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 434c7764b..0bf74a576 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 has_response: bool, @@ -59,12 +65,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(), @@ -74,6 +88,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()) { @@ -111,6 +127,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) } @@ -188,6 +206,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, @@ -216,6 +236,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); } @@ -264,6 +286,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 @@ -278,6 +303,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 ea2d94214d2c15a73917abec9a39a152ef39f74d 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/10] 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 505f32afb..2a8da24e3 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 0bf74a576..b5ebd4044 100644 --- a/crates/openshell-supervisor-network/src/l7/jsonrpc.rs +++ b/crates/openshell-supervisor-network/src/l7/jsonrpc.rs @@ -302,26 +302,26 @@ fn parse_mcp_call(value: &serde_json::Value) -> std::result::Result Date: Tue, 16 Jun 2026 15:24:02 -0700 Subject: [PATCH 06/10] 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 f781c8dfe..592561173 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2285,6 +2285,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 8c9606b5620eedb039925ecad366df125b314fbf 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/10] fix(mcp): permit streamable http control traffic Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- .../data/sandbox-policy.rego | 28 ++++-- .../src/l7/relay.rs | 54 ++++++------ .../openshell-supervisor-network/src/opa.rs | 86 +++++++++++++++++++ e2e/mcp-conformance/expected-failures.yml | 22 ++++- 4 files changed, 157 insertions(+), 33 deletions(-) diff --git a/crates/openshell-supervisor-network/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index 3d9ee14a7..4e9138f1f 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -435,11 +435,27 @@ request_allowed_for_endpoint(request, endpoint) if { 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" +} + +# The following methodless allowances are a narrow MCP Streamable HTTP +# conformance exception. Receive streams and client JSON-RPC responses do not +# carry a method to match with mcp_method/rpc_method, so enforcement is scoped +# to JSON-RPC-family endpoints after host, path, binary, and parse-shape checks. +# MCP version 2026-07-28 removes the GET stream endpoint and client-sent +# JSON-RPC responses, so these allowances should be version-gated or removed +# once OpenShell enforces that transport version. +# 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) jsonrpc_no_parse_error(request.jsonrpc) @@ -447,9 +463,9 @@ request_allowed_for_endpoint(request, endpoint) if { # MCP clients send JSON-RPC responses (for example elicitation replies) back to # the server without a method. Allow response-only POSTs once endpoint path and -# binary matching has already selected this JSON-RPC endpoint. +# binary matching has already selected this JSON-RPC-family endpoint. request_allowed_for_endpoint(request, endpoint) if { - endpoint.protocol == "json-rpc" + jsonrpc_family_endpoint(endpoint) request.method == "POST" is_object(request.jsonrpc) jsonrpc_no_parse_error(request.jsonrpc) diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 592561173..4afd4da0d 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -271,31 +271,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 5c5cc91f7..f1c49db7f 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -2921,6 +2921,66 @@ network_policies: assert!(!eval_l7(&engine, &deny_input)); } + #[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: + mcp_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": {}, + "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": {}, + "error": null + } + } + }); + assert!(!eval_l7(&engine, &deny_input)); + } + #[test] fn l7_jsonrpc_response_post_is_allowed_for_matching_endpoint() { let data = r#" @@ -2947,6 +3007,32 @@ network_policies: assert!(!eval_l7(&engine, &deny_input)); } + #[test] + fn l7_mcp_response_post_is_allowed_for_matching_endpoint() { + let data = r#" +network_policies: + mcp_response: + name: mcp_response + endpoints: + - host: mcp.response.test + port: 8000 + path: /mcp + protocol: mcp + enforcement: enforce + rules: + - allow: + mcp_method: initialize + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml"); + let allow_input = l7_jsonrpc_response_input("mcp.response.test", 8000, "/mcp"); + assert!(eval_l7(&engine, &allow_input)); + + let deny_input = l7_jsonrpc_response_input("mcp.response.test", 8000, "/other"); + assert!(!eval_l7(&engine, &deny_input)); + } + #[test] fn l7_jsonrpc_params_rules_filter_tools_call() { let data = r#" 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 819a9ba7168a8342ce5d83f4c5eef81982f788de 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/10] fix(mcp): align conformance fixture workarounds Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- e2e/mcp-conformance.sh | 99 +++++++++++++++---- e2e/mcp-conformance/README.md | 14 ++- .../client-through-openshell.sh | 18 +--- e2e/mcp-conformance/expected-failures.yml | 24 +---- 4 files changed, 99 insertions(+), 56 deletions(-) diff --git a/e2e/mcp-conformance.sh b/e2e/mcp-conformance.sh index 1c1143214..d956191f2 100755 --- a/e2e/mcp-conformance.sh +++ b/e2e/mcp-conformance.sh @@ -6,10 +6,10 @@ set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" 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:-}" @@ -43,6 +43,67 @@ checkout_conformance() { git -C "${CONFORMANCE_DIR}" checkout --force --detach FETCH_HEAD } +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 +} + build_conformance_runner() { ( cd "${CONFORMANCE_DIR}" @@ -94,16 +155,16 @@ for (const name of names) { NODE } -client_scenario_for_runner_scenario() { - case "$1" in - elicitation-sep1034-client-defaults) - printf '%s\n' "elicitation-defaults" - ;; - sse-retry) - printf '%s\n' "tools_call" +wrapper_client_supports_runner_scenario() { + local scenario=$1 + local client_file=$2 + + case "${scenario}" in + sse-retry | tools_call | tools-call | elicitation-sep1034-client-defaults) + return 0 ;; *) - printf '%s\n' "$1" + grep -Fxq "${scenario}" "${client_file}" ;; esac } @@ -131,7 +192,7 @@ is_default_mcp_client_scenario() { } list_default_scenarios() { - local runner_file client_file scenario client_scenario runner_scenario runner_only client_only skipped_auth + local runner_file client_file scenario runner_scenario runner_only client_only skipped_auth runner_file="$(mktemp "${TMPDIR:-/tmp}/openshell-mcp-runner-scenarios.XXXXXX")" client_file="$(mktemp "${TMPDIR:-/tmp}/openshell-mcp-client-scenarios.XXXXXX")" @@ -139,8 +200,7 @@ list_default_scenarios() { list_example_client_scenarios >"${client_file}" runner_only="$(while IFS= read -r scenario; do - client_scenario="$(client_scenario_for_runner_scenario "${scenario}")" - if ! grep -Fxq "${client_scenario}" "${client_file}"; then + if ! wrapper_client_supports_runner_scenario "${scenario}" "${client_file}"; then printf '%s\n' "${scenario}" fi done <"${runner_file}")" @@ -162,8 +222,8 @@ list_default_scenarios() { fi skipped_auth="$(while IFS= read -r scenario; do - client_scenario="$(client_scenario_for_runner_scenario "${scenario}")" - if grep -Fxq "${client_scenario}" "${client_file}" && ! is_default_mcp_client_scenario "${scenario}"; then + if wrapper_client_supports_runner_scenario "${scenario}" "${client_file}" && + ! is_default_mcp_client_scenario "${scenario}"; then printf '%s\n' "${scenario}" fi done <"${runner_file}")" @@ -174,8 +234,8 @@ list_default_scenarios() { fi while IFS= read -r scenario; do - client_scenario="$(client_scenario_for_runner_scenario "${scenario}")" - if grep -Fxq "${client_scenario}" "${client_file}" && is_default_mcp_client_scenario "${scenario}"; then + if wrapper_client_supports_runner_scenario "${scenario}" "${client_file}" && + is_default_mcp_client_scenario "${scenario}"; then printf '%s\n' "${scenario}" fi done <"${runner_file}" @@ -237,6 +297,7 @@ main() { echo "MCP conformance spec version: ${SPEC_VERSION}" >&2 checkout_conformance + patch_conformance_clients build_conformance_runner build_client_image run_scenarios "$@" diff --git a/e2e/mcp-conformance/README.md b/e2e/mcp-conformance/README.md index 94d5b7d02..62002d6c3 100644 --- a/e2e/mcp-conformance/README.md +++ b/e2e/mcp-conformance/README.md @@ -26,10 +26,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 6b0fe7fd4..09e1e647b 100755 --- a/e2e/mcp-conformance/client-through-openshell.sh +++ b/e2e/mcp-conformance/client-through-openshell.sh @@ -81,24 +81,13 @@ trap 'rm -f "${POLICY_FILE}"' EXIT CLIENT_SERVER_URL="$(prepare_conformance_target "${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 @@ -118,10 +107,11 @@ export OPENSHELL_E2E_DOCKER_SANDBOX_IMAGE="${OPENSHELL_E2E_DOCKER_SANDBOX_IMAGE: -- \ sh -c ' cd /opt/mcp-conformance - # The v0.1.16 everything client only lists tools for this scenario; - # test2.ts is the bundled client that calls add_numbers. + # 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" 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 6f13d3da78a937a6623502b3ca1233139ec4b4a8 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/10] refactor(policy): align mcp method syntax Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 92 ++++++++++--------- .../data/sandbox-policy.rego | 2 +- .../src/l7/mod.rs | 15 ++- .../src/l7/relay.rs | 8 +- .../openshell-supervisor-network/src/opa.rs | 37 +++++--- docs/reference/policy-schema.mdx | 20 ++-- docs/sandboxes/policies.mdx | 16 ++-- e2e/mcp-conformance/README.md | 2 +- e2e/mcp-conformance/policy-template.yaml | 2 +- 9 files changed, 112 insertions(+), 82 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 2a8da24e3..0fd0a5c0f 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/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index 4e9138f1f..e660c39f3 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -445,7 +445,7 @@ jsonrpc_family_endpoint(endpoint) if { # The following methodless allowances are a narrow MCP Streamable HTTP # conformance exception. Receive streams and client JSON-RPC responses do not -# carry a method to match with mcp_method/rpc_method, so enforcement is scoped +# carry a method to match with method/rpc_method, so enforcement is scoped # to JSON-RPC-family endpoints after host, path, binary, and parse-shape checks. # MCP version 2026-07-28 removes the GET stream endpoint and client-sent # JSON-RPC responses, so these allowances should be version-gated or removed diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 138281e53..518f368cf 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -593,6 +593,12 @@ 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()) @@ -601,12 +607,17 @@ fn validate_jsonrpc_rule_fields( 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}.rpc_method: required for {protocol} L7 rules" + "{loc}.{method_field}: required for {protocol} L7 rules" )); } else if let Some(warning) = check_glob_syntax(rpc_method) { - warnings.push(format!("{loc}.rpc_method: {warning}")); + warnings.push(format!("{loc}.{method_field}: {warning}")); } validate_matcher_map( errors, diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 4afd4da0d..9c214ff51 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2307,12 +2307,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 } diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index f1c49db7f..1f105a30f 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -840,6 +840,11 @@ 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(); let mut grouped_denies = Vec::new(); if let Some(rules) = ep.get_mut("rules") { match rules { @@ -849,9 +854,9 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map) { - if let Some(mcp_method) = rule.remove("mcp_method") +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(), mcp_method); + rule.insert("rpc_method".to_string(), method); } if let Some(tool) = rule.remove("tool") @@ -2935,7 +2944,7 @@ network_policies: enforcement: enforce rules: - allow: - mcp_method: initialize + method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3021,7 +3030,7 @@ network_policies: enforcement: enforce rules: - allow: - mcp_method: initialize + method: initialize binaries: - { path: /usr/bin/curl } "#; @@ -3123,12 +3132,12 @@ network_policies: max_body_bytes: 131072 rules: deny: - - mcp_method: tools/call + - method: tools/call tool: blocked_action allow: - - mcp_method: initialize - - mcp_method: tools/list - - mcp_method: tools/call + - method: initialize + - method: tools/list + - method: tools/call tool: read_status params: arguments: diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index c90175cbc..02808db1a 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -179,9 +179,9 @@ The `access` field accepts one of the following values: | 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. | `rpc_method: "*"` | -| `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | `rpc_method: "*"` | -| `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | `rpc_method: "*"` | +| `full` | All methods and paths. | WebSocket upgrade and all inspected client text-message paths. | All operation types. | MCP `method: "*"` / JSON-RPC `rpc_method: "*"` | +| `read-only` | `GET`, `HEAD`, `OPTIONS`. | WebSocket upgrade handshake only. | `query` operations. | MCP `method: "*"` / JSON-RPC `rpc_method: "*"` | +| `read-write` | `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT`, `PATCH`. | WebSocket upgrade handshake and client text messages. | `query` and `mutation` operations. | MCP `method: "*"` / JSON-RPC `rpc_method: "*"` | For MCP and JSON-RPC endpoints, prefer explicit rules when you need method-level or tool-level control. @@ -286,7 +286,7 @@ Use grouped `rules.allow` and `rules.deny` for MCP policies. Deny rules take pre | Field | Type | Required | Description | |---|---|---|---| -| `mcp_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. | +| `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. | @@ -303,16 +303,16 @@ endpoints: max_body_bytes: 131072 rules: deny: - - mcp_method: tools/call + - method: tools/call tool: send_email - - mcp_method: tools/call + - method: tools/call tool: execute_code allow: - - mcp_method: initialize - - mcp_method: tools/list - - mcp_method: tools/call + - method: initialize + - method: tools/list + - method: tools/call tool: search_web - - mcp_method: tools/call + - method: tools/call tool: create_issue params: arguments: diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 9d5ad9139..f6e1dde9a 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 `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 `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 `mcp_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 `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. @@ -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 `mcp_method`, and can match tool calls with the `tool` alias. +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 `method`, and can match tool calls with the `tool` alias. 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. @@ -601,17 +601,17 @@ MCP and JSON-RPC endpoint policies currently require full policy YAML applied wi max_body_bytes: 131072 rules: allow: - - mcp_method: initialize - - mcp_method: tools/list - - mcp_method: tools/call + - method: initialize + - method: tools/list + - method: tools/call tool: read_status - - mcp_method: tools/call + - method: tools/call tool: submit_report params: arguments: scope: workspace/main deny: - - mcp_method: tools/call + - method: tools/call tool: delete_resource binaries: - { path: /usr/bin/python3 } diff --git a/e2e/mcp-conformance/README.md b/e2e/mcp-conformance/README.md index 62002d6c3..e27e5682b 100644 --- a/e2e/mcp-conformance/README.md +++ b/e2e/mcp-conformance/README.md @@ -17,7 +17,7 @@ wrapper rewrites local URLs to `host.openshell.internal`, the alias that network. The generated policy uses `protocol: mcp` and allows valid MCP requests to the -conformance server with `mcp_method: "*"`. That keeps OpenShell deny-by-default +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. diff --git a/e2e/mcp-conformance/policy-template.yaml b/e2e/mcp-conformance/policy-template.yaml index 5104522e1..5360a4e51 100644 --- a/e2e/mcp-conformance/policy-template.yaml +++ b/e2e/mcp-conformance/policy-template.yaml @@ -47,7 +47,7 @@ network_policies: max_body_bytes: 131072 rules: allow: - - mcp_method: "*" + - method: "*" binaries: - path: /bin/sh - path: /usr/bin/env From da2a2fdf72847a17ed8dc7e2e1ab4eac5375aaef 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/10] refactor(policy): align mcp deny rules Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com> --- crates/openshell-policy/src/lib.rs | 121 ++++-------------- .../src/l7/relay.rs | 16 ++- .../openshell-supervisor-network/src/opa.rs | 89 +++---------- docs/reference/policy-schema.mdx | 29 +++-- docs/sandboxes/policies.mdx | 19 +-- e2e/mcp-conformance/policy-template.yaml | 4 +- 6 files changed, 84 insertions(+), 194 deletions(-) diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 0fd0a5c0f..0bc79ae2e 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 { @@ -635,9 +584,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 +718,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 +1988,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 +2003,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 "; @@ -2121,15 +2053,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 +2073,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-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index 9c214ff51..493377b2c 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -2306,14 +2306,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 1f105a30f..2b1b1b26a 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -845,57 +845,19 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map { - for rule in items { - 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); - } - } - } - serde_json::Value::Object(groups) => { - let allow_rules = groups - .remove("allow") - .and_then(|v| v.as_array().cloned()) - .unwrap_or_default() - .into_iter() - .map(|mut allow| { - if let Some(allow_obj) = allow.as_object_mut() { - normalize_l7_rule_aliases(allow_obj, &protocol); - } - serde_json::json!({ "allow": allow }) - }) - .collect::>(); - let deny_rules = groups - .remove("deny") - .and_then(|v| v.as_array().cloned()) - .unwrap_or_default() - .into_iter() - .map(|mut deny| { - if let Some(deny_obj) = deny.as_object_mut() { - normalize_l7_rule_aliases(deny_obj, &protocol); - } - deny - }) - .collect::>(); - *rules = serde_json::Value::Array(allow_rules); - grouped_denies = deny_rules; + 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 !grouped_denies.is_empty() { - append_denies(ep, grouped_denies); - } - 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() { @@ -905,21 +867,6 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map, - mut deny_rules: Vec, -) { - match ep.get_mut("deny_rules") { - Some(serde_json::Value::Array(existing)) => existing.append(&mut deny_rules), - Some(_) | None => { - ep.insert( - "deny_rules".to_string(), - serde_json::Value::Array(deny_rules), - ); - } - } -} - fn normalize_l7_rule_aliases( rule: &mut serde_json::Map, protocol: &str, @@ -3117,7 +3064,7 @@ network_policies: } #[test] - fn l7_mcp_grouped_rules_filter_tools_call() { + fn l7_mcp_rules_filter_tools_call() { let data = r#" network_policies: mcp_params: @@ -3131,17 +3078,19 @@ network_policies: mcp: max_body_bytes: 131072 rules: - deny: - - method: tools/call - tool: blocked_action - allow: - - method: initialize - - method: tools/list - - method: tools/call + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call tool: read_status params: arguments: scope: workspace/main + deny_rules: + - method: tools/call + tool: blocked_action binaries: - { path: /usr/bin/curl } "#; diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index 02808db1a..15e2f8041 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -159,8 +159,8 @@ 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`. | -| `rules` | list or grouped object | No | Fine-grained protocol-specific allow rules. For MCP, prefer grouped `rules.allow` and `rules.deny`. 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. Existing policies can keep this shape; new MCP policies should prefer grouped `rules.deny`. | +| `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`. | | `websocket_credential_rewrite` | bool | No | When `true` on a `protocol: rest` or `protocol: websocket` endpoint, OpenShell rewrites credential placeholders in client-to-server WebSocket text messages after an allowed HTTP `101` upgrade. Binary frames are relayed but not rewritten. 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. -Use grouped `rules.allow` and `rules.deny` for MCP policies. 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 | |---|---|---|---| @@ -302,21 +302,24 @@ endpoints: mcp: max_body_bytes: 131072 rules: - deny: - - method: tools/call - tool: send_email - - method: tools/call - tool: execute_code - allow: - - method: initialize - - method: tools/list - - method: tools/call + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call tool: search_web - - method: tools/call + - 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`) diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index f6e1dde9a..b77f9ba20 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -600,19 +600,22 @@ MCP and JSON-RPC endpoint policies currently require full policy YAML applied wi mcp: max_body_bytes: 131072 rules: - allow: - - method: initialize - - method: tools/list - - method: tools/call + - allow: + method: initialize + - allow: + method: tools/list + - allow: + method: tools/call tool: read_status - - method: tools/call + - allow: + method: tools/call tool: submit_report params: arguments: scope: workspace/main - deny: - - method: tools/call - tool: delete_resource + deny_rules: + - method: tools/call + tool: delete_resource binaries: - { path: /usr/bin/python3 } ``` diff --git a/e2e/mcp-conformance/policy-template.yaml b/e2e/mcp-conformance/policy-template.yaml index 5360a4e51..eb815f7bb 100644 --- a/e2e/mcp-conformance/policy-template.yaml +++ b/e2e/mcp-conformance/policy-template.yaml @@ -46,8 +46,8 @@ network_policies: mcp: max_body_bytes: 131072 rules: - allow: - - method: "*" + - allow: + method: "*" binaries: - path: /bin/sh - path: /usr/bin/env