fix(supervisor-network): L7 endpoint validation edge cases

crates/openshell-supervisor-network/src/l7/mod.rs

@@ -958,6 +958,11 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, 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);
@@ -982,9 +987,13 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, 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() {
@@ -1537,7 +1546,13 @@ pub fn expand_access_presets(data: &mut serde_json::Value) -> Vec<String> {
                 && !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("*")]),
                 );
@@ -1562,9 +1577,13 @@ pub fn expand_access_presets(data: &mut serde_json::Value) -> Vec<String> {
                 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"
+                ));
+            }
         }
     }
 
@@ -1609,6 +1628,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(
@@ -2830,6 +2877,43 @@ mod tests {
         );
     }
 
+    #[test]
+    fn expand_access_presets_skips_non_object_endpoint_and_expands_valid() {
+        let mut data = serde_json::json!({
+            "network_policies": {
+                "test": {
+                    "endpoints": [
+                        "not-an-object",
+                        {
+                            "host": "api.example.com",
+                            "port": 80,
+                            "protocol": "rest",
+                            "access": "read-only"
+                        },
+                    ],
+                    "binaries": []
+                }
+            }
+        });
+        let warnings = expand_access_presets(&mut data);
+        let endpoints = data["network_policies"]["test"]["endpoints"]
+            .as_array()
+            .unwrap();
+        // Invalid entry is left untouched.
+        assert_eq!(endpoints[0], serde_json::json!("not-an-object"));
+        // Valid preset endpoint is expanded.
+        let rules = endpoints[1]["rules"].as_array().unwrap();
+        assert_eq!(rules.len(), 3);
+        let methods: Vec<&str> = rules
+            .iter()
+            .map(|r| r["allow"]["method"].as_str().unwrap())
+            .collect();
+        assert!(methods.contains(&"GET"));
+        assert!(methods.contains(&"HEAD"));
+        assert!(methods.contains(&"OPTIONS"));
+        assert!(warnings.is_empty(), "expected no warnings: {warnings:?}");
+    }
+
     #[test]
     fn expand_graphql_readonly_preset() {
         let mut data = serde_json::json!({

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

@@ -1272,14 +1272,27 @@ fn normalize_endpoint_ports(data: &mut serde_json::Value) {
                 continue;
             };
 
-            // If "ports" already exists and is non-empty, keep it.
+            // A present "ports" array takes precedence over scalar "port".
+            // Record whether it existed before filtering so an all-zero array
+            // does not silently fall back to the scalar.
+            let ports_was_present = ep_obj.contains_key("ports");
+
+            // If "ports" already exists, filter out numeric zero values so
+            // OPA never sees a zero port. Non-numeric entries are left intact
+            // so downstream validation can fail closed on malformed input
+            // rather than silently dropping it.
+            if let Some(ports) = ep_obj.get_mut("ports").and_then(|v| v.as_array_mut()) {
+                ports.retain(|p| p.as_u64().is_none_or(|n| n != 0));
+            }
+
             let has_ports = ep_obj
                 .get("ports")
                 .and_then(|v| v.as_array())
                 .is_some_and(|a| !a.is_empty());
 
-            if !has_ports {
-                // Promote scalar "port" to "ports" array.
+            if !ports_was_present && !has_ports {
+                // Promote scalar "port" to "ports" array only when no
+                // "ports" key was originally present.
                 let port = ep_obj
                     .get("port")
                     .and_then(serde_json::Value::as_u64)
@@ -1647,10 +1660,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".
+                    // Normalize port/ports: filter zero ports first so OPA
+                    // never sees them, then a present `ports` array takes
+                    // precedence over the scalar `port`. Rego always sees "ports".
+                    let filtered: Vec<u32> = e.ports.iter().copied().filter(|&p| p > 0).collect();
                     let ports: Vec<u32> = if !e.ports.is_empty() {
-                        e.ports.clone()
+                        filtered
                     } else if e.port > 0 {
                         vec![e.port]
                     } else {
@@ -7964,4 +7979,175 @@ 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();
+        // A present "ports" array takes precedence over scalar "port", even
+        // when filtering leaves the array empty.
+        assert_eq!(endpoints[0]["ports"], serde_json::json!([]));
+        assert!(endpoints[0].get("port").is_none());
+    }
+
+    #[test]
+    fn normalize_endpoint_ports_preserves_non_numeric_entries() {
+        let mut data = serde_json::json!({
+            "network_policies": {
+                "p": {
+                    "endpoints": [
+                        {"host": "h.test", "ports": [0, "bad", 443]},
+                    ]
+                }
+            }
+        });
+        normalize_endpoint_ports(&mut data);
+        let endpoints = data["network_policies"]["p"]["endpoints"]
+            .as_array()
+            .unwrap();
+        // Numeric zeros are removed; non-numeric entries are preserved so
+        // downstream validation can fail closed on malformed input.
+        assert_eq!(endpoints[0]["ports"], serde_json::json!(["bad", 443]));
+    }
+
+    fn proto_with_endpoint_ports(port: u32, ports: Vec<u32>) -> 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: present `ports` wins,
+        // so the scalar port is NOT 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!([])
+        );
+
+        // Positive scalar port with positive array: array wins.
+        let proto = proto_with_endpoint_ports(8080, vec![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])
+        );
+    }
 }