Skip to content

Improve database credential redaction after betterleaks#1045

Merged
gtrrz-victor merged 7 commits intomainfrom
redact
Apr 28, 2026
Merged

Improve database credential redaction after betterleaks#1045
gtrrz-victor merged 7 commits intomainfrom
redact

Conversation

@peyton-alt
Copy link
Copy Markdown
Contributor

@peyton-alt peyton-alt commented Apr 27, 2026

Summary

Redact for database credential gaps that remain after #1043 switched redaction to Betterleaks.

  • Redacts low-entropy database connection strings that Betterleaks/default entropy detection miss, including Postgres keyword DSNs, SQL Server strings, ODBC strings, and JDBC/DB URL query-password forms.
  • Redacts bounded DB credential values such as DB_PASSWORD=..., PGPASSWORD=..., and REDIS_PASSWORD=... while preserving placeholders and already-redacted values.
  • Redacts structured JSON database password fields like {"db":{"password":"...","host":"...","user":"..."}} without redacting unrelated fields that happen to share the same value.
  • Keeps over-redaction guardrails for non-credential URLs, SSH remotes, prose, paths, IDs, placeholders, and already-redacted values.
  • Updates security/privacy docs to describe the split between Betterleaks, existing credentialed URI detection, and the additional DB-specific redaction layer.

Scope After #1043

  • Private key block redaction is covered by Betterleaks default rules, so this PR does not add custom private-key block logic.
  • Generic scheme://user:password@host URI userinfo redaction is covered by the existing credentialedURIPattern in redact/redact.go, so the DB-specific detector no longer owns userinfo URLs.
  • This PR focuses on DB DSNs, DB URL query passwords, bounded DB credential values, and structured JSON DB password fields.

Also Included

Two unrelated concurrency fixes that surfaced during this work:

  • cmd/entire/cli/logging/logger.go — holds the read lock across l.Log so Init/Close cannot close the log writer mid-write. Adds TestLogging_ConcurrentInitAndLog as a regression guard (now exercises Close too).
  • cmd/entire/cli/dispatch_tui_test.go — drops t.Parallel() on two tests that mutate the package-level newDispatchProgram factory; adds a doc comment on the var so future contributors don't reintroduce the race.

Bundled here rather than split into separate PRs because both are small.

Testing

  • GOCACHE=/tmp/entire-go-cache go test ./redact -count=1
  • mise run check
  • mise run lint

Notes

mise run check and mise run lint were run outside the sandbox because golangci-lint fetches its schema, Go uses the user cache, and integration tests bind localhost httptest servers.


Note

Medium Risk
Medium risk because it changes always-on redaction behavior (could still under- or over-redact real user data) and adjusts shared logger state access patterns under concurrency.

Overview
Improves secret redaction coverage for database credentials. redact.String now adds always-on detectors for DB connection strings (JDBC, keyword DSNs, semicolon/ODBC-style, and DB URLs with password/pwd query params) plus bounded credential assignments like DB_PASSWORD=..., while explicitly skipping placeholders/already-redacted values.

Makes JSON/JSONL redaction more field-aware. JSON replacement tracking now keys replacements by JSON field so skipped fields (e.g., session_id) don't get redacted due to value collisions elsewhere, and structured credential objects/keys (normalized variants like "DB Password") get their password values redacted without touching unrelated fields.

Hardens CLI logging under concurrency. Removes helper getters and reads logger/currentSessionID under the package RW lock inside log, and adds a concurrent Init+log stress test. Docs are updated to describe the expanded redaction layers.

Reviewed by Cursor Bugbot for commit 3115fad. Configure here.

Copilot AI review requested due to automatic review settings April 27, 2026 13:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens Entire CLI’s automatic transcript redaction by adding more secret detectors (beyond entropy + gitleaks) and expanding regression coverage to reduce both under- and over-redaction in common credential formats.

Changes:

  • Add redaction for full private key blocks, credentialed URIs, database DSNs/URLs, and bounded password-like key/value pairs.
  • Enhance JSON/JSONL redaction to support key-scoped replacements for structured credential fields.
  • Expand unit tests and update security/privacy documentation to reflect the new detection layers.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
redact/redact.go Adds new secret detectors (private keys, credentialed URIs, connection strings, bounded credentials) and extends JSONL replacement handling.
redact/redact_test.go Adds extensive coverage for new redaction behaviors, including guardrails against over-redaction.
docs/security-and-privacy.md Updates documentation to describe the expanded automatic secret redaction methods.

Comment thread redact/redact_test.go
Comment thread redact/redact.go
Comment thread redact/redact.go Outdated
Entire-Checkpoint: d271418e7c6f
Entire-Checkpoint: 7242e212bf5b
@peyton-alt peyton-alt changed the title Improve automatic secret redaction Improve database credential redaction Apr 27, 2026
Entire-Checkpoint: 2dc818c76ce5
@peyton-alt
Copy link
Copy Markdown
Contributor Author

@BugBot review

Comment thread redact/redact.go
Entire-Checkpoint: de0e66c1ffc3
@peyton-alt
Copy link
Copy Markdown
Contributor Author

@BugBot review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3115fad. Configure here.

@peyton-alt peyton-alt marked this pull request as ready for review April 27, 2026 15:39
@peyton-alt peyton-alt requested a review from a team as a code owner April 27, 2026 15:39
Entire-Checkpoint: 93f541fa4dd4
@peyton-alt peyton-alt changed the title Improve database credential redaction Improve database credential redaction after betterleaks Apr 27, 2026
Hot-path simplifications in redact/redact.go: replace per-replacement
regex compilation with a manual JSON scan (replaceKeyedJSONValue),
derive lowercase placeholder forms from RedactedPlaceholder, early-out
on inputs without '=' before running connection-string rules, and
single-pass key normalization in isCredentialJSONObject.

Coverage and review follow-ups: broaden credentialValuePattern boundary
so prefixed env vars (APP_DB_PASSWORD, PROD_MYSQL_PWD) match; add a
Close goroutine to TestLogging_ConcurrentInitAndLog to exercise the
close-during-write race; document why log() holds the read lock across
l.Log; document why dispatch tests using newDispatchProgram cannot use
t.Parallel; clarify docs/security-and-privacy.md that connection-string
redaction is conditional on a real (non-placeholder) password.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 084f6db5e688
@gtrrz-victor gtrrz-victor merged commit 08e59af into main Apr 28, 2026
9 checks passed
@gtrrz-victor gtrrz-victor deleted the redact branch April 28, 2026 10:24
@Soph Soph mentioned this pull request Apr 28, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants