-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix(aws): remove duplicate bucket logging rule #1423
Conversation
@simar7 The tests failed at the OPA install stage: |
I believe it's one of the existing Go rules that we've transformed into Rego. In this case we would want to remove the Go rule and keep the rego rule. Does the rego rule scan the misconfiguration correctly? |
These are the rules I moved into the |
@simar7 OK, I'll return rego and remove the GO rule. By the way, the rego rule contained an invalid AVD ID. |
Is it possible this duplication also generates a false positive when you do something like this?:
|
If this comment doesn't help you, could you describe in more detail what happened to you and what you expected to see? |
@@ -1,44 +0,0 @@ | |||
package s3 |
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.
We need to keep the examples (in Go) still available to us as they are used for doc generation. An example on how to use them is here https://github.com/aquasecurity/defsec/blob/master/rules/cloud/policies/aws/rds/disable_public_access.rego#L22-L25
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.
@simar7 The good_example
field specifies the GO file, but the file also contains a bad example and links. How does it work?
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.
@nikpivkin they are loaded like such
Line 266 in 063dcd6
if val, ok := sMap["good_examples"].(string); ok { |
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.
@simar7 I think I found a code that parses GO files to generate documentation.
I have a couple of questions:
- Why are only good examples generated for documentation, but not bad ones?
- If there are several good examples, then only 1 is generated
- Links in the metadata of the engine must be specified as values, not links to GO files, and it is impossible to specify multiple links:
# terraform:
# good_examples: "rules/cloud/policies/aws/s3/enable_bucket_logging.tf.go"
# links: "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket"
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.
@simar7 By the way, the "RDS Publicly Accessible" rule is duplicated:
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.
@simar7 By the way, the "RDS Publicly Accessible" rule is duplicated:
New issue aquasecurity/trivy#5065
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.
@simar7 I think I found a code that parses GO files to generate documentation.
I have a couple of questions:
- Why are only good examples generated for documentation, but not bad ones?
- If there are several good examples, then only 1 is generated
- Links in the metadata of the engine must be specified as values, not links to GO files, and it is impossible to specify multiple links:
# terraform: # good_examples: "rules/cloud/policies/aws/s3/enable_bucket_logging.tf.go" # links: "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket"
All of the above are improvements that we can add. The existing documentation at the time of implementing mostly had good examples than bad so support was added for them at first.
@@ -1,37 +0,0 @@ | |||
package s3 |
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.
ditto for terraform example.
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.
left comments
@simar7 I updated the config for |
Sounds good. Let's create a separate issue and track it there. |
91a30d7
to
9ee7e28
Compare
We already have a bucket logging rule.
See aquasecurity/trivy#4975
Fixes aquasecurity/trivy#5004