From a7b0236035408b26ce73772eeb4a3bc2ec47c05e Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 21 Oct 2026 19:17:20 +0200 Subject: [PATCH 1/2] fix(trace): preserve password-field selectors during redaction Allow only the two complete login selectors that triggered generic-password false positives in platform PR #6986, including JSON-encoded tool output. Keep adjacent credentials and selector-like passwords detectable, and bump the redaction ruleset version. Assisted-by: Codex Signed-off-by: Miguel Martinez Trivino --- internal/redaction/betterleaks.go | 2 +- internal/redaction/betterleaks.toml | 7 ++++ internal/redaction/precision_test.go | 57 ++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 5342f22ff..567d31126 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -79,7 +79,7 @@ var rulesConfig string // betterleaks.toml, the betterleaks version, or the handling of findings in // this file changes what gets redacted, so that policies can tell placeholders // from different rulesets apart. Version 1 is the plain default ruleset. -const RulesetVersion = 3 +const RulesetVersion = 4 var ( defaultScanner = sync.OnceValues(newBetterleaksScanner) diff --git a/internal/redaction/betterleaks.toml b/internal/redaction/betterleaks.toml index a02a0fdec..106d83bd1 100644 --- a/internal/redaction/betterleaks.toml +++ b/internal/redaction/betterleaks.toml @@ -44,6 +44,13 @@ regexes = [ '''^\([0-9]+(?:\.[0-9]+)?(?:ns|us|µs|ms|s|m|h)\)(?:\\+[nrt].*)?$''', ] +[[allowlists]] +description = "Exact password-field CSS selectors used by login helpers" +targetRules = ["generic-password"] +# Match only these complete selectors, including JSON-escaped quotes in tool +# results. Never allow the whole line: it can carry other password assignments. +regexes = ['''^(?:\[id=\\*"1-password\\*"\]|input\[name=\\*"password\\*"\])$'''] + [[allowlists]] description = "Placeholders in documentation, such as Bearer YOUR_API_TOKEN" targetRules = ["curl-auth-header"] diff --git a/internal/redaction/precision_test.go b/internal/redaction/precision_test.go index 06a99169a..da3f779dc 100644 --- a/internal/redaction/precision_test.go +++ b/internal/redaction/precision_test.go @@ -17,6 +17,7 @@ package redaction import ( "context" + "encoding/json" "strings" "testing" @@ -164,6 +165,50 @@ func TestRulesConfigKeepsNonSecrets(t *testing.T) { } } +// Codex tool results carry JSON-encoded command output inside a text leaf. +// Check both that shape and plain output, with a real password beside the +// selector so the exemption cannot hide other findings in the same leaf. +func TestRedactPasswordFieldSelectors(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + unfiltered := defaultRulesScanner(t) + + for _, selector := range []string{`[id="1-password"]`, `input[name="password"]`} { + for _, encoded := range []bool{false, true} { + name := selector + if encoded { + name += "/json tool output" + } + t.Run(name, func(t *testing.T) { + text := "const AUTH0 = {\n email: 'input[name=\"login\"]',\n password: '" + selector + "',\n};\n" + wrap := func(text string) string { + if !encoded { + return text + } + b, err := json.Marshal(map[string]string{"output": text}) + require.NoError(t, err) + return string(b) + } + + _, report, err := New(unfiltered).RedactText(context.Background(), wrap(text)) + require.NoError(t, err) + require.Contains(t, report.RuleIDs(), genericPasswordRuleID, "the original ruleset must detect the selector") + + got, report, err := New(scanner).RedactText(context.Background(), wrap(text)) + require.NoError(t, err) + assert.Equal(t, wrap(text), got) + assert.False(t, report.Changed()) + + text += "user: " + sampleEmail + "\npassword: " + samplePassword + ";\n" + got, report, err = New(scanner).RedactText(context.Background(), wrap(text)) + require.NoError(t, err) + assert.Equal(t, wrap(strings.ReplaceAll(text, samplePassword, DefaultPlaceholder(genericPasswordRuleID))), got) + assert.Equal(t, map[string]int{genericPasswordRuleID: 1}, report.ByRule) + }) + } + } +} + // TestRulesConfigStillRedactsSecrets is the counterpart of // TestRulesConfigKeepsNonSecrets: a real secret of each shape is still // replaced. @@ -204,6 +249,18 @@ func TestRulesConfigStillRedactsSecrets(t *testing.T) { secret: sampleAPIKey, wantRule: "curl-auth-header", }, + { + name: "password with a selector suffix", + text: "user: " + sampleEmail + "\npassword: '" + samplePassword + `[id="1-password"]'` + "\n", + secret: samplePassword, + wantRule: genericPasswordRuleID, + }, + { + name: "password with a selector prefix", + text: "user: " + sampleEmail + "\npassword: '" + `input[name="password"]` + samplePassword + "'\n", + secret: samplePassword, + wantRule: genericPasswordRuleID, + }, { name: "password with a logical operator in it", text: "host: db.internal\nuser: " + sampleEmail + "\npassword: Xk9m||Q2vL" + "p7wRt4z\n", From 6dd1d1a19510c5a880a712414b5c453450a9003f Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 21 Oct 2026 19:28:06 +0200 Subject: [PATCH 2/2] fix(trace): bound selector escaping without breaking JSON output Allow only paired plain, one-backslash, and three-backslash quote forms. Redactor adds a JSON layer before scanning, so encoded tool output needs the three-backslash form. Add regression coverage that keeps selectors with extra escaping detectable. Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Assisted-by: Codex Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 01a15f82-e12d-7492-a2b4-f2edf0cc2aea --- internal/redaction/betterleaks.toml | 12 +++++++++--- internal/redaction/precision_test.go | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/internal/redaction/betterleaks.toml b/internal/redaction/betterleaks.toml index 106d83bd1..5839fcc1a 100644 --- a/internal/redaction/betterleaks.toml +++ b/internal/redaction/betterleaks.toml @@ -47,9 +47,15 @@ regexes = [ [[allowlists]] description = "Exact password-field CSS selectors used by login helpers" targetRules = ["generic-password"] -# Match only these complete selectors, including JSON-escaped quotes in tool -# results. Never allow the whole line: it can carry other password assignments. -regexes = ['''^(?:\[id=\\*"1-password\\*"\]|input\[name=\\*"password\\*"\])$'''] +# Match only these complete selectors. Redactor JSON-encodes text before +# scanning, so plain quotes arrive with one backslash and JSON-encoded tool +# results with three. Keep those forms explicit and paired; do not accept +# arbitrary escaping or the whole line, which can carry other passwords. +regexes = [ + '''^(?:\[id="1-password"\]|input\[name="password"\])$''', + '''^(?:\[id=\\"1-password\\"\]|input\[name=\\"password\\"\])$''', + '''^(?:\[id=\\{3}"1-password\\{3}"\]|input\[name=\\{3}"password\\{3}"\])$''', +] [[allowlists]] description = "Placeholders in documentation, such as Bearer YOUR_API_TOKEN" diff --git a/internal/redaction/precision_test.go b/internal/redaction/precision_test.go index da3f779dc..df40de86b 100644 --- a/internal/redaction/precision_test.go +++ b/internal/redaction/precision_test.go @@ -209,6 +209,25 @@ func TestRedactPasswordFieldSelectors(t *testing.T) { } } +// Extra escaping must not turn a credential into an allowlisted selector. +func TestRedactOverescapedPasswordFieldSelectors(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + + for _, selector := range []string{`[id="1-password"]`, `input[name="password"]`} { + for _, backslashes := range []int{2, 3} { + secret := strings.ReplaceAll(selector, `"`, strings.Repeat(`\`, backslashes)+`"`) + t.Run(secret, func(t *testing.T) { + text := "user: " + sampleEmail + "\npassword: '" + secret + "'\n" + got, report, err := New(scanner).RedactText(context.Background(), text) + require.NoError(t, err) + assert.Equal(t, strings.ReplaceAll(text, secret, DefaultPlaceholder(genericPasswordRuleID)), got) + assert.Equal(t, map[string]int{genericPasswordRuleID: 1}, report.ByRule) + }) + } + } +} + // TestRulesConfigStillRedactsSecrets is the counterpart of // TestRulesConfigKeepsNonSecrets: a real secret of each shape is still // replaced.