From b41390844adbba3d1657e2be03a4418b9c6bca54 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 06:31:49 -0500 Subject: [PATCH 1/4] fix(supervisor-network): validate L7 endpoint shape and filter zero ports validate_l7_policies assumed every endpoint entry was a JSON object and used ep.as_object_mut().unwrap() in expand_access_presets. A non-object endpoint would panic. Add an object-shape validation error and replace the unwraps with safe pattern matching. Also filter zero values out of the ports array to match the scalar port validation (which already rejects port == 0). Signed-off-by: Andrew White --- .../src/l7/mod.rs | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index f054695a4f..d265232585 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -962,6 +962,11 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< }; for (i, ep) in endpoints.iter().enumerate() { + let loc = format!("{name}.endpoints[{i}]"); + if !ep.is_object() { + errors.push(format!("{loc}: endpoint entry must be an object")); + continue; + } let protocol = ep.get("protocol").and_then(|v| v.as_str()).unwrap_or(""); let l7_protocol = L7Protocol::parse(protocol); let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family); @@ -986,9 +991,13 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< .into_iter() .collect() }, - |arr| arr.iter().filter_map(serde_json::Value::as_u64).collect(), + |arr| { + arr.iter() + .filter_map(serde_json::Value::as_u64) + .filter(|p| *p > 0) + .collect() + }, ); - let loc = format!("{name}.endpoints[{i}]"); if protocol == "mcp" { if host.trim().is_empty() { @@ -1557,7 +1566,13 @@ pub fn expand_access_presets(data: &mut serde_json::Value) -> Vec { && !has_rules && mcp_allow_all_known_mcp_methods { - ep.as_object_mut().unwrap().insert( + let Some(obj) = ep.as_object_mut() else { + warnings.push(format!( + "{name}.endpoints[{i}]: endpoint entry is not an object; skipping access preset expansion" + )); + continue; + }; + obj.insert( "rules".to_string(), serde_json::Value::Array(vec![jsonrpc_rule_json("*")]), ); @@ -1582,9 +1597,13 @@ pub fn expand_access_presets(data: &mut serde_json::Value) -> Vec { continue; }; - ep.as_object_mut() - .unwrap() - .insert("rules".to_string(), serde_json::Value::Array(rules)); + if let Some(obj) = ep.as_object_mut() { + obj.insert("rules".to_string(), serde_json::Value::Array(rules)); + } else { + warnings.push(format!( + "{name}.endpoints[{i}]: endpoint entry is not an object; skipping access preset expansion" + )); + } } } From 583f62ef7019a4c54b23fe91e6ef289c2ca02d4a Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 06:31:49 -0500 Subject: [PATCH 2/4] fix(supervisor-network): drop zero ports in agent-proposal endpoint parser network_endpoint_from_json accepted port 0 from the ports array without filtering, creating an endpoint with no usable ports. Drop zero entries so the array behaves consistently with the scalar port field. Signed-off-by: Andrew White --- crates/openshell-supervisor-network/src/policy_local.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/openshell-supervisor-network/src/policy_local.rs b/crates/openshell-supervisor-network/src/policy_local.rs index e915c18c9b..1b260c49e7 100644 --- a/crates/openshell-supervisor-network/src/policy_local.rs +++ b/crates/openshell-supervisor-network/src/policy_local.rs @@ -1104,6 +1104,7 @@ fn network_endpoint_from_json( } let mut ports = endpoint.ports; + ports.retain(|p| *p > 0); if ports.is_empty() && endpoint.port > 0 { ports.push(endpoint.port); } From 200ecd2f0444b5ef60d33c2332990c546e521dc5 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 28 Jul 2026 14:46:23 -0500 Subject: [PATCH 3/4] fix(supervisor-network): normalize zero ports for OPA and add regression tests Addresses gator-agent review feedback on #2464: - Filter zero values from endpoint ports arrays in normalize_endpoint_ports so OPA never sees a zero port. - Promote positive scalar port to ports array; leave all-zero arrays empty. - Skip non-object endpoints during normalization instead of panicking. - Add regression tests for non-object endpoint validation, zero-port filtering, and scalar fallback. Signed-off-by: Andrew White --- .../src/l7/mod.rs | 28 +++++++ .../openshell-supervisor-network/src/opa.rs | 84 ++++++++++++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index d265232585..56e0badf28 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -1648,6 +1648,34 @@ fn graphql_rule_json(operation_type: &str) -> serde_json::Value { mod tests { use super::*; + #[test] + fn validate_l7_policies_rejects_non_object_endpoint() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [ + "not-an-object", + {"host": "api.example.com", "port": 443, "protocol": "rest"}, + 42, + ], + "binaries": [] + } + } + }); + let (errors, _warnings) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("endpoint entry must be an object")), + "expected non-object endpoint error: {errors:?}" + ); + // The valid object endpoint should not produce an error. + assert!( + !errors.iter().any(|e| e.contains("api.example.com")), + "valid endpoint should not be blamed: {errors:?}" + ); + } + #[test] fn parse_l7_config_rest_enforce() { let val = regorus::Value::from_json_str( diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index f0654c287d..aa3741b9d9 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -1118,7 +1118,13 @@ fn normalize_endpoint_ports(data: &mut serde_json::Value) { continue; }; - // If "ports" already exists and is non-empty, keep it. + // If "ports" already exists, filter out zero values so OPA never + // sees a zero port. An all-zero array becomes empty and falls back + // to scalar "port" promotion below. + if let Some(ports) = ep_obj.get_mut("ports").and_then(|v| v.as_array_mut()) { + ports.retain(|p| p.as_u64().is_some_and(|n| n > 0)); + } + let has_ports = ep_obj .get("ports") .and_then(|v| v.as_array()) @@ -7705,4 +7711,80 @@ network_policies: cmdline_paths: vec![], } } + + #[test] + fn normalize_endpoint_ports_filters_zero_values() { + let mut data = serde_json::json!({ + "network_policies": { + "p": { + "endpoints": [ + {"host": "h1.test", "ports": [0, 443]}, + {"host": "h2.test", "ports": [0]}, + {"host": "h3.test", "port": 0}, + {"host": "h4.test", "port": 8080}, + ] + } + } + }); + normalize_endpoint_ports(&mut data); + let endpoints = data["network_policies"]["p"]["endpoints"] + .as_array() + .unwrap(); + + // Mixed array: zero removed, positive kept. + assert_eq!(endpoints[0]["ports"], serde_json::json!([443])); + // All-zero array: becomes empty, no fallback port. + assert_eq!(endpoints[1]["ports"], serde_json::json!([])); + assert!(endpoints[1].get("port").is_none()); + // Zero scalar port: not promoted, removed. + assert!(endpoints[2].get("ports").is_none()); + assert!(endpoints[2].get("port").is_none()); + // Positive scalar port: promoted to ports array. + assert_eq!(endpoints[3]["ports"], serde_json::json!([8080])); + assert!(endpoints[3].get("port").is_none()); + } + + #[test] + fn normalize_endpoint_ports_skips_non_object_endpoints() { + let mut data = serde_json::json!({ + "network_policies": { + "p": { + "endpoints": [ + "not-an-object", + {"host": "h.test", "port": 443}, + 42, + ] + } + } + }); + normalize_endpoint_ports(&mut data); + let endpoints = data["network_policies"]["p"]["endpoints"] + .as_array() + .unwrap(); + + // Non-object entries are left untouched rather than panicking. + assert_eq!(endpoints[0], serde_json::json!("not-an-object")); + assert_eq!(endpoints[1]["ports"], serde_json::json!([443])); + assert_eq!(endpoints[2], serde_json::json!(42)); + } + + #[test] + fn normalize_endpoint_ports_empty_array_after_filtering() { + let mut data = serde_json::json!({ + "network_policies": { + "p": { + "endpoints": [ + {"host": "h.test", "ports": [0], "port": 8080}, + ] + } + } + }); + normalize_endpoint_ports(&mut data); + let endpoints = data["network_policies"]["p"]["endpoints"] + .as_array() + .unwrap(); + // Zero-only ports array becomes empty; scalar port is promoted. + assert_eq!(endpoints[0]["ports"], serde_json::json!([8080])); + assert!(endpoints[0].get("port").is_none()); + } } From ed4a4b63649a5857e2405a8f72bbb62f99a93282 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 29 Jul 2026 19:46:39 -0500 Subject: [PATCH 4/4] fix(supervisor-network): filter zero ports in production OPA path + parser tests proto_to_opa_data_json cloned e.ports unfiltered, so a policy with ports: [0, 443] still reached OPA with the zero even though the YAML preprocess path normalized it. Filter zero ports in the production path the same way (mixed arrays drop zeros, zero-only arrays fall back to a positive scalar port, zero scalar port is not promoted). Add tests: - proto_to_opa_data_json_filters_zero_ports_in_production_path - proposal_chunks_from_body_filters_mixed_zero_ports - proposal_chunks_from_body_rejects_zero_only_ports - proposal_chunks_from_body_zero_ports_falls_back_to_scalar_port cargo test -p openshell-supervisor-network --lib: 977 passed. Signed-off-by: Andrew White --- .../openshell-supervisor-network/src/opa.rs | 74 ++++++++++++++++++- .../src/policy_local.rs | 52 +++++++++++++ 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index aa3741b9d9..e6197300ab 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -1499,10 +1499,12 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St .endpoints .iter() .map(|e| { - // Normalize port/ports: ports takes precedence, then - // single port promoted to array. Rego always sees "ports". - let ports: Vec = if !e.ports.is_empty() { - e.ports.clone() + // Normalize port/ports: filter zero ports first so OPA + // never sees them, then ports takes precedence over a + // single promoted port. Rego always sees "ports". + let filtered: Vec = e.ports.iter().copied().filter(|&p| p > 0).collect(); + let ports: Vec = if !filtered.is_empty() { + filtered } else if e.port > 0 { vec![e.port] } else { @@ -7787,4 +7789,68 @@ network_policies: assert_eq!(endpoints[0]["ports"], serde_json::json!([8080])); assert!(endpoints[0].get("port").is_none()); } + + fn proto_with_endpoint_ports(port: u32, ports: Vec) -> ProtoSandboxPolicy { + let mut network_policies = std::collections::HashMap::new(); + network_policies.insert( + "p".to_string(), + NetworkPolicyRule { + name: "p".to_string(), + endpoints: vec![NetworkEndpoint { + host: "api.example.com".to_string(), + port, + ports, + ..Default::default() + }], + binaries: vec![], + }, + ); + ProtoSandboxPolicy { + version: 1, + filesystem: None, + landlock: None, + process: None, + network_policies, + network_middlewares: std::collections::HashMap::default(), + } + } + + #[test] + fn proto_to_opa_data_json_filters_zero_ports_in_production_path() { + // Mixed array: zero removed, positive kept. + let proto = proto_with_endpoint_ports(0, vec![0, 443]); + let parsed: serde_json::Value = + serde_json::from_str(&proto_to_opa_data_json(&proto, 0)).unwrap(); + assert_eq!( + parsed["network_policies"]["p"]["endpoints"][0]["ports"], + serde_json::json!([443]) + ); + + // Zero-only array: becomes empty, no fallback to scalar port. + let proto = proto_with_endpoint_ports(0, vec![0]); + let parsed: serde_json::Value = + serde_json::from_str(&proto_to_opa_data_json(&proto, 0)).unwrap(); + assert_eq!( + parsed["network_policies"]["p"]["endpoints"][0]["ports"], + serde_json::json!([]) + ); + + // Zero scalar port: not promoted. + let proto = proto_with_endpoint_ports(0, vec![]); + let parsed: serde_json::Value = + serde_json::from_str(&proto_to_opa_data_json(&proto, 0)).unwrap(); + assert_eq!( + parsed["network_policies"]["p"]["endpoints"][0]["ports"], + serde_json::json!([]) + ); + + // Positive scalar port with zero-only array: scalar promoted. + let proto = proto_with_endpoint_ports(8080, vec![0]); + let parsed: serde_json::Value = + serde_json::from_str(&proto_to_opa_data_json(&proto, 0)).unwrap(); + assert_eq!( + parsed["network_policies"]["p"]["endpoints"][0]["ports"], + serde_json::json!([8080]) + ); + } } diff --git a/crates/openshell-supervisor-network/src/policy_local.rs b/crates/openshell-supervisor-network/src/policy_local.rs index 1b260c49e7..4ba6353e26 100644 --- a/crates/openshell-supervisor-network/src/policy_local.rs +++ b/crates/openshell-supervisor-network/src/policy_local.rs @@ -1437,6 +1437,58 @@ mod tests { ); } + fn proposal_body_with_endpoint_ports(port: u32, ports: serde_json::Value) -> Vec { + serde_json::json!({ + "operations": [ + { + "addRule": { + "ruleName": "ports_case", + "rule": { + "endpoints": [ + { + "host": "api.example.com", + "port": port, + "ports": ports, + } + ], + "binaries": [] + } + } + } + ] + }) + .to_string() + .into_bytes() + } + + #[test] + fn proposal_chunks_from_body_filters_mixed_zero_ports() { + let body = proposal_body_with_endpoint_ports(0, serde_json::json!([0, 443])); + let chunks = proposal_chunks_from_body(&body).unwrap(); + let rule = chunks[0].proposed_rule.as_ref().unwrap(); + assert_eq!(rule.endpoints[0].ports, vec![443]); + assert_eq!(rule.endpoints[0].port, 443); + } + + #[test] + fn proposal_chunks_from_body_rejects_zero_only_ports() { + let body = proposal_body_with_endpoint_ports(0, serde_json::json!([0])); + let err = proposal_chunks_from_body(&body).unwrap_err(); + assert!( + err.contains("endpoint.port or endpoint.ports is required"), + "unexpected error: {err}" + ); + } + + #[test] + fn proposal_chunks_from_body_zero_ports_falls_back_to_scalar_port() { + let body = proposal_body_with_endpoint_ports(8080, serde_json::json!([0])); + let chunks = proposal_chunks_from_body(&body).unwrap(); + let rule = chunks[0].proposed_rule.as_ref().unwrap(); + assert_eq!(rule.endpoints[0].ports, vec![8080]); + assert_eq!(rule.endpoints[0].port, 8080); + } + #[test] fn proposal_chunks_from_body_rejects_query_in_l7_path() { let body = br#"{