feat(config): add compliance guardrails for the SDK config#11667
Closed
pedrooot wants to merge 9 commits into
Closed
feat(config): add compliance guardrails for the SDK config#11667pedrooot wants to merge 9 commits into
pedrooot wants to merge 9 commits into
Conversation
github-actions
Bot
added
documentation
compliance
Contributor
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a compliance config constraint evaluation engine ( ChangesCompliance Config Constraint Evaluation
Sequence Diagram(s)sequenceDiagram
participant transform as transform / get_table
participant build_requirement_config_status
participant get_scan_audit_config
participant Provider
participant evaluate_config_constraints
participant apply_config_status
transform->>build_requirement_config_status: compliance.Requirements
build_requirement_config_status->>get_scan_audit_config: (lazy call)
get_scan_audit_config->>Provider: get_global_provider().audit_config
Provider-->>get_scan_audit_config: audit_config dict or {}
get_scan_audit_config-->>build_requirement_config_status: audit_config
loop per requirement with ConfigRequirements
build_requirement_config_status->>evaluate_config_constraints: constraints list, audit_config
evaluate_config_constraints-->>build_requirement_config_status: (is_compliant, reason)
end
build_requirement_config_status-->>transform: {req_id → (is_compliant, reason)}
loop per finding × requirement match
transform->>apply_config_status: finding.status, finding.status_extended, config_status_tuple
apply_config_status-->>transform: (row_status, row_status_extended)
end
transform-->>transform: emit output rows with derived Status/StatusExtended
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ All necessary |
Contributor
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
prowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.py (1)
46-71: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid recording one finding as both PASS and FAIL.
Line 68 can add the same
indextofail_counteven if Line 72 already added it topass_countfor an earlier requirement. Compute a single finding-level effective status across the matched requirements before updatingsections,pass_count, andfail_count.Suggested direction
- for requirement in compliance.Requirements: + requirement_statuses = [] + finding_effective_status = finding.status + for requirement in compliance.Requirements: # A requirement whose configurable checks ran with an invalid # config can't be trusted: treat the finding as FAIL. config_status = resolve_requirement_config_status( requirement, audit_config, config_status_cache ) - effective_status = get_effective_status( + requirement_effective_status = get_effective_status( finding.status, config_status ) + requirement_statuses.append(requirement) + if requirement_effective_status == "FAIL": + finding_effective_status = "FAIL" + + for requirement in requirement_statuses: for attribute in requirement.Attributes: @@ - if effective_status == "FAIL" and index not in fail_count: + if finding_effective_status == "FAIL" and index not in fail_count: fail_count.append(index) sections[section]["Status"]["FAIL"] += 1 - elif effective_status == "PASS" and index not in pass_count: + elif finding_effective_status == "PASS" and index not in pass_count:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.py` around lines 46 - 71, The issue is that the same finding index can be recorded in both pass_count and fail_count if it matches multiple requirements with different statuses. Instead of checking and updating pass_count and fail_count inside the loop that iterates over requirement.Attributes, first collect all effective statuses for a single finding across all matched requirements, determine the overall finding-level status, and then perform a single update to sections, pass_count, and fail_count based on that consolidated status. This prevents the same index from being added to multiple status counters.prowler/lib/outputs/compliance/mitre_attack/mitre_attack.py (1)
45-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake PASS/FAIL aggregation mutually exclusive per finding.
effective_statusis scoped to each requirement, butpass_countandfail_countare scoped to the whole table. If a later requirement resolves to FAIL after an earlier PASS, the same finding is counted in both lists. Precompute the finding’s final effective status before updating tactic counters.Suggested direction
- for requirement in compliance.Requirements: + requirement_statuses = [] + finding_effective_status = finding.status + for requirement in compliance.Requirements: # A requirement whose configurable checks ran with an invalid # config can't be trusted: treat the finding as FAIL. config_status = resolve_requirement_config_status( requirement, audit_config, config_status_cache ) - effective_status = get_effective_status( + requirement_effective_status = get_effective_status( finding.status, config_status ) + requirement_statuses.append(requirement) + if requirement_effective_status == "FAIL": + finding_effective_status = "FAIL" + + for requirement in requirement_statuses: for tactic in requirement.Tactics: @@ - if effective_status == "FAIL": + if finding_effective_status == "FAIL": if index not in fail_count: fail_count.append(index) tactics[tactic]["FAIL"] += 1 - elif effective_status == "PASS": + elif finding_effective_status == "PASS":🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/mitre_attack/mitre_attack.py` around lines 45 - 66, The current code computes effective_status per requirement, which causes findings to be counted in multiple status categories if different requirements produce different results. Restructure the logic by precomputing a single final effective_status for each finding across all its requirements before updating the tactic counters. This ensures that each finding is only counted once in either pass_count, fail_count, or muted_count based on its overall final status, not repeatedly as each requirement is evaluated. The index should be checked against the appropriate count list only once using the precomputed final status.prowler/lib/outputs/compliance/ccc/ccc.py (1)
42-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve the finding’s effective status before updating counters.
Line 59 can append an index to
fail_countafter an earlier requirement already appended the same index topass_count, becauseeffective_statusis recomputed per requirement but the count lists are scan-wide. Derive the worst effective status for the finding first, then mutatepass_count/fail_countonce.Suggested direction
- for requirement in compliance.Requirements: + requirement_statuses = [] + finding_effective_status = finding.status + for requirement in compliance.Requirements: # A requirement whose configurable checks ran with an invalid # config can't be trusted: treat the finding as FAIL. config_status = resolve_requirement_config_status( requirement, audit_config, config_status_cache ) - effective_status = get_effective_status( + requirement_effective_status = get_effective_status( finding.status, config_status ) + requirement_statuses.append(requirement) + if requirement_effective_status == "FAIL": + finding_effective_status = "FAIL" + + for requirement in requirement_statuses: for attribute in requirement.Attributes: section = attribute.Section @@ - if effective_status == "FAIL" and index not in fail_count: + if finding_effective_status == "FAIL" and index not in fail_count: fail_count.append(index) sections[section]["FAIL"] += 1 - elif effective_status == "PASS" and index not in pass_count: + elif finding_effective_status == "PASS" and index not in pass_count:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/ccc/ccc.py` around lines 42 - 62, The effective_status is being recalculated for each requirement attribute, but the fail_count and pass_count lists are scan-wide, allowing the same index to be added to both lists if different requirements produce different effective statuses for the same finding. Move the calculation of effective_status using get_effective_status outside of the loop that iterates through requirement.Attributes so that a single, final effective status is determined per finding before processing any requirements. Then use this pre-calculated effective_status value inside the loop when updating the fail_count, pass_count, and muted_count lists to ensure each index is counted only once with the correct status.prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py (1)
51-108: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDerive the final effective status before scoring.
A finding can add PASS score at Line 75, then be counted as FAIL at Line 92 if a later requirement resolves invalid config. That makes one finding both improve ThreatScore and increase FAIL totals. Resolve the worst effective status across requirements first, then use that single value for score and counter updates.
Suggested direction
- for requirement in compliance.Requirements: + requirement_statuses = [] + finding_effective_status = finding.status + for requirement in compliance.Requirements: # A requirement whose configurable checks ran with an invalid # config can't be trusted: treat the finding as FAIL (it stops # contributing to the pillar/generic score and counts as FAIL). config_status = resolve_requirement_config_status( requirement, audit_config, config_status_cache ) - effective_status = get_effective_status( + requirement_effective_status = get_effective_status( finding.status, config_status ) + requirement_statuses.append(requirement) + if requirement_effective_status == "FAIL": + finding_effective_status = "FAIL" + + for requirement in requirement_statuses: for attribute in requirement.Attributes: @@ - if effective_status == "PASS": + if finding_effective_status == "PASS": @@ - if effective_status == "FAIL" and index not in fail_count: + if finding_effective_status == "FAIL" and index not in fail_count: fail_count.append(index) pillars[pillar]["FAIL"] += 1 - elif effective_status == "PASS" and index not in pass_count: + elif finding_effective_status == "PASS" and index not in pass_count: @@ - if effective_status == "PASS": + if finding_effective_status == "PASS":🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py` around lines 51 - 108, The current logic calculates effective_status separately for each requirement in the inner loop, which allows a single finding (identified by index) to contribute both PASS score at the condition on line 75 and FAIL counts at the condition on line 92 if different requirements resolve to different config statuses. Before processing the requirement attributes, determine the final effective status by evaluating the worst status across all requirements for the current finding, then use this single effective status value consistently throughout the scoring operations (the conditions checking effective_status == "PASS" and effective_status == "FAIL" in the loops over requirement.Attributes and the generic score calculation). This ensures each finding contributes to scores and counters based on a single resolved status rather than potentially multiple conflicting statuses.prowler/lib/outputs/compliance/universal/universal_table.py (1)
185-203: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAggregate with requirement-aware fail precedence.
effective_statusis now requirement-specific, but the pass/fail and scoring dedupe still keys only by findingindex. If one finding maps to multiple requirements, a PASS requirement can add the index topass_countor score buckets, then a later config-invalid requirement can add the same index tofail_count, producing double-counted overview percentages and inflated scored output. Accumulate statuses per finding/group first, or make FAIL remove/supersede any prior PASS before mutating counts and scores.Also applies to: 290-318, 413-449
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/universal/universal_table.py` around lines 185 - 203, The deduplication logic using index-based tracking in fail_count, pass_count, and muted_count does not account for requirement-specific effective_status values. When a single finding maps to multiple requirements, the same index can be added to both pass_count and fail_count buckets due to different effective_status values per requirement, causing double-counting in group aggregations. Refactor the logic to either accumulate all effective_status values per finding/group combination first before updating counts (ensuring FAIL takes precedence over PASS), or implement a requirement-aware deduplication key that combines finding index with requirement context. This pattern appears in the loops processing requirements across _get_group_key iterations, so ensure the fix applies consistently wherever effective_status is evaluated against the count tracking lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/developer-guide/configurable-checks.mdx`:
- Line 49: The section header "## Adding a Parameter to the Provider Schema" on
line 49 is missing the required Version Badge for documenting new functionality.
Add a Version Badge directly after this header in semantic version format
(without the `v` prefix) on its own line, followed by a blank line before the
next content. This follows the documentation guidelines for MDX files when
introducing new functionality tied to a specific release.
- Line 72: The type mapping documentation on line 72 has an inconsistency: the
description states "List of strings or ints" but the type annotation shown is
`Optional[list[str]] = None`, which only includes strings and excludes ints. To
fix this, either update the description to read "List of strings" to match the
type annotation, or update the type declaration to include both strings and ints
(using Union or the pipe operator syntax) to match the description. Choose
whichever option is correct for your API.
In `@prowler/CHANGELOG.md`:
- Line 131: The 5.27.0 changelog entry at line 131 is incomplete as it only
documents the bounds validation feature but omits the compliance requirement
config constraints feature that was also shipped in this release. Add a
companion changelog bullet point after the existing entry that documents the
compliance requirement config constraints capability, ensuring it describes how
this feature can override effective requirement status to FAIL when audit_config
is invalid, so the release notes accurately reflect all delivered behaviors in
this cohort.
In `@prowler/config/schema/aws.py`:
- Line 130: The AWSProviderConfig class is missing a docstring which is required
by the project's coding guidelines. Add a Google-style class docstring to
AWSProviderConfig that describes its purpose and functionality. The docstring
should be placed immediately after the class definition line and provide clear
documentation about what this configuration class does and how it's used.
In `@prowler/config/schema/azure.py`:
- Line 20: The AzureProviderConfig class is missing a docstring which violates
the Python documentation requirements. Add a Google-style docstring to the
AzureProviderConfig class definition that describes the purpose and
functionality of this configuration class. The docstring should follow Google
style format with a brief summary of what the class does.
In `@prowler/config/schema/gcp.py`:
- Line 10: The class GCPProviderConfig is missing a docstring, which violates
the coding guidelines requiring all classes to have documentation. Add a
Google-style docstring immediately after the class definition line for
GCPProviderConfig that describes the purpose and responsibility of this class.
The docstring should be concise and explain what configuration this class
manages for GCP providers.
In `@prowler/config/schema/github.py`:
- Line 10: Add a class docstring to the GitHubProviderConfig class definition
that follows Google style documentation format. The docstring should be placed
immediately after the class declaration line (after the colon) and should
describe the purpose and functionality of the GitHubProviderConfig class. The
docstring must be enclosed in triple quotes and provide a clear explanation of
what this configuration class does, following the prowler project's Python
coding standards for mandatory class docstrings.
In `@prowler/config/schema/kubernetes.py`:
- Line 10: The `KubernetesProviderConfig` class is missing a docstring required
by the prowler documentation guidelines. Add a class docstring immediately after
the class declaration that follows Google style documentation format. The
docstring should briefly describe the purpose of the class and what
configuration it manages for Kubernetes providers. Ensure the docstring uses
triple quotes and provides clear, concise documentation of the class's
functionality.
In `@prowler/config/schema/m365.py`:
- Line 10: The M365ProviderConfig class is missing a docstring which violates
the repository's Python documentation guidelines. Add a Google-style docstring
to the M365ProviderConfig class that describes its purpose and functionality.
The docstring should be placed immediately after the class definition line and
follow Google's documentation format to ensure consistency with the rest of the
codebase.
In `@prowler/config/schema/validator.py`:
- Around line 45-65: The code logs sensitive information that could expose
secrets from scan configs. In the first logger.warning call around the
raw.get(key) reference, remove the raw value from the log message and only log
the key name and error reason. In the second logger.error call that references
exc2.errors(), replace the full error payload logging with just a simple count
or generic message indicating validation failed, without exposing the detailed
Pydantic error structure which may contain sensitive data. Keep only the
key/path information and validation failure reasons in both log messages.
In `@prowler/lib/check/compliance_config_eval.py`:
- Around line 50-51: In the compliance_config_eval.py file, the constraint
evaluation logic currently returns True when an unknown operator is encountered,
which incorrectly treats malformed constraints as satisfied. Change the return
statement from True to False in the unknown operator handling block (around line
51) so that invalid constraints fail-closed and are treated as not satisfied,
preventing incorrect PASS outcomes in compliance reports.
In `@prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py`:
- Around line 42-47: The issue is that findings can appear in both PASS and FAIL
lists when different requirements compute different effective statuses for the
same finding. After computing effective_status for each requirement using
resolve_requirement_config_status() and get_effective_status(), the code needs
to consolidate the finding's overall status to enforce FAIL-over-PASS precedence
rather than tracking it separately per requirement. Modify the logic in the
section around lines 61-64 where findings are added to separate PASS and FAIL
lists to first determine the overall effective status for each finding across
all requirements (where FAIL takes precedence over PASS), and then add each
finding to only one appropriate list based on that consolidated status, not to
both lists.
In `@prowler/lib/outputs/compliance/c5/c5.py`:
- Around line 42-47: The issue is that the same finding index can be added to
both pass_count and fail_count because the conditions at lines 59-63 are not
mutually exclusive. After computing the effective_status per requirement in the
resolve_requirement_config_status and get_effective_status functions, ensure
that when the index is added to pass_count or fail_count based on the
effective_status value, the logic uses mutually exclusive conditions (such as
if/elif instead of separate if statements) to prevent the same finding from
being counted in both pass and fail totals simultaneously.
In `@prowler/lib/outputs/compliance/cis/cis.py`:
- Around line 44-49: The issue is that the same finding can be counted in both
pass_count and fail_count buckets across different requirements, since
effective_status is calculated per requirement and then used to increment the
count buckets. To fix this, ensure each finding index is only added to either
the pass or fail bucket once, regardless of how many requirements are processed.
Track which findings have already been counted (using a set to store finding
indices that have been added to pass_count or fail_count) and check this
tracking set before incrementing pass_count or fail_count at lines 65-68. This
will make the PASS/FAIL buckets mutually exclusive per finding and ensure the
overview math and percentages remain accurate.
In `@prowler/lib/outputs/compliance/ens/ens.py`:
- Around line 34-51: The current implementation allows the same finding
(identified by index) to be counted in both fail_count and pass_count when it
receives different effective_status values across multiple requirements. To fix
this, implement FAIL precedence logic in the counting section around lines 69-78
where effective_status is evaluated. Track which findings have already been
counted as FAIL (using a set or similar data structure keyed by finding index),
and skip incrementing pass_count for any finding that has already been counted
as FAIL in a previous requirement. This ensures each finding contributes to only
one count category, with FAIL taking precedence over PASS.
In
`@tests/lib/outputs/compliance/universal/universal_table_config_requirements_test.py`:
- Around line 60-73: The _render function uses a hardcoded Unix-specific path
"/tmp" for the output_directory parameter in the get_universal_table call, which
will fail on non-Unix systems. Modify the _render function to accept a tmp_path
parameter from pytest's tmp_path fixture, and replace the hardcoded "/tmp"
string with str(tmp_path) when calling get_universal_table. Apply the same
change to any other test functions that use this pattern (noted as applying to
lines 77-90 as well).
---
Outside diff comments:
In `@prowler/lib/outputs/compliance/ccc/ccc.py`:
- Around line 42-62: The effective_status is being recalculated for each
requirement attribute, but the fail_count and pass_count lists are scan-wide,
allowing the same index to be added to both lists if different requirements
produce different effective statuses for the same finding. Move the calculation
of effective_status using get_effective_status outside of the loop that iterates
through requirement.Attributes so that a single, final effective status is
determined per finding before processing any requirements. Then use this
pre-calculated effective_status value inside the loop when updating the
fail_count, pass_count, and muted_count lists to ensure each index is counted
only once with the correct status.
In `@prowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.py`:
- Around line 46-71: The issue is that the same finding index can be recorded in
both pass_count and fail_count if it matches multiple requirements with
different statuses. Instead of checking and updating pass_count and fail_count
inside the loop that iterates over requirement.Attributes, first collect all
effective statuses for a single finding across all matched requirements,
determine the overall finding-level status, and then perform a single update to
sections, pass_count, and fail_count based on that consolidated status. This
prevents the same index from being added to multiple status counters.
In `@prowler/lib/outputs/compliance/mitre_attack/mitre_attack.py`:
- Around line 45-66: The current code computes effective_status per requirement,
which causes findings to be counted in multiple status categories if different
requirements produce different results. Restructure the logic by precomputing a
single final effective_status for each finding across all its requirements
before updating the tactic counters. This ensures that each finding is only
counted once in either pass_count, fail_count, or muted_count based on its
overall final status, not repeatedly as each requirement is evaluated. The index
should be checked against the appropriate count list only once using the
precomputed final status.
In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py`:
- Around line 51-108: The current logic calculates effective_status separately
for each requirement in the inner loop, which allows a single finding
(identified by index) to contribute both PASS score at the condition on line 75
and FAIL counts at the condition on line 92 if different requirements resolve to
different config statuses. Before processing the requirement attributes,
determine the final effective status by evaluating the worst status across all
requirements for the current finding, then use this single effective status
value consistently throughout the scoring operations (the conditions checking
effective_status == "PASS" and effective_status == "FAIL" in the loops over
requirement.Attributes and the generic score calculation). This ensures each
finding contributes to scores and counters based on a single resolved status
rather than potentially multiple conflicting statuses.
In `@prowler/lib/outputs/compliance/universal/universal_table.py`:
- Around line 185-203: The deduplication logic using index-based tracking in
fail_count, pass_count, and muted_count does not account for
requirement-specific effective_status values. When a single finding maps to
multiple requirements, the same index can be added to both pass_count and
fail_count buckets due to different effective_status values per requirement,
causing double-counting in group aggregations. Refactor the logic to either
accumulate all effective_status values per finding/group combination first
before updating counts (ensuring FAIL takes precedence over PASS), or implement
a requirement-aware deduplication key that combines finding index with
requirement context. This pattern appears in the loops processing requirements
across _get_group_key iterations, so ensure the fix applies consistently
wherever effective_status is evaluated against the count tracking lists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9f33f6f-b43e-4354-8fc0-78fac6ba615f
⛔ Files ignored due to path filters (61)
prowler/compliance/alibabacloud/cis_2.0_alibabacloud.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/alibabacloud/prowler_threatscore_alibabacloud.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/asd_essential_eight_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/aws_account_security_onboarding_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/aws_ai_security_framework_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/aws_foundational_security_best_practices_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/aws_foundational_technical_review_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/aws_well_architected_framework_security_pillar_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/c5_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ccc_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_1.4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_1.5_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_2.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_3.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_4.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_5.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cis_6.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/cisa_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ens_rd2022_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/fedramp_20x_ksi_low_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/fedramp_low_revision_4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/fedramp_moderate_revision_4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ffiec_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/gdpr_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/gxp_21_cfr_part_11_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/gxp_eu_annex_11_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/hipaa_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/iso27001_2013_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/iso27001_2022_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_korean_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/mitre_attack_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nis2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_171_revision_2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_53_revision_4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_53_revision_5_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_csf_1.1_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_csf_2.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/pci_3.2.1_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/pci_4.0_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/prowler_threatscore_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/rbi_cyber_security_framework_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/secnumcloud_3.2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/soc2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/c5_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/ccc_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/cis_4.0_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/cis_5.0_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/hipaa_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/nis2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/secnumcloud_3.2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/soc2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/csa_ccm_4.0.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/dora.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/gcp/ccc_gcp.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/cis_1.10_kubernetes.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/cis_1.11_kubernetes.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/cis_1.12_kubernetes.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/cis_1.8_kubernetes.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/pci_4.0_kubernetes.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/kubernetes/prowler_threatscore_kubernetes.jsonis excluded by!prowler/compliance/**/*.json
📒 Files selected for processing (83)
docs/developer-guide/configurable-checks.mdxprowler/CHANGELOG.mdprowler/config/config.pyprowler/config/scan_config_schema.pyprowler/config/schema/__init__.pyprowler/config/schema/aws.pyprowler/config/schema/azure.pyprowler/config/schema/base.pyprowler/config/schema/cloudflare.pyprowler/config/schema/gcp.pyprowler/config/schema/github.pyprowler/config/schema/kubernetes.pyprowler/config/schema/m365.pyprowler/config/schema/mongodbatlas.pyprowler/config/schema/registry.pyprowler/config/schema/validator.pyprowler/config/schema/validators.pyprowler/config/schema/vercel.pyprowler/lib/check/compliance_config_eval.pyprowler/lib/check/compliance_models.pyprowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.pyprowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight_aws.pyprowler/lib/outputs/compliance/aws_well_architected/aws_well_architected.pyprowler/lib/outputs/compliance/c5/c5.pyprowler/lib/outputs/compliance/c5/c5_aws.pyprowler/lib/outputs/compliance/c5/c5_azure.pyprowler/lib/outputs/compliance/c5/c5_gcp.pyprowler/lib/outputs/compliance/ccc/ccc.pyprowler/lib/outputs/compliance/ccc/ccc_aws.pyprowler/lib/outputs/compliance/ccc/ccc_azure.pyprowler/lib/outputs/compliance/ccc/ccc_gcp.pyprowler/lib/outputs/compliance/cis/cis.pyprowler/lib/outputs/compliance/cis/cis_alibabacloud.pyprowler/lib/outputs/compliance/cis/cis_aws.pyprowler/lib/outputs/compliance/cis/cis_azure.pyprowler/lib/outputs/compliance/cis/cis_gcp.pyprowler/lib/outputs/compliance/cis/cis_github.pyprowler/lib/outputs/compliance/cis/cis_googleworkspace.pyprowler/lib/outputs/compliance/cis/cis_kubernetes.pyprowler/lib/outputs/compliance/cis/cis_m365.pyprowler/lib/outputs/compliance/cis/cis_oraclecloud.pyprowler/lib/outputs/compliance/cisa_scuba/cisa_scuba_googleworkspace.pyprowler/lib/outputs/compliance/ens/ens.pyprowler/lib/outputs/compliance/ens/ens_aws.pyprowler/lib/outputs/compliance/ens/ens_azure.pyprowler/lib/outputs/compliance/ens/ens_gcp.pyprowler/lib/outputs/compliance/generic/generic.pyprowler/lib/outputs/compliance/generic/generic_table.pyprowler/lib/outputs/compliance/iso27001/iso27001_aws.pyprowler/lib/outputs/compliance/iso27001/iso27001_azure.pyprowler/lib/outputs/compliance/iso27001/iso27001_gcp.pyprowler/lib/outputs/compliance/iso27001/iso27001_kubernetes.pyprowler/lib/outputs/compliance/iso27001/iso27001_m365.pyprowler/lib/outputs/compliance/iso27001/iso27001_nhn.pyprowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.pyprowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp_aws.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack_aws.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack_azure.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack_gcp.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_alibaba.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_aws.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_azure.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_gcp.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_kubernetes.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore_m365.pyprowler/lib/outputs/compliance/universal/ocsf_compliance.pyprowler/lib/outputs/compliance/universal/universal_table.pytests/config/schema/__init__.pytests/config/schema/aws_schema_test.pytests/config/schema/bounds_test.pytests/config/schema/loader_integration_test.pytests/config/schema/other_providers_schema_test.pytests/config/schema/validator_test.pytests/lib/check/compliance_config_constraint_model_test.pytests/lib/check/compliance_config_eval_test.pytests/lib/check/compliance_config_requirements_data_test.pytests/lib/outputs/compliance/cis/cis_aws_config_requirements_test.pytests/lib/outputs/compliance/cis/cis_azure_config_requirements_test.pytests/lib/outputs/compliance/ens/ens_aws_config_requirements_test.pytests/lib/outputs/compliance/universal/ocsf_compliance_config_requirements_test.pytests/lib/outputs/compliance/universal/universal_table_config_requirements_test.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (16)
docs/developer-guide/configurable-checks.mdx (2)
49-49: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a Version Badge for This New Schema-Validation Functionality.
Line 49 introduces new functionality guidance but does not include the required Version Badge directly under the header. Please add the badge (semantic version format, no
vprefix) on its own line with a blank line after it.As per coding guidelines,
docs/**/*.mdxsections documenting new functionality must include a Version Badge immediately after the section header; based on learnings, this applies when the section is tied to a specific release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/developer-guide/configurable-checks.mdx` at line 49, The section header "## Adding a Parameter to the Provider Schema" on line 49 is missing the required Version Badge for documenting new functionality. Add a Version Badge directly after this header in semantic version format (without the `v` prefix) on its own line, followed by a blank line before the next content. This follows the documentation guidelines for MDX files when introducing new functionality tied to a specific release.Sources: Coding guidelines, Learnings
72-72: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the Type Mapping Example to Match Its Description.
Line 72 says “List of strings or ints” but the declaration shown is
Optional[list[str]] = None, which excludes ints. Please either change the description to only strings or update the declaration example to include ints.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/developer-guide/configurable-checks.mdx` at line 72, The type mapping documentation on line 72 has an inconsistency: the description states "List of strings or ints" but the type annotation shown is `Optional[list[str]] = None`, which only includes strings and excludes ints. To fix this, either update the description to read "List of strings" to match the type annotation, or update the type declaration to include both strings and ints (using Union or the pipe operator syntax) to match the description. Choose whichever option is correct for your API.prowler/CHANGELOG.md (1)
131-131: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Expand the 5.27.0 Entry to Include Compliance-Constraint Status Overrides.
This bullet captures schema bounds validation but omits the other shipped feature in this cohort: compliance requirement config constraints that can override effective requirement status to
FAILwhenaudit_configis invalid. Please add a companion changelog bullet so release notes reflect both delivered behaviors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/CHANGELOG.md` at line 131, The 5.27.0 changelog entry at line 131 is incomplete as it only documents the bounds validation feature but omits the compliance requirement config constraints feature that was also shipped in this release. Add a companion changelog bullet point after the existing entry that documents the compliance requirement config constraints capability, ensuring it describes how this feature can override effective requirement status to FAIL when audit_config is invalid, so the release notes accurately reflect all delivered behaviors in this cohort.prowler/config/schema/aws.py (1)
130-130: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a Class Docstring to
AWSProviderConfig.
AWSProviderConfigis missing a class docstring, which breaks the Python documentation requirement forprowler/**/*.py.Suggested Patch
class AWSProviderConfig(ProviderConfigBase): + """AWS provider configuration schema. + + Defines optional, validated scan settings consumed by AWS checks. + """ # --- IAM ---------------------------------------------------------------As per coding guidelines, "Docstrings are required for all classes and methods in Python code, following Google style documentation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/aws.py` at line 130, The AWSProviderConfig class is missing a docstring which is required by the project's coding guidelines. Add a Google-style class docstring to AWSProviderConfig that describes its purpose and functionality. The docstring should be placed immediately after the class definition line and provide clear documentation about what this configuration class does and how it's used.Source: Coding guidelines
prowler/config/schema/azure.py (1)
20-20: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a Class Docstring to
AzureProviderConfig.
AzureProviderConfigcurrently has no class docstring, so it does not meet the Python docstring requirement for this path.Suggested Patch
class AzureProviderConfig(ProviderConfigBase): + """Azure provider configuration schema. + + Defines optional, validated scan settings consumed by Azure checks. + """ # --- Network ---------------------------------------------------------As per coding guidelines, "Docstrings are required for all classes and methods in Python code, following Google style documentation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/azure.py` at line 20, The AzureProviderConfig class is missing a docstring which violates the Python documentation requirements. Add a Google-style docstring to the AzureProviderConfig class definition that describes the purpose and functionality of this configuration class. The docstring should follow Google style format with a brief summary of what the class does.Source: Coding guidelines
prowler/config/schema/gcp.py (1)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a Class Docstring for
GCPProviderConfig.
GCPProviderConfigis missing a class docstring, which violates theprowler/**/*.pydocstring requirement.As per coding guidelines: "Docstrings are required for all classes and methods in Python code, following Google style documentation."Proposed fix
class GCPProviderConfig(ProviderConfigBase): + """Configuration knobs for GCP checks. + + Attributes: + shodan_api_key: API key used for Shodan-based lookups. + mig_min_zones: Minimum number of zones required for MIG distribution. + max_snapshot_age_days: Maximum allowed snapshot age in days. + max_unused_account_days: Maximum inactivity window for accounts/keys. + storage_min_retention_days: Minimum Cloud Storage retention period. + """ shodan_api_key: Optional[str] = Field(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.class GCPProviderConfig(ProviderConfigBase): """Configuration knobs for GCP checks. Attributes: shodan_api_key: API key used for Shodan-based lookups. mig_min_zones: Minimum number of zones required for MIG distribution. max_snapshot_age_days: Maximum allowed snapshot age in days. max_unused_account_days: Maximum inactivity window for accounts/keys. storage_min_retention_days: Minimum Cloud Storage retention period. """ shodan_api_key: Optional[str] = Field(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/gcp.py` at line 10, The class GCPProviderConfig is missing a docstring, which violates the coding guidelines requiring all classes to have documentation. Add a Google-style docstring immediately after the class definition line for GCPProviderConfig that describes the purpose and responsibility of this class. The docstring should be concise and explain what configuration this class manages for GCP providers.Source: Coding guidelines
prowler/config/schema/github.py (1)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a Class Docstring for
GitHubProviderConfig.
GitHubProviderConfigis missing a class docstring required by the Python coding standards forprowler/**/*.py.As per coding guidelines: "Docstrings are required for all classes and methods in Python code, following Google style documentation."Proposed fix
class GitHubProviderConfig(ProviderConfigBase): + """Configuration knobs for GitHub checks. + + Attributes: + inactive_not_archived_days_threshold: Maximum inactivity period for + non-archived repositories before flagging. + """ inactive_not_archived_days_threshold: Optional[int] = Field(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/github.py` at line 10, Add a class docstring to the GitHubProviderConfig class definition that follows Google style documentation format. The docstring should be placed immediately after the class declaration line (after the colon) and should describe the purpose and functionality of the GitHubProviderConfig class. The docstring must be enclosed in triple quotes and provide a clear explanation of what this configuration class does, following the prowler project's Python coding standards for mandatory class docstrings.Source: Coding guidelines
prowler/config/schema/kubernetes.py (1)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a Class Docstring for
KubernetesProviderConfig.
KubernetesProviderConfigneeds a class docstring to comply with theprowler/**/*.pydocumentation rule.As per coding guidelines: "Docstrings are required for all classes and methods in Python code, following Google style documentation."Proposed fix
class KubernetesProviderConfig(ProviderConfigBase): + """Configuration knobs for Kubernetes checks. + + Attributes: + audit_log_maxbackup: Minimum retained rotated audit log files. + audit_log_maxsize: Maximum size per audit log file before rotation. + audit_log_maxage: Minimum retention duration for audit logs. + apiserver_strong_ciphers: Allowed strong TLS ciphers for API server. + kubelet_strong_ciphers: Allowed strong TLS ciphers for kubelet. + """ audit_log_maxbackup: Optional[int] = Field(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/kubernetes.py` at line 10, The `KubernetesProviderConfig` class is missing a docstring required by the prowler documentation guidelines. Add a class docstring immediately after the class declaration that follows Google style documentation format. The docstring should briefly describe the purpose of the class and what configuration it manages for Kubernetes providers. Ensure the docstring uses triple quotes and provides clear, concise documentation of the class's functionality.Source: Coding guidelines
prowler/config/schema/m365.py (1)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a class docstring for
M365ProviderConfig.This class is missing a docstring, which breaks the repo’s Python documentation rule.
Suggested fix
class M365ProviderConfig(ProviderConfigBase): + """M365 provider configuration schema. + + Defines optional configuration parameters used by M365 security checks, + including sign-in, Teams, Exchange, malware-policy, and mailbox-auditing bounds. + """ # --- Entra (sign-in policy) ----------------------------------------As per coding guidelines, "Docstrings are required for all classes and methods in Python code, following Google style documentation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/m365.py` at line 10, The M365ProviderConfig class is missing a docstring which violates the repository's Python documentation guidelines. Add a Google-style docstring to the M365ProviderConfig class that describes its purpose and functionality. The docstring should be placed immediately after the class definition line and follow Google's documentation format to ensure consistency with the rest of the codebase.Source: Coding guidelines
prowler/config/schema/validator.py (1)
45-65: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log raw invalid values or full Pydantic error payloads.
raw.get(key)!randexc2.errors()can expose secrets from scan configs (including plugin-defined keys) into logs. Log key/path and reason only, with value redaction.Suggested fix
for err in exc.errors(): loc = err.get("loc") or () @@ bad_keys.add(key) logger.warning( - f"prowler.config[{provider}.{key}] = {raw.get(key)!r} is invalid " - f"({err.get('msg', 'validation error')}); the value will be ignored " + f"prowler.config[{provider}.{key}] is invalid " + f"({err.get('msg', 'validation error')}); the value will be ignored " f"and the built-in default will be used." ) @@ except ValidationError as exc2: + compact_errors = [ + { + "loc": e.get("loc"), + "msg": e.get("msg", "validation error"), + "type": e.get("type"), + } + for e in exc2.errors() + ] logger.error( f"prowler.config[{provider}] could not be revalidated after dropping " f"invalid keys ({bad_keys}); passing through the cleaned dict as-is. " - f"Underlying errors: {exc2.errors()}" + f"Underlying errors: {compact_errors}" ) return cleaned🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/config/schema/validator.py` around lines 45 - 65, The code logs sensitive information that could expose secrets from scan configs. In the first logger.warning call around the raw.get(key) reference, remove the raw value from the log message and only log the key name and error reason. In the second logger.error call that references exc2.errors(), replace the full error payload logging with just a simple count or generic message indicating validation failed, without exposing the detailed Pydantic error structure which may contain sensitive data. Keep only the key/path information and validation failure reasons in both log messages.prowler/lib/check/compliance_config_eval.py (1)
50-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail Closed for Unknown Constraint Operators.
Returning
Truefor an unknown operator silently marks malformed constraints as satisfied, which can yield incorrect PASS outcomes in compliance rendering. This should returnFalseso invalid constraints are treated as not satisfied.Suggested Fix
- # Unknown operator: do not block the requirement on a malformed constraint. - return True + # Unknown operator: treat as not satisfied (fail-closed). + return False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Unknown operator: treat as not satisfied (fail-closed). return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/check/compliance_config_eval.py` around lines 50 - 51, In the compliance_config_eval.py file, the constraint evaluation logic currently returns True when an unknown operator is encountered, which incorrectly treats malformed constraints as satisfied. Change the return statement from True to False in the unknown operator handling block (around line 51) so that invalid constraints fail-closed and are treated as not satisfied, preventing incorrect PASS outcomes in compliance reports.prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py (1)
42-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce FAIL-Over-PASS Precedence for Per-Finding Summary Counts
Line 42-Line 47 computes
effective_statusper requirement, but Line 61-Line 64 tracks PASS/FAIL in separate lists. A finding can end up in both lists when requirements resolve differently, skewing the overview counts.💡 Suggested Fix
- pass_count = [] - fail_count = [] + pass_count: set[int] = set() + fail_count: set[int] = set() @@ - if effective_status == "FAIL" and index not in fail_count: - fail_count.append(index) + if effective_status == "FAIL" and index not in fail_count: + pass_count.discard(index) + fail_count.add(index) sections[section]["FAIL"] += 1 - elif effective_status == "PASS" and index not in pass_count: - pass_count.append(index) + elif ( + effective_status == "PASS" + and index not in pass_count + and index not in fail_count + ): + pass_count.add(index) sections[section]["PASS"] += 1Also applies to: 61-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py` around lines 42 - 47, The issue is that findings can appear in both PASS and FAIL lists when different requirements compute different effective statuses for the same finding. After computing effective_status for each requirement using resolve_requirement_config_status() and get_effective_status(), the code needs to consolidate the finding's overall status to enforce FAIL-over-PASS precedence rather than tracking it separately per requirement. Modify the logic in the section around lines 61-64 where findings are added to separate PASS and FAIL lists to first determine the overall effective status for each finding across all requirements (where FAIL takes precedence over PASS), and then add each finding to only one appropriate list based on that consolidated status, not to both lists.prowler/lib/outputs/compliance/c5/c5.py (1)
42-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent Double-Counting the Same Finding as Both PASS and FAIL
Because
effective_statusis computed per requirement (Line 42-Line 47), Line 59-Line 63 can add the sameindexto bothpass_countandfail_count. That produces inconsistent overview percentages and totals.💡 Suggested Fix
- pass_count = [] - fail_count = [] + pass_count: set[int] = set() + fail_count: set[int] = set() @@ - if effective_status == "FAIL" and index not in fail_count: - fail_count.append(index) + if effective_status == "FAIL" and index not in fail_count: + pass_count.discard(index) + fail_count.add(index) sections[section]["FAIL"] += 1 - elif effective_status == "PASS" and index not in pass_count: - pass_count.append(index) + elif ( + effective_status == "PASS" + and index not in pass_count + and index not in fail_count + ): + pass_count.add(index) sections[section]["PASS"] += 1Also applies to: 59-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/c5/c5.py` around lines 42 - 47, The issue is that the same finding index can be added to both pass_count and fail_count because the conditions at lines 59-63 are not mutually exclusive. After computing the effective_status per requirement in the resolve_requirement_config_status and get_effective_status functions, ensure that when the index is added to pass_count or fail_count based on the effective_status value, the logic uses mutually exclusive conditions (such as if/elif instead of separate if statements) to prevent the same finding from being counted in both pass and fail totals simultaneously.prowler/lib/outputs/compliance/cis/cis.py (1)
44-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make PASS/FAIL Buckets Mutually Exclusive Per Finding
effective_statusis requirement-specific (Line 44-Line 49), but Line 65-Line 68 allows one finding index to land in bothpass_countandfail_countwhen different requirements resolve differently. That breaks the overview math (total_findings_count) and can show inflated percentages.💡 Suggested Fix
- pass_count = [] - fail_count = [] + pass_count: set[int] = set() + fail_count: set[int] = set() @@ - if effective_status == "FAIL" and index not in fail_count: - fail_count.append(index) - elif effective_status == "PASS" and index not in pass_count: - pass_count.append(index) + if effective_status == "FAIL": + pass_count.discard(index) + fail_count.add(index) + elif effective_status == "PASS" and index not in fail_count: + pass_count.add(index)Also applies to: 65-68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/cis/cis.py` around lines 44 - 49, The issue is that the same finding can be counted in both pass_count and fail_count buckets across different requirements, since effective_status is calculated per requirement and then used to increment the count buckets. To fix this, ensure each finding index is only added to either the pass or fail bucket once, regardless of how many requirements are processed. Track which findings have already been counted (using a set to store finding indices that have been added to pass_count or fail_count) and check this tracking set before incrementing pass_count or fail_count at lines 65-68. This will make the PASS/FAIL buckets mutually exclusive per finding and ensure the overview math and percentages remain accurate.prowler/lib/outputs/compliance/ens/ens.py (1)
34-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent Double-Counting the Same Finding as Both PASS and FAIL
After Line 34 introduced requirement-level
effective_status, Line 69 and Line 78 can classify one finding differently across requirements. The sameindexcan end up in bothfail_countandpass_count, skewing ENS percentages and totals. Enforce FAIL precedence when counting.💡 Suggested fix
else: if effective_status == "FAIL": if ( attribute.Tipo != "recomendacion" and index not in fail_count ): fail_count.append(index) + if index in pass_count: + pass_count.remove(index) marcos[marco_categoria][ "Estado" ] = f"{Fore.RED}NO CUMPLE{Style.RESET_ALL}" - elif effective_status == "PASS" and index not in pass_count: + elif ( + effective_status == "PASS" + and index not in pass_count + and index not in fail_count + ): pass_count.append(index)Also applies to: 69-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/ens/ens.py` around lines 34 - 51, The current implementation allows the same finding (identified by index) to be counted in both fail_count and pass_count when it receives different effective_status values across multiple requirements. To fix this, implement FAIL precedence logic in the counting section around lines 69-78 where effective_status is evaluated. Track which findings have already been counted as FAIL (using a set or similar data structure keyed by finding index), and skip incrementing pass_count for any finding that has already been counted as FAIL in a previous requirement. This ensures each finding contributes to only one count category, with FAIL taking precedence over PASS.tests/lib/outputs/compliance/universal/universal_table_config_requirements_test.py (1)
60-73: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use a portable temp output directory instead of hardcoded
"/tmp"Line 68 hardcodes a Unix-specific path, which can make this test fail on non-Unix runners. Prefer pytest
tmp_pathand thread it through callers.Suggested patch
-def _render(audit_config, capsys): +def _render(audit_config, capsys, tmp_path): with patch(_MODULE) as mock_gp: mock_gp.return_value.audit_config = audit_config get_universal_table( findings=_FINDINGS, bulk_checks_metadata={}, compliance_framework_name="testfw_1.0_aws", output_filename="out", - output_directory="/tmp", + output_directory=str(tmp_path), compliance_overview=False, framework=_framework(), provider="aws", ) return capsys.readouterr().out @@ - def test_violating_config_counts_pass_finding_as_fail(self, capsys): - out = _render({"mute_non_default_regions": True}, capsys) + def test_violating_config_counts_pass_finding_as_fail(self, capsys, tmp_path): + out = _render({"mute_non_default_regions": True}, capsys, tmp_path) @@ - def test_valid_config_keeps_pass_count(self, capsys): - out = _render({"mute_non_default_regions": False}, capsys) + def test_valid_config_keeps_pass_count(self, capsys, tmp_path): + out = _render({"mute_non_default_regions": False}, capsys, tmp_path) @@ - def test_absent_config_keeps_pass_count(self, capsys): - out = _render({}, capsys) + def test_absent_config_keeps_pass_count(self, capsys, tmp_path): + out = _render({}, capsys, tmp_path)Also applies to: 77-90
🧰 Tools
🪛 ast-grep (0.44.0)
[info] 67-67: Do not hardcode temporary file or directory names
Context: "/tmp"
Note: [CWE-377] Insecure Temporary File.(hardcoded-tmp-file)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/lib/outputs/compliance/universal/universal_table_config_requirements_test.py` around lines 60 - 73, The _render function uses a hardcoded Unix-specific path "/tmp" for the output_directory parameter in the get_universal_table call, which will fail on non-Unix systems. Modify the _render function to accept a tmp_path parameter from pytest's tmp_path fixture, and replace the hardcoded "/tmp" string with str(tmp_path) when calling get_universal_table. Apply the same change to any other test functions that use this pattern (noted as applying to lines 77-90 as well).Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py (1)
74-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThreat score can be overstated due to first-hit status locking.
Line 74-Line 85 and Line 114-Line 122 lock score contribution on first encounter per finding/pillar and per finding/generic. With per-requirement
effective_status, a later FAIL for the same finding cannot revoke a prior PASS contribution, so pillar/generic scores and PASS counts can be overstated.This needs a two-phase or precedence-aware aggregation (record worst status per
(finding, pillar)and per finding first, then compute counters/scores once from that resolved status).Also applies to: 91-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py` around lines 74 - 85, The scoring logic in the section checking `if index not in counted_findings_per_pillar[pillar]` and the corresponding generic counting logic (lines 91-122) locks in the contribution on the first encounter of a finding. This causes later status updates for the same finding to be ignored, overstating scores and pass counts. Refactor to use a two-phase approach: first, resolve the final/worst effective_status for each (finding, pillar) pair by iterating through all findings and tracking the worst status per combination, then in a second phase, compute score_per_pillar, max_score_per_pillar, counted_findings_per_pillar and their generic equivalents based on those pre-resolved statuses rather than processing findings sequentially with first-hit locking.prowler/lib/outputs/compliance/ens/ens.py (1)
78-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake PASS/FAIL buckets mutually exclusive with FAIL precedence.
Line 78-Line 88 can place the same non-muted finding in both
fail_countandpass_countwheneffective_statusdiffers across requirements. This can skew percentages and framework totals.A minimal fix is to treat FAIL as dominant:
- when adding to FAIL, remove from PASS;
- only add PASS if the index is not already in FAIL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/ens/ens.py` around lines 78 - 88, The logic for populating fail_count and pass_count lists does not enforce mutual exclusivity, allowing the same index to be added to both lists across different requirement iterations. When effective_status equals "FAIL" and the condition for non-recomendacion is met, after adding the index to fail_count, also remove it from pass_count if it exists there to ensure FAIL takes precedence. Additionally, modify the PASS condition to only add the index to pass_count if it is not already present in fail_count, ensuring that once an index is marked as FAIL, it cannot be moved back to PASS. This ensures FAIL is the dominant status and prevents findings from being counted in both buckets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py`:
- Around line 61-81: The per-section counting logic in the section_seen block
prevents updating a finding's status once it has been recorded for a section.
When the same finding index appears with different effective_status values
across multiple requirements within the same section, only the first status is
counted while subsequent statuses are ignored. This hides FAIL statuses that
come after an initial PASS status. Modify the per-section counting to track the
status of each finding per section (instead of just tracking presence) and
update it to FAIL whenever a FAIL status is encountered for that section/finding
pair, ensuring FAIL precedence takes priority over any previously recorded PASS
status.
In `@prowler/lib/outputs/compliance/c5/c5.py`:
- Around line 58-78: The overview counters (muted_count, fail_count, pass_count)
and the per-section counters have divergent logic that allows a single finding
to be counted in multiple status buckets. Add a guard mechanism for the overview
counters similar to how section_seen is used for per-section counts: introduce a
tracking mechanism (such as a set) to ensure that once a finding identified by
index is assigned to one status bucket (PASS, FAIL, or Muted), it is not counted
in other status buckets. This will maintain one-finding-one-bucket semantics for
the overview totals while also aligning the logic between the overview counts
and the per-section counts blocks.
In `@prowler/lib/outputs/compliance/ccc/ccc.py`:
- Around line 58-78: The deduplication logic for overview totals (fail_count,
pass_count, muted_count lists) allows a single finding index to be added to
multiple status lists when effective_status varies across different requirements
for the same CheckID. Fix this by implementing a "FAIL wins" deduplication
strategy: replace the separate status lists with a dictionary that maps each
index to its final status, then apply precedence logic where FAIL takes priority
over PASS and muted takes priority over both. This ensures each finding is
counted exactly once in the overview totals with its worst status. Apply the
same fix pattern across all affected files (cis.py, kisa_ismsp.py,
mitre_attack.py, and universal_table.py) wherever similar index-based
deduplication exists.
---
Outside diff comments:
In `@prowler/lib/outputs/compliance/ens/ens.py`:
- Around line 78-88: The logic for populating fail_count and pass_count lists
does not enforce mutual exclusivity, allowing the same index to be added to both
lists across different requirement iterations. When effective_status equals
"FAIL" and the condition for non-recomendacion is met, after adding the index to
fail_count, also remove it from pass_count if it exists there to ensure FAIL
takes precedence. Additionally, modify the PASS condition to only add the index
to pass_count if it is not already present in fail_count, ensuring that once an
index is marked as FAIL, it cannot be moved back to PASS. This ensures FAIL is
the dominant status and prevents findings from being counted in both buckets.
In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py`:
- Around line 74-85: The scoring logic in the section checking `if index not in
counted_findings_per_pillar[pillar]` and the corresponding generic counting
logic (lines 91-122) locks in the contribution on the first encounter of a
finding. This causes later status updates for the same finding to be ignored,
overstating scores and pass counts. Refactor to use a two-phase approach: first,
resolve the final/worst effective_status for each (finding, pillar) pair by
iterating through all findings and tracking the worst status per combination,
then in a second phase, compute score_per_pillar, max_score_per_pillar,
counted_findings_per_pillar and their generic equivalents based on those
pre-resolved statuses rather than processing findings sequentially with
first-hit locking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a93caf80-74a7-4b12-bdda-7909e5f00bb6
⛔ Files ignored due to path filters (19)
prowler/compliance/aws/aws_well_architected_framework_security_pillar_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ccc_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ens_rd2022_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/fedramp_moderate_revision_4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ffiec_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/gxp_21_cfr_part_11_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/iso27001_2013_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_korean_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_171_revision_2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_53_revision_5_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/rbi_cyber_security_framework_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/secnumcloud_3.2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/cis_5.0_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/hipaa_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/secnumcloud_3.2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/soc2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/csa_ccm_4.0.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/dora_2022_2554.jsonis excluded by!prowler/compliance/**/*.json
📒 Files selected for processing (10)
prowler/lib/check/compliance_models.pyprowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.pyprowler/lib/outputs/compliance/c5/c5.pyprowler/lib/outputs/compliance/ccc/ccc.pyprowler/lib/outputs/compliance/cis/cis.pyprowler/lib/outputs/compliance/ens/ens.pyprowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.pyprowler/lib/outputs/compliance/universal/universal_table.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py (1)
74-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThreat score can be overstated due to first-hit status locking.
Line 74-Line 85 and Line 114-Line 122 lock score contribution on first encounter per finding/pillar and per finding/generic. With per-requirement
effective_status, a later FAIL for the same finding cannot revoke a prior PASS contribution, so pillar/generic scores and PASS counts can be overstated.This needs a two-phase or precedence-aware aggregation (record worst status per
(finding, pillar)and per finding first, then compute counters/scores once from that resolved status).Also applies to: 91-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py` around lines 74 - 85, The scoring logic in the section checking `if index not in counted_findings_per_pillar[pillar]` and the corresponding generic counting logic (lines 91-122) locks in the contribution on the first encounter of a finding. This causes later status updates for the same finding to be ignored, overstating scores and pass counts. Refactor to use a two-phase approach: first, resolve the final/worst effective_status for each (finding, pillar) pair by iterating through all findings and tracking the worst status per combination, then in a second phase, compute score_per_pillar, max_score_per_pillar, counted_findings_per_pillar and their generic equivalents based on those pre-resolved statuses rather than processing findings sequentially with first-hit locking.prowler/lib/outputs/compliance/ens/ens.py (1)
78-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake PASS/FAIL buckets mutually exclusive with FAIL precedence.
Line 78-Line 88 can place the same non-muted finding in both
fail_countandpass_countwheneffective_statusdiffers across requirements. This can skew percentages and framework totals.A minimal fix is to treat FAIL as dominant:
- when adding to FAIL, remove from PASS;
- only add PASS if the index is not already in FAIL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/ens/ens.py` around lines 78 - 88, The logic for populating fail_count and pass_count lists does not enforce mutual exclusivity, allowing the same index to be added to both lists across different requirement iterations. When effective_status equals "FAIL" and the condition for non-recomendacion is met, after adding the index to fail_count, also remove it from pass_count if it exists there to ensure FAIL takes precedence. Additionally, modify the PASS condition to only add the index to pass_count if it is not already present in fail_count, ensuring that once an index is marked as FAIL, it cannot be moved back to PASS. This ensures FAIL is the dominant status and prevents findings from being counted in both buckets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py`:
- Around line 61-81: The per-section counting logic in the section_seen block
prevents updating a finding's status once it has been recorded for a section.
When the same finding index appears with different effective_status values
across multiple requirements within the same section, only the first status is
counted while subsequent statuses are ignored. This hides FAIL statuses that
come after an initial PASS status. Modify the per-section counting to track the
status of each finding per section (instead of just tracking presence) and
update it to FAIL whenever a FAIL status is encountered for that section/finding
pair, ensuring FAIL precedence takes priority over any previously recorded PASS
status.
In `@prowler/lib/outputs/compliance/c5/c5.py`:
- Around line 58-78: The overview counters (muted_count, fail_count, pass_count)
and the per-section counters have divergent logic that allows a single finding
to be counted in multiple status buckets. Add a guard mechanism for the overview
counters similar to how section_seen is used for per-section counts: introduce a
tracking mechanism (such as a set) to ensure that once a finding identified by
index is assigned to one status bucket (PASS, FAIL, or Muted), it is not counted
in other status buckets. This will maintain one-finding-one-bucket semantics for
the overview totals while also aligning the logic between the overview counts
and the per-section counts blocks.
In `@prowler/lib/outputs/compliance/ccc/ccc.py`:
- Around line 58-78: The deduplication logic for overview totals (fail_count,
pass_count, muted_count lists) allows a single finding index to be added to
multiple status lists when effective_status varies across different requirements
for the same CheckID. Fix this by implementing a "FAIL wins" deduplication
strategy: replace the separate status lists with a dictionary that maps each
index to its final status, then apply precedence logic where FAIL takes priority
over PASS and muted takes priority over both. This ensures each finding is
counted exactly once in the overview totals with its worst status. Apply the
same fix pattern across all affected files (cis.py, kisa_ismsp.py,
mitre_attack.py, and universal_table.py) wherever similar index-based
deduplication exists.
---
Outside diff comments:
In `@prowler/lib/outputs/compliance/ens/ens.py`:
- Around line 78-88: The logic for populating fail_count and pass_count lists
does not enforce mutual exclusivity, allowing the same index to be added to both
lists across different requirement iterations. When effective_status equals
"FAIL" and the condition for non-recomendacion is met, after adding the index to
fail_count, also remove it from pass_count if it exists there to ensure FAIL
takes precedence. Additionally, modify the PASS condition to only add the index
to pass_count if it is not already present in fail_count, ensuring that once an
index is marked as FAIL, it cannot be moved back to PASS. This ensures FAIL is
the dominant status and prevents findings from being counted in both buckets.
In `@prowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.py`:
- Around line 74-85: The scoring logic in the section checking `if index not in
counted_findings_per_pillar[pillar]` and the corresponding generic counting
logic (lines 91-122) locks in the contribution on the first encounter of a
finding. This causes later status updates for the same finding to be ignored,
overstating scores and pass counts. Refactor to use a two-phase approach: first,
resolve the final/worst effective_status for each (finding, pillar) pair by
iterating through all findings and tracking the worst status per combination,
then in a second phase, compute score_per_pillar, max_score_per_pillar,
counted_findings_per_pillar and their generic equivalents based on those
pre-resolved statuses rather than processing findings sequentially with
first-hit locking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a93caf80-74a7-4b12-bdda-7909e5f00bb6
⛔ Files ignored due to path filters (19)
prowler/compliance/aws/aws_well_architected_framework_security_pillar_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ccc_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ens_rd2022_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/fedramp_moderate_revision_4_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/ffiec_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/gxp_21_cfr_part_11_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/iso27001_2013_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/kisa_isms_p_2023_korean_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_171_revision_2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/nist_800_53_revision_5_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/rbi_cyber_security_framework_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/aws/secnumcloud_3.2_aws.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/cis_5.0_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/hipaa_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/secnumcloud_3.2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/azure/soc2_azure.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/csa_ccm_4.0.jsonis excluded by!prowler/compliance/**/*.jsonprowler/compliance/dora_2022_2554.jsonis excluded by!prowler/compliance/**/*.json
📒 Files selected for processing (10)
prowler/lib/check/compliance_models.pyprowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.pyprowler/lib/outputs/compliance/c5/c5.pyprowler/lib/outputs/compliance/ccc/ccc.pyprowler/lib/outputs/compliance/cis/cis.pyprowler/lib/outputs/compliance/ens/ens.pyprowler/lib/outputs/compliance/kisa_ismsp/kisa_ismsp.pyprowler/lib/outputs/compliance/mitre_attack/mitre_attack.pyprowler/lib/outputs/compliance/prowler_threatscore/prowler_threatscore.pyprowler/lib/outputs/compliance/universal/universal_table.py
🛑 Comments failed to post (3)
prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py (1)
61-81: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce FAIL precedence for mixed per-requirement statuses.
Line 62-Line 70 and Line 74-Line 81 can classify one non-muted finding as both PASS and FAIL when different requirements resolve different
effective_statusvalues.section_seenthen keeps the first status and can hide a later FAIL for the same section/finding pair.Suggested fix pattern
- pass_count = [] - fail_count = [] - muted_count = [] - section_seen = {} + pass_count: set[int] = set() + fail_count: set[int] = set() + muted_count: set[int] = set() + section_seen: dict[str, dict[int, str]] = {} @@ - section_seen[section] = set() + section_seen[section] = {} @@ - if finding.muted: - if index not in muted_count: - muted_count.append(index) - elif effective_status == "FAIL": - if index not in fail_count: - fail_count.append(index) - elif effective_status == "PASS": - if index not in pass_count: - pass_count.append(index) + if finding.muted: + muted_count.add(index) + elif effective_status == "FAIL": + fail_count.add(index) + pass_count.discard(index) + elif index not in fail_count and effective_status == "PASS": + pass_count.add(index) @@ - if index not in section_seen[section]: - section_seen[section].add(index) + seen = section_seen[section].get(index) + if seen is None: + section_seen[section][index] = effective_status if finding.muted: sections[section]["Muted"] += 1 elif effective_status == "FAIL": sections[section]["FAIL"] += 1 elif effective_status == "PASS": sections[section]["PASS"] += 1 + elif ( + not finding.muted + and seen == "PASS" + and effective_status == "FAIL" + ): + section_seen[section][index] = "FAIL" + sections[section]["PASS"] -= 1 + sections[section]["FAIL"] += 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/asd_essential_eight/asd_essential_eight.py` around lines 61 - 81, The per-section counting logic in the section_seen block prevents updating a finding's status once it has been recorded for a section. When the same finding index appears with different effective_status values across multiple requirements within the same section, only the first status is counted while subsequent statuses are ignored. This hides FAIL statuses that come after an initial PASS status. Modify the per-section counting to track the status of each finding per section (instead of just tracking presence) and update it to FAIL whenever a FAIL status is encountered for that section/finding pair, ensuring FAIL precedence takes priority over any previously recorded PASS status.prowler/lib/outputs/compliance/c5/c5.py (1)
58-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Overview and section counters can diverge on mixed effective statuses.
Line 58-Line 67 and Line 71-Line 78 have the same first-hit behavior: a finding can end up in both
pass_countandfail_count, and section aggregation may keep PASS if FAIL is seen later for the same section/finding. This breaks one-finding-one-bucket overview semantics after config-based overrides.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/c5/c5.py` around lines 58 - 78, The overview counters (muted_count, fail_count, pass_count) and the per-section counters have divergent logic that allows a single finding to be counted in multiple status buckets. Add a guard mechanism for the overview counters similar to how section_seen is used for per-section counts: introduce a tracking mechanism (such as a set) to ensure that once a finding identified by index is assigned to one status bucket (PASS, FAIL, or Muted), it is not counted in other status buckets. This will maintain one-finding-one-bucket semantics for the overview totals while also aligning the logic between the overview counts and the per-section counts blocks.prowler/lib/outputs/compliance/ccc/ccc.py (1)
58-78: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Overview totals can double-count a finding once
effective_statusvaries per requirement.A single
CheckIDcan map to several requirements in the same framework, andconfig_status(henceeffective_status) is resolved per requirement. The index-based dedup assumed the status was constant per finding (true forfinding.status), but it is no longer. If one requirement yieldsFAIL(config invalid) and another yieldsPASSfor the same finding, the index is appended to bothfail_countandpass_count, inflatingtotal_findings_countand skewing the overview percentages.The per-section counters (Line 71-78) don't double-count, but the section verdict now depends on requirement iteration order.
Make
FAILwin for overview dedup so a finding is counted once:🐛 Proposed fix
if finding.muted: if index not in muted_count: muted_count.append(index) elif effective_status == "FAIL": if index not in fail_count: fail_count.append(index) + if index in pass_count: + pass_count.remove(index) elif effective_status == "PASS": - if index not in pass_count: + if index not in pass_count and index not in fail_count: pass_count.append(index)The same pattern affects
cis.py,kisa_ismsp.py,mitre_attack.py, anduniversal_table.py.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Overview totals: count each finding once per framework if finding.muted: if index not in muted_count: muted_count.append(index) elif effective_status == "FAIL": if index not in fail_count: fail_count.append(index) if index in pass_count: pass_count.remove(index) elif effective_status == "PASS": if index not in pass_count and index not in fail_count: pass_count.append(index) # Per-section counts: count each finding once per section # it belongs to (a finding can map to several sections). if index not in section_seen[section]: section_seen[section].add(index) if finding.muted: sections[section]["Muted"] += 1 elif effective_status == "FAIL": sections[section]["FAIL"] += 1 elif effective_status == "PASS": sections[section]["PASS"] += 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prowler/lib/outputs/compliance/ccc/ccc.py` around lines 58 - 78, The deduplication logic for overview totals (fail_count, pass_count, muted_count lists) allows a single finding index to be added to multiple status lists when effective_status varies across different requirements for the same CheckID. Fix this by implementing a "FAIL wins" deduplication strategy: replace the separate status lists with a dictionary that maps each index to its final status, then apply precedence logic where FAIL takes priority over PASS and muted takes priority over both. This ensures each finding is counted exactly once in the overview totals with its worst status. Apply the same fix pattern across all affected files (cis.py, kisa_ismsp.py, mitre_attack.py, and universal_table.py) wherever similar index-based deduplication exists.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11667 +/- ##
===========================================
- Coverage 94.12% 75.89% -18.23%
===========================================
Files 247 1408 +1161
Lines 36541 45835 +9294
===========================================
+ Hits 34393 34786 +393
- Misses 2148 11049 +8901
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Member
Author
|
closing by #11669 11669 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Steps to review
Please add a detailed description of how to review this PR.
Checklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Summary by CodeRabbit
Release Notes
New Features
[CONFIG NOT VALID].Tests