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(terraform): Update CKV_AZURE_164 to correct check on trust policy #6757

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

paddymorgan84
Copy link
Contributor

@paddymorgan84 paddymorgan84 commented Oct 8, 2024

User description

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

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

New/Edited policies (Delete if not relevant)

Description

Include a description of what makes it a violation and any relevant external links.

Fix

How does someone fix the issue in code and/or in runtime?

Checklist:

  • 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

Generated description

Below is a concise technical summary of the changes proposed in this PR:

Update the ACRUseSignedImages class to correct the trust policy check for Azure Container Registries. Modify the scan_resource_conf method to accurately evaluate the trust_policy_enabled and trust_policy/enabled keys, ensuring that the check passes only when the trust policy is enabled. Adjust the test cases in test_ACRUseSignedImages.py to reflect these changes, ensuring that both new and old configurations are tested for compliance.

TopicDetails
Test Case Update Update test cases to validate the new trust policy logic in Azure Container Registry configurations.
Modified files (3)
  • tests/terraform/checks/resource/azure/test_ACRUseSignedImages.py
  • tests/terraform/checks/resource/azure/example_ACRUseSignedImages/main.tf
  • tests/terraform/checks/resource/azure/example_ACRGeoreplicated/main.tf
Latest Contributors(2)
UserCommitDate
patrickmorgan1984@gmai...fix-terraform-Update-C...November 27, 2024
JamesWoolfendenfeat-terraform-new-azu...December 28, 2022
Trust Policy Fix Correct the trust policy check in ACRUseSignedImages to ensure accurate evaluation of Azure Container Registry configurations.
Modified files (1)
  • checkov/terraform/checks/resource/azure/ACRUseSignedImages.py
Latest Contributors(2)
UserCommitDate
patrickmorgan1984@gmai...fix-terraform-Update-C...November 27, 2024
JamesWoolfendenfeat-terraform-Azure-u...November 27, 2022
This pull request is reviewed by Baz. Join @paddymorgan84 and the rest of your team on (Baz).

@SteveVaknin SteveVaknin changed the title fix: Update CKV_AZURE_164 to correct check on trust policy fix(terraform): Update CKV_AZURE_164 to correct check on trust policy Nov 13, 2024
@paddymorgan84
Copy link
Contributor Author

paddymorgan84 commented Nov 19, 2024

Changes should be ready @tsmithv11, including the linting I missed from the previous PR. I've also raised a PR in prisma-cloud-docs to update documentation: hlxsites/prisma-cloud-docs#933

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you modify this file? Looks unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a quick search on where else trust_policy was used and thought it would be worth updating others to match the latest version too. I can remove if this isn't preferable

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not necessary, but since the check associated with that file doesn't look at that field, it shouldn't matter. I'll trigger the UTs to confirm.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good!

@paddymorgan84
Copy link
Contributor Author

@tsmithv11 Any suggestions for someone else to review this for me so it's good to merge?

Copy link
Contributor

@talazuri talazuri left a comment

Choose a reason for hiding this comment

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

LGTM

@tsmithv11 tsmithv11 merged commit aa0c12c into bridgecrewio:main Nov 27, 2024
37 of 38 checks passed
Saarett pushed a commit that referenced this pull request Nov 27, 2024
…#6757)

fix: Update CKV_AZURE_164 to correct check on trust policy

Co-authored-by: Taylor <[email protected]>
@paddymorgan84 paddymorgan84 deleted the trust-policy branch November 27, 2024 14:57
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.

4 participants