Skip to content

Commit

Permalink
cli: fix breaking change on vulnerability hashes
Browse files Browse the repository at this point in the history 
On pr #636 we add the rule id on description of vulnerability, but the
Details of vulnerability is used to generate the vulnerability hash, so
adding the rule id on details generate a different hash which cause a
breaking change. So this commit remove the rule id prefix from Details
field of Vulnerability.

Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>
  • Loading branch information
@matheusalcantarazup
matheusalcantarazup committed Oct 14, 2021
1 parent 28ea3ed commit 046c8d3
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 29 deletions.
2 changes: 1 addition & 1 deletion internal/services/formatters/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (s *Service) newVulnerabilityFromFinding(finding *engine.Finding, tool tool
Confidence: confidence.Confidence(finding.Confidence),
File: s.removeHorusecFolder(finding.SourceLocation.Filename),
Code: s.GetCodeWithMaxCharacters(finding.CodeSample, finding.SourceLocation.Column),
Details: fmt.Sprintf("%s: %s\n%s", finding.ID, finding.Name, finding.Description),
Details: fmt.Sprintf("%s\n%s", finding.Name, finding.Description),
SecurityTool: tool,
Language: language,
Severity: severities.GetSeverityByString(finding.Severity),
Expand Down
113 changes: 85 additions & 28 deletions internal/services/formatters/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,83 @@ package formatters

import (
"errors"
"fmt"
"strconv"
"testing"

"github.com/ZupIT/horusec-devkit/pkg/enums/confidence"
"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
engine "github.com/ZupIT/horusec-engine"

commitauthor "github.com/ZupIT/horusec/internal/entities/commit_author"
"github.com/ZupIT/horusec/internal/entities/toolsconfig"

entitiesAnalysis "github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
commitAuthor "github.com/ZupIT/horusec/internal/entities/commit_author"
"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
"github.com/ZupIT/horusec/config"
dockerEntities "github.com/ZupIT/horusec/internal/entities/docker"
dockerentities "github.com/ZupIT/horusec/internal/entities/docker"
"github.com/ZupIT/horusec/internal/entities/workdir"
"github.com/ZupIT/horusec/internal/services/docker"
"github.com/ZupIT/horusec/internal/services/engines/java"
)

func TestParseFindingsToVulnerabilities(t *testing.T) {
analysis := new(analysis.Analysis)
svc := NewFormatterService(analysis, &docker.Mock{}, config.New())

rule := java.NewAWSQueryInjection()
findings := []engine.Finding{
{
ID: rule.ID,
Name: rule.Name,
Severity: rule.Severity,
CodeSample: "testing",
Confidence: rule.Confidence,
Description: rule.Description,
SourceLocation: engine.Location{
Filename: "Test.java",
Line: 10,
Column: 20,
},
},
}
svc.ParseFindingsToVulnerabilities(findings, tools.HorusecEngine, languages.Java)

expectedVulnerabilities := []vulnerability.Vulnerability{
{
Line: "10",
Column: "20",
Confidence: confidence.Confidence(rule.Confidence),
File: "Test.java",
Code: "testing",
Details: fmt.Sprintf("%s\n%s", rule.Name, rule.Description),
SecurityTool: tools.HorusecEngine,
Language: languages.Java,
Severity: severities.GetSeverityByString(rule.Severity),
CommitAuthor: "-",
CommitDate: "-",
CommitEmail: "-",
CommitHash: "-",
CommitMessage: "-",
// NOTE: We hard coded vulnerability hash here to assert that we are not breaking existing hashes
VulnHash: "b9ffd6959275a840254c9ddc9ab0cc5edd6f7950f1b71103d772ac5a17ca988d",
},
}

require.Len(t, analysis.AnalysisVulnerabilities, len(expectedVulnerabilities))
for idx := range expectedVulnerabilities {
assert.Equal(t, expectedVulnerabilities[idx], analysis.AnalysisVulnerabilities[idx].Vulnerability)

}
}

func TestMock_AddWorkDirInCmd(t *testing.T) {
mock := &Mock{}
t.Run("Should mock with success", func(t *testing.T) {
Expand All @@ -44,18 +101,18 @@ func TestMock_AddWorkDirInCmd(t *testing.T) {
mock.On("SetAnalysisError").Return()
mock.On("ExecuteContainer").Return("", nil)
mock.On("GetAnalysisIDErrorMessage").Return("")
mock.On("GetCommitAuthor").Return(commitAuthor.CommitAuthor{})
mock.On("GetCommitAuthor").Return(commitauthor.CommitAuthor{})
mock.On("AddWorkDirInCmd").Return("")
mock.On("GetConfigProjectPath").Return("")
mock.On("GetAnalysis").Return(&entitiesAnalysis.Analysis{})
mock.On("GetAnalysis").Return(&analysis.Analysis{})
mock.On("SetToolFinishedAnalysis").Return()
mock.On("LogAnalysisError").Return()
mock.On("SetMonitor").Return()
mock.On("RemoveSrcFolderFromPath").Return("")
mock.On("GetCodeWithMaxCharacters").Return("")
mock.LogDebugWithReplace("", "", "")
_ = mock.GetAnalysisID()
_, _ = mock.ExecuteContainer(&dockerEntities.AnalysisData{})
_, _ = mock.ExecuteContainer(&dockerentities.AnalysisData{})
_ = mock.GetAnalysisIDErrorMessage("", "")
_ = mock.GetCommitAuthor("", "")
_ = mock.AddWorkDirInCmd("", "", "")
Expand All @@ -68,14 +125,14 @@ func TestMock_AddWorkDirInCmd(t *testing.T) {

func TestExecuteContainer(t *testing.T) {
t.Run("should return no error when success set is finished", func(t *testing.T) {
analysis := &entitiesAnalysis.Analysis{}
analysis := &analysis.Analysis{}

dockerAPIControllerMock := &docker.Mock{}
dockerAPIControllerMock.On("SetAnalysisID")
dockerAPIControllerMock.On("CreateLanguageAnalysisContainer").Return("test", nil)

monitorController := NewFormatterService(analysis, dockerAPIControllerMock, &config.Config{})
result, err := monitorController.ExecuteContainer(&dockerEntities.AnalysisData{})
result, err := monitorController.ExecuteContainer(&dockerentities.AnalysisData{})

assert.NoError(t, err)
assert.Equal(t, "test", result)
Expand All @@ -84,7 +141,7 @@ func TestExecuteContainer(t *testing.T) {

func TestGetAnalysisIDErrorMessage(t *testing.T) {
t.Run("should success get error message with replaces", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})

result := monitorController.GetAnalysisIDErrorMessage(tools.Bandit, "test")

Expand All @@ -96,7 +153,7 @@ func TestGetAnalysisIDErrorMessage(t *testing.T) {

func TestGetCommitAuthor(t *testing.T) {
t.Run("should get commit author", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})

result := monitorController.GetCommitAuthor("", "")

Expand All @@ -109,7 +166,7 @@ func TestGetConfigProjectPath(t *testing.T) {
cliConfig := &config.Config{}
cliConfig.ProjectPath = "test"

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, cliConfig)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, cliConfig)

result := monitorController.GetConfigProjectPath()

Expand All @@ -125,7 +182,7 @@ func TestAddWorkDirInCmd(t *testing.T) {
CSharp: []string{"test"},
}

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, cliConfig)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, cliConfig)

result := monitorController.AddWorkDirInCmd("test", "C#", tools.SecurityCodeScan)

Expand All @@ -138,7 +195,7 @@ func TestAddWorkDirInCmd(t *testing.T) {
CSharp: []string{"test"},
}

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, cliConfig)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, cliConfig)

result := monitorController.AddWorkDirInCmd("test", "C#", tools.SecurityCodeScan)

Expand All @@ -148,7 +205,7 @@ func TestAddWorkDirInCmd(t *testing.T) {

func TestLogDebugWithReplace(t *testing.T) {
t.Run("should log debug and not panics", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})

assert.NotPanics(t, func() {
monitorController.LogDebugWithReplace("test", tools.NpmAudit, languages.Javascript)
Expand All @@ -159,21 +216,21 @@ func TestLogDebugWithReplace(t *testing.T) {
func TestGetAnalysisID(t *testing.T) {
t.Run("should success get analysis id", func(t *testing.T) {
id := uuid.New()
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{ID: id}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{ID: id}, &docker.Mock{}, &config.Config{})
assert.Equal(t, id.String(), monitorController.GetAnalysisID())
})
}

func TestLogAnalysisError(t *testing.T) {
t.Run("should not panic when logging error", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})

assert.NotPanics(t, func() {
monitorController.SetAnalysisError(errors.New("test"), tools.GoSec, "")
})
})
t.Run("should not panic when logging error and exists projectSubPath", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})

assert.NotPanics(t, func() {
monitorController.SetAnalysisError(errors.New("test"), tools.GoSec, "/tmp")
Expand All @@ -188,7 +245,7 @@ func TestToolIsToIgnore(t *testing.T) {
toolsconfig.ToolsConfigsStruct{GoSec: toolsconfig.ToolConfig{IsToIgnore: true}},
)

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, configs)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, configs)

assert.Equal(t, true, monitorController.ToolIsToIgnore(tools.GoSec))
})
Expand All @@ -198,7 +255,7 @@ func TestToolIsToIgnore(t *testing.T) {
toolsconfig.ToolsConfigsStruct{GoSec: toolsconfig.ToolConfig{IsToIgnore: true}},
)

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, configs)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, configs)

assert.Equal(t, true, monitorController.ToolIsToIgnore(tools.GoSec))
})
Expand All @@ -211,7 +268,7 @@ func TestToolIsToIgnore(t *testing.T) {
},
)

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, configs)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, configs)

assert.Equal(t, true, monitorController.ToolIsToIgnore(tools.GoSec))
})
Expand All @@ -221,29 +278,29 @@ func TestToolIsToIgnore(t *testing.T) {
toolsconfig.ToolsConfigsStruct{SecurityCodeScan: toolsconfig.ToolConfig{IsToIgnore: true}},
)

monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, configs)
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, configs)

assert.Equal(t, false, monitorController.ToolIsToIgnore(tools.GoSec))
})
}

func TestService_GetCodeWithMaxCharacters(t *testing.T) {
t.Run("should return default code", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
column := 0
newCode := monitorController.GetCodeWithMaxCharacters(code, column)
assert.Equal(t, "text", newCode)
})
t.Run("should return default code if column is negative", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
column := -1
newCode := monitorController.GetCodeWithMaxCharacters(code, column)
assert.Equal(t, "text", newCode)
})
t.Run("should return 4:105 characters when text is so bigger", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
for i := 0; i < 10; i++ {
for i := 0; i <= 9; i++ {
Expand All @@ -255,7 +312,7 @@ func TestService_GetCodeWithMaxCharacters(t *testing.T) {
assert.Equal(t, "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789", newCode)
})
t.Run("should return first 100 characters when text is so bigger", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
for i := 0; i < 10; i++ {
for i := 0; i <= 9; i++ {
Expand All @@ -267,7 +324,7 @@ func TestService_GetCodeWithMaxCharacters(t *testing.T) {
assert.Equal(t, "text012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345", newCode)
})
t.Run("should return first 100 characters when text contains breaking lines", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := `22: func GetMD5(s string) string {
23: h := md5.New()
24: io.WriteString(h, s) // #nohorus
Expand All @@ -280,7 +337,7 @@ func TestService_GetCodeWithMaxCharacters(t *testing.T) {
`, newCode)
})
t.Run("should return first 100 characters when text is so bigger", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
for i := 0; i <= 200; i++ {
code += strconv.Itoa(i)
Expand All @@ -290,7 +347,7 @@ func TestService_GetCodeWithMaxCharacters(t *testing.T) {
assert.Equal(t, "4041424344454647484950515253545556575859606162636465666768697071727374757677787980818283848586878889", newCode)
})
t.Run("should return first 100 characters when text is so bigger", func(t *testing.T) {
monitorController := NewFormatterService(&entitiesAnalysis.Analysis{}, &docker.Mock{}, &config.Config{})
monitorController := NewFormatterService(&analysis.Analysis{}, &docker.Mock{}, &config.Config{})
code := "text"
for i := 0; i <= 200; i++ {
code += strconv.Itoa(i)
Expand Down

0 comments on commit 046c8d3

Please sign in to comment.