From 745410f5d43bff90c95c627aca4efca48ebbf133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 16 Jul 2026 16:22:56 +0200 Subject: [PATCH 1/3] Fix case-sensitivity probe corrupting directory path in MergeOutputFileHelper IsDirectoryCaseSensitive lower-cased the entire probe path, corrupting the directory portion. A case-insensitive child directory beneath a case-sensitive parent was then probed at a non-existent lowercased parent, so File.Exists returned false and the location was misreported as case-sensitive (Ordinal). That let EnsureOutputDoesNotAliasInput miss that a.trx and A.trx are the same file, risking overwrite of a read-only input. Now only the generated probe file name is lower-cased, keeping the real directory path intact. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f1a49b-b1e0-4f38-b8a1-3870dd0842d2 --- .../MergeOutputFileHelper.cs | 12 +++-- .../CtrfReportMergerTests.cs | 51 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs b/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs index 3b52b5f752..ab32581d9b 100644 --- a/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs +++ b/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs @@ -172,10 +172,16 @@ private static bool IsDirectoryCaseSensitive(string directory) { try { - string upper = Path.Combine(directory, "CASESENSITIVEPROBE" + Guid.NewGuid().ToString("N")); - using (new FileStream(upper, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, 0x1000, FileOptions.DeleteOnClose)) + string probeName = "CASESENSITIVEPROBE" + Guid.NewGuid().ToString("N"); + string probePath = Path.Combine(directory, probeName); + using (new FileStream(probePath, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, 0x1000, FileOptions.DeleteOnClose)) { - return !File.Exists(upper.ToLowerInvariant()); + // Only lower-case the generated probe FILE NAME, keeping the real 'directory' path intact. + // Lower-casing the whole path would corrupt the directory portion, so a case-insensitive + // child directory sitting beneath a case-sensitive parent (e.g. an ext4 casefold dir named + // with uppercase chars) would be probed at a non-existent lowercased parent, File.Exists + // would return false, and the location would be misreported as case-sensitive. + return !File.Exists(Path.Combine(directory, probeName.ToLowerInvariant())); } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs index b103983f64..9ffa26864f 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs @@ -220,6 +220,57 @@ await Assert.ThrowsExactlyAsync( } } + [TestMethod] + public async Task MergeToFileAsync_WhenOutputDiffersFromInputOnlyByCase_IsRejectedOnCaseInsensitiveDirectory() + { + // Regression: the case-sensitivity probe used to lower-case the WHOLE candidate path, corrupting the + // directory portion, so a case-insensitive location (e.g. a Windows temp dir) could be misreported as + // case-sensitive. That made the alias check use an ordinal comparison and miss that 'report.json' and + // 'REPORT.json' are the SAME file, letting the merge overwrite a read-only input. With the probe fixed + // (only the generated file name is lower-cased), a case-only output must alias the input and be + // rejected on a case-insensitive directory; on a genuinely case-sensitive one the names are distinct. + string tempDirectory = Path.Combine(Path.GetTempPath(), $"ctrf-merge-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDirectory); + try + { + string input = Path.Combine(tempDirectory, "report.json"); + File.WriteAllText(input, BuildReport()); + + string casedOutput = Path.Combine(tempDirectory, "REPORT.json"); + if (IsDirectoryCaseInsensitive(tempDirectory)) + { + await Assert.ThrowsExactlyAsync( + () => CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None)); + Assert.IsTrue(File.Exists(input)); + } + else + { + await CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None); + Assert.IsTrue(File.Exists(input)); + Assert.IsTrue(File.Exists(casedOutput)); + } + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + private static bool IsDirectoryCaseInsensitive(string directory) + { + string probe = Path.Combine(directory, "CaseProbe" + Guid.NewGuid().ToString("N")); + File.WriteAllText(probe, string.Empty); + try + { + // Only the file name is lower-cased so the (possibly case-sensitive) directory path stays intact. + return File.Exists(Path.Combine(directory, Path.GetFileName(probe).ToLowerInvariant())); + } + finally + { + File.Delete(probe); + } + } + [TestMethod] public async Task MergeToFileAsync_WritesMergedFileToDisk() { From e061182214e47805a1418ad17b2ee97ce184c52f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 16 Jul 2026 16:51:20 +0200 Subject: [PATCH 2/3] Make case-sensitivity probe fix testable via a seam and add effective regression test Addresses review feedback: the previous black-box test could not distinguish the fix from the bug on ordinary filesystems (the regression only manifests on a mixed-sensitivity topology). Extract the probe-path construction into an internal BuildCaseFoldedProbePath seam and assert directly that only the generated file name is case-folded while the directory path is preserved. Reverting the production fix now fails this test on every platform. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f1a49b-b1e0-4f38-b8a1-3870dd0842d2 --- .../InternalAPI/InternalAPI.Unshipped.txt | 1 + .../InternalAPI/InternalAPI.Unshipped.txt | 1 + .../InternalAPI/InternalAPI.Unshipped.txt | 1 + .../MergeOutputFileHelper.cs | 17 +++++--- .../CtrfReportMergerTests.cs | 40 ++++++++++++++++--- 5 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt index 47b0d80c90..a576082f6b 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.CtrfReport/InternalAPI/InternalAPI.Unshipped.txt @@ -7,5 +7,6 @@ static Microsoft.Testing.Platform.Resources.PlatformResources.NamedPipeDirectory static Microsoft.Testing.Platform.Resources.PlatformResources.NamedPipePathTooLongErrorMessage.get -> string! static Microsoft.Testing.Platform.Services.ArtifactNamingHelper.ResolveAndSanitize(string! template, string! processName, string! processId, System.DateTimeOffset timestamp, System.Func! sanitizeLeafFileName) -> string! Microsoft.Testing.Extensions.MergeOutputFileHelper +static Microsoft.Testing.Extensions.MergeOutputFileHelper.BuildCaseFoldedProbePath(string! directory, string! probeFileName) -> string! static Microsoft.Testing.Extensions.MergeOutputFileHelper.EnsureOutputDoesNotAliasInput(System.Collections.Generic.IReadOnlyList! inputPaths, string! outputPath) -> void static Microsoft.Testing.Extensions.MergeOutputFileHelper.WriteViaTemporarySiblingAsync(string! outputPath, System.Func! writeToTempAsync) -> System.Threading.Tasks.Task! diff --git a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt index 1798ddc8b0..6e34273969 100644 --- a/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.JUnitReport/InternalAPI/InternalAPI.Unshipped.txt @@ -7,5 +7,6 @@ static Microsoft.Testing.Platform.Resources.PlatformResources.NamedPipeDirectory static Microsoft.Testing.Platform.Resources.PlatformResources.NamedPipePathTooLongErrorMessage.get -> string! static Microsoft.Testing.Platform.Services.ArtifactNamingHelper.ResolveAndSanitize(string! template, string! processName, string! processId, System.DateTimeOffset timestamp, System.Func! sanitizeLeafFileName) -> string! Microsoft.Testing.Extensions.MergeOutputFileHelper +static Microsoft.Testing.Extensions.MergeOutputFileHelper.BuildCaseFoldedProbePath(string! directory, string! probeFileName) -> string! static Microsoft.Testing.Extensions.MergeOutputFileHelper.EnsureOutputDoesNotAliasInput(System.Collections.Generic.IReadOnlyList! inputPaths, string! outputPath) -> void static Microsoft.Testing.Extensions.MergeOutputFileHelper.WriteViaTemporarySiblingAsync(string! outputPath, System.Func! writeToTempAsync) -> System.Threading.Tasks.Task! diff --git a/src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt index 457e1df4d2..29cc0293ea 100644 --- a/src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt @@ -20,5 +20,6 @@ static Microsoft.Testing.Platform.IPC.Serializers.BaseSerializer.WriteListPayloa const Microsoft.Testing.Platform.IPC.FailedTestResultMessageFieldsId.Expected = 10 -> ushort const Microsoft.Testing.Platform.IPC.FailedTestResultMessageFieldsId.Actual = 11 -> ushort Microsoft.Testing.Extensions.MergeOutputFileHelper +static Microsoft.Testing.Extensions.MergeOutputFileHelper.BuildCaseFoldedProbePath(string! directory, string! probeFileName) -> string! static Microsoft.Testing.Extensions.MergeOutputFileHelper.EnsureOutputDoesNotAliasInput(System.Collections.Generic.IReadOnlyList! inputPaths, string! outputPath) -> void static Microsoft.Testing.Extensions.MergeOutputFileHelper.WriteViaTemporarySiblingAsync(string! outputPath, System.Func! writeToTempAsync) -> System.Threading.Tasks.Task! diff --git a/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs b/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs index ab32581d9b..d0af88bf6d 100644 --- a/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs +++ b/src/Platform/SharedExtensionHelpers/MergeOutputFileHelper.cs @@ -176,12 +176,7 @@ private static bool IsDirectoryCaseSensitive(string directory) string probePath = Path.Combine(directory, probeName); using (new FileStream(probePath, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, 0x1000, FileOptions.DeleteOnClose)) { - // Only lower-case the generated probe FILE NAME, keeping the real 'directory' path intact. - // Lower-casing the whole path would corrupt the directory portion, so a case-insensitive - // child directory sitting beneath a case-sensitive parent (e.g. an ext4 casefold dir named - // with uppercase chars) would be probed at a non-existent lowercased parent, File.Exists - // would return false, and the location would be misreported as case-sensitive. - return !File.Exists(Path.Combine(directory, probeName.ToLowerInvariant())); + return !File.Exists(BuildCaseFoldedProbePath(directory, probeName)); } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException) @@ -192,6 +187,16 @@ private static bool IsDirectoryCaseSensitive(string directory) } } + // Builds the case-folded candidate used to detect case sensitivity: ONLY the generated probe file name + // is lower-cased, while the real 'directory' path is preserved verbatim. Lower-casing the whole combined + // path (the previous bug) corrupts the directory portion, so a case-insensitive child directory sitting + // beneath a case-sensitive, differently-cased ancestor (e.g. an ext4 casefold dir named with uppercase + // chars) would be probed at a non-existent lowercased ancestor: File.Exists returns false and the + // location is misreported as case-sensitive. Kept as its own seam so this behaviour can be unit-tested + // without needing to materialize a mixed-sensitivity filesystem. + internal static string BuildCaseFoldedProbePath(string directory, string probeFileName) + => Path.Combine(directory, probeFileName.ToLowerInvariant()); + private static string GetTempSiblingPath(string outputPath) { string directory = Path.GetDirectoryName(Path.GetFullPath(outputPath)) is { Length: > 0 } dir diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs index 9ffa26864f..fabc3ef803 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Reflection; using System.Text.Json.Nodes; using Microsoft.Testing.Extensions.CtrfReport; @@ -220,15 +221,42 @@ await Assert.ThrowsExactlyAsync( } } + [TestMethod] + public void BuildCaseFoldedProbePath_LowerCasesOnlyFileName_PreservingCaseSensitiveDirectory() + { + // Regression seam for the case-sensitivity probe. The probe compares the created file against a + // case-folded candidate to decide whether the directory is case-sensitive. The bug lower-cased the + // WHOLE combined path, corrupting the directory portion: a case-insensitive child directory beneath a + // case-sensitive, differently-cased ancestor was then probed at a non-existent lowercased ancestor, so + // File.Exists returned false and the directory was misreported as case-sensitive. That in turn made + // EnsureOutputDoesNotAliasInput compare paths ordinally and miss that 'a.trx' and 'A.trx' are the same + // file, risking overwrite of a read-only input. Assert directly that only the generated file name is + // case-folded while the (potentially case-sensitive, uppercased) directory path is preserved verbatim. + // Reverting the production fix to lower-case the whole path fails this test on every platform. + // The helper is an internal type linked into several extension assemblies, so it is reached via the + // unambiguous CtrfReport assembly (a simple-name reference would be ambiguous across those copies). + MethodInfo buildProbe = typeof(CtrfReportMerger).Assembly + .GetType("Microsoft.Testing.Extensions.MergeOutputFileHelper", throwOnError: true)! + .GetMethod("BuildCaseFoldedProbePath", BindingFlags.Static | BindingFlags.NonPublic)!; + + string directory = Path.Combine("SomeCaseSensitive", "PARENT", "Child"); + const string probeFileName = "CASESENSITIVEPROBEabc123"; + + string candidate = (string)buildProbe.Invoke(null, [directory, probeFileName])!; + + Assert.AreEqual(Path.Combine(directory, "casesensitiveprobeabc123"), candidate); + Assert.AreEqual(directory, Path.GetDirectoryName(candidate)); + Assert.AreEqual("casesensitiveprobeabc123", Path.GetFileName(candidate)); + } + [TestMethod] public async Task MergeToFileAsync_WhenOutputDiffersFromInputOnlyByCase_IsRejectedOnCaseInsensitiveDirectory() { - // Regression: the case-sensitivity probe used to lower-case the WHOLE candidate path, corrupting the - // directory portion, so a case-insensitive location (e.g. a Windows temp dir) could be misreported as - // case-sensitive. That made the alias check use an ordinal comparison and miss that 'report.json' and - // 'REPORT.json' are the SAME file, letting the merge overwrite a read-only input. With the probe fixed - // (only the generated file name is lower-cased), a case-only output must alias the input and be - // rejected on a case-insensitive directory; on a genuinely case-sensitive one the names are distinct. + // End-to-end sibling of the seam test above: on a case-insensitive directory (Windows/macOS temp dirs + // are), an output that differs from an input only by CASE aliases that input and must be rejected so a + // read-only source is never overwritten. On a genuinely case-sensitive directory the two names are + // distinct, so the merge is allowed. The directory's real sensitivity is probed independently (folding + // only the file name) so the assertion adapts to the host filesystem. string tempDirectory = Path.Combine(Path.GetTempPath(), $"ctrf-merge-{Guid.NewGuid():N}"); Directory.CreateDirectory(tempDirectory); try From 92cce01b8766c890b590115cf300d68468b03674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 16 Jul 2026 16:57:38 +0200 Subject: [PATCH 3/3] Split filesystem case-sensitivity merge test into two single-scenario tests Addresses the automated test-quality grade: the end-to-end case-only alias test used an if/else on filesystem sensitivity to pick which assertions ran. Split it into two focused tests (one asserting rejection on a case-insensitive filesystem, one asserting the merge is allowed on a case-sensitive filesystem), each skipping via Assert.Inconclusive on the non-matching host so exactly one scenario drives the assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f1a49b-b1e0-4f38-b8a1-3870dd0842d2 --- .../CtrfReportMergerTests.cs | 54 +++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs index fabc3ef803..7efdd64aba 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportMergerTests.cs @@ -250,33 +250,57 @@ public void BuildCaseFoldedProbePath_LowerCasesOnlyFileName_PreservingCaseSensit } [TestMethod] - public async Task MergeToFileAsync_WhenOutputDiffersFromInputOnlyByCase_IsRejectedOnCaseInsensitiveDirectory() + public async Task MergeToFileAsync_WhenOutputAliasesInputByCaseOnly_IsRejectedOnCaseInsensitiveFilesystem() { - // End-to-end sibling of the seam test above: on a case-insensitive directory (Windows/macOS temp dirs - // are), an output that differs from an input only by CASE aliases that input and must be rejected so a - // read-only source is never overwritten. On a genuinely case-sensitive directory the two names are - // distinct, so the merge is allowed. The directory's real sensitivity is probed independently (folding - // only the file name) so the assertion adapts to the host filesystem. + // End-to-end sibling of the seam test above, scoped to a single scenario: on a case-insensitive + // directory (Windows/macOS temp dirs are), an output that differs from an input only by CASE aliases + // that input and must be rejected so a read-only source is never overwritten. Skipped on a genuinely + // case-sensitive host, where the two names are distinct (that scenario is covered by the sibling test). string tempDirectory = Path.Combine(Path.GetTempPath(), $"ctrf-merge-{Guid.NewGuid():N}"); Directory.CreateDirectory(tempDirectory); try { + if (!IsDirectoryCaseInsensitive(tempDirectory)) + { + Assert.Inconclusive("Host temp filesystem is case-sensitive; covered by the case-sensitive sibling test."); + } + string input = Path.Combine(tempDirectory, "report.json"); File.WriteAllText(input, BuildReport()); string casedOutput = Path.Combine(tempDirectory, "REPORT.json"); + await Assert.ThrowsExactlyAsync( + () => CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None)); + Assert.IsTrue(File.Exists(input)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [TestMethod] + public async Task MergeToFileAsync_WhenOutputDiffersByCaseOnly_IsAllowedOnCaseSensitiveFilesystem() + { + // Complementary scenario to the case-insensitive test above: on a genuinely case-sensitive directory, + // an output that differs from an input only by CASE is a distinct file, so the merge is allowed and the + // input is preserved. Skipped on a case-insensitive host (that scenario is the sibling test). + string tempDirectory = Path.Combine(Path.GetTempPath(), $"ctrf-merge-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempDirectory); + try + { if (IsDirectoryCaseInsensitive(tempDirectory)) { - await Assert.ThrowsExactlyAsync( - () => CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None)); - Assert.IsTrue(File.Exists(input)); - } - else - { - await CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None); - Assert.IsTrue(File.Exists(input)); - Assert.IsTrue(File.Exists(casedOutput)); + Assert.Inconclusive("Host temp filesystem is case-insensitive; covered by the case-insensitive sibling test."); } + + string input = Path.Combine(tempDirectory, "report.json"); + File.WriteAllText(input, BuildReport()); + + string casedOutput = Path.Combine(tempDirectory, "REPORT.json"); + await CtrfReportMerger.MergeToFileAsync([input], casedOutput, CancellationToken.None); + Assert.IsTrue(File.Exists(input)); + Assert.IsTrue(File.Exists(casedOutput)); } finally {