[Varshith-Kali]

Bug

E3688 (SecurityGroupAllToAndFromPorts) validates security group rules against all_to_and_from_ports.json. For IpProtocol "1"/"icmp" the schema only required that FromPort: -1 comes with ToPort: -1; for every other protocol it required both directions to be -1 whenever one was. That two-way branch reported three kinds of rule that EC2 and CloudFormation accept (verified with a real stack in us-west-2 on 2026-10-20):

1. IpProtocol: "-1" with one port set to -1 or absent. EC2 stores every such rule as -1/-1; protocol -1 "allows traffic on all ports, regardless of any port range you specify".
2. icmpv6/"58" with a specific type and ToPort: -1, the ICMPv6 form of the icmp 8/-1 shape allowed since Allow ICMP to have fromport other than -1 with to port -1 #3482.
3. Upper-case "ICMP" with a specific type and ToPort: -1, which missed the case-sensitive enum; EC2 stores it as icmp.

Root cause

The schema's outer if only special-cased "1"/"icmp" (case-sensitive). Everything else, including protocol -1, icmpv6/"58", and "ICMP", fell into the else branch demanding both ports be -1 when one is.

Fix

Restructured all_to_and_from_ports.json into three branches:

• Protocol -1 (int or string): no pairing constraint at all.
• icmp/1 and icmpv6/58, matched case-insensitively via (?i) patterns (a convention already used in this repo's schemas), with type: ["string", "integer"] guards so intrinsic-function values keep their previous behavior: only a wildcard FromPort (-1, all types) requires a wildcard ToPort (-1, all codes), matching the documented ICMP constraint that EC2 also enforces for ICMPv6.
• Everything else keeps the existing two-way check.

The rule description was updated to document the exemptions.

Test evidence

• Reproduced with the exact templates from E3688: false positive on IpProtocol: "-1", icmpv6/"58" and "ICMP" rules with a -1 port #4800 before the fix: template A produced 9 E3688 false positives, template B produced the 4 correct E3688 reports.
• After the fix: template A produces 0 E3688 (only the expected W3687 warnings remain), template B keeps all 4 E3688 reports.
• Extended test/unit/rules/resources/ec2/test_sg_all_to_and_from_ports.py: the 9 existing cases (schema paths updated for the restructure) plus 17 new good/bad cases covering protocol -1 port combinations, icmpv6/"58"/int 58, case variants (ICMP, Icmpv6, ICMPV6), and the still-rejected shapes (tcp/udp with a -1 port, wildcard icmp/icmpv6 type with a specific code). Result: 26 passed.
• Broader regression run: test/unit/rules/resources/ec2/ and test/unit/rules/resources/ectwo/ all pass (133 passed).
• Confirmed intrinsic-function protocols (e.g. {"Ref": "..."}) behave exactly as before the change (no errors either way).

Fixes #4800

[Varshith-Kali]

Hi, the [CI] Test PR / Test PR Data runs on this PR are parked waiting for a maintainer approval (first-time-contributor gate). Could someone approve them when you get a chance? Thanks.

[Varshith-Kali]

Hi @kddejong, the [CI] Test PR and Test PR Data runs here are still parked waiting for a maintainer approval (first-time-contributor gate) after my follow-up push, and your earlier approval now sits on a superseded commit. Could you approve the workflow runs when you get a chance? Thanks.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.67%. Comparing base (2a10fb4) to head (495af03).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4806   +/-   ##
=======================================
  Coverage   94.67%   94.67%           
=======================================
  Files         439      439           
  Lines       15995    15995           
  Branches     3104     3104           
=======================================
  Hits        15144    15144           
  Misses        468      468           
  Partials      383      383           

Flag	Coverage Δ
unittests	94.67% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.