Add changelog bundle support for hiding private links#3002
Add changelog bundle support for hiding private links#3002
Conversation
📝 WalkthroughWalkthroughIntroduces optional private-link sanitization for changelog bundles. When enabled via Sequence DiagramsequenceDiagram
participant User as CLI/User
participant Bundler as ChangelogBundlingService
participant ConfigLoader as ChangelogConfigurationLoader
participant AssemblyProvider as ConfigurationContext (assembler.yml)
participant Sanitizer as PrivateChangelogLinkSanitizer
participant Renderer as Markdown/Asciidoc Renderer
User->>Bundler: run changelog bundle --sanitize-private-links (or config)
Bundler->>ConfigLoader: load changelog.yml
ConfigLoader-->>Bundler: SanitizePrivateLinks=true
Bundler->>AssemblyProvider: load assembler.yml references
AssemblyProvider-->>Bundler: ReferenceRepositories (with private flags)
Bundler->>Sanitizer: TrySanitizeBundle(bundle, assembly, defaults)
Sanitizer->>Sanitizer: for each entry.Prs/Issues
Sanitizer->>Sanitizer: parse reference -> owner/repo
Sanitizer->>AssemblyProvider: lookup repository by owner/repo
AssemblyProvider-->>Sanitizer: Repository (Private? true/false)
alt Repository.Private == true
Sanitizer->>Sanitizer: replace ref with "# PRIVATE: <ref>"
else
Sanitizer->>Sanitizer: keep original ref
end
Sanitizer-->>Bundler: sanitized bundle
Bundler->>Renderer: render bundle entries
Renderer->>Renderer: FormatPr/IssueLink detects "# PRIVATE:" -> returns empty
Renderer-->>User: output with private links hidden
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/cli/release/changelog-bundle-amend.md`:
- Line 71: The markdown link uses a nonexistent anchor
"#changelog-bundle-private-link-sanitization"; update the link target in the
sentence "Private link sanitization at bundle time" to point to the correct
anchor or page — either remove the fragment and link to
/contribute/changelog.md, or replace the fragment with the correct anchor name
that exists in contribute/changelog.md (search for the actual heading for
"bundle private link sanitization" and use that heading's slug).
In `@docs/cli/release/changelog-gh-release.md`:
- Line 54: Update the broken markdown link on the line that currently reads
"/contribute/changelog.md#changelog-bundle-private-link-sanitization": change
the target file to "/cli/release/changelog-bundle.md" and the anchor to
"#private-link-sanitization" so the final link is
"/cli/release/changelog-bundle.md#private-link-sanitization".
In `@src/Elastic.Documentation/ReleaseNotes/ChangelogTextUtilities.cs`:
- Around line 233-245: The parsing logic in ChangelogTextUtilities that extracts
owner/repo from GitHub URLs returns true too eagerly; update it to validate
additional path segments and fragments before accepting a match: ensure the
AbsolutePath has exactly two segments to accept plain owner/repo, and if there
are extra segments require the pattern to be /owner/repo/issues/<number> or
/owner/repo/pull/<number> (i.e., the third segment must be "issues" or "pull"
and the fourth segment must parse as an integer); likewise, if the input uses a
fragment (owner/repo#123) validate the fragment is a numeric issue/PR id before
returning true. Apply the same stricter validation to the second similar block
in this file (the block around the later range) so malformed refs like
/pull/not-a-number or `#abc` return false instead of being treated as valid
owner/repo.
- Around line 279-280: The sentinel handling currently returns an empty string
which still makes renderers treat a reference as present; change the checks that
do if (pr.StartsWith(PrivateReferenceSentinelPrefix,
StringComparison.OrdinalIgnoreCase)) return string.Empty; (and the similar
checks at the other occurrences) to return null instead, so callers can
distinguish "no reference" vs "empty text"; then update the Markdown rendering
call sites (where entry.Prs / entry.Issues are enumerated in
MarkdownRendererBase and IndexMarkdownRenderer) to filter out nulls before
deciding whether to emit the surrounding prose. Ensure every occurrence of
PrivateReferenceSentinelPrefix handling (lines shown: the three pairs) is
updated consistently.
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`:
- Around line 117-123: The current check uses sanitizePrivateLinks and
shouldResolve but only guards the new amend file, allowing --resolve to bypass
an unresolved parent bundle; update the logic in ChangelogBundleAmendService so
that when sanitizePrivateLinks is true you also verify the original/parent
bundle is resolved (e.g., check the existing/parent bundle resolved flag or
resolution state before proceeding) and emit the same collector.EmitError
message if the parent bundle is unresolved, preventing sanitize from running
when only the new amend is resolved; apply the same fix to the other occurrence
noted around the 151-170 block.
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs`:
- Around line 434-437: The ternary that computes sanitizePrivateLinks ignores
the CLI override when input.Profile is set; update the expression so that when
Profile is provided you still prefer a non-null input.SanitizePrivateLinksCli
before falling back to ProcessProfile value and config. Concretely, change the
branch that currently picks input.SanitizePrivateLinks when input.Profile is set
so it instead uses input.SanitizePrivateLinks ?? input.SanitizePrivateLinksCli
?? config.Bundle.SanitizePrivateLinks (and keep the other branch using
input.SanitizePrivateLinksCli ?? config.Bundle.SanitizePrivateLinks),
referencing the sanitizePrivateLinks local and the input.* properties
(NoSanitizePrivateLinks, Profile, SanitizePrivateLinks, SanitizePrivateLinksCli)
and config.Bundle.SanitizePrivateLinks.
In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs`:
- Around line 504-505: Add a new changelog YAML entry under docs/changelog
describing the new bundle flags referenced in ChangelogCommand.cs: the
sanitizePrivateLinks and noSanitizePrivateLinks options (CLI flags
--sanitize-private-links / --no-sanitize-private-links) and that
sanitize_private_links behavior defaults to bundle.sanitize_private_links when
omitted; ensure the YAML includes a short title, date, PR/author reference, and
a clear description of user-visible behavior and which command/class it affects
(reference ChangelogCommand and the parameter names sanitizePrivateLinks /
noSanitizePrivateLinks) so assembler-preview stops failing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06353714-34c7-48e9-bcf2-4e3e3e75228e
📒 Files selected for processing (20)
config/assembler.ymlconfig/changelog.example.ymldocs/cli/release/changelog-bundle-amend.mddocs/cli/release/changelog-bundle.mddocs/cli/release/changelog-gh-release.mddocs/contribute/changelog.mddocs/syntax/changelog.mdsrc/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cssrc/Elastic.Documentation/ReleaseNotes/ChangelogTextUtilities.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogInlineRenderer.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogLinkVisibility.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cssrc/services/Elastic.Changelog/Configuration/ChangelogConfigurationLoader.cssrc/services/Elastic.Changelog/Serialization/ChangelogConfigurationYaml.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cstests/Elastic.Changelog.Tests/Changelogs/AssemblerConfigurationYamlTests.cstests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs
src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`:
- Around line 132-138: The code currently only checks
parentBundleForSanitize.IsResolved; instead run the same private-link sanitizer
used for amendBundle against parentBundleForSanitize (using the sanitizer
method/class already used elsewhere in ChangelogBundleAmendService) and compare
the sanitized result to the original parent; if sanitization would change the
parent, call collector.EmitError (same message context) and return false,
instructing the user to rebuild the original bundle with resolve enabled or
disable bundle.sanitize_private_links; apply the same check to the second code
path that currently only inspects IsResolved (the block covering the later
amend/merge logic).
In `@src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs`:
- Around line 39-48: The current early-return that checks
assembly.ReferenceRepositories.Count and calls collector.EmitError causes
sanitize_private_links to fail before any entries are scanned; remove or bypass
this upfront check in PrivateChangelogLinkSanitizer and instead defer emitting
the "empty assembler.yml references" error until you actually encounter a
reference that requires privacy classification (i.e., when processing a specific
changelog entry or reference and you need to look up its repo but
assembly.ReferenceRepositories is empty). Update the logic in the method that
iterates references (the sanitizer's processing loop) to detect missing
reference repositories at the point of classification, and call
collector.EmitError with the same message only when a PR/issue link is being
evaluated and cannot be classified due to an empty ReferenceRepositories
collection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70b83c14-9303-4715-8b77-21eeb2e8eee5
📒 Files selected for processing (10)
docs/changelog/changelog-private-link-sanitization.yamlsrc/Elastic.Documentation/ReleaseNotes/ChangelogTextUtilities.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/AsciidocRendererBase.cssrc/services/Elastic.Changelog/Rendering/Markdown/IndexMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cstests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs
✅ Files skipped from review due to trivial changes (1)
- docs/changelog/changelog-private-link-sanitization.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs
- tests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs
- src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs
- src/Elastic.Documentation/ReleaseNotes/ChangelogTextUtilities.cs
src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
Outdated
Show resolved
Hide resolved
src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs`:
- Around line 133-155: The repository resolution duplicates the same repo-only
check in the fallback loop, causing it to miss keys like "elastic/<repo>"; in
the method PrivateChangelogLinkSanitizer.ResolveRepository (or the surrounding
resolution block) change the second-loop comparison from comparing kvp.Key to
repo to comparing kvp.Key to the owner-prefixed form (e.g. $"{owner}/{repo}" or
specifically "elastic/{repo}" when owner == "elastic") using the same
StringComparison.OrdinalIgnoreCase, so the fallback will find entries stored
with the "owner/repo" key and return the repository correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b9f2122-00e1-4940-91cf-54edb5722155
📒 Files selected for processing (3)
src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cstests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
- tests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs
| var key = string.Equals(owner, "elastic", StringComparison.OrdinalIgnoreCase) | ||
| ? repo | ||
| : $"{owner}/{repo}"; | ||
|
|
||
| foreach (var kvp in assembly.ReferenceRepositories) | ||
| { | ||
| if (string.Equals(kvp.Key, key, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| repository = kvp.Value; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| if (string.Equals(owner, "elastic", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| foreach (var kvp in assembly.ReferenceRepositories) | ||
| { | ||
| if (string.Equals(kvp.Key, repo, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| repository = kvp.Value; | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Repository resolution has a duplicated condition and can falsely fail known repos
At Line 146-Line 155, the fallback loop repeats the same kvp.Key == repo check already done for owner == "elastic", so it never checks elastic/<repo> and can emit a false “not listed” error.
Proposed fix
private static bool TryFindReferenceRepository(
string owner,
string repo,
AssemblyConfiguration assembly,
out Repository? repository)
{
- var key = string.Equals(owner, "elastic", StringComparison.OrdinalIgnoreCase)
- ? repo
- : $"{owner}/{repo}";
+ var fullName = $"{owner}/{repo}";
+ var isElasticOwner = string.Equals(owner, "elastic", StringComparison.OrdinalIgnoreCase);
foreach (var kvp in assembly.ReferenceRepositories)
{
- if (string.Equals(kvp.Key, key, StringComparison.OrdinalIgnoreCase))
+ if (string.Equals(kvp.Key, fullName, StringComparison.OrdinalIgnoreCase) ||
+ (isElasticOwner && string.Equals(kvp.Key, repo, StringComparison.OrdinalIgnoreCase)) ||
+ string.Equals(kvp.Value.Origin, fullName, StringComparison.OrdinalIgnoreCase))
{
repository = kvp.Value;
return true;
}
}
-
- if (string.Equals(owner, "elastic", StringComparison.OrdinalIgnoreCase))
- {
- foreach (var kvp in assembly.ReferenceRepositories)
- {
- if (string.Equals(kvp.Key, repo, StringComparison.OrdinalIgnoreCase))
- {
- repository = kvp.Value;
- return true;
- }
- }
- }
repository = null;
return false;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs`
around lines 133 - 155, The repository resolution duplicates the same repo-only
check in the fallback loop, causing it to miss keys like "elastic/<repo>"; in
the method PrivateChangelogLinkSanitizer.ResolveRepository (or the surrounding
resolution block) change the second-loop comparison from comparing kvp.Key to
repo to comparing kvp.Key to the owner-prefixed form (e.g. $"{owner}/{repo}" or
specifically "elastic/{repo}" when owner == "elastic") using the same
StringComparison.OrdinalIgnoreCase, so the fallback will find entries stored
with the "owner/repo" key and return the repository correctly.
| var parentText = await _fileSystem.File.ReadAllTextAsync(input.BundlePath, ctx); | ||
| parentBundleForSanitize = ReleaseNotesSerialization.DeserializeBundle(parentText); |
There was a problem hiding this comment.
We are already reading and deserializing input.BundlePath on InferResolve(). We should turn it into bool TryInferResolve(..., out var bundle) and use that data instead here.
Summary
Adds opt-in bundle-time private link sanitization for changelogs.
When enabled, PR and issue links that point at repositories marked
private: trueinassembler.ymlare rewritten to quoted# PRIVATE: …sentinels in bundle YAML. Renderers skip those sentinels in output while keeping them for audit purposes.Default behavior is unchanged (
sanitize_private_linksdefaults tofalse).A new
link-visibilityoption is also added to the changelog directive (equivalent to thechangelog rendercommand's--inputvisibility options) so that teams working from private repos have the option of showing public repo links (if any) after sanitizing the private links. Previously the directive hid all links from bundles derived from private repos.Configuration & CLI
changelog.yml:bundle.sanitize_private_links(defaultfalse); optional per-profilesanitize_private_linksoverride.changelog bundlecommand supports--sanitize-private-links/--no-sanitize-private-linksoptions (or the newsanitize_private_linksconfig settings for profile-based comandsbundle.resolvemust betrue( or--resolvecommand option specified) when sanitization is on; otherwise the command errors.assembler.ymlreferences(unknown repos fail the bundle).privatedefaults tofalseif omitted.Implementation highlights
PrivateChangelogLinkSanitizer: Parses refs viaChangelogTextUtilities.TryGetGitHubRepo, applies assembler registry + privacy checks, quotes sentinels in YAML.ChangelogBundlingService: Runs sanitization after bundle rules when enabled and resolved; loads assembler config for validation.changelog gh-release: No sanitize CLI flags; respectsbundle.sanitize_private_linksandbundle.resolvefrom config when creating the bundle (same bundling pipeline as option-basedbundle).changelog bundle-amend: Applies the same sanitization when config enables it and the amend is resolved; no sanitize CLI flags.ChangelogTextUtilities:FormatPrLink/FormatIssueLinkomit output for# PRIVATE:prefixes;TryGetGitHubRepofor URLs,owner/repo#n, and bare numbers.{changelog}directive:link-visibility:option:auto|keep-links|hide-links(aligns with render-time link hiding vs. bundle-time target sanitization).Config & docs
config/assembler.yml: e.g.kibana-teamentry for private registry / link filtering.config/changelog.example.yml: bundle + profile examples forsanitize_private_links.docs/contribute/changelog.md,docs/syntax/changelog.md,docs/cli/release/changelog-bundle.md,changelog-gh-release.md,changelog-bundle-amend.md(behavior, CLI matrix, limitations).Tests
PrivateChangelogLinkSanitizerTests: parsing, sentinel formatting, sanitize/unknown-repo behavior.AssemblerConfigurationYamlTests: repoconfig/assembler.ymldeserializes (keeps registry YAML valid in CI).Steps to test
Create changelog configuration file and a bunch of changelogs. For example, check out Kibana release notes grouped by team label kibana#258941
Update the changelog configuration file to include the new option (ensure that
resolve: trueis also set):Re-create the bundles, for example:
This command correctly fails because there's a repo mentioned in one of the changelogs that doesn't exist in
assembler.yml:Add the appropriate repos to the
assembler.ymlthen re-run the bundle command. NOTE: You must re-run thepublishbinariescommand to pick up these changes.Verify that the private links have been commented out. For example:
Generate more test bundles, for example:
Verify that the private links do not appear in the output via changelog directives. For example, the private link is omitted here:

Verify that the private links do not appear in the output via
changelog rendercommands. For example:/path/to/docs-builder/.artifacts/publish/docs-builder/release/docs-builder changelog render --input "./docs/releases/kibana/9.3.0.yaml" --output ./docs/release-notes/_snippets/When it generates markdown output (by default), the private link is successfully omitted:
When run with the
--file-type asciidocoption, the same is also true:* Allow users to edit schedule config. {elastic-pull}241928[#241928]Generative AI disclosure
Tool(s) and model(s) used: composer-2-fast