Skip to content

Commit da2a2fd

Browse files
ddurst-nvidiaddurst-nvidia
committed
refactor(policy): align mcp deny rules
Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com>
1 parent 6f13d3d commit da2a2fd

6 files changed

Lines changed: 84 additions & 194 deletions

File tree

‎crates/openshell-policy/src/lib.rs‎

Lines changed: 27 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ struct NetworkEndpointDef {
108108
enforcement: String,
109109
#[serde(default, skip_serializing_if = "String::is_empty")]
110110
access: String,
111-
#[serde(default, skip_serializing_if = "RulesDef::is_empty")]
112-
rules: RulesDef,
111+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
112+
rules: Vec<L7RuleDef>,
113113
#[serde(default, skip_serializing_if = "Vec::is_empty")]
114114
allowed_ips: Vec<String>,
115115
#[serde(default, skip_serializing_if = "Vec::is_empty")]
@@ -195,57 +195,6 @@ struct L7RuleDef {
195195
allow: L7AllowDef,
196196
}
197197

198-
// Preserve the original `rules: [{ allow: ... }]` shape while accepting the
199-
// newer grouped shape (`rules.allow` / `rules.deny`) used by MCP examples.
200-
#[derive(Debug, Serialize, Deserialize)]
201-
#[serde(untagged)]
202-
enum RulesDef {
203-
Legacy(Vec<L7RuleDef>),
204-
Grouped(L7RuleGroupsDef),
205-
}
206-
207-
impl Default for RulesDef {
208-
fn default() -> Self {
209-
Self::Legacy(Vec::new())
210-
}
211-
}
212-
213-
impl RulesDef {
214-
// Serde needs this for `skip_serializing_if`; keeping it on the enum keeps
215-
// call sites from having to know which rules shape was parsed.
216-
fn is_empty(&self) -> bool {
217-
match self {
218-
Self::Legacy(rules) => rules.is_empty(),
219-
Self::Grouped(groups) => groups.allow.is_empty() && groups.deny.is_empty(),
220-
}
221-
}
222-
223-
// The proto model still carries allow rules and deny rules separately. This
224-
// folds both YAML spellings back into that stable internal representation.
225-
fn into_parts(self) -> (Vec<L7RuleDef>, Vec<L7DenyRuleDef>) {
226-
match self {
227-
Self::Legacy(rules) => (rules, Vec::new()),
228-
Self::Grouped(groups) => (
229-
groups
230-
.allow
231-
.into_iter()
232-
.map(|allow| L7RuleDef { allow })
233-
.collect(),
234-
groups.deny,
235-
),
236-
}
237-
}
238-
}
239-
240-
#[derive(Debug, Serialize, Deserialize)]
241-
#[serde(deny_unknown_fields)]
242-
struct L7RuleGroupsDef {
243-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
244-
allow: Vec<L7AllowDef>,
245-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
246-
deny: Vec<L7DenyRuleDef>,
247-
}
248-
249198
#[derive(Debug, Serialize, Deserialize)]
250199
#[serde(deny_unknown_fields)]
251200
struct L7AllowDef {
@@ -635,9 +584,8 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy {
635584
.into_iter()
636585
.map(|e| {
637586
let protocol = e.protocol;
638-
let (allow_rules, grouped_deny_rules) = e.rules.into_parts();
639-
let mut deny_rules = grouped_deny_rules;
640-
deny_rules.extend(e.deny_rules);
587+
let allow_rules = e.rules;
588+
let deny_rules = e.deny_rules;
641589
// Normalize port/ports: ports takes precedence, else
642590
// single port is promoted to ports array.
643591
let normalized_ports: Vec<u32> = if !e.ports.is_empty() {
@@ -770,39 +718,21 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
770718
(clamp(e.ports.first().copied().unwrap_or(e.port)), vec![])
771719
};
772720
let protocol = e.protocol.clone();
773-
let allow_defs: Vec<L7AllowDef> = e
721+
let rules = e
774722
.rules
775723
.iter()
776-
.map(|r| {
777-
allow_proto_to_def(&protocol, r.allow.clone().unwrap_or_default())
724+
.map(|r| L7RuleDef {
725+
allow: allow_proto_to_def(
726+
&protocol,
727+
r.allow.clone().unwrap_or_default(),
728+
),
778729
})
779730
.collect();
780-
let deny_defs: Vec<L7DenyRuleDef> = e
731+
let deny_rules: Vec<L7DenyRuleDef> = e
781732
.deny_rules
782733
.iter()
783734
.map(|d| deny_proto_to_def(&protocol, d))
784735
.collect();
785-
let (rules, deny_rules) = if is_mcp_protocol(&protocol)
786-
&& (!allow_defs.is_empty() || !deny_defs.is_empty())
787-
{
788-
(
789-
RulesDef::Grouped(L7RuleGroupsDef {
790-
allow: allow_defs,
791-
deny: deny_defs,
792-
}),
793-
Vec::new(),
794-
)
795-
} else {
796-
(
797-
RulesDef::Legacy(
798-
allow_defs
799-
.into_iter()
800-
.map(|allow| L7RuleDef { allow })
801-
.collect(),
802-
),
803-
deny_defs,
804-
)
805-
};
806736
let (json_rpc, mcp) = if is_mcp_protocol(&protocol) {
807737
(None, mcp_config_from_proto(e.json_rpc_max_body_bytes))
808738
} else {
@@ -2058,7 +1988,7 @@ network_policies:
20581988
}
20591989

20601990
#[test]
2061-
fn parse_grouped_mcp_rules_to_runtime_fields() {
1991+
fn parse_mcp_rules_to_runtime_fields() {
20621992
let yaml = r"
20631993
version: 1
20641994
network_policies:
@@ -2073,17 +2003,19 @@ network_policies:
20732003
mcp:
20742004
max_body_bytes: 131072
20752005
rules:
2076-
deny:
2077-
- method: tools/call
2078-
tool: send_email
2079-
allow:
2080-
- method: initialize
2081-
- method: tools/list
2082-
- method: tools/call
2006+
- allow:
2007+
method: initialize
2008+
- allow:
2009+
method: tools/list
2010+
- allow:
2011+
method: tools/call
20832012
tool: search_web
20842013
params:
20852014
arguments:
20862015
repository: NVIDIA/OpenShell
2016+
deny_rules:
2017+
- method: tools/call
2018+
tool: send_email
20872019
binaries:
20882020
- path: /usr/bin/curl
20892021
";
@@ -2121,15 +2053,15 @@ network_policies:
21212053
mcp:
21222054
max_body_bytes: 131072
21232055
rules:
2124-
allow:
2125-
- method: tools/call
2056+
- allow:
2057+
method: tools/call
21262058
tool: search_web
21272059
params:
21282060
arguments:
21292061
repository: NVIDIA/OpenShell
2130-
deny:
2131-
- method: tools/call
2132-
tool: send_email
2062+
deny_rules:
2063+
- method: tools/call
2064+
tool: send_email
21332065
binaries:
21342066
- path: /usr/bin/curl
21352067
";
@@ -2141,6 +2073,7 @@ network_policies:
21412073
assert!(yaml_out.contains("method: tools/call"));
21422074
assert!(yaml_out.contains("tool: search_web"));
21432075
assert!(yaml_out.contains("tool: send_email"));
2076+
assert!(yaml_out.contains("deny_rules:"));
21442077
assert!(yaml_out.contains("arguments:"));
21452078
assert!(yaml_out.contains("repository: NVIDIA/OpenShell"));
21462079
assert!(!yaml_out.contains("arguments.repository"));

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2306,14 +2306,16 @@ network_policies:
23062306
mcp:
23072307
max_body_bytes: 131072
23082308
rules:
2309-
deny:
2310-
- method: tools/call
2311-
tool: delete_resource
2312-
allow:
2313-
- method: initialize
2314-
- method: tools/list
2315-
- method: tools/call
2309+
- allow:
2310+
method: initialize
2311+
- allow:
2312+
method: tools/list
2313+
- allow:
2314+
method: tools/call
23162315
tool: read_status
2316+
deny_rules:
2317+
- method: tools/call
2318+
tool: delete_resource
23172319
binaries:
23182320
- { path: /usr/bin/node }
23192321
"#;

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

Lines changed: 19 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -845,57 +845,19 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map<String, serde_json::Value
845845
.and_then(serde_json::Value::as_str)
846846
.unwrap_or("")
847847
.to_string();
848-
let mut grouped_denies = Vec::new();
849-
if let Some(rules) = ep.get_mut("rules") {
850-
match rules {
851-
serde_json::Value::Array(items) => {
852-
for rule in items {
853-
if let Some(allow) = rule
854-
.get_mut("allow")
855-
.and_then(serde_json::Value::as_object_mut)
856-
{
857-
normalize_l7_rule_aliases(allow, &protocol);
858-
} else if let Some(allow) = rule.as_object_mut() {
859-
normalize_l7_rule_aliases(allow, &protocol);
860-
}
861-
}
862-
}
863-
serde_json::Value::Object(groups) => {
864-
let allow_rules = groups
865-
.remove("allow")
866-
.and_then(|v| v.as_array().cloned())
867-
.unwrap_or_default()
868-
.into_iter()
869-
.map(|mut allow| {
870-
if let Some(allow_obj) = allow.as_object_mut() {
871-
normalize_l7_rule_aliases(allow_obj, &protocol);
872-
}
873-
serde_json::json!({ "allow": allow })
874-
})
875-
.collect::<Vec<_>>();
876-
let deny_rules = groups
877-
.remove("deny")
878-
.and_then(|v| v.as_array().cloned())
879-
.unwrap_or_default()
880-
.into_iter()
881-
.map(|mut deny| {
882-
if let Some(deny_obj) = deny.as_object_mut() {
883-
normalize_l7_rule_aliases(deny_obj, &protocol);
884-
}
885-
deny
886-
})
887-
.collect::<Vec<_>>();
888-
*rules = serde_json::Value::Array(allow_rules);
889-
grouped_denies = deny_rules;
848+
if let Some(rules) = ep.get_mut("rules").and_then(|v| v.as_array_mut()) {
849+
for rule in rules {
850+
if let Some(allow) = rule
851+
.get_mut("allow")
852+
.and_then(serde_json::Value::as_object_mut)
853+
{
854+
normalize_l7_rule_aliases(allow, &protocol);
855+
} else if let Some(allow) = rule.as_object_mut() {
856+
normalize_l7_rule_aliases(allow, &protocol);
890857
}
891-
_ => {}
892858
}
893859
}
894860

895-
if !grouped_denies.is_empty() {
896-
append_denies(ep, grouped_denies);
897-
}
898-
899861
if let Some(denies) = ep.get_mut("deny_rules").and_then(|v| v.as_array_mut()) {
900862
for deny in denies {
901863
if let Some(deny_obj) = deny.as_object_mut() {
@@ -905,21 +867,6 @@ fn normalize_l7_rules_aliases(ep: &mut serde_json::Map<String, serde_json::Value
905867
}
906868
}
907869

908-
fn append_denies(
909-
ep: &mut serde_json::Map<String, serde_json::Value>,
910-
mut deny_rules: Vec<serde_json::Value>,
911-
) {
912-
match ep.get_mut("deny_rules") {
913-
Some(serde_json::Value::Array(existing)) => existing.append(&mut deny_rules),
914-
Some(_) | None => {
915-
ep.insert(
916-
"deny_rules".to_string(),
917-
serde_json::Value::Array(deny_rules),
918-
);
919-
}
920-
}
921-
}
922-
923870
fn normalize_l7_rule_aliases(
924871
rule: &mut serde_json::Map<String, serde_json::Value>,
925872
protocol: &str,
@@ -3117,7 +3064,7 @@ network_policies:
31173064
}
31183065

31193066
#[test]
3120-
fn l7_mcp_grouped_rules_filter_tools_call() {
3067+
fn l7_mcp_rules_filter_tools_call() {
31213068
let data = r#"
31223069
network_policies:
31233070
mcp_params:
@@ -3131,17 +3078,19 @@ network_policies:
31313078
mcp:
31323079
max_body_bytes: 131072
31333080
rules:
3134-
deny:
3135-
- method: tools/call
3136-
tool: blocked_action
3137-
allow:
3138-
- method: initialize
3139-
- method: tools/list
3140-
- method: tools/call
3081+
- allow:
3082+
method: initialize
3083+
- allow:
3084+
method: tools/list
3085+
- allow:
3086+
method: tools/call
31413087
tool: read_status
31423088
params:
31433089
arguments:
31443090
scope: workspace/main
3091+
deny_rules:
3092+
- method: tools/call
3093+
tool: blocked_action
31453094
binaries:
31463095
- { path: /usr/bin/curl }
31473096
"#;

‎docs/reference/policy-schema.mdx‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ Each endpoint defines a reachable destination and optional inspection rules.
159159
| `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. |
160160
| `enforcement` | string | No | `enforce` actively blocks disallowed requests. `audit` logs violations but allows traffic through. |
161161
| `access` | string | No | Access preset. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. |
162-
| `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`. |
163-
| `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`. |
162+
| `rules` | list of allow rule objects | No | Fine-grained protocol-specific allow rules. Mutually exclusive with `access`. |
163+
| `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. |
164164
| `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. |
165165
| `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`. |
166166
| `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
282282

283283
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.
284284

285-
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.
285+
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.
286286

287287
| Field | Type | Required | Description |
288288
|---|---|---|---|
@@ -302,21 +302,24 @@ endpoints:
302302
mcp:
303303
max_body_bytes: 131072
304304
rules:
305-
deny:
306-
- method: tools/call
307-
tool: send_email
308-
- method: tools/call
309-
tool: execute_code
310-
allow:
311-
- method: initialize
312-
- method: tools/list
313-
- method: tools/call
305+
- allow:
306+
method: initialize
307+
- allow:
308+
method: tools/list
309+
- allow:
310+
method: tools/call
314311
tool: search_web
315-
- method: tools/call
312+
- allow:
313+
method: tools/call
316314
tool: create_issue
317315
params:
318316
arguments:
319317
repository: NVIDIA/OpenShell
318+
deny_rules:
319+
- method: tools/call
320+
tool: send_email
321+
- method: tools/call
322+
tool: execute_code
320323
```
321324

322325
##### JSON-RPC Allow Rule (`protocol: json-rpc`)

0 commit comments

Comments
 (0)