Checkmarx / kics

Find security vulnerabilities, compliance issues, and infrastructure misconfigurations early in the development cycle of your infrastructure-as-code with KICS by Checkmarx.
https://kics.io
Apache License 2.0
1.97k stars 295 forks source link

feat(query): add new query for tencentcloud CVM resource #7122

Closed SevenEarth closed 2 weeks ago

SevenEarth commented 2 weeks ago

Hello, There is something new query for TencentCloud CVM resource:

  1. cvm_instance_using_default_vpc
  2. cvm_instance_using_default_security_group
  3. cvm_instance_has_public_ip
  4. cvm_instance_disable_monitor_service

Please help review the PR Thanks

I submit this contribution under the Apache-2.0 license.

gabriel-cx commented 2 weeks ago

Hi @SevenEarth ,

Thank you very much for this contribution!! We'd like to request you 2 steps:

SevenEarth commented 2 weeks ago

Hi @SevenEarth ,

Thank you very much for this contribution!! We'd like to request you 2 steps:

  • Kindly take a look at the failing tests
image
  • Please add a prefix "(Beta)" to the queries name, like in this one. For now we need this because when all the tests pass we will merge this PR, but the queries will need to be reviewed by our AppSec later on as well. This "(Beta)" prefix will help the users to know that this queries are part of KICS but they are still under review.

Hello, Thank you for your review. I have a question about the first bug: I ran the following code locally for testing, and the result was normal without any errors go run ./cmd/console/main.go scan -p "/Users/yanxiang/Tencent/Kics/kics/assets/queries/terraform/tencentcloud/cvm_instance_has_public_ip/test" -d "/Users/yanxiang/Tencent/Kics/kics/assets/queries/terraform/tencentcloud/cvm_instance_has_public_ip/test/input.json"

image

May I ask where is the detailed log of the failed e2e test? I want to see the reason for the error?

SevenEarth commented 2 weeks ago

ohhh, I guess it's probably a 'module' issue

gabriel-cx commented 2 weeks ago

@SevenEarth , the unit-tests are the ones that are failing. The e2e is failing because another reason, do not worry! :D Seems like what you expected to happen in your positive_expected_result.json is not happening, for all the 4 new queries added.

SevenEarth commented 2 weeks ago

@SevenEarth , the unit-tests are the ones that are failing. The e2e is failing because another reason, do not worry! :D Seems like what you expected to happen in your positive_expected_result.json is not happening, for all the 4 new queries added.

Thank you I fix all problems There is my local test Please help review the PR 😁

企业微信截图_058de1f9-eb47-43a0-aba7-6e657d5183db
gabriel-cx commented 2 weeks ago

@SevenEarth , thank you for this amazing contribution!!