fix: bound captured script output across execution engines

docs/claude-reference.md

@@ -23,6 +23,14 @@ verwalteten WinRM-Pfad. Ohne Maschine beziehungsweise bei Loopback ohne Credenti
 läuft es lokal auf dem API-Host. `engine: auto` startet Nutzerskripte in Windows
 PowerShell 5.1. Eingebaute Activities verwenden lokal den In-Process-Pool.
 
+`Engine:MaxCapturedOutputChars` begrenzt die während einer Skriptausführung gesammelte
+Ausgabe über alle Streams auf insgesamt 16.777.216 Zeichen (positiver Wert, Neustart erforderlich).
+Der Standard lässt Platz für die beiden auf je 1 MiB begrenzten Ausgaben von `startProgram`
+einschließlich JSON-Escaping. Kleinere Werte begrenzen auch diese Aktivität entsprechend.
+Das gemeinsame Limit gilt für Prozess-, Runspace- und WinRM-Ausführung. Überschreitung
+beendet den Schritt mit einem Ausgabelimit-Fehler; das spätere Kürzen gespeicherter Ergebnisse
+ist davon unabhängig. Prozess-Pipes werden auch nach Erreichen des Limits weiter geleert.
+
 Selbst aufgebautes Remoting innerhalb eines lokalen Skripts liegt außerhalb der
 verwalteten WinRM-Verbindung. Dabei entfallen deren Credential-Anbindung,
 Target-Zuordnung, Telemetrie und Transport-Hardening. Das Skript muss seine

src/NodePilot.Api/appsettings.json

@@ -161,6 +161,7 @@
     },
     "MaxConcurrentSteps": 600,
     "IsolatedDrainGraceSeconds": 5,
+    "MaxCapturedOutputChars": 16777216,
     "Runspace": {
       "MinRunspaces": 256,
       "MaxRunspaces": 768

src/NodePilot.Core/Execution/ScriptOutputBudget.cs

@@ -0,0 +1,64 @@
+using System.Text;
+
+namespace NodePilot.Core.Execution;
+
+/// <summary>A shared character budget for all output streams of one script invocation.</summary>
+public sealed class ScriptOutputBudget : IDisposable
+{
+    public const string ConfigurationKey = "Engine:MaxCapturedOutputChars";
+    // Fits Start Program's two capped 1 MiB streams, including JSON escape expansion.
+    public const int DefaultLimit = 16 * 1024 * 1024;
+    private readonly object _gate = new();
+    private readonly CancellationTokenSource _limitReached = new();
+    private readonly int _limit;
+    private int _captured;
+    private bool _disposed;
+
+    public ScriptOutputBudget(int limit = DefaultLimit)
+    {
+        ArgumentOutOfRangeException.ThrowIfNegativeOrZero(limit);
+        _limit = limit;
+    }
+
+    public CancellationToken LimitReached => _limitReached.Token;
+    public bool Exceeded { get; private set; }
+    public string Error => $"Script output limit of {_limit} characters exceeded ({ConfigurationKey}).";
+    public Buffer CreateBuffer() => new(this);
+
+    public sealed class Buffer(ScriptOutputBudget budget)
+    {
+        private readonly StringBuilder _text = new();
+        public string Text { get { lock (budget._gate) return _text.ToString(); } }
+
+        public void Append(ReadOnlySpan<char> value)
+        {
+            lock (budget._gate)
+            {
+                if (budget._disposed || budget.Exceeded) return;
+                var count = Math.Min(value.Length, budget._limit - budget._captured);
+                _text.Append(value[..count]);
+                budget._captured += count;
+                if (count < value.Length)
+                {
+                    budget.Exceeded = true;
+                    budget._limitReached.Cancel();
+                }
+            }
+        }
+
+        public void AppendLine(string value)
+        {
+            Append(value);
+            Append(Environment.NewLine);
+        }
+    }
+
+    public void Dispose()
+    {
+        lock (_gate)
+        {
+            _disposed = true;
+            _limitReached.Dispose();
+        }
+    }
+}

src/NodePilot.Engine/PowerShell/PowerShellEngineFactory.cs

@@ -1,6 +1,7 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
 using NodePilot.Core.Configuration;
+using NodePilot.Core.Execution;
 
 namespace NodePilot.Engine.PowerShell;
 
@@ -43,8 +44,9 @@ public PowerShellEngineFactory(
         var drainGraceSeconds = configuration?.GetValue<int?>("Engine:IsolatedDrainGraceSeconds") ?? 5;
         var isolatedDrainGrace = drainGraceSeconds > 0 ? TimeSpan.FromSeconds(drainGraceSeconds) : (TimeSpan?)null;
 
-        _pwsh = ProcessExecutionEngine.CreatePwsh(logger, isolatedDrainGrace);
-        _windowsPowerShell = ProcessExecutionEngine.CreateWindowsPowerShell(logger, isolatedDrainGrace);
+        var outputLimit = configuration?.GetValue<int?>(ScriptOutputBudget.ConfigurationKey) ?? ScriptOutputBudget.DefaultLimit;
+        _pwsh = ProcessExecutionEngine.CreatePwsh(logger, isolatedDrainGrace, outputLimit);
+        _windowsPowerShell = ProcessExecutionEngine.CreateWindowsPowerShell(logger, isolatedDrainGrace, outputLimit);
         ProcessExecutionEngine.DeleteOrphanedTempScripts(logger);
 
         // Runspace pool sizing belongs to the process-wide PerformancePlan (hardware-derived,
@@ -56,7 +58,7 @@ public PowerShellEngineFactory(
             new Dictionary<string, int?>());
         var minRunspaces = plan.MinRunspaces.Value;
         var maxRunspaces = plan.MaxRunspaces.Value;
-        _runspace = new RunspaceExecutionEngine(logger, minRunspaces, maxRunspaces);
+        _runspace = new RunspaceExecutionEngine(logger, minRunspaces, maxRunspaces, outputLimit);
 
         logger.LogInformation("PowerShell engines: pwsh={PwshAvailable}, powershell={PSAvailable}, runspace=true (min={Min}, max={Max})",
             _pwsh.IsAvailable, _windowsPowerShell.IsAvailable, minRunspaces, maxRunspaces);

src/NodePilot.Engine/PowerShell/ProcessExecutionEngine.cs

@@ -4,6 +4,7 @@
 using System.Security.Principal;
 using System.Text;
 using Microsoft.Extensions.Logging;
+using NodePilot.Core.Execution;
 using NodePilot.Engine.Security;
 
 namespace NodePilot.Engine.PowerShell;
@@ -17,6 +18,7 @@ public class ProcessExecutionEngine : IPowerShellExecutionEngine
 {
     private readonly string _executable;
     private readonly ILogger _logger;
+    private readonly int _maxCapturedOutputChars;
 
     // Grace period for the stdout/stderr drain AFTER the script process has exited (isolated: and
     // its job tree has been terminated). A read still open then is held by an inherited pipe
@@ -114,25 +116,29 @@ internal static void DeleteOrphanedTempScripts(ILogger logger, string? directory
     // NodePilot.Engine.Tests access; production still uses the CreatePwsh/CreateWindowsPowerShell
     // factory methods.
     internal ProcessExecutionEngine(string engineType, string executable, bool available, ILogger logger,
-        TimeSpan? isolatedDrainGrace = null)
+        TimeSpan? isolatedDrainGrace = null, int maxCapturedOutputChars = ScriptOutputBudget.DefaultLimit)
     {
         EngineType = engineType;
         _executable = executable;
         IsAvailable = available;
         _logger = logger;
+        ArgumentOutOfRangeException.ThrowIfNegativeOrZero(maxCapturedOutputChars);
+        _maxCapturedOutputChars = maxCapturedOutputChars;
         _isolatedDrainGrace = isolatedDrainGrace is { } g && g > TimeSpan.Zero ? g : TimeSpan.FromSeconds(5);
     }
 
-    public static ProcessExecutionEngine CreatePwsh(ILogger logger, TimeSpan? isolatedDrainGrace = null)
+    public static ProcessExecutionEngine CreatePwsh(ILogger logger, TimeSpan? isolatedDrainGrace = null,
+        int maxCapturedOutputChars = ScriptOutputBudget.DefaultLimit)
     {
         var path = FindExecutable("pwsh.exe", "pwsh");
-        return new ProcessExecutionEngine("pwsh", path ?? "pwsh.exe", path is not null, logger, isolatedDrainGrace);
+        return new ProcessExecutionEngine("pwsh", path ?? "pwsh.exe", path is not null, logger, isolatedDrainGrace, maxCapturedOutputChars);
     }
 
-    public static ProcessExecutionEngine CreateWindowsPowerShell(ILogger logger, TimeSpan? isolatedDrainGrace = null)
+    public static ProcessExecutionEngine CreateWindowsPowerShell(ILogger logger, TimeSpan? isolatedDrainGrace = null,
+        int maxCapturedOutputChars = ScriptOutputBudget.DefaultLimit)
     {
         var path = FindExecutable("powershell.exe", "powershell");
-        return new ProcessExecutionEngine("powershell", path ?? "powershell.exe", path is not null, logger, isolatedDrainGrace);
+        return new ProcessExecutionEngine("powershell", path ?? "powershell.exe", path is not null, logger, isolatedDrainGrace, maxCapturedOutputChars);
     }
 
     public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionRequest request, CancellationToken ct)
@@ -181,14 +187,9 @@ public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionReq
             // background program the script started without a new window (Start-Process
             // -NoNewWindow, cmd /c start) inherits the pipes and would hold the step until it exits.
             using var process = new Process { StartInfo = psi, EnableRaisingEvents = true };
-            var stdout = new StringBuilder();
-            var stderr = new StringBuilder();
-            var stdoutClosed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
-            var stderrClosed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+            using var budget = new ScriptOutputBudget(_maxCapturedOutputChars);
             var exited = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
 
-            process.OutputDataReceived += (_, e) => Collect(stdout, stdoutClosed, e.Data);
-            process.ErrorDataReceived += (_, e) => Collect(stderr, stderrClosed, e.Data);
             process.Exited += (_, _) => exited.TrySetResult();
 
             _logger.LogDebug("Starting {Engine}: {File}", EngineType, tempScript);
@@ -200,10 +201,10 @@ public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionReq
             {
                 process.Start();
             }
-            process.BeginOutputReadLine();
-            process.BeginErrorReadLine();
+            var stdout = new OutputCapture(process.StandardOutput, ct, budget.CreateBuffer());
+            var stderr = new OutputCapture(process.StandardError, ct, budget.CreateBuffer());
 
-            using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
+            using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct, budget.LimitReached);
             if (request.Timeout is { } configuredTimeout)
                 cts.CancelAfter(configuredTimeout);
 
@@ -220,11 +221,16 @@ public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionReq
                 // The process is killed either way; only the verdict differs.
                 if (ct.IsCancellationRequested)
                     throw new OperationCanceledException(IPowerShellExecutionEngine.CancelledMessage, ct);
+                if (budget.Exceeded)
+                {
+                    await DrainReadsAsync(stdout, stderr, _isolatedDrainGrace, ct);
+                    return EngineFailure(budget.Error, sw.Elapsed);
+                }
                 return new PowerShellExecutionResult
                 {
                     Success = false,
                     ExitCode = -1,
-                    Output = Snapshot(stdout).TrimEnd(),
+                    Output = stdout.Text.TrimEnd(),
                     Error = $"Script execution timed out after {request.Timeout!.Value.TotalSeconds:0}s",
                     TimedOut = true,
                     Duration = sw.Elapsed,
@@ -233,11 +239,8 @@ public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionReq
 
             // The script has exited, so everything it wrote is in the pipes. A read still open
             // after the grace is held by a background program; keep what has been collected.
-            try
-            {
-                await Task.WhenAll(stdoutClosed.Task, stderrClosed.Task).WaitAsync(_isolatedDrainGrace, ct);
-            }
-            catch (TimeoutException)
+            var drain = await DrainReadsAsync(stdout, stderr, _isolatedDrainGrace, ct);
+            if (drain.DrainTimedOut)
             {
                 _logger.LogDebug(
                     "{Engine} (pid {Pid}): output stayed open {Grace:0}s after the script exited; a background " +
@@ -246,8 +249,9 @@ public async Task<PowerShellExecutionResult> ExecuteAsync(PowerShellExecutionReq
             }
 
             sw.Stop();
-            var stdoutText = Snapshot(stdout);
-            var stderrText = StripSerializedStreams(Snapshot(stderr)).TrimEnd();
+            if (budget.Exceeded) return EngineFailure(budget.Error, sw.Elapsed);
+            var stdoutText = stdout.Text;
+            var stderrText = StripSerializedStreams(stderr.Text).TrimEnd();
             return new PowerShellExecutionResult
             {
                 // Error-based success, as over WinRM (!HadErrors): the script failed if it wrote
@@ -302,11 +306,12 @@ private async Task<PowerShellExecutionResult> ExecuteIsolatedWindowsAsync(PowerS
 
             // Start draining the pipes IMMEDIATELY and concurrently with the wait — a noisy script
             // would otherwise deadlock once the pipe buffers fill (child blocks writing, we block
-            // on exit). Mirrors BeginOutputReadLine on the non-isolated path.
-            var stdoutCapture = new OutputCapture(launched.StandardOutput, ct);
-            var stderrCapture = new OutputCapture(launched.StandardError, ct);
+            // on exit). Both process paths use fixed-size reads, including lines without a newline.
+            using var budget = new ScriptOutputBudget(_maxCapturedOutputChars);
+            var stdoutCapture = new OutputCapture(launched.StandardOutput, ct, budget.CreateBuffer());
+            var stderrCapture = new OutputCapture(launched.StandardError, ct, budget.CreateBuffer());
 
-            using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
+            using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct, budget.LimitReached);
             if (request.Timeout is { } configuredTimeout)
                 cts.CancelAfter(configuredTimeout);
 
@@ -319,7 +324,7 @@ private async Task<PowerShellExecutionResult> ExecuteIsolatedWindowsAsync(PowerS
             catch (OperationCanceledException)
             {
                 userCancel = ct.IsCancellationRequested;
-                timedOut = !userCancel;
+                timedOut = !userCancel && !budget.Exceeded;
             }
 
             // Step-end semantics: the step ends when the ROOT process exits (or we time out /
@@ -364,6 +369,8 @@ private async Task<PowerShellExecutionResult> ExecuteIsolatedWindowsAsync(PowerS
             if (userCancel || ct.IsCancellationRequested)
                 throw new OperationCanceledException(IPowerShellExecutionEngine.CancelledMessage, ct);
 
+            if (budget.Exceeded) return EngineFailure(budget.Error, sw.Elapsed);
+
             if (timedOut)
             {
                 return new PowerShellExecutionResult
@@ -462,13 +469,15 @@ private async Task<PowerShellExecutionResult> ExecuteIsolatedWindowsAsync(PowerS
 
     internal sealed class OutputCapture
     {
-        private readonly StringBuilder _output = new();
+        private readonly ScriptOutputBudget.Buffer _output;
         internal Task Completion { get; }
-        internal string Text => Snapshot(_output);
+        internal string Text => _output.Text;
 
-        internal OutputCapture(TextReader reader, CancellationToken ct)
+        internal OutputCapture(TextReader reader, CancellationToken ct, ScriptOutputBudget.Buffer? output = null)
         {
+            _output = output ?? new ScriptOutputBudget().CreateBuffer();
             Completion = ReadAsync(reader, ct);
+            ObserveAbandonedRead(Completion);
         }
 
         private async Task ReadAsync(TextReader reader, CancellationToken ct)
@@ -477,24 +486,11 @@ private async Task ReadAsync(TextReader reader, CancellationToken ct)
             int count;
             while ((count = await reader.ReadAsync(buffer.AsMemory(), ct).ConfigureAwait(false)) != 0)
             {
-                lock (_output) _output.Append(buffer, 0, count);
+                _output.Append(buffer.AsSpan(0, count));
             }
         }
     }
 
-    // Line callbacks of one stream arrive in order, but a snapshot may be taken while they still
-    // run, hence the lock. A null line is the stream's end.
-    private static void Collect(StringBuilder sink, TaskCompletionSource closed, string? line)
-    {
-        if (line is null) { closed.TrySetResult(); return; }
-        lock (sink) sink.AppendLine(line);
-    }
-
-    private static string Snapshot(StringBuilder sink)
-    {
-        lock (sink) return sink.ToString();
-    }
-
     /// <summary>
     /// The engine never got far enough to have a script result: no exit code, no output, just
     /// the reason. Callers stop the stopwatch first so the reported duration excludes the