diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 92d02a700..357312c33 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -332,12 +332,46 @@ 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" + +// 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. +// +// 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 { + 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 +} + // 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..efbe4fdfe 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,101 @@ 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", + }, + { + 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. + 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()) + }) + } +} + +// 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. 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"}`,