Skip to content

fix(query): fixed false negative for "App Service Authentication Disabled" query missing resources#7591

Merged
cx-ricardo-jesus merged 13 commits into
Checkmarx:masterfrom
cx-ricardo-jesus:AST-40766
Aug 12, 2025
Merged

fix(query): fixed false negative for "App Service Authentication Disabled" query missing resources#7591
cx-ricardo-jesus merged 13 commits into
Checkmarx:masterfrom
cx-ricardo-jesus:AST-40766

Conversation

@cx-ricardo-jesus

@cx-ricardo-jesus cx-ricardo-jesus commented Jul 25, 2025

Copy link
Copy Markdown
Contributor

Closes #

Reason for Proposed Changes

  • The query App Service Authentication Disabled was not taking into consideration the resources azurerm_linux_web_app and azurerm_windows_web_app not verifying if the authentication settings for these two resources are enabled.

Proposed Changes

  • After analyzing the documentation regarding the "azurerm_app_service", "azurerm_linux_web_app" and "azurerm_windows_web_app" resources, I maintained the same verifications for the resource that was already being analyzed and extended the query in order to verify the authentication field on the two other resources.
  • Comparing the resources azurerm_linux_web_app and azurerm_windows_web_app with the resource azurerm_app_service I realized that there is another authentication field called auth_settings_v2 that was not present on the last one.
  • So, taking what I said in the last point into account, I kept the policies that tackles the vulnerabilities on the resource azurerm_app_service, and added an extra one that uses a helper function called prepare_issues, which verifies not only the auth_settings field but also the auth_settings_v2 field and handles all the cases regarding the new resources that are going to be analyzed in this query for these two fields.
  • For the new resources, I kept the same reasoning used on the resource that was already on the query for the auth_settings field because, according to the information present on the documentation, this field has equal settings and fields within for all the resources.
  • For the field auth_settings_v2, there is small things that changes in comparison to the field auth_settings, more precisely, the field auth_settings_v2 not being required and set to false by default, which should return a positive result, made me deal with it in a slightly different way, returning a positive result on the query when this field is not defined.
  • On top of that, I added eight new samples that should return a positive result and four more Terraform samples that should return a negative result. All of these new test sample are provided in order to cover all the possibilities regarding positive and negative results.
  • NOTE: When both fields auth_settings and auth_settings_v2 are not defined, I only inserted the auth_settings block on the remediation field and not the auth_settings_v2 but this positive case can be solved defining the field auth_settings_v2 instead of the auth_settings field.
    I submit this contribution under the Apache-2.0 license.

@cx-ricardo-jesus cx-ricardo-jesus requested a review from a team as a code owner July 25, 2025 14:56
@github-actions github-actions Bot added query New query feature terraform Terraform query labels Jul 25, 2025

@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, nice work on all the test cases included.
Just left a change on your comment but nothing too important.

Comment thread assets/queries/terraform/azure/app_service_authentication_disabled/query.rego Outdated
cx-ricardo-jesus and others added 2 commits August 7, 2025 09:37
…bled/query.rego

Co-authored-by: Artur Ribeiro <153724638+cx-artur-ribeiro@users.noreply.github.com>
@gitguardian

gitguardian Bot commented Aug 7, 2025

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
4266022 Triggered Generic Password 1602b39 assets/queries/cloudFormation/aws/amplify_branch_basic_auth_config_password_exposed/test/negative7.yaml View secret
9419039 Triggered Username Password 1602b39 assets/queries/cloudFormation/aws/amplify_branch_basic_auth_config_password_exposed/test/positive5.yaml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@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

@cx-ricardo-jesus cx-ricardo-jesus merged commit 969a27a into Checkmarx:master Aug 12, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

query New query feature terraform Terraform query

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants