Skip to content

fix(queries): better interpreter for gcp queries#7912

Merged
cx-ricardo-jesus merged 48 commits into
masterfrom
AST-126467-update-metric-log-filter-to-be-more-permissive
Mar 3, 2026
Merged

fix(queries): better interpreter for gcp queries#7912
cx-ricardo-jesus merged 48 commits into
masterfrom
AST-126467-update-metric-log-filter-to-be-more-permissive

Conversation

@cx-andre-pereira

@cx-andre-pereira cx-andre-pereira commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

Closes #

Reason for Proposed Changes

  • The current query implementations use a regex to determine whether a filter is valid or not. This was not the best approach, as the filter could not appear defined exactly as defined in the CIS_Google_Cloud_Platform_Foundation_Benchmark_v4.0.0.

Proposed Changes

  • All the three queries, does the same verifications:

    • There is at least one"google_logging_metric" resource in the project and none contain the correct filter.
    • There is at least one "google_monitoring_alert_policy" resource in the project and none contain the filter/reference a logging metric that contains the correct filter.
    • There is at least one "google_monitoring_alert_policy" resource that contains the filter but none of them declare "notification_channels".
  • The queries Beta - Logs And Alerts Missing Project Ownership Assignement And Changes and Beta - Logs And Alerts Missing Audit Configuration Changes are using a really similar approach each one to handle the filter field, taking into account the following filters present in the respective CIS Benchmark page:

    • Query Beta - Logs And Alerts Missing Project Ownershio Assignement And Changes
      image
    • Query Beta - Logs And Alerts Missing Audit Configuration Changes:
      protoPayload.methodName="SetIamPolicy" AND protoPayload.serviceData.policyDelta.auditConfigDeltas:*
      
  • Both of them have at the beginning of the query the patterns that the queries are covering. The filter field is analysed using the single_match helper function that firstly removes the white spaces from the filter and then lowers all the characters within. After that, the filter does the verification that is has to do using the is_valid_filter helper function.

  • On the Beta - Logs And Alerts Missing Project Ownershio Assignement And Changes, this helper functions returns true if the service name is valid (defined to cloudresourcemanager.googleapis.com), if (ProjectOwnership OR projectOwnerInvitee), if (action="REMOVE" AND role="roles/owner") is present and if (action="ADD" AND role="roles/owner"). All these verifications take into account that the conditions can change places and the cases when a NOT is used in the filter.

  • For the query Beta - Logs And Alerts Missing Audit Configuration Changes is more simple, as it firstly only checks if the following cases happen:

    • methodName defined to SetIamPolicy AND auditConfigDeltas="*"
    • auditConfigDeltas="*" AND methodName="SetIamPolicy"
    • Cases when the De Morgan law happen - NOT(NOT A OR NOT B) == A AND B.
  • The query Beta - Logs And Alerts Missing Custom Role Changes took a different approach regarding the way it handles the filter field.

  • It uses the single_match helper function that firstly processes the filter by splitting it into an array with each element beginning with AND or OR operations, and then it checks if the field filter uses an improper filter using the is_improper_filter helper function.

  • This is_impproper_filter helper function firstly checks if the resource.type is not defined to iam.role using the correct_resource_type helper function, and then, if the resource type is correctly defined, it checks the other methodNames using the contains_method helper function.

  • That contains_method helper function firstly checks if there are not NOT statements in the methodNames being tackled by this query, and then it gathers a list of methodNames in the filters and checks if all of the four methodNames(CreateRole, DeleteRole, UpdateRole and UndeleteRole) are defined.

  • If all the method names aren't defined, it checks if a wildcard(protoPayload.nethodName="*") for the methodName is present in the filter.

I submit this contribution under the Apache-2.0 license.

@github-actions github-actions Bot added query New query feature gcp PR related with GCP Cloud labels Dec 12, 2025
@github-actions

github-actions Bot commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

kics-logo

KICS version: v2.1.18

Category Results
CRITICAL CRITICAL 0
HIGH HIGH 0
MEDIUM MEDIUM 0
LOW LOW 0
INFO INFO 0
TRACE TRACE 0
TOTAL TOTAL 0
Metric Values
Files scanned placeholder 1
Files parsed placeholder 1
Files failed to scan placeholder 0
Total executed queries placeholder 47
Queries failed to execute placeholder 0
Execution time placeholder 0

@github-actions github-actions Bot added the community Community contribution label Jan 20, 2026
@cx-ricardo-jesus cx-ricardo-jesus changed the title fix(query): better interpreter for gcp queries fix(queries): better interpreter for gcp queries Jan 21, 2026
@github-actions github-actions Bot added the terraform Terraform query label Jan 21, 2026
@cx-ricardo-jesus cx-ricardo-jesus marked this pull request as ready for review January 21, 2026 17:08
@cx-ricardo-jesus cx-ricardo-jesus requested a review from a team as a code owner January 21, 2026 17:08
@cx-ricardo-jesus cx-ricardo-jesus marked this pull request as draft February 9, 2026 09:58

@cx-miguel-dasilva cx-miguel-dasilva 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.

Since the 3 queries rely basically on the same logic to check the Log query, align them as much as possible to make them easier to maintain in the future. Add some function to rego libraries if you believe that can be achieved.

@cx-miguel-dasilva cx-miguel-dasilva 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.

Great Work!

@cx-ricardo-jesus cx-ricardo-jesus merged commit 9b5e90a into master Mar 3, 2026
29 checks passed
@cx-ricardo-jesus cx-ricardo-jesus deleted the AST-126467-update-metric-log-filter-to-be-more-permissive branch March 3, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Community contribution gcp PR related with GCP Cloud query New query feature terraform Terraform query

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants