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(MT.1006 and MT.1014): Correctly handle empty includeUsers collection in policy conditions #558

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

PTMohr
Copy link
Contributor

@PTMohr PTMohr commented Dec 6, 2024

Fix Policy Conditions Bug

This pull request fixes a bug in the policy conditions logic where an empty includeUsers collection would incorrectly return false when checking if 'All' is not included.

The bug was caused by the use of the -ne operator, which filters out the right-hand side of the comparison and returns the rest of the collection. When the collection is empty, this would return an empty collection, which is always considered false.

By changing the comparison to use the -notcontains operator, we ensure that the check always returns a boolean value, correctly handling the case where the includeUsers collection is empty.

Changes:

  • Replaced -ne with -notcontains in policy conditions logic

@PTMohr PTMohr requested a review from a team as a code owner December 6, 2024 11:59
@PTMohr PTMohr changed the title fix(MT.1006): Correctly handle empty includeUsers collection in policy conditions fix(MT.1006 and MT.1014): Correctly handle empty includeUsers collection in policy conditions Dec 6, 2024
Copy link
Contributor

@merill merill left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks @PTMohr!

@merill merill merged commit 9929c4a into maester365:main Dec 8, 2024
4 checks passed
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.

2 participants