From 316fe0a336bbd24da66a15c81c83c488700ece4e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 05:36:25 +0000 Subject: [PATCH 1/4] Let a caller close a command's standard input Standard output and standard error were redirected but standard input was not, so a command inherited the caller's handle and a command that reads it waited there. In a host that is not a console that handle never produces data and never closes, so the wait ended only on cancellation: a hang rather than an error, on the callers least able to notice it. CommandOptions.StandardInput selects between inheriting, which is what commands did before and what an interactive command needs, and closing, which gives the command its own standard input and closes it so a read reports end of stream. Redirecting alone is not enough - it leaves the command holding a pipe nobody writes to, which is the same wait - so the stream is closed immediately after the process starts. Elevation forces UseShellExecute, which has no stream to redirect, so combining the two throws up front as EnvironmentVariables already does. Two tests, because one is not enough to catch both ways this can regress. The behavioural test fails if the stream is redirected but left open. The second compares /proc/self/fd/0 between the caller and the command, which is what catches the option being ignored altogether: a test runner whose own standard input is already at end of stream hands a child the same answer by inheritance, so the behavioural test alone passes there even when nothing is redirected. Both were confirmed to fail against a mutated build. Fixes #81 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Sd7rWEYm4btqSsd2Hbuic --- CLAUDE.md | 30 +++++++-- README.md | 30 +++++++++ RunCommand.Test/RunCommandTests.cs | 97 ++++++++++++++++++++++++++++++ RunCommand/CommandOptions.cs | 12 ++++ RunCommand/RunCommand.cs | 19 ++++++ RunCommand/StandardInputMode.cs | 36 +++++++++++ 6 files changed, 219 insertions(+), 5 deletions(-) create mode 100644 RunCommand/StandardInputMode.cs 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..e16b81a 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -814,4 +814,101 @@ 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. + /// + private static (string FileName, string[] Arguments) GetReadStandardInputCommand() => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? ("cmd", ["/c", "set \"line=\" & set /p line= & echo read:[%line%]"]) + : ("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, +} From 936c0790ce49cb6aa4b2f9ab23786e5c9bb62c4d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 05:42:34 +0000 Subject: [PATCH 2/4] Report the Windows read through delayed expansion The Windows arm of GetReadStandardInputCommand echoed %line%, which came back literal: at a cmd /c command line an undefined variable is left as written rather than expanding to nothing, which is a batch-file behaviour and not a command-line one. So the report read "read:[%line%]" and said nothing about what the read did. /v:on and !line! expand when the echo runs rather than when the line is parsed, so an undefined variable reports as empty and the Windows arm now means what the POSIX one means. The prompt is left empty by putting nothing between = and the separator, so no prompt text reaches the captured output. Test-only. The library change was already right on Windows: the failing run completed the command in 64ms rather than waiting, which is the behaviour under test - standard input was redirected and closed, and set /p returned at end of stream. Only the assertion's report was wrong. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Sd7rWEYm4btqSsd2Hbuic --- RunCommand.Test/RunCommandTests.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index e16b81a..8502e49 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -824,9 +824,18 @@ public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() /// read ends immediately, and with it inherited from a handle nobody writes to the command waits /// there instead. /// + /// + /// The Windows arm needs /v:on. At a cmd /c command line an undefined + /// %line% is left literal rather than expanding to nothing — that is a batch-file + /// behaviour, not a command-line one — so the report came back as read:[%line%] and said + /// nothing about the read. Delayed expansion also evaluates !line! when the echo runs + /// rather than when the line is parsed, which is what makes it report the read at all. 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= & echo read:[%line%]"]) + ? ("cmd", ["/v:on", "/c", "set /p line=&echo read:[!line!]"]) : ("sh", ["-c", "read line; echo \"read:[$line]\""]); [TestMethod] From 542e9796f6577b7898d12f463c23f2e3c19c6d64 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 05:47:19 +0000 Subject: [PATCH 3/4] Report the Windows read without shell expansion Neither expansion survives this command line. %line% came back literal, because expanding an undefined variable to nothing is batch-file behaviour rather than command-line behaviour, and !line! under /v:on came back literal too. Both reported the variable's own name instead of what the read did, so the assertion was checking nothing on Windows either way. The command now reports through control flow: if defined is a run-time test on the name and needs no expansion, and every string reaching standard output is a literal. set line= runs first so a variable inherited from the environment cannot make it report a read that never happened. Still test-only, and the behaviour under test has been green on Windows throughout: both failing runs completed the command in well under the token's 30 seconds, which is what says standard input was redirected and closed and that set /p returned at end of stream. Only the report was wrong. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Sd7rWEYm4btqSsd2Hbuic --- RunCommand.Test/RunCommandTests.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 8502e49..5cb65cb 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -825,17 +825,20 @@ public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() /// there instead. /// /// - /// The Windows arm needs /v:on. At a cmd /c command line an undefined - /// %line% is left literal rather than expanding to nothing — that is a batch-file - /// behaviour, not a command-line one — so the report came back as read:[%line%] and said - /// nothing about the read. Delayed expansion also evaluates !line! when the echo runs - /// rather than when the line is parsed, which is what makes it report the read at all. The - /// prompt is left empty by putting nothing between = and the separator, so no prompt text - /// reaches the captured output. + /// 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", ["/v:on", "/c", "set /p line=&echo read:[!line!]"]) + ? ("cmd", ["/c", "set line=&set /p line=&if defined line (echo read:[unexpected]) else (echo read:[])"]) : ("sh", ["-c", "read line; echo \"read:[$line]\""]); [TestMethod] From 0b59ebc2f53182f7a40ccdd3578bb4ea596e6fd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 10:51:55 +0000 Subject: [PATCH 4/4] Fold the Windows note into one remarks element The method carried two elements, because the Windows note was appended as a second block rather than merged into the existing one. A member takes one, so a documentation tool reading this would keep one and drop the other. It builds clean either way, which is why it survived: the method is private, so no documentation warning fires on it. The Windows note is now a inside the single remarks element. Text unchanged, no behaviour touched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Sd7rWEYm4btqSsd2Hbuic --- RunCommand.Test/RunCommandTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 5cb65cb..eadac38 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -823,8 +823,7 @@ public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() /// 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 @@ -835,6 +834,7 @@ public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() /// 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)