cli: fix breaking change on vulnerability hashes (#678)

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 and also add a workaround to users that is
already using the new hash as a false positive and risk accept.
To support the two ways of hashing the vulnerability a new field was
added on Vulnerability struct that represents the breaking way, so we
generate the two hashes of vulnerability and when we set the
vulnerability to false positive/risk accept according to config file we
use the two hashes to match.

Fixes #680

Signed-off-by: Matheus Alcantara <matheus.alcantara@zup.com.br>

go.mod

@@ -3,7 +3,7 @@ module github.com/ZupIT/horusec
 go 1.17
 
 require (
-	github.com/ZupIT/horusec-devkit v1.0.18
+	github.com/ZupIT/horusec-devkit v1.0.19
 	github.com/ZupIT/horusec-engine v0.3.6
 	github.com/bmatcuk/doublestar/v4 v4.0.2
 	github.com/briandowns/spinner v1.16.0

go.sum

@@ -92,8 +92,8 @@ github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWX
 github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
 github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g=
 github.com/ZupIT/horusec-devkit v1.0.17/go.mod h1:wTsXrXTD1YrChTQEng8EvVg+zL9nMUIQkhUG85sQwuQ=
-github.com/ZupIT/horusec-devkit v1.0.18 h1:Nbl7YQTBnwPFRmIEn0LKINN5u2SwvCu6Jt8wmlhHTO8=
-github.com/ZupIT/horusec-devkit v1.0.18/go.mod h1:8rEUFNoFOGeAIG1unUfaF5qP6agHPnf9WsMtGfQR/iU=
+github.com/ZupIT/horusec-devkit v1.0.19 h1:eQNzst0a91YkAzSWy+1A/brBR9Lon2dMmzs0vZ3dUzQ=
+github.com/ZupIT/horusec-devkit v1.0.19/go.mod h1:8rEUFNoFOGeAIG1unUfaF5qP6agHPnf9WsMtGfQR/iU=
 github.com/ZupIT/horusec-engine v0.3.6 h1:m/kL9K8+OVAaYjagoDmNFFDEA3BnyJbcx0DfNYGyaDM=
 github.com/ZupIT/horusec-engine v0.3.6/go.mod h1:s3SZQ9gXXlEcIagEuopZJga+Dw6RBFWMD7Rh5A+tIys=
 github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c=

internal/controllers/analyzer/analyzer.go

@@ -28,6 +28,7 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
 	enumsAnalysis "github.com/ZupIT/horusec-devkit/pkg/enums/analysis"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/confidence"
 	"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
@@ -443,11 +444,14 @@ func (a *Analyzer) logProjectSubPath(language languages.Language, subPath string
 	}
 }
 
+// nolint:gocyclo
 func (a *Analyzer) checkIfNoExistHashAndLog(list []string) {
 	for _, hash := range list {
 		existing := false
-		for keyAv := range a.analysis.AnalysisVulnerabilities {
-			if hash == a.analysis.AnalysisVulnerabilities[keyAv].Vulnerability.VulnHash {
+		for idx := range a.analysis.AnalysisVulnerabilities {
+			vulnHash := a.analysis.AnalysisVulnerabilities[idx].Vulnerability.VulnHash
+			vulnHashInvalid := a.analysis.AnalysisVulnerabilities[idx].Vulnerability.VulnHashInvalid
+			if hash == vulnHash || hash == vulnHashInvalid {
 				existing = true
 				break
 			}
@@ -484,21 +488,30 @@ func (a *Analyzer) getCustomOrDefaultImage(language languages.Language) string {
 	return fmt.Sprintf("%s/%s", images.DefaultRegistry, images.MapValues()[language])
 }
 
-func (a *Analyzer) SetFalsePositivesAndRiskAcceptInVulnerabilities(
-	listFalsePositive, listRiskAccept []string) *analysis.Analysis {
-	for key := range a.analysis.AnalysisVulnerabilities {
-		a.setVulnerabilityType(key, listFalsePositive, enumsVulnerability.FalsePositive)
-		a.setVulnerabilityType(key, listRiskAccept, enumsVulnerability.RiskAccepted)
+// SetFalsePositivesAndRiskAcceptInVulnerabilities set analysis vulnerabiltieis to false
+// positive or risk accept if the hash exists on falsePositive and riskAccept params.
+//
+// nolint:lll
+func (a *Analyzer) SetFalsePositivesAndRiskAcceptInVulnerabilities(falsePositive, riskAccept []string) *analysis.Analysis {
+	for idx := range a.analysis.AnalysisVulnerabilities {
+		a.setVulnerabilityType(
+			&a.analysis.AnalysisVulnerabilities[idx].Vulnerability, falsePositive, enumsVulnerability.FalsePositive,
+		)
+		a.setVulnerabilityType(
+			&a.analysis.AnalysisVulnerabilities[idx].Vulnerability, riskAccept, enumsVulnerability.RiskAccepted,
+		)
 	}
 	return a.analysis
 }
 
-func (a *Analyzer) setVulnerabilityType(keyAnalysisVulnerabilities int,
-	listToCheck []string, vulnerabilityType enumsVulnerability.Type) {
-	currentHash := a.analysis.AnalysisVulnerabilities[keyAnalysisVulnerabilities].Vulnerability.VulnHash
-	for _, flagVulnerabilityHash := range listToCheck {
-		if flagVulnerabilityHash != "" && strings.TrimSpace(currentHash) == strings.TrimSpace(flagVulnerabilityHash) {
-			a.analysis.AnalysisVulnerabilities[keyAnalysisVulnerabilities].Vulnerability.Type = vulnerabilityType
+func (a *Analyzer) setVulnerabilityType(
+	vuln *vulnerability.Vulnerability, hashes []string, vulnType enumsVulnerability.Type,
+) {
+	for _, hash := range hashes {
+		hash = strings.TrimSpace(hash)
+		// See vulnerability.Vulnerability.VulnHashInvalid docs for more info.
+		if hash != "" && (strings.TrimSpace(vuln.VulnHash) == hash || strings.TrimSpace(vuln.VulnHashInvalid) == hash) {
+			vuln.Type = vulnType
 		}
 	}
 }

internal/controllers/analyzer/analyzer_test.go

@@ -17,15 +17,20 @@ package analyzer
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
 	entitiesAnalysis "github.com/ZupIT/horusec-devkit/pkg/entities/analysis"
+	"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
+	vulnerabilityenum "github.com/ZupIT/horusec-devkit/pkg/enums/vulnerability"
 	"github.com/ZupIT/horusec-devkit/pkg/utils/logger"
 	horusecAPI "github.com/ZupIT/horusec/internal/services/horusec_api"
 	"github.com/ZupIT/horusec/internal/utils/mock"
+	vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"
 
 	"github.com/ZupIT/horusec/internal/entities/workdir"
 
@@ -65,6 +70,84 @@ func BenchmarkAnalyzerAnalyze(b *testing.B) {
 	}
 }
 
+func TestAnalyzerSetFalsePositivesAndRiskAcceptInVulnerabilities(t *testing.T) {
+	vuln := vulnerability.Vulnerability{
+		RuleID:  "HS-TEST-1",
+		Line:    "10",
+		Column:  "20",
+		File:    "testing",
+		Code:    "testing",
+		Details: fmt.Sprintf("Test\nDescription testing"),
+	}
+	vulnhash.Bind(&vuln)
+
+	testcases := []struct {
+		name          string
+		vulnerability vulnerability.Vulnerability
+		hashes        []string
+		expectedType  vulnerabilityenum.Type
+	}{
+		{
+			name:          "ChangeCorrectHashToFalsePositive",
+			vulnerability: vuln,
+			hashes:        []string{vuln.VulnHash},
+			expectedType:  vulnerabilityenum.FalsePositive,
+		},
+		{
+			name:          "ChangeBreakingHashToFalsePositive",
+			vulnerability: vuln,
+			hashes:        []string{vuln.VulnHashInvalid},
+			expectedType:  vulnerabilityenum.FalsePositive,
+		},
+		{
+			name:          "ChangeCorrectHashToRiskAccept",
+			vulnerability: vuln,
+			hashes:        []string{vuln.VulnHash},
+			expectedType:  vulnerabilityenum.RiskAccepted,
+		},
+		{
+			name:          "ChangeBreakingHashToRiskAccept",
+			vulnerability: vuln,
+			hashes:        []string{vuln.VulnHashInvalid},
+			expectedType:  vulnerabilityenum.RiskAccepted,
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			analyzer := NewAnalyzer(config.New())
+
+			analyzer.analysis.AnalysisVulnerabilities = append(
+				analyzer.analysis.AnalysisVulnerabilities, analysis.AnalysisVulnerabilities{
+					AnalysisID:    uuid.New(),
+					Vulnerability: tt.vulnerability,
+				},
+			)
+			var (
+				falsePositiveHashes []string
+				riskAcceptHashes    []string
+			)
+
+			switch tt.expectedType {
+			case vulnerabilityenum.FalsePositive:
+				falsePositiveHashes = tt.hashes
+			case vulnerabilityenum.RiskAccepted:
+				riskAcceptHashes = tt.hashes
+			default:
+				t.Fatalf("invalid type %s", tt.expectedType)
+			}
+
+			analyzer.SetFalsePositivesAndRiskAcceptInVulnerabilities(falsePositiveHashes, riskAcceptHashes)
+
+			require.Len(t, analyzer.analysis.AnalysisVulnerabilities, len(tt.hashes))
+			for _, vuln := range analyzer.analysis.AnalysisVulnerabilities {
+				assert.Equal(t, tt.expectedType, vuln.Vulnerability.Type)
+			}
+
+		})
+	}
+}
+
 func TestNewAnalyzer(t *testing.T) {
 	t.Run("Should return type os struct correctly", func(t *testing.T) {
 		assert.IsType(t, &Analyzer{}, NewAnalyzer(&config.Config{}))

internal/controllers/printresults/print_results.go

@@ -277,6 +277,9 @@ func (pr *PrintResults) printTextOutputVulnerabilityData(vulnerability *vulnerab
 	pr.printLNF("Confidence: %s", vulnerability.Confidence)
 	pr.printLNF("File: %s", pr.getProjectPath(vulnerability.File))
 	pr.printLNF("Code: %s", vulnerability.Code)
+	if vulnerability.RuleID != "" {
+		pr.printLNF("RuleID: %s", vulnerability.RuleID)
+	}
 	pr.printLNF("Details: %s", vulnerability.Details)
 	pr.printLNF("Type: %s", vulnerability.Type)
 

internal/services/formatters/service.go

@@ -237,12 +237,13 @@ func (s *Service) AddNewVulnerabilityIntoAnalysis(vuln *vulnerability.Vulnerabil
 func (s *Service) newVulnerabilityFromFinding(finding *engine.Finding, tool tools.Tool,
 	language languages.Language) *vulnerability.Vulnerability {
 	return &vulnerability.Vulnerability{
+		RuleID:       finding.ID,
 		Line:         strconv.Itoa(finding.SourceLocation.Line),
 		Column:       strconv.Itoa(finding.SourceLocation.Column),
 		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),