From edce00336667263780e2b8edc670c96ac300acc5 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 12:55:50 +0100 Subject: [PATCH 01/15] Fix partial masking bug and support new allowlist format --- .../loki/secretfilter/secretfilter.go | 46 +++++++--- .../loki/secretfilter/secretfilter_test.go | 83 +++++++++++++++++++ 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index ec6b11dd19..cc758aac59 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -112,6 +112,10 @@ type GitLeaksConfig struct { StopWords []string Regexes []string } + Allowlists []struct { + StopWords []string + Regexes []string + } } } @@ -194,16 +198,16 @@ func (c *Component) processEntry(entry loki.Entry) loki.Entry { // Check if the secret is in the allowlist var allowRule *AllowRule = nil - // First check the rule-specific allowlist - for _, a := range r.allowlist { + // First check the global allowlist + for _, a := range c.AllowList { if a.Regex.MatchString(secret) { allowRule = &a break } } - // Then check the global allowlist + // Then check the rule-specific allowlists if allowRule == nil { - for _, a := range c.AllowList { + for _, a := range r.allowlist { if a.Regex.MatchString(secret) { allowRule = &a break @@ -231,8 +235,16 @@ func (c *Component) redactLine(line string, secret string, ruleName string) stri redactWith = strings.ReplaceAll(redactWith, "$SECRET_NAME", ruleName) redactWith = strings.ReplaceAll(redactWith, "$SECRET_HASH", hashSecret(secret)) } - if c.args.PartialMask > 0 { - redactWith = secret[:c.args.PartialMask] + redactWith + + partialMask := int(c.args.PartialMask) + if partialMask < 0 { + partialMask = 0 + } + if partialMask > 0 { + // Don't apply partial masking if the secret is too short + if len(secret) > partialMask+3 { + redactWith = secret[:partialMask] + redactWith + } } line = strings.ReplaceAll(line, secret, redactWith) @@ -306,6 +318,16 @@ func (c *Component) Update(args component.Arguments) error { } allowlist = append(allowlist, AllowRule{Regex: re, Source: fmt.Sprintf("rule %s", rule.ID)}) } + for _, currAllowList := range rule.Allowlists { + for _, r := range currAllowList.Regexes { + re, err := regexp.Compile(r) + if err != nil { + level.Error(c.opts.Logger).Log("msg", "error compiling allowlist regex", "error", err) + return err + } + allowlist = append(allowlist, AllowRule{Regex: re, Source: fmt.Sprintf("rule %s", rule.ID)}) + } + } newRule := Rule{ name: rule.ID, @@ -325,23 +347,23 @@ func (c *Component) Update(args component.Arguments) error { } // Compiling global allowlist regexes - // From the Gitleaks config - for _, r := range gitleaksCfg.AllowList.Regexes { + // From the arguments + for _, r := range c.args.AllowList { re, err := regexp.Compile(r) if err != nil { level.Error(c.opts.Logger).Log("msg", "error compiling allowlist regex", "error", err) return err } - c.AllowList = append(c.AllowList, AllowRule{Regex: re, Source: "gitleaks config"}) + c.AllowList = append(c.AllowList, AllowRule{Regex: re, Source: "alloy config"}) } - // From the arguments - for _, r := range c.args.AllowList { + // From the Gitleaks config + for _, r := range gitleaksCfg.AllowList.Regexes { re, err := regexp.Compile(r) if err != nil { level.Error(c.opts.Logger).Log("msg", "error compiling allowlist regex", "error", err) return err } - c.AllowList = append(c.AllowList, AllowRule{Regex: re, Source: "alloy config"}) + c.AllowList = append(c.AllowList, AllowRule{Regex: re, Source: "gitleaks config"}) } // Add the generic API key rule last if needed diff --git a/internal/component/loki/secretfilter/secretfilter_test.go b/internal/component/loki/secretfilter/secretfilter_test.go index f6d529f809..5aa64f1c9c 100644 --- a/internal/component/loki/secretfilter/secretfilter_test.go +++ b/internal/component/loki/secretfilter/secretfilter_test.go @@ -43,6 +43,36 @@ var customGitleaksConfig = map[string]string{ description = "Identified a fake secret" regex = '''(?i)\b(fakeSecret\d{5})(?:['|\"|\n|\r|\s|\x60|;]|$)''' `, + "short_secret": ` + title = "gitleaks custom config" + + [[rules]] + id = "short-secret" + description = "Identified a fake short secret" + regex = '''(?i)\b(abc)(?:['|\"|\n|\r|\s|\x60|;]|$)''' + `, + "allow_list_old": ` + title = "gitleaks custom config" + + [[rules]] + id = "my-fake-secret" + description = "Identified a fake secret" + regex = '''(?i)\b(fakeSecret\d{5})(?:['|\"|\n|\r|\s|\x60|;]|$)''' + [rules.allowlist] + regexes = ["abc\\d{3}", "fakeSecret[9]{5}"] + `, + "allow_list_new": ` + title = "gitleaks custom config" + + [[rules]] + id = "my-fake-secret" + description = "Identified a fake secret" + regex = '''(?i)\b(fakeSecret\d{5})(?:['|\"|\n|\r|\s|\x60|;]|$)''' + [[rules.allowlists]] + regexes = ["def\\d{3}", "test\\d{5}"] + [[rules.allowlists]] + regexes = ["abc\\d{3}", "fakeSecret[9]{5}"] + `, } var defaultRedactionString = "REDACTED-SECRET" @@ -65,6 +95,11 @@ var testConfigs = map[string]string{ forward_to = [] partial_mask = 4 `, + "partial_mask_custom": ` + forward_to = [] + partial_mask = 4 + gitleaks_config = "not-empty" // This will be replaced with the actual path to the temporary gitleaks config file + `, "custom_types": ` forward_to = [] redact_with = "<` + customRedactionString + `:$SECRET_NAME>" @@ -115,6 +150,14 @@ var fakeSecrets = map[string]fakeSecret{ name: "my-fake-secret", value: "fakeSec" + "ret12345", }, + "custom-fake-secret-all9": { + name: "my-fake-secret", + value: "fakeSec" + "ret99999", + }, + "short-secret": { + name: "short-secret", + value: "abc", + }, } // List of fake log entries to use for testing @@ -155,12 +198,24 @@ var testLogs = map[string]testLog{ }`, secrets: []fakeSecret{fakeSecrets["custom-fake-secret"]}, }, + "simple_secret_custom_all9": { + log: `{ + "message": "This is a simple log message with a secret value ` + fakeSecrets["custom-fake-secret-all9"].value + ` ! + }`, + secrets: []fakeSecret{fakeSecrets["custom-fake-secret-all9"]}, + }, "multiple_secrets": { log: `{ "message": "This is a simple log message with a secret value ` + fakeSecrets["grafana-api-key"].value + ` and another secret value ` + fakeSecrets["gcp-api-key"].value + ` ! }`, secrets: []fakeSecret{fakeSecrets["grafana-api-key"], fakeSecrets["gcp-api-key"]}, }, + "short_secret": { + log: `{ + "message": "This is a simple log message with a secret value ` + fakeSecrets["short-secret"].value + ` ! + }`, + secrets: []fakeSecret{fakeSecrets["short-secret"]}, + }, } // Test cases for the secret filter @@ -213,6 +268,13 @@ var tt = []struct { testLogs["simple_secret"].log, replaceSecrets(testLogs["simple_secret"].log, testLogs["simple_secret"].secrets, true, false, defaultRedactionString), }, + { + "partial_mask_too_short", + testConfigs["partial_mask_custom"], + customGitleaksConfig["short_secret"], + testLogs["short_secret"].log, + replaceSecrets(testLogs["short_secret"].log, testLogs["short_secret"].secrets, false, false, defaultRedactionString), + }, { "gcp_secret", testConfigs["default"], @@ -255,6 +317,27 @@ var tt = []struct { testLogs["simple_secret_custom"].log, replaceSecrets(testLogs["simple_secret_custom"].log, testLogs["simple_secret_custom"].secrets, false, false, defaultRedactionString), }, + { + "custom_gitleaks_file_allow_list_old", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_old"], + testLogs["simple_secret_custom-all9"].log, + testLogs["simple_secret_custom-all9"].log, // In the allowlist + }, + { + "custom_gitleaks_file_allow_list_new", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_new"], + testLogs["simple_secret_custom-all9"].log, + testLogs["simple_secret_custom-all9"].log, // In the allowlist + }, + { + "custom_gitleaks_file_allow_list_new_redact", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_new"], + testLogs["simple_secret_custom"].log, + replaceSecrets(testLogs["simple_secret_custom"].log, testLogs["simple_secret_custom"].secrets, false, false, defaultRedactionString), + }, { "multiple_secrets", testConfigs["default"], From 3b0ca15cef05a4a4b555242651faa35b1f863af2 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 13:06:34 +0100 Subject: [PATCH 02/15] Add docs and changelog --- CHANGELOG.md | 4 ++++ docs/sources/reference/components/loki/loki.secretfilter.md | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 685f799212..b9e0fe7289 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ Main (unreleased) - Update `prometheus.write.queue` library for performance increases in cpu. (@mattdurham) +- Update `loki.secretfilter` to be compatible with the new `[[rules.allowlists]]` gitleaks allowlist format (@romain-gaillard) + ### Bugfixes - Fixed issue with automemlimit logging bad messages and trying to access cgroup on non-linux builds (@dehaansa) @@ -70,6 +72,8 @@ Main (unreleased) - Fixed an issue where the `otelcol.processor.interval` could not be used because the debug metrics were not set to default. (@wildum) +- Fixed an issue where `loki.secretfilter` would crash if the secret was shorter than the `partial_mask` value. (@romain-gaillard) + ### Other changes - Change the stability of the `livedebugging` feature from "experimental" to "generally available". (@wildum) diff --git a/docs/sources/reference/components/loki/loki.secretfilter.md b/docs/sources/reference/components/loki/loki.secretfilter.md index cb412d2225..54f6e6152f 100644 --- a/docs/sources/reference/components/loki/loki.secretfilter.md +++ b/docs/sources/reference/components/loki/loki.secretfilter.md @@ -18,7 +18,7 @@ The detection is based on regular expression patterns, defined in the [Gitleaks {{< admonition type="caution" >}} Personally Identifiable Information (PII) isn't currently in scope and some secrets could remain undetected. -This component may generate false positives. +This component may generate false positives or redact too much. Don't rely solely on this component to redact sensitive information. {{< /admonition >}} @@ -49,6 +49,10 @@ Name | Type | Description The `gitleaks_config` argument is the path to the custom `gitleaks.toml` file. The Gitleaks configuration file embedded in the component is used if you don't provide the path to a custom configuration file. +{{< admonition type="note" >}} +This component does not support all the features of the Gitleaks configuration file. Currently, it only supports the regex-based rules, `secretGroup`, and allowlist regexes (`regexTarget` only supports the default value `secret`). Other features such as `keywords`, `entropy`, `paths`, and `stopwords` are not supported. The `extend` feature is also not supported, meaning that a custom configuration file must contain all the rules to use. +{{< /admonition >}} + The `types` argument is a map of secret types to look for. The values are used as prefixes for the secret types in the Gitleaks configuration. If you don't provide this argument, all types are used. {{< admonition type="note" >}} From 427d5372ef4215279f19b34bc694bd27c23e799b Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 13:20:55 +0100 Subject: [PATCH 03/15] Update docs --- docs/sources/reference/components/loki/loki.secretfilter.md | 1 + internal/component/loki/secretfilter/secretfilter.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/sources/reference/components/loki/loki.secretfilter.md b/docs/sources/reference/components/loki/loki.secretfilter.md index 54f6e6152f..3de989d808 100644 --- a/docs/sources/reference/components/loki/loki.secretfilter.md +++ b/docs/sources/reference/components/loki/loki.secretfilter.md @@ -78,6 +78,7 @@ A secret will not be redacted if it matches any of the regular expressions. The The `partial_mask` argument is the number of characters to show from the beginning of the secret before the redact string is added. If set to `0`, the entire secret is redacted. +Secrets shorter than the value of `partial_mask` + 3 are redacted entirely. ## Blocks diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index cc758aac59..13065dd7e1 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -242,7 +242,7 @@ func (c *Component) redactLine(line string, secret string, ruleName string) stri } if partialMask > 0 { // Don't apply partial masking if the secret is too short - if len(secret) > partialMask+3 { + if len(secret) > partialMask+2 { redactWith = secret[:partialMask] + redactWith } } From b57bde599d92c599daa6d37cdaa7dc6f7e63df05 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 13:22:29 +0100 Subject: [PATCH 04/15] Add comments --- internal/component/loki/secretfilter/secretfilter.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index 13065dd7e1..415d20fa9d 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -108,10 +108,12 @@ type GitLeaksConfig struct { Keywords []string SecretGroup int + // Old format, kept for compatibility Allowlist struct { StopWords []string Regexes []string } + // New format Allowlists []struct { StopWords []string Regexes []string From ba96cb3cbf294c355b09d5478e47d9df7f398516 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 13:22:58 +0100 Subject: [PATCH 05/15] Add comments --- internal/component/loki/secretfilter/secretfilter.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index 415d20fa9d..d7b9383b74 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -98,24 +98,24 @@ type Component struct { type GitLeaksConfig struct { AllowList struct { Description string - Paths []string + Paths []string // Not used Regexes []string } Rules []struct { ID string - Description string + Description string // Not used Regex string - Keywords []string + Keywords []string // Not used SecretGroup int // Old format, kept for compatibility Allowlist struct { - StopWords []string + StopWords []string // Not used Regexes []string } // New format Allowlists []struct { - StopWords []string + StopWords []string // Not used Regexes []string } } From 1cb0a46245a5e2539eb63deba41b4c4a22d84b6b Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 13:34:16 +0100 Subject: [PATCH 06/15] Minor docs update --- docs/sources/reference/components/loki/loki.secretfilter.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/reference/components/loki/loki.secretfilter.md b/docs/sources/reference/components/loki/loki.secretfilter.md index 3de989d808..3b734aeb7b 100644 --- a/docs/sources/reference/components/loki/loki.secretfilter.md +++ b/docs/sources/reference/components/loki/loki.secretfilter.md @@ -78,7 +78,7 @@ A secret will not be redacted if it matches any of the regular expressions. The The `partial_mask` argument is the number of characters to show from the beginning of the secret before the redact string is added. If set to `0`, the entire secret is redacted. -Secrets shorter than the value of `partial_mask` + 3 are redacted entirely. +Secrets shorter than `partial_mask + 3` are redacted entirely. ## Blocks From d8070a94aa3096038d390db0f671c6ce9c4c1655 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 16:35:49 +0100 Subject: [PATCH 07/15] Add more tests --- .../loki/secretfilter/secretfilter.go | 2 +- .../loki/secretfilter/secretfilter_test.go | 89 ++++++++++++++++++- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index d7b9383b74..e609897813 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -244,7 +244,7 @@ func (c *Component) redactLine(line string, secret string, ruleName string) stri } if partialMask > 0 { // Don't apply partial masking if the secret is too short - if len(secret) > partialMask+2 { + if len(secret) > partialMask*2 { redactWith = secret[:partialMask] + redactWith } } diff --git a/internal/component/loki/secretfilter/secretfilter_test.go b/internal/component/loki/secretfilter/secretfilter_test.go index 5aa64f1c9c..ec2c248775 100644 --- a/internal/component/loki/secretfilter/secretfilter_test.go +++ b/internal/component/loki/secretfilter/secretfilter_test.go @@ -73,6 +73,16 @@ var customGitleaksConfig = map[string]string{ [[rules.allowlists]] regexes = ["abc\\d{3}", "fakeSecret[9]{5}"] `, + `allow_list_global`: ` + title = "gitleaks custom config" + + [[rules]] + id = "my-fake-secret" + description = "Identified a fake secret" + regex = '''(?i)\b(fakeSecret\d{5})(?:['|\"|\n|\r|\s|\x60|;]|$)''' + [allowlist] + regexes = ["abc\\d{3}", "fakeSecret[9]{5}"] + `, } var defaultRedactionString = "REDACTED-SECRET" @@ -321,15 +331,22 @@ var tt = []struct { "custom_gitleaks_file_allow_list_old", testConfigs["custom_gitleaks_file_simple"], customGitleaksConfig["allow_list_old"], - testLogs["simple_secret_custom-all9"].log, - testLogs["simple_secret_custom-all9"].log, // In the allowlist + testLogs["simple_secret_custom_all9"].log, + testLogs["simple_secret_custom_all9"].log, // In the allowlist }, { "custom_gitleaks_file_allow_list_new", testConfigs["custom_gitleaks_file_simple"], customGitleaksConfig["allow_list_new"], - testLogs["simple_secret_custom-all9"].log, - testLogs["simple_secret_custom-all9"].log, // In the allowlist + testLogs["simple_secret_custom_all9"].log, + testLogs["simple_secret_custom_all9"].log, // In the allowlist + }, + { + "custom_gitleaks_file_allow_list_old_redact", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_old"], + testLogs["simple_secret_custom"].log, + replaceSecrets(testLogs["simple_secret_custom"].log, testLogs["simple_secret_custom"].secrets, false, false, defaultRedactionString), }, { "custom_gitleaks_file_allow_list_new_redact", @@ -338,6 +355,20 @@ var tt = []struct { testLogs["simple_secret_custom"].log, replaceSecrets(testLogs["simple_secret_custom"].log, testLogs["simple_secret_custom"].secrets, false, false, defaultRedactionString), }, + { + "custom_gitleaks_file_allow_list_global", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_global"], + testLogs["simple_secret_custom_all9"].log, + testLogs["simple_secret_custom_all9"].log, // In the allowlist + }, + { + "custom_gitleaks_file_allow_list_global_redact", + testConfigs["custom_gitleaks_file_simple"], + customGitleaksConfig["allow_list_global"], + testLogs["simple_secret_custom"].log, + replaceSecrets(testLogs["simple_secret_custom"].log, testLogs["simple_secret_custom"].secrets, false, false, defaultRedactionString), + }, { "multiple_secrets", testConfigs["default"], @@ -355,12 +386,62 @@ func TestSecretFiltering(t *testing.T) { } } +func TestPartialMasking(t *testing.T) { + // Start testing with common cases + component := &Component{} + component.args = Arguments{PartialMask: 4} + // Too short to be partially masked + redacted := component.redactLine("This is a short secret abc123 in a log line", "abc123", "test-rule") + require.Equal(t, "This is a short secret in a log line", redacted) + + // Too short to be partially masked + redacted = component.redactLine("This is a longer secret abcd1234 in a log line", "abcd1234", "test-rule") + require.Equal(t, "This is a longer secret in a log line", redacted) + + // Will be partially masked + redacted = component.redactLine("This is a long enough secret abcd12345 in a log line", "abcd12345", "test-rule") + require.Equal(t, "This is a long enough secret abcd in a log line", redacted) + + // Will be partially masked + redacted = component.redactLine("This is the longest secret abcdef12345678 in a log line", "abcdef12345678", "test-rule") + require.Equal(t, "This is the longest secret abcd in a log line", redacted) + + // Test with different secret lengths and partial masking values + for _, partialMasking := range []int{1, 3, 4, 5, 9} { + for secretLength := range 30 { + if secretLength < 2 { + continue + } + expected := secretLength > partialMasking*2 + checkPartialMasking(t, partialMasking, secretLength, expected) + } + } +} + +func checkPartialMasking(t *testing.T, partialMasking int, secretLength int, partialRedactionExpected bool) { + component := &Component{} + component.args = Arguments{PartialMask: uint(partialMasking)} + secret := strings.Repeat("A", secretLength) + inputLog := fmt.Sprintf("This is a test with a secret %s in a log line", secret) + redacted := component.redactLine(inputLog, secret, "test-rule") + + prefix := "" + if partialRedactionExpected { + prefix = strings.Repeat("A", partialMasking) + } + expectedLog := fmt.Sprintf("This is a test with a secret %s in a log line", prefix) + require.Equal(t, expectedLog, redacted) +} + func runTest(t *testing.T, config string, gitLeaksConfigContent string, inputLog string, expectedLog string) { ch1 := loki.NewLogsReceiver() var args Arguments require.NoError(t, syntax.Unmarshal([]byte(config), &args)) args.ForwardTo = []loki.LogsReceiver{ch1} + // Making sure we're not testing with an empty log line by mistake + require.NotEmpty(t, inputLog) + // If needed, create a temporary gitleaks config file if args.GitleaksConfig != "" { args.GitleaksConfig = createTempGitleaksConfig(t, gitLeaksConfigContent) From c27fb758222724113d92d5b4cf8195fdede96264 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Tue, 31 Dec 2024 16:48:13 +0100 Subject: [PATCH 08/15] Change criteria for partial redaction --- .../components/loki/loki.secretfilter.md | 2 +- .../component/loki/secretfilter/secretfilter.go | 2 +- .../loki/secretfilter/secretfilter_test.go | 15 ++++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/sources/reference/components/loki/loki.secretfilter.md b/docs/sources/reference/components/loki/loki.secretfilter.md index 3b734aeb7b..0a954c8ff7 100644 --- a/docs/sources/reference/components/loki/loki.secretfilter.md +++ b/docs/sources/reference/components/loki/loki.secretfilter.md @@ -78,7 +78,7 @@ A secret will not be redacted if it matches any of the regular expressions. The The `partial_mask` argument is the number of characters to show from the beginning of the secret before the redact string is added. If set to `0`, the entire secret is redacted. -Secrets shorter than `partial_mask + 3` are redacted entirely. +If a secret is not at least 3 characters long and twice as long as the `partial_mask`, the entire secret is redacted. ## Blocks diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index e609897813..372fea5eaa 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -244,7 +244,7 @@ func (c *Component) redactLine(line string, secret string, ruleName string) stri } if partialMask > 0 { // Don't apply partial masking if the secret is too short - if len(secret) > partialMask*2 { + if len(secret) >= 3 && len(secret) >= partialMask*2 { redactWith = secret[:partialMask] + redactWith } } diff --git a/internal/component/loki/secretfilter/secretfilter_test.go b/internal/component/loki/secretfilter/secretfilter_test.go index ec2c248775..f2b21738e1 100644 --- a/internal/component/loki/secretfilter/secretfilter_test.go +++ b/internal/component/loki/secretfilter/secretfilter_test.go @@ -390,16 +390,17 @@ func TestPartialMasking(t *testing.T) { // Start testing with common cases component := &Component{} component.args = Arguments{PartialMask: 4} + // Too short to be partially masked - redacted := component.redactLine("This is a short secret abc123 in a log line", "abc123", "test-rule") - require.Equal(t, "This is a short secret in a log line", redacted) + redacted := component.redactLine("This is a very short secret ab in a log line", "ab", "test-rule") + require.Equal(t, "This is a very short secret in a log line", redacted) // Too short to be partially masked - redacted = component.redactLine("This is a longer secret abcd1234 in a log line", "abcd1234", "test-rule") - require.Equal(t, "This is a longer secret in a log line", redacted) + redacted = component.redactLine("This is a short secret abc123 in a log line", "abc123", "test-rule") + require.Equal(t, "This is a short secret in a log line", redacted) // Will be partially masked - redacted = component.redactLine("This is a long enough secret abcd12345 in a log line", "abcd12345", "test-rule") + redacted = component.redactLine("This is a long enough secret abcd1234 in a log line", "abcd1234", "test-rule") require.Equal(t, "This is a long enough secret abcd in a log line", redacted) // Will be partially masked @@ -407,12 +408,12 @@ func TestPartialMasking(t *testing.T) { require.Equal(t, "This is the longest secret abcd in a log line", redacted) // Test with different secret lengths and partial masking values - for _, partialMasking := range []int{1, 3, 4, 5, 9} { + for _, partialMasking := range []int{1, 2, 3, 4, 5, 9} { for secretLength := range 30 { if secretLength < 2 { continue } - expected := secretLength > partialMasking*2 + expected := secretLength >= 3 && secretLength >= partialMasking*2 checkPartialMasking(t, partialMasking, secretLength, expected) } } From 7fb2246ad5cdf2210b9361bbf35e4a4ae8373c69 Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Thu, 2 Jan 2025 13:47:57 +0100 Subject: [PATCH 09/15] Changes to partial masking rules --- .../loki/secretfilter/secretfilter.go | 13 +++++-- .../loki/secretfilter/secretfilter_test.go | 37 ++++++++++++++----- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/internal/component/loki/secretfilter/secretfilter.go b/internal/component/loki/secretfilter/secretfilter.go index 372fea5eaa..8e6144ea8a 100644 --- a/internal/component/loki/secretfilter/secretfilter.go +++ b/internal/component/loki/secretfilter/secretfilter.go @@ -238,15 +238,20 @@ func (c *Component) redactLine(line string, secret string, ruleName string) stri redactWith = strings.ReplaceAll(redactWith, "$SECRET_HASH", hashSecret(secret)) } + // If partialMask is set, show the first N characters of the secret partialMask := int(c.args.PartialMask) if partialMask < 0 { partialMask = 0 } - if partialMask > 0 { - // Don't apply partial masking if the secret is too short - if len(secret) >= 3 && len(secret) >= partialMask*2 { - redactWith = secret[:partialMask] + redactWith + runesSecret := []rune(secret) + // Only do it if the secret is long enough + if partialMask > 0 && len(runesSecret) >= 6 { + // Show at most half of the secret + if partialMask > len(runesSecret)/2 { + partialMask = len(runesSecret) / 2 } + prefix := string(runesSecret[:partialMask]) + redactWith = prefix + redactWith } line = strings.ReplaceAll(line, secret, redactWith) diff --git a/internal/component/loki/secretfilter/secretfilter_test.go b/internal/component/loki/secretfilter/secretfilter_test.go index f2b21738e1..4d84e17420 100644 --- a/internal/component/loki/secretfilter/secretfilter_test.go +++ b/internal/component/loki/secretfilter/secretfilter_test.go @@ -396,9 +396,13 @@ func TestPartialMasking(t *testing.T) { require.Equal(t, "This is a very short secret in a log line", redacted) // Too short to be partially masked - redacted = component.redactLine("This is a short secret abc123 in a log line", "abc123", "test-rule") + redacted = component.redactLine("This is a short secret abc12 in a log line", "abc12", "test-rule") require.Equal(t, "This is a short secret in a log line", redacted) + // Will be partially masked (limited by secret length) + redacted = component.redactLine("This is a longer secret abc123 in a log line", "abc123", "test-rule") + require.Equal(t, "This is a longer secret abc in a log line", redacted) + // Will be partially masked redacted = component.redactLine("This is a long enough secret abcd1234 in a log line", "abcd1234", "test-rule") require.Equal(t, "This is a long enough secret abcd in a log line", redacted) @@ -407,31 +411,44 @@ func TestPartialMasking(t *testing.T) { redacted = component.redactLine("This is the longest secret abcdef12345678 in a log line", "abcdef12345678", "test-rule") require.Equal(t, "This is the longest secret abcd in a log line", redacted) + // Test with a non-ASCII character + redacted = component.redactLine("This is a line with a complex secret aBc\U0001f512De\U0001f5124 in a log line", "aBc\U0001f512De\U0001f5124", "test-rule") + require.Equal(t, "This is a line with a complex secret aBc\U0001f512 in a log line", redacted) + // Test with different secret lengths and partial masking values - for _, partialMasking := range []int{1, 2, 3, 4, 5, 9} { - for secretLength := range 30 { + for partialMasking := range 20 { + for secretLength := range 50 { if secretLength < 2 { continue } - expected := secretLength >= 3 && secretLength >= partialMasking*2 - checkPartialMasking(t, partialMasking, secretLength, expected) + expectedPrefixLength := 0 + if secretLength >= 6 { + expectedPrefixLength = min(secretLength/2, partialMasking) + } + checkPartialMasking(t, partialMasking, secretLength, expectedPrefixLength) } } } -func checkPartialMasking(t *testing.T, partialMasking int, secretLength int, partialRedactionExpected bool) { +func checkPartialMasking(t *testing.T, partialMasking int, secretLength int, expectedPrefixLength int) { component := &Component{} component.args = Arguments{PartialMask: uint(partialMasking)} secret := strings.Repeat("A", secretLength) inputLog := fmt.Sprintf("This is a test with a secret %s in a log line", secret) redacted := component.redactLine(inputLog, secret, "test-rule") - prefix := "" - if partialRedactionExpected { - prefix = strings.Repeat("A", partialMasking) - } + // Test with a simple ASCII character + prefix := strings.Repeat("A", expectedPrefixLength) expectedLog := fmt.Sprintf("This is a test with a secret %s in a log line", prefix) require.Equal(t, expectedLog, redacted) + + // Test with a non-ASCII character + secret = strings.Repeat("\U0001f512", secretLength) + inputLog = fmt.Sprintf("This is a test with a secret %s in a log line", secret) + redacted = component.redactLine(inputLog, secret, "test-rule") + prefix = strings.Repeat("\U0001f512", expectedPrefixLength) + expectedLog = fmt.Sprintf("This is a test with a secret %s in a log line", prefix) + require.Equal(t, expectedLog, redacted) } func runTest(t *testing.T, config string, gitLeaksConfigContent string, inputLog string, expectedLog string) { From 30a23a9fe5da1ed4253858d5aa36cceafeae7f4e Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Thu, 2 Jan 2025 14:27:29 +0100 Subject: [PATCH 10/15] Fix comment location --- internal/component/loki/secretfilter/secretfilter_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/component/loki/secretfilter/secretfilter_test.go b/internal/component/loki/secretfilter/secretfilter_test.go index 4d84e17420..cc40698a10 100644 --- a/internal/component/loki/secretfilter/secretfilter_test.go +++ b/internal/component/loki/secretfilter/secretfilter_test.go @@ -433,11 +433,11 @@ func TestPartialMasking(t *testing.T) { func checkPartialMasking(t *testing.T, partialMasking int, secretLength int, expectedPrefixLength int) { component := &Component{} component.args = Arguments{PartialMask: uint(partialMasking)} + + // Test with a simple ASCII character secret := strings.Repeat("A", secretLength) inputLog := fmt.Sprintf("This is a test with a secret %s in a log line", secret) redacted := component.redactLine(inputLog, secret, "test-rule") - - // Test with a simple ASCII character prefix := strings.Repeat("A", expectedPrefixLength) expectedLog := fmt.Sprintf("This is a test with a secret %s in a log line", prefix) require.Equal(t, expectedLog, redacted) From 73358f84c5c0cf4e9a6d9401e319920790c93c6c Mon Sep 17 00:00:00 2001 From: Romain Gaillard Date: Fri, 3 Jan 2025 14:45:27 +0100 Subject: [PATCH 11/15] Clarify usage of secret types --- .../components/loki/loki.secretfilter.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/sources/reference/components/loki/loki.secretfilter.md b/docs/sources/reference/components/loki/loki.secretfilter.md index 0a954c8ff7..b94c3cb5fc 100644 --- a/docs/sources/reference/components/loki/loki.secretfilter.md +++ b/docs/sources/reference/components/loki/loki.secretfilter.md @@ -39,12 +39,12 @@ loki.secretfilter "