diff --git a/README.md b/README.md index 2f13f8e2b9..9a7f96eabe 100644 --- a/README.md +++ b/README.md @@ -172,7 +172,7 @@ downgrading `cdidx`. ### Upgrade and uninstall -`cdidx upgrade --check-only` reports whether a newer GitHub release is available. `cdidx upgrade` downloads the current `install.sh`, refuses unwritable install directories, and reruns the installer with `CDIDX_INSTALL_DIR` pointed at the current binary directory. +`cdidx upgrade --check-only` reports whether a newer GitHub release is available. `cdidx upgrade` downloads `install.sh` from the resolved release tag, refuses unwritable install directories, and reruns the installer with `CDIDX_INSTALL_DIR` pointed at the current binary directory. Direct `install.sh` installs can be removed with: diff --git a/changelog.d/unreleased/2816.security.md b/changelog.d/unreleased/2816.security.md new file mode 100644 index 0000000000..3dec876939 --- /dev/null +++ b/changelog.d/unreleased/2816.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 2816 +affected: + - src/CodeIndex/Cli/ProgramRunner.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs + - README.md +--- + +## English + +- **`cdidx upgrade` now pins `install.sh` to the resolved release tag (#2816)** — upgrades fetch the installer from the release tag reported by GitHub instead of mutable `main`, so installer behavior matches the release being installed. + +## 日本語 + +- **`cdidx upgrade` が `install.sh` を解決済み release tag に固定するようになりました (#2816)** — upgrade 時は mutable な `main` ではなく GitHub が返した release tag から installer を取得するため、インストール対象 release と installer の挙動が一致します。 diff --git a/changelog.d/unreleased/2875.security.md b/changelog.d/unreleased/2875.security.md new file mode 100644 index 0000000000..61ed10ee63 --- /dev/null +++ b/changelog.d/unreleased/2875.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 2875 +affected: + - src/CodeIndex/Cli/BoundedHttpContentReader.cs + - src/CodeIndex/Cli/ProgramRunner.cs + - tests/CodeIndex.Tests/BoundedHttpContentReaderTests.cs +--- + +## English + +- **`cdidx upgrade` now bounds and privately writes installer downloads (#2875)** — installer responses are streamed with an explicit byte limit and temporary scripts are created with owner-only permissions on POSIX systems. + +## 日本語 + +- **`cdidx upgrade` の installer download に上限と private 書き込みを追加しました (#2875)** — installer response は明示的な byte 上限付きで streaming され、POSIX では一時 script を owner-only 権限で作成します。 diff --git a/changelog.d/unreleased/2907.security.md b/changelog.d/unreleased/2907.security.md new file mode 100644 index 0000000000..314be993c2 --- /dev/null +++ b/changelog.d/unreleased/2907.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 2907 +affected: + - src/CodeIndex/Cli/UpdateChecker.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Update checks now bound GitHub release responses before JSON parsing (#2907)** — the latest-release response is read with a byte limit and parsed with a JSON depth limit before extracting `tag_name`. + +## 日本語 + +- **update check が GitHub release response を JSON parse 前に制限するようになりました (#2907)** — latest-release response は `tag_name` 抽出前に byte 上限付きで読み込まれ、JSON depth 上限付きで parse されます。 diff --git a/src/CodeIndex/Cli/BoundedHttpContentReader.cs b/src/CodeIndex/Cli/BoundedHttpContentReader.cs new file mode 100644 index 0000000000..adf7576b77 --- /dev/null +++ b/src/CodeIndex/Cli/BoundedHttpContentReader.cs @@ -0,0 +1,112 @@ +using System.Buffers; + +namespace CodeIndex.Cli; + +internal static class BoundedHttpContentReader +{ + private const int BufferSize = 81920; + + internal static async Task ReadAsByteArrayAsync( + HttpContent content, + long maxBytes, + CancellationToken cancellationToken) + { + ValidateMaxBytes(maxBytes); + ThrowIfContentLengthExceedsLimit(content, maxBytes); + + await using var source = await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + using var destination = CreateMemoryStream(content); + await CopyToAsync(source, destination, maxBytes, cancellationToken).ConfigureAwait(false); + return destination.ToArray(); + } + + internal static async Task WriteToPrivateFileAsync( + HttpContent content, + string path, + long maxBytes, + CancellationToken cancellationToken) + { + ValidateMaxBytes(maxBytes); + ThrowIfContentLengthExceedsLimit(content, maxBytes); + + await using var source = await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + await using var destination = CreatePrivateFileStream(path); + await CopyToAsync(source, destination, maxBytes, cancellationToken).ConfigureAwait(false); + } + + private static MemoryStream CreateMemoryStream(HttpContent content) + { + if (content.Headers.ContentLength is long contentLength + && contentLength > 0 + && contentLength <= int.MaxValue) + { + return new MemoryStream((int)contentLength); + } + + return new MemoryStream(); + } + + private static FileStream CreatePrivateFileStream(string path) + { + var options = new FileStreamOptions + { + Mode = FileMode.CreateNew, + Access = FileAccess.Write, + Share = FileShare.None, + BufferSize = BufferSize, + Options = FileOptions.SequentialScan, + }; + + if (!OperatingSystem.IsWindows()) + options.UnixCreateMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; + + return new FileStream(path, options); + } + + private static async Task CopyToAsync( + Stream source, + Stream destination, + long maxBytes, + CancellationToken cancellationToken) + { + var buffer = ArrayPool.Shared.Rent(BufferSize); + try + { + long copied = 0; + while (true) + { + var remaining = maxBytes - copied; + var readLimit = remaining >= buffer.Length ? buffer.Length : (int)remaining + 1; + var read = await source.ReadAsync(buffer.AsMemory(0, readLimit), cancellationToken).ConfigureAwait(false); + if (read == 0) + return; + + if (read > maxBytes - copied) + throw CreateExceededLimitException(maxBytes); + + await destination.WriteAsync(buffer.AsMemory(0, read), cancellationToken).ConfigureAwait(false); + copied += read; + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } + } + + private static void ThrowIfContentLengthExceedsLimit(HttpContent content, long maxBytes) + { + var contentLength = content.Headers.ContentLength; + if (contentLength.HasValue && contentLength.Value > maxBytes) + throw CreateExceededLimitException(maxBytes); + } + + private static InvalidDataException CreateExceededLimitException(long maxBytes) + => new($"HTTP response body exceeded the {maxBytes} byte limit."); + + private static void ValidateMaxBytes(long maxBytes) + { + if (maxBytes < 0) + throw new ArgumentOutOfRangeException(nameof(maxBytes), maxBytes, "The byte limit must be non-negative."); + } +} diff --git a/src/CodeIndex/Cli/ProgramRunner.cs b/src/CodeIndex/Cli/ProgramRunner.cs index 6a439d799f..06c19b59cc 100644 --- a/src/CodeIndex/Cli/ProgramRunner.cs +++ b/src/CodeIndex/Cli/ProgramRunner.cs @@ -17,6 +17,8 @@ namespace CodeIndex.Cli; internal static class ProgramRunner { internal const string QuietEnvironmentVariable = "CDIDX_QUIET"; + private const string InstallerScriptUrlTemplate = "https://raw.githubusercontent.com/Widthdom/CodeIndex/{0}/install.sh"; + private const long MaxInstallerScriptBytes = 1024 * 1024; internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; internal static int Run( @@ -2176,10 +2178,14 @@ internal static int RunUpgrade(string[] cmdArgs, JsonSerializerOptions jsonOptio { using (var client = new HttpClient { Timeout = TimeSpan.FromSeconds(20) }) { - var script = client.GetStringAsync("https://raw.githubusercontent.com/Widthdom/CodeIndex/main/install.sh") + DownloadInstallerScriptAsync( + client, + result.LatestVersion, + scriptPath, + TimeSpan.FromSeconds(20), + CancellationToken.None) .GetAwaiter() .GetResult(); - File.WriteAllText(scriptPath, script); } var startInfo = new ProcessStartInfo("bash", $"{QuoteShellArg(scriptPath)} {QuoteShellArg(result.LatestVersion)}") @@ -2208,6 +2214,34 @@ internal static int RunUpgrade(string[] cmdArgs, JsonSerializerOptions jsonOptio } } + internal static string BuildInstallerScriptUrl(string releaseTag) + => string.Format( + CultureInfo.InvariantCulture, + InstallerScriptUrlTemplate, + Uri.EscapeDataString(releaseTag.Trim())); + + internal static async Task DownloadInstallerScriptAsync( + HttpClient client, + string releaseTag, + string scriptPath, + TimeSpan timeout, + CancellationToken cancellationToken) + { + using var downloadCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + downloadCts.CancelAfter(timeout); + using var request = new HttpRequestMessage(HttpMethod.Get, BuildInstallerScriptUrl(releaseTag)); + using var response = await client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + downloadCts.Token).ConfigureAwait(false); + response.EnsureSuccessStatusCode(); + await BoundedHttpContentReader.WriteToPrivateFileAsync( + response.Content, + scriptPath, + MaxInstallerScriptBytes, + downloadCts.Token).ConfigureAwait(false); + } + private static bool CanWriteDirectory(string directory) { try diff --git a/src/CodeIndex/Cli/UpdateChecker.cs b/src/CodeIndex/Cli/UpdateChecker.cs index f04026a287..68f2e9e603 100644 --- a/src/CodeIndex/Cli/UpdateChecker.cs +++ b/src/CodeIndex/Cli/UpdateChecker.cs @@ -9,6 +9,8 @@ internal static class UpdateChecker { internal const string DisableEnvVar = "CDIDX_DISABLE_UPDATE_CHECK"; private const string LatestReleaseUrl = "https://api.github.com/repos/Widthdom/CodeIndex/releases/latest"; + internal const long MaxLatestReleaseResponseBytes = 64 * 1024; + internal const int MaxLatestReleaseJsonDepth = 16; private static readonly TimeSpan CacheTtl = TimeSpan.FromHours(24); private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(2); @@ -123,17 +125,40 @@ private static string FormatHint(string latestTag) private static async Task FetchLatestReleaseTagAsync(CancellationToken cancellationToken) { - using var client = new HttpClient { Timeout = RequestTimeout }; + using var client = new HttpClient { Timeout = Timeout.InfiniteTimeSpan }; + return await FetchLatestReleaseTagAsync(client, RequestTimeout, cancellationToken).ConfigureAwait(false); + } + + internal static async Task FetchLatestReleaseTagAsync( + HttpClient client, + TimeSpan timeout, + CancellationToken cancellationToken) + { + using var requestCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + requestCts.CancelAfter(timeout); using var request = new HttpRequestMessage(HttpMethod.Get, LatestReleaseUrl); request.Headers.UserAgent.Add(new ProductInfoHeaderValue("cdidx", ConsoleUi.LoadVersion())); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json")); - using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false); + using var response = await client.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + requestCts.Token).ConfigureAwait(false); if (!response.IsSuccessStatusCode) return null; - await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var doc = await JsonDocument.ParseAsync(stream, cancellationToken: cancellationToken).ConfigureAwait(false); + return await ReadLatestReleaseTagAsync(response.Content, requestCts.Token).ConfigureAwait(false); + } + + internal static async Task ReadLatestReleaseTagAsync(HttpContent content, CancellationToken cancellationToken) + { + var payload = await BoundedHttpContentReader.ReadAsByteArrayAsync( + content, + MaxLatestReleaseResponseBytes, + cancellationToken).ConfigureAwait(false); + using var doc = JsonDocument.Parse( + payload.AsMemory(), + new JsonDocumentOptions { MaxDepth = MaxLatestReleaseJsonDepth }); return doc.RootElement.TryGetProperty("tag_name", out var tag) ? tag.GetString() : null; diff --git a/tests/CodeIndex.Tests/BoundedHttpContentReaderTests.cs b/tests/CodeIndex.Tests/BoundedHttpContentReaderTests.cs new file mode 100644 index 0000000000..874d9912c2 --- /dev/null +++ b/tests/CodeIndex.Tests/BoundedHttpContentReaderTests.cs @@ -0,0 +1,101 @@ +using System.Net; +using System.Text; +using CodeIndex.Cli; + +namespace CodeIndex.Tests; + +public class BoundedHttpContentReaderTests +{ + [Fact] + public async Task WriteToPrivateFileAsync_WritesContentWithOwnerOnlyMode() + { + var path = Path.Combine(Path.GetTempPath(), $"cdidx-install-test-{Guid.NewGuid():N}.sh"); + try + { + await BoundedHttpContentReader.WriteToPrivateFileAsync( + new UnknownLengthContent(Encoding.UTF8.GetBytes("#!/bin/sh\nexit 0\n")), + path, + maxBytes: 1024, + CancellationToken.None); + + Assert.Equal("#!/bin/sh\nexit 0\n", File.ReadAllText(path)); + if (!OperatingSystem.IsWindows()) + { + var permissions = File.GetUnixFileMode(path) & PermissionBits; + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, permissions); + } + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + [Fact] + public async Task WriteToPrivateFileAsync_RejectsStreamOverLimit() + { + var path = Path.Combine(Path.GetTempPath(), $"cdidx-install-test-{Guid.NewGuid():N}.sh"); + try + { + var ex = await Assert.ThrowsAsync(() => + BoundedHttpContentReader.WriteToPrivateFileAsync( + new UnknownLengthContent(Encoding.UTF8.GetBytes("12345")), + path, + maxBytes: 4, + CancellationToken.None)); + + Assert.Contains("4 byte limit", ex.Message); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + [Fact] + public async Task ReadAsByteArrayAsync_RejectsDeclaredLengthOverLimit() + { + using var content = new ByteArrayContent([1]); + content.Headers.ContentLength = 5; + + var ex = await Assert.ThrowsAsync(() => + BoundedHttpContentReader.ReadAsByteArrayAsync(content, maxBytes: 4, CancellationToken.None)); + + Assert.Contains("4 byte limit", ex.Message); + } + + private const UnixFileMode PermissionBits = + UnixFileMode.UserRead + | UnixFileMode.UserWrite + | UnixFileMode.UserExecute + | UnixFileMode.GroupRead + | UnixFileMode.GroupWrite + | UnixFileMode.GroupExecute + | UnixFileMode.OtherRead + | UnixFileMode.OtherWrite + | UnixFileMode.OtherExecute; + + private sealed class UnknownLengthContent : HttpContent + { + private readonly byte[] _payload; + + internal UnknownLengthContent(byte[] payload) + { + _payload = payload; + } + + protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context) + => stream.WriteAsync(_payload, 0, _payload.Length); + + protected override Task CreateContentReadStreamAsync() + => Task.FromResult(new MemoryStream(_payload, writable: false)); + + protected override bool TryComputeLength(out long length) + { + length = 0; + return false; + } + } +} diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index a3600b0c3f..f4279f8a6b 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -1,3 +1,5 @@ +using System.Net; +using System.Text; using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; @@ -305,6 +307,85 @@ public void UpdateChecker_Check_ReportsNewerRelease() } } + [Theory] + [InlineData("v1.26.0", "https://raw.githubusercontent.com/Widthdom/CodeIndex/v1.26.0/install.sh")] + [InlineData(" release/test ", "https://raw.githubusercontent.com/Widthdom/CodeIndex/release%2Ftest/install.sh")] + public void BuildInstallerScriptUrl_UsesResolvedReleaseTag(string releaseTag, string expected) + { + Assert.Equal(expected, ProgramRunner.BuildInstallerScriptUrl(releaseTag)); + } + + [Fact] + public async Task DownloadInstallerScriptAsync_CancelsStalledBody() + { + var path = Path.Combine(Path.GetTempPath(), $"cdidx-install-timeout-{Guid.NewGuid():N}.sh"); + using var client = new HttpClient(new StaticResponseHandler(new StalledContent())) + { + Timeout = Timeout.InfiniteTimeSpan, + }; + try + { + await Assert.ThrowsAnyAsync(() => + ProgramRunner.DownloadInstallerScriptAsync( + client, + "v1.27.0", + path, + TimeSpan.FromMilliseconds(25), + CancellationToken.None)); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + [Fact] + public async Task UpdateChecker_ReadLatestReleaseTagAsync_ParsesTagName() + { + using var content = new ByteArrayContent(Encoding.UTF8.GetBytes("""{"tag_name":"v1.27.0"}""")); + + var tag = await UpdateChecker.ReadLatestReleaseTagAsync(content, CancellationToken.None); + + Assert.Equal("v1.27.0", tag); + } + + [Fact] + public async Task UpdateChecker_ReadLatestReleaseTagAsync_RejectsOverLimitResponse() + { + using var content = new ByteArrayContent(new byte[(int)UpdateChecker.MaxLatestReleaseResponseBytes + 1]); + + var ex = await Assert.ThrowsAsync(() => + UpdateChecker.ReadLatestReleaseTagAsync(content, CancellationToken.None)); + + Assert.Contains($"{UpdateChecker.MaxLatestReleaseResponseBytes} byte limit", ex.Message); + } + + [Fact] + public async Task UpdateChecker_ReadLatestReleaseTagAsync_RejectsDeepJson() + { + var depth = UpdateChecker.MaxLatestReleaseJsonDepth + 8; + using var content = new ByteArrayContent(Encoding.UTF8.GetBytes(new string('[', depth) + new string(']', depth))); + + await Assert.ThrowsAnyAsync(() => + UpdateChecker.ReadLatestReleaseTagAsync(content, CancellationToken.None)); + } + + [Fact] + public async Task UpdateChecker_FetchLatestReleaseTagAsync_CancelsStalledBody() + { + using var client = new HttpClient(new StaticResponseHandler(new StalledContent())) + { + Timeout = Timeout.InfiniteTimeSpan, + }; + + await Assert.ThrowsAnyAsync(() => + UpdateChecker.FetchLatestReleaseTagAsync( + client, + TimeSpan.FromMilliseconds(25), + CancellationToken.None)); + } + [Theory] [InlineData("~/cdidx-logs", "cdidx-logs")] [InlineData("$HOME/cdidx-logs", "cdidx-logs")] @@ -1513,4 +1594,67 @@ public void TryConsumeAuditLogFlags_DbValueLooksLikeAuditFlag_PreservedAsDbValue Assert.Null(options.Path); Assert.Equal(new[] { "--db", "--audit-log" }, args); } + + private sealed class StaticResponseHandler : HttpMessageHandler + { + private readonly HttpContent _content; + + internal StaticResponseHandler(HttpContent content) + { + _content = content; + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = _content }); + } + + private sealed class StalledContent : HttpContent + { + protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context) + => Task.CompletedTask; + + protected override Task CreateContentReadStreamAsync() + => Task.FromResult(new StalledStream()); + + protected override bool TryComputeLength(out long length) + { + length = 0; + return false; + } + } + + private sealed class StalledStream : Stream + { + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override void Flush() + { + } + + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + return 0; + } + + public override long Seek(long offset, SeekOrigin origin) + => throw new NotSupportedException(); + + public override void SetLength(long value) + => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + } }