From 3c9754b03cc942e74e62a402696ef3c10e526033 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Thu, 23 Dec 2021 10:16:21 -0300 Subject: [PATCH] tfsec:chore - improve tests and code cleaning This commit add some new asserts on successful parsing tfsec results to verify that all fields of Vulnerability was filled. Some code organization was also made, and the entities packages was removed and the tfsec schema output was moved to tfsec package. Updates #718 Signed-off-by: Matheus Alcantara --- .../formatters/hcl/tfsec/formatter.go | 40 +++--- .../formatters/hcl/tfsec/formatter_test.go | 133 +++++++++++++----- .../hcl/tfsec/{entities => }/location.go | 2 +- .../hcl/tfsec/{entities => }/result.go | 16 +-- .../hcl/tfsec/{entities => }/result_test.go | 14 +- .../tfsec/{entities => }/vulnerabilities.go | 6 +- 6 files changed, 135 insertions(+), 76 deletions(-) rename internal/services/formatters/hcl/tfsec/{entities => }/location.go (97%) rename internal/services/formatters/hcl/tfsec/{entities => }/result.go (79%) rename internal/services/formatters/hcl/tfsec/{entities => }/result_test.go (82%) rename internal/services/formatters/hcl/tfsec/{entities => }/vulnerabilities.go (87%) diff --git a/internal/services/formatters/hcl/tfsec/formatter.go b/internal/services/formatters/hcl/tfsec/formatter.go index f8afb4a1a..04467b137 100644 --- a/internal/services/formatters/hcl/tfsec/formatter.go +++ b/internal/services/formatters/hcl/tfsec/formatter.go @@ -16,7 +16,7 @@ package tfsec import ( "encoding/json" - "fmt" + "errors" "strings" "github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability" @@ -24,11 +24,10 @@ import ( "github.com/ZupIT/horusec-devkit/pkg/enums/tools" "github.com/ZupIT/horusec-devkit/pkg/utils/logger" - 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/tfsec/entities" vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash" ) @@ -64,8 +63,8 @@ func (f *Formatter) startTfSec(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.TfSec), Language: languages.HCL, } @@ -74,37 +73,32 @@ func (f *Formatter) getDockerConfig(projectSubPath string) *dockerEntities.Analy } func (f *Formatter) parseOutput(output string) error { - var vulnerabilities *entities.Vulnerabilities + var vulnerabilities *tfsecVulnerabilities if err := json.Unmarshal([]byte(output), &vulnerabilities); err != nil { if !strings.Contains(output, "panic") { - return fmt.Errorf("{HORUSEC_CLI} Error %s", output) + return errors.New(output) } - return err } for index := range vulnerabilities.Results { - f.AddNewVulnerabilityIntoAnalysis(f.setVulnerabilityData(index, vulnerabilities.Results)) + f.AddNewVulnerabilityIntoAnalysis(f.newVulnerability(&vulnerabilities.Results[index])) } return nil } -func (f *Formatter) setVulnerabilityData(index int, results []entities.Result) *vulnerability.Vulnerability { - vuln := f.getDefaultVulnerabilityData() - vuln.Severity = results[index].GetSeverity() - vuln.Details = results[index].GetDetails() - vuln.Line = results[index].GetStartLine() - vuln.Code = f.GetCodeWithMaxCharacters(results[index].GetCode(), 0) - vuln.File = f.RemoveSrcFolderFromPath(results[index].GetFilename()) +func (f *Formatter) newVulnerability(result *tfsecResult) *vulnerability.Vulnerability { + vuln := &vulnerability.Vulnerability{ + SecurityTool: tools.TfSec, + Language: languages.HCL, + Severity: result.getSeverity(), + Details: result.getDetails(), + Line: result.getStartLine(), + Code: f.GetCodeWithMaxCharacters(result.getCode(), 0), + File: f.RemoveSrcFolderFromPath(result.getFilename()), + } vuln = vulnhash.Bind(vuln) return f.SetCommitAuthor(vuln) } - -func (f *Formatter) getDefaultVulnerabilityData() *vulnerability.Vulnerability { - vulnerabilitySeverity := &vulnerability.Vulnerability{} - vulnerabilitySeverity.SecurityTool = tools.TfSec - vulnerabilitySeverity.Language = languages.HCL - return vulnerabilitySeverity -} diff --git a/internal/services/formatters/hcl/tfsec/formatter_test.go b/internal/services/formatters/hcl/tfsec/formatter_test.go index 068756e7c..5f386fd25 100644 --- a/internal/services/formatters/hcl/tfsec/formatter_test.go +++ b/internal/services/formatters/hcl/tfsec/formatter_test.go @@ -18,83 +18,148 @@ 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 TestStartHCLTfSec(t *testing.T) { - t.Run("should success execute container and process output", func(t *testing.T) { +func TestTFSecParseOutput(t *testing.T) { + t.Run("should add 5 vulnerabilities on analysis with no errors", func(t *testing.T) { dockerAPIControllerMock := testutil.NewDockerMock() - analysis := &entitiesAnalysis.Analysis{} - config := &cliConfig.Config{} - config.WorkDir = &workdir.WorkDir{} - - output := `{"results":[{"rule_id":"AWS018","link":"https://github.com/liamg/tfsec/wiki/AWS018","location":{"filename":"/src/terraform/main.tf","start_line":2,"end_line":5},"description":"Resource 'aws_security_group_rule.my-rule' should include a description for auditing purposes.","severity":"ERROR"},{"rule_id":"AWS006","link":"https://github.com/liamg/tfsec/wiki/AWS006","location":{"filename":"/src/terraform/main.tf","start_line":4,"end_line":4},"description":"Resource 'aws_security_group_rule.my-rule' defines a fully open ingress security group rule.","severity":"WARNING"},{"rule_id":"AWS004","link":"https://github.com/liamg/tfsec/wiki/AWS004","location":{"filename":"/src/terraform/main.tf","start_line":9,"end_line":9},"description":"Resource 'aws_alb_listener.my-alb-listener' uses plain HTTP instead of HTTPS.","severity":"ERROR"},{"rule_id":"AWS003","link":"https://github.com/liamg/tfsec/wiki/AWS003","location":{"filename":"/src/terraform/main.tf","start_line":12,"end_line":14},"description":"Resource 'aws_db_security_group.my-group' uses EC2 Classic. Use a VPC instead.","severity":"ERROR"},{"rule_id":"AZU003","link":"https://github.com/liamg/tfsec/wiki/AZU003","location":{"filename":"/src/terraform/main.tf","start_line":18,"end_line":18},"description":"Resource 'azurerm_managed_disk.source' defines an unencrypted managed disk.","severity":"ERROR"}]}` - 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, 5) + for _, v := range analysis.AnalysisVulnerabilities { + vuln := v.Vulnerability + + assert.Equal(t, tools.TfSec, vuln.SecurityTool) + assert.Equal(t, languages.HCL, vuln.Language) + assert.NotEmpty(t, vuln.Details, "Expected not empty details") + assert.NotEmpty(t, vuln.Code, "Expected not empty code") + assert.NotEmpty(t, vuln.File, "Expected not emppty 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 of 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) { - analysis := &entitiesAnalysis.Analysis{} + analysis := new(analysis.Analysis) + dockerAPIControllerMock := testutil.NewDockerMock() - config := &cliConfig.Config{} - config.WorkDir = &workdir.WorkDir{} + + config := config.New() config.ToolsConfig = toolsconfig.ToolsConfig{ tools.TfSec: toolsconfig.Config{ IsToIgnore: true, }, } + service := formatters.NewFormatterService(analysis, dockerAPIControllerMock, config) formatter := NewFormatter(service) - formatter.StartAnalysis("") }) } + +const output = ` +{ + "results": [ + { + "rule_id": "AWS018", + "link": "https://github.com/liamg/tfsec/wiki/AWS018", + "location": { + "filename": "/src/terraform/main.tf", + "start_line": 2, + "end_line": 5 + }, + "description": "Resource 'aws_security_group_rule.my-rule' should include a description for auditing purposes.", + "severity": "ERROR" + }, + { + "rule_id": "AWS006", + "link": "https://github.com/liamg/tfsec/wiki/AWS006", + "location": { + "filename": "/src/terraform/main.tf", + "start_line": 4, + "end_line": 4 + }, + "description": "Resource 'aws_security_group_rule.my-rule' defines a fully open ingress security group rule.", + "severity": "WARNING" + }, + { + "rule_id": "AWS004", + "link": "https://github.com/liamg/tfsec/wiki/AWS004", + "location": { + "filename": "/src/terraform/main.tf", + "start_line": 9, + "end_line": 9 + }, + "description": "Resource 'aws_alb_listener.my-alb-listener' uses plain HTTP instead of HTTPS.", + "severity": "ERROR" + }, + { + "rule_id": "AWS003", + "link": "https://github.com/liamg/tfsec/wiki/AWS003", + "location": { + "filename": "/src/terraform/main.tf", + "start_line": 12, + "end_line": 14 + }, + "description": "Resource 'aws_db_security_group.my-group' uses EC2 Classic. Use a VPC instead.", + "severity": "ERROR" + }, + { + "rule_id": "AZU003", + "link": "https://github.com/liamg/tfsec/wiki/AZU003", + "location": { + "filename": "/src/terraform/main.tf", + "start_line": 18, + "end_line": 18 + }, + "description": "Resource 'azurerm_managed_disk.source' defines an unencrypted managed disk.", + "severity": "ERROR" + } + ] +} +` diff --git a/internal/services/formatters/hcl/tfsec/entities/location.go b/internal/services/formatters/hcl/tfsec/location.go similarity index 97% rename from internal/services/formatters/hcl/tfsec/entities/location.go rename to internal/services/formatters/hcl/tfsec/location.go index a5c05800b..052dec481 100644 --- a/internal/services/formatters/hcl/tfsec/entities/location.go +++ b/internal/services/formatters/hcl/tfsec/location.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package entities +package tfsec type Location struct { Filename string `json:"filename"` diff --git a/internal/services/formatters/hcl/tfsec/entities/result.go b/internal/services/formatters/hcl/tfsec/result.go similarity index 79% rename from internal/services/formatters/hcl/tfsec/entities/result.go rename to internal/services/formatters/hcl/tfsec/result.go index 7ead059f4..f284a9c04 100644 --- a/internal/services/formatters/hcl/tfsec/entities/result.go +++ b/internal/services/formatters/hcl/tfsec/result.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package entities +package tfsec import ( "fmt" @@ -21,7 +21,7 @@ import ( "github.com/ZupIT/horusec-devkit/pkg/enums/severities" ) -type Result struct { +type tfsecResult struct { RuleID string `json:"rule_id"` Link string `json:"link"` Location Location `json:"location"` @@ -29,27 +29,27 @@ type Result struct { Severity string `json:"severity"` } -func (r *Result) GetDetails() string { +func (r *tfsecResult) getDetails() string { return r.RuleID + " -> [" + r.Description + "]" } -func (r *Result) GetStartLine() string { +func (r *tfsecResult) getStartLine() string { return strconv.Itoa(r.Location.StartLine) } -func (r *Result) GetCode() string { +func (r *tfsecResult) getCode() string { return fmt.Sprintf("code beetween line %d and %d.", r.Location.StartLine, r.Location.EndLine) } -func (r *Result) GetFilename() string { +func (r *tfsecResult) getFilename() string { return r.Location.Filename } -func (r *Result) GetSeverity() severities.Severity { +func (r *tfsecResult) getSeverity() severities.Severity { return r.mapSeverityValues()[r.Severity] } -func (r *Result) mapSeverityValues() map[string]severities.Severity { +func (r *tfsecResult) mapSeverityValues() map[string]severities.Severity { return map[string]severities.Severity{ "ERROR": severities.High, "WARNING": severities.Medium, diff --git a/internal/services/formatters/hcl/tfsec/entities/result_test.go b/internal/services/formatters/hcl/tfsec/result_test.go similarity index 82% rename from internal/services/formatters/hcl/tfsec/entities/result_test.go rename to internal/services/formatters/hcl/tfsec/result_test.go index cc4376e13..e157972af 100644 --- a/internal/services/formatters/hcl/tfsec/entities/result_test.go +++ b/internal/services/formatters/hcl/tfsec/result_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package entities +package tfsec import ( "testing" @@ -20,8 +20,8 @@ import ( "github.com/stretchr/testify/assert" ) -func resultMock() *Result { - return &Result{ +func resultMock() *tfsecResult { + return &tfsecResult{ RuleID: "AWS123", Link: "test", Location: Location{ @@ -36,24 +36,24 @@ func resultMock() *Result { func TestGetDetails(t *testing.T) { t.Run("should success get result details", func(t *testing.T) { - assert.NotEmpty(t, resultMock().GetDetails()) + assert.NotEmpty(t, resultMock().getDetails()) }) } func TestGetStartLine(t *testing.T) { t.Run("should success get start line", func(t *testing.T) { - assert.NotEmpty(t, resultMock().GetStartLine()) + assert.NotEmpty(t, resultMock().getStartLine()) }) } func TestGetCode(t *testing.T) { t.Run("should success get code", func(t *testing.T) { - assert.NotEmpty(t, resultMock().GetCode()) + assert.NotEmpty(t, resultMock().getCode()) }) } func TestGetFilename(t *testing.T) { t.Run("should success get filename", func(t *testing.T) { - assert.NotEmpty(t, resultMock().GetFilename()) + assert.NotEmpty(t, resultMock().getFilename()) }) } diff --git a/internal/services/formatters/hcl/tfsec/entities/vulnerabilities.go b/internal/services/formatters/hcl/tfsec/vulnerabilities.go similarity index 87% rename from internal/services/formatters/hcl/tfsec/entities/vulnerabilities.go rename to internal/services/formatters/hcl/tfsec/vulnerabilities.go index 66ab003fb..337ca4243 100644 --- a/internal/services/formatters/hcl/tfsec/entities/vulnerabilities.go +++ b/internal/services/formatters/hcl/tfsec/vulnerabilities.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -package entities +package tfsec -type Vulnerabilities struct { - Results []Result `json:"results"` +type tfsecVulnerabilities struct { + Results []tfsecResult `json:"results"` }