feat(scanner): add Azure enterprise resilience security packs - #198
feat(scanner): add Azure enterprise resilience security packs#198TFT444 wants to merge 9 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.
* feat(scanner): add Supply Chain security pack (AZ-SC-001..008) Closes the gap identified in #213: OpenShield had zero rules covering OWASP Top 10:2025's A03 Software Supply Chain Failures category, the highest-incidence category (5.72%) and one enterprise CSPM vendors treat as core coverage. Container Registry hardening (#214): - AZ-SC-001: admin user enabled - AZ-SC-002: public network access enabled - AZ-SC-003: anonymous pull enabled - AZ-SC-004: missing retention/quarantine policy New dependency: azure-mgmt-containerregistry. Terraform/IaC state exposure (#214): - AZ-SC-005: state container publicly readable - AZ-SC-006: state storage account missing versioning/soft delete Reuses the existing azure-mgmt-storage dependency at the per-container level via new AzureClient.get_blob_containers() and get_blob_service_properties() accessors. Azure DevOps pipeline scanning (#215, #216, #217): - New scanner/devops_client.py, reusing the existing service principal credential scoped to Azure DevOps' well-known resource ID rather than a separate stored credential (e.g. a PAT). - AzureClient.devops_client is None when AZURE_DEVOPS_ORG_URL / AZURE_DEVOPS_PROJECT are not configured; the two rules below treat that as not applicable, not an indeterminate failure. - AZ-SC-007: service connection scoped to subscription and shared across every pipeline - AZ-SC-008: service connection uses a stored secret instead of workload identity federation New dependency: azure-devops. All 8 rules are added to all four compliance framework JSONs (CIS, NIST CSF, ISO 27001, SOC 2) with matching control IDs, per the pattern established after PR #198's review. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com> * style: fix ruff format on markdown-embedded Python code blocks CI's ruff (installed fresh, unpinned) is newer than the local dev environment's and now reformats Python code fences inside markdown files. Reformats the embedded examples in CONTRIBUTING.md, docs/adding-a-rule.md, and docs/architecture.md - whitespace only, no content changes. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com> * fix: correct three real logic bugs found in self-review AZ-SC-004: quarantine is a Premium-tier-only ACR feature, but the rule required it on every registry regardless of SKU. Every Basic/Standard registry (the common tier) would permanently fail this check even when correctly hardened for its tier. Now only requires quarantine on Premium registries; retention is still required on all tiers. AZ-SC-007: is_shared means shared with other projects per the Azure DevOps SDK docs, not shared across every pipeline in this project as the rule claimed. Renamed to drop the and Shared framing and flag subscription-scoped connections on their scope alone, which is itself a real, sufficient finding regardless of is_shared. AZ-SC-008: the federated-scheme allowlist only included WorkloadIdentityFederation, missing ManagedServiceIdentity, a second, equally secretless authentication scheme. Service connections using managed identity auth were being incorrectly flagged as using a stored secret. Adds regression tests proving each fix, and updates the two rules compliance framework JSON entries and docs/rules-reference.md to match. Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com> --------- Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
…e-resilience Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com> # Conflicts: # compliance/frameworks/cis_azure_benchmark.json # compliance/frameworks/iso27001.json # compliance/frameworks/nist_csf.json # compliance/frameworks/soc2.json # docs/adding-a-rule.md # docs/architecture.md # docs/rules-reference.md # tests/test_azure_client_management.py
|
@Vishnu2707 I merged the latest dev into #198, resolved all conflicts, preserved both rule sets, and completed local validation successfully. DCO may still fail because it checks legacy commits created before the policy; #232 fixes that issue and is already green. Once #232 is merged, this PR can be rechecked for merge. |
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
|
@Vishnu2707 #198 is updated with the latest dev, all conflicts are resolved, and every CI/security check including DCO is now passing. Please review and merge when ready. |
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