From 141b781ea2614f6d3b8b6912b7aac367ae01d2f8 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 30 Sep 2026 13:30:49 +0200 Subject: [PATCH 1/2] perf(redaction): scan AI sessions in parallel chunks with RE2 Secret redaction of a long AI coding session took tens of minutes, long enough for the remote to close the git connection during pre-push. - Select the betterleaks RE2 engine. The library defaults to the standard library engine, which is several times slower on the default ruleset. The betterleaks CLI defaults to RE2. - Split the scanned text into line-aligned chunks of about 100 KB. With one fragment for the whole document, the keyword prefilter selected nearly every rule for every byte. Each chunk re-reads the trailing 64 lines of its predecessor, so that composite rules still match pairs that straddle a boundary. - Scan the chunks concurrently and join the findings in chunk order, so that the redacted output stays deterministic. Refs: #3498 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: ba0ac486-c03f-4584-99da-b10a229310bc --- go.mod | 2 + go.sum | 4 + internal/redaction/betterleaks.go | 160 +++++++++++++-- internal/redaction/betterleaks_test.go | 264 +++++++++++++++++++++++++ 4 files changed, 413 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index d8290c1f7..0c75afb6c 100644 --- a/go.mod +++ b/go.mod @@ -185,6 +185,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/ssooidc v1.43.0 // indirect github.com/bahlo/generic-list-go v0.2.0 // indirect github.com/beorn7/perks v1.0.1 // indirect + github.com/betterleaks/go-re2 v1.11.0-betterleaks.3 // indirect github.com/blang/semver v3.5.1+incompatible // indirect github.com/bmatcuk/doublestar v1.3.4 // indirect github.com/bmatcuk/doublestar/v4 v4.8.1 // indirect @@ -427,6 +428,7 @@ require ( github.com/transparency-dev/merkle v0.0.2 // indirect github.com/ulikunitz/xz v0.5.15 // indirect github.com/valyala/fastjson v1.6.10 // indirect + github.com/wasilibs/wazero-helpers v0.0.0-20250123031827-cd30c44769bb // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect diff --git a/go.sum b/go.sum index 2725a6827..d436cc5e9 100644 --- a/go.sum +++ b/go.sum @@ -220,6 +220,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/betterleaks/betterleaks v1.8.1 h1:tTSQAdQe+S4MF69o6VtpRDlml1b73F1xNUUA3PcrHiI= github.com/betterleaks/betterleaks v1.8.1/go.mod h1:M8pR9QvWFx+iWcRH2HG6Qe1wP9DLXhHt75iMNyZ7Y+s= +github.com/betterleaks/go-re2 v1.11.0-betterleaks.3 h1:89efHsGJOOCl1KH6GxBtixZNBkNzYIlKPtmQJizCBXU= +github.com/betterleaks/go-re2 v1.11.0-betterleaks.3/go.mod h1:UzEofSJPdMG4B9tgB7ExxybS/jEV4nZxVdd2T5PcsEY= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= @@ -1284,6 +1286,8 @@ github.com/valyala/fastjson v1.6.10 h1:/yjJg8jaVQdYR3arGxPE2X5z89xrlhS0eGXdv+ADT github.com/valyala/fastjson v1.6.10/go.mod h1:e6FubmQouUNP73jtMLmcbxS6ydWIpOfhz34TSfO3JaE= github.com/vektah/gqlparser/v2 v2.5.37 h1:jbb1Ilv+xBklV6653tKb4oVUupPNTLb5LmrnBKVI12Y= github.com/vektah/gqlparser/v2 v2.5.37/go.mod h1:9O4Ox6Ngd3Y12bMD3w6i3CRQXh8W1oC1q0m6olCymDM= +github.com/wasilibs/wazero-helpers v0.0.0-20250123031827-cd30c44769bb h1:gQ+ZV4wJke/EBKYciZ2MshEouEHFuinB85dY3f5s1q8= +github.com/wasilibs/wazero-helpers v0.0.0-20250123031827-cd30c44769bb/go.mod h1:jMeV4Vpbi8osrE/pKUxRZkVaA0EX7NZN0A9/oRzgpgY= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 9ba4c7924..788c95c6b 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -18,10 +18,38 @@ package redaction import ( "context" "fmt" + "runtime" + "slices" + "strconv" + "strings" "sync" "github.com/betterleaks/betterleaks/detect" + betterleaksregexp "github.com/betterleaks/betterleaks/regexp" + "github.com/betterleaks/betterleaks/regexp/re2" "github.com/betterleaks/betterleaks/sources" + "golang.org/x/sync/errgroup" +) + +const ( + // defaultChunkSize is how much new text each fragment handed to the detector + // carries. A rule only runs on a fragment that contains one of its keywords, + // and a whole transcript contains nearly all of them, so scanning it as a + // single fragment runs nearly every rule over every byte. Small fragments let + // the keyword prefilter skip most rules. It matches the chunk size betterleaks + // itself reads files with. + defaultChunkSize = 100_000 + + // chunkOverlapLines is how many trailing lines of a chunk the next chunk + // scans again. Composite rules only fire when a component is found within a + // window of lines around the primary match, so the overlap has to cover the + // widest window in the ruleset or a pair straddling a boundary is missed. + // TestChunkOverlapCoversComponentWindows keeps it in step with the ruleset. + chunkOverlapLines = 64 + + // attrChunk tags each fragment, and therefore each finding, with the index of + // its chunk, so that findings can be put back in document order. + attrChunk = "chainloop.redaction.chunk" ) // betterleaksScanner detects secrets with the betterleaks default ruleset. @@ -30,21 +58,40 @@ type betterleaksScanner struct { // (ValidationCounts is cleared at the start of every Run), so concurrent // scans on a shared detector would race. Redaction is not on a hot // concurrent path, so serialising is cheaper than owning a detector per - // caller: constructing one compiles the whole ruleset. + // caller: constructing one compiles the whole ruleset. A single scan still + // uses every core, by scanning its chunks concurrently. mu sync.Mutex detector *detect.Detector + // chunkSize is the amount of new text per fragment; see defaultChunkSize. + chunkSize int } -var defaultScanner = sync.OnceValues(newBetterleaksScanner) +var ( + defaultScanner = sync.OnceValues(newBetterleaksScanner) + // useRE2 selects the regex engine. It is process-wide and only affects + // patterns compiled afterwards, so it has to run before the ruleset loads. + useRE2 = sync.OnceFunc(func() { betterleaksregexp.SetEngine(re2.RE2{}) }) +) // DefaultScanner returns the process-wide betterleaks-backed scanner. // Constructing a detector compiles several hundred regexes and builds a keyword // trie, so it is built once, lazily, and only for attestations that need it. func DefaultScanner() (Scanner, error) { - return defaultScanner() + s, err := defaultScanner() + if err != nil { + // Returned bare so that a failure is a nil interface, not a nil pointer + // wrapped in one. + return nil, err + } + return s, nil } -func newBetterleaksScanner() (Scanner, error) { +func newBetterleaksScanner() (*betterleaksScanner, error) { + // The library defaults to the standard library engine, which is several + // times slower on this ruleset; the betterleaks CLI defaults to RE2 for the + // same reason. + useRE2() + // Validation stays off, which is what the default constructor gives us: // validating would reach out to third-party APIs to check whether a candidate // credential is live, and crafting a material must not do that. @@ -73,7 +120,7 @@ func newBetterleaksScanner() (Scanner, error) { // bound. Results are read from the Run iterator instead. d.SkipFindingAppend = true - return &betterleaksScanner{detector: d}, nil + return &betterleaksScanner{detector: d, chunkSize: defaultChunkSize}, nil } func (s *betterleaksScanner) Scan(ctx context.Context, text string) ([]Finding, error) { @@ -84,13 +131,27 @@ func (s *betterleaksScanner) Scan(ctx context.Context, text string) ([]Finding, s.mu.Lock() defer s.mu.Unlock() - var findings []Finding - for result := range s.detector.Run(ctx, stringSource{text: text}) { + source := chunkedSource{text: text, chunks: splitLines(text, s.chunkSize, chunkOverlapLines)} + + // Chunks are scanned concurrently, so findings arrive interleaved. They are + // collected per chunk and joined in chunk order, which keeps the result + // deterministic: a single goroutine scans each chunk, so a chunk's own + // findings arrive in the detector's order. That matters because when two + // rules report the same secret the first finding names its placeholder, and + // the placeholder is part of the redacted document's digest. Duplicates from + // the chunk overlap are left for the caller, which deduplicates by secret. + perChunk := make([][]Finding, len(source.chunks)) + for result := range s.detector.Run(ctx, source) { if result.Err != nil { return nil, fmt.Errorf("scanning for secrets: %w", result.Err) } - findings = appendSecret(findings, result.Finding.RuleID, result.Finding.Secret) + chunk, err := strconv.Atoi(result.Finding.Attr(attrChunk)) + if err != nil || chunk < 0 || chunk >= len(perChunk) { + return nil, fmt.Errorf("scanning for secrets: finding with an invalid chunk index %q", result.Finding.Attr(attrChunk)) + } + + perChunk[chunk] = appendSecret(perChunk[chunk], result.Finding.RuleID, result.Finding.Secret) // Composite rules report only their primary match. An AWS access key id, // for instance, only matches when a secret access key is found near it, @@ -103,7 +164,7 @@ func (s *betterleaksScanner) Scan(ctx context.Context, text string) ([]Finding, if component == nil { continue } - findings = appendSecret(findings, component.RuleID, component.Secret) + perChunk[chunk] = appendSecret(perChunk[chunk], component.RuleID, component.Secret) } } } @@ -114,7 +175,7 @@ func (s *betterleaksScanner) Scan(ctx context.Context, text string) ([]Finding, return nil, err } - return findings, nil + return slices.Concat(perChunk...), nil } // appendSecret records a locatable secret. A finding without one cannot be @@ -126,16 +187,81 @@ func appendSecret(dst []Finding, ruleID, secret string) []Finding { return append(dst, Finding{RuleID: ruleID, Secret: secret}) } -// stringSource adapts an in-memory document to the source interface the scanner -// consumes. Run is the only scanning entry point that is neither deprecated nor +// span is a half-open byte range [start, end) of a text. +type span struct { + start, end int +} + +// splitLines cuts text into chunks that each carry at least size bytes of new +// text, ending at a line end. Each chunk after the first also begins with up to +// overlap trailing lines of its predecessor, capped at size bytes so that a run +// of very long lines is not scanned again and again. +// +// A line is never split, even when it is longer than size. That is what keeps a +// secret whole, and it relies on how Redactor renders the document: as indented +// JSON, where a line holds at most one string leaf and newlines inside strings +// are escaped, so no secret it can locate spans a line break. +func splitLines(text string, size, overlap int) []span { + if len(text) <= size { + return []span{{0, len(text)}} + } + + var chunks []span + for start, pos := 0, 0; pos < len(text); { + for fresh := pos; pos < len(text) && pos-fresh < size; { + if nl := strings.IndexByte(text[pos:], '\n'); nl >= 0 { + pos += nl + 1 + } else { + pos = len(text) + } + } + chunks = append(chunks, span{start, pos}) + + // Walk back over the trailing lines the next chunk re-reads. + start = pos + for range overlap { + if start == 0 { + break + } + lineStart := strings.LastIndexByte(text[:start-1], '\n') + 1 + if pos-lineStart > size { + break + } + start = lineStart + } + } + return chunks +} + +// chunkedSource adapts an in-memory document to the source interface the +// scanner consumes, yielding its chunks from several goroutines at once. Run +// supports that: betterleaks' own file source scans files concurrently the same +// way. Run is the only scanning entry point that is neither deprecated nor // context-blind, and it takes a source rather than a string. -type stringSource struct { - text string +type chunkedSource struct { + text string + chunks []span } -func (s stringSource) Fragments(ctx context.Context, yield sources.FragmentsFunc) error { - if err := ctx.Err(); err != nil { +func (s chunkedSource) Fragments(ctx context.Context, yield sources.FragmentsFunc) error { + // The group's context is cancelled once Wait returns, so the caller's is the + // one checked at the end. + g, gctx := errgroup.WithContext(ctx) + g.SetLimit(runtime.GOMAXPROCS(0)) + + for i, c := range s.chunks { + if gctx.Err() != nil { + break + } + g.Go(func() error { + fragment := sources.Fragment{Raw: s.text[c.start:c.end]} + fragment.SetAttr(attrChunk, strconv.Itoa(i)) + return yield(fragment, nil) + }) + } + + if err := g.Wait(); err != nil { return err } - return yield(sources.Fragment{Raw: s.text}, nil) + return ctx.Err() } diff --git a/internal/redaction/betterleaks_test.go b/internal/redaction/betterleaks_test.go index 31cc1832e..9efc14778 100644 --- a/internal/redaction/betterleaks_test.go +++ b/internal/redaction/betterleaks_test.go @@ -17,11 +17,14 @@ package redaction import ( "context" + "regexp" + "strconv" "strings" "sync" "testing" betterleaksconfig "github.com/betterleaks/betterleaks/config" + betterleaksregexp "github.com/betterleaks/betterleaks/regexp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -276,6 +279,267 @@ func TestRedactCredentialInURIConverges(t *testing.T) { 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. +func TestDefaultScannerUsesRE2(t *testing.T) { + _, err := DefaultScanner() + require.NoError(t, err) + + assert.Equal(t, "re2", betterleaksregexp.Version()) +} + +func TestSplitLines(t *testing.T) { + testCases := []struct { + name string + text string + size int + overlap int + want []string + }{ + { + name: "empty text is a single empty chunk", + text: "", + size: 4, + want: []string{""}, + }, + { + name: "text that fits is a single chunk", + text: "a\nb\n", + size: 16, + want: []string{"a\nb\n"}, + }, + { + name: "splits only at line ends", + text: "a\nb\nc\nd\n", + size: 4, + want: []string{"a\nb\n", "c\nd\n"}, + }, + { + name: "each chunk re-reads the last lines of the previous one", + text: "a\nb\nc\nd\n", + size: 4, + overlap: 1, + want: []string{"a\nb\n", "b\nc\nd\n"}, + }, + { + name: "a final line without a newline is kept", + text: "a\nb\nc", + size: 4, + want: []string{"a\nb\n", "c"}, + }, + { + name: "a line longer than the chunk size is never split", + text: "aaaaaaaa\nb\n", + size: 4, + want: []string{"aaaaaaaa\n", "b\n"}, + }, + { + // Without the cap, a run of huge lines would be scanned again by + // every following chunk. + name: "overlap is capped at the chunk size in bytes", + text: "aaaaaaaa\nb\nc\n", + size: 4, + overlap: 1, + want: []string{"aaaaaaaa\n", "b\nc\n"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var got []string + for _, c := range splitLines(tc.text, tc.size, tc.overlap) { + got = append(got, tc.text[c.start:c.end]) + } + assert.Equal(t, tc.want, got) + }) + } +} + +// TestSplitLinesCoversText checks that chunking never drops content: the chunks +// start at the beginning, end at the end, each one moves forward, and none leaves +// a gap after its predecessor. +func TestSplitLinesCoversText(t *testing.T) { + var sb strings.Builder + for i := range 500 { + sb.WriteString(strings.Repeat("x", i%37)) + sb.WriteString("\n") + } + sb.WriteString("tail without newline") + text := sb.String() + + for _, overlap := range []int{0, 1, 5, 64} { + chunks := splitLines(text, 256, overlap) + require.Greater(t, len(chunks), 1) + + assert.Equal(t, 0, chunks[0].start) + assert.Equal(t, len(text), chunks[len(chunks)-1].end) + for i := 1; i < len(chunks); i++ { + prev, cur := chunks[i-1], chunks[i] + assert.Equal(t, byte('\n'), text[prev.end-1], "overlap=%d: chunk %d must end at a line end", overlap, i-1) + assert.LessOrEqual(t, cur.start, prev.end, "overlap=%d: chunk %d leaves a gap", overlap, i) + assert.Greater(t, cur.end, prev.end, "overlap=%d: chunk %d adds no new text", overlap, i) + if overlap == 0 { + assert.Equal(t, prev.end, cur.start, "chunks must not overlap without an overlap") + } + } + } +} + +// TestChunkOverlapCoversComponentWindows is the gate on chunkOverlapLines. A +// composite rule only fires when its components sit within a window of lines +// around the primary match, so a chunk has to re-read at least that many lines of +// its predecessor or a pair straddling the boundary is missed. Checked against the +// shipped ruleset so that upgrading it cannot silently widen a window. +func TestChunkOverlapCoversComponentWindows(t *testing.T) { + cfg, err := betterleaksconfig.Default() + require.NoError(t, err) + + lines := regexp.MustCompile(`(\d+)L`) + for id, rule := range cfg.Rules { + for _, c := range rule.Components { + if c.Within == "" { + // Unbounded: the component may be anywhere in the fragment. With + // chunking that means anywhere in the same chunk, which is how + // betterleaks itself treats files. + continue + } + matches := lines.FindAllStringSubmatch(c.Within, -1) + require.NotEmpty(t, matches, "rule %q component %q has a window %q without a line bound; chunking only preserves line windows", id, c.RuleID, c.Within) + for _, m := range matches { + n, err := strconv.Atoi(m[1]) + require.NoError(t, err) + assert.LessOrEqual(t, n, chunkOverlapLines, "rule %q component %q window %q exceeds the chunk overlap", id, c.RuleID, c.Within) + } + } + } +} + +// chunkedScanner returns a scanner with a small chunk size, so that tests can +// exercise chunk boundaries without multi-megabyte inputs. +func chunkedScanner(t *testing.T, size int) *betterleaksScanner { + t.Helper() + s, err := newBetterleaksScanner() + require.NoError(t, err) + s.chunkSize = size + return s +} + +func TestChunkedScanDetects(t *testing.T) { + const size = 512 + filler := func(n int) string { + var sb strings.Builder + for i := range n { + sb.WriteString(` "content": "ordinary transcript line ` + strconv.Itoa(i) + `",` + "\n") + } + return sb.String() + } + + keyLine := ` "content": "AWS_ACCESS_KEY_ID=` + fakeAWSKey + `",` + "\n" + secretLine := ` "content": "AWS_SECRET_ACCESS_KEY=` + fakeAWSSecret + `",` + "\n" + // Pads the text so that the next chunk boundary falls right after keyLine. + pad := ` "content": "` + strings.Repeat("p", size-len(keyLine)-len(` "content": "",`)-1) + `",` + "\n" + + testCases := []struct { + name string + text string + wantRules []string + }{ + { + name: "secrets in chunks far apart are all found", + text: ` "repository": "https://oauth2:` + fakeGitHubPAT + `@github.com/example/repo.git",` + "\n" + filler(200) + ` "content": "ANTHROPIC_API_KEY=` + fakeAnthropicKey + `"` + "\n", + wantRules: []string{"github-pat", "anthropic-api-key"}, + }, + { + // aws-access-token needs its secret access key within 5 lines. The + // boundary falls between the two, so only the overlap lets the chunk + // holding the secret also see the key id. + name: "a composite pair straddling a chunk boundary is found", + text: pad + keyLine + secretLine + filler(50), + wantRules: []string{"aws-access-token", "aws-secret-access-key"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + scanner := chunkedScanner(t, size) + require.Greater(t, len(splitLines(tc.text, size, chunkOverlapLines)), 1, "the test text must span several chunks") + + findings, err := scanner.Scan(context.Background(), tc.text) + require.NoError(t, err) + + var rules []string + for _, f := range findings { + rules = append(rules, f.RuleID) + assert.Contains(t, tc.text, f.Secret) + } + for _, want := range tc.wantRules { + assert.Contains(t, rules, want) + } + }) + } +} + +// TestChunkedScanIsDeterministic guards the order of findings. Chunks are +// scanned concurrently, but when two rules report the same secret the first +// finding names the placeholder, so the order has to be stable or the redacted +// document's digest would change from one run to the next. +func TestChunkedScanIsDeterministic(t *testing.T) { + scanner := chunkedScanner(t, 256) + + // A distinct token per chunk, so that the order in which concurrently + // scanned chunks report is visible in the result rather than deduplicated + // away. + var sb strings.Builder + const tokens = 60 + for i := range tokens { + pat := fakeGitHubPAT[:len(fakeGitHubPAT)-2] + string(rune('A'+i%26)) + string(rune('a'+i/26)) + sb.WriteString(` "repository": "https://oauth2:` + pat + `@github.com/example/repo.git",` + "\n") + sb.WriteString(` "content": "` + strings.Repeat("ordinary transcript text ", 10) + `",` + "\n") + } + text := sb.String() + + first, err := scanner.Scan(context.Background(), text) + require.NoError(t, err) + require.GreaterOrEqual(t, len(first), tokens) + + for range 10 { + again, err := scanner.Scan(context.Background(), text) + require.NoError(t, err) + assert.Equal(t, first, again) + } +} + +// TestChunkedRedact runs the whole redaction over a document that spans many +// chunks and checks that nothing is left behind. +func TestChunkedRedact(t *testing.T) { + scanner := chunkedScanner(t, 256) + + var sb strings.Builder + sb.WriteString(`{"data":{"raw_session":{"main":[`) + for i := range 200 { + if i > 0 { + sb.WriteString(",") + } + content := "ordinary transcript text " + strconv.Itoa(i) + switch i { + case 3: + content = "git clone https://oauth2:" + fakeGitHubPAT + "@github.com/example/repo.git" + case 197: + content = "ANTHROPIC_API_KEY=" + fakeAnthropicKey + } + sb.WriteString(`{"role":"user","content":"` + content + `"}`) + } + sb.WriteString(`]}}}`) + + out, report, err := New(scanner).Redact(context.Background(), []byte(sb.String())) + require.NoError(t, err) + assert.NotContains(t, string(out), fakeGitHubPAT) + assert.NotContains(t, string(out), fakeAnthropicKey) + assert.Equal(t, 1, report.ByRule["github-pat"]) + assert.Equal(t, 1, report.ByRule["anthropic-api-key"]) +} + func BenchmarkDefaultScannerInit(b *testing.B) { for b.Loop() { if _, err := newBetterleaksScanner(); err != nil { From f4c954c31748b238af8fbde7472437826aabdcec Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 30 Sep 2026 14:46:20 +0200 Subject: [PATCH 2/2] fix(redaction): always re-read the last line of a chunk The chunk overlap was capped at the chunk size in bytes, so a trailing line longer than a chunk was not scanned again. A composite match on that line, with its component at the start of the next chunk, was then missed. The last line is now always part of the overlap. The byte cap still applies to the lines before it. The straddling test cases now also assert that their pair is split across a chunk boundary when the overlap is left out. Refs: #3498 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: ba0ac486-c03f-4584-99da-b10a229310bc --- internal/redaction/betterleaks.go | 11 ++++--- internal/redaction/betterleaks_test.go | 40 +++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/internal/redaction/betterleaks.go b/internal/redaction/betterleaks.go index 788c95c6b..85464be7a 100644 --- a/internal/redaction/betterleaks.go +++ b/internal/redaction/betterleaks.go @@ -194,8 +194,9 @@ type span struct { // splitLines cuts text into chunks that each carry at least size bytes of new // text, ending at a line end. Each chunk after the first also begins with up to -// overlap trailing lines of its predecessor, capped at size bytes so that a run -// of very long lines is not scanned again and again. +// overlap trailing lines of its predecessor. Only the last of those may be +// longer than size bytes; the others must fit within size bytes together, so +// that a run of very long lines is not scanned again and again. // // A line is never split, even when it is longer than size. That is what keeps a // secret whole, and it relies on how Redactor renders the document: as indented @@ -217,14 +218,16 @@ func splitLines(text string, size, overlap int) []span { } chunks = append(chunks, span{start, pos}) - // Walk back over the trailing lines the next chunk re-reads. + // Walk back over the trailing lines the next chunk re-reads. The last + // line always is, however long: a composite match on it may have its + // component at the start of the next chunk. start = pos for range overlap { if start == 0 { break } lineStart := strings.LastIndexByte(text[:start-1], '\n') + 1 - if pos-lineStart > size { + if pos-lineStart > size && start != pos { break } start = lineStart diff --git a/internal/redaction/betterleaks_test.go b/internal/redaction/betterleaks_test.go index 9efc14778..418347440 100644 --- a/internal/redaction/betterleaks_test.go +++ b/internal/redaction/betterleaks_test.go @@ -335,13 +335,22 @@ func TestSplitLines(t *testing.T) { want: []string{"aaaaaaaa\n", "b\n"}, }, { - // Without the cap, a run of huge lines would be scanned again by - // every following chunk. - name: "overlap is capped at the chunk size in bytes", + // A composite primary on a long line can have its component on the + // next line, so the last line is re-read however long it is. + name: "the last line is re-read even when it is longer than the chunk size", text: "aaaaaaaa\nb\nc\n", size: 4, overlap: 1, - want: []string{"aaaaaaaa\n", "b\nc\n"}, + want: []string{"aaaaaaaa\n", "aaaaaaaa\nb\nc\n"}, + }, + { + // Without the cap, a run of huge lines would be scanned again by + // every following chunk. + name: "earlier lines are re-read only within the chunk size in bytes", + text: "aaaaaaaa\nbbbbbbbb\nc\n", + size: 4, + overlap: 2, + want: []string{"aaaaaaaa\n", "aaaaaaaa\nbbbbbbbb\n", "bbbbbbbb\nc\n"}, }, } @@ -439,11 +448,17 @@ func TestChunkedScanDetects(t *testing.T) { secretLine := ` "content": "AWS_SECRET_ACCESS_KEY=` + fakeAWSSecret + `",` + "\n" // Pads the text so that the next chunk boundary falls right after keyLine. pad := ` "content": "` + strings.Repeat("p", size-len(keyLine)-len(` "content": "",`)-1) + `",` + "\n" + // A key id at the end of a line longer than a whole chunk: the line is a + // chunk of its own, and its secret starts the next one. + longKeyLine := ` "content": "` + strings.Repeat("p ", size) + `AWS_ACCESS_KEY_ID=` + fakeAWSKey + `",` + "\n" testCases := []struct { name string text string wantRules []string + // straddle, when set, names two substrings the test relies on being in + // different chunks once the overlap is left out. + straddle [2]string }{ { name: "secrets in chunks far apart are all found", @@ -457,6 +472,13 @@ func TestChunkedScanDetects(t *testing.T) { name: "a composite pair straddling a chunk boundary is found", text: pad + keyLine + secretLine + filler(50), wantRules: []string{"aws-access-token", "aws-secret-access-key"}, + straddle: [2]string{fakeAWSKey, fakeAWSSecret}, + }, + { + name: "a composite pair straddling a boundary after a line longer than a chunk is found", + text: filler(1) + longKeyLine + secretLine + filler(50), + wantRules: []string{"aws-access-token", "aws-secret-access-key"}, + straddle: [2]string{fakeAWSKey, fakeAWSSecret}, }, } @@ -465,6 +487,16 @@ func TestChunkedScanDetects(t *testing.T) { scanner := chunkedScanner(t, size) require.Greater(t, len(splitLines(tc.text, size, chunkOverlapLines)), 1, "the test text must span several chunks") + // Pins the premise: without the overlap no chunk holds both halves, so + // the case really goes through the overlap rather than past it. + if tc.straddle != [2]string{} { + for _, c := range splitLines(tc.text, size, 0) { + chunk := tc.text[c.start:c.end] + require.False(t, strings.Contains(chunk, tc.straddle[0]) && strings.Contains(chunk, tc.straddle[1]), + "the pair must straddle a chunk boundary") + } + } + findings, err := scanner.Scan(context.Background(), tc.text) require.NoError(t, err)