From 1f5a18c27016bec061b4276f0a7f6ba92443d732 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:23:43 +0900 Subject: [PATCH 1/4] Bound TestTelemetry TRX processing (#3442) --- changelog.d/unreleased/3442.security.md | 16 ++ tests/CodeIndex.Tests/TestTelemetryTests.cs | 103 +++++++++++++ tools/CodeIndex.TestTelemetry/Program.cs | 163 +++++++++++++++----- 3 files changed, 247 insertions(+), 35 deletions(-) create mode 100644 changelog.d/unreleased/3442.security.md diff --git a/changelog.d/unreleased/3442.security.md b/changelog.d/unreleased/3442.security.md new file mode 100644 index 0000000000..aee6786701 --- /dev/null +++ b/changelog.d/unreleased/3442.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3442 +affected: + - tools/CodeIndex.TestTelemetry/Program.cs + - tests/CodeIndex.Tests/TestTelemetryTests.cs +--- + +## English + +- **Test telemetry TRX processing is now bounded (#3442)** — the telemetry helper now caps `--top`, TRX file discovery, TRX file size, XML parser work, and retained result collections so unusually large test result trees cannot force unbounded processing. + +## 日本語 + +- **Test telemetry の TRX 処理に上限を設けました (#3442)** — telemetry helper は `--top`、TRX ファイル探索、TRX ファイルサイズ、XML parser の処理量、保持する結果コレクションを制限し、過大なテスト結果ツリーで無制限の処理が発生しないようにしました。 diff --git a/tests/CodeIndex.Tests/TestTelemetryTests.cs b/tests/CodeIndex.Tests/TestTelemetryTests.cs index 8afe6d6853..5dc1f72c6a 100644 --- a/tests/CodeIndex.Tests/TestTelemetryTests.cs +++ b/tests/CodeIndex.Tests/TestTelemetryTests.cs @@ -75,4 +75,107 @@ public void Load_MissingDirectoryReturnsWarningInsteadOfFailingCiSummary() Assert.Single(summary.Warnings); Assert.Contains("Results directory not found", summary.Warnings[0], StringComparison.Ordinal); } + + [Fact] + public void Load_RejectsTopValuesAboveTelemetryCap() + { + var exception = Assert.Throws(() => + TrxTelemetry.Load(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")), TrxTelemetry.MaxTop + 1)); + + Assert.Contains($"between 1 and {TrxTelemetry.MaxTop}", exception.Message, StringComparison.Ordinal); + } + + [Fact] + public void Load_CapsTrxDiscovery() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_cap"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + + for (var i = 0; i < TrxTelemetry.MaxTrxFiles + 1; i++) + { + File.WriteAllText(Path.Combine(resultsDirectory, $"results-{i:D4}.trx"), MinimalTrx($"Test{i:D4}")); + } + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(TrxTelemetry.MaxTrxFiles, summary.TrxFileCount); + Assert.Equal(TrxTelemetry.MaxTrxFiles, summary.Total); + Assert.Contains(summary.Warnings, warning => + warning.Contains("TRX file cap reached", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public void Load_SkipsTrxFilesAboveSizeCap() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_size"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + var largeTrx = Path.Combine(resultsDirectory, "too-large.trx"); + using (var stream = File.Create(largeTrx)) + { + stream.SetLength(TrxTelemetry.MaxTrxFileBytes + 1); + } + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(1, summary.TrxFileCount); + Assert.Equal(0, summary.Total); + Assert.Contains(summary.Warnings, warning => + warning.Contains("byte cap", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public void Load_RejectsTrxDtds() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_dtd"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + File.WriteAllText(Path.Combine(resultsDirectory, "with-dtd.trx"), """ + + ]> + + + + + + """); + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(0, summary.Total); + Assert.Contains(summary.Warnings, warning => + warning.Contains("Could not parse", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + private static string MinimalTrx(string testName) => $$""" + + + + + + + """; } diff --git a/tools/CodeIndex.TestTelemetry/Program.cs b/tools/CodeIndex.TestTelemetry/Program.cs index 360c12a185..f4fc927436 100644 --- a/tools/CodeIndex.TestTelemetry/Program.cs +++ b/tools/CodeIndex.TestTelemetry/Program.cs @@ -1,5 +1,5 @@ using System.Globalization; -using System.Xml.Linq; +using System.Xml; namespace CodeIndex.TestTelemetry; @@ -60,8 +60,12 @@ private static SummarizeOptions ParseSummarizeOptions(string[] args) if (i + 1 >= args.Length) throw new TelemetryException("Missing value for --top."); - if (!int.TryParse(args[++i], NumberStyles.None, CultureInfo.InvariantCulture, out top) || top <= 0) - throw new TelemetryException("--top must be a positive integer."); + if (!int.TryParse(args[++i], NumberStyles.None, CultureInfo.InvariantCulture, out top) || + top <= 0 || + top > TrxTelemetry.MaxTop) + { + throw new TelemetryException($"--top must be between 1 and {TrxTelemetry.MaxTop}."); + } continue; } @@ -77,10 +81,14 @@ private sealed record SummarizeOptions(string ResultsDirectory, int Top); public static class TrxTelemetry { + public const int MaxTop = 100; + public const int MaxTrxFiles = 256; + public const long MaxTrxFileBytes = 16 * 1024 * 1024; + public static TrxTelemetrySummary Load(string resultsDirectory, int top) { - if (top <= 0) - throw new TelemetryException("Top count must be positive."); + if (top <= 0 || top > MaxTop) + throw new TelemetryException($"Top count must be between 1 and {MaxTop}."); if (!Directory.Exists(resultsDirectory)) { @@ -97,19 +105,42 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) Warnings: [$"Results directory not found: {resultsDirectory}"]); } - var trxFiles = Directory - .EnumerateFiles(resultsDirectory, "*.trx", SearchOption.AllDirectories) - .OrderBy(path => path, StringComparer.Ordinal) - .ToList(); - - var tests = new List(); var warnings = new List(); + var trxFiles = EnumerateTrxFiles(resultsDirectory, warnings); + var slowest = new List(Math.Min(top, 16)); + var failures = new List(Math.Min(top, 16)); + var total = 0; + var passed = 0; + var failed = 0; + var skipped = 0; foreach (var path in trxFiles) { + if (!CanReadTrxFile(path, warnings)) + continue; + try { - tests.AddRange(ReadResults(path)); + foreach (var result in ReadResults(path)) + { + total++; + + if (IsOutcome(result, "Passed")) + { + passed++; + } + else if (IsFailureOutcome(result)) + { + failed++; + AddTopResult(failures, result, top); + } + else if (IsOutcome(result, "NotExecuted") || IsOutcome(result, "Skipped")) + { + skipped++; + } + + AddTopResult(slowest, result, top); + } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or System.Xml.XmlException) { @@ -117,28 +148,12 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) } } - var passed = tests.Count(result => IsOutcome(result, "Passed")); - var failed = tests.Count(IsFailureOutcome); - var skipped = tests.Count(result => IsOutcome(result, "NotExecuted") || IsOutcome(result, "Skipped")); - var other = tests.Count - passed - failed - skipped; - - var slowest = tests - .OrderByDescending(result => result.Duration) - .ThenBy(result => result.TestName, StringComparer.Ordinal) - .Take(top) - .ToList(); - - var failures = tests - .Where(IsFailureOutcome) - .OrderByDescending(result => result.Duration) - .ThenBy(result => result.TestName, StringComparer.Ordinal) - .Take(top) - .ToList(); + var other = total - passed - failed - skipped; return new TrxTelemetrySummary( ResultsDirectory: resultsDirectory, TrxFileCount: trxFiles.Count, - Total: tests.Count, + Total: total, Passed: passed, Failed: failed, Skipped: skipped, @@ -148,13 +163,64 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) Warnings: warnings); } + private static List EnumerateTrxFiles(string resultsDirectory, List warnings) + { + var trxFiles = new List(MaxTrxFiles); + + try + { + foreach (var path in Directory.EnumerateFiles(resultsDirectory, "*.trx", SearchOption.AllDirectories)) + { + if (trxFiles.Count >= MaxTrxFiles) + { + warnings.Add($"TRX file cap reached: using first {MaxTrxFiles} files."); + break; + } + + trxFiles.Add(path); + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + warnings.Add($"Could not enumerate TRX files: {ex.Message}"); + } + + trxFiles.Sort(StringComparer.Ordinal); + return trxFiles; + } + + private static bool CanReadTrxFile(string path, List warnings) + { + try + { + var file = new FileInfo(path); + if (file.Length > MaxTrxFileBytes) + { + warnings.Add($"TRX file exceeds {MaxTrxFileBytes} byte cap: {path}"); + return false; + } + + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + warnings.Add($"Could not inspect {path}: {ex.Message}"); + return false; + } + } + private static IEnumerable ReadResults(string path) { - var document = XDocument.Load(path, LoadOptions.None); - foreach (var element in document.Descendants().Where(element => element.Name.LocalName == "UnitTestResult")) + using var stream = File.OpenRead(path); + using var reader = XmlReader.Create(stream, CreateXmlReaderSettings()); + + while (reader.Read()) { - var testName = (string?)element.Attribute("testName"); - var outcome = (string?)element.Attribute("outcome"); + if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "UnitTestResult") + continue; + + var testName = reader.GetAttribute("testName"); + var outcome = reader.GetAttribute("outcome"); if (string.IsNullOrWhiteSpace(testName) || string.IsNullOrWhiteSpace(outcome)) continue; @@ -162,10 +228,37 @@ private static IEnumerable ReadResults(string path) yield return new TrxTestResult( TestName: testName.Trim(), Outcome: outcome.Trim(), - Duration: ParseDuration((string?)element.Attribute("duration"))); + Duration: ParseDuration(reader.GetAttribute("duration"))); } } + private static XmlReaderSettings CreateXmlReaderSettings() => new() + { + DtdProcessing = DtdProcessing.Prohibit, + IgnoreComments = true, + IgnoreProcessingInstructions = true, + MaxCharactersFromEntities = 0, + MaxCharactersInDocument = MaxTrxFileBytes, + XmlResolver = null + }; + + private static void AddTopResult(List results, TrxTestResult result, int limit) + { + results.Add(result); + results.Sort(CompareByDurationDescendingThenName); + + if (results.Count > limit) + results.RemoveAt(limit); + } + + private static int CompareByDurationDescendingThenName(TrxTestResult left, TrxTestResult right) + { + var duration = right.Duration.CompareTo(left.Duration); + return duration != 0 + ? duration + : string.Compare(left.TestName, right.TestName, StringComparison.Ordinal); + } + private static TimeSpan ParseDuration(string? value) { if (string.IsNullOrWhiteSpace(value)) From 64ec70fecf3df3806d1aae6b630502a9b04503fb Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:26:45 +0900 Subject: [PATCH 2/4] Sanitize TestTelemetry TRX diagnostics (#3443) --- changelog.d/unreleased/3443.security.md | 16 +++++++ tests/CodeIndex.Tests/TestTelemetryTests.cs | 44 +++++++++++++++++--- tools/CodeIndex.TestTelemetry/Program.cs | 46 +++++++++++++++++---- 3 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 changelog.d/unreleased/3443.security.md diff --git a/changelog.d/unreleased/3443.security.md b/changelog.d/unreleased/3443.security.md new file mode 100644 index 0000000000..baba748a7f --- /dev/null +++ b/changelog.d/unreleased/3443.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3443 +affected: + - tools/CodeIndex.TestTelemetry/Program.cs + - tests/CodeIndex.Tests/TestTelemetryTests.cs +--- + +## English + +- **Test telemetry TRX warnings now avoid leaking local paths and XML parser details (#3443)** — parse and inspection warnings now use relative TRX paths when possible and stable reason codes instead of raw exception messages. + +## 日本語 + +- **Test telemetry の TRX warning がローカルパスや XML parser 詳細を漏らさないようになりました (#3443)** — parse / inspection warning は可能な限り相対 TRX パスと安定した reason code を使い、生の例外メッセージを出力しないようにしました。 diff --git a/tests/CodeIndex.Tests/TestTelemetryTests.cs b/tests/CodeIndex.Tests/TestTelemetryTests.cs index 5dc1f72c6a..7f90716e8e 100644 --- a/tests/CodeIndex.Tests/TestTelemetryTests.cs +++ b/tests/CodeIndex.Tests/TestTelemetryTests.cs @@ -119,8 +119,9 @@ public void Load_SkipsTrxFilesAboveSizeCap() try { var resultsDirectory = Path.Combine(projectRoot, "TestResults"); - Directory.CreateDirectory(resultsDirectory); - var largeTrx = Path.Combine(resultsDirectory, "too-large.trx"); + var nestedDirectory = Path.Combine(resultsDirectory, "nested"); + Directory.CreateDirectory(nestedDirectory); + var largeTrx = Path.Combine(nestedDirectory, "too-large.trx"); using (var stream = File.Create(largeTrx)) { stream.SetLength(TrxTelemetry.MaxTrxFileBytes + 1); @@ -130,8 +131,10 @@ public void Load_SkipsTrxFilesAboveSizeCap() Assert.Equal(1, summary.TrxFileCount); Assert.Equal(0, summary.Total); - Assert.Contains(summary.Warnings, warning => - warning.Contains("byte cap", StringComparison.Ordinal)); + var warning = Assert.Single(summary.Warnings); + Assert.Contains("byte cap", warning, StringComparison.Ordinal); + Assert.Contains("nested/too-large.trx", warning, StringComparison.Ordinal); + Assert.DoesNotContain(projectRoot, warning, StringComparison.Ordinal); } finally { @@ -162,7 +165,38 @@ public void Load_RejectsTrxDtds() Assert.Equal(0, summary.Total); Assert.Contains(summary.Warnings, warning => - warning.Contains("Could not parse", StringComparison.Ordinal)); + string.Equals(warning, "Could not parse with-dtd.trx: invalid_xml", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public void Load_SanitizesInvalidXmlWarnings() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_xml_warning"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + var nestedDirectory = Path.Combine(resultsDirectory, "nested"); + Directory.CreateDirectory(nestedDirectory); + File.WriteAllText(Path.Combine(nestedDirectory, "broken.trx"), """ + + + + + + """); + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + var warning = Assert.Single(summary.Warnings); + Assert.Equal("Could not parse nested/broken.trx: invalid_xml", warning); + Assert.DoesNotContain(projectRoot, warning, StringComparison.Ordinal); + Assert.DoesNotContain("Name cannot begin", warning, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("position", warning, StringComparison.OrdinalIgnoreCase); } finally { diff --git a/tools/CodeIndex.TestTelemetry/Program.cs b/tools/CodeIndex.TestTelemetry/Program.cs index f4fc927436..a26a49b485 100644 --- a/tools/CodeIndex.TestTelemetry/Program.cs +++ b/tools/CodeIndex.TestTelemetry/Program.cs @@ -116,7 +116,7 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) foreach (var path in trxFiles) { - if (!CanReadTrxFile(path, warnings)) + if (!CanReadTrxFile(resultsDirectory, path, warnings)) continue; try @@ -142,9 +142,9 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) AddTopResult(slowest, result, top); } } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or System.Xml.XmlException) + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or XmlException) { - warnings.Add($"Could not parse {path}: {ex.Message}"); + warnings.Add($"Could not parse {FormatTrxPath(resultsDirectory, path)}: {GetWarningReason(ex)}"); } } @@ -182,21 +182,21 @@ private static List EnumerateTrxFiles(string resultsDirectory, List warnings) + private static bool CanReadTrxFile(string resultsDirectory, string path, List warnings) { try { var file = new FileInfo(path); if (file.Length > MaxTrxFileBytes) { - warnings.Add($"TRX file exceeds {MaxTrxFileBytes} byte cap: {path}"); + warnings.Add($"TRX file exceeds {MaxTrxFileBytes} byte cap: {FormatTrxPath(resultsDirectory, path)}"); return false; } @@ -204,11 +204,43 @@ private static bool CanReadTrxFile(string path, List warnings) } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - warnings.Add($"Could not inspect {path}: {ex.Message}"); + warnings.Add($"Could not inspect {FormatTrxPath(resultsDirectory, path)}: {GetWarningReason(ex)}"); return false; } } + private static string FormatTrxPath(string resultsDirectory, string path) + { + try + { + var relativePath = Path.GetRelativePath(resultsDirectory, path); + if (!IsParentTraversal(relativePath) && !Path.IsPathFullyQualified(relativePath)) + return NormalizePath(relativePath); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException) + { + } + + var fileName = Path.GetFileName(path); + return string.IsNullOrWhiteSpace(fileName) ? "" : fileName; + } + + private static bool IsParentTraversal(string path) => + path == ".." || + path.StartsWith("../", StringComparison.Ordinal) || + path.StartsWith(@"..\", StringComparison.Ordinal); + + private static string NormalizePath(string path) => + path.Replace(Path.DirectorySeparatorChar, '/').Replace(Path.AltDirectorySeparatorChar, '/'); + + private static string GetWarningReason(Exception ex) => ex switch + { + XmlException => "invalid_xml", + UnauthorizedAccessException => "access_denied", + IOException => "io_error", + _ => "unknown_error" + }; + private static IEnumerable ReadResults(string path) { using var stream = File.OpenRead(path); From 968e51c08f76c30ab0dd7d73a227b480210a10a2 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:59:52 +0900 Subject: [PATCH 3/4] Complete TestTelemetry traversal bounds (#3442) --- changelog.d/unreleased/3442.security.md | 4 +- tests/CodeIndex.Tests/TestTelemetryTests.cs | 58 ++++++ tools/CodeIndex.TestTelemetry/Program.cs | 209 ++++++++++++++++---- 3 files changed, 230 insertions(+), 41 deletions(-) diff --git a/changelog.d/unreleased/3442.security.md b/changelog.d/unreleased/3442.security.md index aee6786701..e11d98b060 100644 --- a/changelog.d/unreleased/3442.security.md +++ b/changelog.d/unreleased/3442.security.md @@ -9,8 +9,8 @@ affected: ## English -- **Test telemetry TRX processing is now bounded (#3442)** — the telemetry helper now caps `--top`, TRX file discovery, TRX file size, XML parser work, and retained result collections so unusually large test result trees cannot force unbounded processing. +- **Test telemetry TRX processing is now bounded (#3442)** — the telemetry helper now caps `--top`, TRX directory and entry traversal, TRX file discovery, TRX file size, XML parser work, and retained result collections so unusually large test result trees cannot force unbounded processing. Malformed TRX files are also discarded atomically instead of contributing partial pass/fail counts. ## 日本語 -- **Test telemetry の TRX 処理に上限を設けました (#3442)** — telemetry helper は `--top`、TRX ファイル探索、TRX ファイルサイズ、XML parser の処理量、保持する結果コレクションを制限し、過大なテスト結果ツリーで無制限の処理が発生しないようにしました。 +- **Test telemetry の TRX 処理に上限を設けました (#3442)** — telemetry helper は `--top`、TRX ディレクトリ / エントリ走査、TRX ファイル探索、TRX ファイルサイズ、XML parser の処理量、保持する結果コレクションを制限し、過大なテスト結果ツリーで無制限の処理が発生しないようにしました。壊れた TRX ファイルは pass/fail 件数へ部分的に反映せず、ファイル単位で破棄します。 diff --git a/tests/CodeIndex.Tests/TestTelemetryTests.cs b/tests/CodeIndex.Tests/TestTelemetryTests.cs index 7f90716e8e..3004e41f67 100644 --- a/tests/CodeIndex.Tests/TestTelemetryTests.cs +++ b/tests/CodeIndex.Tests/TestTelemetryTests.cs @@ -112,6 +112,33 @@ public void Load_CapsTrxDiscovery() } } + [Fact] + public void Load_CapsTrxDirectoryTraversal() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_directory_cap"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + + for (var i = 0; i < TrxTelemetry.MaxTraversalDirectories + 1; i++) + { + Directory.CreateDirectory(Path.Combine(resultsDirectory, $"dir-{i:D4}")); + } + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(0, summary.TrxFileCount); + Assert.Equal(0, summary.Total); + Assert.Contains(summary.Warnings, warning => + warning.Contains("directory traversal cap", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public void Load_SkipsTrxFilesAboveSizeCap() { @@ -173,6 +200,37 @@ public void Load_RejectsTrxDtds() } } + [Fact] + public void Load_DiscardsPartialResultsFromMalformedTrx() + { + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_partial_xml"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + File.WriteAllText(Path.Combine(resultsDirectory, "partial.trx"), """ + + + + + + + """); + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(0, summary.Total); + Assert.Equal(0, summary.Passed); + Assert.Empty(summary.Slowest); + Assert.Contains(summary.Warnings, warning => + string.Equals(warning, "Could not parse partial.trx: invalid_xml", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public void Load_SanitizesInvalidXmlWarnings() { diff --git a/tools/CodeIndex.TestTelemetry/Program.cs b/tools/CodeIndex.TestTelemetry/Program.cs index a26a49b485..f8c3010e0d 100644 --- a/tools/CodeIndex.TestTelemetry/Program.cs +++ b/tools/CodeIndex.TestTelemetry/Program.cs @@ -83,6 +83,8 @@ public static class TrxTelemetry { public const int MaxTop = 100; public const int MaxTrxFiles = 256; + public const int MaxTraversalDirectories = 256; + public const int MaxTraversalEntries = 4096; public const long MaxTrxFileBytes = 16 * 1024 * 1024; public static TrxTelemetrySummary Load(string resultsDirectory, int top) @@ -107,12 +109,7 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) var warnings = new List(); var trxFiles = EnumerateTrxFiles(resultsDirectory, warnings); - var slowest = new List(Math.Min(top, 16)); - var failures = new List(Math.Min(top, 16)); - var total = 0; - var passed = 0; - var failed = 0; - var skipped = 0; + var results = new TrxResultAccumulator(top); foreach (var path in trxFiles) { @@ -121,26 +118,13 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) try { + var fileResults = new TrxResultAccumulator(top); foreach (var result in ReadResults(path)) { - total++; - - if (IsOutcome(result, "Passed")) - { - passed++; - } - else if (IsFailureOutcome(result)) - { - failed++; - AddTopResult(failures, result, top); - } - else if (IsOutcome(result, "NotExecuted") || IsOutcome(result, "Skipped")) - { - skipped++; - } - - AddTopResult(slowest, result, top); + fileResults.Add(result); } + + results.Merge(fileResults); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or XmlException) { @@ -148,45 +132,127 @@ public static TrxTelemetrySummary Load(string resultsDirectory, int top) } } - var other = total - passed - failed - skipped; - return new TrxTelemetrySummary( ResultsDirectory: resultsDirectory, TrxFileCount: trxFiles.Count, - Total: total, - Passed: passed, - Failed: failed, - Skipped: skipped, - Other: other, - Slowest: slowest, - Failures: failures, + Total: results.Total, + Passed: results.Passed, + Failed: results.Failed, + Skipped: results.Skipped, + Other: results.Other, + Slowest: results.Slowest, + Failures: results.Failures, Warnings: warnings); } private static List EnumerateTrxFiles(string resultsDirectory, List warnings) { var trxFiles = new List(MaxTrxFiles); + var pendingDirectories = new Queue(); + pendingDirectories.Enqueue(resultsDirectory); + var visitedDirectories = 0; + var visitedEntries = 0; - try + while (pendingDirectories.Count > 0) { - foreach (var path in Directory.EnumerateFiles(resultsDirectory, "*.trx", SearchOption.AllDirectories)) + if (visitedDirectories >= MaxTraversalDirectories) { + warnings.Add($"TRX directory traversal cap reached: visited first {MaxTraversalDirectories} directories."); + break; + } + + var directory = pendingDirectories.Dequeue(); + visitedDirectories++; + + foreach (var entry in EnumerateDirectoryEntries(directory, warnings)) + { + visitedEntries++; + if (visitedEntries > MaxTraversalEntries) + { + warnings.Add($"TRX entry traversal cap reached: visited first {MaxTraversalEntries} entries."); + trxFiles.Sort(StringComparer.Ordinal); + return trxFiles; + } + + if (!TryGetAttributes(entry, warnings, out var attributes)) + continue; + + if ((attributes & FileAttributes.ReparsePoint) != 0) + continue; + + if ((attributes & FileAttributes.Directory) != 0) + { + pendingDirectories.Enqueue(entry); + continue; + } + + if (!string.Equals(Path.GetExtension(entry), ".trx", StringComparison.OrdinalIgnoreCase)) + continue; + if (trxFiles.Count >= MaxTrxFiles) { warnings.Add($"TRX file cap reached: using first {MaxTrxFiles} files."); - break; + trxFiles.Sort(StringComparer.Ordinal); + return trxFiles; } - trxFiles.Add(path); + trxFiles.Add(entry); } } + + trxFiles.Sort(StringComparer.Ordinal); + return trxFiles; + } + + private static IEnumerable EnumerateDirectoryEntries(string directory, List warnings) + { + IEnumerator? enumerator; + try + { + enumerator = Directory.EnumerateFileSystemEntries(directory).GetEnumerator(); + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - warnings.Add($"Could not enumerate TRX files: {GetWarningReason(ex)}"); + warnings.Add($"Could not enumerate TRX directory: {GetWarningReason(ex)}"); + yield break; } - trxFiles.Sort(StringComparer.Ordinal); - return trxFiles; + using (enumerator) + { + while (true) + { + string entry; + try + { + if (!enumerator.MoveNext()) + yield break; + + entry = enumerator.Current; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + warnings.Add($"Could not enumerate TRX directory: {GetWarningReason(ex)}"); + yield break; + } + + yield return entry; + } + } + } + + private static bool TryGetAttributes(string path, List warnings, out FileAttributes attributes) + { + try + { + attributes = File.GetAttributes(path); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + warnings.Add($"Could not inspect TRX traversal entry: {GetWarningReason(ex)}"); + attributes = default; + return false; + } } private static bool CanReadTrxFile(string resultsDirectory, string path, List warnings) @@ -291,6 +357,71 @@ private static int CompareByDurationDescendingThenName(TrxTestResult left, TrxTe : string.Compare(left.TestName, right.TestName, StringComparison.Ordinal); } + private sealed class TrxResultAccumulator + { + private readonly int _top; + + public TrxResultAccumulator(int top) + { + _top = top; + Slowest = new List(Math.Min(top, 16)); + Failures = new List(Math.Min(top, 16)); + } + + public int Total { get; private set; } + + public int Passed { get; private set; } + + public int Failed { get; private set; } + + public int Skipped { get; private set; } + + public int Other => Total - Passed - Failed - Skipped; + + public List Slowest { get; } + + public List Failures { get; } + + public void Add(TrxTestResult result) + { + Total++; + + if (IsOutcome(result, "Passed")) + { + Passed++; + } + else if (IsFailureOutcome(result)) + { + Failed++; + AddTopResult(Failures, result, _top); + } + else if (IsOutcome(result, "NotExecuted") || IsOutcome(result, "Skipped")) + { + Skipped++; + } + + AddTopResult(Slowest, result, _top); + } + + public void Merge(TrxResultAccumulator other) + { + Total += other.Total; + Passed += other.Passed; + Failed += other.Failed; + Skipped += other.Skipped; + + foreach (var result in other.Slowest) + { + AddTopResult(Slowest, result, _top); + } + + foreach (var result in other.Failures) + { + AddTopResult(Failures, result, _top); + } + } + } + private static TimeSpan ParseDuration(string? value) { if (string.IsNullOrWhiteSpace(value)) From 295f095d2fd666c31f8cf4fac1f1c3e9cb2752e5 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sun, 14 Jun 2026 00:18:52 +0900 Subject: [PATCH 4/4] Skip non-regular TestTelemetry TRX entries (#3442) --- changelog.d/unreleased/3442.security.md | 4 +- tests/CodeIndex.Tests/TestTelemetryTests.cs | 29 ++++++++ tools/CodeIndex.TestTelemetry/Program.cs | 81 +++++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/changelog.d/unreleased/3442.security.md b/changelog.d/unreleased/3442.security.md index e11d98b060..6072239523 100644 --- a/changelog.d/unreleased/3442.security.md +++ b/changelog.d/unreleased/3442.security.md @@ -9,8 +9,8 @@ affected: ## English -- **Test telemetry TRX processing is now bounded (#3442)** — the telemetry helper now caps `--top`, TRX directory and entry traversal, TRX file discovery, TRX file size, XML parser work, and retained result collections so unusually large test result trees cannot force unbounded processing. Malformed TRX files are also discarded atomically instead of contributing partial pass/fail counts. +- **Test telemetry TRX processing is now bounded (#3442)** — the telemetry helper now caps `--top`, TRX directory and entry traversal, TRX file discovery, TRX file size, XML parser work, and retained result collections so unusually large test result trees cannot force unbounded processing. It also skips non-regular `.trx` filesystem entries and discards malformed TRX files atomically instead of contributing partial pass/fail counts. ## 日本語 -- **Test telemetry の TRX 処理に上限を設けました (#3442)** — telemetry helper は `--top`、TRX ディレクトリ / エントリ走査、TRX ファイル探索、TRX ファイルサイズ、XML parser の処理量、保持する結果コレクションを制限し、過大なテスト結果ツリーで無制限の処理が発生しないようにしました。壊れた TRX ファイルは pass/fail 件数へ部分的に反映せず、ファイル単位で破棄します。 +- **Test telemetry の TRX 処理に上限を設けました (#3442)** — telemetry helper は `--top`、TRX ディレクトリ / エントリ走査、TRX ファイル探索、TRX ファイルサイズ、XML parser の処理量、保持する結果コレクションを制限し、過大なテスト結果ツリーで無制限の処理が発生しないようにしました。通常ファイルではない `.trx` filesystem entry をスキップし、壊れた TRX ファイルは pass/fail 件数へ部分的に反映せずファイル単位で破棄します。 diff --git a/tests/CodeIndex.Tests/TestTelemetryTests.cs b/tests/CodeIndex.Tests/TestTelemetryTests.cs index 3004e41f67..6fb1d35d56 100644 --- a/tests/CodeIndex.Tests/TestTelemetryTests.cs +++ b/tests/CodeIndex.Tests/TestTelemetryTests.cs @@ -169,6 +169,32 @@ public void Load_SkipsTrxFilesAboveSizeCap() } } + [Fact] + public void Load_SkipsUnixFifoTrxEntries() + { + if (OperatingSystem.IsWindows()) + return; + + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_trx_telemetry_fifo"); + try + { + var resultsDirectory = Path.Combine(projectRoot, "TestResults"); + Directory.CreateDirectory(resultsDirectory); + var fifoPath = Path.Combine(resultsDirectory, "pipe.trx"); + if (Mkfifo(fifoPath, Convert.ToUInt32("600", 8)) != 0) + throw new IOException($"mkfifo failed with errno {System.Runtime.InteropServices.Marshal.GetLastWin32Error()}."); + + var summary = TrxTelemetry.Load(resultsDirectory, top: 1); + + Assert.Equal(0, summary.TrxFileCount); + Assert.Equal(0, summary.Total); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public void Load_RejectsTrxDtds() { @@ -270,4 +296,7 @@ private static string MinimalTrx(string testName) => $$""" """; + + [System.Runtime.InteropServices.DllImport("libc", EntryPoint = "mkfifo", SetLastError = true)] + private static extern int Mkfifo(string path, uint mode); } diff --git a/tools/CodeIndex.TestTelemetry/Program.cs b/tools/CodeIndex.TestTelemetry/Program.cs index f8c3010e0d..5280eba1d9 100644 --- a/tools/CodeIndex.TestTelemetry/Program.cs +++ b/tools/CodeIndex.TestTelemetry/Program.cs @@ -186,6 +186,9 @@ private static List EnumerateTrxFiles(string resultsDirectory, List EnumerateDirectoryEntries(string directory, L } } + private static bool IsRegularFile(string path, FileAttributes attributes, List warnings) + { + if ((attributes & (FileAttributes.Directory | FileAttributes.ReparsePoint | FileAttributes.Device)) != 0) + return false; + + if (OperatingSystem.IsWindows()) + return true; + + if (!UnixFileStatus.TryGetFileMode(path, out var mode)) + { + warnings.Add("Could not inspect TRX traversal entry: file_type_unavailable"); + return false; + } + + return (mode & UnixFileStatus.FileTypeMask) == UnixFileStatus.RegularFile; + } + private static bool TryGetAttributes(string path, List warnings, out FileAttributes attributes) { try @@ -422,6 +442,67 @@ public void Merge(TrxResultAccumulator other) } } + private static class UnixFileStatus + { + internal const int FileTypeMask = 0xF000; + internal const int RegularFile = 0x8000; + + internal static bool TryGetFileMode(string filePath, out int mode) + { + mode = 0; + try + { + if (NativeMethods.Stat(filePath, out var status) != 0) + return false; + + mode = status.Mode; + return true; + } + catch (DllNotFoundException) + { + return false; + } + catch (EntryPointNotFoundException) + { + return false; + } + } + + [System.Runtime.InteropServices.StructLayout(System.Runtime.InteropServices.LayoutKind.Sequential)] + private struct FileStatus + { + internal FileStatusFlags Flags; + internal int Mode; + internal uint Uid; + internal uint Gid; + internal long Size; + internal long ATime; + internal long ATimeNsec; + internal long MTime; + internal long MTimeNsec; + internal long CTime; + internal long CTimeNsec; + internal long BirthTime; + internal long BirthTimeNsec; + internal long Dev; + internal long RDev; + internal long Ino; + internal uint UserFlags; + } + + [System.Flags] + private enum FileStatusFlags : uint + { + None = 0, + } + + private static class NativeMethods + { + [System.Runtime.InteropServices.DllImport("libSystem.Native", EntryPoint = "SystemNative_Stat", CharSet = System.Runtime.InteropServices.CharSet.Ansi)] + internal static extern int Stat(string path, out FileStatus output); + } + } + private static TimeSpan ParseDuration(string? value) { if (string.IsNullOrWhiteSpace(value))