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

fix(query): terraform/aws/iam_access_key_is_exposed #5846

Conversation

jycamier
Copy link
Contributor

Closes #5845

Referring to AWS CIS Controls 1.12

Proposed Changes

  • fix the query
  • fix the tests

I submit this contribution under the Apache-2.0 license.

@rafaela-soares rafaela-soares added community Community contribution query New query feature labels Sep 28, 2022
Copy link
Collaborator

@cxMiguelSilva cxMiguelSilva left a comment

Choose a reason for hiding this comment

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

Hi @jycamier, thank you so much for your contribution!!
Regarding the changes to the Security Query, I think that indeed the approach is incorrect and your approach fixes it to the correct behavior.
I would like just to request some changes.

  • Change the keyExpectedValue keyword from must to should in order to align the approach recently taken for all the new/changed Security Queries
  • Change the query matadata.json descriptionText to IAM Access Key should not be active for root users
  • If You feel comfortable updating the Ansible Security Query IAM Access Key Is Exposed to be aligned with the Terraform approach

@jycamier jycamier force-pushed the fix/query/iam_access_key_is_exposed branch from c6889bc to bc2fe5d Compare September 29, 2022 09:52
@jycamier
Copy link
Contributor Author

Hi @cxMiguelSilva

Change the keyExpectedValue keyword from must to should in order to align the approach recently taken for all the new/changed Security Queries

Done

Change the query matadata.json descriptionText to IAM Access Key should not be active for root users

Done

If You feel comfortable updating the Ansible Security Query IAM Access Key Is Exposed to be aligned with the Terraform approach

The rego, without any trouble.
The Ansible tests, not at all.

Copy link
Collaborator

@cxMiguelSilva cxMiguelSilva left a comment

Choose a reason for hiding this comment

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

@jycamier thank you so much for your contribution.
Regarding the Ansible Security Query do not worry about it, we will address it.
The changes LGTM

Copy link
Contributor

@rafaela-soares rafaela-soares left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Thank you so much, @jycamier!

@rafaela-soares rafaela-soares merged commit 97db4d0 into Checkmarx:master Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the query terraform/aws/iam_access_key_is_exposed seems to be wrong
3 participants