Skip to content

Commit 67b774d

Browse files
authored
Bound subprocess execution for git helpers and upgrade installer (#2962)
* Fix bounded git helper subprocess capture (#2832) * Fix bounded upgrade installer launch (#2827) * Isolate git helper timeout override (#2832) * Keep upgrade installer timeout internal (#2827) * Drain git helper output after timeout kill (#2832)
1 parent 8e8cc02 commit 67b774d

6 files changed

Lines changed: 370 additions & 19 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
category: fixed
3+
issues:
4+
- 2827
5+
affected:
6+
- src/CodeIndex/Cli/ProgramRunner.cs
7+
- tests/CodeIndex.Tests/ProgramRunnerTests.cs
8+
---
9+
10+
## English
11+
12+
- **`cdidx upgrade` now launches the installer with `ArgumentList` and a timeout (#2827)** — the upgrade path avoids shell-joined installer arguments and terminates a stalled installer with a clear diagnostic instead of waiting indefinitely.
13+
14+
## 日本語
15+
16+
- **`cdidx upgrade` は installer を `ArgumentList` と timeout 付きで起動するようになりました (#2827)** — upgrade 経路は shell 連結の installer 引数を使わず、停止した installer を明確な診断付きで終了するため、無期限に待ち続けなくなりました。
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
category: fixed
3+
issues:
4+
- 2832
5+
affected:
6+
- src/CodeIndex/Cli/GitHelper.cs
7+
- tests/CodeIndex.Tests/GitHelperTests.cs
8+
---
9+
10+
## English
11+
12+
- **Git helper subprocesses now have bounded runtime and capture size (#2832)** — git helper commands now fail with explicit diagnostics when a git subprocess times out or captured stdout/stderr exceeds the configured cap, preventing hung helpers and unbounded output accumulation.
13+
14+
## 日本語
15+
16+
- **Git helper の subprocess に実行時間とキャプチャサイズの上限を追加しました (#2832)** — git helper コマンドは git subprocess が timeout した場合や stdout/stderr のキャプチャが上限を超えた場合に明示的な診断で失敗するようになり、helper のハングと無制限の出力蓄積を防ぎます。

‎src/CodeIndex/Cli/GitHelper.cs‎

Lines changed: 121 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Diagnostics;
2+
using System.Globalization;
23
using System.Text;
34
using System.Text.RegularExpressions;
45
using CodeIndex.Indexer;
@@ -52,6 +53,18 @@ public sealed record WorktreeStatus(bool IsDirty, IReadOnlyList<string> Unresolv
5253
"UU",
5354
};
5455

56+
internal const int MaxCapturedGitOutputChars = 1024 * 1024;
57+
private static readonly TimeSpan DefaultGitCommandTimeout = TimeSpan.FromSeconds(60);
58+
private static readonly AsyncLocal<TimeSpan?> GitCommandTimeoutOverride = new();
59+
internal static TimeSpan GitCommandTimeout
60+
{
61+
get => GitCommandTimeoutOverride.Value ?? DefaultGitCommandTimeout;
62+
set => GitCommandTimeoutOverride.Value = value;
63+
}
64+
65+
private static readonly TimeSpan GitKillWaitTimeout = TimeSpan.FromSeconds(5);
66+
private const int GitProcessFailureExitCode = -1;
67+
5568
/// <summary>
5669
/// Resolve the common git directory for a project root, handling both normal repos and worktrees.
5770
/// プロジェクトルートの共通gitディレクトリを解決する。通常リポジトリとworktreeの両方に対応。
@@ -696,20 +709,124 @@ private static (int ExitCode, string Output, string Error)? RunProcessCapturingO
696709
using var process = new Process { StartInfo = psi };
697710
var stdout = new StringBuilder();
698711
var stderr = new StringBuilder();
712+
string? failureReason = null;
713+
var failureLock = new object();
714+
715+
void MarkFailure(string reason)
716+
{
717+
lock (failureLock)
718+
{
719+
if (failureReason != null)
720+
return;
721+
failureReason = reason;
722+
}
723+
TryKillProcessTree(process);
724+
}
725+
699726
// Always terminate captured lines with '\n' (not Environment.NewLine) so callers that
700727
// split on '\n' see identical output on Windows and POSIX — git writes LF-only to pipes.
701728
// キャプチャ行は常に '\n' 区切りにし、Windows/POSIX 双方で git のパイプ出力(LF)と一致させる。
702-
process.OutputDataReceived += (_, e) => { if (e.Data != null) stdout.Append(e.Data).Append('\n'); };
703-
process.ErrorDataReceived += (_, e) => { if (e.Data != null) stderr.Append(e.Data).Append('\n'); };
729+
process.OutputDataReceived += (_, e) =>
730+
{
731+
if (e.Data != null)
732+
AppendBoundedCapturedLine(stdout, e.Data, "stdout", MarkFailure);
733+
};
734+
process.ErrorDataReceived += (_, e) =>
735+
{
736+
if (e.Data != null)
737+
AppendBoundedCapturedLine(stderr, e.Data, "stderr", MarkFailure);
738+
};
704739

705740
if (!process.Start())
706741
return null;
707742

708743
process.BeginOutputReadLine();
709744
process.BeginErrorReadLine();
710-
process.WaitForExit();
745+
if (!process.WaitForExit(ToWaitMilliseconds(GitCommandTimeout)))
746+
{
747+
MarkFailure($"git command timed out after {FormatDuration(GitCommandTimeout)}.");
748+
if (!process.WaitForExit(ToWaitMilliseconds(GitKillWaitTimeout)))
749+
return (GitProcessFailureExitCode, ReadCaptured(stdout), CombineCapturedError(ReadCaptured(stderr), failureReason!));
750+
process.WaitForExit();
751+
}
752+
else
753+
{
754+
process.WaitForExit();
755+
}
756+
757+
var output = ReadCaptured(stdout);
758+
var error = ReadCaptured(stderr);
759+
if (failureReason != null)
760+
return (GitProcessFailureExitCode, output, CombineCapturedError(error, failureReason));
711761

712-
return (process.ExitCode, stdout.ToString(), stderr.ToString());
762+
return (process.ExitCode, output, error);
763+
}
764+
765+
private static string ReadCaptured(StringBuilder builder)
766+
{
767+
lock (builder)
768+
return builder.ToString();
769+
}
770+
771+
private static void AppendBoundedCapturedLine(
772+
StringBuilder builder,
773+
string data,
774+
string streamName,
775+
Action<string> markFailure)
776+
{
777+
lock (builder)
778+
{
779+
var remaining = MaxCapturedGitOutputChars - builder.Length;
780+
if (remaining <= 0)
781+
{
782+
markFailure(BuildCaptureLimitMessage(streamName));
783+
return;
784+
}
785+
786+
var required = data.Length + 1;
787+
if (required <= remaining)
788+
{
789+
builder.Append(data).Append('\n');
790+
return;
791+
}
792+
793+
builder.Append(data.AsSpan(0, Math.Min(data.Length, remaining)));
794+
}
795+
796+
markFailure(BuildCaptureLimitMessage(streamName));
797+
}
798+
799+
private static string BuildCaptureLimitMessage(string streamName)
800+
=> $"git command captured {streamName} exceeded {MaxCapturedGitOutputChars.ToString(CultureInfo.InvariantCulture)} characters.";
801+
802+
private static string CombineCapturedError(string stderr, string diagnostic)
803+
=> string.IsNullOrWhiteSpace(stderr)
804+
? diagnostic
805+
: stderr.TrimEnd('\r', '\n') + "\n" + diagnostic;
806+
807+
private static int ToWaitMilliseconds(TimeSpan timeout)
808+
{
809+
if (timeout <= TimeSpan.Zero)
810+
return 1;
811+
if (timeout.TotalMilliseconds >= int.MaxValue)
812+
return int.MaxValue;
813+
return Math.Max(1, (int)Math.Ceiling(timeout.TotalMilliseconds));
814+
}
815+
816+
private static string FormatDuration(TimeSpan timeout)
817+
=> timeout.TotalSeconds.ToString("0.###", CultureInfo.InvariantCulture) + "s";
818+
819+
private static void TryKillProcessTree(Process process)
820+
{
821+
try
822+
{
823+
if (!process.HasExited)
824+
process.Kill(entireProcessTree: true);
825+
}
826+
catch
827+
{
828+
// Best-effort cleanup only; callers receive the timeout/capture diagnostic.
829+
}
713830
}
714831

715832
private static bool ProbeFileSystemIgnoreCase(string projectRoot)

‎src/CodeIndex/Cli/ProgramRunner.cs‎

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ internal static class ProgramRunner
2020
internal const string QuietEnvironmentVariable = "CDIDX_QUIET";
2121
private const string InstallerScriptUrlTemplate = "https://raw.githubusercontent.com/Widthdom/CodeIndex/{0}/install.sh";
2222
private const long MaxInstallerScriptBytes = 1024 * 1024;
23+
private static readonly TimeSpan InstallerRunTimeout = TimeSpan.FromMinutes(5);
24+
private static readonly TimeSpan InstallerKillWaitTimeout = TimeSpan.FromSeconds(5);
2325
internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System;
2426

2527
internal static int Run(
@@ -2209,19 +2211,8 @@ internal static int RunUpgrade(string[] cmdArgs, JsonSerializerOptions jsonOptio
22092211
.GetResult();
22102212
}
22112213

2212-
var startInfo = new ProcessStartInfo("bash", $"{QuoteShellArg(scriptPath)} {QuoteShellArg(result.LatestVersion)}")
2213-
{
2214-
UseShellExecute = false,
2215-
};
2216-
startInfo.Environment["CDIDX_INSTALL_DIR"] = installDir;
2217-
var process = Process.Start(startInfo);
2218-
if (process == null)
2219-
{
2220-
Console.Error.WriteLine("Error: failed to start install.sh for upgrade.");
2221-
return CommandExitCodes.DatabaseError;
2222-
}
2223-
process.WaitForExit();
2224-
return process.ExitCode;
2214+
var startInfo = CreateInstallerProcessStartInfo(scriptPath, result.LatestVersion, installDir);
2215+
return RunInstallerProcess(startInfo, InstallerRunTimeout);
22252216
}
22262217
catch (Exception ex)
22272218
{
@@ -2235,6 +2226,40 @@ internal static int RunUpgrade(string[] cmdArgs, JsonSerializerOptions jsonOptio
22352226
}
22362227
}
22372228

2229+
internal static ProcessStartInfo CreateInstallerProcessStartInfo(string scriptPath, string releaseTag, string installDir)
2230+
{
2231+
var startInfo = new ProcessStartInfo
2232+
{
2233+
FileName = "bash",
2234+
UseShellExecute = false,
2235+
};
2236+
startInfo.ArgumentList.Add(scriptPath);
2237+
startInfo.ArgumentList.Add(releaseTag);
2238+
startInfo.Environment["CDIDX_INSTALL_DIR"] = installDir;
2239+
return startInfo;
2240+
}
2241+
2242+
internal static int RunInstallerProcess(ProcessStartInfo startInfo, TimeSpan timeout)
2243+
{
2244+
using var process = Process.Start(startInfo);
2245+
if (process == null)
2246+
{
2247+
Console.Error.WriteLine("Error: failed to start install.sh for upgrade.");
2248+
return CommandExitCodes.DatabaseError;
2249+
}
2250+
2251+
if (process.WaitForExit(ToWaitMilliseconds(timeout)))
2252+
return process.ExitCode;
2253+
2254+
TryKillProcessTree(process);
2255+
if (!process.WaitForExit(ToWaitMilliseconds(InstallerKillWaitTimeout)))
2256+
Console.Error.WriteLine("Error: install.sh timed out and did not exit after cancellation.");
2257+
else
2258+
Console.Error.WriteLine($"Error: install.sh timed out after {FormatDuration(timeout)}.");
2259+
Console.Error.WriteLine("Hint: rerun `install.sh` manually for the desired release.");
2260+
return CommandExitCodes.DatabaseError;
2261+
}
2262+
22382263
internal static string BuildInstallerScriptUrl(string releaseTag)
22392264
=> string.Format(
22402265
CultureInfo.InvariantCulture,
@@ -2279,8 +2304,30 @@ private static bool CanWriteDirectory(string directory)
22792304
}
22802305
}
22812306

2282-
private static string QuoteShellArg(string value)
2283-
=> "'" + value.Replace("'", "'\\''", StringComparison.Ordinal) + "'";
2307+
private static int ToWaitMilliseconds(TimeSpan timeout)
2308+
{
2309+
if (timeout <= TimeSpan.Zero)
2310+
return 1;
2311+
if (timeout.TotalMilliseconds >= int.MaxValue)
2312+
return int.MaxValue;
2313+
return Math.Max(1, (int)Math.Ceiling(timeout.TotalMilliseconds));
2314+
}
2315+
2316+
private static string FormatDuration(TimeSpan timeout)
2317+
=> timeout.TotalSeconds.ToString("0.###", CultureInfo.InvariantCulture) + "s";
2318+
2319+
private static void TryKillProcessTree(Process process)
2320+
{
2321+
try
2322+
{
2323+
if (!process.HasExited)
2324+
process.Kill(entireProcessTree: true);
2325+
}
2326+
catch
2327+
{
2328+
// Best-effort cleanup only; callers receive the timeout diagnostic.
2329+
}
2330+
}
22842331

22852332
// `--version` is now build-aware so dev builds from main are not
22862333
// indistinguishable from tagged releases in bug reports (#1550). Human

0 commit comments

Comments
 (0)