-
Notifications
You must be signed in to change notification settings - Fork 487
feat(rules): Standardize CIS rule IDs with provider prefixes #2334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 9 files
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
jychp
approved these changes
Feb 3, 2026
Collaborator
jychp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 non blockling comment
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
Signed-off-by: Kunaal Sikka <kunaal@subimage.io>
10 tasks
jychp
pushed a commit
that referenced
this pull request
Feb 3, 2026
#2336) ### Type of change - [ ] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Refactoring (no functional changes) - [ ] Documentation update - [ ] Other (please describe): ### Summary The `unrestricted_all_ports` rule was incorrectly placed in `cis_aws_networking.py` despite not being an actual CIS benchmark control: - It lacked a proper CIS control ID tag (e.g., `cis:5.X`) - only had `cis:aws-5.0` (benchmark version) - The rule ID was `unrestricted_all_ports` instead of `cis_aws_5_X_...` naming convention - Comments explicitly marked it as "Additional: Unrestricted All Ports" (not a CIS control) - The existing CIS 5.1 (SSH) and CIS 5.2 (RDP) rules already catch `protocol=-1` cases via their `OR rule.protocol = '-1'` clauses This PR removes the rule to maintain accuracy of CIS benchmark mappings. ### Related issues or links - Related to #2334 (feat(rules): Standardize CIS rule IDs with provider prefixes) ### Checklist #### General - [x] I have read the [contributing guidelines](https://cartography-cncf.github.io/cartography/dev/developer-guide.html). - [x] The linter passes locally (`make lint`). - [x] I have added/updated tests that prove my fix is effective or my feature works. #### Proof of functionality - [x] New or updated unit/integration tests. N/A - This is a removal of a rule that had no tests. Existing tests continue to pass. ### Notes for reviewers The rule being removed checked for security groups with `protocol = '-1'` (all traffic) from `0.0.0.0/0`. While this is a valid security concern, it's not a CIS benchmark control. The existing CIS 5.1 and 5.2 rules already catch this case through their `OR rule.protocol = '-1'` conditions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Kunaal Sikka <kunaal@subimage.io> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Type of change
Summary
Standardized CIS rule IDs to include provider prefixes (e.g., AWS and Google Workspace), updated registry mappings, docs, and tests.
Rationale: CIS publishes separate benchmarks for many vendor product families, so control numbering is scoped to each benchmark. This causes collisions like "3.1" meaning different things across providers (AWS 3.1 is CloudTrail multi-Region logging, while GCP 3.1 is default network removal). Provider prefixes make rule IDs globally unambiguous for cross-provider filtering/reporting.
Breaking changes
Rule IDs now include provider prefixes. Downstream references to legacy rule IDs must be updated to the new prefixed IDs.
How was this tested?
make testChecklist
General
make lint).Proof of functionality
If you are adding or modifying a synced entity
If you are changing a node or relationship
If you are implementing a new intel module
Notes for reviewers