From ed58449fe18ee98e6c8817c3a51db6900da6bc04 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Wed, 19 Aug 2026 20:23:02 +1000 Subject: [PATCH] [minor] Add environment variables to CommandOptions ProcessStartInfo.Environment was never touched, so a child always inherited the host's environment verbatim with no way to add, override or remove a variable for a single call. That blocks a class of CLI behaviour with no flag equivalent: GIT_TERMINAL_PROMPT=0, without which a fetch needing credentials blocks forever on a prompt no terminal will answer; GIT_ASKPASS and SSH_ASKPASS, the supported way to supply credentials without exposing them in a process listing; and LC_ALL=C, without which output parsing silently depends on the host locale. The entries are an overlay rather than a replacement, so a name that is not listed keeps whatever the caller had, and a null value removes a variable, matching ProcessStartInfo.Environment semantics. Leaving the property null preserves today's behaviour exactly. Elevation requires UseShellExecute, which has nowhere to put an environment, so combining the two throws rather than silently dropping variables a caller may be relying on for credentials. Fixes #40 --- README.md | 19 +++++ RunCommand.Test/RunCommandTests.cs | 123 +++++++++++++++++++++++++++++ RunCommand/CommandOptions.cs | 13 +++ RunCommand/RunCommand.cs | 26 ++++++ 4 files changed, 181 insertions(+) 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;