feat(scanner): add Azure enterprise resilience security packs#198
feat(scanner): add Azure enterprise resilience security packs#198TFT444 wants to merge 7 commits into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
m-khan-97
left a comment
m-khan-97
left a comment
There was a problem hiding this comment.
Fetched the branch and ran the tests (39 passed) — the scanning logic is sound. The _UNSET caching, tri-state _unsafe (None=unknown→skip, True=finding, False=compliant), the "public endpoint stays public even with an approved private endpoint" thesis, and the data-minimization (states/counts only) are all well done. One concrete issue to fix before merge, plus minor notes.
Blocking: the 15 new rules are invisible to NIST / ISO 27001 / SOC 2 compliance posture.
Each rule's FRAMEWORKS dict declares NIST/ISO/SOC2 mappings (e.g. AZ-BAK-001 → NIST PR.IP-4, ISO A.12.3.1, SOC2 A1.2), but this PR only adds the AZ-FUNC/PE/BAK entries to cis_azure_benchmark.json — not to nist_csf.json, iso27001.json, or soc2.json. I confirmed on the branch: grep -c "AZ-FUNC-\|AZ-PE-\|AZ-BAK-" returns 0 for all three.
That matters because DatabaseManager.get_compliance_score() iterates the framework JSON's controls keys, not the rule's FRAMEWORKS dict (verified in api/models/finding.py:572). So a failing AZ-BAK-001 will count against CIS posture but silently not against NIST/ISO/SOC2 — even though the rule advertises those mappings in its metadata and in the rules-reference table. CI doesn't catch this: the xref check only validates JSON→rule (that every control maps to a rule file), never rule→JSON.
PR #197 handled this correctly — it added AZ-IDN-010..015 to all four framework JSONs. Please mirror that here (add the 15 rules to nist_csf.json, iso27001.json, soc2.json with the control IDs already declared in each rule's FRAMEWORKS), so the compliance rollups match what the rules claim.
Non-blocking notes:
- The FUNC/PE rules inherit the shared-default
NIST=PR.AC-5 / ISO=A.13.1.1 / SOC2=CC6.6from_enterprise_resilience_common.FRAMEWORKSand only override CIS. For e.g. AZ-FUNC-001 (HTTPS-only) a data-in-transit mapping (PR.DS-2 / A.13.2.1) would be more precise than the generic network-controls default. Worth tightening while you're adding the other three JSONs anyway. az_pe_004(service "web") flags both Web Apps and Function Apps since both areMicrosoft.Web/sites, overlapping conceptually with the AZ-FUNC pack — different concern (network vs config) so it's fine, just noting the intentional overlap.
Everything else looks good — happy to re-approve once the framework JSONs are squared up.
Resolves conflicts in cis_azure_benchmark.json (keep both dev's AZ-IDN-010..015 and this branch's AZ-FUNC/AZ-PE/AZ-BAK placeholder entries) and scanner/azure_client.py (keep both dev's identity cache fields and this branch's enterprise resilience cache fields). Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
An earlier git stash/pop during the origin/dev merge silently reverted
several of dev's non-conflicting changes back toward this branch's
pre-merge state, even though the merge itself reported success. This
restores everything that was lost:
- scanner/azure_client.py: enum_str() helper, get_applications(),
get_managed_identity_service_principals(),
get_subscription_role_assignments(), their cache fields, and the
hardened parse_resource_id().
- compliance/frameworks/cis_azure_benchmark.json: dev's AZ-IDN-010..015
placeholder entries.
- scanner/rules/az_idn_006.py, az_db_002.py, az_net_003.py: dev's
refactors to use the new cached accessors and enum_str() instead of
ad hoc Graph calls / naive str().
- tests/helpers/mock_azure.py, tests/test_rules_identity.py,
tests/test_engine_integration.py, tests/test_rules_database.py,
tests/test_rules_network.py: matching test updates.
- .github/workflows/ci.yml: the missing SAST (Semgrep) job and the
correct --cov-fail-under=80 gate.
- Various docs, README, frontend files, and website/content.js that
had reverted to pre-merge wording/values.
Also fixes two bugs an independent review surfaced in this PR's own
new code: get_private_endpoint_posture() and
get_function_app_security_posture() each wrapped multiple independent
Azure API calls in one try/except, so one resource type failing
(e.g. SQL RP not registered) silently discarded every other resource
type's already-collected results. Each fetch is now isolated.
Extends compliance/frameworks/{nist_csf,iso27001,soc2}.json and
docs/rules-reference.md, architecture.md, adding-a-rule.md,
CONTRIBUTING.md, and website/content.js with this PR's 15 new
AZ-BAK/AZ-FUNC/AZ-PE rules, which previously only had CIS mappings —
without NIST/ISO/SOC2 entries their findings were invisible to those
frameworks' compliance rollups.
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Covers the two get_private_endpoint_posture / get_function_app_security_posture fixes from the previous commit: one resource type or one app failing must not discard results already collected for the others. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Per Ibrahim's non-blocking review note: AZ-FUNC-001 (HTTPS-only) and AZ-FUNC-002 (min TLS version) are data-in-transit concerns, not the generic network-boundary default the enterprise resilience rule pack inherits for everything else. Map both to NIST PR.DS-2 / ISO A.13.2.1 instead, matching the existing AZ-STOR-002/AZ-DB-003 pattern. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
|
@TFT444 , as good practises before merging PR, please do update in the chat if you have made modifications to the codebase post the PR requested changes, to move forward |
|
@Vishnu2707 @m-khan-97 Addressed the review:
Also merged the latest dev in and fixed a couple of things found along the way (a merge issue, and two bugs with tests added). CI is green, no conflicts. Ready for another look whenever you get a chance. |
m-khan-97
left a comment
m-khan-97
left a comment
There was a problem hiding this comment.
Re-reviewed after the fixes — the blocking issue is fully resolved. Verified on the branch:
- All 15 rules (AZ-FUNC/PE/BAK) are now present in
nist_csf.json,iso27001.json, andsoc2.json(15 in each), so NIST/ISO/SOC2 compliance posture will now include them. Spot-checked AZ-BAK-001: the rule'sFRAMEWORKS(CIS TBD-BAK-001, NIST PR.IP-4, ISO A.12.3.1, SOC2 A1.2) matches all four JSON control_ids exactly — no drift between what the rule advertises and what the scoring reads. - You also tightened AZ-FUNC-001/002 to data-in-transit controls (PR.DS-2) per my non-blocking note — nice.
- Re-ran the resilience tests: 40 passed (up from 39 — the partial-failure isolation regression test is a good add).
CI green. Thanks for the thorough turnaround. Approving.
3 participants
Summary
Adds enterprise-focused Azure security coverage for issues #194, #195, and #196 in one reviewable PR:
AZ-FUNC-001through005)AZ-PE-001through006)AZ-BAK-001,002,004,006)Security behavior
kind; application settings, keys, credentials, and source code are never requested.blobendpoint group as the universal minimum and does not infer unused subresources.Deliberate first-phase boundary
AZ-BAK-003(production vault not locked) andAZ-BAK-005(LRS without exception) remain deferred because production classification and redundancy exceptions require organization policy. Immutability locking is irreversible and is detection/manual-review only.Dependency Review
Adds official Microsoft package
azure-mgmt-recoveryservices==4.1.0. Its wheel omits license metadata, so Dependency Review receives an exact-package exception; the upstream Azure SDK repository is MIT licensed.Validation
54 passedfocused engine, rule, clean-scan, and CIS mapping tests409 passed, 2 skippedacross tracked tests; one unrelated local Chroma vector-store test cannot run because the local persisted schema is stalegit diff --checkpassCloses #194
Closes #195
Closes #196