Require an execute bit before selecting a Unix apphost

src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerProcess.cs

@@ -84,8 +84,8 @@ public int ProcessId
     /// Launches the MTP application at <paramref name="source"/> and waits for it to connect back.
     /// </summary>
     /// <param name="source">
-    /// Path to the test application. May be a managed <c>.dll</c> (launched via its sibling apphost
-    /// <c>.exe</c> when present, otherwise via <c>dotnet &lt;dll&gt;</c>) or a native <c>.exe</c>.
+    /// Path to the test application. May be a managed <c>.dll</c> (launched via its sibling apphost when that
+    /// apphost is usable on the current OS, otherwise via <c>dotnet &lt;dll&gt;</c>) or a native executable.
     /// </param>
     /// <param name="options">Client options (name, connection timeout, environment, logger).</param>
     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 <dll>`.
+        // 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 <dll>`.
         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,55 @@ 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);
     }
 
+    /// <summary>
+    /// Determines whether <paramref name="apphost"/> can actually be launched on the current operating system.
+    /// </summary>
+    /// <remarks>
+    /// 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
+    /// <c>Permission denied</c> 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 <c>dotnet &lt;dll&gt;</c>,
+    /// which needs no permissions on the apphost at all.
+    /// </remarks>
+    internal static bool IsUsableApphost(string apphost)
+    {
+        if (!File.Exists(apphost))
+        {
+            return false;
+        }
+
+#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;
+            return (File.GetUnixFileMode(apphost) & ExecuteBits) != 0;
+        }
+#endif
+
+        return true;
+    }
+
     private static void SafeStop(TcpListener listener, IMtpClientLogger logger)
     {
         try

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 <dll>`.
+        const string Logical = "Client/MtpServerProcess.cs";
+        const string Fence = "#if NET7_0_OR_GREATER";
+        const string GuardedCall = "File.GetUnixFileMode(";
+
+        List<string> 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()
     {

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;
+
+/// <summary>
+/// Tests for how <see cref="MtpServerProcess"/> chooses between a sibling apphost and <c>dotnet &lt;dll&gt;</c>.
+/// </summary>
+/// <remarks>
+/// 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 <c>App.exe</c> travels next to <c>App.dll</c> 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 <c>Permission denied</c>, so
+/// the launcher has to reject both and fall back to <c>dotnet &lt;dll&gt;</c>.
+/// </remarks>
+[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));
+    }
+
+    /// <summary>
+    /// Grants the owner execute permission only, so the test also proves the check does not require all three
+    /// execute bits.
+    /// </summary>
+    [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);
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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.
+            }
+        }
+    }
+}