Skip to content

feat(scanner): add rule AZ-STOR-010 storage account missing private endpoint - #327

Open
shariqueahmad108-ship-it wants to merge 2 commits into
OWASP:devfrom
shariqueahmad108-ship-it:rule/az-stor-010-private-endpoint
Open

feat(scanner): add rule AZ-STOR-010 storage account missing private endpoint#327
shariqueahmad108-ship-it wants to merge 2 commits into
OWASP:devfrom
shariqueahmad108-ship-it:rule/az-stor-010-private-endpoint

Conversation

@shariqueahmad108-ship-it

Copy link
Copy Markdown
Contributor

Implements #322 — a Storage rule that detects storage accounts reachable over the public network with no approved Private Endpoint.

Note on the rule ID: the issue proposed AZ-STOR-007, but that ID is already used (Storage Account Allows TLS Below 1.2) and the storage rules currently run through AZ-STOR-009, so this lands as AZ-STOR-010. Happy to renumber if you'd prefer.

Detection

For each storage account:

  • If public_network_access is DisabledNOT_APPLICABLE (already network-isolated regardless of private endpoints — no false finding).
  • Else, flag (HIGH) when there is no privateEndpointConnections entry whose privateLinkServiceConnectionState.status is Approved (empty, null, or only Pending/Rejected connections).

Reads the real azure.mgmt.storage.models shape (StorageAccount.private_endpoint_connectionsPrivateEndpointConnection.private_link_service_connection_state.status).

Compliance mappings

Framework Control
NIST CSF PR.AC-5 Network integrity is protected
ISO 27001 A.13.1.3 Segregation in networks
SOC 2 CC6.6 Restricts access from outside the network boundary
CIS N/A-STOR-010 No numbered CIS control for storage private endpoints — repo N/A convention (as AZ-STOR-006..009)

Tests

Four unit tests in tests/test_rules_storage.py, exercising genuine SDK models (PrivateEndpointConnection / PrivateLinkServiceConnectionState), not just SimpleNamespace:

  • approved Private Endpoint → no finding
  • public_network_access=Disabled → NOT_APPLICABLE
  • no Private Endpoint, public reachable → one HIGH finding
  • only a Pending connection → flagged (not Approved)
pytest tests/test_rules_storage.py   -> 23 passed
ruff check .            -> clean
ruff format --check .   -> clean

Honest testing note

I don't have an Azure subscription, so I couldn't run this against a live account. The detection is fully covered by the unit tests above, which instantiate real azure-mgmt-storage models so the attribute shape (private_endpoint_connections, private_link_service_connection_state.status) is validated against the pinned SDK rather than invented.

Includes the remediation playbook (fix_az_stor_010.sh) with guarded args.

Resolves #322.

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rule logic, SDK attribute paths, enum handling, playbook, and compliance mappings are all correct. The four tests cover the main paths well. Two issues need fixing before merge.


Bug 1 No test for private_endpoint_connections = None (potential false positive in production)

The rule's _has_approved_private_endpoint helper does:

for connection in getattr(account, "private_endpoint_connections", None) or []:

The or [] means a None value (what StorageAccount.__init__ sets by default before the SDK populates it from the API response) is silently treated as "no approved endpoint," flagging the account as HIGH. That is the safe direction for a security rule, but if get_storage_accounts() returns real SDK objects whose PE list was not populated (e.g., because the caller lacks Microsoft.Storage/storageAccounts/privateEndpointConnections/read permission), every such account gets a false HIGH finding.

The fix is not to change the rule logic but to:

  1. Add a test asserting the current behavior so it is documented and intentional.
  2. Add a logger.warning for the None case (consistent with how az_stor_009.py handles None container lists and az_sc_002.py handles unknown public_network_access), so operators can distinguish a real absence of PEs from a permissions gap.

Bug 2 docs/rules-reference.md not updated

Every existing storage rule through AZ-STOR-009 has a row in docs/rules-reference.md. AZ-STOR-010 has no row. The engine auto-discovers rules via glob so the rule runs, but the reference doc is the human-readable registry rendered on the learn page. Please add the missing row.


Observation (not blocking, pre-existing) stale control ID in nist_csf.json

The AZ-STOR-010 NIST CSF entry in the diff is correct (PR.AC-5), but a nearby pre-existing entry for AZ-DB-007 in the same file uses "control_id": "A.12.4.1" which is an ISO 27001 ID in a NIST file. Not introduced by this PR and not a blocker here, but flagging since the author touched this file.


Verified correct

Detection field path (private_endpoint_connectionsprivate_link_service_connection_state.status) is correct per the pinned Azure SDK. enum_str handles both real SDK enums and plain strings correctly. No RULE_ID collision. Playbook (fix_az_stor_010.sh) exists, uses set -euo pipefail, validates args, correctly creates the PE then disables public network access. All 4 compliance frameworks updated with accurate control mappings. NOT_APPLICABLE handling for public_network_access == "Disabled" accounts is correct.

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the rule implementation, Azure SDK usage, compliance mappings, remediation playbook, and tests.

The core detection logic looks correct, but two issues remain: unavailable private-endpoint evidence is silently treated as confirmed absence, and AZ-STOR-010 is missing from the rules reference documentation.

Please address both issues and add the corresponding regression coverage before requesting another review.

…ndpoint

Adds a Storage rule that flags storage accounts reachable over the public
network that have no approved Private Endpoint connection, so their
blob/file/queue/table endpoints stay reachable from the internet instead of
staying on a private IP inside a VNet.

Detection reads the real azure-mgmt-storage model shape: an account is flagged
when private_endpoint_connections has no entry whose
private_link_service_connection_state.status is "Approved". An account whose
public_network_access is already "Disabled" is treated as NOT_APPLICABLE
(network-isolated by another means), so the rule does not raise a false finding.

Includes the remediation playbook (creates a blob Private Endpoint and sets
public network access to Disabled; args guarded per the fix_az_net_016.sh
convention), four unit tests (approved / pending-only / none / public-disabled)
that exercise genuine SDK models, and framework mappings: NIST PR.AC-5,
ISO 27001 A.13.1.3, SOC 2 CC6.6, and the repo's N/A convention for CIS.

Resolves OWASP#322. The issue proposed the id AZ-STOR-007, but that id is already in
use (TLS below 1.2) and the storage rules run through AZ-STOR-009, so this
lands as AZ-STOR-010.

Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
…terminate; document the rule

Addresses review feedback on OWASP#327:

- Indeterminate evidence: private_endpoint_connections of None means the field
  was not populated / could not be read, not a confirmed absence. The rule now
  skips such an account (logging a warning) instead of flagging it, so it does
  not raise a false finding from missing evidence. A genuine empty list, or
  connections with none in the Approved state, is still a finding.
- Adds a regression test for the None (unavailable) case.
- Documents AZ-STOR-010 in docs/rules-reference.md.

Signed-off-by: shariqueahmad108-ship-it <shariqueahmad108@gmail.com>
@shariqueahmad108-ship-it
shariqueahmad108-ship-it force-pushed the rule/az-stor-010-private-endpoint branch from 786f14b to 94ea7cf Compare September 9, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add rule AZ‑STOR‑007 — Storage Account Missing Private Endpoint

3 participants