redaction: catch compound secret keys + non-bearer auth schemes; stop scanner over-redaction

[gnanam1990]

Audit of internal/redaction + internal/secrets surfaced three verified gaps in the secret-redaction last line of defense. The configured provider key is already exact-scrubbed; these affect secondary secrets that appear in tool/command output.

Fixes

1. Compound secret keys (leak) — IsSensitiveKey was exact-match, so db_password, session_secret, stripe_secret_key, auth_token, csrf_token, ssh_private_key, backup_api_key … passed through. New keyLooksSensitive uses conservative structural rules. Critically token-count-safe: max_tokens / prompt_tokens / token_count and ordinary primary_key / public_key / cache_key are not redacted (token only triggers as a singular trailing segment; bare key never triggers).
2. Auth header schemes (leak) — Authorization: now redacts token / apikey / digest / negotiate / oauth / aws4-hmac-sha256, not just bearer/basic.
3. Scanner over-redaction (correctness) — added \b to the prefixed patterns so kebab-case words (task-…, ask-…, risk-…) no longer match as a fake key, while real secrets (preceded by space/=/:/quote/start) still match.

Deliberately not shipped

A free-text key: value colon pattern — it mangled file.go:line: message output (a filename's secret segment read as a sensitive key), the same over-redaction class fix #3 addresses. Structured paths (RedactValue, plus assign/JSON/query in RedactString) are covered; free-text colon-form is left as a known limitation, with TestRedactString_PreservesFileLineColons guarding against re-introduction.

Tests

audit_fixes_test.go (compound-key + token-count matrix, auth schemes, file:line guard, RedactValue nesting) and boundary_test.go (scanner over-redaction negatives + real-secret positives). gofmt · go vet · staticcheck · -race on redaction+secrets+verify · 9 consumer packages — all green.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved sensitive field detection with enhanced heuristics for identifying secret-like keys
  • Expanded HTTP header redaction to support additional authentication schemes
  • Fixed secret detection patterns to reduce false positives by requiring word boundaries in pattern matching

• Tests

  • Added comprehensive test coverage for redaction behavior and secret detection

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Free

Run ID: 0eee7a9d-d685-42e5-819e-3ff1fecaf39e

📥 Commits

Reviewing files that changed from the base of the PR and between eb60874 and 39045f2.

📒 Files selected for processing (4)

• internal/redaction/audit_fixes_test.go
• internal/redaction/redaction.go
• internal/secrets/boundary_test.go
• internal/secrets/scanner.go

---

Walkthrough

Redaction heuristics in redaction.go are extended with segment-based IsSensitiveKey logic and a broader headerPattern covering more auth schemes. Secret-detection regexes in scanner.go gain leading \b word-boundary anchors. New tests in both packages validate false-positive prevention (token counts, kebab-case words) and true-positive detection (real secrets).

Changes

Secret Detection Accuracy Improvements

Layer / File(s)	Summary
IsSensitiveKey heuristics and headerPattern expansion internal/redaction/redaction.go	headerPattern regex broadened to match token, apikey, api-key, digest, negotiate, oauth, and aws4-hmac-sha256 schemes. IsSensitiveKey reworked to delegate to keyLooksSensitive, introducing secretKeySegments with underscore-segment inspection, singular token trailing rule, and api_key/private_key adjacency detection while explicitly skipping plural/count-style token fields.
Redaction audit tests internal/redaction/audit_fixes_test.go	Five new test functions covering: IsSensitiveKey classification of secret vs. token-count keys; RedactString across key=value, JSON-like, query-string, and auth-header input forms; colon-prefix (file.go:line:) preservation; and RedactValue on nested maps with mixed secret and numeric fields.
Word-boundary anchors and boundary tests internal/secrets/scanner.go, internal/secrets/boundary_test.go	Each prefixed-secret regex in patterns gains a leading \b anchor to prevent partial mid-word matches. New boundary tests assert that kebab-case ordinary words (task, ask, risk containing sk-) produce zero findings, and that realistic secret strings (OPENAI_API_KEY=sk-proj-..., etc.) are detected.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

---

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: f5e11444461b
Changed files (4): internal/redaction/audit_fixes_test.go, internal/redaction/redaction.go, internal/secrets/boundary_test.go, internal/secrets/scanner.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[anandh8x]

High: #249 still leaks credentials for multi-part authorization schemes.

internal/redaction/redaction.go:86 captures only one non-space/comma/semicolon token after Digest or AWS4-HMAC-SHA256, then internal/redaction/redaction.go:180 replaces only that captured token. Real headers such as Authorization: Digest username=..., nonce=..., response=... or AWS SigV4 Credential=..., SignedHeaders=..., Signature=... leave response / Signature visible. Please redact the full auth header value for parameterized schemes and add tests with realistic Digest and AWS headers.

Verification: diff review plus git diff --check passed for this PR.

[Vasanthdev2004]

APPROVE

Walked the 4 files (216 +, 8 -) at 39045f2 against origin/main. Three independent fixes to the redaction last line of defense, each with a tight test surface. No regressions on the existing 5 redaction tests or 7 secrets tests, and the 7 new tests all pass. The test for the explicit keyLooksSensitive negatives (token-counts, ordinary *_key fields, idempotency_key) is the right discipline for an additive heuristic.

1. Compound secret keys via keyLooksSensitive

internal/redaction/redaction.go:104-145 adds a new structural check layered behind the existing explicit sensitiveKeys map and ExtraSensitiveKeys options. The check is conservative and well-documented:

• secretKeySegments (password, passwd, passphrase, secret, credential, credentials, apikey) — fires on any underscore-delimited segment match.
• <x>_token (singular, trailing only) — catches auth_token, csrf_token, vault_token; deliberately excludes tokens (plural count) and token_<x> (prefix form) so max_tokens, prompt_tokens, token_count stay un-redacted. The i > 0 && i == len(segments)-1 guard is the right shape.
• api_key / private_key (adjacent segments) — narrow switch on the previous segment, not a bare key match, so primary_key, public_key, cache_key, foreign_key, idempotency_key stay un-redacted.

Verified the explicit test list end-to-end. db_password, session_secret, stripe_secret_key, auth_token, csrf_token, ssh_private_key, backup_api_key, aws_credential, gcp_credentials all hit the new check. max_tokens, prompt_tokens, primary_key, public_key, idempotency_key, username, message correctly stay un-redacted. The new check is purely additive — anything the explicit list or ExtraSensitiveKeys flagged before still gets flagged.

2. Auth-scheme expansion in headerPattern

internal/redaction/redaction.go:86 extends the scheme alternation from (bearer|basic) to (bearer|basic|token|apikey|api-key|digest|negotiate|oauth|aws4-hmac-sha256). Catches:

• GitHub's Authorization: token ghp_... (the most common token form, distinct from bearer)
• Custom-provider ApiKey / API-Key headers (case-insensitive match on the scheme, so ApiKey and API-Key both hit)
• Proxy / Kerberos Proxy-Authorization: Digest ... / Negotiate ...
• SigV4 Authorization: AWS4-HMAC-SHA256 Credential=.../..., SignedHeaders=..., Signature=... (the trailing ([^\s,;]+) captures the full credential+signature blob, which is the right thing to redact)
• Authorization: OAuth ... (custom OAuth realms)

The opaque-value test cases in TestRedactString_AuthHeaderSchemes isolate the scheme fix from the text-pattern fallback (the values are deliberately non-prefixed so only the header/colon logic can catch them). Solid.

3. Word-boundary anchors on scanner patterns

internal/secrets/scanner.go:30-49 adds a leading \b to all 7 prefixed patterns (AWS, GitHub, GitHub PAT, Slack, Google, OpenAI, JWT) and to the private-key block the doc comment is updated. The fix closes the over-redaction class where sk- inside task-management-and-coordination matched as a fake openai_key because the regex fired mid-word. The comment explicitly enumerates the satisfied-by-real-secrets delimiters (space, quote, =, :, start-of-string), so the regression-risk surface is documented.

TestScan_NoOverRedactionOnKebabWords covers five representative over-redaction cases (task-management-and-coordination, ask-the-user-about-this, risk-assessment-and-mitigation, disk-usage-monitoring-and-alerting, desk-booking-and-reservation). TestScan_RealSecretsStillCaught covers five positives (env-export, JSON, mid-sentence, raw AKIA..., raw ghp_...) to confirm no real coverage is lost. The real-secrets test would have caught a regression where the \b anchor is misplaced.

Design call worth flagging

The PR body's "dropped prototype" note — a free-text key: value colon pattern was prototyped and abandoned because it mangled file.go:line: message output. The decision is correct, and TestRedactString_PreservesFileLineColons is exactly the regression guard you'd want to keep that decision honest: a filename containing the literal segment secret plus a colon, line, colon, and a real token on the same line is exactly the test case where a free-text colon pattern would over-reach. Good test to keep.

Non-blocking follow-ups

1. Auth-scheme list isn't exhaustive. NTLM (Windows), Signature (some custom auth flows), and per-provider idiosyncratic schemes are not covered. The list is right for the well-known cases (the test matrix is the strongest signal of intent); an explicit opt-in via Options.ExtraAuthSchemes would be a clean way to add providers without re-cutting the regex. Not gating; can land as a follow-up.

2. keyLooksSensitive won't catch auth_token_v2 or similar future-variants. The trailing-token rule requires i == len(segments)-1, so any suffix breaks the match. The explicit list covers today's common cases, but a similar "trailing-<secret_segment> with any suffix" pattern would be more future-proof. Not a blocker; the test matrix pins today's intent.

3. The headerPattern capture group is unanchored on the right. The ([^\s,;]+) captures the value up to whitespace, comma, or semicolon, so an embedded comma in a JWT-style value (rare but possible in some custom auth schemes) would truncate the redaction. Worth a one-line note in the regex comment so the next person doesn't widen the class silently.

Verification

• internal/redaction and internal/secrets test suites pass cleanly on the branch (16 tests, including the 7 new ones). go test -count=1 ./internal/redaction/... ./internal/secrets/... -v is green.
• The new keyLooksSensitive is the only behavior change to IsSensitiveKey; the explicit map and ExtraSensitiveKeys short-circuits are unchanged, so existing callers of RedactString / RedactValue that rely on the explicit list are byte-identical to before for already-covered keys.

This is a clean hardening pass. The three fixes are independent, the test surface is complete, and the explicit negatives (token-counts, ordinary *_key, kebab-case words, file:line output) are exactly the regression class these heuristics risk introducing. Happy to see it land.

[Vasanthdev2004]

Review posted (id 4525748750, APPROVE on 39045f2).

Three independent hardening fixes, each with a tight test surface and no regressions on the existing 12 tests:

1. Compound secret keys — keyLooksSensitive (internal/redaction/redaction.go:104-145) layers a conservative structural check behind the explicit sensitiveKeys map. Catches db_password, session_secret, stripe_secret_key, auth_token, csrf_token, ssh_private_key, backup_api_key, aws_credential, gcp_credentials, etc., while deliberately excluding max_tokens, prompt_tokens, primary_key, public_key, idempotency_key. The trailing-token rule is well-shaped (singular, last-segment, and i > 0 to avoid matching a leading token_<x> count form).

2. Auth-scheme expansion — headerPattern (internal/redaction/redaction.go:86) now catches token, apikey/api-key, digest, negotiate, oauth, and aws4-hmac-sha256 in addition to bearer/basic. Closes the gap on GitHub's Authorization: token ghp_..., custom ApiKey headers, SigV4 AWS4-HMAC-SHA256, and Kerberos Negotiate/Digest flows.

3. Word-boundary anchors on scanner patterns — internal/secrets/scanner.go:30-49 adds a leading \b to all 7 prefixed patterns so sk- inside task-management-and-coordination no longer over-matches. TestScan_NoOverRedactionOnKebabWords and TestScan_RealSecretsStillCaught together pin the new behavior.

Verified go test -count=1 ./internal/redaction/... ./internal/secrets/... -v is green on the branch (16 tests, including the 7 new ones). The keyLooksSensitive change is purely additive — anything the explicit list or ExtraSensitiveKeys flagged before still gets flagged.

Three non-blocking follow-ups in the review: auth-scheme list isn't exhaustive (NTLM, custom Signature schemes), keyLooksSensitive won't catch suffix variants like auth_token_v2, and the headerPattern capture group is unanchored on the right (could truncate at an embedded comma). All three can land as follow-ups.

The TestRedactString_PreservesFileLineColons regression guard is exactly the right discipline to keep the "dropped free-text colon pattern" prototype decision honest — worth keeping.

[gnanam1990]

Good catch — fixed in f5e1144. The header pattern now redacts the entire credential after the scheme (to end of line), so multi-part schemes are fully covered: Digest …, response=… and AWS4-HMAC-SHA256 …, Signature=… are wiped, not just the first param. Added TestRedactString_MultiPartAuthSchemes with realistic Digest + AWS SigV4 headers asserting response/Signature/Credential are all gone while the scheme name stays visible. PTAL.

rereview