fixes

Author: Suresh Mandalapu

cleancloud/providers/azure/scan.py

@@ -59,6 +59,7 @@ class SubscriptionScanResult:
     status: str  # "success" | "failed"
     findings: List[Finding] = field(default_factory=list)
     skipped_rules: List[dict] = field(default_factory=list)
+    rules_failed: int = 0
     error: Optional[str] = None
 
     @property
@@ -255,14 +256,15 @@ def scan_azure_subscriptions(
                 sub_id = futures[future]
                 sub_name = sub_name_map.get(sub_id, sub_id)
                 try:
-                    sub_findings, sub_skipped = future.result()
+                    sub_findings, sub_skipped, sub_rules_failed = future.result()
                     results.append(
                         SubscriptionScanResult(
                             subscription_id=sub_id,
                             subscription_name=sub_name,
                             status="success",
                             findings=sub_findings,
                             skipped_rules=sub_skipped,
+                            rules_failed=sub_rules_failed,
                         )
                     )
                 except Exception as e:
@@ -288,7 +290,7 @@ def _scan_azure_subscription(
     credential,
     region_filter: Optional[str],
     rules: Optional[List[Callable]] = None,
-) -> Tuple[List[Finding], List[dict]]:
+) -> Tuple[List[Finding], List[dict], int]:
     findings: List[Finding] = []
     skipped_rules: List[dict] = []
     rules_succeeded = 0
@@ -370,4 +372,4 @@ def _scan_azure_subscription(
             f"This indicates a serious configuration or permissions issue."
         )
 
-    return findings, skipped_rules
+    return findings, skipped_rules, rules_failed

cleancloud/scan/command.py

@@ -729,6 +729,9 @@ def scan(
             summary["total_rules"] = len(azure_rules_to_run)
             summary["subscription_selection_mode"] = subscription_selection_mode
             summary["subscriptions_scanned"] = subscriptions_scanned
+            total_rules_failed = sum(r.rules_failed for r in azure_sub_results)
+            if total_rules_failed > 0:
+                summary["rules_failed"] = total_rules_failed
             failed_subs = [r for r in azure_sub_results if r.status == "failed"]
             if failed_subs:
                 summary["subscriptions_failed"] = [
@@ -745,6 +748,7 @@ def scan(
                     "name": r.subscription_name,
                     "status": r.status,
                     "findings": len(r.findings),
+                    "rules_failed": r.rules_failed,
                     "estimated_monthly_cost_usd": round(r.estimated_monthly_cost, 2),
                 }
                 for r in sorted(azure_sub_results, key=lambda r: r.subscription_name)
@@ -776,13 +780,17 @@ def scan(
                     }
                     for r in sorted(gcp_project_results, key=lambda r: r.project_name)
                 ]
-        # Build rules_evaluated: {rule_id: finding_count} for all rules that ran
+        # Build rules_evaluated: {rule_id: finding_count} for all rules that ran.
+        # Unwrap functools.partial objects so parameterized rules still appear in the display.
         if provider == "aws":
-            _active_rule_map = {v: k for k, v in aws_rule_map.items() if v in aws_rules_to_run}
+            _active_funcs = {getattr(r, "func", r) for r in aws_rules_to_run}
+            _active_rule_map = {v: k for k, v in aws_rule_map.items() if v in _active_funcs}
         elif provider == "azure":
-            _active_rule_map = {v: k for k, v in azure_rule_map.items() if v in azure_rules_to_run}
+            _active_funcs = {getattr(r, "func", r) for r in azure_rules_to_run}
+            _active_rule_map = {v: k for k, v in azure_rule_map.items() if v in _active_funcs}
         else:
-            _active_rule_map = {v: k for k, v in gcp_rule_map.items() if v in gcp_rules_to_run}
+            _active_funcs = {getattr(r, "func", r) for r in gcp_rules_to_run}
+            _active_rule_map = {v: k for k, v in gcp_rule_map.items() if v in _active_funcs}
         _findings_by_rule = Counter(f.rule_id for f in findings)
         summary["rules_evaluated"] = {
             rule_id: _findings_by_rule.get(rule_id, 0)

docs/configuration.md

@@ -177,11 +177,13 @@ See [rules.md](rules.md) for the full list of rule IDs and their supported param
 | `idle_days` | `azure.ml.compute_instance.idle` | 14 | Days since last control-plane activity before flagging |
 | `idle_days` | `azure.sql.database.idle` | 14 | Days of no connections before flagging |
 | `idle_days` | `azure.app_service.idle` | 14 | Days of zero requests before flagging |
+| `days_unused` | `azure.container_registry.unused` | 90 | Days with zero successful pulls and pushes before flagging |
 | `max_age_days` | `aws.ec2.ami.old` | 180 | Age in days before flagging |
 | `max_age_days` | `aws.ebs.snapshot.old` | 90 | Age in days before flagging |
 | `max_age_days` | `aws.rds.snapshot.old` | 90 | Age in days before flagging |
 | `max_age_days` | `aws.ec2.eni.detached` | 60 | Age in days before flagging |
 | `max_age_days` | `aws.ec2.instance.stopped` | 30 | Days stopped before flagging |
+| `max_age_days` | `azure.compute.snapshot.old` | 90 | Age in days for the higher-confidence snapshot review band |
 | `max_age_days` | `gcp.compute.snapshot.old` | 90 | Age in days before flagging |
 | `max_age_days` | `gcp.compute.vm.stopped` | 30 | Days stopped before flagging |
 | `days_unattached` | `aws.ec2.elastic_ip.unattached` | 30 | Days unattached before flagging |

docs/example-outputs.md

@@ -751,19 +751,20 @@ Found 7 hygiene issues:
      - days_idle_threshold: 14
      - subscription: Production
 
-6. [AZURE] Unused Container Registry (90+ Days No Pulls)
+6. [AZURE] Unused Container Registry (90+ Days No Pulls or Pushes)
    Risk       : Low
    Confidence : High
    Resource   : azure.container_registry → /subscriptions/.../registries/acr-old-project
    Region     : eastus
    Rule       : azure.container_registry.unused
-   Reason     : Container registry has zero pull activity for 90+ days
+   Reason     : SuccessfulPullCount and SuccessfulPushCount both evaluated to ZERO over a 90-day window
    Detected   : 2026-02-08T14:45:18+00:00
    Details:
-     - registry_name: acr-old-project
-     - sku: Standard
-     - days_unused_threshold: 90
-     - subscription: Staging
+      - registry_name: acr-old-project
+      - sku: Standard
+      - created_at: 2025-08-01T00:00:00+00:00
+      - days_unused_threshold: 90
+      - subscription: Staging
 
 7. [AZURE] Untagged Resource
    Risk       : Low
@@ -1272,31 +1273,32 @@ Scanned at: 2026-02-08T14:45:19+00:00
       "resource_type": "azure.container_registry",
       "resource_id": "/subscriptions/f9e8d7c6-b5a4-3210-fedc-ba0987654321/resourceGroups/rg-staging/providers/Microsoft.ContainerRegistry/registries/acr-old-project",
       "region": "eastus",
-      "title": "Unused Container Registry (90+ Days No Pulls)",
-      "summary": "Container Registry 'acr-old-project' (Standard) has had no image pulls for 90+ days.",
-      "reason": "Container registry has zero pull activity for 90+ days",
+      "title": "Unused Container Registry (90+ Days No Pulls or Pushes)",
+      "summary": "Container Registry 'acr-old-project' (Standard) has had no successful pulls or pushes for 90+ days.",
+      "reason": "SuccessfulPullCount and SuccessfulPushCount both evaluated to ZERO over a 90-day window",
       "risk": "low",
       "confidence": "high",
       "detected_at": "2026-02-08T14:45:18+00:00",
       "details": {
         "registry_name": "acr-old-project",
         "sku": "Standard",
         "location": "eastus",
+        "created_at": "2025-08-01T00:00:00+00:00",
         "days_unused_threshold": 90
       },
       "estimated_monthly_cost_usd": 20.0,
       "evidence": {
         "signals_used": [
-          "Zero successful image pulls for 90 days (Azure Monitor: SuccessfulPullCount)",
-          "Zero successful image pushes for 90 days (Azure Monitor: SuccessfulPushCount)",
-          "No push or pull activity detected across the entire 90-day window",
+          "Registry creation date satisfies properties.creationDate <= window_start",
+          "SuccessfulPullCount and SuccessfulPushCount both evaluated to ZERO for the 90-day window",
           "Registry SKU: Standard",
           "ACR Standard tier costs ~$20/month plus storage"
         ],
         "signals_not_checked": [
-          "Geo-replication pull activity in other regions",
-          "Planned reactivation or migration",
-          "Images referenced by stopped but not deleted workloads"
+          "Planned reactivation or migration intent",
+          "Images referenced by stopped or undeployed workloads",
+          "Failed pull or login attempts not treated as active use",
+          "Storage charges not included in estimated base monthly cost"
         ],
         "time_window": "90 days"
       }

docs/rules.md

@@ -63,15 +63,15 @@ Every finding includes a confidence level:
 |---|---|---|
 | `azure.vm.stopped_not_deallocated` | Compute | Stopped but not deallocated VMs (full charges) |
 | `azure.compute.disk.unattached` | Storage | Managed disks not attached to any VM |
-| `azure.compute.snapshot.old` | Storage | Snapshots older than 30–90 days |
+| `azure.compute.snapshot.old` | Storage | Old managed snapshots as conservative review candidates |
 | `azure.network.public_ip.unused` | Network | Public IPs not attached to any interface |
 | `azure.load_balancer.no_backends` | Network | Standard LBs with zero backend members |
 | `azure.application_gateway.no_backends` | Network | App Gateways with zero backend targets |
 | `azure.virtual_network_gateway.idle` | Network | VPN/ExpressRoute Gateways with no connections |
 | `azure.app_service_plan.empty` | Platform | Paid App Service Plans with zero apps |
 | `azure.app_service.idle` | Platform | App Services with zero HTTP requests 14+ days |
 | `azure.sql.database.idle` | Platform | Azure SQL databases with zero connections 14+ days |
-| `azure.container_registry.unused` | Platform | Container registries with no pulls 90+ days |
+| `azure.container_registry.unused` | Platform | Container registries with zero successful pulls and pushes 90+ days |
 | `azure.resource.untagged` | Governance | Disks and snapshots with zero tags |
 | `azure.aml.compute.idle` | AI/ML | AML compute clusters with min_node_count > 0 and no active nodes 14+ days *(opt-in: `--category ai`)* |
 | `azure.ml.compute_instance.idle` | AI/ML | Azure ML Compute Instances Running with no control-plane activity 14+ days *(opt-in: `--category ai`)* |
@@ -1266,35 +1266,45 @@ for disk in disks.list():
 
 **Rule ID:** `azure.compute.snapshot.old`
 
-**What it detects:** Snapshots older than configured thresholds
+**What it detects:** Old managed snapshots that meet the conservative review threshold (default: 30 days) and are surfaced as review candidates only
 
 **Confidence:**
 
 Confidence thresholds and signal weighting are documented in [confidence.md](confidence.md).
 
-- **MEDIUM:** Age ≥ 30 days (conservative for all ages — age alone is a moderate signal)
+- **LOW:** Age ≥ 30 days and < `max_age_days` (default 90) — conservative review candidate
+- **MEDIUM:** Age ≥ `max_age_days` (default 90) — very old snapshot, higher review priority
 - Not flagged: < 30 days
+- `HIGH` is never used — age alone cannot establish HIGH confidence
+
+**Configurable params:**
+- `max_age_days` (default: `90`) — age threshold for the MEDIUM confidence band
 
 **Detection logic:**
 ```python
 for snapshot in snapshots.list():
+    if not snapshot.id or snapshot.provisioning_state != "Succeeded":
+        continue
+    if snapshot.completion_percent is not None and snapshot.completion_percent < 100:
+        continue
     age_days = (now - snapshot.time_created).days
-    if age_days >= 90:
-        confidence = "MEDIUM"  # conservative even at high age
-    elif age_days >= 30:
-        confidence = "MEDIUM"
-    else:
-        continue  # too new to flag
+    if age_days < 30:
+        continue
+    confidence = "MEDIUM" if age_days >= max_age_days else "LOW"
 ```
 
+**Cost model:** `estimated_monthly_cost_usd` is always `None`. Azure bills snapshots on **used size**, not `diskSizeGB`, so no per-snapshot cost estimate is possible from the API response alone.
+
 **Limitations:**
-- Does NOT check if snapshot is referenced by images
-- Conservative to avoid false positives
+- Age alone does not prove a snapshot is unused, orphaned, or safe to delete
+- Does not check backup ownership, DR retention intent, or application restore references
+- If Azure surfaces `completionPercent`, incomplete background-copy snapshots are skipped conservatively
+- Conservative by design — flags review candidates only
 
 **Common causes:**
-- Snapshots from backup jobs
+- Snapshots from backup jobs retained beyond their useful life
 - Over-retention without lifecycle policies
-- Snapshots from deleted disks
+- Snapshots from deleted or migrated disks
 
 **Required permission:** `Microsoft.Compute/snapshots/read`
 
@@ -1658,7 +1668,7 @@ Cost assumes one instance. Scaled-out plans (multiple instances) will cost propo
 
 **Rule ID:** `azure.container_registry.unused`
 
-**What it detects:** Container registries with zero image pulls for 90+ days (default, configurable)
+**What it detects:** Container registries with zero **successful** pulls and pushes for 90+ days (default, configurable), after the registry is old enough to cover the full inactivity window
 
 **Confidence:**
 
@@ -1676,15 +1686,19 @@ Confidence thresholds and signal weighting are documented in [confidence.md](con
 **Detection logic:**
 ```python
 for registry in registries.list():
-    if registry.provisioning_state == "Succeeded":
-        pulls = monitor.metrics("SuccessfulPullCount", period=days_unused)
-        pushes = monitor.metrics("SuccessfulPushCount", period=days_unused)
-        if pulls == 0 and pushes == 0:
-            confidence = "HIGH"
-            risk = "LOW"
+    if registry.provisioning_state != "Succeeded":
+        continue
+    if registry.creation_date is None or registry.creation_date > window_start:
+        continue
+
+    pulls = evaluate_metric("SuccessfulPullCount", interval="PT1H")
+    pushes = evaluate_metric("SuccessfulPushCount", interval="PT1H")
+    if pulls == "ZERO" and pushes == "ZERO":
+        confidence = "HIGH"
+        risk = "LOW"
 ```
 
-Registries with active push activity (e.g. CI pipelines writing images) but zero pulls are **not** flagged — they are in active use.
+Registries with active push activity (for example CI pipelines writing images) but zero pulls are **not** flagged. Registries with sparse, failed, or low-coverage metrics are skipped rather than emitted.
 
 **Common causes:**
 - Workloads migrated to another registry (e.g., Docker Hub → ACR → GHCR)
@@ -1696,7 +1710,9 @@ Registries with active push activity (e.g. CI pipelines writing images) but zero
 - Standard: ~$20/month + storage
 - Premium: ~$50/month + storage
 
-These are floor estimates. ACR also charges per GB of stored images (~$0.003/GB-day). For registries with large image layers, storage can exceed the base fee — actual cost may be significantly higher.
+Unknown or future SKU labels are still evaluated for inactivity, but `estimated_monthly_cost_usd` is left unset when the SKU is not one of `Basic`, `Standard`, or `Premium`.
+
+These are base monthly registry fees only. Storage charges and related Azure costs are not included.
 
 **Required permissions:**
 - `Microsoft.ContainerRegistry/registries/read`

tests/cleancloud/providers/azure/test_azure_scan_graceful_degradation.py

@@ -53,12 +53,12 @@ def _http_404_rule(subscription_id, credential, region_filter=None):
 
 
 def test_permission_error_skips_azure_rule():
-    """PermissionError from an Azure rule goes to skipped_rules."""
+    """PermissionError from an Azure rule goes to skipped_rules, not rules_failed."""
     with patch(
         "cleancloud.providers.azure.scan.AZURE_RULES",
         [_good_rule, _permission_error_rule],
     ):
-        findings, skipped_rules = _scan_azure_subscription(
+        findings, skipped_rules, rules_failed = _scan_azure_subscription(
             subscription_id="sub-123",
             subscription_name="test-sub",
             credential=MagicMock(),
@@ -69,6 +69,7 @@ def test_permission_error_skips_azure_rule():
     assert len(skipped_rules) == 1
     assert skipped_rules[0]["rule"] == "_permission_error_rule"
     assert "Microsoft.Compute/disks/read" in skipped_rules[0]["missing_permissions"]
+    assert rules_failed == 0
 
 
 def test_http_403_skips_azure_rule():
@@ -77,7 +78,7 @@ def test_http_403_skips_azure_rule():
         "cleancloud.providers.azure.scan.AZURE_RULES",
         [_good_rule, _http_403_rule],
     ):
-        findings, skipped_rules = _scan_azure_subscription(
+        findings, skipped_rules, rules_failed = _scan_azure_subscription(
             subscription_id="sub-123",
             subscription_name="test-sub",
             credential=MagicMock(),
@@ -88,33 +89,34 @@ def test_http_403_skips_azure_rule():
     assert len(skipped_rules) == 1
     assert skipped_rules[0]["rule"] == "_http_403_rule"
     assert "403" in skipped_rules[0]["missing_permissions"]
+    assert rules_failed == 0
 
 
 def test_http_non_403_is_still_a_failure():
-    """HttpResponseError with non-403 status is a rule failure, not a skip."""
+    """HttpResponseError with non-403 status increments rules_failed, not skipped_rules."""
     with patch(
         "cleancloud.providers.azure.scan.AZURE_RULES",
         [_good_rule, _http_404_rule],
     ):
-        findings, skipped_rules = _scan_azure_subscription(
+        findings, skipped_rules, rules_failed = _scan_azure_subscription(
             subscription_id="sub-123",
             subscription_name="test-sub",
             credential=MagicMock(),
             region_filter=None,
         )
 
     assert len(findings) == 1
-    # 404 is not a permission skip
     assert len(skipped_rules) == 0
+    assert rules_failed == 1
 
 
 def test_all_permission_errors_no_exception():
-    """All Azure rules failing with PermissionError returns ([], all_skipped) without raising."""
+    """All Azure rules failing with PermissionError returns empty findings without raising."""
     with patch(
         "cleancloud.providers.azure.scan.AZURE_RULES",
         [_permission_error_rule, _http_403_rule],
     ):
-        findings, skipped_rules = _scan_azure_subscription(
+        findings, skipped_rules, rules_failed = _scan_azure_subscription(
             subscription_id="sub-123",
             subscription_name="test-sub",
             credential=MagicMock(),
@@ -123,3 +125,4 @@ def test_all_permission_errors_no_exception():
 
     assert findings == []
     assert len(skipped_rules) == 2
+    assert rules_failed == 0