diff --git a/CLAUDE.md b/CLAUDE.md index fc26f27..ea79198 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,11 +53,12 @@ The test project targets `net10.0` only. ### Key Files - `RunCommand/RunCommand.cs` - Static class holding the whole public execution API and the private `CreateStartInfo`/`RunAsync`/`TryKill` core -- `RunCommand/CommandOptions.cs` - Record carrying process-shaping settings (working directory, environment variables, elevation) +- `RunCommand/CommandOptions.cs` - Record carrying process-shaping settings (working directory, environment variables, elevation, standard input) - `RunCommand/OutputHandler.cs` - Base output handler delivering raw chunks - `RunCommand/LineOutputHandler.cs` - Derived handler that buffers chunks into complete lines - `RunCommand/AsyncProcessStreamReader.cs` - Internal concurrent reader for stdout and stderr - `RunCommand/Elevation.cs` - Enum selecting the privilege level +- `RunCommand/StandardInputMode.cs` - Enum selecting what standard input is connected to ### Dependencies @@ -113,10 +114,29 @@ builds can only kill the process itself. ### Elevation constraints -Elevation forces `UseShellExecute = true`, which is incompatible with both output redirection and -setting an environment. Consequently an `OutputHandler` is silently not invoked under elevation -(documented behaviour), while combining `EnvironmentVariables` with elevation throws -`ArgumentException` up front rather than failing opaquely inside `Process.Start`. +Elevation forces `UseShellExecute = true`, which is incompatible with output redirection, setting an +environment, and redirecting standard input. Consequently an `OutputHandler` is silently not invoked +under elevation (documented behaviour), while combining either `EnvironmentVariables` or +`StandardInputMode.Closed` with elevation throws `ArgumentException` up front rather than failing +opaquely inside `Process.Start`. + +### Standard input + +Standard input is inherited by default, which is what commands did before `CommandOptions.StandardInput` +existed and what an interactive command needs. `StandardInputMode.Closed` redirects it and closes the +stream immediately after `Process.Start`, so a command that reads it sees end of stream. + +Both halves are load-bearing. Redirecting alone leaves the command holding a pipe nobody writes to, +which is the same wait as inheriting; closing is what turns a read into end of stream. Callers that +are not consoles — services, daemons, background workers — want `Closed`, because an inherited handle +that stays open without producing data turns a command that reads it into a hang that ends only on +cancellation. + +Testing this needs care. A test runner whose own standard input is already at end of stream hands a +child the same answer by inheritance, so a behavioural test alone passes even when the option is +ignored entirely. `ExecuteAsyncShouldGiveTheCommandItsOwnStandardInputWhenClosed` compares +`/proc/self/fd/0` between the caller and the command to pin the redirection itself, and is Linux-only +for that reason. ### Argument escaping diff --git a/README.md b/README.md index e67f772..6388281 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,26 @@ Environment variables are the only control surface some tools expose, so this co > **_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._ +### Standard Input + +By default a command inherits the calling process's standard input, which is what an interactive command needs. That is a hazard anywhere the caller is not a console: if the inherited handle stays open without ever producing data, a command that reads it waits there, and the run ends only when you cancel it. Standard output and standard error are already redirected away from your console, so a command that prompts cannot be answered anyway. + +Set `StandardInputMode.Closed` to give the command its own standard input and close it, so a read reports end of stream instead: + +```csharp +int exitCode = RunCommand.Execute( + "git", + ["fetch", "--prune"], + new OutputHandler(Console.Write, Console.Error.Write), + new CommandOptions { StandardInput = StandardInputMode.Closed }); +``` + +This is what a long-running host wants — a service, a daemon, a background worker — where a command that waits forever is a hang rather than an error. It also isolates the command from your own standard input, so nothing it reads can consume input you meant to read yourself. + +`GIT_TERMINAL_PROMPT=0` and similar environment settings cover the case where a tool deliberately prompts, but not a command that simply reads standard input for its own reasons; closing the stream covers both. + +> **_NOTE:_** _`StandardInputMode.Closed` cannot be combined with `Elevation.Elevated` on Windows. Elevation requires `UseShellExecute`, which offers no stream to redirect, so the call throws `ArgumentException` rather than starting a command whose standard input is still yours._ + ## Elevation (Windows) To run a command with elevated privileges, set `Elevation.Elevated`. On Windows this launches the process with the `runas` verb, which triggers a UAC prompt: @@ -345,6 +365,16 @@ Record describing how to shape the process a command runs in. Every member defau | `WorkingDirectory` | `AbsoluteDirectoryPath?` | The directory the process starts in, or `null` to inherit the caller's current directory. | | `EnvironmentVariables` | `IReadOnlyDictionary?` | Variables applied over the inherited environment, or `null` to inherit it unchanged. A `null` value removes a variable. | | `Elevation` | `Elevation` | The privilege level under which to run the command. Defaults to `Elevation.Default`. | +| `StandardInput` | `StandardInputMode` | What the command's standard input is connected to. Defaults to `StandardInputMode.Inherit`. | + +### `StandardInputMode` + +Enum specifying what a command's standard input is connected to. + +| Name | Description | +|------|-------------| +| `Inherit` | Inherit the calling process's standard input. A command reading an inherited handle that never produces data waits until it closes. | +| `Closed` | Redirect the command's standard input and close it, so a read reports end of stream. Cannot be combined with `Elevation.Elevated` on Windows. | ### `OutputHandler` diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 03fd4c4..eadac38 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -814,4 +814,113 @@ public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() thrown.Flatten().InnerExceptions.Any(e => e is DecoderFallbackException), $"Expected a decode failure, got: {thrown}"); } + + /// + /// Returns a command that reads one line from standard input and then reports what it read. + /// + /// + /// At end of stream the read fails and the variable stays empty, so the command still reaches its + /// report and exits. That is the whole distinction being tested: with standard input closed the + /// read ends immediately, and with it inherited from a handle nobody writes to the command waits + /// there instead. + /// + /// The Windows arm reports the read through control flow rather than by echoing the variable, + /// because neither kind of expansion survives this command line. %line% comes back + /// literal — expanding an undefined variable to nothing is batch-file behaviour, not + /// command-line behaviour — and !line! under /v:on came back literal too, so both + /// reported read:[%line%] and read:[!line!] instead of saying what the read did. + /// if defined is a run-time test on the name, so it needs no expansion at all, and every + /// string that reaches standard output is a literal. set line= first, so a variable + /// inherited from the environment cannot make the command report a read that never happened. + /// The prompt is left empty by putting nothing between = and the separator, so no prompt + /// text reaches the captured output. + /// + /// + private static (string FileName, string[] Arguments) GetReadStandardInputCommand() => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? ("cmd", ["/c", "set line=&set /p line=&if defined line (echo read:[unexpected]) else (echo read:[])"]) + : ("sh", ["-c", "read line; echo \"read:[$line]\""]); + + [TestMethod] + public async Task ExecuteAsyncShouldEndACommandThatReadsStandardInputWhenStandardInputIsClosed() + { + // The token is the assertion. A command whose standard input is redirected but left open waits + // on a pipe nobody writes to, exactly as one inheriting an idle handle does, so a regression + // on either half ends this test by cancelling it rather than by hanging the suite. + using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(30)); + + StringBuilder output = new(); + (string fileName, string[] arguments) = GetReadStandardInputCommand(); + + int exitCode = await RunCommand.ExecuteAsync( + fileName, + arguments, + new OutputHandler(o => output.Append(o)), + new CommandOptions { StandardInput = StandardInputMode.Closed }, + cancellation.Token).ConfigureAwait(false); + + Assert.AreEqual(0, exitCode, $"Expected the command to run to completion. Output: {output}"); + Assert.Contains("read:[]", output.ToString(), $"Expected the read to report end of stream. Output: {output}"); + } + + [TestMethod] + public async Task ExecuteAsyncShouldGiveTheCommandItsOwnStandardInputWhenClosed() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + Assert.Inconclusive("Needs procfs to name a file descriptor. The redirection this covers is in platform independent code, so the other legs cover it."); + } + + // Reaching end of stream does not by itself prove the command got the library's pipe. A caller + // whose own standard input is already at end of stream — /dev/null under most test runners — + // hands its child the same answer by inheritance, so a change that ignored the option + // altogether would still look right there. Comparing the descriptors tells them apart: + // redirection always makes a new pipe, so the command's standard input is whatever this + // process has only when it was inherited. + using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(30)); + + string ownStandardInput = File.ResolveLinkTarget("/proc/self/fd/0", returnFinalTarget: false)?.FullName ?? ""; + Assert.AreNotEqual("", ownStandardInput, "Expected to be able to name this process's own standard input."); + + StringBuilder output = new(); + + int exitCode = await RunCommand.ExecuteAsync( + "sh", + ["-c", "readlink /proc/self/fd/0"], + new OutputHandler(o => output.Append(o)), + new CommandOptions { StandardInput = StandardInputMode.Closed }, + cancellation.Token).ConfigureAwait(false); + + Assert.AreEqual(0, exitCode, $"Expected the probe to run successfully. Output: {output}"); + + string commandStandardInput = output.ToString().Trim(); + Assert.AreNotEqual( + ownStandardInput, + commandStandardInput, + "Expected the command's standard input to be the library's own pipe rather than this process's inherited handle."); + Assert.StartsWith("pipe:", commandStandardInput, $"Expected the command's standard input to be a pipe, got: {commandStandardInput}"); + } + + [TestMethod] + public async Task ExecuteAsyncShouldRejectClosedStandardInputCombinedWithElevation() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Inconclusive("Elevation only changes how the process is started on Windows."); + } + + // Elevation forces UseShellExecute, which has no stream to redirect. Failing loudly beats + // starting a command whose standard input is still the caller's, which is the wait the option + // was asked for to avoid. + await Assert.ThrowsAsync( + () => RunCommand.ExecuteAsync( + "cmd", + ["/c", "exit 0"], + new OutputHandler(), + new CommandOptions + { + Elevation = Elevation.Elevated, + StandardInput = StandardInputMode.Closed, + })).ConfigureAwait(false); + } } diff --git a/RunCommand/CommandOptions.cs b/RunCommand/CommandOptions.cs index fed091d..e7fbb3e 100644 --- a/RunCommand/CommandOptions.cs +++ b/RunCommand/CommandOptions.cs @@ -41,4 +41,16 @@ public sealed record CommandOptions /// Gets the privilege level under which to run the command. /// public Elevation Elevation { get; init; } = Elevation.Default; + + /// + /// Gets what the command's standard input is connected to. + /// + /// + /// Defaults to , which is what commands did before this + /// option existed. Standard output and standard error are already redirected away from the + /// caller's console, so a command that prompts cannot be answered anyway; a caller that is not + /// itself a console generally wants , so that a command + /// reading standard input ends rather than waiting on a handle nobody will write to. + /// + public StandardInputMode StandardInput { get; init; } = StandardInputMode.Inherit; } diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 731bf16..3ea6e54 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -321,6 +321,17 @@ private static ProcessStartInfo CreateStartInfo(string fileName, OutputHandler o bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); useElevation = options.Elevation == Elevation.Elevated && isWindows; + if (useElevation && options.StandardInput == StandardInputMode.Closed) + { + // Same reason as the environment check below: UseShellExecute offers no stream to + // redirect, so the request cannot be honoured. Saying so beats starting a command whose + // standard input is still the caller's, which is the hang StandardInputMode.Closed exists + // to prevent. + throw new ArgumentException( + "Standard input cannot be closed for an elevated command, because elevation requires UseShellExecute.", + nameof(options)); + } + if (useElevation && options.EnvironmentVariables is not null) { // Elevation needs UseShellExecute, which starts the process through the shell and offers @@ -354,6 +365,7 @@ private static ProcessStartInfo CreateStartInfo(string fileName, OutputHandler o startInfo.RedirectStandardError = true; startInfo.StandardOutputEncoding = outputHandler.Encoding; startInfo.StandardErrorEncoding = outputHandler.Encoding; + startInfo.RedirectStandardInput = options.StandardInput == StandardInputMode.Closed; startInfo.UseShellExecute = false; if (options.EnvironmentVariables is not null) @@ -452,6 +464,13 @@ private static async Task RunAsync(ProcessStartInfo startInfo, OutputHandle process.Start(); + if (startInfo.RedirectStandardInput) + { + // Redirecting alone would leave the command holding a pipe nobody writes to, which is the + // same wait as inheriting. Closing it is what turns a read into end of stream. + process.StandardInput.Close(); + } + // Killing the process is what makes the await actually stop: without it a cancelled wait // would leave the child running unsupervised. using CancellationTokenRegistration registration = cancellationToken.Register(() => TryKill(process)); diff --git a/RunCommand/StandardInputMode.cs b/RunCommand/StandardInputMode.cs new file mode 100644 index 0000000..e37ef4c --- /dev/null +++ b/RunCommand/StandardInputMode.cs @@ -0,0 +1,36 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.RunCommand; + +/// +/// Specifies what a command's standard input is connected to. +/// +public enum StandardInputMode +{ + /// + /// Let the command inherit the calling process's standard input. + /// + /// + /// A command that reads standard input then reads the caller's, which is what an interactive + /// command needs and what commands did before this option existed. It is also a hazard for a + /// caller that is not a console: if the inherited handle stays open without ever producing data, + /// a command that reads it blocks until the handle closes, and the run only ends when the caller + /// cancels it. Prefer in a long-running host, where a command that waits + /// forever is a hang rather than an error. + /// + Inherit, + + /// + /// Redirect the command's standard input and close it immediately, so a read reports end of + /// stream rather than waiting for input that is never coming. + /// + /// + /// The command is isolated from the caller's standard input, so nothing it reads can consume + /// input the caller intended for itself. + /// + /// This cannot be combined with on Windows, because elevation + /// requires UseShellExecute, which offers no stream to redirect. + /// + /// + Closed, +}