From 7598bd37fcc023b6de6e2936ff9aa84d516166d3 Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Tue, 18 Aug 2026 16:05:16 +0200 Subject: [PATCH 1/2] Require an execute bit before selecting a Unix apphost The server-mode client picks a sibling apphost for a managed .dll by asking File.Exists, which says yes to a file that cannot be launched. A payload built on a Windows agent and run on a Linux machine arrives with its extensionless apphost stripped of POSIX permission bits, because a zip written on Windows records none. Process.Start then throws Permission denied and aborts the run instead of falling back. Gate the choice on IsUsableApphost, which additionally requires an execute bit on Unix. File.GetUnixFileMode is .NET 7+, so net462 and netstandard2.0 consumers keep the existence-only check, which stays correct because the apphost path probe is already OS-aware. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Client/MtpServerProcess.cs | 51 ++++- .../MtpServerProcessTests.cs | 203 ++++++++++++++++++ 2 files changed, 249 insertions(+), 5 deletions(-) create mode 100644 test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerProcessTests.cs diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs index 0b45971e1c..b54bc97523 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs @@ -84,8 +84,8 @@ public int ProcessId /// Launches the MTP application at and waits for it to connect back. /// /// - /// Path to the test application. May be a managed .dll (launched via its sibling apphost - /// .exe when present, otherwise via dotnet <dll>) or a native .exe. + /// Path to the test application. May be a managed .dll (launched via its sibling apphost when that + /// apphost is usable on the current OS, otherwise via dotnet <dll>) or a native executable. /// /// Client options (name, connection timeout, environment, logger). public static MtpServerProcess Start(string source, MtpServerClientOptions? options = null) @@ -284,17 +284,19 @@ private static string GetStandardError(StringBuilder buffer) } } - private static LaunchCommand BuildLaunch(string source, int port) + internal static LaunchCommand BuildLaunch(string source, int port) { string serverArgs = $"{ServerArgument} {ClientPortArgument} {port} {NoBannerArgument}"; string workingDirectory = Path.GetDirectoryName(source) ?? Directory.GetCurrentDirectory(); string extension = Path.GetExtension(source); // A managed .NET assembly must be launched through its apphost (preferred) or `dotnet `. + // The apphost is only preferred when it is actually launchable here: a candidate that merely + // exists is not enough (see IsUsableApphost), and an unusable one falls back to `dotnet `. if (extension.Equals(".dll", StringComparison.OrdinalIgnoreCase)) { string apphost = GetAppHostPath(source); - return File.Exists(apphost) + return IsUsableApphost(apphost) ? new LaunchCommand(apphost, serverArgs, workingDirectory) : new LaunchCommand("dotnet", $"\"{source}\" {serverArgs}", workingDirectory); } @@ -307,7 +309,7 @@ private static LaunchCommand BuildLaunch(string source, int port) // A named launch descriptor rather than a value tuple: System.ValueTuple is not in the .NET // Framework before 4.7, and this source is compiled into consumers that may target net462 without // referencing the System.ValueTuple package. A tiny class keeps the package dependency-free. - private sealed class LaunchCommand + internal sealed class LaunchCommand { public LaunchCommand(string fileName, string arguments, string workingDirectory) { @@ -335,12 +337,51 @@ private static string GetAppHostPath(string managedAssembly) #else bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); #endif + + // The probe is OS-aware rather than always appending ".exe": a test payload built on a Windows + // agent and executed on a Linux machine (the dotnet/aspnetcore Helix layout) ships a Windows PE + // `Foo.exe` next to `Foo.dll`. Probing for ".exe" on Linux finds that Windows binary and launching + // it aborts the run, which is the CI failure fixed in microsoft/vstest#16336. A Unix apphost has + // no extension, so asking for the right name per OS never selects the foreign one. string appHostFileName = isWindows ? nameWithoutExtension + ".exe" : nameWithoutExtension; return Path.Combine(directory, appHostFileName); } + /// + /// Determines whether can actually be launched on the current operating system. + /// + /// + /// Existence is not sufficient on Unix. Archive formats used to move test payloads between agents (zip in + /// particular) do not carry the POSIX permission bits, so an extensionless apphost that survives a + /// Windows-build/Linux-run round trip can arrive without its execute bit. Launching such a file throws + /// Permission denied instead of degrading, which is the second half of the fix in + /// microsoft/vstest#16336. Requiring an execute bit lets the caller fall back to dotnet <dll>, + /// which needs no permissions on the apphost at all. + /// + internal static bool IsUsableApphost(string apphost) + { + if (!File.Exists(apphost)) + { + return false; + } + +#if NETCOREAPP + // File.GetUnixFileMode is .NET 7+. Consumers compiling the netstandard2.0 slice (net462, and + // net5.0-net7.0 which select that slice) fall back to the existence check above. That stays correct + // because GetAppHostPath already refuses to consider a Windows ".exe" on Unix, so the only case left + // uncovered there is an extensionless file that lost its execute bit. + if (!OperatingSystem.IsWindows()) + { + const UnixFileMode ExecuteBits = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + return (File.GetUnixFileMode(apphost) & ExecuteBits) != 0; + } +#endif + + return true; + } + private static void SafeStop(TcpListener listener, IMtpClientLogger logger) { try diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerProcessTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerProcessTests.cs new file mode 100644 index 0000000000..412882e885 --- /dev/null +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerProcessTests.cs @@ -0,0 +1,203 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests; + +/// +/// Tests for how chooses between a sibling apphost and dotnet <dll>. +/// +/// +/// The scenario these guard is a test payload built on a Windows agent and executed on a Linux machine. Two +/// things go wrong there: a Windows PE App.exe travels next to App.dll and is not a Linux +/// executable, and a zip round trip does not carry POSIX permission bits, so the real extensionless apphost +/// can arrive without its execute bit. Launching either one aborts the run with Permission denied, so +/// the launcher has to reject both and fall back to dotnet <dll>. +/// +[TestClass] +public sealed class MtpServerProcessTests +{ + // Any value works: BuildLaunch only formats the port into the argument string, it never binds it. + private const int Port = 12345; + + [TestMethod] + public void BuildLaunchWhenSourceIsExeLaunchesItDirectly() + { + using var temp = TempDirectory.Create(); + string exe = temp.CreateFile("App.exe"); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(exe, Port); + + Assert.AreEqual(exe, launch.FileName, "A native executable source must be launched directly."); + Assert.AreEqual(temp.Path, launch.WorkingDirectory); + Assert.Contains("--server", launch.Arguments); + Assert.Contains($"--client-port {Port}", launch.Arguments); + Assert.Contains("--no-banner", launch.Arguments); + } + + [TestMethod] + public void BuildLaunchWhenDllHasNoApphostFallsBackToDotnet() + { + using var temp = TempDirectory.Create(); + string dll = temp.CreateFile("App.dll"); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(dll, Port); + + Assert.AreEqual("dotnet", launch.FileName, "Without an apphost the assembly must be launched by the muxer."); + Assert.Contains($"\"{dll}\"", launch.Arguments, "The muxer needs the quoted assembly path as its first argument."); + Assert.AreEqual(temp.Path, launch.WorkingDirectory); + } + + [TestMethod] + [OSCondition(ConditionMode.Include, OperatingSystems.Windows, IgnoreMessage = "A '.exe' apphost is only launchable on Windows.")] + public void BuildLaunchOnWindowsSelectsSiblingExeApphost() + { + using var temp = TempDirectory.Create(); + string dll = temp.CreateFile("App.dll"); + string exe = temp.CreateFile("App.exe"); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(dll, Port); + + Assert.AreEqual(exe, launch.FileName, "On Windows the sibling '.exe' apphost is the preferred launch target."); + } + + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "Asserts that a Windows PE sibling is rejected, which only matters off Windows.")] + public void BuildLaunchOnUnixIgnoresSiblingWindowsExeAndFallsBackToDotnet() + { + using var temp = TempDirectory.Create(); + string dll = temp.CreateFile("App.dll"); + + // The Windows apphost that rode along in the payload. It exists, but it is a Windows PE binary. + _ = temp.CreateFile("App.exe"); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(dll, Port); + + Assert.AreEqual("dotnet", launch.FileName, "A Windows '.exe' must never be selected as the apphost on Unix."); + } + +#if NET + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "Unix file modes are a Unix concept.")] + [UnsupportedOSPlatform("windows")] + public void BuildLaunchOnUnixSelectsExecutableExtensionlessApphost() + { + using var temp = TempDirectory.Create(); + string dll = temp.CreateFile("App.dll"); + string apphost = temp.CreateFile("App"); + MakeExecutable(apphost); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(dll, Port); + + Assert.AreEqual(apphost, launch.FileName, "An executable extensionless apphost is the preferred launch target on Unix."); + } + + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "Unix file modes are a Unix concept.")] + [UnsupportedOSPlatform("windows")] + public void BuildLaunchOnUnixIgnoresNonExecutableApphostAndFallsBackToDotnet() + { + using var temp = TempDirectory.Create(); + string dll = temp.CreateFile("App.dll"); + + // The apphost survived the trip but the archive dropped its permission bits. + string apphost = temp.CreateFile("App"); + File.SetUnixFileMode(apphost, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + MtpServerProcess.LaunchCommand launch = MtpServerProcess.BuildLaunch(dll, Port); + + Assert.AreEqual( + "dotnet", + launch.FileName, + "Starting a file with no execute bit throws 'Permission denied', so the launch must fall back to the muxer."); + } +#endif + + [TestMethod] + public void IsUsableApphostReturnsFalseWhenFileMissing() + { + using var temp = TempDirectory.Create(); + + Assert.IsFalse(MtpServerProcess.IsUsableApphost(Path.Combine(temp.Path, "Missing"))); + } + + [TestMethod] + [OSCondition(ConditionMode.Include, OperatingSystems.Windows, IgnoreMessage = "Windows has no execute bit, so this asserts the Windows-only branch.")] + public void IsUsableApphostOnWindowsReturnsTrueForExistingFile() + { + using var temp = TempDirectory.Create(); + string apphost = temp.CreateFile("App.exe"); + + Assert.IsTrue(MtpServerProcess.IsUsableApphost(apphost), "Windows has no execute bit, so existence is the whole check there."); + } + +#if NET + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "Unix file modes are a Unix concept.")] + [UnsupportedOSPlatform("windows")] + public void IsUsableApphostOnUnixReturnsFalseForNonExecutableFile() + { + using var temp = TempDirectory.Create(); + string apphost = temp.CreateFile("App"); + File.SetUnixFileMode(apphost, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + Assert.IsFalse(MtpServerProcess.IsUsableApphost(apphost)); + } + + [TestMethod] + [OSCondition(ConditionMode.Exclude, OperatingSystems.Windows, IgnoreMessage = "Unix file modes are a Unix concept.")] + [UnsupportedOSPlatform("windows")] + public void IsUsableApphostOnUnixReturnsTrueForExecutableFile() + { + using var temp = TempDirectory.Create(); + string apphost = temp.CreateFile("App"); + MakeExecutable(apphost); + + Assert.IsTrue(MtpServerProcess.IsUsableApphost(apphost)); + } + + /// + /// Grants the owner execute permission only, so the test also proves the check does not require all three + /// execute bits. + /// + [UnsupportedOSPlatform("windows")] + private static void MakeExecutable(string path) + => File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); +#endif + + private sealed class TempDirectory : IDisposable + { + private TempDirectory(string path) => Path = path; + + public string Path { get; } + + public static TempDirectory Create() + { + string path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"mtp-apphost-{Guid.NewGuid():N}"); + _ = Directory.CreateDirectory(path); + return new TempDirectory(path); + } + + /// + /// Creates an empty file in the directory and returns its full path. The content is irrelevant: the + /// launcher only looks at the path, its existence, and its permissions. + /// + public string CreateFile(string fileName) + { + string fullPath = System.IO.Path.Combine(Path, fileName); + File.WriteAllText(fullPath, string.Empty); + return fullPath; + } + + public void Dispose() + { + try + { + Directory.Delete(Path, recursive: true); + } + catch (IOException) + { + // Best-effort cleanup: a temp directory left behind must never fail a test. + } + } + } +} From 01d27ff089dac9ffea01a0f9471e287cd4b9765e Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Thu, 20 Aug 2026 12:03:33 +0200 Subject: [PATCH 2/2] Fence the Unix apphost execute-bit check on NET7_0_OR_GREATER The check was fenced on NETCOREAPP, which the pack transform rewrites to MTP_CLIENT_USE_MODERN_DOTNET. That symbol records which JSON slice a consumer compiles and is defined only for net8.0+, but NuGet serves the net5.0 slice to net5.0, net6.0 and net7.0 consumers alike. A Linux net7.0 consumer therefore stayed on the existence-only path, selected a non-executable apphost and failed with Permission denied, even though File.GetUnixFileMode is available to it. Fence on the target framework instead, which is what actually tracks the API and which the pack transform leaves alone. Add an anti-drift test asserting the call sits inside a NET7_0_OR_GREATER block in every packed slice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Client/MtpServerProcess.cs | 14 ++++-- .../MtpServerClientSourcePackageTests.cs | 50 +++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs index b54bc97523..041e28fa77 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs @@ -367,11 +367,15 @@ internal static bool IsUsableApphost(string apphost) return false; } -#if NETCOREAPP - // File.GetUnixFileMode is .NET 7+. Consumers compiling the netstandard2.0 slice (net462, and - // net5.0-net7.0 which select that slice) fall back to the existence check above. That stays correct - // because GetAppHostPath already refuses to consider a Windows ".exe" on Unix, so the only case left - // uncovered there is an extensionless file that lost its execute bit. +#if NET7_0_OR_GREATER + // Fenced on the target framework because File.GetUnixFileMode is .NET 7+. Deliberately not the + // package's modern-.NET compilation symbol: that one records which JSON slice a consumer compiles + // and is defined only for net8.0+, while NuGet serves the net5.0 slice to net5.0, net6.0 and net7.0 + // consumers alike. Fencing on it would drop a Linux net7.0 consumer back to the existence-only path + // even though it has the API. net462, netstandard2.0, net5.0 and net6.0 consumers genuinely lack the + // API and keep the existence check above, which stays correct because GetAppHostPath already refuses + // to consider a Windows ".exe" on Unix, so the only case left uncovered there is an extensionless + // file that lost its execute bit. if (!OperatingSystem.IsWindows()) { const UnixFileMode ExecuteBits = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs index cf5b6340d0..5df8f24676 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs @@ -337,6 +337,56 @@ public void SourcePackage_ShipsBuildTargets_AndNet462SafetyGuardsSurviveTransfor string.Join(Environment.NewLine, missingGuard)); } + [TestMethod] + public void SourcePackage_UnixApphostExecuteBitCheck_IsFencedOnTargetFramework_NotOnTheModernDotnetSymbol() + { + // File.GetUnixFileMode is .NET 7+, so the apphost execute-bit check is fenced on NET7_0_OR_GREATER, + // which the pack transform leaves alone and every net7.0+ consumer defines. It must NOT ride on + // MTP_CLIENT_USE_MODERN_DOTNET (what the transform makes of NETCOREAPP): that symbol records which + // JSON slice the consumer compiles and is defined only for net8.0+. NuGet serves the net5.0 slice to + // net5.0, net6.0 and net7.0 consumers, so reusing it here would drop a Linux net7.0 consumer back to + // the existence-only check, select a non-executable apphost, and abort with 'Permission denied' + // instead of falling back to `dotnet `. + const string Logical = "Client/MtpServerProcess.cs"; + const string Fence = "#if NET7_0_OR_GREATER"; + const string GuardedCall = "File.GetUnixFileMode("; + + List offenders = []; + foreach (string tfm in Package.TargetFrameworks) + { + if (!Package.PackedTextByTfm[tfm].TryGetValue(Logical, out string? text)) + { + offenders.Add($"{tfm}: '{Logical}' is not packed."); + continue; + } + + int call = text.IndexOf(GuardedCall, StringComparison.Ordinal); + if (call < 0) + { + offenders.Add($"{tfm}: no '{GuardedCall}' call, so the Unix apphost execute-bit check is gone."); + continue; + } + + int fence = text.IndexOf(Fence, StringComparison.Ordinal); + if (fence < 0 || call < fence) + { + offenders.Add($"{tfm}: '{GuardedCall}' is not fenced on '{Fence}'."); + continue; + } + + int endIf = text.IndexOf("#endif", fence, StringComparison.Ordinal); + if (endIf >= 0 && call > endIf) + { + offenders.Add($"{tfm}: '{GuardedCall}' sits outside the '{Fence}' block."); + } + } + + Assert.IsEmpty( + offenders, + $"Every consumer that has File.GetUnixFileMode must require an execute bit before selecting a Unix apphost:{Environment.NewLine}" + + string.Join(Environment.NewLine, offenders)); + } + [TestMethod] public void SourcePackage_JsoniteNamespace_IsPackageQualified_NotTopLevel() {