refactor(redact): trim review-talk in providers.go, document over-redaction

d929dae→main· suhaanthayyil·4d ago·2 files·+46 added/-13 removed

Shorten the provider-token file header to the durable technical content (the betterleaks composite-rule dependency and the publishable-key non-redaction rationale) and drop the issue-#1716 narrative and restated PR-review rationale.

Also document a known, accepted false-positive class: since the body charset includes underscore and the {20,} length check has no upper bound, long snake_case identifiers that merely start with a provider prefix get redacted even when they aren't secrets, including mid-word since the prefix is deliberately unanchored. Pin the tradeoff with a test so a future change doesn't "fix" it by reintroducing anchors.

Changes

2

• redact

• Mproviders.go+19/-13

• Mredact_test.go+27

3 unmodified lines
4
5
6
7
8
9
10
11
7
8
9
10
11
12
13
14
15
16
17
18
19
20
17
18
19
14 unmodified lines
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
1 unmodified line
51
52
53
48
49
50
51
54
55
56
57
58
59
60

3 unmodified lines

// Provider-specific deterministic secret patterns.
//
// These catch credential formats whose Shannon entropy can fall below the
// entropyThreshold (4.5) and whose surrounding variable name is not
// password-shaped, so none of the entropy, credentialed-URI,
// connection-string, or credential-key/value layers reliably flag them.
// The betterleaks layer also misses them in isolation: its Supabase
// Detection here is purely prefix + length based: it never depends on
// entropy or the surrounding key name, so it catches low-entropy
// credential formats the other secret layers don't reliably flag.
//
// The betterleaks layer misses these in isolation: its Supabase
// secret-key rule is a *composite* rule (RequiredRules:
// supabase-project-url) that only fires when a matching "*.supabase.co"
// URL is present in the same content, plus an entropy filter. A secret
// captured on its own therefore passes straight through.
//
// Detection here is purely prefix + length based: it never depends on
// entropy or the surrounding key name, matching the deterministic
// behaviour requested in issue #1716.
//
// Supabase (https://supabase.com/docs/guides/getting-started/api-keys):
//   - sb_secret_...      secret API key (replaces the legacy service_role
//     key; bypasses row-level security, server-side
14 unmodified lines

// catches the current and plausibly-longer future formats while rejecting
// short identifier-like collisions such as "sb_secret_short".
//
// Known false-positive class: because the body charset includes `_` and
// the {20,} length check is open-ended, sufficiently long snake_case
// identifiers that merely start with a provider prefix are redacted even
// though they aren't secrets — e.g. `sb_secret_key_rotation_handler`, or
// mid-word inside a longer identifier like `libsbp_something_long`. This is
// accepted: over-redaction is the safe direction here (see
// TestString_SupabaseProviderTokenLongIdentifierOverRedaction), and adding
// anchors or capping the body length to eliminate it would reopen the
// low-entropy under-redaction gap below.
//
// The prefix is deliberately NOT preceded by a \b word boundary. \b requires
// the character before the prefix to be a non-word char, so a secret glued to
// a preceding word character — an underscore-joined name (FOO_sb_secret_…) or,
1 unmodified line

// letter abuts the prefix (…line1\nsb_secret_…, where the byte before "sb" is
// the literal 'n') — would slip past. Because these low-entropy secrets are
// backed up by no other layer, missing them means the raw key reaches the
// checkpoint blob. Dropping the anchor is redaction-completeness-safe: the
// 10/4-char prefixes plus the {20,} floor make a legitimate mid-word collision
// vanishingly unlikely, and any high-entropy incidental match would already be
// caught by the entropy layer.
// checkpoint blob. Dropping the anchor is redaction-completeness-safe: any
// high-entropy incidental match would already be caught by the entropy layer,
// and a mid-word identifier collision (documented above) only ever
// over-redacts, never under-redacts.
var providerTokenPatterns = []*regexp.Regexp{
    regexp.MustCompile(`sb_secret_[A-Za-z0-9_-]{20,}`),
    regexp.MustCompile(`sbp_[a-z0-9_-]{20,}`),

Mredact/providers.go+19/-13

453 unmodified lines

454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486

453 unmodified lines

})
}
// TestString_SupabaseProviderTokenLongIdentifierOverRedaction documents a
// known, accepted false-positive class: because the body charset includes
// underscore and the length check is {20,} with no upper bound, sufficiently
// long snake_case identifiers that merely start with a provider prefix are
// redacted even though they are not secrets — including mid-word, since the
// prefix is deliberately not anchored (see the package comment in
// providers.go). This is intentional: over-redaction is the safe direction,
// and reintroducing a \b anchor or a body-length cap to "fix" this would
// reopen the low-entropy under-redaction gap the provider layer exists to
// close. This test pins the tradeoff so it isn't silently reversed.
func TestString_SupabaseProviderTokenLongIdentifierOverRedaction(t *testing.T) {
    t.Parallel()

assertStringRedactionCases(t, []stringRedactionCase{
        {
            name:  "long snake_case identifier starting with sb_secret_ is over-redacted",
            input: "func " + supabaseSecretPrefix() + "key_rotation_handler() {}",
            want:  "func REDACTED() {}",
        },
        {
            name:  "sbp_ mid-word inside a longer identifier is over-redacted",
            input: "call lib" + supabasePersonalPrefix() + "something_long_enough_value()",
            want:  "call libREDACTED()",
        },
    })
}

// TestJSONLContent_SupabaseSecretRedacted drives the secret through the
// field-aware JSONL path used by checkpoint condensation, mirroring a Claude
// Code transcript line where the secret lives in a message-content leaf.