Skip to content

Commit efd4941

Browse files
cotticotti
committed
Enforce stripping strategy and add tests
1 parent 70af618 commit efd4941

3 files changed

Lines changed: 277 additions & 2 deletions

File tree

‎src/infra/docs-lambda-changelog-scrubber/Program.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,14 @@ async Task<string> ScrubBundle(string content, ILambdaContext context)
186186
if (!changed)
187187
{
188188
context.Logger.LogInformation("Bundle had no private references, writing unchanged");
189+
LinkAllowlistSanitizer.ValidateNoPrivateReferences(content, allowRepos);
189190
return content;
190191
}
191192

192-
return ReleaseNotesSerialization.SerializeBundle(sanitized);
193+
sanitized = LinkAllowlistSanitizer.StripBundleSentinels(sanitized);
194+
var result = ReleaseNotesSerialization.SerializeBundle(sanitized);
195+
LinkAllowlistSanitizer.ValidateNoPrivateReferences(result, allowRepos);
196+
return result;
193197
}
194198

195199
async Task<string> ScrubChangelog(string content, ILambdaContext context)
@@ -220,6 +224,7 @@ async Task<string> ScrubChangelog(string content, ILambdaContext context)
220224
if (!changed)
221225
{
222226
context.Logger.LogInformation("Changelog entry had no private references, writing unchanged");
227+
LinkAllowlistSanitizer.ValidateNoPrivateReferences(content, allowRepos);
223228
return content;
224229
}
225230

@@ -232,5 +237,7 @@ async Task<string> ScrubChangelog(string content, ILambdaContext context)
232237
Issues = sanitized.Issues
233238
};
234239

235-
return ReleaseNotesSerialization.SerializeEntry(scrubEntry);
240+
var result = ReleaseNotesSerialization.SerializeEntry(scrubEntry);
241+
LinkAllowlistSanitizer.ValidateNoPrivateReferences(result, allowRepos);
242+
return result;
236243
}

‎src/services/Elastic.Changelog/Bundling/LinkAllowlistSanitizer.cs‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,60 @@ public static bool TryApplyChangelogEntry(
237237
return result;
238238
}
239239

240+
/// <summary>
241+
/// Strips <c># PRIVATE:</c> sentinel entries from all entries in a bundle.
242+
/// Use after <see cref="TryApplyBundle"/> when the output is destined for a public bucket.
243+
/// </summary>
244+
public static Bundle StripBundleSentinels(Bundle bundle)
245+
{
246+
var newEntries = new List<BundledEntry>(bundle.Entries.Count);
247+
foreach (var entry in bundle.Entries)
248+
{
249+
var dummy = false;
250+
var prs = DropSentinels(entry.Prs, ref dummy);
251+
var issues = DropSentinels(entry.Issues, ref dummy);
252+
newEntries.Add(entry with { Prs = prs, Issues = issues });
253+
}
254+
255+
return bundle with { Entries = newEntries };
256+
}
257+
258+
/// <summary>
259+
/// Scans serialized YAML for GitHub references not in <paramref name="allowRepos"/>.
260+
/// Throws <see cref="InvalidOperationException"/> if any are found, providing defense-in-depth
261+
/// against new or renamed fields leaking private references.
262+
/// </summary>
263+
public static void ValidateNoPrivateReferences(string serializedYaml, IReadOnlyList<string> allowRepos)
264+
{
265+
var allow = BuildAllowSet(allowRepos);
266+
var violations = new List<string>();
267+
268+
foreach (var match in GitHubUrlRegex().EnumerateMatches(serializedYaml))
269+
{
270+
var text = serializedYaml.Substring(match.Index, match.Length);
271+
var m = GitHubUrlRegex().Match(text);
272+
var fullName = $"{m.Groups["owner"].Value}/{m.Groups["repo"].Value}";
273+
if (!allow.Contains(fullName))
274+
violations.Add(text);
275+
}
276+
277+
foreach (var match in ShortFormRefRegex().EnumerateMatches(serializedYaml))
278+
{
279+
var text = serializedYaml.Substring(match.Index, match.Length);
280+
var m = ShortFormRefRegex().Match(text);
281+
var fullName = $"{m.Groups["owner"].Value}/{m.Groups["repo"].Value}";
282+
if (!allow.Contains(fullName))
283+
violations.Add(text);
284+
}
285+
286+
if (serializedYaml.Contains(SentinelPrefix, StringComparison.OrdinalIgnoreCase))
287+
violations.Add("Residual # PRIVATE: sentinel found");
288+
289+
if (violations.Count > 0)
290+
throw new InvalidOperationException(
291+
$"Post-serialize validation failed: {violations.Count} private reference(s) found in public output: {string.Join(", ", violations)}");
292+
}
293+
240294
/// <summary>
241295
/// Overload that accepts a list of allowed repos (converts to the internal <see cref="HashSet{T}"/>).
242296
/// </summary>

‎tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,4 +685,218 @@ public void ScrubText_IssueUrl_ReplacesPrivate()
685685
changed.Should().BeTrue();
686686
result.Should().NotContain("secret-repo");
687687
}
688+
689+
// --- StripBundleSentinels ---
690+
691+
[Fact]
692+
public void StripBundleSentinels_RemovesSentinelsFromPrsAndIssues()
693+
{
694+
var bundle = new Bundle
695+
{
696+
Entries =
697+
[
698+
new()
699+
{
700+
Title = "Entry with sentinels",
701+
Prs = ["https://github.com/elastic/elasticsearch/pull/1", "# PRIVATE: https://github.com/elastic/secret/pull/2"],
702+
Issues = ["# PRIVATE: elastic/secret#3", "https://github.com/elastic/elasticsearch/issues/4"]
703+
}
704+
]
705+
};
706+
707+
var result = LinkAllowlistSanitizer.StripBundleSentinels(bundle);
708+
709+
result.Entries[0].Prs.Should().HaveCount(1);
710+
result.Entries[0].Prs![0].Should().Be("https://github.com/elastic/elasticsearch/pull/1");
711+
result.Entries[0].Issues.Should().HaveCount(1);
712+
result.Entries[0].Issues![0].Should().Be("https://github.com/elastic/elasticsearch/issues/4");
713+
}
714+
715+
[Fact]
716+
public void StripBundleSentinels_AllSentinels_ReturnsEmptyLists()
717+
{
718+
var bundle = new Bundle
719+
{
720+
Entries =
721+
[
722+
new()
723+
{
724+
Title = "All private",
725+
Prs = ["# PRIVATE: https://github.com/elastic/secret/pull/1"],
726+
Issues = ["# PRIVATE: elastic/secret#2"]
727+
}
728+
]
729+
};
730+
731+
var result = LinkAllowlistSanitizer.StripBundleSentinels(bundle);
732+
733+
result.Entries[0].Prs.Should().BeEmpty();
734+
result.Entries[0].Issues.Should().BeEmpty();
735+
}
736+
737+
[Fact]
738+
public void StripBundleSentinels_NullLists_PreservesNull()
739+
{
740+
var bundle = new Bundle
741+
{
742+
Entries = [new() { Title = "No refs", Prs = null, Issues = null }]
743+
};
744+
745+
var result = LinkAllowlistSanitizer.StripBundleSentinels(bundle);
746+
747+
result.Entries[0].Prs.Should().BeNull();
748+
result.Entries[0].Issues.Should().BeNull();
749+
}
750+
751+
[Fact]
752+
public void StripBundleSentinels_MultipleEntries_StripsAll()
753+
{
754+
var bundle = new Bundle
755+
{
756+
Entries =
757+
[
758+
new() { Title = "A", Prs = ["# PRIVATE: x"] },
759+
new() { Title = "B", Issues = ["# PRIVATE: y", "elastic/elasticsearch#1"] }
760+
]
761+
};
762+
763+
var result = LinkAllowlistSanitizer.StripBundleSentinels(bundle);
764+
765+
result.Entries[0].Prs.Should().BeEmpty();
766+
result.Entries[1].Issues.Should().HaveCount(1);
767+
result.Entries[1].Issues![0].Should().Be("elastic/elasticsearch#1");
768+
}
769+
770+
// --- ValidateNoPrivateReferences ---
771+
772+
[Fact]
773+
public void ValidateNoPrivateReferences_CleanYaml_DoesNotThrow()
774+
{
775+
var yaml = """
776+
title: Feature
777+
prs:
778+
- https://github.com/elastic/elasticsearch/pull/1
779+
description: Some clean text
780+
""";
781+
782+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearch);
783+
act.Should().NotThrow();
784+
}
785+
786+
[Fact]
787+
public void ValidateNoPrivateReferences_PrivateUrl_Throws()
788+
{
789+
var yaml = """
790+
title: Feature
791+
prs:
792+
- https://github.com/elastic/secret-repo/pull/42
793+
""";
794+
795+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearch);
796+
act.Should().Throw<InvalidOperationException>()
797+
.WithMessage("*secret-repo*");
798+
}
799+
800+
[Fact]
801+
public void ValidateNoPrivateReferences_PrivateShortForm_Throws()
802+
{
803+
var yaml = """
804+
description: See elastic/secret-team#99
805+
""";
806+
807+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearch);
808+
act.Should().Throw<InvalidOperationException>()
809+
.WithMessage("*secret-team*");
810+
}
811+
812+
[Fact]
813+
public void ValidateNoPrivateReferences_ResidualSentinel_Throws()
814+
{
815+
var yaml = """
816+
prs:
817+
- "# PRIVATE: https://github.com/elastic/secret/pull/1"
818+
""";
819+
820+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearch);
821+
act.Should().Throw<InvalidOperationException>()
822+
.WithMessage("*PRIVATE*");
823+
}
824+
825+
[Fact]
826+
public void ValidateNoPrivateReferences_AllowedUrl_DoesNotThrow()
827+
{
828+
var yaml = "See https://github.com/elastic/elasticsearch/pull/100 and elastic/kibana#50";
829+
830+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearchAndKibana);
831+
act.Should().NotThrow();
832+
}
833+
834+
[Fact]
835+
public void ValidateNoPrivateReferences_EmptyYaml_DoesNotThrow()
836+
{
837+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences("", AllowElasticsearch);
838+
act.Should().NotThrow();
839+
}
840+
841+
[Fact]
842+
public void ValidateNoPrivateReferences_MixedAllowedAndPrivate_Throws()
843+
{
844+
var yaml = "Public https://github.com/elastic/elasticsearch/pull/1 and private https://github.com/elastic/secret/issues/2";
845+
846+
var act = () => LinkAllowlistSanitizer.ValidateNoPrivateReferences(yaml, AllowElasticsearch);
847+
act.Should().Throw<InvalidOperationException>()
848+
.WithMessage("*secret*");
849+
}
850+
851+
// --- TryApplyChangelogEntry mixed scenarios ---
852+
853+
[Fact]
854+
public void TryApplyChangelogEntry_MixedPrs_KeepsAllowedDropsPrivate()
855+
{
856+
var entry = new BundledEntry
857+
{
858+
Title = "Mixed refs",
859+
Prs = [
860+
"https://github.com/elastic/elasticsearch/pull/1",
861+
"https://github.com/elastic/secret-repo/pull/2",
862+
"https://github.com/elastic/kibana/pull/3"
863+
]
864+
};
865+
866+
var ok = LinkAllowlistSanitizer.TryApplyChangelogEntry(
867+
Collector, entry, AllowElasticsearchAndKibana, "elastic", "elasticsearch",
868+
out var sanitized, out var changed);
869+
870+
ok.Should().BeTrue();
871+
changed.Should().BeTrue();
872+
sanitized.Prs.Should().HaveCount(2);
873+
sanitized.Prs.Should().Contain("https://github.com/elastic/elasticsearch/pull/1");
874+
sanitized.Prs.Should().Contain("https://github.com/elastic/kibana/pull/3");
875+
sanitized.Prs.Should().NotContain(r => r.Contains("secret-repo"));
876+
}
877+
878+
[Fact]
879+
public void TryApplyChangelogEntry_PrivateRefsInAllFields_ScrubsEverything()
880+
{
881+
var entry = new BundledEntry
882+
{
883+
Title = "Full scrub test",
884+
Prs = ["https://github.com/elastic/secret/pull/1"],
885+
Issues = ["elastic/secret#2"],
886+
Description = "Caused by https://github.com/elastic/secret/issues/3",
887+
Impact = "See elastic/secret#4",
888+
Action = "Use https://github.com/elastic/secret/pull/5 to migrate"
889+
};
890+
891+
var ok = LinkAllowlistSanitizer.TryApplyChangelogEntry(
892+
Collector, entry, AllowElasticsearch, "elastic", "elasticsearch",
893+
out var sanitized, out _);
894+
895+
ok.Should().BeTrue();
896+
sanitized.Prs.Should().BeEmpty();
897+
sanitized.Issues.Should().BeEmpty();
898+
sanitized.Description.Should().NotContain("secret");
899+
sanitized.Impact.Should().NotContain("secret");
900+
sanitized.Action.Should().NotContain("secret");
901+
}
688902
}

0 commit comments

Comments
 (0)