Skip to content

[REVIEW] azure-review: add ACR admin and private-network evidence gates #1206

@z707693052

Description

@z707693052

Skill Being Reviewed

Skill name: azure-review
Skill path: skills/cloud/azure-review/

False Positive Analysis

Benign production ACR configuration that should not be failed as public or admin-credential exposure:

resource "azurerm_container_registry" "prod" {
  name                          = "prodacr"
  resource_group_name           = azurerm_resource_group.prod.name
  location                      = azurerm_resource_group.prod.location
  sku                           = "Premium"
  admin_enabled                 = false
  public_network_access_enabled = false
  network_rule_bypass_option    = "AzureServices"
}

resource "azurerm_private_endpoint" "acr" {
  name                = "prod-acr-pe"
  resource_group_name = azurerm_resource_group.prod.name
  location            = azurerm_resource_group.prod.location
  subnet_id           = azurerm_subnet.private_endpoints.id

  private_service_connection {
    name                           = "prod-acr"
    is_manual_connection           = false
    private_connection_resource_id = azurerm_container_registry.prod.id
    subresource_names              = ["registry"]
  }
}

resource "azurerm_user_assigned_identity" "aks_pull" {
  name                = "aks-acr-pull"
  resource_group_name = azurerm_resource_group.prod.name
  location            = azurerm_resource_group.prod.location
}

resource "azurerm_role_assignment" "aks_can_pull_acr" {
  scope                = azurerm_container_registry.prod.id
  role_definition_name = "AcrPull"
  principal_id         = azurerm_user_assigned_identity.aks_pull.principal_id
}

Why this is a false positive:

The current azure-review skill has strong CIS Azure coverage, but it does not distinguish Azure Container Registry-specific hardening states. Microsoft documents that the ACR admin account is disabled by default and should be enabled only when necessary; Private Link can remove public exposure for Premium registries; and network-restricted registries need explicit handling for trusted Microsoft services such as Defender. A private registry with admin disabled, private endpoint evidence, and scoped managed identity pull access should pass the supplemental registry review rather than being lumped into generic public-network findings.

Coverage Gaps

Missed variant 1: enabled ACR admin account on production registry

resource "azurerm_container_registry" "prod" {
  name          = "prodacr"
  sku           = "Premium"
  admin_enabled = true
}

Why it should be caught:

The admin account has broad push/pull access and collapses activity to a shared credential. Production reviews should require a documented exception and migration to Entra identities, managed identities, service principals, or repository-scoped tokens.

Missed variant 2: public registry endpoint with no firewall default deny or private endpoint evidence

resource "azurerm_container_registry" "prod" {
  name                          = "prodacr"
  sku                           = "Premium"
  public_network_access_enabled = true
}

Why it should be caught:

ACR accepts public network connections by default. The skill should require either disabled public access with Private Link, or explicit firewall/default-deny evidence plus a business justification for public reachability.

Missed variant 3: broad registry role assignments or unbounded repository-scoped tokens

resource "azurerm_role_assignment" "ci_push" {
  scope                = azurerm_resource_group.prod.id
  role_definition_name = "AcrPush"
  principal_id         = azuread_service_principal.ci.object_id
}

Why it should be caught:

Registry pull/push permissions should be scoped to the registry or narrower where possible. Repository-scoped tokens are useful for non-Entra clients, but the review needs scope-map, action, owner, rotation, and expiration evidence.

Missed variant 4: image scanning evidence missing for network-restricted registry

resource "azurerm_container_registry" "prod" {
  name                          = "prodacr"
  sku                           = "Premium"
  public_network_access_enabled = false
  network_rule_bypass_option    = "None"
}

Why it should be caught:

Defender for Containers can provide registry image vulnerability assessment, but network-restricted registries may require trusted-services access or other documented configuration. The review should record whether scanning coverage still works after network restrictions are applied.

Edge Cases

  • Public access can be legitimate for partner, edge, or migration workflows, but the report should capture default deny, exact allowed IP ranges, environment criticality, and expiry of the exception.
  • Private endpoints require DNS and build-agent reachability evidence; simply seeing azurerm_private_endpoint is not enough.
  • Trusted-services bypass can be needed for Defender or build workflows; it should be documented rather than blanket-failed.
  • Repository-scoped tokens are appropriate for non-Entra clients, but broad write/delete actions or no rotation evidence should change severity.
  • Retention, quarantine, soft delete, and export controls vary by SKU and operational requirements, so they should be calibrated to workload sensitivity rather than treated as universal Critical findings.

Remediation Quality

  • Fix resolves the vulnerability
  • Fix doesn't introduce new security issues
  • Fix doesn't break functionality
  • Issues found: Add supplemental Azure Container Registry gates to azure-review for admin account state, public network access, private endpoint/firewall evidence, RBAC/managed identity/service principal scope, repository-scoped tokens, Defender image scanning prerequisites, and retention/quarantine/export controls.

Comparison to Other Tools

Tool Catches this? Notes
Checkov / Trivy config / tfsec-style IaC scanners Partial Can flag some ACR admin or public network settings, but often miss the combined evidence chain across private endpoint, DNS, Defender access, scoped identities, and token scope maps.
Microsoft Defender for Cloud Partial Useful for runtime posture and registry image assessment, but IaC review still needs intended-state evidence and exception context.
Azure Policy Partial Can enforce selected ACR settings when policies are assigned, but the skill still needs to verify policy scope, exemptions, and identity/token posture.
CodeQL No Not a strong fit for Azure Terraform/Bicep registry posture evidence.

Overall Assessment

Strengths:

  • The skill already covers CIS Azure v2.1.0 identity, Defender for Cloud, storage, database, logging, networking, VM, Key Vault, and App Service sections.
  • Existing private endpoint, Defender plan, RBAC, and Key Vault checks provide a good pattern for supplemental service-specific controls.
  • The report format is already structured enough to add supplemental findings without disrupting CIS scoring.

Needs improvement:

  • Add ACR-specific evidence gates for azurerm_container_registry, ARM/Bicep Microsoft.ContainerRegistry/registries, private endpoints, network rule sets, and role assignments.
  • Distinguish public-to-all registries from public endpoint plus default-deny firewall exceptions.
  • Require admin-account exceptions and identity/RBAC evidence instead of treating registry authentication generically.
  • Record Defender image scanning and network-restricted registry prerequisites.

Priority recommendations:

  1. Add a supplemental ACR section to benchmark-checklist.md with ACR-REG-* controls that are reported separately from CIS scores.
  2. Add SKILL.md process/output guidance for supplemental Azure service findings.
  3. Include Terraform patterns for admin_enabled, public_network_access_enabled, network_rule_set, network_rule_bypass_option, azurerm_private_endpoint, role assignments, scope maps, tokens, Defender pricing, retention, and quarantine.
  4. Calibrate severity based on environment criticality, public reachability, identity scope, admin credential state, and scanning evidence.

References

Bounty Info

  • I have read and agree to the CONTRIBUTING.md bounty terms
  • Preferred payment method: PayPal or Base USDC, details to be provided privately after acceptance

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions