-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat(stepfunctions): add stepfunctions service and check stepfunctions_statemachine_logging_enabled
#5466
base: master
Are you sure you want to change the base?
Conversation
You can check the documentation for this PR here -> Prowler Documentation |
fcf088e
to
6636b62
Compare
You can check the documentation for this PR here -> Prowler Documentation |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5466 +/- ##
==========================================
+ Coverage 89.88% 89.92% +0.03%
==========================================
Files 1132 1136 +4
Lines 35275 35450 +175
==========================================
+ Hits 31706 31877 +171
- Misses 3569 3573 +4 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
…ging_enabled check
6636b62
to
4999bf0
Compare
You can check the documentation for this PR here -> Prowler Documentation |
…hines-Have-Logging-Enabled
You can check the documentation for this PR here -> Prowler Documentation |
...ctions_statemachine_logging_enabled/stepfunctions_statemachine_logging_enabled.metadata.json
Outdated
Show resolved
Hide resolved
…hines-Have-Logging-Enabled
You can check the documentation for this PR here -> Prowler Documentation |
Co-authored-by: Rubén De la Torre Vico <[email protected]>
You can check the documentation for this PR here -> Prowler Documentation |
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.
@AdriiiPRodri, to simplify the check, please ensure it only verifies whether the state machine has logging enabled, regardless of the log level (similar to how other logging checks in Prowler work). Additionally, include only the necessary attributes and functions required to verify logging, avoiding any extra details that Prowler doesn't currently use. This approach will help minimize potential errors in unused attributes or functions.
Thanks for your work on this! 😊
In this case I have added the logic to check the logging level enabled since it seems that SecurityHub does the same, so I replicated the behavior to make it work like SecurityHub: https://docs.aws.amazon.com/securityhub/latest/userguide/stepfunctions-controls.html#stepfunctions-1 If this is not really necessary I will remove that logic, I would need confirmation as this logging check is not like other services. |
As agreed, the specific logging level check has been removed. Now it is ready for review @MrCloudSec @puchy22 |
prowler/providers/aws/services/stepfunctions/stepfunctions_service.py
Outdated
Show resolved
Hide resolved
prowler/providers/aws/services/stepfunctions/stepfunctions_service.py
Outdated
Show resolved
Hide resolved
prowler/providers/aws/services/stepfunctions/stepfunctions_service.py
Outdated
Show resolved
Hide resolved
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.
Please @AdriiiPRodri, review my comments.
…ved specified logging level check
9ef9a54
to
6cf1db1
Compare
prowler/providers/aws/services/stepfunctions/stepfunctions_service.py
Outdated
Show resolved
Hide resolved
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.
Could you update the tests to use Moto? It’s important to ensure the responses match AWS API behavior as closely as possible. Let me know if you need help with it!
Context
To cover the checks for the Step Functions service, in this PR we have added that service and have also included the logging enabled check: [StepFunctions.1] Step Functions state machines should have logging turned on
Description
This PR adds:
stepfunctions_statemachine_logging_enabled
check.Checklist
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.