Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(terraform): Mask secret values in Terraform plan file reports by resource #3868

Merged
merged 13 commits into from
Nov 14, 2022

Conversation

arielkru
Copy link
Contributor

@arielkru arielkru commented Nov 13, 2022

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

  • This PR enables masking secret values that are not classified as secrets.
  • The solution is a general one, implemented only in tf plan runner for now, for 3 types of resource & attributes that their values should be hidden (azurerm_key_vault_secret: value, aws_secretsmanager_secret_version: secret_string, google_kms_secret_ciphertext: plaintext).
  • Can be extended to future runners by adding the resource_attributes_to_omit map when calling omit_secret_value_from_checks()

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@arielkru arielkru marked this pull request as ready for review November 14, 2022 08:10
@gruebel gruebel changed the title feat(terraform): Censor secret values from tfplan file reports by resource feat(terraform): Mask secret values in Terraform plan file reports by resource Nov 14, 2022
Copy link
Contributor

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

nice work, added few minor suggestions

checkov/common/typing.py Outdated Show resolved Hide resolved
checkov/common/util/secrets.py Outdated Show resolved Hide resolved
checkov/common/util/secrets.py Outdated Show resolved Hide resolved
checkov/common/util/secrets.py Outdated Show resolved Hide resolved
checkov/common/util/secrets.py Outdated Show resolved Hide resolved
checkov/terraform/plan_runner.py Outdated Show resolved Hide resolved
checkov/terraform/plan_runner.py Outdated Show resolved Hide resolved
Copy link
Contributor

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

🍸

@arielkru arielkru merged commit 3ca2d60 into master Nov 14, 2022
@arielkru arielkru deleted the censor_secrets_by_resource branch November 14, 2022 12:39
ayajbara pushed a commit that referenced this pull request Nov 15, 2022
… resource (#3868)

* add test

* update set with list[str]

* typing

* linting

* add secret based test

* aws provider multi secret test

* Update checkov/common/util/secrets.py

Co-authored-by: Anton Grübel <[email protected]>

* code improvements

* code improvements

* code improvements

* add resources to censor

* dict to Dict

* typing

Co-authored-by: Anton Grübel <[email protected]>
ayajbara pushed a commit that referenced this pull request Nov 15, 2022
… resource (#3868)

* add test

* update set with list[str]

* typing

* linting

* add secret based test

* aws provider multi secret test

* Update checkov/common/util/secrets.py

Co-authored-by: Anton Grübel <[email protected]>

* code improvements

* code improvements

* code improvements

* add resources to censor

* dict to Dict

* typing

Co-authored-by: Anton Grübel <[email protected]>
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.

3 participants