checkov:chore - improve tests and code cleaning (#900)

This commit add some new asserts on successful parsing Checkov results
to verify that all fields of Vulnerability was filled.

Some code organization was also made, and the entities packages was
removed and the Checkov schema output was moved to checkov package.

Updates #718

Signed-off-by: Matheus Alcantara <[email protected]>

internal/services/formatters/hcl/checkov/entities/check.go → internal/services/formatters/hcl/checkov/check.go

@@ -12,16 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package checkov
 
 import (
 	"fmt"
 	"strconv"
-
-	"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
 )
 
-type Check struct {
+type checkovCheck struct {
 	CheckID       string  `json:"check_id"`
 	BCCheckID     string  `json:"bc_check_id"`
 	CheckName     string  `json:"check_name"`
@@ -33,22 +31,14 @@ type Check struct {
 	Guideline     *string `json:"guideline"`
 }
 
-func (c *Check) GetDetails() string {
+func (c *checkovCheck) getDetails() string {
 	return fmt.Sprintf("%s -> [%s]", c.CheckID, c.CheckName)
 }
 
-func (c *Check) GetStartLine() string {
+func (c *checkovCheck) getStartLine() string {
 	return strconv.Itoa(c.FileLineRange[0])
 }
 
-func (c *Check) GetCode() string {
+func (c *checkovCheck) getCode() string {
 	return fmt.Sprintf("code beetween line %d and %d.", c.FileLineRange[0], c.FileLineRange[1])
 }
-
-func (c *Check) GetFilename() string {
-	return c.FileAbsPath
-}
-
-func (c *Check) GetSeverity() severities.Severity {
-	return severities.Unknown
-}

internal/services/formatters/hcl/checkov/entities/check_test.go → internal/services/formatters/hcl/checkov/check_test.go

@@ -12,17 +12,17 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package entities
+package checkov
 
 import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func checkMock() *Check {
+func checkMock() *checkovCheck {
 	guideline := "test"
-	return &Check{
+	return &checkovCheck{
 		CheckID:       "CKV_AWS_41",
 		CheckName:     "test",
 		Guideline:     &guideline,
@@ -33,24 +33,18 @@ func checkMock() *Check {
 
 func TestGetDetails(t *testing.T) {
 	t.Run("should success get result details", func(t *testing.T) {
-		assert.NotEmpty(t, checkMock().GetDetails())
+		assert.NotEmpty(t, checkMock().getDetails())
 	})
 }
 
 func TestGetStartLine(t *testing.T) {
 	t.Run("should success get start line", func(t *testing.T) {
-		assert.NotEmpty(t, checkMock().GetStartLine())
+		assert.NotEmpty(t, checkMock().getStartLine())
 	})
 }
 
 func TestGetCode(t *testing.T) {
 	t.Run("should success get code", func(t *testing.T) {
-		assert.NotEmpty(t, checkMock().GetCode())
-	})
-}
-
-func TestGetFilename(t *testing.T) {
-	t.Run("should success get filename", func(t *testing.T) {
-		assert.NotEmpty(t, checkMock().GetFilename())
+		assert.NotEmpty(t, checkMock().getCode())
 	})
 }

internal/services/formatters/hcl/checkov/config.go

@@ -16,4 +16,4 @@ package checkov
 
 const CMD = `{{WORK_DIR}} checkov --quiet --framework terraform -d . -o json `
 
-var CheckovEmptyValue = []byte("[]\r\n")
+var checkovEmptyValue = []byte("[]\r\n")

internal/services/formatters/hcl/checkov/formatter.go

@@ -20,15 +20,15 @@ import (
 
 	"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
+	"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
 	"github.com/ZupIT/horusec-devkit/pkg/utils/logger"
 	"github.com/pborman/ansi"
 
-	dockerEntities "github.com/ZupIT/horusec/internal/entities/docker"
+	"github.com/ZupIT/horusec/internal/entities/docker"
 	"github.com/ZupIT/horusec/internal/enums/images"
 	"github.com/ZupIT/horusec/internal/helpers/messages"
 	"github.com/ZupIT/horusec/internal/services/formatters"
-	"github.com/ZupIT/horusec/internal/services/formatters/hcl/checkov/entities"
 	vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"
 )
 
@@ -64,8 +64,8 @@ func (f *Formatter) startCheckov(projectSubPath string) (string, error) {
 	return output, f.parseOutput(output)
 }
 
-func (f *Formatter) getDockerConfig(projectSubPath string) *dockerEntities.AnalysisData {
-	analysisData := &dockerEntities.AnalysisData{
+func (f *Formatter) getDockerConfig(projectSubPath string) *docker.AnalysisData {
+	analysisData := &docker.AnalysisData{
 		CMD:      f.AddWorkDirInCmd(CMD, projectSubPath, tools.Checkov),
 		Language: languages.HCL,
 	}
@@ -74,36 +74,31 @@ func (f *Formatter) getDockerConfig(projectSubPath string) *dockerEntities.Analy
 }
 
 func (f *Formatter) parseOutput(output string) error {
-	var vuln *entities.Vulnerability
+	var vuln *checkovVulnerability
 	binary, _ := ansi.Strip([]byte(output))
 	// For some reason checkov returns an empty list when no vulnerabilities are found
 	// and an object if vulnerabitilies are found, this checks ignores result when we have no vulnerabilities
-	if bytes.Equal(binary, CheckovEmptyValue) {
+	if bytes.Equal(binary, checkovEmptyValue) {
 		return nil
 	}
 	if err := json.Unmarshal(binary, &vuln); err != nil {
 		return err
 	}
 	for _, check := range vuln.Results.FailedChecks {
-		f.AddNewVulnerabilityIntoAnalysis(f.setVulnerabilityData(check))
+		f.AddNewVulnerabilityIntoAnalysis(f.newVulnerability(check))
 	}
 	return nil
 }
 
-func (f *Formatter) setVulnerabilityData(check *entities.Check) *vulnerability.Vulnerability {
-	vuln := f.getDefaultVulnerabilityData()
-	vuln.Severity = check.GetSeverity()
-	vuln.Details = check.GetDetails()
-	vuln.Line = check.GetStartLine()
-	vuln.Code = f.GetCodeWithMaxCharacters(check.GetCode(), 0)
-	vuln.File = f.RemoveSrcFolderFromPath(check.GetFilename())
-	vuln = vulnhash.Bind(vuln)
-	return f.SetCommitAuthor(vuln)
-}
-
-func (f *Formatter) getDefaultVulnerabilityData() *vulnerability.Vulnerability {
-	vulnerabilitySeverity := &vulnerability.Vulnerability{}
-	vulnerabilitySeverity.SecurityTool = tools.Checkov
-	vulnerabilitySeverity.Language = languages.HCL
-	return vulnerabilitySeverity
+func (f *Formatter) newVulnerability(check *checkovCheck) *vulnerability.Vulnerability {
+	vuln := &vulnerability.Vulnerability{
+		SecurityTool: tools.Checkov,
+		Language:     languages.HCL,
+		Severity:     severities.Unknown,
+		Details:      check.getDetails(),
+		Line:         check.getStartLine(),
+		Code:         f.GetCodeWithMaxCharacters(check.getCode(), 0),
+		File:         f.RemoveSrcFolderFromPath(check.FileAbsPath),
+	}
+	return f.SetCommitAuthor(vulnhash.Bind(vuln))
 }

internal/services/formatters/hcl/checkov/formatter_test.go

@@ -18,76 +18,79 @@ import (
 	"errors"
 	"testing"
 
-	entitiesAnalysis "github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
 	"github.com/stretchr/testify/assert"
 
-	cliConfig "github.com/ZupIT/horusec/config"
+	"github.com/ZupIT/horusec/config"
 	"github.com/ZupIT/horusec/internal/entities/toolsconfig"
-	"github.com/ZupIT/horusec/internal/entities/workdir"
 	"github.com/ZupIT/horusec/internal/services/formatters"
 	"github.com/ZupIT/horusec/internal/utils/testutil"
 )
 
-func TestStartHCLCheckov(t *testing.T) {
-	t.Run("should successfully execute container and process output", func(t *testing.T) {
+func TestCheckovParseOutput(t *testing.T) {
+	t.Run("should add 1 vulnerability on analysis with no errors", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
-
-		output := `{"check_type":"terraform","results":{"passed_checks":[],"failed_checks":[{"check_id":"CKV_AWS_158","bc_check_id":null,"check_name":"Ensure that CloudWatch Log Group is encrypted by KMS","check_result":{"result":"FAILED","evaluated_keys":["kms_key_id"]},"file_path":"/terraform/main.tf","file_abs_path":"/tf/terraform/main.tf","repo_file_path":"/tf/terraform/main.tf","file_line_range":[528,531],"resource":"aws_cloudwatch_log_group.log_group","evaluations":null,"check_class":"checkov.terraform.checks.resource.aws.CloudWatchLogGroupKMSKey","fixed_definition":null,"entity_tags":null,"caller_file_path":null,"caller_file_line_range":null},{"check_id":"CKV_AWS_147","bc_check_id":null,"check_name":"Ensure that CodeBuild projects are encrypted","check_result":{"result":"FAILED","evaluated_keys":["encryption_key"]},"file_path":"/terraform/main.tf","file_abs_path":"/tf/terraform/main.tf","repo_file_path":"/tf/terraform/main.tf","file_line_range":[677,733],"resource":"aws_codebuild_project.legacy","evaluations":null,"check_class":"checkov.terraform.checks.resource.aws.CodeBuildEncrypted","fixed_definition":null,"entity_tags":null,"caller_file_path":null,"caller_file_line_range":null},{"check_id":"CKV_AWS_78","bc_check_id":null,"check_name":"Ensure that CodeBuild Project encryption is not disabled","check_result":{"result":"FAILED","evaluated_keys":["artifacts/[0]/encryption_disabled"]},"file_path":"/terraform/main.tf","file_abs_path":"/tf/terraform/main.tf","repo_file_path":"/tf/terraform/main.tf","file_line_range":[677,733],"resource":"aws_codebuild_project.legacy","evaluations":null,"check_class":"checkov.terraform.checks.resource.aws.CodeBuildProjectEncryption","fixed_definition":null,"entity_tags":null,"caller_file_path":null,"caller_file_line_range":null,"guideline":"https://docs.bridgecrew.io/docs/bc_aws_general_30"}],"skipped_checks":[],"parsing_errors":[]},"summary":{"passed":0,"failed":3,"skipped":0,"parsing_errors":0,"resource_count":7,"checkov_version":"2.0.330"}}`
-
 		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)
 
+		analysis := new(analysis.Analysis)
+		config := config.New()
+
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
 
 		formatter.StartAnalysis("")
 
-		assert.NotEmpty(t, analysis)
 		assert.Len(t, analysis.AnalysisVulnerabilities, 3)
+		for _, v := range analysis.AnalysisVulnerabilities {
+			vuln := v.Vulnerability
+			assert.Equal(t, tools.Checkov, vuln.SecurityTool)
+			assert.Equal(t, languages.HCL, vuln.Language)
+			assert.NotEmpty(t, vuln.Details, "Exepcted not empty details")
+			assert.NotContains(t, vuln.Details, "()")
+			assert.NotEmpty(t, vuln.Code, "Expected not empty code")
+			assert.NotEmpty(t, vuln.File, "Expected not empty file name")
+			assert.NotEmpty(t, vuln.Line, "Expected not empty line")
+			assert.NotEmpty(t, vuln.Severity, "Expected not empty severity")
+		}
 	})
 
-	t.Run("should return error when invalid output", func(t *testing.T) {
+	t.Run("should add error on analysis when invalid output", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
+		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("invalid", nil)
 
-		output := "!@#!@#"
+		analysis := new(analysis.Analysis)
 
-		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return(output, nil)
+		config := config.New()
 
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
+		formatter.StartAnalysis("")
 
-		assert.NotPanics(t, func() {
-			formatter.StartAnalysis("")
-		})
+		assert.True(t, analysis.HasErrors(), "Expected errors on analysis")
 	})
 
-	t.Run("should return error when executing container", func(t *testing.T) {
+	t.Run("should add error from executing container on analysis", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
-
 		dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("", errors.New("test"))
 
+		analysis := new(analysis.Analysis)
+
+		config := config.New()
+
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
+		formatter.StartAnalysis("")
 
-		assert.NotPanics(t, func() {
-			formatter.StartAnalysis("")
-		})
+		assert.True(t, analysis.HasErrors(), "Expected errors on analysis")
 	})
 
 	t.Run("Should not execute tool because it's ignored", func(t *testing.T) {
 		dockerAPIControllerMock := testutil.NewDockerMock()
-		analysis := &entitiesAnalysis.Analysis{}
-		config := &cliConfig.Config{}
-		config.WorkDir = &workdir.WorkDir{}
+
+		analysis := new(analysis.Analysis)
+		config := config.New()
 		config.ToolsConfig = toolsconfig.ToolsConfig{
 			tools.Checkov: toolsconfig.Config{
 				IsToIgnore: true,
@@ -96,7 +99,103 @@ func TestStartHCLCheckov(t *testing.T) {
 
 		service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config)
 		formatter := NewFormatter(service)
-
 		formatter.StartAnalysis("")
 	})
 }
+
+const output = `
+{
+  "check_type": "terraform",
+  "results": {
+    "passed_checks": [],
+    "failed_checks": [
+      {
+        "check_id": "CKV_AWS_158",
+        "bc_check_id": null,
+        "check_name": "Ensure that CloudWatch Log Group is encrypted by KMS",
+        "check_result": {
+          "result": "FAILED",
+          "evaluated_keys": [
+            "kms_key_id"
+          ]
+        },
+        "file_path": "/terraform/main.tf",
+        "file_abs_path": "/tf/terraform/main.tf",
+        "repo_file_path": "/tf/terraform/main.tf",
+        "file_line_range": [
+          528,
+          531
+        ],
+        "resource": "aws_cloudwatch_log_group.log_group",
+        "evaluations": null,
+        "check_class": "checkov.terraform.checks.resource.aws.CloudWatchLogGroupKMSKey",
+        "fixed_definition": null,
+        "entity_tags": null,
+        "caller_file_path": null,
+        "caller_file_line_range": null
+      },
+      {
+        "check_id": "CKV_AWS_147",
+        "bc_check_id": null,
+        "check_name": "Ensure that CodeBuild projects are encrypted",
+        "check_result": {
+          "result": "FAILED",
+          "evaluated_keys": [
+            "encryption_key"
+          ]
+        },
+        "file_path": "/terraform/main.tf",
+        "file_abs_path": "/tf/terraform/main.tf",
+        "repo_file_path": "/tf/terraform/main.tf",
+        "file_line_range": [
+          677,
+          733
+        ],
+        "resource": "aws_codebuild_project.legacy",
+        "evaluations": null,
+        "check_class": "checkov.terraform.checks.resource.aws.CodeBuildEncrypted",
+        "fixed_definition": null,
+        "entity_tags": null,
+        "caller_file_path": null,
+        "caller_file_line_range": null
+      },
+      {
+        "check_id": "CKV_AWS_78",
+        "bc_check_id": null,
+        "check_name": "Ensure that CodeBuild Project encryption is not disabled",
+        "check_result": {
+          "result": "FAILED",
+          "evaluated_keys": [
+            "artifacts/[0]/encryption_disabled"
+          ]
+        },
+        "file_path": "/terraform/main.tf",
+        "file_abs_path": "/tf/terraform/main.tf",
+        "repo_file_path": "/tf/terraform/main.tf",
+        "file_line_range": [
+          677,
+          733
+        ],
+        "resource": "aws_codebuild_project.legacy",
+        "evaluations": null,
+        "check_class": "checkov.terraform.checks.resource.aws.CodeBuildProjectEncryption",
+        "fixed_definition": null,
+        "entity_tags": null,
+        "caller_file_path": null,
+        "caller_file_line_range": null,
+        "guideline": "https://docs.bridgecrew.io/docs/bc_aws_general_30"
+      }
+    ],
+    "skipped_checks": [],
+    "parsing_errors": []
+  },
+  "summary": {
+    "passed": 0,
+    "failed": 3,
+    "skipped": 0,
+    "parsing_errors": 0,
+    "resource_count": 7,
+    "checkov_version": "2.0.330"
+  }
+}
+`