Repository navigation
fix(streaming): redact NATS endpoint credentials in logs - #11291
Merged
ReubenBond merged 1 commit intoSep 17, 2026
Merged
Conversation
Remove URI usernames and passwords from every configured NATS seed endpoint before writing connection and JetStream log fields. Preserve the configured client URLs for authentication and retain endpoint addresses for diagnostics. Extracted from dotnet#10798. Original implementation: 61eefc2 (Reuben Bond <reuben.bond@gmail.com>). Add focused offline exact-output regressions for user/password, token, multi-seed, and invalid endpoint descriptions.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues remain, and focused regression tests are included.
Review effort: Lite
Findings: None
What changed in this PR
This PR prevents NATS endpoint credentials from appearing in logs while preserving original URLs for authentication.
Changes:
- Adds credential-safe endpoint descriptions at all logging call sites.
- Adds regression tests for redaction, normalization, and invalid endpoints.
| File | Description |
|---|---|
test/Extensions/Orleans.Streaming.NATS.Tests/NatsConnectionManagerTests.cs |
Tests credential redaction and endpoint handling. |
src/Orleans.Streaming.NATS/Providers/NatsConnectionManager.cs |
Redacts credentials before endpoint logging. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
Contributor
Code coverage
Report-only conclusion: regressed. The current-main baseline is commit Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
ReubenBond
added a commit
to ReubenBond/orleans
that referenced
this pull request
Sep 17, 2026
Move independent NATS producer selection and dimension validation to dotnet#11287 and dotnet#11293. Move standalone redaction, credential, and journaling regression coverage to dotnet#11288, dotnet#11289, dotnet#11291, and dotnet#11292; move the independent DynamoDB README recipes to dotnet#11292. Retain Token redaction, credential binding and endpoint behavior, journaling activation, and NATS credential-safe logging as prerequisites until dotnet#11288, dotnet#11290, dotnet#11292, dotnet#11289, and dotnet#11291 are merged by humans. The original PR remains focused on Aspire configuration, resource ownership, and integration coverage.
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
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.
Problem
The NATS connection manager writes configured server URLs directly into connection and JetStream log fields. Seed URLs can contain username/password or token credentials, exposing them in logs.
Solution
Extract the endpoint-description helper from #10798 and apply it at all five existing endpoint logging call sites. Each comma-separated seed endpoint has its URI username and password removed, while its address remains available for diagnostics. Connection options retain their original URLs for authentication.
Focused offline regression tests assert exact descriptions for username/password and token credentials, multiple seed endpoints, encoded credentials, endpoint normalization, and invalid endpoint placeholders.
Rationale
This isolates the existing logging fix as a small prerequisite to the Aspire integration: one runtime source file and one focused test file. The original implementation is from commit 61eefc2 by Reuben Bond reuben.bond@gmail.com; the extraction preserves that authorship and records the source commit.
#10798 retains the helper and its shared-connection use until this prerequisite is human-merged.
Microsoft Reviewers: Open in CodeFlow