Skip to content

fix(query): change to keyExpectedValue on ARM 'default_azure_storage_account_network_access_is_too_permissive' query - #8101

Open
cx-andre-pereira wants to merge 7 commits into
masterfrom
AST-169654-minor-fix-to-query-keyExpectedValue
Open

fix(query): change to keyExpectedValue on ARM 'default_azure_storage_account_network_access_is_too_permissive' query#8101
cx-andre-pereira wants to merge 7 commits into
masterfrom
AST-169654-minor-fix-to-query-keyExpectedValue

Conversation

@cx-andre-pereira

@cx-andre-pereira cx-andre-pereira commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Reason for Proposed Changes

  • Currently the "keyExpectedValue" field on the "default_azure_storage_account_network_access_is_too_permissive" query states that "publicNetworkAccess should be set to false", when the actual value recommendation should be setting the target publicNetworkAccess field to 'Disabled'.
  • Official documentation explicitly lists the valid values for the target field, those being :
    • 'Disabled' | 'Enabled' | 'SecuredByPerimeter'.

Proposed Changes

  • Simply adjusted the query's keyExpectedValue to properly reflect the valid options; now it is recommended that the publicNetworkAccess field be set to 'Disabled'.

  • Another small adjustment was made on the "deny" recommendation to reflect use of single quotes (for consistency sake given the keyActualValue(s) used) , and the use of uppercase on the value ('Deny'), as per the documented valid values on the defaultAction field.

Note

The query does not explicitly handle the case for the 'SecuredByPerimeter' value, considering that older documentation (2021) does not mention this value as a possibility, it is likely the query did not take it into account when first developed.

  • It was decided that the 'SecuredByPerimeter' value be considered unsafe and so changes to the query structure itself took place:

    • Adjusted the auxiliary functions of the query to, instead of specifically targeting the "Enabled"/"Allow" values as the only ones that should be flagged, take a more reliable approach of flagging for any value when publicNetworkAccess and/or defaultAction are not set to "Disabled"/"Deny" .
  • With this change any additions to the list of possible values for each of the fields will cause the query to flag said value and properly display it on the new keyActualValue(s).

  • I believe this approach to be more reliable in the sense that it's likely that future new values will also be deemed too permissive and, even if they are not, this way we will see False Positive results pop up which are substantially easier to identify over the False Negatives that could go by unnoticed with the current approach.

  • Added new tests for scenarios involving the 'SecuredByPerimeter' value of the publicNetworkAccess field.

I submit this contribution under the Apache-2.0 license.

…nexpected values that are not enabled/allow, like the new 'SecuredByPerimeter'; Added bew samples for the new possible value
@cx-andre-pereira
cx-andre-pereira marked this pull request as ready for review August 20, 2026 13:33

@cx-artur-ribeiro cx-artur-ribeiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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.

2 participants