feat(github): add check for stale review dismissal via repository rulesets#10369
feat(github): add check for stale review dismissal via repository rulesets#10369tejas0077 wants to merge 3 commits intoprowler-cloud:masterfrom
Conversation
- Add Ruleset model to repository_service.py - Add repository_ruleset_dismisses_stale_reviews check - Support both modern rulesets and legacy branch protection - Closes prowler-cloud#9530
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
There was a problem hiding this comment.
Thanks for the contribution. I think there are a few issues that need to be addressed before this can be merged.
-
The new GitHub check does not look compatible with the current check implementation pattern.
In the current codebase, GitHub repository checks useCheckReportGithub, but this PR introducesCheckResultandCheckStatus. I do not see those types added anywhere in this PR, so as submitted this looks like it will not import/run correctly. -
The check is placed under
prowler/providers/github/services/repository/checks/..., which does not match Prowler’s standard check discovery layout.
The loader expects checks under:
prowler.providers.<provider>.services.<service>.<check_name>.<check_name>not under an extrachecks/directory. With the current path, this check is very likely not going to be auto-discovered. -
repo.rulesetsis added to theRepomodel, but I do not see it being populated inrepository_service.py.
That means the new check will never evaluate real repository rulesets and will always fall back to the legacy branch protection path. -
The ruleset evaluation logic is too broad and can produce false positives.
The check currently passes if it finds any active ruleset withdismiss_stale_reviews_on_push = true, but it does not verify that the
ruleset actually applies to the repository’s default branch / target branch being evaluated. -
There are no tests for this new behavior.
This needs coverage for:- ruleset parsing in the repository service
- discovery/loading of the new check
- PASS / FAIL / MANUAL scenarios
- legacy branch protection fallback behavior
-
This PR also introduces changes to GCP compliance file (
rbi_cyber_security_framework_gcp.json), which is unrelated to the GitHub repository ruleset check.
These are two separate concerns and should be split into different PRs. Mixing a GitHub provider check with a GCP compliance framework makes review and validation much harder.
I would recommend splitting the compliance addition into its own PR, then reworking this GitHub check to:
- follow the standard repository check layout
- use the existing GitHub check reporting model
- populate rulesets from the GitHub API in
repository_service.py - validate applicability to the default branch
- add provider tests
|
Hi @HugoPBrito, thank you for the detailed review! I'll address all the issues:
Will push fixes shortly! |
- Move check to correct folder structure (no extra checks/ dir) - Use CheckReportGithub instead of CheckResult/CheckStatus - Populate rulesets in _process_repository() via _get_repository_rulesets() - Add Ruleset model to repository_service.py - Filter rulesets by target=branch for default branch validation - Remove unrelated GCP compliance changes
|
Hi @HugoPBrito, I've addressed all the review feedback:
Still working on adding tests - will push shortly. Please re-review when you get a chance. Thanks! |
This PR adds support for GitHub Repository Rulesets in Prowler's GitHub provider checks.
GitHub introduced Repository Rulesets as the modern replacement for legacy branch protection rules. Organizations that have migrated to rulesets were getting false positives in existing Prowler checks because the checks only read legacy branch protection data.
Fix #9530
Adds a new check
repository_ruleset_dismisses_stale_reviewsthat verifies whether stale PR approvals are dismissed when new commits are pushed — using the modern GitHub Rulesets API.Changes:
Rulesetmodel torepository_service.pyrepository_ruleset_dismisses_stale_reviewsNo new dependencies required.
repository_service.py— Ruleset model added at bottom of filerepository_ruleset_dismisses_stale_reviews.py— check logicrepository_ruleset_dismisses_stale_reviews.metadata.json— metadata