feat(engine): fixed parsing for tfplan files and update for "Encryption On Managed Disk Disabled" query #7866
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cx-andre-pereira
changed the title
Contributor
|
KICS version: v2.1.17
|
cx-andre-pereira
changed the title
…ttps://github.com/Checkmarx/kics into AST-121572_FN_encryption_on_managed_disk_disabled
…21572_FN_encryption_on_managed_disk_disabled
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #7516, closes #7307, closes #7265
Reason for Proposed Changes
Currently support for terraform "tfplan" files has some issues. The main problem was that, during parsing, resources of the same "type" and same "name" would overlap during the assignment process making it so only the last instance of any given type/name pair would be included in the resulting payload.
This can and did lead to many false negatives as a large part of the tfplan's "planned values" (the block that is supported by KICS), could go missing. In the sample used to raise this issue for example the payload ended up being 1/4 the expected size since all resources for each specific type had the same generic name associated with them.
As for the "Encryption On Managed Disk Disabled" query, it was very outdated since it was based on a deprecated field from the target
encryption_settingsblock on theazurerm_managed_diskresource. The legacy encryption_settings block had a required "enabled" field which the current query's logic is based off of.Since the
enabledfield no longer exists it is possible to have emptyencryption_settingsblocks declared and, in that instance, the query will not flag because it assumes the "enabled" field must be explicitly set to false for encryption to be disabled.Proposed Changes
To solve the issue on the "
tfplan" parsing logic of the "readModule" function was altered to use the "address" field when creating the objects for each resource instance. The "address" field is the key field used to index each individual resource instances as per the documentation. It should always have been the key for the resulting payload's resources as well.Fixed the target query's logic to handle not only the case of the
encryption_settingsblock being undefined orenabledset to false, but also the case ofencryption_settingsbeing an empty block.I submit this contribution under the Apache-2.0 license.