Fix minor bugs (#134)

sureshcsdp · web-flow · commit 5bc053c5dbce · 2026-04-09T22:41:55.000+01:00
diff --git a/cleancloud/providers/aws/rules/elb_idle.py b/cleancloud/providers/aws/rules/elb_idle.py
@@ -396,7 +396,10 @@ def _get_metric_sum(
             return 1
         return 0
 
-    except ClientError:
-        # If we can't get metrics, assume there might be traffic
-        # to avoid false positives
+    except ClientError as e:
+        if e.response["Error"]["Code"] in ("AccessDenied", "UnauthorizedOperation"):
+            raise PermissionError(
+                "Missing required IAM permissions: cloudwatch:GetMetricStatistics"
+            ) from e
+        # Other errors (throttle, transient): assume traffic to avoid false positives
         return 1
diff --git a/cleancloud/providers/aws/rules/nat_gateway_idle.py b/cleancloud/providers/aws/rules/nat_gateway_idle.py
@@ -234,7 +234,10 @@ def _get_metric_sum(
         total = sum(dp.get("Sum", 0) for dp in datapoints)
         return int(total)
 
-    except ClientError:
-        # If we can't get metrics, assume there might be traffic
-        # to avoid false positives
-        return 1  # Non-zero to indicate possible traffic
+    except ClientError as e:
+        if e.response["Error"]["Code"] in ("AccessDenied", "UnauthorizedOperation"):
+            raise PermissionError(
+                "Missing required IAM permissions: cloudwatch:GetMetricStatistics"
+            ) from e
+        # Other errors (throttle, transient): assume traffic to avoid false positives
+        return 1
diff --git a/cleancloud/providers/aws/rules/rds_idle.py b/cleancloud/providers/aws/rules/rds_idle.py
@@ -143,7 +143,7 @@ def find_idle_rds_instances(
                         details={
                             "engine": f"{engine} {engine_version}",
                             "instance_class": instance_class,
-                            "connections_14d": total_connections,
+                            f"connections_{idle_days}d": total_connections,
                             "estimated_monthly_cost": estimated_monthly_cost,
                             "multi_az": multi_az,
                             "allocated_storage_gb": storage_gb,
@@ -200,10 +200,13 @@ def _get_metric_sum(
             return 1
         return 0
 
-    except ClientError:
-        # If we can't get metrics, assume there might be connections
-        # to avoid false positives
-        return 1  # Non-zero to indicate possible connections
+    except ClientError as e:
+        if e.response["Error"]["Code"] in ("AccessDenied", "UnauthorizedOperation"):
+            raise PermissionError(
+                "Missing required IAM permissions: cloudwatch:GetMetricStatistics"
+            ) from e
+        # Other errors (throttle, transient): assume connections to avoid false positives
+        return 1
 
 
 def _estimate_monthly_cost(instance_class: str, multi_az: bool) -> str:
diff --git a/cleancloud/providers/aws/rules/untagged_resources.py b/cleancloud/providers/aws/rules/untagged_resources.py
@@ -2,6 +2,7 @@
 from typing import List, Optional
 
 import boto3
+from botocore.exceptions import ClientError
 
 from cleancloud.core.confidence import ConfidenceLevel
 from cleancloud.core.evidence import Evidence
@@ -75,8 +76,11 @@ def find_untagged_resources(
         bucket_name = bucket["Name"]
         try:
             tag_set = s3.get_bucket_tagging(Bucket=bucket_name).get("TagSet", [])
-        except s3.exceptions.ClientError:
-            tag_set = []
+        except ClientError as e:
+            if e.response["Error"]["Code"] == "NoSuchTagSet":
+                tag_set = []
+            else:
+                raise
 
         if not tag_set:
             evidence = Evidence(
diff --git a/cleancloud/providers/azure/rules/app_service_plan_empty.py b/cleancloud/providers/azure/rules/app_service_plan_empty.py
@@ -61,7 +61,7 @@ def find_empty_app_service_plans(
     for plan in web_client.app_service_plans.list():
         # Azure Web SDK returns display names ("West Europe") not short names ("westeurope")
         # Normalize to short name format for consistent output and filtering
-        location = plan.location.lower().replace(" ", "")
+        location = (plan.location or "").lower().replace(" ", "")
 
         if region_filter and location != region_filter.lower().replace(" ", ""):
             continue
diff --git a/cleancloud/providers/azure/rules/ebs_snapshots_old.py b/cleancloud/providers/azure/rules/ebs_snapshots_old.py
@@ -44,7 +44,7 @@ def find_old_snapshots(
     )
 
     for snapshot in compute_client.snapshots.list():
-        if region_filter and snapshot.location != region_filter:
+        if region_filter and (snapshot.location or "").lower() != region_filter.lower():
             continue
 
         if not snapshot.time_created:
@@ -53,7 +53,7 @@ def find_old_snapshots(
         age_days = _age_in_days(snapshot.time_created)
 
         if age_days >= max_age_days:
-            confidence_value = ConfidenceLevel.MEDIUM  # conservative
+            confidence_value = ConfidenceLevel.HIGH
         elif age_days >= MIN_AGE_DAYS_MEDIUM:
             confidence_value = ConfidenceLevel.MEDIUM
         else:
@@ -67,7 +67,7 @@ def find_old_snapshots(
                 "Disaster recovery or backup intent",
                 "Future planned usage",
             ],
-            time_window=f"{MIN_AGE_DAYS_MEDIUM}-{MIN_AGE_DAYS_HIGH} days",
+            time_window=f"{MIN_AGE_DAYS_MEDIUM}-{max_age_days} days",
         )
 
         # ~$0.05/GB-month for managed snapshots
diff --git a/cleancloud/providers/azure/rules/sql_database_idle.py b/cleancloud/providers/azure/rules/sql_database_idle.py
@@ -118,9 +118,17 @@ def find_idle_sql_databases(
         if region_filter and server_location != region_filter.lower():
             continue
 
-        resource_group = _extract_resource_group(server.id)
+        try:
+            resource_group = _extract_resource_group(server.id)
+        except ValueError:
+            continue
+
+        try:
+            db_list = list(sql_client.databases.list_by_server(resource_group, server.name))
+        except Exception:
+            continue
 
-        for db in sql_client.databases.list_by_server(resource_group, server.name):
+        for db in db_list:
             # Skip system databases
             if db.name == "master":
                 continue
@@ -157,7 +165,7 @@ def find_idle_sql_databases(
 
             signals = [
                 f"Zero successful connections for {idle_days} days (Azure Monitor metrics)",
-                f"Connections (14d sum): {total_connections}",
+                f"Connections ({idle_days}d sum): {total_connections}",
                 f"SKU: {sku_name} ({sku_tier})",
                 f"Server: {server.name}",
             ]
@@ -200,7 +208,7 @@ def find_idle_sql_databases(
                         "sku_tier": sku_tier,
                         "max_size_bytes": getattr(db, "max_size_bytes", None),
                         "location": db.location,
-                        "connections_14d": total_connections,
+                        f"connections_{idle_days}d": total_connections,
                         "estimated_monthly_cost": estimated_monthly_cost,
                         "tags": db.tags,
                     },
diff --git a/cleancloud/providers/azure/rules/untagged_resources.py b/cleancloud/providers/azure/rules/untagged_resources.py
@@ -44,7 +44,7 @@ def find_untagged_resources(
     # Managed Disks
     # ======================
     for disk in compute_client.disks.list():
-        if region_filter and disk.location != region_filter:
+        if region_filter and (disk.location or "").lower() != region_filter.lower():
             continue
 
         if disk.tags:
@@ -92,7 +92,7 @@ def find_untagged_resources(
     # Snapshots
     # ======================
     for snap in compute_client.snapshots.list():
-        if region_filter and snap.location != region_filter:
+        if region_filter and (snap.location or "").lower() != region_filter.lower():
             continue
 
         if snap.tags:
diff --git a/cleancloud/providers/azure/rules/vm_stopped_not_deallocated.py b/cleancloud/providers/azure/rules/vm_stopped_not_deallocated.py
@@ -54,11 +54,14 @@ def find_stopped_not_deallocated_vms(
         if region_filter and (vm.location or "").lower() != region_filter.lower():
             continue
 
-        resource_group = _extract_resource_group(vm.id)
-        instance_view = compute_client.virtual_machines.instance_view(
-            resource_group_name=resource_group,
-            vm_name=vm.name,
-        )
+        try:
+            resource_group = _extract_resource_group(vm.id)
+            instance_view = compute_client.virtual_machines.instance_view(
+                resource_group_name=resource_group,
+                vm_name=vm.name,
+            )
+        except Exception:
+            continue
 
         power_state = _get_power_state(instance_view)
 
diff --git a/cleancloud/providers/gcp/rules/sql_instance_idle.py b/cleancloud/providers/gcp/rules/sql_instance_idle.py
@@ -280,7 +280,7 @@ def find_idle_sql_instances(
                 evidence=Evidence(
                     signals_used=signals_used,
                     signals_not_checked=[
-                        "Short-lived or batch connections (cron jobs, ETL) not visible in Cloud Monitoring connection metrics over the 7-day window",
+                        f"Short-lived or batch connections (cron jobs, ETL) not visible in Cloud Monitoring connection metrics over the {idle_days}-day window",
                         "Non-TCP workloads or Unix socket connections via Cloud SQL Proxy",
                         "Scheduled maintenance window",
                         "Planned reactivation for upcoming sprint",
diff --git a/cleancloud/providers/gcp/rules/vertex_endpoint_idle.py b/cleancloud/providers/gcp/rules/vertex_endpoint_idle.py
@@ -192,8 +192,6 @@ def find_idle_vertex_endpoints(
 
         # Effective window: cap to age so we don't look back before the endpoint existed
         effective_window = min(_DAYS_IDLE, age_days) if age_days is not None else _DAYS_IDLE
-        if effective_window < 3:
-            continue
 
         # Accurate cost: sum per deployed model (handles mixed machine types correctly)
         deployed_models = endpoint.get("deployedModels", [])
diff --git a/docs/aws.md b/docs/aws.md
@@ -301,7 +301,17 @@ For the complete production workflow with enforcement flags, scheduling, and art
 
 ## IAM Policy (Minimum Required Permissions)
 
-Attach this policy to your IAM role or user:
+> **Policy files are split by category.** The canonical policies live in [`security/aws/`](../security/aws/) as three separate files:
+>
+> | File | Contains | Required when |
+> |------|----------|---------------|
+> | `base-readonly.json` | `sts:GetCallerIdentity`, `cloudwatch:GetMetricStatistics` | **Always — every scan, every category** |
+> | `hygiene-readonly.json` | EC2, RDS, ELB, S3, logs | `--category hygiene` (default) |
+> | `ai-readonly.json` | SageMaker | `--category ai` |
+>
+> `base-readonly.json` must be attached alongside any category file. It provides `cloudwatch:GetMetricStatistics` (used by the NAT gateway, RDS, and ELB idle rules) and `sts:GetCallerIdentity` (used at startup to verify credentials). A role with only `hygiene-readonly.json` attached will have CloudWatch metric calls fail silently on those rules.
+
+Attach this policy to your IAM role or user (combined view — for the split files see above):
 
 ```json
 {
@@ -544,6 +554,8 @@ aws iam put-role-policy \
 
 > The policy files are in the [CleanCloud repo](https://github.com/cleancloud-io/cleancloud/tree/main/security/aws). Download or clone the repo first, then run the commands above.
 >
+> **`base-readonly.json` is always required** — it provides `cloudwatch:GetMetricStatistics` and `sts:GetCallerIdentity` used across all scan categories. Never attach a category policy without it.
+>
 > To also enable AI/ML rules (`--category ai`), attach the AI policy:
 > ```bash
 > aws iam put-role-policy \
diff --git a/docs/gcp.md b/docs/gcp.md
@@ -505,7 +505,7 @@ CleanCloud requires **read-only** IAM permissions only. No write access is neede
 | `compute.globalAddresses.list` | `gcp.compute.ip.unused` | `roles/compute.viewer` |
 | `compute.snapshots.list` | `gcp.compute.snapshot.old` | `roles/compute.viewer` |
 | `cloudsql.instances.list` | `gcp.sql.instance.idle` | `roles/cloudsql.viewer` |
-| `monitoring.timeSeries.list` | `gcp.sql.instance.idle` | `roles/monitoring.viewer` |
+| `monitoring.timeSeries.list` | `gcp.sql.instance.idle`, `gcp.vertex.endpoint.idle` | `roles/monitoring.viewer` |
 | `resourcemanager.projects.get` | project enumeration (`--all-projects`) | `roles/browser` |
 
 ### Graceful Degradation
diff --git a/docs/rules.md b/docs/rules.md
@@ -80,7 +80,7 @@ Every finding includes a confidence level:
 | `gcp.compute.disk.unattached` | Storage | Persistent Disks in READY state with no attached VM |
 | `gcp.compute.snapshot.old` | Storage | Disk snapshots older than 90 days |
 | `gcp.compute.ip.unused` | Network | Reserved static IPs (regional and global) in RESERVED state |
-| `gcp.sql.instance.idle` | Platform | Cloud SQL instances with zero connections for 7+ days |
+| `gcp.sql.instance.idle` | Platform | Cloud SQL instances with zero connections for 14+ days |
 | `gcp.vertex.endpoint.idle` | AI/ML | Vertex AI Online Prediction endpoints with dedicated capacity and zero predictions for 14+ days (`--category ai`) |
 
 ---
@@ -1588,11 +1588,11 @@ for address in global_addresses_client.list(project=project_id):
 
 **Rule ID:** `gcp.sql.instance.idle`
 
-**What it detects:** Cloud SQL instances in `RUNNABLE` state with zero database connections for 7+ days
+**What it detects:** Cloud SQL instances in `RUNNABLE` state with zero database connections for 14+ days
 
 **Confidence:**
 
-- **HIGH:** Monitoring confirms zero connections for the full 7-day window
+- **HIGH:** Monitoring confirms zero connections for the full 14-day window
 
 **Risk:** HIGH
 
@@ -1606,7 +1606,7 @@ for address in global_addresses_client.list(project=project_id):
 ```python
 for instance in sql_admin_api.list(project_id):
     if instance.state == "RUNNABLE" and not is_read_replica(instance):
-        if not has_connections(monitoring_client, project_id, instance.name, days=7):
+        if not has_connections(monitoring_client, project_id, instance.name, days=14):
             flag(instance)
 ```
 
diff --git a/pyproject.toml b/pyproject.toml
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "cleancloud"
-version = "1.14.0"
+version = "1.14.1"
 description = "Read-only cloud hygiene for AWS, Azure, and GCP. Multi-account org scanning, CI/CD enforcement, and deterministic cost modeling. No agents, no telemetry."
 readme = "README.md"
 requires-python = ">=3.10"
