Skip a non-executable PATH match when looking up a helper [patch]

NoSleep.Test/PlatformIntegrationTests.cs

@@ -3,6 +3,7 @@
 namespace ktsu.NoSleep.Test;
 
 using System;
+using System.IO;
 using System.Linq;
 using System.Reflection;
 using ktsu.NoSleep.Contracts;
@@ -74,6 +75,85 @@
 	public void Find_ReturnsNullForACommandThatIsNotOnThePath() =>
 		Assert.IsNull(ExecutableLocator.Find("definitely-not-a-real-command-9f3a1"));
 
+	[TestMethod]
+	public void Find_SkipsANonExecutableMatchEarlierOnThePath()
+	{
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Windows has no execute bit, so every match there counts as executable.");
+			return;
+		}
+
+		const string command = "nosleep-test-helper";
+		string root = Directory.CreateTempSubdirectory("nosleep-path-").FullName;
+
+		try
+		{
+			string shadow = Directory.CreateDirectory(Path.Join(root, "shadow")).FullName;
+			string real = Directory.CreateDirectory(Path.Join(root, "real")).FullName;
+
+			string stray = Path.Join(shadow, command);
+			File.WriteAllText(stray, "#!/bin/sh\n");
+			File.SetUnixFileMode(stray, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.OtherRead);
+
+			// A directory named like the command is not a match either.
+			string directoryNamedLikeTheCommand = Path.Join(root, "directory");
+			Directory.CreateDirectory(Path.Join(directoryNamedLikeTheCommand, command));
+
+			string helper = Path.Join(real, command);
+			File.WriteAllText(helper, "#!/bin/sh\n");
+			File.SetUnixFileMode(helper, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
+
+			string path = string.Join(Path.PathSeparator, shadow, directoryNamedLikeTheCommand, real);
+
+			Assert.AreEqual(helper, ExecutableLocator.Find(command, path));
+			Assert.IsNull(ExecutableLocator.Find(command, string.Join(Path.PathSeparator, shadow, directoryNamedLikeTheCommand)));
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	[TestMethod]
+	public void Find_ReturnsAnExecutableMatchOnTheGivenPath()
+	{
+		string command = OperatingSystem.IsWindows() ? "nosleep-test-helper.exe" : "nosleep-test-helper";
+		string root = Directory.CreateTempSubdirectory("nosleep-path-").FullName;
+
+		try
+		{
+			string helper = Path.Join(root, command);
+			File.WriteAllText(helper, "#!/bin/sh\n");
+			if (!OperatingSystem.IsWindows())
+			{
+				File.SetUnixFileMode(helper, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
+			}
+
+			Assert.AreEqual(helper, ExecutableLocator.Find(command, root));
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	[TestMethod]
+	[DataRow(UnixFileMode.UserExecute, true)]
+	[DataRow(UnixFileMode.GroupExecute, true)]
+	[DataRow(UnixFileMode.OtherExecute, true)]
+	[DataRow(UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.OtherRead, false)]
+	[DataRow(UnixFileMode.None, false)]
+	public void HasExecuteBit_IsTrueOnlyWhenSomeoneMayExecute(UnixFileMode mode, bool expected) =>
+		Assert.AreEqual(expected, ExecutableLocator.HasExecuteBit("helper", _ => mode));
+
+	[TestMethod]
+	public void HasExecuteBit_IsFalseWhenTheModeCannotBeRead()
+	{
+		Assert.IsFalse(ExecutableLocator.HasExecuteBit("helper", _ => throw new FileNotFoundException("gone")));
+		Assert.IsFalse(ExecutableLocator.HasExecuteBit("helper", _ => throw new UnauthorizedAccessException("denied")));
+	}
+
 	[TestMethod]
 	[DataRow("")]
 	[DataRow("   ")]

NoSleep/Platforms/ExecutableLocator.cs

@@ -20,11 +20,23 @@ public static class ExecutableLocator
 	/// <param name="fileName">The command name to look for, without a directory.</param>
 	/// <returns>The full path to the first match, or <see langword="null"/> when nothing on <c>PATH</c> matches.</returns>
 	/// <exception cref="ArgumentException"><paramref name="fileName"/> is <see langword="null"/> or whitespace.</exception>
-	public static string? Find(string fileName)
+	/// <remarks>
+	/// A file that cannot be executed is skipped, as the shell, <c>command -v</c> and <c>execvp</c> skip it, so a
+	/// stray non-executable copy earlier on <c>PATH</c> does not hide a working helper later on it.
+	/// </remarks>
+	public static string? Find(string fileName) => Find(fileName, Environment.GetEnvironmentVariable("PATH"));
+
+	/// <summary>
+	/// Finds an executable on the given search path.
+	/// </summary>
+	/// <param name="fileName">The command name to look for, without a directory.</param>
+	/// <param name="path">The search path, in the same form as the <c>PATH</c> environment variable.</param>
+	/// <returns>The full path to the first match, or <see langword="null"/> when nothing on <paramref name="path"/> matches.</returns>
+	/// <exception cref="ArgumentException"><paramref name="fileName"/> is <see langword="null"/> or whitespace.</exception>
+	internal static string? Find(string fileName, string? path)
 	{
 		ArgumentException.ThrowIfNullOrWhiteSpace(fileName);
 
-		string? path = Environment.GetEnvironmentVariable("PATH");
 		if (string.IsNullOrEmpty(path))
 		{
 			return null;
@@ -50,7 +62,7 @@ public static class ExecutableLocator
 				continue;
 			}
 
-			if (File.Exists(candidate))
+			if (IsExecutableFile(candidate))
 			{
 				return candidate;
 			}
@@ -59,6 +71,55 @@ public static class ExecutableLocator
 		return null;
 	}
 
+	/// <summary>
+	/// Reports whether a path names a regular file that this process could execute.
+	/// </summary>
+	/// <remarks>
+	/// <see cref="File.Exists(string)"/> is false for a directory already. Off Windows, a file with no execute bit
+	/// for anyone is skipped; Windows has no such bit, so any file counts there.
+	/// </remarks>
+	private static bool IsExecutableFile(string candidate)
+	{
+		if (!File.Exists(candidate))
+		{
+			return false;
+		}
+
+		if (OperatingSystem.IsWindows())
+		{
+			return true;
+		}
+
+		return HasExecuteBit(candidate, File.GetUnixFileMode);
+	}
+
+	/// <summary>
+	/// Reports whether the file's mode, as <paramref name="readMode"/> reads it, grants execute to anyone.
+	/// </summary>
+	/// <param name="candidate">The file to ask about.</param>
+	/// <param name="readMode">Reads the file's Unix mode; <see cref="File.GetUnixFileMode(string)"/> in production.</param>
+	/// <returns><see langword="true"/> when any execute bit is set; <see langword="false"/> when none is, or the mode cannot be read.</returns>
+	/// <remarks>
+	/// The mode reader is a parameter so the decision, including what a file that vanishes or cannot be read
+	/// means, is covered on every platform rather than only where the bit exists.
+	/// </remarks>
+	internal static bool HasExecuteBit(string candidate, Func<string, UnixFileMode> readMode)
+	{
+		try
+		{
+			const UnixFileMode anyExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute;
+			return (readMode(candidate) & anyExecute) != 0;
+		}
+		catch (IOException)
+		{
+			return false;
+		}
+		catch (UnauthorizedAccessException)
+		{
+			return false;
+		}
+	}
+
 	/// <summary>
 	/// Reports whether an executable is available on <c>PATH</c>.
 	/// </summary>