From 7e7f2f33097ec7a3f0dd7668af0041d51933a22e Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 1 Oct 2026 23:49:29 +0200 Subject: [PATCH 1/2] fix(redaction): replace only the JWT when it is followed by a JSON escape The scanner reads the document as JSON-encoded text. The jwt rule allows a backslash in its last two segments, so its match can end on the first backslash of an escape such as \" or run through \n into the next word. In the first case the replacement broke the escape, the leaf no longer decoded, and the whole leaf was replaced with the placeholder. That removed the presigned-URL host that ai-config-no-secrets needs to suppress a signed-URL JWT. The engine now drops a trailing backslash that starts an escape the secret does not include, for every rule. The jwt rule's secret is also cut at the first quote or whitespace escape, because a JWT cannot contain one. Fixes #3481 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 9dfd2d47-4405-4b51-adb4-a990f965c3a7 --- internal/redaction/betterleaks.go | 29 +++++++++++++ internal/redaction/betterleaks_test.go | 60 ++++++++++++++++++++++++++ internal/redaction/redaction.go | 31 ++++++++++--- internal/redaction/redaction_test.go | 16 +++++++ 4 files changed, 130 insertions(+), 6 deletions(-) diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 92d02a700..a6da4d059 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -332,12 +332,41 @@ func byteSpans(text string, ranges []lineSpan) []span { // appendSecret records a locatable secret. A finding without one cannot be // searched for in the document, so it is dropped rather than reported. func appendSecret(dst []Finding, ruleID, secret string) []Finding { + if ruleID == jwtRuleID { + secret = cutAtTerminatorEscape(secret) + } if secret == "" { return dst } return append(dst, Finding{RuleID: ruleID, Secret: secret}) } +// jwtRuleID is the default ruleset's rule for JSON Web Tokens. +const jwtRuleID = "jwt" + +// jwtTerminatorEscapes are the JSON escapes that decode to a character a JWT +// cannot contain, so a JWT ends where one of them starts. +var jwtTerminatorEscapes = []string{`\"`, `\n`, `\r`, `\t`} + +// cutAtTerminatorEscape removes the text from the first whitespace or quote +// escape onwards. +// +// The jwt rule allows a backslash in its last two segments, so in JSON-encoded +// text its match runs through an escape such as `\n` and into the word after +// it. Replacing that match would remove text that is not part of the token. A +// JWT is base64url, so the part before such an escape is the whole token. Other +// escapes, such as `\/` or `\\`, are kept: they are not proof that the token has +// ended, and keeping them can only over-redact. +func cutAtTerminatorEscape(secret string) string { + end := len(secret) + for _, esc := range jwtTerminatorEscapes { + if i := strings.Index(secret, esc); i >= 0 && i < end { + end = i + } + } + return secret[:end] +} + // span is a half-open byte range [start, end) of a text. type span struct { start, end int diff --git a/internal/redaction/betterleaks_test.go b/internal/redaction/betterleaks_test.go index 7de8a4737..225d745bf 100644 --- a/internal/redaction/betterleaks_test.go +++ b/internal/redaction/betterleaks_test.go @@ -17,6 +17,7 @@ package redaction import ( "context" + "encoding/json" "regexp" "strconv" "strings" @@ -42,6 +43,7 @@ const ( fakeAWSKey = "AKIA" + "4G7TI63VCBIRS4GW" fakeAWSSecret = "kQ7zXn2VbW9pLm4RtY6" + "uHs3JdF8gA1cE5oPzQwXn" fakeGitHubPAT = "ghp_erOZlZv0B1e3amrQ" + "ugdwZ8Ro2W4kDql9WPTf" + fakeJWT = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9" + ".eyJzdWIiOiJmYWtlLXVwbG9hZCIsImV4cCI6MTc5MDAwMDAwMH0" + ".c2lnbmF0dXJlLWZha2UtZm9yLXJlcHJv" ) var fakeAnthropicKey = "sk-ant-api03-" + strings.Repeat("a", 93) + "AA" @@ -279,6 +281,64 @@ func TestRedactCredentialInURIConverges(t *testing.T) { assert.False(t, report.Changed()) } +// TestRedactJWTKeepsSurroundingText covers a JWT that is followed by a JSON +// escape sequence in its string leaf. The jwt rule allows a backslash in its +// last segments, so the scanner reports the secret with the backslash that +// starts the escape. Only the JWT must be replaced: the text around it, such as +// the host of a presigned URL, is what lets policies tell a short-lived URL +// signature from a leaked credential. +func TestRedactJWTKeepsSurroundingText(t *testing.T) { + scanner, err := DefaultScanner() + require.NoError(t, err) + + testCases := []struct { + name string + leaf string + want string + }{ + { + // A tool result that holds a JSON document as text: the quote after + // the JWT is escaped in the leaf. + name: "followed by an escaped quote in a nested JSON string", + leaf: `{"url":"https://uploads.example.com/o/a/b?signature=` + fakeJWT + `"}`, + want: `{"url":"https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]"}`, + }, + { + name: "followed by an escaped newline", + leaf: "https://uploads.example.com/o/a/b?signature=" + fakeJWT + "\nnext line", + want: "https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]\nnext line", + }, + { + // The literal backslash is a complete `\\` escape, which is kept in + // the secret: the leaf loses that one character but not its context. + name: "followed by a literal backslash", + leaf: `https://uploads.example.com/o/a/b?signature=` + fakeJWT + `\"`, + want: `https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]"`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + doc, err := json.Marshal(map[string]string{"text": tc.leaf}) + require.NoError(t, err) + + once, report, err := New(scanner).Redact(context.Background(), doc) + require.NoError(t, err) + require.True(t, report.Changed()) + assert.Equal(t, map[string]int{"jwt": 1}, report.ByRule) + + var got map[string]string + require.NoError(t, json.Unmarshal(once, &got)) + assert.Equal(t, tc.want, got["text"], "only the JWT is replaced") + + twice, report, err := New(scanner).Redact(context.Background(), once) + require.NoError(t, err) + assert.Equal(t, string(once), string(twice)) + assert.False(t, report.Changed()) + }) + } +} + // TestDefaultScannerUsesRE2 pins the regex engine. The library default is the // standard library engine, which is several times slower on the default ruleset // and made large sessions take tens of minutes to redact. diff --git a/internal/redaction/redaction.go b/internal/redaction/redaction.go index 516355718..dfa1641b0 100644 --- a/internal/redaction/redaction.go +++ b/internal/redaction/redaction.go @@ -312,23 +312,24 @@ func pendingSecrets(findings []Finding, skip map[string]struct{}, isPlaceholder out := make([]secretRule, 0, len(findings)) for _, f := range findings { - if f.Secret == "" { + secret := trimPartialEscape(f.Secret) + if secret == "" { continue } - if _, skipped := skip[f.Secret]; skipped { + if _, skipped := skip[secret]; skipped { continue } // A finding whose whole secret is a placeholder is a rule matching the // position it sits in, not a credential. Rewriting it would churn the // document on every run and never terminate. - if isPlaceholder != nil && isPlaceholder(f.Secret) { + if isPlaceholder != nil && isPlaceholder(secret) { continue } - if _, dup := seen[f.Secret]; dup { + if _, dup := seen[secret]; dup { continue } - seen[f.Secret] = struct{}{} - out = append(out, secretRule{secret: f.Secret, ruleID: f.RuleID}) + seen[secret] = struct{}{} + out = append(out, secretRule{secret: secret, ruleID: f.RuleID}) } sort.Slice(out, func(i, j int) bool { @@ -340,6 +341,24 @@ func pendingSecrets(findings []Finding, skip map[string]struct{}, isPlaceholder return out } +// trimPartialEscape removes a trailing backslash that starts a JSON escape +// sequence the secret does not include. +// +// The scanner sees JSON-encoded text, where every backslash starts or completes +// an escape. A rule whose character class allows a backslash can end its match +// on the first character of an escape such as `\"` or `\n`. Replacing that +// secret would leave the rest of the escape behind, the leaf would no longer +// decode, and the whole leaf would be replaced. An odd run of trailing +// backslashes is the sign of that: an even run is a set of complete `\\` +// escapes, which are part of the secret. +func trimPartialEscape(secret string) string { + run := len(secret) - len(strings.TrimRight(secret, `\`)) + if run%2 == 1 { + return secret[:len(secret)-1] + } + return secret +} + // rewriter walks a decoded JSON value tree replacing secrets in eligible string // leaves. type rewriter struct { diff --git a/internal/redaction/redaction_test.go b/internal/redaction/redaction_test.go index ec19ee4be..28374dc5a 100644 --- a/internal/redaction/redaction_test.go +++ b/internal/redaction/redaction_test.go @@ -150,6 +150,22 @@ func TestRedact(t *testing.T) { mustNotContain: []string{"before", "after"}, mustContain: []string{"[REDACTED:r1]"}, }, + { + name: "secret ending on the backslash of an escape keeps the leaf", + doc: `{"a":"{\"url\":\"https://h/x?sig=SEC\"}"}`, + findings: []Finding{{RuleID: "r1", Secret: `SEC\`}}, + wantReplacements: 1, + wantByRule: map[string]int{"r1": 1}, + mustContain: []string{`{"a":"{\"url\":\"https://h/x?sig=[REDACTED:r1]\"}"}`}, + }, + { + name: "secret ending on a complete escaped backslash keeps it", + doc: `{"a":"SEC\\ after"}`, + findings: []Finding{{RuleID: "r1", Secret: `SEC\\`}}, + wantReplacements: 1, + wantByRule: map[string]int{"r1": 1}, + mustContain: []string{`{"a":"[REDACTED:r1] after"}`}, + }, { name: "protected path is left alone and recorded", doc: `{"keepme":"SEC","other":"plain"}`, From 672b20754649fcd2b584295efd7c0b0d0c7fce2f Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Fri, 2 Oct 2026 00:00:36 +0200 Subject: [PATCH 2/2] fix(redaction): read escapes as units when cutting a JWT secret A complete `\\` escape followed by n, r, t or a quote was taken as the start of a terminator escape, so the cut was made one character too early. The cut now reads escapes from the left as units. Tests also cover the `\r` and `\t` terminators. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 9dfd2d47-4405-4b51-adb4-a990f965c3a7 --- internal/redaction/betterleaks.go | 23 +++++++++------- internal/redaction/betterleaks_test.go | 37 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index a6da4d059..357312c33 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -344,10 +344,6 @@ func appendSecret(dst []Finding, ruleID, secret string) []Finding { // jwtRuleID is the default ruleset's rule for JSON Web Tokens. const jwtRuleID = "jwt" -// jwtTerminatorEscapes are the JSON escapes that decode to a character a JWT -// cannot contain, so a JWT ends where one of them starts. -var jwtTerminatorEscapes = []string{`\"`, `\n`, `\r`, `\t`} - // cutAtTerminatorEscape removes the text from the first whitespace or quote // escape onwards. // @@ -357,14 +353,23 @@ var jwtTerminatorEscapes = []string{`\"`, `\n`, `\r`, `\t`} // JWT is base64url, so the part before such an escape is the whole token. Other // escapes, such as `\/` or `\\`, are kept: they are not proof that the token has // ended, and keeping them can only over-redact. +// +// Escapes are read as units from the left, so the second backslash of a `\\` +// escape is never taken as the start of another escape. func cutAtTerminatorEscape(secret string) string { - end := len(secret) - for _, esc := range jwtTerminatorEscapes { - if i := strings.Index(secret, esc); i >= 0 && i < end { - end = i + for i := 0; i+1 < len(secret); i++ { + if secret[i] != '\\' { + continue + } + switch secret[i+1] { + case '"', 'n', 'r', 't': + return secret[:i] } + // Skip the escaped character, so that it is not read as the start of + // an escape. + i++ } - return secret[:end] + return secret } // span is a half-open byte range [start, end) of a text. diff --git a/internal/redaction/betterleaks_test.go b/internal/redaction/betterleaks_test.go index 225d745bf..efbe4fdfe 100644 --- a/internal/redaction/betterleaks_test.go +++ b/internal/redaction/betterleaks_test.go @@ -308,6 +308,16 @@ func TestRedactJWTKeepsSurroundingText(t *testing.T) { leaf: "https://uploads.example.com/o/a/b?signature=" + fakeJWT + "\nnext line", want: "https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]\nnext line", }, + { + name: "followed by an escaped carriage return", + leaf: "https://uploads.example.com/o/a/b?signature=" + fakeJWT + "\rnext line", + want: "https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]\rnext line", + }, + { + name: "followed by an escaped tab", + leaf: "https://uploads.example.com/o/a/b?signature=" + fakeJWT + "\tnext line", + want: "https://uploads.example.com/o/a/b?signature=[REDACTED:jwt]\tnext line", + }, { // The literal backslash is a complete `\\` escape, which is kept in // the secret: the leaf loses that one character but not its context. @@ -339,6 +349,33 @@ func TestRedactJWTKeepsSurroundingText(t *testing.T) { } } +// TestCutAtTerminatorEscape checks that the cut is made only at a real escape. +// In JSON-encoded text a backslash that belongs to a `\\` escape does not start +// another escape, so `\\n` is a literal backslash followed by the letter n. +func TestCutAtTerminatorEscape(t *testing.T) { + testCases := []struct { + name string + secret string + want string + }{ + {name: "no escape", secret: `plain`, want: `plain`}, + {name: "escaped quote", secret: `quote\"after`, want: `quote`}, + {name: "escaped newline", secret: `newline\nafter`, want: `newline`}, + {name: "escaped carriage return", secret: `return\rafter`, want: `return`}, + {name: "escaped tab", secret: `tab\tafter`, want: `tab`}, + {name: "escaped backslash then n is not a newline", secret: `bs\\nafter`, want: `bs\\nafter`}, + {name: "escaped backslash then quote escape", secret: `bsquote\\\"after`, want: `bsquote\\`}, + {name: "escaped slash is kept", secret: `slash\/kept\nafter`, want: `slash\/kept`}, + {name: "trailing lone backslash is left to the engine", secret: `lone\`, want: `lone\`}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, cutAtTerminatorEscape(tc.secret)) + }) + } +} + // TestDefaultScannerUsesRE2 pins the regex engine. The library default is the // standard library engine, which is several times slower on the default ruleset // and made large sessions take tens of minutes to redact.