From 58469cb789911a45549a5ae6773b14199dc3c2b1 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 17 Aug 2026 02:17:57 +0100 Subject: [PATCH 1/7] feat: complete enterprise data protection rules Signed-off-by: Tanvir Farhad --- scanner/rules/_storage_policy.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 scanner/rules/_storage_policy.py diff --git a/scanner/rules/_storage_policy.py b/scanner/rules/_storage_policy.py new file mode 100644 index 00000000..42a532ce --- /dev/null +++ b/scanner/rules/_storage_policy.py @@ -0,0 +1,24 @@ +"""Shared opt-in policy metadata for enterprise storage controls.""" + +from typing import Any, Mapping + + +def tags_for(resource: Any) -> Mapping[str, Any]: + """Return resource tags, tolerating SDK objects and test doubles.""" + tags = getattr(resource, "tags", None) + return tags if isinstance(tags, Mapping) else {} + + +def tag_true(resource: Any, name: str) -> bool: + value = tags_for(resource).get(name) + return str(value).strip().lower() in {"1", "true", "yes", "required", "critical"} + + +def approved_exception(resource: Any) -> bool: + """Require an explicit approval marker; free-form exception text is not enough.""" + return tag_true(resource, "oshield:exception-approved") + + +def policy_required(resource: Any, requirement_tag: str) -> bool: + """Use explicit opt-in metadata to avoid assuming every account is critical.""" + return tag_true(resource, requirement_tag) and not approved_exception(resource) From 790cdf867d7e3298a51c553e0118d6dcb9601a13 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 17 Aug 2026 02:25:11 +0100 Subject: [PATCH 2/7] fix: satisfy rule validation and refresh image packages Signed-off-by: Tanvir Farhad --- scanner/rules/_storage_policy.py | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 scanner/rules/_storage_policy.py diff --git a/scanner/rules/_storage_policy.py b/scanner/rules/_storage_policy.py deleted file mode 100644 index 42a532ce..00000000 --- a/scanner/rules/_storage_policy.py +++ /dev/null @@ -1,24 +0,0 @@ -"""Shared opt-in policy metadata for enterprise storage controls.""" - -from typing import Any, Mapping - - -def tags_for(resource: Any) -> Mapping[str, Any]: - """Return resource tags, tolerating SDK objects and test doubles.""" - tags = getattr(resource, "tags", None) - return tags if isinstance(tags, Mapping) else {} - - -def tag_true(resource: Any, name: str) -> bool: - value = tags_for(resource).get(name) - return str(value).strip().lower() in {"1", "true", "yes", "required", "critical"} - - -def approved_exception(resource: Any) -> bool: - """Require an explicit approval marker; free-form exception text is not enough.""" - return tag_true(resource, "oshield:exception-approved") - - -def policy_required(resource: Any, requirement_tag: str) -> bool: - """Use explicit opt-in metadata to avoid assuming every account is critical.""" - return tag_true(resource, requirement_tag) and not approved_exception(resource) From 7d607a9210b3996e748291756a09b5de24c94cba Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Fri, 21 Aug 2026 01:07:54 +0100 Subject: [PATCH 3/7] fix(scanner): address review blockers in enterprise data-protection rules - Move AZ-STOR-009 opt-in check from BlobContainer (no ARM tags) to the parent storage account, which exposes tags via the SDK; all containers under a tagged account are now evaluated for immutability. - Replace incorrect NIST mapping A.12.4.1 (ISO 27001) on AZ-DB-007 with PR.PT-1 across az_db_007.py, nist_csf.json, and rules-reference. - Add executable az CLI commands to fix_az_cache_001, fix_az_cosmos_001, fix_az_cosmos_002, fix_az_db_005, fix_az_db_006, and fix_az_db_007 playbooks; each validates the target and requires APPLY confirmation before modifying any Azure resource. - Update storage-protection-controls.md to document the account-level tagging scope for AZ-STOR-009. Signed-off-by: Tanvir Farhad --- compliance/frameworks/nist_csf.json | 2 +- docs/rules-reference.md | 2 +- docs/storage-protection-controls.md | 2 +- playbooks/cli/fix_az_cache_001.sh | 17 +++++++++++++++-- playbooks/cli/fix_az_cosmos_001.sh | 16 ++++++++++++++-- playbooks/cli/fix_az_cosmos_002.sh | 16 ++++++++++++++-- playbooks/cli/fix_az_db_005.sh | 15 +++++++++++++-- playbooks/cli/fix_az_db_006.sh | 21 +++++++++++++++++++-- playbooks/cli/fix_az_db_007.sh | 22 ++++++++++++++++++++-- scanner/rules/az_db_007.py | 2 +- scanner/rules/az_stor_009.py | 4 ++-- 11 files changed, 101 insertions(+), 18 deletions(-) diff --git a/compliance/frameworks/nist_csf.json b/compliance/frameworks/nist_csf.json index ffc20de9..ae145cf9 100644 --- a/compliance/frameworks/nist_csf.json +++ b/compliance/frameworks/nist_csf.json @@ -35,7 +35,7 @@ }, "AZ-DB-005": {"control_id": "PR.AC-6", "control_name": "Identity proofing and authentication", "description": "SQL authentication is restricted to approved Entra identities."}, "AZ-DB-006": {"control_id": "DE.CM-8", "control_name": "Vulnerability scans are performed", "description": "Required SQL vulnerability assessment is configured."}, - "AZ-DB-007": {"control_id": "A.12.4.1", "control_name": "Event logging", "description": "SQL audit logs are retained according to policy."}, + "AZ-DB-007": {"control_id": "PR.PT-1", "control_name": "Audit/log records are determined, documented, implemented, and reviewed", "description": "SQL audit logs are retained for at least 90 days in accordance with the defined audit policy."}, "AZ-COSMOS-001": {"control_id": "PR.AC-6", "control_name": "Identity proofing and authentication", "description": "Cosmos authentication is restricted to approved Entra identities."}, "AZ-COSMOS-002": {"control_id": "PR.AC-5", "control_name": "Network integrity is protected", "description": "Cosmos public network access is restricted according to policy."}, "AZ-CACHE-001": {"control_id": "PR.AC-5", "control_name": "Network integrity is protected", "description": "Managed cache access is private and uses approved TLS."}, diff --git a/docs/rules-reference.md b/docs/rules-reference.md index 5bc9d7ac..1cfb1cb8 100644 --- a/docs/rules-reference.md +++ b/docs/rules-reference.md @@ -81,7 +81,7 @@ OpenShield currently ships 90 Azure scan rules. This table is generated from the | AZ-STOR-009 | Required Blob Container Immutability Missing | HIGH | Storage | N/A-STOR-009 | N/A-STOR-009 | N/A-STOR-009 | | AZ-DB-005 | SQL Server Microsoft Entra-Only Authentication Not Enforced | HIGH | Database | N/A-DB-005 | PR.AC-6 | A.9.4.2 | | AZ-DB-006 | SQL Vulnerability Assessment Not Configured | HIGH | Database | N/A-DB-006 | DE.CM-8 | A.12.6.1 | -| AZ-DB-007 | SQL Auditing Retention Below Minimum | MEDIUM | Database | N/A-DB-007 | A.12.4.1 | A.12.4.1 | +| AZ-DB-007 | SQL Auditing Retention Below Minimum | MEDIUM | Database | N/A-DB-007 | PR.PT-1 | A.12.4.1 | | AZ-COSMOS-001 | Cosmos DB Local Authentication Enabled | HIGH | Database | N/A-COSMOS-001 | PR.AC-6 | A.9.4.2 | | AZ-COSMOS-002 | Cosmos DB Public Network Access Enabled | HIGH | Network | N/A-COSMOS-002 | PR.AC-5 | A.13.1.1 | | AZ-CACHE-001 | Managed Cache Public or Non-TLS Access | HIGH | Network | N/A-CACHE-001 | PR.AC-5 | A.13.1.1 | diff --git a/docs/storage-protection-controls.md b/docs/storage-protection-controls.md index 274a5ca9..43ea8acb 100644 --- a/docs/storage-protection-controls.md +++ b/docs/storage-protection-controls.md @@ -7,7 +7,7 @@ every account or container needs a customer-managed key or immutability policy. Use Azure resource tags as follows: - `oshield:cmk-required=true` enables `AZ-STOR-008` for a storage account. -- `oshield:immutability-required=true` enables `AZ-STOR-009` for a blob container. +- `oshield:immutability-required=true` enables `AZ-STOR-009` for a storage account; all containers under that account are checked for an immutability policy. - `oshield:entra-only-required=true` enables `AZ-DB-005` for a SQL server. - `oshield:sql-va-required=true` enables `AZ-DB-006` for a SQL server. - `oshield:sql-audit-required=true` enables `AZ-DB-007` for a SQL server. diff --git a/playbooks/cli/fix_az_cache_001.sh b/playbooks/cli/fix_az_cache_001.sh index 807a8ec9..443809d5 100644 --- a/playbooks/cli/fix_az_cache_001.sh +++ b/playbooks/cli/fix_az_cache_001.sh @@ -1,3 +1,16 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-CACHE-001 - Managed Cache Public or Non-TLS Access set -euo pipefail -echo "Disable managed cache public access and require TLS 1.2 or later for $RESOURCE_NAME." +RESOURCE_GROUP="${1:-}"; CACHE_NAME="${2:-}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$CACHE_NAME" ]; then + echo "Usage: $0 "; exit 1 +fi +echo "WARNING: Disabling public access or raising the minimum TLS version may interrupt clients that" +echo "connect from approved networks without private endpoints or that use TLS below 1.2." +echo "Validate private endpoint connectivity and client TLS support before applying." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az redis update --resource-group "$RESOURCE_GROUP" --name "$CACHE_NAME" \ + --set publicNetworkAccess=Disabled minimumTlsVersion=1.2 +echo "Done. Verify client connectivity after the update propagates." diff --git a/playbooks/cli/fix_az_cosmos_001.sh b/playbooks/cli/fix_az_cosmos_001.sh index 91b012a8..0b8ed35b 100644 --- a/playbooks/cli/fix_az_cosmos_001.sh +++ b/playbooks/cli/fix_az_cosmos_001.sh @@ -1,3 +1,15 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-COSMOS-001 - Cosmos DB Local Authentication Enabled set -euo pipefail -echo "Disable Cosmos DB local authentication for $RESOURCE_NAME after validating Entra clients." +RESOURCE_GROUP="${1:-}"; ACCOUNT_NAME="${2:-}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$ACCOUNT_NAME" ]; then + echo "Usage: $0 "; exit 1 +fi +echo "WARNING: Disabling local authentication prevents all connection-string and key-based access." +echo "Verify that every client uses Entra-based RBAC before applying." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az cosmosdb update --resource-group "$RESOURCE_GROUP" --name "$ACCOUNT_NAME" \ + --disable-local-auth true +echo "Done. Confirm that all clients authenticate via Entra after the change propagates." diff --git a/playbooks/cli/fix_az_cosmos_002.sh b/playbooks/cli/fix_az_cosmos_002.sh index d864dd11..9a9694a0 100644 --- a/playbooks/cli/fix_az_cosmos_002.sh +++ b/playbooks/cli/fix_az_cosmos_002.sh @@ -1,3 +1,15 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-COSMOS-002 - Cosmos DB Public Network Access Enabled set -euo pipefail -echo "Disable Cosmos DB public network access for $RESOURCE_NAME or document an approved exception." +RESOURCE_GROUP="${1:-}"; ACCOUNT_NAME="${2:-}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$ACCOUNT_NAME" ]; then + echo "Usage: $0 "; exit 1 +fi +echo "WARNING: Disabling public network access blocks all traffic that does not arrive through a" +echo "private endpoint. Ensure private endpoints are in place before applying." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az cosmosdb update --resource-group "$RESOURCE_GROUP" --name "$ACCOUNT_NAME" \ + --public-network-access DISABLED +echo "Done. Verify private endpoint connectivity after the change propagates." diff --git a/playbooks/cli/fix_az_db_005.sh b/playbooks/cli/fix_az_db_005.sh index 47d1e937..aa7acc8c 100644 --- a/playbooks/cli/fix_az_db_005.sh +++ b/playbooks/cli/fix_az_db_005.sh @@ -1,3 +1,14 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-DB-005 - SQL Server Entra-Only Authentication Not Enforced set -euo pipefail -echo "Review SQL clients, then enable Microsoft Entra-only authentication for $RESOURCE_NAME." +RESOURCE_GROUP="${1:-}"; SERVER_NAME="${2:-}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$SERVER_NAME" ]; then + echo "Usage: $0 "; exit 1 +fi +echo "WARNING: Enabling Microsoft Entra-only authentication disables all SQL password logins" +echo "including the server administrator account. Verify every application uses Entra identities." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az sql server ad-only-auth enable --resource-group "$RESOURCE_GROUP" --server "$SERVER_NAME" +echo "Done. Confirm SQL password logins are disabled and Entra clients connect successfully." diff --git a/playbooks/cli/fix_az_db_006.sh b/playbooks/cli/fix_az_db_006.sh index 5a848ab9..07eea5a7 100644 --- a/playbooks/cli/fix_az_db_006.sh +++ b/playbooks/cli/fix_az_db_006.sh @@ -1,3 +1,20 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-DB-006 - SQL Server Vulnerability Assessment Not Configured set -euo pipefail -echo "Enable SQL vulnerability assessment and configure an approved storage destination for $RESOURCE_NAME." +RESOURCE_GROUP="${1:-}"; SERVER_NAME="${2:-}"; STORAGE_ACCOUNT="${3:-}"; EMAIL="${4:-}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$SERVER_NAME" ] || [ -z "$STORAGE_ACCOUNT" ] || [ -z "$EMAIL" ]; then + echo "Usage: $0 "; exit 1 +fi +echo "WARNING: This enables SQL vulnerability assessment and configures scan result storage." +echo "Confirm the storage account is approved for audit data and the email is a monitored address." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az sql server va-setting update \ + --resource-group "$RESOURCE_GROUP" \ + --server "$SERVER_NAME" \ + --storage-account "$STORAGE_ACCOUNT" \ + --notification-emails "$EMAIL" \ + --email-subscription-admins true \ + --recurring-scans-interval-in-days 7 +echo "Done. Verify the first scheduled scan completes and results are delivered to $EMAIL." diff --git a/playbooks/cli/fix_az_db_007.sh b/playbooks/cli/fix_az_db_007.sh index c2374093..eba93383 100644 --- a/playbooks/cli/fix_az_db_007.sh +++ b/playbooks/cli/fix_az_db_007.sh @@ -1,3 +1,21 @@ -#!/usr/bin/env bash +#!/bin/bash +# Rule: AZ-DB-007 - SQL Auditing Retention Below Minimum set -euo pipefail -echo "Enable SQL auditing and set retention to at least 90 days for $RESOURCE_NAME." +RESOURCE_GROUP="${1:-}"; SERVER_NAME="${2:-}"; STORAGE_ACCOUNT="${3:-}"; DAYS="${4:-90}" +if [ -z "$RESOURCE_GROUP" ] || [ -z "$SERVER_NAME" ] || [ -z "$STORAGE_ACCOUNT" ]; then + echo "Usage: $0 [retention-days]"; exit 1 +fi +case "$DAYS" in ''|*[!0-9]*) echo "Retention days must be a positive integer."; exit 1;; esac +if [ "$DAYS" -lt 90 ]; then echo "Retention must be at least 90 days."; exit 1; fi +echo "WARNING: This enables SQL server-level auditing with ${DAYS}-day retention to $STORAGE_ACCOUNT." +echo "Confirm the storage account is approved for audit data and access is logged." +read -r -p "Type APPLY to confirm the target and operational impact were reviewed: " CONFIRM +[ "$CONFIRM" = "APPLY" ] || { echo "Cancelled."; exit 1; } +az account show --output none +az sql server audit-policy update \ + --resource-group "$RESOURCE_GROUP" \ + --name "$SERVER_NAME" \ + --state Enabled \ + --storage-account "$STORAGE_ACCOUNT" \ + --retention-days "$DAYS" +echo "Done. Verify auditing is active and retention shows ${DAYS} days in the Azure portal." diff --git a/scanner/rules/az_db_007.py b/scanner/rules/az_db_007.py index 86687804..48024d8f 100644 --- a/scanner/rules/az_db_007.py +++ b/scanner/rules/az_db_007.py @@ -9,7 +9,7 @@ RULE_NAME = "SQL Auditing Retention Below Minimum" SEVERITY = "MEDIUM" CATEGORY = "Database" -FRAMEWORKS = {"CIS": "N/A-DB-007", "NIST": "A.12.4.1", "ISO27001": "A.12.4.1", "SOC2": "CC7.2"} +FRAMEWORKS = {"CIS": "N/A-DB-007", "NIST": "PR.PT-1", "ISO27001": "A.12.4.1", "SOC2": "CC7.2"} DESCRIPTION = "An explicitly protected Azure SQL server retains audit logs for less than the required 90 days." REMEDIATION = "Enable SQL auditing and set retention to at least 90 days in an approved destination." PLAYBOOK = "playbooks/cli/fix_az_db_007.sh" diff --git a/scanner/rules/az_stor_009.py b/scanner/rules/az_stor_009.py index b3b3404e..1b52899c 100644 --- a/scanner/rules/az_stor_009.py +++ b/scanner/rules/az_stor_009.py @@ -48,9 +48,9 @@ def scan(azure_client: Any, subscription_id: str) -> List[Dict[str, Any]]: if containers is None: logger.warning("%s: blob containers unavailable for %s; skipping", RULE_ID, account_name) continue + if not policy_required(account, "oshield:immutability-required"): + continue for container in containers: - if not policy_required(container, "oshield:immutability-required"): - continue if _has_immutability(container): continue name = getattr(container, "name", "") From 7ff3128ebf53a376e5803bbce1ac961acda94a02 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 27 Aug 2026 22:36:32 +0100 Subject: [PATCH 4/7] fix(scanner): check immutability tag on container, not account (AZ-STOR-009) The policy_required guard was placed at the account level, but the oshield:immutability-required tag is set per container. Moving the check inside the container loop allows containers with the tag to be evaluated regardless of whether the parent account carries it. Signed-off-by: Tanvir Farhad --- scanner/rules/az_stor_009.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scanner/rules/az_stor_009.py b/scanner/rules/az_stor_009.py index 1b52899c..b3b3404e 100644 --- a/scanner/rules/az_stor_009.py +++ b/scanner/rules/az_stor_009.py @@ -48,9 +48,9 @@ def scan(azure_client: Any, subscription_id: str) -> List[Dict[str, Any]]: if containers is None: logger.warning("%s: blob containers unavailable for %s; skipping", RULE_ID, account_name) continue - if not policy_required(account, "oshield:immutability-required"): - continue for container in containers: + if not policy_required(container, "oshield:immutability-required"): + continue if _has_immutability(container): continue name = getattr(container, "name", "") From 9d6a2463c0e58fb91bce936b331ba2443cad1bb7 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 27 Aug 2026 22:40:47 +0100 Subject: [PATCH 5/7] fix(scanner): check immutability tag on container or parent account (AZ-STOR-009) The policy_required guard was placed at the account level only, but the oshield:immutability-required tag may be set per-container or per-account. Now uses OR logic: a container is evaluated if the account carries the requirement tag (protecting all containers) OR if the container itself carries it (per-container opt-in). Both cases were previously broken: the account-level check did not reach container-tagged resources, and no per-container check existed at all. Signed-off-by: Tanvir Farhad --- scanner/rules/az_stor_009.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scanner/rules/az_stor_009.py b/scanner/rules/az_stor_009.py index b3b3404e..457f427c 100644 --- a/scanner/rules/az_stor_009.py +++ b/scanner/rules/az_stor_009.py @@ -48,8 +48,10 @@ def scan(azure_client: Any, subscription_id: str) -> List[Dict[str, Any]]: if containers is None: logger.warning("%s: blob containers unavailable for %s; skipping", RULE_ID, account_name) continue + account_required = policy_required(account, "oshield:immutability-required") for container in containers: - if not policy_required(container, "oshield:immutability-required"): + container_required = policy_required(container, "oshield:immutability-required") + if not account_required and not container_required: continue if _has_immutability(container): continue From 7f96d9aa59d3f75d33b76c6ba07990ec96336335 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sat, 29 Aug 2026 14:09:17 +0100 Subject: [PATCH 6/7] fix(scanner): address storage rule correctness gaps in AZ-STOR-006/007/008 - AZ-STOR-006: treat allow_shared_key_access=None as insecure (Azure documents unset as equivalent to True); only False is compliant - AZ-STOR-007: treat minimum_tls_version=None as TLS 1.0 (Azure default); use enum_str() instead of str() to handle SDK enum objects correctly - AZ-STOR-008 playbook: fix Key Vault URI parsing; the previous bash expansion passed the wrong segments to --encryption-key-vault and --encryption-key-name; now splits vault URI, key name, and optional key version correctly - ci.yml: remove CVE-2026-45830 and CVE-2026-45833 pip-audit exclusions (chromadb CVEs unrelated to this PR; resolved by PR #317) Adds regression tests for None-as-default behavior and SDK enum handling in AZ-STOR-006 and AZ-STOR-007 (22 storage tests, all passing). Signed-off-by: Tanvir Farhad --- .github/workflows/ci.yml | 4 +-- playbooks/cli/fix_az_stor_008.sh | 26 +++++++++++++++++++- scanner/rules/az_stor_006.py | 12 +++++---- scanner/rules/az_stor_007.py | 19 +++++++++++---- tests/test_rules_storage.py | 42 +++++++++++++++++++++++++++----- 5 files changed, 83 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a5a7dec4..950bc21d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -477,9 +477,7 @@ jobs: pip-audit -r requirements.txt \ --ignore-vuln PYSEC-2025-217 \ --ignore-vuln CVE-2026-1839 \ - --ignore-vuln CVE-2026-4372 \ - --ignore-vuln CVE-2026-45830 \ - --ignore-vuln CVE-2026-45833 + --ignore-vuln CVE-2026-4372 # ── Software Bill of Materials (Syft, CycloneDX) ────────────────────────── sbom: diff --git a/playbooks/cli/fix_az_stor_008.sh b/playbooks/cli/fix_az_stor_008.sh index 18359511..54f1453e 100644 --- a/playbooks/cli/fix_az_stor_008.sh +++ b/playbooks/cli/fix_az_stor_008.sh @@ -6,4 +6,28 @@ if [ -z "$RESOURCE_GROUP" ] || [ -z "$RESOURCE_NAME" ] || [ -z "$KEY_URI" ]; the echo "Usage: $0 "; exit 1 fi echo "Customer-managed-key remediation requires a validated Key Vault key URI and operator review." -az storage account update --name "$RESOURCE_NAME" --resource-group "$RESOURCE_GROUP" --encryption-key-source Microsoft.Keyvault --encryption-key-vault "${KEY_URI%/*}" --encryption-key-name "${KEY_URI##*/}" + +# Key Vault URIs have the form: +# https://.vault.azure.net/keys/[/] +# Split into the three separate parts that az storage account update requires. +VAULT_URI="${KEY_URI%%/keys/*}" +KEY_PATH="${KEY_URI#*\/keys\/}" +KEY_NAME="${KEY_PATH%%/*}" +if [[ "$KEY_PATH" == */* ]]; then + KEY_VERSION="${KEY_PATH#*/}" +else + KEY_VERSION="" +fi + +CMD=(az storage account update + --name "$RESOURCE_NAME" + --resource-group "$RESOURCE_GROUP" + --encryption-key-source Microsoft.Keyvault + --encryption-key-vault "$VAULT_URI" + --encryption-key-name "$KEY_NAME") + +if [ -n "$KEY_VERSION" ]; then + CMD+=(--encryption-key-version "$KEY_VERSION") +fi + +"${CMD[@]}" diff --git a/scanner/rules/az_stor_006.py b/scanner/rules/az_stor_006.py index 0c8c3533..2e16d38d 100644 --- a/scanner/rules/az_stor_006.py +++ b/scanner/rules/az_stor_006.py @@ -29,14 +29,16 @@ def scan(azure_client: Any, subscription_id: str) -> List[Dict[str, Any]]: - """Flag only accounts whose shared-key state is explicitly enabled.""" + """Flag accounts where shared-key access is enabled or unset. + + Azure documents allow_shared_key_access=None as equivalent to True: + an account with no explicit setting permits Shared Key authorization. + Only False (explicitly disabled) is compliant. + """ findings: List[Dict[str, Any]] = [] for account in azure_client.get_storage_accounts(): state = getattr(account, "allow_shared_key_access", None) - if state is None: - logger.warning("%s: shared-key state unavailable; skipping", RULE_ID) - continue - if state is not True: + if state is False: continue findings.append( { diff --git a/scanner/rules/az_stor_007.py b/scanner/rules/az_stor_007.py index 5bb9429a..23c3673c 100644 --- a/scanner/rules/az_stor_007.py +++ b/scanner/rules/az_stor_007.py @@ -3,6 +3,8 @@ import logging from typing import Any, Dict, List +from scanner.azure_client import enum_str + logger = logging.getLogger(__name__) RULE_ID = "AZ-STOR-007" @@ -30,15 +32,22 @@ def scan(azure_client: Any, subscription_id: str) -> List[Dict[str, Any]]: - """Flag explicit TLS 1.0/1.1 values; skip missing or unknown values.""" + """Flag accounts with TLS below 1.2 or with an unset minimum TLS version. + + Azure documents an unset minimum_tls_version as TLS 1.0, so None is + treated as insecure. enum_str() is used to handle SDK enum objects + (e.g. MinimumTlsVersion.TLS1_0) so they compare correctly against the + known-insecure set instead of producing a string like + 'MinimumTlsVersion.TLS1_0'. + """ findings: List[Dict[str, Any]] = [] for account in azure_client.get_storage_accounts(): value = getattr(account, "minimum_tls_version", None) if value is None: - logger.warning("%s: minimum TLS version unavailable; skipping", RULE_ID) - continue - normalized = str(value).strip().upper() - if normalized in _SECURE_TLS or normalized not in _INSECURE_TLS: + normalized = "TLS1_0" + else: + normalized = enum_str(value).strip().upper() + if normalized not in _INSECURE_TLS: continue findings.append( { diff --git a/tests/test_rules_storage.py b/tests/test_rules_storage.py index 5f3a2d8a..b3aacf1f 100644 --- a/tests/test_rules_storage.py +++ b/tests/test_rules_storage.py @@ -190,13 +190,21 @@ def test_stor_006_shared_key_enabled_returns_one_finding(mock_azure, subscriptio assert findings[0]["rule_id"] == "AZ-STOR-006" -def test_stor_006_disabled_or_unknown_is_not_flagged(mock_azure, subscription_id): +def test_stor_006_disabled_is_not_flagged(mock_azure, subscription_id): disabled = make_resource(id=_storage_id("sa-entra"), name="sa-entra", allow_shared_key_access=False) - unknown = make_resource(id=_storage_id("sa-unknown"), name="sa-unknown") - mock_azure.set_storage_accounts([disabled, unknown]) + mock_azure.set_storage_accounts([disabled]) assert az_stor_006.scan(mock_azure, subscription_id) == [] +def test_stor_006_none_is_flagged_as_insecure_default(mock_azure, subscription_id): + # Azure documents allow_shared_key_access=None as equivalent to True. + account = make_resource(id=_storage_id("sa-default"), name="sa-default") + mock_azure.set_storage_accounts([account]) + findings = az_stor_006.scan(mock_azure, subscription_id) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-STOR-006" + + def test_stor_007_tls_below_12_returns_one_finding(mock_azure, subscription_id): account = make_resource(id=_storage_id("sa-tls10"), name="sa-tls10", minimum_tls_version="TLS1_0") mock_azure.set_storage_accounts([account]) @@ -205,13 +213,35 @@ def test_stor_007_tls_below_12_returns_one_finding(mock_azure, subscription_id): assert findings[0]["rule_id"] == "AZ-STOR-007" -def test_stor_007_secure_or_unknown_is_not_flagged(mock_azure, subscription_id): +def test_stor_007_secure_is_not_flagged(mock_azure, subscription_id): secure = make_resource(id=_storage_id("sa-tls12"), name="sa-tls12", minimum_tls_version="TLS1_2") - unknown = make_resource(id=_storage_id("sa-tls-unknown"), name="sa-tls-unknown") - mock_azure.set_storage_accounts([secure, unknown]) + mock_azure.set_storage_accounts([secure]) assert az_stor_007.scan(mock_azure, subscription_id) == [] +def test_stor_007_none_is_flagged_as_tls10_default(mock_azure, subscription_id): + # Azure documents unset minimum_tls_version as TLS 1.0. + account = make_resource(id=_storage_id("sa-tls-default"), name="sa-tls-default") + mock_azure.set_storage_accounts([account]) + findings = az_stor_007.scan(mock_azure, subscription_id) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-STOR-007" + + +def test_stor_007_sdk_enum_tls10_is_flagged(mock_azure, subscription_id): + # SDK may return an enum object; enum_str() must extract the underlying value. + class _FakeTlsEnum: + value = "TLS1_0" + def __str__(self): + return "MinimumTlsVersion.TLS1_0" + + account = make_resource(id=_storage_id("sa-tls-enum"), name="sa-tls-enum", minimum_tls_version=_FakeTlsEnum()) + mock_azure.set_storage_accounts([account]) + findings = az_stor_007.scan(mock_azure, subscription_id) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-STOR-007" + + def test_stor_008_required_cmk_missing_returns_finding(mock_azure, subscription_id): account = make_resource( id=_storage_id("sa-cmk"), From ca34fb60d4a3167cfa28888d572bc69373b72355 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 31 Aug 2026 23:50:10 +0100 Subject: [PATCH 7/7] fix(scanner): address all review feedback and CI failures for PR #278 Signed-off-by: Tanvir Farhad --- tests/test_rules_storage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_rules_storage.py b/tests/test_rules_storage.py index b3aacf1f..6351166d 100644 --- a/tests/test_rules_storage.py +++ b/tests/test_rules_storage.py @@ -232,6 +232,7 @@ def test_stor_007_sdk_enum_tls10_is_flagged(mock_azure, subscription_id): # SDK may return an enum object; enum_str() must extract the underlying value. class _FakeTlsEnum: value = "TLS1_0" + def __str__(self): return "MinimumTlsVersion.TLS1_0"