Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions RunCommand.Test/RunCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,56 @@ public async Task ExecuteAsyncShouldSkipANonExecutableFileOnTheOverlayPath()
Assert.AreSequenceEqual(ExpectedOverlayCopy, output, "Expected the search to pass over a file without an execute bit, as a shell does.");
}

[TestMethod]
public async Task ExecuteAsyncShouldSkipAFileOnTheOverlayPathThatOnlyOthersMayExecute()
{
SkipOnWindows();
if (Environment.UserName == "root")
{
Assert.Inconclusive("root may execute any file that has an execute bit, so the owner's missing bit is not visible.");
}

// Group and others may execute this file but its owner, the caller, may not, so a shell passes over it.
// Testing for any execute bit accepted it and the start failed with "Permission denied" (#139).
string root = CreateDirectoryForTest();
string othersOnly = Directory.CreateDirectory(Path.Join(root, "a")).FullName;
string executable = Directory.CreateDirectory(Path.Join(root, "b")).FullName;
string name = $"runcommand-{nameof(ExecuteAsyncShouldSkipAFileOnTheOverlayPathThatOnlyOthersMayExecute)}";
WriteScript(othersOnly, name, "echo ran the file the caller cannot run", executable: false);
if (!OperatingSystem.IsWindows())
{
File.SetUnixFileMode(
Path.Join(othersOnly, name),
UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.GroupExecute | UnixFileMode.OtherRead | UnixFileMode.OtherExecute);
}

WriteScript(executable, name, "echo ran overlay copy", executable: true);

List<string> output = await RunOnOverlayPathAsync(name, $"{othersOnly}{Path.PathSeparator}{executable}").ConfigureAwait(false);

Assert.AreSequenceEqual(ExpectedOverlayCopy, output, "Expected the search to ask whether the caller may execute the file, as a shell does.");
}

[TestMethod]
public void CanExecuteShouldReportWhetherTheCallerMayExecuteTheFile()
{
SkipOnWindows();
string directory = CreateDirectoryForTest();
WriteScript(directory, "runs", "exit 0", executable: true);
WriteScript(directory, "does-not-run", "exit 0", executable: false);

Assert.IsTrue(NativeMethods.CanExecute(Path.Join(directory, "runs")));
Assert.IsFalse(NativeMethods.CanExecute(Path.Join(directory, "does-not-run")));
}

[TestMethod]
public void CanExecuteShouldKeepTheFileWhenTheCLibraryCannotBeCalled()
{
// Without access(), the search keeps every existing file, as it did before it checked permissions.
Assert.IsTrue(NativeMethods.CanExecute("/any/file", (_, _) => throw new DllNotFoundException("libc")));
Assert.IsTrue(NativeMethods.CanExecute("/any/file", (_, _) => throw new EntryPointNotFoundException("access")));
}

[TestMethod]
public async Task ExecuteAsyncShouldFindTheToolOnTheCallersPathWhenTheOverlayRemovesPath()
{
Expand Down
52 changes: 52 additions & 0 deletions RunCommand/NativeMethods.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) 2023-2026 ktsu-dev contributors

namespace ktsu.RunCommand;

using System;
using System.Runtime.InteropServices;

/// <summary>
/// Calls into the C library on Linux and macOS.
/// </summary>
internal static class NativeMethods
{
/// <summary>The <c>X_OK</c> mode of <c>access</c>: test for permission to execute.</summary>
private const int ExecuteOk = 1;

/// <summary>
/// Returns whether the calling process may execute the file at <paramref name="path"/>.
/// </summary>
/// <param name="path">The full path of an existing file.</param>
/// <returns>
/// <see langword="true"/> when <c>access(path, X_OK)</c> succeeds, or when the C library cannot be called, which
/// keeps the file as a candidate the way the search did before it checked permissions.
/// </returns>
/// <remarks>
/// <c>access</c> answers for the caller, as a shell's <c>PATH</c> search does. Testing for any execute bit instead
/// would accept a <c>0700</c> file another user owns, and <c>File.GetUnixFileMode</c> does not exist below .NET 7,
/// so the net5.0, net6.0 and netstandard builds had no check at all (ktsu-dev/RunCommand#139).
/// </remarks>
internal static bool CanExecute(string path) => CanExecute(path, Access);

/// <summary>
/// Returns whether <paramref name="access"/> reports that the calling process may execute <paramref name="path"/>.
/// </summary>
/// <param name="path">The full path of an existing file.</param>
/// <param name="access">Performs <c>access(path, mode)</c>. A parameter so tests can make the C library unavailable.</param>
/// <returns>As <see cref="CanExecute(string)"/>.</returns>
internal static bool CanExecute(string path, Func<string, int, int> access)
{
try
{
return access(path, ExecuteOk) == 0;
}
catch (Exception exception) when (exception is DllNotFoundException or EntryPointNotFoundException)
{
return true;
}
}

[DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)]
[DllImport("libc", EntryPoint = "access", CharSet = CharSet.Ansi, BestFitMapping = false, ThrowOnUnmappableChar = true)]
private static extern int Access(string path, int mode);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the P/Invoke. The suggested fix would bring #139 back:

access(X_OK) is the managed-free equivalent of what a shell's PATH search asks. The interop is one call in NativeMethods, with DefaultDllImportSearchPaths set. It falls back to keeping the candidate if libc can't be loaded, and it only runs off Windows. I'm leaving this thread open for the maintainer's call.


Generated by Claude Code

}
9 changes: 3 additions & 6 deletions RunCommand/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 27 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command) =>
ExecuteAsync(command).Result;
Expand All @@ -35,7 +35,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 38 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler) =>
ExecuteAsync(command, outputHandler).Result;
Expand All @@ -46,7 +46,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 49 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, Elevation elevation) =>
ExecuteAsync(command, elevation).Result;
Expand All @@ -62,7 +62,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 65 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler, Elevation elevation) =>
ExecuteAsync(command, outputHandler, elevation).Result;
Expand Down Expand Up @@ -109,7 +109,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 112 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command)
=> await ExecuteAsync(command, new OutputHandler()).ConfigureAwait(false);
Expand All @@ -120,7 +120,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 123 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler)
=> await ExecuteAsync(command, outputHandler, Elevation.Default).ConfigureAwait(false);
Expand All @@ -131,7 +131,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 134 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, Elevation elevation)
=> await ExecuteAsync(command, new(), elevation).ConfigureAwait(false);
Expand All @@ -147,7 +147,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 150 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)
=> await ExecuteAsync(command, outputHandler, elevation, CancellationToken.None).ConfigureAwait(false);
Expand All @@ -160,7 +160,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 163 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, CancellationToken cancellationToken)
=> await ExecuteAsync(command, new OutputHandler(), cancellationToken).ConfigureAwait(false);
Expand All @@ -175,7 +175,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 178 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken)
=> await ExecuteAsync(command, outputHandler, Elevation.Default, cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -504,8 +504,8 @@
/// <param name="directory">A directory from the search path.</param>
/// <param name="fileName">The file name to look for.</param>
/// <returns>
/// The full path, or <see langword="null"/> if there is no such file. Outside Windows the file also needs an
/// execute permission bit, where the platform exposes one.
/// The full path, or <see langword="null"/> if there is no such file. Outside Windows the calling process also
/// needs permission to execute the file, as <c>access(X_OK)</c> reports it, on every target framework.
/// </returns>
private static string? TryExecutable(string directory, string fileName)
{
Expand All @@ -524,13 +524,10 @@
return null;
}

#if NET7_0_OR_GREATER
const UnixFileMode anyExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute;
if (!OperatingSystem.IsWindows() && (File.GetUnixFileMode(candidate) & anyExecute) == 0)
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !NativeMethods.CanExecute(candidate))
{
return null;
}
#endif

return candidate;
}
Expand Down
Loading