environment: check execute permission in TryLocateExecutable

In f1a1ae5 (environment: manually scan $PATH on POSIX systems,
2022-05-31) the `which`-based lookup was replaced with a manual PATH
scan that only checks FileExists, without verifying execute permissions.
Unlike `which`, this means a non-executable file earlier in PATH can
shadow a valid executable, causing process creation to fail when GCM
later tries to run the located path.

Add an IsExecutable check that verifies at least one execute bit is set
on POSIX systems, matching the behaviour of `which`. On Windows, any
existing file is considered executable. Guard the POSIX-specific
File.GetUnixFileMode call with #if !NETFRAMEWORK for net472
compatibility.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Author: mjcheetham

src/shared/Core.Tests/EnvironmentTests.cs

@@ -94,6 +94,7 @@ public void PosixEnvironment_TryLocateExecutable_Exists_ReturnTrueAndPath()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -116,6 +117,32 @@ public void PosixEnvironment_TryLocateExecutable_ExistsMultiple_ReturnTrueAndFir
                     ["/bin/foo"] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
+            fs.SetExecutable("/usr/local/bin/foo");
+            fs.SetExecutable("/bin/foo");
+            var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
+            var env = new PosixEnvironment(fs, envars);
+
+            bool actualResult = env.TryLocateExecutable(PosixExecName, out string actualPath);
+
+            Assert.True(actualResult);
+            Assert.Equal(expectedPath, actualPath);
+        }
+
+        [PosixFact]
+        public void PosixEnvironment_TryLocateExecutable_NotExecutable_SkipsToNextMatch()
+        {
+            string nonExecPath = "/home/john.doe/bin/foo";
+            string expectedPath = "/usr/local/bin/foo";
+            var fs = new TestFileSystem
+            {
+                Files = new Dictionary<string, byte[]>
+                {
+                    [nonExecPath] = Array.Empty<byte>(),
+                    [expectedPath] = Array.Empty<byte>(),
+                }
+            };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -142,6 +169,8 @@ public void MacOSEnvironment_TryLocateExecutable_Paths_Are_Ignored()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(pathsToIgnore.FirstOrDefault());
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 

src/shared/Core/EnvironmentBase.cs

@@ -138,7 +138,8 @@ internal virtual bool TryLocateExecutable(string program, ICollection<string> pa
                 {
                     string candidatePath = Path.Combine(basePath, program);
                     if (FileSystem.FileExists(candidatePath) && (pathsToIgnore is null ||
-                        !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase)))
+                        !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase))
+                        && FileSystem.FileIsExecutable(candidatePath))
                     {
                         path = candidatePath;
                         return true;

src/shared/Core/FileSystem.cs

@@ -34,6 +34,14 @@ public interface IFileSystem
         /// <returns>True if a file exists, false otherwise.</returns>
         bool FileExists(string path);
 
+        /// <summary>
+        /// Check if a file has execute permissions.
+        /// On Windows this always returns true. On POSIX it checks for any execute bit.
+        /// </summary>
+        /// <param name="path">Full path to file to test.</param>
+        /// <returns>True if the file is executable, false otherwise.</returns>
+        bool FileIsExecutable(string path);
+
         /// <summary>
         /// Check if a directory exists at the specified path.
         /// </summary>
@@ -122,6 +130,23 @@ public abstract class FileSystem : IFileSystem
 
         public bool FileExists(string path) => File.Exists(path);
 
+#if NETFRAMEWORK
+        public bool FileIsExecutable(string path) => true;
+#else
+        public bool FileIsExecutable(string path)
+        {
+            if (!PlatformUtils.IsPosix())
+                return true;
+
+#pragma warning disable CA1416 // Platform guard via PlatformUtils.IsPosix()
+            var mode = File.GetUnixFileMode(path);
+            return (mode & (UnixFileMode.UserExecute |
+                            UnixFileMode.GroupExecute |
+                            UnixFileMode.OtherExecute)) != 0;
+#pragma warning restore CA1416
+        }
+#endif
+
         public bool DirectoryExists(string path) => Directory.Exists(path);
 
         public string GetCurrentDirectory() => Directory.GetCurrentDirectory();

src/shared/TestInfrastructure/Objects/TestFileSystem.cs

@@ -11,6 +11,7 @@ public class TestFileSystem : IFileSystem
         public string UserHomePath { get; set; }
         public string UserDataDirectoryPath { get; set; }
         public IDictionary<string, byte[]> Files { get; set; } = new Dictionary<string, byte[]>();
+        public ISet<string> ExecutableFiles { get; } = new HashSet<string>();
         public ISet<string> Directories { get; set; } = new HashSet<string>();
         public string CurrentDirectory { get; set; } = Path.GetTempPath();
         public bool IsCaseSensitive { get; set; } = false;
@@ -36,6 +37,14 @@ bool IFileSystem.FileExists(string path)
             return Files.ContainsKey(path);
         }
 
+        bool IFileSystem.FileIsExecutable(string path)
+        {
+            if (!Files.ContainsKey(path))
+                throw new FileNotFoundException("File not found", path);
+
+            return ExecutableFiles.Contains(path);
+        }
+
         bool IFileSystem.DirectoryExists(string path)
         {
             return Directories.Contains(TrimSlash(path));
@@ -130,6 +139,20 @@ string[] IFileSystem.ReadAllLines(string path)
 
         #endregion
 
+        /// <summary>
+        /// Mark a test file as executable. File must exist in <see cref="Files"/> already.
+        /// </summary>
+        public void SetExecutable(string path, bool isExecutable = true)
+        {
+            if (!Files.ContainsKey(path))
+                throw new FileNotFoundException("File not found", path);
+
+            if (isExecutable)
+                ExecutableFiles.Add(path);
+            else
+                ExecutableFiles.Remove(path);
+        }
+
         /// <summary>
         /// Trim trailing slashes from a path.
         /// </summary>