diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index f054695a4..56e0badf2 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" + )); + } } } @@ -1629,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 f0654c287..e6197300a 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()) @@ -1493,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 { @@ -7705,4 +7713,144 @@ 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()); + } + + 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 e915c18c9..4ba6353e2 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); } @@ -1436,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#"{