feat: add VM Scale Set inventory collector and rule AZ-CMP-006

[dipeshrayg]

What does this PR do?

Adds the first VM Scale Set coverage to OpenShield: a new AzureClient.get_virtual_machine_scale_sets() inventory collector, plus AZ-CMP-005, which flags VMSS network interface configurations with a public IP and no NSG.

Type of change

• New scan rule
• Remediation playbook
• Compliance mapping

Rule details

• Rule ID: AZ-CMP-005
• Severity: HIGH
• Category: Compute
• Frameworks mapped: CIS (see note below) / NIST / ISO 27001 / SOC 2

Design notes

Per the issue's guidance, this reuses AZ-CMP-001's detection logic (public IP + no NSG) rather than inventing new logic, applied to the VMSS network profile template instead of a standalone VM's NIC. A VMSS's virtual_machine_profile.network_profile.network_interface_configurations[] embeds ip_configurations[].public_ip_address_configuration and .network_security_group directly (confirmed against the installed azure-mgmt-compute SDK models), so no second collector call (e.g. resolving a NIC by ID) is needed the way AZ-CMP-001 does.

Testing

• Tested against a real Azure free trial subscription — not done, no live subscription available
• Returns correct JSON output — tests/test_rules_compute.py covers compliant (NSG present), compliant (no public IP), non-compliant, missing-profile, and multi-config cases; tests/test_azure_client_management.py covers the new collector's success/failure paths
• All seven CI-equivalent checks reproduced locally and passing: rule syntax, rule structure/duplicate-ID validation, hardcoded-credential scan, playbook existence + bash syntax, compliance JSON validity, API syntax, and compliance↔rule cross-reference
• No hardcoded credentials or secrets
• Full test suite (253 relevant tests) + ruff check pass locally

Related issue

Closes #271

Note on the CIS mapping

CIS 7.1 ("Ensure that Network Security Groups are attached to network interfaces with public IP addresses") is the same real control, already assigned to AZ-CMP-001 under this repo's one-CIS-ID-per-rule convention. Rather than reuse 7.1 or invent a second number CIS doesn't define, AZ-CMP-005 is mapped to N/A-CMP-005, mirroring the pattern already established for AZ-KV-001 (N/A-KV-001, referencing AZ-KV-004's 8.5). NIST/ISO27001/SOC2 do reuse AZ-CMP-001's control IDs directly, consistent with test_cis_benchmark_mapping.py's own stated policy that only CIS enforces one-rule-per-control.

Checklist

• Every commit includes a DCO Signed-off-by trailer (git commit -s)
• My code follows the rule template in CONTRIBUTING.md
• I added or updated the matching CLI playbook
• I added or updated all four compliance framework mappings
• I have not committed any real Azure credentials
• My branch name follows the convention: feat/description

[TFT444]

@dipeshrayg The VMSS work is useful, but it conflicts with #273 because both PRs use AZ-CMP-005. Please rename this rule to AZ-CMP-006 and update the rule, playbook, tests and all framework keys. Also add the missing SOC 2 entry to the rule's FRAMEWORKS mapping and account for subnet-level NSGs so a protected VMSS is not falsely flagged.

[m-khan-97]

Heads-up: AZ-CMP-005 is already claimed by #273 (@shariqueahmad108-ship-it, opened a day before this one) — Trusted Launch (Secure Boot + vTPM) on Gen2 VMs. Both PRs create scanner/rules/az_cmp_005.py at the same path with the same RULE_ID, and both add an AZ-CMP-005 entry to all four compliance framework JSONs.

Whichever merges first will claim the ID cleanly; this one will then hit a git conflict on rebase (new file at an already-existing path) and, if force-resolved, fail CI's duplicate-RULE_ID structure check.

Since this PR's rule is materially different (VMSS network interface exposure, not Trusted Launch), the fix is just a renumber — could you rebase this onto whichever of #273 or dev merges first and bump this rule to AZ-CMP-006 (rule file, playbook, and all four compliance JSON entries)? Happy to take another look once that's done.

[dipeshrayg]

Thanks both for catching this.

• Renumbered to AZ-CMP-006 — rule file, playbook, tests, and all four compliance framework entries (rule filename/path, RULE_ID/RULE_NAME references, playbook header, and JSON keys in cis_azure_benchmark/nist_csf/iso27001/soc2).
• Added the missing SOC2 entry to the rule's own FRAMEWORKS dict (it was only in soc2.json before).
• Fixed the subnet-level NSG gap: the rule now resolves each network interface configuration's subnet via the existing get_virtual_networks() collector and treats either a NIC-level or subnet-level NSG as compliant — no new collector needed, mirrors how AZ-NET-010 already reads subnet.network_security_group. Added two regression tests for this (subnet-protected VMSS returns no findings, VMSS with neither NIC nor subnet NSG still flags).

Verified locally: full test suite (46 compute/client/mapping tests + the two new subnet cases), ruff clean, and all 7 CI-equivalent checks (syntax, structure/dup-ID, credential scan, playbook existence+syntax, compliance JSON validity, API syntax, cross-reference) reproduced and passing — no more AZ-CMP-005 references anywhere in the tree. Rebased is unnecessary since #273 hasn't merged yet and this PR no longer touches AZ-CMP-005 at all, so there's nothing left to conflict on.

[dipeshrayg]

Hey @TFT444 @m-khan-97 — I've pushed the fixes for both of your comments: renumbered to AZ-CMP-006, added the SOC2 mapping, and fixed the subnet-level NSG false positive with new regression tests for it. All CI checks are green. Let me know if there's anything else you'd like changed, happy to take another pass whenever you get a chance to look.

[parthrohit22]

Nice piece of work @dipeshrayg , Went through this in detail and ran the full suite against it (564 passed, 3 skipped, clean ruff). Nice work on the NSG check — you're checking both the NIC-level and subnet-level NSG before flagging, which a lot of first-pass rules like this get wrong (would've been a false-positive magnet otherwise). Also double-checked network_security_group, ip_configurations, public_ip_address_configuration etc. against the actual azure-mgmt-compute/network SDK models — all correct, no attribute typos. AZ-CMP-006 is free, doesn't collide with anything else in flight. Approving. keep it up

Superseded: this metadata mismatch should not have been submitted as a blocking implementation change.

[ritiksah141]

There is one correctness issue that can create false findings in scanner/rules/az_cmp_006.py.

AzureClient.get_virtual_networks() returns [] when the API call fails. _subnet_nsg_map() therefore produces an empty/incomplete map, and this expression treats every unresolved subnet exactly like a successfully resolved subnet with no NSG:

subnet_nsgs.get(subnet_id, False)

That means a VMSS whose subnet really has an NSG will be reported as exposed whenever VNet collection fails, permissions are insufficient, the subnet ID casing differs, or the referenced subnet is otherwise absent from the collected map. This directly reintroduces the subnet-level NSG false positive the latest commit was intended to fix.

Please distinguish “subnet resolved and has no NSG” from “subnet could not be resolved.” An unresolved dependency should not produce a definitive HIGH finding; use the repository's unknown/incomplete-data behavior or safely skip that VMSS/configuration. Normalize Azure resource IDs case-insensitively as well, and add regression tests for an unresolved subnet/VNet collection failure and differently-cased resource IDs.

[dipeshrayg]

Good catch, that's a real regression. Fixed: _subnet_nsg_map and the lookup now distinguish a resolved subnet confirmed to have no NSG from one that couldn't be resolved at all. An unresolved subnet now produces a LOW-severity indeterminate finding instead of a confirmed HIGH, following the same confirmed/indeterminate pattern az_cmp_002.py already uses. Subnet IDs are also normalized to lowercase before comparison now, since the two APIs involved don't guarantee matching casing. Added regression tests for both cases (VNet collection returning empty, and a differently-cased subnet ID match). All 48 relevant tests pass.

[ritiksah141]

Looks good to me, Approving it.

Dismissing — posted before user review

[dipeshrayg]

Rebased this branch onto current dev (it had drifted since 08-19 and picked up a real merge conflict with the JIT-access rule that landed later, AZ-CMP-007). Conflicts were all additive, both rules' entries kept side by side in the four compliance JSON files and in test_rules_compute.py.

One real bug turned up while rebasing: AZ-CMP-007's tests and this PR's AZ-CMP-006 subnet-fix tests each defined their own _subnet_id() helper with a different signature. Landing both in the same file let the second definition silently shadow the first and broke one of AZ-CMP-007's tests. Fixed by renaming this PR's helper to _vnet_subnet_id().

Verified after the rebase: 870/870 tests pass, ruff clean, rule structure check clean (107 rules, no duplicate IDs), no more merge conflict. All 20 CI checks are green on the new head.

The force-push reset the review requirement, so this needs a fresh look even though nothing in the original diff changed, just the base and the one collision fix above.

[dipeshrayg]

Thanks, both blockers and the major issue were real. Fixed:

• Non-primary ip_config subnet false HIGH: subnet status is now only checked for ip_configurations that actually carry a public IP. A non-primary ip_config with no public IP no longer contributes to the finding.
• Suppressed subsequent exposed configs: removed the break. Every non-compliant net_config on a VMSS now produces its own finding.
• Degraded VNet collection with no operator signal: indeterminate findings now carry vnets_collected in metadata, so a persistent zero across many findings is visible as a collection problem rather than reading as an ordinary per-subnet indeterminate result. Full error-vs-empty disambiguation isn't possible without an AzureClient change (list methods return [] on both today, a repo-wide convention, not something specific to this rule), so this is the scoped fix within the rule itself.

Added regression tests for all three: a non-primary ip_config with an unresolved subnet on an otherwise NIC-protected config produces no finding, a VMSS with multiple exposed configs reports every one of them, and an indeterminate finding's metadata reports vnets_collected: 0 when VNet collection returns nothing.

873/873 tests pass, ruff clean, rule structure check clean. Pushed.

[TFT444]

All three blockers from the original TFT444 CHANGES_REQUESTED (rename to AZ-CMP-006, add SOC2, subnet-level NSG) are resolved. The three issues from TFT444's most recent review (non-primary ip_config false positive, break suppressing multi-config findings, VNet failure degradation) are also addressed. One correctness bug remains.

---

[BLOCKER] has_subnet_nsg aggregation creates a false PASS when one public ip_config is protected but another on the same net_config is not

In scanner/rules/az_cmp_006.py, lines 246-248:

subnet_statuses = [_subnet_nsg_status(subnet_nsgs, ip_cfg) for ip_cfg in public_ip_configs]
has_subnet_nsg = any(status is True for status in subnet_statuses)
if has_subnet_nsg:
    continue  # protected at the subnet level, compliant

any(status is True ...) returns True if any public ip_config's subnet has an NSG, then skips the entire net_config as compliant. A net_config can carry multiple public ip_configs referencing different subnets; if subnet A has an NSG and subnet B does not, the net_config is silently skipped despite having an exposed second ip_config.

Fix: evaluate subnet protection per ip_config. A net_config is only compliant at the subnet level if all public ip_configs have a resolved subnet NSG. has_subnet_nsg should use all with a narrowing guard, or the loop should descend to the individual ip_config level. A regression test is also needed: two public ip_configs, subnet A with NSG, subnet B without, expecting one finding.

---

[MINOR] PR title still references AZ-CMP-005

All files are correctly named AZ-CMP-006; the PR title is cosmetic but misleads triage history. Please update via the GitHub UI.

---

Rule ID collision (resolved)

The rename to AZ-CMP-006 fully resolves the conflict with PR #273 (AZ-CMP-005, Trusted Launch). No collision exists; neither az_cmp_005.py nor az_cmp_006.py exist on main today.

---

Items verified as resolved

• Renamed to AZ-CMP-006 throughout (rule file, playbook, framework keys, tests)
• SOC2 CC6.6 entry added
• Subnet-level NSG lookup implemented via _subnet_nsg_map + _subnet_nsg_status
• Unresolved subnet correctly returns None and produces an indeterminate finding rather than a false PASS
• Case-insensitive subnet ID matching via .lower() normalization
• Non-primary ip_config without a public IP excluded from subnet checks
• break removed; every exposed net_config emits its own finding
• VNet collection failure surfaces vnets_collected: 0 in finding metadata

Collector calls compute_client.virtual_machine_scale_sets.list_all() correctly. Playbook exists and passes bash -n. All 4 compliance framework JSONs contain AZ-CMP-006 entries. Engine auto-discovers via glob; no explicit registration needed.

[dipeshrayg]

Good catch, that was real. any(status is True for status in subnet_statuses) let one protected public ip_config wave through a net_config that had another public ip_config still exposed on a different, unprotected subnet.

Fixed: compliance now requires every public ip_config on the net_config to be protected (all(status is True ...)), not just one. A confirmed-unprotected ip_config makes the net_config non-compliant even if another ip_config on it is only unresolved, matching the existing confirmed-beats-indeterminate severity model elsewhere in this rule.

Added the regression test you described: two public ip_configs on different subnets, one protected and one not, expects one confirmed HIGH finding.

Also renamed the PR title to AZ-CMP-006.

996/996 tests pass, ruff clean, rule structure check clean (127 rules, no duplicate IDs). Pushed.