fix(redaction): reduce false positives in AI coding session redaction

app/cli/internal/trace/README.md

@@ -85,6 +85,7 @@ How the evidence is protected before it leaves the machine.
 |---------|---------------|-------------|--------|--------------|------------|
 | Secret redaction | Secrets in the session evidence and in the spec materials are replaced with a placeholder before upload. | Yes | Yes | Yes | Yes |
 | Media skipped by redaction | The secret redaction does not scan base64 media (images, PDFs) in the transcript, so it is faster and does not break the media. | Yes | Unknown | Not applicable | Not applicable |
+| Allow markers in redaction | A secret on a line that carries an inline `gitleaks:allow` or `betterleaks:allow` marker, such as a test fixture the agent read, is not redacted. The marker covers only its own line. | Yes | Yes | Yes | Yes |
 
 Why not Yes:
 

internal/redaction/betterleaks.go

@@ -18,13 +18,15 @@ package redaction
 import (
 	"context"
 	"crypto/sha256"
+	_ "embed"
 	"fmt"
 	"runtime"
 	"slices"
 	"strconv"
 	"strings"
 	"sync"
 
+	"github.com/betterleaks/betterleaks/config"
 	"github.com/betterleaks/betterleaks/detect"
 	betterleaksregexp "github.com/betterleaks/betterleaks/regexp"
 	"github.com/betterleaks/betterleaks/regexp/re2"
@@ -67,6 +69,18 @@ type betterleaksScanner struct {
 	chunkSize int
 }
 
+// rulesConfig is the betterleaks default ruleset, extended with allowlists for
+// values the generic rules match but which are not credentials.
+//
+//go:embed betterleaks.toml
+var rulesConfig string
+
+// RulesetVersion identifies what the scanner detects. Bump it whenever
+// 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 = 2
+
 var (
 	defaultScanner = sync.OnceValues(newBetterleaksScanner)
 	// useRE2 selects the regex engine. It is process-wide and only affects
@@ -88,21 +102,30 @@ func DefaultScanner() (Scanner, error) {
 }
 
 func newBetterleaksScanner() (*betterleaksScanner, error) {
+	return newScannerWithRules(rulesConfig)
+}
+
+// newScannerWithRules builds a scanner from a betterleaks TOML configuration.
+func newScannerWithRules(rules string) (*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.
-	d, err := detect.NewDetectorDefaultConfig()
+	cfg, err := config.ParseTOMLString(rules, "")
 	if err != nil {
-		return nil, fmt.Errorf("loading the default secret scanning rules: %w", err)
+		return nil, fmt.Errorf("loading the secret scanning rules: %w", err)
 	}
 
-	// A session transcript is text the model was free to write, so an in-band
-	// "betterleaks:allow" or "gitleaks:allow" must not switch redaction off.
+	// Validation stays off: validating would reach out to third-party APIs to
+	// check whether a candidate credential is live, and crafting a material
+	// must not do that.
+	d := detect.NewDetectorContext(context.Background(), cfg, detect.ValidationOptions{})
+
+	// The detector checks an allow marker against the whole line of its match.
+	// A string leaf of the scanned document is a single line, so one marker
+	// would suppress every finding in a whole tool result. Redactor checks the
+	// marker against the line of each occurrence instead, when asked to.
 	d.IgnoreGitleaksAllow = true
 	// Findings must carry the verbatim secret: it is what we search for in the
 	// document in order to replace it.
@@ -169,10 +192,12 @@ func (s *betterleaksScanner) scanChunks(ctx context.Context, text string, chunks
 		// and the rule for that component is marked as not independently
 		// reportable — so the primary finding names the harmless public
 		// identifier while the actual credential arrives only as a component.
-		// Both halves have to be redacted.
+		// Both halves have to be redacted. An optional component is context
+		// that only raises the confidence of the finding, such as the username
+		// next to a generic password, and is not a secret.
 		for _, set := range result.Finding.ComponentSets {
 			for _, component := range set.Components {
-				if component == nil {
+				if component == nil || component.Optional {
 					continue
 				}
 				perChunk[chunk] = appendSecret(perChunk[chunk], component.RuleID, component.Secret)
@@ -332,17 +357,29 @@ 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 {
+	switch ruleID {
+	case jwtRuleID:
 		secret = cutAtTerminatorEscape(secret)
+	case genericPasswordRuleID:
+		secret = cutAtLineEscape(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"
+const (
+	// jwtRuleID is the default ruleset's rule for JSON Web Tokens.
+	jwtRuleID = "jwt"
+	// genericPasswordRuleID is the default ruleset's rule for a password
+	// assigned to a password key. Its unquoted value stops at whitespace in
+	// source text, but in JSON-encoded text a line break is the escape `\n`, so
+	// the match runs into the next line. That swallowed, for instance, the line
+	// break after a placeholder the transcript already showed, and reported the
+	// placeholder as a new secret.
+	genericPasswordRuleID = "generic-password"
+)
 
 // cutAtTerminatorEscape removes the text from the first whitespace or quote
 // escape onwards.
@@ -357,12 +394,24 @@ const jwtRuleID = "jwt"
 // 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 {
+	return cutAtEscape(secret, `"nrt`)
+}
+
+// cutAtLineEscape removes the text from the first whitespace escape onwards. A
+// password is not cut at a quote escape: its value often starts with one,
+// `password: \"...\"`, which is part of it.
+func cutAtLineEscape(secret string) string {
+	return cutAtEscape(secret, "nrt")
+}
+
+// cutAtEscape removes the text from the first escape of one of the given
+// characters onwards.
+func cutAtEscape(secret, terminators string) string {
 	for i := 0; i+1 < len(secret); i++ {
 		if secret[i] != '\\' {
 			continue
 		}
-		switch secret[i+1] {
-		case '"', 'n', 'r', 't':
+		if strings.IndexByte(terminators, secret[i+1]) >= 0 {
 			return secret[:i]
 		}
 		// Skip the escaped character, so that it is not read as the start of

internal/redaction/betterleaks.toml

@@ -0,0 +1,64 @@
+# Secret scanning rules for redaction: the betterleaks default ruleset, with
+# allowlists for values the generic rules match in AI coding sessions but which
+# are not credentials.
+#
+# Every allowlist targets one or more low-precision rules and tests the found
+# secret, or the rule's match. None tests the whole line: a string leaf of the
+# scanned document is a single line, so a line allowlist would drop every
+# finding in a whole tool result.
+
+[extend]
+useDefault = true
+
+[[allowlists]]
+description = "UUIDs are identifiers, not credentials"
+targetRules = ["generic-api-key", "generic-password"]
+regexes = ['''^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$''']
+
+[[allowlists]]
+description = "Content digests, such as sha256:<hex>"
+targetRules = ["generic-api-key"]
+regexTarget = "match"
+regexes = ['''(?i)\bsha(?:1|224|256|384|512):[0-9a-f]{40,128}\b''']
+
+[[allowlists]]
+description = "Code expressions, test durations and the bare key word, which are not password values"
+targetRules = ["generic-password"]
+# The scanned text is JSON-encoded, so a match of this rule can run on through
+# an escaped line break, or through a \n written in source code, which reaches
+# the scanner as \\n. The scanner cuts the secret at the first, but only after
+# these regexes ran, so each one also accepts such a tail.
+regexes = [
+    # An expression such as t.password||n.password in minified JavaScript:
+    # property accesses, and only those, joined by logical operators.
+    '''^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+(?:(?:\|\||&&|\?\?)[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+)+(?:\\+[nrt].*)?$''',
+    # A property access such as e.password or config.db_password, alone or
+    # followed by logical operators and identifiers, as in e.password||n.
+    '''^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*\.\w*(?i:passw(?:or)?d|pwd)\w*(?:(?:\|\||&&|\?\?)[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*)*(?:\\+[nrt].*)?$''',
+    # A redaction placeholder the transcript shows again, in the current or
+    # the legacy format.
+    '''^\[(?:CHAINLOOP_TRACE_)?REDACTED(?::[^\]\s]*)?\](?:\\+[nrt].*)?$''',
+    # Documentation such as (password: `password`).
+    '''(?i)^(?:password|passwd|pwd)(?:\\+[nrt].*)?$''',
+    # The duration of a Go test, as in "--- PASS: TestPassword: (0.01s)".
+    '''^\([0-9]+(?:\.[0-9]+)?(?:ns|us|µs|ms|s|m|h)\)(?:\\+[nrt].*)?$''',
+]
+
+[[allowlists]]
+description = "Placeholders in documentation, such as Bearer YOUR_API_TOKEN"
+targetRules = ["curl-auth-header"]
+regexes = ['''^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$''']
+
+[[allowlists]]
+description = "A host and port read as the user and password of a URI, such as http://localhost:8000@example.com"
+targetRules = ["generic-credential-uri"]
+regexTarget = "match"
+# The "user" must be localhost, an IPv4 address or a dotted host name, so a
+# numeric password next to a plain user name, as in admin:12345@, is still
+# redacted.
+regexes = ['''(?i)://(?:localhost|[0-9]{1,3}(?:\.[0-9]{1,3}){3}|[a-z0-9-]+(?:\.[a-z0-9-]+)+):[0-9]{1,5}@''']
+
+[[allowlists]]
+description = "An elided password in documentation, such as http://user:…@example.com"
+targetRules = ["generic-credential-uri"]
+regexes = ['''^(?:…|\.{3})$''']

internal/redaction/betterleaks_test.go

@@ -79,8 +79,9 @@ func TestDefaultScannerDetects(t *testing.T) {
 			wantRule: "github-pat",
 		},
 		{
-			// A session transcript is attacker-influenceable text, so neither
-			// in-band bypass marker may suppress a finding.
+			// The detector would apply a marker to its whole line, which is a
+			// whole string leaf of the document. Redactor applies it to the
+			// line of each occurrence instead, so the scanner reports it.
 			name:     "gitleaks:allow must not suppress the finding",
 			text:     `"content": "` + awsPair + ` // gitleaks:allow"`,
 			wantRule: "aws-access-token",