Merge pull request #1184 from HollowMan6/sarif-fix

feat(sarif): add fix object in generated reports

core/pkg/fixhandler/fixhandler.go

@@ -201,14 +201,14 @@ func (h *FixHandler) ApplyChanges(ctx context.Context, resourcesToFix []Resource
 	fileYamlExpressions := h.getFileYamlExpressions(resourcesToFix)
 
 	for filepath, yamlExpression := range fileYamlExpressions {
-		fileAsString, err := getFileString(filepath)
+		fileAsString, err := GetFileString(filepath)
 
 		if err != nil {
 			errors = append(errors, err)
 			continue
 		}
 
-		fixedYamlString, err := h.ApplyFixToContent(ctx, fileAsString, yamlExpression)
+		fixedYamlString, err := ApplyFixToContent(ctx, fileAsString, yamlExpression)
 
 		if err != nil {
 			errors = append(errors, fmt.Errorf("Failed to fix file %s: %w ", filepath, err))
@@ -242,7 +242,7 @@ func (h *FixHandler) getFilePathAndIndex(filePathWithIndex string) (filePath str
 	}
 }
 
-func (h *FixHandler) ApplyFixToContent(ctx context.Context, yamlAsString, yamlExpression string) (fixedString string, err error) {
+func ApplyFixToContent(ctx context.Context, yamlAsString, yamlExpression string) (fixedString string, err error) {
 	newline := determineNewlineSeparator(yamlAsString)
 
 	yamlLines := strings.Split(yamlAsString, newline)
@@ -301,7 +301,7 @@ func (rfi *ResourceFixInfo) addYamlExpressionsFromResourceAssociatedControl(docu
 				continue
 			}
 
-			yamlExpression := fixPathToValidYamlExpression(rulePaths.FixPath.Path, rulePaths.FixPath.Value, documentIndex)
+			yamlExpression := FixPathToValidYamlExpression(rulePaths.FixPath.Path, rulePaths.FixPath.Value, documentIndex)
 			rfi.YamlExpressions[yamlExpression] = rulePaths.FixPath
 		}
 	}
@@ -317,7 +317,7 @@ func reduceYamlExpressions(resource *ResourceFixInfo) string {
 	return strings.Join(expressions, " | ")
 }
 
-func fixPathToValidYamlExpression(fixPath, value string, documentIndexInYaml int) string {
+func FixPathToValidYamlExpression(fixPath, value string, documentIndexInYaml int) string {
 	isStringValue := true
 	if _, err := strconv.ParseBool(value); err == nil {
 		isStringValue = false
@@ -340,7 +340,7 @@ func joinStrings(inputStrings ...string) string {
 	return strings.Join(inputStrings, "")
 }
 
-func getFileString(filepath string) (string, error) {
+func GetFileString(filepath string) (string, error) {
 	bytes, err := os.ReadFile(filepath)
 
 	if err != nil {

core/pkg/fixhandler/fixhandler_test.go

@@ -182,9 +182,7 @@ func TestApplyFixKeepsFormatting(t *testing.T) {
 			want := string(wantRaw)
 			expression := tc.yamlExpression
 
-			h, _ := NewFixHandlerMock()
-
-			got, _ := h.ApplyFixToContent(context.TODO(), string(input), expression)
+			got, _ := ApplyFixToContent(context.TODO(), string(input), expression)
 
 			assert.Equalf(
 				t, want, got,
@@ -241,7 +239,7 @@ func Test_fixPathToValidYamlExpression(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := fixPathToValidYamlExpression(tt.args.fixPath, tt.args.value, tt.args.documentIndexInYaml); got != tt.want {
+			if got := FixPathToValidYamlExpression(tt.args.fixPath, tt.args.value, tt.args.documentIndexInYaml); got != tt.want {
 				t.Errorf("fixPathToValidYamlExpression() = %v, want %v", got, tt.want)
 			}
 		})

core/pkg/resultshandling/printer/v2/sarifprinter.go

@@ -3,6 +3,7 @@ package printer
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"os"
 	"path"
 	"path/filepath"
@@ -12,6 +13,7 @@ import (
 	logger "github.com/kubescape/go-logger"
 	"github.com/kubescape/go-logger/helpers"
 	"github.com/kubescape/kubescape/v2/core/cautils"
+	"github.com/kubescape/kubescape/v2/core/pkg/fixhandler"
 	"github.com/kubescape/kubescape/v2/core/pkg/resultshandling/locationresolver"
 	"github.com/kubescape/kubescape/v2/core/pkg/resultshandling/printer"
 	"github.com/kubescape/opa-utils/objectsenvelopes/localworkload"
@@ -19,6 +21,7 @@ import (
 	"github.com/kubescape/opa-utils/reporthandling/results/v1/resourcesresults"
 	v2 "github.com/kubescape/opa-utils/reporthandling/v2"
 	"github.com/owenrumney/go-sarif/v2/sarif"
+	"github.com/sergi/go-diff/diffmatchpatch"
 )
 
 const (
@@ -91,10 +94,10 @@ func (sp *SARIFPrinter) addRule(scanRun *sarif.Run, control reportsummary.IContr
 }
 
 // addResult adds a result of checking a rule to the scan run based on the given control summary
-func (sp *SARIFPrinter) addResult(scanRun *sarif.Run, ctl reportsummary.IControlSummary, filepath string, location locationresolver.Location) {
-	scanRun.CreateResultForRule(ctl.GetID()).
+func (sp *SARIFPrinter) addResult(scanRun *sarif.Run, ctl reportsummary.IControlSummary, filepath string, location locationresolver.Location) *sarif.Result {
+	return scanRun.CreateResultForRule(ctl.GetID()).
 		WithMessage(sarif.NewTextMessage(ctl.GetDescription())).
-		AddLocation(
+		WithLocations([]*sarif.Location{
 			sarif.NewLocationWithPhysicalLocation(
 				sarif.NewPhysicalLocation().
 					WithArtifactLocation(
@@ -103,10 +106,10 @@ func (sp *SARIFPrinter) addResult(scanRun *sarif.Run, ctl reportsummary.IControl
 					sarif.NewRegion().WithStartLine(location.Line).WithStartColumn(location.Column),
 				),
 			),
-		)
+		})
 }
 
-func (sp *SARIFPrinter) ActionPrint(_ context.Context, opaSessionObj *cautils.OPASessionObj) {
+func (sp *SARIFPrinter) ActionPrint(ctx context.Context, opaSessionObj *cautils.OPASessionObj) {
 	report, err := sarif.New(sarif.Version210)
 	if err != nil {
 		panic(err)
@@ -139,7 +142,8 @@ func (sp *SARIFPrinter) ActionPrint(_ context.Context, opaSessionObj *cautils.OP
 					location := sp.resolveFixLocation(opaSessionObj, locationResolver, &ac, resourceID)
 
 					sp.addRule(run, ctl)
-					sp.addResult(run, ctl, filepath, location)
+					result := sp.addResult(run, ctl, filepath, location)
+					collectFixes(ctx, result, ac, opaSessionObj, resourceID, filepath)
 				}
 			}
 		}
@@ -172,18 +176,12 @@ func (sp *SARIFPrinter) resolveFixLocation(opaSessionObj *cautils.OPASessionObj,
 		return defaultLocation
 	}
 
-	resource := opaSessionObj.AllResources[resourceID]
-	localworkload, ok := resource.(*localworkload.LocalWorkload)
+	docIndex, ok := getDocIndex(opaSessionObj, resourceID)
+
 	if !ok {
 		return defaultLocation
 	}
 
-	splittedPath := strings.Split(localworkload.GetPath(), ":")
-	if len(splittedPath) <= 1 {
-		return defaultLocation
-	}
-
-	docIndex, _ := strconv.Atoi(splittedPath[1])
 	location, _ = locationResolver.ResolveLocation(fixPath, docIndex)
 	if location.Line == 0 {
 		return defaultLocation
@@ -192,6 +190,142 @@ func (sp *SARIFPrinter) resolveFixLocation(opaSessionObj *cautils.OPASessionObj,
 	return location
 }
 
+func addFix(result *sarif.Result, filepath string, startLine int, startColumn int, endLine int, endColumn int, text string) {
+	result.AddFix(
+		sarif.NewFix().
+			WithArtifactChanges([]*sarif.ArtifactChange{
+				sarif.NewArtifactChange(
+					sarif.NewSimpleArtifactLocation(filepath),
+				).WithReplacement(
+					sarif.NewReplacement(sarif.NewRegion().
+						WithStartLine(startLine).
+						WithStartColumn(startColumn).
+						WithEndLine(endLine).
+						WithEndColumn(endColumn),
+					).WithInsertedContent(
+						sarif.NewArtifactContent().WithText(text),
+					),
+				),
+			}),
+	)
+}
+
+func calculateMove(str string, file []string, endColumn int, endLine int) (int, int, bool) {
+	num, err := strconv.Atoi(str)
+	if err != nil {
+		logger.L().Debug("failed to get move from string "+str, helpers.Error(err))
+		return 0, 0, false
+	}
+	for num+endColumn-1 > len(file[endLine-1]) {
+		num -= len(file[endLine-1]) - endColumn + 2
+		endLine++
+		endColumn = 1
+	}
+	endColumn += num
+	return endLine, endColumn, true
+}
+
+func collectDiffs(dmp *diffmatchpatch.DiffMatchPatch, diffs []diffmatchpatch.Diff, result *sarif.Result, filepath string, fileAsString string) {
+	file := strings.Split(fileAsString, "\n")
+	text := ""
+	startLine := 1
+	startColumn := 1
+	endLine := 1
+	endColumn := 1
+
+	delta := strings.Split(dmp.DiffToDelta(diffs), "\t")
+	for index, seg := range delta {
+		switch seg[0] {
+		case '+':
+			var err error
+			text, err = url.QueryUnescape(seg[1:])
+			if err != nil {
+				logger.L().Debug("failed to unescape string", helpers.Error(err))
+				continue
+			}
+			if index >= len(delta)-1 || delta[index+1][0] == '=' {
+				addFix(result, filepath, startLine, startColumn, endLine, endColumn, text)
+			}
+		case '-':
+			var ok bool
+			endLine, endColumn, ok = calculateMove(seg[1:], file, endColumn, endLine)
+			if !ok {
+				continue
+			}
+			if index >= len(delta)-1 || delta[index+1][0] == '=' {
+				addFix(result, filepath, startLine, startColumn, endLine, endColumn, text)
+			}
+		case '=':
+			var ok bool
+			endLine, endColumn, ok = calculateMove(seg[1:], file, endColumn, endLine)
+			if !ok {
+				continue
+			}
+			startLine = endLine
+			startColumn = endColumn
+			text = ""
+		}
+	}
+}
+
+func collectFixes(ctx context.Context, result *sarif.Result, ac resourcesresults.ResourceAssociatedControl, opaSessionObj *cautils.OPASessionObj, resourceID string, filepath string) {
+	for _, rule := range ac.ResourceAssociatedRules {
+		if !rule.GetStatus(nil).IsFailed() {
+			continue
+		}
+
+		for _, rulePaths := range rule.Paths {
+			if rulePaths.FixPath.Path == "" {
+				continue
+			}
+			// if strings.HasPrefix(rulePaths.FixPath.Value, fixhandler.UserValuePrefix) {
+			// 	continue
+			// }
+
+			documentIndex, ok := getDocIndex(opaSessionObj, resourceID)
+			if !ok {
+				continue
+			}
+
+			yamlExpression := fixhandler.FixPathToValidYamlExpression(rulePaths.FixPath.Path, rulePaths.FixPath.Value, documentIndex)
+			fileAsString, err := fixhandler.GetFileString(filepath)
+			if err != nil {
+				logger.L().Debug("failed to access "+filepath, helpers.Error(err))
+				continue
+			}
+
+			fixedYamlString, err := fixhandler.ApplyFixToContent(ctx, fileAsString, yamlExpression)
+			if err != nil {
+				logger.L().Debug("failed to fix "+filepath+" with "+yamlExpression, helpers.Error(err))
+				continue
+			}
+
+			dmp := diffmatchpatch.New()
+			diffs := dmp.DiffMain(fileAsString, fixedYamlString, false)
+			collectDiffs(dmp, diffs, result, filepath, fileAsString)
+		}
+	}
+}
+
+func getDocIndex(opaSessionObj *cautils.OPASessionObj, resourceID string) (int, bool) {
+	resource := opaSessionObj.AllResources[resourceID]
+	localworkload, ok := resource.(*localworkload.LocalWorkload)
+	if !ok {
+		return 0, false
+	}
+
+	splittedPath := strings.Split(localworkload.GetPath(), ":")
+	if len(splittedPath) <= 1 {
+		return 0, false
+	}
+
+	docIndex, err := strconv.Atoi(splittedPath[1])
+	if err != nil {
+		return 0, false
+	}
+	return docIndex, true
+}
+
 func getBasePathFromMetadata(opaSessionObj cautils.OPASessionObj) string {
 	if opaSessionObj.Metadata.ScanMetadata.ScanningTarget == v2.GitLocal {
 		return opaSessionObj.Metadata.ContextMetadata.RepoContextMetadata.LocalRootPath

core/pkg/resultshandling/printer/v2/sarifprinter_test.go

@@ -1,6 +1,11 @@
 package printer
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/owenrumney/go-sarif/v2/sarif"
+	"github.com/sergi/go-diff/diffmatchpatch"
+)
 
 func Test_scoreToSeverityLevel(t *testing.T) {
 	tc := []struct {
@@ -25,3 +30,94 @@ func Test_scoreToSeverityLevel(t *testing.T) {
 		})
 	}
 }
+
+func Test_collectDiffs(t *testing.T) {
+	tc := []struct {
+		Name        string
+		fileString  string
+		fixedString string
+		fixesNum    int
+		region      [][4]int
+		text        []string
+	}{
+		{
+			"Collect diffs should work for fix object in sarif",
+
+			`apiVersion: v1
+kind: Pod
+metadata:
+  name: test
+
+spec:
+  containers:
+  - name: nginx_container
+    image: nginx
+    securityContext:
+      capabilities:
+        drop: [NET_RAW]
+      runAsRoot: true`,
+
+			`apiVersion: v1
+kind: Pod
+metadata:
+  name: test
+
+spec:
+  containers:
+  - name: nginx_container
+    image: nginx
+    securityContext:
+      capabilities:
+        drop: [NET_RAW, SYS_ADM]
+      runAsRoot: false
+      allowPrivilegeEscalation: false`,
+			3,
+			[][4]int{
+				{12, 23, 12, 23},
+				{13, 18, 13, 19},
+				{13, 20, 13, 21},
+			},
+			[]string{
+				", SYS_ADM",
+				`false
+      allowP`,
+				"ivilegeEscalation: fals",
+			},
+		},
+	}
+
+	for _, testCase := range tc {
+		t.Run(testCase.Name, func(t *testing.T) {
+			dmp := diffmatchpatch.New()
+			diffs := dmp.DiffMain(testCase.fileString, testCase.fixedString, false)
+			run := sarif.NewRunWithInformationURI(toolName, toolInfoURI)
+			result := run.CreateResultForRule("0")
+			collectDiffs(dmp, diffs, result, "", testCase.fileString)
+			if len(result.Fixes) != testCase.fixesNum {
+				t.Errorf("wrong number of fixes, got %d, want %d", len(result.Fixes), testCase.fixesNum)
+			}
+			for index, fix := range result.Fixes {
+				if len(fix.ArtifactChanges) != 1 {
+					t.Errorf("wrong number of artifactChanges in fix %d, got %d, want %d", index, len(fix.ArtifactChanges), 1)
+				}
+				replacements := fix.ArtifactChanges[0].Replacements
+				if len(replacements) != 1 {
+					t.Errorf("wrong number of replacements in fix %d, got %d, want %d", index, len(replacements), 1)
+				}
+				startLine := *replacements[0].DeletedRegion.StartLine
+				startColumn := *replacements[0].DeletedRegion.StartColumn
+				endLine := *replacements[0].DeletedRegion.EndLine
+				endColumn := *replacements[0].DeletedRegion.EndColumn
+				location := testCase.region[index]
+				if location[0] != startLine || location[1] != startColumn || location[2] != endLine || location[3] != endColumn {
+					t.Errorf("wrong deleted region in fix %d, got (%d, %d, %d, %d), want (%d, %d, %d, %d)",
+						index, startLine, startColumn, endLine, endColumn, location[0], location[1], location[2], location[3])
+				}
+				if testCase.text[index] != *replacements[0].InsertedContent.Text {
+					t.Errorf("wrong inserted text in fix %d, got (%s), want (%s)",
+						index, *replacements[0].InsertedContent.Text, testCase.text[index])
+				}
+			}
+		})
+	}
+}