fix(ec2): NACL rule-number precedence + reject out-of-range NetworkPolicy ports

crates/fakecloud-ec2/src/runtime/firewall.rs

@@ -63,10 +63,14 @@ pub struct InstanceRules {
     pub egress: Vec<FirewallRule>,
 }
 
-/// A subnet-level NACL entry (allow/deny, ordered by rule number by the
-/// caller). NACLs are stateless and apply to the whole subnet.
+/// A subnet-level NACL entry. NACLs are stateless and apply to the whole
+/// subnet; AWS evaluates them in ascending `rule_number` order, first match
+/// wins (so a lower-numbered `allow` shadows a higher-numbered `deny` for the
+/// same traffic).
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct NaclRule {
+    /// AWS rule number; lower numbers evaluate first.
+    pub rule_number: i64,
     pub egress: bool,
     /// True = allow, false = deny.
     pub allow: bool,
@@ -114,8 +118,26 @@ pub fn render_ruleset(subnets: &[SubnetFirewall]) -> String {
     for subnet in subnets {
         out.push_str(&format!("    # subnet {}\n", subnet.network_name));
 
-        // Subnet-wide NACL denies first (stateless, highest precedence).
-        for rule in subnet.nacl.iter().filter(|r| !r.allow) {
+        // Subnet-wide NACL denies, evaluated in ascending rule-number order so
+        // a lower-numbered `allow` shadows a higher-numbered `deny` for the
+        // same traffic (AWS first-match semantics). A deny is emitted as a drop
+        // only when no earlier-numbered allow covers the identical
+        // direction/protocol/ports/CIDR — otherwise the allow wins and the deny
+        // never fires (bug-hunt 2026-06-18 finding 1.4). NACL allows ride the
+        // default-accept policy (the SG layer below still applies; NACL and SG
+        // are independent gates, both must permit).
+        let mut ordered = subnet.nacl.clone();
+        ordered.sort_by_key(|r| r.rule_number);
+        for (i, rule) in ordered.iter().enumerate() {
+            if rule.allow {
+                continue;
+            }
+            let shadowed = ordered[..i]
+                .iter()
+                .any(|earlier| earlier.allow && nacl_same_traffic(earlier, rule));
+            if shadowed {
+                continue;
+            }
             if let Some(line) = render_nacl_drop(rule) {
                 out.push_str(&format!("    {line}\n"));
             }
@@ -182,6 +204,19 @@ fn render_rule(rule: &FirewallRule, dir: Direction, instance_ip: &str) -> String
     parts.join(" ")
 }
 
+/// Whether two NACL entries match the *same* traffic (same direction,
+/// protocol, port range, CIDR) — used to decide when a lower-numbered allow
+/// shadows a higher-numbered deny. Conservative: only exact matches shadow, so
+/// partially-overlapping rules still emit their drop (safer to over-deny than
+/// to silently allow).
+fn nacl_same_traffic(a: &NaclRule, b: &NaclRule) -> bool {
+    a.egress == b.egress
+        && a.protocol == b.protocol
+        && a.from_port == b.from_port
+        && a.to_port == b.to_port
+        && a.cidr == b.cidr
+}
+
 /// Render a NACL deny as a drop line scoped to its direction + match. Returns
 /// `None` for an allow rule (allows are the default-accept policy; only denies
 /// need an explicit line).
@@ -407,6 +442,7 @@ mod tests {
                 egress: vec![],
             }],
             nacl: vec![NaclRule {
+                rule_number: 100,
                 egress: false,
                 allow: false,
                 protocol: "tcp".into(),
@@ -428,6 +464,44 @@ mod tests {
         assert!(!rs.contains("nacl-allow"));
     }
 
+    #[test]
+    fn nacl_lower_numbered_allow_shadows_higher_numbered_deny() {
+        // AWS first-match-by-rule-number: `100 allow tcp/22 10/8` must win over
+        // `200 deny tcp/22 10/8`, so the deny is NOT emitted (finding 1.4).
+        let nacl_entry = |rule_number, allow| NaclRule {
+            rule_number,
+            egress: false,
+            allow,
+            protocol: "tcp".into(),
+            from_port: 22,
+            to_port: 22,
+            cidr: Some("10.0.0.0/8".into()),
+        };
+        let model = vec![SubnetFirewall {
+            network_name: "fakecloud-subnet-a".into(),
+            instances: vec![InstanceFirewall {
+                private_ip: "172.30.0.2".into(),
+                ingress: vec![],
+                egress: vec![],
+            }],
+            // Intentionally out of order to exercise the sort.
+            nacl: vec![nacl_entry(200, false), nacl_entry(100, true)],
+        }];
+        let rs = render_ruleset(&model);
+        assert!(
+            !rs.contains("ip saddr 10.0.0.0/8 tcp dport 22 drop"),
+            "a lower-numbered allow must shadow the deny:\n{rs}"
+        );
+
+        // Reverse the precedence: deny 100 before allow 200 -> deny fires.
+        let model2 = vec![SubnetFirewall {
+            network_name: "fakecloud-subnet-a".into(),
+            instances: vec![],
+            nacl: vec![nacl_entry(100, false), nacl_entry(200, true)],
+        }];
+        assert!(render_ruleset(&model2).contains("ip saddr 10.0.0.0/8 tcp dport 22 drop"));
+    }
+
     #[test]
     fn enforcement_mode_is_opt_in_and_capability_gated() {
         // not opted in -> disabled regardless of nft availability
@@ -479,6 +553,7 @@ mod tests {
         nacls.insert(
             "net-a".to_string(),
             vec![NaclRule {
+                rule_number: 100,
                 egress: false,
                 allow: false,
                 protocol: "-1".into(),

crates/fakecloud-ec2/src/runtime/netpolicy.rs

@@ -133,7 +133,12 @@ fn ports(r: &FirewallRule) -> Option<Vec<NetworkPolicyPort>> {
         // port -> leave ports unset (all ports of all protocols).
         _ => return None,
     };
-    if r.from_port < 0 || r.to_port < 0 {
+    // Ports are parsed permissively as i64; a value outside the valid
+    // 0..=65535 TCP/UDP port range can't be a real port. Reject the whole rule
+    // rather than silently truncating via `as i32` into a wrong/negative port
+    // (bug-hunt 2026-06-18 finding 2.1).
+    let valid = |p: i64| (0..=65535).contains(&p);
+    if !valid(r.from_port) || !valid(r.to_port) {
         return None;
     }
     let mut port = NetworkPolicyPort {
@@ -285,6 +290,24 @@ mod tests {
         assert_eq!(port.end_port, Some(8100));
     }
 
+    #[test]
+    fn out_of_range_port_is_rejected_not_truncated() {
+        // A port > 65535 (parsed permissively as i64) must not silently
+        // truncate via `as i32` into a wrong/negative port (finding 2.1).
+        let huge = FirewallRule {
+            protocol: "tcp".into(),
+            from_port: 70000,
+            to_port: 70000,
+            cidr: None,
+        };
+        let r = rules("i-1", vec![huge], vec![]);
+        let p = &build_policies(&[r], "fakecloud", "x")[0];
+        // No ports clause -> the rule isn't narrowed to a bogus port.
+        assert!(p.spec.as_ref().unwrap().ingress.as_ref().unwrap()[0]
+            .ports
+            .is_none());
+    }
+
     #[test]
     fn referenced_member_ip_becomes_ipblock() {
         let r = rules("i-1", vec![tcp(80, Some("172.30.0.3/32"))], vec![]);

crates/fakecloud-ec2/src/service/firewall_model.rs

@@ -54,15 +54,17 @@ fn flatten_rule(
 }
 
 /// NACL deny/allow entries that apply to a subnet, as the renderer's model.
-/// (Only denies become explicit nft lines; allows ride the default-accept
-/// policy. We still pass both so future ordering work has the full picture.)
+/// Both allows and denies are carried with their `rule_number` so the renderer
+/// can apply AWS first-match-by-rule-number precedence (allow shadows a
+/// higher-numbered deny).
 fn nacl_rules(acl: &NetworkAcl) -> Vec<NaclRule> {
     acl.entries
         .iter()
         // The catch-all `*` deny (rule 32767) is the implicit policy, not a
         // user rule — skip it so it doesn't blackhole everything.
         .filter(|e| e.rule_number != 32767)
         .map(|e| NaclRule {
+            rule_number: e.rule_number,
             egress: e.egress,
             allow: e.rule_action == "allow",
             protocol: e.protocol.clone(),