From 4f3f95074cdd20f1106e8bf562d2dfbe538ffeda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:33:47 +0000 Subject: [PATCH 01/28] Initial plan From 92de2998c9470c6d5da14ea519f77dd5fe438465 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:42:28 +0000 Subject: [PATCH 02/28] Add ProcessStartOptions class with path resolution and tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Kernel32/Interop.GetWindowsDirectoryW.cs | 13 + .../ref/System.Diagnostics.Process.cs | 11 + .../src/System.Diagnostics.Process.csproj | 6 + .../System/Diagnostics/ProcessStartOptions.cs | 348 ++++++++++++++++++ .../tests/ProcessStartOptionsTests.Unix.cs | 203 ++++++++++ .../tests/ProcessStartOptionsTests.Windows.cs | 200 ++++++++++ .../tests/ProcessStartOptionsTests.cs | 224 +++++++++++ .../System.Diagnostics.Process.Tests.csproj | 3 + 8 files changed, 1008 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs new file mode 100644 index 00000000000000..05710a47a019f9 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + internal static partial uint GetWindowsDirectoryW(ref char lpBuffer, uint uSize); + } +} diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 17366ae9811736..c6b6565016ea69 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -261,6 +261,17 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string WorkingDirectory { get { throw null; } set { } } } + public sealed partial class ProcessStartOptions + { + public ProcessStartOptions(string fileName) { } + public System.Collections.Generic.IList Arguments { get { throw null; } set { } } + public bool CreateNewProcessGroup { get { throw null; } set { } } + public System.Collections.Generic.IDictionary Environment { get { throw null; } } + public string FileName { get { throw null; } } + public System.Collections.Generic.IList InheritedHandles { get { throw null; } set { } } + public bool KillOnParentExit { get { throw null; } set { } } + public string? WorkingDirectory { get { throw null; } set { } } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 986ee67013d3d7..a674056838985d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -12,6 +12,7 @@ $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) SR.Process_PlatformNotSupported true + $(DefineConstants);WINDOWS @@ -26,6 +27,7 @@ + @@ -136,6 +138,10 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> + + + /// Specifies options for starting a new process. + /// + public sealed class ProcessStartOptions + { + private readonly string _fileName; + private IList? _arguments; + private DictionaryWrapper? _environment; + private IList? _inheritedHandles; + + /// + /// Gets the application to start. + /// + public string FileName => _fileName; + + /// + /// Gets or sets the command-line arguments to pass to the application. + /// + public IList Arguments + { + get => _arguments ??= new List(); + set => _arguments = value; + } + + /// + /// Gets the environment variables that apply to this process and its child processes. + /// + /// + /// By default, the environment is a copy of the current process environment. + /// + public IDictionary Environment + { + get + { + if (_environment == null) + { + IDictionary envVars = System.Environment.GetEnvironmentVariables(); + + _environment = new DictionaryWrapper(new Dictionary( + envVars.Count, + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + + // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. + IDictionaryEnumerator e = envVars.GetEnumerator(); + Debug.Assert(!(e is IDisposable), "Environment.GetEnvironmentVariables should not be IDisposable."); + while (e.MoveNext()) + { + DictionaryEntry entry = e.Entry; + _environment.Add((string)entry.Key, (string?)entry.Value); + } + } + return _environment; + } + } + + /// + /// Gets or sets the working directory for the process to be started. + /// + public string? WorkingDirectory { get; set; } + + /// + /// Gets a list of handles that will be inherited by the child process. + /// + /// + /// + /// Handles do not need to have inheritance enabled beforehand. + /// They are also not duplicated, just added as-is to the child process + /// so the exact same handle values can be used in the child process. + /// + /// + /// On Windows, the implementation will automatically enable inheritance on any handle added to this list + /// by modifying the handle's flags using SetHandleInformation. + /// + /// + /// On Unix, the implementation will modify the copy of every handle in the child process + /// by removing FD_CLOEXEC flag. It happens after the fork and before the exec, so it does not affect parent process. + /// + /// + public IList InheritedHandles + { + get => _inheritedHandles ??= new List(); + set => _inheritedHandles = value; + } + + /// + /// Gets or sets a value indicating whether the child process should be terminated when the parent process exits. + /// + public bool KillOnParentExit { get; set; } + + /// + /// Gets or sets a value indicating whether to create the process in a new process group. + /// + /// + /// + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// on Windows and provides process group isolation on all platforms. + /// + /// + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// process group, which can be useful for background processes that should continue running independently. + /// + /// + public bool CreateNewProcessGroup { get; set; } + + /// + /// Initializes a new instance of the class. + /// + /// The application to start. + /// Thrown when is null or empty. + /// Thrown when cannot be resolved to an existing file. + public ProcessStartOptions(string fileName) + { + ArgumentException.ThrowIfNullOrEmpty(fileName); + + _fileName = ResolvePath(fileName); + } + + /// Resolves a path to the filename. + /// The filename. + /// The resolved path. + /// Thrown when cannot be resolved to an existing file. + internal static string ResolvePath(string filename) + { + // If the filename is a complete path, use it, regardless of whether it exists. + if (Path.IsPathRooted(filename)) + { + // In this case, it doesn't matter whether the file exists or not; + // it's what the caller asked for, so it's what they'll get + return filename; + } + +#if WINDOWS + // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + // "If the file name does not contain an extension, .exe is appended. + // Therefore, if the file name extension is .com, this parameter must include the .com extension. + // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." + + // HasExtension returns false for trailing dot, so we need to check that separately + if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + { + filename += ".exe"; + } +#endif + + // Then check the executable's directory + string? path = Environment.ProcessPath; + if (path != null) + { + try + { + path = Path.Combine(Path.GetDirectoryName(path)!, filename); + if (File.Exists(path)) + { + return path; + } + } + catch (ArgumentException) { } // ignore any errors in data that may come from the exe path + } + + // Then check the current directory + path = Path.Combine(Directory.GetCurrentDirectory(), filename); + if (File.Exists(path)) + { + return path; + } + +#if WINDOWS + // Windows-specific search locations (from CreateProcessW documentation) + + // Check the 32-bit Windows system directory (It can't change over app lifetime) + path = GetSystemDirectory(); + if (path != null) + { + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } + + // Check the Windows directory + path = GetWindowsDirectory(); + if (path != null) + { + // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + string systemPath = Path.Combine(path, "System", filename); + if (File.Exists(systemPath)) + { + return systemPath; + } + + // Check the Windows directory itself + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } +#endif + + // Then check each directory listed in the PATH environment variables + return FindProgramInPath(filename); + } + + /// + /// Gets the path to the program by searching in PATH environment variable. + /// + /// The program name. + /// The full path to the program. + /// Thrown when the program cannot be found in PATH. + private static string FindProgramInPath(string program) + { + string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + if (pathEnvVar != null) + { +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + while (pathParser.MoveNext()) + { + string subPath = pathParser.ExtractCurrent(); + string path = Path.Combine(subPath, program); + if (IsExecutableFile(path)) + { + return path; + } + } + } + + throw new FileNotFoundException("Could not resolve the file.", program); + } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return IsExecutable(path); +#endif + } + +#if !WINDOWS + private static bool IsExecutable(string fullPath) + { + Interop.Sys.FileStatus fileinfo; + + if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) + { + return false; + } + + // Check if the path is a directory. + if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) + { + return false; + } + + const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + + UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; + + // Avoid checking user/group when permission. + if (permissions == AllExecute) + { + return true; + } + else if (permissions == 0) + { + return false; + } + + uint euid = Interop.Sys.GetEUid(); + + if (euid == 0) + { + return true; // We're root. + } + + if (euid == fileinfo.Uid) + { + // We own the file. + return (permissions & UnixFileMode.UserExecute) != 0; + } + + bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; + bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; + + // Avoid group check when group and other have same permissions. + if (groupCanExecute == otherCanExecute) + { + return groupCanExecute; + } + + if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) + { + return groupCanExecute; + } + else + { + return otherCanExecute; + } + } +#endif + +#if WINDOWS + private static string? s_cachedSystemDirectory; + + private static string? GetSystemDirectory() + { + if (s_cachedSystemDirectory == null) + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetSystemDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedSystemDirectory = new string(buffer.Slice(0, (int)length)); + } + } + return s_cachedSystemDirectory; + } + + private static string? GetWindowsDirectory() + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + return new string(buffer.Slice(0, (int)length)); + } + return null; + } +#endif + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs new file mode 100644 index 00000000000000..2cb8c3cc5f5488 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -0,0 +1,203 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Xunit; + +namespace System.Diagnostics.Tests +{ + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + public class ProcessStartOptionsTests_Unix + { + [Fact] + public void TestResolvePath_FindsInPath() + { + // sh should be findable in PATH on all Unix systems + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("sh", options.FileName); + } + + [Fact] + public void TestResolvePath_DoesNotAddExeExtension() + { + // On Unix, no .exe extension should be added + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testscript.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_PathSeparatorIsColon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testscript"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Add temp directory to PATH using colon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [Fact] + public void TestResolvePath_ChecksExecutablePermissions() + { + // Create a file without execute permissions + string tempDir = Path.GetTempPath(); + string fileName = "nonexecutable.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Explicitly make it non-executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + // Should throw because file is not executable + Assert.Throws(() => new ProcessStartOptions(fileName)); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + + [Fact] + public void TestResolvePath_FindsCommonUtilities() + { + // Test common Unix utilities + string[] commonUtils = { "ls", "cat", "echo", "sh" }; + + foreach (string util in commonUtils) + { + ProcessStartOptions options = new ProcessStartOptions(util); + Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); + Assert.Contains(util, options.FileName); + } + } + + [Fact] + public void TestResolvePath_RejectsDirectories() + { + // Create a directory with executable permissions + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + // Try to use the directory name as a command + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs new file mode 100644 index 00000000000000..287cb573fedf16 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -0,0 +1,200 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests_Windows + { + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AddsExeExtension() + { + // Test that .exe is appended when no extension is provided + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + { + // "If the file name ends in a period (.) with no extension, .exe is not appended." + // This should fail since "notepad." won't exist + Assert.Throws(() => new ProcessStartOptions("notepad.")); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PreservesComExtension() + { + // The .com extension should be preserved + string fileName = "test.com"; + string tempDir = Path.GetTempPath(); + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInSystemDirectory() + { + // cmd.exe should be found in system directory + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInWindowsDirectory() + { + // Some utilities are in Windows directory + // We'll test with a file that's commonly in Windows directory + // Note: This might not exist on all systems, so we make it conditional + try + { + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); + } + catch (FileNotFoundException) + { + // Skip if notepad is not found - it's not critical for this test + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInPath() + { + // powershell.exe should be findable in PATH on most Windows systems + try + { + ProcessStartOptions options = new ProcessStartOptions("powershell"); + Assert.True(File.Exists(options.FileName)); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + } + catch (FileNotFoundException) + { + // Skip if PowerShell is not found - it might not be in PATH + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testapp.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PathSeparatorIsSemicolon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testexe.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Add temp directory to PATH using semicolon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new ProcessStartOptions("testexe"); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + // Rename to remove extension to test that .exe is not added for absolute paths + string noExtFile = Path.ChangeExtension(tempFile, null); + File.Move(tempFile, noExtFile); + tempFile = noExtFile; + + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs new file mode 100644 index 00000000000000..cf80a3cd7181ce --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -0,0 +1,224 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Runtime.InteropServices; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests : ProcessTestBase + { + [Fact] + public void TestConstructor_NullFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(null)); + } + + [Fact] + public void TestConstructor_EmptyFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(string.Empty)); + } + + [Fact] + public void TestConstructor_NonExistentFile_Throws() + { + string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); + Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestConstructor_ResolvesCmdOnWindows() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + [Fact] + public void TestConstructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.Contains("sh", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [Fact] + public void TestConstructor_WithAbsolutePath() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + File.Delete(tempFile); + } + } + + [Fact] + public void TestArguments_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList args = options.Arguments; + Assert.NotNull(args); + Assert.Empty(args); + } + + [Fact] + public void TestArguments_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.Arguments.Add("arg1"); + options.Arguments.Add("arg2"); + Assert.Equal(2, options.Arguments.Count); + Assert.Equal("arg1", options.Arguments[0]); + Assert.Equal("arg2", options.Arguments[1]); + + options.Arguments = new List { "newArg" }; + Assert.Single(options.Arguments); + Assert.Equal("newArg", options.Arguments[0]); + } + + [Fact] + public void TestEnvironment_LazyInitialization() + { + if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) + { + // Whole list of environment variables can no longer be accessed on non-OSX apple platforms + return; + } + + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + Assert.NotNull(env); + Assert.NotEmpty(env); + } + + [Fact] + public void TestEnvironment_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + int originalCount = env.Count; + env["TestKey1"] = "TestValue1"; + env["TestKey2"] = "TestValue2"; + Assert.Equal(originalCount + 2, env.Count); + Assert.Equal("TestValue1", env["TestKey1"]); + Assert.Equal("TestValue2", env["TestKey2"]); + + env.Remove("TestKey1"); + Assert.Equal(originalCount + 1, env.Count); + Assert.False(env.ContainsKey("TestKey1")); + } + + [Fact] + public void TestEnvironment_CaseInsensitivityOnWindows() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + env["TestKey"] = "TestValue"; + + if (OperatingSystem.IsWindows()) + { + Assert.True(env.ContainsKey("testkey")); + Assert.Equal("TestValue", env["TESTKEY"]); + } + else + { + Assert.False(env.ContainsKey("testkey")); + } + } + + [Fact] + public void TestInheritedHandles_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList handles = options.InheritedHandles; + Assert.NotNull(handles); + Assert.Empty(handles); + } + + [Fact] + public void TestInheritedHandles_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + List newHandles = new List(); + options.InheritedHandles = newHandles; + Assert.Same(newHandles, options.InheritedHandles); + } + + [Fact] + public void TestWorkingDirectory_DefaultIsNull() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.Null(options.WorkingDirectory); + } + + [Fact] + public void TestWorkingDirectory_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + string tempDir = Path.GetTempPath(); + options.WorkingDirectory = tempDir; + Assert.Equal(tempDir, options.WorkingDirectory); + } + + [Fact] + public void TestKillOnParentExit_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.KillOnParentExit); + } + + [Fact] + public void TestKillOnParentExit_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.KillOnParentExit = true; + Assert.True(options.KillOnParentExit); + } + + [Fact] + public void TestCreateNewProcessGroup_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.CreateNewProcessGroup); + } + + [Fact] + public void TestCreateNewProcessGroup_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.CreateNewProcessGroup = true; + Assert.True(options.CreateNewProcessGroup); + } + + private string GetCurrentProcessName() + { + // Get a valid executable path for testing + if (OperatingSystem.IsWindows()) + { + return Path.Combine(Environment.SystemDirectory, "cmd.exe"); + } + else + { + return "/bin/sh"; + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index 93cb8d34b091e1..63523ef48c86fb 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -28,6 +28,7 @@ + @@ -40,6 +41,7 @@ + + From 0c9d3e24d4f6e8107da0ef0c4af7c1703c16b40c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:07:48 +0000 Subject: [PATCH 03/28] Fix build errors and test failures in ProcessStartOptions Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 4 ++-- .../System/Diagnostics/ProcessStartOptions.cs | 17 +++++++++++++---- .../tests/ProcessStartOptionsTests.cs | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index a674056838985d..78eee4897b0b89 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -49,6 +49,8 @@ Link="Common\System\Text\ValueStringBuilder.cs" /> + @@ -241,8 +243,6 @@ - InheritedHandles /// /// /// - /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) /// on Windows and provides process group isolation on all platforms. /// /// - /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's /// process group, which can be useful for background processes that should continue running independently. /// /// @@ -154,13 +155,17 @@ internal static string ResolvePath(string filename) #endif // Then check the executable's directory - string? path = Environment.ProcessPath; + string? path = System.Environment.ProcessPath; if (path != null) { try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -170,7 +175,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -221,7 +230,7 @@ internal static string ResolvePath(string filename) /// Thrown when the program cannot be found in PATH. private static string FindProgramInPath(string program) { - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index cf80a3cd7181ce..da8c649e8037a7 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -15,7 +15,7 @@ public class ProcessStartOptionsTests : ProcessTestBase [Fact] public void TestConstructor_NullFileName_Throws() { - Assert.Throws(() => new ProcessStartOptions(null)); + Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] From 794a8a206dffbbbf4bb406b95b3f328798616647 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:09:54 +0000 Subject: [PATCH 04/28] Improve code comments based on review feedback Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index f96cf7d9d8abf6..ea6e9799952ab9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -187,7 +187,7 @@ internal static string ResolvePath(string filename) #if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) - // Check the 32-bit Windows system directory (It can't change over app lifetime) + // Check the system directory (e.g., System32) path = GetSystemDirectory(); if (path != null) { @@ -202,7 +202,7 @@ internal static string ResolvePath(string filename) path = GetWindowsDirectory(); if (path != null) { - // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + // Check the legacy System subdirectory of Windows directory (for compatibility) string systemPath = Path.Combine(path, "System", filename); if (File.Exists(systemPath)) { @@ -282,7 +282,7 @@ private static bool IsExecutable(string fullPath) UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - // Avoid checking user/group when permission. + // Avoid checking user/group when all execute permissions are set. if (permissions == AllExecute) { return true; From a1b34263e30cf4ce70534bf2e112972b0f0cf880 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:11:44 +0000 Subject: [PATCH 05/28] Fix exception documentation and add caching comment Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index ea6e9799952ab9..16a1cf66178a00 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -118,7 +118,8 @@ public IList InheritedHandles /// Initializes a new instance of the class. /// /// The application to start. - /// Thrown when is null or empty. + /// Thrown when is null. + /// Thrown when is empty. /// Thrown when cannot be resolved to an existing file. public ProcessStartOptions(string fileName) { @@ -344,6 +345,7 @@ private static bool IsExecutable(string fullPath) private static string? GetWindowsDirectory() { + // Don't cache Windows directory as it's user-specific and can change during app lifetime Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) From 10ff95ae2a5112f28a0c43589abb4dd0b95f7c41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 16:09:04 +0000 Subject: [PATCH 06/28] Address PR feedback: use Process.ResolvePath, fix tests, add null checks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 6 +- .../System/Diagnostics/ProcessStartOptions.cs | 123 +++++------------- .../tests/ProcessStartOptionsTests.Unix.cs | 19 +-- .../tests/ProcessStartOptionsTests.cs | 15 +-- 4 files changed, 43 insertions(+), 120 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 2dde9eb778cdc9..b2fe96562dfcd9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -685,7 +685,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// Resolves a path to the filename passed to ProcessStartInfo. /// The filename. /// The resolved path. It can return null in case of URLs. - private static string? ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // Follow the same resolution that Windows uses with CreateProcess: // 1. First try the exact path provided @@ -734,7 +734,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// /// /// - private static string? FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); @@ -754,7 +754,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) return null; } - private static bool IsExecutable(string fullPath) + internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 16a1cf66178a00..b70b042bc96ca5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -134,6 +142,21 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { +#if WINDOWS + return ResolvePathWindows(filename); +#else + string? resolved = Process.ResolvePath(filename); + if (resolved == null) + { + throw new FileNotFoundException("Could not resolve the file.", filename); + } + return resolved; +#endif + } + +#if WINDOWS + private static string ResolvePathWindows(string filename) + { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -142,7 +165,6 @@ internal static string ResolvePath(string filename) return filename; } -#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -153,7 +175,6 @@ internal static string ResolvePath(string filename) { filename += ".exe"; } -#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -162,11 +183,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,16 +193,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } -#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -217,10 +229,9 @@ internal static string ResolvePath(string filename) return path; } } -#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); + return FindProgramInPathWindows(filename); } /// @@ -229,22 +240,17 @@ internal static string ResolvePath(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + private static string FindProgramInPathWindows(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif - var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) + if (File.Exists(path)) { return path; } @@ -253,77 +259,6 @@ private static string FindProgramInPath(string program) throw new FileNotFoundException("Could not resolve the file.", program); } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return IsExecutable(path); -#endif - } - -#if !WINDOWS - private static bool IsExecutable(string fullPath) - { - Interop.Sys.FileStatus fileinfo; - - if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) - { - return false; - } - - // Check if the path is a directory. - if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) - { - return false; - } - - const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; - - UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - - // Avoid checking user/group when all execute permissions are set. - if (permissions == AllExecute) - { - return true; - } - else if (permissions == 0) - { - return false; - } - - uint euid = Interop.Sys.GetEUid(); - - if (euid == 0) - { - return true; // We're root. - } - - if (euid == fileinfo.Uid) - { - // We own the file. - return (permissions & UnixFileMode.UserExecute) != 0; - } - - bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; - bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; - - // Avoid group check when group and other have same permissions. - if (groupCanExecute == otherCanExecute) - { - return groupCanExecute; - } - - if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) - { - return groupCanExecute; - } - else - { - return otherCanExecute; - } - } #endif #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 2cb8c3cc5f5488..79ba8ced885c37 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -15,7 +15,7 @@ public void TestResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new ProcessStartOptions("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("sh", options.FileName); + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -33,27 +33,20 @@ public void TestResolvePath_UsesCurrentDirectory() string fileName = "testscript.sh"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); // Make it executable File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index da8c649e8037a7..4bd075a8227948 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -31,16 +31,11 @@ public void TestConstructor_NonExistentFile_Throws() Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestConstructor_ResolvesCmdOnWindows() { - if (!OperatingSystem.IsWindows()) - { - return; - } - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.EndsWith("cmd.exe", options.FileName); Assert.True(File.Exists(options.FileName)); } @@ -69,7 +64,7 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_LazyInitialization() + public void TestArguments_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList args = options.Arguments; @@ -145,7 +140,7 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_LazyInitialization() + public void TestInheritedHandles_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList handles = options.InheritedHandles; @@ -217,7 +212,7 @@ private string GetCurrentProcessName() } else { - return "/bin/sh"; + return Environment.ProcessPath ?? "/bin/sh"; } } } From fc0fd30aad754d6c470d2bc8565375eedcecd9ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 17:16:30 +0000 Subject: [PATCH 07/28] Refactor: ProcessStartOptions.ResolvePath as main impl, Process calls it Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 51 +-------------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++++++--------- 2 files changed, 37 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index b2fe96562dfcd9..3d1ab17a9abb1d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -446,7 +446,7 @@ private bool StartCore(ProcessStartInfo startInfo) } else { - filename = ResolvePath(startInfo.FileName); + filename = ProcessStartOptions.ResolvePath(startInfo.FileName); argv = ParseArgv(startInfo); if (Directory.Exists(filename)) { @@ -682,59 +682,12 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// Resolves a path to the filename passed to ProcessStartInfo. - /// The filename. - /// The resolved path. It can return null in case of URLs. - internal static string? ResolvePath(string filename) - { - // Follow the same resolution that Windows uses with CreateProcess: - // 1. First try the exact path provided - // 2. Then try the file relative to the executable directory - // 3. Then try the file relative to the current directory - // 4. then try the file in each of the directories specified in PATH - // Windows does additional Windows-specific steps between 3 and 4, - // and we ignore those here. - - // If the filename is a complete path, use it, regardless of whether it exists. - if (Path.IsPathRooted(filename)) - { - // In this case, it doesn't matter whether the file exists or not; - // it's what the caller asked for, so it's what they'll get - return filename; - } - - // Then check the executable's directory - string? path = Environment.ProcessPath; - if (path != null) - { - try - { - path = Path.Combine(Path.GetDirectoryName(path)!, filename); - if (File.Exists(path)) - { - return path; - } - } - catch (ArgumentException) { } // ignore any errors in data that may come from the exe path - } - - // Then check the current directory - path = Path.Combine(Directory.GetCurrentDirectory(), filename); - if (File.Exists(path)) - { - return path; - } - - // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); - } - /// /// Gets the path to the program /// /// /// - internal static string? FindProgramInPath(string program) + private static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index b70b042bc96ca5..1cdac7cbbbb042 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,11 +31,7 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _arguments = value; - } + set => _arguments = value; } /// @@ -95,11 +91,7 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _inheritedHandles = value; - } + set => _inheritedHandles = value; } /// @@ -142,21 +134,6 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { -#if WINDOWS - return ResolvePathWindows(filename); -#else - string? resolved = Process.ResolvePath(filename); - if (resolved == null) - { - throw new FileNotFoundException("Could not resolve the file.", filename); - } - return resolved; -#endif - } - -#if WINDOWS - private static string ResolvePathWindows(string filename) - { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -165,6 +142,7 @@ private static string ResolvePathWindows(string filename) return filename; } +#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -175,6 +153,7 @@ private static string ResolvePathWindows(string filename) { filename += ".exe"; } +#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -183,7 +162,11 @@ private static string ResolvePathWindows(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -193,11 +176,16 @@ private static string ResolvePathWindows(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } +#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -229,9 +217,10 @@ private static string ResolvePathWindows(string filename) return path; } } +#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPathWindows(filename); + return FindProgramInPath(filename); } /// @@ -240,17 +229,22 @@ private static string ResolvePathWindows(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPathWindows(string program) + private static string FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { - var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (File.Exists(path)) + if (IsExecutableFile(path)) { return path; } @@ -259,6 +253,18 @@ private static string FindProgramInPathWindows(string program) throw new FileNotFoundException("Could not resolve the file.", program); } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return Process.IsExecutable(path); +#endif + } + +#if !WINDOWS + // Unix executable checking is in Process.IsExecutable #endif #if WINDOWS From f687eb8f1620ea24a22d2b902ffc47c56516ff78 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Feb 2026 19:19:59 +0100 Subject: [PATCH 08/28] address my own feedback --- .../src/System/Diagnostics/Process.Linux.cs | 2 +- .../src/System/Diagnostics/Process.SunOS.cs | 2 +- .../src/System/Diagnostics/Process.Unix.cs | 27 +------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++------------- 4 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index 5cf8c5c76a993d..a47c8885afa9e4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -108,7 +108,7 @@ private static DateTime BootTime ReadOnlySpan allowedProgramsToRun = ["xdg-open", "gnome-open", "kfmclient"]; foreach (var program in allowedProgramsToRun) { - string? pathToProgram = FindProgramInPath(program); + string? pathToProgram = ProcessStartOptions.FindProgramInPath(program); if (!string.IsNullOrEmpty(pathToProgram)) { return pathToProgram; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs index f40076a3882aca..3a3bff81abd0a4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs @@ -38,7 +38,7 @@ internal DateTime StartTimeCore /// Gets execution path private static string? GetPathToOpenFile() { - return FindProgramInPath("xdg-open"); + return ProcessStartOptions.FindProgramInPath("xdg-open"); } /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 3d1ab17a9abb1d..fc46f14f918e7b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -663,7 +663,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) // find filename on PATH else { - resolvedFilename = FindProgramInPath(filename); + resolvedFilename = ProcessStartOptions.FindProgramInPath(filename); } } @@ -682,31 +682,6 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// - /// Gets the path to the program - /// - /// - /// - private static string? FindProgramInPath(string program) - { - string path; - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); - if (pathEnvVar != null) - { - var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); - while (pathParser.MoveNext()) - { - string subPath = pathParser.ExtractCurrent(); - path = Path.Combine(subPath, program); - if (IsExecutable(path)) - { - return path; - } - } - } - return null; - } - internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1cdac7cbbbb042..3ba446f13dcf7a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -125,14 +133,10 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName); + _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; } - /// Resolves a path to the filename. - /// The filename. - /// The resolved path. - /// Thrown when cannot be resolved to an existing file. - internal static string ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) @@ -162,11 +166,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,11 +176,7 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -219,54 +215,34 @@ internal static string ResolvePath(string filename) } #endif - // Then check each directory listed in the PATH environment variables return FindProgramInPath(filename); } - /// - /// Gets the path to the program by searching in PATH environment variable. - /// - /// The program name. - /// The full path to the program. - /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif + char pathSeparator = OperatingSystem.IsWindows() ? ';' : ':'; var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) +#if WINDOWS + if (File.Exists(path)) +#else + if (Process.IsExecutable(path)) +#endif { return path; } } } - throw new FileNotFoundException("Could not resolve the file.", program); - } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return Process.IsExecutable(path); -#endif + return null; } -#if !WINDOWS - // Unix executable checking is in Process.IsExecutable -#endif - #if WINDOWS private static string? s_cachedSystemDirectory; From 5459ec9dc9124fc37bf52c4588a367864e0a67ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 18:48:29 +0000 Subject: [PATCH 09/28] Simplify Windows tests: remove ConditionalFact, fix nested try-finally Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 16 +++---- .../tests/ProcessStartOptionsTests.Windows.cs | 42 ++++--------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 79ba8ced885c37..7114346ef3b34f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -63,6 +63,7 @@ public void TestResolvePath_PathSeparatorIsColon() string fileName = "testscript"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); @@ -70,20 +71,13 @@ public void TestResolvePath_PathSeparatorIsColon() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); // Add temp directory to PATH using colon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 287cb573fedf16..1a00288aea5caa 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public class ProcessStartOptionsTests_Windows { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -18,7 +18,7 @@ public void TestResolvePath_AddsExeExtension() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." @@ -26,7 +26,7 @@ public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() Assert.Throws(() => new ProcessStartOptions("notepad.")); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PreservesComExtension() { // The .com extension should be preserved @@ -72,37 +72,11 @@ public void TestResolvePath_FindsInSystemDirectory() [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestResolvePath_FindsInWindowsDirectory() { - // Some utilities are in Windows directory - // We'll test with a file that's commonly in Windows directory - // Note: This might not exist on all systems, so we make it conditional - try - { - ProcessStartOptions options = new ProcessStartOptions("notepad"); - Assert.True(File.Exists(options.FileName)); - } - catch (FileNotFoundException) - { - // Skip if notepad is not found - it's not critical for this test - } - } - - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInPath() - { - // powershell.exe should be findable in PATH on most Windows systems - try - { - ProcessStartOptions options = new ProcessStartOptions("powershell"); - Assert.True(File.Exists(options.FileName)); - Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); - } - catch (FileNotFoundException) - { - // Skip if PowerShell is not found - it might not be in PATH - } + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); @@ -135,7 +109,7 @@ public void TestResolvePath_UsesCurrentDirectory() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file @@ -174,7 +148,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); From c3517ee471e104bd4b8593e2369913a2e142c098 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 07:51:04 +0000 Subject: [PATCH 10/28] Refactor tests: partial classes, remove Test prefix, use new() syntax, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 106 ++++++------------ .../tests/ProcessStartOptionsTests.Windows.cs | 92 +++++++-------- .../tests/ProcessStartOptionsTests.cs | 104 ++++++----------- 3 files changed, 106 insertions(+), 196 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 7114346ef3b34f..ee8db2da0474f9 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -7,27 +7,35 @@ namespace System.Diagnostics.Tests { [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] - public class ProcessStartOptionsTests_Unix + public partial class ProcessStartOptionsTests { [Fact] - public void TestResolvePath_FindsInPath() + public void Constructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Equal("/bin/sh", options.FileName); + } + + [Fact] + public void ResolvePath_FindsInPath() { // sh should be findable in PATH on all Unix systems - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); Assert.EndsWith("sh", options.FileName); } [Fact] - public void TestResolvePath_DoesNotAddExeExtension() + public void ResolvePath_DoesNotAddExeExtension() { // On Unix, no .exe extension should be added - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testscript.sh"; @@ -41,8 +49,8 @@ public void TestResolvePath_UsesCurrentDirectory() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -55,7 +63,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsColon() + public void ResolvePath_PathSeparatorIsColon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -72,8 +80,8 @@ public void TestResolvePath_PathSeparatorIsColon() // Add temp directory to PATH using colon separator Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -90,48 +98,12 @@ public void TestResolvePath_PathSeparatorIsColon() } [Fact] - public void TestResolvePath_ChecksExecutablePermissions() - { - // Create a file without execute permissions - string tempDir = Path.GetTempPath(); - string fileName = "nonexecutable.sh"; - string fullPath = Path.Combine(tempDir, fileName); - - try - { - File.WriteAllText(fullPath, "#!/bin/sh\necho test"); - // Explicitly make it non-executable - File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - // Should throw because file is not executable - Assert.Throws(() => new ProcessStartOptions(fileName)); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } - } - finally - { - if (File.Exists(fullPath)) - { - File.Delete(fullPath); - } - } - } - - [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -143,43 +115,35 @@ public void TestResolvePath_AbsolutePathIsNotModified() } } - [Fact] - public void TestResolvePath_FindsCommonUtilities() + [Theory] + [InlineData("ls")] + [InlineData("cat")] + [InlineData("echo")] + [InlineData("sh")] + public void ResolvePath_FindsCommonUtilities(string utilName) { - // Test common Unix utilities - string[] commonUtils = { "ls", "cat", "echo", "sh" }; - - foreach (string util in commonUtils) - { - ProcessStartOptions options = new ProcessStartOptions(util); - Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); - Assert.Contains(util, options.FileName); - } + ProcessStartOptions options = new(utilName); + Assert.True(File.Exists(options.FileName), $"{utilName} should be found and exist"); + Assert.Contains(utilName, options.FileName); } [Fact] - public void TestResolvePath_RejectsDirectories() + public void ResolvePath_RejectsDirectories() { // Create a directory with executable permissions string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); Directory.CreateDirectory(tempDir); + string oldDir = Directory.GetCurrentDirectory(); try { // Try to use the directory name as a command - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(Path.GetTempPath()); - Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); } finally { + Directory.SetCurrentDirectory(oldDir); if (Directory.Exists(tempDir)) { Directory.Delete(tempDir); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 1a00288aea5caa..fd75560d0677c0 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -7,52 +7,52 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests_Windows + public partial class ProcessStartOptionsTests { - [Fact] - public void TestResolvePath_AddsExeExtension() + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void Constructor_ResolvesCmdOnWindows() + { + ProcessStartOptions options = new("cmd"); + Assert.EndsWith("cmd.exe", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new ProcessStartOptions("notepad.")); + Assert.Throws(() => new("notepad.")); } [Fact] - public void TestResolvePath_PreservesComExtension() + public void ResolvePath_PreservesComExtension() { // The .com extension should be preserved string fileName = "test.com"; string tempDir = Path.GetTempPath(); string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -61,47 +61,39 @@ public void TestResolvePath_PreservesComExtension() } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInSystemDirectory() + public void ResolvePath_FindsInSystemDirectory() { // cmd.exe should be found in system directory - ProcessStartOptions options = new ProcessStartOptions("cmd"); + ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInWindowsDirectory() + public void ResolvePath_FindsInWindowsDirectory() { - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testapp.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -110,7 +102,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsSemicolon() + public void ResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -118,25 +110,17 @@ public void TestResolvePath_PathSeparatorIsSemicolon() string fileName = "testexe.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "test"); - - // Add temp directory to PATH using semicolon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); - ProcessStartOptions options = new ProcessStartOptions("testexe"); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new("testexe"); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -149,7 +133,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try @@ -159,7 +143,7 @@ public void TestResolvePath_AbsolutePathIsNotModified() File.Move(tempFile, noExtFile); tempFile = noExtFile; - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index 4bd075a8227948..75b361e3337877 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -10,51 +10,34 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests : ProcessTestBase + public partial class ProcessStartOptionsTests { [Fact] - public void TestConstructor_NullFileName_Throws() + public void Constructor_NullFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] - public void TestConstructor_EmptyFileName_Throws() + public void Constructor_EmptyFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(string.Empty)); } [Fact] - public void TestConstructor_NonExistentFile_Throws() + public void Constructor_NonExistentFile_Throws() { string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestConstructor_ResolvesCmdOnWindows() - { - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.EndsWith("cmd.exe", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] [Fact] - public void TestConstructor_ResolvesShOnUnix() - { - ProcessStartOptions options = new ProcessStartOptions("sh"); - Assert.Contains("sh", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [Fact] - public void TestConstructor_WithAbsolutePath() + public void Constructor_WithAbsolutePath() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -64,18 +47,18 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_DefaultIsEmpty() + public void Arguments_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList args = options.Arguments; Assert.NotNull(args); Assert.Empty(args); } [Fact] - public void TestArguments_CanAddAndModify() + public void Arguments_CanAddAndModify() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.Arguments.Add("arg1"); options.Arguments.Add("arg2"); Assert.Equal(2, options.Arguments.Count); @@ -88,24 +71,9 @@ public void TestArguments_CanAddAndModify() } [Fact] - public void TestEnvironment_LazyInitialization() + public void Environment_CanAddAndModify() { - if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) - { - // Whole list of environment variables can no longer be accessed on non-OSX apple platforms - return; - } - - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); - IDictionary env = options.Environment; - Assert.NotNull(env); - Assert.NotEmpty(env); - } - - [Fact] - public void TestEnvironment_CanAddAndModify() - { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; int originalCount = env.Count; @@ -121,9 +89,9 @@ public void TestEnvironment_CanAddAndModify() } [Fact] - public void TestEnvironment_CaseInsensitivityOnWindows() + public void Environment_CaseSensitivityIsPlatformSpecific() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; env["TestKey"] = "TestValue"; @@ -140,80 +108,74 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_DefaultIsEmpty() + public void InheritedHandles_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList handles = options.InheritedHandles; Assert.NotNull(handles); Assert.Empty(handles); } [Fact] - public void TestInheritedHandles_CanSet() + public void InheritedHandles_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); List newHandles = new List(); options.InheritedHandles = newHandles; Assert.Same(newHandles, options.InheritedHandles); } [Fact] - public void TestWorkingDirectory_DefaultIsNull() + public void WorkingDirectory_DefaultIsNull() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.Null(options.WorkingDirectory); } [Fact] - public void TestWorkingDirectory_CanSet() + public void WorkingDirectory_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); string tempDir = Path.GetTempPath(); options.WorkingDirectory = tempDir; Assert.Equal(tempDir, options.WorkingDirectory); } [Fact] - public void TestKillOnParentExit_DefaultIsFalse() + public void KillOnParentExit_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.KillOnParentExit); } [Fact] - public void TestKillOnParentExit_CanSet() + public void KillOnParentExit_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.KillOnParentExit = true; Assert.True(options.KillOnParentExit); } [Fact] - public void TestCreateNewProcessGroup_DefaultIsFalse() + public void CreateNewProcessGroup_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.CreateNewProcessGroup); } [Fact] - public void TestCreateNewProcessGroup_CanSet() + public void CreateNewProcessGroup_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.CreateNewProcessGroup = true; Assert.True(options.CreateNewProcessGroup); } private string GetCurrentProcessName() { - // Get a valid executable path for testing - if (OperatingSystem.IsWindows()) - { - return Path.Combine(Environment.SystemDirectory, "cmd.exe"); - } - else - { - return Environment.ProcessPath ?? "/bin/sh"; - } + return Environment.ProcessPath ?? (OperatingSystem.IsWindows() + ? Path.Combine(Environment.SystemDirectory, "cmd.exe") + : "/bin/sh"); } } } From bbea91e0a3a5eaaafe1e7b24460f40d9e3a5dd3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 08:03:00 +0000 Subject: [PATCH 11/28] Fix test conditions: remove redundant IsWindows check, use GetFullPath for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 2 +- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index ee8db2da0474f9..820411c954e747 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal("/bin/sh", options.FileName); + Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index fd75560d0677c0..eb9fa34225029f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public partial class ProcessStartOptionsTests { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void Constructor_ResolvesCmdOnWindows() { ProcessStartOptions options = new("cmd"); From d9d381fef63440991a40ecfab8197cc9cd4e6b77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:53:18 +0000 Subject: [PATCH 12/28] Fix test failures: use ResolveTarget helper for symlinks, fix lambda syntax Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 10 ++++++++-- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 820411c954e747..325d580d9a6673 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); } [Fact] @@ -23,7 +23,13 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.EndsWith("sh", options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + } + + private static string ResolveTarget(string path) + { + FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); + return target?.FullName ?? path; } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index eb9fa34225029f..06371aae1bcd88 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -31,7 +31,7 @@ public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new("notepad.")); + Assert.Throws(() => new ProcessStartOptions("notepad.")); } [Fact] From b4d6e6495d7c730be47f45b1a724d775894b0f45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:53:03 +0000 Subject: [PATCH 13/28] Fix Unix test failures: use EndsWith instead of exact path comparison Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 325d580d9a6673..cf357680eb9da6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,8 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -23,7 +24,8 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } private static string ResolveTarget(string path) From 98d420113a6a647a22812a15db5e47770c17c621 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:34:26 +0000 Subject: [PATCH 14/28] Address code review: add file check, fix .exe logic, update comments, remove dead code Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 3 +++ .../src/System/Diagnostics/ProcessStartOptions.cs | 14 +++++++++++--- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ------ .../tests/ProcessStartOptionsTests.Windows.cs | 7 ++++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 3cc0d1e1d09e61..67d05840e9796b 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -336,4 +336,7 @@ Invalid performance counter data with type '{0}'. + + Could not resolve the file. + \ No newline at end of file diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 3ba446f13dcf7a..1d65eb12dc2076 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -133,7 +133,12 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; + string? resolved = ResolvePath(fileName); + if (resolved == null || !File.Exists(resolved)) + { + throw new FileNotFoundException(SR.FileNotFoundResolvePath, fileName); + } + _fileName = resolved; } internal static string? ResolvePath(string filename) @@ -153,7 +158,9 @@ public ProcessStartOptions(string fileName) // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." // HasExtension returns false for trailing dot, so we need to check that separately - if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + if (filename[filename.Length - 1] != '.' + && string.IsNullOrEmpty(Path.GetDirectoryName(filename)) + && !Path.HasExtension(filename)) { filename += ".exe"; } @@ -262,7 +269,8 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // Don't cache Windows directory as it's user-specific and can change during app lifetime + // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, + // and this method is not expected to be called frequently enough to warrant additional caching. Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index cf357680eb9da6..22a2c535077f6a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -28,12 +28,6 @@ public void ResolvePath_FindsInPath() Assert.EndsWith("sh", options.FileName); } - private static string ResolveTarget(string path) - { - FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); - return target?.FullName ?? path; - } - [Fact] public void ResolvePath_DoesNotAddExeExtension() { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 06371aae1bcd88..1600c71e5ebfb4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -17,7 +17,7 @@ public void Constructor_ResolvesCmdOnWindows() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -66,10 +66,11 @@ public void ResolvePath_FindsInSystemDirectory() // cmd.exe should be found in system directory ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + string systemDirectory = Environment.SystemDirectory; + Assert.StartsWith(systemDirectory, options.FileName, StringComparison.OrdinalIgnoreCase); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_FindsInWindowsDirectory() { ProcessStartOptions options = new("notepad"); From 5b024d4a24cdb6d65c2fb48f15258b0f6ca79657 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:54:27 +0000 Subject: [PATCH 15/28] Cache Windows directory value for performance Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/ProcessStartOptions.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1d65eb12dc2076..90d5acbf378f63 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -252,6 +252,7 @@ public ProcessStartOptions(string fileName) #if WINDOWS private static string? s_cachedSystemDirectory; + private static string? s_cachedWindowsDirectory; private static string? GetSystemDirectory() { @@ -269,15 +270,16 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, - // and this method is not expected to be called frequently enough to warrant additional caching. - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) + if (s_cachedWindowsDirectory == null) { - return new string(buffer.Slice(0, (int)length)); + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedWindowsDirectory = new string(buffer.Slice(0, (int)length)); + } } - return null; + return s_cachedWindowsDirectory; } #endif } From add4af64bb0ef4b754ca90ac8a8b1dfab3210f61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 19:54:45 +0000 Subject: [PATCH 16/28] Remove DictionaryWrapper and fix Unix path test for macOS symlinks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- .../tests/ProcessStartOptionsTests.Unix.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 90d5acbf378f63..fb12746af8399a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -17,7 +17,7 @@ public sealed class ProcessStartOptions { private readonly string _fileName; private IList? _arguments; - private DictionaryWrapper? _environment; + private Dictionary? _environment; private IList? _inheritedHandles; /// @@ -52,9 +52,9 @@ public IList Arguments { IDictionary envVars = System.Environment.GetEnvironmentVariables(); - _environment = new DictionaryWrapper(new Dictionary( + _environment = new Dictionary( envVars.Count, - OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. IDictionaryEnumerator e = envVars.GetEnumerator(); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 22a2c535077f6a..9aae9937f37f7d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,7 +52,8 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - Assert.Equal(Path.GetFullPath(fullPath), options.FileName); + // Use Path.GetFullPath on both sides to handle symlinks (e.g., /tmp -> /private/tmp on macOS) + Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); } finally { From b3939ad14ad133fa41aaf445ac839d814d12230b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 06:23:42 +0000 Subject: [PATCH 17/28] Use Environment.SystemDirectory and fix Unix path test for macOS Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 11 +---------- .../tests/ProcessStartOptionsTests.Unix.cs | 5 +++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index fb12746af8399a..30eb189f480f1d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -256,16 +256,7 @@ public ProcessStartOptions(string fileName) private static string? GetSystemDirectory() { - if (s_cachedSystemDirectory == null) - { - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetSystemDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) - { - s_cachedSystemDirectory = new string(buffer.Slice(0, (int)length)); - } - } - return s_cachedSystemDirectory; + return s_cachedSystemDirectory ??= System.Environment.SystemDirectory; } private static string? GetWindowsDirectory() diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 9aae9937f37f7d..0c6060135af691 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,8 +52,9 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // Use Path.GetFullPath on both sides to handle symlinks (e.g., /tmp -> /private/tmp on macOS) - Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); + // options.FileName is already the resolved full path + // Use Path.GetFullPath to canonicalize fullPath (handles /tmp -> /private/tmp on macOS) + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { From 63e7fc0fbb448ee1c526f10cad9b0d87d6aa4687 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 06:37:48 +0000 Subject: [PATCH 18/28] Inline GetSystemDirectory, fix Unix path test, use EndsWith for util names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 2 +- .../tests/ProcessStartOptionsTests.Unix.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 30eb189f480f1d..4d9045c30c1064 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -192,7 +192,7 @@ public ProcessStartOptions(string fileName) // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) - path = GetSystemDirectory(); + path = s_cachedSystemDirectory ??= System.Environment.SystemDirectory; if (path != null) { path = Path.Combine(path, filename); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 0c6060135af691..8ab1fd04bafe76 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,9 +52,9 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // options.FileName is already the resolved full path - // Use Path.GetFullPath to canonicalize fullPath (handles /tmp -> /private/tmp on macOS) - Assert.Equal(Path.GetFullPath(fullPath), options.FileName); + // Path.GetFullPath resolves symlinks in the path (e.g., /tmp -> /private/tmp on macOS) + // options.FileName is already fully resolved by ProcessStartOptions + Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); } finally { @@ -128,7 +128,7 @@ public void ResolvePath_FindsCommonUtilities(string utilName) { ProcessStartOptions options = new(utilName); Assert.True(File.Exists(options.FileName), $"{utilName} should be found and exist"); - Assert.Contains(utilName, options.FileName); + Assert.EndsWith(utilName, options.FileName); } [Fact] From 9ceaad179af3f483fa4b29b83edc55c6a2412118 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Feb 2026 08:26:24 +0100 Subject: [PATCH 19/28] Update src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj --- .../src/System.Diagnostics.Process.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 78eee4897b0b89..53e635042d4e21 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -140,8 +140,6 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> - Date: Fri, 13 Feb 2026 12:33:17 +0100 Subject: [PATCH 20/28] fix the ResolvePath_UsesCurrentDirectory test --- .../tests/ProcessStartOptionsTests.Unix.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 8ab1fd04bafe76..71b6ed911a9a7f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,9 +52,10 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - // Path.GetFullPath resolves symlinks in the path (e.g., /tmp -> /private/tmp on macOS) - // options.FileName is already fully resolved by ProcessStartOptions - Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); + + Assert.True(File.Exists(options.FileName)); + // on macOS, we need to handle /tmp/testscript.sh -> /private/tmp/testscript.sh + Assert.EndsWith(fullPath, options.FileName); } finally { From bf02bd96d32c8eaa71505b3fdf17fd14c34385ac Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 15 Feb 2026 16:59:49 +0100 Subject: [PATCH 21/28] address code review feedback: - add comments - cache Environment.SystemDirectory - handle fully qualified paths on Windows --- .../System/Diagnostics/ProcessStartOptions.cs | 105 ++++++++++++++++-- .../src/System/Environment.Windows.cs | 31 +++--- 2 files changed, 110 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 4d9045c30c1064..f86c9f645cab79 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -21,8 +21,22 @@ public sealed class ProcessStartOptions private IList? _inheritedHandles; /// - /// Gets the application to start. + /// Gets the absolute path of the application to start. /// + /// + /// The absolute path to the executable file. This path is resolved from the fileName parameter + /// passed to the constructor by searching through various directories if needed. + /// + /// + /// + /// The path is "resolved" meaning it has been converted to an absolute path and verified to exist. + /// If a relative path was provided to the constructor, it was located by searching in the executable's + /// directory, current directory, system directories (on Windows), and directories in the PATH environment variable. + /// + /// + /// See for complete details on the resolution process. + /// + /// public string FileName => _fileName; /// @@ -129,10 +143,45 @@ public IList InheritedHandles /// Thrown when is null. /// Thrown when is empty. /// Thrown when cannot be resolved to an existing file. + /// + /// + /// The is resolved to an absolute path before starting the process. + /// + /// + /// When the is a rooted path, it is used as-is without any resolution. + /// + /// + /// On Windows, when is not a rooted path, the system searches + /// for the executable in the following order (mimicking the behavior of the CreateProcess system call): + /// + /// + /// The directory of the executable that started the currently executing process. + /// The current directory. + /// The System directory. + /// The Windows directory. + /// The directories listed in the PATH environment variable. + /// + /// + /// On Unix, when is not a rooted path, the system searches + /// for the executable in the following order (mimicking the Windows search order): + /// + /// + /// The directory of the executable that started the currently executing process. + /// The current directory. + /// The directories listed in the PATH environment variable. + /// + /// + /// On Windows, if the does not have an extension, ".exe" is appended to it before searching. + /// + /// public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); + // The file could be deleted or replaced after this check and before the process is started (TOCTOU). + // In such case, the process creation will fail. + // We resolve the path here to provide unified error handling and to avoid + // starting a process that will fail immediately after creation. string? resolved = ResolvePath(fileName); if (resolved == null || !File.Exists(resolved)) { @@ -141,16 +190,56 @@ public ProcessStartOptions(string fileName) _fileName = resolved; } + // There are two ways to create a process on Windows using CreateProcess sys-call: + // 1. With NULL lpApplicationName and non-NULL lpCommandLine, where the first token of the + // command line is the executable name. In this case, the system will resolve the executable + // name to an actual file on disk using documented search order. + // 2. With non-NULL lpApplicationName, where the system will use the provided application + // name as-is without any resolution, and the command line is passed as-is to the process. + // + // The official documentation mentions: + // "(..) do not pass NULL for lpApplicationName. If you do pass NULL for lpApplicationName, + // use quotation marks around the executable path in lpCommandLine." + // + // We have asked the CreateProcess owners (Windows Team) for feedback. This is what they wrote: + // "Applications should not under any circumstances pass user-controlled input directly to CreateProcess; + // or, if they intend to do so (for passing user-originated parameters), they should be filling out the lpApplicationName parameter." + // + // We could either document that the FileName should never be user-controlled input or resolve it ourselves, + // pass it as lpApplicationName and arguments as lpCommandLine. + // + // On Unix, we were already resolving the executable path before starting the process. + // We were doing it for two reasons: + // 1. To mimic Windows resolution path order (consistency across platforms). + // 2. To avoid forking a process and then failing in exec because the file cannot be found. + // By resolving the path ourselves before forking we can avoid creating a child process that will fail immediately after creation. + // + // Changing the resolution logic could introduce breaking changes. For example: + // "If the executable module is a 16-bit application, lpApplicationName should be NULL, and the + // string pointed to by lpCommandLine should specify the executable module as well as its arguments." + // + // That is why we don't change the resolution logic for existing APIs, but since we are introducing a new API, + // we have the opportunity to do it in a more secure way by always resolving the executable name. + // Moreover, it gives the users the opportunity to: + // - check the resolved executable path before starting the process. + // - cache the resolved path and reuse it for subsequent process creations (perf). + // + // In upcoming .NET 11 previews, we may consider changing the order. internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. - if (Path.IsPathRooted(filename)) + if (Path.IsPathFullyQualified(filename)) { - // In this case, it doesn't matter whether the file exists or not; - // it's what the caller asked for, so it's what they'll get return filename; } + // Handle rooted but not fully qualified paths (e.g., C:foo.exe, \foo.exe on Windows) + // On Unix, this never executes since rooted paths are always fully qualified + if (Path.IsPathRooted(filename)) + { + return Path.GetFullPath(filename); // Resolve to absolute path + } + #if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. @@ -192,7 +281,7 @@ public ProcessStartOptions(string fileName) // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) - path = s_cachedSystemDirectory ??= System.Environment.SystemDirectory; + path = System.Environment.SystemDirectory; if (path != null) { path = Path.Combine(path, filename); @@ -251,14 +340,8 @@ public ProcessStartOptions(string fileName) } #if WINDOWS - private static string? s_cachedSystemDirectory; private static string? s_cachedWindowsDirectory; - private static string? GetSystemDirectory() - { - return s_cachedSystemDirectory ??= System.Environment.SystemDirectory; - } - private static string? GetWindowsDirectory() { if (s_cachedWindowsDirectory == null) diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs index 2f251a11a2970e..80624afcb23a44 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs @@ -155,25 +155,26 @@ private static unsafe OperatingSystem GetOSVersion() new OperatingSystem(PlatformID.Win32NT, version); } - public static string SystemDirectory - { - get - { - // Normally this will be C:\Windows\System32 - var builder = new ValueStringBuilder(stackalloc char[32]); + private static string? s_systemDirectory; - uint length; - while ((length = Interop.Kernel32.GetSystemDirectoryW(ref builder.GetPinnableReference(), (uint)builder.Capacity)) > builder.Capacity) - { - builder.EnsureCapacity((int)length); - } + public static string SystemDirectory => s_systemDirectory ??= GetSystemDirectory(); - if (length == 0) - throw Win32Marshal.GetExceptionForLastWin32Error(); + private static string GetSystemDirectory() + { + // Normally this will be C:\Windows\System32 + var builder = new ValueStringBuilder(stackalloc char[32]); - builder.Length = (int)length; - return builder.ToString(); + uint length; + while ((length = Interop.Kernel32.GetSystemDirectoryW(ref builder.GetPinnableReference(), (uint)builder.Capacity)) > builder.Capacity) + { + builder.EnsureCapacity((int)length); } + + if (length == 0) + throw Win32Marshal.GetExceptionForLastWin32Error(); + + builder.Length = (int)length; + return builder.ToString(); } public static unsafe bool UserInteractive From e778275a596c43e7f88e3bc61a208b02e271520d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 08:32:04 +0000 Subject: [PATCH 22/28] Remove executable directory probing per code review feedback Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/ProcessStartOptions.cs | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index f86c9f645cab79..4c92167f53008d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -155,7 +155,6 @@ public IList InheritedHandles /// for the executable in the following order (mimicking the behavior of the CreateProcess system call): /// /// - /// The directory of the executable that started the currently executing process. /// The current directory. /// The System directory. /// The Windows directory. @@ -163,13 +162,8 @@ public IList InheritedHandles /// /// /// On Unix, when is not a rooted path, the system searches - /// for the executable in the following order (mimicking the Windows search order): + /// for the executable in the current directory and then in the directories listed in the PATH environment variable. /// - /// - /// The directory of the executable that started the currently executing process. - /// The current directory. - /// The directories listed in the PATH environment variable. - /// /// /// On Windows, if the does not have an extension, ".exe" is appended to it before searching. /// @@ -255,23 +249,8 @@ public ProcessStartOptions(string fileName) } #endif - // Then check the executable's directory - string? path = System.Environment.ProcessPath; - if (path != null) - { - try - { - path = Path.Combine(Path.GetDirectoryName(path)!, filename); - if (File.Exists(path)) - { - return path; - } - } - catch (ArgumentException) { } // ignore any errors in data that may come from the exe path - } - - // Then check the current directory - path = Path.Combine(Directory.GetCurrentDirectory(), filename); + // Check the current directory + string path = Path.Combine(Directory.GetCurrentDirectory(), filename); if (File.Exists(path)) { return path; From 168ddf685bedeb861196fc0a2d9ce21dd02a3813 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 10:43:20 +0000 Subject: [PATCH 23/28] Simplify path resolution: remove CWD and Windows dir probing, require explicit ./ Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Kernel32/Interop.GetWindowsDirectoryW.cs | 13 ----- .../src/System.Diagnostics.Process.csproj | 2 - .../System/Diagnostics/ProcessStartOptions.cs | 56 ++----------------- .../tests/ProcessStartOptionsTests.Unix.cs | 22 ++++++-- .../tests/ProcessStartOptionsTests.Windows.cs | 55 ++++++++++++++---- 5 files changed, 63 insertions(+), 85 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs deleted file mode 100644 index 05710a47a019f9..00000000000000 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Kernel32 - { - [LibraryImport(Libraries.Kernel32, SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] - internal static partial uint GetWindowsDirectoryW(ref char lpBuffer, uint uSize); - } -} diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 53e635042d4e21..202839abae807b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -140,8 +140,6 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> - InheritedHandles /// /// /// On Windows, when is not a rooted path, the system searches - /// for the executable in the following order (mimicking the behavior of the CreateProcess system call): + /// for the executable in the following order: /// /// - /// The current directory. /// The System directory. - /// The Windows directory. /// The directories listed in the PATH environment variable. /// /// /// On Unix, when is not a rooted path, the system searches - /// for the executable in the current directory and then in the directories listed in the PATH environment variable. + /// for the executable in the directories listed in the PATH environment variable. /// /// /// On Windows, if the does not have an extension, ".exe" is appended to it before searching. @@ -249,39 +247,11 @@ public ProcessStartOptions(string fileName) } #endif - // Check the current directory - string path = Path.Combine(Directory.GetCurrentDirectory(), filename); - if (File.Exists(path)) - { - return path; - } - #if WINDOWS - // Windows-specific search locations (from CreateProcessW documentation) - - // Check the system directory (e.g., System32) - path = System.Environment.SystemDirectory; - if (path != null) - { - path = Path.Combine(path, filename); - if (File.Exists(path)) - { - return path; - } - } - - // Check the Windows directory - path = GetWindowsDirectory(); + // Windows-specific search location: the system directory (e.g., C:\Windows\System32) + string path = System.Environment.SystemDirectory; if (path != null) { - // Check the legacy System subdirectory of Windows directory (for compatibility) - string systemPath = Path.Combine(path, "System", filename); - if (File.Exists(systemPath)) - { - return systemPath; - } - - // Check the Windows directory itself path = Path.Combine(path, filename); if (File.Exists(path)) { @@ -317,23 +287,5 @@ public ProcessStartOptions(string fileName) return null; } - -#if WINDOWS - private static string? s_cachedWindowsDirectory; - - private static string? GetWindowsDirectory() - { - if (s_cachedWindowsDirectory == null) - { - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) - { - s_cachedWindowsDirectory = new string(buffer.Slice(0, (int)length)); - } - } - return s_cachedWindowsDirectory; - } -#endif } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 71b6ed911a9a7f..a0c5a3b2325262 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -36,8 +36,10 @@ public void ResolvePath_DoesNotAddExeExtension() Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); } - [Fact] - public void ResolvePath_UsesCurrentDirectory() + [Theory] + [InlineData("./testscript.sh", true)] + [InlineData("testscript.sh", false)] + public void ResolvePath_UsesCurrentDirectory(string fileNameFormat, bool shouldSucceed) { string tempDir = Path.GetTempPath(); string fileName = "testscript.sh"; @@ -51,11 +53,19 @@ public void ResolvePath_UsesCurrentDirectory() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new(fileName); - Assert.True(File.Exists(options.FileName)); - // on macOS, we need to handle /tmp/testscript.sh -> /private/tmp/testscript.sh - Assert.EndsWith(fullPath, options.FileName); + if (shouldSucceed) + { + ProcessStartOptions options = new(fileNameFormat); + Assert.True(File.Exists(options.FileName)); + // on macOS, we need to handle /tmp/testscript.sh -> /private/tmp/testscript.sh + Assert.EndsWith(fullPath, options.FileName); + } + else + { + // Without ./ prefix, should not find file in CWD and should throw + Assert.Throws(() => new ProcessStartOptions(fileNameFormat)); + } } finally { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 1600c71e5ebfb4..1d89c9512f1a86 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -47,7 +47,7 @@ public void ResolvePath_PreservesComExtension() { File.WriteAllText(fullPath, "test"); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new(fileName); + ProcessStartOptions options = new($".\\{fileName}"); Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); } finally @@ -70,15 +70,10 @@ public void ResolvePath_FindsInSystemDirectory() Assert.StartsWith(systemDirectory, options.FileName, StringComparison.OrdinalIgnoreCase); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] - public void ResolvePath_FindsInWindowsDirectory() - { - ProcessStartOptions options = new("notepad"); - Assert.True(File.Exists(options.FileName)); - } - - [Fact] - public void ResolvePath_UsesCurrentDirectory() + [Theory] + [InlineData(".\\testapp.exe", true)] + [InlineData("testapp.exe", false)] + public void ResolvePath_UsesCurrentDirectory(string fileNameFormat, bool shouldSucceed) { string tempDir = Path.GetTempPath(); string fileName = "testapp.exe"; @@ -89,8 +84,17 @@ public void ResolvePath_UsesCurrentDirectory() { File.WriteAllText(fullPath, "test"); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new(fileName); - Assert.Equal(fullPath, options.FileName); + + if (shouldSucceed) + { + ProcessStartOptions options = new(fileNameFormat); + Assert.Equal(fullPath, options.FileName); + } + else + { + // Without .\ prefix, should not find file in CWD and should throw + Assert.Throws(() => new ProcessStartOptions(fileNameFormat)); + } } finally { @@ -155,5 +159,32 @@ public void ResolvePath_AbsolutePathIsNotModified() } } } + + [Fact] + public void ResolvePath_RootedButNotFullyQualifiedPath() + { + // Test paths like "C:foo.exe" which are rooted but not fully qualified + // These should be resolved to full paths + string tempFile = Path.GetTempFileName(); + try + { + // Get the drive letter and create a rooted but not fully qualified path + string drive = Path.GetPathRoot(tempFile); + string fileName = Path.GetFileName(tempFile); + string rootedPath = drive + fileName; // e.g., "C:tempfile.tmp" + + ProcessStartOptions options = new(rootedPath); + // Should resolve to fully qualified path + Assert.True(Path.IsPathFullyQualified(options.FileName)); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } } } From 200d9f4e2954c040b51cd93083beb99b9e83c5f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 10:47:56 +0000 Subject: [PATCH 24/28] Restore Process.Unix.cs ResolvePath for backward compatibility Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 74 ++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index fc46f14f918e7b..de4730fd603cad 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -446,7 +446,7 @@ private bool StartCore(ProcessStartInfo startInfo) } else { - filename = ProcessStartOptions.ResolvePath(startInfo.FileName); + filename = ResolvePath(startInfo.FileName); argv = ParseArgv(startInfo); if (Directory.Exists(filename)) { @@ -682,6 +682,78 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } + /// Resolves a path to the filename passed to ProcessStartInfo. + /// The filename. + /// The resolved path. It can return null in case of URLs. + private static string? ResolvePath(string filename) + { + // Follow the same resolution that Windows uses with CreateProcess: + // 1. First try the exact path provided + // 2. Then try the file relative to the executable directory + // 3. Then try the file relative to the current directory + // 4. then try the file in each of the directories specified in PATH + // Windows does additional Windows-specific steps between 3 and 4, + // and we ignore those here. + + // If the filename is a complete path, use it, regardless of whether it exists. + if (Path.IsPathRooted(filename)) + { + // In this case, it doesn't matter whether the file exists or not; + // it's what the caller asked for, so it's what they'll get + return filename; + } + + // Then check the executable's directory + string? path = Environment.ProcessPath; + if (path != null) + { + try + { + path = Path.Combine(Path.GetDirectoryName(path)!, filename); + if (File.Exists(path)) + { + return path; + } + } + catch (ArgumentException) { } // ignore any errors in data that may come from the exe path + } + + // Then check the current directory + path = Path.Combine(Directory.GetCurrentDirectory(), filename); + if (File.Exists(path)) + { + return path; + } + + // Then check each directory listed in the PATH environment variables + return FindProgramInPath(filename); + } + + /// + /// Gets the path to the program + /// + /// + /// + private static string? FindProgramInPath(string program) + { + string path; + string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + if (pathEnvVar != null) + { + var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); + while (pathParser.MoveNext()) + { + string subPath = pathParser.ExtractCurrent(); + path = Path.Combine(subPath, program); + if (IsExecutable(path)) + { + return path; + } + } + } + return null; + } + internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; From ec3f354a5c78e7e4854cdd5056f0da6890c8cb6c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 16 Feb 2026 13:59:43 +0100 Subject: [PATCH 25/28] address my own feedback, fix the tests --- .../System/Diagnostics/ProcessStartOptions.cs | 36 +++++++------------ .../tests/ProcessStartOptionsTests.Windows.cs | 34 +++++++++++------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index de785bb034b692..64a04b17b67424 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -30,8 +30,6 @@ public sealed class ProcessStartOptions /// /// /// The path is "resolved" meaning it has been converted to an absolute path and verified to exist. - /// If a relative path was provided to the constructor, it was located by searching in the executable's - /// directory, current directory, system directories (on Windows), and directories in the PATH environment variable. /// /// /// See for complete details on the resolution process. @@ -185,7 +183,7 @@ public ProcessStartOptions(string fileName) // There are two ways to create a process on Windows using CreateProcess sys-call: // 1. With NULL lpApplicationName and non-NULL lpCommandLine, where the first token of the // command line is the executable name. In this case, the system will resolve the executable - // name to an actual file on disk using documented search order. + // name to an actual file on disk using an algorithm that is not fully documented. // 2. With non-NULL lpApplicationName, where the system will use the provided application // name as-is without any resolution, and the command line is passed as-is to the process. // @@ -200,23 +198,9 @@ public ProcessStartOptions(string fileName) // We could either document that the FileName should never be user-controlled input or resolve it ourselves, // pass it as lpApplicationName and arguments as lpCommandLine. // - // On Unix, we were already resolving the executable path before starting the process. - // We were doing it for two reasons: - // 1. To mimic Windows resolution path order (consistency across platforms). - // 2. To avoid forking a process and then failing in exec because the file cannot be found. - // By resolving the path ourselves before forking we can avoid creating a child process that will fail immediately after creation. - // - // Changing the resolution logic could introduce breaking changes. For example: - // "If the executable module is a 16-bit application, lpApplicationName should be NULL, and the - // string pointed to by lpCommandLine should specify the executable module as well as its arguments." - // - // That is why we don't change the resolution logic for existing APIs, but since we are introducing a new API, - // we have the opportunity to do it in a more secure way by always resolving the executable name. - // Moreover, it gives the users the opportunity to: - // - check the resolved executable path before starting the process. - // - cache the resolved path and reuse it for subsequent process creations (perf). - // - // In upcoming .NET 11 previews, we may consider changing the order. + // Changing the resolution logic could introduce breaking changes, but since we are introducing a new API, + // we take it as an opportunity to clean up the legacy baggage to have simpler, easier to understand + // and more secure filename resolution algorithm that is consistent across OSes and aligned with other modern platforms. internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. @@ -232,6 +216,14 @@ public ProcessStartOptions(string fileName) return Path.GetFullPath(filename); // Resolve to absolute path } + // Check if this is an explicit relative path (contains directory separators like .\file or ../file) + // If so, resolve it relative to the current directory + ReadOnlySpan directoryName = Path.GetDirectoryName(filename); + if (!directoryName.IsEmpty) + { + return Path.GetFullPath(filename); + } + #if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. @@ -240,14 +232,12 @@ public ProcessStartOptions(string fileName) // HasExtension returns false for trailing dot, so we need to check that separately if (filename[filename.Length - 1] != '.' - && string.IsNullOrEmpty(Path.GetDirectoryName(filename)) + && directoryName.IsEmpty && !Path.HasExtension(filename)) { filename += ".exe"; } -#endif -#if WINDOWS // Windows-specific search location: the system directory (e.g., C:\Windows\System32) string path = System.Environment.SystemDirectory; if (path != null) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 1d89c9512f1a86..36670c87499b00 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -163,26 +163,36 @@ public void ResolvePath_AbsolutePathIsNotModified() [Fact] public void ResolvePath_RootedButNotFullyQualifiedPath() { - // Test paths like "C:foo.exe" which are rooted but not fully qualified - // These should be resolved to full paths - string tempFile = Path.GetTempFileName(); + // Test paths like "C:foo.exe" (without backslash after colon) which are rooted but not fully qualified + // These resolve relative to the current directory on that drive + string tempDir = Path.GetTempPath(); + string fileName = "test_rooted.tmp"; + string fullPath = Path.Combine(tempDir, fileName); + + string oldDir = Directory.GetCurrentDirectory(); try { - // Get the drive letter and create a rooted but not fully qualified path - string drive = Path.GetPathRoot(tempFile); - string fileName = Path.GetFileName(tempFile); - string rootedPath = drive + fileName; // e.g., "C:tempfile.tmp" - + File.WriteAllText(fullPath, "test"); + Directory.SetCurrentDirectory(tempDir); + + // Create a rooted but not fully qualified path: "C:filename" (no backslash after drive) + string drive = Path.GetPathRoot(tempDir)!.TrimEnd('\\', '/'); // e.g., "C:" + string rootedPath = $"{drive}{fileName}"; // e.g., "C:test_rooted.tmp" + + Assert.True(Path.IsPathRooted(rootedPath)); + Assert.False(Path.IsPathFullyQualified(rootedPath)); + ProcessStartOptions options = new(rootedPath); - // Should resolve to fully qualified path + Assert.True(Path.IsPathFullyQualified(options.FileName)); - Assert.Equal(tempFile, options.FileName); + Assert.Equal(fullPath, options.FileName); } finally { - if (File.Exists(tempFile)) + Directory.SetCurrentDirectory(oldDir); + if (File.Exists(fullPath)) { - File.Delete(tempFile); + File.Delete(fullPath); } } } From bdd63016443cd64e0a333901ee1a2fd5064df921 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 16 Feb 2026 15:29:13 +0100 Subject: [PATCH 26/28] update comments, minor cleanup --- .../System/Diagnostics/ProcessStartOptions.cs | 58 ++++++++++--------- .../tests/ProcessStartOptionsTests.Windows.cs | 7 +-- .../tests/ProcessStartOptionsTests.cs | 8 +-- 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 64a04b17b67424..51809244581625 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -138,30 +138,42 @@ public IList InheritedHandles /// Initializes a new instance of the class. /// /// The application to start. - /// Thrown when is null. - /// Thrown when is empty. - /// Thrown when cannot be resolved to an existing file. + /// is . + /// is empty. + /// cannot be resolved to an existing file. /// /// /// The is resolved to an absolute path before starting the process. /// /// - /// When the is a rooted path, it is used as-is without any resolution. + /// When the is a fully qualified path, it is used as-is without any resolution. /// /// - /// On Windows, when is not a rooted path, the system searches - /// for the executable in the following order: + /// When the is a rooted but not fully qualified path (for example, C:foo.exe or \foo\bar.exe on Windows), + /// it is resolved to an absolute path using the current directory context. + /// + /// + /// When the is an explicit relative path containing directory separators (for example, .\foo.exe or ../bar), + /// it is resolved relative to the current directory. + /// + /// + /// When the is a bare filename without directory separators, the system searches for the executable in the following locations: + /// + /// + /// On Windows: /// /// - /// The System directory. + /// The System directory (for example, C:\Windows\System32). /// The directories listed in the PATH environment variable. /// /// - /// On Unix, when is not a rooted path, the system searches - /// for the executable in the directories listed in the PATH environment variable. + /// On Unix: /// + /// + /// The directories listed in the PATH environment variable. + /// /// - /// On Windows, if the does not have an extension, ".exe" is appended to it before searching. + /// On Windows, if the does not have an extension and does not contain directory separators, .exe is appended before searching. /// /// public ProcessStartOptions(string fileName) @@ -187,20 +199,12 @@ public ProcessStartOptions(string fileName) // 2. With non-NULL lpApplicationName, where the system will use the provided application // name as-is without any resolution, and the command line is passed as-is to the process. // - // The official documentation mentions: - // "(..) do not pass NULL for lpApplicationName. If you do pass NULL for lpApplicationName, - // use quotation marks around the executable path in lpCommandLine." - // - // We have asked the CreateProcess owners (Windows Team) for feedback. This is what they wrote: - // "Applications should not under any circumstances pass user-controlled input directly to CreateProcess; - // or, if they intend to do so (for passing user-originated parameters), they should be filling out the lpApplicationName parameter." - // - // We could either document that the FileName should never be user-controlled input or resolve it ourselves, - // pass it as lpApplicationName and arguments as lpCommandLine. + // The recommended way is to use the second approach and provide the resolved executable path. // - // Changing the resolution logic could introduce breaking changes, but since we are introducing a new API, - // we take it as an opportunity to clean up the legacy baggage to have simpler, easier to understand - // and more secure filename resolution algorithm that is consistent across OSes and aligned with other modern platforms. + // Changing the resolution logic for existing Process APIs would introduce breaking changes. + // Since we are introducing a new API, we take it as an opportunity to clean up the legacy baggage + // to have simpler, easier to understand and more secure filename resolution algorithm + // that is more consistent across OSes and aligned with other modern platforms. internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. @@ -224,6 +228,8 @@ public ProcessStartOptions(string fileName) return Path.GetFullPath(filename); } + // We want to keep the resolution logic in one place for better maintainability and consistency. + // That is why we don't provide platform-specific implementations files and use #if directives here. #if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. @@ -231,9 +237,7 @@ public ProcessStartOptions(string fileName) // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." // HasExtension returns false for trailing dot, so we need to check that separately - if (filename[filename.Length - 1] != '.' - && directoryName.IsEmpty - && !Path.HasExtension(filename)) + if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) { filename += ".exe"; } @@ -259,7 +263,7 @@ public ProcessStartOptions(string fileName) if (pathEnvVar != null) { char pathSeparator = OperatingSystem.IsWindows() ? ';' : ':'; - var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + StringParser pathParser = new(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 36670c87499b00..df2dabdae7624d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; -using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.Diagnostics.Tests @@ -48,7 +47,7 @@ public void ResolvePath_PreservesComExtension() File.WriteAllText(fullPath, "test"); Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new($".\\{fileName}"); - Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.EndsWith(".com", options.FileName, StringComparison.Ordinal); } finally { @@ -66,8 +65,8 @@ public void ResolvePath_FindsInSystemDirectory() // cmd.exe should be found in system directory ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); - string systemDirectory = Environment.SystemDirectory; - Assert.StartsWith(systemDirectory, options.FileName, StringComparison.OrdinalIgnoreCase); + string expectedPath = Path.Combine(Environment.SystemDirectory, "cmd.exe"); + Assert.Equal(expectedPath, options.FileName); } [Theory] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index 75b361e3337877..ed202b5d22648c 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -4,8 +4,6 @@ using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; -using Microsoft.DotNet.RemoteExecutor; -using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.Diagnostics.Tests @@ -93,9 +91,9 @@ public void Environment_CaseSensitivityIsPlatformSpecific() { ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; - + env["TestKey"] = "TestValue"; - + if (OperatingSystem.IsWindows()) { Assert.True(env.ContainsKey("testkey")); @@ -120,7 +118,7 @@ public void InheritedHandles_DefaultIsEmpty() public void InheritedHandles_CanSet() { ProcessStartOptions options = new(GetCurrentProcessName()); - List newHandles = new List(); + List newHandles = []; options.InheritedHandles = newHandles; Assert.Same(newHandles, options.InheritedHandles); } From 02905f0a2bf5632e715b5bb679548e628b6d40fe Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 16 Feb 2026 15:51:55 +0100 Subject: [PATCH 27/28] changes after reading everything again --- .../src/System/Diagnostics/Process.Unix.cs | 27 +------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index de4730fd603cad..1e9a9d79f164b6 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -726,32 +726,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); - } - - /// - /// Gets the path to the program - /// - /// - /// - private static string? FindProgramInPath(string program) - { - string path; - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); - if (pathEnvVar != null) - { - var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); - while (pathParser.MoveNext()) - { - string subPath = pathParser.ExtractCurrent(); - path = Path.Combine(subPath, program); - if (IsExecutable(path)) - { - return path; - } - } - } - return null; + return ProcessStartOptions.FindProgramInPath(filename); } internal static bool IsExecutable(string fullPath) From bf6e551cc5effa75c1223114c818f61800041472 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 16 Feb 2026 16:11:17 +0100 Subject: [PATCH 28/28] apply improved suggestion --- .../src/System/Diagnostics/ProcessStartOptions.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 51809244581625..5b81936571727c 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -243,14 +243,10 @@ public ProcessStartOptions(string fileName) } // Windows-specific search location: the system directory (e.g., C:\Windows\System32) - string path = System.Environment.SystemDirectory; - if (path != null) + string path = Path.Combine(System.Environment.SystemDirectory, filename); + if (File.Exists(path)) { - path = Path.Combine(path, filename); - if (File.Exists(path)) - { - return path; - } + return path; } #endif