Replace changelog sanitize_private_links with link_allow_repos

[lcawl]

Summary

Replaces the assembler-based private-link sanitization (#3002) with an explicit allowlist model (no implicit allow).

Design principles

• Explicit allow only: repos in bundle.link_allow_repos (this avoids confusion of whether the current repo is included/excluded particularly when it's a private repo )
• Single-repo enforcement: merged repos are a changelog directive rendering thing, not a bundling thing
• When a link is disallowed: strip to # PRIVATE: sentinel + warn in diagnostics
• Private bundles safe-by-default: empty allowlist → all links stripped (no assembler check needed)
• Amend reads fresh allowlist from current config (consistency)
• Render reuses the existing # PRIVATE: skip logic from PR Add changelog bundle support for hiding private links #3002
• Assembler: optional validation only (warn if allowlisted repo is marked private)

Implementation details

1. New Sanitizer: Created LinkAllowlistSanitizer.cs with TryApplyBundle() and optional EmitAssemblerDiagnostics().
2. Config Model: Updated BundleConfiguration — replaced SanitizePrivateLinks (bool) with LinkAllowRepos (list), removed profile SanitizePrivateLinks.
3. YAML/Loader: Updated YAML DTOs and ParseBundleConfiguration() to parse link_allow_repos via YamlLenientList, validate bundle.repo has no + syntax.
4. Bundling Service: Wired LinkAllowRepos into BundleChangelogsArguments, ApplyConfigDefaults(), and allowlist application with optional assembler warnings.
5. Amend Service: Mirrored allowlist logic with "parent already conforms" checks.
6. CLI Command: Removed --sanitize-private-links / --no-sanitize-private-links flags and profile field from ChangelogCommand.cs.
7. Tests: Replaced PrivateChangelogLinkSanitizerTests.cs with LinkAllowlistSanitizerTests.cs (allowlist-focused; kept TryGetGitHubRepo / GetFirstRepoSegmentFromBundleRepo tests).
8. Docs: Updated bundle.md (new "PR and issue link allowlist" section), changelog.md (replaced sanitize_private_links with link_allow_repos), config/changelog.example.yml (updated examples), and changelog-private-link-sanitization.yaml (update notes).
9. Removed: Deleted old PrivateChangelogLinkSanitizer.cs.

Steps to test

1. Create a bunch of changelogs, or check out a PR like Kibana release notes grouped by team label kibana#258941
2. Update the changelog configuration file to use the new option:

     link_allow_repos:
   - elastic/elasticsearch
   - elastic/kibana
   - elastic/roadmap

3. Create bundles and confirm that only links to these repos are uncommented. For example:

    /path/to/.artifacts/publish/docs-builder/release/docs-builder changelog bundle kibana-release 9.3.0 ./docs/9.3.0-kibana.txt 

Generative AI disclosure

1. Did you use a generative AI (GenAI) tool to assist in creating this contribution?

• Yes
• No

3. If you answered "Yes" to the previous question, please specify the tool(s) and model(s) used (e.g., Google Gemini, OpenAI ChatGPT-4, etc.).

Tool(s) and model(s) used: composer-2, claude-4.5-haiku

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces the boolean sanitize_private_links bundle option with an allowlist link_allow_repos (list of owner/repo) used at bundle time to filter PR/issue links. When link_allow_repos is present (including empty), references whose resolved owner/repo is not in the allowlist are rewritten to # PRIVATE: sentinels; when omitted, no filtering is applied. The old PrivateChangelogLinkSanitizer is removed and replaced by LinkAllowlistSanitizer. Validation now forbids merged-repo (+) syntax in bundle.repo and enforces owner/repo format for allowlist entries. CLI flags for sanitization are removed and docs updated.

Sequence Diagram

sequenceDiagram
    actor User as User/CLI
    participant Config as Configuration Loader
    participant Bundler as Bundling Service
    participant Sanitizer as LinkAllowlistSanitizer
    participant Output as Bundle Output

    User->>Config: Load changelog.yml with link_allow_repos
    Config->>Config: Validate bundle.repo (no + syntax) and allowlist entries (owner/repo)
    Config-->>Bundler: Return validated BundleConfiguration

    Bundler->>Bundler: Ensure bundle is resolved when applying allowlist
    alt link_allow_repos present
        Bundler->>Sanitizer: TryApplyBundle(bundle, allowlist, defaults)
        Sanitizer->>Sanitizer: For each PR/issue, parse resolved owner/repo
        Sanitizer->>Sanitizer: Compare against allowlist
        alt In allowlist
            Sanitizer->>Sanitizer: Keep reference unchanged
        else Not in allowlist
            Sanitizer->>Sanitizer: Rewrite to "# PRIVATE:" sentinel and emit warning
        end
        Sanitizer-->>Bundler: Return sanitized bundle and change flag
    else link_allow_repos absent
        Bundler-->>Bundler: Skip link filtering
    end

    Bundler-->>Output: Finalized bundle

Loading

Possibly related PRs

• Add changelog bundle support for hiding private links #3002: Introduced the original boolean sanitize_private_links sanitization and related utilities that this PR replaces with an allowlist-based implementation.

Suggested labels

breaking, documentation

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely and clearly summarizes the main change: replacing an old feature (sanitize_private_links) with a new one (link_allow_repos).
Description check	✅ Passed	The PR description clearly explains the change from assembler-based private-link sanitization to an explicit allowlist model, detailing design principles, implementation changes, testing steps, and AI disclosure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch changelog-allowlist

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/changelog.example.yml`:
- Around line 215-221: Remove the live entries from the example config so new
projects don't enable filtering by default: in the changelog example YAML change
the link_allow_repos block to be empty (e.g. link_allow_repos: [] ) or replace
the populated list with commented example values, and ensure the template
emitted by the docs-builder "changelog init" command uses that empty/commented
allowlist (refer to the link_allow_repos key and the changelog init generator in
ChangelogCommand.cs).

In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`:
- Around line 234-248: The code unconditionally reads assembler.yml and calls
LinkAllowlistSanitizer.EmitAssemblerDiagnostics even when
bundle.link_allow_repos is an empty list, which causes unnecessary file I/O and
misleading warnings; update the block that uses configurationContext and
linkAllowRepos (the variable passed as linkAllowRepos! to
EmitAssemblerDiagnostics) to first check that linkAllowRepos is not null and not
empty (e.g., Count > 0) and only then perform ReadToEnd, Deserialize, and call
LinkAllowlistSanitizer.EmitAssemblerDiagnostics; keep the existing exception
filter and collector.EmitWarning behavior for real I/O/deserialize failures.

In `@src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs`:
- Around line 296-310: Skip loading assembler.yml and running
LinkAllowlistSanitizer.EmitAssemblerDiagnostics when there are no allowlist
entries: guard the whole configurationContext block with a check that
input.LinkAllowRepos.Count > 0 (or Any()) before attempting to read
configurationContext.ConfigurationFileProvider.AssemblerFile or calling
LinkAllowlistSanitizer.EmitAssemblerDiagnostics; this prevents unnecessary reads
and warnings when the allowlist is empty while keeping existing exception
handling around AssemblyConfiguration.Deserialize and the collector.EmitWarning
behavior.

In `@src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs`:
- Around line 133-135: The current check in LinkAllowlistSanitizer returns an
empty list for null/empty refs ("return refs ?? []"), which converts originally
null Prs/Issues to empty arrays while changesApplied remains false; change the
early return to preserve null by returning refs directly (e.g., replace "return
refs ?? []" with "return refs") so that null lists remain null and untouched
bundles are not rewritten; adjust in the same method where the local variable
refs and the changesApplied flag are used.

🪄 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: c3f6fb9c-660f-49b7-8319-677783ca2724

📥 Commits

Reviewing files that changed from the base of the PR and between c4473c9 and 2956b4f.

📒 Files selected for processing (14)

• config/changelog.example.yml
• docs/changelog/changelog-private-link-sanitization.yaml
• docs/cli/changelog/bundle.md
• docs/contribute/changelog.md
• src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs
• src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
• src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs
• src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs
• src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs
• src/services/Elastic.Changelog/Configuration/ChangelogConfigurationLoader.cs
• src/services/Elastic.Changelog/Serialization/ChangelogConfigurationYaml.cs
• src/tooling/docs-builder/Commands/ChangelogCommand.cs
• tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs
• tests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs

💤 Files with no reviewable changes (2)

• src/services/Elastic.Changelog/Bundling/PrivateChangelogLinkSanitizer.cs
• tests/Elastic.Changelog.Tests/Changelogs/PrivateChangelogLinkSanitizerTests.cs

[lcawl]

Actionable comments posted: 4

Here’s what was implemented:

1. Assembler diagnostics guards

• src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs — Assembler read / EmitAssemblerDiagnostics only runs when configurationContext != null and input.LinkAllowRepos.Count > 0.
• src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs — Same pattern with linkAllowRepos!.Count > 0.

2. Preserve null PR/issue lists

• src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs — ApplyToReferenceList now returns null when refs is null, and returns the empty list unchanged when Count == 0 (no refs ?? []).
• TryApplyBundle — Treats failure only when prs == null && entry.Prs is not null (and the same for issues), so a preserved null list is not treated as an error.

3. Test

• TryApplyBundle_NullPrsAndIssues_PreservesNull_WhenUnchanged in tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs.

dotnet test on LinkAllowlistSanitizerTests reported 25 passed (including the new test).

[coderabbitai]

🧹 Nitpick comments (1)

src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs (1)

220-221: Consider extracting duplicated owner/repo logic.

Lines 220-221 duplicate the owner/repo extraction from lines 168-169. Minor DRY improvement opportunity—could extract a local helper, but not blocking.

♻️ Optional: Extract helper

+		static (string owner, string? repo) GetOwnerRepo(Bundle bundle) =>
+			bundle.Products.Count > 0
+				? (bundle.Products[0].Owner ?? "elastic", bundle.Products[0].Repo)
+				: ("elastic", null);

Then use var (owner, repo) = GetOwnerRepo(parentBundleForAllowlist); in both locations.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`
around lines 220 - 221, Duplicate logic extracts owner/repo from
parentBundleForAllowlist in two places; extract a small helper to avoid
repetition. Add a private method like GetOwnerRepo(ParentBundleType
parentBundleForAllowlist) that returns (string owner, string repo) or a
ValueTuple, implement the same logic currently used (default owner "elastic",
repo null if no products), and replace both inline expressions (the occurrences
around parentBundleForAllowlist.Products access) with var (owner, repo) =
GetOwnerRepo(parentBundleForAllowlist); to centralize behavior and keep
semantics identical.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`:
- Around line 220-221: Duplicate logic extracts owner/repo from
parentBundleForAllowlist in two places; extract a small helper to avoid
repetition. Add a private method like GetOwnerRepo(ParentBundleType
parentBundleForAllowlist) that returns (string owner, string repo) or a
ValueTuple, implement the same logic currently used (default owner "elastic",
repo null if no products), and replace both inline expressions (the occurrences
around parentBundleForAllowlist.Products access) with var (owner, repo) =
GetOwnerRepo(parentBundleForAllowlist); to centralize behavior and keep
semantics identical.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0ee91852-c2dd-4157-a4ce-0698287ba521

📥 Commits

Reviewing files that changed from the base of the PR and between 2956b4f and e590a0c.

📒 Files selected for processing (5)

• config/changelog.example.yml
• src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
• src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs
• src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs
• tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs

✅ Files skipped from review due to trivial changes (1)

• src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs

🚧 Files skipped from review as they are similar to previous changes (3)

• config/changelog.example.yml
• src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs
• tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs