diff --git a/README.md b/README.md index c456b40..fac9307 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,11 @@ class Program options: new() { WorkingDirectory = AbsoluteDirectoryPath.Create(@"C:\repos\my project"), + EnvironmentVariables = new Dictionary + { + ["GIT_TERMINAL_PROMPT"] = "0", + ["LC_ALL"] = "C", + }, }); Console.WriteLine($"Process exited with code: {exitCode}"); @@ -172,6 +177,19 @@ class Program Without a `WorkingDirectory` the process inherits the current directory of the calling process, which is what commands did before this option existed. +`EnvironmentVariables` is an overlay on the inherited environment, not a replacement: a name you do not list keeps whatever the calling process had. A `null` value removes a variable, which is how you unset something the parent had set: + +```csharp +EnvironmentVariables = new Dictionary +{ + ["GIT_DIR"] = null, +} +``` + +Environment variables are the only control surface some tools expose, so this covers behaviour with no command-line equivalent — `GIT_TERMINAL_PROMPT=0` to make an authenticating `git fetch` fail rather than block forever on a prompt no terminal will answer, `GIT_ASKPASS`/`SSH_ASKPASS` to supply credentials without putting them on a command line where any process listing can read them, and `LC_ALL=C` to force stable, machine-parseable output rather than whatever the host locale produces. + +> **_NOTE:_** _`EnvironmentVariables` cannot be combined with `Elevation.Elevated` on Windows. Elevation requires `UseShellExecute`, which offers nowhere to pass an environment, so the call throws `ArgumentException` rather than silently dropping the variables._ + The type is `AbsoluteDirectoryPath` rather than a string on purpose. A relative directory would have to be resolved against the caller's current directory — the process-global state this option exists to avoid depending on, since it is shared by every thread and races with concurrent calls. `CommandOptions.Elevation` carries the privilege level, so a single options object replaces the separate `Elevation` argument. @@ -219,6 +237,7 @@ class Program ### CommandOptions Record - `WorkingDirectory`: An `AbsoluteDirectoryPath` naming the directory the process starts in, or `null` to inherit the caller's current directory. +- `EnvironmentVariables`: An `IReadOnlyDictionary` applied over the inherited environment, or `null` to inherit it unchanged. A `null` value removes a variable. - `Elevation`: The privilege level under which to run the command. Defaults to `Elevation.Default`. ### Elevation Enum diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 42093c2..b523908 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -323,6 +323,35 @@ private static (string FileName, string[] Arguments) GetPrintWorkingDirectoryCom private static string CreateDirectoryForTest([CallerMemberName] string caller = "") => Directory.CreateDirectory(Path.Join(Path.GetTempPath(), $"{nameof(RunCommandTests)} {caller}")).FullName; + /// + /// Returns a command that prints the value of an environment variable, wrapped in brackets so + /// that an empty value is still distinguishable from no output at all. + /// + private static (string FileName, string[] Arguments) GetPrintEnvironmentVariableCommand(string name) => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? ("cmd", ["/c", $"echo [%{name}%]"]) + : ("sh", ["-c", $"echo \"[${name}]\""]); + + // Tests run in parallel and the host environment is process-wide, so each test needs its own + // variable name to avoid stepping on another test's value. + private static string EnvironmentVariableNameFor([CallerMemberName] string caller = "") => + $"RUNCOMMAND_TEST_{caller.ToUpperInvariant()}"; + + private static async Task ReadEnvironmentVariableFromChildAsync(string name, CommandOptions options) + { + (string fileName, string[] arguments) = GetPrintEnvironmentVariableCommand(name); + List output = []; + + int exitCode = await RunCommand.ExecuteAsync( + fileName, + arguments, + new LineOutputHandler(onStandardOutput: output.Add), + options).ConfigureAwait(false); + + Assert.AreEqual(0, exitCode, "Expected the command to run successfully."); + return string.Concat(output).Trim(); + } + /// /// Returns a command that runs for long enough to be cancelled mid-flight. /// @@ -477,4 +506,98 @@ public async Task ExecuteAsyncShouldThrowArgumentNullExceptionWhenOptionsAreNull await Assert.ThrowsAsync( () => RunCommand.ExecuteAsync("dotnet", ["--version"], new OutputHandler(), null!)).ConfigureAwait(false); } + [TestMethod] + public async Task ExecuteAsyncShouldSetAnEnvironmentVariableForTheChildProcess() + { + string name = EnvironmentVariableNameFor(); + + string reported = await ReadEnvironmentVariableFromChildAsync( + name, + new CommandOptions { EnvironmentVariables = new Dictionary { [name] = "expected" } }).ConfigureAwait(false); + + Assert.AreEqual("[expected]", reported, "Expected the child to see the variable that was set for it."); + } + + [TestMethod] + public async Task ExecuteAsyncShouldOverrideAnInheritedEnvironmentVariable() + { + string name = EnvironmentVariableNameFor(); + Environment.SetEnvironmentVariable(name, "inherited"); + + try + { + string reported = await ReadEnvironmentVariableFromChildAsync( + name, + new CommandOptions { EnvironmentVariables = new Dictionary { [name] = "override" } }).ConfigureAwait(false); + + Assert.AreEqual("[override]", reported, "Expected the overlay to win over the inherited value."); + } + finally + { + Environment.SetEnvironmentVariable(name, null); + } + } + + [TestMethod] + public async Task ExecuteAsyncShouldRemoveAnInheritedEnvironmentVariableWhenTheValueIsNull() + { + string name = EnvironmentVariableNameFor(); + Environment.SetEnvironmentVariable(name, "inherited"); + + try + { + string reported = await ReadEnvironmentVariableFromChildAsync( + name, + new CommandOptions { EnvironmentVariables = new Dictionary { [name] = null } }).ConfigureAwait(false); + + // An unset variable prints differently per shell -- cmd echoes the name back verbatim, + // sh prints nothing -- so this pins the part that matters on both: the inherited value + // did not reach the child. + Assert.DoesNotContain("inherited", reported, "Expected a null value to remove the inherited variable."); + } + finally + { + Environment.SetEnvironmentVariable(name, null); + } + } + + [TestMethod] + public async Task ExecuteAsyncShouldInheritTheEnvironmentWhenNoVariablesAreGiven() + { + string name = EnvironmentVariableNameFor(); + Environment.SetEnvironmentVariable(name, "inherited"); + + try + { + string reported = await ReadEnvironmentVariableFromChildAsync(name, new CommandOptions()).ConfigureAwait(false); + + Assert.AreEqual("[inherited]", reported, "Expected an unset overlay to leave the previous behaviour untouched."); + } + finally + { + Environment.SetEnvironmentVariable(name, null); + } + } + + [TestMethod] + public async Task ExecuteAsyncShouldRejectEnvironmentVariablesCombinedWithElevation() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Inconclusive("Elevation only changes how the process is started on Windows."); + } + + // Elevation forces UseShellExecute, which cannot carry an environment. Failing loudly beats + // silently dropping variables the caller may be relying on. + await Assert.ThrowsAsync( + () => RunCommand.ExecuteAsync( + "cmd", + ["/c", "exit 0"], + new OutputHandler(), + new CommandOptions + { + Elevation = Elevation.Elevated, + EnvironmentVariables = new Dictionary { ["ANY"] = "value" }, + })).ConfigureAwait(false); + } } diff --git a/RunCommand/CommandOptions.cs b/RunCommand/CommandOptions.cs index 7d13390..fed091d 100644 --- a/RunCommand/CommandOptions.cs +++ b/RunCommand/CommandOptions.cs @@ -2,6 +2,7 @@ namespace ktsu.RunCommand; +using System.Collections.Generic; using ktsu.Semantics.Paths; /// @@ -24,6 +25,18 @@ public sealed record CommandOptions /// public AbsoluteDirectoryPath? WorkingDirectory { get; init; } + /// + /// Gets the environment variables to apply over the inherited environment, or + /// to inherit the calling process's environment unchanged. + /// + /// + /// The entries are an overlay rather than a replacement: a name not listed here keeps whatever + /// the calling process had. A value removes a variable, matching the + /// semantics of , which is how a + /// caller unsets something the parent had set. + /// + public IReadOnlyDictionary? EnvironmentVariables { get; init; } + /// /// Gets the privilege level under which to run the command. /// diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 6b4add5..4267a41 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -298,6 +298,17 @@ private static ProcessStartInfo CreateStartInfo(string fileName, OutputHandler o bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); useElevation = options.Elevation == Elevation.Elevated && isWindows; + if (useElevation && options.EnvironmentVariables is not null) + { + // Elevation needs UseShellExecute, which starts the process through the shell and offers + // nowhere to put an environment. Saying so here beats letting Process.Start fail with a + // message that does not mention either setting, and beats silently dropping variables a + // caller may be relying on for credentials or machine-parseable output. + throw new ArgumentException( + "Environment variables cannot be set for an elevated command, because elevation requires UseShellExecute.", + nameof(options)); + } + ProcessStartInfo startInfo = new() { FileName = fileName, @@ -322,6 +333,21 @@ private static ProcessStartInfo CreateStartInfo(string fileName, OutputHandler o startInfo.StandardErrorEncoding = outputHandler.Encoding; startInfo.UseShellExecute = false; + if (options.EnvironmentVariables is not null) + { + foreach (KeyValuePair variable in options.EnvironmentVariables) + { + if (variable.Value is null) + { + _ = startInfo.Environment.Remove(variable.Key); + } + else + { + startInfo.Environment[variable.Key] = variable.Value; + } + } + } + if (isWindows) { startInfo.LoadUserProfile = true;