Skip to content

Conversation

@tinar10
Copy link
Collaborator

@tinar10 tinar10 commented Jan 19, 2026

Added the remaining 13 controls for Section 2 of the M365 CIS benchmark. Created the local tests and meticulously validated that their output from the collectors is correct.

These are the current results compared with the M365 portal settings for Microsoft Defender.
image

randomly performed negative testing for Section 2 controls 2.1.3 and 2.4.4.
image

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f935d4fd72

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


# Main evaluation
result := output if {
policies := input.teams_protection_policies

Choose a reason for hiding this comment

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

P1 Badge Read Safe Links data from the wrong input key

This control reads input.teams_protection_policies, but the Safe Links collector populates safe_links_policies (see collectors/exchange/protection/safe_links_policy.py). When evaluated, policies is undefined/empty and non_compliant becomes empty, so the rule reports compliant even if Safe Links for Office is disabled. That produces false positives for CIS 2.1.1.

Useful? React with 👍 / 👎.

Comment on lines 78 to 82
# Non-compliant: policies exist but none match requirements
antiphish_configured if {
count(input.antiPhishPolicies) > 0
count({p | matching_policy[p]}) == 0
}

Choose a reason for hiding this comment

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

P1 Badge Marking non-matching anti-phish policies as compliant

This branch sets antiphish_configured to true when policies exist but none satisfy the required settings. That inverts the intended result and will mark misconfigured tenants as compliant whenever there are any anti-phish policies but no matches. This should set the boolean false (or use a separate non-compliant flag) instead.

Useful? React with 👍 / 👎.

Comment on lines 45 to 46
ip_allow_list_is_empty := false if {
input.IPAllowList != {} # non-empty object

Choose a reason for hiding this comment

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

P2 Badge Empty IPAllowList triggers conflicting rule values

The ip_allow_list_is_empty := false if input.IPAllowList != {} branch is true for the compliant empty-array case ([] != {}), which conflicts with the earlier == [] branch. For an empty allow list, this yields multiple values for a complete rule, causing the evaluation to be undefined or error. The condition should distinguish object vs array or use else.

Useful? React with 👍 / 👎.

Comment on lines 42 to 43
enable_safe_list_is_false := null if {
not input.EnableSafeList # If EnableSafeList is missing, return null

Choose a reason for hiding this comment

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

P2 Badge Missing-value branch fires for compliant false value

The not input.EnableSafeList check is also true when EnableSafeList is explicitly false (the compliant state). That produces both true and null for enable_safe_list_is_false, so the rule can conflict or become undefined. The missing-value branch should only match an undefined field (e.g., input.EnableSafeList == null).

Useful? React with 👍 / 👎.

Comment on lines 33 to 35
"ActionWhenThresholdReached": "BlockUser",
"NotifyOutboundSpamRecipients": {"monitored@example.com"}
}

Choose a reason for hiding this comment

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

P2 Badge Hard-coded recipient forces false negatives

Compliance currently requires NotifyOutboundSpamRecipients to equal the literal set {"monitored@example.com"}. Any tenant that correctly configures different administrator recipients will be flagged non-compliant even though the control only requires recipients to be configured. The unused validate_notify_outbound_spam_recipients suggests this should be a non-empty check instead of a fixed address.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75eb3a7204

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 46 to 48
matching_policy[policy_obj] if {
idx := input.antiPhishPolicies[_]
policy_obj := input.antiPhishPolicies[idx]

Choose a reason for hiding this comment

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

P1 Badge Fix anti-phish policy iteration to avoid undefined lookups

When input.antiPhishPolicies is an array (the typical collector shape), idx := input.antiPhishPolicies[_] binds idx to the element value, not the index. The subsequent input.antiPhishPolicies[idx] lookup is therefore undefined, so matching_policy never yields any policy objects and matching_rule can’t match. This makes the control report noncompliant even when a valid policy exists. Consider iterating over the elements directly (or binding an index with input.antiPhishPolicies[i]) so the lookup succeeds for array inputs.

Useful? React with 👍 / 👎.

Comment on lines 23 to 27
non_compliant_policies = [policy.Name |
policy := input.policies[_]
policy.EnableATPForSPOTeamsODB == false
policy.EnableSafeDocs == false
policy.AllowSafeDocsOpen == true

Choose a reason for hiding this comment

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

P2 Badge Flag any single misconfigured Safe Attachments setting

The noncompliance list only includes policies where all three settings are wrong simultaneously (EnableATPForSPOTeamsODB == false, EnableSafeDocs == false, and AllowSafeDocsOpen == true). If just one of these settings is misconfigured (e.g., EnableSafeDocs is false while the others are correct), the policy is omitted and the control reports compliant, yielding a false negative. This check should treat any single required setting failure as noncompliance.

Useful? React with 👍 / 👎.

Comment on lines 34 to 36
allowed_sender_domains_undefined := false if {
input.AllowedSenderDomains != null # If AllowedSenderDomains is defined
}

Choose a reason for hiding this comment

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

P2 Badge Accept empty AllowedSenderDomains as compliant

The rule treats any defined AllowedSenderDomains value as noncompliant (input.AllowedSenderDomains != null), which includes an explicitly empty list. The CIS recommendation is “Do not define any allowed domains,” and collectors often represent that as an empty list rather than a missing field. With the current logic, those compliant configurations are flagged as noncompliant, causing false positives for tenants that have the list present but empty.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17cce10758

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 25 to 28
required_fields := {
"ZapEnabled": true,
"ExceptIfConditions": "valid_condition"
}

Choose a reason for hiding this comment

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

P1 Badge Validate Safe Links for Office instead of unrelated fields

This control is about Safe Links for Office applications, but the compliance logic only checks ZapEnabled and ExceptIfConditions. That means a policy can be marked compliant even when Safe Links for Office is disabled, as long as those unrelated fields happen to match, leading to false positives. The check should key off the Office Safe Links setting (e.g., EnableSafeLinksForOffice) rather than these fields.

Useful? React with 👍 / 👎.

Comment on lines 98 to 101
result := {
"compliant": antiphish_configured == true,
"message": generate_message(antiphish_configured),
"affected_resources": generate_affected_resources(antiphish_configured, input),

Choose a reason for hiding this comment

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

P1 Badge Treat unmatched anti-phish configs as noncompliant

When policies/rules exist but none meet the required settings, antiphish_configured is never set to false (only true or null). In that case generate_message(antiphish_configured) has no matching rule, so the result falls back to the default “Evaluation failed” instead of reporting noncompliance. This misclassifies real misconfigurations; add an explicit false assignment (e.g., from antiphish_non_compliant) so non-matching policies are reported as noncompliant.

Useful? React with 👍 / 👎.

Comment on lines 59 to 60
rule := input.malware_filter_rules[_]
rule.malware_filter_policy == policy.id

Choose a reason for hiding this comment

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

P1 Badge Avoid reliance on uncollected malware_filter_rules

This rule requires input.malware_filter_rules to join policies to enabled rules, but the repo only collects malware filter policies (there is no collector that populates malware_filter_rules). As a result this block never matches and any_policy_compliant stays undefined, causing the policy to fall back to the default failure even when settings are correct. Either collect and pass the rules data or remove the rule dependency so compliance can be evaluated.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3ff683537

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 23 to 24
non_compliant_policies = [policy.Name |
policy := input.policies[_]

Choose a reason for hiding this comment

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

P1 Badge Use collector output for ATP policy evaluation

The ATP policy collector returns a single atp_policy object plus top-level booleans (see engine/collectors/exchange/protection/atp_policy_o365.py), but this rule iterates input.policies. When the collector output is used, input.policies is absent, so the list comprehension is empty and compliant becomes true by default, which will incorrectly pass this control even when EnableATPForSPOTeamsODB/EnableSafeDocs are disabled. This is a false-positive risk for any tenant whose configuration is actually non‑compliant.

Useful? React with 👍 / 👎.

Comment on lines 47 to 51
policy_identity := policy.identity

missing := [ext |
ext := attach_exts[_]
not ext in policy.file_types

Choose a reason for hiding this comment

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

P2 Badge Use raw malware policy field names

The malware filter collector passes through raw PowerShell policy objects (see engine/collectors/exchange/protection/malware_filter_policy.py), which use Identity, FileTypes, and EnableFileFilter. This rule looks for policy.identity and policy.file_types, so the extension checks and identity mapping evaluate against undefined fields; as a result, missing_exts/is_compliant never match real data and the control will report non‑compliance even when the policy is correctly configured. This will systematically mis-score tenants with valid malware filter settings.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6474b3ebbb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 42 to 46
outbound_spam_monitoring_enabled := null if {
not input.BccSuspiciousOutboundMail
not input.NotifyOutboundSpam
not input.BccSuspiciousOutboundAdditionalRecipients
not input.NotifyOutboundSpamRecipients

Choose a reason for hiding this comment

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

P2 Badge Avoid conflicting complete rules for outbound_spam_monitoring_enabled

The outbound_spam_monitoring_enabled complete rule is defined elsewhere in this file as := false with an else chain, but this separate := null rule is not part of that chain. When the booleans are false and the recipient arrays are omitted (a common shape when these settings are disabled), both the false rule and this null rule can match, which causes Rego to throw a “complete rules must not produce multiple outputs” error and the control fails to evaluate instead of reporting non-compliance/unknown.

Useful? React with 👍 / 👎.

Comment on lines 47 to 50
count({k | required_policy_fields[k] == input[k]}) == count(required_policy_fields)

# Ensure at least one monitored recipient is set
validate_notify_outbound_spam_recipients

Choose a reason for hiding this comment

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

P2 Badge Don’t require notification recipients for message-limit control

This control is scoped to outbound message limits and the over‑limit action, but the compliance logic also requires NotifyOutboundSpamRecipients to be populated. That means a policy with the correct limits/action but no notification recipients (which is covered by CIS‑2.1.6) will be marked non‑compliant here, producing false negatives for organizations that intentionally separate message limits from notification settings.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29a8c08902

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 24 to 26
file_filter_enabled := true if input.data.enable_file_filter == true
file_filter_enabled := false if input.data.enable_file_filter != true
file_filter_enabled := null if input.data.enable_file_filter == null

Choose a reason for hiding this comment

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

P2 Badge Read common attachment flag from correct input path

The malware filter collector returns enable_file_filter at the top level (engine/collectors/exchange/protection/malware_filter_policy.py:48-54), but this policy reads input.data.enable_file_filter, which will be undefined because the collected data is passed directly as the OPA input. As a result file_filter_enabled never resolves and the control falls back to the default “Evaluation failed” even when the collector has the correct value.

Useful? React with 👍 / 👎.

Comment on lines 25 to 32
notifications_enabled if {
count(input.policies) > 0
}

violating_policies[policy.Identity] if {
some i
policy := input.policies[i]
not policy.EnableInternalSenderAdminNotifications

Choose a reason for hiding this comment

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

P2 Badge Iterate malware policies from collected field name

The malware filter collector exposes policies as malware_filter_policies (engine/collectors/exchange/protection/malware_filter_policy.py:48-54), but this rule iterates input.policies, which will be undefined. That makes notifications_enabled/policies_count undefined and the result falls back to the default failure even when policies are collected.

Useful? React with 👍 / 👎.

Comment on lines 25 to 28
input.data.enable == true
input.data.action == "Block"
input.data.quarantine_tag == "AdminOnlyAccessPolicy"
input.data.identity == "Built-In Protection Policy"

Choose a reason for hiding this comment

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

P2 Badge Use safe attachment collector fields instead of input.data

The safe attachment collector returns safe_attachment_policies and policies_with_protection (engine/collectors/exchange/protection/safe_attachment_policy.py:53-56), but this policy reads input.data.*, which is never populated. With the current input shape, the compliance rule cannot evaluate and will always emit the default “Evaluation failed” result.

Useful? React with 👍 / 👎.

Comment on lines 27 to 31
outbound_spam_monitoring_enabled := true {
input.BccSuspiciousOutboundMail == true
input.NotifyOutboundSpam == true
count(input.BccSuspiciousOutboundAdditionalRecipients) > 0
count(input.NotifyOutboundSpamRecipients) > 0

Choose a reason for hiding this comment

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

P2 Badge Align outbound spam notification fields with collector

The hosted outbound spam collector outputs bcc_suspicious_outbound_mail and notify_outbound_spam in snake_case (engine/collectors/exchange/protection/hosted_outbound_spam_filter.py:50-56) and does not include the additional-recipient lists this rule expects. Reading input.BccSuspiciousOutboundMail/input.NotifyOutboundSpamRecipients therefore yields undefined values, leaving outbound_spam_monitoring_enabled null and reporting unknown even when the collector has valid data.

Useful? React with 👍 / 👎.

Comment on lines 46 to 56
matching_policy[policy_obj] {
policy_obj := input.antiPhishPolicies[_]
count({k | required_policy_fields[k] == policy_obj[k]}) == count(required_policy_fields)

# Optional: check targeted users are within limits
count(policy_obj.TargetedUsersToProtect) > 0
count(policy_obj.TargetedUsersToProtect) <= 350
}

matching_rule[rule_obj] {
rule_obj := input.antiPhishRules[_]

Choose a reason for hiding this comment

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

P2 Badge Avoid expecting anti-phish rules that aren’t collected

The anti-phish collector only returns anti_phish_policies and no rules (engine/collectors/exchange/protection/anti_phish_policy.py:47-50), but this policy requires input.antiPhishPolicies and input.antiPhishRules. With the current input shape, matching_rule and the null checks never evaluate as intended, so the control cannot report compliance even when policies are present.

Useful? React with 👍 / 👎.

Comment on lines 33 to 40
ip_allow_list_is_empty := true {
input.IPAllowList == [] # empty array
} else := true {
input.IPAllowList == {} # empty object
} else := false {
input.IPAllowList != {} # non-empty object
} else := false {
input.IPAllowList != [] # non-empty array

Choose a reason for hiding this comment

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

P2 Badge Read IP allow list from collector’s snake_case key

The hosted connection filter collector returns ip_allow_list (engine/collectors/exchange/protection/hosted_connection_filter.py:50-55), but this policy reads input.IPAllowList. That field will be undefined, so ip_allow_list_is_empty resolves to null and the control always reports “status unknown” even when the allow list is empty.

Useful? React with 👍 / 👎.

Comment on lines 30 to 34
enable_safe_list_is_false = if input.EnableSafeList == null {
null # Field missing → null
} else {
!input.EnableSafeList # Explicit value → invert: false means compliant
}

Choose a reason for hiding this comment

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

P2 Badge Read safe-list flag from collector’s snake_case key

The hosted connection filter collector exposes enable_safe_list (engine/collectors/exchange/protection/hosted_connection_filter.py:50-55), not EnableSafeList. As written, enable_safe_list_is_false evaluates against an undefined field and the control will always return “Unable to determine” even when the safe list is disabled.

Useful? React with 👍 / 👎.

Comment on lines 30 to 36
allowed_sender_domains_undefined := true if {
input.AllowedSenderDomains == null
}

allowed_sender_domains_undefined := true if {
input.AllowedSenderDomains != null
count(input.AllowedSenderDomains) == 0

Choose a reason for hiding this comment

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

P2 Badge Use allowed_sender_domains from collector output

The hosted content filter collector returns allowed_sender_domains (engine/collectors/exchange/protection/hosted_content_filter.py:50-55), but this policy reads input.AllowedSenderDomains. That mismatch makes the allowed-domain checks evaluate against undefined data and the control can’t correctly report compliance.

Useful? React with 👍 / 👎.

Comment on lines 36 to 41
limits_compliant if {
input != null
all k in required_policy_fields {
input[k] != null
input[k] == required_policy_fields[k]
}

Choose a reason for hiding this comment

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

P2 Badge Read message limits from the collected policy object

The outbound spam collector only exposes default_policy and a few snake_case fields (engine/collectors/exchange/protection/hosted_outbound_spam_filter.py:50-56), but this policy expects input.RecipientLimit* at the top level. Those keys are never set, so limits_compliant never becomes true and the control always falls back to the default failure state.

Useful? React with 👍 / 👎.

Comment on lines 30 to 35
zero_hour_auto_purge_enabled := true if {
input.ZeroHourAutoPurgeEnabled == true
}

zero_hour_auto_purge_enabled := false if {
input.ZeroHourAutoPurgeEnabled != true

Choose a reason for hiding this comment

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

P2 Badge Use Teams ZAP flag exposed by collector

The Teams protection collector returns zap_enabled (engine/collectors/exchange/protection/teams_protection_policy.py:35-38), but this policy checks input.ZeroHourAutoPurgeEnabled, which is never populated. That means the rule always treats the setting as unknown even when ZAP for Teams is enabled.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 19, 2026

In progress: correcting the automated review suggestions on my PR

@romil-bijarnia
Copy link
Collaborator

In progress: correcting the automated review suggestions on my PR

Would you please be able to add some screenshots to this pull request where testing for false positives/negatives has been done after logging in to the test environment

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 24, 2026

added negative testing for section 2 policies. (2.1.2, 2.1.5, 2.1.13, 2.1.12, 2.4.4)
Screenshot 2026-01-25 at 12 26 11 AM

actual control sets from Microsoft Defender https://security.microsoft.com

Screenshot 2026-01-25 at 12 28 53 AM Screenshot 2026-01-25 at 12 29 38 AM

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 24, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b801ceb1bd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 64 to 68
"Found %d targeted user(s) protected by compliant anti-phishing policy",
[count(targeted_users)]
)
} if {
count(targeted_users) > 0

Choose a reason for hiding this comment

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

P2 Badge Do not require targeted users for anti-phishing policy

Compliance is only reported when count(targeted_users) > 0, but the default anti-phishing policy commonly applies to all users without explicitly listing TargetedUsersToProtect. In that common case (no targeted users but compliant settings), neither result rule fires, so the control yields no compliant result even though a valid policy exists.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 24, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c43ecb59e5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +39 to +43
scan_result = {
"compliant": true,
"message": "Outbound spam BCC and notification settings are enabled",
"affected_resources": [],
"details": {

Choose a reason for hiding this comment

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

P1 Badge Define result rule for OPA evaluation

OPAClient.evaluate_policy posts to the /result rule (see OPAClient.evaluate_policy in engine/opa_client.py), but this policy only defines scan_result. That means OPA returns an empty object for this control, so it never reports compliance/non-compliance in any environment. Renaming or aliasing scan_result to result is required for the control to emit results.

Useful? React with 👍 / 👎.

Comment on lines 22 to 24
missing_bcc if {
not input.default_policy.bcc_suspicious_outbound_mail
}

Choose a reason for hiding this comment

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

P1 Badge Read outbound spam flags from collected fields

The HostedOutboundSpamFilterDataCollector returns bcc_suspicious_outbound_mail and notify_outbound_spam at the top level (and default_policy uses PowerShell’s PascalCase keys). Accessing input.default_policy.bcc_suspicious_outbound_mail here will always be undefined, so missing_settings is always populated and the control flags non-compliant even when the settings are enabled. Use the top-level fields or the correct key casing on default_policy to avoid false failures.

Useful? React with 👍 / 👎.

Comment on lines 24 to 28
compliant if {
input.data.enable == true
input.data.action == "Block"
input.data.quarantine_tag == "AdminOnlyAccessPolicy"
input.data.identity == "Built-In Protection Policy"

Choose a reason for hiding this comment

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

P1 Badge Use collector output instead of input.data for 2.1.4

The exchange.protection.safe_attachment_policy collector returns safe_attachment_policies, total_policies, and policies_with_protection at the top level (no data field). Referencing input.data.* makes all checks and message rules undefined, so the result rule never produces output and OPA returns {}. This control won’t report compliance in any environment unless the policy reads from the collector’s actual output shape.

Useful? React with 👍 / 👎.

Comment on lines 63 to 66
"RecipientLimitExternalPerHour": input.RecipientLimitExternalPerHour,
"RecipientLimitInternalPerHour": input.RecipientLimitInternalPerHour,
"RecipientLimitPerDay": input.RecipientLimitPerDay,
"ActionWhenThresholdReached": input.ActionWhenThresholdReached,

Choose a reason for hiding this comment

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

P1 Badge Pull message limit fields from default policy data

The hosted outbound spam filter collector does not expose RecipientLimit* or ActionWhenThresholdReached at the top level—those live inside default_policy (if present). As written, these top-level fields are undefined, so limits_compliant never evaluates and the result rule fails, yielding no output. The control needs to read from input.default_policy.<Field> or extend the collector to surface these fields.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 25, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad7237a3f6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 52 to 54
some f
required_fields[f] # f is a key in required_fields
field_non_compliant(p, f)

Choose a reason for hiding this comment

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

P2 Badge Validate false-required Safe Links fields

The comprehension that builds non_compliant_fields only iterates keys where required_fields[f] is truthy. Because AllowClickThrough and DisableUrlRewrite are expected to be false, those keys are skipped entirely, so a policy with AllowClickThrough=true or DisableUrlRewrite=true will still be reported compliant. This yields false negatives whenever those settings are misconfigured; consider iterating keys without relying on the truthiness of the required value (e.g., check key existence via object.get).

Useful? React with 👍 / 👎.

Comment on lines 43 to 46
invalid_fields := {f |
some f
required_values[f]
field_non_compliant(p, f)

Choose a reason for hiding this comment

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

P2 Badge Ensure false-required Safe Docs setting is checked

The invalid_fields comprehension uses required_values[f] as a truthiness guard, which drops any required value that is false. As a result, the AllowSafeDocsOpen=false requirement is never validated, and policies with AllowSafeDocsOpen=true will still be considered compliant. This causes false negatives for the exact misconfiguration the control is meant to catch.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 25, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fde013e9fb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 68 to 71
any_policy_compliant if {
some i
is_compliant[input.malware_filter_policies[i].Identity]
}

Choose a reason for hiding this comment

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

P2 Badge Require all malware filter policies to be compliant

The rule sets compliance based on any_policy_compliant, so a single compliant malware filter policy makes the whole control pass even if other policies in the tenant have EnableFileFilter disabled or omit many extensions. In environments with scoped policies, this produces false positives because users covered by noncompliant policies can still receive risky attachments while the control reports compliant. Consider requiring every policy (or at least the default policy plus any enabled scoped policies) to meet the extension list, or report noncompliant policies in the result.

Useful? React with 👍 / 👎.

Comment on lines 29 to 31
ip_allow_list := object.get(input, "ip_allow_list", [])
ip_allow_list_status := "empty" if count(ip_allow_list) == 0
ip_allow_list_status := "not_empty" if count(ip_allow_list) > 0

Choose a reason for hiding this comment

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

P2 Badge Check all connection filter policies for IP allow lists

This rule only inspects input.ip_allow_list (derived from the default policy) and never evaluates the per-policy data in connection_filter_policies. If a non-default hosted connection filter policy has an IP allow list configured, the check still reports compliant because the default list is empty, leaving scoped users unprotected. To avoid false negatives, iterate over input.connection_filter_policies and fail when any policy has a non-empty IPAllowList.

Useful? React with 👍 / 👎.

@tinar10
Copy link
Collaborator Author

tinar10 commented Jan 25, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e4a67e30f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +31 to +35
enable_safe_list := object.get(input, "enable_safe_list", false)

result := {
"compliant": true,
"message": "EnableSafeList is False for Exchange Online Hosted Connection Filter",

Choose a reason for hiding this comment

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

P2 Badge Evaluate all connection filter policies, not just default

This rule only evaluates input.enable_safe_list, which is derived from the default Hosted Connection Filter policy. If an organization has multiple connection filter policies and any non-default policy has EnableSafeList set to true, this check will still report compliant even though some recipients are protected by a non‑compliant policy. That creates false negatives for tenants using scoped policies.

Useful? React with 👍 / 👎.

Comment on lines +32 to +35
allowed_sender_domains := object.get(input, "allowed_sender_domains", [])

allowed_sender_domains_undefined := true if count(allowed_sender_domains) == 0
allowed_sender_domains_undefined := false if count(allowed_sender_domains) > 0

Choose a reason for hiding this comment

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

P2 Badge Scan all inbound anti-spam policies for allowed domains

The control claims to ensure inbound anti‑spam policies do not contain allowed domains, but the implementation only checks allowed_sender_domains (the default policy’s list). If a non‑default Hosted Content Filter policy has AllowedSenderDomains populated, this will still return compliant, so tenants with scoped policies can be misreported.

Useful? React with 👍 / 👎.

Copy link
Collaborator

@romil-bijarnia romil-bijarnia left a comment

Choose a reason for hiding this comment

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

Thanks for adding those screenshots, the changes look good

@romil-bijarnia romil-bijarnia merged commit 01cad43 into main Jan 26, 2026
16 of 17 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