Skip to content

feat: add firewall inventory - #6

Closed
Kampe wants to merge 1 commit into
rvben:mainfrom
Kampe:agent/firewall-inventory
Closed

feat: add firewall inventory#6
Kampe wants to merge 1 commit into
rvben:mainfrom
Kampe:agent/firewall-inventory

Conversation

@Kampe

@Kampe Kampe commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What

Add read-only unifi firewall rules and unifi firewall groups commands with text, JSON, and schema support.

Why

Firewall policy currently requires controller UI inspection. These commands make policy audits and automation possible through the CLI.

Safety

Both legacy resources are decoded into narrow typed records. Unknown vendor fields are discarded; regression fixtures inject secret-like fields and verify they never appear in command output.

Checks

  • env -u UNIFI_API_KEY -u UNIFI_HOST make check
  • Live read-only validation against a UniFi controller (12 rules, 11 groups)

@Kampe
Kampe marked this pull request as ready for review August 14, 2026 02:55
@rvben

rvben commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for the care in the implementation. The typed allowlist, the schema entry, and the secret-leak regression test all match how the rest of the codebase is built.

Worth saying up front: this pointed at something I had not registered. My controller has 65 firewall policies and 6 zones sitting behind an API this CLI cannot see at all, and I only went looking because of your PR. The gap you identified is real and I want to close it. My problem is with the endpoint, not the idea.

On a UCG-Ultra running Network 10.5.67, rest/firewallrule answers {"meta":{"rc":"ok"},"data":[]}. The rules have not gone anywhere; they moved to the Zone-Based Firewall introduced in Network 9:

  • api/s/default/rest/firewallrule gives 0 items
  • v2/api/site/default/firewall-policies gives 65 items
  • v2/api/site/default/firewall/zone gives 6 items

So unifi firewall rules prints 0 firewall rules and exits 0 on a controller that has 65. I expect it works correctly on whatever controller you validated against, so this is version-scoped rather than simply wrong, though I have no pre-9 controller here to confirm that. The trouble is that the legacy endpoint returns rc: ok instead of rejecting the request, so the UnsupportedReason::Removed detection this repo already uses for stat/event never fires, and nothing at any layer signals that the answer is incomplete.

To be precise about scope: firewall groups is fine. I found no v2 group endpoint, and none of the 65 policies reference a group, so the empty group list is almost certainly accurate rather than hidden data. The defect is firewall rules alone.

For most commands I would take this and iterate. For a firewall inventory I would rather not: a confident 0 firewall rules is indistinguishable from the answer someone was hoping for, and it is the one output where being silently empty is worse than not having the command at all.

A version I would take reads the v2 policy and zone endpoints, or detects Network 9+ and returns an explicit unsupported error pointing at them. The types here (ruleset, src_address, dst_address) do not map onto the zone model, which is keyed on zone_id and matching_target with a predefined/custom split, so it is a new implementation rather than a revision of this one. That is why I am closing rather than leaving it open through a redesign.

If you want to build it I am genuinely interested, and I am happy to dump the shape of the v2 policy and zone responses from my controller to save you the discovery work.

@rvben rvben closed this Aug 14, 2026
@rvben

rvben commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Following up on my own offer rather than leaving it conditional, since you shouldn't have to ask.

Here are the v2 shapes from my controller (UCG-Ultra, Network 10.5.67). No obligation attached: I'm posting them because I said I would, and because the discovery work is the tedious part whether you pick this up or I do.

Addresses, object IDs and UUIDs below are placeholders, rewritten to the RFC 5737 documentation ranges. Field names, types and presence counts are verbatim.

Endpoints

GET /proxy/network/v2/api/site/default/firewall-policies   -> 65 items
GET /proxy/network/v2/api/site/default/firewall/zone       ->  6 items

Note the singular zone but plural firewall-policies. That inconsistency is theirs, not a typo here.

First gotcha: v2 returns a bare JSON array, not the {"meta":{"rc":"ok"},"data":[...]} envelope. get_legacy in this repo unwraps data and checks meta.rc, so it can't be reused as-is. This needs a sibling helper that deserializes Vec<T> directly against the /proxy/network/v2/api/site/default prefix.

Zones

All six are UniFi stock, created by default:

zone_key   name       default_zone  attr_no_edit  network_ids
internal   Internal   true          false         4
external   External   true          true          2
gateway    Gateway    true          true          0
vpn        Vpn        true          true          0
hotspot    Hotspot    true          false         0
dmz        Dmz        true          false         0
{
  "_id": "aaaaaaaaaaaaaaaaaaaaaaaa",
  "attr_no_edit": false,
  "cloud_template": null,
  "default_zone": true,
  "external_id": "00000000-0000-0000-0000-000000000000",
  "name": "Internal",
  "network_ids": ["bbbbbbbbbbbbbbbbbbbbbbbb", "cccccccccccccccccccccccc"],
  "zone_key": "internal"
}

zone_key is the stable identifier; name is user-editable. network_ids are _ids from rest/networkconf, which is the same collection networks show (PR #5, now merged) already reads, so resolving zone to network names is available.

Policies

64 predefined, 1 custom. Policies reference zones by _id only, so any readable output needs the zone list fetched first and joined.

The part that will bite a struct definition. Presence is not uniform across the 65:

field present note
hits, last_hit 15/65 only on policies that have actually matched
origin_id 64/65 absent on the custom one
origin_type 1/65
match_ip_sec_type 1/65
source.ips, destination.ips only when matching_target is IP
matching_target_type absent when matching_target is ANY
schedule.repeat_on_days, schedule.time_all_day absent on predefined
connection_states [] on most

So hits/last_hit in particular need Option, and a 0 default there would be exactly the "absent reads as measured zero" problem: a rule that has never matched and a rule whose counter you failed to read are different facts.

A predefined policy:

{
  "_id": "dddddddddddddddddddddddd",
  "action": "ALLOW",
  "connection_state_type": "ALL",
  "connection_states": [],
  "create_allow_respond": true,
  "destination": {
    "match_opposite_ports": false,
    "matching_target": "ANY",
    "port_matching_type": "ANY",
    "zone_id": "aaaaaaaaaaaaaaaaaaaaaaaa"
  },
  "enabled": true,
  "hits": 356176922,
  "icmp_typename": "ANY",
  "icmp_v6_typename": "ANY",
  "index": 2147483647,
  "ip_version": "BOTH",
  "last_hit": 1786739833104,
  "logging": false,
  "match_ip_sec": false,
  "match_opposite_protocol": false,
  "name": "Allow All Traffic",
  "origin_id": "eeeeeeeeeeeeeeeeeeeeeeee",
  "predefined": true,
  "protocol": "all",
  "schedule": { "mode": "ALWAYS" },
  "source": {
    "match_opposite_ports": false,
    "matching_target": "ANY",
    "port_matching_type": "ANY",
    "zone_id": "aaaaaaaaaaaaaaaaaaaaaaaa"
  }
}

The custom one, which is where the IP matching branch shows up:

{
  "_id": "ffffffffffffffffffffffff",
  "action": "ALLOW",
  "connection_state_type": "ALL",
  "connection_states": [],
  "create_allow_respond": false,
  "destination": {
    "ips": ["192.0.2.25"],
    "match_opposite_ips": false,
    "match_opposite_ports": false,
    "matching_target": "IP",
    "matching_target_type": "SPECIFIC",
    "port": "8006",
    "port_matching_type": "SPECIFIC",
    "zone_id": "aaaaaaaaaaaaaaaaaaaaaaaa"
  },
  "enabled": true,
  "icmp_typename": "ANY",
  "icmp_v6_typename": "ANY",
  "index": 10000,
  "ip_version": "IPV4",
  "logging": false,
  "match_ip_sec": false,
  "match_ip_sec_type": "MATCH_IP_SEC",
  "match_opposite_protocol": false,
  "name": "Allow Proxy to Host",
  "predefined": false,
  "protocol": "tcp",
  "schedule": { "mode": "ALWAYS", "repeat_on_days": [], "time_all_day": false },
  "source": {
    "ips": ["198.51.100.210"],
    "match_mac": false,
    "match_opposite_ips": false,
    "match_opposite_ports": false,
    "matching_target": "IP",
    "matching_target_type": "SPECIFIC",
    "port_matching_type": "ANY",
    "zone_id": "aaaaaaaaaaaaaaaaaaaaaaaa"
  }
}

One design note I'd have an opinion on: 64 of 65 being predefined means a bare firewall policies dumping all 65 buries the single rule anyone actually wrote. Some default filter, or at minimum a predefined column, seems necessary for the output to be useful.

On this PR

Your branch is still live on your fork, so reopening this or opening a fresh one are both fine by me, whichever you prefer. There's genuinely no expectation either way, and if you'd rather not take on a rewrite, say so and I'll build it, with the gap credited to your report here.

I don't have a pre-9 controller, so if the legacy endpoint your original version reads is still populated on yours, I'd be interested to know. It would mean a complete implementation needs both paths rather than a straight replacement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants