Repository: Checkmarx/kics (language: python)

Issue:
Mask secrets in code snippets even with --disable-secrets
Currently, when a scan is set up to detect both vulnerabilities and secrets, if a vulnerability code snippet includes a secret, the secret will be masked:
```
Cluster Master Authentication Disabled, Severity: HIGH, Results: 1
...

	[1]: ../../path/myconfig.tf:7

		005:   initial_node_count = 3
		006:
		007:   master_auth {
		008:     username = ""
		009:     password = <SECRET-MASKED-ON-PURPOSE>
```

However, if the --disable-secrets flag is used, the secrets in the vulnerability code snippet are displayed in plain text:
```
Cluster Master Authentication Disabled, Severity: HIGH, Results: 1
...

	[1]: ../../path/myconfig.tf:7

		005:   initial_node_count = 3
		006:
		007:   master_auth {
		008:     username = ""
		009:     password = "root"
``` 

 It can be useful to have the ability to mask secrets in vulnerable code snippets even when the --disable-secrets option is enabled.


Discussion / hints:
Hi @ayala-orca , thanks for your suggestion,
We can do this by default, but we will need to run all secret queries under the hood anyway to detect and mask the secrets.
We can do the following:
1. the default is the same: always run secret queries and mask secrets when displaying them
2. the `--disable-secrets` behavior remains the same: don't run secret queries and therefore display them
3. add a new flag, e.g.,  `--always-mask-secrets` that changes the behavior of `--disable-secrets` by making the queries run under the hood, hide the results but mask the detected secrets when they are displayed in other queries.

WDYT?
@nunoocx 
The direction sounds right.
i do think the following, for bullet 3, will improve the performance and scan time - 
If only masking is needed, do not perform a full secret scan. Instead scan only files with issues (or, only the code snippets, but this is probably much harder to achieve)
@lior-orca that's actually a good idea!
Thanks for the suggestion.

Fix patch:
diff --git a/pkg/engine/secrets/inspector.go b/pkg/engine/secrets/inspector.go
index fec2072d207..bc5b07996ca 100644
--- a/pkg/engine/secrets/inspector.go
+++ b/pkg/engine/secrets/inspector.go
@@ -156,7 +156,7 @@ func NewInspector(
 		return nil, err
 	}
 
-	allowRules, err := compileRegex(allRegexQueries.AllowRules)
+	allowRules, err := CompileRegex(allRegexQueries.AllowRules)
 	if err != nil {
 		return nil, err
 	}
@@ -282,7 +282,8 @@ func compileRegexQueries(
 	return regexQueries, nil
 }
 
-func compileRegex(allowRules []AllowRule) ([]AllowRule, error) {
+// CompileRegex compiles the regex allow rules
+func CompileRegex(allowRules []AllowRule) ([]AllowRule, error) {
 	for j := range allowRules {
 		compiledRegex, err := regexp.Compile(allowRules[j].RegexStr)
 		if err != nil {
@@ -307,7 +308,7 @@ func isValueInArray(value string, array []string) bool {
 }
 
 func (c *Inspector) isSecret(s string, query *RegexQuery) (isSecretRet bool, groups [][]string) {
-	if isAllowRule(s, query.AllowRules) || isAllowRule(s, c.allowRules) {
+	if IsAllowRule(s, query.AllowRules) || IsAllowRule(s, c.allowRules) {
 		return false, [][]string{}
 	}
 
@@ -339,7 +340,8 @@ func (c *Inspector) isSecret(s string, query *RegexQuery) (isSecretRet bool, gro
 	return false, [][]string{}
 }
 
-func isAllowRule(s string, allowRules []AllowRule) bool {
+// IsAllowRule check if string matches any of the allow rules for the secret queries
+func IsAllowRule(s string, allowRules []AllowRule) bool {
 	for i := range allowRules {
 		if allowRules[i].Regex.MatchString(s) {
 			return true
diff --git a/pkg/scan/post_scan.go b/pkg/scan/post_scan.go
index 58fc58aefee..9ab34482695 100644
--- a/pkg/scan/post_scan.go
+++ b/pkg/scan/post_scan.go
@@ -105,6 +105,15 @@ func (c *Client) postScan(scanResults *Results) error {
 		}
 	}
 
+	// mask results preview if Secrets Scan is disabled
+	if c.ScanParams.DisableSecrets {
+		err := maskPreviewLines(c.ScanParams.SecretsRegexesPath, scanResults)
+		if err != nil {
+			log.Err(err)
+			return err
+		}
+	}
+
 	summary := c.getSummary(scanResults.Results, time.Now(), model.PathParameters{
 		ScannedPaths:      c.ScanParams.Path,
 		PathExtractionMap: scanResults.ExtractedPaths.ExtractionMap,
diff --git a/pkg/scan/preview_secrets_mask.go b/pkg/scan/preview_secrets_mask.go
new file mode 100644
index 00000000000..c192f503f2c
--- /dev/null
+++ b/pkg/scan/preview_secrets_mask.go
@@ -0,0 +1,151 @@
+// Package scan implements functions and helpers to ensure the proper scan of the specified files
+package scan
+
+import (
+	"encoding/json"
+	"regexp"
+	"strings"
+
+	"github.com/Checkmarx/kics/pkg/engine/secrets"
+	"github.com/Checkmarx/kics/pkg/model"
+)
+
+func maskPreviewLines(secretsPath string, scanResults *Results) error {
+	secretsRegexRulesContent, err := getSecretsRegexRules(secretsPath)
+	if err != nil {
+		return err
+	}
+
+	var allRegexQueries secrets.RegexRuleStruct
+
+	err = json.Unmarshal([]byte(secretsRegexRulesContent), &allRegexQueries)
+	if err != nil {
+		return err
+	}
+
+	allowRules, err := secrets.CompileRegex(allRegexQueries.AllowRules)
+	if err != nil {
+		return err
+	}
+
+	rules, err := compileRegexQueries(allRegexQueries.Rules)
+	if err != nil {
+		return err
+	}
+
+	for i := range scanResults.Results {
+		item := scanResults.Results[i]
+		hideSecret(item.VulnLines, &allowRules, &rules)
+	}
+	return nil
+}
+
+func compileRegexQueries(allRegexQueries []secrets.RegexQuery) ([]secrets.RegexQuery, error) {
+	for i := range allRegexQueries {
+		compiledRegexp, err := regexp.Compile(allRegexQueries[i].RegexStr)
+		if err != nil {
+			return allRegexQueries, err
+		}
+		allRegexQueries[i].Regex = compiledRegexp
+
+		for j := range allRegexQueries[i].AllowRules {
+			allRegexQueries[i].AllowRules[j].Regex = regexp.MustCompile(allRegexQueries[i].AllowRules[j].RegexStr)
+		}
+	}
+	return allRegexQueries, nil
+}
+
+func hideSecret(lines *[]model.CodeLine, allowRules *[]secrets.AllowRule, rules *[]secrets.RegexQuery) {
+	for idx, line := range *lines {
+		for i := range *rules {
+			rule := (*rules)[i]
+
+			isSecret, groups := isSecret(line.Line, &rule, allowRules)
+			// if not a secret skip to next line
+			if !isSecret {
+				continue
+			}
+
+			if len(rule.Entropies) == 0 {
+				maskSecret(&rule, lines, idx)
+			}
+
+			if len(groups[0]) > 0 {
+				for _, entropy := range rule.Entropies {
+					// if matched group does not exist continue
+					if len(groups[0]) <= entropy.Group {
+						return
+					}
+					isMatch, _ := secrets.CheckEntropyInterval(
+						entropy,
+						groups[0][entropy.Group],
+					)
+					if isMatch {
+						maskSecret(&rule, lines, idx)
+					}
+				}
+			}
+		}
+	}
+}
+
+func maskSecret(rule *secrets.RegexQuery, lines *[]model.CodeLine, idx int) {
+	if rule.SpecialMask == "all" {
+		(*lines)[idx].Line = "<SECRET-MASKED-ON-PURPOSE>"
+		return
+	}
+
+	regex := rule.RegexStr
+	line := (*lines)[idx]
+
+	if len(rule.SpecialMask) > 0 {
+		regex = "(.+)" + rule.SpecialMask
+	}
+
+	var re = regexp.MustCompile(regex)
+	match := re.FindString(line.Line)
+
+	if len(rule.SpecialMask) > 0 {
+		match = line.Line[len(match):]
+	}
+
+	if match != "" {
+		(*lines)[idx].Line = strings.Replace(line.Line, match, "<SECRET-MASKED-ON-PURPOSE>", 1)
+	} else {
+		(*lines)[idx].Line = "<SECRET-MASKED-ON-PURPOSE>"
+	}
+}
+
+// repurposed isSecret from inspector
+func isSecret(line string, rule *secrets.RegexQuery, allowRules *[]secrets.AllowRule) (isSecretRet bool, groups [][]string) {
+	if secrets.IsAllowRule(line, *allowRules) {
+		return false, [][]string{}
+	}
+
+	groups = rule.Regex.FindAllStringSubmatch(line, -1)
+
+	for _, group := range groups {
+		splitedText := strings.Split(line, "\n")
+		max := -1
+		for i, splited := range splitedText {
+			if len(groups) < rule.Multiline.DetectLineGroup {
+				if strings.Contains(splited, group[rule.Multiline.DetectLineGroup]) && i > max {
+					max = i
+				}
+			}
+		}
+		if max == -1 {
+			continue
+		}
+		secret, newGroups := isSecret(strings.Join(append(splitedText[:max], splitedText[max+1:]...), "\n"), rule, allowRules)
+		if !secret {
+			continue
+		}
+		groups = append(groups, newGroups...)
+	}
+
+	if len(groups) > 0 {
+		return true, groups
+	}
+	return false, [][]string{}
+}


Test patch:
diff --git a/pkg/scan/preview_secrets_mask_test.go b/pkg/scan/preview_secrets_mask_test.go
new file mode 100644
index 00000000000..c39437d45b9
--- /dev/null
+++ b/pkg/scan/preview_secrets_mask_test.go
@@ -0,0 +1,151 @@
+package scan
+
+import (
+	"github.com/Checkmarx/kics/internal/tracker"
+	"github.com/Checkmarx/kics/pkg/model"
+	"github.com/Checkmarx/kics/pkg/printer"
+	"github.com/Checkmarx/kics/pkg/progress"
+	"github.com/stretchr/testify/require"
+	"strings"
+	"testing"
+)
+
+func Test_maskSecrets(t *testing.T) {
+	tests := []struct {
+		name           string
+		filename       string
+		scanParameters Parameters
+		tracker        tracker.CITracker
+		scanResults    *Results
+	}{
+		{
+			name:     "print with masked secrets",
+			filename: "results",
+			scanParameters: Parameters{
+				DisableSecrets: true,
+			},
+			tracker: tracker.CITracker{
+				FoundFiles:         1,
+				FoundCountLines:    9,
+				ParsedCountLines:   9,
+				ParsedFiles:        1,
+				LoadedQueries:      146,
+				ExecutingQueries:   146,
+				ExecutedQueries:    146,
+				FailedSimilarityID: 0,
+				Version: model.Version{
+					Latest:           true,
+					LatestVersionTag: "Dev",
+				},
+			},
+			scanResults: &Results{
+				Results: []model.Vulnerability{
+					{
+						VulnLines: &[]model.CodeLine{
+							{
+								Position: 4,
+								Line:     " metadata:",
+							},
+							{
+								Position: 5,
+								Line:     "   name: secret-basic-auth:",
+							}, {
+								Position: 6,
+								Line:     "  password: \"abcd\"",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := Client{}
+			c.Tracker = &tt.tracker
+			c.ScanParams = &tt.scanParameters
+			c.ProBarBuilder = progress.InitializePbBuilder(true, false, true)
+			c.Printer = printer.NewPrinter(true)
+
+			err := c.postScan(tt.scanResults)
+			require.NoError(t, err)
+
+			for _, line := range (*tt.scanResults).Results {
+				for _, vulnLine := range *line.VulnLines {
+					if strings.Contains(vulnLine.Line, "password") {
+						require.Contains(t, vulnLine.Line, "<SECRET-MASKED-ON-PURPOSE>")
+					}
+				}
+			}
+
+		})
+
+	}
+}
+
+func Test_maskSecretsEntropies(t *testing.T) {
+	tests := []struct {
+		name           string
+		filename       string
+		scanParameters Parameters
+		tracker        tracker.CITracker
+		scanResults    *Results
+	}{
+		{
+			name:     "not print with masked secrets with invalid entropies",
+			filename: "results",
+			scanParameters: Parameters{
+				DisableSecrets: true,
+			},
+			tracker: tracker.CITracker{
+				FoundFiles:         1,
+				FoundCountLines:    9,
+				ParsedCountLines:   9,
+				ParsedFiles:        1,
+				LoadedQueries:      146,
+				ExecutingQueries:   146,
+				ExecutedQueries:    146,
+				FailedSimilarityID: 0,
+				Version: model.Version{
+					Latest:           true,
+					LatestVersionTag: "Dev",
+				},
+			},
+			scanResults: &Results{
+				Results: []model.Vulnerability{
+					{
+						VulnLines: &[]model.CodeLine{
+							{
+								Position: 4,
+								Line:     "secret = \"eeeeee\"",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := Client{}
+			c.Tracker = &tt.tracker
+			c.ScanParams = &tt.scanParameters
+			c.ProBarBuilder = progress.InitializePbBuilder(true, false, true)
+			c.Printer = printer.NewPrinter(true)
+
+			err := c.postScan(tt.scanResults)
+			require.NoError(t, err)
+
+			for _, line := range (*tt.scanResults).Results {
+				for _, vulnLine := range *line.VulnLines {
+					require.NotContains(t, vulnLine.Line, "<SECRET-MASKED-ON-PURPOSE>")
+
+				}
+			}
+
+		})
+
+	}
+}