fix(redaction): strip C0/C1 bytes before shape matching

internal/redaction/redaction.go

@@ -7,8 +7,10 @@ import (
 	"reflect"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 	"unicode"
+	"unicode/utf8"
 )
 
 const (
@@ -68,12 +70,58 @@ var sensitiveKeys = map[string]struct{}{
 	"zero_api_key":          {},
 }
 
+// ctrlGap matches C0/C1 bytes (Cc other than tab/LF/CR, plus lone Latin-1 C1)
+// between characters of a secret shape. Matching stays on the original string:
+// a deleted control is never a join, so \b still treats wordchar+control as a
+// boundary and tokens that were never adjacent stay that way. Tab/LF/CR are
+// excluded so log line structure is unchanged. \x{FFFD} lets the regexp locate
+// a lone invalid UTF-8 byte; validSecretControlGaps subsequently accepts only
+// raw C1 bytes and rejects a real, valid UTF-8 U+FFFD rune.
+const ctrlGap = `[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\x80-\x9f\x{FFFD}]*`
+
+// ctrlLit quotes s as a regexp literal with ctrlGap strictly between runes, so
+// a NUL/ESC/C1 may split the literal without letting a match end on a gap.
+func ctrlLit(s string) string {
+	var b strings.Builder
+	b.Grow(len(s) * (1 + len(ctrlGap)))
+	first := true
+	for _, r := range s {
+		if !first {
+			b.WriteString(ctrlGap)
+		}
+		b.WriteString(regexp.QuoteMeta(string(r)))
+		first = false
+	}
+	return b.String()
+}
+
+func ctrlJoin(parts ...string) string {
+	return strings.Join(parts, ctrlGap)
+}
+
+// secretBody keeps gaps strictly between the minimum required body characters.
+// Once an unbounded shape has reached that high-confidence minimum, its
+// optional tail stays contiguous: a later control is a suffix delimiter rather
+// than permission to absorb the following token into the secret (which could
+// feed unrelated suffix text into the OpenAI kebab-case exception).
+func secretBody(class string, minimum int, unbounded bool) string {
+	if minimum <= 0 {
+		return ""
+	}
+	quantifier := strconv.Itoa(minimum - 1)
+	body := class + `(?:` + ctrlGap + class + `){` + quantifier + `}`
+	if unbounded {
+		body += class + `*`
+	}
+	return body
+}
+
 // openaiKeyPattern mirrors secrets.Scan's broad sk- body. Known OpenAI
 // prefixes (sk-proj-/sk-svcacct-/sk-admin-) are always redacted; other sk-
 // digit-free matches with an interior hyphen are left alone (kebab-case false
 // positives), while digit-free legacy sk- credentials are still redacted.
 // Applied via ReplaceAllStringFunc rather than the plain list below.
-var openaiKeyPattern = regexp.MustCompile(`\bsk-[A-Za-z0-9_-]{20,}`)
+var openaiKeyPattern = regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("sk-"), secretBody(`[A-Za-z0-9_-]`, 20, true)))
 
 // textSecretPatterns mirror secrets.Scan for end-boundary behavior and the
 // shared high-confidence shapes. A leading \b keeps each pattern from firing
@@ -84,16 +132,18 @@ var openaiKeyPattern = regexp.MustCompile(`\bsk-[A-Za-z0-9_-]{20,}`)
 // (not in secrets.Scan); ASIA temporary access keys are kept alongside AKIA.
 // openai keys are handled separately (digit filter). JWT has a strict form
 // (both segments start with eyJ) and a looser three-segment form.
+// ctrlGap between shape characters keeps NUL/ESC/C1 split secrets matching
+// without stripping those bytes out of the subject first.
 var textSecretPatterns = []*regexp.Regexp{
-	regexp.MustCompile(`\bsk-ant-(?:api\d{2}-)?[A-Za-z0-9_-]{20,}`),
-	regexp.MustCompile(`\bgithub_pat_[A-Za-z0-9_]{22,}`),
-	regexp.MustCompile(`\bgh[pousr]_[A-Za-z0-9]{36,}`),
-	regexp.MustCompile(`\bglpat-[A-Za-z0-9_-]{12,}`),
-	regexp.MustCompile(`\bAIza[0-9A-Za-z\-_]{35,}`),
-	regexp.MustCompile(`\bxox[baprs]-[A-Za-z0-9-]{10,}`),
-	regexp.MustCompile(`\b(?:AKIA|ASIA)[A-Z0-9]{16}`),
-	regexp.MustCompile(`\beyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}`),
-	regexp.MustCompile(`\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}`),
+	regexp.MustCompile(`\b` + ctrlLit("sk-ant-") + ctrlGap + `(?:` + ctrlJoin(ctrlLit("api"), `\d`, `\d`, `-`) + ctrlGap + `)?` + secretBody(`[A-Za-z0-9_-]`, 20, true)),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("github_pat_"), secretBody(`[A-Za-z0-9_]`, 22, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("gh"), `[pousr]`, `_`, secretBody(`[A-Za-z0-9]`, 36, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("glpat-"), secretBody(`[A-Za-z0-9_-]`, 12, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("AIza"), secretBody(`[0-9A-Za-z\-_]`, 35, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("xox"), `[baprs]`, `-`, secretBody(`[A-Za-z0-9-]`, 10, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(`(?:`+ctrlLit("AKIA")+`|`+ctrlLit("ASIA")+`)`, secretBody(`[A-Z0-9]`, 16, false))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("eyJ"), secretBody(`[A-Za-z0-9_-]`, 10, true), `\.`, ctrlLit("eyJ"), secretBody(`[A-Za-z0-9_-]`, 10, true), `\.`, secretBody(`[A-Za-z0-9_-]`, 10, true))),
+	regexp.MustCompile(`\b` + ctrlJoin(ctrlLit("eyJ"), secretBody(`[A-Za-z0-9_-]`, 10, true), `\.`, secretBody(`[A-Za-z0-9_-]`, 10, true), `\.`, secretBody(`[A-Za-z0-9_-]`, 10, true))),
 }
 
 var (
@@ -170,8 +220,90 @@ func keyLooksSensitive(normalized string) bool {
 	return false
 }
 
+// stripControlBytes removes C0/C1 controls (Cc other than tab, LF, and CR).
+// Used to normalize an already-matched secret so prefix/digit checks see the
+// rejoined shape. It is matching-time only and must not be applied to
+// RedactString's input or return value. Tab/LF/CR stay. Lone Latin-1 C1 bytes
+// (0x80–0x9F, invalid UTF-8) are stripped too; UTF-8 continuation bytes are
+// not, because they are not controls.
+func stripControlBytes(s string) string {
+	for i := 0; i < len(s); {
+		c := s[i]
+		if c < 0x80 {
+			if c != '\t' && c != '\n' && c != '\r' && (c < 0x20 || c == 0x7F) {
+				return stripControlBytesFrom(s, i)
+			}
+			i++
+			continue
+		}
+		if c <= 0x9F {
+			// 0x80–0x9F at a rune boundary is a lone C1 byte, not UTF-8.
+			return stripControlBytesFrom(s, i)
+		}
+		r, size := utf8.DecodeRuneInString(s[i:])
+		if unicode.IsControl(r) {
+			return stripControlBytesFrom(s, i)
+		}
+		i += size
+	}
+	return s
+}
+
+func stripControlBytesFrom(s string, start int) string {
+	var b strings.Builder
+	b.Grow(len(s))
+	b.WriteString(s[:start])
+	for i := start; i < len(s); {
+		c := s[i]
+		if c < 0x80 {
+			if c != '\t' && c != '\n' && c != '\r' && (c < 0x20 || c == 0x7F) {
+				i++
+				continue
+			}
+			b.WriteByte(c)
+			i++
+			continue
+		}
+		if c <= 0x9F {
+			i++
+			continue
+		}
+		r, size := utf8.DecodeRuneInString(s[i:])
+		if unicode.IsControl(r) {
+			i += size
+			continue
+		}
+		b.WriteString(s[i : i+size])
+		i += size
+	}
+	return b.String()
+}
+
+// validSecretControlGaps disambiguates RuneError matches at the byte boundary.
+// Go's regexp engine represents both a malformed single byte and a legitimate
+// U+FFFD rune as RuneError. Only raw invalid C1 bytes (0x80-0x9F) are supported
+// gaps; a valid UTF-8 replacement rune, or another malformed byte, must keep
+// the candidate split and prevent redaction.
+func validSecretControlGaps(match string) bool {
+	for i := 0; i < len(match); {
+		r, size := utf8.DecodeRuneInString(match[i:])
+		if r != utf8.RuneError {
+			i += size
+			continue
+		}
+		if size != 1 || match[i] < 0x80 || match[i] > 0x9F {
+			return false
+		}
+		i++
+	}
+	return true
+}
+
 func RedactString(value string, options Options) string {
 	replacement := replacement(options)
+	// Match on the original string. Shape patterns allow C0/C1 gaps between
+	// characters so a split secret still matches; stripping first would join
+	// tokens that were never adjacent and make \b miss a leading wordchar.
 	redacted := value
 	if len(options.ExtraSecretValues) > 0 {
 		secrets := append([]string{}, options.ExtraSecretValues...)
@@ -224,18 +356,31 @@ func RedactString(value string, options Options) string {
 		}
 		return parts[1] + parts[2] + "=" + replacement
 	})
-	// openai keys first so the filter can drop kebab-case false positives
-	// before any other pattern rewrites nearby text.
+	// Match high-confidence specialized shapes first. In particular, the broad
+	// sk- pattern may reach its minimum before a control inside a longer
+	// Anthropic key; letting the Anthropic shape consume that split first avoids
+	// leaving a recognizable credential suffix behind.
+	for _, pattern := range textSecretPatterns {
+		redacted = pattern.ReplaceAllStringFunc(redacted, func(match string) string {
+			if !validSecretControlGaps(match) {
+				return match
+			}
+			return replacement
+		})
+	}
+	// Apply the broad OpenAI shape after specialized keys so its kebab-case
+	// false-positive filter considers only the matched key, never suffix text.
 	redacted = openaiKeyPattern.ReplaceAllStringFunc(redacted, func(match string) string {
-		if !knownOpenAIKeyPrefix(match) && !secretMatchHasDigit(match) &&
-			strings.Contains(strings.TrimPrefix(match, "sk-"), "-") {
+		if !validSecretControlGaps(match) {
+			return match
+		}
+		normalized := stripControlBytes(match)
+		if !knownOpenAIKeyPrefix(normalized) && !secretMatchHasDigit(normalized) &&
+			strings.Contains(strings.TrimPrefix(normalized, "sk-"), "-") {
 			return match
 		}
 		return replacement
 	})
-	for _, pattern := range textSecretPatterns {
-		redacted = pattern.ReplaceAllString(redacted, replacement)
-	}
 	return redacted
 }
 

internal/redaction/redaction_test.go

@@ -141,3 +141,131 @@ func containsCircular(v any) bool {
 	}
 	return false
 }
+
+func TestRedactStringCatchesSecretsSplitByControlBytes(t *testing.T) {
+	// Unsplit passing is not coverage: a NUL/ESC/C1 in the body splits the
+	// shape so the patterns miss it unless matching allows those controls as
+	// gaps between body characters (without joining unrelated tokens).
+	const prefix = "sk-ant-api03-"
+	const body = "abcdefghijklmnopqrstuvwxyz"
+	unsplit := prefix + body
+	if got := RedactString(unsplit, Options{}); strings.Contains(got, body) {
+		t.Fatalf("unsplit secret not redacted (test setup): %q", got)
+	}
+
+	cases := []struct {
+		name  string
+		split string
+	}{
+		{name: "NUL", split: "\x00"},
+		{name: "ESC", split: "\x1b"},
+		{name: "C1", split: "\x9b"},
+		{name: "UTF-8 C1", split: string(rune(0x9B))},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			inputs := []struct {
+				placement string
+				input     string
+			}{
+				{placement: "prefix-body boundary", input: prefix + tc.split + body},
+				{placement: "inside body", input: prefix + body[:13] + tc.split + body[13:]},
+			}
+			for _, input := range inputs {
+				t.Run(input.placement, func(t *testing.T) {
+					got := RedactString(input.input, Options{})
+					if strings.Contains(got, body) {
+						t.Fatalf("secret split by %s %s leaked in %q", tc.name, input.placement, got)
+					}
+					if strings.Contains(got, prefix) {
+						t.Fatalf("secret prefix split by %s %s leaked in %q", tc.name, input.placement, got)
+					}
+					if !strings.Contains(got, RedactedSecret) {
+						t.Fatalf("expected %q after %s %s split, got %q", RedactedSecret, tc.name, input.placement, got)
+					}
+				})
+			}
+		})
+	}
+}
+
+func TestRedactStringPreservesControlAfterCredential(t *testing.T) {
+	const secret = "sk-ant-api03-abcdefghijklmnopqrstuvwxyz"
+	input := "key=" + secret + "\x00path/one.go\x00path/two.go"
+	want := "key=" + RedactedSecret + "\x00path/one.go\x00path/two.go"
+	if got := RedactString(input, Options{}); got != want {
+		t.Fatalf("terminal credential separator changed:\n got=%q\nwant=%q", got, want)
+	}
+}
+
+func TestRedactStringSuffixCannotDisableOpenAIKeyMatch(t *testing.T) {
+	const secret = "sk-aaaaaaaaaaaaaaaaaaaabcdefgh"
+	input := "key " + secret + "\x1bkebab-case tail"
+	want := "key " + RedactedSecret + "\x1bkebab-case tail"
+	if got := RedactString(input, Options{}); got != want {
+		t.Fatalf("suffix changed OpenAI key classification:\n got=%q\nwant=%q", got, want)
+	}
+}
+
+func TestRedactStringDistinguishesInvalidC1FromValidReplacementRune(t *testing.T) {
+	const prefix = "sk-ant-api03-"
+	const body = "abcdefghijklmnopqrstuvwxyz"
+
+	invalidC1 := prefix + body[:13] + "\x9b" + body[13:]
+	if got := RedactString(invalidC1, Options{}); got != RedactedSecret {
+		t.Fatalf("raw invalid C1 split was not redacted: %q", got)
+	}
+
+	validReplacement := prefix + body[:13] + "\uFFFD" + body[13:]
+	got := RedactString(validReplacement, Options{})
+	if !strings.Contains(got, "\uFFFD"+body[13:]) {
+		t.Fatalf("valid U+FFFD was treated as a control gap: %q", got)
+	}
+}
+
+func TestRedactStringPreservesAllowedWhitespaceAndUTF8(t *testing.T) {
+	input := "safe\tline\nnext\rfinal café"
+	if got := RedactString(input, Options{}); got != input {
+		t.Fatalf("unexpected normalization: %q", got)
+	}
+}
+
+func TestRedactStringWordcharBeforeNULAnthropicKey(t *testing.T) {
+	// Matching on a control-stripped copy joins "id42" and the key, so \b in
+	// textSecretPatterns misses and the secret leaks. Matching on the original
+	// treats the NUL as a boundary; leaked must be false.
+	const secret = "sk-ant-api03-abcdefghijklmnopqrstuvwxyz"
+	if got := RedactString(secret, Options{}); strings.Contains(got, "sk-ant-api03-") {
+		t.Fatalf("unsplit secret not redacted (test setup): %q", got)
+	}
+	input := "id42\x00" + secret
+	got := RedactString(input, Options{})
+	leaked := strings.Contains(got, secret) || strings.Contains(got, "sk-ant-api03-")
+	if leaked {
+		t.Fatalf("wordchar-before-NUL+anthropic-key leaked=true out=%q", got)
+	}
+	if !strings.Contains(got, RedactedSecret) {
+		t.Fatalf("wordchar-before-NUL+anthropic-key leaked=false want %q, got %q", RedactedSecret, got)
+	}
+}
+
+func TestRedactStringControlBytesWithoutSecretStayIdentical(t *testing.T) {
+	// scrubResultSecrets sets Result.Redacted when RedactString's result !=
+	// Output. Stripping is matching-time only: no-secret control bytes must
+	// remain byte-identical so Redacted stays false.
+	cases := []struct {
+		name  string
+		input string
+	}{
+		{name: "form feed in source", input: "package main\n\ffunc main() {}\n"},
+		{name: "Windows-1252 quotes", input: "Don\x92t \x93quote\x94 me\n"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := RedactString(tc.input, Options{})
+			if got != tc.input {
+				t.Fatalf("no-secret input not byte-identical:\n in=%q\nout=%q", tc.input, got)
+			}
+		})
+	}
+}