Skip to content

Commit

Permalink
checkov:chore - improve tests and code cleaning
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@matheusalcantarazup
matheusalcantarazup committed Dec 23, 2021
1 parent 6b79cad commit 33e35fa
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 88 deletions.
20 changes: 5 additions & 15 deletions .../formatters/hcl/checkov/entities/check.go → .../services/formatters/hcl/checkov/check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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
}
6 changes: 3 additions & 3 deletions ...atters/hcl/checkov/entities/check_test.go → ...ices/formatters/hcl/checkov/check_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/services/formatters/hcl/checkov/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
41 changes: 18 additions & 23 deletions internal/services/formatters/hcl/checkov/formatter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
}
Expand All @@ -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))
}
165 changes: 132 additions & 33 deletions internal/services/formatters/hcl/checkov/formatter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
}
}
`
12 changes: 6 additions & 6 deletions ...formatters/hcl/checkov/entities/result.go → ...services/formatters/hcl/checkov/result.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package entities
package checkov

type Result struct {
PassedChecks []*Check `json:"passed_checks"` // ignore results other than vulnerabilities
FailedChecks []*Check `json:"failed_checks"`
SkippedChecks []*Check `json:"skipped_checks"` // ignore results other than vulnerabilities
ParsingErrors []*Check `json:"parsing_errors"` // ignore results other than vulnerabilities
type checkovResult struct {
PassedChecks []*checkovCheck `json:"passed_checks"` // ignore results other than vulnerabilities
FailedChecks []*checkovCheck `json:"failed_checks"`
SkippedChecks []*checkovCheck `json:"skipped_checks"` // ignore results other than vulnerabilities
ParsingErrors []*checkovCheck `json:"parsing_errors"` // ignore results other than vulnerabilities
}
4 changes: 2 additions & 2 deletions ...ormatters/hcl/checkov/entities/summary.go → ...ervices/formatters/hcl/checkov/summary.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package entities
package checkov

type Summary struct {
type checkovSummary struct {
Passed int `json:"passed"`
Failed int `json:"failed"`
Skipped int `json:"skipped"`
Expand Down
Loading

0 comments on commit 33e35fa

Please sign in to comment.