sec(logsafe,jobs): scrub connection-URI userinfo from persisted/logged errors - #61
Merged
Merged
Conversation
…d errors
Closes SEC-WORKER FINDING-3 (CWE-532, P1) + FINDING-6 (CWE-209, P3).
Driver / gRPC errors that bubble up from probes (real_prober.go) or
propagation runs (propagation_runner.go) can embed the full connection
URI verbatim — mongo-driver in particular emits
"mongodb://user:secret@host/..." in ApplyURI errors. Today those error
strings flow into three persistent surfaces:
1. resources.degraded_reason (DB column → dashboard banner)
2. audit_log.metadata.error (JSON column → admin/audit UI)
3. slog.Error (New Relic Logs)
4. pending_propagations.last_error (DB column → propagation audit)
…with no userinfo scrubbing.
Fix: extend `logsafe` with `ScrubURL(s string) string` that strips the
`scheme://userinfo@` segment from any embedded URI in conservative
RFC 3986 syntax. Apply at two persistence boundaries:
- provisioner_reconciler.go:probeErrString — DB / audit / NR Logs surface
- propagation_runner.go:truncatePropagationError — DB / audit surface
Scrub BEFORE truncation so a chopped half-URI never escapes.
Tests:
- TestScrubURL (10 subcases: empty, no-url, pg/redis/mongo with userinfo,
SRV, two URIs in one error, case-insensitive scheme, idempotent)
- TestScrubURL_NoLeakOfSecretSuffix — literal password substring MUST
NOT appear in scrubbed output (regression guard against regex
regressions that over-scope the userinfo group)
Production LOC delta: 24 (well under the 50-LOC threshold for a P1 hotfix).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes SEC-WORKER FINDING-3 (CWE-532, P1) + FINDING-6 (CWE-209, P3) from
/tmp/qa-session/shared/SEC-INBOX.md.Why
Driver / gRPC errors that bubble up from probes (
real_prober.go) or propagation runs (propagation_runner.go) can embed the full connection URI verbatim —mongo-driverin particular emitsmongodb://user:secret@host/...inoptions.ApplyURIerrors.Those error strings flow into four persistent surfaces with no userinfo scrubbing:
resources.degraded_reason(DB column → dashboard banner)audit_log.metadata.error(JSON column → admin/audit UI)slog.Error(shipped to New Relic Logs)pending_propagations.last_error(DB column → propagation audit)An attacker with NR Logs read access can grep
degraded_reasonfor:+@to harvest secrets.What
internal/logsafe/logsafe.go(+48 LOC) — addsScrubURL(s string) stringthat stripsscheme://userinfo@segments via a conservative RFC 3986 regex. Idempotent. Case-insensitive on scheme. Catches multiple embedded URIs in one string.internal/logsafe/logsafe_test.go(+60 LOC) — 10-subcase table-driven test + secret-suffix regression guard.internal/jobs/provisioner_reconciler.go(+14 LOC) — applieslogsafe.ScrubURLinprobeErrString(the SEC-WORKER FINDING-3 site).internal/jobs/propagation_runner.go(+10 LOC) — applieslogsafe.ScrubURLintruncatePropagationErrorBEFORE truncation (so a chopped half-URI never escapes — SEC-WORKER FINDING-6 site).Production LOC delta: 24 (well under the 50-LOC threshold for a P1 hotfix).
Verification
make gategreen (build + vet +go test ./... -short -count=1).go test ./internal/logsafe/ -v -run ScrubURL→ 11/11 pass.TestScrubURL_NoLeakOfSecretSuffixasserts the literal password substring does NOT appear in the scrubbed output (regex regression guard).Tracking: SEC-WORKER FINDING-3 + FINDING-6 in
/tmp/qa-session/shared/SEC-INBOX.md.🤖 Generated with Claude Code