Redact connection-string passwords in generated package docs - #1410
Merged
Conversation
PackageJsonGenerator copies package XML doc comments verbatim into the frontend data JSON. Several packages document example connection strings containing a literal placeholder password (e.g. SqlServer's GetConnectionString returns "Server=host,port;User ID=sa;Password=password;TrustServerCertificate=true"). These are not real secrets, but the literal Password=<value> token trips 1ES / CredScan push protection (SEC101/037 SqlLegacyCredentials, VS403654) when the public repo is mirrored to the internal AzDO remote, blocking the deploy and deploy-vnext-release branch syncs. Add DocumentationSanitizer.RedactConnectionStringPasswords, applied to text, inline-code and code-block doc nodes, which rewrites connection-string Password=/Pwd= literals to <password>. C# default parameter values (password = null) are left untouched because the match requires no whitespace around '='. Regenerate the four affected data files accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4125af76-bbc2-4532-adee-98add8fdb6b7
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents AzDO push-protection secret scanning from blocking syncs by redacting connection-string password literals (for example, Password=password) during package-doc JSON generation, and regenerates the affected package data snapshots.
Changes:
- Added
DocumentationSanitizer.RedactConnectionStringPasswordsto redactPassword=/Pwd=values in connection-string style text. - Applied the sanitizer when building doc nodes in
CanonicalModelBuilder(text, inline code, and code blocks). - Regenerated affected
src/frontend/src/data/pkgs/*.jsonsnapshots and added unit tests for the sanitizer.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/PackageJsonGenerator.Tests/DocumentationSanitizerTests.cs | Adds coverage for redaction behavior, casing, idempotency, and non-matches. |
| src/tools/PackageJsonGenerator/Helpers/DocumentationSanitizer.cs | Introduces the regex-based sanitizer for connection-string password key/value pairs. |
| src/tools/PackageJsonGenerator/Helpers/CanonicalModelBuilder.cs | Hooks sanitization into doc-node extraction for emitted package JSON. |
| src/frontend/src/data/pkgs/CommunityToolkit.Aspire.Hosting.SurrealDb.13.4.0.json | Regenerated snapshot with redacted placeholder. |
| src/frontend/src/data/pkgs/CommunityToolkit.Aspire.Hosting.Minio.13.4.0.json | Regenerated snapshot with redacted placeholder. |
| src/frontend/src/data/pkgs/Aspire.Hosting.SqlServer.13.4.6.json | Regenerated snapshot with redacted placeholder. |
| src/frontend/src/data/pkgs/Aspire.Hosting.PostgreSQL.13.4.6.json | Regenerated snapshot with redacted placeholder. |
Comments suppressed due to low confidence (1)
src/tools/PackageJsonGenerator/Helpers/DocumentationSanitizer.cs:52
- RedactConnectionStringPasswords() currently accepts/returns non-nullable string, but the implementation explicitly supports null/empty (string.IsNullOrEmpty) and tests pass null via the null-forgiving operator. Consider making the API nullable-aware (string? in/out) to accurately model behavior and avoid callers needing
!.
public static string RedactConnectionStringPasswords(string text)
{
if (string.IsNullOrEmpty(text) || !text.Contains('='))
{
return text;
}
return ConnectionStringPasswordRegex.Replace(
text,
static match => $"{match.Groups[1].Value}={PasswordPlaceholder}");
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// </summary> | ||
| public static class DocumentationSanitizer | ||
| { | ||
| private const string PasswordPlaceholder = "<password>"; |
Member
Author
There was a problem hiding this comment.
Good catch, fixed in c9749cd. Switched the placeholder from <password> to {password} so it survives Markdown rendering. Turns out {...} is already the convention in this generated data (e.g. mysql://{user}:{password}@{host} in the MySql docs), so this also makes it consistent. Also excluded {/} from the regex value class to keep the redaction idempotent, and added a test asserting the placeholder contains no angle brackets.
Comment on lines
703
to
709
| case XText textNode: | ||
| var text = textNode.Value; | ||
| // Normalize whitespace within text runs (newlines → spaces, collapse runs) | ||
| text = text.Replace("\r\n", " ").Replace("\n", " ").Replace("\r", " "); | ||
| while (text.Contains(" ")) text = text.Replace(" ", " "); | ||
| text = DocumentationSanitizer.RedactConnectionStringPasswords(text); | ||
| if (!string.IsNullOrEmpty(text)) |
Member
Author
There was a problem hiding this comment.
Fixed in c9749cd. ExtractDocExample now runs RedactConnectionStringPasswords on all three paths that previously bypassed it: the plain-text Code fallback, the description text nodes, and Code = codeElement.Value. So passwords inside <example> descriptions/code are redacted too and won't re-trigger CredScan.
Contributor
Frontend HTML artifact readyThe latest frontend build uploaded the This comment updates automatically when a new frontend build artifact is uploaded. |
…llable API
- Use "{password}" instead of "<password>". Angle brackets are dropped as raw
HTML when doc nodes render to Markdown (csharp-api-markdown.ts concatenates
text without escaping), which would hide the value in connection-string
examples. "{password}" is also the existing placeholder convention already
used across the generated data (e.g. mysql://{user}:{password}@{host}).
- Sanitize the <example> extraction paths in ExtractDocExample (plain-text
code, description text nodes, and example code) so connection-string
passwords there cannot re-trigger CredScan in future data refreshes.
- Make DocumentationSanitizer.RedactConnectionStringPasswords nullable-aware
(string? in/out) to match its behavior and drop the null-forgiving operator
in tests. Exclude '{'/'}' from the value class to keep redaction idempotent.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4125af76-bbc2-4532-adee-98add8fdb6b7
radical
reviewed
Jul 28, 2026
radical
left a comment
radical
left a comment
Member
There was a problem hiding this comment.
[automated] Automated review notes on the connection-string password redaction change (3 inline comments below).
The connection-string password sanitizer redacted values to `{password}`.
Switch the token to a bare `Placeholder`, which is the value 1ES
recommends for scrubbed credential examples in generated content.
Update the sanitizer unit-test expectations to match, and update the four
affected package data files. The pre-existing `{password}` doc template
tokens in those files (author-written placeholders, not sanitizer output)
are intentionally left unchanged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c2fe46b-fcb4-48bf-8c62-5f9d3d7a470e
Two hardening fixes to the connection-string password redaction added in
this PR.
F1 - the redaction regex over-consumed trailing delimiters. The value
character class only excluded ';', quotes, comma, whitespace, backslash,
and brace/angle markers. A value immediately followed by a markdown or
URI delimiter ('`', ')', ']', '&', '|') swallowed that delimiter into the
match, so an inline-code fence lost its closing backtick and a link label
lost its closing paren, corrupting the rendered doc. The class now also
stops at those delimiters. A trailing sentence period is preserved by
trimming it off the captured value in the replacement callback rather
than excluding '.' from the class, which would truncate legitimate dotted
values.
F4 - two documentation paths reached the generated JSON unsanitized. Enum
member descriptions (Description = ExtractSummary(f)) and <see href="...">
link labels were emitted verbatim, so a connection string in an enum
member's <summary> or a link label bypassed redaction. Both now run
through the sanitizer. This changes no committed data (no such values
exist in the current package set); the fix is preventive.
Also refreshes the sanitizer comment and XML docs to describe the
Placeholder token and the widened exclusion set.
Tests: added markdown/URI delimiter and trailing-period cases to the
sanitizer unit tests, and an end-to-end test asserting an enum member
whose summary contains a connection string is redacted in the generated
JSON. All 31 tests pass.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8c2fe46b-fcb4-48bf-8c62-5f9d3d7a470e
radical
enabled auto-merge (squash)
July 28, 2026 22:53
radical
approved these changes
Jul 28, 2026
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 scheduled pipeline that mirrors public
mainto the internal AzDO remote (and merges it intodeploy/deploy-vnext-release) is blocked by 1ES / CredScan push protection:The flagged token is a documentation placeholder, not a real secret.
PackageJsonGeneratorcopies package XML doc comments verbatim into the frontend data JSON, and several packages document an example connection string that embeds a literalPassword=password:Aspire.Hosting.SqlServer-Server=host,port;User ID=sa;Password=password;TrustServerCertificate=trueAspire.Hosting.PostgreSQL,CommunityToolkit.Aspire.Hosting.Minio,CommunityToolkit.Aspire.Hosting.SurrealDb.config/CredScanSuppressions.jsonalready listspassword, but that file is only honored by the Guardian CI CredScan task - the server-side push protection ignores it and scans per-commit.Fix
Sanitize the placeholder at the source so it stops recurring:
DocumentationSanitizer.RedactConnectionStringPasswords, applied to text / inline-code / code-block doc nodes and the<example>extraction paths inCanonicalModelBuilder. It rewrites connection-stringPassword=/Pwd=literals to{password}. The match requires no whitespace around=, so C# default parameter values (string? password = null) are untouched.{password}is used (rather than<password>) because it is Markdown-safe - angle brackets are dropped as raw HTML when doc nodes render to Markdown - and it matches the placeholder convention already used elsewhere in the generated data (e.g.mysql://{user}:{password}@{host}).pkgs/*.jsonfiles (1 line each).Note on the one-time bypass
Push protection scans per-commit, so this forward fix cannot retroactively clean the value already committed in history. The currently-stuck sync still needs a one-time soft-block bypass in the AzDO Advanced Security portal (admin action). After that, this change keeps future data refreshes clean.