bug(misconf): AVD-KSV-0109 false positive on Helm ConfigMap using string interpolation

[nikpivkin]

Discussed in #8865

Some checks are currently applied to different types of configuration files, which in some cases can lead to false positives.

For example:

• Kubernetes checks are applied both to plain Kubernetes manifests and to rendered Helm templates. The AVD-KSV-0109 check that detects secrets in ConfigMaps, falsely triggers on interpolated strings like:

  REDIS_MASTER_PASSWORD: "$(cat ${REDIS_MASTER_PASSWORD_FILE})"

  Such expressions do not indicate a secret leak in the case of Helm, as they remain unchanged in the rendered manifest. However, if a user applies a Kubernetes manifest using envsubst, like:

  SOME_ACCESS_TOKEN=test envsubst < config.yaml | kubectl apply -f -

  — the variables are actually substituted, and sensitive data may be exposed.

• Dockerfile checks, such as AVD-DS-0016, are applied both to original Dockerfiles and to those restored from image history.
  However, multistage builds cannot be accurately restored - information about stages (FROM ... AS <name>) is not stored in the image history. As a result, making the AVD-DS-0016 check invalid in this context.
  We have already addressed this issue by filtering such checks directly in the image history analyzer after scanning.

Possible solutions:

1. Ignore interpolated strings, such as ${VAR} or $(...).
   This is reasonable for Helm templates, where variables remain unresolved. However, this approach is unsafe for regular Kubernetes manifests, as they may be pre-rendered by the user and variables may contain actual secrets.

2. Completely disable the check for Helm. This reduces coverage but ensures that false positives do not occur.

3. Expose the scanner type or configuration source to Rego, so that checks can determine applicability themselves:

   deny contains res if {
     not skip_check
     ...
   }
   
   skip_check(s) if {
     input.scanner == "helm"
     startswith(s, "${")
   }

   This enables flexible policies that take into account the scan context (source, format, etc.).

[simar7]

My thoughts:

Ignore interpolated strings, such as ${VAR} or $(...).

I think this is the most reasonable solution. If the user expects Trivy to scan an input where a decision could be made conditionally, they need to provide us with the required input in the first place. To me not having the variables populated (just placeholders) is insufficient information to evaluate.

Completely disable the check for Helm. This reduces coverage but ensures that false positives do not occur.

If we implement point 1, why would we need to completely disable the check?

Expose the scanner type or configuration source to Rego, so that checks can determine applicability themselves

While this is nice, I don't think we can expect this from the average Trivy user to do. It could be an advanced user feature but still writing a new rego check to get around seems a bit difficult to do. It would be easier for users to provide Trivy with populated values, similar to what we do with --tf-vars.

Another idea partially based on the last point is if we provide users with a new flag that can accept a comma separated value of key, value pairs where the user could provide us with the values. This would be similar to --tf-vars but for any templated/placeholder value.

[nikpivkin]

they need to provide us with the required input in the first place

I think this is appropriate in the case of Terraform variables or CloudFormation parameters, but the k8s manifest we scan as is and I don't see the need for users to pass in input data as no evaluation of the manifest can be done. In the issue description I gave an example of such a case. I thought again and came to the conclusion that also with k8s the data could be stored as is and then the scan would cause a false positive.

If we implement point 1, why would we need to completely disable the check?

I suggested different solutions to the problem, these are not steps of implementation of one solution :)

While this is nice, I don't think we can expect this from the average Trivy user to do

I forgot to write that the export of scanner type or configuration source will be done inside Trivy, nothing needs to be done from the user side.

So, should we go with the first option?

[nikpivkin]

Fixed in aquasecurity/trivy-checks#442